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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221130230503.elc4hbxl6fcemx7y@skbuf>
Date:   Thu, 1 Dec 2022 01:05:03 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Christian Eggers <ceggers@...i.de>
Cc:     Arun Ramadoss <arun.ramadoss@...rochip.com>,
        Pavan Chebbi <pavan.chebbi@...adcom.com>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        woojung.huh@...rochip.com, UNGLinuxDriver@...rochip.com,
        andrew@...n.ch, vivien.didelot@...il.com, f.fainelli@...il.com,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, linux@...linux.org.uk,
        Tristram.Ha@...rochip.com, richardcochran@...il.com
Subject: Re: [Patch net-next v1 01/12] net: dsa: microchip: ptp: add the
 posix clock support

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.