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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a15b3974-33ca-70c9-2579-bcdd0d9eb5b1@microchip.com>
Date:   Tue, 16 Jan 2018 11:33:08 +0200
From:   Claudiu Beznea <Claudiu.Beznea@...rochip.com>
To:     Neil Armstrong <narmstrong@...libre.com>,
        <thierry.reding@...il.com>, <robh+dt@...nel.org>,
        <mark.rutland@....com>, <linux@...linux.org.uk>,
        <daniel@...que.org>, <haojian.zhuang@...il.com>,
        <robert.jarzmik@...e.fr>, <corbet@....net>,
        <nicolas.ferre@...rochip.com>,
        <alexandre.belloni@...e-electrons.com>
CC:     <linux-pwm@...r.kernel.org>, Mike Dunn <mikedunn@...sguy.com>,
        Alexander Shiyan <shc_work@...l.ru>,
        <devicetree@...r.kernel.org>,
        Brian Norris <briannorris@...omium.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-rockchip@...ts.infradead.org>,
        <linux-rpi-kernel@...ts.infradead.org>,
        <linux-amlogic@...ts.infradead.org>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 01/16] drivers: pwm: core: use a single of xlate
 function



On 16.01.2018 11:07, Neil Armstrong wrote:
> On 12/01/2018 15:22, Claudiu Beznea wrote:
>> Remove of_pwm_simple_xlate() and of_pwm_xlate_with_flags() functions
>> and add of_pwm_xlate() which is used in all cases no mather if the OF
>> bindings are with PWM flags or not. This should not affect the old
>> behavior since the xlate will be based on #pwm-cells property of the
>> PWM controller. Based on #pwm-cells property the xlate will consider
>> the flags or not. This will permit the addition of other inputs to OF
>> xlate by just adding proper code at the end of of_pwm_xlate() and a new
>> input to enum pwm_args_xlate_options. With this changes there will be
>> no need to fill of_xlate and of_pwm_n_cells of struct pwm_chip from
>> the drivers probe methods. References in drives to references to of_xlate
>> and of_pwm_n_cells were removed. Drivers which used private of_xlate
>> functions switched to the generic of_pwm_xlate() function which fits
>> for it but with little changes in device trees (these drivers translated
>> differently the "pwms" bindings; the "pwms" bindings now are generic to
>> all drivers and all drivers should provide them in the format described
>> in pwm documentation).
>>
>> Cc: Thierry Reding <thierry.reding@...il.com>
>> Cc: Mike Dunn <mikedunn@...sguy.com>
>> Cc: Brian Norris <briannorris@...omium.org>
>> Cc: Alexander Shiyan <shc_work@...l.ru>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@...rochip.com>
>> ---
>>
>> This patch (and the next 7) could be applied independetly by this series, if
>> any, but I choosed to have it here since it makes easy the PWM modes parsing.
>> If you feel it could be independently of this series I could send a new version.
>>
>> Also, Thierry, Mike, Brian, Shiyan, please take an extra look over pwm-pxa.c,
>> pwm-cros-ec.c and pwm-clps711x.c since these were moved to use the generic
>> pwms (minimum 2 pwm-cells).
>>
>>  drivers/pwm/core.c             | 56 +++++++++++-------------------------------
>>  drivers/pwm/pwm-atmel-hlcdc.c  |  2 --
>>  drivers/pwm/pwm-atmel-tcb.c    |  2 --
>>  drivers/pwm/pwm-atmel.c        |  6 -----
>>  drivers/pwm/pwm-bcm-iproc.c    |  2 --
>>  drivers/pwm/pwm-bcm-kona.c     |  2 --
>>  drivers/pwm/pwm-bcm2835.c      |  2 --
>>  drivers/pwm/pwm-berlin.c       |  2 --
>>  drivers/pwm/pwm-clps711x.c     | 11 ---------
>>  drivers/pwm/pwm-cros-ec.c      | 20 ---------------
>>  drivers/pwm/pwm-fsl-ftm.c      |  2 --
>>  drivers/pwm/pwm-hibvt.c        |  2 --
>>  drivers/pwm/pwm-imx.c          |  8 ------
>>  drivers/pwm/pwm-lpc18xx-sct.c  |  2 --
>>  drivers/pwm/pwm-meson.c        |  2 --
>>  drivers/pwm/pwm-omap-dmtimer.c |  2 --
>>  drivers/pwm/pwm-pxa.c          | 19 --------------
>>  drivers/pwm/pwm-renesas-tpu.c  |  2 --
>>  drivers/pwm/pwm-rockchip.c     |  5 ----
>>  drivers/pwm/pwm-samsung.c      |  3 ---
>>  drivers/pwm/pwm-sun4i.c        |  2 --
>>  drivers/pwm/pwm-tiecap.c       |  2 --
>>  drivers/pwm/pwm-tiehrpwm.c     |  2 --
>>  drivers/pwm/pwm-vt8500.c       |  2 --
>>  drivers/pwm/pwm-zx.c           |  2 --
>>  include/linux/pwm.h            | 23 ++++++++++-------
>>  26 files changed, 29 insertions(+), 156 deletions(-)
>>
> 
> [...]
>> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
>> index 0767deba8e62..6702fcc203ce 100644
>> --- a/drivers/pwm/pwm-meson.c
>> +++ b/drivers/pwm/pwm-meson.c
>> @@ -535,8 +535,6 @@ static int meson_pwm_probe(struct platform_device *pdev)
>>  	meson->chip.ops = &meson_pwm_ops;
>>  	meson->chip.base = -1;
>>  	meson->chip.npwm = 2;
>> -	meson->chip.of_xlate = of_pwm_xlate_with_flags;
>> -	meson->chip.of_pwm_n_cells = 3;
>>  
>>  	meson->data = of_device_get_match_data(&pdev->dev);
>>  	meson->inverter_mask = BIT(meson->chip.npwm) - 1;
>> diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
>> index 5ad42f33e70c..6bd32ae6dd3e 100644
>> --- a/drivers/pwm/pwm-omap-dmtimer.c
>> +++ b/drivers/pwm/pwm-omap-dmtimer.c
>> @@ -317,8 +317,6 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
>>  	omap->chip.ops = &pwm_omap_dmtimer_ops;
>>  	omap->chip.base = -1;
>>  	omap->chip.npwm = 1;
>> -	omap->chip.of_xlate = of_pwm_xlate_with_flags;
>> -	omap->chip.of_pwm_n_cells = 3;
>>  
>>  	mutex_init(&omap->mutex);
>>  
> [...]
> 
> Hi Claudiu,
> 
> Please avoid changing the device tree plumbing, this of_xlate fields are needed since
> some platforms will need to have their own translate functions.
> 
> Moving a to single mandatory xlate function is going backward, a possible move
> would be to default to of_pwm_xlate_with_flags and 3 cells if of_xlate is NULL,
> or use the platforms xlate if provided.
> 
> Moving to a single xlate function is unrelated to your needs and can break some platforms.

Hi Neil,

Thank you for your feedback. Looking over the drivers I saw that only 3 drivers
are using private of_xlate functions, the rest of them uses of_pwm_xlate_with_flags
and of_pwm_simple_xlate function combined with pwm-cells binding and of_pwm_n_cells
(which I think is redundant with pwm-cells). I though that switching to a common
of_xlate to xlate everything based on pwm-cells binding value would be better
and remove redundant code.

But if this is a downward direction I agree to drop the changes related to of_xlate.

Thanks,
Claudiu

> 
> Neil
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