[PATCH v10 0/6] drm/sun4i: Allwinner A64 MIPI-DSI support

[PATCH v10 0/6] drm/sun4i: Allwinner A64 MIPI-DSI support

[Jagan Teki]

This is v10 version for Allwinner A64 MIPI-DSI support
and here is the previous version set[1].

This series on top of drm-misc/for-linux-next along with video start
delay fix [2]

Changes for v10:
- updated dt-bindings as per .yaml format
- rebased on drm-misc/for-linux-next
Changes for v9:
- moved dsi fixes in separate series on top of A33 [2]
- rebase on linux-next and on top of [2]
Changes for v8:
- rebased on drm-misc change along with linux-next
- reworked video start delay patch
- tested on 4 different dsi panels
- reworked commit messages
Changes for v7:
- moved vcc-dsi binding to required filed.
- drop quotes on fallback dphy bindings.
- drop min_rate clock pll-mipi patches.
- introduce dclk divider computation as like A64 BSP.
- add A64 DSI quark patches.
- fixed A64 DSI pipeline.
- add proper commit messages.
- collect Merlijn Wajer Tested-by credits.
Changes for v6:
- dropped unneeded changes, patches
- fixed all burst mode patches as per previous version comments
- rebase on master
- update proper commit message
- dropped unneeded comments
- order the patches that make review easy
Changes for v5:
- collect Rob, Acked-by
- droped "Fix VBP size calculation" patch
- updated vblk timing calculation.
- droped techstar, bananapi dsi panel drivers which may require
  bridge or other setup. it's under discussion.
Changes for v4:
- droppoed untested CCU_FEATURE_FIXED_POSTDIV check code in
  nkm min, max rate patches
- create two patches for "Add Allwinner A64 MIPI DSI support"
  one for has_mod_clk quirk and other one for A64 support
- use existing driver code construct for hblk computation
- dropped "Increase hfp packet overhead" patch [2], though BSP added
  this but we have no issues as of now.
  (no issues on panel side w/o this change)
- create separate function for vblk computation 
- enable vcc-dsi regulator in dsi_runtime_resume
- collect Rob, Acked-by
- update MAINTAINERS file for panel drivers
- cleanup commit messages
- fixed checkpatch warnings/errors

[1] https://patchwork.freedesktop.org/series/61310/
[2] https://patchwork.freedesktop.org/patch/334086/

Any inputs?
Jagan.

Jagan Teki (6):
  dt-bindings: sun6i-dsi: Add A64 MIPI-DSI compatible
  dt-bindings: sun6i-dsi: Add A64 DPHY compatible (w/ A31 fallback)
  drm/sun4i: dsi: Add has_mod_clk quirk
  drm/sun4i: dsi: Add Allwinner A64 MIPI DSI support
  arm64: dts: allwinner: a64: Add MIPI DSI pipeline
  [DO NOT MERGE] arm64: dts: allwinner: bananapi-m64: Enable Bananapi S070WV20-CT16 DSI
    panel

 .../display/allwinner,sun6i-a31-mipi-dsi.yaml |  4 +-
 .../phy/allwinner,sun6i-a31-mipi-dphy.yaml    |  6 ++-
 .../dts/allwinner/sun50i-a64-bananapi-m64.dts | 31 ++++++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c        | 42 ++++++++++++++-----
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h        |  5 +++
 6 files changed, 114 insertions(+), 12 deletions(-)

-- 
2.18.0.321.gffc6fa0e3

[PATCH v10 1/6] dt-bindings: sun6i-dsi: Add A64 MIPI-DSI compatible

[Jagan Teki]

The MIPI DSI controller in Allwinner A64 is similar to A33.

But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
to with separate compatible for A64 on the same driver.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 .../bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml        | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

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 dafc0980c4fa..cfcc84d38084 100644
--- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
@@ -15,7 +15,9 @@ properties:
   "#size-cells": true
 
   compatible:
-    const: allwinner,sun6i-a31-mipi-dsi
+    enum:
+      - const: allwinner,sun6i-a31-mipi-dsi
+      - const: allwinner,sun50i-a64-mipi-dsi
 
   reg:
     maxItems: 1
-- 
2.18.0.321.gffc6fa0e3

[PATCH v10 2/6] dt-bindings: sun6i-dsi: Add A64 DPHY compatible (w/ A31 fallback)

[Jagan Teki]

The MIPI DSI PHY controller on Allwinner A64 is similar
on the one on A31.

Add A64 compatible and append A31 compatible as fallback.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 .../bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml         | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
index fa46670de299..8841938050b2 100644
--- a/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
+++ b/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
@@ -15,7 +15,11 @@ properties:
     const: 0
 
   compatible:
-    const: allwinner,sun6i-a31-mipi-dphy
+    oneOf:
+      - const: allwinner,sun6i-a31-mipi-dphy
+      - items:
+          - const: allwinner,sun50i-a64-mipi-dphy
+          - const: allwinner,sun6i-a31-mipi-dphy
 
   reg:
     maxItems: 1
-- 
2.18.0.321.gffc6fa0e3

[PATCH v10 3/6] drm/sun4i: dsi: Add has_mod_clk quirk

[Jagan Teki]

As per the user manual, look like mod clock is not mandatory
for all Allwinner MIPI DSI controllers, it is connected to
CLK_DSI_SCLK for A31 and not available in A64.

So add has_mod_clk quirk and process the clk accordingly.

Tested-by: Merlijn Wajer <merlijn@wizzup.org>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 35 ++++++++++++++++++--------
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  5 ++++
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index b8a0d0501ca7..6724a14c816d 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -11,6 +11,7 @@
 #include <linux/crc-ccitt.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/phy/phy-mipi-dphy.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
@@ -1092,6 +1093,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 	dsi->dev = dev;
 	dsi->host.ops = &sun6i_dsi_host_ops;
 	dsi->host.dev = dev;
+	dsi->variant = of_device_get_match_data(dev);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	base = devm_ioremap_resource(dev, res);
@@ -1119,17 +1121,20 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 		return PTR_ERR(dsi->reset);
 	}
 
-	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");
-		return PTR_ERR(dsi->mod_clk);
+	if (dsi->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");
+			return PTR_ERR(dsi->mod_clk);
+		}
 	}
 
 	/*
 	 * In order to operate properly, that clock seems to be always
 	 * set to 297MHz.
 	 */
-	clk_set_rate_exclusive(dsi->mod_clk, 297000000);
+	if (dsi->variant->has_mod_clk)
+		clk_set_rate_exclusive(dsi->mod_clk, 297000000);
 
 	dsi->dphy = devm_phy_get(dev, "dphy");
 	if (IS_ERR(dsi->dphy)) {
@@ -1159,7 +1164,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 err_pm_disable:
 	pm_runtime_disable(dev);
 err_unprotect_clk:
-	clk_rate_exclusive_put(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk)
+		clk_rate_exclusive_put(dsi->mod_clk);
 	return ret;
 }
 
@@ -1171,7 +1177,8 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
 	component_del(&pdev->dev, &sun6i_dsi_ops);
 	mipi_dsi_host_unregister(&dsi->host);
 	pm_runtime_disable(dev);
-	clk_rate_exclusive_put(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk)
+		clk_rate_exclusive_put(dsi->mod_clk);
 
 	return 0;
 }
@@ -1188,7 +1195,8 @@ static int __maybe_unused sun6i_dsi_runtime_resume(struct device *dev)
 	}
 
 	reset_control_deassert(dsi->reset);
-	clk_prepare_enable(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk)
+		clk_prepare_enable(dsi->mod_clk);
 
 	/*
 	 * Enable the DSI block.
@@ -1229,9 +1237,16 @@ static const struct dev_pm_ops sun6i_dsi_pm_ops = {
 			   NULL)
 };
 
+static const struct sun6i_dsi_variant sun6i_a31_mipi_dsi = {
+	.has_mod_clk = true,
+};
+
 static const struct of_device_id sun6i_dsi_of_table[] = {
-	{ .compatible = "allwinner,sun6i-a31-mipi-dsi" },
-	{ }
+	{
+		.compatible = "allwinner,sun6i-a31-mipi-dsi",
+		.data = &sun6i_a31_mipi_dsi,
+	},
+	{ /* sentinel */ }
 };
 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 3f4846f581ef..d791c9f6fccf 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -15,6 +15,10 @@
 
 #define SUN6I_DSI_TCON_DIV	4
 
+struct sun6i_dsi_variant {
+	bool			has_mod_clk;
+};
+
 struct sun6i_dsi {
 	struct drm_connector	connector;
 	struct drm_encoder	encoder;
@@ -31,6 +35,7 @@ struct sun6i_dsi {
 	struct sun4i_drv	*drv;
 	struct mipi_dsi_device	*device;
 	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.18.0.321.gffc6fa0e3

[PATCH v10 4/6] drm/sun4i: dsi: Add Allwinner A64 MIPI DSI support

[Jagan Teki]

The MIPI DSI controller in Allwinner A64 is similar to A33.

But unlike A33, A64 doesn't have DSI_SCLK gating so add compatible
for Allwinner A64 with uninitialized has_mod_clk driver.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Merlijn Wajer <merlijn@wizzup.org>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 6724a14c816d..e3aab815fa73 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -1241,11 +1241,18 @@ static const struct sun6i_dsi_variant sun6i_a31_mipi_dsi = {
 	.has_mod_clk = true,
 };
 
+static const struct sun6i_dsi_variant sun50i_a64_mipi_dsi = {
+};
+
 static const struct of_device_id sun6i_dsi_of_table[] = {
 	{
 		.compatible = "allwinner,sun6i-a31-mipi-dsi",
 		.data = &sun6i_a31_mipi_dsi,
 	},
+	{
+		.compatible = "allwinner,sun50i-a64-mipi-dsi",
+		.data = &sun50i_a64_mipi_dsi,
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table);
-- 
2.18.0.321.gffc6fa0e3

[PATCH v10 5/6] arm64: dts: allwinner: a64: Add MIPI DSI pipeline

[Jagan Teki]

Add MIPI DSI pipeline for Allwinner A64.

- dsi node, with A64 compatible since it doesn't support
  DSI_SCLK gating unlike A33
- dphy node, with A64 compatible with A33 fallback since
  DPHY on A64 and A33 is similar
- finally, attach the dsi_in to tcon0 for complete MIPI DSI

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Merlijn Wajer <merlijn@wizzup.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 69128a6dfc46..ad4170b8aee0 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -382,6 +382,12 @@
 					#address-cells = <1>;
 					#size-cells = <0>;
 					reg = <1>;
+
+					tcon0_out_dsi: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&dsi_in_tcon0>;
+						allwinner,tcon-channel = <1>;
+					};
 				};
 			};
 		};
@@ -1003,6 +1009,38 @@
 			status = "disabled";
 		};
 
+		dsi: dsi@1ca0000 {
+			compatible = "allwinner,sun50i-a64-mipi-dsi";
+			reg = <0x01ca0000 0x1000>;
+			interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_MIPI_DSI>;
+			clock-names = "bus";
+			resets = <&ccu RST_BUS_MIPI_DSI>;
+			phys = <&dphy>;
+			phy-names = "dphy";
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port {
+				dsi_in_tcon0: endpoint {
+					remote-endpoint = <&tcon0_out_dsi>;
+				};
+			};
+		};
+
+		dphy: d-phy@1ca1000 {
+			compatible = "allwinner,sun50i-a64-mipi-dphy",
+				     "allwinner,sun6i-a31-mipi-dphy";
+			reg = <0x01ca1000 0x1000>;
+			clocks = <&ccu CLK_BUS_MIPI_DSI>,
+				 <&ccu CLK_DSI_DPHY>;
+			clock-names = "bus", "mod";
+			resets = <&ccu RST_BUS_MIPI_DSI>;
+			status = "disabled";
+			#phy-cells = <0>;
+		};
+
 		hdmi: hdmi@1ee0000 {
 			compatible = "allwinner,sun50i-a64-dw-hdmi",
 				     "allwinner,sun8i-a83t-dw-hdmi";
-- 
2.18.0.321.gffc6fa0e3

[DO NOT MERGE] [PATCH v10 6/6] arm64: dts: allwinner: bananapi-m64: Enable Bananapi S070WV20-CT16 DSI panel

[Jagan Teki]

This patch add support for Bananapi S070WV20-CT16 DSI panel to
BPI-M64 board.

DSI panel connected via board DSI port with,
- DLDO1 as VCC-DSI supply
- DCDC1 as VDD supply
- PD7 gpio for lcd enable pin
- PD6 gpio for lcd reset pin
- PD5 gpio for backlight enable pin

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 .../dts/allwinner/sun50i-a64-bananapi-m64.dts | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
index 208373efee49..6beaecdd802a 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
@@ -45,6 +45,7 @@
 #include "sun50i-a64.dtsi"
 
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	model = "BananaPi-M64";
@@ -56,6 +57,14 @@
 		serial1 = &uart1;
 	};
 
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&r_pwm 0 50000 PWM_POLARITY_INVERTED>;
+		brightness-levels = <1 2 4 8 16 32 64 128 255>;
+		default-brightness-level = <2>;
+		enable-gpios = <&pio 3 5 GPIO_ACTIVE_HIGH>; /* LCD-BL-EN: PD5 */
+	};
+
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
@@ -116,6 +125,24 @@
 	status = "okay";
 };
 
+&dphy {
+	status = "okay";
+};
+
+&dsi {
+	vcc-dsi-supply = <&reg_dldo1>;		/* VCC3V3-DSI */
+	status = "okay";
+
+	panel@0 {
+		compatible = "bananapi,s070wv20-ct16-icn6211";
+		reg = <0>;
+		enable-gpios = <&pio 3 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PD7 */
+		reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* LCD-RST: PD6 */
+		vdd-supply = <&reg_dcdc1>;
+		backlight = <&backlight>;
+	};
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -206,6 +233,10 @@
 	status = "okay";
 };
 
+&r_pwm {
+	status = "okay";
+};
+
 &r_rsb {
 	status = "okay";
 
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v10 1/6] dt-bindings: sun6i-dsi: Add A64 MIPI-DSI compatible

[Maxime Ripard]

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

On Sat, Oct 05, 2019 at 07:49:08PM +0530, Jagan Teki wrote:
> The MIPI DSI controller in Allwinner A64 is similar to A33.
>
> But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> to with separate compatible for A64 on the same driver.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  .../bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml        | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> 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 dafc0980c4fa..cfcc84d38084 100644
> --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> @@ -15,7 +15,9 @@ properties:
>    "#size-cells": true
>
>    compatible:
> -    const: allwinner,sun6i-a31-mipi-dsi
> +    enum:
> +      - const: allwinner,sun6i-a31-mipi-dsi
> +      - const: allwinner,sun50i-a64-mipi-dsi

How did you test this? It will report an error when running the
validation

Maxime

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

Re: [PATCH v10 5/6] arm64: dts: allwinner: a64: Add MIPI DSI pipeline

[Maxime Ripard]

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

