Re: [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support

[Sudeep Holla <sudeep.holla@arm.com>]

On Fri, May 22, 2020 at 08:41:59PM +0200, Arnd Bergmann wrote:
> On Fri, May 22, 2020 at 6:54 PM Sudeep Holla <sudeep.holla@arm.com> wrote:

[...]

> >
> > Not sure if I understand your concerns completely here.
> >
> > Anyways I wanted to clarify that the jep106 encoding is applicable only
> > for manufacturer's id and not for SoC ID or revision. Not sure if that
> > changes anything about your concerns.
>
> The problem I see is that by looking at just the existing attributes,
> you have no way of telling what namespace the strings are in,
> and a script that tries pattern matching could confuse two
> hexadecimal numbers from a different namespace, such as
> pci vendor/device or usb vendor/device IDs that are similar
> in spirit to the jep106 codes.
>

Ah OK, understood.

> > > - I think we should have something unique in "family" just because
> > >   existing scripts can use that as the primary indentifier
>
> This is part of the same issue: If we put just "jep106 identified SoC"
> as the "family", it would be something a driver could match against.
>

I understand that and that's the reason I was introducing new field as
manufacture but I now see there is no point in adding that.

[...]

> > But this just indicates manufacturer id and nothing related to SoC family.
> > If it is jep106:043b, all it indicates is Arm Ltd and assigning it to
> > family doesn't sound right to me.
> >
> > I had requests for both of the above during the design of interface but
> > I was told vendors were happy with the interface. I will let the authors
> > speak about that.
>
> In most cases, the existing drivers put a hardcoded string into the
> family, such as
>
> "Samsung Exynos"
> "Freescale i.MX"
> "Amlogic Meson"
>
> or slightly more specific
>
> "R-Car Gen2"
>

Hmmm....

> Having a numeric identifier for the SoC manufacturer here is a
> bit more coarse than that, but would be similar in spirit.
>

Agreed.


[..]

> >
> > OK, I agree on ease part. But for me, we don't have any property in the
> > list to indicate the vendor/manufacturer's name. I don't see issue adding
> > one, name can be fixed as jep106_identification_code is too specific.
> >
> > How about manufacturer with the value in the format "jep106:1234" if
> > it is not normal string but jep106 encoding.
>
> I don't think we need a real name like "Arm" or "Samsung" here,
> but just a number is not enough, it should at least be something
> that can be assumed to never conflict with the name of a chip
> by another scheme.
>

Fair enough.

> jep106:5678 (the IMP_DEF_SOC_ID field in my example) would
> probably be sufficient to not conflict with a another soc_device
> driver, but is quite likely to clash with an ID used by another
> manufacturer.
>

IIUC, you are fine with "jep106:1234:5678" where 1234 is jep106 manufacture
id code and 5678 is soc_id as it may avoid all the conflicts across
the manufacture namespaces.

> jep106:1234 (the manufacturer ID) in turn seems too broad from
> the soc_id field, as that would include every chip made by one
> company.
>

I understand, and IIUC you prefer this to be embedded with soc_id
especially to avoid namespace conflicts which makes sense.

Thanks for all the discussion and valuable inputs. I am now bit nervous
to add SoC info from SMCCC ARCH_SOC_ID to sysfs yet as we need more info
especially "machine" and "serial_number" for elsewhere(OEM firmware mostly
from DT or ACPI).


TBH, the mix might be bit of a mess as there are soc drivers that rely
on DT completely today. Anyways, this SOC_ID can be added as library that
can be used by a *generic* soc driver once we define a standard way to
fetch other information("machine" and "serial_number"). I am happy to
get suggestions on that front especially from you and Francois as you
have got some context already.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel