| Message ID | 20210724122503.2486624-5-anup.patel@wdc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ACLINT MTIMER improvements | expand |
On Sat, Jul 24, 2021 at 5:25 AM Anup Patel <anup.patel@wdc.com> wrote: > > We can have ACLINT MTIMER devices which only support 32-bit MMIO > accesses on RV64 system so this patch adds a boolean DT property > "mtimer,no-64bit-mmio" to detect this from MTIMER DT node. > Where is this property documented ? I couldn't find it in your v3 aclint patch [1]. Did I miss something? I also think "mtimer,32bit-mmio" may be a better choice than "mtimer,no-64bit-mmio" [1]https://github.com/avpatel/linux/commits/riscv_aclint_v3 > Signed-off-by: Anup Patel <anup.patel@wdc.com> > --- > lib/utils/timer/fdt_timer_mtimer.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/utils/timer/fdt_timer_mtimer.c b/lib/utils/timer/fdt_timer_mtimer.c > index b08ed38..15a36ed 100644 > --- a/lib/utils/timer/fdt_timer_mtimer.c > +++ b/lib/utils/timer/fdt_timer_mtimer.c > @@ -58,6 +58,9 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff, > mt->mtime_size = size[0]; > mt->mtimecmp_addr = addr[1]; > mt->mtimecmp_size = size[1]; > + /* Parse additional ACLINT MTIMER properties */ > + if (fdt_getprop(fdt, nodeoff, "mtimer,no-64bit-mmio", &rc)) > + mt->has_64bit_mmio = false; > } > > rc = aclint_mtimer_cold_init(mt, mtmaster); > -- > 2.25.1 > > > -- > opensbi mailing list > opensbi@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/opensbi
On Tue, Aug 3, 2021 at 1:19 AM Atish Patra <atishp@atishpatra.org> wrote: > > On Sat, Jul 24, 2021 at 5:25 AM Anup Patel <anup.patel@wdc.com> wrote: > > > > We can have ACLINT MTIMER devices which only support 32-bit MMIO > > accesses on RV64 system so this patch adds a boolean DT property > > "mtimer,no-64bit-mmio" to detect this from MTIMER DT node. > > > > Where is this property documented ? I couldn't find it in your v3 > aclint patch [1]. > Did I miss something? I have updated DT bindings in linux v3 series. Please check again. > > I also think "mtimer,32bit-mmio" may be a better choice than > "mtimer,no-64bit-mmio" > > [1]https://github.com/avpatel/linux/commits/riscv_aclint_v3 Actually, "mtimer,no-64bit-mmio" is more appropriate because all registers of MTIMER are 64-bits wide so by default RV64 systems should support 64-bit MMIO for MTIMER. In fact, most existing RV64 systems do support 64-bit MMIO for MTIMER. Regards, Anup > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > --- > > lib/utils/timer/fdt_timer_mtimer.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/lib/utils/timer/fdt_timer_mtimer.c b/lib/utils/timer/fdt_timer_mtimer.c > > index b08ed38..15a36ed 100644 > > --- a/lib/utils/timer/fdt_timer_mtimer.c > > +++ b/lib/utils/timer/fdt_timer_mtimer.c > > @@ -58,6 +58,9 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff, > > mt->mtime_size = size[0]; > > mt->mtimecmp_addr = addr[1]; > > mt->mtimecmp_size = size[1]; > > + /* Parse additional ACLINT MTIMER properties */ > > + if (fdt_getprop(fdt, nodeoff, "mtimer,no-64bit-mmio", &rc)) > > + mt->has_64bit_mmio = false; > > } > > > > rc = aclint_mtimer_cold_init(mt, mtmaster); > > -- > > 2.25.1 > > > > > > -- > > opensbi mailing list > > opensbi@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/opensbi > > > > -- > Regards, > Atish
On 24 Jul 2021, at 13:25, Anup Patel <anup.patel@wdc.com> wrote: > > We can have ACLINT MTIMER devices which only support 32-bit MMIO > accesses on RV64 system so this patch adds a boolean DT property > "mtimer,no-64bit-mmio" to detect this from MTIMER DT node. Can the ACLINT not just mandate 64-bit accesses must work? FreeBSD and Linux aren’t going to want to support such broken implementations, and I would hope at least the upcoming platform spec will mandate 64-bit access, if not the ACLINT itself. Jess > Signed-off-by: Anup Patel <anup.patel@wdc.com> > --- > lib/utils/timer/fdt_timer_mtimer.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/utils/timer/fdt_timer_mtimer.c b/lib/utils/timer/fdt_timer_mtimer.c > index b08ed38..15a36ed 100644 > --- a/lib/utils/timer/fdt_timer_mtimer.c > +++ b/lib/utils/timer/fdt_timer_mtimer.c > @@ -58,6 +58,9 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff, > mt->mtime_size = size[0]; > mt->mtimecmp_addr = addr[1]; > mt->mtimecmp_size = size[1]; > + /* Parse additional ACLINT MTIMER properties */ > + if (fdt_getprop(fdt, nodeoff, "mtimer,no-64bit-mmio", &rc)) > + mt->has_64bit_mmio = false; > } > > rc = aclint_mtimer_cold_init(mt, mtmaster); > -- > 2.25.1 > > > -- > opensbi mailing list > opensbi@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/opensbi
On Sat, Aug 7, 2021 at 7:55 PM Jessica Clarke <jrtc27@jrtc27.com> wrote: > > On 24 Jul 2021, at 13:25, Anup Patel <anup.patel@wdc.com> wrote: > > > > We can have ACLINT MTIMER devices which only support 32-bit MMIO > > accesses on RV64 system so this patch adds a boolean DT property > > "mtimer,no-64bit-mmio" to detect this from MTIMER DT node. > > Can the ACLINT not just mandate 64-bit accesses must work? FreeBSD and > Linux aren’t going to want to support such broken implementations, and > I would hope at least the upcoming platform spec will mandate 64-bit > access, if not the ACLINT itself. This has been discussed on the AIA mailing lists. The ACLINT spec can't mandate 64-bit MMIO access because there is a cost associated in using wider bus between harts and ACLINT device to support 64-bit MMIO accesses. Andrew confirmed that the RISC-V privilege spec does not explicitly state that 64-MMIO accesses are mandatory for MTIME and MTIMECMP on RV64 systems. Refer: https://lists.riscv.org/g/tech-aia/message/114 https://lists.riscv.org/g/tech-aia/message/115 https://lists.riscv.org/g/tech-aia/message/117 https://lists.riscv.org/g/tech-aia/message/118 https://lists.riscv.org/g/tech-aia/message/119 https://lists.riscv.org/g/tech-aia/message/120 Regards, Anup > > Jess > > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > --- > > lib/utils/timer/fdt_timer_mtimer.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/lib/utils/timer/fdt_timer_mtimer.c b/lib/utils/timer/fdt_timer_mtimer.c > > index b08ed38..15a36ed 100644 > > --- a/lib/utils/timer/fdt_timer_mtimer.c > > +++ b/lib/utils/timer/fdt_timer_mtimer.c > > @@ -58,6 +58,9 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff, > > mt->mtime_size = size[0]; > > mt->mtimecmp_addr = addr[1]; > > mt->mtimecmp_size = size[1]; > > + /* Parse additional ACLINT MTIMER properties */ > > + if (fdt_getprop(fdt, nodeoff, "mtimer,no-64bit-mmio", &rc)) > > + mt->has_64bit_mmio = false; > > } > > > > rc = aclint_mtimer_cold_init(mt, mtmaster); > > -- > > 2.25.1 > > > > > > -- > > opensbi mailing list > > opensbi@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/opensbi >
diff --git a/lib/utils/timer/fdt_timer_mtimer.c b/lib/utils/timer/fdt_timer_mtimer.c index b08ed38..15a36ed 100644 --- a/lib/utils/timer/fdt_timer_mtimer.c +++ b/lib/utils/timer/fdt_timer_mtimer.c @@ -58,6 +58,9 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff, mt->mtime_size = size[0]; mt->mtimecmp_addr = addr[1]; mt->mtimecmp_size = size[1]; + /* Parse additional ACLINT MTIMER properties */ + if (fdt_getprop(fdt, nodeoff, "mtimer,no-64bit-mmio", &rc)) + mt->has_64bit_mmio = false; } rc = aclint_mtimer_cold_init(mt, mtmaster);
We can have ACLINT MTIMER devices which only support 32-bit MMIO accesses on RV64 system so this patch adds a boolean DT property "mtimer,no-64bit-mmio" to detect this from MTIMER DT node. Signed-off-by: Anup Patel <anup.patel@wdc.com> --- lib/utils/timer/fdt_timer_mtimer.c | 3 +++ 1 file changed, 3 insertions(+)