[U-Boot] [PATCH v3] net: phy: add TSE PCS support to dwmac-socfpga
Ooi, Joyce
joyce.ooi at intel.com
Fri Feb 1 03:45:44 UTC 2019
> -----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?
>
> --
> Best regards,
> Marek Vasut
More information about the U-Boot
mailing list