2014-10-21 09:08:16

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v2 0/5] Berlin BG2 AHCI and SATA PHY

This patch set is v2 of the catch up with latest Berlin improvements
provided by Antoine - in particular SATA PHY support and AHCI generic
for Berlin BG2.

Marvell BSP code for BG2 suggests more differences between the two
PHY revisions found on BG2 and BG2Q, but the only important one seems
to be the PHY_BASE used in AHCI vendor-specific registers. I also
confirmed that power_off does indeed power off the PHY on BG2, too
(It wasn't very clear in BSP code).

Anyway, I have tested this on BG2-based Sony NSZ-GS7 and attached
SATA HDD is successfully detected and partitions are displayed.

This patch set is now based on v3.18-rc1. As usual a branch based on
v3.18-rc1 can be found at

git://git.infradead.org/users/hesselba/linux-berlin.git devel/bg2-sata-v2

Compared to v1, there is only a minor change that removes an unrequired
status = "disabled" property from BG2 SATA node.

Patches 1-3 should go through Kishon's PHY tree, I pick up DT patches 4
and 5.

Patch 1 prepares phy-berlin-sata to support different PHY_BASE addresses
by moving the constant to driver private data.

Patches 2 and 3 add a new compatible to driver and DT documentation that
reflects the differences between BG2Q and BG2 SATA PHY.

Patches 4 and 5 finally add DT nodes to both Berlin2 SoC dtsi and Sony
NSZ-GS7 board DT file. SATA plug on NSZ-GS7 is unpopulated but can be
very easily equipped with SATA receptable and some 0402 caps. I decided
to enable SATA by default although not all users may populate it.

Sebastian Hesselbarth (5):
phy: berlin-sata: Move PHY_BASE into private data struct
phy: berlin-sata: Add support for BG2 SATA PHY
phy: berlin-sata: Document BG2 compatible
ARM: berlin: Add AHCI and SATA PHY nodes to BG2
ARM: berlin: Enable SATA on Sony NSZ-GS7

.../devicetree/bindings/phy/berlin-sata-phy.txt | 4 +-
arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts | 7 ++++
arch/arm/boot/dts/berlin2.dtsi | 39 ++++++++++++++++++
drivers/phy/phy-berlin-sata.c | 48 +++++++++++++++-------
4 files changed, 83 insertions(+), 15 deletions(-)

---
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: "Antoine Ténart" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
--
2.1.1


2014-10-21 09:08:11

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v2 2/5] phy: berlin-sata: Add support for BG2 SATA PHY

Berlin BG2 also has a SATA PHY compatible with the current driver
except different PHY_BASE. Add a new compatible to the driver
reflecting the different PHY_BASE.

Acked-by: Antoine Ténart <[email protected]>
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: "Antoine Ténart" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/phy/phy-berlin-sata.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
index 9682b0f66177..71568f5444ac 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -30,6 +30,7 @@
#define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
#define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)

+#define BG2_PHY_BASE 0x080
#define BG2Q_PHY_BASE 0x200

/* register 0x01 */
@@ -186,10 +187,15 @@ static u32 phy_berlin_power_down_bits[] = {
POWER_DOWN_PHY1,
};

+static u32 bg2_sata_phy_base = BG2_PHY_BASE;
static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;

static const struct of_device_id phy_berlin_sata_of_match[] = {
{
+ .compatible = "marvell,berlin2-sata-phy",
+ .data = &bg2_sata_phy_base,
+ },
+ {
.compatible = "marvell,berlin2q-sata-phy",
.data = &bg2q_sata_phy_base,
},
--
2.1.1

2014-10-21 09:08:19

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v2 4/5] ARM: berlin: Add AHCI and SATA PHY nodes to BG2

Add DT nodes for the AHCI controller and SATA PHY found on Marvell
Berlin2 SoCs.

Acked-by: Antoine Ténart <[email protected]>
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Changelog:
v1->v2:
- remove status = "disabled" from SATA controller node (Suggested by
Antoine)

Cc: Kishon Vijay Abraham I <[email protected]>
Cc: "Antoine Ténart" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arm/boot/dts/berlin2.dtsi | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi
index 9d7c810ebd0b..f39090491eb2 100644
--- a/arch/arm/boot/dts/berlin2.dtsi
+++ b/arch/arm/boot/dts/berlin2.dtsi
@@ -246,6 +246,45 @@
};
};

+ ahci: sata@e90000 {
+ compatible = "marvell,berlin2-ahci", "generic-ahci";
+ reg = <0xe90000 0x1000>;
+ interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&chip CLKID_SATA>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ sata0: sata-port@0 {
+ reg = <0>;
+ phys = <&sata_phy 0>;
+ status = "disabled";
+ };
+
+ sata1: sata-port@1 {
+ reg = <1>;
+ phys = <&sata_phy 1>;
+ status = "disabled";
+ };
+ };
+
+ sata_phy: phy@e900a0 {
+ compatible = "marvell,berlin2-sata-phy";
+ reg = <0xe900a0 0x200>;
+ clocks = <&chip CLKID_SATA>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #phy-cells = <1>;
+ status = "disabled";
+
+ sata-phy@0 {
+ reg = <0>;
+ };
+
+ sata-phy@1 {
+ reg = <1>;
+ };
+ };
+
chip: chip-control@ea0000 {
compatible = "marvell,berlin2-chip-ctrl";
#clock-cells = <1>;
--
2.1.1

2014-10-21 09:08:14

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v2 5/5] ARM: berlin: Enable SATA on Sony NSZ-GS7

Marvell Berlin BG2 based Sony NSZ-GS7 has an unpopulated SATA plug
on its PCB solder side. As it is quite easy to populate and I have
done it, enable AHCI and SATA by default.

Acked-by: Antoine Ténart <[email protected]>
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: "Antoine Ténart" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts b/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts
index c72bfd468d10..0a13e8a5da15 100644
--- a/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts
+++ b/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts
@@ -26,4 +26,11 @@
};
};

