Re: [PATCH 2/8] usb: cdns3: Split core.c into cdns3-plat and core.c file

From: Chunfeng Yun
Date: Mon Sep 28 2020 - 21:48:56 EST

Next message: David Miller: "Re: [PATCH][next] net/sched: cls_u32: Replace one-element array with flexible-array member"
Previous message: David Miller: "Re: [PATCH][next] qed/qed_ll2: Replace one-element array with flexible-array member"
In reply to: Pawel Laszczak: "[PATCH 2/8] usb: cdns3: Split core.c into cdns3-plat and core.c file"
Next in thread: Pawel Laszczak: "RE: [PATCH 2/8] usb: cdns3: Split core.c into cdns3-plat and core.c file"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon, 2020-09-28 at 14:27 +0200, Pawel Laszczak wrote:
> Patch splits file core.c into core.c containing the common reusable code
> and cnd3-plat.c containing device platform specific code. These changes
> are required to make possible reuse DRD part of CDNS3 driver in CDNSP
> driver.
>
> Signed-off-by: Pawel Laszczak <pawell@xxxxxxxxxxx>
> ---
> drivers/usb/cdns3/Makefile | 2 +-
> drivers/usb/cdns3/cdns3-plat.c | 209 +++++++++++++++++++++++++++++++++
> drivers/usb/cdns3/core.c | 181 +++-------------------------
> drivers/usb/cdns3/core.h | 8 +-
> 4 files changed, 234 insertions(+), 166 deletions(-)
> create mode 100644 drivers/usb/cdns3/cdns3-plat.c
>
> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
> index d47e341a6f39..a1fe9612053a 100644
> --- a/drivers/usb/cdns3/Makefile
> +++ b/drivers/usb/cdns3/Makefile
> @@ -2,7 +2,7 @@
> # define_trace.h needs to know how to find our header
> CFLAGS_trace.o := -I$(src)
>
> -cdns3-y := core.o drd.o
> +cdns3-y := cdns3-plat.o core.o drd.o
>
> obj-$(CONFIG_USB_CDNS3) += cdns3.o
> cdns3-$(CONFIG_USB_CDNS3_GADGET) += gadget.o ep0.o
> diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
> new file mode 100644
> index 000000000000..f35e9dca30fe
> --- /dev/null
> +++ b/drivers/usb/cdns3/cdns3-plat.c
> @@ -0,0 +1,209 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Cadence USBSS DRD Driver.
> + *
> + * Copyright (C) 2018-2020 Cadence.
> + * Copyright (C) 2017-2018 NXP
> + * Copyright (C) 2019 Texas Instrumentsq
> + *
> + *
> + * Author: Peter Chen <peter.chen@xxxxxxx>
> + * Pawel Laszczak <pawell@xxxxxxxxxxx>
> + * Roger Quadros <rogerq@xxxxxx>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "core.h"
> +
> +/**
> + * cdns3_plat_probe - probe for cdns3 core device
> + * @pdev: Pointer to cdns3 core platform device
> + *
> + * Returns 0 on success otherwise negative errno
> + */
> +static int cdns3_plat_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + struct cdns3 *cdns;
> + void __iomem *regs;
> + int ret;
> +
> + cdns = devm_kzalloc(dev, sizeof(*cdns), GFP_KERNEL);
> + if (!cdns)
> + return -ENOMEM;
> +
> + cdns->dev = dev;
> +
> + platform_set_drvdata(pdev, cdns);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "host");
> + if (!res) {
> + dev_err(dev, "missing host IRQ\n");
> + return -ENODEV;
> + }
> +
> + cdns->xhci_res[0] = *res;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "xhci");
> + if (!res) {
> + dev_err(dev, "couldn't get xhci resource\n");
> + return -ENXIO;
> + }
> +
> + cdns->xhci_res[1] = *res;
> +
> + cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");
> + if (cdns->dev_irq == -EPROBE_DEFER)
> + return cdns->dev_irq;
> +
> + if (cdns->dev_irq < 0)
> + dev_err(dev, "couldn't get peripheral irq\n");
Use platform_get_irq_byname_optional? otherwise no need print this log,
platform_get_irq_byname() will print it.

