Re: [PATCH 2/2] leds: rt8515: Add Richtek RT8515 LED driver

From: Jacek Anaszewski <jacek.anaszewski@gmail.com>
To: Linus Walleij <linus.walleij@linaro.org>,
	Hans Verkuil <hverkuil@xs4all.nl>
Cc: Pavel Machek <pavel@ucw.cz>, Dan Murphy <dmurphy@ti.com>,
	Linux LED Subsystem <linux-leds@vger.kernel.org>,
	newbytee@protonmail.com, Stephan Gerhold <stephan@gerhold.net>,
	Sakari Ailus <sakari.ailus@linux.intel.com>
Subject: Re: [PATCH 2/2] leds: rt8515: Add Richtek RT8515 LED driver
Date: Thu, 15 Oct 2020 22:39:06 +0200	[thread overview]
Message-ID: <542c2f4c-a51a-cc90-632f-31c891b7146f@gmail.com> (raw)
In-Reply-To: <CACRpkdYgf8OPc2EqmPu=O2dr+ns4e3kTGyVi3A2Thd9v1wQGuA@mail.gmail.com>

Hi Linus,

On 10/14/20 12:52 PM, Linus Walleij wrote:
> Hi Jakub!

Jacek :-)

> thanks for the review. I fixed all comments but I have a problem
> with the below:
> 
> On Thu, Aug 13, 2020 at 11:04 PM Jacek Anaszewski
> <jacek.anaszewski@gmail.com> wrote:
>> On 8/12/20 11:07 AM, Linus Walleij wrote:
> 
>>> +     /* Init flash intensity setting */
>>> +     s = &v4l2_sd_cfg->intensity;
>>> +     s->min = 0;
>>> +     s->max = rt->fled.led_cdev.max_brightness;
>>> +     s->step = 1;
>>
>> struct v4l2_flash_config's intensity property tells V4L2 flash framework
>> how to convert LED class brightness levels to microamperes used by
>> the V4L2 flash subdevice.
>>
>> See max77693_init_v4l2_flash_config() in drivers/leds/leds-max77693.c
>> for a reference.
> 
> I understand, but I'm a bit of lost here, because I do not have
> any datsheet for the Richtek RT8515. The outoftree code
> that exists for example for Asus Zenfone:
> https://github.com/ZenfoneArea/android_kernel_asus_zenfone5/blob/master/linux/modules/camera/drivers/media/i2c/rt8515.c
> 
> The intensity is set to min/max in percent
> (0-100%) so the numerals 1-100 step 1 so
> the same as the brightness.

In such case there is not much we can do and your solution
seems reasonable.

> Is the V4L2 API such defined that if you don't know
> what microamperes that correspond to a certain setting
> it is better to simply not implement it? I can certainly
> remove it.

That would be a question to the author of V4L2 API.

Adding Sakari.

> I don't quite understand why V4L2 needs to control this
> in uA but I suppose they have a good reason. It however
> creates a really high bar for out-of-tree code from
> non-cooperative vendors. I'll try to send a mail to
> Richtek and ask for the datasheet though, stranger things
> have happened.

-- 
Best regards,
Jacek Anaszewski