USB/ISP1760: Add support for the generic platfrom device centralized driver model
PATCH 1/1 USB/ISP1760: Add support for the generic platfrom device centralized driver model
From: Michael Hennerich <[email protected]>
--- drivers/usb/host/isp1760-if.c | 98 +++++++++++++++++++++++++++++++++++++++++ 1 files changed, 98 insertions(+), 0 deletions(-)
drivers/usb/host/isp1760-if.c +98 -0
@@ -3,6 +3,7 @@* Currently there is support for * - OpenFirmware * - PCI +* - PDEV (generic platform device centralized driver model) * * (c) 2007 Sebastian Siewior <[email protected]> *
@@ -23,6 +24,12 @@#include <linux/pci.h> #endif +#if !defined(CONFIG_USB_ISP1760_OF ) && !defined(CONFIG_USB_ISP1760_PCI ) +#define USB_ISP1760_PDEV +#include <linux/platform_device.h> +#include <linux/usb/isp1760.h> +#endif +#ifdef CONFIG_USB_ISP1760_OF static int of_isp1760_probe (struct of_device *dev,const struct of_device_id *match)
@@ -286,12 +293,100 @@ static struct pci_driver isp1761_pci_driver = { };#endif +#ifdef USB_ISP1760_PDEV +static int __devinit +isp1760_pdev_probe (struct platform_device *pdev) +{ +struct usb_hcd *hcd; +struct resource *addr; +int irq; +unsigned int devflags =0 ; +struct isp1760_platform_data *priv = pdev->dev.platform_data; + +/* basic sanity checks first. board-specific init logic should +* have initialized these two resources and probably board +* specific platform_data. we don't probe for IRQs, and do only +* minimal sanity checking. +*/ + +if (usb_disabled ()) +return -ENODEV ; + +if (pdev->num_resources <2 ) +return -ENODEV ; + + addr =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + irq =platform_get_irq (pdev,0 ); + +if (!addr || irq <0 ) +return -ENODEV ; + +if (priv) { +if (priv->is_isp1761) + devflags |=ISP1760_FLAG_ISP1761 ; +if (priv->port1_disable) + devflags |=ISP1760_FLAG_PORT1_DIS ; +if (priv->bus_width_16) + devflags |=ISP1760_FLAG_BUS_WIDTH_16 ; +if (priv->port1_otg) + devflags |=ISP1760_FLAG_OTG_EN ; +if (priv->analog_oc) + devflags |=ISP1760_FLAG_ANALOG_OC ; +if (priv->dack_polarity_high) + devflags |=ISP1760_FLAG_DACK_POL_HIGH ; +if (priv->dreq_polarity_high) + devflags |=ISP1760_FLAG_DREQ_POL_HIGH ; + } + + hcd =isp1760_register (addr->start,resource_size (addr), irq, +IRQF_DISABLED |IRQF_SHARED |IRQF_TRIGGER_FALLING , + &pdev->dev,dev_name (&pdev->dev), + devflags); + +if (IS_ERR (hcd)) +return PTR_ERR (hcd); + +return 0 ; +} + +static int __devexit +isp1760_pdev_remove (struct platform_device *pdev) +{ +struct usb_hcd *hcd =platform_get_drvdata (pdev); + +platform_set_drvdata (pdev,NULL ); + +usb_remove_hcd (hcd); +iounmap (hcd->regs); +usb_put_hcd (hcd); +return 0 ; +} + +/* this driver is exported so sl811_cs can depend on it */ +struct platform_driver isp1760_pdev_driver = { + .probe = isp1760_pdev_probe, + .remove =__devexit_p (isp1760_pdev_remove), + .driver = { + .name ="isp1760-hcd" , + .owner =THIS_MODULE , + }, +}; +#endif /* USB_ISP1760_PDEV */ +static int __init isp1760_init (void ) {int ret = -ENODEV ;init_kmem_once (); +#ifdef USB_ISP1760_PDEV + ret =platform_driver_register (&isp1760_pdev_driver); +if (ret) { +deinit_kmem_cache (); +return ret; + } +#endif +#ifdef CONFIG_USB_ISP1760_OF ret =of_register_platform_driver (&isp1760_of_driver);if (ret) {
@@ -325,6 +420,9 @@ static void __exit isp1760_exit(void)#ifdef CONFIG_USB_ISP1760_PCI pci_unregister_driver (&isp1761_pci_driver);#endif +#ifdef USB_ISP1760_PDEV +platform_driver_unregister (&isp1760_pdev_driver); +#endif deinit_kmem_cache (); }module_exit (isp1760_exit);
Signature · 1 line
1.5.6.3
Quoted 21 lines · * Bryan Wu | 2008-11-06 17:25:49 [+0800]:
From: Michael Hennerich <[email protected]>
Signed-off-by Michael Hennerich <[email protected]>Signed-off-by Bryan Wu <[email protected]>---
drivers/usb/host/isp1760-if.c| 98 +++++++++++++++++++++++++++++++++++++++++1 files changed, 98 insertions(+), 0 deletions(-)drivers/usb/host/isp1760-if.c +1
-0@@ -3,6 +3,7 @@* Currently there is support for * - OpenFirmware * - PCI +* - PDEV (generic platform device centralized driver model) * * (c) 2007 Sebastian Siewior <[email protected]> *
drivers/usb/host/isp1760-if.c line 28
Quoted 5 lines
drivers/usb/host/isp1760-if.c +2
-0@@ -23,6 +24,12 @@#include <linux/pci.h> #endif +#if !defined(CONFIG_USB_ISP1760_OF ) && !defined(CONFIG_USB_ISP1760_PCI ) +#define USB_ISP1760_PDEV
Usually I would prefer to make it conditional on CONFIGU_USB_ISP1760_PDEV. But since
http://marc.info/?l=linux-usb&m=122563596420156&w=2 I would prefer to have unconditional.
Any reason why you only enable it PDEV if you have neiher PCI nor OF?
drivers/usb/host/isp1760-if.c line 30
Quoted 2 lines
drivers/usb/host/isp1760-if.c +2
-0+#include <linux/platform_device.h> +#include <linux/usb/isp1760.h>
You can't include files which are not mainline
drivers/usb/host/isp1760-if.c static struct pci_driver isp1761_pci_driver = { · line 299
Quoted 13 lines
drivers/usb/host/isp1760-if.c +6
-0+#endif +#ifdef CONFIG_USB_ISP1760_OF static int of_isp1760_probe (struct of_device *dev,const struct of_device_id *match)@@ -286,12 +293,100 @@ static struct pci_driver isp1761_pci_driver = { };#endif +#ifdef USB_ISP1760_PDEV +static int __devinit +isp1760_pdev_probe (struct platform_device *pdev) +{
Please make it
static int __devinit isp1760_pdev_probe(struct platform_device *pdev)
Quoted 33 lines
drivers/usb/host/isp1760-if.c +33
-0+struct usb_hcd *hcd; +struct resource *addr; +int irq; +unsigned int devflags =0 ; +struct isp1760_platform_data *priv = pdev->dev.platform_data; + +/* basic sanity checks first. board-specific init logic should +* have initialized these two resources and probably board +* specific platform_data. we don't probe for IRQs, and do only +* minimal sanity checking. +*/ + +if (usb_disabled ()) +return -ENODEV ; + +if (pdev->num_resources <2 ) +return -ENODEV ; + + addr =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + irq =platform_get_irq (pdev,0 ); + +if (!addr || irq <0 ) +return -ENODEV ; + +if (priv) { +if (priv->is_isp1761) + devflags |=ISP1760_FLAG_ISP1761 ; +if (priv->port1_disable) + devflags |=ISP1760_FLAG_PORT1_DIS ; +if (priv->bus_width_16) + devflags |=ISP1760_FLAG_BUS_WIDTH_16 ; +if (priv->port1_otg) + devflags |=ISP1760_FLAG_OTG_EN ;
drivers/usb/host/isp1760-if.c static struct pci_driver isp1761_pci_driver = { · line 342
Quoted 10 lines
drivers/usb/host/isp1760-if.c +10
-0+if (priv->analog_oc) + devflags |=ISP1760_FLAG_ANALOG_OC ; +if (priv->dack_polarity_high) + devflags |=ISP1760_FLAG_DACK_POL_HIGH ; +if (priv->dreq_polarity_high) + devflags |=ISP1760_FLAG_DREQ_POL_HIGH ; + } + + hcd = isp1760_register(addr->start, resource_size(addr), irq, + IRQF_DISABLED | IRQF_SHARED | IRQF_TRIGGER_FALLING,
This won't work. The chip itself is configured as level, active low. You have to make sure the chip is configured as edge as well.
Quoted 13 lines
drivers/usb/host/isp1760-if.c +13
-0+ &pdev->dev,dev_name (&pdev->dev), + devflags); + +if (IS_ERR (hcd)) +return PTR_ERR (hcd); + +return 0 ; +} + +static int __devexit +isp1760_pdev_remove (struct platform_device *pdev) +{ +struct usb_hcd *hcd =platform_get_drvdata (pdev);
drivers/usb/host/isp1760-if.c static struct pci_driver isp1761_pci_driver = { · line 365
Quoted 10 lines
drivers/usb/host/isp1760-if.c +10
-0+ +platform_set_drvdata (pdev,NULL ); + +usb_remove_hcd (hcd); +iounmap (hcd->regs); +usb_put_hcd (hcd); +return 0 ; +} + +/* this driver is exported so sl811_cs can depend on it */
What are you telling me here?
Quoted 39 lines
drivers/usb/host/isp1760-if.c +21
-0+struct platform_driver isp1760_pdev_driver = { + .probe = isp1760_pdev_probe, + .remove =__devexit_p (isp1760_pdev_remove), + .driver = { + .name ="isp1760-hcd" , + .owner =THIS_MODULE , + }, +}; +#endif /* USB_ISP1760_PDEV */ +static int __init isp1760_init (void ) {int ret = -ENODEV ;init_kmem_once (); +#ifdef USB_ISP1760_PDEV + ret =platform_driver_register (&isp1760_pdev_driver); +if (ret) { +deinit_kmem_cache (); +return ret; + } +#endif +#ifdef CONFIG_USB_ISP1760_OF ret =of_register_platform_driver (&isp1760_of_driver);if (ret) {@@ -325,6 +420,9 @@ static void __exit isp1760_exit(void)#ifdef CONFIG_USB_ISP1760_PCI pci_unregister_driver (&isp1761_pci_driver);#endif +#ifdef USB_ISP1760_PDEV +platform_driver_unregister (&isp1760_pdev_driver); +#endif deinit_kmem_cache (); }module_exit (isp1760_exit);Signature · 1 line
1.5.6.3
Sebastian
Sebastian,
Thanks for your feedback, see my comments below.
We will resubmit a patch soon.
Thanks and best regards,
Michael
Quoted 13 lines
-----Original Message-----
From: Sebastian Andrzej Siewior [mailto:[email protected]]
Sent: Thursday, November 06, 2008 6:57 PM
To: Bryan Wu
Cc: [email protected]; [email protected]; Michael Hennerich
Subject: Re: [PATCH] USB/ISP1760: Add support for the generic platfrom device centralized driver modelQuoted 2 lines · * Bryan Wu | 2008-11-06 17:25:49 [+0800]:
From: Michael Hennerich <[email protected]>
Quoted 8 lines
Quoted 4 lines
Signed-off-by Michael Hennerich <[email protected]>Signed-off-by Bryan Wu <[email protected]>---
drivers/usb/host/isp1760-if.c| 98+++++++++++++++++++++++++++++++++++++++++
Review
Quoted 3 lines
1 files changed, 98 insertions(+), 0 deletions(-)(quoted diff) +0
-0
b/drivers/usb/host/isp1760-if.c
Quoted 15 lines
Quoted 15 lines
index af849f5..dc16698 100644
drivers/usb/host/isp1760-if.c +1
-0@@ -3,6 +3,7 @@* Currently there is support for * - OpenFirmware * - PCI +* - PDEV (generic platform device centralized driver model) * * (c) 2007 Sebastian Siewior <[email protected]> * @@ -23,6 +24,12 @@#include <linux/pci.h> #endif
Quoted 1 line
drivers/usb/host/isp1760-if.c line 27
Quoted 1 line
drivers/usb/host/isp1760-if.c +1
-0+#if !defined(CONFIG_USB_ISP1760_OF) &&
!defined(CONFIG_USB_ISP1760_PCI)
Quoted 6 lines
drivers/usb/host/isp1760-if.c line 28
Quoted 1 line
drivers/usb/host/isp1760-if.c +1
-0+#define USB_ISP1760_PDEV Usually I would prefer to make it conditional on
CONFIGU_USB_ISP1760_PDEV. But since
http://marc.info/?l=linux-usb&m=122563596420156&w=2 I would prefer to have unconditional.
Any reason why you only enable it PDEV if you have neiher PCI nor OF?
Originally I had this kconfig option, but later decided to toss it. Why would you use pdev if you have PCI or OF, was my argument... :-) I'll add it back.
Quoted 4 lines
drivers/usb/host/isp1760-if.c line 30
Quoted 3 lines
drivers/usb/host/isp1760-if.c +2
-0+#include <linux/platform_device.h> +#include <linux/usb/isp1760.h> You can't include files which are not mainline
My tree features this file.
It simply misses in this patch.
Quoted 10 lines
Quoted 10 lines
drivers/usb/host/isp1760-if.c +2
-0+#endif +#ifdef CONFIG_USB_ISP1760_OF static int of_isp1760_probe (struct of_device *dev,const struct of_device_id *match)@@ -286,12 +293,100 @@ static struct pci_driver isp1761_pci_driver = { };#endif
Quoted 6 lines
drivers/usb/host/isp1760-if.c static struct pci_driver isp1761_pci_driver = { · line 299
Quoted 4 lines
drivers/usb/host/isp1760-if.c +4
-0+#ifdef USB_ISP1760_PDEV +static int __devinit +isp1760_pdev_probe (struct platform_device *pdev) +{Please make it
static int__devinitisp1760_pdev_probe(struct platform_device *pdev)
I see - single line...
Quoted 37 lines
Quoted 37 lines
drivers/usb/host/isp1760-if.c +36
-0+struct usb_hcd *hcd; +struct resource *addr; +int irq; +unsigned int devflags =0 ; +struct isp1760_platform_data *priv = pdev->dev.platform_data; + +/* basic sanity checks first. board-specific init logic should +* have initialized these two resources and probably board +* specific platform_data. we don't probe for IRQs, and do only +* minimal sanity checking. +*/ + +if (usb_disabled ()) +return -ENODEV ; + +if (pdev->num_resources <2 ) +return -ENODEV ; + + addr =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + irq =platform_get_irq (pdev,0 ); + +if (!addr || irq <0 ) +return -ENODEV ; + +if (priv) { +if (priv->is_isp1761) + devflags |=ISP1760_FLAG_ISP1761 ; +if (priv->port1_disable) + devflags |=ISP1760_FLAG_PORT1_DIS ; +if (priv->bus_width_16) + devflags |=ISP1760_FLAG_BUS_WIDTH_16 ; +if (priv->port1_otg) + devflags |=ISP1760_FLAG_OTG_EN ; +if (priv->analog_oc) + devflags |=ISP1760_FLAG_ANALOG_OC ; +if (priv->dack_polarity_high)
Quoted 8 lines
drivers/usb/host/isp1760-if.c static struct pci_driver isp1761_pci_driver = { · line 342
Quoted 7 lines
drivers/usb/host/isp1760-if.c +7
-0+ devflags |=ISP1760_FLAG_DACK_POL_HIGH ; +if (priv->dreq_polarity_high) + devflags |=ISP1760_FLAG_DREQ_POL_HIGH ; + } + + hcd = isp1760_register(addr->start, resource_size(addr), irq, + IRQF_DISABLED | IRQF_SHARED | IRQF_TRIGGER_FALLING,This won't work. The chip itself is configured as level, active low.
You
Quoted 1 line
have to make sure the chip is configured as edge as well.
Well it somehow worked - I'll fix it.
Quoted 17 lines
Quoted 17 lines
drivers/usb/host/isp1760-if.c +16
-0+ &pdev->dev,dev_name (&pdev->dev), + devflags); + +if (IS_ERR (hcd)) +return PTR_ERR (hcd); + +return 0 ; +} + +static int __devexit +isp1760_pdev_remove (struct platform_device *pdev) +{ +struct usb_hcd *hcd =platform_get_drvdata (pdev); + +platform_set_drvdata (pdev,NULL ); +
Quoted 8 lines
drivers/usb/host/isp1760-if.c static struct pci_driver isp1761_pci_driver = { · line 365
Quoted 7 lines
drivers/usb/host/isp1760-if.c +7
-0+usb_remove_hcd (hcd); +iounmap (hcd->regs); +usb_put_hcd (hcd); +return 0 ; +} + +/* this driver is exported so sl811_cs can depend on it */ What are you telling me here?
Noting - I'll remove it
Quoted 42 lines
Quoted 39 lines
drivers/usb/host/isp1760-if.c +21
-1+struct platform_driver isp1760_pdev_driver = { + .probe = isp1760_pdev_probe, + .remove =__devexit_p (isp1760_pdev_remove), + .driver = { + .name ="isp1760-hcd" , + .owner =THIS_MODULE , + }, +}; +#endif /* USB_ISP1760_PDEV */ +static int __init isp1760_init (void ) {int ret = -ENODEV ;init_kmem_once (); +#ifdef USB_ISP1760_PDEV + ret =platform_driver_register (&isp1760_pdev_driver); +if (ret) { +deinit_kmem_cache (); +return ret; + } +#endif +#ifdef CONFIG_USB_ISP1760_OF ret =of_register_platform_driver (&isp1760_of_driver);if (ret) {@@ -325,6 +420,9 @@ static void __exit isp1760_exit(void)#ifdef CONFIG_USB_ISP1760_PCI pci_unregister_driver (&isp1761_pci_driver);#endif +#ifdef USB_ISP1760_PDEV +platform_driver_unregister (&isp1760_pdev_driver); +#endif deinit_kmem_cache (); }module_exit (isp1760_exit); --Quoted 1 line
1.5.6.3
Sebastian
Quoted 61 lines · Michael
Sebastian,
Thanks for your feedback, see my comments below.
We will resubmit a patch soon.Thanks and best regards,
MichaelQuoted 13 lines
-----Original Message-----
From: Sebastian Andrzej Siewior [mailto:[email protected]]
Sent: Thursday, November 06, 2008 6:57 PM
To: Bryan Wu
Cc: [email protected]; [email protected]; Michael Hennerich
Subject: Re: [PATCH] USB/ISP1760: Add support for the generic platfrom device centralized driver modelQuoted 2 lines · * Bryan Wu | 2008-11-06 17:25:49 [+0800]:
From: Michael Hennerich <[email protected]>
Quoted 8 lines
Quoted 4 lines
Signed-off-by Michael Hennerich <[email protected]>Signed-off-by Bryan Wu <[email protected]>---
drivers/usb/host/isp1760-if.c| 98+++++++++++++++++++++++++++++++++++++++++
Review
Quoted 3 lines
1 files changed, 98 insertions(+), 0 deletions(-)(quoted diff) +0
-0
b/drivers/usb/host/isp1760-if.cQuoted 15 lines
Quoted 15 lines
index af849f5..dc16698 100644
drivers/usb/host/isp1760-if.c +1
-0@@ -3,6 +3,7 @@* Currently there is support for * - OpenFirmware * - PCI +* - PDEV (generic platform device centralized driver model) * * (c) 2007 Sebastian Siewior <[email protected]> * @@ -23,6 +24,12 @@#include <linux/pci.h> #endif Quoted 1 line
drivers/usb/host/isp1760-if.c line 27
Quoted 1 line
drivers/usb/host/isp1760-if.c +1
-0+#if !defined(CONFIG_USB_ISP1760_OF) &&!defined(CONFIG_USB_ISP1760_PCI)
Quoted 6 lines
drivers/usb/host/isp1760-if.c line 28
Quoted 1 line
drivers/usb/host/isp1760-if.c +1
-0+#define USB_ISP1760_PDEV Usually I would prefer to make it conditional on
CONFIGU_USB_ISP1760_PDEV. But since
http://marc.info/?l=linux-usb&m=122563596420156&w=2 I would prefer to have unconditional.
Any reason why you only enable it PDEV if you have neiher PCI nor OF?Originally I had this kconfig option, but later decided to toss it. Why would you use pdev if you have PCI or OF, was my argument... :-) I'll add it back.
Quoted 1 line
Quoted 7 lines
Quoted 3 lines
drivers/usb/host/isp1760-if.c line 30
Quoted 2 lines
drivers/usb/host/isp1760-if.c +2
-0+#include <linux/platform_device.h> +#include <linux/usb/isp1760.h> You can't include files which are not mainline
My tree features this file.
It simply misses in this patch.
My fault, I forgot to git-add this new isp1760.h before sending out this patch.
Michael, after you fix up this patch according to Sebastian, I will send out the update version.
-Bryan