Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1700175 > unrolled thread

[RFC 0/5] Add I3C subsystem

Started byBoris Brezillon <boris.brezillon@free-electrons.com>
First post2017-07-31 18:30 +0200
Last post2017-08-02 04:20 +0200
Articles 20 on this page of 33 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [RFC 0/5] Add I3C subsystem Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-07-31 18:30 +0200
    [RFC 5/5] dt-bindings: i3c: Document Cadence I3C master bindings Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-07-31 18:30 +0200
    Re: [RFC 2/5] i3c: Add core I3C infrastructure Wolfram Sang <wsa@the-dreams.de> - 2017-07-31 21:20 +0200
      Re: [RFC 2/5] i3c: Add core I3C infrastructure Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-07-31 22:50 +0200
    Re: [RFC 0/5] Add I3C subsystem Wolfram Sang <wsa@the-dreams.de> - 2017-07-31 21:20 +0200
      Re: [RFC 0/5] Add I3C subsystem Wolfram Sang <wsa@the-dreams.de> - 2017-07-31 22:50 +0200
      Re: [RFC 0/5] Add I3C subsystem Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-07-31 22:50 +0200
    Re: [RFC 2/5] i3c: Add core I3C infrastructure Arnd Bergmann <arnd@arndb.de> - 2017-07-31 22:20 +0200
      Re: [RFC 2/5] i3c: Add core I3C infrastructure Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-07-31 23:20 +0200
        Re: [RFC 2/5] i3c: Add core I3C infrastructure Wolfram Sang <wsa@the-dreams.de> - 2017-07-31 23:50 +0200
          Re: [RFC 2/5] i3c: Add core I3C infrastructure "Andrew F. Davis" <afd@ti.com> - 2017-08-01 18:50 +0200
            Re: [RFC 2/5] i3c: Add core I3C infrastructure Wolfram Sang <wsa@the-dreams.de> - 2017-08-01 19:30 +0200
              Re: [RFC 2/5] i3c: Add core I3C infrastructure Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-08-01 23:50 +0200
                Re: [RFC 2/5] i3c: Add core I3C infrastructure Wolfram Sang <wsa@the-dreams.de> - 2017-08-02 12:30 +0200
        Re: [RFC 2/5] i3c: Add core I3C infrastructure Arnd Bergmann <arnd@arndb.de> - 2017-08-01 14:10 +0200
          Re: [RFC 2/5] i3c: Add core I3C infrastructure Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-08-01 14:30 +0200
            Re: [RFC 2/5] i3c: Add core I3C infrastructure Arnd Bergmann <arnd@arndb.de> - 2017-08-01 15:20 +0200
              Re: [RFC 2/5] i3c: Add core I3C infrastructure Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-08-01 15:40 +0200
                Re: [RFC 2/5] i3c: Add core I3C infrastructure Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-08-01 16:00 +0200
                  Re: [RFC 2/5] i3c: Add core I3C infrastructure Arnd Bergmann <arnd@arndb.de> - 2017-08-01 16:30 +0200
                    Re: [RFC 2/5] i3c: Add core I3C infrastructure Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-08-01 17:20 +0200
                      Re: [RFC 2/5] i3c: Add core I3C infrastructure Arnd Bergmann <arnd@arndb.de> - 2017-08-01 22:20 +0200
                Re: [RFC 2/5] i3c: Add core I3C infrastructure Wolfram Sang <wsa@the-dreams.de> - 2017-08-01 16:20 +0200
                  Re: [RFC 2/5] i3c: Add core I3C infrastructure Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-08-01 16:50 +0200
                    Re: [RFC 2/5] i3c: Add core I3C infrastructure Wolfram Sang <wsa@the-dreams.de> - 2017-08-01 17:10 +0200
                      Re: [RFC 2/5] i3c: Add core I3C infrastructure Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-08-01 17:30 +0200
                        Re: [RFC 2/5] i3c: Add core I3C infrastructure Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-08-03 10:10 +0200
    Re: [RFC 2/5] i3c: Add core I3C infrastructure Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-08-01 03:50 +0200
      Re: [RFC 2/5] i3c: Add core I3C infrastructure Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-08-01 12:50 +0200
        Re: [RFC 2/5] i3c: Add core I3C infrastructure Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-08-01 20:00 +0200
          Re: [RFC 2/5] i3c: Add core I3C infrastructure Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-08-01 23:40 +0200
            Re: [RFC 2/5] i3c: Add core I3C infrastructure Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-08-02 03:00 +0200
            Re: [RFC 2/5] i3c: Add core I3C infrastructure Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-08-02 04:20 +0200

Page 1 of 2  [1] 2  Next page →


#1700175 — [RFC 0/5] Add I3C subsystem

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2017-07-31 18:30 +0200
Subject[RFC 0/5] Add I3C subsystem
Message-ID<u9lJU-8uS-7@gated-at.bofh.it>
This patch series is a proposal for a new I3C [1] subsystem.

This infrastructure is not complete yet and will be extended over
time.

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 bus element is a separate object and is not implicitly described
  by the master (as done in I2C). The reason is that I want to be able
  to handle multiple master connected to the same bus and visible to
  Linux.
  In this situation, we should only have one instance of the device and
  not one per master, and sharing the bus object would be part of the
  solution to gracefully handle this case.
  I'm not sure if we will ever need to deal with multiple masters
  controlling the same bus and exposed under Linux, but separating the
  bus and master concept is pretty easy, hence the decision to do it
  now, just in case we need it some day.
  The other benefit of separating the bus and master concepts is that
  master devices appear under the bus directory in sysfs.
- 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.
- the whole API is exposed through a single header file (i3c.h), but I'm
  seriously considering the option of splitting the I3C driver/user API
  and the I3C master one, mainly to hide I3C core internals and restrict
  what I3C users can do to a limited set of functionalities (send
  I3C/I2C frames to a specific device and that's all).

Missing features in this preliminary version:
- no support for IBI (In Band Interrupts). This is something I'm working
  on, and I'm still unsure how to represent it: an irqchip or a
  completely independent representation that would be I3C specific.
  Right now, I'm more inclined to go for the irqchip approach, since
  this is something people are used to deal with already.
- no Hot Join support, which is similar to hotplug
- 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

If possible, I'd like to have reviews from people that are familiar
with the device model and complex/autodiscoverable busses like USB.
Arnd, Greg, I think your feedback would be very valuable here.

Wolfram, feel free to comment on the integration with the I2C subsystem,
and let me know if you see a better option.

I'd also like to get feedback on the doc. Should I detail a bit more
the protocol or the framework API? Is this the kind of things you
expect in a subsystem doc?

I'm also unsure how far I should go with sysfs attributes. Right
now, I have exposed things that should matter to udev & co plus some
extra information about I3C dev capabilities (sysfs files have not
been documented yet, but I'll do it for the next version of this patch
series). I could go even further and expose more details like device
limitations (in terms of speed), device status, etc. However, I don't
know if those information are relevant to user-space applications.

If you know other people that might be interested by this patchset,
just let me know and I'll Cc them on the next version.

Thanks,

Boris

[1]https://www.mipi.org/specifications/i3c-sensor-specification

Boris Brezillon (5):
  i2c: Export of_i2c_get_board_info()
  i3c: Add core I3C infrastructure
  dt-bindings: i3c: Document core bindings
  i3c: master: Add driver for Cadence IP
  dt-bindings: i3c: Document Cadence I3C master bindings

 .../devicetree/bindings/i3c/cdns,i3c-master.txt    |   45 +
 Documentation/devicetree/bindings/i3c/i3c.txt      |   90 ++
 Documentation/i3c/conf.py                          |   10 +
 Documentation/i3c/device-driver-api.rst            |    7 +
 Documentation/i3c/index.rst                        |    9 +
 Documentation/i3c/master-driver-api.rst            |    8 +
 Documentation/i3c/protocol.rst                     |  199 +++
 Documentation/index.rst                            |    1 +
 drivers/Kconfig                                    |    2 +
 drivers/Makefile                                   |    2 +-
 drivers/i2c/i2c-core-base.c                        |    2 +-
 drivers/i2c/i2c-core-of.c                          |   64 +-
 drivers/i3c/Kconfig                                |   24 +
 drivers/i3c/Makefile                               |    3 +
 drivers/i3c/core.c                                 |  532 ++++++++
 drivers/i3c/device.c                               |  138 ++
 drivers/i3c/internals.h                            |   45 +
 drivers/i3c/master.c                               | 1225 +++++++++++++++++
 drivers/i3c/master/Kconfig                         |    4 +
 drivers/i3c/master/Makefile                        |    1 +
 drivers/i3c/master/i3c-master-cdns.c               | 1382 ++++++++++++++++++++
 include/linux/i2c.h                                |   10 +
 include/linux/i3c/ccc.h                            |  389 ++++++
 include/linux/i3c/device.h                         |  212 +++
 include/linux/i3c/master.h                         |  453 +++++++
 include/linux/mod_devicetable.h                    |   15 +
 26 files changed, 4843 insertions(+), 29 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
 create mode 100644 Documentation/devicetree/bindings/i3c/i3c.txt
 create mode 100644 Documentation/i3c/conf.py
 create mode 100644 Documentation/i3c/device-driver-api.rst
 create mode 100644 Documentation/i3c/index.rst
 create mode 100644 Documentation/i3c/master-driver-api.rst
 create mode 100644 Documentation/i3c/protocol.rst
 create mode 100644 drivers/i3c/Kconfig
 create mode 100644 drivers/i3c/Makefile
 create mode 100644 drivers/i3c/core.c
 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

-- 
2.7.4

[toc] | [next] | [standalone]


#1700176 — [RFC 5/5] dt-bindings: i3c: Document Cadence I3C master bindings

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2017-07-31 18:30 +0200
Subject[RFC 5/5] dt-bindings: i3c: Document Cadence I3C master bindings
Message-ID<u9lJU-8uS-21@gated-at.bofh.it>
In reply to#1700175
Document Cadence I3C master DT bindings.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 .../devicetree/bindings/i3c/cdns,i3c-master.txt    | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt

diff --git a/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt b/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
new file mode 100644
index 000000000000..f9e4af4ff1c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
@@ -0,0 +1,45 @@
+Bindings for cadence I3C master block
+=====================================
+
+Required properties:
+--------------------
+- compatible: shall be "cdns,i3c-master"
+- clocks: shall reference the pclk and sysclk
+- clock-names: shall contain "pclk" and "sysclk"
+- interrupts: the interrupt line connected to this I3C master
+- reg: I3C master registers
+
+Mandatory properties defined by the generic binding (see
+Documentation/devicetree/bindings/i3c/i3c.txt for more details):
+
+- #address-cells: shall be set to 1
+- #size-cells: shall be set to 0
+
+Optional properties defined by the generic binding (see
+Documentation/devicetree/bindings/i3c/i3c.txt for more details):
+
+- i2c-scl-frequency
+- i3c-scl-frequency
+
+I3C device connected on the bus follow the generic description (see
+Documentation/devicetree/bindings/i3c/i3c.txt for more details).
+
+Example:
+
+	i3c-master@0d040000 {
+		compatible = "cdns,i3c-master";
+		clocks = <&coreclock>, <&i3csysclock>;
+		clock-names = "pclk", "sysclk";
+		interrupts = <3 0>;
+		reg = <0x0d040000 0x1000>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		i2c-scl-frequency = <100000>;
+
+		nunchuk: nunchuk@52 {
+			compatible = "nintendo,nunchuk";
+			reg = <0x52>;
+			i3c-lvr = <0x10>;
+		};
+	};
+
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1700333 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromWolfram Sang <wsa@the-dreams.de>
Date2017-07-31 21:20 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9ooq-1IS-11@gated-at.bofh.it>
In reply to#1700175

[Multipart message — attachments visible in raw view] — view raw

> +This document is just a brief introduction to the I3C protocol and the concepts
> +it brings on the table. If you need more information, please refer to the MIPI
> +I3C specification.

I wish I could.

> +
> +Introduction
> +============
> +
> +The I3C (I-Cube-C) is a MIPI standardized protocol designed to overcome I2C

"Eye-three-See", according to:
http://eecatalog.com/sensors/2017/07/05/after-35-years-of-i2c-i3c-improves-capability-and-performance/

> +Backward compatibility with I2C devices
> +=======================================
> +
> +The I3C protocol has been designed to be backward compatible with I2C devices.
> +This backward compatibility allows one to connect a mix of I2C and I3C devices
> +on the same bus, though, in order to be really efficient, I2C devices should
> +be equipped with 50 ns spike filters.

I just found a slide which says I3C does not support clock stretching.
That should be mentioned here, too.

[toc] | [prev] | [next] | [standalone]


#1700382 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2017-07-31 22:50 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9pNv-2t7-5@gated-at.bofh.it>
In reply to#1700333
Le Mon, 31 Jul 2017 21:17:21 +0200,
Wolfram Sang <wsa@the-dreams.de> a écrit :

> > +This document is just a brief introduction to the I3C protocol and the concepts
> > +it brings on the table. If you need more information, please refer to the MIPI
> > +I3C specification.  
> 
> I wish I could.
> 
> > +
> > +Introduction
> > +============
> > +
> > +The I3C (I-Cube-C) is a MIPI standardized protocol designed to overcome I2C  
> 
> "Eye-three-See", according to:
> http://eecatalog.com/sensors/2017/07/05/after-35-years-of-i2c-i3c-improves-capability-and-performance/

I remember hearing eye-cube-see during the discussion we had with
Cadence engineers but I might wrong. I'll double check (or maybe I'll
just drop any mention of the pronunciation).

> 
> > +Backward compatibility with I2C devices
> > +=======================================
> > +
> > +The I3C protocol has been designed to be backward compatible with I2C devices.
> > +This backward compatibility allows one to connect a mix of I2C and I3C devices
> > +on the same bus, though, in order to be really efficient, I2C devices should
> > +be equipped with 50 ns spike filters.  
> 
> I just found a slide which says I3C does not support clock stretching.
> That should be mentioned here, too.
> 

You're right, clock stretching is not allowed, and if devices without a
50ns spike filter are connected to the bus, it lowers the maximum
speed for all devices, which renders I3C kind of useless.

[toc] | [prev] | [next] | [standalone]


#1700334

FromWolfram Sang <wsa@the-dreams.de>
Date2017-07-31 21:20 +0200
Message-ID<u9ooq-1IS-15@gated-at.bofh.it>
In reply to#1700175

[Multipart message — attachments visible in raw view] — view raw

Hi Boris,

> This patch series is a proposal for a new I3C [1] subsystem.

Nice. Good luck with that!

Some hi-level comments from me related to I2C. I can't say a lot more
because the specs are not public :(

> - the bus element is a separate object and is not implicitly described
>   by the master (as done in I2C). The reason is that I want to be able
>   to handle multiple master connected to the same bus and visible to
>   Linux.
>   In this situation, we should only have one instance of the device and
>   not one per master, and sharing the bus object would be part of the
>   solution to gracefully handle this case.
>   I'm not sure if we will ever need to deal with multiple masters
>   controlling the same bus and exposed under Linux, but separating the
>   bus and master concept is pretty easy, hence the decision to do it
>   now, just in case we need it some day.

From my experience, it is a good thing to have this separation.

> - 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.

I agree this is the least invasive and also the most compatible
approach. The other solution would probably be to have some kind of
emulation layer?

> I'd also like to get feedback on the doc. Should I detail a bit more
> the protocol or the framework API? Is this the kind of things you
> expect in a subsystem doc?

Since the spec is not public, details about the protocol will be
especially useful, I'd say.

Regards,

   Wolfram

[toc] | [prev] | [next] | [standalone]


#1700380

FromWolfram Sang <wsa@the-dreams.de>
Date2017-07-31 22:50 +0200
Message-ID<u9pNv-2t7-1@gated-at.bofh.it>
In reply to#1700334

[Multipart message — attachments visible in raw view] — view raw

> > I agree this is the least invasive and also the most compatible
> > approach. The other solution would probably be to have some kind of
> > emulation layer?
> 
> Could you detail a bit more what you mean by "emulation layer"?

Not really. That was more a extremly high level approach of what
theoretically could be possible. When I try to think about details, it
gets pretty invasive.

> > Since the spec is not public, details about the protocol will be
> > especially useful, I'd say.
> 
> Okay, I'll see what I can do.

Thanks.

[toc] | [prev] | [next] | [standalone]


#1700381

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2017-07-31 22:50 +0200
Message-ID<u9pNv-2t7-3@gated-at.bofh.it>
In reply to#1700334
Hi Wolfram, 

Le Mon, 31 Jul 2017 21:17:45 +0200,
Wolfram Sang <wsa@the-dreams.de> a écrit :

> Hi Boris,
> 
> > This patch series is a proposal for a new I3C [1] subsystem.  
> 
> Nice. Good luck with that!
> 
> Some hi-level comments from me related to I2C. I can't say a lot more
> because the specs are not public :(

Unfortunately they're not :(.

> 
> > - the bus element is a separate object and is not implicitly described
> >   by the master (as done in I2C). The reason is that I want to be able
> >   to handle multiple master connected to the same bus and visible to
> >   Linux.
> >   In this situation, we should only have one instance of the device and
> >   not one per master, and sharing the bus object would be part of the
> >   solution to gracefully handle this case.
> >   I'm not sure if we will ever need to deal with multiple masters
> >   controlling the same bus and exposed under Linux, but separating the
> >   bus and master concept is pretty easy, hence the decision to do it
> >   now, just in case we need it some day.  
> 
> From my experience, it is a good thing to have this separation.

Good to hear that you agree with this approach.

> 
> > - 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.  
> 
> I agree this is the least invasive and also the most compatible
> approach. The other solution would probably be to have some kind of
> emulation layer?

Could you detail a bit more what you mean by "emulation layer"?

> 
> > I'd also like to get feedback on the doc. Should I detail a bit more
> > the protocol or the framework API? Is this the kind of things you
> > expect in a subsystem doc?  
> 
> Since the spec is not public, details about the protocol will be
> especially useful, I'd say.

Okay, I'll see what I can do.

Thanks,

Boris

[toc] | [prev] | [next] | [standalone]


#1700368 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromArnd Bergmann <arnd@arndb.de>
Date2017-07-31 22:20 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9pku-2jJ-9@gated-at.bofh.it>
In reply to#1700175
On Mon, Jul 31, 2017 at 6:24 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> Add core infrastructure to support I3C in Linux and document it.

> - 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.

Can you describe the reasons for making i3c a separate subsystem then,
rather than extending the i2c subsystem to handle both i2c devices as
before and also i3c devices and hosts?

        Arnd

[toc] | [prev] | [next] | [standalone]


#1700395 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2017-07-31 23:20 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9qgx-2Se-7@gated-at.bofh.it>
In reply to#1700368
Hi Arnd,

Le Mon, 31 Jul 2017 22:16:42 +0200,
Arnd Bergmann <arnd@arndb.de> a écrit :

> On Mon, Jul 31, 2017 at 6:24 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > Add core infrastructure to support I3C in Linux and document it.  
> 
> > - 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.  
> 
> Can you describe the reasons for making i3c a separate subsystem then,
> rather than extending the i2c subsystem to handle both i2c devices as
> before and also i3c devices and hosts?

Actually, that's the first option I considered, but I3C and I2C are
really different. I'm not talking about the physical layer here, but
the way the bus has to be handled by the software layer. Actually, I
thing the I3C bus is philosophically closer to auto-discoverable busses
like USB than I2C or SPI.

Indeed, all I3C devices can be discovered and do not need to be
described at the board level (using DT, board files, ACPI or whatever).
Also, some I3C devices are hotpluggable, and most importantly, all I3C
devices describe themselves during the discovery procedure (called DAA
in the I3C world).

There is some kind of "device class" concept. In the I3C world it's
called DCR (Device Characteristic Register), but it plays the same role:
it's a set of generic interfaces devices have to comply with when they
declare themselves as being compatible with a DCR ID (like
accelerometer, gyroscope, or whatever). See this table of normalized
DCR for more information [1].

Devices also expose a 48-bit Provisional ID which is made of
sub-fields. Two of them are particularly interesting: the manufacturer
ID and the part ID, which are comparable to the vendor and product ID in
the USB world.

These three information (DCR, ManufacturerID and PartID) can be used to
match drivers instead of the compatible string or driver-name used for
I2C devices

So, as you can imagine, dealing with an I3C bus is really different
from dealing with an I2C bus, and I found the "expose an i2c_adapter
object for each i3c_master" way simpler (and less invasive) than
extending the I2C framework to support I3C devices.

Of course, I can move all the code in drivers/i2c/, but that won't
change the fact that I3C and I2C busses are completely different
with little to share between them.

To me, the I2C backward compatibility is just a nice feature that was
added to help people smoothly transition from mixed I3C busses with
both I2C and I3C devices connected to it (I2C devices being here
when no (affordable) equivalent exist in the I3C world) to pure I3C
busses with only I3C devices connected to it.

This being said, I'd be happy if you prove me wrong and propose a
solution that allows us to extend the I2C framework to support I3C
without to much pain ;-).

Thanks,

Boris

[1]https://www.mipi.org/MIPI_I3C_device_characteristics_register

[toc] | [prev] | [next] | [standalone]


#1700408 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromWolfram Sang <wsa@the-dreams.de>
Date2017-07-31 23:50 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9qJA-323-7@gated-at.bofh.it>
In reply to#1700395

[Multipart message — attachments visible in raw view] — view raw

> Actually, that's the first option I considered, but I3C and I2C are
> really different. I'm not talking about the physical layer here, but
> the way the bus has to be handled by the software layer. Actually, I
> thing the I3C bus is philosophically closer to auto-discoverable busses
> like USB than I2C or SPI.

Acked-by: Wolfram Sang <wsa@the-dreams.de>

> Of course, I can move all the code in drivers/i2c/, but that won't
> change the fact that I3C and I2C busses are completely different
> with little to share between them.

That wouldn't make sense.

> To me, the I2C backward compatibility is just a nice feature that was
> added to help people smoothly transition from mixed I3C busses with
> both I2C and I3C devices connected to it (I2C devices being here
> when no (affordable) equivalent exist in the I3C world) to pure I3C
> busses with only I3C devices connected to it.

Yeah, and it is still to be seen how good this really works. Devices
which do clock stretching are out of the question. Probably everything
which needs an interrupt as well?

> This being said, I'd be happy if you prove me wrong and propose a
> solution that allows us to extend the I2C framework to support I3C
> without to much pain ;-).

From all I know, I don't see that coming.

[toc] | [prev] | [next] | [standalone]


#1701234 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

From"Andrew F. Davis" <afd@ti.com>
Date2017-08-01 18:50 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9IwP-6mc-15@gated-at.bofh.it>
In reply to#1700408
On 07/31/2017 04:42 PM, Wolfram Sang wrote:
> 
>> Actually, that's the first option I considered, but I3C and I2C are
>> really different. I'm not talking about the physical layer here, but
>> the way the bus has to be handled by the software layer. Actually, I
>> thing the I3C bus is philosophically closer to auto-discoverable busses
>> like USB than I2C or SPI.
> 
> Acked-by: Wolfram Sang <wsa@the-dreams.de>
> 
>> Of course, I can move all the code in drivers/i2c/, but that won't
>> change the fact that I3C and I2C busses are completely different
>> with little to share between them.
> 
> That wouldn't make sense.
> 
>> To me, the I2C backward compatibility is just a nice feature that was
>> added to help people smoothly transition from mixed I3C busses with
>> both I2C and I3C devices connected to it (I2C devices being here
>> when no (affordable) equivalent exist in the I3C world) to pure I3C
>> busses with only I3C devices connected to it.
> 
> Yeah, and it is still to be seen how good this really works. Devices
> which do clock stretching are out of the question. Probably everything
> which needs an interrupt as well?
> 

