diff mbox series

[v2,4/4] hw/i2c: add pca954x i2c-mux switch

Message ID 20210409162545.3705962-5-venture@google.com (mailing list archive)
State New, archived
Headers show
Series hw/i2c: Adds pca954x i2c mux switch device | expand

Commit Message

Patrick Venture April 9, 2021, 4:25 p.m. UTC
The pca954x is an i2c mux, and this adds support for two variants of
this device: the pca9546 and pca9548.

This device is very common on BMCs to route a different channel to each
PCIe i2c bus downstream from the BMC.

Signed-off-by: Patrick Venture <venture@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Havard Skinnemoen <hskinnemoen@google.com>
---
 MAINTAINERS                      |   6 +
 hw/i2c/Kconfig                   |   4 +
 hw/i2c/i2c_mux_pca954x.c         | 290 +++++++++++++++++++++++++++++++
 hw/i2c/meson.build               |   1 +
 hw/i2c/trace-events              |   5 +
 include/hw/i2c/i2c_mux_pca954x.h |  19 ++
 6 files changed, 325 insertions(+)
 create mode 100644 hw/i2c/i2c_mux_pca954x.c
 create mode 100644 include/hw/i2c/i2c_mux_pca954x.h

Comments

Philippe Mathieu-Daudé April 9, 2021, 4:51 p.m. UTC | #1
Hi Patrick,

On 4/9/21 6:25 PM, Patrick Venture wrote:
> The pca954x is an i2c mux, and this adds support for two variants of
> this device: the pca9546 and pca9548.
> 
> This device is very common on BMCs to route a different channel to each
> PCIe i2c bus downstream from the BMC.
> 
> Signed-off-by: Patrick Venture <venture@google.com>
> Reviewed-by: Hao Wu <wuhaotsh@google.com>
> Reviewed-by: Havard Skinnemoen <hskinnemoen@google.com>
> ---
>  MAINTAINERS                      |   6 +
>  hw/i2c/Kconfig                   |   4 +
>  hw/i2c/i2c_mux_pca954x.c         | 290 +++++++++++++++++++++++++++++++
>  hw/i2c/meson.build               |   1 +
>  hw/i2c/trace-events              |   5 +
>  include/hw/i2c/i2c_mux_pca954x.h |  19 ++
>  6 files changed, 325 insertions(+)
>  create mode 100644 hw/i2c/i2c_mux_pca954x.c
>  create mode 100644 include/hw/i2c/i2c_mux_pca954x.h

> diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig
> index 09642a6dcb..8d120a25d5 100644
> --- a/hw/i2c/Kconfig
> +++ b/hw/i2c/Kconfig
> @@ -28,3 +28,7 @@ config IMX_I2C
>  config MPC_I2C
>      bool
>      select I2C
> +
> +config PCA954X
> +    bool
> +    select I2C

Do you have a circular dependency when also using:

       depends on I2C

?

> +static void pca954x_realize(DeviceState *dev, Error **errp)
> +{
> +    Pca954xState *s = PCA954X(dev);
> +    Pca954xClass *c = PCA954X_GET_CLASS(s);
> +    int i;
> +
> +    /* SMBus modules. Cannot fail. */
> +    for (i = 0; i < c->nchans; i++) {
> +        Object *obj = OBJECT(&s->channel[i]);
> +        sysbus_realize(SYS_BUS_DEVICE(obj), &error_abort);

No need to cast to Object:

           sysbus_realize(SYS_BUS_DEVICE(&s->channel[i]), &error_abort);

> +    }
> +}
Patrick Venture April 9, 2021, 5:21 p.m. UTC | #2
On Fri, Apr 9, 2021 at 9:51 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi Patrick,
>
> On 4/9/21 6:25 PM, Patrick Venture wrote:
> > The pca954x is an i2c mux, and this adds support for two variants of
> > this device: the pca9546 and pca9548.
> >
> > This device is very common on BMCs to route a different channel to each
> > PCIe i2c bus downstream from the BMC.
> >
> > Signed-off-by: Patrick Venture <venture@google.com>
> > Reviewed-by: Hao Wu <wuhaotsh@google.com>
> > Reviewed-by: Havard Skinnemoen <hskinnemoen@google.com>
> > ---
> >  MAINTAINERS                      |   6 +
> >  hw/i2c/Kconfig                   |   4 +
> >  hw/i2c/i2c_mux_pca954x.c         | 290 +++++++++++++++++++++++++++++++
> >  hw/i2c/meson.build               |   1 +
> >  hw/i2c/trace-events              |   5 +
> >  include/hw/i2c/i2c_mux_pca954x.h |  19 ++
> >  6 files changed, 325 insertions(+)
> >  create mode 100644 hw/i2c/i2c_mux_pca954x.c
> >  create mode 100644 include/hw/i2c/i2c_mux_pca954x.h
>
> > diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig
> > index 09642a6dcb..8d120a25d5 100644
> > --- a/hw/i2c/Kconfig
> > +++ b/hw/i2c/Kconfig
> > @@ -28,3 +28,7 @@ config IMX_I2C
> >  config MPC_I2C
> >      bool
> >      select I2C
> > +
> > +config PCA954X
> > +    bool
> > +    select I2C
>
> Do you have a circular dependency when also using:
>
>        depends on I2C
>
> ?

I'm somewhat new to qemu -- I don't know what you mean, since I2C
doesn't depend on pca954x, I don't imagine there could be a circular
dependency.

>
> > +static void pca954x_realize(DeviceState *dev, Error **errp)
> > +{
> > +    Pca954xState *s = PCA954X(dev);
> > +    Pca954xClass *c = PCA954X_GET_CLASS(s);
> > +    int i;
> > +
> > +    /* SMBus modules. Cannot fail. */
> > +    for (i = 0; i < c->nchans; i++) {
> > +        Object *obj = OBJECT(&s->channel[i]);
> > +        sysbus_realize(SYS_BUS_DEVICE(obj), &error_abort);
>
> No need to cast to Object:
>
>            sysbus_realize(SYS_BUS_DEVICE(&s->channel[i]), &error_abort);
>

Ack, will fix in v3.

> > +    }
> > +}
Corey Minyard April 9, 2021, 6:34 p.m. UTC | #3
On Fri, Apr 09, 2021 at 09:25:45AM -0700, Patrick Venture wrote:
> The pca954x is an i2c mux, and this adds support for two variants of
> this device: the pca9546 and pca9548.
> 
> This device is very common on BMCs to route a different channel to each
> PCIe i2c bus downstream from the BMC.
> 
> Signed-off-by: Patrick Venture <venture@google.com>
> Reviewed-by: Hao Wu <wuhaotsh@google.com>
> Reviewed-by: Havard Skinnemoen <hskinnemoen@google.com>
> ---
>  MAINTAINERS                      |   6 +
>  hw/i2c/Kconfig                   |   4 +
>  hw/i2c/i2c_mux_pca954x.c         | 290 +++++++++++++++++++++++++++++++
>  hw/i2c/meson.build               |   1 +
>  hw/i2c/trace-events              |   5 +
>  include/hw/i2c/i2c_mux_pca954x.h |  19 ++
>  6 files changed, 325 insertions(+)
>  create mode 100644 hw/i2c/i2c_mux_pca954x.c
>  create mode 100644 include/hw/i2c/i2c_mux_pca954x.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 58f342108e..5ea0b60b8a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2039,6 +2039,12 @@ S: Maintained
>  F: hw/net/tulip.c
>  F: hw/net/tulip.h
>  
> +pca954x
> +M: Patrick Venture <venture@google.com>
> +S: Maintained
> +F: hw/i2c/i2c_mux_pca954x.c
> +F: include/hw/i2c/i2c_mux_pca954x.h
> +
>  Generic Loader
>  M: Alistair Francis <alistair@alistair23.me>
>  S: Maintained
> diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig
> index 09642a6dcb..8d120a25d5 100644
> --- a/hw/i2c/Kconfig
> +++ b/hw/i2c/Kconfig
> @@ -28,3 +28,7 @@ config IMX_I2C
>  config MPC_I2C
>      bool
>      select I2C
> +
> +config PCA954X
> +    bool
> +    select I2C
> diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
> new file mode 100644
> index 0000000000..9aa1a8872f
> --- /dev/null
> +++ b/hw/i2c/i2c_mux_pca954x.c
> @@ -0,0 +1,290 @@
> +/*
> + * I2C multiplexer for PCA954x series of I2C multiplexer/switch chips.
> + *
> + * Copyright 2021 Google LLC
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by 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/i2c/i2c.h"
> +#include "hw/i2c/i2c_mux_pca954x.h"
> +#include "hw/i2c/smbus_slave.h"
> +#include "hw/qdev-core.h"
> +#include "hw/sysbus.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +#include "qemu/queue.h"
> +#include "qom/object.h"
> +#include "trace.h"
> +
> +#define PCA9548_CHANNEL_COUNT 8
> +#define PCA9546_CHANNEL_COUNT 4
> +
> +/*
> + * struct Pca954xChannel - The i2c mux device will have N of these states
> + * that own the i2c channel bus.
> + * @bus: The owned channel bus.
> + * @enabled: Is this channel active?
> + */
> +typedef struct Pca954xChannel {
> +    SysBusDevice parent;
> +
> +    I2CBus       *bus;
> +
> +    bool         enabled;
> +} Pca954xChannel;
> +
> +#define TYPE_PCA954X_CHANNEL "pca954x-channel"
> +#define PCA954X_CHANNEL(obj) \
> +    OBJECT_CHECK(Pca954xChannel, (obj), TYPE_PCA954X_CHANNEL)
> +
> +/*
> + * struct Pca954xState - The pca954x state object.
> + * @control: The value written to the mux control.
> + * @channel: The set of i2c channel buses that act as channels which own the
> + * i2c children.
> + */
> +typedef struct Pca954xState {
> +    SMBusDevice parent;
> +
> +    uint8_t control;
> +
> +    /* The channel i2c buses. */
> +    Pca954xChannel channel[PCA9548_CHANNEL_COUNT];
> +} Pca954xState;
> +
> +/*
> + * struct Pca954xClass - The pca954x class object.
> + * @nchans: The number of i2c channels this device has.
> + */
> +typedef struct Pca954xClass {
> +    SMBusDeviceClass parent;
> +
> +    uint8_t nchans;
> +} Pca954xClass;
> +
> +#define TYPE_PCA954X "pca954x"
> +OBJECT_DECLARE_TYPE(Pca954xState, Pca954xClass, PCA954X)
> +
> +/*
> + * For each channel, if it's enabled, recursively call match on those children.
> + */
> +static bool pca954x_match(I2CSlave *candidate, uint8_t address,
> +                          bool broadcast,
> +                          I2CNodeList *current_devs)
> +{
> +    Pca954xState *mux = PCA954X(candidate);
> +    Pca954xClass *mc = PCA954X_GET_CLASS(mux);
> +    int i;
> +
> +    /* They are talking to the mux itself (or all devices enabled. */

Missing close paren in the comment above.  Really minor nit :)

> +    if ((candidate->address == address) || broadcast) {
> +        I2CNode *node = g_malloc(sizeof(struct I2CNode));
> +        node->elt = candidate;
> +        QLIST_INSERT_HEAD(current_devs, node, next);
> +        if (!broadcast) {
> +            return true;
> +        }
> +    }
> +
> +    for (i = 0; i < mc->nchans; i++) {
> +        if (!mux->channel[i].enabled) {
> +            continue;
> +        }
> +
> +        if (i2c_scan_bus(mux->channel[i].bus, address, broadcast)) {
> +            if (!broadcast) {
> +                return true;
> +            }
> +        }
> +    }
> +
> +    /* If we arrived here we didn't find a match, return broadcast. */
> +    return broadcast;
> +}
> +
> +static void pca954x_enable_channel(Pca954xState *s, uint8_t enable_mask)
> +{
> +    Pca954xClass *mc = PCA954X_GET_CLASS(s);
> +    int i;
> +
> +    /*
> +     * For each channel, check if their bit is set in enable_mask and if yes,
> +     * enable it, otherwise disable, hide it.
> +     */
> +    for (i = 0; i < mc->nchans; i++) {
> +        if (enable_mask & (1 << i)) {
> +            s->channel[i].enabled = true;
> +        } else {
> +            s->channel[i].enabled = false;
> +        }
> +    }
> +}
> +
> +static void pca954x_write(Pca954xState *s, uint8_t data)
> +{
> +    s->control = data;
> +    pca954x_enable_channel(s, data);
> +
> +    trace_pca954x_write_bytes(data);
> +}
> +
> +static int pca954x_write_data(SMBusDevice *d, uint8_t *buf, uint8_t len)
> +{
> +    Pca954xState *s = PCA954X(d);
> +
> +    if (len == 0) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: writing empty data\n", __func__);
> +        return -1;
> +    }
> +
> +    /*
> +     * len should be 1, because they write one byte to enable/disable channels.
> +     */
> +    if (len > 1) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +            "%s: extra data after channel selection mask\n",
> +            __func__);
> +        return -1;
> +    }
> +
> +    pca954x_write(s, buf[0]);
> +    return 0;
> +}
> +
> +static uint8_t pca954x_read_byte(SMBusDevice *d)
> +{
> +    Pca954xState *s = PCA954X(d);
> +    uint8_t data = s->control;
> +    trace_pca954x_read_data(data);
> +    return data;
> +}
> +
> +static void pca954x_enter_reset(Object *obj, ResetType type)
> +{
> +    Pca954xState *s = PCA954X(obj);
> +    /* Reset will disable all channels. */
> +    pca954x_write(s, 0);
> +}
> +
> +I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel)
> +{
> +    Pca954xClass *pc = PCA954X_GET_CLASS(mux);
> +    Pca954xState *pca954x = PCA954X(mux);
> +
> +    g_assert(channel < pc->nchans);
> +    return I2C_BUS(qdev_get_child_bus(DEVICE(&pca954x->channel[channel]),
> +                                      "i2c-bus"));
> +}
> +
> +static void pca954x_smbus_init(Object *obj)
> +{
> +    Pca954xChannel *s = PCA954X_CHANNEL(obj);
> +    s->bus = i2c_init_bus(DEVICE(s), "i2c-bus");
> +
> +    /* Start all channels as disabled. */
> +    s->enabled = false;
> +}
> +
> +static void pca954x_channel_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    dc->desc = "Pca954x Channel";
> +}
> +
> +static void pca9546_class_init(ObjectClass *klass, void *data)
> +{
> +    Pca954xClass *s = PCA954X_CLASS(klass);
> +    s->nchans = PCA9546_CHANNEL_COUNT;
> +}
> +
> +static void pca9548_class_init(ObjectClass *oc, void *data)
> +{
> +    Pca954xClass *s = PCA954X_CLASS(oc);
> +    s->nchans = PCA9548_CHANNEL_COUNT;
> +}
> +
> +static void pca954x_realize(DeviceState *dev, Error **errp)
> +{
> +    Pca954xState *s = PCA954X(dev);
> +    Pca954xClass *c = PCA954X_GET_CLASS(s);
> +    int i;
> +
> +    /* SMBus modules. Cannot fail. */
> +    for (i = 0; i < c->nchans; i++) {
> +        Object *obj = OBJECT(&s->channel[i]);
> +        sysbus_realize(SYS_BUS_DEVICE(obj), &error_abort);
> +    }
> +}
> +
> +static void pca954x_init(Object *obj)
> +{
> +    int i;
> +    Pca954xState *s = PCA954X(obj);
> +    Pca954xClass *c = PCA954X_GET_CLASS(obj);
> +
> +    /* Only initialize the children we expect. */
> +    for (i = 0; i < c->nchans; i++) {
> +        object_initialize_child(obj, "channel[*]", &s->channel[i],
> +                                TYPE_PCA954X_CHANNEL);
> +    }
> +}
> +
> +static void pca954x_class_init(ObjectClass *klass, void *data)
> +{
> +    I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
> +    ResettableClass *rc = RESETTABLE_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    SMBusDeviceClass *k = SMBUS_DEVICE_CLASS(klass);
> +
> +    sc->match_and_add = pca954x_match;
> +
> +    rc->phases.enter = pca954x_enter_reset;
> +
> +    dc->desc = "Pca954x i2c-mux";
> +    dc->realize = pca954x_realize;
> +
> +    k->write_data = pca954x_write_data;
> +    k->receive_byte = pca954x_read_byte;
> +}
> +
> +static const TypeInfo pca954x_info[] = {
> +    {
> +        .name          = TYPE_PCA954X,
> +        .parent        = TYPE_SMBUS_DEVICE,
> +        .instance_size = sizeof(Pca954xState),
> +        .instance_init = pca954x_init,
> +        .class_size    = sizeof(Pca954xClass),
> +        .class_init    = pca954x_class_init,
> +        .abstract      = true,
> +    },
> +    {
> +        .name          = TYPE_PCA9546,
> +        .parent        = TYPE_PCA954X,
> +        .class_init    = pca9546_class_init,
> +    },
> +    {
> +        .name          = TYPE_PCA9548,
> +        .parent        = TYPE_PCA954X,
> +        .class_init    = pca9548_class_init,
> +    },
> +    {
> +        .name = TYPE_PCA954X_CHANNEL,
> +        .parent = TYPE_SYS_BUS_DEVICE,
> +        .class_init = pca954x_channel_class_init,
> +        .instance_size = sizeof(Pca954xChannel),
> +        .instance_init = pca954x_smbus_init,
> +    }
> +};
> +
> +DEFINE_TYPES(pca954x_info)
> diff --git a/hw/i2c/meson.build b/hw/i2c/meson.build
> index cdcd694a7f..dd3aef02b2 100644
> --- a/hw/i2c/meson.build
> +++ b/hw/i2c/meson.build
> @@ -14,4 +14,5 @@ i2c_ss.add(when: 'CONFIG_SMBUS_EEPROM', if_true: files('smbus_eeprom.c'))
>  i2c_ss.add(when: 'CONFIG_VERSATILE_I2C', if_true: files('versatile_i2c.c'))
>  i2c_ss.add(when: 'CONFIG_OMAP', if_true: files('omap_i2c.c'))
>  i2c_ss.add(when: 'CONFIG_PPC4XX', if_true: files('ppc4xx_i2c.c'))
> +i2c_ss.add(when: 'CONFIG_PCA954X', if_true: files('i2c_mux_pca954x.c'))
>  softmmu_ss.add_all(when: 'CONFIG_I2C', if_true: i2c_ss)
> diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
> index 82fe6f965f..82f19e6a2d 100644
> --- a/hw/i2c/trace-events
> +++ b/hw/i2c/trace-events
> @@ -26,3 +26,8 @@ npcm7xx_smbus_recv_byte(const char *id, uint8_t value) "%s recv byte: 0x%02x"
>  npcm7xx_smbus_stop(const char *id) "%s stopping"
>  npcm7xx_smbus_nack(const char *id) "%s nacking"
>  npcm7xx_smbus_recv_fifo(const char *id, uint8_t received, uint8_t expected) "%s recv fifo: received %u, expected %u"
> +
> +# i2c-mux-pca954x.c
> +
> +pca954x_write_bytes(uint8_t value) "PCA954X write data: 0x%02x"
> +pca954x_read_data(uint8_t value) "PCA954X read data: 0x%02x"
> diff --git a/include/hw/i2c/i2c_mux_pca954x.h b/include/hw/i2c/i2c_mux_pca954x.h
> new file mode 100644
> index 0000000000..8aaf9bbc39
> --- /dev/null
> +++ b/include/hw/i2c/i2c_mux_pca954x.h
> @@ -0,0 +1,19 @@
> +#ifndef QEMU_I2C_MUX_PCA954X
> +#define QEMU_I2C_MUX_PCA954X
> +
> +#include "hw/i2c/i2c.h"
> +
> +#define TYPE_PCA9546 "pca9546"
> +#define TYPE_PCA9548 "pca9548"
> +
> +/**
> + * Retrieves the i2c bus associated with the specified channel on this i2c
> + * mux.
> + * @mux: an i2c mux device.
> + * @channel: the i2c channel requested
> + *
> + * Returns: a pointer to the associated i2c bus.
> + */
> +I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel);

I assume your machine-specific code will be referencing this, which is
the way it should work.

I'm happy with this.

-corey

> +
> +#endif
> -- 
> 2.31.1.295.g9ea45b61b8-goog
>
Patrick Venture April 9, 2021, 7:30 p.m. UTC | #4
On Fri, Apr 9, 2021 at 11:34 AM Corey Minyard <cminyard@mvista.com> wrote:
>
> On Fri, Apr 09, 2021 at 09:25:45AM -0700, Patrick Venture wrote:
> > The pca954x is an i2c mux, and this adds support for two variants of
> > this device: the pca9546 and pca9548.
> >
> > This device is very common on BMCs to route a different channel to each
> > PCIe i2c bus downstream from the BMC.
> >
> > Signed-off-by: Patrick Venture <venture@google.com>
> > Reviewed-by: Hao Wu <wuhaotsh@google.com>
> > Reviewed-by: Havard Skinnemoen <hskinnemoen@google.com>
> > ---
> >  MAINTAINERS                      |   6 +
> >  hw/i2c/Kconfig                   |   4 +
> >  hw/i2c/i2c_mux_pca954x.c         | 290 +++++++++++++++++++++++++++++++
> >  hw/i2c/meson.build               |   1 +
> >  hw/i2c/trace-events              |   5 +
> >  include/hw/i2c/i2c_mux_pca954x.h |  19 ++
> >  6 files changed, 325 insertions(+)
> >  create mode 100644 hw/i2c/i2c_mux_pca954x.c
> >  create mode 100644 include/hw/i2c/i2c_mux_pca954x.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 58f342108e..5ea0b60b8a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2039,6 +2039,12 @@ S: Maintained
> >  F: hw/net/tulip.c
> >  F: hw/net/tulip.h
> >
> > +pca954x
> > +M: Patrick Venture <venture@google.com>
> > +S: Maintained
> > +F: hw/i2c/i2c_mux_pca954x.c
> > +F: include/hw/i2c/i2c_mux_pca954x.h
> > +
> >  Generic Loader
> >  M: Alistair Francis <alistair@alistair23.me>
> >  S: Maintained
> > diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig
> > index 09642a6dcb..8d120a25d5 100644
> > --- a/hw/i2c/Kconfig
> > +++ b/hw/i2c/Kconfig
> > @@ -28,3 +28,7 @@ config IMX_I2C
> >  config MPC_I2C
> >      bool
> >      select I2C
> > +
> > +config PCA954X
> > +    bool
> > +    select I2C
> > diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
> > new file mode 100644
> > index 0000000000..9aa1a8872f
> > --- /dev/null
> > +++ b/hw/i2c/i2c_mux_pca954x.c
> > @@ -0,0 +1,290 @@
> > +/*
> > + * I2C multiplexer for PCA954x series of I2C multiplexer/switch chips.
> > + *
> > + * Copyright 2021 Google LLC
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License as published by 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/i2c/i2c.h"
> > +#include "hw/i2c/i2c_mux_pca954x.h"
> > +#include "hw/i2c/smbus_slave.h"
> > +#include "hw/qdev-core.h"
> > +#include "hw/sysbus.h"
> > +#include "qemu/log.h"
> > +#include "qemu/module.h"
> > +#include "qemu/queue.h"
> > +#include "qom/object.h"
> > +#include "trace.h"
> > +
> > +#define PCA9548_CHANNEL_COUNT 8
> > +#define PCA9546_CHANNEL_COUNT 4
> > +
> > +/*
> > + * struct Pca954xChannel - The i2c mux device will have N of these states
> > + * that own the i2c channel bus.
> > + * @bus: The owned channel bus.
> > + * @enabled: Is this channel active?
> > + */
> > +typedef struct Pca954xChannel {
> > +    SysBusDevice parent;
> > +
> > +    I2CBus       *bus;
> > +
> > +    bool         enabled;
> > +} Pca954xChannel;
> > +
> > +#define TYPE_PCA954X_CHANNEL "pca954x-channel"
> > +#define PCA954X_CHANNEL(obj) \
> > +    OBJECT_CHECK(Pca954xChannel, (obj), TYPE_PCA954X_CHANNEL)
> > +
> > +/*
> > + * struct Pca954xState - The pca954x state object.
> > + * @control: The value written to the mux control.
> > + * @channel: The set of i2c channel buses that act as channels which own the
> > + * i2c children.
> > + */
> > +typedef struct Pca954xState {
> > +    SMBusDevice parent;
> > +
> > +    uint8_t control;
> > +
> > +    /* The channel i2c buses. */
> > +    Pca954xChannel channel[PCA9548_CHANNEL_COUNT];
> > +} Pca954xState;
> > +
> > +/*
> > + * struct Pca954xClass - The pca954x class object.
> > + * @nchans: The number of i2c channels this device has.
> > + */
> > +typedef struct Pca954xClass {
> > +    SMBusDeviceClass parent;
> > +
> > +    uint8_t nchans;
> > +} Pca954xClass;
> > +
> > +#define TYPE_PCA954X "pca954x"
> > +OBJECT_DECLARE_TYPE(Pca954xState, Pca954xClass, PCA954X)
> > +
> > +/*
> > + * For each channel, if it's enabled, recursively call match on those children.
> > + */
> > +static bool pca954x_match(I2CSlave *candidate, uint8_t address,
> > +                          bool broadcast,
> > +                          I2CNodeList *current_devs)
> > +{
> > +    Pca954xState *mux = PCA954X(candidate);
> > +    Pca954xClass *mc = PCA954X_GET_CLASS(mux);
> > +    int i;
> > +
> > +    /* They are talking to the mux itself (or all devices enabled. */
>
> Missing close paren in the comment above.  Really minor nit :)

Ack, will fix in v3.

>
> > +    if ((candidate->address == address) || broadcast) {
> > +        I2CNode *node = g_malloc(sizeof(struct I2CNode));
> > +        node->elt = candidate;
> > +        QLIST_INSERT_HEAD(current_devs, node, next);
> > +        if (!broadcast) {
> > +            return true;
> > +        }
> > +    }
> > +
> > +    for (i = 0; i < mc->nchans; i++) {
> > +        if (!mux->channel[i].enabled) {
> > +            continue;
> > +        }
> > +
> > +        if (i2c_scan_bus(mux->channel[i].bus, address, broadcast)) {
> > +            if (!broadcast) {
> > +                return true;
> > +            }
> > +        }
> > +    }
> > +
> > +    /* If we arrived here we didn't find a match, return broadcast. */
> > +    return broadcast;
> > +}
> > +
> > +static void pca954x_enable_channel(Pca954xState *s, uint8_t enable_mask)
> > +{
> > +    Pca954xClass *mc = PCA954X_GET_CLASS(s);
> > +    int i;
> > +
> > +    /*
> > +     * For each channel, check if their bit is set in enable_mask and if yes,
> > +     * enable it, otherwise disable, hide it.
> > +     */
> > +    for (i = 0; i < mc->nchans; i++) {
> > +        if (enable_mask & (1 << i)) {
> > +            s->channel[i].enabled = true;
> > +        } else {
> > +            s->channel[i].enabled = false;
> > +        }
> > +    }
> > +}
> > +
> > +static void pca954x_write(Pca954xState *s, uint8_t data)
> > +{
> > +    s->control = data;
> > +    pca954x_enable_channel(s, data);
> > +
> > +    trace_pca954x_write_bytes(data);
> > +}
> > +
> > +static int pca954x_write_data(SMBusDevice *d, uint8_t *buf, uint8_t len)
> > +{
> > +    Pca954xState *s = PCA954X(d);
> > +
> > +    if (len == 0) {
> > +        qemu_log_mask(LOG_GUEST_ERROR, "%s: writing empty data\n", __func__);
> > +        return -1;
> > +    }
> > +
> > +    /*
> > +     * len should be 1, because they write one byte to enable/disable channels.
> > +     */
> > +    if (len > 1) {
> > +        qemu_log_mask(LOG_GUEST_ERROR,
> > +            "%s: extra data after channel selection mask\n",
> > +            __func__);
> > +        return -1;
> > +    }
> > +
> > +    pca954x_write(s, buf[0]);
> > +    return 0;
> > +}
> > +
> > +static uint8_t pca954x_read_byte(SMBusDevice *d)
> > +{
> > +    Pca954xState *s = PCA954X(d);
> > +    uint8_t data = s->control;
> > +    trace_pca954x_read_data(data);
> > +    return data;
> > +}
> > +
> > +static void pca954x_enter_reset(Object *obj, ResetType type)
> > +{
> > +    Pca954xState *s = PCA954X(obj);
> > +    /* Reset will disable all channels. */
> > +    pca954x_write(s, 0);
> > +}
> > +
> > +I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel)
> > +{
> > +    Pca954xClass *pc = PCA954X_GET_CLASS(mux);
> > +    Pca954xState *pca954x = PCA954X(mux);
> > +
> > +    g_assert(channel < pc->nchans);
> > +    return I2C_BUS(qdev_get_child_bus(DEVICE(&pca954x->channel[channel]),
> > +                                      "i2c-bus"));
> > +}
> > +
> > +static void pca954x_smbus_init(Object *obj)
> > +{
> > +    Pca954xChannel *s = PCA954X_CHANNEL(obj);
> > +    s->bus = i2c_init_bus(DEVICE(s), "i2c-bus");
> > +
> > +    /* Start all channels as disabled. */
> > +    s->enabled = false;
> > +}
> > +
> > +static void pca954x_channel_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +    dc->desc = "Pca954x Channel";
> > +}
> > +
> > +static void pca9546_class_init(ObjectClass *klass, void *data)
> > +{
> > +    Pca954xClass *s = PCA954X_CLASS(klass);
> > +    s->nchans = PCA9546_CHANNEL_COUNT;
> > +}
> > +
> > +static void pca9548_class_init(ObjectClass *oc, void *data)
> > +{
> > +    Pca954xClass *s = PCA954X_CLASS(oc);
> > +    s->nchans = PCA9548_CHANNEL_COUNT;
> > +}
> > +
> > +static void pca954x_realize(DeviceState *dev, Error **errp)
> > +{
> > +    Pca954xState *s = PCA954X(dev);
> > +    Pca954xClass *c = PCA954X_GET_CLASS(s);
> > +    int i;
> > +
> > +    /* SMBus modules. Cannot fail. */
> > +    for (i = 0; i < c->nchans; i++) {
> > +        Object *obj = OBJECT(&s->channel[i]);
> > +        sysbus_realize(SYS_BUS_DEVICE(obj), &error_abort);
> > +    }
> > +}
> > +
> > +static void pca954x_init(Object *obj)
> > +{
> > +    int i;
> > +    Pca954xState *s = PCA954X(obj);
> > +    Pca954xClass *c = PCA954X_GET_CLASS(obj);
> > +
> > +    /* Only initialize the children we expect. */
> > +    for (i = 0; i < c->nchans; i++) {
> > +        object_initialize_child(obj, "channel[*]", &s->channel[i],
> > +                                TYPE_PCA954X_CHANNEL);
> > +    }
> > +}
> > +
> > +static void pca954x_class_init(ObjectClass *klass, void *data)
> > +{
> > +    I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
> > +    ResettableClass *rc = RESETTABLE_CLASS(klass);
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +    SMBusDeviceClass *k = SMBUS_DEVICE_CLASS(klass);
> > +
> > +    sc->match_and_add = pca954x_match;
> > +
> > +    rc->phases.enter = pca954x_enter_reset;
> > +
> > +    dc->desc = "Pca954x i2c-mux";
> > +    dc->realize = pca954x_realize;
> > +
> > +    k->write_data = pca954x_write_data;
> > +    k->receive_byte = pca954x_read_byte;
> > +}
> > +
> > +static const TypeInfo pca954x_info[] = {
> > +    {
> > +        .name          = TYPE_PCA954X,
> > +        .parent        = TYPE_SMBUS_DEVICE,
> > +        .instance_size = sizeof(Pca954xState),
> > +        .instance_init = pca954x_init,
> > +        .class_size    = sizeof(Pca954xClass),
> > +        .class_init    = pca954x_class_init,
> > +        .abstract      = true,
> > +    },
> > +    {
> > +        .name          = TYPE_PCA9546,
> > +        .parent        = TYPE_PCA954X,
> > +        .class_init    = pca9546_class_init,
> > +    },
> > +    {
> > +        .name          = TYPE_PCA9548,
> > +        .parent        = TYPE_PCA954X,
> > +        .class_init    = pca9548_class_init,
> > +    },
> > +    {
> > +        .name = TYPE_PCA954X_CHANNEL,
> > +        .parent = TYPE_SYS_BUS_DEVICE,
> > +        .class_init = pca954x_channel_class_init,
> > +        .instance_size = sizeof(Pca954xChannel),
> > +        .instance_init = pca954x_smbus_init,
> > +    }
> > +};
> > +
> > +DEFINE_TYPES(pca954x_info)
> > diff --git a/hw/i2c/meson.build b/hw/i2c/meson.build
> > index cdcd694a7f..dd3aef02b2 100644
> > --- a/hw/i2c/meson.build
> > +++ b/hw/i2c/meson.build
> > @@ -14,4 +14,5 @@ i2c_ss.add(when: 'CONFIG_SMBUS_EEPROM', if_true: files('smbus_eeprom.c'))
> >  i2c_ss.add(when: 'CONFIG_VERSATILE_I2C', if_true: files('versatile_i2c.c'))
> >  i2c_ss.add(when: 'CONFIG_OMAP', if_true: files('omap_i2c.c'))
> >  i2c_ss.add(when: 'CONFIG_PPC4XX', if_true: files('ppc4xx_i2c.c'))
> > +i2c_ss.add(when: 'CONFIG_PCA954X', if_true: files('i2c_mux_pca954x.c'))
> >  softmmu_ss.add_all(when: 'CONFIG_I2C', if_true: i2c_ss)
> > diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
> > index 82fe6f965f..82f19e6a2d 100644
> > --- a/hw/i2c/trace-events
> > +++ b/hw/i2c/trace-events
> > @@ -26,3 +26,8 @@ npcm7xx_smbus_recv_byte(const char *id, uint8_t value) "%s recv byte: 0x%02x"
> >  npcm7xx_smbus_stop(const char *id) "%s stopping"
> >  npcm7xx_smbus_nack(const char *id) "%s nacking"
> >  npcm7xx_smbus_recv_fifo(const char *id, uint8_t received, uint8_t expected) "%s recv fifo: received %u, expected %u"
> > +
> > +# i2c-mux-pca954x.c
> > +
> > +pca954x_write_bytes(uint8_t value) "PCA954X write data: 0x%02x"
> > +pca954x_read_data(uint8_t value) "PCA954X read data: 0x%02x"
> > diff --git a/include/hw/i2c/i2c_mux_pca954x.h b/include/hw/i2c/i2c_mux_pca954x.h
> > new file mode 100644
> > index 0000000000..8aaf9bbc39
> > --- /dev/null
> > +++ b/include/hw/i2c/i2c_mux_pca954x.h
> > @@ -0,0 +1,19 @@
> > +#ifndef QEMU_I2C_MUX_PCA954X
> > +#define QEMU_I2C_MUX_PCA954X
> > +
> > +#include "hw/i2c/i2c.h"
> > +
> > +#define TYPE_PCA9546 "pca9546"
> > +#define TYPE_PCA9548 "pca9548"
> > +
> > +/**
> > + * Retrieves the i2c bus associated with the specified channel on this i2c
> > + * mux.
> > + * @mux: an i2c mux device.
> > + * @channel: the i2c channel requested
> > + *
> > + * Returns: a pointer to the associated i2c bus.
> > + */
> > +I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel);
>
> I assume your machine-specific code will be referencing this, which is
> the way it should work.

Yes. The ARM board inits create these devices, and then have a pointer
to then request specific channels.

>
> I'm happy with this.
>
> -corey
>
> > +
> > +#endif
> > --
> > 2.31.1.295.g9ea45b61b8-goog
> >
Philippe Mathieu-Daudé April 9, 2021, 9:20 p.m. UTC | #5
+Paolo/Thomas

On 4/9/21 7:21 PM, Patrick Venture wrote:
> On Fri, Apr 9, 2021 at 9:51 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> Hi Patrick,
>>
>> On 4/9/21 6:25 PM, Patrick Venture wrote:
>>> The pca954x is an i2c mux, and this adds support for two variants of
>>> this device: the pca9546 and pca9548.
>>>
>>> This device is very common on BMCs to route a different channel to each
>>> PCIe i2c bus downstream from the BMC.
>>>
>>> Signed-off-by: Patrick Venture <venture@google.com>
>>> Reviewed-by: Hao Wu <wuhaotsh@google.com>
>>> Reviewed-by: Havard Skinnemoen <hskinnemoen@google.com>
>>> ---
>>>  MAINTAINERS                      |   6 +
>>>  hw/i2c/Kconfig                   |   4 +
>>>  hw/i2c/i2c_mux_pca954x.c         | 290 +++++++++++++++++++++++++++++++
>>>  hw/i2c/meson.build               |   1 +
>>>  hw/i2c/trace-events              |   5 +
>>>  include/hw/i2c/i2c_mux_pca954x.h |  19 ++
>>>  6 files changed, 325 insertions(+)
>>>  create mode 100644 hw/i2c/i2c_mux_pca954x.c
>>>  create mode 100644 include/hw/i2c/i2c_mux_pca954x.h
>>
>>> diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig
>>> index 09642a6dcb..8d120a25d5 100644
>>> --- a/hw/i2c/Kconfig
>>> +++ b/hw/i2c/Kconfig
>>> @@ -28,3 +28,7 @@ config IMX_I2C
>>>  config MPC_I2C
>>>      bool
>>>      select I2C
>>> +
>>> +config PCA954X
>>> +    bool
>>> +    select I2C
>>
>> Do you have a circular dependency when also using:
>>
>>        depends on I2C
>>
>> ?
> 
> I'm somewhat new to qemu -- I don't know what you mean, since I2C
> doesn't depend on pca954x, I don't imagine there could be a circular
> dependency.

See
https://qemu-project.gitlab.io/qemu/devel/kconfig.html#guidelines-for-writing-kconfig-files

PCA954X is plugged on an I2C bus
-> depends on I2C

PCA954X provides I2C buses
-> select I2C

Your device is a particular case consuming and providing the Kconfig
'I2C' symbol. I expect a circular dependency problem. Easy to test with
your series but I haven't.

I suppose in this case, the "select" takes over on "depends on" so this
is OK.

Now (unrelated to your series) thinking at the graphical Kconfig tree
representation (like this one generated 2 years ago:
https://drive.google.com/open?id=1kvwl7guuAmCh2Y2UqeXynlA2HmjWcRs9),
I'd rather see a circular dep.

Regards,

Phil.
Patrick Venture April 9, 2021, 9:32 p.m. UTC | #6
On Fri, Apr 9, 2021 at 2:20 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> +Paolo/Thomas
>
> On 4/9/21 7:21 PM, Patrick Venture wrote:
> > On Fri, Apr 9, 2021 at 9:51 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >>
> >> Hi Patrick,
> >>
> >> On 4/9/21 6:25 PM, Patrick Venture wrote:
> >>> The pca954x is an i2c mux, and this adds support for two variants of
> >>> this device: the pca9546 and pca9548.
> >>>
> >>> This device is very common on BMCs to route a different channel to each
> >>> PCIe i2c bus downstream from the BMC.
> >>>
> >>> Signed-off-by: Patrick Venture <venture@google.com>
> >>> Reviewed-by: Hao Wu <wuhaotsh@google.com>
> >>> Reviewed-by: Havard Skinnemoen <hskinnemoen@google.com>
> >>> ---
> >>>  MAINTAINERS                      |   6 +
> >>>  hw/i2c/Kconfig                   |   4 +
> >>>  hw/i2c/i2c_mux_pca954x.c         | 290 +++++++++++++++++++++++++++++++
> >>>  hw/i2c/meson.build               |   1 +
> >>>  hw/i2c/trace-events              |   5 +
> >>>  include/hw/i2c/i2c_mux_pca954x.h |  19 ++
> >>>  6 files changed, 325 insertions(+)
> >>>  create mode 100644 hw/i2c/i2c_mux_pca954x.c
> >>>  create mode 100644 include/hw/i2c/i2c_mux_pca954x.h
> >>
> >>> diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig
> >>> index 09642a6dcb..8d120a25d5 100644
> >>> --- a/hw/i2c/Kconfig
> >>> +++ b/hw/i2c/Kconfig
> >>> @@ -28,3 +28,7 @@ config IMX_I2C
> >>>  config MPC_I2C
> >>>      bool
> >>>      select I2C
> >>> +
> >>> +config PCA954X
> >>> +    bool
> >>> +    select I2C
> >>
> >> Do you have a circular dependency when also using:
> >>
> >>        depends on I2C
> >>
> >> ?
> >
> > I'm somewhat new to qemu -- I don't know what you mean, since I2C
> > doesn't depend on pca954x, I don't imagine there could be a circular
> > dependency.
>
> See
> https://qemu-project.gitlab.io/qemu/devel/kconfig.html#guidelines-for-writing-kconfig-files
>
> PCA954X is plugged on an I2C bus
> -> depends on I2C
>
> PCA954X provides I2C buses
> -> select I2C

So from the guide it looks like my KConfig should have _depends_ on
I2C.  My board that I'm testing with selects PCA954X and doesn't
explicitly select I2C.  My device _does_ provide I2C buses, as you
say.

>
> Your device is a particular case consuming and providing the Kconfig
> 'I2C' symbol. I expect a circular dependency problem. Easy to test with
> your series but I haven't.
>
> I suppose in this case, the "select" takes over on "depends on" so this
> is OK.

I have to imagine there is a similar situation for PCIe bridges, as
they depend on PCI but also provide it.

>
> Now (unrelated to your series) thinking at the graphical Kconfig tree
> representation (like this one generated 2 years ago:
> https://drive.google.com/open?id=1kvwl7guuAmCh2Y2UqeXynlA2HmjWcRs9),
> I'd rather see a circular dep.
>
> Regards,
>
> Phil.
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 58f342108e..5ea0b60b8a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2039,6 +2039,12 @@  S: Maintained
 F: hw/net/tulip.c
 F: hw/net/tulip.h
 
+pca954x
+M: Patrick Venture <venture@google.com>
+S: Maintained
+F: hw/i2c/i2c_mux_pca954x.c
+F: include/hw/i2c/i2c_mux_pca954x.h
+
 Generic Loader
 M: Alistair Francis <alistair@alistair23.me>
 S: Maintained
diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig
index 09642a6dcb..8d120a25d5 100644
--- a/hw/i2c/Kconfig
+++ b/hw/i2c/Kconfig
@@ -28,3 +28,7 @@  config IMX_I2C
 config MPC_I2C
     bool
     select I2C
+
+config PCA954X
+    bool
+    select I2C
diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
new file mode 100644
index 0000000000..9aa1a8872f
--- /dev/null
+++ b/hw/i2c/i2c_mux_pca954x.c
@@ -0,0 +1,290 @@ 
+/*
+ * I2C multiplexer for PCA954x series of I2C multiplexer/switch chips.
+ *
+ * Copyright 2021 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by 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/i2c/i2c.h"
+#include "hw/i2c/i2c_mux_pca954x.h"
+#include "hw/i2c/smbus_slave.h"
+#include "hw/qdev-core.h"
+#include "hw/sysbus.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/queue.h"
+#include "qom/object.h"
+#include "trace.h"
+
+#define PCA9548_CHANNEL_COUNT 8
+#define PCA9546_CHANNEL_COUNT 4
+
+/*
+ * struct Pca954xChannel - The i2c mux device will have N of these states
+ * that own the i2c channel bus.
+ * @bus: The owned channel bus.
+ * @enabled: Is this channel active?
+ */
+typedef struct Pca954xChannel {
+    SysBusDevice parent;
+
+    I2CBus       *bus;
+
+    bool         enabled;
+} Pca954xChannel;
+
+#define TYPE_PCA954X_CHANNEL "pca954x-channel"
+#define PCA954X_CHANNEL(obj) \
+    OBJECT_CHECK(Pca954xChannel, (obj), TYPE_PCA954X_CHANNEL)
+
+/*
+ * struct Pca954xState - The pca954x state object.
+ * @control: The value written to the mux control.
+ * @channel: The set of i2c channel buses that act as channels which own the
+ * i2c children.
+ */
+typedef struct Pca954xState {
+    SMBusDevice parent;
+
+    uint8_t control;
+
+    /* The channel i2c buses. */
+    Pca954xChannel channel[PCA9548_CHANNEL_COUNT];
+} Pca954xState;
+
+/*
+ * struct Pca954xClass - The pca954x class object.
+ * @nchans: The number of i2c channels this device has.
+ */
+typedef struct Pca954xClass {
+    SMBusDeviceClass parent;
+
+    uint8_t nchans;
+} Pca954xClass;
+
+#define TYPE_PCA954X "pca954x"
+OBJECT_DECLARE_TYPE(Pca954xState, Pca954xClass, PCA954X)
+
+/*
+ * For each channel, if it's enabled, recursively call match on those children.
+ */
+static bool pca954x_match(I2CSlave *candidate, uint8_t address,
+                          bool broadcast,
+                          I2CNodeList *current_devs)
+{
+    Pca954xState *mux = PCA954X(candidate);
+    Pca954xClass *mc = PCA954X_GET_CLASS(mux);
+    int i;
+
+    /* They are talking to the mux itself (or all devices enabled. */
+    if ((candidate->address == address) || broadcast) {
+        I2CNode *node = g_malloc(sizeof(struct I2CNode));
+        node->elt = candidate;
+        QLIST_INSERT_HEAD(current_devs, node, next);
+        if (!broadcast) {
+            return true;
+        }
+    }
+
+    for (i = 0; i < mc->nchans; i++) {
+        if (!mux->channel[i].enabled) {
+            continue;
+        }
+
+        if (i2c_scan_bus(mux->channel[i].bus, address, broadcast)) {
+            if (!broadcast) {
+                return true;
+            }
+        }
+    }
+
+    /* If we arrived here we didn't find a match, return broadcast. */
+    return broadcast;
+}
+
+static void pca954x_enable_channel(Pca954xState *s, uint8_t enable_mask)
+{
+    Pca954xClass *mc = PCA954X_GET_CLASS(s);
+    int i;
+
+    /*
+     * For each channel, check if their bit is set in enable_mask and if yes,
+     * enable it, otherwise disable, hide it.
+     */
+    for (i = 0; i < mc->nchans; i++) {
+        if (enable_mask & (1 << i)) {
+            s->channel[i].enabled = true;
+        } else {
+            s->channel[i].enabled = false;
+        }
+    }
+}
+
+static void pca954x_write(Pca954xState *s, uint8_t data)
+{
+    s->control = data;
+    pca954x_enable_channel(s, data);
+
+    trace_pca954x_write_bytes(data);
+}
+
+static int pca954x_write_data(SMBusDevice *d, uint8_t *buf, uint8_t len)
+{
+    Pca954xState *s = PCA954X(d);
+
+    if (len == 0) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: writing empty data\n", __func__);
+        return -1;
+    }
+
+    /*
+     * len should be 1, because they write one byte to enable/disable channels.
+     */
+    if (len > 1) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+            "%s: extra data after channel selection mask\n",
+            __func__);
+        return -1;
+    }
+
+    pca954x_write(s, buf[0]);
+    return 0;
+}
+
+static uint8_t pca954x_read_byte(SMBusDevice *d)
+{
+    Pca954xState *s = PCA954X(d);
+    uint8_t data = s->control;
+    trace_pca954x_read_data(data);
+    return data;
+}
+
+static void pca954x_enter_reset(Object *obj, ResetType type)
+{
+    Pca954xState *s = PCA954X(obj);
+    /* Reset will disable all channels. */
+    pca954x_write(s, 0);
+}
+
+I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel)
+{
+    Pca954xClass *pc = PCA954X_GET_CLASS(mux);
+    Pca954xState *pca954x = PCA954X(mux);
+
+    g_assert(channel < pc->nchans);
+    return I2C_BUS(qdev_get_child_bus(DEVICE(&pca954x->channel[channel]),
+                                      "i2c-bus"));
+}
+
+static void pca954x_smbus_init(Object *obj)
+{
+    Pca954xChannel *s = PCA954X_CHANNEL(obj);
+    s->bus = i2c_init_bus(DEVICE(s), "i2c-bus");
+
+    /* Start all channels as disabled. */
+    s->enabled = false;
+}
+
+static void pca954x_channel_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    dc->desc = "Pca954x Channel";
+}
+
+static void pca9546_class_init(ObjectClass *klass, void *data)
+{
+    Pca954xClass *s = PCA954X_CLASS(klass);
+    s->nchans = PCA9546_CHANNEL_COUNT;
+}
+
+static void pca9548_class_init(ObjectClass *oc, void *data)
+{
+    Pca954xClass *s = PCA954X_CLASS(oc);
+    s->nchans = PCA9548_CHANNEL_COUNT;
+}
+
+static void pca954x_realize(DeviceState *dev, Error **errp)
+{
+    Pca954xState *s = PCA954X(dev);
+    Pca954xClass *c = PCA954X_GET_CLASS(s);
+    int i;
+
+    /* SMBus modules. Cannot fail. */
+    for (i = 0; i < c->nchans; i++) {
+        Object *obj = OBJECT(&s->channel[i]);
+        sysbus_realize(SYS_BUS_DEVICE(obj), &error_abort);
+    }
+}
+
+static void pca954x_init(Object *obj)
+{
+    int i;
+    Pca954xState *s = PCA954X(obj);
+    Pca954xClass *c = PCA954X_GET_CLASS(obj);
+
+    /* Only initialize the children we expect. */
+    for (i = 0; i < c->nchans; i++) {
+        object_initialize_child(obj, "channel[*]", &s->channel[i],
+                                TYPE_PCA954X_CHANNEL);
+    }
+}
+
+static void pca954x_class_init(ObjectClass *klass, void *data)
+{
+    I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SMBusDeviceClass *k = SMBUS_DEVICE_CLASS(klass);
+
+    sc->match_and_add = pca954x_match;
+
+    rc->phases.enter = pca954x_enter_reset;
+
+    dc->desc = "Pca954x i2c-mux";
+    dc->realize = pca954x_realize;
+
+    k->write_data = pca954x_write_data;
+    k->receive_byte = pca954x_read_byte;
+}
+
+static const TypeInfo pca954x_info[] = {
+    {
+        .name          = TYPE_PCA954X,
+        .parent        = TYPE_SMBUS_DEVICE,
+        .instance_size = sizeof(Pca954xState),
+        .instance_init = pca954x_init,
+        .class_size    = sizeof(Pca954xClass),
+        .class_init    = pca954x_class_init,
+        .abstract      = true,
+    },
+    {
+        .name          = TYPE_PCA9546,
+        .parent        = TYPE_PCA954X,
+        .class_init    = pca9546_class_init,
+    },
+    {
+        .name          = TYPE_PCA9548,
+        .parent        = TYPE_PCA954X,
+        .class_init    = pca9548_class_init,
+    },
+    {
+        .name = TYPE_PCA954X_CHANNEL,
+        .parent = TYPE_SYS_BUS_DEVICE,
+        .class_init = pca954x_channel_class_init,
+        .instance_size = sizeof(Pca954xChannel),
+        .instance_init = pca954x_smbus_init,
+    }
+};
+
+DEFINE_TYPES(pca954x_info)
diff --git a/hw/i2c/meson.build b/hw/i2c/meson.build
index cdcd694a7f..dd3aef02b2 100644
--- a/hw/i2c/meson.build
+++ b/hw/i2c/meson.build
@@ -14,4 +14,5 @@  i2c_ss.add(when: 'CONFIG_SMBUS_EEPROM', if_true: files('smbus_eeprom.c'))
 i2c_ss.add(when: 'CONFIG_VERSATILE_I2C', if_true: files('versatile_i2c.c'))
 i2c_ss.add(when: 'CONFIG_OMAP', if_true: files('omap_i2c.c'))
 i2c_ss.add(when: 'CONFIG_PPC4XX', if_true: files('ppc4xx_i2c.c'))
+i2c_ss.add(when: 'CONFIG_PCA954X', if_true: files('i2c_mux_pca954x.c'))
 softmmu_ss.add_all(when: 'CONFIG_I2C', if_true: i2c_ss)
diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
index 82fe6f965f..82f19e6a2d 100644
--- a/hw/i2c/trace-events
+++ b/hw/i2c/trace-events
@@ -26,3 +26,8 @@  npcm7xx_smbus_recv_byte(const char *id, uint8_t value) "%s recv byte: 0x%02x"
 npcm7xx_smbus_stop(const char *id) "%s stopping"
 npcm7xx_smbus_nack(const char *id) "%s nacking"
 npcm7xx_smbus_recv_fifo(const char *id, uint8_t received, uint8_t expected) "%s recv fifo: received %u, expected %u"
+
+# i2c-mux-pca954x.c
+
+pca954x_write_bytes(uint8_t value) "PCA954X write data: 0x%02x"
+pca954x_read_data(uint8_t value) "PCA954X read data: 0x%02x"
diff --git a/include/hw/i2c/i2c_mux_pca954x.h b/include/hw/i2c/i2c_mux_pca954x.h
new file mode 100644
index 0000000000..8aaf9bbc39
--- /dev/null
+++ b/include/hw/i2c/i2c_mux_pca954x.h
@@ -0,0 +1,19 @@ 
+#ifndef QEMU_I2C_MUX_PCA954X
+#define QEMU_I2C_MUX_PCA954X
+
+#include "hw/i2c/i2c.h"
+
+#define TYPE_PCA9546 "pca9546"
+#define TYPE_PCA9548 "pca9548"
+
+/**
+ * Retrieves the i2c bus associated with the specified channel on this i2c
+ * mux.
+ * @mux: an i2c mux device.
+ * @channel: the i2c channel requested
+ *
+ * Returns: a pointer to the associated i2c bus.
+ */
+I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel);
+
+#endif