RE: [PATCH] clocksource: fix clocksource_mmio_readX_down

[Li.Xiubo@xxxxxxxxxxxxx]

> > 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
> 
>