Re: [PATCH 1/2] backlight: pwm: don't call legacy pwm request for device defined in dt

[Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>]

Hello Thierry,

On 07.11.2014 16:57, Vladimir Zapolskiy wrote:
> Thierry,
> 
> On 07.11.2014 16:19, Vladimir Zapolskiy wrote:
>> Hi Thierry,
>>
>> On 07.11.2014 15:48, Thierry Reding wrote:
>>> On Sat, Oct 11, 2014 at 04:46:25PM +0300, Vladimir Zapolskiy wrote:
>>>> Platform PWM backlight data provided by board's device tree should be
>>>> complete enough to successfully request a pwm device using pwm_get() API.
>>>>
>>>> Based on initial implementation done by Dmitry Eremin-Solenikov.
>>>>
>>>> Reported-by: Dmitry Eremin-Solenikov <dmitry_eremin@mentor.com>
>>>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
>>>> Cc: Thierry Reding <thierry.reding@gmail.com>
>>>> Cc: Jingoo Han <jg1.han@samsung.com>
>>>> Cc: Bryan Wu <cooloney@gmail.com>
>>>> Cc: Lee Jones <lee.jones@linaro.org>
>>>> ---
>>>>  drivers/video/backlight/pwm_bl.c |   14 +++++++-------
>>>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>> I don't really understand what this is supposed to do. The commit
>>> message doesn't make a very good job of explaining it either.
>>>
>>> Can you describe in more detail what problem this fixes and why it
>>> should be merged?
>>
>> thank you for review.
>>
>> As it is shown by the code this particular change rejects fallback to
>> legacy PWM device request (which itself in turn is fixed in the next
>> commit) for boards with supplied DTS, "pwm-backlight" compatible node
>> and unregistered corresponding PWM device in that node.
>>
>> I don't know if there is a good enough reason to register PWM backlight
>> device connected to some quite arbitrary PWM device, if no PWM device
>> information is given in the "pwm-backlight" compatible node, so I think
>> it makes sense to change the default policy.
>>
> 
> also please note that
> Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
> quite fairly describes "pwms" as a required property, but right now this
> statement from the documentation is wrong, it is possible to register
> pwm-backlight device driver (using notorious pwm_request() legacy API)
> connected to some unspecified pwm device.
> 
> I don't think that the current registration policy is correct, that's
> why I propose to fix the logic instead of making a documentation update.
> 

have you had a chance to check the rationale of the change?

If you accept it, should I make the commit message more verbose?

--
With best wishes,
Vladimir