Re: [PATCH net-next v5 04/13] net: wwan: t7xx: Add port proxy infrastructure

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Wed, 23 Feb 2022, Ricardo Martinez wrote:

> From: Haijun Liu <haijun.liu@mediatek.com>
> 
> Port-proxy provides a common interface to interact with different types
> of ports. Ports export their configuration via `struct t7xx_port` and
> operate as defined by `struct port_ops`.
> 
> Signed-off-by: Haijun Liu <haijun.liu@mediatek.com>
> Co-developed-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.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>
> 
> >From a WWAN framework perspective:
> Reviewed-by: Loic Poulain <loic.poulain@linaro.org>
> ---


> +static u16 t7xx_port_next_rx_seq_num(struct t7xx_port *port, struct ccci_header *ccci_h)
> +{
> +	u16 seq_num, next_seq_num, assert_bit;

assert_bit could be bool.

> +
> +	seq_num = FIELD_GET(CCCI_H_SEQ_FLD, le32_to_cpu(ccci_h->status));
> +	next_seq_num = (seq_num + 1) & FIELD_MAX(CCCI_H_SEQ_FLD);
> +	assert_bit = !!(le32_to_cpu(ccci_h->status) & CCCI_H_AST_BIT);
> +	if (!assert_bit || port->seq_nums[MTK_RX] > FIELD_MAX(CCCI_H_SEQ_FLD))

Is this an obfuscated way to say:
	... || port->seq_nums[MTK_RX] == INVALID_SEQ_NUM
?

> +int t7xx_port_proxy_node_control(struct t7xx_modem *md, struct port_msg *port_msg)
> +{
> +	u32 *port_info_base = (void *)port_msg + sizeof(*port_msg);

__le32 *port_info_base = (void *)port_msg + sizeof(*port_msg);

As port_msg has __le32 fields, the endianess likely should disappear in 
this casting?

> +	struct device *dev = &md->t7xx_dev->pdev->dev;
> +	unsigned int version, ports, i;
> +
> +	version = FIELD_GET(PORT_MSG_VERSION, le32_to_cpu(port_msg->info));
> +	if (version != PORT_ENUM_VER ||
> +	    le32_to_cpu(port_msg->head_pattern) != PORT_ENUM_HEAD_PATTERN ||
> +	    le32_to_cpu(port_msg->tail_pattern) != PORT_ENUM_TAIL_PATTERN) {
> +		dev_err(dev, "Invalid port control message %x:%x:%x\n",
> +			version, le32_to_cpu(port_msg->head_pattern),
> +			le32_to_cpu(port_msg->tail_pattern));
> +		return -EFAULT;
> +	}
> +
> +	ports = FIELD_GET(PORT_MSG_PRT_CNT, le32_to_cpu(port_msg->info));
> +	for (i = 0; i < ports; i++) {
> +		struct t7xx_port_static *port_static;
> +		u32 *port_info = port_info_base + i;

__le32 *port_info = port_info_base + i;

> +		struct t7xx_port *port;
> +		unsigned int ch_id;
> +		bool en_flag;
> +
> +		ch_id = FIELD_GET(PORT_INFO_CH_ID, *port_info);

ch_id = FIELD_GET(PORT_INFO_CH_ID, le32_to_cpu(*port_info));

> +		port = t7xx_proxy_get_port_by_ch(md->port_prox, ch_id);
> +		if (!port) {
> +			dev_dbg(dev, "Port:%x not found\n", ch_id);
> +			continue;
> +		}
> +
> +		en_flag = !!(PORT_INFO_ENFLG & *port_info);

*port_info & PORT_INFO_ENFLG


-- 
 i.