lkml.org 
[lkml]   [2018]   [Jan]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 03/16] pwm: cros-ec: update documentation regarding pwm-cells
From
Date


On 18.01.2018 01:10, Brian Norris wrote:
> On Wed, Jan 17, 2018 at 10:29:53AM +0200, Claudiu Beznea wrote:
>> With these changes, if pwm-cells=1 then only PWM-channel will be parsed,
>
> I'm not sure if I'm understanding you correctly but...no. If cells is 1,
> then your driver change just causes us not to parse correctly, and
> everything fails.
My bad, agree with you, will fail with pwm-cells=1. I forgot about:
+ if (args->args_count < PWM_ARGS_CNT_XLATE_PERIOD ||
+ args->args_count > PWM_ARGS_CNT_XLATE_MAX)
return ERR_PTR(-EINVAL);
restriction.

>
>> if it is 2 PWM-channel and PWM-period will be parsed, if pwm-cells=3
>> then PWM-channel, PWM-period and PWM-flags will be parsed.
>> In your driver you used to have only one cell because you wanted to allow
>> user to give as argument only PWM channel, and you did not want a change
>> of PWM period (and in of_xlate function you initialize pwm period with 0xffff
>> value: this is why I changed the binding in patch 7 of this series, file
>
> It's not a matter of "allow", it's a matter of description. The period
> isn't actually even 0xffff, that's just a pseudo-period, to reflect that
> you have a choice of duty cycles of 0 to 0xffff. I (justifiably, I
> think) didn't think putting this false value in the device tree was
> accurate.
Ok, I didn't investigate the driver to see what is truly set in HW.
>
>> rk3399-gru-kevin.dts). But e.g. sysfs could try to change the PWM period,
>> there is no restriction to change the PWM period from sysfs, in the sysfs
>> interface but the restriction is in PWM apply of the drive. The same things
>> happens with these changes too. The user could introduce any PWM period via
>> DT but the pwm apply function of the driver will return error.
>
> sysfs has no bearing on a device tree binding. Just because we have a
> broken interface here doesn't mean we should change how we describe the
> hardware.
>
Based on [1] and the comments I will drop the first 7 patches of this series.

Thanks,
Claudiu

[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/ABI.txt
> Brian
>

\
 
 \ /
  Last update: 2018-01-18 10:18    [W:0.092 / U:0.240 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site