Re: [PATCH 05/12] hw/ssi: Introduce a ssi_get_cs() helper

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

+QOM tinkerers

On 31/5/23 07:59, Cédric Le Goater wrote:
> On 5/30/23 23:15, Philippe Mathieu-Daudé wrote:
>> On 30/5/23 22:34, Philippe Mathieu-Daudé wrote:
>>> On 8/5/23 09:58, Cédric Le Goater wrote:
>>>> Simple routine to retrieve a DeviceState object on a SPI bus using its
>>>> address/cs. It will be useful for the board to wire the CS lines.
>>>>
>>>> Cc: Alistair Francis <alistair@alistair23.me>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> ---
>>>>   include/hw/ssi/ssi.h |  2 ++
>>>>   hw/ssi/ssi.c         | 15 +++++++++++++++
>>>>   2 files changed, 17 insertions(+)
>>>>
>>>> diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
>>>> index ffd3a34ba4..c7beabdb09 100644
>>>> --- a/include/hw/ssi/ssi.h
>>>> +++ b/include/hw/ssi/ssi.h
>>>> @@ -112,4 +112,6 @@ SSIBus *ssi_create_bus(DeviceState *parent, 
>>>> const char *name);
>>>>   uint32_t ssi_transfer(SSIBus *bus, uint32_t val);
>>>> +DeviceState *ssi_get_cs(SSIBus *bus, int addr);
>>
>> Also, this helper should (preferably) return a SSIPeripheral type.
> 
> Well, having a DeviceState is handy for the callers (today) and
> ssi_create_peripheral() returns a DeviceState. Let's keep it that
> way.

Yes I know it is handy :) I'm not against your patch; besides other
APIs do that. I'm wondering about QOM design here. Having Foo device,
should FOO API return the common qdev abstract type (DeviceState) or
a Foo type? Either ways we keep QOM-casting around, but I still tend
to consider FOO API returning Foo pointer provides some type check
safety, and also provides the API user hints about what is used.
Need more coffee.