[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