> +
> + regs = devm_platform_ioremap_resource_byname(pdev, "dev");
> + if (IS_ERR(regs))
> + return PTR_ERR(regs);
> + cdns->dev_regs = regs;
> +
> + cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
> + if (cdns->otg_irq == -EPROBE_DEFER)
> + return cdns->otg_irq;
> +
> + if (cdns->otg_irq < 0) {
> + dev_err(dev, "couldn't get otg irq\n");
> + return cdns->otg_irq;
> + }
ditto

> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg");
> + if (!res) {
> + dev_err(dev, "couldn't get otg resource\n");
> + return -ENXIO;
> + }
> +
> + cdns->otg_res = *res;
> +
> + cdns->usb2_phy = devm_phy_optional_get(dev, "cdns3,usb2-phy");
> + if (IS_ERR(cdns->usb2_phy))
> + return PTR_ERR(cdns->usb2_phy);
> +
> + ret = phy_init(cdns->usb2_phy);
> + if (ret)
> + return ret;
> +
> + cdns->usb3_phy = devm_phy_optional_get(dev, "cdns3,usb3-phy");
> + if (IS_ERR(cdns->usb3_phy))
> + return PTR_ERR(cdns->usb3_phy);
> +
> + ret = phy_init(cdns->usb3_phy);
> + if (ret)
> + goto err_phy3_init;
> +
> + ret = phy_power_on(cdns->usb2_phy);
> + if (ret)
> + goto err_phy2_power_on;
> +
> + ret = phy_power_on(cdns->usb3_phy);
> + if (ret)
> + goto err_phy3_power_on;
> +
> + ret = cdns3_init(cdns);
> + if (ret)
> + goto err_cdns_init;
> +
> + device_set_wakeup_capable(dev, true);
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> +
> + /*
> + * The controller needs less time between bus and controller suspend,
> + * and we also needs a small delay to avoid frequently entering low
> + * power mode.
> + */
> + pm_runtime_set_autosuspend_delay(dev, 20);
> + pm_runtime_mark_last_busy(dev);
> +
> + return 0;
> +
> +err_cdns_init:
> + phy_power_off(cdns->usb3_phy);
> +err_phy3_power_on:
> + phy_power_off(cdns->usb2_phy);
> +err_phy2_power_on:
> + phy_exit(cdns->usb3_phy);
> +err_phy3_init:
> + phy_exit(cdns->usb2_phy);
> +
> + return ret;
> +}
> +
> +/**
> + * cdns3_remove - unbind drd driver and clean up
> + * @pdev: Pointer to Linux platform device
> + *
> + * Returns 0 on success otherwise negative errno
> + */
> +static int cdns3_plat_remove(struct platform_device *pdev)
> +{
> + struct cdns3 *cdns = platform_get_drvdata(pdev);
> + struct device *dev = cdns->dev;
> +
> + pm_runtime_get_sync(dev);
> + pm_runtime_disable(dev);
> + pm_runtime_put_noidle(dev);
> + cdns3_remove(cdns);
> + phy_power_off(cdns->usb2_phy);
> + phy_power_off(cdns->usb3_phy);
> + phy_exit(cdns->usb2_phy);
> + phy_exit(cdns->usb3_phy);
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +
> +static int cdns3_plat_suspend(struct device *dev)
> +{
> + struct cdns3 *cdns = dev_get_drvdata(dev);
> +
> + return cdns3_suspend(cdns);
> +}
> +
> +static int cdns3_plat_resume(struct device *dev)
> +{
> + struct cdns3 *cdns = dev_get_drvdata(dev);
> +
> + return cdns3_resume(cdns);
> +}
> +#endif
> +
> +static const struct dev_pm_ops cdns3_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(cdns3_plat_suspend, cdns3_plat_resume)
> +};
> +
> +#ifdef CONFIG_OF
No need it if only supports devicetree

