All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wei Xu <xuwei5@hisilicon.com>
To: Arnd Bergmann <arnd@kernel.org>
Cc: Zhen Lei <thunder.leizhen@huawei.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Rob Herring <robh+dt@kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	devicetree <devicetree@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Zhangfei Gao <zhangfei.gao@linaro.org>,
	Chen Feng <puck.chen@hisilicon.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Subject: Re: [PATCH v3 2/4] arm64: dts: correct vendor prefix hisi to hisilicon
Date: Fri, 29 Jan 2021 09:02:50 +0800	[thread overview]
Message-ID: <60135EBA.5040803@hisilicon.com> (raw)
In-Reply-To: <CAK8P3a31po51NtRhuMsruy2nbqhjguyGP8ZcXwPAwwEiGtLBkg@mail.gmail.com>

Hi Arnd,

On 2021/1/28 22:08, Arnd Bergmann wrote:
> On Wed, Jan 27, 2021 at 1:42 AM Wei Xu <xuwei5@hisilicon.com> wrote:
>> On 2021/1/27 6:23, Arnd Bergmann wrote:
>>> On Tue, Dec 8, 2020 at 1:46 PM Zhen Lei <thunder.leizhen@huawei.com> wrote:
>>>>
>>>> The vendor prefix of "Hisilicon Limited" is "hisilicon", it is clearly
>>>> stated in "vendor-prefixes.yaml".
>>>>
>>>> Fixes: 35ca8168133c ("arm64: dts: Add dts files for Hisilicon Hi3660 SoC")
>>>> Fixes: dd8c7b78c11b ("arm64: dts: Add devicetree for Hisilicon Hi3670 SoC")
>>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>>>> Cc: Chen Feng <puck.chen@hisilicon.com>
>>>> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>>
>>> I see this change in the pull request I got, but I'm a bit worried about the
>>> incompatible binding change. Wouldn't the correct path forward be to
>>> list both the correct and the incorrect properties, both in the dts file
>>> and in the driver that interprets the properties?
>>
>> Thanks for the comment!
>> The reset driver will look for "hisilicon" firstly and fall back to "hisi".
>> And the DTS is shipped with the driver together.
>> So I think there is no compatible issue here.
>> Please let me know if missed anything. Thanks!
> 
> There are three things that can go wrong here, and this is only addressing
> one of them:

Thanks for the detailed explanation! 

> 
> 1. Updating the kernel on a machine with a dtb provided by the firmware
>   is a problem if the new driver can not handle the old properties. This
>   is correctly handled by the driver's fallback as soon as both trees
>   are merged.

Agreed and the driver has been merged into the v5.11-rc1.

> 
> 2. Updating the dtb while running an older kernel is now broken since
>   the driver can no longer read the property. This is less critical, but
>   it does seem easy enough to work around here by leaving both
>   properties in place.

Yes, it is.
But if leaving both in place, the dtbs_check will report following warning again:
	'hisi,rst-syscon' does not match any of the regexes

That is why leizhen changed the dtb.
Do you think it is OK to assume no one will use the new dtb with an older kernel?

> 
> 3. Bisecting through the git history across an incompatible change
>   means you can run into broken commits. We try hard to avoid that
>   if we are aware of a problem in advance. In this case it could be
>   avoided by only merging the incompatible DT change in a following
>   merge window after the driver change, or (better) by making it
>   a backward-compatible change the same way as addressing 2.

Yes, agreed.
And The DT change pull request is sent after the driver has been merged into v5.11-rc1.

Really appreciate the detail you went to!

Best Regards,
Wei

> 
>          Arnd
> .
> 

WARNING: multiple messages have this Message-ID (diff)
From: Wei Xu <xuwei5@hisilicon.com>
To: Arnd Bergmann <arnd@kernel.org>
Cc: devicetree <devicetree@vger.kernel.org>,
	Chen Feng <puck.chen@hisilicon.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Zhen Lei <thunder.leizhen@huawei.com>,
	Zhangfei Gao <zhangfei.gao@linaro.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3 2/4] arm64: dts: correct vendor prefix hisi to hisilicon
Date: Fri, 29 Jan 2021 09:02:50 +0800	[thread overview]
Message-ID: <60135EBA.5040803@hisilicon.com> (raw)
In-Reply-To: <CAK8P3a31po51NtRhuMsruy2nbqhjguyGP8ZcXwPAwwEiGtLBkg@mail.gmail.com>

Hi Arnd,

On 2021/1/28 22:08, Arnd Bergmann wrote:
> On Wed, Jan 27, 2021 at 1:42 AM Wei Xu <xuwei5@hisilicon.com> wrote:
>> On 2021/1/27 6:23, Arnd Bergmann wrote:
>>> On Tue, Dec 8, 2020 at 1:46 PM Zhen Lei <thunder.leizhen@huawei.com> wrote:
>>>>
>>>> The vendor prefix of "Hisilicon Limited" is "hisilicon", it is clearly
>>>> stated in "vendor-prefixes.yaml".
>>>>
>>>> Fixes: 35ca8168133c ("arm64: dts: Add dts files for Hisilicon Hi3660 SoC")
>>>> Fixes: dd8c7b78c11b ("arm64: dts: Add devicetree for Hisilicon Hi3670 SoC")
>>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>>>> Cc: Chen Feng <puck.chen@hisilicon.com>
>>>> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>>
>>> I see this change in the pull request I got, but I'm a bit worried about the
>>> incompatible binding change. Wouldn't the correct path forward be to
>>> list both the correct and the incorrect properties, both in the dts file
>>> and in the driver that interprets the properties?
>>
>> Thanks for the comment!
>> The reset driver will look for "hisilicon" firstly and fall back to "hisi".
>> And the DTS is shipped with the driver together.
>> So I think there is no compatible issue here.
>> Please let me know if missed anything. Thanks!
> 
> There are three things that can go wrong here, and this is only addressing
> one of them:

Thanks for the detailed explanation! 

> 
> 1. Updating the kernel on a machine with a dtb provided by the firmware
>   is a problem if the new driver can not handle the old properties. This
>   is correctly handled by the driver's fallback as soon as both trees
>   are merged.

Agreed and the driver has been merged into the v5.11-rc1.

> 
> 2. Updating the dtb while running an older kernel is now broken since
>   the driver can no longer read the property. This is less critical, but
>   it does seem easy enough to work around here by leaving both
>   properties in place.

Yes, it is.
But if leaving both in place, the dtbs_check will report following warning again:
	'hisi,rst-syscon' does not match any of the regexes

That is why leizhen changed the dtb.
Do you think it is OK to assume no one will use the new dtb with an older kernel?

> 
> 3. Bisecting through the git history across an incompatible change
>   means you can run into broken commits. We try hard to avoid that
>   if we are aware of a problem in advance. In this case it could be
>   avoided by only merging the incompatible DT change in a following
>   merge window after the driver change, or (better) by making it
>   a backward-compatible change the same way as addressing 2.

Yes, agreed.
And The DT change pull request is sent after the driver has been merged into v5.11-rc1.

Really appreciate the detail you went to!

Best Regards,
Wei

> 
>          Arnd
> .
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-01-29  1:05 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-08 12:46 [PATCH v3 0/4] dt-bindings: reset: convert Hisilicon reset controller bindings to json-schema Zhen Lei
2020-12-08 12:46 ` Zhen Lei
2020-12-08 12:46 ` [PATCH v3 1/4] reset: hisilicon: correct vendor prefix Zhen Lei
2020-12-08 12:46   ` Zhen Lei
2020-12-08 12:46 ` [PATCH v3 2/4] arm64: dts: correct vendor prefix hisi to hisilicon Zhen Lei
2020-12-08 12:46   ` Zhen Lei
2021-01-26  8:53   ` Wei Xu
2021-01-26  8:53     ` Wei Xu
2021-01-26 22:23   ` Arnd Bergmann
2021-01-26 22:23     ` Arnd Bergmann
2021-01-27  0:42     ` Wei Xu
2021-01-27  0:42       ` Wei Xu
2021-01-28 14:08       ` Arnd Bergmann
2021-01-28 14:08         ` Arnd Bergmann
2021-01-29  1:02         ` Wei Xu [this message]
2021-01-29  1:02           ` Wei Xu
2021-01-29  1:55           ` Wei Xu
2021-01-29  1:55             ` Wei Xu
2021-01-29  8:21             ` Arnd Bergmann
2021-01-29  8:27               ` Wei Xu
2021-01-27  1:21     ` Leizhen (ThunderTown)
2021-01-27  1:21       ` Leizhen (ThunderTown)
2020-12-08 12:46 ` [PATCH v3 3/4] dt-bindings: reset: " Zhen Lei
2020-12-08 12:46   ` Zhen Lei
2020-12-10 14:01   ` Rob Herring
2020-12-10 14:01     ` Rob Herring
2020-12-08 12:46 ` [PATCH v3 4/4] dt-bindings: reset: convert Hisilicon reset controller bindings to json-schema Zhen Lei
2020-12-08 12:46   ` Zhen Lei
2020-12-10 14:02   ` Rob Herring
2020-12-10 14:02     ` Rob Herring
2020-12-10 15:06 ` [PATCH v3 0/4] " Philipp Zabel
2020-12-10 15:06   ` Philipp Zabel

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=60135EBA.5040803@hisilicon.com \
    --to=xuwei5@hisilicon.com \
    --cc=arnd@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manivannan.sadhasivam@linaro.org \
    --cc=p.zabel@pengutronix.de \
    --cc=puck.chen@hisilicon.com \
    --cc=robh+dt@kernel.org \
    --cc=thunder.leizhen@huawei.com \
    --cc=zhangfei.gao@linaro.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.