Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
On Wednesday 19 August 2015 14:28:33 Duc Dang wrote:
>
> Hi Arnd,
>
> So the check will look like this, please let me know what do you think:
> if (!pdev->dev.dma_mask) {
> WARN_ON(1);
> /* Initialize dma_mask if the broken platform code has
> not done so */
> pdev->dev.dma_mask = >dev.coherent_dma_mask;
> }
The condition can be written as
if (WARN_ON(!pdev->dev.dma_mask))
and I'd use dma_coerce_mask_and_coherent() instead of manually setting the
pointer, as an annotation for the fact that we are knowingly violating the
API here.
Those two points are just cosmetic though, aside from them, your code
above is what I had in mind.
Thanks,
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
On Wednesday 19 August 2015 14:28:33 Duc Dang wrote:
Hi Arnd,
So the check will look like this, please let me know what do you think:
if (!pdev-dev.dma_mask) {
WARN_ON(1);
/* Initialize dma_mask if the broken platform code has
not done so */
pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
}
The condition can be written as
if (WARN_ON(!pdev-dev.dma_mask))
and I'd use dma_coerce_mask_and_coherent() instead of manually setting the
pointer, as an annotation for the fact that we are knowingly violating the
API here.
Those two points are just cosmetic though, aside from them, your code
above is what I had in mind.
Thanks,
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
On Sat, Aug 15, 2015 at 1:05 PM, Arnd Bergmann wrote:
>
> On Saturday 08 August 2015 13:31:02 Duc Dang wrote:
> > >
> > > If we know that pdev->dev.dma_mask will always be initialised at this
> > > point, then the above change is fine. If not, it's introducing a
> > > regression - dma_set_mask_and_coherent() will fail if pdev->dev.dma_mask
> > > is NULL (depending on the architectures implementation of dma_set_mask()).
> > >
> > > Prefixing the above change with the two lines I mention above would
> > > ensure equivalent behaviour. Even if we do want to get rid of this,
> > > I'd advise to do it as a separate patch after this change, which can
> > > be independently reverted if there's problems with its removal.
> > >
> > Hi Russell,
> >
> > I will add the 2 lines you mentioned back to next version of the
> > patch. It is safer to do it that way as I do not see
> > pdev->dev.dma_mask gets initialized before the call
> > dma_set_mask_and_coherent inside this xhci_plat.c file.
>
> It would be good to add a WARN_ON() to the case where dma_mask
> is a NULL pointer at the least. That way, we will at least
> find out if there are some broken platforms that do not correctly
> initialize the mask pointer.
Hi Arnd,
So the check will look like this, please let me know what do you think:
if (!pdev->dev.dma_mask) {
WARN_ON(1);
/* Initialize dma_mask if the broken platform code has
not done so */
pdev->dev.dma_mask = >dev.coherent_dma_mask;
}
>
> Arnd
--
Regards,
Duc Dang.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
On Sat, Aug 15, 2015 at 1:05 PM, Arnd Bergmann a...@arndb.de wrote:
On Saturday 08 August 2015 13:31:02 Duc Dang wrote:
If we know that pdev-dev.dma_mask will always be initialised at this
point, then the above change is fine. If not, it's introducing a
regression - dma_set_mask_and_coherent() will fail if pdev-dev.dma_mask
is NULL (depending on the architectures implementation of dma_set_mask()).
Prefixing the above change with the two lines I mention above would
ensure equivalent behaviour. Even if we do want to get rid of this,
I'd advise to do it as a separate patch after this change, which can
be independently reverted if there's problems with its removal.
Hi Russell,
I will add the 2 lines you mentioned back to next version of the
patch. It is safer to do it that way as I do not see
pdev-dev.dma_mask gets initialized before the call
dma_set_mask_and_coherent inside this xhci_plat.c file.
It would be good to add a WARN_ON() to the case where dma_mask
is a NULL pointer at the least. That way, we will at least
find out if there are some broken platforms that do not correctly
initialize the mask pointer.
Hi Arnd,
So the check will look like this, please let me know what do you think:
if (!pdev-dev.dma_mask) {
WARN_ON(1);
/* Initialize dma_mask if the broken platform code has
not done so */
pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
}
Arnd
--
Regards,
Duc Dang.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
On Saturday 08 August 2015 13:31:02 Duc Dang wrote: > > > > If we know that pdev->dev.dma_mask will always be initialised at this > > point, then the above change is fine. If not, it's introducing a > > regression - dma_set_mask_and_coherent() will fail if pdev->dev.dma_mask > > is NULL (depending on the architectures implementation of dma_set_mask()). > > > > Prefixing the above change with the two lines I mention above would > > ensure equivalent behaviour. Even if we do want to get rid of this, > > I'd advise to do it as a separate patch after this change, which can > > be independently reverted if there's problems with its removal. > > > Hi Russell, > > I will add the 2 lines you mentioned back to next version of the > patch. It is safer to do it that way as I do not see > pdev->dev.dma_mask gets initialized before the call > dma_set_mask_and_coherent inside this xhci_plat.c file. It would be good to add a WARN_ON() to the case where dma_mask is a NULL pointer at the least. That way, we will at least find out if there are some broken platforms that do not correctly initialize the mask pointer. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
On Saturday 08 August 2015 13:31:02 Duc Dang wrote: If we know that pdev-dev.dma_mask will always be initialised at this point, then the above change is fine. If not, it's introducing a regression - dma_set_mask_and_coherent() will fail if pdev-dev.dma_mask is NULL (depending on the architectures implementation of dma_set_mask()). Prefixing the above change with the two lines I mention above would ensure equivalent behaviour. Even if we do want to get rid of this, I'd advise to do it as a separate patch after this change, which can be independently reverted if there's problems with its removal. Hi Russell, I will add the 2 lines you mentioned back to next version of the patch. It is safer to do it that way as I do not see pdev-dev.dma_mask gets initialized before the call dma_set_mask_and_coherent inside this xhci_plat.c file. It would be good to add a WARN_ON() to the case where dma_mask is a NULL pointer at the least. That way, we will at least find out if there are some broken platforms that do not correctly initialize the mask pointer. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
On Sat, Aug 8, 2015 at 2:22 AM, Russell King - ARM Linux
wrote:
> On Fri, Aug 07, 2015 at 08:18:48PM -0700, Duc Dang wrote:
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 890ad9d..5d03f8b 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -93,14 +93,14 @@ static int xhci_plat_probe(struct platform_device *pdev)
>> if (irq < 0)
>> return -ENODEV;
>>
>> - /* Initialize dma_mask and coherent_dma_mask to 32-bits */
>> - ret = dma_set_coherent_mask(>dev, DMA_BIT_MASK(32));
>> - if (ret)
>> - return ret;
>> - if (!pdev->dev.dma_mask)
>> - pdev->dev.dma_mask = >dev.coherent_dma_mask;
>> - else
>> - dma_set_mask(>dev, DMA_BIT_MASK(32));
>> + /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
>> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
>> + if (ret) {
>> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
>> + if (ret)
>> + return ret;
>> + }
>
> Note that dma_set_mask_and_coherent() and the original code are not
> equivalent because of this:
>
> if (!pdev->dev.dma_mask)
> pdev->dev.dma_mask = >dev.coherent_dma_mask;
>
> If we know that pdev->dev.dma_mask will always be initialised at this
> point, then the above change is fine. If not, it's introducing a
> regression - dma_set_mask_and_coherent() will fail if pdev->dev.dma_mask
> is NULL (depending on the architectures implementation of dma_set_mask()).
>
> Prefixing the above change with the two lines I mention above would
> ensure equivalent behaviour. Even if we do want to get rid of this,
> I'd advise to do it as a separate patch after this change, which can
> be independently reverted if there's problems with its removal.
>
Hi Russell,
I will add the 2 lines you mentioned back to next version of the
patch. It is safer to do it that way as I do not see
pdev->dev.dma_mask gets initialized before the call
dma_set_mask_and_coherent inside this xhci_plat.c file.
> --
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.
--
Regards,
Duc Dang.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
On Fri, Aug 07, 2015 at 08:18:48PM -0700, Duc Dang wrote:
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 890ad9d..5d03f8b 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -93,14 +93,14 @@ static int xhci_plat_probe(struct platform_device *pdev)
> if (irq < 0)
> return -ENODEV;
>
> - /* Initialize dma_mask and coherent_dma_mask to 32-bits */
> - ret = dma_set_coherent_mask(>dev, DMA_BIT_MASK(32));
> - if (ret)
> - return ret;
> - if (!pdev->dev.dma_mask)
> - pdev->dev.dma_mask = >dev.coherent_dma_mask;
> - else
> - dma_set_mask(>dev, DMA_BIT_MASK(32));
> + /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
> + if (ret) {
> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
> + if (ret)
> + return ret;
> + }
Note that dma_set_mask_and_coherent() and the original code are not
equivalent because of this:
if (!pdev->dev.dma_mask)
pdev->dev.dma_mask = >dev.coherent_dma_mask;
If we know that pdev->dev.dma_mask will always be initialised at this
point, then the above change is fine. If not, it's introducing a
regression - dma_set_mask_and_coherent() will fail if pdev->dev.dma_mask
is NULL (depending on the architectures implementation of dma_set_mask()).
Prefixing the above change with the two lines I mention above would
ensure equivalent behaviour. Even if we do want to get rid of this,
I'd advise to do it as a separate patch after this change, which can
be independently reverted if there's problems with its removal.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
On Fri, Aug 07, 2015 at 08:18:48PM -0700, Duc Dang wrote:
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 890ad9d..5d03f8b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -93,14 +93,14 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (irq 0)
return -ENODEV;
- /* Initialize dma_mask and coherent_dma_mask to 32-bits */
- ret = dma_set_coherent_mask(pdev-dev, DMA_BIT_MASK(32));
- if (ret)
- return ret;
- if (!pdev-dev.dma_mask)
- pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
- else
- dma_set_mask(pdev-dev, DMA_BIT_MASK(32));
+ /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
+ ret = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(64));
+ if (ret) {
+ ret = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(32));
+ if (ret)
+ return ret;
+ }
Note that dma_set_mask_and_coherent() and the original code are not
equivalent because of this:
if (!pdev-dev.dma_mask)
pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
If we know that pdev-dev.dma_mask will always be initialised at this
point, then the above change is fine. If not, it's introducing a
regression - dma_set_mask_and_coherent() will fail if pdev-dev.dma_mask
is NULL (depending on the architectures implementation of dma_set_mask()).
Prefixing the above change with the two lines I mention above would
ensure equivalent behaviour. Even if we do want to get rid of this,
I'd advise to do it as a separate patch after this change, which can
be independently reverted if there's problems with its removal.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
On Sat, Aug 8, 2015 at 2:22 AM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
On Fri, Aug 07, 2015 at 08:18:48PM -0700, Duc Dang wrote:
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 890ad9d..5d03f8b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -93,14 +93,14 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (irq 0)
return -ENODEV;
- /* Initialize dma_mask and coherent_dma_mask to 32-bits */
- ret = dma_set_coherent_mask(pdev-dev, DMA_BIT_MASK(32));
- if (ret)
- return ret;
- if (!pdev-dev.dma_mask)
- pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
- else
- dma_set_mask(pdev-dev, DMA_BIT_MASK(32));
+ /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
+ ret = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(64));
+ if (ret) {
+ ret = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(32));
+ if (ret)
+ return ret;
+ }
Note that dma_set_mask_and_coherent() and the original code are not
equivalent because of this:
if (!pdev-dev.dma_mask)
pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
If we know that pdev-dev.dma_mask will always be initialised at this
point, then the above change is fine. If not, it's introducing a
regression - dma_set_mask_and_coherent() will fail if pdev-dev.dma_mask
is NULL (depending on the architectures implementation of dma_set_mask()).
Prefixing the above change with the two lines I mention above would
ensure equivalent behaviour. Even if we do want to get rid of this,
I'd advise to do it as a separate patch after this change, which can
be independently reverted if there's problems with its removal.
Hi Russell,
I will add the 2 lines you mentioned back to next version of the
patch. It is safer to do it that way as I do not see
pdev-dev.dma_mask gets initialized before the call
dma_set_mask_and_coherent inside this xhci_plat.c file.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
Regards,
Duc Dang.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
The xhci platform driver needs to work on systems that
either only support 64-bit DMA or only support 32-bit DMA.
Attempt to set a coherent dma mask for 64-bit DMA, and
attempt again with 32-bit DMA if that fails.
[dhdang: Regenerate the patch over 4.2-rc5]
Signed-off-by: Mark Langsdorf
Tested-by: Mark Salter
Signed-off-by: Duc Dang
---
Changes from v4:
None
Changes from v3:
Re-generate the patch over 4.2-rc5
No code change.
Changes from v2:
None
Changes from v1:
Consolidated to use dma_set_mask_and_coherent
Got rid of the check against sizeof(dma_addr_t)
drivers/usb/host/xhci-plat.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 890ad9d..5d03f8b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -93,14 +93,14 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (irq < 0)
return -ENODEV;
- /* Initialize dma_mask and coherent_dma_mask to 32-bits */
- ret = dma_set_coherent_mask(>dev, DMA_BIT_MASK(32));
- if (ret)
- return ret;
- if (!pdev->dev.dma_mask)
- pdev->dev.dma_mask = >dev.coherent_dma_mask;
- else
- dma_set_mask(>dev, DMA_BIT_MASK(32));
+ /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
+ ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
+ if (ret) {
+ ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
+ if (ret)
+ return ret;
+ }
+
hcd = usb_create_hcd(driver, >dev, dev_name(>dev));
if (!hcd)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
The xhci platform driver needs to work on systems that
either only support 64-bit DMA or only support 32-bit DMA.
Attempt to set a coherent dma mask for 64-bit DMA, and
attempt again with 32-bit DMA if that fails.
[dhdang: Regenerate the patch over 4.2-rc5]
Signed-off-by: Mark Langsdorf mlang...@redhat.com
Tested-by: Mark Salter msal...@redhat.com
Signed-off-by: Duc Dang dhd...@apm.com
---
Changes from v4:
None
Changes from v3:
Re-generate the patch over 4.2-rc5
No code change.
Changes from v2:
None
Changes from v1:
Consolidated to use dma_set_mask_and_coherent
Got rid of the check against sizeof(dma_addr_t)
drivers/usb/host/xhci-plat.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 890ad9d..5d03f8b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -93,14 +93,14 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (irq 0)
return -ENODEV;
- /* Initialize dma_mask and coherent_dma_mask to 32-bits */
- ret = dma_set_coherent_mask(pdev-dev, DMA_BIT_MASK(32));
- if (ret)
- return ret;
- if (!pdev-dev.dma_mask)
- pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
- else
- dma_set_mask(pdev-dev, DMA_BIT_MASK(32));
+ /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
+ ret = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(64));
+ if (ret) {
+ ret = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(32));
+ if (ret)
+ return ret;
+ }
+
hcd = usb_create_hcd(driver, pdev-dev, dev_name(pdev-dev));
if (!hcd)
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
