diff mbox

OMAPDSS: restore "name" sysfs entry.

Message ID 54ED8CB6.4010308@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tomi Valkeinen Feb. 25, 2015, 8:49 a.m. UTC
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(-)

Comments

NeilBrown Feb. 25, 2015, 9:20 a.m. UTC | #1
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
Tomi Valkeinen Feb. 25, 2015, 9:32 a.m. UTC | #2
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
NeilBrown Feb. 25, 2015, 10:33 p.m. UTC | #3
On Wed, 25 Feb 2015 11:32:18 +0200 Tomi Valkeinen <tomi.valkeinen@ti.com>
wrote:


> 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.

How close is that?  Is it worth experimenting yet?  Is there an xorg driver
available?  Maybe a wiki page telling me how to set it up?

http://processors.wiki.ti.com/index.php/Linux_Core_DSS_User%27s_Guide

seems to have some useful suggestions, but no mention of Xorg...

Thanks,
NeilBrown

> 
> 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
> 
>
diff mbox

Patch

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;