On Wed, 15 Feb 2023 at 01:09, Cristian Ciocaltea
[off-list ref] wrote:

On 2/11/23 18:11, Andrew Lunn wrote:

quoted

quoted

+
+#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

quoted

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.

quoted

quoted

+    if (!of_property_read_u32(np, "starfive,gtxclk-dlychain", &gtxclk_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.

quoted

      Andrew

Thanks for reviewing,
Cristian

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel