Re: [PATCH v2 05/14] net: wwan: t7xx: Add control port
From: Sergey Ryazanov <ryazanov.s.a@gmail.com>
Date: 2021-11-06 18:06:02
Also in:
netdev
On Mon, Nov 1, 2021 at 6:57 AM Ricardo Martinez [off-list ref] wrote:
Control Port implements driver control messages such as modem-host handshaking, controls port enumeration, and handles exception messages. The handshaking process between the driver and the modem happens during the init sequence. The process involves the exchange of a list of supported runtime features to make sure that modem and host are ready to provide proper feature lists including port enumeration. Further features can be enabled and controlled in this handshaking process.
[skipped]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/wwan/t7xx/t7xx_modem_ops.c b/drivers/net/wwan/t7xx/t7xx_modem_ops.c... +struct feature_query { + u32 head_pattern;
Looks like this field should be __le32 since it is sent to modem as is.
+ u8 feature_set[FEATURE_COUNT]; + u32 tail_pattern;
Ditto.
+};
+
+static void prepare_host_rt_data_query(struct core_sys_info *core)
+{
...
+ ft_query->head_pattern = MD_FEATURE_QUERY_ID;This should be a ft_query->head_pattern = cpu_to_le32(MD_FEATURE_QUERY_ID); to run on the big-endians CPU. strace will notify you about each endians mismatch as soon as you change head_pattern field type to __le32.
+ memcpy(ft_query->feature_set, core->feature_set, FEATURE_COUNT); + ft_query->tail_pattern = MD_FEATURE_QUERY_ID;
Ditto.
+ /* send HS1 message to device */ + port_proxy_send_skb(core->ctl_port, skb, 0); +}
[skipped]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c b/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c... +static void fsm_ee_message_handler(struct sk_buff *skb) +{ ... + ctrl_msg_h = (struct ctrl_msg_header *)skb->data; ... + switch (ctrl_msg_h->ctrl_msg_id) {
This should be:
switch (le32_to_cpu(ctrl_msg_h->ctrl_msg_id)) {
+ case CTL_ID_MD_EX:
+ if (ctrl_msg_h->reserved != MD_EX_CHK_ID) {Why is this field called 'reserved', but used to perform message validation?
...
+static void control_msg_handler(struct t7xx_port *port, struct sk_buff *skb)
+{
+ struct ctrl_msg_header *ctrl_msg_h;
...
+ ctrl_msg_h = (struct ctrl_msg_header *)skb->data;
..
+ switch (ctrl_msg_h->ctrl_msg_id) {
This should be something like this:
switch (le32_to_cpu(ctrl_msg_h->ctrl_msg_id)) {
--
Sergey