Re: [PATCH v1 net-next] net: stmmac: Add support for MDIO interrupts

[David Miller]

From: Voon Weifeng <weifeng.voon@xxxxxxxxx>
Date: Tue, 27 Aug 2019 09:45:20 +0800

> From: "Chuah, Kim Tatt" <kim.tatt.chuah@xxxxxxxxx>
> 
> DW EQoS v5.xx controllers added capability for interrupt generation
> when MDIO interface is done (GMII Busy bit is cleared).
> This patch adds support for this interrupt on supported HW to avoid
> polling on GMII Busy bit.
> 
> stmmac_mdio_read() & stmmac_mdio_write() will sleep until wake_up() is
> called by the interrupt handler.
> 
> Reviewed-by: Voon Weifeng <weifeng.voon@xxxxxxxxx>
> Reviewed-by: Kweh, Hock Leong <hock.leong.kweh@xxxxxxxxx>
> Reviewed-by: Ong Boon Leong <boon.leong.ong@xxxxxxxxx>
> Signed-off-by: Chuah, Kim Tatt <kim.tatt.chuah@xxxxxxxxx>
> Signed-off-by: Ong Boon Leong <boon.leong.ong@xxxxxxxxx>
> Signed-off-by: Voon Weifeng <weifeng.voon@xxxxxxxxx>

I know there are some design changes that will occur with this patch but
coding style wise:

> @@ -276,6 +284,10 @@ int stmmac_hwif_init(struct stmmac_priv *priv)
>  		mac->mode = mac->mode ? : entry->mode;
>  		mac->tc = mac->tc ? : entry->tc;
>  		mac->mmc = mac->mmc ? : entry->mmc;
> +		mac->mdio_intr_en = mac->mdio_intr_en ? : entry->mdio_intr_en;
> +
> +		if (mac->mdio_intr_en)
> +			init_waitqueue_head(&mac->mdio_busy_wait);

I'd say always unconditionally initialize wait queues, mutexes, etc.

> +static bool stmmac_mdio_intr_done(struct mii_bus *bus)
> +{
> +	struct net_device *ndev = bus->priv;
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	unsigned int mii_address = priv->hw->mii.addr;

Reverse christmas tree here, please.