Received-SPF: pass (google.com: domain of linux-kernel+bounces-14725-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Subject: Re: [PATCH net v3 1/1] net: ravb: Wait for operating mode to be
 applied
To: Claudiu <claudiu.beznea@tuxon.dev>, <davem@davemloft.net>,
	<edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>
CC: <mitsuhiro.kimura.kc@renesas.com>, <netdev@vger.kernel.org>,
	<linux-renesas-soc@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Claudiu
 Beznea <claudiu.beznea.uj@bp.renesas.com>
References: <20240102110116.4005187-1-claudiu.beznea.uj@bp.renesas.com>
 <20240102110116.4005187-2-claudiu.beznea.uj@bp.renesas.com>
From: Sergey Shtylyov <s.shtylyov@omp.ru>
Organization: Open Mobile Platform
Message-ID: <df73cd5f-4185-233d-eada-7b5598a070fc@omp.ru>
Date: Tue, 2 Jan 2024 21:43:14 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.1
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <20240102110116.4005187-2-claudiu.beznea.uj@bp.renesas.com>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
X-KSE-Attachment-Filter-Triggered-Filters: Clean
X-KSE-BulkMessagesFiltering-Scan-Result: InTheLimit

On 1/2/24 2:01 PM, Claudiu wrote:

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> CSR.OPS bits specify the current operating mode and (according to
> documentation) they are updated by HW when the operating mode change
> request is processed. To comply with this check CSR.OPS before proceeding.
> 
> Commit introduces ravb_set_opmode() that does all the necessities for
> setting the operating mode (set CCC.OPC (and CCC.GAC, CCC.CSEL, if any) and
> wait for CSR.OPS) and call it where needed. This should comply with all the
> HW manuals requirements as different manual variants specify that different
> modes need to be checked in CSR.OPS when setting CCC.OPC.
> 
> In case of platforms with GAC, if GAC needs to be enabled, the CCC.GAC and

   Better to spell it out, I think: in case of platforms that support gPTP
while in the config[uration] mode..

> CCC.CSEL needs to be configured along with CCC.OPC. For this,
> ravb_set_opmode() allows passing GAC and CSEL as part of opmode and the
> function updates accordingly CCC register.
> 
> Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>

[...]

> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c> index 664eda4b5a11..9835d18a7adf 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -66,16 +66,23 @@ int ravb_wait(struct net_device *ndev, enum ravb_reg reg, u32 mask, u32 value)
>  	return -ETIMEDOUT;
>  }
>  
> -static int ravb_config(struct net_device *ndev)
> +static int ravb_set_opmode(struct net_device *ndev, u32 opmode)
>  {
> +	u32 csr_ops = 1U << (opmode & CCC_OPC);
> +	u32 ccc_mask = CCC_OPC;
>  	int error;
>  
> -	/* Set config mode */
> -	ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
> -	/* Check if the operating mode is changed to the config mode */
> -	error = ravb_wait(ndev, CSR, CSR_OPS, CSR_OPS_CONFIG);
> -	if (error)
> -		netdev_err(ndev, "failed to switch device to config mode\n");
> +	if (opmode & CCC_GAC)

   Worth a comment?

> +		ccc_mask |= CCC_GAC | CCC_CSEL;

   Adding CCC.GAC to the mask not strictly necessary but OK...

[...]

MBR, Sergey