diff mbox series

[RFC,5/5] i3c: add i3cdev module to expose i3c dev in /dev

Message ID f9f20eaf900ed5629dd3d824bc1e90c7e6b4a371.1575977795.git.vitor.soares@synopsys.com (mailing list archive)
State Changes Requested
Headers show
Series Introduce i3c device userspace interface | expand

Commit Message

Vitor Soares Dec. 10, 2019, 3:37 p.m. UTC
This patch adds user-mode support to I3C SDR transfers.

The module is based on i2c-dev.c with the following features:
  - expose on /dev the i3c devices dynamically based on if they have
    a device driver bound.
  - Dynamically allocate the char device Major number.

Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
---
 drivers/i3c/Kconfig             |  15 ++
 drivers/i3c/Makefile            |   1 +
 drivers/i3c/i3cdev.c            | 438 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/i3c/i3cdev.h |  27 +++
 4 files changed, 481 insertions(+)
 create mode 100644 drivers/i3c/i3cdev.c
 create mode 100644 include/uapi/linux/i3c/i3cdev.h

Comments

Arnd Bergmann Dec. 10, 2019, 5:51 p.m. UTC | #1
On Tue, Dec 10, 2019 at 4:37 PM Vitor Soares <Vitor.Soares@synopsys.com> wrote:
>
> +/* IOCTL commands */
> +#define I3C_DEV_IOC_MAGIC      0x07
> +
> +struct i3c_ioc_priv_xfer {
> +       struct i3c_priv_xfer __user *xfers;     /* pointers to i3c_priv_xfer */
> +       __u32 nxfers;                           /* number of i3c_priv_xfer */
> +};
> +
> +#define I3C_IOC_PRIV_XFER      \
> +       _IOW(I3C_DEV_IOC_MAGIC, 30, struct i3c_ioc_priv_xfer)
> +
> +#define  I3C_IOC_PRIV_XFER_MAX_MSGS    42

This is not a great data structure for UAPI, please see
https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/tree/Documentation/core-api/ioctl.rst?h=compat-ioctl-endgame&id=927324b7900ee9b877691a8b237e272fabb21bf5

for some background. I'm planning to submit that documentation for
mainline integration soon.

     Arnd
Vitor Soares Dec. 10, 2019, 7:15 p.m. UTC | #2
Hi Arnd,

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, Dec 10, 2019 at 17:51:14

> On Tue, Dec 10, 2019 at 4:37 PM Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> >
> > +/* IOCTL commands */
> > +#define I3C_DEV_IOC_MAGIC      0x07
> > +
> > +struct i3c_ioc_priv_xfer {
> > +       struct i3c_priv_xfer __user *xfers;     /* pointers to i3c_priv_xfer */
> > +       __u32 nxfers;                           /* number of i3c_priv_xfer */
> > +};
> > +
> > +#define I3C_IOC_PRIV_XFER      \
> > +       _IOW(I3C_DEV_IOC_MAGIC, 30, struct i3c_ioc_priv_xfer)
> > +
> > +#define  I3C_IOC_PRIV_XFER_MAX_MSGS    42
> 
> This is not a great data structure for UAPI, please see
> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_arnd_playground.git_tree_Documentation_core-2Dapi_ioctl.rst-3Fh-3Dcompat-2Dioctl-2Dendgame-26id-3D927324b7900ee9b877691a8b237e272fabb21bf5&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=qVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=5Q9WjK0o93NR7DQ9NM6So6mfdgpNnZnSaP8qMpgaC7E&s=LzzjrUQAG8fx5jkVyK73dBDrahNAvk09Cxxlx3KOiXI&e= 
> 
> for some background. I'm planning to submit that documentation for
> mainline integration soon.
> 
>      Arnd

Thanks for sharing the document.

My understanding is that I should use a data structure like the struct 
spi_ioc_transfer, with this I may also use the same ioctl command 
definition. Am I right?
In the documentation you also refer the compact_ioctl() and It is not 
clear to me if the compact_ioctl() is mandatory in this case. Should I 
implement it as well?

Best regards,
Vitor Soares
Arnd Bergmann Dec. 10, 2019, 7:37 p.m. UTC | #3
On Tue, Dec 10, 2019 at 8:15 PM Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, Dec 10, 2019 at 17:51:14
>
> > On Tue, Dec 10, 2019 at 4:37 PM Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> > >
> > > +/* IOCTL commands */
> > > +#define I3C_DEV_IOC_MAGIC      0x07
> > > +
> > > +struct i3c_ioc_priv_xfer {
> > > +       struct i3c_priv_xfer __user *xfers;     /* pointers to i3c_priv_xfer */
> > > +       __u32 nxfers;                           /* number of i3c_priv_xfer */
> > > +};
> > > +
> > > +#define I3C_IOC_PRIV_XFER      \
> > > +       _IOW(I3C_DEV_IOC_MAGIC, 30, struct i3c_ioc_priv_xfer)
> > > +
> > > +#define  I3C_IOC_PRIV_XFER_MAX_MSGS    42
> >
> > This is not a great data structure for UAPI, please see
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_arnd_playground.git_tree_Documentation_core-2Dapi_ioctl.rst-3Fh-3Dcompat-2Dioctl-2Dendgame-26id-3D927324b7900ee9b877691a8b237e272fabb21bf5&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=qVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=5Q9WjK0o93NR7DQ9NM6So6mfdgpNnZnSaP8qMpgaC7E&s=LzzjrUQAG8fx5jkVyK73dBDrahNAvk09Cxxlx3KOiXI&e=
> >
> > for some background. I'm planning to submit that documentation for
> > mainline integration soon.
> >
> >      Arnd
>
> Thanks for sharing the document.
>
> My understanding is that I should use a data structure like the struct
> spi_ioc_transfer, with this I may also use the same ioctl command
> definition. Am I right?

Yes, that would be an example of a structure that follows the best
practices from my document. It is still rather complex, so if you
can make it any simpler, that would be ideal.

> In the documentation you also refer the compact_ioctl() and It is not
> clear to me if the compact_ioctl() is mandatory in this case. Should I
> implement it as well?

If the structure is defined like that, you just need to set
".compat_ioctl=compat_ptr_ioctl," in the file_operations structure
and it will work, but you cannot skip that step.

As your interface is basically just read/write based, I wonder
if there is a way to completely avoid the ioctl and instead
use io_submit() as the primary interface.

      Arnd
Vitor Soares Dec. 11, 2019, 3:07 p.m. UTC | #4
Hi Arnd,

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, Dec 10, 2019 at 19:37:14

> On Tue, Dec 10, 2019 at 8:15 PM Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > Date: Tue, Dec 10, 2019 at 17:51:14
> >
> > > On Tue, Dec 10, 2019 at 4:37 PM Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> > > >
> > > > +/* IOCTL commands */
> > > > +#define I3C_DEV_IOC_MAGIC      0x07
> > > > +
> > > > +struct i3c_ioc_priv_xfer {
> > > > +       struct i3c_priv_xfer __user *xfers;     /* pointers to i3c_priv_xfer */
> > > > +       __u32 nxfers;                           /* number of i3c_priv_xfer */
> > > > +};
> > > > +
> > > > +#define I3C_IOC_PRIV_XFER      \
> > > > +       _IOW(I3C_DEV_IOC_MAGIC, 30, struct i3c_ioc_priv_xfer)
> > > > +
> > > > +#define  I3C_IOC_PRIV_XFER_MAX_MSGS    42
> > >
> > > This is not a great data structure for UAPI, please see
> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_arnd_playground.git_tree_Documentation_core-2Dapi_ioctl.rst-3Fh-3Dcompat-2Dioctl-2Dendgame-26id-3D927324b7900ee9b877691a8b237e272fabb21bf5&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=qVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=5Q9WjK0o93NR7DQ9NM6So6mfdgpNnZnSaP8qMpgaC7E&s=LzzjrUQAG8fx5jkVyK73dBDrahNAvk09Cxxlx3KOiXI&e=
> > >
> > > for some background. I'm planning to submit that documentation for
> > > mainline integration soon.
> > >
> > >      Arnd
> >
> > Thanks for sharing the document.
> >
> > My understanding is that I should use a data structure like the struct
> > spi_ioc_transfer, with this I may also use the same ioctl command
> > definition. Am I right?
> 
> Yes, that would be an example of a structure that follows the best
> practices from my document. It is still rather complex, so if you
> can make it any simpler, that would be ideal.

I will try to do that.

> 
> > In the documentation you also refer the compact_ioctl() and It is not
> > clear to me if the compact_ioctl() is mandatory in this case. Should I
> > implement it as well?
> 
> If the structure is defined like that, you just need to set
> ".compat_ioctl=compat_ptr_ioctl," in the file_operations structure
> and it will work, but you cannot skip that step.

Thanks, now I know that is mandatory 
Arnd Bergmann Dec. 11, 2019, 3:33 p.m. UTC | #5
On Wed, Dec 11, 2019 at 4:07 PM Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, Dec 10, 2019 at 19:37:14
> > On Tue, Dec 10, 2019 at 8:15 PM Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> > > From: Arnd Bergmann <arnd@arndb.de>
> > > Date: Tue, Dec 10, 2019 at 17:51:14
> > > > On Tue, Dec 10, 2019 at 4:37 PM Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> > As your interface is basically just read/write based, I wonder
> > if there is a way to completely avoid the ioctl and instead
> > use io_submit() as the primary interface.
>
> I confess that I wasn't familiar with io_submit() until now and went
> straightway for the ioctl() approach.
> So far, my understanding is that io_submit() will call .write or .read of
> i3cdev module depending on the iocb command. if so, we won't be able to
> do a repeated start between a multiple iocb in the same list, right?

I'm not sure what you mean with "repeated start", but it's definitely
possible that io_submit() is not a useful interface for i3c. The main
advantage would be that it avoids creating a complex ioctl command.

> Apart from this private read/write need, another requirement that leads
> me to use the ioctl() was:
> - When we support HDR command in i3c subsystem we can expand the ioctl()
> command to support it.
> - For now, device API doesn't expose CCC commands but some of them are
> used for a private contract between master and device and we likely need
> that support in the future as well.

I think you could still have both the io_submit() interface for basic I/O
(if you can get it to do what you want), plus an ioctl interface for more
complex interactions.

     Arnd
Greg KH Dec. 12, 2019, 2:44 p.m. UTC | #6
On Tue, Dec 10, 2019 at 04:37:33PM +0100, Vitor Soares wrote:
> +static int __init i3cdev_init(void)
> +{
> +	int res;
> +
> +	pr_info("i3c /dev entries driver\n");

Please remove debugging information, kernel code should be quiet unless
something goes wrong.

> +	/* Dynamically request unused major number */
> +	res = alloc_chrdev_region(&i3cdev_number, 0, N_I3C_MINORS, "i3c");

Do you really need a whole major, or will a few minors work?

thanks,

greg k-h
Greg KH Dec. 12, 2019, 2:46 p.m. UTC | #7
On Tue, Dec 10, 2019 at 04:37:33PM +0100, Vitor Soares wrote:
> +static struct i3cdev_data *i3cdev_get_by_minor(unsigned int minor)

Why?  Why not just embed the structure in the cdev if you really need
it?

> +static struct i3cdev_data *get_free_i3cdev(struct i3c_device *i3c)
> +{
> +	struct i3cdev_data *i3cdev;
> +	unsigned long minor;
> +
> +	minor = find_first_zero_bit(minors, N_I3C_MINORS);

No locking, fun!!!

:(

Why not use an idr instead, that is what it is there for.  Don't try to
roll your own.

thanks,

greg k-h
Vitor Soares Dec. 12, 2019, 2:56 p.m. UTC | #8
Hi Greg,

From: Greg KH <gregkh@linuxfoundation.org>
Date: Thu, Dec 12, 2019 at 14:44:59

> On Tue, Dec 10, 2019 at 04:37:33PM +0100, Vitor Soares wrote:
> > +static int __init i3cdev_init(void)
> > +{
> > +	int res;
> > +
> > +	pr_info("i3c /dev entries driver\n");
> 
> Please remove debugging information, kernel code should be quiet unless
> something goes wrong.

I will remove it.

> 
> > +	/* Dynamically request unused major number */
> > +	res = alloc_chrdev_region(&i3cdev_number, 0, N_I3C_MINORS, "i3c");
> 
> Do you really need a whole major, or will a few minors work?
> 
> thanks,
> 
> greg k-h

I'm reserving one per device. What do you suggest?

Best regards,
Vitor Soares
Greg KH Dec. 12, 2019, 4 p.m. UTC | #9
On Thu, Dec 12, 2019 at 02:56:56PM +0000, Vitor Soares wrote:
> Hi Greg,
> 
> From: Greg KH <gregkh@linuxfoundation.org>
> Date: Thu, Dec 12, 2019 at 14:44:59
> 
> > On Tue, Dec 10, 2019 at 04:37:33PM +0100, Vitor Soares wrote:
> > > +static int __init i3cdev_init(void)
> > > +{
> > > +	int res;
> > > +
> > > +	pr_info("i3c /dev entries driver\n");
> > 
> > Please remove debugging information, kernel code should be quiet unless
> > something goes wrong.
> 
> I will remove it.
> 
> > 
> > > +	/* Dynamically request unused major number */
> > > +	res = alloc_chrdev_region(&i3cdev_number, 0, N_I3C_MINORS, "i3c");
> > 
> > Do you really need a whole major, or will a few minors work?
> > 
> > thanks,
> > 
> > greg k-h
> 
> I'm reserving one per device. What do you suggest?

How many devices do you have in a system?
Vitor Soares Dec. 12, 2019, 5:25 p.m. UTC | #10
From: Greg KH <gregkh@linuxfoundation.org>
Date: Thu, Dec 12, 2019 at 16:00:45

> On Thu, Dec 12, 2019 at 02:56:56PM +0000, Vitor Soares wrote:
> > Hi Greg,
> > 
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Date: Thu, Dec 12, 2019 at 14:44:59
> > 
> > > On Tue, Dec 10, 2019 at 04:37:33PM +0100, Vitor Soares wrote:
> > > > +static int __init i3cdev_init(void)
> > > > +{
> > > > +	int res;
> > > > +
> > > > +	pr_info("i3c /dev entries driver\n");
> > > 
> > > Please remove debugging information, kernel code should be quiet unless
> > > something goes wrong.
> > 
> > I will remove it.
> > 
> > > 
> > > > +	/* Dynamically request unused major number */
> > > > +	res = alloc_chrdev_region(&i3cdev_number, 0, N_I3C_MINORS, "i3c");
> > > 
> > > Do you really need a whole major, or will a few minors work?
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > 
> > I'm reserving one per device. What do you suggest?
> 
> How many devices do you have in a system?

According to MIPI I3C spec, the maximum number of devices on a bus will 
depend on trace length, bus load, and clock requirements.

Frankly, I don't know what is the best compromise because it depends from 
system to system and the end-use of it. In my case, I started developing 
this to help me during the HC controller driver development.
Even If I choose a fixed major, I wouldn't know in which one i3c fits.

Best regards,
Vitor Soares
Greg KH Dec. 12, 2019, 5:32 p.m. UTC | #11
On Thu, Dec 12, 2019 at 05:25:28PM +0000, Vitor Soares wrote:
> From: Greg KH <gregkh@linuxfoundation.org>
> Date: Thu, Dec 12, 2019 at 16:00:45
> 
> > On Thu, Dec 12, 2019 at 02:56:56PM +0000, Vitor Soares wrote:
> > > Hi Greg,
> > > 
> > > From: Greg KH <gregkh@linuxfoundation.org>
> > > Date: Thu, Dec 12, 2019 at 14:44:59
> > > 
> > > > On Tue, Dec 10, 2019 at 04:37:33PM +0100, Vitor Soares wrote:
> > > > > +static int __init i3cdev_init(void)
> > > > > +{
> > > > > +	int res;
> > > > > +
> > > > > +	pr_info("i3c /dev entries driver\n");
> > > > 
> > > > Please remove debugging information, kernel code should be quiet unless
> > > > something goes wrong.
> > > 
> > > I will remove it.
> > > 
> > > > 
> > > > > +	/* Dynamically request unused major number */
> > > > > +	res = alloc_chrdev_region(&i3cdev_number, 0, N_I3C_MINORS, "i3c");
> > > > 
> > > > Do you really need a whole major, or will a few minors work?
> > > > 
> > > > thanks,
> > > > 
> > > > greg k-h
> > > 
> > > I'm reserving one per device. What do you suggest?
> > 
> > How many devices do you have in a system?
> 
> According to MIPI I3C spec, the maximum number of devices on a bus will 
> depend on trace length, bus load, and clock requirements.
> 
> Frankly, I don't know what is the best compromise because it depends from 
> system to system and the end-use of it. In my case, I started developing 
> this to help me during the HC controller driver development.
> Even If I choose a fixed major, I wouldn't know in which one i3c fits.

Ok, a full major will be fine, I was worried you only wanted 1 or 2
minor numbers, which would mean you could have used a misc device
instead.

This is fine.

thanks,

greg k-h
Vitor Soares Dec. 20, 2019, 12:39 p.m. UTC | #12
Hi Arnd,

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, Dec 10, 2019 at 17:51:14

> On Tue, Dec 10, 2019 at 4:37 PM Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> >
> > +/* IOCTL commands */
> > +#define I3C_DEV_IOC_MAGIC      0x07
> > +
> > +struct i3c_ioc_priv_xfer {
> > +       struct i3c_priv_xfer __user *xfers;     /* pointers to i3c_priv_xfer */
> > +       __u32 nxfers;                           /* number of i3c_priv_xfer */
> > +};
> > +
> > +#define I3C_IOC_PRIV_XFER      \
> > +       _IOW(I3C_DEV_IOC_MAGIC, 30, struct i3c_ioc_priv_xfer)
> > +
> > +#define  I3C_IOC_PRIV_XFER_MAX_MSGS    42
> 
> This is not a great data structure for UAPI, please see
> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_arnd_playground.git_tree_Documentation_core-2Dapi_ioctl.rst-3Fh-3Dcompat-2Dioctl-2Dendgame-26id-3D927324b7900ee9b877691a8b237e272fabb21bf5&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=qVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=5Q9WjK0o93NR7DQ9NM6So6mfdgpNnZnSaP8qMpgaC7E&s=LzzjrUQAG8fx5jkVyK73dBDrahNAvk09Cxxlx3KOiXI&e= 
> 
> for some background. I'm planning to submit that documentation for
> mainline integration soon.
> 
>      Arnd

I was checking other code examples for this and found that some authors 
add extra reserved fields for future use. Should I do the same?
 
I have some doubt too if it is ok to use the same ioctl command approach 
as in spidev:

#define SPI_MSGSIZE(N) \
  	((((N)*(sizeof (struct spi_ioc_transfer))) < (1 << _IOC_SIZEBITS)) \
		? ((N)*(sizeof (struct spi_ioc_transfer))) : 0)
#define SPI_IOC_MESSAGE(N) _IOW(SPI_IOC_MAGIC, 0, char[SPI_MSGSIZE(N)])

Or the best is to use another structure to embedded N transfers like in 
this patch.

Thanks in advance for your help.
Best regards,
Vitor Soares
diff mbox series

Patch

diff --git a/drivers/i3c/Kconfig b/drivers/i3c/Kconfig
index 30a4415..0164276 100644
--- a/drivers/i3c/Kconfig
+++ b/drivers/i3c/Kconfig
@@ -20,5 +20,20 @@  menuconfig I3C
 	  will be called i3c.
 
 if I3C
+
+config I3CDEV
+	tristate "I3C device interface"
+	depends on I3C
+	help
+	  Say Y here to use i3c-* device files, usually found in the /dev
+	  directory on your system.  They make it possible to have user-space
+	  programs use the I3C devices.
+
+	  This support is also available as a module.  If so, the module
+	  will be called i3cdev.
+
+	  Note that this application programming interface is EXPERIMENTAL
+	  and hence SUBJECT TO CHANGE WITHOUT NOTICE while it stabilizes.
+
 source "drivers/i3c/master/Kconfig"
 endif # I3C
diff --git a/drivers/i3c/Makefile b/drivers/i3c/Makefile
index 11982ef..606d422 100644
--- a/drivers/i3c/Makefile
+++ b/drivers/i3c/Makefile
@@ -1,4 +1,5 @@ 
 # SPDX-License-Identifier: GPL-2.0
 i3c-y				:= device.o master.o
 obj-$(CONFIG_I3C)		+= i3c.o
+obj-$(CONFIG_I3CDEV)		+= i3cdev.o
 obj-$(CONFIG_I3C)		+= master/
diff --git a/drivers/i3c/i3cdev.c b/drivers/i3c/i3cdev.c
new file mode 100644
index 0000000..4d4b83c
--- /dev/null
+++ b/drivers/i3c/i3cdev.c
@@ -0,0 +1,438 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 Synopsys, Inc. and/or its affiliates.
+ *
+ * Author: Vitor Soares <soares@synopsys.com>
+ */
+
+#include <linux/cdev.h>
+#include <linux/compat.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+
+#include <linux/i3c/i3cdev.h>
+
+#include "internals.h"
+
+#define I3C_MINORS	MINORMASK
+#define N_I3C_MINORS	16 /* For now */
+
+static DECLARE_BITMAP(minors, N_I3C_MINORS);
+
+struct i3cdev_data {
+	struct list_head list;
+	struct i3c_device *i3c;
+	struct cdev cdev;
+	struct device *dev;
+	dev_t devt;
+};
+
+static dev_t i3cdev_number; /* Alloted device number */
+
+static LIST_HEAD(i3cdev_list);
+static DEFINE_SPINLOCK(i3cdev_list_lock);
+
+static struct i3cdev_data *i3cdev_get_by_minor(unsigned int minor)
+{
+	struct i3cdev_data *i3cdev;
+
+	spin_lock(&i3cdev_list_lock);
+	list_for_each_entry(i3cdev, &i3cdev_list, list) {
+		if (MINOR(i3cdev->devt) == minor)
+			goto found;
+	}
+
+	i3cdev = NULL;
+
+found:
+	spin_unlock(&i3cdev_list_lock);
+	return i3cdev;
+}
+
+static struct i3cdev_data *i3cdev_get_by_i3c(struct i3c_device *i3c)
+{
+	struct i3cdev_data *i3cdev;
+
+	spin_lock(&i3cdev_list_lock);
+	list_for_each_entry(i3cdev, &i3cdev_list, list) {
+		if (i3cdev->i3c == i3c)
+			goto found;
+	}
+
+	i3cdev = NULL;
+
+found:
+	spin_unlock(&i3cdev_list_lock);
+	return i3cdev;
+}
+
+static struct i3cdev_data *get_free_i3cdev(struct i3c_device *i3c)
+{
+	struct i3cdev_data *i3cdev;
+	unsigned long minor;
+
+	minor = find_first_zero_bit(minors, N_I3C_MINORS);
+	if (minor >= N_I3C_MINORS) {
+		pr_err("i3cdev: no minor number available!\n");
+		return ERR_PTR(-ENODEV);
+	}
+
+	i3cdev = kzalloc(sizeof(*i3cdev), GFP_KERNEL);
+	if (!i3cdev)
+		return ERR_PTR(-ENOMEM);
+
+	i3cdev->i3c = i3c;
+	i3cdev->devt = MKDEV(MAJOR(i3cdev_number), minor);
+	set_bit(minor, minors);
+
+	spin_lock(&i3cdev_list_lock);
+	list_add_tail(&i3cdev->list, &i3cdev_list);
+	spin_unlock(&i3cdev_list_lock);
+
+	return i3cdev;
+}
+
+static void put_i3cdev(struct i3cdev_data *i3cdev)
+{
+	spin_lock(&i3cdev_list_lock);
+	list_del(&i3cdev->list);
+	spin_unlock(&i3cdev_list_lock);
+	kfree(i3cdev);
+}
+
+static ssize_t
+i3cdev_read(struct file *file, char __user *buf, size_t count, loff_t *f_pos)
+{
+	struct i3c_device *i3c = file->private_data;
+	struct i3c_priv_xfer xfers = {
+		.rnw = true,
+		.len = count,
+	};
+	char *tmp;
+	int ret;
+
+	tmp = kzalloc(count, GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	xfers.data.in = tmp;
+
+	dev_dbg(&i3c->dev, "Reading %zu bytes.\n", count);
+
+	ret = i3c_device_do_priv_xfers(i3c, &xfers, 1);
+	if (!ret)
+		ret = copy_to_user(buf, tmp, count) ? -EFAULT : ret;
+
+	kfree(tmp);
+	return ret;
+}
+
+static ssize_t
+i3cdev_write(struct file *file, const char __user *buf, size_t count,
+	     loff_t *f_pos)
+{
+	struct i3c_device *i3c = file->private_data;
+	struct i3c_priv_xfer xfers = {
+		.rnw = false,
+		.len = count,
+	};
+	char *tmp;
+	int ret;
+
+	tmp = memdup_user(buf, count);
+	if (IS_ERR(tmp))
+		return PTR_ERR(tmp);
+
+	xfers.data.out = tmp;
+
+	dev_dbg(&i3c->dev, "Writing %zu bytes.\n", count);
+
+	ret = i3c_device_do_priv_xfers(i3c, &xfers, 1);
+	kfree(tmp);
+	return (!ret) ? count : ret;
+}
+
+static int
+i3cdev_do_priv_xfer(struct i3c_device *dev, struct i3c_priv_xfer *xfers,
+		    unsigned int nxfers)
+{
+	void __user **data_ptrs;
+	unsigned int i;
+	int ret = 0;
+
+	data_ptrs = kmalloc_array(nxfers, sizeof(*data_ptrs), GFP_KERNEL);
+	if (!data_ptrs)
+		return -ENOMEM;
+
+	for (i = 0; i < nxfers; i++) {
+		if (xfers[i].rnw) {
+			data_ptrs[i] = (void __user *)xfers[i].data.in;
+			xfers[i].data.in = memdup_user(data_ptrs[i],
+						       xfers[i].len);
+			if (IS_ERR(xfers[i].data.in)) {
+				ret = PTR_ERR(xfers[i].data.in);
+				break;
+			}
+		} else {
+			data_ptrs[i] = (void __user *)xfers[i].data.out;
+			xfers[i].data.out = memdup_user(data_ptrs[i],
+							xfers[i].len);
+			if (IS_ERR(xfers[i].data.out)) {
+				ret = PTR_ERR(xfers[i].data.out);
+				break;
+			}
+		}
+	}
+
+	if (ret < 0) {
+		unsigned int j;
+
+		for (j = 0; j < i; ++j) {
+			if (xfers[i].rnw)
+				kfree(xfers[i].data.in);
+			else
+				kfree(xfers[i].data.out);
+		}
+
+		kfree(data_ptrs);
+		return ret;
+	}
+
+	ret = i3c_device_do_priv_xfers(dev, xfers, nxfers);
+	while (i-- > 0) {
+		if (ret >= 0 && xfers[i].rnw) {
+			if (copy_to_user(data_ptrs[i], xfers[i].data.in,
+					 xfers[i].len))
+				ret = -EFAULT;
+		}
+
+		if (xfers[i].rnw)
+			kfree(xfers[i].data.in);
+		else
+			kfree(xfers[i].data.out);
+	}
+
+	kfree(data_ptrs);
+	return ret;
+}
+
+static int
+i3cdev_ioc_priv_xfer(struct i3c_device *i3c,
+		     struct i3c_ioc_priv_xfer __user *u_ioc_xfers)
+{
+	struct i3c_ioc_priv_xfer k_ioc_xfer;
+	struct i3c_priv_xfer *xfers;
+	int ret;
+
+	if (copy_from_user(&k_ioc_xfer, u_ioc_xfers, sizeof(k_ioc_xfer)))
+		return -EFAULT;
+
+	xfers = memdup_user(k_ioc_xfer.xfers,
+			    k_ioc_xfer.nxfers * sizeof(struct i3c_priv_xfer));
+	if (IS_ERR(xfers))
+		return PTR_ERR(xfers);
+
+	ret = i3cdev_do_priv_xfer(i3c, xfers, k_ioc_xfer.nxfers);
+	kfree(xfers);
+
+	return ret;
+}
+
+static long
+i3cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	struct i3c_device *i3c = file->private_data;
+
+	dev_dbg(&i3c->dev, "ioctl, cmd=0x%02x, arg=0x%02lx\n", cmd, arg);
+
+	if (_IOC_TYPE(cmd) != I3C_DEV_IOC_MAGIC)
+		return -ENOTTY;
+
+	if (cmd == I3C_IOC_PRIV_XFER)
+		return i3cdev_ioc_priv_xfer(i3c,
+					(struct i3c_ioc_priv_xfer __user *)arg);
+
+	return 0;
+}
+
+static int i3cdev_open(struct inode *inode, struct file *file)
+{
+	unsigned int minor = iminor(inode);
+	struct i3cdev_data *i3cdev;
+
+	i3cdev = i3cdev_get_by_minor(minor);
+	if (!i3cdev)
+		return -ENODEV;
+
+	file->private_data = i3cdev->i3c;
+
+	return 0;
+}
+
+static int i3cdev_release(struct inode *inode, struct file *file)
+{
+	file->private_data = NULL;
+
+	return 0;
+}
+
+static const struct file_operations i3cdev_fops = {
+	.owner		= THIS_MODULE,
+	.read		= i3cdev_read,
+	.write		= i3cdev_write,
+	.unlocked_ioctl	= i3cdev_ioctl,
+	.open		= i3cdev_open,
+	.release	= i3cdev_release,
+};
+
+/* ------------------------------------------------------------------------- */
+
+static struct class *i3cdev_class;
+
+static int i3cdev_attach(struct device *dev, void *dummy)
+{
+	struct i3c_device *i3c;
+	struct i3cdev_data *i3cdev;
+	int res;
+
+	if (dev->type == &i3c_masterdev_type || dev->driver)
+		return 0;
+
+	i3c = dev_to_i3cdev(dev);
+
+	/* Get a device */
+	i3cdev = get_free_i3cdev(i3c);
+	if (IS_ERR(i3cdev))
+		return PTR_ERR(i3cdev);
+
+	cdev_init(&i3cdev->cdev, &i3cdev_fops);
+	i3cdev->cdev.owner = THIS_MODULE;
+	res = cdev_add(&i3cdev->cdev, i3cdev->devt, 1);
+	if (res)
+		goto error_cdev;
+
+	/* register this i3c device with the driver core */
+	i3cdev->dev = device_create(i3cdev_class, &i3c->dev,
+				    i3cdev->devt, NULL,
+				    "i3c-%s", dev_name(&i3c->dev));
+	if (IS_ERR(i3cdev->dev)) {
+		res = PTR_ERR(i3cdev->dev);
+		goto error;
+	}
+	pr_debug("i3c-cdev: I3C device [%s] registered as minor %d\n",
+		 dev_name(&i3c->dev), MINOR(i3cdev->devt));
+	return 0;
+
+error:
+	cdev_del(&i3cdev->cdev);
+error_cdev:
+	put_i3cdev(i3cdev);
+	return res;
+}
+
+static int i3cdev_detach(struct device *dev, void *dummy)
+{
+	struct i3c_device *i3c;
+	struct i3cdev_data *i3cdev;
+
+	if (dev->type == &i3c_masterdev_type)
+		return 0;
+
+	i3c = dev_to_i3cdev(dev);
+
+	i3cdev = i3cdev_get_by_i3c(i3c);
+	if (!i3cdev)
+		return 0;
+
+	clear_bit(MINOR(i3cdev->devt), minors);
+	cdev_del(&i3cdev->cdev);
+	device_destroy(i3cdev_class, i3cdev->devt);
+	put_i3cdev(i3cdev);
+
+	pr_debug("i3c-busdev: bus [%s] unregistered\n",
+		 dev_name(&i3c->dev));
+
+	return 0;
+}
+
+static int i3cdev_notifier_call(struct notifier_block *nb,
+				unsigned long action,
+				void *data)
+{
+	struct device *dev = data;
+
+	switch (action) {
+	case BUS_NOTIFY_ADD_DEVICE:
+	case BUS_NOTIFY_UNBOUND_DRIVER:
+		return i3cdev_attach(dev, NULL);
+	case BUS_NOTIFY_DEL_DEVICE:
+	case BUS_NOTIFY_BOUND_DRIVER:
+		return i3cdev_detach(dev, NULL);
+	}
+
+	return 0;
+}
+
+static struct notifier_block i3c_notifier = {
+	.notifier_call = i3cdev_notifier_call,
+};
+
+static int __init i3cdev_init(void)
+{
+	int res;
+
+	pr_info("i3c /dev entries driver\n");
+
+	/* Dynamically request unused major number */
+	res = alloc_chrdev_region(&i3cdev_number, 0, N_I3C_MINORS, "i3c");
+	if (res)
+		goto out;
+
+	/* Create a classe to populate sysfs entries*/
+	i3cdev_class = class_create(THIS_MODULE, "i3c-dev");
+	if (IS_ERR(i3cdev_class)) {
+		res = PTR_ERR(i3cdev_class);
+		goto out_unreg_chrdev;
+	}
+
+	/* Keep track of busses which have devices to add or remove later */
+	res = bus_register_notifier(&i3c_bus_type, &i3c_notifier);
+	if (res)
+		goto out_unreg_class;
+
+	/* Bind to already existing device without driver right away */
+	i3c_for_each_dev(NULL, i3cdev_attach);
+
+	return 0;
+
+out_unreg_class:
+	class_destroy(i3cdev_class);
+out_unreg_chrdev:
+	unregister_chrdev_region(i3cdev_number, I3C_MINORS);
+out:
+	pr_err("%s: Driver Initialisation failed\n", __FILE__);
+	return res;
+}
+
+static void __exit i3cdev_exit(void)
+{
+	bus_unregister_notifier(&i3c_bus_type, &i3c_notifier);
+	i3c_for_each_dev(NULL, i3cdev_detach);
+	class_destroy(i3cdev_class);
+	unregister_chrdev_region(i3cdev_number, I3C_MINORS);
+}
+
+MODULE_AUTHOR("Vitor Soares <soares@synopsys.com>");
+MODULE_DESCRIPTION("I3C /dev entries driver");
+MODULE_LICENSE("GPL");
+
+module_init(i3cdev_init);
+module_exit(i3cdev_exit);
diff --git a/include/uapi/linux/i3c/i3cdev.h b/include/uapi/linux/i3c/i3cdev.h
new file mode 100644
index 0000000..4030043
--- /dev/null
+++ b/include/uapi/linux/i3c/i3cdev.h
@@ -0,0 +1,27 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2019 Synopsys, Inc. and/or its affiliates.
+ *
+ * Author: Vitor Soares <vitor.soares@synopsys.com>
+ */
+
+#ifndef _UAPI_I3C_DEV_H_
+#define _UAPI_I3C_DEV_H_
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+/* IOCTL commands */
+#define I3C_DEV_IOC_MAGIC	0x07
+
+struct i3c_ioc_priv_xfer {
+	struct i3c_priv_xfer __user *xfers;	/* pointers to i3c_priv_xfer */
+	__u32 nxfers;				/* number of i3c_priv_xfer */
+};
+
+#define I3C_IOC_PRIV_XFER	\
+	_IOW(I3C_DEV_IOC_MAGIC, 30, struct i3c_ioc_priv_xfer)
+
+#define  I3C_IOC_PRIV_XFER_MAX_MSGS	42
+
+#endif