Re: [PATCH 1/2] phy: marvell: comphy: Convert internal SMCC firmware return codes to errno

[Andrew Lunn <andrew@lunn.ch>]

On Wed, Sep 02, 2020 at 07:05:25PM +0200, Pali Rohár wrote:
> On Wednesday 02 September 2020 19:00:10 Andrew Lunn wrote:
> > > > > +	switch (ret) {
> > > > > +	case SMCCC_RET_SUCCESS:
> > > > > +		return 0;
> > > > > +	case SMCCC_RET_NOT_SUPPORTED:
> > > > > +		return -EOPNOTSUPP;
> > > > > +	default:
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > >  }
> > > > 
> > > > Hi Pali
> > > > 
> > > > Maybe this should be a global helper translating SMCCC_RET_* into a
> > > > standard errno value?
> > > > 
> > > > 	 Andrew
> > > 
> > > Hello Andrew!
> > > 
> > > Well, I'm not sure if some standard global helper is the correct way for
> > > marvell comphy handler. It returns 0 for success and -1 on error when
> > > handler is not supported.
> > 
> > No, i was meaning just 
> > 
> > switch (ret) {
> > case SMCCC_RET_SUCCESS:
> > 	return 0;
> > case SMCCC_RET_NOT_SUPPORTED:
> > 	return -EOPNOTSUPP;
> > default:
> > 	return -EINVAL;
> > }
> 
> But this is not a complete generic helper. There are more generic SMCC
> return codes and generic helper should define and translate all of them.

/*
 * Return codes defined in ARM DEN 0070A
 * ARM DEN 0070A is now merged/consolidated into ARM DEN 0028 C
 */
#define SMCCC_RET_SUCCESS			0
#define SMCCC_RET_NOT_SUPPORTED			-1
#define SMCCC_RET_NOT_REQUIRED			-2
#define SMCCC_RET_INVALID_PARAMETER		-3

I only see problems with SMCCC_RET_NOT_REQUIRED and what value to use
for it. Do you have any idea what is actually means? A parameter was
passed which was not required? Or that the call itself is not
required? Looking at the uses of it currently in the kernel, it does
not seem to be an actual error. So maybe just return 0?

	Andrew