On 1/4/19 10:26 AM, Ooi, Joyce wrote:
>> -----Original Message-----
>> From: Marek Vasut [mailto:ma...@denx.de]
>> Sent: Friday, December 28, 2018 6:08 PM
>> To: Ooi, Joyce <joyce....@intel.com>; Joe Hershberger
>> <joe.hershber...@ni.com>
>> Cc: See, Chin Liang <chin.liang....@intel.com>; Ong, Hean Loong
>> <hean.loong....@intel.com>; Priyanka Jain <priyanka.j...@nxp.com>; u-
>> b...@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:ma...@denx.de]
>>>> Sent: Thursday, December 27, 2018 2:55 AM
>>>> To: Ooi, Joyce <joyce....@intel.com>; Joe Hershberger
>>>> <joe.hershber...@ni.com>
>>>> Cc: See, Chin Liang <chin.liang....@intel.com>; Ong, Hean Loong
>>>> <hean.loong....@intel.com>; Priyanka Jain <priyanka.j...@nxp.com>; u-
>>>> b...@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-boun...@lists.denx.de] On Behalf Of
>>>>>> Ooi, Joyce
>>>>>> Sent: Tuesday, November 27, 2018 5:40 PM
>>>>>> To: Joe Hershberger <joe.hershber...@ni.com>
>>>>>> Cc: Ong, Hean Loong <hean.loong....@intel.com>; Priyanka Jain
>>>>>> <priyanka.j...@nxp.com>; See, Chin Liang
>>>>>> <chin.liang....@intel.com>;
>>>>>> u- b...@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.hershber...@ni.com>
>>>>>>> Cc: Grygorii Strashko <grygorii.stras...@ti.com>; Neil Armstrong
>>>>>>> <narmstr...@baylibre.com>; Mario Six <mario....@gdsys.cc>; Florian
>>>>>>> Fainelli <f.faine...@gmail.com>; Priyanka Jain
>>>>>>> <priyanka.j...@nxp.com>; Ooi, Joyce <joyce....@intel.com>; See,
>>>>>>> Chin Liang <chin.liang....@intel.com>; Ong, Hean Loong
>>>>>>> <hean.loong....@intel.com>; u-boot@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....@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.

>> 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to