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