Re: [PATCH 2/2] hw/arm/palm.c: Encapsulate misc GPIO handling in a device

[Philippe Mathieu-Daudé]

On 7/13/20 12:31 PM, Peter Maydell wrote:
> On Mon, 13 Jul 2020 at 11:21, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> On 7/13/20 12:05 PM, Peter Maydell wrote:
>>> On Mon, 13 Jul 2020 at 09:57, Philippe Mathieu-Daudé <f4bug@amsat.org> 
>>> wrote:
>>>> Why not make it a generic container in the MachineState and create
>>>> the container in hw/core/machine.c::machine_initfn()?
>>>
>>> I don't think we create containers like that for any other
>>> machine, do we?
>>
>> No but maybe we could. Most boards have some GPIO/LED/reset switch
>> button. Do all machines have a NUMA memory device? Do all machines
>> have a dtb? Do all machines use NVDIMM devices? I think we have
>> more machines using GPIOs than machine using NVDIMM. Anyway I don't
>> mind, I was just trying to figure where this container belong on QOM.
> 
> I think that if machines were qdev objects with the usual
> reset/gpio/etc capabilities, I might have just implemented
> this as part of the machine object; but they aren't, and
> it didn't really seem like the right approach to create an
> ad-hoc "container that sort of corresponds to the whole
> machine". Also, since these machines are largely orphan
> I tend to favour smaller-scale interventions that push them
> in a better direction rather than more sweeping changes.

Fair enough. There is something that bugs me in the MachineClass,
but this is not this series fault. I guess by adding such containers
in machines, we'll eventually figure out what is the best QOM design
for it.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>