Messages in this thread | | | Date | Sun, 06 Sep 2020 19:15:21 +0200 | From | Paul Cercueil <> | Subject | Re: [PATCH v2 1/1] USB: PHY: JZ4770: Use the generic PHY framework. |
| |
Le lun. 7 sept. 2020 à 1:06, Zhou Yanjie <zhouyanjie@wanyeetech.com> a écrit : > Hi Paul, > > 在 2020/9/4 下午10:10, Paul Cercueil 写道: >> Hi Zhou, >> >> Le lun. 31 août 2020 à 21:50, 周琰杰 (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. >>> >>> 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". >>> >>> 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} | 256 >>> ++++++++++++--------- >>> drivers/usb/phy/Kconfig | 8 - >>> drivers/usb/phy/Makefile | 1 - >>> 7 files changed, 165 insertions(+), 116 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} (63%) >>> >>> 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..b9581eae89dd >>> --- /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 (MACH_INGENIC && MIPS) || COMPILE_TEST >> >> The original driver depends on MIPS || COMPILE_TEST, so you should >> do the same, otherwise you change more than what the patch >> description suggests. >> > > Sure, I will change it in the next version. > >>> + 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 63% >>> rename from drivers/usb/phy/phy-jz4770.c >>> rename to drivers/phy/ingenic/phy-ingenic-usb.c >>> index f6d3731581eb..86a95b498785 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,49 @@ 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); >>> - u32 reg; >>> + struct ingenic_usb_phy *priv = phy_get_drvdata(phy); >>> + int err; >>> >>> - 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; >>> } >>> >>> - 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); >>> + priv->soc_info->usb_phy_init(phy); >>> >>> 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 +147,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; >>> } >> >> I think the diff should be a bit smaller (and easier to review) if >> you move ingenic_usb_phy_init / ingenic_usb_phy_exit here, where >> they used to be. >> > > Sure. >>> >>> -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 | >>> @@ -206,11 +219,16 @@ static void jz4770_usb_phy_init(struct >>> usb_phy *phy) >>> USBPCR_TXFSLSTUNE_DFT | USBPCR_TXRISETUNE_DFT | >>> USBPCR_TXVREFTUNE_DFT | >>> USBPCR_POR; >>> writel(reg, priv->base + REG_USBPCR_OFFSET); >>> + >>> + /* Wait for PHY to reset */ >>> + usleep_range(30, 300); >>> + writel(reg & ~USBPCR_POR, priv->base + REG_USBPCR_OFFSET); >>> + usleep_range(300, 1000); >>> } >>> >>> -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 >>> | >>> @@ -219,11 +237,16 @@ static void jz4780_usb_phy_init(struct >>> usb_phy *phy) >>> >>> reg = USBPCR_TXPREEMPHTUNE | USBPCR_COMMONONN | USBPCR_POR; >>> writel(reg, priv->base + REG_USBPCR_OFFSET); >>> + >>> + /* Wait for PHY to reset */ >>> + usleep_range(30, 300); >>> + writel(reg & ~USBPCR_POR, priv->base + REG_USBPCR_OFFSET); >>> + usleep_range(300, 1000); >>> } >>> >>> -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; >>> @@ -233,11 +256,16 @@ static void x1000_usb_phy_init(struct usb_phy >>> *phy) >>> USBPCR_TXHSXVTUNE_DCR_15MV | USBPCR_TXVREFTUNE_INC_25PPT | >>> USBPCR_COMMONONN | USBPCR_POR; >>> writel(reg, priv->base + REG_USBPCR_OFFSET); >>> + >>> + /* Wait for PHY to reset */ >>> + usleep_range(30, 300); >>> + writel(reg & ~USBPCR_POR, priv->base + REG_USBPCR_OFFSET); >>> + usleep_range(300, 1000); >>> } >>> >>> -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 */ >>> @@ -250,6 +278,11 @@ static void x1830_usb_phy_init(struct usb_phy >>> *phy) >>> reg = USBPCR_IDPULLUP_OTG | USBPCR_VBUSVLDEXT | >>> USBPCR_TXPREEMPHTUNE | >>> USBPCR_COMMONONN | USBPCR_POR; >>> writel(reg, priv->base + REG_USBPCR_OFFSET); >>> + >>> + /* Wait for PHY to reset */ >>> + usleep_range(30, 300); >>> + writel(reg & ~USBPCR_POR, priv->base + REG_USBPCR_OFFSET); >>> + usleep_range(300, 1000); >> >> Why is that code repeated four times now? The old driver had that in >> ingenic_usb_phy_init(). >> > > This is my fault, I forgot to make the corresponding changes after I > cherry-pick it from v6, I will fix this problem in the next version. > >>> } >>> >>> static const struct ingenic_soc_info jz4770_soc_info = { >>> @@ -276,87 +309,96 @@ 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 device *dev = &pdev->dev; >>> - struct jz4770_phy *priv; >>> + struct ingenic_usb_phy *priv; >>> + struct phy_provider *provider; >>> int err; >>> >>> - priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >>> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); >> >> I'd prefer that you keep a local 'dev' variable. Otherwise it only >> makes the diff bigger and it's harder to review. >> > > Sure. > >>> if (!priv) >>> return -ENOMEM; >>> >>> + priv->dev = &pdev->dev; >>> + >>> priv->soc_info = device_get_match_data(&pdev->dev); >>> if (!priv->soc_info) { >>> dev_err(&pdev->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)) { >>> - dev_err(dev, "Failed to map registers\n"); >>> + dev_err(priv->dev, "Failed to map registers\n"); >>> return PTR_ERR(priv->base); >>> } >>> >>> - priv->clk = devm_clk_get(dev, NULL); >>> + priv->clk = devm_clk_get(priv->dev, NULL); >>> if (IS_ERR(priv->clk)) { >>> err = PTR_ERR(priv->clk); >>> if (err != -EPROBE_DEFER) >>> - dev_err(dev, "Failed to get clock\n"); >>> + dev_err(priv->dev, "Failed to get clock\n"); >>> return err; >>> } >>> >>> - priv->vcc_supply = devm_regulator_get(dev, "vcc"); >>> + priv->vcc_supply = devm_regulator_get(priv->dev, "vcc"); >>> if (IS_ERR(priv->vcc_supply)) { >>> err = PTR_ERR(priv->vcc_supply); >>> if (err != -EPROBE_DEFER) >>> - dev_err(dev, "Failed to get regulator\n"); >>> + dev_err(priv->dev, "Failed to get regulator\n"); >>> 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)) { >>> + dev_err(priv->dev, "Failed to create PHY: %ld\n", >>> PTR_ERR(priv)); >>> + return PTR_ERR(priv); >>> + } >> >> There's a stray tabulation character here. >> >> Also, no need to print error codes in the probe function - they will >> be printed anyway since the driver will fail to probe. >> > Sure. >>> + >>> + provider = devm_of_phy_provider_register(priv->dev, >>> of_phy_simple_xlate); >>> + if (IS_ERR(provider)) { >>> + dev_err(priv->dev, "Failed to register PHY provider: >>> %ld\n", PTR_ERR(provider)); >>> + return PTR_ERR(provider); >>> } >> >> Same here. >> >>> >>> - return devm_add_action_or_reset(dev, ingenic_usb_phy_remove, >>> &priv->phy); >>> + platform_set_drvdata(pdev, priv); >>> + phy_set_drvdata(priv->phy, priv); >> >> These two do the same thing. Also, you must do it before registering >> the PHY, otherwise you have a race. >> > OK, I will fix it in the next version. >>> + >>> + return 0; >>> +} >>> + >>> +static int ingenic_usb_phy_remove(struct platform_device *pdev) >>> +{ >>> + struct ingenic_usb_phy *priv = platform_get_drvdata(pdev); >>> + >>> + clk_disable_unprepare(priv->clk); >>> + regulator_disable(priv->vcc_supply); >> >> I assume that ingenic_usb_phy_power_off() and ingenic_usb_phy_exit() >> are automatically called when the module is removed, did you test >> module removal? >> > > I think I have an oversignt, only the module install was tested, but > the module removal was not tested. > >>> + >>> + 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); >> >> You moved that code around, which only made the diff bigger and >> harder to review. Please keep it where it was. >> > > Sure. > >>> + >>> +static struct platform_driver ingenic_usb_phy_driver = { >>> + .probe = ingenic_usb_phy_probe, >>> + .remove = ingenic_usb_phy_remove, >>> .driver = { >>> - .name = "jz4770-phy", >>> - .of_match_table = of_match_ptr(ingenic_usb_phy_of_matches), >>> + .name = "ingenic-usb-phy", >>> + .of_match_table = ingenic_usb_phy_of_matches, >> >> You removed of_match_ptr(), which is a valid change (Ingenic SoCs >> all depend on Device Tree), but is unrelated to this patch. >> > > It was not removed in the previous version, so the test robot sent me > an email. In this case, should I remove it directly herer or remove > it in a separate patch?
Separate patch please, before the move to the generic PHY.
Cheers, -Paul
>>> }, >>> }; >>> -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"); >> >> Actually that would be "jz4770-phy". >> > > Sure, I'll change it in the next version. > > 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 >>> >>
| |