| Message ID | 20220705094239.17174-1-johan+linaro@kernel.org |
|---|---|
| Headers | show |
| Series | phy: qcom,qmp: fix dt-bindings and deprecate lane suffix | expand |
On 05/07/2022 11:42, Johan Hovold wrote: > The pipe clock is defined in the "lane" node so there's no need to keep > adding a redundant lane-number suffix to the clock name. > > Drop the lane suffix from the pipe clock name, but continue supporting > the legacy name as a fall back. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > index 385ea3d8de08..254ad25591b9 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > @@ -2210,8 +2210,12 @@ int qcom_qmp_phy_pcie_create(struct device *dev, struct device_node *np, int id, > if (!qphy->pcs_misc) > dev_vdbg(dev, "PHY pcs_misc-reg not used\n"); > > - snprintf(prop_name, sizeof(prop_name), "pipe%d", id); > - qphy->pipe_clk = devm_get_clk_from_child(dev, np, prop_name); > + qphy->pipe_clk = devm_get_clk_from_child(dev, np, "pipe"); Just get first clock and no need for handling any deprecation. Best regards, Krzysztof
On 05/07/2022 11:42, Johan Hovold wrote: > The pipe clock is defined in the "lane" node so there's no need to keep > adding a redundant lane-number suffix to the clock name. > > Drop the lane suffix from the pipe clock name, but continue supporting > the legacy name as a fall back. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > index b266514ae9ee..7b2eb234cfc2 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > @@ -2558,8 +2558,12 @@ int qcom_qmp_phy_combo_create(struct device *dev, struct device_node *np, int id > * Otherwise, we initialize pipe clock to NULL for > * all phys that don't need this. > */ > - snprintf(prop_name, sizeof(prop_name), "pipe%d", id); > - qphy->pipe_clk = devm_get_clk_from_child(dev, np, prop_name); > + qphy->pipe_clk = devm_get_clk_from_child(dev, np, "pipe"); Just grab first clock. Best regards, Krzysztof
On 05/07/2022 11:42, Johan Hovold wrote: > The lane reset is defined in the "lane" node so there's no need to keep > adding a redundant lane-number suffix to the reset name. > > Drop the lane-number suffix from the lane reset name, but continue > supporting the legacy name as a fall back. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-pcie-msm8996.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie-msm8996.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie-msm8996.c > index b8481dab54db..9ddb42fa5f7a 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie-msm8996.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie-msm8996.c > @@ -915,9 +915,12 @@ int qcom_qmp_phy_pcie_msm8996_create(struct device *dev, struct device_node *np, > "failed to get lane%d pipe clock\n", id); > } > > - /* Get lane reset, if any */ > - snprintf(prop_name, sizeof(prop_name), "lane%d", id); > - qphy->lane_rst = of_reset_control_get_exclusive(np, prop_name); > + qphy->lane_rst = of_reset_control_get_exclusive(np, "lane"); As well, just grab first entry and ABI is kept stable. Best regards, Krzysztof
On 05/07/2022 12:42, Johan Hovold wrote: > Drop the unused lane reset pointer which isn't used by any combo PHY. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 2 -- > 1 file changed, 2 deletions(-)
On 05/07/2022 13:20, Krzysztof Kozlowski wrote: > On 05/07/2022 11:42, Johan Hovold wrote: >> The pipe clock is defined in the "lane" node so there's no need to keep >> adding a redundant lane-number suffix to the clock name. >> >> Drop the lane suffix from the pipe clock name, but continue supporting >> the legacy name as a fall back. >> >> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> >> --- >> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >> index 385ea3d8de08..254ad25591b9 100644 >> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >> @@ -2210,8 +2210,12 @@ int qcom_qmp_phy_pcie_create(struct device *dev, struct device_node *np, int id, >> if (!qphy->pcs_misc) >> dev_vdbg(dev, "PHY pcs_misc-reg not used\n"); >> >> - snprintf(prop_name, sizeof(prop_name), "pipe%d", id); >> - qphy->pipe_clk = devm_get_clk_from_child(dev, np, prop_name); >> + qphy->pipe_clk = devm_get_clk_from_child(dev, np, "pipe"); > > Just get first clock and no need for handling any deprecation. If I got it correctly, passing NULL instead of the name would do the trick.
On Tue, Jul 05, 2022 at 02:13:04PM +0300, Dmitry Baryshkov wrote: > On 05/07/2022 13:20, Krzysztof Kozlowski wrote: > > On 05/07/2022 11:42, Johan Hovold wrote: > >> The pipe clock is defined in the "lane" node so there's no need to keep > >> adding a redundant lane-number suffix to the clock name. > >> > >> Drop the lane suffix from the pipe clock name, but continue supporting > >> the legacy name as a fall back. > >> > >> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > >> --- > >> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 8 ++++++-- > >> 1 file changed, 6 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > >> index 385ea3d8de08..254ad25591b9 100644 > >> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > >> @@ -2210,8 +2210,12 @@ int qcom_qmp_phy_pcie_create(struct device *dev, struct device_node *np, int id, > >> if (!qphy->pcs_misc) > >> dev_vdbg(dev, "PHY pcs_misc-reg not used\n"); > >> > >> - snprintf(prop_name, sizeof(prop_name), "pipe%d", id); > >> - qphy->pipe_clk = devm_get_clk_from_child(dev, np, prop_name); > >> + qphy->pipe_clk = devm_get_clk_from_child(dev, np, "pipe"); > > > > Just get first clock and no need for handling any deprecation. I still want to deprecate the current name as it makes no sense and risks introducing inconsistencies when adding new resources (e.g. should they also get a bogus suffix). > If I got it correctly, passing NULL instead of the name would do the trick. Ah, thanks for spotting that. I feared this would require adding a host of new devres wrappers otherwise. Would still be needed for the upcoming second pipediv2 clock though... Johan
On 05/07/2022 13:58, Johan Hovold wrote: > On Tue, Jul 05, 2022 at 02:13:04PM +0300, Dmitry Baryshkov wrote: >> On 05/07/2022 13:20, Krzysztof Kozlowski wrote: >>> On 05/07/2022 11:42, Johan Hovold wrote: >>>> The pipe clock is defined in the "lane" node so there's no need to keep >>>> adding a redundant lane-number suffix to the clock name. >>>> >>>> Drop the lane suffix from the pipe clock name, but continue supporting >>>> the legacy name as a fall back. >>>> >>>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> >>>> --- >>>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 8 ++++++-- >>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >>>> index 385ea3d8de08..254ad25591b9 100644 >>>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >>>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >>>> @@ -2210,8 +2210,12 @@ int qcom_qmp_phy_pcie_create(struct device *dev, struct device_node *np, int id, >>>> if (!qphy->pcs_misc) >>>> dev_vdbg(dev, "PHY pcs_misc-reg not used\n"); >>>> >>>> - snprintf(prop_name, sizeof(prop_name), "pipe%d", id); >>>> - qphy->pipe_clk = devm_get_clk_from_child(dev, np, prop_name); >>>> + qphy->pipe_clk = devm_get_clk_from_child(dev, np, "pipe"); >>> >>> Just get first clock and no need for handling any deprecation. > > I still want to deprecate the current name as it makes no sense and > risks introducing inconsistencies when adding new resources (e.g. should > they also get a bogus suffix). And it was suggested to you to deprecate entire property... There is no need to handle anything in the driver. > >> If I got it correctly, passing NULL instead of the name would do the trick. > > Ah, thanks for spotting that. I feared this would require adding a host > of new devres wrappers otherwise. > > Would still be needed for the upcoming second pipediv2 clock though... Best regards, Krzysztof
When adding support for SC8280XP to the QMP PHY driver I noticed that the PHY provider child node was not described by the current DT schema. The SC8280XP PHYs also need a second fixed-divider PIPE clock ("pipediv2") and I didn't want to have to add a bogus "lane" suffix to the clock name just to match the current "pipe0" name so I decided to deprecate the unnecessary suffix in the current binding instead. To be able to add the missing child-node schema and handle device specifics like additional PIPE clocks, it quickly became obvious that the binding needs to be split up. This series fixes some issue with the current schema before splitting it up in separate schemas for PCIe, UFS and USB, which are then cleaned up further before adding missing parts like the child PHY provider nodes and examples. The MSM8996 PCIe PHY gets its own schema as this is the only non-combo PHY that actually provides more than one PHY per IP block. Note that the "lane" suffix is still unnecessary and misleading. The final patches adds support for the new resource names to the (recently split up) PHY drivers. Included is also a related combo PHY cleanup. I'll post a separate series with related DTS fixes caught while developing this series. Johan Johan Hovold (43): dt-bindings: phy: qcom,qmp: fix bogus clock-cells property dt-bindings: phy: qcom,qmp: sort compatible strings dt-bindings: phy: qcom,qmp: clean up descriptions dt-bindings: phy: qcom,qmp: clean up example dt-bindings: phy: qcom,qmp: drop child-node comment dt-bindings: phy: qcom,qmp: split out msm8996-qmp-pcie-phy dt-bindings: phy: qcom,msm8996-qmp-pcie: clean up constraints dt-bindings: phy: qcom,msm8996-qmp-pcie: add missing child node schema dt-bindings: phy: qcom,msm8996-qmp-pcie: add example node dt-bindings: phy: qcom,msm8996-qmp-pcie: deprecate PIPE clock names dt-bindings: phy: qcom,msm8996-qmp-pcie: deprecate reset names dt-bindings: phy: qcom,qmp: split out PCIe PHY binding dt-bindings: phy: qcom,qmp-pcie: clean up register constraints dt-bindings: phy: qcom,qmp-pcie: clean up clock constraints dt-bindings: phy: qcom,qmp-pcie: clean up reset constraints dt-bindings: phy: qcom,qmp-pcie: drop unused vddp-ref-clk supply dt-bindings: phy: qcom,qmp-pcie: add missing child node schema dt-bindings: phy: qcom,qmp-pcie: add example node dt-bindings: phy: qcom,qmp-pcie: deprecate PIPE clock name dt-bindings: phy: qcom,qmp: split out UFS PHY binding dt-bindings: phy: qcom,qmp-ufs: clean up supplies dt-bindings: phy: qcom,qmp-ufs: clean up reset providers dt-bindings: phy: qcom,qmp-ufs: clean up clock constraints dt-bindings: phy: qcom,qmp-ufs: clean up register constraints dt-bindings: phy: qcom,qmp-ufs: add missing SM8450 clock dt-bindings: phy: qcom,qmp-ufs: add missing SM8150 power domain dt-bindings: phy: qcom,qmp-ufs: add missing child node schema dt-bindings: phy: qcom,qmp-ufs: add example node dt-bindings: phy: qcom,qmp: split out USB binding dt-bindings: phy: qcom,qmp-usb: clean up clock constraints dt-bindings: phy: qcom,qmp-usb: clean up supplies dt-bindings: phy: qcom,qmp-usb: drop unused vddp-ref-clk supply dt-bindings: phy: qcom,qmp-usb: clean up reset providers dt-bindings: phy: qcom,qmp-usb: add missing child node schema dt-bindings: phy: qcom,qmp-usb: deprecate PIPE clock name dt-bindings: phy: qcom,qmp-usb3-dp: fix bogus clock-cells property dt-bindings: phy: qcom,qmp-usb3-dp: deprecate USB PIPE clock name phy: qcom-qmp-pcie: drop pipe clock lane suffix phy: qcom-qmp-combo: drop unused lane reset phy: qcom-qmp-combo: drop pipe clock lane suffix phy: qcom-qmp-pcie-msm8996: drop pipe clock lane suffix phy: qcom-qmp-pcie-msm8996: drop reset lane suffix phy: qcom-qmp-usb: drop pipe clock lane suffix .../phy/qcom,msm8996-qmp-pcie-phy.yaml | 212 ++++++++ .../bindings/phy/qcom,qmp-pcie-phy.yaml | 309 +++++++++++ .../devicetree/bindings/phy/qcom,qmp-phy.yaml | 500 ------------------ .../bindings/phy/qcom,qmp-ufs-phy.yaml | 249 +++++++++ .../bindings/phy/qcom,qmp-usb-phy.yaml | 417 +++++++++++++++ .../bindings/phy/qcom,qmp-usb3-dp-phy.yaml | 15 +- drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 10 +- .../phy/qualcomm/phy-qcom-qmp-pcie-msm8996.c | 17 +- drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 8 +- drivers/phy/qualcomm/phy-qcom-qmp-usb.c | 8 +- 10 files changed, 1224 insertions(+), 521 deletions(-) create mode 100644 Documentation/devicetree/bindings/phy/qcom,msm8996-qmp-pcie-phy.yaml create mode 100644 Documentation/devicetree/bindings/phy/qcom,qmp-pcie-phy.yaml delete mode 100644 Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml create mode 100644 Documentation/devicetree/bindings/phy/qcom,qmp-ufs-phy.yaml create mode 100644 Documentation/devicetree/bindings/phy/qcom,qmp-usb-phy.yaml