Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Vitor Soares <Vitor.Soares@synopsys.com>
To:     Boris Brezillon <boris.brezillon@collabora.com>
CC:     "bbrezillon@kernel.org" <bbrezillon@kernel.org>,
        Joao Pinto <Joao.Pinto@synopsys.com>,
        Jose Abreu <Jose.Abreu@synopsys.com>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "wsa@the-dreams.de" <wsa@the-dreams.de>,
        "arnd@arndb.de" <arnd@arndb.de>,
        "broonie@kernel.org" <broonie@kernel.org>,
        "corbet@lwn.net" <corbet@lwn.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-i3c@lists.infradead.org" <linux-i3c@lists.infradead.org>
Subject: RE: [PATCH v3 0/5] Introduce i3c device userspace interface
Thread-Topic: [PATCH v3 0/5] Introduce i3c device userspace interface
Thread-Index: AQHV5rqI90LrTPLnik2TPIdRwWN8OaghqdAQgACCowCAA55iQA==
Date:   Fri, 21 Feb 2020 17:08:17 +0000
Message-ID: <CH2PR12MB4216ECDC745C8255DF8106A3AE120@CH2PR12MB4216.namprd12.prod.outlook.com>
References: <cover.1582069402.git.vitor.soares@synopsys.com>
        <CH2PR12MB421604E9272413A6C456AB16AE100@CH2PR12MB4216.namprd12.prod.outlook.com>
 <20200219091658.7506e7bd@collabora.com>
In-Reply-To: <20200219091658.7506e7bd@collabora.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: synopsys.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 1af7a4d0-dd44-4acc-0876-08d7b6f0a4ee
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Feb 2020 17:08:17.0200
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: c33c9f88-1eb7-4099-9700-16013fd9e8aa
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: GjpJZTIQ341jrPcIIsfBq8NA3/yPN+h4t6+JbiYl0Q/TQXpB9qMcAdO0V75dTNNVdlyw5jqfpsj5G/GYDQXW1g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR12MB4296
X-OriginatorOrg: synopsys.com
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Boris,

From: Boris Brezillon <boris.brezillon@collabora.com>
Date: Wed, Feb 19, 2020 at 08:16:58

> On Wed, 19 Feb 2020 00:39:31 +0000
> Vitor Soares <Vitor.Soares@synopsys.com> wrote:
>=20
> > Hi Boris,
> >=20
> > From: Vitor Soares <vitor.soares@synopsys.com>
> > Date: Wed, Feb 19, 2020 at 00:20:38
> >=20
> > > 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 devi=
ce
> > > or i3c host controllers development.
> > >=20
> > > The i3cdev module is highly based on i2c-dev and yet I tried to addre=
ss
> > > the concerns raised in [1].
> > >=20
> > > NOTES:
> > > - The i3cdev dynamically request an unused major number.
> > >=20
> > > - The i3c devices are dynamically exposed/removed from dev/ folder ba=
sed
> > >   on if they have a device driver bound to it.
> > >=20
> > > - For now, the module exposes i3c devices without device driver on
> > >   dev/bus/i3c/<bus>-<pid>
> > >=20
> > > - 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.
> > >=20
> > > - Since the i3c API only exposes i3c_priv_xfer to devices, for now, t=
he
> > >   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 availabl=
e
> > >   through sysfs. We can add more device attributes to expose more
> > >   information or add a dedicated ioctl() request for that purpose or =
both.
> > >=20
> > > - 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 appre=
ciate
> > >   your feedback about it as well.
> > >=20
> > > [1] https://urldefense.proofpoint.com/v2/url?u=3Dhttps-3A__lkml.org_l=
kml_2018_11_15_853&d=3DDwICAg&c=3DDPL6_X_6JkXFx7AXWqB0tg&r=3DqVuU64u9x77Y0K=
d0PhDK_lpxFgg6PK9PateHwjb_DY0&m=3D1N2OHEqhXNvZn7HIjf_EmPX5N9wwBWLsUakJ9a1zn=
o0&s=3DRJ3Lr0SIvJYiKxLCFaV0_pNArfUJj7TxsJecNgYbTQA&e=3D=20
> > > [2] https://urldefense.proofpoint.com/v2/url?u=3Dhttps-3A__github.com=
_vitor-2Dsoares-2Dsnps_i3c-2Dtools.git&d=3DDwICAg&c=3DDPL6_X_6JkXFx7AXWqB0t=
g&r=3DqVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=3D1N2OHEqhXNvZn7HIjf_Em=
PX5N9wwBWLsUakJ9a1zno0&s=3Dq4Oi7UD-dZ1YMlTN9prqycS1G1QvEUuBFnsABMGRuBk&e=3D=
=20
> > >=20
> > > 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 o=
ccur
> > >   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 allocati=
on
> > >   check
> > >   Pass i3cdev to dev->driver_data
> > >   Make all minors available
> > >   Add API documentation
> > >=20
> > > Changes in v2:
> > >   Use IDR api for minor numbering
> > >   Modify ioctl struct
> > >   Fix SPDX license
> > >=20
> > > 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
> > >=20
> > >  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
> > >=20
> > > --=20
> > > 2.7.4 =20
> >=20
> > I want to make you know that none of your previous comments was ignored=
=20
> > and  I would like to start the discussion from this point.
>=20
> 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=20
driver is loaded after i3cdev? Currently, this is done automatically=20
without any command, and for me, this is a requirement.
  - For the ioctl command structure, there is no rule about the way I did=20
or what you proposed, both are currently used in the kernel. For me it is=20
one more structure to deal with, can you point the advantages of your=20
purpose?
  - Regarding the ioctl codes, I tried to use those after I2C.


Best regards,
Vitor Soares