qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Glenn Miles <milesg@linux.vnet.ibm.com>,
	qemu-devel@nongnu.org, qemu-ppc@nongnu.org
Cc: "Cédric Le Goater" <clg@kaod.org>,
	"Nicholas Piggin" <npiggin@gmail.com>,
	"Frédéric Barrat" <fbarrat@linux.ibm.com>,
	"Reza Arbab" <arbab@linux.ibm.com>
Subject: Re: [PATCH v7 7/9] misc: Add a pca9554 GPIO device model
Date: Mon, 4 Mar 2024 23:32:23 +0100	[thread overview]
Message-ID: <e818c2ac-149f-4767-bb5a-53501729762d@redhat.com> (raw)
In-Reply-To: <20240125224818.146499-8-milesg@linux.vnet.ibm.com>

On 1/25/24 23:48, Glenn Miles wrote:
> Specs are available here:
> 
>      https://www.nxp.com/docs/en/data-sheet/PCA9554_9554A.pdf
> 
> This is a simple model supporting the basic registers for GPIO
> mode.  The device also supports an interrupt output line but the
> model does not yet support this.
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
> ---
> 
> No changes from previous version
> 
>   MAINTAINERS                    |  10 +-
>   hw/misc/pca9554.c              | 328 +++++++++++++++++++++++++++++++++

Not a huge deal, but this should have been in hw/gpio.

Paolo

>   include/hw/misc/pca9554.h      |  36 ++++
>   include/hw/misc/pca9554_regs.h |  19 ++
>   4 files changed, 391 insertions(+), 2 deletions(-)
>   create mode 100644 hw/misc/pca9554.c
>   create mode 100644 include/hw/misc/pca9554.h
>   create mode 100644 include/hw/misc/pca9554_regs.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dfaca8323e..51861e3c7d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1169,9 +1169,7 @@ R: Joel Stanley <joel@jms.id.au>
>   L: qemu-arm@nongnu.org
>   S: Maintained
>   F: hw/*/*aspeed*
> -F: hw/misc/pca9552.c
>   F: include/hw/*/*aspeed*
> -F: include/hw/misc/pca9552*.h
>   F: hw/net/ftgmac100.c
>   F: include/hw/net/ftgmac100.h
>   F: docs/system/arm/aspeed.rst
> @@ -1540,6 +1538,14 @@ F: include/hw/pci-host/pnv*
>   F: pc-bios/skiboot.lid
>   F: tests/qtest/pnv*
>   
> +pca955x
> +M: Glenn Miles <milesg@linux.vnet.ibm.com>
> +L: qemu-ppc@nongnu.org
> +L: qemu-arm@nongnu.org
> +S: Odd Fixes
> +F: hw/misc/pca955*.c
> +F: include/hw/misc/pca955*.h
> +
>   virtex_ml507
>   M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
>   L: qemu-ppc@nongnu.org
> diff --git a/hw/misc/pca9554.c b/hw/misc/pca9554.c
> new file mode 100644
> index 0000000000..778b32e443
> --- /dev/null
> +++ b/hw/misc/pca9554.c
> @@ -0,0 +1,328 @@
> +/*
> + * PCA9554 I/O port
> + *
> + * Copyright (c) 2023, IBM Corporation.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +#include "qemu/bitops.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/misc/pca9554.h"
> +#include "hw/misc/pca9554_regs.h"
> +#include "hw/irq.h"
> +#include "migration/vmstate.h"
> +#include "qapi/error.h"
> +#include "qapi/visitor.h"
> +#include "trace.h"
> +#include "qom/object.h"
> +
> +struct PCA9554Class {
> +    /*< private >*/
> +    I2CSlaveClass parent_class;
> +    /*< public >*/
> +};
> +typedef struct PCA9554Class PCA9554Class;
> +
> +DECLARE_CLASS_CHECKERS(PCA9554Class, PCA9554,
> +                       TYPE_PCA9554)
> +
> +#define PCA9554_PIN_LOW  0x0
> +#define PCA9554_PIN_HIZ  0x1
> +
> +static const char *pin_state[] = {"low", "high"};
> +
> +static void pca9554_update_pin_input(PCA9554State *s)
> +{
> +    int i;
> +    uint8_t config = s->regs[PCA9554_CONFIG];
> +    uint8_t output = s->regs[PCA9554_OUTPUT];
> +    uint8_t internal_state = config | output;
> +
> +    for (i = 0; i < PCA9554_PIN_COUNT; i++) {
> +        uint8_t bit_mask = 1 << i;
> +        uint8_t internal_pin_state = (internal_state >> i) & 0x1;
> +        uint8_t old_value = s->regs[PCA9554_INPUT] & bit_mask;
> +        uint8_t new_value;
> +
> +        switch (internal_pin_state) {
> +        case PCA9554_PIN_LOW:
> +            s->regs[PCA9554_INPUT] &= ~bit_mask;
> +            break;
> +        case PCA9554_PIN_HIZ:
> +            /*
> +             * pullup sets it to a logical 1 unless
> +             * external device drives it low.
> +             */
> +            if (s->ext_state[i] == PCA9554_PIN_LOW) {
> +                s->regs[PCA9554_INPUT] &= ~bit_mask;
> +            } else {
> +                s->regs[PCA9554_INPUT] |=  bit_mask;
> +            }
> +            break;
> +        default:
> +            break;
> +        }
> +
> +        /* update irq state only if pin state changed */
> +        new_value = s->regs[PCA9554_INPUT] & bit_mask;
> +        if (new_value != old_value) {
> +            if (new_value) {
> +                /* changed from 0 to 1 */
> +                qemu_set_irq(s->gpio_out[i], 1);
> +            } else {
> +                /* changed from 1 to 0 */
> +                qemu_set_irq(s->gpio_out[i], 0);
> +            }
> +        }
> +    }
> +}
> +
> +static uint8_t pca9554_read(PCA9554State *s, uint8_t reg)
> +{
> +    switch (reg) {
> +    case PCA9554_INPUT:
> +        return s->regs[PCA9554_INPUT] ^ s->regs[PCA9554_POLARITY];
> +    case PCA9554_OUTPUT:
> +    case PCA9554_POLARITY:
> +    case PCA9554_CONFIG:
> +        return s->regs[reg];
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected read to register %d\n",
> +                      __func__, reg);
> +        return 0xFF;
> +    }
> +}
> +
> +static void pca9554_write(PCA9554State *s, uint8_t reg, uint8_t data)
> +{
> +    switch (reg) {
> +    case PCA9554_OUTPUT:
> +    case PCA9554_CONFIG:
> +        s->regs[reg] = data;
> +        pca9554_update_pin_input(s);
> +        break;
> +    case PCA9554_POLARITY:
> +        s->regs[reg] = data;
> +        break;
> +    case PCA9554_INPUT:
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected write to register %d\n",
> +                      __func__, reg);
> +    }
> +}
> +
> +static uint8_t pca9554_recv(I2CSlave *i2c)
> +{
> +    PCA9554State *s = PCA9554(i2c);
> +    uint8_t ret;
> +
> +    ret = pca9554_read(s, s->pointer & 0x3);
> +
> +    return ret;
> +}
> +
> +static int pca9554_send(I2CSlave *i2c, uint8_t data)
> +{
> +    PCA9554State *s = PCA9554(i2c);
> +
> +    /* First byte sent by is the register address */
> +    if (s->len == 0) {
> +        s->pointer = data;
> +        s->len++;
> +    } else {
> +        pca9554_write(s, s->pointer & 0x3, data);
> +    }
> +
> +    return 0;
> +}
> +
> +static int pca9554_event(I2CSlave *i2c, enum i2c_event event)
> +{
> +    PCA9554State *s = PCA9554(i2c);
> +
> +    s->len = 0;
> +    return 0;
> +}
> +
> +static void pca9554_get_pin(Object *obj, Visitor *v, const char *name,
> +                            void *opaque, Error **errp)
> +{
> +    PCA9554State *s = PCA9554(obj);
> +    int pin, rc;
> +    uint8_t state;
> +
> +    rc = sscanf(name, "pin%2d", &pin);
> +    if (rc != 1) {
> +        error_setg(errp, "%s: error reading %s", __func__, name);
> +        return;
> +    }
> +    if (pin < 0 || pin > PCA9554_PIN_COUNT) {
> +        error_setg(errp, "%s invalid pin %s", __func__, name);
> +        return;
> +    }
> +
> +    state = pca9554_read(s, PCA9554_CONFIG);
> +    state |= pca9554_read(s, PCA9554_OUTPUT);
> +    state = (state >> pin) & 0x1;
> +    visit_type_str(v, name, (char **)&pin_state[state], errp);
> +}
> +
> +static void pca9554_set_pin(Object *obj, Visitor *v, const char *name,
> +                            void *opaque, Error **errp)
> +{
> +    PCA9554State *s = PCA9554(obj);
> +    int pin, rc, val;
> +    uint8_t state, mask;
> +    char *state_str;
> +
> +    if (!visit_type_str(v, name, &state_str, errp)) {
> +        return;
> +    }
> +    rc = sscanf(name, "pin%2d", &pin);
> +    if (rc != 1) {
> +        error_setg(errp, "%s: error reading %s", __func__, name);
> +        return;
> +    }
> +    if (pin < 0 || pin > PCA9554_PIN_COUNT) {
> +        error_setg(errp, "%s invalid pin %s", __func__, name);
> +        return;
> +    }
> +
> +    for (state = 0; state < ARRAY_SIZE(pin_state); state++) {
> +        if (!strcmp(state_str, pin_state[state])) {
> +            break;
> +        }
> +    }
> +    if (state >= ARRAY_SIZE(pin_state)) {
> +        error_setg(errp, "%s invalid pin state %s", __func__, state_str);
> +        return;
> +    }
> +
> +    /* First, modify the output register bit */
> +    val = pca9554_read(s, PCA9554_OUTPUT);
> +    mask = 0x1 << pin;
> +    if (state == PCA9554_PIN_LOW) {
> +        val &= ~(mask);
> +    } else {
> +        val |= mask;
> +    }
> +    pca9554_write(s, PCA9554_OUTPUT, val);
> +
> +    /* Then, clear the config register bit for output mode */
> +    val = pca9554_read(s, PCA9554_CONFIG);
> +    val &= ~mask;
> +    pca9554_write(s, PCA9554_CONFIG, val);
> +}
> +
> +static const VMStateDescription pca9554_vmstate = {
> +    .name = "PCA9554",
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8(len, PCA9554State),
> +        VMSTATE_UINT8(pointer, PCA9554State),
> +        VMSTATE_UINT8_ARRAY(regs, PCA9554State, PCA9554_NR_REGS),
> +        VMSTATE_UINT8_ARRAY(ext_state, PCA9554State, PCA9554_PIN_COUNT),
> +        VMSTATE_I2C_SLAVE(i2c, PCA9554State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void pca9554_reset(DeviceState *dev)
> +{
> +    PCA9554State *s = PCA9554(dev);
> +
> +    s->regs[PCA9554_INPUT] = 0xFF;
> +    s->regs[PCA9554_OUTPUT] = 0xFF;
> +    s->regs[PCA9554_POLARITY] = 0x0; /* No pins are inverted */
> +    s->regs[PCA9554_CONFIG] = 0xFF; /* All pins are inputs */
> +
> +    memset(s->ext_state, PCA9554_PIN_HIZ, PCA9554_PIN_COUNT);
> +    pca9554_update_pin_input(s);
> +
> +    s->pointer = 0x0;
> +    s->len = 0;
> +}
> +
> +static void pca9554_initfn(Object *obj)
> +{
> +    int pin;
> +
> +    for (pin = 0; pin < PCA9554_PIN_COUNT; pin++) {
> +        char *name;
> +
> +        name = g_strdup_printf("pin%d", pin);
> +        object_property_add(obj, name, "bool", pca9554_get_pin, pca9554_set_pin,
> +                            NULL, NULL);
> +        g_free(name);
> +    }
> +}
> +
> +static void pca9554_set_ext_state(PCA9554State *s, int pin, int level)
> +{
> +    if (s->ext_state[pin] != level) {
> +        s->ext_state[pin] = level;
> +        pca9554_update_pin_input(s);
> +    }
> +}
> +
> +static void pca9554_gpio_in_handler(void *opaque, int pin, int level)
> +{
> +
> +    PCA9554State *s = PCA9554(opaque);
> +
> +    assert((pin >= 0) && (pin < PCA9554_PIN_COUNT));
> +    pca9554_set_ext_state(s, pin, level);
> +}
> +
> +static void pca9554_realize(DeviceState *dev, Error **errp)
> +{
> +    PCA9554State *s = PCA9554(dev);
> +
> +    if (!s->description) {
> +        s->description = g_strdup("pca9554");
> +    }
> +
> +    qdev_init_gpio_out(dev, s->gpio_out, PCA9554_PIN_COUNT);
> +    qdev_init_gpio_in(dev, pca9554_gpio_in_handler, PCA9554_PIN_COUNT);
> +}
> +
> +static Property pca9554_properties[] = {
> +    DEFINE_PROP_STRING("description", PCA9554State, description),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void pca9554_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
> +
> +    k->event = pca9554_event;
> +    k->recv = pca9554_recv;
> +    k->send = pca9554_send;
> +    dc->realize = pca9554_realize;
> +    dc->reset = pca9554_reset;
> +    dc->vmsd = &pca9554_vmstate;
> +    device_class_set_props(dc, pca9554_properties);
> +}
> +
> +static const TypeInfo pca9554_info = {
> +    .name          = TYPE_PCA9554,
> +    .parent        = TYPE_I2C_SLAVE,
> +    .instance_init = pca9554_initfn,
> +    .instance_size = sizeof(PCA9554State),
> +    .class_init    = pca9554_class_init,
> +    .class_size    = sizeof(PCA9554Class),
> +    .abstract      = false,
> +};
> +
> +static void pca9554_register_types(void)
> +{
> +    type_register_static(&pca9554_info);
> +}
> +
> +type_init(pca9554_register_types)
> diff --git a/include/hw/misc/pca9554.h b/include/hw/misc/pca9554.h
> new file mode 100644
> index 0000000000..54bfc4c4c7
> --- /dev/null
> +++ b/include/hw/misc/pca9554.h
> @@ -0,0 +1,36 @@
> +/*
> + * PCA9554 I/O port
> + *
> + * Copyright (c) 2023, IBM Corporation.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef PCA9554_H
> +#define PCA9554_H
> +
> +#include "hw/i2c/i2c.h"
> +#include "qom/object.h"
> +
> +#define TYPE_PCA9554 "pca9554"
> +typedef struct PCA9554State PCA9554State;
> +DECLARE_INSTANCE_CHECKER(PCA9554State, PCA9554,
> +                         TYPE_PCA9554)
> +
> +#define PCA9554_NR_REGS 4
> +#define PCA9554_PIN_COUNT 8
> +
> +struct PCA9554State {
> +    /*< private >*/
> +    I2CSlave i2c;
> +    /*< public >*/
> +
> +    uint8_t len;
> +    uint8_t pointer;
> +
> +    uint8_t regs[PCA9554_NR_REGS];
> +    qemu_irq gpio_out[PCA9554_PIN_COUNT];
> +    uint8_t ext_state[PCA9554_PIN_COUNT];
> +    char *description; /* For debugging purpose only */
> +};
> +
> +#endif
> diff --git a/include/hw/misc/pca9554_regs.h b/include/hw/misc/pca9554_regs.h
> new file mode 100644
> index 0000000000..602c4a90e0
> --- /dev/null
> +++ b/include/hw/misc/pca9554_regs.h
> @@ -0,0 +1,19 @@
> +/*
> + * PCA9554 I/O port registers
> + *
> + * Copyright (c) 2023, IBM Corporation.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef PCA9554_REGS_H
> +#define PCA9554_REGS_H
> +
> +/*
> + * Bits [0:1] are used to address a specific register.
> + */
> +#define PCA9554_INPUT       0 /* read only input register */
> +#define PCA9554_OUTPUT      1 /* read/write pin output state */
> +#define PCA9554_POLARITY    2 /* Set polarity of input register */
> +#define PCA9554_CONFIG      3 /* Set pins as inputs our ouputs */
> +
> +#endif



  reply	other threads:[~2024-03-04 22:33 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-25 22:48 [PATCH v7 0/9] Add powernv10 I2C devices and tests Glenn Miles
2024-01-25 22:48 ` [PATCH v7 1/9] misc/pca9552: Fix inverted input status Glenn Miles
2024-01-25 22:48 ` [PATCH v7 2/9] misc/pca9552: Let external devices set pca9552 inputs Glenn Miles
2024-01-25 22:48 ` [PATCH v7 3/9] ppc/pnv: New powernv10-rainier machine type Glenn Miles
2024-01-25 22:48 ` [PATCH v7 4/9] ppc/pnv: Add pca9552 to powernv10-rainier for PCIe hotplug power control Glenn Miles
2024-01-25 22:48 ` [PATCH v7 5/9] ppc/pnv: Wire up pca9552 GPIO pins " Glenn Miles
2024-01-25 22:48 ` [PATCH v7 6/9] ppc/pnv: Use resettable interface to reset child I2C buses Glenn Miles
2024-01-25 22:48 ` [PATCH v7 7/9] misc: Add a pca9554 GPIO device model Glenn Miles
2024-03-04 22:32   ` Paolo Bonzini [this message]
2024-03-05  6:20     ` Cédric Le Goater
2024-03-05 17:03       ` Miles Glenn
2024-03-05 23:38       ` Paolo Bonzini
2024-01-25 22:48 ` [PATCH v7 8/9] ppc/pnv: Add a pca9554 I2C device to powernv10-rainier Glenn Miles
2024-01-25 22:48 ` [PATCH v7 9/9] ppc/pnv: Test pnv i2c master and connected devices Glenn Miles
2024-01-26  7:52   ` Cédric Le Goater

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e818c2ac-149f-4767-bb5a-53501729762d@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=arbab@linux.ibm.com \
    --cc=clg@kaod.org \
    --cc=fbarrat@linux.ibm.com \
    --cc=milesg@linux.vnet.ibm.com \
    --cc=npiggin@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).