From: Andrew Morton on
On Tue, 1 Jun 2010 22:27:15 +0200
Daniel Mack <daniel(a)caiaq.de> wrote:

> On Tue, Jun 01, 2010 at 01:12:44PM -0700, Andrew Morton wrote:
> > On Mon, 24 May 2010 16:34:25 +0530 (IST)
> > "Hemanth V" <hemanthv(a)ti.com> wrote:
> >
> > > This patch adds support for ROHM BH1780GLI Ambient light sensor.
> > >
> > > BH1780 supports I2C interface. Driver supports read/update of power state and
> > > read of lux value (through SYSFS). Writing value 3 to power_state enables the
> > > sensor and current lux value could be read.
> >
> > There are at least two other ambient light sensor drivers:
> > drivers/misc/isl29003.c and drivers/misc/tsl2550.c.
> >
> > Is there any standardisation of the ABIs whcih these drivers offer? If
> > so, does this new driver comply with that?
>
> Jonathan proposed the ALS framework for these type of devices, but it
> was rejected (don't know about the reasons, I didn't follow the
> discussions). The new idea is to put such drivers in the industrial IO
> subsystem, but I don't know how mature that approach is currently.
>
> For the time being, these drivers cook up whatever sysfs interface they
> like, and their userspace ABIs are not standardized, unfortunately.

Well can we fix that? Look at the existing drivers, pick one and make
this new driver provide the same interface?

It's not a grand plan - more of an incremental baby step, but it's
better than this random proliferation.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jonathan Cameron on
On 06/01/10 21:27, Daniel Mack wrote:
> On Tue, Jun 01, 2010 at 01:12:44PM -0700, Andrew Morton wrote:
>> On Mon, 24 May 2010 16:34:25 +0530 (IST)
>> "Hemanth V" <hemanthv(a)ti.com> wrote:
>>
>>> This patch adds support for ROHM BH1780GLI Ambient light sensor.
>>>
>>> BH1780 supports I2C interface. Driver supports read/update of power state and
>>> read of lux value (through SYSFS). Writing value 3 to power_state enables the
>>> sensor and current lux value could be read.
>>
>> There are at least two other ambient light sensor drivers:
>> drivers/misc/isl29003.c and drivers/misc/tsl2550.c.
>>
>> Is there any standardisation of the ABIs whcih these drivers offer? If
>> so, does this new driver comply with that?
>
> Jonathan proposed the ALS framework for these type of devices, but it
> was rejected (don't know about the reasons, I didn't follow the
> discussions).
Ah the wonder of emails crossing ;)

