Re: [RFC][PATCH 3/4 v3] usb: host: ehci-platform: add platform specific power callback

2012-08-06 Thread Alan Stern
On Sun, 5 Aug 2012 kuninori.morimoto...@renesas.com wrote:

> This patch enables to call platform specific power callback function.
> 
> Signed-off-by: Kuninori Morimoto 
> ---
> v2 -> v3
> 
>  - add power multi functions
>  - call it by macro
> 
>  drivers/usb/host/ehci-platform.c |   40 ++---
>  include/linux/usb/ehci_pdriver.h |5 
>  2 files changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-platform.c 
> b/drivers/usb/host/ehci-platform.c
> index db27dfe..bcdb9e4 100644
> --- a/drivers/usb/host/ehci-platform.c
> +++ b/drivers/usb/host/ehci-platform.c
> @@ -22,6 +22,8 @@
>  #include 
>  
>  #define ehci_pdata_get(pdata, x) ((pdata) ? (pdata)->x : 0)
> +#define ehci_pdata_call(pdata, x, args...) \
> + ((!(pdata) || !((pdata)->x)) ? 0 : (pdata)->x(args))

I prefer not to have this.  Just do the test before making each call.

>  static int ehci_platform_resume(struct device *dev)
>  {
>   struct usb_hcd *hcd = dev_get_drvdata(dev);
> + struct usb_ehci_pdata *pdata = dev->platform_data;
> + struct platform_device *pdev =
> + container_of(dev, struct platform_device, dev);
> +
> + /* power resume if platform supported */
> + ehci_pdata_call(pdata, power_resume, pdev);

You didn't check the return value.

>  
>   ehci_resume(hcd, false);
>   return 0;

> --- a/include/linux/usb/ehci_pdriver.h
> +++ b/include/linux/usb/ehci_pdriver.h
> @@ -41,6 +41,11 @@ struct usb_ehci_pdata {
>   unsignedbig_endian_mmio:1;
>   unsignedport_power_on:1;
>   unsignedport_power_off:1;
> +
> + int (*power_on)(struct platform_device *pdev);
> + void (*power_off)(struct platform_device *pdev);
> + void (*power_suspend)(struct platform_device *pdev);
> + void (*power_resume)(struct platform_device *pdev);

Shouldn't power_resume and power_on be the same function always?  They 
do the same thing, even though the initial states of the power supplies 
might be different.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC][PATCH 3/4 v3] usb: host: ehci-platform: add platform specific power callback

2012-08-05 Thread kuninori . morimoto . gx
This patch enables to call platform specific power callback function.

Signed-off-by: Kuninori Morimoto 
---
v2 -> v3

 - add power multi functions
 - call it by macro

 drivers/usb/host/ehci-platform.c |   40 ++---
 include/linux/usb/ehci_pdriver.h |5 
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index db27dfe..bcdb9e4 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -22,6 +22,8 @@
 #include 
 
 #define ehci_pdata_get(pdata, x) ((pdata) ? (pdata)->x : 0)
+#define ehci_pdata_call(pdata, x, args...) \
+   ((!(pdata) || !((pdata)->x)) ? 0 : (pdata)->x(args))
 
 static int ehci_platform_reset(struct usb_hcd *hcd)
 {
@@ -84,10 +86,11 @@ static int __devinit ehci_platform_probe(struct 
platform_device *dev)
 {
struct usb_hcd *hcd;
struct resource *res_mem;
+   struct usb_ehci_pdata *pdata = dev->dev.platform_data;
int irq;
int err = -ENOMEM;
 
-   WARN_ON(!dev->dev.platform_data);
+   WARN_ON(!pdata);
 
if (usb_disabled())
return -ENODEV;
@@ -103,10 +106,17 @@ static int __devinit ehci_platform_probe(struct 
platform_device *dev)
return -ENXIO;
}
 
+   /* power ON if platform supported */
+   err = ehci_pdata_call(pdata, power_on, dev);
+   if (err < 0)
+   return err;
+
hcd = usb_create_hcd(&ehci_platform_hc_driver, &dev->dev,
 dev_name(&dev->dev));
-   if (!hcd)
-   return -ENOMEM;
+   if (!hcd) {
+   err = -ENOMEM;
+   goto err_power;
+   }
 
hcd->rsrc_start = res_mem->start;
hcd->rsrc_len = resource_size(res_mem);
@@ -134,12 +144,16 @@ err_release_region:
release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 err_put_hcd:
usb_put_hcd(hcd);
+err_power:
+   ehci_pdata_call(pdata, power_off, dev);
+
return err;
 }
 
 static int __devexit ehci_platform_remove(struct platform_device *dev)
 {
struct usb_hcd *hcd = platform_get_drvdata(dev);
+   struct usb_ehci_pdata *pdata = dev->dev.platform_data;
 
usb_remove_hcd(hcd);
iounmap(hcd->regs);
@@ -147,6 +161,9 @@ static int __devexit ehci_platform_remove(struct 
platform_device *dev)
usb_put_hcd(hcd);
platform_set_drvdata(dev, NULL);
 
+   /* power OFF if platform supported */
+   ehci_pdata_call(pdata, power_off, dev);
+
return 0;
 }
 
@@ -155,14 +172,29 @@ static int __devexit ehci_platform_remove(struct 
platform_device *dev)
 static int ehci_platform_suspend(struct device *dev)
 {
struct usb_hcd *hcd = dev_get_drvdata(dev);
+   struct usb_ehci_pdata *pdata = dev->platform_data;
+   struct platform_device *pdev =
+   container_of(dev, struct platform_device, dev);
bool do_wakeup = device_may_wakeup(dev);
+   int ret;
 
-   return ehci_suspend(hcd, do_wakeup);
+   ret = ehci_suspend(hcd, do_wakeup);
+
+   /* power suspend if platform supported */
+   ehci_pdata_call(pdata, power_suspend, pdev);
+
+   return ret;
 }
 
 static int ehci_platform_resume(struct device *dev)
 {
struct usb_hcd *hcd = dev_get_drvdata(dev);
+   struct usb_ehci_pdata *pdata = dev->platform_data;
+   struct platform_device *pdev =
+   container_of(dev, struct platform_device, dev);
+
+   /* power resume if platform supported */
+   ehci_pdata_call(pdata, power_resume, pdev);
 
ehci_resume(hcd, false);
return 0;
diff --git a/include/linux/usb/ehci_pdriver.h b/include/linux/usb/ehci_pdriver.h
index 1894f42..23a2f38 100644
--- a/include/linux/usb/ehci_pdriver.h
+++ b/include/linux/usb/ehci_pdriver.h
@@ -41,6 +41,11 @@ struct usb_ehci_pdata {
unsignedbig_endian_mmio:1;
unsignedport_power_on:1;
unsignedport_power_off:1;
+
+   int (*power_on)(struct platform_device *pdev);
+   void (*power_off)(struct platform_device *pdev);
+   void (*power_suspend)(struct platform_device *pdev);
+   void (*power_resume)(struct platform_device *pdev);
 };
 
 #endif /* __USB_CORE_EHCI_PDRIVER_H */
-- 
1.7.5.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html