Skip to contentSkip to conversation

driver:usb&net: add missing platform_driver owner

PATCH In review Libo Chen 6 participants 11 messages May 21, 2013 – May 22, 2013

lore
Share & tools
lore

Libo Chen

author
#0 lore raw

cover driver:usb&net: add missing platform_driver owner

I find a lot of mistakes using struct platform_driver without owner. So I pick up some of them including usb and net modules

Libo Chen (19):
drivers/usb/musb: add missing platform_driver owner drivers/usb/host: add missing platform_driver owner drivers/usb/chipidea: add missing platform_driver owner drivers/usb/dwc3: add missing platform_driver owner drivers/ust/renesas_usbhs: add missing platform_driver owner drivers/usb/phy: add missing platform_driver owner drivers/usb/gadget: add missing platform_driver owner drivers/net/ethernet/amd: add missing platform_driver owner drivers/net/ethernet: add missing platform_driver owner drivers/net/ethernet/freescale: add missing platform_driver owner drivers/net/ethernet/natsemi: add missing platform_driver owner drivers/net/ethernet/xscale: add missing platform_driver owner drivers/net/ethernet/calxeda: add missing platform_driver owner drivers/net/wireless/brcm80211/brcmfmac: add missing platform_driver
owner
drivers/net/irda: add missing platform_driver owner drivers/net/wan: add missing platform_driver owner drivers/net/ethernet/nxp: add missing platform_driver owner drivers/net/ethernet/marvell: add missing platform_driver owne drivers/net/ethernet/renesas: add missing platform_driver owner

 drivers/net/ethernet/amd/am79c961a.c                   | 1 +
 drivers/net/ethernet/calxeda/xgmac.c                   | 1 +
 drivers/net/ethernet/dnet.c                            | 3 ++-
 drivers/net/ethernet/freescale/xgmac_mdio.c            | 1 +
 drivers/net/ethernet/korina.c                          | 1 +
 drivers/net/ethernet/marvell/mvneta.c                  | 1 +
 drivers/net/ethernet/natsemi/xtsonic.c                 | 1 +
 drivers/net/ethernet/nxp/lpc_eth.c                     | 1 +
 drivers/net/ethernet/renesas/sh_eth.c                  | 1 +
 drivers/net/ethernet/xscale/ixp4xx_eth.c               | 1 +
 drivers/net/irda/bfin_sir.c                            | 1 +
 drivers/net/irda/sh_irda.c                             | 1 +
 drivers/net/irda/sh_sir.c                              | 1 +
 drivers/net/irda/smsc-ircc2.c                          | 1 +
 drivers/net/wan/ixp4xx_hss.c                           | 1 +
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c | 3 ++-
 drivers/usb/chipidea/ci13xxx_msm.c                     | 2 +-
 drivers/usb/chipidea/core.c                            | 1 +
 drivers/usb/dwc3/core.c                                | 1 +
 drivers/usb/dwc3/dwc3-exynos.c                         | 1 +
 drivers/usb/dwc3/dwc3-omap.c                           | 1 +
 drivers/usb/gadget/r8a66597-udc.c                      | 1 +
 drivers/usb/host/ehci-atmel.c                          | 1 +
 drivers/usb/host/ehci-msm.c                            | 1 +
 drivers/usb/host/ehci-mv.c                             | 1 +
 drivers/usb/host/ehci-mxc.c                            | 1 +
 drivers/usb/host/ehci-omap.c                           | 1 +
 drivers/usb/host/ehci-spear.c                          | 1 +
 drivers/usb/host/isp1760-if.c                          | 1 +
 drivers/usb/host/oxu210hp-hcd.c                        | 1 +
 drivers/usb/host/xhci-plat.c                           | 1 +
 drivers/usb/musb/am35x.c                               | 1 +
 drivers/usb/musb/blackfin.c                            | 1 +
 drivers/usb/musb/da8xx.c                               | 1 +
 drivers/usb/musb/davinci.c                             | 1 +
 drivers/usb/musb/omap2430.c                            | 1 +
 drivers/usb/musb/tusb6010.c                            | 1 +
 drivers/usb/musb/ux500.c                               | 1 +
 drivers/usb/phy/phy-rcar-usb.c                         | 1 +
 drivers/usb/renesas_usbhs/common.c                     | 1 +
 40 files changed, 42 insertions(+), 3 deletions(-)
Signature · 1 line
1.8.1.2
#1 lore raw

From: Libo Chen <[email protected]>
Date: Tue, 21 May 2013 10:42:00 +0800

Quoted 2 lines

I find a lot of mistakes using struct platform_driver without owner. So I pick up some of them including usb and net modules

Instead of doing it this way, which is obviously error prone and easy to forget, make platform_driver_register() be a macro which sets the module owner field then calls the real __platform_driver_register().

#2 lore raw

Dear David Miller,

Quoted 10 lines · David Miller

From: Libo Chen <[email protected]>
Date: Tue, 21 May 2013 10:42:00 +0800

Quoted 2 lines

I find a lot of mistakes using struct platform_driver without owner. So I pick up some of them including usb and net modules

Instead of doing it this way, which is obviously error prone and easy to forget, make platform_driver_register() be a macro which sets the module owner field then calls the real __platform_driver_register().

Or, maybe make the existing module_platform_driver() macro do this?

Best regards,

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#3 lore raw

From: Thomas Petazzoni <[email protected]>
Date: Tue, 21 May 2013 08:41:08 +0200

Quoted 9 lines

Dear David Miller,

Quoted 6 lines · David Miller

From: Libo Chen <[email protected]>
Date: Tue, 21 May 2013 10:42:00 +0800

Quoted 2 lines

I find a lot of mistakes using struct platform_driver without owner. So I pick up some of them including usb and net modules

