Re: [PATCH 5/8] add Texas Instruments AR7 support

[Ralf Baechle <ralf@linux-mips.org>]

On Wed, Jun 24, 2009 at 12:28:56PM +0100, Alexander Clouter wrote:

> Florian Fainelli <florian@openwrt.org> wrote:
> > Le Tuesday 23 June 2009 20:15:09 Ralf Baechle, vous avez écrit :
> >> On Tue, Jun 23, 2009 at 11:28:27AM +0200, Florian Fainelli wrote:
> >>
> >> AR7 time again - the platform pending longest ...  Let's see:
> > 
> > Thank you very much for your comments Ralf, please find below an updated
> > version which addresses all of your comments. I hope this time we are going
> > to make it ;)
> >
> I am hoping someone can have a tackle of the lzma/bzip2 kernel/initramfs 
> generic compression code myself, but I guess one thing at a time. :)  If 
> you have a simple way for me to produce a LZMA'd image, I'll test it 
> this on my WAG54Gv2 (I need the image to be less than 700kB).
> 
> My comments, for what they are worth, below:
> 
> > From: Florian Fainelli <florian@openwrt.org>
> > Subject: Add support for Texas Instruments AR7 System-on-a-Chip
> > 
> > This patch adds support for the Texas Instruments AR7 System-on-a-Chip.
> > It supports the TNETD7100, 7200 and 7300 versions of the SoC.
> > 
> > Changes from v4:
> > [snipped]
> > 
> > Signed-off-by: Matteo Croce <matteo@openwrt.org>
> > Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> > Signed-off-by: Eugene Konev <ejka@openwrt.org>
> > Signed-off-by: Nicolas Thill <nico@openwrt.org>
> > Signed-off-by: Florian Fainelli <florian@openwrt.org>
> > -- 
> > [snipped]
> >
> > diff --git a/arch/mips/ar7/clock.c b/arch/mips/ar7/clock.c
> > new file mode 100644
> > index 0000000..7ce5f07
> > --- /dev/null
> > +++ b/arch/mips/ar7/clock.c
> > @@ -0,0 +1,450 @@
> > [snipped]
> > +static void __init tnetd7300_init_clocks(void)
> > +{
> > +       u32 *bootcr = (u32 *)ioremap_nocache(AR7_REGS_DCL, 4);
> > +       struct tnetd7300_clocks *clocks =
> > +                                       (struct tnetd7300_clocks *)
> > +                                       ioremap_nocache(AR7_REGS_POWER + 0x20,
> > +                                       sizeof(struct tnetd7300_clocks));
> > +
> >
> Needless cast'ing and also should you not check that NULL is not 
> returned for both of these ioremap's?

That's because we "know" it can't ever fail for addresses < 0x20000000.
Downside - sparse will bitch about it.

But yes, the cast indeed is unnecessary.

  Ralf