> +static const struct of_device_id of_cdns3_match[] = {
> + { .compatible = "cdns,usb3" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, of_cdns3_match);
> +#endif
> +
> +static struct platform_driver cdns3_driver = {
> + .probe = cdns3_plat_probe,
> + .remove = cdns3_plat_remove,
> + .driver = {
> + .name = "cdns-usb3",
> + .of_match_table = of_match_ptr(of_cdns3_match),
> + .pm = &cdns3_pm_ops,
> + },
> +};
> +
> +module_platform_driver(cdns3_driver);
> +
> +MODULE_ALIAS("platform:cdns3");
> +MODULE_AUTHOR("Pawel Laszczak <pawell@xxxxxxxxxxx>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Cadence USB3 DRD Controller Driver");
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index 371128e9a4ae..079bd2abf65d 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -2,7 +2,7 @@
> /*
> * Cadence USBSS DRD Driver.
> *
> - * Copyright (C) 2018-2019 Cadence.
> + * Copyright (C) 2018-2020 Cadence.
> * Copyright (C) 2017-2018 NXP
> * Copyright (C) 2019 Texas Instruments
> *
> @@ -383,17 +383,14 @@ static int cdns3_role_set(struct usb_role_switch *sw, enum usb_role role)
>
> /**
> * cdns3_probe - probe for cdns3 core device
> - * @pdev: Pointer to cdns3 core platform device
> + * @cdns: Pointer to cdnsp structure.
> *
> * Returns 0 on success otherwise negative errno
> */
> -static int cdns3_probe(struct platform_device *pdev)
> +int cdns3_init(struct cdns3 *cdns)
> {
> struct usb_role_switch_desc sw_desc = { };
> - struct device *dev = &pdev->dev;
> - struct resource *res;
> - struct cdns3 *cdns;
> - void __iomem *regs;
> + struct device *dev = cdns->dev;
> int ret;
>
> ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> @@ -402,85 +399,8 @@ static int cdns3_probe(struct platform_device *pdev)
> return ret;
> }
>
> - cdns = devm_kzalloc(dev, sizeof(*cdns), GFP_KERNEL);
> - if (!cdns)
> - return -ENOMEM;
> -
> - cdns->dev = dev;
> -
> - platform_set_drvdata(pdev, cdns);
> -
> - res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "host");
> - if (!res) {
> - dev_err(dev, "missing host IRQ\n");
> - return -ENODEV;
> - }
> -
> - cdns->xhci_res[0] = *res;
> -
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "xhci");
> - if (!res) {
> - dev_err(dev, "couldn't get xhci resource\n");
> - return -ENXIO;
> - }
> -
> - cdns->xhci_res[1] = *res;
> -
> - cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");
> - if (cdns->dev_irq == -EPROBE_DEFER)
> - return cdns->dev_irq;
> -
> - if (cdns->dev_irq < 0)
> - dev_err(dev, "couldn't get peripheral irq\n");
> -
> - regs = devm_platform_ioremap_resource_byname(pdev, "dev");
> - if (IS_ERR(regs))
> - return PTR_ERR(regs);
> - cdns->dev_regs = regs;
> -
> - cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
> - if (cdns->otg_irq == -EPROBE_DEFER)
> - return cdns->otg_irq;
> -
> - if (cdns->otg_irq < 0) {
> - dev_err(dev, "couldn't get otg irq\n");
> - return cdns->otg_irq;
> - }
> -
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg");
> - if (!res) {
> - dev_err(dev, "couldn't get otg resource\n");
> - return -ENXIO;
> - }
> -
> - cdns->otg_res = *res;
> -
> mutex_init(&cdns->mutex);
>
> - cdns->usb2_phy = devm_phy_optional_get(dev, "cdns3,usb2-phy");
> - if (IS_ERR(cdns->usb2_phy))
> - return PTR_ERR(cdns->usb2_phy);
> -
> - ret = phy_init(cdns->usb2_phy);
> - if (ret)
> - return ret;
> -
> - cdns->usb3_phy = devm_phy_optional_get(dev, "cdns3,usb3-phy");
> - if (IS_ERR(cdns->usb3_phy))
> - return PTR_ERR(cdns->usb3_phy);
> -
> - ret = phy_init(cdns->usb3_phy);
> - if (ret)
> - goto err1;
> -
> - ret = phy_power_on(cdns->usb2_phy);
> - if (ret)
> - goto err2;
> -
> - ret = phy_power_on(cdns->usb3_phy);
> - if (ret)
> - goto err3;
> -
> sw_desc.set = cdns3_role_set;
> sw_desc.get = cdns3_role_get;
> sw_desc.allow_userspace_control = true;
> @@ -490,78 +410,47 @@ static int cdns3_probe(struct platform_device *pdev)
>
> cdns->role_sw = usb_role_switch_register(dev, &sw_desc);
> if (IS_ERR(cdns->role_sw)) {
> - ret = PTR_ERR(cdns->role_sw);
> dev_warn(dev, "Unable to register Role Switch\n");
> - goto err4;
> + return PTR_ERR(cdns->role_sw);
> }
>
> ret = cdns3_drd_init(cdns);
> if (ret)
> - goto err5;
> + goto init_failed;
>
> ret = cdns3_core_init_role(cdns);
> if (ret)
> - goto err5;
> -
> - device_set_wakeup_capable(dev, true);
> - pm_runtime_set_active(dev);
> - pm_runtime_enable(dev);
> + goto init_failed;
>
> - /*
> - * The controller needs less time between bus and controller suspend,
> - * and we also needs a small delay to avoid frequently entering low
> - * power mode.
> - */
> - pm_runtime_set_autosuspend_delay(dev, 20);
> - pm_runtime_mark_last_busy(dev);
> - pm_runtime_use_autosuspend(dev);
> dev_dbg(dev, "Cadence USB3 core: probe succeed\n");
>
> return 0;
> -err5:
> +init_failed:
> cdns3_drd_exit(cdns);
> usb_role_switch_unregister(cdns->role_sw);
> -err4:
> - phy_power_off(cdns->usb3_phy);
> -
> -err3:
> - phy_power_off(cdns->usb2_phy);
> -err2:
> - phy_exit(cdns->usb3_phy);
> -err1:
> - phy_exit(cdns->usb2_phy);
>
> return ret;
> }
>
> /**
> * cdns3_remove - unbind drd driver and clean up
> - * @pdev: Pointer to Linux platform device
> + * @cdns: Pointer to cdnsp structure.
> *
> * Returns 0 on success otherwise negative errno
> */
> -static int cdns3_remove(struct platform_device *pdev)
> +int cdns3_remove(struct cdns3 *cdns)
> {
> - struct cdns3 *cdns = platform_get_drvdata(pdev);
> -
> - pm_runtime_get_sync(&pdev->dev);
> - pm_runtime_disable(&pdev->dev);
> - pm_runtime_put_noidle(&pdev->dev);
> cdns3_exit_roles(cdns);
> usb_role_switch_unregister(cdns->role_sw);
> - phy_power_off(cdns->usb2_phy);
> - phy_power_off(cdns->usb3_phy);
> - phy_exit(cdns->usb2_phy);
> - phy_exit(cdns->usb3_phy);
> +
> return 0;
> }
>
> #ifdef CONFIG_PM_SLEEP
>
> -static int cdns3_suspend(struct device *dev)
> +int cdns3_suspend(struct cdns3 *cdns)
> {
> - struct cdns3 *cdns = dev_get_drvdata(dev);
> - unsigned long flags;
> + struct device *dev = cdns->dev;
>
> if (cdns->role == USB_ROLE_HOST)
> return 0;
> @@ -569,28 +458,21 @@ static int cdns3_suspend(struct device *dev)
> if (pm_runtime_status_suspended(dev))
> pm_runtime_resume(dev);
>
> - if (cdns->roles[cdns->role]->suspend) {
> - spin_lock_irqsave(&cdns->gadget_dev->lock, flags);
> + if (cdns->roles[cdns->role]->suspend)
> cdns->roles[cdns->role]->suspend(cdns, false);
> - spin_unlock_irqrestore(&cdns->gadget_dev->lock, flags);
> - }
>
> return 0;
> }
>
> -static int cdns3_resume(struct device *dev)
> +int cdns3_resume(struct cdns3 *cdns)
> {
> - struct cdns3 *cdns = dev_get_drvdata(dev);
> - unsigned long flags;
> + struct device *dev = cdns->dev;
>
> if (cdns->role == USB_ROLE_HOST)
> return 0;
>
> - if (cdns->roles[cdns->role]->resume) {
> - spin_lock_irqsave(&cdns->gadget_dev->lock, flags);
> + if (cdns->roles[cdns->role]->resume)
> cdns->roles[cdns->role]->resume(cdns, false);
> - spin_unlock_irqrestore(&cdns->gadget_dev->lock, flags);
> - }
>
> pm_runtime_disable(dev);
> pm_runtime_set_active(dev);
> @@ -599,32 +481,3 @@ static int cdns3_resume(struct device *dev)
> return 0;
> }
> #endif
> -
> -static const struct dev_pm_ops cdns3_pm_ops = {
> - SET_SYSTEM_SLEEP_PM_OPS(cdns3_suspend, cdns3_resume)
> -};
> -
> -#ifdef CONFIG_OF
> -static const struct of_device_id of_cdns3_match[] = {
> - { .compatible = "cdns,usb3" },
> - { },
> -};
> -MODULE_DEVICE_TABLE(of, of_cdns3_match);
> -#endif
> -
> -static struct platform_driver cdns3_driver = {
> - .probe = cdns3_probe,
> - .remove = cdns3_remove,
> - .driver = {
> - .name = "cdns-usb3",
> - .of_match_table = of_match_ptr(of_cdns3_match),
> - .pm = &cdns3_pm_ops,
> - },
> -};
> -
> -module_platform_driver(cdns3_driver);
> -
> -MODULE_ALIAS("platform:cdns3");
> -MODULE_AUTHOR("Pawel Laszczak <pawell@xxxxxxxxxxx>");
> -MODULE_LICENSE("GPL v2");
> -MODULE_DESCRIPTION("Cadence USB3 DRD Controller Driver");
> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
> index c09fdde3ae8f..284707c19620 100644
> --- a/drivers/usb/cdns3/core.h
> +++ b/drivers/usb/cdns3/core.h
> @@ -3,7 +3,7 @@
> * Cadence USBSS DRD Header File.
> *
> * Copyright (C) 2017-2018 NXP
> - * Copyright (C) 2018-2019 Cadence.
> + * Copyright (C) 2018-2020 Cadence.
> *
> * Authors: Peter Chen <peter.chen@xxxxxxx>
> * Pawel Laszczak <pawell@xxxxxxxxxxx>
> @@ -97,5 +97,11 @@ struct cdns3 {
> };
>
> int cdns3_hw_role_switch(struct cdns3 *cdns);
> +int cdns3_init(struct cdns3 *cdns);
> +int cdns3_remove(struct cdns3 *cdns);
>
> +#ifdef CONFIG_PM_SLEEP
> +int cdns3_resume(struct cdns3 *cdns);
> +int cdns3_suspend(struct cdns3 *cdns);
> +#endif /* CONFIG_PM_SLEEP */
> #endif /* __LINUX_CDNS3_CORE_H */

Next message: David Miller: "Re: [PATCH][next] net/sched: cls_u32: Replace one-element array with flexible-array member"
Previous message: David Miller: "Re: [PATCH][next] qed/qed_ll2: Replace one-element array with flexible-array member"
In reply to: Pawel Laszczak: "[PATCH 2/8] usb: cdns3: Split core.c into cdns3-plat and core.c file"
Next in thread: Pawel Laszczak: "RE: [PATCH 2/8] usb: cdns3: Split core.c into cdns3-plat and core.c file"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]