| Message ID | 7b64c7c2-b51b-7526-e65e-3648f98fef5c@cogentembedded.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] sh_eth: TSU_QTAG0/1 registers the same as TSU_QTAGM0/1 | expand |
Hi Sergei, On Sat, Feb 24, 2018 at 6:28 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > The TSU_QTAG0/1 registers found in the Gigabit Ether controllers actually > have the same long name as the TSU_QTAGM0/1 registers in the early Ether > controllers: Qtag Addition/Deletion Set Register (Port 0/1 to 1/0); thus > there's no need to make a difference in sh_eth_tsu_init() between those > controllers. Unfortunately, we can't just remove TSU_QTAG0/1 from the > register *enum* because that would break the ethtool register dump... > > Fixes: b0ca2a21f769 ("sh_eth: Add support of SH7763 to sh_eth") > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Thanks for your patch! > --- net-next.orig/drivers/net/ethernet/renesas/sh_eth.c > +++ net-next/drivers/net/ethernet/renesas/sh_eth.c > @@ -2097,8 +2097,6 @@ static size_t __sh_eth_get_regs(struct n > add_tsu_reg(TSU_FWSL0); > add_tsu_reg(TSU_FWSL1); > add_tsu_reg(TSU_FWSLC); > - add_tsu_reg(TSU_QTAG0); > - add_tsu_reg(TSU_QTAG1); Shouldn't you keep the above for ethtool register dump? > add_tsu_reg(TSU_QTAGM0); > add_tsu_reg(TSU_QTAGM1); > add_tsu_reg(TSU_FWSR); > --- net-next.orig/drivers/net/ethernet/renesas/sh_eth.h > +++ net-next/drivers/net/ethernet/renesas/sh_eth.h > @@ -118,8 +118,8 @@ enum { > TSU_FWSL0, > TSU_FWSL1, > TSU_FWSLC, > - TSU_QTAG0, > - TSU_QTAG1, > + TSU_QTAG0, /* Same as TSU_QTAGM0 */ > + TSU_QTAG1, /* Same as TSU_QTAGM1 */ > TSU_QTAGM0, > TSU_QTAGM1, > TSU_FWSR, Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 02/25/2018 04:14 PM, Geert Uytterhoeven wrote: >> The TSU_QTAG0/1 registers found in the Gigabit Ether controllers actually >> have the same long name as the TSU_QTAGM0/1 registers in the early Ether >> controllers: Qtag Addition/Deletion Set Register (Port 0/1 to 1/0); thus >> there's no need to make a difference in sh_eth_tsu_init() between those >> controllers. Unfortunately, we can't just remove TSU_QTAG0/1 from the >> register *enum* because that would break the ethtool register dump... >> >> Fixes: b0ca2a21f769 ("sh_eth: Add support of SH7763 to sh_eth") >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > Thanks for your patch! > >> --- net-next.orig/drivers/net/ethernet/renesas/sh_eth.c >> +++ net-next/drivers/net/ethernet/renesas/sh_eth.c > >> @@ -2097,8 +2097,6 @@ static size_t __sh_eth_get_regs(struct n >> add_tsu_reg(TSU_FWSL0); >> add_tsu_reg(TSU_FWSL1); >> add_tsu_reg(TSU_FWSLC); >> - add_tsu_reg(TSU_QTAG0); >> - add_tsu_reg(TSU_QTAG1); > > Shouldn't you keep the above for ethtool register dump? Why dump the same registers twice? These are no longer marked as valid in the dump buffer and a dump user's only source of info about the valid registers is the bitmap at the start of the buffer... Note that the dump is only done for the registers actually used by the driver, and these 2 regs are no longer used anywhere... >> add_tsu_reg(TSU_QTAGM0); >> add_tsu_reg(TSU_QTAGM1); >> add_tsu_reg(TSU_FWSR); MBR, Sergei
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Date: Sat, 24 Feb 2018 20:28:16 +0300 > The TSU_QTAG0/1 registers found in the Gigabit Ether controllers actually > have the same long name as the TSU_QTAGM0/1 registers in the early Ether > controllers: Qtag Addition/Deletion Set Register (Port 0/1 to 1/0); thus > there's no need to make a difference in sh_eth_tsu_init() between those > controllers. Unfortunately, we can't just remove TSU_QTAG0/1 from the > register *enum* because that would break the ethtool register dump... > > Fixes: b0ca2a21f769 ("sh_eth: Add support of SH7763 to sh_eth") > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Applied.
Index: net-next/drivers/net/ethernet/renesas/sh_eth.c =================================================================== --- net-next.orig/drivers/net/ethernet/renesas/sh_eth.c +++ net-next/drivers/net/ethernet/renesas/sh_eth.c @@ -123,8 +123,8 @@ static const u16 sh_eth_offset_gigabit[S [TSU_FWSL0] = 0x0030, [TSU_FWSL1] = 0x0034, [TSU_FWSLC] = 0x0038, - [TSU_QTAG0] = 0x0040, - [TSU_QTAG1] = 0x0044, + [TSU_QTAGM0] = 0x0040, + [TSU_QTAGM1] = 0x0044, [TSU_FWSR] = 0x0050, [TSU_FWINMK] = 0x0054, [TSU_ADQT0] = 0x0048, @@ -2097,8 +2097,6 @@ static size_t __sh_eth_get_regs(struct n add_tsu_reg(TSU_FWSL0); add_tsu_reg(TSU_FWSL1); add_tsu_reg(TSU_FWSLC); - add_tsu_reg(TSU_QTAG0); - add_tsu_reg(TSU_QTAG1); add_tsu_reg(TSU_QTAGM0); add_tsu_reg(TSU_QTAGM1); add_tsu_reg(TSU_FWSR); @@ -2934,13 +2932,8 @@ static void sh_eth_tsu_init(struct sh_et sh_eth_tsu_write(mdp, 0, TSU_FWSL0); sh_eth_tsu_write(mdp, 0, TSU_FWSL1); sh_eth_tsu_write(mdp, TSU_FWSLC_POSTENU | TSU_FWSLC_POSTENL, TSU_FWSLC); - if (sh_eth_is_gether(mdp)) { - sh_eth_tsu_write(mdp, 0, TSU_QTAG0); /* Disable QTAG(0->1) */ - sh_eth_tsu_write(mdp, 0, TSU_QTAG1); /* Disable QTAG(1->0) */ - } else { - sh_eth_tsu_write(mdp, 0, TSU_QTAGM0); /* Disable QTAG(0->1) */ - sh_eth_tsu_write(mdp, 0, TSU_QTAGM1); /* Disable QTAG(1->0) */ - } + sh_eth_tsu_write(mdp, 0, TSU_QTAGM0); /* Disable QTAG(0->1) */ + sh_eth_tsu_write(mdp, 0, TSU_QTAGM1); /* Disable QTAG(1->0) */ sh_eth_tsu_write(mdp, 0, TSU_FWSR); /* all interrupt status clear */ sh_eth_tsu_write(mdp, 0, TSU_FWINMK); /* Disable all interrupt */ sh_eth_tsu_write(mdp, 0, TSU_TEN); /* Disable all CAM entry */ Index: net-next/drivers/net/ethernet/renesas/sh_eth.h =================================================================== --- net-next.orig/drivers/net/ethernet/renesas/sh_eth.h +++ net-next/drivers/net/ethernet/renesas/sh_eth.h @@ -118,8 +118,8 @@ enum { TSU_FWSL0, TSU_FWSL1, TSU_FWSLC, - TSU_QTAG0, - TSU_QTAG1, + TSU_QTAG0, /* Same as TSU_QTAGM0 */ + TSU_QTAG1, /* Same as TSU_QTAGM1 */ TSU_QTAGM0, TSU_QTAGM1, TSU_FWSR,
The TSU_QTAG0/1 registers found in the Gigabit Ether controllers actually have the same long name as the TSU_QTAGM0/1 registers in the early Ether controllers: Qtag Addition/Deletion Set Register (Port 0/1 to 1/0); thus there's no need to make a difference in sh_eth_tsu_init() between those controllers. Unfortunately, we can't just remove TSU_QTAG0/1 from the register *enum* because that would break the ethtool register dump... Fixes: b0ca2a21f769 ("sh_eth: Add support of SH7763 to sh_eth") Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- drivers/net/ethernet/renesas/sh_eth.c | 15 ++++----------- drivers/net/ethernet/renesas/sh_eth.h | 4 ++-- 2 files changed, 6 insertions(+), 13 deletions(-)