Linux-Doc Archive on lore.kernel.org
 help / color / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Florinel Iordache <florinel.iordache@nxp.com>
Cc: "davem@davemloft.net" <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"f.fainelli@gmail.com" <f.fainelli@gmail.com>,
	"hkallweit1@gmail.com" <hkallweit1@gmail.com>,
	"linux@armlinux.org.uk" <linux@armlinux.org.uk>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	Leo Li <leoyang.li@nxp.com>,
	"Madalin Bucur (OSS)" <madalin.bucur@oss.nxp.com>,
	Ioana Ciornei <ioana.ciornei@nxp.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next 6/9] net: phy: add backplane kr driver support
Date: Wed, 1 Apr 2020 15:51:35 +0200
Message-ID: <20200401135135.GA62290@lunn.ch> (raw)
In-Reply-To: <AM0PR04MB5443E8D583734C98C54C519EFBC90@AM0PR04MB5443.eurprd04.prod.outlook.com>

On Wed, Apr 01, 2020 at 01:35:36PM +0000, Florinel Iordache wrote:
> > On Thu, Mar 26, 2020 at 03:51:19PM +0200, Florinel Iordache wrote:
> > > +static void setup_supported_linkmode(struct phy_device *bpphy) {
> > > +     struct backplane_phy_info *bp_phy = bpphy->priv;
> > 
> > I'm not sure it is a good idea to completely take over phydev->priv like this, in
> > what is just helper code. What if the PHY driver needs memory of its own? There
> > are a few examples of this already in other PHY drivers. Could a KR PHY contain
> > a temperature sensor? Could it contain statistics counters which need
> > accumulating?
> > 
> >         Andrew
> 
> Backplane KR driver allocates memory for structure backplane_phy_info
> which is saved in phydev->priv. After all this is the purpose of priv
> according to its description in phy.h: <<private data pointer For use
> by PHYs to maintain extra state>>. Here the priv is used to maintain
> extra state needed for backplane. This way the backplane specific data
> becomes available for all PHY callbacks (defined in struct phy_driver)
> that receive a pointer to phy_device structure. This initial version
> doesn't include accumulating statistics counters but we have in plan
> to add these in future versions. The counters will be kept in specific
> structures as members of the main backplane data mentioned above
> and entire support will be integrated with ethtool.

Hi Florinel

And what about hwmon, or anything else which a driver needs memory
for?

As far as i see it, we have two bodies of code here. There is a set of
helpers which implement most of the backplane functionality. And then
there is an example driver for your hardware. In the future we expect
other drivers to be added for other vendors hardware.

phydev->priv is for the driver. helpers should not assume they have
complete control over it.

Anyway, this may be a mute point. Lets first solve the problem of how
a PCS is represented.

  Andrew


  reply index

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-01 13:35 Florinel Iordache
2020-04-01 13:51 ` Andrew Lunn [this message]
2020-04-01 14:21   ` [EXT] " Florinel Iordache
  -- strict thread matches above, loose matches on Subject: below --
2020-04-24 12:14 Florinel Iordache
2020-04-01 14:01 Florinel Iordache
2020-04-01 14:11 ` Andrew Lunn
2020-04-01 14:14 ` Russell King - ARM Linux admin
2020-03-26 13:51 [PATCH net-next 0/9] net: ethernet backplane support Florinel Iordache
     [not found] ` <1585230682-24417-7-git-send-email-florinel.iordache@nxp.com>
2020-03-26 18:53   ` [PATCH net-next 6/9] net: phy: add backplane kr driver support David Miller
2020-03-26 18:55     ` Joe Perches
2020-03-26 19:07       ` David Miller
2020-03-26 19:42         ` Joe Perches
2020-03-27  1:07   ` Andrew Lunn
2020-03-27 17:43     ` Florian Fainelli
2020-03-27 14:22   ` Andrew Lunn
2020-03-27 18:25     ` Joe Perches
2020-03-27 14:28   ` Andrew Lunn
2020-03-27 14:33   ` Andrew Lunn
2020-03-27 14:38   ` Andrew Lunn

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=20200401135135.GA62290@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=florinel.iordache@nxp.com \
    --cc=hkallweit1@gmail.com \
    --cc=ioana.ciornei@nxp.com \
    --cc=kuba@kernel.org \
    --cc=leoyang.li@nxp.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=madalin.bucur@oss.nxp.com \
    --cc=mark.rutland@arm.com \
    --cc=netdev@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=shawnguo@kernel.org \
    /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

Linux-Doc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-doc/0 linux-doc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-doc linux-doc/ https://lore.kernel.org/linux-doc \
		linux-doc@vger.kernel.org
	public-inbox-index linux-doc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-doc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git