> > Renesas R-Car Gen2 has two I2C IP cores. One can do DMA and automatic
> > transfers to the PMIC, the other has I2C slave functionality. One cannot
> > do I2C_SMBUS_QUICK, the other can. And some more kind of quirks.
> > Sometimes you can mux the pins to GPIO, so you have a third option.
> >
> > This setup is the reason the demux driver exists.
>
> Have you run into scenarios where you dynamically switch between
> the two masters in order to do different things (on one slave, or
> on multiple slaves), or could you always decide on one of them
> at boot time with that particular chip?

My personal use case is debugging. R-Car H2 is great because I can
always pinmux this or that I2C IP core to the same set of pins, and in 2
out of 4 cases even GPIO bitbang on top of that. So, it is great to
compare behaviour, do scopes with the same type of setup, etc...
For that, I do runtime switches, but the slaves are not really under
real usage.

I have absolutely no idea how $customers use it, sadly.

> I think an SoC design we will likely see is an i3c master multiplexed with
> an i2c master to access one bus. The i2c master can then use clock
> stretching and other things that may not work in the i3c master, and it
> may be used in the absence of proper i3c drivers in the OS.

Multiplexed? Well, as soon you want to use I3C features like IBI, this
is not going to work, right? It will not even work with Linux being an
I2C slave itself. Or do you mean running the I3C and I2C controller
simultaneously using the same wires?

> However, that case cannot be handled with the abstraction in the
> proposed i3c framework, which can only deal with multiple i3c
> standard compliant masters. I'm also not sure if it can be added
> to the i2c-demux-pinctrl driver.

The I2C demuxer maps the whole bus to an i2c_adapter. You cannot select
a master per client.

Attachments:

(No filename) (1.86 kB)
signature.asc (849.00 B)
Download all attachments

2018-07-24 15:59:50

by Arnd Bergmann

[permalink] [raw]

Subject: Re: [PATCH v6 00/10] Add the I3C subsystem

On Tue, Jul 24, 2018 at 5:46 PM, Geert Uytterhoeven
<[email protected]> wrote:
> Hi Arnd,
>
> On Tue, Jul 24, 2018 at 5:40 PM Arnd Bergmann <[email protected]> wrote:
>> On Tue, Jul 24, 2018 at 5:15 PM, Geert Uytterhoeven
>> <[email protected]> wrote:
>> > On Tue, Jul 24, 2018 at 5:05 PM Arnd Bergmann <[email protected]> wrote:
>>
>> >> That's not the case I was describing here, I was thinking of what
>> >> Wolfram described with the Renesas SoC that has two i2c masters
>> >> multiplexed through the pinmux layer. I would assume that we
>> >> can still do the same thing in i3c by shutting down the current
>> >> master without a handover, and reprobing everything from scratch.
>> >
>> > The major disadvantage of reprobing is that it may cause visual disturbances
>> > when i2c slaves are involved with e.g. the display pipeline (think HDMI encoders
>> > etc.).
>>
>> Do you mean we should reuse the device pointer and association with
>> the driver even when we switch out the i3c master using the pinmux?
>>
>> Or do you mean we need to be prepared for driving a single
>> slave through multiple masters over the lifetime of that device,
>> but using the i3c master handover protocol?
>> In the second case, how do we decide which master to use
>> for accessing a device for a given request?
>
> I'll have to defer to Wolfram. He's the i2c and muxing expert.

On i2c, we only have the first case, and Wolfram said that it
intentionally does the reprobe to avoid the problems we discussed.
The question is what to do about this if it happens again on i3c.
Peter seemed to think that it was possibly something we might
have to handle, while Boris said that it wouldn't be because it's
not coverered by the i3c spec.

The second case is the one that started the discussion, and
this is where I said I'd prefer to associate each slave with at
most one master at boot time, while the current v6 patch
is prepared for having one slave be accessed alternatingly
by multiple masters using the master handover, though so
far nobody has been able to describe exactly how we'd pick
which master is active at what point, or what specific scenario
would require it.

Arnd

2018-07-24 16:05:43

[permalink] [raw]

