RE: [PATCH v2 03/10] usb: cdns3: Moves reusable code to separate module

From: Pawel Laszczak
Date: Tue Nov 10 2020 - 04:21:42 EST

Next message: David Brazdil: "Re: [PATCH v1 17/24] kvm: arm64: Add __hyp_pa_symbol helper macro"
Previous message: Tony Lindgren: "[PATCHv2] bus: ti-sysc: Fix bogus resetdone warning on enable for cpsw"
In reply to: Peter Chen: "Re: [PATCH v2 03/10] usb: cdns3: Moves reusable code to separate module"
Next in thread: Peter Chen: "Re: [PATCH v2 03/10] usb: cdns3: Moves reusable code to separate module"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi,

>
>On 20-11-06 12:42:53, Pawel Laszczak wrote:
>> Patch moves common reusable code used by cdns3 and cdnsp driver
>> to cdns-usb-common library. This library include core.c, drd.c
>> and host.c files.
>>
>> Signed-off-by: Pawel Laszczak <pawell@xxxxxxxxxxx>
>> ---
>> drivers/usb/cdns3/Kconfig | 8 ++++++++
>> drivers/usb/cdns3/Makefile | 8 +++++---
>> drivers/usb/cdns3/cdns3-plat.c | 2 ++
>> drivers/usb/cdns3/core.c | 18 +++++++++++++++---
>> drivers/usb/cdns3/core.h | 11 +++++++----
>> drivers/usb/cdns3/drd.c | 3 ++-
>> drivers/usb/cdns3/drd.h | 4 ++--
>> 7 files changed, 41 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
>> index 84716d216ae5..58154c0a73ac 100644
>> --- a/drivers/usb/cdns3/Kconfig
>> +++ b/drivers/usb/cdns3/Kconfig
>> @@ -1,8 +1,15 @@
>> +config CDNS_USB_COMMON
>> + tristate
>> +
>> +config CDNS_USB_HOST
>> + bool
>> +
>> config USB_CDNS3
>> tristate "Cadence USB3 Dual-Role Controller"
>> depends on USB_SUPPORT && (USB || USB_GADGET) && HAS_DMA
>> select USB_XHCI_PLATFORM if USB_XHCI_HCD
>> select USB_ROLE_SWITCH
>> + select CDNS_USB_COMMON
>> help
>> Say Y here if your system has a Cadence USB3 dual-role controller.
>> It supports: dual-role switch, Host-only, and Peripheral-only.
>> @@ -25,6 +32,7 @@ config USB_CDNS3_GADGET
>> config USB_CDNS3_HOST
>> bool "Cadence USB3 host controller"
>> depends on USB=y || USB=USB_CDNS3
>> + select CDNS_USB_HOST
>> help
>> Say Y here to enable host controller functionality of the
>> Cadence driver.
>> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
>> index a1fe9612053a..16df87abf3cf 100644
>> --- a/drivers/usb/cdns3/Makefile
>> +++ b/drivers/usb/cdns3/Makefile
>> @@ -2,17 +2,19 @@
>> # define_trace.h needs to know how to find our header
>> CFLAGS_trace.o := -I$(src)
>>
>> -cdns3-y := cdns3-plat.o core.o drd.o
>> +cdns-usb-common-y := core.o drd.o
>> +cdns3-y := cdns3-plat.o
>>
>> obj-$(CONFIG_USB_CDNS3) += cdns3.o
>> +obj-$(CONFIG_CDNS_USB_COMMON) += cdns-usb-common.o
>> +
>> +cdns-usb-common-$(CONFIG_CDNS_USB_HOST) += host.o
>> cdns3-$(CONFIG_USB_CDNS3_GADGET) += gadget.o ep0.o
>>
>> ifneq ($(CONFIG_USB_CDNS3_GADGET),)
>> cdns3-$(CONFIG_TRACING) += trace.o
>> endif
>>
>> -cdns3-$(CONFIG_USB_CDNS3_HOST) += host.o
>> -
>> obj-$(CONFIG_USB_CDNS3_PCI_WRAP) += cdns3-pci-wrap.o
>> obj-$(CONFIG_USB_CDNS3_TI) += cdns3-ti.o
>> obj-$(CONFIG_USB_CDNS3_IMX) += cdns3-imx.o
>> diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
>> index b74882af3a9f..562163c81911 100644
>> --- a/drivers/usb/cdns3/cdns3-plat.c
>> +++ b/drivers/usb/cdns3/cdns3-plat.c
>> @@ -18,6 +18,7 @@
>> #include <linux/pm_runtime.h>
>>
>> #include "core.h"
>> +#include "gadget-export.h"
>>
>> static int set_phy_power_on(struct cdns3 *cdns)
>> {
>> @@ -134,6 +135,7 @@ static int cdns3_plat_probe(struct platform_device *pdev)
>> if (ret)
>> goto err_phy_power_on;
>>
>> + cdns->gadget_init = cdns3_gadget_init;
>> ret = cdns3_init(cdns);
>> if (ret)
>> goto err_cdns_init;
>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>> index 758fd5d67196..4fedf32855af 100644
>> --- a/drivers/usb/cdns3/core.c
>> +++ b/drivers/usb/cdns3/core.c
>> @@ -19,10 +19,8 @@
>> #include <linux/io.h>
>> #include <linux/pm_runtime.h>
>>
>> -#include "gadget.h"
>> #include "core.h"
>> #include "host-export.h"
>> -#include "gadget-export.h"
>> #include "drd.h"
>>
>> static int cdns3_idle_init(struct cdns3 *cdns);
>> @@ -147,7 +145,11 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
>> }
>>
>> if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
>> - ret = cdns3_gadget_init(cdns);
>> + if (cdns->gadget_init)
>> + ret = cdns->gadget_init(cdns);
>> + else
>> + ret = -ENXIO;
>> +
>> if (ret) {
>> dev_err(dev, "Device initialization failed with %d\n",
>> ret);
>> @@ -473,6 +475,7 @@ int cdns3_init(struct cdns3 *cdns)
>>
>> return ret;
>> }
>> +EXPORT_SYMBOL_GPL(cdns3_init);
>>
>> /**
>> * cdns3_remove - unbind drd driver and clean up
>> @@ -487,6 +490,7 @@ int cdns3_remove(struct cdns3 *cdns)
>>
>> return 0;
>> }
>> +EXPORT_SYMBOL_GPL(cdns3_remove);
>>
>> #ifdef CONFIG_PM_SLEEP
>> int cdns3_suspend(struct cdns3 *cdns)
>> @@ -505,6 +509,7 @@ int cdns3_suspend(struct cdns3 *cdns)
>>
>> return 0;
>> }
>> +EXPORT_SYMBOL_GPL(cdns3_suspend);
>>
>> int cdns3_resume(struct cdns3 *cdns, u8 set_active)
>> {
>> @@ -521,4 +526,11 @@ int cdns3_resume(struct cdns3 *cdns, u8 set_active)
>>
>> return 0;
>> }
>> +EXPORT_SYMBOL_GPL(cdns3_resume);
>> #endif /* CONFIG_PM_SLEEP */
>> +
>> +MODULE_AUTHOR("Peter Chen <peter.chen@xxxxxxx>");
>> +MODULE_AUTHOR("Pawel Laszczak <pawell@xxxxxxxxxxx>");
>> +MODULE_AUTHOR("Roger Quadros <rogerq@xxxxxx>");
>> +MODULE_DESCRIPTION("Cadence USBSS and USBSSP DRD Driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
>> index 7e5d9a344a53..96bdab7e8357 100644
>> --- a/drivers/usb/cdns3/core.h
>> +++ b/drivers/usb/cdns3/core.h
>> @@ -75,6 +75,7 @@ struct cdns3_platform_data {
>> * @wakeup_pending: wakeup interrupt pending
>> * @pdata: platform data from glue layer
>> * @lock: spinlock structure
>> + * @gadget_init: pointer to gadget initialization function
>> */
>> struct cdns3 {
>> struct device *dev;
>> @@ -111,14 +112,16 @@ struct cdns3 {
>> bool wakeup_pending;
>> struct cdns3_platform_data *pdata;
>> spinlock_t lock;
>> +
>> + int (*gadget_init)(struct cdns3 *cdns);
>> };
>>
>> int cdns3_hw_role_switch(struct cdns3 *cdns);
>> -int cdns3_init(struct cdns3 *cdns);
>> -int cdns3_remove(struct cdns3 *cdns);
>> +extern int cdns3_init(struct cdns3 *cdns);
>> +extern int cdns3_remove(struct cdns3 *cdns);
>
>Why add "extern" here and below?
>

These functions are the API between cdnsp and cdns3 modules.
It's looks like a common approach in kernel.
Many or even most of API function in kernel has "extern".

Of course, here we have little different situation because these API functions
are limited only to cdns3 directory.

was not sure about that, but I think that this extern is the
information that these functions are used, or can be used
by other modules.

Am I right ?

>>
>> #ifdef CONFIG_PM_SLEEP
>> -int cdns3_resume(struct cdns3 *cdns, u8 set_active);
>> -int cdns3_suspend(struct cdns3 *cdns);
>> +extern int cdns3_resume(struct cdns3 *cdns, u8 set_active);
>> +extern int cdns3_suspend(struct cdns3 *cdns);
>> #endif /* CONFIG_PM_SLEEP */
>> #endif /* __LINUX_CDNS3_CORE_H */
>> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
>> index ed8cde91a02c..1874dc6018f0 100644
>> --- a/drivers/usb/cdns3/drd.c
>> +++ b/drivers/usb/cdns3/drd.c
>> @@ -15,7 +15,6 @@
>> #include <linux/iopoll.h>
>> #include <linux/usb/otg.h>
>>
>> -#include "gadget.h"
>> #include "drd.h"
>> #include "core.h"
>>
>> @@ -226,6 +225,7 @@ int cdns3_drd_gadget_on(struct cdns3 *cdns)
>> phy_set_mode(cdns->usb3_phy, PHY_MODE_USB_DEVICE);
>> return 0;
>> }
>> +EXPORT_SYMBOL_GPL(cdns3_drd_gadget_on);
>>
>> /**
>> * cdns3_drd_gadget_off - stop gadget.
>> @@ -249,6 +249,7 @@ void cdns3_drd_gadget_off(struct cdns3 *cdns)
>> 1, 2000000);
>> phy_set_mode(cdns->usb3_phy, PHY_MODE_INVALID);
>> }
>> +EXPORT_SYMBOL_GPL(cdns3_drd_gadget_off);
>>
>> /**
>> * cdns3_init_otg_mode - initialize drd controller
>> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
>> index d752d8806a38..972aba8a40b6 100644
>> --- a/drivers/usb/cdns3/drd.h
>> +++ b/drivers/usb/cdns3/drd.h
>> @@ -209,8 +209,8 @@ int cdns3_get_vbus(struct cdns3 *cdns);
>> int cdns3_drd_init(struct cdns3 *cdns);
>> int cdns3_drd_exit(struct cdns3 *cdns);
>> int cdns3_drd_update_mode(struct cdns3 *cdns);
>> -int cdns3_drd_gadget_on(struct cdns3 *cdns);
>> -void cdns3_drd_gadget_off(struct cdns3 *cdns);
>> +extern int cdns3_drd_gadget_on(struct cdns3 *cdns);
>> +extern void cdns3_drd_gadget_off(struct cdns3 *cdns);
>> int cdns3_drd_host_on(struct cdns3 *cdns);
>> void cdns3_drd_host_off(struct cdns3 *cdns);
>>
>> --
>> 2.17.1
>>

--
Thanks
Pawel Laszczak

Next message: David Brazdil: "Re: [PATCH v1 17/24] kvm: arm64: Add __hyp_pa_symbol helper macro"
Previous message: Tony Lindgren: "[PATCHv2] bus: ti-sysc: Fix bogus resetdone warning on enable for cpsw"
In reply to: Peter Chen: "Re: [PATCH v2 03/10] usb: cdns3: Moves reusable code to separate module"
Next in thread: Peter Chen: "Re: [PATCH v2 03/10] usb: cdns3: Moves reusable code to separate module"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]