From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=3.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 55C2CC83000 for ; Wed, 29 Apr 2020 19:16:30 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F229F21D82 for ; Wed, 29 Apr 2020 19:16:29 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="UkpJ9TZR" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F229F21D82 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:49514 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jTsC4-0005Xy-Gx for qemu-devel@archiver.kernel.org; Wed, 29 Apr 2020 15:16:29 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:42742) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jTsA1-00040T-DX for qemu-devel@nongnu.org; Wed, 29 Apr 2020 15:14:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.90_1) (envelope-from ) id 1jTs9w-0005DW-29 for qemu-devel@nongnu.org; Wed, 29 Apr 2020 15:14:20 -0400 Received: from mail-io1-xd43.google.com ([2607:f8b0:4864:20::d43]:43943) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jTs9u-0005Cb-HU for qemu-devel@nongnu.org; Wed, 29 Apr 2020 15:14:15 -0400 Received: by mail-io1-xd43.google.com with SMTP id 19so3453904ioz.10 for ; Wed, 29 Apr 2020 12:14:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=F3Uj8NYvbKVQOGRc4G/gk2DIE8ze9Dqd3vOMeHS0KE0=; b=UkpJ9TZR5NCKAPQpeqrSDkcKHJ2TbaSWuDdVJAF1/QDzzjnTgc3tTwHTT/IPSfBIze aTVYHS7OLCEGwaWmxpuu+ZFn4ytNy79NfhnHTexTLZ9/d1Ff3rw9upl9pPE7M57Eyy9X IepGXD58qgMCn/4mpKaSu42j58LmQNczFS3k34IbJK8lZk6qcXpPPxCoc5wRs31biiFU LkhzfJSPf9Z1e2jht3CpNfQFk2+ueYj1ywIn0qn0s6IyhxN8OuOYTqRMYjZWKBP+td7t d9FAvRgtJRqqvs7ZFWu7mYytlDAfHkc0McOkpG0SNYjzlJ47GT2IaOLFuamxGQz2i/vd TCqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=F3Uj8NYvbKVQOGRc4G/gk2DIE8ze9Dqd3vOMeHS0KE0=; b=udf0c62f5w9bgR4c4QrjGEUmcXup0M0HBgY1A5kE4ITOVbwvf8nNntyTSdfAhgcegU inLOkEjRvti0VScbw8yHYNcQvIZ7gvu9M/WKgnFbhGWJ6d64NhM2ckdWQM2ABKHWzEzA +2P0ETs54CdGWcRi+y/WHVKo6+k2QSWUYkqzPGdm8XNSB0UEmXyWmdSZNh421Z6hQwtm GzemFAGKZgAYjZOXoD8HKdS8TvPgqACP5e1UVTKNXECcI5C3eIaCVeHhta95f8QU1l+v sZfUAOaIf5R35jtbCjIAB9gG2tqdL9Y7mtC7J68ra8XEbgptYoCZRw4lHq9TIJfutOOS 9M5w== X-Gm-Message-State: AGi0Pua3o5/yhGbv7rlRTjSIEbsBQkXAektBtih8PR4raENEwB9dITyl +G7syWU/I0YbF6zrtfmVBfQJNVjnX7Gv2QeuW9A= X-Google-Smtp-Source: APiQypJADtqa3gVdZ/Ws5ibwMsoiFTIBcN53sUVZKAaazn2pua/qKDaEv1G2BwY7iIJ1as3I5Kxqe7r4UBW/PeLPyYQ= X-Received: by 2002:a02:9f13:: with SMTP id z19mr30836563jal.29.1588187650260; Wed, 29 Apr 2020 12:14:10 -0700 (PDT) MIME-Version: 1.0 References: <20200428022232.18875-1-pauldzim@gmail.com> <20200428022232.18875-2-pauldzim@gmail.com> In-Reply-To: From: Paul Zimmerman Date: Wed, 29 Apr 2020 12:13:43 -0700 Message-ID: Subject: Re: [PATCH v4 1/7] raspi: add BCM2835 SOC MPHI emulation To: =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= Content-Type: multipart/alternative; boundary="000000000000bb06f805a472c0b7" Received-SPF: pass client-ip=2607:f8b0:4864:20::d43; envelope-from=pauldzim@gmail.com; helo=mail-io1-xd43.google.com X-detected-operating-system: by eggs.gnu.org: Error: [-] PROGRAM ABORT : Malformed IPv6 address (bad octet value). Location : parse_addr6(), p0f-client.c:67 X-Received-From: 2607:f8b0:4864:20::d43 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: QEMU Developers , Peter Maydell , John Snow , Gerd Hoffmann , Stefan Hajnoczi Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --000000000000bb06f805a472c0b7 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Phil, On Tue, Apr 28, 2020 at 12:06 AM Philippe Mathieu-Daud=C3=A9 wrote: > Hi Paul, > > On 4/28/20 4:22 AM, Paul Zimmerman wrote: > > Add BCM2835 SOC MPHI (Message-based Parallel Host Interface) > > emulation. It is very basic, only providing the FIQ interrupt > > needed to allow the dwc-otg USB host controller driver in the > > Raspbian kernel to function. > > > > Signed-off-by: Paul Zimmerman > > --- > > hw/arm/bcm2835_peripherals.c | 17 +++ > > hw/misc/Makefile.objs | 1 + > > hw/misc/bcm2835_mphi.c | 190 +++++++++++++++++++++++++++ > > include/hw/arm/bcm2835_peripherals.h | 2 + > > include/hw/misc/bcm2835_mphi.h | 48 +++++++ > > 5 files changed, 258 insertions(+) > > create mode 100644 hw/misc/bcm2835_mphi.c > > create mode 100644 include/hw/misc/bcm2835_mphi.h > > > > diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.= c > > index edcaa4916d..5e2c832d95 100644 > > --- a/hw/arm/bcm2835_peripherals.c > > +++ b/hw/arm/bcm2835_peripherals.c > > @@ -124,6 +124,10 @@ static void bcm2835_peripherals_init(Object *obj) > > sysbus_init_child_obj(obj, "gpio", &s->gpio, sizeof(s->gpio), > > TYPE_BCM2835_GPIO); > > > > + /* Mphi */ > > + sysbus_init_child_obj(obj, "mphi", &s->mphi, sizeof(s->mphi), > > + TYPE_BCM2835_MPHI); > > + > > object_property_add_const_link(OBJECT(&s->gpio), "sdbus-sdhci", > > OBJECT(&s->sdhci.sdbus), > &error_abort); > > object_property_add_const_link(OBJECT(&s->gpio), "sdbus-sdhost", > > @@ -368,6 +372,19 @@ static void bcm2835_peripherals_realize(DeviceStat= e > *dev, Error **errp) > > return; > > } > > > > + /* Mphi */ > > + object_property_set_bool(OBJECT(&s->mphi), true, "realized", &err)= ; > > + if (err) { > > + error_propagate(errp, err); > > + return; > > + } > > + > > + memory_region_add_subregion(&s->peri_mr, MPHI_OFFSET, > > + sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mphi), 0)); > > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->mphi), 0, > > + qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ, > > + INTERRUPT_HOSTPORT)); > > + > > create_unimp(s, &s->armtmr, "bcm2835-sp804", > ARMCTRL_TIMER0_1_OFFSET, 0x40); > > create_unimp(s, &s->cprman, "bcm2835-cprman", CPRMAN_OFFSET, > 0x1000); > > create_unimp(s, &s->a2w, "bcm2835-a2w", A2W_OFFSET, 0x1000); > > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs > > index 68aae2eabb..91085cc21b 100644 > > --- a/hw/misc/Makefile.objs > > +++ b/hw/misc/Makefile.objs > > @@ -57,6 +57,7 @@ common-obj-$(CONFIG_OMAP) +=3D omap_l4.o > > common-obj-$(CONFIG_OMAP) +=3D omap_sdrc.o > > common-obj-$(CONFIG_OMAP) +=3D omap_tap.o > > common-obj-$(CONFIG_RASPI) +=3D bcm2835_mbox.o > > +common-obj-$(CONFIG_RASPI) +=3D bcm2835_mphi.o > > common-obj-$(CONFIG_RASPI) +=3D bcm2835_property.o > > common-obj-$(CONFIG_RASPI) +=3D bcm2835_rng.o > > common-obj-$(CONFIG_RASPI) +=3D bcm2835_thermal.o > > diff --git a/hw/misc/bcm2835_mphi.c b/hw/misc/bcm2835_mphi.c > > new file mode 100644 > > index 0000000000..66fc4a9cd3 > > --- /dev/null > > +++ b/hw/misc/bcm2835_mphi.c > > @@ -0,0 +1,190 @@ > > +/* > > + * BCM2835 SOC MPHI emulation > > + * > > + * Very basic emulation, only providing the FIQ interrupt needed to > > + * allow the dwc-otg USB host controller driver in the Raspbian kernel > > + * to function. > > + * > > + * Copyright (c) 2020 Paul Zimmerman > > + * > > + * This program is free software; you can redistribute it and/or modif= y > > + * it under the terms of the GNU General Public License as published b= y > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + */ > > + > > +#include "qemu/osdep.h" > > +#include "qapi/error.h" > > +#include "hw/misc/bcm2835_mphi.h" > > +#include "migration/vmstate.h" > > +#include "qemu/error-report.h" > > +#include "qemu/main-loop.h" > > + > > +static inline void mphi_raise_irq(BCM2835MphiState *s) > > +{ > > + qemu_set_irq(s->irq, 1); > > +} > > + > > +static inline void mphi_lower_irq(BCM2835MphiState *s) > > +{ > > + qemu_set_irq(s->irq, 0); > > +} > > + > > +static uint64_t mphi_reg_read(void *ptr, hwaddr addr, unsigned size) > > +{ > > + BCM2835MphiState *s =3D ptr; > > + uint32_t reg =3D s->regbase + addr; > > + uint32_t val =3D 0; > > + > > + switch (reg) { > > + case 0x28: /* outdda */ > > + val =3D s->outdda; > > + break; > > + case 0x2c: /* outddb */ > > + val =3D s->outddb; > > + break; > > + case 0x4c: /* ctrl */ > > + val =3D s->ctrl; > > + val |=3D 1 << 17; > > + break; > > + case 0x50: /* intstat */ > > + val =3D s->intstat; > > + break; > > + case 0x1f0: /* swirq_set */ > > + val =3D s->swirq_set; > > + break; > > + case 0x1f4: /* swirq_clr */ > > + val =3D s->swirq_clr; > > + break; > > I'm surprised this register is read. > Maybe it=E2=80=99s not, I=E2=80=99ll check the driver code. > > + default: > > + break; > > + } > > + > > + return val; > > +} > > + > > +static void mphi_reg_write(void *ptr, hwaddr addr, uint64_t val, > unsigned size) > > +{ > > + BCM2835MphiState *s =3D ptr; > > + uint32_t reg =3D s->regbase + addr; > > + int do_irq =3D 0; > > + > > + val &=3D 0xffffffff; > > Using '.impl.max_access_size =3D 4' (see below) this line is not necessar= y. > Ok. I just copied this code from one of the USB drivers, I=E2=80=99ll have = a look into this. > > + > > + switch (reg) { > > + case 0x28: /* outdda */ > > + s->outdda =3D val; > > + break; > > + case 0x2c: /* outddb */ > > + s->outddb =3D val; > > + if (val & (1 << 29)) { > > + do_irq =3D 1; > > + } > > Any idea what outdda/b means? > The Raspbian driver has a comment about triggering a fake DMA transfer, so something to do with DMA. I should say that this emulation was 100% developed without any documentation, just reverse engineered from what the driver does. > > + break; > > + case 0x4c: /* ctrl */ > > + s->ctrl =3D val; > > + if (val & (1 << 16)) { > > Any idea what are bits 16/17 for? > > > + do_irq =3D -1; > > + } > > I suppose this register is INT_ENA (inverted?) > > > + break; > > + case 0x50: /* intstat */ > > + s->intstat =3D val; > > + if (val & ((1 << 16) | (1 << 29))) { > > + do_irq =3D -1; > > I suppose writting INT_STAT acknowledges interrupts. > > > + } > > + break; > > Here ... > > > + case 0x1f0: /* swirq_set */ > > + s->swirq_set =3D val; > > + do_irq =3D 1; > > + break; > > + case 0x1f4: /* swirq_clr */ > > + s->swirq_clr =3D val; > > + do_irq =3D -1; > > + break; > > ... we access the same register, 's->swirq'. > > 0x1f0 sets the bits: > > s->swirq =3D val; > > 0x1f4 clears the bits: > > s->swirq &=3D ~val; > > Then you could simplify with qemu_set_irq(s->irq, ... && s->swirq); > Yep I think you=E2=80=99re right, I=E2=80=99ll do that. > > + default: > > Please log unimplemented accesses: > > qemu_log_mask(LOG_UNIMP, ...); > Ah, I was wondering about the right way to do that, thanks. > > + break; > > return? > > > + } > > + > > + if (do_irq > 0) { > > + mphi_raise_irq(s); > > + } else if (do_irq < 0) { > > + mphi_lower_irq(s); > > + } > > +} > > + > > +static const MemoryRegionOps mphi_mmio_ops =3D { > > + .read =3D mphi_reg_read, > > + .write =3D mphi_reg_write, > > + .valid.min_access_size =3D 4, > > + .valid.max_access_size =3D 4, > > I don't know what are the valid bus access sizes, but per your > implementation you want: > > .impl.min_access_size =3D 4, > .impl.max_access_size =3D 4, > > > + .endianness =3D DEVICE_LITTLE_ENDIAN, > > +}; > > + > > +static void mphi_reset(DeviceState *dev) > > +{ > > +} > > + > > +static void mphi_realize(DeviceState *dev, Error **errp) > > +{ > > + SysBusDevice *sbd =3D SYS_BUS_DEVICE(dev); > > + BCM2835MphiState *s =3D BCM2835_MPHI(dev); > > + > > + sysbus_init_irq(sbd, &s->irq); > > +} > > + > > +static void mphi_init(Object *obj) > > +{ > > + SysBusDevice *sbd =3D SYS_BUS_DEVICE(obj); > > + BCM2835MphiState *s =3D BCM2835_MPHI(obj); > > + > > + s->regbase =3D 0; > > + memory_region_init(&s->mem, obj, "mphi", MPHI_MMIO_SIZE); > > + memory_region_init_io(&s->mem_reg, obj, &mphi_mmio_ops, s, > "global", 0x200); > > + memory_region_add_subregion(&s->mem, s->regbase, &s->mem_reg); > > I'm not sure why you use a container. You can simplify using a single: > > memory_region_init_io(&s->iomem, obj, &mphi_mmio_ops, s, "mphi", > MPHI_MMIO_SIZE); OK > > + sysbus_init_mmio(sbd, &s->mem); > > +} > > + > > +const VMStateDescription vmstate_mphi_state =3D { > > + .name =3D "mphi", > > + .version_id =3D 1, > > + .minimum_version_id =3D 1, > > + .fields =3D (VMStateField[]) { > > + VMSTATE_UINT16(regbase, BCM2835MphiState), > > + VMSTATE_UINT32(outdda, BCM2835MphiState), > > + VMSTATE_UINT32(outddb, BCM2835MphiState), > > + VMSTATE_UINT32(ctrl, BCM2835MphiState), > > + VMSTATE_UINT32(intstat, BCM2835MphiState), > > + VMSTATE_UINT32(swirq_set, BCM2835MphiState), > > + VMSTATE_UINT32(swirq_clr, BCM2835MphiState), > > + VMSTATE_END_OF_LIST() > > + } > > +}; > > + > > +static void mphi_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *dc =3D DEVICE_CLASS(klass); > > + > > + dc->realize =3D mphi_realize; > > + dc->reset =3D mphi_reset; > > + dc->vmsd =3D &vmstate_mphi_state; > > +} > > + > > +static const TypeInfo bcm2835_mphi_type_info =3D { > > + .name =3D TYPE_BCM2835_MPHI, > > + .parent =3D TYPE_SYS_BUS_DEVICE, > > + .instance_size =3D sizeof(BCM2835MphiState), > > + .instance_init =3D mphi_init, > > + .class_init =3D mphi_class_init, > > +}; > > + > > +static void bcm2835_mphi_register_types(void) > > +{ > > + type_register_static(&bcm2835_mphi_type_info); > > +} > > + > > +type_init(bcm2835_mphi_register_types) > > diff --git a/include/hw/arm/bcm2835_peripherals.h > b/include/hw/arm/bcm2835_peripherals.h > > index 2e8655a7c2..7a7a8f6141 100644 > > --- a/include/hw/arm/bcm2835_peripherals.h > > +++ b/include/hw/arm/bcm2835_peripherals.h > > @@ -21,6 +21,7 @@ > > #include "hw/misc/bcm2835_property.h" > > #include "hw/misc/bcm2835_rng.h" > > #include "hw/misc/bcm2835_mbox.h" > > +#include "hw/misc/bcm2835_mphi.h" > > #include "hw/misc/bcm2835_thermal.h" > > #include "hw/sd/sdhci.h" > > #include "hw/sd/bcm2835_sdhost.h" > > @@ -42,6 +43,7 @@ typedef struct BCM2835PeripheralState { > > qemu_irq irq, fiq; > > > > BCM2835SystemTimerState systmr; > > + BCM2835MphiState mphi; > > UnimplementedDeviceState armtmr; > > UnimplementedDeviceState cprman; > > UnimplementedDeviceState a2w; > > diff --git a/include/hw/misc/bcm2835_mphi.h > b/include/hw/misc/bcm2835_mphi.h > > new file mode 100644 > > index 0000000000..6d070b04a5 > > --- /dev/null > > +++ b/include/hw/misc/bcm2835_mphi.h > > @@ -0,0 +1,48 @@ > > +/* > > + * BCM2835 SOC MPHI state definitions > > + * > > + * Copyright (c) 2020 Paul Zimmerman > > + * > > + * This program is free software; you can redistribute it and/or modif= y > > + * it under the terms of the GNU General Public License as published b= y > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + */ > > + > > +#ifndef HW_MISC_BCM2835_MPHI_H > > +#define HW_MISC_BCM2835_MPHI_H > > + > > +#include "hw/irq.h" > > +#include "hw/sysbus.h" > > +#include "sysemu/dma.h" > > sysemu/dma.h not used. > Will remove. > > > + > > +#define MPHI_MMIO_SIZE 0x1000 > > + > > +typedef struct BCM2835MphiState BCM2835MphiState; > > + > > +struct BCM2835MphiState { > > + SysBusDevice parent_obj; > > + qemu_irq irq; > > + MemoryRegion mem; > > + MemoryRegion mem_reg; > > + uint16_t regbase; > > You can remove regbase. > Right, thanks, > > + > > + uint32_t outdda; > > + uint32_t outddb; > > + uint32_t ctrl; > > + uint32_t intstat; > > + uint32_t swirq_set; > > + uint32_t swirq_clr; > > As suggested previously, you can use a single 'swirq' register. > > > +}; > > + > > +#define TYPE_BCM2835_MPHI "bcm2835-mphi" > > + > > +#define BCM2835_MPHI(obj) \ > > + OBJECT_CHECK(BCM2835MphiState, (obj), TYPE_BCM2835_MPHI) > > + > > +#endif > > > > I made a lot of picky comments, mostly to simplify, but this patch is in > good shape otherwise (being aware we have no documentation on this > device). Maybe Peter/Gerd are OK to accept it as it (as it is your first > contribution). > > Thanks for the review! Paul Regards, > > Phil. > --000000000000bb06f805a472c0b7 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Phil,

On Tue, Apr 28, 2020 at 12:06 AM Philippe= Mathieu-Daud=C3=A9 <f4bug@amsat.org<= /a>> wrote:
Hi Paul,

On 4/28/20 4:22 AM, Paul Zimmerman wrote:
> Add BCM2835 SOC MPHI (Message-based Parallel Host Interface)
> emulation. It is very basic, only providing the FIQ interrupt
> needed to allow the dwc-otg USB host controller driver in the
> Raspbian kernel to function.
>
> Signed-off-by: Paul Zimmerman <pauldzim@gmail.com>
> ---
>=C2=A0 hw/arm/bcm2835_peripherals.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|= =C2=A0 17 +++
>=C2=A0 hw/misc/Makefile.objs=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 |=C2=A0 =C2=A01 +
>=C2=A0 hw/misc/bcm2835_mphi.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0| 190 +++++++++++++++++++++++++++
>=C2=A0 include/hw/arm/bcm2835_peripherals.h |=C2=A0 =C2=A02 +
>=C2=A0 include/hw/misc/bcm2835_mphi.h=C2=A0 =C2=A0 =C2=A0 =C2=A0|=C2=A0= 48 +++++++
>=C2=A0 5 files changed, 258 insertions(+)
>=C2=A0 create mode 100644 hw/misc/bcm2835_mphi.c
>=C2=A0 create mode 100644 include/hw/misc/bcm2835_mphi.h
>
> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals= .c
> index edcaa4916d..5e2c832d95 100644
> --- a/hw/arm/bcm2835_peripherals.c
> +++ b/hw/arm/bcm2835_peripherals.c
> @@ -124,6 +124,10 @@ static void bcm2835_peripherals_init(Object *obj)=
>=C2=A0 =C2=A0 =C2=A0 sysbus_init_child_obj(obj, "gpio", &= s->gpio, sizeof(s->gpio),
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 TYPE_BCM2835_GPIO);
>=C2=A0
> +=C2=A0 =C2=A0 /* Mphi */
> +=C2=A0 =C2=A0 sysbus_init_child_obj(obj, "mphi", &s->= ;mphi, sizeof(s->mphi),
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 TYPE_BCM2835_MPHI);
> +
>=C2=A0 =C2=A0 =C2=A0 object_property_add_const_link(OBJECT(&s->g= pio), "sdbus-sdhci",
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0OBJECT(&s= ->sdhci.sdbus), &error_abort);
>=C2=A0 =C2=A0 =C2=A0 object_property_add_const_link(OBJECT(&s->g= pio), "sdbus-sdhost",
> @@ -368,6 +372,19 @@ static void bcm2835_peripherals_realize(DeviceSta= te *dev, Error **errp)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
>=C2=A0 =C2=A0 =C2=A0 }
>=C2=A0
> +=C2=A0 =C2=A0 /* Mphi */
> +=C2=A0 =C2=A0 object_property_set_bool(OBJECT(&s->mphi), true,= "realized", &err);
> +=C2=A0 =C2=A0 if (err) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 error_propagate(errp, err);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 memory_region_add_subregion(&s->peri_mr, MPHI_OF= FSET,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sysbus_mmio_g= et_region(SYS_BUS_DEVICE(&s->mphi), 0));
> +=C2=A0 =C2=A0 sysbus_connect_irq(SYS_BUS_DEVICE(&s->mphi), 0,<= br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 qdev_get_gpio_in_named(DEVICE(&s->= ic), BCM2835_IC_GPU_IRQ,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0INTERRUPT_HOSTPORT));
> +
>=C2=A0 =C2=A0 =C2=A0 create_unimp(s, &s->armtmr, "bcm2835-s= p804", ARMCTRL_TIMER0_1_OFFSET, 0x40);
>=C2=A0 =C2=A0 =C2=A0 create_unimp(s, &s->cprman, "bcm2835-c= prman", CPRMAN_OFFSET, 0x1000);
>=C2=A0 =C2=A0 =C2=A0 create_unimp(s, &s->a2w, "bcm2835-a2w&= quot;, A2W_OFFSET, 0x1000);
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 68aae2eabb..91085cc21b 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -57,6 +57,7 @@ common-obj-$(CONFIG_OMAP) +=3D omap_l4.o
>=C2=A0 common-obj-$(CONFIG_OMAP) +=3D omap_sdrc.o
>=C2=A0 common-obj-$(CONFIG_OMAP) +=3D omap_tap.o
>=C2=A0 common-obj-$(CONFIG_RASPI) +=3D bcm2835_mbox.o
> +common-obj-$(CONFIG_RASPI) +=3D bcm2835_mphi.o
>=C2=A0 common-obj-$(CONFIG_RASPI) +=3D bcm2835_property.o
>=C2=A0 common-obj-$(CONFIG_RASPI) +=3D bcm2835_rng.o
>=C2=A0 common-obj-$(CONFIG_RASPI) +=3D bcm2835_thermal.o
> diff --git a/hw/misc/bcm2835_mphi.c b/hw/misc/bcm2835_mphi.c
> new file mode 100644
> index 0000000000..66fc4a9cd3
> --- /dev/null
> +++ b/hw/misc/bcm2835_mphi.c
> @@ -0,0 +1,190 @@
> +/*
> + * BCM2835 SOC MPHI emulation
> + *
> + * Very basic emulation, only providing the FIQ interrupt needed to > + * allow the dwc-otg USB host controller driver in the Raspbian kerne= l
> + * to function.
> + *
> + * Copyright (c) 2020 Paul Zimmerman <pauldzim@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modi= fy
> + * it under the terms of the GNU General Public License as published = by
> + * the Free Software Foundation; either version 2 of the License, or<= br> > + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/misc/bcm2835_mphi.h"
> +#include "migration/vmstate.h"
> +#include "qemu/error-report.h"
> +#include "qemu/main-loop.h"
> +
> +static inline void mphi_raise_irq(BCM2835MphiState *s)
> +{
> +=C2=A0 =C2=A0 qemu_set_irq(s->irq, 1);
> +}
> +
> +static inline void mphi_lower_irq(BCM2835MphiState *s)
> +{
> +=C2=A0 =C2=A0 qemu_set_irq(s->irq, 0);
> +}
> +
> +static uint64_t mphi_reg_read(void *ptr, hwaddr addr, unsigned size)<= br> > +{
> +=C2=A0 =C2=A0 BCM2835MphiState *s =3D ptr;
> +=C2=A0 =C2=A0 uint32_t reg =3D s->regbase + addr;
> +=C2=A0 =C2=A0 uint32_t val =3D 0;
> +
> +=C2=A0 =C2=A0 switch (reg) {
> +=C2=A0 =C2=A0 case 0x28:=C2=A0 /* outdda */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 val =3D s->outdda;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case 0x2c:=C2=A0 /* outddb */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 val =3D s->outddb;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case 0x4c:=C2=A0 /* ctrl */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 val =3D s->ctrl;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 val |=3D 1 << 17;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case 0x50:=C2=A0 /* intstat */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 val =3D s->intstat;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case 0x1f0: /* swirq_set */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 val =3D s->swirq_set;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case 0x1f4: /* swirq_clr */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 val =3D s->swirq_clr;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;

I'm surprised this register is read.

Maybe it=E2=80=99= s not, I=E2=80=99ll check the driver code.


> +=C2=A0 =C2=A0 default:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 return val;
> +}
> +
> +static void mphi_reg_write(void *ptr, hwaddr addr, uint64_t val, unsi= gned size)
> +{
> +=C2=A0 =C2=A0 BCM2835MphiState *s =3D ptr;
> +=C2=A0 =C2=A0 uint32_t reg =3D s->regbase + addr;
> +=C2=A0 =C2=A0 int do_irq =3D 0;
> +
> +=C2=A0 =C2=A0 val &=3D 0xffffffff;

Using '.impl.max_access_size =3D 4' (see below) this line is not ne= cessary.

Ok. I just copied this code from one of the USB = drivers, I=E2=80=99ll have a
look into this.


> +
> +=C2=A0 =C2=A0 switch (reg) {
> +=C2=A0 =C2=A0 case 0x28:=C2=A0 /* outdda */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->outdda =3D val;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case 0x2c:=C2=A0 /* outddb */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->outddb =3D val;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (val & (1 << 29)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 do_irq =3D 1;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }

Any idea what outdda/b means?
=C2=A0
The Raspbian driver has = a comment about triggering a fake DMA transfer,
so something to do with DMA. I shoul= d say that this emulation was 100%
developed without any documentation, just revers= e engineered from what
the driver does.


> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case 0x4c:=C2=A0 /* ctrl */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->ctrl =3D val;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (val & (1 << 16)) {

Any idea what are bits 16/17 for?

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 do_irq =3D -1;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }

I suppose this register is INT_ENA (inverted?)

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case 0x50:=C2=A0 /* intstat */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->intstat =3D val;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (val & ((1 << 16) | (1 <&= lt; 29))) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 do_irq =3D -1;

I suppose writting INT_STAT acknowledges interrupts.

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;

Here ...

> +=C2=A0 =C2=A0 case 0x1f0: /* swirq_set */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->swirq_set =3D val;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 do_irq =3D 1;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case 0x1f4: /* swirq_clr */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->swirq_clr =3D val;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 do_irq =3D -1;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;

... we access the same register, 's->swirq'.

0x1f0 sets the bits:

=C2=A0 s->swirq =3D val;

0x1f4 clears the bits:

=C2=A0 s->swirq &=3D ~val;

Then you could simplify with qemu_set_irq(s->irq, ... && s->s= wirq);

Yep I think you=E2=80=99re right, I=E2=80=99ll do t= hat.

=

> +=C2=A0 =C2=A0 default:

Please log unimplemented accesses:

=C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_log_mask(LOG_UNIMP, ...);
<= div>
= Ah, I was wondering about the right way to do that, thanks.


> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return?

> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 if (do_irq > 0) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 mphi_raise_irq(s);
> +=C2=A0 =C2=A0 } else if (do_irq < 0) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 mphi_lower_irq(s);
> +=C2=A0 =C2=A0 }
> +}
> +
> +static const MemoryRegionOps mphi_mmio_ops =3D {
> +=C2=A0 =C2=A0 .read =3D mphi_reg_read,
> +=C2=A0 =C2=A0 .write =3D mphi_reg_write,
> +=C2=A0 =C2=A0 .valid.min_access_size =3D 4,
> +=C2=A0 =C2=A0 .valid.max_access_size =3D 4,

I don't know what are the valid bus access sizes, but per your
implementation you want:

=C2=A0 =C2=A0 =C2=A0 =C2=A0.impl.min_access_size =3D 4,
=C2=A0 =C2=A0 =C2=A0 =C2=A0.impl.max_access_size =3D 4,

> +=C2=A0 =C2=A0 .endianness =3D DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static void mphi_reset(DeviceState *dev)
> +{
> +}
> +
> +static void mphi_realize(DeviceState *dev, Error **errp)
> +{
> +=C2=A0 =C2=A0 SysBusDevice *sbd =3D SYS_BUS_DEVICE(dev);
> +=C2=A0 =C2=A0 BCM2835MphiState *s =3D BCM2835_MPHI(dev);
> +
> +=C2=A0 =C2=A0 sysbus_init_irq(sbd, &s->irq);
> +}
> +
> +static void mphi_init(Object *obj)
> +{
> +=C2=A0 =C2=A0 SysBusDevice *sbd =3D SYS_BUS_DEVICE(obj);
> +=C2=A0 =C2=A0 BCM2835MphiState *s =3D BCM2835_MPHI(obj);
> +
> +=C2=A0 =C2=A0 s->regbase =3D 0;
> +=C2=A0 =C2=A0 memory_region_init(&s->mem, obj, "mphi"= ;, MPHI_MMIO_SIZE);
> +=C2=A0 =C2=A0 memory_region_init_io(&s->mem_reg, obj, &mph= i_mmio_ops, s, "global", 0x200);
> +=C2=A0 =C2=A0 memory_region_add_subregion(&s->mem, s->regba= se, &s->mem_reg);

I'm not sure why you use a container. You can simplify using a single:<= br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0memory_region_init_io(&s->iomem, obj, &am= p;mphi_mmio_ops, s, "mphi",
MPHI_MMIO_SIZE);

OK


> +=C2=A0 =C2=A0 sysbus_init_mmio(sbd, &s->mem);
> +}
> +
> +const VMStateDescription vmstate_mphi_state =3D {
> +=C2=A0 =C2=A0 .name =3D "mphi",
> +=C2=A0 =C2=A0 .version_id =3D 1,
> +=C2=A0 =C2=A0 .minimum_version_id =3D 1,
> +=C2=A0 =C2=A0 .fields =3D (VMStateField[]) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 VMSTATE_UINT16(regbase, BCM2835MphiState)= ,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 VMSTATE_UINT32(outdda, BCM2835MphiState),=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 VMSTATE_UINT32(outddb, BCM2835MphiState),=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 VMSTATE_UINT32(ctrl, BCM2835MphiState), > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 VMSTATE_UINT32(intstat, BCM2835MphiState)= ,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 VMSTATE_UINT32(swirq_set, BCM2835MphiStat= e),
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 VMSTATE_UINT32(swirq_clr, BCM2835MphiStat= e),
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 VMSTATE_END_OF_LIST()
> +=C2=A0 =C2=A0 }
> +};
> +
> +static void mphi_class_init(ObjectClass *klass, void *data)
> +{
> +=C2=A0 =C2=A0 DeviceClass *dc =3D DEVICE_CLASS(klass);
> +
> +=C2=A0 =C2=A0 dc->realize =3D mphi_realize;
> +=C2=A0 =C2=A0 dc->reset =3D mphi_reset;
> +=C2=A0 =C2=A0 dc->vmsd =3D &vmstate_mphi_state;
> +}
> +
> +static const TypeInfo bcm2835_mphi_type_info =3D {
> +=C2=A0 =C2=A0 .name=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D TYPE_BCM283= 5_MPHI,
> +=C2=A0 =C2=A0 .parent=C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D TYPE_SYS_BUS_DEV= ICE,
> +=C2=A0 =C2=A0 .instance_size =3D sizeof(BCM2835MphiState),
> +=C2=A0 =C2=A0 .instance_init =3D mphi_init,
> +=C2=A0 =C2=A0 .class_init=C2=A0 =C2=A0 =3D mphi_class_init,
> +};
> +
> +static void bcm2835_mphi_register_types(void)
> +{
> +=C2=A0 =C2=A0 type_register_static(&bcm2835_mphi_type_info);
> +}
> +
> +type_init(bcm2835_mphi_register_types)
> diff --git a/include/hw/arm/bcm2835_peripherals.h b/include/hw/arm/bcm= 2835_peripherals.h
> index 2e8655a7c2..7a7a8f6141 100644
> --- a/include/hw/arm/bcm2835_peripherals.h
> +++ b/include/hw/arm/bcm2835_peripherals.h
> @@ -21,6 +21,7 @@
>=C2=A0 #include "hw/misc/bcm2835_property.h"
>=C2=A0 #include "hw/misc/bcm2835_rng.h"
>=C2=A0 #include "hw/misc/bcm2835_mbox.h"
> +#include "hw/misc/bcm2835_mphi.h"
>=C2=A0 #include "hw/misc/bcm2835_thermal.h"
>=C2=A0 #include "hw/sd/sdhci.h"
>=C2=A0 #include "hw/sd/bcm2835_sdhost.h"
> @@ -42,6 +43,7 @@ typedef struct BCM2835PeripheralState {
>=C2=A0 =C2=A0 =C2=A0 qemu_irq irq, fiq;
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 BCM2835SystemTimerState systmr;
> +=C2=A0 =C2=A0 BCM2835MphiState mphi;
>=C2=A0 =C2=A0 =C2=A0 UnimplementedDeviceState armtmr;
>=C2=A0 =C2=A0 =C2=A0 UnimplementedDeviceState cprman;
>=C2=A0 =C2=A0 =C2=A0 UnimplementedDeviceState a2w;
> diff --git a/include/hw/misc/bcm2835_mphi.h b/include/hw/misc/bcm2835_= mphi.h
> new file mode 100644
> index 0000000000..6d070b04a5
> --- /dev/null
> +++ b/include/hw/misc/bcm2835_mphi.h
> @@ -0,0 +1,48 @@
> +/*
> + * BCM2835 SOC MPHI state definitions
> + *
> + * Copyright (c) 2020 Paul Zimmerman <pauldzim@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modi= fy
> + * it under the terms of the GNU General Public License as published = by
> + * the Free Software Foundation; either version 2 of the License, or<= br> > + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef HW_MISC_BCM2835_MPHI_H
> +#define HW_MISC_BCM2835_MPHI_H
> +
> +#include "hw/irq.h"
> +#include "hw/sysbus.h"
> +#include "sysemu/dma.h"

sysemu/dma.h not used.

Will remove.

> +
> +#define MPHI_MMIO_SIZE=C2=A0 =C2=A0 =C2=A0 0x1000
> +
> +typedef struct BCM2835MphiState BCM2835MphiState;
> +
> +struct BCM2835MphiState {
> +=C2=A0 =C2=A0 SysBusDevice parent_obj;
> +=C2=A0 =C2=A0 qemu_irq irq;
> +=C2=A0 =C2=A0 MemoryRegion mem;
> +=C2=A0 =C2=A0 MemoryRegion mem_reg;
> +=C2=A0 =C2=A0 uint16_t regbase;

You can remove regbase.

Right, thanks,


> +
> +=C2=A0 =C2=A0 uint32_t outdda;
> +=C2=A0 =C2=A0 uint32_t outddb;
> +=C2=A0 =C2=A0 uint32_t ctrl;
> +=C2=A0 =C2=A0 uint32_t intstat;
> +=C2=A0 =C2=A0 uint32_t swirq_set;
> +=C2=A0 =C2=A0 uint32_t swirq_clr;

As suggested previously, you can use a single 'swirq' register.

> +};
> +
> +#define TYPE_BCM2835_MPHI=C2=A0 =C2=A0"bcm2835-mphi"
> +
> +#define BCM2835_MPHI(obj) \
> +=C2=A0 =C2=A0 OBJECT_CHECK(BCM2835MphiState, (obj), TYPE_BCM2835_MPHI= )
> +
> +#endif
>

I made a lot of picky comments, mostly to simplify, but this patch is in good shape otherwise (being aware we have no documentation on this
device). Maybe Peter/Gerd are OK to accept it as it (as it is your first contribution).

Thank= s for the review!

Paul

Regards,

Phil.
--000000000000bb06f805a472c0b7--