| Message ID | 20170821222817.17376-17-hauke@hauke-m.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Arend, can you review this? johannes -- To unsubscribe from this list: send the line "unsubscribe backports" in
On 06-09-17 17:05, Johannes Berg wrote:
> Arend, can you review this?
Sorry for the late response. I saw the patch flying by and intended to
look into it, but did not get to it yet. Raising the prio ;-)
Regards,
Arend
--
To unsubscribe from this list: send the line "unsubscribe backports" in
On 07-09-17 11:04, KAVITA MATHUR wrote: > Hi, > > I am using backports git and ath10k git to download latest changes in ath0k driver. > There are build errors in ath10k drivers due to definitions not backported from kernel > 4.12. Backpors-git is backported kernel till 4.8. Please suggest how to fix these issues. > > Following is the build error log: > > CC [M] /home/shw/kavita/backports/backports-output_14Aug17/compat/backport-3.13.o > CC [M] /home/shw/kavita/backports/backports-output_14Aug17/compat/backport-3.15.o > CC [M] /home/shw/kavita/backports/backports-output_14Aug17/compat/backport-3.18.o > CC [M] /home/shw/kavita/backports/backports-output_14Aug17/compat/backport-3.19.o > CC [M] /home/shw/kavita/backports/backports-output_14Aug17/compat/backport-4.1.o > CC [M] /home/shw/kavita/backports/backports-output_14Aug17/compat/backport-4.2.o > CC [M] /home/shw/kavita/backports/backports-output_14Aug17/compat/backport-4.4.o > CC [M] /home/shw/kavita/backports/backports-output_14Aug17/compat/backport-4.5.o > CC [M] /home/shw/kavita/backports/backports-output_14Aug17/compat/backport-4.7.o > LD [M] /home/shw/kavita/backports/backports-output_14Aug17/compat/compat.o > CC [M] > /home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/main.o > CC [M] > /home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/regd.o > CC [M] /home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/hw.o > CC [M] /home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/key.o > CC [M] > /home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/dfs_pattern_detector.o > CC [M] > /home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/dfs_pri_detector.o > LD [M] /home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/ath.o > CC [M] > /home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/ath10k/mac.o > In file included from > /home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/ath10k/mac.h:22, > from > /home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/ath10k/mac.c:18: > /home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/ath10k/core.h:465: > error: expected specifier-qualifier-list before 'guid_t' > /home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/ath10k/mac.c: > In function 'ath10k_tx_h_add_p2p_noa_ie': > /home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/ath10k/mac.c:3492: > error: implicit declaration of function 'skb_put_data' > make[8]: *** > [/home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/ath10k/mac.o] > Error 1 > make[7]: *** > [/home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath/ath10k] > Error 2 > make[6]: *** > [/home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless/ath] Error 2 > make[5]: *** [/home/shw/kavita/backports/backports-output_14Aug17/drivers/net/wireless] > Error 2 > make[4]: *** [_module_/home/shw/kavita/backports/backports-output_14Aug17] Error 2 > make[3]: *** [modules] Error 2 > make[2]: *** [modules] Error 2 > make[1]: *** [modules] Error 2 > make: *** [default] Error 2 Hi Kavita, You could try the patch submitted by Hauke titled "[PATCH 03/21] header: skbuff: add skb_put_zero(), skb_put_data() and skb_put_u8()", but you will likely stumble on more. His entire patch series should do the trick. It is still under review. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe backports" in
On 22-08-17 00:28, Hauke Mehrtens wrote: > brcmfmac uses a complicated netdev destructor handling. The > brcmf_net_attach() function just adds a normal destructor and later the > brcmf_add_if() function sets the needs_free_netdev callback. > > The normal spatch was not applied correctly to this file, add a patch > before to try to fx this problem manually. Way overdue, but better late than never. I think we prefer to use spatch, but I understand the destructor handling in brcmfmac is complicated. The story above does not tell it right. brcmf_add_if() is called first doing the alloc_netdev() setting needs_free_netdev to true and subsequently brcmf_net_attach() is called doing the register_netdevice(). When that is successful and only then I set the priv_destructor. The reason for this was to keep the error path simple, because when register_netdevice() fails it calls the priv_destructor although that is not documented in struct net_device: * @priv_destructor: Called from unregister I think I will make an attempt to change brcmfmac so we can get rid of this patch file and rely on the spatch, but for now I am fine with it. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe backports" in
On Wed, 2017-09-13 at 21:36 +0200, Arend van Spriel wrote: > > Way overdue, but better late than never. No worries, I already applied the patch anyway ;-) > I think we prefer to use > spatch, but I understand the destructor handling in brcmfmac is > complicated. Agree - and making changes across header files is hard in spatch. > The story above does not tell it right. brcmf_add_if() is called > first > doing the alloc_netdev() setting needs_free_netdev to true and > subsequently brcmf_net_attach() is called doing the > register_netdevice(). When that is successful and only then I set > the > priv_destructor. The reason for this was to keep the error path > simple, > because when register_netdevice() fails it calls the priv_destructor > although that is not documented in struct net_device: > > * @priv_destructor: Called from unregister Ok, too late now I guess, since the patch is in. But at least we'll have your explanation here :) > I think I will make an attempt to change brcmfmac so we can get rid > of this patch file and rely on the spatch, but for now I am fine with > it. Thanks for the review! johannes -- To unsubscribe from this list: send the line "unsubscribe backports" in
On 09/14/2017 09:21 AM, Johannes Berg wrote: > On Wed, 2017-09-13 at 21:36 +0200, Arend van Spriel wrote: >> >> Way overdue, but better late than never. > > No worries, I already applied the patch anyway ;-) > >> I think we prefer to use >> spatch, but I understand the destructor handling in brcmfmac is >> complicated. > > Agree - and making changes across header files is hard in spatch. I have no idea how to do this with spatch, I would appreciate if you could convert this to an spatch, I do not know how. >> The story above does not tell it right. brcmf_add_if() is called >> first >> doing the alloc_netdev() setting needs_free_netdev to true and >> subsequently brcmf_net_attach() is called doing the >> register_netdevice(). When that is successful and only then I set >> the >> priv_destructor. The reason for this was to keep the error path >> simple, >> because when register_netdevice() fails it calls the priv_destructor >> although that is not documented in struct net_device: >> >> * @priv_destructor: Called from unregister > > Ok, too late now I guess, since the patch is in. But at least we'll > have your explanation here :) > >> I think I will make an attempt to change brcmfmac so we can get rid >> of this patch file and rely on the spatch, but for now I am fine with >> it. > > Thanks for the review! I fixed this patch in a later commit again, there was a problem with recent kernel versions. Hauke -- To unsubscribe from this list: send the line "unsubscribe backports" in
On 9/18/2017 12:00 AM, Hauke Mehrtens wrote: > On 09/14/2017 09:21 AM, Johannes Berg wrote: >> On Wed, 2017-09-13 at 21:36 +0200, Arend van Spriel wrote: >>> >>> Way overdue, but better late than never. >> >> No worries, I already applied the patch anyway ;-) >> >>> I think we prefer to use >>> spatch, but I understand the destructor handling in brcmfmac is >>> complicated. >> >> Agree - and making changes across header files is hard in spatch. > > I have no idea how to do this with spatch, I would appreciate if you > could convert this to an spatch, I do not know how. No problem. Well, maybe it is, but I will give it a try ;-) >>> The story above does not tell it right. brcmf_add_if() is called >>> first >>> doing the alloc_netdev() setting needs_free_netdev to true and >>> subsequently brcmf_net_attach() is called doing the >>> register_netdevice(). When that is successful and only then I set >>> the >>> priv_destructor. The reason for this was to keep the error path >>> simple, >>> because when register_netdevice() fails it calls the priv_destructor >>> although that is not documented in struct net_device: >>> >>> * @priv_destructor: Called from unregister >> >> Ok, too late now I guess, since the patch is in. But at least we'll >> have your explanation here :) >> >>> I think I will make an attempt to change brcmfmac so we can get rid >>> of this patch file and rely on the spatch, but for now I am fine with >>> it. >> >> Thanks for the review! > > I fixed this patch in a later commit again, there was a problem with > recent kernel versions. I saw that patch, but did not incorporate it in this response. Thanks for fixing it. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe backports" in
diff --git a/patches/0079-netdev-destructor/brcmfmac.patch b/patches/0079-netdev-destructor/brcmfmac.patch new file mode 100644 index 00000000..3f328b26 --- /dev/null +++ b/patches/0079-netdev-destructor/brcmfmac.patch @@ -0,0 +1,35 @@ +diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +index b5a561b..6f5466f 100644 +--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c ++++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +@@ -462,6 +462,18 @@ static const struct net_device_ops brcmf_netdev_ops_pri = { + .ndo_set_rx_mode = brcmf_netdev_set_multicast_list + }; + ++#undef netdev_set_priv_destructor ++#define netdev_set_priv_destructor(_dev, _destructor) \ ++ (_dev)->destructor = _destructor ++ ++#if LINUX_VERSION_IS_LESS(4,12,0) ++static void __brcmf_cfg80211_free_netdev(struct net_device *ndev) ++{ ++ brcmf_cfg80211_free_netdev(ndev); ++ free_netdev(ndev); ++} ++#endif ++ + int brcmf_net_attach(struct brcmf_if *ifp, bool rtnl_locked) + { + struct brcmf_pub *drvr = ifp->drvr; +@@ -634,7 +646,11 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx, + if (!ndev) + return ERR_PTR(-ENOMEM); + ++#if LINUX_VERSION_IS_LESS(4,12,0) ++ ndev->priv_destructor = __brcmf_cfg80211_free_netdev; ++#else + ndev->needs_free_netdev = true; ++#endif + ifp = netdev_priv(ndev); + ifp->ndev = ndev; + /* store mapping ifidx to bsscfgidx */
brcmfmac uses a complicated netdev destructor handling. The brcmf_net_attach() function just adds a normal destructor and later the brcmf_add_if() function sets the needs_free_netdev callback. The normal spatch was not applied correctly to this file, add a patch before to try to fx this problem manually. Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> --- patches/0079-netdev-destructor/brcmfmac.patch | 35 +++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 patches/0079-netdev-destructor/brcmfmac.patch