Received-SPF: pass (google.com: domain of linux-kernel+bounces-94774-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Thu, 7 Mar 2024 01:19:52 +0100
From: Andrew Lunn <andrew@lunn.ch>
To: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, horms@kernel.org, saeedm@nvidia.com,
	anthony.l.nguyen@intel.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, corbet@lwn.net,
	linux-doc@vger.kernel.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, horatiu.vultur@microchip.com,
	ruanjinjie@huawei.com, steen.hegelund@microchip.com,
	vladimir.oltean@nxp.com, UNGLinuxDriver@microchip.com,
	Thorsten.Kummermehr@microchip.com, Pier.Beruto@onsemi.com,
	Selvamani.Rajagopal@onsemi.com, Nicolas.Ferre@microchip.com,
	benjamin.bigler@bernformulastudent.ch
Subject: Re: [PATCH net-next v3 03/12] net: ethernet: oa_tc6: implement
 register read operation
Message-ID: <48b65759-6e69-46ef-a2ed-857d04eadac8@lunn.ch>
References: <20240306085017.21731-1-Parthiban.Veerasooran@microchip.com>
 <20240306085017.21731-4-Parthiban.Veerasooran@microchip.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240306085017.21731-4-Parthiban.Veerasooran@microchip.com>

>  enum oa_tc6_register_op {
> +	OA_TC6_CTRL_REG_READ = 0,
>  	OA_TC6_CTRL_REG_WRITE = 1,
>  };

I thought it looked a little odd when the enum was added in the
previous patch with the first value of 1, and only one value. Now it
makes more sense.

The actual value appears to not matter? It is always 

> +	if (reg_op == OA_TC6_CTRL_REG_WRITE)

So i would drop the numbering, and leave it to the compiler. The
patches will then look less odd.

> +static int oa_tc6_check_ctrl_read_reply(struct oa_tc6 *tc6, u8 size)
> +{
> +	u32 *tx_buf = tc6->spi_ctrl_tx_buf;
> +	u32 *rx_buf = tc6->spi_ctrl_rx_buf + OA_TC6_CTRL_IGNORED_SIZE;
> +
> +	/* The echoed control read header must match with the one that was
> +	 * transmitted.
> +	 */
> +	if (*tx_buf != *rx_buf)
> +		return -ENODEV;

Another case where -EPROTO might be better?

	Andrew