| Message ID | 20220705094239.17174-39-johan+linaro@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| 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 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
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"); + if (IS_ERR(qphy->pipe_clk) && PTR_ERR(qphy->pipe_clk) != -EPROBE_DEFER) { + /* Fall back to the legacy pipe clock name. */ + snprintf(prop_name, sizeof(prop_name), "pipe%d", id); + qphy->pipe_clk = devm_get_clk_from_child(dev, np, prop_name); + } if (IS_ERR(qphy->pipe_clk)) { return dev_err_probe(dev, PTR_ERR(qphy->pipe_clk), "failed to get lane%d pipe clock\n", id);
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(-)