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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Boris,


On 06-09-2018 15:14, Boris Brezillon wrote:
On Thu, 6 Sep 2018 14:50:03 +0100
vitor <Vitor.Soares@xxxxxxxxxxxx> wrote:

Hi,


On 06-09-2018 14:20, Boris Brezillon wrote:
On Thu, 6 Sep 2018 15:14:37 +0200
Boris Brezillon <boris.brezillon@xxxxxxxxxxx> wrote:
On Thu, 6 Sep 2018 14:59:46 +0200
Arnd Bergmann <arnd@xxxxxxxx> wrote:
On Thu, Sep 6, 2018 at 2:43 PM Przemyslaw Gaj <pgaj@xxxxxxxxxxx> 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.


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.

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.

What do you think about this?


Best regards,
Vitor Soares





[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux