| Message ID | 20171125050832.11803-3-wens@csie.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | sunxi: Enable EMAC on various boards | expand |
On Fri, Nov 24, 2017 at 11:08 PM, Chen-Yu Tsai <wens@csie.org> wrote: > The EMAC syscon has configurable RX/TX delay chains for use with RGMII > PHYs. > > This adds support for configuring them via device tree properties. The > property names and format were defined in Linux's dwmac-sun8i binding > that was merged at one point. I'm not seeing this in doc/device-tree-bindings/net/
On Wed, Dec 6, 2017 at 4:50 AM, Joe Hershberger <joe.hershberger@ni.com> wrote: > On Fri, Nov 24, 2017 at 11:08 PM, Chen-Yu Tsai <wens@csie.org> wrote: >> The EMAC syscon has configurable RX/TX delay chains for use with RGMII >> PHYs. >> >> This adds support for configuring them via device tree properties. The >> property names and format were defined in Linux's dwmac-sun8i binding >> that was merged at one point. > > I'm not seeing this in doc/device-tree-bindings/net/ See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/dwmac-sun8i.txt The bindings have been restored as of v4.15-rc1. We are following DT bindings as defined in the Linux kernel. Deviation is kept to a minimum, and eliminated if possible. We still need to migrate the driver to the new bindings for the internal PHY bits. But that bit might still be changed during the 4.15 release cycle. ChenYu
On Tue, Dec 5, 2017 at 8:34 PM, Chen-Yu Tsai <wens@csie.org> wrote: > On Wed, Dec 6, 2017 at 4:50 AM, Joe Hershberger <joe.hershberger@ni.com> wrote: >> On Fri, Nov 24, 2017 at 11:08 PM, Chen-Yu Tsai <wens@csie.org> wrote: >>> The EMAC syscon has configurable RX/TX delay chains for use with RGMII >>> PHYs. >>> >>> This adds support for configuring them via device tree properties. The >>> property names and format were defined in Linux's dwmac-sun8i binding >>> that was merged at one point. >> >> I'm not seeing this in doc/device-tree-bindings/net/ > > See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/dwmac-sun8i.txt > > The bindings have been restored as of v4.15-rc1. > > We are following DT bindings as defined in the Linux kernel. Deviation > is kept to a minimum, and eliminated if possible. We still need to > migrate the driver to the new bindings for the internal PHY bits. > But that bit might still be changed during the 4.15 release cycle. That's good, but we want to have the currently supported bindings copied into the U-Boot tree under doc/device-tree-bindings/net/. Please include a patch that adds the bindings that your driver is using. Thanks, -Joe
On Thu, Dec 7, 2017 at 4:20 AM, Joe Hershberger <joe.hershberger@gmail.com> wrote: > On Tue, Dec 5, 2017 at 8:34 PM, Chen-Yu Tsai <wens@csie.org> wrote: >> On Wed, Dec 6, 2017 at 4:50 AM, Joe Hershberger <joe.hershberger@ni.com> wrote: >>> On Fri, Nov 24, 2017 at 11:08 PM, Chen-Yu Tsai <wens@csie.org> wrote: >>>> The EMAC syscon has configurable RX/TX delay chains for use with RGMII >>>> PHYs. >>>> >>>> This adds support for configuring them via device tree properties. The >>>> property names and format were defined in Linux's dwmac-sun8i binding >>>> that was merged at one point. >>> >>> I'm not seeing this in doc/device-tree-bindings/net/ >> >> See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/dwmac-sun8i.txt >> >> The bindings have been restored as of v4.15-rc1. >> >> We are following DT bindings as defined in the Linux kernel. Deviation >> is kept to a minimum, and eliminated if possible. We still need to >> migrate the driver to the new bindings for the internal PHY bits. >> But that bit might still be changed during the 4.15 release cycle. > > That's good, but we want to have the currently supported bindings > copied into the U-Boot tree under doc/device-tree-bindings/net/. > Please include a patch that adds the bindings that your driver is > using. Looks like this is a new requirement. Or it wasn't really enforced before. Doesn't this make U-boot prone to having diverging device tree bindings? It has already happened with the regulator bindings, specifically the "regulator-name" property. And by "currently supported", are you referring to what the driver expects, and not what the end result, i.e. the accepted bindings in Linux, should be? This driver is still in a state of catchup. The driver supports a previously merged then reverted set of bindings. These bindings were then brought back and updated for Linux v4.15 (unreleased yet, so may still change, again). So which set of bindings should I submit here? And the goal _is_ to migrate the driver to what Linux is using, so we can share the device tree files. Regards ChenYu
On Wed, Dec 6, 2017 at 8:35 PM, Chen-Yu Tsai <wens@csie.org> wrote: > On Thu, Dec 7, 2017 at 4:20 AM, Joe Hershberger > <joe.hershberger@gmail.com> wrote: >> On Tue, Dec 5, 2017 at 8:34 PM, Chen-Yu Tsai <wens@csie.org> wrote: >>> On Wed, Dec 6, 2017 at 4:50 AM, Joe Hershberger <joe.hershberger@ni.com> wrote: >>>> On Fri, Nov 24, 2017 at 11:08 PM, Chen-Yu Tsai <wens@csie.org> wrote: >>>>> The EMAC syscon has configurable RX/TX delay chains for use with RGMII >>>>> PHYs. >>>>> >>>>> This adds support for configuring them via device tree properties. The >>>>> property names and format were defined in Linux's dwmac-sun8i binding >>>>> that was merged at one point. >>>> >>>> I'm not seeing this in doc/device-tree-bindings/net/ >>> >>> See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/dwmac-sun8i.txt >>> >>> The bindings have been restored as of v4.15-rc1. >>> >>> We are following DT bindings as defined in the Linux kernel. Deviation >>> is kept to a minimum, and eliminated if possible. We still need to >>> migrate the driver to the new bindings for the internal PHY bits. >>> But that bit might still be changed during the 4.15 release cycle. >> >> That's good, but we want to have the currently supported bindings >> copied into the U-Boot tree under doc/device-tree-bindings/net/. >> Please include a patch that adds the bindings that your driver is >> using. > > Looks like this is a new requirement. Or it wasn't really enforced > before. Doesn't this make U-boot prone to having diverging device > tree bindings? It has already happened with the regulator bindings, > specifically the "regulator-name" property. > > And by "currently supported", are you referring to what the driver > expects, and not what the end result, i.e. the accepted bindings > in Linux, should be? This driver is still in a state of catchup. > The driver supports a previously merged then reverted set of > bindings. These bindings were then brought back and updated for > Linux v4.15 (unreleased yet, so may still change, again). So which > set of bindings should I submit here? > > And the goal _is_ to migrate the driver to what Linux is using, so > we can share the device tree files. I believe the goal is to have them match the current state of the driver as supported in U-Boot, and a higher-level goal of keeping the bindings + driver in sync with Linux for sharing. Correct, Simon? Thanks, -Joe
Hi Joe, On 7 December 2017 at 11:45, Joe Hershberger <joe.hershberger@gmail.com> wrote: > > On Wed, Dec 6, 2017 at 8:35 PM, Chen-Yu Tsai <wens@csie.org> wrote: > > On Thu, Dec 7, 2017 at 4:20 AM, Joe Hershberger > > <joe.hershberger@gmail.com> wrote: > >> On Tue, Dec 5, 2017 at 8:34 PM, Chen-Yu Tsai <wens@csie.org> wrote: > >>> On Wed, Dec 6, 2017 at 4:50 AM, Joe Hershberger <joe.hershberger@ni.com> wrote: > >>>> On Fri, Nov 24, 2017 at 11:08 PM, Chen-Yu Tsai <wens@csie.org> wrote: > >>>>> The EMAC syscon has configurable RX/TX delay chains for use with RGMII > >>>>> PHYs. > >>>>> > >>>>> This adds support for configuring them via device tree properties. The > >>>>> property names and format were defined in Linux's dwmac-sun8i binding > >>>>> that was merged at one point. > >>>> > >>>> I'm not seeing this in doc/device-tree-bindings/net/ > >>> > >>> See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/dwmac-sun8i.txt > >>> > >>> The bindings have been restored as of v4.15-rc1. > >>> > >>> We are following DT bindings as defined in the Linux kernel. Deviation > >>> is kept to a minimum, and eliminated if possible. We still need to > >>> migrate the driver to the new bindings for the internal PHY bits. > >>> But that bit might still be changed during the 4.15 release cycle. > >> > >> That's good, but we want to have the currently supported bindings > >> copied into the U-Boot tree under doc/device-tree-bindings/net/. > >> Please include a patch that adds the bindings that your driver is > >> using. > > > > Looks like this is a new requirement. Or it wasn't really enforced > > before. Doesn't this make U-boot prone to having diverging device > > tree bindings? It has already happened with the regulator bindings, > > specifically the "regulator-name" property. > > > > And by "currently supported", are you referring to what the driver > > expects, and not what the end result, i.e. the accepted bindings > > in Linux, should be? This driver is still in a state of catchup. > > The driver supports a previously merged then reverted set of > > bindings. These bindings were then brought back and updated for > > Linux v4.15 (unreleased yet, so may still change, again). So which > > set of bindings should I submit here? > > > > And the goal _is_ to migrate the driver to what Linux is using, so > > we can share the device tree files. > > I believe the goal is to have them match the current state of the > driver as supported in U-Boot, and a higher-level goal of keeping the > bindings + driver in sync with Linux for sharing. Yes, they should match, and we should have the same binding file. Also, please use dev_read_...() instead of fdtdec...() for all new code since that supports live tree. > > Correct, Simon? > > Thanks, > -Joe Regards, Simon
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 3ccc6b0bb612..ea26450d34bc 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -14,6 +14,7 @@ #include <asm/io.h> #include <asm/arch/clock.h> #include <asm/arch/gpio.h> +#include <bitfield.h> #include <common.h> #include <dm.h> #include <fdt_support.h> @@ -56,6 +57,8 @@ #define H3_EPHY_SELECT BIT(15) /* 1: internal PHY, 0: external PHY */ #define SC_RMII_EN BIT(13) +#define SC_TXDC_MASK GENMASK(12, 10) +#define SC_RXDC_MASK GENMASK(9, 5) #define SC_EPIT BIT(2) /* 1: RGMII, 0: MII */ #define SC_ETCS_MASK GENMASK(1, 0) #define SC_ETCS_EXT_GMII 0x1 @@ -125,6 +128,8 @@ struct emac_eth_dev { u32 addr; u32 tx_slot; bool use_internal_phy; + u32 tx_delay; + u32 rx_delay; enum emac_variant variant; void *mac_reg; @@ -290,6 +295,10 @@ static int sun8i_emac_set_syscon(struct emac_eth_dev *priv) if (priv->variant == H3_EMAC || priv->variant == A64_EMAC) reg &= ~SC_RMII_EN; + /* Configure RX/TX delay chains */ + reg = bitfield_replace_by_mask(reg, SC_RXDC_MASK, priv->rx_delay); + reg = bitfield_replace_by_mask(reg, SC_TXDC_MASK, priv->tx_delay); + switch (priv->interface) { case PHY_INTERFACE_MODE_MII: /* default */ @@ -839,6 +848,19 @@ static int sun8i_emac_eth_ofdata_to_platdata(struct udevice *dev) } #endif + /* Get RX/TX delays for RGMII */ + priv->rx_delay = fdtdec_get_uint(gd->fdt_blob, dev_of_offset(dev), + "allwinner,rx-delay-ps", 0); + if (priv->rx_delay % 100 || priv->rx_delay > 3100) + debug("%s: invalid rx delay value\n", __func__); + priv->rx_delay /= 100; + + priv->tx_delay = fdtdec_get_uint(gd->fdt_blob, dev_of_offset(dev), + "allwinner,tx-delay-ps", 0); + if (priv->tx_delay % 100 || priv->tx_delay > 800) + debug("%s: invalid tx delay value\n", __func__); + priv->tx_delay /= 100; + return 0; }
The EMAC syscon has configurable RX/TX delay chains for use with RGMII PHYs. This adds support for configuring them via device tree properties. The property names and format were defined in Linux's dwmac-sun8i binding that was merged at one point. Signed-off-by: Chen-Yu Tsai <wens@csie.org> --- drivers/net/sun8i_emac.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)