+&ahci { status = "okay"; };
+
+/* Unpopulated SATA plug on solder side */
+&sata0 { status = "okay"; };
+
+&sata_phy { status = "okay"; };
+
&uart0 { status = "okay"; };
--
2.1.1

2014-10-21 09:09:43

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v2 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

Currently, Berlin SATA PHY driver assumes PHY_BASE address being
constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
is different. Prepare the driver for BG2 support by moving the phy_base
into private driver data.

Acked-by: Antoine Ténart <[email protected]>
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: "Antoine Ténart" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/phy/phy-berlin-sata.c | 42 ++++++++++++++++++++++++++++--------------
1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
index 69ced52d72aa..9682b0f66177 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -30,7 +30,7 @@
#define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
#define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)

-#define PHY_BASE 0x200
+#define BG2Q_PHY_BASE 0x200

/* register 0x01 */
#define REF_FREF_SEL_25 BIT(0)
@@ -61,15 +61,16 @@ struct phy_berlin_priv {
struct clk *clk;
struct phy_berlin_desc **phys;
unsigned nphys;
+ u32 phy_base;
};

-static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg, u32 reg,
- u32 mask, u32 val)
+static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg,
+ u32 phy_base, u32 reg, u32 mask, u32 val)
{
u32 regval;

/* select register */
- writel(PHY_BASE + reg, ctrl_reg + PORT_VSR_ADDR);
+ writel(phy_base + reg, ctrl_reg + PORT_VSR_ADDR);

/* set bits */
regval = readl(ctrl_reg + PORT_VSR_DATA);
@@ -103,17 +104,20 @@ static int phy_berlin_sata_power_on(struct phy *phy)
writel(regval, priv->base + HOST_VSA_DATA);

/* set PHY mode and ref freq to 25 MHz */
- phy_berlin_sata_reg_setbits(ctrl_reg, 0x1, 0xff,
- REF_FREF_SEL_25 | PHY_MODE_SATA);
+ phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x01,
+ 0x00ff, REF_FREF_SEL_25 | PHY_MODE_SATA);

/* set PHY up to 6 Gbps */
- phy_berlin_sata_reg_setbits(ctrl_reg, 0x25, 0xc00, PHY_GEN_MAX_6_0);
+ phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x25,
+ 0x0c00, PHY_GEN_MAX_6_0);

/* set 40 bits width */
- phy_berlin_sata_reg_setbits(ctrl_reg, 0x23, 0xc00, DATA_BIT_WIDTH_40);
+ phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x23,
+ 0x0c00, DATA_BIT_WIDTH_40);

/* use max pll rate */
- phy_berlin_sata_reg_setbits(ctrl_reg, 0x2, 0x0, USE_MAX_PLL_RATE);
+ phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x02,
+ 0x0000, USE_MAX_PLL_RATE);

/* set Gen3 controller speed */
regval = readl(ctrl_reg + PORT_SCR_CTL);
@@ -182,9 +186,22 @@ static u32 phy_berlin_power_down_bits[] = {
POWER_DOWN_PHY1,
};

+static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+ {
+ .compatible = "marvell,berlin2q-sata-phy",
+ .data = &bg2q_sata_phy_base,
+ },
+ { },
+};
+
static int phy_berlin_sata_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
+ const struct of_device_id *match =
+ of_match_node(phy_berlin_sata_of_match, dev->of_node);
+ const u32 *phy_base = match->data;
struct device_node *child;
struct phy *phy;
struct phy_provider *phy_provider;
@@ -218,6 +235,8 @@ static int phy_berlin_sata_probe(struct platform_device *pdev)
if (!priv->phys)
return -ENOMEM;

+ priv->phy_base = *phy_base;
+
dev_set_drvdata(dev, priv);
spin_lock_init(&priv->lock);

@@ -264,11 +283,6 @@ static int phy_berlin_sata_probe(struct platform_device *pdev)
return 0;
}

-static const struct of_device_id phy_berlin_sata_of_match[] = {
- { .compatible = "marvell,berlin2q-sata-phy" },
- { },
-};
-
static struct platform_driver phy_berlin_sata_driver = {
.probe = phy_berlin_sata_probe,
.driver = {
--
2.1.1

2014-10-21 09:09:42

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v2 3/5] phy: berlin-sata: Document BG2 compatible

Berlin BG2 SATA PHY is slightly different from currently supported
BG2Q SATA PHY. Document the new compatible for BG2's PHY.

Acked-by: Antoine Ténart <[email protected]>
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: "Antoine Ténart" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
Documentation/devicetree/bindings/phy/berlin-sata-phy.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt b/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
index 88f8c23384c0..c0155f842f62 100644
--- a/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
+++ b/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
@@ -2,7 +2,9 @@ Berlin SATA PHY
---------------

Required properties:
-- compatible: should be "marvell,berlin2q-sata-phy"
+- compatible: should be one of
+ "marvell,berlin2-sata-phy"
+ "marvell,berlin2q-sata-phy"
- address-cells: should be 1
- size-cells: should be 0
- phy-cells: from the generic PHY bindings, must be 1
--
2.1.1

2014-10-21 09:34:00

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] phy: berlin-sata: Move PHY_BASE into private data struct



