2020-03-06 17:03:28

by Andrew Geissler

[permalink] [raw]
Subject: [PATCH 1/2] ARM: dts: aspeed: romulus: Add gpio line names

Name the GPIOs to help userspace work with them. The names describe the
functionality the lines provide, not the net or ball name. This makes it
easier to share userspace code across different systems and makes the
use of the lines more obvious.

Signed-off-by: Andrew Geissler <[email protected]>
---
arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts | 35 ++++++++++++++++++--
1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
index edfa44fe1f75..fd2e014dae75 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
@@ -231,23 +231,52 @@
};

&gpio {
+ gpio-line-names =
+ /*A0-A7*/ "","cfam-reset","","","","","fsi-mux","",
+ /*B0-B7*/ "","","","","","","","",
+ /*C0-C7*/ "","","","","","","","",
+ /*D0-D7*/ "fsi-enable","","","nic_func_mode0","nic_func_mode1","","","",
+ /*E0-E7*/ "","","","","","","","",
+ /*F0-F7*/ "","","","","","","","",
+ /*G0-G7*/ "","","","","","","","",
+ /*H0-H7*/ "","","","","","","","",
+ /*I0-I7*/ "","","","power-button","","","","",
+ /*J0-J7*/ "","","checkstop","","","","","",
+ /*K0-K7*/ "","","","","","","","",
+ /*L0-L7*/ "","","","","","","","",
+ /*M0-M7*/ "","","","","","","","",
+ /*N0-N7*/ "","","led-fault","",
+ "led-identify","","","",
+ /*O0-O7*/ "","","","","","","","",
+ /*P0-P7*/ "","","","","","","","",
+ /*Q0-Q7*/ "","","","","","","","id-button",
+ /*R0-R7*/ "","","fsi-trans","","","led-power","","",
+ /*S0-S7*/ "","","","","","","","seq_cont",
+ /*T0-T7*/ "","","","","","","","",
+ /*U0-U7*/ "","","","","","","","",
+ /*V0-V7*/ "","","","","","","","",
+ /*W0-W7*/ "","","","","","","","",
+ /*X0-X7*/ "","","","","","","","",
+ /*Y0-Y7*/ "","","","","","","","",
+ /*Z0-Z7*/ "","","","","","","","",
+ /*AA0-AA7*/ "fsi-clock","","fsi-data","","","","","",
+ /*AB0-AB7*/ "","","","","","","","",
+ /*AC0-AC7*/ "","","","","","","","";
+
nic_func_mode0 {
gpio-hog;
gpios = <ASPEED_GPIO(D, 3) GPIO_ACTIVE_HIGH>;
output-low;
- line-name = "nic_func_mode0";
};
nic_func_mode1 {
gpio-hog;
gpios = <ASPEED_GPIO(D, 4) GPIO_ACTIVE_HIGH>;
output-low;
- line-name = "nic_func_mode1";
};
seq_cont {
gpio-hog;
gpios = <ASPEED_GPIO(S, 7) GPIO_ACTIVE_HIGH>;
output-low;
- line-name = "seq_cont";
};
};

--
2.21.0 (Apple Git-122)


2020-03-06 17:05:14

by Andrew Geissler

[permalink] [raw]
Subject: [PATCH 2/2] ARM: dts: aspeed: zaius: Add gpio line names

Name the GPIOs to help userspace work with them. The names describe the
functionality the lines provide, not the net or ball name. This makes it
easier to share userspace code across different systems and makes the
use of the lines more obvious.

Signed-off-by: Andrew Geissler <[email protected]>
---
arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 37 +++++++++++++++++++---
1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
index bc60ec291681..4bcc82046362 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
@@ -478,32 +478,61 @@
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_gpioh_unbiased>;

