Re: [PATCH] i3c: master: dw: split dw-i3c-master.c into master and bus specific parts

[vitor]

Hi Boris

On 26/11/18 21:33, Boris Brezillon wrote:
> On Mon, 26 Nov 2018 20:11:39 +0000
> vitor <vitor.soares@xxxxxxxxxxxx> wrote:
>
> Look at the bus discovery mechanism, the notion of DCR (close to the
> concept of USB class), or the fact that each dev has a unique
> manufacturer+PID pair (which resemble the product/vendor ID concept)
> that allows us to easily bind a dev to a driver without requiring a
> static description.


The major feature close to USB is this one and it can be found in others 
protocols (standardization process).

Just to close this topic I3C vs USB, IMO it's wrong to pass the message 
that the I3C is closer to USB than I2C even more because I3C support the 
I2C on the fly.


> > I'm not sure but I think that a controller cannot change between gadget
> > to host in USB in runtime.
> That's called USB OTG. Okay, to be fair, it's not exactly the same, and
> the mastership handover in I3C is probably more complex than what we
> have with USB OTG (I'm not a USB expert, so I might be wrong here).
>
> > Even so, this kind of behavior is more likely
> > to have in an I3C bus.
> Maybe.


Sorry, with the proliferation of sensors I cannot see a multi master 
sensor network based on USB.


> > Sorry for that and don't take me wrong... maybe I should rise this
> > question early but this only came up now when I started splitting and
> > thinking where to put what is for master for slave, what is common and
> > the thing of putting everything of controller in a folder.
> So you have such a dual-role controller?


Yes, we already talked about secondary master support.


> What I call a slave controller would be something that lets you reply to
> SDR/DDR transactions or fill a generic regmap that would be exposed to
> other masters on the bus. This way we could implement generic slave
> drivers in Linux (the same way we have gadget drivers). Anything else
> is likely to be to specific to be exposed as a generic framework.


I would bet to do something like in i2c, we don't need the same level of 
complexity found in USB.


> > Taking the USB as exemple do you prefer a dwc folder on i3c root?
> Hm, not sure I like this idea either. So I see 2 options:
>
> 1/ put all controller drivers (both master and slave ones) in a common
>     directory (drivers/i3c/controllers) as you suggest, and prefix them
>     correctly (i3c-master-<ip>.c, i3c-slave-<ip>.c and i3c-dual-<ip>.c)


I agree with the controller folder but not with prefix. Please check 
what is already in the kernel.


> 2/ place them in separate directories: drivers/i3c/{master,slave,dual}
>
> I'm fine either way.


In this case and taking what is already in the kernel it will be 
drivers/i3c/{master, slave, dwc, other with the same architecture as dwc}.


> > > I'm okay changing it, but I want to understand why the proposed
> > > separation is not good.
> >
> > I already tell you my use case and as I said maybe someone can advise :)
> I think I understand your concerns now, but only because you started to
> mention a few things that were not clearly stated before (at least, I
> didn't understand it this way), like the fact that your controller (and
> probably others too) supports dual-role, or the fact that you plan to
> expose your IP through the PCI bus.


I miss to mention PCI but since the beginning refer the slave and the 
common part.

Splitting the driver is something that soon or later I will have to do. 
If you prefer later I'm ok with that.


I think this discussion is starting to be counterproductive with arguing 
of both parts. Unfortunately I don't see anyone given their inputs too.

To be clear, the subsystem is nice and I working with daily. As I said 
this is something that I dealing now and I'm telling what I think that 
is not correct.