On 2/15/23 13:20, Emil Renner Berthing wrote:
On Wed, 15 Feb 2023 at 01:09, Cristian Ciocaltea
<cristian.ciocaltea@xxxxxxxxxxxxx> wrote:
On 2/11/23 18:11, Andrew Lunn wrote:
+
+#define JH7100_SYSMAIN_REGISTER28 0x70
+/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */
+#define JH7100_SYSMAIN_REGISTER49 0xc8
Seems like the comment should be one line earlier?
Well yes, the very generic register names are also bad, but this
comment refers to the fact that it kind of makes sense that register
28 has the offset
28 * 4 bytes pr. register = 0x70
..but then register 49 is oddly out of place at offset 0xc8 instead of
49 * 4 bytes pr. register = 0xc4
There is value in basing the names on the datasheet, but you could
append something meaningful on the end:
#define JH7100_SYSMAIN_REGISTER49_DLYCHAIN 0xc8
Unfortunately the JH7100 datasheet I have access to doesn't provide any
information regarding the SYSCTRL-MAINSYS related registers. Maybe Emil
could provide some details here?
This is reverse engineered from the auto generated headers in their u-boot:
https://github.com/starfive-tech/u-boot/blob/JH7100_VisionFive_devel/arch/riscv/include/asm/arch-jh7100/syscon_sysmain_ctrl_macro.h
Christian, I'm happy that you're working on this, but mess like this
and waiting for the non-coherent dma to be sorted is why I didn't send
it upstream yet.
Thank you for clarifying this and for all the work you have done so far,
Emil! If you don't mind, I would be glad to continue helping with this
mainlining effort.
+ if (!of_property_read_u32(np, "starfive,gtxclk-dlychain", >xclk_dlychain)) {
+ ret = regmap_write(sysmain, JH7100_SYSMAIN_REGISTER49, gtxclk_dlychain);
+ if (ret)
+ return dev_err_probe(dev, ret, "error selecting gtxclk delay chain\n");
+ }
You should probably document that if starfive,gtxclk-dlychain is not
found in the DT blob, the value for the delay chain is undefined. It
would actually be better to define it, set it to 0 for example. That
way, you know you don't have any dependency on the bootloader for
example.
Sure, I will set it to 0.
Andrew
Thanks for reviewing,
Cristian
_______________________________________________
linux-riscv mailing list
linux-riscv@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-riscv
