Hello Arnd,<br><br>Thanks for the comments. Please see inline replies.<br><br>Regards,<br>Tanmay<br><br><div class="gmail_quote">On Wed, Nov 23, 2011 at 7:45 PM, Arnd Bergmann <span dir="ltr"><<a href="mailto:arnd@arndb.de" target="_blank">arnd@arndb.de</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>On Wednesday 23 November 2011, Tanmay Inamdar wrote:<br>
> The AppliedMicro APM8018X embedded processor targets embedded applications that<br>
> require low power and a small footprint. It features a PowerPC 405 processor<br>
> core built in a 65nm low-power CMOS process with a five-stage pipeline executing<br>
> up to one instruction per cycle. The family has 128-kbytes of on-chip memory,<br>
> a 128-bit local bus and on-chip DDR2 SDRAM controller with 16-bit interface.<br>
<br>
</div>Hi Tanmay,<br>
<div><br>
> +#if defined(CONFIG_APM8018X)<br>
> +#define DCRN_CPR0_ADDR 0xa<br>
> +#define DCRN_CPR0_DATA 0xb<br>
> +#else<br>
> /* 440EP Clock/Power-on Reset regs */<br>
> #define DCRN_CPR0_ADDR 0xc<br>
> #define DCRN_CPR0_DATA 0xd<br>
> +#endif /* CONFIG_APM8018X */<br>
<br>
</div>This prevents you from building one kernel that runs on both APM8018X and<br>
others. Better define a new constant name for the new registers and select<br>
the right one at run-time.<br>
<div><br>
> diff --git a/arch/powerpc/boot/dts/klondike.dts b/arch/powerpc/boot/dts/klondike.dts<br>
> new file mode 100644<br>
> index 0000000..9372a52<br>
> --- /dev/null<br>
> +++ b/arch/powerpc/boot/dts/klondike.dts<br>
> @@ -0,0 +1,668 @@<br>
> +/*<br>
> + * Device Tree Source for AMCC Klondike (405)<br>
> + *<br>
<br>
</div>The device tree file for the most part describes the chip, but partly also<br>
the board. Have you considered splitting the soc parts into a .dtsi file<br>
and moving all the configuration into a separate file?<br></blockquote><div><br>Thanks for suggesting this. I will consider using .dtsi now. <br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><br>
<br>
> diff --git a/arch/powerpc/configs/40x/klondike_defconfig b/arch/powerpc/configs/40x/klondike_defconfig<br>
> new file mode 100644<br>
> index 0000000..840f438<br>
> --- /dev/null<br>
> +++ b/arch/powerpc/configs/40x/klondike_defconfig<br>
> @@ -0,0 +1,1353 @@<br>
> +#<br>
> +# Automatically generated file; DO NOT EDIT.<br>
> +# Linux/powerpc 3.2.0-rc2 Kernel Configuration<br>
> +#<br>
> +# CONFIG_PPC64 is not set<br>
> +<br>
<br>
</div>Please use 'make savedefconfig' to create a minimal defconfig file instead of listing<br>
the full configuration here.<br></blockquote><div><br>Yes. <br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><br>
> diff --git a/arch/powerpc/include/asm/dcr-regs.h b/arch/powerpc/include/asm/dcr-regs.h<br>
> index 380274d..c900cfd 100644<br>
> --- a/arch/powerpc/include/asm/dcr-regs.h<br>
> +++ b/arch/powerpc/include/asm/dcr-regs.h<br>
> @@ -24,9 +24,18 @@<br>
> * of the driver main register set<br>
> */<br>
><br>
> +#if defined(CONFIG_APM8018X)<br>
> +/* CPR */<br>
> +#define DCRN_CPR0_CONFIG_ADDR 0xa<br>
> +#define DCRN_CPR1_CONFIG_DATA 0xb<br>
> +/* AHB */<br>
> +#define DCRN_SDR1_CONFIG_ADDR 0xc<br>
> +#define DCRN_SDR1_CONFIG_DATA 0xd<br>
> +#else<br>
> /* CPRs (440GX and 440SP/440SPe) */<br>
> #define DCRN_CPR0_CONFIG_ADDR 0xc<br>
> #define DCRN_CPR0_CONFIG_DATA 0xd<br>
> +#endif /* CONFIG_APM8018X */<br>
<br>
</div>same comment as above.<br></blockquote><div> </div><div>Some existing drivers use these macros. If I change the names, I will have to update the<br>driver code.<br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><br>
> diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c<br>
> index edae5bb..e5c51a6 100644<br>
> --- a/arch/powerpc/kernel/cputable.c<br>
> +++ b/arch/powerpc/kernel/cputable.c<br>
> @@ -1505,6 +1505,58 @@ static struct cpu_spec __initdata cpu_specs[] = {<br>
> .machine_check = machine_check_4xx,<br>
> .platform = "ppc405",<br>
> },<br>
> + { /* APM80186-SK */<br>
> + .pvr_mask = 0xffffffff,<br>
> + .pvr_value = 0x7ff11432,<br>
<br>
</div>If you mask out the lower bits, you only need a single entry instead of<br>
four identical ones.<br></blockquote><div><br>You are right. But each PVR represent different version of SOC. If I create single<br>entry, then I will have to give generic cpu_name which I don't want.<br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div></div><div><br>
> --- a/arch/powerpc/kernel/udbg_16550.c<br>
> +++ b/arch/powerpc/kernel/udbg_16550.c<br>
> @@ -18,6 +18,19 @@ extern void real_writeb(u8 data, volatile u8 __iomem *addr);<br>
> extern u8 real_205_readb(volatile u8 __iomem *addr);<br>
> extern void real_205_writeb(u8 data, volatile u8 __iomem *addr);<br>
><br>
> +#ifdef CONFIG_UART_16550_WORD_ADDRESSABLE<br>
> +struct NS16550 {<br>
> + /* this struct must be packed */<br>
> + unsigned char rbr; /* 0 */ u8 s0[3];<br>
> + unsigned char ier; /* 1 */ u8 s1[3];<br>
> + unsigned char fcr; /* 2 */ u8 s2[3];<br>
> + unsigned char lcr; /* 3 */ u8 s3[3];<br>
> + unsigned char mcr; /* 4 */ u8 s4[3];<br>
> + unsigned char lsr; /* 5 */ u8 s5[3];<br>
> + unsigned char msr; /* 6 */ u8 s6[3];<br>
> + unsigned char scr; /* 7 */ u8 s7[3];<br>
> +};<br>
> +#else<br>
> struct NS16550 {<br>
> /* this struct must be packed */<br>
> unsigned char rbr; /* 0 */<br>
> @@ -29,6 +42,7 @@ struct NS16550 {<br>
> unsigned char msr; /* 6 */<br>
> unsigned char scr; /* 7 */<br>
> };<br>
> +#endif /* CONFIG_UART_16550_WORD_ADDRESSABLE */<br>
><br>
<br>
</div></div>Same things as with the register definitions. Please make this<br>
run-time selectable to allow build-time configurations that<br>
support both layouts.<br></blockquote><div><br>Yes. I will try to find better solution. <br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<font color="#888888"><br>
Arnd<br>
</font></blockquote></div><br>
<pre>CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
is for the sole use of the intended recipient(s) and contains information
that is confidential and proprietary to AppliedMicro Corporation or its subsidiaries.
It is to be used solely for the purpose of furthering the parties' business relationship.
All unauthorized review, use, disclosure or distribution is prohibited.
If you are not the intended recipient, please contact the sender by reply e-mail
and destroy all copies of the original message.