i3c_master_add_i3c_dev_locked() accept PID, BCR, DCR

Boris Brezillon
2018-12-12T08:21:03Z

Hi Przemek,

On Wed, 12 Dec 2018 06:06:18 +0000
Przemyslaw Gaj <pgaj@cadence.com> wrote:

>
> I measured in my setup with a scope and the additional time to get that information is around 110 us per device and with only one device the bus takes around 550 us for the initialization.
>
> So, for now, we are transferring the same information two times over the bus, during DAA procedure and then manually. Three commands per device GETBCR/DCR/PID need six register accesses to write (in our controller). Don't you think it is more significant overhead? I'm just wondering.

Not really, and I think it's actually more than 3 reg accesses per
device (you also have to clear the interrupts and read the RX FIFO). My
point is, this should only happen once at init (DAA) time and once in a
while if devices are joining the bus afterwards (Hot-Join requests). So,
this is not really a hot path, hence my decision to choose simplicity
over perfs: have a single function for the DAA (where PID, BCR and
DCR are available) and SETDASA (where these have to be retrieved using
GETPID, GETDCR, GETBCR) cases.

Nothing is set in stone, and if one of you comes with a patch that
keep things simple while allowing one to skip the GET{PID,BCR,DCR}
commands, I'll consider merging it. But, to be honest, I don't think
this a priority. We still don't have support for DDR and mastership
handover, which in my opinion are much more valuable than optimizing
the bus-discovery path.

>
>
> >> Let's assume we have a huge overhead caused by the OS (and all the
>
> >> scheduling involved), and make it 100usec.
>
> > And let's make it 500usec with the OS overhead, which is still not
>
> > particularly worrisome in term of boot-time.
>
>
>
> Ok, it can be done in the future.
>
> I was thinking also how to skip this in case of mastership request where device information is received in DEFSLVS command, secondary master can't do DAA.

Okay, so this is actually one reason we'd want to add devices without
triggering transfers on the bus. The thing is, we need more than just
PID, BCR and DCR, and those information are not passed in the DEFSLVS
frame (MXDS, MRL, MWL, HDRCAP). IIRC, we started discussing that with
Arnd, Vitor and you and we listed 2 options:

1/ force the secondary master to take bus ownership after it has
received DEFSLVS so that it can retrieve the missing bits and finally
expose devices to its users
2/ bind partially discovered devices to drivers and retrieve the device
information on-demand (when i3c_device_get_info() is called).

If we go for option #2, we'll need a separate function that does more
than just skipping GETPID, GETDCR and GETBCR. Of course #2 is more
elegant in that it doesn't force a mastership handover until someone
really wants to access a device through the secondary master, but #1 is
definitely simpler to implement.

> Mastership takeover can be faster without need to get BCR/DCR from devices again.

Again, this should only happen once, not every time the master changes,
simply because devices on the bus will stay the same. So I don't think
optimization is a good argument.

Regards,

Boris