Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <161db1b2-f52f-2bbb-7121-a4097df8cf48@amperemail.onmicrosoft.com>
Date:   Tue, 19 Oct 2021 17:37:34 +0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.2.0
Subject: Re: [PATCH v2 1/3] ARM: dts: aspeed: mtjade: Add some gpios
Content-Language: en-US
To:     Joel Stanley <joel@jms.id.au>,
        Quan Nguyen <quan@os.amperecomputing.com>
Cc:     Rob Herring <robh+dt@kernel.org>, Andrew Jeffery <andrew@aj.id.au>,
        devicetree <devicetree@vger.kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        linux-aspeed <linux-aspeed@lists.ozlabs.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        OpenBMC Maillist <openbmc@lists.ozlabs.org>,
        Open Source Submission <patches@amperecomputing.com>,
        Phong Vo <phong@os.amperecomputing.com>,
        "Thang Q . Nguyen" <thang@os.amperecomputing.com>
References: <20211019060155.945-1-quan@os.amperecomputing.com>
 <20211019060155.945-2-quan@os.amperecomputing.com>
 <CACPK8Xcp0ruL-7p3AA+yvba3Drrwm-=-hMnMpd=a1aHwQHnE1A@mail.gmail.com>
From:   Thang Nguyen <thang@amperemail.onmicrosoft.com>
In-Reply-To: <CACPK8Xcp0ruL-7p3AA+yvba3Drrwm-=-hMnMpd=a1aHwQHnE1A@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZGxRM2Q0YnFrSFdIYldkdUtJRWNnaEdqcC82WW5hYjFxaUl4ZzdtZ253S3Ja?=
 =?utf-8?B?OUF0bEVQU0puREFLdzN1eFRRNFlBYnFnUGIvcGZnN0h6cUpaS0RQRS8rSWxC?=
 =?utf-8?B?WUljZ3B3Z3l1TzBnTWtRNW9lSnhvZHUyWGlnZEtQSXBpdDhWK1VDQ2tmMlM3?=
 =?utf-8?B?blBJam8zZlJuV3h4VHBiVllPRnhDZ1poYS9GOXg0RFlUeFpDaGpwSVNTRk9S?=
 =?utf-8?B?NUV4RWtlQzN2UnAvNGpQOU80UmpSM3BGaVJlYmRONTc5RnRHbmIwcWRMSUE5?=
 =?utf-8?B?T1orWHhKdkRrQTZiM1BUUXNQRzFtYUtPaUx3bjQ0eFJHTWVMOXRiMlFDQnNM?=
 =?utf-8?B?S0JHd2RSZFRtRkFXQ1NRNzQ5cVNwZEV2K2FUSlJRTStOQjdRT2FkSStzSnJk?=
 =?utf-8?B?OTRhaWpNQzFneVpnMjZ4REZpK2kwZWRvdW9URDQzcDFlNnRxNTBST05nVWpP?=
 =?utf-8?B?bktNYk5iS1JnaEFreVVyc20wcUNWV3pmRXAzWUVjTnIwdmpLNGdPcEdjUDAw?=
 =?utf-8?B?V05yRFU2VWhqQU9ZUjFEZUJJUG9pS1ZxTVQyOHlHcTNFc3NZaHI5aDR1R1J4?=
 =?utf-8?B?S1BLR1B5V0c3MVVXYmxZMkpDcjdXYlBiS21sZGV4UDRNV28vZzVrclZFeFg4?=
 =?utf-8?B?aU85bzM4MmtsREczdTZHbDlSRTBtZWs3WkdiVGxWUzZZNzFjcGpHSlh3bU1M?=
 =?utf-8?B?bHpFajdQV3o1VUxWdnBaN09NZ3dPUnJCMTdWUjlneDRpTkNNcmV4YjFmeFI0?=
 =?utf-8?B?MWZXenRJbzdaRWVteTk5Ylh5LzNvaXdRWW1TWHBoSU9nWTVJZXlhM2Jhb0Fu?=
 =?utf-8?B?T2p3YytVMVdOU1V5N25DM1MvOWxhVzZJWU1Sbk1IK1A0U3YxZXcwREVIWER1?=
 =?utf-8?B?ZzgrVytSRDg2Wjg2ZGFTSHlmdy9lUlhDcVdhUEN0K2lWS3BXZ1VLSmZ4Mm92?=
 =?utf-8?B?UXdQb0h6cUZmbDMyU0dmRkxtQWVrUTlpZ0ZJMy9VMmVtTnMyakphVkN1SjRC?=
 =?utf-8?B?UlVCdFJEcmIrTCt3dEhjQ2ZvZjdVdGtuWUZJd2hPc1dGMC9TWVNwRnhaTlgr?=
 =?utf-8?B?Q1gwbkVkN1Z4RDdpdXhMV05vSUpRME9GakxZUDJEUzBadURjM3pVTlJaNlJZ?=
 =?utf-8?B?bDA4N0dKZFVIdlc0cHZleTRRcm02SjlYbTVvS2t4N1FYUm5jUmo2Z3VVczBU?=
 =?utf-8?B?bVNHTmllY3hac2prV0VieHNZalFmbG1xRDVyYXNNRDJXUmtjL0Z6VDB6eG9K?=
 =?utf-8?B?WFVOakljbkx5QVV5dm9raXhkMDkzb2ZVWWtWcDdGS0s4NWtLQWlGMTZsUG1y?=
 =?utf-8?B?Qml6N3lYemtNeFA3UUlpdmkxSFphNnFSU3RQWGptd1hHS1pUb2R3T0w4UHhJ?=
 =?utf-8?B?YVdtU3hUV0IxNjR5L09ZRGFLVmdURnkyTllzL2VWT082ems5T3VXOEx2TmtR?=
 =?utf-8?B?RE94TUM3WmlOU3VTSyt0Ulh6MEFyUnRBZjVPckEyMEtheHNKT0NYeDhBMW94?=
 =?utf-8?B?cndSUVJqTnA0WVNHOUp3cEVFbmYyOUZTazAwdjJuVjlGeEVmTUJiNlFLdnZk?=
 =?utf-8?B?MlJPdy9Rak9ZNEFMLy9UT3VFWVd3UmVKNFp6alNzbG1XZG1yY25EYlZ2VndW?=
 =?utf-8?B?U1ZFNUp5emJIRTY3dUxqTmVrd3FGNkdLS0R3MnJWeE9OQ2JsaTIrc0lRNFla?=
 =?utf-8?B?SGo5V2lqUzNYeXNVc0hHZ25GK0lZRlZ5cTd6N01JRGlFb1ZLZWVueVF2Wm51?=
 =?utf-8?Q?qLOnsuC62FbcFmf+jPcN1G8tPmsapblt/k/lnP7?=
X-OriginatorOrg: amperemail.onmicrosoft.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 98cc5d47-7daf-4680-35c6-08d992ec7cb0
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR01MB7238.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Oct 2021 10:37:45.4203
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3bc2b170-fd94-476d-b0ce-4229bdc904a7
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: DhvLgr9/9lZN0gPaL2yZllo4XJfTl3rnGTdKD+KvtPQ85KpxYJdsGS2pk5M+VJyz66OmPT8zp2I2i+IyesCH+qiHbuS32cAASm2TCX8/QFOEBfo9KZ8warZRiKFCH5hI
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR01MB5286
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 19/10/2021 13:46, Joel Stanley wrote:
> On Tue, 19 Oct 2021 at 06:02, Quan Nguyen <quan@os.amperecomputing.com> wrote:
>>
>> Add S0_SCP_AUTH_FAIL, S1_SCP_AUTH_FAIL gpios to indicates firmware
>> authentication fail on each socket.
> 
> These use the gpio-keys API to expose the GPIOs. I think OpenBMC is
> moving away from this abstraction, and instead reading the GPIOs with
> the gpio chardev interface.
Can you give an example of the suggested change?
> 
>>
>> Add gpio RTC_BAT_SEN_EN to enable RTC battery adc sensor.
>>
>> Add BMC_I2C4_O_EN gpio to go high at boot to enable access to I2C4 bus.
> 
> OpenBMC has started a process to document GPIOs that are exposed to
> userspace, initially so a common userspace can be used across
> machines. I like doing it for the additional reason that it provides
> consistency in the naming.
[Thang] The BMC_I2C4_O_EN GPIO is used in kernel only which enabling 
access to all I2C devices in I2C4 bus, like FRU EEPROM. As it is used as 
gpio-hog, no userspace access is allowed/intended. It can't be 
configured in userspace as FRU EEPROM is probed only in kernel boot.
> 
> https://github.com/openbmc/docs/blob/master/designs/device-tree-gpio-naming.md
> 
> If you could take a look at that document and add your GPIOs where
> possible, and then update the device tree.
[Thang] I am aware of this GPIO naming convention document. But I think 
it is to define common name for special GPIOs that applications can use.

>>
>> Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com>
>> Signed-off-by: Thang Nguyen <thang@os.amperecomputing.com>
>> ---
>> v2:
>>    - None
>>
>>   .../arm/boot/dts/aspeed-bmc-ampere-mtjade.dts | 21 ++++++++++++++++++-
>>   1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-ampere-mtjade.dts b/arch/arm/boot/dts/aspeed-bmc-ampere-mtjade.dts
>> index 57b0c45a2298..3515d55bd312 100644
>> --- a/arch/arm/boot/dts/aspeed-bmc-ampere-mtjade.dts
>> +++ b/arch/arm/boot/dts/aspeed-bmc-ampere-mtjade.dts
>> @@ -86,6 +86,18 @@ S0_cpu_fault {
>>                          linux,code = <ASPEED_GPIO(J, 1)>;
>>                  };
>>
>> +               S0_scp_auth_fail {
>> +                       label = "S0_SCP_AUTH_FAIL";
>> +                       gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_LOW>;
>> +                       linux,code = <ASPEED_GPIO(J, 2)>;
>> +               };
>> +
>> +               S1_scp_auth_fail {
>> +                       label = "S1_SCP_AUTH_FAIL";
>> +                       gpios = <&gpio ASPEED_GPIO(Z, 5) GPIO_ACTIVE_LOW>;
>> +                       linux,code = <ASPEED_GPIO(Z, 5)>;
>> +               };
>> +
>>                  S1_overtemp {
>>                          label = "S1_OVERTEMP";
>>                          gpios = <&gpio ASPEED_GPIO(Z, 6) GPIO_ACTIVE_LOW>;
>> @@ -590,7 +602,7 @@ &gpio {
>>          /*Q0-Q7*/       "","","","","","UID_BUTTON","","",
>>          /*R0-R7*/       "","","BMC_EXT_HIGHTEMP_L","OCP_AUX_PWREN",
>>                          "OCP_MAIN_PWREN","RESET_BUTTON","","",
>> -       /*S0-S7*/       "","","","","","","","",
>> +       /*S0-S7*/       "","","","","RTC_BAT_SEN_EN","","","",
> 
> I suggest you create a proposal to call this one
> battery-voltage-read-enable. I know that some of the IBM machines
> intend to have this same GPIO.
[Thang] is there any application to use this pin name?
> 
>>          /*T0-T7*/       "","","","","","","","",
>>          /*U0-U7*/       "","","","","","","","",
>>          /*V0-V7*/       "","","","","","","","",
>> @@ -604,4 +616,11 @@ &gpio {
>>                          "S1_BMC_DDR_ADR","","","","",
>>          /*AC0-AC7*/     "SYS_PWR_GD","","","","","BMC_READY","SLAVE_PRESENT_L",
>>                          "BMC_OCP_PG";
>> +
>> +       i2c4_o_en {
>> +               gpio-hog;
>> +               gpios = <ASPEED_GPIO(Y, 2) GPIO_ACTIVE_HIGH>;
>> +               output-high;
>> +               line-name = "BMC_I2C4_O_EN";
>> +       };
>>   };
>> --
>> 2.28.0
>>