ARM: dts: aspeed: rainier: Add N_MODE_VREF gpio
Subsystems (2)
PATCH v2 1/1 ARM: dts: aspeed: rainier: Add N_MODE_VREF gpio
From: Adriana Kobylak <[email protected]>
The N_MODE_VREF gpio is designed to be used to specify how many power supplies the system should have (2 or 4). If enough power supplies fail so that the system no longer has redundancy (no longer n+1), the hardware will signal to the Onboard Chip Controller that the system may be oversubscribed, and performance may need to be reduced so the system can maintain it's powered on state. This gpio is on a 9552, populate all the gpios on that chip for completeness.
---
v2: Update commit message.
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 103 +++++++++++++++++++ 1 file changed, 103 insertions(+)
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +103 -0
@@ -1502,6 +1502,109 @@ eeprom at 51 { reg = <0x51>; }; + pca_pres3: pca9552 at 60 { + compatible = "nxp,pca9552"; + reg = <0x60>; + #address-cells = <1>; + #size-cells = <0>; + gpio-controller; + #gpio-cells = <2>; + + gpio-line-names = + "", + "APSS_RESET_N", + "", "", "", "", + "P10_DCM0_PRES", + "P10_DCM1_PRES", + "", "", + "N_MODE_CPU_N", + "", + "PRESENT_VRM_DCM0_N", + "PRESENT_VRM_DCM1_N", + "N_MODE_VREF", + ""; + + gpio at 0 { + reg = <0>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 1 { + reg = <1>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 2 { + reg = <2>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 3 { + reg = <3>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 4 { + reg = <4>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 5 { + reg = <5>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 6 { + reg = <6>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 7 { + reg = <7>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 8 { + reg = <8>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 9 { + reg = <9>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 10 { + reg = <10>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 11 { + reg = <11>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 12 { + reg = <12>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 13 { + reg = <13>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 14 { + reg = <14>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 15 { + reg = <15>; + type = <PCA955X_TYPE_GPIO>; + }; + }; + pca_pres2: pca9552 at 61 { compatible = "nxp,pca9552"; reg = <0x61>;
Signature · 1 line
2.25.1
Joel Stanley
maintainerQuoted 37 lines · Adriana Kobylak
From: Adriana Kobylak <[email protected]>
The
N_MODE_VREFgpio is designed to be used to specify how many power supplies the system should have (2 or 4). If enough power supplies fail so that the system no longer has redundancy (no longer n+1), the hardware will signal to the Onboard Chip Controller that the system may be oversubscribed, and performance may need to be reduced so the system can maintain it's powered on state. This gpio is on a 9552, populate all the gpios on that chip for completeness.Signed-off-by Adriana Kobylak <[email protected]>---
v2: Update commit message.
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 103 +++++++++++++++++++ 1 file changed, 103 insertions(+)arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +10
-0@@ -1502,6 +1502,109 @@ eeprom at 51 { reg = <0x51>; }; + pca_pres3: pca9552 at 60 { + compatible = "nxp,pca9552"; + reg = <0x60>; + #address-cells = <1>; + #size-cells = <0>; + gpio-controller; + #gpio-cells = <2>; + + gpio-line-names = + "",
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts eeprom at 51 { · line 1524
Quoted 10 lines
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +10
-0+ "APSS_RESET_N", + "", "", "", "", + "P10_DCM0_PRES", + "P10_DCM1_PRES", + "", "", + "N_MODE_CPU_N", + "", + "PRESENT_VRM_DCM0_N", + "PRESENT_VRM_DCM1_N", + "N_MODE_VREF",
Should any (all?) of these names be documented?
https://github.com/openbmc/docs/blob/master/designs/device-tree-gpio-naming.md
Quoted 89 lines
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +83
-1+ ""; + + gpio at 0 { + reg = <0>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 1 { + reg = <1>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 2 { + reg = <2>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 3 { + reg = <3>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 4 { + reg = <4>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 5 { + reg = <5>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 6 { + reg = <6>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 7 { + reg = <7>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 8 { + reg = <8>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 9 { + reg = <9>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 10 { + reg = <10>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 11 { + reg = <11>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 12 { + reg = <12>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 13 { + reg = <13>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 14 { + reg = <14>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 15 { + reg = <15>; + type = <PCA955X_TYPE_GPIO>; + }; + }; + pca_pres2: pca9552 at 61 { compatible = "nxp,pca9552"; reg = <0x61>; --2.25.1
Quoted 55 lines · Joel Stanley
Quoted 37 lines · Adriana Kobylak
From: Adriana Kobylak <[email protected]>
The
N_MODE_VREFgpio is designed to be used to specify how many power supplies the system should have (2 or 4). If enough power supplies fail so that the system no longer has redundancy (no longer n+1), the hardware will signal to the Onboard Chip Controller that the system may be oversubscribed, and performance may need to be reduced so the system can maintain it's powered on state. This gpio is on a 9552, populate all the gpios on that chip for completeness.Signed-off-by Adriana Kobylak <[email protected]>---
v2: Update commit message.
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts| 103 +++++++++++++++++++1 file changed, 103 insertions(+)arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +10
-0@@ -1502,6 +1502,109 @@ eeprom at 51 { reg = <0x51>; }; + pca_pres3: pca9552 at 60 { + compatible = "nxp,pca9552"; + reg = <0x60>; + #address-cells = <1>; + #size-cells = <0>; + gpio-controller; + #gpio-cells = <2>; + + gpio-line-names = + "",arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts eeprom at 51 { · line 1524
Quoted 10 lines
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +10
-0+ "APSS_RESET_N", + "", "", "", "", + "P10_DCM0_PRES", + "P10_DCM1_PRES", + "", "", + "N_MODE_CPU_N", + "", + "PRESENT_VRM_DCM0_N", + "PRESENT_VRM_DCM1_N", + "N_MODE_VREF",Should any (all?) of these names be documented?
Not sure. Seems the openbmc doc is documenting the gpios for gpiochip0 only? The gpio names for 9552 in this patch come from the System Workbook, and doesn?t seem the gpios from the existing 9552 that also come from the System Workbook are documented in the openbmc design doc, such as SLOT6_PRSNT_EN_RSVD, SLOT11_EXPANDER_PRSNT_N, etc.
Quoted 91 lines
Quoted 91 lines
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +83
-1+ ""; + + gpio at 0 { + reg = <0>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 1 { + reg = <1>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 2 { + reg = <2>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 3 { + reg = <3>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 4 { + reg = <4>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 5 { + reg = <5>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 6 { + reg = <6>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 7 { + reg = <7>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 8 { + reg = <8>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 9 { + reg = <9>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 10 { + reg = <10>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 11 { + reg = <11>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 12 { + reg = <12>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 13 { + reg = <13>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 14 { + reg = <14>; + type = <PCA955X_TYPE_GPIO>; + }; + + gpio at 15 { + reg = <15>; + type = <PCA955X_TYPE_GPIO>; + }; + }; + pca_pres2: pca9552 at 61 { compatible = "nxp,pca9552"; reg = <0x61>; --2.25.1
Joel Stanley
maintainerQuoted 56 lines · Adriana Kobylak
Quoted 56 lines · Joel Stanley
Quoted 37 lines · Adriana Kobylak
From: Adriana Kobylak <[email protected]>
The
N_MODE_VREFgpio is designed to be used to specify how many power supplies the system should have (2 or 4). If enough power supplies fail so that the system no longer has redundancy (no longer n+1), the hardware will signal to the Onboard Chip Controller that the system may be oversubscribed, and performance may need to be reduced so the system can maintain it's powered on state. This gpio is on a 9552, populate all the gpios on that chip for completeness.Signed-off-by Adriana Kobylak <[email protected]>---
v2: Update commit message.
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts| 103 +++++++++++++++++++1 file changed, 103 insertions(+)arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +10
-0@@ -1502,6 +1502,109 @@ eeprom at 51 { reg = <0x51>; }; + pca_pres3: pca9552 at 60 { + compatible = "nxp,pca9552"; + reg = <0x60>; + #address-cells = <1>; + #size-cells = <0>; + gpio-controller; + #gpio-cells = <2>; + + gpio-line-names = + "",arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts eeprom at 51 { · line 1524
Quoted 10 lines
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +10
-0+ "APSS_RESET_N", + "", "", "", "", + "P10_DCM0_PRES", + "P10_DCM1_PRES", + "", "", + "N_MODE_CPU_N", + "", + "PRESENT_VRM_DCM0_N", + "PRESENT_VRM_DCM1_N", + "N_MODE_VREF",Should any (all?) of these names be documented?
Quoted 3 lines
Not sure. Seems the openbmc doc is documenting the gpios for gpiochip0 only?
AIUI the document is for GPIOs that are exposed to userspace.
It doesn't matter where they're coming from. If they are going to be used by a libgpio application, they need to have names, and the names should be documented where possible.
Quoted 1 line
The gpio names for 9552 in this patch come from the System Workbook, and doesn?t seem the gpios from the existing 9552 that also come from the System Workbook are documented in the openbmc design doc, such as
SLOT6_PRSNT_EN_RSVD,SLOT11_EXPANDER_PRSNT_N, etc.
They should be fixed, if possible.
Cheers,
Joel
Quoted 69 lines · Joel Stanley
Quoted 19 lines · Adriana Kobylak
Quoted 19 lines · Joel Stanley
Quoted 16 lines · Adriana Kobylak
From: Adriana Kobylak <[email protected]>
The
N_MODE_VREFgpio is designed to be used to specify how many power
supplies the system should have (2 or 4). If enough power supplies fail
so that the system no longer has redundancy (no longer n+1), the hardware will signal to the Onboard Chip Controller that the system may
be oversubscribed, and performance may need to be reduced so the system
can maintain it's powered on state. This gpio is on a 9552, populate all
the gpios on that chip for completeness.Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Signed-off-by Adriana Kobylak <[email protected]>---
v2: Update commit message.
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts| 103+++++++++++++++++++
Quoted 3 lines
Quoted 3 lines
Review
Quoted 3 lines
1 file changed, 103 insertions(+)(quoted diff) +0
-0
b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dtsQuoted 23 lines
Quoted 23 lines
Quoted 23 lines
index
6fd3ddf97a21..d5eea86dc260100644arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +16
-0@@ -1502,6 +1502,109 @@ eeprom at 51 { reg = <0x51>; }; + pca_pres3: pca9552 at 60 { + compatible = "nxp,pca9552"; + reg = <0x60>; + #address-cells = <1>; + #size-cells = <0>; + gpio-controller; + #gpio-cells = <2>; + + gpio-line-names = + "", + "APSS_RESET_N", + "", "", "", "", + "P10_DCM0_PRES", + "P10_DCM1_PRES", + "", "", + "N_MODE_CPU_N",Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts eeprom at 51 { · line 1524
Quoted 4 lines
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +4
-0+ "", + "PRESENT_VRM_DCM0_N", + "PRESENT_VRM_DCM1_N", + "N_MODE_VREF",Should any (all?) of these names be documented?
INVALID URI REMOVED mc_docs_blob_master_designs_device-2Dtree-2Dgpio-2Dnaming.md&d=DwIFaQ &c=jf_iaSHvJObTbx-siA1ZOg&r=bvv7AJEECoRKBU02rcu4F5DWd-EwX8As2xrXeO9ZS o4&m=JzmffOJA0hX_vgi3n0P-A6l60imZToV7q1U2W2h6xt4&s=14_ACuQWMp-IFlhLQa ejLVBN8XVgDnn1_l6336-FBG8&e=
Quoted 1 line
Quoted 9 lines
Quoted 2 lines
Not sure. Seems the openbmc doc is documenting the gpios for gpiochip0 only?
AIUI the document is for GPIOs that are exposed to userspace.
It doesn't matter where they're coming from. If they are going to be used by a libgpio application, they need to have names, and the names should be documented where possible.
I agree which gpiochip is just a board wiring consideration and has no bearing on the documentation.
However, in the introductory sections in the document clearly says the purpose is to establish naming for common (function) GPIOs, and the justification is by using consistent names across machines code will be able to be reused with little to no configuration. In addition it mentions "common" GPIOs must be added to the document in the future. So an evaluation should be made to the likelihood that such code reuse can be anticipated.
Most of the names added in this patch are presence detect signals used
to cross check VPD is read into inventory. I'd expect any such uses to
be configured in an inventory config file listing the name and where the FRU appears in the Dbus or Redfish model. I'd argue the names for any such gpio would be beyond the present document scope.
The one mentioned in the changelog, N_MODE_VREF, is intended to be relayed to the OCC, basically a power management controller in common in POWER processor chips. I can see an argument to list this, but feel it would be in the OpenPOWER specific section unless the activation method is exposed in some method that would be common to other chipsets.
Quoted 4 lines
...documented in the openbmc design
doc, such asSLOT6_PRSNT_EN_RSVD,SLOT11_EXPANDER_PRSNT_N, etc.They should be fixed, if possible.
The scope is clearly use reusable names going forward. The technical debt from past naming can be brought down as new uses are added but we are not renaming every GPIO in every existing platform, and we don't have the review bandwidth to agree on common names should be added for all existing signals.
milton
PATCH v2 1/1 ARM: dts: aspeed: rainier: Add N_MODE_VREF gpio
Quoted 101 lines · Milton Miller II
Quoted 69 lines · Joel Stanley
Quoted 19 lines · Adriana Kobylak
Quoted 19 lines · Joel Stanley
Quoted 16 lines · Adriana Kobylak
From: Adriana Kobylak <[email protected]>
The
N_MODE_VREFgpio is designed to be used to specify how many power
supplies the system should have (2 or 4). If enough power supplies fail
so that the system no longer has redundancy (no longer n+1), the hardware will signal to the Onboard Chip Controller that the system may
be oversubscribed, and performance may need to be reduced so the system
can maintain it's powered on state. This gpio is on a 9552, populate all
the gpios on that chip for completeness.Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Signed-off-by Adriana Kobylak <[email protected]>---
v2: Update commit message.
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts| 103+++++++++++++++++++
Quoted 3 lines
Quoted 3 lines
Review
Quoted 3 lines
1 file changed, 103 insertions(+)(quoted diff) +0
-0
b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dtsQuoted 23 lines
Quoted 23 lines
Quoted 23 lines
index
6fd3ddf97a21..d5eea86dc260100644arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +16
-0@@ -1502,6 +1502,109 @@ eeprom at 51 { reg = <0x51>; }; + pca_pres3: pca9552 at 60 { + compatible = "nxp,pca9552"; + reg = <0x60>; + #address-cells = <1>; + #size-cells = <0>; + gpio-controller; + #gpio-cells = <2>; + + gpio-line-names = + "", + "APSS_RESET_N", + "", "", "", "", + "P10_DCM0_PRES", + "P10_DCM1_PRES", + "", "", + "N_MODE_CPU_N",Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts eeprom at 51 { · line 1524
Quoted 4 lines
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +4
-0+ "", + "PRESENT_VRM_DCM0_N", + "PRESENT_VRM_DCM1_N", + "N_MODE_VREF",Should any (all?) of these names be documented?
INVALID URI REMOVED <INVALID URI REMOVED> mc_docs_blob_master_designs_device-2Dtree-2Dgpio-2Dnaming.md&d=DwIFaQ &c=jf_iaSHvJObTbx-siA1ZOg&r=bvv7AJEECoRKBU02rcu4F5DWd-EwX8As2xrXeO9ZS o4&m=JzmffOJA0hX_vgi3n0P-A6l60imZToV7q1U2W2h6xt4&s=14_ACuQWMp-IFlhLQa ejLVBN8XVgDnn1_l6336-FBG8&e=
Quoted 1 line
Quoted 9 lines
Quoted 2 lines
Not sure. Seems the openbmc doc is documenting the gpios for gpiochip0 only?
AIUI the document is for GPIOs that are exposed to userspace.
It doesn't matter where they're coming from. If they are going to be used by a libgpio application, they need to have names, and the names should be documented where possible.
I agree which gpiochip is just a board wiring consideration and has no bearing on the documentation.
However, in the introductory sections in the document clearly says the purpose is to establish naming for common (function) GPIOs, and the justification is by using consistent names across machines code will be able to be reused with little to no configuration. In addition it mentions "common" GPIOs must be added to the document in the future. So an evaluation should be made to the likelihood that such code reuse can be anticipated.
Most of the names added in this patch are presence detect signals used
to cross check VPD is read into inventory. I'd expect any such uses to
be configured in an inventory config file listing the name and where the FRU appears in the Dbus or Redfish model. I'd argue the names for any such gpio would be beyond the present document scope.
Quoted 6 lines
The one mentioned in the changelog,
N_MODE_VREF, is intended to be relayed to the OCC, basically a power management controller in common in POWER processor chips. I can see an argument to list this, but feel it would be in the OpenPOWER specific section unless the activation method is exposed in some method that would be common to other chipsets.
Thanks. I submitted a proposal to define N_MODE_VREF: https://gerrit.openbmc-project.xyz/c/openbmc/docs/+/46914 <https://gerrit.openbmc-project.xyz/c/openbmc/docs/+/46914>
Once the name is approved I?ll add just that one to the device tree, since the others are not going to be used.
Quoted 6 lines
Quoted 5 lines
...documented in the openbmc design
doc, such asSLOT6_PRSNT_EN_RSVD,SLOT11_EXPANDER_PRSNT_N, etc.They should be fixed, if possible.
Quoted 7 lines
The scope is clearly use reusable names going forward. The technical debt from past naming can be brought down as new uses are added but we are not renaming every GPIO in every existing platform, and we don't have the review bandwidth to agree on common names should be added for all existing signals.
milton
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20210915/9b79deb9/attachment-0001.htm>