as3645a flash userland interface

as3645a flash userland interface

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 482 bytes --]

Hi!

There were some changes to as3645a flash controller. Before we have
stable interface we have to keep forever I want to ask:

What directory are the flash controls in?

/sys/class/leds/led-controller:flash ?

Could we arrange for something less generic, like

/sys/class/leds/main-camera:flash ?

Thanks,

								Pavel
 
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: as3645a flash userland interface

[Sakari Ailus]

Hi Pavel,

On Tue, Sep 12, 2017 at 12:36:28PM +0200, Pavel Machek wrote:
> Hi!
> 
> There were some changes to as3645a flash controller. Before we have
> stable interface we have to keep forever I want to ask:
> 
> What directory are the flash controls in?
> 
> /sys/class/leds/led-controller:flash ?
> 
> Could we arrange for something less generic, like
> 
> /sys/class/leds/main-camera:flash ?
> 
> Thanks,

The LEDs are called as3645a:flash and as3645a:indicator currently, based on
the name of the LED controller's device node. There are no patches related
to this set though; these have already been merged.

The label should be a "human readable string describing the device" (from
ePAPR, please excuse me for not having a newer spec), and the led common
bindings define it as:

- label : The label for this LED. If omitted, the label is taken from the node
          name (excluding the unit address). It has to uniquely identify
          a device, i.e. no other LED class device can be assigned the same
          label.

I don't think that you should be looking to use this to associate it with
the camera as such. The association information with the sensor is
available to the kernel but there's no interface that could meaningfully
expose it to the user right now.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi

Re: as3645a flash userland interface

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2049 bytes --]

Hi!

> On Tue, Sep 12, 2017 at 12:36:28PM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > There were some changes to as3645a flash controller. Before we have
> > stable interface we have to keep forever I want to ask:
> > 
> > What directory are the flash controls in?
> > 
> > /sys/class/leds/led-controller:flash ?
> > 
> > Could we arrange for something less generic, like
> > 
> > /sys/class/leds/main-camera:flash ?
> > 
> > Thanks,
> 
> The LEDs are called as3645a:flash and as3645a:indicator currently, based on
> the name of the LED controller's device node. There are no patches related
> to this set though; these have already been merged.
> 
> The label should be a "human readable string describing the device" (from
> ePAPR, please excuse me for not having a newer spec), and the led common
> bindings define it as:
> 
> - label : The label for this LED. If omitted, the label is taken from the node
>           name (excluding the unit address). It has to uniquely identify
>           a device, i.e. no other LED class device can be assigned the same
>           label.

Ok, can we set the label to "main_camera" for N9 and n950 cases?

"as3645a:flash" is really wrong name for a LED. Information that
as3645 is already present elsewhere in /sys. Information where the LED
is and what it does is not.

I'd like to have torch application that just writes
/sys/class/leds/main_camera:white:flash/brightness . It should not
need to know hardware details of differnet phones.

> I don't think that you should be looking to use this to associate it with
> the camera as such. The association information with the sensor is
> available to the kernel but there's no interface that could meaningfully
> expose it to the user right now.

Yeah, I'm not looking for sensor association. I'm looking for
reasonable userland interface.

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: as3645a flash userland interface

[Sakari Ailus]

Ahoy!

On Tue, Sep 12, 2017 at 01:40:51PM +0200, Pavel Machek wrote:
> Hi!
> 
> > On Tue, Sep 12, 2017 at 12:36:28PM +0200, Pavel Machek wrote:
> > > Hi!
> > > 
> > > There were some changes to as3645a flash controller. Before we have
> > > stable interface we have to keep forever I want to ask:
> > > 
> > > What directory are the flash controls in?
> > > 
> > > /sys/class/leds/led-controller:flash ?
> > > 
> > > Could we arrange for something less generic, like
> > > 
> > > /sys/class/leds/main-camera:flash ?
> > > 
> > > Thanks,
> > 
> > The LEDs are called as3645a:flash and as3645a:indicator currently, based on
> > the name of the LED controller's device node. There are no patches related
> > to this set though; these have already been merged.
> > 
> > The label should be a "human readable string describing the device" (from
> > ePAPR, please excuse me for not having a newer spec), and the led common
> > bindings define it as:
> > 
> > - label : The label for this LED. If omitted, the label is taken from the node
> >           name (excluding the unit address). It has to uniquely identify
> >           a device, i.e. no other LED class device can be assigned the same
> >           label.
> 
> Ok, can we set the label to "main_camera" for N9 and n950 cases?
> 
> "as3645a:flash" is really wrong name for a LED. Information that
> as3645 is already present elsewhere in /sys. Information where the LED
> is and what it does is not.
> 
> I'd like to have torch application that just writes
> /sys/class/leds/main_camera:white:flash/brightness . It should not
> need to know hardware details of differnet phones.

