[U-Boot] [PATCH v3] net: phy: add TSE PCS support to dwmac-socfpga

Ooi, Joyce joyce.ooi at intel.com
Fri Jan 25 14:35:59 UTC 2019


> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Friday, January 4, 2019 10:56 PM
> To: Ooi, Joyce <joyce.ooi at intel.com>; Joe Hershberger
> <joe.hershberger at ni.com>
> Cc: See, Chin Liang <chin.liang.see at intel.com>; Ong, Hean Loong
> <hean.loong.ong at intel.com>; Priyanka Jain <priyanka.jain at nxp.com>; u-
> boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH v3] net: phy: add TSE PCS support to dwmac-
> socfpga
> 
> On 1/4/19 10:26 AM, Ooi, Joyce wrote:
> >> -----Original Message-----
> >> From: Marek Vasut [mailto:marex at denx.de]
> >> Sent: Friday, December 28, 2018 6:08 PM
> >> To: Ooi, Joyce <joyce.ooi at intel.com>; Joe Hershberger
> >> <joe.hershberger at ni.com>
> >> Cc: See, Chin Liang <chin.liang.see at intel.com>; Ong, Hean Loong
> >> <hean.loong.ong at intel.com>; Priyanka Jain <priyanka.jain at nxp.com>; u-
> >> boot at lists.denx.de
> >> Subject: Re: [U-Boot] [PATCH v3] net: phy: add TSE PCS support to
> >> dwmac- socfpga
> >>
> >> On 12/28/18 8:28 AM, Ooi, Joyce wrote:
> >>>> -----Original Message-----
> >>>> From: Marek Vasut [mailto:marex at denx.de]
> >>>> Sent: Thursday, December 27, 2018 2:55 AM
> >>>> To: Ooi, Joyce <joyce.ooi at intel.com>; Joe Hershberger
> >>>> <joe.hershberger at ni.com>
> >>>> Cc: See, Chin Liang <chin.liang.see at intel.com>; Ong, Hean Loong
> >>>> <hean.loong.ong at intel.com>; Priyanka Jain <priyanka.jain at nxp.com>;
> >>>> u- boot at lists.denx.de
> >>>> Subject: Re: [U-Boot] [PATCH v3] net: phy: add TSE PCS support to
> >>>> dwmac- socfpga
> >>>>
> >>>> On 12/26/18 8:47 AM, Ooi, Joyce wrote:
> >>>>> Adding Marek.
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of
> >>>>>> Ooi, Joyce
> >>>>>> Sent: Tuesday, November 27, 2018 5:40 PM
> >>>>>> To: Joe Hershberger <joe.hershberger at ni.com>
> >>>>>> Cc: Ong, Hean Loong <hean.loong.ong at intel.com>; Priyanka Jain
> >>>>>> <priyanka.jain at nxp.com>; See, Chin Liang
> >>>>>> <chin.liang.see at intel.com>;
> >>>>>> u- boot at lists.denx.de
> >>>>>> Subject: Re: [U-Boot] [PATCH v3] net: phy: add TSE PCS support to
> >>>>>> dwmac- socfpga
> >>>>>>
> >>>>>> Hi Joe,
> >>>>>>
> >>>>>> Any comments/feedback on this v3 patch?
> >>>>
> >>>> I thought we already had TSE support in drivers/net/altera_tse.c ,
> >>>> is this related ?
> >>>>
> >>>>>> Thanks.
> >>>>>>
> >>>>>> Regards,
> >>>>>> Joyce Ooi
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: Ooi, Joyce
> >>>>>>> Sent: Friday, November 9, 2018 6:16 PM
> >>>>>>> To: Joe Hershberger <joe.hershberger at ni.com>
> >>>>>>> Cc: Grygorii Strashko <grygorii.strashko at ti.com>; Neil Armstrong
> >>>>>>> <narmstrong at baylibre.com>; Mario Six <mario.six at gdsys.cc>;
> >>>>>>> Florian Fainelli <f.fainelli at gmail.com>; Priyanka Jain
> >>>>>>> <priyanka.jain at nxp.com>; Ooi, Joyce <joyce.ooi at intel.com>; See,
> >>>>>>> Chin Liang <chin.liang.see at intel.com>; Ong, Hean Loong
> >>>>>>> <hean.loong.ong at intel.com>; u-boot at lists.denx.de
> >>>>>>> Subject: [PATCH v3] net: phy: add TSE PCS support to
> >>>>>>> dwmac-socfpga
> >>>>>>>
> >>>>>>> This adds support for TSE PCS (Triple Speed Ethernet Physical
> >>>>>>> Coding
> >>>>>>> Sublayer) that uses SGMII adapter when the phy-mode in device
> >>>>>>> tree is set to sgmii.
> >>>>>>>
> >>>>>>> Signed-off-by: Ooi, Joyce <joyce.ooi at intel.com>
> >>>>>>> ---
> >>>>>>>  arch/arm/mach-socfpga/include/mach/misc.h |    3 +
> >>>>>>>  arch/arm/mach-socfpga/misc_s10.c          |    6 +
> >>>>>>>  drivers/net/Makefile                      |    3 +-
> >>>>>>>  drivers/net/designware.c                  |    5 +
> >>>>>>>  drivers/net/designware.h                  |    1 +
> >>>>>>>  drivers/net/dwmac_socfpga.c               |  122 +++++++++++++++++++
> >>>>>>>  drivers/net/phy/altr_tse_pcs.c            |  184
> >>>>>> +++++++++++++++++++++++++++++
> >>>>>>>  drivers/net/phy/altr_tse_pcs.h            |   56 +++++++++
> >>>>>>>  8 files changed, 379 insertions(+), 1 deletions(-)  create mode
> >>>>>>> 100644 drivers/net/phy/altr_tse_pcs.c  create mode 100644
> >>>>>>> drivers/net/phy/altr_tse_pcs.h
> >>>>>>> ---
> >>>>>>> v2: add a __weak function to make it compatible for all socfpga
> >>>>>>> platforms
> >>>>>>> v3: remove __weak function and use board-specific implementation
> >>>>>>> instead
> >>>>>>>
> >>>>>>> diff --git a/arch/arm/mach-socfpga/include/mach/misc.h
> >>>>>>> b/arch/arm/mach- socfpga/include/mach/misc.h index
> >>>>>>> 4fc9570..751705e
> >>>>>>> 100644
> >>>>>>> --- a/arch/arm/mach-socfpga/include/mach/misc.h
> >>>>>>> +++ b/arch/arm/mach-socfpga/include/mach/misc.h
> >>>>>>> @@ -30,6 +30,9 @@ void socfpga_init_security_policies(void);
> >>>>>>>  void socfpga_sdram_remap_zero(void);  #endif
> >>>>>>>
> >>>>>>> +#ifdef CONFIG_TARGET_SOCFPGA_STRATIX10 int
> >>>>>>> +socfpga_test_fpga_ready(void); #endif
> >>>>>>>  void do_bridge_reset(int enable);
> >>>>>>>
> >>>>>>>  #endif /* _MISC_H_ */
> >>>>>>> diff --git a/arch/arm/mach-socfpga/misc_s10.c b/arch/arm/mach-
> >>>>>>> socfpga/misc_s10.c index e599362..3cd9c30 100644
> >>>>>>> --- a/arch/arm/mach-socfpga/misc_s10.c
> >>>>>>> +++ b/arch/arm/mach-socfpga/misc_s10.c
> >>>>>>> @@ -11,6 +11,7 @@
> >>>>>>>  #include <miiphy.h>
> >>>>>>>  #include <netdev.h>
> >>>>>>>  #include <asm/io.h>
> >>>>>>> +#include <asm/arch/mailbox_s10.h>
> >>>>>>>  #include <asm/arch/reset_manager.h>  #include
> >>>>>>> <asm/arch/system_manager.h>  #include <asm/arch/misc.h> @@ -
> 91,6
> >>>>>>> +92,11 @@ static int socfpga_set_phymode(void)
> >>>>>>>
> >>>>>>>  	return 0;
> >>>>>>>  }
> >>>>>>> +
> >>>>>>> +int socfpga_test_fpga_ready(void) {
> >>>>>>> +	return mbox_get_fpga_config_status(MBOX_CONFIG_STATUS);
> >>>>>>> +}
> >>>>>>>  #else
> >>>>>>>  static int socfpga_set_phymode(void)  { diff --git
> >>>>>>> a/drivers/net/Makefile b/drivers/net/Makefile index
> >>>>>>> 48a2878..c2333b5
> >>>>>>> 100644
> >>>>>>> --- a/drivers/net/Makefile
> >>>>>>> +++ b/drivers/net/Makefile
> >>>>>>> @@ -14,7 +14,7 @@ obj-$(CONFIG_CALXEDA_XGMAC) +=
> >> calxedaxgmac.o
> >>>>>>>  obj-$(CONFIG_CS8900) += cs8900.o
> >>>>>>>  obj-$(CONFIG_TULIP) += dc2114x.o
> >>>>>>>  obj-$(CONFIG_ETH_DESIGNWARE) += designware.o
> >>>>>>> -obj-$(CONFIG_ETH_DESIGNWARE_SOCFPGA) += dwmac_socfpga.o
> >>>>>>> +obj-$(CONFIG_ETH_DESIGNWARE_SOCFPGA) += dwmac-socfpga.o
> >>>>>>>  obj-$(CONFIG_DRIVER_DM9000) += dm9000x.o
> >>>>>>>  obj-$(CONFIG_DNET) += dnet.o
> >>>>>>>  obj-$(CONFIG_E1000) += e1000.o
> >>>>>>> @@ -73,3 +73,4 @@ obj-$(CONFIG_PIC32_ETH) += pic32_mdio.o
> >>>>>>> pic32_eth.o
> >>>>>>>  obj-$(CONFIG_DWC_ETH_QOS) += dwc_eth_qos.o
> >>>>>>>  obj-$(CONFIG_FSL_PFE) += pfe_eth/
> >>>>>>>  obj-$(CONFIG_SNI_AVE) += sni_ave.o
> >>>>>>> +dwmac-socfpga-objs := phy/altr_tse_pcs.o dwmac_socfpga.o
> >>>>>>> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> >>>>>>> index
> >>>>>>> 19db0a8..666cf41 100644
> >>>>>>> --- a/drivers/net/designware.c
> >>>>>>> +++ b/drivers/net/designware.c
> >>>>>>> @@ -235,6 +235,8 @@ static int dw_adjust_link(struct dw_eth_dev
> >>>>>>> *priv, struct eth_mac_regs *mac_p,
> >>>>>>>  			  struct phy_device *phydev)  {
> >>>>>>>  	u32 conf = readl(&mac_p->conf) | FRAMEBURSTENABLE |
> >>>>>> DISABLERXOWN;
> >>>>>>> +	struct udevice *dev = priv->phydev->dev;
> >>>>>>> +	struct dw_eth_pdata *dw_pdata = dev_get_platdata(dev);
> >>>>>>>
> >>>>>>>  	if (!phydev->link) {
> >>>>>>>  		printf("%s: No link.\n", phydev->dev->name); @@ -
> 254,6
> >>>>>>> +256,9 @@ static int dw_adjust_link(struct dw_eth_dev *priv,
> >>>>>>> +struct
> >>>>>>> eth_mac_regs *mac_p,
> >>>>>>>
> >>>>>>>  	writel(conf, &mac_p->conf);
> >>>>>>>
> >>>>>>> +	if (dw_pdata->pcs_adjust_link)
> >>>>>>> +		dw_pdata->pcs_adjust_link(dev, phydev);
> >>>>>>> +
> >>>>>>>  	printf("Speed: %d, %s duplex%s\n", phydev->speed,
> >>>>>>>  	       (phydev->duplex) ? "full" : "half",
> >>>>>>>  	       (phydev->port == PORT_FIBRE) ? ", fiber mode" : "");
> >>>>>>> diff --git a/drivers/net/designware.h b/drivers/net/designware.h
> >>>>>>> index dea12b7..3a5a93f
> >>>>>>> 100644
> >>>>>>> --- a/drivers/net/designware.h
> >>>>>>> +++ b/drivers/net/designware.h
> >>>>>>> @@ -255,6 +255,7 @@ extern const struct eth_ops
> >>>>>>> designware_eth_ops; struct dw_eth_pdata {
> >>>>>>>  	struct eth_pdata eth_pdata;
> >>>>>>>  	u32 reset_delays[3];
> >>>>>>> +	void (*pcs_adjust_link)(struct udevice *dev, struct phy_device
> >>>>>>> +*phydev);
> >>>>>>>  };
> >>>>>>>
> >>>>>>>  int designware_eth_init(struct dw_eth_dev *priv, u8 *enetaddr);
> >>>>>>> diff --git a/drivers/net/dwmac_socfpga.c
> >>>>>>> b/drivers/net/dwmac_socfpga.c index 08fc967..7d13f3c 100644
> >>>>>>> --- a/drivers/net/dwmac_socfpga.c
> >>>>>>> +++ b/drivers/net/dwmac_socfpga.c
> >>>>>>> @@ -9,12 +9,16 @@
> >>>>>>>  #include <asm/io.h>
> >>>>>>>  #include <dm.h>
> >>>>>>>  #include <clk.h>
> >>>>>>> +#include <fdt_support.h>
> >>>>>>>  #include <phy.h>
> >>>>>>>  #include <regmap.h>
> >>>>>>>  #include <reset.h>
> >>>>>>>  #include <syscon.h>
> >>>>>>>  #include "designware.h"
> >>>>>>> +#include "phy/altr_tse_pcs.h"
> >>>>>>>
> >>>>>>> +#include <asm/arch/misc.h>
> >>>>>>> +#include <asm/arch/reset_manager.h>
> >>>>>>>  #include <asm/arch/system_manager.h>
> >>>>>>>
> >>>>>>>  enum dwmac_type {
> >>>>>>> @@ -27,8 +31,123 @@ struct dwmac_socfpga_platdata {
> >>>>>>>  	struct dw_eth_pdata	dw_eth_pdata;
> >>>>>>>  	enum dwmac_type		type;
> >>>>>>>  	void			*phy_intf;
> >>>>>>> +	struct tse_pcs		pcs;
> >>>>>>>  };
> >>>>>>>
> >>>>>>> +static void socfpga_tse_pcs_adjust_link(struct udevice *dev,
> >>>>>>> +					struct phy_device *phydev) {
> >>>>>>> +	struct dwmac_socfpga_platdata *pdata =
> dev_get_platdata(dev);
> >>>>>>> +	phys_addr_t tse_pcs_base = pdata->pcs.tse_pcs_base;
> >>>>>>> +	phys_addr_t sgmii_adapter_base =
> >>>>>>> +pdata->pcs.sgmii_adapter_base;
> >>>>>>> +
> >>>>>>> +	if ((tse_pcs_base) && (sgmii_adapter_base))
> >>>>>>> +		writew(SGMII_ADAPTER_DISABLE,
> >>>>>>> +		       sgmii_adapter_base + SGMII_ADAPTER_CTRL_REG);
> >>>>>>> +
> >>>>>>> +	if ((tse_pcs_base) && (sgmii_adapter_base))
> >>>>>>> +		tse_pcs_fix_mac_speed(&pdata->pcs, phydev, phydev-
> >speed);
> >>>>>>> }
> >>>>>>> +
> >>>>>>> +static int socfpga_dw_tse_pcs_init(struct udevice *dev) {
> >>>>>>> +	struct dwmac_socfpga_platdata *pdata =
> dev_get_platdata(dev);
> >>>>>>> +	struct dw_eth_pdata *dw_pdata = &pdata->dw_eth_pdata;
> >>>>>>> +	struct eth_pdata *eth_pdata = &pdata-
> >dw_eth_pdata.eth_pdata;
> >>>>>>> +	phys_addr_t tse_pcs_base = pdata->pcs.tse_pcs_base;
> >>>>>>> +	phys_addr_t sgmii_adapter_base = pdata-
> >pcs.sgmii_adapter_base;
> >>>>>>> +	const fdt32_t *reg;
> >>>>>>> +	int ret = 0;
> >>>>>>> +	int parent, index, na, ns, offset, len;
> >>>>>>> +
> >>>>>>> +	offset = fdtdec_lookup_phandle(gd->fdt_blob,
> dev_of_offset(dev),
> >>>>>>> +				       "altr,gmii-to-sgmii-converter");
> >>>>>>> +	if (offset > 0) {
> >>>>>>> +#ifdef CONFIG_TARGET_SOCFPGA_STRATIX10
> >>>>>>> +		/* check FPGA status */
> >>>>>>> +		ret = socfpga_test_fpga_ready();
> >>>>>>> +		if (ret) {
> >>>>>>> +			debug("%s: FPGA not ready (%d)\n", __func__,
> ret);
> >>>>>>> +			return -EPERM;
> >>>>>>> +		}
> >>>>>>> +#endif
> >>>>>>> +		/* enable HPS bridge */
> >>>>>>> +		do_bridge_reset(1);
> >>>>
> >>>> Why is generic ethernet driver touching FPGA bridges ?
> >>> This is because the TSE PCS IP is in FPGA whereas DWMAC is in hard
> processor.
> >>> So, in order for DWMAC SGMII to access to TSE PCS, the FPGA bridge
> >>> needs to be enabled.
> >>
> >> This is the wrong place to enable them, that should be handled by the
> >> FPGA manager driver. If you model this correctly in the DT, then this
> >> hack shouldn't be needed here.
> > In that case, would it be better if I move it to the u-boot init
> > sequence? I'll do a checking if FPGA is in usermode - if yes, then
> > I'll enable the bridge. Then, no FPGA codes will need to appear in dwmac
> driver.
> 
> This stuff should be entirely in drivers/fpga/* , if the FPGA gets loaded (thus,
> enters usermode) AND a driver gets bound to anything in the FPGA bridge area,
> then the bridge needs to be enabled.
The drivers/fpga/* does not seem to use the driver model. They're like individual
APIs that get called by others. So, I'm not sure how this can be done in FPGA driver.

> 
> >> btw. if I use nios2 softcore with this block, and altera TSE
> >> ethernet, I suspect I won't need the bridge enable at all ?
> > Yes, you are right. So, if the bridge enable is done during the u-boot
> > init sequence based on board specific, bridge will not be enabled when nios2 is
> used with TSE PCS.
> 
> This doesn't fit into the driver model though. And you cannot tell which bridge
> you need to enable. However, the FPGA driver does/should know that.
> 
> --
> Best regards,
> Marek Vasut


More information about the U-Boot mailing list