RE: [PATCH net-next v6 2/3] net: axienet: Preparatory changes for dmaengine support

[Pandey, Radhey Shyam]

> -----Original Message-----
> From: Russell King <linux@xxxxxxxxxxxxxxx>
> Sent: Tuesday, September 19, 2023 3:22 PM
> To: Pandey, Radhey Shyam <radhey.shyam.pandey@xxxxxxx>
> Cc: davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> pabeni@xxxxxxxxxx; robh+dt@xxxxxxxxxx;
> krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; Simek, Michal
> <michal.simek@xxxxxxx>; netdev@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; git (AMD-Xilinx) <git@xxxxxxx>; Sarath Babu
> Naidu Gaddam <sarath.babu.naidu.gaddam@xxxxxxx>
> Subject: Re: [PATCH net-next v6 2/3] net: axienet: Preparatory changes for
> dmaengine support
> 
> On Tue, Sep 19, 2023 at 12:46:54AM +0530, Radhey Shyam Pandey wrote:
> > +/**
> > + * axienet_open - Driver open routine.
> > + * @ndev:	Pointer to net_device structure
> > + *
> > + * Return: 0, on success.
> > + *	    non-zero error value on failure
> > + *
> > + * This is the driver open routine. It calls phylink_start to start
> > +the
> > + * PHY device.
> > + * It also allocates interrupt service routines, enables the
> > +interrupt lines
> > + * and ISR handling. Axi Ethernet core is reset through Axi DMA core.
> > +Buffer
> > + * descriptors are initialized.
> > + */
> > +static int axienet_open(struct net_device *ndev) {
> > +	int ret;
> > +	struct axienet_local *lp = netdev_priv(ndev);
> > +
> > +	dev_dbg(&ndev->dev, "%s\n", __func__);
> > +
> > +	/* When we do an Axi Ethernet reset, it resets the complete core
> > +	 * including the MDIO. MDIO must be disabled before resetting.
> > +	 * Hold MDIO bus lock to avoid MDIO accesses during the reset.
> > +	 */
> > +	axienet_lock_mii(lp);
> > +	ret = axienet_device_reset(ndev);
> > +	axienet_unlock_mii(lp);
> > +
> > +	ret = phylink_of_phy_connect(lp->phylink, lp->dev->of_node, 0);
> > +	if (ret) {
> > +		dev_err(lp->dev, "phylink_of_phy_connect() failed: %d\n",
> ret);
> > +		return ret;
> > +	}
> > +
> > +	phylink_start(lp->phylink);
> 
> ... and at this point, the link can come up while phylink_start() is completing.
> Could that cause a problem if this happens before:

This preparatory patch keeps same execution sequence and it only 
creates wrapper around dma specific initialization. No functional
change.

There shouldn't be any problem if link come up while phylink_start is 
completing. Packet will be processed only after dma initialization
(Interrupts are enabled) . 

> 
> > +
> > +	if (!lp->use_dmaengine) {
> > +		ret = axienet_init_legacy_dma(ndev);
> > +		if (ret)
> > +			goto error_code;
> > +	}
> 
> ?
> 
> I suppose I should add this statement to the phylink_start() documentation
> so that this point is clear for everyone.

Seems to be a good idea to capture it in documentation.
> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!