[PATCH] drm: sti: fix sub-components bind

[PATCH] drm: sti: fix sub-components bind

[Benjamin Gaignard]

Fix misunderstanding in how use component framework.
drm_platform_init() is now call only when all the
sub-components are register themselves instead of the
previous two stages mechanism.

Update devicetree and bindings documentation according
to this new behavior.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 .../devicetree/bindings/gpu/st,stih4xx.txt         |  7 +-
 arch/arm/boot/dts/stih407.dtsi                     | 82 +++++++++++-----------
 arch/arm/boot/dts/stih410.dtsi                     | 82 +++++++++++-----------
 drivers/gpu/drm/sti/sti_drm_drv.c                  | 45 ++----------
 drivers/gpu/drm/sti/sti_hdmi.c                     | 25 ++++---
 drivers/gpu/drm/sti/sti_tvout.c                    | 46 ++----------
 6 files changed, 105 insertions(+), 182 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/st,stih4xx.txt b/Documentation/devicetree/bindings/gpu/st,stih4xx.txt
index 6b1d75f..efd23a1 100644
--- a/Documentation/devicetree/bindings/gpu/st,stih4xx.txt
+++ b/Documentation/devicetree/bindings/gpu/st,stih4xx.txt
@@ -52,10 +52,9 @@ STMicroelectronics stih4xx platforms
     See ../reset/reset.txt for details.
   - reset-names: names of the resets listed in resets property in the same
     order.
-  - ranges: to allow probing of subdevices
 
 - sti-hdmi: hdmi output block
-  must be a child of sti-tvout
+  must be a child of sti-display-subsystem
   Required properties:
   - compatible: "st,stih<chip>-hdmi";
   - reg: Physical base address of the IP registers and length of memory mapped region.
@@ -72,7 +71,7 @@ STMicroelectronics stih4xx platforms
 
 sti-hda:
   Required properties:
-  must be a child of sti-tvout
+  must be a child of sti-display-subsystem
   - compatible: "st,stih<chip>-hda"
   - reg: Physical base address of the IP registers and length of memory mapped region.
   - reg-names: names of the mapped memory regions listed in regs property in
@@ -85,7 +84,7 @@ sti-hda:
 
 sti-dvo:
   Required properties:
-  must be a child of sti-tvout
+  must be a child of sti-display-subsystem
   - compatible: "st,stih<chip>-dvo"
   - reg: Physical base address of the IP registers and length of memory mapped region.
   - reg-names: names of the mapped memory regions listed in regs property in
diff --git a/arch/arm/boot/dts/stih407.dtsi b/arch/arm/boot/dts/stih407.dtsi
index 3efa3b2..6b914e4 100644
--- a/arch/arm/boot/dts/stih407.dtsi
+++ b/arch/arm/boot/dts/stih407.dtsi
@@ -103,48 +103,46 @@
 							 <&clk_s_d0_quadfs 0>,
 							 <&clk_s_d2_quadfs 0>,
 							 <&clk_s_d2_quadfs 0>;
-				ranges;
-
-				sti-hdmi@8d04000 {
-					compatible = "st,stih407-hdmi";
-					reg = <0x8d04000 0x1000>;
-					reg-names = "hdmi-reg";
-					interrupts = <GIC_SPI 106 IRQ_TYPE_NONE>;
-					interrupt-names	= "irq";
-					clock-names = "pix",
-						      "tmds",
-						      "phy",
-						      "audio",
-						      "main_parent",
-						      "aux_parent";
-
-					clocks = <&clk_s_d2_flexgen CLK_PIX_HDMI>,
-						 <&clk_s_d2_flexgen CLK_TMDS_HDMI>,
-						 <&clk_s_d2_flexgen CLK_REF_HDMIPHY>,
-						 <&clk_s_d0_flexgen CLK_PCM_0>,
-						 <&clk_s_d2_quadfs 0>,
-						 <&clk_s_d2_quadfs 1>;
-
-					hdmi,hpd-gpio = <&pio5 3>;
-					reset-names = "hdmi";
-					resets = <&softreset STIH407_HDMI_TX_PHY_SOFTRESET>;
-					ddc = <&hdmiddc>;
-
-				};
-
-				sti-hda@8d02000 {
-					compatible = "st,stih407-hda";
-					reg = <0x8d02000 0x400>, <0x92b0120 0x4>;
-					reg-names = "hda-reg", "video-dacs-ctrl";
-					clock-names = "pix",
-						      "hddac",
-						      "main_parent",
-						      "aux_parent";
-					clocks = <&clk_s_d2_flexgen CLK_PIX_HDDAC>,
-						 <&clk_s_d2_flexgen CLK_HDDAC>,
-						 <&clk_s_d2_quadfs 0>,
-						 <&clk_s_d2_quadfs 1>;
-				};
+			};
+
+			sti-hdmi@8d04000 {
+				compatible = "st,stih407-hdmi";
+				reg = <0x8d04000 0x1000>;
+				reg-names = "hdmi-reg";
+				interrupts = <GIC_SPI 106 IRQ_TYPE_NONE>;
+				interrupt-names	= "irq";
+				clock-names = "pix",
+					      "tmds",
+					      "phy",
+					      "audio",
+					      "main_parent",
+					      "aux_parent";
+
+				clocks = <&clk_s_d2_flexgen CLK_PIX_HDMI>,
+					 <&clk_s_d2_flexgen CLK_TMDS_HDMI>,
+					 <&clk_s_d2_flexgen CLK_REF_HDMIPHY>,
+					 <&clk_s_d0_flexgen CLK_PCM_0>,
+					 <&clk_s_d2_quadfs 0>,
+					 <&clk_s_d2_quadfs 1>;
+
+				hdmi,hpd-gpio = <&pio5 3>;
+				reset-names = "hdmi";
+				resets = <&softreset STIH407_HDMI_TX_PHY_SOFTRESET>;
+				ddc = <&hdmiddc>;
+			};
+
+			sti-hda@8d02000 {
+				compatible = "st,stih407-hda";
+				reg = <0x8d02000 0x400>, <0x92b0120 0x4>;
+				reg-names = "hda-reg", "video-dacs-ctrl";
+				clock-names = "pix",
+					      "hddac",
+					      "main_parent",
+					      "aux_parent";
+				clocks = <&clk_s_d2_flexgen CLK_PIX_HDDAC>,
+					 <&clk_s_d2_flexgen CLK_HDDAC>,
+					 <&clk_s_d2_quadfs 0>,
+					 <&clk_s_d2_quadfs 1>;
 			};
 		};
 	};
diff --git a/arch/arm/boot/dts/stih410.dtsi b/arch/arm/boot/dts/stih410.dtsi
index 208b5e8..bd1d66e 100644
--- a/arch/arm/boot/dts/stih410.dtsi
+++ b/arch/arm/boot/dts/stih410.dtsi
@@ -174,48 +174,46 @@
 							 <&clk_s_d0_quadfs 0>,
 							 <&clk_s_d2_quadfs 0>,
 							 <&clk_s_d2_quadfs 0>;
-				ranges;
-
-				sti-hdmi@8d04000 {
-					compatible = "st,stih407-hdmi";
-					reg = <0x8d04000 0x1000>;
-					reg-names = "hdmi-reg";
-					interrupts = <GIC_SPI 106 IRQ_TYPE_NONE>;
-					interrupt-names	= "irq";
-					clock-names = "pix",
-						      "tmds",
-						      "phy",
-						      "audio",
-						      "main_parent",
-						      "aux_parent";
-
-					clocks = <&clk_s_d2_flexgen CLK_PIX_HDMI>,
-						 <&clk_s_d2_flexgen CLK_TMDS_HDMI>,
-						 <&clk_s_d2_flexgen CLK_REF_HDMIPHY>,
-						 <&clk_s_d0_flexgen CLK_PCM_0>,
-						 <&clk_s_d2_quadfs 0>,
-						 <&clk_s_d2_quadfs 1>;
-
-					hdmi,hpd-gpio = <&pio5 3>;
-					reset-names = "hdmi";
-					resets = <&softreset STIH407_HDMI_TX_PHY_SOFTRESET>;
-					ddc = <&hdmiddc>;
-
-				};
-
-				sti-hda@8d02000 {
-					compatible = "st,stih407-hda";
-					reg = <0x8d02000 0x400>, <0x92b0120 0x4>;
-					reg-names = "hda-reg", "video-dacs-ctrl";
-					clock-names = "pix",
-						      "hddac",
-						      "main_parent",
-						      "aux_parent";
-					clocks = <&clk_s_d2_flexgen CLK_PIX_HDDAC>,
-						 <&clk_s_d2_flexgen CLK_HDDAC>,
-						 <&clk_s_d2_quadfs 0>,
-						 <&clk_s_d2_quadfs 1>;
-				};
+			};
+
+			sti-hdmi@8d04000 {
+				compatible = "st,stih407-hdmi";
+				reg = <0x8d04000 0x1000>;
+				reg-names = "hdmi-reg";
+				interrupts = <GIC_SPI 106 IRQ_TYPE_NONE>;
+				interrupt-names	= "irq";
+				clock-names = "pix",
+					      "tmds",
+					      "phy",
+					      "audio",
+					      "main_parent",
+					      "aux_parent";
+
+				clocks = <&clk_s_d2_flexgen CLK_PIX_HDMI>,
+					 <&clk_s_d2_flexgen CLK_TMDS_HDMI>,
+					 <&clk_s_d2_flexgen CLK_REF_HDMIPHY>,
+					 <&clk_s_d0_flexgen CLK_PCM_0>,
+					 <&clk_s_d2_quadfs 0>,
+					 <&clk_s_d2_quadfs 1>;
+
+				hdmi,hpd-gpio = <&pio5 3>;
+				reset-names = "hdmi";
+				resets = <&softreset STIH407_HDMI_TX_PHY_SOFTRESET>;
+				ddc = <&hdmiddc>;
+			};
+
+			sti-hda@8d02000 {
+				compatible = "st,stih407-hda";
+				reg = <0x8d02000 0x400>, <0x92b0120 0x4>;
+				reg-names = "hda-reg", "video-dacs-ctrl";
+				clock-names = "pix",
+					      "hddac",
+					      "main_parent",
+					      "aux_parent";
+				clocks = <&clk_s_d2_flexgen CLK_PIX_HDDAC>,
+					 <&clk_s_d2_flexgen CLK_HDDAC>,
+					 <&clk_s_d2_quadfs 0>,
+					 <&clk_s_d2_quadfs 1>;
 			};
 		};
 	};
diff --git a/drivers/gpu/drm/sti/sti_drm_drv.c b/drivers/gpu/drm/sti/sti_drm_drv.c
index d0fb54a..60be6cd 100644
--- a/drivers/gpu/drm/sti/sti_drm_drv.c
+++ b/drivers/gpu/drm/sti/sti_drm_drv.c
@@ -245,15 +245,17 @@ static const struct component_master_ops sti_drm_ops = {
 	.unbind = sti_drm_unbind,
 };
 
-static int sti_drm_master_probe(struct platform_device *pdev)
+static int sti_drm_platform_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *node = dev->parent->of_node;
+	struct device_node *node = dev->of_node;
 	struct device_node *child_np;
 	struct component_match *match = NULL;
 
 	dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
 
+	of_platform_populate(node, NULL, NULL, dev);
+
 	child_np = of_get_next_available_child(node, NULL);
 
 	while (child_np) {
@@ -265,46 +267,11 @@ static int sti_drm_master_probe(struct platform_device *pdev)
 	return component_master_add_with_match(dev, &sti_drm_ops, match);
 }
 
-static int sti_drm_master_remove(struct platform_device *pdev)
-{
-	component_master_del(&pdev->dev, &sti_drm_ops);
-	return 0;
-}
-
-static struct platform_driver sti_drm_master_driver = {
-	.probe = sti_drm_master_probe,
-	.remove = sti_drm_master_remove,
-	.driver = {
-		.name = DRIVER_NAME "__master",
-	},
-};
-
-static int sti_drm_platform_probe(struct platform_device *pdev)
-{
-	struct device *dev = &pdev->dev;
-	struct device_node *node = dev->of_node;
-	struct platform_device *master;
-
-	of_platform_populate(node, NULL, NULL, dev);
-
-	platform_driver_register(&sti_drm_master_driver);
-	master = platform_device_register_resndata(dev,
-			DRIVER_NAME "__master", -1,
-			NULL, 0, NULL, 0);
-	if (IS_ERR(master))
-               return PTR_ERR(master);
-
-	platform_set_drvdata(pdev, master);
-	return 0;
-}
-
 static int sti_drm_platform_remove(struct platform_device *pdev)
 {
-	struct platform_device *master = platform_get_drvdata(pdev);
-
+	component_master_del(&pdev->dev, &sti_drm_ops);
 	of_platform_depopulate(&pdev->dev);
-	platform_device_unregister(master);
-	platform_driver_unregister(&sti_drm_master_driver);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index f28a4d5..06595e9 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -693,21 +693,8 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
 	struct sti_hdmi_connector *connector;
 	struct drm_connector *drm_connector;
 	struct drm_bridge *bridge;
-	struct device_node *ddc;
 	int err;
 
-	ddc = of_parse_phandle(dev->of_node, "ddc", 0);
-	if (ddc) {
-		hdmi->ddc_adapt = of_find_i2c_adapter_by_node(ddc);
-		if (!hdmi->ddc_adapt) {
-			err = -EPROBE_DEFER;
-			of_node_put(ddc);
-			return err;
-		}
-
-		of_node_put(ddc);
-	}
-
 	/* Set the drm device handle */
 	hdmi->drm_dev = drm_dev;
 
@@ -796,6 +783,7 @@ static int sti_hdmi_probe(struct platform_device *pdev)
 	struct sti_hdmi *hdmi;
 	struct device_node *np = dev->of_node;
 	struct resource *res;
+	struct device_node *ddc;
 	int ret;
 
 	DRM_INFO("%s\n", __func__);
@@ -804,6 +792,17 @@ static int sti_hdmi_probe(struct platform_device *pdev)
 	if (!hdmi)
 		return -ENOMEM;
 
+	ddc = of_parse_phandle(pdev->dev.of_node, "ddc", 0);
+	if (ddc) {
+		hdmi->ddc_adapt = of_find_i2c_adapter_by_node(ddc);
+		if (!hdmi->ddc_adapt) {
+			of_node_put(ddc);
+			return -EPROBE_DEFER;
+		}
+
+		of_node_put(ddc);
+	}
+
 	hdmi->dev = pdev->dev;
 
 	/* Get resources */
diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c
index 5cc5311..576b5be 100644
--- a/drivers/gpu/drm/sti/sti_tvout.c
+++ b/drivers/gpu/drm/sti/sti_tvout.c
@@ -644,7 +644,6 @@ static int sti_tvout_bind(struct device *dev, struct device *master, void *data)
 	struct sti_tvout *tvout = dev_get_drvdata(dev);
 	struct drm_device *drm_dev = data;
 	unsigned int i;
-	int ret;
 
 	tvout->drm_dev = drm_dev;
 
@@ -658,17 +657,15 @@ static int sti_tvout_bind(struct device *dev, struct device *master, void *data)
 
 	sti_tvout_create_encoders(drm_dev, tvout);
 
-	ret = component_bind_all(dev, drm_dev);
-	if (ret)
-		sti_tvout_destroy_encoders(tvout);
-
-	return ret;
+	return 0;
 }
 
 static void sti_tvout_unbind(struct device *dev, struct device *master,
 	void *data)
 {
-	/* do nothing */
+	struct sti_tvout *tvout = dev_get_drvdata(dev);
+
+	sti_tvout_destroy_encoders(tvout);
 }
 
 static const struct component_ops sti_tvout_ops = {
@@ -676,34 +673,12 @@ static const struct component_ops sti_tvout_ops = {
 	.unbind	= sti_tvout_unbind,
 };
 
-static int compare_of(struct device *dev, void *data)
-{
-	return dev->of_node == data;
-}
-
-static int sti_tvout_master_bind(struct device *dev)
-{
-	return 0;
-}
-
-static void sti_tvout_master_unbind(struct device *dev)
-{
-	/* do nothing */
-}
-
-static const struct component_master_ops sti_tvout_master_ops = {
-	.bind = sti_tvout_master_bind,
-	.unbind = sti_tvout_master_unbind,
-};
-
 static int sti_tvout_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *node = dev->of_node;
 	struct sti_tvout *tvout;
 	struct resource *res;
-	struct device_node *child_np;
-	struct component_match *match = NULL;
 
 	DRM_INFO("%s\n", __func__);
 
@@ -734,24 +709,11 @@ static int sti_tvout_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, tvout);
 
-	of_platform_populate(node, NULL, NULL, dev);
-
-	child_np = of_get_next_available_child(node, NULL);
-
-	while (child_np) {
-		component_match_add(dev, &match, compare_of, child_np);
-		of_node_put(child_np);
-		child_np = of_get_next_available_child(node, child_np);
-	}
-
-	component_master_add_with_match(dev, &sti_tvout_master_ops, match);
-
 	return component_add(dev, &sti_tvout_ops);
 }
 
 static int sti_tvout_remove(struct platform_device *pdev)
 {
-	component_master_del(&pdev->dev, &sti_tvout_master_ops);
 	component_del(&pdev->dev, &sti_tvout_ops);
 	return 0;
 }
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: sti: fix sub-components bind

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1050 bytes --]

On Wed, Jul 15, 2015 at 03:21:46PM +0200, Benjamin Gaignard wrote:
> Fix misunderstanding in how use component framework.
> drm_platform_init() is now call only when all the
> sub-components are register themselves instead of the
> previous two stages mechanism.
> 
> Update devicetree and bindings documentation according
> to this new behavior.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> ---
>  .../devicetree/bindings/gpu/st,stih4xx.txt         |  7 +-
>  arch/arm/boot/dts/stih407.dtsi                     | 82 +++++++++++-----------
>  arch/arm/boot/dts/stih410.dtsi                     | 82 +++++++++++-----------
>  drivers/gpu/drm/sti/sti_drm_drv.c                  | 45 ++----------
>  drivers/gpu/drm/sti/sti_hdmi.c                     | 25 ++++---
>  drivers/gpu/drm/sti/sti_tvout.c                    | 46 ++----------
>  6 files changed, 105 insertions(+), 182 deletions(-)

Isn't this going to break DT ABI? How are you going to ensure backwards-
compatibility with old DTBs?

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: sti: fix sub-components bind

[Benjamin Gaignard]

It doesn't change any bindings fields, only remove one level of childs on DT.
Old DTBs may not work but it will impact only very few peoples and no
products so it isn't a problem.
The patch fix driver and DT files so I don't think it could create issues.


2015-07-15 15:34 GMT+02:00 Thierry Reding <thierry.reding@gmail.com>:
> On Wed, Jul 15, 2015 at 03:21:46PM +0200, Benjamin Gaignard wrote:
>> Fix misunderstanding in how use component framework.
>> drm_platform_init() is now call only when all the
>> sub-components are register themselves instead of the
>> previous two stages mechanism.
>>
>> Update devicetree and bindings documentation according
>> to this new behavior.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> ---
>>  .../devicetree/bindings/gpu/st,stih4xx.txt         |  7 +-
>>  arch/arm/boot/dts/stih407.dtsi                     | 82 +++++++++++-----------
>>  arch/arm/boot/dts/stih410.dtsi                     | 82 +++++++++++-----------
>>  drivers/gpu/drm/sti/sti_drm_drv.c                  | 45 ++----------
>>  drivers/gpu/drm/sti/sti_hdmi.c                     | 25 ++++---
>>  drivers/gpu/drm/sti/sti_tvout.c                    | 46 ++----------
>>  6 files changed, 105 insertions(+), 182 deletions(-)
>
> Isn't this going to break DT ABI? How are you going to ensure backwards-
> compatibility with old DTBs?
>
> Thierry



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: sti: fix sub-components bind

[Xinwei Kong]

hi ben:

your patch is ok, i don't know your hardware how to use. In this first
code why use that style dts? If you detail research our patch, you will
find that it can compatiable your fixing dts.

If you don't approve our patch better, I will be glad to see you to slove
this bug

Thank you
Xinwei

On 2015/7/15 21:34, Thierry Reding wrote:
> On Wed, Jul 15, 2015 at 03:21:46PM +0200, Benjamin Gaignard wrote:
>> Fix misunderstanding in how use component framework.
>> drm_platform_init() is now call only when all the
>> sub-components are register themselves instead of the
>> previous two stages mechanism.
>>
>> Update devicetree and bindings documentation according
>> to this new behavior.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> ---
>>  .../devicetree/bindings/gpu/st,stih4xx.txt         |  7 +-
>>  arch/arm/boot/dts/stih407.dtsi                     | 82 +++++++++++-----------
>>  arch/arm/boot/dts/stih410.dtsi                     | 82 +++++++++++-----------
>>  drivers/gpu/drm/sti/sti_drm_drv.c                  | 45 ++----------
>>  drivers/gpu/drm/sti/sti_hdmi.c                     | 25 ++++---
>>  drivers/gpu/drm/sti/sti_tvout.c                    | 46 ++----------
>>  6 files changed, 105 insertions(+), 182 deletions(-)
> 
> Isn't this going to break DT ABI? How are you going to ensure backwards-
> compatibility with old DTBs?
> 
> Thierry
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: sti: fix sub-components bind

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2405 bytes --]

On Wed, Jul 15, 2015 at 03:56:41PM +0200, Benjamin Gaignard wrote:
> It doesn't change any bindings fields, only remove one level of childs on DT.
> Old DTBs may not work but it will impact only very few peoples and no
> products so it isn't a problem.

I know, that can be said of most platforms, but the decision was made to
consider DT a stable ABI, so you can't just go and break it. You'll have
to take this up with the ARM SoC maintainers. In the past they've been
known to request people to go through extra pain to avoid breaking DT
backwards-compatibility.

> The patch fix driver and DT files so I don't think it could create issues.

That doesn't count. Somebody could still be using an old DTB and not be
able (or unwilling) to reflash it.

Irrespective, you're probably going to want to split up your patch into
driver and DTS changes. The DTS and binding changes will need to be
reviewed by the device tree maintainers, and I'd expect that the STi
maintainers will want to weigh in as well.

Thierry

> 2015-07-15 15:34 GMT+02:00 Thierry Reding <thierry.reding@gmail.com>:
> > On Wed, Jul 15, 2015 at 03:21:46PM +0200, Benjamin Gaignard wrote:
> >> Fix misunderstanding in how use component framework.
> >> drm_platform_init() is now call only when all the
> >> sub-components are register themselves instead of the
> >> previous two stages mechanism.
> >>
> >> Update devicetree and bindings documentation according
> >> to this new behavior.
> >>
> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> >> ---
> >>  .../devicetree/bindings/gpu/st,stih4xx.txt         |  7 +-
> >>  arch/arm/boot/dts/stih407.dtsi                     | 82 +++++++++++-----------
> >>  arch/arm/boot/dts/stih410.dtsi                     | 82 +++++++++++-----------
> >>  drivers/gpu/drm/sti/sti_drm_drv.c                  | 45 ++----------
> >>  drivers/gpu/drm/sti/sti_hdmi.c                     | 25 ++++---
> >>  drivers/gpu/drm/sti/sti_tvout.c                    | 46 ++----------
> >>  6 files changed, 105 insertions(+), 182 deletions(-)
> >
> > Isn't this going to break DT ABI? How are you going to ensure backwards-
> > compatibility with old DTBs?
> >
> > Thierry
> 
> 
> 
> -- 
> Benjamin Gaignard
> 
> Graphic Working Group
> 
> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro: Facebook | Twitter | Blog

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

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: sti: fix sub-components bind

[Benjamin Gaignard]

Maxime, as STi DT maintainer, could you give us your point of view ?

If needed I could split the patch in two: one for driver and one for DT.

