All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH v2 1/3] video: ssd1307fb: Use gpiod_set_value_cansleep() for reset
@ 2018-09-27  9:24   ` Michal Vokáč
  0 siblings, 0 replies; 27+ messages in thread
From: Michal Vokáč @ 2018-09-27  9:24 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Shawn Guo
  Cc: Fabio Estevam, Rob Herring, devicetree, linux-kernel,
	linux-fbdev, Michal Vokáč

The reset signal can be produced by GPIO expander that can sleep.
In that case the probe function fails. Allow using GPIO expanders for
the reset signal by using the non-atomic gpiod_set_value_cansleep()
function.

Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
v2 changes: add R-by from Fabio

 drivers/video/fbdev/ssd1307fb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index ba66c02..e7ae135 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -728,9 +728,9 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	if (par->reset) {
 		/* Reset the screen */
-		gpiod_set_value(par->reset, 0);
+		gpiod_set_value_cansleep(par->reset, 0);
 		udelay(4);
-		gpiod_set_value(par->reset, 1);
+		gpiod_set_value_cansleep(par->reset, 1);
 		udelay(4);
 	}
 
-- 
2.1.4


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

* [RESEND PATCH v2 1/3] video: ssd1307fb: Use gpiod_set_value_cansleep() for reset
@ 2018-09-27  9:24   ` Michal Vokáč
  0 siblings, 0 replies; 27+ messages in thread
From: Michal Vokáč @ 2018-09-27  9:24 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Shawn Guo
  Cc: Fabio Estevam, Rob Herring, devicetree, linux-kernel,
	linux-fbdev, Michal Vokáč

The reset signal can be produced by GPIO expander that can sleep.
In that case the probe function fails. Allow using GPIO expanders for
the reset signal by using the non-atomic gpiod_set_value_cansleep()
function.

Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
v2 changes: add R-by from Fabio

 drivers/video/fbdev/ssd1307fb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index ba66c02..e7ae135 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -728,9 +728,9 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	if (par->reset) {
 		/* Reset the screen */
-		gpiod_set_value(par->reset, 0);
+		gpiod_set_value_cansleep(par->reset, 0);
 		udelay(4);
-		gpiod_set_value(par->reset, 1);
+		gpiod_set_value_cansleep(par->reset, 1);
 		udelay(4);
 	}
 
-- 
2.1.4

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

* [RESEND PATCH v2 2/3] video: ssd1307fb: Do not hard code active-low reset sequence
  2018-09-27  9:24   ` Michal Vokáč
@ 2018-09-27  9:24     ` Michal Vokáč
  -1 siblings, 0 replies; 27+ messages in thread
From: Michal Vokáč @ 2018-09-27  9:24 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Shawn Guo
  Cc: Fabio Estevam, Rob Herring, devicetree, linux-kernel,
	linux-fbdev, Michal Vokáč

The SSD130x OLED display reset signal is active low. Now the reset
sequence is implemented in such a way that users are forced to
define reset-gpios as GPIO_ACTIVE_HIGH in DT to make the reset work.

