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

Marek Vasut marex at denx.de
Fri Feb 1 08:21:10 UTC 2019


On 2/1/19 4:45 AM, Ooi, Joyce wrote:
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Saturday, January 26, 2019 4:34 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; Tan, Ley Foon <ley.foon.tan at intel.com>; Chee, Tien Fong
>> <tien.fong.chee at intel.com>
>> Subject: Re: [U-Boot] [PATCH v3] net: phy: add TSE PCS support to dwmac-
>> socfpga
>>
>> On 1/25/19 3:35 PM, Ooi, Joyce wrote:
>>>> -----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.
>>
>> CCing Chee, he's been working on the altera FPGA code recently, I hope he can
>> give you some hints. We might need a new API for this, possibly add DM support
>> to FPGA framework (should be easy) and then let the FPGA framework handle
>> the bridges and stuff. Really, this shouldn't be in the network code.
> 
> Since this isn't related to FPGA drivers, I think it's better to have a separate driver for
> the bridge. I can create a bridge driver, perhaps a new drivers/bridge/*. It'll consist of
> a uclass driver, which has new APIs to enable bridge, disable bridge, and check for
> bridge status. The bridge enablement will be based on device tree setting and utilizes
> reset manager APIs. I'll also create a bridge-socfpga driver that implements the bridge
> enablement.
> 
> With this, the appropriate bridge will be enabled when the bridge driver is loaded.
> 
> What do you think?
Can we model this so the framework looks similar to Linux FPGA manager ?

-- 
Best regards,
Marek Vasut


More information about the U-Boot mailing list