2015-07-16 12:59 GMT+02:00 Thierry Reding <thierry.reding@gmail.com>:
> On Wed, Jul 15, 2015 at 03:56:41PM +0200, Benjamin Gaignard wrote:
>> It doesn't change any bindings fields, only remove one level of childs on DT.
>> Old DTBs may not work but it will impact only very few peoples and no
>> products so it isn't a problem.
>
> I know, that can be said of most platforms, but the decision was made to
> consider DT a stable ABI, so you can't just go and break it. You'll have
> to take this up with the ARM SoC maintainers. In the past they've been
> known to request people to go through extra pain to avoid breaking DT
> backwards-compatibility.
>
>> The patch fix driver and DT files so I don't think it could create issues.
>
> That doesn't count. Somebody could still be using an old DTB and not be
> able (or unwilling) to reflash it.
>
> Irrespective, you're probably going to want to split up your patch into
> driver and DTS changes. The DTS and binding changes will need to be
> reviewed by the device tree maintainers, and I'd expect that the STi
> maintainers will want to weigh in as well.
>
> Thierry
>
>> 2015-07-15 15:34 GMT+02:00 Thierry Reding <thierry.reding@gmail.com>:
>> > On Wed, Jul 15, 2015 at 03:21:46PM +0200, Benjamin Gaignard wrote:
>> >> Fix misunderstanding in how use component framework.
>> >> drm_platform_init() is now call only when all the
>> >> sub-components are register themselves instead of the
>> >> previous two stages mechanism.
>> >>
>> >> Update devicetree and bindings documentation according
>> >> to this new behavior.
>> >>
>> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> >> ---
>> >>  .../devicetree/bindings/gpu/st,stih4xx.txt         |  7 +-
>> >>  arch/arm/boot/dts/stih407.dtsi                     | 82 +++++++++++-----------
>> >>  arch/arm/boot/dts/stih410.dtsi                     | 82 +++++++++++-----------
>> >>  drivers/gpu/drm/sti/sti_drm_drv.c                  | 45 ++----------
>> >>  drivers/gpu/drm/sti/sti_hdmi.c                     | 25 ++++---
>> >>  drivers/gpu/drm/sti/sti_tvout.c                    | 46 ++----------
>> >>  6 files changed, 105 insertions(+), 182 deletions(-)
>> >
>> > Isn't this going to break DT ABI? How are you going to ensure backwards-
>> > compatibility with old DTBs?
>> >
>> > Thierry
>>
>>
>>
>> --
>> Benjamin Gaignard
>>
>> Graphic Working Group
>>
>> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro: Facebook | Twitter | Blog



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: sti: fix sub-components bind

[Maxime Coquelin]

Hi Benji,

On 07/16/2015 03:08 PM, Benjamin Gaignard wrote:
> Maxime, as STi DT maintainer, could you give us your point of view ?
I agree these DT nodes are not used in products today, and only used by 
very few people.
These people use aligned version of DT and Kernel, so I think we can 
break the ABI to fix this issue.
>
> If needed I could split the patch in two: one for driver and one for DT.
Yes please.


Regards,
Maxime
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: sti: fix sub-components bind

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 675 bytes --]

On Fri, Jul 17, 2015 at 11:02:26AM +0200, Maxime Coquelin wrote:
> Hi Benji,
> 
> On 07/16/2015 03:08 PM, Benjamin Gaignard wrote:
> >Maxime, as STi DT maintainer, could you give us your point of view ?
> I agree these DT nodes are not used in products today, and only used by very
> few people.
> These people use aligned version of DT and Kernel, so I think we can break
> the ABI to fix this issue.

Like I said, you'll have to take this up with the ARM SoC maintainers.
Please Cc them (and the device tree mailing list) on the binding changes
so that everybody has a chance to object to this if they think breaking
compatibility isn't warranted.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: sti: fix sub-components bind

[Maxime Coquelin]

On 07/17/2015 11:12 AM, Thierry Reding wrote:
> On Fri, Jul 17, 2015 at 11:02:26AM +0200, Maxime Coquelin wrote:
>> Hi Benji,
>>
>> On 07/16/2015 03:08 PM, Benjamin Gaignard wrote:
>>> Maxime, as STi DT maintainer, could you give us your point of view ?
>> I agree these DT nodes are not used in products today, and only used by very
>> few people.
>> These people use aligned version of DT and Kernel, so I think we can break
>> the ABI to fix this issue.
> Like I said, you'll have to take this up with the ARM SoC maintainers.
> Please Cc them (and the device tree mailing list) on the binding changes
> so that everybody has a chance to object to this if they think breaking
> compatibility isn't warranted.
Sure.

Benjamin, can you send a v2 split in two patches and put Arnd, Olof and 
Kevin in Cc?

Regards,
Maxime
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel