clk: qcom: add support for SM8350 DISPCC
PATCH v2 1/2 clk: qcom: add support for SM8350 DISPCC
Add support to the SM8350 display clock controller by extending the SM8250 display clock controller, which is almost identical but has some minor differences.
---
v2: improved diff (don't move sm8150 case around), update comment
drivers/clk/qcom/Kconfig | 4 +-- drivers/clk/qcom/dispcc-sm8250.c | 61 +++++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 3 deletions(-)
drivers/clk/qcom/Kconfig +2 -2
@@ -484,11 +484,11 @@ config SDX_GCC_55 SPI, I2C, USB, SD/UFS, PCIe etc. config SM_DISPCC_8250 - tristate "SM8150 and SM8250 Display Clock Controller" + tristate "SM8150/SM8250/SM8350 Display Clock Controller" depends on SM_GCC_8150 || SM_GCC_8250 help Support for the display clock controller on Qualcomm Technologies, Inc - SM8150 and SM8250 devices. + SM8150/SM8250/SM8350 devices. Say Y if you want to support display devices and functionality such as splash screen.
drivers/clk/qcom/dispcc-sm8250.c +60 -1
87 lines not on this page · raw message
Signature · 1 line
2.26.1
PATCH v2 2/2 dt-bindings: clock: add QCOM SM8350 display clock bindings
Add sm8350 DISPCC bindings, which are simply a symlink to the sm8250 bindings. Update the documentation with the new compatible.
--- .../devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml | 6 ++++-- include/dt-bindings/clock/qcom,dispcc-sm8350.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) create mode 120000 include/dt-bindings/clock/qcom,dispcc-sm8350.h
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml +4 -2
@@ -4,24 +4,26 @@ $id: http://devicetree.org/schemas/clock/qcom,dispcc-sm8x50.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250 +title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250/SM8350 maintainers: - Jonathan Marek <[email protected]> description: | Qualcomm display clock control module which supports the clocks, resets and - power domains on SM8150 and SM8250. + power domains on SM8150/SM8250/SM8350. See also: dt-bindings/clock/qcom,dispcc-sm8150.h dt-bindings/clock/qcom,dispcc-sm8250.h + dt-bindings/clock/qcom,dispcc-sm8350.h properties: compatible: enum: - qcom,sm8150-dispcc - qcom,sm8250-dispcc + - qcom,sm8350-dispcc clocks: items:
include/dt-bindings/clock/qcom,dispcc-sm8350.h new file +1 -0
@@ -0,0 +1 @@ +qcom,dispcc-sm8250.h
Signature · 1 line
2.26.1
Stephen Boyd
maintainerRe: [PATCH v2 2/2] dt-bindings: clock: add QCOM SM8350 display clock bindings
Quoting Jonathan Marek (2021-05-18 17:18:02)
Quoted 11 lines
Add sm8350 DISPCC bindings, which are simply a symlink to the sm8250 bindings. Update the documentation with the new compatible.
Signed-off-by Jonathan Marek <[email protected]>Reviewed-by Rob Herring <[email protected]>--- .../devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml | 6 ++++-- include/dt-bindings/clock/qcom,dispcc-sm8350.h | 1 +2 files changed, 5 insertions(+), 2 deletions(-) create mode 120000 include/dt-bindings/clock/qcom,dispcc-sm8350.h
Why the symlink? Can we have the dt authors use the existing header file instead?
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml line 7
Quoted 11 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml +1
-1@@ -4,24 +4,26 @@ $id: http://devicetree.org/schemas/clock/qcom,dispcc-sm8x50.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250 +title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250/SM8350
Maybe just "Binding for SM8x50 SoCs"
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml line 14
Quoted 8 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml +1
-1maintainers: - Jonathan Marek <[email protected]> description: | Qualcomm display clock control module which supports the clocks, resets and - power domains on SM8150 and SM8250. + power domains on SM8150/SM8250/SM8350.
same 8x50 comment.
Quoted 23 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml +2
-0See also: dt-bindings/clock/qcom,dispcc-sm8150.h dt-bindings/clock/qcom,dispcc-sm8250.h + dt-bindings/clock/qcom,dispcc-sm8350.h properties: compatible: enum: - qcom,sm8150-dispcc - qcom,sm8250-dispcc + - qcom,sm8350-dispcc clocks: items:include/dt-bindings/clock/qcom,dispcc-sm8350.h new file +1
-0@@ -0,0 +1 @@ +qcom,dispcc-sm8250.h
Re: [PATCH v2 2/2] dt-bindings: clock: add QCOM SM8350 display clock bindings
Quoted 10 lines · Stephen Boyd
Quoting Jonathan Marek (2021-05-18 17:18:02)
Quoted 8 lines
Add sm8350 DISPCC bindings, which are simply a symlink to the sm8250 bindings. Update the documentation with the new compatible.
Signed-off-by Jonathan Marek <[email protected]>Reviewed-by Rob Herring <[email protected]>--- .../devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml | 6 ++++-- include/dt-bindings/clock/qcom,dispcc-sm8350.h | 1 +
Quoted 6 lines
Quoted 2 lines
2 files changed, 5 insertions(+), 2 deletions(-)create mode 120000
include/dt-bindings/clock/qcom,dispcc-sm8350.hWhy the symlink? Can we have the dt authors use the existing header file instead?
It would be strange to include bindings with the name of a different SoC. I guess it is a matter a preference, is there any good reason to *not* do it like this?
Quoted 9 lines
Quoted 9 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml +0
-0@@ -4,24 +4,26 @@ $id: http://devicetree.org/schemas/clock/qcom,dispcc-sm8x50.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml#
Quoted 5 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml line 7
Quoted 2 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml +1
-1-title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250 +title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250/SM8350Maybe just "Binding for SM8x50 SoCs"
Its likely these bindings won't be compatible with future "SM8x50" SoCs, listing supported SoCs explicitly will avoid confusion in the future.
Quoted 34 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml line 14
Quoted 8 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml +1
-1maintainers: - Jonathan Marek <[email protected]> description: | Qualcomm display clock control module which supports the clocks, resets and - power domains on SM8150 and SM8250. + power domains on SM8150/SM8250/SM8350.same 8x50 comment.
Quoted 23 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml +2
-0See also: dt-bindings/clock/qcom,dispcc-sm8150.h dt-bindings/clock/qcom,dispcc-sm8250.h + dt-bindings/clock/qcom,dispcc-sm8350.h properties: compatible: enum: - qcom,sm8150-dispcc - qcom,sm8250-dispcc + - qcom,sm8350-dispcc clocks: items:include/dt-bindings/clock/qcom,dispcc-sm8350.h new file +1
-0@@ -0,0 +1 @@ +qcom,dispcc-sm8250.h
Stephen Boyd
maintainerRe: [PATCH v2 2/2] dt-bindings: clock: add QCOM SM8350 display clock bindings
Quoting Jonathan Marek (2021-06-04 10:25:41)
Quoted 14 lines · Stephen Boyd
Quoted 13 lines
Quoting Jonathan Marek (2021-05-18 17:18:02)
Quoted 11 lines
Add sm8350 DISPCC bindings, which are simply a symlink to the sm8250 bindings. Update the documentation with the new compatible.
Signed-off-by Jonathan Marek <[email protected]>Reviewed-by Rob Herring <[email protected]>--- .../devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml | 6 ++++-- include/dt-bindings/clock/qcom,dispcc-sm8350.h | 1 +2 files changed, 5 insertions(+), 2 deletions(-)create mode 120000
include/dt-bindings/clock/qcom,dispcc-sm8350.h
Quoted 7 lines
Quoted 3 lines
Why the symlink? Can we have the dt authors use the existing header file instead?
It would be strange to include bindings with the name of a different SoC. I guess it is a matter a preference, is there any good reason to *not* do it like this?
$ find include/dt-bindings -type l include/dt-bindings/input/linux-event-codes.h include/dt-bindings/clock/qcom,dispcc-sm8150.h
It seems to not be common at all.
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml +0
-0@@ -4,24 +4,26 @@ $id: http://devicetree.org/schemas/clock/qcom,dispcc-sm8x50.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml#
Quoted 8 lines
Quoted 5 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml line 7
Quoted 2 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml +1
-1-title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250 +title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250/SM8350Maybe just "Binding for SM8x50 SoCs"
Its likely these bindings won't be compatible with future "SM8x50" SoCs, listing supported SoCs explicitly will avoid confusion in the future.
The yaml file has sm8x50 in the name. What's the plan there?
Re: [PATCH v2 2/2] dt-bindings: clock: add QCOM SM8350 display clock bindings
In reply to Stephen BoydQuoted 25 lines · Stephen Boyd
Quoting Jonathan Marek (2021-05-18 17:18:02)
Quoted 11 lines
Add sm8350 DISPCC bindings, which are simply a symlink to the sm8250 bindings. Update the documentation with the new compatible.
Signed-off-by Jonathan Marek <[email protected]>Reviewed-by Rob Herring <[email protected]>--- .../devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml | 6 ++++-- include/dt-bindings/clock/qcom,dispcc-sm8350.h | 1 +2 files changed, 5 insertions(+), 2 deletions(-) create mode 120000 include/dt-bindings/clock/qcom,dispcc-sm8350.hWhy the symlink? Can we have the dt authors use the existing header file instead?
Quoted 9 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml +0
-0@@ -4,24 +4,26 @@ $id: http://devicetree.org/schemas/clock/qcom,dispcc-sm8x50.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml#
Quoted 5 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml line 7
Quoted 2 lines
Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml +1
-1-title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250 +title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250/SM8350Maybe just "Binding for SM8x50 SoCs"
That seems like a certain way to ensure that SM8450 etc will be different enough to not match this binding :)
Regards,
Bjorn
Stephen Boyd
maintainerQuoting Jonathan Marek (2021-05-18 17:18:01)
drivers/clk/qcom/dispcc-sm8250.c static struct pll_vco vco_table[] = { · line 39
Quoted 9 lines
drivers/clk/qcom/dispcc-sm8250.c +1
-0@@ -36,6 +36,10 @@ static struct pll_vco vco_table[] = { { 249600000, 2000000000, 0 }, }; +static struct pll_vco lucid_5lpe_vco[] = {
const
Quoted 24 lines
drivers/clk/qcom/dispcc-sm8250.c +5
-1+ { 249600000, 1750000000, 0 }, +}; +static struct alpha_pll_config disp_cc_pll0_config = { .l =0x47 , .alpha = 0xE000,@@ -1039,6 +1043,7 @@ static const struct qcom_cc_desc disp_cc_sm8250_desc = {static const struct of_device_id disp_cc_sm8250_match_table[] = { { .compatible ="qcom,sm8150-dispcc" }, { .compatible ="qcom,sm8250-dispcc" }, + { .compatible ="qcom,sm8350-dispcc" }, { } };MODULE_DEVICE_TABLE (of, disp_cc_sm8250_match_table);@@ -1051,7 +1056,7 @@ static int disp_cc_sm8250_probe(struct platform_device *pdev)if (IS_ERR (regmap))return PTR_ERR (regmap); -/* note: trion == lucid, except for the prepare() op */ +/* Apply differences for SM8150 and SM8350 */ BUILD_BUG_ON (CLK_ALPHA_PLL_TYPE_TRION !=CLK_ALPHA_PLL_TYPE_LUCID );if (of_device_is_compatible (pdev->dev.of_node,"qcom,sm8150-dispcc" )) { disp_cc_pll0_config.config_ctl_hi_val =0x00002267 ;
drivers/clk/qcom/dispcc-sm8250.c static int disp_cc_sm8250_probe(struct platform_device *pdev) · line 1071
Quoted 5 lines
drivers/clk/qcom/dispcc-sm8250.c +2
-0@@ -1062,8 +1067,62 @@ static int disp_cc_sm8250_probe(struct platform_device *pdev) disp_cc_pll1_config.config_ctl_hi1_val =0x00000024 ; disp_cc_pll1_config.user_ctl_hi1_val =0x000000D0 ; disp_cc_pll1_init.ops = &clk_alpha_pll_trion_ops; + }else if (of_device_is_compatible (pdev->dev.of_node,"qcom,sm8350-dispcc" )) { + static struct clk_rcg2* const rcgs[] = {
Please move this to global scope and give it a name.
Quoted 7 lines
drivers/clk/qcom/dispcc-sm8250.c +7
-0+ &disp_cc_mdss_byte0_clk_src, + &disp_cc_mdss_byte1_clk_src, + &disp_cc_mdss_dp_aux1_clk_src, + &disp_cc_mdss_dp_aux_clk_src, + &disp_cc_mdss_dp_link1_clk_src, + &disp_cc_mdss_dp_link_clk_src, + &disp_cc_mdss_dp_pixel1_clk_src,
drivers/clk/qcom/dispcc-sm8250.c static int disp_cc_sm8250_probe(struct platform_device *pdev) · line 1088
Quoted 10 lines
drivers/clk/qcom/dispcc-sm8250.c +10
-0+ &disp_cc_mdss_dp_pixel2_clk_src, + &disp_cc_mdss_dp_pixel_clk_src, + &disp_cc_mdss_esc0_clk_src, + &disp_cc_mdss_mdp_clk_src, + &disp_cc_mdss_pclk0_clk_src, + &disp_cc_mdss_pclk1_clk_src, + &disp_cc_mdss_rot_clk_src, + &disp_cc_mdss_vsync_clk_src, + }; + static struct clk_regmap_div* const divs[] = {
Move global.
drivers/clk/qcom/dispcc-sm8250.c static int disp_cc_sm8250_probe(struct platform_device *pdev) · line 1094
Quoted 6 lines
drivers/clk/qcom/dispcc-sm8250.c +6
-0+ &disp_cc_mdss_byte0_div_clk_src, + &disp_cc_mdss_byte1_div_clk_src, + &disp_cc_mdss_dp_link1_div_clk_src, + &disp_cc_mdss_dp_link_div_clk_src, + }; +unsigned i;
int i? I doubt being unsigned helps.
drivers/clk/qcom/dispcc-sm8250.c static int disp_cc_sm8250_probe(struct platform_device *pdev) · line 1098
Quoted 4 lines
drivers/clk/qcom/dispcc-sm8250.c +4
-0+static bool offset_applied = false; + +/* only apply the offsets once (in case of deferred probe) */ +if (!offset_applied) {
Maybe instead of doing this in probe it can be done when the driver is added in module init? It would mean searching the DT for the compatible string and then if it is present running the subtraction code, but at least we would only do it once and the code would be thrown away after init.
Quoted 11 lines
drivers/clk/qcom/dispcc-sm8250.c +11
-0+for (i =0 ; i <ARRAY_SIZE (rcgs); i++) + rcgs[i]->cmd_rcgr -=4 ; + +for (i =0 ; i <ARRAY_SIZE (divs); i++) { + divs[i]->reg -=4 ; + divs[i]->width =4 ; + } + + disp_cc_mdss_ahb_clk.halt_reg -=4 ; + disp_cc_mdss_ahb_clk.clkr.enable_reg -=4 ; +
drivers/clk/qcom/dispcc-sm8250.c static int disp_cc_sm8250_probe(struct platform_device *pdev) · line 1119
Quoted 10 lines
drivers/clk/qcom/dispcc-sm8250.c +10
-0+ offset_applied = true; + } + + disp_cc_mdss_ahb_clk_src.cmd_rcgr =0x22a0 ; + + disp_cc_pll0_config.config_ctl_hi1_val =0x2A9A699C ; + disp_cc_pll0_config.test_ctl_hi1_val =0x01800000 ; + disp_cc_pll0_init.ops = &clk_alpha_pll_lucid_5lpe_ops; + disp_cc_pll0.vco_table = lucid_5lpe_vco; + disp_cc_pll1_config.config_ctl_hi1_val =0x2A9A699C ;
Lowercase hex please.
drivers/clk/qcom/dispcc-sm8250.c static int disp_cc_sm8250_probe(struct platform_device *pdev) · line 1125
Quoted 6 lines
drivers/clk/qcom/dispcc-sm8250.c +4
-0+ disp_cc_pll1_config.test_ctl_hi1_val =0x01800000 ; + disp_cc_pll1_init.ops = &clk_alpha_pll_lucid_5lpe_ops; + disp_cc_pll1.vco_table = lucid_5lpe_vco; } +/* note for SM8350: downstream lucid_5lpe configure differs slightly */
Is this a TODO?
Quoted 3 lines
drivers/clk/qcom/dispcc-sm8250.c +0
-0clk_lucid_pll_configure (&disp_cc_pll0, regmap, &disp_cc_pll0_config);clk_lucid_pll_configure (&disp_cc_pll1, regmap, &disp_cc_pll1_config);