controller_data handling through spidev.c

controller_data handling through spidev.c

[Soulard, MathieuX]

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

Hi all,

I'm using spidev.c to test a SPI controller driver from the user space. Actually some SPI controller drivers handle a specific spi_device.controller_data as input in their setup operation, and the controller driver I'm testing uses this field in its setup operation, if provided.

The driver spidev.c handles many commands through ioctl but it does not provide a way to setup a specific controller_data.

I would like to get your opinion about this field usage from the protocol driver, since I'm not sure to understand the way this field has be used: is it really supposed to be used by the protocol driver to deliver some controller's specific setup, or, is it a field we should ignore from the protocol driver perspective as it is supposed to be only managed by the controller driver if it needs it ?

Then, I would like to get your opinion about the attached patch I'm using to get an access to controller_data from the user space. I wrote this patch since the SPI driver I'm testing handles a specific spi_device.controller_data in its setup operation. It allows me to provide it from user space through spidev.c.


Thanks
Regards

Mathieu
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[-- Attachment #2: Type: text/plain, Size: 247 bytes --]

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

[-- Attachment #3: Type: text/plain, Size: 210 bytes --]

_______________________________________________
spi-devel-general mailing list
spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Re: controller_data handling through spidev.c

[David Brownell]

--- On Tue, 8/31/10, Soulard, MathieuX <mathieux.soulard-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

> 
> ... Actually some SPI controller drivers handle a
> specific spi_device.controller_data as input in their setup
> operation, and the controller driver I'm testing uses this
> field in its setup operation, if provided.
> 
> The driver spidev.c handles many commands through ioctl but
> it does not provide a way to setup a specific
> controller_data.

And it shouldn't, any more than platform_data.


> 
> I would like to get your opinion about this field usage
> from the protocol driver, since I'm not sure to understand
> the way this field has be used: is it really supposed to be
> used by the protocol driver to deliver some controller's
> specific setup, or, is it a field we should ignore from the
> protocol driver perspective as

Ignore it.


It's a sibling to the platform_data field, and
should be set up with that.  Neither one should
be assigned or understood by protocol drivers.

Historically, controller_data was used ISTR to
cope with tuning that the PXA SPI controller
required (DMA etc).  Most controllers should
have no need for it.



 it is supposed to be only
> managed by the controller driver if it needs it ?
> 
> Then, I would like to get your opinion about the attached
> patch I'm using to get an access to controller_data from the
> user space. I wrote this patch since the SPI driver I'm
> testing handles a specific spi_device.controller_data in its
> setup operation. It allows me to provide it from user space
> through spidev.c.
> 
> 
> Thanks
> Regards
> 
> Mathieu
> ---------------------------------------------------------------------
> Intel Corporation SAS (French simplified joint stock
> company)
> Registered headquarters: "Les Montalets"- 2, rue de Paris,
> 
> 92196 Meudon Cedex, France
> Registration Number:  302 456 199 R.C.S. NANTERRE
> Capital: 4,572,000 Euros
> 
> This e-mail and any attachments may contain confidential
> material for
> the sole use of the intended recipient(s). Any review or
> distribution
> by others is strictly prohibited. If you are not the
> intended
> recipient, please contact the sender and delete all
> copies.
> 
> -----Inline Attachment Follows-----
> 
> ------------------------------------------------------------------------------
> This SF.net Dev2Dev email is sponsored by:
> 
> Show off your parallel programming skills.
> Enter the Intel(R) Threading Challenge 2010.
> http://p.sf.net/sfu/intel-thread-sfd
> -----Inline Attachment Follows-----
> 
> _______________________________________________
> spi-devel-general mailing list
> spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/spi-devel-general
> 


------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

Re: controller_data handling through spidev.c

[Feng Tang]

> 
> It's a sibling to the platform_data field, and
> should be set up with that.  Neither one should
> be assigned or understood by protocol drivers.


> 
> Historically, controller_data was used ISTR to
> cope with tuning that the PXA SPI controller
> required (DMA etc).  Most controllers should
> have no need for it.
> 

DW spi driver is also using the controller data, but as you said
protocol driver itself doesn't need set it. Platform code where
that spi device is registered (by spi_register_board_info) should
set up the controller data.

Thanks,
Feng

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

controller_data handling through spidev.c

[Soulard, MathieuX]

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

Hi all,

It seems the patch that I mentioned in my previous mail has been trashed, here is this path:

>From 97ad3082149695f14e621861e73ef03fa1b33ec7 Mon Sep 17 00:00:00 2001
From: Mathieu SOULARD <mathieux.soulard-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date: Fri, 27 Aug 2010 10:33:11 +0200
Subject: [PATCH] Add controller_data ioctl accessor to spidev.c

Signed-off-by: Mathieu SOULARD <mathieux.soulard-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spidev.c       |   70 ++++++++++++++++++++++++++++++--------------
 include/linux/spi/spidev.h |    3 ++
 2 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index ea1bec3..21d7991 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -433,36 +433,62 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)

               default:
                              /* segmented and/or full-duplex I/O request */
-                             if (_IOC_NR(cmd) != _IOC_NR(SPI_IOC_MESSAGE(0))
+                            if ((_IOC_NR(cmd) != _IOC_NR(SPI_IOC_MESSAGE(0)) &&
+                                           _IOC_NR(cmd) != _IOC_NR(SPI_IOC_WR_CTRL_DATA(0)))
                                                            || _IOC_DIR(cmd) != _IOC_WRITE) {
                                             retval = -ENOTTY;
                                             break;
                              }

-                             tmp = _IOC_SIZE(cmd);
-                             if ((tmp % sizeof(struct spi_ioc_transfer)) != 0) {
-                                            retval = -EINVAL;
-                                            break;
-                             }
-                             n_ioc = tmp / sizeof(struct spi_ioc_transfer);
-                             if (n_ioc == 0)
-                                            break;
+                            if (_IOC_NR(cmd) == _IOC_NR(SPI_IOC_MESSAGE(0))) {
+                                           tmp = _IOC_SIZE(cmd);
+                                           if ((tmp % sizeof(struct spi_ioc_transfer)) != 0) {
+                                                          retval = -EINVAL;
+                                                          break;
+                                           }
+                                           n_ioc = tmp / sizeof(struct spi_ioc_transfer);
+                                           if (n_ioc == 0)
+                                                          break;

-                             /* copy into scratch area */
-                             ioc = kmalloc(tmp, GFP_KERNEL);
-                             if (!ioc) {
-                                            retval = -ENOMEM;
-                                            break;
-                             }
-                             if (__copy_from_user(ioc, (void __user *)arg, tmp)) {
+                                           /* copy into scratch area */
+                                           ioc = kmalloc(tmp, GFP_KERNEL);
+                                           if (!ioc) {
+                                                          retval = -ENOMEM;
+                                                          break;
+                                           }
+                                           if (__copy_from_user(ioc, (void __user *)arg, tmp)) {
+                                                          kfree(ioc);
+                                                          retval = -EFAULT;
+                                                          break;
+                                           }
+                                           /* translate to spi_message, execute */
+                                           retval = spidev_message(spidev, ioc, n_ioc);
                                             kfree(ioc);
-                                            retval = -EFAULT;
-                                            break;
+                            } else if (_IOC_NR(cmd) == _IOC_NR(SPI_IOC_WR_CTRL_DATA(0))) {
+                                           void *save = spi->controller_data;
+
+                                           tmp = _IOC_SIZE(cmd);
+                                           if (tmp == 0) {
+                                                          spi->controller_data = NULL;
+                                           } else {
+                                                          spi->controller_data = kmalloc(tmp, GFP_KERNEL);
+                                                          if (!spi->controller_data) {
+                                                                         spi->controller_data = save;
+                                                                         retval = -ENOMEM;
+                                                                         break;
+                                                          }
+                                                          if (__copy_from_user(spi->controller_data,
+                                                                                        (void __user *)arg, tmp)) {
+                                                                         kfree(spi->controller_data);
+                                                                         spi->controller_data = save;
+                                                                         retval = -EFAULT;
+                                                                         break;
+                                                          }
+                                                          retval = spi_setup(spi);
+                                           }
+                                           if (!save)
+                                                          kfree(save);
                              }
-
-                             /* translate to spi_message, execute */
-                             retval = spidev_message(spidev, ioc, n_ioc);
-                             kfree(ioc);
                              break;
               }

diff --git a/include/linux/spi/spidev.h b/include/linux/spi/spidev.h
index bf0570a..dbc3796 100644
--- a/include/linux/spi/spidev.h
+++ b/include/linux/spi/spidev.h
@@ -126,6 +126,9 @@ struct spi_ioc_transfer {
 #define SPI_IOC_RD_MAX_SPEED_HZ                     _IOR(SPI_IOC_MAGIC, 4, __u32)
 #define SPI_IOC_WR_MAX_SPEED_HZ                    _IOW(SPI_IOC_MAGIC, 4, __u32)

+/* Write SPI controller_data */
+#define SPI_IOC_WR_CTRL_DATA(size)  _IOW(SPI_IOC_MAGIC, 5, char[size])
+


 #endif /* SPIDEV_H */
--
1.6.5.2

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[-- Attachment #2: Type: text/plain, Size: 247 bytes --]

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

[-- Attachment #3: Type: text/plain, Size: 210 bytes --]

_______________________________________________
spi-devel-general mailing list
spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/spi-devel-general