Re: [PATCH 1/2] dt-bindings: arm64: marvell: add solidrun cn9130 clearfog boards

[Josua Mayer]

Am 27.03.24 um 11:19 schrieb Krzysztof Kozlowski:
> On 26/03/2024 20:26, Josua Mayer wrote:
>> Am 26.03.24 um 07:41 schrieb Krzysztof Kozlowski:
>>> On 25/03/2024 21:12, Josua Mayer wrote:
>>>> Am 25.03.24 um 20:34 schrieb Krzysztof Kozlowski:
>>>>> On 22/03/2024 11:08, Josua Mayer wrote:
>>>>>> Am 21.03.24 um 22:47 schrieb Josua Mayer:
>>>>>>> Add bindings for SolidRun Clearfog boards, using a new SoM based on
>>>>>>> CN9130 SoC.
>>>>>>> The carrier boards are identical to the older Armada 388 based Clearfog
>>>>>>> boards. For consistency the carrier part of compatible strings are
>>>>>>> copied, including the established "-a1" suffix.
>>>>>>>
>>>>>>> Signed-off-by: Josua Mayer <josua@xxxxxxxxxxxxx>
>>>>>>> ---
>>>>>>>  .../devicetree/bindings/arm/marvell/armada-7k-8k.yaml        | 12 ++++++++++++
>>>>>>>  1 file changed, 12 insertions(+)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
>>>>>>> index 16d2e132d3d1..36bdfd1bedd9 100644
>>>>>>> --- a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
>>>>>>> @@ -82,4 +82,16 @@ properties:
>>>>>>>            - const: marvell,armada-ap807-quad
>>>>>>>            - const: marvell,armada-ap807
>>>>>>>  
>>>>>>> +      - description:
>>>>>>> +          SolidRun CN9130 clearfog family single-board computers
>>>>>>> +        items:
>>>>>>> +          - enum:
>>>>>>> +              - solidrun,clearfog-base-a1
>>>>>>> +              - solidrun,clearfog-pro-a1
>>>>>>> +          - const: solidrun,clearfog-a1
>>>>>>> +          - const: solidrun,cn9130-sr-som
>>>>>>> +          - const: marvell,cn9130
>>>>>>> +          - const: marvell,armada-ap807-quad
>>>>>>> +          - const: marvell,armada-ap807
>>>>>>> +
>>>>>>>  additionalProperties: true
>>>>>> Before merging I would like some feedback about adding
>>>>>> another product later, to ensure the compatibles above
>>>>>> are adequate? In particular:
>>>>>> - sequence of soc, cp, carrier compatibles
>>>>>> - name of som compatible
>>>>>>
>>>>>> Draft for future bindings:
>>>>>>       - description:
>>>>>>           SolidRun CN9130 SoM based single-board computers
>>>>>>           with 1 external CP on the Carrier.
>>>>>>         items:
>>>>>>           - enum:
>>>>>>               - solidrun,cn9131-solidwan
>>>>>>           - const: marvell,cn9131
>>>>>>           - const: solidrun,cn9130-sr-som
>>>>> This does not look correct. cn9131 is not compatible with your som.
>>>> This is partially my question.
>>>> I considered changing the som to "cn913x-sr-som".
>>>>
>>>> The SoM itself is always 9130, it contains the base SoC
>>>> with 1x AP and 1x CP in a single chip.
>>>> 9131 and 9132 <happen> on the carrier boards.
>>> No wildcards, but if the SoM name is 9130 then use 9130.
>>> The problem is that you use cn9130 SoC as fallback.
>>>
>>>>>>           - const: marvell,cn9130
>>>>> SoCs are compatible only in some cases, e.g. one is a subset of another
>>>>> like stripped out of modem. Are you sure this is your case?
>>>> This is more complex, CN9131 and CN9132 are not single SoCs.
>>>> A "9132" is instantiated by connecting two southbridge chips
>>>> via a Marvell defined bus, each providing additional IO
>>>> such as network, i2c, gpio.
>>>>
>>>> Note that even the first, "9130", while a single chip, contains two dies:
>>>> An "AP" (Application Processor I assume) with very limited IO (1xsdio, 1xi2c),
>>>> and a "CP" (Communication Processor I assume) with lots of IO.
>>>> This CP as far as I know today is identical to the southbridges
>>>> mentioned above.
>>> OK, but how does it affect compatibility between them? Which parts are
>>> the same? Or how much is shared?
>> 9130, 9131, 9132 belong together.
> I don't understand what it means.
>
>> 9130 is single chip including two dies: AP, CP.
>> The CP is available as an individual chip,
>> up to two can be connected to one 9130.
> And? How does it help me to decide? What is 9131 and 9132?
>
>> What does this mean for compatibility?
>> Which compatibility specifically?
>> Is there a definition we can refer to?
> Devicetree spec.
Let me fetch it for future reference:
https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.4
> The compatible property value consists of one or more strings that define the specific programming model for
> the device. This list of strings should be used by a client program for device driver selection. The property
> value consists of a concatenated list of null terminated strings, from most specific to most general. They allow
> a device to express its compatibility with a family of similar devices, potentially allowing a single device driver
> to match against several devices.
>
> The recommended format is "manufacturer,model", where manufacturer is a string describing the name
> of the manufacturer (such as a stock ticker symbol), and model specifies the model number.
>
> The compatible string should consist only of lowercase letters, digits and dashes, and should start with a letter.
> A single comma is typically only used following a vendor prefix. Underscores should not be used.
>
> Example:
> compatible = "fsl,mpc8641", "ns16550";
> In this example, an operating system would first try to locate a device driver that supported fsl,mpc8641. If a
> driver was not found, it would then try to locate a driver that supported the more general ns16550 device type.

I think I understand this for individual components,
but with a SoM or complete product I get confused.

Can I understand a SoM or product as a composite device,
such as a usb to uart + i2c + gpio + spi adapter?

> Let me answer with a question, because you neither answer mine nor
> provide detailed information.
>
> Is Cortex-A15 compatible with Cortex-A7 in the Devicetree? No.
Curious! Actually I don't fully understand why that would be.
Based on the definition above, I would agree that
neither cortex-a7 nor cortex-a17 are specializations of each other.
They just happen to both support armv7-a instruction set.
> Now what
> does it mean to your case?
>
> I don't even understand what is your case.
I see :(
Yes there is a disconnect *somewhere*.

I shall try again:
Marvell is selling two chips:
1. CN9130, High-Performance Multi-Core CPU, System on Chip
(can be used alone)
2. 88F8215, SouthBridge Communication Processor, System on Chip
(only usable in combination with a CN9130)

Now, in terms of compatible string, what happens when a board
has multiples of these?

> What is 9131 and 9132?
I have no idea who came up with 9131 and 9132.
But explanation is given by Grzegorz Jaszczyk <jaz@xxxxxxxxxxxx>
when he submitted cn9131-db.dts (Marvell evaluation board):

Extend the support of the CN9130 by adding an external CP115.
The last number indicates how many external CP115 are used.

>
>
>> From software perspective we can always down-grade,
>> i.e. run software only aware of the AP on 9130, 9131 or 9132.
>> But we can't run software referencing the external CPs
>> if they are not connected.
> Same with Cortex A15 and A7, right?
Right.
>
>
>>>>>>           - const: marvell,armada-ap807-quad
>>>>>>           - const: marvell,armada-ap807
>>>>> Anyway, 6 compatibles is beyond useful amount. What are you expressing
>>>>> here?
>>>> I copied this part from the examples earlier in the file, such as:
>>>>       - description: Armada CN9132 SoC with two external CPs
>>>>         items:
>>>>           - const: marvell,cn9132
>>>>           - const: marvell,cn9131
>>>>           - const: marvell,cn9130
>>>>           - const: marvell,armada-ap807-quad
>>>>           - const: marvell,armada-ap807
>>>>>  Why is this even armada ap807?
>>>> We noticed ap807 != ap806 (cn913x != 8040),
>>>> because the thermal sensor coefficients converting
>>>> raw values to celsius differed.
>>> That's also not the best example.Might be correct but also looks
>>> over-complicated. The point of board-level compatibles is to identify
>>> machine and its common parts. It has little impact inside of kernel (at
>>> least should be almost no users inside!)
>> Indeed, the temperature coefficients are handled by the thermal device
>> compatible string, not board-level.
>>> , but there can be some users,
>>> e.g. firmware or user-space.
>>>
>>> This claims that cn9132 is compatible with ap807, so you have exactly
>>> the same base. The same base is not CPU! It's about the S in SoC, so
>>> "System".
>> I would think since the base is always a single chip combining 1x AP+CP,
>> the "system" is marvell,cn9130.
>> For Armada 8040, the system would be marvell,armada8040 by same
>> logic (also combining 1x AP+CP, different version, not extensible).
>>> Could firmware use marvell,armada-ap807 compatible to properly
>>> detect type of system and treat all these boards as ap807?
>> I have not looked into presence detection for CP's during initialization.
>> U-Boot support without spaghetti is a future Me task.
> ???
>
>> I suspect it is possible with asterisk *, because so far I have only seen
>> configuration with at least 1 CP, never with 0.
>> Presence of a boot-rom on each die e.g. supports this idea.
> I still don't understand.
>
> Best regards,
> Krzysztof
>