All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>
Subject: Re: [PATCH net-next v1 0/5] net: mdiobus: HIde ACPI implementation
Date: Tue, 5 May 2026 07:54:27 +0300	[thread overview]
Message-ID: <afl4A48uIZPJmMeG@ashevche-desk.local> (raw)
In-Reply-To: <28a4ebd7-2670-439d-836b-14559a916ed0@lunn.ch>

On Mon, May 04, 2026 at 05:16:57PM +0200, Andrew Lunn wrote:
> On Mon, May 04, 2026 at 04:49:35PM +0300, Andy Shevchenko wrote:
> > On Mon, May 04, 2026 at 03:02:54PM +0200, Andrew Lunn wrote:
> > > On Mon, May 04, 2026 at 09:29:51AM +0200, Andy Shevchenko wrote:
> > > > This mini-series is dedicated to hiding ACPI implementation details
> > > > from the wider users as they (as of today) do not need to know that.
> > > 
> > > Please could you expand on that. ACPI != OF. They have different
> > > bindings, so you need different implementations.
> > 
> > As of today the users that want ACPI also have the OF support. Even without
> > that if the device is pure ACPI supported one (and somehow never going to DT)
> > the proposed API (see the first patch) will be no-op in case when CONFIG_ACPI=n
> > or when it's a non-ACPI platform with no support of the device. Hence, the
> > pure ACPI (and actually OF as well) do not need to be exposed. The decision is
> > made based on the type of firmware node.
> 
> I see two different things here:
> 
> You are refactoring code into a helper, so reducing the amount of
> duplicated code. That in itself is good.
> 
> However, we have the problem in general that developers think that OF
> and ACPI do the same thing. And that is not true. OF MDIO busses have
> support for a GPIO used as a reset. In retrospect, that was a bad
> idea. The documented ACPI binding does not have this, and if anybody
> was to suggest adding it, i want to NACK it.
> 
> Having OF code and ACPI code to instantiate the MDIO bus makes it
> clearer they are different things. fwnode gives the impression they
> are the same, which is not true. Hence i don't like fwnode at this
> level.
> 
> Where fwnode makes sense is deeper down in the implementation of the
> bindings, for properties which are the same in both bindings.  Then
> you can use fwnode_ for the shared properties, of_ for the OF only
> properties, and acpi_ for the ACPI only properties.
> 
> So although i like the reduction in duplicated code, i think overall
> it makes the code worse because developers are more likely to wrongly
> understand it.

OK.
Thanks for review.

-- 
With Best Regards,
Andy Shevchenko



      reply	other threads:[~2026-05-05  4:54 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-04  7:29 [PATCH net-next v1 0/5] net: mdiobus: HIde ACPI implementation Andy Shevchenko
2026-05-04  7:29 ` [PATCH net-next v1 1/5] net: mdiobus: Provide fwnode_mdiobus_register() Andy Shevchenko
2026-05-04  7:29 ` [PATCH net-next v1 2/5] net: mvmdio: Switch to using fwnode_mdiobus_register() Andy Shevchenko
2026-05-04  7:29 ` [PATCH net-next v1 3/5] net/fsl: xgmac_mdio: " Andy Shevchenko
2026-05-04  7:29 ` [PATCH net-next v1 4/5] net/fsl: xgmac_mdio: Reuse existing pointer to fwnode Andy Shevchenko
2026-05-04  7:29 ` [PATCH net-next v1 5/5] net: mdiobus: Hide acpi_mdio.h Andy Shevchenko
2026-05-10 10:08   ` kernel test robot
2026-05-10 12:41   ` kernel test robot
2026-05-04 13:02 ` [PATCH net-next v1 0/5] net: mdiobus: HIde ACPI implementation Andrew Lunn
2026-05-04 13:49   ` Andy Shevchenko
2026-05-04 15:16     ` Andrew Lunn
2026-05-05  4:54       ` Andy Shevchenko [this message]

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=afl4A48uIZPJmMeG@ashevche-desk.local \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=andrew+netdev@lunn.ch \
    --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 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.