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

Brian Norris
2014-09-10T07:02:56Z

On Tue, Aug 12, 2014 at 09:16:07AM +0800, Huang Shijie wrote:
> 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{} ?

Probably.

> > 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.

Yeah. The point of this series is that without users, the current
callback is both ambiguously defined (it was allegedly serving both a
flash-specific and a controller-specific purpose) and unncecessary. It's
easier to add back once we have a clear purpose.

Brian