> > The fsl_mc_object_allocate function can fail because not all
> > allocatable objects are probed by the fsl_mc_allocator at the call
> > time. Defer the dpaa2-eth probe when this happens.
> >
> > Signed-off-by: Ioana Ciornei <ioana.cior...@nxp.com>
> > ---
> >  drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 30
> > +++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > index 88f7acc..71f5cd4 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > @@ -1434,8 +1434,11 @@ static struct fsl_mc_device *setup_dpcon(struct
> dpaa2_eth_priv *priv)
> >     err = fsl_mc_object_allocate(to_fsl_mc_device(dev),
> >                                  FSL_MC_POOL_DPCON, &dpcon);
> >     if (err) {
> > -           dev_info(dev, "Not enough DPCONs, will go on as-is\n");
> > -           return NULL;
> > +           if (err == -ENXIO)
> > +                   err = -EPROBE_DEFER;
> > +           else
> > +                   dev_info(dev, "Not enough DPCONs, will go on as-
> is\n");
> > +           return ERR_PTR(err);
> >     }
> >
> >     err = dpcon_open(priv->mc_io, 0, dpcon->obj_desc.id,
> > &dpcon->mc_handle); @@ -1493,8 +1496,10 @@ static void
> free_dpcon(struct dpaa2_eth_priv *priv,
> >             return NULL;
> >
> >     channel->dpcon = setup_dpcon(priv);
> > -   if (!channel->dpcon)
> > +   if (IS_ERR_OR_NULL(channel->dpcon)) {
> > +           err = PTR_ERR(channel->dpcon);
> >             goto err_setup;
> > +   }
> 
> Hi Ioana
> 
> You need to be careful with IS_ERR_OR_NULL(). If it is a NULL,
> PTR_ERR() is going to return 0. You then jump to the error cleanup code, but
> return 0, meaning everything is O.K.
> 

Hi Andrew,

I took a closer look at the code path and it seems that if channel->dpcon in 
the snippet above is NULL,
then indeed PTR_ERR will be 0 but in the error cleanup code, in this case the 
err_setup label, 
a reverse ERR_PTR (NULL in this case) will be returned.
Continuing on the code path, alloc_channel then returns NULL and is handled by 
the following snippet.

+               if (IS_ERR_OR_NULL(channel)) {
+                       err = PTR_ERR(channel);
+                       if (err != -EPROBE_DEFER)
+                               dev_info(dev,
+                                        "No affine channel for cpu %d and 
above\n", i);
                        goto err_alloc_ch;
                }
  
In case channel is NULL, then the dev_info will be called and the jump to the 
cleanup is made.

err_alloc_ch:
+       if (err == -EPROBE_DEFER)
+               return err;
+
        if (cpumask_empty(&priv->dpio_cpumask)) {
                dev_err(dev, "No cpu with an affine DPIO/DPCON\n");
                return err;

Here err is 0 so in case the cpumask is empty, 0 will be returned, which is not 
the intended use.
I will send a v2 changing the return value to -ENODEV in case no cpus with an 
affine DPIO is found.

Thanks,
Ioana

Reply via email to