+ gpio-line-names =
+ /*A0-A7*/ "","cfam-reset","","","","","","",
+ /*B0-B7*/ "","","","","","","","",
+ /*C0-C7*/ "","","","","","","","",
+ /*D0-D7*/ "fsi-enable","","","","","led-sys-boot-status","led-attention",
+ "led-fault",
+ /*E0-E7*/ "","","","","","","","presence-pcie-e2b",
+ /*F0-F7*/ "","","","","","","","checkstop",
+ /*G0-G7*/ "fsi-clock","fsi-data","","","","","","",
+ /*H0-H7*/ "onewire0","onewire1","onewire2","onewire3","","","","",
+ /*I0-I7*/ "","","","power-button","","","","",
+ /*J0-J7*/ "","","","","","","","",
+ /*K0-K7*/ "","","","","","","","",
+ /*L0-L7*/ "","","","","","","","",
+ /*M0-M7*/ "","","","","","","","",
+ /*N0-N7*/ "","","","","","","","",
+ /*O0-O7*/ "","","","","iso_u164_en","","fsi-trans","",
+ /*P0-P7*/ "ncsi_mux_en_n","bmc_i2c2_sw_rst_n","","bmc_i2c5_sw_rst_n","",
+ "","fsi-mux","",
+ /*Q0-Q7*/ "","","","","","","","",
+ /*R0-R7*/ "","","","","","","","",
+ /*S0-S7*/ "","","","","","","","",
+ /*T0-T7*/ "","","","","","","","",
+ /*U0-U7*/ "","","","","","","","",
+ /*V0-V7*/ "","","","","","","","",
+ /*W0-W7*/ "","","","","","","","",
+ /*X0-X7*/ "","","","","","","","",
+ /*Y0-Y7*/ "","","","","","","","",
+ /*Z0-Z7*/ "","","","","","","","",
+ /*AA0-AA7*/ "","","led-hdd-fault","","","","","",
+ /*AB0-AB7*/ "","","","","","","","",
+ /*AC0-AC7*/ "","","","","","","","";
+
line_iso_u146_en {
gpio-hog;
gpios = <ASPEED_GPIO(O, 4) GPIO_ACTIVE_HIGH>;
output-high;
- line-name = "iso_u164_en";
};

ncsi_mux_en_n {
gpio-hog;
gpios = <ASPEED_GPIO(P, 0) GPIO_ACTIVE_HIGH>;
output-low;
- line-name = "ncsi_mux_en_n";
};

line_bmc_i2c2_sw_rst_n {
gpio-hog;
gpios = <ASPEED_GPIO(P, 1) GPIO_ACTIVE_HIGH>;
output-high;
- line-name = "bmc_i2c2_sw_rst_n";
};

line_bmc_i2c5_sw_rst_n {
gpio-hog;
gpios = <ASPEED_GPIO(P, 3) GPIO_ACTIVE_HIGH>;
output-high;
- line-name = "bmc_i2c5_sw_rst_n";
};
};

--
2.21.0 (Apple Git-122)

2020-03-26 23:21:46

by Andrew Jeffery

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: aspeed: zaius: Add gpio line names



On Sat, 7 Mar 2020, at 03:32, Andrew Geissler wrote:
> Name the GPIOs to help userspace work with them. The names describe the
> functionality the lines provide, not the net or ball name. This makes it
> easier to share userspace code across different systems and makes the
> use of the lines more obvious.
>
> Signed-off-by: Andrew Geissler <[email protected]>

So we're creating a bit of an ad-hoc ABI here between the DT and userspace.

Where are we documenting it?

Generally I think the idea is good though, so:

Acked-by: Andrew Jeffery <[email protected]>

> ---
> arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 37 +++++++++++++++++++---
> 1 file changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> index bc60ec291681..4bcc82046362 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> @@ -478,32 +478,61 @@
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_gpioh_unbiased>;
>
> + gpio-line-names =
> + /*A0-A7*/ "","cfam-reset","","","","","","",
> + /*B0-B7*/ "","","","","","","","",
> + /*C0-C7*/ "","","","","","","","",
> + /*D0-D7*/ "fsi-enable","","","","","led-sys-boot-status","led-attention",
> + "led-fault",
> + /*E0-E7*/ "","","","","","","","presence-pcie-e2b",
> + /*F0-F7*/ "","","","","","","","checkstop",
> + /*G0-G7*/ "fsi-clock","fsi-data","","","","","","",
> + /*H0-H7*/ "onewire0","onewire1","onewire2","onewire3","","","","",
> + /*I0-I7*/ "","","","power-button","","","","",
> + /*J0-J7*/ "","","","","","","","",
> + /*K0-K7*/ "","","","","","","","",
> + /*L0-L7*/ "","","","","","","","",
> + /*M0-M7*/ "","","","","","","","",
> + /*N0-N7*/ "","","","","","","","",
> + /*O0-O7*/ "","","","","iso_u164_en","","fsi-trans","",
> + /*P0-P7*/ "ncsi_mux_en_n","bmc_i2c2_sw_rst_n","","bmc_i2c5_sw_rst_n","",
> + "","fsi-mux","",
> + /*Q0-Q7*/ "","","","","","","","",
> + /*R0-R7*/ "","","","","","","","",
> + /*S0-S7*/ "","","","","","","","",
> + /*T0-T7*/ "","","","","","","","",
> + /*U0-U7*/ "","","","","","","","",
> + /*V0-V7*/ "","","","","","","","",
> + /*W0-W7*/ "","","","","","","","",
> + /*X0-X7*/ "","","","","","","","",
> + /*Y0-Y7*/ "","","","","","","","",
> + /*Z0-Z7*/ "","","","","","","","",
> + /*AA0-AA7*/ "","","led-hdd-fault","","","","","",
> + /*AB0-AB7*/ "","","","","","","","",
> + /*AC0-AC7*/ "","","","","","","","";
> +
> line_iso_u146_en {
> gpio-hog;
> gpios = <ASPEED_GPIO(O, 4) GPIO_ACTIVE_HIGH>;
> output-high;
> - line-name = "iso_u164_en";
> };
>
> ncsi_mux_en_n {
> gpio-hog;
> gpios = <ASPEED_GPIO(P, 0) GPIO_ACTIVE_HIGH>;
> output-low;
> - line-name = "ncsi_mux_en_n";
> };
>
> line_bmc_i2c2_sw_rst_n {
> gpio-hog;
> gpios = <ASPEED_GPIO(P, 1) GPIO_ACTIVE_HIGH>;
> output-high;
> - line-name = "bmc_i2c2_sw_rst_n";
> };
>
> line_bmc_i2c5_sw_rst_n {
> gpio-hog;
> gpios = <ASPEED_GPIO(P, 3) GPIO_ACTIVE_HIGH>;
> output-high;
> - line-name = "bmc_i2c5_sw_rst_n";
> };
> };
>
> --
> 2.21.0 (Apple Git-122)
>
>

