[U-Boot] [PATCH v2] gpio: xilinx: Convert driver to DM

[Stefan Herbrechtsmeier]

Hi Michal,

Am 23.07.2018 um 13:43 schrieb Michal Simek:
> On 20.7.2018 22:05, Stefan Herbrechtsmeier wrote:
>> Am 13.07.2018 um 17:20 schrieb Michal Simek:
>>> This patch is enabling GPIO_DM support to have an option to use this
>>> driver together with zynq gpio driver.
>>> !DM part is kept there till Microblaze is cleanup which will be done
>>> hopefully soon.
>>>
>>> Just a note:
>>> There is no reason to initialize uc-priv->name because it is completely
>>> unused.
>>>
>>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>>> ---
>>>
>>> Changes in v2:
>>> - Show value in set_value when debug is enabled
>>> - Implement xlate function
>>> - Remove tabs from structures for alignment (to be consistent with the
>>>     rest of code)
>>>
>>>    drivers/gpio/xilinx_gpio.c | 265
>>> ++++++++++++++++++++++++++++++++++++++++++++-
>>>    1 file changed, 264 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpio/xilinx_gpio.c b/drivers/gpio/xilinx_gpio.c
>>> index 74c5be0865d1..48b52c985a55 100644
>>> --- a/drivers/gpio/xilinx_gpio.c
>>> +++ b/drivers/gpio/xilinx_gpio.c

[snip]

>>> +static int xilinx_gpio_probe(struct udevice *dev)
>>> +{
>>> +    struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
>>> +    struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
>>> +
>>> +    uc_priv->bank_name = dev->name;
>> Have you check the "gpio status -a" output? Maybe you could use a
>> gpio-bank-name from the device tree.
> The same as for zynq. gpio-bank-name is not in Linux. And yes I was
> checking output. If you know better way please let me know.

Please see my other answer.

>>> +
>>> +    uc_priv->gpio_count = platdata->bank_max[0] + platdata->bank_max[1];
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int xilinx_gpio_ofdata_to_platdata(struct udevice *dev)
>>> +{
>>> +    struct xilinx_gpio_platdata *platdata = dev_get_platdata(dev);
>>> +    int is_dual;
>>> +
>>> +    platdata->regs = (struct gpio_regs *)dev_read_addr(dev);
>>> +
>>> +    platdata->bank_max[0] = dev_read_u32_default(dev,
>>> +                             "xlnx,gpio-width", 0);
>> The default value of the Linux driver is 32.
> but not in binding doc.

Okay

>>> +    platdata->bank_input[0] = dev_read_u32_default(dev,
>>> +                               "xlnx,all-inputs", 0);
>> This isn't supported by the Linux driver but documented in the device
>> tree bindings.
> correct.
>
>>> +    platdata->bank_output[0] = dev_read_u32_default(dev,
>>> +                            "xlnx,all-outputs", 0);
>> This isn't supported by the Linux driver neither documented in the
>> device tree bindings.
> This IP, driver and dt binding was done pretty long time ago. I could be
> one of the first dt driver that's why there could issues with dt bindings.
> DTG is generating all these properties from day one and in Linux only
> documented property where that one which are used by Linux.
> All that old dt binding docs should be checked again and there is
> actually 22 patches sent to gpio mailing list
> https://patchwork.ozlabs.org/patch/947371/
> but I haven't looked at them yet.

Okay

>>> +
>>> +    is_dual = dev_read_u32_default(dev, "xlnx,is-dual", 0);
>>> +    if (is_dual) {
>>> +        platdata->bank_max[1] = dev_read_u32_default(dev,
>>> +                        "xlnx,gpio2-width", 0);
>>> +        platdata->bank_input[1] = dev_read_u32_default(dev,
>>> +                        "xlnx,all-inputs-2", 0);
>>> +        platdata->bank_output[1] = dev_read_u32_default(dev,
>>> +                        "xlnx,all-outputs-2", 0);
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static const struct udevice_id xilinx_gpio_ids[] = {
>>> +    { .compatible = "xlnx,xps-gpio-1.00.a",},
>>> +    { }
>>> +};
>>> +
>>> +U_BOOT_DRIVER(xilinx_gpio) = {
>>> +    .name = "xlnx_gpio",
>>> +    .id = UCLASS_GPIO,
>>> +    .ops = &xilinx_gpio_ops,
>>> +    .of_match = xilinx_gpio_ids,
>>> +    .ofdata_to_platdata = xilinx_gpio_ofdata_to_platdata,
>>> +    .probe = xilinx_gpio_probe,
>>> +    .platdata_auto_alloc_size = sizeof(struct xilinx_gpio_platdata),
>>> +};
>>> +#endif
>> Maybe the Xilinx AXI GPIO LogiCore IP driver could be integrated into
>> the generic gpio-mmio driver. This driver is compatible to the hardware
>> and doesn't need a default value for the data register in the device tree.
> There is no support for irq that's why I don't think this is going to fly.

I wasn't aware of the irq because the irq support is missing in the 
xilinx driver at the moment. I only realized that the xilinx driver use 
a subset of the generic gpio driver with a different binding.

Best regards
   Stefan