On Mon, Oct 31, 2022 at 4:19 PM Philippe Mathieu-Daudé <phi...@linaro.org>
wrote:
> On 31/10/22 16:12, Philippe Mathieu-Daudé wrote:
> > On 31/10/22 12:54, Philippe Mathieu-Daudé wrote:
> >> From: Bernhard Beschow <shen...@gmail.com>
> >>
> >> Adds missing functionality to e500plat machine which increases the
> >> chance of given "real" firmware images to access SD cards.
> >>
> >> Signed-off-by: Bernhard Beschow <shen...@gmail.com>
> >> Message-Id: <20221018210146.193159-8-shen...@gmail.com>
> >> [PMD: Simplify using create_unimplemented_device("esdhc")]
> >> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> >> ---
> >> docs/system/ppc/ppce500.rst | 12 ++++++++++
> >> hw/ppc/Kconfig | 2 ++
> >> hw/ppc/e500.c | 48 ++++++++++++++++++++++++++++++++++++-
> >> hw/ppc/e500.h | 1 +
> >> hw/ppc/e500plat.c | 1 +
> >> 5 files changed, 63 insertions(+), 1 deletion(-)
> >
> >> @@ -992,6 +1018,26 @@ void ppce500_init(MachineState *machine)
> >> i2c = (I2CBus *)qdev_get_child_bus(dev, "i2c");
> >> i2c_slave_create_simple(i2c, "ds1338", RTC_REGS_OFFSET);
> >> + /* eSDHC */
> >> + if (pmc->has_esdhc) {
> >> + uint64_t sdhci_regsize;
> >> +
> >> + dev = qdev_new(TYPE_SYSBUS_SDHCI);
> >> + /*
> >> + * Compatible with:
> >> + * - SD Host Controller Specification Version 2.0 Part A2
> >> + */
> >> + qdev_prop_set_uint8(dev, "sd-spec-version", 2);
> >> + s = SYS_BUS_DEVICE(dev);
> >> + sysbus_realize_and_unref(s, &error_fatal);
> >> + sysbus_mmio_map(s, 0, pmc->ccsrbar_base +
> >> MPC85XX_ESDHC_REGS_OFFSET);
> >> + sysbus_connect_irq(s, 0, qdev_get_gpio_in(mpicdev,
> >> MPC85XX_ESDHC_IRQ));
> >> + sdhci_regsize = memory_region_size(sysbus_mmio_get_region(s,
> >> 0));
> >> + create_unimplemented_device("esdhc",
> >> + pmc->ccsrbar_base
> >> + + MPC85XX_ESDHC_REGS_OFFSET +
> >> sdhci_regsize,
> >> + MPC85XX_ESDHC_REGS_SIZE -
> >> sdhci_regsize);
> >> + }
> >
> > Since the UNIMP device has lower priority, we can simplify as:
> >
> > if (pmc->has_esdhc) {
> > create_unimplemented_device("esdhc",
> > pmc->ccsrbar_base
> > + MPC85XX_ESDHC_REGS_OFFSET,
> > MPC85XX_ESDHC_REGS_SIZE);
> >
> > dev = qdev_new(TYPE_SYSBUS_SDHCI);
> > /*
> > * Compatible with:
> > * - SD Host Controller Specification Version 2.0 Part A2
> > */
> > qdev_prop_set_uint8(dev, "sd-spec-version", 2);
> > s = SYS_BUS_DEVICE(dev);
> > sysbus_realize_and_unref(s, &error_fatal);
> > sysbus_mmio_map(s, 0, pmc->ccsrbar_base +
> MPC85XX_ESDHC_REGS_OFFSET);
>
memory_region_add_subregion(ccsr_addr_space, MPC85XX_ESDHC_REGS_OFFSET,
sysbus_mmio_get_region(s, 0));
seems to be equivalent, works as well and mimics other devices, e.g. i2c.
So perhaps use that?
>
> So the SDHCI is mapped inside the CCSR block. Better would be to map it
> into ccsr_addr_space.
>
Doesn't the above code map it into ccsr_addr_space?
>
> I presume the CCSR is the device responsible of endian swapping, but TBH
> I have no clue about this board.
>
>
