[U-Boot] litesom.c: Board stuff in SOC ?

[Marcin Niestroj]

Hi Stefano,

I've added Tom to the discussion. So if we make some changes to SOM
code position in u-boot tree, let's make sure we do it for all
architectures the same (at least I mean chilisom code which is in
arch/arm/mach-omap2/am33xx/).

On 18.05.2017 17:20, Stefano Babic wrote:
> Hi Marcin,
> 
> On 18/05/2017 16:57, Marcin Niestroj wrote:
>> Hi Stefano,
>>
>> On 18.05.2017 16:28, Stefano Babic wrote:
>>> Hi Marcin,
>>>
>>> even if it was already merged (and maybe I am guilty of it because I
>>> have not noted before), it is completely crazy that a board is stored
>>> inside the SOC directory. The litesom board is in fact in
>>> ./arch/arm/cpu/armv7/mx6/litesom.c, and there is nothing that justify
>>> this. The code has just board related stuff and nothing common for all
>>> SOC.
>>
>> litesom is not a board, but a SOM.
> 
> It does not matter, sorry. The code is not common for all boards (and if
> you like it, all SOMs) sharing the same SOC or SOC family. In this case,
> i.MX6.
> 
> There is plenty of such as example in U-Boot, please check it in code.
> SOM support is in the boards directory and it must not be here. It will
> be removed.
> 
>> It has only RAM and eMMC memory
>> included with the processor.
> 
> Like all SOMs you find in u-boot from a lot of different vendors...just
> check it.
> 
>> litesom cannot work on it's own. It needs
>> to be part of some board. An example board is liteboard, which support
>> is included in board/grinn/liteboard/. Please visit [2] to visualize
>> what the litesom device is.
> 
> Thanks, nothing new.
> 
> Again: the SOM is specific to a vendor and cannot be in the SOC
> directory. There should be then a board/grinn/common (or whatever you
> want) where SOM code is put. And again, not in SOC directory.
> 
>>
>> The idea about creating a separate file in arch/arm/cpu/armv7/mx6/
> 
> The idea is correct, just in wrong place. There are plenty of examples
> doing this:
> 
> ./engicam/common

Ok, I see. But it was just merged, so I couldn't look at it earlier.
As I understand, you want me to follow this example with litesom.

I do not say that is a bad idea in general and we should not follow it.
*BUT* right now these sources cannot be used when other vendor
will create it's own board. They won't compile, because root Makefile
has a "libs-$(HAVE_VENDOR_COMMON_LIB) += board/$(VENDOR)/common/"
entry, which depends on *board* vendor.

> ./freescale/common

There is a lot of shared code between freescale boards, but I do not see
SOM example there. Could you point me one?

> ./compulab/common

I do not see SOM code there. There is also ./compulab/cl-som-am57x
directory, but it contains *board* support code, which uses
CL-SOM-AM57x, not pure SOM (e.g. they configure UART and SD card,
which are not SOM specific).

> 
> 
> .... and many others.
> 
>> was to be able to reuse code when new boards, that use litesom
>> as it's core, will be added. And these boards need not to be
>> manufactured or designed by Grinn.
> 
> Right, so why are we discussing ? They belong to grinn, that means the
> code should be in board/<vendor>, that is board/grinn. Please move it !
> 
>> So if some other vendor wants
>> to add support for it's board (which will be based on litesom),
>> the code to initialize RAM and eMMC can be reused.
> 
> They will be put code into the grimm directory. See all other vendors
> selling SOMs.
> 
>> When litesom
>> code would be part of board/grinn/ directory, then other vendors
>> could not easily add support for their boards without copying
>> litesom sources.
> 
> I will take care that the code will not be duplicated - not worry.

Ok. Could you tell me how that would be done? That is my main concern
about moving SOM sources to ./board/<vendor>/common. In fact I've
already asked about that in both RFC for adding lite(som/board) [3] [4]
and described my motivation in PATCH adding lite(som/board) [5] support
in u-boot.

[3] https://lists.denx.de/pipermail/u-boot/2016-August/265384.html
[4] https://lists.denx.de/pipermail/u-boot/2016-September/266534.html
[5] https://lists.denx.de/pipermail/u-boot/2016-September/266799.html

> 
> Please move it or it will be removed, thanks !
> 
>>
>> [2] http://grinn-global.com/litesom/
>>
>>>
>>> I just took again the commit and I see:
>>>
>>> Moving arch/arm/mach-litesom/ to arch/arm/cpu/armv7/mx6/ was requested
>>> in [1] during discussion of chiliSOM support patches.
>>>
>>> [1] http://lists.denx.de/pipermail/u-boot/2017-January/279137.html
>>>
>>>
>>> But [1] has nothing to do with the context.  I will tend to revert this
>>> patch and wait for an appropriate patch that add support for the board
>>> just like all other boards in U-Boot - as it is currently, it is wrong.
>>
>> Link [1] was a discussion of adding chilisom support into u-boot.
>> The idea was the same - allow to reuse SOM code for vendors creating
>> their own board based on our SOMs.
> 
> Idea is already used in U-Boot and the SOM vendor has priority. Else
> code inside arch/arm/cpu/*, that must be common to all boards and *all*
> SOMs using those processor is becoming a mess. Please move it.

I do not oppose to move SOM sources from arch/arm/cpu/*, but in my
opinion we need some mechanism for reusing SOM code for all board
vendors before doing so.

-- 
Regards,
Marcin Niestroj