2024-03-05 02:20:23

by Yang Xiwen via B4 Relay

[permalink] [raw]
Subject: [PATCH v4 0/5] phy: hisi-inno-phy: add support for hi3798mv200-usb2-phy

This should be considered a hack. The proper solution would be
extracting write_reg logic to a separate regmap driver. Leaving only
"write BIT(2) to address 0x6" to the PHY driver.

A proper fix should be implemented later.

Signed-off-by: Yang Xiwen <[email protected]>
---
Changes in v4:
- remove reference to histb-clock.h
- remove fallback compatible as it has no use.
- remove phy_type (belongs to host controller)
- fix bot error (Rob Herring)
- split YAML convertion into two commits, the other add mv100 compatible (Krzysztof Kozlowski)
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- address a few binding issue mistakenly missing in v2 (Krzysztof Kozlowski)
- add msg about hi3798mv100 being added to compatible list
- remove minItems for compatible
- remove | for reg:
- fix existing dts (hi3798cv200.dtsi) due to binding change.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- rewrite commit msg to show why hisilicon,hi3798mv100-usb2-phy is added during YAML convertion.
- split required: to multiple line
- add allOf to wrap if:
- remove perictrl wrapper and the second phy in the example
- tested the binding both for mv200 and cv200 dts. fix some silly errors.
- remove Pengcheng Li from To:
Above all are suggested by Krzysztof
- use reset_control_array_* APIs to ensure all resets are controlled
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Yang Xiwen (5):
phy: hisilicon: hisi-inno-phy: enable clocks for every ports
dt-bindings: phy: hisi-inno-usb2: convert to YAML
dt-bindings: phy: hisilicon,inno-usb2-phy: add support for Hi3798MV100 INNO PHY
dt-bindings: phy: hisi-inno-usb2: add compatible of hisilicon,hi3798mv200-usb2-phy
phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY

.../bindings/phy/hisilicon,inno-usb2-phy.yaml | 119 +++++++++++++++++++++
.../devicetree/bindings/phy/phy-hisi-inno-usb2.txt | 71 ------------
drivers/phy/hisilicon/phy-hisi-inno-usb2.c | 70 +++++++-----
3 files changed, 162 insertions(+), 98 deletions(-)
---
base-commit: 8d3dea210042f54b952b481838c1e7dfc4ec751d
change-id: 20240216-inno-phy-a2d872f6b74b

Best regards,
--
Yang Xiwen <[email protected]>



2024-03-05 02:20:26

by Yang Xiwen via B4 Relay

[permalink] [raw]
Subject: [PATCH v4 2/5] dt-bindings: phy: hisi-inno-usb2: convert to YAML

From: Yang Xiwen <[email protected]>

convert the legacy text binding to YAML. While at it, remove
"hisilicon,inno-usb2-phy" fallback compatible.

Signed-off-by: Yang Xiwen <[email protected]>
---
.../bindings/phy/hisilicon,inno-usb2-phy.yaml | 91 ++++++++++++++++++++++
.../devicetree/bindings/phy/phy-hisi-inno-usb2.txt | 71 -----------------
2 files changed, 91 insertions(+), 71 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml b/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml
new file mode 100644
index 000000000000..ba82405ddf51
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/hisilicon,inno-usb2-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HiSilicon HiSTB SoCs INNO USB2 PHY device
+
+maintainers:
+ - Yang Xiwen <[email protected]>
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - hisilicon,hi3798cv200-usb2-phy
+
+ reg:
+ maxItems: 1
+ description: Should be the address space for PHY configuration register in
+ peripheral controller, e.g. PERI_USB0 for USB 2.0 PHY01 on Hi3798CV200 SoC,
+ or direct MMIO address space.
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ clocks:
+ maxItems: 1
+ description: reference clock
+
+ resets:
+ maxItems: 1
+
+patternProperties:
+ 'phy@[0-9a-f]+':
+ type: object
+ additionalProperties: false
+ description: individual ports provided by INNO PHY
+
+ properties:
+ reg:
+ maxItems: 1
+ description: should be the port index (if under a perictrl node)
+ or port address space
+
+ '#phy-cells':
+ const: 0
+
+ resets:
+ maxItems: 1
+
+ required:
+ - reg
+ - '#phy-cells'
+ - resets
+
+required:
+ - compatible
+ - clocks
+ - reg
+ - '#address-cells'
+ - '#size-cells'
+ - resets
+
+additionalProperties: false
+
+examples:
+ - |
+ usb2-phy@120 {
+ compatible = "hisilicon,hi3798cv200-usb2-phy";
+ reg = <0x120 0x4>;
+ clocks = <&clk_ref>;
+ resets = <&crg 0xbc 4>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy@0 {
+ reg = <0>;
+ #phy-cells = <0>;
+ resets = <&crg 0xbc 8>;
+ };
+
+ phy@1 {
+ reg = <1>;
+ #phy-cells = <0>;
+ resets = <&crg 0xbc 9>;
+ };
+ };
diff --git a/Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt b/Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt
deleted file mode 100644
index 104953e849e7..000000000000
--- a/Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt
+++ /dev/null
@@ -1,71 +0,0 @@
-Device tree bindings for HiSilicon INNO USB2 PHY
-
-Required properties:
-- compatible: Should be one of the following strings:
- "hisilicon,inno-usb2-phy",
- "hisilicon,hi3798cv200-usb2-phy".
-- reg: Should be the address space for PHY configuration register in peripheral
- controller, e.g. PERI_USB0 for USB 2.0 PHY01 on Hi3798CV200 SoC.
-- clocks: The phandle and clock specifier pair for INNO USB2 PHY device
- reference clock.
-- resets: The phandle and reset specifier pair for INNO USB2 PHY device reset
- signal.
-- #address-cells: Must be 1.
-- #size-cells: Must be 0.
-
-The INNO USB2 PHY device should be a child node of peripheral controller that
-contains the PHY configuration register, and each device supports up to 2 PHY
-ports which are represented as child nodes of INNO USB2 PHY device.
-
-Required properties for PHY port node:
-- reg: The PHY port instance number.
-- #phy-cells: Defined by generic PHY bindings. Must be 0.
-- resets: The phandle and reset specifier pair for PHY port reset signal.
-
-Refer to phy/phy-bindings.txt for the generic PHY binding properties
-
-Example:
-
-perictrl: peripheral-controller@8a20000 {
- compatible = "hisilicon,hi3798cv200-perictrl", "simple-mfd";
- reg = <0x8a20000 0x1000>;
- #address-cells = <1>;
- #size-cells = <1>;
- ranges = <0x0 0x8a20000 0x1000>;
-
- usb2_phy1: usb2-phy@120 {
- compatible = "hisilicon,hi3798cv200-usb2-phy";
- reg = <0x120 0x4>;
- clocks = <&crg HISTB_USB2_PHY1_REF_CLK>;
- resets = <&crg 0xbc 4>;
- #address-cells = <1>;
- #size-cells = <0>;
-
- usb2_phy1_port0: phy@0 {
- reg = <0>;
- #phy-cells = <0>;
- resets = <&crg 0xbc 8>;
- };
-
- usb2_phy1_port1: phy@1 {
- reg = <1>;
- #phy-cells = <0>;
- resets = <&crg 0xbc 9>;
- };
- };
-
- usb2_phy2: usb2-phy@124 {
- compatible = "hisilicon,hi3798cv200-usb2-phy";
- reg = <0x124 0x4>;
- clocks = <&crg HISTB_USB2_PHY2_REF_CLK>;
- resets = <&crg 0xbc 6>;
- #address-cells = <1>;
- #size-cells = <0>;
-
- usb2_phy2_port0: phy@0 {
- reg = <0>;
- #phy-cells = <0>;
- resets = <&crg 0xbc 10>;
- };
- };
-};

--
2.43.0


2024-03-05 02:21:16

by Yang Xiwen via B4 Relay

[permalink] [raw]
Subject: [PATCH v4 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY

From: Yang Xiwen <[email protected]>

Direct MMIO resgiter access is used by Hi3798MV200. For other models,
of_iomap() returns NULL due to insufficient length. So they are
unaffected.

Also Hi3798MV200 INNO PHY has an extra reset required to be deasserted,
switch to reset_control_bulk_() APIs for that.

Signed-off-by: Yang Xiwen <[email protected]>
---
drivers/phy/hisilicon/phy-hisi-inno-usb2.c | 66 ++++++++++++++++++------------
1 file changed, 40 insertions(+), 26 deletions(-)

diff --git a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
index b7e740eb4752..a759823b5b1e 100644
--- a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
+++ b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
@@ -10,6 +10,7 @@
#include <linux/io.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_address.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/reset.h>
@@ -24,6 +25,7 @@

#define PHY_TYPE_0 0
#define PHY_TYPE_1 1
+#define PHY_TYPE_MMIO 2

#define PHY_TEST_DATA GENMASK(7, 0)
#define PHY_TEST_ADDR_OFFSET 8
@@ -43,6 +45,7 @@
#define PHY_CLK_ENABLE BIT(2)

struct hisi_inno_phy_port {
+ void __iomem *base;
struct reset_control *utmi_rst;
struct hisi_inno_phy_priv *priv;
};
@@ -50,7 +53,7 @@ struct hisi_inno_phy_port {
struct hisi_inno_phy_priv {
void __iomem *mmio;
struct clk *ref_clk;
- struct reset_control *por_rst;
+ struct reset_control *rsts;
unsigned int type;
struct hisi_inno_phy_port ports[INNO_PHY_PORT_NUM];
};
@@ -62,26 +65,31 @@ static void hisi_inno_phy_write_reg(struct hisi_inno_phy_priv *priv,
u32 val;
u32 value;

- if (priv->type == PHY_TYPE_0)
- val = (data & PHY_TEST_DATA) |
- ((addr << PHY_TEST_ADDR_OFFSET) & PHY0_TEST_ADDR) |
- ((port << PHY0_TEST_PORT_OFFSET) & PHY0_TEST_PORT) |
- PHY0_TEST_WREN | PHY0_TEST_RST;
- else
- val = (data & PHY_TEST_DATA) |
- ((addr << PHY_TEST_ADDR_OFFSET) & PHY1_TEST_ADDR) |
- ((port << PHY1_TEST_PORT_OFFSET) & PHY1_TEST_PORT) |
- PHY1_TEST_WREN | PHY1_TEST_RST;
- writel(val, reg);
-
- value = val;
- if (priv->type == PHY_TYPE_0)
- value |= PHY0_TEST_CLK;
- else
- value |= PHY1_TEST_CLK;
- writel(value, reg);
-
- writel(val, reg);
+ if (priv->ports[port].base)
+ /* FIXME: fill stride in priv */
+ writel(data, (u32 *)priv->ports[port].base + addr);
+ else {
+ if (priv->type == PHY_TYPE_0)
+ val = (data & PHY_TEST_DATA) |
+ ((addr << PHY_TEST_ADDR_OFFSET) & PHY0_TEST_ADDR) |
+ ((port << PHY0_TEST_PORT_OFFSET) & PHY0_TEST_PORT) |
+ PHY0_TEST_WREN | PHY0_TEST_RST;
+ else
+ val = (data & PHY_TEST_DATA) |
+ ((addr << PHY_TEST_ADDR_OFFSET) & PHY1_TEST_ADDR) |
+ ((port << PHY1_TEST_PORT_OFFSET) & PHY1_TEST_PORT) |
+ PHY1_TEST_WREN | PHY1_TEST_RST;
+ writel(val, reg);
+
+ value = val;
+ if (priv->type == PHY_TYPE_0)
+ value |= PHY0_TEST_CLK;
+ else
+ value |= PHY1_TEST_CLK;
+ writel(value, reg);
+
+ writel(val, reg);
+ }
}

static void hisi_inno_phy_setup(struct hisi_inno_phy_priv *priv)
@@ -104,7 +112,7 @@ static int hisi_inno_phy_init(struct phy *phy)
return ret;
udelay(REF_CLK_STABLE_TIME);

- reset_control_deassert(priv->por_rst);
+ reset_control_deassert(priv->rsts);
udelay(POR_RST_COMPLETE_TIME);

/* Set up phy registers */
@@ -122,7 +130,7 @@ static int hisi_inno_phy_exit(struct phy *phy)
struct hisi_inno_phy_priv *priv = port->priv;

reset_control_assert(port->utmi_rst);
- reset_control_assert(priv->por_rst);
+ reset_control_assert(priv->rsts);
clk_disable_unprepare(priv->ref_clk);

return 0;
@@ -158,15 +166,16 @@ static int hisi_inno_phy_probe(struct platform_device *pdev)
if (IS_ERR(priv->ref_clk))
return PTR_ERR(priv->ref_clk);

- priv->por_rst = devm_reset_control_get_exclusive(dev, NULL);
- if (IS_ERR(priv->por_rst))
- return PTR_ERR(priv->por_rst);
+ priv->rsts = devm_reset_control_array_get(dev, false, false);
+ if (IS_ERR(priv->rsts))
+ return PTR_ERR(priv->rsts);

priv->type = (uintptr_t) of_device_get_match_data(dev);

for_each_child_of_node(np, child) {
struct reset_control *rst;
struct phy *phy;
+ void __iomem *base;

rst = of_reset_control_get_exclusive(child, NULL);
if (IS_ERR(rst)) {
@@ -174,7 +183,10 @@ static int hisi_inno_phy_probe(struct platform_device *pdev)
return PTR_ERR(rst);
}

+ base = of_iomap(child, 0);
+
priv->ports[i].utmi_rst = rst;
+ priv->ports[i].base = base;
priv->ports[i].priv = priv;

phy = devm_phy_create(dev, child, &hisi_inno_phy_ops);
@@ -205,6 +217,8 @@ static const struct of_device_id hisi_inno_phy_of_match[] = {
.data = (void *) PHY_TYPE_0 },
{ .compatible = "hisilicon,hi3798mv100-usb2-phy",
.data = (void *) PHY_TYPE_1 },
+ { .compatible = "hisilicon,hi3798mv200-usb2-phy",
+ .data = (void *) PHY_TYPE_MMIO },
{ },
};
MODULE_DEVICE_TABLE(of, hisi_inno_phy_of_match);

--
2.43.0


2024-03-05 13:28:23

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY

On Di, 2024-03-05 at 10:19 +0800, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <[email protected]>
>
> Direct MMIO resgiter access is used by Hi3798MV200. For other models,

See

https://lore.kernel.org/all/[email protected]/

regards
Philipp

2024-03-05 14:56:30

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 2/5] dt-bindings: phy: hisi-inno-usb2: convert to YAML


On Tue, 05 Mar 2024 10:19:47 +0800, Yang Xiwen wrote:
> convert the legacy text binding to YAML. While at it, remove
> "hisilicon,inno-usb2-phy" fallback compatible.
>
> Signed-off-by: Yang Xiwen <[email protected]>
> ---
> .../bindings/phy/hisilicon,inno-usb2-phy.yaml | 91 ++++++++++++++++++++++
> .../devicetree/bindings/phy/phy-hisi-inno-usb2.txt | 71 -----------------
> 2 files changed, 91 insertions(+), 71 deletions(-)
>

Reviewed-by: Rob Herring <[email protected]>


2024-03-05 14:57:00

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 2/5] dt-bindings: phy: hisi-inno-usb2: convert to YAML

On Tue, Mar 05, 2024 at 10:19:47AM +0800, Yang Xiwen wrote:
> convert the legacy text binding to YAML. While at it, remove
> "hisilicon,inno-usb2-phy" fallback compatible.
>
> Signed-off-by: Yang Xiwen <[email protected]>
> ---
> .../bindings/phy/hisilicon,inno-usb2-phy.yaml | 91 ++++++++++++++++++++++
> .../devicetree/bindings/phy/phy-hisi-inno-usb2.txt | 71 -----------------
> 2 files changed, 91 insertions(+), 71 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml b/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml
> new file mode 100644
> index 000000000000..ba82405ddf51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml
> @@ -0,0 +1,91 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/hisilicon,inno-usb2-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HiSilicon HiSTB SoCs INNO USB2 PHY device
> +
> +maintainers:
> + - Yang Xiwen <[email protected]>
> +
> +properties:
> + compatible:
> + items:

If there's another version, you can drop 'items' (and its '-' below).

> + - enum:
> + - hisilicon,hi3798cv200-usb2-phy