* [net-next PATCH v7 00/16] ACPI support for dpaa2 driver
@ 2021-03-11 6:19 Calvin Johnson
2021-03-11 6:20 ` [net-next PATCH v7 08/16] net: mdiobus: Introduce fwnode_mdiobus_register_phy() Calvin Johnson
0 siblings, 1 reply; 5+ messages in thread
From: Calvin Johnson @ 2021-03-11 6:19 UTC (permalink / raw)
To: Grant Likely, Rafael J . Wysocki, Jeremy Linton, Andrew Lunn,
Andy Shevchenko, Florian Fainelli, Russell King - ARM Linux admin,
Cristi Sovaiala, Florin Laurentiu Chiculita, Ioana Ciornei,
Madalin Bucur, Heikki Krogerus, Marcin Wojtas,
Pieter Jansen Van Vuuren, Jon, Saravana Kannan, Randy Dunlap
Cc: linux-arm-kernel, Diana Madalina Craciun, linux-acpi,
linux-kernel, linux.cj, netdev, Laurentiu Tudor, Calvin Johnson,
David S. Miller, Frank Rowand, Heiner Kallweit, Ioana Radulescu,
Jakub Kicinski, Jamie Iles, Len Brown, Rafael J. Wysocki,
Rob Herring, devicetree
This patch set provides ACPI support to DPAA2 network drivers.
It also introduces new fwnode based APIs to support phylink and phy
layers
Following functions are defined:
phylink_fwnode_phy_connect()
fwnode_mdiobus_register_phy()
fwnode_mdiobus_register()
fwnode_get_phy_id()
fwnode_phy_find_device()
device_phy_find_device()
fwnode_get_phy_node()
fwnode_mdio_find_device()
acpi_get_local_address()
First one helps in connecting phy to phylink instance.
Next three helps in getting phy_id and registering phy to mdiobus
Next two help in finding a phy on a mdiobus.
Next one helps in getting phy_node from a fwnode.
Last one is used to get local address from _ADR object.
Corresponding OF functions are refactored.
Tested-on: LX2160ARDB
Changes in v7:
- correct fwnode_mdio_find_device() description
- check NULL in unregister_mii_timestamper()
- Call unregister_mii_timestamper() without NULL check
- Create fwnode_mdio.c and move fwnode_mdiobus_register_phy()
- include fwnode_mdio.h
- Include headers directly used in acpi_mdio.c
- Move fwnode_mdiobus_register() to fwnode_mdio.c
- Include fwnode_mdio.h
- Alphabetically sort header inclusions
- remove unnecassary checks
Changes in v6:
- Minor cleanup
- fix warning for function parameter of fwnode_mdio_find_device()
- Initialize mii_ts to NULL
- use GENMASK() and ACPI_COMPANION_SET()
- some cleanup
- remove unwanted header inclusion
- remove OF check for fixed-link
- use dev_fwnode()
- remove useless else
- replace of_device_is_available() to fwnode_device_is_available()
Changes in v5:
- More cleanup
- Replace fwnode_get_id() with acpi_get_local_address()
- add missing MODULE_LICENSE()
- replace fwnode_get_id() with OF and ACPI function calls
- replace fwnode_get_id() with OF and ACPI function calls
Changes in v4:
- More cleanup
- Improve code structure to handle all cases
- Remove redundant else from fwnode_mdiobus_register()
- Cleanup xgmac_mdio_probe()
- call phy_device_free() before returning
Changes in v3:
- Add more info on legacy DT properties "phy" and "phy-device"
- Redefine fwnode_phy_find_device() to follow of_phy_find_device()
- Use traditional comparison pattern
- Use GENMASK
- Modified to retrieve reg property value for ACPI as well
- Resolved compilation issue with CONFIG_ACPI = n
- Added more info into documentation
- Use acpi_mdiobus_register()
- Avoid unnecessary line removal
- Remove unused inclusion of acpi.h
Changes in v2:
- Updated with more description in document
- use reverse christmas tree ordering for local variables
- Refactor OF functions to use fwnode functions
Calvin Johnson (16):
Documentation: ACPI: DSD: Document MDIO PHY
net: phy: Introduce fwnode_mdio_find_device()
net: phy: Introduce phy related fwnode functions
of: mdio: Refactor of_phy_find_device()
net: phy: Introduce fwnode_get_phy_id()
of: mdio: Refactor of_get_phy_id()
net: mii_timestamper: check NULL in unregister_mii_timestamper()
net: mdiobus: Introduce fwnode_mdiobus_register_phy()
of: mdio: Refactor of_mdiobus_register_phy()
ACPI: utils: Introduce acpi_get_local_address()
net: mdio: Add ACPI support code for mdio
net: mdiobus: Introduce fwnode_mdiobus_register()
net/fsl: Use fwnode_mdiobus_register()
net: phylink: introduce phylink_fwnode_phy_connect()
net: phylink: Refactor phylink_of_phy_connect()
net: dpaa2-mac: Add ACPI support for DPAA2 MAC driver
Documentation/firmware-guide/acpi/dsd/phy.rst | 133 ++++++++++++++++++
MAINTAINERS | 2 +
drivers/acpi/utils.c | 14 ++
.../net/ethernet/freescale/dpaa2/dpaa2-mac.c | 84 ++++++-----
drivers/net/ethernet/freescale/xgmac_mdio.c | 22 +--
drivers/net/mdio/Kconfig | 16 +++
drivers/net/mdio/Makefile | 4 +-
drivers/net/mdio/acpi_mdio.c | 56 ++++++++
drivers/net/mdio/fwnode_mdio.c | 98 +++++++++++++
drivers/net/mdio/of_mdio.c | 80 +----------
drivers/net/phy/mii_timestamper.c | 3 +
drivers/net/phy/phy_device.c | 109 +++++++++++++-
drivers/net/phy/phylink.c | 41 ++++--
include/linux/acpi.h | 7 +
include/linux/acpi_mdio.h | 25 ++++
include/linux/fwnode_mdio.h | 29 ++++
include/linux/of_mdio.h | 6 +-
include/linux/phy.h | 31 ++++
include/linux/phylink.h | 3 +
19 files changed, 631 insertions(+), 132 deletions(-)
create mode 100644 Documentation/firmware-guide/acpi/dsd/phy.rst
create mode 100644 drivers/net/mdio/acpi_mdio.c
create mode 100644 drivers/net/mdio/fwnode_mdio.c
create mode 100644 include/linux/acpi_mdio.h
create mode 100644 include/linux/fwnode_mdio.h
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [net-next PATCH v7 08/16] net: mdiobus: Introduce fwnode_mdiobus_register_phy()
2021-03-11 6:19 [net-next PATCH v7 00/16] ACPI support for dpaa2 driver Calvin Johnson
@ 2021-03-11 6:20 ` Calvin Johnson
2021-03-11 12:09 ` Andy Shevchenko
0 siblings, 1 reply; 5+ messages in thread
From: Calvin Johnson @ 2021-03-11 6:20 UTC (permalink / raw)
To: Grant Likely, Rafael J . Wysocki, Jeremy Linton, Andrew Lunn,
Andy Shevchenko, Florian Fainelli, Russell King - ARM Linux admin,
Cristi Sovaiala, Florin Laurentiu Chiculita, Ioana Ciornei,
Madalin Bucur, Heikki Krogerus, Marcin Wojtas,
Pieter Jansen Van Vuuren, Jon, Saravana Kannan, Randy Dunlap
Cc: linux-arm-kernel, Diana Madalina Craciun, linux-acpi,
linux-kernel, linux.cj, netdev, Laurentiu Tudor, Calvin Johnson,
David S. Miller, Frank Rowand, Heiner Kallweit, Jakub Kicinski,
Rob Herring, devicetree
Introduce fwnode_mdiobus_register_phy() to register PHYs on the
mdiobus. From the compatible string, identify whether the PHY is
c45 and based on this create a PHY device instance which is
registered on the mdiobus.
uninitialized symbol 'mii_ts'
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com>
---
Changes in v7:
- Call unregister_mii_timestamper() without NULL check
- Create fwnode_mdio.c and move fwnode_mdiobus_register_phy()
Changes in v6:
- Initialize mii_ts to NULL
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
MAINTAINERS | 1 +
drivers/net/mdio/Kconfig | 9 ++++
drivers/net/mdio/Makefile | 3 +-
drivers/net/mdio/fwnode_mdio.c | 77 ++++++++++++++++++++++++++++++++++
drivers/net/mdio/of_mdio.c | 3 +-
include/linux/fwnode_mdio.h | 24 +++++++++++
include/linux/of_mdio.h | 6 ++-
7 files changed, 120 insertions(+), 3 deletions(-)
create mode 100644 drivers/net/mdio/fwnode_mdio.c
create mode 100644 include/linux/fwnode_mdio.h
diff --git a/MAINTAINERS b/MAINTAINERS
index e1fa5ad9bb30..146de41d2656 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6680,6 +6680,7 @@ F: Documentation/devicetree/bindings/net/mdio*
F: Documentation/devicetree/bindings/net/qca,ar803x.yaml
F: Documentation/networking/phy.rst
F: drivers/net/mdio/
+F: drivers/net/mdio/fwnode_mdio.c
F: drivers/net/mdio/of_mdio.c
F: drivers/net/pcs/
F: drivers/net/phy/
diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index a10cc460d7cf..2d5bf5ccffb5 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -19,6 +19,15 @@ config MDIO_BUS
reflects whether the mdio_bus/mdio_device code is built as a
loadable module or built-in.
+config FWNODE_MDIO
+ def_tristate PHYLIB
+ depends on ACPI
+ depends on OF
+ depends on PHYLIB
+ select FIXED_PHY
+ help
+ FWNODE MDIO bus (Ethernet PHY) accessors
+
config OF_MDIO
def_tristate PHYLIB
depends on OF
diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
index 5c498dde463f..ea5390e2ef84 100644
--- a/drivers/net/mdio/Makefile
+++ b/drivers/net/mdio/Makefile
@@ -1,7 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for Linux MDIO bus drivers
-obj-$(CONFIG_OF_MDIO) += of_mdio.o
+obj-$(CONFIG_FWNODE_MDIO) += fwnode_mdio.o
+obj-$(CONFIG_OF_MDIO) += of_mdio.o
obj-$(CONFIG_MDIO_ASPEED) += mdio-aspeed.o
obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
new file mode 100644
index 000000000000..0982e816a6fb
--- /dev/null
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * fwnode helpers for the MDIO (Ethernet PHY) API
+ *
+ * This file provides helper functions for extracting PHY device information
+ * out of the fwnode and using it to populate an mii_bus.
+ */
+
+#include <linux/acpi.h>
+#include <linux/of.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+
+MODULE_AUTHOR("Calvin Johnson <calvin.johnson@oss.nxp.com>");
+MODULE_LICENSE("GPL");
+
+int fwnode_mdiobus_register_phy(struct mii_bus *bus,
+ struct fwnode_handle *child, u32 addr)
+{
+ struct mii_timestamper *mii_ts = NULL;
+ struct phy_device *phy;
+ bool is_c45 = false;
+ u32 phy_id;
+ int rc;
+
+ if (is_of_node(child)) {
+ mii_ts = of_find_mii_timestamper(to_of_node(child));
+ if (IS_ERR(mii_ts))
+ return PTR_ERR(mii_ts);
+ }
+
+ rc = fwnode_property_match_string(child, "compatible", "ethernet-phy-ieee802.3-c45");
+ if (rc >= 0)
+ is_c45 = true;
+
+ if (is_c45 || fwnode_get_phy_id(child, &phy_id))
+ phy = get_phy_device(bus, addr, is_c45);
+ else
+ phy = phy_device_create(bus, addr, phy_id, 0, NULL);
+ if (IS_ERR(phy)) {
+ unregister_mii_timestamper(mii_ts);
+ return PTR_ERR(phy);
+ }
+
+ if (is_acpi_node(child)) {
+ phy->irq = bus->irq[addr];
+
+ /* Associate the fwnode with the device structure so it
+ * can be looked up later.
+ */
+ phy->mdio.dev.fwnode = child;
+
+ /* All data is now stored in the phy struct, so register it */
+ rc = phy_device_register(phy);
+ if (rc) {
+ phy_device_free(phy);
+ fwnode_handle_put(phy->mdio.dev.fwnode);
+ return rc;
+ }
+ } else if (is_of_node(child)) {
+ rc = of_mdiobus_phy_device_register(bus, phy, to_of_node(child), addr);
+ if (rc) {
+ unregister_mii_timestamper(mii_ts);
+ phy_device_free(phy);
+ return rc;
+ }
+ }
+
+ /* phy->mii_ts may already be defined by the PHY driver. A
+ * mii_timestamper probed via the device tree will still have
+ * precedence.
+ */
+ if (mii_ts)
+ phy->mii_ts = mii_ts;
+ return 0;
+}
+EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index 48b6b8458c17..db293e0b8249 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -32,7 +32,7 @@ static int of_get_phy_id(struct device_node *device, u32 *phy_id)
return fwnode_get_phy_id(of_fwnode_handle(device), phy_id);
}
-static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
+struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
{
struct of_phandle_args arg;
int err;
@@ -49,6 +49,7 @@ static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
return register_mii_timestamper(arg.np, arg.args[0]);
}
+EXPORT_SYMBOL(of_find_mii_timestamper);
int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
struct device_node *child, u32 addr)
diff --git a/include/linux/fwnode_mdio.h b/include/linux/fwnode_mdio.h
new file mode 100644
index 000000000000..8c0392845916
--- /dev/null
+++ b/include/linux/fwnode_mdio.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * FWNODE helper for the MDIO (Ethernet PHY) API
+ */
+
+#ifndef __LINUX_FWNODE_MDIO_H
+#define __LINUX_FWNODE_MDIO_H
+
+#include <linux/phy.h>
+
+#if IS_ENABLED(CONFIG_FWNODE_MDIO)
+int fwnode_mdiobus_register_phy(struct mii_bus *bus,
+ struct fwnode_handle *child, u32 addr);
+
+#else /* CONFIG_FWNODE_MDIO */
+static inline int fwnode_mdiobus_register_phy(struct mii_bus *bus,
+ struct fwnode_handle *child,
+ u32 addr)
+{
+ return -EINVAL;
+}
+#endif
+
+#endif /* __LINUX_FWNODE_MDIO_H */
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index 2b05e7f7c238..e4ee6c4d9431 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -31,6 +31,7 @@ struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
int of_phy_register_fixed_link(struct device_node *np);
void of_phy_deregister_fixed_link(struct device_node *np);
bool of_phy_is_fixed_link(struct device_node *np);
+struct mii_timestamper *of_find_mii_timestamper(struct device_node *np);
int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
struct device_node *child, u32 addr);
@@ -118,7 +119,10 @@ static inline bool of_phy_is_fixed_link(struct device_node *np)
{
return false;
}
-
+static inline struct mii_timestamper *of_find_mii_timestamper(struct device_node *np)
+{
+ return NULL;
+}
static inline int of_mdiobus_phy_device_register(struct mii_bus *mdio,
struct phy_device *phy,
struct device_node *child, u32 addr)
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [net-next PATCH v7 08/16] net: mdiobus: Introduce fwnode_mdiobus_register_phy()
2021-03-11 6:20 ` [net-next PATCH v7 08/16] net: mdiobus: Introduce fwnode_mdiobus_register_phy() Calvin Johnson
@ 2021-03-11 12:09 ` Andy Shevchenko
2021-03-11 18:00 ` Calvin Johnson
0 siblings, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2021-03-11 12:09 UTC (permalink / raw)
To: Calvin Johnson
Cc: Grant Likely, Rafael J . Wysocki, Jeremy Linton, Andrew Lunn,
Florian Fainelli, Russell King - ARM Linux admin, Cristi Sovaiala,
Florin Laurentiu Chiculita, Ioana Ciornei, Madalin Bucur,
Heikki Krogerus, Marcin Wojtas, Pieter Jansen Van Vuuren, Jon,
Saravana Kannan, Randy Dunlap, linux-arm Mailing List,
Diana Madalina Craciun, ACPI Devel Maling List,
Linux Kernel Mailing List, linux.cj, netdev, Laurentiu Tudor,
David S. Miller, Frank Rowand, Heiner Kallweit, Jakub Kicinski,
Rob Herring, devicetree
On Thu, Mar 11, 2021 at 8:21 AM Calvin Johnson
<calvin.johnson@oss.nxp.com> wrote:
>
> Introduce fwnode_mdiobus_register_phy() to register PHYs on the
> mdiobus. From the compatible string, identify whether the PHY is
> c45 and based on this create a PHY device instance which is
> registered on the mdiobus.
> uninitialized symbol 'mii_ts'
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
I don't think it's important to have it in a history of Git. I would
move this after the cutter '---' line below.
> Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com>
> ---
>
> Changes in v7:
> - Call unregister_mii_timestamper() without NULL check
> - Create fwnode_mdio.c and move fwnode_mdiobus_register_phy()
>
> Changes in v6:
> - Initialize mii_ts to NULL
>
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
> MAINTAINERS | 1 +
> drivers/net/mdio/Kconfig | 9 ++++
> drivers/net/mdio/Makefile | 3 +-
> drivers/net/mdio/fwnode_mdio.c | 77 ++++++++++++++++++++++++++++++++++
> drivers/net/mdio/of_mdio.c | 3 +-
> include/linux/fwnode_mdio.h | 24 +++++++++++
> include/linux/of_mdio.h | 6 ++-
> 7 files changed, 120 insertions(+), 3 deletions(-)
> create mode 100644 drivers/net/mdio/fwnode_mdio.c
> create mode 100644 include/linux/fwnode_mdio.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e1fa5ad9bb30..146de41d2656 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6680,6 +6680,7 @@ F: Documentation/devicetree/bindings/net/mdio*
> F: Documentation/devicetree/bindings/net/qca,ar803x.yaml
> F: Documentation/networking/phy.rst
> F: drivers/net/mdio/
> +F: drivers/net/mdio/fwnode_mdio.c
> F: drivers/net/mdio/of_mdio.c
> F: drivers/net/pcs/
> F: drivers/net/phy/
> diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
> index a10cc460d7cf..2d5bf5ccffb5 100644
> --- a/drivers/net/mdio/Kconfig
> +++ b/drivers/net/mdio/Kconfig
> @@ -19,6 +19,15 @@ config MDIO_BUS
> reflects whether the mdio_bus/mdio_device code is built as a
> loadable module or built-in.
>
> +config FWNODE_MDIO
> + def_tristate PHYLIB
(Seems "selectable only" item)
> + depends on ACPI
> + depends on OF
Wouldn't be better to have
depends on (ACPI || OF) || COMPILE_TEST
And honestly I don't understand it in either (AND or OR) variant. Why
do you need a dependency like this for fwnode API?
Moreover dependencies don't work for "selectable only" items.
> + depends on PHYLIB
> + select FIXED_PHY
> + help
> + FWNODE MDIO bus (Ethernet PHY) accessors
> +
> config OF_MDIO
> def_tristate PHYLIB
> depends on OF
> diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
> index 5c498dde463f..ea5390e2ef84 100644
> --- a/drivers/net/mdio/Makefile
> +++ b/drivers/net/mdio/Makefile
> @@ -1,7 +1,8 @@
> # SPDX-License-Identifier: GPL-2.0
> # Makefile for Linux MDIO bus drivers
>
> -obj-$(CONFIG_OF_MDIO) += of_mdio.o
> +obj-$(CONFIG_FWNODE_MDIO) += fwnode_mdio.o
> +obj-$(CONFIG_OF_MDIO) += of_mdio.o
>
> obj-$(CONFIG_MDIO_ASPEED) += mdio-aspeed.o
> obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
> diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
> new file mode 100644
> index 000000000000..0982e816a6fb
> --- /dev/null
> +++ b/drivers/net/mdio/fwnode_mdio.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * fwnode helpers for the MDIO (Ethernet PHY) API
> + *
> + * This file provides helper functions for extracting PHY device information
> + * out of the fwnode and using it to populate an mii_bus.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/of.h>
> +#include <linux/of_mdio.h>
> +#include <linux/phy.h>
> +
> +MODULE_AUTHOR("Calvin Johnson <calvin.johnson@oss.nxp.com>");
> +MODULE_LICENSE("GPL");
> +
> +int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> + struct fwnode_handle *child, u32 addr)
> +{
> + struct mii_timestamper *mii_ts = NULL;
> + struct phy_device *phy;
> + bool is_c45 = false;
> + u32 phy_id;
> + int rc;
> +
> + if (is_of_node(child)) {
> + mii_ts = of_find_mii_timestamper(to_of_node(child));
> + if (IS_ERR(mii_ts))
> + return PTR_ERR(mii_ts);
> + }
> +
> + rc = fwnode_property_match_string(child, "compatible", "ethernet-phy-ieee802.3-c45");
> + if (rc >= 0)
> + is_c45 = true;
> +
> + if (is_c45 || fwnode_get_phy_id(child, &phy_id))
> + phy = get_phy_device(bus, addr, is_c45);
> + else
> + phy = phy_device_create(bus, addr, phy_id, 0, NULL);
> + if (IS_ERR(phy)) {
> + unregister_mii_timestamper(mii_ts);
> + return PTR_ERR(phy);
> + }
> +
> + if (is_acpi_node(child)) {
> + phy->irq = bus->irq[addr];
> +
> + /* Associate the fwnode with the device structure so it
> + * can be looked up later.
> + */
> + phy->mdio.dev.fwnode = child;
> +
> + /* All data is now stored in the phy struct, so register it */
> + rc = phy_device_register(phy);
> + if (rc) {
> + phy_device_free(phy);
> + fwnode_handle_put(phy->mdio.dev.fwnode);
> + return rc;
> + }
> + } else if (is_of_node(child)) {
> + rc = of_mdiobus_phy_device_register(bus, phy, to_of_node(child), addr);
> + if (rc) {
> + unregister_mii_timestamper(mii_ts);
> + phy_device_free(phy);
> + return rc;
> + }
> + }
> +
> + /* phy->mii_ts may already be defined by the PHY driver. A
> + * mii_timestamper probed via the device tree will still have
> + * precedence.
> + */
> + if (mii_ts)
> + phy->mii_ts = mii_ts;
> + return 0;
> +}
> +EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
> diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
> index 48b6b8458c17..db293e0b8249 100644
> --- a/drivers/net/mdio/of_mdio.c
> +++ b/drivers/net/mdio/of_mdio.c
> @@ -32,7 +32,7 @@ static int of_get_phy_id(struct device_node *device, u32 *phy_id)
> return fwnode_get_phy_id(of_fwnode_handle(device), phy_id);
> }
>
> -static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
> +struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
> {
> struct of_phandle_args arg;
> int err;
> @@ -49,6 +49,7 @@ static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
>
> return register_mii_timestamper(arg.np, arg.args[0]);
> }
> +EXPORT_SYMBOL(of_find_mii_timestamper);
>
> int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
> struct device_node *child, u32 addr)
> diff --git a/include/linux/fwnode_mdio.h b/include/linux/fwnode_mdio.h
> new file mode 100644
> index 000000000000..8c0392845916
> --- /dev/null
> +++ b/include/linux/fwnode_mdio.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * FWNODE helper for the MDIO (Ethernet PHY) API
> + */
> +
> +#ifndef __LINUX_FWNODE_MDIO_H
> +#define __LINUX_FWNODE_MDIO_H
> +
> +#include <linux/phy.h>
> +
> +#if IS_ENABLED(CONFIG_FWNODE_MDIO)
> +int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> + struct fwnode_handle *child, u32 addr);
> +
> +#else /* CONFIG_FWNODE_MDIO */
> +static inline int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> + struct fwnode_handle *child,
> + u32 addr)
> +{
> + return -EINVAL;
> +}
> +#endif
> +
> +#endif /* __LINUX_FWNODE_MDIO_H */
> diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
> index 2b05e7f7c238..e4ee6c4d9431 100644
> --- a/include/linux/of_mdio.h
> +++ b/include/linux/of_mdio.h
> @@ -31,6 +31,7 @@ struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
> int of_phy_register_fixed_link(struct device_node *np);
> void of_phy_deregister_fixed_link(struct device_node *np);
> bool of_phy_is_fixed_link(struct device_node *np);
> +struct mii_timestamper *of_find_mii_timestamper(struct device_node *np);
> int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
> struct device_node *child, u32 addr);
>
> @@ -118,7 +119,10 @@ static inline bool of_phy_is_fixed_link(struct device_node *np)
> {
> return false;
> }
> -
> +static inline struct mii_timestamper *of_find_mii_timestamper(struct device_node *np)
> +{
> + return NULL;
> +}
> static inline int of_mdiobus_phy_device_register(struct mii_bus *mdio,
> struct phy_device *phy,
> struct device_node *child, u32 addr)
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net-next PATCH v7 08/16] net: mdiobus: Introduce fwnode_mdiobus_register_phy()
2021-03-11 12:09 ` Andy Shevchenko
@ 2021-03-11 18:00 ` Calvin Johnson
2021-03-11 18:14 ` Andy Shevchenko
0 siblings, 1 reply; 5+ messages in thread
From: Calvin Johnson @ 2021-03-11 18:00 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Grant Likely, Rafael J . Wysocki, Jeremy Linton, Andrew Lunn,
Florian Fainelli, Russell King - ARM Linux admin, Cristi Sovaiala,
Florin Laurentiu Chiculita, Ioana Ciornei, Madalin Bucur,
Heikki Krogerus, Marcin Wojtas, Pieter Jansen Van Vuuren, Jon,
Saravana Kannan, Randy Dunlap, linux-arm Mailing List,
Diana Madalina Craciun, ACPI Devel Maling List,
Linux Kernel Mailing List, linux.cj, netdev, Laurentiu Tudor,
David S. Miller, Frank Rowand, Heiner Kallweit, Jakub Kicinski,
Rob Herring, devicetree
On Thu, Mar 11, 2021 at 02:09:37PM +0200, Andy Shevchenko wrote:
> On Thu, Mar 11, 2021 at 8:21 AM Calvin Johnson
> <calvin.johnson@oss.nxp.com> wrote:
> >
> > Introduce fwnode_mdiobus_register_phy() to register PHYs on the
> > mdiobus. From the compatible string, identify whether the PHY is
> > c45 and based on this create a PHY device instance which is
> > registered on the mdiobus.
>
> > uninitialized symbol 'mii_ts'
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> I don't think it's important to have it in a history of Git. I would
> move this after the cutter '---' line below.
Sorry. I thought I had removed it. Will definitely take care next time.
>
> > Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com>
> > ---
> >
> > Changes in v7:
> > - Call unregister_mii_timestamper() without NULL check
> > - Create fwnode_mdio.c and move fwnode_mdiobus_register_phy()
> >
> > Changes in v6:
> > - Initialize mii_ts to NULL
> >
> > Changes in v5: None
> > Changes in v4: None
> > Changes in v3: None
> > Changes in v2: None
> >
> > MAINTAINERS | 1 +
> > drivers/net/mdio/Kconfig | 9 ++++
> > drivers/net/mdio/Makefile | 3 +-
> > drivers/net/mdio/fwnode_mdio.c | 77 ++++++++++++++++++++++++++++++++++
> > drivers/net/mdio/of_mdio.c | 3 +-
> > include/linux/fwnode_mdio.h | 24 +++++++++++
> > include/linux/of_mdio.h | 6 ++-
> > 7 files changed, 120 insertions(+), 3 deletions(-)
> > create mode 100644 drivers/net/mdio/fwnode_mdio.c
> > create mode 100644 include/linux/fwnode_mdio.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index e1fa5ad9bb30..146de41d2656 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6680,6 +6680,7 @@ F: Documentation/devicetree/bindings/net/mdio*
> > F: Documentation/devicetree/bindings/net/qca,ar803x.yaml
> > F: Documentation/networking/phy.rst
> > F: drivers/net/mdio/
> > +F: drivers/net/mdio/fwnode_mdio.c
> > F: drivers/net/mdio/of_mdio.c
> > F: drivers/net/pcs/
> > F: drivers/net/phy/
> > diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
> > index a10cc460d7cf..2d5bf5ccffb5 100644
> > --- a/drivers/net/mdio/Kconfig
> > +++ b/drivers/net/mdio/Kconfig
> > @@ -19,6 +19,15 @@ config MDIO_BUS
> > reflects whether the mdio_bus/mdio_device code is built as a
> > loadable module or built-in.
> >
> > +config FWNODE_MDIO
> > + def_tristate PHYLIB
>
> (Seems "selectable only" item)
What do you mean by "selectable only" item here? Can you please point to some
other example?
>
> > + depends on ACPI
> > + depends on OF
>
> Wouldn't be better to have
> depends on (ACPI || OF) || COMPILE_TEST
>
> And honestly I don't understand it in either (AND or OR) variant. Why
> do you need a dependency like this for fwnode API?
Here, fwnode_mdiobus_register_phy() uses objects from both ACPI and OF.
>
> Moreover dependencies don't work for "selectable only" items.
>
> > + depends on PHYLIB
> > + select FIXED_PHY
> > + help
> > + FWNODE MDIO bus (Ethernet PHY) accessors
> > +
> > config OF_MDIO
> > def_tristate PHYLIB
> > depends on OF
> > diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
> > index 5c498dde463f..ea5390e2ef84 100644
> > --- a/drivers/net/mdio/Makefile
> > +++ b/drivers/net/mdio/Makefile
> > @@ -1,7 +1,8 @@
> > # SPDX-License-Identifier: GPL-2.0
> > # Makefile for Linux MDIO bus drivers
> >
> > -obj-$(CONFIG_OF_MDIO) += of_mdio.o
> > +obj-$(CONFIG_FWNODE_MDIO) += fwnode_mdio.o
> > +obj-$(CONFIG_OF_MDIO) += of_mdio.o
> >
> > obj-$(CONFIG_MDIO_ASPEED) += mdio-aspeed.o
> > obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
> > diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
> > new file mode 100644
> > index 000000000000..0982e816a6fb
> > --- /dev/null
> > +++ b/drivers/net/mdio/fwnode_mdio.c
> > @@ -0,0 +1,77 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * fwnode helpers for the MDIO (Ethernet PHY) API
> > + *
> > + * This file provides helper functions for extracting PHY device information
> > + * out of the fwnode and using it to populate an mii_bus.
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/of.h>
> > +#include <linux/of_mdio.h>
> > +#include <linux/phy.h>
> > +
> > +MODULE_AUTHOR("Calvin Johnson <calvin.johnson@oss.nxp.com>");
> > +MODULE_LICENSE("GPL");
> > +
> > +int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> > + struct fwnode_handle *child, u32 addr)
> > +{
> > + struct mii_timestamper *mii_ts = NULL;
> > + struct phy_device *phy;
> > + bool is_c45 = false;
> > + u32 phy_id;
> > + int rc;
> > +
> > + if (is_of_node(child)) {
> > + mii_ts = of_find_mii_timestamper(to_of_node(child));
> > + if (IS_ERR(mii_ts))
> > + return PTR_ERR(mii_ts);
> > + }
> > +
> > + rc = fwnode_property_match_string(child, "compatible", "ethernet-phy-ieee802.3-c45");
> > + if (rc >= 0)
> > + is_c45 = true;
> > +
> > + if (is_c45 || fwnode_get_phy_id(child, &phy_id))
> > + phy = get_phy_device(bus, addr, is_c45);
> > + else
> > + phy = phy_device_create(bus, addr, phy_id, 0, NULL);
> > + if (IS_ERR(phy)) {
> > + unregister_mii_timestamper(mii_ts);
> > + return PTR_ERR(phy);
> > + }
> > +
> > + if (is_acpi_node(child)) {
> > + phy->irq = bus->irq[addr];
> > +
> > + /* Associate the fwnode with the device structure so it
> > + * can be looked up later.
> > + */
> > + phy->mdio.dev.fwnode = child;
> > +
> > + /* All data is now stored in the phy struct, so register it */
> > + rc = phy_device_register(phy);
> > + if (rc) {
> > + phy_device_free(phy);
> > + fwnode_handle_put(phy->mdio.dev.fwnode);
> > + return rc;
> > + }
> > + } else if (is_of_node(child)) {
> > + rc = of_mdiobus_phy_device_register(bus, phy, to_of_node(child), addr);
> > + if (rc) {
> > + unregister_mii_timestamper(mii_ts);
> > + phy_device_free(phy);
> > + return rc;
> > + }
> > + }
> > +
> > + /* phy->mii_ts may already be defined by the PHY driver. A
> > + * mii_timestamper probed via the device tree will still have
> > + * precedence.
> > + */
> > + if (mii_ts)
> > + phy->mii_ts = mii_ts;
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
> > diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
> > index 48b6b8458c17..db293e0b8249 100644
> > --- a/drivers/net/mdio/of_mdio.c
> > +++ b/drivers/net/mdio/of_mdio.c
> > @@ -32,7 +32,7 @@ static int of_get_phy_id(struct device_node *device, u32 *phy_id)
> > return fwnode_get_phy_id(of_fwnode_handle(device), phy_id);
> > }
> >
> > -static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
> > +struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
> > {
> > struct of_phandle_args arg;
> > int err;
> > @@ -49,6 +49,7 @@ static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
> >
> > return register_mii_timestamper(arg.np, arg.args[0]);
> > }
> > +EXPORT_SYMBOL(of_find_mii_timestamper);
> >
> > int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
> > struct device_node *child, u32 addr)
> > diff --git a/include/linux/fwnode_mdio.h b/include/linux/fwnode_mdio.h
> > new file mode 100644
> > index 000000000000..8c0392845916
> > --- /dev/null
> > +++ b/include/linux/fwnode_mdio.h
> > @@ -0,0 +1,24 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * FWNODE helper for the MDIO (Ethernet PHY) API
> > + */
> > +
> > +#ifndef __LINUX_FWNODE_MDIO_H
> > +#define __LINUX_FWNODE_MDIO_H
> > +
> > +#include <linux/phy.h>
> > +
> > +#if IS_ENABLED(CONFIG_FWNODE_MDIO)
> > +int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> > + struct fwnode_handle *child, u32 addr);
> > +
> > +#else /* CONFIG_FWNODE_MDIO */
> > +static inline int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> > + struct fwnode_handle *child,
> > + u32 addr)
> > +{
> > + return -EINVAL;
> > +}
> > +#endif
> > +
> > +#endif /* __LINUX_FWNODE_MDIO_H */
> > diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
> > index 2b05e7f7c238..e4ee6c4d9431 100644
> > --- a/include/linux/of_mdio.h
> > +++ b/include/linux/of_mdio.h
> > @@ -31,6 +31,7 @@ struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
> > int of_phy_register_fixed_link(struct device_node *np);
> > void of_phy_deregister_fixed_link(struct device_node *np);
> > bool of_phy_is_fixed_link(struct device_node *np);
> > +struct mii_timestamper *of_find_mii_timestamper(struct device_node *np);
> > int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
> > struct device_node *child, u32 addr);
> >
> > @@ -118,7 +119,10 @@ static inline bool of_phy_is_fixed_link(struct device_node *np)
> > {
> > return false;
> > }
> > -
> > +static inline struct mii_timestamper *of_find_mii_timestamper(struct device_node *np)
> > +{
> > + return NULL;
> > +}
> > static inline int of_mdiobus_phy_device_register(struct mii_bus *mdio,
> > struct phy_device *phy,
> > struct device_node *child, u32 addr)
> > --
> > 2.17.1
> >
>
>
Regards
Calvin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net-next PATCH v7 08/16] net: mdiobus: Introduce fwnode_mdiobus_register_phy()
2021-03-11 18:00 ` Calvin Johnson
@ 2021-03-11 18:14 ` Andy Shevchenko
0 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2021-03-11 18:14 UTC (permalink / raw)
To: Calvin Johnson
Cc: Grant Likely, Rafael J . Wysocki, Jeremy Linton, Andrew Lunn,
Florian Fainelli, Russell King - ARM Linux admin, Cristi Sovaiala,
Florin Laurentiu Chiculita, Ioana Ciornei, Madalin Bucur,
Heikki Krogerus, Marcin Wojtas, Pieter Jansen Van Vuuren, Jon,
Saravana Kannan, Randy Dunlap, linux-arm Mailing List,
Diana Madalina Craciun, ACPI Devel Maling List,
Linux Kernel Mailing List, linux.cj, netdev, Laurentiu Tudor,
David S. Miller, Frank Rowand, Heiner Kallweit, Jakub Kicinski,
Rob Herring, devicetree
On Thu, Mar 11, 2021 at 8:00 PM Calvin Johnson
<calvin.johnson@oss.nxp.com> wrote:
> On Thu, Mar 11, 2021 at 02:09:37PM +0200, Andy Shevchenko wrote:
> > On Thu, Mar 11, 2021 at 8:21 AM Calvin Johnson
> > <calvin.johnson@oss.nxp.com> wrote:
...
> > > +config FWNODE_MDIO
> > > + def_tristate PHYLIB
> >
> > (Seems "selectable only" item)
>
> What do you mean by "selectable only" item here? Can you please point to some
> other example?
The Kconfig sections without descriptions are not user-visible.
No user can run menuconfig and check a box with "I want this to be compiled".
tristate // selectable-only
tristate "bla bla bla" // user visible and selectable
> > > + depends on ACPI
> > > + depends on OF
> >
> > Wouldn't be better to have
> > depends on (ACPI || OF) || COMPILE_TEST
> >
> > And honestly I don't understand it in either (AND or OR) variant. Why
> > do you need a dependency like this for fwnode API?
>
> Here, fwnode_mdiobus_register_phy() uses objects from both ACPI and OF.
APIs? Calls? What really fails if we have !ACPI and / or !OF?
> > Moreover dependencies don't work for "selectable only" items.
> >
> > > + depends on PHYLIB
> > > + select FIXED_PHY
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-03-11 18:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-11 6:19 [net-next PATCH v7 00/16] ACPI support for dpaa2 driver Calvin Johnson
2021-03-11 6:20 ` [net-next PATCH v7 08/16] net: mdiobus: Introduce fwnode_mdiobus_register_phy() Calvin Johnson
2021-03-11 12:09 ` Andy Shevchenko
2021-03-11 18:00 ` Calvin Johnson
2021-03-11 18:14 ` Andy Shevchenko
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).