Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 13 Jun 2023 00:16:30 +0200
From:   Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
To:     "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc:     Daniel Golle <daniel@makrotopia.org>, netdev@vger.kernel.org,
        linux-mediatek@lists.infradead.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Lorenzo Bianconi <lorenzo@kernel.org>,
        Mark Lee <Mark-MC.Lee@mediatek.com>,
        Sean Wang <sean.wang@mediatek.com>,
        John Crispin <john@phrozen.org>, Felix Fietkau <nbd@nbd.name>,
        Conor Dooley <conor+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        "David S. Miller" <davem@davemloft.net>,
        Sam Shih <Sam.Shih@mediatek.com>
Subject: Re: [PATCH net-next 5/8] net: ethernet: mtk_eth_soc: add
 MTK_NETSYS_V3 capability bit
Message-ID: <ZIeZPqFJqdf928f4@lore-desk>
References: <ZIUXf9APDFCNaUG1@makrotopia.org>
 <ZIb/WKKNlzjTIu2h@shell.armlinux.org.uk>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="MZEWyw36ZZKq14wi"
Content-Disposition: inline
In-Reply-To: <ZIb/WKKNlzjTIu2h@shell.armlinux.org.uk>
Precedence: bulk


--MZEWyw36ZZKq14wi
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

> On Sun, Jun 11, 2023 at 01:38:23AM +0100, Daniel Golle wrote:
> > @@ -1333,8 +1354,13 @@ static int mtk_tx_map(struct sk_buff *skb, struc=
t net_device *dev,
> >  	mtk_tx_set_dma_desc(dev, itxd, &txd_info);
> > =20
> >  	itx_buf->flags |=3D MTK_TX_FLAGS_SINGLE0;
> > -	itx_buf->flags |=3D (!mac->id) ? MTK_TX_FLAGS_FPORT0 :
> > -			  MTK_TX_FLAGS_FPORT1;
> > +	if (mac->id =3D=3D MTK_GMAC1_ID)
> > +		itx_buf->flags |=3D MTK_TX_FLAGS_FPORT0;
> > +	else if (mac->id =3D=3D MTK_GMAC2_ID)
> > +		itx_buf->flags |=3D MTK_TX_FLAGS_FPORT1;
> > +	else
> > +		itx_buf->flags |=3D MTK_TX_FLAGS_FPORT2;
>=20
> There appears to be two places that this code structure appears, and
> this is in the path for packet transmission. I wonder if it would be
> more efficient to instead do:
>=20
> 	itx_buf->flags |=3D MTK_TX_FLAGS_SINGLE0 | mac->tx_flags;
>=20
> with mac->tx_flags appropriately initialised?
>=20
> > @@ -2170,7 +2214,9 @@ static int mtk_poll_tx_qdma(struct mtk_eth *eth, =
int budget,
> >  		tx_buf =3D mtk_desc_to_tx_buf(ring, desc,
> >  					    eth->soc->txrx.txd_size);
> >  		if (tx_buf->flags & MTK_TX_FLAGS_FPORT1)
> > -			mac =3D 1;
> > +			mac =3D MTK_GMAC2_ID;
> > +		else if (tx_buf->flags & MTK_TX_FLAGS_FPORT2)
> > +			mac =3D MTK_GMAC3_ID;
>=20
> This has me wondering whether the flags are used for hardware or just
> for the driver's purposes. If it's the latter, can we instead store the
> MAC index in tx_buf, rather than having to decode a bitfield?
>=20
> I suspect these are just for the driver given that the addition of
> MTK_TX_FLAGS_FPORT2 changes all subsequent bit numbers in this struct
> member.

ack, I agree. I will rework it.

Regards,
Lorenzo

>=20
> > =20
> >  		if (!tx_buf->data)
> >  			break;
> > @@ -3783,7 +3829,26 @@ static int mtk_hw_init(struct mtk_eth *eth, bool=
 reset)
> >  	mtk_w32(eth, eth->soc->txrx.rx_irq_done_mask, reg_map->qdma.int_grp +=
 4);
> >  	mtk_w32(eth, 0x21021000, MTK_FE_INT_GRP);
> > =20
> > -	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2)) {
> > +	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V3)) {
> > +		/* PSE should not drop port1, port8 and port9 packets */
> > +		mtk_w32(eth, 0x00000302, PSE_DROP_CFG);
> > +
> > +		/* GDM and CDM Threshold */
> > +		mtk_w32(eth, 0x00000707, MTK_CDMW0_THRES);
> > +		mtk_w32(eth, 0x00000077, MTK_CDMW1_THRES);
> > +
> > +		/* Disable GDM1 RX CRC stripping */
> > +		val =3D mtk_r32(eth, MTK_GDMA_FWD_CFG(0));
> > +		val &=3D ~MTK_GDMA_STRP_CRC;
> > +		mtk_w32(eth, val, MTK_GDMA_FWD_CFG(0));
>=20
> mtk_m32() ?
>=20
> Thanks!
>=20
> --=20
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
>=20

--MZEWyw36ZZKq14wi
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQTquNwa3Txd3rGGn7Y6cBh0uS2trAUCZIeZPgAKCRA6cBh0uS2t
rOWZAP93LCOL1i0nkHkj6wrpRzOraRpEoqEK3GPo84WYL1nFzQEAs2hJgVY3XzfF
KF3PwyoYttShSk1kgjn4c0xzXmKqKgU=
=Fpo3
-----END PGP SIGNATURE-----

--MZEWyw36ZZKq14wi--