[PATCH] mtd: fsl-quadspi: Fix module unbound

Brian Norris computersforpeace at gmail.com
Tue Jan 6 16:43:16 PST 2015


On Tue, Jan 06, 2015 at 10:52:00AM -0200, Fabio Estevam wrote:
> On Tue, Jan 6, 2015 at 4:49 AM, Brian Norris
> <computersforpeace at gmail.com> wrote:
> 
> > Huh? Why was this property even needed in the first place? It seems
> > oddly specific, without actually being very explanatory/descriptive.
> 
> Huang, care to comment as you were the one that added it?
> 
> >
> >> -             has_second_chip = true;
> >> +             q->has_second_chip = true;
> >>
> >>       /* iterate the subnodes. */
> >>       for_each_available_child_of_node(dev->of_node, np) {
> >>               char modalias[40];
> >>
> >>               /* skip the holes */
> >> -             if (!has_second_chip)
> >> +             if (!q->has_second_chip)
> >>                       i *= 2;
> >
> > Why do you need to "skip" anything here? It doesn't really look like
> > you're skpping anything functionally, as this indexing is purely
> > artificial. So you're just jumping through hoops for no reason.
> >
> > Can this just be more straightforward by dropping 'has_second_chip' and
> > indexing straightforwardly? e.g. this patch:
> 
> With your proposed patch I get probe failure:
> 
> root at freescale /$ dmesg | grep qspi
> [    1.688344] fsl-quadspi 21e4000.qspi: s25fl128s (16384 Kbytes)
> [    1.698146] fsl-quadspi 21e4000.qspi: unrecognized JEDEC id bytes: ff, ff, ff
> [    1.705350] fsl-quadspi 21e4000.qspi: Freescale QuadSPI probe failed

I think there was one more subtle piece to this driver; it relies on the
implicit layout of the nor[] array for determining a MMIO offset in
fsl_qspi_set_base_addr(). It seems like it would be clearer to avoid
this pointer subtraction.

Brian



More information about the linux-mtd mailing list