Re: [PATCH 2/2] i3c/master: add the mipi-i3c-hci driver

Nicolas Pitre
2020-08-20T18:44:14Z

On Thu, 20 Aug 2020, Boris Brezillon wrote:

> On Thu, 20 Aug 2020 12:47:49 -0400 (EDT)
> Nicolas Pitre <nico@fluxnic.net> wrote:
>
> > On Thu, 20 Aug 2020, Boris Brezillon wrote:
> >
> > > On Thu, 20 Aug 2020 10:08:29 +0200
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > > > + /*
> > > > > + * TODO: Extend the subsystem layer to allow for registering
> > > > > + * new device and provide BCR/DCR/PID at the same time.
> > > >
> > > > Not sure this is needed if you don't use it directly as the core will
> > > > anyway (in its current form) send the relevant CCC to read these
> > > > registers.
> > >
> > > We considered optimizing that in the past but that means making the DAA
> > > and SETDASA registration different. I'm not sure it's worth it to be
> > > honest, PID/DCR/BCR only happens when initializing devices and I
> > > suspect the overhead of querying those DATA twice in case of DAA is
> > > negligible anyway.
> >
> > Wellllll... I know some people who do feel strongly about this
> > particular issue for some reasons.
>
> Mind developing a bit why? Boot-time maybe?

Mind you, I'd prefer for those people to argue their use case themselves
as I'm still not fully convinced myself. But yeah, this has something to
do with latency.

And v2 of the spec goes a step further by making the DAA procedure give
you the PID/DCR/BCR of the winning device before you provide it the
address to be assigned, so you could skip SETDASA/SETNEWDA altogether by
giving it the final address up front. But in order for this to work the
subsystem would have to provide a query method that would go like:
here's some PID/DCR/BCR: please give me the best address to assign this
device... oh and do so within a 150ms delay.

> > So I'd prefer giving them some hope
> > and leave the door open to some i3c_master_add_i3c_dev_and_info()
> > interface. In the end, it's just a matter of pre-filling the info struct
> > and skipping the PID retrieval in i3c_master_getpid_locked() if already
> > available, etc.
>
> I'm definitely not closing the door, but I'd like to understand why this
> is so important to them :-). Anyway, if the changes are not invasive, I
> don't have a good reason to refuse it.

Right. At least now you've been warned this might be coming.

Nicolas