[PATCH] ata: sata_rcar: Disable DIPM mode for r8a7790 ES1

Discussion:

Simon Horman

2014-10-16 05:50:52 UTC

Unlike other SATA R-Car r8a7790 controllers the r8a7790 ES1 SATA R-Car
controller needs to be run with DIPM disabled.

Loosely based on work by Koji Matsuoka.

Signed-off-by: Simon Horman <horms+***@verge.net.au>
---
This patch is against for-next branch of Tejun's libata tree.

Documentation/devicetree/bindings/ata/sata_rcar.txt | 3 ++-
drivers/ata/sata_rcar.c | 10 ++++++++++
2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt b/Documentation/devicetree/bindings/ata/sata_rcar.txt
index 1e61113..7dd32d3 100644
--- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
+++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
@@ -3,7 +3,8 @@
Required properties:
- compatible : should contain one of the following:
- "renesas,sata-r8a7779" for R-Car H1
- - "renesas,sata-r8a7790" for R-Car H2
+ - "renesas,sata-r8a7790-es1" for R-Car H2 ES1
+ - "renesas,sata-r8a7790" for R-Car H2 other than ES1
- "renesas,sata-r8a7791" for R-Car M2
- reg : address and length of the SATA registers;
- interrupts : must consist of one interrupt specifier.
diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index 61eb6d7..d5cacb5 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -146,6 +146,7 @@
enum sata_rcar_type {
RCAR_GEN1_SATA,
RCAR_GEN2_SATA,
+ RCAR_R8A7790_ES1_SATA,
};

struct sata_rcar_priv {
@@ -763,6 +764,10 @@ static void sata_rcar_setup_port(struct ata_host *host)
ap->udma_mask = ATA_UDMA6;
ap->flags |= ATA_FLAG_SATA;

+ if (priv->type == RCAR_R8A7790_ES1_SATA) {
+ ap->flags |= ATA_FLAG_NO_DIPM;
+ }
+
ioaddr->cmd_addr = base + SDATA_REG;
ioaddr->ctl_addr = base + SSDEVCON_REG;
ioaddr->scr_addr = base + SCRSSTS_REG;
@@ -838,6 +843,10 @@ static struct of_device_id sata_rcar_match[] = {
.data = (void *)RCAR_GEN2_SATA
},
{
+ .compatible = "renesas,sata-r8a7790-es1",
+ .data = (void *)RCAR_R8A7790_ES1_SATA
+ },
+ {
.compatible = "renesas,sata-r8a7791",
.data = (void *)RCAR_GEN2_SATA
},
@@ -849,6 +858,7 @@ static const struct platform_device_id sata_rcar_id_table[] = {
{ "sata_rcar", RCAR_GEN1_SATA }, /* Deprecated by "sata-r8a7779" */
{ "sata-r8a7779", RCAR_GEN1_SATA },
{ "sata-r8a7790", RCAR_GEN2_SATA },
+ { "sata-r8a7790-es1", RCAR_R8A7790_ES1_SATA },
{ "sata-r8a7791", RCAR_GEN2_SATA },
{ },
};

--
2.1.1

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Magnus Damm

2014-10-17 05:25:34 UTC

Permalink

Hi Simon,

On Thu, Oct 16, 2014 at 7:50 AM, Simon Horman

Thanks for cooking up this improved version of the ES1 workaround.

Post by Simon Horman
Documentation/devicetree/bindings/ata/sata_rcar.txt | 3 ++-
drivers/ata/sata_rcar.c | 10 ++++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt b/Documentation/devicetree/bindings/ata/sata_rcar.txt
index 1e61113..7dd32d3 100644
--- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
+++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
@@ -3,7 +3,8 @@
- "renesas,sata-r8a7779" for R-Car H1
- - "renesas,sata-r8a7790" for R-Car H2
+ - "renesas,sata-r8a7790-es1" for R-Car H2 ES1
+ - "renesas,sata-r8a7790" for R-Car H2 other than ES1
- "renesas,sata-r8a7791" for R-Car M2
- reg : address and length of the SATA registers;
- interrupts : must consist of one interrupt specifier.

This portion looks fine to me.

Post by Simon Horman
diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index 61eb6d7..d5cacb5 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -146,6 +146,7 @@
enum sata_rcar_type {
RCAR_GEN1_SATA,
RCAR_GEN2_SATA,
+ RCAR_R8A7790_ES1_SATA,
};
struct sata_rcar_priv {
@@ -763,6 +764,10 @@ static void sata_rcar_setup_port(struct ata_host *host)
ap->udma_mask = ATA_UDMA6;
ap->flags |= ATA_FLAG_SATA;
+ if (priv->type == RCAR_R8A7790_ES1_SATA) {
+ ap->flags |= ATA_FLAG_NO_DIPM;
+ }
+
ioaddr->cmd_addr = base + SDATA_REG;
ioaddr->ctl_addr = base + SSDEVCON_REG;
ioaddr->scr_addr = base + SCRSSTS_REG;
@@ -838,6 +843,10 @@ static struct of_device_id sata_rcar_match[] = {
.data = (void *)RCAR_GEN2_SATA
},
{
+ .compatible = "renesas,sata-r8a7790-es1",
+ .data = (void *)RCAR_R8A7790_ES1_SATA
+ },
+ {
.compatible = "renesas,sata-r8a7791",
.data = (void *)RCAR_GEN2_SATA
},
@@ -849,6 +858,7 @@ static const struct platform_device_id sata_rcar_id_table[] = {
{ "sata_rcar", RCAR_GEN1_SATA }, /* Deprecated by "sata-r8a7779" */
{ "sata-r8a7779", RCAR_GEN1_SATA },
{ "sata-r8a7790", RCAR_GEN2_SATA },
+ { "sata-r8a7790-es1", RCAR_R8A7790_ES1_SATA },
{ "sata-r8a7791", RCAR_GEN2_SATA },
{ },
};
--
2.1.1
From my side I'm not sure if the platform device interface at this

point really needs to cover the ES portion or not. It may not hurt to
be consistent though so no need to change from my side.

Is this patch all needed in the driver to handle the ES1 case?

I'm asking because it looks like the switch() statement in
sata_rcar_init_controller() may accidentally skip some gen2 related
setup code in case of ES1, maybe this is intentional or perhaps some
update is needed.

Cheers,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Simon Horman

2014-10-17 06:44:14 UTC

Permalink

Post by Magnus Damm
Hi Simon,
On Thu, Oct 16, 2014 at 7:50 AM, Simon Horman

Thanks for cooking up this improved version of the ES1 workaround.

This portion looks fine to me.

Thanks Magnus,

that is not intentional, its an oversight on my part.

As it does not appear to be difficult to be consistent for the platform
device case I think it would be good to do so.

It seems that the following incremental change would address your concern.

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index 61eb6d7..01c8505 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -792,6 +792,7 @@ static void sata_rcar_init_controller(struct ata_host *host)
sata_rcar_gen1_phy_init(priv);
break;
case RCAR_GEN2_SATA:
+ case RCAR_R8A7790_ES1_SATA:
sata_rcar_gen2_phy_init(priv);
break;
default:
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Sergei Shtylyov

2014-10-17 19:54:33 UTC

Permalink

Hello.

Post by Simon Horman
Unlike other SATA R-Car r8a7790 controllers the r8a7790 ES1 SATA R-Car
controller needs to be run with DIPM disabled.
Loosely based on work by Koji Matsuoka.
---
This patch is against for-next branch of Tejun's libata tree.
Documentation/devicetree/bindings/ata/sata_rcar.txt | 3 ++-
drivers/ata/sata_rcar.c | 10 ++++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt b/Documentation/devicetree/bindings/ata/sata_rcar.txt
index 1e61113..7dd32d3 100644
--- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
+++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
@@ -3,7 +3,8 @@
- "renesas,sata-r8a7779" for R-Car H1
- - "renesas,sata-r8a7790" for R-Car H2
+ - "renesas,sata-r8a7790-es1" for R-Car H2 ES1
+ - "renesas,sata-r8a7790" for R-Car H2 other than ES1
- "renesas,sata-r8a7791" for R-Car M2
- reg : address and length of the SATA registers;
- interrupts : must consist of one interrupt specifier.
diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index 61eb6d7..d5cacb5 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -146,6 +146,7 @@
enum sata_rcar_type {
RCAR_GEN1_SATA,
RCAR_GEN2_SATA,
+ RCAR_R8A7790_ES1_SATA,

Hm, can't we read the SoC revision somewhere?

Post by Simon Horman
};
struct sata_rcar_priv {
@@ -763,6 +764,10 @@ static void sata_rcar_setup_port(struct ata_host *host)
ap->udma_mask = ATA_UDMA6;
ap->flags |= ATA_FLAG_SATA;
+ if (priv->type == RCAR_R8A7790_ES1_SATA) {
+ ap->flags |= ATA_FLAG_NO_DIPM;
+ }

{} not needed here.

[...]

Back home, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html