linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Boris Brezillon <boris.brezillon@bootlin.com>
Cc: Peter Rosin <peda@axentia.se>, Wolfram Sang <wsa@the-dreams.de>,
	linux-i2c@vger.kernel.org, Jonathan Corbet <corbet@lwn.net>,
	"open list:DOCUMENTATION" <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>,
	Cyprian Wronka <cwronka@cadence.com>,
	Suresh Punnoose <sureshp@cadence.com>,
	Rafal Ciepiela <rafalc@cadence.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.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>,
	DTML <devicetree@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Vitor Soares <Vitor.Soares@synopsys.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Xiang Lin <Xiang.Lin@synaptics.com>,
	linux-gpio@vger.kernel.org, Sekhar Nori <nsekhar@ti.com>,
	Przemyslaw Gaj <pgaj@cadence.com>
Subject: Re: [PATCH v6 00/10] Add the I3C subsystem
Date: Tue, 24 Jul 2018 16:03:38 +0200	[thread overview]
Message-ID: <CAK8P3a14_6uf9VLYU0YnmFp0Q=DZBKc0cAesBh0opanGDZMFRA@mail.gmail.com> (raw)
In-Reply-To: <20180720151751.242d4809@bbrezillon>

On Fri, Jul 20, 2018 at 3:17 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> On Fri, 20 Jul 2018 13:28:10 +0200 Arnd Bergmann <arnd@arndb.de> wrote:
>
>> On Fri, Jul 20, 2018 at 1:13 PM, Peter Rosin <peda@axentia.se> wrote:
>> > On 2018-07-20 12:57, Arnd Bergmann wrote:
>> >> * What I understand from reading i2c-demux-pinctrl.c, a slave device
>> >>   will only ever be observable from one master at a time, when you
>> >>   switch over, all children get removed on one master and added to
>> >>   the other one, to be probed again by their respective drivers.
>> >>   I can see this as a useful feature on i3c as well, in particular to
>> >>   deal with the situation where we have i2c slaves connected to a
>> >>   pinmux that can switch them between an i3c master and an
>> >>   i2c-only master (possibly a gpio based one). That particular use
>> >>   case however doesn't seem to fix well in the current code, which
>> >>   is structure around i3c buses.
>> >
>> > It's pretty easy to come up with examples where this reprobing is
>> > not desirable at all. E.g. if one of the involved I2C devices is
>> > a HDMI encoder (I have a TDA19988 here) sitting in the middle of the
>> > graphics pipeline. Blink-blink on the screen because some *other*
>> > unrelated device needed to be accessed by an alternative master. Not
>> > pretty.
>>
>> Agreed, we definitely don't want to reprobe all devices during normal
>> operation for i3c master handover.
>>
>
> Re-probing would not happen, no matter the solution we choose. It's
> that, in one case, you would have X virtual/linux devices representing
> the same physical device and in the other case, you would just have
> one, and everytime a transfer is requested by the driver, the core
> would pick the appropriate master to do it (most likely the one in
> control of the bus at that time)

I think this is one of the cases I'd want to avoid: controlling multiple
masters that are active at the same time without going through
the handover.

If we have an actual pinmux between two masters and only one
of them can even see the bus, I think we should go through a
complete remove/probe cycle the way that the i2c-demux-pinctrl
does today. If OTOH we a primary/secondary master pair with
handover capability, I would prefer to not see one slave on
both devices at the same time, or (ideally) only use one of the
two masters and disable the other one completely.

>> If we find a case in which it is needed, we could still deal with it
>> like this:
>> - enumerate all slaves connected to the bus for each of the
>>   two masters
>
> That's what will happen if you don't share the same bus representation.

To clarify: I meant list them in the DT representation, not enumerate
them in Linux during boot. Sorry for using a misleading description here.

>> - mark each slave as status="enabled" in at most one of the
>>   buses, and as disabled everywhere else
>
> We shouldn't need to do that. We can just let the driver check whether
> the master provides the necessary capabilities to efficiently
> communicate with the device, and if it does not just return -ENOTSUPP
> in the ->probe() function. This way you'll have a device, but not
> driver controlling it on one bus, and on the other bus, you'll have
> another device (which points to the same physical device) this time
> with a driver attached to it.

I'd still hope that we can completely avoid that case and never
have the case where one physical device has two live
representations in the kernel. It /could/ still be done of course,
but would not always do the right thing, depending on the
type of device (a temperature sensor could just be probed
twice without problems, a network device probably cannot)

      Arnd

  reply	other threads:[~2018-07-24 14:03 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-19 15:29 [PATCH v6 00/10] Add the I3C subsystem Boris Brezillon
2018-07-19 15:29 ` [PATCH v6 01/10] i3c: Add core I3C infrastructure Boris Brezillon
2018-08-03 21:38   ` mshettel
2018-08-04  5:33     ` Boris Brezillon
2018-08-22 16:43   ` vitor
2018-08-24 12:39     ` Boris Brezillon
2018-08-24 17:52       ` vitor
2018-08-24 18:16         ` Boris Brezillon
2018-08-28 11:50           ` vitor
2018-08-28 12:02             ` Boris Brezillon
2018-08-28 12:55               ` Przemyslaw Gaj
2018-08-28 13:01                 ` Boris Brezillon
2018-08-29  7:41                   ` Przemyslaw Gaj
2018-08-28 13:03                 ` Boris Brezillon
2018-08-30 13:57                 ` vitor
2018-08-30 19:00                   ` Przemyslaw Gaj
2018-09-03  9:33                     ` vitor
2018-09-04 11:03                       ` Przemyslaw Gaj
2018-09-06 12:43                       ` Przemyslaw Gaj
2018-09-06 12:59                         ` Arnd Bergmann
2018-09-06 13:14                           ` Boris Brezillon
2018-09-06 13:20                             ` Boris Brezillon
2018-09-06 13:45                               ` Arnd Bergmann
2018-09-06 13:50                               ` vitor
2018-09-06 14:14                                 ` Boris Brezillon
2018-09-06 15:17                                   ` vitor
2018-09-06 16:06                                     ` Boris Brezillon
2018-09-06 16:17                                       ` Przemyslaw Gaj
2018-09-10 16:16                                         ` vitor
2018-09-07  7:51                                       ` Przemyslaw Gaj
2018-09-06 13:47                             ` Przemyslaw Gaj
2018-09-06 14:09                               ` Boris Brezillon
2018-09-06 14:20                                 ` Przemyslaw Gaj
2018-07-19 15:29 ` [PATCH v6 02/10] docs: driver-api: Add I3C documentation Boris Brezillon
2018-07-19 15:29 ` [PATCH v6 03/10] i3c: Add sysfs ABI spec Boris Brezillon
2018-07-19 15:29 ` [PATCH v6 04/10] dt-bindings: i3c: Document core bindings Boris Brezillon
2018-07-19 15:29 ` [PATCH v6 05/10] dt-bindings: i3c: Add macros to help fill I3C/I2C device's reg property Boris Brezillon
2018-07-19 15:29 ` [PATCH v6 06/10] MAINTAINERS: Add myself as the I3C subsystem maintainer Boris Brezillon
2018-07-19 15:29 ` [PATCH v6 07/10] i3c: master: Add driver for Cadence IP Boris Brezillon
2018-07-19 15:29 ` [PATCH v6 08/10] dt-bindings: i3c: Document Cadence I3C master bindings Boris Brezillon
2018-07-19 15:29 ` [PATCH v6 09/10] gpio: Add a driver for Cadence I3C GPIO expander Boris Brezillon
2018-07-19 15:29 ` [PATCH v6 10/10] dt-bindings: gpio: Add bindings for Cadence I3C gpio expander Boris Brezillon
2018-07-20  8:52 ` [PATCH v6 00/10] Add the I3C subsystem Arnd Bergmann
2018-07-20  9:57   ` Peter Rosin
2018-07-20 10:05     ` Boris Brezillon
2018-07-20 10:39       ` Peter Rosin
2018-07-20 10:12     ` Wolfram Sang
2018-07-20 10:57       ` Arnd Bergmann
2018-07-20 11:05         ` Wolfram Sang
2018-07-20 11:13         ` Peter Rosin
2018-07-20 11:28           ` Arnd Bergmann
2018-07-20 13:16             ` Peter Rosin
2018-07-20 15:41               ` Wolfram Sang
2018-07-24 14:14                 ` Arnd Bergmann
2018-07-24 15:57                   ` Wolfram Sang
2018-07-24 16:04                     ` Arnd Bergmann
2018-07-24 20:22                       ` Wolfram Sang
2018-07-24 16:07                     ` Boris Brezillon
2018-07-20 13:17             ` Boris Brezillon
2018-07-24 14:03               ` Arnd Bergmann [this message]
2018-07-24 14:28                 ` Boris Brezillon
2018-07-24 15:05                   ` Arnd Bergmann
2018-07-24 15:15                     ` Geert Uytterhoeven
2018-07-24 15:40                       ` Arnd Bergmann
2018-07-24 15:46                         ` Geert Uytterhoeven
2018-07-24 15:58                           ` Arnd Bergmann
2018-07-24 16:14                             ` Boris Brezillon
2018-07-24 16:25                               ` Arnd Bergmann
2018-07-24 16:54                                 ` Boris Brezillon
2018-07-24 20:21                                   ` Arnd Bergmann
2018-07-24 16:04                       ` 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='CAK8P3a14_6uf9VLYU0YnmFp0Q=DZBKc0cAesBh0opanGDZMFRA@mail.gmail.com' \
    --to=arnd@arndb.de \
    --cc=Vitor.Soares@synopsys.com \
    --cc=Xiang.Lin@synaptics.com \
    --cc=adouglas@cadence.com \
    --cc=agolec@cadence.com \
    --cc=alicja@cadence.com \
    --cc=bfolta@cadence.com \
    --cc=boris.brezillon@bootlin.com \
    --cc=corbet@lwn.net \
    --cc=cwronka@cadence.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dkos@cadence.com \
    --cc=galak@codeaurora.org \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=linus.walleij@linaro.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-gpio@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=nsekhar@ti.com \
    --cc=pawel.moll@arm.com \
    --cc=peda@axentia.se \
    --cc=pgaj@cadence.com \
    --cc=psroka@cadence.com \
    --cc=rafalc@cadence.com \
    --cc=robh+dt@kernel.org \
    --cc=sureshp@cadence.com \
    --cc=thomas.petazzoni@bootlin.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).