Subject: Re: [PATCH net-next 1/6] net: stmmac: add platform library

From: Simon Horman <[email-masked]>
Date: Thu, 14 Sep 2023 13:24:06 +0200

On Tue, Sep 12, 2023 at 11:20:55PM +0100, Russell King (Oracle) wrote:
> On Tue, Sep 12, 2023 at 04:52:27PM +0200, Simon Horman wrote:
> > On Mon, Sep 11, 2023 at 04:29:11PM +0100, Russell King (Oracle) wrote:
> > > +	default:
> > > +		return -ENOTSUPP;
> > 
> > Checkpatch seems to think that EOPNOTSUPP would be more appropriate
> > as "ENOTSUPP is not a SUSV4 error code".
> 
> It needs to be an error code that clk_set_rate() below isn't going to
> return - because if clk_set_rate() does return it, then the users are
> going to end up issuing an incorrect error message to the user. I
> suspect clk_set_rate() could quite legitimately return -EOPNOTSUPP
> or -EINVAL.
> 
> Sadly, the CCF implementation of clk_set_rate() doesn't detail what
> errors it could return, but it looks like -EBUSY, -EINVAL, or something
> from pm_runtime_resume_and_get().

Thanks Russell,

Understood.

In that case perhaps ENOTSUPP is not such a bad choice as:
a) it seems rather unlikely CCF would use it; and
b) the scope of usage is well contained - the helper and any direct callers.

No further objections from my side :)

> 
> Interestingly, while looking at this, pm_runtime_resume_and_get() can
> return '1' if e.g. rpm is disabled and the device is active. It looks
> to me like CCF treats that as an error in multiple locations.

The plot thickens...