From mboxrd@z Thu Jan 1 00:00:00 1970 From: yuhang wang Subject: Re: support DUAL and QUAD[patch v1] Date: Fri, 19 Jul 2013 10:24:56 +0800 Message-ID: References: <20980858CB6D3A4BAE95CA194937D5E73E9E84D2@DBDE04.ent.ti.com> <20980858CB6D3A4BAE95CA194937D5E73E9E851E@DBDE04.ent.ti.com> <593AEF6C47F46446852B067021A273D6D93B49E1@MUCSE039.lantiq.com> <20980858CB6D3A4BAE95CA194937D5E73E9E85D0@DBDE04.ent.ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: "thomas.langer-th3ZBGNqt+7QT0dZR+AlfA@public.gmane.org" , "broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" , "linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , "Gupta, Pekon" , "spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org" , "Poddar, Sourav" To: Trent Piepho Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: spi-devel-general-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: linux-spi.vger.kernel.org Hi Trent, 2013/7/18 Trent Piepho : > On Wed, Jul 17, 2013 at 6:58 PM, yuhang wang wrote: >> >> > > >> >> > > > [Pekon]: Instead of adding new fields you can use existing 'mode' >> >> > > > field >> >> to >> >> > > > pass on the platform specific configurations. And if 'u8 mode' >> >> > > > does not >> >> > > > suffice you can increase it to 'u32'. > > But what about spidev? > >> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c >> index 004b10f..36d2451 100644 >> --- a/drivers/spi/spi.c >> +++ b/drivers/spi/spi.c >> @@ -868,6 +868,14 @@ static void of_register_spi_devices(struct >> spi_master *master) >> spi->mode |= SPI_CS_HIGH; >> if (of_find_property(nc, "spi-3wire", NULL)) >> spi->mode |= SPI_3WIRE; >> + if (of_find_property(nc, "spi-tx-dual", NULL)) >> + spi->mode |= SPI_TX_DUAL; >> + if (of_find_property(nc, "spi-tx-quad", NULL)) >> + spi->mode |= SPI_TX_QUAD; >> + if (of_find_property(nc, "spi-rx-dual", NULL)) >> + spi->mode |= SPI_RX_DUAL; >> + if (of_find_property(nc, "spi-rx-quad", NULL)) >> + spi->mode |= SPI_RX_QUAD; >> >> /* Device speed */ >> prop = of_get_property(nc, "spi-max-frequency", &len); >> @@ -1316,6 +1324,12 @@ int spi_setup(struct spi_device *spi) >> /* help drivers fail *cleanly* when they need options >> * that aren't supported with their current master >> */ >> + if (((spi->mode >> 8) & 0x03) == 0x03 || >> + ((spi->mode >> 10) & 0x03) == 0x03) { >> + dev_err(&spi->dev, >> + "setup: can not select dual and quad at the same time\n"); >> + return -EINVAL; >> + } > > Maybe it would make more sense to have a spi-rx-width and spi-tx-width > than can be set to 1, 2, 4, etc. bits instead of a bunch of properties > that are mutually exclusive? Sooner or later someone is going to want > 8 bits. > >> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h >> index 38c2b92..e9ba1cf 100644 >> --- a/include/linux/spi/spi.h >> +++ b/include/linux/spi/spi.h >> @@ -74,7 +74,7 @@ struct spi_device { >> struct spi_master *master; >> u32 max_speed_hz; >> u8 chip_select; >> - u8 mode; >> + u16 mode; > > What about this, from include/uapi/linux/spi/spidev.h: > > /* Read / Write of SPI mode (SPI_MODE_0..SPI_MODE_3) */ > #define SPI_IOC_RD_MODE _IOR(SPI_IOC_MAGIC, 1, __u8) > #define SPI_IOC_WR_MODE _IOW(SPI_IOC_MAGIC, 1, __u8) > > Note that you can't just change the type in the ioctl define, as it > won't be backward compatible. well, got it.Still need to be considered. Thanks ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ob0-x235.google.com ([2607:f8b0:4003:c01::235]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1V00Ni-0002gP-65 for linux-mtd@lists.infradead.org; Fri, 19 Jul 2013 02:25:18 +0000 Received: by mail-ob0-f181.google.com with SMTP id 16so4540704obc.26 for ; Thu, 18 Jul 2013 19:24:56 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <20980858CB6D3A4BAE95CA194937D5E73E9E84D2@DBDE04.ent.ti.com> <20980858CB6D3A4BAE95CA194937D5E73E9E851E@DBDE04.ent.ti.com> <593AEF6C47F46446852B067021A273D6D93B49E1@MUCSE039.lantiq.com> <20980858CB6D3A4BAE95CA194937D5E73E9E85D0@DBDE04.ent.ti.com> Date: Fri, 19 Jul 2013 10:24:56 +0800 Message-ID: Subject: Re: support DUAL and QUAD[patch v1] From: yuhang wang To: Trent Piepho Content-Type: text/plain; charset=ISO-8859-1 Cc: "thomas.langer@lantiq.com" , "broonie@kernel.org" , "linux-mtd@lists.infradead.org" , "Gupta, Pekon" , "spi-devel-general@lists.sourceforge.net" , "Poddar, Sourav" List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Hi Trent, 2013/7/18 Trent Piepho : > On Wed, Jul 17, 2013 at 6:58 PM, yuhang wang wrote: >> >> > > >> >> > > > [Pekon]: Instead of adding new fields you can use existing 'mode' >> >> > > > field >> >> to >> >> > > > pass on the platform specific configurations. And if 'u8 mode' >> >> > > > does not >> >> > > > suffice you can increase it to 'u32'. > > But what about spidev? > >> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c >> index 004b10f..36d2451 100644 >> --- a/drivers/spi/spi.c >> +++ b/drivers/spi/spi.c >> @@ -868,6 +868,14 @@ static void of_register_spi_devices(struct >> spi_master *master) >> spi->mode |= SPI_CS_HIGH; >> if (of_find_property(nc, "spi-3wire", NULL)) >> spi->mode |= SPI_3WIRE; >> + if (of_find_property(nc, "spi-tx-dual", NULL)) >> + spi->mode |= SPI_TX_DUAL; >> + if (of_find_property(nc, "spi-tx-quad", NULL)) >> + spi->mode |= SPI_TX_QUAD; >> + if (of_find_property(nc, "spi-rx-dual", NULL)) >> + spi->mode |= SPI_RX_DUAL; >> + if (of_find_property(nc, "spi-rx-quad", NULL)) >> + spi->mode |= SPI_RX_QUAD; >> >> /* Device speed */ >> prop = of_get_property(nc, "spi-max-frequency", &len); >> @@ -1316,6 +1324,12 @@ int spi_setup(struct spi_device *spi) >> /* help drivers fail *cleanly* when they need options >> * that aren't supported with their current master >> */ >> + if (((spi->mode >> 8) & 0x03) == 0x03 || >> + ((spi->mode >> 10) & 0x03) == 0x03) { >> + dev_err(&spi->dev, >> + "setup: can not select dual and quad at the same time\n"); >> + return -EINVAL; >> + } > > Maybe it would make more sense to have a spi-rx-width and spi-tx-width > than can be set to 1, 2, 4, etc. bits instead of a bunch of properties > that are mutually exclusive? Sooner or later someone is going to want > 8 bits. > >> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h >> index 38c2b92..e9ba1cf 100644 >> --- a/include/linux/spi/spi.h >> +++ b/include/linux/spi/spi.h >> @@ -74,7 +74,7 @@ struct spi_device { >> struct spi_master *master; >> u32 max_speed_hz; >> u8 chip_select; >> - u8 mode; >> + u16 mode; > > What about this, from include/uapi/linux/spi/spidev.h: > > /* Read / Write of SPI mode (SPI_MODE_0..SPI_MODE_3) */ > #define SPI_IOC_RD_MODE _IOR(SPI_IOC_MAGIC, 1, __u8) > #define SPI_IOC_WR_MODE _IOW(SPI_IOC_MAGIC, 1, __u8) > > Note that you can't just change the type in the ioctl define, as it > won't be backward compatible. well, got it.Still need to be considered. Thanks