Messages in this thread | | | Date | Sat, 11 Feb 2023 17:11:35 +0100 | From | Andrew Lunn <> | Subject | Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC |
| |
> + > +#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?
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
???
> + 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.
Andrew
| |