Re: [PATCH 1/1] ARM: dts: aspeed: Add Inspur NF5280M6 BMC machine

[Ping Guo <guopingjn@gmail.com>]

On 5/25/21 3:27 PM, Joel Stanley wrote:
> On Tue, 25 May 2021 at 07:18, Joel Stanley <joel@jms.id.au> wrote:
>> On Tue, 25 May 2021 at 06:48, <guopingjn@gmail.com> wrote:
>>> From: Ping Guo <guoping@inspur.com>
>>>
>>> The Inspur NF5280M6 is an x86 platform server with an AST2500-based BMC.
>>> This dts file provides a basic configuration for its OpenBMC
>>> development.
>>>
>>> Signed-off-by: George Liu <liuxiwei@inspur.com>
>>> Signed-off-by: Ping Guo <guoping@inspur.com>
>> In the future you can use git format-patch to set a version on your
>> patch. "-v 2" will set the version to 2, for example.
>>
>>> ---
>> Put the changelog of what is different compared to last time just
>> below this ---. It won't be included in the git history of the kernel,
>> but it's for reviewers to understand what you've changed.
>>
>> The patch looks good. I will apply this for v5.14 and to the openbmc
>> dev-5.10 tree.
> I spoke too soon. There's a mistake below:
>
>>> +&gpio {
>>> +       power_out {
>>> +               gpios = <ASPEED_GPIO(AA, 7) GPIO_ACTIVE_LOW>;
> This warns:
>
> ../arch/arm/boot/dts/aspeed-bmc-inspur-nf5280m6.dts:138.3-30: Warning
> (gpios_property): /ahb/apb/gpio@1e780000/power_out:gpios: cell 0 is
> not a phandle reference
> ../arch/arm/boot/dts/aspeed-bmc-inspur-nf5280m6.dts:138.3-30: Warning
> (gpios_property): /ahb/apb/gpio@1e780000/power_out:gpios: Could not
> get phandle node for (cell 0)
>
> Taking a closer look, I'm not sure that it's a valid description. What
> are you trying to do there?
>
> Is it supposed to be a gpio hog?
Thanks for your reminder.I'll see.
>>> +               output-low;
>>> +       };
>>> +};
>>> +
>>> +&gpio {
>>> +       status = "okay";
>>> +       /* Enable GPIOE0 and GPIOE2 pass-through by default */
>>> +       pinctrl-names = "pass-through";
>>> +       pinctrl-0 = <&pinctrl_gpie0_default
>>> +                       &pinctrl_gpie2_default>;
>>> +       gpio-line-names =
>>> +&gfx {
>>> +       status = "okay";
>>> +};
> So you're using the BMC display device?
Negative.I'll remove this node.
>>> +
>>> +&kcs3 {
>>> +       status = "okay";
>>> +       kcs_addr = <0xca2>;
> This is the old binding for KCS. Please update it to the newer style
> with the "aspeed,lpc-io-reg" property.
>
>>> +};
>>> +
>>> +&kcs4 {
>>> +       status = "okay";
>>> +       kcs_addr = <0xca4>;
> as above.
Will modify.