Re: [PATCH v2] usb: chipidea: reuse the platform_data to store the ci info

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 17, 2015 at 07:48:46AM +0000, Barry Song wrote:
> From: Rong Wang <Rong.Wang@xxxxxxx>
> 
> Chipidea puts ci information to drvdata, but this overwrites the drvdata
> placed by EHCI core. EHCI core thinks drvdata is ehci_hcd. We can find this
> from codes like ehci-sysfs.c:
> 
> static ssize_t show_companion(struct device *dev,
> 			      struct device_attribute *attr,
> 			      char *buf) {
> 	struct ehci_hcd *ehci;
> 
> 	ehci = hcd_to_ehci(bus_to_hcd(dev_get_drvdata(dev)));
> 	...
> }
> 
> So overwritting drvdata from chipidea driver actually breaks a part of
> functionalities of EHCI core.
> 
> Since the platform_data would not be accessed after the device is added to
> system after the probe process, so it's safe to move to platform_data here.
> This fix is not elegant but currently it is the quickest fix.
> 
> Signed-off-by: Rong Wang <Rong.Wang@xxxxxxx>
> Signed-off-by: Barry Song <Baohua.Song@xxxxxxx>
> ---
>  -v2: rebase againest 4.3 usb-next
> 
>  drivers/usb/chipidea/core.c    | 12 ++++++------
>  drivers/usb/chipidea/host.c    |  5 ++---
>  drivers/usb/chipidea/otg_fsm.c | 14 +++++++-------
>  3 files changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 3feebf7..63c8cd6 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -915,7 +915,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	platform_set_drvdata(pdev, ci);
> +	dev->platform_data = ci;

It causes oops, the override hcd APIs (like ehci_ci_reset) needs to know
ci, and it is called in host_start which is called before the above line.
Move this change before can fix the problem.

Peter