I'm surprised they didn't allow for slave clock stretching when
communicating with a legacy i2c device, it will prohibit use of a rather
large class of devices. :(

As for interrupts you are always free to wire up an out-of-band
interrupt like before. :)

>> This being said, I'd be happy if you prove me wrong and propose a
>> solution that allows us to extend the I2C framework to support I3C
>> without to much pain ;-).
> 
> From all I know, I don't see that coming.
> 

[toc] | [prev] | [next] | [standalone]


#1701264 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromWolfram Sang <wsa@the-dreams.de>
Date2017-08-01 19:30 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9J9w-6UE-19@gated-at.bofh.it>
In reply to#1701234

[Multipart message — attachments visible in raw view] — view raw

> I'm surprised they didn't allow for slave clock stretching when
> communicating with a legacy i2c device, it will prohibit use of a rather
> large class of devices. :(

Yes, but I3C is push/pull IIRC.

> As for interrupts you are always free to wire up an out-of-band
> interrupt like before. :)

Yes, my wording was a bit too strong. It is possible, sure. Yet, I
understood that one of the features of I3C is to have in-band interrupt
support. We will see if the demand for backward compatibility or "saving
pins" is higher.

[toc] | [prev] | [next] | [standalone]


#1701525 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2017-08-01 23:50 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9Nd7-UI-1@gated-at.bofh.it>
In reply to#1701264
Le Tue, 1 Aug 2017 19:27:03 +0200,
Wolfram Sang <wsa@the-dreams.de> a écrit :

> > I'm surprised they didn't allow for slave clock stretching when
> > communicating with a legacy i2c device, it will prohibit use of a rather
> > large class of devices. :(  
> 
> Yes, but I3C is push/pull IIRC.

It is.

> 
> > As for interrupts you are always free to wire up an out-of-band
> > interrupt like before. :)  
> 
> Yes, my wording was a bit too strong. It is possible, sure. Yet, I
> understood that one of the features of I3C is to have in-band interrupt
> support. We will see if the demand for backward compatibility or "saving
> pins" is higher.
> 

Indeed, you can use in-band interrupts if your device is able to
generate them, but that doesn't prevent I3C device designers from using
an external pin to signal interrupts if they prefer.

[toc] | [prev] | [next] | [standalone]


#1701990 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromWolfram Sang <wsa@the-dreams.de>
Date2017-08-02 12:30 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9Z4C-aO-35@gated-at.bofh.it>
In reply to#1701525

[Multipart message — attachments visible in raw view] — view raw

> > Yes, my wording was a bit too strong. It is possible, sure. Yet, I
> > understood that one of the features of I3C is to have in-band interrupt
> > support. We will see if the demand for backward compatibility or "saving
> > pins" is higher.
> > 
> 
> Indeed, you can use in-band interrupts if your device is able to
> generate them, but that doesn't prevent I3C device designers from using
> an external pin to signal interrupts if they prefer.

Exactly. Thus, "We will see if the demand for backward compatibility or "saving
pins" is higher" :)

[toc] | [prev] | [next] | [standalone]


#1700947 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromArnd Bergmann <arnd@arndb.de>
Date2017-08-01 14:10 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9E9S-3Rb-35@gated-at.bofh.it>
In reply to#1700395
On Mon, Jul 31, 2017 at 11:15 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> Hi Arnd,
>
> Le Mon, 31 Jul 2017 22:16:42 +0200,
> Arnd Bergmann <arnd@arndb.de> a écrit :
>
>> On Mon, Jul 31, 2017 at 6:24 PM, Boris Brezillon
>> <boris.brezillon@free-electrons.com> wrote:
>> > Add core infrastructure to support I3C in Linux and document it.
>>
>> > - 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.
>>
>> Can you describe the reasons for making i3c a separate subsystem then,
>> rather than extending the i2c subsystem to handle both i2c devices as
>> before and also i3c devices and hosts?
>
> Actually, that's the first option I considered, but I3C and I2C are
> really different. I'm not talking about the physical layer here, but
> the way the bus has to be handled by the software layer. Actually, I
> thing the I3C bus is philosophically closer to auto-discoverable busses
> like USB than I2C or SPI.
>
> Indeed, all I3C devices can be discovered and do not need to be
> described at the board level (using DT, board files, ACPI or whatever).
> Also, some I3C devices are hotpluggable, and most importantly, all I3C
> devices describe themselves during the discovery procedure (called DAA
> in the I3C world).

Side note: please make sure you define a way to describe them
in DT anyway. We ended up needing additional DT properties
as well as power sequencing for most discoverable buses (pci,
usb, mmc, ...), I'm sure this one won't be an exception even though
the standard says you don't need it and most devices will work
without it.

> There is some kind of "device class" concept. In the I3C world it's
> called DCR (Device Characteristic Register), but it plays the same role:
> it's a set of generic interfaces devices have to comply with when they
> declare themselves as being compatible with a DCR ID (like
> accelerometer, gyroscope, or whatever). See this table of normalized
> DCR for more information [1].
>
> Devices also expose a 48-bit Provisional ID which is made of
> sub-fields. Two of them are particularly interesting: the manufacturer
> ID and the part ID, which are comparable to the vendor and product ID in
> the USB world.
>
> These three information (DCR, ManufacturerID and PartID) can be used to
> match drivers instead of the compatible string or driver-name used for
> I2C devices

The matching would be fairly easy to accomodate: the i2c bus already
handles two distinct ways: of_device_id tables and matching by
name, so we could easily add another method here.

> So, as you can imagine, dealing with an I3C bus is really different
> from dealing with an I2C bus, and I found the "expose an i2c_adapter
> object for each i3c_master" way simpler (and less invasive) than
> extending the I2C framework to support I3C devices.
>
> Of course, I can move all the code in drivers/i2c/, but that won't
> change the fact that I3C and I2C busses are completely different
> with little to share between them.
>
> To me, the I2C backward compatibility is just a nice feature that was
> added to help people smoothly transition from mixed I3C busses with
> both I2C and I3C devices connected to it (I2C devices being here
> when no (affordable) equivalent exist in the I3C world) to pure I3C
> busses with only I3C devices connected to it.
>
> This being said, I'd be happy if you prove me wrong and propose a
> solution that allows us to extend the I2C framework to support I3C
> without to much pain ;-).

I think the question is not whether it can be done or not, but whether
it is a good idea. Obviously we can create some frankenstein bus
design that combines arbitrary different device types by just containing
the superset of the required information, and sprinking the code
with if()/else() to call one or the other function.

If there is very little shared code between the i2c and i3c
implementations, then the added complexity of having a combined
subsystem is clearly a strong argument against it.

On the other hand, there is value in representing the physical
bus hierarchy in the software model, and if i2c and i3c devices can
be attached to the same host bus, a good abstraction should
show them under the same parent. This is true for both the
kernel representation (in sysfs and the data structures) as well
as the device tree binding (assuming we will need to represent
i3c devices at all). The two don't have to use the same model,
but it's easier if they do.

Another argument for a combined bus would be devices that
can be attached to either i2c and i3c, depending on the host
capabilities. We have discussed whether i2c and spi should be
merged into a single bus_type in the past, as a lot of devices
can be attached to either of them. If it's common enough for i3c
devices to support an i2c fallback mode, having a common
bus_type might noticeably simplify device drivers by only requiring
a single i2c_driver structure. Simplifying many drivers a little
bit can in turn offset the added complexity in the subsystem.

       Arnd

[toc] | [prev] | [next] | [standalone]


#1700972 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2017-08-01 14:30 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9Etb-3XT-5@gated-at.bofh.it>
In reply to#1700947
On Tue, 1 Aug 2017 14:00:05 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Mon, Jul 31, 2017 at 11:15 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > Hi Arnd,
> >
> > Le Mon, 31 Jul 2017 22:16:42 +0200,
> > Arnd Bergmann <arnd@arndb.de> a écrit :
> >  
> >> On Mon, Jul 31, 2017 at 6:24 PM, Boris Brezillon
> >> <boris.brezillon@free-electrons.com> wrote:  
> >> > Add core infrastructure to support I3C in Linux and document it.  
> >>  
> >> > - 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.  
> >>
> >> Can you describe the reasons for making i3c a separate subsystem then,
> >> rather than extending the i2c subsystem to handle both i2c devices as
> >> before and also i3c devices and hosts?  
> >
> > Actually, that's the first option I considered, but I3C and I2C are
> > really different. I'm not talking about the physical layer here, but
> > the way the bus has to be handled by the software layer. Actually, I
> > thing the I3C bus is philosophically closer to auto-discoverable busses
> > like USB than I2C or SPI.
> >
> > Indeed, all I3C devices can be discovered and do not need to be
> > described at the board level (using DT, board files, ACPI or whatever).
> > Also, some I3C devices are hotpluggable, and most importantly, all I3C
> > devices describe themselves during the discovery procedure (called DAA
> > in the I3C world).  
> 
> Side note: please make sure you define a way to describe them
> in DT anyway. We ended up needing additional DT properties
> as well as power sequencing for most discoverable buses (pci,
> usb, mmc, ...), I'm sure this one won't be an exception even though
> the standard says you don't need it and most devices will work
> without it.
> 
> > There is some kind of "device class" concept. In the I3C world it's
> > called DCR (Device Characteristic Register), but it plays the same role:
> > it's a set of generic interfaces devices have to comply with when they
> > declare themselves as being compatible with a DCR ID (like
> > accelerometer, gyroscope, or whatever). See this table of normalized
> > DCR for more information [1].
> >
> > Devices also expose a 48-bit Provisional ID which is made of
> > sub-fields. Two of them are particularly interesting: the manufacturer
> > ID and the part ID, which are comparable to the vendor and product ID in
> > the USB world.
> >
> > These three information (DCR, ManufacturerID and PartID) can be used to
> > match drivers instead of the compatible string or driver-name used for
> > I2C devices  
> 
> The matching would be fairly easy to accomodate: the i2c bus already
> handles two distinct ways: of_device_id tables and matching by
> name, so we could easily add another method here.

Should be doable. All we need to do is define device PIDs (Provisional
IDs) in the DT so that they can be attached to the real device when it's
discovered on the bus. Also note that some I3C devices come with a
static/I2C/legacy address which can be used when this device is
connected on an I2C bus. Such devices can be accessed in I2C mode using
this static address before they get assigned a dynamic one by the I3C
master. So, that would be another solution to describe I3C devs in the
DT, but this won't work for all devs.

> 
> > So, as you can imagine, dealing with an I3C bus is really different
> > from dealing with an I2C bus, and I found the "expose an i2c_adapter
> > object for each i3c_master" way simpler (and less invasive) than
> > extending the I2C framework to support I3C devices.
> >
> > Of course, I can move all the code in drivers/i2c/, but that won't
> > change the fact that I3C and I2C busses are completely different
> > with little to share between them.
> >
> > To me, the I2C backward compatibility is just a nice feature that was
> > added to help people smoothly transition from mixed I3C busses with
> > both I2C and I3C devices connected to it (I2C devices being here
> > when no (affordable) equivalent exist in the I3C world) to pure I3C
> > busses with only I3C devices connected to it.
> >
> > This being said, I'd be happy if you prove me wrong and propose a
> > solution that allows us to extend the I2C framework to support I3C
> > without to much pain ;-).  
> 
> I think the question is not whether it can be done or not, but whether
> it is a good idea. Obviously we can create some frankenstein bus
> design that combines arbitrary different device types by just containing
> the superset of the required information, and sprinking the code
> with if()/else() to call one or the other function.
> 
> If there is very little shared code between the i2c and i3c
> implementations, then the added complexity of having a combined
> subsystem is clearly a strong argument against it.

AFAICT, there is little to share.

> 
> On the other hand, there is value in representing the physical
> bus hierarchy in the software model, and if i2c and i3c devices can
> be attached to the same host bus, a good abstraction should
> show them under the same parent.

I agree here, hence my comment in the cover letter.

> This is true for both the
> kernel representation (in sysfs and the data structures) as well
> as the device tree binding (assuming we will need to represent
> i3c devices at all).

DT representation is already adopting a single bus representation: I2C
devices are directly described under the I3C master/bus node and so
will I3C devs if we ever need to represent them in the DT.

> The two don't have to use the same model,
> but it's easier if they do.

Agreed.

> 
> Another argument for a combined bus would be devices that
> can be attached to either i2c and i3c, depending on the host
> capabilities.

Hm, that's already the case, isn't it? And you'll anyway need to
develop specific code for both cases in the I2C/I3C device driver
because I2C and I3C transfers are different. So I don't see how it
would help to have a single bus here.

> We have discussed whether i2c and spi should be
> merged into a single bus_type in the past, as a lot of devices
> can be attached to either of them.

Oh, really? What's the rational behind that? I mean, I2C and SPI are
quite different, and even if some devices provide both interfaces, I
don't see why we should merge them. But you probably had good reasons
to do so.

> If it's common enough for i3c
> devices to support an i2c fallback mode,

If the device has a static address, it's likely to be compatible with
I2C. Don't know how usual this is though.

> having a common
> bus_type might noticeably simplify device drivers by only requiring
> a single i2c_driver structure.

Well, it's not only about having 2 different driver objects. As I said,
I3C and I2C frames are different and the driver will have to handle the
device differently depending on whether it's addressed in I2C or I3C
mode.

> Simplifying many drivers a little
> bit can in turn offset the added complexity in the subsystem.

Hm, maybe you'll save a few lines (and a few hundred of bytes) by not
having to declare/register 2 different drivers, but IMHO that's not the
part that would be the most difficult when it comes to supporting both
I2C and I3C modes in the same driver.

[toc] | [prev] | [next] | [standalone]


#1701024 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromArnd Bergmann <arnd@arndb.de>
Date2017-08-01 15:20 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9FfB-4tk-21@gated-at.bofh.it>
In reply to#1700972
On Tue, Aug 1, 2017 at 2:29 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Tue, 1 Aug 2017 14:00:05 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:

>> Another argument for a combined bus would be devices that
>> can be attached to either i2c and i3c, depending on the host
>> capabilities.
>
> Hm, that's already the case, isn't it? And you'll anyway need to
> develop specific code for both cases in the I2C/I3C device driver
> because I2C and I3C transfers are different. So I don't see how it
> would help to have a single bus here.
>
>> We have discussed whether i2c and spi should be
>> merged into a single bus_type in the past, as a lot of devices
>> can be attached to either of them.
>
> Oh, really? What's the rational behind that? I mean, I2C and SPI are
> quite different, and even if some devices provide both interfaces, I
> don't see why we should merge them. But you probably had good reasons
> to do so.

Well, we never changed it, so at least the work required to merge
the two was considered too much to justify any advantages.

The main problem with having one driver that can operate on
different bus types (i2c plus either spi or i3c) is the handling for
the various combinations in configurations (e.g. I2C=m, SPI=y).

The easy case is having a module_init function that registers two
device drivers, but that requires having a Kconfig dependency
on both subsystems, and you can't use the module_i2c_driver()
helper.

The second way is to have a number of #ifdef and complex
Kconfig dependencies for the driver to only register the
device_driver objects for the buses that are enabled. This
is also doable, but everyone gets the logic wrong the first time.

What we end up doing to work around this for other drivers is
to have the base driver in one library module, and separate
modules for the bus-specific portions, which can then
use module_i2c_driver again. There are many instances
for combined i2c/spi drivers in the kernel, and it works fine,
but it adds a fair bit of overhead compared to having one
driver that would e.g. use regmap to abstract the differences
in the probe() function and otherwise keeps everything in
one place.

       Arnd

[toc] | [prev] | [next] | [standalone]


#1701052 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2017-08-01 15:40 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9FyX-4zN-31@gated-at.bofh.it>
In reply to#1701024
On Tue, 1 Aug 2017 15:11:44 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Tue, Aug 1, 2017 at 2:29 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Tue, 1 Aug 2017 14:00:05 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:  
> 
> >> Another argument for a combined bus would be devices that
> >> can be attached to either i2c and i3c, depending on the host
> >> capabilities.  
> >
> > Hm, that's already the case, isn't it? And you'll anyway need to
> > develop specific code for both cases in the I2C/I3C device driver
> > because I2C and I3C transfers are different. So I don't see how it
> > would help to have a single bus here.
> >  
> >> We have discussed whether i2c and spi should be
> >> merged into a single bus_type in the past, as a lot of devices
> >> can be attached to either of them.  
> >
> > Oh, really? What's the rational behind that? I mean, I2C and SPI are
> > quite different, and even if some devices provide both interfaces, I
> > don't see why we should merge them. But you probably had good reasons
> > to do so.  
> 
> Well, we never changed it, so at least the work required to merge
> the two was considered too much to justify any advantages.
> 
> The main problem with having one driver that can operate on
> different bus types (i2c plus either spi or i3c) is the handling for
> the various combinations in configurations (e.g. I2C=m, SPI=y).
> 
> The easy case is having a module_init function that registers two
> device drivers, but that requires having a Kconfig dependency
> on both subsystems, and you can't use the module_i2c_driver()
> helper.
> 
> The second way is to have a number of #ifdef and complex
> Kconfig dependencies for the driver to only register the
> device_driver objects for the buses that are enabled. This
> is also doable, but everyone gets the logic wrong the first time.

Hm, I understand now why you'd prefer to have a single bus. Can't we
solve this problem with a module_i3c_i2c_driver() macro that would hide
all this complexity from I2C/I3C drivers?

> 
> What we end up doing to work around this for other drivers is
> to have the base driver in one library module, and separate
> modules for the bus-specific portions, which can then
> use module_i2c_driver again. There are many instances
> for combined i2c/spi drivers in the kernel, and it works fine,
> but it adds a fair bit of overhead compared to having one
> driver that would e.g. use regmap to abstract the differences
> in the probe() function and otherwise keeps everything in
> one place.
> 
>        Arnd

[toc] | [prev] | [next] | [standalone]


#1701075 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2017-08-01 16:00 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9FSh-4Gv-1@gated-at.bofh.it>
In reply to#1701052
On Tue, 1 Aug 2017 15:34:14 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Tue, 1 Aug 2017 15:11:44 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > On Tue, Aug 1, 2017 at 2:29 PM, Boris Brezillon
> > <boris.brezillon@free-electrons.com> wrote:  
> > > On Tue, 1 Aug 2017 14:00:05 +0200
> > > Arnd Bergmann <arnd@arndb.de> wrote:    
> >   
> > >> Another argument for a combined bus would be devices that
> > >> can be attached to either i2c and i3c, depending on the host
> > >> capabilities.    
> > >
> > > Hm, that's already the case, isn't it? And you'll anyway need to
> > > develop specific code for both cases in the I2C/I3C device driver
> > > because I2C and I3C transfers are different. So I don't see how it
> > > would help to have a single bus here.
> > >    
> > >> We have discussed whether i2c and spi should be
> > >> merged into a single bus_type in the past, as a lot of devices
> > >> can be attached to either of them.    
> > >
> > > Oh, really? What's the rational behind that? I mean, I2C and SPI are
> > > quite different, and even if some devices provide both interfaces, I
> > > don't see why we should merge them. But you probably had good reasons
> > > to do so.    
> > 
> > Well, we never changed it, so at least the work required to merge
> > the two was considered too much to justify any advantages.
> > 
> > The main problem with having one driver that can operate on
> > different bus types (i2c plus either spi or i3c) is the handling for
> > the various combinations in configurations (e.g. I2C=m, SPI=y).
> > 
> > The easy case is having a module_init function that registers two
> > device drivers, but that requires having a Kconfig dependency
> > on both subsystems, and you can't use the module_i2c_driver()
> > helper.
> > 
> > The second way is to have a number of #ifdef and complex
> > Kconfig dependencies for the driver to only register the
> > device_driver objects for the buses that are enabled. This
> > is also doable, but everyone gets the logic wrong the first time.  
> 
> Hm, I understand now why you'd prefer to have a single bus. Can't we
> solve this problem with a module_i3c_i2c_driver() macro that would hide
> all this complexity from I2C/I3C drivers?

I just realized I forgot to add a "depends on I2C" in the I3C Kconfig
entry. Indeed, I'm unconditionally calling functions provided by the
I2C framework which have no dummy wrapper when I2C support is disabled.
I could of course conditionally compile some portion of the I3C
framework so that it still builds when I2C is disabled but I'm not sure
it's worth the trouble.

This "depends on I2C" should also solve the I2C+I3C driver issue, since
I2C is necessarily enabled when I3C is.

Am I missing something?

[toc] | [prev] | [next] | [standalone]


#1701111 — Re: [RFC 2/5] i3c: Add core I3C infrastructure

FromArnd Bergmann <arnd@arndb.de>
Date2017-08-01 16:30 +0200
SubjectRe: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID<u9Glj-55w-1@gated-at.bofh.it>
In reply to#1701075
On Tue, Aug 1, 2017 at 3:58 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Tue, 1 Aug 2017 15:34:14 +0200
> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
>> On Tue, 1 Aug 2017 15:11:44 +0200
>> Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Tue, Aug 1, 2017 at 2:29 PM, Boris Brezillon
>> > <boris.brezillon@free-electrons.com> wrote:
> I just realized I forgot to add a "depends on I2C" in the I3C Kconfig
> entry. Indeed, I'm unconditionally calling functions provided by the
> I2C framework which have no dummy wrapper when I2C support is disabled.
> I could of course conditionally compile some portion of the I3C
> framework so that it still builds when I2C is disabled but I'm not sure
> it's worth the trouble.
>
> This "depends on I2C" should also solve the I2C+I3C driver issue, since
> I2C is necessarily enabled when I3C is.
>
> Am I missing something?

That should solve another part of the problem, as a combined driver then
just needs 'depends on I3C'.

On top of that, the i3c_driver structure could also contain callback
pointers for the i2c subsystem, e.g. i2c_probe(), i2c_remove() etc.
When the i2c_probe() callback exists, the i3c layer could construct
a 'struct i2c_driver' with those callbacks and register that under the
cover. This would mean that combined drivers no longer need to
register two driver objects.

         Arnd

[toc] | [prev] | [next] | [standalone]


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web