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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YL6zYgGdqxqL9c0j@lunn.ch>
Date:   Tue, 8 Jun 2021 02:01:38 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     周琰杰 (Zhou Yanjie) 
        <zhouyanjie@...yeetech.com>
Cc:     davem@...emloft.net, kuba@...nel.org, robh+dt@...nel.org,
        peppe.cavallaro@...com, alexandre.torgue@...s.st.com,
        joabreu@...opsys.com, mcoquelin.stm32@...il.com,
        linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, devicetree@...r.kernel.org,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org, dongsheng.qiu@...enic.com,
        aric.pzqi@...enic.com, rick.tyliu@...enic.com,
        sihui.liu@...enic.com, jun.jiang@...enic.com,
        sernia.zhou@...mail.com, paul@...pouillou.net
Subject: Re: [PATCH 2/2] net: stmmac: Add Ingenic SoCs MAC support.

>  config DWMAC_ROCKCHIP
>  	tristate "Rockchip dwmac support"
> -	default ARCH_ROCKCHIP
> +	default MACH_ROCKCHIP
>  	depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
>  	select MFD_SYSCON
>  	help
> @@ -164,7 +176,7 @@ config DWMAC_STI
>  
>  config DWMAC_STM32
>  	tristate "STM32 DWMAC support"
> -	default ARCH_STM32
> +	default MACH_STM32

It would be good to explain in the commit message why you are changing
these two. It is not obvious.

> +static int jz4775_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
> +{
> +	struct ingenic_mac *mac = plat_dat->bsp_priv;
> +	int val;
> +
> +	switch (plat_dat->interface) {
> +	case PHY_INTERFACE_MODE_MII:
> +		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_MII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_MII\n");
> +		break;
> +
> +	case PHY_INTERFACE_MODE_GMII:
> +		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_GMII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_GMII\n");
> +		break;
> +
> +	case PHY_INTERFACE_MODE_RMII:
> +		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n");
> +		break;
> +
> +	case PHY_INTERFACE_MODE_RGMII:

What about the other three RGMII modes?

> +	case PHY_INTERFACE_MODE_RGMII:
> +		val = FIELD_PREP(MACPHYC_TX_SEL_MASK, MACPHYC_TX_SEL_DELAY) |
> +			  FIELD_PREP(MACPHYC_TX_DELAY_MASK, MACPHYC_TX_DELAY_63_UNIT) |
> +			  FIELD_PREP(MACPHYC_RX_SEL_MASK, MACPHYC_RX_SEL_ORIGIN) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RGMII);

What exactly does MACPHYC_TX_DELAY_63_UNIT mean here? Ideally, the MAC
should not be adding any RGMII delays. It should however pass mode
through to the PHY, so it can add the delays, if the mode indicates it
should, e.g. PHY_INTERFACE_MODE_RGMII_ID. This is also why you should
be handling all 4 RGMII modes here, not just one.

	 Andrew

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