All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Horatiu Vultur <horatiu.vultur@microchip.com>,
	andrew@lunn.ch, davem@davemloft.net, kuba@kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next] net: phy: micrel: Add config_init for LAN8814
Date: Fri, 26 Nov 2021 12:09:05 +0000	[thread overview]
Message-ID: <YaDOYc+RWZ35lKjB@shell.armlinux.org.uk> (raw)
In-Reply-To: <402780af-9d12-45dd-e435-e7279f1b9263@gmail.com>

On Fri, Nov 26, 2021 at 12:57:33PM +0100, Heiner Kallweit wrote:
> Not directly related to just this patch:
> Did you consider implementing the read_page and write_page PHY driver
> callbacks? Then you could use phylib functions like phy_modify_paged et al
> and you wouldn't have to open-code the paged register operations.
> 
> I think write_page would just be
> phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, page);
> phy_write(phydev, LAN_EXT_PAGE_ACCESS_ADDRESS_DATA, addr);
> phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, (page | LAN_EXT_PAGE_ACCESS_CTRL_EP_FUNC));
> 
> and read_page
> phy_read(phydev, LAN_EXT_PAGE_ACCESS_CONTROL);

Remember that read_page() and write_page() must be implemented using
the unlocked accessors since the MDIO bus lock is held prior to calling
them. So these should be __phy_write() and __phy_read().

The use of the helpers you mention above also bring greater safety to
the read-modify-write accesses, since with these accessors, the whole
set of accesses are done while holding the bus lock.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

  reply	other threads:[~2021-11-26 12:52 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-26 10:38 [PATCH net-next] net: phy: micrel: Add config_init for LAN8814 Horatiu Vultur
2021-11-26 11:57 ` Heiner Kallweit
2021-11-26 12:09   ` Russell King (Oracle) [this message]
2021-11-29  8:29   ` Horatiu Vultur
2021-11-29 17:07     ` Heiner Kallweit
2021-11-26 15:15 ` Andrew Lunn
2021-11-29  9:01   ` Horatiu Vultur
2021-12-22 11:48 ` Horatiu Vultur
2021-12-22 15:23   ` Jakub Kicinski

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=YaDOYc+RWZ35lKjB@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=hkallweit1@gmail.com \
    --cc=horatiu.vultur@microchip.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.