| Message ID | abda2d9e-7bc2-205f-d53c-84be773821ac@ti.com |
|---|---|
| State | Not Applicable |
| Delegated to: | Simon Glass |
| Headers | show |
Hi, On 6 February 2017 at 11:39, Grygorii Strashko <grygorii.strashko@ti.com> wrote: > > > On 02/06/2017 09:34 AM, Simon Glass wrote: >> >> Hi, >> >> On 31 January 2017 at 13:37, Grygorii Strashko <grygorii.strashko@ti.com> >> wrote: >>> >>> From: Mugunthan V N <mugunthanvnm@ti.com> >>> >>> Implement a NAND uclass so that the NAND devices can be >>> accessed via the DM framework. >>> >>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> >>> --- >>> drivers/mtd/nand/Kconfig | 10 ++++++++++ >>> drivers/mtd/nand/Makefile | 2 ++ >>> drivers/mtd/nand/nand-uclass.c | 38 >>> ++++++++++++++++++++++++++++++++++++++ >>> drivers/mtd/nand/nand.c | 17 +++++++++++++++-- >>> include/dm/uclass-id.h | 1 + >>> 5 files changed, 66 insertions(+), 2 deletions(-) >>> create mode 100644 drivers/mtd/nand/nand-uclass.c >>> > > [...] > >>> + >>> +DECLARE_GLOBAL_DATA_PTR; >>> + >>> +struct mtd_info *get_nand_dev_by_index(int idx) >>> +{ >>> + struct nand_chip *chip; >>> + struct udevice *dev; >>> + int ret; >>> + >>> + ret = uclass_get_device(UCLASS_NAND, idx, &dev); >>> + if (ret) { >>> + debug("NAND device (%d) not found\n", idx); >>> + return NULL; >>> + } >>> + >>> + chip = (struct nand_chip *)dev_get_priv(dev); >>> + >>> + return nand_to_mtd(chip); >> >> >> Can you add some comments to the nand.h header file (perhaps within >> #ifdef CONFIG_DM_NAND) to explain that drivers must have struct >> nand_chip as the first part of their private data? Assuming I have >> this right... >> > > Will below work for you? > > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index d55807b..567f286 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -684,6 +684,16 @@ struct nand_buffers { > * correctable). > * @write_page: [REPLACEABLE] High-level page write function > */ > +#ifdef CONFIG_DM_NAND > +/** > + * DM compatible nand drivers must have struct nand_chip as > + * the first part of their private data: > + * U_BOOT_DRIVER(...) = { > + * ... > + * .priv_auto_alloc_size = sizeof(struct nand_chip), > + * }; > + */ > +#endif Seems good > > struct nand_chip { > struct mtd_info mtd; > > >>> +} >>> + >>> +UCLASS_DRIVER(nand) = { >>> + .id = UCLASS_NAND, >>> + .name = "nand", >>> + .flags = DM_UC_FLAG_SEQ_ALIAS, >>> +}; >> >> >> Also can we have a sandbox NAND driver and some tests. >> > > Sry, for the stupid question, but what's "sandbox NAND driver"? Any ref? I mean you should create a NAND driver for sandbox. It should pretend to be a NAND chip and support operation as such. There are various sandbox drivers for other uclasses that do this. Do you have any operations in your uclass? See for example struct dm_mmc_ops. I added the MMC uclass without the operations, and it was painful to add them later. > > Also does it really required to be done as part of this series? Yes - any new uclass needs a test in test/dm. It can be a simple test, though. Regards, Simon
On 02/10/2017 10:22 AM, Simon Glass wrote: > Hi, > > On 6 February 2017 at 11:39, Grygorii Strashko <grygorii.strashko@ti.com> wrote: >> >> >> On 02/06/2017 09:34 AM, Simon Glass wrote: >>> >>> Hi, >>> >>> On 31 January 2017 at 13:37, Grygorii Strashko <grygorii.strashko@ti.com> >>> wrote: >>>> >>>> From: Mugunthan V N <mugunthanvnm@ti.com> >>>> >>>> Implement a NAND uclass so that the NAND devices can be >>>> accessed via the DM framework. >>>> >>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> >>>> --- >>>> drivers/mtd/nand/Kconfig | 10 ++++++++++ >>>> drivers/mtd/nand/Makefile | 2 ++ >>>> drivers/mtd/nand/nand-uclass.c | 38 >>>> ++++++++++++++++++++++++++++++++++++++ >>>> drivers/mtd/nand/nand.c | 17 +++++++++++++++-- >>>> include/dm/uclass-id.h | 1 + >>>> 5 files changed, 66 insertions(+), 2 deletions(-) >>>> create mode 100644 drivers/mtd/nand/nand-uclass.c >>>> >> >> [...] >> >>>> + >>>> +DECLARE_GLOBAL_DATA_PTR; >>>> + >>>> +struct mtd_info *get_nand_dev_by_index(int idx) >>>> +{ >>>> + struct nand_chip *chip; >>>> + struct udevice *dev; >>>> + int ret; >>>> + >>>> + ret = uclass_get_device(UCLASS_NAND, idx, &dev); >>>> + if (ret) { >>>> + debug("NAND device (%d) not found\n", idx); >>>> + return NULL; >>>> + } >>>> + >>>> + chip = (struct nand_chip *)dev_get_priv(dev); >>>> + >>>> + return nand_to_mtd(chip); >>> >>> >>> Can you add some comments to the nand.h header file (perhaps within >>> #ifdef CONFIG_DM_NAND) to explain that drivers must have struct >>> nand_chip as the first part of their private data? Assuming I have >>> this right... >>> >> >> Will below work for you? >> >> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h >> index d55807b..567f286 100644 >> --- a/include/linux/mtd/nand.h >> +++ b/include/linux/mtd/nand.h >> @@ -684,6 +684,16 @@ struct nand_buffers { >> * correctable). >> * @write_page: [REPLACEABLE] High-level page write function >> */ >> +#ifdef CONFIG_DM_NAND >> +/** >> + * DM compatible nand drivers must have struct nand_chip as >> + * the first part of their private data: >> + * U_BOOT_DRIVER(...) = { >> + * ... >> + * .priv_auto_alloc_size = sizeof(struct nand_chip), >> + * }; >> + */ >> +#endif > > Seems good > >> >> struct nand_chip { >> struct mtd_info mtd; >> >> >>>> +} >>>> + >>>> +UCLASS_DRIVER(nand) = { >>>> + .id = UCLASS_NAND, >>>> + .name = "nand", >>>> + .flags = DM_UC_FLAG_SEQ_ALIAS, >>>> +}; >>> >>> >>> Also can we have a sandbox NAND driver and some tests. >>> >> >> Sry, for the stupid question, but what's "sandbox NAND driver"? Any ref? > > I mean you should create a NAND driver for sandbox. It should pretend > to be a NAND chip and support operation as such. There are various > sandbox drivers for other uclasses that do this. > > Do you have any operations in your uclass? See for example struct > dm_mmc_ops. I added the MMC uclass without the operations, and it was > painful to add them later. > >> >> Also does it really required to be done as part of this series? > > Yes - any new uclass needs a test in test/dm. It can be a simple test, though. > ok. i understood your requests, but, unfortunately, I'm not sure that i'll be able to satisfy them now :( - i'm pretty limited in time. For now I'll resend preparation patches get_nand_dev_by_index() and will try to get back here as soon as i can.
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index d55807b..567f286 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -684,6 +684,16 @@ struct nand_buffers { * correctable). * @write_page: [REPLACEABLE] High-level page write function */ +#ifdef CONFIG_DM_NAND +/** + * DM compatible nand drivers must have struct nand_chip as + * the first part of their private data: + * U_BOOT_DRIVER(...) = { + * ... + * .priv_auto_alloc_size = sizeof(struct nand_chip), + * }; + */ +#endif struct nand_chip {