[BUG] I2S operations are not adhered to by lower-half implementations

[linguini1]

Description / Steps to reproduce the issue

Some lower-half implementations of the I2S driver do not follow the correct return contract.

For instance, i2s_txsamplerate requires the return value to be the current bitrate. However, some drivers (i.e. rp2040_i2s.c) return 0 on success and a negated errno value on failure. This is not correct, it should be the bitrate on success.

nuttx/arch/arm/src/rp2040/rp2040_i2s.c

Lines 737 to 766 in e9ac37e

	/****************************************************************************
	* Name: rp2040_i2s_txsamplerate
	*
	* Description:
	* Set the I2S TX sample rate.
	*
	* Input Parameters:
	* dev - Device-specific state data
	* rate - The I2S sample rate in samples (not bits) per second
	*
	* Returned Value:
	* OK on success; a negated errno value on failure.
	*
	****************************************************************************/
	static uint32_t rp2040_i2s_txsamplerate(struct i2s_dev_s *dev, uint32_t rate)
	{
	struct rp2040_i2s_s *priv = (struct rp2040_i2s_s *)dev;
	DEBUGASSERT(priv && priv->samplerate >= 0 && rate > 0);
	if (rate < 8000)
	{
	return -EINVAL;
	}
	priv->samplerate = rate;
	return 0;
	}

nuttx/include/nuttx/audio/i2s.h

Lines 268 to 287 in e9ac37e

	/****************************************************************************
	* Name: I2S_TXSAMPLERATE
	*
	* Description:
	* Set the I2S TX sample rate. NOTE: This will have no effect if (1) the
	* driver does not support an I2S transmitter or if (2) the sample rate is
	* driven by the I2S frame clock. This may also have unexpected side-
	* effects of the TX sample is coupled with the RX sample rate.
	*
	* Input Parameters:
	* dev - Device-specific state data
	* rate - The I2S sample rate in samples (not bits) per second
	*
	* Returned Value:
	* Returns the resulting bitrate
	*
	****************************************************************************/
	#define I2S_TXSAMPLERATE(d,r) \
	((d)->ops->i2s_txsamplerate ? (d)->ops->i2s_txsamplerate(d,r) : -ENOTTY)

On which OS does this issue occur?

[OS: Linux]

What is the version of your OS?

Arch Linux

NuttX Version

master

Issue Architecture

[Arch: all]

Issue Area

[Area: Drivers]

Host information

No response

Verification

• I have verified before submitting the report.

[cederom]

Fixed with #19307 just need to see if there are no other places having this issue, assigning to myself the rest :-)

[linguini1]

Fixed with #19307 just need to see if there are no other places having this issue, assigning to myself the rest :-)

#19307 just fixed a return type signedness issue, but some of the implementations just don't return the correct values at all.

[cederom]

ACK!

[linguini1]

Okay so I also found that the i2schar driver casts the ioctl arg parameter to the unsigned return types which I fixed. So things need to be adjusted there.

Also, the i2schar driver seems to think that passing a data width of '0 bits' to i2s_txdatawidth should cause the lower-half driver to return the currently set data width. This is not at all what the upper-half driver asks for in the signature; it asks for data width * sample rate. And many things (like the rp2040 lowerhalf) will trigger an assertion if the bit parameter is passed as 0.

So it looks like the whole I2S driver stuff is a mess? Is there anyone who has worked with I2S audio who can weigh in?

[cederom]

Will ask on the dev@ if not we have to clean stuff up :-)