[v10,0/9] Add the I3C subsystem

Hi Greg,

I think we've reached a point where we can eventually consider the I3C
framework for inclusion in 4.20 (5.0?). A few more issues were reported
on v9 and fixed in v10. I can't guarantee that the implementation is
free of bugs but I still think it's worth merging it in v4.20: it's a
new subsystem, so we don't risk regressions, and the only way we can
detect other issues is by having other people experiment with this
implementation.

The only remaining concern raised by Arnd is the fact that both hosts
and slaves share the same bus type and are differentiated thanks to
their device_type, which IMHO is fine since this is what other
subsystems do (plus I don't see other solutions to have both I3C
devices and I3C buses represented under /sys/bus/i3c/).

I'd really like to get this series merged in 4.20 so that other
contributors can work on top of it to add

1/ new host drivers
2/ support for advanced features
3/ new device drivers

So, unless you have strong reasons to reject this request, and,
assuming I get Rob's ack soon enough, I plan to send a PR for this
framework next week.

Here is the usual description copy&pasted from the previous versions:

This patch series is adding a new subsystem to support I3C devices.

This is just adding support for basic features. Extra features will
be added afterwards.

There are a few design choices that are worth mentioning because they
impact the way I3C device drivers can interact with their devices:

- all functions used to send I3C/I2C frames must be called in
  non-atomic context. Mainly done this way to ease implementation, but
  this is still open to discussion. Please let me know if you think it's
  worth considering an asynchronous model here
- the I3C bus and I3C master controller are now tightly coupled even
  though they're still allocated separately. There's now a 1:1
  relationship between these objects, and the I3C master is no longer
  represented under the I3C bus object.
  Arnd, let me know if you had something different in mind, and I'll
  rework the implementation accordingly.

- I2C backward compatibility has been designed to be transparent to I2C
  drivers and the I2C subsystem. The I3C master just registers an I2C
  adapter which creates a new I2C bus. I'd say that, from a
  representation PoV it's not ideal because what should appear as a
  single I3C bus exposing I3C and I2C devices here appears as 2
  different busses connected to each other through the parenting (the
  I3C master is the parent of the I2C and I3C busses).
  On the other hand, I don't see a better solution if we want something
  that is not invasive.

Missing features (will be added in separate patch series after initial
support has been accepted/merged):
- support for HDR modes (has been removed because of lack of real users)
- no support for multi-master and the associated concepts (mastership
  handover, support for secondary masters, ...)
- I2C devices can only be described using DT because this is the only
  use case I have. However, the framework can easily be extended with
  ACPI and board info support
- I3C slave framework. This has been completely omitted, but shouldn't
  have a huge impact on the I3C framework because I3C slaves don't see
  the whole bus, it's only about handling master requests and generating
  IBIs. Some of the struct, constant and enum definitions could be
  shared, but most of the I3C slave framework logic will be different

Note that this patchset is available on the linux-i3c repo[1].

Main change between v8 and v9:
- The DT bindings have been simplified to get rid of the I3C/I3C_DEV()
  macros

Main change between v7 and v8:
- The bus object is now embedded in the master_controller object (as
  suggested by Arnd)

Main changes between v6 and v7:
- I3C bus/master representations have been reworked to match what other
  subsystems are doing (master implicitly represented by the bus
  object)
- I3C dev registration has been fixed
- I3C bus mode selection has been fixed
- Calls to readsl/writesl() in the Cadence I3C master driver have been
  fixed

Main changes between v5 and v6:
- Introduce {i3c,i2c}_dev_desc structures to better match how I3C
  master controllers (reservation of one HW slot for each device
  attached to the bus). With this solution, the resource migration
  that happens when a device lose its dynamic address and is
  re-assigned a different address is simplified on the driver side,
  because most of it is now handled in the core (reserve a new dev
  slot, reserve IBI resources and free all resources attached to the
  old slot)
- Add I3C error codes (M0 to M2) so that the core and device drivers
  can have fine grained information on what caused an EIO error.

Only minor things happened between v3 and v5 (you can go check the
changelog in each patch for more details).

Main changes between v2 and v3 are:
- Reworked the DT bindings as suggested by Rob
- Reworked the bus initialization step as suggested by Vitor
- Added a driver for an I3C GPIO expander

Main changes between the initial RFC and this v2 are:
- Add a generic infrastructure to support IBIs. It's worth mentioning
  that I tried exposing IBIs as a regular IRQs, but after several
  attempts and a discussion with Mark Zyngier, it appeared that it was
  not really fitting in the Linux IRQ model (the fact that you have
  payload attached to IBIs, the fact that most of the time an IBI will
  generate a transfer on the bus which has to be done in an atomic
  context, ...)
  The counterpart of this decision is the latency induced by the
  workqueue approach, but since I don't have real use cases, I don't
  know if this can be a problem or not. 
- Add helpers to support Hot Join
- Add support for IBIs and Hot Join in Cadence I3C master driver
- Address several issues in how I was using the device model

Thanks,

Boris

[1]https://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git/log/?h=i3c/next

*** BLURB HERE ***

Boris Brezillon (9):
  i3c: Add core I3C infrastructure
  docs: driver-api: Add I3C documentation
  i3c: Add sysfs ABI spec
  dt-bindings: i3c: Document core bindings
  MAINTAINERS: Add myself as the I3C subsystem maintainer
  i3c: master: Add driver for Cadence IP
  dt-bindings: i3c: Document Cadence I3C master bindings
  gpio: Add a driver for Cadence I3C GPIO expander
  dt-bindings: gpio: Add bindings for Cadence I3C gpio expander

 Documentation/ABI/testing/sysfs-bus-i3c       |  146 +
 .../bindings/gpio/gpio-cdns-i3c.txt           |   39 +
 .../bindings/i3c/cdns,i3c-master.txt          |   44 +
 Documentation/devicetree/bindings/i3c/i3c.txt |  139 +
 .../driver-api/i3c/device-driver-api.rst      |    9 +
 Documentation/driver-api/i3c/index.rst        |   11 +
 .../driver-api/i3c/master-driver-api.rst      |   10 +
 Documentation/driver-api/i3c/protocol.rst     |  203 ++
 Documentation/driver-api/index.rst            |    1 +
 MAINTAINERS                                   |   12 +
 drivers/Kconfig                               |    2 +
 drivers/Makefile                              |    2 +-
 drivers/gpio/Kconfig                          |   11 +
 drivers/gpio/Makefile                         |    1 +
 drivers/gpio/gpio-cdns-i3c.c                  |  411 +++
 drivers/i3c/Kconfig                           |   24 +
 drivers/i3c/Makefile                          |    4 +
 drivers/i3c/device.c                          |  233 ++
 drivers/i3c/internals.h                       |   26 +
 drivers/i3c/master.c                          | 2661 +++++++++++++++++
 drivers/i3c/master/Kconfig                    |    6 +
 drivers/i3c/master/Makefile                   |    1 +
 drivers/i3c/master/i3c-master-cdns.c          | 1671 +++++++++++
 include/linux/i3c/ccc.h                       |  385 +++
 include/linux/i3c/device.h                    |  331 ++
 include/linux/i3c/master.h                    |  648 ++++
 include/linux/mod_devicetable.h               |   17 +
 27 files changed, 7047 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i3c
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
 create mode 100644 Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
 create mode 100644 Documentation/devicetree/bindings/i3c/i3c.txt
 create mode 100644 Documentation/driver-api/i3c/device-driver-api.rst
 create mode 100644 Documentation/driver-api/i3c/index.rst
 create mode 100644 Documentation/driver-api/i3c/master-driver-api.rst
 create mode 100644 Documentation/driver-api/i3c/protocol.rst
 create mode 100644 drivers/gpio/gpio-cdns-i3c.c
 create mode 100644 drivers/i3c/Kconfig
 create mode 100644 drivers/i3c/Makefile
 create mode 100644 drivers/i3c/device.c
 create mode 100644 drivers/i3c/internals.h
 create mode 100644 drivers/i3c/master.c
 create mode 100644 drivers/i3c/master/Kconfig
 create mode 100644 drivers/i3c/master/Makefile
 create mode 100644 drivers/i3c/master/i3c-master-cdns.c
 create mode 100644 include/linux/i3c/ccc.h
 create mode 100644 include/linux/i3c/device.h
 create mode 100644 include/linux/i3c/master.h

Hi Boris,

On 26/10/18 15:43, Boris Brezillon wrote:
> Hi Greg,
>
> I think we've reached a point where we can eventually consider the I3C
> framework for inclusion in 4.20 (5.0?). A few more issues were reported
> on v9 and fixed in v10. I can't guarantee that the implementation is
> free of bugs but I still think it's worth merging it in v4.20: it's a
> new subsystem, so we don't risk regressions, and the only way we can
> detect other issues is by having other people experiment with this
> implementation.
>
> The only remaining concern raised by Arnd is the fact that both hosts
> and slaves share the same bus type and are differentiated thanks to
> their device_type, which IMHO is fine since this is what other
> subsystems do (plus I don't see other solutions to have both I3C
> devices and I3C buses represented under /sys/bus/i3c/).
>
> I'd really like to get this series merged in 4.20 so that other
> contributors can work on top of it to add
>
> 1/ new host drivers
> 2/ support for advanced features
> 3/ new device drivers
>
> So, unless you have strong reasons to reject this request, and,
> assuming I get Rob's ack soon enough, I plan to send a PR for this
> framework next week.
>
> Here is the usual description copy&pasted from the previous versions:
>
> This patch series is adding a new subsystem to support I3C devices.
>
> This is just adding support for basic features. Extra features will
> be added afterwards.
>
> There are a few design choices that are worth mentioning because they
> impact the way I3C device drivers can interact with their devices:
>
> - all functions used to send I3C/I2C frames must be called in
>    non-atomic context. Mainly done this way to ease implementation, but
>    this is still open to discussion. Please let me know if you think it's
>    worth considering an asynchronous model here
> - the I3C bus and I3C master controller are now tightly coupled even
>    though they're still allocated separately. There's now a 1:1
>    relationship between these objects, and the I3C master is no longer
>    represented under the I3C bus object.
>    Arnd, let me know if you had something different in mind, and I'll
>    rework the implementation accordingly.
>
> - I2C backward compatibility has been designed to be transparent to I2C
>    drivers and the I2C subsystem. The I3C master just registers an I2C
>    adapter which creates a new I2C bus. I'd say that, from a
>    representation PoV it's not ideal because what should appear as a
>    single I3C bus exposing I3C and I2C devices here appears as 2
>    different busses connected to each other through the parenting (the
>    I3C master is the parent of the I2C and I3C busses).
>    On the other hand, I don't see a better solution if we want something
>    that is not invasive.
>
> Missing features (will be added in separate patch series after initial
> support has been accepted/merged):
> - support for HDR modes (has been removed because of lack of real users)
> - no support for multi-master and the associated concepts (mastership
>    handover, support for secondary masters, ...)
> - I2C devices can only be described using DT because this is the only
>    use case I have. However, the framework can easily be extended with
>    ACPI and board info support
> - I3C slave framework. This has been completely omitted, but shouldn't
>    have a huge impact on the I3C framework because I3C slaves don't see
>    the whole bus, it's only about handling master requests and generating
>    IBIs. Some of the struct, constant and enum definitions could be
>    shared, but most of the I3C slave framework logic will be different
>
> Note that this patchset is available on the linux-i3c repo[1].
>
> Main change between v8 and v9:
> - The DT bindings have been simplified to get rid of the I3C/I3C_DEV()
>    macros
>
> Main change between v7 and v8:
> - The bus object is now embedded in the master_controller object (as
>    suggested by Arnd)
>
> Main changes between v6 and v7:
> - I3C bus/master representations have been reworked to match what other
>    subsystems are doing (master implicitly represented by the bus
>    object)
> - I3C dev registration has been fixed
> - I3C bus mode selection has been fixed
> - Calls to readsl/writesl() in the Cadence I3C master driver have been
>    fixed
>
> Main changes between v5 and v6:
> - Introduce {i3c,i2c}_dev_desc structures to better match how I3C
>    master controllers (reservation of one HW slot for each device
>    attached to the bus). With this solution, the resource migration
>    that happens when a device lose its dynamic address and is
>    re-assigned a different address is simplified on the driver side,
>    because most of it is now handled in the core (reserve a new dev
>    slot, reserve IBI resources and free all resources attached to the
>    old slot)
> - Add I3C error codes (M0 to M2) so that the core and device drivers
>    can have fine grained information on what caused an EIO error.
>
> Only minor things happened between v3 and v5 (you can go check the
> changelog in each patch for more details).
>
> Main changes between v2 and v3 are:
> - Reworked the DT bindings as suggested by Rob
> - Reworked the bus initialization step as suggested by Vitor
> - Added a driver for an I3C GPIO expander
>
> Main changes between the initial RFC and this v2 are:
> - Add a generic infrastructure to support IBIs. It's worth mentioning
>    that I tried exposing IBIs as a regular IRQs, but after several
>    attempts and a discussion with Mark Zyngier, it appeared that it was
>    not really fitting in the Linux IRQ model (the fact that you have
>    payload attached to IBIs, the fact that most of the time an IBI will
>    generate a transfer on the bus which has to be done in an atomic
>    context, ...)
>    The counterpart of this decision is the latency induced by the
>    workqueue approach, but since I don't have real use cases, I don't
>    know if this can be a problem or not.
> - Add helpers to support Hot Join
> - Add support for IBIs and Hot Join in Cadence I3C master driver
> - Address several issues in how I was using the device model
>
> Thanks,
>
> Boris
>
> [1]https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_i3c_linux.git_log_-3Fh-3Di3c_next&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=qVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=TkhdwjEerP4IBiC_WzI_vEUyZD2Dcxl03UeycbwNI2Q&s=KK31_5lEEgNN2iyURFULH0HmjkOPWoarNiT1hehvIsY&e=
>
> *** BLURB HERE ***
>
> Boris Brezillon (9):
>    i3c: Add core I3C infrastructure
>    docs: driver-api: Add I3C documentation
>    i3c: Add sysfs ABI spec
>    dt-bindings: i3c: Document core bindings
>    MAINTAINERS: Add myself as the I3C subsystem maintainer
>    i3c: master: Add driver for Cadence IP
>    dt-bindings: i3c: Document Cadence I3C master bindings
>    gpio: Add a driver for Cadence I3C GPIO expander
>    dt-bindings: gpio: Add bindings for Cadence I3C gpio expander
>
>   Documentation/ABI/testing/sysfs-bus-i3c       |  146 +
>   .../bindings/gpio/gpio-cdns-i3c.txt           |   39 +
>   .../bindings/i3c/cdns,i3c-master.txt          |   44 +
>   Documentation/devicetree/bindings/i3c/i3c.txt |  139 +
>   .../driver-api/i3c/device-driver-api.rst      |    9 +
>   Documentation/driver-api/i3c/index.rst        |   11 +
>   .../driver-api/i3c/master-driver-api.rst      |   10 +
>   Documentation/driver-api/i3c/protocol.rst     |  203 ++
>   Documentation/driver-api/index.rst            |    1 +
>   MAINTAINERS                                   |   12 +
>   drivers/Kconfig                               |    2 +
>   drivers/Makefile                              |    2 +-
>   drivers/gpio/Kconfig                          |   11 +
>   drivers/gpio/Makefile                         |    1 +
>   drivers/gpio/gpio-cdns-i3c.c                  |  411 +++
>   drivers/i3c/Kconfig                           |   24 +
>   drivers/i3c/Makefile                          |    4 +
>   drivers/i3c/device.c                          |  233 ++
>   drivers/i3c/internals.h                       |   26 +
>   drivers/i3c/master.c                          | 2661 +++++++++++++++++
>   drivers/i3c/master/Kconfig                    |    6 +
>   drivers/i3c/master/Makefile                   |    1 +
>   drivers/i3c/master/i3c-master-cdns.c          | 1671 +++++++++++
>   include/linux/i3c/ccc.h                       |  385 +++
>   include/linux/i3c/device.h                    |  331 ++
>   include/linux/i3c/master.h                    |  648 ++++
>   include/linux/mod_devicetable.h               |   17 +
>   27 files changed, 7047 insertions(+), 1 deletion(-)
>   create mode 100644 Documentation/ABI/testing/sysfs-bus-i3c
>   create mode 100644 Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
>   create mode 100644 Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
>   create mode 100644 Documentation/devicetree/bindings/i3c/i3c.txt
>   create mode 100644 Documentation/driver-api/i3c/device-driver-api.rst
>   create mode 100644 Documentation/driver-api/i3c/index.rst
>   create mode 100644 Documentation/driver-api/i3c/master-driver-api.rst
>   create mode 100644 Documentation/driver-api/i3c/protocol.rst
>   create mode 100644 drivers/gpio/gpio-cdns-i3c.c
>   create mode 100644 drivers/i3c/Kconfig
>   create mode 100644 drivers/i3c/Makefile
>   create mode 100644 drivers/i3c/device.c
>   create mode 100644 drivers/i3c/internals.h
>   create mode 100644 drivers/i3c/master.c
>   create mode 100644 drivers/i3c/master/Kconfig
>   create mode 100644 drivers/i3c/master/Makefile
>   create mode 100644 drivers/i3c/master/i3c-master-cdns.c
>   create mode 100644 include/linux/i3c/ccc.h
>   create mode 100644 include/linux/i3c/device.h
>   create mode 100644 include/linux/i3c/master.h
>
Can you update the i3c/next tree?


Best regards,

Vitor Soares

On Fri, 26 Oct 2018 16:22:06 +0100
vitor <vitor.soares@synopsys.com> wrote:

> Can you update the i3c/next tree?

Done.

On Fri, 26 Oct 2018 16:43:24 +0200
Boris Brezillon <boris.brezillon@bootlin.com> wrote:

> Hi Greg,
> 
> I think we've reached a point where we can eventually consider the I3C
> framework for inclusion in 4.20 (5.0?). A few more issues were reported
> on v9 and fixed in v10. I can't guarantee that the implementation is
> free of bugs but I still think it's worth merging it in v4.20: it's a
> new subsystem, so we don't risk regressions, and the only way we can
> detect other issues is by having other people experiment with this
> implementation.
> 
> The only remaining concern raised by Arnd is the fact that both hosts
> and slaves share the same bus type and are differentiated thanks to
> their device_type, which IMHO is fine since this is what other
> subsystems do (plus I don't see other solutions to have both I3C
> devices and I3C buses represented under /sys/bus/i3c/).
> 
> I'd really like to get this series merged in 4.20 so that other
> contributors can work on top of it to add
> 
> 1/ new host drivers
> 2/ support for advanced features
> 3/ new device drivers
> 
> So, unless you have strong reasons to reject this request, and,
> assuming I get Rob's ack soon enough, I plan to send a PR for this
> framework next week.
> 
> Here is the usual description copy&pasted from the previous versions:
> 
> This patch series is adding a new subsystem to support I3C devices.
> 
> This is just adding support for basic features. Extra features will
> be added afterwards.
> 
> There are a few design choices that are worth mentioning because they
> impact the way I3C device drivers can interact with their devices:
> 
> - all functions used to send I3C/I2C frames must be called in
>   non-atomic context. Mainly done this way to ease implementation, but
>   this is still open to discussion. Please let me know if you think it's
>   worth considering an asynchronous model here
> - the I3C bus and I3C master controller are now tightly coupled even
>   though they're still allocated separately. There's now a 1:1
>   relationship between these objects, and the I3C master is no longer
>   represented under the I3C bus object.
>   Arnd, let me know if you had something different in mind, and I'll
>   rework the implementation accordingly.
> 
> - I2C backward compatibility has been designed to be transparent to I2C
>   drivers and the I2C subsystem. The I3C master just registers an I2C
>   adapter which creates a new I2C bus. I'd say that, from a
>   representation PoV it's not ideal because what should appear as a
>   single I3C bus exposing I3C and I2C devices here appears as 2
>   different busses connected to each other through the parenting (the
>   I3C master is the parent of the I2C and I3C busses).
>   On the other hand, I don't see a better solution if we want something
>   that is not invasive.
> 
> Missing features (will be added in separate patch series after initial
> support has been accepted/merged):
> - support for HDR modes (has been removed because of lack of real users)
> - no support for multi-master and the associated concepts (mastership
>   handover, support for secondary masters, ...)
> - I2C devices can only be described using DT because this is the only
>   use case I have. However, the framework can easily be extended with
>   ACPI and board info support
> - I3C slave framework. This has been completely omitted, but shouldn't
>   have a huge impact on the I3C framework because I3C slaves don't see
>   the whole bus, it's only about handling master requests and generating
>   IBIs. Some of the struct, constant and enum definitions could be
>   shared, but most of the I3C slave framework logic will be different
> 
> Note that this patchset is available on the linux-i3c repo[1].
> 

