Re: [net-next PATCH v4 3/5] ARM: dts: AM33XX: Add pinmux configuration for CPSW to beaglebone

[Florian Vaussard <florian.vaussard@epfl.ch>]

Hello,

On 06/05/2013 07:08 PM, Mugunthan V N wrote:
> Add pinmux configurations for MII based CPSW ethernet to am335x-bone.
> In this patch, only single named mode/state is added and these pins
> are configured during pinctrl driver initialization.
>
> Default mode is nothing but the values required for the module during
> active state. With this configurations module is functional as
> expected.
>
> Todo:
> - if an idle state is available for pins, add support for it.
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>   arch/arm/boot/dts/am335x-bone.dts |   38 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 38 insertions(+)
>
> diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
> index 4b5a8e0..008a13b 100644
> --- a/arch/arm/boot/dts/am335x-bone.dts
> +++ b/arch/arm/boot/dts/am335x-bone.dts
> @@ -36,6 +36,33 @@
>   				0x60 0x17	/* gpmc_a8.gpio1_24, OUTPUT_PULLUP | MODE7 */
>   			>;
>   		};
> +
> +		cpsw_default: cpsw_default {
> +			pinctrl-single,pins = <
> +				/* Slave 1 */
> +				0x110 0x20	/* mii1_rxerr.mii1_rxerr, MODE0 | INPUT */

Here I guess that the comment should be INPUT_PULLDOWN, instead of INPUT.

> +				0x114 0x0	/* mii1_txen.mii1_txen, MODE0 | OUTPUT */

As bit PULLUDENABLE is 0, the pulldown is enabled according to the TRM, 
right?
Is this a desirable behaviour, as it will consume power when outputting 
a high level?
And consequently, the comment should probably be OUTPUT_PULLDOWN, no?

> +				0x118 0x20	/* mii1_rxdv.mii1_rxdv, MODE0 | INPUT_PULLDOWN */
> +				0x11c 0x0	/* mii1_txd3.mii1_txd3, MODE0 | OUTPUT */
> +				0x120 0x0	/* mii1_txd2.mii1_txd2, MODE0 | OUTPUT */
> +				0x124 0x0	/* mii1_txd1.mii1_txd1, MODE0 | OUTPUT */
> +				0x128 0x0	/* mii1_txd0.mii1_txd0, MODE0 | OUTPUT */

Same questions here.

> +				0x12c 0x20	/* mii1_txclk.mii1_txclk, MODE0 | INPUT_PULLDOWN */
> +				0x130 0x20	/* mii1_rxclk.mii1_rxclk, MODE0 | INPUT_PULLDOWN */
> +				0x134 0x20	/* mii1_rxd3.mii1_rxd3, MODE0 | INPUT_PULLDOWN */
> +				0x138 0x20	/* mii1_rxd2.mii1_rxd2, MODE0 | INPUT_PULLDOWN */
> +				0x13c 0x20	/* mii1_rxd1.mii1_rxd1, MODE0 | INPUT_PULLDOWN */
> +				0x140 0x20	/* mii1_rxd0.mii1_rxd0, MODE0 | INPUT_PULLDOWN */
> +			>;
> +		};
> +
> +		davinci_mdio_default: davinci_mdio_default {
> +			pinctrl-single,pins = <
> +				/* MDIO */
> +				0x148 0x30	/* mdio_data.mdio_data, MODE0 | INPUT_PULLUP */
> +				0x14c 0x10	/* mdio_clk.mdio_clk, MODE0 | OUTPUT_PULLUP */
> +			>;
> +		};
>   	};
>
>   	ocp {
> @@ -138,3 +165,14 @@
>   	phy_id = <&davinci_mdio>, <1>;
>   	phy-mode = "mii";
>   };
> +
> +&mac {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&cpsw_default>;
> +
> +};
> +
> +&davinci_mdio {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&davinci_mdio_default>;
> +};
>

Regards,

Florian