[Gta04-owner] Has dss 'name' attribute fix been discussed upstream.

[NeilBrown]

On Tue, 24 Feb 2015 08:31:22 +0100 "Dr. H. Nikolaus Schaller"
<hns at goldelico.com> wrote:

> Hi Neil,
> 
> Am 24.02.2015 um 04:10 schrieb NeilBrown <neilb at suse.de>:
> 
> > 
> > Hi Nikolaus,
> > I note you have a patch in your tree:
> > 
> > commit 31d50da901ea07db434ca0a299bf13372b8b5534
> > Author: H. Nikolaus Schaller <hns at goldelico.com>
> > Date:   Thu May 22 14:22:27 2014 +0200
> > 
> >    dss: restore compatibility to pre-3.13 xorg systems by partially reverting 3
> > 
> >    Signed-off-by: H. Nikolaus Schaller <hns at goldelico.com>
> > 
> 
> Yes, it was a hack to get it working - without deeper analysis.
> 
> > 
> > I would like to get a fix for this issue upstream.
> > Have you discussed this issue at all with Tomi?
> 
> I think there was one or two mails and if I remember correctly, it was his hint
> to know what had been changed.

Nothing really substantial then - OK.

> 
> > 
> > I'm happy to start a discussion, but if something has already been said I'd
> > like to know all the background before I blunder in.
> > 
> > my proposed patch is below.
> > 
> > Thanks,
> > NeilBrown
> > 
> > From: NeilBrown <neilb at suse.de>
> > Date: Mon, 23 Feb 2015 16:10:55 +1100
> > Subject: [PATCH] OMAPDSS: restore "name" sysfs entry.
> > 
> > commit 303e4697e762dc92a40405f4e4b8aac02cd0d70b
> > Author: Tomi Valkeinen <tomi.valkeinen at ti.com>
> > Date:   Tue Sep 24 17:00:14 2013 +0300
> > 
> >    OMAPDSS: rename display-sysfs 'name‘ entry
> 
> maybe you could add the commit hash

It's right there after the word 'commit' :-)


> 
> > 
> > Broke the X server on my device as it couldn't find the display
> > any more.
> > 
> > That commit claim that 'name' is not compatible with i2c or spi.
> > i2c does register it own 'name' file, but spi does not, hence my
> > problem.
> 
> > 
> > So create a special case for i2c: add the name attribute for non-i2c
> > devices.
> > 
> 
> What I do not clearly understand (from your description) is the relation
> to i2c. AFAIK, there are no panels using I2C? So what role does i2c
> play here?

% grep i2c `git grep -l omapdss_register_display`

suggests that 

  drivers/video/fbdev/omap2/displays-new/connector-dvi.c

is an i2c device which devices as a dss display.

That was added Fri May 24 14:20:45 2013 +0300

The 'name' attribute was changed

   Tue Sep 24 17:00:14 2013 +0300

4 months later.  There could be a connection.



> 
> And we must take care that it does not break on systems we don’t know.

Always a problem ... and of course that is what the offending patch did.

Thanks,
NeilBrown


> 
> for those I know it is:
> GTA04:	RGB, SPI
> OpenPandora:	RGB, SPI
> L3704/L7004:	RGB, neither I2C nor SPI
> Pyra: MIPI, neither I2C nor SPI
> 
> BR,
> Nikolaus
> 
> 
> > Signed-off-by: NeilBrown <neilb at suse.de>
> > 
> > diff --git a/drivers/video/fbdev/omap2/dss/display-sysfs.c b/drivers/video/fbdev/omap2/dss/display-sysfs.c
> > index 5a2095a98ed8..dff6c6764eec 100644
> > --- a/drivers/video/fbdev/omap2/dss/display-sysfs.c
> > +++ b/drivers/video/fbdev/omap2/dss/display-sysfs.c
> > @@ -24,6 +24,7 @@
> > #include <linux/module.h>
> > #include <linux/platform_device.h>
> > #include <linux/sysfs.h>
> > +#include <linux/i2c.h>
> > 
> > #include <video/omapdss.h>
> > #include "dss.h"
> > @@ -278,6 +279,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 +317,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 +354,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);
> > 	}
> > }

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.goldelico.com/pipermail/gta04-owner/attachments/20150224/a49de907/attachment-0001.asc>