diff mbox

[v3,3/4] arm/dts: AM33XX: Configure pinmuxs for D_CAN1 on AM335x-EVM

Message ID 1345119610-5247-4-git-send-email-anilkumar@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

AnilKumar, Chimata Aug. 16, 2012, 12:20 p.m. UTC
Add D_CAN1 pinctrl node to am3358_pinmux master node to export
D_CAN functionality on AM335x EVM according to pinctrl-single
driver.

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
Changes from v2:
	- Incorporated Vaibhav H's comments on v2
	  * Added dcan0 instances to am33xx.dtsi file

Changes from v1:
	- These two patches separated from c_can DT support
	  patch series.

 arch/arm/boot/dts/am335x-evm.dts |   12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Tony Lindgren Aug. 24, 2012, 8:14 p.m. UTC | #1
* AnilKumar Ch <anilkumar@ti.com> [120816 05:20]:
> Add D_CAN1 pinctrl node to am3358_pinmux master node to export
> D_CAN functionality on AM335x EVM according to pinctrl-single
> driver.
> 
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> ---
> Changes from v2:
> 	- Incorporated Vaibhav H's comments on v2
> 	  * Added dcan0 instances to am33xx.dtsi file
> 
> Changes from v1:
> 	- These two patches separated from c_can DT support
> 	  patch series.
> 
>  arch/arm/boot/dts/am335x-evm.dts |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
> index 5dd8a6b..a64c30a 100644
> --- a/arch/arm/boot/dts/am335x-evm.dts
> +++ b/arch/arm/boot/dts/am335x-evm.dts
> @@ -18,6 +18,18 @@
>  		reg = <0x80000000 0x10000000>; /* 256 MB */
>  	};
>  
> +	am3358_pinmux: pinmux@44E10800 {

This should be just &am3358_pinmux as you already include it.

> +		pinctrl-names = "default";
> +		pinctrl-0 = <&d_can1_pins>;
> +
> +		d_can1_pins: pinmux_d_can_pins {
> +			pinctrl-single,pins = <
> +				0x168 0x2	/* uart0_ctsn.d_can1_tx, OUTPUT | MODE2 */
> +				0x16C 0x32	/* uart0_rtsn.d_can1_rx, INPUT_PULLUP | MODE2 */
> +			>;
> +		};
> +	};
> +

It would better to claim these pins in the dcan driver entry
rather than set them as pins enabled by the pinctrl driver.
The reason is that in case you want to enable runtime PM related
stuff, like some more advanced wake-up features, for the rx pin
you can do it in the driver.

Then one nit.. Please lowercase all hex number in this series as that's the style
here.

Regards,

Tony
Vaibhav Hiremath Aug. 28, 2012, 6:21 a.m. UTC | #2
On 8/25/2012 1:44 AM, Tony Lindgren wrote:
> * AnilKumar Ch <anilkumar@ti.com> [120816 05:20]:
>> Add D_CAN1 pinctrl node to am3358_pinmux master node to export
>> D_CAN functionality on AM335x EVM according to pinctrl-single
>> driver.
>>
>> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
>> ---
>> Changes from v2:
>> 	- Incorporated Vaibhav H's comments on v2
>> 	  * Added dcan0 instances to am33xx.dtsi file
>>
>> Changes from v1:
>> 	- These two patches separated from c_can DT support
>> 	  patch series.
>>
>>  arch/arm/boot/dts/am335x-evm.dts |   12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
>> index 5dd8a6b..a64c30a 100644
>> --- a/arch/arm/boot/dts/am335x-evm.dts
>> +++ b/arch/arm/boot/dts/am335x-evm.dts
>> @@ -18,6 +18,18 @@
>>  		reg = <0x80000000 0x10000000>; /* 256 MB */
>>  	};
>>  
>> +	am3358_pinmux: pinmux@44E10800 {
> 
> This should be just &am3358_pinmux as you already include it.
> 

I think it won't work this way, it will result into syntax error.
The reason being is, unlike OMAP (and like other platforms, like, imx)
here we are again representing device hierarchy starting from root,

am335x-evm.dts:
/ {

	ocp {
		...
	};

};

Although I am not too familiar with DTS compiler, but below code results
in syntax error:

am335x-evm.dts:
/ {

	ocp {
		...
		&am3358_pinmux {
			...
		};
	};

};


Thanks,
Vaibhav

>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&d_can1_pins>;
>> +
>> +		d_can1_pins: pinmux_d_can_pins {
>> +			pinctrl-single,pins = <
>> +				0x168 0x2	/* uart0_ctsn.d_can1_tx, OUTPUT | MODE2 */
>> +				0x16C 0x32	/* uart0_rtsn.d_can1_rx, INPUT_PULLUP | MODE2 */
>> +			>;
>> +		};
>> +	};
>> +
> 
> It would better to claim these pins in the dcan driver entry
> rather than set them as pins enabled by the pinctrl driver.
> The reason is that in case you want to enable runtime PM related
> stuff, like some more advanced wake-up features, for the rx pin
> you can do it in the driver.
> 
> Then one nit.. Please lowercase all hex number in this series as that's the style
> here.
> 
> Regards,
> 
> Tony
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>
diff mbox

Patch

diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 5dd8a6b..a64c30a 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -18,6 +18,18 @@ 
 		reg = <0x80000000 0x10000000>; /* 256 MB */
 	};
 
+	am3358_pinmux: pinmux@44E10800 {
+		pinctrl-names = "default";
+		pinctrl-0 = <&d_can1_pins>;
+
+		d_can1_pins: pinmux_d_can_pins {
+			pinctrl-single,pins = <
+				0x168 0x2	/* uart0_ctsn.d_can1_tx, OUTPUT | MODE2 */
+				0x16C 0x32	/* uart0_rtsn.d_can1_rx, INPUT_PULLUP | MODE2 */
+			>;
+		};
+	};
+
 	ocp {
 		uart1: serial@44E09000 {
 			status = "okay";