Re: [PATCH net-next 08/18] ravb: Add R-Car common features
From: Sergei Shtylyov <hidden>
Date: 2021-07-27 20:49:05
Also in:
linux-renesas-soc
Hello! On 7/22/21 5:13 PM, Biju Das wrote:
The below features are supported by both R-Car Gen2 and Gen3. 1) magic packet detection 2) multiple TSRQ support 3) extended descriptor in rx
I think this one should better be called timestamping...
4) No half duplex support
Couldn't we avoid the "negative" features?
5) override mtu change
Hm, I'd vote for the individual patches covering only single feature...
quoted hunk ↗ jump to hunk
Add features bits to support the same. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- drivers/net/ethernet/renesas/ravb_main.c | 110 +++++++++++++++-------- 1 file changed, 71 insertions(+), 39 deletions(-)diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index b3c99f974632..4ef2565534d2 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
quoted hunk ↗ jump to hunk
@@ -680,11 +694,14 @@ static void ravb_rcv_snd_enable(struct net_device *ndev) /* function for waiting dma process finished */ static int ravb_stop_dma(struct net_device *ndev) { + struct ravb_private *priv = netdev_priv(ndev); + const struct ravb_drv_data *info = priv->info; int error; /* Wait for stopping the hardware TX process */ - error = ravb_wait(ndev, TCCR, - TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, 0); + if (info->features & RAVB_MULTI_TSRQ) + error = ravb_wait(ndev, TCCR, + TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, 0); if (error)
What if the above *if* skips the ravb_wait() call -- didn't you get a complaint from gcc about the unnintialized variable? [...]
quoted hunk ↗ jump to hunk
@@ -808,11 +826,14 @@ static bool ravb_queue_interrupt(struct net_device *ndev, int q) static bool ravb_timestamp_interrupt(struct net_device *ndev) { + struct ravb_private *priv = netdev_priv(ndev); + const struct ravb_drv_data *info = priv->info; u32 tis = ravb_read(ndev, TIS); if (tis & TIS_TFUF) { ravb_write(ndev, ~(TIS_TFUF | TIS_RESERVED), TIS); - ravb_get_tx_tstamp(ndev); + if (info->features & RAVB_EX_RX_DESC)
Yeah, definitely a bad feature name...
+ ravb_get_tx_tstamp(ndev); return true; } return false;
[...]
quoted hunk ↗ jump to hunk
@@ -1069,15 +1091,17 @@ static int ravb_phy_init(struct net_device *ndev) netdev_info(ndev, "limited PHY to 100Mbit/s\n"); } - /* 10BASE, Pause and Asym Pause is not supported */ - phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT); - phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT); - phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT); - phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Asym_Pause_BIT); + if (info->features & RAVB_NO_HALF_DUPLEX) { + /* 10BASE, Pause and Asym Pause is not supported */ + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT); + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT); + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT); + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Asym_Pause_BIT); - /* Half Duplex is not supported */ - phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT); - phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT); + /* Half Duplex is not supported */ + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT); + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
Mhm? Some of the half-duplex modes sre unsupported still?
[...]quoted hunk ↗ jump to hunk
@@ -1314,8 +1338,9 @@ static void ravb_get_wol(struct net_device *ndev, struct ethtool_wolinfo *wol) static int ravb_set_wol(struct net_device *ndev, struct ethtool_wolinfo *wol) { struct ravb_private *priv = netdev_priv(ndev); + const struct ravb_drv_data *info = priv->info; - if (wol->wolopts & ~WAKE_MAGIC) + if ((wol->wolopts & ~WAKE_MAGIC) || (!(info->features & RAVB_MAGIC)))
Parens about !x not needed. And I think the second operand should come first instead...
return -EOPNOTSUPP; priv->wol_enabled = !!(wol->wolopts & WAKE_MAGIC);
[...]
quoted hunk ↗ jump to hunk
@@ -1595,28 +1621,30 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev) desc->dptr = cpu_to_le32(dma_addr); /* TX timestamp required */ - if (q == RAVB_NC) { - ts_skb = kmalloc(sizeof(*ts_skb), GFP_ATOMIC); - if (!ts_skb) { - if (num_tx_desc > 1) { - desc--; - dma_unmap_single(ndev->dev.parent, dma_addr, - len, DMA_TO_DEVICE); + if (info->features & RAVB_EX_RX_DESC) {
Definitely a bad name... [...]
quoted hunk ↗ jump to hunk
@@ -2205,8 +2235,10 @@ static int ravb_probe(struct platform_device *pdev) } clk_prepare_enable(priv->refclk); - ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN); - ndev->min_mtu = ETH_MIN_MTU; + if (info->features & RAVB_OVERRIDE_MTU_CHANGE) {
Why? :-/ Could you tell me more details?
+ ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN); + ndev->min_mtu = ETH_MIN_MTU; + } priv->num_tx_desc = info->num_tx_desc;
MBR, Sergei