On Sat, Oct 05, 2019 at 07:49:12PM +0530, Jagan Teki wrote:
> Add MIPI DSI pipeline for Allwinner A64.
>
> - dsi node, with A64 compatible since it doesn't support
>   DSI_SCLK gating unlike A33
> - dphy node, with A64 compatible with A33 fallback since
>   DPHY on A64 and A33 is similar
> - finally, attach the dsi_in to tcon0 for complete MIPI DSI
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 69128a6dfc46..ad4170b8aee0 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -382,6 +382,12 @@
>  					#address-cells = <1>;
>  					#size-cells = <0>;
>  					reg = <1>;
> +
> +					tcon0_out_dsi: endpoint@1 {
> +						reg = <1>;
> +						remote-endpoint = <&dsi_in_tcon0>;
> +						allwinner,tcon-channel = <1>;
> +					};
>  				};
>  			};
>  		};
> @@ -1003,6 +1009,38 @@
>  			status = "disabled";
>  		};
>
> +		dsi: dsi@1ca0000 {
> +			compatible = "allwinner,sun50i-a64-mipi-dsi";
> +			reg = <0x01ca0000 0x1000>;
> +			interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&ccu CLK_BUS_MIPI_DSI>;
> +			clock-names = "bus";

This won't validate with the bindings you have either here, since it
still expects bus and mod.

I guess in that cas, we can just drop clock-names, which will require
a bit of work on the driver side as well.

Maxime

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

Re: [PATCH v10 1/6] dt-bindings: sun6i-dsi: Add A64 MIPI-DSI compatible

[Jagan Teki]

On Mon, Oct 7, 2019 at 3:01 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Sat, Oct 05, 2019 at 07:49:08PM +0530, Jagan Teki wrote:
> > The MIPI DSI controller in Allwinner A64 is similar to A33.
> >
> > But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> > to with separate compatible for A64 on the same driver.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  .../bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml        | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > 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 dafc0980c4fa..cfcc84d38084 100644
> > --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > @@ -15,7 +15,9 @@ properties:
> >    "#size-cells": true
> >
> >    compatible:
> > -    const: allwinner,sun6i-a31-mipi-dsi
> > +    enum:
> > +      - const: allwinner,sun6i-a31-mipi-dsi
> > +      - const: allwinner,sun50i-a64-mipi-dsi
>
> How did you test this? It will report an error when running the
> validation

I did follow the v9 comments [1] and forgot to do dt-doc-validate.
will send the v11 for this patch alone, will that be okay?

[1] https://patchwork.freedesktop.org/patch/307499/

Re: [PATCH v10 1/6] dt-bindings: sun6i-dsi: Add A64 MIPI-DSI compatible

[Maxime Ripard]

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

On Sun, Oct 13, 2019 at 10:32:18PM +0530, Jagan Teki wrote:
> On Mon, Oct 7, 2019 at 3:01 PM Maxime Ripard <mripard@kernel.org> wrote:
> >
> > On Sat, Oct 05, 2019 at 07:49:08PM +0530, Jagan Teki wrote:
> > > The MIPI DSI controller in Allwinner A64 is similar to A33.
> > >
> > > But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> > > to with separate compatible for A64 on the same driver.
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > >  .../bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml        | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > 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 dafc0980c4fa..cfcc84d38084 100644
> > > --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > @@ -15,7 +15,9 @@ properties:
> > >    "#size-cells": true
> > >
> > >    compatible:
> > > -    const: allwinner,sun6i-a31-mipi-dsi
> > > +    enum:
> > > +      - const: allwinner,sun6i-a31-mipi-dsi
> > > +      - const: allwinner,sun50i-a64-mipi-dsi
> >
> > How did you test this? It will report an error when running the
> > validation
>
> I did follow the v9 comments [1] and forgot to do dt-doc-validate.
> will send the v11 for this patch alone, will that be okay?
>
> [1] https://patchwork.freedesktop.org/patch/307499/

There's some changes to the DTSI to do as well, so please do them,
check that the device trees are validated properly, and send a new
version.

Maxime

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

Re: [PATCH v10 5/6] arm64: dts: allwinner: a64: Add MIPI DSI pipeline

[Jagan Teki]

On Mon, Oct 7, 2019 at 4:27 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Sat, Oct 05, 2019 at 07:49:12PM +0530, Jagan Teki wrote:
> > Add MIPI DSI pipeline for Allwinner A64.
> >
> > - dsi node, with A64 compatible since it doesn't support
> >   DSI_SCLK gating unlike A33
> > - dphy node, with A64 compatible with A33 fallback since
> >   DPHY on A64 and A33 is similar
> > - finally, attach the dsi_in to tcon0 for complete MIPI DSI
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > ---
> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > index 69128a6dfc46..ad4170b8aee0 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > @@ -382,6 +382,12 @@
> >                                       #address-cells = <1>;
> >                                       #size-cells = <0>;
> >                                       reg = <1>;
> > +
> > +                                     tcon0_out_dsi: endpoint@1 {
> > +                                             reg = <1>;
> > +                                             remote-endpoint = <&dsi_in_tcon0>;
> > +                                             allwinner,tcon-channel = <1>;
> > +                                     };
> >                               };
> >                       };
> >               };
> > @@ -1003,6 +1009,38 @@
> >                       status = "disabled";
> >               };
> >
> > +             dsi: dsi@1ca0000 {
> > +                     compatible = "allwinner,sun50i-a64-mipi-dsi";
> > +                     reg = <0x01ca0000 0x1000>;
> > +                     interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > +                     clocks = <&ccu CLK_BUS_MIPI_DSI>;
> > +                     clock-names = "bus";
>
> This won't validate with the bindings you have either here, since it
> still expects bus and mod.
>
> I guess in that cas, we can just drop clock-names, which will require
> a bit of work on the driver side as well.

Okay.
mod clock is not required for a64, ie reason we have has_mod_clk quirk
patch. Adjust the clock-names: on dt-bindings would make sense here,
what do you think?

Re: [PATCH v10 5/6] arm64: dts: allwinner: a64: Add MIPI DSI pipeline

[Maxime Ripard]

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

On Mon, Oct 14, 2019 at 05:37:50PM +0530, Jagan Teki wrote:
> On Mon, Oct 7, 2019 at 4:27 PM Maxime Ripard <mripard@kernel.org> wrote:
> >
> > On Sat, Oct 05, 2019 at 07:49:12PM +0530, Jagan Teki wrote:
> > > Add MIPI DSI pipeline for Allwinner A64.
> > >
> > > - dsi node, with A64 compatible since it doesn't support
> > >   DSI_SCLK gating unlike A33
> > > - dphy node, with A64 compatible with A33 fallback since
> > >   DPHY on A64 and A33 is similar
> > > - finally, attach the dsi_in to tcon0 for complete MIPI DSI
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > > ---
> > >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++++
> > >  1 file changed, 38 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > index 69128a6dfc46..ad4170b8aee0 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > @@ -382,6 +382,12 @@
> > >                                       #address-cells = <1>;
> > >                                       #size-cells = <0>;
> > >                                       reg = <1>;
> > > +
> > > +                                     tcon0_out_dsi: endpoint@1 {
> > > +                                             reg = <1>;
> > > +                                             remote-endpoint = <&dsi_in_tcon0>;
> > > +                                             allwinner,tcon-channel = <1>;
> > > +                                     };
> > >                               };
> > >                       };
> > >               };
> > > @@ -1003,6 +1009,38 @@
> > >                       status = "disabled";
> > >               };
> > >
> > > +             dsi: dsi@1ca0000 {
> > > +                     compatible = "allwinner,sun50i-a64-mipi-dsi";
> > > +                     reg = <0x01ca0000 0x1000>;
> > > +                     interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > > +                     clocks = <&ccu CLK_BUS_MIPI_DSI>;
> > > +                     clock-names = "bus";
> >
> > This won't validate with the bindings you have either here, since it
> > still expects bus and mod.
> >
> > I guess in that cas, we can just drop clock-names, which will require
> > a bit of work on the driver side as well.
>
> Okay.
> mod clock is not required for a64, ie reason we have has_mod_clk quirk
> patch. Adjust the clock-names: on dt-bindings would make sense here,
> what do you think?

I'm confused, what are you suggesting?

Maxime

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

Re: [PATCH v10 5/6] arm64: dts: allwinner: a64: Add MIPI DSI pipeline

[Jagan Teki]

On Wed, Oct 16, 2019 at 1:33 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Mon, Oct 14, 2019 at 05:37:50PM +0530, Jagan Teki wrote:
> > On Mon, Oct 7, 2019 at 4:27 PM Maxime Ripard <mripard@kernel.org> wrote:
> > >
> > > On Sat, Oct 05, 2019 at 07:49:12PM +0530, Jagan Teki wrote:
> > > > Add MIPI DSI pipeline for Allwinner A64.
> > > >
> > > > - dsi node, with A64 compatible since it doesn't support
> > > >   DSI_SCLK gating unlike A33
> > > > - dphy node, with A64 compatible with A33 fallback since
> > > >   DPHY on A64 and A33 is similar
> > > > - finally, attach the dsi_in to tcon0 for complete MIPI DSI
> > > >
> > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > > > ---
> > > >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++++
> > > >  1 file changed, 38 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > index 69128a6dfc46..ad4170b8aee0 100644
> > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > @@ -382,6 +382,12 @@
> > > >                                       #address-cells = <1>;
> > > >                                       #size-cells = <0>;
> > > >                                       reg = <1>;
> > > > +
> > > > +                                     tcon0_out_dsi: endpoint@1 {
> > > > +                                             reg = <1>;
> > > > +                                             remote-endpoint = <&dsi_in_tcon0>;
> > > > +                                             allwinner,tcon-channel = <1>;
> > > > +                                     };
> > > >                               };
> > > >                       };
> > > >               };
> > > > @@ -1003,6 +1009,38 @@
> > > >                       status = "disabled";
> > > >               };
> > > >
> > > > +             dsi: dsi@1ca0000 {
> > > > +                     compatible = "allwinner,sun50i-a64-mipi-dsi";
> > > > +                     reg = <0x01ca0000 0x1000>;
> > > > +                     interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > > > +                     clocks = <&ccu CLK_BUS_MIPI_DSI>;
> > > > +                     clock-names = "bus";
> > >
> > > This won't validate with the bindings you have either here, since it
> > > still expects bus and mod.
> > >
> > > I guess in that cas, we can just drop clock-names, which will require
> > > a bit of work on the driver side as well.
> >
> > Okay.
> > mod clock is not required for a64, ie reason we have has_mod_clk quirk
> > patch. Adjust the clock-names: on dt-bindings would make sense here,
> > what do you think?
>
> I'm confused, what are you suggesting?

Sorry for the confusion.

The mod clock is not required for A64 and we have a patch for handling
mod clock using has_mod_clk quirk(on the series), indeed the mod clock
is available in A31 and not needed for A64. So, to satisfy this
requirement the clock-names on dt-bindings can update to make mod
clock-name is optional and bus clock is required.

I'm not exactly sure, this is correct but trying to understand if it
is possible or not? something like

   clocks:
      minItems: 1
      maxItems: 2
     items:
       - description: Bus Clock
       - description: Module Clock

   clock-names:
      minItems: 1
      maxItems: 2
     items:
       - const: bus
       - const: mod

Re: [PATCH v10 5/6] arm64: dts: allwinner: a64: Add MIPI DSI pipeline

[Maxime Ripard]

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

On Wed, Oct 16, 2019 at 02:19:44PM +0530, Jagan Teki wrote:
> On Wed, Oct 16, 2019 at 1:33 PM Maxime Ripard <mripard@kernel.org> wrote:
> >
> > On Mon, Oct 14, 2019 at 05:37:50PM +0530, Jagan Teki wrote:
> > > On Mon, Oct 7, 2019 at 4:27 PM Maxime Ripard <mripard@kernel.org> wrote:
> > > >
> > > > On Sat, Oct 05, 2019 at 07:49:12PM +0530, Jagan Teki wrote:
> > > > > Add MIPI DSI pipeline for Allwinner A64.
> > > > >
> > > > > - dsi node, with A64 compatible since it doesn't support
> > > > >   DSI_SCLK gating unlike A33
> > > > > - dphy node, with A64 compatible with A33 fallback since
> > > > >   DPHY on A64 and A33 is similar
> > > > > - finally, attach the dsi_in to tcon0 for complete MIPI DSI
> > > > >
> > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > > > > ---
> > > > >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++++
> > > > >  1 file changed, 38 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > index 69128a6dfc46..ad4170b8aee0 100644
> > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > @@ -382,6 +382,12 @@
> > > > >                                       #address-cells = <1>;
> > > > >                                       #size-cells = <0>;
> > > > >                                       reg = <1>;
> > > > > +
> > > > > +                                     tcon0_out_dsi: endpoint@1 {
> > > > > +                                             reg = <1>;
> > > > > +                                             remote-endpoint = <&dsi_in_tcon0>;
> > > > > +                                             allwinner,tcon-channel = <1>;
> > > > > +                                     };
> > > > >                               };
> > > > >                       };
> > > > >               };
> > > > > @@ -1003,6 +1009,38 @@
> > > > >                       status = "disabled";
> > > > >               };
> > > > >
> > > > > +             dsi: dsi@1ca0000 {
> > > > > +                     compatible = "allwinner,sun50i-a64-mipi-dsi";
> > > > > +                     reg = <0x01ca0000 0x1000>;
> > > > > +                     interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > > > > +                     clocks = <&ccu CLK_BUS_MIPI_DSI>;
> > > > > +                     clock-names = "bus";
> > > >
> > > > This won't validate with the bindings you have either here, since it
> > > > still expects bus and mod.
> > > >
> > > > I guess in that cas, we can just drop clock-names, which will require
> > > > a bit of work on the driver side as well.
> > >
> > > Okay.
> > > mod clock is not required for a64, ie reason we have has_mod_clk quirk
> > > patch. Adjust the clock-names: on dt-bindings would make sense here,
> > > what do you think?
> >
> > I'm confused, what are you suggesting?
>
> Sorry for the confusion.
>
> The mod clock is not required for A64 and we have a patch for handling
> mod clock using has_mod_clk quirk(on the series), indeed the mod clock
> is available in A31 and not needed for A64. So, to satisfy this
> requirement the clock-names on dt-bindings can update to make mod
> clock-name is optional and bus clock is required.

No, the bus clock name is not needed if there's only one clock.

> I'm not exactly sure, this is correct but trying to understand if it
> is possible or not? something like
>
>    clocks:
>       minItems: 1
>       maxItems: 2
>      items:
>        - description: Bus Clock
>        - description: Module Clock

That's correct.

>    clock-names:
>       minItems: 1
>       maxItems: 2
>      items:
>        - const: bus
>        - const: mod

Here, just keep the current clock-names definition, and make it
required only for SoCs that are not the A64

Maxime

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

Re: [PATCH v10 5/6] arm64: dts: allwinner: a64: Add MIPI DSI pipeline

[Jagan Teki]

On Thu, Oct 17, 2019 at 3:22 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Wed, Oct 16, 2019 at 02:19:44PM +0530, Jagan Teki wrote:
> > On Wed, Oct 16, 2019 at 1:33 PM Maxime Ripard <mripard@kernel.org> wrote:
> > >
> > > On Mon, Oct 14, 2019 at 05:37:50PM +0530, Jagan Teki wrote:
> > > > On Mon, Oct 7, 2019 at 4:27 PM Maxime Ripard <mripard@kernel.org> wrote:
> > > > >
> > > > > On Sat, Oct 05, 2019 at 07:49:12PM +0530, Jagan Teki wrote:
> > > > > > Add MIPI DSI pipeline for Allwinner A64.
> > > > > >
> > > > > > - dsi node, with A64 compatible since it doesn't support
> > > > > >   DSI_SCLK gating unlike A33
> > > > > > - dphy node, with A64 compatible with A33 fallback since
> > > > > >   DPHY on A64 and A33 is similar
> > > > > > - finally, attach the dsi_in to tcon0 for complete MIPI DSI
> > > > > >
> > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > > > > > ---
> > > > > >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++++
> > > > > >  1 file changed, 38 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > index 69128a6dfc46..ad4170b8aee0 100644
> > > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > @@ -382,6 +382,12 @@
> > > > > >                                       #address-cells = <1>;
> > > > > >                                       #size-cells = <0>;
> > > > > >                                       reg = <1>;
> > > > > > +
> > > > > > +                                     tcon0_out_dsi: endpoint@1 {
> > > > > > +                                             reg = <1>;
> > > > > > +                                             remote-endpoint = <&dsi_in_tcon0>;
> > > > > > +                                             allwinner,tcon-channel = <1>;
> > > > > > +                                     };
> > > > > >                               };
> > > > > >                       };
> > > > > >               };
> > > > > > @@ -1003,6 +1009,38 @@
> > > > > >                       status = "disabled";
> > > > > >               };
> > > > > >
> > > > > > +             dsi: dsi@1ca0000 {
> > > > > > +                     compatible = "allwinner,sun50i-a64-mipi-dsi";
> > > > > > +                     reg = <0x01ca0000 0x1000>;
> > > > > > +                     interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > +                     clocks = <&ccu CLK_BUS_MIPI_DSI>;
> > > > > > +                     clock-names = "bus";
> > > > >
> > > > > This won't validate with the bindings you have either here, since it
> > > > > still expects bus and mod.
> > > > >
> > > > > I guess in that cas, we can just drop clock-names, which will require
> > > > > a bit of work on the driver side as well.
> > > >
> > > > Okay.
> > > > mod clock is not required for a64, ie reason we have has_mod_clk quirk
> > > > patch. Adjust the clock-names: on dt-bindings would make sense here,
> > > > what do you think?
> > >
> > > I'm confused, what are you suggesting?
> >
> > Sorry for the confusion.
> >
> > The mod clock is not required for A64 and we have a patch for handling
> > mod clock using has_mod_clk quirk(on the series), indeed the mod clock
> > is available in A31 and not needed for A64. So, to satisfy this
> > requirement the clock-names on dt-bindings can update to make mod
> > clock-name is optional and bus clock is required.
>
> No, the bus clock name is not needed if there's only one clock.

Okay, is it because the same clock handle it on PHY side?

>
> > I'm not exactly sure, this is correct but trying to understand if it
> > is possible or not? something like
> >
> >    clocks:
> >       minItems: 1
> >       maxItems: 2
> >      items:
> >        - description: Bus Clock
> >        - description: Module Clock
>
> That's correct.
>
> >    clock-names:
> >       minItems: 1
> >       maxItems: 2
> >      items:
> >        - const: bus
> >        - const: mod
>
> Here, just keep the current clock-names definition, and make it
> required only for SoCs that are not the A64

Okay, please have a look here I have pasted the diff for comments.

   clocks:
+    minItems: 2
     items:
       - description: Bus Clock
       - description: Module Clock
@@ -64,14 +65,26 @@ required:
   - compatible
   - reg
   - interrupts
-  - clocks
-  - clock-names
   - phys
   - phy-names
   - resets
   - vcc-dsi-supply
   - port

+allOf:
+  - if:
+      properties:
+         compatible:
+           contains:
+             const: allwinner,sun6i-a31-mipi-dsi
+      then:
+        properties:
+          clocks:
+            minItems: 2
+        required:
+          - clocks
+          - clock-names
+
 additionalProperties: false

I have marked minItems: 2 on clocks since we need to use minimum of 2
clocks like both bus and mod not mod clock alone.

Please let me know your comments.

Jagan.

Re: [PATCH v10 5/6] arm64: dts: allwinner: a64: Add MIPI DSI pipeline

[Jagan Teki]

On Thu, Oct 17, 2019 at 3:22 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Wed, Oct 16, 2019 at 02:19:44PM +0530, Jagan Teki wrote:
> > On Wed, Oct 16, 2019 at 1:33 PM Maxime Ripard <mripard@kernel.org> wrote:
> > >
> > > On Mon, Oct 14, 2019 at 05:37:50PM +0530, Jagan Teki wrote:
> > > > On Mon, Oct 7, 2019 at 4:27 PM Maxime Ripard <mripard@kernel.org> wrote:
> > > > >
> > > > > On Sat, Oct 05, 2019 at 07:49:12PM +0530, Jagan Teki wrote:
> > > > > > Add MIPI DSI pipeline for Allwinner A64.
> > > > > >
> > > > > > - dsi node, with A64 compatible since it doesn't support
> > > > > >   DSI_SCLK gating unlike A33
> > > > > > - dphy node, with A64 compatible with A33 fallback since
> > > > > >   DPHY on A64 and A33 is similar
> > > > > > - finally, attach the dsi_in to tcon0 for complete MIPI DSI
> > > > > >
> > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > > > > > ---
> > > > > >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++++
> > > > > >  1 file changed, 38 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > index 69128a6dfc46..ad4170b8aee0 100644
> > > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > @@ -382,6 +382,12 @@
> > > > > >                                       #address-cells = <1>;
> > > > > >                                       #size-cells = <0>;
> > > > > >                                       reg = <1>;
> > > > > > +
> > > > > > +                                     tcon0_out_dsi: endpoint@1 {
> > > > > > +                                             reg = <1>;
> > > > > > +                                             remote-endpoint = <&dsi_in_tcon0>;
> > > > > > +                                             allwinner,tcon-channel = <1>;
> > > > > > +                                     };
> > > > > >                               };
> > > > > >                       };
> > > > > >               };
> > > > > > @@ -1003,6 +1009,38 @@
> > > > > >                       status = "disabled";
> > > > > >               };
> > > > > >
> > > > > > +             dsi: dsi@1ca0000 {
> > > > > > +                     compatible = "allwinner,sun50i-a64-mipi-dsi";
> > > > > > +                     reg = <0x01ca0000 0x1000>;
> > > > > > +                     interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > +                     clocks = <&ccu CLK_BUS_MIPI_DSI>;
> > > > > > +                     clock-names = "bus";
> > > > >
> > > > > This won't validate with the bindings you have either here, since it
> > > > > still expects bus and mod.
> > > > >
> > > > > I guess in that cas, we can just drop clock-names, which will require
> > > > > a bit of work on the driver side as well.
> > > >
> > > > Okay.
> > > > mod clock is not required for a64, ie reason we have has_mod_clk quirk
> > > > patch. Adjust the clock-names: on dt-bindings would make sense here,
> > > > what do you think?
> > >
> > > I'm confused, what are you suggesting?
> >
> > Sorry for the confusion.
> >
> > The mod clock is not required for A64 and we have a patch for handling
> > mod clock using has_mod_clk quirk(on the series), indeed the mod clock
> > is available in A31 and not needed for A64. So, to satisfy this
> > requirement the clock-names on dt-bindings can update to make mod
> > clock-name is optional and bus clock is required.
>
> No, the bus clock name is not needed if there's only one clock.

Looks like we need "bus" clock required since the
devm_regmap_init_mmio_clk is created only if bus clock-names added in
dt.

Re: [PATCH v10 5/6] arm64: dts: allwinner: a64: Add MIPI DSI pipeline

[Maxime Ripard]

On Thu, Oct 24, 2019 at 01:28:28PM +0530, Jagan Teki wrote:
> On Thu, Oct 17, 2019 at 3:22 PM Maxime Ripard <mripard@kernel.org> wrote:
> >
> > On Wed, Oct 16, 2019 at 02:19:44PM +0530, Jagan Teki wrote:
> > > On Wed, Oct 16, 2019 at 1:33 PM Maxime Ripard <mripard@kernel.org> wrote:
> > > >
> > > > On Mon, Oct 14, 2019 at 05:37:50PM +0530, Jagan Teki wrote:
> > > > > On Mon, Oct 7, 2019 at 4:27 PM Maxime Ripard <mripard@kernel.org> wrote:
> > > > > >
> > > > > > On Sat, Oct 05, 2019 at 07:49:12PM +0530, Jagan Teki wrote:
> > > > > > > Add MIPI DSI pipeline for Allwinner A64.
> > > > > > >
> > > > > > > - dsi node, with A64 compatible since it doesn't support
> > > > > > >   DSI_SCLK gating unlike A33
> > > > > > > - dphy node, with A64 compatible with A33 fallback since
> > > > > > >   DPHY on A64 and A33 is similar
> > > > > > > - finally, attach the dsi_in to tcon0 for complete MIPI DSI
> > > > > > >
> > > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > > > > > > ---
> > > > > > >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++++
> > > > > > >  1 file changed, 38 insertions(+)
> > > > > > >
> > > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > > index 69128a6dfc46..ad4170b8aee0 100644
> > > > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > > @@ -382,6 +382,12 @@
> > > > > > >                                       #address-cells = <1>;
> > > > > > >                                       #size-cells = <0>;
> > > > > > >                                       reg = <1>;
> > > > > > > +
> > > > > > > +                                     tcon0_out_dsi: endpoint@1 {
> > > > > > > +                                             reg = <1>;
> > > > > > > +                                             remote-endpoint = <&dsi_in_tcon0>;
> > > > > > > +                                             allwinner,tcon-channel = <1>;
> > > > > > > +                                     };
> > > > > > >                               };
> > > > > > >                       };
> > > > > > >               };
> > > > > > > @@ -1003,6 +1009,38 @@
> > > > > > >                       status = "disabled";
> > > > > > >               };
> > > > > > >
> > > > > > > +             dsi: dsi@1ca0000 {
> > > > > > > +                     compatible = "allwinner,sun50i-a64-mipi-dsi";
> > > > > > > +                     reg = <0x01ca0000 0x1000>;
> > > > > > > +                     interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > > +                     clocks = <&ccu CLK_BUS_MIPI_DSI>;
> > > > > > > +                     clock-names = "bus";
> > > > > >
> > > > > > This won't validate with the bindings you have either here, since it
> > > > > > still expects bus and mod.
> > > > > >
> > > > > > I guess in that cas, we can just drop clock-names, which will require
> > > > > > a bit of work on the driver side as well.
> > > > >
> > > > > Okay.
> > > > > mod clock is not required for a64, ie reason we have has_mod_clk quirk
> > > > > patch. Adjust the clock-names: on dt-bindings would make sense here,
> > > > > what do you think?
> > > >
> > > > I'm confused, what are you suggesting?
> > >
> > > Sorry for the confusion.
> > >
> > > The mod clock is not required for A64 and we have a patch for handling
> > > mod clock using has_mod_clk quirk(on the series), indeed the mod clock
> > > is available in A31 and not needed for A64. So, to satisfy this
> > > requirement the clock-names on dt-bindings can update to make mod
> > > clock-name is optional and bus clock is required.
> >
> > No, the bus clock name is not needed if there's only one clock.
>
> Okay, is it because the same clock handle it on PHY side?

No, because there's only one clock and thus you don't need to
differentiate them.

> >
> > > I'm not exactly sure, this is correct but trying to understand if it
> > > is possible or not? something like
> > >
> > >    clocks:
> > >       minItems: 1
> > >       maxItems: 2
> > >      items:
> > >        - description: Bus Clock
> > >        - description: Module Clock
> >
> > That's correct.
> >
> > >    clock-names:
> > >       minItems: 1
> > >       maxItems: 2
> > >      items:
> > >        - const: bus
> > >        - const: mod
> >
> > Here, just keep the current clock-names definition, and make it
> > required only for SoCs that are not the A64
>
> Okay, please have a look here I have pasted the diff for comments.
>
>    clocks:
> +    minItems: 2
>      items:
>        - description: Bus Clock
>        - description: Module Clock

Didn't you tell me that you didn't need the module clock?

How do you handle the case were you just have the bus clock then?

Maxime

Re: [PATCH v10 5/6] arm64: dts: allwinner: a64: Add MIPI DSI pipeline

[Maxime Ripard]

On Thu, Oct 24, 2019 at 06:26:36PM +0530, Jagan Teki wrote:
> On Thu, Oct 17, 2019 at 3:22 PM Maxime Ripard <mripard@kernel.org> wrote:
> >
> > On Wed, Oct 16, 2019 at 02:19:44PM +0530, Jagan Teki wrote:
> > > On Wed, Oct 16, 2019 at 1:33 PM Maxime Ripard <mripard@kernel.org> wrote:
> > > >
> > > > On Mon, Oct 14, 2019 at 05:37:50PM +0530, Jagan Teki wrote:
> > > > > On Mon, Oct 7, 2019 at 4:27 PM Maxime Ripard <mripard@kernel.org> wrote:
> > > > > >
> > > > > > On Sat, Oct 05, 2019 at 07:49:12PM +0530, Jagan Teki wrote:
> > > > > > > Add MIPI DSI pipeline for Allwinner A64.
> > > > > > >
> > > > > > > - dsi node, with A64 compatible since it doesn't support
> > > > > > >   DSI_SCLK gating unlike A33
> > > > > > > - dphy node, with A64 compatible with A33 fallback since
> > > > > > >   DPHY on A64 and A33 is similar
> > > > > > > - finally, attach the dsi_in to tcon0 for complete MIPI DSI
> > > > > > >
> > > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > > > > > > ---
> > > > > > >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++++
> > > > > > >  1 file changed, 38 insertions(+)
> > > > > > >
> > > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > > index 69128a6dfc46..ad4170b8aee0 100644
> > > > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > > @@ -382,6 +382,12 @@
> > > > > > >                                       #address-cells = <1>;
> > > > > > >                                       #size-cells = <0>;
> > > > > > >                                       reg = <1>;
> > > > > > > +
> > > > > > > +                                     tcon0_out_dsi: endpoint@1 {
> > > > > > > +                                             reg = <1>;
> > > > > > > +                                             remote-endpoint = <&dsi_in_tcon0>;
> > > > > > > +                                             allwinner,tcon-channel = <1>;
> > > > > > > +                                     };
> > > > > > >                               };
> > > > > > >                       };
> > > > > > >               };
> > > > > > > @@ -1003,6 +1009,38 @@
> > > > > > >                       status = "disabled";
> > > > > > >               };
> > > > > > >
> > > > > > > +             dsi: dsi@1ca0000 {
> > > > > > > +                     compatible = "allwinner,sun50i-a64-mipi-dsi";
> > > > > > > +                     reg = <0x01ca0000 0x1000>;
> > > > > > > +                     interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > > +                     clocks = <&ccu CLK_BUS_MIPI_DSI>;
> > > > > > > +                     clock-names = "bus";
> > > > > >
> > > > > > This won't validate with the bindings you have either here, since it
> > > > > > still expects bus and mod.
> > > > > >
> > > > > > I guess in that cas, we can just drop clock-names, which will require
> > > > > > a bit of work on the driver side as well.
> > > > >
> > > > > Okay.
> > > > > mod clock is not required for a64, ie reason we have has_mod_clk quirk
> > > > > patch. Adjust the clock-names: on dt-bindings would make sense here,
> > > > > what do you think?
> > > >
> > > > I'm confused, what are you suggesting?
> > >
> > > Sorry for the confusion.
> > >
> > > The mod clock is not required for A64 and we have a patch for handling
> > > mod clock using has_mod_clk quirk(on the series), indeed the mod clock
> > > is available in A31 and not needed for A64. So, to satisfy this
> > > requirement the clock-names on dt-bindings can update to make mod
> > > clock-name is optional and bus clock is required.
> >
> > No, the bus clock name is not needed if there's only one clock.
>
> Looks like we need "bus" clock required since the
> devm_regmap_init_mmio_clk is created only if bus clock-names added in
> dt.

Yeah, hence why I said you'd need "a bit of work on the driver side"

Replacing the clock name by NULL should work.

Maxime

Re: [PATCH v10 5/6] arm64: dts: allwinner: a64: Add MIPI DSI pipeline

[Jagan Teki]

On Fri, Oct 25, 2019 at 12:33 AM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Thu, Oct 24, 2019 at 01:28:28PM +0530, Jagan Teki wrote:
> > On Thu, Oct 17, 2019 at 3:22 PM Maxime Ripard <mripard@kernel.org> wrote:
> > >
> > > On Wed, Oct 16, 2019 at 02:19:44PM +0530, Jagan Teki wrote:
> > > > On Wed, Oct 16, 2019 at 1:33 PM Maxime Ripard <mripard@kernel.org> wrote:
> > > > >
> > > > > On Mon, Oct 14, 2019 at 05:37:50PM +0530, Jagan Teki wrote:
> > > > > > On Mon, Oct 7, 2019 at 4:27 PM Maxime Ripard <mripard@kernel.org> wrote:
> > > > > > >
> > > > > > > On Sat, Oct 05, 2019 at 07:49:12PM +0530, Jagan Teki wrote:
> > > > > > > > Add MIPI DSI pipeline for Allwinner A64.
> > > > > > > >
> > > > > > > > - dsi node, with A64 compatible since it doesn't support
> > > > > > > >   DSI_SCLK gating unlike A33
> > > > > > > > - dphy node, with A64 compatible with A33 fallback since
> > > > > > > >   DPHY on A64 and A33 is similar
> > > > > > > > - finally, attach the dsi_in to tcon0 for complete MIPI DSI
> > > > > > > >
> > > > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > > > > > > > ---
> > > > > > > >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++++
> > > > > > > >  1 file changed, 38 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > > > index 69128a6dfc46..ad4170b8aee0 100644
> > > > > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > > > @@ -382,6 +382,12 @@
> > > > > > > >                                       #address-cells = <1>;
> > > > > > > >                                       #size-cells = <0>;
> > > > > > > >                                       reg = <1>;
> > > > > > > > +
> > > > > > > > +                                     tcon0_out_dsi: endpoint@1 {
> > > > > > > > +                                             reg = <1>;
> > > > > > > > +                                             remote-endpoint = <&dsi_in_tcon0>;
> > > > > > > > +                                             allwinner,tcon-channel = <1>;
> > > > > > > > +                                     };
> > > > > > > >                               };
> > > > > > > >                       };
> > > > > > > >               };
> > > > > > > > @@ -1003,6 +1009,38 @@
> > > > > > > >                       status = "disabled";
> > > > > > > >               };
> > > > > > > >
> > > > > > > > +             dsi: dsi@1ca0000 {
> > > > > > > > +                     compatible = "allwinner,sun50i-a64-mipi-dsi";
> > > > > > > > +                     reg = <0x01ca0000 0x1000>;
> > > > > > > > +                     interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > > > +                     clocks = <&ccu CLK_BUS_MIPI_DSI>;
> > > > > > > > +                     clock-names = "bus";
> > > > > > >
> > > > > > > This won't validate with the bindings you have either here, since it
> > > > > > > still expects bus and mod.
> > > > > > >
> > > > > > > I guess in that cas, we can just drop clock-names, which will require
> > > > > > > a bit of work on the driver side as well.
> > > > > >
> > > > > > Okay.
> > > > > > mod clock is not required for a64, ie reason we have has_mod_clk quirk
> > > > > > patch. Adjust the clock-names: on dt-bindings would make sense here,
> > > > > > what do you think?
> > > > >
> > > > > I'm confused, what are you suggesting?
> > > >
> > > > Sorry for the confusion.
> > > >
> > > > The mod clock is not required for A64 and we have a patch for handling
> > > > mod clock using has_mod_clk quirk(on the series), indeed the mod clock
> > > > is available in A31 and not needed for A64. So, to satisfy this
> > > > requirement the clock-names on dt-bindings can update to make mod
> > > > clock-name is optional and bus clock is required.
> > >
> > > No, the bus clock name is not needed if there's only one clock.
> >
> > Okay, is it because the same clock handle it on PHY side?
>
> No, because there's only one clock and thus you don't need to
> differentiate them.
>
> > >
> > > > I'm not exactly sure, this is correct but trying to understand if it
> > > > is possible or not? something like
> > > >
> > > >    clocks:
> > > >       minItems: 1
> > > >       maxItems: 2
> > > >      items:
> > > >        - description: Bus Clock
> > > >        - description: Module Clock
> > >
> > > That's correct.
> > >
> > > >    clock-names:
> > > >       minItems: 1
> > > >       maxItems: 2
> > > >      items:
> > > >        - const: bus
> > > >        - const: mod
> > >
> > > Here, just keep the current clock-names definition, and make it
> > > required only for SoCs that are not the A64
> >
> > Okay, please have a look here I have pasted the diff for comments.
> >
> >    clocks:
> > +    minItems: 2
> >      items:
> >        - description: Bus Clock
> >        - description: Module Clock
>
> Didn't you tell me that you didn't need the module clock?
>
> How do you handle the case were you just have the bus clock then?

Make sense, it is my mistake then. we don't require to specify here I
think since it implies globally. I think it should be sufficient to
mention on allOf: section based on the SoC like I mentioned in above
snippet.