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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 5 Dec 2020 11:58:02 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Joakim Zhang <qiangqing.zhang@....com>
Cc:     peppe.cavallaro@...com, alexandre.torgue@...com,
        joabreu@...opsys.com, davem@...emloft.net, linux-imx@....com,
        netdev@...r.kernel.org
Subject: Re: [PATCH] net: stmmac: implement .set_intf_mode() callback for
 imx8dxl

On Thu,  3 Dec 2020 12:10:38 +0800 Joakim Zhang wrote:
> From: Fugang Duan <fugang.duan@....com>
> 
> Implement .set_intf_mode() callback for imx8dxl.
> 
> Signed-off-by: Fugang Duan <fugang.duan@....com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@....com>

Couple minor issues.

> @@ -86,7 +88,37 @@ imx8dxl_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
>  {
>  	int ret = 0;
>  
> -	/* TBD: depends on imx8dxl scu interfaces to be upstreamed */
> +	struct imx_sc_ipc *ipc_handle;
> +	int val;

Looks like you're gonna have a empty line in the middle of variable
declarations?

Please remove it and order the variable lines longest to shortest.

> +
> +	ret = imx_scu_get_handle(&ipc_handle);
> +	if (ret)
> +		return ret;
> +
> +	switch (plat_dat->interface) {
> +	case PHY_INTERFACE_MODE_MII:
> +		val = GPR_ENET_QOS_INTF_SEL_MII;
> +		break;
> +	case PHY_INTERFACE_MODE_RMII:
> +		val = GPR_ENET_QOS_INTF_SEL_RMII;
> +		break;
> +	case PHY_INTERFACE_MODE_RGMII:
> +	case PHY_INTERFACE_MODE_RGMII_ID:
> +	case PHY_INTERFACE_MODE_RGMII_RXID:
> +	case PHY_INTERFACE_MODE_RGMII_TXID:
> +		val = GPR_ENET_QOS_INTF_SEL_RGMII;
> +		break;
> +	default:
> +		pr_debug("imx dwmac doesn't support %d interface\n",
> +			 plat_dat->interface);
> +		return -EINVAL;
> +	}
> +
> +	ret = imx_sc_misc_set_control(ipc_handle, IMX_SC_R_ENET_1,
> +				      IMX_SC_C_INTF_SEL, val >> 16);
> +	ret |= imx_sc_misc_set_control(ipc_handle, IMX_SC_R_ENET_1,
> +				       IMX_SC_C_CLK_GEN_EN, 0x1);
>  	return ret;

These calls may return different errors AFAICT.

You can't just errno values to gether the result will be meaningless.

please use the normal flow, and return the result of the second call
directly:

	ret = func1();
	if (ret)
		return ret;

	return func2();

Please also CC the maintainers of the Ethernet PHY subsystem on v2, 
to make sure there is nothing wrong with the patch from their PoV.

Thanks!

Powered by blists - more mailing lists