Do not hard code the active-low sequence into the driver but instead
allow the user to specify the gpio as GPIO_ACTIVE_LOW to reflect
the real world.

Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
v2 changes: Split the DT changes into separate patch.

 drivers/video/fbdev/ssd1307fb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index e7ae135..7b5bc42 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -728,10 +728,10 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	if (par->reset) {
 		/* Reset the screen */
-		gpiod_set_value_cansleep(par->reset, 0);
-		udelay(4);
 		gpiod_set_value_cansleep(par->reset, 1);
 		udelay(4);
+		gpiod_set_value_cansleep(par->reset, 0);
+		udelay(4);
 	}
 
 	if (par->vbat_reg) {
-- 
2.1.4


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

* [RESEND PATCH v2 2/3] video: ssd1307fb: Do not hard code active-low reset sequence
@ 2018-09-27  9:24     ` Michal Vokáč
  0 siblings, 0 replies; 27+ messages in thread
From: Michal Vokáč @ 2018-09-27  9:24 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Shawn Guo
  Cc: Fabio Estevam, Rob Herring, devicetree, linux-kernel,
	linux-fbdev, Michal Vokáč

The SSD130x OLED display reset signal is active low. Now the reset
sequence is implemented in such a way that users are forced to
define reset-gpios as GPIO_ACTIVE_HIGH in DT to make the reset work.

Do not hard code the active-low sequence into the driver but instead
allow the user to specify the gpio as GPIO_ACTIVE_LOW to reflect
the real world.

Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
v2 changes: Split the DT changes into separate patch.

 drivers/video/fbdev/ssd1307fb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index e7ae135..7b5bc42 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -728,10 +728,10 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	if (par->reset) {
 		/* Reset the screen */
-		gpiod_set_value_cansleep(par->reset, 0);
-		udelay(4);
 		gpiod_set_value_cansleep(par->reset, 1);
 		udelay(4);
+		gpiod_set_value_cansleep(par->reset, 0);
+		udelay(4);
 	}
 
 	if (par->vbat_reg) {
-- 
2.1.4

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

* [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
  2018-09-27  9:24   ` Michal Vokáč
@ 2018-09-27  9:24     ` Michal Vokáč
  -1 siblings, 0 replies; 27+ messages in thread
From: Michal Vokáč @ 2018-09-27  9:24 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Shawn Guo
  Cc: Fabio Estevam, Rob Herring, devicetree, linux-kernel,
	linux-fbdev, Michal Vokáč

The reset signal of the SSD1306 OLED display is actually active-low.
Adapt the DT to reflect the real world.

Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
v2 changes: New patch in the series

 arch/arm/boot/dts/imx28-cfa10036.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx28-cfa10036.dts b/arch/arm/boot/dts/imx28-cfa10036.dts
index e54f5ab..be3406e 100644
--- a/arch/arm/boot/dts/imx28-cfa10036.dts
+++ b/arch/arm/boot/dts/imx28-cfa10036.dts
@@ -11,6 +11,7 @@
 
 /dts-v1/;
 #include "imx28.dtsi"
+#include <dt-bindings/gpio/gpio.h>
 
 / {
 	model = "Crystalfontz CFA-10036 Board";
@@ -95,7 +96,7 @@
 					pinctrl-names = "default";
 					pinctrl-0 = <&ssd1306_cfa10036>;
 					reg = <0x3c>;
-					reset-gpios = <&gpio2 7 0>;
+					reset-gpios = <&gpio2 7 GPIO_ACTIVE_LOW>;
 					solomon,height = <32>;
 					solomon,width = <128>;
 					solomon,page-offset = <0>;
-- 
2.1.4


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

* [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
@ 2018-09-27  9:24     ` Michal Vokáč
  0 siblings, 0 replies; 27+ messages in thread
From: Michal Vokáč @ 2018-09-27  9:24 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Shawn Guo
  Cc: Fabio Estevam, Rob Herring, devicetree, linux-kernel,
	linux-fbdev, Michal Vokáč

The reset signal of the SSD1306 OLED display is actually active-low.
Adapt the DT to reflect the real world.

Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
v2 changes: New patch in the series

 arch/arm/boot/dts/imx28-cfa10036.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx28-cfa10036.dts b/arch/arm/boot/dts/imx28-cfa10036.dts
index e54f5ab..be3406e 100644
--- a/arch/arm/boot/dts/imx28-cfa10036.dts
+++ b/arch/arm/boot/dts/imx28-cfa10036.dts
@@ -11,6 +11,7 @@
 
 /dts-v1/;
 #include "imx28.dtsi"
+#include <dt-bindings/gpio/gpio.h>
 
 / {
 	model = "Crystalfontz CFA-10036 Board";
@@ -95,7 +96,7 @@
 					pinctrl-names = "default";
 					pinctrl-0 = <&ssd1306_cfa10036>;
 					reg = <0x3c>;
-					reset-gpios = <&gpio2 7 0>;
+					reset-gpios = <&gpio2 7 GPIO_ACTIVE_LOW>;
 					solomon,height = <32>;
 					solomon,width = <128>;
 					solomon,page-offset = <0>;
-- 
2.1.4

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

* Re: [RESEND PATCH v2 1/3] video: ssd1307fb: Use gpiod_set_value_cansleep() for reset
  2018-09-27  9:24   ` Michal Vokáč
@ 2018-10-08 10:53     ` Bartlomiej Zolnierkiewicz
  -1 siblings, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-10-08 10:53 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Shawn Guo, Fabio Estevam, Rob Herring, devicetree, linux-kernel,
	linux-fbdev


On 09/27/2018 11:24 AM, Michal Vokáč wrote:
> The reset signal can be produced by GPIO expander that can sleep.
> In that case the probe function fails. Allow using GPIO expanders for
> the reset signal by using the non-atomic gpiod_set_value_cansleep()
> function.
> 
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Patch queued for 4.20, thanks.

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

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

* Re: [RESEND PATCH v2 1/3] video: ssd1307fb: Use gpiod_set_value_cansleep() for reset
@ 2018-10-08 10:53     ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-10-08 10:53 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Shawn Guo, Fabio Estevam, Rob Herring, devicetree, linux-kernel,
	linux-fbdev


On 09/27/2018 11:24 AM, Michal Vokáč wrote:
> The reset signal can be produced by GPIO expander that can sleep.
> In that case the probe function fails. Allow using GPIO expanders for
> the reset signal by using the non-atomic gpiod_set_value_cansleep()
> function.
> 
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Patch queued for 4.20, thanks.

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

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

* Re: [RESEND PATCH v2 2/3] video: ssd1307fb: Do not hard code active-low reset sequence
  2018-09-27  9:24     ` Michal Vokáč
@ 2018-10-08 10:53       ` Bartlomiej Zolnierkiewicz
  -1 siblings, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-10-08 10:53 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Shawn Guo, Fabio Estevam, Rob Herring, devicetree, linux-kernel,
	linux-fbdev


On 09/27/2018 11:24 AM, Michal Vokáč wrote:
> The SSD130x OLED display reset signal is active low. Now the reset
> sequence is implemented in such a way that users are forced to
> define reset-gpios as GPIO_ACTIVE_HIGH in DT to make the reset work.
> 
> Do not hard code the active-low sequence into the driver but instead
> allow the user to specify the gpio as GPIO_ACTIVE_LOW to reflect
> the real world.
> 
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>

Patch queued for 4.20, thanks.

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

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

* Re: [RESEND PATCH v2 2/3] video: ssd1307fb: Do not hard code active-low reset sequence
@ 2018-10-08 10:53       ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-10-08 10:53 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Shawn Guo, Fabio Estevam, Rob Herring, devicetree, linux-kernel,
	linux-fbdev


On 09/27/2018 11:24 AM, Michal Vokáč wrote:
> The SSD130x OLED display reset signal is active low. Now the reset
> sequence is implemented in such a way that users are forced to
> define reset-gpios as GPIO_ACTIVE_HIGH in DT to make the reset work.
> 
> Do not hard code the active-low sequence into the driver but instead
> allow the user to specify the gpio as GPIO_ACTIVE_LOW to reflect
> the real world.
> 
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>

Patch queued for 4.20, thanks.

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

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
  2018-09-27  9:24     ` Michal Vokáč
@ 2018-10-08 11:45       ` Vokáč Michal
  -1 siblings, 0 replies; 27+ messages in thread
From: Vokáč Michal @ 2018-10-08 11:45 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Shawn Guo
  Cc: Fabio Estevam, Rob Herring, devicetree, linux-kernel, linux-fbdev

On 27.9.2018 11:24, Michal Vokáč wrote:
> The reset signal of the SSD1306 OLED display is actually active-low.
> Adapt the DT to reflect the real world.
> 
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> ---
> v2 changes: New patch in the series

Bartlomiej just queued the first two patches for v4.20.
Will somebody take this one? Otherwise this SoM will end up with
broken OLED display reset.

Thanks, Michal.

> 
>   arch/arm/boot/dts/imx28-cfa10036.dts | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/imx28-cfa10036.dts b/arch/arm/boot/dts/imx28-cfa10036.dts
> index e54f5ab..be3406e 100644
> --- a/arch/arm/boot/dts/imx28-cfa10036.dts
> +++ b/arch/arm/boot/dts/imx28-cfa10036.dts
> @@ -11,6 +11,7 @@
>   
>   /dts-v1/;
>   #include "imx28.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
>   
>   / {
>   	model = "Crystalfontz CFA-10036 Board";
> @@ -95,7 +96,7 @@
>   					pinctrl-names = "default";
>   					pinctrl-0 = <&ssd1306_cfa10036>;
>   					reg = <0x3c>;
> -					reset-gpios = <&gpio2 7 0>;
> +					reset-gpios = <&gpio2 7 GPIO_ACTIVE_LOW>;
>   					solomon,height = <32>;
>   					solomon,width = <128>;
>   					solomon,page-offset = <0>;
> 

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
@ 2018-10-08 11:45       ` Vokáč Michal
  0 siblings, 0 replies; 27+ messages in thread
From: Vokáč Michal @ 2018-10-08 11:45 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Shawn Guo
  Cc: Fabio Estevam, Rob Herring, devicetree, linux-kernel, linux-fbdev

T24gMjcuOS4yMDE4IDExOjI0LCBNaWNoYWwgVm9rw6HEjSB3cm90ZToNCj4gVGhlIHJlc2V0IHNp
Z25hbCBvZiB0aGUgU1NEMTMwNiBPTEVEIGRpc3BsYXkgaXMgYWN0dWFsbHkgYWN0aXZlLWxvdy4N
Cj4gQWRhcHQgdGhlIERUIHRvIHJlZmxlY3QgdGhlIHJlYWwgd29ybGQuDQo+IA0KPiBTaWduZWQt
b2ZmLWJ5OiBNaWNoYWwgVm9rw6HEjSA8bWljaGFsLnZva2FjQHlzb2Z0LmNvbT4NCj4gLS0tDQo+
IHYyIGNoYW5nZXM6IE5ldyBwYXRjaCBpbiB0aGUgc2VyaWVzDQoNCkJhcnRsb21pZWoganVzdCBx
dWV1ZWQgdGhlIGZpcnN0IHR3byBwYXRjaGVzIGZvciB2NC4yMC4NCldpbGwgc29tZWJvZHkgdGFr
ZSB0aGlzIG9uZT8gT3RoZXJ3aXNlIHRoaXMgU29NIHdpbGwgZW5kIHVwIHdpdGgNCmJyb2tlbiBP
TEVEIGRpc3BsYXkgcmVzZXQuDQoNClRoYW5rcywgTWljaGFsLg0KDQo+IA0KPiAgIGFyY2gvYXJt
L2Jvb3QvZHRzL2lteDI4LWNmYTEwMDM2LmR0cyB8IDMgKystDQo+ICAgMSBmaWxlIGNoYW5nZWQs
IDIgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL2FyY2gv
YXJtL2Jvb3QvZHRzL2lteDI4LWNmYTEwMDM2LmR0cyBiL2FyY2gvYXJtL2Jvb3QvZHRzL2lteDI4
LWNmYTEwMDM2LmR0cw0KPiBpbmRleCBlNTRmNWFiLi5iZTM0MDZlIDEwMDY0NA0KPiAtLS0gYS9h
cmNoL2FybS9ib290L2R0cy9pbXgyOC1jZmExMDAzNi5kdHMNCj4gKysrIGIvYXJjaC9hcm0vYm9v
dC9kdHMvaW14MjgtY2ZhMTAwMzYuZHRzDQo+IEBAIC0xMSw2ICsxMSw3IEBADQo+ICAgDQo+ICAg
L2R0cy12MS87DQo+ICAgI2luY2x1ZGUgImlteDI4LmR0c2kiDQo+ICsjaW5jbHVkZSA8ZHQtYmlu
ZGluZ3MvZ3Bpby9ncGlvLmg+DQo+ICAgDQo+ICAgLyB7DQo+ICAgCW1vZGVsID0gIkNyeXN0YWxm
b250eiBDRkEtMTAwMzYgQm9hcmQiOw0KPiBAQCAtOTUsNyArOTYsNyBAQA0KPiAgIAkJCQkJcGlu
Y3RybC1uYW1lcyA9ICJkZWZhdWx0IjsNCj4gICAJCQkJCXBpbmN0cmwtMCA9IDwmc3NkMTMwNl9j
ZmExMDAzNj47DQo+ICAgCQkJCQlyZWcgPSA8MHgzYz47DQo+IC0JCQkJCXJlc2V0LWdwaW9zID0g
PCZncGlvMiA3IDA+Ow0KPiArCQkJCQlyZXNldC1ncGlvcyA9IDwmZ3BpbzIgNyBHUElPX0FDVElW
RV9MT1c+Ow0KPiAgIAkJCQkJc29sb21vbixoZWlnaHQgPSA8MzI+Ow0KPiAgIAkJCQkJc29sb21v
bix3aWR0aCA9IDwxMjg+Ow0KPiAgIAkJCQkJc29sb21vbixwYWdlLW9mZnNldCA9IDwwPjsNCj4g
DQo

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
  2018-10-08 11:45       ` Vokáč Michal
@ 2018-10-09  0:20         ` Shawn Guo
  -1 siblings, 0 replies; 27+ messages in thread
From: Shawn Guo @ 2018-10-09  0:20 UTC (permalink / raw)
  To: Vokáč Michal
  Cc: Bartlomiej Zolnierkiewicz, Fabio Estevam, Rob Herring,
	devicetree, linux-kernel, linux-fbdev

On Mon, Oct 08, 2018 at 11:45:36AM +0000, Vokáč Michal wrote:
> On 27.9.2018 11:24, Michal Vokáč wrote:
> > The reset signal of the SSD1306 OLED display is actually active-low.
> > Adapt the DT to reflect the real world.
> > 
> > Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> > ---
> > v2 changes: New patch in the series
> 
> Bartlomiej just queued the first two patches for v4.20.
> Will somebody take this one? Otherwise this SoM will end up with
> broken OLED display reset.

Well, it means the change breaks the ABI between kernel and device tree,
e.g. the new kernel will not work with existing/installed DTBs.

Shawn

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
@ 2018-10-09  0:20         ` Shawn Guo
  0 siblings, 0 replies; 27+ messages in thread
From: Shawn Guo @ 2018-10-09  0:20 UTC (permalink / raw)
  To: Vokáč Michal
  Cc: Bartlomiej Zolnierkiewicz, Fabio Estevam, Rob Herring,
	devicetree, linux-kernel, linux-fbdev

On Mon, Oct 08, 2018 at 11:45:36AM +0000, Vokáč Michal wrote:
> On 27.9.2018 11:24, Michal Vokáč wrote:
> > The reset signal of the SSD1306 OLED display is actually active-low.
> > Adapt the DT to reflect the real world.
> > 
> > Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> > ---
> > v2 changes: New patch in the series
> 
> Bartlomiej just queued the first two patches for v4.20.
> Will somebody take this one? Otherwise this SoM will end up with
> broken OLED display reset.

Well, it means the change breaks the ABI between kernel and device tree,
e.g. the new kernel will not work with existing/installed DTBs.

Shawn

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
  2018-10-09  0:20         ` Shawn Guo
@ 2018-10-09  7:51           ` Bartlomiej Zolnierkiewicz
  -1 siblings, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-10-09  7:51 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Vokáč Michal, Fabio Estevam, Rob Herring, devicetree,
	linux-kernel, linux-fbdev


On 10/09/2018 02:20 AM, Shawn Guo wrote:
> On Mon, Oct 08, 2018 at 11:45:36AM +0000, Vokáč Michal wrote:
>> On 27.9.2018 11:24, Michal Vokáč wrote:
>>> The reset signal of the SSD1306 OLED display is actually active-low.
>>> Adapt the DT to reflect the real world.
>>>
>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>>> ---
>>> v2 changes: New patch in the series
>>
>> Bartlomiej just queued the first two patches for v4.20.
>> Will somebody take this one? Otherwise this SoM will end up with
>> broken OLED display reset.
> 
> Well, it means the change breaks the ABI between kernel and device tree,
> e.g. the new kernel will not work with existing/installed DTBs.

Should I revert this sdd10307fb patch then?

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

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
@ 2018-10-09  7:51           ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-10-09  7:51 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Vokáč Michal, Fabio Estevam, Rob Herring, devicetree,
	linux-kernel, linux-fbdev


On 10/09/2018 02:20 AM, Shawn Guo wrote:
> On Mon, Oct 08, 2018 at 11:45:36AM +0000, Vokáč Michal wrote:
>> On 27.9.2018 11:24, Michal Vokáč wrote:
>>> The reset signal of the SSD1306 OLED display is actually active-low.
>>> Adapt the DT to reflect the real world.
>>>
>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>>> ---
>>> v2 changes: New patch in the series
>>
>> Bartlomiej just queued the first two patches for v4.20.
>> Will somebody take this one? Otherwise this SoM will end up with
>> broken OLED display reset.
> 
> Well, it means the change breaks the ABI between kernel and device tree,
> e.g. the new kernel will not work with existing/installed DTBs.

Should I revert this sdd10307fb patch then?

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

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
  2018-10-09  7:51           ` Bartlomiej Zolnierkiewicz
@ 2018-10-09  8:29             ` Vokáč Michal
  -1 siblings, 0 replies; 27+ messages in thread
From: Vokáč Michal @ 2018-10-09  8:29 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Shawn Guo
  Cc: Fabio Estevam, Rob Herring, devicetree, linux-kernel, linux-fbdev

On 9.10.2018 09:51, Bartlomiej Zolnierkiewicz wrote:
> 
> On 10/09/2018 02:20 AM, Shawn Guo wrote:
>> On Mon, Oct 08, 2018 at 11:45:36AM +0000, Vokáč Michal wrote:
>>> On 27.9.2018 11:24, Michal Vokáč wrote:
>>>> The reset signal of the SSD1306 OLED display is actually active-low.
>>>> Adapt the DT to reflect the real world.
>>>>
>>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>>>> ---
>>>> v2 changes: New patch in the series
>>>
>>> Bartlomiej just queued the first two patches for v4.20.
>>> Will somebody take this one? Otherwise this SoM will end up with
>>> broken OLED display reset.
>>
>> Well, it means the change breaks the ABI between kernel and device tree,
>> e.g. the new kernel will not work with existing/installed DTBs.
> 
> Should I revert this sdd10307fb patch then?

Sorry for the inconvenience :( Lesson learned..

So in other words (no offense): broken drivers need to stay broken because
users may already get used to the broken behavior?

Personally I would not describe this change as a device tree ABI change.
It is not a change in the DT binding. Or are "stable device tree API" and
"device tree ABI" really two different things?

The first patch should be OK though.

Michal

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
@ 2018-10-09  8:29             ` Vokáč Michal
  0 siblings, 0 replies; 27+ messages in thread
From: Vokáč Michal @ 2018-10-09  8:29 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Shawn Guo
  Cc: Fabio Estevam, Rob Herring, devicetree, linux-kernel, linux-fbdev

T24gOS4xMC4yMDE4IDA5OjUxLCBCYXJ0bG9taWVqIFpvbG5pZXJraWV3aWN6IHdyb3RlOg0KPiAN
Cj4gT24gMTAvMDkvMjAxOCAwMjoyMCBBTSwgU2hhd24gR3VvIHdyb3RlOg0KPj4gT24gTW9uLCBP
Y3QgMDgsIDIwMTggYXQgMTE6NDU6MzZBTSArMDAwMCwgVm9rw6HEjSBNaWNoYWwgd3JvdGU6DQo+
Pj4gT24gMjcuOS4yMDE4IDExOjI0LCBNaWNoYWwgVm9rw6HEjSB3cm90ZToNCj4+Pj4gVGhlIHJl
c2V0IHNpZ25hbCBvZiB0aGUgU1NEMTMwNiBPTEVEIGRpc3BsYXkgaXMgYWN0dWFsbHkgYWN0aXZl
LWxvdy4NCj4+Pj4gQWRhcHQgdGhlIERUIHRvIHJlZmxlY3QgdGhlIHJlYWwgd29ybGQuDQo+Pj4+
DQo+Pj4+IFNpZ25lZC1vZmYtYnk6IE1pY2hhbCBWb2vDocSNIDxtaWNoYWwudm9rYWNAeXNvZnQu
Y29tPg0KPj4+PiAtLS0NCj4+Pj4gdjIgY2hhbmdlczogTmV3IHBhdGNoIGluIHRoZSBzZXJpZXMN
Cj4+Pg0KPj4+IEJhcnRsb21pZWoganVzdCBxdWV1ZWQgdGhlIGZpcnN0IHR3byBwYXRjaGVzIGZv
ciB2NC4yMC4NCj4+PiBXaWxsIHNvbWVib2R5IHRha2UgdGhpcyBvbmU/IE90aGVyd2lzZSB0aGlz
IFNvTSB3aWxsIGVuZCB1cCB3aXRoDQo+Pj4gYnJva2VuIE9MRUQgZGlzcGxheSByZXNldC4NCj4+
DQo+PiBXZWxsLCBpdCBtZWFucyB0aGUgY2hhbmdlIGJyZWFrcyB0aGUgQUJJIGJldHdlZW4ga2Vy
bmVsIGFuZCBkZXZpY2UgdHJlZSwNCj4+IGUuZy4gdGhlIG5ldyBrZXJuZWwgd2lsbCBub3Qgd29y
ayB3aXRoIGV4aXN0aW5nL2luc3RhbGxlZCBEVEJzLg0KPiANCj4gU2hvdWxkIEkgcmV2ZXJ0IHRo
aXMgc2RkMTAzMDdmYiBwYXRjaCB0aGVuPw0KDQpTb3JyeSBmb3IgdGhlIGluY29udmVuaWVuY2Ug
OiggTGVzc29uIGxlYXJuZWQuLg0KDQpTbyBpbiBvdGhlciB3b3JkcyAobm8gb2ZmZW5zZSk6IGJy
b2tlbiBkcml2ZXJzIG5lZWQgdG8gc3RheSBicm9rZW4gYmVjYXVzZQ0KdXNlcnMgbWF5IGFscmVh
ZHkgZ2V0IHVzZWQgdG8gdGhlIGJyb2tlbiBiZWhhdmlvcj8NCg0KUGVyc29uYWxseSBJIHdvdWxk
IG5vdCBkZXNjcmliZSB0aGlzIGNoYW5nZSBhcyBhIGRldmljZSB0cmVlIEFCSSBjaGFuZ2UuDQpJ
dCBpcyBub3QgYSBjaGFuZ2UgaW4gdGhlIERUIGJpbmRpbmcuIE9yIGFyZSAic3RhYmxlIGRldmlj
ZSB0cmVlIEFQSSIgYW5kDQoiZGV2aWNlIHRyZWUgQUJJIiByZWFsbHkgdHdvIGRpZmZlcmVudCB0
aGluZ3M/DQoNClRoZSBmaXJzdCBwYXRjaCBzaG91bGQgYmUgT0sgdGhvdWdoLg0KDQpNaWNoYWwN
Cg=

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
  2018-10-09  8:29             ` Vokáč Michal
  (?)
@ 2018-10-09 12:36               ` Fabio Estevam
  -1 siblings, 0 replies; 27+ messages in thread
From: Fabio Estevam @ 2018-10-09 12:36 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Bartlomiej Zolnierkiewicz, Shawn Guo, Fabio Estevam, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, linux-fbdev

Hi Michal,

On Tue, Oct 9, 2018 at 5:30 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote:

> Sorry for the inconvenience :( Lesson learned..
>
> So in other words (no offense): broken drivers need to stay broken because
> users may already get used to the broken behavior?

In order to keep the old dtb's working you could introduce a new
property (like reset-gpio-active-low, for example).

Then the driver behavior can be made untouched for the old dtb's and
only new dtb's with this new property would have the correct GPIO
reset behavior.

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
@ 2018-10-09 12:36               ` Fabio Estevam
  0 siblings, 0 replies; 27+ messages in thread
From: Fabio Estevam @ 2018-10-09 12:36 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Bartlomiej Zolnierkiewicz, Shawn Guo, Fabio Estevam, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, linux-fbdev

Hi Michal,

On Tue, Oct 9, 2018 at 5:30 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote:

> Sorry for the inconvenience :( Lesson learned..
>
> So in other words (no offense): broken drivers need to stay broken because
> users may already get used to the broken behavior?

In order to keep the old dtb's working you could introduce a new
property (like reset-gpio-active-low, for example).

Then the driver behavior can be made untouched for the old dtb's and
only new dtb's with this new property would have the correct GPIO
reset behavior.

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
@ 2018-10-09 12:36               ` Fabio Estevam
  0 siblings, 0 replies; 27+ messages in thread
From: Fabio Estevam @ 2018-10-09 12:36 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Bartlomiej Zolnierkiewicz, Shawn Guo, Fabio Estevam, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, linux-fbdev

Hi Michal,

On Tue, Oct 9, 2018 at 5:30 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote:

> Sorry for the inconvenience :( Lesson learned..
>
> So in other words (no offense): broken drivers need to stay broken because
> users may already get used to the broken behavior?

In order to keep the old dtb's working you could introduce a new
property (like reset-gpio-active-low, for example).

Then the driver behavior can be made untouched for the old dtb's and
only new dtb's with this new property would have the correct GPIO
reset behavior.

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
  2018-10-09 12:36               ` Fabio Estevam
  (?)
@ 2018-10-09 13:18                 ` Vokáč Michal
  -1 siblings, 0 replies; 27+ messages in thread
From: Vokáč Michal @ 2018-10-09 13:18 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Bartlomiej Zolnierkiewicz, Shawn Guo, Fabio Estevam, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, linux-fbdev

On 9.10.2018 14:36, Fabio Estevam wrote:
> Hi Michal,
> 
> On Tue, Oct 9, 2018 at 5:30 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote:
> 
>> Sorry for the inconvenience :( Lesson learned..
>>
>> So in other words (no offense): broken drivers need to stay broken because
>> users may already get used to the broken behavior?
> 
> In order to keep the old dtb's working you could introduce a new
> property (like reset-gpio-active-low, for example).
> 
> Then the driver behavior can be made untouched for the old dtb's and
> only new dtb's with this new property would have the correct GPIO
> reset behavior.

Thank you very much Fabio!
I saw these xxx-active-low/high properties in many device tree
sources wondering why the heck people use them when they could
use GPIO_ACTIVE_LOW/HIGH. And this is the explanation.

And I feel like an idiot once again: git grep -l "reset-active-low"
first hit is:

  Documentation/devicetree/bindings/display/ssd1307fb.txt

Oooops.
The weird thing is that usage of reset-active-low is documented
in the example but it is not implemented.

So the patch no.2 should be reverted and patch no.3 not applied at all.

I will prepare a new patch utilizing the reset-active-low property.

Again, sorry for the troubles and thank you for your comments.
Michal

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
@ 2018-10-09 13:18                 ` Vokáč Michal
  0 siblings, 0 replies; 27+ messages in thread
From: Vokáč Michal @ 2018-10-09 13:18 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Bartlomiej Zolnierkiewicz, Shawn Guo, Fabio Estevam, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, linux-fbdev

On 9.10.2018 14:36, Fabio Estevam wrote:
> Hi Michal,
> 
> On Tue, Oct 9, 2018 at 5:30 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote:
> 
>> Sorry for the inconvenience :( Lesson learned..
>>
>> So in other words (no offense): broken drivers need to stay broken because
>> users may already get used to the broken behavior?
> 
> In order to keep the old dtb's working you could introduce a new
> property (like reset-gpio-active-low, for example).
> 
> Then the driver behavior can be made untouched for the old dtb's and
> only new dtb's with this new property would have the correct GPIO
> reset behavior.

Thank you very much Fabio!
I saw these xxx-active-low/high properties in many device tree
sources wondering why the heck people use them when they could
use GPIO_ACTIVE_LOW/HIGH. And this is the explanation.

And I feel like an idiot once again: git grep -l "reset-active-low"
first hit is:

  Documentation/devicetree/bindings/display/ssd1307fb.txt

Oooops.
The weird thing is that usage of reset-active-low is documented
in the example but it is not implemented.

So the patch no.2 should be reverted and patch no.3 not applied at all.

I will prepare a new patch utilizing the reset-active-low property.

Again, sorry for the troubles and thank you for your comments.
Michal

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
@ 2018-10-09 13:18                 ` Vokáč Michal
  0 siblings, 0 replies; 27+ messages in thread
From: Vokáč Michal @ 2018-10-09 13:18 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Bartlomiej Zolnierkiewicz, Shawn Guo, Fabio Estevam, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, linux-fbdev

T24gOS4xMC4yMDE4IDE0OjM2LCBGYWJpbyBFc3RldmFtIHdyb3RlOg0KPiBIaSBNaWNoYWwsDQo+
IA0KPiBPbiBUdWUsIE9jdCA5LCAyMDE4IGF0IDU6MzAgQU0gVm9rw6HEjSBNaWNoYWwgPE1pY2hh
bC5Wb2thY0B5c29mdC5jb20+IHdyb3RlOg0KPiANCj4+IFNvcnJ5IGZvciB0aGUgaW5jb252ZW5p
ZW5jZSA6KCBMZXNzb24gbGVhcm5lZC4uDQo+Pg0KPj4gU28gaW4gb3RoZXIgd29yZHMgKG5vIG9m
ZmVuc2UpOiBicm9rZW4gZHJpdmVycyBuZWVkIHRvIHN0YXkgYnJva2VuIGJlY2F1c2UNCj4+IHVz
ZXJzIG1heSBhbHJlYWR5IGdldCB1c2VkIHRvIHRoZSBicm9rZW4gYmVoYXZpb3I/DQo+IA0KPiBJ
biBvcmRlciB0byBrZWVwIHRoZSBvbGQgZHRiJ3Mgd29ya2luZyB5b3UgY291bGQgaW50cm9kdWNl
IGEgbmV3DQo+IHByb3BlcnR5IChsaWtlIHJlc2V0LWdwaW8tYWN0aXZlLWxvdywgZm9yIGV4YW1w
bGUpLg0KPiANCj4gVGhlbiB0aGUgZHJpdmVyIGJlaGF2aW9yIGNhbiBiZSBtYWRlIHVudG91Y2hl
ZCBmb3IgdGhlIG9sZCBkdGIncyBhbmQNCj4gb25seSBuZXcgZHRiJ3Mgd2l0aCB0aGlzIG5ldyBw
cm9wZXJ0eSB3b3VsZCBoYXZlIHRoZSBjb3JyZWN0IEdQSU8NCj4gcmVzZXQgYmVoYXZpb3IuDQoN
ClRoYW5rIHlvdSB2ZXJ5IG11Y2ggRmFiaW8hDQpJIHNhdyB0aGVzZSB4eHgtYWN0aXZlLWxvdy9o
aWdoIHByb3BlcnRpZXMgaW4gbWFueSBkZXZpY2UgdHJlZQ0Kc291cmNlcyB3b25kZXJpbmcgd2h5
IHRoZSBoZWNrIHBlb3BsZSB1c2UgdGhlbSB3aGVuIHRoZXkgY291bGQNCnVzZSBHUElPX0FDVElW
RV9MT1cvSElHSC4gQW5kIHRoaXMgaXMgdGhlIGV4cGxhbmF0aW9uLg0KDQpBbmQgSSBmZWVsIGxp
a2UgYW4gaWRpb3Qgb25jZSBhZ2FpbjogZ2l0IGdyZXAgLWwgInJlc2V0LWFjdGl2ZS1sb3ciDQpm
aXJzdCBoaXQgaXM6DQoNCiAgRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL2Rpc3Bs
YXkvc3NkMTMwN2ZiLnR4dA0KDQpPb29vcHMuDQpUaGUgd2VpcmQgdGhpbmcgaXMgdGhhdCB1c2Fn
ZSBvZiByZXNldC1hY3RpdmUtbG93IGlzIGRvY3VtZW50ZWQNCmluIHRoZSBleGFtcGxlIGJ1dCBp
dCBpcyBub3QgaW1wbGVtZW50ZWQuDQoNClNvIHRoZSBwYXRjaCBuby4yIHNob3VsZCBiZSByZXZl
cnRlZCBhbmQgcGF0Y2ggbm8uMyBub3QgYXBwbGllZCBhdCBhbGwuDQoNCkkgd2lsbCBwcmVwYXJl
IGEgbmV3IHBhdGNoIHV0aWxpemluZyB0aGUgcmVzZXQtYWN0aXZlLWxvdyBwcm9wZXJ0eS4NCg0K
QWdhaW4sIHNvcnJ5IGZvciB0aGUgdHJvdWJsZXMgYW5kIHRoYW5rIHlvdSBmb3IgeW91ciBjb21t
ZW50cy4NCk1pY2hhbA0K

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
  2018-10-09 13:18                 ` Vokáč Michal
  (?)
@ 2018-10-09 13:30                   ` Bartlomiej Zolnierkiewicz
  -1 siblings, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-10-09 13:30 UTC (permalink / raw)
  To: Vokáč Michal
  Cc: Fabio Estevam, Shawn Guo, Fabio Estevam, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, linux-fbdev


On 10/09/2018 03:18 PM, Vokáč Michal wrote:
> On 9.10.2018 14:36, Fabio Estevam wrote:
>> Hi Michal,
>>
>> On Tue, Oct 9, 2018 at 5:30 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote:
>>
>>> Sorry for the inconvenience :( Lesson learned..
>>>
>>> So in other words (no offense): broken drivers need to stay broken because
>>> users may already get used to the broken behavior?
>>
>> In order to keep the old dtb's working you could introduce a new
>> property (like reset-gpio-active-low, for example).
>>
>> Then the driver behavior can be made untouched for the old dtb's and
>> only new dtb's with this new property would have the correct GPIO
>> reset behavior.
> 
> Thank you very much Fabio!
> I saw these xxx-active-low/high properties in many device tree
> sources wondering why the heck people use them when they could
> use GPIO_ACTIVE_LOW/HIGH. And this is the explanation.
> 
> And I feel like an idiot once again: git grep -l "reset-active-low"
> first hit is:
> 
>   Documentation/devicetree/bindings/display/ssd1307fb.txt
> 
> Oooops.
> The weird thing is that usage of reset-active-low is documented
> in the example but it is not implemented.
> 
> So the patch no.2 should be reverted and patch no.3 not applied at all.

OK, I've applied the patch below to fbdev-for-next tree.

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


From 64f83a816b27c7b5e026a74ecb5c61dbabfae997 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Date: Tue, 9 Oct 2018 15:18:42 +0200
Subject: [PATCH] Revert "video: ssd1307fb: Do not hard code active-low reset
 sequence"

This reverts commit 9827f26374fb85e1811f2adbcc25c8a3992dbe7f.

On 10/09/2018 02:20 AM, Shawn Guo wrote:

> Well, it means the change breaks the ABI between kernel and device tree,
> e.g. the new kernel will not work with existing/installed DTBs.

Revert the change until DTB compatibility issue is resolved.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/video/fbdev/ssd1307fb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 3b361bc..4061a20 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -667,10 +667,10 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	if (par->reset) {
 		/* Reset the screen */
-		gpiod_set_value_cansleep(par->reset, 1);
-		udelay(4);
 		gpiod_set_value_cansleep(par->reset, 0);
 		udelay(4);
+		gpiod_set_value_cansleep(par->reset, 1);
+		udelay(4);
 	}
 
 	if (par->vbat_reg) {
-- 
1.9.1


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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
@ 2018-10-09 13:30                   ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-10-09 13:30 UTC (permalink / raw)
  To: Vokáč Michal
  Cc: Fabio Estevam, Shawn Guo, Fabio Estevam, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, linux-fbdev


On 10/09/2018 03:18 PM, Vokáč Michal wrote:
> On 9.10.2018 14:36, Fabio Estevam wrote:
>> Hi Michal,
>>
>> On Tue, Oct 9, 2018 at 5:30 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote:
>>
>>> Sorry for the inconvenience :( Lesson learned..
>>>
>>> So in other words (no offense): broken drivers need to stay broken because
>>> users may already get used to the broken behavior?
>>
>> In order to keep the old dtb's working you could introduce a new
>> property (like reset-gpio-active-low, for example).
>>
>> Then the driver behavior can be made untouched for the old dtb's and
>> only new dtb's with this new property would have the correct GPIO
>> reset behavior.
> 
> Thank you very much Fabio!
> I saw these xxx-active-low/high properties in many device tree
> sources wondering why the heck people use them when they could
> use GPIO_ACTIVE_LOW/HIGH. And this is the explanation.
> 
> And I feel like an idiot once again: git grep -l "reset-active-low"
> first hit is:
> 
>   Documentation/devicetree/bindings/display/ssd1307fb.txt
> 
> Oooops.
> The weird thing is that usage of reset-active-low is documented
> in the example but it is not implemented.
> 
> So the patch no.2 should be reverted and patch no.3 not applied at all.

OK, I've applied the patch below to fbdev-for-next tree.

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


>From 64f83a816b27c7b5e026a74ecb5c61dbabfae997 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Date: Tue, 9 Oct 2018 15:18:42 +0200
Subject: [PATCH] Revert "video: ssd1307fb: Do not hard code active-low reset
 sequence"

This reverts commit 9827f26374fb85e1811f2adbcc25c8a3992dbe7f.

On 10/09/2018 02:20 AM, Shawn Guo wrote:

> Well, it means the change breaks the ABI between kernel and device tree,
> e.g. the new kernel will not work with existing/installed DTBs.

Revert the change until DTB compatibility issue is resolved.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/video/fbdev/ssd1307fb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 3b361bc..4061a20 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -667,10 +667,10 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	if (par->reset) {
 		/* Reset the screen */
-		gpiod_set_value_cansleep(par->reset, 1);
-		udelay(4);
 		gpiod_set_value_cansleep(par->reset, 0);
 		udelay(4);
+		gpiod_set_value_cansleep(par->reset, 1);
+		udelay(4);
 	}
 
 	if (par->vbat_reg) {
-- 
1.9.1

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

* Re: [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
@ 2018-10-09 13:30                   ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-10-09 13:30 UTC (permalink / raw)
  To: Vokáč Michal
  Cc: Fabio Estevam, Shawn Guo, Fabio Estevam, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, linux-fbdev


On 10/09/2018 03:18 PM, Vokáč Michal wrote:
> On 9.10.2018 14:36, Fabio Estevam wrote:
>> Hi Michal,
>>
>> On Tue, Oct 9, 2018 at 5:30 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote:
>>
>>> Sorry for the inconvenience :( Lesson learned..
>>>
>>> So in other words (no offense): broken drivers need to stay broken because
>>> users may already get used to the broken behavior?
>>
>> In order to keep the old dtb's working you could introduce a new
>> property (like reset-gpio-active-low, for example).
>>
>> Then the driver behavior can be made untouched for the old dtb's and
>> only new dtb's with this new property would have the correct GPIO
>> reset behavior.
> 
> Thank you very much Fabio!
> I saw these xxx-active-low/high properties in many device tree
> sources wondering why the heck people use them when they could
> use GPIO_ACTIVE_LOW/HIGH. And this is the explanation.
> 
> And I feel like an idiot once again: git grep -l "reset-active-low"
> first hit is:
> 
>   Documentation/devicetree/bindings/display/ssd1307fb.txt
> 
> Oooops.
> The weird thing is that usage of reset-active-low is documented
> in the example but it is not implemented.
> 
> So the patch no.2 should be reverted and patch no.3 not applied at all.

OK, I've applied the patch below to fbdev-for-next tree.

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


From 64f83a816b27c7b5e026a74ecb5c61dbabfae997 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Date: Tue, 9 Oct 2018 15:18:42 +0200
Subject: [PATCH] Revert "video: ssd1307fb: Do not hard code active-low reset
 sequence"

This reverts commit 9827f26374fb85e1811f2adbcc25c8a3992dbe7f.

On 10/09/2018 02:20 AM, Shawn Guo wrote:

> Well, it means the change breaks the ABI between kernel and device tree,
> e.g. the new kernel will not work with existing/installed DTBs.

Revert the change until DTB compatibility issue is resolved.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/video/fbdev/ssd1307fb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 3b361bc..4061a20 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -667,10 +667,10 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	if (par->reset) {
 		/* Reset the screen */
-		gpiod_set_value_cansleep(par->reset, 1);
-		udelay(4);
 		gpiod_set_value_cansleep(par->reset, 0);
 		udelay(4);
+		gpiod_set_value_cansleep(par->reset, 1);
+		udelay(4);
 	}
 
 	if (par->vbat_reg) {
-- 
1.9.1

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

end of thread, other threads:[~2018-10-09 13:30 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20180927092738epcas5p28c53516b73fe7747133914f40c84d6c4@epcas5p2.samsung.com>
2018-09-27  9:24 ` [RESEND PATCH v2 1/3] video: ssd1307fb: Use gpiod_set_value_cansleep() for reset Michal Vokáč
2018-09-27  9:24   ` Michal Vokáč
2018-09-27  9:24   ` [RESEND PATCH v2 2/3] video: ssd1307fb: Do not hard code active-low reset sequence Michal Vokáč
2018-09-27  9:24     ` Michal Vokáč
2018-10-08 10:53     ` Bartlomiej Zolnierkiewicz
2018-10-08 10:53       ` Bartlomiej Zolnierkiewicz
2018-09-27  9:24   ` [RESEND PATCH v2 3/3] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity Michal Vokáč
2018-09-27  9:24     ` Michal Vokáč
2018-10-08 11:45     ` Vokáč Michal
2018-10-08 11:45       ` Vokáč Michal
2018-10-09  0:20       ` Shawn Guo
2018-10-09  0:20         ` Shawn Guo
2018-10-09  7:51         ` Bartlomiej Zolnierkiewicz
2018-10-09  7:51           ` Bartlomiej Zolnierkiewicz
2018-10-09  8:29           ` Vokáč Michal
2018-10-09  8:29             ` Vokáč Michal
2018-10-09 12:36             ` Fabio Estevam
2018-10-09 12:36               ` Fabio Estevam
2018-10-09 12:36               ` Fabio Estevam
2018-10-09 13:18               ` Vokáč Michal
2018-10-09 13:18                 ` Vokáč Michal
2018-10-09 13:18                 ` Vokáč Michal
2018-10-09 13:30                 ` Bartlomiej Zolnierkiewicz
2018-10-09 13:30                   ` Bartlomiej Zolnierkiewicz
2018-10-09 13:30                   ` Bartlomiej Zolnierkiewicz
2018-10-08 10:53   ` [RESEND PATCH v2 1/3] video: ssd1307fb: Use gpiod_set_value_cansleep() for reset Bartlomiej Zolnierkiewicz
2018-10-08 10:53     ` Bartlomiej Zolnierkiewicz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.