Subject: Re: [PATCH v6 01/10] i3c: Add core I3C infrastructure

Hi Boris, Vitor,

On 9/6/18, 6:07 PM, "Boris Brezillon" <[email protected]> wrote:

EXTERNAL MAIL

On Thu, 6 Sep 2018 16:17:58 +0100
vitor <[email protected]> wrote:

> Hi Boris,
>
>
> On 06-09-2018 15:14, Boris Brezillon wrote:
> > On Thu, 6 Sep 2018 14:50:03 +0100
> > vitor <[email protected]> wrote:
> >
> >> Hi,
> >>
> >>
> >> On 06-09-2018 14:20, Boris Brezillon wrote:
> >>> On Thu, 6 Sep 2018 15:14:37 +0200
> >>> Boris Brezillon <[email protected]> wrote:
> >>>
> >>>> On Thu, 6 Sep 2018 14:59:46 +0200
> >>>> Arnd Bergmann <[email protected]> wrote:
> >>>>
> >>>>> On Thu, Sep 6, 2018 at 2:43 PM Przemyslaw Gaj <[email protected]> wrote:
> >>>>>> Hi Boris, Vitor,
> >>>>>>
> >>>>>> This repository does not contain full kernel sources, but it should be enough to discuss mastership request feature.
> >>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_przemekgaj_i3c-2Dlinux_commit_d54fe68a9d3e573c0c454a2c6f1afafc20142ec5&d=DwICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=qVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=Q9DWw3KGmshGw0f5QTiffbpbESyUlPx6KmASuDBtX9Y&s=HHE_y1kyMszJvP_tSP9JkDlPYxDywBeHwkMGgCR11uI&e=
> >>>>>>
> >>>>>> Please keep in mind that this is initial version, but mastership works correctly. I added one property to DT to reflect relationship between masters.
> >>>>>> It is possible to request mastership on demand (using sysfs. Useful in case when Linux machine is equipped only with secondary master controller) or automatically change operation mode when device driver wants to read/write something from/to device.
> >>>>>>
> >>>>>> I'm sure I will have to rework something because this was implemented on sources from PATCH v4. I saw that Boris released v7 yesterday :)
> >>>>> Can you explain the reason for having a user space interface and DT property?
> >>>>> I thought we had concluded earlier that we wouldn't need that, but it's possible
> >>>>> that I missed something in the discussion since then.
> >>>> I don't think the sysfs knob is needed, this being said, after thinking
> >>>> a bit more about mastership handover and the secondary master case, I
> >>>> think we have something important to solve.
> >>>>
> >>>> When a master is not in control of the bus, it gets informed of devices
> >>>> present on the bus by monitoring DAA or DEFSLVS broadcast events. That
> >>>> means the secondary master should populate the bus with I3C/I2C devices
> >>>> on such events, but that's not enough, because DEFSLVS/DAA do not
> >>>> provide all device info.Some of them (like read/write/ibi limitations)
> >>>> require extra CCC commands, and, to send those CCC commands, the
> >>>> secondary master must claim the bus. We could add a case where we
> >>>> declare devices as partially discovered until the master acquires
> >>>> ownership of the bus, but that means part of the data returned by
> >>>> i3c_device_get_info() will be inaccurate, which might have an impact on
> >>>> some i3c driver ->probe() functions.
> >>> Hm, one possible solution would be to register partially discovered
> >>> devices to the device model and let i3c_device_get_info() claim the bus
> >>> and request missing data when needed. This way, if the driver needs to
> >>> call i3c_device_get_info() in its probe path, it should work just fine.
> >> Why don't use the i3c_master_add_i3c_dev_locked that job? It create,
> >> attach and retrieve the device info.
> > When will you call i3c_master_add_i3c_dev_locked()? After receiving a
> > DEFSLVS interrupt/event? When that happens you're not in control of the
> > bus, which means you'll have to force bus ownership handover. Is this
> > really what we want?
> >
> > These are not rhetorical questions, I'm really asking for your opinion
> > here.
> What I understand from last discussion was that every time that device
> need to do something (private messages) on the bus and don't have the
> bus control it should force ownership handover.
> If my understanding is correct this can be also applied to CCC commands.

