| Message ID | 20210829203027.276143-3-marijn.suijten@somainline.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] arm: dts: qcom: apq8064: Use 27MHz PXO clock as DSI PLL reference | expand |
Hi Dmitry, On 8/29/21 10:39 PM, Dmitry Baryshkov wrote: > Hi, > > On Sun, 29 Aug 2021 at 23:30, Marijn Suijten > <marijn.suijten@somainline.org> wrote: >> >> All DSI PHY/PLL drivers were referencing their VCO parent clock by a >> global name, most of which don't exist or have been renamed. These >> clock drivers seem to function fine without that except the 14nm driver >> for the sdm6xx [1]. >> >> At the same time all DTs provide a "ref" clock as per the requirements >> of dsi-phy-common.yaml, but the clock is never used. This patchset puts >> that clock to use without relying on a global clock name, so that all >> dependencies are explicitly defined in DT (the firmware) in the end. > > msm8974 (28nm-hpm) does not define the "ref" clock. So you'd have to: > 1) add ref clock to the dtsi (should come in a separate patch). Thanks for double-checking and noticing this! I've queued up this patch for v2. > 2) add .name = "xo" as a fallback to the 28nm driver (to be compatible > with older devices) Are there msm8974 devices out there that might upgrade kernels, but not firmware (DT)? On other boards (sdm630) I'm removing these from various drivers as to not have any possibility of relying on global names, in favour of having the clock dependencies fully specified in the DT. > Other than that this looks good to me. Any r-b/a-b/t-b I can pick up for the next round? - Marijn
On Mon, 30 Aug 2021 at 00:53, Marijn Suijten <marijn.suijten@somainline.org> wrote: > > Hi Dmitry, > > On 8/29/21 10:39 PM, Dmitry Baryshkov wrote: > > Hi, > > > > On Sun, 29 Aug 2021 at 23:30, Marijn Suijten > > <marijn.suijten@somainline.org> wrote: > >> > >> All DSI PHY/PLL drivers were referencing their VCO parent clock by a > >> global name, most of which don't exist or have been renamed. These > >> clock drivers seem to function fine without that except the 14nm driver > >> for the sdm6xx [1]. > >> > >> At the same time all DTs provide a "ref" clock as per the requirements > >> of dsi-phy-common.yaml, but the clock is never used. This patchset puts > >> that clock to use without relying on a global clock name, so that all > >> dependencies are explicitly defined in DT (the firmware) in the end. > > > > msm8974 (28nm-hpm) does not define the "ref" clock. So you'd have to: > > 1) add ref clock to the dtsi (should come in a separate patch). > > > Thanks for double-checking and noticing this! I've queued up this patch > for v2. > > > 2) add .name = "xo" as a fallback to the 28nm driver (to be compatible > > with older devices) > > > Are there msm8974 devices out there that might upgrade kernels, but not > firmware (DT)? On other boards (sdm630) I'm removing these from various > drivers as to not have any possibility of relying on global names, in > favour of having the clock dependencies fully specified in the DT. IIUC it is a general policy of trying to be (somewhat) backwards-compatible. For example because your dts might come from a different source/be a part of different build process/etc. > > > Other than that this looks good to me. > > > Any r-b/a-b/t-b I can pick up for the next round? Let's get those issues fixed and I'll respond with R-B tags -- With best wishes Dmitry
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c index e46b10fc793a..3cbb1f1475e8 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c @@ -562,7 +562,9 @@ static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm, struct clk_hw **prov char clk_name[32], parent[32], vco_name[32]; char parent2[32], parent3[32], parent4[32]; struct clk_init_data vco_init = { - .parent_names = (const char *[]){ "xo" }, + .parent_data = &(const struct clk_parent_data) { + .fw_name = "ref", + }, .num_parents = 1, .name = vco_name, .flags = CLK_IGNORE_UNUSED, diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c index bb31230721bd..406470265408 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c @@ -804,7 +804,9 @@ static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **prov { char clk_name[32], parent[32], vco_name[32]; struct clk_init_data vco_init = { - .parent_names = (const char *[]){ "xo" }, + .parent_data = &(const struct clk_parent_data) { + .fw_name = "ref", + }, .num_parents = 1, .name = vco_name, .flags = CLK_IGNORE_UNUSED, diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c index 2da673a2add6..8ee9c9c0548d 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c @@ -521,7 +521,9 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov { char clk_name[32], parent1[32], parent2[32], vco_name[32]; struct clk_init_data vco_init = { - .parent_names = (const char *[]){ "xo" }, + .parent_data = &(const struct clk_parent_data) { + .fw_name = "ref", + }, .num_parents = 1, .name = vco_name, .flags = CLK_IGNORE_UNUSED, diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c index aaa37456f4ee..9662cb236468 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c @@ -385,7 +385,9 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov { char *clk_name, *parent_name, *vco_name; struct clk_init_data vco_init = { - .parent_names = (const char *[]){ "pxo" }, + .parent_data = &(const struct clk_parent_data) { + .fw_name = "ref", + }, .num_parents = 1, .flags = CLK_IGNORE_UNUSED, .ops = &clk_ops_dsi_pll_28nm_vco, diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c index 7c23d4c47338..c77c30628cca 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c @@ -590,7 +590,9 @@ static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provide char clk_name[32], parent[32], vco_name[32]; char parent2[32], parent3[32], parent4[32]; struct clk_init_data vco_init = { - .parent_names = (const char *[]){ "bi_tcxo" }, + .parent_data = &(const struct clk_parent_data) { + .fw_name = "ref", + }, .num_parents = 1, .name = vco_name, .flags = CLK_IGNORE_UNUSED,
All DSI PHY/PLL drivers were referencing their VCO parent clock by a global name, most of which don't exist or have been renamed. These clock drivers seem to function fine without that except the 14nm driver for the sdm6xx [1]. At the same time all DTs provide a "ref" clock as per the requirements of dsi-phy-common.yaml, but the clock is never used. This patchset puts that clock to use without relying on a global clock name, so that all dependencies are explicitly defined in DT (the firmware) in the end. [1]: https://lore.kernel.org/linux-arm-msm/386db1a6-a1cd-3c7d-a88e-dc83f8a1be96@somainline.org/ Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> --- drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 4 +++- drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 4 +++- drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 4 +++- drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c | 4 +++- drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 4 +++- 5 files changed, 15 insertions(+), 5 deletions(-)