[U-Boot] [PATCH 2/2] sunxi: gpio: Add support for the gpio banks which are part of the R-io cluster

[Simon Glass]

Hi Hans,

On 18 August 2015 at 03:29, Hans de Goede <hdegoede at redhat.com> wrote:
> Hi,
>
>
> On 18-08-15 00:14, Simon Glass wrote:
>>
>> Hi Hans,
>>
>> On 6 August 2015 at 12:13, Hans de Goede <hdegoede at redhat.com> wrote:
>>>
>>> sun6i and later have a couple of io-blocks which are shared between the
>>> main CPU core and the "R" cpu which is small embedded cpu which can be
>>> active while the main system is suspended.
>>>
>>> These gpio banks sit at a different mmio address then the normal banks,
>>> and have a separate devicetree node and compatible, this adds support for
>>> these banks to the sunxi-gpio code when built with device-model support.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>> ---
>>>   drivers/gpio/sunxi_gpio.c | 22 ++++++++++++++++++----
>>>   1 file changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpio/sunxi_gpio.c b/drivers/gpio/sunxi_gpio.c
>>> index afa165a..57b78e5 100644
>>> --- a/drivers/gpio/sunxi_gpio.c
>>> +++ b/drivers/gpio/sunxi_gpio.c
>>> @@ -266,16 +266,28 @@ static int gpio_sunxi_bind(struct udevice *parent)
>>>   {
>>>          struct sunxi_gpio_platdata *plat = parent->platdata;
>>>          struct sunxi_gpio_reg *ctlr;
>>> -       int bank;
>>> -       int ret;
>>> +       int bank, no_banks, ret, start;
>>>
>>>          /* If this is a child device, there is nothing to do here */
>>>          if (plat)
>>>                  return 0;
>>>
>>> +       if (fdt_node_check_compatible(gd->fdt_blob, parent->of_offset,
>>> +                               "allwinner,sun6i-a31-r-pinctrl") == 0) {
>>
>>
>> If I understand what you are doing here, the correct way is to use a
>> .data field in sunxi_gpio_ids[]  below.
>
>
> Yes that would work too, but that indirection makes it harder to follow
> what the code is doing without giving us much benefit IMHO.
>
>> In any case you should not be
>> checking the compatible string of a parent.
>
>
> AFAICT this is the right way to check a compatible string in a bind
> function,
> the parent variable name here is confusing as this actually points to the
> udevice passed into the bind function, not its parent. I guess it is named
> parent rather then device to distinguish it from the per bank gpio udevices
> the bind function is instantiating.

Sounds fine.

>
> So I'm not really checking the parents compatible but its own, it just looks
> that way because of the bind function parameter name being parent.

See for example, drivers/i2c/tegra_i2c.c:

static const struct udevice_id tegra_i2c_ids[] = {
{ .compatible = "nvidia,tegra114-i2c", .data = TYPE_114 },
{ .compatible = "nvidia,tegra20-i2c", .data = TYPE_STD },
{ .compatible = "nvidia,tegra20-i2c-dvc", .data = TYPE_DVC },
{ }
};

...

i2c_bus->type = dev_get_driver_data(dev);

which gets the 'data' value. So it's pretty easy to use and is the
standard way to make a driver support multiple hardware types.

>
> Regards,
>
> Hans
>
>
>
>>
>>> +               start = 'L' - 'A';
>>> +               no_banks = 2; /* L & M */
>>> +       } else if (fdt_node_check_compatible(gd->fdt_blob,
>>> parent->of_offset,
>>> +                               "allwinner,sun8i-a23-r-pinctrl") == 0) {
>>> +               start = 'L' - 'A';
>>> +               no_banks = 1; /* L only */
>>> +       } else {
>>> +               start = 0;
>>> +               no_banks = SUNXI_GPIO_BANKS;
>>> +       }
>>> +
>>>          ctlr = (struct sunxi_gpio_reg *)fdtdec_get_addr(gd->fdt_blob,
>>>                                                     parent->of_offset,
>>> "reg");
>>> -       for (bank = 0; bank < SUNXI_GPIO_BANKS; bank++) {
>>> +       for (bank = 0; bank < no_banks; bank++) {
>>>                  struct sunxi_gpio_platdata *plat;
>>>                  struct udevice *dev;
>>>
>>> @@ -283,7 +295,7 @@ static int gpio_sunxi_bind(struct udevice *parent)
>>>                  if (!plat)
>>>                          return -ENOMEM;
>>>                  plat->regs = &ctlr->gpio_bank[bank];
>>> -               plat->bank_name = gpio_bank_name(bank);
>>> +               plat->bank_name = gpio_bank_name(start + bank);
>>>                  plat->gpio_count = SUNXI_GPIOS_PER_BANK;
>>>
>>>                  ret = device_bind(parent, parent->driver,
>>> @@ -306,6 +318,8 @@ static const struct udevice_id sunxi_gpio_ids[] = {
>>>          { .compatible = "allwinner,sun8i-a23-pinctrl" },
>>>          { .compatible = "allwinner,sun8i-a33-pinctrl" },
>>>          { .compatible = "allwinner,sun9i-a80-pinctrl" },
>>> +       { .compatible = "allwinner,sun6i-a31-r-pinctrl" },
>>> +       { .compatible = "allwinner,sun8i-a23-r-pinctrl" },
>>>          { }
>>>   };
>>>
>>> --
>>> 2.4.3
>>>
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot

Regards,
Simon