| Message ID | 1506418634-20366-2-git-send-email-awais.masood@vadion.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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,
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.
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.
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,
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.
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.
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.
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.
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 /*
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(-)