| Message ID | 20201106114300.1245-4-pawell@cadence.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduced new Cadence USBSSP DRD Driver. | expand |
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@cadence.com> > --- > 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@nxp.com>"); > +MODULE_AUTHOR("Pawel Laszczak <pawell@cadence.com>"); > +MODULE_AUTHOR("Roger Quadros <rogerq@ti.com>"); > +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? Peter > > #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 >
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@cadence.com> >> --- >> 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@nxp.com>"); >> +MODULE_AUTHOR("Pawel Laszczak <pawell@cadence.com>"); >> +MODULE_AUTHOR("Roger Quadros <rogerq@ti.com>"); >> +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
On 20-11-10 09:20:54, Pawel Laszczak wrote: > Hi, > > >> > >> 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". > Even you have not written "extern" keyword, the "extern" is added implicitly by compiler. Usually, we use "extern" for variable or the function is defined at assembly. You could see some "extern" keyword use cases at include/linux/device.h. Never mind, it is not a issue. Peter > 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
On Tue, Nov 10, 2020 at 11:21:22AM +0000, Peter Chen wrote: > On 20-11-10 09:20:54, Pawel Laszczak wrote: > > Hi, > > > > >> > > >> 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". > > > > Even you have not written "extern" keyword, the "extern" is > added implicitly by compiler. Usually, we use "extern" for variable > or the function is defined at assembly. You could see some > "extern" keyword use cases at include/linux/device.h. We are moving away from using this keyword for functions now, if at all possible please. Only use it for variables, I think checkpatch now catches it as well. thanks, greg k-h
> >On Tue, Nov 10, 2020 at 11:21:22AM +0000, Peter Chen wrote: >> On 20-11-10 09:20:54, Pawel Laszczak wrote: >> > Hi, >> > >> > >> >> > >> 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". >> > >> >> Even you have not written "extern" keyword, the "extern" is >> added implicitly by compiler. Usually, we use "extern" for variable >> or the function is defined at assembly. You could see some >> "extern" keyword use cases at include/linux/device.h. > >We are moving away from using this keyword for functions now, if at all >possible please. Only use it for variables, I think checkpatch now >catches it as well. > Ok, I will remove all extern from driver. Removing it also will remove checkpatch.pl warmings. Thanks Pawel
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@nxp.com>"); +MODULE_AUTHOR("Pawel Laszczak <pawell@cadence.com>"); +MODULE_AUTHOR("Roger Quadros <rogerq@ti.com>"); +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); #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);
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@cadence.com> --- 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(-)