[U-Boot] [PATCH] arm64: zynqmp: Create fdtfile from compatible string

[Alexander Graf]

On 14.02.19 13:33, Michal Simek wrote:
> On 14. 02. 19 13:27, Alexander Graf wrote:
>>
>>
>> On 14.02.19 13:18, Michal Simek wrote:
>>> distro boot expects that fdtfile name is setup for alternative DTB.
>>> Create this file based on the first platform compatible string.
>>> This should ensure that one rootfs can store multiple DTBs for different
>>> boards.
>>>
>>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>>> ---
>>>
>>> How good/bad idea is this?
>>>
>>> ---
>>>  board/xilinx/zynqmp/zynqmp.c | 30 ++++++++++++++++++++++++++++++
>>>  1 file changed, 30 insertions(+)
>>>
>>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>>> index a49f2f3e9ebe..64800d869819 100644
>>> --- a/board/xilinx/zynqmp/zynqmp.c
>>> +++ b/board/xilinx/zynqmp/zynqmp.c
>>> @@ -514,6 +514,32 @@ static u32 reset_reason(void)
>>>  	return ret;
>>>  }
>>>  
>>> +static int set_fdtfile(void)
>>> +{
>>> +	char *compatible, *fdtfile;
>>> +
>>> +	if (env_get("fdtfile"))
>>> +		return 0;
>>> +
>>> +	compatible = (char *)fdt_getprop(gd->fdt_blob, 0, "compatible", NULL);
>>> +	if (compatible) {
>>> +		debug("Compatible: %s\n", compatible);
>>> +
>>> +		strsep(&compatible, ",");
>>> +
>>> +		fdtfile = calloc(1, strlen(compatible) + 4);
>>> +		if (!fdtfile)
>>> +			return -ENOMEM;
>>> +
>>> +		sprintf(fdtfile, "%s%s", compatible, ".dtb");
>>
>> AArch64 dtbs are usually stored in a directory structure which you want
>> to match again. Is this the case for the compatible string? From a quick
>> glimpse, it looks like it - but please make sure that something is in
>> place to ensure it stays that way (upstream review for example).
> 
> I have actually remove xlnx prefix from compatible string but just doing
> simple conversion xlnx,zynqmp-zcu102-revA to xlnx/zynqmp-zcu102-revA.dtb
> looks good to me too.

That won't work, as the kernel will install the dtb in a directory
called "xilinx".

> Vendor prefixes should be stable enough to ensures that.
> 
>> So the only change I would make here is that this should be "xilinx/%s.dtb".
> 
> Any issue with xlnx prefix?

Yup :). Please stick to exactly where the kernel installs the dtbs to.


Alex

>