Quoted 6 lines
Quoted 4 lines

Instead of doing it this way, which is obviously error prone and easy to forget, make platform_driver_register() be a macro which sets the module owner field then calls the real __platform_driver_register().

Or, maybe make the existing module_platform_driver() macro do this?

Even better.

#4 lore raw
In reply to Thomas Petazzoni
Quoted 9 lines · Thomas Petazzoni

Dear David Miller,

Quoted 6 lines · David Miller

From: Libo Chen <[email protected]>
Date: Tue, 21 May 2013 10:42:00 +0800

Quoted 2 lines

I find a lot of mistakes using struct platform_driver without owner. So I pick up some of them including usb and net modules

Quoted 6 lines
Quoted 4 lines

Instead of doing it this way, which is obviously error prone and easy to forget, make platform_driver_register() be a macro which sets the module owner field then calls the real __platform_driver_register().

Or, maybe make the existing module_platform_driver() macro do this?

But not all the modules use module_platform_driver() macro to replace the module init/exit.

Thanks,
Gu

Quoted 4 lines

Best regards,

Thomas

#5 lore raw
Quoted 16 lines · Gu Zheng
Quoted 14 lines · Thomas Petazzoni

Dear David Miller,

Quoted 10 lines · David Miller

From: Libo Chen <[email protected]>
Date: Tue, 21 May 2013 10:42:00 +0800

Quoted 2 lines

I find a lot of mistakes using struct platform_driver without owner. So I pick up some of them including usb and net modules

Instead of doing it this way, which is obviously error prone and easy to forget, make platform_driver_register() be a macro which sets the module owner field then calls the real __platform_driver_register().

Quoted 8 lines
Quoted 1 line

Or, maybe make the existing module_platform_driver() macro do this?

But not all the modules use module_platform_driver() macro to replace the module init/exit.

Thanks,
Gu

yes, there are many drivers register platform_driver by platform_driver_register manually.

make both platform_driver_register() and module_platform_driver() to check and set the module owner field?

Quoted 9 lines
Quoted 4 lines

Best regards,

Thomas

.

#6 lore raw
Quoted 22 lines · Libo Chen
Quoted 21 lines · Gu Zheng
Quoted 9 lines · Thomas Petazzoni

Dear David Miller,

Quoted 6 lines · David Miller

From: Libo Chen <[email protected]>
Date: Tue, 21 May 2013 10:42:00 +0800

Quoted 2 lines

I find a lot of mistakes using struct platform_driver without owner. So I pick up some of them including usb and net modules

Quoted 6 lines
Quoted 4 lines

Instead of doing it this way, which is obviously error prone and easy to forget, make platform_driver_register() be a macro which sets the module owner field then calls the real __platform_driver_register().

Or, maybe make the existing module_platform_driver() macro do this?

But not all the modules use module_platform_driver() macro to replace the module init/exit.

Quoted 7 lines
Quoted 3 lines

Thanks,
Gu

yes, there are many drivers register platform_driver by platform_driver_register manually.

make both platform_driver_register() and module_platform_driver() to check and set the module owner field?

No, module_platform_driver() will call platform_driver_register() to register platform_driver, just do it in platform_driver_register() is enough.
As David mentioned, making platform_driver_register() be a macro and sets the module owner field in it, seems a good way.

Thanks,
Gu

Quoted 20 lines
Quoted 4 lines
Quoted 4 lines
Quoted 8 lines
Quoted 3 lines

Best regards,

Thomas

.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#7 lore raw
In reply to Gu Zheng

Dear Gu Zheng,

Quoted 3 lines · Gu Zheng
Quoted 1 line

Or, maybe make the existing module_platform_driver() macro do this?

But not all the modules use module_platform_driver() macro to replace the module init/exit.

Then maybe it's a good opportunity to convert those ones to use module_platform_driver() ?

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#8 lore raw
Quoted 4 lines · Thomas Petazzoni

Dear Gu Zheng,

On Tue, 21 May 2013 16:00:19 +0800, Gu Zheng wrote:

Quoted 9 lines
Quoted 3 lines
Quoted 1 line

Or, maybe make the existing module_platform_driver() macro do this?

But not all the modules use module_platform_driver() macro to replace the module init/exit.

Then maybe it's a good opportunity to convert those ones to use module_platform_driver() ?

Thomas

In my opinion, not all modules can use module_platform_driver() macro to replace the module init/exit easily, like us3mc_init.

Furthermore this work will touch various platforms and architectures, I am worried it is hard to *test* (compile and boot).

What do you think?

#9 lore raw
Quoted 12 lines · Libo Chen
Quoted 11 lines · Thomas Petazzoni

Dear Gu Zheng,

Quoted 3 lines · Gu Zheng
Quoted 1 line

Or, maybe make the existing module_platform_driver() macro do this?

But not all the modules use module_platform_driver() macro to replace the module init/exit.

Then maybe it's a good opportunity to convert those ones to use module_platform_driver() ?

Quoted 6 lines
Quoted 2 lines

Thomas

In my opinion, not all modules can use module_platform_driver() macro to replace the module init/exit easily, like us3mc_init.

Furthermore this work will touch various platforms and architectures, I am worried it is hard to *test* (compile and boot).

Agree. I stick to the way that David mentioned.

Thanks,
Gu

Quoted 4 lines

What do you think?

#10 lore raw
In reply to Libo Chen
Quoted 2 lines · Libo Chen

I find a lot of mistakes using struct platform_driver without owner. So I pick up some of them including usb and net modules

Like others said, make the function call a macro that sets the module owner in it, like all of the other major subsystem registration calls do (usb_drivers, pci_drivers, etc.)

thanks,

greg k-h