| Message ID | 20201102112410.1049272-42-lee.jones@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [01/41] wil6210: wmi: Correct misnamed function parameter 'ptr_' | expand |
On Mon, Nov 2, 2020 at 3:25 AM Lee Jones <lee.jones@linaro.org> wrote: > --- a/drivers/net/wireless/realtek/rtw88/pci.h > +++ b/drivers/net/wireless/realtek/rtw88/pci.h > @@ -212,6 +212,10 @@ struct rtw_pci { > void __iomem *mmap; > }; > > +int rtw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id); > +void rtw_pci_remove(struct pci_dev *pdev); > +void rtw_pci_shutdown(struct pci_dev *pdev); > + > These definitions are already in 4 other header files: drivers/net/wireless/realtek/rtw88/rtw8723de.h drivers/net/wireless/realtek/rtw88/rtw8821ce.h drivers/net/wireless/realtek/rtw88/rtw8822be.h drivers/net/wireless/realtek/rtw88/rtw8822ce.h Seems like you should be moving them, not just adding yet another duplicate. Brian
On Mon, 02 Nov 2020, Brian Norris wrote: > On Mon, Nov 2, 2020 at 3:25 AM Lee Jones <lee.jones@linaro.org> wrote: > > --- a/drivers/net/wireless/realtek/rtw88/pci.h > > +++ b/drivers/net/wireless/realtek/rtw88/pci.h > > @@ -212,6 +212,10 @@ struct rtw_pci { > > void __iomem *mmap; > > }; > > > > +int rtw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id); > > +void rtw_pci_remove(struct pci_dev *pdev); > > +void rtw_pci_shutdown(struct pci_dev *pdev); > > + > > > > These definitions are already in 4 other header files: > > drivers/net/wireless/realtek/rtw88/rtw8723de.h > drivers/net/wireless/realtek/rtw88/rtw8821ce.h > drivers/net/wireless/realtek/rtw88/rtw8822be.h > drivers/net/wireless/realtek/rtw88/rtw8822ce.h > > Seems like you should be moving them, not just adding yet another duplicate. I followed the current convention. Happy to optimise if that's what is required.
Lee Jones <lee.jones@linaro.org> writes: > On Mon, 02 Nov 2020, Brian Norris wrote: > >> On Mon, Nov 2, 2020 at 3:25 AM Lee Jones <lee.jones@linaro.org> wrote: >> > --- a/drivers/net/wireless/realtek/rtw88/pci.h >> > +++ b/drivers/net/wireless/realtek/rtw88/pci.h >> > @@ -212,6 +212,10 @@ struct rtw_pci { >> > void __iomem *mmap; >> > }; >> > >> > +int rtw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id); >> > +void rtw_pci_remove(struct pci_dev *pdev); >> > +void rtw_pci_shutdown(struct pci_dev *pdev); >> > + >> > >> >> These definitions are already in 4 other header files: >> >> drivers/net/wireless/realtek/rtw88/rtw8723de.h >> drivers/net/wireless/realtek/rtw88/rtw8821ce.h >> drivers/net/wireless/realtek/rtw88/rtw8822be.h >> drivers/net/wireless/realtek/rtw88/rtw8822ce.h >> >> Seems like you should be moving them, not just adding yet another duplicate. > > I followed the current convention. > > Happy to optimise if that's what is required. I agree with Brian, these and rtw_pm_ops should be moved to pci.h to avoid code duplication.
On Sat, 07 Nov 2020, Kalle Valo wrote: > Lee Jones <lee.jones@linaro.org> writes: > > > On Mon, 02 Nov 2020, Brian Norris wrote: > > > >> On Mon, Nov 2, 2020 at 3:25 AM Lee Jones <lee.jones@linaro.org> wrote: > >> > --- a/drivers/net/wireless/realtek/rtw88/pci.h > >> > +++ b/drivers/net/wireless/realtek/rtw88/pci.h > >> > @@ -212,6 +212,10 @@ struct rtw_pci { > >> > void __iomem *mmap; > >> > }; > >> > > >> > +int rtw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id); > >> > +void rtw_pci_remove(struct pci_dev *pdev); > >> > +void rtw_pci_shutdown(struct pci_dev *pdev); > >> > + > >> > > >> > >> These definitions are already in 4 other header files: > >> > >> drivers/net/wireless/realtek/rtw88/rtw8723de.h > >> drivers/net/wireless/realtek/rtw88/rtw8821ce.h > >> drivers/net/wireless/realtek/rtw88/rtw8822be.h > >> drivers/net/wireless/realtek/rtw88/rtw8822ce.h > >> > >> Seems like you should be moving them, not just adding yet another duplicate. > > > > I followed the current convention. > > > > Happy to optimise if that's what is required. > > I agree with Brian, these and rtw_pm_ops should be moved to pci.h to > avoid code duplication. Will do, thanks.
diff --git a/drivers/net/wireless/realtek/rtw88/pci.h b/drivers/net/wireless/realtek/rtw88/pci.h index ca17aa9cf7dc7..7cdbb9533a09a 100644 --- a/drivers/net/wireless/realtek/rtw88/pci.h +++ b/drivers/net/wireless/realtek/rtw88/pci.h @@ -212,6 +212,10 @@ struct rtw_pci { void __iomem *mmap; }; +int rtw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id); +void rtw_pci_remove(struct pci_dev *pdev); +void rtw_pci_shutdown(struct pci_dev *pdev); + static inline u32 max_num_of_tx_queue(u8 queue) { u32 max_num;
Fixes the following W=1 kernel build warning(s): drivers/net/wireless/realtek/rtw88/pci.c:1488:5: warning: no previous prototype for ‘rtw_pci_probe’ [-Wmissing-prototypes] 1488 | int rtw_pci_probe(struct pci_dev *pdev, | ^~~~~~~~~~~~~ drivers/net/wireless/realtek/rtw88/pci.c:1568:6: warning: no previous prototype for ‘rtw_pci_remove’ [-Wmissing-prototypes] 1568 | void rtw_pci_remove(struct pci_dev *pdev) | ^~~~~~~~~~~~~~ drivers/net/wireless/realtek/rtw88/pci.c:1590:6: warning: no previous prototype for ‘rtw_pci_shutdown’ [-Wmissing-prototypes] 1590 | void rtw_pci_shutdown(struct pci_dev *pdev) | ^~~~~~~~~~~~~~~~ Cc: Yan-Hsuan Chuang <yhchuang@realtek.com> Cc: Kalle Valo <kvalo@codeaurora.org> Cc: "David S. Miller" <davem@davemloft.net> Cc: Jakub Kicinski <kuba@kernel.org> Cc: linux-wireless@vger.kernel.org Cc: netdev@vger.kernel.org Signed-off-by: Lee Jones <lee.jones@linaro.org> --- drivers/net/wireless/realtek/rtw88/pci.h | 4 ++++ 1 file changed, 4 insertions(+)