ata: sata_rcar: exclude setting of PHY registers in Gen3

From: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>

According to documentation, setting of PHY registers is unnecessary with
R-Car Gen3. The registers are not even described. So, don't initialize
them.

Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
[wsa: updated commit message]
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Tested with a Renesas R-Car M3N Salvator-XS board. I did a platform suspend to
check if the controller and PHY work properly after resume.

 drivers/ata/sata_rcar.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Hi Wolfram,

On Mon, Aug 6, 2018 at 12:43 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> From: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
>
> According to documentation, setting of PHY registers is unnecessary with
> R-Car Gen3. The registers are not even described. So, don't initialize
> them.
>
> Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> [wsa: updated commit message]
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Thanks for your patch!

> Tested with a Renesas R-Car M3N Salvator-XS board. I did a platform suspend to
> check if the controller and PHY work properly after resume.

"platform suspend"? Is that s2ram aka a full PSCI system suspend?
I guess not, as it is supposed to fail without PCA9654 suspend/resume support...

So more information is needed to convince me ;-)

Gr{oetje,eeting}s,

                        Geert

On 08/06/2018 01:42 PM, Wolfram Sang wrote:

> From: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> 
> According to documentation, setting of PHY registers is unnecessary with
> R-Car Gen3. The registers are not even described. So, don't initialize
> them.
> 
> Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> [wsa: updated commit message]
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[...]

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

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

> "platform suspend"? Is that s2ram aka a full PSCI system suspend?

It is this:

echo platform > /sys/power/pm_test
echo mem > /sys/power/state

Is there a better name for it?

The HDD was also spinning down/up during the cycle...

> I guess not, as it is supposed to fail without PCA9654 suspend/resume support...

Yeah, I was wondering about that last time, too.

> So more information is needed to convince me ;-)

Here they are :)

Hi Wolfram,

On Mon, Aug 6, 2018 at 5:00 PM Wolfram Sang <wsa@the-dreams.de> wrote:
> > "platform suspend"? Is that s2ram aka a full PSCI system suspend?
>
> It is this:
>
> echo platform > /sys/power/pm_test
> echo mem > /sys/power/state

Thanks!

> Is there a better name for it?

I don't know. It never gets to the actual system suspend step.

> The HDD was also spinning down/up during the cycle...
>
> > I guess not, as it is supposed to fail without PCA9654 suspend/resume support...
>
> Yeah, I was wondering about that last time, too.
>
> > So more information is needed to convince me ;-)
>
> Here they are :)

With "platform", it doesn't do the full cycle.
Please try without that step, or do "echo none > /sys/power/pm_test".

Thanks!

Gr{oetje,eeting}s,

                        Geert

Hi Geert,

> Please try without that step, or do "echo none > /sys/power/pm_test".

Did this now with a q'n'd hacked gpio driver (see below). Worked like a
charm. So, the sata_rcar patch under discussion here seems to be fine.
We only need the GPIO resume on specific boards. This is a seperate
task. D'accord?

Thanks,

   Wolfram

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 023a32cfac42..17ab27ca0187 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -914,6 +914,7 @@ static int pca953x_probe(struct i2c_client *client,
 			dev_warn(&client->dev, "setup failed, %d\n", ret);
 	}
 
+	dev_set_drvdata(&client->dev, chip);
 	i2c_set_clientdata(client, chip);
 	return 0;
 
@@ -986,11 +987,27 @@ static const struct of_device_id pca953x_dt_ids[] = {
 
 MODULE_DEVICE_TABLE(of, pca953x_dt_ids);
 
+static int pca953x_resume(struct device *dev)
+{
+	struct pca953x_chip *chip = dev_get_drvdata(dev);
+
+	pca953x_write_regs(chip, chip->regs->output, chip->reg_output);
+
+	pca953x_write_regs(chip, chip->regs->direction,
+				chip->reg_direction);
+	return 0;
+}
+
+static const struct dev_pm_ops pca953x_dev_pm_ops = {
+	.resume = pca953x_resume,
+};
+
 static struct i2c_driver pca953x_driver = {
 	.driver = {
 		.name	= "pca953x",
 		.of_match_table = pca953x_dt_ids,
 		.acpi_match_table = ACPI_PTR(pca953x_acpi_ids),
+		.pm	= &pca953x_dev_pm_ops,
 	},
 	.probe		= pca953x_probe,
 	.remove		= pca953x_remove,

On Mon, Aug 06, 2018 at 12:42:00PM +0200, Wolfram Sang wrote:
> From: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> 
> According to documentation, setting of PHY registers is unnecessary with
> R-Car Gen3. The registers are not even described. So, don't initialize
> them.
> 
> Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> [wsa: updated commit message]
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied to libata/for-4.19.

Thanks.

Hi Wolfram,

On Mon, Aug 6, 2018 at 7:04 PM Wolfram Sang <wsa@the-dreams.de> wrote:
> > Please try without that step, or do "echo none > /sys/power/pm_test".
>
> Did this now with a q'n'd hacked gpio driver (see below). Worked like a
> charm. So, the sata_rcar patch under discussion here seems to be fine.
> We only need the GPIO resume on specific boards. This is a seperate
> task. D'accord?

Thanks for testing!

Gr{oetje,eeting}s,

                        Geert