Skip to contentSkip to conversation

Fixes for omapdrm on OpenPandora and GTA04

PATCH Superseded H. Nikolaus Schaller 6 participants 23 messages Nov 28, 2017 – Dec 11, 2017

Subsystems (7)
Device tree bindings DRM drivers DRM misc Framebuffer layer +3 more subsystems

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

H. Nikolaus Schaller

author
#0 lore raw

v3 cover Fixes for omapdrm on OpenPandora and GTA04

Changes V3:
* stay compatible with old DTB files which still use "toppoly" (suggested by Tomi Valkeinen)
* replaced MODULE_ALIAS entries by MODULE_DEVICE_TABLE (suggested by Andrew F. Davis)
* removed DSI VDDS patch as it has already been accepted

2017-11-16 09:50:22: Changes V2:

* replaced patch to fix DSI VDDS for OMAP3 by equivalent patch from Laurent Pinchart
* keep previous compatibility option in panel driver to handle older device tree binaries

2017-11-08 22:09:36:

This patch set fixes vendor names of the panels and fixes a problem on omapdrm with enabling VDD_DSI for OMAP3 which is needed for displaying the Red and Green channel on OMAP3530 (Pandora).

H. Nikolaus Schaller (4):
omapdrm: panel: fix compatible vendor string for td028ttec1
omapdrm: panel: td028ttec1: replace MODULE_ALIAS by
MODULE_DEVICE_TABLE
DTS: GTA04: fix panel compatibility string
DTS: Pandora: fix panel compatibility string

 .../panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt}    |  4 ++--
 arch/arm/boot/dts/omap3-gta04.dtsi                          |  2 +-
 arch/arm/boot/dts/omap3-pandora-common.dtsi                 |  2 +-
 drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c     | 13 ++++++++++++-
 .../fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c      | 12 +++++++++++-
 5 files changed, 27 insertions(+), 6 deletions(-)
 rename Documentation/devicetree/bindings/display/panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt} (84%)
Signature · 1 line
2.12.2
author
#1 lore raw

PATCH v3 1/4 omapdrm: panel: fix compatible vendor string for td028ttec1

The vendor name was "toppoly" but other panels and the vendor list have defined it as "tpo". So let's fix it in driver and bindings.

We keep the old definition in parallel to stay compatible with potential older DTB setup.

Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 .../display/panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt}      | 4 ++--
 drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c               | 3 +++
 drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c      | 3 +++
 3 files changed, 8 insertions(+), 2 deletions(-)
 rename Documentation/devicetree/bindings/display/panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt} (84%)
Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt renamed from Documentation/devicetree/bindings/display/panel/toppoly,td028ttec1.txt +2 -2
similarity index 84%
rename from Documentation/devicetree/bindings/display/panel/toppoly,td028ttec1.txt
rename to Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt
index 7175dc3740ac..ed34253d9fb1 100644
@@ -2,7 +2,7 @@ Toppoly TD028TTEC1 Panel
 ========================
 
 Required properties:
-- compatible: "toppoly,td028ttec1"
+- compatible: "tpo,td028ttec1"
 
 Optional properties:
 - label: a symbolic name for the panel

@@ -14,7 +14,7 @@ Example
 -------
 
 lcd-panel: td028ttec1@0 {
-	compatible = "toppoly,td028ttec1";
+	compatible = "tpo,td028ttec1";
 	reg = <0>;
 	spi-max-frequency = <100000>;
 	spi-cpol;
drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c +3 -0
index 0a38a0e8c925..a0dfa14f4fab 100644
@@ -452,6 +452,8 @@ static int td028ttec1_panel_remove(struct spi_device *spi)
 }
 
 static const struct of_device_id td028ttec1_of_match[] = {
+	{ .compatible = "omapdss,tpo,td028ttec1", },
+	/* keep to not break older DTB */
 	{ .compatible = "omapdss,toppoly,td028ttec1", },
 	{},
 };

@@ -471,6 +473,7 @@ static struct spi_driver td028ttec1_spi_driver = {
 
 module_spi_driver(td028ttec1_spi_driver);
 
+MODULE_ALIAS("spi:tpo,td028ttec1");
 MODULE_ALIAS("spi:toppoly,td028ttec1");
 MODULE_AUTHOR("H. Nikolaus Schaller <[email protected]>");
 MODULE_DESCRIPTION("Toppoly TD028TTEC1 panel driver");
drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c +3 -0
index 57e9e146ff74..4aeb908f2d1e 100644
@@ -455,6 +455,8 @@ static int td028ttec1_panel_remove(struct spi_device *spi)
 }
 
 static const struct of_device_id td028ttec1_of_match[] = {
+	{ .compatible = "omapdss,tpo,td028ttec1", },
+	/* keep to not break older DTB */
 	{ .compatible = "omapdss,toppoly,td028ttec1", },
 	{},
 };

