linux-kernel - Re: [PATCH 1/6] unicore32: pwm: Properly remap memory-mapped registers

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120907125035.GA29340@avionic-0098.mockup.avionic-design.de>
Date:	Fri, 7 Sep 2012 14:50:35 +0200
From:	Thierry Reding <thierry.reding@...onic-design.de>
To:	guanxuetao@...c.pku.edu.cn
Cc:	Guan Xuetao <gxt@...c.pku.edu.cn>,
	Mike Turquette <mturquette@...aro.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] unicore32: pwm: Properly remap memory-mapped
 registers

On Thu, Sep 06, 2012 at 04:38:15PM +0800, guanxuetao@...c.pku.edu.cn wrote:
> > Instead of writing to the timer controller registers by dereferencing a
> > pointer to the memory location, properly remap the memory region with a
> > call to ioremap_nocache() and access the registers using writel().
> >
> > Signed-off-by: Thierry Reding <thierry.reding@...onic-design.de>
> > ---
> >  arch/unicore32/kernel/pwm.c | 25 ++++++++++++++++++++++---
> >  1 file changed, 22 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/unicore32/kernel/pwm.c b/arch/unicore32/kernel/pwm.c
> > index 4615d51..410b786 100644
> > --- a/arch/unicore32/kernel/pwm.c
> > +++ b/arch/unicore32/kernel/pwm.c
> > @@ -23,10 +23,16 @@
> >  #include <asm/div64.h>
> >  #include <mach/hardware.h>
> >
> > +#define PWCR 0x00
> > +#define DCCR 0x04
> > +#define PCR  0x08
> I think old register names could be used here by some small modifications.
> Please see arch/unicore32/include/mach/regs-ost.h
> We can avoid ioremap and use writel/readl directly on these registers.
> 
> Guan

The whole point of this patch was to make the PWM driver behave more
like other drivers. If the registers are addressed directly, there is no
way that the driver will work for a second instance of the PWM
controller. I know that there probably is no second instance right now,
but given that pretty much every regular driver accesses its register
through the ioremap()'ed addresses and this patch doesn't go through
hoops to achieve this, I think this is a perfectly valid cleanup.

Thierry

> > +
> >  struct pwm_device {
> >  	struct list_head	node;
> >  	struct platform_device *pdev;
> >
> > +	void __iomem	*base;
> > +
> >  	const char	*label;
> >  	struct clk	*clk;
> >  	int		clk_enabled;
> > @@ -69,9 +75,11 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int
> > period_ns)
> >  	 * before writing to the registers
> >  	 */
> >  	clk_enable(pwm->clk);
> > -	OST_PWMPWCR = prescale;
> > -	OST_PWMDCCR = pv - dc;
> > -	OST_PWMPCR  = pv;
> > +
> > +	writel(prescale, pwm->base + PWCR);
> > +	writel(pv - dc, pwm->base + DCCR);
> > +	writel(pv, pwm->base + PCR);
> > +
> >  	clk_disable(pwm->clk);
> >
> >  	return 0;
> > @@ -190,10 +198,19 @@ static struct pwm_device *pwm_probe(struct
> > platform_device *pdev,
> >  		goto err_free_clk;
> >  	}
> >
> > +	pwm->base = ioremap_nocache(r->start, resource_size(r));
> > +	if (pwm->base == NULL) {
> > +		dev_err(&pdev->dev, "failed to remap memory resource\n");
> > +		ret = -EADDRNOTAVAIL;
> > +		goto err_release_mem;
> > +	}
> > +
> >  	__add_pwm(pwm);
> >  	platform_set_drvdata(pdev, pwm);
> >  	return pwm;
> >
> > +err_release_mem:
> > +	release_mem_region(r->start, resource_size(r));
> >  err_free_clk:
> >  	clk_put(pwm->clk);
> >  err_free:
> > @@ -224,6 +241,8 @@ static int __devexit pwm_remove(struct platform_device
> > *pdev)
> >  	list_del(&pwm->node);
> >  	mutex_unlock(&pwm_lock);
> >
> > +	iounmap(pwm->base);
> > +
> >  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	release_mem_region(r->start, resource_size(r));
> >
> > --
> > 1.7.12
> >
> 
> 
> 

Content of type "application/pgp-signature" skipped