[PATCH v5 net-next 2/4] sh_eth: Add support for r7s72100
From: Sergei Shtylyov <hidden>
Date: 2014-01-15 22:26:11
Also in:
linux-sh, netdev
Hello. On 01/15/2014 09:12 AM, Simon Horman wrote:
This is a fast ethernet controller.
I have to say it's not exact enough patch description: R7S72100 is not
Ethernet controller itself, it's a SoC containing the Ethernet controller.
Signed-off-by: Simon Horman <redacted>
---
v5
* As suggested by Sergei Shtylyov
- Do not use sh_eth_chip_reset_r8a7740 as it accesses non-existent
RMII registers. Instead use sh_eth_chip_reset.
- Do not use sh_eth_set_rate_gether as it accesses non-existent registers.
- Do not use reserved LCHNG bit of ECSR
- Do not use reserved LCHNGIP bit of ECSIPR
- Document that R8A779x also needs a 16 bit shift of the RFS bits
- Do not document that the R7S72100 has GECMR, it does notThe above change list was moved from v2 section and doesn't match the real changes done in v5. ;-)
v4 * As requested by David Miller - Use a boolean for the return value of sh_eth_is_rz_fast_ether() - Correct coding style in sh_eth_get_stats()
v3 * No change
v2 * As suggested by Magnus Damm and Sergei Shtylyov - r7s72100 ethernet is not gigabit so do not refer to it as such
* As suggested by Magnus Damm
- As RZ specific register layout rather than using the gigabit layout
which includes registers that do not exist on this chip.
---
drivers/net/ethernet/renesas/sh_eth.c | 126 ++++++++++++++++++++++++++++++++--
drivers/net/ethernet/renesas/sh_eth.h | 3 +-
2 files changed, 121 insertions(+), 8 deletions(-)quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c index 4f5cfad..a7a0555 100644 --- a/drivers/net/ethernet/renesas/sh_eth.c +++ b/drivers/net/ethernet/renesas/sh_eth.c@@ -190,6 +190,64 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = { [TRIMD] = 0x027c, }; +static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {
Shouldn't this map precede R-Car one since this SoC is newer the same way
you've reordered *enum* values, etc.? Sorry for not noticing in the previous
review...
[...]+ [ARSTR] = 0x0000, + [TSU_CTRST] = 0x0004, + [TSU_VTAG0] = 0x0058, + [TSU_ADSBSY] = 0x0060, + [TSU_TEN] = 0x0064, + [TXNLCR0] = 0x0080, + [TXALCR0] = 0x0084, + [RXNLCR0] = 0x0088, + [RXALCR0] = 0x008C,
Well, the above counter register subgroup stands out from the TSU_*
registers in the Gigabit mapping, not sure if we should follow that. These
registers are not currently used anyway...
quoted hunk ↗ jump to hunk
+ [TSU_ADRH0] = 0x0100, + [TSU_ADRL0] = 0x0104, + [TSU_ADRH31] = 0x01f8, + [TSU_ADRL31] = 0x01fc, +}; + static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = { [ECMR] = 0x0100, [RFLR] = 0x0108,@@ -318,6 +376,14 @@ static bool sh_eth_is_gether(struct sh_eth_private *mdp) return false; } +static bool sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp) +{ + if (mdp->reg_offset == sh_eth_offset_fast_rz) + return true; + else + return false;
Perhaps you should compress the above functions to one-liners as Joe has
suggested. Or I/you could do it in a separate patch...
+}
+
static void sh_eth_select_mii(struct net_device *ndev)
{
u32 value = 0x0;[...]
quoted hunk ↗ jump to hunk
@@ -1309,9 +1409,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota) /* In case of almost all GETHER/ETHERs, the Receive Frame State * (RFS) bits in the Receive Descriptor 0 are from bit 9 to - * bit 0. However, in case of the R8A7740's GETHER, the RFS - * bits are from bit 25 to bit 16. So, the driver needs right - * shifting by 16. + * bit 0. However, in case of the R8A7740, R8A779x and
Small nit: comma needed before "and" as far as I know English grammar.
+ * R7S72100 the RFS bits are from bit 25 to bit 16. So, the + * driver needs right shifting by 16. */ if (mdp->cd->shift_rd0) desc_status >>= 16;
Other than that, this looks fine now, you can add my:
Acked-by: Sergei Shtylyov <redacted>
WBR, Sergei