2014-06-12 17:40:43

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH 0/2] ahci: Add support for 64-bit DMA mask driver

From: Suravee Suthikulpanit <[email protected]>

V2:
- Changes based on review comments from Hans and Bartlomiej.
- Remove the logic in ahci_xgene as suggested by Bartlomiej.

Suravee Suthikulpanit (2):
ata: Check and set 64-bit DMA mask for platform AHCI driver
ahci/xgene: Remove logic to set 64-bit DMA mask

drivers/ata/ahci_xgene.c | 10 ----------
drivers/ata/libahci_platform.c | 13 +++++++++++++
2 files changed, 13 insertions(+), 10 deletions(-)

--
1.9.0


2014-06-12 17:40:57

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH 1/2] ahci: Check and set 64-bit DMA mask for platform AHCI driver

From: Suravee Suthikulpanit <[email protected]>

The current platform AHCI driver does not set the dma_mask correctly
for 64-bit DMA capable AHCI controller. This patch checks the AHCI
capability bit and set the dma_mask and coherent_dma_mask accordingly.

Signed-off-by: Suravee Suthikulpanit <[email protected]>
Reviewed-by: Bartlomiej Zolnierkiewicz <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
---
drivers/ata/libahci_platform.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 3a5b4ed..a958a2b 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -364,6 +364,19 @@ int ahci_platform_init_host(struct platform_device *pdev,
ap->ops = &ata_dummy_port_ops;
}

+ if (hpriv->cap & HOST_CAP_64) {
+ rc = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
+ if (rc) {
+ rc = dma_coerce_mask_and_coherent(dev,
+ DMA_BIT_MASK(32));
+ if (rc) {
+ dev_err(dev, "Failed to enable 64-bit DMA.\n");
+ return rc;
+ }
+ dev_warn(dev, "Enable 32-bit DMA instead of 64-bit.\n");
+ }
+ }
+
rc = ahci_reset_controller(host);
if (rc)
return rc;
--
1.9.0

2014-06-12 17:41:16

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH 2/2] ahci/xgene: Remove logic to set 64-bit DMA mask

From: Suravee Suthikulpanit <[email protected]>

Instead of doing the check here, this should be handled
in the common AHCI platform code.

Signed-off-by: Suravee Suthikulpanit <[email protected]>
Suggested-by: Bartlomiej Zolnierkiewicz <[email protected]>
Cc: Loc Ho <[email protected]>
Cc: Tuan Phan <[email protected]>
Cc: Suman Triphati <[email protected]>
---
drivers/ata/ahci_xgene.c | 10 ----------
1 file changed, 10 deletions(-)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 042a9bb..a9fc2ae 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -440,16 +440,6 @@ static int xgene_ahci_probe(struct platform_device *pdev)
/* Configure the host controller */
xgene_ahci_hw_init(hpriv);

- /*
- * Setup DMA mask. This is preliminary until the DMA range is sorted
- * out.
- */
- rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
- if (rc) {
- dev_err(dev, "Unable to set dma mask\n");
- goto disable_resources;
- }
-
hflags = AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ;

rc = ahci_platform_init_host(pdev, hpriv, &xgene_ahci_port_info,
--
1.9.0

2014-06-12 17:47:26

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 1/2] ahci: Check and set 64-bit DMA mask for platform AHCI driver

Hello.

On 06/12/2014 09:40 PM, [email protected] wrote:

> From: Suravee Suthikulpanit <[email protected]>

> The current platform AHCI driver does not set the dma_mask correctly
> for 64-bit DMA capable AHCI controller. This patch checks the AHCI
> capability bit and set the dma_mask and coherent_dma_mask accordingly.

> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> Reviewed-by: Bartlomiej Zolnierkiewicz <[email protected]>
> Reviewed-by: Hans de Goede <[email protected]>
> ---
> drivers/ata/libahci_platform.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)

> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index 3a5b4ed..a958a2b 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -364,6 +364,19 @@ int ahci_platform_init_host(struct platform_device *pdev,
> ap->ops = &ata_dummy_port_ops;
> }
>
> + if (hpriv->cap & HOST_CAP_64) {
> + rc = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
> + if (rc) {
> + rc = dma_coerce_mask_and_coherent(dev,
> + DMA_BIT_MASK(32));
> + if (rc) {
> + dev_err(dev, "Failed to enable 64-bit DMA.\n");

Not 32-bit?

> + return rc;
> + }
> + dev_warn(dev, "Enable 32-bit DMA instead of 64-bit.\n");
> + }
> + }
> +

WBR, Sergei

2014-06-13 08:40:34

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 1/2] ahci: Check and set 64-bit DMA mask for platform AHCI driver

Hi,

On 06/12/2014 07:40 PM, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> The current platform AHCI driver does not set the dma_mask correctly
> for 64-bit DMA capable AHCI controller. This patch checks the AHCI
> capability bit and set the dma_mask and coherent_dma_mask accordingly.
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> Reviewed-by: Bartlomiej Zolnierkiewicz <[email protected]>
> Reviewed-by: Hans de Goede <[email protected]>

I did not review this patch, just because you CC-ed me does not mean
I've reviewed it. Please don't add my Reviewed-by unless you've gotten
a mail from me saying something along these lines:

This patch is:
Reviewed-by: Hans de Goede <[email protected]>

More in general please do not add any tags, other then Cc or your own
Signed-off-by, unless you've actually received those tags in an email
from people.

Regards,

Hans

p.s.

About the patch itself, I'll try to test this on sunxi devices to
make sure that it does not break anything this weekend.



> ---
> drivers/ata/libahci_platform.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index 3a5b4ed..a958a2b 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -364,6 +364,19 @@ int ahci_platform_init_host(struct platform_device *pdev,
> ap->ops = &ata_dummy_port_ops;
> }
>
> + if (hpriv->cap & HOST_CAP_64) {
> + rc = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
> + if (rc) {
> + rc = dma_coerce_mask_and_coherent(dev,
> + DMA_BIT_MASK(32));
> + if (rc) {
> + dev_err(dev, "Failed to enable 64-bit DMA.\n");
> + return rc;
> + }
> + dev_warn(dev, "Enable 32-bit DMA instead of 64-bit.\n");
> + }
> + }
> +
> rc = ahci_reset_controller(host);
> if (rc)
> return rc;
>

2014-06-13 12:45:34

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: Re: [PATCH 1/2] ahci: Check and set 64-bit DMA mask for platform AHCI driver

On 6/12/2014 12:47 PM, Sergei Shtylyov wrote:
>> diff --git a/drivers/ata/libahci_platform.c
>> b/drivers/ata/libahci_platform.c
>> index 3a5b4ed..a958a2b 100644
>> --- a/drivers/ata/libahci_platform.c
>> +++ b/drivers/ata/libahci_platform.c
>> @@ -364,6 +364,19 @@ int ahci_platform_init_host(struct
>> platform_device *pdev,
>> ap->ops = &ata_dummy_port_ops;
>> }
>>
>> + if (hpriv->cap & HOST_CAP_64) {
>> + rc = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
>> + if (rc) {
>> + rc = dma_coerce_mask_and_coherent(dev,
>> + DMA_BIT_MASK(32));
>> + if (rc) {
>> + dev_err(dev, "Failed to enable 64-bit DMA.\n");
>
> Not 32-bit?
Actually, I intended to say 64 since this is supposed to be setting up
64-bit DMA mask. Or we could just say failed to set up DMA mask.

Suravee

2014-06-13 20:21:29

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 1/2] ahci: Check and set 64-bit DMA mask for platform AHCI driver

Hi,

On 06/12/2014 07:40 PM, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> The current platform AHCI driver does not set the dma_mask correctly
> for 64-bit DMA capable AHCI controller. This patch checks the AHCI
> capability bit and set the dma_mask and coherent_dma_mask accordingly.
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> Reviewed-by: Bartlomiej Zolnierkiewicz <[email protected]>
> Reviewed-by: Hans de Goede <[email protected]>

I see no adverse effects from this on sunxi, please drop the
Reviewed-by: Hans .... tag and replace it with:

Tested-by: Hans de Goede <[email protected]>

Regards,

Hans



> ---
> drivers/ata/libahci_platform.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index 3a5b4ed..a958a2b 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -364,6 +364,19 @@ int ahci_platform_init_host(struct platform_device *pdev,
> ap->ops = &ata_dummy_port_ops;
> }
>
> + if (hpriv->cap & HOST_CAP_64) {
> + rc = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
> + if (rc) {
> + rc = dma_coerce_mask_and_coherent(dev,
> + DMA_BIT_MASK(32));
> + if (rc) {
> + dev_err(dev, "Failed to enable 64-bit DMA.\n");
> + return rc;
> + }
> + dev_warn(dev, "Enable 32-bit DMA instead of 64-bit.\n");
> + }
> + }
> +
> rc = ahci_reset_controller(host);
> if (rc)
> return rc;
>

2014-06-17 19:03:49

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH 1/2] ahci: Check and set 64-bit DMA mask for platform AHCI driver

On Thu, Jun 12, 2014 at 12:40:23PM -0500, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> The current platform AHCI driver does not set the dma_mask correctly
> for 64-bit DMA capable AHCI controller. This patch checks the AHCI
> capability bit and set the dma_mask and coherent_dma_mask accordingly.
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> Reviewed-by: Bartlomiej Zolnierkiewicz <[email protected]>
> Reviewed-by: Hans de Goede <[email protected]>

Applied 1-2 to libata/for-3.17.

Thanks.

--
tejun