Hi Wolfram, On 20.09.2023 22:41, Wolfram Sang wrote:
Hi Dirk, long time no see!
Got a chance to look at kernel 6.1 :)
There are Renesas ARM Trusted Firmware version out there which e.g. configure 0x201 (shifted logical right by 2: 0x80) and with this match the added { STPnHCK | 0, 1 }:IIRC, that means that the ATF uses 200MHz for the data channel but disables the 800MHz for the SCC. Because of that, I assume ATF doesn't do tuning then? Isn't that risky to operate at 200MHz without tuning?static const struct clk_div_table cpg_sdh_div_table[] = { - { 0, 1 }, { 1, 2 }, { STPnHCK | 2, 4 }, { STPnHCK | 3, 8 }, - { STPnHCK | 4, 16 }, { 0, 0 }, + { 0, 1 }, { 1, 2 }, { STPnHCK | 0, 1 }, { STPnHCK | 1, 2 }, + { STPnHCK | 2, 4 }, { STPnHCK | 3, 8 }, { STPnHCK | 4, 16 }, { 0, 0 },Anyhow, since such ATF seems to be in the wild then, I assume we should at least support reading such configuration values. I'd reorder it like this, though: + { 0, 1 }, { STPnHCK | 0, 1 }, { 1, 2 }, { STPnHCK | 1, 2 }, + { STPnHCK | 2, 4 }, { STPnHCK | 3, 8 }, { STPnHCK | 4, 16 }, { 0, 0 }, And probably add a comment that the duplicate entries are only for reading and are not recommended for use with Linux (which will still use the first matching pair i.e. without STPnHCK).
Yes, I can do that :)
Geert, does this all make sense to you?
I was just wondering why this table includes STPnHCK? I mean, this is 'just' a translation table between the value read from the register to the divider (1, 2, 4, 8, 16)? Couldn't we mask 7 instead of 8 bits and with this drop STPnHCK from the comparison? The resulting divider would stay the same. Or even better just mask SDnSRCFC[2:0] (i.e. 3 bits)?
Best regards dirk
