Re: [PATCH 2/3] ARM64: dts: amlogic: Add basic support for Amlogic S905X

[Kevin Hilman]

On Mon, Sep 12, 2016 at 2:43 PM, Andreas FÃrber <afaerber@xxxxxxx> wrote:
> Am 12.09.2016 um 22:43 schrieb Kevin Hilman:
>> Carlo Caione <carlo@xxxxxxxxxx> writes:
>>> On Mon, Sep 12, 2016 at 6:28 PM, Andreas FÃrber <afaerber@xxxxxxx> wrote:
>>>
>>>>> +Boards with the Amlogic Meson GXL SoC shall have the following properties:
>>>>> +  Required root node property:
>>>>> +    compatible: "amlogic,meson-gxl-s905x", "amlogic,meson-gxl";
>>>>
>>>> Can we please use "amlogic,s905x", "amlogic,meson-gxl"? No need to
>>>> complicate the name. Also affects .dtsi and .dts below.
>>>
>>> gxl != s905x.
>
> Huh? You're seemingly completely missing my point...
>
> But you are right that _Neil's_ heading needs to be fixed, too:
> Clearly not all GXL SoCs need to have an S905X compatible string!
> So it should be "Boards with the Amlogic S905X SoC shall ..." or so.
>
>>> AFAWK to the GXL family belong several different SoCs, like S905X,
>>> S905D, etc... (see patch 3/3)
>
> Thanks, I already know that, that's why you have two compatible strings
> instead of just one like for GXBB. We can certainly prepend one for
> symmetry there, too, if it makes you happier.
>
>>> This is why we use meson-gxl-s905x, meson-gxl-s905d, etc...
>>
>> Correct.
>>
>>> We could s/meson-gxl-s905x/meson-s905x/ and
>>> s/meson-gxl-s905d/meson-s905d/ but I honestly prefer this way because
>>> we can clearly see which family the SoC belongs to (the Amlogic naming
>>> convention is already messy enough).
>>> I mean, yes it's longer, but it's for the sake of documentation IMO.
>>
>> +1
>
> I still don't follow that conclusion. The board is called "amlogic,p231"
> because P231 is a unique identifier within the Amlogic namespace, so why
> not call the SoC "amlogic,s905x" for the same reason? The documentation
> is already there in having both "amlogic,s905x" _and_
> "amlogic,meson-gxl" - please re-read my post. There is no S905X in GXL
> family and another S905X in some other Amlogic SoC family, so it's
> unique and there is no reason to encode any hierarchies into its name
> other than vendor,name.
>
> I'm not arguing over the file name, where it perfectly makes sense to
> have a meson-gxl- prefix (already discussed), just about the compatible
> string where we don't have "amlogic,meson-gxl-s905x-p231" either because
> it is completely unnecessary and does _not_ add any value.

Sorry, I'm guilty of not fully reading the original post.  I was
thinking only of the filenames, which it seems were already agreed on
to be long.

> Not that we're checking this string anywhere anyway... If you want to
> check for the GXL family you have to use "amlogic,meson-gxl"; if you
> want to check for the specific SoC you use "amlogic,s905x". Simple. We
> never match partial strings, so there is no sense in a hardcoded prefix
> that is duplicating information already available.

For the compatible strings, I think your proposed shorter (but still
unique) forms are fine with me.

Kevin