2019-03-07 08:44:26

by Christoph Muellner

[permalink] [raw]
Subject: [PATCH v2 1/4] phy: rockchip-emmc: Allow to set drive impedance via DTS.

The rockchip-emmc PHY can be configured with different
drive impedance values. Currenlty a value of 50 Ohm is
hard coded into the driver.

This patch introduces the DTS property 'drive-impedance-ohm'
for the rockchip-emmc phy node, which uses the value from the DTS
to setup the drive impedance accordingly.

Signed-off-by: Christoph Muellner <[email protected]>
Signed-off-by: Philipp Tomsich <[email protected]>
---
drivers/phy/rockchip/phy-rockchip-emmc.c | 30 ++++++++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-emmc.c b/drivers/phy/rockchip/phy-rockchip-emmc.c
index 19bf84f0bc67..b804c6bf4b55 100644
--- a/drivers/phy/rockchip/phy-rockchip-emmc.c
+++ b/drivers/phy/rockchip/phy-rockchip-emmc.c
@@ -87,6 +87,7 @@ struct rockchip_emmc_phy {
unsigned int reg_offset;
struct regmap *reg_base;
struct clk *emmcclk;
+ unsigned int drive_impedance;
};

static int rockchip_emmc_phy_power(struct phy *phy, bool on_off)
@@ -281,10 +282,10 @@ static int rockchip_emmc_phy_power_on(struct phy *phy)
{
struct rockchip_emmc_phy *rk_phy = phy_get_drvdata(phy);

- /* Drive impedance: 50 Ohm */
+ /* Drive impedance: from DTS */
regmap_write(rk_phy->reg_base,
rk_phy->reg_offset + GRF_EMMCPHY_CON6,
- HIWORD_UPDATE(PHYCTRL_DR_50OHM,
+ HIWORD_UPDATE(rk_phy->drive_impedance,
PHYCTRL_DR_MASK,
PHYCTRL_DR_SHIFT));

@@ -314,6 +315,26 @@ static const struct phy_ops ops = {
.owner = THIS_MODULE,
};

+static u32 convert_drive_impedance_ohm(struct platform_device *pdev, u32 dr_ohm)
+{
+ switch (dr_ohm) {
+ case 100:
+ return PHYCTRL_DR_100OHM;
+ case 66:
+ return PHYCTRL_DR_66OHM;
+ case 50:
+ return PHYCTRL_DR_50OHM;
+ case 40:
+ return PHYCTRL_DR_40OHM;
+ case 33:
+ return PHYCTRL_DR_33OHM;
+ }
+
+ dev_warn(&pdev->dev, "Invalid value %u for drive-impedance-ohm.\n",
+ dr_ohm);
+ return PHYCTRL_DR_50OHM;
+}
+
static int rockchip_emmc_phy_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -322,6 +343,7 @@ static int rockchip_emmc_phy_probe(struct platform_device *pdev)
struct phy_provider *phy_provider;
struct regmap *grf;
unsigned int reg_offset;
+ u32 val;

if (!dev->parent || !dev->parent->of_node)
return -ENODEV;
@@ -344,6 +366,10 @@ static int rockchip_emmc_phy_probe(struct platform_device *pdev)

rk_phy->reg_offset = reg_offset;
rk_phy->reg_base = grf;
+ rk_phy->drive_impedance = PHYCTRL_DR_50OHM;
+
+ if (!of_property_read_u32(dev->of_node, "drive-impedance-ohm", &val))
+ rk_phy->drive_impedance = convert_drive_impedance_ohm(pdev, val);

generic_phy = devm_phy_create(dev, dev->of_node, &ops);
if (IS_ERR(generic_phy)) {
--
2.11.0



2019-03-07 08:43:48

by Christoph Muellner

[permalink] [raw]
Subject: [PATCH v2 3/4] arm64: dts: rockchip: Define drive-impedance-ohm for RK3399's emmc-phy.

A previous patch introduced the property 'drive-impedance-ohm'
for the RK3399's emmc phy node. This patch sets this value
explicitly to the default value of 50 Ohm.

Signed-off-by: Christoph Muellner <[email protected]>
Signed-off-by: Philipp Tomsich <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 6cc1c9fa4ea6..b875364a7709 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1450,6 +1450,7 @@
clock-names = "refclk";
#phy-cells = <1>;
resets = <&cru SRST_PCIEPHY>;
+ drive-impedance-ohm = <50>;
reset-names = "phy";
status = "disabled";
};
--
2.11.0


2019-03-07 08:44:17

by Christoph Muellner

[permalink] [raw]
Subject: [PATCH v2 2/4] dt-bindings: phy: Add a new property drive-impedance-ohm for RK's emmc PHY

This patch documents the new proprty drive-impedance-ohm for
Rockchip's eMMC PHY node.

Signed-off-by: Christoph Muellner <[email protected]>
Signed-off-by: Philipp Tomsich <[email protected]>
---
Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt | 3 +++
1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
index e3ea55763b0a..5f5d999dacc3 100644
--- a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
+++ b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
@@ -13,6 +13,9 @@ specified by name:
(because most boards can get basic functionality without having
access to it), it is strongly suggested.
- clocks: Should have a phandle to the card clock exported by the SDHCI driver.
+ - drive-impedance-ohm: Specifies the drive impedance in Ohm.
+ Possible values are 33, 40, 50, 66 and 100.
+ If not set, the default value of 50 will be applied.

Example:

--
2.11.0


2019-03-07 08:45:36

by Christoph Muellner

[permalink] [raw]
Subject: [PATCH v2 4/4] arm64: dts: rockchip: Decrease emmc-phy's drive impedance on rk3399-puma

The RK3399-Q7 (Puma) requires 33 Ohm drive strength to ensure signal
integrity at HS-400 (200MHz clock, DDR signalling).

A repeated EMC testing run validates that this increase does not
negatively impact EMC compliance (emissions have ample distance to
the regulatory limits).

Signed-off-by: Christoph Muellner <[email protected]>
Signed-off-by: Philipp Tomsich <[email protected]>
Tested-by: Jakob Unterwurzacher <[email protected]>
Tested-by: Klaus Goger <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
index 0130b9f98c9d..4f75bb6b2f14 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
@@ -146,6 +146,7 @@

&emmc_phy {
status = "okay";
+ drive-impedance-ohm = <33>;
};

&gmac {
--
2.11.0


2019-03-12 12:53:59

by Heiko Stübner

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: phy: Add a new property drive-impedance-ohm for RK's emmc PHY

Hi Christoph,

Am Donnerstag, 7. M?rz 2019, 09:41:54 CET schrieb Christoph Muellner:
> This patch documents the new proprty drive-impedance-ohm for
> Rockchip's eMMC PHY node.
>
> Signed-off-by: Christoph Muellner <[email protected]>
> Signed-off-by: Philipp Tomsich <[email protected]>
> ---
> Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
> index e3ea55763b0a..5f5d999dacc3 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
> @@ -13,6 +13,9 @@ specified by name:

you might want to adapt the section headline as well, as right now it
talks about "Optional clocks", but should likely become the generic
"Optional properties" as in other bindings (including moving the clock-
binding.txt reference down into the clock elements)


Heiko

> (because most boards can get basic functionality without having
> access to it), it is strongly suggested.
> - clocks: Should have a phandle to the card clock exported by the SDHCI driver.
> + - drive-impedance-ohm: Specifies the drive impedance in Ohm.
> + Possible values are 33, 40, 50, 66 and 100.
> + If not set, the default value of 50 will be applied.
>
> Example:
>
>





2019-03-12 13:03:01

by Christoph Muellner

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: phy: Add a new property drive-impedance-ohm for RK's emmc PHY


> On 12.03.2019, at 13:52, Heiko Stuebner <[email protected]> wrote:
>
> Hi Christoph,
>
> Am Donnerstag, 7. März 2019, 09:41:54 CET schrieb Christoph Muellner:
>> This patch documents the new proprty drive-impedance-ohm for
>> Rockchip's eMMC PHY node.
>>
>> Signed-off-by: Christoph Muellner <[email protected]>
>> Signed-off-by: Philipp Tomsich <[email protected]>
>> ---
>> Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
>> index e3ea55763b0a..5f5d999dacc3 100644
>> --- a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
>> +++ b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
>> @@ -13,6 +13,9 @@ specified by name:
>
> you might want to adapt the section headline as well, as right now it
> talks about "Optional clocks", but should likely become the generic
> "Optional properties" as in other bindings (including moving the clock-
> binding.txt reference down into the clock elements)

Thank's for spotting this.
Will do for v3.

Thanks,
Christoph

>
>
> Heiko
>
>> (because most boards can get basic functionality without having
>> access to it), it is strongly suggested.
>> - clocks: Should have a phandle to the card clock exported by the SDHCI driver.
>> + - drive-impedance-ohm: Specifies the drive impedance in Ohm.
>> + Possible values are 33, 40, 50, 66 and 100.
>> + If not set, the default value of 50 will be applied.
>>
>> Example:
>>
>>
>
>
>
>


2019-03-12 13:08:51

by Heiko Stübner

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] phy: rockchip-emmc: Allow to set drive impedance via DTS.

Am Donnerstag, 7. M?rz 2019, 09:41:53 CET schrieb Christoph Muellner:
> The rockchip-emmc PHY can be configured with different
> drive impedance values. Currenlty a value of 50 Ohm is
> hard coded into the driver.
>
> This patch introduces the DTS property 'drive-impedance-ohm'
> for the rockchip-emmc phy node, which uses the value from the DTS
> to setup the drive impedance accordingly.
>
> Signed-off-by: Christoph Muellner <[email protected]>
> Signed-off-by: Philipp Tomsich <[email protected]>

change itself looks good
Reviewed-by: Heiko Stuebner <[email protected]>

Though I cannot say too much about the electrical side.


Heiko

> ---
> drivers/phy/rockchip/phy-rockchip-emmc.c | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-emmc.c b/drivers/phy/rockchip/phy-rockchip-emmc.c
> index 19bf84f0bc67..b804c6bf4b55 100644
> --- a/drivers/phy/rockchip/phy-rockchip-emmc.c
> +++ b/drivers/phy/rockchip/phy-rockchip-emmc.c
> @@ -87,6 +87,7 @@ struct rockchip_emmc_phy {
> unsigned int reg_offset;
> struct regmap *reg_base;
> struct clk *emmcclk;
> + unsigned int drive_impedance;
> };
>
> static int rockchip_emmc_phy_power(struct phy *phy, bool on_off)
> @@ -281,10 +282,10 @@ static int rockchip_emmc_phy_power_on(struct phy *phy)
> {
> struct rockchip_emmc_phy *rk_phy = phy_get_drvdata(phy);
>
> - /* Drive impedance: 50 Ohm */
> + /* Drive impedance: from DTS */
> regmap_write(rk_phy->reg_base,
> rk_phy->reg_offset + GRF_EMMCPHY_CON6,
> - HIWORD_UPDATE(PHYCTRL_DR_50OHM,
> + HIWORD_UPDATE(rk_phy->drive_impedance,
> PHYCTRL_DR_MASK,
> PHYCTRL_DR_SHIFT));
>
> @@ -314,6 +315,26 @@ static const struct phy_ops ops = {
> .owner = THIS_MODULE,
> };
>
> +static u32 convert_drive_impedance_ohm(struct platform_device *pdev, u32 dr_ohm)
> +{
> + switch (dr_ohm) {
> + case 100:
> + return PHYCTRL_DR_100OHM;
> + case 66:
> + return PHYCTRL_DR_66OHM;
> + case 50:
> + return PHYCTRL_DR_50OHM;
> + case 40:
> + return PHYCTRL_DR_40OHM;
> + case 33:
> + return PHYCTRL_DR_33OHM;
> + }
> +
> + dev_warn(&pdev->dev, "Invalid value %u for drive-impedance-ohm.\n",
> + dr_ohm);
> + return PHYCTRL_DR_50OHM;
> +}
> +
> static int rockchip_emmc_phy_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -322,6 +343,7 @@ static int rockchip_emmc_phy_probe(struct platform_device *pdev)
> struct phy_provider *phy_provider;
> struct regmap *grf;
> unsigned int reg_offset;
> + u32 val;
>
> if (!dev->parent || !dev->parent->of_node)
> return -ENODEV;
> @@ -344,6 +366,10 @@ static int rockchip_emmc_phy_probe(struct platform_device *pdev)
>
> rk_phy->reg_offset = reg_offset;
> rk_phy->reg_base = grf;
> + rk_phy->drive_impedance = PHYCTRL_DR_50OHM;
> +
> + if (!of_property_read_u32(dev->of_node, "drive-impedance-ohm", &val))
> + rk_phy->drive_impedance = convert_drive_impedance_ohm(pdev, val);
>
> generic_phy = devm_phy_create(dev, dev->of_node, &ops);
> if (IS_ERR(generic_phy)) {
>