Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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", &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.


       Andrew

Thanks for reviewing,
Cristian

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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux