Re: [PATCH v4 2/3] pwm: mtk-disp: move the commit to clock enabled

From: Jitao Shi <jitao.shi@mediatek.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Thierry Reding <thierry.reding@gmail.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	<linux-pwm@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-mediatek@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>, <srv_heupstream@mediatek.com>,
	<yingjoe.chen@mediatek.com>, <eddie.huang@mediatek.com>,
	<cawa.cheng@mediatek.com>, <bibby.hsieh@mediatek.com>,
	<ck.hu@mediatek.com>, <stonea168@163.com>,
	<huijuan.xie@mediatek.com>
Subject: Re: [PATCH v4 2/3] pwm: mtk-disp: move the commit to clock enabled
Date: Fri, 11 Jun 2021 10:29:19 +0800	[thread overview]
Message-ID: <1623378559.24490.12.camel@mszsdaap41> (raw)
In-Reply-To: <20210606211457.ya5nbp6fqevuhzwa@pengutronix.de>

On Sun, 2021-06-06 at 23:14 +0200, Uwe Kleine-König wrote:
> On Thu, Jun 03, 2021 at 06:05:30PM +0800, Jitao Shi wrote:
> > Due to the clock sequence changing, so move the reg commit to
> 
> Which change do you refer to, here? The previous patch? If so, I assume
> this means the series is not bisectable because the driver is broken
> when only the first patch is applied?
> 
Yes, this patch is depend the previous patch.
I'll squash it to the previous patch in next version.

> > config().
> > 
> > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> > ---
> >  drivers/pwm/pwm-mtk-disp.c | 20 +++++++-------------
> >  1 file changed, 7 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> > index b5771e2c54b8..b87b3c00a685 100644
> > --- a/drivers/pwm/pwm-mtk-disp.c
> > +++ b/drivers/pwm/pwm-mtk-disp.c
> > @@ -135,6 +135,13 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> >  		mtk_disp_pwm_update_bits(mdp, mdp->data->commit,
> >  					 mdp->data->commit_mask,
> >  					 0x0);
> > +	} else {
> 
> You dropped the code comment? Is it wrong? Or is it too obvious to be
> mentioned?
> 

I'll fix them next verison.

> > +		mtk_disp_pwm_update_bits(mdp, mdp->data->bls_debug,
> > +					 mdp->data->bls_debug_mask,
> > +					 mdp->data->bls_debug_mask);
> > +		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> > +					 mdp->data->con0_sel,
> > +					 mdp->data->con0_sel);
> >  	}
> >  
> >  	return 0;
> > @@ -208,19 +215,6 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, mdp);
> >  
> > -	/*
> > -	 * For MT2701, disable double buffer before writing register
> > -	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
> > -	 */
> > -	if (!mdp->data->has_commit) {
> > -		mtk_disp_pwm_update_bits(mdp, mdp->data->bls_debug,
> > -					 mdp->data->bls_debug_mask,
> > -					 mdp->data->bls_debug_mask);
> > -		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> > -					 mdp->data->con0_sel,
> > -					 mdp->data->con0_sel);
> > -	}
> > -
> >  	return 0;
> >  }
> 
> Best regards
> Uwe
> 

Best Regards
Jitao