Re: [PATCH v2 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

[Boris Brezillon <boris.brezillon@collabora.com>]

On Mon, 20 Apr 2020 13:41:28 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Mon, Apr 20, 2020 at 12:28:59PM +0200, Boris Brezillon wrote:
> > On Mon, 20 Apr 2020 13:14:42 +0300
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:  
> > > On Mon, Apr 20, 2020 at 12:53 PM Boris Brezillon
> > > <boris.brezillon@collabora.com> wrote:  
> > > > On Mon, 20 Apr 2020 12:44:51 +0300
> > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:  
> > > > > On Mon, Apr 20, 2020 at 12:21 PM Boris Brezillon
> > > > > <boris.brezillon@collabora.com> wrote:    
> > > > > > On Mon, 20 Apr 2020 01:20:40 +0300
> > > > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:    
> > > > > > > On Sat, Apr 18, 2020 at 10:55:33AM +0200, Boris Brezillon wrote:    
> > > > > > > > On Fri, 17 Apr 2020 16:21:47 +0800
> > > > > > > > "Ramuthevar,Vadivel MuruganX"
> > > > > > > > <vadivel.muruganx.ramuthevar@linux.intel.com> wrote:  
> 
> ...
> 
> > > > > > > > > +static const struct of_device_id lgm_nand_match[] = {
> > > > > > > > > + { .compatible = "intel,lgm-nand", },
> > > > > > > > > + {}
> > > > > > > > > +};
> > > > > > > > > +MODULE_DEVICE_TABLE(of, lgm_nand_match);    
> > > > > > > >
> > > > > > > > You probably have a missing "depends on OF" in your Kconfig.    
> > > > > > >
> > > > > > > Since it's using device property API, dependency is not needed.
> > > > > > >    
> > > > > >
> > > > > > There's no compile-time dependency, but this driver will be pretty
> > > > > > useless if all its users have the NAND controller node defined in their
> > > > > > DT and CONFIG_OF is not enabled.    
> > > > >
> > > > > No, it's not.
> > > > > See [1] for the details how ACPI may utilize this table.
> > > > >
> > > > > [1]: https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html#device-tree-namespace-link-device-id    
> > > >
> > > > Except the NAND framework does use the OF lib when parsing common DT
> > > > properties (like nand-ecc-mode, etc), so it does depend on OF if you
> > > > want those props to be parsed, which, according to the DT binding patch,
> > > > is the case.    
> > > 
> > > I see, so, NAND framework can be transformed at some point. In any
> > > case, from driver perspective it's OF independent.
> > >   
> > 
> > Well, it uses it only if the driver passes an OF node which this driver
> > does (see the nand_set_flash_node() call), so no, it's really a driver
> > dependency.  
> 
> Look like still it's framework dependency which driver has to rely on.
> Means more work would be needed in case NAND to convert to fwnode API.
> 

Sorry, but I'm lost here. The patch series contains a DT bindings doc,
meaning that it's using a DT representation no matter where it comes
from (the fact that it might be embedded in an ACPI table doesn't
matter, right?). The framework just provides convenient DT parsing
helpers, but they are not mandatory since they are only called if the
NAND is attached a DT node (some drivers extract those info from
driver-specific pdata structs).

To me, the lack of support of a generic fwnode parsing logic in the
NAND framework is orthogonal to this "depend on OF" problem, since, no
matter what abstraction you use to parse the DT node (fwnode would just
be a wrapper around DT parsing in this specific case), the fact
remains, for this driver, in its current state, you need OF support to
make it useful.