* [PATCH v2 1/2] xen/arm64: Add Support for Allwinner H5 (sun50i)
2017-09-26 9:37 ` [PATCH v2 0/2] xen/arm64/ns16550: Support for Allwinner H5 SoC Awais Masood
@ 2017-09-26 9:37 ` Awais Masood
2017-09-28 20:03 ` Julien Grall
2017-09-26 9:37 ` [PATCH v2 2/2] xen/ns16550: Fix ISR lockup on Designware 8250 (H5) Awais Masood
2017-10-04 11:44 ` [PATCH v3] xen/ns16550: Fix ISR lockup on Allwinner uart Awais Masood
2 siblings, 1 reply; 20+ messages in thread
From: Awais Masood @ 2017-09-26 9:37 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, Stefano Stabellini, xen-devel, Awais Masood
This patch adds support for Allwinner H5/sun50i SoC.
Makefile updated to enable ARM64 compilation for sunxi.c.
sunxi.c updates include:
- Addition of H5/sun50i dt compatibility string.
- Handling of different Watchdog timer base addresses on sun7i
and sun50i.
Tested on Orange Pi PC2
Signed-off-by: Awais Masood <awais.masood@vadion.com>
---
Changes since v1:
- Improved patch description
---
xen/arch/arm/platforms/Makefile | 1 +
xen/arch/arm/platforms/sunxi.c | 40 +++++++++++++++++++++++++++++++---------
2 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
index 49fa683..722897a 100644
--- a/xen/arch/arm/platforms/Makefile
+++ b/xen/arch/arm/platforms/Makefile
@@ -6,5 +6,6 @@ obj-$(CONFIG_ARM_32) += omap5.o
obj-$(CONFIG_ARM_32) += rcar2.o
obj-$(CONFIG_ARM_64) += seattle.o
obj-$(CONFIG_ARM_32) += sunxi.o
+obj-$(CONFIG_ARM_64) += sunxi.o
obj-$(CONFIG_ARM_64) += xgene-storm.o
obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
diff --git a/xen/arch/arm/platforms/sunxi.c b/xen/arch/arm/platforms/sunxi.c
index 0ba7b3d..06d62e7 100644
--- a/xen/arch/arm/platforms/sunxi.c
+++ b/xen/arch/arm/platforms/sunxi.c
@@ -22,18 +22,18 @@
#include <asm/io.h>
/* Watchdog constants: */
-#define SUNXI_WDT_BASE 0x01c20c90
+#define SUNXI_WDT_A20_BASE 0x01c20c90
+#define SUNXI_WDT_H5_BASE 0x01c20cA0
#define SUNXI_WDT_MODE 0x04
-#define SUNXI_WDT_MODEADDR (SUNXI_WDT_BASE + SUNXI_WDT_MODE)
#define SUNXI_WDT_MODE_EN (1 << 0)
#define SUNXI_WDT_MODE_RST_EN (1 << 1)
-static void sunxi_reset(void)
+static void sunxi_reset(u32 base)
{
void __iomem *wdt;
- wdt = ioremap_nocache(SUNXI_WDT_MODEADDR & PAGE_MASK, PAGE_SIZE);
+ wdt = ioremap_nocache((base + SUNXI_WDT_MODE) & PAGE_MASK, PAGE_SIZE);
if ( !wdt )
{
dprintk(XENLOG_ERR, "Unable to map watchdog register!\n");
@@ -42,19 +42,35 @@ static void sunxi_reset(void)
/* Enable watchdog to trigger a reset after 500 ms: */
writel(SUNXI_WDT_MODE_EN | SUNXI_WDT_MODE_RST_EN,
- wdt + (SUNXI_WDT_MODEADDR & ~PAGE_MASK));
+ wdt + ((base + SUNXI_WDT_MODE) & ~PAGE_MASK));
iounmap(wdt);
for (;;)
wfi();
}
-static const char * const sunxi_dt_compat[] __initconst =
+static void sunxi_a20_reset(void)
+{
+ sunxi_reset(SUNXI_WDT_A20_BASE);
+}
+
+static void sunxi_h5_reset(void)
+{
+ sunxi_reset(SUNXI_WDT_H5_BASE);
+}
+
+static const char * const sunxi_dt_allwinner_a20_compat[] __initconst =
{
"allwinner,sun7i-a20",
NULL
};
+static const char * const sunxi_dt_allwinner_h5_compat[] __initconst =
+{
+ "allwinner,sun50i-h5",
+ NULL
+};
+
static const struct dt_device_match sunxi_blacklist_dev[] __initconst =
{
/*
@@ -65,10 +81,16 @@ static const struct dt_device_match sunxi_blacklist_dev[] __initconst =
{ /* sentinel */ },
};
-PLATFORM_START(sunxi, "Allwinner A20")
- .compatible = sunxi_dt_compat,
+PLATFORM_START(sunxia20, "Allwinner A20")
+ .compatible = sunxi_dt_allwinner_a20_compat,
+ .blacklist_dev = sunxi_blacklist_dev,
+ .reset = sunxi_a20_reset,
+PLATFORM_END
+
+PLATFORM_START(sunxih5, "Allwinner H5")
+ .compatible = sunxi_dt_allwinner_h5_compat,
.blacklist_dev = sunxi_blacklist_dev,
- .reset = sunxi_reset,
+ .reset = sunxi_h5_reset,
PLATFORM_END
/*
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 1/2] xen/arm64: Add Support for Allwinner H5 (sun50i)
2017-09-26 9:37 ` [PATCH v2 1/2] xen/arm64: Add Support for Allwinner H5 (sun50i) Awais Masood
@ 2017-09-28 20:03 ` Julien Grall
2017-09-28 22:49 ` Andre Przywara
0 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2017-09-28 20:03 UTC (permalink / raw)
To: Awais Masood, xen-devel; +Cc: Andre Przywara, Stefano Stabellini, Xen-devel
Hi,
On 09/26/2017 10:37 AM, Awais Masood wrote:
> This patch adds support for Allwinner H5/sun50i SoC.
>
> Makefile updated to enable ARM64 compilation for sunxi.c.
>
> sunxi.c updates include:
> - Addition of H5/sun50i dt compatibility string.
> - Handling of different Watchdog timer base addresses on sun7i
> and sun50i.
>
> Tested on Orange Pi PC2
>
> Signed-off-by: Awais Masood <awais.masood@vadion.com>
>
> ---
> Changes since v1:
> - Improved patch description
> ---
> xen/arch/arm/platforms/Makefile | 1 +
> xen/arch/arm/platforms/sunxi.c | 40 +++++++++++++++++++++++++++++++---------
> 2 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
> index 49fa683..722897a 100644
> --- a/xen/arch/arm/platforms/Makefile
> +++ b/xen/arch/arm/platforms/Makefile
> @@ -6,5 +6,6 @@ obj-$(CONFIG_ARM_32) += omap5.o
> obj-$(CONFIG_ARM_32) += rcar2.o
> obj-$(CONFIG_ARM_64) += seattle.o
> obj-$(CONFIG_ARM_32) += sunxi.o
> +obj-$(CONFIG_ARM_64) += sunxi.o
Please use obj-y += sunxi.o as the platform is now supported by both
Arm32 and Arm64.
> obj-$(CONFIG_ARM_64) += xgene-storm.o
> obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
> diff --git a/xen/arch/arm/platforms/sunxi.c b/xen/arch/arm/platforms/sunxi.c
> index 0ba7b3d..06d62e7 100644
> --- a/xen/arch/arm/platforms/sunxi.c
> +++ b/xen/arch/arm/platforms/sunxi.c
> @@ -22,18 +22,18 @@
> #include <asm/io.h>
>
> /* Watchdog constants: */
> -#define SUNXI_WDT_BASE 0x01c20c90
> +#define SUNXI_WDT_A20_BASE 0x01c20c90
> +#define SUNXI_WDT_H5_BASE 0x01c20cA0
I know that we hardcoded this value for the A20. However, I am wondering
if we could find this address from the Device-Tree?
> #define SUNXI_WDT_MODE 0x04
> -#define SUNXI_WDT_MODEADDR (SUNXI_WDT_BASE + SUNXI_WDT_MODE)
> #define SUNXI_WDT_MODE_EN (1 << 0)
> #define SUNXI_WDT_MODE_RST_EN (1 << 1)
>
>
> -static void sunxi_reset(void)
> +static void sunxi_reset(u32 base)
> {
> void __iomem *wdt;
>
> - wdt = ioremap_nocache(SUNXI_WDT_MODEADDR & PAGE_MASK, PAGE_SIZE);
> + wdt = ioremap_nocache((base + SUNXI_WDT_MODE) & PAGE_MASK, PAGE_SIZE);
> if ( !wdt )
> {
> dprintk(XENLOG_ERR, "Unable to map watchdog register!\n");
> @@ -42,19 +42,35 @@ static void sunxi_reset(void)
>
> /* Enable watchdog to trigger a reset after 500 ms: */
> writel(SUNXI_WDT_MODE_EN | SUNXI_WDT_MODE_RST_EN,
> - wdt + (SUNXI_WDT_MODEADDR & ~PAGE_MASK));
> + wdt + ((base + SUNXI_WDT_MODE) & ~PAGE_MASK));
> iounmap(wdt); >
> for (;;)
> wfi();
> }
>
> -static const char * const sunxi_dt_compat[] __initconst =
> +static void sunxi_a20_reset(void)
> +{
> + sunxi_reset(SUNXI_WDT_A20_BASE);
> +}
> +
> +static void sunxi_h5_reset(void)
> +{
> + sunxi_reset(SUNXI_WDT_H5_BASE);
If I read correctly the Device-Tree for
(linux/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi), the firmware is
supporting PSCI 0.2.
PSCI 0.2 provides call for power-off/reset, so implementation the reset
callback should not be necessary.
Similarly the cubietrucks we have in osstest are using PSCI 0.2 and
should not need the reset. Andre do you know if it is the case for all
the A20?
For H5, I would impose PSCI 0.2 as the way to reset the platform. I am
leaning towards the same for A20 given that it would just be a matter of
upgrading the bootloader. Most likely you would have already done that
to get fixes.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 1/2] xen/arm64: Add Support for Allwinner H5 (sun50i)
2017-09-28 20:03 ` Julien Grall
@ 2017-09-28 22:49 ` Andre Przywara
2017-09-29 16:35 ` Andre Przywara
2017-10-03 11:32 ` Julien Grall
0 siblings, 2 replies; 20+ messages in thread
From: Andre Przywara @ 2017-09-28 22:49 UTC (permalink / raw)
To: Julien Grall, Awais Masood, xen-devel; +Cc: Stefano Stabellini, Xen-devel
Hi,
On 09/28/2017 01:03 PM, Julien Grall wrote:
> Hi,
>
> On 09/26/2017 10:37 AM, Awais Masood wrote:
>> This patch adds support for Allwinner H5/sun50i SoC.
>>
>> Makefile updated to enable ARM64 compilation for sunxi.c.
>>
>> sunxi.c updates include:
>> - Addition of H5/sun50i dt compatibility string.
>> - Handling of different Watchdog timer base addresses on sun7i
>> and sun50i.
>>
>> Tested on Orange Pi PC2
>>
>> Signed-off-by: Awais Masood <awais.masood@vadion.com>
>>
>> ---
>> Changes since v1:
>> - Improved patch description
>> ---
>> xen/arch/arm/platforms/Makefile | 1 +
>> xen/arch/arm/platforms/sunxi.c | 40
>> +++++++++++++++++++++++++++++++---------
>> 2 files changed, 32 insertions(+), 9 deletions(-)
>>
>> diff --git a/xen/arch/arm/platforms/Makefile
>> b/xen/arch/arm/platforms/Makefile
>> index 49fa683..722897a 100644
>> --- a/xen/arch/arm/platforms/Makefile
>> +++ b/xen/arch/arm/platforms/Makefile
>> @@ -6,5 +6,6 @@ obj-$(CONFIG_ARM_32) += omap5.o
>> obj-$(CONFIG_ARM_32) += rcar2.o
>> obj-$(CONFIG_ARM_64) += seattle.o
>> obj-$(CONFIG_ARM_32) += sunxi.o
>> +obj-$(CONFIG_ARM_64) += sunxi.o
>
> Please use obj-y += sunxi.o as the platform is now supported by both
> Arm32 and Arm64.
>
>> obj-$(CONFIG_ARM_64) += xgene-storm.o
>> obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
>> diff --git a/xen/arch/arm/platforms/sunxi.c
>> b/xen/arch/arm/platforms/sunxi.c
>> index 0ba7b3d..06d62e7 100644
>> --- a/xen/arch/arm/platforms/sunxi.c
>> +++ b/xen/arch/arm/platforms/sunxi.c
>> @@ -22,18 +22,18 @@
>> #include <asm/io.h>
>> /* Watchdog constants: */
>> -#define SUNXI_WDT_BASE 0x01c20c90
>> +#define SUNXI_WDT_A20_BASE 0x01c20c90
>> +#define SUNXI_WDT_H5_BASE 0x01c20cA0
>
> I know that we hardcoded this value for the A20. However, I am wondering
> if we could find this address from the Device-Tree?
Yes, both sun7i-a20.dtsi and the H5 .dts have the WDT.
Its compatible strings are sun4i-a10-wdt and sun6i-a31-wdt,
respectively. I have to check what the differences are, but I guess for
our purposes these should be small.
That seems like a call to some proper DT driven timer/WDT driver?
>
>> #define SUNXI_WDT_MODE 0x04
>> -#define SUNXI_WDT_MODEADDR (SUNXI_WDT_BASE + SUNXI_WDT_MODE)
>> #define SUNXI_WDT_MODE_EN (1 << 0)
>> #define SUNXI_WDT_MODE_RST_EN (1 << 1)
>> -static void sunxi_reset(void)
>> +static void sunxi_reset(u32 base)
>> {
>> void __iomem *wdt;
>> - wdt = ioremap_nocache(SUNXI_WDT_MODEADDR & PAGE_MASK, PAGE_SIZE);
>> + wdt = ioremap_nocache((base + SUNXI_WDT_MODE) & PAGE_MASK,
>> PAGE_SIZE);
>> if ( !wdt )
>> {
>> dprintk(XENLOG_ERR, "Unable to map watchdog register!\n");
>> @@ -42,19 +42,35 @@ static void sunxi_reset(void)
>> /* Enable watchdog to trigger a reset after 500 ms: */
>> writel(SUNXI_WDT_MODE_EN | SUNXI_WDT_MODE_RST_EN,
>> - wdt + (SUNXI_WDT_MODEADDR & ~PAGE_MASK));
>> + wdt + ((base + SUNXI_WDT_MODE) & ~PAGE_MASK));
>> iounmap(wdt); >
>> for (;;)
>> wfi();
>> }
>>
>> -static const char * const sunxi_dt_compat[] __initconst =
>> +static void sunxi_a20_reset(void)
>> +{
>> + sunxi_reset(SUNXI_WDT_A20_BASE);
>> +}
>> +
>> +static void sunxi_h5_reset(void)
>> +{
>> + sunxi_reset(SUNXI_WDT_H5_BASE);
>
> If I read correctly the Device-Tree for
> (linux/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi), the firmware is
> supporting PSCI 0.2.
>
> PSCI 0.2 provides call for power-off/reset, so implementation the reset
> callback should not be necessary.
Yes, indeed, on the H5 PSCI 0.2 reset works via ATF.
> Similarly the cubietrucks we have in osstest are using PSCI 0.2 and
> should not need the reset. Andre do you know if it is the case for all
> the A20?
It claims 0.2, but in fact it seems not to be fully compliant, as (from
looking at the code) U-Boot lacks the reset and poweroff calls. But it
looks rather straight-forward to add them, as U-Boot knows how to reset
and one would just need to wire up psci_system_reset to this.
> For H5, I would impose PSCI 0.2 as the way to reset the platform.
Yes.
> I am
> leaning towards the same for A20 given that it would just be a matter of
> upgrading the bootloader. Most likely you would have already done that
> to get fixes.
Not sure we should push people to upgrade U-Boot in general to be able
to use Xen, but as even current mainline U-Boot doesn't seem to support
it, I would rather leave the current reset support code in. Last time I
checked Linux does the same.
Cheers,
Andre.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 1/2] xen/arm64: Add Support for Allwinner H5 (sun50i)
2017-09-28 22:49 ` Andre Przywara
@ 2017-09-29 16:35 ` Andre Przywara
2017-10-04 9:16 ` Awais Masood
2017-10-03 11:32 ` Julien Grall
1 sibling, 1 reply; 20+ messages in thread
From: Andre Przywara @ 2017-09-29 16:35 UTC (permalink / raw)
To: Julien Grall, Awais Masood, xen-devel; +Cc: Stefano Stabellini, Xen-devel
Hi,
On 09/28/2017 03:49 PM, Andre Przywara wrote:
> Hi,
>
> On 09/28/2017 01:03 PM, Julien Grall wrote:
>> Hi,
>>
>> On 09/26/2017 10:37 AM, Awais Masood wrote:
>>> This patch adds support for Allwinner H5/sun50i SoC.
>>>
>>> Makefile updated to enable ARM64 compilation for sunxi.c.
...
>>> --- a/xen/arch/arm/platforms/sunxi.c
>>> +++ b/xen/arch/arm/platforms/sunxi.c
>>> @@ -22,18 +22,18 @@
>>> #include <asm/io.h>
>>> /* Watchdog constants: */
>>> -#define SUNXI_WDT_BASE 0x01c20c90
>>> +#define SUNXI_WDT_A20_BASE 0x01c20c90
>>> +#define SUNXI_WDT_H5_BASE 0x01c20cA0
>>
>> I know that we hardcoded this value for the A20. However, I am
>> wondering if we could find this address from the Device-Tree?
>
> Yes, both sun7i-a20.dtsi and the H5 .dts have the WDT.
> Its compatible strings are sun4i-a10-wdt and sun6i-a31-wdt,
> respectively. I have to check what the differences are, but I guess for
> our purposes these should be small.
> That seems like a call to some proper DT driven timer/WDT driver?
Scratch that. I just see that this is solely used for the reset
function. So we should not need this for the H5 (and the A64 for that
matter). We may need this for the H3 (Cortex-A7) support, however, which
seems quite popular on cheap boards.
Cheers,
Andre
>>> #define SUNXI_WDT_MODE 0x04
>>> -#define SUNXI_WDT_MODEADDR (SUNXI_WDT_BASE + SUNXI_WDT_MODE)
>>> #define SUNXI_WDT_MODE_EN (1 << 0)
>>> #define SUNXI_WDT_MODE_RST_EN (1 << 1)
>>> -static void sunxi_reset(void)
>>> +static void sunxi_reset(u32 base)
>>> {
>>> void __iomem *wdt;
>>> - wdt = ioremap_nocache(SUNXI_WDT_MODEADDR & PAGE_MASK, PAGE_SIZE);
>>> + wdt = ioremap_nocache((base + SUNXI_WDT_MODE) & PAGE_MASK,
>>> PAGE_SIZE);
>>> if ( !wdt )
>>> {
>>> dprintk(XENLOG_ERR, "Unable to map watchdog register!\n");
>>> @@ -42,19 +42,35 @@ static void sunxi_reset(void)
>>> /* Enable watchdog to trigger a reset after 500 ms: */
>>> writel(SUNXI_WDT_MODE_EN | SUNXI_WDT_MODE_RST_EN,
>>> - wdt + (SUNXI_WDT_MODEADDR & ~PAGE_MASK));
>>> + wdt + ((base + SUNXI_WDT_MODE) & ~PAGE_MASK));
>>> iounmap(wdt); >
>>> for (;;)
>>> wfi();
>>> }
>>>
>>> -static const char * const sunxi_dt_compat[] __initconst =
>>> +static void sunxi_a20_reset(void)
>>> +{
>>> + sunxi_reset(SUNXI_WDT_A20_BASE);
>>> +}
>>> +
>>> +static void sunxi_h5_reset(void)
>>> +{
>>> + sunxi_reset(SUNXI_WDT_H5_BASE);
>>
>> If I read correctly the Device-Tree for
>> (linux/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi), the firmware is
>> supporting PSCI 0.2.
>>
>> PSCI 0.2 provides call for power-off/reset, so implementation the
>> reset callback should not be necessary.
>
> Yes, indeed, on the H5 PSCI 0.2 reset works via ATF.
>
>> Similarly the cubietrucks we have in osstest are using PSCI 0.2 and
>> should not need the reset. Andre do you know if it is the case for all
>> the A20?
>
> It claims 0.2, but in fact it seems not to be fully compliant, as (from
> looking at the code) U-Boot lacks the reset and poweroff calls. But it
> looks rather straight-forward to add them, as U-Boot knows how to reset
> and one would just need to wire up psci_system_reset to this.
>
>> For H5, I would impose PSCI 0.2 as the way to reset the platform.
>
> Yes.
>
>> I am leaning towards the same for A20 given that it would just be a
>> matter of upgrading the bootloader. Most likely you would have already
>> done that to get fixes.
>
> Not sure we should push people to upgrade U-Boot in general to be able
> to use Xen, but as even current mainline U-Boot doesn't seem to support
> it, I would rather leave the current reset support code in. Last time I
> checked Linux does the same.
>
> Cheers,
> Andre.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 1/2] xen/arm64: Add Support for Allwinner H5 (sun50i)
2017-09-29 16:35 ` Andre Przywara
@ 2017-10-04 9:16 ` Awais Masood
2017-10-04 9:26 ` Andre Przywara
0 siblings, 1 reply; 20+ messages in thread
From: Awais Masood @ 2017-10-04 9:16 UTC (permalink / raw)
To: Andre Przywara, Julien Grall, xen-devel; +Cc: Stefano Stabellini
Hi,
On 09/29/2017 09:35 PM, Andre Przywara wrote:
> Hi,
>
> On 09/28/2017 03:49 PM, Andre Przywara wrote:
>> Hi,
>>
>> On 09/28/2017 01:03 PM, Julien Grall wrote:
>>> Hi,
>>>
>>> On 09/26/2017 10:37 AM, Awais Masood wrote:
>>>> This patch adds support for Allwinner H5/sun50i SoC.
>>>>
>>>> Makefile updated to enable ARM64 compilation for sunxi.c.
>
> ...
>
>>>> --- a/xen/arch/arm/platforms/sunxi.c
>>>> +++ b/xen/arch/arm/platforms/sunxi.c
>>>> @@ -22,18 +22,18 @@
>>>> #include <asm/io.h>
>>>> /* Watchdog constants: */
>>>> -#define SUNXI_WDT_BASE 0x01c20c90
>>>> +#define SUNXI_WDT_A20_BASE 0x01c20c90
>>>> +#define SUNXI_WDT_H5_BASE 0x01c20cA0
>>>
>>> I know that we hardcoded this value for the A20. However, I am wondering if we could find this address from the Device-Tree?
>>
>> Yes, both sun7i-a20.dtsi and the H5 .dts have the WDT.
>> Its compatible strings are sun4i-a10-wdt and sun6i-a31-wdt, respectively. I have to check what the differences are, but I guess for our purposes these should be small.
>> That seems like a call to some proper DT driven timer/WDT driver?
>
> Scratch that. I just see that this is solely used for the reset function. So we should not need this for the H5 (and the A64 for that matter). We may need this for the H3 (Cortex-A7) support, however, which seems quite popular on cheap boards.
>
Since reset routine will not be required with PSCI, I assume should revert the reset code changes for this H5 patch and leave the DT retrieval for another patch that adds H3 support. Or should I try that stuff for next version of this patch?
Awais
> Cheers,
> Andre
>
>>>> #define SUNXI_WDT_MODE 0x04
>>>> -#define SUNXI_WDT_MODEADDR (SUNXI_WDT_BASE + SUNXI_WDT_MODE)
>>>> #define SUNXI_WDT_MODE_EN (1 << 0)
>>>> #define SUNXI_WDT_MODE_RST_EN (1 << 1)
>>>> -static void sunxi_reset(void)
>>>> +static void sunxi_reset(u32 base)
>>>> {
>>>> void __iomem *wdt;
>>>> - wdt = ioremap_nocache(SUNXI_WDT_MODEADDR & PAGE_MASK, PAGE_SIZE);
>>>> + wdt = ioremap_nocache((base + SUNXI_WDT_MODE) & PAGE_MASK, PAGE_SIZE);
>>>> if ( !wdt )
>>>> {
>>>> dprintk(XENLOG_ERR, "Unable to map watchdog register!\n");
>>>> @@ -42,19 +42,35 @@ static void sunxi_reset(void)
>>>> /* Enable watchdog to trigger a reset after 500 ms: */
>>>> writel(SUNXI_WDT_MODE_EN | SUNXI_WDT_MODE_RST_EN,
>>>> - wdt + (SUNXI_WDT_MODEADDR & ~PAGE_MASK));
>>>> + wdt + ((base + SUNXI_WDT_MODE) & ~PAGE_MASK));
>>>> iounmap(wdt); >
>>>> for (;;)
>>>> wfi();
>>>> }
>>>>
>>>> -static const char * const sunxi_dt_compat[] __initconst =
>>>> +static void sunxi_a20_reset(void)
>>>> +{
>>>> + sunxi_reset(SUNXI_WDT_A20_BASE);
>>>> +}
>>>> +
>>>> +static void sunxi_h5_reset(void)
>>>> +{
>>>> + sunxi_reset(SUNXI_WDT_H5_BASE);
>>>
>>> If I read correctly the Device-Tree for (linux/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi), the firmware is supporting PSCI 0.2.
>>>
>>> PSCI 0.2 provides call for power-off/reset, so implementation the reset callback should not be necessary.
>>
>> Yes, indeed, on the H5 PSCI 0.2 reset works via ATF.
>>
>>> Similarly the cubietrucks we have in osstest are using PSCI 0.2 and should not need the reset. Andre do you know if it is the case for all the A20?
>>
>> It claims 0.2, but in fact it seems not to be fully compliant, as (from looking at the code) U-Boot lacks the reset and poweroff calls. But it looks rather straight-forward to add them, as U-Boot knows how to reset and one would just need to wire up psci_system_reset to this.
>>
>>> For H5, I would impose PSCI 0.2 as the way to reset the platform.
>>
>> Yes.
>>
>>> I am leaning towards the same for A20 given that it would just be a matter of upgrading the bootloader. Most likely you would have already done that to get fixes.
>>
>> Not sure we should push people to upgrade U-Boot in general to be able to use Xen, but as even current mainline U-Boot doesn't seem to support it, I would rather leave the current reset support code in. Last time I checked Linux does the same.
>>
>> Cheers,
>> Andre.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 1/2] xen/arm64: Add Support for Allwinner H5 (sun50i)
2017-10-04 9:16 ` Awais Masood
@ 2017-10-04 9:26 ` Andre Przywara
2017-10-04 9:39 ` Awais Masood
0 siblings, 1 reply; 20+ messages in thread
From: Andre Przywara @ 2017-10-04 9:26 UTC (permalink / raw)
To: Awais Masood, Julien Grall, xen-devel; +Cc: Stefano Stabellini
Hi Awais,
On 04/10/17 10:16, Awais Masood wrote:
> Hi,
>
> On 09/29/2017 09:35 PM, Andre Przywara wrote:
>> Hi,
>>
>> On 09/28/2017 03:49 PM, Andre Przywara wrote:
>>> Hi,
>>>
>>> On 09/28/2017 01:03 PM, Julien Grall wrote:
>>>> Hi,
>>>>
>>>> On 09/26/2017 10:37 AM, Awais Masood wrote:
>>>>> This patch adds support for Allwinner H5/sun50i SoC.
>>>>>
>>>>> Makefile updated to enable ARM64 compilation for sunxi.c.
>>
>> ...
>>
>>>>> --- a/xen/arch/arm/platforms/sunxi.c
>>>>> +++ b/xen/arch/arm/platforms/sunxi.c
>>>>> @@ -22,18 +22,18 @@
>>>>> #include <asm/io.h>
>>>>> /* Watchdog constants: */
>>>>> -#define SUNXI_WDT_BASE 0x01c20c90
>>>>> +#define SUNXI_WDT_A20_BASE 0x01c20c90
>>>>> +#define SUNXI_WDT_H5_BASE 0x01c20cA0
>>>>
>>>> I know that we hardcoded this value for the A20. However, I am wondering if we could find this address from the Device-Tree?
>>>
>>> Yes, both sun7i-a20.dtsi and the H5 .dts have the WDT.
>>> Its compatible strings are sun4i-a10-wdt and sun6i-a31-wdt, respectively. I have to check what the differences are, but I guess for our purposes these should be small.
>>> That seems like a call to some proper DT driven timer/WDT driver?
>>
>> Scratch that. I just see that this is solely used for the reset function. So we should not need this for the H5 (and the A64 for that matter). We may need this for the H3 (Cortex-A7) support, however, which seems quite popular on cheap boards.
>>
>
> Since reset routine will not be required with PSCI, I assume should revert the reset code changes for this H5 patch and leave the DT retrieval for another patch that adds H3 support. Or should I try that stuff for next version of this patch?
Thanks for the offer, but I already made a patch that adds support for
basically all virtualization capable Allwinner SoCs (both v7 and v8
ones). This looks into the DT for ARMv7 SoCs, but relies entirely on
PSCI for ARMv8 SoCs. I just need to test it, then will send it out.
So actually we won't need anything from that patch here at all, since my
patch supersedes it in a more generic way.
Do you plan on reworking/resending the UART fix (which should come
first, btw, as it is a prerequisite for H5 enablement)?
You could either send the UART fix on its own if there are changes or I
include it as patch 1/2 of my Allwinner "series".
Thanks!
Andre.
>> Cheers,
>> Andre
>>
>>>>> #define SUNXI_WDT_MODE 0x04
>>>>> -#define SUNXI_WDT_MODEADDR (SUNXI_WDT_BASE + SUNXI_WDT_MODE)
>>>>> #define SUNXI_WDT_MODE_EN (1 << 0)
>>>>> #define SUNXI_WDT_MODE_RST_EN (1 << 1)
>>>>> -static void sunxi_reset(void)
>>>>> +static void sunxi_reset(u32 base)
>>>>> {
>>>>> void __iomem *wdt;
>>>>> - wdt = ioremap_nocache(SUNXI_WDT_MODEADDR & PAGE_MASK, PAGE_SIZE);
>>>>> + wdt = ioremap_nocache((base + SUNXI_WDT_MODE) & PAGE_MASK, PAGE_SIZE);
>>>>> if ( !wdt )
>>>>> {
>>>>> dprintk(XENLOG_ERR, "Unable to map watchdog register!\n");
>>>>> @@ -42,19 +42,35 @@ static void sunxi_reset(void)
>>>>> /* Enable watchdog to trigger a reset after 500 ms: */
>>>>> writel(SUNXI_WDT_MODE_EN | SUNXI_WDT_MODE_RST_EN,
>>>>> - wdt + (SUNXI_WDT_MODEADDR & ~PAGE_MASK));
>>>>> + wdt + ((base + SUNXI_WDT_MODE) & ~PAGE_MASK));
>>>>> iounmap(wdt); >
>>>>> for (;;)
>>>>> wfi();
>>>>> }
>>>>>
>>>>> -static const char * const sunxi_dt_compat[] __initconst =
>>>>> +static void sunxi_a20_reset(void)
>>>>> +{
>>>>> + sunxi_reset(SUNXI_WDT_A20_BASE);
>>>>> +}
>>>>> +
>>>>> +static void sunxi_h5_reset(void)
>>>>> +{
>>>>> + sunxi_reset(SUNXI_WDT_H5_BASE);
>>>>
>>>> If I read correctly the Device-Tree for (linux/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi), the firmware is supporting PSCI 0.2.
>>>>
>>>> PSCI 0.2 provides call for power-off/reset, so implementation the reset callback should not be necessary.
>>>
>>> Yes, indeed, on the H5 PSCI 0.2 reset works via ATF.
>>>
>>>> Similarly the cubietrucks we have in osstest are using PSCI 0.2 and should not need the reset. Andre do you know if it is the case for all the A20?
>>>
>>> It claims 0.2, but in fact it seems not to be fully compliant, as (from looking at the code) U-Boot lacks the reset and poweroff calls. But it looks rather straight-forward to add them, as U-Boot knows how to reset and one would just need to wire up psci_system_reset to this.
>>>
>>>> For H5, I would impose PSCI 0.2 as the way to reset the platform.
>>>
>>> Yes.
>>>
>>>> I am leaning towards the same for A20 given that it would just be a matter of upgrading the bootloader. Most likely you would have already done that to get fixes.
>>>
>>> Not sure we should push people to upgrade U-Boot in general to be able to use Xen, but as even current mainline U-Boot doesn't seem to support it, I would rather leave the current reset support code in. Last time I checked Linux does the same.
>>>
>>> Cheers,
>>> Andre.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 1/2] xen/arm64: Add Support for Allwinner H5 (sun50i)
2017-10-04 9:26 ` Andre Przywara
@ 2017-10-04 9:39 ` Awais Masood
2017-10-04 10:03 ` Andre Przywara
0 siblings, 1 reply; 20+ messages in thread
From: Awais Masood @ 2017-10-04 9:39 UTC (permalink / raw)
To: Andre Przywara, Julien Grall, xen-devel; +Cc: Stefano Stabellini
Hi,
On 10/04/2017 02:26 PM, Andre Przywara wrote:
> Hi Awais,
>
> On 04/10/17 10:16, Awais Masood wrote:
>> Hi,
>>
>> On 09/29/2017 09:35 PM, Andre Przywara wrote:
>>> Hi,
>>>
>>> On 09/28/2017 03:49 PM, Andre Przywara wrote:
>>>> Hi,
>>>>
>>>> On 09/28/2017 01:03 PM, Julien Grall wrote:
>>>>> Hi,
>>>>>
>>>>> On 09/26/2017 10:37 AM, Awais Masood wrote:
>>>>>> This patch adds support for Allwinner H5/sun50i SoC.
>>>>>>
>>>>>> Makefile updated to enable ARM64 compilation for sunxi.c.
>>>
>>> ...
>>>
>>>>>> --- a/xen/arch/arm/platforms/sunxi.c
>>>>>> +++ b/xen/arch/arm/platforms/sunxi.c
>>>>>> @@ -22,18 +22,18 @@
>>>>>> #include <asm/io.h>
>>>>>> /* Watchdog constants: */
>>>>>> -#define SUNXI_WDT_BASE 0x01c20c90
>>>>>> +#define SUNXI_WDT_A20_BASE 0x01c20c90
>>>>>> +#define SUNXI_WDT_H5_BASE 0x01c20cA0
>>>>>
>>>>> I know that we hardcoded this value for the A20. However, I am wondering if we could find this address from the Device-Tree?
>>>>
>>>> Yes, both sun7i-a20.dtsi and the H5 .dts have the WDT.
>>>> Its compatible strings are sun4i-a10-wdt and sun6i-a31-wdt, respectively. I have to check what the differences are, but I guess for our purposes these should be small.
>>>> That seems like a call to some proper DT driven timer/WDT driver?
>>>
>>> Scratch that. I just see that this is solely used for the reset function. So we should not need this for the H5 (and the A64 for that matter). We may need this for the H3 (Cortex-A7) support, however, which seems quite popular on cheap boards.
>>>
>>
>> Since reset routine will not be required with PSCI, I assume should revert the reset code changes for this H5 patch and leave the DT retrieval for another patch that adds H3 support. Or should I try that stuff for next version of this patch?
>
> Thanks for the offer, but I already made a patch that adds support for
> basically all virtualization capable Allwinner SoCs (both v7 and v8
> ones). This looks into the DT for ARMv7 SoCs, but relies entirely on
> PSCI for ARMv8 SoCs. I just need to test it, then will send it out.
>
> So actually we won't need anything from that patch here at all, since my
> patch supersedes it in a more generic way.
> Do you plan on reworking/resending the UART fix (which should come
> first, btw, as it is a prerequisite for H5 enablement)?
>
> You could either send the UART fix on its own if there are changes or I
> include it as patch 1/2 of my Allwinner "series".
I'll send the latest version of UART fix as a standalone patch.
Awais
>
> Thanks!
> Andre.
>
>>> Cheers,
>>> Andre
>>>
>>>>>> #define SUNXI_WDT_MODE 0x04
>>>>>> -#define SUNXI_WDT_MODEADDR (SUNXI_WDT_BASE + SUNXI_WDT_MODE)
>>>>>> #define SUNXI_WDT_MODE_EN (1 << 0)
>>>>>> #define SUNXI_WDT_MODE_RST_EN (1 << 1)
>>>>>> -static void sunxi_reset(void)
>>>>>> +static void sunxi_reset(u32 base)
>>>>>> {
>>>>>> void __iomem *wdt;
>>>>>> - wdt = ioremap_nocache(SUNXI_WDT_MODEADDR & PAGE_MASK, PAGE_SIZE);
>>>>>> + wdt = ioremap_nocache((base + SUNXI_WDT_MODE) & PAGE_MASK, PAGE_SIZE);
>>>>>> if ( !wdt )
>>>>>> {
>>>>>> dprintk(XENLOG_ERR, "Unable to map watchdog register!\n");
>>>>>> @@ -42,19 +42,35 @@ static void sunxi_reset(void)
>>>>>> /* Enable watchdog to trigger a reset after 500 ms: */
>>>>>> writel(SUNXI_WDT_MODE_EN | SUNXI_WDT_MODE_RST_EN,
>>>>>> - wdt + (SUNXI_WDT_MODEADDR & ~PAGE_MASK));
>>>>>> + wdt + ((base + SUNXI_WDT_MODE) & ~PAGE_MASK));
>>>>>> iounmap(wdt); >
>>>>>> for (;;)
>>>>>> wfi();
>>>>>> }
>>>>>>
>>>>>> -static const char * const sunxi_dt_compat[] __initconst =
>>>>>> +static void sunxi_a20_reset(void)
>>>>>> +{
>>>>>> + sunxi_reset(SUNXI_WDT_A20_BASE);
>>>>>> +}
>>>>>> +
>>>>>> +static void sunxi_h5_reset(void)
>>>>>> +{
>>>>>> + sunxi_reset(SUNXI_WDT_H5_BASE);
>>>>>
>>>>> If I read correctly the Device-Tree for (linux/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi), the firmware is supporting PSCI 0.2.
>>>>>
>>>>> PSCI 0.2 provides call for power-off/reset, so implementation the reset callback should not be necessary.
>>>>
>>>> Yes, indeed, on the H5 PSCI 0.2 reset works via ATF.
>>>>
>>>>> Similarly the cubietrucks we have in osstest are using PSCI 0.2 and should not need the reset. Andre do you know if it is the case for all the A20?
>>>>
>>>> It claims 0.2, but in fact it seems not to be fully compliant, as (from looking at the code) U-Boot lacks the reset and poweroff calls. But it looks rather straight-forward to add them, as U-Boot knows how to reset and one would just need to wire up psci_system_reset to this.
>>>>
>>>>> For H5, I would impose PSCI 0.2 as the way to reset the platform.
>>>>
>>>> Yes.
>>>>
>>>>> I am leaning towards the same for A20 given that it would just be a matter of upgrading the bootloader. Most likely you would have already done that to get fixes.
>>>>
>>>> Not sure we should push people to upgrade U-Boot in general to be able to use Xen, but as even current mainline U-Boot doesn't seem to support it, I would rather leave the current reset support code in. Last time I checked Linux does the same.
>>>>
>>>> Cheers,
>>>> Andre.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 1/2] xen/arm64: Add Support for Allwinner H5 (sun50i)
2017-10-04 9:39 ` Awais Masood
@ 2017-10-04 10:03 ` Andre Przywara
0 siblings, 0 replies; 20+ messages in thread
From: Andre Przywara @ 2017-10-04 10:03 UTC (permalink / raw)
To: Awais Masood, Julien Grall, xen-devel; +Cc: Stefano Stabellini
Hi,
....
>>> Since reset routine will not be required with PSCI, I assume should revert the reset code changes for this H5 patch and leave the DT retrieval for another patch that adds H3 support. Or should I try that stuff for next version of this patch?
>>
>> Thanks for the offer, but I already made a patch that adds support for
>> basically all virtualization capable Allwinner SoCs (both v7 and v8
>> ones). This looks into the DT for ARMv7 SoCs, but relies entirely on
>> PSCI for ARMv8 SoCs. I just need to test it, then will send it out.
>>
>> So actually we won't need anything from that patch here at all, since my
>> patch supersedes it in a more generic way.
>> Do you plan on reworking/resending the UART fix (which should come
>> first, btw, as it is a prerequisite for H5 enablement)?
>>
>> You could either send the UART fix on its own if there are changes or I
>> include it as patch 1/2 of my Allwinner "series".
>
> I'll send the latest version of UART fix as a standalone patch.
Thanks Awais, much appreciated. I don't have that other patch here at
the moment, but will send it to you for testing ASAP.
Cheers,
Andre.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] xen/arm64: Add Support for Allwinner H5 (sun50i)
2017-09-28 22:49 ` Andre Przywara
2017-09-29 16:35 ` Andre Przywara
@ 2017-10-03 11:32 ` Julien Grall
1 sibling, 0 replies; 20+ messages in thread
From: Julien Grall @ 2017-10-03 11:32 UTC (permalink / raw)
To: Andre Przywara, Awais Masood, xen-devel; +Cc: Stefano Stabellini, Xen-devel
On 28/09/17 23:49, Andre Przywara wrote:
> Hi,
Hi,
> On 09/28/2017 01:03 PM, Julien Grall wrote:
>>
>>> #define SUNXI_WDT_MODE 0x04
>>> -#define SUNXI_WDT_MODEADDR (SUNXI_WDT_BASE + SUNXI_WDT_MODE)
>>> #define SUNXI_WDT_MODE_EN (1 << 0)
>>> #define SUNXI_WDT_MODE_RST_EN (1 << 1)
>>> -static void sunxi_reset(void)
>>> +static void sunxi_reset(u32 base)
>>> {
>>> void __iomem *wdt;
>>> - wdt = ioremap_nocache(SUNXI_WDT_MODEADDR & PAGE_MASK, PAGE_SIZE);
>>> + wdt = ioremap_nocache((base + SUNXI_WDT_MODE) & PAGE_MASK,
>>> PAGE_SIZE);
>>> if ( !wdt )
>>> {
>>> dprintk(XENLOG_ERR, "Unable to map watchdog register!\n");
>>> @@ -42,19 +42,35 @@ static void sunxi_reset(void)
>>> /* Enable watchdog to trigger a reset after 500 ms: */
>>> writel(SUNXI_WDT_MODE_EN | SUNXI_WDT_MODE_RST_EN,
>>> - wdt + (SUNXI_WDT_MODEADDR & ~PAGE_MASK));
>>> + wdt + ((base + SUNXI_WDT_MODE) & ~PAGE_MASK));
>>> iounmap(wdt); >
>>> for (;;)
>>> wfi();
>>> }
>>>
>>> -static const char * const sunxi_dt_compat[] __initconst =
>>> +static void sunxi_a20_reset(void)
>>> +{
>>> + sunxi_reset(SUNXI_WDT_A20_BASE);
>>> +}
>>> +
>>> +static void sunxi_h5_reset(void)
>>> +{
>>> + sunxi_reset(SUNXI_WDT_H5_BASE);
>>
>> If I read correctly the Device-Tree for
>> (linux/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi), the firmware is
>> supporting PSCI 0.2.
>>
>> PSCI 0.2 provides call for power-off/reset, so implementation the
>> reset callback should not be necessary.
>
> Yes, indeed, on the H5 PSCI 0.2 reset works via ATF.
>
>> Similarly the cubietrucks we have in osstest are using PSCI 0.2 and
>> should not need the reset. Andre do you know if it is the case for all
>> the A20?
>
> It claims 0.2, but in fact it seems not to be fully compliant, as (from
> looking at the code) U-Boot lacks the reset and poweroff calls. But it
> looks rather straight-forward to add them, as U-Boot knows how to reset
> and one would just need to wire up psci_system_reset to this.
Hmmm ok :/. Please ignore my suggestion below to drop reset callback for
all the allwinner platforms.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/2] xen/ns16550: Fix ISR lockup on Designware 8250 (H5)
2017-09-26 9:37 ` [PATCH v2 0/2] xen/arm64/ns16550: Support for Allwinner H5 SoC Awais Masood
2017-09-26 9:37 ` [PATCH v2 1/2] xen/arm64: Add Support for Allwinner H5 (sun50i) Awais Masood
@ 2017-09-26 9:37 ` Awais Masood
2017-09-26 11:58 ` Jan Beulich
2017-10-04 11:44 ` [PATCH v3] xen/ns16550: Fix ISR lockup on Allwinner uart Awais Masood
2 siblings, 1 reply; 20+ messages in thread
From: Awais Masood @ 2017-09-26 9:37 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Awais Masood, Ian Jackson, Tim Deegan, Jan Beulich, xen-devel
On Allwinner H5 (Orange Pi PC2) serial driver goes into an
infinite loop when interrupts are enabled. The reason is a
residual "busy detect" interrupt. Since the condition
UART_IIR_NOINT will not be true unless this interrupt is
cleared, the interrupt handler will remain locked up in this
while loop.
A hw quirk fix was previously added for designware uart under
commit:
50417cd978aa54930d065ac1f139f935d14af76d
It checks for a busy condition during setup and clears the
condition by reading UART_USR register.
On Allwinner H5 (and H3), the "busy detect" condition occurs
later because an LCR write is performed during setup 'after'
this clear and if uart is busy, the "busy detect" condition
will trigger again and cause the ISR lockup.
To solve this problem, the same UART_USR read operation is
added within the interrupt handler to clear the condition.
Linux dw 8250 driver also handles this condition within
interrupt handler
http://elixir.free-electrons.com/linux/latest/source/drivers/tty/serial/8250/8250_dw.c#L233
Tested on Orange Pi PC2 (H5). Earlier this issue was seen on H3
as well and the same fix should help.
Signed-off-by: Awais Masood <awais.masood@vadion.com>
---
Changes since v1:
* Common quirk fix code moved to a helper function
* Patch description improved with earlier commit link
---
xen/drivers/char/ns16550.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 6ab5ec3..44ed4ec 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -505,6 +505,19 @@ static int ns16550_ioport_invalid(struct ns16550 *uart)
return ns_read_reg(uart, UART_IER) == 0xff;
}
+static void ns16550_handle_dw_usr_busy_quirk(struct ns16550 *uart)
+{
+ if ( uart->dw_usr_bsy &&
+ (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
+ {
+ /* DesignWare 8250 detects if LCR is written while the UART is
+ * busy and raises a "busy detect" interrupt. Read the UART
+ * Status Register to clear this state.
+ */
+ ns_read_reg(uart, UART_USR);
+ }
+}
+
static void ns16550_interrupt(
int irq, void *dev_id, struct cpu_user_regs *regs)
{
@@ -521,6 +534,9 @@ static void ns16550_interrupt(
serial_tx_interrupt(port, regs);
if ( lsr & UART_LSR_DR )
serial_rx_interrupt(port, regs);
+
+ /* Handle the DesignWare 8250 'busy-detect' quirk. */
+ ns16550_handle_dw_usr_busy_quirk(uart);
}
}
@@ -623,15 +639,8 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
/* No interrupts. */
ns_write_reg(uart, UART_IER, 0);
- if ( uart->dw_usr_bsy &&
- (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
- {
- /* DesignWare 8250 detects if LCR is written while the UART is
- * busy and raises a "busy detect" interrupt. Read the UART
- * Status Register to clear this state.
- */
- ns_read_reg(uart, UART_USR);
- }
+ /* Handle the DesignWare 8250 'busy-detect' quirk. */
+ ns16550_handle_dw_usr_busy_quirk(uart);
/* Line control and baud-rate generator. */
ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 2/2] xen/ns16550: Fix ISR lockup on Designware 8250 (H5)
2017-09-26 9:37 ` [PATCH v2 2/2] xen/ns16550: Fix ISR lockup on Designware 8250 (H5) Awais Masood
@ 2017-09-26 11:58 ` Jan Beulich
0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-09-26 11:58 UTC (permalink / raw)
To: Awais Masood
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, xen-devel
>>> On 26.09.17 at 11:37, <awais.masood@vadion.com> wrote:
First of all please only send to one of xen-devel@lists.xen.org and
xen-devel@lists.xenproject.org.
> On Allwinner H5 (Orange Pi PC2) serial driver goes into an
> infinite loop when interrupts are enabled. The reason is a
> residual "busy detect" interrupt. Since the condition
> UART_IIR_NOINT will not be true unless this interrupt is
> cleared, the interrupt handler will remain locked up in this
> while loop.
>
> A hw quirk fix was previously added for designware uart under
> commit:
> 50417cd978aa54930d065ac1f139f935d14af76d
>
> It checks for a busy condition during setup and clears the
> condition by reading UART_USR register.
>
> On Allwinner H5 (and H3), the "busy detect" condition occurs
> later because an LCR write is performed during setup 'after'
> this clear and if uart is busy, the "busy detect" condition
> will trigger again and cause the ISR lockup.
>
> To solve this problem, the same UART_USR read operation is
> added within the interrupt handler to clear the condition.
>
> Linux dw 8250 driver also handles this condition within
> interrupt handler
> http://elixir.free-electrons.com/linux/latest/source/drivers/tty/serial/8250/
> 8250_dw.c#L233
>
> Tested on Orange Pi PC2 (H5). Earlier this issue was seen on H3
> as well and the same fix should help.
So the description almost exclusively talks about Allwinner hardware,
but ...
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -505,6 +505,19 @@ static int ns16550_ioport_invalid(struct ns16550 *uart)
> return ns_read_reg(uart, UART_IER) == 0xff;
> }
>
> +static void ns16550_handle_dw_usr_busy_quirk(struct ns16550 *uart)
> +{
> + if ( uart->dw_usr_bsy &&
> + (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
> + {
> + /* DesignWare 8250 detects if LCR is written while the UART is
> + * busy and raises a "busy detect" interrupt. Read the UART
> + * Status Register to clear this state.
> + */
> + ns_read_reg(uart, UART_USR);
> + }
> +}
... here you don't mention it at all, and ...
> @@ -521,6 +534,9 @@ static void ns16550_interrupt(
> serial_tx_interrupt(port, regs);
> if ( lsr & UART_LSR_DR )
> serial_rx_interrupt(port, regs);
> +
> + /* Handle the DesignWare 8250 'busy-detect' quirk. */
> + ns16550_handle_dw_usr_busy_quirk(uart);
> }
> }
... here, according to the description, the issue doesn't even apply
to DesignWare hardware.
Also while indeed there are many example to the contrary in this
file, please don't follow the bad habit of prefixing local functions
with something derived from the file name.
handle_dw_usr_busy_quirk() is perfectly fine a name for the new
function.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3] xen/ns16550: Fix ISR lockup on Allwinner uart
2017-09-26 9:37 ` [PATCH v2 0/2] xen/arm64/ns16550: Support for Allwinner H5 SoC Awais Masood
2017-09-26 9:37 ` [PATCH v2 1/2] xen/arm64: Add Support for Allwinner H5 (sun50i) Awais Masood
2017-09-26 9:37 ` [PATCH v2 2/2] xen/ns16550: Fix ISR lockup on Designware 8250 (H5) Awais Masood
@ 2017-10-04 11:44 ` Awais Masood
2017-10-06 14:50 ` Jan Beulich
2017-10-10 23:59 ` Stefano Stabellini
2 siblings, 2 replies; 20+ messages in thread
From: Awais Masood @ 2017-10-04 11:44 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Awais Masood, Ian Jackson,
Tim Deegan, Jan Beulich
This patch fixes an ISR lockup seen on Allwinner uart
On Allwinner H5, serial driver goes into an infinite loop
when interrupts are enabled. The reason is a residual
"busy detect" interrupt. Since the condition UART_IIR_NOINT
will not be true unless this interrupt is cleared, the
interrupt handler will remain locked up in this while loop.
A HW quirk fix was previously added for designware uart under
commit:
50417cd978aa54930d065ac1f139f935d14af76d
It checks for a busy condition during setup and clears the
condition by reading UART_USR register.
On Allwinner hardware, the "busy detect" condition occurs
later because an LCR write is performed during setup 'after'
this clear and if uart is busy, the "busy detect" condition
will trigger again and cause the ISR lockup.
To solve this problem, the same UART_USR read operation needs
to be performed within the interrupt handler to clear this
condition.
Linux dw 8250 driver also handles this condition within
interrupt handler
http://elixir.free-electrons.com/linux/latest/source/drivers/tty/serial/8250/8250_dw.c#L233
Tested on Orange Pi PC2 (H5). This issue is seen on H3
as well and the same fix works.
Signed-off-by: Awais Masood <awais.masood@vadion.com>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wei.liu2@citrix.com>
Changes since v2
- Updated comments to clarify that fix is for Allwinner hardware
- Removed ns16550 prefix from local function
Changes since v1
- Common quirk fix code moved to a helper function
- Patch description improved with earlier commit link
---
xen/drivers/char/ns16550.c | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 6ab5ec3..e0f8199 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -505,6 +505,23 @@ static int ns16550_ioport_invalid(struct ns16550 *uart)
return ns_read_reg(uart, UART_IER) == 0xff;
}
+static void handle_dw_usr_busy_quirk(struct ns16550 *uart)
+{
+ if ( uart->dw_usr_bsy &&
+ (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
+ {
+ /* DesignWare 8250 detects if LCR is written while the UART is
+ * busy and raises a "busy detect" interrupt. Read the UART
+ * Status Register to clear this state.
+ *
+ * Allwinner/sunxi UART hardware is similar to DesignWare 8250
+ * and also contains a "busy detect" interrupt. So this quirk
+ * fix will also be used for Allwinner UART.
+ */
+ ns_read_reg(uart, UART_USR);
+ }
+}
+
static void ns16550_interrupt(
int irq, void *dev_id, struct cpu_user_regs *regs)
{
@@ -521,6 +538,16 @@ static void ns16550_interrupt(
serial_tx_interrupt(port, regs);
if ( lsr & UART_LSR_DR )
serial_rx_interrupt(port, regs);
+
+ /* A "busy-detect" condition is observed on Allwinner/sunxi UART
+ * after LCR is written during setup. It needs to be cleared at
+ * this point or UART_IIR_NOINT will never be set and this loop
+ * will continue forever.
+ *
+ * This state can be cleared by calling the dw_usr_busy quirk
+ * handler that resolves "busy-detect" for DesignWare uart.
+ */
+ handle_dw_usr_busy_quirk(uart);
}
}
@@ -623,15 +650,8 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
/* No interrupts. */
ns_write_reg(uart, UART_IER, 0);
- if ( uart->dw_usr_bsy &&
- (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
- {
- /* DesignWare 8250 detects if LCR is written while the UART is
- * busy and raises a "busy detect" interrupt. Read the UART
- * Status Register to clear this state.
- */
- ns_read_reg(uart, UART_USR);
- }
+ /* Handle the DesignWare 8250 'busy-detect' quirk. */
+ handle_dw_usr_busy_quirk(uart);
/* Line control and baud-rate generator. */
ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v3] xen/ns16550: Fix ISR lockup on Allwinner uart
2017-10-04 11:44 ` [PATCH v3] xen/ns16550: Fix ISR lockup on Allwinner uart Awais Masood
@ 2017-10-06 14:50 ` Jan Beulich
2017-10-10 23:59 ` Stefano Stabellini
1 sibling, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-10-06 14:50 UTC (permalink / raw)
To: Awais Masood
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, xen-devel
>>> On 04.10.17 at 13:44, <awais.masood@vadion.com> wrote:
> This patch fixes an ISR lockup seen on Allwinner uart
>
> On Allwinner H5, serial driver goes into an infinite loop
> when interrupts are enabled. The reason is a residual
> "busy detect" interrupt. Since the condition UART_IIR_NOINT
> will not be true unless this interrupt is cleared, the
> interrupt handler will remain locked up in this while loop.
>
> A HW quirk fix was previously added for designware uart under
> commit:
> 50417cd978aa54930d065ac1f139f935d14af76d
>
> It checks for a busy condition during setup and clears the
> condition by reading UART_USR register.
>
> On Allwinner hardware, the "busy detect" condition occurs
> later because an LCR write is performed during setup 'after'
> this clear and if uart is busy, the "busy detect" condition
> will trigger again and cause the ISR lockup.
>
> To solve this problem, the same UART_USR read operation needs
> to be performed within the interrupt handler to clear this
> condition.
>
> Linux dw 8250 driver also handles this condition within
> interrupt handler
> http://elixir.free-electrons.com/linux/latest/source/drivers/tty/serial/8250/
> 8250_dw.c#L233
>
> Tested on Orange Pi PC2 (H5). This issue is seen on H3
> as well and the same fix works.
>
> Signed-off-by: Awais Masood <awais.masood@vadion.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3] xen/ns16550: Fix ISR lockup on Allwinner uart
2017-10-04 11:44 ` [PATCH v3] xen/ns16550: Fix ISR lockup on Allwinner uart Awais Masood
2017-10-06 14:50 ` Jan Beulich
@ 2017-10-10 23:59 ` Stefano Stabellini
1 sibling, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2017-10-10 23:59 UTC (permalink / raw)
To: Awais Masood
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Jan Beulich, xen-devel
On Wed, 4 Oct 2017, Awais Masood wrote:
> This patch fixes an ISR lockup seen on Allwinner uart
>
> On Allwinner H5, serial driver goes into an infinite loop
> when interrupts are enabled. The reason is a residual
> "busy detect" interrupt. Since the condition UART_IIR_NOINT
> will not be true unless this interrupt is cleared, the
> interrupt handler will remain locked up in this while loop.
>
> A HW quirk fix was previously added for designware uart under
> commit:
> 50417cd978aa54930d065ac1f139f935d14af76d
>
> It checks for a busy condition during setup and clears the
> condition by reading UART_USR register.
>
> On Allwinner hardware, the "busy detect" condition occurs
> later because an LCR write is performed during setup 'after'
> this clear and if uart is busy, the "busy detect" condition
> will trigger again and cause the ISR lockup.
>
> To solve this problem, the same UART_USR read operation needs
> to be performed within the interrupt handler to clear this
> condition.
>
> Linux dw 8250 driver also handles this condition within
> interrupt handler
> http://elixir.free-electrons.com/linux/latest/source/drivers/tty/serial/8250/8250_dw.c#L233
>
> Tested on Orange Pi PC2 (H5). This issue is seen on H3
> as well and the same fix works.
>
> Signed-off-by: Awais Masood <awais.masood@vadion.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: George Dunlap <George.Dunlap@eu.citrix.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Tim Deegan <tim@xen.org>
> CC: Wei Liu <wei.liu2@citrix.com>
>
> Changes since v2
> - Updated comments to clarify that fix is for Allwinner hardware
> - Removed ns16550 prefix from local function
>
> Changes since v1
> - Common quirk fix code moved to a helper function
> - Patch description improved with earlier commit link
> ---
> xen/drivers/char/ns16550.c | 38 +++++++++++++++++++++++++++++---------
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 6ab5ec3..e0f8199 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -505,6 +505,23 @@ static int ns16550_ioport_invalid(struct ns16550 *uart)
> return ns_read_reg(uart, UART_IER) == 0xff;
> }
>
> +static void handle_dw_usr_busy_quirk(struct ns16550 *uart)
> +{
> + if ( uart->dw_usr_bsy &&
> + (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
> + {
> + /* DesignWare 8250 detects if LCR is written while the UART is
> + * busy and raises a "busy detect" interrupt. Read the UART
> + * Status Register to clear this state.
> + *
> + * Allwinner/sunxi UART hardware is similar to DesignWare 8250
> + * and also contains a "busy detect" interrupt. So this quirk
> + * fix will also be used for Allwinner UART.
> + */
> + ns_read_reg(uart, UART_USR);
> + }
> +}
> +
> static void ns16550_interrupt(
> int irq, void *dev_id, struct cpu_user_regs *regs)
> {
> @@ -521,6 +538,16 @@ static void ns16550_interrupt(
> serial_tx_interrupt(port, regs);
> if ( lsr & UART_LSR_DR )
> serial_rx_interrupt(port, regs);
> +
> + /* A "busy-detect" condition is observed on Allwinner/sunxi UART
> + * after LCR is written during setup. It needs to be cleared at
> + * this point or UART_IIR_NOINT will never be set and this loop
> + * will continue forever.
> + *
> + * This state can be cleared by calling the dw_usr_busy quirk
> + * handler that resolves "busy-detect" for DesignWare uart.
> + */
> + handle_dw_usr_busy_quirk(uart);
> }
> }
>
> @@ -623,15 +650,8 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
> /* No interrupts. */
> ns_write_reg(uart, UART_IER, 0);
>
> - if ( uart->dw_usr_bsy &&
> - (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
> - {
> - /* DesignWare 8250 detects if LCR is written while the UART is
> - * busy and raises a "busy detect" interrupt. Read the UART
> - * Status Register to clear this state.
> - */
> - ns_read_reg(uart, UART_USR);
> - }
> + /* Handle the DesignWare 8250 'busy-detect' quirk. */
> + handle_dw_usr_busy_quirk(uart);
>
> /* Line control and baud-rate generator. */
> ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
> --
> 2.7.4
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread