Hi Vitor,

On Tue, 27 Nov 2018 11:50:53 +0000
vitor [off-list ref] wrote:

Hi Boris

On 26/11/18 21:33, Boris Brezillon wrote:

quoted

On Mon, 26 Nov 2018 20:11:39 +0000
vitor [off-list ref] 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 think you didn't read my reply carefully. I'm not saying I3C == USB,
I'm just saying that the way you interact with an I3C from a SW PoV is
not at all the same as you would do for an I2C device. Do you deny that?

quoted

quoted

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).
 

quoted

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.

Looks like there's a misunderstanding here. The question is not whether
I3C will replace I2C or USB, of course it's meant to overcome the
limitations of I2C. I'm just pointing out that, if we have to expose
I3C devices, we should look at what other discoverable buses do (PCI,
USB, ...), not what I2C does.

quoted

quoted

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.

There's a difference between a secondary master that waits for its time
to become the currrent master, and a secondary master that provides I3C
device features when it's acting as a slave (sensor, GPIO
controller, ...). So far we focused on supporting the former. If
there's a need for the latter, then we should start thinking about the
slave framework...

quoted

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.

Can you detail a bit more what you have in mind? I don't think we can
do like I2C, simply because we need to expose a valid DCR +
manuf-ID/PID so that other masters can bind the device to the
appropriate driver on their side. Plus, if we're about to expose
generic profiles, we likely don't want each I3C slave controller driver
to do that on its own.

quoted

quoted

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.

If we mix everything in the same subdir, I'd like to have an easy way
to quickly identify those that are slave controllers and those that are
master controllers. For the dual-role thing, maybe we can consider them
as master (ones with advances slave features).

Would you be okay with drivers/i3c/controllers/{designware,dw}/..., so
that you can have all designware drivers (for both slave and master
blocks) in the same dir?

For those that are placed directly under drivers/i3c/controllers/...
(because they only have one .c file), I'd like to keep a standard
prefix.

quoted

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}.

And again, I'm questioning the necessity of per-IP directories at the
root level. I'm not against per-IP directories, as long as they are
classified like other HW blocks: drivers/i3c/{master,slave}/<ip>/...

quoted

quoted

quoted

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.

No it's not vain, it's how we do discuss things in the community. I'm
not saying I'm always right, but I need to understand the problems
you're trying to solve to take a decision, and I don't think you
initially gave all the details I needed to understand your PoV. That's
a bit clearer now, even if I still disagree on a few aspects.

Unfortunately I don't see anyone given their inputs too.

They will come.

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.

Come on! All I've seen so far are complaints on tiny details, it
definitely doesn't prevent you from adding new features.

Regards,

Boris