All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH linux dev-4.18] ASPEED: dts: Define the gpio-line-names property for aspeed-g5
@ 2018-09-10  7:27 Wang, Kuiying
  2018-09-10  7:36 ` Andrew Jeffery
  0 siblings, 1 reply; 4+ messages in thread
From: Wang, Kuiying @ 2018-09-10  7:27 UTC (permalink / raw)
  To: Andrew Jeffery, Andrew Geissler, Joel Stanley
  Cc: Brad Bishop, Andrew Geissler, chunhui.jia, kunyi731, Mihm, James,
	Tanous, Ed, Feist, James, Jia, Chunhui, Patrick Venture,
	OpenBMC Maillist, Li, Yong B, Yang, Cheng C, Xu, Qiang, Nguyen,
	Hai V

Gpio-line-names property of aspeed-g5 is not defined in the upstream submission.
It is required by new gpio API "linux/gpio.h"

commit 9dfd72ff5f628d7aaa10728756ce42bd5f768d4f (HEAD -> dev-4.18)
Author: Kuiying Wang <kuiying.wang@intel.com>
Date:   Fri Sep 7 18:01:41 2018 +0800

    Define the gpio-line-names property for aspeed-g5

    Based on aspeed AST-2500 Datasheet spec, there are 232 gpios.
    So defines the gpio line name from "A0" to "AC7".

    Signed-off-by: Kuiying Wang <kuiying.wang@intel.com>

diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index d92f047907de..6d664f4f1621 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -265,6 +265,36 @@
                                gpio-ranges = <&pinctrl 0 0 220>;
                                clocks = <&syscon ASPEED_CLK_APB>;
                                interrupt-controller;
+                             gpio-line-names = "A0","A1","A2","A3","A4","A5","A6","A7",
+                                   "B0","B1","B2","B3","B4","B5","B6","B7",
+                                   "C0","C1","C2","C3","C4","C5","C6","C7",
+                                   "D0","D1","D2","D3","D4","D5","D6","D7",
+                                   "E0","E1","E2","E3","E4","E5","E6","E7",
+                                   "F0","F1","F2","F3","F4","F5","F6","F7",
+                                   "G0","G1","G2","G3","G4","G5","G6","G7",
+                                   "H0","H1","H2","H3","H4","H5","H6","H7",
+                                   "I0","I1","I2","I3","I4","I5","I6","I7",
+                                   "J0","J1","J2","J3","J4","J5","J6","J7",
+                                   "K0","E1","E2","E3","E4","E5","E6","E7",
+                                   "L0","L1","L2","L3","L4","L5","L6","L7",
+                                   "M0","M1","M2","M3","M4","M5","M6","M7",
+                                   "N0","N1","N2","N3","N4","N5","N6","N7",
+                                   "O0","O1","O2","O3","O4","O5","O6","O7",
+                                   "P0","P1","P2","P3","P4","P5","P6","P7",
+                                   "Q0","Q1","Q2","Q3","Q4","Q5","Q6","Q7",
+                                   "R0","R1","R2","R3","R4","R5","R6","R7",
+                                   "S0","S1","S2","S3","S4","S5","S6","S7",
+                                   "T0","T1","T2","T3","T4","T5","T6","T7",
+                                   "U0","U1","U2","U3","U4","U5","U6","U7",
+                                   "V0","V1","V2","V3","V4","V5","V6","V7",
+                                   "W0","W1","W2","W3","W4","W5","W6","W7",
+                                   "X0","X1","X2","X3","X4","X5","X6","X7",
+                                   "Y0","Y1","Y2","Y3","Y4","Y5","Y6","Y7",
+                                   "Z0","Z1","Z2","Z3","Z4","Z5","Z6","Z7",
+                                   "AA0","AA1","AA2","AA3","AA4","AA5","AA6","AA7",
+                                   "AB0","AB1","AB2","AB3","AB4","AB5","AB6","AB7",
+                                   "AC0","AC1","AC2","AC3","AC4","AC5","AC6","AC7"
+                             ;
                        };

Thanks,
Kuiying.

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH linux dev-4.18] ASPEED: dts: Define the gpio-line-names property for aspeed-g5
  2018-09-10  7:27 [PATCH linux dev-4.18] ASPEED: dts: Define the gpio-line-names property for aspeed-g5 Wang, Kuiying
@ 2018-09-10  7:36 ` Andrew Jeffery
  2018-09-10  7:56   ` Wang, Kuiying
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Jeffery @ 2018-09-10  7:36 UTC (permalink / raw)
  To: Wang, Kuiying, Andrew Geissler, Joel Stanley
  Cc: Brad Bishop, Andrew Geissler, chunhui.jia, kunyi731, Mihm, James,
	Tanous, Ed, Feist, James, Jia, Chunhui, Patrick Venture,
	OpenBMC Maillist, Li, Yong B, Yang, Cheng C, Xu, Qiang, Nguyen,
	Hai V

Hi Kuiying,

On Mon, 10 Sep 2018, at 16:57, Wang, Kuiying wrote:
> Gpio-line-names property of aspeed-g5 is not defined in the upstream submission.
> It is required by new gpio API "linux/gpio.h"
> 
> commit 9dfd72ff5f628d7aaa10728756ce42bd5f768d4f (HEAD -> dev-4.18)
> Author: Kuiying Wang <kuiying.wang@intel.com>
> Date:   Fri Sep 7 18:01:41 2018 +0800
> 
>     Define the gpio-line-names property for aspeed-g5
> 
>     Based on aspeed AST-2500 Datasheet spec, there are 232 gpios.
>     So defines the gpio line name from "A0" to "AC7".
> 
>     Signed-off-by: Kuiying Wang <kuiying.wang@intel.com>
> 
> diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/
> aspeed-g5.dtsi
> index d92f047907de..6d664f4f1621 100644
> --- a/arch/arm/boot/dts/aspeed-g5.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g5.dtsi
> @@ -265,6 +265,36 @@
>                                 gpio-ranges = <&pinctrl 0 0 220>;
>                                 clocks = <&syscon ASPEED_CLK_APB>;
>                                 interrupt-controller;
> +                             gpio-line-names = 
> "A0","A1","A2","A3","A4","A5","A6","A7",
> +                                   
> "B0","B1","B2","B3","B4","B5","B6","B7",
> +                                   
> "C0","C1","C2","C3","C4","C5","C6","C7",
> +                                   
> "D0","D1","D2","D3","D4","D5","D6","D7",
> +                                   
> "E0","E1","E2","E3","E4","E5","E6","E7",
> +                                   
> "F0","F1","F2","F3","F4","F5","F6","F7",
> +                                   
> "G0","G1","G2","G3","G4","G5","G6","G7",
> +                                   
> "H0","H1","H2","H3","H4","H5","H6","H7",
> +                                   
> "I0","I1","I2","I3","I4","I5","I6","I7",
> +                                   
> "J0","J1","J2","J3","J4","J5","J6","J7",
> +                                   
> "K0","E1","E2","E3","E4","E5","E6","E7",
> +                                   
> "L0","L1","L2","L3","L4","L5","L6","L7",
> +                                   
> "M0","M1","M2","M3","M4","M5","M6","M7",
> +                                   
> "N0","N1","N2","N3","N4","N5","N6","N7",
> +                                   
> "O0","O1","O2","O3","O4","O5","O6","O7",
> +                                   
> "P0","P1","P2","P3","P4","P5","P6","P7",
> +                                   
> "Q0","Q1","Q2","Q3","Q4","Q5","Q6","Q7",
> +                                   
> "R0","R1","R2","R3","R4","R5","R6","R7",
> +                                   
> "S0","S1","S2","S3","S4","S5","S6","S7",
> +                                   
> "T0","T1","T2","T3","T4","T5","T6","T7",
> +                                   
> "U0","U1","U2","U3","U4","U5","U6","U7",
> +                                   
> "V0","V1","V2","V3","V4","V5","V6","V7",
> +                                   
> "W0","W1","W2","W3","W4","W5","W6","W7",
> +                                   
> "X0","X1","X2","X3","X4","X5","X6","X7",
> +                                   
> "Y0","Y1","Y2","Y3","Y4","Y5","Y6","Y7",
> +                                   
> "Z0","Z1","Z2","Z3","Z4","Z5","Z6","Z7",
> +                                   
> "AA0","AA1","AA2","AA3","AA4","AA5","AA6","AA7",
> +                                   
> "AB0","AB1","AB2","AB3","AB4","AB5","AB6","AB7",
> +                                   
> "AC0","AC1","AC2","AC3","AC4","AC5","AC6","AC7"
> +                             ;
>                         };

I have to say I don't find this terribly useful. It's much better if the platform DTS adds a name that captures the semantic meaning of the line rather than simply indicating its position in a bank. It's pretty straight forward to convert between the raw index and the "bank name" (e.g. GPIOV3). If it helps Joel wrote a little python script for it:

https://github.com/shenki/aspeed-kernel-tests/blob/master/num-to-gpio

Cheers,

Andrew

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH linux dev-4.18] ASPEED: dts: Define the gpio-line-names property for aspeed-g5
  2018-09-10  7:36 ` Andrew Jeffery
@ 2018-09-10  7:56   ` Wang, Kuiying
  2018-09-11  1:28     ` Andrew Jeffery
  0 siblings, 1 reply; 4+ messages in thread
From: Wang, Kuiying @ 2018-09-10  7:56 UTC (permalink / raw)
  To: Andrew Jeffery, Andrew Geissler, Joel Stanley
  Cc: Brad Bishop, Andrew Geissler, chunhui.jia, kunyi731, Mihm, James,
	Tanous, Ed, Feist, James, Jia, Chunhui, Patrick Venture,
	OpenBMC Maillist, Li, Yong B, Yang, Cheng C, Xu, Qiang, Nguyen,
	Hai V

Hi Andrew J,
This is just common definition for aspeed-g5.
There will be different configuration for specified platform just like following:
https://gerrit.openbmc-project.xyz/#/c/openbmc/meta-intel/+/12178/1/meta-intel/meta-s2600wf/recipes-phosphor/skeleton/obmc-libobmc-intf/gpio_defs.json 


Thanks,
Kuiying.


-----Original Message-----
From: Andrew Jeffery [mailto:andrew@aj.id.au] 
Sent: Monday, September 10, 2018 3:37 PM
To: Wang, Kuiying <kuiying.wang@intel.com>; Andrew Geissler <geissonator@gmail.com>; Joel Stanley <joel@jms.id.au>
Cc: Brad Bishop <bradleyb@fuzziesquirrel.com>; Andrew Geissler <geissonator@yahoo.com>; chunhui.jia@linux.intel.com; kunyi731@gmail.com; Mihm, James <james.mihm@intel.com>; Tanous, Ed <ed.tanous@intel.com>; Feist, James <james.feist@intel.com>; Jia, Chunhui <chunhui.jia@intel.com>; Patrick Venture <venture@google.com>; OpenBMC Maillist <openbmc@lists.ozlabs.org>; Li, Yong B <yong.b.li@intel.com>; Yang, Cheng C <cheng.c.yang@intel.com>; Xu, Qiang <qiang.xu@intel.com>; Nguyen, Hai V <hai.v.nguyen@intel.com>
Subject: Re: [PATCH linux dev-4.18] ASPEED: dts: Define the gpio-line-names property for aspeed-g5

Hi Kuiying,

On Mon, 10 Sep 2018, at 16:57, Wang, Kuiying wrote:
> Gpio-line-names property of aspeed-g5 is not defined in the upstream submission.
> It is required by new gpio API "linux/gpio.h"
> 
> commit 9dfd72ff5f628d7aaa10728756ce42bd5f768d4f (HEAD -> dev-4.18)
> Author: Kuiying Wang <kuiying.wang@intel.com>
> Date:   Fri Sep 7 18:01:41 2018 +0800
> 
>     Define the gpio-line-names property for aspeed-g5
> 
>     Based on aspeed AST-2500 Datasheet spec, there are 232 gpios.
>     So defines the gpio line name from "A0" to "AC7".
> 
>     Signed-off-by: Kuiying Wang <kuiying.wang@intel.com>
> 
> diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/ 
> aspeed-g5.dtsi index d92f047907de..6d664f4f1621 100644
> --- a/arch/arm/boot/dts/aspeed-g5.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g5.dtsi
> @@ -265,6 +265,36 @@
>                                 gpio-ranges = <&pinctrl 0 0 220>;
>                                 clocks = <&syscon ASPEED_CLK_APB>;
>                                 interrupt-controller;
> +                             gpio-line-names =
> "A0","A1","A2","A3","A4","A5","A6","A7",
> +                                   
> "B0","B1","B2","B3","B4","B5","B6","B7",
> +                                   
> "C0","C1","C2","C3","C4","C5","C6","C7",
> +                                   
> "D0","D1","D2","D3","D4","D5","D6","D7",
> +                                   
> "E0","E1","E2","E3","E4","E5","E6","E7",
> +                                   
> "F0","F1","F2","F3","F4","F5","F6","F7",
> +                                   
> "G0","G1","G2","G3","G4","G5","G6","G7",
> +                                   
> "H0","H1","H2","H3","H4","H5","H6","H7",
> +                                   
> "I0","I1","I2","I3","I4","I5","I6","I7",
> +                                   
> "J0","J1","J2","J3","J4","J5","J6","J7",
> +                                   
> "K0","E1","E2","E3","E4","E5","E6","E7",
> +                                   
> "L0","L1","L2","L3","L4","L5","L6","L7",
> +                                   
> "M0","M1","M2","M3","M4","M5","M6","M7",
> +                                   
> "N0","N1","N2","N3","N4","N5","N6","N7",
> +                                   
> "O0","O1","O2","O3","O4","O5","O6","O7",
> +                                   
> "P0","P1","P2","P3","P4","P5","P6","P7",
> +                                   
> "Q0","Q1","Q2","Q3","Q4","Q5","Q6","Q7",
> +                                   
> "R0","R1","R2","R3","R4","R5","R6","R7",
> +                                   
> "S0","S1","S2","S3","S4","S5","S6","S7",
> +                                   
> "T0","T1","T2","T3","T4","T5","T6","T7",
> +                                   
> "U0","U1","U2","U3","U4","U5","U6","U7",
> +                                   
> "V0","V1","V2","V3","V4","V5","V6","V7",
> +                                   
> "W0","W1","W2","W3","W4","W5","W6","W7",
> +                                   
> "X0","X1","X2","X3","X4","X5","X6","X7",
> +                                   
> "Y0","Y1","Y2","Y3","Y4","Y5","Y6","Y7",
> +                                   
> "Z0","Z1","Z2","Z3","Z4","Z5","Z6","Z7",
> +                                   
> "AA0","AA1","AA2","AA3","AA4","AA5","AA6","AA7",
> +                                   
> "AB0","AB1","AB2","AB3","AB4","AB5","AB6","AB7",
> +                                   
> "AC0","AC1","AC2","AC3","AC4","AC5","AC6","AC7"
> +                             ;
>                         };

