Re: [PATCH 1/2] i3c: Add support for mastership request to I3C subsystem

[Boris Brezillon <bbrezillon@kernel.org>]

On Fri, 14 Dec 2018 13:47:39 +0000
Przemyslaw Gaj <pgaj@cadence.com> wrote:

> >>> @@ -2497,6 +2629,13 @@ int i3c_master_register(struct i3c_master_controller *master,
> >>> 		goto err_del_dev;
> >>> 
> >>> 	/*
> >>> +	 * We can stop here. Devices will be attached after bus takeover.
> >>> +	 * Secondary masters can't do DAA.
> >>> +	 */
> >>> +	if (master->secondary)
> >>> +		return 0;    
> >> 
> >>    See, I don't think we should stop here. We should provide a hook to let
> >>    secondary slaves populate the dev list based on data they might have
> >>    received before that.
> >> 
> >> Do we want to populate list with data received by DEFSLVS?   
> > 
> > I guess we do. I mean, we shouldn't populate the list directly, but
> > we should request bus ownership, and then add I3C devices one by one
> > using i3c_master_add_i3c_dev_locked(). This function will do the rest
> > (query PID, BCR, DCR, ... and then add the device to the list).
> >   
> >> If yes, I can populate list but we are not able to register devices because we don’t have 
> >> PID yet and device drivers are matched by PID.  
> > 
> > Now I understand why you want it to be driven by ENEC(MR). Can't we
> > have a situation where i3c_master_register() is called after it has
> > received both DEFSLV and ENEC(MR). Shouldn't we request bus ownership
> > directly at init time in this case.  
> 
> I think it's possible and I think we should request mastership and finish
> initialization in this case. And what if MR is disabled during initialization?

Then you'd have to delay this part until ENEC(MR) is received, so maybe
it's just easier to let master controller drivers handle that part on
their own.

> Do we want to wait for DEFSLVS or ENEC(MR) event and then finish
> initialization? I just wanted to have generic solution for both cases.

It really depends what we mean by "bus initialization". If we decide
that secondary master bus init is just about initializing the
i3c_master_controller and i3c_bus structs (and maybe enable the
controller and send an HJ event on the bus) and nothing more, then we
should let I3C master controller drivers register I3C dev on their own.

> 
> > 
> > Looks like all of this will be I3C master controller driven though, so
> > maybe it's just better to let master drivers handle all of that on
> > their own.  
> 
> So, I can try adding devices received by DEFSLVS on driver side and
> then call i3c_master_register(). Is that what you meant? I'm sorry if I
> misunderstood something.

I don't know. I was just thinking out loud. Right now, I can't tell
which option is best:

1/ provide a minimal secondary master init routine and let drivers do
   all the hard work
2/ provide an automated procedure to simplify driver's life and take
   the risk of making it too restrictive

If I had to work on that, I'd probably go for option #1 and try to
refine things when we start having more controllers, but I might be
wrong.


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c