qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH 1/3] hw/i2c/smbus_eeprom: Set QOM parent


From: Philippe Mathieu-Daudé
Subject: Re: [RFC PATCH 1/3] hw/i2c/smbus_eeprom: Set QOM parent
Date: Fri, 26 Jun 2020 16:15:40 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0

On 6/26/20 4:03 PM, BALATON Zoltan wrote:
> On Fri, 26 Jun 2020, Philippe Mathieu-Daudé wrote:
>> + Eduardo / Mark / Edgard / Alistair / Fred for QOM design.
>>
>> On 6/26/20 12:54 PM, BALATON Zoltan wrote:
>>> On Fri, 26 Jun 2020, BALATON Zoltan wrote:
>>>> On Fri, 26 Jun 2020, Philippe Mathieu-Daudé wrote:
>>>>> Suggested-by: Markus Armbruster <armbru@redhat.com>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>> ---
>>>>> Aspeed change pending latest ARM pull-request, so meanwhile sending
>>>>> as RFC.
>>>>> ---
>>>>> include/hw/i2c/smbus_eeprom.h |  9 ++++++---
>>>>> hw/i2c/smbus_eeprom.c         | 13 ++++++++++---
>>>>> hw/mips/fuloong2e.c           |  2 +-
>>>>> hw/ppc/sam460ex.c             |  2 +-
>>>>> 4 files changed, 18 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/include/hw/i2c/smbus_eeprom.h
>>>>> b/include/hw/i2c/smbus_eeprom.h
>>>>> index 68b0063ab6..037612bbbb 100644
>>>>> --- a/include/hw/i2c/smbus_eeprom.h
>>>>> +++ b/include/hw/i2c/smbus_eeprom.h
>>>>> @@ -26,9 +26,12 @@
>>>>> #include "exec/cpu-common.h"
>>>>> #include "hw/i2c/i2c.h"
>>>>>
>>>>> -void smbus_eeprom_init_one(I2CBus *bus, uint8_t address, uint8_t
>>>>> *eeprom_buf);
>>>>> -void smbus_eeprom_init(I2CBus *bus, int nb_eeprom,
>>>>> -                       const uint8_t *eeprom_spd, int size);
>>>>> +void smbus_eeprom_init_one(Object *parent_obj, const char
>>>>> *child_name,
>>>>> +                           I2CBus *smbus, uint8_t address,
>>>>> +                           uint8_t *eeprom_buf);
>>>>> +void smbus_eeprom_init(Object *parent_obj, const char
>>>>> *child_name_prefix,
>>>>> +                       I2CBus *smbus, int nb_eeprom,
>>>>> +                       const uint8_t *eeprom_spd, int
>>>>> eeprom_spd_size);
>>>>
>>>> Keeping I2CBus *smbus and uint8_t address as first parameters before
>>>> parent_obj and name looks better to me. These functions still operate
>>>> on an I2Cbus so could be regarded as methods of I2CBus therefore first
>>>> parameter should be that.
>>>
>>> Also isn't parent_obj is the I2Cbus itself? Why is that need to be
>>> passed? The i2c_init_bus() also takes parent and name params so both
>>> I2Cbus and it's parent should be available as parents of the new I2C
>>> device here without more parameters. What am I missing here?
>>
>> This is where I'm confused too and what I want to resolve with this
>> RFC series :)
>>
>> The SPD EEPROM is soldered on the DIMM module. The DIMM exposes the
>> memory address/data pins and the i2c pins. We plug DIMMs on a
>> (mother)board.
>>
>> I see the DIMM module being the parent. As we don't model it in QOM,
>> I used the MemoryRegion (which is what the SPD is describing).
>>
>> We could represent the DIMM as a container of DRAM + SPD EEPROM, but
>> it makes the modeling slightly more complex. The only benefit is a
>> clearer modeling.
>>
>> I'm not sure why the I2C bus is expected to be the parent. Maybe an
>> old wrong assumption?
> 
> I guess it's a question of what the parent should mean? Is it parent of
> the object in which case it's the I2CBus (which is kind of logical view
> of the object tree modelling the machine) or the parent of the thing
> modelled in the machine (which is physical view of the machine
> components) then it should be the RAM module. The confusion probably
> comes from this question not answered. Also the DIMM module is not
> modelled so when you assign SPD eeproms to memory region it could be
> multiple SPD eeproms will be parented by a single RAM memory region
> (possibly not covering it fully as in the mac_oldworld or sam460ex case
> discussed in another thread). This does not seem too intuitive.

>From the bus perspective, requests are sent hoping for a device to
answer to the requested address ("Hello, do I have children? Hello?
Anybody here?"), if nobody is here, the request timeouts.
So there is not really a strong family relationship here.

If you unplug a DIMM, you remove both the MemoryRegion and the EEPROM.
This is how I understand the QOM parent relationship so far (if you
remove a parent, you also remove its children).

> 
> Regards,
> BALATON Zoltan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]