[09/15] dt-bindings: phy: qcom,qmp-pcie: mark current bindings as legacy

  The current QMP PCIe PHY bindings are based on the original MSM8996
binding which provided multiple PHYs per IP block and these in turn were
described by child nodes.

Later QMP PCIe PHY blocks only provide a single PHY and the remnant
child node does not really reflect the hardware.

The original MSM8996 binding also ended up describing the individual
register blocks as belonging to either the wrapper node or the PHY child
nodes.

This is an unnecessary level of detail which has lead to problems when
later IP blocks using different register layouts have been forced to fit
the original mould rather than updating the binding. The bindings are
arguable also incomplete as they only the describe register blocks used
by the current Linux drivers (e.g. does not include the per lane PCS
registers).

In preparation for adding new bindings for SC8280XP which further
bindings can be based on, mark the current bindings as "legacy".

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 .../{qcom,qmp-pcie-phy.yaml => qcom,qmp-pcie-phy-legacy.yaml} | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
 rename Documentation/devicetree/bindings/phy/{qcom,qmp-pcie-phy.yaml => qcom,qmp-pcie-phy-legacy.yaml} (98%)
  

On 17/10/2022 10:53, Johan Hovold wrote:
> The current QMP PCIe PHY bindings are based on the original MSM8996
> binding which provided multiple PHYs per IP block and these in turn were
> described by child nodes.
> 
> Later QMP PCIe PHY blocks only provide a single PHY and the remnant
> child node does not really reflect the hardware.
> 
> The original MSM8996 binding also ended up describing the individual
> register blocks as belonging to either the wrapper node or the PHY child
> nodes.
> 
> This is an unnecessary level of detail which has lead to problems when
> later IP blocks using different register layouts have been forced to fit
> the original mould rather than updating the binding. The bindings are
> arguable also incomplete as they only the describe register blocks used
> by the current Linux drivers (e.g. does not include the per lane PCS
> registers).
> 
> In preparation for adding new bindings for SC8280XP which further
> bindings can be based on, mark the current bindings as "legacy".
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  .../{qcom,qmp-pcie-phy.yaml => qcom,qmp-pcie-phy-legacy.yaml} | 4 ++--

I don't think we should rename anything as legacy. These are "normal"
platforms, not legacy ones. SM8450 is not even that old.

The recommendation is to keep names matching the compatibles, not adding
some legacy/newer/newest suffixes.

Best regards,
Krzysztof
  

On Mon, Oct 17, 2022 at 01:15:45PM -0400, Krzysztof Kozlowski wrote:
> On 17/10/2022 10:53, Johan Hovold wrote:
> > The current QMP PCIe PHY bindings are based on the original MSM8996
> > binding which provided multiple PHYs per IP block and these in turn were
> > described by child nodes.
> > 
> > Later QMP PCIe PHY blocks only provide a single PHY and the remnant
> > child node does not really reflect the hardware.
> > 
> > The original MSM8996 binding also ended up describing the individual
> > register blocks as belonging to either the wrapper node or the PHY child
> > nodes.
> > 
> > This is an unnecessary level of detail which has lead to problems when
> > later IP blocks using different register layouts have been forced to fit
> > the original mould rather than updating the binding. The bindings are
> > arguable also incomplete as they only the describe register blocks used
> > by the current Linux drivers (e.g. does not include the per lane PCS
> > registers).
> > 
> > In preparation for adding new bindings for SC8280XP which further
> > bindings can be based on, mark the current bindings as "legacy".
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >  .../{qcom,qmp-pcie-phy.yaml => qcom,qmp-pcie-phy-legacy.yaml} | 4 ++--
> 
> I don't think we should rename anything as legacy. These are "normal"
> platforms, not legacy ones. SM8450 is not even that old.

I'm not really referring to the platforms as legacy, but the rather the
format of the bindings. The intent is that by marking the current ones
as such, people will not base new bindings on the old scheme.

There's no problem supporting both schemes in the driver also for the
current compatibles, but expressing such a deprecation in DT schema
sounds like it would be painful. We instead decided to simple draw the
line at SC8280XP and have future bindings be based on its binding.

> The recommendation is to keep names matching the compatibles, not adding
> some legacy/newer/newest suffixes.

Yeah, I know, but that's not what the current bindings do. And if we
keep 

	qcom,qmp-pcie-phy.yaml