>  	ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED,
>  			ci->platdata->name, ci);
>  	if (ret)
> @@ -948,7 +948,7 @@ deinit_phy:
>  
>  static int ci_hdrc_remove(struct platform_device *pdev)
>  {
> -	struct ci_hdrc *ci = platform_get_drvdata(pdev);
> +	struct ci_hdrc *ci = dev_get_platdata(&pdev->dev);
>  
>  	if (ci->supports_runtime_pm) {
>  		pm_runtime_get_sync(&pdev->dev);
> @@ -1003,7 +1003,7 @@ static void ci_controller_suspend(struct ci_hdrc *ci)
>  
>  static int ci_controller_resume(struct device *dev)
>  {
> -	struct ci_hdrc *ci = dev_get_drvdata(dev);
> +	struct ci_hdrc *ci = dev_get_platdata(dev);
>  
>  	dev_dbg(dev, "at %s\n", __func__);
>  
> @@ -1035,7 +1035,7 @@ static int ci_controller_resume(struct device *dev)
>  #ifdef CONFIG_PM_SLEEP
>  static int ci_suspend(struct device *dev)
>  {
> -	struct ci_hdrc *ci = dev_get_drvdata(dev);
> +	struct ci_hdrc *ci = dev_get_platdata(dev);
>  
>  	if (ci->wq)
>  		flush_workqueue(ci->wq);
> @@ -1068,7 +1068,7 @@ static int ci_suspend(struct device *dev)
>  
>  static int ci_resume(struct device *dev)
>  {
> -	struct ci_hdrc *ci = dev_get_drvdata(dev);
> +	struct ci_hdrc *ci = dev_get_platdata(dev);
>  	int ret;
>  
>  	if (device_may_wakeup(dev))
> @@ -1090,7 +1090,7 @@ static int ci_resume(struct device *dev)
>  
>  static int ci_runtime_suspend(struct device *dev)
>  {
> -	struct ci_hdrc *ci = dev_get_drvdata(dev);
> +	struct ci_hdrc *ci = dev_get_platdata(dev);
>  
>  	dev_dbg(dev, "at %s\n", __func__);
>  
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index 3d24304..8bd54be 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -44,7 +44,7 @@ static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
>  	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
>  	struct ehci_ci_priv *priv = (struct ehci_ci_priv *)ehci->priv;
>  	struct device *dev = hcd->self.controller;
> -	struct ci_hdrc *ci = dev_get_drvdata(dev);
> +	struct ci_hdrc *ci = dev_get_platdata(dev);
>  	int ret = 0;
>  	int port = HCS_N_PORTS(ehci->hcs_params);
>  
> @@ -80,7 +80,7 @@ static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
>  static int ehci_ci_reset(struct usb_hcd *hcd)
>  {
>  	struct device *dev = hcd->self.controller;
> -	struct ci_hdrc *ci = dev_get_drvdata(dev);
> +	struct ci_hdrc *ci = dev_get_platdata(dev);
>  	int ret;
>  
>  	ret = ehci_setup(hcd);
> @@ -117,7 +117,6 @@ static int host_start(struct ci_hdrc *ci)
>  	if (!hcd)
>  		return -ENOMEM;
>  
> -	dev_set_drvdata(ci->dev, ci);
>  	hcd->rsrc_start = ci->hw_bank.phys;
>  	hcd->rsrc_len = ci->hw_bank.size;
>  	hcd->regs = ci->hw_bank.abs;
> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> index 00ab59d..8dbf1d4 100644
> --- a/drivers/usb/chipidea/otg_fsm.c
> +++ b/drivers/usb/chipidea/otg_fsm.c
> @@ -36,7 +36,7 @@ get_a_bus_req(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>  	char		*next;
>  	unsigned	size, t;
> -	struct ci_hdrc	*ci = dev_get_drvdata(dev);
> +	struct ci_hdrc	*ci = dev_get_platdata(dev);
>  
>  	next = buf;
>  	size = PAGE_SIZE;
> @@ -51,7 +51,7 @@ static ssize_t
>  set_a_bus_req(struct device *dev, struct device_attribute *attr,
>  					const char *buf, size_t count)
>  {
> -	struct ci_hdrc *ci = dev_get_drvdata(dev);
> +	struct ci_hdrc *ci = dev_get_platdata(dev);
>  
>  	if (count > 2)
>  		return -1;
> @@ -80,7 +80,7 @@ get_a_bus_drop(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>  	char		*next;
>  	unsigned	size, t;
> -	struct ci_hdrc	*ci = dev_get_drvdata(dev);
> +	struct ci_hdrc	*ci = dev_get_platdata(dev);
>  
>  	next = buf;
>  	size = PAGE_SIZE;
> @@ -95,7 +95,7 @@ static ssize_t
>  set_a_bus_drop(struct device *dev, struct device_attribute *attr,
>  					const char *buf, size_t count)
>  {
> -	struct ci_hdrc	*ci = dev_get_drvdata(dev);
> +	struct ci_hdrc	*ci = dev_get_platdata(dev);
>  
>  	if (count > 2)
>  		return -1;
> @@ -121,7 +121,7 @@ get_b_bus_req(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>  	char		*next;
>  	unsigned	size, t;
> -	struct ci_hdrc	*ci = dev_get_drvdata(dev);
> +	struct ci_hdrc	*ci = dev_get_platdata(dev);
>  
>  	next = buf;
>  	size = PAGE_SIZE;
> @@ -136,7 +136,7 @@ static ssize_t
>  set_b_bus_req(struct device *dev, struct device_attribute *attr,
>  					const char *buf, size_t count)
>  {
> -	struct ci_hdrc	*ci = dev_get_drvdata(dev);
> +	struct ci_hdrc	*ci = dev_get_platdata(dev);
>  
>  	if (count > 2)
>  		return -1;
> @@ -158,7 +158,7 @@ static ssize_t
>  set_a_clr_err(struct device *dev, struct device_attribute *attr,
>  					const char *buf, size_t count)
>  {
> -	struct ci_hdrc	*ci = dev_get_drvdata(dev);
> +	struct ci_hdrc	*ci = dev_get_platdata(dev);
>  
>  	if (count > 2)
>  		return -1;
> -- 
> 1.9.1
> 

-- 

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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux