Re: [PATCH v4 2/6] PCI: spacemit-k1: Add multiple PHY handles support

On Fri, Jul 10, 2026 at 07:42:18AM -0500, Alex Elder wrote:
> On 7/10/26 5:55 AM, Inochi Amaoto wrote:
> > > > > > +	k1->phy_count = i;
> > > > > > +	if (k1->phy_count == 0)
> > > > > > +		return -EINVAL;
> > > > > > +
> > > > > > +	return 0;
> > > > > This doesn't seem correct to me, I would expect phy_count to be assigned only
> > > > > when it's valid. (Yes, perhaps 0 is the same as it was, but semantically it's
> > > > > different 0 in this case.)
> > > > I guess you think 0 is a valid number? I can not understand what you thing
> > > > Assign this to 0 if there is no phy is fine to me, which shows there is 0
> > > > vaild phy found.
> > > Isn't it already 0? Semantically code is wrong in a flow (not in the result).
> > > 
> > In fact it is already 0 here. But I am not understand why you thing is wrong.
> > Could you explain it in detail? (Maybe you think it is not good to return
> > -EINVAL?)
> 
> What Andy is saying is that assigning the value of phy_count
> before checking that it is valid is not the expected order of
> things.
> 
> The point is about best practice, not about the end result.
> 
> Best practice would be "don't touch anything if the return
> value will indicate an error."

Yes, precisely.

> And therefore, better coding practice would be to check for
> a bad value, and only after that has been done should you
> assign the k1->phy_count value.

-- 
With Best Regards,
Andy Shevchenko