Linux CAN drivers development
 help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: linux-can@vger.kernel.org, linux-phy@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Marc Kleine-Budde <mkl@pengutronix.de>,
	Vincent Mailhol <mailhol@kernel.org>,
	Vinod Koul <vkoul@kernel.org>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Josua Mayer <josua@solid-run.com>
Subject: Re: [PATCH v1 1/4] phy: phy-can-transceiver: Convert to use device property API
Date: Tue, 24 Feb 2026 20:30:00 +0200	[thread overview]
Message-ID: <20260224183000.txlazzyw7z34nhsj@skbuf> (raw)
In-Reply-To: <aZ3X2J1rBq1pMkae@smile.fi.intel.com>

On Tue, Feb 24, 2026 at 06:54:48PM +0200, Andy Shevchenko wrote:
> On Tue, Feb 24, 2026 at 06:26:06PM +0200, Vladimir Oltean wrote:
> > On Thu, Feb 19, 2026 at 09:26:19PM +0100, Andy Shevchenko wrote:
> 
> ...
> 
> > > -	if (!of_property_present(dev->of_node, "mux-states"))
> > > +	if (!device_property_present(dev, "mux-states"))
> > 
> > There's an entire saga with this function - devm_mux_state_get_optional().
> > Josua Mayer is preparing to move it to the MUX core, which will be a cross-tree series.
> > Would you mind not touching this, to avoid complicating what is already
> > a complicated operation? It is going away anyway, and from what I can
> > see in Josua's last series, its implementation from drivers/mux/core.c
> > is already using device property APIs:
> > https://lore.kernel.org/linux-phy/20260208-rz-sdio-mux-v9-2-9a3be13c1280@solid-run.com/
> 
> Basically you ask me to postpone the series until that will be in. Since this
> file is a mess in terms of OF/fwnode API use in exchange I would like whoever
> is doing the other part to speed up a bit if possible.
> 
> I prefer to see cleaner solution to be applied sooner and last in a long distance,
> that's why I see either mine first but soon, or that first but also soon should
> be in. Can we try to achieve that?

The idea is that Ulf already expressed the availability to take the phy-can-transceiver
patch through the mmc tree and provide back a tag to be pulled into linux-phy:
https://lore.kernel.org/linux-phy/CAPDyKFrtTaJ5fqqbGrE_K6SAdTZYUfp-BycGjtWs4SabwBysKA@mail.gmail.com/

If linux-phy takes your patch first, there will be a conflict when pulling the
stable branch, and it won't be so fun, plus we can't even build-test Josua's
submission on linux-phy, so that's obviously not great.

So yeah, I'm not requesting you to postpone the entire series, just not
touch devm_mux_state_get_optional() and don't let it appear in your
patch context.

Somebody will have to remove "#include <linux/of.h>" at the end of the
whole process, but that's minor.

> ...
> 
> > > -		phy = devm_phy_create(dev, dev->of_node, &can_transceiver_phy_ops);
> > > +		phy = devm_phy_create(dev, NULL, &can_transceiver_phy_ops);
> > 
> > It is not obvious why you replaced dev->of_node with NULL here.
> > It doesn't appear correct. You seem to be breaking OF-based PHY lookups.
> 
> It's the default. Yeah, I probably have to explain this in the commit message.

Ah, ok. Found the "phy->dev.of_node = node ?: dev->of_node;" assignment.
Sorry and noted, but please add it to the commit message too.

> Basically all devm_phy_create(dev, dev->of_node, ...) for clarity should be
> converted to that approach. Or even better, a new (agnostic) API should take
> default fwnode from the same device.
> 
> 		phy = devm_phy_create_simple(dev, &..._phy_ops);
> 
> // name was quickly chosen and may be not the best we can come up with

I agree in principle. PHY drivers shouldn't be given a function where
they routinely have to set one of the arguments to NULL, but a simpler
function without that argument.

But the phy-core.c doesn't support fwnode at all yet, it uses OF
throughout. I think it would be preferable to leave this change to
somebody who has business in that area.

(are you interested in PHYs with a fwnode for any particular reason, or
just because the API is more "generic" just in case?)

  reply	other threads:[~2026-02-24 18:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-19 20:26 [PATCH v1 0/4] phy: phy-can-transceiver: Ad-hoc cleanups and refactoring Andy Shevchenko
2026-02-19 20:26 ` [PATCH v1 1/4] phy: phy-can-transceiver: Convert to use device property API Andy Shevchenko
2026-02-24 16:26   ` Vladimir Oltean
2026-02-24 16:54     ` Andy Shevchenko
2026-02-24 18:30       ` Vladimir Oltean [this message]
2026-02-28 11:09         ` Andy Shevchenko
2026-03-17 10:41           ` Andy Shevchenko
2026-03-17 20:13             ` Andy Shevchenko
2026-02-19 20:26 ` [PATCH v1 2/4] phy: phy-can-transceiver: Move OF ID table closer to their user Andy Shevchenko
2026-02-19 20:26 ` [PATCH v1 3/4] phy: phy-can-transceiver: Don't check for specific errors when parsing properties Andy Shevchenko
2026-02-19 20:26 ` [PATCH v1 4/4] phy: phy-can-transceiver: Drop unused include Andy Shevchenko

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=20260224183000.txlazzyw7z34nhsj@skbuf \
    --to=olteanv@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=josua@solid-run.com \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=mailhol@kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=neil.armstrong@linaro.org \
    --cc=vkoul@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox