[PATCHv2 3/3] ARM:dts:sunxi:ps2 dt nodes for A10/A20 PS2 controller.

[PATCHv2 3/3] ARM:dts:sunxi:ps2 dt nodes for A10/A20 PS2 controller.

[vishnupatekar]

 Added ps2 nodes in lime2 board dts. By default ps20 and ps21 nodes are
 commented as ps20 pins conflict with HDMI connector
 on Lime2 Board.

Signed-off-by: vishnupatekar <VishnuPatekar0510-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/sun4i-a10.dtsi                |   27 +++++++++++++++++++++
 arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts |   13 ++++++++++
 arch/arm/boot/dts/sun7i-a20.dtsi                |   29 +++++++++++++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 5e2ec2d..4726e8d 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -615,6 +615,19 @@
 				allwinner,drive = <0>;
 				allwinner,pull = <0>;
 			};
+			ps2_0_pins: ps2_0@0 {
+				    allwinner,pins = "PI20","PI21";
+				    allwinner,function = "ps2";
+				    allwinner,drive = <0>;
+				    allwinner,pull = <0>;
+			};
+			ps2_1_pins: ps2_1@0 {
+				    allwinner,pins = "PH12","PH13";
+				    allwinner,function = "ps2";
+				    allwinner,drive = <0>;
+				    allwinner,pull = <0>;
+			};
+
 		};
 
 		timer@01c20c00 {
@@ -781,5 +794,19 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 		};
+		ps20: ps2@0x01c2a000 {
+			compatible = "allwinner,sun4i-a10-ps2";
+			reg = <0x01c2a000 0x400>;
+			interrupts = <0 62 4>;
+			clocks = <&apb1_gates 6>;
+			status = "disabled";
+		};
+		ps21: ps2@0x01c2a400 {
+			compatible = "allwinner,sun4i-a10-ps2";
+			reg = <0x01c2a400 0x400>;
+			interrupts = <0 63 4>;
+			clocks = <&apb1_gates 7>;
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
index ed364d5..5624e63 100644
--- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
@@ -112,6 +112,19 @@
 			pinctrl-0 = <&uart0_pins_a>;
 			status = "okay";
 		};
+		/* PS2 0 and PS2 1 are disabled by default
+		To enable PS2 0 and PS2 1 uncomment below ps20 and ps21 nodes
+		Please note that ps20 pins conflict with HDMI on Lime2 Board*/
+		/*ps20: ps2@0x01c2a000 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&ps2_0_pins>;
+			status = "okay";
+		};
+		ps21: ps2@0x01c2a400 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&ps2_1_pins>;
+			status = "okay";
+		};*/
 
 		i2c0: i2c@01c2ac00 {
 			pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 8605f2b..96fe37d 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -852,6 +852,19 @@
 				    allwinner,drive = <0>;
 				    allwinner,pull = <0>;
 			};
+			ps2_0_pins: ps2_0@0 {
+				    allwinner,pins = "PI20","PI21";
+				    allwinner,function = "ps2";
+				    allwinner,drive = <0>;
+				    allwinner,pull = <0>;
+			};
+			ps2_1_pins: ps2_1@0 {
+				    allwinner,pins = "PH12","PH13";
+				    allwinner,function = "ps2";
+				    allwinner,drive = <0>;
+				    allwinner,pull = <0>;
+			};
+
 		};
 
 		timer@01c20c00 {
@@ -1079,5 +1092,21 @@
 			#interrupt-cells = <3>;
 			interrupts = <1 9 0xf04>;
 		};
+		ps20: ps2@0x01c2a000 {
+			compatible = "allwinner,sun7i-a20-ps2";
+			reg = <0x01c2a000 0x400>;
+			interrupts = <0 62 4>;
+			clocks = <&apb1_gates 6>;
+			status = "disabled";
+		};
+		ps21: ps2@0x01c2a400 {
+			compatible = "allwinner,sun7i-a20-ps2";
+			reg = <0x01c2a400 0x400>;
+			interrupts = <0 63 4>;
+			clocks = <&apb1_gates 7>;
+			status = "disabled";
+		};
+
+
 	};
 };
-- 
1.7.9.5

Re: [PATCHv2 3/3] ARM:dts:sunxi:ps2 dt nodes for A10/A20 PS2 controller.

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3421 bytes --]

Hi,

Your commit title should be less that 80 chars.

It should also be formatted using "ARM: sunxi: dts:" as a prefix (More
specific to less specific).

