diff mbox series

[net-next] r8153_ecm: avoid to be prior to r8152 driver

Message ID 1394712342-15778-393-Taiwan-albertk@realtek.com (mailing list archive)
State Superseded
Headers show
Series [net-next] r8153_ecm: avoid to be prior to r8152 driver | expand

Commit Message

Hayes Wang Nov. 16, 2020, 6:52 a.m. UTC
Avoid r8153_ecm is compiled as built-in, if r8152 driver is compiled
as modules. Otherwise, the r8153_ecm would be used, even though the
device is supported by r8152 driver.

Fixes: c1aedf015ebd ("net/usb/r8153_ecm: support ECM mode for RTL8153")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/Makefile | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Marek Szyprowski Nov. 16, 2020, 9:18 a.m. UTC | #1
Hi

On 16.11.2020 07:52, Hayes Wang wrote:
> Avoid r8153_ecm is compiled as built-in, if r8152 driver is compiled
> as modules. Otherwise, the r8153_ecm would be used, even though the
> device is supported by r8152 driver.
>
> Fixes: c1aedf015ebd ("net/usb/r8153_ecm: support ECM mode for RTL8153")
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>

Yes, this fixes this issue, although I would prefer a separate Kconfig 
entry for r8153_ecm with proper dependencies instead of this ifdefs in 
Makefile.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>   drivers/net/usb/Makefile | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
> index 99381e6bea78..98f4c100955e 100644
> --- a/drivers/net/usb/Makefile
> +++ b/drivers/net/usb/Makefile
> @@ -13,7 +13,7 @@ obj-$(CONFIG_USB_LAN78XX)	+= lan78xx.o
>   obj-$(CONFIG_USB_NET_AX8817X)	+= asix.o
>   asix-y := asix_devices.o asix_common.o ax88172a.o
>   obj-$(CONFIG_USB_NET_AX88179_178A)      += ax88179_178a.o
> -obj-$(CONFIG_USB_NET_CDCETHER)	+= cdc_ether.o r8153_ecm.o
> +obj-$(CONFIG_USB_NET_CDCETHER)	+= cdc_ether.o
>   obj-$(CONFIG_USB_NET_CDC_EEM)	+= cdc_eem.o
>   obj-$(CONFIG_USB_NET_DM9601)	+= dm9601.o
>   obj-$(CONFIG_USB_NET_SR9700)	+= sr9700.o
> @@ -41,3 +41,11 @@ obj-$(CONFIG_USB_NET_QMI_WWAN)	+= qmi_wwan.o
>   obj-$(CONFIG_USB_NET_CDC_MBIM)	+= cdc_mbim.o
>   obj-$(CONFIG_USB_NET_CH9200)	+= ch9200.o
>   obj-$(CONFIG_USB_NET_AQC111)	+= aqc111.o
> +
> +ifdef CONFIG_USB_NET_CDCETHER
> +ifeq ($(CONFIG_USB_RTL8152), m)
> +obj-$(CONFIG_USB_RTL8152)	+= r8153_ecm.o
> +else
> +obj-$(CONFIG_USB_NET_CDCETHER)	+= r8153_ecm.o
> +endif
> +endif

Best regards
Jakub Kicinski Nov. 16, 2020, 5:02 p.m. UTC | #2
On Mon, 16 Nov 2020 10:18:13 +0100 Marek Szyprowski wrote:
> On 16.11.2020 07:52, Hayes Wang wrote:
> > Avoid r8153_ecm is compiled as built-in, if r8152 driver is compiled
> > as modules. Otherwise, the r8153_ecm would be used, even though the
> > device is supported by r8152 driver.
> >
> > Fixes: c1aedf015ebd ("net/usb/r8153_ecm: support ECM mode for RTL8153")
> > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Signed-off-by: Hayes Wang <hayeswang@realtek.com>  
> 
> Yes, this fixes this issue, although I would prefer a separate Kconfig 
> entry for r8153_ecm with proper dependencies instead of this ifdefs in 
> Makefile.

Agreed, this is what dependency resolution is for.

Let's just make this a separate Kconfig entry.
Hayes Wang Nov. 17, 2020, 1:50 a.m. UTC | #3
Jakub Kicinski <kuba@kernel.org>
> Sent: Tuesday, November 17, 2020 1:03 AM
[...]
> > Yes, this fixes this issue, although I would prefer a separate Kconfig
> > entry for r8153_ecm with proper dependencies instead of this ifdefs in
> > Makefile.
> 
> Agreed, this is what dependency resolution is for.
> 
> Let's just make this a separate Kconfig entry.

Excuse me. I am not familiar with Kconfig.

I wish r8153_ecm could be used, even
CONFIG_USB_RTL8152 is not defined.

How should set it in Kconfig? 

Best Regards,
Hayes
Jakub Kicinski Nov. 17, 2020, 4:11 p.m. UTC | #4
On Tue, 17 Nov 2020 01:50:03 +0000 Hayes Wang wrote:
> Jakub Kicinski <kuba@kernel.org>
> > Sent: Tuesday, November 17, 2020 1:03 AM  
> [...]
> > > Yes, this fixes this issue, although I would prefer a separate Kconfig
> > > entry for r8153_ecm with proper dependencies instead of this ifdefs in
> > > Makefile.  
> > 
> > Agreed, this is what dependency resolution is for.
> > 
> > Let's just make this a separate Kconfig entry.  
> 
> Excuse me. I am not familiar with Kconfig.
> 
> I wish r8153_ecm could be used, even
> CONFIG_USB_RTL8152 is not defined.
> 
> How should set it in Kconfig? 

Something like this?

config USB_RTL8153_ECM
	tristate <headline text>
	select MII
	select USB_NET_CDCETHER
	depends on USB_RTL8152 || USB_RTL8152=n
	help
		<you help text>


select clauses will pull in the dependencies you need, and the
dependency on RTL8152 will be satisfied either when RTL8152's code 
is reachable (both are modules or RTL8152 is built in) or when RTL8152
is not built at all.

Does that help?
Hayes Wang Nov. 18, 2020, 1:21 a.m. UTC | #5
Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, November 18, 2020 12:12 AM
[...]
> Something like this?
> 
> config USB_RTL8153_ECM
> 	tristate <headline text>
> 	select MII
> 	select USB_NET_CDCETHER
> 	depends on USB_RTL8152 || USB_RTL8152=n
> 	help
> 		<you help text>
> 
> 
> select clauses will pull in the dependencies you need, and the
> dependency on RTL8152 will be satisfied either when RTL8152's code
> is reachable (both are modules or RTL8152 is built in) or when RTL8152
> is not built at all.
> 
> Does that help?

Thanks a lot.
I would test it.

Best Regards,
Hayes
diff mbox series

Patch

diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index 99381e6bea78..98f4c100955e 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -13,7 +13,7 @@  obj-$(CONFIG_USB_LAN78XX)	+= lan78xx.o
 obj-$(CONFIG_USB_NET_AX8817X)	+= asix.o
 asix-y := asix_devices.o asix_common.o ax88172a.o
 obj-$(CONFIG_USB_NET_AX88179_178A)      += ax88179_178a.o
-obj-$(CONFIG_USB_NET_CDCETHER)	+= cdc_ether.o r8153_ecm.o
+obj-$(CONFIG_USB_NET_CDCETHER)	+= cdc_ether.o
 obj-$(CONFIG_USB_NET_CDC_EEM)	+= cdc_eem.o
 obj-$(CONFIG_USB_NET_DM9601)	+= dm9601.o
 obj-$(CONFIG_USB_NET_SR9700)	+= sr9700.o
@@ -41,3 +41,11 @@  obj-$(CONFIG_USB_NET_QMI_WWAN)	+= qmi_wwan.o
 obj-$(CONFIG_USB_NET_CDC_MBIM)	+= cdc_mbim.o
 obj-$(CONFIG_USB_NET_CH9200)	+= ch9200.o
 obj-$(CONFIG_USB_NET_AQC111)	+= aqc111.o
+
+ifdef CONFIG_USB_NET_CDCETHER
+ifeq ($(CONFIG_USB_RTL8152), m)
+obj-$(CONFIG_USB_RTL8152)	+= r8153_ecm.o
+else
+obj-$(CONFIG_USB_NET_CDCETHER)	+= r8153_ecm.o
+endif
+endif