2010-11-23 12:57:12

by Rupjyoti Sarmah

[permalink] [raw]
Subject: [PATCH v1] ppc44x:PHY fixup for USB on canyonlands board

This fix is a reset for USB PHY that requires some amount of time for power to be stable on Canyonlands.

Signed-off-by: Rupjyoti Sarmah <[email protected]>
---
arch/powerpc/boot/dts/canyonlands.dts | 13 +++
arch/powerpc/platforms/44x/44x.h | 5 +
arch/powerpc/platforms/44x/Makefile | 1 +
arch/powerpc/platforms/44x/canyonlands.c | 122 ++++++++++++++++++++++++++++
arch/powerpc/platforms/44x/ppc44x_simple.c | 1 -
5 files changed, 141 insertions(+), 1 deletions(-)
create mode 100644 arch/powerpc/platforms/44x/canyonlands.c

diff --git a/arch/powerpc/boot/dts/canyonlands.dts b/arch/powerpc/boot/dts/canyonlands.dts
index a303703..a9f7538 100644
--- a/arch/powerpc/boot/dts/canyonlands.dts
+++ b/arch/powerpc/boot/dts/canyonlands.dts
@@ -224,6 +224,13 @@
};
};

+ cpld@2,0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "apm,ppc460ex-bcsr";
+ reg = <2 0x0 0x9>;
+ };
+
ndfc@3,0 {
compatible = "ibm,ndfc";
reg = <0x00000003 0x00000000 0x00002000>;
@@ -320,6 +327,12 @@
interrupts = <0x3 0x4>;
};

+ GPIO0: gpio@ef600b00 {
+ compatible = "ibm,ppc4xx-gpio";
+ reg = <0xef600b00 0x00000048>;
+ gpio-controller;
+ };
+
ZMII0: emac-zmii@ef600d00 {
compatible = "ibm,zmii-460ex", "ibm,zmii";
reg = <0xef600d00 0x0000000c>;
diff --git a/arch/powerpc/platforms/44x/44x.h b/arch/powerpc/platforms/44x/44x.h
index dbc4d2b..bc2ab7a 100644
--- a/arch/powerpc/platforms/44x/44x.h
+++ b/arch/powerpc/platforms/44x/44x.h
@@ -4,4 +4,9 @@
extern u8 as1_readb(volatile u8 __iomem *addr);
extern void as1_writeb(u8 data, volatile u8 __iomem *addr);

+#define BCSR_USB_EN 0x11
+#define GPIO0_OSRH 0xC
+#define GPIO0_TSRH 0x14
+#define GPIO0_ISR1H 0x34
+
#endif /* __POWERPC_PLATFORMS_44X_44X_H */
diff --git a/arch/powerpc/platforms/44x/Makefile b/arch/powerpc/platforms/44x/Makefile
index 82ff326..6854e73 100644
--- a/arch/powerpc/platforms/44x/Makefile
+++ b/arch/powerpc/platforms/44x/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_WARP) += warp.o
obj-$(CONFIG_XILINX_VIRTEX_5_FXT) += virtex.o
obj-$(CONFIG_XILINX_ML510) += virtex_ml510.o
obj-$(CONFIG_ISS4xx) += iss4xx.o
+obj-$(CONFIG_CANYONLANDS)+= canyonlands.o
diff --git a/arch/powerpc/platforms/44x/canyonlands.c b/arch/powerpc/platforms/44x/canyonlands.c
new file mode 100644
index 0000000..f13b62f
--- /dev/null
+++ b/arch/powerpc/platforms/44x/canyonlands.c
@@ -0,0 +1,122 @@
+/*
+ * This contain platform specific code for Canyonalnds board based on
+ * APM ppc44x series of processors.
+ *
+ * Copyright (c) 2010, Applied Micro Circuits Corporation
+ * Author: Rupjyoti Sarmah <[email protected]>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <asm/pci-bridge.h>
+#include <asm/ppc4xx.h>
+#include <asm/udbg.h>
+#include <asm/uic.h>
+#include <linux/of_platform.h>
+#include "44x.h"
+
+static __initdata struct of_device_id ppc44x_of_bus[] = {
+ { .compatible = "ibm,plb4", },
+ { .compatible = "ibm,opb", },
+ { .compatible = "ibm,ebc", },
+ { .compatible = "simple-bus", },
+ {},
+};
+
+static int __init ppc44x_device_probe(void)
+{
+ of_platform_bus_probe(NULL, ppc44x_of_bus, NULL);
+
+ return 0;
+}
+machine_device_initcall(canyonlands, ppc44x_device_probe);
+
+/* Using this code only for the Canyonlands board. */
+
+static int __init ppc44x_probe(void)
+{
+ unsigned long root = of_get_flat_dt_root();
+ if (of_flat_dt_is_compatible(root, "amcc,canyonlands")) {
+ ppc_pci_set_flags(PPC_PCI_REASSIGN_ALL_RSRC);
+ return 1;
+ }
+ return 0;
+}
+
+/* PHY fixup code on Canyonlands kit. */
+
+static int __init ppc460ex_canyonlands_fixup(void)
+{
+ u8 __iomem *bcsr ;
+ void __iomem *vaddr;
+ struct device_node *np;
+ u32 val ;
+
+ np = of_find_compatible_node(NULL, NULL, "apm,ppc460ex-bcsr");
+ if (!np) {
+ printk(KERN_ERR "failed did not find apm, ppc460ex bcsr node\n");
+ return -ENODEV;
+ }
+
+ bcsr = of_iomap(np, 0);
+ of_node_put(np);
+
+ if (!bcsr) {
+ printk(KERN_CRIT "Could not remap bcsr\n");
+ return -ENODEV;
+ }
+
+ np = of_find_compatible_node(NULL, NULL, "ibm,ppc4xx-gpio");
+ vaddr = of_iomap(np, 0);
+ if (!vaddr) {
+ printk(KERN_CRIT "Could not get gpio node address\n");
+ return -ENODEV;
+ }
+
+ setbits8(&bcsr[7], BCSR_USB_EN);
+ udelay(100000);
+
+ clrbits8(&bcsr[7], BCSR_USB_EN);
+ udelay(100000);
+
+ /* configure gpio16 and gpio19 as alternate1 */
+
+ /* GPIO0_ISR1H for alternate 1 settings */
+ val = in_be32(vaddr + GPIO0_ISR1H);
+ out_be32((vaddr + GPIO0_ISR1H), val | 0x4200000);
+
+ /* GPIO0_OSRH for alternate 1 settings */
+ val = in_be32(vaddr + GPIO0_OSRH);
+ out_be32((vaddr + GPIO0_OSRH), val | 0x42000000);
+
+ /* GPIO0_TSRH for alternate 1 settings */
+ val = in_be32(vaddr + GPIO0_TSRH);
+ out_be32((vaddr + GPIO0_TSRH), val | 0x42000000);
+ of_node_put(np);
+ return 0;
+}
+machine_device_initcall(canyonlands, ppc460ex_canyonlands_fixup);
+define_machine(canyonlands) {
+ .name = "Canyonlands",
+ .probe = ppc44x_probe,
+ .progress = udbg_progress,
+ .init_IRQ = uic_init_tree,
+ .get_irq = uic_get_irq,
+ .restart = ppc4xx_reset_system,
+ .calibrate_decr = generic_calibrate_decr,
+};
diff --git a/arch/powerpc/platforms/44x/ppc44x_simple.c b/arch/powerpc/platforms/44x/ppc44x_simple.c
index 7ddcba3..c81c19c 100644
--- a/arch/powerpc/platforms/44x/ppc44x_simple.c
+++ b/arch/powerpc/platforms/44x/ppc44x_simple.c
@@ -53,7 +53,6 @@ static char *board[] __initdata = {
"amcc,arches",
"amcc,bamboo",
"amcc,bluestone",
- "amcc,canyonlands",
"amcc,glacier",
"ibm,ebony",
"amcc,eiger",
--
1.5.6.3


2010-11-23 13:21:22

by Stefan Roese

[permalink] [raw]
Subject: Re: [PATCH v1] ppc44x:PHY fixup for USB on canyonlands board

Hi Rup,

On Tuesday 23 November 2010 13:56:53 Rupjyoti Sarmah wrote:
> This fix is a reset for USB PHY that requires some amount of time for power
> to be stable on Canyonlands.

Since this is version 2 of your patch, "[PATCH v2]" would have been a bit
better in the subject line. Its also a good practice to summarize the changes
between patch versions below the "---" line.

Please find a some further comments below.

<snip>

> --- a/arch/powerpc/platforms/44x/44x.h
> +++ b/arch/powerpc/platforms/44x/44x.h
> @@ -4,4 +4,9 @@
> extern u8 as1_readb(volatile u8 __iomem *addr);
> extern void as1_writeb(u8 data, volatile u8 __iomem *addr);
>
> +#define BCSR_USB_EN 0x11

This define is wrong here. Its not common for all 44x platforms but
Canyonlands specific.

> +#define GPIO0_OSRH 0xC
> +#define GPIO0_TSRH 0x14
> +#define GPIO0_ISR1H 0x34
> +
> #endif /* __POWERPC_PLATFORMS_44X_44X_H */
> diff --git a/arch/powerpc/platforms/44x/Makefile
> b/arch/powerpc/platforms/44x/Makefile index 82ff326..6854e73 100644
> --- a/arch/powerpc/platforms/44x/Makefile
> +++ b/arch/powerpc/platforms/44x/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_WARP) += warp.o
> obj-$(CONFIG_XILINX_VIRTEX_5_FXT) += virtex.o
> obj-$(CONFIG_XILINX_ML510) += virtex_ml510.o
> obj-$(CONFIG_ISS4xx) += iss4xx.o
> +obj-$(CONFIG_CANYONLANDS)+= canyonlands.o
> diff --git a/arch/powerpc/platforms/44x/canyonlands.c
> b/arch/powerpc/platforms/44x/canyonlands.c new file mode 100644
> index 0000000..f13b62f
> --- /dev/null
> +++ b/arch/powerpc/platforms/44x/canyonlands.c
> @@ -0,0 +1,122 @@
> +/*
> + * This contain platform specific code for Canyonalnds board based on
^^^^^^^^^^^
Canyonlands

> + * APM ppc44x series of processors.
> + *
> + * Copyright (c) 2010, Applied Micro Circuits Corporation
> + * Author: Rupjyoti Sarmah <[email protected]>
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + *
> + */
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <asm/pci-bridge.h>
> +#include <asm/ppc4xx.h>
> +#include <asm/udbg.h>
> +#include <asm/uic.h>
> +#include <linux/of_platform.h>
> +#include "44x.h"
> +
> +static __initdata struct of_device_id ppc44x_of_bus[] = {
> + { .compatible = "ibm,plb4", },
> + { .compatible = "ibm,opb", },
> + { .compatible = "ibm,ebc", },
> + { .compatible = "simple-bus", },
> + {},
> +};
> +
> +static int __init ppc44x_device_probe(void)
> +{
> + of_platform_bus_probe(NULL, ppc44x_of_bus, NULL);
> +
> + return 0;
> +}
> +machine_device_initcall(canyonlands, ppc44x_device_probe);
> +
> +/* Using this code only for the Canyonlands board. */
> +
> +static int __init ppc44x_probe(void)
> +{
> + unsigned long root = of_get_flat_dt_root();
> + if (of_flat_dt_is_compatible(root, "amcc,canyonlands")) {
> + ppc_pci_set_flags(PPC_PCI_REASSIGN_ALL_RSRC);
> + return 1;
> + }
> + return 0;
> +}
> +
> +/* PHY fixup code on Canyonlands kit. */

PHY is a bit unspecific. One might think that this is an ethernet PHY (see
remark below about better comments in the code)?

> +
> +static int __init ppc460ex_canyonlands_fixup(void)
> +{
> + u8 __iomem *bcsr ;
> + void __iomem *vaddr;

Double space before *vaddr.

> + struct device_node *np;
> + u32 val ;
> +
> + np = of_find_compatible_node(NULL, NULL, "apm,ppc460ex-bcsr");
> + if (!np) {
> + printk(KERN_ERR "failed did not find apm, ppc460ex bcsr
node\n");
> + return -ENODEV;
> + }
> +
> + bcsr = of_iomap(np, 0);
> + of_node_put(np);
> +
> + if (!bcsr) {
> + printk(KERN_CRIT "Could not remap bcsr\n");
> + return -ENODEV;
> + }
> +
> + np = of_find_compatible_node(NULL, NULL, "ibm,ppc4xx-gpio");
> + vaddr = of_iomap(np, 0);
> + if (!vaddr) {
> + printk(KERN_CRIT "Could not get gpio node address\n");
> + return -ENODEV;
> + }
> +
> + setbits8(&bcsr[7], BCSR_USB_EN);
> + udelay(100000);
> +
> + clrbits8(&bcsr[7], BCSR_USB_EN);
> + udelay(100000);

Thats a total bootup delay of 200ms. Is this really needed?

> +
> + /* configure gpio16 and gpio19 as alternate1 */
> +
> + /* GPIO0_ISR1H for alternate 1 settings */
> + val = in_be32(vaddr + GPIO0_ISR1H);
> + out_be32((vaddr + GPIO0_ISR1H), val | 0x4200000);

setbits32 might be even simpler.

> + /* GPIO0_OSRH for alternate 1 settings */
> + val = in_be32(vaddr + GPIO0_OSRH);
> + out_be32((vaddr + GPIO0_OSRH), val | 0x42000000);

Same here.

> + /* GPIO0_TSRH for alternate 1 settings */
> + val = in_be32(vaddr + GPIO0_TSRH);
> + out_be32((vaddr + GPIO0_TSRH), val | 0x42000000);

And here.

And I suggest to add a few comments to the code explaining why exactly you are
setting/clearing the bits in the BCSR and the GPIO registers.

Cheers,
Stefan

2010-11-23 21:10:51

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH v1] ppc44x:PHY fixup for USB on canyonlands board


> > + setbits8(&bcsr[7], BCSR_USB_EN);
> > + udelay(100000);
> > +
> > + clrbits8(&bcsr[7], BCSR_USB_EN);
> > + udelay(100000);
>
> Thats a total bootup delay of 200ms. Is this really needed?

In addition, so large delays should use msleep() if possible (depends
how early we are here).

Cheers,
Ben,

> And I suggest to add a few comments to the code explaining why exactly you are
> setting/clearing the bits in the BCSR and the GPIO registers.

Seconded,

Cheers,
Ben.

2010-11-24 05:00:13

by Rupjyoti Sarmah

[permalink] [raw]
Subject: RE: [PATCH v1] ppc44x:PHY fixup for USB on canyonlands board

>>
>> +#define BCSR_USB_EN 0x11
>This define is wrong here. Its not common for all 44x platforms but
Canyonlands specific.

So, do you suggest to move this macro to the canyonlands.c ? Or introduce
canyonlands.h ?

Regards,
Rup

2010-11-24 05:30:34

by Stefan Roese

[permalink] [raw]
Subject: Re: [PATCH v1] ppc44x:PHY fixup for USB on canyonlands board

On Wednesday 24 November 2010 05:55:03 Rupjyoti Sarmah wrote:
> >> +#define BCSR_USB_EN 0x11
> >
> >This define is wrong here. Its not common for all 44x platforms but
>
> Canyonlands specific.
>
> So, do you suggest to move this macro to the canyonlands.c ? Or introduce
> canyonlands.h ?

canyonlands.c is fine with me.

Cheers,
Stefan

2010-11-24 08:46:15

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: RE: [PATCH v1] ppc44x:PHY fixup for USB on canyonlands board

On Wed, 2010-11-24 at 10:25 +0530, Rupjyoti Sarmah wrote:
>
> So, do you suggest to move this macro to the canyonlands.c ? Or
> introduce
> canyonlands.h ?

Just move it to the .c file.

Cheers,
Ben.