| Message ID | 20231018124023.2927710-1-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] hw/sh4/sh7750: Add STBCR/STBCR2 register support | expand |
Hi Geert! On Wed, 2023-10-18 at 14:40 +0200, Geert Uytterhoeven wrote: > The new Linux SH7750 clock driver uses the registers for power-down > mode control, causing a crash: > > byte read to SH7750_STBCR_A7 (0x000000001fc00004) not supported > Aborted (core dumped) > > Fix this by adding support for the Standby Control Registers STBCR and > STBCR2. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Is this supposed to be applied on top of Yoshinori's DT conversion series? Thanks, Adrian
Hi Adrian, On Wed, Oct 18, 2023 at 2:46 PM John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> wrote: > On Wed, 2023-10-18 at 14:40 +0200, Geert Uytterhoeven wrote: > > The new Linux SH7750 clock driver uses the registers for power-down > > mode control, causing a crash: > > > > byte read to SH7750_STBCR_A7 (0x000000001fc00004) not supported > > Aborted (core dumped) > > > > Fix this by adding support for the Standby Control Registers STBCR and > > STBCR2. > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Is this supposed to be applied on top of Yoshinori's DT conversion series? No, it's a patch for QEMU. Sorry for the confusion. Gr{oetje,eeting}s, Geert
On Wed, 18 Oct 2023 21:40:23 +0900, Geert Uytterhoeven wrote: > > The new Linux SH7750 clock driver uses the registers for power-down > mode control, causing a crash: > > byte read to SH7750_STBCR_A7 (0x000000001fc00004) not supported > Aborted (core dumped) > > Fix this by adding support for the Standby Control Registers STBCR and > STBCR2. FRQCR is also not returning the correct value, so it needs to be fixed. Here are my changes. https://gitlab.com/yoshinori.sato/qemu.git It include. - Minimal CPG support. - DT support - Add target LANDISK. > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > [RFC PATCH v3 12/35] drivers/clk/renesas: clk-sh7750.c SH7750/7751 CPG driver. > https://lore.kernel.org/all/a772e1b6de89af22057d3af31cc03dcad7964fc7.1697199949.git.ysato@users.sourceforge.jp > > Accesses to CLKSTP00 and CLKSTCLK00 (0xfe0a0000/0x1e0a0000 and > 0xfe0a0008/0x1e0a0008) don't seem to cause any issues, although I can't > see immediately where they are handled? > > --- > hw/sh4/sh7750.c | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c > index ebe0fd96d94ca17b..deeb83b4540bbf2b 100644 > --- a/hw/sh4/sh7750.c > +++ b/hw/sh4/sh7750.c > @@ -59,6 +59,9 @@ typedef struct SH7750State { > uint16_t bcr3; > uint32_t bcr4; > uint16_t rfcr; > + /* Power-Down Modes */ > + uint8_t stbcr; > + uint8_t stbcr2; > /* PCMCIA controller */ > uint16_t pcr; > /* IO ports */ > @@ -219,7 +222,13 @@ static void ignore_access(const char *kind, hwaddr addr) > > static uint32_t sh7750_mem_readb(void *opaque, hwaddr addr) > { > + SH7750State *s = opaque; > + > switch (addr) { > + case SH7750_STBCR_A7: > + return s->stbcr; > + case SH7750_STBCR2_A7: > + return s->stbcr2; > default: > error_access("byte read", addr); > abort(); > @@ -318,14 +327,24 @@ static uint32_t sh7750_mem_readl(void *opaque, hwaddr addr) > static void sh7750_mem_writeb(void *opaque, hwaddr addr, > uint32_t mem_value) > { > + SH7750State *s = opaque; > > if (is_in_sdrmx(addr, 2) || is_in_sdrmx(addr, 3)) { > ignore_access("byte write", addr); > return; > } > > - error_access("byte write", addr); > - abort(); > + switch (addr) { > + case SH7750_STBCR_A7: > + s->stbcr = mem_value; > + return; > + case SH7750_STBCR2_A7: > + s->stbcr2 = mem_value; > + return; > + default: > + error_access("byte write", addr); > + abort(); > + } > } > > static void sh7750_mem_writew(void *opaque, hwaddr addr, > -- > 2.34.1 >
Hi Sato-san, On Thu, Oct 19, 2023 at 4:03 AM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > On Wed, 18 Oct 2023 21:40:23 +0900, > Geert Uytterhoeven wrote: > > The new Linux SH7750 clock driver uses the registers for power-down > > mode control, causing a crash: > > > > byte read to SH7750_STBCR_A7 (0x000000001fc00004) not supported > > Aborted (core dumped) > > > > Fix this by adding support for the Standby Control Registers STBCR and > > STBCR2. > > FRQCR is also not returning the correct value, so it needs to be fixed. I knew there would be more, hence the RFC ;-) > Here are my changes. > https://gitlab.com/yoshinori.sato/qemu.git > > It include. > - Minimal CPG support. > - DT support > - Add target LANDISK. Thank you very much! It would be a good idea to mention this is the cover letter of your Linux patch series, so your test audience doesn't have to fix already-solved problems... BTW, your commit da64d6541226a516 ("hw/sh4: sh7750.c allow access STBCR and STBCR2.") just ignores writes, and always returns zero when reading. This may cause issues with Linux code relying on clock_ops.is_enabled() to return correct data. Gr{oetje,eeting}s, Geert
diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c index ebe0fd96d94ca17b..deeb83b4540bbf2b 100644 --- a/hw/sh4/sh7750.c +++ b/hw/sh4/sh7750.c @@ -59,6 +59,9 @@ typedef struct SH7750State { uint16_t bcr3; uint32_t bcr4; uint16_t rfcr; + /* Power-Down Modes */ + uint8_t stbcr; + uint8_t stbcr2; /* PCMCIA controller */ uint16_t pcr; /* IO ports */ @@ -219,7 +222,13 @@ static void ignore_access(const char *kind, hwaddr addr) static uint32_t sh7750_mem_readb(void *opaque, hwaddr addr) { + SH7750State *s = opaque; + switch (addr) { + case SH7750_STBCR_A7: + return s->stbcr; + case SH7750_STBCR2_A7: + return s->stbcr2; default: error_access("byte read", addr); abort(); @@ -318,14 +327,24 @@ static uint32_t sh7750_mem_readl(void *opaque, hwaddr addr) static void sh7750_mem_writeb(void *opaque, hwaddr addr, uint32_t mem_value) { + SH7750State *s = opaque; if (is_in_sdrmx(addr, 2) || is_in_sdrmx(addr, 3)) { ignore_access("byte write", addr); return; } - error_access("byte write", addr); - abort(); + switch (addr) { + case SH7750_STBCR_A7: + s->stbcr = mem_value; + return; + case SH7750_STBCR2_A7: + s->stbcr2 = mem_value; + return; + default: + error_access("byte write", addr); + abort(); + } } static void sh7750_mem_writew(void *opaque, hwaddr addr,
The new Linux SH7750 clock driver uses the registers for power-down mode control, causing a crash: byte read to SH7750_STBCR_A7 (0x000000001fc00004) not supported Aborted (core dumped) Fix this by adding support for the Standby Control Registers STBCR and STBCR2. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- [RFC PATCH v3 12/35] drivers/clk/renesas: clk-sh7750.c SH7750/7751 CPG driver. https://lore.kernel.org/all/a772e1b6de89af22057d3af31cc03dcad7964fc7.1697199949.git.ysato@users.sourceforge.jp Accesses to CLKSTP00 and CLKSTCLK00 (0xfe0a0000/0x1e0a0000 and 0xfe0a0008/0x1e0a0008) don't seem to cause any issues, although I can't see immediately where they are handled? --- hw/sh4/sh7750.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)