Forgot to add:

Main changes between v9 and v10:
- Fix the example in the DT bindings file
- Dynamically allocate CCC payloads to make them DMA-safe
- Fix a deadlock
- Mention that i2c bufs are not guaranteed to be DMA-safe

> Main change between v8 and v9:
> - The DT bindings have been simplified to get rid of the I3C/I3C_DEV()
>   macros
> 
> Main change between v7 and v8:
> - The bus object is now embedded in the master_controller object (as
>   suggested by Arnd)
> 
> Main changes between v6 and v7:
> - I3C bus/master representations have been reworked to match what other
>   subsystems are doing (master implicitly represented by the bus
>   object)
> - I3C dev registration has been fixed
> - I3C bus mode selection has been fixed
> - Calls to readsl/writesl() in the Cadence I3C master driver have been
>   fixed
> 
> Main changes between v5 and v6:
> - Introduce {i3c,i2c}_dev_desc structures to better match how I3C
>   master controllers (reservation of one HW slot for each device
>   attached to the bus). With this solution, the resource migration
>   that happens when a device lose its dynamic address and is
>   re-assigned a different address is simplified on the driver side,
>   because most of it is now handled in the core (reserve a new dev
>   slot, reserve IBI resources and free all resources attached to the
>   old slot)
> - Add I3C error codes (M0 to M2) so that the core and device drivers
>   can have fine grained information on what caused an EIO error.
> 
> Only minor things happened between v3 and v5 (you can go check the
> changelog in each patch for more details).
> 
> Main changes between v2 and v3 are:
> - Reworked the DT bindings as suggested by Rob
> - Reworked the bus initialization step as suggested by Vitor
> - Added a driver for an I3C GPIO expander
> 
> Main changes between the initial RFC and this v2 are:
> - Add a generic infrastructure to support IBIs. It's worth mentioning
>   that I tried exposing IBIs as a regular IRQs, but after several
>   attempts and a discussion with Mark Zyngier, it appeared that it was
>   not really fitting in the Linux IRQ model (the fact that you have
>   payload attached to IBIs, the fact that most of the time an IBI will
>   generate a transfer on the bus which has to be done in an atomic
>   context, ...)
>   The counterpart of this decision is the latency induced by the
>   workqueue approach, but since I don't have real use cases, I don't
>   know if this can be a problem or not. 
> - Add helpers to support Hot Join
> - Add support for IBIs and Hot Join in Cadence I3C master driver
> - Address several issues in how I was using the device model
> 
> Thanks,
> 
> Boris
> 
> [1]https://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git/log/?h=i3c/next
> 
> *** BLURB HERE ***
> 
> Boris Brezillon (9):
>   i3c: Add core I3C infrastructure
>   docs: driver-api: Add I3C documentation
>   i3c: Add sysfs ABI spec
>   dt-bindings: i3c: Document core bindings
>   MAINTAINERS: Add myself as the I3C subsystem maintainer
>   i3c: master: Add driver for Cadence IP
>   dt-bindings: i3c: Document Cadence I3C master bindings
>   gpio: Add a driver for Cadence I3C GPIO expander
>   dt-bindings: gpio: Add bindings for Cadence I3C gpio expander
> 
>  Documentation/ABI/testing/sysfs-bus-i3c       |  146 +
>  .../bindings/gpio/gpio-cdns-i3c.txt           |   39 +
>  .../bindings/i3c/cdns,i3c-master.txt          |   44 +
>  Documentation/devicetree/bindings/i3c/i3c.txt |  139 +
>  .../driver-api/i3c/device-driver-api.rst      |    9 +
>  Documentation/driver-api/i3c/index.rst        |   11 +
>  .../driver-api/i3c/master-driver-api.rst      |   10 +
>  Documentation/driver-api/i3c/protocol.rst     |  203 ++
>  Documentation/driver-api/index.rst            |    1 +
>  MAINTAINERS                                   |   12 +
>  drivers/Kconfig                               |    2 +
>  drivers/Makefile                              |    2 +-
>  drivers/gpio/Kconfig                          |   11 +
>  drivers/gpio/Makefile                         |    1 +
>  drivers/gpio/gpio-cdns-i3c.c                  |  411 +++
>  drivers/i3c/Kconfig                           |   24 +
>  drivers/i3c/Makefile                          |    4 +
>  drivers/i3c/device.c                          |  233 ++
>  drivers/i3c/internals.h                       |   26 +
>  drivers/i3c/master.c                          | 2661 +++++++++++++++++
>  drivers/i3c/master/Kconfig                    |    6 +
>  drivers/i3c/master/Makefile                   |    1 +
>  drivers/i3c/master/i3c-master-cdns.c          | 1671 +++++++++++
>  include/linux/i3c/ccc.h                       |  385 +++
>  include/linux/i3c/device.h                    |  331 ++
>  include/linux/i3c/master.h                    |  648 ++++
>  include/linux/mod_devicetable.h               |   17 +
>  27 files changed, 7047 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-i3c
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
>  create mode 100644 Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
>  create mode 100644 Documentation/devicetree/bindings/i3c/i3c.txt
>  create mode 100644 Documentation/driver-api/i3c/device-driver-api.rst
>  create mode 100644 Documentation/driver-api/i3c/index.rst
>  create mode 100644 Documentation/driver-api/i3c/master-driver-api.rst
>  create mode 100644 Documentation/driver-api/i3c/protocol.rst
>  create mode 100644 drivers/gpio/gpio-cdns-i3c.c
>  create mode 100644 drivers/i3c/Kconfig
>  create mode 100644 drivers/i3c/Makefile
>  create mode 100644 drivers/i3c/device.c
>  create mode 100644 drivers/i3c/internals.h
>  create mode 100644 drivers/i3c/master.c
>  create mode 100644 drivers/i3c/master/Kconfig
>  create mode 100644 drivers/i3c/master/Makefile
>  create mode 100644 drivers/i3c/master/i3c-master-cdns.c
>  create mode 100644 include/linux/i3c/ccc.h
>  create mode 100644 include/linux/i3c/device.h
>  create mode 100644 include/linux/i3c/master.h
>

On 26/10/18 17:15, Boris Brezillon wrote:
> On Fri, 26 Oct 2018 16:22:06 +0100
> vitor <vitor.soares@synopsys.com> wrote:
>
>> Can you update the i3c/next tree?
> Done.

Thanks.

I will apply the driver and them I give you feedback.

On Fri, 26 Oct 2018 17:20:42 +0100
vitor <vitor.soares@synopsys.com> wrote:

> On 26/10/18 17:15, Boris Brezillon wrote:
> > On Fri, 26 Oct 2018 16:22:06 +0100
> > vitor <vitor.soares@synopsys.com> wrote:
> >  
> >> Can you update the i3c/next tree?  
> > Done.  
> 
> Thanks.
> 
> I will apply the driver and them I give you feedback.

Great! Note that the bug in the ->send_ccc_cmd() path should be fixed,
but you might have issues with i2c transfers as msg->buf is not
guaranteed to be aligned on 32-bit.

Don't know if you've followed the discussion with Arnd, but it seems
some (most?) archs are making sure writesl()/readsl() work for
unaligned bufs. Maybe we should fix that for ARC. The other solution is
to use i2c_get_dma_safe_msg_buf() to get something aligned on a
cache-line and by extension aligned on 32bits.

On Fri, Oct 26, 2018 at 04:43:24PM +0200, Boris Brezillon wrote:
> Hi Greg,
> 
> I think we've reached a point where we can eventually consider the I3C
> framework for inclusion in 4.20 (5.0?). A few more issues were reported
> on v9 and fixed in v10. I can't guarantee that the implementation is
> free of bugs but I still think it's worth merging it in v4.20: it's a
> new subsystem, so we don't risk regressions, and the only way we can
> detect other issues is by having other people experiment with this
> implementation.
> 
> The only remaining concern raised by Arnd is the fact that both hosts
> and slaves share the same bus type and are differentiated thanks to
> their device_type, which IMHO is fine since this is what other
> subsystems do (plus I don't see other solutions to have both I3C
> devices and I3C buses represented under /sys/bus/i3c/).

Yeah, it's not the nicest, but it will work, we did it also for USB and
greybus and it solves the issue.

This all looks good to me, so I've queued it up.  Let's see if
linux-next has any problems with it.  Thanks for sticking with it, nice
work!

greg k-h

Hi Greg,

On Sun, 11 Nov 2018 09:39:32 -0800
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Fri, Oct 26, 2018 at 04:43:24PM +0200, Boris Brezillon wrote:
> > Hi Greg,
> > 
> > I think we've reached a point where we can eventually consider the I3C
> > framework for inclusion in 4.20 (5.0?). A few more issues were reported
> > on v9 and fixed in v10. I can't guarantee that the implementation is
> > free of bugs but I still think it's worth merging it in v4.20: it's a
> > new subsystem, so we don't risk regressions, and the only way we can
> > detect other issues is by having other people experiment with this
> > implementation.
> > 
> > The only remaining concern raised by Arnd is the fact that both hosts
> > and slaves share the same bus type and are differentiated thanks to
> > their device_type, which IMHO is fine since this is what other
> > subsystems do (plus I don't see other solutions to have both I3C
> > devices and I3C buses represented under /sys/bus/i3c/).  
> 
> Yeah, it's not the nicest, but it will work, we did it also for USB and
> greybus and it solves the issue.
> 
> This all looks good to me, so I've queued it up.  Let's see if
> linux-next has any problems with it.

I recently asked Stephen to add the linux-i3c tree to linux-next, so
I'm expecting conflicts :-/. Sorry, I didn't know you were planning to
take these patches through your tree.

BTW, I also fixed a couple of things when rebasing on top of 4.20-rc1:

- KernelVersion in the sysfs ABI doc has been updated to 5.0
- Fixed i3c_master_getmxds_locked() (bug reported/fixed by Colin here
  https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1799850.html)
- Removed a blank line at the end of master-driver-api.rst

For the record, the i3c/next branch pulled by Stephen is available here
[1].

> Thanks for sticking with it, nice work!

Thanks for reviewing it! Greg, Stephen, let me know if you want me to
reset i3c/next to v4.20-rc1 to avoid conflicts in linux-next.

Regards,

Boris

[1]https://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git/log/?h=i3c/next

On Sun, Nov 11, 2018 at 07:10:17PM +0100, Boris Brezillon wrote:
> Hi Greg,
> 
> On Sun, 11 Nov 2018 09:39:32 -0800
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> > On Fri, Oct 26, 2018 at 04:43:24PM +0200, Boris Brezillon wrote:
> > > Hi Greg,
> > > 
> > > I think we've reached a point where we can eventually consider the I3C
> > > framework for inclusion in 4.20 (5.0?). A few more issues were reported
> > > on v9 and fixed in v10. I can't guarantee that the implementation is
> > > free of bugs but I still think it's worth merging it in v4.20: it's a
> > > new subsystem, so we don't risk regressions, and the only way we can
> > > detect other issues is by having other people experiment with this
> > > implementation.
> > > 
> > > The only remaining concern raised by Arnd is the fact that both hosts
> > > and slaves share the same bus type and are differentiated thanks to
> > > their device_type, which IMHO is fine since this is what other
> > > subsystems do (plus I don't see other solutions to have both I3C
> > > devices and I3C buses represented under /sys/bus/i3c/).  
> > 
> > Yeah, it's not the nicest, but it will work, we did it also for USB and
> > greybus and it solves the issue.
> > 
> > This all looks good to me, so I've queued it up.  Let's see if
> > linux-next has any problems with it.
> 
> I recently asked Stephen to add the linux-i3c tree to linux-next, so
> I'm expecting conflicts :-/. Sorry, I didn't know you were planning to
> take these patches through your tree.
> 
> BTW, I also fixed a couple of things when rebasing on top of 4.20-rc1:
> 
> - KernelVersion in the sysfs ABI doc has been updated to 5.0

There is no 5.0 yet :)

> - Fixed i3c_master_getmxds_locked() (bug reported/fixed by Colin here
>   https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1799850.html)
> - Removed a blank line at the end of master-driver-api.rst
> 
> For the record, the i3c/next branch pulled by Stephen is available here
> [1].
> 
> > Thanks for sticking with it, nice work!
> 
> Thanks for reviewing it! Greg, Stephen, let me know if you want me to
> reset i3c/next to v4.20-rc1 to avoid conflicts in linux-next.

So do you want me to just drop these patches from my tree?  If so, I
can, but i have no problem sending these to Linus for the next -rc1
merge window through my tree if that is easier.

It's up to you.

greg k-h

On Sun, 11 Nov 2018 11:10:20 -0800
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Sun, Nov 11, 2018 at 07:10:17PM +0100, Boris Brezillon wrote:
> > Hi Greg,
> > 
> > On Sun, 11 Nov 2018 09:39:32 -0800
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> >   
> > > On Fri, Oct 26, 2018 at 04:43:24PM +0200, Boris Brezillon wrote:  
> > > > Hi Greg,
> > > > 
> > > > I think we've reached a point where we can eventually consider the I3C
> > > > framework for inclusion in 4.20 (5.0?). A few more issues were reported
> > > > on v9 and fixed in v10. I can't guarantee that the implementation is
> > > > free of bugs but I still think it's worth merging it in v4.20: it's a
> > > > new subsystem, so we don't risk regressions, and the only way we can
> > > > detect other issues is by having other people experiment with this
> > > > implementation.
> > > > 
> > > > The only remaining concern raised by Arnd is the fact that both hosts
> > > > and slaves share the same bus type and are differentiated thanks to
> > > > their device_type, which IMHO is fine since this is what other
> > > > subsystems do (plus I don't see other solutions to have both I3C
> > > > devices and I3C buses represented under /sys/bus/i3c/).    
> > > 
> > > Yeah, it's not the nicest, but it will work, we did it also for USB and
> > > greybus and it solves the issue.
> > > 
> > > This all looks good to me, so I've queued it up.  Let's see if
> > > linux-next has any problems with it.  
> > 
> > I recently asked Stephen to add the linux-i3c tree to linux-next, so
> > I'm expecting conflicts :-/. Sorry, I didn't know you were planning to
> > take these patches through your tree.
> > 
> > BTW, I also fixed a couple of things when rebasing on top of 4.20-rc1:
> > 
> > - KernelVersion in the sysfs ABI doc has been updated to 5.0  
> 
> There is no 5.0 yet :)

Actually I had a hard time choosing between 4.21 and 5.0, and then I
saw Linus' email announcing 4.20-rc1 ;-). But I can change it back to
4.21 if you prefer.

> 
> > - Fixed i3c_master_getmxds_locked() (bug reported/fixed by Colin here
> >   https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1799850.html)
> > - Removed a blank line at the end of master-driver-api.rst

One extra thing I didn't mention (and didn't fix yet) is the I3C
mailing list. I asked Dave Miller if he'd be okay to create the
linux-i3c ML on vger.kernel.org, and he suggested that we use the
linux-i2c ML instead which I know Wolfram is not fond of. I might
decide to just put linux-kernel@vger.kernel.org as the ML to Cc for
I3C patches until we settle on something. 

> > 
> > For the record, the i3c/next branch pulled by Stephen is available here
> > [1].
> >   
> > > Thanks for sticking with it, nice work!  
> > 
> > Thanks for reviewing it! Greg, Stephen, let me know if you want me to
> > reset i3c/next to v4.20-rc1 to avoid conflicts in linux-next.  
> 
> So do you want me to just drop these patches from my tree?  If so, I
> can, but i have no problem sending these to Linus for the next -rc1
> merge window through my tree if that is easier.
> 
> It's up to you.

I think that's easier for me and for you if I take them in the i3c tree
and then send a PR to Linus myself. This way I won't bother you if
fixes are needed or if I decide to apply patches adding support for
other I3C controllers (I see you commented on the Synopsys driver
already, and I might indeed decide to queue this patchset for this
release).

Doing that also allows me to get everything in place since I'll anyway
have to send PRs to Linus at some point.

Regards,

Boris

PS: If you're fine with me taking the I3C patches, I'll need your
Acked-by.

On Sun, Nov 11, 2018 at 09:08:18PM +0100, Boris Brezillon wrote:
> On Sun, 11 Nov 2018 11:10:20 -0800
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> > On Sun, Nov 11, 2018 at 07:10:17PM +0100, Boris Brezillon wrote:
> > > Hi Greg,
> > > 
> > > On Sun, 11 Nov 2018 09:39:32 -0800
> > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > >   
> > > > On Fri, Oct 26, 2018 at 04:43:24PM +0200, Boris Brezillon wrote:  
> > > > > Hi Greg,
> > > > > 
> > > > > I think we've reached a point where we can eventually consider the I3C
> > > > > framework for inclusion in 4.20 (5.0?). A few more issues were reported
> > > > > on v9 and fixed in v10. I can't guarantee that the implementation is
> > > > > free of bugs but I still think it's worth merging it in v4.20: it's a
> > > > > new subsystem, so we don't risk regressions, and the only way we can
> > > > > detect other issues is by having other people experiment with this
> > > > > implementation.
> > > > > 
> > > > > The only remaining concern raised by Arnd is the fact that both hosts
> > > > > and slaves share the same bus type and are differentiated thanks to
> > > > > their device_type, which IMHO is fine since this is what other
> > > > > subsystems do (plus I don't see other solutions to have both I3C
> > > > > devices and I3C buses represented under /sys/bus/i3c/).    
> > > > 
> > > > Yeah, it's not the nicest, but it will work, we did it also for USB and
> > > > greybus and it solves the issue.
> > > > 
> > > > This all looks good to me, so I've queued it up.  Let's see if
> > > > linux-next has any problems with it.  
> > > 
> > > I recently asked Stephen to add the linux-i3c tree to linux-next, so
> > > I'm expecting conflicts :-/. Sorry, I didn't know you were planning to
> > > take these patches through your tree.
> > > 
> > > BTW, I also fixed a couple of things when rebasing on top of 4.20-rc1:
> > > 
> > > - KernelVersion in the sysfs ABI doc has been updated to 5.0  
> > 
> > There is no 5.0 yet :)
> 
> Actually I had a hard time choosing between 4.21 and 5.0, and then I
> saw Linus' email announcing 4.20-rc1 ;-). But I can change it back to
> 4.21 if you prefer.
> 
> > 
> > > - Fixed i3c_master_getmxds_locked() (bug reported/fixed by Colin here
> > >   https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1799850.html)
> > > - Removed a blank line at the end of master-driver-api.rst
> 
> One extra thing I didn't mention (and didn't fix yet) is the I3C
> mailing list. I asked Dave Miller if he'd be okay to create the
> linux-i3c ML on vger.kernel.org, and he suggested that we use the
> linux-i2c ML instead which I know Wolfram is not fond of. I might
> decide to just put linux-kernel@vger.kernel.org as the ML to Cc for
> I3C patches until we settle on something. 
> 
> > > 
> > > For the record, the i3c/next branch pulled by Stephen is available here
> > > [1].
> > >   
> > > > Thanks for sticking with it, nice work!  
> > > 
> > > Thanks for reviewing it! Greg, Stephen, let me know if you want me to
> > > reset i3c/next to v4.20-rc1 to avoid conflicts in linux-next.  
> > 
> > So do you want me to just drop these patches from my tree?  If so, I
> > can, but i have no problem sending these to Linus for the next -rc1
> > merge window through my tree if that is easier.
> > 
> > It's up to you.
> 
> I think that's easier for me and for you if I take them in the i3c tree
> and then send a PR to Linus myself. This way I won't bother you if
> fixes are needed or if I decide to apply patches adding support for
> other I3C controllers (I see you commented on the Synopsys driver
> already, and I might indeed decide to queue this patchset for this
> release).
> 
> Doing that also allows me to get everything in place since I'll anyway
> have to send PRs to Linus at some point.
> 
> Regards,
> 
> Boris
> 
> PS: If you're fine with me taking the I3C patches, I'll need your
> Acked-by.

Sure, feel free to do it all yourself, I do not object to that!  :)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

On Sun, Nov 11, 2018 at 12:57:18PM -0800, Greg Kroah-Hartman wrote:
> On Sun, Nov 11, 2018 at 09:08:18PM +0100, Boris Brezillon wrote:
> > On Sun, 11 Nov 2018 11:10:20 -0800
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > 
> > > On Sun, Nov 11, 2018 at 07:10:17PM +0100, Boris Brezillon wrote:
> > > > Hi Greg,
> > > > 
> > > > On Sun, 11 Nov 2018 09:39:32 -0800
> > > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > > >   
> > > > > On Fri, Oct 26, 2018 at 04:43:24PM +0200, Boris Brezillon wrote:  
> > > > > > Hi Greg,
> > > > > > 
> > > > > > I think we've reached a point where we can eventually consider the I3C
> > > > > > framework for inclusion in 4.20 (5.0?). A few more issues were reported
> > > > > > on v9 and fixed in v10. I can't guarantee that the implementation is
> > > > > > free of bugs but I still think it's worth merging it in v4.20: it's a
> > > > > > new subsystem, so we don't risk regressions, and the only way we can
> > > > > > detect other issues is by having other people experiment with this
> > > > > > implementation.
> > > > > > 
> > > > > > The only remaining concern raised by Arnd is the fact that both hosts
> > > > > > and slaves share the same bus type and are differentiated thanks to
> > > > > > their device_type, which IMHO is fine since this is what other
> > > > > > subsystems do (plus I don't see other solutions to have both I3C
> > > > > > devices and I3C buses represented under /sys/bus/i3c/).    
> > > > > 
> > > > > Yeah, it's not the nicest, but it will work, we did it also for USB and
> > > > > greybus and it solves the issue.
> > > > > 
> > > > > This all looks good to me, so I've queued it up.  Let's see if
> > > > > linux-next has any problems with it.  
> > > > 
> > > > I recently asked Stephen to add the linux-i3c tree to linux-next, so
> > > > I'm expecting conflicts :-/. Sorry, I didn't know you were planning to
> > > > take these patches through your tree.
> > > > 
> > > > BTW, I also fixed a couple of things when rebasing on top of 4.20-rc1:
> > > > 
> > > > - KernelVersion in the sysfs ABI doc has been updated to 5.0  
> > > 
> > > There is no 5.0 yet :)
> > 
> > Actually I had a hard time choosing between 4.21 and 5.0, and then I
> > saw Linus' email announcing 4.20-rc1 ;-). But I can change it back to
> > 4.21 if you prefer.
> > 
> > > 
> > > > - Fixed i3c_master_getmxds_locked() (bug reported/fixed by Colin here
> > > >   https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1799850.html)
> > > > - Removed a blank line at the end of master-driver-api.rst
> > 
> > One extra thing I didn't mention (and didn't fix yet) is the I3C
> > mailing list. I asked Dave Miller if he'd be okay to create the
> > linux-i3c ML on vger.kernel.org, and he suggested that we use the
> > linux-i2c ML instead which I know Wolfram is not fond of. I might
> > decide to just put linux-kernel@vger.kernel.org as the ML to Cc for
> > I3C patches until we settle on something. 
> > 
> > > > 
> > > > For the record, the i3c/next branch pulled by Stephen is available here
> > > > [1].
> > > >   
> > > > > Thanks for sticking with it, nice work!  
> > > > 
> > > > Thanks for reviewing it! Greg, Stephen, let me know if you want me to
> > > > reset i3c/next to v4.20-rc1 to avoid conflicts in linux-next.  
> > > 
> > > So do you want me to just drop these patches from my tree?  If so, I
> > > can, but i have no problem sending these to Linus for the next -rc1
> > > merge window through my tree if that is easier.
> > > 
> > > It's up to you.
> > 
> > I think that's easier for me and for you if I take them in the i3c tree
> > and then send a PR to Linus myself. This way I won't bother you if
> > fixes are needed or if I decide to apply patches adding support for
> > other I3C controllers (I see you commented on the Synopsys driver
> > already, and I might indeed decide to queue this patchset for this
> > release).
> > 
> > Doing that also allows me to get everything in place since I'll anyway
> > have to send PRs to Linus at some point.
> > 
> > Regards,
> > 
> > Boris
> > 
> > PS: If you're fine with me taking the I3C patches, I'll need your
> > Acked-by.
> 
> Sure, feel free to do it all yourself, I do not object to that!  :)
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

And I've now dropped all of these patches from my tree, so no need to
worry about any conflicts.

thanks,

greg k-h

Hi Boris,

Given the current state of the subsystem I think it might worth start to 
think how to expose the devices under /dev.
My initial thoughts are to do the same think as for i2c, expose the 
buses or the i3c_devices and use ioctl for private transfers. Some 
direct CCC commands can be sent through the /sys as you plan for SETNEWDA .

What do you think about this?

Best regards,
Vitor Soares

+Mark Brown for the question about /dev/spidev

Hi Vitor,

On Thu, 15 Nov 2018 12:14:37 +0000
vitor <vitor.soares@synopsys.com> wrote:

> Hi Boris,
> 
> Given the current state of the subsystem I think it might worth start to 
> think how to expose the devices under /dev.

Thanks for starting this discussion. I'm not against the idea in
general, we just need to be careful when doing that.

> My initial thoughts are to do the same think as for i2c, expose the 
> buses or the i3c_devices and use ioctl for private transfers.

Exposing the bus is dangerous IMO, because an I3C bus is not like an
I2C bus:

   * I3C device needs to be discovered through DAA
   * I2C devices need to be declared ahead of time, and LVR is used to
     determine the limitations on the bus at runtime

So you'd anyway be able to interact only with devices that have
previously been discovered.

Note that the virtual I2C bus is already exposed, but any command
targeting an address that is not attached to a registered I2C dev will
get a -ENOENT error.

What we could do though, is expose I3C devices that do not have a
driver in kernel space, like spidev does.

> Some 
> direct CCC commands can be sent through the /sys as you plan for SETNEWDA .

Yes, CCC commands that need to be exposed to userspace should be
exposed through sysfs, or, if we decide to create a /dev/i3cX device
per bus, through ioctls.

> 
> What do you think about this?

I think this request is perfectly valid, we just need to decide how it
should be done, and before we take this decision, I'd like to get
inputs from other maintainers.

Mark, Wolfram, Arnd, Greg, any opinion?

Regards,

Boris

On Thu, Nov 15, 2018 at 4:58 AM Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> +Mark Brown for the question about /dev/spidev
> On Thu, 15 Nov 2018 12:14:37 +0000
> vitor <vitor.soares@synopsys.com> wrote:
> > My initial thoughts are to do the same think as for i2c, expose the
> > buses or the i3c_devices and use ioctl for private transfers.
>
> Exposing the bus is dangerous IMO, because an I3C bus is not like an
> I2C bus:
>
>    * I3C device needs to be discovered through DAA
>    * I2C devices need to be declared ahead of time, and LVR is used to
>      determine the limitations on the bus at runtime
>
> So you'd anyway be able to interact only with devices that have
> previously been discovered.
>
> Note that the virtual I2C bus is already exposed, but any command
> targeting an address that is not attached to a registered I2C dev will
> get a -ENOENT error.
>
> What we could do though, is expose I3C devices that do not have a
> driver in kernel space, like spidev does.
>
> > Some
> > direct CCC commands can be sent through the /sys as you plan for SETNEWDA .
>
> Yes, CCC commands that need to be exposed to userspace should be
> exposed through sysfs, or, if we decide to create a /dev/i3cX device
> per bus, through ioctls.
>
> >
> > What do you think about this?
>
> I think this request is perfectly valid, we just need to decide how it
> should be done, and before we take this decision, I'd like to get
> inputs from other maintainers.
>
> Mark, Wolfram, Arnd, Greg, any opinion?

I think for a new user space interface, it makes sense to explore a number of
different options before making the final decision.

I agree about better not exposing the bus as a /dev/i3c* node, and that we
probably do need to expose individual devices in some form to allow
writing complete user space drivers that can do everything a kernel driver
can do.

Can you describe what a low-level interface to the device looks like
in the kernel? Can this be abstracted as simply pread()/pwrite() plus
an interrupt mechanism, or do we need a set of ioctl() operations as
well?

If it can be purely based on a regmap abstraction, a sysfs inteface
might be sufficient, though that has some downsides with permission
management compared to a /dev/* node.

Another option might be the use of a socket interface, which also
has some issues in terms of permission management, but might
be a good fit if we could abstract bus transactions as packets that
can be queued.

       Arnd

Hi Boris,

> What we could do though, is expose I3C devices that do not have a
> driver in kernel space, like spidev does.

...

> Mark, Wolfram, Arnd, Greg, any opinion?

Is there a benefit for having drivers in userspace? My gut feeling is to
encourage people to write kernel drivers. If this is, for some reason,
not possible for some driver, then we have a use case at hand to test
the then-to-be-developed userspace interface against. Until then, I
personally wouldn't waste effort on designing it without a user in
sight.

Dunno if you have that, but a debug interface (exchanging data with
clients) on the other hand would be super useful most probably. Maybe
you can start having that in debugfs and already learn from it if you
ever want to move some interface outside of debugfs?

Kind regards,

   Wolfram

On Thu, Nov 15, 2018 at 7:01 AM Wolfram Sang <wsa@the-dreams.de> wrote:
> > What we could do though, is expose I3C devices that do not have a
> > driver in kernel space, like spidev does.
>
> ...
>
> > Mark, Wolfram, Arnd, Greg, any opinion?
>
> Is there a benefit for having drivers in userspace? My gut feeling is to
> encourage people to write kernel drivers. If this is, for some reason,
> not possible for some driver, then we have a use case at hand to test
> the then-to-be-developed userspace interface against. Until then, I
> personally wouldn't waste effort on designing it without a user in
> sight.
>
> Dunno if you have that, but a debug interface (exchanging data with
> clients) on the other hand would be super useful most probably. Maybe
> you can start having that in debugfs and already learn from it if you
> ever want to move some interface outside of debugfs?

I think it may depend a little bit on the complexity we require
for a user interface. If it's basically just pread/pwrite, then the
debugfs would not look any different from a stable interface,
and there is little risk of getting it wrong. The more complex
the interface turns out to be, the more cautious we may want
to be about declaring it stable.

Other than that, I agree we should encourage users to write kernel
drivers, but given the precedent of uio, libusb, spidev, i2c-dev,
and vfio, it does seem extremely likely that users will have
requirements for it, and I think it's a good idea to start the design
discussion before users start building their own interfaces to do the
same thing badly.

        Arnd

Hi,

On 15/11/18 12:57, Boris Brezillon wrote:
> +Mark Brown for the question about /dev/spidev
>
> Hi Vitor,
>
> On Thu, 15 Nov 2018 12:14:37 +0000
> vitor <vitor.soares@synopsys.com> wrote:
>
>> Hi Boris,
>>
>> Given the current state of the subsystem I think it might worth start to
>> think how to expose the devices under /dev.
> Thanks for starting this discussion. I'm not against the idea in
> general, we just need to be careful when doing that.
>
>> My initial thoughts are to do the same think as for i2c, expose the
>> buses or the i3c_devices and use ioctl for private transfers.
> Exposing the bus is dangerous IMO, because an I3C bus is not like an
> I2C bus:
>
>     * I3C device needs to be discovered through DAA
>     * I2C devices need to be declared ahead of time, and LVR is used to
>       determine the limitations on the bus at runtime
>
> So you'd anyway be able to interact only with devices that have
> previously been discovered.
>
> Note that the virtual I2C bus is already exposed, but any command
> targeting an address that is not attached to a registered I2C dev will
> get a -ENOENT error.
I initially thought to do the same thing for the i3c devices adding a 
routine get_i3c_dev_by_addr()...
>
> What we could do though, is expose I3C devices that do not have a
> driver in kernel space, like spidev does.

...but I like more this approach.

>> Some
>> direct CCC commands can be sent through the /sys as you plan for SETNEWDA .
> Yes, CCC commands that need to be exposed to userspace should be
> exposed through sysfs, or, if we decide to create a /dev/i3cX device
> per bus, through ioctls.

There already some attributes exposed, just need to add the possibility 
to write to them and off course add some that are missing like GETSTATUS.

>
>> What do you think about this?
> I think this request is perfectly valid, we just need to decide how it
> should be done, and before we take this decision, I'd like to get
> inputs from other maintainers.
>
> Mark, Wolfram, Arnd, Greg, any opinion?
>
> Regards,
>
> Boris


Best regards,

Vitor Soares

Hi Arnd,

On 15/11/18 14:25, Arnd Bergmann wrote:
> I agree about better not exposing the bus as a /dev/i3c* node, and that we
> probably do need to expose individual devices in some form to allow
> writing complete user space drivers that can do everything a kernel driver
> can do.
>
> Can you describe what a low-level interface to the device looks like
> in the kernel? Can this be abstracted as simply pread()/pwrite() plus
> an interrupt mechanism, or do we need a set of ioctl() operations as
> well?

Like in i2c is likely to need the ioctl() too.


Best regards,

Vitor Soares

On Thu, 15 Nov 2018 16:01:37 +0100
Wolfram Sang <wsa@the-dreams.de> wrote:

> Hi Boris,
> 
> > What we could do though, is expose I3C devices that do not have a
> > driver in kernel space, like spidev does.  
> 
> ...
> 
> > Mark, Wolfram, Arnd, Greg, any opinion?  
> 
> Is there a benefit for having drivers in userspace? My gut feeling is to
> encourage people to write kernel drivers. If this is, for some reason,
> not possible for some driver, then we have a use case at hand to test
> the then-to-be-developed userspace interface against. Until then, I
> personally wouldn't waste effort on designing it without a user in
> sight.

I kind of agree with that. Vitor, do you have a use case in mind for
such userspace drivers? I don't think it's worth designing an API for
something we don't need (yet).

Hi Boris,


On 15/11/18 15:28, Boris Brezillon wrote:
> On Thu, 15 Nov 2018 16:01:37 +0100
> Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> Hi Boris,
>>
>>> What we could do though, is expose I3C devices that do not have a
>>> driver in kernel space, like spidev does.
>> ...
>>
>>> Mark, Wolfram, Arnd, Greg, any opinion?
>> Is there a benefit for having drivers in userspace? My gut feeling is to
>> encourage people to write kernel drivers. If this is, for some reason,
>> not possible for some driver, then we have a use case at hand to test
>> the then-to-be-developed userspace interface against. Until then, I
>> personally wouldn't waste effort on designing it without a user in
>> sight.
> I kind of agree with that. Vitor, do you have a use case in mind for
> such userspace drivers? I don't think it's worth designing an API for
> something we don't need (yet).

My use case is a tool for tests, lets say like the i2c tools. There is 
other subsystems, some of them mentioned on this thread, that have and 
ioctl system call or other method to change parameters or send data.


I rise this topic because I really think it worth to define now how this 
should be design (and for me how to do the things right) to avoid future 
issues.


Best regards,

Vitor Soares

On Thu, 15 Nov 2018 18:03:47 +0000
vitor <vitor.soares@synopsys.com> wrote:

> Hi Boris,
> 
> 
> On 15/11/18 15:28, Boris Brezillon wrote:
> > On Thu, 15 Nov 2018 16:01:37 +0100
> > Wolfram Sang <wsa@the-dreams.de> wrote:
> >  
> >> Hi Boris,
> >>  
> >>> What we could do though, is expose I3C devices that do not have a
> >>> driver in kernel space, like spidev does.  
> >> ...
> >>  
> >>> Mark, Wolfram, Arnd, Greg, any opinion?  
> >> Is there a benefit for having drivers in userspace? My gut feeling is to
> >> encourage people to write kernel drivers. If this is, for some reason,
> >> not possible for some driver, then we have a use case at hand to test
> >> the then-to-be-developed userspace interface against. Until then, I
> >> personally wouldn't waste effort on designing it without a user in
> >> sight.  
> > I kind of agree with that. Vitor, do you have a use case in mind for
> > such userspace drivers? I don't think it's worth designing an API for
> > something we don't need (yet).  
> 
> My use case is a tool for tests, lets say like the i2c tools.

What would you like to test exactly?

> There is 
> other subsystems, some of them mentioned on this thread, that have and 
> ioctl system call or other method to change parameters or send data.

I don't think they added the /dev interface before having a real use
case for it.

> 
> 
> I rise this topic because I really think it worth to define now how this 
> should be design (and for me how to do the things right) to avoid future 
> issues.

Actually it should be done the other way around: you should have a real
need and the /dev interface should be designed to fulfill this need.
Based on this real use case we can discuss other potential usage that
might appear in the future and try to design something more
future-proof, but clearly, this userspace interface should be driven by
a real/well-defined use case.

Also, exposing things to userspace is way more risky than adding a new
in-kernel subsystem/framework, because it then becomes part of the
stable ABI.

To make things clearer, I'm not against the idea of exposing I3C
devices (or I3C buses) to userspace, but I'd like to understand what you
plan to do with that. If this is about testing, what kind of tests
you'd like to run. If this is about developing drivers in userspace,
why can't these be done in kernel space (license issues?), and what
would those drivers be allowed to do?

On Thu, 15 Nov 2018 15:25:11 +0000
vitor <vitor.soares@synopsys.com> wrote:

> Hi Arnd,
> 
> On 15/11/18 14:25, Arnd Bergmann wrote:
> > I agree about better not exposing the bus as a /dev/i3c* node, and that we
> > probably do need to expose individual devices in some form to allow
> > writing complete user space drivers that can do everything a kernel driver
> > can do.
> >
> > Can you describe what a low-level interface to the device looks like
> > in the kernel? Can this be abstracted as simply pread()/pwrite() plus
> > an interrupt mechanism, or do we need a set of ioctl() operations as
> > well?  
> 
> Like in i2c is likely to need the ioctl() too.

Yep, I think we'll need an ioctl given the various type of transfers
one case use to interact with a device.

On Thu, Nov 15, 2018 at 01:57:31PM +0100, Boris Brezillon wrote:

> What we could do though, is expose I3C devices that do not have a
> driver in kernel space, like spidev does.

Yes, this is much safer and more robust especially if the bus has
enumeration requirements like you're saying it does.  It's much easier
and more sensible to do this if the bus can be enumerated than it is
with SPI, USB would be a good example here I guess (though I've never
looked at the details of how USB does this on the implementation side 
so this may be me speaking in blissful ignorance).

I do agree that there will be use cases that turn up for this.

Hi Boris,


On 15/11/18 19:00, Boris Brezillon wrote:
> On Thu, 15 Nov 2018 18:03:47 +0000
> vitor <vitor.soares@synopsys.com> wrote:
>
>> Hi Boris,
>>
>>
>> On 15/11/18 15:28, Boris Brezillon wrote:
>>> On Thu, 15 Nov 2018 16:01:37 +0100
>>> Wolfram Sang <wsa@the-dreams.de> wrote:
>>>   
>>>> Hi Boris,
>>>>   
>>>>> What we could do though, is expose I3C devices that do not have a
>>>>> driver in kernel space, like spidev does.
>>>> ...
>>>>   
>>>>> Mark, Wolfram, Arnd, Greg, any opinion?
>>>> Is there a benefit for having drivers in userspace? My gut feeling is to
>>>> encourage people to write kernel drivers. If this is, for some reason,
>>>> not possible for some driver, then we have a use case at hand to test
>>>> the then-to-be-developed userspace interface against. Until then, I
>>>> personally wouldn't waste effort on designing it without a user in
>>>> sight.
>>> I kind of agree with that. Vitor, do you have a use case in mind for
>>> such userspace drivers? I don't think it's worth designing an API for
>>> something we don't need (yet).
>> My use case is a tool for tests, lets say like the i2c tools.
> What would you like to test exactly?
>
>> There is
>> other subsystems, some of them mentioned on this thread, that have and
>> ioctl system call or other method to change parameters or send data.
> I don't think they added the /dev interface before having a real use
> case for it.
>
>>
>> I rise this topic because I really think it worth to define now how this
>> should be design (and for me how to do the things right) to avoid future
>> issues.
> Actually it should be done the other way around: you should have a real
> need and the /dev interface should be designed to fulfill this need.
> Based on this real use case we can discuss other potential usage that
> might appear in the future and try to design something more
> future-proof, but clearly, this userspace interface should be driven by
> a real/well-defined use case.
>
> Also, exposing things to userspace is way more risky than adding a new
> in-kernel subsystem/framework, because it then becomes part of the
> stable ABI.
>
> To make things clearer, I'm not against the idea of exposing I3C
> devices (or I3C buses) to userspace, but I'd like to understand what you
> plan to do with that. If this is about testing, what kind of tests
> you'd like to run. If this is about developing drivers in userspace,
> why can't these be done in kernel space (license issues?), and what
> would those drivers be allowed to do?


Basically I need a tool that help me during the development and to avoid 
me to write a dummy driver for each device that I test.

For instances do some read/write, get/set ccc commands, if something 
goes wrong during the bus initialization have a to debug etc...


For me this is a valid use case and I imagine when people start to 
develop in i3c this interface will help everyone.


Best regards,

Vitor Soares

Hi Vitor,

On 11/16/18, 1:32 PM, "vitor" <vitor.soares@synopsys.com> wrote:

    EXTERNAL MAIL
    
    
    Hi Boris,
    
    
    On 15/11/18 19:00, Boris Brezillon wrote:
    > On Thu, 15 Nov 2018 18:03:47 +0000
    > vitor <vitor.soares@synopsys.com> wrote:
    >
    >> Hi Boris,
    >>
    >>
    >> On 15/11/18 15:28, Boris Brezillon wrote:
    >>> On Thu, 15 Nov 2018 16:01:37 +0100
    >>> Wolfram Sang <wsa@the-dreams.de> wrote:
    >>>   
    >>>> Hi Boris,
    >>>>   
    >>>>> What we could do though, is expose I3C devices that do not have a
    >>>>> driver in kernel space, like spidev does.
    >>>> ...
    >>>>   
    >>>>> Mark, Wolfram, Arnd, Greg, any opinion?
    >>>> Is there a benefit for having drivers in userspace? My gut feeling is to
    >>>> encourage people to write kernel drivers. If this is, for some reason,
    >>>> not possible for some driver, then we have a use case at hand to test
    >>>> the then-to-be-developed userspace interface against. Until then, I
    >>>> personally wouldn't waste effort on designing it without a user in
    >>>> sight.
    >>> I kind of agree with that. Vitor, do you have a use case in mind for
    >>> such userspace drivers? I don't think it's worth designing an API for
    >>> something we don't need (yet).
    >> My use case is a tool for tests, lets say like the i2c tools.
    > What would you like to test exactly?
    >
    >> There is
    >> other subsystems, some of them mentioned on this thread, that have and
    >> ioctl system call or other method to change parameters or send data.
    > I don't think they added the /dev interface before having a real use
    > case for it.
    >
    >>
    >> I rise this topic because I really think it worth to define now how this
    >> should be design (and for me how to do the things right) to avoid future
    >> issues.
    > Actually it should be done the other way around: you should have a real
    > need and the /dev interface should be designed to fulfill this need.
    > Based on this real use case we can discuss other potential usage that
    > might appear in the future and try to design something more
    > future-proof, but clearly, this userspace interface should be driven by
    > a real/well-defined use case.
    >
    > Also, exposing things to userspace is way more risky than adding a new
    > in-kernel subsystem/framework, because it then becomes part of the
    > stable ABI.
    >
    > To make things clearer, I'm not against the idea of exposing I3C
    > devices (or I3C buses) to userspace, but I'd like to understand what you
    > plan to do with that. If this is about testing, what kind of tests
    > you'd like to run. If this is about developing drivers in userspace,
    > why can't these be done in kernel space (license issues?), and what
    > would those drivers be allowed to do?
    
    
    Basically I need a tool that help me during the development and to avoid 
    me to write a dummy driver for each device that I test.

For now we are doing it that way. Separate dummy driver for each device.
    
    For instances do some read/write, get/set ccc commands, if something 
    goes wrong during the bus initialization have a to debug etc...
    
That sounds good to have possibility to make simple reads/writes and 
send basic ccc commands. But of course keeping in mind that I3C bus is 
more complicated than I2C, as Boris mentioned before.
    
    For me this is a valid use case and I imagine when people start to 
    develop in i3c this interface will help everyone.
    
    
    Best regards,
    
    Vitor Soares
    
Regards,
Przemek

On Fri, 16 Nov 2018 12:31:42 +0000
vitor <vitor.soares@synopsys.com> wrote:

> Hi Boris,
> 
> 
> On 15/11/18 19:00, Boris Brezillon wrote:
> > On Thu, 15 Nov 2018 18:03:47 +0000
> > vitor <vitor.soares@synopsys.com> wrote:
> >  
> >> Hi Boris,
> >>
> >>
> >> On 15/11/18 15:28, Boris Brezillon wrote:  
> >>> On Thu, 15 Nov 2018 16:01:37 +0100
> >>> Wolfram Sang <wsa@the-dreams.de> wrote:
> >>>     
> >>>> Hi Boris,
> >>>>     
> >>>>> What we could do though, is expose I3C devices that do not have a
> >>>>> driver in kernel space, like spidev does.  
> >>>> ...
> >>>>     
> >>>>> Mark, Wolfram, Arnd, Greg, any opinion?  
> >>>> Is there a benefit for having drivers in userspace? My gut feeling is to
> >>>> encourage people to write kernel drivers. If this is, for some reason,
> >>>> not possible for some driver, then we have a use case at hand to test
> >>>> the then-to-be-developed userspace interface against. Until then, I
> >>>> personally wouldn't waste effort on designing it without a user in
> >>>> sight.  
> >>> I kind of agree with that. Vitor, do you have a use case in mind for
> >>> such userspace drivers? I don't think it's worth designing an API for
> >>> something we don't need (yet).  
> >> My use case is a tool for tests, lets say like the i2c tools.  
> > What would you like to test exactly?
> >  
> >> There is
> >> other subsystems, some of them mentioned on this thread, that have and
> >> ioctl system call or other method to change parameters or send data.  
> > I don't think they added the /dev interface before having a real use
> > case for it.
> >  
> >>
> >> I rise this topic because I really think it worth to define now how this
> >> should be design (and for me how to do the things right) to avoid future
> >> issues.  
> > Actually it should be done the other way around: you should have a real
> > need and the /dev interface should be designed to fulfill this need.
> > Based on this real use case we can discuss other potential usage that
> > might appear in the future and try to design something more
> > future-proof, but clearly, this userspace interface should be driven by
> > a real/well-defined use case.
> >
> > Also, exposing things to userspace is way more risky than adding a new
> > in-kernel subsystem/framework, because it then becomes part of the
> > stable ABI.
> >
> > To make things clearer, I'm not against the idea of exposing I3C
> > devices (or I3C buses) to userspace, but I'd like to understand what you
> > plan to do with that. If this is about testing, what kind of tests
> > you'd like to run. If this is about developing drivers in userspace,
> > why can't these be done in kernel space (license issues?), and what
> > would those drivers be allowed to do?  
> 
> 
> Basically I need a tool that help me during the development and to avoid 
> me to write a dummy driver for each device that I test.

But we want I3C device drivers to be upstreamed, so why not developing a
real driver everytime you test a new device and submitting it upstream?

> 
> For instances do some read/write,

Doing SDR/DDR transfers is probably acceptable, but I still think we
should push hard to have kernel drivers when that's possible.

> get/set ccc commands,

Exposing CCC commands is definitely not a good idea, since they're not
even exposed to kernel drivers.

> if something 
> goes wrong during the bus initialization have a to debug etc...

Can't we add such a debug infrastructure in the kernel. Maybe we can
expose debugfs files too if that helps, though if those debugfs files
are actually used by userspace libs/tools, it's not any better than
ioctls or sysfs files, since they will anyway become a stable ABI.

> 
> 
> For me this is a valid use case and I imagine when people start to 
> develop in i3c this interface will help everyone.

How about you propose an i3cdev driver that allow users to do SDR
transfers throuh an ioctl?

Hi Boris,

On 16/11/18 13:16, Boris Brezillon wrote:
> On Fri, 16 Nov 2018 12:31:42 +0000
> vitor <vitor.soares@synopsys.com> wrote:
>
>> Hi Boris,
>>
>>
>> On 15/11/18 19:00, Boris Brezillon wrote:
>>> On Thu, 15 Nov 2018 18:03:47 +0000
>>> vitor <vitor.soares@synopsys.com> wrote:
>>>   
>>>> Hi Boris,
>>>>
>>>>
>>>> On 15/11/18 15:28, Boris Brezillon wrote:
>>>>> On Thu, 15 Nov 2018 16:01:37 +0100
>>>>> Wolfram Sang <wsa@the-dreams.de> wrote:
>>>>>      
>>>>>> Hi Boris,
>>>>>>      
>>>>>>> What we could do though, is expose I3C devices that do not have a
>>>>>>> driver in kernel space, like spidev does.
>>>>>> ...
>>>>>>      
>>>>>>> Mark, Wolfram, Arnd, Greg, any opinion?
>>>>>> Is there a benefit for having drivers in userspace? My gut feeling is to
>>>>>> encourage people to write kernel drivers. If this is, for some reason,
>>>>>> not possible for some driver, then we have a use case at hand to test
>>>>>> the then-to-be-developed userspace interface against. Until then, I
>>>>>> personally wouldn't waste effort on designing it without a user in
>>>>>> sight.
>>>>> I kind of agree with that. Vitor, do you have a use case in mind for
>>>>> such userspace drivers? I don't think it's worth designing an API for
>>>>> something we don't need (yet).
>>>> My use case is a tool for tests, lets say like the i2c tools.
>>> What would you like to test exactly?
>>>   
>>>> There is
>>>> other subsystems, some of them mentioned on this thread, that have and
>>>> ioctl system call or other method to change parameters or send data.
>>> I don't think they added the /dev interface before having a real use
>>> case for it.
>>>   
>>>> I rise this topic because I really think it worth to define now how this
>>>> should be design (and for me how to do the things right) to avoid future
>>>> issues.
>>> Actually it should be done the other way around: you should have a real
>>> need and the /dev interface should be designed to fulfill this need.
>>> Based on this real use case we can discuss other potential usage that
>>> might appear in the future and try to design something more
>>> future-proof, but clearly, this userspace interface should be driven by
>>> a real/well-defined use case.
>>>
>>> Also, exposing things to userspace is way more risky than adding a new
>>> in-kernel subsystem/framework, because it then becomes part of the
>>> stable ABI.
>>>
>>> To make things clearer, I'm not against the idea of exposing I3C
>>> devices (or I3C buses) to userspace, but I'd like to understand what you
>>> plan to do with that. If this is about testing, what kind of tests
>>> you'd like to run. If this is about developing drivers in userspace,
>>> why can't these be done in kernel space (license issues?), and what
>>> would those drivers be allowed to do?
>>
>> Basically I need a tool that help me during the development and to avoid
>> me to write a dummy driver for each device that I test.
> But we want I3C device drivers to be upstreamed, so why not developing a
> real driver everytime you test a new device and submitting it upstream?


Usually the devices that I test aren't the final product so it isn't 
easy to do the upstream.

But when possible I plan to do that.


>
>> For instances do some read/write,
> Doing SDR/DDR transfers is probably acceptable, but I still think we
> should push hard to have kernel drivers when that's possible.
>
>> get/set ccc commands,
> Exposing CCC commands is definitely not a good idea, since they're not
> even exposed to kernel drivers.
>
>> if something
>> goes wrong during the bus initialization have a to debug etc...
> Can't we add such a debug infrastructure in the kernel. Maybe we can
> expose debugfs files too if that helps, though if those debugfs files
> are actually used by userspace libs/tools, it's not any better than
> ioctls or sysfs files, since they will anyway become a stable ABI.
>
>>
>> For me this is a valid use case and I imagine when people start to
>> develop in i3c this interface will help everyone.
> How about you propose an i3cdev driver that allow users to do SDR
> transfers throuh an ioctl?

I think that was for v6 I started to something to expose the bus like in 
i2c-dev, but I liked the idea of expose only the device doesn't have a 
driver. Do you know if  there is already something in the kernel doing 
the same?


Best regards,

Vitor Soares

On Mon, 19 Nov 2018 12:35:42 +0000
vitor <vitor.soares@synopsys.com> wrote:

> Hi Boris,
> 
> On 16/11/18 13:16, Boris Brezillon wrote:
> > On Fri, 16 Nov 2018 12:31:42 +0000
> > vitor <vitor.soares@synopsys.com> wrote:
> >  
> >> Hi Boris,
> >>
> >>
> >> On 15/11/18 19:00, Boris Brezillon wrote:  
> >>> On Thu, 15 Nov 2018 18:03:47 +0000
> >>> vitor <vitor.soares@synopsys.com> wrote:
> >>>     
> >>>> Hi Boris,
> >>>>
> >>>>
> >>>> On 15/11/18 15:28, Boris Brezillon wrote:  
> >>>>> On Thu, 15 Nov 2018 16:01:37 +0100
> >>>>> Wolfram Sang <wsa@the-dreams.de> wrote:
> >>>>>        
> >>>>>> Hi Boris,
> >>>>>>        
> >>>>>>> What we could do though, is expose I3C devices that do not have a
> >>>>>>> driver in kernel space, like spidev does.  
> >>>>>> ...
> >>>>>>        
> >>>>>>> Mark, Wolfram, Arnd, Greg, any opinion?  
> >>>>>> Is there a benefit for having drivers in userspace? My gut feeling is to
> >>>>>> encourage people to write kernel drivers. If this is, for some reason,
> >>>>>> not possible for some driver, then we have a use case at hand to test
> >>>>>> the then-to-be-developed userspace interface against. Until then, I
> >>>>>> personally wouldn't waste effort on designing it without a user in
> >>>>>> sight.  
> >>>>> I kind of agree with that. Vitor, do you have a use case in mind for
> >>>>> such userspace drivers? I don't think it's worth designing an API for
> >>>>> something we don't need (yet).  
> >>>> My use case is a tool for tests, lets say like the i2c tools.  
> >>> What would you like to test exactly?
> >>>     
> >>>> There is
> >>>> other subsystems, some of them mentioned on this thread, that have and
> >>>> ioctl system call or other method to change parameters or send data.  
> >>> I don't think they added the /dev interface before having a real use
> >>> case for it.
> >>>     
> >>>> I rise this topic because I really think it worth to define now how this
> >>>> should be design (and for me how to do the things right) to avoid future
> >>>> issues.  
> >>> Actually it should be done the other way around: you should have a real
> >>> need and the /dev interface should be designed to fulfill this need.
> >>> Based on this real use case we can discuss other potential usage that
> >>> might appear in the future and try to design something more
> >>> future-proof, but clearly, this userspace interface should be driven by
> >>> a real/well-defined use case.
> >>>
> >>> Also, exposing things to userspace is way more risky than adding a new
> >>> in-kernel subsystem/framework, because it then becomes part of the
> >>> stable ABI.
> >>>
> >>> To make things clearer, I'm not against the idea of exposing I3C
> >>> devices (or I3C buses) to userspace, but I'd like to understand what you
> >>> plan to do with that. If this is about testing, what kind of tests
> >>> you'd like to run. If this is about developing drivers in userspace,
> >>> why can't these be done in kernel space (license issues?), and what
> >>> would those drivers be allowed to do?  
> >>
> >> Basically I need a tool that help me during the development and to avoid
> >> me to write a dummy driver for each device that I test.  
> > But we want I3C device drivers to be upstreamed, so why not developing a
> > real driver everytime you test a new device and submitting it upstream?  
> 
> 
> Usually the devices that I test aren't the final product so it isn't 
> easy to do the upstream.
> 
> But when possible I plan to do that.
> 
> 
> >  
> >> For instances do some read/write,  
> > Doing SDR/DDR transfers is probably acceptable, but I still think we
> > should push hard to have kernel drivers when that's possible.
> >  
> >> get/set ccc commands,  
> > Exposing CCC commands is definitely not a good idea, since they're not
> > even exposed to kernel drivers.
> >  
> >> if something
> >> goes wrong during the bus initialization have a to debug etc...  
> > Can't we add such a debug infrastructure in the kernel. Maybe we can
> > expose debugfs files too if that helps, though if those debugfs files
> > are actually used by userspace libs/tools, it's not any better than
> > ioctls or sysfs files, since they will anyway become a stable ABI.
> >  
> >>
> >> For me this is a valid use case and I imagine when people start to
> >> develop in i3c this interface will help everyone.  
> > How about you propose an i3cdev driver that allow users to do SDR
> > transfers throuh an ioctl?  
> 
> I think that was for v6 I started to something to expose the bus like in 
> i2c-dev, but I liked the idea of expose only the device doesn't have a 
> driver. Do you know if  there is already something in the kernel doing 
> the same?

I know [1], but there might be other subsystems doing the same thing.

[1]https://elixir.bootlin.com/linux/v4.20-rc3/source/drivers/spi/spidev.c

On 19/11/18 12:43, Boris Brezillon wrote:
> I know [1], but there might be other subsystems doing the same thing.
>
> [1]https://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.com_linux_v4.20-2Drc3_source_drivers_spi_spidev.c&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=qVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=S_YF5FjfGFFGFa7x6nHrT6npsIFjUbYRorSANaPpaiI&s=xXnwGBSGrVxandwp6biURmXq-85iMMLd8-n2McDsIWY&e=


Thanks.