[PATCH 4/5] mtd: m25p80: remove M25PXX_USE_FAST_READ Kconfig

[Brian Norris]

On Thu, Oct 24, 2013 at 02:42:53PM +0530, Sourav Poddar wrote:
> On Thursday 24 October 2013 02:37 PM, Sourav Poddar wrote:
> >On Thursday 24 October 2013 08:28 AM, Brian Norris wrote:
> >>--- a/drivers/mtd/devices/m25p80.c
> >>+++ b/drivers/mtd/devices/m25p80.c
> >>@@ -1055,13 +1055,14 @@ static int m25p_probe(struct spi_device *spi)
> >>      flash->page_size = info->page_size;
> >>      flash->mtd.writebufsize = flash->page_size;
> >>
> >>-    flash->fast_read = false;
> >>-    if (np&&  of_property_read_bool(np, "m25p,fast-read"))
> >>+    if (np)
> >>+        /* If we were instantiated by DT, use it */
> >>+        flash->fast_read = of_property_read_bool(np, "m25p,fast-read");
> >>+    else
> >>+        /* If we weren't instantiated by DT, default to fast-read */
> >>          flash->fast_read = true;
> >>
> >This comment is in sync with my quad read mode support patch on
> >the mtd list.
> >
> >Here, you are defaulting the fast read to be true.

It was already default true for non-DT case where
M25PXX_USE_FAST_READ=y. I believe my code is purely a refactoring of the
code, assuming M25PXX_USE_FAST_READ is always enabled (and removing the
Kconfig).

> >Once I add quad mode
> >on top of this, I will set flash->quad_read = true. So, we will have both
> >fast and quad read set(which will not be correct). So, it is
> >necessary to default to fast read ?

I think my patch is a correct refactoring by itself. Any problem your
quad read patch has with it would still be a problem without this patch.

So, without quad-read support, we should default to fast-read unless the
chip doesn't support it. I think this is reasonable.

Now, once you add quad-read, you need to have quad-read override
fast-read.

> Though, we will hit this scenario only for a non dt case. For dt
> case, things will be fine.

Yes, but we need to make sure things make sense for all cases.

> >>-#ifdef CONFIG_M25PXX_USE_FAST_READ
> >>-    flash->fast_read = true;
> >>-#endif
> >>+    /* Some devices cannot do fast-read, no matter what DT tells us */
> >>      if (info->flags&  M25P_NO_FR)
> >>          flash->fast_read = false;
> >>
> >

I just realized this: fast-read and quad-read are mutually exclusive, so
the field that is currently 'bool m25p.fast_read' could easily just become
an 'enum m25p.read_type' (with values M25P_READ_NORMAL, M25P_READ_FAST,
M25P_READ_QUAD). That may or may not help your quad read patch.

I can delay this patch until others have looked at it, if you'd like.
You can resubmit your quad read patch without this patch 4. (But I think
this patch will help clean up your patch slightly.)

Brian