Messages in this thread | | | Date | Wed, 15 Feb 2023 13:51:26 +0200 | Subject | Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC | From | Cristian Ciocaltea <> |
| |
On 2/15/23 13:20, Emil Renner Berthing wrote: > On Wed, 15 Feb 2023 at 01:09, Cristian Ciocaltea > <cristian.ciocaltea@collabora.com> 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@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv
| |