2020-03-30 18:18:11

by Andrew Geissler

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: aspeed: zaius: Add gpio line names



> On Mar 26, 2020, at 6:20 PM, Andrew Jeffery <[email protected]> wrote:
>
>
>
> On Sat, 7 Mar 2020, at 03:32, Andrew Geissler wrote:
>> Name the GPIOs to help userspace work with them. The names describe the
>> functionality the lines provide, not the net or ball name. This makes it
>> easier to share userspace code across different systems and makes the
>> use of the lines more obvious.
>>
>> Signed-off-by: Andrew Geissler <[email protected]>
>
> So we're creating a bit of an ad-hoc ABI here between the DT and userspace.
>
> Where are we documenting it?

Yeah, so far it’s basically design by precedent. If you want your OpenBMC
function to work then follow the standards we're setting in other dts’s.

Is there a good place to document this? I could create a OpenBMC design
doc but that would not address non-OpenBMC areas.

>
> Generally I think the idea is good though, so:
>
> Acked-by: Andrew Jeffery <[email protected]>

Thanks

>
>> ---
>> arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 37 +++++++++++++++++++---
>> 1 file changed, 33 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
>> b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
>> index bc60ec291681..4bcc82046362 100644
>> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
>> @@ -478,32 +478,61 @@
>> pinctrl-names = "default";
>> pinctrl-0 = <&pinctrl_gpioh_unbiased>;
>>
>> + gpio-line-names =
>> + /*A0-A7*/ "","cfam-reset","","","","","","",
>> + /*B0-B7*/ "","","","","","","","",
>> + /*C0-C7*/ "","","","","","","","",
>> + /*D0-D7*/ "fsi-enable","","","","","led-sys-boot-status","led-attention",
>> + "led-fault",
>> + /*E0-E7*/ "","","","","","","","presence-pcie-e2b",
>> + /*F0-F7*/ "","","","","","","","checkstop",
>> + /*G0-G7*/ "fsi-clock","fsi-data","","","","","","",
>> + /*H0-H7*/ "onewire0","onewire1","onewire2","onewire3","","","","",
>> + /*I0-I7*/ "","","","power-button","","","","",
>> + /*J0-J7*/ "","","","","","","","",
>> + /*K0-K7*/ "","","","","","","","",
>> + /*L0-L7*/ "","","","","","","","",
>> + /*M0-M7*/ "","","","","","","","",
>> + /*N0-N7*/ "","","","","","","","",
>> + /*O0-O7*/ "","","","","iso_u164_en","","fsi-trans","",
>> + /*P0-P7*/ "ncsi_mux_en_n","bmc_i2c2_sw_rst_n","","bmc_i2c5_sw_rst_n","",
>> + "","fsi-mux","",
>> + /*Q0-Q7*/ "","","","","","","","",
>> + /*R0-R7*/ "","","","","","","","",
>> + /*S0-S7*/ "","","","","","","","",
>> + /*T0-T7*/ "","","","","","","","",
>> + /*U0-U7*/ "","","","","","","","",
>> + /*V0-V7*/ "","","","","","","","",
>> + /*W0-W7*/ "","","","","","","","",
>> + /*X0-X7*/ "","","","","","","","",
>> + /*Y0-Y7*/ "","","","","","","","",
>> + /*Z0-Z7*/ "","","","","","","","",
>> + /*AA0-AA7*/ "","","led-hdd-fault","","","","","",
>> + /*AB0-AB7*/ "","","","","","","","",
>> + /*AC0-AC7*/ "","","","","","","","";
>> +
>> line_iso_u146_en {
>> gpio-hog;
>> gpios = <ASPEED_GPIO(O, 4) GPIO_ACTIVE_HIGH>;
>> output-high;
>> - line-name = "iso_u164_en";
>> };
>>
>> ncsi_mux_en_n {
>> gpio-hog;
>> gpios = <ASPEED_GPIO(P, 0) GPIO_ACTIVE_HIGH>;
>> output-low;
>> - line-name = "ncsi_mux_en_n";
>> };
>>
>> line_bmc_i2c2_sw_rst_n {
>> gpio-hog;
>> gpios = <ASPEED_GPIO(P, 1) GPIO_ACTIVE_HIGH>;
>> output-high;
>> - line-name = "bmc_i2c2_sw_rst_n";
>> };
>>
>> line_bmc_i2c5_sw_rst_n {
>> gpio-hog;
>> gpios = <ASPEED_GPIO(P, 3) GPIO_ACTIVE_HIGH>;
>> output-high;
>> - line-name = "bmc_i2c5_sw_rst_n";
>> };
>> };
>>
>> --
>> 2.21.0 (Apple Git-122)
>>
>>

2020-04-03 00:53:24

by Andrew Jeffery

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: aspeed: zaius: Add gpio line names



On Tue, 31 Mar 2020, at 04:46, Andrew Geissler wrote:
>
>
> > On Mar 26, 2020, at 6:20 PM, Andrew Jeffery <[email protected]> wrote:
> >
> >
> >
> > On Sat, 7 Mar 2020, at 03:32, Andrew Geissler wrote:
> >> Name the GPIOs to help userspace work with them. The names describe the
> >> functionality the lines provide, not the net or ball name. This makes it
> >> easier to share userspace code across different systems and makes the
> >> use of the lines more obvious.
> >>
> >> Signed-off-by: Andrew Geissler <[email protected]>
> >
> > So we're creating a bit of an ad-hoc ABI here between the DT and userspace.
> >
> > Where are we documenting it?
>
> Yeah, so far it’s basically design by precedent. If you want your OpenBMC
> function to work then follow the standards we're setting in other dts’s.
>
> Is there a good place to document this? I could create a OpenBMC design
> doc but that would not address non-OpenBMC areas.

Don't let perfect be the enemy of good enough :) Lets document it in OpenBMC
and then look at alternatives if we find it's necessary. I don't think we will given
that the contract is between the kernel and OpenBMC userspace.

Andrew

2020-04-03 16:52:00

by Andrew Geissler

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: aspeed: zaius: Add gpio line names



> On Apr 2, 2020, at 7:51 PM, Andrew Jeffery <[email protected]> wrote:
>
>
>
> On Tue, 31 Mar 2020, at 04:46, Andrew Geissler wrote:
>>
>>
>>> On Mar 26, 2020, at 6:20 PM, Andrew Jeffery <[email protected]> wrote:
>>>
>>>
>>>
>>> On Sat, 7 Mar 2020, at 03:32, Andrew Geissler wrote:
>>>> Name the GPIOs to help userspace work with them. The names describe the
>>>> functionality the lines provide, not the net or ball name. This makes it
>>>> easier to share userspace code across different systems and makes the
>>>> use of the lines more obvious.
>>>>
>>>> Signed-off-by: Andrew Geissler <[email protected]>
>>>
>>> So we're creating a bit of an ad-hoc ABI here between the DT and userspace.
>>>
>>> Where are we documenting it?
>>
>> Yeah, so far it’s basically design by precedent. If you want your OpenBMC
>> function to work then follow the standards we're setting in other dts’s.
>>
>> Is there a good place to document this? I could create a OpenBMC design
>> doc but that would not address non-OpenBMC areas.
>
> Don't let perfect be the enemy of good enough :) Lets document it in OpenBMC
> and then look at alternatives if we find it's necessary. I don't think we will given
> that the contract is between the kernel and OpenBMC userspace.

Ok, I put a doc up for review here:
https://gerrit.openbmc-project.xyz/c/openbmc/docs/+/30988

>
> Andrew