Re: [PATCH] ARM: i.MX8MP: add Koenig+Bauer Alphajet board

[Johannes Zink <j.zink@pengutronix.de>]

On 6/1/23 11:14, Ahmad Fatoum wrote:
> On 01.06.23 11:08, Johannes Zink wrote:
>> On 6/1/23 10:49, Ahmad Fatoum wrote:
>>> On 01.06.23 09:40, Johannes Zink wrote:
>>>> +static const struct of_device_id kb_alphajet_of_match[] = {
>>>> +    { .compatible = "kb,alphajet" },
>>>
>>> Is anything of the above really alphajet specific? If both PHY and QSPI flash
>>> are on SOM, you should match against SoM compatible here.
>>
>> No, the above can be moved to a generic SoM part. I assume though, that stuff will not work properly (TM) as long as the individual boards don't enable deep probe support (at least it did not work for my machine), though I think this should be kept in the baseboard-specific part, right?
> 
> I'd just enable deep probe for the SoM compatible. If someone else's
> baseboard support clashes with deep probe, they should fix that.
> 

sounds good to me.

>>> We want to avoid future boards with Congatec modules needing to duplicate
>>> code. Can you split this up:
>>>
>>> config MACH_CONGATEC_QX8MP
>>>      bool "Koenig+Bauer AlphaJet Board"
>>
>> I guess this should probably be
>> bool "Congatec conga-QMX8-Plus"
>> then?
> 
> Just drop the prompt altogether and add _SOM at the end of the
> config symbol. If we ever add the Congatec baseboard, it would
> just select the SoM, but there's no reason to be able to select
> the SoM from menuconfig without a board.

ack.

> 
>>
>>>      select ARCH_IMX8MP
>>>      select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>>>      select FIRMWARE_IMX8MP_ATF
>>>      select ARM_SMCCC
>>>      select MCI_IMX_ESDHC_PBL
>>>      select IMX8M_DRAM
>>>      select I2C_IMX_EARLY
>>>
>>> config MACH_KB_ALPHAJET
>>>      bool "Koenig+Bauer AlphaJet Board"
>>>      select MACH_CONGATEC_QX8MP
>>>
>>
>> ack.
>>
>>>> +
>>>>    config MACH_PHYTEC_SOM_IMX8MQ
>>>>        bool "Phytec i.MX8M SOM"
>>>>        select ARCH_IMX8MQ
>>>> diff --git a/images/Makefile.imx b/images/Makefile.imx
>>>> index 9e70ba50a162..bfb2fcdd8ede 100644
>>>> --- a/images/Makefile.imx
>>>> +++ b/images/Makefile.imx
>>>> @@ -473,6 +473,11 @@ CFG_start_variscite_imx8mp_dart.pblb.imximg = $(board)/variscite-dt8mcustomboard
>>>>    FILE_barebox-variscite-imx8mp-dart-cb.img = start_variscite_imx8mp_dart.pblb.pimximg
>>>>    image-$(CONFIG_MACH_VARISCITE_DT8MCUSTOMBOARD_IMX8MP) += barebox-variscite-imx8mp-dart-cb.img
>>>>    +pblb-$(CONFIG_MACH_KB_ALPHAJET) += start_congatec_qx8p
>>>
>>> This is defined no where. Should this be qx8mp too?

I have to look into this, this _might_ be an old remainder I overlooked when I 
cleaned up my patch(es).

> 
> Also, you should add the board to arch/arm/configs/{multi_v8_defconfig,imx_v8_defconfig}
> 

ack

>>>
>>>> +CFG_start_kb_alphajet.pblb.imximg = $(board)/kb-imx8mp-alphajet/flash-header-imx8mp-cgtqx8p.imxcfg
>>>> +FILE_barebox-kb-alphajet.img = start_kb_alphajet.pblb.pimximg
>>>> +image-$(CONFIG_MACH_KB_ALPHAJET) += barebox-kb-alphajet.img
>>>> +
>>>>    # ----------------------- i.MX8mq based boards --------------------------
>>>>    pblb-$(CONFIG_MACH_NXP_IMX8MQ_EVK) += start_nxp_imx8mq_evk
>>>>    CFG_start_nxp_imx8mq_evk.pblb.imximg = $(board)/nxp-imx8mq-evk/flash-header-imx8mq-evk.imxcfg
>>>>
>>>> ---
>>>> base-commit: 64c528dd0bff0953192d90fbbc34b3d45d8116d0
>>>> change-id: 20230601-koenigbauer-alphajet-upstreaming-2085dc8b9f77
>>>>
>>>> Best regards,
>>>
>>
>> Best regards
>> Johannes
>>
> 

-- 
Pengutronix e.K.                | Johannes Zink                  |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |