Thread (18 messages) 18 messages, 4 authors, 2015-06-12

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

From: Vladimir Zapolskiy <hidden>
Date: 2014-12-01 14:47:38
Also in: linux-pwm

Hello Thierry,

On 07.11.2014 16:57, Vladimir Zapolskiy wrote:
Thierry,

On 07.11.2014 16:19, Vladimir Zapolskiy wrote:
quoted
Hi Thierry,

On 07.11.2014 15:48, Thierry Reding wrote:
quoted
On Sat, Oct 11, 2014 at 04:46:25PM +0300, Vladimir Zapolskiy wrote:
quoted
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 <redacted>
Signed-off-by: Vladimir Zapolskiy <redacted>
Cc: Thierry Reding <redacted>
Cc: Jingoo Han <redacted>
Cc: Bryan Wu <redacted>
Cc: Lee Jones <redacted>
---
 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help