On Thu, Jun 14, 2018 at 09:51:33AM +0200, BALATON Zoltan wrote: > On Thu, 14 Jun 2018, David Gibson wrote: > > On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote: > > > Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu> > > > > Patch looks good, but it needs a commit message. What is the > > directcntl register? Now does the bitbang interface play into that? > > (Note that I know the answer to those questions right now, but it > > needs to be in the commit message for the benefit of people looking > > back in the future). > > If you know the answer could you please suggest an appropriate commit > message? I'm a bit lost what should be written here that would explain the > patch to someone who knows nothing about what this is. And for those who > know, the patch itself should be fairly simple and not need more > explanation.
It's not really about explaining it to someone ho knows nothing about
what it is. Rather it's about explaining to someone who has a general
familiarity but hasn't been looking at this code recently. Try
imagining explaining itself to yourself in several years time having
been working on something entirely different.
So, perhaps:
| As well as being able to generate its own i2c transactions, the
| ppc4xx i2c controller has a DIRECTCNTL register which allows
| explicit control of the i2c lines.
|
| Using this register an OS can directly bitbang i2c operations. In
| order to let emulated i2c devices respond to this, we need to wire
| up the DIRECTCNTL register to qemu's bitbanged i2c handling code.
>
> Regards,
> BALATON Zoltan
>
> > > ---
> > > default-configs/ppc-softmmu.mak | 1 +
> > > default-configs/ppcemb-softmmu.mak | 1 +
> > > hw/i2c/ppc4xx_i2c.c | 13 ++++++++++++-
> > > include/hw/i2c/ppc4xx_i2c.h | 4 ++++
> > > 4 files changed, 18 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/default-configs/ppc-softmmu.mak
> > > b/default-configs/ppc-softmmu.mak
> > > index 4d7be45..7d0dc2f 100644
> > > --- a/default-configs/ppc-softmmu.mak
> > > +++ b/default-configs/ppc-softmmu.mak
> > > @@ -26,6 +26,7 @@ CONFIG_USB_EHCI_SYSBUS=y
> > > CONFIG_SM501=y
> > > CONFIG_IDE_SII3112=y
> > > CONFIG_I2C=y
> > > +CONFIG_BITBANG_I2C=y
> > >
> > > # For Macs
> > > CONFIG_MAC=y
> > > diff --git a/default-configs/ppcemb-softmmu.mak
> > > b/default-configs/ppcemb-softmmu.mak
> > > index 67d18b2..37af193 100644
> > > --- a/default-configs/ppcemb-softmmu.mak
> > > +++ b/default-configs/ppcemb-softmmu.mak
> > > @@ -19,3 +19,4 @@ CONFIG_USB_EHCI_SYSBUS=y
> > > CONFIG_SM501=y
> > > CONFIG_IDE_SII3112=y
> > > CONFIG_I2C=y
> > > +CONFIG_BITBANG_I2C=y
> > > diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c
> > > index 4e0aaae..c0a1930 100644
> > > --- a/hw/i2c/ppc4xx_i2c.c
> > > +++ b/hw/i2c/ppc4xx_i2c.c
> > > @@ -30,6 +30,7 @@
> > > #include "cpu.h"
> > > #include "hw/hw.h"
> > > #include "hw/i2c/ppc4xx_i2c.h"
> > > +#include "bitbang_i2c.h"
> > >
> > > #define PPC4xx_I2C_MEM_SIZE 18
> > >
> > > @@ -46,6 +47,11 @@
> > >
> > > #define IIC_XTCNTLSS_SRST (1 << 0)
> > >
> > > +#define IIC_DIRECTCNTL_SDAC (1 << 3)
> > > +#define IIC_DIRECTCNTL_SCLC (1 << 2)
> > > +#define IIC_DIRECTCNTL_MSDA (1 << 1)
> > > +#define IIC_DIRECTCNTL_MSCL (1 << 0)
> > > +
> > > static void ppc4xx_i2c_reset(DeviceState *s)
> > > {
> > > PPC4xxI2CState *i2c = PPC4xx_I2C(s);
> > > @@ -289,7 +295,11 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr
> > > addr, uint64_t value,
> > > i2c->xtcntlss = value;
> > > break;
> > > case 16:
> > > - i2c->directcntl = value & 0x7;
> > > + i2c->directcntl = value & (IIC_DIRECTCNTL_SDAC &
> > > IIC_DIRECTCNTL_SCLC);
> > > + i2c->directcntl |= (value & IIC_DIRECTCNTL_SCLC ? 1 : 0);
> > > + bitbang_i2c_set(i2c->bitbang, BITBANG_I2C_SCL, i2c->directcntl &
> > > 1);
> > > + i2c->directcntl |= bitbang_i2c_set(i2c->bitbang, BITBANG_I2C_SDA,
> > > + (value & IIC_DIRECTCNTL_SDAC) != 0) << 1;
> > > break;
> > > default:
> > > if (addr < PPC4xx_I2C_MEM_SIZE) {
> > > @@ -322,6 +332,7 @@ static void ppc4xx_i2c_init(Object *o)
> > > sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
> > > sysbus_init_irq(SYS_BUS_DEVICE(s), &s->irq);
> > > s->bus = i2c_init_bus(DEVICE(s), "i2c");
> > > + s->bitbang = bitbang_i2c_init(s->bus);
> > > }
> > >
> > > static void ppc4xx_i2c_class_init(ObjectClass *klass, void *data)
> > > diff --git a/include/hw/i2c/ppc4xx_i2c.h b/include/hw/i2c/ppc4xx_i2c.h
> > > index e4b6ded..ea6c8e1 100644
> > > --- a/include/hw/i2c/ppc4xx_i2c.h
> > > +++ b/include/hw/i2c/ppc4xx_i2c.h
> > > @@ -31,6 +31,9 @@
> > > #include "hw/sysbus.h"
> > > #include "hw/i2c/i2c.h"
> > >
> > > +/* from hw/i2c/bitbang_i2c.h */
> > > +typedef struct bitbang_i2c_interface bitbang_i2c_interface;
> > > +
> > > #define TYPE_PPC4xx_I2C "ppc4xx-i2c"
> > > #define PPC4xx_I2C(obj) OBJECT_CHECK(PPC4xxI2CState, (obj),
> > > TYPE_PPC4xx_I2C)
> > >
> > > @@ -42,6 +45,7 @@ typedef struct PPC4xxI2CState {
> > > I2CBus *bus;
> > > qemu_irq irq;
> > > MemoryRegion iomem;
> > > + bitbang_i2c_interface *bitbang;
> > > uint8_t mdata;
> > > uint8_t lmadr;
> > > uint8_t hmadr;
> >
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
