* [PATCH] devicetree: zynqmp.dtsi: Add bootmode selection support
@ 2020-02-19 12:20 Mike Looijmans
2020-02-19 18:23 ` Vesa Jääskeläinen
0 siblings, 1 reply; 6+ messages in thread
From: Mike Looijmans @ 2020-02-19 12:20 UTC (permalink / raw)
To: robh+dt, michal.simek, mark.rutland, devicetree
Cc: m.tretter, nava.manne, rajan.vaja, manish.narani,
linux-arm-kernel, linux-kernel, Mike Looijmans
Add bootmode override support for ZynqMP devices. Allows one to select
a boot device by running "reboot qspi32" for example. Activate config
item CONFIG_SYSCON_REBOOT_MODE to make this work.
Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index 26d926eb1431..4c38d77ecbba 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -246,6 +246,30 @@
};
};
+ /* Clock and Reset control registers for LPD */
+ lpd_apb: apb@ff5e0000 {
+ compatible = "syscon", "simple-mfd";
+ reg = <0x0 0xff5e0000 0x0 0x400>;
+ reboot-mode {
+ compatible = "syscon-reboot-mode";
+ offset = <0x200>;
+ mask = <0xf100>;
+ /* Bit(8) is the "force user" bit */
+ mode-normal = <0x0000>;
+ mode-psjtag = <0x0100>;
+ mode-qspi24 = <0x1100>;
+ mode-qspi32 = <0x2100>;
+ mode-sd0 = <0x3100>;
+ mode-nand = <0x4100>;
+ mode-sd1 = <0x6100>;
+ mode-emmc = <0x6100>;
+ mode-usb0 = <0x7100>;
+ mode-pjtag0 = <0x8100>;
+ mode-pjtag1 = <0x9100>;
+ mode-sd1ls = <0xe100>;
+ };
+ };
+
/* GDMA */
fpd_dma_chan1: dma@fd500000 {
status = "disabled";
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] devicetree: zynqmp.dtsi: Add bootmode selection support
2020-02-19 12:20 [PATCH] devicetree: zynqmp.dtsi: Add bootmode selection support Mike Looijmans
@ 2020-02-19 18:23 ` Vesa Jääskeläinen
2020-02-20 6:49 ` Michal Simek
2020-02-20 6:56 ` Mike Looijmans
0 siblings, 2 replies; 6+ messages in thread
From: Vesa Jääskeläinen @ 2020-02-19 18:23 UTC (permalink / raw)
To: Mike Looijmans, robh+dt, michal.simek, mark.rutland, devicetree
Cc: m.tretter, nava.manne, rajan.vaja, manish.narani,
linux-arm-kernel, linux-kernel
Hi Mike,
On 19.2.2020 14.20, Mike Looijmans wrote:
> Add bootmode override support for ZynqMP devices. Allows one to select
> a boot device by running "reboot qspi32" for example. Activate config
> item CONFIG_SYSCON_REBOOT_MODE to make this work.
>
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> ---
> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> index 26d926eb1431..4c38d77ecbba 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> @@ -246,6 +246,30 @@
> };
> };
>
> + /* Clock and Reset control registers for LPD */
> + lpd_apb: apb@ff5e0000 {
> + compatible = "syscon", "simple-mfd";
> + reg = <0x0 0xff5e0000 0x0 0x400>;
> + reboot-mode {
> + compatible = "syscon-reboot-mode";
> + offset = <0x200>;
> + mask = <0xf100>;
> + /* Bit(8) is the "force user" bit */
> + mode-normal = <0x0000>;
> + mode-psjtag = <0x0100>;
> + mode-qspi24 = <0x1100>;
> + mode-qspi32 = <0x2100>;
> + mode-sd0 = <0x3100>;
> + mode-nand = <0x4100>;
> + mode-sd1 = <0x6100>;
> + mode-emmc = <0x6100>;
> + mode-usb0 = <0x7100>;
> + mode-pjtag0 = <0x8100>;
> + mode-pjtag1 = <0x9100>;
> + mode-sd1ls = <0xe100>;
This kinda looks a bit misuse of reboot mode support.
Usually you are signal with reboot-mode that you want to do factory
reset, enter recovery mode or such things.
Now this signaling here is telling that this is used for selecting from
what device to boot from.
Another problem is that this now modifies all Xilinx Zynq MPSoCs which
is kinda wrong. This behavior should really be product/board specific
and not common for all boards -- undoing this in product/board is
somewhat cumbersome. Now this change hijacks the "reboot <arg>" with
this behavior which is not so nice.
Thanks,
Vesa Jääskeläinen
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] devicetree: zynqmp.dtsi: Add bootmode selection support
2020-02-19 18:23 ` Vesa Jääskeläinen
@ 2020-02-20 6:49 ` Michal Simek
2020-02-20 6:56 ` Mike Looijmans
1 sibling, 0 replies; 6+ messages in thread
From: Michal Simek @ 2020-02-20 6:49 UTC (permalink / raw)
To: Vesa Jääskeläinen, Mike Looijmans, robh+dt,
michal.simek, mark.rutland, devicetree
Cc: m.tretter, nava.manne, rajan.vaja, manish.narani,
linux-arm-kernel, linux-kernel
On 19. 02. 20 19:23, Vesa Jääskeläinen wrote:
> Hi Mike,
>
> On 19.2.2020 14.20, Mike Looijmans wrote:
>> Add bootmode override support for ZynqMP devices. Allows one to select
>> a boot device by running "reboot qspi32" for example. Activate config
>> item CONFIG_SYSCON_REBOOT_MODE to make this work.
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>> ---
>> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 24 ++++++++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> index 26d926eb1431..4c38d77ecbba 100644
>> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> @@ -246,6 +246,30 @@
>> };
>> };
>> + /* Clock and Reset control registers for LPD */
>> + lpd_apb: apb@ff5e0000 {
>> + compatible = "syscon", "simple-mfd";
>> + reg = <0x0 0xff5e0000 0x0 0x400>;
>> + reboot-mode {
>> + compatible = "syscon-reboot-mode";
>> + offset = <0x200>;
>> + mask = <0xf100>;
>> + /* Bit(8) is the "force user" bit */
>> + mode-normal = <0x0000>;
>> + mode-psjtag = <0x0100>;
>> + mode-qspi24 = <0x1100>;
>> + mode-qspi32 = <0x2100>;
>> + mode-sd0 = <0x3100>;
>> + mode-nand = <0x4100>;
>> + mode-sd1 = <0x6100>;
>> + mode-emmc = <0x6100>;
>> + mode-usb0 = <0x7100>;
>> + mode-pjtag0 = <0x8100>;
>> + mode-pjtag1 = <0x9100>;
>> + mode-sd1ls = <0xe100>;
>
> This kinda looks a bit misuse of reboot mode support.
>
> Usually you are signal with reboot-mode that you want to do factory
> reset, enter recovery mode or such things.
>
> Now this signaling here is telling that this is used for selecting from
> what device to boot from.
>
> Another problem is that this now modifies all Xilinx Zynq MPSoCs which
> is kinda wrong. This behavior should really be product/board specific
> and not common for all boards -- undoing this in product/board is
> somewhat cumbersome. Now this change hijacks the "reboot <arg>" with
> this behavior which is not so nice.
Another reason is that on arm64 these regs shoulnd't be accessed by non
secure software and you should setup protection not to enable it.
If this functionality is useful for your design please keep it in your
board dts file.
Thanks,
Michal
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] devicetree: zynqmp.dtsi: Add bootmode selection support
2020-02-19 18:23 ` Vesa Jääskeläinen
2020-02-20 6:49 ` Michal Simek
@ 2020-02-20 6:56 ` Mike Looijmans
2020-02-20 7:01 ` Michal Simek
1 sibling, 1 reply; 6+ messages in thread
From: Mike Looijmans @ 2020-02-20 6:56 UTC (permalink / raw)
To: Vesa Jääskeläinen, robh+dt, michal.simek,
mark.rutland, devicetree
Cc: m.tretter, nava.manne, rajan.vaja, manish.narani,
linux-arm-kernel, linux-kernel
On 19-02-2020 19:23, Vesa Jääskeläinen wrote:
> Hi Mike,
>
> On 19.2.2020 14.20, Mike Looijmans wrote:
>> Add bootmode override support for ZynqMP devices. Allows one to select
>> a boot device by running "reboot qspi32" for example. Activate config
>> item CONFIG_SYSCON_REBOOT_MODE to make this work.
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>> ---
>> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 24 ++++++++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> index 26d926eb1431..4c38d77ecbba 100644
>> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> @@ -246,6 +246,30 @@
>> };
>> };
>> + /* Clock and Reset control registers for LPD */
>> + lpd_apb: apb@ff5e0000 {
>> + compatible = "syscon", "simple-mfd";
>> + reg = <0x0 0xff5e0000 0x0 0x400>;
>> + reboot-mode {
>> + compatible = "syscon-reboot-mode";
>> + offset = <0x200>;
>> + mask = <0xf100>;
>> + /* Bit(8) is the "force user" bit */
>> + mode-normal = <0x0000>;
>> + mode-psjtag = <0x0100>;
>> + mode-qspi24 = <0x1100>;
>> + mode-qspi32 = <0x2100>;
>> + mode-sd0 = <0x3100>;
>> + mode-nand = <0x4100>;
>> + mode-sd1 = <0x6100>;
>> + mode-emmc = <0x6100>;
>> + mode-usb0 = <0x7100>;
>> + mode-pjtag0 = <0x8100>;
>> + mode-pjtag1 = <0x9100>;
>> + mode-sd1ls = <0xe100>;
>
> This kinda looks a bit misuse of reboot mode support.
>
> Usually you are signal with reboot-mode that you want to do factory reset,
> enter recovery mode or such things.
>
> Now this signaling here is telling that this is used for selecting from what
> device to boot from.
On the ZynqMP this is the only way to communicate with the ROM.
> Another problem is that this now modifies all Xilinx Zynq MPSoCs which is
> kinda wrong. This behavior should really be product/board specific and not
> common for all boards -- undoing this in product/board is somewhat cumbersome.
The boot mode setting is in the SOC, and is not board specific. The ROM
interprets this field. The only board specific thing is that you may not
actually have a NAND chip attached to it.
My idea was that a board could easily add say 'mode-recovery=<0x2100>;' to
make the QPSI boot the method of recovery. The bootloader also has access to
this register, so it can see that there was a boot mode override in effect.
> Now this change hijacks the "reboot <arg>" with this behavior which is not so
> nice.
If anyone has a better suggestion as to where this should go, I'd be more than
happy to hear about it. It's the only interface that I could find in the
kernel to attach a bootmode override to.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] devicetree: zynqmp.dtsi: Add bootmode selection support
2020-02-20 6:56 ` Mike Looijmans
@ 2020-02-20 7:01 ` Michal Simek
2020-02-20 8:01 ` Mike Looijmans
0 siblings, 1 reply; 6+ messages in thread
From: Michal Simek @ 2020-02-20 7:01 UTC (permalink / raw)
To: Mike Looijmans, Vesa Jääskeläinen, robh+dt,
michal.simek, mark.rutland, devicetree
Cc: m.tretter, nava.manne, rajan.vaja, manish.narani,
linux-arm-kernel, linux-kernel
On 20. 02. 20 7:56, Mike Looijmans wrote:
> On 19-02-2020 19:23, Vesa Jääskeläinen wrote:
>> Hi Mike,
>>
>> On 19.2.2020 14.20, Mike Looijmans wrote:
>>> Add bootmode override support for ZynqMP devices. Allows one to select
>>> a boot device by running "reboot qspi32" for example. Activate config
>>> item CONFIG_SYSCON_REBOOT_MODE to make this work.
>>>
>>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>>> ---
>>> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 24 ++++++++++++++++++++++++
>>> 1 file changed, 24 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>>> b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>>> index 26d926eb1431..4c38d77ecbba 100644
>>> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>>> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>>> @@ -246,6 +246,30 @@
>>> };
>>> };
>>> + /* Clock and Reset control registers for LPD */
>>> + lpd_apb: apb@ff5e0000 {
>>> + compatible = "syscon", "simple-mfd";
>>> + reg = <0x0 0xff5e0000 0x0 0x400>;
>>> + reboot-mode {
>>> + compatible = "syscon-reboot-mode";
>>> + offset = <0x200>;
>>> + mask = <0xf100>;
>>> + /* Bit(8) is the "force user" bit */
>>> + mode-normal = <0x0000>;
>>> + mode-psjtag = <0x0100>;
>>> + mode-qspi24 = <0x1100>;
>>> + mode-qspi32 = <0x2100>;
>>> + mode-sd0 = <0x3100>;
>>> + mode-nand = <0x4100>;
>>> + mode-sd1 = <0x6100>;
>>> + mode-emmc = <0x6100>;
>>> + mode-usb0 = <0x7100>;
>>> + mode-pjtag0 = <0x8100>;
>>> + mode-pjtag1 = <0x9100>;
>>> + mode-sd1ls = <0xe100>;
>>
>> This kinda looks a bit misuse of reboot mode support.
>>
>> Usually you are signal with reboot-mode that you want to do factory
>> reset, enter recovery mode or such things.
>>
>> Now this signaling here is telling that this is used for selecting
>> from what device to boot from.
>
> On the ZynqMP this is the only way to communicate with the ROM.
>
>> Another problem is that this now modifies all Xilinx Zynq MPSoCs which
>> is kinda wrong. This behavior should really be product/board specific
>> and not common for all boards -- undoing this in product/board is
>> somewhat cumbersome.
>
> The boot mode setting is in the SOC, and is not board specific. The ROM
> interprets this field. The only board specific thing is that you may not
> actually have a NAND chip attached to it.
>
> My idea was that a board could easily add say 'mode-recovery=<0x2100>;'
> to make the QPSI boot the method of recovery. The bootloader also has
> access to this register, so it can see that there was a boot mode
> override in effect.
>
>> Now this change hijacks the "reboot <arg>" with this behavior which is
>> not so nice.
>
> If anyone has a better suggestion as to where this should go, I'd be
> more than happy to hear about it. It's the only interface that I could
> find in the kernel to attach a bootmode override to.
IIRC as the part of PSCI 1.1 spec is SYSTEM_RESET2 where you can device
reset_type. IIRC that 0 as warm reset was coming based on discussion
with Xilinx (and maybe others) and I think this is what Xilinx is still
using. But didn't track it if that was really implemented or not.
Thanks,
Michal
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] devicetree: zynqmp.dtsi: Add bootmode selection support
2020-02-20 7:01 ` Michal Simek
@ 2020-02-20 8:01 ` Mike Looijmans
0 siblings, 0 replies; 6+ messages in thread
From: Mike Looijmans @ 2020-02-20 8:01 UTC (permalink / raw)
To: Michal Simek, Vesa Jääskeläinen, robh+dt,
mark.rutland, devicetree
Cc: m.tretter, nava.manne, rajan.vaja, manish.narani,
linux-arm-kernel, linux-kernel
On 20-02-2020 08:01, Michal Simek wrote:
> On 20. 02. 20 7:56, Mike Looijmans wrote:
>> On 19-02-2020 19:23, Vesa Jääskeläinen wrote:
>>> Hi Mike,
>>>
>>> On 19.2.2020 14.20, Mike Looijmans wrote:
>>>> Add bootmode override support for ZynqMP devices. Allows one to select
>>>> a boot device by running "reboot qspi32" for example. Activate config
>>>> item CONFIG_SYSCON_REBOOT_MODE to make this work.
>>>>
>>>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>>>> ---
>>>> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 24 ++++++++++++++++++++++++
>>>> 1 file changed, 24 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>>>> b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>>>> index 26d926eb1431..4c38d77ecbba 100644
>>>> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>>>> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>>>> @@ -246,6 +246,30 @@
>>>> };
>>>> };
>>>> + /* Clock and Reset control registers for LPD */
>>>> + lpd_apb: apb@ff5e0000 {
>>>> + compatible = "syscon", "simple-mfd";
>>>> + reg = <0x0 0xff5e0000 0x0 0x400>;
>>>> + reboot-mode {
>>>> + compatible = "syscon-reboot-mode";
>>>> + offset = <0x200>;
>>>> + mask = <0xf100>;
>>>> + /* Bit(8) is the "force user" bit */
>>>> + mode-normal = <0x0000>;
>>>> + mode-psjtag = <0x0100>;
>>>> + mode-qspi24 = <0x1100>;
>>>> + mode-qspi32 = <0x2100>;
>>>> + mode-sd0 = <0x3100>;
>>>> + mode-nand = <0x4100>;
>>>> + mode-sd1 = <0x6100>;
>>>> + mode-emmc = <0x6100>;
>>>> + mode-usb0 = <0x7100>;
>>>> + mode-pjtag0 = <0x8100>;
>>>> + mode-pjtag1 = <0x9100>;
>>>> + mode-sd1ls = <0xe100>;
>>>
>>> This kinda looks a bit misuse of reboot mode support.
>>>
>>> Usually you are signal with reboot-mode that you want to do factory
>>> reset, enter recovery mode or such things.
>>>
>>> Now this signaling here is telling that this is used for selecting
>>> from what device to boot from.
>>
>> On the ZynqMP this is the only way to communicate with the ROM.
>>
>>> Another problem is that this now modifies all Xilinx Zynq MPSoCs which
>>> is kinda wrong. This behavior should really be product/board specific
>>> and not common for all boards -- undoing this in product/board is
>>> somewhat cumbersome.
>>
>> The boot mode setting is in the SOC, and is not board specific. The ROM
>> interprets this field. The only board specific thing is that you may not
>> actually have a NAND chip attached to it.
>>
>> My idea was that a board could easily add say 'mode-recovery=<0x2100>;'
>> to make the QPSI boot the method of recovery. The bootloader also has
>> access to this register, so it can see that there was a boot mode
>> override in effect.
>>
>>> Now this change hijacks the "reboot <arg>" with this behavior which is
>>> not so nice.
>>
>> If anyone has a better suggestion as to where this should go, I'd be
>> more than happy to hear about it. It's the only interface that I could
>> find in the kernel to attach a bootmode override to.
>
> IIRC as the part of PSCI 1.1 spec is SYSTEM_RESET2 where you can device
> reset_type. IIRC that 0 as warm reset was coming based on discussion
> with Xilinx (and maybe others) and I think this is what Xilinx is still
> using. But didn't track it if that was really implemented or not.
I checked the arm-trusted-firmware code, there's no Xilinx vendor specific
implementation of SYSTEM_RESET2 today, so it will only do a generic warm
reboot and nothing else.
I'll move the patch to our BSP, we need the functionality for software
update/recovery mechanisms.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-20 8:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-19 12:20 [PATCH] devicetree: zynqmp.dtsi: Add bootmode selection support Mike Looijmans
2020-02-19 18:23 ` Vesa Jääskeläinen
2020-02-20 6:49 ` Michal Simek
2020-02-20 6:56 ` Mike Looijmans
2020-02-20 7:01 ` Michal Simek
2020-02-20 8:01 ` Mike Looijmans
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).