Re: [PATCH 1/4] powerpc/mpc512x: Add initial support for TWR-MPC5125
On Fri, Mar 18, 2011 at 02:35:24PM +0300, vooon...@gmail.com wrote:
> diff --git a/arch/powerpc/platforms/512x/clock.c
> b/arch/powerpc/platforms/512x/clock.c
> index 3dc2a8d..5cadf8e 100644
> --- a/arch/powerpc/platforms/512x/clock.c
> +++ b/arch/powerpc/platforms/512x/clock.c
> @@ -606,6 +606,21 @@ static void rate_clks_init(void)
> */
> struct clk dev_clks[2][32];
>
> +char *mpc512x_select_psc_compat(void)
> +{
> + char *psc_compats[] = {
> + "fsl,mpc5121-psc",
> + "fsl,mpc5125-psc"
> + };
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(psc_compats); i++)
> + if (of_find_compatible_node(NULL, NULL, psc_compats[i]))
> + return psc_compats[i];
> +
> + return NULL;
> +}
Function looks good to me. Shouldn't that rather be in mpc512x_shared?
> +// IOCTL registers for USB1/FEC2
No c++-style comments, please (here and later).
> +static void mpc5125_psc_iopad_init(void __iomem *ioctl, char *name)
> +{
> + struct device_node *np;
> + const u32 *cell_index;
> + char *default_psc = "fsl,mpc5125-psc";
> + char *psc_name;
> +
> + if (name)
> + psc_name = name;
> + else
> + psc_name = default_psc;
Caller sets name to NULL. Is this really used?
> +
> + for_each_compatible_node(np, NULL, psc_name) {
> + cell_index = of_get_property(np, "cell-index", NULL);
I seem to recall 'cell-index' is deprecated. Grant?
> + if (cell_index) {
> + u8 __iomem *pscioctl;
> + int psc_num = *cell_index;
> + if (psc_num > 1)
> + continue;
> +
> + pscioctl = ioctl + PSC_TO_IOCTL_OFFSET(psc_num);
> + out_8(pscioctl + IOCTL_PSCx_0, IOCTL_PSCx_0_MODE); //
> NOTE maybe wrong
Why is it 'maybe wrong'? Can it be improved somehow?
> + out_8(pscioctl + IOCTL_PSCx_1, IOCTL_DEFAULT_MODE);
> + out_8(pscioctl + IOCTL_PSCx_2, IOCTL_DEFAULT_MODE);
> + out_8(pscioctl + IOCTL_PSCx_3, IOCTL_DEFAULT_MODE);
> + out_8(pscioctl + IOCTL_PSCx_4, IOCTL_DEFAULT_MODE);
The defines make it much more readable, thanks.
> + }
> + }
> +}
Is this function really board-specific or platform specific?
> +static void mpc5125_fec2_usb_io_init(void __iomem *ioctl, int isusb)
> +{
> + int i;
> + const u8 offset[12] = {
> + IOCTL_USB1_DATA0, IOCTL_USB1_DATA1,
> + IOCTL_USB1_DATA2, IOCTL_USB1_DATA3,
> + IOCTL_USB1_DATA4, IOCTL_USB1_DATA5,
> + IOCTL_USB1_DATA6, IOCTL_USB1_DATA7,
> + IOCTL_USB1_STOP, IOCTL_USB1_CLK,
> + IOCTL_USB1_NEXT, IOCTL_USB1_DIR
> + };
> + u8 mode;
> +
> + mode = (isusb) ? IOCTL_DEFAULT_MODE : IOCTL_FEC2_MODE;
> + for (i = 0; i < ARRAY_SIZE(offset); i++)
> + out_8(ioctl + offset[i], mode);
> +}
Same question here and later. If it is board specific, the function name should
have something like 'twr' in it; but a few things seem mpc5125-generic
to me, if I am not mistaken?
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/4] powerpc/mpc512x: Add initial support for TWR-MPC5125
Hi Wolfram.
I create function for select compat string and give name for registers.
commit fe8895542d537567f43f99af8234e7326451197e
Author: Ermakov Vladimir
Date: Thu Mar 17 11:10:49 2011 +0300
Adds Freescale TWR-MPC5125 device tree and platform code.
Currently following is supported:
- NAND
- FEC1 and FEC2
- RTC
- PSC UART
Signed-off-by: Vladimir Ermakov
---
v2:
- add PSC compat string selection
- add ioctl defines
diff --git a/arch/powerpc/boot/dts/mpc5125twr.dts
b/arch/powerpc/boot/dts/mpc5125twr.dts
new file mode 100644
index 000..54f568f
--- /dev/null
+++ b/arch/powerpc/boot/dts/mpc5125twr.dts
@@ -0,0 +1,394 @@
+/*
+ * STx/Freescale ADS5125 MPC5125 silicon
+ *
+ * Copyright (C) 2009 Freescale Semiconductor Inc. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+/dts-v1/;
+
+/ {
+ model = "mpc5125ads";
+ compatible = "fsl,mpc5125ads";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ PowerPC,5125@0 {
+ device_type = "cpu";
+ reg = <0>;
+ d-cache-line-size = <0x20>; // 32 bytes
+ i-cache-line-size = <0x20>; // 32 bytes
+ d-cache-size = <0x8000>;// L1, 32K
+ i-cache-size = <0x8000>;// L1, 32K
+ timebase-frequency = <4950>;// 49.5 MHz (csb/4)
+ bus-frequency = <19800>;// 198 MHz csb bus
+ clock-frequency = <39600>; // 396 MHz ppc core
+ };
+ };
+
+ memory {
+ device_type = "memory";
+ reg = <0x 0x1000>; // 256MB at 0
+ };
+
+ sram@3000 {
+ compatible = "fsl,mpc5121-sram";
+ reg = <0x3000 0x08000>; // 32K at 0x3000
+ };
+
+ nfc@4000 {
+ compatible = "fsl,mpc5125-nfc";
+ reg = <0x4000 0x10>;// 1M at 0x4000
+ interrupts = <6 0x8>;
+ interrupt-parent = < &ipic >;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ bank-width = <1>;
+ write-size = <4096>;
+ spare-size = <128>;
+ chips = <1>;
+ // NOTE: partition map different than in BSP
+ nand-spl@0 {
+ label = "loader";
+ reg = <0x 0x0010>;
+ read-only;
+ };
+ uboot@10 {
+ label = "uboot";
+ reg = <0x0010 0x0010>;
+ read-only;
+ };
+ uboot-env@20 {
+ label = "uboot-env";
+ reg = <0x0020 0x0010>;
+ read-only;
+ };
+ kernel30 {
+ label = "kernel";
+ reg = <0x0030 0x0080>;
+ };
+ device-tree0 {
+ label = "device-tree";
+ reg = <0x00b0 0x0010>;
+ };
+ ramboot-rootfs@c0 {
+ label = "ramboot-rootfs";
+ reg = <0x00c0 0x0080>;
+ };
+ rootfs@140 {
+ label = "rootfs";
+ reg = <0x0140 0x0140>;
+ };
+ user@280 {
+ label = "user";
+ reg = <0x0280 0x0140>;
+ };
+ SRAM@420 {
+ label = "SRAM"; // NVRAM emul
+ reg = <0x0420 0x0140>;
+ };
+ prom@560 {
+ label = "prom";
+ reg = <0x0560 0x0140>;
+ };
+ //data@280 {
+ // label = "data";
+ // reg = <0x2800 0xeac0>;
+ //};
+ };
+
+ soc@8000 {
+ compatible = "fsl,mpc5121-immr";
+ device_type = "soc";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ #interrupt-cells = <2>;
+ ranges = <0x0 0x8000 0x40>;
+ reg = <0x8000 0x40>;
+ bus-frequency = <6600>; // 66 MHz ips bus
+
+
+ // IPIC
+ // interrupts cell =
+ // sen
Re: [PATCH 1/4] powerpc/mpc512x: Add initial support for TWR-MPC5125
Hi Vladimir,
(if possible, please provide a diffstat with the patches)
> diff --git a/arch/powerpc/platforms/512x/clock.c
> b/arch/powerpc/platforms/512x/clock.c
> index 3dc2a8d..962c0ba 100644
> --- a/arch/powerpc/platforms/512x/clock.c
> +++ b/arch/powerpc/platforms/512x/clock.c
> @@ -669,6 +669,13 @@ static void psc_calc_rate(struct clk *clk, int pscnum,
> struct device_node *np)
> clk->rate = mclk_src / mclk_div;
> }
>
> +
> +#ifdef CONFIG_PPC_MPC5125
> +#define PSC_PREFIX "mpc5125"
> +#else
> +#define PSC_PREFIX "mpc5121"
> +#endif
> +
> /*
> * Find all psc nodes in device tree and assign a clock
> * with name "psc%d_mclk" and dev pointing at the device
> @@ -680,7 +687,7 @@ static void psc_clks_init(void)
> const u32 *cell_index;
> struct platform_device *ofdev;
>
> - for_each_compatible_node(np, NULL, "fsl,mpc5121-psc") {
> + for_each_compatible_node(np, NULL, "fsl," PSC_PREFIX "-psc") {
Uh, that makes it impossible to have one kernel for mpc5121/5.
> -void __init mpc512x_psc_fifo_init(void)
> +void __init mpc512x_psc_fifo_init(char *psc_name)
> {
> struct device_node *np;
> void __iomem *psc;
> unsigned int tx_fifo_size;
> unsigned int rx_fifo_size;
> + char *default_psc = "fsl,mpc5121-psc";
> int fifobase = 0; /* current fifo address in 32 bit words */
>
> - for_each_compatible_node(np, NULL, "fsl,mpc5121-psc") {
> + if (!psc_name)
> + psc_name = default_psc;
> +
> + for_each_compatible_node(np, NULL, psc_name) {
I think this goes more to the right direction, although you passed the
non-default string for mpc5125 in the board-config, which is the wrong place,
because it is a platform thing.
What about something like:
if of_find_compatible_node(startpoint, NULL, "fsl,mpc5121-psc")
psc_compat = "fsl,mpc5121-psc";
else if of_find_compatible_node(startpoint, NULL, "fsl,mpc5125-psc")
psc_compat = "fsl,mpc5125-psc";
else if
/* Problem handling */
Dunno, might be worth to put it into a function as it could be used here and in
the block above.
Also, I noticed quite a number of magic values (e.g. 0x76). I guess those are
register and bit names, which should be used instead.
Thanks,
Wolfram
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/4] powerpc/mpc512x: Add initial support for TWR-MPC5125
Adds Freescale TWR-MPC5125 device tree and platform code.
Currently following is supported:
- NAND
- FEC1 and FEC2
- RTC
- PSC UART
Signed-off-by: Vladimir Ermakov
---
diff --git a/arch/powerpc/boot/dts/mpc5125twr.dts
b/arch/powerpc/boot/dts/mpc5125twr.dts
new file mode 100644
index 000..54f568f
--- /dev/null
+++ b/arch/powerpc/boot/dts/mpc5125twr.dts
@@ -0,0 +1,394 @@
+/*
+ * STx/Freescale ADS5125 MPC5125 silicon
+ *
+ * Copyright (C) 2009 Freescale Semiconductor Inc. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+/dts-v1/;
+
+/ {
+ model = "mpc5125ads";
+ compatible = "fsl,mpc5125ads";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ PowerPC,5125@0 {
+ device_type = "cpu";
+ reg = <0>;
+ d-cache-line-size = <0x20>; // 32 bytes
+ i-cache-line-size = <0x20>; // 32 bytes
+ d-cache-size = <0x8000>;// L1, 32K
+ i-cache-size = <0x8000>;// L1, 32K
+ timebase-frequency = <4950>;// 49.5 MHz (csb/4)
+ bus-frequency = <19800>;// 198 MHz csb bus
+ clock-frequency = <39600>; // 396 MHz ppc core
+ };
+ };
+
+ memory {
+ device_type = "memory";
+ reg = <0x 0x1000>; // 256MB at 0
+ };
+
+ sram@3000 {
+ compatible = "fsl,mpc5121-sram";
+ reg = <0x3000 0x08000>; // 32K at 0x3000
+ };
+
+ nfc@4000 {
+ compatible = "fsl,mpc5125-nfc";
+ reg = <0x4000 0x10>;// 1M at 0x4000
+ interrupts = <6 0x8>;
+ interrupt-parent = < &ipic >;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ bank-width = <1>;
+ write-size = <4096>;
+ spare-size = <128>;
+ chips = <1>;
+ // NOTE: partition map different than in BSP
+ nand-spl@0 {
+ label = "loader";
+ reg = <0x 0x0010>;
+ read-only;
+ };
+ uboot@10 {
+ label = "uboot";
+ reg = <0x0010 0x0010>;
+ read-only;
+ };
+ uboot-env@20 {
+ label = "uboot-env";
+ reg = <0x0020 0x0010>;
+ read-only;
+ };
+ kernel30 {
+ label = "kernel";
+ reg = <0x0030 0x0080>;
+ };
+ device-tree0 {
+ label = "device-tree";
+ reg = <0x00b0 0x0010>;
+ };
+ ramboot-rootfs@c0 {
+ label = "ramboot-rootfs";
+ reg = <0x00c0 0x0080>;
+ };
+ rootfs@140 {
+ label = "rootfs";
+ reg = <0x0140 0x0140>;
+ };
+ user@280 {
+ label = "user";
+ reg = <0x0280 0x0140>;
+ };
+ SRAM@420 {
+ label = "SRAM"; // NVRAM emul
+ reg = <0x0420 0x0140>;
+ };
+ prom@560 {
+ label = "prom";
+ reg = <0x0560 0x0140>;
+ };
+ //data@280 {
+ // label = "data";
+ // reg = <0x2800 0xeac0>;
+ //};
+ };
+
+ soc@8000 {
+ compatible = "fsl,mpc5121-immr";
+ device_type = "soc";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ #interrupt-cells = <2>;
+ ranges = <0x0 0x8000 0x40>;
+ reg = <0x8000 0x40>;
+ bus-frequency = <6600>; // 66 MHz ips bus
+
+
+ // IPIC
+ // interrupts cell =
+ // sense values match linux IORESOURCE_IRQ_* defines:
+ // sense == 8: Level, low assertion
+ // sense == 2: Edge, high-to-low change
+ //
+ ipic: interrupt-controller@c00 {
+ compatible = "fsl,mpc5121-ipic", "fsl,ipic";
+
