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

[Chris Brandt]

On Wednesday, March 29, 2017, Linus Walleij wrote:
> On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> wrote:
> 
> > Add dt-bindings for Renesas r7s72100 pin controller header file.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> 
> > +/*
> > + * Pin is bi-directional.
> > + * An alternate function that needs both input/output functionalities
> > +shall
> > + * be configured as bidirectional.
> > + * Eg. SDA/SCL pins of an I2c interface.
> > + */
> > +#define BI_DIR                 (1 << 3)
> 
> Any specific reason why this should not simply be added to
> include/linux/pinctrl/pinconf-generic.h
> as PIN_CONFIG_BIDIRECTIONAL and parsed in drivers/pinctrl/pinconf-
> generic.c from the (new) DT property "bidirectional" simply?

I see your point. It would cut down from every driver out there
inventing some new property or config instead of everyone just sharing
a fixed set.
Maybe someone else out there will end up having a need for a
"bidirectional" option.


> > +/*
> > + * 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;
	};


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"?

	/* Ethernet */
	ether_pins: ether {
		/* Ethernet on Ports 1,2,3,5 */
		pins = <PIN(1, 14) | FUNC_4)>, /* P1_14 = ET_COL */
			<PIN(5, 9) | FUNC_2)>,	/* P5_9 = ET_MDC */
			<PIN(3, 3) | FUNC_2)>,	/* P3_3 = ET_MDIO !!!! (bi dir) !!!!!! */
			<PIN(3, 4) | FUNC_2)>,	/* P3_4 = ET_RXCLK */
			<PIN(3, 5) | FUNC_2)>,	/* P3_5 = ET_RXER */
			<PIN(3, 6) | FUNC_2)>,	/* P3_6 = ET_RXDV */
			<PIN(2, 0) | FUNC_2)>,	/* P2_0 = ET_TXCLK */
			<PIN(2, 1) | FUNC_2)>,	/* P2_1 = ET_TXER */
			<PIN(2, 2) | FUNC_2)>,	/* P2_2 = ET_TXEN */
			<PIN(2, 3) | FUNC_2)>,	/* P2_3 = ET_CRS */
			<PIN(2, 4) | FUNC_2)>,	/* P2_4 = ET_TXD0 */
			<PIN(2, 5) | FUNC_2)>,	/* P2_5 = ET_TXD1 */
			<PIN(2, 6) | FUNC_2)>,	/* P2_6 = ET_TXD2 */
			<PIN(2, 7) | FUNC_2)>,	/* P2_7 = ET_TXD3 */
			<PIN(2, 8) | FUNC_2)>,	/* P2_8 = ET_RXD0 */
			<PIN(2, 9) | FUNC_2)>,	/* P2_9 = ET_RXD1 */
			<PIN(2, 10) | FUNC_2)>, /* P2_10 = ET_RXD2 */
			<PIN(2, 11) | FUNC_2)>; /* P2_11 = ET_RXD3 */
	};


Chris