2015-04-09 08:34:05

by Ivan T. Ivanov

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

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 <[email protected]>
---
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


2015-04-09 14:49:07

by Alan Stern

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

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 <[email protected]>
> ---
> 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
>
>
>

2015-04-15 15:58:12

by Vivek Gautam

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

On Thu, Apr 9, 2015 at 8:19 PM, Alan Stern <[email protected]> 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 <[email protected]>
>> ---
>> 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 [email protected]
> 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

2015-04-16 07:42:34

by Ivan T. Ivanov

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


Hi,

On Wed, 2015-04-15 at 21:28 +0530, Vivek Gautam wrote:
> On Thu, Apr 9, 2015 at 8:19 PM, Alan Stern <[email protected]> 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

2015-04-20 08:18:41

by Ivan T. Ivanov

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


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 <[email protected]> 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

2015-04-20 14:14:57

by Alan Stern

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

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

2015-04-20 15:55:12

by Ivan T. Ivanov

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


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

2015-04-20 17:36:48

by Alan Stern

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

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