Re: [PATCH v3 2/6] PCI: kirin: Tidy up _probe() related function with dev_err_probe()

["Krzysztof Wilczyński" <kw@linux.com>]

Hello,

[...]
> >  	usleep_range(PIPE_CLK_WAIT_MIN, PIPE_CLK_WAIT_MAX);
> >  	reg_val = kirin_apb_phy_readl(phy, PCIE_APB_PHY_STATUS0);
> > -	if (reg_val & PIPE_CLK_STABLE) {
> > -		dev_err(dev, "PIPE clk is not stable\n");
> > -		return -EINVAL;
> > -	}
> > +	if (reg_val & PIPE_CLK_STABLE)
> > +		return dev_err_probe(dev, -EINVAL,
> > +				     "PIPE clk is not stable\n");
> 
> I guess this is a timeout issue, so -ETIMEDOUT.

I can update this directly on the branch.

[...]
> > -	if (!dev->of_node) {
> > -		dev_err(dev, "NULL node\n");
> > -		return -EINVAL;
> > -	}
> > +	if (!dev->of_node)
> > +		return dev_err_probe(dev, -EINVAL, "NULL node\n");
> 
> This check is pointless, so you should drop it.

Some other drivers are also doing this, I suppose, as a precaution.

> >  	data = of_device_get_match_data(dev);
> > -	if (!data) {
> > -		dev_err(dev, "OF data missing\n");
> > -		return -EINVAL;
> > -	}
> > +	if (!data)
> > +		return dev_err_probe(dev, -EINVAL, "OF data missing\n");
> 
> -ENODATA

Almost all of the other drivers return -EINVAL in this case.

We can update this here, but I wonder if a follow-up patch to update other
drivers accordingly where it makes sense of course, would be better here?

Thoughts?

	Krzysztof