* [PATCH] fbdev: ssd1307fb: Fix chargepump setting
2015-11-12 14:07 [PATCH] fbdev: ssd1307fb: Fix chargepump setting Julian Scheel
@ 2015-11-15 13:36 ` Julian Scheel
2016-01-18 20:20 ` Julian Scheel
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Julian Scheel @ 2015-11-15 13:36 UTC (permalink / raw)
To: linux-fbdev
The charge pump setting must have bit D4 set all time according to the SSD1306
App Note. Instead of doing an logical and off shifted setting bit with 0x14 it
must be an logical or with 0x10 to ensure D4 is set.
Signed-off-by: Julian Scheel <julian@jusst.de>
---
drivers/video/fbdev/ssd1307fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 1611215..5965a9b 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -389,7 +389,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
return ret;
ret = ssd1307fb_write_cmd(par->client,
- (par->device_info->need_chargepump & 0x1 << 2) & 0x14);
+ 0x10 | ((par->device_info->need_chargepump & 0x01) << 2));
if (ret < 0)
return ret;
--
2.6.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] fbdev: ssd1307fb: Fix chargepump setting
2015-11-12 14:07 [PATCH] fbdev: ssd1307fb: Fix chargepump setting Julian Scheel
2015-11-15 13:36 ` Julian Scheel
@ 2016-01-18 20:20 ` Julian Scheel
2016-01-29 11:55 ` Tomi Valkeinen
2016-01-29 12:00 ` Julian Scheel
3 siblings, 0 replies; 5+ messages in thread
From: Julian Scheel @ 2016-01-18 20:20 UTC (permalink / raw)
To: linux-fbdev
On 15.11.15 14:36, Julian Scheel wrote:
> The charge pump setting must have bit D4 set all time according to the SSD1306
> App Note. Instead of doing an logical and off shifted setting bit with 0x14 it
> must be an logical or with 0x10 to ensure D4 is set.
>
> Signed-off-by: Julian Scheel <julian@jusst.de>
> ---
> drivers/video/fbdev/ssd1307fb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 1611215..5965a9b 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -389,7 +389,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
> return ret;
>
> ret = ssd1307fb_write_cmd(par->client,
> - (par->device_info->need_chargepump & 0x1 << 2) & 0x14);
> + 0x10 | ((par->device_info->need_chargepump & 0x01) << 2));
> if (ret < 0)
> return ret;
>
>
+ Adding Jean-Christophe and Tomi in CC as maintainers and Thomas as the
one who did last major changes to the driver.
Any objections on this patch?
Regards,
Julian
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fbdev: ssd1307fb: Fix chargepump setting
2015-11-12 14:07 [PATCH] fbdev: ssd1307fb: Fix chargepump setting Julian Scheel
2015-11-15 13:36 ` Julian Scheel
2016-01-18 20:20 ` Julian Scheel
@ 2016-01-29 11:55 ` Tomi Valkeinen
2016-01-29 12:00 ` Julian Scheel
3 siblings, 0 replies; 5+ messages in thread
From: Tomi Valkeinen @ 2016-01-29 11:55 UTC (permalink / raw)
To: linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 1343 bytes --]
On 18/01/16 22:20, Julian Scheel wrote:
> On 15.11.15 14:36, Julian Scheel wrote:
>> The charge pump setting must have bit D4 set all time according to the
>> SSD1306
>> App Note. Instead of doing an logical and off shifted setting bit with
>> 0x14 it
>> must be an logical or with 0x10 to ensure D4 is set.
The code change looks ok, but I can't quite decipher the commit
description. "an logical and off shifted setting bit with 0x14"?
>> Signed-off-by: Julian Scheel <julian@jusst.de>
>> ---
>> drivers/video/fbdev/ssd1307fb.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/ssd1307fb.c
>> b/drivers/video/fbdev/ssd1307fb.c
>> index 1611215..5965a9b 100644
>> --- a/drivers/video/fbdev/ssd1307fb.c
>> +++ b/drivers/video/fbdev/ssd1307fb.c
>> @@ -389,7 +389,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
>> return ret;
>>
>> ret = ssd1307fb_write_cmd(par->client,
>> - (par->device_info->need_chargepump & 0x1 << 2) & 0x14);
>> + 0x10 | ((par->device_info->need_chargepump & 0x01) << 2));
I presume 'need_chargepump' is really supposed to be a bool, instead of
int. And if it's really bool, something like this makes it more readable
to me:
BIT(4) | (par->device_info->need_chargepump ? BIT(2) : 0)
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fbdev: ssd1307fb: Fix chargepump setting
2015-11-12 14:07 [PATCH] fbdev: ssd1307fb: Fix chargepump setting Julian Scheel
` (2 preceding siblings ...)
2016-01-29 11:55 ` Tomi Valkeinen
@ 2016-01-29 12:00 ` Julian Scheel
3 siblings, 0 replies; 5+ messages in thread
From: Julian Scheel @ 2016-01-29 12:00 UTC (permalink / raw)
To: linux-fbdev
Am 29.01.2016 um 12:55 schrieb Tomi Valkeinen:
>
> On 18/01/16 22:20, Julian Scheel wrote:
>> On 15.11.15 14:36, Julian Scheel wrote:
>>> The charge pump setting must have bit D4 set all time according to the
>>> SSD1306
>>> App Note. Instead of doing an logical and off shifted setting bit with
>>> 0x14 it
>>> must be an logical or with 0x10 to ensure D4 is set.
>
> The code change looks ok, but I can't quite decipher the commit
> description. "an logical and off shifted setting bit with 0x14"?
Appears I was not awake when writing the commit message. Besides that
the sentence is really cludgy it must be "of" instead of "off".
It might be simplified to: "Applying logical and to 0x14 and the
chargepump bit prevents bit 4 from being set. Fix it by or'ing bit 4 and
the chargepump enable bit"
>>> Signed-off-by: Julian Scheel <julian@jusst.de>
>>> ---
>>> drivers/video/fbdev/ssd1307fb.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/video/fbdev/ssd1307fb.c
>>> b/drivers/video/fbdev/ssd1307fb.c
>>> index 1611215..5965a9b 100644
>>> --- a/drivers/video/fbdev/ssd1307fb.c
>>> +++ b/drivers/video/fbdev/ssd1307fb.c
>>> @@ -389,7 +389,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
>>> return ret;
>>>
>>> ret = ssd1307fb_write_cmd(par->client,
>>> - (par->device_info->need_chargepump & 0x1 << 2) & 0x14);
>>> + 0x10 | ((par->device_info->need_chargepump & 0x01) << 2));
>
> I presume 'need_chargepump' is really supposed to be a bool, instead of
> int. And if it's really bool, something like this makes it more readable
> to me:
>
> BIT(4) | (par->device_info->need_chargepump ? BIT(2) : 0)
Yes, this is easier to read.
Let me update the patch and send a v2.
-Julian
^ permalink raw reply [flat|nested] 5+ messages in thread