| Message ID | 20230530101655.2275731-1-suijingfeng@loongson.cn |
|---|---|
| State | New |
| Headers | show |
| Series | linux/pci.h: add a dummy implement for pci_clear_master() | expand |
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. 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. Bjorn [1] https://lore.kernel.org/all/20230530160643.2344551-6-suijingfeng@loongson.cn/ > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202305301659.4guSLavL-lkp@intel.com/ > Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> > --- > include/linux/pci.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index d0c19ff0c958..71c85380676c 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1904,6 +1904,7 @@ static inline int pci_dev_present(const struct pci_device_id *ids) > #define pci_dev_put(dev) do { } while (0) > > static inline void pci_set_master(struct pci_dev *dev) { } > +static inline void pci_clear_master(struct pci_dev *dev) { } > static inline int pci_enable_device(struct pci_dev *dev) { return -EIO; } > static inline void pci_disable_device(struct pci_dev *dev) { } > static inline int pcim_enable_device(struct pci_dev *pdev) { return -EIO; } > -- > 2.25.1 >
Hi, 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. This has side effects, but they are not severe. It boils down to the compilation time thing, while originally we want it to be a runtime thing. Driver writers have to isolate PCI driver-related subroutines in a separate source file. with the DRM_ETNAVIV_PCI_DRIVER option, guard callers of those subroutines, to let them not get compiled when CONFIG_PCIis disabled. > Bjorn > > [1] https://lore.kernel.org/all/20230530160643.2344551-6-suijingfeng@loongson.cn/ > >> Reported-by: kernel test robot <lkp@intel.com> >> Closes: https://lore.kernel.org/oe-kbuild-all/202305301659.4guSLavL-lkp@intel.com/ >> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> >> --- >> include/linux/pci.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/include/linux/pci.h b/include/linux/pci.h >> index d0c19ff0c958..71c85380676c 100644 >> --- a/include/linux/pci.h >> +++ b/include/linux/pci.h >> @@ -1904,6 +1904,7 @@ static inline int pci_dev_present(const struct pci_device_id *ids) >> #define pci_dev_put(dev) do { } while (0) >> >> static inline void pci_set_master(struct pci_dev *dev) { } >> +static inline void pci_clear_master(struct pci_dev *dev) { } >> static inline int pci_enable_device(struct pci_dev *dev) { return -EIO; } >> static inline void pci_disable_device(struct pci_dev *dev) { } >> static inline int pcim_enable_device(struct pci_dev *pdev) { return -EIO; } >> -- >> 2.25.1 >>
Hi Sui, On Tue, May 30, 2023 at 12:21 PM Sui Jingfeng <suijingfeng@loongson.cn> 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 > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202305301659.4guSLavL-lkp@intel.com/ > Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> Thanks for your patch! > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1904,6 +1904,7 @@ static inline int pci_dev_present(const struct pci_device_id *ids) > #define pci_dev_put(dev) do { } while (0) > > static inline void pci_set_master(struct pci_dev *dev) { } > +static inline void pci_clear_master(struct pci_dev *dev) { } > static inline int pci_enable_device(struct pci_dev *dev) { return -EIO; } > static inline void pci_disable_device(struct pci_dev *dev) { } > static inline int pcim_enable_device(struct pci_dev *pdev) { return -EIO; } Makes perfect sense, given there has been a dummy for pci_set_master() since the git stone age. Apparently adding the dummy was forgotten when pci_clear_master() was introduced. Fixes: 6a479079c07211bf ("PCI: Add pci_clear_master() as opposite of pci_set_master()") Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> Gr{oetje,eeting}s, Geert
Hi, thanks a lot. On 2023/5/31 15:04, Geert Uytterhoeven wrote: > Hi Sui, > > On Tue, May 30, 2023 at 12:21 PM Sui Jingfeng <suijingfeng@loongson.cn> 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 >> >> Reported-by: kernel test robot <lkp@intel.com> >> Closes: https://lore.kernel.org/oe-kbuild-all/202305301659.4guSLavL-lkp@intel.com/ >> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> > Thanks for your patch! > >> --- a/include/linux/pci.h >> +++ b/include/linux/pci.h >> @@ -1904,6 +1904,7 @@ static inline int pci_dev_present(const struct pci_device_id *ids) >> #define pci_dev_put(dev) do { } while (0) >> >> static inline void pci_set_master(struct pci_dev *dev) { } >> +static inline void pci_clear_master(struct pci_dev *dev) { } >> static inline int pci_enable_device(struct pci_dev *dev) { return -EIO; } >> static inline void pci_disable_device(struct pci_dev *dev) { } >> static inline int pcim_enable_device(struct pci_dev *pdev) { return -EIO; } > Makes perfect sense, given there has been a dummy for pci_set_master() > since the git stone age. Apparently adding the dummy was forgotten > when pci_clear_master() was introduced. > > Fixes: 6a479079c07211bf ("PCI: Add pci_clear_master() as opposite of > pci_set_master()") > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> > > Gr{oetje,eeting}s, > > Geert >
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. Bjorn
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
Hi Sui, On Wed, May 31, 2023 at 7:46 PM Sui Jingfeng <suijingfeng@loongson.cn> 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@tilquin.amer.corp.natinst.com Gr{oetje,eeting}s, Geert
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 > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202305301659.4guSLavL-lkp@intel.com/ > Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> I applied this to pci/misc for v6.5, with commit log as below. But I suspect that it might make your life easier if you include it with your etnaviv series. You may be able to avoid adding the CONFIG_DRM_ETNAVIV_PCI_DRIVER symbol. If so, feel free to include this patch in that series with my ack: Acked-by: Bjorn Helgaas <bhelgaas@google.com> If you do include it in your series, please use the commit log below and let me know so I can drop it from my queue. Bjorn Author: Sui Jingfeng <suijingfeng@loongson.cn> Date: Wed May 31 18:27:44 2023 +0800 PCI: Add pci_clear_master() stub for non-CONFIG_PCI Add a pci_clear_master() stub when CONFIG_PCI is not set so drivers that support both PCI and platform devices don't need #ifdefs or extra Kconfig symbols for the PCI parts. [bhelgaas: commit log] Fixes: 6a479079c072 ("PCI: Add pci_clear_master() as opposite of pci_set_master()") Link: https://lore.kernel.org/r/20230531102744.2354313-1-suijingfeng@loongson.cn Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> Acked-by: Bjorn Helgaas <bhelgaas@google.com> > --- > include/linux/pci.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index d0c19ff0c958..71c85380676c 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1904,6 +1904,7 @@ static inline int pci_dev_present(const struct pci_device_id *ids) > #define pci_dev_put(dev) do { } while (0) > > static inline void pci_set_master(struct pci_dev *dev) { } > +static inline void pci_clear_master(struct pci_dev *dev) { } > static inline int pci_enable_device(struct pci_dev *dev) { return -EIO; } > static inline void pci_disable_device(struct pci_dev *dev) { } > static inline int pcim_enable_device(struct pci_dev *pdev) { return -EIO; } > -- > 2.25.1 >
Hi, On 2023/6/7 00:13, 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 >> >> Reported-by: kernel test robot <lkp@intel.com> >> Closes: https://lore.kernel.org/oe-kbuild-all/202305301659.4guSLavL-lkp@intel.com/ >> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> > I applied this to pci/misc for v6.5, with commit log as below. > > But I suspect that it might make your life easier if you include it > with your etnaviv series. You may be able to avoid adding the > CONFIG_DRM_ETNAVIV_PCI_DRIVER symbol. > > If so, feel free to include this patch in that series with my ack: > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> I do want add this tag to my all of the patches in the drm/etnaviv series, thanks. and I really love you this idea. > If you do include it in your series, please use the commit log below > and let me know so I can drop it from my queue. No, please keep this patch merged by you. Because this patch belong to drivers/pci, Its belong to you domain. I might choose to remove the CONFIG_DRM_ETNAVIV_PCI_DRIVER option at next version of my drm/etnaviv patch If it arrived to drm/tip branch. But I guess some reviewers may still prefer a CONFIG_DRM_ETNAVIV_PCI_DRIVER option, depend on the attitude of etnaviv folks. > Bjorn > > > Author: Sui Jingfeng <suijingfeng@loongson.cn> > Date: Wed May 31 18:27:44 2023 +0800 > > PCI: Add pci_clear_master() stub for non-CONFIG_PCI > > Add a pci_clear_master() stub when CONFIG_PCI is not set so drivers that > support both PCI and platform devices don't need #ifdefs or extra Kconfig > symbols for the PCI parts. > > [bhelgaas: commit log] > Fixes: 6a479079c072 ("PCI: Add pci_clear_master() as opposite of pci_set_master()") > Link: https://lore.kernel.org/r/20230531102744.2354313-1-suijingfeng@loongson.cn > Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > >> --- >> include/linux/pci.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/include/linux/pci.h b/include/linux/pci.h >> index d0c19ff0c958..71c85380676c 100644 >> --- a/include/linux/pci.h >> +++ b/include/linux/pci.h >> @@ -1904,6 +1904,7 @@ static inline int pci_dev_present(const struct pci_device_id *ids) >> #define pci_dev_put(dev) do { } while (0) >> >> static inline void pci_set_master(struct pci_dev *dev) { } >> +static inline void pci_clear_master(struct pci_dev *dev) { } >> static inline int pci_enable_device(struct pci_dev *dev) { return -EIO; } >> static inline void pci_disable_device(struct pci_dev *dev) { } >> static inline int pcim_enable_device(struct pci_dev *pdev) { return -EIO; } >> -- >> 2.25.1 >>
On Wed, Jun 07, 2023 at 01:48:38AM +0800, Sui Jingfeng wrote: > On 2023/6/7 00:13, 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 > > > > > > Reported-by: kernel test robot <lkp@intel.com> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202305301659.4guSLavL-lkp@intel.com/ > > > Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> > > I applied this to pci/misc for v6.5, with commit log as below. > > > > But I suspect that it might make your life easier if you include it > > with your etnaviv series. You may be able to avoid adding the > > CONFIG_DRM_ETNAVIV_PCI_DRIVER symbol. > > > > If so, feel free to include this patch in that series with my ack: > > > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > I do want add this tag to my all of the patches in the drm/etnaviv series, > thanks. > > and I really love you this idea. Just to be clear, my ack only applies to the addition of the pci_clear_master() stub in pci.h. It does not apply to the patches in the drm/etnaviv series at https://lore.kernel.org/r/20230603105943.3042766-1-15330273260@189.cn > > If you do include it in your series, please use the commit log below > > and let me know so I can drop it from my queue. > > No, please keep this patch merged by you. > > Because this patch belong to drivers/pci, Its belong to you domain. > > > I might choose to remove the CONFIG_DRM_ETNAVIV_PCI_DRIVER option at next > version > > of my drm/etnaviv patch If it arrived to drm/tip branch. > > > But I guess some reviewers may still prefer a CONFIG_DRM_ETNAVIV_PCI_DRIVER > option, > > depend on the attitude of etnaviv folks. The purpose of my ack is to enable you to merge the patch along with the series that uses it. This is a normal way of handling things that cross subsystem boundaries, so you don't need to feel uncomfortable about it. It is much more difficult to remove config options than it is to add them, because you must ensure that the removal doesn't break old .config files. I will keep this patch in my queue unless you tell me to drop it. Bjorn > > Author: Sui Jingfeng <suijingfeng@loongson.cn> > > Date: Wed May 31 18:27:44 2023 +0800 > > > > PCI: Add pci_clear_master() stub for non-CONFIG_PCI > > Add a pci_clear_master() stub when CONFIG_PCI is not set so drivers that > > support both PCI and platform devices don't need #ifdefs or extra Kconfig > > symbols for the PCI parts. > > [bhelgaas: commit log] > > Fixes: 6a479079c072 ("PCI: Add pci_clear_master() as opposite of pci_set_master()") > > Link: https://lore.kernel.org/r/20230531102744.2354313-1-suijingfeng@loongson.cn > > Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> > > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > > > > --- > > > include/linux/pci.h | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > > index d0c19ff0c958..71c85380676c 100644 > > > --- a/include/linux/pci.h > > > +++ b/include/linux/pci.h > > > @@ -1904,6 +1904,7 @@ static inline int pci_dev_present(const struct pci_device_id *ids) > > > #define pci_dev_put(dev) do { } while (0) > > > static inline void pci_set_master(struct pci_dev *dev) { } > > > +static inline void pci_clear_master(struct pci_dev *dev) { } > > > static inline int pci_enable_device(struct pci_dev *dev) { return -EIO; } > > > static inline void pci_disable_device(struct pci_dev *dev) { } > > > static inline int pcim_enable_device(struct pci_dev *pdev) { return -EIO; } > > > -- > > > 2.25.1 > > > > -- > Jingfeng >
Hi, On 2023/6/7 02:46, Bjorn Helgaas wrote: > On Wed, Jun 07, 2023 at 01:48:38AM +0800, Sui Jingfeng wrote: >> On 2023/6/7 00:13, 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 >>>> >>>> Reported-by: kernel test robot <lkp@intel.com> >>>> Closes: https://lore.kernel.org/oe-kbuild-all/202305301659.4guSLavL-lkp@intel.com/ >>>> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> >>> I applied this to pci/misc for v6.5, with commit log as below. >>> >>> But I suspect that it might make your life easier if you include it >>> with your etnaviv series. You may be able to avoid adding the >>> CONFIG_DRM_ETNAVIV_PCI_DRIVER symbol. >>> >>> If so, feel free to include this patch in that series with my ack: >>> >>> Acked-by: Bjorn Helgaas <bhelgaas@google.com> >> I do want add this tag to my all of the patches in the drm/etnaviv series, >> thanks. >> >> and I really love you this idea. > Just to be clear, my ack only applies to the addition of the > pci_clear_master() stub in pci.h. It does not apply to the patches in > the drm/etnaviv series at > https://lore.kernel.org/r/20230603105943.3042766-1-15330273260@189.cn Ok, I understand what you meant then. >>> If you do include it in your series, please use the commit log below >>> and let me know so I can drop it from my queue. >> No, please keep this patch merged by you. >> >> Because this patch belong to drivers/pci, Its belong to you domain. >> >> >> I might choose to remove the CONFIG_DRM_ETNAVIV_PCI_DRIVER option at next >> version >> >> of my drm/etnaviv patch If it arrived to drm/tip branch. >> >> >> But I guess some reviewers may still prefer a CONFIG_DRM_ETNAVIV_PCI_DRIVER >> option, >> >> depend on the attitude of etnaviv folks. > The purpose of my ack is to enable you to merge the patch along with > the series that uses it. This is a normal way of handling things that > cross subsystem boundaries, so you don't need to feel uncomfortable > about it. > > It is much more difficult to remove config options than it is to add > them, because you must ensure that the removal doesn't break old > .config files. > > I will keep this patch in my queue unless you tell me to drop it. > > Bjorn > >>> Author: Sui Jingfeng <suijingfeng@loongson.cn> >>> Date: Wed May 31 18:27:44 2023 +0800 >>> >>> PCI: Add pci_clear_master() stub for non-CONFIG_PCI >>> Add a pci_clear_master() stub when CONFIG_PCI is not set so drivers that >>> support both PCI and platform devices don't need #ifdefs or extra Kconfig >>> symbols for the PCI parts. >>> [bhelgaas: commit log] >>> Fixes: 6a479079c072 ("PCI: Add pci_clear_master() as opposite of pci_set_master()") >>> Link: https://lore.kernel.org/r/20230531102744.2354313-1-suijingfeng@loongson.cn >>> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> >>> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> >>> Acked-by: Bjorn Helgaas <bhelgaas@google.com> >>> >>>> --- >>>> include/linux/pci.h | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/include/linux/pci.h b/include/linux/pci.h >>>> index d0c19ff0c958..71c85380676c 100644 >>>> --- a/include/linux/pci.h >>>> +++ b/include/linux/pci.h >>>> @@ -1904,6 +1904,7 @@ static inline int pci_dev_present(const struct pci_device_id *ids) >>>> #define pci_dev_put(dev) do { } while (0) >>>> static inline void pci_set_master(struct pci_dev *dev) { } >>>> +static inline void pci_clear_master(struct pci_dev *dev) { } >>>> static inline int pci_enable_device(struct pci_dev *dev) { return -EIO; } >>>> static inline void pci_disable_device(struct pci_dev *dev) { } >>>> static inline int pcim_enable_device(struct pci_dev *pdev) { return -EIO; } >>>> -- >>>> 2.25.1 >>>> >> -- >> Jingfeng >>
diff --git a/include/linux/pci.h b/include/linux/pci.h index d0c19ff0c958..71c85380676c 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1904,6 +1904,7 @@ static inline int pci_dev_present(const struct pci_device_id *ids) #define pci_dev_put(dev) do { } while (0) static inline void pci_set_master(struct pci_dev *dev) { } +static inline void pci_clear_master(struct pci_dev *dev) { } static inline int pci_enable_device(struct pci_dev *dev) { return -EIO; } static inline void pci_disable_device(struct pci_dev *dev) { } static inline int pcim_enable_device(struct pci_dev *pdev) { return -EIO; }