linux-kernel - RE: [PATCH] clocksource: fix clocksource_mmio_readX

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

Message-ID: <2f174001f11b4e1b89558cb170973875@BY2PR03MB505.namprd03.prod.outlook.com>
Date:	Wed, 23 Apr 2014 02:27:22 +0000
From:	"Li.Xiubo@...escale.com" <Li.Xiubo@...escale.com>
To:	Daniel Lezcano <daniel.lezcano@...aro.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] clocksource: fix clocksource_mmio_readX_down

> > diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c
> > index c0e2512..f17a0d1 100644
> > --- a/drivers/clocksource/mmio.c
> > +++ b/drivers/clocksource/mmio.c
> > @@ -27,7 +27,7 @@ cycle_t clocksource_mmio_readl_up(struct clocksource *c)
> >
> >   cycle_t clocksource_mmio_readl_down(struct clocksource *c)
> >   {
> > -	return ~readl_relaxed(to_mmio_clksrc(c)->reg);
> > +	return ~readl_relaxed(to_mmio_clksrc(c)->reg) & c->mask;
> >   }
> >
> >   cycle_t clocksource_mmio_readw_up(struct clocksource *c)
> > @@ -37,7 +37,7 @@ cycle_t clocksource_mmio_readw_up(struct clocksource *c)
> >
> >   cycle_t clocksource_mmio_readw_down(struct clocksource *c)
> >   {
> > -	return ~(unsigned)readw_relaxed(to_mmio_clksrc(c)->reg);
> > +	return ~(unsigned)readw_relaxed(to_mmio_clksrc(c)->reg) & c->mask;
> >   }
> >
> >   /**
> 
> 
> Hi,
> 
> I realize there is some type confusion here:
> 
> cycle_t -> u64
> readl_relaxed -> u32
> readw_relaxed -> u16
> 
> and clocksource_mmio_readw_down returns a cast to unsigned (u32)
> 
> This patch makes sense but it obfuscate more the types in these
> functions. Worth to clarify the functions first ?
> 

Yes, though the short type could be converted to the longer type automatically,
It's better and worth to clarify it firstly.

I'll fix this, please see the next version.


Thanks,
BRs
Xiubo


> 
> --
>   <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
> 
>