Hi Peppe, On Tue, May 31, 2016 at 9:52 PM, Giuseppe CAVALLARO <peppe.cavall...@st.com> wrote: > Hello Loh Tien > > On 5/31/2016 11:10 AM, Loh Tien Hock wrote: >> >> Hi Peppe, >> >> Sorry for the late reply. > > > no pbl at all. > >> >> I believe my patch's title is a little confusing. The patch is to >> enable Altera TSE PCS SGMII support, not to add SGMII support to >> stmmac (well that in a way tests SGMII for stmmac though) > > > ok > >> I went through the code and it seems like if we were to refactor PCS >> defines (and codes) into common code it would bloat up the common >> codes. >> Would it be better if we create new PCS source files (eg. >> altr_tse_pcs.c/.h)? This means the rest of the PCS supports should >> also be refactored out as well for consistency purposes. > > > yes agree.
OK let me refactor the PCS codes out. Will send a v2 patch. I'll update the devicetree binding docs in patch v2. > > Just an info to better understand the case; after enabling the SGMII on > Altera, do you test in some way the PCS SGMII mode in the main layer? Are you referring to the codes in stmmac_main.c? If so, no, the TSE PCS doesn't exercise the codes there. > > Regards > Peppe > > >> >> Thanks >> >> On Fri, May 13, 2016 at 3:47 PM, Giuseppe CAVALLARO >> <peppe.cavall...@st.com> wrote: >>> >>> Hello Tien >>> >>> On 5/13/2016 9:01 AM, th...@altera.com wrote: >>>> >>>> >>>> From: Tien Hock Loh <th...@altera.com> >>>> >>>> Adds SGMII support for dwmac-socfpga to enable the SGMII PHY when >>>> phy-mode >>>> of the dwmac is set to sgmii. >>> >>> >>> >>> I wonder if part of this patch can be unified to the common code >>> reusing (or improving) existent PCS support (see, for example the >>> defines inside the common.h and dwmac1000.h header files). >>> I had added some code for RGMII but never tested the SGMII indeed. >>> >>> what is your feeling about that? >>> >>> Peppe >>> >>> >>>> >>>> Signed-off-by: Tien Hock Loh <th...@altera.com> >>>> --- >>>> .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 339 >>>> ++++++++++++++++++++- >>>> 1 file changed, 329 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>> index 41f4c58..a59d590 100644 >>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>> @@ -40,6 +40,43 @@ >>>> #define EMAC_SPLITTER_CTRL_SPEED_100 0x3 >>>> #define EMAC_SPLITTER_CTRL_SPEED_1000 0x0 >>>> >>>> +#define TSE_PCS_CONTROL_AN_EN_MASK 0x1000 >>>> +#define TSE_PCS_CONTROL_REG 0x00 >>>> +#define TSE_PCS_CONTROL_RESTART_AN_MASK 0x0200 >>>> +#define TSE_PCS_IF_MODE_REG 0x28 >>>> +#define TSE_PCS_LINK_TIMER_0_REG 0x24 >>>> +#define TSE_PCS_LINK_TIMER_1_REG 0x26 >>>> +#define TSE_PCS_SIZE 0x40 >>>> +#define TSE_PCS_STATUS_AN_COMPLETED_MASK 0x0020 >>>> +#define TSE_PCS_STATUS_LINK_MASK 0x0004 >>>> +#define TSE_PCS_STATUS_REG 0x02 >>>> +#define TSE_PCS_SGMII_SPEED_1000 0x8 >>>> +#define TSE_PCS_SGMII_SPEED_100 0x4 >>>> +#define TSE_PCS_SGMII_SPEED_10 0x0 >>>> +#define TSE_PCS_SW_RST_MASK 0x8000 >>>> +#define TSE_PCS_PARTNER_ABILITY_REG 0x0A >>>> +#define TSE_PCS_PARTNER_DUPLEX_FULL 0x1000 >>>> +#define TSE_PCS_PARTNER_DUPLEX_HALF 0x0000 >>>> +#define TSE_PCS_PARTNER_DUPLEX_MASK 0x1000 >>>> +#define TSE_PCS_PARTNER_SPEED_MASK 0x0c00 >>>> +#define TSE_PCS_PARTNER_SPEED_1000 0x0800 >>>> +#define TSE_PCS_PARTNER_SPEED_100 0x0400 >>>> +#define TSE_PCS_PARTNER_SPEED_10 0x0000 >>>> +#define TSE_PCS_PARTNER_SPEED_1000 0x0800 >>>> +#define TSE_PCS_PARTNER_SPEED_100 0x0400 >>>> +#define TSE_PCS_PARTNER_SPEED_10 0x0000 >>>> +#define TSE_PCS_SGMII_SPEED_MASK 0x000C >>>> +#define TSE_PCS_SGMII_LINK_TIMER_0 0x0D40 >>>> +#define TSE_PCS_SGMII_LINK_TIMER_1 0x0003 >>>> +#define TSE_PCS_SW_RESET_TIMEOUT 100 >>>> +#define TSE_PCS_USE_SGMII_AN_MASK 0x0002 >>>> + >>>> +#define SGMII_ADAPTER_CTRL_REG 0x00 >>>> +#define SGMII_ADAPTER_DISABLE 0x0001 >>>> +#define SGMII_ADAPTER_ENABLE 0x0000 >>>> +#define LINK_TIMER 20 >>>> +#define AUTONEGO_TIMER 20 >>>> + >>>> struct socfpga_dwmac { >>>> int interface; >>>> u32 reg_offset; >>>> @@ -49,18 +86,17 @@ struct socfpga_dwmac { >>>> struct reset_control *stmmac_rst; >>>> void __iomem *splitter_base; >>>> bool f2h_ptp_ref_clk; >>>> + void __iomem *tse_pcs_base; >>>> + void __iomem *sgmii_adapter_base; >>>> + struct timer_list an_timer; >>>> + struct timer_list link_timer; >>>> }; >>>> >>>> -static void socfpga_dwmac_fix_mac_speed(void *priv, unsigned int speed) >>>> +static void config_emac_splitter_speed(void __iomem *base, unsigned int >>>> speed) >>>> { >>>> - struct socfpga_dwmac *dwmac = (struct socfpga_dwmac *)priv; >>>> - void __iomem *splitter_base = dwmac->splitter_base; >>>> u32 val; >>>> >>>> - if (!splitter_base) >>>> - return; >>>> - >>>> - val = readl(splitter_base + EMAC_SPLITTER_CTRL_REG); >>>> + val = readl(base + EMAC_SPLITTER_CTRL_REG); >>>> val &= ~EMAC_SPLITTER_CTRL_SPEED_MASK; >>>> >>>> switch (speed) { >>>> @@ -76,8 +112,214 @@ static void socfpga_dwmac_fix_mac_speed(void *priv, >>>> unsigned int speed) >>>> default: >>>> return; >>>> } >>>> + writel(val, base + EMAC_SPLITTER_CTRL_REG); >>>> +} >>>> + >>>> +static void config_tx_buffer(u16 data, void __iomem *base) >>>> +{ >>>> + writew(data, base + SGMII_ADAPTER_CTRL_REG); >>>> +} >>>> + >>>> +static void tse_pcs_reset(void __iomem *base, struct socfpga_dwmac >>>> *dwmac) >>>> +{ >>>> + int counter = 0; >>>> + u16 val; >>>> + >>>> + val = readw(base + TSE_PCS_CONTROL_REG); >>>> + val |= TSE_PCS_SW_RST_MASK; >>>> + writew(val, base + TSE_PCS_CONTROL_REG); >>>> + >>>> + while (counter < TSE_PCS_SW_RESET_TIMEOUT) { >>>> + val = readw(base + TSE_PCS_CONTROL_REG); >>>> + val &= TSE_PCS_SW_RST_MASK; >>>> + if (val == 0) >>>> + break; >>>> + counter++; >>>> + udelay(1); >>>> + } >>>> + if (counter >= TSE_PCS_SW_RESET_TIMEOUT) >>>> + dev_err(dwmac->dev, "PCS could not get out of sw >>>> reset\n"); >>>> +} >>>> + >>>> +static void tse_pcs_init(void __iomem *base, struct socfpga_dwmac >>>> *dwmac) >>>> +{ >>>> + writew(0x0001, base + TSE_PCS_IF_MODE_REG); >>>> + >>>> + writew(TSE_PCS_SGMII_LINK_TIMER_0, base + >>>> TSE_PCS_LINK_TIMER_0_REG); >>>> + writew(TSE_PCS_SGMII_LINK_TIMER_1, base + >>>> TSE_PCS_LINK_TIMER_1_REG); >>>> + >>>> + tse_pcs_reset(base, dwmac); >>>> +} >>>> + >>>> +static void pcs_link_timer_callback(unsigned long data) >>>> +{ >>>> + u16 val = 0; >>>> + >>>> + struct socfpga_dwmac *dwmac = (struct socfpga_dwmac *)data; >>>> + void __iomem *tse_pcs_base = dwmac->tse_pcs_base; >>>> + void __iomem *sgmii_adapter_base = dwmac->sgmii_adapter_base; >>>> + >>>> + val = readw(tse_pcs_base + TSE_PCS_STATUS_REG); >>>> + val &= TSE_PCS_STATUS_LINK_MASK; >>>> + >>>> + if (val != 0) { >>>> + dev_dbg(dwmac->dev, "Adapter: Link is established\n"); >>>> + config_tx_buffer(SGMII_ADAPTER_ENABLE, >>>> sgmii_adapter_base); >>>> + } else { >>>> + mod_timer(&dwmac->link_timer, jiffies + >>>> + msecs_to_jiffies(LINK_TIMER)); >>>> + } >>>> +} >>>> + >>>> +static void auto_nego_timer_callback(unsigned long data) >>>> +{ >>>> + u16 val = 0; >>>> + u16 speed = 0; >>>> + u16 duplex = 0; >>>> + >>>> + struct socfpga_dwmac *dwmac = (struct socfpga_dwmac *)data; >>>> + void __iomem *tse_pcs_base = dwmac->tse_pcs_base; >>>> + void __iomem *sgmii_adapter_base = dwmac->sgmii_adapter_base; >>>> + >>>> + val = readw(tse_pcs_base + TSE_PCS_STATUS_REG); >>>> + val &= TSE_PCS_STATUS_AN_COMPLETED_MASK; >>>> + >>>> + if (val != 0) { >>>> + dev_dbg(dwmac->dev, "Adapter: Auto Negotiation is >>>> completed\n"); >>>> + val = readw(tse_pcs_base + TSE_PCS_PARTNER_ABILITY_REG); >>>> + speed = val & TSE_PCS_PARTNER_SPEED_MASK; >>>> + duplex = val & TSE_PCS_PARTNER_DUPLEX_MASK; >>>> + >>>> + if (speed == TSE_PCS_PARTNER_SPEED_10 && >>>> + duplex == TSE_PCS_PARTNER_DUPLEX_FULL) >>>> + dev_dbg(dwmac->dev, >>>> + "Adapter: Link Partner is Up - >>>> 10/Full\n"); >>>> + else if (speed == TSE_PCS_PARTNER_SPEED_100 && >>>> + duplex == TSE_PCS_PARTNER_DUPLEX_FULL) >>>> + dev_dbg(dwmac->dev, >>>> + "Adapter: Link Partner is Up - >>>> 100/Full\n"); >>>> + else if (speed == TSE_PCS_PARTNER_SPEED_1000 && >>>> + duplex == TSE_PCS_PARTNER_DUPLEX_FULL) >>>> + dev_dbg(dwmac->dev, >>>> + "Adapter: Link Partner is Up - >>>> 1000/Full\n"); >>>> + else if (speed == TSE_PCS_PARTNER_SPEED_10 && >>>> + duplex == TSE_PCS_PARTNER_DUPLEX_HALF) >>>> + dev_err(dwmac->dev, >>>> + "Adapter does not support Half >>>> Duplex\n"); >>>> + else if (speed == TSE_PCS_PARTNER_SPEED_100 && >>>> + duplex == TSE_PCS_PARTNER_DUPLEX_HALF) >>>> + dev_err(dwmac->dev, >>>> + "Adapter does not support Half >>>> Duplex\n"); >>>> + else if (speed == TSE_PCS_PARTNER_SPEED_1000 && >>>> + duplex == TSE_PCS_PARTNER_DUPLEX_HALF) >>>> + dev_err(dwmac->dev, >>>> + "Adapter does not support Half >>>> Duplex\n"); >>>> + else >>>> + dev_err(dwmac->dev, >>>> + "Adapter: Invalid Partner Speed and >>>> Duplex\n"); >>>> + >>>> + config_tx_buffer(SGMII_ADAPTER_ENABLE, >>>> sgmii_adapter_base); >>>> + } else { >>>> + val = readw(tse_pcs_base + TSE_PCS_CONTROL_REG); >>>> + val |= TSE_PCS_CONTROL_RESTART_AN_MASK; >>>> + writew(val, tse_pcs_base + TSE_PCS_CONTROL_REG); >>>> + >>>> + tse_pcs_reset(tse_pcs_base, dwmac); >>>> + mod_timer(&dwmac->an_timer, jiffies + >>>> + msecs_to_jiffies(AUTONEGO_TIMER)); >>>> + } >>>> +} >>>> + >>>> +static void socfpga_dwmac_fix_mac_speed(void *priv, unsigned int speed) >>>> +{ >>>> + struct socfpga_dwmac *dwmac = (struct socfpga_dwmac *)priv; >>>> + void __iomem *splitter_base = dwmac->splitter_base; >>>> + void __iomem *tse_pcs_base = dwmac->tse_pcs_base; >>>> + void __iomem *sgmii_adapter_base = dwmac->sgmii_adapter_base; >>>> + struct device *dev = dwmac->dev; >>>> + struct net_device *ndev = dev_get_drvdata(dev); >>>> + struct phy_device *phy_dev = ndev->phydev; >>>> + u32 val; >>>> >>>> - writel(val, splitter_base + EMAC_SPLITTER_CTRL_REG); >>>> + if ((tse_pcs_base) && (sgmii_adapter_base)) { >>>> + config_tx_buffer(SGMII_ADAPTER_DISABLE, >>>> sgmii_adapter_base); >>>> + if (splitter_base) >>>> + config_emac_splitter_speed(splitter_base, >>>> speed); >>>> + >>>> + if (phy_dev->autoneg == AUTONEG_ENABLE) { >>>> + val = readw(tse_pcs_base + TSE_PCS_CONTROL_REG); >>>> + val |= TSE_PCS_CONTROL_AN_EN_MASK; >>>> + writew(val, tse_pcs_base + TSE_PCS_CONTROL_REG); >>>> + >>>> + val = readw(tse_pcs_base + TSE_PCS_IF_MODE_REG); >>>> + val |= TSE_PCS_USE_SGMII_AN_MASK; >>>> + writew(val, tse_pcs_base + TSE_PCS_IF_MODE_REG); >>>> + >>>> + val = readw(tse_pcs_base + TSE_PCS_CONTROL_REG); >>>> + val |= TSE_PCS_CONTROL_RESTART_AN_MASK; >>>> + writew(val, tse_pcs_base + TSE_PCS_CONTROL_REG); >>>> + >>>> + tse_pcs_reset(tse_pcs_base, dwmac); >>>> + >>>> + setup_timer(&dwmac->an_timer, >>>> + auto_nego_timer_callback, >>>> + (unsigned long)dwmac); >>>> + mod_timer(&dwmac->an_timer, jiffies + >>>> + msecs_to_jiffies(AUTONEGO_TIMER)); >>>> + } else if (phy_dev->autoneg == AUTONEG_DISABLE) { >>>> + val = readw(tse_pcs_base + TSE_PCS_CONTROL_REG); >>>> + val &= ~TSE_PCS_CONTROL_AN_EN_MASK; >>>> + writew(val, tse_pcs_base + TSE_PCS_CONTROL_REG); >>>> + >>>> + val = readw(tse_pcs_base + TSE_PCS_IF_MODE_REG); >>>> + val &= ~TSE_PCS_USE_SGMII_AN_MASK; >>>> + writew(val, tse_pcs_base + TSE_PCS_IF_MODE_REG); >>>> + >>>> + val = readw(tse_pcs_base + TSE_PCS_IF_MODE_REG); >>>> + val &= ~TSE_PCS_SGMII_SPEED_MASK; >>>> + >>>> + switch (speed) { >>>> + case 1000: >>>> + val |= TSE_PCS_SGMII_SPEED_1000; >>>> + break; >>>> + case 100: >>>> + val |= TSE_PCS_SGMII_SPEED_100; >>>> + break; >>>> + case 10: >>>> + val |= TSE_PCS_SGMII_SPEED_10; >>>> + break; >>>> + default: >>>> + return; >>>> + } >>>> + writew(val, tse_pcs_base + TSE_PCS_IF_MODE_REG); >>>> + >>>> + tse_pcs_reset(tse_pcs_base, dwmac); >>>> + >>>> + setup_timer(&dwmac->link_timer, >>>> + pcs_link_timer_callback, >>>> + (unsigned long)dwmac); >>>> + mod_timer(&dwmac->link_timer, jiffies + >>>> + msecs_to_jiffies(LINK_TIMER)); >>>> + } >>>> + } else if (splitter_base) { >>>> + val = readl(splitter_base + EMAC_SPLITTER_CTRL_REG); >>>> + val &= ~EMAC_SPLITTER_CTRL_SPEED_MASK; >>>> + >>>> + switch (speed) { >>>> + case 1000: >>>> + val |= EMAC_SPLITTER_CTRL_SPEED_1000; >>>> + break; >>>> + case 100: >>>> + val |= EMAC_SPLITTER_CTRL_SPEED_100; >>>> + break; >>>> + case 10: >>>> + val |= EMAC_SPLITTER_CTRL_SPEED_10; >>>> + break; >>>> + default: >>>> + return; >>>> + } >>>> + writel(val, splitter_base + EMAC_SPLITTER_CTRL_REG); >>>> + } >>>> } >>>> >>>> static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, >>>> struct >>>> device *dev) >>>> @@ -85,9 +327,13 @@ static int socfpga_dwmac_parse_data(struct >>>> socfpga_dwmac *dwmac, struct device * >>>> struct device_node *np = dev->of_node; >>>> struct regmap *sys_mgr_base_addr; >>>> u32 reg_offset, reg_shift; >>>> - int ret; >>>> - struct device_node *np_splitter; >>>> + int ret, index; >>>> + struct device_node *np_splitter = NULL; >>>> + struct device_node *np_sgmii_adapter = NULL; >>>> + >>>> struct resource res_splitter; >>>> + struct resource res_tse_pcs; >>>> + struct resource res_sgmii_adapter; >>>> >>>> dwmac->stmmac_rst = devm_reset_control_get(dev, >>>> STMMAC_RESOURCE_NAME); >>>> @@ -134,6 +380,72 @@ static int socfpga_dwmac_parse_data(struct >>>> socfpga_dwmac *dwmac, struct device * >>>> } >>>> } >>>> >>>> + np_sgmii_adapter = of_parse_phandle(np, >>>> + >>>> "altr,gmii_to_sgmii_converter", 0); >>>> + if (np_sgmii_adapter) { >>>> + index = of_property_match_string(np_sgmii_adapter, >>>> "reg-names", >>>> + >>>> "hps_emac_interface_splitter_avalon_slave"); >>>> + >>>> + if (index >= 0) { >>>> + if (of_address_to_resource(np_sgmii_adapter, >>>> index, >>>> + &res_splitter)) { >>>> + dev_err(dev, "%s: ERROR: missing emac >>>> splitter " >>>> + "address\n", __func__); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + dwmac->splitter_base = devm_ioremap_resource( >>>> + dev, &res_splitter); >>>> + >>>> + if (IS_ERR(dwmac->splitter_base)) { >>>> + dev_err(dev, "%s: ERROR: failed to >>>> mapping >>>> emac" >>>> + " splitter\n", __func__); >>>> + return PTR_ERR(dwmac->splitter_base); >>>> + } >>>> + } >>>> + >>>> + index = of_property_match_string(np_sgmii_adapter, >>>> "reg-names", >>>> + >>>> "gmii_to_sgmii_adapter_avalon_slave"); >>>> + >>>> + if (index >= 0) { >>>> + if (of_address_to_resource(np_sgmii_adapter, >>>> index, >>>> + &res_sgmii_adapter)) >>>> { >>>> + dev_err(dev, "%s: ERROR: failed to >>>> mapping >>>> adapter\n", >>>> + __func__); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + dwmac->sgmii_adapter_base = >>>> devm_ioremap_resource( >>>> + dev, &res_sgmii_adapter); >>>> + >>>> + if (IS_ERR(dwmac->sgmii_adapter_base)) { >>>> + dev_err(dev, "%s: failed to mapping >>>> adapter\n", >>>> + __func__); >>>> + return >>>> PTR_ERR(dwmac->sgmii_adapter_base); >>>> + } >>>> + } >>>> + >>>> + index = of_property_match_string(np_sgmii_adapter, >>>> "reg-names", >>>> + >>>> "eth_tse_control_port"); >>>> + >>>> + if (index >= 0) { >>>> + if (of_address_to_resource(np_sgmii_adapter, >>>> index, >>>> + &res_tse_pcs)) { >>>> + dev_err(dev, "%s: ERROR: failed mapping >>>> tse control port\n", >>>> + __func__); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + dwmac->tse_pcs_base = devm_ioremap_resource( >>>> + dev, &res_tse_pcs); >>>> + >>>> + if (IS_ERR(dwmac->tse_pcs_base)) { >>>> + dev_err(dev, "%s: failed to mapping tse >>>> control port\n", >>>> + __func__); >>>> + return >>>> PTR_ERR(dwmac->sgmii_adapter_base); >>>> + } >>>> + } >>>> + } >>>> dwmac->reg_offset = reg_offset; >>>> dwmac->reg_shift = reg_shift; >>>> dwmac->sys_mgr_base_addr = sys_mgr_base_addr; >>>> @@ -157,6 +469,7 @@ static int socfpga_dwmac_setup(struct socfpga_dwmac >>>> *dwmac) >>>> break; >>>> case PHY_INTERFACE_MODE_MII: >>>> case PHY_INTERFACE_MODE_GMII: >>>> + case PHY_INTERFACE_MODE_SGMII: >>>> val = SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_GMII_MII; >>>> break; >>>> default: >>>> @@ -181,6 +494,12 @@ static int socfpga_dwmac_setup(struct socfpga_dwmac >>>> *dwmac) >>>> ctrl &= ~(SYSMGR_EMACGRP_CTRL_PTP_REF_CLK_MASK << >>>> (reg_shift / 2)); >>>> >>>> regmap_write(sys_mgr_base_addr, reg_offset, ctrl); >>>> + >>>> + if (phymode == PHY_INTERFACE_MODE_SGMII) { >>>> + tse_pcs_init(dwmac->tse_pcs_base, dwmac); >>>> + config_tx_buffer(SGMII_ADAPTER_ENABLE, >>>> + dwmac->sgmii_adapter_base); >>>> + } >>>> return 0; >>>> } >>>> >>>> >>> >> >