This also greatly simplifies the power_{on,off} callbacks and make them
work on platform device id instead of checking the OHCI controller base
address like what was done in ohci-au1xxx.c.
Signed-off-by: Florian Fainelli <[email protected]>
---
arch/mips/alchemy/common/platform.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/arch/mips/alchemy/common/platform.c b/arch/mips/alchemy/common/platform.c
index 57335a2..cd12458 100644
--- a/arch/mips/alchemy/common/platform.c
+++ b/arch/mips/alchemy/common/platform.c
@@ -18,6 +18,7 @@
#include <linux/serial_8250.h>
#include <linux/slab.h>
#include <linux/usb/ehci_pdriver.h>
+#include <linux/usb/ohci_pdriver.h>
#include <asm/mach-au1x00/au1000.h>
#include <asm/mach-au1x00/au1xxx_dbdma.h>
@@ -142,6 +143,34 @@ static struct usb_ehci_pdata alchemy_ehci_pdata = {
.power_suspend = alchemy_ehci_power_off,
};
+/* Power on callback for the ohci platform driver */
+static int alchemy_ohci_power_on(struct platform_device *pdev)
+{
+ int unit;
+
+ unit = (pdev->id == 1) ?
+ ALCHEMY_USB_OHCI1 : ALCHEMY_USB_OHCI0;
+
+ return alchemy_usb_control(unit, 1);
+}
+
+/* Power off/suspend callback for the ohci platform driver */
+static void alchemy_ohci_power_off(struct platform_device *pdev)
+{
+ int unit;
+
+ unit = (pdev->id == 1) ?
+ ALCHEMY_USB_OHCI1 : ALCHEMY_USB_OHCI0;
+
+ alchemy_usb_control(unit, 0);
+}
+
+static struct usb_ohci_pdata alchemy_ohci_pdata = {
+ .power_on = alchemy_ohci_power_on,
+ .power_off = alchemy_ohci_power_off,
+ .power_suspend = alchemy_ohci_power_off,
+};
+
static unsigned long alchemy_ohci_data[][2] __initdata = {
[ALCHEMY_CPU_AU1000] = { AU1000_USB_OHCI_PHYS_ADDR, AU1000_USB_HOST_INT },
[ALCHEMY_CPU_AU1500] = { AU1000_USB_OHCI_PHYS_ADDR, AU1500_USB_HOST_INT },
@@ -192,6 +221,7 @@ static void __init alchemy_setup_usb(int ctype)
pdev->name = "au1xxx-ohci";
pdev->id = 0;
pdev->dev.dma_mask = &alchemy_ohci_dmamask;
+ pdev->dev.platform_data = &alchemy_ohci_pdata;
if (platform_device_register(pdev))
printk(KERN_INFO "Alchemy USB: cannot add OHCI0\n");
@@ -231,6 +261,7 @@ static void __init alchemy_setup_usb(int ctype)
pdev->name = "au1xxx-ohci";
pdev->id = 1;
pdev->dev.dma_mask = &alchemy_ohci_dmamask;
+ pdev->dev.platform_data = &alchemy_ohci_pdata;
if (platform_device_register(pdev))
printk(KERN_INFO "Alchemy USB: cannot add OHCI1\n");
--
1.7.9.5
On Wed, Oct 3, 2012 at 5:03 PM, Florian Fainelli <[email protected]> wrote:
> This also greatly simplifies the power_{on,off} callbacks and make them
> work on platform device id instead of checking the OHCI controller base
> address like what was done in ohci-au1xxx.c.
That was by design -- the base address is far more reliable in identifying the
correct controller instance than the platform device id. There are systems
in the field which don't use the alchemy/common/platform.c file at all.
Manuel
On Wednesday 03 October 2012 17:21:37 Manuel Lauss wrote:
> On Wed, Oct 3, 2012 at 5:03 PM, Florian Fainelli <[email protected]> wrote:
> > This also greatly simplifies the power_{on,off} callbacks and make them
> > work on platform device id instead of checking the OHCI controller base
> > address like what was done in ohci-au1xxx.c.
>
> That was by design -- the base address is far more reliable in identifying
the
> correct controller instance than the platform device id. There are systems
> in the field which don't use the alchemy/common/platform.c file at all.
Fair enough, but the way it was done previously was very error-prone if the
base address changed for any reason in the platform code, and you did not
notice it had to be changed in the OHCI driver too, then it simply did not
work. By systems in the field you mean out of tree users? If so, I'd say that
it's up to you to get them maintained or merged.
--
Florian
On Wed, Oct 3, 2012 at 5:24 PM, Florian Fainelli <[email protected]> wrote:
> On Wednesday 03 October 2012 17:21:37 Manuel Lauss wrote:
>> On Wed, Oct 3, 2012 at 5:03 PM, Florian Fainelli <[email protected]> wrote:
>> > This also greatly simplifies the power_{on,off} callbacks and make them
>> > work on platform device id instead of checking the OHCI controller base
>> > address like what was done in ohci-au1xxx.c.
>>
>> That was by design -- the base address is far more reliable in identifying
> the
>> correct controller instance than the platform device id. There are systems
>> in the field which don't use the alchemy/common/platform.c file at all.
>
> Fair enough, but the way it was done previously was very error-prone if the
> base address changed for any reason in the platform code, and you did not
> notice it had to be changed in the OHCI driver too, then it simply did not
Since the Alchemy line is dead this point is moot.
> work. By systems in the field you mean out of tree users? If so, I'd say that
> it's up to you to get them maintained or merged.
I'm not against the patch at all, quite the contrary.
Manuel
On Wed, Oct 3, 2012 at 5:03 PM, Florian Fainelli <[email protected]> wrote:
> This also greatly simplifies the power_{on,off} callbacks and make them
> work on platform device id instead of checking the OHCI controller base
> address like what was done in ohci-au1xxx.c.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> arch/mips/alchemy/common/platform.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/arch/mips/alchemy/common/platform.c b/arch/mips/alchemy/common/platform.c
> index 57335a2..cd12458 100644
> --- a/arch/mips/alchemy/common/platform.c
> +++ b/arch/mips/alchemy/common/platform.c
> @@ -18,6 +18,7 @@
> #include <linux/serial_8250.h>
> #include <linux/slab.h>
> #include <linux/usb/ehci_pdriver.h>
> +#include <linux/usb/ohci_pdriver.h>
>
> #include <asm/mach-au1x00/au1000.h>
> #include <asm/mach-au1x00/au1xxx_dbdma.h>
> @@ -142,6 +143,34 @@ static struct usb_ehci_pdata alchemy_ehci_pdata = {
> .power_suspend = alchemy_ehci_power_off,
> };
>
> +/* Power on callback for the ohci platform driver */
> +static int alchemy_ohci_power_on(struct platform_device *pdev)
> +{
> + int unit;
> +
> + unit = (pdev->id == 1) ?
> + ALCHEMY_USB_OHCI1 : ALCHEMY_USB_OHCI0;
> +
> + return alchemy_usb_control(unit, 1);
> +}
> +
> +/* Power off/suspend callback for the ohci platform driver */
> +static void alchemy_ohci_power_off(struct platform_device *pdev)
> +{
> + int unit;
> +
> + unit = (pdev->id == 1) ?
> + ALCHEMY_USB_OHCI1 : ALCHEMY_USB_OHCI0;
> +
> + alchemy_usb_control(unit, 0);
> +}
> +
> +static struct usb_ohci_pdata alchemy_ohci_pdata = {
> + .power_on = alchemy_ohci_power_on,
> + .power_off = alchemy_ohci_power_off,
> + .power_suspend = alchemy_ohci_power_off,
> +};
> +
> static unsigned long alchemy_ohci_data[][2] __initdata = {
> [ALCHEMY_CPU_AU1000] = { AU1000_USB_OHCI_PHYS_ADDR, AU1000_USB_HOST_INT },
> [ALCHEMY_CPU_AU1500] = { AU1000_USB_OHCI_PHYS_ADDR, AU1500_USB_HOST_INT },
> @@ -192,6 +221,7 @@ static void __init alchemy_setup_usb(int ctype)
> pdev->name = "au1xxx-ohci";
Should be "ohci-platform" (2x). With this change USB works on all my
Alchemy boards.
I'd also suggest to move drivers/usb/host/alchemy-common.c to
arch/mips/alchemy/common/usb.c.
(same for octeon2-common.c)
Manuel
On Wednesday 03 October 2012 18:07:28 Manuel Lauss wrote:
> On Wed, Oct 3, 2012 at 5:03 PM, Florian Fainelli <[email protected]> wrote:
> > This also greatly simplifies the power_{on,off} callbacks and make them
> > work on platform device id instead of checking the OHCI controller base
> > address like what was done in ohci-au1xxx.c.
> >
> > Signed-off-by: Florian Fainelli <[email protected]>
> > ---
> > arch/mips/alchemy/common/platform.c | 31
+++++++++++++++++++++++++++++++
> > 1 file changed, 31 insertions(+)
> >
> > diff --git a/arch/mips/alchemy/common/platform.c
b/arch/mips/alchemy/common/platform.c
> > index 57335a2..cd12458 100644
> > --- a/arch/mips/alchemy/common/platform.c
> > +++ b/arch/mips/alchemy/common/platform.c
> > @@ -18,6 +18,7 @@
> > #include <linux/serial_8250.h>
> > #include <linux/slab.h>
> > #include <linux/usb/ehci_pdriver.h>
> > +#include <linux/usb/ohci_pdriver.h>
> >
> > #include <asm/mach-au1x00/au1000.h>
> > #include <asm/mach-au1x00/au1xxx_dbdma.h>
> > @@ -142,6 +143,34 @@ static struct usb_ehci_pdata alchemy_ehci_pdata = {
> > .power_suspend = alchemy_ehci_power_off,
> > };
> >
> > +/* Power on callback for the ohci platform driver */
> > +static int alchemy_ohci_power_on(struct platform_device *pdev)
> > +{
> > + int unit;
> > +
> > + unit = (pdev->id == 1) ?
> > + ALCHEMY_USB_OHCI1 : ALCHEMY_USB_OHCI0;
> > +
> > + return alchemy_usb_control(unit, 1);
> > +}
> > +
> > +/* Power off/suspend callback for the ohci platform driver */
> > +static void alchemy_ohci_power_off(struct platform_device *pdev)
> > +{
> > + int unit;
> > +
> > + unit = (pdev->id == 1) ?
> > + ALCHEMY_USB_OHCI1 : ALCHEMY_USB_OHCI0;
> > +
> > + alchemy_usb_control(unit, 0);
> > +}
> > +
> > +static struct usb_ohci_pdata alchemy_ohci_pdata = {
> > + .power_on = alchemy_ohci_power_on,
> > + .power_off = alchemy_ohci_power_off,
> > + .power_suspend = alchemy_ohci_power_off,
> > +};
> > +
> > static unsigned long alchemy_ohci_data[][2] __initdata = {
> > [ALCHEMY_CPU_AU1000] = { AU1000_USB_OHCI_PHYS_ADDR,
AU1000_USB_HOST_INT },
> > [ALCHEMY_CPU_AU1500] = { AU1000_USB_OHCI_PHYS_ADDR,
AU1500_USB_HOST_INT },
> > @@ -192,6 +221,7 @@ static void __init alchemy_setup_usb(int ctype)
> > pdev->name = "au1xxx-ohci";
>
> Should be "ohci-platform" (2x). With this change USB works on all my
> Alchemy boards.
Yes, Hauke Merthens just pointed this issue at me.
> I'd also suggest to move drivers/usb/host/alchemy-common.c to
> arch/mips/alchemy/common/usb.c.
> (same for octeon2-common.c)
Ok, sounds good.
--
Florian