diff mbox series

[U-Boot,6/8] arm: ls1021atwr: Convert to use driver model TSEC driver

Message ID 20190623174818.12773-7-olteanv@gmail.com
State Superseded
Delegated to: Prabhakar Kushwaha
Headers show
Series NXP LS1021A-TSN Board | expand

Commit Message

Vladimir Oltean June 23, 2019, 5:48 p.m. UTC 
From: Bin Meng <bmeng.cn@gmail.com>

Now that we have added driver model support to the TSEC driver,
convert ls1021atwr board to use it.

This depends on previous DM series for ls1021atwr:
http://patchwork.ozlabs.org/patch/561855/

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

[Vladimir] Made the following changes:
- Added 'status = "disabled";' for all Ethernet ports in ls1021a.dtsi
- Fixed the confusion between the SGMII/TBI PCS for enet0 and enet1 -
  a mistake ported over from Linux. Each SGMII PCS lies on the private
  MDIO bus of the interface (and the RGMII enet2 has no SGMII PCS).

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 arch/arm/cpu/armv7/ls102xa/cpu.c        |  2 +-
 arch/arm/cpu/armv7/ls102xa/fdt.c        | 10 ++++++++
 arch/arm/dts/ls1021a-twr.dtsi           | 32 +++++++++++++++++++++++++
 arch/arm/dts/ls1021a.dtsi               | 28 ++++++++++++++++++++--
 board/freescale/ls1021atwr/ls1021atwr.c |  2 +-
 configs/ls1021atwr_nor_defconfig        |  1 +
 configs/ls1021atwr_nor_lpuart_defconfig |  1 +
 include/configs/ls1021atwr.h            |  4 ++++
 8 files changed, 76 insertions(+), 4 deletions(-)

Comments

Joe Hershberger July 12, 2019, 8:46 p.m. UTC | #1 
On Sun, Jun 23, 2019 at 12:53 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> From: Bin Meng <bmeng.cn@gmail.com>
>
> Now that we have added driver model support to the TSEC driver,
> convert ls1021atwr board to use it.
>
> This depends on previous DM series for ls1021atwr:
> http://patchwork.ozlabs.org/patch/561855/
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

Generally looks good, but a few nits below...

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

