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

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(+)

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
>

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(+)

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(+)
>

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.

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.