@@ -474,6 +476,7 @@ static struct spi_driver td028ttec1_spi_driver = {
 
 module_spi_driver(td028ttec1_spi_driver);
 
+MODULE_ALIAS("spi:tpo,td028ttec1");
 MODULE_ALIAS("spi:toppoly,td028ttec1");
 MODULE_AUTHOR("H. Nikolaus Schaller <[email protected]>");
 MODULE_DESCRIPTION("Toppoly TD028TTEC1 panel driver");
Signature · 1 line
2.12.2
maintainer
#2 lore raw

Re: [PATCH v3 1/4] omapdrm: panel: fix compatible vendor string for td028ttec1

Quoted 44 lines · H. Nikolaus Schaller

The vendor name was "toppoly" but other panels and the vendor list have defined it as "tpo". So let's fix it in driver and bindings.

We keep the old definition in parallel to stay compatible with potential older DTB setup.

Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 .../display/panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt}      | 4 ++--
 drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c               | 3 +++
 drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c      | 3 +++
 3 files changed, 8 insertions(+), 2 deletions(-)
 rename Documentation/devicetree/bindings/display/panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt} (84%)
Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt renamed from Documentation/devicetree/bindings/display/panel/toppoly,td028ttec1.txt +2 -2
similarity index 84%
rename from Documentation/devicetree/bindings/display/panel/toppoly,td028ttec1.txt
rename to Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt
index 7175dc3740ac..ed34253d9fb1 100644
@@ -2,7 +2,7 @@ Toppoly TD028TTEC1 Panel
 ========================
 
 Required properties:
-- compatible: "toppoly,td028ttec1"
+- compatible: "tpo,td028ttec1"
 
 Optional properties:
 - label: a symbolic name for the panel

@@ -14,7 +14,7 @@ Example
 -------
 
 lcd-panel: td028ttec1@0 {
-	compatible = "toppoly,td028ttec1";
+	compatible = "tpo,td028ttec1";
 	reg = <0>;
 	spi-max-frequency = <100000>;
 	spi-cpol;
drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c +0 -0
index 0a38a0e8c925..a0dfa14f4fab 100644

drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c static int td028ttec1_panel_remove(struct spi_device *spi) · line 455

Quoted 4 lines
drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c +1 -0
@@ -452,6 +452,8 @@ static int td028ttec1_panel_remove(struct spi_device *spi)
 }
 
 static const struct of_device_id td028ttec1_of_match[] = {
+	{ .compatible = "omapdss,tpo,td028ttec1", },

Why the omapdss part?

author
#3 lore raw

Re: [PATCH v3 1/4] omapdrm: panel: fix compatible vendor string for td028ttec1

Quoted 50 lines

Am 01.12.2017 um 02:57 schrieb Rob Herring <[email protected]>:

Quoted 46 lines · H. Nikolaus Schaller

The vendor name was "toppoly" but other panels and the vendor list have defined it as "tpo". So let's fix it in driver and bindings.

We keep the old definition in parallel to stay compatible with potential older DTB setup.

Signed-off-by H. Nikolaus Schaller <[email protected]>

---

.../display/panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt}      | 4 ++--
drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c               | 3 +++
drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c      | 3 +++
3 files changed, 8 insertions(+), 2 deletions(-)

rename Documentation/devicetree/bindings/display/panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt} (84%)

Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt renamed from Documentation/devicetree/bindings/display/panel/toppoly,td028ttec1.txt +0 -0
similarity index 84%
rename from Documentation/devicetree/bindings/display/panel/toppoly,td028ttec1.txt
rename to Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt
index 7175dc3740ac..ed34253d9fb1 100644
@@ -2,7 +2,7 @@ Toppoly TD028TTEC1 Panel

========================

Required properties:
-- compatible: "toppoly,td028ttec1"
+- compatible: "tpo,td028ttec1"

Optional properties:
- label: a symbolic name for the panel

Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt renamed from Documentation/devicetree/bindings/display/panel/toppoly,td028ttec1.txt +1 -2
@@ -14,7 +14,7 @@ Example
-------
 
 lcd-panel: td028ttec1@0 {
-	compatible = "toppoly,td028ttec1";
+	compatible = "tpo,td028ttec1";
 	reg = <0>;
 	spi-max-frequency = <100000>;
 	spi-cpol;
drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c +0 -0
index 0a38a0e8c925..a0dfa14f4fab 100644
@@ -452,6 +452,8 @@ static int td028ttec1_panel_remove(struct spi_device *spi)
 }
Quoted 4 lines

drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c static int td028ttec1_panel_remove(struct spi_device *spi) · line 455

