Re: [RFC PATCH v1] irqchip: add support for SMP irq router

[Sebastian Frias]

Hi Jason,

On 07/06/2016 06:28 PM, Jason Cooper wrote:
> Hi Sebastian,
> 
> On Wed, Jul 06, 2016 at 01:37:21PM +0200, Sebastian Frias wrote:
>> On 07/05/2016 06:16 PM, Jason Cooper wrote:
>>>> Come to think of it, I'm not sure the *name* of the file documenting
>>>> a binding is as important to DT maintainers as the compatible string.
>>>
>>> Correct.  devicetee compatible strings need to be as specific as
>>> possible.  
>>
>> Specific with respect to what thing? To the HW module they are describing
>> (USB, IRQ controller, etc.) or to the chip the HW module it is embedded
>> into?
> 
> The compatible string uniquely identifies an interface between an IP
> block and the software (the devicetree binding).  We use the most
> specific model number or name we can for that IP block when we create
> the binding and the compatible string.

Ok, so we agree that the string identifies the HW block itself, and not the
chip in which it is embedded into.

> 
> If future SoCs come out and the IP block contained within, regardless of
> identifier, is compatible with the existing binding, then we can reuse
> it.  This is what I was trying to show with my little chart quoted
> below.

I understand, is just that the chart was using chip IDs so I was a bit
confused.

> 
> For ethernet and other major blocks it's easy because they have their
> own model numbers and such.  For the smaller blocks, like irqchips, we
> have to use the model number or unique name of the SoC we first found it
> in.

Ok, but I'm not sure I understand the logic, I thought (and it is my
understanding of your previous paragraphs) that the string is supposed to
identify the IP block (or HW module) itself.

That seems more generic and flexible than attaching it some "external"
property, like the name of the chip the HW module happens to be embedded into.

Normally such "small HW blocks" you talk about, because they are not generic
enough, probably require documentation/engineering effort from the SoC
manufacturer, so I would guess that the naming can be decided with the SoC
them.

Why is there a difference on the naming convention for "smaller blocks, 
like irqchips" and other blocks (ethernet, USB, UART, etc.) ?

>> A SoC is composed of several HW modules, some are shared among different
>> manufacturers (i.e.: "generic-xhci"), and some are shared among different
>> product lines of the same manufacturer (i.e.: "sigma,smp,irqrouter").
> 
> Yes, that's why it's critical to be specific.  We want to reuse drivers
> where it makes sense.  We can if the interface is the same.

Ok, so is "sigma,smp,irqrouter" good or not?

> 
>> And the DT for a given chip should describe the collection of HW modules
>> that make up the SoC, regardless of what chip introduced them or what
>> other chip uses it. Why would that be relevant anyway?
> 
> Because the first time we see a new IP block, we can't predict the
> future.  We don't know where it's going to be reused.  We don't know
> when it's going to be changed, or how.  So we mark it as specifically as
> we can based on the information we have when we first encounter it.
> 
> When a new version comes out, we see if the interface is still
> compatible.  If it is, there's nothing to do.  If it changed, we see if
> we can address it by adding a property to the existing binding.  Failing
> that, we create a new compatible string to indicate a new interface.

I see. I think this process is ok for when there's no information, like when
somebody reverse engineer a HW block of a chip of a given line of chips.
In that case I can understand that there's no way to know if previous,
similar or future chips use the same block, nor its name.

Yet, I still fail to see the reason to attach the chip ID to what clearly
are submodules. Especially when one can assume that such submodules can
be shared on different chip IDs.

> 
>> By that reasoning, I also think that drivers/net/ethernet/aurora/nb8800.c
>> should have had a string like "aurora,nb8800,sigma" or something, to
>> specify that it is a NB8800 from Aurora integrated in a Sigma platform.
> 
> The fact that it is in the Sigma dts file tells this already.  Based on
> the above, did the interface change when adding it to the sigma
> platform?  Can the binding be reused?

I think it did not change.
Actually, I think only Sigma uses it, as the driver only has Sigma-specific
initialisations for two Sigma-specific strings.

> 
>> That way it can behave as vanilla NB8800 when the string is "aurora,nb8800"
>> and it can adapt its behaviour to Sigma's platform when a different string
>> (like "aurora,nb8800,sigma") is used in the DT.
> 
> I would have to look at the specifics of "adapt its behaviour" in order
> to determine wether a new compatible is warranted, or maybe just add a
> property to the binding.

Ok.

> 
>> Later if Sigma modified the nb8800 HW, it could be "aurora,nb8800,sigma-v2".
>> The same way, if later Sigma used a different Aurora HW module, the DT
>> would say "aurora,nb2000,sigma" (to signal that another driver is required)
> 
> The compatible string is important, but we try not to overload it.
> There are many flexible ways we can indicate properties or quirks in the
> binding.

Ok.

> 
>> Instead, drivers/net/ethernet/aurora/nb8800.c has "sigma,smp8642-ethernet"
>> string, which ties it to a particular chip, and I don't see how does that
>> convey version about the module or other relevant information; Plus it is
>> confusing if the same module is then embedded on a SMP8756 chip.
> 
> Sorry, it's just understood that the compatible string means they have
> the same binding/interface.

Ok, so same binding/interface = same module, so we could as well name the
compatible string after the module's name, and that, regardless of which
chip it is embedded into, right? Obviously some prefix can be used, and
that's why I had proposed "sigma,smp".

> 
>> Would you mind explaining how does that contrasts with the logic used
>> by DT naming conventions?
> 
> I think you may be looking at DT the wrong way :-P  It's not an
> inventory of components for the end user to look at.  It a list of
> hardware interfaces and how they're attached; for the OS to interpret
> and use.

Exactly, and such hardware interfaces (usually HW IP blocks) are usually
shared by a number of chips, sometimes from different companies, sometimes
from the same company.

That is why I find strange to use a given chip ID to describe some HW IP
submodule.

Best regards,

Sebastian