[PATCH] usb: ehci-msm: Don't ioremap configuration space exclusively

[PATCH] usb: ehci-msm: Don't ioremap configuration space exclusively

[Ivan T. Ivanov]

This allow same IO space to be shared between HCD and Device
controller driver. Which can be loaded simultaneously and
started/stopped on demand by USB OTG PHY driver.

Signed-off-by: Ivan T. Ivanov <ivan.ivanov@linaro.org>
---
 drivers/usb/host/ehci-msm.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
index 9db74ca..f059e15 100644
--- a/drivers/usb/host/ehci-msm.c
+++ b/drivers/usb/host/ehci-msm.c
@@ -88,13 +88,17 @@ static int ehci_msm_probe(struct platform_device *pdev)
 	}

 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (!res)
+		return -ENODEV;
+
+	hcd->rsrc_start = res->start;
+	hcd->rsrc_len = resource_size(res);
+
+	hcd->regs = devm_ioremap(&pdev->dev, hcd->rsrc_start, hcd->rsrc_len);
 	if (IS_ERR(hcd->regs)) {
 		ret = PTR_ERR(hcd->regs);
 		goto put_hcd;
 	}
-	hcd->rsrc_start = res->start;
-	hcd->rsrc_len = resource_size(res);

 	/*
 	 * OTG driver takes care of PHY initialization, clock management,
--
1.9.1

Re: [PATCH] usb: ehci-msm: Don't ioremap configuration space exclusively

[Alan Stern]

On Thu, 9 Apr 2015, Ivan T. Ivanov wrote:

> This allow same IO space to be shared between HCD and Device
> controller driver. Which can be loaded simultaneously and
> started/stopped on demand by USB OTG PHY driver.

You really should CC the person who wrote the code you are changing.  
This is almost exactly the same as reverting commit 70843f623b58 (usb: 
host: ehci-msm: Use devm_ioremap_resource instead of devm_ioremap).

Vivek, what do you think?

Alan Stern

> Signed-off-by: Ivan T. Ivanov <ivan.ivanov@linaro.org>
> ---
>  drivers/usb/host/ehci-msm.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
> index 9db74ca..f059e15 100644
> --- a/drivers/usb/host/ehci-msm.c
> +++ b/drivers/usb/host/ehci-msm.c
> @@ -88,13 +88,17 @@ static int ehci_msm_probe(struct platform_device *pdev)
>  	}
> 
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	hcd->regs = devm_ioremap_resource(&pdev->dev, res);
> +	if (!res)
> +		return -ENODEV;
> +
> +	hcd->rsrc_start = res->start;
> +	hcd->rsrc_len = resource_size(res);
> +
> +	hcd->regs = devm_ioremap(&pdev->dev, hcd->rsrc_start, hcd->rsrc_len);
>  	if (IS_ERR(hcd->regs)) {
>  		ret = PTR_ERR(hcd->regs);
>  		goto put_hcd;
>  	}
> -	hcd->rsrc_start = res->start;
> -	hcd->rsrc_len = resource_size(res);
> 
>  	/*
>  	 * OTG driver takes care of PHY initialization, clock management,
> --
> 1.9.1
> 
> 
> 

Re: [PATCH] usb: ehci-msm: Don't ioremap configuration space exclusively

[Vivek Gautam]

On Thu, Apr 9, 2015 at 8:19 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 9 Apr 2015, Ivan T. Ivanov wrote:
>
>> This allow same IO space to be shared between HCD and Device
>> controller driver. Which can be loaded simultaneously and
>> started/stopped on demand by USB OTG PHY driver.

Are you sure ?
Will ehci controller registers overlap with the Device controller's register
region ?

>
> You really should CC the person who wrote the code you are changing.
> This is almost exactly the same as reverting commit 70843f623b58 (usb:
> host: ehci-msm: Use devm_ioremap_resource instead of devm_ioremap).
>
> Vivek, what do you think?

Yea, the idea was to prevent any unintentional overlapping of
ioremapped regions by two device drivers.

I still believe that the register region used by ehci-msm may
not be overlapping with the device-controller's register memory region.

>
> Alan Stern
>
>> Signed-off-by: Ivan T. Ivanov <ivan.ivanov@linaro.org>
>> ---
>>  drivers/usb/host/ehci-msm.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
>> index 9db74ca..f059e15 100644
>> --- a/drivers/usb/host/ehci-msm.c
>> +++ b/drivers/usb/host/ehci-msm.c
>> @@ -88,13 +88,17 @@ static int ehci_msm_probe(struct platform_device *pdev)
>>       }
>>
>>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -     hcd->regs = devm_ioremap_resource(&pdev->dev, res);
>> +     if (!res)
>> +             return -ENODEV;
>> +
>> +     hcd->rsrc_start = res->start;
>> +     hcd->rsrc_len = resource_size(res);
>> +
>> +     hcd->regs = devm_ioremap(&pdev->dev, hcd->rsrc_start, hcd->rsrc_len);
>>       if (IS_ERR(hcd->regs)) {
>>               ret = PTR_ERR(hcd->regs);
>>               goto put_hcd;
>>       }
>> -     hcd->rsrc_start = res->start;
>> -     hcd->rsrc_len = resource_size(res);
>>
>>       /*
>>        * OTG driver takes care of PHY initialization, clock management,
>> --
>> 1.9.1
>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

Re: [PATCH] usb: ehci-msm: Don't ioremap configuration space exclusively

[Ivan T. Ivanov]

Hi,

On Wed, 2015-04-15 at 21:28 +0530, Vivek Gautam wrote:
> On Thu, Apr 9, 2015 at 8:19 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> > On Thu, 9 Apr 2015, Ivan T. Ivanov wrote:
> > 
> > > This allow same IO space to be shared between HCD and Device
> > > controller driver. Which can be loaded simultaneously and
> > > started/stopped on demand by USB OTG PHY driver.
> 
> Are you sure ?

No.

> Will ehci controller registers overlap with the Device controller's register
> region ?
> 

Well, not exactly DC vs HC region, but at least USB_AHBBURST, USB_AHBMODE,
USB_USBMODE are used by both OTG phy-msm-usb and this ehci-msm driver. 
And this is broken right now. 

Ivan

Re: [PATCH] usb: ehci-msm: Don't ioremap configuration space exclusively

[Ivan T. Ivanov]

On Thu, 2015-04-16 at 10:42 +0300, Ivan T. Ivanov wrote:
> Hi,
> 
> On Wed, 2015-04-15 at 21:28 +0530, Vivek Gautam wrote:
> > On Thu, Apr 9, 2015 at 8:19 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> > > On Thu, 9 Apr 2015, Ivan T. Ivanov wrote:
> > > 
> > > > This allow same IO space to be shared between HCD and Device
> > > > controller driver. Which can be loaded simultaneously and
> > > > started/stopped on demand by USB OTG PHY driver.
> > 
> > Are you sure ?
> 
> No.
> 
> > Will ehci controller registers overlap with the Device controller's register
> > region ?
> > 
> 
> Well, not exactly DC vs HC region, but at least USB_AHBBURST, USB_AHBMODE,
> USB_USBMODE are used by both OTG phy-msm-usb and this ehci-msm driver.
> And this is broken right now.

Hi Alan, 

Perhaps I have to resend this patch with updated commit
message? Are they any other obstacles? 

Regards,
Ivan

Re: [PATCH] usb: ehci-msm: Don't ioremap configuration space exclusively

[Alan Stern]

On Mon, 20 Apr 2015, Ivan T. Ivanov wrote:

> Hi Alan, 
> 
> Perhaps I have to resend this patch with updated commit
> message? Are they any other obstacles? 

Instead of submitting this new patch, would it be okay to revert commit 
70843f623b58?  That would be simpler.

Also, I'd like to get an Acked-by from Vivek before accepting this.

Alan Stern

Re: [PATCH] usb: ehci-msm: Don't ioremap configuration space exclusively

[Ivan T. Ivanov]

On Mon, 2015-04-20 at 10:14 -0400, Alan Stern wrote:
> On Mon, 20 Apr 2015, Ivan T. Ivanov wrote:
> 
> > Hi Alan,
> >
> > Perhaps I have to resend this patch with updated commit
> > message? Are they any other obstacles?
> 
> Instead of submitting this new patch, would it be okay to revert commit
> 70843f623b58?  That would be simpler.

Sure, wherever is working better for you.

> 
> Also, I'd like to get an Acked-by from Vivek before accepting this.
> 
> Alan Stern
> 

Do you expect something from my side?

Thanks,
Ivan

Re: [PATCH] usb: ehci-msm: Don't ioremap configuration space exclusively

[Alan Stern]

On Mon, 20 Apr 2015, Ivan T. Ivanov wrote:

> 
> On Mon, 2015-04-20 at 10:14 -0400, Alan Stern wrote:
> > On Mon, 20 Apr 2015, Ivan T. Ivanov wrote:
> > 
> > > Hi Alan,
> > >
> > > Perhaps I have to resend this patch with updated commit
> > > message? Are they any other obstacles?
> > 
> > Instead of submitting this new patch, would it be okay to revert commit
> > 70843f623b58?  That would be simpler.
> 
> Sure, wherever is working better for you.
> 
> > 
> > Also, I'd like to get an Acked-by from Vivek before accepting this.
> > 
> > Alan Stern
> > 
> 
> Do you expect something from my side?

Just submit a new version of the patch (reverting that commit) and CC:
Vivek on the submission.

Alan Stern