[U-Boot] [PATCH v4 1/3] dm: implement a MTD uclass

Jagan Teki
2015-11-07T12:35:37Z

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!
--
Jagan | openedev.