Re: [PATCH] spi: bcm2835aux: ensure interrupts are enabled for shared handler

[Stefan Wahren <stefan.wahren@xxxxxxxx>]

Hi,

[added Martin, Noralf and Phil]


> Rob Herring <robh@xxxxxxxxxx> hat am 3. Mai 2018 um 20:09 geschrieben:
> 
> 
> The BCM2835 AUX SPI has a shared interrupt line (with AUX UART).
> Downstream fixes this with an AUX irqchip to demux the IRQ sources and a
> DT change which breaks compatibility with older kernels. The AUX irqchip
> was already rejected for upstream[1] and the DT change would break
> working systems if the DTB is updated to a newer one. The latter issue
> was brought to my attention by Alex Graf.
> 
> The root cause however is a bug in the shared handler. Shared handlers
> must check that interrupts are actually enabled before servicing the
> interrupt. Add a check that the TXEMPTY or IDLE interrupts are enabled.
> 
> [1] https://patchwork.kernel.org/patch/9781221/
> 
> Cc: Alexander Graf <agraf@xxxxxxx>
> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
> Cc: Mark Brown <broonie@xxxxxxxxxx>
> Cc: Eric Anholt <eric@xxxxxxxxxx>
> Cc: Stefan Wahren <stefan.wahren@xxxxxxxx>
> Cc: Florian Fainelli <f.fainelli@xxxxxxxxx>
> Cc: Ray Jui <rjui@xxxxxxxxxxxx>
> Cc: Scott Branden <sbranden@xxxxxxxxxxxx>
> Cc: bcm-kernel-feedback-list@xxxxxxxxxxxx
> Cc: linux-spi@xxxxxxxxxxxxxxx
> Cc: linux-rpi-kernel@xxxxxxxxxxxxxxxxxxx
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
> ---
> Compile tested only. I'll add something to the related github issue on 
> this and hopefully someone can test and confirm.
> 
> I'm assuming the 8250 driver can handle shared irqs correctly.
> 
> Rob
> 
>  drivers/spi/spi-bcm2835aux.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
> index 1431cb98fe40..3094d818cf06 100644
> --- a/drivers/spi/spi-bcm2835aux.c
> +++ b/drivers/spi/spi-bcm2835aux.c
> @@ -184,6 +184,11 @@ static irqreturn_t bcm2835aux_spi_interrupt(int irq, void *dev_id)
>  	struct bcm2835aux_spi *bs = spi_master_get_devdata(master);
>  	irqreturn_t ret = IRQ_NONE;
>  
> +	/* IRQ may be shared, so return if our interrupts are disabled */
> +	if (!(bcm2835aux_rd(bs, BCM2835_AUX_SPI_CNTL1) &
> +	      (BCM2835_AUX_SPI_CNTL1_TXEMPTY | BCM2835_AUX_SPI_CNTL1_IDLE)))
> +		return ret;
> +
>  	/* check if we have data to read */
>  	while (bs->rx_len &&
>  	       (!(bcm2835aux_rd(bs, BCM2835_AUX_SPI_STAT) &
> -- 
> 2.17.0

does anyone have a setup to test this patch (i assume it requires a compute module)?

@Rob: Thank you very much for this patch
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html