Re: [PATCH 09/23] hw/ppc/mpc8544_guts: Populate POR PLL ratio status register

[Bernhard Beschow]

Am 24. September 2024 09:59:21 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Mon, 23 Sep 2024, Bernhard Beschow wrote:
>> Am 23. September 2024 10:43:19 UTC schrieb BALATON Zoltan 
>> <balaton@eik.bme.hu>:
>>> On Mon, 23 Sep 2024, Bernhard Beschow wrote:
>>>> Populate this read-only register with some arbitrary values which avoids
>>>> U-Boot's get_clocks() to hang().
>>> 
>>> Maybe this should be a property settable by the machine as each board may 
>>> have different values and it may need to use the correct value for the 
>>> machine.
>> 
>> I actually considered this but went with the pragmatic solution to avoid 
>> over-engineering. In particular, I wanted to avoid further machine-specitic 
>> attributes in the machine class struct. Or do you expect a new e500 machine 
>> to be added? In that case I'd set above arbitrary values as default and 
>> expect a new machine to override these properties.
>
>Can't override if there's no property for it. There's one machine I may be 
>interested in that uses a Freescale e500 SoC. That one seems to use 0x0606180c 
>for this value which I think corresponds to 0/1 Ratio both 3:1, DDR Ratio 12:1 
>and Plat Ratio 6:1. I think one property to set the 32 bit value without 
>individual fields would be enough and we can put comments next to the value if 
>needed to note what components it comes from. Or if you just need any value 
>here maybe you could take this one then that would be good for me as well.

Let's use your numbers then without a property. Any specific machine I should 
mention in the commit message? Tabor?

> (I have some patches adding second i2c bus and SPD data that are needed for 
> U-Boot for memory detection but it needs more clean up before I can submit it 
> and also waiting for these patches to avoid conflict.)

Neat. That means we'll get DDR3 support?

Best regards,
Bernhard

>
>Regards,
>BALATON Zoltan
>
>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>> ---
>>>> hw/ppc/mpc8544_guts.c | 12 ++++++++++++
>>>> 1 file changed, 12 insertions(+)
>>>> 
>>>> diff --git a/hw/ppc/mpc8544_guts.c b/hw/ppc/mpc8544_guts.c
>>>> index e3540b0281..6688fd44c3 100644
>>>> --- a/hw/ppc/mpc8544_guts.c
>>>> +++ b/hw/ppc/mpc8544_guts.c
>>>> @@ -29,6 +29,12 @@
>>>> #define MPC8544_GUTS_RSTCR_RESET      0x02
>>>> 
>>>> #define MPC8544_GUTS_ADDR_PORPLLSR    0x00
>>>> +REG32(GUTS_PORPLLSR, 0x00)
>>>> +    FIELD(GUTS_PORPLLSR, E500_1_RATIO, 16, 6)
>>>> +    FIELD(GUTS_PORPLLSR, E500_0_RATIO, 16, 6)
>>>> +    FIELD(GUTS_PORPLLSR, DDR_RATIO, 9, 5)
>>>> +    FIELD(GUTS_PORPLLSR, PLAT_RATIO, 1, 5)
>>>> +
>>>> #define MPC8544_GUTS_ADDR_PORBMSR     0x04
>>>> #define MPC8544_GUTS_ADDR_PORIMPSCR   0x08
>>>> #define MPC8544_GUTS_ADDR_PORDEVSR    0x0C
>>>> @@ -75,6 +81,12 @@ static uint64_t mpc8544_guts_read(void *opaque, hwaddr 
>>>> addr,
>>>> 
>>>>     addr &= MPC8544_GUTS_MMIO_SIZE - 1;
>>>>     switch (addr) {
>>>> +    case MPC8544_GUTS_ADDR_PORPLLSR:
>>>> +        value = FIELD_DP32(value, GUTS_PORPLLSR, E500_1_RATIO, 3); /* 3:2 
>>>> */
>>>> +        value = FIELD_DP32(value, GUTS_PORPLLSR, E500_0_RATIO, 3); /* 3:2 
>>>> */
>>>> +        value = FIELD_DP32(value, GUTS_PORPLLSR, DDR_RATIO, 6); /* 6:1 */
>>>> +        value = FIELD_DP32(value, GUTS_PORPLLSR, PLAT_RATIO, 4); /* 4:1 */
>>>> +        break;
>>>>     case MPC8544_GUTS_ADDR_PVR:
>>>>         value = env->spr[SPR_PVR];
>>>>         break;
>>>> 
>> 
>>