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

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

[Bartosz Golaszewski]

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

The first patch adds DT bindings for the ahci-da850 driver.

The second enables relevant modules in davinci_all_defconfig.

Patches 03/14-06/14 modify the way the clocks are handled regarding
SATA on the da850 platform. We modify the ahci driver to retrieve
the clock via con_id and model the external SATA oscillator as
a real clock.

Patches 07/14-11/14 extend the ahci-da850 driver. Add DT support,
implement workarounds necessary to make SATA work on the da850-lcdk
board and un-hardcode the external clock multiplier.

Last three patches add device tree changes required to probe the
driver.

v1 -> v2:
- dropped patch 04/10 - replaced with local changes in the
  ahci-da850 driver
- added comments explaining the workaround in ahci softreset
- s/0x218000/218000 in the sata DT node label
- added patches chaning the way clocks are handled in the da850 SATA
  code both in arch/ and in the ahci driver
- dropped the clock multiplier property in the DT bindings in favor
  of using struct clk to pass the refclk rate to the driver
- minor tweaks in commit messages

Bartosz Golaszewski (14):
  devicetree: bindings: add bindings for ahci-da850
  ARM: davinci_all_defconfig: enable SATA modules
  ARM: davinci: add a clock lookup entry for the SATA clock
  sata: ahci-da850: get the sata clock using a connector id
  ARM: davinci: da850: add con_id for the SATA clock
  ARM: davinci: da850: model the SATA refclk
  sata: ahci-da850: add device tree match table
  sata: ahci-da850: implement a workaround for the softreset quirk
  sata: ahci: export ahci_do_hardreset() locally
  sata: ahci-da850: add a workaround for controller instability
  sata: ahci-da850: un-hardcode the MPY bits
  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         |  18 +++
 arch/arm/boot/dts/da850-lcdk.dts                   |   4 +
 arch/arm/boot/dts/da850.dtsi                       |  30 ++++
 arch/arm/configs/davinci_all_defconfig             |   2 +
 arch/arm/mach-davinci/da850.c                      |   2 +-
 arch/arm/mach-davinci/da8xx-dt.c                   |   9 ++
 arch/arm/mach-davinci/devices-da8xx.c              |  23 +++
 arch/arm/mach-davinci/include/mach/da8xx.h         |   1 +
 drivers/ata/ahci.h                                 |   3 +
 drivers/ata/ahci_da850.c                           | 172 +++++++++++++++++++--
 drivers/ata/libahci.c                              |  18 ++-
 11 files changed, 262 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-da850.txt

-- 
2.9.3

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

[Bartosz Golaszewski]

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

The first patch adds DT bindings for the ahci-da850 driver.

The second enables relevant modules in davinci_all_defconfig.

Patches 03/14-06/14 modify the way the clocks are handled regarding
SATA on the da850 platform. We modify the ahci driver to retrieve
the clock via con_id and model the external SATA oscillator as
a real clock.

Patches 07/14-11/14 extend the ahci-da850 driver. Add DT support,
implement workarounds necessary to make SATA work on the da850-lcdk
board and un-hardcode the external clock multiplier.

Last three patches add device tree changes required to probe the
driver.

v1 -> v2:
- dropped patch 04/10 - replaced with local changes in the
  ahci-da850 driver
- added comments explaining the workaround in ahci softreset
- s/0x218000/218000 in the sata DT node label
- added patches chaning the way clocks are handled in the da850 SATA
  code both in arch/ and in the ahci driver
- dropped the clock multiplier property in the DT bindings in favor
  of using struct clk to pass the refclk rate to the driver
- minor tweaks in commit messages

Bartosz Golaszewski (14):
  devicetree: bindings: add bindings for ahci-da850
  ARM: davinci_all_defconfig: enable SATA modules
  ARM: davinci: add a clock lookup entry for the SATA clock
  sata: ahci-da850: get the sata clock using a connector id
  ARM: davinci: da850: add con_id for the SATA clock
  ARM: davinci: da850: model the SATA refclk
  sata: ahci-da850: add device tree match table
  sata: ahci-da850: implement a workaround for the softreset quirk
  sata: ahci: export ahci_do_hardreset() locally
  sata: ahci-da850: add a workaround for controller instability
  sata: ahci-da850: un-hardcode the MPY bits
  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         |  18 +++
 arch/arm/boot/dts/da850-lcdk.dts                   |   4 +
 arch/arm/boot/dts/da850.dtsi                       |  30 ++++
 arch/arm/configs/davinci_all_defconfig             |   2 +
 arch/arm/mach-davinci/da850.c                      |   2 +-
 arch/arm/mach-davinci/da8xx-dt.c                   |   9 ++
 arch/arm/mach-davinci/devices-da8xx.c              |  23 +++
 arch/arm/mach-davinci/include/mach/da8xx.h         |   1 +
 drivers/ata/ahci.h                                 |   3 +
 drivers/ata/ahci_da850.c                           | 172 +++++++++++++++++++--
 drivers/ata/libahci.c                              |  18 ++-
 11 files changed, 262 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-da850.txt

-- 
2.9.3

[PATCH v2 01/14] devicetree: bindings: add bindings for ahci-da850

[Bartosz Golaszewski]

Add DT bindings for the TI DA850 AHCI SATA controller.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 Documentation/devicetree/bindings/ata/ahci-da850.txt | 18 ++++++++++++++++++
 1 file changed, 18 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..e7111b4
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-da850.txt
@@ -0,0 +1,18 @@
+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)
+
+Example:
+
+	sata: ahci@218000 {
+		compatible = "ti,da850-ahci";
+		reg = <0x218000 0x2000>, <0x22c018 0x4>;
+		interrupts = <67>;
+	};
-- 
2.9.3

[PATCH v2 01/14] devicetree: bindings: add bindings for ahci-da850

[Bartosz Golaszewski]

Add DT bindings for the TI DA850 AHCI SATA controller.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 Documentation/devicetree/bindings/ata/ahci-da850.txt | 18 ++++++++++++++++++
 1 file changed, 18 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..e7111b4
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-da850.txt
@@ -0,0 +1,18 @@
+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)
+
+Example:
+
+	sata: ahci at 218000 {
+		compatible = "ti,da850-ahci";
+		reg = <0x218000 0x2000>, <0x22c018 0x4>;
+		interrupts = <67>;
+	};
-- 
2.9.3

Re: [PATCH v2 01/14] devicetree: bindings: add bindings for ahci-da850

[David Lechner]

On 01/17/2017 06:26 AM, Bartosz Golaszewski wrote:
> Add DT bindings for the TI DA850 AHCI SATA controller.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  Documentation/devicetree/bindings/ata/ahci-da850.txt | 18 ++++++++++++++++++
>  1 file changed, 18 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..e7111b4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-da850.txt
> @@ -0,0 +1,18 @@
> +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)

Won't you also need a clock-names property since there are two clocks?

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

It would be nice to have clocks and clock-names in the example as well.

[PATCH v2 01/14] devicetree: bindings: add bindings for ahci-da850

[David Lechner]

On 01/17/2017 06:26 AM, Bartosz Golaszewski wrote:
> Add DT bindings for the TI DA850 AHCI SATA controller.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  Documentation/devicetree/bindings/ata/ahci-da850.txt | 18 ++++++++++++++++++
>  1 file changed, 18 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..e7111b4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-da850.txt
> @@ -0,0 +1,18 @@
> +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)

Won't you also need a clock-names property since there are two clocks?

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

It would be nice to have clocks and clock-names in the example as well.

Re: [PATCH v2 01/14] devicetree: bindings: add bindings for ahci-da850

[Sekhar Nori]

On Wednesday 18 January 2017 12:05 AM, David Lechner wrote:
> On 01/17/2017 06:26 AM, Bartosz Golaszewski wrote:
>> Add DT bindings for the TI DA850 AHCI SATA controller.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>>  Documentation/devicetree/bindings/ata/ahci-da850.txt | 18
>> ++++++++++++++++++
>>  1 file changed, 18 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..e7111b4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/ata/ahci-da850.txt
>> @@ -0,0 +1,18 @@
>> +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)
> 
> Won't you also need a clock-names property since there are two clocks?
> 
>> +
>> +Example:
>> +
>> +    sata: ahci@218000 {
>> +        compatible = "ti,da850-ahci";
>> +        reg = <0x218000 0x2000>, <0x22c018 0x4>;
>> +        interrupts = <67>;
>> +    };
>>
> 
> It would be nice to have clocks and clock-names in the example as well.

Well, we don't use the common clock binding today. Is it really
necessary to add them now. Thats basically going to remain completely
untested.

I would drop the mention of clocks specification as an optional property
too.

Thanks,
Sekhar

[PATCH v2 01/14] devicetree: bindings: add bindings for ahci-da850

[Sekhar Nori]

