| Message ID | 20240923093016.66437-10-shentey@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | E500 Cleanup | expand |
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. 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; >
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. 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; >>
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. (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.) 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; >>> > >
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; >>>> >> >>
On Tue, 24 Sep 2024, Bernhard Beschow wrote: > 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? That's also OK with me. No need to mention any specific machine as I'm not sure I'll ever finish it. So far it's only an experiment and only planned to submit the small patches that came out of it but it's not emulating any machine yet. >> (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? That's the plan eventually but I don't know when will I have time to do that. So far I had a hard coded SPD data for testing but I wanted to update the generating function for a while but lacked time for it. This gives another reason to do that finally but may take a while. Regards, BALATON Zoltan
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;
Populate this read-only register with some arbitrary values which avoids U-Boot's get_clocks() to hang(). Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- hw/ppc/mpc8544_guts.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)