[PATCH 4/5] xilinx: zynqmp: add Beckhoff CX8200

[Steffen Dirkwinkel]

Am 01.09.23 um 13:16 schrieb Michal Simek:
> 
> 
> On 9/1/23 11:00, Steffen Dirkwinkel wrote:
>>
>>
>> Am 01.09.23 um 09:36 schrieb Michal Simek:
>>>
>>>
>>> On 8/30/23 16:03, Steffen Dirkwinkel wrote:
>>>> From: Steffen Dirkwinkel <s.dirkwinkel at beckhoff.com>
>>>>
>>>> This adds support for the Beckhoff CX8200 series of industrial 
>>>> embedded PCs.
>>>> There is some information about the device and features here:
>>>> https://www.beckhoff.com/en-en/products/ipc/embedded-pcs/cx8200-arm-cortex-a53/
>>>>
>>>> Currently supported/tested:
>>>> - Boot from microSD
>>>> - Ethernet
>>>> - USB
>>>> - rtc / rtc eeprom
>>>> - tpm access
>>>> - uart
>>>>
>>>> Open points:
>>>> - adding the psgtr usb phy doesn't work in linux (failed to get pll
>>>>    lock)
>>>> - fpga loading currently only as u-boot script or pre launch cmd (type
>>>>    may be stored in eeprom of rtc so this could be made generic)
>>>>
>>>> Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel at beckhoff.com>
>>>> ---
>>>>
>>>>   arch/arm/dts/Makefile                         |    1 +
>>>>   arch/arm/dts/zynqmp-beckhoff-cx8200.dts       |  247 +++
>>>>   .../zynqmp-beckhoff-cx8200/psu_init_gpl.c     | 1960 
>>>> +++++++++++++++++
>>>>   configs/xilinx_zynqmp_virt_defconfig          |    2 +-
>>>>   4 files changed, 2209 insertions(+), 1 deletion(-)
>>>>   create mode 100644 arch/arm/dts/zynqmp-beckhoff-cx8200.dts
>>>>   create mode 100644 
>>>> board/xilinx/zynqmp/zynqmp-beckhoff-cx8200/psu_init_gpl.c
>>>
>>> First of all xilinx folder is not the right location because 
>>> Xilinx/AMD is not manufacturer of this board.
>>
>> Yeah, sorry. I saw the avnet board and copied that.
>>
>>>
>>> Second I am normally pushing back on adding these custom boards 
>>> because it just increase time for maintaining.
>>> Your last commit was in 2019 but at least you have some commits that 
>>> I can trust that you would maintain your board for some time.
>>
>> Our last board [1] only had linux/u-boot support as an afterthought. 
>> Customers had to buy a special option to set the right boot fuses, so 
>> most devices don’t boot u-boot.
>> With this board and a second similar zynqmp board (CX9240 [2]) we’ll 
>> have u-boot as default (and probably only) bootloader, so we’ll be
>> more active. We can also set two of us as maintainers and it would be 
>> fine to drop the boards if nobody responds.
>> We generally support these industrial boards for long time frames and 
>> would like to stay close to mainline instead of maintaining forks.
>>
>> The alternative would be to have a downstream u-boot repository on 
>> github or somewhere. We’ll still likely have something there for build 
>> scripts / firmware builds / integration, but don’t plan to really 
>> diverge from upstream u-boot.
>>
>> The main advantage of being in upstream u-boot would be that we can 
>> trigger internal CI on upstream changes. We can still do that and 
>> apply patches, but even simple patches like adding files to a makefile 
>> may fail to apply and will need fixing.
>>
>>
>>>
>>> My biggest question is in what category is your board unique that it 
>>> should be added it to the tree?
>>
>> Currently the board isn’t really unique. I guess we’d be the only 
>> users of the rtc with eeprom and there might be something needed for 
>> loading the correct fpga file based on eeprom (this might be done in 
>> linux / userspace or even u-boot script though)
>>
>> [1] 
>> https://www.beckhoff.com/en-en/products/ipc/embedded-pcs/cx9020-arm-cortex-a8/cx9020.html#tab_productdetails_1
>> [2] 
>> https://www.beckhoff.com/en-en/products/ipc/embedded-pcs/cx9240-arm-cortex-a53/cx9240.html
> 
> Regarding board itself.
> It is DTB - we use OF_SEPARATE/OF_BOARD it means building is easy. I 
> expect you have pretty much something in PL that's why your DT is bigger 
> if you don't use DT overlays but for fixed design there is actually no 
> reason to use it.
> 
> psu_init_gpl. You are adding it to specific folder which match 
> DEVICE_TREE variable when you use it. You can actually just copy it to 
> board/xilinx/zynqmp/ folder and it will be pick up for your build.
> 
> xilinx_zynqmp_virt_defconfig - you likely don't want to use this in your 
> product because there are things enabled which you don't use on your 
> board. That's why you should tune it for your usage.
> 
> regs.init - that's for boot.bin generation with SPL - BOOT_INIT_FILE 
> should be used and it can't be wired via generic defconfig anyway.
> 
> That pretty much leads to the state that make no sense for you to use 
> xilinx_zynqmp_virt_defconfig.

There are things in the defconfig we don't need, but size hasn't really 
been an issue, since we boot from relatively large sdcards. We might 
still do separate config for all our boards though.

We currently build virt_defconfig and set BOOT_INIT_FILE, 
PMUFW_INIT_FILE and ZYNQMP_SPL_PM_CFG_OBJ_FILE. If we ship this firmware 
somewhere (similar to linux-firmware?) we could set fixed paths for that 
and make rebuilding u-boot simpler. I've found reproducing images for 
zynqmp devboards with and without using petalinux and the xilinx fsbl to 
be quite annoying on the first try and this could be simpler if one just 
wants to support generic arm64 distros.

> 
> I think we are still supporting defconfig fragments which is the way how 
> to maintain your board to be close to mainline as possible.
> 
> And fdt_addr change. I think this is for me the most problematic part 
> which should be solved. Pretty much all these variables should be moved 
> out and that's what we started to work on. But it will take some time to 
> get there.
> 

I agree, I can also work on that, but it will likely be in October since 
I'm on vacation for most of September. Maybe one of our other developers 
will work on it instead.

> Definitely there shouldn't be a problem to merge that rtc driver and I 
> would prefer if you can stay with your board out of upstream and try to 
> maintain it via your build scripts only. And let me know if we can 
> improve/fix something to be able to do it in long term. But pretty much 
> all things should be in place already to do it without pain. At least I 
> would like you to try it and see if there is any major blocker which we 
> can try to fix.

I don't really understand why the board shouldn't be upstream, once it 
is just a dts file + psu_init_gpl + regs. There really isn't much to 
maintain there that wouldn't also break out of tree.

We might be different from most other out of tree zynqmp users, since we 
usually also offer to sell hardware without our software and users would 
then either download an image from us or build their own u-boot + linux 
and only download the fpga bitstreams. I expect most people would look 
at upstream u-boot and linux before looking for our downstream repositories.

We can and will of course start with a downstream repository, I just 
don't understand why real (non-devboard) hardware should be out of tree.

Thanks,
Steffen


> 
> Thanks,
> Michal
>