openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Adriana Kobylak <anoo@linux.ibm.com>
To: Milton Miller II <miltonm@us.ibm.com>
Cc: Matt Spinler <spinler@us.ibm.com>,
	Derek Howard <derekh@us.ibm.com>,
	linux-aspeed <linux-aspeed@lists.ozlabs.org>,
	Andrew Jeffery <andrew@aj.id.au>,
	Adriana Kobylak <anoo@us.ibm.com>,
	Brandon Wyman <bjwyman@gmail.com>,
	Shawn McCarney <shawnmm@us.ibm.com>,
	OpenBMC Maillist <openbmc@lists.ozlabs.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [EXTERNAL] [PATCH v2] ARM: dts: aspeed: rainier: Add N_MODE_VREF gpio
Date: Wed, 15 Sep 2021 14:10:08 -0500	[thread overview]
Message-ID: <26DB52A3-A380-475E-B335-D866DDD0150E@linux.ibm.com> (raw)
In-Reply-To: <OFC8E57D8E.A167EBF7-ON00258751.001786E9-00258751.001CE2C3@ibm.com>

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



> On Sep 15, 2021, at 12:15 AM, Milton Miller II <miltonm@us.ibm.com> wrote:
> 
> On September 14, 2021, Joel Stanley wrote:
>> On Tue, 14 Sept 2021 at 20:46, Adriana Kobylak <anoo@linux.ibm.com>
>> wrote:
>>>> On Sep 14, 2021, at 3:49 AM, Joel Stanley <joel@jms.id.au> wrote:
>>>> On Fri, 10 Sept 2021 at 19:54, Adriana Kobylak
>>>> <anoo@linux.ibm.com> wrote:
>>>>> 
>>>>> From: Adriana Kobylak <anoo@us.ibm.com>
>>>>> 
>>>>> 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.
>>>>> 
>>>>> Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
>>>>> ---
>>>>> 
>>>>> v2: Update commit message.
>>>>> 
>>>>> arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 103
>> +++++++++++++++++++
>>>>> 1 file changed, 103 insertions(+)
>>>>> 
>>>>> diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>> b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>>>>> index 6fd3ddf97a21..d5eea86dc260 100644
>>>>> --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>>>>> +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>>>>> @@ -1502,6 +1502,109 @@ eeprom@51 {
>>>>>               reg = <0x51>;
>>>>>       };
>>>>> 
>>>>> +       pca_pres3: pca9552@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",
>>>> 
>>>> 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= 
>>> 
>>> 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.

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.

> 
>> ...documented in the openbmc design
>> doc, such as SLOT6_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


[-- Attachment #2: Type: text/html, Size: 32293 bytes --]

      reply	other threads:[~2021-09-15 19:11 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-10 19:54 Adriana Kobylak
2021-09-14  8:49 ` Joel Stanley
2021-09-14 20:44   ` Adriana Kobylak
2021-09-15  2:21     ` Joel Stanley
2021-09-15  5:15     ` Milton Miller II
2021-09-15 19:10       ` Adriana Kobylak [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=26DB52A3-A380-475E-B335-D866DDD0150E@linux.ibm.com \
    --to=anoo@linux.ibm.com \
    --cc=andrew@aj.id.au \
    --cc=anoo@us.ibm.com \
    --cc=bjwyman@gmail.com \
    --cc=derekh@us.ibm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=miltonm@us.ibm.com \
    --cc=openbmc@lists.ozlabs.org \
    --cc=shawnmm@us.ibm.com \
    --cc=spinler@us.ibm.com \
    --subject='Re: [EXTERNAL] [PATCH v2] ARM: dts: aspeed: rainier: Add N_MODE_VREF gpio' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).