* [PATCH fbdev-for-next 1/2] dt-bindings: display: ssd1307fb: Add reset-active-low property @ 2018-11-02 14:56 ` Vokáč Michal 0 siblings, 0 replies; 15+ messages in thread From: Vokáč Michal @ 2018-11-02 14:56 UTC (permalink / raw) To: Rob Herring, Bartlomiej Zolnierkiewicz Cc: linux-fbdev, linux-kernel, devicetree, Jyri Sarha, Vokáč Michal This reverts commit 519b4dba586198eed8f72ba07bc71808af2e0e32. It is true that the actual implementation has never been there. But contrary to what the reverted commit message says it does make sense to add it. Current implementation of the reset signal is hard-coded to active low with the assumption that reset-gpios is specified as GPIO_ACTIVE_HIGH. That is technically wrong as the DTS authors should know that SSD130x displays need active low reset and hence they are temped to use GPIO_ACTIVE_LOW. But with that the reset is broken. So reset-acive-low property can be used to invert the signal once again to fix this. Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com> --- Documentation/devicetree/bindings/display/ssd1307fb.txt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt index 209d931..a5ead10 100644 --- a/Documentation/devicetree/bindings/display/ssd1307fb.txt +++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt @@ -16,6 +16,8 @@ Required properties: Optional properties: - reset-gpios: The GPIO used to reset the OLED display, if available. See Documentation/devicetree/bindings/gpio/gpio.txt for details. + - reset-active-low: Bool flag to indicate the GPIO specified in "reset-gpios" + property is active low. - vbat-supply: The supply for VBAT - solomon,segment-no-remap: Display needs normal (non-inverted) data column to segment mapping @@ -35,7 +37,7 @@ ssd1307: oled@3c { compatible = "solomon,ssd1307fb-i2c"; reg = <0x3c>; pwms = <&pwm 4 3000>; - reset-gpios = <&gpio2 7>; + reset-gpios = <&gpio2 7 GPIO_ACTIVE_LOW>; reset-active-low; }; @@ -43,7 +45,7 @@ ssd1306: oled@3c { compatible = "solomon,ssd1306fb-i2c"; reg = <0x3c>; pwms = <&pwm 4 3000>; - reset-gpios = <&gpio2 7>; + reset-gpios = <&gpio2 7 GPIO_ACTIVE_LOW>; reset-active-low; solomon,com-lrremap; solomon,com-invdir; -- 2.1.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH fbdev-for-next 1/2] dt-bindings: display: ssd1307fb: Add reset-active-low property @ 2018-11-02 14:56 ` Vokáč Michal 0 siblings, 0 replies; 15+ messages in thread From: Vokáč Michal @ 2018-11-02 14:56 UTC (permalink / raw) To: Rob Herring, Bartlomiej Zolnierkiewicz Cc: linux-fbdev, linux-kernel, devicetree, Jyri Sarha, Vokáč Michal VGhpcyByZXZlcnRzIGNvbW1pdCA1MTliNGRiYTU4NjE5OGVlZDhmNzJiYTA3YmM3MTgwOGFmMmUw ZTMyLg0KSXQgaXMgdHJ1ZSB0aGF0IHRoZSBhY3R1YWwgaW1wbGVtZW50YXRpb24gaGFzIG5ldmVy IGJlZW4gdGhlcmUuIEJ1dA0KY29udHJhcnkgdG8gd2hhdCB0aGUgcmV2ZXJ0ZWQgY29tbWl0IG1l c3NhZ2Ugc2F5cyBpdCBkb2VzIG1ha2Ugc2Vuc2UNCnRvIGFkZCBpdC4NCg0KQ3VycmVudCBpbXBs ZW1lbnRhdGlvbiBvZiB0aGUgcmVzZXQgc2lnbmFsIGlzIGhhcmQtY29kZWQgdG8gYWN0aXZlIGxv dw0Kd2l0aCB0aGUgYXNzdW1wdGlvbiB0aGF0IHJlc2V0LWdwaW9zIGlzIHNwZWNpZmllZCBhcyBH UElPX0FDVElWRV9ISUdILg0KVGhhdCBpcyB0ZWNobmljYWxseSB3cm9uZyBhcyB0aGUgRFRTIGF1 dGhvcnMgc2hvdWxkIGtub3cgdGhhdCBTU0QxMzB4DQpkaXNwbGF5cyBuZWVkIGFjdGl2ZSBsb3cg cmVzZXQgYW5kIGhlbmNlIHRoZXkgYXJlIHRlbXBlZCB0byB1c2UNCkdQSU9fQUNUSVZFX0xPVy4g QnV0IHdpdGggdGhhdCB0aGUgcmVzZXQgaXMgYnJva2VuLiBTbyByZXNldC1hY2l2ZS1sb3cNCnBy b3BlcnR5IGNhbiBiZSB1c2VkIHRvIGludmVydCB0aGUgc2lnbmFsIG9uY2UgYWdhaW4gdG8gZml4 IHRoaXMuDQoNClNpZ25lZC1vZmYtYnk6IE1pY2hhbCBWb2vDocSNIDxtaWNoYWwudm9rYWNAeXNv ZnQuY29tPg0KLS0tDQogRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL2Rpc3BsYXkv c3NkMTMwN2ZiLnR4dCB8IDYgKysrKy0tDQogMSBmaWxlIGNoYW5nZWQsIDQgaW5zZXJ0aW9ucygr KSwgMiBkZWxldGlvbnMoLSkNCg0KZGlmZiAtLWdpdCBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJl ZS9iaW5kaW5ncy9kaXNwbGF5L3NzZDEzMDdmYi50eHQgYi9Eb2N1bWVudGF0aW9uL2RldmljZXRy ZWUvYmluZGluZ3MvZGlzcGxheS9zc2QxMzA3ZmIudHh0DQppbmRleCAyMDlkOTMxLi5hNWVhZDEw IDEwMDY0NA0KLS0tIGEvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL2Rpc3BsYXkv c3NkMTMwN2ZiLnR4dA0KKysrIGIvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL2Rp c3BsYXkvc3NkMTMwN2ZiLnR4dA0KQEAgLTE2LDYgKzE2LDggQEAgUmVxdWlyZWQgcHJvcGVydGll czoNCiBPcHRpb25hbCBwcm9wZXJ0aWVzOg0KICAgLSByZXNldC1ncGlvczogVGhlIEdQSU8gdXNl ZCB0byByZXNldCB0aGUgT0xFRCBkaXNwbGF5LCBpZiBhdmFpbGFibGUuIFNlZQ0KICAgICAgICAg ICAgICAgICAgRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL2dwaW8vZ3Bpby50eHQg Zm9yIGRldGFpbHMuDQorICAtIHJlc2V0LWFjdGl2ZS1sb3c6IEJvb2wgZmxhZyB0byBpbmRpY2F0 ZSB0aGUgR1BJTyBzcGVjaWZpZWQgaW4gInJlc2V0LWdwaW9zIg0KKyAgICAgICAgICAgICAgICAg ICAgICBwcm9wZXJ0eSBpcyBhY3RpdmUgbG93Lg0KICAgLSB2YmF0LXN1cHBseTogVGhlIHN1cHBs eSBmb3IgVkJBVA0KICAgLSBzb2xvbW9uLHNlZ21lbnQtbm8tcmVtYXA6IERpc3BsYXkgbmVlZHMg bm9ybWFsIChub24taW52ZXJ0ZWQpIGRhdGEgY29sdW1uDQogICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgdG8gc2VnbWVudCBtYXBwaW5nDQpAQCAtMzUsNyArMzcsNyBAQCBzc2QxMzA3OiBv bGVkQDNjIHsNCiAgICAgICAgIGNvbXBhdGlibGUgPSAic29sb21vbixzc2QxMzA3ZmItaTJjIjsN CiAgICAgICAgIHJlZyA9IDwweDNjPjsNCiAgICAgICAgIHB3bXMgPSA8JnB3bSA0IDMwMDA+Ow0K LSAgICAgICAgcmVzZXQtZ3Bpb3MgPSA8JmdwaW8yIDc+Ow0KKyAgICAgICAgcmVzZXQtZ3Bpb3Mg PSA8JmdwaW8yIDcgR1BJT19BQ1RJVkVfTE9XPjsNCiAgICAgICAgIHJlc2V0LWFjdGl2ZS1sb3c7 DQogfTsNCiANCkBAIC00Myw3ICs0NSw3IEBAIHNzZDEzMDY6IG9sZWRAM2Mgew0KICAgICAgICAg Y29tcGF0aWJsZSA9ICJzb2xvbW9uLHNzZDEzMDZmYi1pMmMiOw0KICAgICAgICAgcmVnID0gPDB4 M2M+Ow0KICAgICAgICAgcHdtcyA9IDwmcHdtIDQgMzAwMD47DQotICAgICAgICByZXNldC1ncGlv cyA9IDwmZ3BpbzIgNz47DQorICAgICAgICByZXNldC1ncGlvcyA9IDwmZ3BpbzIgNyBHUElPX0FD VElWRV9MT1c+Ow0KICAgICAgICAgcmVzZXQtYWN0aXZlLWxvdzsNCiAgICAgICAgIHNvbG9tb24s Y29tLWxycmVtYXA7DQogICAgICAgICBzb2xvbW9uLGNvbS1pbnZkaXI7DQotLSANCjIuMS40DQoN Cg= ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the reset-active-low property 2018-11-02 14:56 ` Vokáč Michal @ 2018-11-02 14:56 ` Vokáč Michal -1 siblings, 0 replies; 15+ messages in thread From: Vokáč Michal @ 2018-11-02 14:56 UTC (permalink / raw) To: Rob Herring, Bartlomiej Zolnierkiewicz Cc: linux-fbdev, linux-kernel, devicetree, Jyri Sarha, Vokáč Michal The SSD130x OLED display reset signal is active low. Now the reset sequence is implemented in such a way that DTS authors are forced to define the reset-gpios property with GPIO_ACTIVE_HIGH to make the reset work. Add the reset-active-low property so the signal is inverted once again and the GPIO_ACTIVE_LOW work as expected. Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com> --- drivers/video/fbdev/ssd1307fb.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c index e7ae135..790f1c4 100644 --- a/drivers/video/fbdev/ssd1307fb.c +++ b/drivers/video/fbdev/ssd1307fb.c @@ -608,6 +608,7 @@ static int ssd1307fb_probe(struct i2c_client *client, struct fb_deferred_io *ssd1307fb_defio; u32 vmem_size; struct ssd1307fb_par *par; + bool reset_active_low; u8 *vmem; int ret; @@ -671,6 +672,7 @@ static int ssd1307fb_probe(struct i2c_client *client, par->com_seq = of_property_read_bool(node, "solomon,com-seq"); par->com_lrremap = of_property_read_bool(node, "solomon,com-lrremap"); par->com_invdir = of_property_read_bool(node, "solomon,com-invdir"); + reset_active_low = of_property_read_bool(node, "reset-active-low"); par->contrast = 127; par->vcomh = par->device_info->default_vcomh; @@ -728,9 +730,9 @@ static int ssd1307fb_probe(struct i2c_client *client, if (par->reset) { /* Reset the screen */ - gpiod_set_value_cansleep(par->reset, 0); + gpiod_set_value_cansleep(par->reset, reset_active_low); udelay(4); - gpiod_set_value_cansleep(par->reset, 1); + gpiod_set_value_cansleep(par->reset, !reset_active_low); udelay(4); } -- 2.1.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the reset-active-low property @ 2018-11-02 14:56 ` Vokáč Michal 0 siblings, 0 replies; 15+ messages in thread From: Vokáč Michal @ 2018-11-02 14:56 UTC (permalink / raw) To: Rob Herring, Bartlomiej Zolnierkiewicz Cc: linux-fbdev, linux-kernel, devicetree, Jyri Sarha, Vokáč Michal VGhlIFNTRDEzMHggT0xFRMKgZGlzcGxheSByZXNldCBzaWduYWwgaXMgYWN0aXZlIGxvdy4gTm93 IHRoZSByZXNldA0Kc2VxdWVuY2UgaXMgaW1wbGVtZW50ZWQgaW4gc3VjaCBhIHdheSB0aGF0IERU UyBhdXRob3JzIGFyZSBmb3JjZWQgdG8NCmRlZmluZSB0aGUgcmVzZXQtZ3Bpb3MgcHJvcGVydHkg d2l0aCBHUElPX0FDVElWRV9ISUdIIHRvIG1ha2UgdGhlIHJlc2V0DQp3b3JrLg0KDQpBZGQgdGhl IHJlc2V0LWFjdGl2ZS1sb3cgcHJvcGVydHkgc28gdGhlIHNpZ25hbCBpcyBpbnZlcnRlZCBvbmNl IGFnYWluDQphbmQgdGhlIEdQSU9fQUNUSVZFX0xPVyB3b3JrIGFzIGV4cGVjdGVkLg0KDQpTaWdu ZWQtb2ZmLWJ5OiBNaWNoYWwgVm9rw6HEjSA8bWljaGFsLnZva2FjQHlzb2Z0LmNvbT4NCi0tLQ0K IGRyaXZlcnMvdmlkZW8vZmJkZXYvc3NkMTMwN2ZiLmMgfCA2ICsrKystLQ0KIDEgZmlsZSBjaGFu Z2VkLCA0IGluc2VydGlvbnMoKyksIDIgZGVsZXRpb25zKC0pDQoNCmRpZmYgLS1naXQgYS9kcml2 ZXJzL3ZpZGVvL2ZiZGV2L3NzZDEzMDdmYi5jIGIvZHJpdmVycy92aWRlby9mYmRldi9zc2QxMzA3 ZmIuYw0KaW5kZXggZTdhZTEzNS4uNzkwZjFjNCAxMDA2NDQNCi0tLSBhL2RyaXZlcnMvdmlkZW8v ZmJkZXYvc3NkMTMwN2ZiLmMNCisrKyBiL2RyaXZlcnMvdmlkZW8vZmJkZXYvc3NkMTMwN2ZiLmMN CkBAIC02MDgsNiArNjA4LDcgQEAgc3RhdGljIGludCBzc2QxMzA3ZmJfcHJvYmUoc3RydWN0IGky Y19jbGllbnQgKmNsaWVudCwNCiAJc3RydWN0IGZiX2RlZmVycmVkX2lvICpzc2QxMzA3ZmJfZGVm aW87DQogCXUzMiB2bWVtX3NpemU7DQogCXN0cnVjdCBzc2QxMzA3ZmJfcGFyICpwYXI7DQorCWJv b2wgcmVzZXRfYWN0aXZlX2xvdzsNCiAJdTggKnZtZW07DQogCWludCByZXQ7DQogDQpAQCAtNjcx LDYgKzY3Miw3IEBAIHN0YXRpYyBpbnQgc3NkMTMwN2ZiX3Byb2JlKHN0cnVjdCBpMmNfY2xpZW50 ICpjbGllbnQsDQogCXBhci0+Y29tX3NlcSA9IG9mX3Byb3BlcnR5X3JlYWRfYm9vbChub2RlLCAi c29sb21vbixjb20tc2VxIik7DQogCXBhci0+Y29tX2xycmVtYXAgPSBvZl9wcm9wZXJ0eV9yZWFk X2Jvb2wobm9kZSwgInNvbG9tb24sY29tLWxycmVtYXAiKTsNCiAJcGFyLT5jb21faW52ZGlyID0g b2ZfcHJvcGVydHlfcmVhZF9ib29sKG5vZGUsICJzb2xvbW9uLGNvbS1pbnZkaXIiKTsNCisJcmVz ZXRfYWN0aXZlX2xvdyA9IG9mX3Byb3BlcnR5X3JlYWRfYm9vbChub2RlLCAicmVzZXQtYWN0aXZl LWxvdyIpOw0KIA0KIAlwYXItPmNvbnRyYXN0ID0gMTI3Ow0KIAlwYXItPnZjb21oID0gcGFyLT5k ZXZpY2VfaW5mby0+ZGVmYXVsdF92Y29taDsNCkBAIC03MjgsOSArNzMwLDkgQEAgc3RhdGljIGlu dCBzc2QxMzA3ZmJfcHJvYmUoc3RydWN0IGkyY19jbGllbnQgKmNsaWVudCwNCiANCiAJaWYgKHBh ci0+cmVzZXQpIHsNCiAJCS8qIFJlc2V0IHRoZSBzY3JlZW4gKi8NCi0JCWdwaW9kX3NldF92YWx1 ZV9jYW5zbGVlcChwYXItPnJlc2V0LCAwKTsNCisJCWdwaW9kX3NldF92YWx1ZV9jYW5zbGVlcChw YXItPnJlc2V0LCByZXNldF9hY3RpdmVfbG93KTsNCiAJCXVkZWxheSg0KTsNCi0JCWdwaW9kX3Nl dF92YWx1ZV9jYW5zbGVlcChwYXItPnJlc2V0LCAxKTsNCisJCWdwaW9kX3NldF92YWx1ZV9jYW5z bGVlcChwYXItPnJlc2V0LCAhcmVzZXRfYWN0aXZlX2xvdyk7DQogCQl1ZGVsYXkoNCk7DQogCX0N CiANCi0tIA0KMi4xLjQNCg0K ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the reset-active-low property 2018-11-02 14:56 ` Vokáč Michal (?) @ 2018-11-12 16:55 ` Rob Herring 2018-11-19 15:12 ` Vokáč Michal -1 siblings, 1 reply; 15+ messages in thread From: Rob Herring @ 2018-11-12 16:55 UTC (permalink / raw) To: Vokáč Michal Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel, devicetree, Jyri Sarha On Fri, Nov 02, 2018 at 02:56:35PM +0000, Vokáč Michal wrote: > The SSD130x OLED display reset signal is active low. Now the reset > sequence is implemented in such a way that DTS authors are forced to > define the reset-gpios property with GPIO_ACTIVE_HIGH to make the reset > work. > > Add the reset-active-low property so the signal is inverted once again > and the GPIO_ACTIVE_LOW work as expected. > > Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com> > --- > drivers/video/fbdev/ssd1307fb.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c > index e7ae135..790f1c4 100644 > --- a/drivers/video/fbdev/ssd1307fb.c > +++ b/drivers/video/fbdev/ssd1307fb.c > @@ -608,6 +608,7 @@ static int ssd1307fb_probe(struct i2c_client *client, > struct fb_deferred_io *ssd1307fb_defio; > u32 vmem_size; > struct ssd1307fb_par *par; > + bool reset_active_low; > u8 *vmem; > int ret; > > @@ -671,6 +672,7 @@ static int ssd1307fb_probe(struct i2c_client *client, > par->com_seq = of_property_read_bool(node, "solomon,com-seq"); > par->com_lrremap = of_property_read_bool(node, "solomon,com-lrremap"); > par->com_invdir = of_property_read_bool(node, "solomon,com-invdir"); > + reset_active_low = of_property_read_bool(node, "reset-active-low"); > > par->contrast = 127; > par->vcomh = par->device_info->default_vcomh; > @@ -728,9 +730,9 @@ static int ssd1307fb_probe(struct i2c_client *client, > > if (par->reset) { > /* Reset the screen */ > - gpiod_set_value_cansleep(par->reset, 0); > + gpiod_set_value_cansleep(par->reset, reset_active_low); > udelay(4); > - gpiod_set_value_cansleep(par->reset, 1); > + gpiod_set_value_cansleep(par->reset, !reset_active_low); I think you and whomever wrote the original code are misinterpretting how the gpiod API works. 1 means make the signal active and this case active is low. It is strange, but does mean a reset sequence should always be set to a 1 and then a 0 and it will work with either polarity in the DT. Rob ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the reset-active-low property 2018-11-12 16:55 ` Rob Herring @ 2018-11-19 15:12 ` Vokáč Michal 0 siblings, 0 replies; 15+ messages in thread From: Vokáč Michal @ 2018-11-19 15:12 UTC (permalink / raw) To: Rob Herring Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel, devicetree, Jyri Sarha On 12.11.2018 17:55, Rob Herring wrote: > On Fri, Nov 02, 2018 at 02:56:35PM +0000, Vokáč Michal wrote: >> The SSD130x OLED display reset signal is active low. Now the reset >> sequence is implemented in such a way that DTS authors are forced to >> define the reset-gpios property with GPIO_ACTIVE_HIGH to make the reset >> work. >> >> Add the reset-active-low property so the signal is inverted once again >> and the GPIO_ACTIVE_LOW work as expected. >> >> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com> >> --- >> drivers/video/fbdev/ssd1307fb.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c >> index e7ae135..790f1c4 100644 >> --- a/drivers/video/fbdev/ssd1307fb.c >> +++ b/drivers/video/fbdev/ssd1307fb.c >> @@ -608,6 +608,7 @@ static int ssd1307fb_probe(struct i2c_client *client, >> struct fb_deferred_io *ssd1307fb_defio; >> u32 vmem_size; >> struct ssd1307fb_par *par; >> + bool reset_active_low; >> u8 *vmem; >> int ret; >> >> @@ -671,6 +672,7 @@ static int ssd1307fb_probe(struct i2c_client *client, >> par->com_seq = of_property_read_bool(node, "solomon,com-seq"); >> par->com_lrremap = of_property_read_bool(node, "solomon,com-lrremap"); >> par->com_invdir = of_property_read_bool(node, "solomon,com-invdir"); >> + reset_active_low = of_property_read_bool(node, "reset-active-low"); >> >> par->contrast = 127; >> par->vcomh = par->device_info->default_vcomh; >> @@ -728,9 +730,9 @@ static int ssd1307fb_probe(struct i2c_client *client, >> >> if (par->reset) { >> /* Reset the screen */ >> - gpiod_set_value_cansleep(par->reset, 0); >> + gpiod_set_value_cansleep(par->reset, reset_active_low); >> udelay(4); >> - gpiod_set_value_cansleep(par->reset, 1); >> + gpiod_set_value_cansleep(par->reset, !reset_active_low); > > I think you and whomever wrote the original code are misinterpretting > how the gpiod API works. 1 means make the signal active and this case > active is low. I totally agree and I think I understand that correctly. > It is strange, but does mean a reset sequence should always be set to a > 1 and then a 0 and it will work with either polarity in the DT. I agree the reset should be done as a 0 -> 1 -> 0 sequence and that should just work. The problem is it is implemented vice versa and so it works only if you have GPIO_ACTIVE_HIGH in DT for a signal that is actually active low. And what it actually does is that it holds the controller in reset since the GPIO is successfully acquired (because of GPIOD_OUT_LOW here [1]) and later on it only releases the reset. As a DT author I would like to somehow clearly state that the OLED display uses active low reset in my DT. My first attempt to fix this was to change the reset sequence [2]. It was applied and then reverted as it is not backward compatible with deployed DTB files [3]. [1] https://elixir.bootlin.com/linux/v4.20-rc3/source/drivers/video/fbdev/ssd1307fb.c#L570 [2] https://patchwork.kernel.org/patch/10617729/ [3] https://patchwork.kernel.org/patch/10617731/ Michal ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the reset-active-low property @ 2018-11-19 15:12 ` Vokáč Michal 0 siblings, 0 replies; 15+ messages in thread From: Vokáč Michal @ 2018-11-19 15:12 UTC (permalink / raw) To: Rob Herring Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel, devicetree, Jyri Sarha T24gMTIuMTEuMjAxOCAxNzo1NSwgUm9iIEhlcnJpbmcgd3JvdGU6DQo+IE9uIEZyaSwgTm92IDAy LCAyMDE4IGF0IDAyOjU2OjM1UE0gKzAwMDAsIFZva8OhxI0gTWljaGFsIHdyb3RlOg0KPj4gVGhl IFNTRDEzMHggT0xFRCBkaXNwbGF5IHJlc2V0IHNpZ25hbCBpcyBhY3RpdmUgbG93LiBOb3cgdGhl IHJlc2V0DQo+PiBzZXF1ZW5jZSBpcyBpbXBsZW1lbnRlZCBpbiBzdWNoIGEgd2F5IHRoYXQgRFRT IGF1dGhvcnMgYXJlIGZvcmNlZCB0bw0KPj4gZGVmaW5lIHRoZSByZXNldC1ncGlvcyBwcm9wZXJ0 eSB3aXRoIEdQSU9fQUNUSVZFX0hJR0ggdG8gbWFrZSB0aGUgcmVzZXQNCj4+IHdvcmsuDQo+Pg0K Pj4gQWRkIHRoZSByZXNldC1hY3RpdmUtbG93IHByb3BlcnR5IHNvIHRoZSBzaWduYWwgaXMgaW52 ZXJ0ZWQgb25jZSBhZ2Fpbg0KPj4gYW5kIHRoZSBHUElPX0FDVElWRV9MT1cgd29yayBhcyBleHBl Y3RlZC4NCj4+DQo+PiBTaWduZWQtb2ZmLWJ5OiBNaWNoYWwgVm9rw6HEjSA8bWljaGFsLnZva2Fj QHlzb2Z0LmNvbT4NCj4+IC0tLQ0KPj4gICBkcml2ZXJzL3ZpZGVvL2ZiZGV2L3NzZDEzMDdmYi5j IHwgNiArKysrLS0NCj4+ICAgMSBmaWxlIGNoYW5nZWQsIDQgaW5zZXJ0aW9ucygrKSwgMiBkZWxl dGlvbnMoLSkNCj4+DQo+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy92aWRlby9mYmRldi9zc2QxMzA3 ZmIuYyBiL2RyaXZlcnMvdmlkZW8vZmJkZXYvc3NkMTMwN2ZiLmMNCj4+IGluZGV4IGU3YWUxMzUu Ljc5MGYxYzQgMTAwNjQ0DQo+PiAtLS0gYS9kcml2ZXJzL3ZpZGVvL2ZiZGV2L3NzZDEzMDdmYi5j DQo+PiArKysgYi9kcml2ZXJzL3ZpZGVvL2ZiZGV2L3NzZDEzMDdmYi5jDQo+PiBAQCAtNjA4LDYg KzYwOCw3IEBAIHN0YXRpYyBpbnQgc3NkMTMwN2ZiX3Byb2JlKHN0cnVjdCBpMmNfY2xpZW50ICpj bGllbnQsDQo+PiAgIAlzdHJ1Y3QgZmJfZGVmZXJyZWRfaW8gKnNzZDEzMDdmYl9kZWZpbzsNCj4+ ICAgCXUzMiB2bWVtX3NpemU7DQo+PiAgIAlzdHJ1Y3Qgc3NkMTMwN2ZiX3BhciAqcGFyOw0KPj4g Kwlib29sIHJlc2V0X2FjdGl2ZV9sb3c7DQo+PiAgIAl1OCAqdm1lbTsNCj4+ICAgCWludCByZXQ7 DQo+PiAgIA0KPj4gQEAgLTY3MSw2ICs2NzIsNyBAQCBzdGF0aWMgaW50IHNzZDEzMDdmYl9wcm9i ZShzdHJ1Y3QgaTJjX2NsaWVudCAqY2xpZW50LA0KPj4gICAJcGFyLT5jb21fc2VxID0gb2ZfcHJv cGVydHlfcmVhZF9ib29sKG5vZGUsICJzb2xvbW9uLGNvbS1zZXEiKTsNCj4+ICAgCXBhci0+Y29t X2xycmVtYXAgPSBvZl9wcm9wZXJ0eV9yZWFkX2Jvb2wobm9kZSwgInNvbG9tb24sY29tLWxycmVt YXAiKTsNCj4+ICAgCXBhci0+Y29tX2ludmRpciA9IG9mX3Byb3BlcnR5X3JlYWRfYm9vbChub2Rl LCAic29sb21vbixjb20taW52ZGlyIik7DQo+PiArCXJlc2V0X2FjdGl2ZV9sb3cgPSBvZl9wcm9w ZXJ0eV9yZWFkX2Jvb2wobm9kZSwgInJlc2V0LWFjdGl2ZS1sb3ciKTsNCj4+ICAgDQo+PiAgIAlw YXItPmNvbnRyYXN0ID0gMTI3Ow0KPj4gICAJcGFyLT52Y29taCA9IHBhci0+ZGV2aWNlX2luZm8t PmRlZmF1bHRfdmNvbWg7DQo+PiBAQCAtNzI4LDkgKzczMCw5IEBAIHN0YXRpYyBpbnQgc3NkMTMw N2ZiX3Byb2JlKHN0cnVjdCBpMmNfY2xpZW50ICpjbGllbnQsDQo+PiAgIA0KPj4gICAJaWYgKHBh ci0+cmVzZXQpIHsNCj4+ICAgCQkvKiBSZXNldCB0aGUgc2NyZWVuICovDQo+PiAtCQlncGlvZF9z ZXRfdmFsdWVfY2Fuc2xlZXAocGFyLT5yZXNldCwgMCk7DQo+PiArCQlncGlvZF9zZXRfdmFsdWVf Y2Fuc2xlZXAocGFyLT5yZXNldCwgcmVzZXRfYWN0aXZlX2xvdyk7DQo+PiAgIAkJdWRlbGF5KDQp Ow0KPj4gLQkJZ3Bpb2Rfc2V0X3ZhbHVlX2NhbnNsZWVwKHBhci0+cmVzZXQsIDEpOw0KPj4gKwkJ Z3Bpb2Rfc2V0X3ZhbHVlX2NhbnNsZWVwKHBhci0+cmVzZXQsICFyZXNldF9hY3RpdmVfbG93KTsN Cj4gDQo+IEkgdGhpbmsgeW91IGFuZCB3aG9tZXZlciB3cm90ZSB0aGUgb3JpZ2luYWwgY29kZSBh cmUgbWlzaW50ZXJwcmV0dGluZw0KPiBob3cgdGhlIGdwaW9kIEFQSSB3b3Jrcy4gMSBtZWFucyBt YWtlIHRoZSBzaWduYWwgYWN0aXZlIGFuZCB0aGlzIGNhc2UNCj4gYWN0aXZlIGlzIGxvdy4NCg0K SSB0b3RhbGx5IGFncmVlIGFuZCBJIHRoaW5rIEkgdW5kZXJzdGFuZCB0aGF0IGNvcnJlY3RseS4N Cg0KPiBJdCBpcyBzdHJhbmdlLCBidXQgZG9lcyBtZWFuIGEgcmVzZXQgc2VxdWVuY2Ugc2hvdWxk IGFsd2F5cyBiZSBzZXQgdG8gYQ0KPiAxIGFuZCB0aGVuIGEgMCBhbmQgaXQgd2lsbCB3b3JrIHdp dGggZWl0aGVyIHBvbGFyaXR5IGluIHRoZSBEVC4NCg0KSSBhZ3JlZSB0aGUgcmVzZXQgc2hvdWxk IGJlIGRvbmUgYXMgYSAwIC0+IDEgLT4gMCBzZXF1ZW5jZSBhbmQgdGhhdCBzaG91bGQNCmp1c3Qg d29yay4gVGhlIHByb2JsZW0gaXMgaXQgaXMgaW1wbGVtZW50ZWQgdmljZSB2ZXJzYSBhbmQgc28g aXQgd29ya3Mgb25seQ0KaWYgeW91IGhhdmUgR1BJT19BQ1RJVkVfSElHSCBpbiBEVCBmb3IgYSBz aWduYWwgdGhhdCBpcyBhY3R1YWxseSBhY3RpdmUgbG93Lg0KDQpBbmQgd2hhdCBpdCBhY3R1YWxs eSBkb2VzIGlzIHRoYXQgaXQgaG9sZHMgdGhlIGNvbnRyb2xsZXIgaW4gcmVzZXQgc2luY2UNCnRo ZSBHUElPIGlzIHN1Y2Nlc3NmdWxseSBhY3F1aXJlZCAoYmVjYXVzZSBvZiBHUElPRF9PVVRfTE9X IGhlcmUgWzFdKSBhbmQNCmxhdGVyIG9uIGl0IG9ubHkgcmVsZWFzZXMgdGhlIHJlc2V0Lg0KDQpB cyBhIERUIGF1dGhvciBJIHdvdWxkIGxpa2UgdG8gc29tZWhvdyBjbGVhcmx5IHN0YXRlIHRoYXQg dGhlIE9MRUQgZGlzcGxheQ0KdXNlcyBhY3RpdmUgbG93IHJlc2V0IGluIG15IERULg0KDQpNeSBm aXJzdCBhdHRlbXB0IHRvIGZpeCB0aGlzIHdhcyB0byBjaGFuZ2UgdGhlIHJlc2V0IHNlcXVlbmNl IFsyXS4NCkl0IHdhcyBhcHBsaWVkIGFuZCB0aGVuIHJldmVydGVkIGFzIGl0IGlzIG5vdCBiYWNr d2FyZCBjb21wYXRpYmxlIHdpdGgNCmRlcGxveWVkIERUQiBmaWxlcyBbM10uDQoNClsxXSBodHRw czovL2VsaXhpci5ib290bGluLmNvbS9saW51eC92NC4yMC1yYzMvc291cmNlL2RyaXZlcnMvdmlk ZW8vZmJkZXYvc3NkMTMwN2ZiLmMjTDU3MA0KWzJdIGh0dHBzOi8vcGF0Y2h3b3JrLmtlcm5lbC5v cmcvcGF0Y2gvMTA2MTc3MjkvDQpbM10gaHR0cHM6Ly9wYXRjaHdvcmsua2VybmVsLm9yZy9wYXRj aC8xMDYxNzczMS8NCg0KTWljaGFsDQo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the reset-active-low property 2018-11-19 15:12 ` Vokáč Michal @ 2018-11-19 22:32 ` Rob Herring -1 siblings, 0 replies; 15+ messages in thread From: Rob Herring @ 2018-11-19 22:32 UTC (permalink / raw) To: Michal Vokáč Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel, devicetree, Jyri Sarha On Mon, Nov 19, 2018 at 9:12 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote: > > On 12.11.2018 17:55, Rob Herring wrote: > > On Fri, Nov 02, 2018 at 02:56:35PM +0000, Vokáč Michal wrote: > >> The SSD130x OLED display reset signal is active low. Now the reset > >> sequence is implemented in such a way that DTS authors are forced to > >> define the reset-gpios property with GPIO_ACTIVE_HIGH to make the reset > >> work. > >> > >> Add the reset-active-low property so the signal is inverted once again > >> and the GPIO_ACTIVE_LOW work as expected. > >> > >> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com> > >> --- > >> drivers/video/fbdev/ssd1307fb.c | 6 ++++-- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c > >> index e7ae135..790f1c4 100644 > >> --- a/drivers/video/fbdev/ssd1307fb.c > >> +++ b/drivers/video/fbdev/ssd1307fb.c > >> @@ -608,6 +608,7 @@ static int ssd1307fb_probe(struct i2c_client *client, > >> struct fb_deferred_io *ssd1307fb_defio; > >> u32 vmem_size; > >> struct ssd1307fb_par *par; > >> + bool reset_active_low; > >> u8 *vmem; > >> int ret; > >> > >> @@ -671,6 +672,7 @@ static int ssd1307fb_probe(struct i2c_client *client, > >> par->com_seq = of_property_read_bool(node, "solomon,com-seq"); > >> par->com_lrremap = of_property_read_bool(node, "solomon,com-lrremap"); > >> par->com_invdir = of_property_read_bool(node, "solomon,com-invdir"); > >> + reset_active_low = of_property_read_bool(node, "reset-active-low"); > >> > >> par->contrast = 127; > >> par->vcomh = par->device_info->default_vcomh; > >> @@ -728,9 +730,9 @@ static int ssd1307fb_probe(struct i2c_client *client, > >> > >> if (par->reset) { > >> /* Reset the screen */ > >> - gpiod_set_value_cansleep(par->reset, 0); > >> + gpiod_set_value_cansleep(par->reset, reset_active_low); > >> udelay(4); > >> - gpiod_set_value_cansleep(par->reset, 1); > >> + gpiod_set_value_cansleep(par->reset, !reset_active_low); > > > > I think you and whomever wrote the original code are misinterpretting > > how the gpiod API works. 1 means make the signal active and this case > > active is low. > > I totally agree and I think I understand that correctly. > > > It is strange, but does mean a reset sequence should always be set to a > > 1 and then a 0 and it will work with either polarity in the DT. > > I agree the reset should be done as a 0 -> 1 -> 0 sequence and that should > just work. The problem is it is implemented vice versa and so it works only > if you have GPIO_ACTIVE_HIGH in DT for a signal that is actually active low. > > And what it actually does is that it holds the controller in reset since > the GPIO is successfully acquired (because of GPIOD_OUT_LOW here [1]) and > later on it only releases the reset. > > As a DT author I would like to somehow clearly state that the OLED display > uses active low reset in my DT. > > My first attempt to fix this was to change the reset sequence [2]. > It was applied and then reverted as it is not backward compatible with > deployed DTB files [3]. > > [1] https://elixir.bootlin.com/linux/v4.20-rc3/source/drivers/video/fbdev/ssd1307fb.c#L570 > [2] https://patchwork.kernel.org/patch/10617729/ > [3] https://patchwork.kernel.org/patch/10617731/ Okay, now I understand the background. We've hit this somewhere else too. Rather than have a binding demonstrating what not to do, I'd like to fix this in another way. I also don't want to live with this forever when there's only 1 board affected (in tree at least) and there's only an ABI if someone notices (I'm happy though that the maintainers caught this). There's 2 other options. The 1st is add a fixup to the DT for this platform to ensure that the GPIO is configured active low (the Versatile platform code has an example fixup). With that, apply what was originally applied (or revert the revert). The fixup could be applied only after someone complains their display broke. The 2nd option is just add an of_machine_is_compatible check within this driver. In that case, you wouldn't fix dts file for the platform (unless you also want to manually check reset-gpios). Rob ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the reset-active-low property @ 2018-11-19 22:32 ` Rob Herring 0 siblings, 0 replies; 15+ messages in thread From: Rob Herring @ 2018-11-19 22:32 UTC (permalink / raw) To: Michal Vokáč Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel, devicetree, Jyri Sarha On Mon, Nov 19, 2018 at 9:12 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote: > > On 12.11.2018 17:55, Rob Herring wrote: > > On Fri, Nov 02, 2018 at 02:56:35PM +0000, Vokáč Michal wrote: > >> The SSD130x OLED display reset signal is active low. Now the reset > >> sequence is implemented in such a way that DTS authors are forced to > >> define the reset-gpios property with GPIO_ACTIVE_HIGH to make the reset > >> work. > >> > >> Add the reset-active-low property so the signal is inverted once again > >> and the GPIO_ACTIVE_LOW work as expected. > >> > >> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com> > >> --- > >> drivers/video/fbdev/ssd1307fb.c | 6 ++++-- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c > >> index e7ae135..790f1c4 100644 > >> --- a/drivers/video/fbdev/ssd1307fb.c > >> +++ b/drivers/video/fbdev/ssd1307fb.c > >> @@ -608,6 +608,7 @@ static int ssd1307fb_probe(struct i2c_client *client, > >> struct fb_deferred_io *ssd1307fb_defio; > >> u32 vmem_size; > >> struct ssd1307fb_par *par; > >> + bool reset_active_low; > >> u8 *vmem; > >> int ret; > >> > >> @@ -671,6 +672,7 @@ static int ssd1307fb_probe(struct i2c_client *client, > >> par->com_seq = of_property_read_bool(node, "solomon,com-seq"); > >> par->com_lrremap = of_property_read_bool(node, "solomon,com-lrremap"); > >> par->com_invdir = of_property_read_bool(node, "solomon,com-invdir"); > >> + reset_active_low = of_property_read_bool(node, "reset-active-low"); > >> > >> par->contrast = 127; > >> par->vcomh = par->device_info->default_vcomh; > >> @@ -728,9 +730,9 @@ static int ssd1307fb_probe(struct i2c_client *client, > >> > >> if (par->reset) { > >> /* Reset the screen */ > >> - gpiod_set_value_cansleep(par->reset, 0); > >> + gpiod_set_value_cansleep(par->reset, reset_active_low); > >> udelay(4); > >> - gpiod_set_value_cansleep(par->reset, 1); > >> + gpiod_set_value_cansleep(par->reset, !reset_active_low); > > > > I think you and whomever wrote the original code are misinterpretting > > how the gpiod API works. 1 means make the signal active and this case > > active is low. > > I totally agree and I think I understand that correctly. > > > It is strange, but does mean a reset sequence should always be set to a > > 1 and then a 0 and it will work with either polarity in the DT. > > I agree the reset should be done as a 0 -> 1 -> 0 sequence and that should > just work. The problem is it is implemented vice versa and so it works only > if you have GPIO_ACTIVE_HIGH in DT for a signal that is actually active low. > > And what it actually does is that it holds the controller in reset since > the GPIO is successfully acquired (because of GPIOD_OUT_LOW here [1]) and > later on it only releases the reset. > > As a DT author I would like to somehow clearly state that the OLED display > uses active low reset in my DT. > > My first attempt to fix this was to change the reset sequence [2]. > It was applied and then reverted as it is not backward compatible with > deployed DTB files [3]. > > [1] https://elixir.bootlin.com/linux/v4.20-rc3/source/drivers/video/fbdev/ssd1307fb.c#L570 > [2] https://patchwork.kernel.org/patch/10617729/ > [3] https://patchwork.kernel.org/patch/10617731/ Okay, now I understand the background. We've hit this somewhere else too. Rather than have a binding demonstrating what not to do, I'd like to fix this in another way. I also don't want to live with this forever when there's only 1 board affected (in tree at least) and there's only an ABI if someone notices (I'm happy though that the maintainers caught this). There's 2 other options. The 1st is add a fixup to the DT for this platform to ensure that the GPIO is configured active low (the Versatile platform code has an example fixup). With that, apply what was originally applied (or revert the revert). The fixup could be applied only after someone complains their display broke. The 2nd option is just add an of_machine_is_compatible check within this driver. In that case, you wouldn't fix dts file for the platform (unless you also want to manually check reset-gpios). Rob ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the reset-active-low property 2018-11-19 22:32 ` Rob Herring @ 2018-11-26 12:25 ` Vokáč Michal -1 siblings, 0 replies; 15+ messages in thread From: Vokáč Michal @ 2018-11-26 12:25 UTC (permalink / raw) To: Rob Herring Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel, devicetree, Jyri Sarha On 19.11.2018 23:32, Rob Herring wrote: > On Mon, Nov 19, 2018 at 9:12 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote: >> On 12.11.2018 17:55, Rob Herring wrote: >>> On Fri, Nov 02, 2018 at 02:56:35PM +0000, Vokáč Michal wrote: >>>> The SSD130x OLED display reset signal is active low. Now the reset >>>> sequence is implemented in such a way that DTS authors are forced to >>>> define the reset-gpios property with GPIO_ACTIVE_HIGH to make the reset >>>> work. >>>> >>>> Add the reset-active-low property so the signal is inverted once again >>>> and the GPIO_ACTIVE_LOW work as expected. >>>> >>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com> >>>> --- >>>> drivers/video/fbdev/ssd1307fb.c | 6 ++++-- >>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c >>>> index e7ae135..790f1c4 100644 >>>> --- a/drivers/video/fbdev/ssd1307fb.c >>>> +++ b/drivers/video/fbdev/ssd1307fb.c >>>> @@ -608,6 +608,7 @@ static int ssd1307fb_probe(struct i2c_client *client, >>>> struct fb_deferred_io *ssd1307fb_defio; >>>> u32 vmem_size; >>>> struct ssd1307fb_par *par; >>>> + bool reset_active_low; >>>> u8 *vmem; >>>> int ret; >>>> >>>> @@ -671,6 +672,7 @@ static int ssd1307fb_probe(struct i2c_client *client, >>>> par->com_seq = of_property_read_bool(node, "solomon,com-seq"); >>>> par->com_lrremap = of_property_read_bool(node, "solomon,com-lrremap"); >>>> par->com_invdir = of_property_read_bool(node, "solomon,com-invdir"); >>>> + reset_active_low = of_property_read_bool(node, "reset-active-low"); >>>> >>>> par->contrast = 127; >>>> par->vcomh = par->device_info->default_vcomh; >>>> @@ -728,9 +730,9 @@ static int ssd1307fb_probe(struct i2c_client *client, >>>> >>>> if (par->reset) { >>>> /* Reset the screen */ >>>> - gpiod_set_value_cansleep(par->reset, 0); >>>> + gpiod_set_value_cansleep(par->reset, reset_active_low); >>>> udelay(4); >>>> - gpiod_set_value_cansleep(par->reset, 1); >>>> + gpiod_set_value_cansleep(par->reset, !reset_active_low); >>> >>> I think you and whomever wrote the original code are misinterpretting >>> how the gpiod API works. 1 means make the signal active and this case >>> active is low. >> >> I totally agree and I think I understand that correctly. >> >>> It is strange, but does mean a reset sequence should always be set to a >>> 1 and then a 0 and it will work with either polarity in the DT. >> >> I agree the reset should be done as a 0 -> 1 -> 0 sequence and that should >> just work. The problem is it is implemented vice versa and so it works only >> if you have GPIO_ACTIVE_HIGH in DT for a signal that is actually active low. >> >> And what it actually does is that it holds the controller in reset since >> the GPIO is successfully acquired (because of GPIOD_OUT_LOW here [1]) and >> later on it only releases the reset. >> >> As a DT author I would like to somehow clearly state that the OLED display >> uses active low reset in my DT. >> >> My first attempt to fix this was to change the reset sequence [2]. >> It was applied and then reverted as it is not backward compatible with >> deployed DTB files [3]. >> >> [1] https://elixir.bootlin.com/linux/v4.20-rc3/source/drivers/video/fbdev/ssd1307fb.c#L570 >> [2] https://patchwork.kernel.org/patch/10617729/ >> [3] https://patchwork.kernel.org/patch/10617731/ > > Okay, now I understand the background. We've hit this somewhere else too. > > Rather than have a binding demonstrating what not to do, I'd like to > fix this in another way. I also don't want to live with this forever > when there's only 1 board affected (in tree at least) and there's only > an ABI if someone notices (I'm happy though that the maintainers > caught this). There's 2 other options. The 1st is add a fixup to the > DT for this platform to ensure that the GPIO is configured active low > (the Versatile platform code has an example fixup). With that, apply > what was originally applied (or revert the revert). The fixup could be > applied only after someone complains their display broke. The 2nd > option is just add an of_machine_is_compatible check within this > driver. In that case, you wouldn't fix dts file for the platform > (unless you also want to manually check reset-gpios). Hi Rob, I am still trying to figure out what exactly you meant by the 1st and 2nd option. Both concepts are new to me. Regarding the 1st option, what you meant by "this platform" here: > Add a fixup to the DT for this platform The only board in tree that uses the OLED (imx28-cfa10036) and its dts file? I am also not sure where to look for the example. When you say Versatile platform code I tend to look into plat-versatile or mach-versatile. I could not find anything I could use as an example in there. I think that is not what you meant. Regarding the 2nd option, you suggest to use of_machine_is_compatible to decide what reset sequence to use? In case of imx28-cfa10036 use the old 0 -> 1, in all other cases use a new correct sequence 1 -> 0? That also does not seem right. I will appreciate more details how to proceed. Thank you, Michal ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the reset-active-low property @ 2018-11-26 12:25 ` Vokáč Michal 0 siblings, 0 replies; 15+ messages in thread From: Vokáč Michal @ 2018-11-26 12:25 UTC (permalink / raw) To: Rob Herring Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel, devicetree, Jyri Sarha T24gMTkuMTEuMjAxOCAyMzozMiwgUm9iIEhlcnJpbmcgd3JvdGU6DQo+IE9uIE1vbiwgTm92IDE5 LCAyMDE4IGF0IDk6MTIgQU0gVm9rw6HEjSBNaWNoYWwgPE1pY2hhbC5Wb2thY0B5c29mdC5jb20+ IHdyb3RlOg0KPj4gT24gMTIuMTEuMjAxOCAxNzo1NSwgUm9iIEhlcnJpbmcgd3JvdGU6DQo+Pj4g T24gRnJpLCBOb3YgMDIsIDIwMTggYXQgMDI6NTY6MzVQTSArMDAwMCwgVm9rw6HEjSBNaWNoYWwg d3JvdGU6DQo+Pj4+IFRoZSBTU0QxMzB4IE9MRUQgZGlzcGxheSByZXNldCBzaWduYWwgaXMgYWN0 aXZlIGxvdy4gTm93IHRoZSByZXNldA0KPj4+PiBzZXF1ZW5jZSBpcyBpbXBsZW1lbnRlZCBpbiBz dWNoIGEgd2F5IHRoYXQgRFRTIGF1dGhvcnMgYXJlIGZvcmNlZCB0bw0KPj4+PiBkZWZpbmUgdGhl IHJlc2V0LWdwaW9zIHByb3BlcnR5IHdpdGggR1BJT19BQ1RJVkVfSElHSCB0byBtYWtlIHRoZSBy ZXNldA0KPj4+PiB3b3JrLg0KPj4+Pg0KPj4+PiBBZGQgdGhlIHJlc2V0LWFjdGl2ZS1sb3cgcHJv cGVydHkgc28gdGhlIHNpZ25hbCBpcyBpbnZlcnRlZCBvbmNlIGFnYWluDQo+Pj4+IGFuZCB0aGUg R1BJT19BQ1RJVkVfTE9XIHdvcmsgYXMgZXhwZWN0ZWQuDQo+Pj4+DQo+Pj4+IFNpZ25lZC1vZmYt Ynk6IE1pY2hhbCBWb2vDocSNIDxtaWNoYWwudm9rYWNAeXNvZnQuY29tPg0KPj4+PiAtLS0NCj4+ Pj4gICAgZHJpdmVycy92aWRlby9mYmRldi9zc2QxMzA3ZmIuYyB8IDYgKysrKy0tDQo+Pj4+ICAg IDEgZmlsZSBjaGFuZ2VkLCA0IGluc2VydGlvbnMoKyksIDIgZGVsZXRpb25zKC0pDQo+Pj4+DQo+ Pj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3ZpZGVvL2ZiZGV2L3NzZDEzMDdmYi5jIGIvZHJpdmVy cy92aWRlby9mYmRldi9zc2QxMzA3ZmIuYw0KPj4+PiBpbmRleCBlN2FlMTM1Li43OTBmMWM0IDEw MDY0NA0KPj4+PiAtLS0gYS9kcml2ZXJzL3ZpZGVvL2ZiZGV2L3NzZDEzMDdmYi5jDQo+Pj4+ICsr KyBiL2RyaXZlcnMvdmlkZW8vZmJkZXYvc3NkMTMwN2ZiLmMNCj4+Pj4gQEAgLTYwOCw2ICs2MDgs NyBAQCBzdGF0aWMgaW50IHNzZDEzMDdmYl9wcm9iZShzdHJ1Y3QgaTJjX2NsaWVudCAqY2xpZW50 LA0KPj4+PiAgICAgICBzdHJ1Y3QgZmJfZGVmZXJyZWRfaW8gKnNzZDEzMDdmYl9kZWZpbzsNCj4+ Pj4gICAgICAgdTMyIHZtZW1fc2l6ZTsNCj4+Pj4gICAgICAgc3RydWN0IHNzZDEzMDdmYl9wYXIg KnBhcjsNCj4+Pj4gKyAgICBib29sIHJlc2V0X2FjdGl2ZV9sb3c7DQo+Pj4+ICAgICAgIHU4ICp2 bWVtOw0KPj4+PiAgICAgICBpbnQgcmV0Ow0KPj4+Pg0KPj4+PiBAQCAtNjcxLDYgKzY3Miw3IEBA IHN0YXRpYyBpbnQgc3NkMTMwN2ZiX3Byb2JlKHN0cnVjdCBpMmNfY2xpZW50ICpjbGllbnQsDQo+ Pj4+ICAgICAgIHBhci0+Y29tX3NlcSA9IG9mX3Byb3BlcnR5X3JlYWRfYm9vbChub2RlLCAic29s b21vbixjb20tc2VxIik7DQo+Pj4+ICAgICAgIHBhci0+Y29tX2xycmVtYXAgPSBvZl9wcm9wZXJ0 eV9yZWFkX2Jvb2wobm9kZSwgInNvbG9tb24sY29tLWxycmVtYXAiKTsNCj4+Pj4gICAgICAgcGFy LT5jb21faW52ZGlyID0gb2ZfcHJvcGVydHlfcmVhZF9ib29sKG5vZGUsICJzb2xvbW9uLGNvbS1p bnZkaXIiKTsNCj4+Pj4gKyAgICByZXNldF9hY3RpdmVfbG93ID0gb2ZfcHJvcGVydHlfcmVhZF9i b29sKG5vZGUsICJyZXNldC1hY3RpdmUtbG93Iik7DQo+Pj4+DQo+Pj4+ICAgICAgIHBhci0+Y29u dHJhc3QgPSAxMjc7DQo+Pj4+ICAgICAgIHBhci0+dmNvbWggPSBwYXItPmRldmljZV9pbmZvLT5k ZWZhdWx0X3Zjb21oOw0KPj4+PiBAQCAtNzI4LDkgKzczMCw5IEBAIHN0YXRpYyBpbnQgc3NkMTMw N2ZiX3Byb2JlKHN0cnVjdCBpMmNfY2xpZW50ICpjbGllbnQsDQo+Pj4+DQo+Pj4+ICAgICAgIGlm IChwYXItPnJlc2V0KSB7DQo+Pj4+ICAgICAgICAgICAgICAgLyogUmVzZXQgdGhlIHNjcmVlbiAq Lw0KPj4+PiAtICAgICAgICAgICAgZ3Bpb2Rfc2V0X3ZhbHVlX2NhbnNsZWVwKHBhci0+cmVzZXQs IDApOw0KPj4+PiArICAgICAgICAgICAgZ3Bpb2Rfc2V0X3ZhbHVlX2NhbnNsZWVwKHBhci0+cmVz ZXQsIHJlc2V0X2FjdGl2ZV9sb3cpOw0KPj4+PiAgICAgICAgICAgICAgIHVkZWxheSg0KTsNCj4+ Pj4gLSAgICAgICAgICAgIGdwaW9kX3NldF92YWx1ZV9jYW5zbGVlcChwYXItPnJlc2V0LCAxKTsN Cj4+Pj4gKyAgICAgICAgICAgIGdwaW9kX3NldF92YWx1ZV9jYW5zbGVlcChwYXItPnJlc2V0LCAh cmVzZXRfYWN0aXZlX2xvdyk7DQo+Pj4NCj4+PiBJIHRoaW5rIHlvdSBhbmQgd2hvbWV2ZXIgd3Jv dGUgdGhlIG9yaWdpbmFsIGNvZGUgYXJlIG1pc2ludGVycHJldHRpbmcNCj4+PiBob3cgdGhlIGdw aW9kIEFQSSB3b3Jrcy4gMSBtZWFucyBtYWtlIHRoZSBzaWduYWwgYWN0aXZlIGFuZCB0aGlzIGNh c2UNCj4+PiBhY3RpdmUgaXMgbG93Lg0KPj4NCj4+IEkgdG90YWxseSBhZ3JlZSBhbmQgSSB0aGlu ayBJIHVuZGVyc3RhbmQgdGhhdCBjb3JyZWN0bHkuDQo+Pg0KPj4+IEl0IGlzIHN0cmFuZ2UsIGJ1 dCBkb2VzIG1lYW4gYSByZXNldCBzZXF1ZW5jZSBzaG91bGQgYWx3YXlzIGJlIHNldCB0byBhDQo+ Pj4gMSBhbmQgdGhlbiBhIDAgYW5kIGl0IHdpbGwgd29yayB3aXRoIGVpdGhlciBwb2xhcml0eSBp biB0aGUgRFQuDQo+Pg0KPj4gSSBhZ3JlZSB0aGUgcmVzZXQgc2hvdWxkIGJlIGRvbmUgYXMgYSAw IC0+IDEgLT4gMCBzZXF1ZW5jZSBhbmQgdGhhdCBzaG91bGQNCj4+IGp1c3Qgd29yay4gVGhlIHBy b2JsZW0gaXMgaXQgaXMgaW1wbGVtZW50ZWQgdmljZSB2ZXJzYSBhbmQgc28gaXQgd29ya3Mgb25s eQ0KPj4gaWYgeW91IGhhdmUgR1BJT19BQ1RJVkVfSElHSCBpbiBEVCBmb3IgYSBzaWduYWwgdGhh dCBpcyBhY3R1YWxseSBhY3RpdmUgbG93Lg0KPj4NCj4+IEFuZCB3aGF0IGl0IGFjdHVhbGx5IGRv ZXMgaXMgdGhhdCBpdCBob2xkcyB0aGUgY29udHJvbGxlciBpbiByZXNldCBzaW5jZQ0KPj4gdGhl IEdQSU8gaXMgc3VjY2Vzc2Z1bGx5IGFjcXVpcmVkIChiZWNhdXNlIG9mIEdQSU9EX09VVF9MT1cg aGVyZSBbMV0pIGFuZA0KPj4gbGF0ZXIgb24gaXQgb25seSByZWxlYXNlcyB0aGUgcmVzZXQuDQo+ Pg0KPj4gQXMgYSBEVCBhdXRob3IgSSB3b3VsZCBsaWtlIHRvIHNvbWVob3cgY2xlYXJseSBzdGF0 ZSB0aGF0IHRoZSBPTEVEIGRpc3BsYXkNCj4+IHVzZXMgYWN0aXZlIGxvdyByZXNldCBpbiBteSBE VC4NCj4+DQo+PiBNeSBmaXJzdCBhdHRlbXB0IHRvIGZpeCB0aGlzIHdhcyB0byBjaGFuZ2UgdGhl IHJlc2V0IHNlcXVlbmNlIFsyXS4NCj4+IEl0IHdhcyBhcHBsaWVkIGFuZCB0aGVuIHJldmVydGVk IGFzIGl0IGlzIG5vdCBiYWNrd2FyZCBjb21wYXRpYmxlIHdpdGgNCj4+IGRlcGxveWVkIERUQiBm aWxlcyBbM10uDQo+Pg0KPj4gWzFdIGh0dHBzOi8vZWxpeGlyLmJvb3RsaW4uY29tL2xpbnV4L3Y0 LjIwLXJjMy9zb3VyY2UvZHJpdmVycy92aWRlby9mYmRldi9zc2QxMzA3ZmIuYyNMNTcwDQo+PiBb Ml0gaHR0cHM6Ly9wYXRjaHdvcmsua2VybmVsLm9yZy9wYXRjaC8xMDYxNzcyOS8NCj4+IFszXSBo dHRwczovL3BhdGNod29yay5rZXJuZWwub3JnL3BhdGNoLzEwNjE3NzMxLw0KPiANCj4gT2theSwg bm93IEkgdW5kZXJzdGFuZCB0aGUgYmFja2dyb3VuZC4gV2UndmUgaGl0IHRoaXMgc29tZXdoZXJl IGVsc2UgdG9vLg0KPiANCj4gUmF0aGVyIHRoYW4gaGF2ZSBhIGJpbmRpbmcgZGVtb25zdHJhdGlu ZyB3aGF0IG5vdCB0byBkbywgSSdkIGxpa2UgdG8NCj4gZml4IHRoaXMgaW4gYW5vdGhlciB3YXku IEkgYWxzbyBkb24ndCB3YW50IHRvIGxpdmUgd2l0aCB0aGlzIGZvcmV2ZXINCj4gd2hlbiB0aGVy ZSdzIG9ubHkgMSBib2FyZCBhZmZlY3RlZCAoaW4gdHJlZSBhdCBsZWFzdCkgYW5kIHRoZXJlJ3Mg b25seQ0KPiBhbiBBQkkgaWYgc29tZW9uZSBub3RpY2VzIChJJ20gaGFwcHkgdGhvdWdoIHRoYXQg dGhlIG1haW50YWluZXJzDQo+IGNhdWdodCB0aGlzKS4gVGhlcmUncyAyIG90aGVyIG9wdGlvbnMu IFRoZSAxc3QgaXMgYWRkIGEgZml4dXAgdG8gdGhlDQo+IERUIGZvciB0aGlzIHBsYXRmb3JtIHRv IGVuc3VyZSB0aGF0IHRoZSBHUElPIGlzIGNvbmZpZ3VyZWQgYWN0aXZlIGxvdw0KPiAodGhlIFZl cnNhdGlsZSBwbGF0Zm9ybSBjb2RlIGhhcyBhbiBleGFtcGxlIGZpeHVwKS4gV2l0aCB0aGF0LCBh cHBseQ0KPiB3aGF0IHdhcyBvcmlnaW5hbGx5IGFwcGxpZWQgKG9yIHJldmVydCB0aGUgcmV2ZXJ0 KS4gVGhlIGZpeHVwIGNvdWxkIGJlDQo+IGFwcGxpZWQgb25seSBhZnRlciBzb21lb25lIGNvbXBs YWlucyB0aGVpciBkaXNwbGF5IGJyb2tlLiBUaGUgMm5kDQo+IG9wdGlvbiBpcyBqdXN0IGFkZCBh biBvZl9tYWNoaW5lX2lzX2NvbXBhdGlibGUgY2hlY2sgd2l0aGluIHRoaXMNCj4gZHJpdmVyLiBJ biB0aGF0IGNhc2UsIHlvdSB3b3VsZG4ndCBmaXggZHRzIGZpbGUgZm9yIHRoZSBwbGF0Zm9ybQ0K PiAodW5sZXNzIHlvdSBhbHNvIHdhbnQgdG8gbWFudWFsbHkgY2hlY2sgcmVzZXQtZ3Bpb3MpLg0K DQpIaSBSb2IsDQoNCkkgYW0gc3RpbGwgdHJ5aW5nIHRvIGZpZ3VyZSBvdXQgd2hhdCBleGFjdGx5 IHlvdSBtZWFudCBieSB0aGUgMXN0IGFuZA0KMm5kIG9wdGlvbi4gQm90aCBjb25jZXB0cyBhcmUg bmV3IHRvIG1lLg0KDQpSZWdhcmRpbmcgdGhlIDFzdCBvcHRpb24sIHdoYXQgeW91IG1lYW50IGJ5 ICJ0aGlzIHBsYXRmb3JtIiBoZXJlOg0KPiBBZGQgYSBmaXh1cCB0byB0aGUgRFQgZm9yIHRoaXMg cGxhdGZvcm0NClRoZSBvbmx5IGJvYXJkIGluIHRyZWUgdGhhdCB1c2VzIHRoZSBPTEVEIChpbXgy OC1jZmExMDAzNikgYW5kIGl0cw0KZHRzIGZpbGU/DQoNCkkgYW0gYWxzbyBub3Qgc3VyZSB3aGVy ZSB0byBsb29rIGZvciB0aGUgZXhhbXBsZS4gV2hlbiB5b3Ugc2F5DQpWZXJzYXRpbGUgcGxhdGZv cm0gY29kZSBJIHRlbmQgdG8gbG9vayBpbnRvIHBsYXQtdmVyc2F0aWxlIG9yDQptYWNoLXZlcnNh dGlsZS4gSSBjb3VsZCBub3QgZmluZCBhbnl0aGluZyBJIGNvdWxkIHVzZSBhcyBhbiBleGFtcGxl DQppbiB0aGVyZS4gSSB0aGluayB0aGF0IGlzIG5vdCB3aGF0IHlvdSBtZWFudC4NCg0KUmVnYXJk aW5nIHRoZSAybmQgb3B0aW9uLCB5b3Ugc3VnZ2VzdCB0byB1c2Ugb2ZfbWFjaGluZV9pc19jb21w YXRpYmxlDQp0byBkZWNpZGUgd2hhdCByZXNldCBzZXF1ZW5jZSB0byB1c2U/IEluIGNhc2Ugb2Yg aW14MjgtY2ZhMTAwMzYgdXNlDQp0aGUgb2xkIDAgLT4gMSwgaW4gYWxsIG90aGVyIGNhc2VzIHVz ZSBhIG5ldyBjb3JyZWN0IHNlcXVlbmNlIDEgLT4gMD8NClRoYXQgYWxzbyBkb2VzIG5vdCBzZWVt IHJpZ2h0Lg0KDQpJIHdpbGwgYXBwcmVjaWF0ZSBtb3JlIGRldGFpbHMgaG93IHRvIHByb2NlZWQu IFRoYW5rIHlvdSwNCk1pY2hhbA0K ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the reset-active-low property 2018-11-26 12:25 ` Vokáč Michal @ 2018-11-26 13:49 ` Rob Herring -1 siblings, 0 replies; 15+ messages in thread From: Rob Herring @ 2018-11-26 13:49 UTC (permalink / raw) To: Michal Vokáč Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel, devicetree, Jyri Sarha On Mon, Nov 26, 2018 at 6:25 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote: > > On 19.11.2018 23:32, Rob Herring wrote: > > On Mon, Nov 19, 2018 at 9:12 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote: > >> On 12.11.2018 17:55, Rob Herring wrote: > >>> On Fri, Nov 02, 2018 at 02:56:35PM +0000, Vokáč Michal wrote: > >>>> The SSD130x OLED display reset signal is active low. Now the reset > >>>> sequence is implemented in such a way that DTS authors are forced to > >>>> define the reset-gpios property with GPIO_ACTIVE_HIGH to make the reset > >>>> work. > >>>> > >>>> Add the reset-active-low property so the signal is inverted once again > >>>> and the GPIO_ACTIVE_LOW work as expected. > >>>> > >>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com> > >>>> --- > >>>> drivers/video/fbdev/ssd1307fb.c | 6 ++++-- > >>>> 1 file changed, 4 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c > >>>> index e7ae135..790f1c4 100644 > >>>> --- a/drivers/video/fbdev/ssd1307fb.c > >>>> +++ b/drivers/video/fbdev/ssd1307fb.c > >>>> @@ -608,6 +608,7 @@ static int ssd1307fb_probe(struct i2c_client *client, > >>>> struct fb_deferred_io *ssd1307fb_defio; > >>>> u32 vmem_size; > >>>> struct ssd1307fb_par *par; > >>>> + bool reset_active_low; > >>>> u8 *vmem; > >>>> int ret; > >>>> > >>>> @@ -671,6 +672,7 @@ static int ssd1307fb_probe(struct i2c_client *client, > >>>> par->com_seq = of_property_read_bool(node, "solomon,com-seq"); > >>>> par->com_lrremap = of_property_read_bool(node, "solomon,com-lrremap"); > >>>> par->com_invdir = of_property_read_bool(node, "solomon,com-invdir"); > >>>> + reset_active_low = of_property_read_bool(node, "reset-active-low"); > >>>> > >>>> par->contrast = 127; > >>>> par->vcomh = par->device_info->default_vcomh; > >>>> @@ -728,9 +730,9 @@ static int ssd1307fb_probe(struct i2c_client *client, > >>>> > >>>> if (par->reset) { > >>>> /* Reset the screen */ > >>>> - gpiod_set_value_cansleep(par->reset, 0); > >>>> + gpiod_set_value_cansleep(par->reset, reset_active_low); > >>>> udelay(4); > >>>> - gpiod_set_value_cansleep(par->reset, 1); > >>>> + gpiod_set_value_cansleep(par->reset, !reset_active_low); > >>> > >>> I think you and whomever wrote the original code are misinterpretting > >>> how the gpiod API works. 1 means make the signal active and this case > >>> active is low. > >> > >> I totally agree and I think I understand that correctly. > >> > >>> It is strange, but does mean a reset sequence should always be set to a > >>> 1 and then a 0 and it will work with either polarity in the DT. > >> > >> I agree the reset should be done as a 0 -> 1 -> 0 sequence and that should > >> just work. The problem is it is implemented vice versa and so it works only > >> if you have GPIO_ACTIVE_HIGH in DT for a signal that is actually active low. > >> > >> And what it actually does is that it holds the controller in reset since > >> the GPIO is successfully acquired (because of GPIOD_OUT_LOW here [1]) and > >> later on it only releases the reset. > >> > >> As a DT author I would like to somehow clearly state that the OLED display > >> uses active low reset in my DT. > >> > >> My first attempt to fix this was to change the reset sequence [2]. > >> It was applied and then reverted as it is not backward compatible with > >> deployed DTB files [3]. > >> > >> [1] https://elixir.bootlin.com/linux/v4.20-rc3/source/drivers/video/fbdev/ssd1307fb.c#L570 > >> [2] https://patchwork.kernel.org/patch/10617729/ > >> [3] https://patchwork.kernel.org/patch/10617731/ > > > > Okay, now I understand the background. We've hit this somewhere else too. > > > > Rather than have a binding demonstrating what not to do, I'd like to > > fix this in another way. I also don't want to live with this forever > > when there's only 1 board affected (in tree at least) and there's only > > an ABI if someone notices (I'm happy though that the maintainers > > caught this). There's 2 other options. The 1st is add a fixup to the > > DT for this platform to ensure that the GPIO is configured active low > > (the Versatile platform code has an example fixup). With that, apply > > what was originally applied (or revert the revert). The fixup could be > > applied only after someone complains their display broke. The 2nd > > option is just add an of_machine_is_compatible check within this > > driver. In that case, you wouldn't fix dts file for the platform > > (unless you also want to manually check reset-gpios). > > Hi Rob, > > I am still trying to figure out what exactly you meant by the 1st and > 2nd option. Both concepts are new to me. > > Regarding the 1st option, what you meant by "this platform" here: > > Add a fixup to the DT for this platform > The only board in tree that uses the OLED (imx28-cfa10036) and its > dts file? Yes, that one. > > I am also not sure where to look for the example. When you say > Versatile platform code I tend to look into plat-versatile or > mach-versatile. I could not find anything I could use as an example > in there. I think that is not what you meant. See versatile_dt_pci_init(). Or look for other callers of of_update_property(). > Regarding the 2nd option, you suggest to use of_machine_is_compatible > to decide what reset sequence to use? In case of imx28-cfa10036 use > the old 0 -> 1, in all other cases use a new correct sequence 1 -> 0? > That also does not seem right. Correct. Though if you fix imx28-cfa10036 dts, then you have to handle that case too. Why is it not right? Ugly yes, but it's not wrong. Rob ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the reset-active-low property @ 2018-11-26 13:49 ` Rob Herring 0 siblings, 0 replies; 15+ messages in thread From: Rob Herring @ 2018-11-26 13:49 UTC (permalink / raw) To: Michal Vokáč Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel, devicetree, Jyri Sarha On Mon, Nov 26, 2018 at 6:25 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote: > > On 19.11.2018 23:32, Rob Herring wrote: > > On Mon, Nov 19, 2018 at 9:12 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote: > >> On 12.11.2018 17:55, Rob Herring wrote: > >>> On Fri, Nov 02, 2018 at 02:56:35PM +0000, Vokáč Michal wrote: > >>>> The SSD130x OLED display reset signal is active low. Now the reset > >>>> sequence is implemented in such a way that DTS authors are forced to > >>>> define the reset-gpios property with GPIO_ACTIVE_HIGH to make the reset > >>>> work. > >>>> > >>>> Add the reset-active-low property so the signal is inverted once again > >>>> and the GPIO_ACTIVE_LOW work as expected. > >>>> > >>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com> > >>>> --- > >>>> drivers/video/fbdev/ssd1307fb.c | 6 ++++-- > >>>> 1 file changed, 4 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c > >>>> index e7ae135..790f1c4 100644 > >>>> --- a/drivers/video/fbdev/ssd1307fb.c > >>>> +++ b/drivers/video/fbdev/ssd1307fb.c > >>>> @@ -608,6 +608,7 @@ static int ssd1307fb_probe(struct i2c_client *client, > >>>> struct fb_deferred_io *ssd1307fb_defio; > >>>> u32 vmem_size; > >>>> struct ssd1307fb_par *par; > >>>> + bool reset_active_low; > >>>> u8 *vmem; > >>>> int ret; > >>>> > >>>> @@ -671,6 +672,7 @@ static int ssd1307fb_probe(struct i2c_client *client, > >>>> par->com_seq = of_property_read_bool(node, "solomon,com-seq"); > >>>> par->com_lrremap = of_property_read_bool(node, "solomon,com-lrremap"); > >>>> par->com_invdir = of_property_read_bool(node, "solomon,com-invdir"); > >>>> + reset_active_low = of_property_read_bool(node, "reset-active-low"); > >>>> > >>>> par->contrast = 127; > >>>> par->vcomh = par->device_info->default_vcomh; > >>>> @@ -728,9 +730,9 @@ static int ssd1307fb_probe(struct i2c_client *client, > >>>> > >>>> if (par->reset) { > >>>> /* Reset the screen */ > >>>> - gpiod_set_value_cansleep(par->reset, 0); > >>>> + gpiod_set_value_cansleep(par->reset, reset_active_low); > >>>> udelay(4); > >>>> - gpiod_set_value_cansleep(par->reset, 1); > >>>> + gpiod_set_value_cansleep(par->reset, !reset_active_low); > >>> > >>> I think you and whomever wrote the original code are misinterpretting > >>> how the gpiod API works. 1 means make the signal active and this case > >>> active is low. > >> > >> I totally agree and I think I understand that correctly. > >> > >>> It is strange, but does mean a reset sequence should always be set to a > >>> 1 and then a 0 and it will work with either polarity in the DT. > >> > >> I agree the reset should be done as a 0 -> 1 -> 0 sequence and that should > >> just work. The problem is it is implemented vice versa and so it works only > >> if you have GPIO_ACTIVE_HIGH in DT for a signal that is actually active low. > >> > >> And what it actually does is that it holds the controller in reset since > >> the GPIO is successfully acquired (because of GPIOD_OUT_LOW here [1]) and > >> later on it only releases the reset. > >> > >> As a DT author I would like to somehow clearly state that the OLED display > >> uses active low reset in my DT. > >> > >> My first attempt to fix this was to change the reset sequence [2]. > >> It was applied and then reverted as it is not backward compatible with > >> deployed DTB files [3]. > >> > >> [1] https://elixir.bootlin.com/linux/v4.20-rc3/source/drivers/video/fbdev/ssd1307fb.c#L570 > >> [2] https://patchwork.kernel.org/patch/10617729/ > >> [3] https://patchwork.kernel.org/patch/10617731/ > > > > Okay, now I understand the background. We've hit this somewhere else too. > > > > Rather than have a binding demonstrating what not to do, I'd like to > > fix this in another way. I also don't want to live with this forever > > when there's only 1 board affected (in tree at least) and there's only > > an ABI if someone notices (I'm happy though that the maintainers > > caught this). There's 2 other options. The 1st is add a fixup to the > > DT for this platform to ensure that the GPIO is configured active low > > (the Versatile platform code has an example fixup). With that, apply > > what was originally applied (or revert the revert). The fixup could be > > applied only after someone complains their display broke. The 2nd > > option is just add an of_machine_is_compatible check within this > > driver. In that case, you wouldn't fix dts file for the platform > > (unless you also want to manually check reset-gpios). > > Hi Rob, > > I am still trying to figure out what exactly you meant by the 1st and > 2nd option. Both concepts are new to me. > > Regarding the 1st option, what you meant by "this platform" here: > > Add a fixup to the DT for this platform > The only board in tree that uses the OLED (imx28-cfa10036) and its > dts file? Yes, that one. > > I am also not sure where to look for the example. When you say > Versatile platform code I tend to look into plat-versatile or > mach-versatile. I could not find anything I could use as an example > in there. I think that is not what you meant. See versatile_dt_pci_init(). Or look for other callers of of_update_property(). > Regarding the 2nd option, you suggest to use of_machine_is_compatible > to decide what reset sequence to use? In case of imx28-cfa10036 use > the old 0 -> 1, in all other cases use a new correct sequence 1 -> 0? > That also does not seem right. Correct. Though if you fix imx28-cfa10036 dts, then you have to handle that case too. Why is it not right? Ugly yes, but it's not wrong. Rob ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the reset-active-low property 2018-11-26 13:49 ` Rob Herring @ 2018-11-26 14:20 ` Vokáč Michal -1 siblings, 0 replies; 15+ messages in thread From: Vokáč Michal @ 2018-11-26 14:20 UTC (permalink / raw) To: Rob Herring Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel, devicetree, Jyri Sarha On 26.11.2018 14:49, Rob Herring wrote: > On Mon, Nov 26, 2018 at 6:25 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote: >> >> On 19.11.2018 23:32, Rob Herring wrote: >>> On Mon, Nov 19, 2018 at 9:12 AM Vokáč Michal <Michal.Vokac@ysoft.com> wrote: >>>> On 12.11.2018 17:55, Rob Herring wrote: >>>>> On Fri, Nov 02, 2018 at 02:56:35PM +0000, Vokáč Michal wrote: >>>>>> The SSD130x OLED display reset signal is active low. Now the reset >>>>>> sequence is implemented in such a way that DTS authors are forced to >>>>>> define the reset-gpios property with GPIO_ACTIVE_HIGH to make the reset >>>>>> work. >>>>>> >>>>>> Add the reset-active-low property so the signal is inverted once again >>>>>> and the GPIO_ACTIVE_LOW work as expected. >>>>>> >>>>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com> >>>>>> --- >>>>>> drivers/video/fbdev/ssd1307fb.c | 6 ++++-- >>>>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c >>>>>> index e7ae135..790f1c4 100644 >>>>>> --- a/drivers/video/fbdev/ssd1307fb.c >>>>>> +++ b/drivers/video/fbdev/ssd1307fb.c >>>>>> @@ -608,6 +608,7 @@ static int ssd1307fb_probe(struct i2c_client *client, >>>>>> struct fb_deferred_io *ssd1307fb_defio; >>>>>> u32 vmem_size; >>>>>> struct ssd1307fb_par *par; >>>>>> + bool reset_active_low; >>>>>> u8 *vmem; >>>>>> int ret; >>>>>> >>>>>> @@ -671,6 +672,7 @@ static int ssd1307fb_probe(struct i2c_client *client, >>>>>> par->com_seq = of_property_read_bool(node, "solomon,com-seq"); >>>>>> par->com_lrremap = of_property_read_bool(node, "solomon,com-lrremap"); >>>>>> par->com_invdir = of_property_read_bool(node, "solomon,com-invdir"); >>>>>> + reset_active_low = of_property_read_bool(node, "reset-active-low"); >>>>>> >>>>>> par->contrast = 127; >>>>>> par->vcomh = par->device_info->default_vcomh; >>>>>> @@ -728,9 +730,9 @@ static int ssd1307fb_probe(struct i2c_client *client, >>>>>> >>>>>> if (par->reset) { >>>>>> /* Reset the screen */ >>>>>> - gpiod_set_value_cansleep(par->reset, 0); >>>>>> + gpiod_set_value_cansleep(par->reset, reset_active_low); >>>>>> udelay(4); >>>>>> - gpiod_set_value_cansleep(par->reset, 1); >>>>>> + gpiod_set_value_cansleep(par->reset, !reset_active_low); >>>>> >>>>> I think you and whomever wrote the original code are misinterpretting >>>>> how the gpiod API works. 1 means make the signal active and this case >>>>> active is low. >>>> >>>> I totally agree and I think I understand that correctly. >>>> >>>>> It is strange, but does mean a reset sequence should always be set to a >>>>> 1 and then a 0 and it will work with either polarity in the DT. >>>> >>>> I agree the reset should be done as a 0 -> 1 -> 0 sequence and that should >>>> just work. The problem is it is implemented vice versa and so it works only >>>> if you have GPIO_ACTIVE_HIGH in DT for a signal that is actually active low. >>>> >>>> And what it actually does is that it holds the controller in reset since >>>> the GPIO is successfully acquired (because of GPIOD_OUT_LOW here [1]) and >>>> later on it only releases the reset. >>>> >>>> As a DT author I would like to somehow clearly state that the OLED display >>>> uses active low reset in my DT. >>>> >>>> My first attempt to fix this was to change the reset sequence [2]. >>>> It was applied and then reverted as it is not backward compatible with >>>> deployed DTB files [3]. >>>> >>>> [1] https://elixir.bootlin.com/linux/v4.20-rc3/source/drivers/video/fbdev/ssd1307fb.c#L570 >>>> [2] https://patchwork.kernel.org/patch/10617729/ >>>> [3] https://patchwork.kernel.org/patch/10617731/ >>> >>> Okay, now I understand the background. We've hit this somewhere else too. >>> >>> Rather than have a binding demonstrating what not to do, I'd like to >>> fix this in another way. I also don't want to live with this forever >>> when there's only 1 board affected (in tree at least) and there's only >>> an ABI if someone notices (I'm happy though that the maintainers >>> caught this). There's 2 other options. The 1st is add a fixup to the >>> DT for this platform to ensure that the GPIO is configured active low >>> (the Versatile platform code has an example fixup). With that, apply >>> what was originally applied (or revert the revert). The fixup could be >>> applied only after someone complains their display broke. The 2nd >>> option is just add an of_machine_is_compatible check within this >>> driver. In that case, you wouldn't fix dts file for the platform >>> (unless you also want to manually check reset-gpios). >> >> Hi Rob, >> >> I am still trying to figure out what exactly you meant by the 1st and >> 2nd option. Both concepts are new to me. >> >> Regarding the 1st option, what you meant by "this platform" here: >>> Add a fixup to the DT for this platform >> The only board in tree that uses the OLED (imx28-cfa10036) and its >> dts file? > > Yes, that one. > >> I am also not sure where to look for the example. When you say >> Versatile platform code I tend to look into plat-versatile or >> mach-versatile. I could not find anything I could use as an example >> in there. I think that is not what you meant. > > See versatile_dt_pci_init(). Or look for other callers of of_update_property(). Excellent, I will look at that. >> Regarding the 2nd option, you suggest to use of_machine_is_compatible >> to decide what reset sequence to use? In case of imx28-cfa10036 use >> the old 0 -> 1, in all other cases use a new correct sequence 1 -> 0? >> That also does not seem right. > > Correct. Though if you fix imx28-cfa10036 dts, then you have to handle > that case too. > > Why is it not right? Ugly yes, but it's not wrong. Ugly is what I probably meant. It seems like other users (among drivers) of of_machine_is_compatible are mostly drivers for IP blocks that need slightly different handling on different SoC variants. Thank you very much, Michal ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the reset-active-low property @ 2018-11-26 14:20 ` Vokáč Michal 0 siblings, 0 replies; 15+ messages in thread From: Vokáč Michal @ 2018-11-26 14:20 UTC (permalink / raw) To: Rob Herring Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel, devicetree, Jyri Sarha T24gMjYuMTEuMjAxOCAxNDo0OSwgUm9iIEhlcnJpbmcgd3JvdGU6DQo+IE9uIE1vbiwgTm92IDI2 LCAyMDE4IGF0IDY6MjUgQU0gVm9rw6HEjSBNaWNoYWwgPE1pY2hhbC5Wb2thY0B5c29mdC5jb20+ IHdyb3RlOg0KPj4NCj4+IE9uIDE5LjExLjIwMTggMjM6MzIsIFJvYiBIZXJyaW5nIHdyb3RlOg0K Pj4+IE9uIE1vbiwgTm92IDE5LCAyMDE4IGF0IDk6MTIgQU0gVm9rw6HEjSBNaWNoYWwgPE1pY2hh bC5Wb2thY0B5c29mdC5jb20+IHdyb3RlOg0KPj4+PiBPbiAxMi4xMS4yMDE4IDE3OjU1LCBSb2Ig SGVycmluZyB3cm90ZToNCj4+Pj4+IE9uIEZyaSwgTm92IDAyLCAyMDE4IGF0IDAyOjU2OjM1UE0g KzAwMDAsIFZva8OhxI0gTWljaGFsIHdyb3RlOg0KPj4+Pj4+IFRoZSBTU0QxMzB4IE9MRUQgZGlz cGxheSByZXNldCBzaWduYWwgaXMgYWN0aXZlIGxvdy4gTm93IHRoZSByZXNldA0KPj4+Pj4+IHNl cXVlbmNlIGlzIGltcGxlbWVudGVkIGluIHN1Y2ggYSB3YXkgdGhhdCBEVFMgYXV0aG9ycyBhcmUg Zm9yY2VkIHRvDQo+Pj4+Pj4gZGVmaW5lIHRoZSByZXNldC1ncGlvcyBwcm9wZXJ0eSB3aXRoIEdQ SU9fQUNUSVZFX0hJR0ggdG8gbWFrZSB0aGUgcmVzZXQNCj4+Pj4+PiB3b3JrLg0KPj4+Pj4+DQo+ Pj4+Pj4gQWRkIHRoZSByZXNldC1hY3RpdmUtbG93IHByb3BlcnR5IHNvIHRoZSBzaWduYWwgaXMg aW52ZXJ0ZWQgb25jZSBhZ2Fpbg0KPj4+Pj4+IGFuZCB0aGUgR1BJT19BQ1RJVkVfTE9XIHdvcmsg YXMgZXhwZWN0ZWQuDQo+Pj4+Pj4NCj4+Pj4+PiBTaWduZWQtb2ZmLWJ5OiBNaWNoYWwgVm9rw6HE jSA8bWljaGFsLnZva2FjQHlzb2Z0LmNvbT4NCj4+Pj4+PiAtLS0NCj4+Pj4+PiAgICAgZHJpdmVy cy92aWRlby9mYmRldi9zc2QxMzA3ZmIuYyB8IDYgKysrKy0tDQo+Pj4+Pj4gICAgIDEgZmlsZSBj aGFuZ2VkLCA0IGluc2VydGlvbnMoKyksIDIgZGVsZXRpb25zKC0pDQo+Pj4+Pj4NCj4+Pj4+PiBk aWZmIC0tZ2l0IGEvZHJpdmVycy92aWRlby9mYmRldi9zc2QxMzA3ZmIuYyBiL2RyaXZlcnMvdmlk ZW8vZmJkZXYvc3NkMTMwN2ZiLmMNCj4+Pj4+PiBpbmRleCBlN2FlMTM1Li43OTBmMWM0IDEwMDY0 NA0KPj4+Pj4+IC0tLSBhL2RyaXZlcnMvdmlkZW8vZmJkZXYvc3NkMTMwN2ZiLmMNCj4+Pj4+PiAr KysgYi9kcml2ZXJzL3ZpZGVvL2ZiZGV2L3NzZDEzMDdmYi5jDQo+Pj4+Pj4gQEAgLTYwOCw2ICs2 MDgsNyBAQCBzdGF0aWMgaW50IHNzZDEzMDdmYl9wcm9iZShzdHJ1Y3QgaTJjX2NsaWVudCAqY2xp ZW50LA0KPj4+Pj4+ICAgICAgICBzdHJ1Y3QgZmJfZGVmZXJyZWRfaW8gKnNzZDEzMDdmYl9kZWZp bzsNCj4+Pj4+PiAgICAgICAgdTMyIHZtZW1fc2l6ZTsNCj4+Pj4+PiAgICAgICAgc3RydWN0IHNz ZDEzMDdmYl9wYXIgKnBhcjsNCj4+Pj4+PiArICAgIGJvb2wgcmVzZXRfYWN0aXZlX2xvdzsNCj4+ Pj4+PiAgICAgICAgdTggKnZtZW07DQo+Pj4+Pj4gICAgICAgIGludCByZXQ7DQo+Pj4+Pj4NCj4+ Pj4+PiBAQCAtNjcxLDYgKzY3Miw3IEBAIHN0YXRpYyBpbnQgc3NkMTMwN2ZiX3Byb2JlKHN0cnVj dCBpMmNfY2xpZW50ICpjbGllbnQsDQo+Pj4+Pj4gICAgICAgIHBhci0+Y29tX3NlcSA9IG9mX3By b3BlcnR5X3JlYWRfYm9vbChub2RlLCAic29sb21vbixjb20tc2VxIik7DQo+Pj4+Pj4gICAgICAg IHBhci0+Y29tX2xycmVtYXAgPSBvZl9wcm9wZXJ0eV9yZWFkX2Jvb2wobm9kZSwgInNvbG9tb24s Y29tLWxycmVtYXAiKTsNCj4+Pj4+PiAgICAgICAgcGFyLT5jb21faW52ZGlyID0gb2ZfcHJvcGVy dHlfcmVhZF9ib29sKG5vZGUsICJzb2xvbW9uLGNvbS1pbnZkaXIiKTsNCj4+Pj4+PiArICAgIHJl c2V0X2FjdGl2ZV9sb3cgPSBvZl9wcm9wZXJ0eV9yZWFkX2Jvb2wobm9kZSwgInJlc2V0LWFjdGl2 ZS1sb3ciKTsNCj4+Pj4+Pg0KPj4+Pj4+ICAgICAgICBwYXItPmNvbnRyYXN0ID0gMTI3Ow0KPj4+ Pj4+ICAgICAgICBwYXItPnZjb21oID0gcGFyLT5kZXZpY2VfaW5mby0+ZGVmYXVsdF92Y29taDsN Cj4+Pj4+PiBAQCAtNzI4LDkgKzczMCw5IEBAIHN0YXRpYyBpbnQgc3NkMTMwN2ZiX3Byb2JlKHN0 cnVjdCBpMmNfY2xpZW50ICpjbGllbnQsDQo+Pj4+Pj4NCj4+Pj4+PiAgICAgICAgaWYgKHBhci0+ cmVzZXQpIHsNCj4+Pj4+PiAgICAgICAgICAgICAgICAvKiBSZXNldCB0aGUgc2NyZWVuICovDQo+ Pj4+Pj4gLSAgICAgICAgICAgIGdwaW9kX3NldF92YWx1ZV9jYW5zbGVlcChwYXItPnJlc2V0LCAw KTsNCj4+Pj4+PiArICAgICAgICAgICAgZ3Bpb2Rfc2V0X3ZhbHVlX2NhbnNsZWVwKHBhci0+cmVz ZXQsIHJlc2V0X2FjdGl2ZV9sb3cpOw0KPj4+Pj4+ICAgICAgICAgICAgICAgIHVkZWxheSg0KTsN Cj4+Pj4+PiAtICAgICAgICAgICAgZ3Bpb2Rfc2V0X3ZhbHVlX2NhbnNsZWVwKHBhci0+cmVzZXQs IDEpOw0KPj4+Pj4+ICsgICAgICAgICAgICBncGlvZF9zZXRfdmFsdWVfY2Fuc2xlZXAocGFyLT5y ZXNldCwgIXJlc2V0X2FjdGl2ZV9sb3cpOw0KPj4+Pj4NCj4+Pj4+IEkgdGhpbmsgeW91IGFuZCB3 aG9tZXZlciB3cm90ZSB0aGUgb3JpZ2luYWwgY29kZSBhcmUgbWlzaW50ZXJwcmV0dGluZw0KPj4+ Pj4gaG93IHRoZSBncGlvZCBBUEkgd29ya3MuIDEgbWVhbnMgbWFrZSB0aGUgc2lnbmFsIGFjdGl2 ZSBhbmQgdGhpcyBjYXNlDQo+Pj4+PiBhY3RpdmUgaXMgbG93Lg0KPj4+Pg0KPj4+PiBJIHRvdGFs bHkgYWdyZWUgYW5kIEkgdGhpbmsgSSB1bmRlcnN0YW5kIHRoYXQgY29ycmVjdGx5Lg0KPj4+Pg0K Pj4+Pj4gSXQgaXMgc3RyYW5nZSwgYnV0IGRvZXMgbWVhbiBhIHJlc2V0IHNlcXVlbmNlIHNob3Vs ZCBhbHdheXMgYmUgc2V0IHRvIGENCj4+Pj4+IDEgYW5kIHRoZW4gYSAwIGFuZCBpdCB3aWxsIHdv cmsgd2l0aCBlaXRoZXIgcG9sYXJpdHkgaW4gdGhlIERULg0KPj4+Pg0KPj4+PiBJIGFncmVlIHRo ZSByZXNldCBzaG91bGQgYmUgZG9uZSBhcyBhIDAgLT4gMSAtPiAwIHNlcXVlbmNlIGFuZCB0aGF0 IHNob3VsZA0KPj4+PiBqdXN0IHdvcmsuIFRoZSBwcm9ibGVtIGlzIGl0IGlzIGltcGxlbWVudGVk IHZpY2UgdmVyc2EgYW5kIHNvIGl0IHdvcmtzIG9ubHkNCj4+Pj4gaWYgeW91IGhhdmUgR1BJT19B Q1RJVkVfSElHSCBpbiBEVCBmb3IgYSBzaWduYWwgdGhhdCBpcyBhY3R1YWxseSBhY3RpdmUgbG93 Lg0KPj4+Pg0KPj4+PiBBbmQgd2hhdCBpdCBhY3R1YWxseSBkb2VzIGlzIHRoYXQgaXQgaG9sZHMg dGhlIGNvbnRyb2xsZXIgaW4gcmVzZXQgc2luY2UNCj4+Pj4gdGhlIEdQSU8gaXMgc3VjY2Vzc2Z1 bGx5IGFjcXVpcmVkIChiZWNhdXNlIG9mIEdQSU9EX09VVF9MT1cgaGVyZSBbMV0pIGFuZA0KPj4+ PiBsYXRlciBvbiBpdCBvbmx5IHJlbGVhc2VzIHRoZSByZXNldC4NCj4+Pj4NCj4+Pj4gQXMgYSBE VCBhdXRob3IgSSB3b3VsZCBsaWtlIHRvIHNvbWVob3cgY2xlYXJseSBzdGF0ZSB0aGF0IHRoZSBP TEVEIGRpc3BsYXkNCj4+Pj4gdXNlcyBhY3RpdmUgbG93IHJlc2V0IGluIG15IERULg0KPj4+Pg0K Pj4+PiBNeSBmaXJzdCBhdHRlbXB0IHRvIGZpeCB0aGlzIHdhcyB0byBjaGFuZ2UgdGhlIHJlc2V0 IHNlcXVlbmNlIFsyXS4NCj4+Pj4gSXQgd2FzIGFwcGxpZWQgYW5kIHRoZW4gcmV2ZXJ0ZWQgYXMg aXQgaXMgbm90IGJhY2t3YXJkIGNvbXBhdGlibGUgd2l0aA0KPj4+PiBkZXBsb3llZCBEVEIgZmls ZXMgWzNdLg0KPj4+Pg0KPj4+PiBbMV0gaHR0cHM6Ly9lbGl4aXIuYm9vdGxpbi5jb20vbGludXgv djQuMjAtcmMzL3NvdXJjZS9kcml2ZXJzL3ZpZGVvL2ZiZGV2L3NzZDEzMDdmYi5jI0w1NzANCj4+ Pj4gWzJdIGh0dHBzOi8vcGF0Y2h3b3JrLmtlcm5lbC5vcmcvcGF0Y2gvMTA2MTc3MjkvDQo+Pj4+ IFszXSBodHRwczovL3BhdGNod29yay5rZXJuZWwub3JnL3BhdGNoLzEwNjE3NzMxLw0KPj4+DQo+ Pj4gT2theSwgbm93IEkgdW5kZXJzdGFuZCB0aGUgYmFja2dyb3VuZC4gV2UndmUgaGl0IHRoaXMg c29tZXdoZXJlIGVsc2UgdG9vLg0KPj4+DQo+Pj4gUmF0aGVyIHRoYW4gaGF2ZSBhIGJpbmRpbmcg ZGVtb25zdHJhdGluZyB3aGF0IG5vdCB0byBkbywgSSdkIGxpa2UgdG8NCj4+PiBmaXggdGhpcyBp biBhbm90aGVyIHdheS4gSSBhbHNvIGRvbid0IHdhbnQgdG8gbGl2ZSB3aXRoIHRoaXMgZm9yZXZl cg0KPj4+IHdoZW4gdGhlcmUncyBvbmx5IDEgYm9hcmQgYWZmZWN0ZWQgKGluIHRyZWUgYXQgbGVh c3QpIGFuZCB0aGVyZSdzIG9ubHkNCj4+PiBhbiBBQkkgaWYgc29tZW9uZSBub3RpY2VzIChJJ20g aGFwcHkgdGhvdWdoIHRoYXQgdGhlIG1haW50YWluZXJzDQo+Pj4gY2F1Z2h0IHRoaXMpLiBUaGVy ZSdzIDIgb3RoZXIgb3B0aW9ucy4gVGhlIDFzdCBpcyBhZGQgYSBmaXh1cCB0byB0aGUNCj4+PiBE VCBmb3IgdGhpcyBwbGF0Zm9ybSB0byBlbnN1cmUgdGhhdCB0aGUgR1BJTyBpcyBjb25maWd1cmVk IGFjdGl2ZSBsb3cNCj4+PiAodGhlIFZlcnNhdGlsZSBwbGF0Zm9ybSBjb2RlIGhhcyBhbiBleGFt cGxlIGZpeHVwKS4gV2l0aCB0aGF0LCBhcHBseQ0KPj4+IHdoYXQgd2FzIG9yaWdpbmFsbHkgYXBw bGllZCAob3IgcmV2ZXJ0IHRoZSByZXZlcnQpLiBUaGUgZml4dXAgY291bGQgYmUNCj4+PiBhcHBs aWVkIG9ubHkgYWZ0ZXIgc29tZW9uZSBjb21wbGFpbnMgdGhlaXIgZGlzcGxheSBicm9rZS4gVGhl IDJuZA0KPj4+IG9wdGlvbiBpcyBqdXN0IGFkZCBhbiBvZl9tYWNoaW5lX2lzX2NvbXBhdGlibGUg Y2hlY2sgd2l0aGluIHRoaXMNCj4+PiBkcml2ZXIuIEluIHRoYXQgY2FzZSwgeW91IHdvdWxkbid0 IGZpeCBkdHMgZmlsZSBmb3IgdGhlIHBsYXRmb3JtDQo+Pj4gKHVubGVzcyB5b3UgYWxzbyB3YW50 IHRvIG1hbnVhbGx5IGNoZWNrIHJlc2V0LWdwaW9zKS4NCj4+DQo+PiBIaSBSb2IsDQo+Pg0KPj4g SSBhbSBzdGlsbCB0cnlpbmcgdG8gZmlndXJlIG91dCB3aGF0IGV4YWN0bHkgeW91IG1lYW50IGJ5 IHRoZSAxc3QgYW5kDQo+PiAybmQgb3B0aW9uLiBCb3RoIGNvbmNlcHRzIGFyZSBuZXcgdG8gbWUu DQo+Pg0KPj4gUmVnYXJkaW5nIHRoZSAxc3Qgb3B0aW9uLCB3aGF0IHlvdSBtZWFudCBieSAidGhp cyBwbGF0Zm9ybSIgaGVyZToNCj4+PiBBZGQgYSBmaXh1cCB0byB0aGUgRFQgZm9yIHRoaXMgcGxh dGZvcm0NCj4+IFRoZSBvbmx5IGJvYXJkIGluIHRyZWUgdGhhdCB1c2VzIHRoZSBPTEVEIChpbXgy OC1jZmExMDAzNikgYW5kIGl0cw0KPj4gZHRzIGZpbGU/DQo+IA0KPiBZZXMsIHRoYXQgb25lLg0K PiANCj4+IEkgYW0gYWxzbyBub3Qgc3VyZSB3aGVyZSB0byBsb29rIGZvciB0aGUgZXhhbXBsZS4g V2hlbiB5b3Ugc2F5DQo+PiBWZXJzYXRpbGUgcGxhdGZvcm0gY29kZSBJIHRlbmQgdG8gbG9vayBp bnRvIHBsYXQtdmVyc2F0aWxlIG9yDQo+PiBtYWNoLXZlcnNhdGlsZS4gSSBjb3VsZCBub3QgZmlu ZCBhbnl0aGluZyBJIGNvdWxkIHVzZSBhcyBhbiBleGFtcGxlDQo+PiBpbiB0aGVyZS4gSSB0aGlu ayB0aGF0IGlzIG5vdCB3aGF0IHlvdSBtZWFudC4NCj4gDQo+IFNlZSB2ZXJzYXRpbGVfZHRfcGNp X2luaXQoKS4gT3IgbG9vayBmb3Igb3RoZXIgY2FsbGVycyBvZiBvZl91cGRhdGVfcHJvcGVydHko KS4NCg0KRXhjZWxsZW50LCBJIHdpbGwgbG9vayBhdCB0aGF0Lg0KICANCj4+IFJlZ2FyZGluZyB0 aGUgMm5kIG9wdGlvbiwgeW91IHN1Z2dlc3QgdG8gdXNlIG9mX21hY2hpbmVfaXNfY29tcGF0aWJs ZQ0KPj4gdG8gZGVjaWRlIHdoYXQgcmVzZXQgc2VxdWVuY2UgdG8gdXNlPyBJbiBjYXNlIG9mIGlt eDI4LWNmYTEwMDM2IHVzZQ0KPj4gdGhlIG9sZCAwIC0+IDEsIGluIGFsbCBvdGhlciBjYXNlcyB1 c2UgYSBuZXcgY29ycmVjdCBzZXF1ZW5jZSAxIC0+IDA/DQo+PiBUaGF0IGFsc28gZG9lcyBub3Qg c2VlbSByaWdodC4NCj4gDQo+IENvcnJlY3QuIFRob3VnaCBpZiB5b3UgZml4IGlteDI4LWNmYTEw MDM2IGR0cywgdGhlbiB5b3UgaGF2ZSB0byBoYW5kbGUNCj4gdGhhdCBjYXNlIHRvby4NCj4gDQo+ IFdoeSBpcyBpdCBub3QgcmlnaHQ/IFVnbHkgeWVzLCBidXQgaXQncyBub3Qgd3JvbmcuDQoNClVn bHkgaXMgd2hhdCBJIHByb2JhYmx5IG1lYW50LiBJdCBzZWVtcyBsaWtlIG90aGVyIHVzZXJzIChh bW9uZyBkcml2ZXJzKQ0Kb2Ygb2ZfbWFjaGluZV9pc19jb21wYXRpYmxlIGFyZSBtb3N0bHkgZHJp dmVycyBmb3IgSVAgYmxvY2tzIHRoYXQgbmVlZA0Kc2xpZ2h0bHkgZGlmZmVyZW50IGhhbmRsaW5n IG9uIGRpZmZlcmVudCBTb0MgdmFyaWFudHMuDQoNClRoYW5rIHlvdSB2ZXJ5IG11Y2gsDQpNaWNo YWwNCg= ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-11-26 14:20 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-02 14:56 [PATCH fbdev-for-next 1/2] dt-bindings: display: ssd1307fb: Add reset-active-low property Vokáč Michal 2018-11-02 14:56 ` Vokáč Michal 2018-11-02 14:56 ` [PATCH fbdev-for-next 2/2] video: ssd1307fb: Add support for the " Vokáč Michal 2018-11-02 14:56 ` Vokáč Michal 2018-11-12 16:55 ` Rob Herring 2018-11-19 15:12 ` Vokáč Michal 2018-11-19 15:12 ` Vokáč Michal 2018-11-19 22:32 ` Rob Herring 2018-11-19 22:32 ` Rob Herring 2018-11-26 12:25 ` Vokáč Michal 2018-11-26 12:25 ` Vokáč Michal 2018-11-26 13:49 ` Rob Herring 2018-11-26 13:49 ` Rob Herring 2018-11-26 14:20 ` Vokáč Michal 2018-11-26 14:20 ` Vokáč Michal
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.