[U-Boot] [PATCH v4 2/3] cfi_flash: convert to driver model

[Simon Glass]

Hi Thomas,

On 5 November 2015 at 20:34, Thomas Chou <thomas at wytron.com.tw> wrote:
> Hi Simon,
>
> On 2015年11月06日 11:15, Simon Glass wrote:
>>>
>>> +config CFI_FLASH
>>> +       bool "Enable Driver Model for CFI Flash driver"
>>> +       depends on MTD
>>> +       help
>>> +         The Common Flash Interface specification was developed by
>>> Intel,
>>> +         AMD and other flash manufactures that provides a universal
>>> method
>>
>>
>>           flash manufacturers. It provides a universal method
>>
>
> OK. Thanks.
>
>>> +#ifdef CONFIG_CFI_FLASH /* for driver model */
>>> +static void cfi_flash_init_dm(void)
>>> +{
>>> +       struct udevice *dev;
>>> +
>>> +       cfi_flash_num_flash_banks = 0;
>>> +       /*
>>> +        * The uclass_first_device() will probe the first device and
>>> +        * uclass_next_device() will probe the rest if they exist. So
>>> +        * that cfi_flash_probe() will get called assigning the base
>>> +        * addresses that are available.
>>> +        */
>>> +       for (uclass_first_device(UCLASS_MTD, &dev);
>>> +            dev;
>>> +            uclass_next_device(&dev)) {
>>> +       }
>>
>>
>> Why do you need to probe these before they are used? Also I think you
>> need error checking.
>
>
> This is the place that the device is going to be used. In flash_init() the
> cfi flash is probed to find flash type and size.
>
> There may be other MTD devices, so we scan through them. If there is not cfi
> flash device, cfi_flash_num_flash_banks will be zero and the flash_init will
> return size as zero. This is not an error.
>
>>> +#ifdef CONFIG_FLASH_CFI_MTD
>>> +       dev->uclass_priv = &cfi_mtd_info[0];
>>
>>
>> Could you put cfi_mtd_info in a struct and use auto-allocation to allocate
>> it?
>
>
> I planned this as follow-up patches which will merge cfi-mtd.c and use mtd
> ops as major flash ops. Then the mtd_info will be auto-allocated. This is
> quite similar to Jagan's work on spi-nor.

Hopefully you can move cfi_mtd_info into uclass-private data at some point.

Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
Simon