Re: [PATCH 6/8] mtd: spi-nor: drop replaceable wait-till-ready function pointer

Huang Shijie
2014-08-12T01:16:26Z

On Mon, Aug 11, 2014 at 11:43:11AM -0700, Brian Norris wrote:
> On Sat, Aug 09, 2014 at 05:53:03PM +0800, Huang Shijie wrote:
> > On Wed, Aug 06, 2014 at 06:17:00PM -0700, Brian Norris wrote:
> > > We don't need to expose a 'wait-till-ready' interface to drivers. Status
> > > register polling should be handled by the core spi-nor.c library, and as
> > > of now, I see no need to provide a special driver-specific hook for it.
> >
> > Please do not drop this hook, please see why we add this hook:
> > http://lists.infradead.org/pipermail/linux-mtd/2013-December/050561.html
> >
> > "
> > The default implementation would do just as you suggest, and I would
> > expect most H/W drivers to use the default implementation. However, some H/W
> > Controllers can automate the polling of status registers, so having a hook allows
> > the driver override the default behaviour.
> > "
> >
> > The spi-nor framework will used by more drivers besides the m25p80 and
> > fsl-quadspi. Some NOR flash controller may be very strange.
>
> OK, but the wait-till-ready hook should not look like the current one.
> If it is used as an optimization of sorts, then we can provide it in
> *addition* to the checks we do, but not *instead of*. I sincerely doubt
> that any specialized SPI NOR controller will know how to check both SR
> and FSR where appropriate, and it probably won't understand other
> specialized sequences as they are developed / thrown on our lap by flash
> vendors.
>
> So, the spi-nor.c "wait until ready" might have a sequence like this:
>
> 1. (Optionally) use low-level driver's "wait" function; skip if not
> present
>
> 2. use the read register hooks to check SR/FSR to confirm completion
>
> I do not want to toss #2 into the low-level driver, so if there is a
> need for #1, it should be done in addition to our spi-nor.c code, not
> instead. (To this end, I believe Marek also complained about this when
> we were adding the FSR-checking code; we should not have drivers and
> spi-nor.c fighting over callback functions.)
>
> So I'm inclined to at most address #1 through an optional callback, and

but where to put the optional callback? in the spi_nor{} ?
> possibly even to ignore that until we see an actual driver that needs
> it.
If you insist to remove it, i will be okay too.
anyway, we can add it back when an actual driver appears.

thanks
Huang Shijie