netdev - Re: [PATCH net v2 2/3] net: dsa: mv88e6xxx: read cycle counter period from hardware

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: <f33e48fd-b8fe-4c7f-9180-fe6d23c1e48a@lunn.ch>
Date: Sun, 13 Oct 2024 19:01:42 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Shenghao Yang <me@...nghaoyang.info>
Cc: netdev@...r.kernel.org, f.fainelli@...il.com, olteanv@...il.com,
	pavana.sharma@...i.com, ashkan.boldaji@...i.com, kabel@...nel.org
Subject: Re: [PATCH net v2 2/3] net: dsa: mv88e6xxx: read cycle counter
 period from hardware

On Sun, Oct 13, 2024 at 01:26:53PM +0800, Shenghao Yang wrote:
> 
> >> diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
> >>  struct mv88e6xxx_ptp_ops;
> >>  struct mv88e6xxx_pcs_ops;
> >> +struct mv88e6xxx_cc_coeffs;
> >>  
> >> -struct mv88e6xxx_cc_coeffs;
> >> -
> > 
> > It is better to put it in the correct place with the first patch,
> > rather than move it in the second patch.
> 
> Hi Andrew,
> 
> Thanks! Happy to spin a v3 if preferred.
>  
> >>  	memset(&chip->tstamp_cc, 0, sizeof(chip->tstamp_cc));
> >>  	chip->tstamp_cc.read	= mv88e6xxx_ptp_clock_read;
> >>  	chip->tstamp_cc.mask	= CYCLECOUNTER_MASK(32);
> >> -	chip->tstamp_cc.mult	= ptp_ops->cc_coeffs->cc_mult;
> >> -	chip->tstamp_cc.shift	= ptp_ops->cc_coeffs->cc_shift;
> >> +	chip->tstamp_cc.mult	= chip->cc_coeffs->cc_mult;
> >> +	chip->tstamp_cc.shift	= chip->cc_coeffs->cc_shift;
> > 
> > Once these patches are merged, it would be nice to remove
> > chip->tstamp_cc.mult and chip->tstamp_cc.shift and use
> > chip->cc_coeffs->cc_mult and chip->cc_coeffs->cc_shift. We don't need
> > the same values in two places.
> 
> I've looked around a bit and this doesn't seem possible - the common
> timecounter code in time/timecounter.c and linux/timecounter.h has a
> dependency on the cc_mult and cc_shift fields within the cyclecounter
> tstamp_cc.

Ah, sorry. I did not see that tstamp_cc was a timecounter structure.

Maybe have 3 const struct cyclecounter similar to you having 3 const
mv88e6xxx_cc_coeffs. Assign the appropriate one to chip. mult and
shift can then be dropped from mv88ex6xxx_cc_coeffs?

	Andrew