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]
Date: Thu, 30 Nov 2023 20:38:17 +0100
From: Christian Marangi <ansuelsmth@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Andy Gross <agross@...nel.org>,
	Bjorn Andersson <andersson@...nel.org>,
	Konrad Dybcio <konrad.dybcio@...aro.org>,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	linux-arm-msm@...r.kernel.org
Subject: Re: [net-next PATCH 06/14] net: phy: at803x: move at8031 specific
 data out of generic at803x_priv

On Thu, Nov 30, 2023 at 04:21:50PM +0100, Andrew Lunn wrote:
> > +struct at8031_data {
> > +	bool is_fiber;
> > +	bool is_1000basex;
> > +	struct regulator_dev *vddio_rdev;
> > +	struct regulator_dev *vddh_rdev;
> > +};
> > +
> >  struct at803x_priv {
> >  	int flags;
> >  	u16 clk_25m_reg;
> >  	u16 clk_25m_mask;
> >  	u8 smarteee_lpi_tw_1g;
> >  	u8 smarteee_lpi_tw_100m;
> > -	bool is_fiber;
> > -	bool is_1000basex;
> > -	struct regulator_dev *vddio_rdev;
> > -	struct regulator_dev *vddh_rdev;
> > +
> > +	/* Specific data for at8031 PHYs */
> > +	void *data;
> >  };
> 
> I don't really like this void *
> 
> Go through at803x_priv and find out what is common to them all, and
> keep that in one structure. Add per family private structures which
> include the common as a member.

As you notice later in the patches, only at803x have stuff in common
qca803xx and qca808x doesn't use the struct at all (aside from stats)

And in the at803x PHY family only at8031 have fiber 1000basex and
regulators.

> 
> By having real types everywhere you get the compiler doing checks for
> you.

Main problem is that adding something like
'struct at8031_data* at8031_data' looks also bad.

Maybe I can rework the 2 bool to flags (they are used only by at803x)
and keep the 2 regulator pointer?

> 
> As Russell pointed out, this patch series is going to be too big. So
> break it up. We can move fast on patches which are simple and
> obviously correct.
> 
-- 
	Ansuel

Powered by blists - more mailing lists