* Re: [PATCH 8/8] fbdev: ssd1307fb: Turn off display on driver unload.
From: Maxime Ripard @ 2015-02-07 11:45 UTC (permalink / raw)
To: niederp; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <1423261694-5939-9-git-send-email-niederp@physik.uni-kl.de>
[-- Attachment #1: Type: text/plain, Size: 1186 bytes --]
On Fri, Feb 06, 2015 at 11:28:14PM +0100, niederp@physik.uni-kl.de wrote:
> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
>
A commit log is always nice :)
> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> ---
> drivers/video/fbdev/ssd1307fb.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 02931c7..be91dfc 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -762,6 +762,11 @@ static int ssd1307fb_remove(struct i2c_client *client)
> {
> struct fb_info *info = i2c_get_clientdata(client);
> struct ssd1307fb_par *par = info->par;
> + int ret = 0;
> +
> + ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_OFF);
> + if (ret < 0)
> + return ret;
I don't think we really care about the return value here.
It might be even worse actually, since you'll end up in a intermediate
state, where you won't have freed everything, but your remove method
has been called still.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 7/8] fbdev: ssd1307fb: Add sysfs handles to expose contrast and dim setting to userspace.
From: Maxime Ripard @ 2015-02-07 11:43 UTC (permalink / raw)
To: niederp; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <1423261694-5939-8-git-send-email-niederp@physik.uni-kl.de>
[-- Attachment #1: Type: text/plain, Size: 5142 bytes --]
On Fri, Feb 06, 2015 at 11:28:13PM +0100, niederp@physik.uni-kl.de wrote:
> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
>
> This patch adds sysfs handles to enable userspace control over the display
> contrast as well as the dim mode. The handles are available as "contrast"
> and "dim" in the framebuffers sysfs domain.
>
> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> ---
> drivers/video/fbdev/ssd1307fb.c | 88 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 87 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index b38315d..02931c7 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -33,6 +33,7 @@
> #define SSD1307FB_CONTRAST 0x81
> #define SSD1307FB_CHARGE_PUMP 0x8d
> #define SSD1307FB_SEG_REMAP_ON 0xa1
> +#define SSD1307FB_DISPLAY_DIM 0xac
> #define SSD1307FB_DISPLAY_OFF 0xae
> #define SSD1307FB_SET_MULTIPLEX_RATIO 0xa8
> #define SSD1307FB_DISPLAY_ON 0xaf
> @@ -43,6 +44,9 @@
> #define SSD1307FB_SET_COM_PINS_CONFIG 0xda
> #define SSD1307FB_SET_VCOMH 0xdb
>
> +#define MIN_CONTRAST 0
> +#define MAX_CONTRAST 255
> +
> #define BITSPERPIXEL 1
> #define DELAYDIVIDER 20
>
> @@ -69,6 +73,7 @@ struct ssd1307fb_par {
> u32 dclk_div;
> u32 dclk_frq;
> struct ssd1307fb_deviceinfo *device_info;
> + u32 dim;
> struct i2c_client *client;
> u32 height;
> struct fb_info *info;
> @@ -515,6 +520,79 @@ static const struct of_device_id ssd1307fb_of_match[] = {
> };
> MODULE_DEVICE_TABLE(of, ssd1307fb_of_match);
>
> +static ssize_t show_contrast(struct device *device,
> + struct device_attribute *attr, char *buf)
> +{
> + struct fb_info *info = dev_get_drvdata(device);
> + struct ssd1307fb_par *par = info->par;
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n", par->contrast);
> +}
> +
> +static ssize_t store_contrast(struct device *device,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct fb_info *info = dev_get_drvdata(device);
> + struct ssd1307fb_par *par = info->par;
> + unsigned long contrastval;
> + int ret;
> +
> + ret = kstrtoul(buf, 0, &contrastval);
> + if (ret < 0)
> + return ret;
> +
> + par->contrast = max(min(contrastval,
> + (ulong)MAX_CONTRAST), (ulong)MIN_CONTRAST);
> +
> + ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CONTRAST);
> + ret = ret & ssd1307fb_write_cmd(par->client, par->contrast);
> + if (ret < 0)
> + return ret;
> +
> + return count;
> +}
> +
> +
> +static ssize_t show_dim(struct device *device,
> + struct device_attribute *attr, char *buf)
> +{
> + struct fb_info *info = dev_get_drvdata(device);
> + struct ssd1307fb_par *par = info->par;
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n", par->dim);
> +}
> +
> +static ssize_t store_dim(struct device *device,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct fb_info *info = dev_get_drvdata(device);
> + struct ssd1307fb_par *par = info->par;
> + unsigned long dimval;
> + int ret;
> +
> + ret = kstrtoul(buf, 0, &dimval);
> + if (ret < 0)
> + return ret;
> +
> + par->dim = max(min(dimval, (ulong)1), (ulong)0);
> + if (par->dim)
> + ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_DIM);
> + else
> + ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
> + if (ret < 0)
> + return ret;
> +
> + return count;
> +}
> +
> +static struct device_attribute device_attrs[] = {
> + __ATTR(contrast, S_IRUGO|S_IWUSR, show_contrast, store_contrast),
> + __ATTR(dim, S_IRUGO|S_IWUSR, show_dim, store_dim),
> +
> +};
> +
I would have thought this was something accessible through the
framebuffer ioctl.
Apparently it's not, at least for the contrast, so maybe it should be
added there, instead of doing it for a single driver?
(oh, and btw, every sysfs file should be documented in
Documentation/ABI)
> static int ssd1307fb_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -523,7 +601,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
> u32 vmem_size;
> struct ssd1307fb_par *par;
> u8 *vmem;
> - int ret;
> + int ret, i;
>
> if (!node) {
> dev_err(&client->dev, "No device tree data found!\n");
> @@ -650,6 +728,14 @@ static int ssd1307fb_probe(struct i2c_client *client,
> goto reset_oled_error;
>
> ret = register_framebuffer(info);
> +
> + for (i = 0; i < ARRAY_SIZE(device_attrs); i++)
> + ret = device_create_file(info->dev, &device_attrs[i]);
> +
> + if (ret) {
> + dev_err(&client->dev, "Couldn't register sysfs nodes\n");
> + }
> +
sysfs_create_groups does pretty much that already.
And don't forget to remove these files in the .remove()
> if (ret) {
> dev_err(&client->dev, "Couldn't register the framebuffer\n");
> goto panel_init_error;
> --
> 2.1.1
>
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 6/8] fbdev: ssd1307fb: Add module parameter to set update delay of the deffered io.
From: Maxime Ripard @ 2015-02-07 11:26 UTC (permalink / raw)
To: niederp; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <1423261694-5939-7-git-send-email-niederp@physik.uni-kl.de>
[-- Attachment #1: Type: text/plain, Size: 2303 bytes --]
On Fri, Feb 06, 2015 at 11:28:12PM +0100, niederp@physik.uni-kl.de wrote:
> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
>
> This patch adds the module parameter "delaydivider" to set delay for the
> deferred io. Effectively this is setting the refresh rate for mmap access
> to the framebuffer. The delay for the deferred io is HZ/delaydivider.
So this is actually a refresh rate?
Maybe you could expose it as such, and pass a frequency in Hz as an
argument.
Exposing the divider directly has some issues, since the bootloader
that set the parameter won't know the HZ value, you'll end up with
different rates for different configurations, without any way to do
something about it.
>
> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> ---
> drivers/video/fbdev/ssd1307fb.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 1d81877..b38315d 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -44,10 +44,14 @@
> #define SSD1307FB_SET_VCOMH 0xdb
>
> #define BITSPERPIXEL 1
> +#define DELAYDIVIDER 20
>
> static u_int bitsperpixel = BITSPERPIXEL;
> module_param(bitsperpixel, uint, 0);
>
> +static u_int delaydivider = DELAYDIVIDER;
> +module_param(delaydivider, uint, 0);
> +
You're breaking the existing behaviour.
> struct ssd1307fb_par;
>
> struct ssd1307fb_deviceinfo {
> @@ -312,7 +316,7 @@ static void ssd1307fb_deferred_io(struct fb_info *info,
> }
>
> static struct fb_deferred_io ssd1307fb_defio = {
> - .delay = HZ,
> + .delay = HZ/DELAYDIVIDER,
> .deferred_io = ssd1307fb_deferred_io,
> };
>
> @@ -601,6 +605,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
> info->fix = ssd1307fb_fix;
> info->fix.line_length = par->width * bitsperpixel / 8;
> info->fbdefio = &ssd1307fb_defio;
> + info->fbdefio->delay = HZ/delaydivider;
That won't work with multiple instances of the same driver
unfortunately.
>
> info->var = ssd1307fb_var;
> info->var.bits_per_pixel = bitsperpixel;
> --
> 2.1.1
>
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 5/8] fbdev: ssd1307fb: Add module parameter bitsperpixel.
From: Maxime Ripard @ 2015-02-07 11:20 UTC (permalink / raw)
To: niederp; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <1423261694-5939-6-git-send-email-niederp@physik.uni-kl.de>
[-- Attachment #1: Type: text/plain, Size: 763 bytes --]
On Fri, Feb 06, 2015 at 11:28:11PM +0100, niederp@physik.uni-kl.de wrote:
> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
>
> This patch adds a module parameter 'bitsperpixel' to adjust the colordepth
> of the framebuffer. All values >1 will result in memory map of the requested
> color depth. However only the MSB of each pixel will be sent to the device.
> The framebuffer identifies itself as a grayscale display with the specified
> depth.
I'm not sure this is the right thing to do.
The bits per pixel for this display is rightfully defined, used and
reported to the userspace, why would you want to change that?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Maxime Ripard @ 2015-02-07 11:18 UTC (permalink / raw)
To: niederp; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <1423261694-5939-5-git-send-email-niederp@physik.uni-kl.de>
[-- Attachment #1: Type: text/plain, Size: 3715 bytes --]
Hi,
On Fri, Feb 06, 2015 at 11:28:10PM +0100, niederp@physik.uni-kl.de wrote:
> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
>
> It makes sense to use vmalloc to allocate the video buffer since it
> has to be page aligned memory for using it with mmap.
Please wrap your commit log at 80 chars.
It looks like there's numerous fbdev drivers using this (especially
since you copy pasted that code, without mentionning it).
That should be turned into an allocator so that drivers all get this
right.
> Also deffered io seems buggy in combination with kmalloc'ed memory
> (crash on unloading the module).
And maybe that's the real issue to fix.
> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> ---
> drivers/video/fbdev/ssd1307fb.c | 43 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 01cfb46..945ded9 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -11,6 +11,7 @@
> #include <linux/i2c.h>
> #include <linux/fb.h>
> #include <linux/uaccess.h>
> +#include <linux/vmalloc.h>
> #include <linux/of_device.h>
> #include <linux/of_gpio.h>
> #include <linux/pwm.h>
> @@ -93,6 +94,43 @@ static struct fb_var_screeninfo ssd1307fb_var = {
> .bits_per_pixel = 1,
> };
>
> +static void *rvmalloc(unsigned long size)
> +{
> + void *mem;
> + unsigned long adr;
> +
> + size = PAGE_ALIGN(size);
Isn't vmalloc already taking care of that?
> + mem = vmalloc_32(size);
Why the 32 bits variant?
> + if (!mem)
> + return NULL;
> +
> + memset(mem, 0, size); /* Clear the ram out, no junk to the user */
> + adr = (unsigned long) mem;
> + while (size > 0) {
> + SetPageReserved(vmalloc_to_page((void *)adr));
I'm not really sure it makes sense to mark all pages reserved if we're
not even sure we're going to use mmap.
And why do you need to mark these pages reserved in the first place?
> + adr += PAGE_SIZE;
> + size -= PAGE_SIZE;
> + }
> +
> + return mem;
> +}
> +
> +static void rvfree(void *mem, unsigned long size)
> +{
> + unsigned long adr;
> +
> + if (!mem)
> + return;
> +
> + adr = (unsigned long) mem;
> + while ((long) size > 0) {
> + ClearPageReserved(vmalloc_to_page((void *)adr));
> + adr += PAGE_SIZE;
> + size -= PAGE_SIZE;
> + }
> + vfree(mem);
> +}
> +
> static struct ssd1307fb_array *ssd1307fb_alloc_array(u32 len, u8 type)
> {
> struct ssd1307fb_array *array;
> @@ -538,13 +576,13 @@ static int ssd1307fb_probe(struct i2c_client *client,
> if (of_property_read_u32(node, "solomon,vcomh", &par->vcomh))
> par->vcomh = par->device_info->default_vcomh;
>
> - vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
> if (of_property_read_u32(node, "solomon,dclk-div", &par->dclk_div))
> par->dclk_div = par->device_info->default_dclk_div;
>
> if (of_property_read_u32(node, "solomon,dclk-frq", &par->dclk_frq))
> par->dclk_frq = par->device_info->default_dclk_frq;
>
> + vmem = rvmalloc(vmem_size);
> if (!vmem) {
> dev_err(&client->dev, "Couldn't allocate graphical memory.\n");
> ret = -ENOMEM;
> @@ -570,7 +608,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
> info->var.blue.offset = 0;
>
> info->screen_base = (u8 __force __iomem *)vmem;
> - info->fix.smem_start = (unsigned long)vmem;
> + info->fix.smem_start = __pa(vmem);
Why are you changing from virtual to physical address here?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/8] fbdev: ssd1307fb: Unify init code and make controller configurable from device tree
From: Maxime Ripard @ 2015-02-07 10:42 UTC (permalink / raw)
To: niederp; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <1423261694-5939-3-git-send-email-niederp@physik.uni-kl.de>
[-- Attachment #1: Type: text/plain, Size: 14496 bytes --]
Hi,
On Fri, Feb 06, 2015 at 11:28:08PM +0100, niederp@physik.uni-kl.de wrote:
> From: Thomas Niederprüm <niederp@physik.uni-kl.de>
>
> This patches unifies the init code for the ssd130X chips and
> adds device tree bindings to describe the hardware configuration
> of the used controller. This gets rid of the magic bit values
> used in the init code so far.
>
> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> ---
> .../devicetree/bindings/video/ssd1307fb.txt | 11 +
> drivers/video/fbdev/ssd1307fb.c | 243 ++++++++++++++-------
> 2 files changed, 174 insertions(+), 80 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt b/Documentation/devicetree/bindings/video/ssd1307fb.txt
> index 7a12542..1230f68 100644
> --- a/Documentation/devicetree/bindings/video/ssd1307fb.txt
> +++ b/Documentation/devicetree/bindings/video/ssd1307fb.txt
> @@ -15,6 +15,17 @@ Required properties:
>
> Optional properties:
> - reset-active-low: Is the reset gpio is active on physical low?
> + - solomon,segment-remap: Invert the order of data column to segment mapping
> + - solomon,offset: Map the display start line to one of COM0 - COM63
> + - solomon,contrast: Set initial contrast of the display
> + - solomon,prechargep1: Set the duration of the precharge period phase1
> + - solomon,prechargep2: Set the duration of the precharge period phase2
> + - solomon,com-alt: Enable/disable alternate COM pin configuration
> + - solomon,com-lrremap: Enable/disable left-right remap of COM pins
> + - solomon,com-invdir: Invert COM scan direction
> + - solomon,vcomh: Set VCOMH regulator voltage
> + - solomon,dclk-div: Set display clock divider
> + - solomon,dclk-frq: Set display clock frequency
I'm sorry, but this is the wrong approach, for at least two reasons:
you broke all existing users of that driver, which is a clear no-go,
and the DT itself should not contain any direct mapping of the
registers.
>
> [0]: Documentation/devicetree/bindings/pwm/pwm.txt
>
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 3d6611f..4f435aa 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -16,6 +16,9 @@
> #include <linux/pwm.h>
> #include <linux/delay.h>
>
> +#define DEVID_SSD1306 6
> +#define DEVID_SSD1307 7
> +
> #define SSD1307FB_DATA 0x40
> #define SSD1307FB_COMMAND 0x80
>
> @@ -40,20 +43,33 @@
>
> struct ssd1307fb_par;
>
> -struct ssd1307fb_ops {
> - int (*init)(struct ssd1307fb_par *);
> - int (*remove)(struct ssd1307fb_par *);
> +struct ssd1307fb_deviceinfo {
> + int device_id;
> + u32 default_vcomh;
> + u32 default_dclk_div;
> + u32 default_dclk_frq;
> };
>
> struct ssd1307fb_par {
> + u32 com_alt;
> + u32 com_invdir;
> + u32 com_lrremap;
> + u32 contrast;
> + u32 dclk_div;
> + u32 dclk_frq;
> + struct ssd1307fb_deviceinfo *device_info;
> struct i2c_client *client;
> u32 height;
> struct fb_info *info;
> - struct ssd1307fb_ops *ops;
> + u32 offset;
> u32 page_offset;
> + u32 prechargep1;
> + u32 prechargep2;
> struct pwm_device *pwm;
> u32 pwm_period;
> int reset;
> + u32 seg_remap;
> + u32 vcomh;
> u32 width;
> };
>
> @@ -254,127 +270,151 @@ static struct fb_deferred_io ssd1307fb_defio = {
> .deferred_io = ssd1307fb_deferred_io,
> };
>
> -static int ssd1307fb_ssd1307_init(struct ssd1307fb_par *par)
> +static int ssd1307fb_init(struct ssd1307fb_par *par)
> {
> int ret;
> + u32 precharge, dclk, com_invdir, compins;
>
> - par->pwm = pwm_get(&par->client->dev, NULL);
> - if (IS_ERR(par->pwm)) {
> - dev_err(&par->client->dev, "Could not get PWM from device tree!\n");
> - return PTR_ERR(par->pwm);
> - }
> -
> - par->pwm_period = pwm_get_period(par->pwm);
> - /* Enable the PWM */
> - pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
> - pwm_enable(par->pwm);
> -
> - dev_dbg(&par->client->dev, "Using PWM%d with a %dns period.\n",
> - par->pwm->pwm, par->pwm_period);
> -
> - /* Map column 127 of the OLED to segment 0 */
> - ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SEG_REMAP_ON);
> - if (ret < 0)
> - return ret;
> -
> - /* Turn on the display */
> - ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
> - if (ret < 0)
> - return ret;
> -
> - return 0;
> -}
> -
> -static int ssd1307fb_ssd1307_remove(struct ssd1307fb_par *par)
> -{
> - pwm_disable(par->pwm);
> - pwm_put(par->pwm);
> - return 0;
> -}
> + if (par->device_info->device_id == DEVID_SSD1307) {
> + par->pwm = pwm_get(&par->client->dev, NULL);
> + if (IS_ERR(par->pwm)) {
> + dev_err(&par->client->dev, "Could not get PWM from device tree!\n");
> + return PTR_ERR(par->pwm);
> + }
>
> -static struct ssd1307fb_ops ssd1307fb_ssd1307_ops = {
> - .init = ssd1307fb_ssd1307_init,
> - .remove = ssd1307fb_ssd1307_remove,
> -};
> + par->pwm_period = pwm_get_period(par->pwm);
> + /* Enable the PWM */
> + pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
> + pwm_enable(par->pwm);
>
> -static int ssd1307fb_ssd1306_init(struct ssd1307fb_par *par)
> -{
> - int ret;
> + dev_dbg(&par->client->dev, "Using PWM%d with a %dns period.\n",
> + par->pwm->pwm, par->pwm_period);
> + };
>
> /* Set initial contrast */
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CONTRAST);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0x7f);
> if (ret < 0)
> return ret;
>
> - /* Set COM direction */
> - ret = ssd1307fb_write_cmd(par->client, 0xc8);
> + ret = ssd1307fb_write_cmd(par->client, par->contrast);
> if (ret < 0)
> return ret;
>
> /* Set segment re-map */
> - ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SEG_REMAP_ON);
> + if (par->seg_remap) {
> + ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SEG_REMAP_ON);
> + if (ret < 0)
> + return ret;
> + };
> +
> + /* Set COM direction */
> + com_invdir = 0xc0 | (par->com_invdir & 0xf) << 3;
> + ret = ssd1307fb_write_cmd(par->client, com_invdir);
> if (ret < 0)
> return ret;
>
> /* Set multiplex ratio value */
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_MULTIPLEX_RATIO);
> - ret = ret & ssd1307fb_write_cmd(par->client, par->height - 1);
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client, par->height - 1);
> if (ret < 0)
> return ret;
>
> /* set display offset value */
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_DISPLAY_OFFSET);
> - ret = ssd1307fb_write_cmd(par->client, 0x20);
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client, par->offset);
> if (ret < 0)
> return ret;
>
> /* Set clock frequency */
> + dclk = (par->dclk_div & 0xf) | (par->dclk_frq & 0xf) << 4;
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_CLOCK_FREQ);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0xf0);
> if (ret < 0)
> return ret;
>
> - /* Set precharge period in number of ticks from the internal clock */
> + ret = ssd1307fb_write_cmd(par->client, dclk);
> + if (ret < 0)
> + return ret;
> +
> + /* Set precharge period in number of ticks from the internal clock*/
> + precharge = (par->prechargep1 & 0xf) | (par->prechargep2 & 0xf) << 4;
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_PRECHARGE_PERIOD);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0x22);
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client, precharge);
> if (ret < 0)
> return ret;
>
> /* Set COM pins configuration */
> + compins = 0x02 | (par->com_alt & 0x1) << 4
> + | (par->com_lrremap & 0x1) << 5;
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_COM_PINS_CONFIG);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0x22);
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client, compins);
> if (ret < 0)
> return ret;
>
> /* Set VCOMH */
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_VCOMH);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0x49);
> if (ret < 0)
> return ret;
>
> - /* Turn on the DC-DC Charge Pump */
> - ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CHARGE_PUMP);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0x14);
> + ret = ssd1307fb_write_cmd(par->client, par->vcomh);
> if (ret < 0)
> return ret;
>
> + if (par->device_info->device_id == DEVID_SSD1306) {
> + /* Turn on the DC-DC Charge Pump */
> + ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CHARGE_PUMP);
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client, 0x14);
> + if (ret < 0)
> + return ret;
> + };
> +
> /* Switch to horizontal addressing mode */
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_ADDRESS_MODE);
> - ret = ret & ssd1307fb_write_cmd(par->client,
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client,
> SSD1307FB_SET_ADDRESS_MODE_HORIZONTAL);
> if (ret < 0)
> return ret;
>
> + /* Set column range */
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_COL_RANGE);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0x0);
> - ret = ret & ssd1307fb_write_cmd(par->client, par->width - 1);
> if (ret < 0)
> return ret;
>
> + ret = ssd1307fb_write_cmd(par->client, 0x0);
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client, par->width - 1);
> + if (ret < 0)
> + return ret;
> +
> + /* Set page range */
> ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_PAGE_RANGE);
> - ret = ret & ssd1307fb_write_cmd(par->client, 0x0);
> - ret = ret & ssd1307fb_write_cmd(par->client,
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client, 0x0);
> + if (ret < 0)
> + return ret;
> +
> + ret = ssd1307fb_write_cmd(par->client,
> par->page_offset + (par->height / 8) - 1);
> if (ret < 0)
> return ret;
> @@ -387,18 +427,28 @@ static int ssd1307fb_ssd1306_init(struct ssd1307fb_par *par)
> return 0;
> }
>
> -static struct ssd1307fb_ops ssd1307fb_ssd1306_ops = {
> - .init = ssd1307fb_ssd1306_init,
> +static struct ssd1307fb_deviceinfo ssd1307fb_ssd1306_deviceinfo = {
> + .device_id = DEVID_SSD1306,
> + .default_vcomh = 0x20,
> + .default_dclk_div = 0,
> + .default_dclk_frq = 8,
> +};
> +
> +static struct ssd1307fb_deviceinfo ssd1307fb_ssd1307_deviceinfo = {
> + .device_id = DEVID_SSD1307,
> + .default_vcomh = 0x20,
> + .default_dclk_div = 1,
> + .default_dclk_frq = 12,
> };
>
> static const struct of_device_id ssd1307fb_of_match[] = {
> {
> .compatible = "solomon,ssd1306fb-i2c",
> - .data = (void *)&ssd1307fb_ssd1306_ops,
> + .data = (void *)&ssd1307fb_ssd1306_deviceinfo,
> },
> {
> .compatible = "solomon,ssd1307fb-i2c",
> - .data = (void *)&ssd1307fb_ssd1307_ops,
> + .data = (void *)&ssd1307fb_ssd1307_deviceinfo,
> },
> {},
> };
> @@ -429,8 +479,8 @@ static int ssd1307fb_probe(struct i2c_client *client,
> par->info = info;
> par->client = client;
>
> - par->ops = (struct ssd1307fb_ops *)of_match_device(ssd1307fb_of_match,
> - &client->dev)->data;
> + par->device_info = (struct ssd1307fb_deviceinfo *)of_match_device(
> + ssd1307fb_of_match, &client->dev)->data;
>
> par->reset = of_get_named_gpio(client->dev.of_node,
> "reset-gpios", 0);
> @@ -449,8 +499,40 @@ static int ssd1307fb_probe(struct i2c_client *client,
> par->page_offset = 1;
>
> vmem_size = par->width * par->height / 8;
> + if (of_property_read_u32(node, "solomon,segment-remap", &par->seg_remap))
> + par->seg_remap = 0;
> +
> + if (of_property_read_u32(node, "solomon,offset", &par->offset))
> + par->offset = 0;
> +
> + if (of_property_read_u32(node, "solomon,contrast", &par->contrast))
> + par->contrast = 128;
> +
> + if (of_property_read_u32(node, "solomon,prechargep1", &par->prechargep1))
> + par->prechargep1 = 2;
> +
> + if (of_property_read_u32(node, "solomon,prechargep2", &par->prechargep2))
> + par->prechargep2 = 2;
> +
> + if (of_property_read_u32(node, "solomon,com-alt", &par->com_alt))
> + par->com_alt = 1;
> +
> + if (of_property_read_u32(node, "solomon,com-lrremap", &par->com_lrremap))
> + par->com_lrremap = 0;
> +
> + if (of_property_read_u32(node, "solomon,com-invdir", &par->com_invdir))
> + par->com_invdir = 0;
> +
> + if (of_property_read_u32(node, "solomon,vcomh", &par->vcomh))
> + par->vcomh = par->device_info->default_vcomh;
>
> vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
> + if (of_property_read_u32(node, "solomon,dclk-div", &par->dclk_div))
> + par->dclk_div = par->device_info->default_dclk_div;
> +
> + if (of_property_read_u32(node, "solomon,dclk-frq", &par->dclk_frq))
> + par->dclk_frq = par->device_info->default_dclk_frq;
> +
> if (!vmem) {
> dev_err(&client->dev, "Couldn't allocate graphical memory.\n");
> ret = -ENOMEM;
> @@ -499,11 +581,9 @@ static int ssd1307fb_probe(struct i2c_client *client,
> gpio_set_value(par->reset, 1);
> udelay(4);
>
> - if (par->ops->init) {
> - ret = par->ops->init(par);
> - if (ret)
> - goto reset_oled_error;
> - }
> + ret = ssd1307fb_init(par);
> + if (ret)
> + goto reset_oled_error;
>
> ret = register_framebuffer(info);
> if (ret) {
> @@ -516,8 +596,10 @@ static int ssd1307fb_probe(struct i2c_client *client,
> return 0;
>
> panel_init_error:
> - if (par->ops->remove)
> - par->ops->remove(par);
> + if (par->device_info->device_id == DEVID_SSD1307) {
> + pwm_disable(par->pwm);
> + pwm_put(par->pwm);
> + };
> reset_oled_error:
> fb_deferred_io_cleanup(info);
> fb_alloc_error:
> @@ -531,11 +613,12 @@ static int ssd1307fb_remove(struct i2c_client *client)
> struct ssd1307fb_par *par = info->par;
>
> unregister_framebuffer(info);
> - if (par->ops->remove)
> - par->ops->remove(par);
> + if (par->device_info->device_id == DEVID_SSD1307) {
> + pwm_disable(par->pwm);
> + pwm_put(par->pwm);
> + };
> fb_deferred_io_cleanup(info);
> framebuffer_release(info);
> -
> return 0;
> }
>
> --
> 2.1.1
>
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH 8/8] fbdev: ssd1307fb: Turn off display on driver unload.
From: niederp @ 2015-02-06 22:28 UTC (permalink / raw)
To: linux-fbdev, plagnioj, tomi.valkeinen, maxime.ripard
Cc: linux-kernel, Thomas Niederprüm
In-Reply-To: <1423261694-5939-1-git-send-email-niederp@physik.uni-kl.de>
From: Thomas Niederprüm <niederp@physik.uni-kl.de>
Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
---
drivers/video/fbdev/ssd1307fb.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 02931c7..be91dfc 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -762,6 +762,11 @@ static int ssd1307fb_remove(struct i2c_client *client)
{
struct fb_info *info = i2c_get_clientdata(client);
struct ssd1307fb_par *par = info->par;
+ int ret = 0;
+
+ ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_OFF);
+ if (ret < 0)
+ return ret;
unregister_framebuffer(info);
if (par->device_info->device_id = DEVID_SSD1307) {
--
2.1.1
^ permalink raw reply related
* [PATCH 7/8] fbdev: ssd1307fb: Add sysfs handles to expose contrast and dim setting to userspace.
From: niederp @ 2015-02-06 22:28 UTC (permalink / raw)
To: linux-fbdev, plagnioj, tomi.valkeinen, maxime.ripard
Cc: linux-kernel, Thomas Niederprüm
In-Reply-To: <1423261694-5939-1-git-send-email-niederp@physik.uni-kl.de>
From: Thomas Niederprüm <niederp@physik.uni-kl.de>
This patch adds sysfs handles to enable userspace control over the display
contrast as well as the dim mode. The handles are available as "contrast"
and "dim" in the framebuffers sysfs domain.
Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
---
drivers/video/fbdev/ssd1307fb.c | 88 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 87 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index b38315d..02931c7 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -33,6 +33,7 @@
#define SSD1307FB_CONTRAST 0x81
#define SSD1307FB_CHARGE_PUMP 0x8d
#define SSD1307FB_SEG_REMAP_ON 0xa1
+#define SSD1307FB_DISPLAY_DIM 0xac
#define SSD1307FB_DISPLAY_OFF 0xae
#define SSD1307FB_SET_MULTIPLEX_RATIO 0xa8
#define SSD1307FB_DISPLAY_ON 0xaf
@@ -43,6 +44,9 @@
#define SSD1307FB_SET_COM_PINS_CONFIG 0xda
#define SSD1307FB_SET_VCOMH 0xdb
+#define MIN_CONTRAST 0
+#define MAX_CONTRAST 255
+
#define BITSPERPIXEL 1
#define DELAYDIVIDER 20
@@ -69,6 +73,7 @@ struct ssd1307fb_par {
u32 dclk_div;
u32 dclk_frq;
struct ssd1307fb_deviceinfo *device_info;
+ u32 dim;
struct i2c_client *client;
u32 height;
struct fb_info *info;
@@ -515,6 +520,79 @@ static const struct of_device_id ssd1307fb_of_match[] = {
};
MODULE_DEVICE_TABLE(of, ssd1307fb_of_match);
+static ssize_t show_contrast(struct device *device,
+ struct device_attribute *attr, char *buf)
+{
+ struct fb_info *info = dev_get_drvdata(device);
+ struct ssd1307fb_par *par = info->par;
+
+ return snprintf(buf, PAGE_SIZE, "%d\n", par->contrast);
+}
+
+static ssize_t store_contrast(struct device *device,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fb_info *info = dev_get_drvdata(device);
+ struct ssd1307fb_par *par = info->par;
+ unsigned long contrastval;
+ int ret;
+
+ ret = kstrtoul(buf, 0, &contrastval);
+ if (ret < 0)
+ return ret;
+
+ par->contrast = max(min(contrastval,
+ (ulong)MAX_CONTRAST), (ulong)MIN_CONTRAST);
+
+ ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CONTRAST);
+ ret = ret & ssd1307fb_write_cmd(par->client, par->contrast);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+
+
+static ssize_t show_dim(struct device *device,
+ struct device_attribute *attr, char *buf)
+{
+ struct fb_info *info = dev_get_drvdata(device);
+ struct ssd1307fb_par *par = info->par;
+
+ return snprintf(buf, PAGE_SIZE, "%d\n", par->dim);
+}
+
+static ssize_t store_dim(struct device *device,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fb_info *info = dev_get_drvdata(device);
+ struct ssd1307fb_par *par = info->par;
+ unsigned long dimval;
+ int ret;
+
+ ret = kstrtoul(buf, 0, &dimval);
+ if (ret < 0)
+ return ret;
+
+ par->dim = max(min(dimval, (ulong)1), (ulong)0);
+ if (par->dim)
+ ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_DIM);
+ else
+ ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+
+static struct device_attribute device_attrs[] = {
+ __ATTR(contrast, S_IRUGO|S_IWUSR, show_contrast, store_contrast),
+ __ATTR(dim, S_IRUGO|S_IWUSR, show_dim, store_dim),
+
+};
+
static int ssd1307fb_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -523,7 +601,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
u32 vmem_size;
struct ssd1307fb_par *par;
u8 *vmem;
- int ret;
+ int ret, i;
if (!node) {
dev_err(&client->dev, "No device tree data found!\n");
@@ -650,6 +728,14 @@ static int ssd1307fb_probe(struct i2c_client *client,
goto reset_oled_error;
ret = register_framebuffer(info);
+
+ for (i = 0; i < ARRAY_SIZE(device_attrs); i++)
+ ret = device_create_file(info->dev, &device_attrs[i]);
+
+ if (ret) {
+ dev_err(&client->dev, "Couldn't register sysfs nodes\n");
+ }
+
if (ret) {
dev_err(&client->dev, "Couldn't register the framebuffer\n");
goto panel_init_error;
--
2.1.1
^ permalink raw reply related
* [PATCH 6/8] fbdev: ssd1307fb: Add module parameter to set update delay of the deffered io.
From: niederp @ 2015-02-06 22:28 UTC (permalink / raw)
To: linux-fbdev, plagnioj, tomi.valkeinen, maxime.ripard
Cc: linux-kernel, Thomas Niederprüm
In-Reply-To: <1423261694-5939-1-git-send-email-niederp@physik.uni-kl.de>
From: Thomas Niederprüm <niederp@physik.uni-kl.de>
This patch adds the module parameter "delaydivider" to set delay for the
deferred io. Effectively this is setting the refresh rate for mmap access
to the framebuffer. The delay for the deferred io is HZ/delaydivider.
Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
---
drivers/video/fbdev/ssd1307fb.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 1d81877..b38315d 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -44,10 +44,14 @@
#define SSD1307FB_SET_VCOMH 0xdb
#define BITSPERPIXEL 1
+#define DELAYDIVIDER 20
static u_int bitsperpixel = BITSPERPIXEL;
module_param(bitsperpixel, uint, 0);
+static u_int delaydivider = DELAYDIVIDER;
+module_param(delaydivider, uint, 0);
+
struct ssd1307fb_par;
struct ssd1307fb_deviceinfo {
@@ -312,7 +316,7 @@ static void ssd1307fb_deferred_io(struct fb_info *info,
}
static struct fb_deferred_io ssd1307fb_defio = {
- .delay = HZ,
+ .delay = HZ/DELAYDIVIDER,
.deferred_io = ssd1307fb_deferred_io,
};
@@ -601,6 +605,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
info->fix = ssd1307fb_fix;
info->fix.line_length = par->width * bitsperpixel / 8;
info->fbdefio = &ssd1307fb_defio;
+ info->fbdefio->delay = HZ/delaydivider;
info->var = ssd1307fb_var;
info->var.bits_per_pixel = bitsperpixel;
--
2.1.1
^ permalink raw reply related
* [PATCH 5/8] fbdev: ssd1307fb: Add module parameter bitsperpixel.
From: niederp @ 2015-02-06 22:28 UTC (permalink / raw)
To: linux-fbdev, plagnioj, tomi.valkeinen, maxime.ripard
Cc: linux-kernel, Thomas Niederprüm
In-Reply-To: <1423261694-5939-1-git-send-email-niederp@physik.uni-kl.de>
From: Thomas Niederprüm <niederp@physik.uni-kl.de>
This patch adds a module parameter 'bitsperpixel' to adjust the colordepth
of the framebuffer. All values >1 will result in memory map of the requested
color depth. However only the MSB of each pixel will be sent to the device.
The framebuffer identifies itself as a grayscale display with the specified
depth.
Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
---
drivers/video/fbdev/ssd1307fb.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 945ded9..1d81877 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -43,6 +43,11 @@
#define SSD1307FB_SET_COM_PINS_CONFIG 0xda
#define SSD1307FB_SET_VCOMH 0xdb
+#define BITSPERPIXEL 1
+
+static u_int bitsperpixel = BITSPERPIXEL;
+module_param(bitsperpixel, uint, 0);
+
struct ssd1307fb_par;
struct ssd1307fb_deviceinfo {
@@ -91,7 +96,8 @@ static struct fb_fix_screeninfo ssd1307fb_fix = {
};
static struct fb_var_screeninfo ssd1307fb_var = {
- .bits_per_pixel = 1,
+ .bits_per_pixel = BITSPERPIXEL,
+ .grayscale = 1,
};
static void *rvmalloc(unsigned long size)
@@ -223,10 +229,11 @@ static void ssd1307fb_update_display(struct ssd1307fb_par *par)
array->data[array_idx] = 0;
for (k = 0; k < 8; k++) {
u32 page_length = par->width * i;
- u32 index = page_length + (par->width * k + j) / 8;
+ u32 index = page_length * bitsperpixel + (par->width * k + j) * bitsperpixel / 8;
u8 byte = *(vmem + index);
- u8 bit = byte & (1 << (j % 8));
- bit = bit >> (j % 8);
+ u8 bit = byte & (((1 << (bitsperpixel))-1) << (j % 8/bitsperpixel));
+
+ bit = (bit >> (j % 8/bitsperpixel)) >> (bitsperpixel-1);
array->data[array_idx] |= bit << k;
}
}
@@ -548,7 +555,6 @@ static int ssd1307fb_probe(struct i2c_client *client,
if (of_property_read_u32(node, "solomon,page-offset", &par->page_offset))
par->page_offset = 1;
- vmem_size = par->width * par->height / 8;
if (of_property_read_u32(node, "solomon,segment-remap", &par->seg_remap))
par->seg_remap = 0;
@@ -582,6 +588,8 @@ static int ssd1307fb_probe(struct i2c_client *client,
if (of_property_read_u32(node, "solomon,dclk-frq", &par->dclk_frq))
par->dclk_frq = par->device_info->default_dclk_frq;
+ vmem_size = par->width * par->height * bitsperpixel / 8;
+
vmem = rvmalloc(vmem_size);
if (!vmem) {
dev_err(&client->dev, "Couldn't allocate graphical memory.\n");
@@ -591,20 +599,21 @@ static int ssd1307fb_probe(struct i2c_client *client,
info->fbops = &ssd1307fb_ops;
info->fix = ssd1307fb_fix;
- info->fix.line_length = par->width / 8;
+ info->fix.line_length = par->width * bitsperpixel / 8;
info->fbdefio = &ssd1307fb_defio;
info->var = ssd1307fb_var;
+ info->var.bits_per_pixel = bitsperpixel;
info->var.xres = par->width;
info->var.xres_virtual = par->width;
info->var.yres = par->height;
info->var.yres_virtual = par->height;
- info->var.red.length = 1;
+ info->var.red.length = bitsperpixel;
info->var.red.offset = 0;
- info->var.green.length = 1;
+ info->var.green.length = bitsperpixel;
info->var.green.offset = 0;
- info->var.blue.length = 1;
+ info->var.blue.length = bitsperpixel;
info->var.blue.offset = 0;
info->screen_base = (u8 __force __iomem *)vmem;
--
2.1.1
^ permalink raw reply related
* [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: niederp @ 2015-02-06 22:28 UTC (permalink / raw)
To: linux-fbdev, plagnioj, tomi.valkeinen, maxime.ripard
Cc: linux-kernel, Thomas Niederprüm
In-Reply-To: <1423261694-5939-1-git-send-email-niederp@physik.uni-kl.de>
From: Thomas Niederprüm <niederp@physik.uni-kl.de>
It makes sense to use vmalloc to allocate the video buffer since it has to be page aligned memory for using it with
mmap. Also deffered io seems buggy in combination with kmalloc'ed memory (crash on unloading the module).
Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
---
drivers/video/fbdev/ssd1307fb.c | 43 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 41 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 01cfb46..945ded9 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -11,6 +11,7 @@
#include <linux/i2c.h>
#include <linux/fb.h>
#include <linux/uaccess.h>
+#include <linux/vmalloc.h>
#include <linux/of_device.h>
#include <linux/of_gpio.h>
#include <linux/pwm.h>
@@ -93,6 +94,43 @@ static struct fb_var_screeninfo ssd1307fb_var = {
.bits_per_pixel = 1,
};
+static void *rvmalloc(unsigned long size)
+{
+ void *mem;
+ unsigned long adr;
+
+ size = PAGE_ALIGN(size);
+ mem = vmalloc_32(size);
+ if (!mem)
+ return NULL;
+
+ memset(mem, 0, size); /* Clear the ram out, no junk to the user */
+ adr = (unsigned long) mem;
+ while (size > 0) {
+ SetPageReserved(vmalloc_to_page((void *)adr));
+ adr += PAGE_SIZE;
+ size -= PAGE_SIZE;
+ }
+
+ return mem;
+}
+
+static void rvfree(void *mem, unsigned long size)
+{
+ unsigned long adr;
+
+ if (!mem)
+ return;
+
+ adr = (unsigned long) mem;
+ while ((long) size > 0) {
+ ClearPageReserved(vmalloc_to_page((void *)adr));
+ adr += PAGE_SIZE;
+ size -= PAGE_SIZE;
+ }
+ vfree(mem);
+}
+
static struct ssd1307fb_array *ssd1307fb_alloc_array(u32 len, u8 type)
{
struct ssd1307fb_array *array;
@@ -538,13 +576,13 @@ static int ssd1307fb_probe(struct i2c_client *client,
if (of_property_read_u32(node, "solomon,vcomh", &par->vcomh))
par->vcomh = par->device_info->default_vcomh;
- vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
if (of_property_read_u32(node, "solomon,dclk-div", &par->dclk_div))
par->dclk_div = par->device_info->default_dclk_div;
if (of_property_read_u32(node, "solomon,dclk-frq", &par->dclk_frq))
par->dclk_frq = par->device_info->default_dclk_frq;
+ vmem = rvmalloc(vmem_size);
if (!vmem) {
dev_err(&client->dev, "Couldn't allocate graphical memory.\n");
ret = -ENOMEM;
@@ -570,7 +608,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
info->var.blue.offset = 0;
info->screen_base = (u8 __force __iomem *)vmem;
- info->fix.smem_start = (unsigned long)vmem;
+ info->fix.smem_start = __pa(vmem);
info->fix.smem_len = vmem_size;
fb_deferred_io_init(info);
@@ -614,6 +652,7 @@ panel_init_error:
};
reset_oled_error:
fb_deferred_io_cleanup(info);
+ rvfree(vmem, vmem_size);
fb_alloc_error:
framebuffer_release(info);
return ret;
--
2.1.1
^ permalink raw reply related
* [PATCH 3/8] fbdev: ssd1307fb: Add support for SSD1305
From: niederp @ 2015-02-06 22:28 UTC (permalink / raw)
To: linux-fbdev, plagnioj, tomi.valkeinen, maxime.ripard
Cc: linux-kernel, Thomas Niederprüm
In-Reply-To: <1423261694-5939-1-git-send-email-niederp@physik.uni-kl.de>
From: Thomas Niederprüm <niederp@physik.uni-kl.de>
This patch adds support for the SSD1305 OLED controller.
Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
---
Documentation/devicetree/bindings/video/ssd1307fb.txt | 2 +-
drivers/video/fbdev/ssd1307fb.c | 13 +++++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt b/Documentation/devicetree/bindings/video/ssd1307fb.txt
index 1230f68..51fa263 100644
--- a/Documentation/devicetree/bindings/video/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/video/ssd1307fb.txt
@@ -2,7 +2,7 @@
Required properties:
- compatible: Should be "solomon,<chip>fb-<bus>". The only supported bus for
- now is i2c, and the supported chips are ssd1306 and ssd1307.
+ now is i2c, and the supported chips are ssd1305, ssd1306 and ssd1307.
- reg: Should contain address of the controller on the I2C bus. Most likely
0x3c or 0x3d
- pwm: Should contain the pwm to use according to the OF device tree PWM
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 4f435aa..01cfb46 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -16,6 +16,7 @@
#include <linux/pwm.h>
#include <linux/delay.h>
+#define DEVID_SSD1305 5
#define DEVID_SSD1306 6
#define DEVID_SSD1307 7
@@ -427,6 +428,13 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
return 0;
}
+static struct ssd1307fb_deviceinfo ssd1307fb_ssd1305_deviceinfo = {
+ .device_id = DEVID_SSD1305,
+ .default_vcomh = 0x34,
+ .default_dclk_div = 0,
+ .default_dclk_frq = 7,
+};
+
static struct ssd1307fb_deviceinfo ssd1307fb_ssd1306_deviceinfo = {
.device_id = DEVID_SSD1306,
.default_vcomh = 0x20,
@@ -443,6 +451,10 @@ static struct ssd1307fb_deviceinfo ssd1307fb_ssd1307_deviceinfo = {
static const struct of_device_id ssd1307fb_of_match[] = {
{
+ .compatible = "solomon,ssd1305fb-i2c",
+ .data = (void *)&ssd1307fb_ssd1305_deviceinfo,
+ },
+ {
.compatible = "solomon,ssd1306fb-i2c",
.data = (void *)&ssd1307fb_ssd1306_deviceinfo,
},
@@ -623,6 +635,7 @@ static int ssd1307fb_remove(struct i2c_client *client)
}
static const struct i2c_device_id ssd1307fb_i2c_id[] = {
+ { "ssd1305fb", 0 },
{ "ssd1306fb", 0 },
{ "ssd1307fb", 0 },
{ }
--
2.1.1
^ permalink raw reply related
* [PATCH 2/8] fbdev: ssd1307fb: Unify init code and make controller configurable from device tree
From: niederp @ 2015-02-06 22:28 UTC (permalink / raw)
To: linux-fbdev, plagnioj, tomi.valkeinen, maxime.ripard
Cc: linux-kernel, Thomas Niederprüm
In-Reply-To: <1423261694-5939-1-git-send-email-niederp@physik.uni-kl.de>
From: Thomas Niederprüm <niederp@physik.uni-kl.de>
This patches unifies the init code for the ssd130X chips and
adds device tree bindings to describe the hardware configuration
of the used controller. This gets rid of the magic bit values
used in the init code so far.
Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
---
.../devicetree/bindings/video/ssd1307fb.txt | 11 +
drivers/video/fbdev/ssd1307fb.c | 243 ++++++++++++++-------
2 files changed, 174 insertions(+), 80 deletions(-)
diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt b/Documentation/devicetree/bindings/video/ssd1307fb.txt
index 7a12542..1230f68 100644
--- a/Documentation/devicetree/bindings/video/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/video/ssd1307fb.txt
@@ -15,6 +15,17 @@ Required properties:
Optional properties:
- reset-active-low: Is the reset gpio is active on physical low?
+ - solomon,segment-remap: Invert the order of data column to segment mapping
+ - solomon,offset: Map the display start line to one of COM0 - COM63
+ - solomon,contrast: Set initial contrast of the display
+ - solomon,prechargep1: Set the duration of the precharge period phase1
+ - solomon,prechargep2: Set the duration of the precharge period phase2
+ - solomon,com-alt: Enable/disable alternate COM pin configuration
+ - solomon,com-lrremap: Enable/disable left-right remap of COM pins
+ - solomon,com-invdir: Invert COM scan direction
+ - solomon,vcomh: Set VCOMH regulator voltage
+ - solomon,dclk-div: Set display clock divider
+ - solomon,dclk-frq: Set display clock frequency
[0]: Documentation/devicetree/bindings/pwm/pwm.txt
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 3d6611f..4f435aa 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -16,6 +16,9 @@
#include <linux/pwm.h>
#include <linux/delay.h>
+#define DEVID_SSD1306 6
+#define DEVID_SSD1307 7
+
#define SSD1307FB_DATA 0x40
#define SSD1307FB_COMMAND 0x80
@@ -40,20 +43,33 @@
struct ssd1307fb_par;
-struct ssd1307fb_ops {
- int (*init)(struct ssd1307fb_par *);
- int (*remove)(struct ssd1307fb_par *);
+struct ssd1307fb_deviceinfo {
+ int device_id;
+ u32 default_vcomh;
+ u32 default_dclk_div;
+ u32 default_dclk_frq;
};
struct ssd1307fb_par {
+ u32 com_alt;
+ u32 com_invdir;
+ u32 com_lrremap;
+ u32 contrast;
+ u32 dclk_div;
+ u32 dclk_frq;
+ struct ssd1307fb_deviceinfo *device_info;
struct i2c_client *client;
u32 height;
struct fb_info *info;
- struct ssd1307fb_ops *ops;
+ u32 offset;
u32 page_offset;
+ u32 prechargep1;
+ u32 prechargep2;
struct pwm_device *pwm;
u32 pwm_period;
int reset;
+ u32 seg_remap;
+ u32 vcomh;
u32 width;
};
@@ -254,127 +270,151 @@ static struct fb_deferred_io ssd1307fb_defio = {
.deferred_io = ssd1307fb_deferred_io,
};
-static int ssd1307fb_ssd1307_init(struct ssd1307fb_par *par)
+static int ssd1307fb_init(struct ssd1307fb_par *par)
{
int ret;
+ u32 precharge, dclk, com_invdir, compins;
- par->pwm = pwm_get(&par->client->dev, NULL);
- if (IS_ERR(par->pwm)) {
- dev_err(&par->client->dev, "Could not get PWM from device tree!\n");
- return PTR_ERR(par->pwm);
- }
-
- par->pwm_period = pwm_get_period(par->pwm);
- /* Enable the PWM */
- pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
- pwm_enable(par->pwm);
-
- dev_dbg(&par->client->dev, "Using PWM%d with a %dns period.\n",
- par->pwm->pwm, par->pwm_period);
-
- /* Map column 127 of the OLED to segment 0 */
- ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SEG_REMAP_ON);
- if (ret < 0)
- return ret;
-
- /* Turn on the display */
- ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
- if (ret < 0)
- return ret;
-
- return 0;
-}
-
-static int ssd1307fb_ssd1307_remove(struct ssd1307fb_par *par)
-{
- pwm_disable(par->pwm);
- pwm_put(par->pwm);
- return 0;
-}
+ if (par->device_info->device_id = DEVID_SSD1307) {
+ par->pwm = pwm_get(&par->client->dev, NULL);
+ if (IS_ERR(par->pwm)) {
+ dev_err(&par->client->dev, "Could not get PWM from device tree!\n");
+ return PTR_ERR(par->pwm);
+ }
-static struct ssd1307fb_ops ssd1307fb_ssd1307_ops = {
- .init = ssd1307fb_ssd1307_init,
- .remove = ssd1307fb_ssd1307_remove,
-};
+ par->pwm_period = pwm_get_period(par->pwm);
+ /* Enable the PWM */
+ pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
+ pwm_enable(par->pwm);
-static int ssd1307fb_ssd1306_init(struct ssd1307fb_par *par)
-{
- int ret;
+ dev_dbg(&par->client->dev, "Using PWM%d with a %dns period.\n",
+ par->pwm->pwm, par->pwm_period);
+ };
/* Set initial contrast */
ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CONTRAST);
- ret = ret & ssd1307fb_write_cmd(par->client, 0x7f);
if (ret < 0)
return ret;
- /* Set COM direction */
- ret = ssd1307fb_write_cmd(par->client, 0xc8);
+ ret = ssd1307fb_write_cmd(par->client, par->contrast);
if (ret < 0)
return ret;
/* Set segment re-map */
- ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SEG_REMAP_ON);
+ if (par->seg_remap) {
+ ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SEG_REMAP_ON);
+ if (ret < 0)
+ return ret;
+ };
+
+ /* Set COM direction */
+ com_invdir = 0xc0 | (par->com_invdir & 0xf) << 3;
+ ret = ssd1307fb_write_cmd(par->client, com_invdir);
if (ret < 0)
return ret;
/* Set multiplex ratio value */
ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_MULTIPLEX_RATIO);
- ret = ret & ssd1307fb_write_cmd(par->client, par->height - 1);
+ if (ret < 0)
+ return ret;
+
+ ret = ssd1307fb_write_cmd(par->client, par->height - 1);
if (ret < 0)
return ret;
/* set display offset value */
ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_DISPLAY_OFFSET);
- ret = ssd1307fb_write_cmd(par->client, 0x20);
+ if (ret < 0)
+ return ret;
+
+ ret = ssd1307fb_write_cmd(par->client, par->offset);
if (ret < 0)
return ret;
/* Set clock frequency */
+ dclk = (par->dclk_div & 0xf) | (par->dclk_frq & 0xf) << 4;
ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_CLOCK_FREQ);
- ret = ret & ssd1307fb_write_cmd(par->client, 0xf0);
if (ret < 0)
return ret;
- /* Set precharge period in number of ticks from the internal clock */
+ ret = ssd1307fb_write_cmd(par->client, dclk);
+ if (ret < 0)
+ return ret;
+
+ /* Set precharge period in number of ticks from the internal clock*/
+ precharge = (par->prechargep1 & 0xf) | (par->prechargep2 & 0xf) << 4;
ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_PRECHARGE_PERIOD);
- ret = ret & ssd1307fb_write_cmd(par->client, 0x22);
+ if (ret < 0)
+ return ret;
+
+ ret = ssd1307fb_write_cmd(par->client, precharge);
if (ret < 0)
return ret;
/* Set COM pins configuration */
+ compins = 0x02 | (par->com_alt & 0x1) << 4
+ | (par->com_lrremap & 0x1) << 5;
ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_COM_PINS_CONFIG);
- ret = ret & ssd1307fb_write_cmd(par->client, 0x22);
+ if (ret < 0)
+ return ret;
+
+ ret = ssd1307fb_write_cmd(par->client, compins);
if (ret < 0)
return ret;
/* Set VCOMH */
ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_VCOMH);
- ret = ret & ssd1307fb_write_cmd(par->client, 0x49);
if (ret < 0)
return ret;
- /* Turn on the DC-DC Charge Pump */
- ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CHARGE_PUMP);
- ret = ret & ssd1307fb_write_cmd(par->client, 0x14);
+ ret = ssd1307fb_write_cmd(par->client, par->vcomh);
if (ret < 0)
return ret;
+ if (par->device_info->device_id = DEVID_SSD1306) {
+ /* Turn on the DC-DC Charge Pump */
+ ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CHARGE_PUMP);
+ if (ret < 0)
+ return ret;
+
+ ret = ssd1307fb_write_cmd(par->client, 0x14);
+ if (ret < 0)
+ return ret;
+ };
+
/* Switch to horizontal addressing mode */
ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_ADDRESS_MODE);
- ret = ret & ssd1307fb_write_cmd(par->client,
+ if (ret < 0)
+ return ret;
+
+ ret = ssd1307fb_write_cmd(par->client,
SSD1307FB_SET_ADDRESS_MODE_HORIZONTAL);
if (ret < 0)
return ret;
+ /* Set column range */
ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_COL_RANGE);
- ret = ret & ssd1307fb_write_cmd(par->client, 0x0);
- ret = ret & ssd1307fb_write_cmd(par->client, par->width - 1);
if (ret < 0)
return ret;
+ ret = ssd1307fb_write_cmd(par->client, 0x0);
+ if (ret < 0)
+ return ret;
+
+ ret = ssd1307fb_write_cmd(par->client, par->width - 1);
+ if (ret < 0)
+ return ret;
+
+ /* Set page range */
ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_PAGE_RANGE);
- ret = ret & ssd1307fb_write_cmd(par->client, 0x0);
- ret = ret & ssd1307fb_write_cmd(par->client,
+ if (ret < 0)
+ return ret;
+
+ ret = ssd1307fb_write_cmd(par->client, 0x0);
+ if (ret < 0)
+ return ret;
+
+ ret = ssd1307fb_write_cmd(par->client,
par->page_offset + (par->height / 8) - 1);
if (ret < 0)
return ret;
@@ -387,18 +427,28 @@ static int ssd1307fb_ssd1306_init(struct ssd1307fb_par *par)
return 0;
}
-static struct ssd1307fb_ops ssd1307fb_ssd1306_ops = {
- .init = ssd1307fb_ssd1306_init,
+static struct ssd1307fb_deviceinfo ssd1307fb_ssd1306_deviceinfo = {
+ .device_id = DEVID_SSD1306,
+ .default_vcomh = 0x20,
+ .default_dclk_div = 0,
+ .default_dclk_frq = 8,
+};
+
+static struct ssd1307fb_deviceinfo ssd1307fb_ssd1307_deviceinfo = {
+ .device_id = DEVID_SSD1307,
+ .default_vcomh = 0x20,
+ .default_dclk_div = 1,
+ .default_dclk_frq = 12,
};
static const struct of_device_id ssd1307fb_of_match[] = {
{
.compatible = "solomon,ssd1306fb-i2c",
- .data = (void *)&ssd1307fb_ssd1306_ops,
+ .data = (void *)&ssd1307fb_ssd1306_deviceinfo,
},
{
.compatible = "solomon,ssd1307fb-i2c",
- .data = (void *)&ssd1307fb_ssd1307_ops,
+ .data = (void *)&ssd1307fb_ssd1307_deviceinfo,
},
{},
};
@@ -429,8 +479,8 @@ static int ssd1307fb_probe(struct i2c_client *client,
par->info = info;
par->client = client;
- par->ops = (struct ssd1307fb_ops *)of_match_device(ssd1307fb_of_match,
- &client->dev)->data;
+ par->device_info = (struct ssd1307fb_deviceinfo *)of_match_device(
+ ssd1307fb_of_match, &client->dev)->data;
par->reset = of_get_named_gpio(client->dev.of_node,
"reset-gpios", 0);
@@ -449,8 +499,40 @@ static int ssd1307fb_probe(struct i2c_client *client,
par->page_offset = 1;
vmem_size = par->width * par->height / 8;
+ if (of_property_read_u32(node, "solomon,segment-remap", &par->seg_remap))
+ par->seg_remap = 0;
+
+ if (of_property_read_u32(node, "solomon,offset", &par->offset))
+ par->offset = 0;
+
+ if (of_property_read_u32(node, "solomon,contrast", &par->contrast))
+ par->contrast = 128;
+
+ if (of_property_read_u32(node, "solomon,prechargep1", &par->prechargep1))
+ par->prechargep1 = 2;
+
+ if (of_property_read_u32(node, "solomon,prechargep2", &par->prechargep2))
+ par->prechargep2 = 2;
+
+ if (of_property_read_u32(node, "solomon,com-alt", &par->com_alt))
+ par->com_alt = 1;
+
+ if (of_property_read_u32(node, "solomon,com-lrremap", &par->com_lrremap))
+ par->com_lrremap = 0;
+
+ if (of_property_read_u32(node, "solomon,com-invdir", &par->com_invdir))
+ par->com_invdir = 0;
+
+ if (of_property_read_u32(node, "solomon,vcomh", &par->vcomh))
+ par->vcomh = par->device_info->default_vcomh;
vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
+ if (of_property_read_u32(node, "solomon,dclk-div", &par->dclk_div))
+ par->dclk_div = par->device_info->default_dclk_div;
+
+ if (of_property_read_u32(node, "solomon,dclk-frq", &par->dclk_frq))
+ par->dclk_frq = par->device_info->default_dclk_frq;
+
if (!vmem) {
dev_err(&client->dev, "Couldn't allocate graphical memory.\n");
ret = -ENOMEM;
@@ -499,11 +581,9 @@ static int ssd1307fb_probe(struct i2c_client *client,
gpio_set_value(par->reset, 1);
udelay(4);
- if (par->ops->init) {
- ret = par->ops->init(par);
- if (ret)
- goto reset_oled_error;
- }
+ ret = ssd1307fb_init(par);
+ if (ret)
+ goto reset_oled_error;
ret = register_framebuffer(info);
if (ret) {
@@ -516,8 +596,10 @@ static int ssd1307fb_probe(struct i2c_client *client,
return 0;
panel_init_error:
- if (par->ops->remove)
- par->ops->remove(par);
+ if (par->device_info->device_id = DEVID_SSD1307) {
+ pwm_disable(par->pwm);
+ pwm_put(par->pwm);
+ };
reset_oled_error:
fb_deferred_io_cleanup(info);
fb_alloc_error:
@@ -531,11 +613,12 @@ static int ssd1307fb_remove(struct i2c_client *client)
struct ssd1307fb_par *par = info->par;
unregister_framebuffer(info);
- if (par->ops->remove)
- par->ops->remove(par);
+ if (par->device_info->device_id = DEVID_SSD1307) {
+ pwm_disable(par->pwm);
+ pwm_put(par->pwm);
+ };
fb_deferred_io_cleanup(info);
framebuffer_release(info);
-
return 0;
}
--
2.1.1
^ permalink raw reply related
* [PATCH 1/8] Documentation: dts: add missing Solomon Systech vendor prefix.
From: niederp @ 2015-02-06 22:28 UTC (permalink / raw)
To: linux-fbdev, plagnioj, tomi.valkeinen, maxime.ripard
Cc: linux-kernel, Thomas Niederprüm
In-Reply-To: <1423261694-5939-1-git-send-email-niederp@physik.uni-kl.de>
From: Thomas Niederprüm <niederp@physik.uni-kl.de>
This patch adds the solomon prefix for Solomon Systech Limited.
Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
---
Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index a344ec2..b1d9470 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -143,6 +143,7 @@ sitronix Sitronix Technology Corporation
smsc Standard Microsystems Corporation
snps Synopsys, Inc.
solidrun SolidRun
+solomon Solomon Systech Limited
sony Sony Corporation
spansion Spansion Inc.
st STMicroelectronics
--
2.1.1
^ permalink raw reply related
* [PATCH 0/8] Cleanup and add support for SSD1305
From: niederp @ 2015-02-06 22:28 UTC (permalink / raw)
To: linux-fbdev, plagnioj, tomi.valkeinen, maxime.ripard
Cc: linux-kernel, Thomas Niederprüm
From: Thomas Niederprüm <niederp@physik.uni-kl.de>
This patch series is the result of making the ssd1307fb driver work with
a Newhaven OLED display using the Solomon SSD1305 controller. To achieve
this the intialization code for the SSD1306 and the SSD1307 is merged
and based on device tree configuration. This gets rid of the magic bit
values that were used so far.
Based on these changes it was straight forward to add support for the
SSD1305 controller.
While working with the driver I realized that it was not possible to
correctly mmap the video memory from userspace since the memory
reserved by kzalloc is not page aligned. This problem is fixed by
using vmalloc as it is done inthe vfb driver.
Furthermore module parameters are added to set the bits per pixel
and the delay for the deferred io update. It makes sense to set
the bits per pixel for the video memory to 8 bits since there is
only very poor userspace support for 1 bit framebuffers.
Also sysfs handles are added to make the contrast settings and dim
mode setting available in userspace.
Thomas Niederprüm (8):
Documentation: dts: add missing Solomon Systech vendor prefix.
fbdev: ssd1307fb: Unify init code and make controller configurable
from device tree
fbdev: ssd1307fb: Add support for SSD1305
fbdev: ssd1307fb: Use vmalloc to allocate video memory.
fbdev: ssd1307fb: Add module parameter bitsperpixel.
fbdev: ssd1307fb: Add module parameter to set update delay of the
deffered io.
fbdev: ssd1307fb: Add sysfs handles to expose contrast and dim setting
to userspace.
fbdev: ssd1307fb: Turn off display on driver unload.
.../devicetree/bindings/vendor-prefixes.txt | 1 +
.../devicetree/bindings/video/ssd1307fb.txt | 13 +-
drivers/video/fbdev/ssd1307fb.c | 426 ++++++++++++++++-----
3 files changed, 346 insertions(+), 94 deletions(-)
--
2.1.1
^ permalink raw reply
* Re: [PATCH RFC] video: fbdev: sis: condition with no effect
From: Thomas Winischhofer @ 2015-02-06 14:32 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: Tormod Volden, Scot Doyle, Nicholas Mc Guire,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel
In-Reply-To: <20150206134726.GB32696@opentech.at>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Nicholas Mc Guire wrote:
> On Fri, 06 Feb 2015, Thomas Winischhofer wrote:
>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Tormod Volden wrote:
>>> On Thu, Feb 5, 2015 at 9:45 PM, Scot Doyle wrote:
>>>> On Wed, 4 Feb 2015, Nicholas Mc Guire wrote:
>>>>> The if and the else branch code are identical - so the condition has no
>>>>> effect on the effective code - this patch removes the condition and the
>>>>> duplicated code.
>>>>>
>>>>> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
>>>>> ---
>>>>>
>>>>> This code has been in here since commit 544393fe584d ("sisfb update") so I guess it is
>>>>> safe to simply remove the duplicated code if nobody noticed for 10 years.
>>>>>
>>>>> Note that the code is not really CodingStyle compliant - the lines inserted were formatted
>>>>> to satisfy the coding style but I'm unsure if it is not better to leave it in the
>>>>> old format.
>>>>>
>>>>> Patch was only compile tested with x86_64_defconfig +
>>>>> CONFIG_FB_SIS=m, CONFIG_FB_SIS_300=y, CONFIG_FB_SIS_315=y
>>>>>
>>>>> Patch is against 3.19.0-rc7 (localversion-next is -next-20150204)
>>>>>
>>>>> drivers/video/fbdev/sis/init301.c | 9 ++-------
>>>>> 1 file changed, 2 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/video/fbdev/sis/init301.c b/drivers/video/fbdev/sis/init301.c
>>>>> index 295e0de..9533a8ab 100644
>>>>> --- a/drivers/video/fbdev/sis/init301.c
>>>>> +++ b/drivers/video/fbdev/sis/init301.c
>>>>> @@ -7971,13 +7971,8 @@ SiS_SetCHTVReg(struct SiS_Private *SiS_Pr, unsigned short ModeNo, unsigned short
>>>>> }
>>>>> } else { /* ---- PAL ---- */
>>>>> /* We don't play around with FSCI in PAL mode */
>>>>> - if(resindex = 0x04) {
>>>>> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
>>>>> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
>>>>> - } else {
>>>>> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
>>>>> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
>>>>> - }
>>>>> + SiS_SetCH70xxANDOR(SiS_Pr, 0x20, 0x00, 0xEF); /* loop filter off */
>>>>> + SiS_SetCH70xxANDOR(SiS_Pr, 0x21, 0x01, 0xFE); /* ACIV on */
>>>>> }
>>>>>
>>>>> #endif /* 300 */
>>>> The code covering the PAL case had this redundancy when it was introduced
>>>> in Linux 2.4.19.
>>>>
>>>> Lines 7934-7981 consider three variables: PAL, overscan, and resindex.
>>>> Given the "#ifdef 0" block, couldn't the current six sections collapse
>>>> into two? One for (!PAL && overscan && resindex=5) and another for the
>>>> rest?
>>> Are we sure there isn't a typo in one of the duplicate clauses? Or
>>> wrong copy-pasting? Generally I am skeptical to "fixing" code without
>>> understanding what is behind or testing it, and just cosmetically
>>> brush over it. For now at least it is obvious that there is something
>>> wrong. In case (although an unlikely one) someone who understands the
>>> code and knows this chip comes along, he would quickly spot this.
>>> After your "fixups" this will be all forgotten. Additionally it adds
>>> to the impression that this code is being maintained, which is wrong.
>>>
>>> I would understand an argument about annoying compiler warnings and
>>> the like, but in that case I would prefer to #if 0 it instead of
>>> "prettifying" it.
>>>
>>> 0.02
>>> Tormod
>>>
>> The code is partly unfinished due to a lack of hardware to test this
>> with. SiS announced SiS+Chrontel 7019 combos at some point but I have
>> never seen one. The code was written based on the Chrontel datasheets,
>> which weren't clear to some extent, and there wasn't ever any test
>> hardware. I don't recall this one exactly, but identical if-else
>> statements mean that one alternative is (assumingly) correct, while the
>> other is uncertain and/or untested. I left such redunant if-statements
>> in the code to remember the conditions and the fact that there is a
>> second alternative.
>>
>> Considering the long time I'd say it's safe to simplify this.
>>
>> A word on other changes I monitored recently: Please bear in mind that
>> with video hardware reading and writing registers is not simple like
>> reading and writing to memory. Sometimes reading causes an effect in the
>> hardware as well (latches, etc), so removing seemingly redundant
>> GetReg/SetReg sequences might actually have an effect.
>>
> thanks for that note - will add that to my checklist of sideffects
> for future patches.
>
> thx!
> hofrat
>
PS: Correction: This code is for the SiS+Chrontel 7005 (not 7019) case,
and there actually WAS hardware. Therefore I also probably tested this,
or this is the remains of a test, and as a consequence it is safe to
simplify/remove.
- --
Thomas Winischhofer
thomas AT winischhofer DOT net
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
iD8DBQFU1NCFzydIRAktyUcRAkN4AJ9CqUVCbEKyUkSOPvCkRWzKDeaPPQCfdQ4e
ffzCiVCH5Ul7kAXiL/K0RDU=tt8c
-----END PGP SIGNATURE-----
^ permalink raw reply
* [PATCH] fbdev: aty: remove some unneeded variables
From: Sudip Mukherjee @ 2015-02-06 14:29 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Benjamin Herrenschmidt
Cc: Sudip Mukherjee, linux-fbdev, linux-kernel
mach64RefFreq, hSyncPol, vSyncPol, cSync and bytpp were only being
assigned some values but were never used after that.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/video/fbdev/aty/mach64_gx.c | 3 +--
drivers/video/fbdev/aty/radeon_base.c | 10 ++--------
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/video/fbdev/aty/mach64_gx.c b/drivers/video/fbdev/aty/mach64_gx.c
index 10c988a..fcb51b1 100644
--- a/drivers/video/fbdev/aty/mach64_gx.c
+++ b/drivers/video/fbdev/aty/mach64_gx.c
@@ -617,14 +617,13 @@ static int aty_var_to_pll_8398(const struct fb_info *info, u32 vclk_per,
u32 mhz100; /* in 0.01 MHz */
u32 program_bits;
/* u32 post_divider; */
- u32 mach64MinFreq, mach64MaxFreq, mach64RefFreq;
+ u32 mach64MinFreq, mach64MaxFreq;
u16 m, n, k = 0, save_m, save_n, twoToKth;
/* Calculate the programming word */
mhz100 = 100000000 / vclk_per;
mach64MinFreq = MIN_FREQ_2595;
mach64MaxFreq = MAX_FREQ_2595;
- mach64RefFreq = REF_FREQ_2595; /* 14.32 MHz */
save_m = 0;
save_n = 0;
diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
index 26d80a4..aee65ba 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -1532,14 +1532,14 @@ static int radeonfb_set_par(struct fb_info *info)
struct fb_var_screeninfo *mode = &info->var;
struct radeon_regs *newmode;
int hTotal, vTotal, hSyncStart, hSyncEnd,
- hSyncPol, vSyncStart, vSyncEnd, vSyncPol, cSync;
+ vSyncStart, vSyncEnd;
u8 hsync_adj_tab[] = {0, 0x12, 9, 9, 6, 5};
u8 hsync_fudge_fp[] = {2, 2, 0, 0, 5, 5};
u32 sync, h_sync_pol, v_sync_pol, dotClock, pixClock;
int i, freq;
int format = 0;
int nopllcalc = 0;
- int hsync_start, hsync_fudge, bytpp, hsync_wid, vsync_wid;
+ int hsync_start, hsync_fudge, hsync_wid, vsync_wid;
int primary_mon = PRIMARY_MONITOR(rinfo);
int depth = var_to_depth(mode);
int use_rmx = 0;
@@ -1612,13 +1612,7 @@ static int radeonfb_set_par(struct fb_info *info)
else if (vsync_wid > 0x1f) /* max */
vsync_wid = 0x1f;
- hSyncPol = mode->sync & FB_SYNC_HOR_HIGH_ACT ? 0 : 1;
- vSyncPol = mode->sync & FB_SYNC_VERT_HIGH_ACT ? 0 : 1;
-
- cSync = mode->sync & FB_SYNC_COMP_HIGH_ACT ? (1 << 4) : 0;
-
format = radeon_get_dstbpp(depth);
- bytpp = mode->bits_per_pixel >> 3;
if ((primary_mon = MT_DFP) || (primary_mon = MT_LCD))
hsync_fudge = hsync_fudge_fp[format-1];
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH RFC] video: fbdev: sis: condition with no effect
From: Nicholas Mc Guire @ 2015-02-06 13:47 UTC (permalink / raw)
To: Thomas Winischhofer
Cc: Tormod Volden, Scot Doyle, Nicholas Mc Guire,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel
In-Reply-To: <54D46694.7050108@winischhofer.net>
On Fri, 06 Feb 2015, Thomas Winischhofer wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Tormod Volden wrote:
> > On Thu, Feb 5, 2015 at 9:45 PM, Scot Doyle wrote:
> >> On Wed, 4 Feb 2015, Nicholas Mc Guire wrote:
> >>> The if and the else branch code are identical - so the condition has no
> >>> effect on the effective code - this patch removes the condition and the
> >>> duplicated code.
> >>>
> >>> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> >>> ---
> >>>
> >>> This code has been in here since commit 544393fe584d ("sisfb update") so I guess it is
> >>> safe to simply remove the duplicated code if nobody noticed for 10 years.
> >>>
> >>> Note that the code is not really CodingStyle compliant - the lines inserted were formatted
> >>> to satisfy the coding style but I'm unsure if it is not better to leave it in the
> >>> old format.
> >>>
> >>> Patch was only compile tested with x86_64_defconfig +
> >>> CONFIG_FB_SIS=m, CONFIG_FB_SIS_300=y, CONFIG_FB_SIS_315=y
> >>>
> >>> Patch is against 3.19.0-rc7 (localversion-next is -next-20150204)
> >>>
> >>> drivers/video/fbdev/sis/init301.c | 9 ++-------
> >>> 1 file changed, 2 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/drivers/video/fbdev/sis/init301.c b/drivers/video/fbdev/sis/init301.c
> >>> index 295e0de..9533a8ab 100644
> >>> --- a/drivers/video/fbdev/sis/init301.c
> >>> +++ b/drivers/video/fbdev/sis/init301.c
> >>> @@ -7971,13 +7971,8 @@ SiS_SetCHTVReg(struct SiS_Private *SiS_Pr, unsigned short ModeNo, unsigned short
> >>> }
> >>> } else { /* ---- PAL ---- */
> >>> /* We don't play around with FSCI in PAL mode */
> >>> - if(resindex = 0x04) {
> >>> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
> >>> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
> >>> - } else {
> >>> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
> >>> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
> >>> - }
> >>> + SiS_SetCH70xxANDOR(SiS_Pr, 0x20, 0x00, 0xEF); /* loop filter off */
> >>> + SiS_SetCH70xxANDOR(SiS_Pr, 0x21, 0x01, 0xFE); /* ACIV on */
> >>> }
> >>>
> >>> #endif /* 300 */
> >> The code covering the PAL case had this redundancy when it was introduced
> >> in Linux 2.4.19.
> >>
> >> Lines 7934-7981 consider three variables: PAL, overscan, and resindex.
> >> Given the "#ifdef 0" block, couldn't the current six sections collapse
> >> into two? One for (!PAL && overscan && resindex=5) and another for the
> >> rest?
> >
> > Are we sure there isn't a typo in one of the duplicate clauses? Or
> > wrong copy-pasting? Generally I am skeptical to "fixing" code without
> > understanding what is behind or testing it, and just cosmetically
> > brush over it. For now at least it is obvious that there is something
> > wrong. In case (although an unlikely one) someone who understands the
> > code and knows this chip comes along, he would quickly spot this.
> > After your "fixups" this will be all forgotten. Additionally it adds
> > to the impression that this code is being maintained, which is wrong.
> >
> > I would understand an argument about annoying compiler warnings and
> > the like, but in that case I would prefer to #if 0 it instead of
> > "prettifying" it.
> >
> > 0.02
> > Tormod
> >
>
> The code is partly unfinished due to a lack of hardware to test this
> with. SiS announced SiS+Chrontel 7019 combos at some point but I have
> never seen one. The code was written based on the Chrontel datasheets,
> which weren't clear to some extent, and there wasn't ever any test
> hardware. I don't recall this one exactly, but identical if-else
> statements mean that one alternative is (assumingly) correct, while the
> other is uncertain and/or untested. I left such redunant if-statements
> in the code to remember the conditions and the fact that there is a
> second alternative.
>
> Considering the long time I'd say it's safe to simplify this.
>
> A word on other changes I monitored recently: Please bear in mind that
> with video hardware reading and writing registers is not simple like
> reading and writing to memory. Sometimes reading causes an effect in the
> hardware as well (latches, etc), so removing seemingly redundant
> GetReg/SetReg sequences might actually have an effect.
>
thanks for that note - will add that to my checklist of sideffects
for future patches.
thx!
hofrat
^ permalink raw reply
* Re: [PATCH RFC] video: fbdev: sis: condition with no effect
From: Nicholas Mc Guire @ 2015-02-06 13:43 UTC (permalink / raw)
To: Tormod Volden
Cc: Scot Doyle, Nicholas Mc Guire, Thomas Winischhofer,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel
In-Reply-To: <CAArsGaaTfkYcmL2tbxdbv_Jjwuj6HLE3URxHpq4-ju8wK-Ga7w@mail.gmail.com>
On Thu, 05 Feb 2015, Tormod Volden wrote:
> On Thu, Feb 5, 2015 at 9:45 PM, Scot Doyle wrote:
> > On Wed, 4 Feb 2015, Nicholas Mc Guire wrote:
> >> The if and the else branch code are identical - so the condition has no
> >> effect on the effective code - this patch removes the condition and the
> >> duplicated code.
> >>
> >> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> >> ---
> >>
> >> This code has been in here since commit 544393fe584d ("sisfb update") so I guess it is
> >> safe to simply remove the duplicated code if nobody noticed for 10 years.
> >>
> >> Note that the code is not really CodingStyle compliant - the lines inserted were formatted
> >> to satisfy the coding style but I'm unsure if it is not better to leave it in the
> >> old format.
> >>
> >> Patch was only compile tested with x86_64_defconfig +
> >> CONFIG_FB_SIS=m, CONFIG_FB_SIS_300=y, CONFIG_FB_SIS_315=y
> >>
> >> Patch is against 3.19.0-rc7 (localversion-next is -next-20150204)
> >>
> >> drivers/video/fbdev/sis/init301.c | 9 ++-------
> >> 1 file changed, 2 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/video/fbdev/sis/init301.c b/drivers/video/fbdev/sis/init301.c
> >> index 295e0de..9533a8ab 100644
> >> --- a/drivers/video/fbdev/sis/init301.c
> >> +++ b/drivers/video/fbdev/sis/init301.c
> >> @@ -7971,13 +7971,8 @@ SiS_SetCHTVReg(struct SiS_Private *SiS_Pr, unsigned short ModeNo, unsigned short
> >> }
> >> } else { /* ---- PAL ---- */
> >> /* We don't play around with FSCI in PAL mode */
> >> - if(resindex = 0x04) {
> >> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
> >> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
> >> - } else {
> >> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
> >> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
> >> - }
> >> + SiS_SetCH70xxANDOR(SiS_Pr, 0x20, 0x00, 0xEF); /* loop filter off */
> >> + SiS_SetCH70xxANDOR(SiS_Pr, 0x21, 0x01, 0xFE); /* ACIV on */
> >> }
> >>
> >> #endif /* 300 */
> >
> > The code covering the PAL case had this redundancy when it was introduced
> > in Linux 2.4.19.
> >
> > Lines 7934-7981 consider three variables: PAL, overscan, and resindex.
> > Given the "#ifdef 0" block, couldn't the current six sections collapse
> > into two? One for (!PAL && overscan && resindex=5) and another for the
> > rest?
>
> Are we sure there isn't a typo in one of the duplicate clauses? Or
> wrong copy-pasting? Generally I am skeptical to "fixing" code without
> understanding what is behind or testing it, and just cosmetically
> brush over it. For now at least it is obvious that there is something
> wrong. In case (although an unlikely one) someone who understands the
> code and knows this chip comes along, he would quickly spot this.
> After your "fixups" this will be all forgotten. Additionally it adds
> to the impression that this code is being maintained, which is wrong.
>
> I would understand an argument about annoying compiler warnings and
> the like, but in that case I would prefer to #if 0 it instead of
> "prettifying" it.
>
Its actually a static code checker that is fussing at this.
The #if 0 case is on my list as well - but thats a different
scanner - and thus goes into a separate patch.
I agree that it could be a hidden bug - but given that its this
way for 10 years I doubt this.
thx!
hofrat
^ permalink raw reply
* [PATCH] video: fbdev: use msecs_to_jiffies for time conversion
From: Nicholas Mc Guire @ 2015-02-06 9:14 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard
Cc: Tomi Valkeinen, Jingoo Han, Daniel Vetter, Fabian Frederick,
Laurent Pinchart, Nicholas Mc Guire, Wolfram Sang, linux-fbdev,
linux-kernel
This is only an API consolidation and should make things more readable
it replaces var * HZ / 1000 by msecs_to_jiffies(var).
Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---
Patch was only compile tested with viper_defconfig (implies CONFIG_FB_PXA=m)
Patch is against 3.19.0-rc7 (localversion-next is -next-20150204)
drivers/video/fbdev/pxafb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index da2431e..d8b4743 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c
@@ -1285,7 +1285,7 @@ static int pxafb_smart_thread(void *arg)
mutex_unlock(&fbi->ctrlr_lock);
set_current_state(TASK_INTERRUPTIBLE);
- schedule_timeout(30 * HZ / 1000);
+ schedule_timeout(msecs_to_jiffies(30));
}
pr_debug("%s(): task ending\n", __func__);
@@ -1460,7 +1460,7 @@ static void pxafb_disable_controller(struct pxafb_info *fbi)
#ifdef CONFIG_FB_PXA_SMARTPANEL
if (fbi->lccr0 & LCCR0_LCDT) {
wait_for_completion_timeout(&fbi->refresh_done,
- 200 * HZ / 1000);
+ msecs_to_jiffies(200);
return;
}
#endif
@@ -1472,7 +1472,7 @@ static void pxafb_disable_controller(struct pxafb_info *fbi)
lcd_writel(fbi, LCCR0, lccr0);
lcd_writel(fbi, LCCR0, lccr0 | LCCR0_DIS);
- wait_for_completion_timeout(&fbi->disable_done, 200 * HZ / 1000);
+ wait_for_completion_timeout(&fbi->disable_done, msecs_to_jiffies(200));
/* disable LCD controller clock */
clk_disable_unprepare(fbi->clk);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH RFC] video: fbdev: sis: condition with no effect
From: Thomas Winischhofer @ 2015-02-06 7:00 UTC (permalink / raw)
To: Tormod Volden
Cc: Scot Doyle, Nicholas Mc Guire, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <CAArsGaaTfkYcmL2tbxdbv_Jjwuj6HLE3URxHpq4-ju8wK-Ga7w@mail.gmail.com>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Tormod Volden wrote:
> On Thu, Feb 5, 2015 at 9:45 PM, Scot Doyle wrote:
>> On Wed, 4 Feb 2015, Nicholas Mc Guire wrote:
>>> The if and the else branch code are identical - so the condition has no
>>> effect on the effective code - this patch removes the condition and the
>>> duplicated code.
>>>
>>> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
>>> ---
>>>
>>> This code has been in here since commit 544393fe584d ("sisfb update") so I guess it is
>>> safe to simply remove the duplicated code if nobody noticed for 10 years.
>>>
>>> Note that the code is not really CodingStyle compliant - the lines inserted were formatted
>>> to satisfy the coding style but I'm unsure if it is not better to leave it in the
>>> old format.
>>>
>>> Patch was only compile tested with x86_64_defconfig +
>>> CONFIG_FB_SIS=m, CONFIG_FB_SIS_300=y, CONFIG_FB_SIS_315=y
>>>
>>> Patch is against 3.19.0-rc7 (localversion-next is -next-20150204)
>>>
>>> drivers/video/fbdev/sis/init301.c | 9 ++-------
>>> 1 file changed, 2 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/video/fbdev/sis/init301.c b/drivers/video/fbdev/sis/init301.c
>>> index 295e0de..9533a8ab 100644
>>> --- a/drivers/video/fbdev/sis/init301.c
>>> +++ b/drivers/video/fbdev/sis/init301.c
>>> @@ -7971,13 +7971,8 @@ SiS_SetCHTVReg(struct SiS_Private *SiS_Pr, unsigned short ModeNo, unsigned short
>>> }
>>> } else { /* ---- PAL ---- */
>>> /* We don't play around with FSCI in PAL mode */
>>> - if(resindex = 0x04) {
>>> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
>>> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
>>> - } else {
>>> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
>>> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
>>> - }
>>> + SiS_SetCH70xxANDOR(SiS_Pr, 0x20, 0x00, 0xEF); /* loop filter off */
>>> + SiS_SetCH70xxANDOR(SiS_Pr, 0x21, 0x01, 0xFE); /* ACIV on */
>>> }
>>>
>>> #endif /* 300 */
>> The code covering the PAL case had this redundancy when it was introduced
>> in Linux 2.4.19.
>>
>> Lines 7934-7981 consider three variables: PAL, overscan, and resindex.
>> Given the "#ifdef 0" block, couldn't the current six sections collapse
>> into two? One for (!PAL && overscan && resindex=5) and another for the
>> rest?
>
> Are we sure there isn't a typo in one of the duplicate clauses? Or
> wrong copy-pasting? Generally I am skeptical to "fixing" code without
> understanding what is behind or testing it, and just cosmetically
> brush over it. For now at least it is obvious that there is something
> wrong. In case (although an unlikely one) someone who understands the
> code and knows this chip comes along, he would quickly spot this.
> After your "fixups" this will be all forgotten. Additionally it adds
> to the impression that this code is being maintained, which is wrong.
>
> I would understand an argument about annoying compiler warnings and
> the like, but in that case I would prefer to #if 0 it instead of
> "prettifying" it.
>
> 0.02
> Tormod
>
The code is partly unfinished due to a lack of hardware to test this
with. SiS announced SiS+Chrontel 7019 combos at some point but I have
never seen one. The code was written based on the Chrontel datasheets,
which weren't clear to some extent, and there wasn't ever any test
hardware. I don't recall this one exactly, but identical if-else
statements mean that one alternative is (assumingly) correct, while the
other is uncertain and/or untested. I left such redunant if-statements
in the code to remember the conditions and the fact that there is a
second alternative.
Considering the long time I'd say it's safe to simplify this.
A word on other changes I monitored recently: Please bear in mind that
with video hardware reading and writing registers is not simple like
reading and writing to memory. Sometimes reading causes an effect in the
hardware as well (latches, etc), so removing seemingly redundant
GetReg/SetReg sequences might actually have an effect.
Regards
Thomas
- --
Thomas Winischhofer
thomas AT winischhofer DOT net
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
iD8DBQFU1GaUzydIRAktyUcRAuQlAJ9NL3moUDf0yUMbE9qi4L26hT69NwCcDOk2
GyZjN8fic9bITTtdK9OG0R8=s9pA
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: linux-next for fbdev
From: Sudip Mukherjee @ 2015-02-06 4:44 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <54D38B76.5070005@ti.com>
On Thu, Feb 05, 2015 at 05:25:42PM +0200, Tomi Valkeinen wrote:
> Hi,
>
> On 05/02/15 17:20, Sudip Mukherjee wrote:
> > Hi,
> > I have noticed that we do not have anything called fbdev-next so patches to fbdev does not appear in linux-next, but instead waits for the merge window. That may be because you might not have enough time to maintain a separate tree or any other reason.
> > Can i help in any way to have a fbdev-next tree? I can have a tree in my server, where i can apply the patches that you are queuing for the next merge window (usually you will send reply to the list that it is queued). And that tree can act as fbdev-next.
> >
> > This was just an idea, and but I am willing to help you in any way to have a fbdev-next.
>
> I have 'for-next' branch in my tree
> git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git which gets
> pulled into linux-next, and that's where I push fbdev patches.
>
The reason for my mail Stephen in his mail "linux-next: tidy up of trees" mentioned that fbdev is not getting updated. But now when I am checking i am seeing my fbdev patches in linux-next. I should have checked before sending you the mail.
Sorry for the noise.
Sudip
> Tomi
>
>
^ permalink raw reply
* Re: [PATCH RFC] video: fbdev: sis: condition with no effect
From: Scot Doyle @ 2015-02-05 23:37 UTC (permalink / raw)
To: Tormod Volden
Cc: Nicholas Mc Guire, Thomas Winischhofer,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel
In-Reply-To: <CAArsGaaTfkYcmL2tbxdbv_Jjwuj6HLE3URxHpq4-ju8wK-Ga7w@mail.gmail.com>
On Thu, 5 Feb 2015, Tormod Volden wrote:
> On Thu, Feb 5, 2015 at 9:45 PM, Scot Doyle wrote:
> > On Wed, 4 Feb 2015, Nicholas Mc Guire wrote:
> >> The if and the else branch code are identical - so the condition has no
> >> effect on the effective code - this patch removes the condition and the
> >> duplicated code.
> >>
> >> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> >> ---
> >>
> >> This code has been in here since commit 544393fe584d ("sisfb update") so I guess it is
> >> safe to simply remove the duplicated code if nobody noticed for 10 years.
> >>
> >> Note that the code is not really CodingStyle compliant - the lines inserted were formatted
> >> to satisfy the coding style but I'm unsure if it is not better to leave it in the
> >> old format.
> >>
> >> Patch was only compile tested with x86_64_defconfig +
> >> CONFIG_FB_SIS=m, CONFIG_FB_SIS_300=y, CONFIG_FB_SIS_315=y
> >>
> >> Patch is against 3.19.0-rc7 (localversion-next is -next-20150204)
> >>
> >> drivers/video/fbdev/sis/init301.c | 9 ++-------
> >> 1 file changed, 2 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/video/fbdev/sis/init301.c b/drivers/video/fbdev/sis/init301.c
> >> index 295e0de..9533a8ab 100644
> >> --- a/drivers/video/fbdev/sis/init301.c
> >> +++ b/drivers/video/fbdev/sis/init301.c
> >> @@ -7971,13 +7971,8 @@ SiS_SetCHTVReg(struct SiS_Private *SiS_Pr, unsigned short ModeNo, unsigned short
> >> }
> >> } else { /* ---- PAL ---- */
> >> /* We don't play around with FSCI in PAL mode */
> >> - if(resindex = 0x04) {
> >> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
> >> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
> >> - } else {
> >> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
> >> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
> >> - }
> >> + SiS_SetCH70xxANDOR(SiS_Pr, 0x20, 0x00, 0xEF); /* loop filter off */
> >> + SiS_SetCH70xxANDOR(SiS_Pr, 0x21, 0x01, 0xFE); /* ACIV on */
> >> }
> >>
> >> #endif /* 300 */
> >
> > The code covering the PAL case had this redundancy when it was introduced
> > in Linux 2.4.19.
> >
> > Lines 7934-7981 consider three variables: PAL, overscan, and resindex.
> > Given the "#ifdef 0" block, couldn't the current six sections collapse
> > into two? One for (!PAL && overscan && resindex=5) and another for the
> > rest?
>
> Are we sure there isn't a typo in one of the duplicate clauses? Or
> wrong copy-pasting? Generally I am skeptical to "fixing" code without
> understanding what is behind or testing it, and just cosmetically
> brush over it. For now at least it is obvious that there is something
> wrong. In case (although an unlikely one) someone who understands the
> code and knows this chip comes along, he would quickly spot this.
> After your "fixups" this will be all forgotten. Additionally it adds
> to the impression that this code is being maintained, which is wrong.
>
> I would understand an argument about annoying compiler warnings and
> the like, but in that case I would prefer to #if 0 it instead of
> "prettifying" it.
>
> 0.02
> Tormod
Yes, I also wondered how this code came to be. The general intention of
the code seems clear from the code comments and the diff between 2.4.18
and 2.4.19. The redundancy pointed out in the patch existed in the !PAL
case, and became obvious when copied and reduced to the PAL case.
Thanks for pointing out that it hasn't been maintained, I missed that.
^ permalink raw reply
* Re: [PATCH RFC] video: fbdev: sis: condition with no effect
From: Tormod Volden @ 2015-02-05 21:27 UTC (permalink / raw)
To: Scot Doyle, Nicholas Mc Guire
Cc: Thomas Winischhofer, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <alpine.LNX.2.11.1502052013430.700@localhost>
On Thu, Feb 5, 2015 at 9:45 PM, Scot Doyle wrote:
> On Wed, 4 Feb 2015, Nicholas Mc Guire wrote:
>> The if and the else branch code are identical - so the condition has no
>> effect on the effective code - this patch removes the condition and the
>> duplicated code.
>>
>> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
>> ---
>>
>> This code has been in here since commit 544393fe584d ("sisfb update") so I guess it is
>> safe to simply remove the duplicated code if nobody noticed for 10 years.
>>
>> Note that the code is not really CodingStyle compliant - the lines inserted were formatted
>> to satisfy the coding style but I'm unsure if it is not better to leave it in the
>> old format.
>>
>> Patch was only compile tested with x86_64_defconfig +
>> CONFIG_FB_SIS=m, CONFIG_FB_SIS_300=y, CONFIG_FB_SIS_315=y
>>
>> Patch is against 3.19.0-rc7 (localversion-next is -next-20150204)
>>
>> drivers/video/fbdev/sis/init301.c | 9 ++-------
>> 1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/sis/init301.c b/drivers/video/fbdev/sis/init301.c
>> index 295e0de..9533a8ab 100644
>> --- a/drivers/video/fbdev/sis/init301.c
>> +++ b/drivers/video/fbdev/sis/init301.c
>> @@ -7971,13 +7971,8 @@ SiS_SetCHTVReg(struct SiS_Private *SiS_Pr, unsigned short ModeNo, unsigned short
>> }
>> } else { /* ---- PAL ---- */
>> /* We don't play around with FSCI in PAL mode */
>> - if(resindex = 0x04) {
>> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
>> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
>> - } else {
>> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
>> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
>> - }
>> + SiS_SetCH70xxANDOR(SiS_Pr, 0x20, 0x00, 0xEF); /* loop filter off */
>> + SiS_SetCH70xxANDOR(SiS_Pr, 0x21, 0x01, 0xFE); /* ACIV on */
>> }
>>
>> #endif /* 300 */
>
> The code covering the PAL case had this redundancy when it was introduced
> in Linux 2.4.19.
>
> Lines 7934-7981 consider three variables: PAL, overscan, and resindex.
> Given the "#ifdef 0" block, couldn't the current six sections collapse
> into two? One for (!PAL && overscan && resindex=5) and another for the
> rest?
Are we sure there isn't a typo in one of the duplicate clauses? Or
wrong copy-pasting? Generally I am skeptical to "fixing" code without
understanding what is behind or testing it, and just cosmetically
brush over it. For now at least it is obvious that there is something
wrong. In case (although an unlikely one) someone who understands the
code and knows this chip comes along, he would quickly spot this.
After your "fixups" this will be all forgotten. Additionally it adds
to the impression that this code is being maintained, which is wrong.
I would understand an argument about annoying compiler warnings and
the like, but in that case I would prefer to #if 0 it instead of
"prettifying" it.
0.02
Tormod
^ permalink raw reply
* Re: [PATCH RFC] video: fbdev: sis: condition with no effect
From: Scot Doyle @ 2015-02-05 20:45 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: Thomas Winischhofer, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <1423049774-16305-1-git-send-email-hofrat@osadl.org>
On Wed, 4 Feb 2015, Nicholas Mc Guire wrote:
> The if and the else branch code are identical - so the condition has no
> effect on the effective code - this patch removes the condition and the
> duplicated code.
>
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> ---
>
> This code has been in here since commit 544393fe584d ("sisfb update") so I guess it is
> safe to simply remove the duplicated code if nobody noticed for 10 years.
>
> Note that the code is not really CodingStyle compliant - the lines inserted were formatted
> to satisfy the coding style but I'm unsure if it is not better to leave it in the
> old format.
>
> Patch was only compile tested with x86_64_defconfig +
> CONFIG_FB_SIS=m, CONFIG_FB_SIS_300=y, CONFIG_FB_SIS_315=y
>
> Patch is against 3.19.0-rc7 (localversion-next is -next-20150204)
>
> drivers/video/fbdev/sis/init301.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/video/fbdev/sis/init301.c b/drivers/video/fbdev/sis/init301.c
> index 295e0de..9533a8ab 100644
> --- a/drivers/video/fbdev/sis/init301.c
> +++ b/drivers/video/fbdev/sis/init301.c
> @@ -7971,13 +7971,8 @@ SiS_SetCHTVReg(struct SiS_Private *SiS_Pr, unsigned short ModeNo, unsigned short
> }
> } else { /* ---- PAL ---- */
> /* We don't play around with FSCI in PAL mode */
> - if(resindex = 0x04) {
> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
> - } else {
> - SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
> - SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
> - }
> + SiS_SetCH70xxANDOR(SiS_Pr, 0x20, 0x00, 0xEF); /* loop filter off */
> + SiS_SetCH70xxANDOR(SiS_Pr, 0x21, 0x01, 0xFE); /* ACIV on */
> }
>
> #endif /* 300 */
The code covering the PAL case had this redundancy when it was introduced
in Linux 2.4.19.
Lines 7934-7981 consider three variables: PAL, overscan, and resindex.
Given the "#ifdef 0" block, couldn't the current six sections collapse
into two? One for (!PAL && overscan && resindex=5) and another for the
rest?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox