[PATCH v2 5/7] serial: qcom-geni: fix rx cancel dma status bit

Johan Hovold posted 7 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v2 5/7] serial: qcom-geni: fix rx cancel dma status bit
Posted by Johan Hovold 1 month, 3 weeks ago
Cancelling an rx command is signalled using bit 14 of the rx DMA status
register and not bit 11.

This bit is currently unused, but this error becomes apparent, for
example, when tracing the status register when closing the port.

Fixes: eddac5af0654 ("soc: qcom: Add GENI based QUP Wrapper driver")
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 include/linux/soc/qcom/geni-se.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h
index c3bca9c0bf2c..2996a3c28ef3 100644
--- a/include/linux/soc/qcom/geni-se.h
+++ b/include/linux/soc/qcom/geni-se.h
@@ -258,8 +258,8 @@ struct geni_se {
 #define RX_DMA_PARITY_ERR		BIT(5)
 #define RX_DMA_BREAK			GENMASK(8, 7)
 #define RX_GENI_GP_IRQ			GENMASK(10, 5)
-#define RX_GENI_CANCEL_IRQ		BIT(11)
 #define RX_GENI_GP_IRQ_EXT		GENMASK(13, 12)
+#define RX_GENI_CANCEL_IRQ		BIT(14)
 
 /* SE_HW_PARAM_0 fields */
 #define TX_FIFO_WIDTH_MSK		GENMASK(29, 24)
-- 
2.45.2
Re: [PATCH v2 5/7] serial: qcom-geni: fix rx cancel dma status bit
Posted by Doug Anderson 1 month, 3 weeks ago
Hi,

On Tue, Oct 1, 2024 at 5:51 AM Johan Hovold <johan+linaro@kernel.org> wrote:
>
> Cancelling an rx command is signalled using bit 14 of the rx DMA status
> register and not bit 11.
>
> This bit is currently unused, but this error becomes apparent, for
> example, when tracing the status register when closing the port.
>
> Fixes: eddac5af0654 ("soc: qcom: Add GENI based QUP Wrapper driver")
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  include/linux/soc/qcom/geni-se.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h
> index c3bca9c0bf2c..2996a3c28ef3 100644
> --- a/include/linux/soc/qcom/geni-se.h
> +++ b/include/linux/soc/qcom/geni-se.h
> @@ -258,8 +258,8 @@ struct geni_se {
>  #define RX_DMA_PARITY_ERR              BIT(5)
>  #define RX_DMA_BREAK                   GENMASK(8, 7)
>  #define RX_GENI_GP_IRQ                 GENMASK(10, 5)
> -#define RX_GENI_CANCEL_IRQ             BIT(11)
>  #define RX_GENI_GP_IRQ_EXT             GENMASK(13, 12)
> +#define RX_GENI_CANCEL_IRQ             BIT(14)

This looks right, but do you want to fix all the rest of the wrong
bits in this list while you're at it? Things look OK up to the
"RX_FLUSH_DONE" and then they're wrong. Specifically:

* My datasheet doesn't have RX_DMA_PARITY_ERR. Unless maybe it's one
of the "GP" IRQs?

* My datassheet doesn't have RX_DMA_BREAK. Unless maybe it's one of
the "GP" IRQs (though why would it be two bits big?)

* RX_GENI_GP_IRQ is 12:5, not 10:5

* My datasheet has RX_GENI_CMD_FAILURE as BIT(15).

In any case, this does make it better so:

Reviewed-by: Douglas Anderson <dianders@chromium.org>


-Doug
Re: [PATCH v2 5/7] serial: qcom-geni: fix rx cancel dma status bit
Posted by Johan Hovold 1 month, 2 weeks ago
On Thu, Oct 03, 2024 at 12:55:39PM -0700, Doug Anderson wrote:
> On Tue, Oct 1, 2024 at 5:51 AM Johan Hovold <johan+linaro@kernel.org> wrote:

> >  #define RX_DMA_PARITY_ERR              BIT(5)
> >  #define RX_DMA_BREAK                   GENMASK(8, 7)
> >  #define RX_GENI_GP_IRQ                 GENMASK(10, 5)
> > -#define RX_GENI_CANCEL_IRQ             BIT(11)
> >  #define RX_GENI_GP_IRQ_EXT             GENMASK(13, 12)
> > +#define RX_GENI_CANCEL_IRQ             BIT(14)
> 
> This looks right, but do you want to fix all the rest of the wrong
> bits in this list while you're at it? Things look OK up to the
> "RX_FLUSH_DONE" and then they're wrong. Specifically:
> 
> * My datasheet doesn't have RX_DMA_PARITY_ERR. Unless maybe it's one
> of the "GP" IRQs?

As you noticed, this one appears to be correct.

> * My datassheet doesn't have RX_DMA_BREAK. Unless maybe it's one of
> the "GP" IRQs (though why would it be two bits big?)

And same here, apparently one is break on and the other break off.

> * RX_GENI_GP_IRQ is 12:5, not 10:5
> 
> * My datasheet has RX_GENI_CMD_FAILURE as BIT(15).

I'll just leave the rest as is for now.

Johan