public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Richard Cochran <richardcochran@gmail.com>,
	netdev@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Subject: Re: [net-next 4/4] net: ethernet: renesas: rcar_gen4_ptp: Hide private data from users
Date: Tue, 3 Feb 2026 07:39:10 +0100	[thread overview]
Message-ID: <20260203063910.GB2323039@ragnatech.se> (raw)
In-Reply-To: <20260202191121.71326ca2@kernel.org>

Hello Jakub,

Thanks for your feedback.

On 2026-02-02 19:11:21 -0800, Jakub Kicinski wrote:
> On Sun,  1 Feb 2026 19:37:45 +0100 Niklas Söderlund wrote:
> > The Gen4 PTP helper module is already used by RTSN and RSWITCH to
> > support PTP clocks and will be used by RAVB too. Hide the Gen4 PTP
> > private data structure to make sure none of the users poke at it.
> > 
> > This will be more important for RAVB use-cases as more then one RAVB
> > device will need to cooperate using one PTP clock source.
> 
> IMO hiding type definitions in C is an anti-pattern.
> Sooner or later you'll need a sizeof or a static inline helper 
> and you'll have to bend over backwards to access the type info.
> 
> We can take this if you have a strong preference, but please
> think again if you really need this or it's a code cleanliness
> instinct carried over from objective programming, hence foreign..

On principle I agree with you, hiding type definitions is usually not a 
good idea long term, but here I would still like to do it.

My reason is that the goal is to completely remove the type from the 
header file and only expose the struct ptp_clock to users.

The private data will need to be reworked as the next device where 
support for the Gen4 PTP clock need to support the same PTP clock device 
to be used by multiple network devices. This will make the private data 
more fragile and I don't want to expose that to the users as the helpers 
will then be mandatory to use.

I'm sure there are multiple paths to achieve my goal, but unless you 
strongly object against this intermediate step I would like to move 
forward with it.

-- 
Kind Regards,
Niklas Söderlund

  reply	other threads:[~2026-02-03  6:39 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-01 18:37 [net-next 0/4] net: ethernet: renesas: rcar_gen4_ptp: Hide private data Niklas Söderlund
2026-02-01 18:37 ` [net-next 1/4] net: ethernet: renesas: rcar_gen4_ptp: Move address assignment Niklas Söderlund
2026-02-01 18:37 ` [net-next 2/4] net: ethernet: renesas: rcar_gen4_ptp: Add helper to get clock index Niklas Söderlund
2026-02-01 18:37 ` [net-next 3/4] net: ethernet: renesas: rcar_gen4_ptp: Add helper to read time Niklas Söderlund
2026-02-01 18:37 ` [net-next 4/4] net: ethernet: renesas: rcar_gen4_ptp: Hide private data from users Niklas Söderlund
2026-02-03  3:11   ` Jakub Kicinski
2026-02-03  6:39     ` Niklas Söderlund [this message]
2026-02-04  3:50 ` [net-next 0/4] net: ethernet: renesas: rcar_gen4_ptp: Hide private data patchwork-bot+netdevbpf

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=20260203063910.GB2323039@ragnatech.se \
    --to=niklas.soderlund+renesas@ragnatech.se \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=geert+renesas@glider.be \
    --cc=kuba@kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=magnus.damm@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=richardcochran@gmail.com \
    --cc=yoshihiro.shimoda.uh@renesas.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