Re: [PATCH v5 00/22] Add support for the Mobileye EyeQ5 SoC

Gregory CLEMENT
2023-12-21T09:14:01Z

Gregory CLEMENT <gregory.clement@bootlin.com> writes:

[...]

>>>
>>> A few weeks ago, you were concerned about the introduction of the
>>> specific kconfig CONFIG_USE_XKPHYS to support EyeQ5, and you wanted us
>>> to set up a new platform instead. Since then, Jiaxun proposed a series
>>> that was merged here to provide more generic support.
>>
>> well, there is more to improve and stuff I don't like in Jaixun series.
>> For example misusing CONFIG_PHYSICAL_START to force a load address via config
>> (IMHO it's already a hack for CRASH_DUMP).
>>
>> As there is your series and Jiaxun series, where should I comment more
>> detailed ?
>
> I think you could start on Jiaxun series but the one merged in my
> series, because I already had a few fixes for it.

This sentence was not very clear, let me rephrase it: I recommend
starting the review with Jiaxun's series, specifically examining the
code that has been incorporated into my series. This is important as I
have already made several modifications to his original code

>>
>>> I had other issues in the initial series, and I think that now I've
>>> fixed all of them. So, I would like to know what your opinion is now
>>> about this series.
>>>
>>> Will you accept it, or do you still think that a new platform has to be
>>> set up?
>>
>> things have improved, but I'm still in favor to use a new platform.
>> And my main point stays. A "generic" kernel compiled for EyeQ5 will
>> just run on that platform, which doesn't sound generic to me.
>
> I do not oppose the addition of a new platform, even though, like
> Jiaxun, I would prefer to avoid duplicating code. The only thing
> preventing the use of the same kernel for EyeQ5 and other platforms is
> the starting address. Therefore, if it were possible to have a
> relocatable kernel, this issue would disappear.
>
> However, while waiting for your feedback on Jiaxun's part, I will
> attempt to add a new platform to assess exactly what the implications
> are.

Is it possible for you to apply the first patch of this series, which is
only a fix? This would enable me to have a slightly shorter
series. Additionally, it would facilitate dividing the entire series
into two parts: the first part for XKPHYS support and the second part
for EyeQ5 support.

Gregory

--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering