From mboxrd@z Thu Jan 1 00:00:00 1970 From: Trent Piepho Subject: Re: support DUAL and QUAD[patch v1] Date: Wed, 17 Jul 2013 19:25:55 -0700 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: yuhang wang 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 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. ------------------------------------------------------------------------------ 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-ie0-x231.google.com ([2607:f8b0:4001:c03::231]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Uzdv6-0006U4-K8 for linux-mtd@lists.infradead.org; Thu, 18 Jul 2013 02:26:17 +0000 Received: by mail-ie0-f177.google.com with SMTP id aq17so5704668iec.22 for ; Wed, 17 Jul 2013 19:25:55 -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: Wed, 17 Jul 2013 19:25:55 -0700 Message-ID: Subject: Re: support DUAL and QUAD[patch v1] From: Trent Piepho To: yuhang wang 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: , 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.