On Wednesday 18 January 2017 12:05 AM, David Lechner wrote:
> On 01/17/2017 06:26 AM, Bartosz Golaszewski wrote:
>> Add DT bindings for the TI DA850 AHCI SATA controller.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>>  Documentation/devicetree/bindings/ata/ahci-da850.txt | 18
>> ++++++++++++++++++
>>  1 file changed, 18 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..e7111b4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/ata/ahci-da850.txt
>> @@ -0,0 +1,18 @@
>> +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)
> 
> Won't you also need a clock-names property since there are two clocks?
> 
>> +
>> +Example:
>> +
>> +    sata: ahci at 218000 {
>> +        compatible = "ti,da850-ahci";
>> +        reg = <0x218000 0x2000>, <0x22c018 0x4>;
>> +        interrupts = <67>;
>> +    };
>>
> 
> It would be nice to have clocks and clock-names in the example as well.

Well, we don't use the common clock binding today. Is it really
necessary to add them now. Thats basically going to remain completely
untested.

I would drop the mention of clocks specification as an optional property
too.

Thanks,
Sekhar

Re: [PATCH v2 01/14] devicetree: bindings: add bindings for ahci-da850

[Sekhar Nori]

On Wednesday 18 January 2017 12:05 AM, David Lechner wrote:
> On 01/17/2017 06:26 AM, Bartosz Golaszewski wrote:
>> Add DT bindings for the TI DA850 AHCI SATA controller.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>>  Documentation/devicetree/bindings/ata/ahci-da850.txt | 18
>> ++++++++++++++++++
>>  1 file changed, 18 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..e7111b4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/ata/ahci-da850.txt
>> @@ -0,0 +1,18 @@
>> +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)
> 
> Won't you also need a clock-names property since there are two clocks?
> 
>> +
>> +Example:
>> +
>> +    sata: ahci@218000 {
>> +        compatible = "ti,da850-ahci";
>> +        reg = <0x218000 0x2000>, <0x22c018 0x4>;
>> +        interrupts = <67>;
>> +    };
>>
> 
> It would be nice to have clocks and clock-names in the example as well.

Well, we don't use the common clock binding today. Is it really
necessary to add them now. Thats basically going to remain completely
untested.

I would drop the mention of clocks specification as an optional property
too.

Thanks,
Sekhar

[PATCH v2 02/14] ARM: davinci_all_defconfig: enable SATA modules

[Bartosz Golaszewski]

Add the da850-ahci driver to davinci defconfig.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/configs/davinci_all_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
index 8806754..a1b9c58 100644
--- a/arch/arm/configs/davinci_all_defconfig
+++ b/arch/arm/configs/davinci_all_defconfig
@@ -78,6 +78,8 @@ CONFIG_IDE=m
 CONFIG_BLK_DEV_PALMCHIP_BK3710=m
 CONFIG_SCSI=m
 CONFIG_BLK_DEV_SD=m
+CONFIG_ATA=m
+CONFIG_AHCI_DA850=m
 CONFIG_NETDEVICES=y
 CONFIG_NETCONSOLE=y
 CONFIG_TUN=m
-- 
2.9.3

[PATCH v2 02/14] ARM: davinci_all_defconfig: enable SATA modules

[Bartosz Golaszewski]

Add the da850-ahci driver to davinci defconfig.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/configs/davinci_all_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
index 8806754..a1b9c58 100644
--- a/arch/arm/configs/davinci_all_defconfig
+++ b/arch/arm/configs/davinci_all_defconfig
@@ -78,6 +78,8 @@ CONFIG_IDE=m
 CONFIG_BLK_DEV_PALMCHIP_BK3710=m
 CONFIG_SCSI=m
 CONFIG_BLK_DEV_SD=m
+CONFIG_ATA=m
+CONFIG_AHCI_DA850=m
 CONFIG_NETDEVICES=y
 CONFIG_NETCONSOLE=y
 CONFIG_TUN=m
-- 
2.9.3

[PATCH v2 03/14] ARM: davinci: add a clock lookup entry for the SATA clock

[Bartosz Golaszewski]

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

[PATCH v2 03/14] ARM: davinci: add a clock lookup entry for the SATA clock

[Bartosz Golaszewski]

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

[PATCH v2 04/14] sata: ahci-da850: get the sata clock using a connector id

[Bartosz Golaszewski]

In preparation for using two clocks in the driver (the sysclk2-based
clock and the external REFCLK), check if we got a functional clock
after calling ahci_platform_get_resources(). If not, retry calling
get_clk() with con_id specified.

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

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index 267a3d3..18f57c2 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -71,12 +71,28 @@ static int ahci_da850_probe(struct platform_device *pdev)
 	struct ahci_host_priv *hpriv;
 	struct resource *res;
 	void __iomem *pwrdn_reg;
+	struct clk *clk;
 	int rc;
 
 	hpriv = ahci_platform_get_resources(pdev);
 	if (IS_ERR(hpriv))
 		return PTR_ERR(hpriv);
 
+	/*
+	 * Internally ahci_platform_get_resources() calls clk_get(dev, NULL)
+	 * when trying to obtain the first clock. This SATA controller uses
+	 * two clocks for which we specify two connector ids. If we don't
+	 * have a clock at this point - call clk_get() again with
+	 * con_id = "sata".
+	 */
+	if (!hpriv->clks[0]) {
+		clk = clk_get(dev, "sata");
+		if (IS_ERR(clk))
+			return PTR_ERR(clk);
+
+		hpriv->clks[0] = clk;
+	}
+
 	rc = ahci_platform_enable_resources(hpriv);
 	if (rc)
 		return rc;
-- 
2.9.3

[PATCH v2 04/14] sata: ahci-da850: get the sata clock using a connector id

[Bartosz Golaszewski]

In preparation for using two clocks in the driver (the sysclk2-based
clock and the external REFCLK), check if we got a functional clock
after calling ahci_platform_get_resources(). If not, retry calling
get_clk() with con_id specified.

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

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index 267a3d3..18f57c2 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -71,12 +71,28 @@ static int ahci_da850_probe(struct platform_device *pdev)
 	struct ahci_host_priv *hpriv;
 	struct resource *res;
 	void __iomem *pwrdn_reg;
+	struct clk *clk;
 	int rc;
 
 	hpriv = ahci_platform_get_resources(pdev);
 	if (IS_ERR(hpriv))
 		return PTR_ERR(hpriv);
 
+	/*
+	 * Internally ahci_platform_get_resources() calls clk_get(dev, NULL)
+	 * when trying to obtain the first clock. This SATA controller uses
+	 * two clocks for which we specify two connector ids. If we don't
+	 * have a clock at this point - call clk_get() again with
+	 * con_id = "sata".
+	 */
+	if (!hpriv->clks[0]) {
+		clk = clk_get(dev, "sata");
+		if (IS_ERR(clk))
+			return PTR_ERR(clk);
+
+		hpriv->clks[0] = clk;
+	}
+
 	rc = ahci_platform_enable_resources(hpriv);
 	if (rc)
 		return rc;
-- 
2.9.3

[PATCH v2 04/14] sata: ahci-da850: get the sata clock using a connector id

[Bartosz Golaszewski]

In preparation for using two clocks in the driver (the sysclk2-based
clock and the external REFCLK), check if we got a functional clock
after calling ahci_platform_get_resources(). If not, retry calling
get_clk() with con_id specified.

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

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index 267a3d3..18f57c2 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -71,12 +71,28 @@ static int ahci_da850_probe(struct platform_device *pdev)
 	struct ahci_host_priv *hpriv;
 	struct resource *res;
 	void __iomem *pwrdn_reg;
+	struct clk *clk;
 	int rc;
 
 	hpriv = ahci_platform_get_resources(pdev);
 	if (IS_ERR(hpriv))
 		return PTR_ERR(hpriv);
 
+	/*
+	 * Internally ahci_platform_get_resources() calls clk_get(dev, NULL)
+	 * when trying to obtain the first clock. This SATA controller uses
+	 * two clocks for which we specify two connector ids. If we don't
+	 * have a clock at this point - call clk_get() again with
+	 * con_id = "sata".
+	 */
+	if (!hpriv->clks[0]) {
+		clk = clk_get(dev, "sata");
+		if (IS_ERR(clk))
+			return PTR_ERR(clk);
+
+		hpriv->clks[0] = clk;
+	}
+
 	rc = ahci_platform_enable_resources(hpriv);
 	if (rc)
 		return rc;
-- 
2.9.3

Re: [PATCH v2 04/14] sata: ahci-da850: get the sata clock using a connector id

[Sergei Shtylyov]

Hello!

On 01/17/2017 03:26 PM, Bartosz Golaszewski wrote:

> In preparation for using two clocks in the driver (the sysclk2-based
> clock and the external REFCLK), check if we got a functional clock
> after calling ahci_platform_get_resources(). If not, retry calling
> get_clk() with con_id specified.

    clk_get().

> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/ata/ahci_da850.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
> index 267a3d3..18f57c2 100644
> --- a/drivers/ata/ahci_da850.c
> +++ b/drivers/ata/ahci_da850.c
> @@ -71,12 +71,28 @@ static int ahci_da850_probe(struct platform_device *pdev)
>  	struct ahci_host_priv *hpriv;
>  	struct resource *res;
>  	void __iomem *pwrdn_reg;
> +	struct clk *clk;
>  	int rc;
>
>  	hpriv = ahci_platform_get_resources(pdev);
>  	if (IS_ERR(hpriv))
>  		return PTR_ERR(hpriv);
>
> +	/*
> +	 * Internally ahci_platform_get_resources() calls clk_get(dev, NULL)
> +	 * when trying to obtain the first clock. This SATA controller uses
> +	 * two clocks for which we specify two connector ids. If we don't

    It's called connection ID, IIRC.

[...]

MBR, Sergei

[PATCH v2 04/14] sata: ahci-da850: get the sata clock using a connector id

[Sergei Shtylyov]

Hello!

On 01/17/2017 03:26 PM, Bartosz Golaszewski wrote:

> In preparation for using two clocks in the driver (the sysclk2-based
> clock and the external REFCLK), check if we got a functional clock
> after calling ahci_platform_get_resources(). If not, retry calling
> get_clk() with con_id specified.

    clk_get().

> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/ata/ahci_da850.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
> index 267a3d3..18f57c2 100644
> --- a/drivers/ata/ahci_da850.c
> +++ b/drivers/ata/ahci_da850.c
> @@ -71,12 +71,28 @@ static int ahci_da850_probe(struct platform_device *pdev)
>  	struct ahci_host_priv *hpriv;
>  	struct resource *res;
>  	void __iomem *pwrdn_reg;
> +	struct clk *clk;
>  	int rc;
>
>  	hpriv = ahci_platform_get_resources(pdev);
>  	if (IS_ERR(hpriv))
>  		return PTR_ERR(hpriv);
>
> +	/*
> +	 * Internally ahci_platform_get_resources() calls clk_get(dev, NULL)
> +	 * when trying to obtain the first clock. This SATA controller uses
> +	 * two clocks for which we specify two connector ids. If we don't

    It's called connection ID, IIRC.

[...]

MBR, Sergei

[PATCH v2 05/14] ARM: davinci: da850: add con_id for the SATA clock

[Bartosz Golaszewski]

The ahci-da850 SATA driver is now capable of retrieving clocks by
con_id. Add the connector id for the sysclk2-derived SATA clock.

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

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 1d873d1..dbf1daa 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -571,7 +571,7 @@ static struct clk_lookup da850_clks[] = {
 	CLK("spi_davinci.0",	NULL,		&spi0_clk),
 	CLK("spi_davinci.1",	NULL,		&spi1_clk),
 	CLK("vpif",		NULL,		&vpif_clk),
-	CLK("ahci_da850",		NULL,		&sata_clk),
+	CLK("ahci_da850",	"sata",		&sata_clk),
 	CLK("davinci-rproc.0",	NULL,		&dsp_clk),
 	CLK(NULL,		NULL,		&ehrpwm_clk),
 	CLK("ehrpwm.0",		"fck",		&ehrpwm0_clk),
-- 
2.9.3

[PATCH v2 05/14] ARM: davinci: da850: add con_id for the SATA clock

[Bartosz Golaszewski]

The ahci-da850 SATA driver is now capable of retrieving clocks by
con_id. Add the connector id for the sysclk2-derived SATA clock.

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

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 1d873d1..dbf1daa 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -571,7 +571,7 @@ static struct clk_lookup da850_clks[] = {
 	CLK("spi_davinci.0",	NULL,		&spi0_clk),
 	CLK("spi_davinci.1",	NULL,		&spi1_clk),
 	CLK("vpif",		NULL,		&vpif_clk),
-	CLK("ahci_da850",		NULL,		&sata_clk),
+	CLK("ahci_da850",	"sata",		&sata_clk),
 	CLK("davinci-rproc.0",	NULL,		&dsp_clk),
 	CLK(NULL,		NULL,		&ehrpwm_clk),
 	CLK("ehrpwm.0",		"fck",		&ehrpwm0_clk),
-- 
2.9.3

[PATCH v2 06/14] ARM: davinci: da850: model the SATA refclk

[Bartosz Golaszewski]

Register a dummy clock modelling the external SATA oscillator for
da850 DT mode. For non-DT boot we don't register the clock - instead
we rely on the default MPY value defined in the da850 ahci driver (as
is done currently).

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/mach-davinci/da8xx-dt.c           |  8 ++++++++
 arch/arm/mach-davinci/devices-da8xx.c      | 23 +++++++++++++++++++++++
 arch/arm/mach-davinci/include/mach/da8xx.h |  1 +
 3 files changed, 32 insertions(+)

diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index b83e5d1..13137cb 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -61,6 +61,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_refclk(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();
 }
diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index c2457b3..2bb5b69 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -24,6 +24,7 @@
 #include <mach/common.h>
 #include <mach/time.h>
 #include <mach/da8xx.h>
+#include <mach/clock.h>
 #include "cpuidle.h"
 #include "sram.h"
 
@@ -1023,6 +1024,28 @@ int __init da8xx_register_spi_bus(int instance, unsigned num_chipselect)
 }
 
 #ifdef CONFIG_ARCH_DAVINCI_DA850
+static struct clk sata_refclk = {
+	.name		= "sata_refclk",
+	.set_rate	= davinci_simple_set_rate,
+};
+
+static struct clk_lookup sata_refclk_lookup =
+		CLK("ahci_da850", "refclk", &sata_refclk);
+
+int __init da850_register_sata_refclk(int rate)
+{
+	int ret;
+
+	sata_refclk.rate = rate;
+	ret = clk_register(&sata_refclk);
+	if (ret)
+		return ret;
+
+	clkdev_add(&sata_refclk_lookup);
+
+	return 0;
+}
+
 static struct resource da850_sata_resources[] = {
 	{
 		.start	= DA850_SATA_BASE,
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index 85ff218..7e46422 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -95,6 +95,7 @@ int da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata);
 int da8xx_register_usb_refclkin(int rate);
 int da8xx_register_usb20_phy_clk(bool use_usb_refclkin);
 int da8xx_register_usb11_phy_clk(bool use_usb_refclkin);
+int da850_register_sata_refclk(int rate);
 int da8xx_register_emac(void);
 int da8xx_register_uio_pruss(void);
 int da8xx_register_lcdc(struct da8xx_lcdc_platform_data *pdata);
-- 
2.9.3

[PATCH v2 06/14] ARM: davinci: da850: model the SATA refclk

[Bartosz Golaszewski]

Register a dummy clock modelling the external SATA oscillator for
da850 DT mode. For non-DT boot we don't register the clock - instead
we rely on the default MPY value defined in the da850 ahci driver (as
is done currently).

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/mach-davinci/da8xx-dt.c           |  8 ++++++++
 arch/arm/mach-davinci/devices-da8xx.c      | 23 +++++++++++++++++++++++
 arch/arm/mach-davinci/include/mach/da8xx.h |  1 +
 3 files changed, 32 insertions(+)

diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index b83e5d1..13137cb 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -61,6 +61,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_refclk(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();
 }
diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index c2457b3..2bb5b69 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -24,6 +24,7 @@
 #include <mach/common.h>
 #include <mach/time.h>
 #include <mach/da8xx.h>
+#include <mach/clock.h>
 #include "cpuidle.h"
 #include "sram.h"
 
@@ -1023,6 +1024,28 @@ int __init da8xx_register_spi_bus(int instance, unsigned num_chipselect)
 }
 
 #ifdef CONFIG_ARCH_DAVINCI_DA850
+static struct clk sata_refclk = {
+	.name		= "sata_refclk",
+	.set_rate	= davinci_simple_set_rate,
+};
+
+static struct clk_lookup sata_refclk_lookup =
+		CLK("ahci_da850", "refclk", &sata_refclk);
+
+int __init da850_register_sata_refclk(int rate)
+{
+	int ret;
+
+	sata_refclk.rate = rate;
+	ret = clk_register(&sata_refclk);
+	if (ret)
+		return ret;
+
+	clkdev_add(&sata_refclk_lookup);
+
+	return 0;
+}
+
 static struct resource da850_sata_resources[] = {
 	{
 		.start	= DA850_SATA_BASE,
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index 85ff218..7e46422 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -95,6 +95,7 @@ int da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata);
 int da8xx_register_usb_refclkin(int rate);
 int da8xx_register_usb20_phy_clk(bool use_usb_refclkin);
 int da8xx_register_usb11_phy_clk(bool use_usb_refclkin);
+int da850_register_sata_refclk(int rate);
 int da8xx_register_emac(void);
 int da8xx_register_uio_pruss(void);
 int da8xx_register_lcdc(struct da8xx_lcdc_platform_data *pdata);
-- 
2.9.3

[PATCH v2 06/14] ARM: davinci: da850: model the SATA refclk

[Bartosz Golaszewski]

Register a dummy clock modelling the external SATA oscillator for
da850 DT mode. For non-DT boot we don't register the clock - instead
we rely on the default MPY value defined in the da850 ahci driver (as
is done currently).

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/mach-davinci/da8xx-dt.c           |  8 ++++++++
 arch/arm/mach-davinci/devices-da8xx.c      | 23 +++++++++++++++++++++++
 arch/arm/mach-davinci/include/mach/da8xx.h |  1 +
 3 files changed, 32 insertions(+)

diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index b83e5d1..13137cb 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -61,6 +61,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_refclk(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();
 }
diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index c2457b3..2bb5b69 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -24,6 +24,7 @@
 #include <mach/common.h>
 #include <mach/time.h>
 #include <mach/da8xx.h>
+#include <mach/clock.h>
 #include "cpuidle.h"
 #include "sram.h"
 
@@ -1023,6 +1024,28 @@ int __init da8xx_register_spi_bus(int instance, unsigned num_chipselect)
 }
 
 #ifdef CONFIG_ARCH_DAVINCI_DA850
+static struct clk sata_refclk = {
+	.name		= "sata_refclk",
+	.set_rate	= davinci_simple_set_rate,
+};
+
+static struct clk_lookup sata_refclk_lookup =
+		CLK("ahci_da850", "refclk", &sata_refclk);
+
+int __init da850_register_sata_refclk(int rate)
+{
+	int ret;
+
+	sata_refclk.rate = rate;
+	ret = clk_register(&sata_refclk);
+	if (ret)
+		return ret;
+
+	clkdev_add(&sata_refclk_lookup);
+
+	return 0;
+}
+
 static struct resource da850_sata_resources[] = {
 	{
 		.start	= DA850_SATA_BASE,
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index 85ff218..7e46422 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -95,6 +95,7 @@ int da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata);
 int da8xx_register_usb_refclkin(int rate);
 int da8xx_register_usb20_phy_clk(bool use_usb_refclkin);
 int da8xx_register_usb11_phy_clk(bool use_usb_refclkin);
+int da850_register_sata_refclk(int rate);
 int da8xx_register_emac(void);
 int da8xx_register_uio_pruss(void);
 int da8xx_register_lcdc(struct da8xx_lcdc_platform_data *pdata);
-- 
2.9.3

Re: [PATCH v2 06/14] ARM: davinci: da850: model the SATA refclk

[David Lechner]

On 01/17/2017 06:26 AM, Bartosz Golaszewski wrote:
> Register a dummy clock modelling the external SATA oscillator for
> da850 DT mode. For non-DT boot we don't register the clock - instead
> we rely on the default MPY value defined in the da850 ahci driver (as
> is done currently).

Why not register a clock for non-DT boot as well?

[PATCH v2 06/14] ARM: davinci: da850: model the SATA refclk

[David Lechner]

On 01/17/2017 06:26 AM, Bartosz Golaszewski wrote:
> Register a dummy clock modelling the external SATA oscillator for
> da850 DT mode. For non-DT boot we don't register the clock - instead
> we rely on the default MPY value defined in the da850 ahci driver (as
> is done currently).

Why not register a clock for non-DT boot as well?

Re: [PATCH v2 06/14] ARM: davinci: da850: model the SATA refclk

[David Lechner]

On 01/17/2017 06:26 AM, Bartosz Golaszewski wrote:
> Register a dummy clock modelling the external SATA oscillator for
> da850 DT mode. For non-DT boot we don't register the clock - instead
> we rely on the default MPY value defined in the da850 ahci driver (as
> is done currently).

Why not register a clock for non-DT boot as well?

Re: [PATCH v2 06/14] ARM: davinci: da850: model the SATA refclk

[Sekhar Nori]

On Tuesday 17 January 2017 05:56 PM, Bartosz Golaszewski wrote:
> Register a dummy clock modelling the external SATA oscillator for

I would not call it a dummy clock. Its representing something physically
present. Just call it "fixed rate clock" ?

> da850 DT mode. For non-DT boot we don't register the clock - instead
> we rely on the default MPY value defined in the da850 ahci driver (as
> is done currently).

Here too, like David suggested, it will be nice to register it both for
DT and non-DT case. With that I think your driver will be simple too
since you dont have to worry about the case when refclkpn is not supplied.

> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/mach-davinci/da8xx-dt.c           |  8 ++++++++
>  arch/arm/mach-davinci/devices-da8xx.c      | 23 +++++++++++++++++++++++
>  arch/arm/mach-davinci/include/mach/da8xx.h |  1 +
>  3 files changed, 32 insertions(+)
> 
> diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
> index b83e5d1..13137cb 100644
> --- a/arch/arm/mach-davinci/da8xx-dt.c
> +++ b/arch/arm/mach-davinci/da8xx-dt.c
> @@ -61,6 +61,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_refclk(100000000);
> +		if (ret)
> +			pr_warn("%s: registering SATA_REFCLK clock failed: %d",
> +				__func__, ret);
> +	}

Since all supported boards use 100 Mhz refclk anyway, I would drop the
machine check and just do:

	/* All existing boards use 100MHz SATA refclkpn */
	unsigned long sata_refclkpn = 100 * 1000 * 1000;

	ret = da850_register_sata_refclk(sata_refclkpn);

It should then be easy for the odd board (when it comes) to set
sata_refclkpn to a different value.

Thanks,
Sekhar

[PATCH v2 06/14] ARM: davinci: da850: model the SATA refclk

[Sekhar Nori]

On Tuesday 17 January 2017 05:56 PM, Bartosz Golaszewski wrote:
> Register a dummy clock modelling the external SATA oscillator for

I would not call it a dummy clock. Its representing something physically
present. Just call it "fixed rate clock" ?

> da850 DT mode. For non-DT boot we don't register the clock - instead
> we rely on the default MPY value defined in the da850 ahci driver (as
> is done currently).

Here too, like David suggested, it will be nice to register it both for
DT and non-DT case. With that I think your driver will be simple too
since you dont have to worry about the case when refclkpn is not supplied.

> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/mach-davinci/da8xx-dt.c           |  8 ++++++++
>  arch/arm/mach-davinci/devices-da8xx.c      | 23 +++++++++++++++++++++++
>  arch/arm/mach-davinci/include/mach/da8xx.h |  1 +
>  3 files changed, 32 insertions(+)
> 
> diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
> index b83e5d1..13137cb 100644
> --- a/arch/arm/mach-davinci/da8xx-dt.c
> +++ b/arch/arm/mach-davinci/da8xx-dt.c
> @@ -61,6 +61,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_refclk(100000000);
> +		if (ret)
> +			pr_warn("%s: registering SATA_REFCLK clock failed: %d",
> +				__func__, ret);
> +	}

Since all supported boards use 100 Mhz refclk anyway, I would drop the
machine check and just do:

	/* All existing boards use 100MHz SATA refclkpn */
	unsigned long sata_refclkpn = 100 * 1000 * 1000;

	ret = da850_register_sata_refclk(sata_refclkpn);

It should then be easy for the odd board (when it comes) to set
sata_refclkpn to a different value.

Thanks,
Sekhar

Re: [PATCH v2 06/14] ARM: davinci: da850: model the SATA refclk

[Sekhar Nori]

On Tuesday 17 January 2017 05:56 PM, Bartosz Golaszewski wrote:
> Register a dummy clock modelling the external SATA oscillator for

I would not call it a dummy clock. Its representing something physically
present. Just call it "fixed rate clock" ?

> da850 DT mode. For non-DT boot we don't register the clock - instead
> we rely on the default MPY value defined in the da850 ahci driver (as
> is done currently).

Here too, like David suggested, it will be nice to register it both for
DT and non-DT case. With that I think your driver will be simple too
since you dont have to worry about the case when refclkpn is not supplied.

> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/mach-davinci/da8xx-dt.c           |  8 ++++++++
>  arch/arm/mach-davinci/devices-da8xx.c      | 23 +++++++++++++++++++++++
>  arch/arm/mach-davinci/include/mach/da8xx.h |  1 +
>  3 files changed, 32 insertions(+)
> 
> diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
> index b83e5d1..13137cb 100644
> --- a/arch/arm/mach-davinci/da8xx-dt.c
> +++ b/arch/arm/mach-davinci/da8xx-dt.c
> @@ -61,6 +61,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_refclk(100000000);
> +		if (ret)
> +			pr_warn("%s: registering SATA_REFCLK clock failed: %d",
> +				__func__, ret);
> +	}

Since all supported boards use 100 Mhz refclk anyway, I would drop the
machine check and just do:

	/* All existing boards use 100MHz SATA refclkpn */
	unsigned long sata_refclkpn = 100 * 1000 * 1000;

	ret = da850_register_sata_refclk(sata_refclkpn);

It should then be easy for the odd board (when it comes) to set
sata_refclkpn to a different value.

Thanks,
Sekhar

[PATCH v2 07/14] sata: ahci-da850: add device tree match table

[Bartosz Golaszewski]

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

Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 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 18f57c2..c071701 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -121,11 +121,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

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

[PATCH v2 07/14] sata: ahci-da850: add device tree match table

[Bartosz Golaszewski]

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 18f57c2..c071701 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -121,11 +121,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

[PATCH v2 07/14] sata: ahci-da850: add device tree match table

[Bartosz Golaszewski]

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 18f57c2..c071701 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -121,11 +121,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

[PATCH v2 09/14] sata: ahci: export ahci_do_hardreset() locally

[Bartosz Golaszewski]

We need a way to retrieve the information about the online state of
the link in the ahci-da850 driver.

Create a new function: ahci_do_hardreset() which is called from
ahci_hardreset() for backwards compatibility, but has an additional
argument: 'online' - which can be used to check if the link is online
after this function returns.

