On 2 March 2016 at 05:27, Jean-Christophe Dubois <j...@tribudubois.net> wrote:
> This controller is also present in i.MX5X devices but they are not
> yet emulated by QEMU.
>
> Signed-off-by: Jean-Christophe Dubois <j...@tribudubois.net>
> ---
> +static void imx6_src_reset(DeviceState *dev)
> +{
> + IMX6SRCState *s = IMX6_SRC(dev);
> +
> + DPRINTF("\n");
> +
> + memset(s->regs, 0, SRC_MAX * sizeof(uint32_t));
memset(s->regs, 0, sizeof(s->regs));
is slightly better.
> +
> + /* Set reset values */
> + s->regs[SRC_SCR] = 0x521;
> + s->regs[SRC_SRSR] = 0x1;
> + s->regs[SRC_SIMR] = 0x1F;
> +}
> +
> +static uint64_t imx6_src_read(void *opaque, hwaddr offset, unsigned size)
> +{
> + uint32_t value = 0;
> + IMX6SRCState *s = (IMX6SRCState *)opaque;
> + uint32_t index = offset >> 2;
> +
> + if (index < SRC_MAX) {
> + value = s->regs[index];
> + } else {
> + qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad register at offset 0x%"
> + HWADDR_PRIx "\n", TYPE_IMX6_SRC, __func__, offset);
> +
> + }
> +
> + DPRINTF("reg[%s] => 0x%" PRIx32 "\n", imx6_src_reg_name(index), value);
> +
> + return (uint64_t)value;
This cast is unnecessary.
> +}
> +
> +static void imx6_src_write(void *opaque, hwaddr offset, uint64_t value,
> + unsigned size)
> +{
> + IMX6SRCState *s = (IMX6SRCState *)opaque;
> + uint32_t index = offset >> 2;
> + uint64_t change_mask;
> +
> + if (index >= SRC_MAX) {
> + qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad register at offset 0x%"
> + HWADDR_PRIx "\n", TYPE_IMX6_SRC, __func__, offset);
> + return;
> + }
> +
> + DPRINTF("reg[%s] <= 0x%" PRIx32 "\n", imx6_src_reg_name(index),
> + (uint32_t)value);
> +
> + change_mask = s->regs[index] ^ (uint32_t)value;
s->regs[] is 32 bit, and you've explicitly cast value to 32-bits,
so why is change_mask 64 bits?
> + if (EXTRACT(change_mask, CORE3_RST)) {
> + arm_reset_cpu(3);
> + clear_bit(CORE3_RST_SHIFT, &value);
> + }
> + if (EXTRACT(change_mask, SW_IPU2_RST)) {
> + /* We pretend the IPU2 is reseted */
"reset" (and below).
> + clear_bit(SW_IPU2_RST_SHIFT, &value);
> + }
> + if (EXTRACT(change_mask, SW_IPU1_RST)) {
> + /* We pretend the IPU1 is reseted */
> + clear_bit(SW_IPU1_RST_SHIFT, &value);
> + }
> + s->regs[index] = value;
> + break;
Otherwise
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
thanks
-- PMM