On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
> Currently, Berlin SATA PHY driver assumes PHY_BASE address being
> constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
> is different. Prepare the driver for BG2 support by moving the phy_base
> into private driver data.
>
> Acked-by: Antoine Ténart <[email protected]>
> Signed-off-by: Sebastian Hesselbarth <[email protected]>
> ---
> Cc: Kishon Vijay Abraham I <[email protected]>
> Cc: "Antoine Ténart" <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/phy/phy-berlin-sata.c | 42 ++++++++++++++++++++++++++++--------------
> 1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
> index 69ced52d72aa..9682b0f66177 100644
> --- a/drivers/phy/phy-berlin-sata.c
> +++ b/drivers/phy/phy-berlin-sata.c
> @@ -30,7 +30,7 @@
> #define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
> #define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)
>
> -#define PHY_BASE 0x200
> +#define BG2Q_PHY_BASE 0x200
>
> /* register 0x01 */
> #define REF_FREF_SEL_25 BIT(0)
> @@ -61,15 +61,16 @@ struct phy_berlin_priv {
> struct clk *clk;
> struct phy_berlin_desc **phys;
> unsigned nphys;
> + u32 phy_base;
> };
>
> -static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg, u32 reg,
> - u32 mask, u32 val)
> +static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg,
> + u32 phy_base, u32 reg, u32 mask, u32 val)
> {
> u32 regval;
>
> /* select register */
> - writel(PHY_BASE + reg, ctrl_reg + PORT_VSR_ADDR);
> + writel(phy_base + reg, ctrl_reg + PORT_VSR_ADDR);
>
> /* set bits */
> regval = readl(ctrl_reg + PORT_VSR_DATA);
> @@ -103,17 +104,20 @@ static int phy_berlin_sata_power_on(struct phy *phy)
> writel(regval, priv->base + HOST_VSA_DATA);
>
> /* set PHY mode and ref freq to 25 MHz */
> - phy_berlin_sata_reg_setbits(ctrl_reg, 0x1, 0xff,
> - REF_FREF_SEL_25 | PHY_MODE_SATA);
> + phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x01,
> + 0x00ff, REF_FREF_SEL_25 | PHY_MODE_SATA);
>
> /* set PHY up to 6 Gbps */
> - phy_berlin_sata_reg_setbits(ctrl_reg, 0x25, 0xc00, PHY_GEN_MAX_6_0);
> + phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x25,
> + 0x0c00, PHY_GEN_MAX_6_0);
>
> /* set 40 bits width */
> - phy_berlin_sata_reg_setbits(ctrl_reg, 0x23, 0xc00, DATA_BIT_WIDTH_40);
> + phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x23,
> + 0x0c00, DATA_BIT_WIDTH_40);
>
> /* use max pll rate */
> - phy_berlin_sata_reg_setbits(ctrl_reg, 0x2, 0x0, USE_MAX_PLL_RATE);
> + phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x02,
> + 0x0000, USE_MAX_PLL_RATE);
>
> /* set Gen3 controller speed */
> regval = readl(ctrl_reg + PORT_SCR_CTL);
> @@ -182,9 +186,22 @@ static u32 phy_berlin_power_down_bits[] = {
> POWER_DOWN_PHY1,
> };
>
> +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
> +
> +static const struct of_device_id phy_berlin_sata_of_match[] = {
> + {
> + .compatible = "marvell,berlin2q-sata-phy",
> + .data = &bg2q_sata_phy_base,

Can't the base directly come from dt?

Thanks
Kishon

2014-10-21 09:40:11

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:
> On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
>> Currently, Berlin SATA PHY driver assumes PHY_BASE address being
>> constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
>> is different. Prepare the driver for BG2 support by moving the phy_base
>> into private driver data.
>>
>> Acked-by: Antoine Ténart <[email protected]>
>> Signed-off-by: Sebastian Hesselbarth <[email protected]>
...
>> ---
>> drivers/phy/phy-berlin-sata.c | 42 ++++++++++++++++++++++++++++--------------
>> 1 file changed, 28 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
>> index 69ced52d72aa..9682b0f66177 100644
>> --- a/drivers/phy/phy-berlin-sata.c
>> +++ b/drivers/phy/phy-berlin-sata.c
>> @@ -30,7 +30,7 @@
>> #define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
>> #define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)
>>
>> -#define PHY_BASE 0x200
>> +#define BG2Q_PHY_BASE 0x200
[...]
>> +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
>> +
>> +static const struct of_device_id phy_berlin_sata_of_match[] = {
>> + {
>> + .compatible = "marvell,berlin2q-sata-phy",
>> + .data = &bg2q_sata_phy_base,
>
> Can't the base directly come from dt?

You are suggesting a "marvell,phy-base-address" property, right?
I have no strong opinion about it, I accept your call (or DT maintainer
ones).

Sebastian

2014-10-24 20:15:05

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

On 21.10.2014 11:40, Sebastian Hesselbarth wrote:
> On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:
>> On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
>>> Currently, Berlin SATA PHY driver assumes PHY_BASE address being
>>> constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
>>> is different. Prepare the driver for BG2 support by moving the phy_base
>>> into private driver data.
>>>
>>> Acked-by: Antoine Ténart <[email protected]>
>>> Signed-off-by: Sebastian Hesselbarth <[email protected]>
> ...
>>> ---
>>> drivers/phy/phy-berlin-sata.c | 42
>>> ++++++++++++++++++++++++++++--------------
>>> 1 file changed, 28 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/phy/phy-berlin-sata.c
>>> b/drivers/phy/phy-berlin-sata.c
>>> index 69ced52d72aa..9682b0f66177 100644
>>> --- a/drivers/phy/phy-berlin-sata.c
>>> +++ b/drivers/phy/phy-berlin-sata.c
>>> @@ -30,7 +30,7 @@
>>> #define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
>>> #define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)
>>>
>>> -#define PHY_BASE 0x200
>>> +#define BG2Q_PHY_BASE 0x200
> [...]
>>> +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
>>> +
>>> +static const struct of_device_id phy_berlin_sata_of_match[] = {
>>> + {
>>> + .compatible = "marvell,berlin2q-sata-phy",
>>> + .data = &bg2q_sata_phy_base,
>>
>> Can't the base directly come from dt?
>
> You are suggesting a "marvell,phy-base-address" property, right?
> I have no strong opinion about it, I accept your call (or DT maintainer
> ones).

Kishon,

I still have the DT patches for BG2Q queued up for v3.19 (I missed the
arm-soc merge window for v3.18). That means, there has been no release
with the phy binding used and I can rework a little more.

Can you please confirm that you want a DT property for the phy base
address, e.g. marvell,phy-base-address = <{0x200,0x80}> ?

If so, I'd also rename the compatible from berlin2q-sata-phy to more
generic berlin-sata-phy.

Sebastian

2014-10-24 20:26:00

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

Hi,

On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:
> On 21.10.2014 11:40, Sebastian Hesselbarth wrote:
> >On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:
> >>On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
> >>>Currently, Berlin SATA PHY driver assumes PHY_BASE address being
> >>>constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
> >>>is different. Prepare the driver for BG2 support by moving the phy_base
> >>>into private driver data.
> >>>
> >>>Acked-by: Antoine T?nart <[email protected]>
> >>>Signed-off-by: Sebastian Hesselbarth <[email protected]>
> >...
> >>>---
> >>> drivers/phy/phy-berlin-sata.c | 42
> >>>++++++++++++++++++++++++++++--------------
> >>> 1 file changed, 28 insertions(+), 14 deletions(-)
> >>>
> >>>diff --git a/drivers/phy/phy-berlin-sata.c
> >>>b/drivers/phy/phy-berlin-sata.c
> >>>index 69ced52d72aa..9682b0f66177 100644
> >>>--- a/drivers/phy/phy-berlin-sata.c
> >>>+++ b/drivers/phy/phy-berlin-sata.c
> >>>@@ -30,7 +30,7 @@
> >>> #define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
> >>> #define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)
> >>>
> >>>-#define PHY_BASE 0x200
> >>>+#define BG2Q_PHY_BASE 0x200
> >[...]
> >>>+static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
> >>>+
> >>>+static const struct of_device_id phy_berlin_sata_of_match[] = {
> >>>+ {
> >>>+ .compatible = "marvell,berlin2q-sata-phy",
> >>>+ .data = &bg2q_sata_phy_base,
> >>
> >>Can't the base directly come from dt?
> >
> >You are suggesting a "marvell,phy-base-address" property, right?
> >I have no strong opinion about it, I accept your call (or DT maintainer
> >ones).
>
> Kishon,
>
> I still have the DT patches for BG2Q queued up for v3.19 (I missed the
> arm-soc merge window for v3.18). That means, there has been no release
> with the phy binding used and I can rework a little more.
>
> Can you please confirm that you want a DT property for the phy base address,
> e.g. marvell,phy-base-address = <{0x200,0x80}> ?
>
> If so, I'd also rename the compatible from berlin2q-sata-phy to more
> generic berlin-sata-phy.

I think what Kishon is asking, is why this 0x200 offset isn't already on
reg. so that instead of, e.g.:

reg = <0x40000000 0x1000>;

you would have:

reg = <0x40000200 0x1000>;

then everybody's happy. It's unfortunate, however, that we already
shipped DT sources with the bogus (?) reg property and now we have to
support that broken binding. My suggestion would be to add a new
compatible which comes with proper reg property and still add that weird
phy_base for the old compatible, so that:

if (of_device_is_compatible(node, "marvell,berlin2q-sata-phy"))
phy->phy_base = PHY_BASE;

then, if new compatible comes with proper 'reg', phy->phy_base would be
zero and everything should still work. How does this sound ?

--
balbi


Attachments:
(No filename) (2.84 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-10-24 20:35:49

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

On 24.10.2014 22:25, Felipe Balbi wrote:
> Hi,
>
> On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:
>> On 21.10.2014 11:40, Sebastian Hesselbarth wrote:
>>> On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:
>>>> On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
>>>>> Currently, Berlin SATA PHY driver assumes PHY_BASE address being
>>>>> constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
>>>>> is different. Prepare the driver for BG2 support by moving the phy_base
>>>>> into private driver data.
>>>>>
>>>>> Acked-by: Antoine T?nart <[email protected]>
>>>>> Signed-off-by: Sebastian Hesselbarth <[email protected]>
>>> ...
>>>>> ---
>>>>> drivers/phy/phy-berlin-sata.c | 42
>>>>> ++++++++++++++++++++++++++++--------------
>>>>> 1 file changed, 28 insertions(+), 14 deletions(-)
>>>>>
>>>>> diff --git a/drivers/phy/phy-berlin-sata.c
>>>>> b/drivers/phy/phy-berlin-sata.c
>>>>> index 69ced52d72aa..9682b0f66177 100644
>>>>> --- a/drivers/phy/phy-berlin-sata.c
>>>>> +++ b/drivers/phy/phy-berlin-sata.c
>>>>> @@ -30,7 +30,7 @@
>>>>> #define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
>>>>> #define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)
>>>>>
>>>>> -#define PHY_BASE 0x200
>>>>> +#define BG2Q_PHY_BASE 0x200
>>> [...]
>>>>> +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
>>>>> +
>>>>> +static const struct of_device_id phy_berlin_sata_of_match[] = {
>>>>> + {
>>>>> + .compatible = "marvell,berlin2q-sata-phy",
>>>>> + .data = &bg2q_sata_phy_base,
>>>>
>>>> Can't the base directly come from dt?
>>>
>>> You are suggesting a "marvell,phy-base-address" property, right?
>>> I have no strong opinion about it, I accept your call (or DT maintainer
>>> ones).
>>
>> Kishon,
>>
>> I still have the DT patches for BG2Q queued up for v3.19 (I missed the
>> arm-soc merge window for v3.18). That means, there has been no release
>> with the phy binding used and I can rework a little more.
>>
>> Can you please confirm that you want a DT property for the phy base address,
>> e.g. marvell,phy-base-address = <{0x200,0x80}> ?
>>
>> If so, I'd also rename the compatible from berlin2q-sata-phy to more
>> generic berlin-sata-phy.
>
> I think what Kishon is asking, is why this 0x200 offset isn't already on
> reg. so that instead of, e.g.:
>
> reg = <0x40000000 0x1000>;
>
> you would have:
>
> reg = <0x40000200 0x1000>;

Because it is the PHY_BASE offset within the _indirect_ register
access trough AHCI registers.

> then everybody's happy. It's unfortunate, however, that we already
> shipped DT sources with the bogus (?) reg property and now we have to
> support that broken binding. My suggestion would be to add a new
> compatible which comes with proper reg property and still add that weird
> phy_base for the old compatible, so that:

Nope, the reg property is correct and describes the (vendor-specific)
AHCI registers that are used for indirect access to PHY registers. When
writing to PHY registers, BG2 and BG2Q are different with respect to
the PHY_BASE address offset that has to be passed to indirect address
register.

> if (of_device_is_compatible(node, "marvell,berlin2q-sata-phy"))
> phy->phy_base = PHY_BASE;
>
> then, if new compatible comes with proper 'reg', phy->phy_base would be
> zero and everything should still work. How does this sound ?

The above is basically equivalent to the node match that I added with
this patch series. I can, of course, use above compatible match instead
of passing the phy_base in the of_device_id's .data.

Sebastian

2014-10-27 12:27:42

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

Hi,

On Saturday 25 October 2014 01:55 AM, Felipe Balbi wrote:
> Hi,
>
> On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:
>> On 21.10.2014 11:40, Sebastian Hesselbarth wrote:
>>> On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:
>>>> On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
>>>>> Currently, Berlin SATA PHY driver assumes PHY_BASE address being
>>>>> constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
>>>>> is different. Prepare the driver for BG2 support by moving the phy_base
>>>>> into private driver data.
>>>>>
>>>>> Acked-by: Antoine T?nart <[email protected]>
>>>>> Signed-off-by: Sebastian Hesselbarth <[email protected]>
>>> ...
>>>>> ---
>>>>> drivers/phy/phy-berlin-sata.c | 42
>>>>> ++++++++++++++++++++++++++++--------------
>>>>> 1 file changed, 28 insertions(+), 14 deletions(-)
>>>>>
>>>>> diff --git a/drivers/phy/phy-berlin-sata.c
>>>>> b/drivers/phy/phy-berlin-sata.c
>>>>> index 69ced52d72aa..9682b0f66177 100644
>>>>> --- a/drivers/phy/phy-berlin-sata.c
>>>>> +++ b/drivers/phy/phy-berlin-sata.c
>>>>> @@ -30,7 +30,7 @@
>>>>> #define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
>>>>> #define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)
>>>>>
>>>>> -#define PHY_BASE 0x200
>>>>> +#define BG2Q_PHY_BASE 0x200
>>> [...]
>>>>> +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
>>>>> +
>>>>> +static const struct of_device_id phy_berlin_sata_of_match[] = {
>>>>> + {
>>>>> + .compatible = "marvell,berlin2q-sata-phy",
>>>>> + .data = &bg2q_sata_phy_base,
>>>>
>>>> Can't the base directly come from dt?
>>>
>>> You are suggesting a "marvell,phy-base-address" property, right?
>>> I have no strong opinion about it, I accept your call (or DT maintainer
>>> ones).
>>
>> Kishon,
>>
>> I still have the DT patches for BG2Q queued up for v3.19 (I missed the
>> arm-soc merge window for v3.18). That means, there has been no release
>> with the phy binding used and I can rework a little more.
>>
>> Can you please confirm that you want a DT property for the phy base address,
>> e.g. marvell,phy-base-address = <{0x200,0x80}> ?
>>
>> If so, I'd also rename the compatible from berlin2q-sata-phy to more
>> generic berlin-sata-phy.
>
> I think what Kishon is asking, is why this 0x200 offset isn't already on
> reg. so that instead of, e.g.:
>
> reg = <0x40000000 0x1000>;
>
> you would have:
>
> reg = <0x40000200 0x1000>;

I had something similar to what Sebastian suggested in mind. I think phy_base
is used for a different reason and can't be directly used in 'reg'.

Thanks
Kishon

2014-10-27 18:33:07

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

On 10/27/2014 01:27 PM, Kishon Vijay Abraham I wrote:
> On Saturday 25 October 2014 01:55 AM, Felipe Balbi wrote:
>> On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:
>>> On 21.10.2014 11:40, Sebastian Hesselbarth wrote:
>>>> On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:
>>>>> On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
>>>>>> Currently, Berlin SATA PHY driver assumes PHY_BASE address being
>>>>>> constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
>>>>>> is different. Prepare the driver for BG2 support by moving the phy_base
>>>>>> into private driver data.
>>>>>>
>>>>>> Acked-by: Antoine T?nart <[email protected]>
>>>>>> Signed-off-by: Sebastian Hesselbarth <[email protected]>
>>>> ...
>>>>>> ---
>>>>>> drivers/phy/phy-berlin-sata.c | 42
>>>>>> ++++++++++++++++++++++++++++--------------
>>>>>> 1 file changed, 28 insertions(+), 14 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/phy/phy-berlin-sata.c
>>>>>> b/drivers/phy/phy-berlin-sata.c
>>>>>> index 69ced52d72aa..9682b0f66177 100644
>>>>>> --- a/drivers/phy/phy-berlin-sata.c
>>>>>> +++ b/drivers/phy/phy-berlin-sata.c
>>>>>> @@ -30,7 +30,7 @@
>>>>>> #define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
>>>>>> #define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)
>>>>>>
>>>>>> -#define PHY_BASE 0x200
>>>>>> +#define BG2Q_PHY_BASE 0x200
>>>> [...]
>>>>>> +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
>>>>>> +
>>>>>> +static const struct of_device_id phy_berlin_sata_of_match[] = {
>>>>>> + {
>>>>>> + .compatible = "marvell,berlin2q-sata-phy",
>>>>>> + .data = &bg2q_sata_phy_base,
>>>>>
>>>>> Can't the base directly come from dt?
>>>>
>>>> You are suggesting a "marvell,phy-base-address" property, right?
>>>> I have no strong opinion about it, I accept your call (or DT maintainer
>>>> ones).
>>>
>>> I still have the DT patches for BG2Q queued up for v3.19 (I missed the
>>> arm-soc merge window for v3.18). That means, there has been no release
>>> with the phy binding used and I can rework a little more.
>>>
>>> Can you please confirm that you want a DT property for the phy base address,
>>> e.g. marvell,phy-base-address = <{0x200,0x80}> ?
>>>
>>> If so, I'd also rename the compatible from berlin2q-sata-phy to more
>>> generic berlin-sata-phy.
>>
>> I think what Kishon is asking, is why this 0x200 offset isn't already on
>> reg. so that instead of, e.g.:
>>
>> reg = <0x40000000 0x1000>;
>>
>> you would have:
>>
>> reg = <0x40000200 0x1000>;
>
> I had something similar to what Sebastian suggested in mind. I think phy_base
> is used for a different reason and can't be directly used in 'reg'.

Kishon,

thanks for the clarification. While the extra marvell,phy-base-address
property basically works and I agree with it, I may have some
_potential_ draw-backs:

The Marvell BSP code (which I have no clue _why_ it does what it does
or if it is required) has some magic writes to "improve" serial signal
quality. I left them out as my HDD was detected with and without them.

Now, if we find that they are required, we have to find a way to make
the PHY driver know about the PHY revision. We'd usually add a
different compatible and deal with it accordingly.

So, not adding the compatible now _may_ just postpone a follow-up patch
for the different PHY setup of BG2 and render the new phy_base property
basically useless.

If you are just unhappy with the "static u32 bg2q_sata_phy_base"
assigned to of_device_id.data, I can convert that to Felipe's proposal.

Sebastian

2014-10-30 05:27:36

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] phy: berlin-sata: Move PHY_BASE into private data struct



On Tuesday 28 October 2014 12:02 AM, Sebastian Hesselbarth wrote:
> On 10/27/2014 01:27 PM, Kishon Vijay Abraham I wrote:
>> On Saturday 25 October 2014 01:55 AM, Felipe Balbi wrote:
>>> On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:
>>>> On 21.10.2014 11:40, Sebastian Hesselbarth wrote:
>>>>> On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:
>>>>>> On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
>>>>>>> Currently, Berlin SATA PHY driver assumes PHY_BASE address being
>>>>>>> constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
>>>>>>> is different. Prepare the driver for BG2 support by moving the phy_base
>>>>>>> into private driver data.
>>>>>>>
>>>>>>> Acked-by: Antoine T?nart <[email protected]>
>>>>>>> Signed-off-by: Sebastian Hesselbarth <[email protected]>
>>>>> ...
>>>>>>> ---
>>>>>>> drivers/phy/phy-berlin-sata.c | 42
>>>>>>> ++++++++++++++++++++++++++++--------------
>>>>>>> 1 file changed, 28 insertions(+), 14 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/phy/phy-berlin-sata.c
>>>>>>> b/drivers/phy/phy-berlin-sata.c
>>>>>>> index 69ced52d72aa..9682b0f66177 100644
>>>>>>> --- a/drivers/phy/phy-berlin-sata.c
>>>>>>> +++ b/drivers/phy/phy-berlin-sata.c
>>>>>>> @@ -30,7 +30,7 @@
>>>>>>> #define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
>>>>>>> #define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)
>>>>>>>
>>>>>>> -#define PHY_BASE 0x200
>>>>>>> +#define BG2Q_PHY_BASE 0x200
>>>>> [...]
>>>>>>> +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
>>>>>>> +
>>>>>>> +static const struct of_device_id phy_berlin_sata_of_match[] = {
>>>>>>> + {
>>>>>>> + .compatible = "marvell,berlin2q-sata-phy",
>>>>>>> + .data = &bg2q_sata_phy_base,
>>>>>>
>>>>>> Can't the base directly come from dt?
>>>>>
>>>>> You are suggesting a "marvell,phy-base-address" property, right?
>>>>> I have no strong opinion about it, I accept your call (or DT maintainer
>>>>> ones).
>>>>
>>>> I still have the DT patches for BG2Q queued up for v3.19 (I missed the
>>>> arm-soc merge window for v3.18). That means, there has been no release
>>>> with the phy binding used and I can rework a little more.
>>>>
>>>> Can you please confirm that you want a DT property for the phy base address,
>>>> e.g. marvell,phy-base-address = <{0x200,0x80}> ?
>>>>
>>>> If so, I'd also rename the compatible from berlin2q-sata-phy to more
>>>> generic berlin-sata-phy.
>>>
>>> I think what Kishon is asking, is why this 0x200 offset isn't already on
>>> reg. so that instead of, e.g.:
>>>
>>> reg = <0x40000000 0x1000>;
>>>
>>> you would have:
>>>
>>> reg = <0x40000200 0x1000>;
>>
>> I had something similar to what Sebastian suggested in mind. I think phy_base
>> is used for a different reason and can't be directly used in 'reg'.
>
> Kishon,
>
> thanks for the clarification. While the extra marvell,phy-base-address
> property basically works and I agree with it, I may have some
> _potential_ draw-backs:
>
> The Marvell BSP code (which I have no clue _why_ it does what it does
> or if it is required) has some magic writes to "improve" serial signal
> quality. I left them out as my HDD was detected with and without them.
>
> Now, if we find that they are required, we have to find a way to make
> the PHY driver know about the PHY revision. We'd usually add a
> different compatible and deal with it accordingly.
>
> So, not adding the compatible now _may_ just postpone a follow-up patch
> for the different PHY setup of BG2 and render the new phy_base property
> basically useless.
>
> If you are just unhappy with the "static u32 bg2q_sata_phy_base"
> assigned to of_device_id.data, I can convert that to Felipe's proposal.

Either way is fine with me.

Thanks
Kishon

2014-10-30 10:21:44

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v3 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

Currently, Berlin SATA PHY driver assumes PHY_BASE address being
constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
is different. Prepare the driver for BG2 support by moving the phy_base
into private driver data.

Acked-by: Antoine Ténart <[email protected]>
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Changelog:
v1->v2:
- None
v2->v3:
- Do not use of_device_id.data for phy_base (Suggested by
Kishon Vijay Abraham I)

Cc: Kishon Vijay Abraham I <[email protected]>
Cc: "Antoine Ténart" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/phy/phy-berlin-sata.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
index 69ced52d72aa..cdb46d1203a4 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -30,7 +30,7 @@
#define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
#define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)

-#define PHY_BASE 0x200
+#define BG2Q_PHY_BASE 0x200

/* register 0x01 */
#define REF_FREF_SEL_25 BIT(0)
@@ -61,15 +61,16 @@ struct phy_berlin_priv {
struct clk *clk;
struct phy_berlin_desc **phys;
unsigned nphys;
+ u32 phy_base;
};

-static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg, u32 reg,
- u32 mask, u32 val)
+static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg,
+ u32 phy_base, u32 reg, u32 mask, u32 val)
{
u32 regval;

/* select register */
- writel(PHY_BASE + reg, ctrl_reg + PORT_VSR_ADDR);
+ writel(phy_base + reg, ctrl_reg + PORT_VSR_ADDR);

/* set bits */
regval = readl(ctrl_reg + PORT_VSR_DATA);
@@ -103,17 +104,20 @@ static int phy_berlin_sata_power_on(struct phy *phy)
writel(regval, priv->base + HOST_VSA_DATA);

/* set PHY mode and ref freq to 25 MHz */
- phy_berlin_sata_reg_setbits(ctrl_reg, 0x1, 0xff,
- REF_FREF_SEL_25 | PHY_MODE_SATA);
+ phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x01,
+ 0x00ff, REF_FREF_SEL_25 | PHY_MODE_SATA);

/* set PHY up to 6 Gbps */
- phy_berlin_sata_reg_setbits(ctrl_reg, 0x25, 0xc00, PHY_GEN_MAX_6_0);
+ phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x25,
+ 0x0c00, PHY_GEN_MAX_6_0);

/* set 40 bits width */
- phy_berlin_sata_reg_setbits(ctrl_reg, 0x23, 0xc00, DATA_BIT_WIDTH_40);
+ phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x23,
+ 0x0c00, DATA_BIT_WIDTH_40);

/* use max pll rate */
- phy_berlin_sata_reg_setbits(ctrl_reg, 0x2, 0x0, USE_MAX_PLL_RATE);
+ phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x02,
+ 0x0000, USE_MAX_PLL_RATE);

/* set Gen3 controller speed */
regval = readl(ctrl_reg + PORT_SCR_CTL);
@@ -218,6 +222,8 @@ static int phy_berlin_sata_probe(struct platform_device *pdev)
if (!priv->phys)
return -ENOMEM;

+ priv->phy_base = BG2Q_PHY_BASE;
+
dev_set_drvdata(dev, priv);
spin_lock_init(&priv->lock);

--
2.1.1

2014-10-30 10:21:48

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v3 4/5] ARM: berlin: Add AHCI and SATA PHY nodes to BG2

Add DT nodes for the AHCI controller and SATA PHY found on Marvell
Berlin2 SoCs.

Acked-by: Antoine Ténart <[email protected]>
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Changelog:
v1->v2:
- remove status = "disabled" from SATA controller node (Suggested by
Antoine)

Cc: Kishon Vijay Abraham I <[email protected]>
Cc: "Antoine Ténart" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arm/boot/dts/berlin2.dtsi | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi
index 9d7c810ebd0b..f39090491eb2 100644
--- a/arch/arm/boot/dts/berlin2.dtsi
+++ b/arch/arm/boot/dts/berlin2.dtsi
@@ -246,6 +246,45 @@
};
};

+ ahci: sata@e90000 {
+ compatible = "marvell,berlin2-ahci", "generic-ahci";
+ reg = <0xe90000 0x1000>;
+ interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&chip CLKID_SATA>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ sata0: sata-port@0 {
+ reg = <0>;
+ phys = <&sata_phy 0>;
+ status = "disabled";
+ };
+
+ sata1: sata-port@1 {
+ reg = <1>;
+ phys = <&sata_phy 1>;
+ status = "disabled";
+ };
+ };
+
+ sata_phy: phy@e900a0 {
+ compatible = "marvell,berlin2-sata-phy";
+ reg = <0xe900a0 0x200>;
+ clocks = <&chip CLKID_SATA>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #phy-cells = <1>;
+ status = "disabled";
+
+ sata-phy@0 {
+ reg = <0>;
+ };
+
+ sata-phy@1 {
+ reg = <1>;
+ };
+ };
+
chip: chip-control@ea0000 {
compatible = "marvell,berlin2-chip-ctrl";
#clock-cells = <1>;
--
2.1.1

2014-10-30 10:21:43

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v3 0/5] Berlin BG2 AHCI and SATA PHY

This patch set is v3 of the catch up with latest Berlin improvements
provided by Antoine - in particular SATA PHY support and AHCI generic
for Berlin BG2.

Marvell BSP code for BG2 suggests more differences between the two
PHY revisions found on BG2 and BG2Q, but the only important one seems
to be the PHY_BASE used in AHCI vendor-specific registers. I also
confirmed that power_off does indeed power off the PHY on BG2, too
(It wasn't very clear in BSP code).

Anyway, I have tested this on BG2-based Sony NSZ-GS7 and attached
SATA HDD is successfully detected and partitions are displayed.

This patch set is based on v3.18-rc1. As usual a branch based on
v3.18-rc1 can be found at

git://git.infradead.org/users/hesselba/linux-berlin.git devel/bg2-sata-v3

Compared to v1, there is only a minor change that removes an unrequired
status = "disabled" property from BG2 SATA node.
Compared to v2, BG2 PHY still uses its own compatible but the driver now
uses of_device_is_compatible instead of of_device_id.data for setting the
base address. This is the outcome of some discussion with Kishon and Felipe,
where we (hopefully) agreed on this approach. It will allow future SoC-
specific PHY setup routines if required.

Patches 1-3 should go through Kishon's PHY tree, I pick up DT patches 4
and 5.

Patch 1 prepares phy-berlin-sata to support different PHY_BASE addresses
by moving the constant to driver private data.

Patches 2 and 3 add a new compatible to driver and DT documentation that
reflects the differences between BG2Q and BG2 SATA PHY.

Patches 4 and 5 finally add DT nodes to both Berlin2 SoC dtsi and Sony
NSZ-GS7 board DT file. SATA plug on NSZ-GS7 is unpopulated but can be
very easily equipped with SATA receptable and some 0402 caps. I decided
to enable SATA by default although not all users may populate it.

Sebastian Hesselbarth (5):
phy: berlin-sata: Move PHY_BASE into private data struct
phy: berlin-sata: Add support for BG2 SATA PHY
phy: berlin-sata: Document BG2 compatible
ARM: berlin: Add AHCI and SATA PHY nodes to BG2
ARM: berlin: Enable SATA on Sony NSZ-GS7

.../devicetree/bindings/phy/berlin-sata-phy.txt | 4 ++-
arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts | 7 ++++
arch/arm/boot/dts/berlin2.dtsi | 39 ++++++++++++++++++++++
drivers/phy/phy-berlin-sata.c | 29 +++++++++++-----
4 files changed, 69 insertions(+), 10 deletions(-)

---
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: "Antoine Ténart" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
--
2.1.1

2014-10-30 10:22:21

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v3 5/5] ARM: berlin: Enable SATA on Sony NSZ-GS7

Marvell Berlin BG2 based Sony NSZ-GS7 has an unpopulated SATA plug
on its PCB solder side. As it is quite easy to populate and I have
done it, enable AHCI and SATA by default.

Acked-by: Antoine Ténart <[email protected]>
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: "Antoine Ténart" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts b/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts
index c72bfd468d10..0a13e8a5da15 100644
--- a/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts
+++ b/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts
@@ -26,4 +26,11 @@
};
};

+&ahci { status = "okay"; };
+
+/* Unpopulated SATA plug on solder side */
+&sata0 { status = "okay"; };
+
+&sata_phy { status = "okay"; };
+
&uart0 { status = "okay"; };
--
2.1.1

2014-10-30 10:23:03

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v3 2/5] phy: berlin-sata: Add support for BG2 SATA PHY

Berlin BG2 also has a SATA PHY compatible with the current driver
except different PHY_BASE. Add a new compatible to the driver
reflecting the different PHY_BASE.

Acked-by: Antoine Ténart <[email protected]>
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Changelog:
v1->v2:
- none
v2->v3:
- Use of_device_is_compatible for different PHY_BASE (Suggested by
Felipe Balbi)

Cc: Kishon Vijay Abraham I <[email protected]>
Cc: "Antoine Ténart" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/phy/phy-berlin-sata.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
index cdb46d1203a4..873e7a890fee 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -30,6 +30,7 @@
#define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
#define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)

+#define BG2_PHY_BASE 0x080
#define BG2Q_PHY_BASE 0x200

/* register 0x01 */
@@ -222,7 +223,10 @@ static int phy_berlin_sata_probe(struct platform_device *pdev)
if (!priv->phys)
return -ENOMEM;

- priv->phy_base = BG2Q_PHY_BASE;
+ if (of_device_is_compatible(dev->of_node, "marvell,berlin2-sata-phy"))
+ priv->phy_base = BG2_PHY_BASE;
+ else
+ priv->phy_base = BG2Q_PHY_BASE;

dev_set_drvdata(dev, priv);
spin_lock_init(&priv->lock);
@@ -271,6 +275,7 @@ static int phy_berlin_sata_probe(struct platform_device *pdev)
}

static const struct of_device_id phy_berlin_sata_of_match[] = {
+ { .compatible = "marvell,berlin2-sata-phy" },
{ .compatible = "marvell,berlin2q-sata-phy" },
{ },
};
--
2.1.1

2014-10-30 10:27:28

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v3 3/5] phy: berlin-sata: Document BG2 compatible

Berlin BG2 SATA PHY is slightly different from currently supported
BG2Q SATA PHY. Document the new compatible for BG2's PHY.

Acked-by: Antoine Ténart <[email protected]>
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: "Antoine Ténart" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
Documentation/devicetree/bindings/phy/berlin-sata-phy.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt b/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
index 88f8c23384c0..c0155f842f62 100644
--- a/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
+++ b/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
@@ -2,7 +2,9 @@ Berlin SATA PHY
---------------

Required properties:
-- compatible: should be "marvell,berlin2q-sata-phy"
+- compatible: should be one of
+ "marvell,berlin2-sata-phy"
+ "marvell,berlin2q-sata-phy"
- address-cells: should be 1
- size-cells: should be 0
- phy-cells: from the generic PHY bindings, must be 1
--
2.1.1

2014-11-11 23:23:14

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] ARM: berlin: Add AHCI and SATA PHY nodes to BG2

On 30.10.2014 11:21, Sebastian Hesselbarth wrote:
> Add DT nodes for the AHCI controller and SATA PHY found on Marvell
> Berlin2 SoCs.
>
> Acked-by: Antoine Ténart <[email protected]>
> Signed-off-by: Sebastian Hesselbarth <[email protected]>

Applied both DT patches to berlin/dt.

Sebastian

> ---
> arch/arm/boot/dts/berlin2.dtsi | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi
> index 9d7c810ebd0b..f39090491eb2 100644
> --- a/arch/arm/boot/dts/berlin2.dtsi
> +++ b/arch/arm/boot/dts/berlin2.dtsi
> @@ -246,6 +246,45 @@
> };
> };
>
> + ahci: sata@e90000 {
> + compatible = "marvell,berlin2-ahci", "generic-ahci";
> + reg = <0xe90000 0x1000>;
> + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&chip CLKID_SATA>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sata0: sata-port@0 {
> + reg = <0>;
> + phys = <&sata_phy 0>;
> + status = "disabled";
> + };
> +
> + sata1: sata-port@1 {
> + reg = <1>;
> + phys = <&sata_phy 1>;
> + status = "disabled";
> + };
> + };
> +
> + sata_phy: phy@e900a0 {
> + compatible = "marvell,berlin2-sata-phy";
> + reg = <0xe900a0 0x200>;
> + clocks = <&chip CLKID_SATA>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #phy-cells = <1>;
> + status = "disabled";
> +
> + sata-phy@0 {
> + reg = <0>;
> + };
> +
> + sata-phy@1 {
> + reg = <1>;
> + };
> + };
> +
> chip: chip-control@ea0000 {
> compatible = "marvell,berlin2-chip-ctrl";
> #clock-cells = <1>;
>