and add

	qcom,sc8280xp-qmp-pcie-phy.yaml

then I fear that people will base their bindings on the former rather
than the latter.

I guess I can just add a comment in the old schema file with a reference
to the sc8280xp bindings to try to prevent people from adding new ones
in the wrong place.

If I understand you correctly this is what you are suggesting? And that
the new file should still be named "qcom,sc8280xp-qmp-pcie-phy.yaml"
also as new bindings are added to that file?

I could also rename the old schema file after one of the old platforms
platforms therein (e.g. qcom,msm8998-qmp-pcie-phy) to make it sounds
less like a generic schema for new bindings.

That is

	qcom,msm8998-qmp-pcie-phy.yaml + comment (for current bindings)
	qcom,sc8280xp-qmp-pcie-phy.yaml (for new bindings)

Johan
  

Hi,

On Mon, 17 Oct 2022 at 17:54, Johan Hovold <johan+linaro@kernel.org> wrote:
>
> The current QMP PCIe PHY bindings are based on the original MSM8996
> binding which provided multiple PHYs per IP block and these in turn were
> described by child nodes.
>
> Later QMP PCIe PHY blocks only provide a single PHY and the remnant
> child node does not really reflect the hardware.
>
> The original MSM8996 binding also ended up describing the individual
> register blocks as belonging to either the wrapper node or the PHY child
> nodes.
>
> This is an unnecessary level of detail which has lead to problems when
> later IP blocks using different register layouts have been forced to fit
> the original mould rather than updating the binding. The bindings are
> arguable also incomplete as they only the describe register blocks used
> by the current Linux drivers (e.g. does not include the per lane PCS
> registers).

I'd like to point out that it's not only a problem peculiar to the
PCIe PHYs. Other QMP PHY families also follow the same approach of
separating the serdes into the common part and rx/tx/PCS/whatever into
the PHY subnodes.
For the USB+DP combo PHYs we have to have subnodes, however it would
also be logical to move serdes register to the subnode devices,
leaving only DP_COM in the base node.

That said, I think we should rethink and agree on QMP PHY bindings,
before renaming the bindings. And yes, I think we should also upgrade
older DTs, keeping drivers backwards compatible (for some time?).

> In preparation for adding new bindings for SC8280XP which further
> bindings can be based on, mark the current bindings as "legacy".
>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  .../{qcom,qmp-pcie-phy.yaml => qcom,qmp-pcie-phy-legacy.yaml} | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>  rename Documentation/devicetree/bindings/phy/{qcom,qmp-pcie-phy.yaml => qcom,qmp-pcie-phy-legacy.yaml} (98%)
  

On Tue, Oct 18, 2022 at 12:52:03PM +0300, Dmitry Baryshkov wrote:
> Hi,
> 
> On Mon, 17 Oct 2022 at 17:54, Johan Hovold <johan+linaro@kernel.org> wrote:
> >
> > The current QMP PCIe PHY bindings are based on the original MSM8996
> > binding which provided multiple PHYs per IP block and these in turn were
> > described by child nodes.
> >
> > Later QMP PCIe PHY blocks only provide a single PHY and the remnant
> > child node does not really reflect the hardware.
> >
> > The original MSM8996 binding also ended up describing the individual
> > register blocks as belonging to either the wrapper node or the PHY child
> > nodes.
> >
> > This is an unnecessary level of detail which has lead to problems when
> > later IP blocks using different register layouts have been forced to fit
> > the original mould rather than updating the binding. The bindings are
> > arguable also incomplete as they only the describe register blocks used
> > by the current Linux drivers (e.g. does not include the per lane PCS
> > registers).
> 
> I'd like to point out that it's not only a problem peculiar to the
> PCIe PHYs. Other QMP PHY families also follow the same approach of
> separating the serdes into the common part and rx/tx/PCS/whatever into
> the PHY subnodes.

Yeah, I already mentioned that in the cover letter.

> For the USB+DP combo PHYs we have to have subnodes, however it would
> also be logical to move serdes register to the subnode devices,
> leaving only DP_COM in the base node.

No, not at all. First, we may not even need the subnodes (the individual
PHYs can be indexed), but even if we do keep them for the combo case,
the register block should go in the wrapper node as the registers can be
and are shared (e.g. for sc8280xp for which the current binding is
completely broken).

> That said, I think we should rethink and agree on QMP PHY bindings,
> before renaming the bindings.

Isn't that what we are doing just now?

> And yes, I think we should also upgrade
> older DTs, keeping drivers backwards compatible (for some time?).

Possibly, but I'm not sure it's worth the dts churn. As I mentioned
elsewhere, supporting both the old and new binding in the driver is
mostly trivial, while encoding the deprecated bindings in DT schema
sounds like it would be painful.

On the other hand, adding support for new features to (or fixing bugs
in) old platforms using the current bindings may potentially become
easier if they are also converted.

> > In preparation for adding new bindings for SC8280XP which further
> > bindings can be based on, mark the current bindings as "legacy".
> >
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >  .../{qcom,qmp-pcie-phy.yaml => qcom,qmp-pcie-phy-legacy.yaml} | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >  rename Documentation/devicetree/bindings/phy/{qcom,qmp-pcie-phy.yaml => qcom,qmp-pcie-phy-legacy.yaml} (98%)

Johan
  

On Tue, 18 Oct 2022 at 13:21, Johan Hovold <johan@kernel.org> wrote:
>
> On Tue, Oct 18, 2022 at 12:52:03PM +0300, Dmitry Baryshkov wrote:
> > Hi,
> >
> > On Mon, 17 Oct 2022 at 17:54, Johan Hovold <johan+linaro@kernel.org> wrote:
> > >
> > > The current QMP PCIe PHY bindings are based on the original MSM8996
> > > binding which provided multiple PHYs per IP block and these in turn were
> > > described by child nodes.
> > >
> > > Later QMP PCIe PHY blocks only provide a single PHY and the remnant
> > > child node does not really reflect the hardware.
> > >
> > > The original MSM8996 binding also ended up describing the individual
> > > register blocks as belonging to either the wrapper node or the PHY child
> > > nodes.
> > >
> > > This is an unnecessary level of detail which has lead to problems when
> > > later IP blocks using different register layouts have been forced to fit
> > > the original mould rather than updating the binding. The bindings are
> > > arguable also incomplete as they only the describe register blocks used
> > > by the current Linux drivers (e.g. does not include the per lane PCS
> > > registers).
> >
> > I'd like to point out that it's not only a problem peculiar to the
> > PCIe PHYs. Other QMP PHY families also follow the same approach of
> > separating the serdes into the common part and rx/tx/PCS/whatever into
> > the PHY subnodes.
>
> Yeah, I already mentioned that in the cover letter.
>
> > For the USB+DP combo PHYs we have to have subnodes, however it would
> > also be logical to move serdes register to the subnode devices,
> > leaving only DP_COM in the base node.
>
> No, not at all. First, we may not even need the subnodes (the individual
> PHYs can be indexed), but even if we do keep them for the combo case,
> the register block should go in the wrapper node as the registers can be
> and are shared (e.g. for sc8280xp for which the current binding is
> completely broken).

Hmm, which register blocks are shared on the sc8280xp combo PHY? Could
you please lightly describe it, if possible?

> > That said, I think we should rethink and agree on QMP PHY bindings,
> > before renaming the bindings.
>
> Isn't that what we are doing just now?

Yeah, thanks for starting the discussion!

> > And yes, I think we should also upgrade
> > older DTs, keeping drivers backwards compatible (for some time?).
>
> Possibly, but I'm not sure it's worth the dts churn. As I mentioned
> elsewhere, supporting both the old and new binding in the driver is
> mostly trivial, while encoding the deprecated bindings in DT schema
> sounds like it would be painful.

This is probably the time where Krzysztof can advise us. I'm still not
sure when it is expected to encode both old and new bindings in the
schema and when we can update both the schema and the DT.

> On the other hand, adding support for new features to (or fixing bugs
> in) old platforms using the current bindings may potentially become
> easier if they are also converted.

Yes!

> > > In preparation for adding new bindings for SC8280XP which further
> > > bindings can be based on, mark the current bindings as "legacy".
> > >
> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > > ---
> > >  .../{qcom,qmp-pcie-phy.yaml => qcom,qmp-pcie-phy-legacy.yaml} | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >  rename Documentation/devicetree/bindings/phy/{qcom,qmp-pcie-phy.yaml => qcom,qmp-pcie-phy-legacy.yaml} (98%)
>
> Johan
  

