Re: [PATCH 0/1] Fix OMAP2 NAND ONFI device detection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 06, 2013 at 08:54:56PM +0000, Gupta, Pekon wrote:
> > From: Ezequiel Garcia [mailto:ezequiel.garcia@xxxxxxxxxxxxxxxxxx]
> > > On Wed, Oct 30, 2013 at 09:18:53PM +0000, Gupta, Pekon wrote:
> > > >
> > > > I'm not sure, of course, but I don't see why not. It's more likely to
> > > > break for x16 than it is for x8.
> > > >
> > > Another question here is ..
> > > The above patch assumes that user has configured GPMC bus-width
> > > correctly, so if user is already providing GPMC bus-width information
> > > via DT, then do we really need to detect NAND device bus-width again
> > > using NAND_BUSWIDTH_AUTO ?
> > >
> > 
> > Hm.. I think you're forgetting the original motivation for this patch,
> > which was initially explained by you :-) The problem is ONFI doesn't work
> > in 16-bit mode.
> > 
> > Let me clarify. Since GPMC and NAND widths *must* match (otherwise, it
> > wouldn't make much sense, right?) we don't really need to autodetect the
> > NAND width.
> > 
> > However, since ONFI doesn't work in 16-bit mode, we would have to do
> > something like this (untested pseudo code):
> > 
> >   nand_scan_ident(...);
> > 
> >   if (platform_data->devsize == 16)
> >   	nand_chip->options |= NAND_BUSWIDTH_16;
> > 
> > And in some cases we might even get away with such solution. However,
> > we can't guarantee nand_scan_ident() doesn't need to swith to 16-bit mode
> > inside to perform other commands (maybe some ONFI extended parameter
> > page).
> > 
> > Therefore, and given the width can be autodetected in any case (array
> > or ONFI based carry such information) it's much cleaner to simply do:
> > 
> >   nand_chip->options |= NAND_BUSWIDTH_AUTOMAGIC;
> >   nand_scan_ident(...);
> > 
> > See? Much cleaner, no?
> > 
> but still dependent on DT property for GPMC configurations...
> I preferred NAND bus-width detection to be completely independent
> of 'any' DT property.
> 

I think we're still mixing GPMC and NAND, and I don't think it's sane.

> > And remember: the fact that we must know the device width to configure
> > the GPMC correctly (which being a hardware parameter belongs to the DT)
> > is OMAP specific. Other SoCs might not have such memory controller
> > and thus won't need such knowledge before hand, although that sounds
> > unlikely to me.
> > 
> > The outcome of this discussion seems to be that no driver should ever
> > need to specify the 'nand-bus-width' DT property, as Brian
> > previously suggested, right?
> > 
> Right.. And this is where I'm suggesting that in-order to completely
> eliminate the dependency on 'nand-bus-width' DT property, you need
> to also update GPMC driver, so that its registers can be re-configured with
> correct NAND bus-width after nand_scan_ident().
> 


Well, as I said: I don't think there's any technical reason why you
can't just export a GPMC function to re-configure it from the NAND
driver. I just think it's ugly, and not useful.

So, I think you (or me, or anybody else) can push an RFC/PATCH to see
how ugly would that really be. Maybe it's not so bad.

But: on the other hand, I'd really like you to convince me as to
why is it so bad to require the DTB to have the proper GPMC bus width.

Once again:
1. the NAND devices aren't hot-pluggable
2. the "user" (who is actually an engineer, not some regular dummy user)
knows perfectly well the width of the device.

What's the problem with describing the hardware in the DT and saving us
lots of runtime re-configuration trouble?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux