[PATCH 0/4] drm/sun4i: dsi: Support the A100/D1 controller variant

[PATCH 0/4] drm/sun4i: dsi: Support the A100/D1 controller variant

[Samuel Holland]

This series adds support for the digital part of the DSI controller
found in the A100 and D1 SoCs (plus T7, which is not supported by
mainline Linux). There are two changes to the hardware integration:
  1) the module clock routes through the TCON TOP, and
  2) the separate I/O domain is removed.

The actual register interface appears to be the same as before. The
register definitions in the D1 BSP exactly match the A64 BSP.

The BSP describes this as the "40nm" DSI controller variant. There is
also a "28nm" variant with a different register interface; that one is
found in a different subset of SoCs (V5 and A50).

A100/D1 also come with an updated DPHY, described by the BSP as a
"combo" PHY, which is now also used for LVDS channel 0. (LVDS and DSI
share the same pins on Port D.) Since that is a different subsystem,
I am sending that as a separate series.


Samuel Holland (4):
  dt-bindings: display: sun6i-dsi: Fix clock conditional
  dt-bindings: display: sun6i-dsi: Add the A100 variant
  drm/sun4i: dsi: Add a variant structure
  drm/sun4i: dsi: Add the A100 variant

 .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 30 +++++++---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c        | 58 +++++++++++++------
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h        |  7 +++
 3 files changed, 69 insertions(+), 26 deletions(-)

-- 
2.35.1

[PATCH 1/4] dt-bindings: display: sun6i-dsi: Fix clock conditional

[Samuel Holland]

The A64 case should have limited maxItems, instead of duplicating the
minItems value from the main binding. While here, simplify the binding
by making this an "else" case of the two-clock conditional block.

Fixes: fe5040f2843a ("dt-bindings: sun6i-dsi: Document A64 MIPI-DSI controller")
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 .../bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
index bf0bdf54e5f9..ae55ef3fb1fe 100644
--- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
@@ -78,16 +78,10 @@ allOf:
       required:
         - clock-names
 
-  - if:
-      properties:
-        compatible:
-          contains:
-            const: allwinner,sun50i-a64-mipi-dsi
-
-    then:
+    else:
       properties:
         clocks:
-          minItems: 1
+          maxItems: 1
 
 unevaluatedProperties: false
 
-- 
2.35.1

[PATCH 2/4] dt-bindings: display: sun6i-dsi: Add the A100 variant

[Samuel Holland]

The "40nm" MIPI DSI controller found in the A100 and D1 SoCs has the
same register layout as previous SoC integrations. However, its module
clock now comes from the TCON, which means it no longer runs at a fixed
rate, so this needs to be distinguished in the driver.

The controller also now uses pins on Port D instead of dedicated pins,
so it drops the separate power domain.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
Removal of the vcc-dsi-supply is maybe a bit questionable. Since there
is no "VCC-DSI" pin anymore, it's not obvious which pin actually does
power the DSI controller/PHY. Possibly power comes from VCC-PD or VCC-IO
or VCC-LVDS. So far, all boards have all of these as always-on supplies,
so it is hard to test.

 .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 28 +++++++++++++++----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
index ae55ef3fb1fe..c53c25b87bd4 100644
--- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
@@ -12,9 +12,14 @@ maintainers:
 
 properties:
   compatible:
-    enum:
-      - allwinner,sun6i-a31-mipi-dsi
-      - allwinner,sun50i-a64-mipi-dsi
+    oneOf:
+      - enum:
+          - allwinner,sun6i-a31-mipi-dsi
+          - allwinner,sun50i-a64-mipi-dsi
+          - allwinner,sun50i-a100-mipi-dsi
+      - items:
+          - const: allwinner,sun20i-d1-mipi-dsi
+          - const: allwinner,sun50i-a100-mipi-dsi
 
   reg:
     maxItems: 1
@@ -59,7 +64,6 @@ required:
   - phys
   - phy-names
   - resets
-  - vcc-dsi-supply
   - port
 
 allOf:
@@ -68,7 +72,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: allwinner,sun6i-a31-mipi-dsi
+            enum:
+              - allwinner,sun6i-a31-mipi-dsi
+              - allwinner,sun50i-a100-mipi-dsi
 
     then:
       properties:
@@ -83,6 +89,18 @@ allOf:
         clocks:
           maxItems: 1
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - allwinner,sun6i-a31-mipi-dsi
+              - allwinner,sun50i-a64-mipi-dsi
+
+    then:
+      required:
+        - vcc-dsi-supply
+
 unevaluatedProperties: false
 
 examples:
-- 
2.35.1

[PATCH 3/4] drm/sun4i: dsi: Add a variant structure

[Samuel Holland]

Replace the ad-hoc calls to of_device_is_compatible() with a structure
describing the differences between variants. This is in preparation for
adding more variants to the driver.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 50 +++++++++++++++++---------
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  7 ++++
 2 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index b4dfa166eccd..6479ade416b9 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -1101,12 +1101,16 @@ static const struct component_ops sun6i_dsi_ops = {
 
 static int sun6i_dsi_probe(struct platform_device *pdev)
 {
+	const struct sun6i_dsi_variant *variant;
 	struct device *dev = &pdev->dev;
-	const char *bus_clk_name = NULL;
 	struct sun6i_dsi *dsi;
 	void __iomem *base;
 	int ret;
 
+	variant = device_get_match_data(dev);
+	if (!variant)
+		return -EINVAL;
+
 	dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
 	if (!dsi)
 		return -ENOMEM;
@@ -1114,10 +1118,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 	dsi->dev = dev;
 	dsi->host.ops = &sun6i_dsi_host_ops;
 	dsi->host.dev = dev;
-
-	if (of_device_is_compatible(dev->of_node,
-				    "allwinner,sun6i-a31-mipi-dsi"))
-		bus_clk_name = "bus";
+	dsi->variant = variant;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base)) {
@@ -1142,7 +1143,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 		return PTR_ERR(dsi->regs);
 	}
 
-	dsi->bus_clk = devm_clk_get(dev, bus_clk_name);
+	dsi->bus_clk = devm_clk_get(dev, variant->has_mod_clk ? "bus" : NULL);
 	if (IS_ERR(dsi->bus_clk))
 		return dev_err_probe(dev, PTR_ERR(dsi->bus_clk),
 				     "Couldn't get the DSI bus clock\n");
@@ -1151,21 +1152,21 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	if (of_device_is_compatible(dev->of_node,
-				    "allwinner,sun6i-a31-mipi-dsi")) {
+	if (variant->has_mod_clk) {
 		dsi->mod_clk = devm_clk_get(dev, "mod");
 		if (IS_ERR(dsi->mod_clk)) {
 			dev_err(dev, "Couldn't get the DSI mod clock\n");
 			ret = PTR_ERR(dsi->mod_clk);
 			goto err_attach_clk;
 		}
-	}
 
-	/*
-	 * In order to operate properly, that clock seems to be always
-	 * set to 297MHz.
-	 */
-	clk_set_rate_exclusive(dsi->mod_clk, 297000000);
+		/*
+		 * In order to operate properly, the module clock on the
+		 * A31 variant always seems to be set to 297MHz.
+		 */
+		if (variant->set_mod_clk)
+			clk_set_rate_exclusive(dsi->mod_clk, 297000000);
+	}
 
 	dsi->dphy = devm_phy_get(dev, "dphy");
 	if (IS_ERR(dsi->dphy)) {
@@ -1205,16 +1206,31 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
 
 	component_del(&pdev->dev, &sun6i_dsi_ops);
 	mipi_dsi_host_unregister(&dsi->host);
-	clk_rate_exclusive_put(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk && dsi->variant->set_mod_clk)
+		clk_rate_exclusive_put(dsi->mod_clk);
 
 	regmap_mmio_detach_clk(dsi->regs);
 
 	return 0;
 }
 
+static const struct sun6i_dsi_variant sun6i_a31_mipi_dsi_variant = {
+	.has_mod_clk	= true,
+	.set_mod_clk	= true,
+};
+
+static const struct sun6i_dsi_variant sun50i_a64_mipi_dsi_variant = {
+};
+
 static const struct of_device_id sun6i_dsi_of_table[] = {
-	{ .compatible = "allwinner,sun6i-a31-mipi-dsi" },
-	{ .compatible = "allwinner,sun50i-a64-mipi-dsi" },
+	{
+		.compatible	= "allwinner,sun6i-a31-mipi-dsi",
+		.data		= &sun6i_a31_mipi_dsi_variant,
+	},
+	{
+		.compatible	= "allwinner,sun50i-a64-mipi-dsi",
+		.data		= &sun50i_a64_mipi_dsi_variant,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table);
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index c863900ae3b4..f1ddefe0f554 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -15,6 +15,11 @@
 
 #define SUN6I_DSI_TCON_DIV	4
 
+struct sun6i_dsi_variant {
+	bool			has_mod_clk;
+	bool			set_mod_clk;
+};
+
 struct sun6i_dsi {
 	struct drm_connector	connector;
 	struct drm_encoder	encoder;
@@ -31,6 +36,8 @@ struct sun6i_dsi {
 	struct mipi_dsi_device	*device;
 	struct drm_device	*drm;
 	struct drm_panel	*panel;
+
+	const struct sun6i_dsi_variant *variant;
 };
 
 static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
-- 
2.35.1

[PATCH 4/4] drm/sun4i: dsi: Add the A100 variant

[Samuel Holland]

The A100 variant of the MIPI DSI controller now gets its module clock
from the TCON via the TCON TOP, so the clock rate cannot be set to a
fixed value. Otherwise, it appears to be the same as the A31 variant.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 6479ade416b9..5db5ecdc2fc6 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -1222,6 +1222,10 @@ static const struct sun6i_dsi_variant sun6i_a31_mipi_dsi_variant = {
 static const struct sun6i_dsi_variant sun50i_a64_mipi_dsi_variant = {
 };
 
+static const struct sun6i_dsi_variant sun50i_a100_mipi_dsi_variant = {
+	.has_mod_clk	= true,
+};
+
 static const struct of_device_id sun6i_dsi_of_table[] = {
 	{
 		.compatible	= "allwinner,sun6i-a31-mipi-dsi",
@@ -1231,6 +1235,10 @@ static const struct of_device_id sun6i_dsi_of_table[] = {
 		.compatible	= "allwinner,sun50i-a64-mipi-dsi",
 		.data		= &sun50i_a64_mipi_dsi_variant,
 	},
+	{
+		.compatible	= "allwinner,sun50i-a100-mipi-dsi",
+		.data		= &sun50i_a100_mipi_dsi_variant,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table);
-- 
2.35.1

Re: [PATCH 1/4] dt-bindings: display: sun6i-dsi: Fix clock conditional

[Krzysztof Kozlowski]

On 12/08/2022 10:42, Samuel Holland wrote:
> The A64 case should have limited maxItems, instead of duplicating the
> minItems value from the main binding. While here, simplify the binding
> by making this an "else" case of the two-clock conditional block.
> 
> Fixes: fe5040f2843a ("dt-bindings: sun6i-dsi: Document A64 MIPI-DSI controller")
> Signed-off-by: Samuel Holland <samuel@sholland.org>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH 2/4] dt-bindings: display: sun6i-dsi: Add the A100 variant

[Krzysztof Kozlowski]

On 12/08/2022 10:42, Samuel Holland wrote:
> The "40nm" MIPI DSI controller found in the A100 and D1 SoCs has the
> same register layout as previous SoC integrations. However, its module
> clock now comes from the TCON, which means it no longer runs at a fixed
> rate, so this needs to be distinguished in the driver.
> 
> The controller also now uses pins on Port D instead of dedicated pins,
> so it drops the separate power domain.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> Removal of the vcc-dsi-supply is maybe a bit questionable. Since there
> is no "VCC-DSI" pin anymore, it's not obvious which pin actually does
> power the DSI controller/PHY. Possibly power comes from VCC-PD or VCC-IO
> or VCC-LVDS. So far, all boards have all of these as always-on supplies,
> so it is hard to test.
> 
>  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 28 +++++++++++++++----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> index ae55ef3fb1fe..c53c25b87bd4 100644
> --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> @@ -12,9 +12,14 @@ maintainers:
>  
>  properties:
>    compatible:
> -    enum:
> -      - allwinner,sun6i-a31-mipi-dsi
> -      - allwinner,sun50i-a64-mipi-dsi
> +    oneOf:
> +      - enum:
> +          - allwinner,sun6i-a31-mipi-dsi
> +          - allwinner,sun50i-a64-mipi-dsi
> +          - allwinner,sun50i-a100-mipi-dsi

While you are moving code, how about bringing alphabetical order?

> +      - items:
> +          - const: allwinner,sun20i-d1-mipi-dsi
> +          - const: allwinner,sun50i-a100-mipi-dsi
>  
>    reg:
>      maxItems: 1
> @@ -59,7 +64,6 @@ required:
>    - phys
>    - phy-names
>    - resets
> -  - vcc-dsi-supply
>    - port
>  
>  allOf:
> @@ -68,7 +72,9 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            const: allwinner,sun6i-a31-mipi-dsi
> +            enum:
> +              - allwinner,sun6i-a31-mipi-dsi
> +              - allwinner,sun50i-a100-mipi-dsi

Here as well

>  
>      then:
>        properties:
> @@ -83,6 +89,18 @@ allOf:
>          clocks:
>            maxItems: 1
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - allwinner,sun6i-a31-mipi-dsi
> +              - allwinner,sun50i-a64-mipi-dsi

and here


Best regards,
Krzysztof

Re: [PATCH 2/4] dt-bindings: display: sun6i-dsi: Add the A100 variant

[Samuel Holland]

Hi Krzysztof,

On 8/12/22 5:49 AM, Krzysztof Kozlowski wrote:
> On 12/08/2022 10:42, Samuel Holland wrote:
>> The "40nm" MIPI DSI controller found in the A100 and D1 SoCs has the
>> same register layout as previous SoC integrations. However, its module
>> clock now comes from the TCON, which means it no longer runs at a fixed
>> rate, so this needs to be distinguished in the driver.
>>
>> The controller also now uses pins on Port D instead of dedicated pins,
>> so it drops the separate power domain.
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>> Removal of the vcc-dsi-supply is maybe a bit questionable. Since there
>> is no "VCC-DSI" pin anymore, it's not obvious which pin actually does
>> power the DSI controller/PHY. Possibly power comes from VCC-PD or VCC-IO
>> or VCC-LVDS. So far, all boards have all of these as always-on supplies,
>> so it is hard to test.
>>
>>  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 28 +++++++++++++++----
>>  1 file changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
>> index ae55ef3fb1fe..c53c25b87bd4 100644
>> --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
>> +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
>> @@ -12,9 +12,14 @@ maintainers:
>>  
>>  properties:
>>    compatible:
>> -    enum:
>> -      - allwinner,sun6i-a31-mipi-dsi
>> -      - allwinner,sun50i-a64-mipi-dsi
>> +    oneOf:
>> +      - enum:
>> +          - allwinner,sun6i-a31-mipi-dsi
>> +          - allwinner,sun50i-a64-mipi-dsi
>> +          - allwinner,sun50i-a100-mipi-dsi
> 
> While you are moving code, how about bringing alphabetical order?

I have put the sun*i prefix in numeric order, which matches (almost) all of our
other bindings. It roughly corresponds to chronological order as well. It
doesn't make much sense to me to sort sun50i (ARM64 SoCs) between sun5i and
sun6i (early ARMv7 SoCs).

Regards,
Samuel

Re: [PATCH 3/4] drm/sun4i: dsi: Add a variant structure

[Jernej Škrabec]

Dne petek, 12. avgust 2022 ob 09:42:55 CEST je Samuel Holland napisal(a):
> Replace the ad-hoc calls to of_device_is_compatible() with a structure
> describing the differences between variants. This is in preparation for
> adding more variants to the driver.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Re: [PATCH 4/4] drm/sun4i: dsi: Add the A100 variant

[Jernej Škrabec]

Dne petek, 12. avgust 2022 ob 09:42:56 CEST je Samuel Holland napisal(a):
> The A100 variant of the MIPI DSI controller now gets its module clock
> from the TCON via the TCON TOP, so the clock rate cannot be set to a
> fixed value. Otherwise, it appears to be the same as the A31 variant.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Re: [PATCH 3/4] drm/sun4i: dsi: Add a variant structure

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3443 bytes --]

Hi Samuel,

On Fri, Aug 12, 2022 at 02:42:55AM -0500, Samuel Holland wrote:
> Replace the ad-hoc calls to of_device_is_compatible() with a structure
> describing the differences between variants. This is in preparation for
> adding more variants to the driver.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 50 +++++++++++++++++---------
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  7 ++++
>  2 files changed, 40 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index b4dfa166eccd..6479ade416b9 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -1101,12 +1101,16 @@ static const struct component_ops sun6i_dsi_ops = {
>  
>  static int sun6i_dsi_probe(struct platform_device *pdev)
>  {
> +	const struct sun6i_dsi_variant *variant;
>  	struct device *dev = &pdev->dev;
> -	const char *bus_clk_name = NULL;
>  	struct sun6i_dsi *dsi;
>  	void __iomem *base;
>  	int ret;
>  
> +	variant = device_get_match_data(dev);
> +	if (!variant)
> +		return -EINVAL;
> +
>  	dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
>  	if (!dsi)
>  		return -ENOMEM;
> @@ -1114,10 +1118,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
>  	dsi->dev = dev;
>  	dsi->host.ops = &sun6i_dsi_host_ops;
>  	dsi->host.dev = dev;
> -
> -	if (of_device_is_compatible(dev->of_node,
> -				    "allwinner,sun6i-a31-mipi-dsi"))
> -		bus_clk_name = "bus";
> +	dsi->variant = variant;
>  
>  	base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(base)) {
> @@ -1142,7 +1143,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
>  		return PTR_ERR(dsi->regs);
>  	}
>  
> -	dsi->bus_clk = devm_clk_get(dev, bus_clk_name);
> +	dsi->bus_clk = devm_clk_get(dev, variant->has_mod_clk ? "bus" : NULL);
>  	if (IS_ERR(dsi->bus_clk))
>  		return dev_err_probe(dev, PTR_ERR(dsi->bus_clk),
>  				     "Couldn't get the DSI bus clock\n");
> @@ -1151,21 +1152,21 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	if (of_device_is_compatible(dev->of_node,
> -				    "allwinner,sun6i-a31-mipi-dsi")) {
> +	if (variant->has_mod_clk) {
>  		dsi->mod_clk = devm_clk_get(dev, "mod");
>  		if (IS_ERR(dsi->mod_clk)) {
>  			dev_err(dev, "Couldn't get the DSI mod clock\n");
>  			ret = PTR_ERR(dsi->mod_clk);
>  			goto err_attach_clk;
>  		}
> -	}
>  
> -	/*
> -	 * In order to operate properly, that clock seems to be always
> -	 * set to 297MHz.
> -	 */
> -	clk_set_rate_exclusive(dsi->mod_clk, 297000000);
> +		/*
> +		 * In order to operate properly, the module clock on the
> +		 * A31 variant always seems to be set to 297MHz.
> +		 */
> +		if (variant->set_mod_clk)
> +			clk_set_rate_exclusive(dsi->mod_clk, 297000000);
> +	}
>
>
>  	dsi->dphy = devm_phy_get(dev, "dphy");
>  	if (IS_ERR(dsi->dphy)) {
> @@ -1205,16 +1206,31 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
>  
>  	component_del(&pdev->dev, &sun6i_dsi_ops);
>  	mipi_dsi_host_unregister(&dsi->host);
> -	clk_rate_exclusive_put(dsi->mod_clk);
> +	if (dsi->variant->has_mod_clk && dsi->variant->set_mod_clk)
> +		clk_rate_exclusive_put(dsi->mod_clk);

There's also a clk_rate_exclusive_put call in the bind error path

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/4] dt-bindings: display: sun6i-dsi: Add the A100 variant

[Krzysztof Kozlowski]

On 13/08/2022 01:58, Samuel Holland wrote:
> Hi Krzysztof,
> 
> On 8/12/22 5:49 AM, Krzysztof Kozlowski wrote:
>> On 12/08/2022 10:42, Samuel Holland wrote:
>>> The "40nm" MIPI DSI controller found in the A100 and D1 SoCs has the
>>> same register layout as previous SoC integrations. However, its module
>>> clock now comes from the TCON, which means it no longer runs at a fixed
>>> rate, so this needs to be distinguished in the driver.
>>>
>>> The controller also now uses pins on Port D instead of dedicated pins,
>>> so it drops the separate power domain.
>>>
>>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>>> ---
>>> Removal of the vcc-dsi-supply is maybe a bit questionable. Since there
>>> is no "VCC-DSI" pin anymore, it's not obvious which pin actually does
>>> power the DSI controller/PHY. Possibly power comes from VCC-PD or VCC-IO
>>> or VCC-LVDS. So far, all boards have all of these as always-on supplies,
>>> so it is hard to test.
>>>
>>>  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 28 +++++++++++++++----
>>>  1 file changed, 23 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
>>> index ae55ef3fb1fe..c53c25b87bd4 100644
>>> --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
>>> +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
>>> @@ -12,9 +12,14 @@ maintainers:
>>>  
>>>  properties:
>>>    compatible:
>>> -    enum:
>>> -      - allwinner,sun6i-a31-mipi-dsi
>>> -      - allwinner,sun50i-a64-mipi-dsi
>>> +    oneOf:
>>> +      - enum:
>>> +          - allwinner,sun6i-a31-mipi-dsi
>>> +          - allwinner,sun50i-a64-mipi-dsi
>>> +          - allwinner,sun50i-a100-mipi-dsi
>>
>> While you are moving code, how about bringing alphabetical order?
> 
> I have put the sun*i prefix in numeric order, which matches (almost) all of our

5 is before 6, so strictly numerical order would be:
allwinner,sun50i-a64-mipi-dsi
allwinner,sun50i-a100-mipi-dsi
allwinner,sun6i-a31-mipi-dsi

> other bindings. It roughly corresponds to chronological order as well. It
> doesn't make much sense to me to sort sun50i (ARM64 SoCs) between sun5i and
> sun6i (early ARMv7 SoCs).

However if you say you already implemented some order (obvious for
Allwinner folks), then of course it is fine with me. I just hope other
people will get figure out this order, so they can maintain it.

So assuming there is some order:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof