| Message ID | 1446556146-9876-1-git-send-email-thomas@wytron.com.tw |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
Hi Thomas, On 3 November 2015 at 18:39, Thomas Chou <thomas@wytron.com.tw> wrote: > Implement a Memory Technology Device (MTD) uclass. It should > include most flash drivers in the future. Though no uclass ops > are defined yet, the MTD ops could be used. > > The NAND flash driver is based on MTD. The CFI flash and SPI > flash support MTD, too. It should make sense to convert them > to MTD uclass. Why does MTD require driver model? Should drivers like nand, cfi or etc register mtd core should need to move on dm? > > Signed-off-by: Thomas Chou <thomas@wytron.com.tw> > --- > v3 > change to MTD uclass. > v4 > add mtd_info to flash_info in flash.h. > > drivers/mtd/Kconfig | 12 ++++++++++++ > drivers/mtd/Makefile | 1 + > drivers/mtd/mtd-uclass.c | 20 ++++++++++++++++++++ > include/dm/uclass-id.h | 1 + > include/flash.h | 3 +++ > include/linux/mtd/mtd.h | 3 +++ > include/mtd.h | 23 +++++++++++++++++++++++ > 7 files changed, 63 insertions(+) > create mode 100644 drivers/mtd/mtd-uclass.c > create mode 100644 include/mtd.h > > diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig > index 59278d1..23dff48 100644 > --- a/drivers/mtd/Kconfig > +++ b/drivers/mtd/Kconfig > @@ -1,3 +1,15 @@ > +menu "MTD Support" > + > +config MTD > + bool "Enable Driver Model for MTD drivers" > + depends on DM > + help > + Enable driver model for Memory Technology Devices (MTD), such as > + flash, RAM and similar chips, often used for solid state file > + systems on embedded devices. > + > +endmenu > + > source "drivers/mtd/nand/Kconfig" > > source "drivers/mtd/spi/Kconfig" > diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile > index a623f4c..c23c0c1 100644 > --- a/drivers/mtd/Makefile > +++ b/drivers/mtd/Makefile > @@ -8,6 +8,7 @@ > ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF))) > obj-y += mtdcore.o mtd_uboot.o > endif > +obj-$(CONFIG_MTD) += mtd-uclass.o > obj-$(CONFIG_MTD_PARTITIONS) += mtdpart.o > obj-$(CONFIG_MTD_CONCAT) += mtdconcat.o > obj-$(CONFIG_HAS_DATAFLASH) += at45.o > diff --git a/drivers/mtd/mtd-uclass.c b/drivers/mtd/mtd-uclass.c > new file mode 100644 > index 0000000..8bd3e6b > --- /dev/null > +++ b/drivers/mtd/mtd-uclass.c > @@ -0,0 +1,20 @@ > +/* > + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > +#include <dm.h> > +#include <errno.h> > +#include <mtd.h> > + > +/* > + * Implement a MTD uclass which should include most flash drivers. > + * The uclass private is pointed to mtd_info. > + */ > + > +UCLASS_DRIVER(mtd) = { > + .id = UCLASS_MTD, > + .name = "mtd", > +}; > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h > index 886a44c..fcc9784 100644 > --- a/include/dm/uclass-id.h > +++ b/include/dm/uclass-id.h > @@ -42,6 +42,7 @@ enum uclass_id { > UCLASS_MISC, /* Miscellaneous device */ > UCLASS_MMC, /* SD / MMC card or chip */ > UCLASS_MOD_EXP, /* RSA Mod Exp device */ > + UCLASS_MTD, /* Memory Technology Device (MTD) device */ > UCLASS_PCH, /* x86 platform controller hub */ > UCLASS_PCI, /* PCI bus */ > UCLASS_PCI_GENERIC, /* Generic PCI bus device */ > diff --git a/include/flash.h b/include/flash.h > index dc0645e..f53ace7 100644 > --- a/include/flash.h > +++ b/include/flash.h > @@ -44,6 +44,9 @@ typedef struct { > ulong addr_unlock2; /* unlock address 2 for AMD flash roms */ > const char *name; /* human-readable name */ > #endif > +#ifdef CONFIG_MTD > + struct mtd_info *mtd; > +#endif > } flash_info_t; > > extern flash_info_t flash_info[]; /* info for FLASH chips */ > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h > index e3d3fc7..0ab6128 100644 > --- a/include/linux/mtd/mtd.h > +++ b/include/linux/mtd/mtd.h > @@ -18,6 +18,7 @@ > > #include <asm/div64.h> > #else > +#include <dm.h> > #include <linux/compat.h> > #include <mtd/mtd-abi.h> > #include <asm/errno.h> > @@ -272,6 +273,8 @@ struct mtd_info { > struct module *owner; > #ifndef __UBOOT__ > struct device dev; > +#else > + struct udevice *dev; > #endif > int usecount; > }; > diff --git a/include/mtd.h b/include/mtd.h > new file mode 100644 > index 0000000..3f8c293 > --- /dev/null > +++ b/include/mtd.h > @@ -0,0 +1,23 @@ > +/* > + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#ifndef _MTD_H_ > +#define _MTD_H_ > + > +#include <linux/mtd/mtd.h> > + > +/* > + * Get mtd_info structure of the dev, which is stored as uclass private. > + * > + * @dev: The MTD device > + * @return: pointer to mtd_info, NULL on error > + */ > +static inline struct mtd_info *mtd_get_info(struct udevice *dev) > +{ > + return dev_get_uclass_priv(dev); > +} > + > +#endif /* _MTD_H_ */ > -- > 2.5.0 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
Hi Jagan, On 2015年11月03日 22:41, Jagan Teki wrote: > Hi Thomas, > > On 3 November 2015 at 18:39, Thomas Chou <thomas@wytron.com.tw> wrote: >> Implement a Memory Technology Device (MTD) uclass. It should >> include most flash drivers in the future. Though no uclass ops >> are defined yet, the MTD ops could be used. >> >> The NAND flash driver is based on MTD. The CFI flash and SPI >> flash support MTD, too. It should make sense to convert them >> to MTD uclass. > > Why does MTD require driver model? Should drivers like nand, cfi or > etc register mtd core should need to move on dm? The driver model combined with device tree control of u-boot offers dynamic binding of drivers and devices. It is expected that all drivers will be converted to driver model, including nand, cfi and spi flash. Best regards, Thomas
On 3 November 2015 at 20:19, Thomas Chou <thomas@wytron.com.tw> wrote: > Hi Jagan, > > On 2015年11月03日 22:41, Jagan Teki wrote: >> >> Hi Thomas, >> >> On 3 November 2015 at 18:39, Thomas Chou <thomas@wytron.com.tw> wrote: >>> >>> Implement a Memory Technology Device (MTD) uclass. It should >>> include most flash drivers in the future. Though no uclass ops >>> are defined yet, the MTD ops could be used. >>> >>> The NAND flash driver is based on MTD. The CFI flash and SPI >>> flash support MTD, too. It should make sense to convert them >>> to MTD uclass. >> >> >> Why does MTD require driver model? Should drivers like nand, cfi or >> etc register mtd core should need to move on dm? > > > The driver model combined with device tree control of u-boot offers dynamic > binding of drivers and devices. It is expected that all drivers will be > converted to driver model, including nand, cfi and spi flash. So, mtd_info ops like _erase, _write and _read will also change or something like this struct dm_mtd_info { struct mtd_info *info; struct udevice *dev; }; thanks!
On 3 November 2015 at 20:25, Jagan Teki <jteki@openedev.com> wrote: > On 3 November 2015 at 20:19, Thomas Chou <thomas@wytron.com.tw> wrote: >> Hi Jagan, >> >> On 2015年11月03日 22:41, Jagan Teki wrote: >>> >>> Hi Thomas, >>> >>> On 3 November 2015 at 18:39, Thomas Chou <thomas@wytron.com.tw> wrote: >>>> >>>> Implement a Memory Technology Device (MTD) uclass. It should >>>> include most flash drivers in the future. Though no uclass ops >>>> are defined yet, the MTD ops could be used. >>>> >>>> The NAND flash driver is based on MTD. The CFI flash and SPI >>>> flash support MTD, too. It should make sense to convert them >>>> to MTD uclass. >>> >>> >>> Why does MTD require driver model? Should drivers like nand, cfi or >>> etc register mtd core should need to move on dm? >> >> >> The driver model combined with device tree control of u-boot offers dynamic >> binding of drivers and devices. It is expected that all drivers will be >> converted to driver model, including nand, cfi and spi flash. > > So, mtd_info ops like _erase, _write and _read will also change or > something like this > > struct dm_mtd_info { > struct mtd_info *info; > struct udevice *dev; > }; See for example, I have recently added MTD support to spi_flash [1] [2] [1] https://patchwork.ozlabs.org/patch/529397/ [2] https://patchwork.ozlabs.org/patch/529399/
On 2015年11月03日 22:55, Jagan Teki wrote: > On 3 November 2015 at 20:19, Thomas Chou <thomas@wytron.com.tw> wrote: >> Hi Jagan, >> >> On 2015年11月03日 22:41, Jagan Teki wrote: >>> >>> Hi Thomas, >>> >>> On 3 November 2015 at 18:39, Thomas Chou <thomas@wytron.com.tw> wrote: >>>> >>>> Implement a Memory Technology Device (MTD) uclass. It should >>>> include most flash drivers in the future. Though no uclass ops >>>> are defined yet, the MTD ops could be used. >>>> >>>> The NAND flash driver is based on MTD. The CFI flash and SPI >>>> flash support MTD, too. It should make sense to convert them >>>> to MTD uclass. >>> >>> >>> Why does MTD require driver model? Should drivers like nand, cfi or >>> etc register mtd core should need to move on dm? >> >> >> The driver model combined with device tree control of u-boot offers dynamic >> binding of drivers and devices. It is expected that all drivers will be >> converted to driver model, including nand, cfi and spi flash. > > So, mtd_info ops like _erase, _write and _read will also change or > something like this > > struct dm_mtd_info { > struct mtd_info *info; > struct udevice *dev; > }; Not exactly. I included udevice in mtd_info as it was device for Linux. @@ -272,6 +273,8 @@ struct mtd_info { struct module *owner; #ifndef __UBOOT__ struct device dev; +#else + struct udevice *dev; #endif int usecount; }; I think the mtd ops is more complete and widely used. There might be no need to reinvent the dm_mtd ops. The mtd uclass priv is set to mtd_info and we can get it with mtd_get_info(dev). Then call mtd ops, like mtd_read() mtd_write and mtd_erase(), directly. > See for example, I have recently added MTD support to spi_flash [1] [2] > > [1] https://patchwork.ozlabs.org/patch/529397/ > [2] https://patchwork.ozlabs.org/patch/529399/ It seems we are working toward the same direction. :) Simon suggested that we can have an unified flash class (for all cfi, spi and nand flash) after the discussion between Bin Meng and I. So I dropped the earlier cfi-flash uclass, and found the mtd might be a better uclass. We see the same point, "MTD has proven core for flash operations". The work on cfi-flash is not complete yet. It needs to reshape to use mtd ops like your earlier patches. But I have to work on others. The spi-flash uclass should be merged into mtd uclass and use mtd ops. Maybe you will be interested and will help. Thanks in advance. The nand flash is more ready. But need to convert to driver model. Best regards, Thomas
On 3 November 2015 at 06:09, Thomas Chou <thomas@wytron.com.tw> wrote: > Implement a Memory Technology Device (MTD) uclass. It should > include most flash drivers in the future. Though no uclass ops > are defined yet, the MTD ops could be used. > > The NAND flash driver is based on MTD. The CFI flash and SPI > flash support MTD, too. It should make sense to convert them > to MTD uclass. > > Signed-off-by: Thomas Chou <thomas@wytron.com.tw> > --- > v3 > change to MTD uclass. > v4 > add mtd_info to flash_info in flash.h. > > drivers/mtd/Kconfig | 12 ++++++++++++ > drivers/mtd/Makefile | 1 + > drivers/mtd/mtd-uclass.c | 20 ++++++++++++++++++++ > include/dm/uclass-id.h | 1 + > include/flash.h | 3 +++ > include/linux/mtd/mtd.h | 3 +++ > include/mtd.h | 23 +++++++++++++++++++++++ > 7 files changed, 63 insertions(+) > create mode 100644 drivers/mtd/mtd-uclass.c > create mode 100644 include/mtd.h > > diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig > index 59278d1..23dff48 100644 > --- a/drivers/mtd/Kconfig > +++ b/drivers/mtd/Kconfig > @@ -1,3 +1,15 @@ > +menu "MTD Support" > + > +config MTD > + bool "Enable Driver Model for MTD drivers" > + depends on DM > + help > + Enable driver model for Memory Technology Devices (MTD), such as > + flash, RAM and similar chips, often used for solid state file > + systems on embedded devices. > + > +endmenu > + > source "drivers/mtd/nand/Kconfig" > > source "drivers/mtd/spi/Kconfig" > diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile > index a623f4c..c23c0c1 100644 > --- a/drivers/mtd/Makefile > +++ b/drivers/mtd/Makefile > @@ -8,6 +8,7 @@ > ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF))) > obj-y += mtdcore.o mtd_uboot.o > endif > +obj-$(CONFIG_MTD) += mtd-uclass.o > obj-$(CONFIG_MTD_PARTITIONS) += mtdpart.o > obj-$(CONFIG_MTD_CONCAT) += mtdconcat.o > obj-$(CONFIG_HAS_DATAFLASH) += at45.o > diff --git a/drivers/mtd/mtd-uclass.c b/drivers/mtd/mtd-uclass.c > new file mode 100644 > index 0000000..8bd3e6b > --- /dev/null > +++ b/drivers/mtd/mtd-uclass.c > @@ -0,0 +1,20 @@ > +/* > + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > +#include <dm.h> > +#include <errno.h> > +#include <mtd.h> > + > +/* > + * Implement a MTD uclass which should include most flash drivers. > + * The uclass private is pointed to mtd_info. > + */ > + > +UCLASS_DRIVER(mtd) = { > + .id = UCLASS_MTD, > + .name = "mtd", > +}; > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h > index 886a44c..fcc9784 100644 > --- a/include/dm/uclass-id.h > +++ b/include/dm/uclass-id.h > @@ -42,6 +42,7 @@ enum uclass_id { > UCLASS_MISC, /* Miscellaneous device */ > UCLASS_MMC, /* SD / MMC card or chip */ > UCLASS_MOD_EXP, /* RSA Mod Exp device */ > + UCLASS_MTD, /* Memory Technology Device (MTD) device */ > UCLASS_PCH, /* x86 platform controller hub */ > UCLASS_PCI, /* PCI bus */ > UCLASS_PCI_GENERIC, /* Generic PCI bus device */ > diff --git a/include/flash.h b/include/flash.h > index dc0645e..f53ace7 100644 > --- a/include/flash.h > +++ b/include/flash.h > @@ -44,6 +44,9 @@ typedef struct { > ulong addr_unlock2; /* unlock address 2 for AMD flash roms */ > const char *name; /* human-readable name */ > #endif > +#ifdef CONFIG_MTD > + struct mtd_info *mtd; > +#endif > } flash_info_t; > > extern flash_info_t flash_info[]; /* info for FLASH chips */ > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h > index e3d3fc7..0ab6128 100644 > --- a/include/linux/mtd/mtd.h > +++ b/include/linux/mtd/mtd.h > @@ -18,6 +18,7 @@ > > #include <asm/div64.h> > #else > +#include <dm.h> I'm not keen on adding this header here. Why is it needed? Can we instead include <m.h> in the C files that need it? > #include <linux/compat.h> > #include <mtd/mtd-abi.h> > #include <asm/errno.h> > @@ -272,6 +273,8 @@ struct mtd_info { > struct module *owner; > #ifndef __UBOOT__ > struct device dev; > +#else > + struct udevice *dev; > #endif > int usecount; > }; > diff --git a/include/mtd.h b/include/mtd.h > new file mode 100644 > index 0000000..3f8c293 > --- /dev/null > +++ b/include/mtd.h > @@ -0,0 +1,23 @@ > +/* > + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#ifndef _MTD_H_ > +#define _MTD_H_ > + > +#include <linux/mtd/mtd.h> > + > +/* > + * Get mtd_info structure of the dev, which is stored as uclass private. > + * > + * @dev: The MTD device > + * @return: pointer to mtd_info, NULL on error > + */ > +static inline struct mtd_info *mtd_get_info(struct udevice *dev) > +{ > + return dev_get_uclass_priv(dev); > +} That function is a nice idea I think. > + > +#endif /* _MTD_H_ */ > -- > 2.5.0 > Regards, Simon
Hi Simon, On 2015年11月06日 20:06, Simon Glass wrote: >> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h >> index e3d3fc7..0ab6128 100644 >> --- a/include/linux/mtd/mtd.h >> +++ b/include/linux/mtd/mtd.h >> @@ -18,6 +18,7 @@ >> >> #include <asm/div64.h> >> #else >> +#include <dm.h> > > I'm not keen on adding this header here. Why is it needed? Can we > instead include <m.h> in the C files that need it? > It is needed for the udevice in mtd_info. Some drivers use mtd.h but are not converted to driver model. Maybe I should remove the dm here, and add an #elif for the udevice below? >> #include <linux/compat.h> >> #include <mtd/mtd-abi.h> >> #include <asm/errno.h> >> @@ -272,6 +273,8 @@ struct mtd_info { >> struct module *owner; >> #ifndef __UBOOT__ >> struct device dev; >> +#else #elif CONFIG_IS_ENABLED(MTD) >> + struct udevice *dev; >> #endif >> int usecount; >> }; Thanks a lot for your review. Best regards, Thomas
Hi Thomas, On 6 November 2015 at 05:25, Thomas Chou <thomas@wytron.com.tw> wrote: > Hi Simon, > > On 2015年11月06日 20:06, Simon Glass wrote: >>> >>> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h >>> index e3d3fc7..0ab6128 100644 >>> --- a/include/linux/mtd/mtd.h >>> +++ b/include/linux/mtd/mtd.h >>> @@ -18,6 +18,7 @@ >>> >>> #include <asm/div64.h> >>> #else >>> +#include <dm.h> >> >> >> I'm not keen on adding this header here. Why is it needed? Can we >> instead include <m.h> in the C files that need it? >> > > It is needed for the udevice in mtd_info. Some drivers use mtd.h but are not > converted to driver model. Maybe I should remove the dm here, and add an > #elif for the udevice below? Are you sure? It should be possible to declare a 'struct udevice *' without this header. > >>> #include <linux/compat.h> >>> #include <mtd/mtd-abi.h> >>> #include <asm/errno.h> >>> @@ -272,6 +273,8 @@ struct mtd_info { >>> struct module *owner; >>> #ifndef __UBOOT__ >>> struct device dev; >>> +#else > > > #elif CONFIG_IS_ENABLED(MTD) > >>> + struct udevice *dev; >>> #endif >>> int usecount; >>> }; > > > Thanks a lot for your review. > > Best regards, > Thomas Regards, Simon
Hi Thomas/Simon, On 4 November 2015 at 08:42, Thomas Chou <thomas@wytron.com.tw> wrote: > > > On 2015年11月03日 22:55, Jagan Teki wrote: >> >> On 3 November 2015 at 20:19, Thomas Chou <thomas@wytron.com.tw> wrote: >>> >>> Hi Jagan, >>> >>> On 2015年11月03日 22:41, Jagan Teki wrote: >>>> >>>> >>>> Hi Thomas, >>>> >>>> On 3 November 2015 at 18:39, Thomas Chou <thomas@wytron.com.tw> wrote: >>>>> >>>>> >>>>> Implement a Memory Technology Device (MTD) uclass. It should >>>>> include most flash drivers in the future. Though no uclass ops >>>>> are defined yet, the MTD ops could be used. >>>>> >>>>> The NAND flash driver is based on MTD. The CFI flash and SPI >>>>> flash support MTD, too. It should make sense to convert them >>>>> to MTD uclass. >>>> >>>> >>>> >>>> Why does MTD require driver model? Should drivers like nand, cfi or >>>> etc register mtd core should need to move on dm? >>> >>> >>> >>> The driver model combined with device tree control of u-boot offers >>> dynamic >>> binding of drivers and devices. It is expected that all drivers will be >>> converted to driver model, including nand, cfi and spi flash. >> >> >> So, mtd_info ops like _erase, _write and _read will also change or >> something like this >> >> struct dm_mtd_info { >> struct mtd_info *info; >> struct udevice *dev; >> }; > > > Not exactly. I included udevice in mtd_info as it was device for Linux. > > @@ -272,6 +273,8 @@ struct mtd_info { > struct module *owner; > #ifndef __UBOOT__ > struct device dev; > +#else > + struct udevice *dev; > #endif > int usecount; > }; > > I think the mtd ops is more complete and widely used. There might be no need > to reinvent the dm_mtd ops. The mtd uclass priv is set to mtd_info and we > can get it with mtd_get_info(dev). Then call mtd ops, like mtd_read() > mtd_write and mtd_erase(), directly. > >> See for example, I have recently added MTD support to spi_flash [1] [2] >> >> [1] https://patchwork.ozlabs.org/patch/529397/ >> [2] https://patchwork.ozlabs.org/patch/529399/ > > It seems we are working toward the same direction. :) Sorry, I couldn't understand this looks we're in different direction. Let me explain what I thought about mtd_info usage. udevice should be part of underlying flash structure's like cfi, nand and spi_flash and mtd_info should be used for it's core api's like _erase. _read and _write and underlying driver will use their global structure that include's mtd and udevice as a function pointer like this. struct spi_flash { struct mtd_info *info; struct udevice *device; } struct spi_flash_priv { struct spi_flash flash; struct mtd_info mtd; }; static int spi_flash_std_probe(struct udevice *dev) { struct spi_flash_priv *priv = dev_get_uclass_priv(dev); struct spi_slave *spi = dev_get_parent_priv(dev); struct spi_flash *flash; int ret; flash = &priv->flash; flash->mtd = &priv->mtd; flash->spi = spi; flash->priv = priv; priv->mtd.priv = flash; flash->dev = dev; } U_BOOT_DRIVER(spi_flash_std) = { .name = "spi_flash_std", .id = UCLASS_SPI_FLASH, .of_match = spi_flash_std_ids, .probe = spi_flash_std_probe, .priv_auto_alloc_size = sizeof(struct spi_flash_priv), }; This is the way I have implemented mtd on spi-flash[1] [2] [1] https://patchwork.ozlabs.org/patch/529397/ [2] https://patchwork.ozlabs.org/patch/529399/ Please explain how this related your approach of adding udevice to mtd. > > Simon suggested that we can have an unified flash class (for all cfi, spi > and nand flash) after the discussion between Bin Meng and I. So I dropped > the earlier cfi-flash uclass, and found the mtd might be a better uclass. We > see the same point, "MTD has proven core for flash operations". > > The work on cfi-flash is not complete yet. It needs to reshape to use mtd > ops like your earlier patches. But I have to work on others. > > The spi-flash uclass should be merged into mtd uclass and use mtd ops. Maybe > you will be interested and will help. Thanks in advance. > > The nand flash is more ready. But need to convert to driver model. thanks!
HI Jagan, On 2015年11月07日 20:35, Jagan Teki wrote: >> It seems we are working toward the same direction. :) > > Sorry, I couldn't understand this looks we're in different direction. > > Let me explain what I thought about mtd_info usage. udevice should be > part of underlying flash structure's like cfi, nand and spi_flash and > mtd_info should be used for it's core api's like _erase. _read and > _write and underlying driver will use their global structure that > include's mtd and udevice as a function pointer like this. Please see v5 of this patch and v3 of altera qspi. The uclass priv of mtd class is an auto-allocated mtd_info. The spi_flash_priv includes both mtd_info and spi_flash. So the only difference is the spi_flash. This is because cfi uses struct flash, while spi flash uses struct spi_flash. So it is better to leave the struct flash to driver allocation. The mtd->priv points to struct spi_flash for spi flash, and points to struct flash for cfi flash. It serves the same purpose. The struct flash has *mtd. The struct spi_flash has *mtd, too. I added struct udevice *dev to mtd_info. The struct spi_flash has *dev, but struct flash has not. > > struct spi_flash { > struct mtd_info *info; > struct udevice *device; > } > struct flash { ... struct mtd_info *info; } > struct spi_flash_priv { > struct spi_flash flash; > struct mtd_info mtd; > }; > Simply, struct mtd_info. > static int spi_flash_std_probe(struct udevice *dev) > { > struct spi_flash_priv *priv = dev_get_uclass_priv(dev); > struct spi_slave *spi = dev_get_parent_priv(dev); > struct spi_flash *flash; > int ret; > > flash = &priv->flash; > flash->mtd = &priv->mtd; mtd = dev_get_uclass_priv(dev); flash->mtd = mtd; > > flash->spi = spi; > flash->priv = priv; > > priv->mtd.priv = flash; > flash->dev = dev; mtd->priv = flash; mtd->dev = dev; flash->mtd->dev is the same as spi's flash->dev > } > > U_BOOT_DRIVER(spi_flash_std) = { > .name = "spi_flash_std", > .id = UCLASS_SPI_FLASH, > .of_match = spi_flash_std_ids, > .probe = spi_flash_std_probe, > .priv_auto_alloc_size = sizeof(struct spi_flash_priv), > }; > > This is the way I have implemented mtd on spi-flash[1] [2] > [1] https://patchwork.ozlabs.org/patch/529397/ > [2] https://patchwork.ozlabs.org/patch/529399/ > > Please explain how this related your approach of adding udevice to mtd. > The flash ops which u-boot commands calls are built upon mtd ops. eg, write_buff() { mtd_write(); } flash_erase() { mtd_erase(); } Please let me know what do you think. Thanks. :) Best regards, Thomas
diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig index 59278d1..23dff48 100644 --- a/drivers/mtd/Kconfig +++ b/drivers/mtd/Kconfig @@ -1,3 +1,15 @@ +menu "MTD Support" + +config MTD + bool "Enable Driver Model for MTD drivers" + depends on DM + help + Enable driver model for Memory Technology Devices (MTD), such as + flash, RAM and similar chips, often used for solid state file + systems on embedded devices. + +endmenu + source "drivers/mtd/nand/Kconfig" source "drivers/mtd/spi/Kconfig" diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile index a623f4c..c23c0c1 100644 --- a/drivers/mtd/Makefile +++ b/drivers/mtd/Makefile @@ -8,6 +8,7 @@ ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF))) obj-y += mtdcore.o mtd_uboot.o endif +obj-$(CONFIG_MTD) += mtd-uclass.o obj-$(CONFIG_MTD_PARTITIONS) += mtdpart.o obj-$(CONFIG_MTD_CONCAT) += mtdconcat.o obj-$(CONFIG_HAS_DATAFLASH) += at45.o diff --git a/drivers/mtd/mtd-uclass.c b/drivers/mtd/mtd-uclass.c new file mode 100644 index 0000000..8bd3e6b --- /dev/null +++ b/drivers/mtd/mtd-uclass.c @@ -0,0 +1,20 @@ +/* + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <errno.h> +#include <mtd.h> + +/* + * Implement a MTD uclass which should include most flash drivers. + * The uclass private is pointed to mtd_info. + */ + +UCLASS_DRIVER(mtd) = { + .id = UCLASS_MTD, + .name = "mtd", +}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 886a44c..fcc9784 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -42,6 +42,7 @@ enum uclass_id { UCLASS_MISC, /* Miscellaneous device */ UCLASS_MMC, /* SD / MMC card or chip */ UCLASS_MOD_EXP, /* RSA Mod Exp device */ + UCLASS_MTD, /* Memory Technology Device (MTD) device */ UCLASS_PCH, /* x86 platform controller hub */ UCLASS_PCI, /* PCI bus */ UCLASS_PCI_GENERIC, /* Generic PCI bus device */ diff --git a/include/flash.h b/include/flash.h index dc0645e..f53ace7 100644 --- a/include/flash.h +++ b/include/flash.h @@ -44,6 +44,9 @@ typedef struct { ulong addr_unlock2; /* unlock address 2 for AMD flash roms */ const char *name; /* human-readable name */ #endif +#ifdef CONFIG_MTD + struct mtd_info *mtd; +#endif } flash_info_t; extern flash_info_t flash_info[]; /* info for FLASH chips */ diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index e3d3fc7..0ab6128 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -18,6 +18,7 @@ #include <asm/div64.h> #else +#include <dm.h> #include <linux/compat.h> #include <mtd/mtd-abi.h> #include <asm/errno.h> @@ -272,6 +273,8 @@ struct mtd_info { struct module *owner; #ifndef __UBOOT__ struct device dev; +#else + struct udevice *dev; #endif int usecount; }; diff --git a/include/mtd.h b/include/mtd.h new file mode 100644 index 0000000..3f8c293 --- /dev/null +++ b/include/mtd.h @@ -0,0 +1,23 @@ +/* + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef _MTD_H_ +#define _MTD_H_ + +#include <linux/mtd/mtd.h> + +/* + * Get mtd_info structure of the dev, which is stored as uclass private. + * + * @dev: The MTD device + * @return: pointer to mtd_info, NULL on error + */ +static inline struct mtd_info *mtd_get_info(struct udevice *dev) +{ + return dev_get_uclass_priv(dev); +} + +#endif /* _MTD_H_ */
Implement a Memory Technology Device (MTD) uclass. It should include most flash drivers in the future. Though no uclass ops are defined yet, the MTD ops could be used. The NAND flash driver is based on MTD. The CFI flash and SPI flash support MTD, too. It should make sense to convert them to MTD uclass. Signed-off-by: Thomas Chou <thomas@wytron.com.tw> --- v3 change to MTD uclass. v4 add mtd_info to flash_info in flash.h. drivers/mtd/Kconfig | 12 ++++++++++++ drivers/mtd/Makefile | 1 + drivers/mtd/mtd-uclass.c | 20 ++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/flash.h | 3 +++ include/linux/mtd/mtd.h | 3 +++ include/mtd.h | 23 +++++++++++++++++++++++ 7 files changed, 63 insertions(+) create mode 100644 drivers/mtd/mtd-uclass.c create mode 100644 include/mtd.h