netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Buday Csaba <buday.csaba@prolan.hu>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next 1/1] net: mdio: reset PHY before attempting to access ID register
Date: Thu, 27 Nov 2025 12:17:24 +0100	[thread overview]
Message-ID: <aSgzRMf1LZycQoni@debianbuilder> (raw)
In-Reply-To: <20251125184335.040f015e@kernel.org>

On Tue, Nov 25, 2025 at 06:43:35PM -0800, Jakub Kicinski wrote:
> On Tue, 25 Nov 2025 12:15:51 +0100 Buday Csaba wrote:
> > When the ID of an Ethernet PHY is not provided by the 'compatible'
> > string in the device tree, its actual ID is read via the MDIO bus.
> > For some PHYs this could be unsafe, since a hard reset may be
> > necessary to safely access the MDIO registers.
> 
> You may be missing exports because it doesn't build with allmodconfig:
> 
> ERROR: modpost: "mdio_device_register_reset" [drivers/net/mdio/fwnode_mdio.ko] undefined!
> ERROR: modpost: "mdio_device_unregister_reset" [drivers/net/mdio/fwnode_mdio.ko] undefined!
> -- 
> pw-bot: cr
> 

I require some advice on how to do it properly.
The high level functionality belongs to either fwnode_mdio.c or maybe
phy_device.c

The latter may be better, since get_phy_device() already performs some
kind of recovery for PHYs with a certain unexpected behaviour.

But the low level functionality: registering the reset properties are
now in their proper place in mdio_device.c

These three source files build into three different modules, so I only
see the following options:

a) make mdio_device_register_reset() and its counterpart public
   But we have already agreed that they should not be, and keep them
   internal

b) create a new helper function in mdio_device.c, and make that public
   This could work, but then what is the point of hiding
   mdio_device_register_reset()? My idea was something like
   mdio_device_reset_strobe(), which calls register/unregister reset
   while also performing the assertion/deassertion of the reset.
   But such a function is unsafe on an already established mdio_device,
   so making that exported may be questionable as well.
   It is possible to work around that, but then it is getting out of hand
   fast, and does not follow the keep it simple and stupid principle.

c) what about using EXPORT_SYMBOL_FOR_MODULES() on the problematic
   functions? Are there any objections against it?
   This type of export is rarely used in the kernel, so I am uncertain
   about that. Is using it on functions declared in private headers
   also discouraged?

Thank you in advance,
Csaba


  parent reply	other threads:[~2025-11-27 11:17 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-25 11:15 [PATCH net-next 1/1] net: mdio: reset PHY before attempting to access ID register Buday Csaba
2025-11-26  2:43 ` Jakub Kicinski
2025-11-26  7:03   ` Buday Csaba
2025-11-27 11:17   ` Buday Csaba [this message]
2025-11-27 15:56     ` Andrew Lunn
2025-11-26 15:17 ` kernel test robot

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=aSgzRMf1LZycQoni@debianbuilder \
    --to=buday.csaba@prolan.hu \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@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 \
    /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).