Re: [PATCH v2 2/2] spi: lantiq-ssc: add support for Lantiq SSC SPI controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Dec 11, 2016 at 09:03:50PM +0100, Hauke Mehrtens wrote:

> +config SPI_LANTIQ_SSC
> +	tristate "Lantiq SSC SPI controller"
> +	depends on LANTIQ

No || COMPILE_TEST?

> +	if (t->speed_hz)
> +		speed_hz = t->speed_hz;
> +	else
> +		speed_hz = spidev->max_speed_hz;

The core will ensure the transfers are always initialized.

> +static int lantiq_ssc_unprepare_message(struct spi_master *master,
> +					struct spi_message *message)
> +{
> +	struct lantiq_ssc_spi *spi = spi_master_get_devdata(master);
> +
> +	/* Disable transmitter and receiver while idle */
> +	lantiq_ssc_maskl(spi, 0, SPI_CON_TXOFF | SPI_CON_RXOFF, SPI_CON);
> +
> +	return 0;
> +}

Why is this done per message and not when the device actually goes idle?

> +		default:
> +			WARN_ON(1);
> +			data = 0;
> +		}

Missing break.

> +		case 24:
> +		case 32:
> +			rx32 = (u32 *) spi->rx;
> +			*rx32 = data;
> +			spi->rx_todo -= 4;
> +			spi->rx += 4;
> +			break;

This looks like the device doesn't actually support 24 bits per word or
at least is expecting the data to be in memory with pad bytes which
isn't what Linux expects.  You'd need to repack the data for the
hardware or just not support it.

> +static irqreturn_t lantiq_ssc_err_interrupt(int irq, void *data)
> +{
> +	struct lantiq_ssc_spi *spi = data;
> +	u32 stat = lantiq_ssc_readl(spi, SPI_STAT);
> +
> +	if (stat & SPI_STAT_RUE)
> +		dev_err(spi->dev, "receive underflow error\n");
> +	if (stat & SPI_STAT_TUE)
> +		dev_err(spi->dev, "transmit underflow error\n");
> +	if (stat & SPI_STAT_RE)
> +		dev_err(spi->dev, "receive overflow error\n");
> +	if (stat & SPI_STAT_TE)
> +		dev_err(spi->dev, "transmit overflow error\n");
> +	if (stat & SPI_STAT_ME)
> +		dev_err(spi->dev, "mode error\n");
> +
> +	/* Clear error flags */
> +	lantiq_ssc_maskl(spi, 0, SPI_WHBSTATE_CLR_ERRORS, SPI_WHBSTATE);
> +
> +	/* set bad status so it can be retried */
> +	spi->status = -EIO;
> +	spi_finalize_current_transfer(spi->master);
> +
> +	return IRQ_HANDLED;
> +}

This unconditionally returns IRQ_HANDLED even if nothing was flagged.

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux