diff mbox series

[v3,5/6] misc: add pca9552 LED blinker model

Message ID 20171013142852.916-6-clg@kaod.org
State New
Headers show
Series aspeed: add a witherspoon-bmc machine | expand

Commit Message

Cédric Le Goater Oct. 13, 2017, 2:28 p.m. UTC
Specs are available here :

  https://www.nxp.com/docs/en/data-sheet/PCA9552.pdf

This is a simple model supporting the basic registers for led and GPIO
mode. The device also supports two blinking rates but not the model
yet.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes since v2:

 - removed comments on the I2C buffer size, but kept the array. I did
   not want to rewrite the buffer handling

 default-configs/arm-softmmu.mak |   1 +
 hw/misc/Makefile.objs           |   1 +
 hw/misc/pca9552.c               | 212 ++++++++++++++++++++++++++++++++++++++++
 include/hw/misc/pca9552.h       |  32 ++++++
 4 files changed, 246 insertions(+)
 create mode 100644 hw/misc/pca9552.c
 create mode 100644 include/hw/misc/pca9552.h

Comments

Peter Maydell Oct. 17, 2017, 5:13 p.m. UTC | #1
On 13 October 2017 at 15:28, Cédric Le Goater <clg@kaod.org> wrote:
> Specs are available here :
>
>   https://www.nxp.com/docs/en/data-sheet/PCA9552.pdf
>
> This is a simple model supporting the basic registers for led and GPIO
> mode. The device also supports two blinking rates but not the model
> yet.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
>  Changes since v2:
>
>  - removed comments on the I2C buffer size, but kept the array. I did
>    not want to rewrite the buffer handling
>
>  default-configs/arm-softmmu.mak |   1 +
>  hw/misc/Makefile.objs           |   1 +
>  hw/misc/pca9552.c               | 212 ++++++++++++++++++++++++++++++++++++++++
>  include/hw/misc/pca9552.h       |  32 ++++++
>  4 files changed, 246 insertions(+)
>  create mode 100644 hw/misc/pca9552.c
>  create mode 100644 include/hw/misc/pca9552.h
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 5059d134c816..d868d1095a6c 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -16,6 +16,7 @@ CONFIG_TSC2005=y
>  CONFIG_LM832X=y
>  CONFIG_TMP105=y
>  CONFIG_TMP421=y
> +CONFIG_PCA9552=y
>  CONFIG_STELLARIS=y
>  CONFIG_STELLARIS_INPUT=y
>  CONFIG_STELLARIS_ENET=y
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index e8f0a02f35af..e4e22880dbbc 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -7,6 +7,7 @@ common-obj-$(CONFIG_SGA) += sga.o
>  common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o
>  common-obj-$(CONFIG_PCI_TESTDEV) += pci-testdev.o
>  common-obj-$(CONFIG_EDU) += edu.o
> +common-obj-$(CONFIG_PCA9552) += pca9552.o
>
>  common-obj-y += unimp.o
>
> diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
> new file mode 100644
> index 000000000000..22460f4c14fe
> --- /dev/null
> +++ b/hw/misc/pca9552.c
> @@ -0,0 +1,212 @@
> +/*
> + * PCA9552 I2C LED blinker
> + *
> + * Copyright (c) 2017, IBM Corporation.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later. See the COPYING file in the top-level directory.

Adding the url of the datasheet in the header comment
would also be useful.

> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "hw/hw.h"
> +#include "hw/misc/pca9552.h"
> +
> +#define PCA9552_INPUT0   0 /* read only input register 0 */
> +#define PCA9552_INPUT1   1 /* read only input register 1  */
> +#define PCA9552_PSC0     2 /* read/write frequency prescaler 0 */
> +#define PCA9552_PWM0     3 /* read/write PWM register 0 */
> +#define PCA9552_PSC1     4 /* read/write frequency prescaler 1 */
> +#define PCA9552_PWM1     5 /* read/write PWM register 1 */
> +#define PCA9552_LS0      6 /* read/write LED0 to LED3 selector */
> +#define PCA9552_LS1      7 /* read/write LED4 to LED7 selector */
> +#define PCA9552_LS2      8 /* read/write LED8 to LED11 selector */
> +#define PCA9552_LS3      9 /* read/write LED12 to LED15 selector */
> +
> +#define PCA9552_LED_ON   0x0
> +#define PCA9552_LED_OFF  0x1
> +#define PCA9552_LED_PWM0 0x2
> +#define PCA9552_LED_PWM1 0x3
> +
> +static uint8_t pca9552_pin_get_config(PCA9552State *s, int pin)
> +{
> +    uint8_t reg   = PCA9552_LS0 + (pin / 4);
> +    uint8_t shift = (pin % 4) << 1;
> +
> +    return (s->regs[reg] >> shift) & 0x3;

extract32() is probably more readable than handcoded
shift-and-mask.

> +}
> +
> +static void pca9552_update_pin_input(PCA9552State *s)
> +{
> +    int i;
> +
> +    for (i = 0; i < 16; i++) {
> +        uint8_t input_reg = PCA9552_INPUT0 + (i / 8);
> +        uint8_t input_shift = (i % 8);
> +        uint8_t config = pca9552_pin_get_config(s, i);
> +
> +        switch (config) {
> +        case PCA9552_LED_ON:
> +            s->regs[input_reg] |= 1 << input_shift;
> +            break;
> +        case PCA9552_LED_OFF:
> +            s->regs[input_reg] &= ~(1 << input_shift);
> +            break;
> +        case PCA9552_LED_PWM0:
> +        case PCA9552_LED_PWM1:
> +            /* ??? */
> +        default:
> +            break;
> +        }
> +    }
> +}
> +
> +static void pca9552_read(PCA9552State *s)
> +{
> +    uint8_t reg = s->pointer & 0xf;
> +
> +    s->len = 0;
> +
> +    switch (reg) {
> +    case PCA9552_INPUT0:
> +    case PCA9552_INPUT1:
> +    case PCA9552_PSC0:
> +    case PCA9552_PWM0:
> +    case PCA9552_PSC1:
> +    case PCA9552_PWM1:
> +    case PCA9552_LS0:
> +    case PCA9552_LS1:
> +    case PCA9552_LS2:
> +    case PCA9552_LS3:
> +        s->buf[s->len++] = s->regs[reg];
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected read to register %d\n",
> +                      __func__, reg);
> +    }
> +}
> +
> +static void pca9552_write(PCA9552State *s)
> +{
> +    uint8_t reg = s->pointer & 0xf;
> +
> +    switch (reg) {
> +    case PCA9552_PSC0:
> +    case PCA9552_PWM0:
> +    case PCA9552_PSC1:
> +    case PCA9552_PWM1:
> +        s->regs[reg] = s->buf[0];
> +        break;
> +
> +    case PCA9552_LS0:
> +    case PCA9552_LS1:
> +    case PCA9552_LS2:
> +    case PCA9552_LS3:
> +        s->regs[reg] = s->buf[0];
> +        pca9552_update_pin_input(s);
> +        break;
> +
> +    case PCA9552_INPUT0:
> +    case PCA9552_INPUT1:
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected write to register %d\n",
> +                      __func__, reg);
> +    }
> +}
> +
> +static int pca9552_recv(I2CSlave *i2c)
> +{
> +    PCA9552State *s = PCA9552(i2c);
> +
> +    if (s->len < sizeof(s->buf)) {
> +        return s->buf[s->len++];
> +    } else {
> +        return 0xff;
> +    }
> +}
> +
> +static int pca9552_send(I2CSlave *i2c, uint8_t data)
> +{
> +    PCA9552State *s = PCA9552(i2c);
> +
> +    if (s->len == 0) {
> +        s->pointer = data;
> +        s->len++;
> +    } else {
> +        if (s->len <= sizeof(s->buf)) {
> +            s->buf[s->len - 1] = data;
> +        }
> +        s->len++;
> +        pca9552_write(s);
> +    }
> +
> +    return 0;
> +}
> +
> +static int pca9552_event(I2CSlave *i2c, enum i2c_event event)
> +{
> +    PCA9552State *s = PCA9552(i2c);
> +
> +    if (event == I2C_START_RECV) {
> +        pca9552_read(s);
> +    }
> +
> +    s->len = 0;
> +    return 0;
> +}

Reading the data sheet, it doesn't look like this is
correctly implementing reads and writes of more than one byte.
If you look at figures 11, 12 and 13 the guest can:
 * read or write multiple registers at once with a
   single transaction with multiple bytes, using the
   autoincrement (AI) bit in the command byte
 * read or writes multiple bytes of data from a port
   configured for GPIO with a single transaction
   (in this case AI would be clear)

There's a clearer description in the application note:
https://www.nxp.com/docs/en/application-note/AN264.pdf
(on page 12).

I think to implement this you don't need buf[] at all
(and len is only there to distinguish "first byte" from
"not first byte").

Rather than calling pca9552_read() from pca9552_event()
you should call it from pca9552_recv(), which means that
it gets called for each byte requested and you don't
need to stuff the value into buf[] and then fish it
back out again.

Similarly, rather than pca9552_send() writing the data
into s->buf[] and then pca9552_write() fishing it out,
you can just pass it directly to pca9552_write().

In both cases you want to implement the "increment
pointer as required if AI bit is set" so multibyte
transfers step through the register set, rolling over
from 9 to 0.

I don't think the Linux driver bothers to use this, but it's
worth getting it right from the start because it affects how
we represent the device state and thus migration compat.

> +static const VMStateDescription pca9552_vmstate = {
> +    .name = "PCA9552",
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8(len, PCA9552State),
> +        VMSTATE_UINT8(pointer, PCA9552State),
> +        VMSTATE_UINT8_ARRAY(buf, PCA9552State, 1),
> +        VMSTATE_UINT8_ARRAY(regs, PCA9552State, PCA9552_NR_REGS),
> +        VMSTATE_I2C_SLAVE(i2c, PCA9552State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void pca9552_reset(DeviceState *dev)
> +{
> +    PCA9552State *s = PCA9552(dev);
> +
> +    s->regs[PCA9552_PSC0] = 0xFF;
> +    s->regs[PCA9552_PWM0] = 0x80;
> +    s->regs[PCA9552_PSC1] = 0xFF;
> +    s->regs[PCA9552_PWM1] = 0x80;
> +    s->regs[PCA9552_LS0] = 0x55; /* all OFF */
> +    s->regs[PCA9552_LS1] = 0x55;
> +    s->regs[PCA9552_LS2] = 0x55;
> +    s->regs[PCA9552_LS3] = 0x55;
> +
> +    pca9552_update_pin_input(s);

Don't we also need to reset the pointer and len fields?


thanks
-- PMM
Cédric Le Goater Oct. 18, 2017, 2:24 p.m. UTC | #2
On 10/17/2017 07:13 PM, Peter Maydell wrote:
> On 13 October 2017 at 15:28, Cédric Le Goater <clg@kaod.org> wrote:
>> Specs are available here :
>>
>>   https://www.nxp.com/docs/en/data-sheet/PCA9552.pdf
>>
>> This is a simple model supporting the basic registers for led and GPIO
>> mode. The device also supports two blinking rates but not the model
>> yet.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>>  Changes since v2:
>>
>>  - removed comments on the I2C buffer size, but kept the array. I did
>>    not want to rewrite the buffer handling
>>
>>  default-configs/arm-softmmu.mak |   1 +
>>  hw/misc/Makefile.objs           |   1 +
>>  hw/misc/pca9552.c               | 212 ++++++++++++++++++++++++++++++++++++++++
>>  include/hw/misc/pca9552.h       |  32 ++++++
>>  4 files changed, 246 insertions(+)
>>  create mode 100644 hw/misc/pca9552.c
>>  create mode 100644 include/hw/misc/pca9552.h
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index 5059d134c816..d868d1095a6c 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -16,6 +16,7 @@ CONFIG_TSC2005=y
>>  CONFIG_LM832X=y
>>  CONFIG_TMP105=y
>>  CONFIG_TMP421=y
>> +CONFIG_PCA9552=y
>>  CONFIG_STELLARIS=y
>>  CONFIG_STELLARIS_INPUT=y
>>  CONFIG_STELLARIS_ENET=y
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index e8f0a02f35af..e4e22880dbbc 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -7,6 +7,7 @@ common-obj-$(CONFIG_SGA) += sga.o
>>  common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o
>>  common-obj-$(CONFIG_PCI_TESTDEV) += pci-testdev.o
>>  common-obj-$(CONFIG_EDU) += edu.o
>> +common-obj-$(CONFIG_PCA9552) += pca9552.o
>>
>>  common-obj-y += unimp.o
>>
>> diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
>> new file mode 100644
>> index 000000000000..22460f4c14fe
>> --- /dev/null
>> +++ b/hw/misc/pca9552.c
>> @@ -0,0 +1,212 @@
>> +/*
>> + * PCA9552 I2C LED blinker
>> + *
>> + * Copyright (c) 2017, IBM Corporation.
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>> + * later. See the COPYING file in the top-level directory.
> 
> Adding the url of the datasheet in the header comment
> would also be useful.

yes.


> 
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/log.h"
>> +#include "hw/hw.h"
>> +#include "hw/misc/pca9552.h"
>> +
>> +#define PCA9552_INPUT0   0 /* read only input register 0 */
>> +#define PCA9552_INPUT1   1 /* read only input register 1  */
>> +#define PCA9552_PSC0     2 /* read/write frequency prescaler 0 */
>> +#define PCA9552_PWM0     3 /* read/write PWM register 0 */
>> +#define PCA9552_PSC1     4 /* read/write frequency prescaler 1 */
>> +#define PCA9552_PWM1     5 /* read/write PWM register 1 */
>> +#define PCA9552_LS0      6 /* read/write LED0 to LED3 selector */
>> +#define PCA9552_LS1      7 /* read/write LED4 to LED7 selector */
>> +#define PCA9552_LS2      8 /* read/write LED8 to LED11 selector */
>> +#define PCA9552_LS3      9 /* read/write LED12 to LED15 selector */
>> +
>> +#define PCA9552_LED_ON   0x0
>> +#define PCA9552_LED_OFF  0x1
>> +#define PCA9552_LED_PWM0 0x2
>> +#define PCA9552_LED_PWM1 0x3
>> +
>> +static uint8_t pca9552_pin_get_config(PCA9552State *s, int pin)
>> +{
>> +    uint8_t reg   = PCA9552_LS0 + (pin / 4);
>> +    uint8_t shift = (pin % 4) << 1;
>> +
>> +    return (s->regs[reg] >> shift) & 0x3;
> 
> extract32() is probably more readable than handcoded
> shift-and-mask.
> 

ok


>> +}
>> +
>> +static void pca9552_update_pin_input(PCA9552State *s)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < 16; i++) {
>> +        uint8_t input_reg = PCA9552_INPUT0 + (i / 8);
>> +        uint8_t input_shift = (i % 8);
>> +        uint8_t config = pca9552_pin_get_config(s, i);
>> +
>> +        switch (config) {
>> +        case PCA9552_LED_ON:
>> +            s->regs[input_reg] |= 1 << input_shift;
>> +            break;
>> +        case PCA9552_LED_OFF:
>> +            s->regs[input_reg] &= ~(1 << input_shift);
>> +            break;
>> +        case PCA9552_LED_PWM0:
>> +        case PCA9552_LED_PWM1:
>> +            /* ??? */
>> +        default:
>> +            break;
>> +        }
>> +    }
>> +}
>> +
>> +static void pca9552_read(PCA9552State *s)
>> +{
>> +    uint8_t reg = s->pointer & 0xf;
>> +
>> +    s->len = 0;
>> +
>> +    switch (reg) {
>> +    case PCA9552_INPUT0:
>> +    case PCA9552_INPUT1:
>> +    case PCA9552_PSC0:
>> +    case PCA9552_PWM0:
>> +    case PCA9552_PSC1:
>> +    case PCA9552_PWM1:
>> +    case PCA9552_LS0:
>> +    case PCA9552_LS1:
>> +    case PCA9552_LS2:
>> +    case PCA9552_LS3:
>> +        s->buf[s->len++] = s->regs[reg];
>> +        break;
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected read to register %d\n",
>> +                      __func__, reg);
>> +    }
>> +}
>> +
>> +static void pca9552_write(PCA9552State *s)
>> +{
>> +    uint8_t reg = s->pointer & 0xf;
>> +
>> +    switch (reg) {
>> +    case PCA9552_PSC0:
>> +    case PCA9552_PWM0:
>> +    case PCA9552_PSC1:
>> +    case PCA9552_PWM1:
>> +        s->regs[reg] = s->buf[0];
>> +        break;
>> +
>> +    case PCA9552_LS0:
>> +    case PCA9552_LS1:
>> +    case PCA9552_LS2:
>> +    case PCA9552_LS3:
>> +        s->regs[reg] = s->buf[0];
>> +        pca9552_update_pin_input(s);
>> +        break;
>> +
>> +    case PCA9552_INPUT0:
>> +    case PCA9552_INPUT1:
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected write to register %d\n",
>> +                      __func__, reg);
>> +    }
>> +}
>> +
>> +static int pca9552_recv(I2CSlave *i2c)
>> +{
>> +    PCA9552State *s = PCA9552(i2c);
>> +
>> +    if (s->len < sizeof(s->buf)) {
>> +        return s->buf[s->len++];
>> +    } else {
>> +        return 0xff;
>> +    }
>> +}
>> +
>> +static int pca9552_send(I2CSlave *i2c, uint8_t data)
>> +{
>> +    PCA9552State *s = PCA9552(i2c);
>> +
>> +    if (s->len == 0) {
>> +        s->pointer = data;
>> +        s->len++;
>> +    } else {
>> +        if (s->len <= sizeof(s->buf)) {
>> +            s->buf[s->len - 1] = data;
>> +        }
>> +        s->len++;
>> +        pca9552_write(s);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int pca9552_event(I2CSlave *i2c, enum i2c_event event)
>> +{
>> +    PCA9552State *s = PCA9552(i2c);
>> +
>> +    if (event == I2C_START_RECV) {
>> +        pca9552_read(s);
>> +    }
>> +
>> +    s->len = 0;
>> +    return 0;
>> +}
> 
> Reading the data sheet, it doesn't look like this is
> correctly implementing reads and writes of more than one byte.
> If you look at figures 11, 12 and 13 the guest can:
>  * read or write multiple registers at once with a
>    single transaction with multiple bytes, using the
>    autoincrement (AI) bit in the command byte
>  * read or writes multiple bytes of data from a port
>    configured for GPIO with a single transaction
>    (in this case AI would be clear)

I completely overlooked the AI support but it does not 
seem too complex to implement.

> There's a clearer description in the application note:
> https://www.nxp.com/docs/en/application-note/AN264.pdf
> (on page 12).

This is a much better document than the one I had found ...

> I think to implement this you don't need buf[] at all
> (and len is only there to distinguish "first byte" from
> "not first byte").
> 
> Rather than calling pca9552_read() from pca9552_event()
> you should call it from pca9552_recv(), which means that
> it gets called for each byte requested and you don't
> need to stuff the value into buf[] and then fish it
> back out again.
> 
> Similarly, rather than pca9552_send() writing the data
> into s->buf[] and then pca9552_write() fishing it out,
> you can just pass it directly to pca9552_write().

Yes. These are all good cleanups. I suspect other I2C models
would also benefit from your suggestions. I will take a look
later on.

> In both cases you want to implement the "increment
> pointer as required if AI bit is set" so multibyte
> transfers step through the register set, rolling over
> from 9 to 0.

I will send an updated version with AI support in the next 
round. 

> I don't think the Linux driver bothers to use this, but it's
> worth getting it right from the start because it affects how
> we represent the device state and thus migration compat.

yes.

>> +static const VMStateDescription pca9552_vmstate = {
>> +    .name = "PCA9552",
>> +    .version_id = 0,
>> +    .minimum_version_id = 0,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT8(len, PCA9552State),
>> +        VMSTATE_UINT8(pointer, PCA9552State),
>> +        VMSTATE_UINT8_ARRAY(buf, PCA9552State, 1),
>> +        VMSTATE_UINT8_ARRAY(regs, PCA9552State, PCA9552_NR_REGS),
>> +        VMSTATE_I2C_SLAVE(i2c, PCA9552State),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static void pca9552_reset(DeviceState *dev)
>> +{
>> +    PCA9552State *s = PCA9552(dev);
>> +
>> +    s->regs[PCA9552_PSC0] = 0xFF;
>> +    s->regs[PCA9552_PWM0] = 0x80;
>> +    s->regs[PCA9552_PSC1] = 0xFF;
>> +    s->regs[PCA9552_PWM1] = 0x80;
>> +    s->regs[PCA9552_LS0] = 0x55; /* all OFF */
>> +    s->regs[PCA9552_LS1] = 0x55;
>> +    s->regs[PCA9552_LS2] = 0x55;
>> +    s->regs[PCA9552_LS3] = 0x55;
>> +
>> +    pca9552_update_pin_input(s);
> 
> Don't we also need to reset the pointer and len fields?
> 

yes. indeed.

Thanks,

C. 


> thanks
> -- PMM
>
diff mbox series

Patch

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 5059d134c816..d868d1095a6c 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -16,6 +16,7 @@  CONFIG_TSC2005=y
 CONFIG_LM832X=y
 CONFIG_TMP105=y
 CONFIG_TMP421=y
+CONFIG_PCA9552=y
 CONFIG_STELLARIS=y
 CONFIG_STELLARIS_INPUT=y
 CONFIG_STELLARIS_ENET=y
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index e8f0a02f35af..e4e22880dbbc 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -7,6 +7,7 @@  common-obj-$(CONFIG_SGA) += sga.o
 common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o
 common-obj-$(CONFIG_PCI_TESTDEV) += pci-testdev.o
 common-obj-$(CONFIG_EDU) += edu.o
+common-obj-$(CONFIG_PCA9552) += pca9552.o
 
 common-obj-y += unimp.o
 
diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
new file mode 100644
index 000000000000..22460f4c14fe
--- /dev/null
+++ b/hw/misc/pca9552.c
@@ -0,0 +1,212 @@ 
+/*
+ * PCA9552 I2C LED blinker
+ *
+ * Copyright (c) 2017, IBM Corporation.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/hw.h"
+#include "hw/misc/pca9552.h"
+
+#define PCA9552_INPUT0   0 /* read only input register 0 */
+#define PCA9552_INPUT1   1 /* read only input register 1  */
+#define PCA9552_PSC0     2 /* read/write frequency prescaler 0 */
+#define PCA9552_PWM0     3 /* read/write PWM register 0 */
+#define PCA9552_PSC1     4 /* read/write frequency prescaler 1 */
+#define PCA9552_PWM1     5 /* read/write PWM register 1 */
+#define PCA9552_LS0      6 /* read/write LED0 to LED3 selector */
+#define PCA9552_LS1      7 /* read/write LED4 to LED7 selector */
+#define PCA9552_LS2      8 /* read/write LED8 to LED11 selector */
+#define PCA9552_LS3      9 /* read/write LED12 to LED15 selector */
+
+#define PCA9552_LED_ON   0x0
+#define PCA9552_LED_OFF  0x1
+#define PCA9552_LED_PWM0 0x2
+#define PCA9552_LED_PWM1 0x3
+
+static uint8_t pca9552_pin_get_config(PCA9552State *s, int pin)
+{
+    uint8_t reg   = PCA9552_LS0 + (pin / 4);
+    uint8_t shift = (pin % 4) << 1;
+
+    return (s->regs[reg] >> shift) & 0x3;
+}
+
+static void pca9552_update_pin_input(PCA9552State *s)
+{
+    int i;
+
+    for (i = 0; i < 16; i++) {
+        uint8_t input_reg = PCA9552_INPUT0 + (i / 8);
+        uint8_t input_shift = (i % 8);
+        uint8_t config = pca9552_pin_get_config(s, i);
+
+        switch (config) {
+        case PCA9552_LED_ON:
+            s->regs[input_reg] |= 1 << input_shift;
+            break;
+        case PCA9552_LED_OFF:
+            s->regs[input_reg] &= ~(1 << input_shift);
+            break;
+        case PCA9552_LED_PWM0:
+        case PCA9552_LED_PWM1:
+            /* ??? */
+        default:
+            break;
+        }
+    }
+}
+
+static void pca9552_read(PCA9552State *s)
+{
+    uint8_t reg = s->pointer & 0xf;
+
+    s->len = 0;
+
+    switch (reg) {
+    case PCA9552_INPUT0:
+    case PCA9552_INPUT1:
+    case PCA9552_PSC0:
+    case PCA9552_PWM0:
+    case PCA9552_PSC1:
+    case PCA9552_PWM1:
+    case PCA9552_LS0:
+    case PCA9552_LS1:
+    case PCA9552_LS2:
+    case PCA9552_LS3:
+        s->buf[s->len++] = s->regs[reg];
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected read to register %d\n",
+                      __func__, reg);
+    }
+}
+
+static void pca9552_write(PCA9552State *s)
+{
+    uint8_t reg = s->pointer & 0xf;
+
+    switch (reg) {
+    case PCA9552_PSC0:
+    case PCA9552_PWM0:
+    case PCA9552_PSC1:
+    case PCA9552_PWM1:
+        s->regs[reg] = s->buf[0];
+        break;
+
+    case PCA9552_LS0:
+    case PCA9552_LS1:
+    case PCA9552_LS2:
+    case PCA9552_LS3:
+        s->regs[reg] = s->buf[0];
+        pca9552_update_pin_input(s);
+        break;
+
+    case PCA9552_INPUT0:
+    case PCA9552_INPUT1:
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected write to register %d\n",
+                      __func__, reg);
+    }
+}
+
+static int pca9552_recv(I2CSlave *i2c)
+{
+    PCA9552State *s = PCA9552(i2c);
+
+    if (s->len < sizeof(s->buf)) {
+        return s->buf[s->len++];
+    } else {
+        return 0xff;
+    }
+}
+
+static int pca9552_send(I2CSlave *i2c, uint8_t data)
+{
+    PCA9552State *s = PCA9552(i2c);
+
+    if (s->len == 0) {
+        s->pointer = data;
+        s->len++;
+    } else {
+        if (s->len <= sizeof(s->buf)) {
+            s->buf[s->len - 1] = data;
+        }
+        s->len++;
+        pca9552_write(s);
+    }
+
+    return 0;
+}
+
+static int pca9552_event(I2CSlave *i2c, enum i2c_event event)
+{
+    PCA9552State *s = PCA9552(i2c);
+
+    if (event == I2C_START_RECV) {
+        pca9552_read(s);
+    }
+
+    s->len = 0;
+    return 0;
+}
+
+static const VMStateDescription pca9552_vmstate = {
+    .name = "PCA9552",
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(len, PCA9552State),
+        VMSTATE_UINT8(pointer, PCA9552State),
+        VMSTATE_UINT8_ARRAY(buf, PCA9552State, 1),
+        VMSTATE_UINT8_ARRAY(regs, PCA9552State, PCA9552_NR_REGS),
+        VMSTATE_I2C_SLAVE(i2c, PCA9552State),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void pca9552_reset(DeviceState *dev)
+{
+    PCA9552State *s = PCA9552(dev);
+
+    s->regs[PCA9552_PSC0] = 0xFF;
+    s->regs[PCA9552_PWM0] = 0x80;
+    s->regs[PCA9552_PSC1] = 0xFF;
+    s->regs[PCA9552_PWM1] = 0x80;
+    s->regs[PCA9552_LS0] = 0x55; /* all OFF */
+    s->regs[PCA9552_LS1] = 0x55;
+    s->regs[PCA9552_LS2] = 0x55;
+    s->regs[PCA9552_LS3] = 0x55;
+
+    pca9552_update_pin_input(s);
+}
+
+static void pca9552_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
+
+    k->event = pca9552_event;
+    k->recv = pca9552_recv;
+    k->send = pca9552_send;
+    dc->reset = pca9552_reset;
+    dc->vmsd = &pca9552_vmstate;
+}
+
+static const TypeInfo pca9552_info = {
+    .name          = TYPE_PCA9552,
+    .parent        = TYPE_I2C_SLAVE,
+    .instance_size = sizeof(PCA9552State),
+    .class_init    = pca9552_class_init,
+};
+
+static void pca9552_register_types(void)
+{
+    type_register_static(&pca9552_info);
+}
+
+type_init(pca9552_register_types)
diff --git a/include/hw/misc/pca9552.h b/include/hw/misc/pca9552.h
new file mode 100644
index 000000000000..6c66d4f69e9e
--- /dev/null
+++ b/include/hw/misc/pca9552.h
@@ -0,0 +1,32 @@ 
+/*
+ * PCA9552 I2C LED blinker
+ *
+ * Copyright (c) 2017, IBM Corporation.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+#ifndef PCA9552_H
+#define PCA9552_H
+
+#include "hw/i2c/i2c.h"
+
+#define TYPE_PCA9552 "pca9552"
+#define PCA9552(obj) OBJECT_CHECK(PCA9552State, (obj), TYPE_PCA9552)
+
+
+#define PCA9552_NR_REGS 10
+
+typedef struct PCA9552State {
+    /*< private >*/
+    I2CSlave i2c;
+    /*< public >*/
+
+    uint8_t len;
+    uint8_t pointer;
+    uint8_t buf[1];
+
+    uint8_t regs[PCA9552_NR_REGS];
+} PCA9552State;
+
+#endif