linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: Christian Eggers <ceggers@arri.de>
Cc: Arun Ramadoss <arun.ramadoss@microchip.com>,
	Pavan Chebbi <pavan.chebbi@broadcom.com>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	woojung.huh@microchip.com, UNGLinuxDriver@microchip.com,
	andrew@lunn.ch, vivien.didelot@gmail.com, f.fainelli@gmail.com,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, linux@armlinux.org.uk,
	Tristram.Ha@microchip.com, richardcochran@gmail.com
Subject: Re: [Patch net-next v1 01/12] net: dsa: microchip: ptp: add the posix clock support
Date: Thu, 1 Dec 2022 01:05:03 +0200	[thread overview]
Message-ID: <20221130230503.elc4hbxl6fcemx7y@skbuf> (raw)
In-Reply-To: <5639053.DvuYhMxLoT@n95hx1g2>

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.

  reply	other threads:[~2022-11-30 23:05 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-28 10:32 [Patch net-next v1 00/12] net: dsa: microchip: add PTP support for KSZ9563/KSZ8563 and LAN937x Arun Ramadoss
2022-11-28 10:32 ` [Patch net-next v1 01/12] net: dsa: microchip: ptp: add the posix clock support Arun Ramadoss
2022-11-28 14:49   ` Pavan Chebbi
2022-11-28 14:56     ` Christian Eggers
2022-11-30 23:05       ` Vladimir Oltean [this message]
2022-11-30  4:53     ` Arun.Ramadoss
2022-12-01  0:17   ` Vladimir Oltean
2022-12-01 10:01     ` Arun.Ramadoss
2022-11-28 10:32 ` [Patch net-next v1 02/12] net: dsa: microchip: ptp: Initial hardware time stamping support Arun Ramadoss
2022-11-29  8:49   ` Pavan Chebbi
2022-11-30  4:32     ` Arun.Ramadoss
2022-12-01  0:39   ` Vladimir Oltean
2022-12-01 10:17     ` Arun.Ramadoss
2022-11-28 10:32 ` [Patch net-next v1 03/12] net: dsa: microchip: ptp: add 4 bytes in tail tag when ptp enabled Arun Ramadoss
2022-12-01  0:52   ` Vladimir Oltean
2022-12-01 10:56     ` Arun.Ramadoss
2022-11-28 10:32 ` [Patch net-next v1 04/12] net: dsa: microchip: ptp: Manipulating absolute time using ptp hw clock Arun Ramadoss
2022-11-29  8:43   ` Pavan Chebbi
2022-11-30  4:22     ` Arun.Ramadoss
2022-11-30  6:11       ` Pavan Chebbi
2022-12-01  1:04   ` Vladimir Oltean
2022-12-02  9:40     ` Arun.Ramadoss
2022-11-28 10:32 ` [Patch net-next v1 05/12] net: dsa: microchip: ptp: enable interrupt for timestamping Arun Ramadoss
2022-11-28 10:32 ` [Patch net-next v1 06/12] net: ptp: add helper for one-step P2P clocks Arun Ramadoss
2022-11-28 10:32 ` [Patch net-next v1 07/12] net: dsa: microchip: ptp: add packet reception timestamping Arun Ramadoss
2022-11-29  0:43   ` kernel test robot
2022-11-28 10:32 ` [Patch net-next v1 08/12] net: dsa: microchip: ptp: add packet transmission timestamping Arun Ramadoss
2022-11-28 10:32 ` [Patch net-next v1 09/12] net: dsa: microchip: ptp: move pdelay_rsp correction field to tail tag Arun Ramadoss
2022-11-28 10:32 ` [Patch net-next v1 10/12] net: dsa: microchip: ptp: add 2 step timestamping for LAN937x Arun Ramadoss
2022-11-28 10:32 ` [Patch net-next v1 11/12] net: dsa: microchip: ptp: add periodic output signal Arun Ramadoss
2022-11-29  8:53   ` Pavan Chebbi
2022-11-29  9:57     ` Pavan Chebbi
2022-11-30  4:48       ` Arun.Ramadoss
2022-11-30  4:41     ` Arun.Ramadoss
2022-11-28 10:32 ` [Patch net-next v1 12/12] net: dsa: microchip: ptp: add support for perout programmable pins Arun Ramadoss

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221130230503.elc4hbxl6fcemx7y@skbuf \
    --to=olteanv@gmail.com \
    --cc=Tristram.Ha@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=arun.ramadoss@microchip.com \
    --cc=ceggers@arri.de \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pavan.chebbi@broadcom.com \
    --cc=richardcochran@gmail.com \
    --cc=vivien.didelot@gmail.com \
    --cc=woojung.huh@microchip.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).