* [PATCH v5 0/5] phy: hisi-inno-phy: add support for hi3798mv200-usb2-phy
@ 2024-03-05 13:32 Yang Xiwen via B4 Relay
2024-03-05 13:32 ` [PATCH v5 1/5] phy: hisilicon: hisi-inno-phy: enable clocks for every ports Yang Xiwen via B4 Relay
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: Yang Xiwen via B4 Relay @ 2024-03-05 13:32 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiancheng Xue, Shawn Guo,
Philipp Zabel
Cc: linux-phy, devicetree, linux-kernel, Kishon Vijay Abraham I,
David Yang, Yang Xiwen
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 <forbidden405@outlook.com>
---
Changes in v5:
- commit msg: bulk->array. (Philipp)
- use devm_reset_control_array_exclusive() instead. (Philipp)
- Link to v4: https://lore.kernel.org/r/20240305-inno-phy-v4-0-a03204c9cf1c@outlook.com
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/20240220-inno-phy-v3-0-893cdf8633b4@outlook.com
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/20240217-inno-phy-v2-0-3bf7e87b0e9e@outlook.com
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/20240216-inno-phy-v1-0-1ab912f0533f@outlook.com
---
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 <forbidden405@outlook.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 1/5] phy: hisilicon: hisi-inno-phy: enable clocks for every ports
2024-03-05 13:32 [PATCH v5 0/5] phy: hisi-inno-phy: add support for hi3798mv200-usb2-phy Yang Xiwen via B4 Relay
@ 2024-03-05 13:32 ` Yang Xiwen via B4 Relay
2024-03-05 13:32 ` [PATCH v5 2/5] dt-bindings: phy: hisi-inno-usb2: convert to YAML Yang Xiwen via B4 Relay
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Yang Xiwen via B4 Relay @ 2024-03-05 13:32 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiancheng Xue, Shawn Guo,
Philipp Zabel
Cc: linux-phy, devicetree, linux-kernel, Kishon Vijay Abraham I,
David Yang, Yang Xiwen
From: Yang Xiwen <forbidden405@outlook.com>
This is needed for port1 to work.
Fixes: ba8b0ee81fbb ("phy: add inno-usb2-phy driver for hi3798cv200 SoC")
Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
drivers/phy/hisilicon/phy-hisi-inno-usb2.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
index c138cd4807d6..b7e740eb4752 100644
--- a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
+++ b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
@@ -86,8 +86,10 @@ static void hisi_inno_phy_write_reg(struct hisi_inno_phy_priv *priv,
static void hisi_inno_phy_setup(struct hisi_inno_phy_priv *priv)
{
+ int i;
/* The phy clk is controlled by the port0 register 0x06. */
- hisi_inno_phy_write_reg(priv, 0, 0x06, PHY_CLK_ENABLE);
+ for (i = 0; i < INNO_PHY_PORT_NUM; i++)
+ hisi_inno_phy_write_reg(priv, i, 0x06, PHY_CLK_ENABLE);
msleep(PHY_CLK_STABLE_TIME);
}
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 2/5] dt-bindings: phy: hisi-inno-usb2: convert to YAML
2024-03-05 13:32 [PATCH v5 0/5] phy: hisi-inno-phy: add support for hi3798mv200-usb2-phy Yang Xiwen via B4 Relay
2024-03-05 13:32 ` [PATCH v5 1/5] phy: hisilicon: hisi-inno-phy: enable clocks for every ports Yang Xiwen via B4 Relay
@ 2024-03-05 13:32 ` Yang Xiwen via B4 Relay
2024-03-05 13:32 ` [PATCH v5 3/5] dt-bindings: phy: hisilicon,inno-usb2-phy: add support for Hi3798MV100 INNO PHY Yang Xiwen via B4 Relay
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Yang Xiwen via B4 Relay @ 2024-03-05 13:32 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiancheng Xue, Shawn Guo,
Philipp Zabel
Cc: linux-phy, devicetree, linux-kernel, Kishon Vijay Abraham I,
David Yang, Yang Xiwen
From: Yang Xiwen <forbidden405@outlook.com>
convert the legacy text binding to YAML. While at it, remove
"hisilicon,inno-usb2-phy" fallback compatible.
Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
.../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 <forbidden405@outlook.com>
+
+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
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 3/5] dt-bindings: phy: hisilicon,inno-usb2-phy: add support for Hi3798MV100 INNO PHY
2024-03-05 13:32 [PATCH v5 0/5] phy: hisi-inno-phy: add support for hi3798mv200-usb2-phy Yang Xiwen via B4 Relay
2024-03-05 13:32 ` [PATCH v5 1/5] phy: hisilicon: hisi-inno-phy: enable clocks for every ports Yang Xiwen via B4 Relay
2024-03-05 13:32 ` [PATCH v5 2/5] dt-bindings: phy: hisi-inno-usb2: convert to YAML Yang Xiwen via B4 Relay
@ 2024-03-05 13:32 ` Yang Xiwen via B4 Relay
2024-03-05 13:32 ` [PATCH v5 4/5] dt-bindings: phy: hisi-inno-usb2: add compatible of hisilicon,hi3798mv200-usb2-phy Yang Xiwen via B4 Relay
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Yang Xiwen via B4 Relay @ 2024-03-05 13:32 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiancheng Xue, Shawn Guo,
Philipp Zabel
Cc: linux-phy, devicetree, linux-kernel, Kishon Vijay Abraham I,
David Yang, Yang Xiwen
From: Yang Xiwen <forbidden405@outlook.com>
Hi3798MV100 also has a similar INNO USB2 PHY with slightly different
register fields offsets. Document it in the binding.
Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml b/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml
index ba82405ddf51..7f0811a2dc2b 100644
--- a/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml
@@ -14,6 +14,7 @@ properties:
items:
- enum:
- hisilicon,hi3798cv200-usb2-phy
+ - hisilicon,hi3798mv100-usb2-phy
reg:
maxItems: 1
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 4/5] dt-bindings: phy: hisi-inno-usb2: add compatible of hisilicon,hi3798mv200-usb2-phy
2024-03-05 13:32 [PATCH v5 0/5] phy: hisi-inno-phy: add support for hi3798mv200-usb2-phy Yang Xiwen via B4 Relay
` (2 preceding siblings ...)
2024-03-05 13:32 ` [PATCH v5 3/5] dt-bindings: phy: hisilicon,inno-usb2-phy: add support for Hi3798MV100 INNO PHY Yang Xiwen via B4 Relay
@ 2024-03-05 13:32 ` Yang Xiwen via B4 Relay
2024-03-05 13:32 ` [PATCH v5 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY Yang Xiwen via B4 Relay
2024-03-05 14:13 ` [PATCH v5 0/5] phy: hisi-inno-phy: add support for hi3798mv200-usb2-phy Krzysztof Kozlowski
5 siblings, 0 replies; 10+ messages in thread
From: Yang Xiwen via B4 Relay @ 2024-03-05 13:32 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiancheng Xue, Shawn Guo,
Philipp Zabel
Cc: linux-phy, devicetree, linux-kernel, Kishon Vijay Abraham I,
David Yang, Yang Xiwen
From: Yang Xiwen <forbidden405@outlook.com>
Hi3798MV200 INNO USB2 PHY is attached directly to system bus. Add
compatible "hisilicon,hi3798mv200-usb2-phy" for it. The ports of
Hi3798MV200 INNO PHY has its own address space, so
Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
.../bindings/phy/hisilicon,inno-usb2-phy.yaml | 31 ++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml b/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml
index 7f0811a2dc2b..92559bdc4fef 100644
--- a/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/hisilicon,inno-usb2-phy.yaml
@@ -15,6 +15,7 @@ properties:
- enum:
- hisilicon,hi3798cv200-usb2-phy
- hisilicon,hi3798mv100-usb2-phy
+ - hisilicon,hi3798mv200-usb2-phy
reg:
maxItems: 1
@@ -22,18 +23,29 @@ properties:
peripheral controller, e.g. PERI_USB0 for USB 2.0 PHY01 on Hi3798CV200 SoC,
or direct MMIO address space.
+ ranges:
+ maxItems: 1
+
'#address-cells':
const: 1
'#size-cells':
- const: 0
+ enum: [0, 1]
clocks:
maxItems: 1
description: reference clock
resets:
- maxItems: 1
+ minItems: 1
+ items:
+ - description: port reset
+ - description: optional external test bus reset
+
+ reset-names:
+ items:
+ - const: port
+ - const: test
patternProperties:
'phy@[0-9a-f]+':
@@ -66,6 +78,21 @@ required:
- '#size-cells'
- resets
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: hisilicon,hi3798mv200-usb2-phy
+ then:
+ required:
+ - ranges
+ - reset-names
+ else:
+ properties:
+ ranges: false
+ reset-names: false
+
additionalProperties: false
examples:
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY
2024-03-05 13:32 [PATCH v5 0/5] phy: hisi-inno-phy: add support for hi3798mv200-usb2-phy Yang Xiwen via B4 Relay
` (3 preceding siblings ...)
2024-03-05 13:32 ` [PATCH v5 4/5] dt-bindings: phy: hisi-inno-usb2: add compatible of hisilicon,hi3798mv200-usb2-phy Yang Xiwen via B4 Relay
@ 2024-03-05 13:32 ` Yang Xiwen via B4 Relay
2024-04-05 16:52 ` Vinod Koul
2024-03-05 14:13 ` [PATCH v5 0/5] phy: hisi-inno-phy: add support for hi3798mv200-usb2-phy Krzysztof Kozlowski
5 siblings, 1 reply; 10+ messages in thread
From: Yang Xiwen via B4 Relay @ 2024-03-05 13:32 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiancheng Xue, Shawn Guo,
Philipp Zabel
Cc: linux-phy, devicetree, linux-kernel, Kishon Vijay Abraham I,
David Yang, Yang Xiwen
From: Yang Xiwen <forbidden405@outlook.com>
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_array_*() APIs for that.
Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
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..df154cd99ed8 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_exclusive(dev);
+ 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
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v5 0/5] phy: hisi-inno-phy: add support for hi3798mv200-usb2-phy
2024-03-05 13:32 [PATCH v5 0/5] phy: hisi-inno-phy: add support for hi3798mv200-usb2-phy Yang Xiwen via B4 Relay
` (4 preceding siblings ...)
2024-03-05 13:32 ` [PATCH v5 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY Yang Xiwen via B4 Relay
@ 2024-03-05 14:13 ` Krzysztof Kozlowski
5 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-05 14:13 UTC (permalink / raw)
To: forbidden405, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiancheng Xue, Shawn Guo,
Philipp Zabel
Cc: linux-phy, devicetree, linux-kernel, Kishon Vijay Abraham I,
David Yang
On 05/03/2024 14:32, Yang Xiwen via B4 Relay wrote:
> 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 <forbidden405@outlook.com>
> ---
> Changes in v5:
> - commit msg: bulk->array. (Philipp)
> - use devm_reset_control_array_exclusive() instead. (Philipp)
> - Link to v4: https://lore.kernel.org/r/20240305-inno-phy-v4-0-a03204c9cf1c@outlook.com
One patchset per 24h. Allow people to actually review your code, before
posting new version.
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY
2024-03-05 13:32 ` [PATCH v5 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY Yang Xiwen via B4 Relay
@ 2024-04-05 16:52 ` Vinod Koul
2024-04-05 17:53 ` Yang Xiwen
0 siblings, 1 reply; 10+ messages in thread
From: Vinod Koul @ 2024-04-05 16:52 UTC (permalink / raw)
To: forbidden405
Cc: Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jiancheng Xue, Shawn Guo, Philipp Zabel, linux-phy,
devicetree, linux-kernel, Kishon Vijay Abraham I, David Yang
On 05-03-24, 21:32, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <forbidden405@outlook.com>
That is quite an email id!
>
> Direct MMIO resgiter access is used by Hi3798MV200. For other models,
> of_iomap() returns NULL due to insufficient length. So they are
so how is that fixed... Pls describe the change...
> unaffected.
>
> Also Hi3798MV200 INNO PHY has an extra reset required to be deasserted,
> switch to reset_control_array_*() APIs for that.
That probably should be a different patch
>
> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
> ---
> 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..df154cd99ed8 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 */
when?
> + 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);
val and value are very helpful variables, do consider naming them
better!
> + }
> }
>
> 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_exclusive(dev);
> + 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
--
~Vinod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY
2024-04-05 16:52 ` Vinod Koul
@ 2024-04-05 17:53 ` Yang Xiwen
2024-04-06 6:19 ` Vinod Koul
0 siblings, 1 reply; 10+ messages in thread
From: Yang Xiwen @ 2024-04-05 17:53 UTC (permalink / raw)
To: Vinod Koul
Cc: Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jiancheng Xue, Shawn Guo, Philipp Zabel, linux-phy,
devicetree, linux-kernel, Kishon Vijay Abraham I, David Yang
On 4/6/2024 12:52 AM, Vinod Koul wrote:
> On 05-03-24, 21:32, Yang Xiwen via B4 Relay wrote:
>> From: Yang Xiwen <forbidden405@outlook.com>
> That is quite an email id!
>
>> Direct MMIO resgiter access is used by Hi3798MV200. For other models,
>> of_iomap() returns NULL due to insufficient length. So they are
> so how is that fixed... Pls describe the change...
The commit log will be rewritten in next revision. I'll try to emphasize
the PHY and its configuration interface briefly. Though i don't have
access to the datasheets and TRM so most things can not be verified.
For CV200 and MV100 INNO PHY, the configuration interface is attached to
PERICTRL(Peripheral Control Block). So we just use a register called
PERI_USB3 to configure the PHY. The bus reset, clock are all controlled
in PERI_USB3 register. To read/write to a register of the PHY, a special
sequence of register writes and reads are needed, which was implemented
in this driver.
But for MV200 INNO PHY, the configuration interface is attached directly
to system bus(MMIO). The bus clocks and resets are controlled via Clock
Reset Generator(CRG). Now we have to control them with the help of linux
clk and reset framework because they are provided by other modules.
>> unaffected.
>>
>> Also Hi3798MV200 INNO PHY has an extra reset required to be deasserted,
>> switch to reset_control_array_*() APIs for that.
The commit msg is misleading here. There is no extra reset actually. The
reset also exist for existing users. The initial author just decided to
manage it in the hisi_inno_phy_write_reg() routine(without using
reset_control_* APIs) and omit it in the binding.
> That probably should be a different patch
I guess so. From my point of view, the whole patch is to introduce the
support for Hi3798MV200 variant of the INNO PHY. So i've decided to
squash the two changes into one single commit.
>
>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
>> ---
>> 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..df154cd99ed8 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 */
> when?
I'm not sure. Maybe until some other users with stride other than 3? I
don't have much knowledge about other SoCs.
Maybe replace the FIXME here with some additional information.
>
>> + 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);
> val and value are very helpful variables, do consider naming them
> better!
I'll consider renaming them in the next revision. Maybe val and val2?
They are just some temp vars to store register values.
>
>> + }
>> }
>>
>> 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_exclusive(dev);
>> + 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
--
Regards,
Yang Xiwen
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY
2024-04-05 17:53 ` Yang Xiwen
@ 2024-04-06 6:19 ` Vinod Koul
0 siblings, 0 replies; 10+ messages in thread
From: Vinod Koul @ 2024-04-06 6:19 UTC (permalink / raw)
To: Yang Xiwen
Cc: Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jiancheng Xue, Shawn Guo, Philipp Zabel, linux-phy,
devicetree, linux-kernel, Kishon Vijay Abraham I, David Yang
On 06-04-24, 01:53, Yang Xiwen wrote:
> On 4/6/2024 12:52 AM, Vinod Koul wrote:
> > On 05-03-24, 21:32, Yang Xiwen via B4 Relay wrote:
> > > From: Yang Xiwen <forbidden405@outlook.com>
> > That is quite an email id!
> >
> > > Direct MMIO resgiter access is used by Hi3798MV200. For other models,
> > > of_iomap() returns NULL due to insufficient length. So they are
> > so how is that fixed... Pls describe the change...
>
>
> The commit log will be rewritten in next revision. I'll try to emphasize the
> PHY and its configuration interface briefly. Though i don't have access to
> the datasheets and TRM so most things can not be verified.
>
>
> For CV200 and MV100 INNO PHY, the configuration interface is attached to
> PERICTRL(Peripheral Control Block). So we just use a register called
> PERI_USB3 to configure the PHY. The bus reset, clock are all controlled in
> PERI_USB3 register. To read/write to a register of the PHY, a special
> sequence of register writes and reads are needed, which was implemented in
> this driver.
>
>
> But for MV200 INNO PHY, the configuration interface is attached directly to
> system bus(MMIO). The bus clocks and resets are controlled via Clock Reset
> Generator(CRG). Now we have to control them with the help of linux clk and
> reset framework because they are provided by other modules.
Okay better log is welcome
>
>
> > > unaffected.
> > >
> > > Also Hi3798MV200 INNO PHY has an extra reset required to be deasserted,
> > > switch to reset_control_array_*() APIs for that.
>
>
> The commit msg is misleading here. There is no extra reset actually. The
> reset also exist for existing users. The initial author just decided to
> manage it in the hisi_inno_phy_write_reg() routine(without using
> reset_control_* APIs) and omit it in the binding.
>
>
> > That probably should be a different patch
>
>
> I guess so. From my point of view, the whole patch is to introduce the
> support for Hi3798MV200 variant of the INNO PHY. So i've decided to squash
> the two changes into one single commit.
Not really you can build smaller reviewable changes leading up to adding
the Hi3798MV200 support
>
>
> >
> > > Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
> > > ---
> > > 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..df154cd99ed8 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 */
> > when?
>
>
> I'm not sure. Maybe until some other users with stride other than 3? I don't
> have much knowledge about other SoCs.
>
>
> Maybe replace the FIXME here with some additional information.
Better
>
>
> >
> > > + 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);
> > val and value are very helpful variables, do consider naming them
> > better!
>
>
> I'll consider renaming them in the next revision. Maybe val and val2? They
> are just some temp vars to store register values.
Yeah, that might be better
>
>
> >
> > > + }
> > > }
> > > 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_exclusive(dev);
> > > + 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
>
>
> --
> Regards,
> Yang Xiwen
--
~Vinod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-04-06 6:19 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-05 13:32 [PATCH v5 0/5] phy: hisi-inno-phy: add support for hi3798mv200-usb2-phy Yang Xiwen via B4 Relay
2024-03-05 13:32 ` [PATCH v5 1/5] phy: hisilicon: hisi-inno-phy: enable clocks for every ports Yang Xiwen via B4 Relay
2024-03-05 13:32 ` [PATCH v5 2/5] dt-bindings: phy: hisi-inno-usb2: convert to YAML Yang Xiwen via B4 Relay
2024-03-05 13:32 ` [PATCH v5 3/5] dt-bindings: phy: hisilicon,inno-usb2-phy: add support for Hi3798MV100 INNO PHY Yang Xiwen via B4 Relay
2024-03-05 13:32 ` [PATCH v5 4/5] dt-bindings: phy: hisi-inno-usb2: add compatible of hisilicon,hi3798mv200-usb2-phy Yang Xiwen via B4 Relay
2024-03-05 13:32 ` [PATCH v5 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY Yang Xiwen via B4 Relay
2024-04-05 16:52 ` Vinod Koul
2024-04-05 17:53 ` Yang Xiwen
2024-04-06 6:19 ` Vinod Koul
2024-03-05 14:13 ` [PATCH v5 0/5] phy: hisi-inno-phy: add support for hi3798mv200-usb2-phy Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).