Sure, but the question is more, when do we want to do that?

>
> >
> >> This can be triggered after the secondary master receive ENEC MR until
> >> them is keep in the driver memory.
> > But that means no-one will actually trigger a mastership request,
> > because devices won't be registered until all info are available. If we
> > take this path, we should have a way to explicitly trigger a mastership
> > request (sysfs knob or any other means).
> By the current flow that we have now, we enable the IBI events at the
> end of .do_daa. At this time the main master bus is initialized with all
> devices.
>
> From the point of view of secondary master, it first participate on DAA
> process, next it send its info (Main master do
> i3c_master_retrieve_dev_info()) and them receive DEFSLVS. In the stage
> it cannot request the bus mastership it need the MR enable.

You mean the broadcast ENEC(MR) event, right? Then yes, this one is
probably missing (or maybe I added it in the master controller driver,
I don't remember).

>
> So, what I would suggest is to keep DEFSLVS data in secondary master
> driver memory . When secondary master receive the MR enable, it can get
> the bus ownership and do i3c_master_add_i3c_dev_locked() and each device
> in DEFSLVS command. After this step delegate the bus ownership to the
> main master.

That's an option, indeed.

>
> What do you think about this?

Sounds like a good start. If MR is rejected by the master, we will just
keep all devices in an unregistered state. We can also add a sysfs
entry to manually re-trigger this operation in case the initial one
failed.

I found that version. I added also mastership request after ENEC(MR) event to run
tests and verify if everything works correctly. It looked quite simple. Secondary master
does not give control back automatically after bus initialization and master doesn't
yet request mastership when device driver wants to use the bus / transfer data.

This can be the base for further works.
https://github.com/przemekgaj/i3c-linux/tree/mastership_takeover

Regards,
Przemek

2018-09-10 16:18:44

by Vitor Soares

[permalink] [raw]

Subject: Re: [PATCH v6 01/10] i3c: Add core I3C infrastructure

Hi,

On 06-09-2018 17:17, Przemyslaw Gaj wrote:
>
> On 9/6/18, 6:07 PM, "Boris Brezillon" <[email protected]> wrote:
>
> EXTERNAL MAIL
>
>
> On Thu, 6 Sep 2018 16:17:58 +0100
> vitor <[email protected]> wrote:
>
> > Hi Boris,
> >
> >
> > On 06-09-2018 15:14, Boris Brezillon wrote:
> > > On Thu, 6 Sep 2018 14:50:03 +0100
> > > vitor <[email protected]> wrote:
> > >
> > >> Hi,
> > >>
> > >>
> > >> On 06-09-2018 14:20, Boris Brezillon wrote:
> > >>> On Thu, 6 Sep 2018 15:14:37 +0200
> > >>> Boris Brezillon <[email protected]> wrote:
> > >>>
> > >>>> On Thu, 6 Sep 2018 14:59:46 +0200
> > >>>> Arnd Bergmann <[email protected]> wrote:
> > >>>>
> > >>>>> On Thu, Sep 6, 2018 at 2:43 PM Przemyslaw Gaj <[email protected]> wrote:
> > >>>>>> Hi Boris, Vitor,
> > >>>>>>
> > >>>>>> This repository does not contain full kernel sources, but it should be enough to discuss mastership request feature.
> > >>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_przemekgaj_i3c-2Dlinux_commit_d54fe68a9d3e573c0c454a2c6f1afafc20142ec5&d=DwICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=qVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=Q9DWw3KGmshGw0f5QTiffbpbESyUlPx6KmASuDBtX9Y&s=HHE_y1kyMszJvP_tSP9JkDlPYxDywBeHwkMGgCR11uI&e=
> > >>>>>>
> > >>>>>> Please keep in mind that this is initial version, but mastership works correctly. I added one property to DT to reflect relationship between masters.
> > >>>>>> It is possible to request mastership on demand (using sysfs. Useful in case when Linux machine is equipped only with secondary master controller) or automatically change operation mode when device driver wants to read/write something from/to device.
> > >>>>>>
> > >>>>>> I'm sure I will have to rework something because this was implemented on sources from PATCH v4. I saw that Boris released v7 yesterday :)
> > >>>>> Can you explain the reason for having a user space interface and DT property?
> > >>>>> I thought we had concluded earlier that we wouldn't need that, but it's possible
> > >>>>> that I missed something in the discussion since then.
> > >>>> I don't think the sysfs knob is needed, this being said, after thinking
> > >>>> a bit more about mastership handover and the secondary master case, I
> > >>>> think we have something important to solve.
> > >>>>
> > >>>> When a master is not in control of the bus, it gets informed of devices
> > >>>> present on the bus by monitoring DAA or DEFSLVS broadcast events. That
> > >>>> means the secondary master should populate the bus with I3C/I2C devices
> > >>>> on such events, but that's not enough, because DEFSLVS/DAA do not
> > >>>> provide all device info.Some of them (like read/write/ibi limitations)
> > >>>> require extra CCC commands, and, to send those CCC commands, the
> > >>>> secondary master must claim the bus. We could add a case where we
> > >>>> declare devices as partially discovered until the master acquires
> > >>>> ownership of the bus, but that means part of the data returned by
> > >>>> i3c_device_get_info() will be inaccurate, which might have an impact on
> > >>>> some i3c driver ->probe() functions.
> > >>> Hm, one possible solution would be to register partially discovered
> > >>> devices to the device model and let i3c_device_get_info() claim the bus
> > >>> and request missing data when needed. This way, if the driver needs to
> > >>> call i3c_device_get_info() in its probe path, it should work just fine.
> > >> Why don't use the i3c_master_add_i3c_dev_locked that job? It create,
> > >> attach and retrieve the device info.
> > > When will you call i3c_master_add_i3c_dev_locked()? After receiving a
> > > DEFSLVS interrupt/event? When that happens you're not in control of the
> > > bus, which means you'll have to force bus ownership handover. Is this
> > > really what we want?
> > >
> > > These are not rhetorical questions, I'm really asking for your opinion
> > > here.
> > What I understand from last discussion was that every time that device
> > need to do something (private messages) on the bus and don't have the
> > bus control it should force ownership handover.
> > If my understanding is correct this can be also applied to CCC commands.
>
> Sure, but the question is more, when do we want to do that?
>
> >
> > >
> > >> This can be triggered after the secondary master receive ENEC MR until
> > >> them is keep in the driver memory.
> > > But that means no-one will actually trigger a mastership request,
> > > because devices won't be registered until all info are available. If we
> > > take this path, we should have a way to explicitly trigger a mastership
> > > request (sysfs knob or any other means).
> > By the current flow that we have now, we enable the IBI events at the
> > end of .do_daa. At this time the main master bus is initialized with all
> > devices.
> >
> > From the point of view of secondary master, it first participate on DAA
> > process, next it send its info (Main master do
> > i3c_master_retrieve_dev_info()) and them receive DEFSLVS. In the stage
> > it cannot request the bus mastership it need the MR enable.
>
> You mean the broadcast ENEC(MR) event, right? Then yes, this one is
> probably missing (or maybe I added it in the master controller driver,
> I don't remember).
>
> >
> > So, what I would suggest is to keep DEFSLVS data in secondary master
> > driver memory . When secondary master receive the MR enable, it can get
> > the bus ownership and do i3c_master_add_i3c_dev_locked() and each device
> > in DEFSLVS command. After this step delegate the bus ownership to the
> > main master.
>
> That's an option, indeed.
>
> >
> > What do you think about this?
>
> Sounds like a good start. If MR is rejected by the master, we will just
> keep all devices in an unregistered state. We can also add a sysfs
> entry to manually re-trigger this operation in case the initial one
> failed.
>
> This everything sounds like my previous version. Enec event was missing,
> everything was triggered manually from sysfs.
>
Does make sense to do the ->bus_init() only after receive the first
ENEC-MR event? The bus initialization assume that we already know the
I2C devices present on the line (bus->mode) and i3c_master_set_info only
have meaning if the device will act as a master otherwise it can act as
"slave".

Best regards,
Vitor Soares