On Tue, Oct 18, 2022 at 02:37:35PM +0300, Dmitry Baryshkov wrote:
> On Tue, 18 Oct 2022 at 13:21, Johan Hovold <johan@kernel.org> wrote:
> >
> > On Tue, Oct 18, 2022 at 12:52:03PM +0300, Dmitry Baryshkov wrote:
> > > Hi,
> > >
> > > On Mon, 17 Oct 2022 at 17:54, Johan Hovold <johan+linaro@kernel.org> wrote:
> > > >
> > > > The current QMP PCIe PHY bindings are based on the original MSM8996
> > > > binding which provided multiple PHYs per IP block and these in turn were
> > > > described by child nodes.
> > > >
> > > > Later QMP PCIe PHY blocks only provide a single PHY and the remnant
> > > > child node does not really reflect the hardware.
> > > >
> > > > The original MSM8996 binding also ended up describing the individual
> > > > register blocks as belonging to either the wrapper node or the PHY child
> > > > nodes.
> > > >
> > > > This is an unnecessary level of detail which has lead to problems when
> > > > later IP blocks using different register layouts have been forced to fit
> > > > the original mould rather than updating the binding. The bindings are
> > > > arguable also incomplete as they only the describe register blocks used
> > > > by the current Linux drivers (e.g. does not include the per lane PCS
> > > > registers).
> > >
> > > I'd like to point out that it's not only a problem peculiar to the
> > > PCIe PHYs. Other QMP PHY families also follow the same approach of
> > > separating the serdes into the common part and rx/tx/PCS/whatever into
> > > the PHY subnodes.
> >
> > Yeah, I already mentioned that in the cover letter.
> >
> > > For the USB+DP combo PHYs we have to have subnodes, however it would
> > > also be logical to move serdes register to the subnode devices,
> > > leaving only DP_COM in the base node.
> >
> > No, not at all. First, we may not even need the subnodes (the individual
> > PHYs can be indexed), but even if we do keep them for the combo case,
> > the register block should go in the wrapper node as the registers can be
> > and are shared (e.g. for sc8280xp for which the current binding is
> > completely broken).
> 
> Hmm, which register blocks are shared on the sc8280xp combo PHY? Could
> you please lightly describe it, if possible?

At least serdes and tx. Note that the combo PHY on sc8280xp also
supports USB4, so there is likely some differences compared to the older
platforms.

And while I haven't looked in detail at the older platforms for the
combo PHYs yet, describing the separate register subregions there
appears to be just as misguided for those (e.g. as the binding currently
only describes what Linux is using) and is mostly an artefact of how the
original Linux driver was implemented.

Johan
  

On 18/10/2022 03:06, Johan Hovold wrote:
> On Mon, Oct 17, 2022 at 01:15:45PM -0400, Krzysztof Kozlowski wrote:
>> On 17/10/2022 10:53, Johan Hovold wrote:
>>> The current QMP PCIe PHY bindings are based on the original MSM8996
>>> binding which provided multiple PHYs per IP block and these in turn were
>>> described by child nodes.
>>>
>>> Later QMP PCIe PHY blocks only provide a single PHY and the remnant
>>> child node does not really reflect the hardware.
>>>
>>> The original MSM8996 binding also ended up describing the individual
>>> register blocks as belonging to either the wrapper node or the PHY child
>>> nodes.
>>>
>>> This is an unnecessary level of detail which has lead to problems when
>>> later IP blocks using different register layouts have been forced to fit
>>> the original mould rather than updating the binding. The bindings are
>>> arguable also incomplete as they only the describe register blocks used
>>> by the current Linux drivers (e.g. does not include the per lane PCS
>>> registers).
>>>
>>> In preparation for adding new bindings for SC8280XP which further
>>> bindings can be based on, mark the current bindings as "legacy".
>>>
>>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>>> ---
>>>  .../{qcom,qmp-pcie-phy.yaml => qcom,qmp-pcie-phy-legacy.yaml} | 4 ++--
>>
>> I don't think we should rename anything as legacy. These are "normal"
>> platforms, not legacy ones. SM8450 is not even that old.
> 
> I'm not really referring to the platforms as legacy, but the rather the
> format of the bindings. The intent is that by marking the current ones
> as such, people will not base new bindings on the old scheme.
> 
> There's no problem supporting both schemes in the driver also for the
> current compatibles, but expressing such a deprecation in DT schema
> sounds like it would be painful. We instead decided to simple draw the
> line at SC8280XP and have future bindings be based on its binding.
> 
>> The recommendation is to keep names matching the compatibles, not adding
>> some legacy/newer/newest suffixes.
> 
> Yeah, I know, but that's not what the current bindings do. And if we
> keep 
> 
> 	qcom,qmp-pcie-phy.yaml
> 
> and add
> 
> 	qcom,sc8280xp-qmp-pcie-phy.yaml
> 
> then I fear that people will base their bindings on the former rather
> than the latter.

