Re: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header

[Geert Uytterhoeven]

Hi Chris,

On Wed, Mar 29, 2017 at 4:55 PM, Chris Brandt <Chris.Brandt@xxxxxxxxxxx> wrote:
> On Wednesday, March 29, 2017, Linus Walleij wrote:
>> On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas@xxxxxxxxxx> wrote:
>> > +/*
>> > + * Flags used to ask software to drive the pin I/O direction
>> > +overriding the
>> > + * alternate function configuration.
>> > + * Some alternate functions require software to force I/O direction
>> > +of a pin,
>> > + * overriding the designated one.
>> > + * Refer to the HW manual to know when this flag shall be used.
>> > + */
>> > +#define SWIO_IN                        (1 << 4)
>> > +#define SWIO_OUT               (1 << 5)
>>
>> What is wrong in doing this with generic pin config using
>> PIN_CONFIG_INPUT_ENABLE and PIN_CONFIG_OUTPUT (ignoring the argument)?
>>
>> In the device tree use input-enable and add a new output-enable (with
>> unspecified value) with proper description and DT bindings?
>
> Again, that's probably fine. It seems we are still doing the same thing
> which is using the DT to pass extra config information to the driver.
> And, we can do whatever we want with that info.
>
>
>> And if you think these have no general applicability, by the end of the
>> day they are *still* pin config, not magic flags we can choose to toss in
>> with the muxing, so you can do what the Qualcomm driver does and add
>> custom pin configurations extending the generic pin config, see
>> drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
>> qcom,pull-up-strength etc.
>
> But, it seems that when you set a config option, it applies to everything
> in "pins"?
>
> I2C Example: (seem OK)
>         /* P1_6 = RIIC3SCL (bi dir) */
>         /* P1_7 = RIIC3SDA (bi dir) */
>         i2c3_pins: i2c3 {
>                 pins = <PIN(1, 6) | FUNC_1>,
>                        <PIN(1, 7) | FUNC_1>;
>                 bidirectional;
>         };

Correct.

> But, what do we do for Ethernet? All the pins are "normal" except just
> the MDIO pin needs to be bidirectional.
> That's the part I'm confused by.
> How do we flag that just the ET_MDIO needs "bidirectional"?

You add subnodes, cfr. arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts:

        avb_pins: avb {
                mux {
                        groups = "avb_link", "avb_phy_int", "avb_mdc",
                                 "avb_mii";
                        function = "avb";
                };

                pins_mdc {
                        groups = "avb_mdc";
                        drive-strength = <24>;
                };

                pins_mii_tx {
                        pins = "PIN_AVB_TX_CTL", "PIN_AVB_TXC", "PIN_AVB_TD0",
                               "PIN_AVB_TD1", "PIN_AVB_TD2", "PIN_AVB_TD3";
                        drive-strength = <12>;
                };
        };

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