* [PATCH v4 0/3] Add USB PHY support for new Ingenic SoCs.
@ 2020-07-22 6:33 周琰杰 (Zhou Yanjie)
2020-07-22 6:33 ` [PATCH v4 1/3] dt-bindings: USB: Add bindings " 周琰杰 (Zhou Yanjie)
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2020-07-22 6:33 UTC (permalink / raw)
To: balbi, gregkh, robh+dt
Cc: linux-usb, linux-kernel, devicetree, paul, prasannatsmkumar,
dongsheng.qiu, aric.pzqi, rick.tyliu, yanfei.li, sernia.zhou,
zhenwenjin
v3->v4:
Fix typos.
周琰杰 (Zhou Yanjie) (3):
dt-bindings: USB: Add bindings for new Ingenic SoCs.
USB: PHY: JZ4770: Add support for new Ingenic SoCs.
USB: PHY: JZ4770: Reformat the code to align it.
.../bindings/usb/ingenic,jz4770-phy.yaml | 6 +-
drivers/usb/phy/Kconfig | 4 +-
drivers/usb/phy/phy-jz4770.c | 252 ++++++++++++++-------
3 files changed, 177 insertions(+), 85 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 1/3] dt-bindings: USB: Add bindings for new Ingenic SoCs.
2020-07-22 6:33 [PATCH v4 0/3] Add USB PHY support for new Ingenic SoCs 周琰杰 (Zhou Yanjie)
@ 2020-07-22 6:33 ` 周琰杰 (Zhou Yanjie)
2020-07-22 6:33 ` [PATCH v4 2/3] USB: PHY: JZ4770: Add support " 周琰杰 (Zhou Yanjie)
2020-07-22 6:33 ` [PATCH v4 3/3] USB: PHY: JZ4770: Reformat the code to align it 周琰杰 (Zhou Yanjie)
2 siblings, 0 replies; 6+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2020-07-22 6:33 UTC (permalink / raw)
To: balbi, gregkh, robh+dt
Cc: linux-usb, linux-kernel, devicetree, paul, prasannatsmkumar,
dongsheng.qiu, aric.pzqi, rick.tyliu, yanfei.li, sernia.zhou,
zhenwenjin
Add the USB PHY bindings for the JZ4780 SoC, the X1000 SoC and
the X1830 SoC from Ingenic.
Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Notes:
v1->v2:
Add bindings for the JZ4780 SoC.
v2->v3:
No change.
v3->v4:
No change.
Documentation/devicetree/bindings/usb/ingenic,jz4770-phy.yaml | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/ingenic,jz4770-phy.yaml b/Documentation/devicetree/bindings/usb/ingenic,jz4770-phy.yaml
index a81b0b1a2226..2d61166ea5cf 100644
--- a/Documentation/devicetree/bindings/usb/ingenic,jz4770-phy.yaml
+++ b/Documentation/devicetree/bindings/usb/ingenic,jz4770-phy.yaml
@@ -4,10 +4,11 @@
$id: http://devicetree.org/schemas/usb/ingenic,jz4770-phy.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: Ingenic JZ4770 USB PHY devicetree bindings
+title: Ingenic SoCs USB PHY devicetree bindings
maintainers:
- Paul Cercueil <paul@crapouillou.net>
+ - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
properties:
$nodename:
@@ -16,6 +17,9 @@ properties:
compatible:
enum:
- ingenic,jz4770-phy
+ - ingenic,jz4780-phy
+ - ingenic,x1000-phy
+ - ingenic,x1830-phy
reg:
maxItems: 1
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v4 2/3] USB: PHY: JZ4770: Add support for new Ingenic SoCs.
2020-07-22 6:33 [PATCH v4 0/3] Add USB PHY support for new Ingenic SoCs 周琰杰 (Zhou Yanjie)
2020-07-22 6:33 ` [PATCH v4 1/3] dt-bindings: USB: Add bindings " 周琰杰 (Zhou Yanjie)
@ 2020-07-22 6:33 ` 周琰杰 (Zhou Yanjie)
2020-07-22 17:19 ` Paul Cercueil
2020-07-22 6:33 ` [PATCH v4 3/3] USB: PHY: JZ4770: Reformat the code to align it 周琰杰 (Zhou Yanjie)
2 siblings, 1 reply; 6+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2020-07-22 6:33 UTC (permalink / raw)
To: balbi, gregkh, robh+dt
Cc: linux-usb, linux-kernel, devicetree, paul, prasannatsmkumar,
dongsheng.qiu, aric.pzqi, rick.tyliu, yanfei.li, sernia.zhou,
zhenwenjin
Add support for probing the phy-jz4770 driver on the JZ4780 SoC,
the X1000 SoC and the X1830 SoC from Ingenic.
Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---
Notes:
v1->v2:
Add bindings for the JZ4780 SoC.
v2->v3:
Use "of_device_get_match_data" instead "of_match_device"
to get version information.
v3->v4:
Fix typos.
drivers/usb/phy/Kconfig | 4 +-
drivers/usb/phy/phy-jz4770.c | 174 ++++++++++++++++++++++++++++++++-----------
2 files changed, 133 insertions(+), 45 deletions(-)
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 4b3fa78995cf..775f0dd7b5f5 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -185,11 +185,11 @@ config USB_ULPI_VIEWPORT
controllers with a viewport register (e.g. Chipidea/ARC controllers).
config JZ4770_PHY
- tristate "Ingenic JZ4770 Transceiver Driver"
+ tristate "Ingenic SoCs Transceiver Driver"
depends on MIPS || COMPILE_TEST
select USB_PHY
help
This driver provides PHY support for the USB controller found
- on the JZ4770 SoC from Ingenic.
+ on the JZ4770/JZ4780/X1000/X1830 SoC from Ingenic.
endmenu
diff --git a/drivers/usb/phy/phy-jz4770.c b/drivers/usb/phy/phy-jz4770.c
index 8f62dc2a90ff..cd49b32b4c13 100644
--- a/drivers/usb/phy/phy-jz4770.c
+++ b/drivers/usb/phy/phy-jz4770.c
@@ -1,27 +1,30 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Ingenic JZ4770 USB PHY driver
+ * Ingenic SoCs USB PHY driver
* Copyright (c) Paul Cercueil <paul@crapouillou.net>
+ * Copyright (c) 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
+ * Copyright (c) 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
*/
#include <linux/clk.h>
#include <linux/io.h>
#include <linux/module.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/regulator/consumer.h>
#include <linux/usb/otg.h>
#include <linux/usb/phy.h>
+/* OTGPHY register offsets */
#define REG_USBPCR_OFFSET 0x00
#define REG_USBRDT_OFFSET 0x04
#define REG_USBVBFIL_OFFSET 0x08
#define REG_USBPCR1_OFFSET 0x0c
-/* USBPCR */
+/* bits within the USBPCR register */
#define USBPCR_USB_MODE BIT(31)
#define USBPCR_AVLD_REG BIT(30)
-#define USBPCR_INCRM BIT(27)
-#define USBPCR_CLK12_EN BIT(26)
+#define USBPCR_INCR_MASK BIT(27)
#define USBPCR_COMMONONN BIT(25)
#define USBPCR_VBUSVLDEXT BIT(24)
#define USBPCR_VBUSVLDEXTSEL BIT(23)
@@ -32,46 +35,80 @@
#define USBPCR_IDPULLUP_LSB 28
#define USBPCR_IDPULLUP_MASK GENMASK(29, USBPCR_IDPULLUP_LSB)
-#define USBPCR_IDPULLUP_ALWAYS (3 << USBPCR_IDPULLUP_LSB)
-#define USBPCR_IDPULLUP_SUSPEND (1 << USBPCR_IDPULLUP_LSB)
-#define USBPCR_IDPULLUP_OTG (0 << USBPCR_IDPULLUP_LSB)
+#define USBPCR_IDPULLUP_ALWAYS (0x2 << USBPCR_IDPULLUP_LSB)
+#define USBPCR_IDPULLUP_SUSPEND (0x1 << USBPCR_IDPULLUP_LSB)
+#define USBPCR_IDPULLUP_OTG (0x0 << USBPCR_IDPULLUP_LSB)
#define USBPCR_COMPDISTUNE_LSB 17
#define USBPCR_COMPDISTUNE_MASK GENMASK(19, USBPCR_COMPDISTUNE_LSB)
-#define USBPCR_COMPDISTUNE_DFT 4
+#define USBPCR_COMPDISTUNE_DFT (0x4 << USBPCR_COMPDISTUNE_LSB)
#define USBPCR_OTGTUNE_LSB 14
#define USBPCR_OTGTUNE_MASK GENMASK(16, USBPCR_OTGTUNE_LSB)
-#define USBPCR_OTGTUNE_DFT 4
+#define USBPCR_OTGTUNE_DFT (0x4 << USBPCR_OTGTUNE_LSB)
#define USBPCR_SQRXTUNE_LSB 11
#define USBPCR_SQRXTUNE_MASK GENMASK(13, USBPCR_SQRXTUNE_LSB)
-#define USBPCR_SQRXTUNE_DFT 3
+#define USBPCR_SQRXTUNE_DCR_20PCT (0x7 << USBPCR_SQRXTUNE_LSB)
+#define USBPCR_SQRXTUNE_DFT (0x3 << USBPCR_SQRXTUNE_LSB)
#define USBPCR_TXFSLSTUNE_LSB 7
#define USBPCR_TXFSLSTUNE_MASK GENMASK(10, USBPCR_TXFSLSTUNE_LSB)
-#define USBPCR_TXFSLSTUNE_DFT 3
+#define USBPCR_TXFSLSTUNE_DCR_50PPT (0xf << USBPCR_TXFSLSTUNE_LSB)
+#define USBPCR_TXFSLSTUNE_DCR_25PPT (0x7 << USBPCR_TXFSLSTUNE_LSB)
+#define USBPCR_TXFSLSTUNE_DFT (0x3 << USBPCR_TXFSLSTUNE_LSB)
+#define USBPCR_TXFSLSTUNE_INC_25PPT (0x1 << USBPCR_TXFSLSTUNE_LSB)
+#define USBPCR_TXFSLSTUNE_INC_50PPT (0x0 << USBPCR_TXFSLSTUNE_LSB)
+
+#define USBPCR_TXHSXVTUNE_LSB 4
+#define USBPCR_TXHSXVTUNE_MASK GENMASK(5, USBPCR_TXHSXVTUNE_LSB)
+#define USBPCR_TXHSXVTUNE_DFT (0x3 << USBPCR_TXHSXVTUNE_LSB)
+#define USBPCR_TXHSXVTUNE_DCR_15MV (0x1 << USBPCR_TXHSXVTUNE_LSB)
#define USBPCR_TXRISETUNE_LSB 4
#define USBPCR_TXRISETUNE_MASK GENMASK(5, USBPCR_TXRISETUNE_LSB)
-#define USBPCR_TXRISETUNE_DFT 3
+#define USBPCR_TXRISETUNE_DFT (0x3 << USBPCR_TXRISETUNE_LSB)
#define USBPCR_TXVREFTUNE_LSB 0
#define USBPCR_TXVREFTUNE_MASK GENMASK(3, USBPCR_TXVREFTUNE_LSB)
-#define USBPCR_TXVREFTUNE_DFT 5
+#define USBPCR_TXVREFTUNE_INC_25PPT (0x7 << USBPCR_TXVREFTUNE_LSB)
+#define USBPCR_TXVREFTUNE_DFT (0x5 << USBPCR_TXVREFTUNE_LSB)
-/* USBRDT */
+/* bits within the USBRDTR register */
+#define USBRDT_UTMI_RST BIT(27)
+#define USBRDT_HB_MASK BIT(26)
#define USBRDT_VBFIL_LD_EN BIT(25)
#define USBRDT_IDDIG_EN BIT(24)
#define USBRDT_IDDIG_REG BIT(23)
-
-#define USBRDT_USBRDT_LSB 0
-#define USBRDT_USBRDT_MASK GENMASK(22, USBRDT_USBRDT_LSB)
-
-/* USBPCR1 */
-#define USBPCR1_UHC_POWON BIT(5)
+#define USBRDT_VBFIL_EN BIT(2)
+
+/* bits within the USBPCR1 register */
+#define USBPCR1_BVLD_REG BIT(31)
+#define USBPCR1_DPPD BIT(29)
+#define USBPCR1_DMPD BIT(28)
+#define USBPCR1_USB_SEL BIT(28)
+#define USBPCR1_WORD_IF_16BIT BIT(19)
+
+#define USBPCR1_REFCLKSEL_LSB 26
+#define USBPCR1_REFCLKSEL_MASK GENMASK(27, USBPCR1_REFCLKDIV_LSB)
+#define USBPCR1_REFCLKSEL_CLKCORE (0x3 << USBPCR1_REFCLKSEL_LSB)
+
+#define USBPCR1_REFCLKDIV_LSB 24
+#define USBPCR1_REFCLKDIV_MASK GENMASK(25, USBPCR1_REFCLKDIV_LSB)
+#define USBPCR1_REFCLKDIV_48M (0x2 << USBPCR1_REFCLKDIV_LSB)
+#define USBPCR1_REFCLKDIV_24M (0x1 << USBPCR1_REFCLKDIV_LSB)
+#define USBPCR1_REFCLKDIV_12M (0x0 << USBPCR1_REFCLKDIV_LSB)
+
+enum ingenic_usb_phy_version {
+ ID_JZ4770,
+ ID_JZ4780,
+ ID_X1000,
+ ID_X1830,
+};
struct jz4770_phy {
+ enum ingenic_usb_phy_version version;
+
struct usb_phy phy;
struct usb_otg otg;
struct device *dev;
@@ -96,6 +133,12 @@ static int jz4770_phy_set_peripheral(struct usb_otg *otg,
struct jz4770_phy *priv = otg_to_jz4770_phy(otg);
u32 reg;
+ if (priv->version >= ID_X1000) {
+ reg = readl(priv->base + REG_USBPCR1_OFFSET);
+ reg |= USBPCR1_BVLD_REG;
+ writel(reg, priv->base + REG_USBPCR1_OFFSET);
+ }
+
reg = readl(priv->base + REG_USBPCR_OFFSET);
reg &= ~USBPCR_USB_MODE;
reg |= USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL | USBPCR_OTG_DISABLE;
@@ -135,17 +178,59 @@ static int jz4770_phy_init(struct usb_phy *phy)
return err;
}
- reg = USBPCR_AVLD_REG | USBPCR_COMMONONN | USBPCR_IDPULLUP_ALWAYS |
- (USBPCR_COMPDISTUNE_DFT << USBPCR_COMPDISTUNE_LSB) |
- (USBPCR_OTGTUNE_DFT << USBPCR_OTGTUNE_LSB) |
- (USBPCR_SQRXTUNE_DFT << USBPCR_SQRXTUNE_LSB) |
- (USBPCR_TXFSLSTUNE_DFT << USBPCR_TXFSLSTUNE_LSB) |
- (USBPCR_TXRISETUNE_DFT << USBPCR_TXRISETUNE_LSB) |
- (USBPCR_TXVREFTUNE_DFT << USBPCR_TXVREFTUNE_LSB) |
- USBPCR_POR;
+ if (priv->version >= ID_X1830) {
+ /* rdt */
+ writel(USBRDT_VBFIL_EN | USBRDT_UTMI_RST, priv->base + REG_USBRDT_OFFSET);
+
+ reg = readl(priv->base + REG_USBPCR1_OFFSET) | USBPCR1_WORD_IF_16BIT |
+ USBPCR1_DMPD | USBPCR1_DPPD;
+ writel(reg, priv->base + REG_USBPCR1_OFFSET);
+
+ reg = USBPCR_IDPULLUP_OTG | USBPCR_VBUSVLDEXT | USBPCR_TXPREEMPHTUNE;
+ } else if (priv->version >= ID_X1000) {
+ reg = readl(priv->base + REG_USBPCR1_OFFSET) | USBPCR1_WORD_IF_16BIT;
+ writel(reg, priv->base + REG_USBPCR1_OFFSET);
+
+ reg = USBPCR_SQRXTUNE_DCR_20PCT | USBPCR_TXPREEMPHTUNE |
+ USBPCR_TXHSXVTUNE_DCR_15MV | USBPCR_TXVREFTUNE_INC_25PPT;
+ } else if (priv->version >= ID_JZ4780) {
+ reg = readl(priv->base + REG_USBPCR1_OFFSET) | USBPCR1_USB_SEL |
+ USBPCR1_WORD_IF_16BIT;
+ writel(reg, priv->base + REG_USBPCR1_OFFSET);
+
+ reg = USBPCR_TXPREEMPHTUNE;
+ } else {
+ reg = USBPCR_AVLD_REG | USBPCR_IDPULLUP_ALWAYS |
+ USBPCR_COMPDISTUNE_DFT | USBPCR_OTGTUNE_DFT |
+ USBPCR_SQRXTUNE_DFT | USBPCR_TXFSLSTUNE_DFT |
+ USBPCR_TXRISETUNE_DFT | USBPCR_TXVREFTUNE_DFT;
+ }
+
+ reg |= USBPCR_COMMONONN | USBPCR_POR;
writel(reg, priv->base + REG_USBPCR_OFFSET);
- /* Wait for PHY to reset */
+ /*
+ * Power-On Reset(POR)
+ * Function:This customer-specific signal resets all test registers
+ * and state machines in the USB 2.0 nanoPHY.
+ * The POR signal must be asserted for a minimum of 10 μs.
+ * For POR timing information:
+ *
+ * T0: Power-on reset (POR) is initiated. 0 (reference).
+ * T1: T1 indicates when POR can be set to 1’b0. (To provide examples,
+ * values for T2 and T3 are also shown where T1 = T0 + 30 μs.);
+ * In general, T1 must be ≥ T0 + 10 μs. T0 + 10 μs ≤ T1.
+ * T2: T2 indicates when PHYCLOCK, CLK48MOHCI, and CLK12MOHCI are
+ * available at the macro output, based on the USB 2.0 nanoPHY
+ * reference clock source.
+ * Crystal:
+ * • When T1 = T0 + 10 μs:
+ * T2 < T1 + 805 μs = T0 + 815 μs
+ * • When T1 = T0 + 30 μs:
+ * T2 < T1 + 805 μs = T0 + 835 μs
+ * see "Reset and Power-Saving Signals" on page 60 an “Powering Up
+ * and Powering Down the USB 2.0 nanoPHY” on page 73.
+ */
usleep_range(30, 300);
writel(reg & ~USBPCR_POR, priv->base + REG_USBPCR_OFFSET);
usleep_range(300, 1000);
@@ -166,6 +251,15 @@ static void jz4770_phy_remove(void *phy)
usb_remove_phy(phy);
}
+static const struct of_device_id ingenic_usb_phy_of_matches[] = {
+ { .compatible = "ingenic,jz4770-phy", .data = (void *) ID_JZ4770 },
+ { .compatible = "ingenic,jz4780-phy", .data = (void *) ID_JZ4780 },
+ { .compatible = "ingenic,x1000-phy", .data = (void *) ID_X1000 },
+ { .compatible = "ingenic,x1830-phy", .data = (void *) ID_X1830 },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ingenic_usb_phy_of_matches);
+
static int jz4770_phy_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -176,11 +270,13 @@ static int jz4770_phy_probe(struct platform_device *pdev)
if (!priv)
return -ENOMEM;
+ priv->version = (enum ingenic_usb_phy_version)of_device_get_match_data(dev);
+
platform_set_drvdata(pdev, priv);
priv->dev = dev;
priv->phy.dev = dev;
priv->phy.otg = &priv->otg;
- priv->phy.label = "jz4770-phy";
+ priv->phy.label = "ingenic-usb-phy";
priv->phy.init = jz4770_phy_init;
priv->phy.shutdown = jz4770_phy_shutdown;
@@ -221,23 +317,15 @@ static int jz4770_phy_probe(struct platform_device *pdev)
return devm_add_action_or_reset(dev, jz4770_phy_remove, &priv->phy);
}
-#ifdef CONFIG_OF
-static const struct of_device_id jz4770_phy_of_matches[] = {
- { .compatible = "ingenic,jz4770-phy" },
- { }
-};
-MODULE_DEVICE_TABLE(of, jz4770_phy_of_matches);
-#endif
-
-static struct platform_driver jz4770_phy_driver = {
+static struct platform_driver ingenic_usb_phy_driver = {
.probe = jz4770_phy_probe,
.driver = {
- .name = "jz4770-phy",
- .of_match_table = of_match_ptr(jz4770_phy_of_matches),
+ .name = "ingenic-usb-phy",
+ .of_match_table = of_match_ptr(ingenic_usb_phy_of_matches),
},
};
-module_platform_driver(jz4770_phy_driver);
+module_platform_driver(ingenic_usb_phy_driver);
MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
-MODULE_DESCRIPTION("Ingenic JZ4770 USB PHY driver");
+MODULE_DESCRIPTION("Ingenic SoCs USB PHY driver");
MODULE_LICENSE("GPL");
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v4 3/3] USB: PHY: JZ4770: Reformat the code to align it.
2020-07-22 6:33 [PATCH v4 0/3] Add USB PHY support for new Ingenic SoCs 周琰杰 (Zhou Yanjie)
2020-07-22 6:33 ` [PATCH v4 1/3] dt-bindings: USB: Add bindings " 周琰杰 (Zhou Yanjie)
2020-07-22 6:33 ` [PATCH v4 2/3] USB: PHY: JZ4770: Add support " 周琰杰 (Zhou Yanjie)
@ 2020-07-22 6:33 ` 周琰杰 (Zhou Yanjie)
2 siblings, 0 replies; 6+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2020-07-22 6:33 UTC (permalink / raw)
To: balbi, gregkh, robh+dt
Cc: linux-usb, linux-kernel, devicetree, paul, prasannatsmkumar,
dongsheng.qiu, aric.pzqi, rick.tyliu, yanfei.li, sernia.zhou,
zhenwenjin
Reformat the code (add one level of indentation before the values),
to align the code in the macro definition section.
Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---
Notes:
v1->v2:
Add support for the JZ4780 SoC.
v2->v3:
No change.
v3->v4:
No change.
drivers/usb/phy/phy-jz4770.c | 100 +++++++++++++++++++++----------------------
1 file changed, 50 insertions(+), 50 deletions(-)
diff --git a/drivers/usb/phy/phy-jz4770.c b/drivers/usb/phy/phy-jz4770.c
index cd49b32b4c13..58af8a014583 100644
--- a/drivers/usb/phy/phy-jz4770.c
+++ b/drivers/usb/phy/phy-jz4770.c
@@ -16,71 +16,71 @@
#include <linux/usb/phy.h>
/* OTGPHY register offsets */
-#define REG_USBPCR_OFFSET 0x00
-#define REG_USBRDT_OFFSET 0x04
-#define REG_USBVBFIL_OFFSET 0x08
-#define REG_USBPCR1_OFFSET 0x0c
+#define REG_USBPCR_OFFSET 0x00
+#define REG_USBRDT_OFFSET 0x04
+#define REG_USBVBFIL_OFFSET 0x08
+#define REG_USBPCR1_OFFSET 0x0c
/* bits within the USBPCR register */
-#define USBPCR_USB_MODE BIT(31)
-#define USBPCR_AVLD_REG BIT(30)
-#define USBPCR_INCR_MASK BIT(27)
-#define USBPCR_COMMONONN BIT(25)
-#define USBPCR_VBUSVLDEXT BIT(24)
-#define USBPCR_VBUSVLDEXTSEL BIT(23)
-#define USBPCR_POR BIT(22)
-#define USBPCR_SIDDQ BIT(21)
-#define USBPCR_OTG_DISABLE BIT(20)
-#define USBPCR_TXPREEMPHTUNE BIT(6)
-
-#define USBPCR_IDPULLUP_LSB 28
-#define USBPCR_IDPULLUP_MASK GENMASK(29, USBPCR_IDPULLUP_LSB)
-#define USBPCR_IDPULLUP_ALWAYS (0x2 << USBPCR_IDPULLUP_LSB)
-#define USBPCR_IDPULLUP_SUSPEND (0x1 << USBPCR_IDPULLUP_LSB)
-#define USBPCR_IDPULLUP_OTG (0x0 << USBPCR_IDPULLUP_LSB)
-
-#define USBPCR_COMPDISTUNE_LSB 17
-#define USBPCR_COMPDISTUNE_MASK GENMASK(19, USBPCR_COMPDISTUNE_LSB)
-#define USBPCR_COMPDISTUNE_DFT (0x4 << USBPCR_COMPDISTUNE_LSB)
-
-#define USBPCR_OTGTUNE_LSB 14
-#define USBPCR_OTGTUNE_MASK GENMASK(16, USBPCR_OTGTUNE_LSB)
-#define USBPCR_OTGTUNE_DFT (0x4 << USBPCR_OTGTUNE_LSB)
-
-#define USBPCR_SQRXTUNE_LSB 11
-#define USBPCR_SQRXTUNE_MASK GENMASK(13, USBPCR_SQRXTUNE_LSB)
+#define USBPCR_USB_MODE BIT(31)
+#define USBPCR_AVLD_REG BIT(30)
+#define USBPCR_INCR_MASK BIT(27)
+#define USBPCR_COMMONONN BIT(25)
+#define USBPCR_VBUSVLDEXT BIT(24)
+#define USBPCR_VBUSVLDEXTSEL BIT(23)
+#define USBPCR_POR BIT(22)
+#define USBPCR_SIDDQ BIT(21)
+#define USBPCR_OTG_DISABLE BIT(20)
+#define USBPCR_TXPREEMPHTUNE BIT(6)
+
+#define USBPCR_IDPULLUP_LSB 28
+#define USBPCR_IDPULLUP_MASK GENMASK(29, USBPCR_IDPULLUP_LSB)
+#define USBPCR_IDPULLUP_ALWAYS (0x2 << USBPCR_IDPULLUP_LSB)
+#define USBPCR_IDPULLUP_SUSPEND (0x1 << USBPCR_IDPULLUP_LSB)
+#define USBPCR_IDPULLUP_OTG (0x0 << USBPCR_IDPULLUP_LSB)
+
+#define USBPCR_COMPDISTUNE_LSB 17
+#define USBPCR_COMPDISTUNE_MASK GENMASK(19, USBPCR_COMPDISTUNE_LSB)
+#define USBPCR_COMPDISTUNE_DFT (0x4 << USBPCR_COMPDISTUNE_LSB)
+
+#define USBPCR_OTGTUNE_LSB 14
+#define USBPCR_OTGTUNE_MASK GENMASK(16, USBPCR_OTGTUNE_LSB)
+#define USBPCR_OTGTUNE_DFT (0x4 << USBPCR_OTGTUNE_LSB)
+
+#define USBPCR_SQRXTUNE_LSB 11
+#define USBPCR_SQRXTUNE_MASK GENMASK(13, USBPCR_SQRXTUNE_LSB)
#define USBPCR_SQRXTUNE_DCR_20PCT (0x7 << USBPCR_SQRXTUNE_LSB)
-#define USBPCR_SQRXTUNE_DFT (0x3 << USBPCR_SQRXTUNE_LSB)
+#define USBPCR_SQRXTUNE_DFT (0x3 << USBPCR_SQRXTUNE_LSB)
-#define USBPCR_TXFSLSTUNE_LSB 7
-#define USBPCR_TXFSLSTUNE_MASK GENMASK(10, USBPCR_TXFSLSTUNE_LSB)
+#define USBPCR_TXFSLSTUNE_LSB 7
+#define USBPCR_TXFSLSTUNE_MASK GENMASK(10, USBPCR_TXFSLSTUNE_LSB)
#define USBPCR_TXFSLSTUNE_DCR_50PPT (0xf << USBPCR_TXFSLSTUNE_LSB)
#define USBPCR_TXFSLSTUNE_DCR_25PPT (0x7 << USBPCR_TXFSLSTUNE_LSB)
-#define USBPCR_TXFSLSTUNE_DFT (0x3 << USBPCR_TXFSLSTUNE_LSB)
+#define USBPCR_TXFSLSTUNE_DFT (0x3 << USBPCR_TXFSLSTUNE_LSB)
#define USBPCR_TXFSLSTUNE_INC_25PPT (0x1 << USBPCR_TXFSLSTUNE_LSB)
#define USBPCR_TXFSLSTUNE_INC_50PPT (0x0 << USBPCR_TXFSLSTUNE_LSB)
-#define USBPCR_TXHSXVTUNE_LSB 4
-#define USBPCR_TXHSXVTUNE_MASK GENMASK(5, USBPCR_TXHSXVTUNE_LSB)
-#define USBPCR_TXHSXVTUNE_DFT (0x3 << USBPCR_TXHSXVTUNE_LSB)
+#define USBPCR_TXHSXVTUNE_LSB 4
+#define USBPCR_TXHSXVTUNE_MASK GENMASK(5, USBPCR_TXHSXVTUNE_LSB)
+#define USBPCR_TXHSXVTUNE_DFT (0x3 << USBPCR_TXHSXVTUNE_LSB)
#define USBPCR_TXHSXVTUNE_DCR_15MV (0x1 << USBPCR_TXHSXVTUNE_LSB)
-#define USBPCR_TXRISETUNE_LSB 4
-#define USBPCR_TXRISETUNE_MASK GENMASK(5, USBPCR_TXRISETUNE_LSB)
-#define USBPCR_TXRISETUNE_DFT (0x3 << USBPCR_TXRISETUNE_LSB)
+#define USBPCR_TXRISETUNE_LSB 4
+#define USBPCR_TXRISETUNE_MASK GENMASK(5, USBPCR_TXRISETUNE_LSB)
+#define USBPCR_TXRISETUNE_DFT (0x3 << USBPCR_TXRISETUNE_LSB)
-#define USBPCR_TXVREFTUNE_LSB 0
-#define USBPCR_TXVREFTUNE_MASK GENMASK(3, USBPCR_TXVREFTUNE_LSB)
+#define USBPCR_TXVREFTUNE_LSB 0
+#define USBPCR_TXVREFTUNE_MASK GENMASK(3, USBPCR_TXVREFTUNE_LSB)
#define USBPCR_TXVREFTUNE_INC_25PPT (0x7 << USBPCR_TXVREFTUNE_LSB)
-#define USBPCR_TXVREFTUNE_DFT (0x5 << USBPCR_TXVREFTUNE_LSB)
+#define USBPCR_TXVREFTUNE_DFT (0x5 << USBPCR_TXVREFTUNE_LSB)
/* bits within the USBRDTR register */
-#define USBRDT_UTMI_RST BIT(27)
-#define USBRDT_HB_MASK BIT(26)
-#define USBRDT_VBFIL_LD_EN BIT(25)
-#define USBRDT_IDDIG_EN BIT(24)
-#define USBRDT_IDDIG_REG BIT(23)
-#define USBRDT_VBFIL_EN BIT(2)
+#define USBRDT_UTMI_RST BIT(27)
+#define USBRDT_HB_MASK BIT(26)
+#define USBRDT_VBFIL_LD_EN BIT(25)
+#define USBRDT_IDDIG_EN BIT(24)
+#define USBRDT_IDDIG_REG BIT(23)
+#define USBRDT_VBFIL_EN BIT(2)
/* bits within the USBPCR1 register */
#define USBPCR1_BVLD_REG BIT(31)
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/3] USB: PHY: JZ4770: Add support for new Ingenic SoCs.
2020-07-22 6:33 ` [PATCH v4 2/3] USB: PHY: JZ4770: Add support " 周琰杰 (Zhou Yanjie)
@ 2020-07-22 17:19 ` Paul Cercueil
2020-07-22 17:43 ` Zhou Yanjie
0 siblings, 1 reply; 6+ messages in thread
From: Paul Cercueil @ 2020-07-22 17:19 UTC (permalink / raw)
To: 周琰杰
Cc: balbi, gregkh, robh+dt, linux-usb, linux-kernel, devicetree,
prasannatsmkumar, dongsheng.qiu, aric.pzqi, rick.tyliu,
yanfei.li, sernia.zhou, zhenwenjin
Hi Zhou,
Le mer. 22 juil. 2020 à 14:33, 周琰杰 (Zhou Yanjie)
<zhouyanjie@wanyeetech.com> a écrit :
> Add support for probing the phy-jz4770 driver on the JZ4780 SoC,
> the X1000 SoC and the X1830 SoC from Ingenic.
>
> Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
>
> Notes:
> v1->v2:
> Add bindings for the JZ4780 SoC.
>
> v2->v3:
> Use "of_device_get_match_data" instead "of_match_device"
> to get version information.
>
> v3->v4:
> Fix typos.
>
> drivers/usb/phy/Kconfig | 4 +-
> drivers/usb/phy/phy-jz4770.c | 174
> ++++++++++++++++++++++++++++++++-----------
> 2 files changed, 133 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index 4b3fa78995cf..775f0dd7b5f5 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -185,11 +185,11 @@ config USB_ULPI_VIEWPORT
> controllers with a viewport register (e.g. Chipidea/ARC
> controllers).
>
> config JZ4770_PHY
> - tristate "Ingenic JZ4770 Transceiver Driver"
> + tristate "Ingenic SoCs Transceiver Driver"
> depends on MIPS || COMPILE_TEST
> select USB_PHY
> help
> This driver provides PHY support for the USB controller found
> - on the JZ4770 SoC from Ingenic.
> + on the JZ4770/JZ4780/X1000/X1830 SoC from Ingenic.
'JZ-series / X-series SoCs' would be more future-proof :)
>
> endmenu
> diff --git a/drivers/usb/phy/phy-jz4770.c
> b/drivers/usb/phy/phy-jz4770.c
> index 8f62dc2a90ff..cd49b32b4c13 100644
> --- a/drivers/usb/phy/phy-jz4770.c
> +++ b/drivers/usb/phy/phy-jz4770.c
> @@ -1,27 +1,30 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * Ingenic JZ4770 USB PHY driver
> + * Ingenic SoCs USB PHY driver
> * Copyright (c) Paul Cercueil <paul@crapouillou.net>
> + * Copyright (c) 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
> + * Copyright (c) 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> */
>
> #include <linux/clk.h>
> #include <linux/io.h>
> #include <linux/module.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/regulator/consumer.h>
> #include <linux/usb/otg.h>
> #include <linux/usb/phy.h>
>
> +/* OTGPHY register offsets */
> #define REG_USBPCR_OFFSET 0x00
> #define REG_USBRDT_OFFSET 0x04
> #define REG_USBVBFIL_OFFSET 0x08
> #define REG_USBPCR1_OFFSET 0x0c
>
> -/* USBPCR */
> +/* bits within the USBPCR register */
> #define USBPCR_USB_MODE BIT(31)
> #define USBPCR_AVLD_REG BIT(30)
> -#define USBPCR_INCRM BIT(27)
> -#define USBPCR_CLK12_EN BIT(26)
> +#define USBPCR_INCR_MASK BIT(27)
Please don't rename macros, it adds a lot of noise to the patch.
> #define USBPCR_COMMONONN BIT(25)
> #define USBPCR_VBUSVLDEXT BIT(24)
> #define USBPCR_VBUSVLDEXTSEL BIT(23)
> @@ -32,46 +35,80 @@
>
> #define USBPCR_IDPULLUP_LSB 28
> #define USBPCR_IDPULLUP_MASK GENMASK(29, USBPCR_IDPULLUP_LSB)
> -#define USBPCR_IDPULLUP_ALWAYS (3 << USBPCR_IDPULLUP_LSB)
> -#define USBPCR_IDPULLUP_SUSPEND (1 << USBPCR_IDPULLUP_LSB)
> -#define USBPCR_IDPULLUP_OTG (0 << USBPCR_IDPULLUP_LSB)
> +#define USBPCR_IDPULLUP_ALWAYS (0x2 << USBPCR_IDPULLUP_LSB)
> +#define USBPCR_IDPULLUP_SUSPEND (0x1 << USBPCR_IDPULLUP_LSB)
> +#define USBPCR_IDPULLUP_OTG (0x0 << USBPCR_IDPULLUP_LSB)
a '3' turned into a '2' here...
>
> #define USBPCR_COMPDISTUNE_LSB 17
> #define USBPCR_COMPDISTUNE_MASK GENMASK(19, USBPCR_COMPDISTUNE_LSB)
> -#define USBPCR_COMPDISTUNE_DFT 4
> +#define USBPCR_COMPDISTUNE_DFT (0x4 << USBPCR_COMPDISTUNE_LSB)
Why? Even if you have a valid reason to do that, it does not belong in
this patch.
>
> #define USBPCR_OTGTUNE_LSB 14
> #define USBPCR_OTGTUNE_MASK GENMASK(16, USBPCR_OTGTUNE_LSB)
> -#define USBPCR_OTGTUNE_DFT 4
> +#define USBPCR_OTGTUNE_DFT (0x4 << USBPCR_OTGTUNE_LSB)
>
> #define USBPCR_SQRXTUNE_LSB 11
> #define USBPCR_SQRXTUNE_MASK GENMASK(13, USBPCR_SQRXTUNE_LSB)
> -#define USBPCR_SQRXTUNE_DFT 3
> +#define USBPCR_SQRXTUNE_DCR_20PCT (0x7 << USBPCR_SQRXTUNE_LSB)
> +#define USBPCR_SQRXTUNE_DFT (0x3 << USBPCR_SQRXTUNE_LSB)
>
> #define USBPCR_TXFSLSTUNE_LSB 7
> #define USBPCR_TXFSLSTUNE_MASK GENMASK(10, USBPCR_TXFSLSTUNE_LSB)
> -#define USBPCR_TXFSLSTUNE_DFT 3
> +#define USBPCR_TXFSLSTUNE_DCR_50PPT (0xf << USBPCR_TXFSLSTUNE_LSB)
> +#define USBPCR_TXFSLSTUNE_DCR_25PPT (0x7 << USBPCR_TXFSLSTUNE_LSB)
> +#define USBPCR_TXFSLSTUNE_DFT (0x3 << USBPCR_TXFSLSTUNE_LSB)
> +#define USBPCR_TXFSLSTUNE_INC_25PPT (0x1 << USBPCR_TXFSLSTUNE_LSB)
> +#define USBPCR_TXFSLSTUNE_INC_50PPT (0x0 << USBPCR_TXFSLSTUNE_LSB)
> +
> +#define USBPCR_TXHSXVTUNE_LSB 4
> +#define USBPCR_TXHSXVTUNE_MASK GENMASK(5, USBPCR_TXHSXVTUNE_LSB)
> +#define USBPCR_TXHSXVTUNE_DFT (0x3 << USBPCR_TXHSXVTUNE_LSB)
> +#define USBPCR_TXHSXVTUNE_DCR_15MV (0x1 << USBPCR_TXHSXVTUNE_LSB)
>
> #define USBPCR_TXRISETUNE_LSB 4
> #define USBPCR_TXRISETUNE_MASK GENMASK(5, USBPCR_TXRISETUNE_LSB)
> -#define USBPCR_TXRISETUNE_DFT 3
> +#define USBPCR_TXRISETUNE_DFT (0x3 << USBPCR_TXRISETUNE_LSB)
>
> #define USBPCR_TXVREFTUNE_LSB 0
> #define USBPCR_TXVREFTUNE_MASK GENMASK(3, USBPCR_TXVREFTUNE_LSB)
> -#define USBPCR_TXVREFTUNE_DFT 5
> +#define USBPCR_TXVREFTUNE_INC_25PPT (0x7 << USBPCR_TXVREFTUNE_LSB)
> +#define USBPCR_TXVREFTUNE_DFT (0x5 << USBPCR_TXVREFTUNE_LSB)
>
> -/* USBRDT */
> +/* bits within the USBRDTR register */
> +#define USBRDT_UTMI_RST BIT(27)
> +#define USBRDT_HB_MASK BIT(26)
> #define USBRDT_VBFIL_LD_EN BIT(25)
> #define USBRDT_IDDIG_EN BIT(24)
> #define USBRDT_IDDIG_REG BIT(23)
> -
> -#define USBRDT_USBRDT_LSB 0
> -#define USBRDT_USBRDT_MASK GENMASK(22, USBRDT_USBRDT_LSB)
> -
> -/* USBPCR1 */
> -#define USBPCR1_UHC_POWON BIT(5)
> +#define USBRDT_VBFIL_EN BIT(2)
> +
> +/* bits within the USBPCR1 register */
> +#define USBPCR1_BVLD_REG BIT(31)
> +#define USBPCR1_DPPD BIT(29)
> +#define USBPCR1_DMPD BIT(28)
> +#define USBPCR1_USB_SEL BIT(28)
> +#define USBPCR1_WORD_IF_16BIT BIT(19)
> +
> +#define USBPCR1_REFCLKSEL_LSB 26
> +#define USBPCR1_REFCLKSEL_MASK GENMASK(27, USBPCR1_REFCLKDIV_LSB)
> +#define USBPCR1_REFCLKSEL_CLKCORE (0x3 << USBPCR1_REFCLKSEL_LSB)
> +
> +#define USBPCR1_REFCLKDIV_LSB 24
> +#define USBPCR1_REFCLKDIV_MASK GENMASK(25, USBPCR1_REFCLKDIV_LSB)
> +#define USBPCR1_REFCLKDIV_48M (0x2 << USBPCR1_REFCLKDIV_LSB)
> +#define USBPCR1_REFCLKDIV_24M (0x1 << USBPCR1_REFCLKDIV_LSB)
> +#define USBPCR1_REFCLKDIV_12M (0x0 << USBPCR1_REFCLKDIV_LSB)
> +
> +enum ingenic_usb_phy_version {
> + ID_JZ4770,
> + ID_JZ4780,
> + ID_X1000,
> + ID_X1830,
> +};
>
> struct jz4770_phy {
> + enum ingenic_usb_phy_version version;
> +
> struct usb_phy phy;
> struct usb_otg otg;
> struct device *dev;
> @@ -96,6 +133,12 @@ static int jz4770_phy_set_peripheral(struct
> usb_otg *otg,
> struct jz4770_phy *priv = otg_to_jz4770_phy(otg);
> u32 reg;
>
> + if (priv->version >= ID_X1000) {
> + reg = readl(priv->base + REG_USBPCR1_OFFSET);
> + reg |= USBPCR1_BVLD_REG;
> + writel(reg, priv->base + REG_USBPCR1_OFFSET);
> + }
> +
> reg = readl(priv->base + REG_USBPCR_OFFSET);
> reg &= ~USBPCR_USB_MODE;
> reg |= USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL |
> USBPCR_OTG_DISABLE;
> @@ -135,17 +178,59 @@ static int jz4770_phy_init(struct usb_phy *phy)
> return err;
> }
>
> - reg = USBPCR_AVLD_REG | USBPCR_COMMONONN | USBPCR_IDPULLUP_ALWAYS |
> - (USBPCR_COMPDISTUNE_DFT << USBPCR_COMPDISTUNE_LSB) |
> - (USBPCR_OTGTUNE_DFT << USBPCR_OTGTUNE_LSB) |
> - (USBPCR_SQRXTUNE_DFT << USBPCR_SQRXTUNE_LSB) |
> - (USBPCR_TXFSLSTUNE_DFT << USBPCR_TXFSLSTUNE_LSB) |
> - (USBPCR_TXRISETUNE_DFT << USBPCR_TXRISETUNE_LSB) |
> - (USBPCR_TXVREFTUNE_DFT << USBPCR_TXVREFTUNE_LSB) |
> - USBPCR_POR;
> + if (priv->version >= ID_X1830) {
> + /* rdt */
> + writel(USBRDT_VBFIL_EN | USBRDT_UTMI_RST, priv->base +
> REG_USBRDT_OFFSET);
> +
> + reg = readl(priv->base + REG_USBPCR1_OFFSET) |
> USBPCR1_WORD_IF_16BIT |
> + USBPCR1_DMPD | USBPCR1_DPPD;
> + writel(reg, priv->base + REG_USBPCR1_OFFSET);
> +
> + reg = USBPCR_IDPULLUP_OTG | USBPCR_VBUSVLDEXT
> | USBPCR_TXPREEMPHTUNE;
> + } else if (priv->version >= ID_X1000) {
> + reg = readl(priv->base + REG_USBPCR1_OFFSET) |
> USBPCR1_WORD_IF_16BIT;
> + writel(reg, priv->base + REG_USBPCR1_OFFSET);
> +
> + reg = USBPCR_SQRXTUNE_DCR_20PCT | USBPCR_TXPREEMPHTUNE |
> + USBPCR_TXHSXVTUNE_DCR_15MV | USBPCR_TXVREFTUNE_INC_25PPT;
> + } else if (priv->version >= ID_JZ4780) {
> + reg = readl(priv->base + REG_USBPCR1_OFFSET) | USBPCR1_USB_SEL |
> + USBPCR1_WORD_IF_16BIT;
> + writel(reg, priv->base + REG_USBPCR1_OFFSET);
> +
> + reg = USBPCR_TXPREEMPHTUNE;
> + } else {
> + reg = USBPCR_AVLD_REG | USBPCR_IDPULLUP_ALWAYS |
> + USBPCR_COMPDISTUNE_DFT | USBPCR_OTGTUNE_DFT |
> + USBPCR_SQRXTUNE_DFT | USBPCR_TXFSLSTUNE_DFT |
> + USBPCR_TXRISETUNE_DFT | USBPCR_TXVREFTUNE_DFT;
> + }
> +
> + reg |= USBPCR_COMMONONN | USBPCR_POR;
> writel(reg, priv->base + REG_USBPCR_OFFSET);
>
> - /* Wait for PHY to reset */
> + /*
> + * Power-On Reset(POR)
> + * Function:This customer-specific signal resets all test registers
> + * and state machines in the USB 2.0 nanoPHY.
> + * The POR signal must be asserted for a minimum of 10 μs.
> + * For POR timing information:
> + *
> + * T0: Power-on reset (POR) is initiated. 0 (reference).
> + * T1: T1 indicates when POR can be set to 1’b0. (To provide
> examples,
> + * values for T2 and T3 are also shown where T1 = T0 + 30 μs.);
> + * In general, T1 must be ≥ T0 + 10 μs. T0 + 10 μs ≤ T1.
> + * T2: T2 indicates when PHYCLOCK, CLK48MOHCI, and CLK12MOHCI are
> + * available at the macro output, based on the USB 2.0 nanoPHY
> + * reference clock source.
> + * Crystal:
> + * • When T1 = T0 + 10 μs:
> + * T2 < T1 + 805 μs = T0 + 815 μs
> + * • When T1 = T0 + 30 μs:
> + * T2 < T1 + 805 μs = T0 + 835 μs
> + * see "Reset and Power-Saving Signals" on page 60 an “Powering Up
> + * and Powering Down the USB 2.0 nanoPHY” on page 73.
> + */
I don't think this comment is very interesting. The code already says
that the hardware needs at least 30 µs to reset, all the low-level
details are in the programming manual for those who want to learn more.
Besides, since you don't actually modify the reset code, why update the
comment in this patch?
> usleep_range(30, 300);
> writel(reg & ~USBPCR_POR, priv->base + REG_USBPCR_OFFSET);
> usleep_range(300, 1000);
> @@ -166,6 +251,15 @@ static void jz4770_phy_remove(void *phy)
> usb_remove_phy(phy);
> }
>
> +static const struct of_device_id ingenic_usb_phy_of_matches[] = {
> + { .compatible = "ingenic,jz4770-phy", .data = (void *) ID_JZ4770 },
> + { .compatible = "ingenic,jz4780-phy", .data = (void *) ID_JZ4780 },
> + { .compatible = "ingenic,x1000-phy", .data = (void *) ID_X1000 },
> + { .compatible = "ingenic,x1830-phy", .data = (void *) ID_X1830 },
I'm not a fan of having ID_* as platform data. Create 'soc_info'
structures, one per supported SoC, and pass them here. Then you can add
a 'version' field of type 'enum ingenic_usb_phy_version'. And add a
'.phy_init' field as a function pointer, that would be called within
jz4770_phy_init(). That would make the jz4770_phy_init() function much
cleaner (no if/else/else/else/...).
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ingenic_usb_phy_of_matches);
> +
> static int jz4770_phy_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -176,11 +270,13 @@ static int jz4770_phy_probe(struct
> platform_device *pdev)
> if (!priv)
> return -ENOMEM;
>
> + priv->version = (enum
> ingenic_usb_phy_version)of_device_get_match_data(dev);
> +
> platform_set_drvdata(pdev, priv);
> priv->dev = dev;
> priv->phy.dev = dev;
> priv->phy.otg = &priv->otg;
> - priv->phy.label = "jz4770-phy";
> + priv->phy.label = "ingenic-usb-phy";
> priv->phy.init = jz4770_phy_init;
> priv->phy.shutdown = jz4770_phy_shutdown;
>
> @@ -221,23 +317,15 @@ static int jz4770_phy_probe(struct
> platform_device *pdev)
> return devm_add_action_or_reset(dev, jz4770_phy_remove, &priv->phy);
> }
>
> -#ifdef CONFIG_OF
> -static const struct of_device_id jz4770_phy_of_matches[] = {
> - { .compatible = "ingenic,jz4770-phy" },
> - { }
> -};
> -MODULE_DEVICE_TABLE(of, jz4770_phy_of_matches);
> -#endif
> -
> -static struct platform_driver jz4770_phy_driver = {
> +static struct platform_driver ingenic_usb_phy_driver = {
> .probe = jz4770_phy_probe,
> .driver = {
> - .name = "jz4770-phy",
> - .of_match_table = of_match_ptr(jz4770_phy_of_matches),
> + .name = "ingenic-usb-phy",
> + .of_match_table = of_match_ptr(ingenic_usb_phy_of_matches),
It's preferred not to rename the functions unless you have a good
reason, and you definitely shouldn't rename the module. It's OK to have
a driver called 'jz4770-phy' even if it supports more SoCs. Renaming it
breaks userspace ABI.
Cheers,
-Paul
> },
> };
> -module_platform_driver(jz4770_phy_driver);
> +module_platform_driver(ingenic_usb_phy_driver);
>
> MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
> -MODULE_DESCRIPTION("Ingenic JZ4770 USB PHY driver");
> +MODULE_DESCRIPTION("Ingenic SoCs USB PHY driver");
> MODULE_LICENSE("GPL");
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/3] USB: PHY: JZ4770: Add support for new Ingenic SoCs.
2020-07-22 17:19 ` Paul Cercueil
@ 2020-07-22 17:43 ` Zhou Yanjie
0 siblings, 0 replies; 6+ messages in thread
From: Zhou Yanjie @ 2020-07-22 17:43 UTC (permalink / raw)
To: Paul Cercueil
Cc: balbi, gregkh, robh+dt, linux-usb, linux-kernel, devicetree,
prasannatsmkumar, dongsheng.qiu, aric.pzqi, rick.tyliu,
yanfei.li, sernia.zhou, zhenwenjin
Hi Paul,
在 2020/7/23 上午1:19, Paul Cercueil 写道:
> Hi Zhou,
>
> Le mer. 22 juil. 2020 à 14:33, 周琰杰 (Zhou Yanjie)
> <zhouyanjie@wanyeetech.com> a écrit :
>> Add support for probing the phy-jz4770 driver on the JZ4780 SoC,
>> the X1000 SoC and the X1830 SoC from Ingenic.
>>
>> Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
>> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> ---
>>
>> Notes:
>> v1->v2:
>> Add bindings for the JZ4780 SoC.
>>
>> v2->v3:
>> Use "of_device_get_match_data" instead "of_match_device"
>> to get version information.
>>
>> v3->v4:
>> Fix typos.
>>
>> drivers/usb/phy/Kconfig | 4 +-
>> drivers/usb/phy/phy-jz4770.c | 174
>> ++++++++++++++++++++++++++++++++-----------
>> 2 files changed, 133 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
>> index 4b3fa78995cf..775f0dd7b5f5 100644
>> --- a/drivers/usb/phy/Kconfig
>> +++ b/drivers/usb/phy/Kconfig
>> @@ -185,11 +185,11 @@ config USB_ULPI_VIEWPORT
>> controllers with a viewport register (e.g. Chipidea/ARC
>> controllers).
>>
>> config JZ4770_PHY
>> - tristate "Ingenic JZ4770 Transceiver Driver"
>> + tristate "Ingenic SoCs Transceiver Driver"
>> depends on MIPS || COMPILE_TEST
>> select USB_PHY
>> help
>> This driver provides PHY support for the USB controller found
>> - on the JZ4770 SoC from Ingenic.
>> + on the JZ4770/JZ4780/X1000/X1830 SoC from Ingenic.
>
> 'JZ-series / X-series SoCs' would be more future-proof :)
>
Sure.
>>
>> endmenu
>> diff --git a/drivers/usb/phy/phy-jz4770.c b/drivers/usb/phy/phy-jz4770.c
>> index 8f62dc2a90ff..cd49b32b4c13 100644
>> --- a/drivers/usb/phy/phy-jz4770.c
>> +++ b/drivers/usb/phy/phy-jz4770.c
>> @@ -1,27 +1,30 @@
>> // SPDX-License-Identifier: GPL-2.0
>> /*
>> - * Ingenic JZ4770 USB PHY driver
>> + * Ingenic SoCs USB PHY driver
>> * Copyright (c) Paul Cercueil <paul@crapouillou.net>
>> + * Copyright (c) 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>> + * Copyright (c) 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> */
>>
>> #include <linux/clk.h>
>> #include <linux/io.h>
>> #include <linux/module.h>
>> +#include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> #include <linux/regulator/consumer.h>
>> #include <linux/usb/otg.h>
>> #include <linux/usb/phy.h>
>>
>> +/* OTGPHY register offsets */
>> #define REG_USBPCR_OFFSET 0x00
>> #define REG_USBRDT_OFFSET 0x04
>> #define REG_USBVBFIL_OFFSET 0x08
>> #define REG_USBPCR1_OFFSET 0x0c
>>
>> -/* USBPCR */
>> +/* bits within the USBPCR register */
>> #define USBPCR_USB_MODE BIT(31)
>> #define USBPCR_AVLD_REG BIT(30)
>> -#define USBPCR_INCRM BIT(27)
>> -#define USBPCR_CLK12_EN BIT(26)
>> +#define USBPCR_INCR_MASK BIT(27)
>
> Please don't rename macros, it adds a lot of noise to the patch.
>
Sure.
>> #define USBPCR_COMMONONN BIT(25)
>> #define USBPCR_VBUSVLDEXT BIT(24)
>> #define USBPCR_VBUSVLDEXTSEL BIT(23)
>> @@ -32,46 +35,80 @@
>>
>> #define USBPCR_IDPULLUP_LSB 28
>> #define USBPCR_IDPULLUP_MASK GENMASK(29, USBPCR_IDPULLUP_LSB)
>> -#define USBPCR_IDPULLUP_ALWAYS (3 << USBPCR_IDPULLUP_LSB)
>> -#define USBPCR_IDPULLUP_SUSPEND (1 << USBPCR_IDPULLUP_LSB)
>> -#define USBPCR_IDPULLUP_OTG (0 << USBPCR_IDPULLUP_LSB)
>> +#define USBPCR_IDPULLUP_ALWAYS (0x2 << USBPCR_IDPULLUP_LSB)
>> +#define USBPCR_IDPULLUP_SUSPEND (0x1 << USBPCR_IDPULLUP_LSB)
>> +#define USBPCR_IDPULLUP_OTG (0x0 << USBPCR_IDPULLUP_LSB)
>
> a '3' turned into a '2' here...
>
From the information in the programming manual, it can be 3 or 2.
I think using 2 with 1 and 0 below seems more in line with reading
habits :)
>>
>> #define USBPCR_COMPDISTUNE_LSB 17
>> #define USBPCR_COMPDISTUNE_MASK GENMASK(19, USBPCR_COMPDISTUNE_LSB)
>> -#define USBPCR_COMPDISTUNE_DFT 4
>> +#define USBPCR_COMPDISTUNE_DFT (0x4 << USBPCR_COMPDISTUNE_LSB)
>
> Why? Even if you have a valid reason to do that, it does not belong in
> this patch.
>
USBPCR_COMPDISTUNE_DFT (and below) is using in jz4770_phy_init(), to do
this can simplify jz4770_phy_init(), do I need separate it as a separate
patch in the next version?
>>
>> #define USBPCR_OTGTUNE_LSB 14
>> #define USBPCR_OTGTUNE_MASK GENMASK(16, USBPCR_OTGTUNE_LSB)
>> -#define USBPCR_OTGTUNE_DFT 4
>> +#define USBPCR_OTGTUNE_DFT (0x4 << USBPCR_OTGTUNE_LSB)
>>
>> #define USBPCR_SQRXTUNE_LSB 11
>> #define USBPCR_SQRXTUNE_MASK GENMASK(13, USBPCR_SQRXTUNE_LSB)
>> -#define USBPCR_SQRXTUNE_DFT 3
>> +#define USBPCR_SQRXTUNE_DCR_20PCT (0x7 << USBPCR_SQRXTUNE_LSB)
>> +#define USBPCR_SQRXTUNE_DFT (0x3 << USBPCR_SQRXTUNE_LSB)
>>
>> #define USBPCR_TXFSLSTUNE_LSB 7
>> #define USBPCR_TXFSLSTUNE_MASK GENMASK(10, USBPCR_TXFSLSTUNE_LSB)
>> -#define USBPCR_TXFSLSTUNE_DFT 3
>> +#define USBPCR_TXFSLSTUNE_DCR_50PPT (0xf << USBPCR_TXFSLSTUNE_LSB)
>> +#define USBPCR_TXFSLSTUNE_DCR_25PPT (0x7 << USBPCR_TXFSLSTUNE_LSB)
>> +#define USBPCR_TXFSLSTUNE_DFT (0x3 << USBPCR_TXFSLSTUNE_LSB)
>> +#define USBPCR_TXFSLSTUNE_INC_25PPT (0x1 << USBPCR_TXFSLSTUNE_LSB)
>> +#define USBPCR_TXFSLSTUNE_INC_50PPT (0x0 << USBPCR_TXFSLSTUNE_LSB)
>> +
>> +#define USBPCR_TXHSXVTUNE_LSB 4
>> +#define USBPCR_TXHSXVTUNE_MASK GENMASK(5, USBPCR_TXHSXVTUNE_LSB)
>> +#define USBPCR_TXHSXVTUNE_DFT (0x3 << USBPCR_TXHSXVTUNE_LSB)
>> +#define USBPCR_TXHSXVTUNE_DCR_15MV (0x1 << USBPCR_TXHSXVTUNE_LSB)
>>
>> #define USBPCR_TXRISETUNE_LSB 4
>> #define USBPCR_TXRISETUNE_MASK GENMASK(5, USBPCR_TXRISETUNE_LSB)
>> -#define USBPCR_TXRISETUNE_DFT 3
>> +#define USBPCR_TXRISETUNE_DFT (0x3 << USBPCR_TXRISETUNE_LSB)
>>
>> #define USBPCR_TXVREFTUNE_LSB 0
>> #define USBPCR_TXVREFTUNE_MASK GENMASK(3, USBPCR_TXVREFTUNE_LSB)
>> -#define USBPCR_TXVREFTUNE_DFT 5
>> +#define USBPCR_TXVREFTUNE_INC_25PPT (0x7 << USBPCR_TXVREFTUNE_LSB)
>> +#define USBPCR_TXVREFTUNE_DFT (0x5 << USBPCR_TXVREFTUNE_LSB)
>>
>> -/* USBRDT */
>> +/* bits within the USBRDTR register */
>> +#define USBRDT_UTMI_RST BIT(27)
>> +#define USBRDT_HB_MASK BIT(26)
>> #define USBRDT_VBFIL_LD_EN BIT(25)
>> #define USBRDT_IDDIG_EN BIT(24)
>> #define USBRDT_IDDIG_REG BIT(23)
>> -
>> -#define USBRDT_USBRDT_LSB 0
>> -#define USBRDT_USBRDT_MASK GENMASK(22, USBRDT_USBRDT_LSB)
>> -
>> -/* USBPCR1 */
>> -#define USBPCR1_UHC_POWON BIT(5)
>> +#define USBRDT_VBFIL_EN BIT(2)
>> +
>> +/* bits within the USBPCR1 register */
>> +#define USBPCR1_BVLD_REG BIT(31)
>> +#define USBPCR1_DPPD BIT(29)
>> +#define USBPCR1_DMPD BIT(28)
>> +#define USBPCR1_USB_SEL BIT(28)
>> +#define USBPCR1_WORD_IF_16BIT BIT(19)
>> +
>> +#define USBPCR1_REFCLKSEL_LSB 26
>> +#define USBPCR1_REFCLKSEL_MASK GENMASK(27,
>> USBPCR1_REFCLKDIV_LSB)
>> +#define USBPCR1_REFCLKSEL_CLKCORE (0x3 << USBPCR1_REFCLKSEL_LSB)
>> +
>> +#define USBPCR1_REFCLKDIV_LSB 24
>> +#define USBPCR1_REFCLKDIV_MASK GENMASK(25,
>> USBPCR1_REFCLKDIV_LSB)
>> +#define USBPCR1_REFCLKDIV_48M (0x2 << USBPCR1_REFCLKDIV_LSB)
>> +#define USBPCR1_REFCLKDIV_24M (0x1 << USBPCR1_REFCLKDIV_LSB)
>> +#define USBPCR1_REFCLKDIV_12M (0x0 << USBPCR1_REFCLKDIV_LSB)
>> +
>> +enum ingenic_usb_phy_version {
>> + ID_JZ4770,
>> + ID_JZ4780,
>> + ID_X1000,
>> + ID_X1830,
>> +};
>>
>> struct jz4770_phy {
>> + enum ingenic_usb_phy_version version;
>> +
>> struct usb_phy phy;
>> struct usb_otg otg;
>> struct device *dev;
>> @@ -96,6 +133,12 @@ static int jz4770_phy_set_peripheral(struct
>> usb_otg *otg,
>> struct jz4770_phy *priv = otg_to_jz4770_phy(otg);
>> u32 reg;
>>
>> + if (priv->version >= ID_X1000) {
>> + reg = readl(priv->base + REG_USBPCR1_OFFSET);
>> + reg |= USBPCR1_BVLD_REG;
>> + writel(reg, priv->base + REG_USBPCR1_OFFSET);
>> + }
>> +
>> reg = readl(priv->base + REG_USBPCR_OFFSET);
>> reg &= ~USBPCR_USB_MODE;
>> reg |= USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL |
>> USBPCR_OTG_DISABLE;
>> @@ -135,17 +178,59 @@ static int jz4770_phy_init(struct usb_phy *phy)
>> return err;
>> }
>>
>> - reg = USBPCR_AVLD_REG | USBPCR_COMMONONN | USBPCR_IDPULLUP_ALWAYS |
>> - (USBPCR_COMPDISTUNE_DFT << USBPCR_COMPDISTUNE_LSB) |
>> - (USBPCR_OTGTUNE_DFT << USBPCR_OTGTUNE_LSB) |
>> - (USBPCR_SQRXTUNE_DFT << USBPCR_SQRXTUNE_LSB) |
>> - (USBPCR_TXFSLSTUNE_DFT << USBPCR_TXFSLSTUNE_LSB) |
>> - (USBPCR_TXRISETUNE_DFT << USBPCR_TXRISETUNE_LSB) |
>> - (USBPCR_TXVREFTUNE_DFT << USBPCR_TXVREFTUNE_LSB) |
>> - USBPCR_POR;
>> + if (priv->version >= ID_X1830) {
>> + /* rdt */
>> + writel(USBRDT_VBFIL_EN | USBRDT_UTMI_RST, priv->base +
>> REG_USBRDT_OFFSET);
>> +
>> + reg = readl(priv->base + REG_USBPCR1_OFFSET) |
>> USBPCR1_WORD_IF_16BIT |
>> + USBPCR1_DMPD | USBPCR1_DPPD;
>> + writel(reg, priv->base + REG_USBPCR1_OFFSET);
>> +
>> + reg = USBPCR_IDPULLUP_OTG | USBPCR_VBUSVLDEXT |
>> USBPCR_TXPREEMPHTUNE;
>> + } else if (priv->version >= ID_X1000) {
>> + reg = readl(priv->base + REG_USBPCR1_OFFSET) |
>> USBPCR1_WORD_IF_16BIT;
>> + writel(reg, priv->base + REG_USBPCR1_OFFSET);
>> +
>> + reg = USBPCR_SQRXTUNE_DCR_20PCT | USBPCR_TXPREEMPHTUNE |
>> + USBPCR_TXHSXVTUNE_DCR_15MV | USBPCR_TXVREFTUNE_INC_25PPT;
>> + } else if (priv->version >= ID_JZ4780) {
>> + reg = readl(priv->base + REG_USBPCR1_OFFSET) |
>> USBPCR1_USB_SEL |
>> + USBPCR1_WORD_IF_16BIT;
>> + writel(reg, priv->base + REG_USBPCR1_OFFSET);
>> +
>> + reg = USBPCR_TXPREEMPHTUNE;
>> + } else {
>> + reg = USBPCR_AVLD_REG | USBPCR_IDPULLUP_ALWAYS |
>> + USBPCR_COMPDISTUNE_DFT | USBPCR_OTGTUNE_DFT |
>> + USBPCR_SQRXTUNE_DFT | USBPCR_TXFSLSTUNE_DFT |
>> + USBPCR_TXRISETUNE_DFT | USBPCR_TXVREFTUNE_DFT;
>> + }
>> +
>> + reg |= USBPCR_COMMONONN | USBPCR_POR;
>> writel(reg, priv->base + REG_USBPCR_OFFSET);
>>
>> - /* Wait for PHY to reset */
>> + /*
>> + * Power-On Reset(POR)
>> + * Function:This customer-specific signal resets all test registers
>> + * and state machines in the USB 2.0 nanoPHY.
>> + * The POR signal must be asserted for a minimum of 10 μs.
>> + * For POR timing information:
>> + *
>> + * T0: Power-on reset (POR) is initiated. 0 (reference).
>> + * T1: T1 indicates when POR can be set to 1’b0. (To provide
>> examples,
>> + * values for T2 and T3 are also shown where T1 = T0 + 30 μs.);
>> + * In general, T1 must be ≥ T0 + 10 μs. T0 + 10 μs ≤ T1.
>> + * T2: T2 indicates when PHYCLOCK, CLK48MOHCI, and CLK12MOHCI are
>> + * available at the macro output, based on the USB 2.0 nanoPHY
>> + * reference clock source.
>> + * Crystal:
>> + * • When T1 = T0 + 10 μs:
>> + * T2 < T1 + 805 μs = T0 + 815 μs
>> + * • When T1 = T0 + 30 μs:
>> + * T2 < T1 + 805 μs = T0 + 835 μs
>> + * see "Reset and Power-Saving Signals" on page 60 an “Powering Up
>> + * and Powering Down the USB 2.0 nanoPHY” on page 73.
>> + */
>
> I don't think this comment is very interesting. The code already says
> that the hardware needs at least 30 µs to reset, all the low-level
> details are in the programming manual for those who want to learn
> more. Besides, since you don't actually modify the reset code, why
> update the comment in this patch?
>
Okay, I will drop it in the next version.
>> usleep_range(30, 300);
>> writel(reg & ~USBPCR_POR, priv->base + REG_USBPCR_OFFSET);
>> usleep_range(300, 1000);
>> @@ -166,6 +251,15 @@ static void jz4770_phy_remove(void *phy)
>> usb_remove_phy(phy);
>> }
>>
>> +static const struct of_device_id ingenic_usb_phy_of_matches[] = {
>> + { .compatible = "ingenic,jz4770-phy", .data = (void *) ID_JZ4770 },
>> + { .compatible = "ingenic,jz4780-phy", .data = (void *) ID_JZ4780 },
>> + { .compatible = "ingenic,x1000-phy", .data = (void *) ID_X1000 },
>> + { .compatible = "ingenic,x1830-phy", .data = (void *) ID_X1830 },
>
> I'm not a fan of having ID_* as platform data. Create 'soc_info'
> structures, one per supported SoC, and pass them here. Then you can
> add a 'version' field of type 'enum ingenic_usb_phy_version'. And add
> a '.phy_init' field as a function pointer, that would be called within
> jz4770_phy_init(). That would make the jz4770_phy_init() function much
> cleaner (no if/else/else/else/...).
Sure, I will do it in the next version.
>
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, ingenic_usb_phy_of_matches);
>> +
>> static int jz4770_phy_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> @@ -176,11 +270,13 @@ static int jz4770_phy_probe(struct
>> platform_device *pdev)
>> if (!priv)
>> return -ENOMEM;
>>
>> + priv->version = (enum
>> ingenic_usb_phy_version)of_device_get_match_data(dev);
>> +
>> platform_set_drvdata(pdev, priv);
>> priv->dev = dev;
>> priv->phy.dev = dev;
>> priv->phy.otg = &priv->otg;
>> - priv->phy.label = "jz4770-phy";
>> + priv->phy.label = "ingenic-usb-phy";
>> priv->phy.init = jz4770_phy_init;
>> priv->phy.shutdown = jz4770_phy_shutdown;
>>
>> @@ -221,23 +317,15 @@ static int jz4770_phy_probe(struct
>> platform_device *pdev)
>> return devm_add_action_or_reset(dev, jz4770_phy_remove,
>> &priv->phy);
>> }
>>
>> -#ifdef CONFIG_OF
>> -static const struct of_device_id jz4770_phy_of_matches[] = {
>> - { .compatible = "ingenic,jz4770-phy" },
>> - { }
>> -};
>> -MODULE_DEVICE_TABLE(of, jz4770_phy_of_matches);
>> -#endif
>> -
>> -static struct platform_driver jz4770_phy_driver = {
>> +static struct platform_driver ingenic_usb_phy_driver = {
>> .probe = jz4770_phy_probe,
>> .driver = {
>> - .name = "jz4770-phy",
>> - .of_match_table = of_match_ptr(jz4770_phy_of_matches),
>> + .name = "ingenic-usb-phy",
>> + .of_match_table = of_match_ptr(ingenic_usb_phy_of_matches),
>
> It's preferred not to rename the functions unless you have a good
> reason, and you definitely shouldn't rename the module. It's OK to
> have a driver called 'jz4770-phy' even if it supports more SoCs.
> Renaming it breaks userspace ABI.
Sure.
Thanks and best regards!
>
> Cheers,
> -Paul
>
>> },
>> };
>> -module_platform_driver(jz4770_phy_driver);
>> +module_platform_driver(ingenic_usb_phy_driver);
>>
>> MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
>> -MODULE_DESCRIPTION("Ingenic JZ4770 USB PHY driver");
>> +MODULE_DESCRIPTION("Ingenic SoCs USB PHY driver");
>> MODULE_LICENSE("GPL");
>> --
>> 2.11.0
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-22 17:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-22 6:33 [PATCH v4 0/3] Add USB PHY support for new Ingenic SoCs 周琰杰 (Zhou Yanjie)
2020-07-22 6:33 ` [PATCH v4 1/3] dt-bindings: USB: Add bindings " 周琰杰 (Zhou Yanjie)
2020-07-22 6:33 ` [PATCH v4 2/3] USB: PHY: JZ4770: Add support " 周琰杰 (Zhou Yanjie)
2020-07-22 17:19 ` Paul Cercueil
2020-07-22 17:43 ` Zhou Yanjie
2020-07-22 6:33 ` [PATCH v4 3/3] USB: PHY: JZ4770: Reformat the code to align it 周琰杰 (Zhou Yanjie)
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).