Then how about renaming this file to something matching the oldest
supported SoC? Like: qcom,msm8998-qmp-pcie-phy.yaml
(I don't know which one is the oldest there)

Or ipq6018 as the first one appearing in the list.

> 
> I guess I can just add a comment in the old schema file with a reference
> to the sc8280xp bindings to try to prevent people from adding new ones
> in the wrong place.

Yes, that's also works for me. You can change the description part to
have something like:
"QMP PHY controller on SoCs like sc8180x and older. For newer SoCs,
please look at xxxxx.yaml"

> If I understand you correctly this is what you are suggesting? And that
> the new file should still be named "qcom,sc8280xp-qmp-pcie-phy.yaml"
> also as new bindings are added to that file?

Yes.

> 
> I could also rename the old schema file after one of the old platforms
> platforms therein (e.g. qcom,msm8998-qmp-pcie-phy) to make it sounds
> less like a generic schema for new bindings.

Oh, we thought about the same.

> 
> That is
> 
> 	qcom,msm8998-qmp-pcie-phy.yaml + comment (for current bindings)
> 	qcom,sc8280xp-qmp-pcie-phy.yaml (for new bindings)

Yes, please.

Best regards,
Krzysztof
  

On 18/10/2022 07:37, Dmitry Baryshkov wrote:
> 
>>> And yes, I think we should also upgrade
>>> older DTs, keeping drivers backwards compatible (for some time?).
>>
>> Possibly, but I'm not sure it's worth the dts churn. As I mentioned
>> elsewhere, supporting both the old and new binding in the driver is
>> mostly trivial, while encoding the deprecated bindings in DT schema
>> sounds like it would be painful.
> 
> This is probably the time where Krzysztof can advise us. I'm still not
> sure when it is expected to encode both old and new bindings in the
> schema and when we can update both the schema and the DT.

I do not follow what exactly the proposal is. Are you asking whether to:
1. keep existing DTS compatible with old driver?
or
2. update existing DTS so it is working only with new driver (and not
compatible with old driver thus having ABI break)?

If so, it is less question to bindings but more to the usage of DTS in
other projects (like bootloaders, firmware, BSD) and generic
recommendation is: do not break other users, if possible. It is however
up to the platform maintainer (Bjorn) to decide on this, not on me.

Best regards,
Krzysztof
  

On Tue, Oct 18, 2022 at 11:27:35AM -0400, Krzysztof Kozlowski wrote:
> On 18/10/2022 03:06, Johan Hovold wrote:
> > On Mon, Oct 17, 2022 at 01:15:45PM -0400, Krzysztof Kozlowski wrote:
> >> On 17/10/2022 10:53, Johan Hovold wrote:
> >>> The current QMP PCIe PHY bindings are based on the original MSM8996
> >>> binding which provided multiple PHYs per IP block and these in turn were
> >>> described by child nodes.

> >>> In preparation for adding new bindings for SC8280XP which further
> >>> bindings can be based on, mark the current bindings as "legacy".
> >>>
> >>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> >>> ---
> >>>  .../{qcom,qmp-pcie-phy.yaml => qcom,qmp-pcie-phy-legacy.yaml} | 4 ++--
> >>
> >> I don't think we should rename anything as legacy. These are "normal"
> >> platforms, not legacy ones. SM8450 is not even that old.
> > 
> > I'm not really referring to the platforms as legacy, but the rather the
> > format of the bindings. The intent is that by marking the current ones
> > as such, people will not base new bindings on the old scheme.
> > 
> > There's no problem supporting both schemes in the driver also for the
> > current compatibles, but expressing such a deprecation in DT schema
> > sounds like it would be painful. We instead decided to simple draw the
> > line at SC8280XP and have future bindings be based on its binding.
> > 
> >> The recommendation is to keep names matching the compatibles, not adding
> >> some legacy/newer/newest suffixes.
> > 
> > Yeah, I know, but that's not what the current bindings do. And if we
> > keep 
> > 
> > 	qcom,qmp-pcie-phy.yaml
> > 
> > and add
> > 
> > 	qcom,sc8280xp-qmp-pcie-phy.yaml
> > 
> > then I fear that people will base their bindings on the former rather
> > than the latter.
> 
> Then how about renaming this file to something matching the oldest
> supported SoC? Like: qcom,msm8998-qmp-pcie-phy.yaml
> (I don't know which one is the oldest there)
> 
> Or ipq6018 as the first one appearing in the list.

Sounds good. :)

> > I could also rename the old schema file after one of the old platforms
> > platforms therein (e.g. qcom,msm8998-qmp-pcie-phy) to make it sounds
> > less like a generic schema for new bindings.
> 
> Oh, we thought about the same.
> 
> > 
> > That is
> > 
> > 	qcom,msm8998-qmp-pcie-phy.yaml + comment (for current bindings)
> > 	qcom,sc8280xp-qmp-pcie-phy.yaml (for new bindings)
> 
> Yes, please.

I'll go with that then. Thanks!

Johan
  

On Tue, Oct 18, 2022 at 11:32:07AM -0400, Krzysztof Kozlowski wrote:
> On 18/10/2022 07:37, Dmitry Baryshkov wrote:
> > 
> >>> And yes, I think we should also upgrade
> >>> older DTs, keeping drivers backwards compatible (for some time?).
> >>
> >> Possibly, but I'm not sure it's worth the dts churn. As I mentioned
> >> elsewhere, supporting both the old and new binding in the driver is
> >> mostly trivial, while encoding the deprecated bindings in DT schema
> >> sounds like it would be painful.
> > 
> > This is probably the time where Krzysztof can advise us. I'm still not
> > sure when it is expected to encode both old and new bindings in the
> > schema and when we can update both the schema and the DT.
> 
> I do not follow what exactly the proposal is. Are you asking whether to:
> 1. keep existing DTS compatible with old driver?
> or
> 2. update existing DTS so it is working only with new driver (and not
> compatible with old driver thus having ABI break)?
> 
> If so, it is less question to bindings but more to the usage of DTS in
> other projects (like bootloaders, firmware, BSD) and generic
> recommendation is: do not break other users, if possible. It is however
> up to the platform maintainer (Bjorn) to decide on this, not on me.

The question is whether to convert also the current bindings and DTS to
the new (sc8280xp) scheme (e.g. drop the child nodes and register
subregions).

The driver can support both binding schemes using the same compatible
strings for a transition period (or in theory forever) by checking for
the existence of a child node.

Converting the DTS to use the new bindings would obviously prevent using
them with an old kernel (i.e. 2 above), but I don't think that's a
problem (unlike backward compatibility during at least a transition
period).

My concern was how to describe the deprecation in DT schema if we were
convert them. By instead just keeping the old bindings as-is in a
separate file and continuing to support them in the driver we can have a
nice and clean description of the new bindings (sc8280xp) without the
legacy cruft.

If we were to start introducing conditionals on existence of child
nodes, and marking the old bindings as deprecated in one large schema,
then that sounds like it would be very messy and hard to read and
maintain. But perhaps there is some way to do this without such
downsides that I'm not aware of.

Johan
  

On 18/10/2022 12:04, Johan Hovold wrote:
> On Tue, Oct 18, 2022 at 11:32:07AM -0400, Krzysztof Kozlowski wrote:
>> On 18/10/2022 07:37, Dmitry Baryshkov wrote:
>>>
>>>>> And yes, I think we should also upgrade
>>>>> older DTs, keeping drivers backwards compatible (for some time?).
>>>>
>>>> Possibly, but I'm not sure it's worth the dts churn. As I mentioned
>>>> elsewhere, supporting both the old and new binding in the driver is
>>>> mostly trivial, while encoding the deprecated bindings in DT schema
>>>> sounds like it would be painful.
>>>
>>> This is probably the time where Krzysztof can advise us. I'm still not
>>> sure when it is expected to encode both old and new bindings in the
>>> schema and when we can update both the schema and the DT.
>>
>> I do not follow what exactly the proposal is. Are you asking whether to:
>> 1. keep existing DTS compatible with old driver?
>> or
>> 2. update existing DTS so it is working only with new driver (and not
>> compatible with old driver thus having ABI break)?
>>
>> If so, it is less question to bindings but more to the usage of DTS in
>> other projects (like bootloaders, firmware, BSD) and generic
>> recommendation is: do not break other users, if possible. It is however
>> up to the platform maintainer (Bjorn) to decide on this, not on me.
> 
> The question is whether to convert also the current bindings and DTS to
> the new (sc8280xp) scheme (e.g. drop the child nodes and register
> subregions).
> 
> The driver can support both binding schemes using the same compatible
> strings for a transition period (or in theory forever) by checking for
> the existence of a child node.
> 
> Converting the DTS to use the new bindings would obviously prevent using
> them with an old kernel (i.e. 2 above), but I don't think that's a
> problem (unlike backward compatibility during at least a transition
> period).

It is still not nice towards any other users of DTS, because this will
break all of them. I agree this won't be ABI type of break. It is
discouraged though, unless there are clear benefits from this or one
totally does not care about other DTS users...

As I said it is up to platform maintainer.

> 
> My concern was how to describe the deprecation in DT schema if we were
> convert them. By instead just keeping the old bindings as-is in a
> separate file and continuing to support them in the driver we can have a
> nice and clean description of the new bindings (sc8280xp) without the
> legacy cruft.

You cannot have one compatible in two schemas, so for old bindings (and
DTS):
1. Don't convert them,
2. Convert with keeping old properties - as you pointed this might be
full of conditionals/allOf, so difficult to maintain and read,
3. Convert dropping old stuff.

For the option 3. for sure Rob will ask why. :)

> 
> If we were to start introducing conditionals on existence of child
> nodes, and marking the old bindings as deprecated in one large schema,
> then that sounds like it would be very messy and hard to read and
> maintain. But perhaps there is some way to do this without such
> downsides that I'm not aware of.


Best regards,
Krzysztof
  

On Tue, Oct 18, 2022 at 12:44:22PM -0400, Krzysztof Kozlowski wrote:
> On 18/10/2022 12:04, Johan Hovold wrote:

> > The question is whether to convert also the current bindings and DTS to
> > the new (sc8280xp) scheme (e.g. drop the child nodes and register
> > subregions).
> > 
> > The driver can support both binding schemes using the same compatible
> > strings for a transition period (or in theory forever) by checking for
> > the existence of a child node.
> > 
> > Converting the DTS to use the new bindings would obviously prevent using
> > them with an old kernel (i.e. 2 above), but I don't think that's a
> > problem (unlike backward compatibility during at least a transition
> > period).
> 
> It is still not nice towards any other users of DTS, because this will
> break all of them. I agree this won't be ABI type of break. It is
> discouraged though, unless there are clear benefits from this or one
> totally does not care about other DTS users...
> 
> As I said it is up to platform maintainer.

Yeah. When time I spoke to Bjorn about this, we agreed to draw the line
at SC8280XP.

But if it turns out converting older platforms is needed to fix bugs or
add features (e.g. due to the incomplete register descriptions), we may
later have to reconsider this.

> > My concern was how to describe the deprecation in DT schema if we were
> > convert them. By instead just keeping the old bindings as-is in a
> > separate file and continuing to support them in the driver we can have a
> > nice and clean description of the new bindings (sc8280xp) without the
> > legacy cruft.
> 
> You cannot have one compatible in two schemas, so for old bindings (and
> DTS):
> 1. Don't convert them,
> 2. Convert with keeping old properties - as you pointed this might be
> full of conditionals/allOf, so difficult to maintain and read,
> 3. Convert dropping old stuff.
> 
> For the option 3. for sure Rob will ask why. :)

Thanks for confirming.

So I guess we start with keeping the old bindings as they are (1) and if
later needed (or desired) we should simply drop the old bindings (3)
from the schema (we can still have the driver support the old bindings
during a transition period).

> > If we were to start introducing conditionals on existence of child
> > nodes, and marking the old bindings as deprecated in one large schema,
> > then that sounds like it would be very messy and hard to read and
> > maintain. But perhaps there is some way to do this without such
> > downsides that I'm not aware of.

Johan