linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] clk: qcom: add support for SM8350 DISPCC
@ 2021-05-19  0:18 Jonathan Marek
  2021-05-19  0:18 ` [PATCH v2 2/2] dt-bindings: clock: add QCOM SM8350 display clock bindings Jonathan Marek
  2021-06-02 21:34 ` [PATCH v2 1/2] clk: qcom: add support for SM8350 DISPCC Stephen Boyd
  0 siblings, 2 replies; 7+ messages in thread
From: Jonathan Marek @ 2021-05-19  0:18 UTC (permalink / raw)
  To: linux-arm-msm
  Cc: Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
	open list:COMMON CLK FRAMEWORK, open list

Add support to the SM8350 display clock controller by extending the SM8250
display clock controller, which is almost identical but has some minor
differences.

Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
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(-)

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 45646b867cdb..cc60e6ee1654 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -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.
 
diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
index de09cd5c209f..5f22a715e2f0 100644
--- a/drivers/clk/qcom/dispcc-sm8250.c
+++ b/drivers/clk/qcom/dispcc-sm8250.c
@@ -36,6 +36,10 @@ static struct pll_vco vco_table[] = {
 	{ 249600000, 2000000000, 0 },
 };
 
+static struct pll_vco lucid_5lpe_vco[] = {
+	{ 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;
@@ -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[] = {
+			&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,
+			&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[] = {
+			&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;
+		static bool offset_applied = false;
+
+		/* only apply the offsets once (in case of deferred probe) */
+		if (!offset_applied) {
+			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;
+
+			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;
+		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 */
 	clk_lucid_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);
 	clk_lucid_pll_configure(&disp_cc_pll1, regmap, &disp_cc_pll1_config);
 
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/2] dt-bindings: clock: add QCOM SM8350 display clock bindings
  2021-05-19  0:18 [PATCH v2 1/2] clk: qcom: add support for SM8350 DISPCC Jonathan Marek
@ 2021-05-19  0:18 ` Jonathan Marek
  2021-06-02 21:27   ` Stephen Boyd
  2021-06-02 21:34 ` [PATCH v2 1/2] clk: qcom: add support for SM8350 DISPCC Stephen Boyd
  1 sibling, 1 reply; 7+ messages in thread
From: Jonathan Marek @ 2021-05-19  0:18 UTC (permalink / raw)
  To: linux-arm-msm
  Cc: Rob Herring, Andy Gross, Bjorn Andersson, Michael Turquette,
	Stephen Boyd, Rob Herring, open list:COMMON CLK FRAMEWORK,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

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 <jonathan@marek.ca>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../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

diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
index 0cdf53f41f84..8f414642445e 100644
--- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
@@ -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 <jonathan@marek.ca>
 
 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:
diff --git a/include/dt-bindings/clock/qcom,dispcc-sm8350.h b/include/dt-bindings/clock/qcom,dispcc-sm8350.h
new file mode 120000
index 000000000000..0312b4544acb
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,dispcc-sm8350.h
@@ -0,0 +1 @@
+qcom,dispcc-sm8250.h
\ No newline at end of file
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: clock: add QCOM SM8350 display clock bindings
  2021-05-19  0:18 ` [PATCH v2 2/2] dt-bindings: clock: add QCOM SM8350 display clock bindings Jonathan Marek
@ 2021-06-02 21:27   ` Stephen Boyd
  2021-06-04 17:25     ` Jonathan Marek
  2021-06-06  4:11     ` Bjorn Andersson
  0 siblings, 2 replies; 7+ messages in thread
From: Stephen Boyd @ 2021-06-02 21:27 UTC (permalink / raw)
  To: Jonathan Marek, linux-arm-msm
  Cc: Rob Herring, Andy Gross, Bjorn Andersson, Michael Turquette,
	Rob Herring, linux-clk, devicetree, linux-kernel

Quoting Jonathan Marek (2021-05-18 17:18:02)
> 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 <jonathan@marek.ca>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  .../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?

> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
> index 0cdf53f41f84..8f414642445e 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
> @@ -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"

>  
>  maintainers:
>    - Jonathan Marek <jonathan@marek.ca>
>  
>  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.

>  
>    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:
> diff --git a/include/dt-bindings/clock/qcom,dispcc-sm8350.h b/include/dt-bindings/clock/qcom,dispcc-sm8350.h
> new file mode 120000
> index 000000000000..0312b4544acb
> --- /dev/null
> +++ b/include/dt-bindings/clock/qcom,dispcc-sm8350.h
> @@ -0,0 +1 @@
> +qcom,dispcc-sm8250.h
> \ No newline at end of file

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] clk: qcom: add support for SM8350 DISPCC
  2021-05-19  0:18 [PATCH v2 1/2] clk: qcom: add support for SM8350 DISPCC Jonathan Marek
  2021-05-19  0:18 ` [PATCH v2 2/2] dt-bindings: clock: add QCOM SM8350 display clock bindings Jonathan Marek
@ 2021-06-02 21:34 ` Stephen Boyd
  1 sibling, 0 replies; 7+ messages in thread
From: Stephen Boyd @ 2021-06-02 21:34 UTC (permalink / raw)
  To: Jonathan Marek, linux-arm-msm
  Cc: Andy Gross, Bjorn Andersson, Michael Turquette, linux-clk, linux-kernel

Quoting Jonathan Marek (2021-05-18 17:18:01)
> diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
> index de09cd5c209f..5f22a715e2f0 100644
> --- a/drivers/clk/qcom/dispcc-sm8250.c
> +++ b/drivers/clk/qcom/dispcc-sm8250.c
> @@ -36,6 +36,10 @@ static struct pll_vco vco_table[] = {
>         { 249600000, 2000000000, 0 },
>  };
>  
> +static struct pll_vco lucid_5lpe_vco[] = {

const

> +       { 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;
> @@ -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.

> +                       &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,
> +                       &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.

> +                       &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.

> +               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.

> +                       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;
> +
> +                       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.

> +               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?

>         clk_lucid_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);
>         clk_lucid_pll_configure(&disp_cc_pll1, regmap, &disp_cc_pll1_config);
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: clock: add QCOM SM8350 display clock bindings
  2021-06-02 21:27   ` Stephen Boyd
@ 2021-06-04 17:25     ` Jonathan Marek
  2021-06-28  2:39       ` Stephen Boyd
  2021-06-06  4:11     ` Bjorn Andersson
  1 sibling, 1 reply; 7+ messages in thread
From: Jonathan Marek @ 2021-06-04 17:25 UTC (permalink / raw)
  To: Stephen Boyd, linux-arm-msm
  Cc: Rob Herring, Andy Gross, Bjorn Andersson, Michael Turquette,
	Rob Herring, linux-clk, devicetree, linux-kernel

On 6/2/21 5:27 PM, Stephen Boyd wrote:
> Quoting Jonathan Marek (2021-05-18 17:18:02)
>> 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 <jonathan@marek.ca>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> ---
>>   .../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?
> 

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?

>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
>> index 0cdf53f41f84..8f414642445e 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
>> @@ -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"
> 

Its likely these bindings won't be compatible with future "SM8x50" SoCs, 
listing supported SoCs explicitly will avoid confusion in the future.

>>   
>>   maintainers:
>>     - Jonathan Marek <jonathan@marek.ca>
>>   
>>   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.
> 
>>   
>>     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:
>> diff --git a/include/dt-bindings/clock/qcom,dispcc-sm8350.h b/include/dt-bindings/clock/qcom,dispcc-sm8350.h
>> new file mode 120000
>> index 000000000000..0312b4544acb
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/qcom,dispcc-sm8350.h
>> @@ -0,0 +1 @@
>> +qcom,dispcc-sm8250.h
>> \ No newline at end of file

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: clock: add QCOM SM8350 display clock bindings
  2021-06-02 21:27   ` Stephen Boyd
  2021-06-04 17:25     ` Jonathan Marek
@ 2021-06-06  4:11     ` Bjorn Andersson
  1 sibling, 0 replies; 7+ messages in thread
From: Bjorn Andersson @ 2021-06-06  4:11 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Jonathan Marek, linux-arm-msm, Rob Herring, Andy Gross,
	Michael Turquette, Rob Herring, linux-clk, devicetree,
	linux-kernel

On Wed 02 Jun 16:27 CDT 2021, Stephen Boyd wrote:

> Quoting Jonathan Marek (2021-05-18 17:18:02)
> > 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 <jonathan@marek.ca>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> >  .../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?
> 
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
> > index 0cdf53f41f84..8f414642445e 100644
> > --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
> > +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
> > @@ -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"
> 

That seems like a certain way to ensure that SM8450 etc will be
different enough to not match this binding :)

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: clock: add QCOM SM8350 display clock bindings
  2021-06-04 17:25     ` Jonathan Marek
@ 2021-06-28  2:39       ` Stephen Boyd
  0 siblings, 0 replies; 7+ messages in thread
From: Stephen Boyd @ 2021-06-28  2:39 UTC (permalink / raw)
  To: Jonathan Marek, linux-arm-msm
  Cc: Rob Herring, Andy Gross, Bjorn Andersson, Michael Turquette,
	Rob Herring, linux-clk, devicetree, linux-kernel

Quoting Jonathan Marek (2021-06-04 10:25:41)
> On 6/2/21 5:27 PM, Stephen Boyd wrote:
> > Quoting Jonathan Marek (2021-05-18 17:18:02)
> >> 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 <jonathan@marek.ca>
> >> Reviewed-by: Rob Herring <robh@kernel.org>
> >> ---
> >>   .../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?
> > 
> 
> 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.

> 
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
> >> index 0cdf53f41f84..8f414642445e 100644
> >> --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
> >> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
> >> @@ -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"
> > 
> 
> 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?

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-06-28  2:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-19  0:18 [PATCH v2 1/2] clk: qcom: add support for SM8350 DISPCC Jonathan Marek
2021-05-19  0:18 ` [PATCH v2 2/2] dt-bindings: clock: add QCOM SM8350 display clock bindings Jonathan Marek
2021-06-02 21:27   ` Stephen Boyd
2021-06-04 17:25     ` Jonathan Marek
2021-06-28  2:39       ` Stephen Boyd
2021-06-06  4:11     ` Bjorn Andersson
2021-06-02 21:34 ` [PATCH v2 1/2] clk: qcom: add support for SM8350 DISPCC Stephen Boyd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).