All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Tim Harvey <tharvey@gateworks.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Conor Dooley <conor@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>, Li Yang <leoyang.li@nxp.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] dt-bindings: arm: fsl: rename gw7905 to gw75xx
Date: Tue, 28 May 2024 10:58:08 -0500	[thread overview]
Message-ID: <20240528155808.GA695520-robh@kernel.org> (raw)
In-Reply-To: <CAJ+vNU3Rh6f-HrFbBLxNXVP1PwsGh8OyGmmGJBv6+GRwZaTXgw@mail.gmail.com>

On Sat, May 25, 2024 at 12:58:18PM -0700, Tim Harvey wrote:
> On Sat, May 25, 2024 at 11:34 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 24/05/2024 20:40, Conor Dooley wrote:
> > > On Thu, May 23, 2024 at 04:04:50PM -0700, Tim Harvey wrote:
> > >> On Thu, May 23, 2024 at 7:47 AM Conor Dooley <conor@kernel.org> wrote:
> > >>>
> > >>> On Thu, May 23, 2024 at 09:02:46AM +0200, Krzysztof Kozlowski wrote:
> > >>>> On 22/05/2024 23:50, Tim Harvey wrote:
> > >>>>> The GW7905 was renamed to GW7500 before production release.
> > >>>>>
> > >>>>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> > >>>>> ---
> > >>>>>  Documentation/devicetree/bindings/arm/fsl.yaml | 4 ++--
> > >>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> > >>>>>
> > >>>>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > >>>>> index 0027201e19f8..d8bc295079e3 100644
> > >>>>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > >>>>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > >>>>> @@ -920,8 +920,8 @@ properties:
> > >>>>>                - fsl,imx8mm-ddr4-evk       # i.MX8MM DDR4 EVK Board
> > >>>>>                - fsl,imx8mm-evk            # i.MX8MM EVK Board
> > >>>>>                - fsl,imx8mm-evkb           # i.MX8MM EVKB Board
> > >>>>> +              - gateworks,imx8mm-gw75xx-0x # i.MX8MM Gateworks Board
> > >>>>
> > >>>> That's not even equivalent. You 7500 != 75xx.
> > >>>>
> > >>>
> > >>>>>                - gateworks,imx8mm-gw7904
> > >>>>> -              - gateworks,imx8mm-gw7905-0x # i.MX8MM Gateworks Board
> > >>>>
> > >>>> Compatibles do not change. It's just a string. Fixed string.
> > >>>
> > >>> I think there's justification here for removing it, per the commit
> > >>> message, the rename happened before the device was available to
> > >>> customers.
> > >>> Additionally, I think we can give people that upstream things before they're
> > >>> publicly available a bit of slack, otherwise we're just discouraging
> > >>> people from upstreaming early.
> > >>
> > >> Hi Conor,
> > >>
> > >> Thanks for understanding - that's exactly what happened. I'm in the
> > >> habit of submitting patches early and often and it's no fun when
> > >> something like a silly product name gets changed and breaks all the
> > >> hard work.
> > >>
> > >> The board model number is stored in an EEPROM at manufacturing time
> > >> and that EEPROM model is used to build a dt name. So instead of GW7905
> > >> which would be a one-off custom design it was decided to change the
> > >> product to a GW75xx. The difference between GW7500 and GW75xx is
> > >> because we subload components on boards between GW7500/GW7501/GW7502
> > >> etc but the dt is the same.
> > >>
> > >> If there is resistance to a patch that renames it then I guess I'll
> > >> have to submit a patch that removes the obsolete board, then adds back
> > >> the same board under a different name. Shall I do that?
> > >
> > > I think this patch is fine - other than the inconsistency that Krzysztof
> > > pointed out between the "renamed to gw7500" and the "gw75xx" in the new
> > > compatible.
> >
> > I am not a fan of renaming compatibles because of marketing change,
> > because compatible does not have to reflect the marketing name, but
> > there was already precedent from Qualcomm which I did not nak, so fine
> > here as well. Double wildcard 75xx is however a bit worrying.
> >
> 
> Hi Krzysztof,
> 
> Thanks for understanding. The double-wildcard is again a marketing
> tool. All GW75** use the same device-tree by design. The boot firmware
> that chooses the device-tree understands this and for a GW7521 for
> example would look for gw7521 first, gw752x next, gw75xx last.

You haven't documented the other 2 though.

How do "all GW75** use the same device-tree", but then there are 3 
possible DTs for just 1 board?

Selecting a DT is not a unique problem. We don't need unique 
solutions. There's the QCom board-id proposal[1] and OS provided DT[2] 
which are addressing similar issues.

Rob

[1] https://lore.kernel.org/all/20240521-board-ids-v3-0-e6c71d05f4d2@quicinc.com/
[2] https://lists.linaro.org/archives/list/boot-architecture@lists.linaro.org/thread/DZCZSOCRH5BN7YOXEL2OQKSDIY7DCW2M/

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Tim Harvey <tharvey@gateworks.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Conor Dooley <conor@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>, Li Yang <leoyang.li@nxp.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] dt-bindings: arm: fsl: rename gw7905 to gw75xx
Date: Tue, 28 May 2024 10:58:08 -0500	[thread overview]
Message-ID: <20240528155808.GA695520-robh@kernel.org> (raw)
In-Reply-To: <CAJ+vNU3Rh6f-HrFbBLxNXVP1PwsGh8OyGmmGJBv6+GRwZaTXgw@mail.gmail.com>

On Sat, May 25, 2024 at 12:58:18PM -0700, Tim Harvey wrote:
> On Sat, May 25, 2024 at 11:34 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 24/05/2024 20:40, Conor Dooley wrote:
> > > On Thu, May 23, 2024 at 04:04:50PM -0700, Tim Harvey wrote:
> > >> On Thu, May 23, 2024 at 7:47 AM Conor Dooley <conor@kernel.org> wrote:
> > >>>
> > >>> On Thu, May 23, 2024 at 09:02:46AM +0200, Krzysztof Kozlowski wrote:
> > >>>> On 22/05/2024 23:50, Tim Harvey wrote:
> > >>>>> The GW7905 was renamed to GW7500 before production release.
> > >>>>>
> > >>>>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> > >>>>> ---
> > >>>>>  Documentation/devicetree/bindings/arm/fsl.yaml | 4 ++--
> > >>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> > >>>>>
> > >>>>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > >>>>> index 0027201e19f8..d8bc295079e3 100644
> > >>>>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > >>>>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > >>>>> @@ -920,8 +920,8 @@ properties:
> > >>>>>                - fsl,imx8mm-ddr4-evk       # i.MX8MM DDR4 EVK Board
> > >>>>>                - fsl,imx8mm-evk            # i.MX8MM EVK Board
> > >>>>>                - fsl,imx8mm-evkb           # i.MX8MM EVKB Board
> > >>>>> +              - gateworks,imx8mm-gw75xx-0x # i.MX8MM Gateworks Board
> > >>>>
> > >>>> That's not even equivalent. You 7500 != 75xx.
> > >>>>
> > >>>
> > >>>>>                - gateworks,imx8mm-gw7904
> > >>>>> -              - gateworks,imx8mm-gw7905-0x # i.MX8MM Gateworks Board
> > >>>>
> > >>>> Compatibles do not change. It's just a string. Fixed string.
> > >>>
> > >>> I think there's justification here for removing it, per the commit
> > >>> message, the rename happened before the device was available to
> > >>> customers.
> > >>> Additionally, I think we can give people that upstream things before they're
> > >>> publicly available a bit of slack, otherwise we're just discouraging
> > >>> people from upstreaming early.
> > >>
> > >> Hi Conor,
> > >>
> > >> Thanks for understanding - that's exactly what happened. I'm in the
> > >> habit of submitting patches early and often and it's no fun when
> > >> something like a silly product name gets changed and breaks all the
> > >> hard work.
> > >>
> > >> The board model number is stored in an EEPROM at manufacturing time
> > >> and that EEPROM model is used to build a dt name. So instead of GW7905
> > >> which would be a one-off custom design it was decided to change the
> > >> product to a GW75xx. The difference between GW7500 and GW75xx is
> > >> because we subload components on boards between GW7500/GW7501/GW7502
> > >> etc but the dt is the same.
> > >>
> > >> If there is resistance to a patch that renames it then I guess I'll
> > >> have to submit a patch that removes the obsolete board, then adds back
> > >> the same board under a different name. Shall I do that?
> > >
> > > I think this patch is fine - other than the inconsistency that Krzysztof
> > > pointed out between the "renamed to gw7500" and the "gw75xx" in the new
> > > compatible.
> >
> > I am not a fan of renaming compatibles because of marketing change,
> > because compatible does not have to reflect the marketing name, but
> > there was already precedent from Qualcomm which I did not nak, so fine
> > here as well. Double wildcard 75xx is however a bit worrying.
> >
> 
> Hi Krzysztof,
> 
> Thanks for understanding. The double-wildcard is again a marketing
> tool. All GW75** use the same device-tree by design. The boot firmware
> that chooses the device-tree understands this and for a GW7521 for
> example would look for gw7521 first, gw752x next, gw75xx last.

You haven't documented the other 2 though.

How do "all GW75** use the same device-tree", but then there are 3 
possible DTs for just 1 board?

Selecting a DT is not a unique problem. We don't need unique 
solutions. There's the QCom board-id proposal[1] and OS provided DT[2] 
which are addressing similar issues.

Rob

[1] https://lore.kernel.org/all/20240521-board-ids-v3-0-e6c71d05f4d2@quicinc.com/
[2] https://lists.linaro.org/archives/list/boot-architecture@lists.linaro.org/thread/DZCZSOCRH5BN7YOXEL2OQKSDIY7DCW2M/

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

  reply	other threads:[~2024-05-28 15:58 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-22 21:50 [PATCH 1/2] dt-bindings: arm: fsl: rename gw7905 to gw75xx Tim Harvey
2024-05-22 21:50 ` Tim Harvey
2024-05-22 21:50 ` [PATCH 2/2] arm64: dts: freescale: " Tim Harvey
2024-05-22 21:50   ` Tim Harvey
2024-05-23  7:02 ` [PATCH 1/2] dt-bindings: arm: fsl: " Krzysztof Kozlowski
2024-05-23  7:02   ` Krzysztof Kozlowski
2024-05-23 14:47   ` Conor Dooley
2024-05-23 14:47     ` Conor Dooley
2024-05-23 23:04     ` Tim Harvey
2024-05-23 23:04       ` Tim Harvey
2024-05-24 18:40       ` Conor Dooley
2024-05-24 18:40         ` Conor Dooley
2024-05-25 18:34         ` Krzysztof Kozlowski
2024-05-25 18:34           ` Krzysztof Kozlowski
2024-05-25 19:58           ` Tim Harvey
2024-05-25 19:58             ` Tim Harvey
2024-05-28 15:58             ` Rob Herring [this message]
2024-05-28 15:58               ` Rob Herring
2024-05-28 16:23               ` Tim Harvey
2024-05-28 16:23                 ` Tim Harvey
2024-05-30 16:54                 ` Conor Dooley
2024-05-30 16:54                   ` Conor Dooley
2024-05-30 17:36                   ` Tim Harvey
2024-05-30 17:36                     ` Tim Harvey
2024-05-31 14:10                     ` Rob Herring
2024-05-31 14:10                       ` Rob Herring
2024-05-31 14:13 ` Rob Herring
2024-05-31 14:13   ` Rob Herring
2024-08-13 16:22   ` Tim Harvey
2024-08-13 16:34     ` Krzysztof Kozlowski
2024-08-13 17:10       ` Tim Harvey
2024-08-19 13:17         ` Rob Herring

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=20240528155808.GA695520-robh@kernel.org \
    --to=robh@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=conor@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=imx@lists.linux.dev \
    --cc=kernel@pengutronix.de \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=leoyang.li@nxp.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=tharvey@gateworks.com \
    /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.