| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <DM6PR07MB55294E87F6D76BA3C04E510ADDE90@DM6PR07MB5529.namprd07.prod.outlook.com>
Date: Tue, 10 Nov 2020 09:20:54 +0000
From: Pawel Laszczak <pawell@...ence.com>
To: Peter Chen <peter.chen@....com>
CC: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"balbi@...nel.org" <balbi@...nel.org>,
"colin.king@...onical.com" <colin.king@...onical.com>,
"rogerq@...com" <rogerq@...com>, Rahul Kumar <kurahul@...ence.com>,
"nsekhar@...com" <nsekhar@...com>
Subject: RE: [PATCH v2 03/10] usb: cdns3: Moves reusable code to separate
module
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@...ence.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@....com>");
>> +MODULE_AUTHOR("Pawel Laszczak <pawell@...ence.com>");
>> +MODULE_AUTHOR("Roger Quadros <rogerq@...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
Powered by blists - more mailing lists