linux-kernel - Re: [PATCH v2 03/16] pwm: cros-ec: update documentation regarding pwm-cells

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 18 Jan 2018 11:18:02 +0200
From:   Claudiu Beznea <Claudiu.Beznea@...rochip.com>
To:     Brian Norris <briannorris@...omium.org>
CC:     <mark.rutland@....com>, <linux-pwm@...r.kernel.org>,
        <linux-rpi-kernel@...ts.infradead.org>, <corbet@....net>,
        <linux-kernel@...r.kernel.org>, <linux@...linux.org.uk>,
        <robh+dt@...nel.org>, <linux-rockchip@...ts.infradead.org>,
        <devicetree@...r.kernel.org>, <thierry.reding@...il.com>,
        <alexandre.belloni@...e-electrons.com>, <haojian.zhuang@...il.com>,
        <linux-amlogic@...ts.infradead.org>, <robert.jarzmik@...e.fr>,
        <daniel@...que.org>, <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 03/16] pwm: cros-ec: update documentation regarding
 pwm-cells



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
>