> [Vladimir] Made the following changes:
> - Added 'status = "disabled";' for all Ethernet ports in ls1021a.dtsi
> - Fixed the confusion between the SGMII/TBI PCS for enet0 and enet1 -
>   a mistake ported over from Linux. Each SGMII PCS lies on the private
>   MDIO bus of the interface (and the RGMII enet2 has no SGMII PCS).
>
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
> ---
>  arch/arm/cpu/armv7/ls102xa/cpu.c        |  2 +-
>  arch/arm/cpu/armv7/ls102xa/fdt.c        | 10 ++++++++
>  arch/arm/dts/ls1021a-twr.dtsi           | 32 +++++++++++++++++++++++++
>  arch/arm/dts/ls1021a.dtsi               | 28 ++++++++++++++++++++--
>  board/freescale/ls1021atwr/ls1021atwr.c |  2 +-
>  configs/ls1021atwr_nor_defconfig        |  1 +
>  configs/ls1021atwr_nor_lpuart_defconfig |  1 +
>  include/configs/ls1021atwr.h            |  4 ++++
>  8 files changed, 76 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/ls102xa/cpu.c b/arch/arm/cpu/armv7/ls102xa/cpu.c
> index ecf9e869855e..9ccfe1042ce5 100644
> --- a/arch/arm/cpu/armv7/ls102xa/cpu.c
> +++ b/arch/arm/cpu/armv7/ls102xa/cpu.c
> @@ -296,7 +296,7 @@ int cpu_mmc_init(bd_t *bis)
>
>  int cpu_eth_init(bd_t *bis)
>  {
> -#ifdef CONFIG_TSEC_ENET
> +#if defined(CONFIG_TSEC_ENET) && !defined(CONFIG_DM_ETH)
>         tsec_standard_init(bis);
>  #endif
>
> diff --git a/arch/arm/cpu/armv7/ls102xa/fdt.c b/arch/arm/cpu/armv7/ls102xa/fdt.c
> index 8bf9c42b2260..90cf7958f257 100644
> --- a/arch/arm/cpu/armv7/ls102xa/fdt.c
> +++ b/arch/arm/cpu/armv7/ls102xa/fdt.c
> @@ -16,12 +16,17 @@
>  #include <tsec.h>
>  #include <asm/arch/immap_ls102xa.h>
>  #include <fsl_sec.h>
> +#include <dm.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
>  void ft_fixup_enet_phy_connect_type(void *fdt)
>  {
> +#ifndef CONFIG_DM_ETH

Please use positive logic where convenient. I.e. #ifdef CONFIG_DM_ETH
and swap cases.

>         struct eth_device *dev;
> +#else
> +       struct udevice *dev;
> +#endif
>         struct tsec_private *priv;
>         const char *enet_path, *phy_path;
>         char enet[16];
> @@ -29,7 +34,12 @@ void ft_fixup_enet_phy_connect_type(void *fdt)
>         int phy_node;
>         int i = 0;
>         uint32_t ph;
> +#ifndef CONFIG_DM_ETH

Use positive logic.

>         char *name[3] = { "eTSEC1", "eTSEC2", "eTSEC3" };
> +#else
> +       char *name[3] = { "ethernet@2d10000", "ethernet@2d50000",
> +                         "ethernet@2d90000" };
> +#endif
>
>         for (; i < ARRAY_SIZE(name); i++) {
>                 dev = eth_get_dev_by_name(name[i]);
> diff --git a/arch/arm/dts/ls1021a-twr.dtsi b/arch/arm/dts/ls1021a-twr.dtsi
> index 5d3275ced913..27c96f95400a 100644
> --- a/arch/arm/dts/ls1021a-twr.dtsi
> +++ b/arch/arm/dts/ls1021a-twr.dtsi
> @@ -51,6 +51,26 @@
>         };
>  };
>
> +&enet0 {
> +       tbi-handle = <&tbi0>;
> +       phy-handle = <&sgmii_phy2>;
> +       phy-connection-type = "sgmii";
> +       status = "okay";
> +};
> +
> +&enet1 {
> +       tbi-handle = <&tbi1>;
> +       phy-handle = <&sgmii_phy0>;
> +       phy-connection-type = "sgmii";
> +       status = "okay";
> +};
> +
> +&enet2 {
> +       phy-handle = <&rgmii_phy1>;
> +       phy-connection-type = "rgmii-id";
> +       status = "okay";
> +};
> +
>  &i2c0 {
>         status = "okay";
>  };
> @@ -84,12 +104,24 @@
>         sgmii_phy0: ethernet-phy@0 {
>                 reg = <0x0>;
>         };
> +
>         rgmii_phy1: ethernet-phy@1 {
>                 reg = <0x1>;
>         };
> +
>         sgmii_phy2: ethernet-phy@2 {
>                 reg = <0x2>;
>         };
> +
> +       /* SGMII PCS for enet0 */
> +       tbi0: tbi-phy@1f {
> +               reg = <0x1f>;
> +               device_type = "tbi-phy";
> +       };
> +};
> +
> +&mdio1 {
> +       /* SGMII PCS for enet1 */
>         tbi1: tbi-phy@1f {
>                 reg = <0x1f>;
>                 device_type = "tbi-phy";
> diff --git a/arch/arm/dts/ls1021a.dtsi b/arch/arm/dts/ls1021a.dtsi
> index 8a0f473e25ca..c274a302d358 100644
> --- a/arch/arm/dts/ls1021a.dtsi
> +++ b/arch/arm/dts/ls1021a.dtsi
> @@ -350,14 +350,38 @@
>                                  <&platform_clk 1>;
>                 };
>
> +               enet0: ethernet@2d10000 {
> +                       compatible = "fsl,tsec";
> +                       reg = <0x2d10000 0x1000>;
> +                       status = "disabled";
> +               };
> +
> +               enet1: ethernet@2d50000 {
> +                       compatible = "fsl,tsec";
> +                       reg = <0x2d50000 0x1000>;
> +                       status = "disabled";
> +               };
> +
> +               enet2: ethernet@2d90000 {
> +                       compatible = "fsl,tsec";
> +                       reg = <0x2d90000 0x1000>;
> +                       status = "disabled";
> +               };
> +
>                 mdio0: mdio@2d24000 {
> -                       compatible = "gianfar";
> -                       device_type = "mdio";
> +                       compatible = "fsl,tsec-mdio";
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         reg = <0x2d24000 0x4000>;
>                 };
>
> +               mdio1: mdio@2d64000 {
> +                       compatible = "fsl,tsec-mdio";
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg = <0x2d64000 0x4000>;
> +               };
> +
>                 usb@8600000 {
>                         compatible = "fsl-usb2-dr-v2.5", "fsl-usb2-dr";
>                         reg = <0x8600000 0x1000>;
> diff --git a/board/freescale/ls1021atwr/ls1021atwr.c b/board/freescale/ls1021atwr/ls1021atwr.c
> index 01ba1bc96213..a4ff0b7bc7eb 100644
> --- a/board/freescale/ls1021atwr/ls1021atwr.c
> +++ b/board/freescale/ls1021atwr/ls1021atwr.c
> @@ -248,7 +248,7 @@ int board_mmc_init(bd_t *bis)
>
>  int board_eth_init(bd_t *bis)
>  {
> -#ifdef CONFIG_TSEC_ENET
> +#if defined(CONFIG_TSEC_ENET) && !defined(CONFIG_DM_ETH)
>         struct fsl_pq_mdio_info mdio_info;
>         struct tsec_info_struct tsec_info[4];
>         int num = 0;
> diff --git a/configs/ls1021atwr_nor_defconfig b/configs/ls1021atwr_nor_defconfig
> index 9d8c2024c04e..3c9cf9a8c909 100644
> --- a/configs/ls1021atwr_nor_defconfig
> +++ b/configs/ls1021atwr_nor_defconfig
> @@ -40,6 +40,7 @@ CONFIG_SYS_FLASH_CFI=y
>  CONFIG_PHY_GIGE=y
>  CONFIG_E1000=y
>  CONFIG_MII=y
> +CONFIG_DM_ETH=y
>  CONFIG_TSEC_ENET=y
>  CONFIG_PCI=y
>  CONFIG_DM_PCI=y
> diff --git a/configs/ls1021atwr_nor_lpuart_defconfig b/configs/ls1021atwr_nor_lpuart_defconfig
> index b9cfdb6fd69e..762af87b0dd3 100644
> --- a/configs/ls1021atwr_nor_lpuart_defconfig
> +++ b/configs/ls1021atwr_nor_lpuart_defconfig
> @@ -42,6 +42,7 @@ CONFIG_SYS_FLASH_CFI=y
>  CONFIG_PHY_GIGE=y
>  CONFIG_E1000=y
>  CONFIG_MII=y
> +CONFIG_DM_ETH=y
>  CONFIG_TSEC_ENET=y
>  CONFIG_PCI=y
>  CONFIG_DM_PCI=y
> diff --git a/include/configs/ls1021atwr.h b/include/configs/ls1021atwr.h
> index de0c9c7f26af..c967be3a6fce 100644
> --- a/include/configs/ls1021atwr.h
> +++ b/include/configs/ls1021atwr.h
> @@ -260,6 +260,7 @@
>   */
>
>  #ifdef CONFIG_TSEC_ENET
> +#ifndef CONFIG_DM_ETH

Use positive logic

>  #define CONFIG_MII_DEFAULT_TSEC                1
>  #define CONFIG_TSEC1                   1
>  #define CONFIG_TSEC1_NAME              "eTSEC1"
> @@ -287,6 +288,9 @@
>  #define CONFIG_HAS_ETH0
>  #define CONFIG_HAS_ETH1
>  #define CONFIG_HAS_ETH2
> +#else
> +#define CONFIG_ETHPRIME                        "ethernet@2d10000"
> +#endif
>  #endif
>
>  /* PCIe */
> --
> 2.17.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Vladimir Oltean July 13, 2019, 9:39 a.m. UTC | #2 
Hi Joe,

On Fri, 12 Jul 2019 at 23:46, Joe Hershberger <joe.hershberger@ni.com> wrote:
>
> On Sun, Jun 23, 2019 at 12:53 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > From: Bin Meng <bmeng.cn@gmail.com>
> >
> > Now that we have added driver model support to the TSEC driver,
> > convert ls1021atwr board to use it.
> >
> > This depends on previous DM series for ls1021atwr:
> > http://patchwork.ozlabs.org/patch/561855/
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> Generally looks good, but a few nits below...
>
> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
>
> > [Vladimir] Made the following changes:
> > - Added 'status = "disabled";' for all Ethernet ports in ls1021a.dtsi
> > - Fixed the confusion between the SGMII/TBI PCS for enet0 and enet1 -
> >   a mistake ported over from Linux. Each SGMII PCS lies on the private
> >   MDIO bus of the interface (and the RGMII enet2 has no SGMII PCS).
> >
> > Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
> > ---
> >  arch/arm/cpu/armv7/ls102xa/cpu.c        |  2 +-
> >  arch/arm/cpu/armv7/ls102xa/fdt.c        | 10 ++++++++
> >  arch/arm/dts/ls1021a-twr.dtsi           | 32 +++++++++++++++++++++++++
> >  arch/arm/dts/ls1021a.dtsi               | 28 ++++++++++++++++++++--
> >  board/freescale/ls1021atwr/ls1021atwr.c |  2 +-
> >  configs/ls1021atwr_nor_defconfig        |  1 +
> >  configs/ls1021atwr_nor_lpuart_defconfig |  1 +
> >  include/configs/ls1021atwr.h            |  4 ++++
> >  8 files changed, 76 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/cpu/armv7/ls102xa/cpu.c b/arch/arm/cpu/armv7/ls102xa/cpu.c
> > index ecf9e869855e..9ccfe1042ce5 100644
> > --- a/arch/arm/cpu/armv7/ls102xa/cpu.c
> > +++ b/arch/arm/cpu/armv7/ls102xa/cpu.c
> > @@ -296,7 +296,7 @@ int cpu_mmc_init(bd_t *bis)
> >
> >  int cpu_eth_init(bd_t *bis)
> >  {
> > -#ifdef CONFIG_TSEC_ENET
> > +#if defined(CONFIG_TSEC_ENET) && !defined(CONFIG_DM_ETH)
> >         tsec_standard_init(bis);
> >  #endif
> >
> > diff --git a/arch/arm/cpu/armv7/ls102xa/fdt.c b/arch/arm/cpu/armv7/ls102xa/fdt.c
> > index 8bf9c42b2260..90cf7958f257 100644
> > --- a/arch/arm/cpu/armv7/ls102xa/fdt.c
> > +++ b/arch/arm/cpu/armv7/ls102xa/fdt.c
> > @@ -16,12 +16,17 @@
> >  #include <tsec.h>
> >  #include <asm/arch/immap_ls102xa.h>
> >  #include <fsl_sec.h>
> > +#include <dm.h>
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> >  void ft_fixup_enet_phy_connect_type(void *fdt)
> >  {
> > +#ifndef CONFIG_DM_ETH
>
> Please use positive logic where convenient. I.e. #ifdef CONFIG_DM_ETH
> and swap cases.
>

To be honest I don't know why keep compatibility with non-DM ETH at
all for the TWR board. On the LS1021A-TSN I'm not doing that.
Bin, is there any particular reason? If not, I'll just completely
remove your #ifdef's for v2.

> >         struct eth_device *dev;
> > +#else
> > +       struct udevice *dev;
> > +#endif
> >         struct tsec_private *priv;
> >         const char *enet_path, *phy_path;
> >         char enet[16];
> > @@ -29,7 +34,12 @@ void ft_fixup_enet_phy_connect_type(void *fdt)
> >         int phy_node;
> >         int i = 0;
> >         uint32_t ph;
> > +#ifndef CONFIG_DM_ETH
>
> Use positive logic.
>
> >         char *name[3] = { "eTSEC1", "eTSEC2", "eTSEC3" };
> > +#else
> > +       char *name[3] = { "ethernet@2d10000", "ethernet@2d50000",
> > +                         "ethernet@2d90000" };
> > +#endif
> >
> >         for (; i < ARRAY_SIZE(name); i++) {
> >                 dev = eth_get_dev_by_name(name[i]);
> > diff --git a/arch/arm/dts/ls1021a-twr.dtsi b/arch/arm/dts/ls1021a-twr.dtsi
> > index 5d3275ced913..27c96f95400a 100644
> > --- a/arch/arm/dts/ls1021a-twr.dtsi
> > +++ b/arch/arm/dts/ls1021a-twr.dtsi
> > @@ -51,6 +51,26 @@
> >         };
> >  };
> >
> > +&enet0 {
> > +       tbi-handle = <&tbi0>;
> > +       phy-handle = <&sgmii_phy2>;
> > +       phy-connection-type = "sgmii";
> > +       status = "okay";
> > +};
> > +
> > +&enet1 {
> > +       tbi-handle = <&tbi1>;
> > +       phy-handle = <&sgmii_phy0>;
> > +       phy-connection-type = "sgmii";
> > +       status = "okay";
> > +};
> > +
> > +&enet2 {
> > +       phy-handle = <&rgmii_phy1>;
> > +       phy-connection-type = "rgmii-id";
> > +       status = "okay";
> > +};
> > +
> >  &i2c0 {
> >         status = "okay";
> >  };
> > @@ -84,12 +104,24 @@
> >         sgmii_phy0: ethernet-phy@0 {
> >                 reg = <0x0>;
> >         };
> > +
> >         rgmii_phy1: ethernet-phy@1 {
> >                 reg = <0x1>;
> >         };
> > +
> >         sgmii_phy2: ethernet-phy@2 {
> >                 reg = <0x2>;
> >         };
> > +
> > +       /* SGMII PCS for enet0 */
> > +       tbi0: tbi-phy@1f {
> > +               reg = <0x1f>;
> > +               device_type = "tbi-phy";
> > +       };
> > +};
> > +
> > +&mdio1 {
> > +       /* SGMII PCS for enet1 */
> >         tbi1: tbi-phy@1f {
> >                 reg = <0x1f>;
> >                 device_type = "tbi-phy";
> > diff --git a/arch/arm/dts/ls1021a.dtsi b/arch/arm/dts/ls1021a.dtsi
> > index 8a0f473e25ca..c274a302d358 100644
> > --- a/arch/arm/dts/ls1021a.dtsi
> > +++ b/arch/arm/dts/ls1021a.dtsi
> > @@ -350,14 +350,38 @@
> >                                  <&platform_clk 1>;
> >                 };
> >
> > +               enet0: ethernet@2d10000 {
> > +                       compatible = "fsl,tsec";
> > +                       reg = <0x2d10000 0x1000>;
> > +                       status = "disabled";
> > +               };
> > +
> > +               enet1: ethernet@2d50000 {
> > +                       compatible = "fsl,tsec";
> > +                       reg = <0x2d50000 0x1000>;
> > +                       status = "disabled";
> > +               };
> > +
> > +               enet2: ethernet@2d90000 {
> > +                       compatible = "fsl,tsec";
> > +                       reg = <0x2d90000 0x1000>;
> > +                       status = "disabled";
> > +               };
> > +
> >                 mdio0: mdio@2d24000 {
> > -                       compatible = "gianfar";
> > -                       device_type = "mdio";
> > +                       compatible = "fsl,tsec-mdio";
> >                         #address-cells = <1>;
> >                         #size-cells = <0>;
> >                         reg = <0x2d24000 0x4000>;
> >                 };
> >
> > +               mdio1: mdio@2d64000 {
> > +                       compatible = "fsl,tsec-mdio";
> > +                       #address-cells = <1>;
> > +                       #size-cells = <0>;
> > +                       reg = <0x2d64000 0x4000>;
> > +               };
> > +
> >                 usb@8600000 {
> >                         compatible = "fsl-usb2-dr-v2.5", "fsl-usb2-dr";
> >                         reg = <0x8600000 0x1000>;
> > diff --git a/board/freescale/ls1021atwr/ls1021atwr.c b/board/freescale/ls1021atwr/ls1021atwr.c
> > index 01ba1bc96213..a4ff0b7bc7eb 100644
> > --- a/board/freescale/ls1021atwr/ls1021atwr.c
> > +++ b/board/freescale/ls1021atwr/ls1021atwr.c
> > @@ -248,7 +248,7 @@ int board_mmc_init(bd_t *bis)
> >
> >  int board_eth_init(bd_t *bis)
> >  {
> > -#ifdef CONFIG_TSEC_ENET
> > +#if defined(CONFIG_TSEC_ENET) && !defined(CONFIG_DM_ETH)
> >         struct fsl_pq_mdio_info mdio_info;
> >         struct tsec_info_struct tsec_info[4];
> >         int num = 0;
> > diff --git a/configs/ls1021atwr_nor_defconfig b/configs/ls1021atwr_nor_defconfig
> > index 9d8c2024c04e..3c9cf9a8c909 100644
> > --- a/configs/ls1021atwr_nor_defconfig
> > +++ b/configs/ls1021atwr_nor_defconfig
> > @@ -40,6 +40,7 @@ CONFIG_SYS_FLASH_CFI=y
> >  CONFIG_PHY_GIGE=y
> >  CONFIG_E1000=y
> >  CONFIG_MII=y
> > +CONFIG_DM_ETH=y
> >  CONFIG_TSEC_ENET=y
> >  CONFIG_PCI=y
> >  CONFIG_DM_PCI=y
> > diff --git a/configs/ls1021atwr_nor_lpuart_defconfig b/configs/ls1021atwr_nor_lpuart_defconfig
> > index b9cfdb6fd69e..762af87b0dd3 100644
> > --- a/configs/ls1021atwr_nor_lpuart_defconfig
> > +++ b/configs/ls1021atwr_nor_lpuart_defconfig
> > @@ -42,6 +42,7 @@ CONFIG_SYS_FLASH_CFI=y
> >  CONFIG_PHY_GIGE=y
> >  CONFIG_E1000=y
> >  CONFIG_MII=y
> > +CONFIG_DM_ETH=y
> >  CONFIG_TSEC_ENET=y
> >  CONFIG_PCI=y
> >  CONFIG_DM_PCI=y
> > diff --git a/include/configs/ls1021atwr.h b/include/configs/ls1021atwr.h
> > index de0c9c7f26af..c967be3a6fce 100644
> > --- a/include/configs/ls1021atwr.h
> > +++ b/include/configs/ls1021atwr.h
> > @@ -260,6 +260,7 @@
> >   */
> >
> >  #ifdef CONFIG_TSEC_ENET
> > +#ifndef CONFIG_DM_ETH
>
> Use positive logic
>
> >  #define CONFIG_MII_DEFAULT_TSEC                1
> >  #define CONFIG_TSEC1                   1
> >  #define CONFIG_TSEC1_NAME              "eTSEC1"
> > @@ -287,6 +288,9 @@
> >  #define CONFIG_HAS_ETH0
> >  #define CONFIG_HAS_ETH1
> >  #define CONFIG_HAS_ETH2
> > +#else
> > +#define CONFIG_ETHPRIME                        "ethernet@2d10000"
> > +#endif
> >  #endif
> >
> >  /* PCIe */
> > --
> > 2.17.1
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > https://lists.denx.de/listinfo/u-boot

Thanks,
-Vladimir
Bin Meng July 14, 2019, 1:54 a.m. UTC | #3 
Hi Vladimir,

On Sat, Jul 13, 2019 at 5:39 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Hi Joe,
>
> On Fri, 12 Jul 2019 at 23:46, Joe Hershberger <joe.hershberger@ni.com> wrote:
> >
> > On Sun, Jun 23, 2019 at 12:53 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > >
> > > From: Bin Meng <bmeng.cn@gmail.com>
> > >
> > > Now that we have added driver model support to the TSEC driver,
> > > convert ls1021atwr board to use it.
> > >
> > > This depends on previous DM series for ls1021atwr:
> > > http://patchwork.ozlabs.org/patch/561855/
> > >
> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >
> > Generally looks good, but a few nits below...
> >
> > Acked-by: Joe Hershberger <joe.hershberger@ni.com>
> >
> > > [Vladimir] Made the following changes:
> > > - Added 'status = "disabled";' for all Ethernet ports in ls1021a.dtsi
> > > - Fixed the confusion between the SGMII/TBI PCS for enet0 and enet1 -
> > >   a mistake ported over from Linux. Each SGMII PCS lies on the private
> > >   MDIO bus of the interface (and the RGMII enet2 has no SGMII PCS).
> > >
> > > Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
> > > ---
> > >  arch/arm/cpu/armv7/ls102xa/cpu.c        |  2 +-
> > >  arch/arm/cpu/armv7/ls102xa/fdt.c        | 10 ++++++++
> > >  arch/arm/dts/ls1021a-twr.dtsi           | 32 +++++++++++++++++++++++++
> > >  arch/arm/dts/ls1021a.dtsi               | 28 ++++++++++++++++++++--
> > >  board/freescale/ls1021atwr/ls1021atwr.c |  2 +-
> > >  configs/ls1021atwr_nor_defconfig        |  1 +
> > >  configs/ls1021atwr_nor_lpuart_defconfig |  1 +
> > >  include/configs/ls1021atwr.h            |  4 ++++
> > >  8 files changed, 76 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/arm/cpu/armv7/ls102xa/cpu.c b/arch/arm/cpu/armv7/ls102xa/cpu.c
> > > index ecf9e869855e..9ccfe1042ce5 100644
> > > --- a/arch/arm/cpu/armv7/ls102xa/cpu.c
> > > +++ b/arch/arm/cpu/armv7/ls102xa/cpu.c
> > > @@ -296,7 +296,7 @@ int cpu_mmc_init(bd_t *bis)
> > >
> > >  int cpu_eth_init(bd_t *bis)
> > >  {
> > > -#ifdef CONFIG_TSEC_ENET
> > > +#if defined(CONFIG_TSEC_ENET) && !defined(CONFIG_DM_ETH)
> > >         tsec_standard_init(bis);
> > >  #endif
> > >
> > > diff --git a/arch/arm/cpu/armv7/ls102xa/fdt.c b/arch/arm/cpu/armv7/ls102xa/fdt.c
> > > index 8bf9c42b2260..90cf7958f257 100644
> > > --- a/arch/arm/cpu/armv7/ls102xa/fdt.c
> > > +++ b/arch/arm/cpu/armv7/ls102xa/fdt.c
> > > @@ -16,12 +16,17 @@
> > >  #include <tsec.h>
> > >  #include <asm/arch/immap_ls102xa.h>
> > >  #include <fsl_sec.h>
> > > +#include <dm.h>
> > >
> > >  DECLARE_GLOBAL_DATA_PTR;
> > >
> > >  void ft_fixup_enet_phy_connect_type(void *fdt)
> > >  {
> > > +#ifndef CONFIG_DM_ETH
> >
> > Please use positive logic where convenient. I.e. #ifdef CONFIG_DM_ETH
> > and swap cases.
> >
>
> To be honest I don't know why keep compatibility with non-DM ETH at
> all for the TWR board. On the LS1021A-TSN I'm not doing that.
> Bin, is there any particular reason? If not, I'll just completely
> remove your #ifdef's for v2.

I remember at that time there were some PowerPC 83xx/85xx boards that
were not converted to DM but still used TSEC driver. If this is not a
concern now, I am all for it to remove the non-DM TSEC support.

Regards,
Bin
Vladimir Oltean July 14, 2019, 10:04 a.m. UTC | #4 
On Sun, 14 Jul 2019 at 04:55, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Vladimir,
>
> On Sat, Jul 13, 2019 at 5:39 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > Hi Joe,
> >
> > On Fri, 12 Jul 2019 at 23:46, Joe Hershberger <joe.hershberger@ni.com> wrote:
> > >
> > > On Sun, Jun 23, 2019 at 12:53 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > > >
> > > > From: Bin Meng <bmeng.cn@gmail.com>
> > > >
> > > > Now that we have added driver model support to the TSEC driver,
> > > > convert ls1021atwr board to use it.
> > > >
> > > > This depends on previous DM series for ls1021atwr:
> > > > http://patchwork.ozlabs.org/patch/561855/
> > > >
> > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > >
> > > Generally looks good, but a few nits below...
> > >
> > > Acked-by: Joe Hershberger <joe.hershberger@ni.com>
> > >
> > > > [Vladimir] Made the following changes:
> > > > - Added 'status = "disabled";' for all Ethernet ports in ls1021a.dtsi
> > > > - Fixed the confusion between the SGMII/TBI PCS for enet0 and enet1 -
> > > >   a mistake ported over from Linux. Each SGMII PCS lies on the private
> > > >   MDIO bus of the interface (and the RGMII enet2 has no SGMII PCS).
> > > >
> > > > Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
> > > > ---
> > > >  arch/arm/cpu/armv7/ls102xa/cpu.c        |  2 +-
> > > >  arch/arm/cpu/armv7/ls102xa/fdt.c        | 10 ++++++++
> > > >  arch/arm/dts/ls1021a-twr.dtsi           | 32 +++++++++++++++++++++++++
> > > >  arch/arm/dts/ls1021a.dtsi               | 28 ++++++++++++++++++++--
> > > >  board/freescale/ls1021atwr/ls1021atwr.c |  2 +-
> > > >  configs/ls1021atwr_nor_defconfig        |  1 +
> > > >  configs/ls1021atwr_nor_lpuart_defconfig |  1 +
> > > >  include/configs/ls1021atwr.h            |  4 ++++
> > > >  8 files changed, 76 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/arch/arm/cpu/armv7/ls102xa/cpu.c b/arch/arm/cpu/armv7/ls102xa/cpu.c
> > > > index ecf9e869855e..9ccfe1042ce5 100644
> > > > --- a/arch/arm/cpu/armv7/ls102xa/cpu.c
> > > > +++ b/arch/arm/cpu/armv7/ls102xa/cpu.c
> > > > @@ -296,7 +296,7 @@ int cpu_mmc_init(bd_t *bis)
> > > >
> > > >  int cpu_eth_init(bd_t *bis)
> > > >  {
> > > > -#ifdef CONFIG_TSEC_ENET
> > > > +#if defined(CONFIG_TSEC_ENET) && !defined(CONFIG_DM_ETH)
> > > >         tsec_standard_init(bis);
> > > >  #endif
> > > >
> > > > diff --git a/arch/arm/cpu/armv7/ls102xa/fdt.c b/arch/arm/cpu/armv7/ls102xa/fdt.c
> > > > index 8bf9c42b2260..90cf7958f257 100644
> > > > --- a/arch/arm/cpu/armv7/ls102xa/fdt.c
> > > > +++ b/arch/arm/cpu/armv7/ls102xa/fdt.c
> > > > @@ -16,12 +16,17 @@
> > > >  #include <tsec.h>
> > > >  #include <asm/arch/immap_ls102xa.h>
> > > >  #include <fsl_sec.h>
> > > > +#include <dm.h>
> > > >
> > > >  DECLARE_GLOBAL_DATA_PTR;
> > > >
> > > >  void ft_fixup_enet_phy_connect_type(void *fdt)
> > > >  {
> > > > +#ifndef CONFIG_DM_ETH
> > >
> > > Please use positive logic where convenient. I.e. #ifdef CONFIG_DM_ETH
> > > and swap cases.
> > >
> >
> > To be honest I don't know why keep compatibility with non-DM ETH at
> > all for the TWR board. On the LS1021A-TSN I'm not doing that.
> > Bin, is there any particular reason? If not, I'll just completely
> > remove your #ifdef's for v2.
>
> I remember at that time there were some PowerPC 83xx/85xx boards that
> were not converted to DM but still used TSEC driver. If this is not a
> concern now, I am all for it to remove the non-DM TSEC support.
>

For the TSEC driver code, sure, there are still non-DM users out there
and we're still keeping them for now.
I was talking about the board code for LS1021A-TWR when I replied to Joe.

> Regards,
> Bin

Thanks,
-Vladimir
Bin Meng July 15, 2019, 2:08 a.m. UTC | #5 
On Sun, Jul 14, 2019 at 6:04 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Sun, 14 Jul 2019 at 04:55, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Vladimir,
> >
> > On Sat, Jul 13, 2019 at 5:39 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > >
> > > Hi Joe,
> > >
> > > On Fri, 12 Jul 2019 at 23:46, Joe Hershberger <joe.hershberger@ni.com> wrote:
> > > >
> > > > On Sun, Jun 23, 2019 at 12:53 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > > > >
> > > > > From: Bin Meng <bmeng.cn@gmail.com>
> > > > >
> > > > > Now that we have added driver model support to the TSEC driver,
> > > > > convert ls1021atwr board to use it.
> > > > >
> > > > > This depends on previous DM series for ls1021atwr:
> > > > > http://patchwork.ozlabs.org/patch/561855/
> > > > >
> > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > >
> > > > Generally looks good, but a few nits below...
> > > >
> > > > Acked-by: Joe Hershberger <joe.hershberger@ni.com>
> > > >
> > > > > [Vladimir] Made the following changes:
> > > > > - Added 'status = "disabled";' for all Ethernet ports in ls1021a.dtsi
> > > > > - Fixed the confusion between the SGMII/TBI PCS for enet0 and enet1 -
> > > > >   a mistake ported over from Linux. Each SGMII PCS lies on the private
> > > > >   MDIO bus of the interface (and the RGMII enet2 has no SGMII PCS).
> > > > >
> > > > > Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
> > > > > ---
> > > > >  arch/arm/cpu/armv7/ls102xa/cpu.c        |  2 +-
> > > > >  arch/arm/cpu/armv7/ls102xa/fdt.c        | 10 ++++++++
> > > > >  arch/arm/dts/ls1021a-twr.dtsi           | 32 +++++++++++++++++++++++++
> > > > >  arch/arm/dts/ls1021a.dtsi               | 28 ++++++++++++++++++++--
> > > > >  board/freescale/ls1021atwr/ls1021atwr.c |  2 +-
> > > > >  configs/ls1021atwr_nor_defconfig        |  1 +
> > > > >  configs/ls1021atwr_nor_lpuart_defconfig |  1 +
> > > > >  include/configs/ls1021atwr.h            |  4 ++++
> > > > >  8 files changed, 76 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/arch/arm/cpu/armv7/ls102xa/cpu.c b/arch/arm/cpu/armv7/ls102xa/cpu.c
> > > > > index ecf9e869855e..9ccfe1042ce5 100644
> > > > > --- a/arch/arm/cpu/armv7/ls102xa/cpu.c
> > > > > +++ b/arch/arm/cpu/armv7/ls102xa/cpu.c
> > > > > @@ -296,7 +296,7 @@ int cpu_mmc_init(bd_t *bis)
> > > > >
> > > > >  int cpu_eth_init(bd_t *bis)
> > > > >  {
> > > > > -#ifdef CONFIG_TSEC_ENET
> > > > > +#if defined(CONFIG_TSEC_ENET) && !defined(CONFIG_DM_ETH)
> > > > >         tsec_standard_init(bis);
> > > > >  #endif
> > > > >
> > > > > diff --git a/arch/arm/cpu/armv7/ls102xa/fdt.c b/arch/arm/cpu/armv7/ls102xa/fdt.c
> > > > > index 8bf9c42b2260..90cf7958f257 100644
> > > > > --- a/arch/arm/cpu/armv7/ls102xa/fdt.c
> > > > > +++ b/arch/arm/cpu/armv7/ls102xa/fdt.c
> > > > > @@ -16,12 +16,17 @@
> > > > >  #include <tsec.h>
> > > > >  #include <asm/arch/immap_ls102xa.h>
> > > > >  #include <fsl_sec.h>
> > > > > +#include <dm.h>
> > > > >
> > > > >  DECLARE_GLOBAL_DATA_PTR;
> > > > >
> > > > >  void ft_fixup_enet_phy_connect_type(void *fdt)
> > > > >  {
> > > > > +#ifndef CONFIG_DM_ETH
> > > >
> > > > Please use positive logic where convenient. I.e. #ifdef CONFIG_DM_ETH
> > > > and swap cases.
> > > >
> > >
> > > To be honest I don't know why keep compatibility with non-DM ETH at
> > > all for the TWR board. On the LS1021A-TSN I'm not doing that.
> > > Bin, is there any particular reason? If not, I'll just completely
> > > remove your #ifdef's for v2.
> >
> > I remember at that time there were some PowerPC 83xx/85xx boards that
> > were not converted to DM but still used TSEC driver. If this is not a
> > concern now, I am all for it to remove the non-DM TSEC support.
> >
>
> For the TSEC driver code, sure, there are still non-DM users out there
> and we're still keeping them for now.
> I was talking about the board code for LS1021A-TWR when I replied to Joe.
>

For LS1021A-TWR board codes, I agree there is no need to keep non-DM stuff.

Regards,
Bin
diff mbox series

Patch

diff --git a/arch/arm/cpu/armv7/ls102xa/cpu.c b/arch/arm/cpu/armv7/ls102xa/cpu.c
index ecf9e869855e..9ccfe1042ce5 100644
--- a/arch/arm/cpu/armv7/ls102xa/cpu.c
+++ b/arch/arm/cpu/armv7/ls102xa/cpu.c
@@ -296,7 +296,7 @@  int cpu_mmc_init(bd_t *bis)
 
 int cpu_eth_init(bd_t *bis)
 {
-#ifdef CONFIG_TSEC_ENET
+#if defined(CONFIG_TSEC_ENET) && !defined(CONFIG_DM_ETH)
 	tsec_standard_init(bis);
 #endif
 
diff --git a/arch/arm/cpu/armv7/ls102xa/fdt.c b/arch/arm/cpu/armv7/ls102xa/fdt.c
index 8bf9c42b2260..90cf7958f257 100644
--- a/arch/arm/cpu/armv7/ls102xa/fdt.c
+++ b/arch/arm/cpu/armv7/ls102xa/fdt.c
@@ -16,12 +16,17 @@ 
 #include <tsec.h>
 #include <asm/arch/immap_ls102xa.h>
 #include <fsl_sec.h>
+#include <dm.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
 void ft_fixup_enet_phy_connect_type(void *fdt)
 {
+#ifndef CONFIG_DM_ETH
 	struct eth_device *dev;
+#else
+	struct udevice *dev;
+#endif
 	struct tsec_private *priv;
 	const char *enet_path, *phy_path;
 	char enet[16];
@@ -29,7 +34,12 @@  void ft_fixup_enet_phy_connect_type(void *fdt)
 	int phy_node;
 	int i = 0;
 	uint32_t ph;
+#ifndef CONFIG_DM_ETH
 	char *name[3] = { "eTSEC1", "eTSEC2", "eTSEC3" };
+#else
+	char *name[3] = { "ethernet@2d10000", "ethernet@2d50000",
+			  "ethernet@2d90000" };
+#endif
 
 	for (; i < ARRAY_SIZE(name); i++) {
 		dev = eth_get_dev_by_name(name[i]);
diff --git a/arch/arm/dts/ls1021a-twr.dtsi b/arch/arm/dts/ls1021a-twr.dtsi
index 5d3275ced913..27c96f95400a 100644
--- a/arch/arm/dts/ls1021a-twr.dtsi
+++ b/arch/arm/dts/ls1021a-twr.dtsi
@@ -51,6 +51,26 @@ 
 	};
 };
 
+&enet0 {
+	tbi-handle = <&tbi0>;
+	phy-handle = <&sgmii_phy2>;
+	phy-connection-type = "sgmii";
+	status = "okay";
+};
+
+&enet1 {
+	tbi-handle = <&tbi1>;
+	phy-handle = <&sgmii_phy0>;
+	phy-connection-type = "sgmii";
+	status = "okay";
+};
+
+&enet2 {
+	phy-handle = <&rgmii_phy1>;
+	phy-connection-type = "rgmii-id";
+	status = "okay";
+};
+
 &i2c0 {
 	status = "okay";
 };
@@ -84,12 +104,24 @@ 
 	sgmii_phy0: ethernet-phy@0 {
 		reg = <0x0>;
 	};
+
 	rgmii_phy1: ethernet-phy@1 {
 		reg = <0x1>;
 	};
+
 	sgmii_phy2: ethernet-phy@2 {
 		reg = <0x2>;
 	};
+
+	/* SGMII PCS for enet0 */
+	tbi0: tbi-phy@1f {
+		reg = <0x1f>;
+		device_type = "tbi-phy";
+	};
+};
+
+&mdio1 {
+	/* SGMII PCS for enet1 */
 	tbi1: tbi-phy@1f {
 		reg = <0x1f>;
 		device_type = "tbi-phy";
diff --git a/arch/arm/dts/ls1021a.dtsi b/arch/arm/dts/ls1021a.dtsi
index 8a0f473e25ca..c274a302d358 100644
--- a/arch/arm/dts/ls1021a.dtsi
+++ b/arch/arm/dts/ls1021a.dtsi
@@ -350,14 +350,38 @@ 
 				 <&platform_clk 1>;
 		};
 
+		enet0: ethernet@2d10000 {
+			compatible = "fsl,tsec";
+			reg = <0x2d10000 0x1000>;
+			status = "disabled";
+		};
+
+		enet1: ethernet@2d50000 {
+			compatible = "fsl,tsec";
+			reg = <0x2d50000 0x1000>;
+			status = "disabled";
+		};
+
+		enet2: ethernet@2d90000 {
+			compatible = "fsl,tsec";
+			reg = <0x2d90000 0x1000>;
+			status = "disabled";
+		};
+
 		mdio0: mdio@2d24000 {
-			compatible = "gianfar";
-			device_type = "mdio";
+			compatible = "fsl,tsec-mdio";
 			#address-cells = <1>;
 			#size-cells = <0>;
 			reg = <0x2d24000 0x4000>;
 		};
 
+		mdio1: mdio@2d64000 {
+			compatible = "fsl,tsec-mdio";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x2d64000 0x4000>;
+		};
+
 		usb@8600000 {
 			compatible = "fsl-usb2-dr-v2.5", "fsl-usb2-dr";
 			reg = <0x8600000 0x1000>;
diff --git a/board/freescale/ls1021atwr/ls1021atwr.c b/board/freescale/ls1021atwr/ls1021atwr.c
index 01ba1bc96213..a4ff0b7bc7eb 100644
--- a/board/freescale/ls1021atwr/ls1021atwr.c
+++ b/board/freescale/ls1021atwr/ls1021atwr.c
@@ -248,7 +248,7 @@  int board_mmc_init(bd_t *bis)
 
 int board_eth_init(bd_t *bis)
 {
-#ifdef CONFIG_TSEC_ENET
+#if defined(CONFIG_TSEC_ENET) && !defined(CONFIG_DM_ETH)
 	struct fsl_pq_mdio_info mdio_info;
 	struct tsec_info_struct tsec_info[4];
 	int num = 0;
diff --git a/configs/ls1021atwr_nor_defconfig b/configs/ls1021atwr_nor_defconfig
index 9d8c2024c04e..3c9cf9a8c909 100644
--- a/configs/ls1021atwr_nor_defconfig
+++ b/configs/ls1021atwr_nor_defconfig
@@ -40,6 +40,7 @@  CONFIG_SYS_FLASH_CFI=y
 CONFIG_PHY_GIGE=y
 CONFIG_E1000=y
 CONFIG_MII=y
+CONFIG_DM_ETH=y
 CONFIG_TSEC_ENET=y
 CONFIG_PCI=y
 CONFIG_DM_PCI=y
diff --git a/configs/ls1021atwr_nor_lpuart_defconfig b/configs/ls1021atwr_nor_lpuart_defconfig
index b9cfdb6fd69e..762af87b0dd3 100644
--- a/configs/ls1021atwr_nor_lpuart_defconfig
+++ b/configs/ls1021atwr_nor_lpuart_defconfig
@@ -42,6 +42,7 @@  CONFIG_SYS_FLASH_CFI=y
 CONFIG_PHY_GIGE=y
 CONFIG_E1000=y
 CONFIG_MII=y
+CONFIG_DM_ETH=y
 CONFIG_TSEC_ENET=y
 CONFIG_PCI=y
 CONFIG_DM_PCI=y
diff --git a/include/configs/ls1021atwr.h b/include/configs/ls1021atwr.h
index de0c9c7f26af..c967be3a6fce 100644
--- a/include/configs/ls1021atwr.h
+++ b/include/configs/ls1021atwr.h
@@ -260,6 +260,7 @@ 
  */
 
 #ifdef CONFIG_TSEC_ENET
+#ifndef CONFIG_DM_ETH
 #define CONFIG_MII_DEFAULT_TSEC		1
 #define CONFIG_TSEC1			1
 #define CONFIG_TSEC1_NAME		"eTSEC1"
@@ -287,6 +288,9 @@ 
 #define CONFIG_HAS_ETH0
 #define CONFIG_HAS_ETH1
 #define CONFIG_HAS_ETH2
+#else
+#define CONFIG_ETHPRIME			"ethernet@2d10000"
+#endif
 #endif
 
 /* PCIe */