For today there is no way to use i3c devices from user space and
the introduction of such API will help developers during the i3c device
or i3c host controllers development.
The i3cdev module is highly based on i2c-dev and yet I tried to address
the concerns raised in [1].
NOTES:
- The i3cdev dynamically request an unused major number.
- The i3c devices are dynamically exposed/removed from dev/ folder based
on if they have a device driver bound to it.
- For now, the module exposes i3c devices without device driver on
dev/bus/i3c/<bus>-<pid>
- As in the i2c subsystem, here it is exposed the i3c_priv_xfer to
userspace. I tried to use a dedicated structure as in spidev but I don't
see any obvious advantage.
- Since the i3c API only exposes i3c_priv_xfer to devices, for now, the
module just makes use of one ioctl(). This can change in the future with
the introduction hdr commands or by the need of exposing some CCC
commands to the device API (private contract between master-slave).
Regarding the i3c device info, some information is already available
through sysfs. We can add more device attributes to expose more
information or add a dedicated ioctl() request for that purpose or both.
- Similar to i2c, I have also created a tool that you can find in [2]
for testing purposes. If you have some time available I would appreciate
your feedback about it as well.
[1] https://lkml.org/lkml/2018/11/15/853
[2] https://github.com/vitor-soares-snps/i3c-tools.git
Changes in v3:
Use the xfer_lock to prevent device detach during ioctl call
Expose i3cdev under /dev/bus/i3c/ folder like usb does
Change NOTIFY_BOUND to NOTIFY_BIND, this allows the device detach occur
before driver->probe call
Avoid use of IS_ERR_OR_NULL
Use u64_to_user_ptr instead of (void __user *)(uintptr_t) cast
Allocate k_xfer and data_ptrs at once and eliminate double allocation
check
Pass i3cdev to dev->driver_data
Make all minors available
Add API documentation
Changes in v2:
Use IDR api for minor numbering
Modify ioctl struct
Fix SPDX license
Vitor Soares (5):
i3c: master: export i3c_masterdev_type
i3c: master: export i3c_bus_type symbol
i3c: master: add i3c_for_each_dev helper
i3c: add i3cdev module to expose i3c dev in /dev
Documentation: userspac-api: add i3cdev documentation
Documentation/userspace-api/i3c/i3cdev.rst | 116 ++++++++
drivers/i3c/Kconfig | 15 +
drivers/i3c/Makefile | 1 +
drivers/i3c/i3cdev.c | 429 +++++++++++++++++++++++++++++
drivers/i3c/internals.h | 2 +
drivers/i3c/master.c | 16 +-
include/uapi/linux/i3c/i3cdev.h | 38 +++
7 files changed, 616 insertions(+), 1 deletion(-)
create mode 100644 Documentation/userspace-api/i3c/i3cdev.rst
create mode 100644 drivers/i3c/i3cdev.c
create mode 100644 include/uapi/linux/i3c/i3cdev.h
--
2.7.4
Export i3c_bus_type symbol so i3cdev can register a notifier chain for
i3c bus.
Signed-off-by: Vitor Soares <[email protected]>
---
drivers/i3c/master.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 8a0ba34..21c4372 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -321,6 +321,7 @@ struct bus_type i3c_bus_type = {
.probe = i3c_device_probe,
.remove = i3c_device_remove,
};
+EXPORT_SYMBOL_GPL(i3c_bus_type);
static enum i3c_addr_slot_status
i3c_bus_get_addr_slot_status(struct i3c_bus *bus, u16 addr)
--
2.7.4
This patch add documentation for the userspace API of i3cdev module.
Signed-off-by: Vitor Soares <[email protected]>
---
Documentation/userspace-api/i3c/i3cdev.rst | 116 +++++++++++++++++++++++++++++
1 file changed, 116 insertions(+)
create mode 100644 Documentation/userspace-api/i3c/i3cdev.rst
diff --git a/Documentation/userspace-api/i3c/i3cdev.rst b/Documentation/userspace-api/i3c/i3cdev.rst
new file mode 100644
index 0000000..ada269f
--- /dev/null
+++ b/Documentation/userspace-api/i3c/i3cdev.rst
@@ -0,0 +1,116 @@
+====================
+I3C Device Interface
+====================
+
+I3C devices have the flexibility of being accessed from userspace, as well
+through the conventional use of kernel drivers. Userspace access, although
+limited to private SDR I3C transfers, provides the advantage of simplifying
+the implementation of straightforward communication protocols, applicable to
+scenarios where transfers are dedicated such for sensor bring-up scenarios
+(prototyping environments) or for microcontroller slave communication
+implementation.
+
+The major device number is dynamically attributed and it's all reserved for
+the i3c devices. By default, the i3cdev module only exposes the i3c devices
+without device driver bind and aren't of master type in sort of character
+device file under /dev/bus/i3c/ folder. They are identified through its
+<bus id>-<Provisional ID> same way they can be found in /sys/bus/i3c/devices/.
+::
+
+# ls -l /dev/bus/i3c/
+total 0
+crw------- 1 root root 248, 0 Jan 1 00:22 0-6072303904d2
+crw------- 1 root root 248, 1 Jan 1 00:22 0-b7405ba00929
+
+The simplest way to use this interface is to not have an I3C device bound to
+a kernel driver, this can be achieved by not have the kernel driver loaded or
+using the Sysfs to unbind the kernel driver from the device.
+
+BASIC CHARACTER DEVICE API
+===============================
+For now, the API has only support private SDR read and write transfers.
+Those transaction can be achieved by the following:
+
+``read(file, buffer, sizeof(buffer))``
+ The standard read() operation will work as a simple transaction of private
+ SDR read data followed a stop.
+ Return the number of bytes read on success, and a negative error otherwise.
+
+``write(file, buffer, sizeof(buffer))``
+ The standard write() operation will work as a simple transaction of private
+ SDR write data followed a stop.
+ Return the number of bytes written on success, and a negative error otherwise.
+
+``ioctl(file, I3C_IOC_PRIV_XFER(nxfers), struct i3c_ioc_priv_xfer *xfers)``
+ It combines read/write transactions without a stop in between.
+ Return 0 on success, and a negative error otherwise.
+
+NOTES:
+ - According to the MIPI I3C Protocol is the I3C slave that terminates the read
+ transaction otherwise Master can abort early on ninth (T) data bit of each
+ SDR data word.
+
+ - Normal open() and close() operations on /dev/bus/i3c/<bus>-<provisional id>
+ files work as you would expect.
+
+ - As documented in cdev_del() if a device was already open during
+ i3cdev_detach, the read(), write() and ioctl() fops will still be callable
+ yet they will return -EACCES.
+
+C EXAMPLE
+=========
+Working with I3C devices is much like working with files. You will need to open
+a file descriptor, do some I/O operations with it, and then close it.
+
+The following header files should be included in an I3C program::
+
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/ioctl.h>
+#include <linux/types.h>
+#include <linux/i3c/i3cdev.h>
+
+To work with an I3C device, the application must open the driver, made
+available at the device node::
+
+ int file;
+
+ file = open("/dev/bus/i3c/0-6072303904d2", O_RDWR);
+ if (file < 0)
+ exit(1);
+
+Now the file is opened, we can perform the operations available::
+
+ /* Write function */
+ uint_t8 buf[] = {0x00, 0xde, 0xad, 0xbe, 0xef}
+ if (write(file, buf, 5) != 5) {
+ /* ERROR HANDLING: I3C transaction failed */
+ }
+
+ /* Read function */
+ ret = read(file, buf, 5);
+ If (ret < 0) {
+ /* ERROR HANDLING: I3C transaction failed */
+ } else {
+ /* Iterate over buf[] to get the read data */
+ }
+
+ /* IOCTL function */
+ struct i3c_ioc_priv_xfer xfers[2];
+
+ uint8_t tx_buf[] = {0x00, 0xde, 0xad, 0xbe, 0xef};
+ uint8_t rx_buf[10];
+
+ xfers[0].data = (uintptr_t)tx_buf;
+ xfers[0].len = 5;
+ xfers[0].rnw = 0;
+ xfers[1].data = (uintptr_t)rx_buf;
+ xfers[1].len = 10;
+ xfers[1].rnw = 1;
+
+ if (ioctl(file, I3C_IOC_PRIV_XFER(2), xfers) < 0)
+ /* ERROR HANDLING: I3C transaction failed */
+
+The device can be closed when the open file descriptor is no longer required::
+
+ close(file);
\ No newline at end of file
--
2.7.4
Exporte i3c_masterdev_type so i3cdev module can verify if an i3c device
is a master.
Signed-off-by: Vitor Soares <[email protected]>
---
drivers/i3c/internals.h | 1 +
drivers/i3c/master.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
index 86b7b44..bc062e8 100644
--- a/drivers/i3c/internals.h
+++ b/drivers/i3c/internals.h
@@ -11,6 +11,7 @@
#include <linux/i3c/master.h>
extern struct bus_type i3c_bus_type;
+extern const struct device_type i3c_masterdev_type;
void i3c_bus_normaluse_lock(struct i3c_bus *bus);
void i3c_bus_normaluse_unlock(struct i3c_bus *bus);
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 7f8f896..8a0ba34 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -523,9 +523,10 @@ static void i3c_masterdev_release(struct device *dev)
of_node_put(dev->of_node);
}
-static const struct device_type i3c_masterdev_type = {
+const struct device_type i3c_masterdev_type = {
.groups = i3c_masterdev_groups,
};
+EXPORT_SYMBOL_GPL(i3c_masterdev_type);
static int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode,
unsigned long max_i2c_scl_rate)
--
2.7.4
Hi Boris,
From: Vitor Soares <[email protected]>
Date: Wed, Feb 19, 2020 at 00:20:38
> For today there is no way to use i3c devices from user space and
> the introduction of such API will help developers during the i3c device
> or i3c host controllers development.
>
> The i3cdev module is highly based on i2c-dev and yet I tried to address
> the concerns raised in [1].
>
> NOTES:
> - The i3cdev dynamically request an unused major number.
>
> - The i3c devices are dynamically exposed/removed from dev/ folder based
> on if they have a device driver bound to it.
>
> - For now, the module exposes i3c devices without device driver on
> dev/bus/i3c/<bus>-<pid>
>
> - As in the i2c subsystem, here it is exposed the i3c_priv_xfer to
> userspace. I tried to use a dedicated structure as in spidev but I don't
> see any obvious advantage.
>
> - Since the i3c API only exposes i3c_priv_xfer to devices, for now, the
> module just makes use of one ioctl(). This can change in the future with
> the introduction hdr commands or by the need of exposing some CCC
> commands to the device API (private contract between master-slave).
> Regarding the i3c device info, some information is already available
> through sysfs. We can add more device attributes to expose more
> information or add a dedicated ioctl() request for that purpose or both.
>
> - Similar to i2c, I have also created a tool that you can find in [2]
> for testing purposes. If you have some time available I would appreciate
> your feedback about it as well.
>
> [1] https://lkml.org/lkml/2018/11/15/853
> [2] https://github.com/vitor-soares-snps/i3c-tools.git
>
> Changes in v3:
> Use the xfer_lock to prevent device detach during ioctl call
> Expose i3cdev under /dev/bus/i3c/ folder like usb does
> Change NOTIFY_BOUND to NOTIFY_BIND, this allows the device detach occur
> before driver->probe call
> Avoid use of IS_ERR_OR_NULL
> Use u64_to_user_ptr instead of (void __user *)(uintptr_t) cast
> Allocate k_xfer and data_ptrs at once and eliminate double allocation
> check
> Pass i3cdev to dev->driver_data
> Make all minors available
> Add API documentation
>
> Changes in v2:
> Use IDR api for minor numbering
> Modify ioctl struct
> Fix SPDX license
>
> Vitor Soares (5):
> i3c: master: export i3c_masterdev_type
> i3c: master: export i3c_bus_type symbol
> i3c: master: add i3c_for_each_dev helper
> i3c: add i3cdev module to expose i3c dev in /dev
> userspace-api: add i3cdev documentation
>
> Documentation/userspace-api/i3c/i3cdev.rst | 116 ++++++++
> drivers/i3c/Kconfig | 15 +
> drivers/i3c/Makefile | 1 +
> drivers/i3c/i3cdev.c | 429 +++++++++++++++++++++++++++++
> drivers/i3c/internals.h | 2 +
> drivers/i3c/master.c | 16 +-
> include/uapi/linux/i3c/i3cdev.h | 38 +++
> 7 files changed, 616 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/userspace-api/i3c/i3cdev.rst
> create mode 100644 drivers/i3c/i3cdev.c
> create mode 100644 include/uapi/linux/i3c/i3cdev.h
>
> --
> 2.7.4
I want to make you know that none of your previous comments was ignored
and I would like to start the discussion from this point.
Best regards,
Vitor Soares
Hi Jonathan,
From: Vitor Soares <[email protected]>
Date: Wed, Feb 19, 2020 at 00:20:43
> This patch add documentation for the userspace API of i3cdev module.
>
I just realize now that I missed the commit message, it should be:
userspace-api: i3c: add i3cdev documentation
I will do next time.
> Signed-off-by: Vitor Soares <[email protected]>
> ---
> Documentation/userspace-api/i3c/i3cdev.rst | 116 +++++++++++++++++++++++++++++
> 1 file changed, 116 insertions(+)
> create mode 100644 Documentation/userspace-api/i3c/i3cdev.rst
>
> diff --git a/Documentation/userspace-api/i3c/i3cdev.rst b/Documentation/userspace-api/i3c/i3cdev.rst
> new file mode 100644
> index 0000000..ada269f
> --- /dev/null
> +++ b/Documentation/userspace-api/i3c/i3cdev.rst
> @@ -0,0 +1,116 @@
> +====================
> +I3C Device Interface
> +====================
> +
> +I3C devices have the flexibility of being accessed from userspace, as well
> +through the conventional use of kernel drivers. Userspace access, although
> +limited to private SDR I3C transfers, provides the advantage of simplifying
> +the implementation of straightforward communication protocols, applicable to
> +scenarios where transfers are dedicated such for sensor bring-up scenarios
> +(prototyping environments) or for microcontroller slave communication
> +implementation.
> +
> +The major device number is dynamically attributed and it's all reserved for
> +the i3c devices. By default, the i3cdev module only exposes the i3c devices
> +without device driver bind and aren't of master type in sort of character
> +device file under /dev/bus/i3c/ folder. They are identified through its
> +<bus id>-<Provisional ID> same way they can be found in /sys/bus/i3c/devices/.
> +::
> +
> +# ls -l /dev/bus/i3c/
> +total 0
> +crw------- 1 root root 248, 0 Jan 1 00:22 0-6072303904d2
> +crw------- 1 root root 248, 1 Jan 1 00:22 0-b7405ba00929
> +
> +The simplest way to use this interface is to not have an I3C device bound to
> +a kernel driver, this can be achieved by not have the kernel driver loaded or
> +using the Sysfs to unbind the kernel driver from the device.
> +
> +BASIC CHARACTER DEVICE API
> +===============================
> +For now, the API has only support private SDR read and write transfers.
> +Those transaction can be achieved by the following:
> +
> +``read(file, buffer, sizeof(buffer))``
> + The standard read() operation will work as a simple transaction of private
> + SDR read data followed a stop.
> + Return the number of bytes read on success, and a negative error otherwise.
> +
> +``write(file, buffer, sizeof(buffer))``
> + The standard write() operation will work as a simple transaction of private
> + SDR write data followed a stop.
> + Return the number of bytes written on success, and a negative error otherwise.
> +
> +``ioctl(file, I3C_IOC_PRIV_XFER(nxfers), struct i3c_ioc_priv_xfer *xfers)``
> + It combines read/write transactions without a stop in between.
> + Return 0 on success, and a negative error otherwise.
> +
> +NOTES:
> + - According to the MIPI I3C Protocol is the I3C slave that terminates the read
> + transaction otherwise Master can abort early on ninth (T) data bit of each
> + SDR data word.
> +
> + - Normal open() and close() operations on /dev/bus/i3c/<bus>-<provisional id>
> + files work as you would expect.
> +
> + - As documented in cdev_del() if a device was already open during
> + i3cdev_detach, the read(), write() and ioctl() fops will still be callable
> + yet they will return -EACCES.
> +
> +C EXAMPLE
> +=========
> +Working with I3C devices is much like working with files. You will need to open
> +a file descriptor, do some I/O operations with it, and then close it.
> +
> +The following header files should be included in an I3C program::
> +
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <sys/ioctl.h>
> +#include <linux/types.h>
> +#include <linux/i3c/i3cdev.h>
> +
> +To work with an I3C device, the application must open the driver, made
> +available at the device node::
> +
> + int file;
> +
> + file = open("/dev/bus/i3c/0-6072303904d2", O_RDWR);
> + if (file < 0)
> + exit(1);
> +
> +Now the file is opened, we can perform the operations available::
> +
> + /* Write function */
> + uint_t8 buf[] = {0x00, 0xde, 0xad, 0xbe, 0xef}
> + if (write(file, buf, 5) != 5) {
> + /* ERROR HANDLING: I3C transaction failed */
> + }
> +
> + /* Read function */
> + ret = read(file, buf, 5);
> + If (ret < 0) {
> + /* ERROR HANDLING: I3C transaction failed */
> + } else {
> + /* Iterate over buf[] to get the read data */
> + }
> +
> + /* IOCTL function */
> + struct i3c_ioc_priv_xfer xfers[2];
> +
> + uint8_t tx_buf[] = {0x00, 0xde, 0xad, 0xbe, 0xef};
> + uint8_t rx_buf[10];
> +
> + xfers[0].data = (uintptr_t)tx_buf;
> + xfers[0].len = 5;
> + xfers[0].rnw = 0;
> + xfers[1].data = (uintptr_t)rx_buf;
> + xfers[1].len = 10;
> + xfers[1].rnw = 1;
> +
> + if (ioctl(file, I3C_IOC_PRIV_XFER(2), xfers) < 0)
> + /* ERROR HANDLING: I3C transaction failed */
> +
> +The device can be closed when the open file descriptor is no longer required::
> +
> + close(file);
> \ No newline at end of file
> --
> 2.7.4
Best regards,
Vitor Soares
On 2/18/20 4:20 PM, Vitor Soares wrote:
> This patch add documentation for the userspace API of i3cdev module.
>
> Signed-off-by: Vitor Soares <[email protected]>
> ---
> Documentation/userspace-api/i3c/i3cdev.rst | 116 +++++++++++++++++++++++++++++
> 1 file changed, 116 insertions(+)
> create mode 100644 Documentation/userspace-api/i3c/i3cdev.rst
>
> diff --git a/Documentation/userspace-api/i3c/i3cdev.rst b/Documentation/userspace-api/i3c/i3cdev.rst
> new file mode 100644
> index 0000000..ada269f
> --- /dev/null
> +++ b/Documentation/userspace-api/i3c/i3cdev.rst
> @@ -0,0 +1,116 @@
> +====================
> +I3C Device Interface
> +====================
> +
> +I3C devices have the flexibility of being accessed from userspace, as well
> +through the conventional use of kernel drivers. Userspace access, although
> +limited to private SDR I3C transfers, provides the advantage of simplifying
> +the implementation of straightforward communication protocols, applicable to
> +scenarios where transfers are dedicated such for sensor bring-up scenarios
> +(prototyping environments) or for microcontroller slave communication
> +implementation.
> +
> +The major device number is dynamically attributed and it's all reserved for
allocated (?)
> +the i3c devices. By default, the i3cdev module only exposes the i3c devices
I3C I3C
> +without device driver bind and aren't of master type in sort of character
> +device file under /dev/bus/i3c/ folder. They are identified through its
IMO: s/folder/directory/ or sub-directory
> +<bus id>-<Provisional ID> same way they can be found in /sys/bus/i3c/devices/.
in the same way
> +::
> +
> +# ls -l /dev/bus/i3c/
> +total 0
> +crw------- 1 root root 248, 0 Jan 1 00:22 0-6072303904d2
> +crw------- 1 root root 248, 1 Jan 1 00:22 0-b7405ba00929
> +
> +The simplest way to use this interface is to not have an I3C device bound to
> +a kernel driver, this can be achieved by not have the kernel driver loaded or
driver. This by not having
> +using the Sysfs to unbind the kernel driver from the device.
the sysfs interface to unbind
> +
> +BASIC CHARACTER DEVICE API
> +===============================
> +For now, the API has only support private SDR read and write transfers.
only support for private
For the unfamiliar, what is this "SDR"? (thanks)
> +Those transaction can be achieved by the following:
> +
> +``read(file, buffer, sizeof(buffer))``
> + The standard read() operation will work as a simple transaction of private
> + SDR read data followed a stop.
> + Return the number of bytes read on success, and a negative error otherwise.
> +
> +``write(file, buffer, sizeof(buffer))``
> + The standard write() operation will work as a simple transaction of private
> + SDR write data followed a stop.
> + Return the number of bytes written on success, and a negative error otherwise.
> +
> +``ioctl(file, I3C_IOC_PRIV_XFER(nxfers), struct i3c_ioc_priv_xfer *xfers)``
> + It combines read/write transactions without a stop in between.
> + Return 0 on success, and a negative error otherwise.
> +
> +NOTES:
> + - According to the MIPI I3C Protocol is the I3C slave that terminates the read
it is the I3C slave
> + transaction otherwise Master can abort early on ninth (T) data bit of each
> + SDR data word.
> +
> + - Normal open() and close() operations on /dev/bus/i3c/<bus>-<provisional id>
> + files work as you would expect.
> +
> + - As documented in cdev_del() if a device was already open during
> + i3cdev_detach, the read(), write() and ioctl() fops will still be callable
> + yet they will return -EACCES.
> +
> +C EXAMPLE
> +=========
> +Working with I3C devices is much like working with files. You will need to open
> +a file descriptor, do some I/O operations with it, and then close it.
> +
> +The following header files should be included in an I3C program::
> +
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <sys/ioctl.h>
> +#include <linux/types.h>
> +#include <linux/i3c/i3cdev.h>
> +
> +To work with an I3C device, the application must open the driver, made
> +available at the device node::
> +
> + int file;
> +
> + file = open("/dev/bus/i3c/0-6072303904d2", O_RDWR);
> + if (file < 0)
> + exit(1);
better indentation?
> +
> +Now the file is opened, we can perform the operations available::
> +
> + /* Write function */
> + uint_t8 buf[] = {0x00, 0xde, 0xad, 0xbe, 0xef}
I can't find uint_t8. Where is it located?
and the braces should end with a ';'.
> + if (write(file, buf, 5) != 5) {
> + /* ERROR HANDLING: I3C transaction failed */
> + }
> +
> + /* Read function */
> + ret = read(file, buf, 5);
> + If (ret < 0) {
> + /* ERROR HANDLING: I3C transaction failed */
> + } else {
> + /* Iterate over buf[] to get the read data */
> + }
> +
> + /* IOCTL function */
> + struct i3c_ioc_priv_xfer xfers[2];
> +
> + uint8_t tx_buf[] = {0x00, 0xde, 0xad, 0xbe, 0xef};
> + uint8_t rx_buf[10];
> +
> + xfers[0].data = (uintptr_t)tx_buf;
> + xfers[0].len = 5;
> + xfers[0].rnw = 0;
> + xfers[1].data = (uintptr_t)rx_buf;
> + xfers[1].len = 10;
> + xfers[1].rnw = 1;
> +
> + if (ioctl(file, I3C_IOC_PRIV_XFER(2), xfers) < 0)
> + /* ERROR HANDLING: I3C transaction failed */
> +
> +The device can be closed when the open file descriptor is no longer required::
> +
> + close(file);
> \ No newline at end of file
Please fix that warning. ^^^^^
--
~Randy
On Wed, 19 Feb 2020 00:39:31 +0000
Vitor Soares <[email protected]> wrote:
> Hi Boris,
>
> From: Vitor Soares <[email protected]>
> Date: Wed, Feb 19, 2020 at 00:20:38
>
> > For today there is no way to use i3c devices from user space and
> > the introduction of such API will help developers during the i3c device
> > or i3c host controllers development.
> >
> > The i3cdev module is highly based on i2c-dev and yet I tried to address
> > the concerns raised in [1].
> >
> > NOTES:
> > - The i3cdev dynamically request an unused major number.
> >
> > - The i3c devices are dynamically exposed/removed from dev/ folder based
> > on if they have a device driver bound to it.
> >
> > - For now, the module exposes i3c devices without device driver on
> > dev/bus/i3c/<bus>-<pid>
> >
> > - As in the i2c subsystem, here it is exposed the i3c_priv_xfer to
> > userspace. I tried to use a dedicated structure as in spidev but I don't
> > see any obvious advantage.
> >
> > - Since the i3c API only exposes i3c_priv_xfer to devices, for now, the
> > module just makes use of one ioctl(). This can change in the future with
> > the introduction hdr commands or by the need of exposing some CCC
> > commands to the device API (private contract between master-slave).
> > Regarding the i3c device info, some information is already available
> > through sysfs. We can add more device attributes to expose more
> > information or add a dedicated ioctl() request for that purpose or both.
> >
> > - Similar to i2c, I have also created a tool that you can find in [2]
> > for testing purposes. If you have some time available I would appreciate
> > your feedback about it as well.
> >
> > [1] https://lkml.org/lkml/2018/11/15/853
> > [2] https://github.com/vitor-soares-snps/i3c-tools.git
> >
> > Changes in v3:
> > Use the xfer_lock to prevent device detach during ioctl call
> > Expose i3cdev under /dev/bus/i3c/ folder like usb does
> > Change NOTIFY_BOUND to NOTIFY_BIND, this allows the device detach occur
> > before driver->probe call
> > Avoid use of IS_ERR_OR_NULL
> > Use u64_to_user_ptr instead of (void __user *)(uintptr_t) cast
> > Allocate k_xfer and data_ptrs at once and eliminate double allocation
> > check
> > Pass i3cdev to dev->driver_data
> > Make all minors available
> > Add API documentation
> >
> > Changes in v2:
> > Use IDR api for minor numbering
> > Modify ioctl struct
> > Fix SPDX license
> >
> > Vitor Soares (5):
> > i3c: master: export i3c_masterdev_type
> > i3c: master: export i3c_bus_type symbol
> > i3c: master: add i3c_for_each_dev helper
> > i3c: add i3cdev module to expose i3c dev in /dev
> > userspace-api: add i3cdev documentation
> >
> > Documentation/userspace-api/i3c/i3cdev.rst | 116 ++++++++
> > drivers/i3c/Kconfig | 15 +
> > drivers/i3c/Makefile | 1 +
> > drivers/i3c/i3cdev.c | 429 +++++++++++++++++++++++++++++
> > drivers/i3c/internals.h | 2 +
> > drivers/i3c/master.c | 16 +-
> > include/uapi/linux/i3c/i3cdev.h | 38 +++
> > 7 files changed, 616 insertions(+), 1 deletion(-)
> > create mode 100644 Documentation/userspace-api/i3c/i3cdev.rst
> > create mode 100644 drivers/i3c/i3cdev.c
> > create mode 100644 include/uapi/linux/i3c/i3cdev.h
> >
> > --
> > 2.7.4
>
> I want to make you know that none of your previous comments was ignored
> and I would like to start the discussion from this point.
Sure, np. I'll probably wait for a v4 exploring the option I proposed
then.
Hi Randy,
From: Randy Dunlap <[email protected]>
Date: Wed, Feb 19, 2020 at 04:34:00
> On 2/18/20 4:20 PM, Vitor Soares wrote:
> > This patch add documentation for the userspace API of i3cdev module.
> >
> > Signed-off-by: Vitor Soares <[email protected]>
> > ---
> > Documentation/userspace-api/i3c/i3cdev.rst | 116 +++++++++++++++++++++++++++++
> > 1 file changed, 116 insertions(+)
> > create mode 100644 Documentation/userspace-api/i3c/i3cdev.rst
> >
> > diff --git a/Documentation/userspace-api/i3c/i3cdev.rst b/Documentation/userspace-api/i3c/i3cdev.rst
> > new file mode 100644
> > index 0000000..ada269f
> > --- /dev/null
> > +++ b/Documentation/userspace-api/i3c/i3cdev.rst
> > @@ -0,0 +1,116 @@
> > +====================
> > +I3C Device Interface
> > +====================
> > +
> > +I3C devices have the flexibility of being accessed from userspace, as well
> > +through the conventional use of kernel drivers. Userspace access, although
> > +limited to private SDR I3C transfers, provides the advantage of simplifying
> > +the implementation of straightforward communication protocols, applicable to
> > +scenarios where transfers are dedicated such for sensor bring-up scenarios
> > +(prototyping environments) or for microcontroller slave communication
> > +implementation.
> > +
> > +The major device number is dynamically attributed and it's all reserved for
>
> allocated (?)
>
> > +the i3c devices. By default, the i3cdev module only exposes the i3c devices
>
> I3C I3C
>
> > +without device driver bind and aren't of master type in sort of character
> > +device file under /dev/bus/i3c/ folder. They are identified through its
>
> IMO: s/folder/directory/ or sub-directory
>
> > +<bus id>-<Provisional ID> same way they can be found in /sys/bus/i3c/devices/.
>
> in the same way
>
> > +::
> > +
> > +# ls -l /dev/bus/i3c/
> > +total 0
> > +crw------- 1 root root 248, 0 Jan 1 00:22 0-6072303904d2
> > +crw------- 1 root root 248, 1 Jan 1 00:22 0-b7405ba00929
> > +
> > +The simplest way to use this interface is to not have an I3C device bound to
> > +a kernel driver, this can be achieved by not have the kernel driver loaded or
>
> driver. This by not having
>
> > +using the Sysfs to unbind the kernel driver from the device.
>
> the sysfs interface to unbind
>
> > +
> > +BASIC CHARACTER DEVICE API
> > +===============================
> > +For now, the API has only support private SDR read and write transfers.
>
> only support for private
>
> For the unfamiliar, what is this "SDR"? (thanks)
SDR stands for Single Data Rate. In I3C we can also have High Data Rate
(HDR) modes:
- Double Data Rate (HDR-DDR);
- Ternary Symbol Legacy (HDR-TSL)
- Ternary Symbol for Pure Bus (no I2C devices present on the bus)
Should I use Single Data Rate instead SDR for the first time?
>
> > +Those transaction can be achieved by the following:
> > +
> > +``read(file, buffer, sizeof(buffer))``
> > + The standard read() operation will work as a simple transaction of private
> > + SDR read data followed a stop.
> > + Return the number of bytes read on success, and a negative error otherwise.
> > +
> > +``write(file, buffer, sizeof(buffer))``
> > + The standard write() operation will work as a simple transaction of private
> > + SDR write data followed a stop.
> > + Return the number of bytes written on success, and a negative error otherwise.
> > +
> > +``ioctl(file, I3C_IOC_PRIV_XFER(nxfers), struct i3c_ioc_priv_xfer *xfers)``
> > + It combines read/write transactions without a stop in between.
> > + Return 0 on success, and a negative error otherwise.
> > +
> > +NOTES:
> > + - According to the MIPI I3C Protocol is the I3C slave that terminates the read
>
> it is the I3C slave
>
> > + transaction otherwise Master can abort early on ninth (T) data bit of each
> > + SDR data word.
> > +
> > + - Normal open() and close() operations on /dev/bus/i3c/<bus>-<provisional id>
> > + files work as you would expect.
> > +
> > + - As documented in cdev_del() if a device was already open during
> > + i3cdev_detach, the read(), write() and ioctl() fops will still be callable
> > + yet they will return -EACCES.
> > +
> > +C EXAMPLE
> > +=========
> > +Working with I3C devices is much like working with files. You will need to open
> > +a file descriptor, do some I/O operations with it, and then close it.
> > +
> > +The following header files should be included in an I3C program::
> > +
> > +#include <fcntl.h>
> > +#include <unistd.h>
> > +#include <sys/ioctl.h>
> > +#include <linux/types.h>
> > +#include <linux/i3c/i3cdev.h>
> > +
> > +To work with an I3C device, the application must open the driver, made
> > +available at the device node::
> > +
> > + int file;
> > +
> > + file = open("/dev/bus/i3c/0-6072303904d2", O_RDWR);
> > + if (file < 0)
> > + exit(1);
>
> better indentation?
>
> > +
> > +Now the file is opened, we can perform the operations available::
> > +
> > + /* Write function */
> > + uint_t8 buf[] = {0x00, 0xde, 0xad, 0xbe, 0xef}
>
> I can't find uint_t8. Where is it located?
Typo ☹.
> and the braces should end with a ';'.
>
> > + if (write(file, buf, 5) != 5) {
> > + /* ERROR HANDLING: I3C transaction failed */
> > + }
> > +
> > + /* Read function */
> > + ret = read(file, buf, 5);
> > + If (ret < 0) {
> > + /* ERROR HANDLING: I3C transaction failed */
> > + } else {
> > + /* Iterate over buf[] to get the read data */
> > + }
> > +
> > + /* IOCTL function */
> > + struct i3c_ioc_priv_xfer xfers[2];
> > +
> > + uint8_t tx_buf[] = {0x00, 0xde, 0xad, 0xbe, 0xef};
> > + uint8_t rx_buf[10];
> > +
> > + xfers[0].data = (uintptr_t)tx_buf;
> > + xfers[0].len = 5;
> > + xfers[0].rnw = 0;
> > + xfers[1].data = (uintptr_t)rx_buf;
> > + xfers[1].len = 10;
> > + xfers[1].rnw = 1;
> > +
> > + if (ioctl(file, I3C_IOC_PRIV_XFER(2), xfers) < 0)
> > + /* ERROR HANDLING: I3C transaction failed */
> > +
> > +The device can be closed when the open file descriptor is no longer required::
> > +
> > + close(file);
> > \ No newline at end of file
>
> Please fix that warning. ^^^^^
>
>
> --
> ~Randy
Thanks for your feedback. I will address them next version.
Best regards,
Vitor Soares
On 2/21/20 2:31 AM, Vitor Soares wrote:
>>> +BASIC CHARACTER DEVICE API
>>> +===============================
>>> +For now, the API has only support private SDR read and write transfers.
>> only support for private
>>
>> For the unfamiliar, what is this "SDR"? (thanks)
> SDR stands for Single Data Rate. In I3C we can also have High Data Rate
> (HDR) modes:
> - Double Data Rate (HDR-DDR);
> - Ternary Symbol Legacy (HDR-TSL)
> - Ternary Symbol for Pure Bus (no I2C devices present on the bus)
>
> Should I use Single Data Rate instead SDR for the first time?
Yes, please.
thanks.
--
~Randy
Hi Boris,
From: Boris Brezillon <[email protected]>
Date: Wed, Feb 19, 2020 at 08:16:58
> On Wed, 19 Feb 2020 00:39:31 +0000
> Vitor Soares <[email protected]> wrote:
>
> > Hi Boris,
> >
> > From: Vitor Soares <[email protected]>
> > Date: Wed, Feb 19, 2020 at 00:20:38
> >
> > > For today there is no way to use i3c devices from user space and
> > > the introduction of such API will help developers during the i3c device
> > > or i3c host controllers development.
> > >
> > > The i3cdev module is highly based on i2c-dev and yet I tried to address
> > > the concerns raised in [1].
> > >
> > > NOTES:
> > > - The i3cdev dynamically request an unused major number.
> > >
> > > - The i3c devices are dynamically exposed/removed from dev/ folder based
> > > on if they have a device driver bound to it.
> > >
> > > - For now, the module exposes i3c devices without device driver on
> > > dev/bus/i3c/<bus>-<pid>
> > >
> > > - As in the i2c subsystem, here it is exposed the i3c_priv_xfer to
> > > userspace. I tried to use a dedicated structure as in spidev but I don't
> > > see any obvious advantage.
> > >
> > > - Since the i3c API only exposes i3c_priv_xfer to devices, for now, the
> > > module just makes use of one ioctl(). This can change in the future with
> > > the introduction hdr commands or by the need of exposing some CCC
> > > commands to the device API (private contract between master-slave).
> > > Regarding the i3c device info, some information is already available
> > > through sysfs. We can add more device attributes to expose more
> > > information or add a dedicated ioctl() request for that purpose or both.
> > >
> > > - Similar to i2c, I have also created a tool that you can find in [2]
> > > for testing purposes. If you have some time available I would appreciate
> > > your feedback about it as well.
> > >
> > > [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2018_11_15_853&d=DwICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=qVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=1N2OHEqhXNvZn7HIjf_EmPX5N9wwBWLsUakJ9a1zno0&s=RJ3Lr0SIvJYiKxLCFaV0_pNArfUJj7TxsJecNgYbTQA&e=
> > > [2] https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_vitor-2Dsoares-2Dsnps_i3c-2Dtools.git&d=DwICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=qVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=1N2OHEqhXNvZn7HIjf_EmPX5N9wwBWLsUakJ9a1zno0&s=q4Oi7UD-dZ1YMlTN9prqycS1G1QvEUuBFnsABMGRuBk&e=
> > >
> > > Changes in v3:
> > > Use the xfer_lock to prevent device detach during ioctl call
> > > Expose i3cdev under /dev/bus/i3c/ folder like usb does
> > > Change NOTIFY_BOUND to NOTIFY_BIND, this allows the device detach occur
> > > before driver->probe call
> > > Avoid use of IS_ERR_OR_NULL
> > > Use u64_to_user_ptr instead of (void __user *)(uintptr_t) cast
> > > Allocate k_xfer and data_ptrs at once and eliminate double allocation
> > > check
> > > Pass i3cdev to dev->driver_data
> > > Make all minors available
> > > Add API documentation
> > >
> > > Changes in v2:
> > > Use IDR api for minor numbering
> > > Modify ioctl struct
> > > Fix SPDX license
> > >
> > > Vitor Soares (5):
> > > i3c: master: export i3c_masterdev_type
> > > i3c: master: export i3c_bus_type symbol
> > > i3c: master: add i3c_for_each_dev helper
> > > i3c: add i3cdev module to expose i3c dev in /dev
> > > userspace-api: add i3cdev documentation
> > >
> > > Documentation/userspace-api/i3c/i3cdev.rst | 116 ++++++++
> > > drivers/i3c/Kconfig | 15 +
> > > drivers/i3c/Makefile | 1 +
> > > drivers/i3c/i3cdev.c | 429 +++++++++++++++++++++++++++++
> > > drivers/i3c/internals.h | 2 +
> > > drivers/i3c/master.c | 16 +-
> > > include/uapi/linux/i3c/i3cdev.h | 38 +++
> > > 7 files changed, 616 insertions(+), 1 deletion(-)
> > > create mode 100644 Documentation/userspace-api/i3c/i3cdev.rst
> > > create mode 100644 drivers/i3c/i3cdev.c
> > > create mode 100644 include/uapi/linux/i3c/i3cdev.h
> > >
> > > --
> > > 2.7.4
> >
> > I want to make you know that none of your previous comments was ignored
> > and I would like to start the discussion from this point.
>
> Sure, np. I'll probably wait for a v4 exploring the option I proposed
> then.
I would like to check with you:
- How can we prioritize the device driver over the i3cdev driver if the
driver is loaded after i3cdev? Currently, this is done automatically
without any command, and for me, this is a requirement.
- For the ioctl command structure, there is no rule about the way I did
or what you proposed, both are currently used in the kernel. For me it is
one more structure to deal with, can you point the advantages of your
purpose?
- Regarding the ioctl codes, I tried to use those after I2C.
Best regards,
Vitor Soares
On Fri, 21 Feb 2020 17:08:17 +0000
Vitor Soares <[email protected]> wrote:
> Hi Boris,
>
> From: Boris Brezillon <[email protected]>
> Date: Wed, Feb 19, 2020 at 08:16:58
>
> > On Wed, 19 Feb 2020 00:39:31 +0000
> > Vitor Soares <[email protected]> wrote:
> >
> > > Hi Boris,
> > >
> > > From: Vitor Soares <[email protected]>
> > > Date: Wed, Feb 19, 2020 at 00:20:38
> > >
> > > > For today there is no way to use i3c devices from user space and
> > > > the introduction of such API will help developers during the i3c device
> > > > or i3c host controllers development.
> > > >
> > > > The i3cdev module is highly based on i2c-dev and yet I tried to address
> > > > the concerns raised in [1].
> > > >
> > > > NOTES:
> > > > - The i3cdev dynamically request an unused major number.
> > > >
> > > > - The i3c devices are dynamically exposed/removed from dev/ folder based
> > > > on if they have a device driver bound to it.
> > > >
> > > > - For now, the module exposes i3c devices without device driver on
> > > > dev/bus/i3c/<bus>-<pid>
> > > >
> > > > - As in the i2c subsystem, here it is exposed the i3c_priv_xfer to
> > > > userspace. I tried to use a dedicated structure as in spidev but I don't
> > > > see any obvious advantage.
> > > >
> > > > - Since the i3c API only exposes i3c_priv_xfer to devices, for now, the
> > > > module just makes use of one ioctl(). This can change in the future with
> > > > the introduction hdr commands or by the need of exposing some CCC
> > > > commands to the device API (private contract between master-slave).
> > > > Regarding the i3c device info, some information is already available
> > > > through sysfs. We can add more device attributes to expose more
> > > > information or add a dedicated ioctl() request for that purpose or both.
> > > >
> > > > - Similar to i2c, I have also created a tool that you can find in [2]
> > > > for testing purposes. If you have some time available I would appreciate
> > > > your feedback about it as well.
> > > >
> > > > [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2018_11_15_853&d=DwICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=qVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=1N2OHEqhXNvZn7HIjf_EmPX5N9wwBWLsUakJ9a1zno0&s=RJ3Lr0SIvJYiKxLCFaV0_pNArfUJj7TxsJecNgYbTQA&e=
> > > > [2] https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_vitor-2Dsoares-2Dsnps_i3c-2Dtools.git&d=DwICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=qVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=1N2OHEqhXNvZn7HIjf_EmPX5N9wwBWLsUakJ9a1zno0&s=q4Oi7UD-dZ1YMlTN9prqycS1G1QvEUuBFnsABMGRuBk&e=
> > > >
> > > > Changes in v3:
> > > > Use the xfer_lock to prevent device detach during ioctl call
> > > > Expose i3cdev under /dev/bus/i3c/ folder like usb does
> > > > Change NOTIFY_BOUND to NOTIFY_BIND, this allows the device detach occur
> > > > before driver->probe call
> > > > Avoid use of IS_ERR_OR_NULL
> > > > Use u64_to_user_ptr instead of (void __user *)(uintptr_t) cast
> > > > Allocate k_xfer and data_ptrs at once and eliminate double allocation
> > > > check
> > > > Pass i3cdev to dev->driver_data
> > > > Make all minors available
> > > > Add API documentation
> > > >
> > > > Changes in v2:
> > > > Use IDR api for minor numbering
> > > > Modify ioctl struct
> > > > Fix SPDX license
> > > >
> > > > Vitor Soares (5):
> > > > i3c: master: export i3c_masterdev_type
> > > > i3c: master: export i3c_bus_type symbol
> > > > i3c: master: add i3c_for_each_dev helper
> > > > i3c: add i3cdev module to expose i3c dev in /dev
> > > > userspace-api: add i3cdev documentation
> > > >
> > > > Documentation/userspace-api/i3c/i3cdev.rst | 116 ++++++++
> > > > drivers/i3c/Kconfig | 15 +
> > > > drivers/i3c/Makefile | 1 +
> > > > drivers/i3c/i3cdev.c | 429 +++++++++++++++++++++++++++++
> > > > drivers/i3c/internals.h | 2 +
> > > > drivers/i3c/master.c | 16 +-
> > > > include/uapi/linux/i3c/i3cdev.h | 38 +++
> > > > 7 files changed, 616 insertions(+), 1 deletion(-)
> > > > create mode 100644 Documentation/userspace-api/i3c/i3cdev.rst
> > > > create mode 100644 drivers/i3c/i3cdev.c
> > > > create mode 100644 include/uapi/linux/i3c/i3cdev.h
> > > >
> > > > --
> > > > 2.7.4
> > >
> > > I want to make you know that none of your previous comments was ignored
> > > and I would like to start the discussion from this point.
> >
> > Sure, np. I'll probably wait for a v4 exploring the option I proposed
> > then.
>
> I would like to check with you:
> - How can we prioritize the device driver over the i3cdev driver if the
> driver is loaded after i3cdev? Currently, this is done automatically
> without any command, and for me, this is a requirement.
No devs would be bound to the i3cdev driver by default, it would have
to be done explicitly through a sysfs knob. Which makes me realize
we can't use the generic bind knob since it doesn't let the subsystem
know that it's a manual bind. I thought there was a way to distinguish
between manual and auto-bind.
> - For the ioctl command structure, there is no rule about the way I did
> or what you proposed, both are currently used in the kernel. For me it is
> one more structure to deal with, can you point the advantages of your
> purpose?
I don't have a strong opinion on that one, though I find it a bit
easier to follow when the number of xfers is encoded in a separate
struct rather than extracted from the data size passed through the cmd
argument.
> - Regarding the ioctl codes, I tried to use those after I2C.
Why start from 0x30? It doesn't make sense to me. Just because you base
your code on something that already exists doesn't mean you have to
copy all of it.
Hi Boris,
From: Boris Brezillon <[email protected]>
Date: Fri, Feb 21, 2020 at 17:41:16
> > > >
> > > > I want to make you know that none of your previous comments was ignored
> > > > and I would like to start the discussion from this point.
> > >
> > > Sure, np. I'll probably wait for a v4 exploring the option I proposed
> > > then.
> >
> > I would like to check with you:
> > - How can we prioritize the device driver over the i3cdev driver if the
> > driver is loaded after i3cdev? Currently, this is done automatically
> > without any command, and for me, this is a requirement.
>
> No devs would be bound to the i3cdev driver by default, it would have
> to be done explicitly through a sysfs knob. Which makes me realize
> we can't use the generic bind knob since it doesn't let the subsystem
> know that it's a manual bind. I thought there was a way to distinguish
> between manual and auto-bind.
>
> > - For the ioctl command structure, there is no rule about the way I did
> > or what you proposed, both are currently used in the kernel. For me it is
> > one more structure to deal with, can you point the advantages of your
> > purpose?
>
> I don't have a strong opinion on that one, though I find it a bit
> easier to follow when the number of xfers is encoded in a separate
> struct rather than extracted from the data size passed through the cmd
> argument.
I will change it then. Do you have any suggestion for the naming to keep
it short?
>
> > - Regarding the ioctl codes, I tried to use those after I2C.
>
> Why start from 0x30? It doesn't make sense to me. Just because you base
> your code on something that already exists doesn't mean you have to
> copy all of it.
I might be wrong but last I2C command is 0x20 and I tried to let some
free space in case they need.
Also I think that make sense I2C and I3C share the same 'magic number'.
BTW, in ioctl-numbers documentation there is no reference for code 0x07.
Best regards,
Vitor Soares
On Mon, 24 Feb 2020 10:53:25 +0000
Vitor Soares <[email protected]> wrote:
> Hi Boris,
>
> From: Boris Brezillon <[email protected]>
> Date: Fri, Feb 21, 2020 at 17:41:16
>
> > > > >
> > > > > I want to make you know that none of your previous comments was ignored
> > > > > and I would like to start the discussion from this point.
> > > >
> > > > Sure, np. I'll probably wait for a v4 exploring the option I proposed
> > > > then.
> > >
> > > I would like to check with you:
> > > - How can we prioritize the device driver over the i3cdev driver if the
> > > driver is loaded after i3cdev? Currently, this is done automatically
> > > without any command, and for me, this is a requirement.
> >
> > No devs would be bound to the i3cdev driver by default, it would have
> > to be done explicitly through a sysfs knob. Which makes me realize
> > we can't use the generic bind knob since it doesn't let the subsystem
> > know that it's a manual bind. I thought there was a way to distinguish
> > between manual and auto-bind.
> >
> > > - For the ioctl command structure, there is no rule about the way I did
> > > or what you proposed, both are currently used in the kernel. For me it is
> > > one more structure to deal with, can you point the advantages of your
> > > purpose?
> >
> > I don't have a strong opinion on that one, though I find it a bit
> > easier to follow when the number of xfers is encoded in a separate
> > struct rather than extracted from the data size passed through the cmd
> > argument.
>
> I will change it then. Do you have any suggestion for the naming to keep
> it short?
I named it i3cdev_priv_xfers in the patch I sent, but you can pick a
different name if you don't like this one.
>
> >
> > > - Regarding the ioctl codes, I tried to use those after I2C.
> >
> > Why start from 0x30? It doesn't make sense to me. Just because you base
> > your code on something that already exists doesn't mean you have to
> > copy all of it.
>
> I might be wrong but last I2C command is 0x20 and I tried to let some
> free space in case they need.
> Also I think that make sense I2C and I3C share the same 'magic number'.
Hm, I'm not sure that's a good idea. The set of ioctls for I2C and I3C
are likely to be completely different, so I'd recommend using a
separate namespace (AKA ioctl magic number). Wolfram, any opinion?
>
> BTW, in ioctl-numbers documentation there is no reference for code 0x07.
Indeed, looks like it's not documented.