linux-kernel - Re: [PATCH 2/6] pwm: imx: Move PWMv2 software reset code to a separate function

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: <20161024230232.3527fd42@jawa>
Date:   Mon, 24 Oct 2016 23:02:32 +0200
From:   Lukasz Majewski <l.majewski@...ess.pl>
To:     Boris Brezillon <boris.brezillon@...e-electrons.com>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Stefan Agner <stefan@...er.ch>, linux-pwm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Fabio Estevam <fabio.estevam@....com>,
        Fabio Estevam <festevam@...il.com>,
        Lothar Wassmann <LW@...o-electronics.de>,
        Bhuvanchandra DV <bhuvanchandra.dv@...adex.com>,
        kernel@...gutronix.de
Subject: Re: [PATCH 2/6] pwm: imx: Move PWMv2 software reset code to a
 separate function

Hi Boris,

> On Sun, 23 Oct 2016 23:45:42 +0200
> Lukasz Majewski <l.majewski@...ess.pl> wrote:
> 
> > The software reset code has been extracted from imx_pwm_config_v2
> > function and moved to new one - imx_pwm_sw_reset().
> > 
> > This change reduces the overall size of imx_pwm_config_v2() and
> > prepares it for atomic PWM operation.
> > 
> > Suggested-by: Stefan Agner <stefan@...er.ch>
> > Suggested-by: Boris Brezillon <boris.brezillon@...e-electrons.com>
> > Signed-off-by: Lukasz Majewski <l.majewski@...ess.pl>
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@...e-electrons.com>
> 
> Just a nit below ;).
> 
> BTW, can't you just merge path 2 and 3?

I do prefer to have many small patches touching and changing just one
thing.

> 
> > ---
> >  drivers/pwm/pwm-imx.c | 34 ++++++++++++++++++++++------------
> >  1 file changed, 22 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > index 83e43d5..fac5c93 100644
> > --- a/drivers/pwm/pwm-imx.c
> > +++ b/drivers/pwm/pwm-imx.c
> > @@ -131,6 +131,25 @@ static void imx_pwm_disable_v1(struct pwm_chip
> > *chip, struct pwm_device *pwm) clk_disable_unprepare(imx->clk_per);
> >  }
> >  
> > +static void imx_pwm_sw_reset(struct pwm_chip *chip)
> > +{
> > +	struct imx_chip *imx = to_imx_chip(chip);
> > +	struct device *dev = chip->dev;
> > +	int wait_count = 0;
> > +	u32 cr;
> > +
> > +	writel(MX3_PWMCR_SWR, imx->mmio_base + MX3_PWMCR);
> > +	do {
> > +		usleep_range(200, 1000);
> > +		cr = readl(imx->mmio_base + MX3_PWMCR);
> > +	} while ((cr & MX3_PWMCR_SWR) &&
> > +		 (wait_count++ < MX3_PWM_SWR_LOOP));
> > +
> > +	if (cr & MX3_PWMCR_SWR)
> > +		dev_warn(dev, "software reset timeout\n");
> > +}
> > +
> > +
> >  static int imx_pwm_config_v2(struct pwm_chip *chip,
> >  		struct pwm_device *pwm, int duty_ns, int period_ns)
> >  {
> > @@ -140,7 +159,7 @@ static int imx_pwm_config_v2(struct pwm_chip
> > *chip, unsigned long period_cycles, duty_cycles, prescale;
> >  	unsigned int period_ms;
> >  	bool enable = pwm_is_enabled(pwm);
> > -	int wait_count = 0, fifoav;
> > +	int fifoav;
> >  	u32 cr, sr;
> >  
> >  	/*
> > @@ -162,17 +181,8 @@ static int imx_pwm_config_v2(struct pwm_chip
> > *chip, if (fifoav == (sr & MX3_PWMSR_FIFOAV_MASK))
> >  				dev_warn(dev, "there is no free
> > FIFO slot\n"); }
> > -	} else {
> > -		writel(MX3_PWMCR_SWR, imx->mmio_base + MX3_PWMCR);
> > -		do {
> > -			usleep_range(200, 1000);
> > -			cr = readl(imx->mmio_base + MX3_PWMCR);
> > -		} while ((cr & MX3_PWMCR_SWR) &&
> > -			 (wait_count++ < MX3_PWM_SWR_LOOP));
> > -
> > -		if (cr & MX3_PWMCR_SWR)
> > -			dev_warn(dev, "software reset timeout\n");
> > -	}
> > +	} else
> > +		imx_pwm_sw_reset(chip);
> 
> 	} else {
> 		imx_pwm_sw_reset(chip);
> 	}

OK.

> 
> >  
> >  	c = clk_get_rate(imx->clk_per);
> >  	c = c * period_ns;
> 

Best regards,

Łukasz Majewski

Content of type "application/pgp-signature" skipped