Re: [PATCH 1/1] net: ethernet: Add SGMII support to dwmac-socfpga

From: Loh Tien Hock
Date: Tue May 31 2016 - 05:11:23 EST


Hi Peppe,

Sorry for the late reply.

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)

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.

Thanks

On Fri, May 13, 2016 at 3:47 PM, Giuseppe CAVALLARO
<peppe.cavallaro@xxxxxx> wrote:
> Hello Tien
>
> On 5/13/2016 9:01 AM, thloh@xxxxxxxxxx wrote:
>>
>> From: Tien Hock Loh <thloh@xxxxxxxxxx>
>>
>> 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 <thloh@xxxxxxxxxx>
>> ---
>> .../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;
>> }
>>
>>
>