Hi Sui, On Wed, May 31, 2023 at 7:46 PM Sui Jingfeng <suijingfeng@xxxxxxxxxxx> wrote: > On 2023/6/1 01:16, Bjorn Helgaas wrote: > > On Wed, May 31, 2023 at 12:25:10PM +0800, Sui Jingfeng wrote: > >> On 2023/5/31 04:11, Bjorn Helgaas wrote: > >>> On Tue, May 30, 2023 at 06:16:55PM +0800, Sui Jingfeng wrote: > >>>> As some arch(m68k for example) doesn't have config_pci enabled, drivers[1] > >>>> call pci_clear_master() without config_pci guard can not built. > >>>> > >>>> drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c: > >>>> In function 'etnaviv_gpu_pci_fini': > >>>>>> drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c:32:9: > >>>> error: implicit declaration of function 'pci_clear_master'; > >>>> did you mean 'pci_set_master'? [-Werror=implicit-function-declaration] > >>>> 32 | pci_clear_master(pdev); > >>>> | ^~~~~~~~~~~~~~~~ > >>>> | pci_set_master > >>>> cc1: some warnings being treated as errors > >>>> > >>>> [1] https://patchwork.freedesktop.org/patch/539977/?series=118522&rev=1 > >>> I don't mind adding a stub if it's needed, but I don't understand why > >>> it's needed here. > >> For a single driver that supports both platform devices and PCI devices, > >> > >> Sometimes there is no way to separate the PCI driver part and the platform > >> driver part cleanly and clearly. > >> > >> For example, the module_init() and module_exit() functions, > >> > >> where we have to register PCI drivers and platform drivers there. > >> > >> We can't simply let the entire driver depend on PCI in Kconfig, > >> > >> This will make this driver unable to compile, which it's originally could. > >> > >> The PCI core could do such a thing for us, and > >> > >> There is no need to introduce a driver-specific guard then. > >> > >> > >> There is already a dummy stub for pci_set_master(). > >> > >> Therefore, pci_clear_master() should also have a counterpart. > >> > >> They should emerge in pairs. > >> > >> This could probably eliminate pain for PCI driver writers, > >> > >> This patch is still useful. > >> > >> > >>> The caller is in etnaviv_pci_drv.c, and if I > >>> understand the patch at [1], etnaviv_pci_drv.c is only compiled when > >>> CONFIG_PCI=y. > >> Yes, you are right. This is the right thing to do for the driver, though. > >> > >> Pure PCI device driver does not need to worry about this. > >> > >> Like drm/ast, drm/amdgpu, drm/radeon, etc. > >> > >> But drm/etnaviv is special; it's a platform driver that could pass the > >> compile test originally. > >> > >> > >> When patching it (Etnaviv) with PCI device driver support, > >> > >> This forces the PCI driver writer to add another config option. > >> > >> (which depends on the PCI config option.) in the Kconfig. > >> > >> For my case, it's theDRM_ETNAVIV_PCI_DRIVER config option. > > So if I understand correctly, you would prefer not to add the > > DRM_ETNAVIV_PCI_DRIVER config option, and if we add this stub, you > > won't need to add it? > > > > That's a good reason to add this patch. > > Yes, please add this patch. > > Otherwise, other people may suffer from the same issue someday. People already have, several years ago, cfr. https://lore.kernel.org/all/20160309003955.GA1589@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
