Re: [PATCH v2 00/20] Add minimal Tensor/GS101 SoC support and Oriole/Pixel6 board

Henrik Grimler
2023-11-06T15:16:39Z

Hi all,

On Mon, Nov 06, 2023 at 01:36:24PM +0000, Peter Griffin wrote:
> Hi Krzysztof,
>
> On Fri, 3 Nov 2023 at 14:49, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 03/11/2023 14:56, Maksym Holovach wrote:
> > > Hi Peter,
> > >
> > > On 11/3/23 15:11, Peter Griffin wrote:
> > >> Hi Maksym,
> > >>
> > >> Thanks for your feedback.

[ ... ]

> > >> I guess the same is also true for `axis,artpec8` and `tesla,fsd` SoCs.
> > >> IMO the SoC compatible string should be uniquely identifying the actual
> > >> SoC, not a close relative.
> > >>
> > >> Regarding product_id you are correct this reads 0x09845000 but even
> > >> within Samsung Exynos family there are examples where the register
> > >> value does not match the SoC compatible. For example Exynos850 SoC
> > >> has a product ID value of "E3830". Where the Linux compatible is
> > >> matching the Samsung marketing name, not the internal/outdated name.
> > >
> > > I did not know Exynos 850 is also not going under it's real name.
> > > Ultimately, I believe all of those SoCs should go under their technical
> > > name in the exynos/ directory.
> >
> > The initial technical name does not exist outside of vendor sources and
> > part name. E.g. Winlink E850 board hardware manual calls it:
> > "Samsung Exynos 850, S5E3830"
> > and everywhere else Exynos 850 SoC is used.
> >
> > If you start calling it Exynos 3830, only me and Sam (who mainlined it)
> > would know what is it. Everyone else, all users of kernel, would be
> > confused.
> >
> > Therefore using well known final product name is for Exynos850 reasonable.
>
> I agree that was the correct decision IMO, and it is a very similar
> situation here
> as far as I'm concerned. Most people don't know Tensor G1 SoC as Exynos
> 9845.

I am not sure if Exynos 3830 and the like would be *that* confusing to
users. It does not seem like the internal names (like Exynos 3830) are
well guarded secret. The wikipedia article for Exynos even sort of
gives these names for recent Exynos models, Exynos 850 is called
S5E3830 for example, see section for Exynos 800:
https://en.wikipedia.org/wiki/Exynos#Exynos_800_series

Exynos 9845/S5E9845 is not mentioned in the article though.

[ ... ]

> > > Another concern is that Google could in the future license other SoC: be
> > > it Qualcomm, Nvidia or anything. If we put completely different hw under
> > > google/ directory, does it really make sense? In that case, who'll
> > > maintain the google/ directory? Exynos people? Qualcomm people if they
> > > license it? Some other people?
> >
> > That's indeed a problem. Future Tesla SoC might have just few pieces
> > similar to FSD. There would be no common SoC part, except the actual
> > Tesla IP.
> >
> > Same for Google. Future GSXXX, if done by Qualcomm, will be absolutely
> > different than GS101 and the only common part would be the TPU (Tensor).
>
> There is more Google IP than TPU but I see the point you're making.
>
> >
> > So now let's decide what is the common denominator:
> > 1. Core SoC architecture, like buses, pinctrl, clocks, timers, serial,
> > and many IP blocks, which constitute 95% of Devicetree bindings and drivers,
> > 2. The one, big piece made by Samsung's customer: TPU, NPU or whatever.
>
> Or multiple big pieces of IP.
>
> Does having it all under the exynos directory help you somehow with
> maintenance? Has having Alim maintain tesla-fsd in a separate directory
> caused issues?
>
> I structured it like this as I thought it would scale better, and also
> because it was
> consistent with what you had accepted previously for other Exynos derived SoCs
> like Tesla.

Another small benefit of putting GS101 in exynos/ is that it makes it
easier for future contributors to find the code. If someone is for
example trying to add mainline support for their Samsung Galaxy S21
(Exynos 2100/Exynos 9840) and want to find somewhat related boards to
draw inspiration from then it is not clear at a first glance that
google/gs101-* are (somewhat) related.

[ ... ]

> Thanks,
>
> Peter.

Best regards,
Henrik Grimler