* [PATCH v3 0/2] Use the generic PHY framework for Ingenic USB PHY. @ 2020-09-13 6:34 周琰杰 (Zhou Yanjie) 2020-09-13 6:34 ` [PATCH v3 1/2] USB: PHY: JZ4770: Remove unnecessary function calls 周琰杰 (Zhou Yanjie) 2020-09-13 6:34 ` [PATCH v3 2/2] USB: PHY: JZ4770: Use the generic PHY framework 周琰杰 (Zhou Yanjie) 0 siblings, 2 replies; 8+ messages in thread From: 周琰杰 (Zhou Yanjie) @ 2020-09-13 6:34 UTC (permalink / raw) To: balbi, gregkh, kishon, vkoul, paul Cc: linux-kernel, linux-usb, dongsheng.qiu, aric.pzqi, rick.tyliu, yanfei.li, sernia.zhou, zhenwenjin v2->v3: 1.Remove unnecessary "of_match_ptr()" before the move to the generic PHY. 2.Change "depends on (MACH_INGENIC && MIPS) || COMPILE_TEST" to "depends on MIPS || COMPILE_TEST". 3.Keep the adjustments of "ingenic_usb_phy_init()" and "ingenic_usb_phu_exit()" positions in v2 to make them consistent with the order in "ingenic_usb_phy_ops", keep the adjustments to the positions of "ingenic_usb_phy_of_matches[]" in v2 to keep them consistent with the styles of other USB PHY drivers. And remove some unnecessary changes to reduce the diff size. 周琰杰 (Zhou Yanjie) (2): USB: PHY: JZ4770: Remove unnecessary function calls. USB: PHY: JZ4770: Use the generic PHY framework. drivers/phy/Kconfig | 1 + drivers/phy/Makefile | 1 + drivers/phy/ingenic/Kconfig | 12 ++ drivers/phy/ingenic/Makefile | 2 + .../phy-jz4770.c => phy/ingenic/phy-ingenic-usb.c} | 211 +++++++++++---------- drivers/usb/phy/Kconfig | 8 - drivers/usb/phy/Makefile | 1 - 7 files changed, 129 insertions(+), 107 deletions(-) create mode 100644 drivers/phy/ingenic/Kconfig create mode 100644 drivers/phy/ingenic/Makefile rename drivers/{usb/phy/phy-jz4770.c => phy/ingenic/phy-ingenic-usb.c} (70%) -- 2.11.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/2] USB: PHY: JZ4770: Remove unnecessary function calls. 2020-09-13 6:34 [PATCH v3 0/2] Use the generic PHY framework for Ingenic USB PHY 周琰杰 (Zhou Yanjie) @ 2020-09-13 6:34 ` 周琰杰 (Zhou Yanjie) 2020-09-13 6:34 ` [PATCH v3 2/2] USB: PHY: JZ4770: Use the generic PHY framework 周琰杰 (Zhou Yanjie) 1 sibling, 0 replies; 8+ messages in thread From: 周琰杰 (Zhou Yanjie) @ 2020-09-13 6:34 UTC (permalink / raw) To: balbi, gregkh, kishon, vkoul, paul Cc: linux-kernel, linux-usb, dongsheng.qiu, aric.pzqi, rick.tyliu, yanfei.li, sernia.zhou, zhenwenjin Remove unnecessary "of_match_ptr()", because Ingenic SoCs all depend on Device Tree. Suggested-by: Paul Cercueil <paul@crapouillou.net> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> --- Notes: v3: New patch. drivers/usb/phy/phy-jz4770.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/phy/phy-jz4770.c b/drivers/usb/phy/phy-jz4770.c index f6d3731581eb..4025da20b3fd 100644 --- a/drivers/usb/phy/phy-jz4770.c +++ b/drivers/usb/phy/phy-jz4770.c @@ -350,7 +350,7 @@ static struct platform_driver ingenic_phy_driver = { .probe = jz4770_phy_probe, .driver = { .name = "jz4770-phy", - .of_match_table = of_match_ptr(ingenic_usb_phy_of_matches), + .of_match_table = ingenic_usb_phy_of_matches, }, }; module_platform_driver(ingenic_phy_driver); -- 2.11.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/2] USB: PHY: JZ4770: Use the generic PHY framework. 2020-09-13 6:34 [PATCH v3 0/2] Use the generic PHY framework for Ingenic USB PHY 周琰杰 (Zhou Yanjie) 2020-09-13 6:34 ` [PATCH v3 1/2] USB: PHY: JZ4770: Remove unnecessary function calls 周琰杰 (Zhou Yanjie) @ 2020-09-13 6:34 ` 周琰杰 (Zhou Yanjie) 2020-09-15 22:47 ` Paul Cercueil ` (2 more replies) 1 sibling, 3 replies; 8+ messages in thread From: 周琰杰 (Zhou Yanjie) @ 2020-09-13 6:34 UTC (permalink / raw) To: balbi, gregkh, kishon, vkoul, paul Cc: linux-kernel, linux-usb, dongsheng.qiu, aric.pzqi, rick.tyliu, yanfei.li, sernia.zhou, zhenwenjin Used the generic PHY framework API to create the PHY, and move the driver to driver/phy/ingenic. And adjust the position of some codes to make it consistent with the style of other USB PHY drivers. 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: Fix bug, ".of_match_table = of_match_ptr(ingenic_usb_phy_of_matches)" is wrong and should be replaced with ".of_match_table = ingenic_usb_phy_of_matches". v2->v3: 1.Change "depends on (MACH_INGENIC && MIPS) || COMPILE_TEST" to "depends on MIPS || COMPILE_TEST". 2.Keep the adjustments of "ingenic_usb_phy_init()" and "ingenic_usb_phu_exit()" positions in v2 to make them consistent with the order in "ingenic_usb_phy_ops", keep the adjustments to the positions of "ingenic_usb_phy_of_matches[]" in v2 to keep them consistent with the styles of other USB PHY drivers. And remove some unnecessary changes to reduce the diff size, from the original 256 lines change to the current 209 lines. drivers/phy/Kconfig | 1 + drivers/phy/Makefile | 1 + drivers/phy/ingenic/Kconfig | 12 ++ drivers/phy/ingenic/Makefile | 2 + .../phy-jz4770.c => phy/ingenic/phy-ingenic-usb.c} | 209 +++++++++++---------- drivers/usb/phy/Kconfig | 8 - drivers/usb/phy/Makefile | 1 - 7 files changed, 128 insertions(+), 106 deletions(-) create mode 100644 drivers/phy/ingenic/Kconfig create mode 100644 drivers/phy/ingenic/Makefile rename drivers/{usb/phy/phy-jz4770.c => phy/ingenic/phy-ingenic-usb.c} (71%) diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index de9362c25c07..0534b0fdd057 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -55,6 +55,7 @@ source "drivers/phy/broadcom/Kconfig" source "drivers/phy/cadence/Kconfig" source "drivers/phy/freescale/Kconfig" source "drivers/phy/hisilicon/Kconfig" +source "drivers/phy/ingenic/Kconfig" source "drivers/phy/lantiq/Kconfig" source "drivers/phy/marvell/Kconfig" source "drivers/phy/mediatek/Kconfig" diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index c27408e4daae..ab24f0d20763 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -14,6 +14,7 @@ obj-y += allwinner/ \ cadence/ \ freescale/ \ hisilicon/ \ + ingenic/ \ intel/ \ lantiq/ \ marvell/ \ diff --git a/drivers/phy/ingenic/Kconfig b/drivers/phy/ingenic/Kconfig new file mode 100644 index 000000000000..912b14e512cb --- /dev/null +++ b/drivers/phy/ingenic/Kconfig @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# Phy drivers for Ingenic platforms +# +config PHY_INGENIC_USB + tristate "Ingenic SoCs USB PHY Driver" + depends on MIPS || COMPILE_TEST + depends on USB_SUPPORT + select GENERIC_PHY + help + This driver provides USB PHY support for the USB controller found + on the JZ-series and X-series SoCs from Ingenic. diff --git a/drivers/phy/ingenic/Makefile b/drivers/phy/ingenic/Makefile new file mode 100644 index 000000000000..65d5ea00fc9d --- /dev/null +++ b/drivers/phy/ingenic/Makefile @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0 +obj-y += phy-ingenic-usb.o diff --git a/drivers/usb/phy/phy-jz4770.c b/drivers/phy/ingenic/phy-ingenic-usb.c similarity index 71% rename from drivers/usb/phy/phy-jz4770.c rename to drivers/phy/ingenic/phy-ingenic-usb.c index 4025da20b3fd..1f2e811c9125 100644 --- a/drivers/usb/phy/phy-jz4770.c +++ b/drivers/phy/ingenic/phy-ingenic-usb.c @@ -7,12 +7,12 @@ */ #include <linux/clk.h> +#include <linux/delay.h> #include <linux/io.h> #include <linux/module.h> #include <linux/platform_device.h> #include <linux/regulator/consumer.h> -#include <linux/usb/otg.h> -#include <linux/usb/phy.h> +#include <linux/phy/phy.h> /* OTGPHY register offsets */ #define REG_USBPCR_OFFSET 0x00 @@ -97,68 +97,56 @@ enum ingenic_usb_phy_version { struct ingenic_soc_info { enum ingenic_usb_phy_version version; - void (*usb_phy_init)(struct usb_phy *phy); + void (*usb_phy_init)(struct phy *phy); }; -struct jz4770_phy { +struct ingenic_usb_phy { const struct ingenic_soc_info *soc_info; - struct usb_phy phy; - struct usb_otg otg; + struct phy *phy; struct device *dev; void __iomem *base; struct clk *clk; struct regulator *vcc_supply; }; -static inline struct jz4770_phy *otg_to_jz4770_phy(struct usb_otg *otg) +static int ingenic_usb_phy_init(struct phy *phy) { - return container_of(otg, struct jz4770_phy, otg); -} - -static inline struct jz4770_phy *phy_to_jz4770_phy(struct usb_phy *phy) -{ - return container_of(phy, struct jz4770_phy, phy); -} - -static int ingenic_usb_phy_set_peripheral(struct usb_otg *otg, - struct usb_gadget *gadget) -{ - struct jz4770_phy *priv = otg_to_jz4770_phy(otg); + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); + int err; u32 reg; - if (priv->soc_info->version >= ID_X1000) { - reg = readl(priv->base + REG_USBPCR1_OFFSET); - reg |= USBPCR1_BVLD_REG; - writel(reg, priv->base + REG_USBPCR1_OFFSET); + err = clk_prepare_enable(priv->clk); + if (err) { + dev_err(priv->dev, "Unable to start clock: %d\n", err); + return err; } + priv->soc_info->usb_phy_init(phy); + + /* Wait for PHY to reset */ + usleep_range(30, 300); reg = readl(priv->base + REG_USBPCR_OFFSET); - reg &= ~USBPCR_USB_MODE; - reg |= USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL | USBPCR_OTG_DISABLE; - writel(reg, priv->base + REG_USBPCR_OFFSET); + writel(reg & ~USBPCR_POR, priv->base + REG_USBPCR_OFFSET); + usleep_range(300, 1000); return 0; } -static int ingenic_usb_phy_set_host(struct usb_otg *otg, struct usb_bus *host) +static int ingenic_usb_phy_exit(struct phy *phy) { - struct jz4770_phy *priv = otg_to_jz4770_phy(otg); - u32 reg; + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); - reg = readl(priv->base + REG_USBPCR_OFFSET); - reg &= ~(USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL | USBPCR_OTG_DISABLE); - reg |= USBPCR_USB_MODE; - writel(reg, priv->base + REG_USBPCR_OFFSET); + clk_disable_unprepare(priv->clk); + regulator_disable(priv->vcc_supply); return 0; } -static int ingenic_usb_phy_init(struct usb_phy *phy) +static int ingenic_usb_phy_power_on(struct phy *phy) { - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); int err; - u32 reg; err = regulator_enable(priv->vcc_supply); if (err) { @@ -166,39 +154,71 @@ static int ingenic_usb_phy_init(struct usb_phy *phy) return err; } - err = clk_prepare_enable(priv->clk); - if (err) { - dev_err(priv->dev, "Unable to start clock: %d\n", err); - return err; - } - - priv->soc_info->usb_phy_init(phy); - - /* Wait for PHY to reset */ - usleep_range(30, 300); - reg = readl(priv->base + REG_USBPCR_OFFSET); - writel(reg & ~USBPCR_POR, priv->base + REG_USBPCR_OFFSET); - usleep_range(300, 1000); - return 0; } -static void ingenic_usb_phy_shutdown(struct usb_phy *phy) +static int ingenic_usb_phy_power_off(struct phy *phy) { - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); - clk_disable_unprepare(priv->clk); regulator_disable(priv->vcc_supply); + + return 0; } -static void ingenic_usb_phy_remove(void *phy) +static int ingenic_usb_phy_set_mode(struct phy *phy, + enum phy_mode mode, int submode) { - usb_remove_phy(phy); + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); + u32 reg; + + switch (mode) { + case PHY_MODE_USB_HOST: + reg = readl(priv->base + REG_USBPCR_OFFSET); + reg &= ~(USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL | USBPCR_OTG_DISABLE); + reg |= USBPCR_USB_MODE; + writel(reg, priv->base + REG_USBPCR_OFFSET); + + break; + case PHY_MODE_USB_DEVICE: + if (priv->soc_info->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; + writel(reg, priv->base + REG_USBPCR_OFFSET); + + break; + case PHY_MODE_USB_OTG: + reg = readl(priv->base + REG_USBPCR_OFFSET); + reg &= ~USBPCR_OTG_DISABLE; + reg |= USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL | USBPCR_USB_MODE; + writel(reg, priv->base + REG_USBPCR_OFFSET); + + break; + default: + return -EINVAL; + } + + return 0; } -static void jz4770_usb_phy_init(struct usb_phy *phy) +static const struct phy_ops ingenic_usb_phy_ops = { + .init = ingenic_usb_phy_init, + .exit = ingenic_usb_phy_exit, + .power_on = ingenic_usb_phy_power_on, + .power_off = ingenic_usb_phy_power_off, + .set_mode = ingenic_usb_phy_set_mode, + .owner = THIS_MODULE, +}; + +static void jz4770_usb_phy_init(struct phy *phy) { - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); u32 reg; reg = USBPCR_AVLD_REG | USBPCR_COMMONONN | USBPCR_IDPULLUP_ALWAYS | @@ -208,9 +228,9 @@ static void jz4770_usb_phy_init(struct usb_phy *phy) writel(reg, priv->base + REG_USBPCR_OFFSET); } -static void jz4780_usb_phy_init(struct usb_phy *phy) +static void jz4780_usb_phy_init(struct phy *phy) { - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); u32 reg; reg = readl(priv->base + REG_USBPCR1_OFFSET) | USBPCR1_USB_SEL | @@ -221,9 +241,9 @@ static void jz4780_usb_phy_init(struct usb_phy *phy) writel(reg, priv->base + REG_USBPCR_OFFSET); } -static void x1000_usb_phy_init(struct usb_phy *phy) +static void x1000_usb_phy_init(struct phy *phy) { - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); u32 reg; reg = readl(priv->base + REG_USBPCR1_OFFSET) | USBPCR1_WORD_IF_16BIT; @@ -235,9 +255,9 @@ static void x1000_usb_phy_init(struct usb_phy *phy) writel(reg, priv->base + REG_USBPCR_OFFSET); } -static void x1830_usb_phy_init(struct usb_phy *phy) +static void x1830_usb_phy_init(struct phy *phy) { - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); u32 reg; /* rdt */ @@ -276,43 +296,25 @@ static const struct ingenic_soc_info x1830_soc_info = { .usb_phy_init = x1830_usb_phy_init, }; -static const struct of_device_id ingenic_usb_phy_of_matches[] = { - { .compatible = "ingenic,jz4770-phy", .data = &jz4770_soc_info }, - { .compatible = "ingenic,jz4780-phy", .data = &jz4780_soc_info }, - { .compatible = "ingenic,x1000-phy", .data = &x1000_soc_info }, - { .compatible = "ingenic,x1830-phy", .data = &x1830_soc_info }, - { /* sentinel */ } -}; -MODULE_DEVICE_TABLE(of, ingenic_usb_phy_of_matches); - -static int jz4770_phy_probe(struct platform_device *pdev) +static int ingenic_usb_phy_probe(struct platform_device *pdev) { + struct ingenic_usb_phy *priv; + struct phy_provider *provider; struct device *dev = &pdev->dev; - struct jz4770_phy *priv; int err; priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM; - priv->soc_info = device_get_match_data(&pdev->dev); + priv->soc_info = device_get_match_data(dev); if (!priv->soc_info) { - dev_err(&pdev->dev, "Error: No device match found\n"); + dev_err(dev, "Error: No device match found\n"); return -ENODEV; } platform_set_drvdata(pdev, priv); priv->dev = dev; - priv->phy.dev = dev; - priv->phy.otg = &priv->otg; - priv->phy.label = "ingenic-usb-phy"; - priv->phy.init = ingenic_usb_phy_init; - priv->phy.shutdown = ingenic_usb_phy_shutdown; - - priv->otg.state = OTG_STATE_UNDEFINED; - priv->otg.usb_phy = &priv->phy; - priv->otg.set_host = ingenic_usb_phy_set_host; - priv->otg.set_peripheral = ingenic_usb_phy_set_peripheral; priv->base = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(priv->base)) { @@ -336,27 +338,40 @@ static int jz4770_phy_probe(struct platform_device *pdev) return err; } - err = usb_add_phy(&priv->phy, USB_PHY_TYPE_USB2); - if (err) { - if (err != -EPROBE_DEFER) - dev_err(dev, "Unable to register PHY\n"); - return err; - } + priv->phy = devm_phy_create(priv->dev, NULL, &ingenic_usb_phy_ops); + if (IS_ERR(priv)) + return PTR_ERR(priv); + + phy_set_drvdata(priv->phy, priv); - return devm_add_action_or_reset(dev, ingenic_usb_phy_remove, &priv->phy); + provider = devm_of_phy_provider_register(priv->dev, of_phy_simple_xlate); + if (IS_ERR(provider)) + return PTR_ERR(provider); + + return 0; } -static struct platform_driver ingenic_phy_driver = { - .probe = jz4770_phy_probe, +static const struct of_device_id ingenic_usb_phy_of_matches[] = { + { .compatible = "ingenic,jz4770-phy", .data = &jz4770_soc_info }, + { .compatible = "ingenic,jz4780-phy", .data = &jz4780_soc_info }, + { .compatible = "ingenic,x1000-phy", .data = &x1000_soc_info }, + { .compatible = "ingenic,x1830-phy", .data = &x1830_soc_info }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, ingenic_usb_phy_of_matches); + +static struct platform_driver ingenic_usb_phy_driver = { + .probe = ingenic_usb_phy_probe, .driver = { - .name = "jz4770-phy", + .name = "ingenic-usb-phy", .of_match_table = ingenic_usb_phy_of_matches, }, }; -module_platform_driver(ingenic_phy_driver); +module_platform_driver(ingenic_usb_phy_driver); MODULE_AUTHOR("周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>"); MODULE_AUTHOR("漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>"); MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>"); MODULE_DESCRIPTION("Ingenic SoCs USB PHY driver"); +MODULE_ALIAS("jz4770-phy"); MODULE_LICENSE("GPL"); diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index ef4787cd3d37..ff24fca0a2d9 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -184,12 +184,4 @@ config USB_ULPI_VIEWPORT Provides read/write operations to the ULPI phy register set for controllers with a viewport register (e.g. Chipidea/ARC controllers). -config JZ4770_PHY - 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 JZ-series and X-series SoCs from Ingenic. - endmenu diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index b352bdbe8712..df1d99010079 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -24,4 +24,3 @@ obj-$(CONFIG_USB_MXS_PHY) += phy-mxs-usb.o obj-$(CONFIG_USB_ULPI) += phy-ulpi.o obj-$(CONFIG_USB_ULPI_VIEWPORT) += phy-ulpi-viewport.o obj-$(CONFIG_KEYSTONE_USB_PHY) += phy-keystone.o -obj-$(CONFIG_JZ4770_PHY) += phy-jz4770.o -- 2.11.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] USB: PHY: JZ4770: Use the generic PHY framework. 2020-09-13 6:34 ` [PATCH v3 2/2] USB: PHY: JZ4770: Use the generic PHY framework 周琰杰 (Zhou Yanjie) @ 2020-09-15 22:47 ` Paul Cercueil 2020-09-16 16:27 ` Zhou Yanjie 2020-09-16 6:55 ` kernel test robot 2020-09-16 6:55 ` kernel test robot 2 siblings, 1 reply; 8+ messages in thread From: Paul Cercueil @ 2020-09-15 22:47 UTC (permalink / raw) To: 周琰杰 Cc: balbi, gregkh, kishon, vkoul, linux-kernel, linux-usb, dongsheng.qiu, aric.pzqi, rick.tyliu, yanfei.li, sernia.zhou, zhenwenjin Hi Zhou, Le dim. 13 sept. 2020 à 14:34, 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> a écrit : > Used the generic PHY framework API to create the PHY, > and move the driver to driver/phy/ingenic. And adjust > the position of some codes to make it consistent with > the style of other USB PHY drivers. > > 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: > Fix bug, ".of_match_table = > of_match_ptr(ingenic_usb_phy_of_matches)" is wrong > and should be replaced with ".of_match_table = > ingenic_usb_phy_of_matches". > > v2->v3: > 1.Change "depends on (MACH_INGENIC && MIPS) || COMPILE_TEST" to > "depends on MIPS || COMPILE_TEST". > 2.Keep the adjustments of "ingenic_usb_phy_init()" and > "ingenic_usb_phu_exit()" > positions in v2 to make them consistent with the order in > "ingenic_usb_phy_ops", > keep the adjustments to the positions of > "ingenic_usb_phy_of_matches[]" in v2 > to keep them consistent with the styles of other USB PHY > drivers. And remove > some unnecessary changes to reduce the diff size, from the > original 256 lines > change to the current 209 lines. > > drivers/phy/Kconfig | 1 + > drivers/phy/Makefile | 1 + > drivers/phy/ingenic/Kconfig | 12 ++ > drivers/phy/ingenic/Makefile | 2 + > .../phy-jz4770.c => phy/ingenic/phy-ingenic-usb.c} | 209 > +++++++++++---------- > drivers/usb/phy/Kconfig | 8 - > drivers/usb/phy/Makefile | 1 - > 7 files changed, 128 insertions(+), 106 deletions(-) > create mode 100644 drivers/phy/ingenic/Kconfig > create mode 100644 drivers/phy/ingenic/Makefile > rename drivers/{usb/phy/phy-jz4770.c => > phy/ingenic/phy-ingenic-usb.c} (71%) > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index de9362c25c07..0534b0fdd057 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -55,6 +55,7 @@ source "drivers/phy/broadcom/Kconfig" > source "drivers/phy/cadence/Kconfig" > source "drivers/phy/freescale/Kconfig" > source "drivers/phy/hisilicon/Kconfig" > +source "drivers/phy/ingenic/Kconfig" > source "drivers/phy/lantiq/Kconfig" > source "drivers/phy/marvell/Kconfig" > source "drivers/phy/mediatek/Kconfig" > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > index c27408e4daae..ab24f0d20763 100644 > --- a/drivers/phy/Makefile > +++ b/drivers/phy/Makefile > @@ -14,6 +14,7 @@ obj-y += allwinner/ \ > cadence/ \ > freescale/ \ > hisilicon/ \ > + ingenic/ \ > intel/ \ > lantiq/ \ > marvell/ \ > diff --git a/drivers/phy/ingenic/Kconfig b/drivers/phy/ingenic/Kconfig > new file mode 100644 > index 000000000000..912b14e512cb > --- /dev/null > +++ b/drivers/phy/ingenic/Kconfig > @@ -0,0 +1,12 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# > +# Phy drivers for Ingenic platforms > +# > +config PHY_INGENIC_USB > + tristate "Ingenic SoCs USB PHY Driver" > + depends on MIPS || COMPILE_TEST > + depends on USB_SUPPORT > + select GENERIC_PHY > + help > + This driver provides USB PHY support for the USB controller found > + on the JZ-series and X-series SoCs from Ingenic. > diff --git a/drivers/phy/ingenic/Makefile > b/drivers/phy/ingenic/Makefile > new file mode 100644 > index 000000000000..65d5ea00fc9d > --- /dev/null > +++ b/drivers/phy/ingenic/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0 > +obj-y += phy-ingenic-usb.o > diff --git a/drivers/usb/phy/phy-jz4770.c > b/drivers/phy/ingenic/phy-ingenic-usb.c > similarity index 71% > rename from drivers/usb/phy/phy-jz4770.c > rename to drivers/phy/ingenic/phy-ingenic-usb.c > index 4025da20b3fd..1f2e811c9125 100644 > --- a/drivers/usb/phy/phy-jz4770.c > +++ b/drivers/phy/ingenic/phy-ingenic-usb.c > @@ -7,12 +7,12 @@ > */ > > #include <linux/clk.h> > +#include <linux/delay.h> > #include <linux/io.h> > #include <linux/module.h> > #include <linux/platform_device.h> > #include <linux/regulator/consumer.h> > -#include <linux/usb/otg.h> > -#include <linux/usb/phy.h> > +#include <linux/phy/phy.h> > > /* OTGPHY register offsets */ > #define REG_USBPCR_OFFSET 0x00 > @@ -97,68 +97,56 @@ enum ingenic_usb_phy_version { > struct ingenic_soc_info { > enum ingenic_usb_phy_version version; > > - void (*usb_phy_init)(struct usb_phy *phy); > + void (*usb_phy_init)(struct phy *phy); > }; > > -struct jz4770_phy { > +struct ingenic_usb_phy { > const struct ingenic_soc_info *soc_info; > > - struct usb_phy phy; > - struct usb_otg otg; > + struct phy *phy; > struct device *dev; > void __iomem *base; > struct clk *clk; > struct regulator *vcc_supply; > }; > > -static inline struct jz4770_phy *otg_to_jz4770_phy(struct usb_otg > *otg) > +static int ingenic_usb_phy_init(struct phy *phy) > { > - return container_of(otg, struct jz4770_phy, otg); > -} > - > -static inline struct jz4770_phy *phy_to_jz4770_phy(struct usb_phy > *phy) > -{ > - return container_of(phy, struct jz4770_phy, phy); > -} > - > -static int ingenic_usb_phy_set_peripheral(struct usb_otg *otg, > - struct usb_gadget *gadget) > -{ > - struct jz4770_phy *priv = otg_to_jz4770_phy(otg); > + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); > + int err; > u32 reg; > > - if (priv->soc_info->version >= ID_X1000) { > - reg = readl(priv->base + REG_USBPCR1_OFFSET); > - reg |= USBPCR1_BVLD_REG; > - writel(reg, priv->base + REG_USBPCR1_OFFSET); > + err = clk_prepare_enable(priv->clk); > + if (err) { > + dev_err(priv->dev, "Unable to start clock: %d\n", err); > + return err; > } > > + priv->soc_info->usb_phy_init(phy); > + > + /* Wait for PHY to reset */ > + usleep_range(30, 300); > reg = readl(priv->base + REG_USBPCR_OFFSET); > - reg &= ~USBPCR_USB_MODE; > - reg |= USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL | > USBPCR_OTG_DISABLE; > - writel(reg, priv->base + REG_USBPCR_OFFSET); > + writel(reg & ~USBPCR_POR, priv->base + REG_USBPCR_OFFSET); > + usleep_range(300, 1000); > > return 0; > } > > -static int ingenic_usb_phy_set_host(struct usb_otg *otg, struct > usb_bus *host) > +static int ingenic_usb_phy_exit(struct phy *phy) > { > - struct jz4770_phy *priv = otg_to_jz4770_phy(otg); > - u32 reg; > + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); > > - reg = readl(priv->base + REG_USBPCR_OFFSET); > - reg &= ~(USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL | > USBPCR_OTG_DISABLE); > - reg |= USBPCR_USB_MODE; > - writel(reg, priv->base + REG_USBPCR_OFFSET); > + clk_disable_unprepare(priv->clk); > + regulator_disable(priv->vcc_supply); > > return 0; > } That part is still messy. You remove ingenic_usb_phy_set_peripheral() and ingenic_usb_phy_set_host(), then add ingenic_usb_phy_init() and ingenic_usb_phy_exit(). Why were the two first functions removed? Just keep these around, and call them from ingenic_usb_phy_set_mode(). That will automatically make the diff much smaller. And by making the diff smaller, we can spot the actual differences. Like how support for PHY_MODE_USB_OTG was added, without any notice. > > -static int ingenic_usb_phy_init(struct usb_phy *phy) > +static int ingenic_usb_phy_power_on(struct phy *phy) > { > - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); > + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); > int err; > - u32 reg; > > err = regulator_enable(priv->vcc_supply); > if (err) { > @@ -166,39 +154,71 @@ static int ingenic_usb_phy_init(struct usb_phy > *phy) > return err; > } > > - err = clk_prepare_enable(priv->clk); > - if (err) { > - dev_err(priv->dev, "Unable to start clock: %d\n", err); > - return err; > - } > - > - priv->soc_info->usb_phy_init(phy); > - > - /* Wait for PHY to reset */ > - usleep_range(30, 300); > - reg = readl(priv->base + REG_USBPCR_OFFSET); > - writel(reg & ~USBPCR_POR, priv->base + REG_USBPCR_OFFSET); > - usleep_range(300, 1000); > - > return 0; > } > > -static void ingenic_usb_phy_shutdown(struct usb_phy *phy) > +static int ingenic_usb_phy_power_off(struct phy *phy) > { > - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); > + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); > > - clk_disable_unprepare(priv->clk); > regulator_disable(priv->vcc_supply); > + > + return 0; > } > > -static void ingenic_usb_phy_remove(void *phy) > +static int ingenic_usb_phy_set_mode(struct phy *phy, > + enum phy_mode mode, int submode) > { > - usb_remove_phy(phy); > + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); > + u32 reg; > + > + switch (mode) { > + case PHY_MODE_USB_HOST: > + reg = readl(priv->base + REG_USBPCR_OFFSET); > + reg &= ~(USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL | > USBPCR_OTG_DISABLE); > + reg |= USBPCR_USB_MODE; > + writel(reg, priv->base + REG_USBPCR_OFFSET); > + > + break; > + case PHY_MODE_USB_DEVICE: > + if (priv->soc_info->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; > + writel(reg, priv->base + REG_USBPCR_OFFSET); > + > + break; > + case PHY_MODE_USB_OTG: > + reg = readl(priv->base + REG_USBPCR_OFFSET); > + reg &= ~USBPCR_OTG_DISABLE; > + reg |= USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL | USBPCR_USB_MODE; > + writel(reg, priv->base + REG_USBPCR_OFFSET); > + > + break; > + default: > + return -EINVAL; > + } > + > + return 0; > } > > -static void jz4770_usb_phy_init(struct usb_phy *phy) > +static const struct phy_ops ingenic_usb_phy_ops = { > + .init = ingenic_usb_phy_init, > + .exit = ingenic_usb_phy_exit, > + .power_on = ingenic_usb_phy_power_on, > + .power_off = ingenic_usb_phy_power_off, > + .set_mode = ingenic_usb_phy_set_mode, > + .owner = THIS_MODULE, > +}; > + > +static void jz4770_usb_phy_init(struct phy *phy) > { > - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); > + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); > u32 reg; > > reg = USBPCR_AVLD_REG | USBPCR_COMMONONN | USBPCR_IDPULLUP_ALWAYS | > @@ -208,9 +228,9 @@ static void jz4770_usb_phy_init(struct usb_phy > *phy) > writel(reg, priv->base + REG_USBPCR_OFFSET); > } > > -static void jz4780_usb_phy_init(struct usb_phy *phy) > +static void jz4780_usb_phy_init(struct phy *phy) > { > - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); > + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); > u32 reg; > > reg = readl(priv->base + REG_USBPCR1_OFFSET) | USBPCR1_USB_SEL | > @@ -221,9 +241,9 @@ static void jz4780_usb_phy_init(struct usb_phy > *phy) > writel(reg, priv->base + REG_USBPCR_OFFSET); > } > > -static void x1000_usb_phy_init(struct usb_phy *phy) > +static void x1000_usb_phy_init(struct phy *phy) > { > - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); > + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); > u32 reg; > > reg = readl(priv->base + REG_USBPCR1_OFFSET) | > USBPCR1_WORD_IF_16BIT; > @@ -235,9 +255,9 @@ static void x1000_usb_phy_init(struct usb_phy > *phy) > writel(reg, priv->base + REG_USBPCR_OFFSET); > } > > -static void x1830_usb_phy_init(struct usb_phy *phy) > +static void x1830_usb_phy_init(struct phy *phy) > { > - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); > + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); > u32 reg; > > /* rdt */ > @@ -276,43 +296,25 @@ static const struct ingenic_soc_info > x1830_soc_info = { > .usb_phy_init = x1830_usb_phy_init, > }; > > -static const struct of_device_id ingenic_usb_phy_of_matches[] = { > - { .compatible = "ingenic,jz4770-phy", .data = &jz4770_soc_info }, > - { .compatible = "ingenic,jz4780-phy", .data = &jz4780_soc_info }, > - { .compatible = "ingenic,x1000-phy", .data = &x1000_soc_info }, > - { .compatible = "ingenic,x1830-phy", .data = &x1830_soc_info }, > - { /* sentinel */ } > -}; > -MODULE_DEVICE_TABLE(of, ingenic_usb_phy_of_matches); Why move that array? > - > -static int jz4770_phy_probe(struct platform_device *pdev) > +static int ingenic_usb_phy_probe(struct platform_device *pdev) > { > + struct ingenic_usb_phy *priv; > + struct phy_provider *provider; > struct device *dev = &pdev->dev; > - struct jz4770_phy *priv; > int err; > > priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > if (!priv) > return -ENOMEM; > > - priv->soc_info = device_get_match_data(&pdev->dev); > + priv->soc_info = device_get_match_data(dev); > if (!priv->soc_info) { > - dev_err(&pdev->dev, "Error: No device match found\n"); > + dev_err(dev, "Error: No device match found\n"); > return -ENODEV; > } > > platform_set_drvdata(pdev, priv); > priv->dev = dev; > - priv->phy.dev = dev; > - priv->phy.otg = &priv->otg; > - priv->phy.label = "ingenic-usb-phy"; > - priv->phy.init = ingenic_usb_phy_init; > - priv->phy.shutdown = ingenic_usb_phy_shutdown; > - > - priv->otg.state = OTG_STATE_UNDEFINED; > - priv->otg.usb_phy = &priv->phy; > - priv->otg.set_host = ingenic_usb_phy_set_host; > - priv->otg.set_peripheral = ingenic_usb_phy_set_peripheral; > > priv->base = devm_platform_ioremap_resource(pdev, 0); > if (IS_ERR(priv->base)) { > @@ -336,27 +338,40 @@ static int jz4770_phy_probe(struct > platform_device *pdev) > return err; > } > > - err = usb_add_phy(&priv->phy, USB_PHY_TYPE_USB2); > - if (err) { > - if (err != -EPROBE_DEFER) > - dev_err(dev, "Unable to register PHY\n"); > - return err; > - } > + priv->phy = devm_phy_create(priv->dev, NULL, &ingenic_usb_phy_ops); You can use 'dev' directly. > + if (IS_ERR(priv)) > + return PTR_ERR(priv); > + > + phy_set_drvdata(priv->phy, priv); > > - return devm_add_action_or_reset(dev, ingenic_usb_phy_remove, > &priv->phy); > + provider = devm_of_phy_provider_register(priv->dev, > of_phy_simple_xlate); > + if (IS_ERR(provider)) > + return PTR_ERR(provider); > + > + return 0; return demv_of_phy_provider_register(dev, ...) > } > > -static struct platform_driver ingenic_phy_driver = { > - .probe = jz4770_phy_probe, > +static const struct of_device_id ingenic_usb_phy_of_matches[] = { > + { .compatible = "ingenic,jz4770-phy", .data = &jz4770_soc_info }, > + { .compatible = "ingenic,jz4780-phy", .data = &jz4780_soc_info }, > + { .compatible = "ingenic,x1000-phy", .data = &x1000_soc_info }, > + { .compatible = "ingenic,x1830-phy", .data = &x1830_soc_info }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, ingenic_usb_phy_of_matches); > + > +static struct platform_driver ingenic_usb_phy_driver = { > + .probe = ingenic_usb_phy_probe, > .driver = { > - .name = "jz4770-phy", > + .name = "ingenic-usb-phy", > .of_match_table = ingenic_usb_phy_of_matches, > }, > }; > -module_platform_driver(ingenic_phy_driver); > +module_platform_driver(ingenic_usb_phy_driver); > > MODULE_AUTHOR("周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>"); > MODULE_AUTHOR("漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>"); > MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>"); > MODULE_DESCRIPTION("Ingenic SoCs USB PHY driver"); > +MODULE_ALIAS("jz4770-phy"); So I first thought this generic PHY driver would be a drop-in replacement for the old jz4770-phy driver, that's why I suggested to add the jz4770-phy alias. But it turns out that it's a completely different framework, so we can drop the alias. That also means that this patch, without any change to the JZ4740 MUSB driver, breaks USB on all Ingenic SoCs I can test with. So unless you have a patch to the jz4740-musb driver in this series, you can't remove the old jz4770-phy driver yet. Just modify this patch so that the new generic-PHY driver is added, without removing the old one. It will be much easier to review (one new driver vs lots of +/- lines) and you can move the code around if you want to. Then when the jz4740-musb driver is modified to use the generic PHY framework, the old jz4770-phy driver can be "retired". Cheers, -Paul > MODULE_LICENSE("GPL"); > diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig > index ef4787cd3d37..ff24fca0a2d9 100644 > --- a/drivers/usb/phy/Kconfig > +++ b/drivers/usb/phy/Kconfig > @@ -184,12 +184,4 @@ config USB_ULPI_VIEWPORT > Provides read/write operations to the ULPI phy register set for > controllers with a viewport register (e.g. Chipidea/ARC > controllers). > > -config JZ4770_PHY > - 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 JZ-series and X-series SoCs from Ingenic. > - > endmenu > diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile > index b352bdbe8712..df1d99010079 100644 > --- a/drivers/usb/phy/Makefile > +++ b/drivers/usb/phy/Makefile > @@ -24,4 +24,3 @@ obj-$(CONFIG_USB_MXS_PHY) += phy-mxs-usb.o > obj-$(CONFIG_USB_ULPI) += phy-ulpi.o > obj-$(CONFIG_USB_ULPI_VIEWPORT) += phy-ulpi-viewport.o > obj-$(CONFIG_KEYSTONE_USB_PHY) += phy-keystone.o > -obj-$(CONFIG_JZ4770_PHY) += phy-jz4770.o > -- > 2.11.0 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] USB: PHY: JZ4770: Use the generic PHY framework. 2020-09-15 22:47 ` Paul Cercueil @ 2020-09-16 16:27 ` Zhou Yanjie 0 siblings, 0 replies; 8+ messages in thread From: Zhou Yanjie @ 2020-09-16 16:27 UTC (permalink / raw) To: Paul Cercueil Cc: balbi, gregkh, kishon, vkoul, linux-kernel, linux-usb, dongsheng.qiu, aric.pzqi, rick.tyliu, yanfei.li, sernia.zhou, zhenwenjin Hi Paul, 在 2020/9/16 上午6:47, Paul Cercueil 写道: > Hi Zhou, > > Le dim. 13 sept. 2020 à 14:34, 周琰杰 (Zhou Yanjie) > <zhouyanjie@wanyeetech.com> a écrit : >> Used the generic PHY framework API to create the PHY, >> and move the driver to driver/phy/ingenic. And adjust >> the position of some codes to make it consistent with >> the style of other USB PHY drivers. >> >> 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: >> Fix bug, ".of_match_table = >> of_match_ptr(ingenic_usb_phy_of_matches)" is wrong >> and should be replaced with ".of_match_table = >> ingenic_usb_phy_of_matches". >> >> v2->v3: >> 1.Change "depends on (MACH_INGENIC && MIPS) || COMPILE_TEST" to >> "depends on MIPS || COMPILE_TEST". >> 2.Keep the adjustments of "ingenic_usb_phy_init()" and >> "ingenic_usb_phu_exit()" >> positions in v2 to make them consistent with the order in >> "ingenic_usb_phy_ops", >> keep the adjustments to the positions of >> "ingenic_usb_phy_of_matches[]" in v2 >> to keep them consistent with the styles of other USB PHY >> drivers. And remove >> some unnecessary changes to reduce the diff size, from the >> original 256 lines >> change to the current 209 lines. >> >> drivers/phy/Kconfig | 1 + >> drivers/phy/Makefile | 1 + >> drivers/phy/ingenic/Kconfig | 12 ++ >> drivers/phy/ingenic/Makefile | 2 + >> .../phy-jz4770.c => phy/ingenic/phy-ingenic-usb.c} | 209 >> +++++++++++---------- >> drivers/usb/phy/Kconfig | 8 - >> drivers/usb/phy/Makefile | 1 - >> 7 files changed, 128 insertions(+), 106 deletions(-) >> create mode 100644 drivers/phy/ingenic/Kconfig >> create mode 100644 drivers/phy/ingenic/Makefile >> rename drivers/{usb/phy/phy-jz4770.c => >> phy/ingenic/phy-ingenic-usb.c} (71%) >> >> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >> index de9362c25c07..0534b0fdd057 100644 >> --- a/drivers/phy/Kconfig >> +++ b/drivers/phy/Kconfig >> @@ -55,6 +55,7 @@ source "drivers/phy/broadcom/Kconfig" >> source "drivers/phy/cadence/Kconfig" >> source "drivers/phy/freescale/Kconfig" >> source "drivers/phy/hisilicon/Kconfig" >> +source "drivers/phy/ingenic/Kconfig" >> source "drivers/phy/lantiq/Kconfig" >> source "drivers/phy/marvell/Kconfig" >> source "drivers/phy/mediatek/Kconfig" >> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile >> index c27408e4daae..ab24f0d20763 100644 >> --- a/drivers/phy/Makefile >> +++ b/drivers/phy/Makefile >> @@ -14,6 +14,7 @@ obj-y += allwinner/ \ >> cadence/ \ >> freescale/ \ >> hisilicon/ \ >> + ingenic/ \ >> intel/ \ >> lantiq/ \ >> marvell/ \ >> diff --git a/drivers/phy/ingenic/Kconfig b/drivers/phy/ingenic/Kconfig >> new file mode 100644 >> index 000000000000..912b14e512cb >> --- /dev/null >> +++ b/drivers/phy/ingenic/Kconfig >> @@ -0,0 +1,12 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +# >> +# Phy drivers for Ingenic platforms >> +# >> +config PHY_INGENIC_USB >> + tristate "Ingenic SoCs USB PHY Driver" >> + depends on MIPS || COMPILE_TEST >> + depends on USB_SUPPORT >> + select GENERIC_PHY >> + help >> + This driver provides USB PHY support for the USB controller found >> + on the JZ-series and X-series SoCs from Ingenic. >> diff --git a/drivers/phy/ingenic/Makefile b/drivers/phy/ingenic/Makefile >> new file mode 100644 >> index 000000000000..65d5ea00fc9d >> --- /dev/null >> +++ b/drivers/phy/ingenic/Makefile >> @@ -0,0 +1,2 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +obj-y += phy-ingenic-usb.o >> diff --git a/drivers/usb/phy/phy-jz4770.c >> b/drivers/phy/ingenic/phy-ingenic-usb.c >> similarity index 71% >> rename from drivers/usb/phy/phy-jz4770.c >> rename to drivers/phy/ingenic/phy-ingenic-usb.c >> index 4025da20b3fd..1f2e811c9125 100644 >> --- a/drivers/usb/phy/phy-jz4770.c >> +++ b/drivers/phy/ingenic/phy-ingenic-usb.c >> @@ -7,12 +7,12 @@ >> */ >> >> #include <linux/clk.h> >> +#include <linux/delay.h> >> #include <linux/io.h> >> #include <linux/module.h> >> #include <linux/platform_device.h> >> #include <linux/regulator/consumer.h> >> -#include <linux/usb/otg.h> >> -#include <linux/usb/phy.h> >> +#include <linux/phy/phy.h> >> >> /* OTGPHY register offsets */ >> #define REG_USBPCR_OFFSET 0x00 >> @@ -97,68 +97,56 @@ enum ingenic_usb_phy_version { >> struct ingenic_soc_info { >> enum ingenic_usb_phy_version version; >> >> - void (*usb_phy_init)(struct usb_phy *phy); >> + void (*usb_phy_init)(struct phy *phy); >> }; >> >> -struct jz4770_phy { >> +struct ingenic_usb_phy { >> const struct ingenic_soc_info *soc_info; >> >> - struct usb_phy phy; >> - struct usb_otg otg; >> + struct phy *phy; >> struct device *dev; >> void __iomem *base; >> struct clk *clk; >> struct regulator *vcc_supply; >> }; >> >> -static inline struct jz4770_phy *otg_to_jz4770_phy(struct usb_otg *otg) >> +static int ingenic_usb_phy_init(struct phy *phy) >> { >> - return container_of(otg, struct jz4770_phy, otg); >> -} >> - >> -static inline struct jz4770_phy *phy_to_jz4770_phy(struct usb_phy *phy) >> -{ >> - return container_of(phy, struct jz4770_phy, phy); >> -} >> - >> -static int ingenic_usb_phy_set_peripheral(struct usb_otg *otg, >> - struct usb_gadget *gadget) >> -{ >> - struct jz4770_phy *priv = otg_to_jz4770_phy(otg); >> + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); >> + int err; >> u32 reg; >> >> - if (priv->soc_info->version >= ID_X1000) { >> - reg = readl(priv->base + REG_USBPCR1_OFFSET); >> - reg |= USBPCR1_BVLD_REG; >> - writel(reg, priv->base + REG_USBPCR1_OFFSET); >> + err = clk_prepare_enable(priv->clk); >> + if (err) { >> + dev_err(priv->dev, "Unable to start clock: %d\n", err); >> + return err; >> } >> >> + priv->soc_info->usb_phy_init(phy); >> + >> + /* Wait for PHY to reset */ >> + usleep_range(30, 300); >> reg = readl(priv->base + REG_USBPCR_OFFSET); >> - reg &= ~USBPCR_USB_MODE; >> - reg |= USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL | >> USBPCR_OTG_DISABLE; >> - writel(reg, priv->base + REG_USBPCR_OFFSET); >> + writel(reg & ~USBPCR_POR, priv->base + REG_USBPCR_OFFSET); >> + usleep_range(300, 1000); >> >> return 0; >> } >> >> -static int ingenic_usb_phy_set_host(struct usb_otg *otg, struct >> usb_bus *host) >> +static int ingenic_usb_phy_exit(struct phy *phy) >> { >> - struct jz4770_phy *priv = otg_to_jz4770_phy(otg); >> - u32 reg; >> + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); >> >> - reg = readl(priv->base + REG_USBPCR_OFFSET); >> - reg &= ~(USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL | >> USBPCR_OTG_DISABLE); >> - reg |= USBPCR_USB_MODE; >> - writel(reg, priv->base + REG_USBPCR_OFFSET); >> + clk_disable_unprepare(priv->clk); >> + regulator_disable(priv->vcc_supply); >> >> return 0; >> } > > That part is still messy. You remove ingenic_usb_phy_set_peripheral() > and ingenic_usb_phy_set_host(), then add ingenic_usb_phy_init() and > ingenic_usb_phy_exit(). Why were the two first functions removed? Just > keep these around, and call them from ingenic_usb_phy_set_mode(). That > will automatically make the diff much smaller. > > And by making the diff smaller, we can spot the actual differences. > Like how support for PHY_MODE_USB_OTG was added, without any notice. > Indeed, after following your suggestion, the diff can be reduced a lot. I was too entangled in trying to be consistent with the order and style in orther phy drivers. :( >> >> -static int ingenic_usb_phy_init(struct usb_phy *phy) >> +static int ingenic_usb_phy_power_on(struct phy *phy) >> { >> - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); >> + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); >> int err; >> - u32 reg; >> >> err = regulator_enable(priv->vcc_supply); >> if (err) { >> @@ -166,39 +154,71 @@ static int ingenic_usb_phy_init(struct usb_phy >> *phy) >> return err; >> } >> >> - err = clk_prepare_enable(priv->clk); >> - if (err) { >> - dev_err(priv->dev, "Unable to start clock: %d\n", err); >> - return err; >> - } >> - >> - priv->soc_info->usb_phy_init(phy); >> - >> - /* Wait for PHY to reset */ >> - usleep_range(30, 300); >> - reg = readl(priv->base + REG_USBPCR_OFFSET); >> - writel(reg & ~USBPCR_POR, priv->base + REG_USBPCR_OFFSET); >> - usleep_range(300, 1000); >> - >> return 0; >> } >> >> -static void ingenic_usb_phy_shutdown(struct usb_phy *phy) >> +static int ingenic_usb_phy_power_off(struct phy *phy) >> { >> - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); >> + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); >> >> - clk_disable_unprepare(priv->clk); >> regulator_disable(priv->vcc_supply); >> + >> + return 0; >> } >> >> -static void ingenic_usb_phy_remove(void *phy) >> +static int ingenic_usb_phy_set_mode(struct phy *phy, >> + enum phy_mode mode, int submode) >> { >> - usb_remove_phy(phy); >> + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); >> + u32 reg; >> + >> + switch (mode) { >> + case PHY_MODE_USB_HOST: >> + reg = readl(priv->base + REG_USBPCR_OFFSET); >> + reg &= ~(USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL | >> USBPCR_OTG_DISABLE); >> + reg |= USBPCR_USB_MODE; >> + writel(reg, priv->base + REG_USBPCR_OFFSET); >> + >> + break; >> + case PHY_MODE_USB_DEVICE: >> + if (priv->soc_info->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; >> + writel(reg, priv->base + REG_USBPCR_OFFSET); >> + >> + break; >> + case PHY_MODE_USB_OTG: >> + reg = readl(priv->base + REG_USBPCR_OFFSET); >> + reg &= ~USBPCR_OTG_DISABLE; >> + reg |= USBPCR_VBUSVLDEXT | USBPCR_VBUSVLDEXTSEL | >> USBPCR_USB_MODE; >> + writel(reg, priv->base + REG_USBPCR_OFFSET); >> + >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + return 0; >> } >> >> -static void jz4770_usb_phy_init(struct usb_phy *phy) >> +static const struct phy_ops ingenic_usb_phy_ops = { >> + .init = ingenic_usb_phy_init, >> + .exit = ingenic_usb_phy_exit, >> + .power_on = ingenic_usb_phy_power_on, >> + .power_off = ingenic_usb_phy_power_off, >> + .set_mode = ingenic_usb_phy_set_mode, >> + .owner = THIS_MODULE, >> +}; >> + >> +static void jz4770_usb_phy_init(struct phy *phy) >> { >> - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); >> + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); >> u32 reg; >> >> reg = USBPCR_AVLD_REG | USBPCR_COMMONONN | USBPCR_IDPULLUP_ALWAYS | >> @@ -208,9 +228,9 @@ static void jz4770_usb_phy_init(struct usb_phy *phy) >> writel(reg, priv->base + REG_USBPCR_OFFSET); >> } >> >> -static void jz4780_usb_phy_init(struct usb_phy *phy) >> +static void jz4780_usb_phy_init(struct phy *phy) >> { >> - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); >> + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); >> u32 reg; >> >> reg = readl(priv->base + REG_USBPCR1_OFFSET) | USBPCR1_USB_SEL | >> @@ -221,9 +241,9 @@ static void jz4780_usb_phy_init(struct usb_phy *phy) >> writel(reg, priv->base + REG_USBPCR_OFFSET); >> } >> >> -static void x1000_usb_phy_init(struct usb_phy *phy) >> +static void x1000_usb_phy_init(struct phy *phy) >> { >> - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); >> + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); >> u32 reg; >> >> reg = readl(priv->base + REG_USBPCR1_OFFSET) | >> USBPCR1_WORD_IF_16BIT; >> @@ -235,9 +255,9 @@ static void x1000_usb_phy_init(struct usb_phy *phy) >> writel(reg, priv->base + REG_USBPCR_OFFSET); >> } >> >> -static void x1830_usb_phy_init(struct usb_phy *phy) >> +static void x1830_usb_phy_init(struct phy *phy) >> { >> - struct jz4770_phy *priv = phy_to_jz4770_phy(phy); >> + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); >> u32 reg; >> >> /* rdt */ >> @@ -276,43 +296,25 @@ static const struct ingenic_soc_info >> x1830_soc_info = { >> .usb_phy_init = x1830_usb_phy_init, >> }; >> >> -static const struct of_device_id ingenic_usb_phy_of_matches[] = { >> - { .compatible = "ingenic,jz4770-phy", .data = &jz4770_soc_info }, >> - { .compatible = "ingenic,jz4780-phy", .data = &jz4780_soc_info }, >> - { .compatible = "ingenic,x1000-phy", .data = &x1000_soc_info }, >> - { .compatible = "ingenic,x1830-phy", .data = &x1830_soc_info }, >> - { /* sentinel */ } >> -}; >> -MODULE_DEVICE_TABLE(of, ingenic_usb_phy_of_matches); > > Why move that array? > This is the same as before, too entangled in keeping consistent with the order in other phy drivers :( >> - >> -static int jz4770_phy_probe(struct platform_device *pdev) >> +static int ingenic_usb_phy_probe(struct platform_device *pdev) >> { >> + struct ingenic_usb_phy *priv; >> + struct phy_provider *provider; >> struct device *dev = &pdev->dev; >> - struct jz4770_phy *priv; >> int err; >> >> priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >> if (!priv) >> return -ENOMEM; >> >> - priv->soc_info = device_get_match_data(&pdev->dev); >> + priv->soc_info = device_get_match_data(dev); >> if (!priv->soc_info) { >> - dev_err(&pdev->dev, "Error: No device match found\n"); >> + dev_err(dev, "Error: No device match found\n"); >> return -ENODEV; >> } >> >> platform_set_drvdata(pdev, priv); >> priv->dev = dev; >> - priv->phy.dev = dev; >> - priv->phy.otg = &priv->otg; >> - priv->phy.label = "ingenic-usb-phy"; >> - priv->phy.init = ingenic_usb_phy_init; >> - priv->phy.shutdown = ingenic_usb_phy_shutdown; >> - >> - priv->otg.state = OTG_STATE_UNDEFINED; >> - priv->otg.usb_phy = &priv->phy; >> - priv->otg.set_host = ingenic_usb_phy_set_host; >> - priv->otg.set_peripheral = ingenic_usb_phy_set_peripheral; >> >> priv->base = devm_platform_ioremap_resource(pdev, 0); >> if (IS_ERR(priv->base)) { >> @@ -336,27 +338,40 @@ static int jz4770_phy_probe(struct >> platform_device *pdev) >> return err; >> } >> >> - err = usb_add_phy(&priv->phy, USB_PHY_TYPE_USB2); >> - if (err) { >> - if (err != -EPROBE_DEFER) >> - dev_err(dev, "Unable to register PHY\n"); >> - return err; >> - } >> + priv->phy = devm_phy_create(priv->dev, NULL, &ingenic_usb_phy_ops); > > You can use 'dev' directly. > Sure. >> + if (IS_ERR(priv)) >> + return PTR_ERR(priv); >> + >> + phy_set_drvdata(priv->phy, priv); >> >> - return devm_add_action_or_reset(dev, ingenic_usb_phy_remove, >> &priv->phy); >> + provider = devm_of_phy_provider_register(priv->dev, >> of_phy_simple_xlate); >> + if (IS_ERR(provider)) >> + return PTR_ERR(provider); >> + >> + return 0; > > return demv_of_phy_provider_register(dev, ...) > Sure. >> } >> >> -static struct platform_driver ingenic_phy_driver = { >> - .probe = jz4770_phy_probe, >> +static const struct of_device_id ingenic_usb_phy_of_matches[] = { >> + { .compatible = "ingenic,jz4770-phy", .data = &jz4770_soc_info }, >> + { .compatible = "ingenic,jz4780-phy", .data = &jz4780_soc_info }, >> + { .compatible = "ingenic,x1000-phy", .data = &x1000_soc_info }, >> + { .compatible = "ingenic,x1830-phy", .data = &x1830_soc_info }, >> + { /* sentinel */ } >> +}; >> +MODULE_DEVICE_TABLE(of, ingenic_usb_phy_of_matches); >> + >> +static struct platform_driver ingenic_usb_phy_driver = { >> + .probe = ingenic_usb_phy_probe, >> .driver = { >> - .name = "jz4770-phy", >> + .name = "ingenic-usb-phy", >> .of_match_table = ingenic_usb_phy_of_matches, >> }, >> }; >> -module_platform_driver(ingenic_phy_driver); >> +module_platform_driver(ingenic_usb_phy_driver); >> >> MODULE_AUTHOR("周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>"); >> MODULE_AUTHOR("漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>"); >> MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>"); >> MODULE_DESCRIPTION("Ingenic SoCs USB PHY driver"); >> +MODULE_ALIAS("jz4770-phy"); > > So I first thought this generic PHY driver would be a drop-in > replacement for the old jz4770-phy driver, that's why I suggested to > add the jz4770-phy alias. But it turns out that it's a completely > different framework, so we can drop the alias. > > That also means that this patch, without any change to the JZ4740 MUSB > driver, breaks USB on all Ingenic SoCs I can test with. > > So unless you have a patch to the jz4740-musb driver in this series, > you can't remove the old jz4770-phy driver yet. Just modify this patch > so that the new generic-PHY driver is added, without removing the old > one. It will be much easier to review (one new driver vs lots of +/- > lines) and you can move the code around if you want to. Then when the > jz4740-musb driver is modified to use the generic PHY framework, the > old jz4770-phy driver can be "retired". > Sure, this also just solves the problem of order and style that I entangled before. Thanks and best regards! > Cheers, > -Paul > >> MODULE_LICENSE("GPL"); >> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig >> index ef4787cd3d37..ff24fca0a2d9 100644 >> --- a/drivers/usb/phy/Kconfig >> +++ b/drivers/usb/phy/Kconfig >> @@ -184,12 +184,4 @@ config USB_ULPI_VIEWPORT >> Provides read/write operations to the ULPI phy register set for >> controllers with a viewport register (e.g. Chipidea/ARC >> controllers). >> >> -config JZ4770_PHY >> - 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 JZ-series and X-series SoCs from Ingenic. >> - >> endmenu >> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile >> index b352bdbe8712..df1d99010079 100644 >> --- a/drivers/usb/phy/Makefile >> +++ b/drivers/usb/phy/Makefile >> @@ -24,4 +24,3 @@ obj-$(CONFIG_USB_MXS_PHY) += phy-mxs-usb.o >> obj-$(CONFIG_USB_ULPI) += phy-ulpi.o >> obj-$(CONFIG_USB_ULPI_VIEWPORT) += phy-ulpi-viewport.o >> obj-$(CONFIG_KEYSTONE_USB_PHY) += phy-keystone.o >> -obj-$(CONFIG_JZ4770_PHY) += phy-jz4770.o >> -- >> 2.11.0 >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] USB: PHY: JZ4770: Use the generic PHY framework. 2020-09-13 6:34 ` [PATCH v3 2/2] USB: PHY: JZ4770: Use the generic PHY framework 周琰杰 (Zhou Yanjie) 2020-09-15 22:47 ` Paul Cercueil @ 2020-09-16 6:55 ` kernel test robot 2020-09-16 6:55 ` kernel test robot 2 siblings, 0 replies; 8+ messages in thread From: kernel test robot @ 2020-09-16 6:55 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 1168 bytes --] Hi "周琰杰, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on balbi-usb/testing/next] [also build test WARNING on usb/usb-testing linus/master v5.9-rc5 next-20200915] [cannot apply to phy/next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Zhou-Yanjie/Use-the-generic-PHY-framework-for-Ingenic-USB-PHY/20200913-143611 base: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next config: sh-randconfig-c003-20200916 (attached as .config) compiler: sh4-linux-gcc (GCC) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> coccinelle warnings: (new ones prefixed by >>) >> drivers/phy/ingenic/phy-ingenic-usb.c:348:1-3: WARNING: PTR_ERR_OR_ZERO can be used Please review and possibly fold the followup patch. --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 23437 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] USB: PHY: JZ4770: fix ptr_ret.cocci warnings 2020-09-13 6:34 ` [PATCH v3 2/2] USB: PHY: JZ4770: Use the generic PHY framework 周琰杰 (Zhou Yanjie) @ 2020-09-16 6:55 ` kernel test robot 2020-09-16 6:55 ` kernel test robot 2020-09-16 6:55 ` kernel test robot 2 siblings, 0 replies; 8+ messages in thread From: kernel test robot @ 2020-09-16 6:55 UTC (permalink / raw) To: 周琰杰 (Zhou Yanjie), balbi, gregkh, kishon, vkoul, paul Cc: kbuild-all, linux-kernel, linux-usb, dongsheng.qiu, aric.pzqi, rick.tyliu From: kernel test robot <lkp@intel.com> drivers/phy/ingenic/phy-ingenic-usb.c:348:1-3: WARNING: PTR_ERR_OR_ZERO can be used Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR Generated by: scripts/coccinelle/api/ptr_ret.cocci CC: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> Signed-off-by: kernel test robot <lkp@intel.com> --- url: https://github.com/0day-ci/linux/commits/Zhou-Yanjie/Use-the-generic-PHY-framework-for-Ingenic-USB-PHY/20200913-143611 base: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next phy-ingenic-usb.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) --- a/drivers/phy/ingenic/phy-ingenic-usb.c +++ b/drivers/phy/ingenic/phy-ingenic-usb.c @@ -345,10 +345,7 @@ static int ingenic_usb_phy_probe(struct phy_set_drvdata(priv->phy, priv); provider = devm_of_phy_provider_register(priv->dev, of_phy_simple_xlate); - if (IS_ERR(provider)) - return PTR_ERR(provider); - - return 0; + return PTR_ERR_OR_ZERO(provider); } static const struct of_device_id ingenic_usb_phy_of_matches[] = { ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] USB: PHY: JZ4770: fix ptr_ret.cocci warnings @ 2020-09-16 6:55 ` kernel test robot 0 siblings, 0 replies; 8+ messages in thread From: kernel test robot @ 2020-09-16 6:55 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 1110 bytes --] From: kernel test robot <lkp@intel.com> drivers/phy/ingenic/phy-ingenic-usb.c:348:1-3: WARNING: PTR_ERR_OR_ZERO can be used Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR Generated by: scripts/coccinelle/api/ptr_ret.cocci CC: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> Signed-off-by: kernel test robot <lkp@intel.com> --- url: https://github.com/0day-ci/linux/commits/Zhou-Yanjie/Use-the-generic-PHY-framework-for-Ingenic-USB-PHY/20200913-143611 base: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next phy-ingenic-usb.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) --- a/drivers/phy/ingenic/phy-ingenic-usb.c +++ b/drivers/phy/ingenic/phy-ingenic-usb.c @@ -345,10 +345,7 @@ static int ingenic_usb_phy_probe(struct phy_set_drvdata(priv->phy, priv); provider = devm_of_phy_provider_register(priv->dev, of_phy_simple_xlate); - if (IS_ERR(provider)) - return PTR_ERR(provider); - - return 0; + return PTR_ERR_OR_ZERO(provider); } static const struct of_device_id ingenic_usb_phy_of_matches[] = { ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-09-16 20:46 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-13 6:34 [PATCH v3 0/2] Use the generic PHY framework for Ingenic USB PHY 周琰杰 (Zhou Yanjie) 2020-09-13 6:34 ` [PATCH v3 1/2] USB: PHY: JZ4770: Remove unnecessary function calls 周琰杰 (Zhou Yanjie) 2020-09-13 6:34 ` [PATCH v3 2/2] USB: PHY: JZ4770: Use the generic PHY framework 周琰杰 (Zhou Yanjie) 2020-09-15 22:47 ` Paul Cercueil 2020-09-16 16:27 ` Zhou Yanjie 2020-09-16 6:55 ` kernel test robot 2020-09-16 6:55 ` [PATCH] USB: PHY: JZ4770: fix ptr_ret.cocci warnings kernel test robot 2020-09-16 6:55 ` kernel test robot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.