[U-Boot] [PATCHv2 04/10] net: mvpp2x: fix BM configuration overrun issue

[Stefan Roese]

Hi Stefan,

On 09.08.2017 09:37, stefanc at marvell.com wrote:
> From: Stefan Chulski <stefanc at marvell.com>
> 
> Issue:
> BM counters were overrun by probe that called per Network interface and
> caused release of wrong number of buffers during remove procedure.
> 
> Fix:
> Use probe_done and num_ports to call init and remove procedure
> once per communication controller.
> 
> Signed-off-by: Stefan Chulski <stefanc at marvell.com>
> Tested-by: iSoC Platform CI <ykjenk at marvell.com>
> Reviewed-by: Igal Liberman <igall at marvell.com>

Thanks for the new patch versions. One general comment though.
Please add a change history to the patches sent in a new version
as described here:

http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions

This allow much easier review of new patch versions, since the
reviewes don't have to figure out, what was changed in between
the different versions. Especially the changes in this patch
(4/10) are not obvious - at least not to me.

You don't need to do this for this v2 patch version (if Joe agrees
as well), but please add this history in new patches with multiple
versions.

Thanks,
Stefan

> ---
>   drivers/net/mvpp2.c | 25 +++++++++++++++++--------
>   1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
> index 3083111..426b2c2 100644
> --- a/drivers/net/mvpp2.c
> +++ b/drivers/net/mvpp2.c
> @@ -942,6 +942,7 @@ struct mvpp2 {
>   	struct mii_dev *bus;
>   
>   	int probe_done;
> +	u8 num_ports;
>   };
>   
>   struct mvpp2_pcpu_stats {
> @@ -4848,6 +4849,7 @@ static int mvpp2_port_probe(struct udevice *dev,
>   #endif
>   
>   	priv->port_list[port->id] = port;
> +	priv->num_ports++;
>   	return 0;
>   }
>   
> @@ -5512,10 +5514,8 @@ static int mvpp2_probe(struct udevice *dev)
>   	int err;
>   
>   	/* Only call the probe function for the parent once */
> -	if (!priv->probe_done) {
> +	if (!priv->probe_done)
>   		err = mvpp2_base_probe(dev->parent);
> -		priv->probe_done = 1;
> -	}
>   
>   	port->priv = dev_get_priv(dev->parent);
>   
> @@ -5553,11 +5553,15 @@ static int mvpp2_probe(struct udevice *dev)
>   		gop_port_init(port);
>   	}
>   
> -	/* Initialize network controller */
> -	err = mvpp2_init(dev, priv);
> -	if (err < 0) {
> -		dev_err(&pdev->dev, "failed to initialize controller\n");
> -		return err;
> +	if (!priv->probe_done) {
> +		/* Initialize network controller */
> +		err = mvpp2_init(dev, priv);
> +		if (err < 0) {
> +			dev_err(&pdev->dev, "failed to initialize controller\n");
> +			return err;
> +		}
> +		priv->num_ports = 0;
> +		priv->probe_done = 1;
>   	}
>   
>   	err = mvpp2_port_probe(dev, port, dev_of_offset(dev), priv);
> @@ -5585,6 +5589,11 @@ static int mvpp2_remove(struct udevice *dev)
>   	struct mvpp2 *priv = port->priv;
>   	int i;
>   
> +	priv->num_ports--;
> +
> +	if (priv->num_ports)
> +		return 0;
> +
>   	for (i = 0; i < MVPP2_BM_POOLS_NUM; i++)
>   		mvpp2_bm_pool_destroy(dev, priv, &priv->bm_pools[i]);
>   
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de