Message-ID: <a31dc625-56bf-7a59-d9a3-2841360b864e@axentia.se>
Date:   Thu, 12 Jul 2018 06:41:15 +0200
From:   Peter Rosin <peda@...ntia.se>
To:     Boris Brezillon <boris.brezillon@...tlin.com>,
        Arnd Bergmann <arnd@...db.de>
Cc:     Wolfram Sang <wsa@...-dreams.de>, linux-i2c@...r.kernel.org,
        Jonathan Corbet <corbet@....net>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Przemyslaw Sroka <psroka@...ence.com>,
        Arkadiusz Golec <agolec@...ence.com>,
        Alan Douglas <adouglas@...ence.com>,
        Bartosz Folta <bfolta@...ence.com>,
        Damian Kos <dkos@...ence.com>,
        Alicja Jurasik-Urbaniak <alicja@...ence.com>,
        Cyprian Wronka <cwronka@...ence.com>,
        Suresh Punnoose <sureshp@...ence.com>,
        Rafal Ciepiela <rafalc@...ence.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Nishanth Menon <nm@...com>, Rob Herring <robh+dt@...nel.org>,
        Pawel Moll <pawel.moll@....com>,
        Mark Rutland <mark.rutland@....com>,
        Ian Campbell <ijc+devicetree@...lion.org.uk>,
        Kumar Gala <galak@...eaurora.org>,
        DTML <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Vitor Soares <Vitor.Soares@...opsys.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Xiang Lin <Xiang.Lin@...aptics.com>, linux-gpio@...r.kernel.org
Subject: Re: [PATCH v4 01/10] i3c: Add core I3C infrastructure

[tried to send something like this yesterday, but it appears to have been
lost, sorry for any duplicate]

On 2018-07-11 19:12, Boris Brezillon wrote:
> On Wed, 11 Jul 2018 17:39:56 +0200
> Arnd Bergmann <arnd@...db.de> wrote:
> 
>> On Wed, Jul 11, 2018 at 4:41 PM, Boris Brezillon
>> <boris.brezillon@...tlin.com> wrote:
>>> On Wed, 11 Jul 2018 16:01:56 +0200 Arnd Bergmann <arnd@...db.de> wrote:  
>>>>> - 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 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
>>>>>   like that.
>>>>>   The other benefit of separating the bus and master concepts is that
>>>>>   master devices appear under the bus directory in sysfs.  
>>>>
>>>> I'm not following here at all, sorry for missing prior discussion if this
>>>> was already explained. What is the "multiple master" case? Do you
>>>> mean multiple devices that are controlled by Linux and that each talk
>>>> to other devices on the same bus, multiple operating systems that
>>>> have talk to are able to own the bus with the kernel being one of
>>>> them, a controller that controls multiple independent buses,
>>>> or something else?  
>>>
>>> I mean several masters connected to the same bus and all exposed to the
>>> same Linux instance. In this case, the question is, should we have X
>>> I3C buses exposed (X being the number of masters) or should we only
>>> have one?
>>>
>>> Having a bus represented as a separate object allows us to switch to
>>> the "one bus : X masters" representation if we need too.  
>> ...
>>>>
>>>> This feels a bit odd: so you have bus_type that can contain devices
>>>> of three (?) different device types: i3c_device_type, i3c_master_type
>>>> and i3c_busdev_type.
>>>>
>>>> Generally speaking, we don't have a lot of subsystems that even
>>>> use device_types. I assume that the i3c_device_type for a
>>>> device that corresponds to an endpoint on the bus, but I'm
>>>> still confused about the other two, and why they are part of
>>>> the same bus_type.  
>>>
>>> i3c_busdev is just a virtual device representing the bus itself.
>>> i3c_master is representing the I3C master driving the bus. The reason
>>> for having a different type here is to avoid attaching this device to a
>>> driver but still being able to see the master controller as a device on
>>> the bus. And finally, i3c_device are all remote devices that can be
>>> accessed through a given i3c_master.
>>>
>>> This all comes from the design choice I made to represent the bus as a
>>> separate object in order to be able to share it between different
>>> master controllers exposed through the same Linux instance. Since
>>> master controllers are also remote devices for other controllers, we
>>> need to represent them.  
>>
>> Ok, so I think this is the most important question to resolve: do we
>> actually need to control multiple masters on a single bus from one OS
>> or not?
>>
>> The problem that I see is that it breaks the tree abstraction that
>> we use in the dtb interface, in the driver model and in sysfs.
>> If we need to deal with a hardware bus structure like
>>
>>               cpu
>>              /   \
>>             /     \
>>        platdev   platdev
>>            |        |
>>      i3c-master   i3c-master
>>             \      /
>>              \    /
>>             i3c-bus
>>              /    \
>>          device   device
>>
>> then that abstraction no longer holds. Clearly you could build
>> a system like that, and if we have to support it, the i3c infrastructure
>> should be prepared for it, since we wouldn't be able to retrofit
>> it later.
> 
> Exactly. For the DT representation I thought we could have the primary
> master hold the device nodes, and then have secondary masters reference
> the main master with a phandle (i3c-bus = <&main_i3c_master>;). For the
> sysfs representation, it would be the same. Only one of the master
> would create the i3c_bus object and the other masters would just
> reference it.
> 
>>
>> What would be the point of building such a system though?
> 
> This, I don't know. But as you said, if we go for a "one bus per
> master" representation, going back will be difficult.
> 
>> Is this for performance, failover, or something else?
> 
> No, I don't think so, especially since the mastership handover
> operation is not free. So keeping the same master in control is
> probably better in term of perfs.
> 
> One case I can think of is when the primary master does not have enough
> resources to address all devices on the bus, and let the secondary
> master handle all transactions targeting those devices.
> 
>> IOW, what feature would we lose if we were to declare that
>> setup above invalid (and ensure you cannot represent it in DT)?
> 
> That's exactly the sort of discussion I wanted to trigger. Maybe we
> shouldn't care and expose this use case as if it was X different I3C
> buses (with all devices present on the bus being exposed X times to the
> system).

For I2C, this multiple masters for one bus case was retrofitted in
the i2c-demux-pinctrl driver. It's a huge kludge with a number of
undesirable quirks. I don't know if the circumstances for adding
this I2C driver also applies for I3C, but it might be an argument
in favor of the proposed extra bus object...

Cheers,
Peter

Powered by blists - more mailing lists