| Message ID | 20171106154813.19936-12-andrew.smirnov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 6 November 2017 at 15:47, Andrey Smirnov <andrew.smirnov@gmail.com> wrote: > IP block found on several generations of i.MX family does not use > vanilla SDHCI implementation and it comes with a number of quirks. > > Introduce i.MX SDHCI subtype of SDHCI block to add code necessary to > support unmodified Linux guest driver. > > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: Jason Wang <jasowang@redhat.com> > Cc: Philippe Mathieu-Daudé <f4bug@amsat.org> > Cc: qemu-devel@nongnu.org > Cc: qemu-arm@nongnu.org > Cc: yurovsky@gmail.com > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> Hi. Mostly this looks ok; some comments below. > --- > hw/sd/sdhci-internal.h | 15 ++++++ > hw/sd/sdhci.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++- > include/hw/sd/sdhci.h | 8 ++++ > 3 files changed, 148 insertions(+), 2 deletions(-) > > diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h > index 161177cf39..2a1b4b06ee 100644 > --- a/hw/sd/sdhci-internal.h > +++ b/hw/sd/sdhci-internal.h > @@ -91,6 +91,8 @@ > #define SDHC_CTRL_ADMA2_32 0x10 > #define SDHC_CTRL_ADMA2_64 0x18 > #define SDHC_DMA_TYPE(x) ((x) & SDHC_CTRL_DMA_CHECK_MASK) > +#define SDHC_CTRL_4BITBUS 0x02 > +#define SDHC_CTRL_8BITBUS 0x20 > > /* R/W Power Control Register 0x0 */ > #define SDHC_PWRCON 0x29 > @@ -229,4 +231,17 @@ enum { > > extern const VMStateDescription sdhci_vmstate; > > + > +#define ESDHC_MIX_CTRL 0x48 > +#define ESDHC_VENDOR_SPEC 0xc0 > +#define ESDHC_DLL_CTRL 0x60 > + > +#define ESDHC_TUNING_CTRL 0xcc > +#define ESDHC_TUNE_CTRL_STATUS 0x68 > +#define ESDHC_WTMK_LVL 0x44 > + > +#define ESDHC_CTRL_4BITBUS (0x1 << 1) > +#define ESDHC_CTRL_8BITBUS (0x2 << 1) > + > + > #endif > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c > index 6d6a791ee9..f561cc44e3 100644 > --- a/hw/sd/sdhci.c > +++ b/hw/sd/sdhci.c > @@ -265,7 +265,8 @@ static void sdhci_send_command(SDHCIState *s) > } > } > > - if ((s->norintstsen & SDHC_NISEN_TRSCMP) && > + if (!(s->quirks & SDHCI_QUIRK_NO_BUSY_IRQ) && > + (s->norintstsen & SDHC_NISEN_TRSCMP) && > (s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY) { > s->norintsts |= SDHC_NIS_TRSCMP; > } > @@ -1191,6 +1192,8 @@ static void sdhci_initfn(SDHCIState *s) > > s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_raise_insertion_irq, s); > s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer, s); > + > + s->io_ops = &sdhci_mmio_ops; > } > > static void sdhci_uninitfn(SDHCIState *s) > @@ -1347,7 +1350,7 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp) > s->buf_maxsz = sdhci_get_fifolen(s); > s->fifo_buffer = g_malloc0(s->buf_maxsz); > sysbus_init_irq(sbd, &s->irq); > - memory_region_init_io(&s->iomem, OBJECT(s), &sdhci_mmio_ops, s, "sdhci", > + memory_region_init_io(&s->iomem, OBJECT(s), s->io_ops, s, "sdhci", > SDHC_REGISTERS_MAP_SIZE); > sysbus_init_mmio(sbd, &s->iomem); > } > @@ -1386,11 +1389,131 @@ static const TypeInfo sdhci_bus_info = { > .class_init = sdhci_bus_class_init, > }; > > +static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size) > +{ > + SDHCIState *s = SYSBUS_SDHCI(opaque); > + uint32_t ret; > + uint16_t hostctl; > + > + switch (offset) { > + default: > + return sdhci_read(opaque, offset, size); > + > + case SDHC_HOSTCTL: > + hostctl = SDHC_DMA_TYPE(s->hostctl) << 5; > + > + if (s->hostctl & SDHC_CTRL_8BITBUS) { > + hostctl |= ESDHC_CTRL_8BITBUS; > + } > + > + if (s->hostctl & SDHC_CTRL_4BITBUS) { > + hostctl |= ESDHC_CTRL_4BITBUS; > + } > + > + ret = hostctl | (s->blkgap << 16) | > + (s->wakcon << 24); > + > + break; > + > + case ESDHC_DLL_CTRL: > + case ESDHC_TUNE_CTRL_STATUS: > + case 0x6c: > + case ESDHC_TUNING_CTRL: > + case ESDHC_VENDOR_SPEC: > + case ESDHC_MIX_CTRL: > + case ESDHC_WTMK_LVL: > + ret = 0; > + break; > + } > + > + return ret; > +} > + > +static void > +usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size) > +{ > + SDHCIState *s = SYSBUS_SDHCI(opaque); > + uint8_t hostctl = 0; > + uint32_t value = (uint32_t)val; > + > + switch (offset) { > + case ESDHC_DLL_CTRL: > + case ESDHC_TUNE_CTRL_STATUS: > + case 0x6c: > + case ESDHC_TUNING_CTRL: > + case ESDHC_WTMK_LVL: > + case ESDHC_VENDOR_SPEC: > + break; > + > + case SDHC_HOSTCTL: > + if (value & ESDHC_CTRL_8BITBUS) { > + hostctl |= SDHC_CTRL_8BITBUS; > + } > + > + if (value & ESDHC_CTRL_4BITBUS) { > + hostctl |= ESDHC_CTRL_4BITBUS; > + } > + > + hostctl |= SDHC_DMA_TYPE(value >> 5); > + > + value &= ~0xFE; > + value |= hostctl; > + value &= ~0xFF00; > + value |= s->pwrcon; > + > + sdhci_write(opaque, offset, value, size); This is pretty confusing, because we both mess about directly with the pwrcon field and also pass the write through to sdhci_write(). (a) some comments would help and (b) would it be clearer to just implement the different SDHC_HOSTCTL behaviour entirely here rather than trying to reuse the code in sdhci_write()? I get the impression that at least some of this is trying to shuffle stuff around so that code can unshuffle it. > + break; > + > + case ESDHC_MIX_CTRL: > + /* > + * The layout of the register is slightly different, but we > + * don't care about those bits > + */ > + s->trnmod = value & 0xFFFF; > + break; > + case SDHC_TRNMOD: > + sdhci_write(opaque, offset, val | s->trnmod, size); This one's simpler, but a comment would assist. > + break; > + case SDHC_BLKSIZE: > + val |= 0x7 << 12; > + default: /* FALLTHROUGH */ > + sdhci_write(opaque, offset, val, size); > + break; > + } > +} > + > + > +static const MemoryRegionOps usdhc_mmio_ops = { > + .read = usdhc_read, > + .write = usdhc_write, > + .valid = { > + .min_access_size = 1, > + .max_access_size = 4, > + .unaligned = false > + }, > + .endianness = DEVICE_LITTLE_ENDIAN, > +}; > + > +static void imx_usdhc_init(Object *obj) > +{ > + SDHCIState *s = SYSBUS_SDHCI(obj); > + > + s->io_ops = &usdhc_mmio_ops; > + s->quirks = SDHCI_QUIRK_NO_BUSY_IRQ; > +} > + > +static const TypeInfo imx_usdhc_info = { > + .name = TYPE_IMX_USDHC, > + .parent = TYPE_SYSBUS_SDHCI, > + .instance_init = imx_usdhc_init, > +}; > + > static void sdhci_register_types(void) > { > type_register_static(&sdhci_pci_info); > type_register_static(&sdhci_sysbus_info); > type_register_static(&sdhci_bus_info); > + type_register_static(&imx_usdhc_info); > } > > type_init(sdhci_register_types) > diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h > index 0f0c3f1e64..dc1856a33d 100644 > --- a/include/hw/sd/sdhci.h > +++ b/include/hw/sd/sdhci.h > @@ -39,6 +39,7 @@ typedef struct SDHCIState { > }; > SDBus sdbus; > MemoryRegion iomem; > + const MemoryRegionOps *io_ops; > > QEMUTimer *insert_timer; /* timer for 'changing' sd card. */ > QEMUTimer *transfer_timer; > @@ -83,8 +84,13 @@ typedef struct SDHCIState { > /* Force Event Auto CMD12 Error Interrupt Reg - write only */ > /* Force Event Error Interrupt Register- write only */ > /* RO Host Controller Version Register always reads as 0x2401 */ > + > + unsigned long quirks; Don't use 'unsigned long', it differs in size from host to host. > } SDHCIState; > > +/* Controller does not provide transfer-complete interrupt when not busy */ > +#define SDHCI_QUIRK_NO_BUSY_IRQ BIT(14) We only have one quirk, so why is it bit 14? > + > #define TYPE_PCI_SDHCI "sdhci-pci" > #define PCI_SDHCI(obj) OBJECT_CHECK(SDHCIState, (obj), TYPE_PCI_SDHCI) > > @@ -92,4 +98,6 @@ typedef struct SDHCIState { > #define SYSBUS_SDHCI(obj) \ > OBJECT_CHECK(SDHCIState, (obj), TYPE_SYSBUS_SDHCI) > > +#define TYPE_IMX_USDHC "imx-usdhc" > + > #endif /* SDHCI_H */ > -- > 2.13.6 thanks -- PMM
On Tue, Nov 21, 2017 at 10:02 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 6 November 2017 at 15:47, Andrey Smirnov <andrew.smirnov@gmail.com> wrote: >> IP block found on several generations of i.MX family does not use >> vanilla SDHCI implementation and it comes with a number of quirks. >> >> Introduce i.MX SDHCI subtype of SDHCI block to add code necessary to >> support unmodified Linux guest driver. >> >> Cc: Peter Maydell <peter.maydell@linaro.org> >> Cc: Jason Wang <jasowang@redhat.com> >> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org> >> Cc: qemu-devel@nongnu.org >> Cc: qemu-arm@nongnu.org >> Cc: yurovsky@gmail.com >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> > > Hi. Mostly this looks ok; some comments below. > >> --- >> hw/sd/sdhci-internal.h | 15 ++++++ >> hw/sd/sdhci.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++- >> include/hw/sd/sdhci.h | 8 ++++ >> 3 files changed, 148 insertions(+), 2 deletions(-) >> >> diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h >> index 161177cf39..2a1b4b06ee 100644 >> --- a/hw/sd/sdhci-internal.h >> +++ b/hw/sd/sdhci-internal.h >> @@ -91,6 +91,8 @@ >> #define SDHC_CTRL_ADMA2_32 0x10 >> #define SDHC_CTRL_ADMA2_64 0x18 >> #define SDHC_DMA_TYPE(x) ((x) & SDHC_CTRL_DMA_CHECK_MASK) >> +#define SDHC_CTRL_4BITBUS 0x02 >> +#define SDHC_CTRL_8BITBUS 0x20 >> >> /* R/W Power Control Register 0x0 */ >> #define SDHC_PWRCON 0x29 >> @@ -229,4 +231,17 @@ enum { >> >> extern const VMStateDescription sdhci_vmstate; >> >> + >> +#define ESDHC_MIX_CTRL 0x48 >> +#define ESDHC_VENDOR_SPEC 0xc0 >> +#define ESDHC_DLL_CTRL 0x60 >> + >> +#define ESDHC_TUNING_CTRL 0xcc >> +#define ESDHC_TUNE_CTRL_STATUS 0x68 >> +#define ESDHC_WTMK_LVL 0x44 >> + >> +#define ESDHC_CTRL_4BITBUS (0x1 << 1) >> +#define ESDHC_CTRL_8BITBUS (0x2 << 1) >> + >> + >> #endif >> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c >> index 6d6a791ee9..f561cc44e3 100644 >> --- a/hw/sd/sdhci.c >> +++ b/hw/sd/sdhci.c >> @@ -265,7 +265,8 @@ static void sdhci_send_command(SDHCIState *s) >> } >> } >> >> - if ((s->norintstsen & SDHC_NISEN_TRSCMP) && >> + if (!(s->quirks & SDHCI_QUIRK_NO_BUSY_IRQ) && >> + (s->norintstsen & SDHC_NISEN_TRSCMP) && >> (s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY) { >> s->norintsts |= SDHC_NIS_TRSCMP; >> } >> @@ -1191,6 +1192,8 @@ static void sdhci_initfn(SDHCIState *s) >> >> s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_raise_insertion_irq, s); >> s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer, s); >> + >> + s->io_ops = &sdhci_mmio_ops; >> } >> >> static void sdhci_uninitfn(SDHCIState *s) >> @@ -1347,7 +1350,7 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp) >> s->buf_maxsz = sdhci_get_fifolen(s); >> s->fifo_buffer = g_malloc0(s->buf_maxsz); >> sysbus_init_irq(sbd, &s->irq); >> - memory_region_init_io(&s->iomem, OBJECT(s), &sdhci_mmio_ops, s, "sdhci", >> + memory_region_init_io(&s->iomem, OBJECT(s), s->io_ops, s, "sdhci", >> SDHC_REGISTERS_MAP_SIZE); >> sysbus_init_mmio(sbd, &s->iomem); >> } >> @@ -1386,11 +1389,131 @@ static const TypeInfo sdhci_bus_info = { >> .class_init = sdhci_bus_class_init, >> }; >> >> +static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size) >> +{ >> + SDHCIState *s = SYSBUS_SDHCI(opaque); >> + uint32_t ret; >> + uint16_t hostctl; >> + >> + switch (offset) { >> + default: >> + return sdhci_read(opaque, offset, size); >> + >> + case SDHC_HOSTCTL: >> + hostctl = SDHC_DMA_TYPE(s->hostctl) << 5; >> + >> + if (s->hostctl & SDHC_CTRL_8BITBUS) { >> + hostctl |= ESDHC_CTRL_8BITBUS; >> + } >> + >> + if (s->hostctl & SDHC_CTRL_4BITBUS) { >> + hostctl |= ESDHC_CTRL_4BITBUS; >> + } >> + >> + ret = hostctl | (s->blkgap << 16) | >> + (s->wakcon << 24); >> + >> + break; >> + >> + case ESDHC_DLL_CTRL: >> + case ESDHC_TUNE_CTRL_STATUS: >> + case 0x6c: >> + case ESDHC_TUNING_CTRL: >> + case ESDHC_VENDOR_SPEC: >> + case ESDHC_MIX_CTRL: >> + case ESDHC_WTMK_LVL: >> + ret = 0; >> + break; >> + } >> + >> + return ret; >> +} >> + >> +static void >> +usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size) >> +{ >> + SDHCIState *s = SYSBUS_SDHCI(opaque); >> + uint8_t hostctl = 0; >> + uint32_t value = (uint32_t)val; >> + >> + switch (offset) { >> + case ESDHC_DLL_CTRL: >> + case ESDHC_TUNE_CTRL_STATUS: >> + case 0x6c: >> + case ESDHC_TUNING_CTRL: >> + case ESDHC_WTMK_LVL: >> + case ESDHC_VENDOR_SPEC: >> + break; >> + >> + case SDHC_HOSTCTL: >> + if (value & ESDHC_CTRL_8BITBUS) { >> + hostctl |= SDHC_CTRL_8BITBUS; >> + } >> + >> + if (value & ESDHC_CTRL_4BITBUS) { >> + hostctl |= ESDHC_CTRL_4BITBUS; >> + } >> + >> + hostctl |= SDHC_DMA_TYPE(value >> 5); >> + >> + value &= ~0xFE; >> + value |= hostctl; >> + value &= ~0xFF00; >> + value |= s->pwrcon; >> + >> + sdhci_write(opaque, offset, value, size); > > This is pretty confusing, because we both mess about directly > with the pwrcon field and also pass the write through > to sdhci_write(). (a) some comments would help and Sure, will do. > (b) would it be clearer to just implement the different > SDHC_HOSTCTL behaviour entirely here rather than trying to > reuse the code in sdhci_write()? I get the impression that > at least some of this is trying to shuffle stuff around so > that code can unshuffle it. > Main reason I did this is because those bit transformations are the inverse (or at least should be) of what Linux driver does in its "SDHCI -> ESDHC" conversion code. >> + break; >> + >> + case ESDHC_MIX_CTRL: >> + /* >> + * The layout of the register is slightly different, but we >> + * don't care about those bits >> + */ >> + s->trnmod = value & 0xFFFF; >> + break; >> + case SDHC_TRNMOD: >> + sdhci_write(opaque, offset, val | s->trnmod, size); > > This one's simpler, but a comment would assist. > OK, will add. >> + break; >> + case SDHC_BLKSIZE: >> + val |= 0x7 << 12; >> + default: /* FALLTHROUGH */ >> + sdhci_write(opaque, offset, val, size); >> + break; >> + } >> +} >> + >> + >> +static const MemoryRegionOps usdhc_mmio_ops = { >> + .read = usdhc_read, >> + .write = usdhc_write, >> + .valid = { >> + .min_access_size = 1, >> + .max_access_size = 4, >> + .unaligned = false >> + }, >> + .endianness = DEVICE_LITTLE_ENDIAN, >> +}; >> + >> +static void imx_usdhc_init(Object *obj) >> +{ >> + SDHCIState *s = SYSBUS_SDHCI(obj); >> + >> + s->io_ops = &usdhc_mmio_ops; >> + s->quirks = SDHCI_QUIRK_NO_BUSY_IRQ; >> +} >> + >> +static const TypeInfo imx_usdhc_info = { >> + .name = TYPE_IMX_USDHC, >> + .parent = TYPE_SYSBUS_SDHCI, >> + .instance_init = imx_usdhc_init, >> +}; >> + >> static void sdhci_register_types(void) >> { >> type_register_static(&sdhci_pci_info); >> type_register_static(&sdhci_sysbus_info); >> type_register_static(&sdhci_bus_info); >> + type_register_static(&imx_usdhc_info); >> } >> >> type_init(sdhci_register_types) >> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h >> index 0f0c3f1e64..dc1856a33d 100644 >> --- a/include/hw/sd/sdhci.h >> +++ b/include/hw/sd/sdhci.h >> @@ -39,6 +39,7 @@ typedef struct SDHCIState { >> }; >> SDBus sdbus; >> MemoryRegion iomem; >> + const MemoryRegionOps *io_ops; >> >> QEMUTimer *insert_timer; /* timer for 'changing' sd card. */ >> QEMUTimer *transfer_timer; >> @@ -83,8 +84,13 @@ typedef struct SDHCIState { >> /* Force Event Auto CMD12 Error Interrupt Reg - write only */ >> /* Force Event Error Interrupt Register- write only */ >> /* RO Host Controller Version Register always reads as 0x2401 */ >> + >> + unsigned long quirks; > > Don't use 'unsigned long', it differs in size from host to host. > OK, sure. >> } SDHCIState; >> >> +/* Controller does not provide transfer-complete interrupt when not busy */ >> +#define SDHCI_QUIRK_NO_BUSY_IRQ BIT(14) > > We only have one quirk, so why is it bit 14? > I took that code from Linux kernel, so I tried to keep it as is (same with unsigned long in quirks field). Thanks, Andrey Smirnov
On 22 November 2017 at 20:43, Andrey Smirnov <andrew.smirnov@gmail.com> wrote: > On Tue, Nov 21, 2017 at 10:02 AM, Peter Maydell > <peter.maydell@linaro.org> wrote: >> On 6 November 2017 at 15:47, Andrey Smirnov <andrew.smirnov@gmail.com> wrote: >>> +/* Controller does not provide transfer-complete interrupt when not busy */ >>> +#define SDHCI_QUIRK_NO_BUSY_IRQ BIT(14) >> >> We only have one quirk, so why is it bit 14? >> > > I took that code from Linux kernel, so I tried to keep it as is (same > with unsigned long in quirks field). That's fine, but if so we should have a comment that that's what we're doing, so that subsequent additions by other contributors follow the same convention. thanks -- PMM
diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h index 161177cf39..2a1b4b06ee 100644 --- a/hw/sd/sdhci-internal.h +++ b/hw/sd/sdhci-internal.h @@ -91,6 +91,8 @@ #define SDHC_CTRL_ADMA2_32 0x10 #define SDHC_CTRL_ADMA2_64 0x18 #define SDHC_DMA_TYPE(x) ((x) & SDHC_CTRL_DMA_CHECK_MASK) +#define SDHC_CTRL_4BITBUS 0x02 +#define SDHC_CTRL_8BITBUS 0x20 /* R/W Power Control Register 0x0 */ #define SDHC_PWRCON 0x29 @@ -229,4 +231,17 @@ enum { extern const VMStateDescription sdhci_vmstate; + +#define ESDHC_MIX_CTRL 0x48 +#define ESDHC_VENDOR_SPEC 0xc0 +#define ESDHC_DLL_CTRL 0x60 + +#define ESDHC_TUNING_CTRL 0xcc +#define ESDHC_TUNE_CTRL_STATUS 0x68 +#define ESDHC_WTMK_LVL 0x44 + +#define ESDHC_CTRL_4BITBUS (0x1 << 1) +#define ESDHC_CTRL_8BITBUS (0x2 << 1) + + #endif diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 6d6a791ee9..f561cc44e3 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -265,7 +265,8 @@ static void sdhci_send_command(SDHCIState *s) } } - if ((s->norintstsen & SDHC_NISEN_TRSCMP) && + if (!(s->quirks & SDHCI_QUIRK_NO_BUSY_IRQ) && + (s->norintstsen & SDHC_NISEN_TRSCMP) && (s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY) { s->norintsts |= SDHC_NIS_TRSCMP; } @@ -1191,6 +1192,8 @@ static void sdhci_initfn(SDHCIState *s) s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_raise_insertion_irq, s); s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer, s); + + s->io_ops = &sdhci_mmio_ops; } static void sdhci_uninitfn(SDHCIState *s) @@ -1347,7 +1350,7 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp) s->buf_maxsz = sdhci_get_fifolen(s); s->fifo_buffer = g_malloc0(s->buf_maxsz); sysbus_init_irq(sbd, &s->irq); - memory_region_init_io(&s->iomem, OBJECT(s), &sdhci_mmio_ops, s, "sdhci", + memory_region_init_io(&s->iomem, OBJECT(s), s->io_ops, s, "sdhci", SDHC_REGISTERS_MAP_SIZE); sysbus_init_mmio(sbd, &s->iomem); } @@ -1386,11 +1389,131 @@ static const TypeInfo sdhci_bus_info = { .class_init = sdhci_bus_class_init, }; +static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size) +{ + SDHCIState *s = SYSBUS_SDHCI(opaque); + uint32_t ret; + uint16_t hostctl; + + switch (offset) { + default: + return sdhci_read(opaque, offset, size); + + case SDHC_HOSTCTL: + hostctl = SDHC_DMA_TYPE(s->hostctl) << 5; + + if (s->hostctl & SDHC_CTRL_8BITBUS) { + hostctl |= ESDHC_CTRL_8BITBUS; + } + + if (s->hostctl & SDHC_CTRL_4BITBUS) { + hostctl |= ESDHC_CTRL_4BITBUS; + } + + ret = hostctl | (s->blkgap << 16) | + (s->wakcon << 24); + + break; + + case ESDHC_DLL_CTRL: + case ESDHC_TUNE_CTRL_STATUS: + case 0x6c: + case ESDHC_TUNING_CTRL: + case ESDHC_VENDOR_SPEC: + case ESDHC_MIX_CTRL: + case ESDHC_WTMK_LVL: + ret = 0; + break; + } + + return ret; +} + +static void +usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size) +{ + SDHCIState *s = SYSBUS_SDHCI(opaque); + uint8_t hostctl = 0; + uint32_t value = (uint32_t)val; + + switch (offset) { + case ESDHC_DLL_CTRL: + case ESDHC_TUNE_CTRL_STATUS: + case 0x6c: + case ESDHC_TUNING_CTRL: + case ESDHC_WTMK_LVL: + case ESDHC_VENDOR_SPEC: + break; + + case SDHC_HOSTCTL: + if (value & ESDHC_CTRL_8BITBUS) { + hostctl |= SDHC_CTRL_8BITBUS; + } + + if (value & ESDHC_CTRL_4BITBUS) { + hostctl |= ESDHC_CTRL_4BITBUS; + } + + hostctl |= SDHC_DMA_TYPE(value >> 5); + + value &= ~0xFE; + value |= hostctl; + value &= ~0xFF00; + value |= s->pwrcon; + + sdhci_write(opaque, offset, value, size); + break; + + case ESDHC_MIX_CTRL: + /* + * The layout of the register is slightly different, but we + * don't care about those bits + */ + s->trnmod = value & 0xFFFF; + break; + case SDHC_TRNMOD: + sdhci_write(opaque, offset, val | s->trnmod, size); + break; + case SDHC_BLKSIZE: + val |= 0x7 << 12; + default: /* FALLTHROUGH */ + sdhci_write(opaque, offset, val, size); + break; + } +} + + +static const MemoryRegionOps usdhc_mmio_ops = { + .read = usdhc_read, + .write = usdhc_write, + .valid = { + .min_access_size = 1, + .max_access_size = 4, + .unaligned = false + }, + .endianness = DEVICE_LITTLE_ENDIAN, +}; + +static void imx_usdhc_init(Object *obj) +{ + SDHCIState *s = SYSBUS_SDHCI(obj); + + s->io_ops = &usdhc_mmio_ops; + s->quirks = SDHCI_QUIRK_NO_BUSY_IRQ; +} + +static const TypeInfo imx_usdhc_info = { + .name = TYPE_IMX_USDHC, + .parent = TYPE_SYSBUS_SDHCI, + .instance_init = imx_usdhc_init, +}; + static void sdhci_register_types(void) { type_register_static(&sdhci_pci_info); type_register_static(&sdhci_sysbus_info); type_register_static(&sdhci_bus_info); + type_register_static(&imx_usdhc_info); } type_init(sdhci_register_types) diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h index 0f0c3f1e64..dc1856a33d 100644 --- a/include/hw/sd/sdhci.h +++ b/include/hw/sd/sdhci.h @@ -39,6 +39,7 @@ typedef struct SDHCIState { }; SDBus sdbus; MemoryRegion iomem; + const MemoryRegionOps *io_ops; QEMUTimer *insert_timer; /* timer for 'changing' sd card. */ QEMUTimer *transfer_timer; @@ -83,8 +84,13 @@ typedef struct SDHCIState { /* Force Event Auto CMD12 Error Interrupt Reg - write only */ /* Force Event Error Interrupt Register- write only */ /* RO Host Controller Version Register always reads as 0x2401 */ + + unsigned long quirks; } SDHCIState; +/* Controller does not provide transfer-complete interrupt when not busy */ +#define SDHCI_QUIRK_NO_BUSY_IRQ BIT(14) + #define TYPE_PCI_SDHCI "sdhci-pci" #define PCI_SDHCI(obj) OBJECT_CHECK(SDHCIState, (obj), TYPE_PCI_SDHCI) @@ -92,4 +98,6 @@ typedef struct SDHCIState { #define SYSBUS_SDHCI(obj) \ OBJECT_CHECK(SDHCIState, (obj), TYPE_SYSBUS_SDHCI) +#define TYPE_IMX_USDHC "imx-usdhc" + #endif /* SDHCI_H */
IP block found on several generations of i.MX family does not use vanilla SDHCI implementation and it comes with a number of quirks. Introduce i.MX SDHCI subtype of SDHCI block to add code necessary to support unmodified Linux guest driver. Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Jason Wang <jasowang@redhat.com> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org> Cc: qemu-devel@nongnu.org Cc: qemu-arm@nongnu.org Cc: yurovsky@gmail.com Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> --- hw/sd/sdhci-internal.h | 15 ++++++ hw/sd/sdhci.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++- include/hw/sd/sdhci.h | 8 ++++ 3 files changed, 148 insertions(+), 2 deletions(-)