Re: [PATCH net-next v4 05/13] net: wwan: t7xx: Add control port
flat view
From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Date: 2022-01-27 10:40:57
Also in:
linux-wireless
On Thu, 13 Jan 2022, Ricardo Martinez wrote:
From: Haijun Liu <haijun.liu@mediatek.com> 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. Signed-off-by: Haijun Liu <haijun.liu@mediatek.com> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com> Co-developed-by: Ricardo Martinez <ricardo.martinez@linux.intel.com> Signed-off-by: Ricardo Martinez <ricardo.martinez@linux.intel.com> ---
quoted hunk ↗ jump to hunk
+ /* Fill runtime feature */ + for (i = 0; i < FEATURE_COUNT; i++) { + u8 md_feature_mask = FIELD_GET(FEATURE_MSK, md_feature->feature_set[i]); + + memset(&rt_feature, 0, sizeof(rt_feature)); + rt_feature.feature_id = i; + + switch (md_feature_mask) { + case MTK_FEATURE_DOES_NOT_EXIST: + case MTK_FEATURE_MUST_BE_SUPPORTED: + rt_feature.support_info = md_feature->feature_set[i]; + break; + + default: + break;
Please remove empty default blocks from all patches.
quoted hunk ↗ jump to hunk
+ } + + if (FIELD_GET(FEATURE_MSK, rt_feature.support_info) != + MTK_FEATURE_MUST_BE_SUPPORTED) { + memcpy(rt_data, &rt_feature, sizeof(rt_feature)); + rt_data += sizeof(rt_feature); + } + + packet_size += sizeof(struct mtk_runtime_feature); + }
Is it intentional these two additions (rt_data and packet_size) are on different sides of the if block?
quoted hunk ↗ jump to hunk
+static int port_ctl_init(struct t7xx_port *port) +{ + struct t7xx_port_static *port_static = port->port_static; + + port->skb_handler = &control_msg_handler; + port->thread = kthread_run(port_ctl_rx_thread, port, "%s", port_static->name); + if (IS_ERR(port->thread)) { + dev_err(port->dev, "Failed to start port control thread\n"); + return PTR_ERR(port->thread); + } + + port->rx_length_th = CTRL_QUEUE_MAXLEN; + return 0; +} + +static void port_ctl_uninit(struct t7xx_port *port) +{ + unsigned long flags; + struct sk_buff *skb; + + if (port->thread) + kthread_stop(port->thread); + + spin_lock_irqsave(&port->rx_wq.lock, flags); + while ((skb = __skb_dequeue(&port->rx_skb_list)) != NULL) + dev_kfree_skb_any(skb); + + spin_unlock_irqrestore(&port->rx_wq.lock, flags); +}
I wonder if the uninit should set rx_length_th to 0 to prevent further accumulation of skbs?
+ FSM_EVENT_AP_HS2_EXIT,
Never used anywhere. -- i.