lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
 <PAWPR04MB9910633CC9A1E7C98401C9C79CD52@PAWPR04MB9910.eurprd04.prod.outlook.com>
Date: Fri, 7 Mar 2025 16:40:49 +0000
From: Jeff Chen <jeff.chen_1@....com>
To: Francesco Dolcini <francesco@...cini.it>
CC: "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"briannorris@...omium.org" <briannorris@...omium.org>,
	"johannes@...solutions.net" <johannes@...solutions.net>, Pete Hsieh
	<tsung-hsien.hsieh@....com>, "s.hauer@...gutronix.de"
	<s.hauer@...gutronix.de>
Subject: RE: [EXT] Re: [PATCH v3 1/2] wifi: mwifiex: Part A of resolving the
 failure in downloading calibration data.

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@...vell.com>
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@....com>
> > ---
> >  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
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