Message ID | 20240826072648.167004-1-s.hauer@pengutronix.de |
---|---|
Headers | show |
Series | mwifiex: add support for iw61x | expand |
> From: Sascha Hauer <s.hauer@pengutronix.de> > Sent: Monday, August 26, 2024 3:27 PM > To: Francesco Dolcini <francesco@dolcini.it> > Cc: Calvin Owens <calvin@wbinvd.org>; Brian Norris > <briannorris@chromium.org>; Kalle Valo <kvalo@kernel.org>; David Lin > <yu-hao.lin@nxp.com>; linux-wireless@vger.kernel.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de; Sascha Hauer > <s.hauer@pengutronix.de> > Subject: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > This series adds support for the iw61x chips to the mwifiex driver. > There are a few things to address, hence the RFC status. See the commit > messages for details. The series is based on wireless-next/main. > > I am sending this now since people requested it here [1], but as it's out now > feel free to leave your comments to the issues mentioned (or others I haven't > mentioned ;) > > [1] > https://lore.kern/ > el.org%2Fall%2F20240809094533.1660-1-yu-hao.lin%40nxp.com%2F&data=05 > %7C02%7Cyu-hao.lin%40nxp.com%7C184ab4fed58647150f8508dcc5a0789a%7 > C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638602540229716119% > 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB > TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=cACBHxaQvcOqu6ri > BoAlZDONRlGQ4j5DcglEV9T%2BpYU%3D&reserved=0 > > Sascha > > > Sascha Hauer (4): > wifi: mwifiex: release firmware at remove time > wifi: mwifiex: handle VDLL > wifi: mwifiex: wait longer for SDIO card status > mwifiex: add iw61x support > > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 86 +++++++++++++++++++ > drivers/net/wireless/marvell/mwifiex/fw.h | 16 ++++ > drivers/net/wireless/marvell/mwifiex/main.c | 9 +- > drivers/net/wireless/marvell/mwifiex/main.h | 4 + > drivers/net/wireless/marvell/mwifiex/sdio.c | 81 ++++++++++++++++- > drivers/net/wireless/marvell/mwifiex/sdio.h | 3 + > .../net/wireless/marvell/mwifiex/sta_event.c | 4 > + .../net/wireless/marvell/mwifiex/uap_event.c | 4 + > include/linux/mmc/sdio_ids.h | 3 + > 9 files changed, 205 insertions(+), 5 deletions(-) > > -- > 2.39.2 I think you ported VDLL related code from nxpwifi and you also traced our private/downstream MXM driver. If this is the case, I think you should know nxpwifi already cleaned up the porting VDLL code from MXM driver. I check your patch quickly. You ported the new SDIO data type (MWIFIEX_TYPE_VDLL) from nxpwifi, but you did not port the code to support this new data type from nxpwifi. In other word, you did not test your patch before submission (same as some of your patches). Another thing is that this new SDIO data type should be handled carefully with other existed SDIO data type. Nxpwifi only supports new SDIO mode, so the modification to support NXPWIFI_TYPE_VDLL can be clean and simple. If you want to port the code to Mwifiex, there is no one-to-one modification of the code. Another important thing is that you should consider if your modifications will affect existed devices or not. You need to check if you should check firmware version or chip type before adding some code. BTW, no matter if you add the code with or without checking of firmware version or chip type, you should test your modifications for IW61x and at least one legacy device if the code is added to Mwifiex. David
On Mon, Sep 02, 2024 at 02:24:53AM +0000, David Lin wrote: > > From: Sascha Hauer <s.hauer@pengutronix.de> > > Sent: Monday, August 26, 2024 3:27 PM > > To: Francesco Dolcini <francesco@dolcini.it> > > Cc: Calvin Owens <calvin@wbinvd.org>; Brian Norris > > <briannorris@chromium.org>; Kalle Valo <kvalo@kernel.org>; David Lin > > <yu-hao.lin@nxp.com>; linux-wireless@vger.kernel.org; > > linux-kernel@vger.kernel.org; kernel@pengutronix.de; Sascha Hauer > > <s.hauer@pengutronix.de> > > Subject: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > Caution: This is an external email. Please take care when clicking links or > > opening attachments. When in doubt, report the message using the 'Report > > this email' button > > > > > > This series adds support for the iw61x chips to the mwifiex driver. > > There are a few things to address, hence the RFC status. See the commit > > messages for details. The series is based on wireless-next/main. > > > > I am sending this now since people requested it here [1], but as it's out now > > feel free to leave your comments to the issues mentioned (or others I haven't > > mentioned ;) > > > > [1] > > https://lore.kern/ > > el.org%2Fall%2F20240809094533.1660-1-yu-hao.lin%40nxp.com%2F&data=05 > > %7C02%7Cyu-hao.lin%40nxp.com%7C184ab4fed58647150f8508dcc5a0789a%7 > > C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638602540229716119% > > 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB > > TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=cACBHxaQvcOqu6ri > > BoAlZDONRlGQ4j5DcglEV9T%2BpYU%3D&reserved=0 > > > > Sascha > > > > > > Sascha Hauer (4): > > wifi: mwifiex: release firmware at remove time > > wifi: mwifiex: handle VDLL > > wifi: mwifiex: wait longer for SDIO card status > > mwifiex: add iw61x support > > > > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 86 +++++++++++++++++++ > > drivers/net/wireless/marvell/mwifiex/fw.h | 16 ++++ > > drivers/net/wireless/marvell/mwifiex/main.c | 9 +- > > drivers/net/wireless/marvell/mwifiex/main.h | 4 + > > drivers/net/wireless/marvell/mwifiex/sdio.c | 81 ++++++++++++++++- > > drivers/net/wireless/marvell/mwifiex/sdio.h | 3 + > > .../net/wireless/marvell/mwifiex/sta_event.c | 4 > > + .../net/wireless/marvell/mwifiex/uap_event.c | 4 + > > include/linux/mmc/sdio_ids.h | 3 + > > 9 files changed, 205 insertions(+), 5 deletions(-) > > > > -- > > 2.39.2 > > I think you ported VDLL related code from nxpwifi and you also traced > our private/downstream MXM driver. I ported it from this repository: https://github.com/nxp-imx/mwifiex-iw612.git Is that the one you are referring to as MXM driver? > If this is the case, I think you should know nxpwifi already cleaned > up the porting VDLL code from MXM driver. > I check your patch quickly. You ported the new SDIO data type > (MWIFIEX_TYPE_VDLL) from nxpwifi, but you > did not port the code to support this new data type from nxpwifi. In > other word, you did not test your > patch before submission (same as some of your patches). I did test it. It works with the iw61x as well as older chips. There are likely details I haven't tested, but it generally works. If there are details I should test additionally please let me know. > > Another thing is that this new SDIO data type should be handled > carefully with other existed SDIO data type. > > Nxpwifi only supports new SDIO mode, so the modification to support > NXPWIFI_TYPE_VDLL can be clean and simple. If you want to port the > code to Mwifiex, there is no one-to-one modification of the code. > > Another important thing is that you should consider if your > modifications will affect existed devices or not. > You need to check if you should check firmware version or chip type > before adding some code. The VDLL code I added for the iw61x only reacts to the EVENT_VDLL_IND event. So as long as a firmware doesn't send such an event nothing is changed with this patch, and I haven't seen an older chip sending a VDLL event. > > BTW, no matter if you add the code with or without checking of > firmware version or chip type, you should test > your modifications for IW61x and at least one legacy device if the > code is added to Mwifiex. I did. Sascha
> From: Sascha Hauer <s.hauer@pengutronix.de> > Sent: Monday, September 2, 2024 2:41 PM > To: David Lin <yu-hao.lin@nxp.com> > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle Valo > <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > On Mon, Sep 02, 2024 at 02:24:53AM +0000, David Lin wrote: > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > Sent: Monday, August 26, 2024 3:27 PM > > > To: Francesco Dolcini <francesco@dolcini.it> > > > Cc: Calvin Owens <calvin@wbinvd.org>; Brian Norris > > > <briannorris@chromium.org>; Kalle Valo <kvalo@kernel.org>; David Lin > > > <yu-hao.lin@nxp.com>; linux-wireless@vger.kernel.org; > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de; Sascha Hauer > > > <s.hauer@pengutronix.de> > > > Subject: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > > > Caution: This is an external email. Please take care when clicking > > > links or opening attachments. When in doubt, report the message > > > using the 'Report this email' button > > > > > > > > > This series adds support for the iw61x chips to the mwifiex driver. > > > There are a few things to address, hence the RFC status. See the > > > commit messages for details. The series is based on wireless-next/main. > > > > > > I am sending this now since people requested it here [1], but as > > > it's out now feel free to leave your comments to the issues > > > mentioned (or others I haven't mentioned ;) > > > > > > [1] > > > https://lo/ > > > > re.kern%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C6125c51da3704fe10 > a5 > > > > a08dccb1a24ef%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6386 > 08560 > > > > 383160951%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > 2luMz > > > > IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=jfQ6FQimPpwr > nwUo > > > OCEhmpSadtrb15ymGiif%2B1UCdG0%3D&reserved=0 > > > > el.org%2Fall%2F20240809094533.1660-1-yu-hao.lin%40nxp.com%2F&data=05 > > > > %7C02%7Cyu-hao.lin%40nxp.com%7C184ab4fed58647150f8508dcc5a0789a%7 > > > > C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638602540229716119% > > > > 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB > > > > TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=cACBHxaQvcOqu6ri > > > BoAlZDONRlGQ4j5DcglEV9T%2BpYU%3D&reserved=0 > > > > > > Sascha > > > > > > > > > Sascha Hauer (4): > > > wifi: mwifiex: release firmware at remove time > > > wifi: mwifiex: handle VDLL > > > wifi: mwifiex: wait longer for SDIO card status > > > mwifiex: add iw61x support > > > > > > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 86 > +++++++++++++++++++ > > > drivers/net/wireless/marvell/mwifiex/fw.h | 16 ++++ > > > drivers/net/wireless/marvell/mwifiex/main.c | 9 +- > > > drivers/net/wireless/marvell/mwifiex/main.h | 4 + > > > drivers/net/wireless/marvell/mwifiex/sdio.c | 81 ++++++++++++++++- > > > drivers/net/wireless/marvell/mwifiex/sdio.h | 3 + > > > .../net/wireless/marvell/mwifiex/sta_event.c | 4 > > > + .../net/wireless/marvell/mwifiex/uap_event.c | 4 + > > > include/linux/mmc/sdio_ids.h | 3 + > > > 9 files changed, 205 insertions(+), 5 deletions(-) > > > > > > -- > > > 2.39.2 > > > > I think you ported VDLL related code from nxpwifi and you also traced > > our private/downstream MXM driver. > > I ported it from this repository: > > https://github.co/ > m%2Fnxp-imx%2Fmwifiex-iw612.git&data=05%7C02%7Cyu-hao.lin%40nxp.co > m%7C6125c51da3704fe10a5a08dccb1a24ef%7C686ea1d3bc2b4c6fa92cd99c5 > c301635%7C0%7C0%7C638608560383172495%7CUnknown%7CTWFpbGZsb3d > 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3 > D%7C0%7C%7C%7C&sdata=5TgI0r4u2I9Pi1FATJx32Ubn7ufmbYsBR1XkpQLAIyQ > %3D&reserved=0 > > Is that the one you are referring to as MXM driver? > Yes. > > If this is the case, I think you should know nxpwifi already cleaned > > up the porting VDLL code from MXM driver. > > I check your patch quickly. You ported the new SDIO data type > > (MWIFIEX_TYPE_VDLL) from nxpwifi, but you did not port the code to > > support this new data type from nxpwifi. In other word, you did not > > test your patch before submission (same as some of your patches). > > I did test it. It works with the iw61x as well as older chips. There are likely > details I haven't tested, but it generally works. If there are details I should test > additionally please let me know. > > > > > Another thing is that this new SDIO data type should be handled > > carefully with other existed SDIO data type. > > > > Nxpwifi only supports new SDIO mode, so the modification to support > > NXPWIFI_TYPE_VDLL can be clean and simple. If you want to port the > > code to Mwifiex, there is no one-to-one modification of the code. > > > > Another important thing is that you should consider if your > > modifications will affect existed devices or not. > > You need to check if you should check firmware version or chip type > > before adding some code. > > The VDLL code I added for the iw61x only reacts to the EVENT_VDLL_IND > event. So as long as a firmware doesn't send such an event nothing is changed > with this patch, and I haven't seen an older chip sending a VDLL event. > How about IW61x? As I mentioned before, if you test IW61x on DFS channel, command timeout will happen. Without correct VDLL porting, you will encounter command timeout in some other test cases. But testing on DFS channel will be easier to reproduce the issue. BTW, it is not a trivial job to port the support of VDLL data path from MXM driver to Mwifiex. David
> From: David Lin > Sent: Monday, September 2, 2024 2:54 PM > To: 'Sascha Hauer' <s.hauer@pengutronix.de> > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle Valo > <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de > Subject: RE: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > Sent: Monday, September 2, 2024 2:41 PM > > To: David Lin <yu-hao.lin@nxp.com> > > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle > > Valo <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > > linux-kernel@vger.kernel.org; kernel@pengutronix.de > > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > Caution: This is an external email. Please take care when clicking > > links or opening attachments. When in doubt, report the message using > > the 'Report this email' button > > > > > > On Mon, Sep 02, 2024 at 02:24:53AM +0000, David Lin wrote: > > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > Sent: Monday, August 26, 2024 3:27 PM > > > > To: Francesco Dolcini <francesco@dolcini.it> > > > > Cc: Calvin Owens <calvin@wbinvd.org>; Brian Norris > > > > <briannorris@chromium.org>; Kalle Valo <kvalo@kernel.org>; David > > > > Lin <yu-hao.lin@nxp.com>; linux-wireless@vger.kernel.org; > > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de; Sascha Hauer > > > > <s.hauer@pengutronix.de> > > > > Subject: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > > > > > Caution: This is an external email. Please take care when clicking > > > > links or opening attachments. When in doubt, report the message > > > > using the 'Report this email' button > > > > > > > > > > > > This series adds support for the iw61x chips to the mwifiex driver. > > > > There are a few things to address, hence the RFC status. See the > > > > commit messages for details. The series is based on wireless-next/main. > > > > > > > > I am sending this now since people requested it here [1], but as > > > > it's out now feel free to leave your comments to the issues > > > > mentioned (or others I haven't mentioned ;) > > > > > > > > [1] > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > > > lo > > > > > > > re.kern%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C6125c51da3704fe10 > > a5 > > > > > > > a08dccb1a24ef%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6386 > > 08560 > > > > > > > 383160951%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > > 2luMz > > > > > > > IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=jfQ6FQimPpwr > > nwUo > > > > OCEhmpSadtrb15ymGiif%2B1UCdG0%3D&reserved=0 > > > > > > > el.org%2Fall%2F20240809094533.1660-1-yu-hao.lin%40nxp.com%2F&data=05 > > > > > > > %7C02%7Cyu-hao.lin%40nxp.com%7C184ab4fed58647150f8508dcc5a0789a%7 > > > > > > > C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638602540229716119% > > > > > > > 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB > > > > > > > TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=cACBHxaQvcOqu6ri > > > > BoAlZDONRlGQ4j5DcglEV9T%2BpYU%3D&reserved=0 > > > > > > > > Sascha > > > > > > > > > > > > Sascha Hauer (4): > > > > wifi: mwifiex: release firmware at remove time > > > > wifi: mwifiex: handle VDLL > > > > wifi: mwifiex: wait longer for SDIO card status > > > > mwifiex: add iw61x support > > > > > > > > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 86 > > +++++++++++++++++++ > > > > drivers/net/wireless/marvell/mwifiex/fw.h | 16 ++++ > > > > drivers/net/wireless/marvell/mwifiex/main.c | 9 +- > > > > drivers/net/wireless/marvell/mwifiex/main.h | 4 + > > > > drivers/net/wireless/marvell/mwifiex/sdio.c | 81 > ++++++++++++++++- > > > > drivers/net/wireless/marvell/mwifiex/sdio.h | 3 + > > > > .../net/wireless/marvell/mwifiex/sta_event.c | 4 > > > > + .../net/wireless/marvell/mwifiex/uap_event.c | 4 + > > > > include/linux/mmc/sdio_ids.h | 3 + > > > > 9 files changed, 205 insertions(+), 5 deletions(-) > > > > > > > > -- > > > > 2.39.2 > > > > > > I think you ported VDLL related code from nxpwifi and you also > > > traced our private/downstream MXM driver. > > > > I ported it from this repository: > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith > > ub.co > > > m%2Fnxp-imx%2Fmwifiex-iw612.git&data=05%7C02%7Cyu-hao.lin%40nxp.co > > > m%7C6125c51da3704fe10a5a08dccb1a24ef%7C686ea1d3bc2b4c6fa92cd99c5 > > > c301635%7C0%7C0%7C638608560383172495%7CUnknown%7CTWFpbGZsb3d > > > 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3 > > > D%7C0%7C%7C%7C&sdata=5TgI0r4u2I9Pi1FATJx32Ubn7ufmbYsBR1XkpQLAIyQ > > %3D&reserved=0 > > > > Is that the one you are referring to as MXM driver? > > > > Yes. > > > > If this is the case, I think you should know nxpwifi already cleaned > > > up the porting VDLL code from MXM driver. > > > I check your patch quickly. You ported the new SDIO data type > > > (MWIFIEX_TYPE_VDLL) from nxpwifi, but you did not port the code to > > > support this new data type from nxpwifi. In other word, you did not > > > test your patch before submission (same as some of your patches). > > > > I did test it. It works with the iw61x as well as older chips. There > > are likely details I haven't tested, but it generally works. If there > > are details I should test additionally please let me know. > > > > > > > > Another thing is that this new SDIO data type should be handled > > > carefully with other existed SDIO data type. > > > > > > Nxpwifi only supports new SDIO mode, so the modification to support > > > NXPWIFI_TYPE_VDLL can be clean and simple. If you want to port the > > > code to Mwifiex, there is no one-to-one modification of the code. > > > > > > Another important thing is that you should consider if your > > > modifications will affect existed devices or not. > > > You need to check if you should check firmware version or chip type > > > before adding some code. > > > > The VDLL code I added for the iw61x only reacts to the EVENT_VDLL_IND > > event. So as long as a firmware doesn't send such an event nothing is > > changed with this patch, and I haven't seen an older chip sending a VDLL > event. > > > > How about IW61x? As I mentioned before, if you test IW61x on DFS channel, > command timeout will happen. > Without correct VDLL porting, you will encounter command timeout in some > other test cases. But testing on DFS channel will be easier to reproduce the > issue. > > BTW, it is not a trivial job to port the support of VDLL data path from MXM > driver to Mwifiex. > > David Another terrible thing is that driver can't load updated firmware of IW61x without correct VDLL porting. Firmware will request VDLL after firmware is downloaded for updated version. Because VDLL type for SDIO data path is working with existed SDIO data type. Some code are used to let this new SDIO data type can work with existed SDIO data types. If missing of these code, the porting will have issues too. David
On Mon, Sep 02, 2024 at 06:54:07AM +0000, David Lin wrote: > > From: Sascha Hauer <s.hauer@pengutronix.de> > > Sent: Monday, September 2, 2024 2:41 PM > > To: David Lin <yu-hao.lin@nxp.com> > > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle Valo > > <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > > linux-kernel@vger.kernel.org; kernel@pengutronix.de > > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > Caution: This is an external email. Please take care when clicking links or > > opening attachments. When in doubt, report the message using the 'Report > > this email' button > > > > > > On Mon, Sep 02, 2024 at 02:24:53AM +0000, David Lin wrote: > > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > Sent: Monday, August 26, 2024 3:27 PM > > > > To: Francesco Dolcini <francesco@dolcini.it> > > > > Cc: Calvin Owens <calvin@wbinvd.org>; Brian Norris > > > > <briannorris@chromium.org>; Kalle Valo <kvalo@kernel.org>; David Lin > > > > <yu-hao.lin@nxp.com>; linux-wireless@vger.kernel.org; > > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de; Sascha Hauer > > > > <s.hauer@pengutronix.de> > > > > Subject: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > > > > > Caution: This is an external email. Please take care when clicking > > > > links or opening attachments. When in doubt, report the message > > > > using the 'Report this email' button > > > > > > > > > > > > This series adds support for the iw61x chips to the mwifiex driver. > > > > There are a few things to address, hence the RFC status. See the > > > > commit messages for details. The series is based on wireless-next/main. > > > > > > > > I am sending this now since people requested it here [1], but as > > > > it's out now feel free to leave your comments to the issues > > > > mentioned (or others I haven't mentioned ;) > > > > > > > > [1] > > > > https://lo/ > > > > > > re.kern%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C6125c51da3704fe10 > > a5 > > > > > > a08dccb1a24ef%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6386 > > 08560 > > > > > > 383160951%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > > 2luMz > > > > > > IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=jfQ6FQimPpwr > > nwUo > > > > OCEhmpSadtrb15ymGiif%2B1UCdG0%3D&reserved=0 > > > > > > el.org%2Fall%2F20240809094533.1660-1-yu-hao.lin%40nxp.com%2F&data=05 > > > > > > %7C02%7Cyu-hao.lin%40nxp.com%7C184ab4fed58647150f8508dcc5a0789a%7 > > > > > > C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638602540229716119% > > > > > > 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB > > > > > > TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=cACBHxaQvcOqu6ri > > > > BoAlZDONRlGQ4j5DcglEV9T%2BpYU%3D&reserved=0 > > > > > > > > Sascha > > > > > > > > > > > > Sascha Hauer (4): > > > > wifi: mwifiex: release firmware at remove time > > > > wifi: mwifiex: handle VDLL > > > > wifi: mwifiex: wait longer for SDIO card status > > > > mwifiex: add iw61x support > > > > > > > > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 86 > > +++++++++++++++++++ > > > > drivers/net/wireless/marvell/mwifiex/fw.h | 16 ++++ > > > > drivers/net/wireless/marvell/mwifiex/main.c | 9 +- > > > > drivers/net/wireless/marvell/mwifiex/main.h | 4 + > > > > drivers/net/wireless/marvell/mwifiex/sdio.c | 81 ++++++++++++++++- > > > > drivers/net/wireless/marvell/mwifiex/sdio.h | 3 + > > > > .../net/wireless/marvell/mwifiex/sta_event.c | 4 > > > > + .../net/wireless/marvell/mwifiex/uap_event.c | 4 + > > > > include/linux/mmc/sdio_ids.h | 3 + > > > > 9 files changed, 205 insertions(+), 5 deletions(-) > > > > > > > > -- > > > > 2.39.2 > > > > > > I think you ported VDLL related code from nxpwifi and you also traced > > > our private/downstream MXM driver. > > > > I ported it from this repository: > > > > https://github.co/ > > m%2Fnxp-imx%2Fmwifiex-iw612.git&data=05%7C02%7Cyu-hao.lin%40nxp.co > > m%7C6125c51da3704fe10a5a08dccb1a24ef%7C686ea1d3bc2b4c6fa92cd99c5 > > c301635%7C0%7C0%7C638608560383172495%7CUnknown%7CTWFpbGZsb3d > > 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3 > > D%7C0%7C%7C%7C&sdata=5TgI0r4u2I9Pi1FATJx32Ubn7ufmbYsBR1XkpQLAIyQ > > %3D&reserved=0 > > > > Is that the one you are referring to as MXM driver? > > > > Yes. > > > > If this is the case, I think you should know nxpwifi already cleaned > > > up the porting VDLL code from MXM driver. > > > I check your patch quickly. You ported the new SDIO data type > > > (MWIFIEX_TYPE_VDLL) from nxpwifi, but you did not port the code to > > > support this new data type from nxpwifi. In other word, you did not > > > test your patch before submission (same as some of your patches). > > > > I did test it. It works with the iw61x as well as older chips. There are likely > > details I haven't tested, but it generally works. If there are details I should test > > additionally please let me know. > > > > > > > > Another thing is that this new SDIO data type should be handled > > > carefully with other existed SDIO data type. > > > > > > Nxpwifi only supports new SDIO mode, so the modification to support > > > NXPWIFI_TYPE_VDLL can be clean and simple. If you want to port the > > > code to Mwifiex, there is no one-to-one modification of the code. > > > > > > Another important thing is that you should consider if your > > > modifications will affect existed devices or not. > > > You need to check if you should check firmware version or chip type > > > before adding some code. > > > > The VDLL code I added for the iw61x only reacts to the EVENT_VDLL_IND > > event. So as long as a firmware doesn't send such an event nothing is changed > > with this patch, and I haven't seen an older chip sending a VDLL event. > > > > How about IW61x? As I mentioned before, if you test IW61x on DFS > channel, command timeout will happen. Without correct VDLL porting, > you will encounter command timeout in some other test cases. But > testing on DFS channel will be easier to reproduce the issue. The VDLL support in the downstream driver supports a case when a VDLL event comes in while a command is being sent. I catched this with this test: if (adapter->cmd_sent) { mwifiex_dbg(adapter, MSG, "%s: adapter is busy\n", __func__); return -EBUSY; } The downstream driver defers handling of the VDLL event to the main process in this case. I haven't implemented this case in my patch because I wasn't able to trigger it, but is this the case you are referring to? > > BTW, it is not a trivial job to port the support of VDLL data path > from MXM driver to Mwifiex. Nothing is trivial with WiFi, these drivers are complex beasts. Be careful with such arguments though, because duplicating the code into two drivers makes the situation even more complex. Sascha
> From: Sascha Hauer <s.hauer@pengutronix.de> > Sent: Monday, September 2, 2024 3:46 PM > To: David Lin <yu-hao.lin@nxp.com> > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle Valo > <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > On Mon, Sep 02, 2024 at 06:54:07AM +0000, David Lin wrote: > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > Sent: Monday, September 2, 2024 2:41 PM > > > To: David Lin <yu-hao.lin@nxp.com> > > > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > > > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle > > > Valo <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de > > > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > > > Caution: This is an external email. Please take care when clicking > > > links or opening attachments. When in doubt, report the message > > > using the 'Report this email' button > > > > > > > > > On Mon, Sep 02, 2024 at 02:24:53AM +0000, David Lin wrote: > > > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > > Sent: Monday, August 26, 2024 3:27 PM > > > > > To: Francesco Dolcini <francesco@dolcini.it> > > > > > Cc: Calvin Owens <calvin@wbinvd.org>; Brian Norris > > > > > <briannorris@chromium.org>; Kalle Valo <kvalo@kernel.org>; David > > > > > Lin <yu-hao.lin@nxp.com>; linux-wireless@vger.kernel.org; > > > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de; Sascha > > > > > Hauer <s.hauer@pengutronix.de> > > > > > Subject: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > > > > > > > Caution: This is an external email. Please take care when > > > > > clicking links or opening attachments. When in doubt, report the > > > > > message using the 'Report this email' button > > > > > > > > > > > > > > > This series adds support for the iw61x chips to the mwifiex driver. > > > > > There are a few things to address, hence the RFC status. See the > > > > > commit messages for details. The series is based on > wireless-next/main. > > > > > > > > > > I am sending this now since people requested it here [1], but as > > > > > it's out now feel free to leave your comments to the issues > > > > > mentioned (or others I haven't mentioned ;) > > > > > > > > > > [1] > > > > > https://lo/ > > > > > > > > > re.kern%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C6125c51da3704fe10 > > > a5 > > > > > > > > > a08dccb1a24ef%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6386 > > > 08560 > > > > > > > > > 383160951%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > > > 2luMz > > > > > > > > > IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=jfQ6FQimPpwr > > > nwUo > > > > > OCEhmpSadtrb15ymGiif%2B1UCdG0%3D&reserved=0 > > > > > > > > > el.org%2Fall%2F20240809094533.1660-1-yu-hao.lin%40nxp.com%2F&data=05 > > > > > > > > > %7C02%7Cyu-hao.lin%40nxp.com%7C184ab4fed58647150f8508dcc5a0789a%7 > > > > > > > > > C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638602540229716119% > > > > > > > > > 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB > > > > > > > > > TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=cACBHxaQvcOqu6ri > > > > > BoAlZDONRlGQ4j5DcglEV9T%2BpYU%3D&reserved=0 > > > > > > > > > > Sascha > > > > > > > > > > > > > > > Sascha Hauer (4): > > > > > wifi: mwifiex: release firmware at remove time > > > > > wifi: mwifiex: handle VDLL > > > > > wifi: mwifiex: wait longer for SDIO card status > > > > > mwifiex: add iw61x support > > > > > > > > > > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 86 > > > +++++++++++++++++++ > > > > > drivers/net/wireless/marvell/mwifiex/fw.h | 16 ++++ > > > > > drivers/net/wireless/marvell/mwifiex/main.c | 9 +- > > > > > drivers/net/wireless/marvell/mwifiex/main.h | 4 + > > > > > drivers/net/wireless/marvell/mwifiex/sdio.c | 81 > ++++++++++++++++- > > > > > drivers/net/wireless/marvell/mwifiex/sdio.h | 3 + > > > > > .../net/wireless/marvell/mwifiex/sta_event.c | 4 > > > > > + .../net/wireless/marvell/mwifiex/uap_event.c | 4 + > > > > > include/linux/mmc/sdio_ids.h | 3 + > > > > > 9 files changed, 205 insertions(+), 5 deletions(-) > > > > > > > > > > -- > > > > > 2.39.2 > > > > > > > > I think you ported VDLL related code from nxpwifi and you also > > > > traced our private/downstream MXM driver. > > > > > > I ported it from this repository: > > > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > > > > thub.co%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C9f55bf357ece47645 > 0b > > > > 708dccb234dae%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6386 > 08599 > > > > 700151625%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > 2luMz > > > > IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=OLf3OfFX7R% > 2BT3V > > > gWtcsqlls%2FI3ceY8r3bewwYy8bAes%3D&reserved=0 > > > > m%2Fnxp-imx%2Fmwifiex-iw612.git&data=05%7C02%7Cyu-hao.lin%40nxp.co > > > > m%7C6125c51da3704fe10a5a08dccb1a24ef%7C686ea1d3bc2b4c6fa92cd99c5 > > > > c301635%7C0%7C0%7C638608560383172495%7CUnknown%7CTWFpbGZsb3d > > > > 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3 > > > > D%7C0%7C%7C%7C&sdata=5TgI0r4u2I9Pi1FATJx32Ubn7ufmbYsBR1XkpQLAIyQ > > > %3D&reserved=0 > > > > > > Is that the one you are referring to as MXM driver? > > > > > > > Yes. > > > > > > If this is the case, I think you should know nxpwifi already > > > > cleaned up the porting VDLL code from MXM driver. > > > > I check your patch quickly. You ported the new SDIO data type > > > > (MWIFIEX_TYPE_VDLL) from nxpwifi, but you did not port the code to > > > > support this new data type from nxpwifi. In other word, you did > > > > not test your patch before submission (same as some of your patches). > > > > > > I did test it. It works with the iw61x as well as older chips. There > > > are likely details I haven't tested, but it generally works. If > > > there are details I should test additionally please let me know. > > > > > > > > > > > Another thing is that this new SDIO data type should be handled > > > > carefully with other existed SDIO data type. > > > > > > > > Nxpwifi only supports new SDIO mode, so the modification to > > > > support NXPWIFI_TYPE_VDLL can be clean and simple. If you want to > > > > port the code to Mwifiex, there is no one-to-one modification of the > code. > > > > > > > > Another important thing is that you should consider if your > > > > modifications will affect existed devices or not. > > > > You need to check if you should check firmware version or chip > > > > type before adding some code. > > > > > > The VDLL code I added for the iw61x only reacts to the > > > EVENT_VDLL_IND event. So as long as a firmware doesn't send such an > > > event nothing is changed with this patch, and I haven't seen an older chip > sending a VDLL event. > > > > > > > How about IW61x? As I mentioned before, if you test IW61x on DFS > > channel, command timeout will happen. Without correct VDLL porting, > > you will encounter command timeout in some other test cases. But > > testing on DFS channel will be easier to reproduce the issue. > > The VDLL support in the downstream driver supports a case when a VDLL > event comes in while a command is being sent. I catched this with this > test: > > if (adapter->cmd_sent) { > mwifiex_dbg(adapter, MSG, "%s: adapter is busy\n", > __func__); > return -EBUSY; > } > > The downstream driver defers handling of the VDLL event to the main process > in this case. I haven't implemented this case in my patch because I wasn't able > to trigger it, but is this the case you are referring to? > Not only this code segment. In fact, you did not add VDLL data patch support to sdio.c. If you try to add the code and do test, you will know what is missing in your code. > > > > BTW, it is not a trivial job to port the support of VDLL data path > > from MXM driver to Mwifiex. > > Nothing is trivial with WiFi, these drivers are complex beasts. Be careful with > such arguments though, because duplicating the code into two drivers makes > the situation even more complex. > > Sascha We will only support IW61x in nxpwifi. VDLL related code and future code for 11ax won't be added to Mwifiex. Other difficult things are NEW API which are only for new version of firmware. I think you did not aware of that now. David
On Mon, Sep 02, 2024 at 08:00:58AM +0000, David Lin wrote: > > From: Sascha Hauer <s.hauer@pengutronix.de> > > Sent: Monday, September 2, 2024 3:46 PM > > To: David Lin <yu-hao.lin@nxp.com> > > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle Valo > > <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > > linux-kernel@vger.kernel.org; kernel@pengutronix.de > > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > Caution: This is an external email. Please take care when clicking links or > > opening attachments. When in doubt, report the message using the 'Report > > this email' button > > > > > > On Mon, Sep 02, 2024 at 06:54:07AM +0000, David Lin wrote: > > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > Sent: Monday, September 2, 2024 2:41 PM > > > > To: David Lin <yu-hao.lin@nxp.com> > > > > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > > > > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle > > > > Valo <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de > > > > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > > > > > Caution: This is an external email. Please take care when clicking > > > > links or opening attachments. When in doubt, report the message > > > > using the 'Report this email' button > > > > > > > > > > > > On Mon, Sep 02, 2024 at 02:24:53AM +0000, David Lin wrote: > > > > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > > > Sent: Monday, August 26, 2024 3:27 PM > > > > > > To: Francesco Dolcini <francesco@dolcini.it> > > > > > > Cc: Calvin Owens <calvin@wbinvd.org>; Brian Norris > > > > > > <briannorris@chromium.org>; Kalle Valo <kvalo@kernel.org>; David > > > > > > Lin <yu-hao.lin@nxp.com>; linux-wireless@vger.kernel.org; > > > > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de; Sascha > > > > > > Hauer <s.hauer@pengutronix.de> > > > > > > Subject: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > > > > > > > > > Caution: This is an external email. Please take care when > > > > > > clicking links or opening attachments. When in doubt, report the > > > > > > message using the 'Report this email' button > > > > > > > > > > > > > > > > > > This series adds support for the iw61x chips to the mwifiex driver. > > > > > > There are a few things to address, hence the RFC status. See the > > > > > > commit messages for details. The series is based on > > wireless-next/main. > > > > > > > > > > > > I am sending this now since people requested it here [1], but as > > > > > > it's out now feel free to leave your comments to the issues > > > > > > mentioned (or others I haven't mentioned ;) > > > > > > > > > > > > [1] > > > > > > https://lo/ > > > > > > > > > > > > re.kern%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C6125c51da3704fe10 > > > > a5 > > > > > > > > > > > > a08dccb1a24ef%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6386 > > > > 08560 > > > > > > > > > > > > 383160951%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > > > > 2luMz > > > > > > > > > > > > IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=jfQ6FQimPpwr > > > > nwUo > > > > > > OCEhmpSadtrb15ymGiif%2B1UCdG0%3D&reserved=0 > > > > > > > > > > > > el.org%2Fall%2F20240809094533.1660-1-yu-hao.lin%40nxp.com%2F&data=05 > > > > > > > > > > > > %7C02%7Cyu-hao.lin%40nxp.com%7C184ab4fed58647150f8508dcc5a0789a%7 > > > > > > > > > > > > C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638602540229716119% > > > > > > > > > > > > 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB > > > > > > > > > > > > TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=cACBHxaQvcOqu6ri > > > > > > BoAlZDONRlGQ4j5DcglEV9T%2BpYU%3D&reserved=0 > > > > > > > > > > > > Sascha > > > > > > > > > > > > > > > > > > Sascha Hauer (4): > > > > > > wifi: mwifiex: release firmware at remove time > > > > > > wifi: mwifiex: handle VDLL > > > > > > wifi: mwifiex: wait longer for SDIO card status > > > > > > mwifiex: add iw61x support > > > > > > > > > > > > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 86 > > > > +++++++++++++++++++ > > > > > > drivers/net/wireless/marvell/mwifiex/fw.h | 16 ++++ > > > > > > drivers/net/wireless/marvell/mwifiex/main.c | 9 +- > > > > > > drivers/net/wireless/marvell/mwifiex/main.h | 4 + > > > > > > drivers/net/wireless/marvell/mwifiex/sdio.c | 81 > > ++++++++++++++++- > > > > > > drivers/net/wireless/marvell/mwifiex/sdio.h | 3 + > > > > > > .../net/wireless/marvell/mwifiex/sta_event.c | 4 > > > > > > + .../net/wireless/marvell/mwifiex/uap_event.c | 4 + > > > > > > include/linux/mmc/sdio_ids.h | 3 + > > > > > > 9 files changed, 205 insertions(+), 5 deletions(-) > > > > > > > > > > > > -- > > > > > > 2.39.2 > > > > > > > > > > I think you ported VDLL related code from nxpwifi and you also > > > > > traced our private/downstream MXM driver. > > > > > > > > I ported it from this repository: > > > > > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > > > > > > thub.co%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C9f55bf357ece47645 > > 0b > > > > > > 708dccb234dae%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6386 > > 08599 > > > > > > 700151625%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > > 2luMz > > > > > > IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=OLf3OfFX7R% > > 2BT3V > > > > gWtcsqlls%2FI3ceY8r3bewwYy8bAes%3D&reserved=0 > > > > > > m%2Fnxp-imx%2Fmwifiex-iw612.git&data=05%7C02%7Cyu-hao.lin%40nxp.co > > > > > > m%7C6125c51da3704fe10a5a08dccb1a24ef%7C686ea1d3bc2b4c6fa92cd99c5 > > > > > > c301635%7C0%7C0%7C638608560383172495%7CUnknown%7CTWFpbGZsb3d > > > > > > 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3 > > > > > > D%7C0%7C%7C%7C&sdata=5TgI0r4u2I9Pi1FATJx32Ubn7ufmbYsBR1XkpQLAIyQ > > > > %3D&reserved=0 > > > > > > > > Is that the one you are referring to as MXM driver? > > > > > > > > > > Yes. > > > > > > > > If this is the case, I think you should know nxpwifi already > > > > > cleaned up the porting VDLL code from MXM driver. > > > > > I check your patch quickly. You ported the new SDIO data type > > > > > (MWIFIEX_TYPE_VDLL) from nxpwifi, but you did not port the code to > > > > > support this new data type from nxpwifi. In other word, you did > > > > > not test your patch before submission (same as some of your patches). > > > > > > > > I did test it. It works with the iw61x as well as older chips. There > > > > are likely details I haven't tested, but it generally works. If > > > > there are details I should test additionally please let me know. > > > > > > > > > > > > > > Another thing is that this new SDIO data type should be handled > > > > > carefully with other existed SDIO data type. > > > > > > > > > > Nxpwifi only supports new SDIO mode, so the modification to > > > > > support NXPWIFI_TYPE_VDLL can be clean and simple. If you want to > > > > > port the code to Mwifiex, there is no one-to-one modification of the > > code. > > > > > > > > > > Another important thing is that you should consider if your > > > > > modifications will affect existed devices or not. > > > > > You need to check if you should check firmware version or chip > > > > > type before adding some code. > > > > > > > > The VDLL code I added for the iw61x only reacts to the > > > > EVENT_VDLL_IND event. So as long as a firmware doesn't send such an > > > > event nothing is changed with this patch, and I haven't seen an older chip > > sending a VDLL event. > > > > > > > > > > How about IW61x? As I mentioned before, if you test IW61x on DFS > > > channel, command timeout will happen. Without correct VDLL porting, > > > you will encounter command timeout in some other test cases. But > > > testing on DFS channel will be easier to reproduce the issue. > > > > The VDLL support in the downstream driver supports a case when a VDLL > > event comes in while a command is being sent. I catched this with this > > test: > > > > if (adapter->cmd_sent) { > > mwifiex_dbg(adapter, MSG, "%s: adapter is busy\n", > > __func__); > > return -EBUSY; > > } > > > > The downstream driver defers handling of the VDLL event to the main process > > in this case. I haven't implemented this case in my patch because I wasn't able > > to trigger it, but is this the case you are referring to? > > > > Not only this code segment. In fact, you did not add VDLL data patch support to sdio.c. > If you try to add the code and do test, you will know what is missing in your code. Could you point me to the code you mean? Sascha
> From: Sascha Hauer <s.hauer@pengutronix.de> > Sent: Monday, September 2, 2024 9:11 PM > To: David Lin <yu-hao.lin@nxp.com> > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle Valo > <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > > > > > > > > > > > > Sascha > > > > > > > > > > > > > > > > > > > > > Sascha Hauer (4): > > > > > > > wifi: mwifiex: release firmware at remove time > > > > > > > wifi: mwifiex: handle VDLL > > > > > > > wifi: mwifiex: wait longer for SDIO card status > > > > > > > mwifiex: add iw61x support > > > > > > > > > > > > > > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 86 > > > > > +++++++++++++++++++ > > > > > > > drivers/net/wireless/marvell/mwifiex/fw.h | 16 ++++ > > > > > > > drivers/net/wireless/marvell/mwifiex/main.c | 9 +- > > > > > > > drivers/net/wireless/marvell/mwifiex/main.h | 4 + > > > > > > > drivers/net/wireless/marvell/mwifiex/sdio.c | 81 > > > ++++++++++++++++- > > > > > > > drivers/net/wireless/marvell/mwifiex/sdio.h | 3 + > > > > > > > .../net/wireless/marvell/mwifiex/sta_event.c | 4 > > > > > > > + .../net/wireless/marvell/mwifiex/uap_event.c | 4 + > > > > > > > include/linux/mmc/sdio_ids.h | 3 + > > > > > > > 9 files changed, 205 insertions(+), 5 deletions(-) > > > > > > > > > > > > > > -- > > > The VDLL support in the downstream driver supports a case when a > > > VDLL event comes in while a command is being sent. I catched this > > > with this > > > test: > > > > > > if (adapter->cmd_sent) { > > > mwifiex_dbg(adapter, MSG, "%s: adapter is busy\n", > > > __func__); > > > return -EBUSY; > > > } > > > > > > The downstream driver defers handling of the VDLL event to the main > > > process in this case. I haven't implemented this case in my patch > > > because I wasn't able to trigger it, but is this the case you are referring to? > > > > > > > Not only this code segment. In fact, you did not add VDLL data patch support > to sdio.c. > > If you try to add the code and do test, you will know what is missing in your > code. > > Could you point me to the code you mean? > > Sascha > I only know the porting VDLL code in nxpwifi. David
On Tue, Sep 03, 2024 at 01:51:46AM +0000, David Lin wrote: > > From: Sascha Hauer <s.hauer@pengutronix.de> > > Sent: Monday, September 2, 2024 9:11 PM > > To: David Lin <yu-hao.lin@nxp.com> > > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle Valo > > <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > > linux-kernel@vger.kernel.org; kernel@pengutronix.de > > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > > > > > > > > > > > > > > > Sascha > > > > > > > > > > > > > > > > > > > > > > > > Sascha Hauer (4): > > > > > > > > wifi: mwifiex: release firmware at remove time > > > > > > > > wifi: mwifiex: handle VDLL > > > > > > > > wifi: mwifiex: wait longer for SDIO card status > > > > > > > > mwifiex: add iw61x support > > > > > > > > > > > > > > > > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 86 > > > > > > +++++++++++++++++++ > > > > > > > > drivers/net/wireless/marvell/mwifiex/fw.h | 16 ++++ > > > > > > > > drivers/net/wireless/marvell/mwifiex/main.c | 9 +- > > > > > > > > drivers/net/wireless/marvell/mwifiex/main.h | 4 + > > > > > > > > drivers/net/wireless/marvell/mwifiex/sdio.c | 81 > > > > ++++++++++++++++- > > > > > > > > drivers/net/wireless/marvell/mwifiex/sdio.h | 3 + > > > > > > > > .../net/wireless/marvell/mwifiex/sta_event.c | 4 > > > > > > > > + .../net/wireless/marvell/mwifiex/uap_event.c | 4 + > > > > > > > > include/linux/mmc/sdio_ids.h | 3 + > > > > > > > > 9 files changed, 205 insertions(+), 5 deletions(-) > > > > > > > > > > > > > > > > -- > > > > The VDLL support in the downstream driver supports a case when a > > > > VDLL event comes in while a command is being sent. I catched this > > > > with this > > > > test: > > > > > > > > if (adapter->cmd_sent) { > > > > mwifiex_dbg(adapter, MSG, "%s: adapter is busy\n", > > > > __func__); > > > > return -EBUSY; > > > > } > > > > > > > > The downstream driver defers handling of the VDLL event to the main > > > > process in this case. I haven't implemented this case in my patch > > > > because I wasn't able to trigger it, but is this the case you are referring to? > > > > > > > > > > Not only this code segment. In fact, you did not add VDLL data patch support > > to sdio.c. > > > If you try to add the code and do test, you will know what is missing in your > > code. > > > > Could you point me to the code you mean? > > > > Sascha > > > > I only know the porting VDLL code in nxpwifi. Yes, and I asked for a pointer to that code, some function name, or file/line or whatever, because I looked at the nxpwifi driver and don't know what you mean with "VDLL data patch support" in sdio.c. Sascha
> From: Sascha Hauer <s.hauer@pengutronix.de> > Sent: Tuesday, September 3, 2024 2:33 PM > To: David Lin <yu-hao.lin@nxp.com> > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle Valo > <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > On Tue, Sep 03, 2024 at 01:51:46AM +0000, David Lin wrote: > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > Sent: Monday, September 2, 2024 9:11 PM > > > To: David Lin <yu-hao.lin@nxp.com> > > > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > > > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle > > > Valo <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de > > > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > > > > > > > > > > > > > > > > > > Sascha > > > > > > > > > > > > > > > > > > > > > > > > > > > Sascha Hauer (4): > > > > > > > > > wifi: mwifiex: release firmware at remove time > > > > > > > > > wifi: mwifiex: handle VDLL > > > > > > > > > wifi: mwifiex: wait longer for SDIO card status > > > > > > > > > mwifiex: add iw61x support > > > > > > > > > > > > > > > > > > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 86 > > > > > > > +++++++++++++++++++ > > > > > > > > > drivers/net/wireless/marvell/mwifiex/fw.h | 16 ++++ > > > > > > > > > drivers/net/wireless/marvell/mwifiex/main.c | 9 +- > > > > > > > > > drivers/net/wireless/marvell/mwifiex/main.h | 4 + > > > > > > > > > drivers/net/wireless/marvell/mwifiex/sdio.c | 81 > > > > > ++++++++++++++++- > > > > > > > > > drivers/net/wireless/marvell/mwifiex/sdio.h | 3 + > > > > > > > > > .../net/wireless/marvell/mwifiex/sta_event.c | 4 > > > > > > > > > + .../net/wireless/marvell/mwifiex/uap_event.c | 4 + > > > > > > > > > include/linux/mmc/sdio_ids.h | 3 + > > > > > > > > > 9 files changed, 205 insertions(+), 5 deletions(-) > > > > > > > > > > > > > > > > > > -- > > > > > The VDLL support in the downstream driver supports a case when a > > > > > VDLL event comes in while a command is being sent. I catched > > > > > this with this > > > > > test: > > > > > > > > > > if (adapter->cmd_sent) { > > > > > mwifiex_dbg(adapter, MSG, "%s: adapter is > > > > > busy\n", __func__); > > > > > return -EBUSY; > > > > > } > > > > > > > > > > The downstream driver defers handling of the VDLL event to the > > > > > main process in this case. I haven't implemented this case in my > > > > > patch because I wasn't able to trigger it, but is this the case you are > referring to? > > > > > > > > > > > > > Not only this code segment. In fact, you did not add VDLL data > > > > patch support > > > to sdio.c. > > > > If you try to add the code and do test, you will know what is > > > > missing in your > > > code. > > > > > > Could you point me to the code you mean? > > > > > > Sascha > > > > > > > I only know the porting VDLL code in nxpwifi. > > Yes, and I asked for a pointer to that code, some function name, or file/line or > whatever, because I looked at the nxpwifi driver and don't know what you > mean with "VDLL data patch support" in sdio.c. > > Sascha > It is better for you to check MXM driver. It is the same as Mwifiex which support all SDIO modes.
On Tue, Sep 03, 2024 at 06:39:15AM +0000, David Lin wrote: > > > > > Not only this code segment. In fact, you did not add VDLL data > > > > > patch support > > > > to sdio.c. > > > > > If you try to add the code and do test, you will know what is > > > > > missing in your > > > > code. > > > > > > > > Could you point me to the code you mean? > > > > > > > > Sascha > > > > > > > > > > I only know the porting VDLL code in nxpwifi. > > > > Yes, and I asked for a pointer to that code, some function name, or file/line or > > whatever, because I looked at the nxpwifi driver and don't know what you > > mean with "VDLL data patch support" in sdio.c. > > > > Sascha > > > > It is better for you to check MXM driver. It is the same as Mwifiex which support all SDIO modes. Now I am confused. You said: > In fact, you did not add VDLL data patch support to sdio.c I was under the assumption that the nxpwifi driver that you specifically posted for the iw61x chipset should contain this code. Isn't that the case? BTW did you really mean "VDLL data patch" or did you mean "VDLL data path"? Sascha
> From: Sascha Hauer <s.hauer@pengutronix.de> > Sent: Tuesday, September 3, 2024 3:30 PM > To: David Lin <yu-hao.lin@nxp.com> > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle Valo > <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > On Tue, Sep 03, 2024 at 06:39:15AM +0000, David Lin wrote: > > > > > > Not only this code segment. In fact, you did not add VDLL data > > > > > > patch support > > > > > to sdio.c. > > > > > > If you try to add the code and do test, you will know what is > > > > > > missing in your > > > > > code. > > > > > > > > > > Could you point me to the code you mean? > > > > > > > > > > Sascha > > > > > > > > > > > > > I only know the porting VDLL code in nxpwifi. > > > > > > Yes, and I asked for a pointer to that code, some function name, or > > > file/line or whatever, because I looked at the nxpwifi driver and > > > don't know what you mean with "VDLL data patch support" in sdio.c. > > > > > > Sascha > > > > > > > It is better for you to check MXM driver. It is the same as Mwifiex which > support all SDIO modes. > > Now I am confused. You said: > > > In fact, you did not add VDLL data patch support to sdio.c > > I was under the assumption that the nxpwifi driver that you specifically posted > for the iw61x chipset should contain this code. Isn't that the case? > > BTW did you really mean "VDLL data patch" or did you mean "VDLL data > path"? > Sorry VDLL data path. You did not add the code to support this new SDIO data type in your patch. Please check MXM driver which supports all SDIO modes. David
On Tue, Sep 03, 2024 at 07:35:59AM +0000, David Lin wrote: > > From: Sascha Hauer <s.hauer@pengutronix.de> > > Sent: Tuesday, September 3, 2024 3:30 PM > > To: David Lin <yu-hao.lin@nxp.com> > > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle Valo > > <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > > linux-kernel@vger.kernel.org; kernel@pengutronix.de > > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > Caution: This is an external email. Please take care when clicking links or > > opening attachments. When in doubt, report the message using the 'Report > > this email' button > > > > > > On Tue, Sep 03, 2024 at 06:39:15AM +0000, David Lin wrote: > > > > > > > Not only this code segment. In fact, you did not add VDLL data > > > > > > > patch support > > > > > > to sdio.c. > > > > > > > If you try to add the code and do test, you will know what is > > > > > > > missing in your > > > > > > code. > > > > > > > > > > > > Could you point me to the code you mean? > > > > > > > > > > > > Sascha > > > > > > > > > > > > > > > > I only know the porting VDLL code in nxpwifi. > > > > > > > > Yes, and I asked for a pointer to that code, some function name, or > > > > file/line or whatever, because I looked at the nxpwifi driver and > > > > don't know what you mean with "VDLL data patch support" in sdio.c. > > > > > > > > Sascha > > > > > > > > > > It is better for you to check MXM driver. It is the same as Mwifiex which > > support all SDIO modes. > > > > Now I am confused. You said: > > > > > In fact, you did not add VDLL data patch support to sdio.c > > > > I was under the assumption that the nxpwifi driver that you specifically posted > > for the iw61x chipset should contain this code. Isn't that the case? > > > > BTW did you really mean "VDLL data patch" or did you mean "VDLL data > > path"? > > > > Sorry VDLL data path. > You did not add the code to support this new SDIO data type in your patch. > > Please check MXM driver which supports all SDIO modes. But why? The nxpwifi driver is much closer to the mwifiex driver and much better readable, so I would rather pick the missing pieces from there. Sascha
> From: Sascha Hauer <s.hauer@pengutronix.de> > Sent: Tuesday, September 3, 2024 3:52 PM > To: David Lin <yu-hao.lin@nxp.com> > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle Valo > <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > On Tue, Sep 03, 2024 at 07:35:59AM +0000, David Lin wrote: > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > Sent: Tuesday, September 3, 2024 3:30 PM > > > To: David Lin <yu-hao.lin@nxp.com> > > > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > > > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle > > > Valo <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de > > > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > > > Caution: This is an external email. Please take care when clicking > > > links or opening attachments. When in doubt, report the message > > > using the 'Report this email' button > > > > > > > > > On Tue, Sep 03, 2024 at 06:39:15AM +0000, David Lin wrote: > > > > > > > > Not only this code segment. In fact, you did not add VDLL > > > > > > > > data patch support > > > > > > > to sdio.c. > > > > > > > > If you try to add the code and do test, you will know what > > > > > > > > is missing in your > > > > > > > code. > > > > > > > > > > > > > > Could you point me to the code you mean? > > > > > > > > > > > > > > Sascha > > > > > > > > > > > > > > > > > > > I only know the porting VDLL code in nxpwifi. > > > > > > > > > > Yes, and I asked for a pointer to that code, some function name, > > > > > or file/line or whatever, because I looked at the nxpwifi driver > > > > > and don't know what you mean with "VDLL data patch support" in > sdio.c. > > > > > > > > > > Sascha > > > > > > > > > > > > > It is better for you to check MXM driver. It is the same as > > > > Mwifiex which > > > support all SDIO modes. > > > > > > Now I am confused. You said: > > > > > > > In fact, you did not add VDLL data patch support to sdio.c > > > > > > I was under the assumption that the nxpwifi driver that you > > > specifically posted for the iw61x chipset should contain this code. Isn't that > the case? > > > > > > BTW did you really mean "VDLL data patch" or did you mean "VDLL data > > > path"? > > > > > > > Sorry VDLL data path. > > You did not add the code to support this new SDIO data type in your patch. > > > > Please check MXM driver which supports all SDIO modes. > > But why? The nxpwifi driver is much closer to the mwifiex driver and much > better readable, so I would rather pick the missing pieces from there. > > Sascha > Nxpwifi only supports SDIO new mode. MXM and Mwifiex supports normal and new SDIO modes. David
On Tue, Sep 03, 2024 at 07:57:51AM +0000, David Lin wrote: > > From: Sascha Hauer <s.hauer@pengutronix.de> > > Sent: Tuesday, September 3, 2024 3:52 PM > > To: David Lin <yu-hao.lin@nxp.com> > > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle Valo > > <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > > linux-kernel@vger.kernel.org; kernel@pengutronix.de > > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > Caution: This is an external email. Please take care when clicking links or > > opening attachments. When in doubt, report the message using the 'Report > > this email' button > > > > > > On Tue, Sep 03, 2024 at 07:35:59AM +0000, David Lin wrote: > > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > Sent: Tuesday, September 3, 2024 3:30 PM > > > > To: David Lin <yu-hao.lin@nxp.com> > > > > Cc: Francesco Dolcini <francesco@dolcini.it>; Calvin Owens > > > > <calvin@wbinvd.org>; Brian Norris <briannorris@chromium.org>; Kalle > > > > Valo <kvalo@kernel.org>; linux-wireless@vger.kernel.org; > > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de > > > > Subject: Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x > > > > > > > > Caution: This is an external email. Please take care when clicking > > > > links or opening attachments. When in doubt, report the message > > > > using the 'Report this email' button > > > > > > > > > > > > On Tue, Sep 03, 2024 at 06:39:15AM +0000, David Lin wrote: > > > > > > > > > Not only this code segment. In fact, you did not add VDLL > > > > > > > > > data patch support > > > > > > > > to sdio.c. > > > > > > > > > If you try to add the code and do test, you will know what > > > > > > > > > is missing in your > > > > > > > > code. > > > > > > > > > > > > > > > > Could you point me to the code you mean? > > > > > > > > > > > > > > > > Sascha > > > > > > > > > > > > > > > > > > > > > > I only know the porting VDLL code in nxpwifi. > > > > > > > > > > > > Yes, and I asked for a pointer to that code, some function name, > > > > > > or file/line or whatever, because I looked at the nxpwifi driver > > > > > > and don't know what you mean with "VDLL data patch support" in > > sdio.c. > > > > > > > > > > > > Sascha > > > > > > > > > > > > > > > > It is better for you to check MXM driver. It is the same as > > > > > Mwifiex which > > > > support all SDIO modes. > > > > > > > > Now I am confused. You said: > > > > > > > > > In fact, you did not add VDLL data patch support to sdio.c > > > > > > > > I was under the assumption that the nxpwifi driver that you > > > > specifically posted for the iw61x chipset should contain this code. Isn't that > > the case? > > > > > > > > BTW did you really mean "VDLL data patch" or did you mean "VDLL data > > > > path"? > > > > > > > > > > Sorry VDLL data path. > > > You did not add the code to support this new SDIO data type in your patch. > > > > > > Please check MXM driver which supports all SDIO modes. > > > > But why? The nxpwifi driver is much closer to the mwifiex driver and much > > better readable, so I would rather pick the missing pieces from there. > > > > Sascha > > > > Nxpwifi only supports SDIO new mode. MXM and Mwifiex supports normal and new SDIO modes. By SDIO new mode I assume that you mean the supports_sdio_new_mode variable in the mwifiex driver. This is used when a chip supports it The iw61x supports this new mode and the iw61x also is the only chip which needs the VDLL code. So would I have to add something to the "normal" SDIO mode? Sascha