On Tue, Oct 13, 2009 at 12:21:47PM -0400, H Hartley Sweeten wrote:
> On Monday, October 12, 2009 10:47 PM, Dmitry Torokhov wrote:
> > Hi Miguel,
> >
> > On Fri, Oct 09, 2009 at 11:27:05AM -0600, miguel.aguilar@xxxxxxxxxxxx wrote:
> >> From: Miguel Aguilar <miguel.aguilar@xxxxxxxxxxxx>
> >>
> >> Adds the driver for enabling keypad support for DaVinci platforms.
> >>
> >> DM365 is the only platform that uses this driver at the moment.
> >>
> >
> > Looks pretty good, I have one question:
> >
> >> +static void davinci_ks_write(struct davinci_ks *davinci_ks, u32 val, u32 addr)
> >> +{
> >> + u32 base = (u32)davinci_ks->base;
> >> +
> >> + __raw_writel(val,(u32 *)(base + addr));
> >> +}
> >
> > Do you really need these casts? I'd think that bare __raw_writel would
> > work just fine.
>
> The address for __raw_{write/read}* should be void __iomem *, davinci_ks->base
> should already be the correct type due to ioremap().
>
> You can probably just change the whole thing to:
>
> +static void davinci_ks_write(struct davinci_ks *davinci_ks, u32 val, u32 addr)
> +{
> + __raw_writel(val, davinci_ks->base + addr);
> +}
> +
> +static u32 davinci_ks_read(struct davinci_ks *davinci_ks, u32 addr)
> +{
> + return __raw_readl(davinci_ks->base + addr);
> +}
>
Better yet, use __raw_readl() and __raw_writel() directly - there is no
need for wrappers that just rename existig functions not adding any
additional functionality.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
