[OpenWrt-Devel] [PATCH v5 1/5] ar71xx: add eth rx delay for qca955x platforms

[Christian Lamparter]

Ping.

Any comments? what to do about ath79_setup_ar934x_eth_rx_delay.
Leave it as is, or get rid of it?

Regards,

Christian

On Friday, November 27, 2015 08:34:40 PM Christian Lamparter wrote:
> On Saturday, November 21, 2015 08:45:23 PM John Crispin wrote:
> > Hi,
> > 
> > sorry to jump in this late at a V5 but i have a few concerns. see inline
> Well, the *beautiful thing* of this platform is that Cisco charges people
> a yearly fee if they want to stick with the original firmware. So people
> are definitely interested in this openwrt port. Just look at the positive
> response to the support thread [0].
>  
> > On 22/09/2015 18:49, Chris R Blake wrote:
> > > From: Chris R Blake <chrisrblake93 at gmail.com>
> > > 
> > > This patch is to add support for qca955x_eth_rx_delay
> > > to work with the qca955x SoC.
> > > 
> > > Signed-off-by: Chris R Blake <chrisrblake93 at gmail.com>
> > > ---
> > >  ...42-MIPS-ath79-add-qca955x-mac-tx-rx-delay.patch | 58 ++++++++++++++++++++++
> > >  1 file changed, 58 insertions(+)
> > >  create mode 100644 target/linux/ar71xx/patches-4.1/742-MIPS-ath79-add-qca955x-mac-tx-rx-delay.patch
> > > 
> > > diff --git a/target/linux/ar71xx/patches-4.1/742-MIPS-ath79-add-qca955x-mac-tx-rx-delay.patch b/target/linux/ar71xx/patches-4.1/742-MIPS-ath79-add-qca955x-mac-tx-rx-delay.patch
> > > new file mode 100644
> > > index 0000000..75e216e
> > > --- /dev/null
> > > +++ b/target/linux/ar71xx/patches-4.1/742-MIPS-ath79-add-qca955x-mac-tx-rx-delay.patch
> > > @@ -0,0 +1,58 @@
> > > +--- a/arch/mips/ath79/dev-eth.c
> > > ++++ b/arch/mips/ath79/dev-eth.c
> > > +@@ -823,6 +825,32 @@
> > > + 	iounmap(base);
> > > + }
> > > +
> > > ++void __init ath79_setup_qca955x_eth_rx_delay(unsigned int rxd,
> > > ++					      unsigned int rxdv)
> > > ++{
> > > ++	void __iomem *base;
> > > ++	u32 t;
> > > ++
> > > ++	rxd &= QCA955X_ETH_CFG_RXD_DELAY_MASK;
> > > ++	rxdv &= QCA955X_ETH_CFG_RDV_DELAY_MASK;
> > > ++
> > > ++	base = ioremap(QCA955X_GMAC_BASE, QCA955X_GMAC_SIZE);
> > > ++
> > > ++	t = __raw_readl(base + QCA955X_GMAC_REG_ETH_CFG);
> > > ++
> > > ++	t &= ~(QCA955X_ETH_CFG_RXD_DELAY_MASK << QCA955X_ETH_CFG_RXD_DELAY_SHIFT |
> > > ++	       QCA955X_ETH_CFG_RDV_DELAY_MASK << QCA955X_ETH_CFG_RDV_DELAY_SHIFT);
> > > ++
> > > ++	t |= (rxd << QCA955X_ETH_CFG_RXD_DELAY_SHIFT |
> > > ++	      rxdv << QCA955X_ETH_CFG_RDV_DELAY_SHIFT);
> > > ++
> > > ++	__raw_writel(t, base + QCA955X_GMAC_REG_ETH_CFG);
> > > ++	/* flush write */
> > > ++	__raw_readl(base + QCA955X_GMAC_REG_ETH_CFG);
> > > ++
> > > ++	iounmap(base);
> > 
> > this is a pretty ugly way of doing it. the register is also modified at
> > a different place which is also not optimal. the register is part of the
> > ethernet macs io range so this magic setting should be applied inside
> > the the actual driver. could you make such a change ?
> > 
> > 	John
> 
> Wait, the code for this function was just adapted (qca955x uses
> slightly different registers and bitmask offsets) from a similar
> function called:
> 
> void __init ath79_setup_ar934x_eth_rx_delay(unsigned int rxd,
>                                             unsigned int rxdv)
> 
> which is also in the dev-eth.c [1] (added Felix). If this is
> "pretty ugly" then what should be done about ath79_setup_ar934x_eth_rx_delay?
> If it's just the function that bothers you, I can also pass the
> rx/tx delays via the ath79_setup_qca955x_eth_cfg call. but I 
> would like to keep the ar71xx_regs changes in place. Ok?
> 
> > > +--- a/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
> > > ++++ b/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
> > > +@@ -1098,5 +1098,11 @@
> > > +
> > > + #define QCA955X_ETH_CFG_RGMII_EN	BIT(0)
> > > + #define QCA955X_ETH_CFG_GE0_SGMII	BIT(6)
> > > ++#define QCA955X_ETH_CFG_RXD_DELAY	BIT(14)
> > > ++#define QCA955X_ETH_CFG_RXD_DELAY_MASK	0x3
> > > ++#define QCA955X_ETH_CFG_RXD_DELAY_SHIFT	14
> > > ++#define QCA955X_ETH_CFG_RDV_DELAY	BIT(16)
> > > ++#define QCA955X_ETH_CFG_RDV_DELAY_MASK	0x3
> > > ++#define QCA955X_ETH_CFG_RDV_DELAY_SHIFT	16
> 
> Regards,
> Christian
> 
> [0] <https://forum.openwrt.org/viewtopic.php?id=59248>
> [1] <https://dev.openwrt.org/changeset/45523>
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel