Re: [PATCH v2 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc

[jacopo]

Hi Geert, Linus

On Wed, Mar 29, 2017 at 01:20:39PM +0200, Geert Uytterhoeven wrote:
> Hi Linus,
> 
> On Wed, Mar 29, 2017 at 12:15 PM, Linus Walleij
> <linus.walleij@xxxxxxxxxx> wrote:
> > On Wed, Mar 29, 2017 at 9:35 AM, Geert Uytterhoeven
> > <geert@xxxxxxxxxxxxxx> wrote:
> >>> See for example:
> >>> include/dt-bindings/pinctrl/mt65xx.h
> >>>
> >>> And how that is used in:
> >>> arch/arm/boot/dts/mt2701-pinfunc.h
> >>> arch/arm/boot/dts/mt2701-evb.dts
> >>>
> >>> The docs are here:
> >>> Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt
> >>
> >> All of the above pack the information for a pin into a single 32-bit integer.
> >> Is that what you want us to use, too?
> >> Currently we use two integers: 1) pin index, and 2) function/flags combo.
> >
> > I don't really know what you need, sorry. But some kind of figure, yes.
> > I would say whatever makes sense. 16+16 bits makes sense in most
> > combinatorial spaces does it not? If you split 32 bits  in 16 bits for
> > pin and 16 bits for function, do you have more than 2^16 pins or 2^16
> > functions?
> >
> > If you really do we may need to go for u64 but ... really? Is there
> > a rational reason for that other than "we did it like this first"?
> >
> > I do not understand the notion of "flags" here. I hope that is not referring
> 
> Flags refers to BI_DIR, SWIO_IN, and SWIO_OUT, from
> https://patchwork.kernel.org/patch/9643047/
> 
> 32-bit should be enough to cover pins, function, and flags.
> 

Geert already replied, but to avoid any confusion I'll try to remove
from driver the use of "pin config" when referring to this three
flags, which are just additional informations the pin controller needs
to perform pin muxing properly. They're not related the standard pin
config properties (pull-up/down, bias etc.. actually our hardware does
not even support these natively)

> > to pin config, because I expect that to use the standard pin config
> > bindings outside of the pinmux value which should just define the
> > pin+function combo:
> >
> > node {
> >     pinmux = <PIN_NUMBER_PINMUX>;
> >     GENERIC_PINCONFIG;
> > };
> >
> > Example from Mediatek:
> >
> > i2c1_pins_a: i2c1@0 {
> >     pins {
> >         pinmux = <MT8135_PIN_195_SDA1__FUNC_SDA1>,
> >                         <MT8135_PIN_196_SCL1__FUNC_SCL1>;
> 
> If we follow this example, then we can list all combinations in
> include/dt-bindings/pinctrl/r7s72100-pinctrl.h, instead of creating the value
> by combining the bits using a macro where we need it in the DTS.
> 
> It's gonna be a long list, though...
> 

I'm strongly in favour of something like
    pinmux = <PIN(1, 4) | FUNC# | FLAGS>, .... ;

opposed to
    pinmux = <PIN1_4_FUNC#_FLAGS>, ... ;

Not only because it will save use from having a loong list(*) of macros that has
to be kept up to date when/if new RZ hardware will arrive, but also
because of readability and simplicity for down-stream and BSP users.
Speaking of which, I would like to know what does Chris think of this.

Is there any strong preference between the two for you Linus?

Thanks
   j

(*) 12 ports, 16 pins, 8 functions, 3 flags: you can do the math
yourselves and see this is going to be hard to maintain.

> >         bias-pull-up = <55>;
> >     };
> > };
> >
> > So this allows for a combine pin+function number but pull ups,
> > bias etc are not baked into the thing, they have to be added on
> > separately with the generic bindings, which is nice and very readable.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds