diff mbox series

[v2,04/12] hw/ssi: Add an "addr" property to SSIPeripheral

Message ID 20230607043943.1837186-5-clg@kaod.org (mailing list archive)
State New, archived
Headers show
Series aspeed: fixes and extensions | expand

Commit Message

Cédric Le Goater June 7, 2023, 4:39 a.m. UTC 
Boards will use this new property to identify the device CS line and
wire the SPI controllers accordingly.

Cc: Alistair Francis <alistair@alistair23.me>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ssi/ssi.h | 3 +++
 hw/ssi/ssi.c         | 7 +++++++
 2 files changed, 10 insertions(+)

Comments

Joel Stanley June 7, 2023, 8:06 a.m. UTC | #1 
On Wed, 7 Jun 2023 at 04:40, Cédric Le Goater <clg@kaod.org> wrote:
>
> Boards will use this new property to identify the device CS line and
> wire the SPI controllers accordingly.

"addr" and not "cs" or even "chip-select"?

Reviewed-by: Joel Stanley <joel@jms.id.au>

>
> Cc: Alistair Francis <alistair@alistair23.me>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/ssi/ssi.h | 3 +++
>  hw/ssi/ssi.c         | 7 +++++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
> index 6950f86810d3..9e0706a5248c 100644
> --- a/include/hw/ssi/ssi.h
> +++ b/include/hw/ssi/ssi.h
> @@ -64,6 +64,9 @@ struct SSIPeripheral {
>
>      /* Chip select state */
>      bool cs;
> +
> +    /* Chip select address/index */
> +    uint8_t addr;
>  };
>
>  extern const VMStateDescription vmstate_ssi_peripheral;
> diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c
> index d54a109beeb5..d4409535429c 100644
> --- a/hw/ssi/ssi.c
> +++ b/hw/ssi/ssi.c
> @@ -13,6 +13,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "hw/qdev-properties.h"
>  #include "hw/ssi/ssi.h"
>  #include "migration/vmstate.h"
>  #include "qemu/module.h"
> @@ -71,6 +72,11 @@ static void ssi_peripheral_realize(DeviceState *dev, Error **errp)
>      ssc->realize(s, errp);
>  }
>
> +static Property ssi_peripheral_properties[] = {
> +    DEFINE_PROP_UINT8("addr", SSIPeripheral, addr, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void ssi_peripheral_class_init(ObjectClass *klass, void *data)
>  {
>      SSIPeripheralClass *ssc = SSI_PERIPHERAL_CLASS(klass);
> @@ -81,6 +87,7 @@ static void ssi_peripheral_class_init(ObjectClass *klass, void *data)
>      if (!ssc->transfer_raw) {
>          ssc->transfer_raw = ssi_transfer_raw_default;
>      }
> +    device_class_set_props(dc, ssi_peripheral_properties);
>  }
>
>  static const TypeInfo ssi_peripheral_info = {
> --
> 2.40.1
>
Philippe Mathieu-Daudé June 7, 2023, 8:28 a.m. UTC | #2 
On 7/6/23 10:06, Joel Stanley wrote:
> On Wed, 7 Jun 2023 at 04:40, Cédric Le Goater <clg@kaod.org> wrote:
>>
>> Boards will use this new property to identify the device CS line and
>> wire the SPI controllers accordingly.
> 
> "addr" and not "cs" or even "chip-select"?

"chip-select" is a good suggestion!

> Reviewed-by: Joel Stanley <joel@jms.id.au>
> 
>>
>> Cc: Alistair Francis <alistair@alistair23.me>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   include/hw/ssi/ssi.h | 3 +++
>>   hw/ssi/ssi.c         | 7 +++++++
>>   2 files changed, 10 insertions(+)
Cédric Le Goater June 7, 2023, 2:15 p.m. UTC | #3 
On 6/7/23 10:28, Philippe Mathieu-Daudé wrote:
> On 7/6/23 10:06, Joel Stanley wrote:
>> On Wed, 7 Jun 2023 at 04:40, Cédric Le Goater <clg@kaod.org> wrote:
>>>
>>> Boards will use this new property to identify the device CS line and
>>> wire the SPI controllers accordingly.
>>
>> "addr" and not "cs" or even "chip-select"?
> 
> "chip-select" is a good suggestion!

I thought of using "cs" initially as it makes more sense for SPI
controllers, I do agree. But then, I tried to be consistent with
what QEMU is proposing today : "bus" and "addr".

This can be changed.

Thanks,

C.

> 
>> Reviewed-by: Joel Stanley <joel@jms.id.au>
>>
>>>
>>> Cc: Alistair Francis <alistair@alistair23.me>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>   include/hw/ssi/ssi.h | 3 +++
>>>   hw/ssi/ssi.c         | 7 +++++++
>>>   2 files changed, 10 insertions(+)
>
Philippe Mathieu-Daudé June 29, 2023, 10:56 a.m. UTC | #4 
On 7/6/23 16:15, Cédric Le Goater wrote:
> On 6/7/23 10:28, Philippe Mathieu-Daudé wrote:
>> On 7/6/23 10:06, Joel Stanley wrote:
>>> On Wed, 7 Jun 2023 at 04:40, Cédric Le Goater <clg@kaod.org> wrote:
>>>>
>>>> Boards will use this new property to identify the device CS line and
>>>> wire the SPI controllers accordingly.
>>>
>>> "addr" and not "cs" or even "chip-select"?
>>
>> "chip-select" is a good suggestion!
> 
> I thought of using "cs" initially as it makes more sense for SPI
> controllers, I do agree. But then, I tried to be consistent with
> what QEMU is proposing today : "bus" and "addr".

We should use a description that stays close with the terms used
by the hardware we model. In that case "cs" seems more appropriate.
Cédric Le Goater June 29, 2023, 4:08 p.m. UTC | #5 
On 6/29/23 12:56, Philippe Mathieu-Daudé wrote:
> On 7/6/23 16:15, Cédric Le Goater wrote:
>> On 6/7/23 10:28, Philippe Mathieu-Daudé wrote:
>>> On 7/6/23 10:06, Joel Stanley wrote:
>>>> On Wed, 7 Jun 2023 at 04:40, Cédric Le Goater <clg@kaod.org> wrote:
>>>>>
>>>>> Boards will use this new property to identify the device CS line and
>>>>> wire the SPI controllers accordingly.
>>>>
>>>> "addr" and not "cs" or even "chip-select"?
>>>
>>> "chip-select" is a good suggestion!
>>
>> I thought of using "cs" initially as it makes more sense for SPI
>> controllers, I do agree. But then, I tried to be consistent with
>> what QEMU is proposing today : "bus" and "addr".
> 
> We should use a description that stays close with the terms used
> by the hardware we model. In that case "cs" seems more appropriate.

OK. I can change the property to "cs".

Thanks,

C.
diff mbox series

Patch

diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
index 6950f86810d3..9e0706a5248c 100644
--- a/include/hw/ssi/ssi.h
+++ b/include/hw/ssi/ssi.h
@@ -64,6 +64,9 @@  struct SSIPeripheral {
 
     /* Chip select state */
     bool cs;
+
+    /* Chip select address/index */
+    uint8_t addr;
 };
 
 extern const VMStateDescription vmstate_ssi_peripheral;
diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c
index d54a109beeb5..d4409535429c 100644
--- a/hw/ssi/ssi.c
+++ b/hw/ssi/ssi.c
@@ -13,6 +13,7 @@ 
  */
 
 #include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
 #include "hw/ssi/ssi.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
@@ -71,6 +72,11 @@  static void ssi_peripheral_realize(DeviceState *dev, Error **errp)
     ssc->realize(s, errp);
 }
 
+static Property ssi_peripheral_properties[] = {
+    DEFINE_PROP_UINT8("addr", SSIPeripheral, addr, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void ssi_peripheral_class_init(ObjectClass *klass, void *data)
 {
     SSIPeripheralClass *ssc = SSI_PERIPHERAL_CLASS(klass);
@@ -81,6 +87,7 @@  static void ssi_peripheral_class_init(ObjectClass *klass, void *data)
     if (!ssc->transfer_raw) {
         ssc->transfer_raw = ssi_transfer_raw_default;
     }
+    device_class_set_props(dc, ssi_peripheral_properties);
 }
 
 static const TypeInfo ssi_peripheral_info = {