Hi Francesco,
Thank you for reviewing and providing great comments.
>
> Hello Jeff,
>
> On Thu, Feb 20, 2025 at 02:11:42PM +0800, Jeff Chen wrote:
> > This patch corrects the command format used for downloading RF
> > calibration data to the firmware.
>
> Do we need any fixes tag? is this supposed to be backported to stable?
>
> Was the command format always broken? Do this format depends on the
> firmware version? We would need to explain why changing the format of this
> command here is safe.
>
Yes, we need a Fixes tag. It appears that the feature to download CAL from file was broken by the commit below.
commit d39fbc88956ef56a67b8030d53c7e8fa645a4e00
Author: Bing Zhao <bzhao@xxxxxxxxxxx>
Date: Fri Dec 13 18:33:00 2013 -0800
mwifiex: remove cfg_data construction
The cfg_data buffer will include the cfg_data structure header
(action, type, data_len). This makes it work for all data types
without extra parsing.
This patch enables the feature to download RF calibration data from a file. I don't think it needs to be backported to stable.
It doesn't depend on the firmware version.
However, I think it will break the feature to download CAL data from the device tree. See the code segment below.
'mwifiex_dnld_dt_cfgdata()' also uses 'mwifiex_cmd_cfg_data()' to prepare a command.
It would be better to add a new function to download CAL data from a file, instead of modifying the function `mwifiex_cmd_cfg_data()`
int mwifiex_sta_init_cmd(struct mwifiex_private *priv, u8 first_sta, bool init)
{
...
/* Download calibration data to firmware.
* The cal-data can be read from device tree and/or
* a configuration file and downloaded to firmware.
*/
if (adapter->dt_node) {
if (of_property_read_u32(adapter->dt_node,
"marvell,wakeup-pin",
&data) == 0) {
pr_debug("Wakeup pin = 0x%x\n", data);
adapter->hs_cfg.gpio = data;
}
mwifiex_dnld_dt_cfgdata(priv, adapter->dt_node,
"marvell,caldata");
}
...
}
> >
> > This patch is a split from the previous submission.
> >
> > Signed-off-by: Jeff Chen <jeff.chen_1@xxxxxxx>
> > ---
> > drivers/net/wireless/marvell/mwifiex/fw.h | 7 +++++++
> > drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 14 +++++++++-----
> > 2 files changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h
> > b/drivers/net/wireless/marvell/mwifiex/fw.h
> > index 4a96281792cc..0c75a574a7ee 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/fw.h
> > +++ b/drivers/net/wireless/marvell/mwifiex/fw.h
> > @@ -2352,6 +2352,12 @@ struct host_cmd_ds_add_station {
> > u8 tlv[];
> > } __packed;
> >
> > +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 +2437,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 e2800a831c8e..6e7b2b5c7dc5 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> > @@ -1500,18 +1500,19 @@ int mwifiex_dnld_dt_cfgdata(struct
> > mwifiex_private *priv,
> >
> > /* This function prepares command of set_cfg_data. */ static int
> > mwifiex_cmd_cfg_data(struct mwifiex_private *priv,
> > - struct host_cmd_ds_command *cmd,
> void *data_buf)
> > + struct host_cmd_ds_command *cmd,
> void
> > + *data_buf, u16 cmd_action)
> > {
> > struct mwifiex_adapter *adapter = priv->adapter;
> > struct property *prop = data_buf;
> > u32 len;
> > u8 *data = (u8 *)cmd + S_DS_GEN;
> > int ret;
> > + struct host_cmd_ds_802_11_cfg_data *pcfg_data =
> > + &cmd->params.cfg_data;
> >
> > if (prop) {
> > len = prop->length;
> > ret = of_property_read_u8_array(adapter->dt_node,
> prop->name,
> > - data, len);
> > + data +
> > + sizeof(*pcfg_data), len);
> > if (ret)
> > return ret;
> > mwifiex_dbg(adapter, INFO, @@ -1519,15 +1520,18 @@
> > static int mwifiex_cmd_cfg_data(struct mwifiex_private *priv,
> > 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));
> > mwifiex_dbg(adapter, INFO,
> > "download cfg_data from config file\n");
> > } else {
> > return -1;
> > }
> >
> > + pcfg_data->action = cpu_to_le16(cmd_action);
> > + pcfg_data->type = cpu_to_le16(2);
> > + pcfg_data->data_len = cpu_to_le16(len);
> > cmd->command = cpu_to_le16(HostCmd_CMD_CFG_DATA);
> > - cmd->size = cpu_to_le16(S_DS_GEN + len);
> > + cmd->size = cpu_to_le16(S_DS_GEN + sizeof(*pcfg_data) + len);
> >
> > return 0;
> > }
> > @@ -1949,7 +1953,7 @@ int mwifiex_sta_prepare_cmd(struct
> mwifiex_private *priv, uint16_t cmd_no,
> > ret = mwifiex_cmd_get_hw_spec(priv, cmd_ptr);
> > break;
> > case HostCmd_CMD_CFG_DATA:
> > - ret = mwifiex_cmd_cfg_data(priv, cmd_ptr, data_buf);
> > + ret = mwifiex_cmd_cfg_data(priv, cmd_ptr, data_buf,
> > + cmd_action);
> > break;
> > case HostCmd_CMD_MAC_CONTROL:
> > ret = mwifiex_cmd_mac_control(priv, cmd_ptr,
> cmd_action,
> > --
> > 2.34.1
> >
