Re: [PATCH v6 1/2] spi: Add spi driver for Sunplus SP7021

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

On Tue, Jan 18, 2022 at 04:42:38PM +0800, Li-hao Kuo wrote:

Looks mostly good - a couple of small nits below but nothing major.

> +static int sp7021_spi_master_transfer_one(struct spi_controller *ctlr, struct spi_device *spi,
> +				       struct spi_transfer *xfer)
> +{

> +	for (i = 0; i <= xfer_cnt; i++) {
> +		mutex_lock(&pspim->buf_lock);

This lock is redundant: it is only ever held in this function which is
guaranteed by the core to never be called twice concurrently.

> +	ret = devm_request_irq(dev, pspim->m_irq, sp7021_spi_master_irq,
> +			       IRQF_TRIGGER_RISING, pdev->name, pspim);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_request_irq(dev, pspim->s_irq, sp7021_spi_slave_irq,
> +			       IRQF_TRIGGER_RISING, pdev->name, pspim);
> +	if (ret)
> +		return ret;

Are you sure the driver is ready to handle interrupts without any of the
other resources?  Normally interrupts are one of the last things to be
requested.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]