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

[Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>]

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.

--
With best wishes,
Vladimir