Hello Peppe,
On 1/15/2013 11:28 PM, Giuseppe CAVALLARO write:On 1/15/2013 10:45 PM, Byungho An wrote:verified
This patch adds gmac autoneg set function for SGMII, TBI, or RTBI
interface. In case of PHY's autoneg is set, gmac's autoneg enable bit
should set. After checking phy's autoneg if phydev's autoneg is '1'
gmac's ANE bit set for those interface.
Sorry I've some doubts about these patches.
Firstly if the MAC is able to manage RGMII/SGMII etc this should beby looking at the HW cap register: i.e. PCS bit.I agree with you and actually I tried to use a PCS bit previously.
(I have no HW that support this so I cannot do any tests).
(PCS bit indicates TBI, SGMII, or RTBI PHY interface)
But I was confused which one I should use among PSC, ACTPHYIF or phydev to
recognize the interface.
At this time, I want to add auto-negotiation enable because sgmii interface
is not working without ANE using PCS bit(in H/W feature register).
In case of this feature is actually supported then the driver could managethe
everything bypassing the MDIO.
IMO, we could not need to call the stmmac_phy_init and we should not use
the PHYLIB.
I mean if we have the PCS module we could have a minimal support to get
link status, restart ANE etc w/o using at all the PHYLIB (so w/o touching
PHY regs via the MDIO/MDC).Yes. For example SGMII/RGMII/SMII Status Register is useful to know status
so we can use this register to manage status of SGMII/RGMII/SMII
It could also be useful to complete the support with the RGMII... noUnfortunately I have only the SGMII.
extra effort should be needed while adding SGMII if you look at the
core registers.
If you add the RGMII on some platforms we could guarantee to manage
the fix_mac_speed (see stmmac doc).
Looking at the other your patches, the ethtool support is not completed. IWhat is link property? It means link up, speed and duplex mode.
expected to restart ANE, get/set link property etc.
Are there anything else?
I think get link property is already working using ethtool.
In case of SGMII/RGMII/SMII, I think we can use SGMII/RGMII/SMII Status
Register for link status.
I have a question regarding it.
In the mainline code, there is hardcode interrupt mask in
dwmac1000_core_init function.
When I try to get interrupt for link change, interrupt is not occurred
because of this mask.
Can we modify that?
Also pay attention to properly treat EEE. Maybe, as first stage we shouldOK. As I understood, using gmac register instead of phydev or phylib...
disable the feature in this case. We will see it later.
The question is that we could not be able to use some extra features that
currently need to dialog more with the PHY device. I mean what we actually
do by using PHYLIB.
I agree. In my opinion gmac and phy communicate using mdio each other, after
that gmac can get updated status.
...
+++++++++++
Signed-off-by: Byungho An <bh74.an@xxxxxxxxxxx>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 1 +
drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 11
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 9 +++++++++__iomem
3 files changed, 21 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h
b/drivers/net/ethernet/stmicro/stmmac/common.h
index 186d148..72ba769 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -344,6 +344,7 @@ struct stmmac_ops {
void (*reset_eee_mode) (void __iomem *ioaddr);
void (*set_eee_timer) (void __iomem *ioaddr, int ls, int tw);
void (*set_eee_pls) (void __iomem *ioaddr, int link);
+ void (*set_autoneg) (void __iomem *ioaddr);
};
struct mac_link {
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index bfe0226..a0737b39 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -297,6 +297,16 @@ static void dwmac1000_set_eee_timer(void
*ioaddr, int ls, int tw)
writel(value, ioaddr + LPI_TIMER_CTRL);
}
+static void dwmac1000_set_autoneg(void __iomem *ioaddr) {
+ u32 value;
+
+ value = readl(ioaddr + GMAC_AN_CTRL);
+ value |= 0x1000;
pls use define instead of raw values ... see below.
+ writel(value, ioaddr + GMAC_AN_CTRL); }
+
+
static const struct stmmac_ops dwmac1000_ops = {
.core_init = dwmac1000_core_init,
.rx_ipc = dwmac1000_rx_ipc_enable,
@@ -311,6 +321,7 @@ static const struct stmmac_ops dwmac1000_ops = {
.reset_eee_mode = dwmac1000_reset_eee_mode,
.set_eee_timer = dwmac1000_set_eee_timer,
.set_eee_pls = dwmac1000_set_eee_pls,
+ .set_autoneg = dwmac1000_set_autoneg,
};
struct mac_device_info *dwmac1000_setup(void __iomem *ioaddr) diff
--git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f07c061..3e28934 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1007,6 +1007,7 @@ static int stmmac_open(struct net_device *dev)
{
struct stmmac_priv *priv = netdev_priv(dev);
int ret;
+ int interface = priv->plat->interface;
clk_prepare_enable(priv->stmmac_clk);
@@ -1041,6 +1042,14 @@ static int stmmac_open(struct net_device *dev)
/* Initialize the MAC Core */
priv->hw->mac->core_init(priv->ioaddr);
+ /* If phy autoneg is on, set gmac autoneg for SGMII, TBI and RTBI*/
+ if (interface == PHY_INTERFACE_MODE_SGMII ||
+ interface == PHY_INTERFACE_MODE_TBI ||
+ interface == PHY_INTERFACE_MODE_RTBI) {
+ if (priv->phydev->autoneg)
+ priv->hw->mac->set_autoneg(priv->ioaddr);
+ }
we could use the following instead of priv->hw->mac->set_autoneg:
static void dwmac1000_ctrl_ane(void __iomem *ioaddr, bool restart) {
int value = GMAC_CTRL_ANE_EN;
if (restart)
value |= GMAC_CTRL_ANE_RESTART;
writel(value, ioaddr +GMAC_AN_CTRL); }
where we should defines all the missing macros for the registers 48, 49
Actually I tested this code for restart autonegotiation, but even though I
changed link (1Gb -> 100Mb) phy's link is not changed.
For more detail, first time 1Gb I changed the link during system working and
then check speed using ethtool/mii-tool.
I think phy also should do autonegotiation (I'm not sure this is just for
the SGMII). Without phy auto negotiation gmac is not working.
OK, I'll try to check functionality and make macros.
Anyway as a first step, I want to complete to the code regarding ANE (ctrl
function, macros, ethtool support for link status etc..)
I'll send new code before making patch soon.
info to
/* RGMI/SGMII defines */
#define GMAC_CTRL_ANE_SGMII_RAL (1 << 18)
#define GMAC_CTRL_ANE_EN (1 << 12)
#define GMAC_CTRL_ANE_RESTART (1 << 9)
The handler should store the link status that will be used to pass thethe ethtool for example. We need to manage speed and duplex etc.So far, I can get right data speed and duplex and autoneg.
What happens if you run "ethtool eth0" command?
Or if you run mii-tool?
I expect to get the right speed and duplex at least.
what the
Concerning the stmmac_set_pauseparam I'm also not sure you are doing the
right thing. Note that you are not restarting the ANE at all ... (this is
phy_start_aneg does). If set the bit 12 then you are enabling the ane. ToOK, thank you and happy new year.
restart it, you need to set the bit 9 in the reg 48.
I can support you on all the points above. Let me know.
BR,
peppe
Byungho An
+
/* Request the IRQ lines */
ret = request_irq(dev->irq, stmmac_interrupt,
IRQF_SHARED, dev->name, dev);