On Sun, Dec 07, 2014 at 04:45:19AM +0530, vishnupatekar wrote:
>  Added ps2 nodes in lime2 board dts. By default ps20 and ps21 nodes are
>  commented as ps20 pins conflict with HDMI connector
>  on Lime2 Board.
> 
> Signed-off-by: vishnupatekar <VishnuPatekar0510-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  arch/arm/boot/dts/sun4i-a10.dtsi                |   27 +++++++++++++++++++++
>  arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts |   13 ++++++++++
>  arch/arm/boot/dts/sun7i-a20.dtsi                |   29 +++++++++++++++++++++++
>  3 files changed, 69 insertions(+)

This patch should also be split into three different patches:
  - The one adding the nodes for the controller to the DTSI
  - The one adding the pinctrl nodes
  - The one enabling the controller on the board.

> 
> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> index 5e2ec2d..4726e8d 100644
> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> @@ -615,6 +615,19 @@
>  				allwinner,drive = <0>;
>  				allwinner,pull = <0>;
>  			};

A newline here please.

> +			ps2_0_pins: ps2_0@0 {

You called the node ps20, you probably want to call it the same way
here. And we usually have a suffix like pins_a whenever we have
several muxing options (as it's probably the case).

> +				    allwinner,pins = "PI20","PI21";

A whitespace after the comma please.

> +				    allwinner,function = "ps2";
> +				    allwinner,drive = <0>;
> +				    allwinner,pull = <0>;
> +			};

Newline.

> +			ps2_1_pins: ps2_1@0 {
> +				    allwinner,pins = "PH12","PH13";
> +				    allwinner,function = "ps2";
> +				    allwinner,drive = <0>;
> +				    allwinner,pull = <0>;
> +			};
> +
>  		};
>  
>  		timer@01c20c00 {
> @@ -781,5 +794,19 @@
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  		};

Newline.

> +		ps20: ps2@0x01c2a000 {

Drop the 0x

> +			compatible = "allwinner,sun4i-a10-ps2";
> +			reg = <0x01c2a000 0x400>;
> +			interrupts = <0 62 4>;
> +			clocks = <&apb1_gates 6>;
> +			status = "disabled";
> +		};

Newline

> +		ps21: ps2@0x01c2a400 {
> +			compatible = "allwinner,sun4i-a10-ps2";
> +			reg = <0x01c2a400 0x400>;
> +			interrupts = <0 63 4>;
> +			clocks = <&apb1_gates 7>;
> +			status = "disabled";
> +		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> index ed364d5..5624e63 100644
> --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> @@ -112,6 +112,19 @@
>  			pinctrl-0 = <&uart0_pins_a>;
>  			status = "okay";
>  		};
> +		/* PS2 0 and PS2 1 are disabled by default
> +		To enable PS2 0 and PS2 1 uncomment below ps20 and ps21 nodes
> +		Please note that ps20 pins conflict with HDMI on Lime2 Board*/
> +		/*ps20: ps2@0x01c2a000 {
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&ps2_0_pins>;
> +			status = "okay";
> +		};
> +		ps21: ps2@0x01c2a400 {
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&ps2_1_pins>;
> +			status = "okay";
> +		};*/

Hmmm, no, no comments in the DTS.

Especially when it's that trivial to enable.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCHv2 3/3] ARM:dts:sunxi:ps2 dt nodes for A10/A20 PS2 controller.

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

Hi,

Please keep the Cc list.

On Wed, Dec 10, 2014 at 01:39:52AM +0530, Vishnu Patekar wrote:
> > > --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > > +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > > @@ -112,6 +112,19 @@
> > >                       pinctrl-0 = <&uart0_pins_a>;
> > >                       status = "okay";
> > >               };
> > > +             /* PS2 0 and PS2 1 are disabled by default
> > > +             To enable PS2 0 and PS2 1 uncomment below ps20 and ps21
> > nodes
> > > +             Please note that ps20 pins conflict with HDMI on Lime2
> > Board*/
> > > +             /*ps20: ps2@0x01c2a000 {
> > > +                     pinctrl-names = "default";
> > > +                     pinctrl-0 = <&ps2_0_pins>;
> > > +                     status = "okay";
> > > +             };
> > > +             ps21: ps2@0x01c2a400 {
> > > +                     pinctrl-names = "default";
> > > +                     pinctrl-0 = <&ps2_1_pins>;
> > > +                     status = "okay";
> > > +             };*/
> >
> > Hmmm, no, no comments in the DTS.
> >
> > Especially when it's that trivial to enable.
>
> Okie, I'll just keep ps20 and ps21 nodes commented.

No, the comments themselves were great. But the nodes themselves
shouldn't be there in the first place.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com