[linux-yocto] [kernel v5.2/standard/xlnx-soc][PATCH 2/2] arch: arm64: dts: add overlay dts file

[Quanyang Wang]

On 10/14/19 3:58 PM, Michal Simek wrote:
> On 14. 10. 19 9:55, Quanyang Wang wrote:
>> On 10/14/19 2:05 PM, Michal Simek wrote:
>>> On 13. 10. 19 15:33, quanyang.wang at windriver.com wrote:
>>>> From: MengLi <meng.li at windriver.com>
>>>>
>>>> Add overlay dts file for updating FPGA bitstream file on
>>>> zynqmp platform.
>>>>
>>>> Signed-off-by: Meng Li <Meng.Li at windriver.com>
>>>> Signed-off-by: Quanyang Wang <quanyang.wang at windriver.com>
>>>> ---
>>>>   arch/arm64/boot/dts/xilinx/Makefile            |  1 +
>>>>   .../dts/xilinx/zynqmp-zcu102-fpga-update.dts   | 18 ++++++++++++++++++
>>>>   2 files changed, 19 insertions(+)
>>>>   create mode 100644 arch/arm64/boot/dts/xilinx/zynqmp-zcu102-fpga-update.dts
>>>>
>>>> diff --git a/arch/arm64/boot/dts/xilinx/Makefile b/arch/arm64/boot/dts/xilinx/Makefile
>>>> index bec4746fe721..d56c449988d0 100644
>>>> --- a/arch/arm64/boot/dts/xilinx/Makefile
>>>> +++ b/arch/arm64/boot/dts/xilinx/Makefile
>>>> @@ -13,6 +13,7 @@ dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-zcu100-revC.dtb
>>>>   dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-zcu102-revA.dtb
>>>>   dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-zcu102-revB.dtb
>>>>   dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-zcu102-rev1.0.dtb
>>>> +dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-zcu102-fpga-update.dtb
>>>>   dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-zcu104-revA.dtb
>>>>   dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-zcu104-revC.dtb
>>>>   dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-zcu106-revA.dtb
>>>> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-fpga-update.dts b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-fpga-update.dts
>>>> new file mode 100644
>>>> index 000000000000..f1e1506b6210
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-fpga-update.dts
>>>> @@ -0,0 +1,18 @@
>>>> +// overlay dts file.
>>>> +/dts-v1/;
>>>> +/plugin/;
>>>> +
>>>> +/ {
>>>> +	fragment at 0 {
>>>> +		target-path = "/fpga-full";
>>>> +		#address-cells = <1>;
>>>> +		#size-cells = <1>;
>>>> +
>>>> +		__overlay__ {
>>>> +			#address-cells = <2>;
>>>> +			#size-cells = <2>;
>>>> +
>>>> +			firmware-name = "system.bit.bin";
>>>> +		};
>>>> +	};
>>>> +};
>>>>
>>> I understand what you want to do but not sure why you think that this is
>>> the best way how to do it.
>> Hi Michal,
>>
>> This patch is based on the url:
>>
>> https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/18841847/Solution+ZynqMP+PL+Programming#x-Programming+the+PL+through+Linux
>>
>> "FPGA programming using Device Tree Overlay (DTO)".
>>
>> We will describe how to programming FPGA by using 2 methods in README
>> for our customer:
>>
>> one is FPGA programming using sysfs attributes
>> <https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/18841847/Solution+ZynqMP+PL+Programming#SolutionZynqMPPLProgramming-FPGAprogrammingusingsysfsattributes>
>>
>> And the other is FPGA programming using Device Tree Overlay (DTO)
>> <https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/18841847/Solution+ZynqMP+PL+Programming#SolutionZynqMPPLProgramming-FPGAprogrammingusingDeviceTreeOverlay(DTO)>
>>
>> This patch is used for the second method.
>>
>> Is there any better way to do it?
>>
> It is not about that using fragments is incorrect. It is about reasons
> to include this dts file to Linux source code. Because this fragment is
> just doing programming without anything else. It means it is coupled
> with also description what it is in DT which is missing here.
> It means this is just part A (with system.bit.bin generic name - which
> is not fully accurate) and part B is missing.
> In part B there should be what you are including with clock/reset and IP
> part.
> That's why my question remains if make even sense to merge part A
> without any details about part B.

Hi Michal,

The target for this patch is to just simply test the FPGA programming 
function. So it changes nothing.

It seems to be improper to add such a dts file to kernel source.

I will transform the dts file to a text description in README or make it 
to be a more detailed and integral dts file in a V2 patch.

Thanks,

Quanyang

>
> Thanks,
> Michal
>
>
>