1. Patchew
  2. QEMU
  3. View series

[PATCH RESEND v2] hw/i2c: Enable an id for the pca954x devices

Patrick Venture posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230322172136.48010-1-venture@google.com
Maintainers: Patrick Venture <venture@google.com>
hw/i2c/i2c_mux_pca954x.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
[PATCH RESEND v2] hw/i2c: Enable an id for the pca954x devices
Posted by Patrick Venture 1 year ago 
This allows the devices to be more readily found and specified.
Without setting the name field, they can only be found by device type
name, which doesn't let you specify the second of the same device type
behind a bus.

Tested: Verified that by default the device was findable with the name
'pca954x[77]', for an instance attached at that address.

Signed-off-by: Patrick Venture <venture@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
v2: s/id/name/g to use name as the identifier field. left 'id' in subject for email chain.
---
 hw/i2c/i2c_mux_pca954x.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
index 3945de795c..76e69bebc5 100644
--- a/hw/i2c/i2c_mux_pca954x.c
+++ b/hw/i2c/i2c_mux_pca954x.c
@@ -20,6 +20,7 @@
 #include "hw/i2c/i2c_mux_pca954x.h"
 #include "hw/i2c/smbus_slave.h"
 #include "hw/qdev-core.h"
+#include "hw/qdev-properties.h"
 #include "hw/sysbus.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
@@ -43,6 +44,8 @@ typedef struct Pca954xState {
 
     bool enabled[PCA9548_CHANNEL_COUNT];
     I2CBus *bus[PCA9548_CHANNEL_COUNT];
+
+    char *name;
 } Pca954xState;
 
 /*
@@ -181,6 +184,17 @@ static void pca9548_class_init(ObjectClass *klass, void *data)
     s->nchans = PCA9548_CHANNEL_COUNT;
 }
 
+static void pca954x_realize(DeviceState *dev, Error **errp)
+{
+    Pca954xState *s = PCA954X(dev);
+    DeviceState *d = DEVICE(s);
+    if (s->name) {
+        d->id = g_strdup(s->name);
+    } else {
+        d->id = g_strdup_printf("pca954x[%x]", s->parent.i2c.address);
+    }
+}
+
 static void pca954x_init(Object *obj)
 {
     Pca954xState *s = PCA954X(obj);
@@ -197,6 +211,11 @@ static void pca954x_init(Object *obj)
     }
 }
 
+static Property pca954x_props[] = {
+    DEFINE_PROP_STRING("nane", Pca954xState, name),
+    DEFINE_PROP_END_OF_LIST()
+};
+
 static void pca954x_class_init(ObjectClass *klass, void *data)
 {
     I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
@@ -209,9 +228,12 @@ static void pca954x_class_init(ObjectClass *klass, void *data)
     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;
+
+    device_class_set_props(dc, pca954x_props);
 }
 
 static const TypeInfo pca954x_info[] = {
-- 
2.40.0.rc1.284.g88254d51c5-goog
Re: [PATCH RESEND v2] hw/i2c: Enable an id for the pca954x devices
Posted by Corey Minyard 1 year ago 
On Wed, Mar 22, 2023 at 10:21:36AM -0700, Patrick Venture wrote:
> This allows the devices to be more readily found and specified.
> Without setting the name field, they can only be found by device type
> name, which doesn't let you specify the second of the same device type
> behind a bus.
> 
> Tested: Verified that by default the device was findable with the name
> 'pca954x[77]', for an instance attached at that address.

This looks good to me.

Acked-by: Corey Minyard <cminyard@mvista.com>

if you are taking this in through another tree.  Or do you want me to
take this?

-corey

> 
> Signed-off-by: Patrick Venture <venture@google.com>
> Reviewed-by: Hao Wu <wuhaotsh@google.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> v2: s/id/name/g to use name as the identifier field. left 'id' in subject for email chain.
> ---
>  hw/i2c/i2c_mux_pca954x.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
> index 3945de795c..76e69bebc5 100644
> --- a/hw/i2c/i2c_mux_pca954x.c
> +++ b/hw/i2c/i2c_mux_pca954x.c
> @@ -20,6 +20,7 @@
>  #include "hw/i2c/i2c_mux_pca954x.h"
>  #include "hw/i2c/smbus_slave.h"
>  #include "hw/qdev-core.h"
> +#include "hw/qdev-properties.h"
>  #include "hw/sysbus.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> @@ -43,6 +44,8 @@ typedef struct Pca954xState {
>  
>      bool enabled[PCA9548_CHANNEL_COUNT];
>      I2CBus *bus[PCA9548_CHANNEL_COUNT];
> +
> +    char *name;
>  } Pca954xState;
>  
>  /*
> @@ -181,6 +184,17 @@ static void pca9548_class_init(ObjectClass *klass, void *data)
>      s->nchans = PCA9548_CHANNEL_COUNT;
>  }
>  
> +static void pca954x_realize(DeviceState *dev, Error **errp)
> +{
> +    Pca954xState *s = PCA954X(dev);
> +    DeviceState *d = DEVICE(s);
> +    if (s->name) {
> +        d->id = g_strdup(s->name);
> +    } else {
> +        d->id = g_strdup_printf("pca954x[%x]", s->parent.i2c.address);
> +    }
> +}
> +
>  static void pca954x_init(Object *obj)
>  {
>      Pca954xState *s = PCA954X(obj);
> @@ -197,6 +211,11 @@ static void pca954x_init(Object *obj)
>      }
>  }
>  
> +static Property pca954x_props[] = {
> +    DEFINE_PROP_STRING("nane", Pca954xState, name),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
>  static void pca954x_class_init(ObjectClass *klass, void *data)
>  {
>      I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
> @@ -209,9 +228,12 @@ static void pca954x_class_init(ObjectClass *klass, void *data)
>      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;
> +
> +    device_class_set_props(dc, pca954x_props);
>  }
>  
>  static const TypeInfo pca954x_info[] = {
> -- 
> 2.40.0.rc1.284.g88254d51c5-goog
>
Re: [PATCH RESEND v2] hw/i2c: Enable an id for the pca954x devices
Posted by Philippe Mathieu-Daudé 1 year ago 
On 22/3/23 22:19, Corey Minyard wrote:
> On Wed, Mar 22, 2023 at 10:21:36AM -0700, Patrick Venture wrote:
>> This allows the devices to be more readily found and specified.
>> Without setting the name field, they can only be found by device type
>> name, which doesn't let you specify the second of the same device type
>> behind a bus.
>>
>> Tested: Verified that by default the device was findable with the name
>> 'pca954x[77]', for an instance attached at that address.
> 
> This looks good to me.
> 
> Acked-by: Corey Minyard <cminyard@mvista.com>
> 
> if you are taking this in through another tree.  Or do you want me to
> take this?

Since I have to send a MIPS PR, I'll take this one;
to alleviate you and the CI minutes.
Re: [PATCH RESEND v2] hw/i2c: Enable an id for the pca954x devices
Posted by Patrick Venture 10 months, 3 weeks ago 
On Wed, Mar 22, 2023 at 2:40 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> On 22/3/23 22:19, Corey Minyard wrote:
> > On Wed, Mar 22, 2023 at 10:21:36AM -0700, Patrick Venture wrote:
> >> This allows the devices to be more readily found and specified.
> >> Without setting the name field, they can only be found by device type
> >> name, which doesn't let you specify the second of the same device type
> >> behind a bus.
> >>
> >> Tested: Verified that by default the device was findable with the name
> >> 'pca954x[77]', for an instance attached at that address.
> >
> > This looks good to me.
> >
> > Acked-by: Corey Minyard <cminyard@mvista.com>
> >
> > if you are taking this in through another tree.  Or do you want me to
> > take this?
>
> Since I have to send a MIPS PR, I'll take this one;
> to alleviate you and the CI minutes.
>

I don't see this patch yet, did it got lost in the shuffle?

thanks,
Patrick
Re: [PATCH RESEND v2] hw/i2c: Enable an id for the pca954x devices
Posted by Philippe Mathieu-Daudé 10 months, 2 weeks ago 
Hi Patrick,

On 31/5/23 18:34, Patrick Venture wrote:
> 
> 
> On Wed, Mar 22, 2023 at 2:40 PM Philippe Mathieu-Daudé 
> <philmd@linaro.org <mailto:philmd@linaro.org>> wrote:
> 
>     On 22/3/23 22:19, Corey Minyard wrote:
>      > On Wed, Mar 22, 2023 at 10:21:36AM -0700, Patrick Venture wrote:
>      >> This allows the devices to be more readily found and specified.
>      >> Without setting the name field, they can only be found by device
>     type
>      >> name, which doesn't let you specify the second of the same
>     device type
>      >> behind a bus.
>      >>
>      >> Tested: Verified that by default the device was findable with
>     the name
>      >> 'pca954x[77]', for an instance attached at that address.
>      >
>      > This looks good to me.
>      >
>      > Acked-by: Corey Minyard <cminyard@mvista.com
>     <mailto:cminyard@mvista.com>>
>      >
>      > if you are taking this in through another tree.  Or do you want me to
>      > take this?
> 
>     Since I have to send a MIPS PR, I'll take this one;
>     to alleviate you and the CI minutes.
> 
> 
> I don't see this patch yet, did it got lost in the shuffle?

I quickly tried to test the patch before sending the PR and it was not
working, so I dropped it; but since it was a busy day I neglected to
post an update on the list. I apologize for that.

Revisiting the patch, the problem is trivial, a simple typo:

  +static Property pca954x_props[] = {
  +    DEFINE_PROP_STRING("nane", Pca954xState, name),
                           ^^^^
  +    DEFINE_PROP_END_OF_LIST()
  +};

I'm queuing this patch again with s/nane/name/.

Regards,

Phil.

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.