Hi,
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.
After this patch added, I will try respin my etnaviv patch set.
If the PCI core could shield the hazard for us.
I would prefer my etnaviv don't introduce any config option.
Compile anywhere at any case.
Bjorn
--
Jingfeng
