[PATCH] powerpc/40x: Add APM8018X SOC support

[Tanmay]

Hello,

Thanks for the comments. Please see inline.

Regards,
Tanmay

On Wed, Nov 23, 2011 at 3:53 PM, Paul Bolle <pebolle at tiscali.nl> wrote:

> Tanmay,
>
> (Some minor Kconfig related comments follow.)
>
> On Wed, 2011-11-23 at 15:14 +0530, Tanmay Inamdar wrote:
> > The AppliedMicro APM8018X embedded processor targets embedded
> applications that
> > require low power and a small footprint. It features a PowerPC 405
> processor
> > core built in a 65nm low-power CMOS process with a five-stage pipeline
> executing
> > up to one instruction per cycle. The family has 128-kbytes of on-chip
> memory,
> > a 128-bit local bus and on-chip DDR2 SDRAM controller with 16-bit
> interface.
> >
> >[...]
> >
> > Signed-off-by: Tanmay Inamdar <tinamdar at apm.com>
> > ---
> >  arch/powerpc/Kconfig                        |    6 +
> >  arch/powerpc/boot/dcr.h                     |    6 +
> >  arch/powerpc/boot/dts/klondike.dts          |  668 +++++++++++++
> >  arch/powerpc/configs/40x/klondike_defconfig | 1353
> +++++++++++++++++++++++++++
> >  arch/powerpc/include/asm/dcr-regs.h         |   20 +
> >  arch/powerpc/kernel/cputable.c              |   52 +
> >  arch/powerpc/kernel/udbg_16550.c            |   22 +
> >  arch/powerpc/platforms/40x/Kconfig          |   11 +
> >  arch/powerpc/platforms/40x/ppc40x_simple.c  |    4 +-
> >  9 files changed, 2141 insertions(+), 1 deletions(-)
> >  create mode 100644 arch/powerpc/boot/dts/klondike.dts
> >  create mode 100644 arch/powerpc/configs/40x/klondike_defconfig
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index b177caa..3f2cc36 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -978,3 +978,9 @@ config PPC_LIB_RHEAP
> >       bool
> >
> >  source "arch/powerpc/kvm/Kconfig"
> > +
> > +config UART_16550_WORD_ADDRESSABLE
> > +     bool
> > +     default n
> > +     help
> > +        Enable this if your UART 16550 is word addressable.
>
> This is only relevant for this SOC isn't it? If so, it might be better
> to add it to arch/powerpc/platforms/40x/Kconfig.
>
> The help line can be dropped (there's no prompt, so the help won't be
> user visible).
>
> Some people would suggest to use
>        def_bool n
>
> here. (I don't really care.)
>
Agreed. As Kumar Gala suggested in his comment, I will move this and make a
separate patch for UART changes.

>
> > [...]
>
> > diff --git a/arch/powerpc/kernel/udbg_16550.c
> b/arch/powerpc/kernel/udbg_16550.c
> > index 6837f83..dd3bce9 100644
> > --- a/arch/powerpc/kernel/udbg_16550.c
> > +++ b/arch/powerpc/kernel/udbg_16550.c
> > @@ -18,6 +18,19 @@ extern void real_writeb(u8 data, volatile u8 __iomem
> *addr);
> >  extern u8 real_205_readb(volatile u8 __iomem  *addr);
> >  extern void real_205_writeb(u8 data, volatile u8 __iomem *addr);
> >
> > +#ifdef CONFIG_UART_16550_WORD_ADDRESSABLE
> > +struct NS16550 {
> > +     /* this struct must be packed */
> > +     unsigned char rbr;  /* 0 */ u8 s0[3];
> > +     unsigned char ier;  /* 1 */ u8 s1[3];
> > +     unsigned char fcr;  /* 2 */ u8 s2[3];
> > +     unsigned char lcr;  /* 3 */ u8 s3[3];
> > +     unsigned char mcr;  /* 4 */ u8 s4[3];
> > +     unsigned char lsr;  /* 5 */ u8 s5[3];
> > +     unsigned char msr;  /* 6 */ u8 s6[3];
> > +     unsigned char scr;  /* 7 */ u8 s7[3];
> > +};
> > +#else
> >  struct NS16550 {
> >       /* this struct must be packed */
> >       unsigned char rbr;  /* 0 */
> > @@ -29,6 +42,7 @@ struct NS16550 {
> >       unsigned char msr;  /* 6 */
> >       unsigned char scr;  /* 7 */
> >  };
> > +#endif /* CONFIG_UART_16550_WORD_ADDRESSABLE */
> >
> >  #define thr rbr
> >  #define iir fcr
> > [...]
> > diff --git a/arch/powerpc/platforms/40x/Kconfig
> b/arch/powerpc/platforms/40x/Kconfig
> > index 1530229..3d0d1d9 100644
> > --- a/arch/powerpc/platforms/40x/Kconfig
> > +++ b/arch/powerpc/platforms/40x/Kconfig
> > @@ -186,3 +186,14 @@ config IBM405_ERR51
> >  #    bool
> >  #    depends on !STB03xxx && PPC4xx_DMA
> >  #    default y
> > +#
> > +
> > +config APM8018X
> > +     bool "APM8018X"
> > +     depends on 40x
> > +     default y
>
> Why is this "default y"? All other "selectors" of PPC40x_SIMPLE default
> to n.
>
> Yes. This is a mistake. This should be 'n'.


> > +     select PPC40x_SIMPLE
>
> There was recently some powerpc related discussion on using "select" on
> symbols that are themselves user selectable (see
> https://lkml.org/lkml/2011/11/9/426 ). But other symbols already select
> this symbol so this is not specific to this patch.
>
> > +     select UART_16550_WORD_ADDRESSABLE
> > +     help
> > +       This option enables support for the AppliedMicro Klondike board.
> > +
>
> Since you're selecting it here it's good that you made
> UART_16550_WORD_ADDRESSABLE hidden (as it has no prompt). But perhaps
> you could even drop it and in the code just test for CONFIG_APM8018X. Or
> are you expecting more users of UART_16550_WORD_ADDRESSABLE?
>
At this moment, it is only APM8018X SOC expects this. I am not sure about
more users of  UART_16550_WORD_ADDRESSABLE.  I will try to eliminate it or
make it as a run time selection.

>
> > [...]
>
>
> Paul Bolle
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20111124/b05da716/attachment-0001.html>