The new routine will be used in the ahci-da850 driver to avoid code
duplication when implementing a workaround for tha da850 SATA
controller quirk/instability.

Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/ata/ahci.h    |  3 +++
 drivers/ata/libahci.c | 18 +++++++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 0cc08f8..5db6ab2 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -398,6 +398,9 @@ int ahci_do_softreset(struct ata_link *link, unsigned int *class,
 		      int pmp, unsigned long deadline,
 		      int (*check_ready)(struct ata_link *link));
 
+int ahci_do_hardreset(struct ata_link *link, unsigned int *class,
+		      unsigned long deadline, bool *online);
+
 unsigned int ahci_qc_issue(struct ata_queued_cmd *qc);
 int ahci_stop_engine(struct ata_port *ap);
 void ahci_start_fis_rx(struct ata_port *ap);
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index ee7db31..3159f9e 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1519,8 +1519,8 @@ static int ahci_pmp_retry_softreset(struct ata_link *link, unsigned int *class,
 	return rc;
 }
 
-static int ahci_hardreset(struct ata_link *link, unsigned int *class,
-			  unsigned long deadline)
+int ahci_do_hardreset(struct ata_link *link, unsigned int *class,
+		      unsigned long deadline, bool *online)
 {
 	const unsigned long *timing = sata_ehc_deb_timing(&link->eh_context);
 	struct ata_port *ap = link->ap;
@@ -1528,7 +1528,6 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class,
 	struct ahci_host_priv *hpriv = ap->host->private_data;
 	u8 *d2h_fis = pp->rx_fis + RX_FIS_D2H_REG;
 	struct ata_taskfile tf;
-	bool online;
 	int rc;
 
 	DPRINTK("ENTER\n");
@@ -1540,17 +1539,26 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class,
 	tf.command = ATA_BUSY;
 	ata_tf_to_fis(&tf, 0, 0, d2h_fis);
 
-	rc = sata_link_hardreset(link, timing, deadline, &online,
+	rc = sata_link_hardreset(link, timing, deadline, online,
 				 ahci_check_ready);
 
 	hpriv->start_engine(ap);
 
-	if (online)
+	if (*online)
 		*class = ahci_dev_classify(ap);
 
 	DPRINTK("EXIT, rc=%d, class=%u\n", rc, *class);
 	return rc;
 }
+EXPORT_SYMBOL_GPL(ahci_do_hardreset);
+
+static int ahci_hardreset(struct ata_link *link, unsigned int *class,
+			  unsigned long deadline)
+{
+	bool online;
+
+	return ahci_do_hardreset(link, class, deadline, &online);
+}
 
 static void ahci_postreset(struct ata_link *link, unsigned int *class)
 {
-- 
2.9.3

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

[PATCH v2 09/14] sata: ahci: export ahci_do_hardreset() locally

[Bartosz Golaszewski]

We need a way to retrieve the information about the online state of
the link in the ahci-da850 driver.

Create a new function: ahci_do_hardreset() which is called from
ahci_hardreset() for backwards compatibility, but has an additional
argument: 'online' - which can be used to check if the link is online
after this function returns.

The new routine will be used in the ahci-da850 driver to avoid code
duplication when implementing a workaround for tha da850 SATA
controller quirk/instability.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/ata/ahci.h    |  3 +++
 drivers/ata/libahci.c | 18 +++++++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 0cc08f8..5db6ab2 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -398,6 +398,9 @@ int ahci_do_softreset(struct ata_link *link, unsigned int *class,
 		      int pmp, unsigned long deadline,
 		      int (*check_ready)(struct ata_link *link));
 
+int ahci_do_hardreset(struct ata_link *link, unsigned int *class,
+		      unsigned long deadline, bool *online);
+
 unsigned int ahci_qc_issue(struct ata_queued_cmd *qc);
 int ahci_stop_engine(struct ata_port *ap);
 void ahci_start_fis_rx(struct ata_port *ap);
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index ee7db31..3159f9e 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1519,8 +1519,8 @@ static int ahci_pmp_retry_softreset(struct ata_link *link, unsigned int *class,
 	return rc;
 }
 
-static int ahci_hardreset(struct ata_link *link, unsigned int *class,
-			  unsigned long deadline)
+int ahci_do_hardreset(struct ata_link *link, unsigned int *class,
+		      unsigned long deadline, bool *online)
 {
 	const unsigned long *timing = sata_ehc_deb_timing(&link->eh_context);
 	struct ata_port *ap = link->ap;
@@ -1528,7 +1528,6 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class,
 	struct ahci_host_priv *hpriv = ap->host->private_data;
 	u8 *d2h_fis = pp->rx_fis + RX_FIS_D2H_REG;
 	struct ata_taskfile tf;
-	bool online;
 	int rc;
 
 	DPRINTK("ENTER\n");
@@ -1540,17 +1539,26 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class,
 	tf.command = ATA_BUSY;
 	ata_tf_to_fis(&tf, 0, 0, d2h_fis);
 
-	rc = sata_link_hardreset(link, timing, deadline, &online,
+	rc = sata_link_hardreset(link, timing, deadline, online,
 				 ahci_check_ready);
 
 	hpriv->start_engine(ap);
 
-	if (online)
+	if (*online)
 		*class = ahci_dev_classify(ap);
 
 	DPRINTK("EXIT, rc=%d, class=%u\n", rc, *class);
 	return rc;
 }
+EXPORT_SYMBOL_GPL(ahci_do_hardreset);
+
+static int ahci_hardreset(struct ata_link *link, unsigned int *class,
+			  unsigned long deadline)
+{
+	bool online;
+
+	return ahci_do_hardreset(link, class, deadline, &online);
+}
 
 static void ahci_postreset(struct ata_link *link, unsigned int *class)
 {
-- 
2.9.3

[PATCH v2 09/14] sata: ahci: export ahci_do_hardreset() locally

[Bartosz Golaszewski]

We need a way to retrieve the information about the online state of
the link in the ahci-da850 driver.

Create a new function: ahci_do_hardreset() which is called from
ahci_hardreset() for backwards compatibility, but has an additional
argument: 'online' - which can be used to check if the link is online
after this function returns.

The new routine will be used in the ahci-da850 driver to avoid code
duplication when implementing a workaround for tha da850 SATA
controller quirk/instability.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/ata/ahci.h    |  3 +++
 drivers/ata/libahci.c | 18 +++++++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 0cc08f8..5db6ab2 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -398,6 +398,9 @@ int ahci_do_softreset(struct ata_link *link, unsigned int *class,
 		      int pmp, unsigned long deadline,
 		      int (*check_ready)(struct ata_link *link));
 
+int ahci_do_hardreset(struct ata_link *link, unsigned int *class,
+		      unsigned long deadline, bool *online);
+
 unsigned int ahci_qc_issue(struct ata_queued_cmd *qc);
 int ahci_stop_engine(struct ata_port *ap);
 void ahci_start_fis_rx(struct ata_port *ap);
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index ee7db31..3159f9e 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1519,8 +1519,8 @@ static int ahci_pmp_retry_softreset(struct ata_link *link, unsigned int *class,
 	return rc;
 }
 
-static int ahci_hardreset(struct ata_link *link, unsigned int *class,
-			  unsigned long deadline)
+int ahci_do_hardreset(struct ata_link *link, unsigned int *class,
+		      unsigned long deadline, bool *online)
 {
 	const unsigned long *timing = sata_ehc_deb_timing(&link->eh_context);
 	struct ata_port *ap = link->ap;
@@ -1528,7 +1528,6 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class,
 	struct ahci_host_priv *hpriv = ap->host->private_data;
 	u8 *d2h_fis = pp->rx_fis + RX_FIS_D2H_REG;
 	struct ata_taskfile tf;
-	bool online;
 	int rc;
 
 	DPRINTK("ENTER\n");
@@ -1540,17 +1539,26 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class,
 	tf.command = ATA_BUSY;
 	ata_tf_to_fis(&tf, 0, 0, d2h_fis);
 
-	rc = sata_link_hardreset(link, timing, deadline, &online,
+	rc = sata_link_hardreset(link, timing, deadline, online,
 				 ahci_check_ready);
 
 	hpriv->start_engine(ap);
 
-	if (online)
+	if (*online)
 		*class = ahci_dev_classify(ap);
 
 	DPRINTK("EXIT, rc=%d, class=%u\n", rc, *class);
 	return rc;
 }
+EXPORT_SYMBOL_GPL(ahci_do_hardreset);
+
+static int ahci_hardreset(struct ata_link *link, unsigned int *class,
+			  unsigned long deadline)
+{
+	bool online;
+
+	return ahci_do_hardreset(link, class, deadline, &online);
+}
 
 static void ahci_postreset(struct ata_link *link, unsigned int *class)
 {
-- 
2.9.3

[PATCH v2 08/14] sata: ahci-da850: implement a workaround for the softreset quirk

[Bartosz Golaszewski]

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 | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index c071701..e0dc089 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -54,11 +54,42 @@ 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);
+
+	/*
+	 * There's an issue with the SATA controller on da850 SoCs: if we
+	 * enable Port Multiplier support, but the drive is connected directly
+	 * to the board, it can't be detected. As a workaround: if PMP is
+	 * enabled, we first call ahci_do_softreset() and pass it the result of
+	 * sata_srst_pmp(). If this call fails, we retry with pmp = 0.
+	 */
+	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,
+	/*
+	 * No need to override .pmp_softreset - it's only used for actual
+	 * PMP-enabled ports.
+	 */
+};
+
 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

[PATCH v2 08/14] sata: ahci-da850: implement a workaround for the softreset quirk

[Bartosz Golaszewski]

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 | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index c071701..e0dc089 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -54,11 +54,42 @@ 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);
+
+	/*
+	 * There's an issue with the SATA controller on da850 SoCs: if we
+	 * enable Port Multiplier support, but the drive is connected directly
+	 * to the board, it can't be detected. As a workaround: if PMP is
+	 * enabled, we first call ahci_do_softreset() and pass it the result of
+	 * sata_srst_pmp(). If this call fails, we retry with pmp = 0.
+	 */
+	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,
+	/*
+	 * No need to override .pmp_softreset - it's only used for actual
+	 * PMP-enabled ports.
+	 */
+};
+
 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

[PATCH v2 08/14] sata: ahci-da850: implement a workaround for the softreset quirk

[Bartosz Golaszewski]

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 | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index c071701..e0dc089 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -54,11 +54,42 @@ 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);
+
+	/*
+	 * There's an issue with the SATA controller on da850 SoCs: if we
+	 * enable Port Multiplier support, but the drive is connected directly
+	 * to the board, it can't be detected. As a workaround: if PMP is
+	 * enabled, we first call ahci_do_softreset() and pass it the result of
+	 * sata_srst_pmp(). If this call fails, we retry with pmp = 0.
+	 */
+	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,
+	/*
+	 * No need to override .pmp_softreset - it's only used for actual
+	 * PMP-enabled ports.
+	 */
+};
+
 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

