| Message ID | 20221017145328.22090-10-johan+linaro@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | phy: qcom-qmp-pcie: add support for sc8280xp | expand |
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
diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qmp-pcie-phy-legacy.yaml similarity index 98% rename from Documentation/devicetree/bindings/phy/qcom,qmp-pcie-phy.yaml rename to Documentation/devicetree/bindings/phy/qcom,qmp-pcie-phy-legacy.yaml index 324ad7d03a38..2ea880f8c099 100644 --- a/Documentation/devicetree/bindings/phy/qcom,qmp-pcie-phy.yaml +++ b/Documentation/devicetree/bindings/phy/qcom,qmp-pcie-phy-legacy.yaml @@ -1,10 +1,10 @@ # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 --- -$id: http://devicetree.org/schemas/phy/qcom,qmp-pcie-phy.yaml# +$id: http://devicetree.org/schemas/phy/qcom,qmp-pcie-phy-legacy.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: Qualcomm QMP PHY controller (PCIe) +title: Qualcomm QMP PHY controller (PCIe legacy bindings) maintainers: - Vinod Koul <vkoul@kernel.org>
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%)