Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Tue, 12 Apr 2022 14:13:49 +0300 (EEST)
From:   =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>
To:     Ricardo Martinez <ricardo.martinez@linux.intel.com>
cc:     Netdev <netdev@vger.kernel.org>, linux-wireless@vger.kernel.org,
        kuba@kernel.org, davem@davemloft.net, johannes@sipsolutions.net,
        ryazanov.s.a@gmail.com, loic.poulain@linaro.org,
        m.chetan.kumar@intel.com, chandrashekar.devegowda@intel.com,
        linuxwwan@intel.com, chiranjeevi.rapolu@linux.intel.com,
        haijun.liu@mediatek.com, amir.hanania@intel.com,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        dinesh.sharma@intel.com, eliot.lee@intel.com,
        moises.veleta@intel.com, pierre-louis.bossart@intel.com,
        muralidharan.sethuraman@intel.com, Soumya.Prakash.Mishra@intel.com,
        sreehari.kancharla@intel.com, madhusmita.sahu@intel.com
Subject: Re: [PATCH net-next v6 04/13] net: wwan: t7xx: Add port proxy
 infrastructure
In-Reply-To: <20220407223629.21487-5-ricardo.martinez@linux.intel.com>
Message-ID: <d55b3f46-8a56-152-c934-8dae7e245a8f@linux.intel.com>
References: <20220407223629.21487-1-ricardo.martinez@linux.intel.com> <20220407223629.21487-5-ricardo.martinez@linux.intel.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323329-741371815-1649762041=:1546"
Precedence: bulk

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323329-741371815-1649762041=:1546
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 8BIT

On Thu, 7 Apr 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>

This too seems fine. A few nits below.

Reviewed-by: Ilpo J?rvinen <ilpo.jarvinen@linux.intel.com>


> +/* Reused for net TX, Data queue, same bit as RX_FULLED */
> +#define PORT_F_TX_DATA_FULLED	BIT(1)
> +#define PORT_F_TX_ACK_FULLED	BIT(8)

RX_FULLED is gone.

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

I'd add this:
        u32 status = le32_to_cpu(ccci_h->status);

> +	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);

No need for !! as assert_bit is boolean.

> +static int t7xx_proxy_alloc(struct t7xx_modem *md)
> +{
> +	unsigned int port_number = ARRAY_SIZE(t7xx_md_port_conf);

num_ports, port_count or something along those lines.

You might want to do the same rename for the port_prox->port_number too.


-- 
 i.

--8323329-741371815-1649762041=:1546--