Hmm. There don't seem to be a uniform way to form labels.

What I'd do is to look up a LED that implements LED flash class and use
that; it's a flash LED and is likely to be the most powerful in the system.
There could be several as well, some more recent flash controllers have
more than one.

I wonder what Jacek thinks.

> 
> > I don't think that you should be looking to use this to associate it with
> > the camera as such. The association information with the sensor is
> > available to the kernel but there's no interface that could meaningfully
> > expose it to the user right now.
> 
> Yeah, I'm not looking for sensor association. I'm looking for
> reasonable userland interface.

Ack. Hopefully we can provide the association some day, too...

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi

Re: as3645a flash userland interface

[Jacek Anaszewski]

Hi Pavel,

On 09/12/2017 12:36 PM, Pavel Machek wrote:
> Hi!
> 
> There were some changes to as3645a flash controller. Before we have
> stable interface we have to keep forever I want to ask:

Note that we have already two LED flash class drivers - leds-max77693
and leds-aat1290. They have been present in mainline for over two years
now.

> What directory are the flash controls in?
> 
> /sys/class/leds/led-controller:flash ?
> 
> Could we arrange for something less generic, like
> 
> /sys/class/leds/main-camera:flash ?

I'd rather avoid overcomplicating this. LED class device name pattern
is well defined to devicename:colour:function
(see Documentation/leds/leds-class.txt, "LED Device Naming" section).

In this case "flash" in place of the "function" segment makes the
things clear enough I suppose.

-- 
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: as3645a flash userland interface

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]

On Tue 2017-09-12 20:53:33, Jacek Anaszewski wrote:
> Hi Pavel,
> 
> On 09/12/2017 12:36 PM, Pavel Machek wrote:
> > Hi!
> > 
> > There were some changes to as3645a flash controller. Before we have
> > stable interface we have to keep forever I want to ask:
> 
> Note that we have already two LED flash class drivers - leds-max77693
> and leds-aat1290. They have been present in mainline for over two years
> now.

Well.. that's ok. No change there is neccessary.

> > What directory are the flash controls in?
> > 
> > /sys/class/leds/led-controller:flash ?
> > 
> > Could we arrange for something less generic, like
> > 
> > /sys/class/leds/main-camera:flash ?
> 
> I'd rather avoid overcomplicating this. LED class device name pattern
> is well defined to devicename:colour:function
> (see Documentation/leds/leds-class.txt, "LED Device Naming" section).
> 
> In this case "flash" in place of the "function" segment makes the
> things clear enough I suppose.

It does not.

Phones usually have two cameras, front and back, and these days both
cameras have their flash.

And poor userspace flashlight application can not know if as3645
drivers front LED or back LED. Thus, I'd set devicename to
front-camera or main-camera -- because that's what it is associated
with. Userspace does not care what hardware drives the LED, but needs
to know if it is front or back camera.

If LEDs control keyboard backlight, I'd expect the LED name to be
"keyboard::backlight", not "i2c-0020-adp1643::backlight".

Thanks,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: as3645a flash userland interface

[Jacek Anaszewski]

On 09/12/2017 11:55 PM, Pavel Machek wrote:
> On Tue 2017-09-12 20:53:33, Jacek Anaszewski wrote:
>> Hi Pavel,
>>
>> On 09/12/2017 12:36 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>> There were some changes to as3645a flash controller. Before we have
>>> stable interface we have to keep forever I want to ask:
>>
>> Note that we have already two LED flash class drivers - leds-max77693
>> and leds-aat1290. They have been present in mainline for over two years
>> now.
> 
> Well.. that's ok. No change there is neccessary.
> 
>>> What directory are the flash controls in?
>>>
>>> /sys/class/leds/led-controller:flash ?
>>>
>>> Could we arrange for something less generic, like
>>>
>>> /sys/class/leds/main-camera:flash ?
>>
>> I'd rather avoid overcomplicating this. LED class device name pattern
>> is well defined to devicename:colour:function
>> (see Documentation/leds/leds-class.txt, "LED Device Naming" section).
>>
>> In this case "flash" in place of the "function" segment makes the
>> things clear enough I suppose.
> 
> It does not.
> 
> Phones usually have two cameras, front and back, and these days both
> cameras have their flash.
> 
> And poor userspace flashlight application can not know if as3645
> drivers front LED or back LED. Thus, I'd set devicename to
> front-camera or main-camera -- because that's what it is associated
> with. Userspace does not care what hardware drives the LED, but needs
> to know if it is front or back camera.

The name of a LED flash class device isn't fixed and is derived
from DT label property. Name in the example of some DT bindings
will not force people to apply similar pattern for the other
drivers and even for the related one. No worry about having
to keep anything forever basing on that.

-- 
Best regards,
Jacek Anaszewski

Re: as3645a flash userland interface

[Sylwester Nawrocki]

Hi,

On 09/13/2017 07:53 PM, Jacek Anaszewski wrote:
> On 09/12/2017 11:55 PM, Pavel Machek wrote:
>> On Tue 2017-09-12 20:53:33, Jacek Anaszewski wrote:
>>> On 09/12/2017 12:36 PM, Pavel Machek wrote:

>>>> What directory are the flash controls in?
>>>>
>>>> /sys/class/leds/led-controller:flash ?
>>>>
>>>> Could we arrange for something less generic, like
>>>>
>>>> /sys/class/leds/main-camera:flash ?
>>>
>>> I'd rather avoid overcomplicating this. LED class device name pattern
>>> is well defined to devicename:colour:function
>>> (see Documentation/leds/leds-class.txt, "LED Device Naming" section).
>>>
>>> In this case "flash" in place of the "function" segment makes the
>>> things clear enough I suppose.
>>
>> It does not.
>>
>> Phones usually have two cameras, front and back, and these days both
>> cameras have their flash.
>>
>> And poor userspace flashlight application can not know if as3645
>> drivers front LED or back LED. Thus, I'd set devicename to
>> front-camera or main-camera -- because that's what it is associated
>> with. Userspace does not care what hardware drives the LED, but needs
>> to know if it is front or back camera.
> 
> The name of a LED flash class device isn't fixed and is derived
> from DT label property. Name in the example of some DT bindings
> will not force people to apply similar pattern for the other
> drivers and even for the related one. No worry about having
> to keep anything forever basing on that.

Isn't the V4L2 subdev/Media Controller API supposed to provide means
for associating flash LEDs with camera sensors? You seem to be insisting
on using the sysfs leds interface for that, which is not a primary
interface for camera flash AFAICT.

-- 
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: as3645a flash userland interface

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1990 bytes --]

Hi!

> >>>>What directory are the flash controls in?
> >>>>
> >>>>/sys/class/leds/led-controller:flash ?
> >>>>
> >>>>Could we arrange for something less generic, like
> >>>>
> >>>>/sys/class/leds/main-camera:flash ?
> >>>
> >>>I'd rather avoid overcomplicating this. LED class device name pattern
> >>>is well defined to devicename:colour:function
> >>>(see Documentation/leds/leds-class.txt, "LED Device Naming" section).
> >>>
> >>>In this case "flash" in place of the "function" segment makes the
> >>>things clear enough I suppose.
> >>
> >>It does not.
> >>
> >>Phones usually have two cameras, front and back, and these days both
> >>cameras have their flash.
> >>
> >>And poor userspace flashlight application can not know if as3645
> >>drivers front LED or back LED. Thus, I'd set devicename to
> >>front-camera or main-camera -- because that's what it is associated
> >>with. Userspace does not care what hardware drives the LED, but needs
> >>to know if it is front or back camera.
> >
> >The name of a LED flash class device isn't fixed and is derived
> >from DT label property. Name in the example of some DT bindings
> >will not force people to apply similar pattern for the other
> >drivers and even for the related one. No worry about having
> >to keep anything forever basing on that.
> 
> Isn't the V4L2 subdev/Media Controller API supposed to provide means
> for associating flash LEDs with camera sensors? You seem to be insisting
> on using the sysfs leds interface for that, which is not a primary
> interface for camera flash AFAICT.

a) subdev/media controller API currently does not provide such means.

b) if we have /sys/class/leds interface to userland, it should be
useful.

c) having flashlight application going through media controller API is
a bad joke.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: as3645a flash userland interface

[Sakari Ailus]

[-- Attachment #1.1: Type: text/plain, Size: 2634 bytes --]

Hi Pavel and others,

On 09/14/17 13:07, Pavel Machek wrote:
> Hi!
> 
>>>>>> What directory are the flash controls in?
>>>>>>
>>>>>> /sys/class/leds/led-controller:flash ?
>>>>>>
>>>>>> Could we arrange for something less generic, like
>>>>>>
>>>>>> /sys/class/leds/main-camera:flash ?
>>>>>
>>>>> I'd rather avoid overcomplicating this. LED class device name pattern
>>>>> is well defined to devicename:colour:function
>>>>> (see Documentation/leds/leds-class.txt, "LED Device Naming" section).
>>>>>
>>>>> In this case "flash" in place of the "function" segment makes the
>>>>> things clear enough I suppose.
>>>>
>>>> It does not.
>>>>
>>>> Phones usually have two cameras, front and back, and these days both
>>>> cameras have their flash.
>>>>
>>>> And poor userspace flashlight application can not know if as3645
>>>> drivers front LED or back LED. Thus, I'd set devicename to
>>>> front-camera or main-camera -- because that's what it is associated
>>>> with. Userspace does not care what hardware drives the LED, but needs
>>>> to know if it is front or back camera.
>>>
>>> The name of a LED flash class device isn't fixed and is derived
>> >from DT label property. Name in the example of some DT bindings
>>> will not force people to apply similar pattern for the other
>>> drivers and even for the related one. No worry about having
>>> to keep anything forever basing on that.
>>
>> Isn't the V4L2 subdev/Media Controller API supposed to provide means
>> for associating flash LEDs with camera sensors? You seem to be 

It should, and will, but doesn't do that yet. The information will soon
available to the kernel but even that's not yet in mainline.

insisting
>> on using the sysfs leds interface for that, which is not a primary
>> interface for camera flash AFAICT.
> 
> a) subdev/media controller API currently does not provide such means.
> 
> b) if we have /sys/class/leds interface to userland, it should be
> useful.
> 
> c) having flashlight application going through media controller API is
> a bad joke.

d) V4L2 sub-devices are always related to a master device (that usually
has the DMA engine). In the absence of that, there will be no sub-device
node either. In other words, if you don't have a camera, you won't have
a sub-device. And it's perfectly possible to have a flash LED without a
camera.

For what's worth, I think there was a Nokia (S30 ?) phone long, long
time ago with a torch LED but no camera. 1100 maybe? The old one, not
the new one.

-- 
Regards,

Sakari Ailus
sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: as3645a flash userland interface

[Sylwester Nawrocki]

On 09/14/2017 12:07 PM, Pavel Machek wrote:
>> Isn't the V4L2 subdev/Media Controller API supposed to provide means
>> for associating flash LEDs with camera sensors? You seem to be insisting
>> on using the sysfs leds interface for that, which is not a primary
>> interface for camera flash AFAICT.
 >
> a) subdev/media controller API currently does not provide such means.

Yes, but it should, that's what it was designed for AFAIK.

> b) if we have /sys/class/leds interface to userland, it should be
> useful.

At the same time we shouldn't overcomplicate it with the camera
functionality.

> c) having flashlight application going through media controller API is
> a bad joke.

It doesn't have to, maybe I misunderstood what you exactly ask for.
Nevertheless what's missing is some user visible name/label for each
flash LED, right? Currently enumerating flash LEDs can be done by looking
at the function part of /sys/class/leds/<led-controller>:<colour>:
<function> path.

Could additional information be appended to the <function> part, so
user can identify which LED is which? E.g. "flash(rear)", "flash(front)",
etc. This could be achieved by simply adding label property in DT.
Or is the list of supported <function> strings already standardized?

-- 
Regards,
Sylwester

Re: as3645a flash userland interface

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1778 bytes --]

On Thu 2017-09-14 13:01:19, Sylwester Nawrocki wrote:
> On 09/14/2017 12:07 PM, Pavel Machek wrote:
> >>Isn't the V4L2 subdev/Media Controller API supposed to provide means
> >>for associating flash LEDs with camera sensors? You seem to be insisting
> >>on using the sysfs leds interface for that, which is not a primary
> >>interface for camera flash AFAICT.
> >
> >a) subdev/media controller API currently does not provide such means.
> 
> Yes, but it should, that's what it was designed for AFAIK.
> 
> >b) if we have /sys/class/leds interface to userland, it should be
> >useful.
> 
> At the same time we shouldn't overcomplicate it with the camera
> functionality.

I'm advocating adding label = "main_camera" into the .dts. That's all.

> >c) having flashlight application going through media controller API is
> >a bad joke.
> 
> It doesn't have to, maybe I misunderstood what you exactly ask for.
> Nevertheless what's missing is some user visible name/label for each
> flash LED, right? Currently enumerating flash LEDs can be done by looking
> at the function part of /sys/class/leds/<led-controller>:<colour>:
> <function> path.
> 
> Could additional information be appended to the <function> part, so
> user can identify which LED is which? E.g. "flash(rear)", "flash(front)",
> etc. This could be achieved by simply adding label property in DT.
> Or is the list of supported <function> strings already standardized?

label = "flash_main_camera" would work for me, yes. And yes, I'd
prefer to do this before 4.14 release, so that userland-visible
interface does not change.

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: as3645a flash userland interface

[Sylwester Nawrocki]

On 09/14/2017 01:53 PM, Pavel Machek wrote:
> On Thu 2017-09-14 13:01:19, Sylwester Nawrocki wrote:
>> On 09/14/2017 12:07 PM, Pavel Machek wrote:
>>>> Isn't the V4L2 subdev/Media Controller API supposed to provide means
>>>> for associating flash LEDs with camera sensors? You seem to be insisting
>>>> on using the sysfs leds interface for that, which is not a primary
>>>> interface for camera flash AFAICT.
>>>
>>> a) subdev/media controller API currently does not provide such means.
>>
>> Yes, but it should, that's what it was designed for AFAIK.
>>
>>> b) if we have /sys/class/leds interface to userland, it should be
>>> useful.
>>
>> At the same time we shouldn't overcomplicate it with the camera
>> functionality.
> 
> I'm advocating adding label = "main_camera" into the .dts. That's all.
> 
>>> c) having flashlight application going through media controller API is
>>> a bad joke.
>>
>> It doesn't have to, maybe I misunderstood what you exactly ask for.
>> Nevertheless what's missing is some user visible name/label for each
>> flash LED, right? Currently enumerating flash LEDs can be done by looking
>> at the function part of /sys/class/leds/<led-controller>:<colour>:
>> <function> path.
>>
>> Could additional information be appended to the <function> part, so
>> user can identify which LED is which? E.g. "flash(rear)", "flash(front)",
>> etc. This could be achieved by simply adding label property in DT.
>> Or is the list of supported <function> strings already standardized?
> 
> label = "flash_main_camera" would work for me, yes. And yes, I'd
> prefer to do this before 4.14 release, so that userland-visible
> interface does not change.

Looking at existing label entries in DT (git grep label.*led
-- arch/arm/boot/dts | sed 's\^.*label\label\' | sort | uniq)
I can't see why something like this couldn't be added.
Or label = "as3645a::flash_main_camera" so we abide the LED device
naming rules described in Documentation/leds/leds-class.txt.

-- 
Regards,
Sylwester

Re: as3645a flash userland interface

[Sakari Ailus]

Hi Jacek,

On Tue, Sep 12, 2017 at 08:53:33PM +0200, Jacek Anaszewski wrote:
> Hi Pavel,
> 
> On 09/12/2017 12:36 PM, Pavel Machek wrote:
> > Hi!
> > 
> > There were some changes to as3645a flash controller. Before we have
> > stable interface we have to keep forever I want to ask:
> 
> Note that we have already two LED flash class drivers - leds-max77693
> and leds-aat1290. They have been present in mainline for over two years
> now.
> 
> > What directory are the flash controls in?
> > 
> > /sys/class/leds/led-controller:flash ?
> > 
> > Could we arrange for something less generic, like
> > 
> > /sys/class/leds/main-camera:flash ?
> 
> I'd rather avoid overcomplicating this. LED class device name pattern
> is well defined to devicename:colour:function
> (see Documentation/leds/leds-class.txt, "LED Device Naming" section).
> 
> In this case "flash" in place of the "function" segment makes the
> things clear enough I suppose.

Oh. I have to admit I've completely missed this. :-o

I'll address this in v2 of the as3645a fixes, plus submit related V4L2
flash class documentation fixes a little later.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html