linux-kernel - Re: [PATCH RESEND 4/4] pwm: mediatek: add MT2712/MT7622 support

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>] [day] [month] [year] [list]

Message-ID: <f81a5fe1-4d9a-aa54-e680-035ca6075660@phrozen.org>
Date:   Thu, 22 Jun 2017 08:52:54 +0200
From:   John Crispin <john@...ozen.org>
To:     Zhi Mao <zhi.mao@...iatek.com>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        "linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>,
        Zhenbao Liu (刘振宝) 
        <Zhenbao.Liu@...iatek.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        srv_heupstream <srv_heupstream@...iatek.com>,
        Sean Wang (王志亘) <sean.wang@...iatek.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        YT Shen (沈岳霆) <Yt.Shen@...iatek.com>,
        Yingjoe Chen (??英洲) 
        <Yingjoe.Chen@...iatek.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH RESEND 4/4] pwm: mediatek: add MT2712/MT7622 support



On 22/06/17 08:09, Zhi Mao wrote:
> Hi John,
>
> Thanks for your review the code and feedback.
> There are 3 issues in this patch:
> 1.adds PWM_CLK_DIV_MAX which really should go into its own patch
> 2.adds mtk_pwm_com_reg which should also go into its own patch
> 3.remove comments inline /*===*/
>
> for #1 and #3, I will modify them in the next release.
> but for #2, I want to discuss with you,
> adding the structure  "mtk_pwm_com_reg" is only for the registers of 
> MT2712 PWM8,
> so we want to keep this modification in this patch.
>
> what's your opinion about it?
> Any reply is welcome.
>
>
> Regards
> Zhi
>
>

Hi Zhi,

I just had a closer look and noticed that the new CON registers are not 
at the same fixed 0x40 offset as the pwm0-5 ones. sorry i did not notice 
this. 2) needs to be part of this patch in that case. please add a short 
info to the patches description explaining this.

     John


>
>
>
> On Wed, 2017-06-21 at 20:22 +0800, John Crispin wrote:
>> On 21/06/17 10:11, Zhi Mao wrote:
>> > support multiple chip(MT2712, MT7622, MT7623)
>> This patch does more than add extra SoC support. It also
>> * adds PWM_CLK_DIV_MAX which really should go into its own patch
>> * adds mtk_pwm_com_reg which should also go into its own patch
>>
>> more comments inline
>>
>> >
>> > Signed-off-by: Zhi Mao <zhi.mao@...iatek.com <mailto:zhi.mao@...iatek.com>>
>> > ---
>> >   drivers/pwm/pwm-mediatek.c |   63 +++++++++++++++++++++++++++++++++++---------
>> >   1 file changed, 51 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
>> > index c803ff6..d520356 100644
>> > --- a/drivers/pwm/pwm-mediatek.c
>> > +++ b/drivers/pwm/pwm-mediatek.c
>> > @@ -16,6 +16,7 @@
>> >   #include <linux/module.h>
>> >   #include <linux/clk.h>
>> >   #include <linux/of.h>
>> > +#include <linux/of_device.h>
>> >   #include <linux/platform_device.h>
>> >   #include <linux/pwm.h>
>> >   #include <linux/slab.h>
>> [...]
>> > @@ -215,9 +238,25 @@ static int mtk_pwm_remove(struct platform_device *pdev)
>> >   	return pwmchip_remove(&pc->chip);
>> >   }
>> >
>> > +/*==========================================*/
>>
>> please remove these comment lines
>>
>>       John
>> > +static const struct mtk_com_pwm_data mt2712_pwm_data = {
>> > +	.pwm_nums = 8,
>> > +};
>> > +
>> > +static const struct mtk_com_pwm_data mt7622_pwm_data = {
>> > +	.pwm_nums = 6,
>> > +};
>> > +
>> > +static const struct mtk_com_pwm_data mt7623_pwm_data = {
>> > +	.pwm_nums = 5,
>> > +};
>> > +/*==========================================*/
>> > +
>> >   static const struct of_device_id mtk_pwm_of_match[] = {
>> > -	{ .compatible = "mediatek,mt7623-pwm" },
>> > -	{ }
>> > +	{.compatible = "mediatek,mt2712-pwm", .data = &mt2712_pwm_data},
>> > +	{.compatible = "mediatek,mt7622-pwm", .data = &mt7622_pwm_data},
>> > +	{.compatible = "mediatek,mt7623-pwm", .data = &mt7623_pwm_data},
>> > +	{},
>> >   };
>> >   MODULE_DEVICE_TABLE(of, mtk_pwm_of_match);
>> >
>>
>