All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Wolfram Sang <wsa@the-dreams.de>,
	linux-i2c@vger.kernel.org, Jonathan Corbet <corbet@lwn.net>,
	linux-doc@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Przemyslaw Sroka <psroka@cadence.com>,
	Arkadiusz Golec <agolec@cadence.com>,
	Alan Douglas <adouglas@cadence.com>,
	Bartosz Folta <bfolta@cadence.com>, Damian Kos <dkos@cadence.com>,
	Alicja Jurasik-Urbaniak <alicja@cadence.com>,
	Jan Kotas <jank@cadence.com>,
	Cyprian Wronka <cwronka@cadence.com>,
	Alexandre Belloni <alexandre.belloni@free-electrons.com>,
	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	Nishanth Menon <nm@ti.com>, Rob Herring <robh+dt@kernel.org>,
	Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	devicetree@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [RFC 2/5] i3c: Add core I3C infrastructure
Date: Tue, 1 Aug 2017 15:11:44 +0200	[thread overview]
Message-ID: <CAK8P3a0SH4kWcvXK+_iPt9LWuNnpjr+omp+ZpyTLjT=H602n+w@mail.gmail.com> (raw)
In-Reply-To: <20170801142936.5df48702@bbrezillon>

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

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Wolfram Sang <wsa@the-dreams.de>,
	linux-i2c@vger.kernel.org, Jonathan Corbet <corbet@lwn.net>,
	linux-doc@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Przemyslaw Sroka <psroka@cadence.com>,
	Arkadiusz Golec <agolec@cadence.com>,
	Alan Douglas <adouglas@cadence.com>,
	Bartosz Folta <bfolta@cadence.com>, Damian Kos <dkos@cadence.com>,
	Alicja Jurasik-Urbaniak <alicja@cadence.com>,
	Jan Kotas <jank@cadence.com>,
	Cyprian Wronka <cwronka@cadence.com>,
	Alexandre Belloni <alexandre.belloni@free-electrons.com>,
	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	Nishanth Menon <nm@ti.com>, Rob Herring <robh+dt@kernel.org>,
	Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Ku
Subject: Re: [RFC 2/5] i3c: Add core I3C infrastructure
Date: Tue, 1 Aug 2017 15:11:44 +0200	[thread overview]
Message-ID: <CAK8P3a0SH4kWcvXK+_iPt9LWuNnpjr+omp+ZpyTLjT=H602n+w@mail.gmail.com> (raw)
In-Reply-To: <20170801142936.5df48702@bbrezillon>

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

  reply	other threads:[~2017-08-01 13:11 UTC|newest]

Thread overview: 91+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-31 16:24 [RFC 0/5] Add I3C subsystem Boris Brezillon
2017-07-31 16:24 ` [RFC 1/5] i2c: Export of_i2c_get_board_info() Boris Brezillon
2017-07-31 16:24 ` [RFC 2/5] i3c: Add core I3C infrastructure Boris Brezillon
2017-07-31 19:17   ` Wolfram Sang
2017-07-31 19:17     ` Wolfram Sang
2017-07-31 20:46     ` Boris Brezillon
2017-07-31 20:46       ` Boris Brezillon
2017-07-31 20:16   ` Arnd Bergmann
2017-07-31 20:16     ` Arnd Bergmann
2017-07-31 21:15     ` Boris Brezillon
2017-07-31 21:15       ` Boris Brezillon
2017-07-31 21:32       ` Peter Rosin
2017-07-31 21:32         ` Peter Rosin
2017-07-31 21:42       ` Wolfram Sang
2017-07-31 21:42         ` Wolfram Sang
2017-08-01 16:47         ` Andrew F. Davis
2017-08-01 16:47           ` Andrew F. Davis
2017-08-01 17:27           ` Wolfram Sang
2017-08-01 17:27             ` Wolfram Sang
2017-08-01 21:47             ` Boris Brezillon
2017-08-01 21:47               ` Boris Brezillon
2017-08-02 10:21               ` Wolfram Sang
2017-08-02 10:21                 ` Wolfram Sang
2017-08-01 12:00       ` Arnd Bergmann
2017-08-01 12:00         ` Arnd Bergmann
2017-08-01 12:29         ` Boris Brezillon
2017-08-01 12:29           ` Boris Brezillon
2017-08-01 13:11           ` Arnd Bergmann [this message]
2017-08-01 13:11             ` Arnd Bergmann
2017-08-01 13:34             ` Boris Brezillon
2017-08-01 13:34               ` Boris Brezillon
2017-08-01 13:58               ` Boris Brezillon
2017-08-01 13:58                 ` Boris Brezillon
2017-08-01 14:22                 ` Arnd Bergmann
2017-08-01 14:22                   ` Arnd Bergmann
2017-08-01 15:14                   ` Boris Brezillon
2017-08-01 15:14                     ` Boris Brezillon
2017-08-01 20:16                     ` Arnd Bergmann
2017-08-01 20:16                       ` Arnd Bergmann
2017-08-01 14:12               ` Wolfram Sang
2017-08-01 14:12                 ` Wolfram Sang
2017-08-01 14:48                 ` Boris Brezillon
2017-08-01 14:48                   ` Boris Brezillon
2017-08-01 15:01                   ` Wolfram Sang
2017-08-01 15:01                     ` Wolfram Sang
2017-08-01 15:20                     ` Boris Brezillon
2017-08-01 15:20                       ` Boris Brezillon
2017-08-03  8:03                       ` Boris Brezillon
2017-08-03  8:03                         ` Boris Brezillon
2017-08-16 21:03                     ` Geert Uytterhoeven
2017-08-16 21:03                       ` Geert Uytterhoeven
2017-08-17  7:48                       ` Boris Brezillon
2017-08-17  7:48                         ` Boris Brezillon
2017-08-01  1:40   ` Greg Kroah-Hartman
2017-08-01  1:40     ` Greg Kroah-Hartman
2017-08-01 10:48     ` Boris Brezillon
2017-08-01 10:48       ` Boris Brezillon
2017-08-01 17:51       ` Greg Kroah-Hartman
2017-08-01 17:51         ` Greg Kroah-Hartman
2017-08-01 21:30         ` Boris Brezillon
2017-08-01 21:30           ` Boris Brezillon
2017-08-02  0:54           ` Greg Kroah-Hartman
2017-08-02  0:54             ` Greg Kroah-Hartman
2017-08-02  2:13           ` Greg Kroah-Hartman
2017-08-02  2:13             ` Greg Kroah-Hartman
2017-12-13 16:20             ` Boris Brezillon
2017-12-13 16:20               ` Boris Brezillon
2017-12-13 16:51               ` Greg Kroah-Hartman
2017-12-13 16:51                 ` Greg Kroah-Hartman
2017-08-17  9:03   ` Linus Walleij
2017-08-17  9:03     ` Linus Walleij
2017-08-17  9:28     ` Boris Brezillon
2017-08-17  9:28       ` Boris Brezillon
2017-07-31 16:24 ` [RFC 3/5] dt-bindings: i3c: Document core bindings Boris Brezillon
2017-07-31 16:24   ` Boris Brezillon
2017-08-09 23:43   ` Rob Herring
2017-08-09 23:43     ` Rob Herring
2017-08-10  8:49     ` Boris Brezillon
2017-08-10  8:49       ` Boris Brezillon
2017-07-31 16:24 ` [RFC 4/5] i3c: master: Add driver for Cadence IP Boris Brezillon
2017-07-31 16:24 ` [RFC 5/5] dt-bindings: i3c: Document Cadence I3C master bindings Boris Brezillon
2017-07-31 19:17 ` [RFC 0/5] Add I3C subsystem Wolfram Sang
2017-07-31 19:17   ` Wolfram Sang
2017-07-31 20:40   ` Boris Brezillon
2017-07-31 20:40     ` Boris Brezillon
2017-07-31 20:47     ` Wolfram Sang
2017-07-31 20:47       ` Wolfram Sang
2017-12-12 19:58   ` Boris Brezillon
2017-12-12 19:58     ` Boris Brezillon
2017-12-12 22:01     ` Wolfram Sang
2017-12-12 22:01       ` Wolfram Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAK8P3a0SH4kWcvXK+_iPt9LWuNnpjr+omp+ZpyTLjT=H602n+w@mail.gmail.com' \
    --to=arnd@arndb.de \
    --cc=adouglas@cadence.com \
    --cc=agolec@cadence.com \
    --cc=alexandre.belloni@free-electrons.com \
    --cc=alicja@cadence.com \
    --cc=bfolta@cadence.com \
    --cc=boris.brezillon@free-electrons.com \
    --cc=corbet@lwn.net \
    --cc=cwronka@cadence.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dkos@cadence.com \
    --cc=galak@codeaurora.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=jank@cadence.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=nm@ti.com \
    --cc=pawel.moll@arm.com \
    --cc=psroka@cadence.com \
    --cc=robh+dt@kernel.org \
    --cc=thomas.petazzoni@free-electrons.com \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.