From: Zhou Yanjie <zhouyanjie@wanyeetech.com> To: Andrew Lunn <andrew@lunn.ch> Cc: davem@davemloft.net, kuba@kernel.org, robh+dt@kernel.org, peppe.cavallaro@st.com, alexandre.torgue@foss.st.com, joabreu@synopsys.com, mcoquelin.stm32@gmail.com, linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, dongsheng.qiu@ingenic.com, aric.pzqi@ingenic.com, rick.tyliu@ingenic.com, sihui.liu@ingenic.com, jun.jiang@ingenic.com, sernia.zhou@foxmail.com, paul@crapouillou.net Subject: Re: [PATCH 2/2] net: stmmac: Add Ingenic SoCs MAC support. Date: Tue, 8 Jun 2021 19:33:33 +0800 [thread overview] Message-ID: <6532a195-65db-afb3-37a2-f68bfed9d908@wanyeetech.com> (raw) In-Reply-To: <YL6zYgGdqxqL9c0j@lunn.ch> Hi Andrew, On 2021/6/8 上午8:01, Andrew Lunn wrote: >> 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. Apologize for my carelessness, this is left over accidentally when cleaning up the code, I will remove them in the next version. > >> +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. MACPHYC_TX_DELAY_63_UNIT means set MAC TX clk delay to 63 units (similar to the "tx-delay" in dwmac-rk.c). However, the manual does not clearly describe the time span of one unit, after consulting engineer of Ingenic, I learned that the value is recommended to be set to 63. I will change it to be similar to the way done in dwmac-rk.c. Thanks and best regards! > > Andrew
WARNING: multiple messages have this Message-ID (diff)
From: Zhou Yanjie <zhouyanjie@wanyeetech.com> To: Andrew Lunn <andrew@lunn.ch> Cc: davem@davemloft.net, kuba@kernel.org, robh+dt@kernel.org, peppe.cavallaro@st.com, alexandre.torgue@foss.st.com, joabreu@synopsys.com, mcoquelin.stm32@gmail.com, linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, dongsheng.qiu@ingenic.com, aric.pzqi@ingenic.com, rick.tyliu@ingenic.com, sihui.liu@ingenic.com, jun.jiang@ingenic.com, sernia.zhou@foxmail.com, paul@crapouillou.net Subject: Re: [PATCH 2/2] net: stmmac: Add Ingenic SoCs MAC support. Date: Tue, 8 Jun 2021 19:33:33 +0800 [thread overview] Message-ID: <6532a195-65db-afb3-37a2-f68bfed9d908@wanyeetech.com> (raw) In-Reply-To: <YL6zYgGdqxqL9c0j@lunn.ch> Hi Andrew, On 2021/6/8 上午8:01, Andrew Lunn wrote: >> 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. Apologize for my carelessness, this is left over accidentally when cleaning up the code, I will remove them in the next version. > >> +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. MACPHYC_TX_DELAY_63_UNIT means set MAC TX clk delay to 63 units (similar to the "tx-delay" in dwmac-rk.c). However, the manual does not clearly describe the time span of one unit, after consulting engineer of Ingenic, I learned that the value is recommended to be set to 63. I will change it to be similar to the way done in dwmac-rk.c. Thanks and best regards! > > Andrew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-06-08 11:33 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-07 17:27 [PATCH 0/2] Add Ingenic SoCs MAC support 周琰杰 (Zhou Yanjie) 2021-06-07 17:27 ` 周琰杰 (Zhou Yanjie) 2021-06-07 17:27 ` [PATCH 1/2] dt-bindings: dwmac: Add bindings for new Ingenic SoCs 周琰杰 (Zhou Yanjie) 2021-06-07 17:27 ` 周琰杰 (Zhou Yanjie) 2021-06-07 17:27 ` [PATCH 2/2] net: stmmac: Add Ingenic SoCs MAC support 周琰杰 (Zhou Yanjie) 2021-06-07 17:27 ` 周琰杰 (Zhou Yanjie) 2021-06-08 0:01 ` Andrew Lunn 2021-06-08 0:01 ` Andrew Lunn 2021-06-08 11:33 ` Zhou Yanjie [this message] 2021-06-08 11:33 ` Zhou Yanjie 2021-06-08 12:21 ` Andrew Lunn 2021-06-08 12:21 ` Andrew Lunn 2021-06-08 13:48 ` Zhou Yanjie 2021-06-08 13:48 ` Zhou Yanjie 2021-06-08 14:04 ` Andrew Lunn 2021-06-08 14:04 ` Andrew Lunn 2021-06-08 8:46 ` Paul Cercueil 2021-06-08 8:46 ` Paul Cercueil 2021-06-08 13:24 ` Zhou Yanjie 2021-06-08 13:24 ` Zhou Yanjie
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=6532a195-65db-afb3-37a2-f68bfed9d908@wanyeetech.com \ --to=zhouyanjie@wanyeetech.com \ --cc=alexandre.torgue@foss.st.com \ --cc=andrew@lunn.ch \ --cc=aric.pzqi@ingenic.com \ --cc=davem@davemloft.net \ --cc=devicetree@vger.kernel.org \ --cc=dongsheng.qiu@ingenic.com \ --cc=joabreu@synopsys.com \ --cc=jun.jiang@ingenic.com \ --cc=kuba@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mips@vger.kernel.org \ --cc=linux-stm32@st-md-mailman.stormreply.com \ --cc=mcoquelin.stm32@gmail.com \ --cc=netdev@vger.kernel.org \ --cc=paul@crapouillou.net \ --cc=peppe.cavallaro@st.com \ --cc=rick.tyliu@ingenic.com \ --cc=robh+dt@kernel.org \ --cc=sernia.zhou@foxmail.com \ --cc=sihui.liu@ingenic.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.