[U-Boot] [PATCH V2] dm: gpio: Add DM compatibilty to GPIO driver for Davinci

[Simon Glass]

Hi Adam,

On 17 September 2017 at 04:29, Adam Ford <aford173 at gmail.com> wrote:
> On Tue, Sep 12, 2017 at 11:27 PM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Adam,
>>
>> On 12 September 2017 at 21:28, Adam Ford <aford173 at gmail.com> wrote:
>>> This adds DM compatibility for the davinici GPIO driver.
>>> Tested on da850-evm.
>>>
>>> Signed-off-by: Adam Ford <aford173 at gmail.com>
>>> ---
>>> V2:  The bank calculation needs to take into account the size of the struct
>>>      Whitespace fixes
>>>
>>>  arch/arm/mach-davinci/include/mach/gpio.h |  14 +-
>>>  board/davinci/da8xxevm/da850evm.c         |   2 +
>>>  configs/da850evm_defconfig                |   3 +-
>>>  drivers/gpio/da8xx_gpio.c                 | 208 +++++++++++++++++++++++++++---
>>>  include/configs/da850evm.h                |   1 +
>>>  5 files changed, 205 insertions(+), 23 deletions(-)
[...]

>>> +static int davinci_gpio_bind(struct udevice *dev)
>>> +{
>>> +       struct davinci_gpio_platdata *plat = dev->platdata;
>>> +       fdt_addr_t base_addr;
>>> +
>>> +       if (plat)
>>> +               return 0;
>>> +
>>> +       base_addr = devfdt_get_addr(dev);
>>> +       if (base_addr == FDT_ADDR_T_NONE)
>>> +               return -ENODEV;
>>
>> -EINVAL. There is definitely a device.
>>
>> Also we should not be reading the DT in the bind() method. This should
>> happen in ofdata_to_platdata()
>
> Can you point me to an example board you want me to use?  Several
> boards do it this way including omap_gpio.c, mxc_gpio.c, imx_rgpio2.c,
> and others.  I used the omap_gpio.c file as a model for this since
> they are similar.

See for example rk_gpio which is a simple driver.

One mode is tegra_gpio - that driver has an 'empty' parent device and
then creates child GPIO devices in the bind() method.

Another is omap_gpio - that uses platform data and U_BOOT_DEVICE() to
create drivers (for some boards) rather than device tree.

But here you don't see to be doing that. You just have a single GPIO
device, right? If so, you should be able to put everything in
ofdata_to_platdata().

If you set .platdata_auto_alloc_size it will automatically allocate
the platform data.

>>
>>> +
>>> +       /*
>>> +       * TODO:
>>> +       * When every board is converted to driver model and DT is
>>> +       * supported, this can be done by auto-alloc feature, but
>>> +       * not using calloc to alloc memory for platdata.
>>
>> I don't really get this because we are in a driver-model method here.
>> Can we not use the plat data here?
>>
>
> See the above comment.  Some boards have a bind function while others
> use ofdata_to_platdata().  The readme shows two possible ways. I did
> it this way because the examples I followed did it this way.  If it's
> good enough for them, why can't it be good enough this?

It is frustrating when you follow examples and they are not quite
right. I will do a little series to tidy this stuff up a bit and cc
you.

Which readme are you referring to?

Regards,
Simon