For the interested... http://lkml.org/lkml/2010/3/1/367
(the main objections are Linus' email)
> The new idea is to put such drivers in the industrial IO
> subsystem, but I don't know how mature that approach is currently.
We haven't lifted any that didn't start there as moving things into staging
due to a dependency being there seemed a controversial thing to do.
>
> For the time being, these drivers cook up whatever sysfs interface they
> like, and their userspace ABIs are not standardized, unfortunately.
>
>> It would be most useful if the changelog were to fully describe the
>> proposed kernel<->userspace interface. That's the most important part
>> of the driver, because it's the only part we can never change.
>>
>> There is a desultory effort to maintain sysfs API descriptions under
>> Documentation/ABI/. I'd have thought that it would be appropriate to
>> document this driver's ABI in there.
>
> FWIW, I put some documentation about the isl29003 to
> Documentation/misc-devices when I wrote the driver.
>
> Daniel
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on
On Tue, 01 Jun 2010 21:39:10 +0100
Jonathan Cameron <kernel(a)jic23.retrosnub.co.uk> wrote:

> >
> > It would be most useful if the changelog were to fully describe the
> > proposed kernel<->userspace interface. That's the most important part
> > of the driver, because it's the only part we can never change.
> >
> > There is a desultory effort to maintain sysfs API descriptions under
> > Documentation/ABI/. I'd have thought that it would be appropriate to
> > document this driver's ABI in there.
> Agreed, but we get back to the debate of what we should standardise on.

I'd suggest standardising on one of the existing drivers. That way we
have two compliant drivers and only need to change (n-2) others. If we
pick some new standard then we need to change (n) drivers.

And we can't change the drivers, really. They'd all end up needing to
provide two interfaces: one for the shiny-new-standard and one legacy.

> The main point of ALS before it died was exactly putting this standardization
> in place (admittedly the interface was slightly different from what we
> are proposing in IIO, but that was before Greg pointed out that sharing
> with hwmon would be a good idea!)
>
> I have to admit I'm a little loath to spend too much time on this given
> the amount of time wasted previously (ALS).

Well, it's not a waste. This is very important! We appear to be
making a big mess which we can never fix up.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jonathan Cameron on
On 06/01/10 21:54, Andrew Morton wrote:
> On Tue, 01 Jun 2010 21:39:10 +0100
> Jonathan Cameron <kernel(a)jic23.retrosnub.co.uk> wrote:
>
>>>
>>> It would be most useful if the changelog were to fully describe the
>>> proposed kernel<->userspace interface. That's the most important part
>>> of the driver, because it's the only part we can never change.
>>>
>>> There is a desultory effort to maintain sysfs API descriptions under
>>> Documentation/ABI/. I'd have thought that it would be appropriate to
>>> document this driver's ABI in there.
>> Agreed, but we get back to the debate of what we should standardise on.
>
> I'd suggest standardising on one of the existing drivers. That way we
> have two compliant drivers and only need to change (n-2) others. If we
> pick some new standard then we need to change (n) drivers.
I agree but take into account that we are getting a number of superficially
similar interfaces in kernel (hwmon, various sensors in misc and IIO)
and it makes sense to my mind to share interfaces across these where
possible (this is exactly the argument Greg made when we carried the
equivalent standardization out in IIO - for ref, the spec is in
staging/drivers/iio/Documentation/sysfs-class-iio)
(gah, the name needs a change to reflect our move to a sysfs bus - oops)
>
> And we can't change the drivers, really. They'd all end up needing to
> provide two interfaces: one for the shiny-new-standard and one legacy.
True enough. We argued we could do with out this before because we were
fairly sure that we knew everyone who was using the sensors (also 1 was in staging
and another extremely)
>
>> The main point of ALS before it died was exactly putting this standardization
>> in place (admittedly the interface was slightly different from what we
>> are proposing in IIO, but that was before Greg pointed out that sharing
>> with hwmon would be a good idea!)
>>
>> I have to admit I'm a little loath to spend too much time on this given
>> the amount of time wasted previously (ALS).
>
> Well, it's not a waste. This is very important! We appear to be
> making a big mess which we can never fix up.
I agree entirely, picking a driver to copy is the way to go.
(personally I'd argue for the tsl2563 for the reasons above)
I'm just moaning about the time wasted the previous time we tried to clean
this stuff up :)

Note that 'if' these drivers do end up in IIO we will have to add the abi
as per the tsl2563 (can keep the others around for a while). We have a consistent
abi over 6+ sensor types so far and we aren't going to want to break it for
this one. There are still unresolved corners in our abi but
they are all event related so this device is nowhere near them.

Either way, I agree entirely that matching any existing driver is a good thing
even if it isn't the one I'd personally prefer!

Jonathan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Hemanth V on
----- Original Message -----
From: "Andrew Morton" <akpm(a)linux-foundation.org>
To: "Jonathan Cameron" <kernel(a)jic23.retrosnub.co.uk>
Cc: "Hemanth V" <hemanthv(a)ti.com>; <linux-kernel(a)vger.kernel.org>;
<linux-omap(a)vger.kernel.org>; "Daniel Mack" <daniel(a)caiaq.de>; "Jonathan
Cameron" <jic23(a)cam.ac.uk>; "Wolfram Sang" <w.sang(a)pengutronix.de>
Sent: Wednesday, June 02, 2010 2:24 AM
Subject: Re: [PATCH V2] misc : ROHM BH1780GLI Ambient light sensor Driver


> On Tue, 01 Jun 2010 21:39:10 +0100
> Jonathan Cameron <kernel(a)jic23.retrosnub.co.uk> wrote:
>
>> >
>> > It would be most useful if the changelog were to fully describe the
>> > proposed kernel<->userspace interface. That's the most important part
>> > of the driver, because it's the only part we can never change.
>> >
>> > There is a desultory effort to maintain sysfs API descriptions under
>> > Documentation/ABI/. I'd have thought that it would be appropriate to
>> > document this driver's ABI in there.
>> Agreed, but we get back to the debate of what we should standardise on.
>
> I'd suggest standardising on one of the existing drivers. That way we
> have two compliant drivers and only need to change (n-2) others. If we
> pick some new standard then we need to change (n) drivers.
>

Currently this driver follows the same sysfs convention as supported
by isl29003.c in drivers/misc.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/