Hello,

Thanks for the comments. Please see inline.

Regards,
Tanmay

On Wed, Nov 23, 2011 at 3:53 PM, Paul Bolle [off-list ref] wrote:

Tanmay,

(Some minor Kconfig related comments follow.)

On Wed, 2011-11-23 at 15:14 +0530, Tanmay Inamdar wrote:

quoted

The AppliedMicro APM8018X embedded processor targets embedded

applications that

quoted

require low power and a small footprint. It features a PowerPC 405

processor

quoted

core built in a 65nm low-power CMOS process with a five-stage pipeline

executing

quoted

up to one instruction per cycle. The family has 128-kbytes of on-chip

memory,

quoted

a 128-bit local bus and on-chip DDR2 SDRAM controller with 16-bit

interface.

quoted

[...]

Signed-off-by: Tanmay Inamdar <redacted>
---
 arch/powerpc/Kconfig                        |    6 +
 arch/powerpc/boot/dcr.h                     |    6 +
 arch/powerpc/boot/dts/klondike.dts          |  668 +++++++++++++
 arch/powerpc/configs/40x/klondike_defconfig | 1353

+++++++++++++++++++++++++++

quoted

 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.

quoted

[...]

quoted

diff --git a/arch/powerpc/kernel/udbg_16550.c

b/arch/powerpc/kernel/udbg_16550.c

quoted

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

quoted

 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

quoted

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

quoted

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

quoted

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

quoted

[...]

Paul Bolle

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev