Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1558374 > unrolled thread

[PATCH 00/10] ARM: da850-lcdk: add SATA support

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/10] ARM: da850-lcdk: add SATA support Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-13 13:50 +0100
    [PATCH 06/10] sata: ahci_da850: implement a softreset quirk Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-13 13:50 +0100
      Re: [PATCH 06/10] sata: ahci_da850: implement a softreset quirk Tejun Heo <tj@kernel.org> - 2017-01-16 00:20 +0100
        Re: [PATCH 06/10] sata: ahci_da850: implement a softreset quirk Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-16 11:20 +0100
    [PATCH 01/10] ARM: davinci: add a clock lookup entry for the SATA clock Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-13 13:50 +0100
    [PATCH 09/10] ARM: dts: da850: add the SATA node Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-13 13:50 +0100
      Re: [PATCH 09/10] ARM: dts: da850: add the SATA node David Lechner <david@lechnology.com> - 2017-01-13 20:40 +0100
        Re: [PATCH 09/10] ARM: dts: da850: add the SATA node Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-16 11:10 +0100
    [PATCH 03/10] devicetree: bindings: add bindings for ahci-da850 Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-13 13:50 +0100
      Re: [PATCH 03/10] devicetree: bindings: add bindings for ahci-da850 David Lechner <david@lechnology.com> - 2017-01-13 20:30 +0100
        Re: [PATCH 03/10] devicetree: bindings: add bindings for ahci-da850 Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-16 11:20 +0100
          Re: [PATCH 03/10] devicetree: bindings: add bindings for ahci-da850 Sekhar Nori <nsekhar@ti.com> - 2017-01-16 13:50 +0100
            Re: [PATCH 03/10] devicetree: bindings: add bindings for ahci-da850 Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-16 15:40 +0100
              Re: [PATCH 03/10] devicetree: bindings: add bindings for ahci-da850 David Lechner <david@lechnology.com> - 2017-01-16 19:50 +0100
    [PATCH 05/10] sata: ahci_da850: add device tree match table Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-13 13:50 +0100
    [PATCH 08/10] ARM: dts: da850: add pinmux settings for the SATA controller Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-13 13:50 +0100
    [PATCH 07/10] sata: ahci_da850: add support for the da850,clk_multiplier DT property Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-13 13:50 +0100
      Re: [PATCH 07/10] sata: ahci_da850: add support for the  da850,clk_multiplier DT property David Lechner <david@lechnology.com> - 2017-01-13 21:40 +0100
    [PATCH 04/10] sata: hardreset: retry if phys link is down Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-13 13:50 +0100
      Re: [PATCH 04/10] sata: hardreset: retry if phys link is down Tejun Heo <tj@kernel.org> - 2017-01-16 00:20 +0100
        Re: [PATCH 04/10] sata: hardreset: retry if phys link is down Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-16 13:30 +0100
    [PATCH 02/10] ARM: davinci_all_defconfig: enable SATA modules Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2017-01-13 14:50 +0100
    Re: [PATCH 00/10] ARM: da850-lcdk: add SATA support Sekhar Nori <nsekhar@ti.com> - 2017-01-13 15:40 +0100

Page 1 of 2  [1] 2  Next page →

#1558374 — [PATCH 00/10] ARM: da850-lcdk: add SATA support

This series contains all the changes necessary to make SATA work on
the da850-lcdk board.

The first patch adds a clock lookup entry required for the ahci core
to retrieve a functional clock.

The second enables relevant config options for all davinci boards.

The third adds device tree bindings for the ahci_da850 driver.

The fourth adds a workaround for a SATA controller instability we
detected after increasing the PLL0 frequency for proper LCD
controller support.

Patches 5 through 7 extend the ahci_da850 driver - add DT support,
un-hardcode the clock multiplier value and add a workaround for
a quirk present on the da850 SATA controller.

Patches 8-10 add the device tree changes required to probe the driver.

I'm posting the series as a whole to give all reviewers the full
picture and visibility of the changes required, if needed I can resend
the patches separately.

Bartosz Golaszewski (10):
  ARM: davinci: add a clock lookup entry for the SATA clock
  ARM: davinci_all_defconfig: enable SATA modules
  devicetree: bindings: add bindings for ahci-da850
  sata: hardreset: retry if phys link is down
  sata: ahci_da850: add device tree match table
  sata: ahci_da850: implement a softreset quirk
  sata: ahci_da850: add support for the da850,clk_multiplier DT property
  ARM: dts: da850: add pinmux settings for the SATA controller
  ARM: dts: da850: add the SATA node
  ARM: dts: da850-lcdk: enable the SATA node

 .../devicetree/bindings/ata/ahci-da850.txt         |  21 ++++
 arch/arm/boot/dts/da850-lcdk.dts                   |   5 +
 arch/arm/boot/dts/da850.dtsi                       |  30 ++++++
 arch/arm/configs/davinci_all_defconfig             |   2 +
 arch/arm/mach-davinci/da8xx-dt.c                   |   1 +
 drivers/ata/ahci_da850.c                           | 112 +++++++++++++++++++--
 drivers/ata/libata-core.c                          |  16 ++-
 include/linux/libata.h                             |   4 +-
 8 files changed, 177 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-da850.txt

-- 
2.9.3

[toc] | [next] | [standalone]

#1558375 — [PATCH 06/10] sata: ahci_da850: implement a softreset quirk

There's an issue with the da850 SATA controller: if port multiplier
support is compiled in, but we're connecting the drive directly to
the SATA port on the board, the drive can't be detected.

To make SATA work on the da850-lcdk board: first try to softreset
with pmp - if the operation fails with -EBUSY, retry without pmp.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/ata/ahci_da850.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index 5930af81..bb9eb4c 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -54,11 +54,31 @@ static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg,
 	writel(val, ahci_base + SATA_P0PHYCR_REG);
 }
 
+static int ahci_da850_softreset(struct ata_link *link,
+				unsigned int *class, unsigned long deadline)
+{
+	int pmp, ret;
+
+	pmp = sata_srst_pmp(link);
+
+	ret = ahci_do_softreset(link, class, pmp, deadline, ahci_check_ready);
+	if (pmp && ret == -EBUSY)
+		return ahci_do_softreset(link, class, 0,
+					 deadline, ahci_check_ready);
+
+	return ret;
+}
+
+static struct ata_port_operations ahci_da850_port_ops = {
+	.inherits = &ahci_platform_ops,
+	.softreset = ahci_da850_softreset,
+};
+
 static const struct ata_port_info ahci_da850_port_info = {
 	.flags		= AHCI_FLAG_COMMON,
 	.pio_mask	= ATA_PIO4,
 	.udma_mask	= ATA_UDMA6,
-	.port_ops	= &ahci_platform_ops,
+	.port_ops	= &ahci_da850_port_ops,
 };
 
 static struct scsi_host_template ahci_platform_sht = {
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1559361 — Re: [PATCH 06/10] sata: ahci_da850: implement a softreset quirk

On Fri, Jan 13, 2017 at 01:38:00PM +0100, Bartosz Golaszewski wrote:
> +static int ahci_da850_softreset(struct ata_link *link,
> +				unsigned int *class, unsigned long deadline)
> +{
> +	int pmp, ret;
> +
> +	pmp = sata_srst_pmp(link);
> +
> +	ret = ahci_do_softreset(link, class, pmp, deadline, ahci_check_ready);
> +	if (pmp && ret == -EBUSY)
> +		return ahci_do_softreset(link, class, 0,
> +					 deadline, ahci_check_ready);
> +
> +	return ret;
> +}

Please add some comments explaining what's going on.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1559591 — Re: [PATCH 06/10] sata: ahci_da850: implement a softreset quirk

2017-01-16 0:12 GMT+01:00 Tejun Heo <tj@kernel.org>:
> On Fri, Jan 13, 2017 at 01:38:00PM +0100, Bartosz Golaszewski wrote:
>> +static int ahci_da850_softreset(struct ata_link *link,
>> +                             unsigned int *class, unsigned long deadline)
>> +{
>> +     int pmp, ret;
>> +
>> +     pmp = sata_srst_pmp(link);
>> +
>> +     ret = ahci_do_softreset(link, class, pmp, deadline, ahci_check_ready);
>> +     if (pmp && ret == -EBUSY)
>> +             return ahci_do_softreset(link, class, 0,
>> +                                      deadline, ahci_check_ready);
>> +
>> +     return ret;
>> +}
>
> Please add some comments explaining what's going on.

Sure, I'll add some explanation in v2.

Thanks,
Bartosz Golaszewski

[toc] | [prev] | [next] | [standalone]

#1558376 — [PATCH 01/10] ARM: davinci: add a clock lookup entry for the SATA clock

This entry is needed for the ahci driver to get a functional clock.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/mach-davinci/da8xx-dt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index 9ee44da..b83e5d1 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -42,6 +42,7 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("ti,da830-ohci", 0x01e25000, "ohci-da8xx", NULL),
 	OF_DEV_AUXDATA("ti,da830-musb", 0x01e00000, "musb-da8xx", NULL),
 	OF_DEV_AUXDATA("ti,da830-usb-phy", 0x01c1417c, "da8xx-usb-phy", NULL),
+	OF_DEV_AUXDATA("ti,da850-ahci", 0x01e18000, "ahci_da850", NULL),
 	{}
 };
 
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1558377 — [PATCH 09/10] ARM: dts: da850: add the SATA node

Add the SATA node to the da850 device tree.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/boot/dts/da850.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 1f6a47d..f5086b1 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -427,6 +427,12 @@
 			phy-names = "usb-phy";
 			status = "disabled";
 		};
+		sata: ahci@0x218000 {
+			compatible = "ti,da850-ahci";
+			reg = <0x218000 0x2000>, <0x22c018 0x4>;
+			interrupts = <67>;
+			status = "disabled";
+		};
 		mdio: mdio@224000 {
 			compatible = "ti,davinci_mdio";
 			#address-cells = <1>;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1558703 — Re: [PATCH 09/10] ARM: dts: da850: add the SATA node

On 01/13/2017 06:38 AM, Bartosz Golaszewski wrote:
> Add the SATA node to the da850 device tree.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/boot/dts/da850.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 1f6a47d..f5086b1 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -427,6 +427,12 @@
>  			phy-names = "usb-phy";
>  			status = "disabled";
>  		};
> +		sata: ahci@0x218000 {

0x needs to be omitted.

	sata: ahci@218000 {

> +			compatible = "ti,da850-ahci";
> +			reg = <0x218000 0x2000>, <0x22c018 0x4>;
> +			interrupts = <67>;
> +			status = "disabled";
> +		};
>  		mdio: mdio@224000 {
>  			compatible = "ti,davinci_mdio";
>  			#address-cells = <1>;
>

[toc] | [prev] | [next] | [standalone]

#1559586 — Re: [PATCH 09/10] ARM: dts: da850: add the SATA node

2017-01-13 20:36 GMT+01:00 David Lechner <david@lechnology.com>:
> On 01/13/2017 06:38 AM, Bartosz Golaszewski wrote:
>>
>> Add the SATA node to the da850 device tree.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>>  arch/arm/boot/dts/da850.dtsi | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index 1f6a47d..f5086b1 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -427,6 +427,12 @@
>>                         phy-names = "usb-phy";
>>                         status = "disabled";
>>                 };
>> +               sata: ahci@0x218000 {
>
>
> 0x needs to be omitted.
>
>         sata: ahci@218000 {
>

Will fix in v2.

Thanks,
Bartosz Golaszewski

[toc] | [prev] | [next] | [standalone]

#1558378 — [PATCH 03/10] devicetree: bindings: add bindings for ahci-da850

Add DT bindings for the TI DA850 AHCI SATA controller.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 .../devicetree/bindings/ata/ahci-da850.txt          | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-da850.txt

diff --git a/Documentation/devicetree/bindings/ata/ahci-da850.txt b/Documentation/devicetree/bindings/ata/ahci-da850.txt
new file mode 100644
index 0000000..d07c241
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-da850.txt
@@ -0,0 +1,21 @@
+Device tree binding for the TI DA850 AHCI SATA Controller
+---------------------------------------------------------
+
+Required properties:
+  - compatible: must be "ti,da850-ahci"
+  - reg: physical base addresses and sizes of the controller's register areas
+  - interrupts: interrupt specifier (refer to the interrupt binding)
+
+Optional properties:
+  - clocks: clock specifier (refer to the common clock binding)
+  - da850,clk_multiplier: the multiplier for the reference clock needed
+                          for 1.5GHz PLL output
+
+Example:
+
+	sata: ahci@0x218000 {
+		compatible = "ti,da850-ahci";
+		reg = <0x218000 0x2000>, <0x22c018 0x4>;
+		interrupts = <67>;
+		da850,clk_multiplier = <7>;
+	};
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1558699 — Re: [PATCH 03/10] devicetree: bindings: add bindings for ahci-da850

On 01/13/2017 06:37 AM, Bartosz Golaszewski wrote:
> Add DT bindings for the TI DA850 AHCI SATA controller.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  .../devicetree/bindings/ata/ahci-da850.txt          | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/ahci-da850.txt
>
> diff --git a/Documentation/devicetree/bindings/ata/ahci-da850.txt b/Documentation/devicetree/bindings/ata/ahci-da850.txt
> new file mode 100644
> index 0000000..d07c241
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-da850.txt
> @@ -0,0 +1,21 @@
> +Device tree binding for the TI DA850 AHCI SATA Controller
> +---------------------------------------------------------
> +
> +Required properties:
> +  - compatible: must be "ti,da850-ahci"
> +  - reg: physical base addresses and sizes of the controller's register areas
> +  - interrupts: interrupt specifier (refer to the interrupt binding)
> +
> +Optional properties:
> +  - clocks: clock specifier (refer to the common clock binding)
> +  - da850,clk_multiplier: the multiplier for the reference clock needed
> +                          for 1.5GHz PLL output

A clock multiplier property seems redundant if you are specifying a 
clock. It should be possible to get the rate from the clock to determine 
which multiplier is needed.

> +
> +Example:
> +
> +	sata: ahci@0x218000 {
> +		compatible = "ti,da850-ahci";
> +		reg = <0x218000 0x2000>, <0x22c018 0x4>;
> +		interrupts = <67>;
> +		da850,clk_multiplier = <7>;
> +	};
>

[toc] | [prev] | [next] | [standalone]

#1559600 — Re: [PATCH 03/10] devicetree: bindings: add bindings for ahci-da850

2017-01-13 20:25 GMT+01:00 David Lechner <david@lechnology.com>:
> On 01/13/2017 06:37 AM, Bartosz Golaszewski wrote:
>>
>> Add DT bindings for the TI DA850 AHCI SATA controller.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>>  .../devicetree/bindings/ata/ahci-da850.txt          | 21
>> +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/ata/ahci-da850.txt
>>
>> diff --git a/Documentation/devicetree/bindings/ata/ahci-da850.txt
>> b/Documentation/devicetree/bindings/ata/ahci-da850.txt
>> new file mode 100644
>> index 0000000..d07c241
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/ata/ahci-da850.txt
>> @@ -0,0 +1,21 @@
>> +Device tree binding for the TI DA850 AHCI SATA Controller
>> +---------------------------------------------------------
>> +
>> +Required properties:
>> +  - compatible: must be "ti,da850-ahci"
>> +  - reg: physical base addresses and sizes of the controller's register
>> areas
>> +  - interrupts: interrupt specifier (refer to the interrupt binding)
>> +
>> +Optional properties:
>> +  - clocks: clock specifier (refer to the common clock binding)
>> +  - da850,clk_multiplier: the multiplier for the reference clock needed
>> +                          for 1.5GHz PLL output
>
>
> A clock multiplier property seems redundant if you are specifying a clock.
> It should be possible to get the rate from the clock to determine which
> multiplier is needed.
>

I probably should have named it differently. This is not a multiplier
of a clock derived from PLL0 or PLL1. Instead it's a value set by
writing to the Port PHY Control Register (MPY bits) of the SATA
controller that configures the multiplier for the external low-jitter
clock. On the lcdk the signals (REFCLKP, REFCLKN) are provided by
CDCM61001 (SATA OSCILLATOR component on the schematics).

I'll find a better name and comment the property accordingly.

FYI: the da850 platform does not use the common clock framework, so I
don't specify the clock property on the sata node in the device tree.
Instead I add the clock lookup entry in patch [01/10]. This is
transparent for AHCI which can get the clock as usual by calling
clk_get() in ahci_platform_get_resources().

Thanks,
Bartosz Golaszewski

[toc] | [prev] | [next] | [standalone]

#1559697 — Re: [PATCH 03/10] devicetree: bindings: add bindings for ahci-da850

On Monday 16 January 2017 03:43 PM, Bartosz Golaszewski wrote:
> 2017-01-13 20:25 GMT+01:00 David Lechner <david@lechnology.com>:
>> On 01/13/2017 06:37 AM, Bartosz Golaszewski wrote:
>>>
>>> Add DT bindings for the TI DA850 AHCI SATA controller.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> ---
>>>  .../devicetree/bindings/ata/ahci-da850.txt          | 21
>>> +++++++++++++++++++++
>>>  1 file changed, 21 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/ata/ahci-da850.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/ata/ahci-da850.txt
>>> b/Documentation/devicetree/bindings/ata/ahci-da850.txt
>>> new file mode 100644
>>> index 0000000..d07c241
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/ata/ahci-da850.txt
>>> @@ -0,0 +1,21 @@
>>> +Device tree binding for the TI DA850 AHCI SATA Controller
>>> +---------------------------------------------------------
>>> +
>>> +Required properties:
>>> +  - compatible: must be "ti,da850-ahci"
>>> +  - reg: physical base addresses and sizes of the controller's register
>>> areas
>>> +  - interrupts: interrupt specifier (refer to the interrupt binding)
>>> +
>>> +Optional properties:
>>> +  - clocks: clock specifier (refer to the common clock binding)
>>> +  - da850,clk_multiplier: the multiplier for the reference clock needed
>>> +                          for 1.5GHz PLL output
>>
>>
>> A clock multiplier property seems redundant if you are specifying a clock.
>> It should be possible to get the rate from the clock to determine which
>> multiplier is needed.
>>
> 
> I probably should have named it differently. This is not a multiplier
> of a clock derived from PLL0 or PLL1. Instead it's a value set by
> writing to the Port PHY Control Register (MPY bits) of the SATA
> controller that configures the multiplier for the external low-jitter
> clock. On the lcdk the signals (REFCLKP, REFCLKN) are provided by
> CDCM61001 (SATA OSCILLATOR component on the schematics).
> 
> I'll find a better name and comment the property accordingly.
> 
> FYI: the da850 platform does not use the common clock framework, so I
> don't specify the clock property on the sata node in the device tree.
> Instead I add the clock lookup entry in patch [01/10]. This is
> transparent for AHCI which can get the clock as usual by calling
> clk_get() in ahci_platform_get_resources().

I think David's point is that the SATA_REFCLK needs to be modeled as a
actual clock input to the IP. You should be able to get the rate using
clk_get_rate() and make the MPY bits calculation depending on the
incoming rate.

You should be able to model the clock even when not using common clock
framework.

DA850 AHCI does not use a con_id at the moment (it assumes a single
clock), and that needs to change.

Thanks,
Sekhar

[toc] | [prev] | [next] | [standalone]

#1559790 — Re: [PATCH 03/10] devicetree: bindings: add bindings for ahci-da850

2017-01-16 13:45 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
> On Monday 16 January 2017 03:43 PM, Bartosz Golaszewski wrote:
>> 2017-01-13 20:25 GMT+01:00 David Lechner <david@lechnology.com>:
>>>
>>> A clock multiplier property seems redundant if you are specifying a clock.
>>> It should be possible to get the rate from the clock to determine which
>>> multiplier is needed.
>>>
>>
>> I probably should have named it differently. This is not a multiplier
>> of a clock derived from PLL0 or PLL1. Instead it's a value set by
>> writing to the Port PHY Control Register (MPY bits) of the SATA
>> controller that configures the multiplier for the external low-jitter
>> clock. On the lcdk the signals (REFCLKP, REFCLKN) are provided by
>> CDCM61001 (SATA OSCILLATOR component on the schematics).
>>
>> I'll find a better name and comment the property accordingly.
>>
>> FYI: the da850 platform does not use the common clock framework, so I
>> don't specify the clock property on the sata node in the device tree.
>> Instead I add the clock lookup entry in patch [01/10]. This is
>> transparent for AHCI which can get the clock as usual by calling
>> clk_get() in ahci_platform_get_resources().
>
> I think David's point is that the SATA_REFCLK needs to be modeled as a
> actual clock input to the IP. You should be able to get the rate using
> clk_get_rate() and make the MPY bits calculation depending on the
> incoming rate.
>
> You should be able to model the clock even when not using common clock
> framework.
>
> DA850 AHCI does not use a con_id at the moment (it assumes a single
> clock), and that needs to change.
>

It's true that once davinci gets ported (is this planned?) to using
the common clock framework, we could just create a fixed-clock node in
da850-lcdk for the SATA oscillator, so the new property is redundant.

What I don't get is how should I model a clock that is not
configurable and is board-specific? Is hard-coding the relevant rate
in da850.c with a huge FIXME the right way?

Thanks,
Bartosz Golaszewski

[toc] | [prev] | [next] | [standalone]

#1559979 — Re: [PATCH 03/10] devicetree: bindings: add bindings for ahci-da850

On 01/16/2017 08:30 AM, Bartosz Golaszewski wrote:
> 2017-01-16 13:45 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
>> On Monday 16 January 2017 03:43 PM, Bartosz Golaszewski wrote:
>>> 2017-01-13 20:25 GMT+01:00 David Lechner <david@lechnology.com>:
>>>>
>>>> A clock multiplier property seems redundant if you are specifying a clock.
>>>> It should be possible to get the rate from the clock to determine which
>>>> multiplier is needed.
>>>>
>>>
>>> I probably should have named it differently. This is not a multiplier
>>> of a clock derived from PLL0 or PLL1. Instead it's a value set by
>>> writing to the Port PHY Control Register (MPY bits) of the SATA
>>> controller that configures the multiplier for the external low-jitter
>>> clock. On the lcdk the signals (REFCLKP, REFCLKN) are provided by
>>> CDCM61001 (SATA OSCILLATOR component on the schematics).
>>>
>>> I'll find a better name and comment the property accordingly.
>>>
>>> FYI: the da850 platform does not use the common clock framework, so I
>>> don't specify the clock property on the sata node in the device tree.
>>> Instead I add the clock lookup entry in patch [01/10]. This is
>>> transparent for AHCI which can get the clock as usual by calling
>>> clk_get() in ahci_platform_get_resources().
>>
>> I think David's point is that the SATA_REFCLK needs to be modeled as a
>> actual clock input to the IP. You should be able to get the rate using
>> clk_get_rate() and make the MPY bits calculation depending on the
>> incoming rate.
>>
>> You should be able to model the clock even when not using common clock
>> framework.
>>
>> DA850 AHCI does not use a con_id at the moment (it assumes a single
>> clock), and that needs to change.
>>
>
> It's true that once davinci gets ported (is this planned?) to using
> the common clock framework, we could just create a fixed-clock node in
> da850-lcdk for the SATA oscillator, so the new property is redundant.
>

I have some commits[1] where I started on converting da850 to use the 
common clock framework. But, I don't know anything about other davinci 
family devices, so I don't think I could really take that to completion 
without lots of help.

[1]: https://github.com/dlech/ev3dev-kernel/commits/wip-20160509

> What I don't get is how should I model a clock that is not
> configurable and is board-specific? Is hard-coding the relevant rate
> in da850.c with a huge FIXME the right way?

In arch/arm/mach-davinci/usb-da8xx.c, there is a "usb_refclkin" that is 
very similar to the situation with the sata refclk. You could do 
something like this to register the clock...

---

diff --git a/arch/arm/mach-davinci/devices-da8xx.c 
b/arch/arm/mach-davinci/devices-da8xx.c
index c2457b3..790efce9 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -1023,6 +1023,34 @@ int __init da8xx_register_spi_bus(int instance, 
unsigned num_chipselect)
  }

  #ifdef CONFIG_ARCH_DAVINCI_DA850
+
+static struct clk sata_refclkin = {
+       .name           = "sata_refclkin",
+       .set_rate       = davinci_simple_set_rate,
+};
+
+static struct clk_lookup sata_refclkin_lookup =
+       CLK(NULL, "sata_refclkin", &sata_refclkin);
+
+/**
+ * da8xx_register_sata_refclkin - register SATA_REFCLKIN clock
+ *
+ * @rate: The clock rate in Hz
+ */
+int __init da850_register_sata_refclkin(int rate)
+{
+       int ret;
+
+       sata_refclkin.rate = rate;
+       ret = clk_register(&sata_refclkin);
+       if (ret)
+               return ret;
+
+       clkdev_add(&sata_refclkin_lookup);
+
+       return 0;
+}
+
  static struct resource da850_sata_resources[] = {
         {
                 .start  = DA850_SATA_BASE,
@@ -1055,8 +1083,11 @@ static struct platform_device da850_sata_device = {

  int __init da850_register_sata(unsigned long refclkpn)
  {
-       /* please see comment in drivers/ata/ahci_da850.c */
-       BUG_ON(refclkpn != 100 * 1000 * 1000);
+       int err;
+
+       err = da850_register_sata_refclkin(refclkpn);
+       if (err)
+               return err;

         return platform_device_register(&da850_sata_device);
  }

---

Then to get things working from device tree, add this...

---

diff --git a/arch/arm/mach-davinci/da8xx-dt.c 
b/arch/arm/mach-davinci/da8xx-dt.c
index d2be194..b54bdd6 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -60,6 +60,14 @@ static void __init da850_init_machine(void)
                 pr_warn("%s: registering USB 1.1 PHY clock failed: %d",
                         __func__, ret);

+       if (of_machine_is_compatible("ti,da850-evm") ||
+           of_machine_is_compatible("ti,da850-lcdk")) {
+               ret = da850_register_sata_refclkin(100000000);
+               if (ret)
+                       pr_warn("%s: registering SATA_REFCLK clock 
failed: %d",
+                               __func__, ret);
+       }
+
         of_platform_default_populate(NULL, da850_auxdata_lookup, NULL);
         davinci_pm_init();
         pdata_quirks_init();

---

[toc] | [prev] | [next] | [standalone]

#1558379 — [PATCH 05/10] sata: ahci_da850: add device tree match table

We're using device tree for da850-lcdk. Add the match table to allow
to probe the driver.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/ata/ahci_da850.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index 267a3d3..5930af81 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -105,11 +105,18 @@ static int ahci_da850_probe(struct platform_device *pdev)
 static SIMPLE_DEV_PM_OPS(ahci_da850_pm_ops, ahci_platform_suspend,
 			 ahci_platform_resume);
 
+static const struct of_device_id ahci_da850_of_match[] = {
+	{ .compatible = "ti,da850-ahci", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ahci_da850_of_match);
+
 static struct platform_driver ahci_da850_driver = {
 	.probe = ahci_da850_probe,
 	.remove = ata_platform_remove_one,
 	.driver = {
 		.name = DRV_NAME,
+		.of_match_table = ahci_da850_of_match,
 		.pm = &ahci_da850_pm_ops,
 	},
 };
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1558382 — [PATCH 08/10] ARM: dts: da850: add pinmux settings for the SATA controller

Add pinmux sub-nodes for all muxed SATA pins.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/boot/dts/da850.dtsi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 104155d..1f6a47d 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -78,6 +78,30 @@
 					0x10 0x00220000 0x00ff0000
 				>;
 			};
+			sata_cp_det_pin: pinmux_sata_cp_det_pin {
+				pinctrl-single,bits = <
+					/* SATA_CP_DET */
+					0x0c 0x00000000 0xf0000000
+				>;
+			};
+			sata_mp_switch_pin: pinmux_sata_mp_switch_pin {
+				pinctrl-single,bits = <
+					/* SATA_MP_SWITCH */
+					0x0c 0x00000000 0x0f000000
+				>;
+			};
+			sata_cp_pod_pin: pinmux_sata_cp_pod_pin {
+				pinctrl-single,bits = <
+					/* SATA_CP_POD */
+					0x10 0x40000000 0xf0000000
+				>;
+			};
+			sata_led_pin: pinmux_sata_led_pin {
+				pinctrl-single,bits = <
+					/* SATA_LED */
+					0x10 0x04000000 0x0f000000
+				>;
+			};
 			i2c0_pins: pinmux_i2c0_pins {
 				pinctrl-single,bits = <
 					/* I2C0_SDA,I2C0_SCL */
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1558383 — [PATCH 07/10] sata: ahci_da850: add support for the da850,clk_multiplier DT property

Currently the clock multiplier is hardcoded in the driver for
the da850-evm board. Make it configurable over DT, but keep the
previous value as default in case the property is missing.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/ata/ahci_da850.c | 83 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 75 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index bb9eb4c..cd04caf 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -28,17 +28,70 @@
 #define SATA_PHY_TXSWING(x)	((x) << 19)
 #define SATA_PHY_ENPLL(x)	((x) << 31)
 
+struct da850_sata_mpy_mapping {
+	unsigned int multiplier;
+	unsigned int regval;
+};
+
+static const struct da850_sata_mpy_mapping da850_sata_mpy_table[] = {
+	{
+		.multiplier	= 5,
+		.regval		= 0x01,
+	},
+	{
+		.multiplier	= 6,
+		.regval		= 0x02,
+	},
+	{
+		.multiplier	= 8,
+		.regval		= 0x04,
+	},
+	{
+		.multiplier	= 10,
+		.regval		= 0x05,
+	},
+	{
+		.multiplier	= 12,
+		.regval		= 0x06,
+	},
+	/* TODO Add 12.5 multiplier. */
+	{
+		.multiplier	= 15,
+		.regval		= 0x08,
+	},
+	{
+		.multiplier	= 20,
+		.regval		= 0x09,
+	},
+	{
+		.multiplier	= 25,
+		.regval		= 0x0a,
+	}
+};
+
+static const struct da850_sata_mpy_mapping *
+da850_sata_get_mpy(unsigned int multiplier)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(da850_sata_mpy_table); i++)
+		if (da850_sata_mpy_table[i].multiplier == multiplier)
+			return &da850_sata_mpy_table[i];
+
+	return NULL;
+}
+
 /*
  * The multiplier needed for 1.5GHz PLL output.
  *
- * NOTE: This is currently hardcoded to be suitable for 100MHz crystal
- * frequency (which is used by DA850 EVM board) and may need to be changed
- * if you would like to use this driver on some other board.
+ * This is the default value suitable for the 100MHz crystal frequency
+ * used by DA850 EVM board, which doesn't use DT.
  */
-#define DA850_SATA_CLK_MULTIPLIER	7
+#define DA850_SATA_CLK_MULTIPLIER_DEFAULT	15
 
 static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg,
-			    void __iomem *ahci_base)
+			    void __iomem *ahci_base,
+			    const struct da850_sata_mpy_mapping *mpy)
 {
 	unsigned int val;
 
@@ -47,7 +100,7 @@ static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg,
 	val &= ~BIT(0);
 	writel(val, pwrdn_reg);
 
-	val = SATA_PHY_MPY(DA850_SATA_CLK_MULTIPLIER + 1) | SATA_PHY_LOS(1) |
+	val = SATA_PHY_MPY(mpy->regval) | SATA_PHY_LOS(1) |
 	      SATA_PHY_RXCDR(4) | SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) |
 	      SATA_PHY_ENPLL(1);
 
@@ -87,10 +140,12 @@ static struct scsi_host_template ahci_platform_sht = {
 
 static int ahci_da850_probe(struct platform_device *pdev)
 {
+	const struct da850_sata_mpy_mapping *mpy;
 	struct device *dev = &pdev->dev;
 	struct ahci_host_priv *hpriv;
-	struct resource *res;
+	unsigned int multiplier;
 	void __iomem *pwrdn_reg;
+	struct resource *res;
 	int rc;
 
 	hpriv = ahci_platform_get_resources(pdev);
@@ -109,7 +164,19 @@ static int ahci_da850_probe(struct platform_device *pdev)
 	if (!pwrdn_reg)
 		goto disable_resources;
 
-	da850_sata_init(dev, pwrdn_reg, hpriv->mmio);
+	rc = of_property_read_u32(dev->of_node,
+				  "da850,clk_multiplier", &multiplier);
+	if (rc)
+		multiplier = DA850_SATA_CLK_MULTIPLIER_DEFAULT;
+
+	mpy = da850_sata_get_mpy(multiplier);
+	if (!mpy) {
+		dev_err(dev, "invalid multiplier value: %u\n", multiplier);
+		rc = -EINVAL;
+		goto disable_resources;
+	}
+
+	da850_sata_init(dev, pwrdn_reg, hpriv->mmio, mpy);
 
 	rc = ahci_platform_init_host(pdev, hpriv, &ahci_da850_port_info,
 				     &ahci_platform_sht);
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1558741 — Re: [PATCH 07/10] sata: ahci_da850: add support for the da850,clk_multiplier DT property

On 01/13/2017 06:38 AM, Bartosz Golaszewski wrote:
> Currently the clock multiplier is hardcoded in the driver for
> the da850-evm board. Make it configurable over DT, but keep the
> previous value as default in case the property is missing.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/ata/ahci_da850.c | 83 +++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 75 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
> index bb9eb4c..cd04caf 100644
> --- a/drivers/ata/ahci_da850.c
> +++ b/drivers/ata/ahci_da850.c
> @@ -28,17 +28,70 @@
>  #define SATA_PHY_TXSWING(x)	((x) << 19)
>  #define SATA_PHY_ENPLL(x)	((x) << 31)
>
> +struct da850_sata_mpy_mapping {
> +	unsigned int multiplier;
> +	unsigned int regval;
> +};
> +
> +static const struct da850_sata_mpy_mapping da850_sata_mpy_table[] = {
> +	{
> +		.multiplier	= 5,
> +		.regval		= 0x01,
> +	},
> +	{
> +		.multiplier	= 6,
> +		.regval		= 0x02,
> +	},
> +	{
> +		.multiplier	= 8,
> +		.regval		= 0x04,
> +	},
> +	{
> +		.multiplier	= 10,
> +		.regval		= 0x05,
> +	},
> +	{
> +		.multiplier	= 12,
> +		.regval		= 0x06,
> +	},
> +	/* TODO Add 12.5 multiplier. */

Looks like you should be using an enum here for the multiplier field.

> +	{
> +		.multiplier	= 15,
> +		.regval		= 0x08,
> +	},
> +	{
> +		.multiplier	= 20,
> +		.regval		= 0x09,
> +	},
> +	{
> +		.multiplier	= 25,
> +		.regval		= 0x0a,
> +	}
> +};
> +
> +static const struct da850_sata_mpy_mapping *
> +da850_sata_get_mpy(unsigned int multiplier)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(da850_sata_mpy_table); i++)
> +		if (da850_sata_mpy_table[i].multiplier == multiplier)
> +			return &da850_sata_mpy_table[i];
> +
> +	return NULL;
> +}
> +
>  /*
>   * The multiplier needed for 1.5GHz PLL output.
>   *
> - * NOTE: This is currently hardcoded to be suitable for 100MHz crystal
> - * frequency (which is used by DA850 EVM board) and may need to be changed
> - * if you would like to use this driver on some other board.
> + * This is the default value suitable for the 100MHz crystal frequency
> + * used by DA850 EVM board, which doesn't use DT.

As I said in a reply on another patch, it seems like it would be better 
to use a clock that represents the crystal and use clk_get_rate() and 
calculate the multiplier from that.

For example, we have done something like this in 
usb20_phy_clk_set_parent() in arch/arm/mach-davinci/usb-da8xx.c.

>   */
> -#define DA850_SATA_CLK_MULTIPLIER	7
> +#define DA850_SATA_CLK_MULTIPLIER_DEFAULT	15
>
>  static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg,
> -			    void __iomem *ahci_base)
> +			    void __iomem *ahci_base,
> +			    const struct da850_sata_mpy_mapping *mpy)
>  {
>  	unsigned int val;
>
> @@ -47,7 +100,7 @@ static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg,
>  	val &= ~BIT(0);
>  	writel(val, pwrdn_reg);
>
> -	val = SATA_PHY_MPY(DA850_SATA_CLK_MULTIPLIER + 1) | SATA_PHY_LOS(1) |
> +	val = SATA_PHY_MPY(mpy->regval) | SATA_PHY_LOS(1) |
>  	      SATA_PHY_RXCDR(4) | SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) |
>  	      SATA_PHY_ENPLL(1);
>
> @@ -87,10 +140,12 @@ static struct scsi_host_template ahci_platform_sht = {
>
>  static int ahci_da850_probe(struct platform_device *pdev)
>  {
> +	const struct da850_sata_mpy_mapping *mpy;
>  	struct device *dev = &pdev->dev;
>  	struct ahci_host_priv *hpriv;
> -	struct resource *res;
> +	unsigned int multiplier;
>  	void __iomem *pwrdn_reg;
> +	struct resource *res;
>  	int rc;
>
>  	hpriv = ahci_platform_get_resources(pdev);
> @@ -109,7 +164,19 @@ static int ahci_da850_probe(struct platform_device *pdev)
>  	if (!pwrdn_reg)
>  		goto disable_resources;
>
> -	da850_sata_init(dev, pwrdn_reg, hpriv->mmio);
> +	rc = of_property_read_u32(dev->of_node,
> +				  "da850,clk_multiplier", &multiplier);
> +	if (rc)
> +		multiplier = DA850_SATA_CLK_MULTIPLIER_DEFAULT;
> +
> +	mpy = da850_sata_get_mpy(multiplier);
> +	if (!mpy) {
> +		dev_err(dev, "invalid multiplier value: %u\n", multiplier);
> +		rc = -EINVAL;
> +		goto disable_resources;
> +	}
> +
> +	da850_sata_init(dev, pwrdn_reg, hpriv->mmio, mpy);
>
>  	rc = ahci_platform_init_host(pdev, hpriv, &ahci_da850_port_info,
>  				     &ahci_platform_sht);
>

[toc] | [prev] | [next] | [standalone]

#1558384 — [PATCH 04/10] sata: hardreset: retry if phys link is down

The sata core driver already retries to resume the link because some
controllers ignore writes to the SControl register.

We have a use case with the da850 SATA controller where at PLL0
frequency of 456MHz (needed to properly service the LCD controller)
the chip becomes unstable and the hardreset operation is ignored the
first time 50% of times.

Retrying just the resume operation doesn't work - we need to issue
the phy/wake reset again to make it work.

If ata_phys_link_offline() returns true in sata_link_hardreset(),
retry a couple times before really giving up.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/ata/libata-core.c | 16 ++++++++++++----
 include/linux/libata.h    |  4 +++-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 9cd0a2d..3b848a3 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3985,8 +3985,8 @@ int sata_link_hardreset(struct ata_link *link, const unsigned long *timing,
 			unsigned long deadline,
 			bool *online, int (*check_ready)(struct ata_link *))
 {
+	int rc, retry = ATA_LINK_RESET_TRIES;
 	u32 scontrol;
-	int rc;
 
 	DPRINTK("ENTER\n");
 
@@ -4009,7 +4009,7 @@ int sata_link_hardreset(struct ata_link *link, const unsigned long *timing,
 
 		sata_set_spd(link);
 	}
-
+retry:
 	/* issue phy wake/reset */
 	if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol)))
 		goto out;
@@ -4028,9 +4028,17 @@ int sata_link_hardreset(struct ata_link *link, const unsigned long *timing,
 	rc = sata_link_resume(link, timing, deadline);
 	if (rc)
 		goto out;
-	/* if link is offline nothing more to do */
-	if (ata_phys_link_offline(link))
+
+	if (ata_phys_link_offline(link)) {
+		if (retry--) {
+			ata_link_warn(link,
+				      "link still offline after hardreset - retrying\n");
+			goto retry;
+		}
+
+		/* if link is still offline nothing more to do */
 		goto out;
+	}
 
 	/* Link is online.  From this point, -ENODEV too is an error. */
 	if (online)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index c170be5..2c840c0 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -392,8 +392,10 @@ enum {
 	/* max tries if error condition is still set after ->error_handler */
 	ATA_EH_MAX_TRIES	= 5,
 
-	/* sometimes resuming a link requires several retries */
+	/* sometimes resuming a link requires several retries... */
 	ATA_LINK_RESUME_TRIES	= 5,
+	/* ... and sometimes we need to retry the whole reset procedure */
+	ATA_LINK_RESET_TRIES	= 5,
 
 	/* how hard are we gonna try to probe/recover devices */
 	ATA_PROBE_MAX_TRIES	= 3,
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1559362 — Re: [PATCH 04/10] sata: hardreset: retry if phys link is down

Hello,

On Fri, Jan 13, 2017 at 01:37:58PM +0100, Bartosz Golaszewski wrote:
> The sata core driver already retries to resume the link because some
> controllers ignore writes to the SControl register.
> 
> We have a use case with the da850 SATA controller where at PLL0
> frequency of 456MHz (needed to properly service the LCD controller)
> the chip becomes unstable and the hardreset operation is ignored the
> first time 50% of times.
> 
> Retrying just the resume operation doesn't work - we need to issue
> the phy/wake reset again to make it work.
> 
> If ata_phys_link_offline() returns true in sata_link_hardreset(),
> retry a couple times before really giving up.

I think it'd be better to implement the driver specific implementation
rather than changing the behavior for everybody.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web