Quoted 2 lines
drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c +1 -0
 static const struct of_device_id td028ttec1_of_match[] = {
+	{ .compatible = "omapdss,tpo,td028ttec1", },

Why the omapdss part?

I think because they are (currently and still) omapdrm (omapdss) specific since SoC specific drivers for the same panel may exist in parallel.

All panel drivers in drivers/gpu/drm/omapdrm/displays have and need this prefix.

As far as I understand, the omapdss driver takes the DTS compatible string, adds "omapdss," and then looks for a panel driver to probe.

See also: Sebastian Reichel's comment to "[PATCH v3 4/4] DTS: Pandora: fix panel compatibility string"

BR,
Nikolaus Schaller

#4 lore raw

Re: [PATCH v3 1/4] omapdrm: panel: fix compatible vendor string for td028ttec1

In reply to Rob Herring
Quoted 11 lines · Rob Herring

drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c static int td028ttec1_panel_remove(struct spi_device *spi) · line 455

Quoted 9 lines
drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c +1 -0
index 0a38a0e8c925..a0dfa14f4fab 100644
@@ -452,6 +452,8 @@ static int td028ttec1_panel_remove(struct spi_device *spi)
 }
 
 static const struct of_device_id td028ttec1_of_match[] = {
+	{ .compatible = "omapdss,tpo,td028ttec1", },

Why the omapdss part?

This driver is omapdrm specific. But I don't want the dts file to have omapdrm specific compatible strings, so that the dts files do not have to be changed when in the future we move to common DRM panel drivers

So:

- In the .dts, we have compatible = "tpo,td028ttec1"
- At early boot time, we append "omapdss," to compatible strings for panels connected to the DSS node.
- The omapdrm specific drivers match to "omapdss,tpo,td028ttec1"

Tomi

Signature · 2 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
author
#5 lore raw

PATCH v3 2/4 omapdrm: panel: td028ttec1: replace MODULE_ALIAS by MODULE_DEVICE_TABLE

In reply to H. Nikolaus Schaller

to make it easier to keep in sync with the OF device table.

Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c      | 12 ++++++++++--
 .../video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c | 11 +++++++++--
 2 files changed, 19 insertions(+), 4 deletions(-)
drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c +10 -2
index a0dfa14f4fab..2721a86ac5e7 100644
@@ -460,9 +460,19 @@ static const struct of_device_id td028ttec1_of_match[] = {
 
 MODULE_DEVICE_TABLE(of, td028ttec1_of_match);
 
+static const struct spi_device_id td028ttec1_ids[] = {
+	{ "toppoly,td028ttec1", 0 },
+	{ "tpo,td028ttec1", 0},
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(spi, td028ttec1_ids);
+
+
 static struct spi_driver td028ttec1_spi_driver = {
 	.probe		= td028ttec1_panel_probe,
 	.remove		= td028ttec1_panel_remove,
+	.id_table	= td028ttec1_ids,
 
 	.driver         = {
 		.name   = "panel-tpo-td028ttec1",

@@ -473,8 +483,6 @@ static struct spi_driver td028ttec1_spi_driver = {
 
 module_spi_driver(td028ttec1_spi_driver);
 
-MODULE_ALIAS("spi:tpo,td028ttec1");
-MODULE_ALIAS("spi:toppoly,td028ttec1");
 MODULE_AUTHOR("H. Nikolaus Schaller <[email protected]>");
 MODULE_DESCRIPTION("Toppoly TD028TTEC1 panel driver");
 MODULE_LICENSE("GPL");
drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c +9 -2
index 4aeb908f2d1e..f6da8755b859 100644
@@ -463,9 +463,18 @@ static const struct of_device_id td028ttec1_of_match[] = {
 
 MODULE_DEVICE_TABLE(of, td028ttec1_of_match);
 
+static const struct spi_device_id td028ttec1_ids[] = {
+	{ "toppoly,td028ttec1", 0 },
+	{ "tpo,td028ttec1", 0},
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(spi, td028ttec1_ids);
+
 static struct spi_driver td028ttec1_spi_driver = {
 	.probe		= td028ttec1_panel_probe,
 	.remove		= td028ttec1_panel_remove,
+	.id_table	= td028ttec1_ids,
 
 	.driver         = {
 		.name   = "panel-tpo-td028ttec1",

@@ -476,8 +485,6 @@ static struct spi_driver td028ttec1_spi_driver = {
 
 module_spi_driver(td028ttec1_spi_driver);
 
-MODULE_ALIAS("spi:tpo,td028ttec1");
-MODULE_ALIAS("spi:toppoly,td028ttec1");
 MODULE_AUTHOR("H. Nikolaus Schaller <[email protected]>");
 MODULE_DESCRIPTION("Toppoly TD028TTEC1 panel driver");
 MODULE_LICENSE("GPL");
Signature · 6 lines
2.12.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#6 lore raw

PATCH v3 3/4 DTS: GTA04: fix panel compatibility string

In reply to H. Nikolaus Schaller

Official vendor string is now "tpo" and not "toppoly".

Requires patch "omapdrm: panel: fix compatible vendor string for td028ttec1"

Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 arch/arm/boot/dts/omap3-gta04.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/omap3-gta04.dtsi +1 -1
index 4504908c23fe..ec27ed67a22a 100644
@@ -86,7 +86,7 @@
 
 		/* lcd panel */
 		lcd: td028ttec1@0 {
-			compatible = "toppoly,td028ttec1";
+			compatible = "tpo,td028ttec1";
 			reg = <0>;
 			spi-max-frequency = <100000>;
 			spi-cpol;
Signature · 1 line
2.12.2
#7 lore raw

Re: [PATCH v3 3/4] DTS: GTA04: fix panel compatibility string

Quoted 3 lines · H. Nikolaus Schaller

Official vendor string is now "tpo" and not "toppoly".

Requires patch "omapdrm: panel: fix compatible vendor string for td028ttec1"

This is not a fix then, this is a clean up as you change the compatible earlier. Please resend this separately once the driver changes have been merged.

Regards,

Tony

Quoted 21 lines
Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 arch/arm/boot/dts/omap3-gta04.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/omap3-gta04.dtsi +1 -1
index 4504908c23fe..ec27ed67a22a 100644
@@ -86,7 +86,7 @@
 
 		/* lcd panel */
 		lcd: td028ttec1@0 {
-			compatible = "toppoly,td028ttec1";
+			compatible = "tpo,td028ttec1";
 			reg = <0>;
 			spi-max-frequency = <100000>;
 			spi-cpol;
Signature · 1 line
2.12.2
author
#8 lore raw

PATCH v3 4/4 DTS: Pandora: fix panel compatibility string

In reply to H. Nikolaus Schaller

We can remove the unnecessary "omapdss," prefix because the omapdrm driver takes care of it when matching with the driver table.

Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 arch/arm/boot/dts/omap3-pandora-common.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/omap3-pandora-common.dtsi +1 -1
index 53e007abdc71..64d967ec8c58 100644
@@ -626,7 +626,7 @@
 
 	lcd: lcd@1 {
 		reg = <1>;	/* CS1 */
-		compatible =	"omapdss,tpo,td043mtea1";
+		compatible =	"tpo,td043mtea1";
 		spi-max-frequency = <100000>;
 		spi-cpol;
 		spi-cpha;
Signature · 1 line
2.12.2
#9 lore raw

Re: [PATCH v3 4/4] DTS: Pandora: fix panel compatibility string

Quoted 3 lines · H. Nikolaus Schaller

We can remove the unnecessary "omapdss," prefix because the omapdrm driver takes care of it when matching with the driver table.

So is this needed as a fix or is this another clean-up?

So is this is really needed as a fix?

If this is just clean-up, again, please resend once the driver changes have cleared.

Regards,

Tony

Quoted 21 lines
Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 arch/arm/boot/dts/omap3-pandora-common.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/omap3-pandora-common.dtsi +1 -1
index 53e007abdc71..64d967ec8c58 100644
@@ -626,7 +626,7 @@
 
 	lcd: lcd@1 {
 		reg = <1>;	/* CS1 */
-		compatible =	"omapdss,tpo,td043mtea1";
+		compatible =	"tpo,td043mtea1";
 		spi-max-frequency = <100000>;
 		spi-cpol;
 		spi-cpha;
Signature · 1 line
2.12.2
author
#10 lore raw

Re: [PATCH v3 4/4] DTS: Pandora: fix panel compatibility string

Hi Tony,

Quoted 10 lines

Am 28.11.2017 um 17:04 schrieb Tony Lindgren <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

We can remove the unnecessary "omapdss," prefix because the omapdrm driver takes care of it when matching with the driver table.

So is this needed as a fix or is this another clean-up?

So is this is really needed as a fix?

Hm. How do you differentiate between "fix" and "cleanup"? Maybe it is more a wording than a content issue...

For me it is a "fix" because it is semantically wrong to have a prefix where it is not needed. And "fixing" it changes the compiler output by 8 bytes.

"Cleanup" would be for me removing whitespace or empty lines or typos in comments.

Quoted 3 lines

If this is just clean-up, again, please resend once the driver changes have cleared.

There is no change to the pandora driver involved here. The Pandora panel driver is already correct. Just the DTS has some redundant content which should be removed.

So there is no dependency for this patch.

BR,
Nikolaus

Quoted 26 lines

Regards,

Tony

Quoted 21 lines
Signed-off-by H. Nikolaus Schaller <[email protected]>

---
arch/arm/boot/dts/omap3-pandora-common.dtsi | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/omap3-pandora-common.dtsi +1 -1
index 53e007abdc71..64d967ec8c58 100644
@@ -626,7 +626,7 @@
 
 	lcd: lcd@1 {
 		reg = <1>;	/* CS1 */
-		compatible =	"omapdss,tpo,td043mtea1";
+		compatible =	"tpo,td043mtea1";
 		spi-max-frequency = <100000>;
 		spi-cpol;
 		spi-cpha;
Signature · 1 line
2.12.2
#11 lore raw

Re: [PATCH v3 4/4] DTS: Pandora: fix panel compatibility string

Quoted 11 lines · H. Nikolaus Schaller

Hi Tony,

Quoted 9 lines

Am 28.11.2017 um 17:04 schrieb Tony Lindgren <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

We can remove the unnecessary "omapdss," prefix because the omapdrm driver takes care of it when matching with the driver table.

So is this needed as a fix or is this another clean-up?

Quoted 8 lines
Quoted 1 line

So is this is really needed as a fix?

Hm. How do you differentiate between "fix" and "cleanup"? Maybe it is more a wording than a content issue...

For me it is a "fix" because it is semantically wrong to have a prefix where it is not needed. And "fixing" it changes the compiler output by 8 bytes.

How about let's call it a "typo fix" then? :)

Quoted 11 lines

"Cleanup" would be for me removing whitespace or empty lines or typos in comments.

Quoted 2 lines

If this is just clean-up, again, please resend once the driver changes have cleared.

There is no change to the pandora driver involved here. The Pandora panel driver is already correct. Just the DTS has some redundant content which should be removed.

So there is no dependency for this patch.

OK please resend separately after the driver changes have merged then.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#12 lore raw

Re: [PATCH v3 4/4] DTS: Pandora: fix panel compatibility string

Hi,

Quoted 16 lines

Am 28.11.2017 um 17:18 schrieb Tony Lindgren <[email protected]>:

Quoted 13 lines · H. Nikolaus Schaller

Hi Tony,

Quoted 10 lines

Am 28.11.2017 um 17:04 schrieb Tony Lindgren <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

We can remove the unnecessary "omapdss," prefix because the omapdrm driver takes care of it when matching with the driver table.

So is this needed as a fix or is this another clean-up?

So is this is really needed as a fix?

Quoted 8 lines
Quoted 6 lines

Hm. How do you differentiate between "fix" and "cleanup"? Maybe it is more a wording than a content issue...

For me it is a "fix" because it is semantically wrong to have a prefix where it is not needed. And "fixing" it changes the compiler output by 8 bytes.

How about let's call it a "typo fix" then? :)

Well, it is not really a typo.

Quoted 7 lines
Quoted 7 lines

"Cleanup" would be for me removing whitespace or empty lines or typos in comments.

Quoted 2 lines

If this is just clean-up, again, please resend once the driver changes have cleared.

Quoted 8 lines
Quoted 5 lines

There is no change to the pandora driver involved here. The Pandora panel driver is already correct. Just the DTS has some redundant content which should be removed.

So there is no dependency for this patch.

OK please resend separately after the driver changes have merged then.

The Pandora driver does not need an update. Only the DTS. So there is noting to merge or wait for.

Maybe the confusion comes that in both cases (GTA04 and OpenPandora) there are changes to DTS.

But different ones.

GTA04: change vendor prefix to make it consistent
Pandora: remove redundant (unnecessary and potentially wrong) omapdss, prefix

In addition we have to modify the GTA04 panel to handle the correct vendor prefix. The Pandora panel already uses the right one.

Anyways, I think it is now Tomi to decide about the panel driver (vendor prefix) patch 1/4 and 2/4 first. Then we can sort out the DTS changes (3/4 and 4/4).

BR and thanks,
Nikolaus

#13 lore raw

Re: [PATCH v3 4/4] DTS: Pandora: fix panel compatibility string

Quoted 21 lines · H. Nikolaus Schaller

Hi,

Quoted 19 lines

Am 28.11.2017 um 17:18 schrieb Tony Lindgren <[email protected]>:

Quoted 16 lines · H. Nikolaus Schaller

Hi Tony,

Quoted 10 lines

Am 28.11.2017 um 17:04 schrieb Tony Lindgren <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

We can remove the unnecessary "omapdss," prefix because the omapdrm driver takes care of it when matching with the driver table.

So is this needed as a fix or is this another clean-up?

So is this is really needed as a fix?

Hm. How do you differentiate between "fix" and "cleanup"? Maybe it is more a wording than a content issue...

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

For me it is a "fix" because it is semantically wrong to have a prefix where it is not needed. And "fixing" it changes the compiler output by 8 bytes.

How about let's call it a "typo fix" then? :)

Well, it is not really a typo.

Well what if the stable people pick it into earlier stable series based on the word fix in the subject? That has happened before.

I suggest you update the dts patches to use wording like "update compatible to use new naming" or something similar.

Regards,

Tony

#14 lore raw

Re: [PATCH v3 4/4] DTS: Pandora: fix panel compatibility string

Hi,

Quoted 28 lines · Tony Lindgren
Quoted 27 lines · H. Nikolaus Schaller

Hi,

Quoted 24 lines

Am 28.11.2017 um 17:18 schrieb Tony Lindgren <[email protected]>:

Quoted 11 lines · H. Nikolaus Schaller

Hi Tony,

Quoted 9 lines

Am 28.11.2017 um 17:04 schrieb Tony Lindgren <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

We can remove the unnecessary "omapdss," prefix because the omapdrm driver takes care of it when matching with the driver table.

So is this needed as a fix or is this another clean-up?

Quoted 8 lines
Quoted 1 line

So is this is really needed as a fix?

Hm. How do you differentiate between "fix" and "cleanup"? Maybe it is more a wording than a content issue...

For me it is a "fix" because it is semantically wrong to have a prefix where it is not needed. And "fixing" it changes the compiler output by 8 bytes.

How about let's call it a "typo fix" then? :)

Quoted 7 lines
Quoted 1 line

Well, it is not really a typo.

Well what if the stable people pick it into earlier stable series based on the word fix in the subject? That has happened before.

I suggest you update the dts patches to use wording like "update compatible to use new naming" or something similar.

Patch 4/4 is a Fix and should be applied to stable trees. "omapdss," prefix was never supposed to be in the DTS files, is not supposed to be in there now and will break some time in the future.

Explanation: The early init of omapdss adds the prefix at runtime, so that the binding can use generic properties and the kernel can use omapdss specific drivers until the generic ones can be used.

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171130/f6407f70/attachment.sig>

#15 lore raw

Re: [PATCH v3 4/4] DTS: Pandora: fix panel compatibility string

Quoted 10 lines · Sebastian Reichel
Quoted 9 lines
Quoted 3 lines
Quoted 1 line

How about let's call it a "typo fix" then? :)

Well, it is not really a typo.

Well what if the stable people pick it into earlier stable series based on the word fix in the subject? That has happened before.

I suggest you update the dts patches to use wording like "update compatible to use new naming" or something similar.

Quoted 7 lines

Patch 4/4 is a Fix and should be applied to stable trees. "omapdss," prefix was never supposed to be in the DTS files, is not supposed to be in there now and will break some time in the future.

Explanation: The early init of omapdss adds the prefix at runtime, so that the binding can use generic properties and the kernel can use omapdss specific drivers until the generic ones can be used

This is true, but the extra "omapdss" does not cause any issues at the moment, and the time when it causes issues is still many kernel versions in the future.

But, yes, thinking about this, I agree, it's better to pick this one separately as a fix (after rebasing it on top of current maineline so that it doesn't depend on the toppoly name change), and leaving the toppoly->tpo change as a cleanup.

Signature · 2 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
author
#16 lore raw

Re: [PATCH v3 4/4] DTS: Pandora: fix panel compatibility string

Hi Tomi,

Quoted 22 lines

Am 01.12.2017 um 09:13 schrieb Tomi Valkeinen <[email protected]>:

Quoted 10 lines · Sebastian Reichel
Quoted 9 lines
Quoted 3 lines
Quoted 1 line

How about let's call it a "typo fix" then? :)

Well, it is not really a typo.

Well what if the stable people pick it into earlier stable series based on the word fix in the subject? That has happened before.

I suggest you update the dts patches to use wording like "update compatible to use new naming" or something similar.

Quoted 7 lines

Patch 4/4 is a Fix and should be applied to stable trees. "omapdss," prefix was never supposed to be in the DTS files, is not supposed to be in there now and will break some time in the future.

Explanation: The early init of omapdss adds the prefix at runtime, so that the binding can use generic properties and the kernel can use omapdss specific drivers until the generic ones can be used

Quoted 8 lines

This is true, but the extra "omapdss" does not cause any issues at the moment, and the time when it causes issues is still many kernel versions in the future.

But, yes, thinking about this, I agree, it's better to pick this one separately as a fix (after rebasing it on top of current maineline so that it doesn't depend on the toppoly name change), and leaving the toppoly->tpo change as a cleanup.

Just a note: there is no toppoly->tpo change for *this* panel and Pandora board. Just omapdss removal.

The GTA04 needs a toppoly->tpo change but no omapdss, removal.

So they solve different problems and are independent of each other.

GTA04: change vendor string
Pandora: remove omapdss, prefix

BR and thanks,
Nikolaus

Acked-by
#17 lore raw

Re: [PATCH v3 4/4] DTS: Pandora: fix panel compatibility string

Quoted 9 lines · H. Nikolaus Schaller

Just a note: there is no toppoly->tpo change for *this* panel and Pandora board. Just omapdss removal.

The GTA04 needs a toppoly->tpo change but no omapdss, removal.

So they solve different problems and are independent of each other.

GTA04: change vendor string
Pandora: remove omapdss, prefix

Oh, right, I totally missed that. I thought they were changes to the same file...

In that case, Tony, can you pick this one as a fix? I'll pick the toppoly->tpo patch and merge via drm tree, if you give the ack.

For this:

Acked-by Tomi Valkeinen <[email protected]>

Tomi

Signature · 2 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
#18 lore raw

Re: [PATCH v3 4/4] DTS: Pandora: fix panel compatibility string

Quoted 15 lines · Tomi Valkeinen
Quoted 9 lines · H. Nikolaus Schaller

Just a note: there is no toppoly->tpo change for *this* panel and Pandora board. Just omapdss removal.

The GTA04 needs a toppoly->tpo change but no omapdss, removal.

So they solve different problems and are independent of each other.

GTA04: change vendor string
Pandora: remove omapdss, prefix

Oh, right, I totally missed that. I thought they were changes to the same file...

Quoted 6 lines

In that case, Tony, can you pick this one as a fix? I'll pick the toppoly->tpo patch and merge via drm tree, if you give the ack.

For this:

Acked-by Tomi Valkeinen <[email protected]>

Sorry I've lost track of this thread and what if anything is really needed as a fix for v4.16. And now there's newer version of the two dts patches in thread "[PATCH v4 0/2] Fixes for omapdrm on OpenPandora and GTA04" so let's move the discussion there.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#19 lore raw

Re: [PATCH v3 4/4] DTS: Pandora: fix panel compatibility string

In reply to Tony Lindgren
Quoted 29 lines

Am 30.11.2017 um 16:24 schrieb Tony Lindgren <[email protected]>:

Quoted 25 lines · H. Nikolaus Schaller

Hi,

Quoted 23 lines

Am 28.11.2017 um 17:18 schrieb Tony Lindgren <[email protected]>:

Quoted 19 lines · H. Nikolaus Schaller

Hi Tony,

Quoted 10 lines

Am 28.11.2017 um 17:04 schrieb Tony Lindgren <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

We can remove the unnecessary "omapdss," prefix because the omapdrm driver takes care of it when matching with the driver table.

So is this needed as a fix or is this another clean-up?

So is this is really needed as a fix?

Hm. How do you differentiate between "fix" and "cleanup"? Maybe it is more a wording than a content issue...

For me it is a "fix" because it is semantically wrong to have a prefix where it is not needed. And "fixing" it changes the compiler output by 8 bytes.

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

How about let's call it a "typo fix" then? :)

Well, it is not really a typo.

Well what if the stable people pick it into earlier stable series based on the word fix in the subject? That has happened before.

Well, that may happen but IMHO *every* such backport must be checked for reasonability and compatibility and sometimes even modified to apply.

And I think the author of the original patch receives a notification from the stable maintainers and can then veto.

For Example I received "[PATCH 4.7 118/186] w1:omap_hdq: fix regression" "4.7-stable review patch. If anyone has any objections, please let me know."

If that process still fails, we simply have to revert it... Has also happened before. So it will not be end of mankind :)

Quoted 3 lines

I suggest you update the dts patches to use wording like "update compatible to use new naming" or something similar.

I have applied s/fix/improve/ for patch 3/4 and left 4/4 as suggested by Sebastian since it is really a strongly suggested correction.

Since Tomi has already accepted the underlaying driver patch, [PATCH v4] will only have the remaining DTS patches.

BR and thanks,
Nikolaus

#20 lore raw
In reply to H. Nikolaus Schaller
Quoted 22 lines · H. Nikolaus Schaller

Changes V3:
* stay compatible with old DTB files which still use "toppoly" (suggested by Tomi Valkeinen)
* replaced MODULE_ALIAS entries by MODULE_DEVICE_TABLE (suggested by Andrew F. Davis)
* removed DSI VDDS patch as it has already been accepted

2017-11-16 09:50:22: Changes V2:

* replaced patch to fix DSI VDDS for OMAP3 by equivalent patch from Laurent Pinchart
* keep previous compatibility option in panel driver to handle older device tree binaries

2017-11-08 22:09:36:

This patch set fixes vendor names of the panels and fixes a problem on omapdrm with enabling VDD_DSI for OMAP3 which is needed for displaying the Red and Green channel on OMAP3530 (Pandora).

H. Nikolaus Schaller (4):
omapdrm: panel: fix compatible vendor string for td028ttec1
omapdrm: panel: td028ttec1: replace MODULE_ALIAS by
MODULE_DEVICE_TABLE
DTS: GTA04: fix panel compatibility string
DTS: Pandora: fix panel compatibility string

Quoted 8 lines
 .../panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt}    |  4 ++--
 arch/arm/boot/dts/omap3-gta04.dtsi                          |  2 +-
 arch/arm/boot/dts/omap3-pandora-common.dtsi                 |  2 +-
 drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c     | 13 ++++++++++++-
 .../fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c      | 12 +++++++++++-
 5 files changed, 27 insertions(+), 6 deletions(-)
 rename Documentation/devicetree/bindings/display/panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt} (84%)

Thanks. I have picked up patches 1 and 2.

3 can be applied when 1 & 2 are in. The change in 4 could be applied independently, but it conflicts with 3.

Tony, how do you want to handle 3 and 4? I will push 1 and 2 to v4.16. I don't think they are real issues, so I don't see a reason to push them as fixes to v4.15. I think they are mostly just cleanups, and we might as well wait until v4.17, but that's quite far away...

Tomi

Signature · 2 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
#21 lore raw
Quoted 34 lines · Tomi Valkeinen
Quoted 22 lines · H. Nikolaus Schaller

Changes V3:
* stay compatible with old DTB files which still use "toppoly" (suggested by Tomi Valkeinen)
* replaced MODULE_ALIAS entries by MODULE_DEVICE_TABLE (suggested by Andrew F. Davis)
* removed DSI VDDS patch as it has already been accepted

2017-11-16 09:50:22: Changes V2:

* replaced patch to fix DSI VDDS for OMAP3 by equivalent patch from Laurent Pinchart
* keep previous compatibility option in panel driver to handle older device tree binaries

2017-11-08 22:09:36:

This patch set fixes vendor names of the panels and fixes a problem on omapdrm with enabling VDD_DSI for OMAP3 which is needed for displaying the Red and Green channel on OMAP3530 (Pandora).

H. Nikolaus Schaller (4):
omapdrm: panel: fix compatible vendor string for td028ttec1
omapdrm: panel: td028ttec1: replace MODULE_ALIAS by
MODULE_DEVICE_TABLE
DTS: GTA04: fix panel compatibility string
DTS: Pandora: fix panel compatibility string

Quoted 8 lines
 .../panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt}    |  4 ++--
 arch/arm/boot/dts/omap3-gta04.dtsi                          |  2 +-
 arch/arm/boot/dts/omap3-pandora-common.dtsi                 |  2 +-
 drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c     | 13 ++++++++++++-
 .../fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c      | 12 +++++++++++-
 5 files changed, 27 insertions(+), 6 deletions(-)
 rename Documentation/devicetree/bindings/display/panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt} (84%)

Thanks. I have picked up patches 1 and 2.

Quoted 7 lines

3 can be applied when 1 & 2 are in. The change in 4 could be applied independently, but it conflicts with 3.

Tony, how do you want to handle 3 and 4? I will push 1 and 2 to v4.16. I don't think they are real issues, so I don't see a reason to push them as fixes to v4.15. I think they are mostly just cleanups, and we might as well wait until v4.17, but that's quite far away...

In general to avoid the huge hassle of cross tree dependencies and confusion if the dts changes are fixes, I suggest the following:

1. Nikolaus reposts the dts changes to not say word "fix" in them and use "update to use new binding" or something similar to avoid patches wrongly getting picked into earlier stable trees

2. Then I can ack the patches as they are just oneliners and unlikely to conflict with anything else

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#22 lore raw
In reply to Tomi Valkeinen
Quoted 32 lines · Tomi Valkeinen
Quoted 22 lines · H. Nikolaus Schaller

Changes V3:
* stay compatible with old DTB files which still use "toppoly" (suggested by Tomi Valkeinen)
* replaced MODULE_ALIAS entries by MODULE_DEVICE_TABLE (suggested by Andrew F. Davis)
* removed DSI VDDS patch as it has already been accepted

2017-11-16 09:50:22: Changes V2:

* replaced patch to fix DSI VDDS for OMAP3 by equivalent patch from Laurent Pinchart
* keep previous compatibility option in panel driver to handle older device tree binaries

2017-11-08 22:09:36:

This patch set fixes vendor names of the panels and fixes a problem on omapdrm with enabling VDD_DSI for OMAP3 which is needed for displaying the Red and Green channel on OMAP3530 (Pandora).

H. Nikolaus Schaller (4):
omapdrm: panel: fix compatible vendor string for td028ttec1
omapdrm: panel: td028ttec1: replace MODULE_ALIAS by
MODULE_DEVICE_TABLE
DTS: GTA04: fix panel compatibility string
DTS: Pandora: fix panel compatibility string

Quoted 8 lines
 .../panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt}    |  4 ++--
 arch/arm/boot/dts/omap3-gta04.dtsi                          |  2 +-
 arch/arm/boot/dts/omap3-pandora-common.dtsi                 |  2 +-
 drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c     | 13 ++++++++++++-
 .../fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c      | 12 +++++++++++-
 5 files changed, 27 insertions(+), 6 deletions(-)
 rename Documentation/devicetree/bindings/display/panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt} (84%)
Quoted 1 line

Thanks. I have picked up patches 1 and 2.

Thanks for taking care of them (they both look fine to me).

Quoted 7 lines

3 can be applied when 1 & 2 are in. The change in 4 could be applied independently, but it conflicts with 3.

Tony, how do you want to handle 3 and 4? I will push 1 and 2 to v4.16. I don't think they are real issues, so I don't see a reason to push them as fixes to v4.15. I think they are mostly just cleanups, and we might as well wait until v4.17, but that's quite far away...

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel