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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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