[U-Boot] [PATCH v3 17/25] rockchip: rk3288: Add pinctrl driver

[Masahiro Yamada]

Hi Simon,


2015-07-07 2:32 GMT+09:00 Simon Glass <sjg at chromium.org>:
> Hi Masahiro,
>
> On 6 July 2015 at 11:24, Masahiro Yamada <yamada.masahiro at socionext.com> wrote:
>>
>> Hi Simon,
>>
>>
>> 2015-06-24 8:29 GMT+09:00 Simon Glass <sjg at chromium.org>:
>>
>> > +
>> > +static int rk3288_pinctrl_get_periph_id(struct udevice *dev,
>> > +                                       struct udevice *periph)
>> > +{
>> > +       u32 cell[3];
>> > +       int ret;
>> > +
>> > +       ret = fdtdec_get_int_array(gd->fdt_blob, periph->of_offset,
>> > +                                  "interrupts", cell, ARRAY_SIZE(cell));
>> > +       if (ret < 0)
>> > +               return -EINVAL;
>> > +
>> > +       switch (cell[1]) {
>> > +       case 44:
>> > +               return PERIPH_ID_SPI0;
>> > +       case 45:
>> > +               return PERIPH_ID_SPI1;
>> > +       case 46:
>> > +               return PERIPH_ID_SPI2;
>> > +       case 60:
>> > +               return PERIPH_ID_I2C0;
>> > +       case 62: /* Note strange order */
>> > +               return PERIPH_ID_I2C1;
>> > +       case 61:
>> > +               return PERIPH_ID_I2C2;
>> > +       case 63:
>> > +               return PERIPH_ID_I2C3;
>> > +       case 64:
>> > +               return PERIPH_ID_I2C4;
>> > +       case 65:
>> > +               return PERIPH_ID_I2C5;
>> > +       }
>> > +
>> > +       return -ENOENT;
>> > +}
>>
>> Weird.
>>
>> Do you parse the "interrupts" property
>> to identify the peripheral?
>>
>> This is what I really do not like to do.
>
> Yes. It's similar to a clock number. It doesn't really matter what we
> use. The interrupt number is a fixed part of the binding and will
> never change.


I guess it is abuse of "interrupts" property.

Moreover, it can not cover cases where a single signal can be routed to
several pins.


For example, say we want to set pinmux for the I2C channel 0 (SDA0 and SCL0).

On some vendors' SoCs, it is possible to connect SDA0 to either pin10
or pin20 for example.
Likewise, SCL0 can be connected to pin11 or pin21.
(Yes, it is possible on my SoCs)


The interrupt ID can identify it is I2C0, and that's it.

Some boards expect the signals come out of pin10, 11,
and other boards pin20, 21.

The interrupt ID cannot distinguish those cases; the "pinctrl-*"
property can do it.

This is mentioned in Documentation/pinctrl.txt and
that is why a single function can have multiple pin-groups in Linux's
pinctrl drivers.

I think this design cannot meet some vendors' demand.
At least, I am unhappy with it.

Could you hold your pinctrl patches and give me time to discuss a bit
more, please?

I understand how pinctrl drivers work in Linux, so please give me some time
to consider if better design might be possible.



-- 
Best Regards
Masahiro Yamada