Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Pawel Laszczak <pawell@cadence.com>
To:     Peter Chen <peter.chen@nxp.com>
CC:     "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "balbi@kernel.org" <balbi@kernel.org>,
        "colin.king@canonical.com" <colin.king@canonical.com>,
        "rogerq@ti.com" <rogerq@ti.com>, Rahul Kumar <kurahul@cadence.com>,
        "nsekhar@ti.com" <nsekhar@ti.com>
Subject: RE: [PATCH v2 03/10] usb: cdns3: Moves reusable code to separate
 module
Thread-Topic: [PATCH v2 03/10] usb: cdns3: Moves reusable code to separate
 module
Thread-Index: AQHWtDIwjXrTHzGzLka9II0WlmWMgKnBGfiAgAAAUrA=
Date:   Tue, 10 Nov 2020 09:20:54 +0000
Message-ID: <DM6PR07MB55294E87F6D76BA3C04E510ADDE90@DM6PR07MB5529.namprd07.prod.outlook.com>
References: <20201106114300.1245-1-pawell@cadence.com>
 <20201106114300.1245-4-pawell@cadence.com>
 <20201110090854.GB22481@b29397-desktop>
In-Reply-To: <20201110090854.GB22481@b29397-desktop>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 8dba1b45-7ac5-4586-d14c-08d88559ece1
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Nov 2020 09:20:54.3542
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: d36035c5-6ce6-4662-a3dc-e762e61ae4c9
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: lgRPLZR/ax+ljd9rMIuTzCfpi6HpKNNolwQ2+eiowJgtp3HMQQqHgmYTskYAuKoD6W6UGS7B9OvRD10Gl+E4q6Cegh+ey33NSe40RfD8BKs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR07MB5674
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.312,18.0.737
 definitions=2020-11-10_04:2020-11-05,2020-11-10 signatures=0
X-Proofpoint-Spam-Details: rule=outbound_check_notspam policy=outbound_check score=0 phishscore=0
 mlxlogscore=829 mlxscore=0 malwarescore=0 spamscore=0 adultscore=0
 clxscore=1015 lowpriorityscore=0 bulkscore=0 suspectscore=0
 priorityscore=1501 impostorscore=0 classifier=spam adjust=0 reason=mlx
 scancount=1 engine=8.12.0-2009150000 definitions=main-2011100067
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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=3Dy || USB=3DUSB_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				:=3D -I$(src)
>>
>> -cdns3-y					:=3D cdns3-plat.o core.o drd.o
>> +cdns-usb-common-y			:=3D core.o drd.o
>> +cdns3-y					:=3D cdns3-plat.o
>>
>>  obj-$(CONFIG_USB_CDNS3)			+=3D cdns3.o
>> +obj-$(CONFIG_CDNS_USB_COMMON)		+=3D cdns-usb-common.o
>> +
>> +cdns-usb-common-$(CONFIG_CDNS_USB_HOST) +=3D host.o
>>  cdns3-$(CONFIG_USB_CDNS3_GADGET)	+=3D gadget.o ep0.o
>>
>>  ifneq ($(CONFIG_USB_CDNS3_GADGET),)
>>  cdns3-$(CONFIG_TRACING)			+=3D trace.o
>>  endif
>>
>> -cdns3-$(CONFIG_USB_CDNS3_HOST)		+=3D host.o
>> -
>>  obj-$(CONFIG_USB_CDNS3_PCI_WRAP)	+=3D cdns3-pci-wrap.o
>>  obj-$(CONFIG_USB_CDNS3_TI)		+=3D cdns3-ti.o
>>  obj-$(CONFIG_USB_CDNS3_IMX)		+=3D cdns3-imx.o
>> diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-pl=
at.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 =3D cdns3_gadget_init;
>>  	ret =3D 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 =3D=3D USB_DR_MODE_OTG || dr_mode =3D=3D USB_DR_MODE_PERIP=
HERAL) {
>> -		ret =3D cdns3_gadget_init(cdns);
>> +		if (cdns->gadget_init)
>> +			ret =3D cdns->gadget_init(cdns);
>> +		else
>> +			ret =3D -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".=20

Of course, here we have little different situation because these API functi=
ons
are limited only to cdns3 directory.=20

 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