Re: [PATCH v2 04/28] soc: fsl: cpm1: qmc: Extend the API to provide Rx status

[Christophe Leroy <christophe.leroy@xxxxxxxxxx>]

Le 26/07/2023 à 17:02, Herve Codina a écrit :
> In HDLC mode, some status flags related to the data read transfer can be
> set by the hardware and need to be known by a QMC consumer for further
> analysis.
> 
> Extend the API in order to provide these transfer status flags at the
> read complete() call.
> 
> In TRANSPARENT mode, these flags have no meaning. Keep only one read
> complete() API and update the consumers working in transparent mode.
> In this case, the newly introduced flags parameter is simply unused.
> 
> Signed-off-by: Herve Codina <herve.codina@xxxxxxxxxxx>

Reviewed-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>

> ---
>   drivers/soc/fsl/qe/qmc.c      | 29 +++++++++++++++++++++++++----
>   include/soc/fsl/qe/qmc.h      | 15 ++++++++++++++-
>   sound/soc/fsl/fsl_qmc_audio.c |  2 +-
>   3 files changed, 40 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
> index 8dc73cc1a83b..2d2a9d88ba6c 100644
> --- a/drivers/soc/fsl/qe/qmc.c
> +++ b/drivers/soc/fsl/qe/qmc.c
> @@ -166,7 +166,7 @@
>   struct qmc_xfer_desc {
>   	union {
>   		void (*tx_complete)(void *context);
> -		void (*rx_complete)(void *context, size_t length);
> +		void (*rx_complete)(void *context, size_t length, unsigned int flags);
>   	};
>   	void *context;
>   };
> @@ -421,7 +421,8 @@ static void qmc_chan_write_done(struct qmc_chan *chan)
>   }
>   
>   int qmc_chan_read_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,
> -			 void (*complete)(void *context, size_t length), void *context)
> +			 void (*complete)(void *context, size_t length, unsigned int flags),
> +			 void *context)
>   {
>   	struct qmc_xfer_desc *xfer_desc;
>   	unsigned long flags;
> @@ -454,6 +455,10 @@ int qmc_chan_read_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,
>   	xfer_desc->rx_complete = complete;
>   	xfer_desc->context = context;
>   
> +	/* Clear previous status flags */
> +	ctrl &= ~(QMC_BD_RX_L | QMC_BD_RX_F | QMC_BD_RX_LG | QMC_BD_RX_NO |
> +		  QMC_BD_RX_AB | QMC_BD_RX_CR);
> +
>   	/* Activate the descriptor */
>   	ctrl |= (QMC_BD_RX_E | QMC_BD_RX_UB);
>   	wmb(); /* Be sure to flush data before descriptor activation */
> @@ -485,7 +490,7 @@ EXPORT_SYMBOL(qmc_chan_read_submit);
>   
>   static void qmc_chan_read_done(struct qmc_chan *chan)
>   {
> -	void (*complete)(void *context, size_t size);
> +	void (*complete)(void *context, size_t size, unsigned int flags);
>   	struct qmc_xfer_desc *xfer_desc;
>   	unsigned long flags;
>   	cbd_t __iomem *bd;
> @@ -527,7 +532,23 @@ static void qmc_chan_read_done(struct qmc_chan *chan)
>   
>   		if (complete) {
>   			spin_unlock_irqrestore(&chan->rx_lock, flags);
> -			complete(context, datalen);
> +
> +			/*
> +			 * Avoid conversion between internal hardware flags and
> +			 * the software API flags.
> +			 * -> Be sure that the software API flags are consistent
> +			 *    with the hardware flags
> +			 */
> +			BUILD_BUG_ON(QMC_RX_FLAG_HDLC_LAST  != QMC_BD_RX_L);
> +			BUILD_BUG_ON(QMC_RX_FLAG_HDLC_FIRST != QMC_BD_RX_F);
> +			BUILD_BUG_ON(QMC_RX_FLAG_HDLC_OVF   != QMC_BD_RX_LG);
> +			BUILD_BUG_ON(QMC_RX_FLAG_HDLC_UNA   != QMC_BD_RX_NO);
> +			BUILD_BUG_ON(QMC_RX_FLAG_HDLC_ABORT != QMC_BD_RX_AB);
> +			BUILD_BUG_ON(QMC_RX_FLAG_HDLC_CRC   != QMC_BD_RX_CR);
> +
> +			complete(context, datalen,
> +				 ctrl & (QMC_BD_RX_L | QMC_BD_RX_F | QMC_BD_RX_LG |
> +					 QMC_BD_RX_NO | QMC_BD_RX_AB | QMC_BD_RX_CR));
>   			spin_lock_irqsave(&chan->rx_lock, flags);
>   		}
>   
> diff --git a/include/soc/fsl/qe/qmc.h b/include/soc/fsl/qe/qmc.h
> index 3c61a50d2ae2..6f1d6cebc9fe 100644
> --- a/include/soc/fsl/qe/qmc.h
> +++ b/include/soc/fsl/qe/qmc.h
> @@ -9,6 +9,7 @@
>   #ifndef __SOC_FSL_QMC_H__
>   #define __SOC_FSL_QMC_H__
>   
> +#include <linux/bits.h>
>   #include <linux/types.h>
>   
>   struct device_node;
> @@ -56,8 +57,20 @@ int qmc_chan_set_param(struct qmc_chan *chan, const struct qmc_chan_param *param
>   int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,
>   			  void (*complete)(void *context), void *context);
>   
> +/* Flags available (ORed) for read complete() flags parameter in HDLC mode.
> + * No flags are available in transparent mode and the read complete() flags
> + * parameter has no meaning in transparent mode.
> + */
> +#define QMC_RX_FLAG_HDLC_LAST	BIT(11) /* Last in frame */
> +#define QMC_RX_FLAG_HDLC_FIRST	BIT(10) /* First in frame */
> +#define QMC_RX_FLAG_HDLC_OVF	BIT(5)  /* Data overflow */
> +#define QMC_RX_FLAG_HDLC_UNA	BIT(4)  /* Unaligned (ie. bits received not multiple of 8) */
> +#define QMC_RX_FLAG_HDLC_ABORT	BIT(3)  /* Received an abort sequence (seven consecutive ones) */
> +#define QMC_RX_FLAG_HDLC_CRC	BIT(2)  /* CRC error */
> +
>   int qmc_chan_read_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,
> -			 void (*complete)(void *context, size_t length),
> +			 void (*complete)(void *context, size_t length,
> +					  unsigned int flags),
>   			 void *context);
>   
>   #define QMC_CHAN_READ  (1<<0)
> diff --git a/sound/soc/fsl/fsl_qmc_audio.c b/sound/soc/fsl/fsl_qmc_audio.c
> index 7cbb8e4758cc..5d745aae17a8 100644
> --- a/sound/soc/fsl/fsl_qmc_audio.c
> +++ b/sound/soc/fsl/fsl_qmc_audio.c
> @@ -99,7 +99,7 @@ static void qmc_audio_pcm_write_complete(void *context)
>   	snd_pcm_period_elapsed(prtd->substream);
>   }
>   
> -static void qmc_audio_pcm_read_complete(void *context, size_t length)
> +static void qmc_audio_pcm_read_complete(void *context, size_t length, unsigned int flags)
>   {
>   	struct qmc_dai_prtd *prtd = context;
>   	int ret;