| Message ID | 20180330074751.25987-1-boris.brezillon@bootlin.com |
|---|---|
| Headers | show |
| Series | Add the I3C subsystem | expand |
Hi, On Fri, 30 Mar 2018 09:47:41 +0200 Boris Brezillon <boris.brezillon@bootlin.com> wrote: > This patch series is a proposal for a new I3C subsystem. This v4 has been sent almost a month ago and I didn't get any feedback so far apart from Rob's R-b. Greg, is there any chance we can get these patches merged in 4.18? If not, could you tell me what should be addressed/improved/reworked? Thanks, Boris > > 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. > > Missing features in this preliminary version: > - 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 > > 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 > > Note that I2C changes have been sent separately [1] this time. Other > than that, no big changes in this version, I just addressed the > comments I received from Thomas, Peter, Geert and Rob. > > Thanks, > > Boris > > [1]https://patchwork.ozlabs.org/project/linux-i2c/list/?series=35687 > > Boris Brezillon (10): > i3c: Add core I3C infrastructure > docs: driver-api: Add I3C documentation > i3c: Add sysfs ABI spec > dt-bindings: i3c: Document core bindings > dt-bindings: i3c: Add macros to help fill I3C/I2C device's reg > property > 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 | 95 ++ > .../devicetree/bindings/gpio/gpio-cdns-i3c.txt | 39 + > .../devicetree/bindings/i3c/cdns,i3c-master.txt | 44 + > Documentation/devicetree/bindings/i3c/i3c.txt | 140 ++ > Documentation/driver-api/i3c/conf.py | 10 + > Documentation/driver-api/i3c/device-driver-api.rst | 7 + > Documentation/driver-api/i3c/index.rst | 9 + > Documentation/driver-api/i3c/master-driver-api.rst | 8 + > Documentation/driver-api/i3c/protocol.rst | 201 +++ > Documentation/driver-api/index.rst | 1 + > MAINTAINERS | 9 + > drivers/Kconfig | 2 + > drivers/Makefile | 2 +- > drivers/gpio/Kconfig | 11 + > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-cdns-i3c.c | 380 +++++ > drivers/i3c/Kconfig | 24 + > drivers/i3c/Makefile | 4 + > drivers/i3c/core.c | 620 +++++++ > drivers/i3c/device.c | 294 ++++ > drivers/i3c/internals.h | 28 + > drivers/i3c/master.c | 1723 ++++++++++++++++++++ > drivers/i3c/master/Kconfig | 5 + > drivers/i3c/master/Makefile | 1 + > drivers/i3c/master/i3c-master-cdns.c | 1650 +++++++++++++++++++ > include/dt-bindings/i3c/i3c.h | 28 + > include/linux/i3c/ccc.h | 382 +++++ > include/linux/i3c/device.h | 305 ++++ > include/linux/i3c/master.h | 587 +++++++ > include/linux/mod_devicetable.h | 17 + > 30 files changed, 6626 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/conf.py > 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/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/dt-bindings/i3c/i3c.h > create mode 100644 include/linux/i3c/ccc.h > create mode 100644 include/linux/i3c/device.h > create mode 100644 include/linux/i3c/master.h > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 23, 2018 at 07:38:14PM +0200, Boris Brezillon wrote: > Hi, > > On Fri, 30 Mar 2018 09:47:41 +0200 > Boris Brezillon <boris.brezillon@bootlin.com> wrote: > > > This patch series is a proposal for a new I3C subsystem. > > This v4 has been sent almost a month ago and I didn't get any feedback > so far apart from Rob's R-b. Greg, is there any chance we can get these > patches merged in 4.18? If not, could you tell me what should be > addressed/improved/reworked? I'll look over it later this week, thanks. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 23, 2018 at 07:56:46PM +0200, Greg Kroah-Hartman wrote: > On Mon, Apr 23, 2018 at 07:38:14PM +0200, Boris Brezillon wrote: > > Hi, > > > > On Fri, 30 Mar 2018 09:47:41 +0200 > > Boris Brezillon <boris.brezillon@bootlin.com> wrote: > > > > > This patch series is a proposal for a new I3C subsystem. > > > > This v4 has been sent almost a month ago and I didn't get any feedback > > so far apart from Rob's R-b. Greg, is there any chance we can get these > > patches merged in 4.18? If not, could you tell me what should be > > addressed/improved/reworked? > > I'll look over it later this week, thanks. At first glance, it would be great to have at least one other reviewed-by or signed-off-by on the main code here. I don't want to be the only one to have to review it :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Greg, On Sun, 29 Apr 2018 15:36:42 +0200 Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Mon, Apr 23, 2018 at 07:56:46PM +0200, Greg Kroah-Hartman wrote: > > On Mon, Apr 23, 2018 at 07:38:14PM +0200, Boris Brezillon wrote: > > > Hi, > > > > > > On Fri, 30 Mar 2018 09:47:41 +0200 > > > Boris Brezillon <boris.brezillon@bootlin.com> wrote: > > > > > > > This patch series is a proposal for a new I3C subsystem. > > > > > > This v4 has been sent almost a month ago and I didn't get any feedback > > > so far apart from Rob's R-b. Greg, is there any chance we can get these > > > patches merged in 4.18? If not, could you tell me what should be > > > addressed/improved/reworked? > > > > I'll look over it later this week, thanks. > > At first glance, it would be great to have at least one other > reviewed-by or signed-off-by on the main code here. I don't want to be > the only one to have to review it :) I understand. Arnd, Wolfram, any chance you could spend some time reviewing this patch series? I know Arnd is in vacation for the next few weeks, so I don't expect him to be able to do that immediately. Also, Wolfram stated that he didn't have time to review the series in details when I submitted the v1, I don't know if things have changed since then. Anyway, I'll keep searching for people to review the code. Regards, Boris -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
This patch series is a proposal for a new I3C 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. Missing features in this preliminary version: - 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 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 Note that I2C changes have been sent separately [1] this time. Other than that, no big changes in this version, I just addressed the comments I received from Thomas, Peter, Geert and Rob. Thanks, Boris [1]https://patchwork.ozlabs.org/project/linux-i2c/list/?series=35687 Boris Brezillon (10): i3c: Add core I3C infrastructure docs: driver-api: Add I3C documentation i3c: Add sysfs ABI spec dt-bindings: i3c: Document core bindings dt-bindings: i3c: Add macros to help fill I3C/I2C device's reg property 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 | 95 ++ .../devicetree/bindings/gpio/gpio-cdns-i3c.txt | 39 + .../devicetree/bindings/i3c/cdns,i3c-master.txt | 44 + Documentation/devicetree/bindings/i3c/i3c.txt | 140 ++ Documentation/driver-api/i3c/conf.py | 10 + Documentation/driver-api/i3c/device-driver-api.rst | 7 + Documentation/driver-api/i3c/index.rst | 9 + Documentation/driver-api/i3c/master-driver-api.rst | 8 + Documentation/driver-api/i3c/protocol.rst | 201 +++ Documentation/driver-api/index.rst | 1 + MAINTAINERS | 9 + drivers/Kconfig | 2 + drivers/Makefile | 2 +- drivers/gpio/Kconfig | 11 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-cdns-i3c.c | 380 +++++ drivers/i3c/Kconfig | 24 + drivers/i3c/Makefile | 4 + drivers/i3c/core.c | 620 +++++++ drivers/i3c/device.c | 294 ++++ drivers/i3c/internals.h | 28 + drivers/i3c/master.c | 1723 ++++++++++++++++++++ drivers/i3c/master/Kconfig | 5 + drivers/i3c/master/Makefile | 1 + drivers/i3c/master/i3c-master-cdns.c | 1650 +++++++++++++++++++ include/dt-bindings/i3c/i3c.h | 28 + include/linux/i3c/ccc.h | 382 +++++ include/linux/i3c/device.h | 305 ++++ include/linux/i3c/master.h | 587 +++++++ include/linux/mod_devicetable.h | 17 + 30 files changed, 6626 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/conf.py 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/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/dt-bindings/i3c/i3c.h create mode 100644 include/linux/i3c/ccc.h create mode 100644 include/linux/i3c/device.h create mode 100644 include/linux/i3c/master.h