* [PATCH] video: ARM CLCD: Added dt support to set tim2 register
From: Arun Ramamurthy @ 2015-02-25 21:01 UTC (permalink / raw)
To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Russell King, Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: devicetree, linux-kernel, linux-fbdev, Dmitry Torokhov,
Anatol Pomazau, Jonathan Richardson, Scott Branden, Ray Jui,
bcm-kernel-feedback-list, Arun Ramamurthy
Added code based on linaro tree:
http://git.linaro.org/kernel/linux-linaro-stable.git
with commit id:6846e7822c4cab5a84672baace3b768c2d0db142
at drivers/video/amba-clcd.c. This lets the driver set
certain tim2 register bits after reading them from
device tree.
Reviewed-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
---
.../devicetree/bindings/video/arm,pl11x.txt | 17 ++++++++-
drivers/video/fbdev/amba-clcd.c | 41 ++++++++++++++++++++++
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt
index 3e3039a..14d6f87 100644
--- a/Documentation/devicetree/bindings/video/arm,pl11x.txt
+++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt
@@ -35,6 +35,21 @@ Optional properties:
cell's memory interface can handle; if not present, the memory
interface is fast enough to handle all possible video modes
+- tim2: Used to set certain bits in LCDTiming2 register.
+ It can be TIM2_CLKSEL or TIM2_IOE or both
+
+ TIM2_CLKSEL: This bit drives the CLCDCLKSEL signal. It is the select
+ signal for the external LCD clock multiplexor.
+
+ TIM2_IOE: Invert output enable:
+ 0 = CLAC output pin is active HIGH in TFT mode
+ 1 = CLAC output pin is active LOW in TFT mode.
+ This bit selects the active polarity of the output enable signal in
+ TFT mode. In this mode, the CLAC pin is an enable that indicates to
+ the LCD panel when valid display data is available. In active
+ display mode, data is driven onto the LCD data lines at the
+ programmed edge of CLCP when CLAC is in its active state.
+
Required sub-nodes:
- port: describes LCD panel signals, following the common binding
@@ -76,7 +91,7 @@ Example:
clocks = <&oscclk1>, <&oscclk2>;
clock-names = "clcdclk", "apb_pclk";
max-memory-bandwidth = <94371840>; /* Bps, 1024x768@60 16bpp */
-
+ tim2 = "TIM2_CLKSEL";
port {
clcd_pads: endpoint {
remote-endpoint = <&clcd_panel>;
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 32c0b6b..4e4e50f 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -41,6 +41,44 @@
/* This is limited to 16 characters when displayed by X startup */
static const char *clcd_name = "CLCD FB";
+struct string_lookup {
+ const char *string;
+ const u32 val;
+};
+
+static const struct string_lookup tim2_lookups[] = {
+ { "TIM2_CLKSEL", TIM2_CLKSEL},
+ { "TIM2_IOE", TIM2_IOE},
+ { NULL, 0},
+};
+
+static u32 parse_setting(const struct string_lookup *lookup, const char *name)
+{
+ int i = 0;
+
+ while (lookup[i].string != NULL) {
+ if (strcmp(lookup[i].string, name) = 0)
+ return lookup[i].val;
+ ++i;
+ }
+ return 0;
+}
+
+static u32 get_string_lookup(struct device_node *node, const char *name,
+ const struct string_lookup *lookup)
+{
+ const char *string;
+ int count, i;
+ u32 ret = 0;
+
+ count = of_property_count_strings(node, name);
+ if (count >= 0)
+ for (i = 0; i < count; i++)
+ if (of_property_read_string_index(node, name, i,
+ &string) = 0)
+ ret |= parse_setting(lookup, string);
+ return ret;
+}
/*
* Unfortunately, the enable/disable functions may be called either from
* process or IRQ context, and we _need_ to delay. This is _not_ good.
@@ -626,6 +664,9 @@ static int clcdfb_of_init_tft_panel(struct clcd_fb *fb, u32 r0, u32 g0, u32 b0)
/* Bypass pixel clock divider, data output on the falling edge */
fb->panel->tim2 = TIM2_BCD | TIM2_IPC;
+ fb->panel->tim2 |= get_string_lookup(fb->dev->dev.of_node,
+ "tim2", tim2_lookups);
+
/* TFT display, vert. comp. interrupt at the start of the back porch */
fb->panel->cntl |= CNTL_LCDTFT | CNTL_LCDVCOMP(1);
--
2.3.0
^ permalink raw reply related
* [PATCH] omapdss: extend pm notifier to handle hibernation
From: grygorii.strashko @ 2015-02-25 17:03 UTC (permalink / raw)
To: Tomi Valkeinen, Jean-Christophe Plagniol-Villard
Cc: nm, sumit.semwal, linux-omap, kishon, linux-fbdev,
Grygorii Strashko
From: Grygorii Strashko <Grygorii.Strashko@linaro.org>
Add handling of missed events in omap_dss_pm_notif which are
needed to support hibernation (suspend to disk).
Signed-off-by: Grygorii Strashko <Grygorii.Strashko@linaro.org>
---
drivers/video/fbdev/omap2/dss/core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/video/fbdev/omap2/dss/core.c b/drivers/video/fbdev/omap2/dss/core.c
index 6b74f73..e60976a 100644
--- a/drivers/video/fbdev/omap2/dss/core.c
+++ b/drivers/video/fbdev/omap2/dss/core.c
@@ -178,11 +178,15 @@ static int omap_dss_pm_notif(struct notifier_block *b, unsigned long v, void *d)
DSSDBG("pm notif %lu\n", v);
switch (v) {
+ case PM_HIBERNATION_PREPARE:
case PM_SUSPEND_PREPARE:
+ case PM_RESTORE_PREPARE:
DSSDBG("suspending displays\n");
return dss_suspend_all_devices();
case PM_POST_SUSPEND:
+ case PM_POST_HIBERNATION:
+ case PM_POST_RESTORE:
DSSDBG("resuming displays\n");
return dss_resume_all_devices();
--
1.9.1
^ permalink raw reply related
* [patch] fbdev: pm3fb: cleanup some confusing indenting
From: Dan Carpenter @ 2015-02-25 13:25 UTC (permalink / raw)
To: linux-fbdev
This if statement should be pushed out one tab to line up with the rest.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/video/fbdev/pm3fb.c b/drivers/video/fbdev/pm3fb.c
index 4bf3273..77b99ed 100644
--- a/drivers/video/fbdev/pm3fb.c
+++ b/drivers/video/fbdev/pm3fb.c
@@ -1479,9 +1479,9 @@ static void pm3fb_remove(struct pci_dev *dev)
fb_dealloc_cmap(&info->cmap);
#ifdef CONFIG_MTRR
- if (par->mtrr_handle >= 0)
- mtrr_del(par->mtrr_handle, info->fix.smem_start,
- info->fix.smem_len);
+ if (par->mtrr_handle >= 0)
+ mtrr_del(par->mtrr_handle, info->fix.smem_start,
+ info->fix.smem_len);
#endif /* CONFIG_MTRR */
iounmap(info->screen_base);
release_mem_region(fix->smem_start, fix->smem_len);
^ permalink raw reply related
* Re: [PATCH] OMAP: DSS: DPI: disable vt-switch on suspend/resume.
From: Tomi Valkeinen @ 2015-02-25 10:03 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-omap, linux-fbdev, linux-kernel, GTA04 owners
In-Reply-To: <20150225203708.0eb2fa96@notabene.brown>
[-- Attachment #1: Type: text/plain, Size: 1698 bytes --]
On 25/02/15 11:37, NeilBrown wrote:
>
> These devices do not need to return to non-graphic console
> for suspend, so disable that option.
> This means there is less work to do in the suspend/resume cycle,
> making it smoother and cheaper.
>
>
> Signed-off-by: NeilBrown <neil@brown.name>
>
> --
> Hi Tomi,
> I wonder if you would consider this patch too. It works for me and
> removes pointless vt switching. I assume no-one would need or want that
> switching on suspend/resume but I guess I cannot be certain.
>
> Maybe a module-parameter could be used if there was any uncertainty.
I was just yesterday picking patches from TI's kernel on top of
mainline, and there's a similar one for omapfb and for omapdrm. Here's
for omapfb:
http://git.ti.com/android-sdk/kernel-video/commit/5915d8744c927307987b12b14bc6aa37b3bac05c?format=patch
The patch in TI's kernel claims it's needed to make resume work. You're
saying it just optimizes suspend/resume. I hadn't picked this up
earlier, because I didn't understand why pm_set_vt_switch() would be
needed to make resume work. And never had time to study it, so I still
don't know what it's about...
Looking at the drivers/tty/vt/vt_ioctl.c, it sounds to me that we should
always do pm_set_vt_switch(0), as omapdss restores everything just fine
on resume. Although it makes me wonder how it works if there are two
display controllers, one needing the switch and the other not...
Your patch does the call in a bit odd place, so I'll be queuing the
patches for omapfb and omapdrm. Or actually, maybe the call could be
done in one place in omapdss driver, as you do, but in omap_dsshw_probe().
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v3 2/2] fbcon: expose cursor blink interval via sysfs
From: Pavel Machek @ 2015-02-25 9:49 UTC (permalink / raw)
To: Scot Doyle
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Geert Uytterhoeven, linux-fbdev, linux-kernel
In-Reply-To: <alpine.DEB.2.11.1501262040380.2052@localhost.localdomain>
On Mon 2015-01-26 20:41:53, Scot Doyle wrote:
> The fbcon cursor, when set to blink, is hardcoded to toggle display state
> five times per second. Expose this setting via
> /sys/class/graphics/fbcon/cursor_blink_ms
>
> Values written to the interface set the approximate time interval in
> milliseconds between cursor toggles, from 1 to 32767. Since the interval
> is stored internally as a number of jiffies, the millisecond value read
> from the interface may not exactly match the entered value.
>
> An outstanding blink timer is reset after a new value is entered.
>
> If the cursor blink is disabled, either via the 'cursor_blink' boolean
> setting or some other mechanism, the 'cursor_blink_ms' setting may still
> be modified. The new value will be used if the blink is reactivated.
>
> Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
Normally, this would be set by ansi escape sequences, no? We can hide
cursor using them, set its appearance.. makes sense to change timing
value there, too....
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* [PATCH] OMAP: DSS: DPI: disable vt-switch on suspend/resume.
From: NeilBrown @ 2015-02-25 9:37 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, linux-kernel, GTA04 owners
[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]
These devices do not need to return to non-graphic console
for suspend, so disable that option.
This means there is less work to do in the suspend/resume cycle,
making it smoother and cheaper.
Signed-off-by: NeilBrown <neil@brown.name>
--
Hi Tomi,
I wonder if you would consider this patch too. It works for me and
removes pointless vt switching. I assume no-one would need or want that
switching on suspend/resume but I guess I cannot be certain.
Maybe a module-parameter could be used if there was any uncertainty.
thanks,
NeilBrown
diff --git a/drivers/video/fbdev/omap2/dss/dpi.c b/drivers/video/fbdev/omap2/dss/dpi.c
index f83e7b030249..4a29bab4ade3 100644
--- a/drivers/video/fbdev/omap2/dss/dpi.c
+++ b/drivers/video/fbdev/omap2/dss/dpi.c
@@ -32,6 +32,7 @@
#include <linux/string.h>
#include <linux/of.h>
#include <linux/clk.h>
+#include <linux/suspend.h>
#include <video/omapdss.h>
@@ -798,6 +799,8 @@ static int omap_dpi_probe(struct platform_device *pdev)
mutex_init(&dpi->lock);
dpi_init_output(pdev);
+ /* No need to vt_switch in suspend/resume */
+ pm_set_vt_switch(0);
return 0;
}
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply related
* Re: [PATCH] OMAPDSS: restore "name" sysfs entry.
From: Tomi Valkeinen @ 2015-02-25 9:32 UTC (permalink / raw)
To: NeilBrown
Cc: Dr. H. Nikolaus Schaller, linux-omap, linux-fbdev, linux-kernel,
GTA04 owners
In-Reply-To: <20150225202042.2adaea4f@notabene.brown>
[-- Attachment #1: Type: text/plain, Size: 2140 bytes --]
On 25/02/15 11:20, NeilBrown wrote:
> Tested-by: NeilBrown <neilb@suse.de>
>
> Before the patch:
>
> # ls -l /sys/devices/platform/omapdss/display0
> lrwxrwxrwx 1 root root 0 Feb 8 12:57 /sys/devices/platform/omapdss/display0 -> ../spi_lcd/spi_master/spi32766/spi32766.0
>
> After the patch:
>
> # ls -l /sys/devices/platform/omapdss/display0
> total 0
> -r--r--r-- 1 root root 4096 Feb 8 13:37 display_name
> -rw-r--r-- 1 root root 4096 Feb 8 13:37 enabled
> -rw-r--r-- 1 root root 4096 Feb 8 13:37 mirror
> -r--r--r-- 1 root root 4096 Feb 8 13:37 name
> -rw-r--r-- 1 root root 4096 Feb 8 13:37 rotate
> -rw-r--r-- 1 root root 4096 Feb 8 13:37 tear_elim
> -rw-r--r-- 1 root root 4096 Feb 8 13:37 timings
> -rw-r--r-- 1 root root 4096 Feb 8 13:37 wss
>
>
> So as you say it creates a directory just for the display0 device, and that
> has the 'name' that we want.
I think this was something similar than how the sysfs files were set up
originally for omapdss. If I remember right, we didn't have proper
devices for the displays then. Things have evolved quite a bit since then.
There's a small chance of this patch breaking things, of course... If
someone accessed those display sysfs files via the display device
(../spi_lcd/spi_master/spi32766/spi32766.0), the files are now gone.
But I think (hope...) they are always accessed via the omapdss's
displayX directories.
> This works for me, and it seems to me to be a better fit to the general
> structure of /sys/devices - symlinks within /sys/devices are a substantial
> minority, other than 'subsystem', 'device', 'driver' and 'bdi' which have
> very generic meanings.
>
> I guess I'm a little surprised that there doesn't seem to be any linkage from
> the display0 to the spi device. Maybe that isn't important.
Yep, I don't think so. In any case, all this is to be deprecated, and as
soon as omapdrm driver works reliably that should be the driver to use.
So of course we need to keep omapfb working for the years to come, but
I'd rather not add any new sysfs files for a soon deprecated driver.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: restore "name" sysfs entry.
From: NeilBrown @ 2015-02-25 9:20 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Dr. H. Nikolaus Schaller, linux-omap, linux-fbdev, linux-kernel,
GTA04 owners
In-Reply-To: <54ED8CB6.4010308@ti.com>
[-- Attachment #1: Type: text/plain, Size: 14676 bytes --]
On Wed, 25 Feb 2015 10:49:58 +0200 Tomi Valkeinen <tomi.valkeinen@ti.com>
wrote:
> Hi,
>
> On 24/02/15 22:31, NeilBrown wrote:
> > On Tue, 24 Feb 2015 12:40:32 +0200 Tomi Valkeinen <tomi.valkeinen@ti.com>
> > wrote:
> >
> >> Hi,
> >>
> >> On 24/02/15 11:37, NeilBrown wrote:
> >>>
> >>>
> >>> commit 303e4697e762dc92a40405f4e4b8aac02cd0d70b
> >>> OMAPDSS: rename display-sysfs 'name' entry
> >>>
> >>> broke the xorg X server on my device as it couldn't find the display
> >>> any more. It needs the 'name' file and now there isn't one.
> >>>
> >>> That commit claims that 'name' is not compatible with i2c or spi.
> >>> i2c does register it own 'name' file, but spi does not, hence my
> >>> problem - I have an spi display.
> >>>
> >>> So create a special case for i2c: add the name attribute for non-i2c
> >>> devices.
>
> How about this patch instead. It separates the underlying device's sysfs directory
> from the "displayX" directory, and allows us to have name & display_name. So it
> should work for any device type.
>
>
> From 8e411fa684d42fca35628b41837c6d72e87aaff0 Mon Sep 17 00:00:00 2001
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Date: Wed, 25 Feb 2015 10:23:58 +0200
> Subject: [PATCH] OMAPDSS: fix regression with display sysfs files
>
> omapdss's sysfs directories for displays used to have 'name' file,
> giving the name for the display. This file was later renamed to
> 'display_name' to avoid conflicts with i2c sysfs 'name' file. Looks like
> at least xserver-xorg-video-omap3 requires the 'name' file to be
> present.
>
> To fix the regression, this patch creates new kobjects for each display,
> allowing us to create sysfs directories for the displays. This way we
> have the whole directory for omapdss, and there will be no sysfs file
> clashes with the underlying display device's sysfs files.
>
> We can thus add the 'name' sysfs file back.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/video/fbdev/omap2/dss/display-sysfs.c | 179 ++++++++++++++------------
> include/video/omapdss.h | 1 +
> 2 files changed, 96 insertions(+), 84 deletions(-)
>
> diff --git a/drivers/video/fbdev/omap2/dss/display-sysfs.c b/drivers/video/fbdev/omap2/dss/display-sysfs.c
> index 5a2095a98ed8..12186557a9d4 100644
> --- a/drivers/video/fbdev/omap2/dss/display-sysfs.c
> +++ b/drivers/video/fbdev/omap2/dss/display-sysfs.c
> @@ -28,44 +28,22 @@
> #include <video/omapdss.h>
> #include "dss.h"
>
> -static struct omap_dss_device *to_dss_device_sysfs(struct device *dev)
> +static ssize_t display_name_show(struct omap_dss_device *dssdev, char *buf)
> {
> - struct omap_dss_device *dssdev = NULL;
> -
> - for_each_dss_dev(dssdev) {
> - if (dssdev->dev == dev) {
> - omap_dss_put_device(dssdev);
> - return dssdev;
> - }
> - }
> -
> - return NULL;
> -}
> -
> -static ssize_t display_name_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
> -
> return snprintf(buf, PAGE_SIZE, "%s\n",
> dssdev->name ?
> dssdev->name : "");
> }
>
> -static ssize_t display_enabled_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> +static ssize_t display_enabled_show(struct omap_dss_device *dssdev, char *buf)
> {
> - struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
> -
> return snprintf(buf, PAGE_SIZE, "%d\n",
> omapdss_device_is_enabled(dssdev));
> }
>
> -static ssize_t display_enabled_store(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t display_enabled_store(struct omap_dss_device *dssdev,
> const char *buf, size_t size)
> {
> - struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
> int r;
> bool enable;
>
> @@ -90,19 +68,16 @@ static ssize_t display_enabled_store(struct device *dev,
> return size;
> }
>
> -static ssize_t display_tear_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> +static ssize_t display_tear_show(struct omap_dss_device *dssdev, char *buf)
> {
> - struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
> return snprintf(buf, PAGE_SIZE, "%d\n",
> dssdev->driver->get_te ?
> dssdev->driver->get_te(dssdev) : 0);
> }
>
> -static ssize_t display_tear_store(struct device *dev,
> - struct device_attribute *attr, const char *buf, size_t size)
> +static ssize_t display_tear_store(struct omap_dss_device *dssdev,
> + const char *buf, size_t size)
> {
> - struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
> int r;
> bool te;
>
> @@ -120,10 +95,8 @@ static ssize_t display_tear_store(struct device *dev,
> return size;
> }
>
> -static ssize_t display_timings_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> +static ssize_t display_timings_show(struct omap_dss_device *dssdev, char *buf)
> {
> - struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
> struct omap_video_timings t;
>
> if (!dssdev->driver->get_timings)
> @@ -137,10 +110,9 @@ static ssize_t display_timings_show(struct device *dev,
> t.y_res, t.vfp, t.vbp, t.vsw);
> }
>
> -static ssize_t display_timings_store(struct device *dev,
> - struct device_attribute *attr, const char *buf, size_t size)
> +static ssize_t display_timings_store(struct omap_dss_device *dssdev,
> + const char *buf, size_t size)
> {
> - struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
> struct omap_video_timings t = dssdev->panel.timings;
> int r, found;
>
> @@ -176,10 +148,8 @@ static ssize_t display_timings_store(struct device *dev,
> return size;
> }
>
> -static ssize_t display_rotate_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> +static ssize_t display_rotate_show(struct omap_dss_device *dssdev, char *buf)
> {
> - struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
> int rotate;
> if (!dssdev->driver->get_rotate)
> return -ENOENT;
> @@ -187,10 +157,9 @@ static ssize_t display_rotate_show(struct device *dev,
> return snprintf(buf, PAGE_SIZE, "%u\n", rotate);
> }
>
> -static ssize_t display_rotate_store(struct device *dev,
> - struct device_attribute *attr, const char *buf, size_t size)
> +static ssize_t display_rotate_store(struct omap_dss_device *dssdev,
> + const char *buf, size_t size)
> {
> - struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
> int rot, r;
>
> if (!dssdev->driver->set_rotate || !dssdev->driver->get_rotate)
> @@ -207,10 +176,8 @@ static ssize_t display_rotate_store(struct device *dev,
> return size;
> }
>
> -static ssize_t display_mirror_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> +static ssize_t display_mirror_show(struct omap_dss_device *dssdev, char *buf)
> {
> - struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
> int mirror;
> if (!dssdev->driver->get_mirror)
> return -ENOENT;
> @@ -218,10 +185,9 @@ static ssize_t display_mirror_show(struct device *dev,
> return snprintf(buf, PAGE_SIZE, "%u\n", mirror);
> }
>
> -static ssize_t display_mirror_store(struct device *dev,
> - struct device_attribute *attr, const char *buf, size_t size)
> +static ssize_t display_mirror_store(struct omap_dss_device *dssdev,
> + const char *buf, size_t size)
> {
> - struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
> int r;
> bool mirror;
>
> @@ -239,10 +205,8 @@ static ssize_t display_mirror_store(struct device *dev,
> return size;
> }
>
> -static ssize_t display_wss_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> +static ssize_t display_wss_show(struct omap_dss_device *dssdev, char *buf)
> {
> - struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
> unsigned int wss;
>
> if (!dssdev->driver->get_wss)
> @@ -253,10 +217,9 @@ static ssize_t display_wss_show(struct device *dev,
> return snprintf(buf, PAGE_SIZE, "0x%05x\n", wss);
> }
>
> -static ssize_t display_wss_store(struct device *dev,
> - struct device_attribute *attr, const char *buf, size_t size)
> +static ssize_t display_wss_store(struct omap_dss_device *dssdev,
> + const char *buf, size_t size)
> {
> - struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
> u32 wss;
> int r;
>
> @@ -277,50 +240,94 @@ static ssize_t display_wss_store(struct device *dev,
> return size;
> }
>
> -static DEVICE_ATTR(display_name, S_IRUGO, display_name_show, NULL);
> -static DEVICE_ATTR(enabled, S_IRUGO|S_IWUSR,
> +struct display_attribute {
> + struct attribute attr;
> + ssize_t (*show)(struct omap_dss_device *, char *);
> + ssize_t (*store)(struct omap_dss_device *, const char *, size_t);
> +};
> +
> +#define DISPLAY_ATTR(_name, _mode, _show, _store) \
> + struct display_attribute display_attr_##_name = \
> + __ATTR(_name, _mode, _show, _store)
> +
> +static DISPLAY_ATTR(name, S_IRUGO, display_name_show, NULL);
> +static DISPLAY_ATTR(display_name, S_IRUGO, display_name_show, NULL);
> +static DISPLAY_ATTR(enabled, S_IRUGO|S_IWUSR,
> display_enabled_show, display_enabled_store);
> -static DEVICE_ATTR(tear_elim, S_IRUGO|S_IWUSR,
> +static DISPLAY_ATTR(tear_elim, S_IRUGO|S_IWUSR,
> display_tear_show, display_tear_store);
> -static DEVICE_ATTR(timings, S_IRUGO|S_IWUSR,
> +static DISPLAY_ATTR(timings, S_IRUGO|S_IWUSR,
> display_timings_show, display_timings_store);
> -static DEVICE_ATTR(rotate, S_IRUGO|S_IWUSR,
> +static DISPLAY_ATTR(rotate, S_IRUGO|S_IWUSR,
> display_rotate_show, display_rotate_store);
> -static DEVICE_ATTR(mirror, S_IRUGO|S_IWUSR,
> +static DISPLAY_ATTR(mirror, S_IRUGO|S_IWUSR,
> display_mirror_show, display_mirror_store);
> -static DEVICE_ATTR(wss, S_IRUGO|S_IWUSR,
> +static DISPLAY_ATTR(wss, S_IRUGO|S_IWUSR,
> display_wss_show, display_wss_store);
>
> -static const struct attribute *display_sysfs_attrs[] = {
> - &dev_attr_display_name.attr,
> - &dev_attr_enabled.attr,
> - &dev_attr_tear_elim.attr,
> - &dev_attr_timings.attr,
> - &dev_attr_rotate.attr,
> - &dev_attr_mirror.attr,
> - &dev_attr_wss.attr,
> +static struct attribute *display_sysfs_attrs[] = {
> + &display_attr_name.attr,
> + &display_attr_display_name.attr,
> + &display_attr_enabled.attr,
> + &display_attr_tear_elim.attr,
> + &display_attr_timings.attr,
> + &display_attr_rotate.attr,
> + &display_attr_mirror.attr,
> + &display_attr_wss.attr,
> NULL
> };
>
> +static ssize_t display_attr_show(struct kobject *kobj, struct attribute *attr,
> + char *buf)
> +{
> + struct omap_dss_device *dssdev;
> + struct display_attribute *display_attr;
> +
> + dssdev = container_of(kobj, struct omap_dss_device, kobj);
> + display_attr = container_of(attr, struct display_attribute, attr);
> +
> + if (!display_attr->show)
> + return -ENOENT;
> +
> + return display_attr->show(dssdev, buf);
> +}
> +
> +static ssize_t display_attr_store(struct kobject *kobj, struct attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct omap_dss_device *dssdev;
> + struct display_attribute *display_attr;
> +
> + dssdev = container_of(kobj, struct omap_dss_device, kobj);
> + display_attr = container_of(attr, struct display_attribute, attr);
> +
> + if (!display_attr->store)
> + return -ENOENT;
> +
> + return display_attr->store(dssdev, buf, size);
> +}
> +
> +static const struct sysfs_ops display_sysfs_ops = {
> + .show = display_attr_show,
> + .store = display_attr_store,
> +};
> +
> +static struct kobj_type display_ktype = {
> + .sysfs_ops = &display_sysfs_ops,
> + .default_attrs = display_sysfs_attrs,
> +};
> +
> int display_init_sysfs(struct platform_device *pdev)
> {
> struct omap_dss_device *dssdev = NULL;
> int r;
>
> for_each_dss_dev(dssdev) {
> - struct kobject *kobj = &dssdev->dev->kobj;
> -
> - r = sysfs_create_files(kobj, display_sysfs_attrs);
> + r = kobject_init_and_add(&dssdev->kobj, &display_ktype,
> + &pdev->dev.kobj, dssdev->alias);
> if (r) {
> DSSERR("failed to create sysfs files\n");
> - goto err;
> - }
> -
> - r = sysfs_create_link(&pdev->dev.kobj, kobj, dssdev->alias);
> - if (r) {
> - sysfs_remove_files(kobj, display_sysfs_attrs);
> -
> - DSSERR("failed to create sysfs display link\n");
> + omap_dss_put_device(dssdev);
> goto err;
> }
> }
> @@ -338,8 +345,12 @@ void display_uninit_sysfs(struct platform_device *pdev)
> struct omap_dss_device *dssdev = NULL;
>
> for_each_dss_dev(dssdev) {
> - sysfs_remove_link(&pdev->dev.kobj, dssdev->alias);
> - sysfs_remove_files(&dssdev->dev->kobj,
> - display_sysfs_attrs);
> + if (kobject_name(&dssdev->kobj) == NULL)
> + continue;
> +
> + kobject_del(&dssdev->kobj);
> + kobject_put(&dssdev->kobj);
> +
> + memset(&dssdev->kobj, 0, sizeof(dssdev->kobj));
> }
> }
> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
> index 60de61fea8e3..c8ed15daad02 100644
> --- a/include/video/omapdss.h
> +++ b/include/video/omapdss.h
> @@ -689,6 +689,7 @@ struct omapdss_dsi_ops {
> };
>
> struct omap_dss_device {
> + struct kobject kobj;
> struct device *dev;
>
> struct module *owner;
Tested-by: NeilBrown <neilb@suse.de>
Before the patch:
# ls -l /sys/devices/platform/omapdss/display0
lrwxrwxrwx 1 root root 0 Feb 8 12:57 /sys/devices/platform/omapdss/display0 -> ../spi_lcd/spi_master/spi32766/spi32766.0
After the patch:
# ls -l /sys/devices/platform/omapdss/display0
total 0
-r--r--r-- 1 root root 4096 Feb 8 13:37 display_name
-rw-r--r-- 1 root root 4096 Feb 8 13:37 enabled
-rw-r--r-- 1 root root 4096 Feb 8 13:37 mirror
-r--r--r-- 1 root root 4096 Feb 8 13:37 name
-rw-r--r-- 1 root root 4096 Feb 8 13:37 rotate
-rw-r--r-- 1 root root 4096 Feb 8 13:37 tear_elim
-rw-r--r-- 1 root root 4096 Feb 8 13:37 timings
-rw-r--r-- 1 root root 4096 Feb 8 13:37 wss
So as you say it creates a directory just for the display0 device, and that
has the 'name' that we want.
This works for me, and it seems to me to be a better fit to the general
structure of /sys/devices - symlinks within /sys/devices are a substantial
minority, other than 'subsystem', 'device', 'driver' and 'bdi' which have
very generic meanings.
I guess I'm a little surprised that there doesn't seem to be any linkage from
the display0 to the spi device. Maybe that isn't important.
Thanks a lot!
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: restore "name" sysfs entry.
From: Tomi Valkeinen @ 2015-02-25 8:49 UTC (permalink / raw)
To: NeilBrown, Dr. H. Nikolaus Schaller
Cc: linux-omap, linux-fbdev, linux-kernel, GTA04 owners
In-Reply-To: <20150225073131.7fbff605@notabene.brown>
[-- Attachment #1: Type: text/plain, Size: 12604 bytes --]
Hi,
On 24/02/15 22:31, NeilBrown wrote:
> On Tue, 24 Feb 2015 12:40:32 +0200 Tomi Valkeinen <tomi.valkeinen@ti.com>
> wrote:
>
>> Hi,
>>
>> On 24/02/15 11:37, NeilBrown wrote:
>>>
>>>
>>> commit 303e4697e762dc92a40405f4e4b8aac02cd0d70b
>>> OMAPDSS: rename display-sysfs 'name' entry
>>>
>>> broke the xorg X server on my device as it couldn't find the display
>>> any more. It needs the 'name' file and now there isn't one.
>>>
>>> That commit claims that 'name' is not compatible with i2c or spi.
>>> i2c does register it own 'name' file, but spi does not, hence my
>>> problem - I have an spi display.
>>>
>>> So create a special case for i2c: add the name attribute for non-i2c
>>> devices.
How about this patch instead. It separates the underlying device's sysfs directory
from the "displayX" directory, and allows us to have name & display_name. So it
should work for any device type.
From 8e411fa684d42fca35628b41837c6d72e87aaff0 Mon Sep 17 00:00:00 2001
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
Date: Wed, 25 Feb 2015 10:23:58 +0200
Subject: [PATCH] OMAPDSS: fix regression with display sysfs files
omapdss's sysfs directories for displays used to have 'name' file,
giving the name for the display. This file was later renamed to
'display_name' to avoid conflicts with i2c sysfs 'name' file. Looks like
at least xserver-xorg-video-omap3 requires the 'name' file to be
present.
To fix the regression, this patch creates new kobjects for each display,
allowing us to create sysfs directories for the displays. This way we
have the whole directory for omapdss, and there will be no sysfs file
clashes with the underlying display device's sysfs files.
We can thus add the 'name' sysfs file back.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/video/fbdev/omap2/dss/display-sysfs.c | 179 ++++++++++++++------------
include/video/omapdss.h | 1 +
2 files changed, 96 insertions(+), 84 deletions(-)
diff --git a/drivers/video/fbdev/omap2/dss/display-sysfs.c b/drivers/video/fbdev/omap2/dss/display-sysfs.c
index 5a2095a98ed8..12186557a9d4 100644
--- a/drivers/video/fbdev/omap2/dss/display-sysfs.c
+++ b/drivers/video/fbdev/omap2/dss/display-sysfs.c
@@ -28,44 +28,22 @@
#include <video/omapdss.h>
#include "dss.h"
-static struct omap_dss_device *to_dss_device_sysfs(struct device *dev)
+static ssize_t display_name_show(struct omap_dss_device *dssdev, char *buf)
{
- struct omap_dss_device *dssdev = NULL;
-
- for_each_dss_dev(dssdev) {
- if (dssdev->dev == dev) {
- omap_dss_put_device(dssdev);
- return dssdev;
- }
- }
-
- return NULL;
-}
-
-static ssize_t display_name_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
-
return snprintf(buf, PAGE_SIZE, "%s\n",
dssdev->name ?
dssdev->name : "");
}
-static ssize_t display_enabled_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t display_enabled_show(struct omap_dss_device *dssdev, char *buf)
{
- struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
-
return snprintf(buf, PAGE_SIZE, "%d\n",
omapdss_device_is_enabled(dssdev));
}
-static ssize_t display_enabled_store(struct device *dev,
- struct device_attribute *attr,
+static ssize_t display_enabled_store(struct omap_dss_device *dssdev,
const char *buf, size_t size)
{
- struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
int r;
bool enable;
@@ -90,19 +68,16 @@ static ssize_t display_enabled_store(struct device *dev,
return size;
}
-static ssize_t display_tear_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t display_tear_show(struct omap_dss_device *dssdev, char *buf)
{
- struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
return snprintf(buf, PAGE_SIZE, "%d\n",
dssdev->driver->get_te ?
dssdev->driver->get_te(dssdev) : 0);
}
-static ssize_t display_tear_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t size)
+static ssize_t display_tear_store(struct omap_dss_device *dssdev,
+ const char *buf, size_t size)
{
- struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
int r;
bool te;
@@ -120,10 +95,8 @@ static ssize_t display_tear_store(struct device *dev,
return size;
}
-static ssize_t display_timings_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t display_timings_show(struct omap_dss_device *dssdev, char *buf)
{
- struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
struct omap_video_timings t;
if (!dssdev->driver->get_timings)
@@ -137,10 +110,9 @@ static ssize_t display_timings_show(struct device *dev,
t.y_res, t.vfp, t.vbp, t.vsw);
}
-static ssize_t display_timings_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t size)
+static ssize_t display_timings_store(struct omap_dss_device *dssdev,
+ const char *buf, size_t size)
{
- struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
struct omap_video_timings t = dssdev->panel.timings;
int r, found;
@@ -176,10 +148,8 @@ static ssize_t display_timings_store(struct device *dev,
return size;
}
-static ssize_t display_rotate_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t display_rotate_show(struct omap_dss_device *dssdev, char *buf)
{
- struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
int rotate;
if (!dssdev->driver->get_rotate)
return -ENOENT;
@@ -187,10 +157,9 @@ static ssize_t display_rotate_show(struct device *dev,
return snprintf(buf, PAGE_SIZE, "%u\n", rotate);
}
-static ssize_t display_rotate_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t size)
+static ssize_t display_rotate_store(struct omap_dss_device *dssdev,
+ const char *buf, size_t size)
{
- struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
int rot, r;
if (!dssdev->driver->set_rotate || !dssdev->driver->get_rotate)
@@ -207,10 +176,8 @@ static ssize_t display_rotate_store(struct device *dev,
return size;
}
-static ssize_t display_mirror_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t display_mirror_show(struct omap_dss_device *dssdev, char *buf)
{
- struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
int mirror;
if (!dssdev->driver->get_mirror)
return -ENOENT;
@@ -218,10 +185,9 @@ static ssize_t display_mirror_show(struct device *dev,
return snprintf(buf, PAGE_SIZE, "%u\n", mirror);
}
-static ssize_t display_mirror_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t size)
+static ssize_t display_mirror_store(struct omap_dss_device *dssdev,
+ const char *buf, size_t size)
{
- struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
int r;
bool mirror;
@@ -239,10 +205,8 @@ static ssize_t display_mirror_store(struct device *dev,
return size;
}
-static ssize_t display_wss_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t display_wss_show(struct omap_dss_device *dssdev, char *buf)
{
- struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
unsigned int wss;
if (!dssdev->driver->get_wss)
@@ -253,10 +217,9 @@ static ssize_t display_wss_show(struct device *dev,
return snprintf(buf, PAGE_SIZE, "0x%05x\n", wss);
}
-static ssize_t display_wss_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t size)
+static ssize_t display_wss_store(struct omap_dss_device *dssdev,
+ const char *buf, size_t size)
{
- struct omap_dss_device *dssdev = to_dss_device_sysfs(dev);
u32 wss;
int r;
@@ -277,50 +240,94 @@ static ssize_t display_wss_store(struct device *dev,
return size;
}
-static DEVICE_ATTR(display_name, S_IRUGO, display_name_show, NULL);
-static DEVICE_ATTR(enabled, S_IRUGO|S_IWUSR,
+struct display_attribute {
+ struct attribute attr;
+ ssize_t (*show)(struct omap_dss_device *, char *);
+ ssize_t (*store)(struct omap_dss_device *, const char *, size_t);
+};
+
+#define DISPLAY_ATTR(_name, _mode, _show, _store) \
+ struct display_attribute display_attr_##_name = \
+ __ATTR(_name, _mode, _show, _store)
+
+static DISPLAY_ATTR(name, S_IRUGO, display_name_show, NULL);
+static DISPLAY_ATTR(display_name, S_IRUGO, display_name_show, NULL);
+static DISPLAY_ATTR(enabled, S_IRUGO|S_IWUSR,
display_enabled_show, display_enabled_store);
-static DEVICE_ATTR(tear_elim, S_IRUGO|S_IWUSR,
+static DISPLAY_ATTR(tear_elim, S_IRUGO|S_IWUSR,
display_tear_show, display_tear_store);
-static DEVICE_ATTR(timings, S_IRUGO|S_IWUSR,
+static DISPLAY_ATTR(timings, S_IRUGO|S_IWUSR,
display_timings_show, display_timings_store);
-static DEVICE_ATTR(rotate, S_IRUGO|S_IWUSR,
+static DISPLAY_ATTR(rotate, S_IRUGO|S_IWUSR,
display_rotate_show, display_rotate_store);
-static DEVICE_ATTR(mirror, S_IRUGO|S_IWUSR,
+static DISPLAY_ATTR(mirror, S_IRUGO|S_IWUSR,
display_mirror_show, display_mirror_store);
-static DEVICE_ATTR(wss, S_IRUGO|S_IWUSR,
+static DISPLAY_ATTR(wss, S_IRUGO|S_IWUSR,
display_wss_show, display_wss_store);
-static const struct attribute *display_sysfs_attrs[] = {
- &dev_attr_display_name.attr,
- &dev_attr_enabled.attr,
- &dev_attr_tear_elim.attr,
- &dev_attr_timings.attr,
- &dev_attr_rotate.attr,
- &dev_attr_mirror.attr,
- &dev_attr_wss.attr,
+static struct attribute *display_sysfs_attrs[] = {
+ &display_attr_name.attr,
+ &display_attr_display_name.attr,
+ &display_attr_enabled.attr,
+ &display_attr_tear_elim.attr,
+ &display_attr_timings.attr,
+ &display_attr_rotate.attr,
+ &display_attr_mirror.attr,
+ &display_attr_wss.attr,
NULL
};
+static ssize_t display_attr_show(struct kobject *kobj, struct attribute *attr,
+ char *buf)
+{
+ struct omap_dss_device *dssdev;
+ struct display_attribute *display_attr;
+
+ dssdev = container_of(kobj, struct omap_dss_device, kobj);
+ display_attr = container_of(attr, struct display_attribute, attr);
+
+ if (!display_attr->show)
+ return -ENOENT;
+
+ return display_attr->show(dssdev, buf);
+}
+
+static ssize_t display_attr_store(struct kobject *kobj, struct attribute *attr,
+ const char *buf, size_t size)
+{
+ struct omap_dss_device *dssdev;
+ struct display_attribute *display_attr;
+
+ dssdev = container_of(kobj, struct omap_dss_device, kobj);
+ display_attr = container_of(attr, struct display_attribute, attr);
+
+ if (!display_attr->store)
+ return -ENOENT;
+
+ return display_attr->store(dssdev, buf, size);
+}
+
+static const struct sysfs_ops display_sysfs_ops = {
+ .show = display_attr_show,
+ .store = display_attr_store,
+};
+
+static struct kobj_type display_ktype = {
+ .sysfs_ops = &display_sysfs_ops,
+ .default_attrs = display_sysfs_attrs,
+};
+
int display_init_sysfs(struct platform_device *pdev)
{
struct omap_dss_device *dssdev = NULL;
int r;
for_each_dss_dev(dssdev) {
- struct kobject *kobj = &dssdev->dev->kobj;
-
- r = sysfs_create_files(kobj, display_sysfs_attrs);
+ r = kobject_init_and_add(&dssdev->kobj, &display_ktype,
+ &pdev->dev.kobj, dssdev->alias);
if (r) {
DSSERR("failed to create sysfs files\n");
- goto err;
- }
-
- r = sysfs_create_link(&pdev->dev.kobj, kobj, dssdev->alias);
- if (r) {
- sysfs_remove_files(kobj, display_sysfs_attrs);
-
- DSSERR("failed to create sysfs display link\n");
+ omap_dss_put_device(dssdev);
goto err;
}
}
@@ -338,8 +345,12 @@ void display_uninit_sysfs(struct platform_device *pdev)
struct omap_dss_device *dssdev = NULL;
for_each_dss_dev(dssdev) {
- sysfs_remove_link(&pdev->dev.kobj, dssdev->alias);
- sysfs_remove_files(&dssdev->dev->kobj,
- display_sysfs_attrs);
+ if (kobject_name(&dssdev->kobj) == NULL)
+ continue;
+
+ kobject_del(&dssdev->kobj);
+ kobject_put(&dssdev->kobj);
+
+ memset(&dssdev->kobj, 0, sizeof(dssdev->kobj));
}
}
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index 60de61fea8e3..c8ed15daad02 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -689,6 +689,7 @@ struct omapdss_dsi_ops {
};
struct omap_dss_device {
+ struct kobject kobj;
struct device *dev;
struct module *owner;
--
2.3.0
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply related
* Re: [PATCH] OMAPDSS: restore "name" sysfs entry.
From: Tomi Valkeinen @ 2015-02-25 7:04 UTC (permalink / raw)
To: NeilBrown
Cc: linux-omap, linux-fbdev, linux-kernel, GTA04 owners,
Dr. H. Nikolaus Schaller
In-Reply-To: <20150225073131.7fbff605@notabene.brown>
[-- Attachment #1: Type: text/plain, Size: 1774 bytes --]
On 24/02/15 22:31, NeilBrown wrote:
> On Tue, 24 Feb 2015 12:40:32 +0200 Tomi Valkeinen <tomi.valkeinen@ti.com>
> wrote:
>
>> Hi,
>>
>> On 24/02/15 11:37, NeilBrown wrote:
>>>
>>>
>>> commit 303e4697e762dc92a40405f4e4b8aac02cd0d70b
>>> OMAPDSS: rename display-sysfs 'name' entry
>>>
>>> broke the xorg X server on my device as it couldn't find the display
>>> any more. It needs the 'name' file and now there isn't one.
>>>
>>> That commit claims that 'name' is not compatible with i2c or spi.
>>> i2c does register it own 'name' file, but spi does not, hence my
>>> problem - I have an spi display.
>>>
>>> So create a special case for i2c: add the name attribute for non-i2c
>>> devices.
>>
>> What X driver is that? What's it doing with the display name? Is it just
>> using the display name to show something for the user, and the returned
>> value can be essentially any string?
>>
>> Tomi
>>
>>
> /usr/lib/xorg/modules/drivers/omapfb_drv.so
> from package xserver-xorg-video-omap3 in Debian.
>
> I don't know where the main upstream source is, but here:
>
> https://gitorious.org/gnutoo-s-programs-for-shr/xf86-video-omapfb/source/28c006c94e57ea71df11ec4fff79d7ffcfc4860f:src/omapfb-output-dss.c#L258
>
> is the code which reads
> /sys/devices/platform/omapdss/display0/name
> and fails if that file cannot be opened.
Thanks. Unfortunately it looks to me that the omapfb_drv uses the
display name to configure things, and as in i2c's case the 'name' is not
the correct dss name, X will probably fail in interesting ways.
Of course, it's already broken in that way, so this fix improves the
situation for non-i2c displays. I'll have a look if I can figure out how
to fix this for all displays.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: restore "name" sysfs entry.
From: Tony Lindgren @ 2015-02-24 20:48 UTC (permalink / raw)
To: NeilBrown
Cc: Tomi Valkeinen, linux-omap, linux-fbdev, linux-kernel,
GTA04 owners, Dr. H. Nikolaus Schaller
In-Reply-To: <20150225073131.7fbff605@notabene.brown>
* NeilBrown <neilb@suse.de> [150224 12:35]:
> On Tue, 24 Feb 2015 12:40:32 +0200 Tomi Valkeinen <tomi.valkeinen@ti.com>
> wrote:
>
> > Hi,
> >
> > On 24/02/15 11:37, NeilBrown wrote:
> > >
> > >
> > > commit 303e4697e762dc92a40405f4e4b8aac02cd0d70b
> > > OMAPDSS: rename display-sysfs 'name' entry
> > >
> > > broke the xorg X server on my device as it couldn't find the display
> > > any more. It needs the 'name' file and now there isn't one.
> > >
> > > That commit claims that 'name' is not compatible with i2c or spi.
> > > i2c does register it own 'name' file, but spi does not, hence my
> > > problem - I have an spi display.
> > >
> > > So create a special case for i2c: add the name attribute for non-i2c
> > > devices.
> >
> > What X driver is that? What's it doing with the display name? Is it just
> > using the display name to show something for the user, and the returned
> > value can be essentially any string?
> >
> > Tomi
> >
> >
> /usr/lib/xorg/modules/drivers/omapfb_drv.so
> from package xserver-xorg-video-omap3 in Debian.
>
> I don't know where the main upstream source is, but here:
>
> https://gitorious.org/gnutoo-s-programs-for-shr/xf86-video-omapfb/source/28c006c94e57ea71df11ec4fff79d7ffcfc4860f:src/omapfb-output-dss.c#L258
>
> is the code which reads
> /sys/devices/platform/omapdss/display0/name
> and fails if that file cannot be opened.
Sounds like a kernel interface we need to support forever
and fix the regression.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH] OMAPDSS: restore "name" sysfs entry.
From: NeilBrown @ 2015-02-24 20:31 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-omap, linux-fbdev, linux-kernel, GTA04 owners,
Dr. H. Nikolaus Schaller
In-Reply-To: <54EC5520.1020105@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1334 bytes --]
On Tue, 24 Feb 2015 12:40:32 +0200 Tomi Valkeinen <tomi.valkeinen@ti.com>
wrote:
> Hi,
>
> On 24/02/15 11:37, NeilBrown wrote:
> >
> >
> > commit 303e4697e762dc92a40405f4e4b8aac02cd0d70b
> > OMAPDSS: rename display-sysfs 'name' entry
> >
> > broke the xorg X server on my device as it couldn't find the display
> > any more. It needs the 'name' file and now there isn't one.
> >
> > That commit claims that 'name' is not compatible with i2c or spi.
> > i2c does register it own 'name' file, but spi does not, hence my
> > problem - I have an spi display.
> >
> > So create a special case for i2c: add the name attribute for non-i2c
> > devices.
>
> What X driver is that? What's it doing with the display name? Is it just
> using the display name to show something for the user, and the returned
> value can be essentially any string?
>
> Tomi
>
>
/usr/lib/xorg/modules/drivers/omapfb_drv.so
from package xserver-xorg-video-omap3 in Debian.
I don't know where the main upstream source is, but here:
https://gitorious.org/gnutoo-s-programs-for-shr/xf86-video-omapfb/source/28c006c94e57ea71df11ec4fff79d7ffcfc4860f:src/omapfb-output-dss.c#L258
is the code which reads
/sys/devices/platform/omapdss/display0/name
and fails if that file cannot be opened.
Thanks,
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: restore "name" sysfs entry.
From: Dr. H. Nikolaus Schaller @ 2015-02-24 17:08 UTC (permalink / raw)
To: NeilBrown, Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, LKML, GTA04 owners
In-Reply-To: <20150224203706.1eafa129@notabene.brown>
I have tested it with
X.Org X Server 1.12.4
Release Date: 2012-08-27
X Protocol Version 11, Revision 0
Build Operating System: Linux 3.2.0-4-mx5 armv7l Debian
(coming from Debian Wheezy) on:
* gta04 (SPI panel)
* openpandora (SPI panel)
* BeagleBoard XM (with RGB panel)
* PandaBoard ES (w/o panel)
* omap5432-evm + Pyra prototype (MIPI panel)
Without this patch, X Server simply fails with:
Fatal server error:
no screens found
So please add
Tested-by: Nikolaus Schaller <hns@goldelico.com>
Thanks,
Nikolaus
Am 24.02.2015 um 10:37 schrieb NeilBrown <neilb@suse.de>:
>
>
> commit 303e4697e762dc92a40405f4e4b8aac02cd0d70b
> OMAPDSS: rename display-sysfs 'name' entry
>
> broke the xorg X server on my device as it couldn't find the display
> any more. It needs the 'name' file and now there isn't one.
>
> That commit claims that 'name' is not compatible with i2c or spi.
> i2c does register it own 'name' file, but spi does not, hence my
> problem - I have an spi display.
>
> So create a special case for i2c: add the name attribute for non-i2c
> devices.
>
> Fixes: 303e4697e762dc92a40405f4e4b8aac02cd0d70b
> Signed-off-by: NeilBrown <neilb@suse.de>
>
> --
>
> Hi Tomi,
> I wonder if you would consider this for the next merge window (or maybe even sooner as a bug-fix).
> I haven't tested it with an i2c display, but it certainly allows xorg to work
> on my spi-attached display.
>
> Thanks,
> NeilBrown
>
>
> diff --git a/drivers/video/fbdev/omap2/dss/display-sysfs.c b/drivers/video/fbdev/omap2/dss/display-sysfs.c
> index 5a2095a98ed8..53897b743130 100644
> --- a/drivers/video/fbdev/omap2/dss/display-sysfs.c
> +++ b/drivers/video/fbdev/omap2/dss/display-sysfs.c
> @@ -25,6 +25,15 @@
> #include <linux/platform_device.h>
> #include <linux/sysfs.h>
>
> +#if IS_ENABLED(CONFIG_I2C)
> +#include <linux/i2c.h>
> +#else
> +static inline int i2c_verify_client(struct device *dev)
> +{
> + return NULL;
> +}
> +#endif
> +
> #include <video/omapdss.h>
> #include "dss.h"
>
> @@ -278,6 +287,7 @@ static ssize_t display_wss_store(struct device *dev,
> }
>
> static DEVICE_ATTR(display_name, S_IRUGO, display_name_show, NULL);
> +static DEVICE_ATTR(name, S_IRUGO, display_name_show, NULL);
> static DEVICE_ATTR(enabled, S_IRUGO|S_IWUSR,
> display_enabled_show, display_enabled_store);
> static DEVICE_ATTR(tear_elim, S_IRUGO|S_IWUSR,
> @@ -315,6 +325,17 @@ int display_init_sysfs(struct platform_device *pdev)
> DSSERR("failed to create sysfs files\n");
> goto err;
> }
> + if (!i2c_verify_client(dssdev->dev)) {
> + /*
> + * Add 'name' file - not compatible with i2c, but
> + * need by Xorg for other devices.
> + */
> + r = sysfs_create_file(kobj, &dev_attr_name.attr);
> + if (r) {
> + DSSERR("fail to create 'name' sysfs file\n");
> + goto err;
> + }
> + }
>
> r = sysfs_create_link(&pdev->dev.kobj, kobj, dssdev->alias);
> if (r) {
> @@ -341,5 +362,7 @@ void display_uninit_sysfs(struct platform_device *pdev)
> sysfs_remove_link(&pdev->dev.kobj, dssdev->alias);
> sysfs_remove_files(&dssdev->dev->kobj,
> display_sysfs_attrs);
> + sysfs_remove_file(&dssdev->dev->kobj,
> + &dev_attr_name.attr);
> }
> }
^ permalink raw reply
* Re: [PATCH] OMAPDSS: restore "name" sysfs entry.
From: Tomi Valkeinen @ 2015-02-24 10:40 UTC (permalink / raw)
To: NeilBrown
Cc: linux-omap, linux-fbdev, linux-kernel, GTA04 owners,
Dr. H. Nikolaus Schaller
In-Reply-To: <20150224203706.1eafa129@notabene.brown>
[-- Attachment #1: Type: text/plain, Size: 758 bytes --]
Hi,
On 24/02/15 11:37, NeilBrown wrote:
>
>
> commit 303e4697e762dc92a40405f4e4b8aac02cd0d70b
> OMAPDSS: rename display-sysfs 'name' entry
>
> broke the xorg X server on my device as it couldn't find the display
> any more. It needs the 'name' file and now there isn't one.
>
> That commit claims that 'name' is not compatible with i2c or spi.
> i2c does register it own 'name' file, but spi does not, hence my
> problem - I have an spi display.
>
> So create a special case for i2c: add the name attribute for non-i2c
> devices.
What X driver is that? What's it doing with the display name? Is it just
using the display name to show something for the user, and the returned
value can be essentially any string?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] OMAPDSS: restore "name" sysfs entry.
From: NeilBrown @ 2015-02-24 9:37 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-omap, linux-fbdev, linux-kernel, GTA04 owners,
Dr. H. Nikolaus Schaller
[-- Attachment #1: Type: text/plain, Size: 2553 bytes --]
commit 303e4697e762dc92a40405f4e4b8aac02cd0d70b
OMAPDSS: rename display-sysfs 'name' entry
broke the xorg X server on my device as it couldn't find the display
any more. It needs the 'name' file and now there isn't one.
That commit claims that 'name' is not compatible with i2c or spi.
i2c does register it own 'name' file, but spi does not, hence my
problem - I have an spi display.
So create a special case for i2c: add the name attribute for non-i2c
devices.
Fixes: 303e4697e762dc92a40405f4e4b8aac02cd0d70b
Signed-off-by: NeilBrown <neilb@suse.de>
--
Hi Tomi,
I wonder if you would consider this for the next merge window (or maybe even sooner as a bug-fix).
I haven't tested it with an i2c display, but it certainly allows xorg to work
on my spi-attached display.
Thanks,
NeilBrown
diff --git a/drivers/video/fbdev/omap2/dss/display-sysfs.c b/drivers/video/fbdev/omap2/dss/display-sysfs.c
index 5a2095a98ed8..53897b743130 100644
--- a/drivers/video/fbdev/omap2/dss/display-sysfs.c
+++ b/drivers/video/fbdev/omap2/dss/display-sysfs.c
@@ -25,6 +25,15 @@
#include <linux/platform_device.h>
#include <linux/sysfs.h>
+#if IS_ENABLED(CONFIG_I2C)
+#include <linux/i2c.h>
+#else
+static inline int i2c_verify_client(struct device *dev)
+{
+ return NULL;
+}
+#endif
+
#include <video/omapdss.h>
#include "dss.h"
@@ -278,6 +287,7 @@ static ssize_t display_wss_store(struct device *dev,
}
static DEVICE_ATTR(display_name, S_IRUGO, display_name_show, NULL);
+static DEVICE_ATTR(name, S_IRUGO, display_name_show, NULL);
static DEVICE_ATTR(enabled, S_IRUGO|S_IWUSR,
display_enabled_show, display_enabled_store);
static DEVICE_ATTR(tear_elim, S_IRUGO|S_IWUSR,
@@ -315,6 +325,17 @@ int display_init_sysfs(struct platform_device *pdev)
DSSERR("failed to create sysfs files\n");
goto err;
}
+ if (!i2c_verify_client(dssdev->dev)) {
+ /*
+ * Add 'name' file - not compatible with i2c, but
+ * need by Xorg for other devices.
+ */
+ r = sysfs_create_file(kobj, &dev_attr_name.attr);
+ if (r) {
+ DSSERR("fail to create 'name' sysfs file\n");
+ goto err;
+ }
+ }
r = sysfs_create_link(&pdev->dev.kobj, kobj, dssdev->alias);
if (r) {
@@ -341,5 +362,7 @@ void display_uninit_sysfs(struct platform_device *pdev)
sysfs_remove_link(&pdev->dev.kobj, dssdev->alias);
sysfs_remove_files(&dssdev->dev->kobj,
display_sysfs_attrs);
+ sysfs_remove_file(&dssdev->dev->kobj,
+ &dev_attr_name.attr);
}
}
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply related
* Re: [PATCH] staging/sm7xxfb: Make 'struct vgamode' static
From: Sudip Mukherjee @ 2015-02-24 5:58 UTC (permalink / raw)
To: Peter Senna Tschudin; +Cc: teddy.wang, gregkh, linux-fbdev, devel, linux-kernel
In-Reply-To: <1424690176-7966-1-git-send-email-peter.senna@gmail.com>
On Mon, Feb 23, 2015 at 12:16:16PM +0100, Peter Senna Tschudin wrote:
> sparse was complaining that symbol 'vgamode' was not declared.
>
> Declaring vgamode as static.
a similar patch has already been sent to Greg, which is not yet applied.
regards
sudip
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> ---
> --
> 2.1.0
>
^ permalink raw reply
* [PATCH] staging/sm7xxfb: Make 'struct vgamode' static
From: Peter Senna Tschudin @ 2015-02-23 11:16 UTC (permalink / raw)
To: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, devel,
linux-kernel
Cc: Peter Senna Tschudin
sparse was complaining that symbol 'vgamode' was not declared.
Declaring vgamode as static.
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
Tested by compilation only
drivers/staging/sm7xxfb/sm7xx.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/sm7xxfb/sm7xx.h b/drivers/staging/sm7xxfb/sm7xx.h
index 7cc1896..c5d6253 100644
--- a/drivers/staging/sm7xxfb/sm7xx.h
+++ b/drivers/staging/sm7xxfb/sm7xx.h
@@ -119,7 +119,7 @@ struct ModeInit {
/**********************************************************************
SM712 Mode table.
**********************************************************************/
-struct ModeInit vgamode[] = {
+static struct ModeInit vgamode[] = {
{
/* mode#0: 640 x 480 16Bpp 60Hz */
640, 480, 16, 60,
--
2.1.0
^ permalink raw reply related
* Re: [PATCH 2/8] fbdev: ssd1307fb: Unify init code and make controller configurable from device tree
From: Maxime Ripard @ 2015-02-23 9:43 UTC (permalink / raw)
To: Thomas Niederprüm
Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150214171232.11ccb264@maestro.intranet>
[-- Attachment #1: Type: text/plain, Size: 9563 bytes --]
On Sat, Feb 14, 2015 at 05:12:32PM +0100, Thomas Niederprüm wrote:
> Am Thu, 12 Feb 2015 17:41:47 +0100
> schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
>
> > On Sat, Feb 07, 2015 at 03:59:33PM +0100, Thomas Niederprüm wrote:
> > > Am Sat, 7 Feb 2015 11:42:25 +0100
> > > schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> > >
> > > > 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,
> > >
> > > Unfortunately this is true. The problem is that the SSD130X
> > > controllers allow for a very versatile wiring of the display to the
> > > controller. It's over to the manufacturer of the OLED module
> > > (disp+controller) to decide how it's actually wired and during
> > > device initialization the driver has to take care to configure the
> > > SSD130X controller according to that wiring. If the driver fails to
> > > do so you will end up having your display showing
> > > garbage.
> >
> > How so?
>
> One good example is the segment remap. It basically allows to invert the
> order of the output pins connecting to the oled panel. This gives the
> manufacturer of the module the freedom wire it the one way or the
> other, depending on the PCB restrictions/panel layout (Section 10.1.12
> of [0], 10.1.8 in [1], 9.1.8 in [2]). However, once the panel is
> connected to the controller it's determined whether the segment remap
> is needed or not. Setting the segment remap as done in the current
> initialization code for ssd1306 and ssd1307 makes my display module
> show it's contents mirrored left to right, probably since the
> manufacturer decided not to connect the panel in an inverted order.
>
> The same applies to the com-alt, com-lrremap and com-invdir values,
> which define different possibilities for the COM signals pin
> configuration (Section 10.1.26 of [0], 10.1.18 in [1], 9.1.18 in [2])
> and readout direction of the video memory (Section 10.1.21 of [0],
> 10.1.14 in [1], 9.1.14 in [2]). Setting com-alt incorrectly leaves
> every other line of the display blank. Setting com-lrremap incorrectly
> produces a very distorted image. Setting com-invdir incorrectly flips
> the image upside down.
>
> IMHO at least these four hardware-specific properties need to be known
> to the driver in order to initialize the hardware correctly.
I'd agree then.
> > Does it depend on the X, or can it change from one same controller to
> > another? to what extent?
>
> Unfortunately I do not posses any hardware utilizing a ssd1306 or
> ssd1307 controller. My primary and only target device is a Newhaven
> NHD-3.12-25664UCB2 OLED display module using an SSD1305 controller. I
> just inferred from the datasheets of ssd1306/7 [1,2] that they should
> behave the same since the registers are bit to bit identical (except
> for the VHCOM register). Maybe that was a bit too naive :/
I would guess it's a rather safe assumption.
> > The 1306 for example seems to not be using these values at all, while
> > the 1307 does.
>
> That is surprising. In that case I would like to ask the guys from
> Solomon why they describe all these options in the SSD1306 datasheet
> [1]. But in any case, isn't that good news for the problem of setting
> the default values. When the 1306 isn't using these values anyway we can
> not break the initialization by setting different default values. In
> this case the problem of the default values boils down to the segment
> remap only since this is set in the init code of the 1307, while the
> default would be to leave it off.
Indeed.
> > > Unfortunately the current sate of the initialization code of the
> > > ssd1307fb driver is not very flexible in that respect. Taking a look
> > > at the initialization code for the ssd1306 shows that it was written
> > > with one very special display module in mind. Most of the magic bit
> > > values set there are non-default values according to the
> > > datasheet. The result is that the driver works with that one
> > > particular display module but many other (differently wired) display
> > > modules using a ssd1306 controller won't work without changing the
> > > hardcoded magic bit values.
> > >
> > > My idea here was to set all configuration to the default values (as
> > > given in the datasheet) unless it is overwritten by DT. Of course,
> > > without a change in DT, this breaks the driver for all existing
> > > users. The only alternative would be to set the current values as
> > > default. Somehow this feels wrong to me as these values look
> > > arbitrary when you don't know what exact display module they were
> > > set for. But if you insist, I will change the default values.
> >
> > Unfortunately, the DT bindings are to be considered an ABI, and we
> > should support booting with older DTs (not that I personally care
> > about it, but that's another issue). So we really don't have much
> > choice here.
> >
> > Moreover, that issue left aside, modifying bindings like this without
> > fixing up the in-tree users is considered quite rude :)
>
> I didn't intend to be rude, sorry. A quick search revealed that there
> is luckily only one in-tree user, which is imx28-cfa10036.dts. In case
> it will be necessary I will include a patch to fix this.
Please do (and fix the bindings Documentation too).
> > > > and the DT itself should not contain any direct mapping of the
> > > > registers.
> > > >
> > >
> > > I think I don't get what you mean here. Is it because I do no sanity
> > > checks of the numbers set in DT? I was just looking for a way to
> > > hand over the information about the wiring of display to the
> > > driver. How would you propose to solve this?
> >
> > What I meant was that replicating direct registers value is usually a
> > recipe for a later failure, especially if we can have the information
> > under a generic and easy to understand manner.
> >
> > For example, replacing the solomon,dclk-div and solomon,dclk-frq
> > properties by a clock-frequency property in Hz, and computing the
> > divider and that register in your driver is usually better, also
> > because it allows to have different requirements / algorithms to
> > compute that if some other chip needs it.
>
> I'll give that a try, even though that particular one is not trivial
> since the documentation on the actual frequency that is set by the
> dclk-freq is very poor (not present for 1306/1307 [1,2], just a graph
> for 1305 [0]).
>
> For the properties describing the hardware pin configuration (see above)
> I see no real alternative. Maybe they can all be covered by one DT
> property like:
> solomon,com-cfg = PINCFG_SEGREMAP | PINCFG_COMALT | PINCFG_COMINV |
> PINCFG_COMLRRM
> each PINCFG_* setting one bit. The driver will then translate this into
> the correct settings for the 130X registers. The only problem here is
> that this implicitly assumes the default values of each bit to be 0.
A property that would be here or not is better. You can have all the
defaults you want, it's more clear in the DT, and you don't need the
macros.
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
* CONTRACT SUPPLY.
From: Ets Alex Global @ 2015-02-21 15:20 UTC (permalink / raw)
To: linux-fbdev
Dear Sir/Madam,
We are here to introduce our company as a registered Importing company
based in Supplying and Distributing of all kind of goods.with
registration endorsement under the Togo Lome Commerce and Industry.
Our company has been into Importation and distribution of goods for
many years with our international business experience which has given
us the confident to open Three branches offices out side Togo
Republic.
Recently we have being ask by African Trade Union Organization to
source out a reliable firm who will be capable to handle a contract
supply of your products.
We came across your company contact information as a supplier
manufacturer and exporter of above mentioned in your region,With due
respect we request you to please send your prices quotation for our
study in proceedings.
Looking forward to establish a long lasting business with your company.
Regards
Alex Camara.
Alex Global Company Limited
232 Rue Tambina
Phone:+228-90715121.
Lome-Togo.
^ permalink raw reply
* imxfb does not turn on automatically
From: Fabio Estevam @ 2015-02-21 15:18 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Carlos Soto uses a Karo mx25 and he noticed that the LCD only works if we do:
echo 1 > /sys/class/graphics/fb0/blank
echo 0 > /sys/class/graphics/fb0/blank
I can confirm the same on mx25pdk running 3.19 [1], but I could not
understand why the display does not start turned on by default.
[1] http://www.spinics.net/lists/arm-kernel/msg399949.html
Any ideas?
Thanks,
Fabio Estevam
^ permalink raw reply
* [PATCH] video: fbdev-SIS: Deletion of unnecessary checks before three function calls
From: SF Markus Elfring @ 2015-02-21 13:26 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Thomas Winischhofer,
Tomi Valkeinen, linux-fbdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5317A59D.4@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 21 Feb 2015 14:14:56 +0100
The functions sisfb_search_crt2type(), sisfb_search_specialtiming() and
sisfb_search_tvstd() test whether their argument is NULL and then
return immediately. Thus the test around their calls is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/video/fbdev/sis/sis_main.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/video/fbdev/sis/sis_main.c b/drivers/video/fbdev/sis/sis_main.c
index fcf610e..20ca80f 100644
--- a/drivers/video/fbdev/sis/sis_main.c
+++ b/drivers/video/fbdev/sis/sis_main.c
@@ -6605,12 +6605,9 @@ static int __init sisfb_init_module(void)
sisfb_scalelcd = scalelcd ^ 1;
/* Need to check crt2 type first for fstn/dstn */
+ sisfb_search_crt2type(forcecrt2type);
- if(forcecrt2type)
- sisfb_search_crt2type(forcecrt2type);
-
- if(tvstandard)
- sisfb_search_tvstd(tvstandard);
+ sisfb_search_tvstd(tvstandard);
if(mode)
sisfb_search_mode(mode, false);
@@ -6656,9 +6653,7 @@ static int __init sisfb_init_module(void)
sisfb_pdca = (pdc1 & 0x1f);
sisfb_nocrt2rate = nocrt2rate;
-
- if(specialtiming)
- sisfb_search_specialtiming(specialtiming);
+ sisfb_search_specialtiming(specialtiming);
if((lvdshl >= 0) && (lvdshl <= 3))
sisfb_lvdshl = lvdshl;
--
2.3.0
^ permalink raw reply related
* [PATCH] fbdev: goldfishfb: use devres api
From: Prabhakar Lad @ 2015-02-20 17:51 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: linux-fbdev, linux-kernel, Lad, Prabhakar
From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
this patch does the following:
a> uses devm_kzalloc() instead of kzalloc and cleanup the error path
b> uses devm_ioremap() instead of ioremap and cleanup the error path
c> uses devm_request_irq() instead of request_irq and cleanup the error path
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
Note: This patch is compile tested only and applies on linux-next.
drivers/video/fbdev/goldfishfb.c | 61 +++++++++++++++-------------------------
1 file changed, 22 insertions(+), 39 deletions(-)
diff --git a/drivers/video/fbdev/goldfishfb.c b/drivers/video/fbdev/goldfishfb.c
index 7f6c9e6..841514d 100644
--- a/drivers/video/fbdev/goldfishfb.c
+++ b/drivers/video/fbdev/goldfishfb.c
@@ -188,31 +188,25 @@ static int goldfish_fb_probe(struct platform_device *pdev)
u32 width, height;
dma_addr_t fbpaddr;
- fb = kzalloc(sizeof(*fb), GFP_KERNEL);
- if (fb = NULL) {
- ret = -ENOMEM;
- goto err_fb_alloc_failed;
- }
+ fb = devm_kzalloc(&pdev->dev, sizeof(*fb), GFP_KERNEL);
+ if (fb = NULL)
+ return -ENOMEM;
+
spin_lock_init(&fb->lock);
init_waitqueue_head(&fb->wait);
platform_set_drvdata(pdev, fb);
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (r = NULL) {
- ret = -ENODEV;
- goto err_no_io_base;
- }
- fb->reg_base = ioremap(r->start, PAGE_SIZE);
- if (fb->reg_base = NULL) {
- ret = -ENOMEM;
- goto err_no_io_base;
- }
+ if (r = NULL)
+ return -ENODEV;
+
+ fb->reg_base = devm_ioremap(&pdev->dev, r->start, PAGE_SIZE);
+ if (fb->reg_base = NULL)
+ return -ENOMEM;
fb->irq = platform_get_irq(pdev, 0);
- if (fb->irq <= 0) {
- ret = -ENODEV;
- goto err_no_irq;
- }
+ if (fb->irq <= 0)
+ return -ENODEV;
width = readl(fb->reg_base + FB_GET_WIDTH);
height = readl(fb->reg_base + FB_GET_HEIGHT);
@@ -249,43 +243,34 @@ static int goldfish_fb_probe(struct platform_device *pdev)
&fbpaddr, GFP_KERNEL);
pr_debug("allocating frame buffer %d * %d, got %p\n",
width, height, fb->fb.screen_base);
- if (fb->fb.screen_base = NULL) {
- ret = -ENOMEM;
- goto err_alloc_screen_base_failed;
- }
+ if (fb->fb.screen_base = NULL)
+ return -ENOMEM;
+
fb->fb.fix.smem_start = fbpaddr;
fb->fb.fix.smem_len = framesize;
ret = fb_set_var(&fb->fb, &fb->fb.var);
if (ret)
- goto err_fb_set_var_failed;
+ goto error;
- ret = request_irq(fb->irq, goldfish_fb_interrupt, IRQF_SHARED,
- pdev->name, fb);
+ ret = devm_request_irq(&pdev->dev, fb->irq, goldfish_fb_interrupt,
+ IRQF_SHARED, pdev->name, fb);
if (ret)
- goto err_request_irq_failed;
+ goto error;
writel(FB_INT_BASE_UPDATE_DONE, fb->reg_base + FB_INT_ENABLE);
goldfish_fb_pan_display(&fb->fb.var, &fb->fb); /* updates base */
ret = register_framebuffer(&fb->fb);
if (ret)
- goto err_register_framebuffer_failed;
+ goto error;
+
return 0;
-err_register_framebuffer_failed:
- free_irq(fb->irq, fb);
-err_request_irq_failed:
-err_fb_set_var_failed:
+error:
dma_free_coherent(&pdev->dev, framesize,
(void *)fb->fb.screen_base,
fb->fb.fix.smem_start);
-err_alloc_screen_base_failed:
-err_no_irq:
- iounmap(fb->reg_base);
-err_no_io_base:
- kfree(fb);
-err_fb_alloc_failed:
return ret;
}
@@ -296,11 +281,9 @@ static int goldfish_fb_remove(struct platform_device *pdev)
framesize = fb->fb.var.xres_virtual * fb->fb.var.yres_virtual * 2;
unregister_framebuffer(&fb->fb);
- free_irq(fb->irq, fb);
dma_free_coherent(&pdev->dev, framesize, (void *)fb->fb.screen_base,
fb->fb.fix.smem_start);
- iounmap(fb->reg_base);
return 0;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v2] video: fbdev: use msecs_to_jiffies for time conversions
From: Nicholas Mc Guire @ 2015-02-20 14:34 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 by
replacing var * HZ / 1000 by msecs_to_jiffies(var).
Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---
v2: fixed missing closing parenthesis in pxafb_disable_controller.
Compile testing was missing part of the patched code due to
the missing CONFIG_FB_PXA_SMARTPANEL=y, so the missing closing
parenthesis reported by Tomi Valkeinen <tomi.valkeinen@ti.com>
pxafb_disable_controller went unnoticed.
Patch was only compile tested with viper_defconfig CONFIG_FB_PXA=m,
CONFIG_FB_PXA_SMARTPANEL=y
Patch is against 3.19.0 (localversion-next is -next-20150220)
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] video: fbdev: use msecs_to_jiffies for time conversion
From: Nicholas Mc Guire @ 2015-02-20 14:20 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Nicholas Mc Guire, Jean-Christophe Plagniol-Villard, Jingoo Han,
Daniel Vetter, Fabian Frederick, Laurent Pinchart, Wolfram Sang,
linux-fbdev, linux-kernel
In-Reply-To: <54E73F78.3050009@ti.com>
On Fri, 20 Feb 2015, Tomi Valkeinen wrote:
> On 20/02/15 16:01, Nicholas Mc Guire wrote:
>
> >>> 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);
> >>
> >> That will not compile.
>
> > No compile warning or errors
> >
> > could you send me the compile error message and the toolchain you
> > are using - this change should not really have any noticable impact.
>
> I didn't compile it. It's missing a closing parenthesis.
>
> This is one reason I'm not very fond of cleanups to drivers that the
> patch sender cannot test... They may cause more problems than they help.
>
That was the intent of compile testing the patch but it seems
that it was not covered by the config in use - sorry for that, it was
missing the [*] PXA Smartpanel LCD support so
root@debian:~/linux-next# grep CONFIG_FB_PXA_SMARTPANEL .config
# CONFIG_FB_PXA_SMARTPANEL is not set
The motivation for the cleanup simply was code reasdability - will fix this up
and compile check with a proper config.
thx!
hofrat
^ permalink raw reply
* Re: [PATCH] video: fbdev: use msecs_to_jiffies for time conversion
From: Tomi Valkeinen @ 2015-02-20 14:06 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: Nicholas Mc Guire, Jean-Christophe Plagniol-Villard, Jingoo Han,
Daniel Vetter, Fabian Frederick, Laurent Pinchart, Wolfram Sang,
linux-fbdev, linux-kernel
In-Reply-To: <20150220140110.GA25767@opentech.at>
[-- Attachment #1: Type: text/plain, Size: 810 bytes --]
On 20/02/15 16:01, Nicholas Mc Guire wrote:
>>> 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);
>>
>> That will not compile.
> No compile warning or errors
>
> could you send me the compile error message and the toolchain you
> are using - this change should not really have any noticable impact.
I didn't compile it. It's missing a closing parenthesis.
This is one reason I'm not very fond of cleanups to drivers that the
patch sender cannot test... They may cause more problems than they help.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ 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