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

[John Crispin]

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 another look and noticed that the CON registers are not at a 
fixed offset of 0x40 for the new pwm8 register so having 2) inside this 
patch makes sense. please explain in the description that this is the case

    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@xxxxxxxxxxxx <mailto:zhi.mao@xxxxxxxxxxxx>>
> > > ---
> > >   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);
> > >