[PATCH 0/6] fbdev/ssd1307fb: Some changes and improvement

[PATCH 0/6] fbdev/ssd1307fb: Some changes and improvement

[Jyri Sarha]

We needed these changes for couple of our evm. Everything else is quite
straight forward, but removing the reset-active-low dts property is a
questionable at least in theory. However, in practice if anyone was
using the property in their device-tree blobs, they have never been
able to get desired effect with mainline kernel.

Jyri Sarha (3):
  fbdev: ssd1307fb: Start to use gpiod API for reset gpio
  fbdev: ssd1307fb: Remove reset-active-low from the DT binding document
  fbdev: ssd1307fb: Make reset gpio devicetree property optional

Tomi Valkeinen (3):
  fbdev/ssd1307fb: add support to enable VBAT
  fbdev/ssd1307fb: clear screen in probe
  fbdev/ssd1307fb: add support to enable VBAT

 .../devicetree/bindings/display/ssd1307fb.txt      |  5 +-
 drivers/video/fbdev/ssd1307fb.c                    | 55 ++++++++++++++--------
 2 files changed, 38 insertions(+), 22 deletions(-)

-- 
1.9.1

[PATCH 1/6] fbdev: ssd1307fb: Start to use gpiod API for reset gpio

[Jyri Sarha]

Start to use gpiod API for reset gpio.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/video/fbdev/ssd1307fb.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 2925d5c..8ffaaee 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -73,7 +73,7 @@ struct ssd1307fb_par {
 	u32 prechargep2;
 	struct pwm_device *pwm;
 	u32 pwm_period;
-	int reset;
+	struct gpio_desc *reset;
 	u32 seg_remap;
 	u32 vcomh;
 	u32 width;
@@ -561,10 +561,11 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	par->device_info = of_device_get_match_data(&client->dev);
 
-	par->reset = of_get_named_gpio(client->dev.of_node,
-					 "reset-gpios", 0);
-	if (!gpio_is_valid(par->reset)) {
-		ret = -EINVAL;
+	par->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(par->reset)) {
+		dev_err(&client->dev, "failed to get reset gpio: %ld\n",
+			PTR_ERR(par->reset));
+		ret = PTR_ERR(par->reset);
 		goto fb_alloc_error;
 	}
 
@@ -642,22 +643,12 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	fb_deferred_io_init(info);
 
-	ret = devm_gpio_request_one(&client->dev, par->reset,
-				    GPIOF_OUT_INIT_HIGH,
-				    "oled-reset");
-	if (ret) {
-		dev_err(&client->dev,
-			"failed to request gpio %d: %d\n",
-			par->reset, ret);
-		goto reset_oled_error;
-	}
-
 	i2c_set_clientdata(client, info);
 
 	/* Reset the screen */
-	gpio_set_value(par->reset, 0);
+	gpiod_set_value(par->reset, 0);
 	udelay(4);
-	gpio_set_value(par->reset, 1);
+	gpiod_set_value(par->reset, 1);
 	udelay(4);
 
 	ret = ssd1307fb_init(par);
-- 
1.9.1

[PATCH 2/6] fbdev: ssd1307fb: Remove reset-active-low from the DT binding document

[Jyri Sarha]

Remove reset-active-low from the devicetree binding document. The actual
implementation has never been there in the driver code and there is no
reason to add it because the gpiod API supports gpio flags, including
GPIO_ACTIVE_LOW, directly trough its own devicetree binding.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 Documentation/devicetree/bindings/display/ssd1307fb.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt
index eb31ed4..4aee67f 100644
--- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
@@ -8,14 +8,14 @@ Required properties:
          0x3c or 0x3d
   - pwm: Should contain the pwm to use according to the OF device tree PWM
          specification [0]. Only required for the ssd1307.
-  - reset-gpios: Should contain the GPIO used to reset the OLED display
+  - reset-gpios: Should contain the GPIO used to reset the OLED display. See
+                 Documentation/devicetree/bindings/gpio/gpio.txt for details.
   - solomon,height: Height in pixel of the screen driven by the controller
   - solomon,width: Width in pixel of the screen driven by the controller
   - solomon,page-offset: Offset of pages (band of 8 pixels) that the screen is
     mapped to.
 
 Optional properties:
-  - reset-active-low: Is the reset gpio is active on physical low?
   - solomon,segment-no-remap: Display needs normal (non-inverted) data column
                               to segment mapping
   - solomon,com-seq: Display uses sequential COM pin configuration
-- 
1.9.1

[PATCH 3/6] fbdev: ssd1307fb: Make reset gpio devicetree property optional

[Jyri Sarha]

Make reset gpio devicetree property optional. Depending on the board
designing there may not be a dedicated gpio for resetting the
display. Without a proper reset there may be some junk in the display
memory at probe time, so in such a case the display memory is cleared
before turning it on. The devicetree binding document is also updated.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 .../devicetree/bindings/display/ssd1307fb.txt         |  4 ++--
 drivers/video/fbdev/ssd1307fb.c                       | 19 +++++++++++++------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt
index 4aee67f..6617df6 100644
--- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
@@ -8,14 +8,14 @@ Required properties:
          0x3c or 0x3d
   - pwm: Should contain the pwm to use according to the OF device tree PWM
          specification [0]. Only required for the ssd1307.
-  - reset-gpios: Should contain the GPIO used to reset the OLED display. See
-                 Documentation/devicetree/bindings/gpio/gpio.txt for details.
   - solomon,height: Height in pixel of the screen driven by the controller
   - solomon,width: Width in pixel of the screen driven by the controller
   - solomon,page-offset: Offset of pages (band of 8 pixels) that the screen is
     mapped to.
 
 Optional properties:
+  - reset-gpios: The GPIO used to reset the OLED display, if available. See
+                 Documentation/devicetree/bindings/gpio/gpio.txt for details.
   - solomon,segment-no-remap: Display needs normal (non-inverted) data column
                               to segment mapping
   - solomon,com-seq: Display uses sequential COM pin configuration
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 8ffaaee..89372af 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -439,6 +439,10 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 	if (ret < 0)
 		return ret;
 
+	/* Clear the screen if we could not give reset at probe time */
+	if (!par->reset)
+		ssd1307fb_update_display(par);
+
 	/* Turn on the display */
 	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
 	if (ret < 0)
@@ -561,7 +565,8 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	par->device_info = of_device_get_match_data(&client->dev);
 
-	par->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_LOW);
+	par->reset = devm_gpiod_get_optional(&client->dev, "reset",
+					     GPIOD_OUT_LOW);
 	if (IS_ERR(par->reset)) {
 		dev_err(&client->dev, "failed to get reset gpio: %ld\n",
 			PTR_ERR(par->reset));
@@ -645,11 +650,13 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, info);
 
-	/* Reset the screen */
-	gpiod_set_value(par->reset, 0);
-	udelay(4);
-	gpiod_set_value(par->reset, 1);
-	udelay(4);
+	if (par->reset) {
+		/* Reset the screen */
+		gpiod_set_value(par->reset, 0);
+		udelay(4);
+		gpiod_set_value(par->reset, 1);
+		udelay(4);
+	}
 
 	ret = ssd1307fb_init(par);
 	if (ret)
-- 
1.9.1

[PATCH 4/6] fbdev/ssd1307fb: add support to enable VBAT

[Jyri Sarha]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

SSD1306 needs VBAT when it is wired in charge pump configuration. This
patch adds support to the driver to enable VBAT regulator at init time.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/video/fbdev/ssd1307fb.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 89372af..616a6a3 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -16,6 +16,7 @@
 #include <linux/of_gpio.h>
 #include <linux/pwm.h>
 #include <linux/uaccess.h>
+#include <linux/regulator/consumer.h>
 
 #define SSD1307FB_DATA			0x40
 #define SSD1307FB_COMMAND		0x80
@@ -74,6 +75,7 @@ struct ssd1307fb_par {
 	struct pwm_device *pwm;
 	u32 pwm_period;
 	struct gpio_desc *reset;
+	struct regulator *vbat_reg;
 	u32 seg_remap;
 	u32 vcomh;
 	u32 width;
@@ -574,6 +576,14 @@ static int ssd1307fb_probe(struct i2c_client *client,
 		goto fb_alloc_error;
 	}
 
+	par->vbat_reg = devm_regulator_get_optional(&client->dev, "vbat");
+	if (IS_ERR(par->vbat_reg)) {
+		dev_err(&client->dev, "failed to get VBAT regulator: %ld\n",
+			PTR_ERR(par->vbat_reg));
+		ret = PTR_ERR(par->vbat_reg);
+		goto fb_alloc_error;
+	}
+
 	if (of_property_read_u32(node, "solomon,width", &par->width))
 		par->width = 96;
 
@@ -658,9 +668,15 @@ static int ssd1307fb_probe(struct i2c_client *client,
 		udelay(4);
 	}
 
+	ret = regulator_enable(par->vbat_reg);
+	if (ret) {
+		dev_err(&client->dev, "failed to enable VBAT: %d\n", ret);
+		goto reset_oled_error;
+	}
+
 	ret = ssd1307fb_init(par);
 	if (ret)
-		goto reset_oled_error;
+		goto regulator_enable_error;
 
 	ret = register_framebuffer(info);
 	if (ret) {
@@ -693,6 +709,8 @@ static int ssd1307fb_probe(struct i2c_client *client,
 		pwm_disable(par->pwm);
 		pwm_put(par->pwm);
 	};
+regulator_enable_error:
+	regulator_disable(par->vbat_reg);
 reset_oled_error:
 	fb_deferred_io_cleanup(info);
 fb_alloc_error:
-- 
1.9.1

[PATCH 5/6] fbdev/ssd1307fb: clear screen in probe

[Jyri Sarha]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

SSD1306 does not clear the panel's framebuffer automatically, even if a
HW reset happens, so we need to do that at probe time before enabling
the panel.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/video/fbdev/ssd1307fb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 616a6a3..5c87ae4 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -441,9 +441,8 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 	if (ret < 0)
 		return ret;
 
-	/* Clear the screen if we could not give reset at probe time */
-	if (!par->reset)
-		ssd1307fb_update_display(par);
+	/* Clear the screen */
+	ssd1307fb_update_display(par);
 
 	/* Turn on the display */
 	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
-- 
1.9.1

[PATCH 6/6] fbdev/ssd1307fb: add support to enable VBAT

[Jyri Sarha]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

SSD1306 needs VBAT when it is wired in charge pump configuration. This
patch adds support to the driver to enable VBAT regulator at init time.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 Documentation/devicetree/bindings/display/ssd1307fb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt
index 6617df6..209d931 100644
--- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
@@ -16,6 +16,7 @@ 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.
+  - vbat-supply: The supply for VBAT
   - solomon,segment-no-remap: Display needs normal (non-inverted) data column
                               to segment mapping
   - solomon,com-seq: Display uses sequential COM pin configuration
-- 
1.9.1

Re: [PATCH 0/6] fbdev/ssd1307fb: Some changes and improvement

[Jyri Sarha]

I did not notice the latest update on MAINTAINERS when sending this
series. Added Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> to loop.


On 01/12/17 13:16, Jyri Sarha wrote:
> We needed these changes for couple of our evm. Everything else is quite
> straight forward, but removing the reset-active-low dts property is a
> questionable at least in theory. However, in practice if anyone was
> using the property in their device-tree blobs, they have never been
> able to get desired effect with mainline kernel.
> 
> Jyri Sarha (3):
>   fbdev: ssd1307fb: Start to use gpiod API for reset gpio
>   fbdev: ssd1307fb: Remove reset-active-low from the DT binding document
>   fbdev: ssd1307fb: Make reset gpio devicetree property optional
> 
> Tomi Valkeinen (3):
>   fbdev/ssd1307fb: add support to enable VBAT
>   fbdev/ssd1307fb: clear screen in probe
>   fbdev/ssd1307fb: add support to enable VBAT
> 
>  .../devicetree/bindings/display/ssd1307fb.txt      |  5 +-
>  drivers/video/fbdev/ssd1307fb.c                    | 55 ++++++++++++++--------
>  2 files changed, 38 insertions(+), 22 deletions(-)
> 

Re: [PATCH 6/6] fbdev/ssd1307fb: add support to enable VBAT

[Tomi Valkeinen]

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

On 12/01/17 13:16, Jyri Sarha wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> SSD1306 needs VBAT when it is wired in charge pump configuration. This
> patch adds support to the driver to enable VBAT regulator at init time.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Reviewed-by: Roger Quadros <rogerq@ti.com>
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  Documentation/devicetree/bindings/display/ssd1307fb.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt
> index 6617df6..209d931 100644
> --- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
> +++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
> @@ -16,6 +16,7 @@ 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.
> +  - vbat-supply: The supply for VBAT
>    - solomon,segment-no-remap: Display needs normal (non-inverted) data column
>                                to segment mapping
>    - solomon,com-seq: Display uses sequential COM pin configuration
> 

It's rather confusing to have two patches with identical subjects and
descs (this and 4). And there's even an unrelated patch in between this
and the driver change...

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 6/6] fbdev/ssd1307fb: add support to enable VBAT

[Jyri Sarha]

On 01/12/17 14:05, Tomi Valkeinen wrote:
> On 12/01/17 13:16, Jyri Sarha wrote:
>> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>
>> SSD1306 needs VBAT when it is wired in charge pump configuration. This
>> patch adds support to the driver to enable VBAT regulator at init time.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Reviewed-by: Roger Quadros <rogerq@ti.com>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>  Documentation/devicetree/bindings/display/ssd1307fb.txt | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt
>> index 6617df6..209d931 100644
>> --- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
>> +++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
>> @@ -16,6 +16,7 @@ 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.
>> +  - vbat-supply: The supply for VBAT
>>    - solomon,segment-no-remap: Display needs normal (non-inverted) data column
>>                                to segment mapping
>>    - solomon,com-seq: Display uses sequential COM pin configuration
>>
> 
> It's rather confusing to have two patches with identical subjects and
> descs (this and 4). And there's even an unrelated patch in between this
> and the driver change...
> 

Argh. I wonder how did this even happen. Something weird must have been
going on... there is only one such patch in branch I cherry-picked these
patches from. These last patches did not give me any errors and compiled
fine, so did not look too carefully at the results. Anyway, I'll squash
this into the earlier patch with the same subject in version 2 series.

Best regards,
Jyri