linux-kernel - Re: [PATCH net v3 1/1] net: ravb: Wait for operating mode to be applied

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Tue, 2 Jan 2024 21:43:14 +0300
From: Sergey Shtylyov <s.shtylyov@....ru>
To: Claudiu <claudiu.beznea@...on.dev>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>
CC: <mitsuhiro.kimura.kc@...esas.com>, <netdev@...r.kernel.org>,
	<linux-renesas-soc@...r.kernel.org>, <linux-kernel@...r.kernel.org>, Claudiu
 Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH net v3 1/1] net: ravb: Wait for operating mode to be
 applied

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

> From: Claudiu Beznea <claudiu.beznea.uj@...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@...renesas.com>

Reviewed-by: Sergey Shtylyov <s.shtylyov@....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