I have to say I don't find this terribly useful. It's much better if the platform DTS adds a name that captures the semantic meaning of the line rather than simply indicating its position in a bank. It's pretty straight forward to convert between the raw index and the "bank name" (e.g. GPIOV3). If it helps Joel wrote a little python script for it:

https://github.com/shenki/aspeed-kernel-tests/blob/master/num-to-gpio

Cheers,

Andrew

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH linux dev-4.18] ASPEED: dts: Define the gpio-line-names property for aspeed-g5
  2018-09-10  7:56   ` Wang, Kuiying
@ 2018-09-11  1:28     ` Andrew Jeffery
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Jeffery @ 2018-09-11  1:28 UTC (permalink / raw)
  To: Wang, Kuiying, Andrew Geissler, Joel Stanley
  Cc: Brad Bishop, Andrew Geissler, chunhui.jia, kunyi731, Mihm, James,
	Tanous, Ed, Feist, James, Jia, Chunhui, Patrick Venture,
	OpenBMC Maillist, Li, Yong B, Yang, Cheng C, Xu, Qiang, Nguyen,
	Hai V

On Mon, 10 Sep 2018, at 17:26, Wang, Kuiying wrote:
> Hi Andrew J,
> This is just common definition for aspeed-g5.
> There will be different configuration for specified platform just like 
> following:
> https://gerrit.openbmc-project.xyz/#/c/openbmc/meta-intel/+/12178/1/meta-intel/meta-s2600wf/recipes-phosphor/skeleton/obmc-libobmc-intf/gpio_defs.json 

Are you doing a string match on the pin name from the structures returned by ioctls on the gpiochip's chardev? This won't work if someone decides to specify the semantic name of the line in the devicetree like I linked to in my previous reply. Your best bet is to map the pin name from the JSON to the pin index on the gpiochip. Doing it any other way is fragile.

Andrew

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-09-11  1:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-10  7:27 [PATCH linux dev-4.18] ASPEED: dts: Define the gpio-line-names property for aspeed-g5 Wang, Kuiying
2018-09-10  7:36 ` Andrew Jeffery
2018-09-10  7:56   ` Wang, Kuiying
2018-09-11  1:28     ` Andrew Jeffery

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.