Re: [PATCH] HPE BMC GXP SUPPORT

From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
To: Rob Herring <robh+dt@kernel.org>, nick.hawkins@hpe.com
Cc: verdun@hpe.com, "David Airlie" <airlied@linux.ie>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Bartosz Golaszewski" <brgl@bgdev.pl>,
	"Corey Minyard" <minyard@acm.org>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Lee Jones" <lee.jones@linaro.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Mark Brown" <broonie@kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Daniel Lezcano" <daniel.lezcano@linaro.org>,
	"Amit Kucheria" <amitk@kernel.org>,
	"Zhang Rui" <rui.zhang@intel.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Wim Van Sebroeck" <wim@linux-watchdog.org>,
	"Guenter Roeck" <linux@roeck-us.net>,
	"Russell King" <linux@armlinux.org.uk>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Olof Johansson" <olof@lixom.net>, "SoC Team" <soc@kernel.org>,
	"Shawn Guo" <shawnguo@kernel.org>,
	"Stanislav Jakubek" <stano.jakubek@gmail.com>,
	"Sam Ravnborg" <sam@ravnborg.org>,
	"Hao Fang" <fanghao11@huawei.com>,
	"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Marc Zyngier" <maz@kernel.org>,
	"Ard Biesheuvel" <ardb@kernel.org>,
	"Anshuman Khandual" <anshuman.khandual@arm.com>,
	"Wang Kefeng" <wangkefeng.wang@huawei.com>,
	"Lukas Bulwahn" <lukas.bulwahn@gmail.com>,
	"Masahiro Yamada" <masahiroy@kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	devicetree@vger.kernel.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	"Linux I2C" <linux-i2c@vger.kernel.org>,
	openipmi-developer@lists.sourceforge.net,
	"MTD Maling List" <linux-mtd@lists.infradead.org>,
	netdev <netdev@vger.kernel.org>,
	"Linux PWM List" <linux-pwm@vger.kernel.org>,
	"open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>,
	linux-spi <linux-spi@vger.kernel.org>,
	"open list:THERMAL" <linux-pm@vger.kernel.org>,
	"Linux USB List" <linux-usb@vger.kernel.org>,
	LINUX-WATCHDOG <linux-watchdog@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] HPE BMC GXP SUPPORT
Date: Thu, 3 Feb 2022 15:55:56 +0100	[thread overview]
Message-ID: <e79133f2-f872-3ed6-4038-526e94e84909@canonical.com> (raw)
In-Reply-To: <CAL_Jsq+K2t5WYE056so1iZgZr7CBKvDEjAwnJVTyUFQcK-VFSA@mail.gmail.com>

On 03/02/2022 15:29, Rob Herring wrote:
> On Wed, Feb 2, 2022 at 10:55 AM <nick.hawkins@hpe.com> wrote:
>>
>> From: Nick Hawkins <nick.hawkins@hpe.com>
>>

(...)

>> +
>> +       vuart_a: vuart_a@80fd0200 {
> 
> serial@...

Maybe it does not look like, but this is actually a v2. Nick was asked
to change the naming for the nodes already in v1. Unfortunately it did
not happen, so we have vuart, spifi, vic and more.

It is a waste of reviewers' time to ask them to perform the same review
twice or to ignore their comments.

> 
>> +               compatible = "hpe,gxp-vuart";
>> +               reg = <0x80fd0200 0x100>;
>> +               interrupts = <2>;
>> +               interrupt-parent = <&vic1>;
>> +               clock-frequency = <1846153>;
>> +               reg-shift = <0>;
>> +               status = "okay";
>> +               serial-line = <3>;
>> +               vuart_cfg = <&vuart_a_cfg>;
>> +       };

(...)

>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> index 294093d45a23..913f722a6b8d 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> @@ -514,7 +514,9 @@ patternProperties:
>>    "^hoperun,.*":
>>      description: Jiangsu HopeRun Software Co., Ltd.
>>    "^hp,.*":
>> -    description: Hewlett Packard
>> +    description: Hewlett Packard Inc.
> 
> Why are you changing this one?

I guess this is squashing of my patch:
https://lore.kernel.org/all/20220127075229.10299-1-krzysztof.kozlowski@canonical.com/

which is fine to me, but vendor changve should be a separate commit with
its own explanation. Now it looks indeed weird.

> 
>> +  "^hpe,.*":
> 
> You used HPE elsewhere... Lowercase is preferred.

Best regards,
Krzysztof