Hey Arend, On 8 February 2017 at 10:54, Arend Van Spriel <arend.vanspriel@xxxxxxxxxxxx> wrote: > On 2-2-2017 22:33, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@xxxxxxxxxx> >> >> This will allow getting struct device reference from the passed >> brcmf_pub for the needs of dev_err. More detailed messages are really >> important for home routers which frequently have 2 (or even 3) wireless >> cards supported by brcmfmac. >> >> Note that all calls are yet to be updated as for now brcmf_err macro >> always passes NULL. This will be handled in following patch to make this >> change easier to review. > > I prefer brcmf_err() to have struct device reference directly instead of > using brcmf_pub. That would remove the need for patches 5 till 7 as bus > specific code already has struct device. So I have acked the first three > patches, but would like to revise 8 and 9. I wanted to have some better perspective so I analyzed few upstream Linux wireless drivers. It seems to be a pretty common pattern to have one struct describing hardware that gets passed across the whole driver. This way you can access any important info from any place of the driver & some will probably say it also simplifies a driver. For example following drivers use following struct in almost every part of their code (across multiple files): ath9k: struct ath_hw ath10k: struct ath10k iwlmvm: struct iwl_mvm mt7601u: struct mt7601u_dev mwifiex: struct mwifiex_private rt2x00: struct rt2x00_dev The most common / generic struct I found in brcmfmac was struct brcmf_pub. That's why I tried to use it for the commonly used brcmf_err. What's the reason for hiding this struct from few random parts of the code? Is there any real gain from this? -- Rafał
