[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160718225952.GW30372@sirena.org.uk>
Date: Mon, 18 Jul 2016 23:59:52 +0100
From: Mark Brown <broonie@...nel.org>
To: Michal Suchanek <hramrach@...il.com>
Cc: linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/3] spi: spidev: create spidev device for all spi
slaves.
On Tue, Jul 19, 2016 at 12:35:40AM +0200, Michal Suchanek wrote:
> config SPI_SPIDEV
> - tristate "User mode SPI device driver support"
> + bool "User mode SPI device driver support"
This is a step back, it would require spidev to be built in.
> - spin_lock_irq(&spidev->spi_lock);
> - spi = spi_dev_get(spidev->spi);
> - spin_unlock_irq(&spidev->spi_lock);
> + spi = filp->private_data;
> + spi = spi_dev_get(spi);
All this refactoring to move spidev about should've been a separate
patch, it's hard to find the actual content in here.
> - mutex_lock(&device_list_lock);
> + dev = bus_find_device(&spi_bus_type, NULL, (void *) inode->i_rdev,
> + spidev_devt_match);
...
> - dev_dbg(&spidev->spi->dev, "open/ENOMEM\n");
> + spi = to_spi_device(dev);
> +
> + mutex_lock(&spi->buf_lock);
> + spin_lock_irq(&spi->spidev_lock);
> + spi->spidev_users++;
> + spin_unlock_irq(&spi->spidev_lock);
This is broken, it will unconditionally create a spidev for any chip
select regardless of if there's any actual hardware there and (even more
importantly) regardless if that hardware is actually a SPI device. This
is not safe, especially given some of the ideas people seem to have for
userspaces.
I am getting completely fed up of saying this, it's not OK to just
expose pins to userspace when we have no idea what they are connected
to.
Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)
Powered by blists - more mailing lists