[PATCH v2 10/14] sata: ahci-da850: add a workaround for controller instability

[Bartosz Golaszewski]

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.

The sata core driver already retries to resume the link because some
controllers ignore writes to the SControl register, but just retrying
the resume operation doesn't work - we need to issue he phy/wake reset
again to make it work.

Reimplement ahci_hardreset() in the driver and poke the controller a
couple times before really giving up.

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

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index e0dc089..a7a7161 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -16,7 +16,8 @@
 #include <linux/ahci_platform.h>
 #include "ahci.h"
 
-#define DRV_NAME "ahci_da850"
+#define DRV_NAME		"ahci_da850"
+#define HARDRESET_RETRIES	5
 
 /* SATA PHY Control Register offset from AHCI base */
 #define SATA_P0PHYCR_REG	0x178
@@ -76,6 +77,29 @@ static int ahci_da850_softreset(struct ata_link *link,
 	return ret;
 }
 
+static int ahci_da850_hardreset(struct ata_link *link,
+				unsigned int *class, unsigned long deadline)
+{
+	int ret, retry = HARDRESET_RETRIES;
+	bool online;
+
+	/*
+	 * In order to correctly service the LCD controller of the da850 SoC,
+	 * we increased the PLL0 frequency to 456MHz from the default 300MHz.
+	 *
+	 * This made the SATA controller unstable and the hardreset operation
+	 * does not always succeed the first time. Before really giving up to
+	 * bring up the link, retry the reset a couple times.
+	 */
+	do {
+		ret = ahci_do_hardreset(link, class, deadline, &online);
+		if (online)
+			return ret;
+	} while (retry--);
+
+	return ret;
+}
+
 static struct ata_port_operations ahci_da850_port_ops = {
 	.inherits = &ahci_platform_ops,
 	.softreset = ahci_da850_softreset,
@@ -83,6 +107,8 @@ static struct ata_port_operations ahci_da850_port_ops = {
 	 * No need to override .pmp_softreset - it's only used for actual
 	 * PMP-enabled ports.
 	 */
+	.hardreset = ahci_da850_hardreset,
+	.pmp_hardreset = ahci_da850_hardreset,
 };
 
 static const struct ata_port_info ahci_da850_port_info = {
-- 
2.9.3

[PATCH v2 10/14] sata: ahci-da850: add a workaround for controller instability

[Bartosz Golaszewski]

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.

The sata core driver already retries to resume the link because some
controllers ignore writes to the SControl register, but just retrying
the resume operation doesn't work - we need to issue he phy/wake reset
again to make it work.

Reimplement ahci_hardreset() in the driver and poke the controller a
couple times before really giving up.

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

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index e0dc089..a7a7161 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -16,7 +16,8 @@
 #include <linux/ahci_platform.h>
 #include "ahci.h"
 
-#define DRV_NAME "ahci_da850"
+#define DRV_NAME		"ahci_da850"
+#define HARDRESET_RETRIES	5
 
 /* SATA PHY Control Register offset from AHCI base */
 #define SATA_P0PHYCR_REG	0x178
@@ -76,6 +77,29 @@ static int ahci_da850_softreset(struct ata_link *link,
 	return ret;
 }
 
+static int ahci_da850_hardreset(struct ata_link *link,
+				unsigned int *class, unsigned long deadline)
+{
+	int ret, retry = HARDRESET_RETRIES;
+	bool online;
+
+	/*
+	 * In order to correctly service the LCD controller of the da850 SoC,
+	 * we increased the PLL0 frequency to 456MHz from the default 300MHz.
+	 *
+	 * This made the SATA controller unstable and the hardreset operation
+	 * does not always succeed the first time. Before really giving up to
+	 * bring up the link, retry the reset a couple times.
+	 */
+	do {
+		ret = ahci_do_hardreset(link, class, deadline, &online);
+		if (online)
+			return ret;
+	} while (retry--);
+
+	return ret;
+}
+
 static struct ata_port_operations ahci_da850_port_ops = {
 	.inherits = &ahci_platform_ops,
 	.softreset = ahci_da850_softreset,
@@ -83,6 +107,8 @@ static struct ata_port_operations ahci_da850_port_ops = {
 	 * No need to override .pmp_softreset - it's only used for actual
 	 * PMP-enabled ports.
 	 */
+	.hardreset = ahci_da850_hardreset,
+	.pmp_hardreset = ahci_da850_hardreset,
 };
 
 static const struct ata_port_info ahci_da850_port_info = {
-- 
2.9.3

[PATCH v2 11/14] sata: ahci-da850: un-hardcode the MPY bits

[Bartosz Golaszewski]

In order to make the MPY bits configurable, try to obtain the refclk
and calculate the required multiplier from its rate.

If we fail to get the clock, fall back to the default value which
keeps backwards compatibility.

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

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index a7a7161..f48b7d0 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -14,6 +14,7 @@
 #include <linux/platform_device.h>
 #include <linux/libata.h>
 #include <linux/ahci_platform.h>
+#include <asm/div64.h>
 #include "ahci.h"
 
 #define DRV_NAME		"ahci_da850"
@@ -30,16 +31,14 @@
 #define SATA_PHY_ENPLL(x)	((x) << 31)
 
 /*
- * 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 multiplier value used if the refclk could not be
+ * obtained. It corresponds with a crystal rate of 100MHz for 1.5GHz PLL
+ * output.
  */
-#define DA850_SATA_CLK_MULTIPLIER	7
+#define DA850_SATA_MPY_DEFAULT	0x8
 
 static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg,
-			    void __iomem *ahci_base)
+			    void __iomem *ahci_base, u32 mpy)
 {
 	unsigned int val;
 
@@ -48,13 +47,56 @@ 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) |
-	      SATA_PHY_RXCDR(4) | SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) |
-	      SATA_PHY_ENPLL(1);
+	val = SATA_PHY_MPY(mpy) | SATA_PHY_LOS(1) | SATA_PHY_RXCDR(4) |
+	      SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) | SATA_PHY_ENPLL(1);
 
 	writel(val, ahci_base + SATA_P0PHYCR_REG);
 }
 
+static u32 ahci_da850_calculate_mpy(unsigned long refclk_rate)
+{
+	u64 pll_output = 1500000000;
+	u32 needed;
+
+	/*
+	 * We need to determine the value of the multiplier (MPY) bits.
+	 *
+	 * In order to include the 12.5 multiplier we need to first multiply
+	 * the desired rate of 1.5GHz by 10 before division.
+	 */
+	pll_output *= 10;
+	needed = __div64_32(&pll_output, refclk_rate);
+
+	/*
+	 * What we have now is (multiplier * 10).
+	 *
+	 * Let's determine the actual register value we need to write.
+	 */
+
+	switch (needed) {
+	case 50:
+		return 0x1;
+	case 60:
+		return 0x2;
+	case 80:
+		return 0x4;
+	case 100:
+		return 0x5;
+	case 120:
+		return 0x6;
+	case 125:
+		return 0x7;
+	case 150:
+		return 0x8;
+	case 200:
+		return 0x9;
+	case 250:
+		return 0xa;
+	default:
+		return DA850_SATA_MPY_DEFAULT;
+	}
+}
+
 static int ahci_da850_softreset(struct ata_link *link,
 				unsigned int *class, unsigned long deadline)
 {
@@ -126,9 +168,10 @@ static int ahci_da850_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct ahci_host_priv *hpriv;
-	struct resource *res;
 	void __iomem *pwrdn_reg;
+	struct resource *res;
 	struct clk *clk;
+	u32 mpy;
 	int rc;
 
 	hpriv = ahci_platform_get_resources(pdev);
@@ -150,6 +193,27 @@ static int ahci_da850_probe(struct platform_device *pdev)
 		hpriv->clks[0] = clk;
 	}
 
+	/*
+	 * The second clock used by ahci-da850 is the external REFCLK. If we
+	 * didn't get it from ahci_platform_get_resources(), let's try to
+	 * specify the con_id in clk_get(). If we still don't get a clock,
+	 * we'll use the default value that works for the da850-evm board.
+	 */
+	if (!hpriv->clks[1]) {
+		clk = clk_get(dev, "refclk");
+		if (IS_ERR(clk))
+			hpriv->clks[1] = NULL;
+		else
+			hpriv->clks[1] = clk;
+	}
+
+	if (!hpriv->clks[1]) {
+		dev_info(dev, "unable to obtain the reference clock - using default multiplier");
+		mpy = DA850_SATA_MPY_DEFAULT;
+	} else {
+		mpy = ahci_da850_calculate_mpy(clk_get_rate(hpriv->clks[1]));
+	}
+
 	rc = ahci_platform_enable_resources(hpriv);
 	if (rc)
 		return rc;
@@ -162,7 +226,7 @@ static int ahci_da850_probe(struct platform_device *pdev)
 	if (!pwrdn_reg)
 		goto disable_resources;
 
-	da850_sata_init(dev, pwrdn_reg, hpriv->mmio);
+	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

[PATCH v2 11/14] sata: ahci-da850: un-hardcode the MPY bits

[Bartosz Golaszewski]

In order to make the MPY bits configurable, try to obtain the refclk
and calculate the required multiplier from its rate.

If we fail to get the clock, fall back to the default value which
keeps backwards compatibility.

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

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index a7a7161..f48b7d0 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -14,6 +14,7 @@
 #include <linux/platform_device.h>
 #include <linux/libata.h>
 #include <linux/ahci_platform.h>
+#include <asm/div64.h>
 #include "ahci.h"
 
 #define DRV_NAME		"ahci_da850"
@@ -30,16 +31,14 @@
 #define SATA_PHY_ENPLL(x)	((x) << 31)
 
 /*
- * 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 multiplier value used if the refclk could not be
+ * obtained. It corresponds with a crystal rate of 100MHz for 1.5GHz PLL
+ * output.
  */
-#define DA850_SATA_CLK_MULTIPLIER	7
+#define DA850_SATA_MPY_DEFAULT	0x8
 
 static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg,
-			    void __iomem *ahci_base)
+			    void __iomem *ahci_base, u32 mpy)
 {
 	unsigned int val;
 
@@ -48,13 +47,56 @@ 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) |
-	      SATA_PHY_RXCDR(4) | SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) |
-	      SATA_PHY_ENPLL(1);
+	val = SATA_PHY_MPY(mpy) | SATA_PHY_LOS(1) | SATA_PHY_RXCDR(4) |
+	      SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) | SATA_PHY_ENPLL(1);
 
 	writel(val, ahci_base + SATA_P0PHYCR_REG);
 }
 
+static u32 ahci_da850_calculate_mpy(unsigned long refclk_rate)
+{
+	u64 pll_output = 1500000000;
+	u32 needed;
+
+	/*
+	 * We need to determine the value of the multiplier (MPY) bits.
+	 *
+	 * In order to include the 12.5 multiplier we need to first multiply
+	 * the desired rate of 1.5GHz by 10 before division.
+	 */
+	pll_output *= 10;
+	needed = __div64_32(&pll_output, refclk_rate);
+
+	/*
+	 * What we have now is (multiplier * 10).
+	 *
+	 * Let's determine the actual register value we need to write.
+	 */
+
+	switch (needed) {
+	case 50:
+		return 0x1;
+	case 60:
+		return 0x2;
+	case 80:
+		return 0x4;
+	case 100:
+		return 0x5;
+	case 120:
+		return 0x6;
+	case 125:
+		return 0x7;
+	case 150:
+		return 0x8;
+	case 200:
+		return 0x9;
+	case 250:
+		return 0xa;
+	default:
+		return DA850_SATA_MPY_DEFAULT;
+	}
+}
+
 static int ahci_da850_softreset(struct ata_link *link,
 				unsigned int *class, unsigned long deadline)
 {
@@ -126,9 +168,10 @@ static int ahci_da850_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct ahci_host_priv *hpriv;
-	struct resource *res;
 	void __iomem *pwrdn_reg;
+	struct resource *res;
 	struct clk *clk;
+	u32 mpy;
 	int rc;
 
 	hpriv = ahci_platform_get_resources(pdev);
@@ -150,6 +193,27 @@ static int ahci_da850_probe(struct platform_device *pdev)
 		hpriv->clks[0] = clk;
 	}
 
+	/*
+	 * The second clock used by ahci-da850 is the external REFCLK. If we
+	 * didn't get it from ahci_platform_get_resources(), let's try to
+	 * specify the con_id in clk_get(). If we still don't get a clock,
+	 * we'll use the default value that works for the da850-evm board.
+	 */
+	if (!hpriv->clks[1]) {
+		clk = clk_get(dev, "refclk");
+		if (IS_ERR(clk))
+			hpriv->clks[1] = NULL;
+		else
+			hpriv->clks[1] = clk;
+	}
+
+	if (!hpriv->clks[1]) {
+		dev_info(dev, "unable to obtain the reference clock - using default multiplier");
+		mpy = DA850_SATA_MPY_DEFAULT;
+	} else {
+		mpy = ahci_da850_calculate_mpy(clk_get_rate(hpriv->clks[1]));
+	}
+
 	rc = ahci_platform_enable_resources(hpriv);
 	if (rc)
 		return rc;
@@ -162,7 +226,7 @@ static int ahci_da850_probe(struct platform_device *pdev)
 	if (!pwrdn_reg)
 		goto disable_resources;
 
-	da850_sata_init(dev, pwrdn_reg, hpriv->mmio);
+	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

Re: [PATCH v2 11/14] sata: ahci-da850: un-hardcode the MPY bits

[David Lechner]

On 01/17/2017 06:26 AM, Bartosz Golaszewski wrote:
> In order to make the MPY bits configurable, try to obtain the refclk
> and calculate the required multiplier from its rate.
>
> If we fail to get the clock, fall back to the default value which
> keeps backwards compatibility.

It seems like it would be wiser to make is so that if we fail to get the 
clock, it is an error. This way, if someone makes a new board and 
forgets to configure a clock, they will get an error instead of 
wondering why things are not working because it is using the wrong 
multiplier.

>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/ata/ahci_da850.c | 88 +++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 76 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
> index a7a7161..f48b7d0 100644
> --- a/drivers/ata/ahci_da850.c
> +++ b/drivers/ata/ahci_da850.c
> @@ -14,6 +14,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/libata.h>
>  #include <linux/ahci_platform.h>
> +#include <asm/div64.h>
>  #include "ahci.h"
>
>  #define DRV_NAME		"ahci_da850"
> @@ -30,16 +31,14 @@
>  #define SATA_PHY_ENPLL(x)	((x) << 31)
>
>  /*
> - * 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 multiplier value used if the refclk could not be
> + * obtained. It corresponds with a crystal rate of 100MHz for 1.5GHz PLL
> + * output.
>   */
> -#define DA850_SATA_CLK_MULTIPLIER	7
> +#define DA850_SATA_MPY_DEFAULT	0x8
>
>  static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg,
> -			    void __iomem *ahci_base)
> +			    void __iomem *ahci_base, u32 mpy)
>  {
>  	unsigned int val;
>
> @@ -48,13 +47,56 @@ 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) |
> -	      SATA_PHY_RXCDR(4) | SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) |
> -	      SATA_PHY_ENPLL(1);
> +	val = SATA_PHY_MPY(mpy) | SATA_PHY_LOS(1) | SATA_PHY_RXCDR(4) |
> +	      SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) | SATA_PHY_ENPLL(1);
>
>  	writel(val, ahci_base + SATA_P0PHYCR_REG);
>  }
>
> +static u32 ahci_da850_calculate_mpy(unsigned long refclk_rate)
> +{
> +	u64 pll_output = 1500000000;
> +	u32 needed;
> +
> +	/*
> +	 * We need to determine the value of the multiplier (MPY) bits.
> +	 *
> +	 * In order to include the 12.5 multiplier we need to first multiply
> +	 * the desired rate of 1.5GHz by 10 before division.
> +	 */
> +	pll_output *= 10;
> +	needed = __div64_32(&pll_output, refclk_rate);

What if this does not divide evenly and there is a remainder. Shouldn't 
there be an error in that case?

...

[PATCH v2 11/14] sata: ahci-da850: un-hardcode the MPY bits

[David Lechner]

On 01/17/2017 06:26 AM, Bartosz Golaszewski wrote:
> In order to make the MPY bits configurable, try to obtain the refclk
> and calculate the required multiplier from its rate.
>
> If we fail to get the clock, fall back to the default value which
> keeps backwards compatibility.

It seems like it would be wiser to make is so that if we fail to get the 
clock, it is an error. This way, if someone makes a new board and 
forgets to configure a clock, they will get an error instead of 
wondering why things are not working because it is using the wrong 
multiplier.

>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/ata/ahci_da850.c | 88 +++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 76 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
> index a7a7161..f48b7d0 100644
> --- a/drivers/ata/ahci_da850.c
> +++ b/drivers/ata/ahci_da850.c
> @@ -14,6 +14,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/libata.h>
>  #include <linux/ahci_platform.h>
> +#include <asm/div64.h>
>  #include "ahci.h"
>
>  #define DRV_NAME		"ahci_da850"
> @@ -30,16 +31,14 @@
>  #define SATA_PHY_ENPLL(x)	((x) << 31)
>
>  /*
> - * 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 multiplier value used if the refclk could not be
> + * obtained. It corresponds with a crystal rate of 100MHz for 1.5GHz PLL
> + * output.
>   */
> -#define DA850_SATA_CLK_MULTIPLIER	7
> +#define DA850_SATA_MPY_DEFAULT	0x8
>
>  static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg,
> -			    void __iomem *ahci_base)
> +			    void __iomem *ahci_base, u32 mpy)
>  {
>  	unsigned int val;
>
> @@ -48,13 +47,56 @@ 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) |
> -	      SATA_PHY_RXCDR(4) | SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) |
> -	      SATA_PHY_ENPLL(1);
> +	val = SATA_PHY_MPY(mpy) | SATA_PHY_LOS(1) | SATA_PHY_RXCDR(4) |
> +	      SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) | SATA_PHY_ENPLL(1);
>
>  	writel(val, ahci_base + SATA_P0PHYCR_REG);
>  }
>
> +static u32 ahci_da850_calculate_mpy(unsigned long refclk_rate)
> +{
> +	u64 pll_output = 1500000000;
> +	u32 needed;
> +
> +	/*
> +	 * We need to determine the value of the multiplier (MPY) bits.
> +	 *
> +	 * In order to include the 12.5 multiplier we need to first multiply
> +	 * the desired rate of 1.5GHz by 10 before division.
> +	 */
> +	pll_output *= 10;
> +	needed = __div64_32(&pll_output, refclk_rate);

What if this does not divide evenly and there is a remainder. Shouldn't 
there be an error in that case?

...

[PATCH v2 12/14] ARM: dts: da850: add pinmux settings for the SATA controller

[Bartosz Golaszewski]

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

[PATCH v2 12/14] ARM: dts: da850: add pinmux settings for the SATA controller

[Bartosz Golaszewski]

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

Re: [PATCH v2 12/14] ARM: dts: da850: add pinmux settings for the SATA controller

[Sekhar Nori]

Hi Bartosz,

On Tuesday 17 January 2017 05:56 PM, Bartosz Golaszewski wrote:
> Add pinmux sub-nodes for all muxed SATA pins.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Since these are all unused at this time, we should drop adding them
until a time someone really needs these.

Thanks,
Sekhar

[PATCH v2 12/14] ARM: dts: da850: add pinmux settings for the SATA controller

[Sekhar Nori]

Hi Bartosz,

On Tuesday 17 January 2017 05:56 PM, Bartosz Golaszewski wrote:
> Add pinmux sub-nodes for all muxed SATA pins.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Since these are all unused at this time, we should drop adding them
until a time someone really needs these.

Thanks,
Sekhar

Re: [PATCH v2 12/14] ARM: dts: da850: add pinmux settings for the SATA controller

[Sekhar Nori]

Hi Bartosz,

On Tuesday 17 January 2017 05:56 PM, Bartosz Golaszewski wrote:
> Add pinmux sub-nodes for all muxed SATA pins.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Since these are all unused at this time, we should drop adding them
until a time someone really needs these.

Thanks,
Sekhar

[PATCH v2 13/14] ARM: dts: da850: add the SATA node

[Bartosz Golaszewski]

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..7c32bf7 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@218000 {
+			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

[PATCH v2 13/14] ARM: dts: da850: add the SATA node

[Bartosz Golaszewski]

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..7c32bf7 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 at 218000 {
+			compatible = "ti,da850-ahci";
+			reg = <0x218000 0x2000>, <0x22c018 0x4>;
+			interrupts = <67>;
+			status = "disabled";
+		};
 		mdio: mdio at 224000 {
 			compatible = "ti,davinci_mdio";
 			#address-cells = <1>;
-- 
2.9.3

[PATCH v2 14/14] ARM: dts: da850-lcdk: enable the SATA node

[Bartosz Golaszewski]

Enable the SATA node for da850-lcdk. We omit the pinctrl property on
purpose - the muxed SATA pins are not hooked up to anything
SATA-related on the lcdk.

The REFCLKN/P rate on the board is 100MHz, so we need a multiplier of
15 for 1.5GHz PLL rate.

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

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index afcb482..fbeee3c 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -105,6 +105,10 @@
 	status = "okay";
 };
 
+&sata {
+	status = "okay";
+};
+
 &mdio {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mdio_pins>;
-- 
2.9.3

[PATCH v2 14/14] ARM: dts: da850-lcdk: enable the SATA node

[Bartosz Golaszewski]

Enable the SATA node for da850-lcdk. We omit the pinctrl property on
purpose - the muxed SATA pins are not hooked up to anything
SATA-related on the lcdk.

The REFCLKN/P rate on the board is 100MHz, so we need a multiplier of
15 for 1.5GHz PLL rate.

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

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index afcb482..fbeee3c 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -105,6 +105,10 @@
 	status = "okay";
 };
 
+&sata {
+	status = "okay";
+};
+
 &mdio {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mdio_pins>;
-- 
2.9.3

Re: [PATCH v2 14/14] ARM: dts: da850-lcdk: enable the SATA node

[David Lechner]

On 01/17/2017 06:26 AM, Bartosz Golaszewski wrote:
> Enable the SATA node for da850-lcdk. We omit the pinctrl property on
> purpose - the muxed SATA pins are not hooked up to anything
> SATA-related on the lcdk.
>
> The REFCLKN/P rate on the board is 100MHz, so we need a multiplier of
> 15 for 1.5GHz PLL rate.

The multiplier is no longer in device tree, so this comment is not 
necessary.

>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/boot/dts/da850-lcdk.dts | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> index afcb482..fbeee3c 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -105,6 +105,10 @@
>  	status = "okay";
>  };
>
> +&sata {
> +	status = "okay";
> +};
> +
>  &mdio {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&mdio_pins>;
>

[PATCH v2 14/14] ARM: dts: da850-lcdk: enable the SATA node

[David Lechner]

On 01/17/2017 06:26 AM, Bartosz Golaszewski wrote:
> Enable the SATA node for da850-lcdk. We omit the pinctrl property on
> purpose - the muxed SATA pins are not hooked up to anything
> SATA-related on the lcdk.
>
> The REFCLKN/P rate on the board is 100MHz, so we need a multiplier of
> 15 for 1.5GHz PLL rate.

The multiplier is no longer in device tree, so this comment is not 
necessary.

>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/boot/dts/da850-lcdk.dts | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> index afcb482..fbeee3c 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -105,6 +105,10 @@
>  	status = "okay";
>  };
>
> +&sata {
> +	status = "okay";
> +};
> +
>  &mdio {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&mdio_pins>;
>

Re: [PATCH v2 14/14] ARM: dts: da850-lcdk: enable the SATA node

[Kevin Hilman]

Bartosz Golaszewski <bgolaszewski@baylibre.com> writes:

> Enable the SATA node for da850-lcdk. We omit the pinctrl property on
> purpose - the muxed SATA pins are not hooked up to anything
> SATA-related on the lcdk.
>
> The REFCLKN/P rate on the board is 100MHz, so we need a multiplier of
> 15 for 1.5GHz PLL rate.

leftover comment from previous version?

Kevin

> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/boot/dts/da850-lcdk.dts | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> index afcb482..fbeee3c 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -105,6 +105,10 @@
>  	status = "okay";
>  };
>  
> +&sata {
> +	status = "okay";
> +};
> +
>  &mdio {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&mdio_pins>;

[PATCH v2 14/14] ARM: dts: da850-lcdk: enable the SATA node

[Kevin Hilman]

Bartosz Golaszewski <bgolaszewski@baylibre.com> writes:

> Enable the SATA node for da850-lcdk. We omit the pinctrl property on
> purpose - the muxed SATA pins are not hooked up to anything
> SATA-related on the lcdk.
>
> The REFCLKN/P rate on the board is 100MHz, so we need a multiplier of
> 15 for 1.5GHz PLL rate.

leftover comment from previous version?

Kevin

> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/boot/dts/da850-lcdk.dts | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> index afcb482..fbeee3c 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -105,6 +105,10 @@
>  	status = "okay";
>  };
>  
> +&sata {
> +	status = "okay";
> +};
> +
>  &mdio {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&mdio_pins>;