* [PATCH 1/4] dt-bindings: display: ssd1307fb: Remove reset-active-low from examples
2018-12-04 15:03 [PATCH 0/4] Fix ssd1307fb OLED driver reset Vokáč Michal
@ 2018-12-04 15:03 ` Vokáč Michal
2018-12-04 15:03 ` [PATCH 2/4] video: ssd1307fb: Do not hard code active-low reset sequence Vokáč Michal
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Vokáč Michal @ 2018-12-04 15:03 UTC (permalink / raw)
To: Rob Herring, Bartlomiej Zolnierkiewicz
Cc: Shawn Guo, Fabio Estevam, Alexandre Belloni, Maxime Ripard,
linux-fbdev, linux-kernel, devicetree, Vokáč Michal
The reset-active-low property has been removed brom the binding a while
ago. So remove it from the examples as well.
Fixes: 519b4db ("fbdev: ssd1307fb: Remove reset-active-low from the DT binding document")
Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
Documentation/devicetree/bindings/display/ssd1307fb.txt | 2 --
1 file changed, 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt
index 209d931..b67f8ca 100644
--- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
@@ -36,7 +36,6 @@ ssd1307: oled@3c {
reg = <0x3c>;
pwms = <&pwm 4 3000>;
reset-gpios = <&gpio2 7>;
- reset-active-low;
};
ssd1306: oled@3c {
@@ -44,7 +43,6 @@ ssd1306: oled@3c {
reg = <0x3c>;
pwms = <&pwm 4 3000>;
reset-gpios = <&gpio2 7>;
- reset-active-low;
solomon,com-lrremap;
solomon,com-invdir;
solomon,com-offset = <32>;
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] video: ssd1307fb: Do not hard code active-low reset sequence
2018-12-04 15:03 [PATCH 0/4] Fix ssd1307fb OLED driver reset Vokáč Michal
2018-12-04 15:03 ` [PATCH 1/4] dt-bindings: display: ssd1307fb: Remove reset-active-low from examples Vokáč Michal
@ 2018-12-04 15:03 ` Vokáč Michal
2018-12-04 15:03 ` [PATCH 3/4] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity Vokáč Michal
2018-12-04 15:03 ` [PATCH 4/4] ARM: mxs: cfa10036: Fixup OLED display reset polarity Vokáč Michal
3 siblings, 0 replies; 7+ messages in thread
From: Vokáč Michal @ 2018-12-04 15:03 UTC (permalink / raw)
To: Rob Herring, Bartlomiej Zolnierkiewicz
Cc: Shawn Guo, Fabio Estevam, Alexandre Belloni, Maxime Ripard,
linux-fbdev, linux-kernel, devicetree, Vokáč Michal
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>
---
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 4061a20..3b361bc 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, 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] 7+ messages in thread
* [PATCH 3/4] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity
2018-12-04 15:03 [PATCH 0/4] Fix ssd1307fb OLED driver reset Vokáč Michal
2018-12-04 15:03 ` [PATCH 1/4] dt-bindings: display: ssd1307fb: Remove reset-active-low from examples Vokáč Michal
2018-12-04 15:03 ` [PATCH 2/4] video: ssd1307fb: Do not hard code active-low reset sequence Vokáč Michal
@ 2018-12-04 15:03 ` Vokáč Michal
2018-12-04 15:03 ` [PATCH 4/4] ARM: mxs: cfa10036: Fixup OLED display reset polarity Vokáč Michal
3 siblings, 0 replies; 7+ messages in thread
From: Vokáč Michal @ 2018-12-04 15:03 UTC (permalink / raw)
To: Rob Herring, Bartlomiej Zolnierkiewicz
Cc: Shawn Guo, Fabio Estevam, Alexandre Belloni, Maxime Ripard,
linux-fbdev, linux-kernel, devicetree, Vokáč Michal
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>
---
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 8337ca2..d6eca31 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] 7+ messages in thread
* [PATCH 4/4] ARM: mxs: cfa10036: Fixup OLED display reset polarity
2018-12-04 15:03 [PATCH 0/4] Fix ssd1307fb OLED driver reset Vokáč Michal
` (2 preceding siblings ...)
2018-12-04 15:03 ` [PATCH 3/4] ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity Vokáč Michal
@ 2018-12-04 15:03 ` Vokáč Michal
2018-12-19 17:29 ` Rob Herring
3 siblings, 1 reply; 7+ messages in thread
From: Vokáč Michal @ 2018-12-04 15:03 UTC (permalink / raw)
To: Rob Herring, Bartlomiej Zolnierkiewicz
Cc: Shawn Guo, Fabio Estevam, Alexandre Belloni, Maxime Ripard,
linux-fbdev, linux-kernel, devicetree, Vokáč Michal
There was a bug in reset signal generation in ssd1307fb OLED driver.
The display needs an active-low reset signal but the driver produced
the correct sequence only if the GPIO used for reset was specified as
GPIO_ACTIVE_HIGH.
Now as the OLED driver is fixed it is also necessarry to implement
a fixup for all current users of the old DT ABI. There is only one
in-tree user and that is the Crystalfontz CFA-10036 board. In case
this board is booting and GPIO_ACTIVE_HIGH is used for reset we
override it to GPIO_ACTIVE_LOW.
Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
arch/arm/mach-mxs/mach-mxs.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
index 1c6062d..23c260c 100644
--- a/arch/arm/mach-mxs/mach-mxs.c
+++ b/arch/arm/mach-mxs/mach-mxs.c
@@ -21,6 +21,7 @@
#include <linux/reboot.h>
#include <linux/micrel_phy.h>
#include <linux/of_address.h>
+#include <linux/of_gpio.h>
#include <linux/of_platform.h>
#include <linux/phy.h>
#include <linux/pinctrl/consumer.h>
@@ -268,9 +269,53 @@ static void __init apx4devkit_init(void)
apx4devkit_phy_fixup);
}
+#define OLED_RESET_GPIO_LEN 3
+#define OLED_RESET_GPIO_SIZE (OLED_RESET_GPIO_LEN * sizeof(u32))
+
+static void __init crystalfontz_oled_reset_fixup(void)
+{
+ struct property *newgpio;
+ struct device_node *np;
+ u32 *gpiospec;
+ int i, ret;
+
+ np = of_find_compatible_node(NULL, NULL, "solomon,ssd1306fb-i2c");
+ if (!np)
+ return;
+
+ newgpio = kzalloc(sizeof(*newgpio) + OLED_RESET_GPIO_SIZE, GFP_KERNEL);
+ if (!newgpio)
+ return;
+
+ newgpio->value = newgpio + 1;
+ newgpio->length = OLED_RESET_GPIO_SIZE;
+ newgpio->name = kstrdup("reset-gpios", GFP_KERNEL);
+ if (!newgpio->name) {
+ kfree(newgpio);
+ return;
+ }
+
+ gpiospec = newgpio->value;
+ for (i = 0; i < OLED_RESET_GPIO_LEN; i++) {
+ ret = of_property_read_u32_index(np, "reset-gpios", i,
+ &gpiospec[i]);
+ if (ret) {
+ kfree(newgpio);
+ return;
+ }
+ }
+
+ if (!(gpiospec[2] & OF_GPIO_ACTIVE_LOW)) {
+ gpiospec[2] |= OF_GPIO_ACTIVE_LOW;
+ cpu_to_be32_array(gpiospec, gpiospec, OLED_RESET_GPIO_LEN);
+ of_update_property(np, newgpio);
+ }
+}
+
static void __init crystalfontz_init(void)
{
update_fec_mac_prop(OUI_CRYSTALFONTZ);
+ crystalfontz_oled_reset_fixup();
}
static void __init duckbill_init(void)
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4] ARM: mxs: cfa10036: Fixup OLED display reset polarity
2018-12-04 15:03 ` [PATCH 4/4] ARM: mxs: cfa10036: Fixup OLED display reset polarity Vokáč Michal
@ 2018-12-19 17:29 ` Rob Herring
2018-12-20 11:39 ` Vokáč Michal
0 siblings, 1 reply; 7+ messages in thread
From: Rob Herring @ 2018-12-19 17:29 UTC (permalink / raw)
To: Vokáč Michal
Cc: Bartlomiej Zolnierkiewicz, Shawn Guo, Fabio Estevam,
Alexandre Belloni, Maxime Ripard, linux-fbdev, linux-kernel,
devicetree
On Tue, Dec 04, 2018 at 03:03:40PM +0000, Vokáč Michal wrote:
> There was a bug in reset signal generation in ssd1307fb OLED driver.
> The display needs an active-low reset signal but the driver produced
> the correct sequence only if the GPIO used for reset was specified as
> GPIO_ACTIVE_HIGH.
>
> Now as the OLED driver is fixed it is also necessarry to implement
> a fixup for all current users of the old DT ABI. There is only one
> in-tree user and that is the Crystalfontz CFA-10036 board. In case
> this board is booting and GPIO_ACTIVE_HIGH is used for reset we
> override it to GPIO_ACTIVE_LOW.
>
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> ---
> arch/arm/mach-mxs/mach-mxs.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
> index 1c6062d..23c260c 100644
> --- a/arch/arm/mach-mxs/mach-mxs.c
> +++ b/arch/arm/mach-mxs/mach-mxs.c
> @@ -21,6 +21,7 @@
> #include <linux/reboot.h>
> #include <linux/micrel_phy.h>
> #include <linux/of_address.h>
> +#include <linux/of_gpio.h>
> #include <linux/of_platform.h>
> #include <linux/phy.h>
> #include <linux/pinctrl/consumer.h>
> @@ -268,9 +269,53 @@ static void __init apx4devkit_init(void)
> apx4devkit_phy_fixup);
> }
>
> +#define OLED_RESET_GPIO_LEN 3
> +#define OLED_RESET_GPIO_SIZE (OLED_RESET_GPIO_LEN * sizeof(u32))
> +
> +static void __init crystalfontz_oled_reset_fixup(void)
> +{
> + struct property *newgpio;
> + struct device_node *np;
> + u32 *gpiospec;
> + int i, ret;
> +
> + np = of_find_compatible_node(NULL, NULL, "solomon,ssd1306fb-i2c");
> + if (!np)
> + return;
> +
> + newgpio = kzalloc(sizeof(*newgpio) + OLED_RESET_GPIO_SIZE, GFP_KERNEL);
> + if (!newgpio)
> + return;
> +
> + newgpio->value = newgpio + 1;
> + newgpio->length = OLED_RESET_GPIO_SIZE;
> + newgpio->name = kstrdup("reset-gpios", GFP_KERNEL);
> + if (!newgpio->name) {
> + kfree(newgpio);
> + return;
> + }
> +
> + gpiospec = newgpio->value;
> + for (i = 0; i < OLED_RESET_GPIO_LEN; i++) {
> + ret = of_property_read_u32_index(np, "reset-gpios", i,
> + &gpiospec[i]);
Don't we have a helper to read the whole array?
Otherwise, for the series:
Reviewed-by: Rob Herring <robh@kernel.org>
> + if (ret) {
> + kfree(newgpio);
> + return;
> + }
> + }
> +
> + if (!(gpiospec[2] & OF_GPIO_ACTIVE_LOW)) {
> + gpiospec[2] |= OF_GPIO_ACTIVE_LOW;
> + cpu_to_be32_array(gpiospec, gpiospec, OLED_RESET_GPIO_LEN);
> + of_update_property(np, newgpio);
> + }
> +}
> +
> static void __init crystalfontz_init(void)
> {
> update_fec_mac_prop(OUI_CRYSTALFONTZ);
> + crystalfontz_oled_reset_fixup();
> }
>
> static void __init duckbill_init(void)
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4] ARM: mxs: cfa10036: Fixup OLED display reset polarity
2018-12-19 17:29 ` Rob Herring
@ 2018-12-20 11:39 ` Vokáč Michal
0 siblings, 0 replies; 7+ messages in thread
From: Vokáč Michal @ 2018-12-20 11:39 UTC (permalink / raw)
To: Rob Herring
Cc: Bartlomiej Zolnierkiewicz, Shawn Guo, Fabio Estevam,
Alexandre Belloni, Maxime Ripard, linux-fbdev, linux-kernel,
devicetree
On 19.12.2018 18:29, Rob Herring wrote:
> On Tue, Dec 04, 2018 at 03:03:40PM +0000, Vokáč Michal wrote:
>> There was a bug in reset signal generation in ssd1307fb OLED driver.
>> The display needs an active-low reset signal but the driver produced
>> the correct sequence only if the GPIO used for reset was specified as
>> GPIO_ACTIVE_HIGH.
>>
>> Now as the OLED driver is fixed it is also necessarry to implement
>> a fixup for all current users of the old DT ABI. There is only one
>> in-tree user and that is the Crystalfontz CFA-10036 board. In case
>> this board is booting and GPIO_ACTIVE_HIGH is used for reset we
>> override it to GPIO_ACTIVE_LOW.
>>
>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>> ---
>> arch/arm/mach-mxs/mach-mxs.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 45 insertions(+)
>>
>> diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
>> index 1c6062d..23c260c 100644
>> --- a/arch/arm/mach-mxs/mach-mxs.c
>> +++ b/arch/arm/mach-mxs/mach-mxs.c
>> @@ -21,6 +21,7 @@
>> #include <linux/reboot.h>
>> #include <linux/micrel_phy.h>
>> #include <linux/of_address.h>
>> +#include <linux/of_gpio.h>
>> #include <linux/of_platform.h>
>> #include <linux/phy.h>
>> #include <linux/pinctrl/consumer.h>
>> @@ -268,9 +269,53 @@ static void __init apx4devkit_init(void)
>> apx4devkit_phy_fixup);
>> }
>>
>> +#define OLED_RESET_GPIO_LEN 3
>> +#define OLED_RESET_GPIO_SIZE (OLED_RESET_GPIO_LEN * sizeof(u32))
>> +
>> +static void __init crystalfontz_oled_reset_fixup(void)
>> +{
>> + struct property *newgpio;
>> + struct device_node *np;
>> + u32 *gpiospec;
>> + int i, ret;
>> +
>> + np = of_find_compatible_node(NULL, NULL, "solomon,ssd1306fb-i2c");
>> + if (!np)
>> + return;
>> +
>> + newgpio = kzalloc(sizeof(*newgpio) + OLED_RESET_GPIO_SIZE, GFP_KERNEL);
>> + if (!newgpio)
>> + return;
>> +
>> + newgpio->value = newgpio + 1;
>> + newgpio->length = OLED_RESET_GPIO_SIZE;
>> + newgpio->name = kstrdup("reset-gpios", GFP_KERNEL);
>> + if (!newgpio->name) {
>> + kfree(newgpio);
>> + return;
>> + }
>> +
>> + gpiospec = newgpio->value;
>> + for (i = 0; i < OLED_RESET_GPIO_LEN; i++) {
>> + ret = of_property_read_u32_index(np, "reset-gpios", i,
>> + &gpiospec[i]);
>
> Don't we have a helper to read the whole array?
Hi Rob,
yep, I will fix this.
Thank you for your review and for pointing me to the right solution.
Michal
>
> Otherwise, for the series:
>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
>> + if (ret) {
>> + kfree(newgpio);
>> + return;
>> + }
>> + }
>> +
>> + if (!(gpiospec[2] & OF_GPIO_ACTIVE_LOW)) {
>> + gpiospec[2] |= OF_GPIO_ACTIVE_LOW;
>> + cpu_to_be32_array(gpiospec, gpiospec, OLED_RESET_GPIO_LEN);
>> + of_update_property(np, newgpio);
>> + }
>> +}
>> +
>> static void __init crystalfontz_init(void)
>> {
>> update_fec_mac_prop(OUI_CRYSTALFONTZ);
>> + crystalfontz_oled_reset_fixup();
>> }
>>
>> static void __init duckbill_init(void)
>> --
>> 2.1.4
>>
^ permalink raw reply [flat|nested] 7+ messages in thread