Re: [Patch net-next v1 01/12] net: dsa: microchip: ptp: add the posix clock support

From: Vladimir Oltean
Date: Wed Nov 30 2022 - 18:05:14 EST


On Mon, Nov 28, 2022 at 03:56:30PM +0100, Christian Eggers wrote:
> > > +#define PTP_LOAD_TIME BIT(3)
> >
> > PTP_WRITE_TIME sounds more intuitive than PTP_LOAD_TIME?
>
> PTP_LOAD_TIME has been derived from the data sheet:
>
> -------------8<--------------
> PTP Clock Load
> --------------
> Setting this bit will cause the PTP clock to be loaded with the time value in
> registers 0x0502 to 0x050B.
> ------------->8--------------
>
> I would also prefer PTP_WRITE_TIME. But is it ok to deviate from data sheet?

It depends. When the datasheet has succint and uniquely identifiable
names for registers, there's no reason to not use them. Exceptions are
obnoxious things like "BASIC_MODE_CONTROL_REGISTER" or "MDIO_CONTROL_1"
which get abbreviated in kernel code to "BMCR" and "MDIO_CTRL1".

When the register names in the datasheet are literally prose ("PTP Clock Load",
with spaces and all), some divergence from the datasheet will have to
exist no matter what you do. So I guess you can just go for what makes
the most sense and is in line with existing kernel conventions. People
who cross-reference kernel definitions with the datasheet will still
have a hell of a life, but hey, you can tell them it's not your fault
you can't name a C variable "PTP Clock Load".

OTOH, I don't find "PTP_LOAD_TIME" unintuitive, but I won't oppose a
rename if there's agreement from people who care more than me.