| Message ID | 20250318050739.2239376-3-jeff.chen_1@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | wifi: mwifiex: Fix RF calibration data handling issues | expand |
Hello Jeff, On Tue, Mar 18, 2025 at 01:07:39PM +0800, Jeff Chen wrote: > While this patch restores the ability to download RF calibration data > from a file, it may inadvertently break the feature to download > calibration data from the device tree. I do not think this is acceptable. Fixing something by adding another bug is not ok. You should fix the calibration data from file, without breaking the device tree calibration functionality. Francesco
On Tue, Mar 18, 2025 at 01:07:39PM +0800, Jeff Chen wrote: > This patch resolves an issue where RF calibration data from a > file could not be downloaded to the firmware. The feature to > download calibration data from a file was broken by the commit: > d39fbc88956e. > > The issue arose because the function `mwifiex_cmd_cfg_data()` > was modified in a way that prevented proper handling of > file-based calibration data. While this patch restores the ability > to download RF calibration data from a file, it may inadvertently > break the feature to download calibration data from the device > tree. This is because the function `mwifiex_dnld_dt_cfgdata()`, > which also relies on `mwifiex_cmd_cfg_data()`, is still used for > device tree-based calibration data downloads. > > Fixes: d39fbc88956e ("mwifiex: remove cfg_data construction") > Signed-off-by: Jeff Chen <jeff.chen_1@nxp.com> > --- > drivers/net/wireless/marvell/mwifiex/fw.h | 14 ++++++++++++++ > drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 11 +++++++++-- > 2 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h > index 4a96281792cc..91458f3bd14a 100644 > --- a/drivers/net/wireless/marvell/mwifiex/fw.h > +++ b/drivers/net/wireless/marvell/mwifiex/fw.h > @@ -454,6 +454,11 @@ enum mwifiex_channel_flags { > #define HostCmd_RET_BIT 0x8000 > #define HostCmd_ACT_GEN_GET 0x0000 > #define HostCmd_ACT_GEN_SET 0x0001 > +#define HOST_CMD_ACT_GEN_SET 0x0001 > +/* Add this non-CamelCase-style macro to comply with checkpatch requirements. > + * This macro will eventually replace all existing CamelCase-style macros in > + * the future for consistency. > + */ Just ignore this checkpatch warning. We don't want to have duplicated defines just for silencing checkpatch. If anything we could change all the CamelCase defines throughout the driver in one go. Sascha
> Hello Jeff, > > On Tue, Mar 18, 2025 at 01:07:39PM +0800, Jeff Chen wrote: > > While this patch restores the ability to download RF calibration data > > from a file, it may inadvertently break the feature to download > > calibration data from the device tree. > > I do not think this is acceptable. Fixing something by adding another bug is not > ok. You should fix the calibration data from file, without breaking the device > tree calibration functionality. > > Francesco Hello Francesco, There seems to be a misunderstanding. The issue with breaking the device tree calibration functionality was actually introduced in version 3 of the patch. Version 4 resolves this problem by ensuring that both file-based and device tree-based calibration work as intended. The current patch restores the ability to download RF calibration data from a file without affecting the device tree functionality. I'll clarify this in the commit message to avoid further confusion. Best regards, Jeff
> -----Original Message----- > From: Sascha Hauer <s.hauer@pengutronix.de> > Sent: Monday, March 24, 2025 3:02 PM > To: Jeff Chen <jeff.chen_1@nxp.com> > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; > briannorris@chromium.org; johannes@sipsolutions.net; francesco@dolcini.it; > Pete Hsieh <tsung-hsien.hsieh@nxp.com> > Subject: [EXT] Re: [PATCH v4 2/2] wifi: mwifiex: Fix RF calibration data > download from file > > > @@ -454,6 +454,11 @@ enum mwifiex_channel_flags { > > #define HostCmd_RET_BIT 0x8000 > > #define HostCmd_ACT_GEN_GET 0x0000 > > #define HostCmd_ACT_GEN_SET 0x0001 > > +#define HOST_CMD_ACT_GEN_SET 0x0001 > > +/* Add this non-CamelCase-style macro to comply with checkpatch > requirements. > > + * This macro will eventually replace all existing CamelCase-style > > +macros in > > + * the future for consistency. > > + */ > > Just ignore this checkpatch warning. We don't want to have duplicated defines > just for silencing checkpatch. If anything we could change all the CamelCase > defines throughout the driver in one go. > > Sascha > Hello Sascha, I'll proceed with reverting the duplicated defines and ignoring the checkpatch warning for now. Thank you for pointing this out. Best regards, Jeff
diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h index 4a96281792cc..91458f3bd14a 100644 --- a/drivers/net/wireless/marvell/mwifiex/fw.h +++ b/drivers/net/wireless/marvell/mwifiex/fw.h @@ -454,6 +454,11 @@ enum mwifiex_channel_flags { #define HostCmd_RET_BIT 0x8000 #define HostCmd_ACT_GEN_GET 0x0000 #define HostCmd_ACT_GEN_SET 0x0001 +#define HOST_CMD_ACT_GEN_SET 0x0001 +/* Add this non-CamelCase-style macro to comply with checkpatch requirements. + * This macro will eventually replace all existing CamelCase-style macros in + * the future for consistency. + */ #define HostCmd_ACT_GEN_REMOVE 0x0004 #define HostCmd_ACT_BITWISE_SET 0x0002 #define HostCmd_ACT_BITWISE_CLR 0x0003 @@ -2352,6 +2357,14 @@ struct host_cmd_ds_add_station { u8 tlv[]; } __packed; +#define MWIFIEX_CFG_TYPE_CAL 0x2 + +struct host_cmd_ds_802_11_cfg_data { + __le16 action; + __le16 type; + __le16 data_len; +} __packed; + struct host_cmd_ds_command { __le16 command; __le16 size; @@ -2431,6 +2444,7 @@ struct host_cmd_ds_command { struct host_cmd_ds_pkt_aggr_ctrl pkt_aggr_ctrl; struct host_cmd_ds_sta_configure sta_cfg; struct host_cmd_ds_add_station sta_info; + struct host_cmd_ds_802_11_cfg_data cfg_data; } params; } __packed; diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c index c0e6ce1a82fe..52678e213050 100644 --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c @@ -1507,6 +1507,7 @@ static int mwifiex_cmd_cfg_data(struct mwifiex_private *priv, u32 len; u8 *data = (u8 *)cmd + S_DS_GEN; int ret; + struct host_cmd_ds_802_11_cfg_data *pcfg_data; if (prop) { len = prop->length; @@ -1514,12 +1515,19 @@ static int mwifiex_cmd_cfg_data(struct mwifiex_private *priv, data, len); if (ret) return ret; + + cmd->size = cpu_to_le16(S_DS_GEN + len); mwifiex_dbg(adapter, INFO, "download cfg_data from device tree: %s\n", prop->name); } else if (adapter->cal_data->data && adapter->cal_data->size > 0) { len = mwifiex_parse_cal_cfg((u8 *)adapter->cal_data->data, - adapter->cal_data->size, data); + adapter->cal_data->size, data + sizeof(*pcfg_data)); + pcfg_data = &cmd->params.cfg_data; + pcfg_data->action = cpu_to_le16(HOST_CMD_ACT_GEN_SET); + pcfg_data->type = cpu_to_le16(MWIFIEX_CFG_TYPE_CAL); + pcfg_data->data_len = cpu_to_le16(len); + cmd->size = cpu_to_le16(S_DS_GEN + sizeof(*pcfg_data) + len); mwifiex_dbg(adapter, INFO, "download cfg_data from config file\n"); } else { @@ -1527,7 +1535,6 @@ static int mwifiex_cmd_cfg_data(struct mwifiex_private *priv, } cmd->command = cpu_to_le16(HostCmd_CMD_CFG_DATA); - cmd->size = cpu_to_le16(S_DS_GEN + len); return 0; }
This patch resolves an issue where RF calibration data from a file could not be downloaded to the firmware. The feature to download calibration data from a file was broken by the commit: d39fbc88956e. The issue arose because the function `mwifiex_cmd_cfg_data()` was modified in a way that prevented proper handling of file-based calibration data. While this patch restores the ability to download RF calibration data from a file, it may inadvertently break the feature to download calibration data from the device tree. This is because the function `mwifiex_dnld_dt_cfgdata()`, which also relies on `mwifiex_cmd_cfg_data()`, is still used for device tree-based calibration data downloads. Fixes: d39fbc88956e ("mwifiex: remove cfg_data construction") Signed-off-by: Jeff Chen <jeff.chen_1@nxp.com> --- drivers/net/wireless/marvell/mwifiex/fw.h | 14 ++++++++++++++ drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 11 +++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-)