public inbox for linux-phy@lists.infradead.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Vladimir Oltean <olteanv@gmail.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: Sat, 28 Feb 2026 13:09:57 +0200	[thread overview]
Message-ID: <aaLNBTZsUQ2vMfOW@ashevche-desk.local> (raw)
In-Reply-To: <20260224183000.txlazzyw7z34nhsj@skbuf>

On Tue, Feb 24, 2026 at 08:30:00PM +0200, Vladimir Oltean wrote:
> 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.

Thanks for explanation. I prefer that Ulf's staff settles down first as it seems
more important.

> 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.

Sure.

> > 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?)

Because of inconsistency. This makes my mind blown and the code is not good
for others to read and understand when it's inconsistent like this. That's it.

-- 
With Best Regards,
Andy Shevchenko



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

  reply	other threads:[~2026-02-28 11:10 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
2026-02-28 11:09         ` Andy Shevchenko [this message]
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=aaLNBTZsUQ2vMfOW@ashevche-desk.local \
    --to=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=olteanv@gmail.com \
    --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