The main goal of this series is to extend the DW XPCS device support in
the kernel. Particularly the patchset adds a support of the DW XPCS
device with the MCI/APB3 IO interface registered as a platform device. In
order to have them utilized by the DW XPCS core the fwnode-based DW XPCS
descriptor creation procedure has been introduced. Finally the STMMAC
driver has been altered to support the DW XPCS passed via the 'pcs-handle'
property.
Note the series has been significantly re-developed since v1. So I even
had to change the subject. Anyway I've done my best to take all the noted
into account.
The series traditionally starts with a set of the preparation patches.
First one just moves the generic DW XPCS IDs macros from the internal
header file to the external one where some other IDs also reside. Second
patch splits up the xpcs_create() method to a set of the coherent
sub-functions for the sake of the further easier updates and to have it
looking less complicated. The goal of the next three patches is to extend
the DW XPCS ID management code by defining a new dw_xpcs_info structure
with both PCS and PMA IDs.
The next two patches provide the DW XPCS device DT-bindings and the
respective platform-device driver for the memory-mapped DW XPCS devices.
Besides the later patch makes use of the introduced dw_xpcs_info structure
to pre-define the DW XPCS IDs based on the platform-device compatible
string. Thus if there is no way to auto-identify the XPCS device
capabilities it can be done based on the custom device IDs passed via the
MDIO-device platform data.
Final DW XPCS driver change is about adding a new method of the DW XPCS
descriptor creation. The xpcs_create_fwnode() function has been introduced
with the same semantics as a similar method recently added to the Lynx PCS
driver. It's supposed to be called with the fwnode pointing to the DW XPCS
device node, for which the XPCS descriptor will be created.
The series is terminated with two STMMAC driver patches. The former one
simplifies the DW XPCS descriptor registration procedure by dropping the
MDIO-bus scanning and creating the descriptor for the particular device
address. The later patch alters the STMMAC PCS setup method so one would
support the DW XPCS specified via the "pcs-handle" property.
That's it for now. Thanks for review in advance. Any tests are very
welcome. After this series is merged in, I'll submit another one which
adds the generic 10GBase-R and 10GBase-X interfaces support to the STMMAC
and DW XPCS driver with the proper CSRs re-initialization, PMA
initialization and reference clock selection as it's described in the
Synopsys DW XPCS HW manual.
Link: https://lore.kernel.org/netdev/[email protected]
Changelog v2:
- Drop the patches:
[PATCH net-next 01/16] net: pcs: xpcs: Drop sentinel entry from 2500basex ifaces list
[PATCH net-next 02/16] net: pcs: xpcs: Drop redundant workqueue.h include directive
[PATCH net-next 03/16] net: pcs: xpcs: Return EINVAL in the internal methods
[PATCH net-next 04/16] net: pcs: xpcs: Explicitly return error on caps validation
as ones have already been merged into the kernel repo:
Link: https://lore.kernel.org/netdev/[email protected]/
- Drop the patches:
[PATCH net-next 14/16] net: stmmac: Pass netdev to XPCS setup function
[PATCH net-next 15/16] net: stmmac: Add dedicated XPCS cleanup method
as ones have already been merged into the kernel repo:
Link: https://lore.kernel.org/netdev/[email protected]/
- Drop the patch:
[PATCH net-next 06/16] net: pcs: xpcs: Avoid creating dummy XPCS MDIO device
[PATCH net-next 09/16] net: mdio: Add Synopsys DW XPCS management interface support
[PATCH net-next 11/16] net: pcs: xpcs: Change xpcs_create_mdiodev() suffix to "byaddr"
[PATCH net-next 13/16] net: stmmac: intel: Register generic MDIO device
as no longer relevant.
- Add new patches:
[PATCH net-next v2 03/10] net: pcs: xpcs: Convert xpcs_id to dw_xpcs_desc
[PATCH net-next v2 04/10] net: pcs: xpcs: Convert xpcs_compat to dw_xpcs_compat
[PATCH net-next v2 05/10] net: pcs: xpcs: Introduce DW XPCS info structure
[PATCH net-next v2 09/10] net: stmmac: Create DW XPCS device with particular address
- Use the xpcs_create_fwnode() function name and semantics similar to the
Lynx PCS driver.
- Add kdoc describing the DW XPCS registration functions.
- Convert the memory-mapped DW XPCS device driver to being the
platform-device driver.
- Convert the DW XPCS DT-bindings to defining both memory-mapped and MDIO
devices.
- Drop inline'es from the methods statically defined in *.c. (@Maxime)
- Preserve the strict refcount-ing pattern. (@Russell)
Signed-off-by: Serge Semin <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Sagar Cheluvegowda <[email protected]>
Cc: Abhishek Chauhan <[email protected]>
Cc: Andrew Halaney <[email protected]>
Cc: Jiawen Wu <[email protected]>
Cc: Mengyuan Lou <[email protected]>
Cc: Tomer Maimon <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Serge Semin (10):
net: pcs: xpcs: Move native device ID macro to linux/pcs/pcs-xpcs.h
net: pcs: xpcs: Split up xpcs_create() body to sub-functions
net: pcs: xpcs: Convert xpcs_id to dw_xpcs_desc
net: pcs: xpcs: Convert xpcs_compat to dw_xpcs_compat
net: pcs: xpcs: Introduce DW XPCS info structure
dt-bindings: net: Add Synopsys DW xPCS bindings
net: pcs: xpcs: Add Synopsys DW xPCS platform device driver
net: pcs: xpcs: Add fwnode-based descriptor creation method
net: stmmac: Create DW XPCS device with particular address
net: stmmac: Add DW XPCS specified via "pcs-handle" support
.../bindings/net/pcs/snps,dw-xpcs.yaml | 133 +++++
.../net/ethernet/stmicro/stmmac/dwmac-intel.c | 1 +
.../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 28 +-
drivers/net/pcs/Kconfig | 6 +-
drivers/net/pcs/Makefile | 3 +-
drivers/net/pcs/pcs-xpcs-plat.c | 460 ++++++++++++++++++
drivers/net/pcs/pcs-xpcs.c | 361 +++++++++-----
drivers/net/pcs/pcs-xpcs.h | 7 +-
include/linux/pcs/pcs-xpcs.h | 49 +-
include/linux/stmmac.h | 1 +
10 files changed, 905 insertions(+), 144 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
create mode 100644 drivers/net/pcs/pcs-xpcs-plat.c
--
2.43.0
One of the next commits will alter the DW XPCS driver to support setting a
custom device ID for the particular MDIO-device detected on the platform.
The generic DW XPCS ID can be used as a custom ID as well in case if the
DW XPCS-device was erroneously synthesized with no or some undefined ID.
In addition to that having all supported DW XPCS device IDs defined in a
single place will improve the code maintainability and readability.
Note while at it rename the macros to being shorter and looking alike to
the already defined NXP XPCS ID macro.
Signed-off-by: Serge Semin <[email protected]>
---
Changelog v2:
- Alter the commit log so one would refer to the DW XPCS driver change and
would describe the change clearer. (@Russell)
- s/sinle/single (@Vladimir)
---
drivers/net/pcs/pcs-xpcs.c | 8 ++++----
drivers/net/pcs/pcs-xpcs.h | 3 ---
include/linux/pcs/pcs-xpcs.h | 2 ++
3 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 31525fe9c32e..99adbf15ab36 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -1343,16 +1343,16 @@ static const struct xpcs_compat nxp_sja1110_xpcs_compat[DW_XPCS_INTERFACE_MAX] =
static const struct xpcs_id xpcs_id_list[] = {
{
- .id = SYNOPSYS_XPCS_ID,
- .mask = SYNOPSYS_XPCS_MASK,
+ .id = DW_XPCS_ID,
+ .mask = DW_XPCS_ID_MASK,
.compat = synopsys_xpcs_compat,
}, {
.id = NXP_SJA1105_XPCS_ID,
- .mask = SYNOPSYS_XPCS_MASK,
+ .mask = DW_XPCS_ID_MASK,
.compat = nxp_sja1105_xpcs_compat,
}, {
.id = NXP_SJA1110_XPCS_ID,
- .mask = SYNOPSYS_XPCS_MASK,
+ .mask = DW_XPCS_ID_MASK,
.compat = nxp_sja1110_xpcs_compat,
},
};
diff --git a/drivers/net/pcs/pcs-xpcs.h b/drivers/net/pcs/pcs-xpcs.h
index 96c36b32ca99..369e9196f45a 100644
--- a/drivers/net/pcs/pcs-xpcs.h
+++ b/drivers/net/pcs/pcs-xpcs.h
@@ -6,9 +6,6 @@
* Author: Jose Abreu <[email protected]>
*/
-#define SYNOPSYS_XPCS_ID 0x7996ced0
-#define SYNOPSYS_XPCS_MASK 0xffffffff
-
/* Vendor regs access */
#define DW_VENDOR BIT(15)
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index da3a6c30f6d2..8dfe90295f12 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -12,6 +12,8 @@
#define NXP_SJA1105_XPCS_ID 0x00000010
#define NXP_SJA1110_XPCS_ID 0x00000020
+#define DW_XPCS_ID 0x7996ced0
+#define DW_XPCS_ID_MASK 0xffffffff
/* AN mode */
#define DW_AN_C73 1
--
2.43.0
As an initial preparation before adding the fwnode-based DW XPCS device
support let's split the xpcs_create() function code up to a set of the
small sub-functions. Thus the xpcs_create() implementation will get to
look simpler and turn to be more coherent. Further updates will just touch
the new sub-functions a bit: add platform-specific device info, add the
reference clock getting and enabling.
The xpcs_create() method will now contain the next static methods calls:
xpcs_create_data() - create the DW XPCS device descriptor, pre-initialize
it' fields and increase the mdio device refcount-er;
xpcs_init_id() - find XPCS ID instance and save it in the device
descriptor;
xpcs_init_iface() - find MAC/PCS interface descriptor and perform
basic initialization specific to it: soft-reset, disable polling.
The update doesn't imply any semantic change but merely makes the code
looking simpler and more ready for adding new features support.
Note the xpcs_destroy() has been moved to being defined below the
xpcs_create_mdiodev() function as the driver now implies having the
protagonist-then-antagonist functions definition order.
Signed-off-by: Serge Semin <[email protected]>
---
Changelog v2:
- Preserve the strict refcount-ing pattern. (@Russell)
---
drivers/net/pcs/pcs-xpcs.c | 102 +++++++++++++++++++++++++------------
1 file changed, 69 insertions(+), 33 deletions(-)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 99adbf15ab36..2dcfd0ff069a 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -1365,12 +1365,9 @@ static const struct phylink_pcs_ops xpcs_phylink_ops = {
.pcs_link_up = xpcs_link_up,
};
-static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
- phy_interface_t interface)
+static struct dw_xpcs *xpcs_create_data(struct mdio_device *mdiodev)
{
struct dw_xpcs *xpcs;
- u32 xpcs_id;
- int i, ret;
xpcs = kzalloc(sizeof(*xpcs), GFP_KERNEL);
if (!xpcs)
@@ -1378,59 +1375,89 @@ static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
mdio_device_get(mdiodev);
xpcs->mdiodev = mdiodev;
+ xpcs->pcs.ops = &xpcs_phylink_ops;
+ xpcs->pcs.neg_mode = true;
+ xpcs->pcs.poll = true;
+
+ return xpcs;
+}
+
+static void xpcs_free_data(struct dw_xpcs *xpcs)
+{
+ mdio_device_put(xpcs->mdiodev);
+ kfree(xpcs);
+}
+
+static int xpcs_init_id(struct dw_xpcs *xpcs)
+{
+ u32 xpcs_id;
+ int i, ret;
xpcs_id = xpcs_get_id(xpcs);
for (i = 0; i < ARRAY_SIZE(xpcs_id_list); i++) {
const struct xpcs_id *entry = &xpcs_id_list[i];
- const struct xpcs_compat *compat;
if ((xpcs_id & entry->mask) != entry->id)
continue;
xpcs->id = entry;
- compat = xpcs_find_compat(entry, interface);
- if (!compat) {
- ret = -ENODEV;
- goto out;
- }
+ break;
+ }
- ret = xpcs_dev_flag(xpcs);
- if (ret)
- goto out;
+ if (!xpcs->id)
+ return -ENODEV;
- xpcs->pcs.ops = &xpcs_phylink_ops;
- xpcs->pcs.neg_mode = true;
+ ret = xpcs_dev_flag(xpcs);
+ if (ret < 0)
+ return ret;
- if (xpcs->dev_flag != DW_DEV_TXGBE) {
- xpcs->pcs.poll = true;
+ return 0;
+}
- ret = xpcs_soft_reset(xpcs, compat);
- if (ret)
- goto out;
- }
+static int xpcs_init_iface(struct dw_xpcs *xpcs, phy_interface_t interface)
+{
+ const struct xpcs_compat *compat;
- return xpcs;
+ compat = xpcs_find_compat(xpcs->id, interface);
+ if (!compat)
+ return -EINVAL;
+
+ if (xpcs->dev_flag == DW_DEV_TXGBE) {
+ xpcs->pcs.poll = false;
+ return 0;
}
- ret = -ENODEV;
+ return xpcs_soft_reset(xpcs, compat);
+}
+
+static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
+ phy_interface_t interface)
+{
+ struct dw_xpcs *xpcs;
+ int ret;
+
+ xpcs = xpcs_create_data(mdiodev);
+ if (IS_ERR(xpcs))
+ return xpcs;
+
+ ret = xpcs_init_id(xpcs);
+ if (ret)
+ goto out;
+
+ ret = xpcs_init_iface(xpcs, interface);
+ if (ret)
+ goto out;
+
+ return xpcs;
out:
- mdio_device_put(mdiodev);
- kfree(xpcs);
+ xpcs_free_data(xpcs);
return ERR_PTR(ret);
}
-void xpcs_destroy(struct dw_xpcs *xpcs)
-{
- if (xpcs)
- mdio_device_put(xpcs->mdiodev);
- kfree(xpcs);
-}
-EXPORT_SYMBOL_GPL(xpcs_destroy);
-
struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
phy_interface_t interface)
{
@@ -1455,5 +1482,14 @@ struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
}
EXPORT_SYMBOL_GPL(xpcs_create_mdiodev);
+void xpcs_destroy(struct dw_xpcs *xpcs)
+{
+ if (!xpcs)
+ return;
+
+ xpcs_free_data(xpcs);
+}
+EXPORT_SYMBOL_GPL(xpcs_destroy);
+
MODULE_DESCRIPTION("Synopsys DesignWare XPCS library");
MODULE_LICENSE("GPL v2");
--
2.43.0
A structure with the PCS/PMA MMD IDs data is being introduced in one of
the next commits. In order to prevent the names ambiguity let's convert
the xpcs_id structure name to dw_xpcs_desc. The later version is more
suitable since the structure content is indeed the device descriptor
containing the data and callbacks required for the driver to correctly set
the device up.
Signed-off-by: Serge Semin <[email protected]>
---
Changelog v2:
- This is a new patch introduced on v2 stage of the review.
---
drivers/net/pcs/pcs-xpcs.c | 30 +++++++++++++++---------------
include/linux/pcs/pcs-xpcs.h | 4 ++--
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 2dcfd0ff069a..48c61975db22 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -151,19 +151,19 @@ struct xpcs_compat {
int (*pma_config)(struct dw_xpcs *xpcs);
};
-struct xpcs_id {
+struct dw_xpcs_desc {
u32 id;
u32 mask;
const struct xpcs_compat *compat;
};
-static const struct xpcs_compat *xpcs_find_compat(const struct xpcs_id *id,
- phy_interface_t interface)
+static const struct xpcs_compat *
+xpcs_find_compat(const struct dw_xpcs_desc *desc, phy_interface_t interface)
{
int i, j;
for (i = 0; i < DW_XPCS_INTERFACE_MAX; i++) {
- const struct xpcs_compat *compat = &id->compat[i];
+ const struct xpcs_compat *compat = &desc->compat[i];
for (j = 0; j < compat->num_interfaces; j++)
if (compat->interface[j] == interface)
@@ -177,7 +177,7 @@ int xpcs_get_an_mode(struct dw_xpcs *xpcs, phy_interface_t interface)
{
const struct xpcs_compat *compat;
- compat = xpcs_find_compat(xpcs->id, interface);
+ compat = xpcs_find_compat(xpcs->desc, interface);
if (!compat)
return -ENODEV;
@@ -612,7 +612,7 @@ static int xpcs_validate(struct phylink_pcs *pcs, unsigned long *supported,
int i;
xpcs = phylink_pcs_to_xpcs(pcs);
- compat = xpcs_find_compat(xpcs->id, state->interface);
+ compat = xpcs_find_compat(xpcs->desc, state->interface);
if (!compat)
return -EINVAL;
@@ -633,7 +633,7 @@ void xpcs_get_interfaces(struct dw_xpcs *xpcs, unsigned long *interfaces)
int i, j;
for (i = 0; i < DW_XPCS_INTERFACE_MAX; i++) {
- const struct xpcs_compat *compat = &xpcs->id->compat[i];
+ const struct xpcs_compat *compat = &xpcs->desc->compat[i];
for (j = 0; j < compat->num_interfaces; j++)
__set_bit(compat->interface[j], interfaces);
@@ -853,7 +853,7 @@ int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
const struct xpcs_compat *compat;
int ret;
- compat = xpcs_find_compat(xpcs->id, interface);
+ compat = xpcs_find_compat(xpcs->desc, interface);
if (!compat)
return -ENODEV;
@@ -1118,7 +1118,7 @@ static void xpcs_get_state(struct phylink_pcs *pcs,
const struct xpcs_compat *compat;
int ret;
- compat = xpcs_find_compat(xpcs->id, state->interface);
+ compat = xpcs_find_compat(xpcs->desc, state->interface);
if (!compat)
return;
@@ -1341,7 +1341,7 @@ static const struct xpcs_compat nxp_sja1110_xpcs_compat[DW_XPCS_INTERFACE_MAX] =
},
};
-static const struct xpcs_id xpcs_id_list[] = {
+static const struct dw_xpcs_desc xpcs_desc_list[] = {
{
.id = DW_XPCS_ID,
.mask = DW_XPCS_ID_MASK,
@@ -1395,18 +1395,18 @@ static int xpcs_init_id(struct dw_xpcs *xpcs)
xpcs_id = xpcs_get_id(xpcs);
- for (i = 0; i < ARRAY_SIZE(xpcs_id_list); i++) {
- const struct xpcs_id *entry = &xpcs_id_list[i];
+ for (i = 0; i < ARRAY_SIZE(xpcs_desc_list); i++) {
+ const struct dw_xpcs_desc *entry = &xpcs_desc_list[i];
if ((xpcs_id & entry->mask) != entry->id)
continue;
- xpcs->id = entry;
+ xpcs->desc = entry;
break;
}
- if (!xpcs->id)
+ if (!xpcs->desc)
return -ENODEV;
ret = xpcs_dev_flag(xpcs);
@@ -1420,7 +1420,7 @@ static int xpcs_init_iface(struct dw_xpcs *xpcs, phy_interface_t interface)
{
const struct xpcs_compat *compat;
- compat = xpcs_find_compat(xpcs->id, interface);
+ compat = xpcs_find_compat(xpcs->desc, interface);
if (!compat)
return -EINVAL;
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index 8dfe90295f12..e706bd16b986 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -28,11 +28,11 @@
/* dev_flag */
#define DW_DEV_TXGBE BIT(0)
-struct xpcs_id;
+struct dw_xpcs_desc;
struct dw_xpcs {
+ const struct dw_xpcs_desc *desc;
struct mdio_device *mdiodev;
- const struct xpcs_id *id;
struct phylink_pcs pcs;
phy_interface_t interface;
int dev_flag;
--
2.43.0
The being introduced structure will preserve the PCS and PMA IDs retrieved
from the respective DW XPCS MMDs or potentially pre-defined by the client
drivers. (The later change will be introduced later in the framework of
the commit adding the memory-mapped DW XPCS devices support.)
The structure fields are filled in in the xpcs_get_id() function, which
used to be responsible for the PCS Device ID getting only. Besides of the
PCS ID the method now fetches the PMA/PMD IDs too from MMD 1, which used
to be done in xpcs_dev_flag(). The retrieved PMA ID will be from now
utilized for the PMA-specific tweaks like it was introduced for the
Wangxun TxGBE PCS in the commit f629acc6f210 ("net: pcs: xpcs: support to
switch mode for Wangxun NICs").
Note 1. The xpcs_get_id() error-handling semantics has been changed. From
now the error number will be returned from the function. There is no point
in the next IOs or saving 0xffs and then looping over the actual device
IDs if device couldn't be reached. -ENODEV will be returned if the very
first IO operation failed thus indicating that no device could be found.
Note 2. The PCS and PMA IDs macros have been converted to enum'es. The
enum'es will be populated later in another commit with the virtual IDs
identifying the DW XPCS devices which have some platform-specifics, but
have been synthesized with the default PCS/PMA ID.
Signed-off-by: Serge Semin <[email protected]>
---
Changelog v2:
- This is a new patch introduced due to the commit adding the Wangxun
TXGbe PCS support.
---
drivers/net/pcs/pcs-xpcs.c | 104 +++++++++++++++++------------------
include/linux/pcs/pcs-xpcs.h | 28 ++++++----
2 files changed, 67 insertions(+), 65 deletions(-)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 0af6b5995113..e8d5fd43a357 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -237,29 +237,6 @@ int xpcs_write_vpcs(struct dw_xpcs *xpcs, int reg, u16 val)
return xpcs_write_vendor(xpcs, MDIO_MMD_PCS, reg, val);
}
-static int xpcs_dev_flag(struct dw_xpcs *xpcs)
-{
- int ret, oui;
-
- ret = xpcs_read(xpcs, MDIO_MMD_PMAPMD, MDIO_DEVID1);
- if (ret < 0)
- return ret;
-
- oui = ret;
-
- ret = xpcs_read(xpcs, MDIO_MMD_PMAPMD, MDIO_DEVID2);
- if (ret < 0)
- return ret;
-
- ret = (ret >> 10) & 0x3F;
- oui |= ret << 16;
-
- if (oui == DW_OUI_WX)
- xpcs->dev_flag = DW_DEV_TXGBE;
-
- return 0;
-}
-
static int xpcs_poll_reset(struct dw_xpcs *xpcs, int dev)
{
/* Poll until the reset bit clears (50ms per retry == 0.6 sec) */
@@ -684,7 +661,7 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
{
int ret, mdio_ctrl, tx_conf;
- if (xpcs->dev_flag == DW_DEV_TXGBE)
+ if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID)
xpcs_write_vpcs(xpcs, DW_VR_XS_PCS_DIG_CTRL1, DW_CL37_BP | DW_EN_VSMMD1);
/* For AN for C37 SGMII mode, the settings are :-
@@ -722,7 +699,7 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
ret |= (DW_VR_MII_PCS_MODE_C37_SGMII <<
DW_VR_MII_AN_CTRL_PCS_MODE_SHIFT &
DW_VR_MII_PCS_MODE_MASK);
- if (xpcs->dev_flag == DW_DEV_TXGBE) {
+ if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID) {
ret |= DW_VR_MII_AN_CTRL_8BIT;
/* Hardware requires it to be PHY side SGMII */
tx_conf = DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII;
@@ -744,7 +721,7 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
else
ret &= ~DW_VR_MII_DIG_CTRL1_MAC_AUTO_SW;
- if (xpcs->dev_flag == DW_DEV_TXGBE)
+ if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID)
ret |= DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL;
ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_DIG_CTRL1, ret);
@@ -766,7 +743,7 @@ static int xpcs_config_aneg_c37_1000basex(struct dw_xpcs *xpcs,
int ret, mdio_ctrl, adv;
bool changed = 0;
- if (xpcs->dev_flag == DW_DEV_TXGBE)
+ if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID)
xpcs_write_vpcs(xpcs, DW_VR_XS_PCS_DIG_CTRL1, DW_CL37_BP | DW_EN_VSMMD1);
/* According to Chap 7.12, to set 1000BASE-X C37 AN, AN must
@@ -857,7 +834,7 @@ int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
if (!compat)
return -ENODEV;
- if (xpcs->dev_flag == DW_DEV_TXGBE) {
+ if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID) {
ret = txgbe_xpcs_switch_mode(xpcs, interface);
if (ret)
return ret;
@@ -1229,44 +1206,66 @@ static void xpcs_an_restart(struct phylink_pcs *pcs)
}
}
-static u32 xpcs_get_id(struct dw_xpcs *xpcs)
+static int xpcs_get_id(struct dw_xpcs *xpcs)
{
int ret;
u32 id;
- /* First, search C73 PCS using PCS MMD */
+ /* First, search C73 PCS using PCS MMD 3. Return ENODEV if communication
+ * failed indicating that device couldn't be reached.
+ */
ret = xpcs_read(xpcs, MDIO_MMD_PCS, MII_PHYSID1);
if (ret < 0)
- return 0xffffffff;
+ return -ENODEV;
id = ret << 16;
ret = xpcs_read(xpcs, MDIO_MMD_PCS, MII_PHYSID2);
if (ret < 0)
- return 0xffffffff;
+ return ret;
- /* If Device IDs are not all zeros or all ones,
- * we found C73 AN-type device
+ id |= ret;
+
+ /* If Device IDs are not all zeros or ones, then 10GBase-X/R or C73
+ * KR/KX4 PCS found. Otherwise fallback to detecting 1000Base-X or C37
+ * PCS in MII MMD 31.
*/
- if ((id | ret) && (id | ret) != 0xffffffff)
- return id | ret;
+ if (!id || id == 0xffffffff) {
+ ret = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_PHYSID1);
+ if (ret < 0)
+ return ret;
+
+ id = ret << 16;
+
+ ret = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_PHYSID2);
+ if (ret < 0)
+ return ret;
- /* Next, search C37 PCS using Vendor-Specific MII MMD */
- ret = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_PHYSID1);
+ id |= ret;
+ }
+
+ xpcs->info.pcs = id;
+
+ /* Find out PMA/PMD ID from MMD 1 device ID registers */
+ ret = xpcs_read(xpcs, MDIO_MMD_PMAPMD, MDIO_DEVID1);
if (ret < 0)
- return 0xffffffff;
+ return ret;
- id = ret << 16;
+ id = ret;
- ret = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_PHYSID2);
+ ret = xpcs_read(xpcs, MDIO_MMD_PMAPMD, MDIO_DEVID2);
if (ret < 0)
- return 0xffffffff;
+ return ret;
+
+ /* Note the inverted dword order and masked out Model/Revision numbers
+ * with respect to what is done with the PCS ID...
+ */
+ ret = (ret >> 10) & 0x3F;
+ id |= ret << 16;
- /* If Device IDs are not all zeros, we found C37 AN-type device */
- if (id | ret)
- return id | ret;
+ xpcs->info.pma = id;
- return 0xffffffff;
+ return 0;
}
static const struct dw_xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
@@ -1390,15 +1389,16 @@ static void xpcs_free_data(struct dw_xpcs *xpcs)
static int xpcs_init_id(struct dw_xpcs *xpcs)
{
- u32 xpcs_id;
int i, ret;
- xpcs_id = xpcs_get_id(xpcs);
+ ret = xpcs_get_id(xpcs);
+ if (ret < 0)
+ return ret;
for (i = 0; i < ARRAY_SIZE(xpcs_desc_list); i++) {
const struct dw_xpcs_desc *entry = &xpcs_desc_list[i];
- if ((xpcs_id & entry->mask) != entry->id)
+ if ((xpcs->info.pcs & entry->mask) != entry->id)
continue;
xpcs->desc = entry;
@@ -1409,10 +1409,6 @@ static int xpcs_init_id(struct dw_xpcs *xpcs)
if (!xpcs->desc)
return -ENODEV;
- ret = xpcs_dev_flag(xpcs);
- if (ret < 0)
- return ret;
-
return 0;
}
@@ -1424,7 +1420,7 @@ static int xpcs_init_iface(struct dw_xpcs *xpcs, phy_interface_t interface)
if (!compat)
return -EINVAL;
- if (xpcs->dev_flag == DW_DEV_TXGBE) {
+ if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID) {
xpcs->pcs.poll = false;
return 0;
}
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index e706bd16b986..1dc60f5e653f 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -9,11 +9,7 @@
#include <linux/phy.h>
#include <linux/phylink.h>
-
-#define NXP_SJA1105_XPCS_ID 0x00000010
-#define NXP_SJA1110_XPCS_ID 0x00000020
-#define DW_XPCS_ID 0x7996ced0
-#define DW_XPCS_ID_MASK 0xffffffff
+#include <linux/types.h>
/* AN mode */
#define DW_AN_C73 1
@@ -22,20 +18,30 @@
#define DW_AN_C37_1000BASEX 4
#define DW_10GBASER 5
-/* device vendor OUI */
-#define DW_OUI_WX 0x0018fc80
+struct dw_xpcs_desc;
-/* dev_flag */
-#define DW_DEV_TXGBE BIT(0)
+enum dw_xpcs_pcs_id {
+ NXP_SJA1105_XPCS_ID = 0x00000010,
+ NXP_SJA1110_XPCS_ID = 0x00000020,
+ DW_XPCS_ID = 0x7996ced0,
+ DW_XPCS_ID_MASK = 0xffffffff,
+};
-struct dw_xpcs_desc;
+enum dw_xpcs_pma_id {
+ WX_TXGBE_XPCS_PMA_10G_ID = 0x0018fc80,
+};
+
+struct dw_xpcs_info {
+ u32 pcs;
+ u32 pma;
+};
struct dw_xpcs {
+ struct dw_xpcs_info info;
const struct dw_xpcs_desc *desc;
struct mdio_device *mdiodev;
struct phylink_pcs pcs;
phy_interface_t interface;
- int dev_flag;
};
int xpcs_get_an_mode(struct dw_xpcs *xpcs, phy_interface_t interface);
--
2.43.0
Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
providing an interface between the Media Access Control (MAC) and Physical
Medium Attachment Sublayer (PMA) through a Media independent interface.
From software point of view it exposes IEEE std. Clause 45 CSR space and
can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
case the PCS device is supposed to be defined under the respective MDIO
bus DT-node. In the later case the DW xPCS will be just a normal IO
memory-mapped device.
Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
source properties specified. The former one indicates the Clause 73/37
auto-negotiation events like: negotiation page received, AN is completed
or incompatible link partner. The clock DT-properties can describe up to
three clock sources: peripheral bus clock source, internal reference clock
and the externally connected reference clock.
Finally the DW XPCS IP-core can be optionally synthesized with a
vendor-specific interface connected to the Synopsys PMA (also called
DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
portable way. So if the DW XPCS device has the respective PMA attached
then it should be reflected in the DT-node compatible string so the driver
would be aware of the PMA-specific device capabilities (mainly connected
with CSRs available for the fine-tunings).
Signed-off-by: Serge Semin <[email protected]>
---
Changelog v2:
- Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
interface is just a normal memory-mapped device.
---
.../bindings/net/pcs/snps,dw-xpcs.yaml | 133 ++++++++++++++++++
1 file changed, 133 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
new file mode 100644
index 000000000000..7927bceefbf3
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
@@ -0,0 +1,133 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare Ethernet PCS
+
+maintainers:
+ - Serge Semin <[email protected]>
+
+description:
+ Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
+ between Media Access Control and Physical Medium Attachment Sublayer through
+ the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
+ controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
+ optionally synthesized with a vendor-specific interface connected to
+ Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
+ general it can be used to communicate with any compatible PHY.
+
+ The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
+ by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
+ right to the system IO memory space.
+
+properties:
+ compatible:
+ oneOf:
+ - description: Synopsys DesignWare XPCS with none or unknown PMA
+ const: snps,dw-xpcs
+ - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
+ const: snps,dw-xpcs-gen1-3g
+ - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
+ const: snps,dw-xpcs-gen2-3g
+ - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
+ const: snps,dw-xpcs-gen2-6g
+ - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
+ const: snps,dw-xpcs-gen4-3g
+ - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
+ const: snps,dw-xpcs-gen4-6g
+ - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
+ const: snps,dw-xpcs-gen5-10g
+ - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
+ const: snps,dw-xpcs-gen5-12g
+
+ reg:
+ items:
+ - description:
+ In case of the MDIO management interface this just a 5-bits ID
+ of the MDIO bus device. If DW XPCS CSRs space is accessed over the
+ MCI or APB3 management interfaces, then the space mapping can be
+ either 'direct' or 'indirect'. In the former case all Clause 45
+ registers are contiguously mapped within the address space
+ MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
+ to the multiple 256 register sets. There is a special viewport CSR
+ which is responsible for the set selection. The upper part of
+ the CSR address MMD+REG[20:8] is supposed to be written in there
+ so the corresponding subset would be mapped to the lowest 255 CSRs.
+
+ reg-names:
+ items:
+ - enum: [ direct, indirect ]
+
+ reg-io-width:
+ description:
+ The way the CSRs are mapped to the memory is platform depended. Since
+ each Clause 45 CSR is of 16-bits wide the access instructions must be
+ two bytes aligned at least.
+ default: 2
+ enum: [ 2, 4 ]
+
+ interrupts:
+ description:
+ System interface interrupt output (sbd_intr_o) indicating Clause 73/37
+ auto-negotiation events':' Page received, AN is completed or incompatible
+ link partner.
+ maxItems: 1
+
+ clocks:
+ description:
+ Both MCI and APB3 interfaces are supposed to be equipped with a clock
+ source connected via the clk_csr_i line.
+
+ PCS/PMA layer can be clocked by an internal reference clock source
+ (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
+ generator. Both clocks can be supplied at a time.
+ minItems: 1
+ maxItems: 3
+
+ clock-names:
+ minItems: 1
+ maxItems: 3
+ anyOf:
+ - items:
+ enum: [ core, pad ]
+ - items:
+ enum: [ pclk, core, pad ]
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ ethernet-pcs@1f05d000 {
+ compatible = "snps,dw-xpcs";
+ reg = <0x1f05d000 0x1000>;
+ reg-names = "indirect";
+
+ reg-io-width = <4>;
+
+ interrupts = <79 IRQ_TYPE_LEVEL_HIGH>;
+
+ clocks = <&ccu_pclk>, <&ccu_core>, <&ccu_pad>;
+ clock-names = "pclk", "core", "pad";
+ };
+ - |
+ mdio-bus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet-pcs@0 {
+ compatible = "snps,dw-xpcs";
+ reg = <0>;
+
+ clocks = <&ccu_core>, <&ccu_pad>;
+ clock-names = "core", "pad";
+ };
+ };
+...
--
2.43.0
The xpcs_compat structure has been left as the only dw-prefix-less
structure since the previous commit. Let's unify at least the structures
naming in the driver by adding the dw_-prefix to it.
Signed-off-by: Serge Semin <[email protected]>
---
Changelog v2:
- This is a new patch introduced on v2 stage of the review.
---
drivers/net/pcs/pcs-xpcs.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 48c61975db22..0af6b5995113 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -143,7 +143,7 @@ enum {
DW_XPCS_INTERFACE_MAX,
};
-struct xpcs_compat {
+struct dw_xpcs_compat {
const int *supported;
const phy_interface_t *interface;
int num_interfaces;
@@ -154,16 +154,16 @@ struct xpcs_compat {
struct dw_xpcs_desc {
u32 id;
u32 mask;
- const struct xpcs_compat *compat;
+ const struct dw_xpcs_compat *compat;
};
-static const struct xpcs_compat *
+static const struct dw_xpcs_compat *
xpcs_find_compat(const struct dw_xpcs_desc *desc, phy_interface_t interface)
{
int i, j;
for (i = 0; i < DW_XPCS_INTERFACE_MAX; i++) {
- const struct xpcs_compat *compat = &desc->compat[i];
+ const struct dw_xpcs_compat *compat = &desc->compat[i];
for (j = 0; j < compat->num_interfaces; j++)
if (compat->interface[j] == interface)
@@ -175,7 +175,7 @@ xpcs_find_compat(const struct dw_xpcs_desc *desc, phy_interface_t interface)
int xpcs_get_an_mode(struct dw_xpcs *xpcs, phy_interface_t interface)
{
- const struct xpcs_compat *compat;
+ const struct dw_xpcs_compat *compat;
compat = xpcs_find_compat(xpcs->desc, interface);
if (!compat)
@@ -185,7 +185,7 @@ int xpcs_get_an_mode(struct dw_xpcs *xpcs, phy_interface_t interface)
}
EXPORT_SYMBOL_GPL(xpcs_get_an_mode);
-static bool __xpcs_linkmode_supported(const struct xpcs_compat *compat,
+static bool __xpcs_linkmode_supported(const struct dw_xpcs_compat *compat,
enum ethtool_link_mode_bit_indices linkmode)
{
int i;
@@ -277,7 +277,7 @@ static int xpcs_poll_reset(struct dw_xpcs *xpcs, int dev)
}
static int xpcs_soft_reset(struct dw_xpcs *xpcs,
- const struct xpcs_compat *compat)
+ const struct dw_xpcs_compat *compat)
{
int ret, dev;
@@ -418,7 +418,7 @@ static void xpcs_config_usxgmii(struct dw_xpcs *xpcs, int speed)
}
static int _xpcs_config_aneg_c73(struct dw_xpcs *xpcs,
- const struct xpcs_compat *compat)
+ const struct dw_xpcs_compat *compat)
{
int ret, adv;
@@ -463,7 +463,7 @@ static int _xpcs_config_aneg_c73(struct dw_xpcs *xpcs,
}
static int xpcs_config_aneg_c73(struct dw_xpcs *xpcs,
- const struct xpcs_compat *compat)
+ const struct dw_xpcs_compat *compat)
{
int ret;
@@ -482,7 +482,7 @@ static int xpcs_config_aneg_c73(struct dw_xpcs *xpcs,
static int xpcs_aneg_done_c73(struct dw_xpcs *xpcs,
struct phylink_link_state *state,
- const struct xpcs_compat *compat, u16 an_stat1)
+ const struct dw_xpcs_compat *compat, u16 an_stat1)
{
int ret;
@@ -607,7 +607,7 @@ static int xpcs_validate(struct phylink_pcs *pcs, unsigned long *supported,
const struct phylink_link_state *state)
{
__ETHTOOL_DECLARE_LINK_MODE_MASK(xpcs_supported) = { 0, };
- const struct xpcs_compat *compat;
+ const struct dw_xpcs_compat *compat;
struct dw_xpcs *xpcs;
int i;
@@ -633,7 +633,7 @@ void xpcs_get_interfaces(struct dw_xpcs *xpcs, unsigned long *interfaces)
int i, j;
for (i = 0; i < DW_XPCS_INTERFACE_MAX; i++) {
- const struct xpcs_compat *compat = &xpcs->desc->compat[i];
+ const struct dw_xpcs_compat *compat = &xpcs->desc->compat[i];
for (j = 0; j < compat->num_interfaces; j++)
__set_bit(compat->interface[j], interfaces);
@@ -850,7 +850,7 @@ static int xpcs_config_2500basex(struct dw_xpcs *xpcs)
int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
const unsigned long *advertising, unsigned int neg_mode)
{
- const struct xpcs_compat *compat;
+ const struct dw_xpcs_compat *compat;
int ret;
compat = xpcs_find_compat(xpcs->desc, interface);
@@ -915,7 +915,7 @@ static int xpcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
static int xpcs_get_state_c73(struct dw_xpcs *xpcs,
struct phylink_link_state *state,
- const struct xpcs_compat *compat)
+ const struct dw_xpcs_compat *compat)
{
bool an_enabled;
int pcs_stat1;
@@ -1115,7 +1115,7 @@ static void xpcs_get_state(struct phylink_pcs *pcs,
struct phylink_link_state *state)
{
struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs);
- const struct xpcs_compat *compat;
+ const struct dw_xpcs_compat *compat;
int ret;
compat = xpcs_find_compat(xpcs->desc, state->interface);
@@ -1269,7 +1269,7 @@ static u32 xpcs_get_id(struct dw_xpcs *xpcs)
return 0xffffffff;
}
-static const struct xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
+static const struct dw_xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
[DW_XPCS_USXGMII] = {
.supported = xpcs_usxgmii_features,
.interface = xpcs_usxgmii_interfaces,
@@ -1314,7 +1314,7 @@ static const struct xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
},
};
-static const struct xpcs_compat nxp_sja1105_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
+static const struct dw_xpcs_compat nxp_sja1105_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
[DW_XPCS_SGMII] = {
.supported = xpcs_sgmii_features,
.interface = xpcs_sgmii_interfaces,
@@ -1324,7 +1324,7 @@ static const struct xpcs_compat nxp_sja1105_xpcs_compat[DW_XPCS_INTERFACE_MAX] =
},
};
-static const struct xpcs_compat nxp_sja1110_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
+static const struct dw_xpcs_compat nxp_sja1110_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
[DW_XPCS_SGMII] = {
.supported = xpcs_sgmii_features,
.interface = xpcs_sgmii_interfaces,
@@ -1418,7 +1418,7 @@ static int xpcs_init_id(struct dw_xpcs *xpcs)
static int xpcs_init_iface(struct dw_xpcs *xpcs, phy_interface_t interface)
{
- const struct xpcs_compat *compat;
+ const struct dw_xpcs_compat *compat;
compat = xpcs_find_compat(xpcs->desc, interface);
if (!compat)
--
2.43.0
Synopsys DesignWare XPCS IP-core can be synthesized with the device CSRs
being accessible over the MCI or APB3 interface instead of the MDIO bus
(see the CSR_INTERFACE HDL parameter). Thus all the PCS registers can be
just memory mapped and be a subject of the standard MMIO operations of
course taking into account the peculiarities of the Clause C45 CSRs
mapping. From that perspective the DW XPCS devices would look as just
normal platform devices for the kernel.
On the other hand in order to have the DW XPCS devices handled by the
pcs-xpcs.c driver they need to be registered in the framework of the
MDIO-subsystem. So the suggested change is about providing a DW XPCS
platform device driver registering a virtual MDIO-bus with a single
MDIO-device representing the DW XPCS device.
DW XPCS platform device is supposed to be described by the respective
compatible string "snps,dw-xpcs" (or with the PMA-specific compatible
string), CSRs memory space and optional peripheral bus and reference clock
sources. Depending on the INDIRECT_ACCESS IP-core synthesize parameter the
memory-mapped reg-space can be represented as either directly or
indirectly mapped Clause 45 space. In the former case the particular
address is determined based on the MMD device and the registers offset (5
+ 16 bits all together) within the device reg-space. In the later case
there is only 8 lower address bits are utilized for the registers mapping
(255 CSRs). The upper bits are supposed to be written into the respective
viewport CSR in order to select the respective MMD sub-page.
Note, only the peripheral bus clock source is requested in the platform
device probe procedure. The core and pad clocks handling has been
implemented in the framework of the xpcs_create() method intentionally
since the clocks-related setups are supposed to be performed later, during
the DW XPCS main configuration procedures. (For instance they will be
required for the DW Gen5 10G PMA configuration.)
Signed-off-by: Serge Semin <[email protected]>
---
Changelog v2:
- This is a new patch created by merging in two former patches:
[PATCH net-next 09/16] net: mdio: Add Synopsys DW XPCS management interface support
[PATCH net-next 10/16] net: pcs: xpcs: Add generic DW XPCS MDIO-device support
- Drop inline'es from the statically defined in *.c methods. (@Maxime)
---
drivers/net/pcs/Kconfig | 6 +-
drivers/net/pcs/Makefile | 3 +-
drivers/net/pcs/pcs-xpcs-plat.c | 460 ++++++++++++++++++++++++++++++++
drivers/net/pcs/pcs-xpcs.c | 63 ++++-
drivers/net/pcs/pcs-xpcs.h | 6 +
include/linux/pcs/pcs-xpcs.h | 18 ++
6 files changed, 547 insertions(+), 9 deletions(-)
create mode 100644 drivers/net/pcs/pcs-xpcs-plat.c
diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
index 87cf308fc6d8..f6aa437473de 100644
--- a/drivers/net/pcs/Kconfig
+++ b/drivers/net/pcs/Kconfig
@@ -6,11 +6,11 @@
menu "PCS device drivers"
config PCS_XPCS
- tristate
+ tristate "Synopsys DesignWare Ethernet XPCS"
select PHYLINK
help
- This module provides helper functions for Synopsys DesignWare XPCS
- controllers.
+ This module provides a driver and helper functions for Synopsys
+ DesignWare XPCS controllers.
config PCS_LYNX
tristate
diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
index fb1694192ae6..4f7920618b90 100644
--- a/drivers/net/pcs/Makefile
+++ b/drivers/net/pcs/Makefile
@@ -1,7 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for Linux PCS drivers
-pcs_xpcs-$(CONFIG_PCS_XPCS) := pcs-xpcs.o pcs-xpcs-nxp.o pcs-xpcs-wx.o
+pcs_xpcs-$(CONFIG_PCS_XPCS) := pcs-xpcs.o pcs-xpcs-plat.o \
+ pcs-xpcs-nxp.o pcs-xpcs-wx.o
obj-$(CONFIG_PCS_XPCS) += pcs_xpcs.o
obj-$(CONFIG_PCS_LYNX) += pcs-lynx.o
diff --git a/drivers/net/pcs/pcs-xpcs-plat.c b/drivers/net/pcs/pcs-xpcs-plat.c
new file mode 100644
index 000000000000..5b706f6939a6
--- /dev/null
+++ b/drivers/net/pcs/pcs-xpcs-plat.c
@@ -0,0 +1,460 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Synopsys DesignWare XPCS platform device driver
+ *
+ * Copyright (C) 2024 Serge Semin
+ */
+
+#include <linux/atomic.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mdio.h>
+#include <linux/module.h>
+#include <linux/pcs/pcs-xpcs.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/property.h>
+#include <linux/sizes.h>
+
+#include "pcs-xpcs.h"
+
+/* Page select register for the indirect MMIO CSRs access */
+#define DW_VR_CSR_VIEWPORT 0xff
+
+struct dw_xpcs_plat {
+ struct platform_device *pdev;
+ struct mii_bus *bus;
+ bool reg_indir;
+ int reg_width;
+ void __iomem *reg_base;
+ struct clk *pclk;
+};
+
+static ptrdiff_t xpcs_mmio_addr_format(int dev, int reg)
+{
+ return FIELD_PREP(0x1f0000, dev) | FIELD_PREP(0xffff, reg);
+}
+
+static u16 xpcs_mmio_addr_page(ptrdiff_t csr)
+{
+ return FIELD_GET(0x1fff00, csr);
+}
+
+static ptrdiff_t xpcs_mmio_addr_offset(ptrdiff_t csr)
+{
+ return FIELD_GET(0xff, csr);
+}
+
+static int xpcs_mmio_read_reg_indirect(struct dw_xpcs_plat *pxpcs,
+ int dev, int reg)
+{
+ ptrdiff_t csr, ofs;
+ u16 page;
+ int ret;
+
+ csr = xpcs_mmio_addr_format(dev, reg);
+ page = xpcs_mmio_addr_page(csr);
+ ofs = xpcs_mmio_addr_offset(csr);
+
+ ret = pm_runtime_resume_and_get(&pxpcs->pdev->dev);
+ if (ret)
+ return ret;
+
+ switch (pxpcs->reg_width) {
+ case 4:
+ writel(page, pxpcs->reg_base + (DW_VR_CSR_VIEWPORT << 2));
+ ret = readl(pxpcs->reg_base + (ofs << 2));
+ break;
+ default:
+ writew(page, pxpcs->reg_base + (DW_VR_CSR_VIEWPORT << 1));
+ ret = readw(pxpcs->reg_base + (ofs << 1));
+ break;
+ }
+
+ pm_runtime_put(&pxpcs->pdev->dev);
+
+ return ret;
+}
+
+static int xpcs_mmio_write_reg_indirect(struct dw_xpcs_plat *pxpcs,
+ int dev, int reg, u16 val)
+{
+ ptrdiff_t csr, ofs;
+ u16 page;
+ int ret;
+
+ csr = xpcs_mmio_addr_format(dev, reg);
+ page = xpcs_mmio_addr_page(csr);
+ ofs = xpcs_mmio_addr_offset(csr);
+
+ ret = pm_runtime_resume_and_get(&pxpcs->pdev->dev);
+ if (ret)
+ return ret;
+
+ switch (pxpcs->reg_width) {
+ case 4:
+ writel(page, pxpcs->reg_base + (DW_VR_CSR_VIEWPORT << 2));
+ writel(val, pxpcs->reg_base + (ofs << 2));
+ break;
+ default:
+ writew(page, pxpcs->reg_base + (DW_VR_CSR_VIEWPORT << 1));
+ writew(val, pxpcs->reg_base + (ofs << 1));
+ break;
+ }
+
+ pm_runtime_put(&pxpcs->pdev->dev);
+
+ return 0;
+}
+
+static int xpcs_mmio_read_reg_direct(struct dw_xpcs_plat *pxpcs,
+ int dev, int reg)
+{
+ ptrdiff_t csr;
+ int ret;
+
+ csr = xpcs_mmio_addr_format(dev, reg);
+
+ ret = pm_runtime_resume_and_get(&pxpcs->pdev->dev);
+ if (ret)
+ return ret;
+
+ switch (pxpcs->reg_width) {
+ case 4:
+ ret = readl(pxpcs->reg_base + (csr << 2));
+ break;
+ default:
+ ret = readw(pxpcs->reg_base + (csr << 1));
+ break;
+ }
+
+ pm_runtime_put(&pxpcs->pdev->dev);
+
+ return ret;
+}
+
+static int xpcs_mmio_write_reg_direct(struct dw_xpcs_plat *pxpcs,
+ int dev, int reg, u16 val)
+{
+ ptrdiff_t csr;
+ int ret;
+
+ csr = xpcs_mmio_addr_format(dev, reg);
+
+ ret = pm_runtime_resume_and_get(&pxpcs->pdev->dev);
+ if (ret)
+ return ret;
+
+ switch (pxpcs->reg_width) {
+ case 4:
+ writel(val, pxpcs->reg_base + (csr << 2));
+ break;
+ default:
+ writew(val, pxpcs->reg_base + (csr << 1));
+ break;
+ }
+
+ pm_runtime_put(&pxpcs->pdev->dev);
+
+ return 0;
+}
+
+static int xpcs_mmio_read_c22(struct mii_bus *bus, int addr, int reg)
+{
+ struct dw_xpcs_plat *pxpcs = bus->priv;
+
+ if (addr != 0)
+ return -ENODEV;
+
+ if (pxpcs->reg_indir)
+ return xpcs_mmio_read_reg_indirect(pxpcs, MDIO_MMD_VEND2, reg);
+ else
+ return xpcs_mmio_read_reg_direct(pxpcs, MDIO_MMD_VEND2, reg);
+}
+
+static int xpcs_mmio_write_c22(struct mii_bus *bus, int addr, int reg, u16 val)
+{
+ struct dw_xpcs_plat *pxpcs = bus->priv;
+
+ if (addr != 0)
+ return -ENODEV;
+
+ if (pxpcs->reg_indir)
+ return xpcs_mmio_write_reg_indirect(pxpcs, MDIO_MMD_VEND2, reg, val);
+ else
+ return xpcs_mmio_write_reg_direct(pxpcs, MDIO_MMD_VEND2, reg, val);
+}
+
+static int xpcs_mmio_read_c45(struct mii_bus *bus, int addr, int dev, int reg)
+{
+ struct dw_xpcs_plat *pxpcs = bus->priv;
+
+ if (addr != 0)
+ return -ENODEV;
+
+ if (pxpcs->reg_indir)
+ return xpcs_mmio_read_reg_indirect(pxpcs, dev, reg);
+ else
+ return xpcs_mmio_read_reg_direct(pxpcs, dev, reg);
+}
+
+static int xpcs_mmio_write_c45(struct mii_bus *bus, int addr, int dev,
+ int reg, u16 val)
+{
+ struct dw_xpcs_plat *pxpcs = bus->priv;
+
+ if (addr != 0)
+ return -ENODEV;
+
+ if (pxpcs->reg_indir)
+ return xpcs_mmio_write_reg_indirect(pxpcs, dev, reg, val);
+ else
+ return xpcs_mmio_write_reg_direct(pxpcs, dev, reg, val);
+}
+
+static struct dw_xpcs_plat *xpcs_plat_create_data(struct platform_device *pdev)
+{
+ struct dw_xpcs_plat *pxpcs;
+
+ pxpcs = devm_kzalloc(&pdev->dev, sizeof(*pxpcs), GFP_KERNEL);
+ if (!pxpcs)
+ return ERR_PTR(-ENOMEM);
+
+ pxpcs->pdev = pdev;
+
+ dev_set_drvdata(&pdev->dev, pxpcs);
+
+ return pxpcs;
+}
+
+static int xpcs_plat_init_res(struct dw_xpcs_plat *pxpcs)
+{
+ struct platform_device *pdev = pxpcs->pdev;
+ struct device *dev = &pdev->dev;
+ resource_size_t spc_size;
+ struct resource *res;
+
+ if (!device_property_read_u32(dev, "reg-io-width", &pxpcs->reg_width)) {
+ if (pxpcs->reg_width != 2 && pxpcs->reg_width != 4) {
+ dev_err(dev, "Invalid reg-space data width\n");
+ return -EINVAL;
+ }
+ } else {
+ pxpcs->reg_width = 2;
+ }
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "direct") ?:
+ platform_get_resource_byname(pdev, IORESOURCE_MEM, "indirect");
+ if (!res) {
+ dev_err(dev, "No reg-space found\n");
+ return -EINVAL;
+ }
+
+ if (!strcmp(res->name, "indirect"))
+ pxpcs->reg_indir = true;
+
+ if (pxpcs->reg_indir)
+ spc_size = pxpcs->reg_width * SZ_256;
+ else
+ spc_size = pxpcs->reg_width * SZ_2M;
+
+ if (resource_size(res) < spc_size) {
+ dev_err(dev, "Invalid reg-space size\n");
+ return -EINVAL;
+ }
+
+ pxpcs->reg_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(pxpcs->reg_base)) {
+ dev_err(dev, "Failed to map reg-space\n");
+ return PTR_ERR(pxpcs->reg_base);
+ }
+
+ return 0;
+}
+
+static int xpcs_plat_init_clk(struct dw_xpcs_plat *pxpcs)
+{
+ struct device *dev = &pxpcs->pdev->dev;
+ int ret;
+
+ pxpcs->pclk = devm_clk_get(dev, "pclk");
+ if (IS_ERR(pxpcs->pclk))
+ return dev_err_probe(dev, PTR_ERR(pxpcs->pclk),
+ "Failed to get ref clock\n");
+
+ pm_runtime_set_active(dev);
+ ret = devm_pm_runtime_enable(dev);
+ if (ret) {
+ dev_err(dev, "Failed to enable runtime-PM\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int xpcs_plat_init_bus(struct dw_xpcs_plat *pxpcs)
+{
+ struct device *dev = &pxpcs->pdev->dev;
+ static atomic_t id = ATOMIC_INIT(-1);
+ int ret;
+
+ pxpcs->bus = devm_mdiobus_alloc_size(dev, 0);
+ if (!pxpcs->bus)
+ return -ENOMEM;
+
+ pxpcs->bus->name = "DW XPCS MCI/APB3";
+ pxpcs->bus->read = xpcs_mmio_read_c22;
+ pxpcs->bus->write = xpcs_mmio_write_c22;
+ pxpcs->bus->read_c45 = xpcs_mmio_read_c45;
+ pxpcs->bus->write_c45 = xpcs_mmio_write_c45;
+ pxpcs->bus->phy_mask = ~0;
+ pxpcs->bus->parent = dev;
+ pxpcs->bus->priv = pxpcs;
+
+ snprintf(pxpcs->bus->id, MII_BUS_ID_SIZE,
+ "dwxpcs-%x", atomic_inc_return(&id));
+
+ /* MDIO-bus here serves as just a back-end engine abstracting out
+ * the MDIO and MCI/APB3 IO interfaces utilized for the DW XPCS CSRs
+ * access.
+ */
+ ret = devm_mdiobus_register(dev, pxpcs->bus);
+ if (ret) {
+ dev_err(dev, "Failed to create MDIO bus\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+/* Note there is no need in the next function antagonist because the MDIO-bus
+ * de-registration will effectively remove and destroy all the MDIO-devices
+ * registered on the bus.
+ */
+static int xpcs_plat_init_dev(struct dw_xpcs_plat *pxpcs)
+{
+ struct device *dev = &pxpcs->pdev->dev;
+ struct mdio_device *mdiodev;
+ int ret;
+
+ /* There is a single memory-mapped DW XPCS device */
+ mdiodev = mdio_device_create(pxpcs->bus, 0);
+ if (IS_ERR(mdiodev))
+ return PTR_ERR(mdiodev);
+
+ /* Associate the FW-node with the device structure so it can be looked
+ * up later. Make sure DD-core is aware of the OF-node being re-used.
+ */
+ device_set_node(&mdiodev->dev, fwnode_handle_get(dev_fwnode(dev)));
+ mdiodev->dev.of_node_reused = true;
+
+ /* Pass the data further so the DW XPCS driver core could use it */
+ mdiodev->dev.platform_data = (void *)device_get_match_data(dev);
+
+ ret = mdio_device_register(mdiodev);
+ if (ret) {
+ dev_err(dev, "Failed to register MDIO device\n");
+ goto err_clean_data;
+ }
+
+ return 0;
+
+err_clean_data:
+ mdiodev->dev.platform_data = NULL;
+
+ fwnode_handle_put(dev_fwnode(&mdiodev->dev));
+ device_set_node(&mdiodev->dev, NULL);
+
+ mdio_device_free(mdiodev);
+
+ return ret;
+}
+
+static int xpcs_plat_probe(struct platform_device *pdev)
+{
+ struct dw_xpcs_plat *pxpcs;
+ int ret;
+
+ pxpcs = xpcs_plat_create_data(pdev);
+ if (IS_ERR(pxpcs))
+ return PTR_ERR(pxpcs);
+
+ ret = xpcs_plat_init_res(pxpcs);
+ if (ret)
+ return ret;
+
+ ret = xpcs_plat_init_clk(pxpcs);
+ if (ret)
+ return ret;
+
+ ret = xpcs_plat_init_bus(pxpcs);
+ if (ret)
+ return ret;
+
+ ret = xpcs_plat_init_dev(pxpcs);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int __maybe_unused xpcs_plat_pm_runtime_suspend(struct device *dev)
+{
+ struct dw_xpcs_plat *pxpcs = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(pxpcs->pclk);
+
+ return 0;
+}
+
+static int __maybe_unused xpcs_plat_pm_runtime_resume(struct device *dev)
+{
+ struct dw_xpcs_plat *pxpcs = dev_get_drvdata(dev);
+
+ return clk_prepare_enable(pxpcs->pclk);
+}
+
+const struct dev_pm_ops xpcs_plat_pm_ops = {
+ SET_RUNTIME_PM_OPS(xpcs_plat_pm_runtime_suspend,
+ xpcs_plat_pm_runtime_resume,
+ NULL)
+};
+
+DW_XPCS_INFO_DECLARE(xpcs_generic, DW_XPCS_ID_NATIVE, DW_XPCS_PMA_ID_NATIVE);
+DW_XPCS_INFO_DECLARE(xpcs_pma_gen1_3g, DW_XPCS_ID_NATIVE, DW_XPCS_PMA_GEN1_3G_ID);
+DW_XPCS_INFO_DECLARE(xpcs_pma_gen2_3g, DW_XPCS_ID_NATIVE, DW_XPCS_PMA_GEN2_3G_ID);
+DW_XPCS_INFO_DECLARE(xpcs_pma_gen2_6g, DW_XPCS_ID_NATIVE, DW_XPCS_PMA_GEN2_6G_ID);
+DW_XPCS_INFO_DECLARE(xpcs_pma_gen4_3g, DW_XPCS_ID_NATIVE, DW_XPCS_PMA_GEN4_3G_ID);
+DW_XPCS_INFO_DECLARE(xpcs_pma_gen4_6g, DW_XPCS_ID_NATIVE, DW_XPCS_PMA_GEN4_6G_ID);
+DW_XPCS_INFO_DECLARE(xpcs_pma_gen5_10g, DW_XPCS_ID_NATIVE, DW_XPCS_PMA_GEN5_10G_ID);
+DW_XPCS_INFO_DECLARE(xpcs_pma_gen5_12g, DW_XPCS_ID_NATIVE, DW_XPCS_PMA_GEN5_12G_ID);
+
+static const struct of_device_id xpcs_of_ids[] = {
+ { .compatible = "snps,dw-xpcs", .data = &xpcs_generic },
+ { .compatible = "snps,dw-xpcs-gen1-3g", .data = &xpcs_pma_gen1_3g },
+ { .compatible = "snps,dw-xpcs-gen2-3g", .data = &xpcs_pma_gen2_3g },
+ { .compatible = "snps,dw-xpcs-gen2-6g", .data = &xpcs_pma_gen2_6g },
+ { .compatible = "snps,dw-xpcs-gen4-3g", .data = &xpcs_pma_gen4_3g },
+ { .compatible = "snps,dw-xpcs-gen4-6g", .data = &xpcs_pma_gen4_6g },
+ { .compatible = "snps,dw-xpcs-gen5-10g", .data = &xpcs_pma_gen5_10g },
+ { .compatible = "snps,dw-xpcs-gen5-12g", .data = &xpcs_pma_gen5_12g },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, xpcs_of_ids);
+
+static struct platform_driver xpcs_plat_driver = {
+ .probe = xpcs_plat_probe,
+ .driver = {
+ .name = "dwxpcs",
+ .pm = &xpcs_plat_pm_ops,
+ .of_match_table = xpcs_of_ids,
+ },
+};
+module_platform_driver(xpcs_plat_driver);
+
+MODULE_DESCRIPTION("Synopsys DesignWare XPCS platform device driver");
+MODULE_AUTHOR("Signed-off-by: Serge Semin <[email protected]>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index e8d5fd43a357..8f7e3af64fcc 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -6,6 +6,7 @@
* Author: Jose Abreu <[email protected]>
*/
+#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/pcs/pcs-xpcs.h>
#include <linux/mdio.h>
@@ -1244,7 +1245,9 @@ static int xpcs_get_id(struct dw_xpcs *xpcs)
id |= ret;
}
- xpcs->info.pcs = id;
+ /* Set the PCS ID if it hasn't been pre-initialized */
+ if (xpcs->info.pcs == DW_XPCS_ID_NATIVE)
+ xpcs->info.pcs = id;
/* Find out PMA/PMD ID from MMD 1 device ID registers */
ret = xpcs_read(xpcs, MDIO_MMD_PMAPMD, MDIO_DEVID1);
@@ -1263,7 +1266,9 @@ static int xpcs_get_id(struct dw_xpcs *xpcs)
ret = (ret >> 10) & 0x3F;
id |= ret << 16;
- xpcs->info.pma = id;
+ /* Set the PMA ID if it hasn't been pre-initialized */
+ if (xpcs->info.pma == DW_XPCS_PMA_ID_NATIVE)
+ xpcs->info.pma = id;
return 0;
}
@@ -1387,10 +1392,49 @@ static void xpcs_free_data(struct dw_xpcs *xpcs)
kfree(xpcs);
}
+static int xpcs_init_clks(struct dw_xpcs *xpcs)
+{
+ static const char *ids[DW_XPCS_NUM_CLKS] = {
+ [DW_XPCS_CORE_CLK] = "core",
+ [DW_XPCS_PAD_CLK] = "pad",
+ };
+ struct device *dev = &xpcs->mdiodev->dev;
+ int ret, i;
+
+ for (i = 0; i < DW_XPCS_NUM_CLKS; ++i)
+ xpcs->clks[i].id = ids[i];
+
+ ret = clk_bulk_get_optional(dev, DW_XPCS_NUM_CLKS, xpcs->clks);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to get clocks\n");
+
+ ret = clk_bulk_prepare_enable(DW_XPCS_NUM_CLKS, xpcs->clks);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to enable clocks\n");
+
+ return 0;
+}
+
+static void xpcs_clear_clks(struct dw_xpcs *xpcs)
+{
+ clk_bulk_disable_unprepare(DW_XPCS_NUM_CLKS, xpcs->clks);
+
+ clk_bulk_put(DW_XPCS_NUM_CLKS, xpcs->clks);
+}
+
static int xpcs_init_id(struct dw_xpcs *xpcs)
{
+ const struct dw_xpcs_info *info;
int i, ret;
+ info = dev_get_platdata(&xpcs->mdiodev->dev);
+ if (!info) {
+ xpcs->info.pcs = DW_XPCS_ID_NATIVE;
+ xpcs->info.pma = DW_XPCS_PMA_ID_NATIVE;
+ } else {
+ xpcs->info = *info;
+ }
+
ret = xpcs_get_id(xpcs);
if (ret < 0)
return ret;
@@ -1438,17 +1482,24 @@ static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
if (IS_ERR(xpcs))
return xpcs;
+ ret = xpcs_init_clks(xpcs);
+ if (ret)
+ goto out_free_data;
+
ret = xpcs_init_id(xpcs);
if (ret)
- goto out;
+ goto out_clear_clks;
ret = xpcs_init_iface(xpcs, interface);
if (ret)
- goto out;
+ goto out_clear_clks;
return xpcs;
-out:
+out_clear_clks:
+ xpcs_clear_clks(xpcs);
+
+out_free_data:
xpcs_free_data(xpcs);
return ERR_PTR(ret);
@@ -1483,6 +1534,8 @@ void xpcs_destroy(struct dw_xpcs *xpcs)
if (!xpcs)
return;
+ xpcs_clear_clks(xpcs);
+
xpcs_free_data(xpcs);
}
EXPORT_SYMBOL_GPL(xpcs_destroy);
diff --git a/drivers/net/pcs/pcs-xpcs.h b/drivers/net/pcs/pcs-xpcs.h
index 369e9196f45a..fa05adfae220 100644
--- a/drivers/net/pcs/pcs-xpcs.h
+++ b/drivers/net/pcs/pcs-xpcs.h
@@ -6,6 +6,9 @@
* Author: Jose Abreu <[email protected]>
*/
+#include <linux/bits.h>
+#include <linux/pcs/pcs-xpcs.h>
+
/* Vendor regs access */
#define DW_VENDOR BIT(15)
@@ -117,6 +120,9 @@
/* VR MII EEE Control 1 defines */
#define DW_VR_MII_EEE_TRN_LPI BIT(0) /* Transparent Mode Enable */
+#define DW_XPCS_INFO_DECLARE(_name, _pcs, _pma) \
+ static const struct dw_xpcs_info _name = { .pcs = _pcs, .pma = _pma }
+
int xpcs_read(struct dw_xpcs *xpcs, int dev, u32 reg);
int xpcs_write(struct dw_xpcs *xpcs, int dev, u32 reg, u16 val);
int xpcs_read_vpcs(struct dw_xpcs *xpcs, int reg);
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index 1dc60f5e653f..813be644647f 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -7,6 +7,8 @@
#ifndef __LINUX_PCS_XPCS_H
#define __LINUX_PCS_XPCS_H
+#include <linux/clk.h>
+#include <linux/mdio.h>
#include <linux/phy.h>
#include <linux/phylink.h>
#include <linux/types.h>
@@ -21,6 +23,7 @@
struct dw_xpcs_desc;
enum dw_xpcs_pcs_id {
+ DW_XPCS_ID_NATIVE = 0,
NXP_SJA1105_XPCS_ID = 0x00000010,
NXP_SJA1110_XPCS_ID = 0x00000020,
DW_XPCS_ID = 0x7996ced0,
@@ -28,6 +31,14 @@ enum dw_xpcs_pcs_id {
};
enum dw_xpcs_pma_id {
+ DW_XPCS_PMA_ID_NATIVE = 0,
+ DW_XPCS_PMA_GEN1_3G_ID,
+ DW_XPCS_PMA_GEN2_3G_ID,
+ DW_XPCS_PMA_GEN2_6G_ID,
+ DW_XPCS_PMA_GEN4_3G_ID,
+ DW_XPCS_PMA_GEN4_6G_ID,
+ DW_XPCS_PMA_GEN5_10G_ID,
+ DW_XPCS_PMA_GEN5_12G_ID,
WX_TXGBE_XPCS_PMA_10G_ID = 0x0018fc80,
};
@@ -36,10 +47,17 @@ struct dw_xpcs_info {
u32 pma;
};
+enum dw_xpcs_clock {
+ DW_XPCS_CORE_CLK,
+ DW_XPCS_PAD_CLK,
+ DW_XPCS_NUM_CLKS,
+};
+
struct dw_xpcs {
struct dw_xpcs_info info;
const struct dw_xpcs_desc *desc;
struct mdio_device *mdiodev;
+ struct clk_bulk_data clks[DW_XPCS_NUM_CLKS];
struct phylink_pcs pcs;
phy_interface_t interface;
};
--
2.43.0
Currently the only STMMAC platform driver using the DW XPCS code is the
Intel mGBE device driver. (It can be determined by finding all the drivers
having the stmmac_mdio_bus_data::has_xpcs flag set.) At the same time the
low-level platform driver masks out the DW XPCS MDIO-address from being
auto-detected as PHY by the MDIO subsystem core. Seeing the PCS MDIO ID is
known the procedure of the DW XPCS device creation can be simplified by
dropping the loop over all the MDIO IDs. From now the DW XPCS device
descriptor will be created for the pre-defined MDIO-bus address.
Note besides this shall speed up a bit the Intel mGBE probing.
Signed-off-by: Serge Semin <[email protected]>
---
Changelog v2:
- This is a new patch introduced on v2 stage of the review.
---
drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c | 1 +
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 15 ++++-----------
include/linux/stmmac.h | 1 +
3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
index 56649edb18cd..e60b7e955c35 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -586,6 +586,7 @@ static int intel_mgbe_common_data(struct pci_dev *pdev,
if (plat->phy_interface == PHY_INTERFACE_MODE_SGMII ||
plat->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
plat->mdio_bus_data->has_xpcs = true;
+ plat->mdio_bus_data->xpcs_addr = INTEL_MGBE_XPCS_ADDR;
plat->mdio_bus_data->default_an_inband = true;
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index aa43117134d3..807789d7309a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -499,8 +499,7 @@ int stmmac_pcs_setup(struct net_device *ndev)
{
struct dw_xpcs *xpcs = NULL;
struct stmmac_priv *priv;
- int ret = -ENODEV;
- int mode, addr;
+ int addr, mode, ret;
priv = netdev_priv(ndev);
mode = priv->plat->phy_interface;
@@ -509,15 +508,9 @@ int stmmac_pcs_setup(struct net_device *ndev)
ret = priv->plat->pcs_init(priv);
} else if (priv->plat->mdio_bus_data &&
priv->plat->mdio_bus_data->has_xpcs) {
- /* Try to probe the XPCS by scanning all addresses */
- for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
- xpcs = xpcs_create_mdiodev(priv->mii, addr, mode);
- if (IS_ERR(xpcs))
- continue;
-
- ret = 0;
- break;
- }
+ addr = priv->plat->mdio_bus_data->xpcs_addr;
+ xpcs = xpcs_create_mdiodev(priv->mii, addr, mode);
+ ret = PTR_ERR_OR_ZERO(xpcs);
} else {
return 0;
}
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index f337286623bb..a11b850d3672 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -83,6 +83,7 @@ struct stmmac_priv;
struct stmmac_mdio_bus_data {
unsigned int phy_mask;
unsigned int has_xpcs;
+ unsigned int xpcs_addr;
unsigned int default_an_inband;
int *irqs;
int probed_phy_irq;
--
2.43.0
It's now possible to have the DW XPCS device defined as a standard
platform device for instance in the platform DT-file. Although that
functionality is useless unless there is a way to have the device found by
the client drivers (STMMAC/DW *MAC, NXP SJA1105 Eth Switch, etc). Provide
such ability by means of the xpcs_create_fwnode() method. It needs to be
called with the device DW XPCS fwnode instance passed. That node will be
then used to find the MDIO-device instance in order to create the DW XPCS
descriptor.
Note the method semantics and name is similar to what has been recently
introduced in the Lynx PCS driver.
Signed-off-by: Serge Semin <[email protected]>
---
Changelog v2:
- Use the function name and semantics similar to the Lynx PCS driver.
- Add kdoc describing the DW XPCS create functions.
---
drivers/net/pcs/pcs-xpcs.c | 50 ++++++++++++++++++++++++++++++++++++
include/linux/pcs/pcs-xpcs.h | 3 +++
2 files changed, 53 insertions(+)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 8f7e3af64fcc..d45fa6514884 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -10,7 +10,9 @@
#include <linux/delay.h>
#include <linux/pcs/pcs-xpcs.h>
#include <linux/mdio.h>
+#include <linux/phy.h>
#include <linux/phylink.h>
+#include <linux/property.h>
#include "pcs-xpcs.h"
@@ -1505,6 +1507,16 @@ static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
return ERR_PTR(ret);
}
+/**
+ * xpcs_create_mdiodev() - create a DW xPCS instance with the MDIO @addr
+ * @bus: pointer to the MDIO-bus descriptor for the device to be looked at
+ * @addr: device MDIO-bus ID
+ * @requested PHY interface
+ *
+ * If successful, returns a pointer to the DW XPCS handle. Otherwise returns
+ * -ENODEV if device couldn't be found on the bus, other negative errno related
+ * to the data allocation and MDIO-bus communications.
+ */
struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
phy_interface_t interface)
{
@@ -1529,6 +1541,44 @@ struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
}
EXPORT_SYMBOL_GPL(xpcs_create_mdiodev);
+/**
+ * xpcs_create_fwnode() - Create a DW xPCS instance from @fwnode
+ * @node: fwnode handle poining to the DW XPCS device
+ * @interface: requested PHY interface
+ *
+ * If successful, returns a pointer to the DW XPCS handle. Otherwise returns
+ * -ENODEV if the fwnode is marked unavailable or device couldn't be found on
+ * the bus, -EPROBE_DEFER if the respective MDIO-device instance couldn't be
+ * found, other negative errno related to the data allocations and MDIO-bus
+ * communications.
+ */
+struct dw_xpcs *xpcs_create_fwnode(struct fwnode_handle *fwnode,
+ phy_interface_t interface)
+{
+ struct mdio_device *mdiodev;
+ struct dw_xpcs *xpcs;
+
+ if (!fwnode_device_is_available(fwnode))
+ return ERR_PTR(-ENODEV);
+
+ mdiodev = fwnode_mdio_find_device(fwnode);
+ if (!mdiodev)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ xpcs = xpcs_create(mdiodev, interface);
+
+ /* xpcs_create() has taken a refcount on the mdiodev if it was
+ * successful. If xpcs_create() fails, this will free the mdio
+ * device here. In any case, we don't need to hold our reference
+ * anymore, and putting it here will allow mdio_device_put() in
+ * xpcs_destroy() to automatically free the mdio device.
+ */
+ mdio_device_put(mdiodev);
+
+ return xpcs;
+}
+EXPORT_SYMBOL_GPL(xpcs_create_fwnode);
+
void xpcs_destroy(struct dw_xpcs *xpcs)
{
if (!xpcs)
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index 813be644647f..b4a4eb6c8866 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -8,6 +8,7 @@
#define __LINUX_PCS_XPCS_H
#include <linux/clk.h>
+#include <linux/fwnode.h>
#include <linux/mdio.h>
#include <linux/phy.h>
#include <linux/phylink.h>
@@ -72,6 +73,8 @@ int xpcs_config_eee(struct dw_xpcs *xpcs, int mult_fact_100ns,
int enable);
struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
phy_interface_t interface);
+struct dw_xpcs *xpcs_create_fwnode(struct fwnode_handle *fwnode,
+ phy_interface_t interface);
void xpcs_destroy(struct dw_xpcs *xpcs);
#endif /* __LINUX_PCS_XPCS_H */
--
2.43.0
Recently the DW XPCS DT-bindings have been introduced and the DW XPCS
driver has been altered to support the DW XPCS registered as a platform
device. In order to have the DW XPCS DT-device accessed from the STMMAC
driver let's alter the STMMAC PCS-setup procedure to support the
"pcs-handle" property containing the phandle reference to the DW XPCS
device DT-node. The respective fwnode will be then passed to the
xpcs_create_fwnode() function which in its turn will create the DW XPCS
descriptor utilized in the main driver for the PCS-related setups.
Signed-off-by: Serge Semin <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index 807789d7309a..dc040051aa53 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -497,15 +497,22 @@ int stmmac_mdio_reset(struct mii_bus *bus)
int stmmac_pcs_setup(struct net_device *ndev)
{
+ struct fwnode_handle *devnode, *pcsnode;
struct dw_xpcs *xpcs = NULL;
struct stmmac_priv *priv;
int addr, mode, ret;
priv = netdev_priv(ndev);
mode = priv->plat->phy_interface;
+ devnode = priv->plat->port_node;
if (priv->plat->pcs_init) {
ret = priv->plat->pcs_init(priv);
+ } else if (fwnode_property_present(devnode, "pcs-handle")) {
+ pcsnode = fwnode_find_reference(devnode, "pcs-handle", 0);
+ xpcs = xpcs_create_fwnode(pcsnode, mode);
+ fwnode_handle_put(pcsnode);
+ ret = PTR_ERR_OR_ZERO(xpcs);
} else if (priv->plat->mdio_bus_data &&
priv->plat->mdio_bus_data->has_xpcs) {
addr = priv->plat->mdio_bus_data->xpcs_addr;
@@ -515,10 +522,8 @@ int stmmac_pcs_setup(struct net_device *ndev)
return 0;
}
- if (ret) {
- dev_warn(priv->device, "No xPCS found\n");
- return ret;
- }
+ if (ret)
+ return dev_err_probe(priv->device, ret, "No xPCS found\n");
priv->hw->xpcs = xpcs;
--
2.43.0
On Sun, Jun 02, 2024 at 05:36:24PM +0300, Serge Semin wrote:
> Recently the DW XPCS DT-bindings have been introduced and the DW XPCS
> driver has been altered to support the DW XPCS registered as a platform
> device. In order to have the DW XPCS DT-device accessed from the STMMAC
> driver let's alter the STMMAC PCS-setup procedure to support the
> "pcs-handle" property containing the phandle reference to the DW XPCS
> device DT-node. The respective fwnode will be then passed to the
> xpcs_create_fwnode() function which in its turn will create the DW XPCS
> descriptor utilized in the main driver for the PCS-related setups.
>
> Signed-off-by: Serge Semin <[email protected]>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> index 807789d7309a..dc040051aa53 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> @@ -497,15 +497,22 @@ int stmmac_mdio_reset(struct mii_bus *bus)
>
> int stmmac_pcs_setup(struct net_device *ndev)
> {
> + struct fwnode_handle *devnode, *pcsnode;
> struct dw_xpcs *xpcs = NULL;
> struct stmmac_priv *priv;
> int addr, mode, ret;
>
> priv = netdev_priv(ndev);
> mode = priv->plat->phy_interface;
> + devnode = priv->plat->port_node;
>
> if (priv->plat->pcs_init) {
> ret = priv->plat->pcs_init(priv);
> + } else if (fwnode_property_present(devnode, "pcs-handle")) {
> + pcsnode = fwnode_find_reference(devnode, "pcs-handle", 0);
> + xpcs = xpcs_create_fwnode(pcsnode, mode);
> + fwnode_handle_put(pcsnode);
> + ret = PTR_ERR_OR_ZERO(xpcs);
Just figured, we might wish to be a bit more portable in the
"pcs-handle" property semantics implementation seeing there can be at
least three different PCS attached:
DW XPCS
Lynx PCS
Renesas RZ/N1 MII
Any suggestion of how to distinguish the passed handle? Perhaps
named-property, phandle argument, by the compatible string or the
node-name?
-Serge(y)
> } else if (priv->plat->mdio_bus_data &&
> priv->plat->mdio_bus_data->has_xpcs) {
> addr = priv->plat->mdio_bus_data->xpcs_addr;
> @@ -515,10 +522,8 @@ int stmmac_pcs_setup(struct net_device *ndev)
> return 0;
> }
>
> - if (ret) {
> - dev_warn(priv->device, "No xPCS found\n");
> - return ret;
> - }
> + if (ret)
> + return dev_err_probe(priv->device, ret, "No xPCS found\n");
>
> priv->hw->xpcs = xpcs;
>
> --
> 2.43.0
>
On Mon, Jun 03, 2024 at 11:54:22AM +0300, Serge Semin wrote:
> > if (priv->plat->pcs_init) {
> > ret = priv->plat->pcs_init(priv);
>
> > + } else if (fwnode_property_present(devnode, "pcs-handle")) {
> > + pcsnode = fwnode_find_reference(devnode, "pcs-handle", 0);
> > + xpcs = xpcs_create_fwnode(pcsnode, mode);
> > + fwnode_handle_put(pcsnode);
> > + ret = PTR_ERR_OR_ZERO(xpcs);
>
> Just figured, we might wish to be a bit more portable in the
> "pcs-handle" property semantics implementation seeing there can be at
> least three different PCS attached:
> DW XPCS
> Lynx PCS
> Renesas RZ/N1 MII
>
> Any suggestion of how to distinguish the passed handle? Perhaps
> named-property, phandle argument, by the compatible string or the
> node-name?
I can't think of a reasonable solution to this at the moment. One
solution could be pushing this down into the platform code to deal
with as an interim solution, via the new .pcs_init() method.
We could also do that with the current XPCS code, since we know that
only Intel mGBE uses xpcs. This would probably allow us to get rid
of the has_xpcs flag.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Mon, Jun 03, 2024 at 10:03:54AM +0100, Russell King (Oracle) wrote:
> On Mon, Jun 03, 2024 at 11:54:22AM +0300, Serge Semin wrote:
> > > if (priv->plat->pcs_init) {
> > > ret = priv->plat->pcs_init(priv);
> >
> > > + } else if (fwnode_property_present(devnode, "pcs-handle")) {
> > > + pcsnode = fwnode_find_reference(devnode, "pcs-handle", 0);
> > > + xpcs = xpcs_create_fwnode(pcsnode, mode);
> > > + fwnode_handle_put(pcsnode);
> > > + ret = PTR_ERR_OR_ZERO(xpcs);
> >
> > Just figured, we might wish to be a bit more portable in the
> > "pcs-handle" property semantics implementation seeing there can be at
> > least three different PCS attached:
> > DW XPCS
> > Lynx PCS
> > Renesas RZ/N1 MII
> >
> > Any suggestion of how to distinguish the passed handle? Perhaps
> > named-property, phandle argument, by the compatible string or the
> > node-name?
>
> I can't think of a reasonable solution to this at the moment. One
> solution could be pushing this down into the platform code to deal
> with as an interim solution, via the new .pcs_init() method.
>
> We could also do that with the current XPCS code, since we know that
> only Intel mGBE uses xpcs. This would probably allow us to get rid
> of the has_xpcs flag.
Basically you suggest to move the entire stmmac_pcs_setup() to the
platforms, don't you? The patch 9 of this series indeed could have
been converted to just moving the entire PCS-detection loop from
stmmac_pcs_setup() to the Intel-specific pcs_init.
But IMO some default/generic code would be still useful to preserve in
the stmmac_pcs_setup() method. When it comes to the fwnode-based
platform we at least could be falling back to the default DW XPCS
device registration if no plat_stmmacenet_data::pcs_init() callback
was specified and there was the "pcs-handle" property found,
especially seeing DW *MAC and DW XPCS are of the same vendor.
Based on that I can convert patch 9 of this series to introducing the
pcs_init() callback in the Intel mGBE driver, but preserve the
semantics of the rest of the series changes.
-Serge(y)
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Tue, Jun 04, 2024 at 12:04:57PM +0300, Serge Semin wrote:
> On Mon, Jun 03, 2024 at 10:03:54AM +0100, Russell King (Oracle) wrote:
> > I can't think of a reasonable solution to this at the moment. One
> > solution could be pushing this down into the platform code to deal
> > with as an interim solution, via the new .pcs_init() method.
> >
> > We could also do that with the current XPCS code, since we know that
> > only Intel mGBE uses xpcs. This would probably allow us to get rid
> > of the has_xpcs flag.
>
> Basically you suggest to move the entire stmmac_pcs_setup() to the
> platforms, don't you? The patch 9 of this series indeed could have
> been converted to just moving the entire PCS-detection loop from
> stmmac_pcs_setup() to the Intel-specific pcs_init.
Yes, it's not like XPCS is used by more than one platform, it's only
Intel mGBE. So I don't see why it should have a privileged position
over any other PCS implementation that stmmac supports (there's now
three different PCS.)
If you don't want the code in the Intel driver, then what could be
done is provide a core implementation that gets hooked into the
.pcs_init() method.
The same is probably true of other PCSes if they end up being shared
across several different platforms.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Tue, Jun 04, 2024 at 10:29:40AM +0100, Russell King (Oracle) wrote:
> On Tue, Jun 04, 2024 at 12:04:57PM +0300, Serge Semin wrote:
> > On Mon, Jun 03, 2024 at 10:03:54AM +0100, Russell King (Oracle) wrote:
> > > I can't think of a reasonable solution to this at the moment. One
> > > solution could be pushing this down into the platform code to deal
> > > with as an interim solution, via the new .pcs_init() method.
> > >
> > > We could also do that with the current XPCS code, since we know that
> > > only Intel mGBE uses xpcs. This would probably allow us to get rid
> > > of the has_xpcs flag.
> >
> > Basically you suggest to move the entire stmmac_pcs_setup() to the
> > platforms, don't you? The patch 9 of this series indeed could have
> > been converted to just moving the entire PCS-detection loop from
> > stmmac_pcs_setup() to the Intel-specific pcs_init.
>
> Yes, it's not like XPCS is used by more than one platform, it's only
> Intel mGBE. So I don't see why it should have a privileged position
> over any other PCS implementation that stmmac supports (there's now
> three different PCS.)
>
Alas DW XPCS has already got a more privileged position. The STMMAC
driver calls the XPCS driver methods here and there (supported ifaces,
EEE or PHY setup). Unless these calls are converted to some
standard/new phylink_pcs calls IMO it would be better to preserve the
default DW XPCS init at least for the "pcs-handle" property to
motivate the platform drivers developers to follow some pre-defined
device description pattern (e.g. defining DW XPCS devices in device
tree), but leave the .pcs_init() for some platform-specific PCS inits
(including which have already been implemented).
As I already mentioned DW XPCS is of Synopsys vendor. The IP-core has
been invented to provide a bridge between the Synopsys MAC IP-cores
and PMA (mainly Synopsys PMAs) for the 1G/10G links like 1000Base-X,
and 10GBase-X/-R/-KX4/-KR. The reason we see just a single use-case
of the XPCS in the driver is that even though the STMMAC driver has DW
XGMAC support the driver is mainly utilized for the 1G MACs (I don't
see any platform currently having DW XGMAC defined). Since DW GMAC/QoS
Eth can be configured to have the standard PHY interfaces available
there is no need in XPCS in these cases (except a weird Intel mGBE).
But when it comes to DW XGMAC it can be synthesized with GMII and XGMII
interfaces only. These're exactly interfaces which DW XPCS supports on
upstream. Thus basically the DW XPCS IP-core has been mainly produced
for been utilized in a couple with DW XGMAC providing a ready-to-use
solution for the XFP/SFP(+) ports or backplane-based applications. So
should we have more DW XGMACs supported in the kernel we would have met
more DW XPCS defined in there too.
> If you don't want the code in the Intel driver, then what could be
> done is provide a core implementation that gets hooked into the
> .pcs_init() method.
I don't mind converting patch 9 to moving the XPCS registration in the
Intel-specific .pcs_init() (especially seeing it's just a single
xpcs_create_mdiodev() call), but having the "pcs-handle" property
handled generically in the STMMAC core would be a useful thing to have
(see my reasoning above).
-Serge(y)
>
> The same is probably true of other PCSes if they end up being shared
> across several different platforms.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Sun, Jun 02, 2024 at 05:36:21PM +0300, Serge Semin wrote:
...
> diff --git a/drivers/net/pcs/pcs-xpcs-plat.c b/drivers/net/pcs/pcs-xpcs-plat.c
...
> +const struct dev_pm_ops xpcs_plat_pm_ops = {
> + SET_RUNTIME_PM_OPS(xpcs_plat_pm_runtime_suspend,
> + xpcs_plat_pm_runtime_resume,
> + NULL)
> +};
nit: xpcs_plat_pm_ops only seems to be used in this file.
If so it should probably be static.
Flagged by Sparse.
...
> +static struct platform_driver xpcs_plat_driver = {
> + .probe = xpcs_plat_probe,
> + .driver = {
> + .name = "dwxpcs",
> + .pm = &xpcs_plat_pm_ops,
> + .of_match_table = xpcs_of_ids,
> + },
> +};
> +module_platform_driver(xpcs_plat_driver);
...
On Sun, Jun 02, 2024 at 05:36:22PM +0300, Serge Semin wrote:
> It's now possible to have the DW XPCS device defined as a standard
> platform device for instance in the platform DT-file. Although that
> functionality is useless unless there is a way to have the device found by
> the client drivers (STMMAC/DW *MAC, NXP SJA1105 Eth Switch, etc). Provide
> such ability by means of the xpcs_create_fwnode() method. It needs to be
> called with the device DW XPCS fwnode instance passed. That node will be
> then used to find the MDIO-device instance in order to create the DW XPCS
> descriptor.
>
> Note the method semantics and name is similar to what has been recently
> introduced in the Lynx PCS driver.
>
> Signed-off-by: Serge Semin <[email protected]>
Hi Serge,
Some minor nits from my side flagged by kernel-doc -none -Wall
...
> diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
...
> @@ -1505,6 +1507,16 @@ static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
> return ERR_PTR(ret);
> }
>
> +/**
> + * xpcs_create_mdiodev() - create a DW xPCS instance with the MDIO @addr
> + * @bus: pointer to the MDIO-bus descriptor for the device to be looked at
> + * @addr: device MDIO-bus ID
> + * @requested PHY interface
An entry for @interface should go here.
> + *
> + * If successful, returns a pointer to the DW XPCS handle. Otherwise returns
> + * -ENODEV if device couldn't be found on the bus, other negative errno related
> + * to the data allocation and MDIO-bus communications.
Please consider including this information as a Return: section of the
Kernel doc. Likewise for xpcs_create_fwnode().
> + */
> struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
> phy_interface_t interface)
> {
> @@ -1529,6 +1541,44 @@ struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
> }
> EXPORT_SYMBOL_GPL(xpcs_create_mdiodev);
>
> +/**
> + * xpcs_create_fwnode() - Create a DW xPCS instance from @fwnode
> + * @node: fwnode handle poining to the DW XPCS device
s/@node/@fwnode/
> + * @interface: requested PHY interface
> + *
> + * If successful, returns a pointer to the DW XPCS handle. Otherwise returns
> + * -ENODEV if the fwnode is marked unavailable or device couldn't be found on
> + * the bus, -EPROBE_DEFER if the respective MDIO-device instance couldn't be
> + * found, other negative errno related to the data allocations and MDIO-bus
> + * communications.
> + */
> +struct dw_xpcs *xpcs_create_fwnode(struct fwnode_handle *fwnode,
> + phy_interface_t interface)
...
> @@ -482,7 +482,7 @@ static int xpcs_config_aneg_c73(struct dw_xpcs *xpcs,
>
> static int xpcs_aneg_done_c73(struct dw_xpcs *xpcs,
> struct phylink_link_state *state,
> - const struct xpcs_compat *compat, u16 an_stat1)
> + const struct dw_xpcs_compat *compat, u16 an_stat1)
> {
> int ret;
>
> @@ -607,7 +607,7 @@ static int xpcs_validate(struct phylink_pcs *pcs, unsigned long *supported,
> const struct phylink_link_state *state)
> {
> __ETHTOOL_DECLARE_LINK_MODE_MASK(xpcs_supported) = { 0, };
> - const struct xpcs_compat *compat;
> + const struct dw_xpcs_compat *compat;
> struct dw_xpcs *xpcs;
> int i;
>
> @@ -633,7 +633,7 @@ void xpcs_get_interfaces(struct dw_xpcs *xpcs, unsigned long *interfaces)
> int i, j;
>
> for (i = 0; i < DW_XPCS_INTERFACE_MAX; i++) {
> - const struct xpcs_compat *compat = &xpcs->desc->compat[i];
> + const struct dw_xpcs_compat *compat = &xpcs->desc->compat[i];
>
> for (j = 0; j < compat->num_interfaces; j++)
> __set_bit(compat->interface[j], interfaces);
> @@ -850,7 +850,7 @@ static int xpcs_config_2500basex(struct dw_xpcs *xpcs)
> int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
> const unsigned long *advertising, unsigned int neg_mode)
> {
> - const struct xpcs_compat *compat;
> + const struct dw_xpcs_compat *compat;
> int ret;
>
> compat = xpcs_find_compat(xpcs->desc, interface);
> @@ -915,7 +915,7 @@ static int xpcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
>
> static int xpcs_get_state_c73(struct dw_xpcs *xpcs,
> struct phylink_link_state *state,
> - const struct xpcs_compat *compat)
> + const struct dw_xpcs_compat *compat)
> {
> bool an_enabled;
> int pcs_stat1;
> @@ -1115,7 +1115,7 @@ static void xpcs_get_state(struct phylink_pcs *pcs,
> struct phylink_link_state *state)
> {
> struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs);
> - const struct xpcs_compat *compat;
> + const struct dw_xpcs_compat *compat;
> int ret;
>
> compat = xpcs_find_compat(xpcs->desc, state->interface);
> @@ -1269,7 +1269,7 @@ static u32 xpcs_get_id(struct dw_xpcs *xpcs)
> return 0xffffffff;
> }
>
> -static const struct xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> +static const struct dw_xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> [DW_XPCS_USXGMII] = {
> .supported = xpcs_usxgmii_features,
> .interface = xpcs_usxgmii_interfaces,
> @@ -1314,7 +1314,7 @@ static const struct xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> },
> };
>
Serge, Thank you for raising these patches. Minor comments which shows warning on my workspace.
WARNING: line length of 82 exceeds 80 columns
#153: FILE: drivers/net/pcs/pcs-xpcs.c:1272:
+static const struct dw_xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
WARNING: line length of 85 exceeds 80 columns
#162: FILE: drivers/net/pcs/pcs-xpcs.c:1317:
+static const struct dw_xpcs_compat nxp_sja1105_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
WARNING: line length of 85 exceeds 80 columns
#171: FILE: drivers/net/pcs/pcs-xpcs.c:1327:
+static const struct dw_xpcs_compat nxp_sja1110_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> -static const struct xpcs_compat nxp_sja1105_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> +static const struct dw_xpcs_compat nxp_sja1105_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> [DW_XPCS_SGMII] = {
> .supported = xpcs_sgmii_features,
> .interface = xpcs_sgmii_interfaces,
> @@ -1324,7 +1324,7 @@ static const struct xpcs_compat nxp_sja1105_xpcs_compat[DW_XPCS_INTERFACE_MAX] =
> },
> };
>
> -static const struct xpcs_compat nxp_sja1110_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> +static const struct dw_xpcs_compat nxp_sja1110_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> [DW_XPCS_SGMII] = {
> .supported = xpcs_sgmii_features,
> .interface = xpcs_sgmii_interfaces,
> @@ -1418,7 +1418,7 @@ static int xpcs_init_id(struct dw_xpcs *xpcs)
>
> static int xpcs_init_iface(struct dw_xpcs *xpcs, phy_interface_t interface)
> {
> - const struct xpcs_compat *compat;
> + const struct dw_xpcs_compat *compat;
>
> compat = xpcs_find_compat(xpcs->desc, interface);
> if (!compat)
On Sun, Jun 02, 2024 at 05:36:20PM +0300, Serge Semin wrote:
> Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
> providing an interface between the Media Access Control (MAC) and Physical
> Medium Attachment Sublayer (PMA) through a Media independent interface.
> >From software point of view it exposes IEEE std. Clause 45 CSR space and
> can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
> case the PCS device is supposed to be defined under the respective MDIO
> bus DT-node. In the later case the DW xPCS will be just a normal IO
> memory-mapped device.
>
> Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
> source properties specified. The former one indicates the Clause 73/37
> auto-negotiation events like: negotiation page received, AN is completed
> or incompatible link partner. The clock DT-properties can describe up to
> three clock sources: peripheral bus clock source, internal reference clock
> and the externally connected reference clock.
>
> Finally the DW XPCS IP-core can be optionally synthesized with a
> vendor-specific interface connected to the Synopsys PMA (also called
> DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
> portable way. So if the DW XPCS device has the respective PMA attached
> then it should be reflected in the DT-node compatible string so the driver
> would be aware of the PMA-specific device capabilities (mainly connected
> with CSRs available for the fine-tunings).
>
> Signed-off-by: Serge Semin <[email protected]>
>
> ---
>
> Changelog v2:
> - Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
> interface is just a normal memory-mapped device.
> ---
> .../bindings/net/pcs/snps,dw-xpcs.yaml | 133 ++++++++++++++++++
> 1 file changed, 133 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> new file mode 100644
> index 000000000000..7927bceefbf3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> @@ -0,0 +1,133 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DesignWare Ethernet PCS
> +
> +maintainers:
> + - Serge Semin <[email protected]>
> +
> +description:
> + Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
> + between Media Access Control and Physical Medium Attachment Sublayer through
> + the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
> + controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
> + optionally synthesized with a vendor-specific interface connected to
> + Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
> + general it can be used to communicate with any compatible PHY.
> +
> + The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
> + by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
> + right to the system IO memory space.
> +
> +properties:
> + compatible:
> + oneOf:
> + - description: Synopsys DesignWare XPCS with none or unknown PMA
> + const: snps,dw-xpcs
> + - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
> + const: snps,dw-xpcs-gen1-3g
> + - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
> + const: snps,dw-xpcs-gen2-3g
> + - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
> + const: snps,dw-xpcs-gen2-6g
> + - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
> + const: snps,dw-xpcs-gen4-3g
> + - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
> + const: snps,dw-xpcs-gen4-6g
> + - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
> + const: snps,dw-xpcs-gen5-10g
> + - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
> + const: snps,dw-xpcs-gen5-12g
> +
> + reg:
> + items:
> + - description:
> + In case of the MDIO management interface this just a 5-bits ID
> + of the MDIO bus device. If DW XPCS CSRs space is accessed over the
> + MCI or APB3 management interfaces, then the space mapping can be
> + either 'direct' or 'indirect'. In the former case all Clause 45
> + registers are contiguously mapped within the address space
> + MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
> + to the multiple 256 register sets. There is a special viewport CSR
> + which is responsible for the set selection. The upper part of
> + the CSR address MMD+REG[20:8] is supposed to be written in there
> + so the corresponding subset would be mapped to the lowest 255 CSRs.
> +
> + reg-names:
> + items:
> + - enum: [ direct, indirect ]
> +
> + reg-io-width:
> + description:
> + The way the CSRs are mapped to the memory is platform depended. Since
> + each Clause 45 CSR is of 16-bits wide the access instructions must be
> + two bytes aligned at least.
> + default: 2
> + enum: [ 2, 4 ]
> +
> + interrupts:
> + description:
> + System interface interrupt output (sbd_intr_o) indicating Clause 73/37
> + auto-negotiation events':' Page received, AN is completed or incompatible
> + link partner.
> + maxItems: 1
> +
> + clocks:
> + description:
> + Both MCI and APB3 interfaces are supposed to be equipped with a clock
> + source connected via the clk_csr_i line.
> +
> + PCS/PMA layer can be clocked by an internal reference clock source
> + (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
> + generator. Both clocks can be supplied at a time.
> + minItems: 1
> + maxItems: 3
> +
> + clock-names:
> + minItems: 1
> + maxItems: 3
> + anyOf:
> + - items:
> + enum: [ core, pad ]
This has no effect. If it is true, then the 2nd entry is too.
You are saying all the clocks are optional and any combination/order is
valid. Do we really need it so flexible? Doubtful the h/w is that
flexible.
> + - items:
> + enum: [ pclk, core, pad ]
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + ethernet-pcs@1f05d000 {
> + compatible = "snps,dw-xpcs";
> + reg = <0x1f05d000 0x1000>;
> + reg-names = "indirect";
> +
> + reg-io-width = <4>;
> +
> + interrupts = <79 IRQ_TYPE_LEVEL_HIGH>;
> +
> + clocks = <&ccu_pclk>, <&ccu_core>, <&ccu_pad>;
> + clock-names = "pclk", "core", "pad";
> + };
> + - |
> + mdio-bus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethernet-pcs@0 {
> + compatible = "snps,dw-xpcs";
> + reg = <0>;
> +
> + clocks = <&ccu_core>, <&ccu_pad>;
> + clock-names = "core", "pad";
> + };
> + };
> +...
> --
> 2.43.0
>
On Wed, Jun 05, 2024 at 05:29:16PM -0600, Rob Herring wrote:
> On Sun, Jun 02, 2024 at 05:36:20PM +0300, Serge Semin wrote:
> > Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
> > providing an interface between the Media Access Control (MAC) and Physical
> > Medium Attachment Sublayer (PMA) through a Media independent interface.
> > >From software point of view it exposes IEEE std. Clause 45 CSR space and
> > can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
> > case the PCS device is supposed to be defined under the respective MDIO
> > bus DT-node. In the later case the DW xPCS will be just a normal IO
> > memory-mapped device.
> >
> > Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
> > source properties specified. The former one indicates the Clause 73/37
> > auto-negotiation events like: negotiation page received, AN is completed
> > or incompatible link partner. The clock DT-properties can describe up to
> > three clock sources: peripheral bus clock source, internal reference clock
> > and the externally connected reference clock.
> >
> > Finally the DW XPCS IP-core can be optionally synthesized with a
> > vendor-specific interface connected to the Synopsys PMA (also called
> > DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
> > portable way. So if the DW XPCS device has the respective PMA attached
> > then it should be reflected in the DT-node compatible string so the driver
> > would be aware of the PMA-specific device capabilities (mainly connected
> > with CSRs available for the fine-tunings).
> >
> > Signed-off-by: Serge Semin <[email protected]>
> >
> > ---
> >
> > Changelog v2:
> > - Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
> > interface is just a normal memory-mapped device.
> > ---
> > .../bindings/net/pcs/snps,dw-xpcs.yaml | 133 ++++++++++++++++++
> > 1 file changed, 133 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > new file mode 100644
> > index 000000000000..7927bceefbf3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > @@ -0,0 +1,133 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Synopsys DesignWare Ethernet PCS
> > +
> > +maintainers:
> > + - Serge Semin <[email protected]>
> > +
> > +description:
> > + Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
> > + between Media Access Control and Physical Medium Attachment Sublayer through
> > + the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
> > + controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
> > + optionally synthesized with a vendor-specific interface connected to
> > + Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
> > + general it can be used to communicate with any compatible PHY.
> > +
> > + The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
> > + by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
> > + right to the system IO memory space.
> > +
> > +properties:
> > + compatible:
> > + oneOf:
> > + - description: Synopsys DesignWare XPCS with none or unknown PMA
> > + const: snps,dw-xpcs
> > + - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
> > + const: snps,dw-xpcs-gen1-3g
> > + - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
> > + const: snps,dw-xpcs-gen2-3g
> > + - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
> > + const: snps,dw-xpcs-gen2-6g
> > + - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
> > + const: snps,dw-xpcs-gen4-3g
> > + - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
> > + const: snps,dw-xpcs-gen4-6g
> > + - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
> > + const: snps,dw-xpcs-gen5-10g
> > + - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
> > + const: snps,dw-xpcs-gen5-12g
> > +
> > + reg:
> > + items:
> > + - description:
> > + In case of the MDIO management interface this just a 5-bits ID
> > + of the MDIO bus device. If DW XPCS CSRs space is accessed over the
> > + MCI or APB3 management interfaces, then the space mapping can be
> > + either 'direct' or 'indirect'. In the former case all Clause 45
> > + registers are contiguously mapped within the address space
> > + MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
> > + to the multiple 256 register sets. There is a special viewport CSR
> > + which is responsible for the set selection. The upper part of
> > + the CSR address MMD+REG[20:8] is supposed to be written in there
> > + so the corresponding subset would be mapped to the lowest 255 CSRs.
> > +
> > + reg-names:
> > + items:
> > + - enum: [ direct, indirect ]
> > +
> > + reg-io-width:
> > + description:
> > + The way the CSRs are mapped to the memory is platform depended. Since
> > + each Clause 45 CSR is of 16-bits wide the access instructions must be
> > + two bytes aligned at least.
> > + default: 2
> > + enum: [ 2, 4 ]
> > +
> > + interrupts:
> > + description:
> > + System interface interrupt output (sbd_intr_o) indicating Clause 73/37
> > + auto-negotiation events':' Page received, AN is completed or incompatible
> > + link partner.
> > + maxItems: 1
> > +
> > + clocks:
> > + description:
> > + Both MCI and APB3 interfaces are supposed to be equipped with a clock
> > + source connected via the clk_csr_i line.
> > +
> > + PCS/PMA layer can be clocked by an internal reference clock source
> > + (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
> > + generator. Both clocks can be supplied at a time.
> > + minItems: 1
> > + maxItems: 3
> > +
> > + clock-names:
> > + minItems: 1
> > + maxItems: 3
> > + anyOf:
> > + - items:
> > + enum: [ core, pad ]
>
> This has no effect. If it is true, then the 2nd entry is too.
Yeah, from the anyOf logic it's redundant indeed. But the idea was to
signify that the DT-node may have one the next clock-names
combination:
clock-names = "pad";
or clock-names = "core";
or clock-names = "core", "pad";
or clock-names = "pclk";
or clock-names = "pclk", "core";
or clock-names = "pclk", "pad";
or clock-names = "pclk", "core", "pad";
>
> You are saying all the clocks are optional and any combination/order is
> valid. Do we really need it so flexible? Doubtful the h/w is that
> flexible.
Well, I failed to figure out a more restrictive but still simple
constraint. Here are the conditions which need to be taken into
account:
1. "pclk" is specific for the memory-mapped DW XPCS only (DT-nodes
found under normal system bus super-node). DT-nodes placed under the
MDIO-bus super-node obviously have the MDIO-bus communication channel
which is clocked by the internal clock generator.
2. "core" (also mentioned as "alt" in the HW-databooks) and "pad"
clock sources can be found on XPCS with DW Enterprise Gen2, Gen4, Gen5
and Gen6 PMAs. (At least that's what I managed to find in the DW XPCS
v3.11a HW-manual.) Both of these clock sources can be specified at a
time. So it's the software responsibility to choose which one to use.
So based on the notes above it's still possible to have no clock
source specified if it's an MDIO-based DW XPCS with a PMA/PHY with no
ref-clock required.
Any idea of how to implement the constraint with these conditions
followed?
-Serge(y)
>
> > + - items:
> > + enum: [ pclk, core, pad ]
> > +
> > +required:
> > + - compatible
> > + - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > + ethernet-pcs@1f05d000 {
> > + compatible = "snps,dw-xpcs";
> > + reg = <0x1f05d000 0x1000>;
> > + reg-names = "indirect";
> > +
> > + reg-io-width = <4>;
> > +
> > + interrupts = <79 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > + clocks = <&ccu_pclk>, <&ccu_core>, <&ccu_pad>;
> > + clock-names = "pclk", "core", "pad";
> > + };
> > + - |
> > + mdio-bus {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + ethernet-pcs@0 {
> > + compatible = "snps,dw-xpcs";
> > + reg = <0>;
> > +
> > + clocks = <&ccu_core>, <&ccu_pad>;
> > + clock-names = "core", "pad";
> > + };
> > + };
> > +...
> > --
> > 2.43.0
> >
Hi Abhishek
On Wed, Jun 05, 2024 at 12:15:54PM -0700, Abhishek Chauhan (ABC) wrote:
>
> > @@ -482,7 +482,7 @@ static int xpcs_config_aneg_c73(struct dw_xpcs *xpcs,
> >
> > static int xpcs_aneg_done_c73(struct dw_xpcs *xpcs,
> > struct phylink_link_state *state,
> > - const struct xpcs_compat *compat, u16 an_stat1)
> > + const struct dw_xpcs_compat *compat, u16 an_stat1)
> > {
> > int ret;
> >
> > @@ -607,7 +607,7 @@ static int xpcs_validate(struct phylink_pcs *pcs, unsigned long *supported,
> > const struct phylink_link_state *state)
> > {
> > __ETHTOOL_DECLARE_LINK_MODE_MASK(xpcs_supported) = { 0, };
> > - const struct xpcs_compat *compat;
> > + const struct dw_xpcs_compat *compat;
> > struct dw_xpcs *xpcs;
> > int i;
> >
> > @@ -633,7 +633,7 @@ void xpcs_get_interfaces(struct dw_xpcs *xpcs, unsigned long *interfaces)
> > int i, j;
> >
> > for (i = 0; i < DW_XPCS_INTERFACE_MAX; i++) {
> > - const struct xpcs_compat *compat = &xpcs->desc->compat[i];
> > + const struct dw_xpcs_compat *compat = &xpcs->desc->compat[i];
> >
> > for (j = 0; j < compat->num_interfaces; j++)
> > __set_bit(compat->interface[j], interfaces);
> > @@ -850,7 +850,7 @@ static int xpcs_config_2500basex(struct dw_xpcs *xpcs)
> > int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
> > const unsigned long *advertising, unsigned int neg_mode)
> > {
> > - const struct xpcs_compat *compat;
> > + const struct dw_xpcs_compat *compat;
> > int ret;
> >
> > compat = xpcs_find_compat(xpcs->desc, interface);
> > @@ -915,7 +915,7 @@ static int xpcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
> >
> > static int xpcs_get_state_c73(struct dw_xpcs *xpcs,
> > struct phylink_link_state *state,
> > - const struct xpcs_compat *compat)
> > + const struct dw_xpcs_compat *compat)
> > {
> > bool an_enabled;
> > int pcs_stat1;
> > @@ -1115,7 +1115,7 @@ static void xpcs_get_state(struct phylink_pcs *pcs,
> > struct phylink_link_state *state)
> > {
> > struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs);
> > - const struct xpcs_compat *compat;
> > + const struct dw_xpcs_compat *compat;
> > int ret;
> >
> > compat = xpcs_find_compat(xpcs->desc, state->interface);
> > @@ -1269,7 +1269,7 @@ static u32 xpcs_get_id(struct dw_xpcs *xpcs)
> > return 0xffffffff;
> > }
> >
> > -static const struct xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> > +static const struct dw_xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> > [DW_XPCS_USXGMII] = {
> > .supported = xpcs_usxgmii_features,
> > .interface = xpcs_usxgmii_interfaces,
> > @@ -1314,7 +1314,7 @@ static const struct xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> > },
> > };
> >
> Serge, Thank you for raising these patches. Minor comments which shows warning on my workspace.
>
> WARNING: line length of 82 exceeds 80 columns
> #153: FILE: drivers/net/pcs/pcs-xpcs.c:1272:
> +static const struct dw_xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
>
> WARNING: line length of 85 exceeds 80 columns
> #162: FILE: drivers/net/pcs/pcs-xpcs.c:1317:
> +static const struct dw_xpcs_compat nxp_sja1105_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
>
> WARNING: line length of 85 exceeds 80 columns
> #171: FILE: drivers/net/pcs/pcs-xpcs.c:1327:
> +static const struct dw_xpcs_compat nxp_sja1110_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
>
My checkpatch didn't warn about that even with the strict argument
specified.
Note there is just 3 and 6 characters over the preferable limit.
Splitting the lines will make the code less readable (in some extent).
So from that perspective it's ok to exceed 80 characters limit in this
case and not to break the generic kernel coding style convention.
Unless the networking subsystem has a more strict requirement in this
matter.
-Serge(y)
> > -static const struct xpcs_compat nxp_sja1105_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> > +static const struct dw_xpcs_compat nxp_sja1105_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> > [DW_XPCS_SGMII] = {
> > .supported = xpcs_sgmii_features,
> > .interface = xpcs_sgmii_interfaces,
> > @@ -1324,7 +1324,7 @@ static const struct xpcs_compat nxp_sja1105_xpcs_compat[DW_XPCS_INTERFACE_MAX] =
> > },
> > };
> >
> > -static const struct xpcs_compat nxp_sja1110_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> > +static const struct dw_xpcs_compat nxp_sja1110_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
> > [DW_XPCS_SGMII] = {
> > .supported = xpcs_sgmii_features,
> > .interface = xpcs_sgmii_interfaces,
> > @@ -1418,7 +1418,7 @@ static int xpcs_init_id(struct dw_xpcs *xpcs)
> >
> > static int xpcs_init_iface(struct dw_xpcs *xpcs, phy_interface_t interface)
> > {
> > - const struct xpcs_compat *compat;
> > + const struct dw_xpcs_compat *compat;
> >
> > compat = xpcs_find_compat(xpcs->desc, interface);
> > if (!compat)
Hi Simon
On Wed, Jun 05, 2024 at 06:49:20PM +0100, Simon Horman wrote:
> On Sun, Jun 02, 2024 at 05:36:22PM +0300, Serge Semin wrote:
> > It's now possible to have the DW XPCS device defined as a standard
> > platform device for instance in the platform DT-file. Although that
> > functionality is useless unless there is a way to have the device found by
> > the client drivers (STMMAC/DW *MAC, NXP SJA1105 Eth Switch, etc). Provide
> > such ability by means of the xpcs_create_fwnode() method. It needs to be
> > called with the device DW XPCS fwnode instance passed. That node will be
> > then used to find the MDIO-device instance in order to create the DW XPCS
> > descriptor.
> >
> > Note the method semantics and name is similar to what has been recently
> > introduced in the Lynx PCS driver.
> >
> > Signed-off-by: Serge Semin <[email protected]>
>
> Hi Serge,
>
> Some minor nits from my side flagged by kernel-doc -none -Wall
>
> ...
>
> > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
>
> ...
>
> > @@ -1505,6 +1507,16 @@ static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
> > return ERR_PTR(ret);
> > }
> >
> > +/**
> > + * xpcs_create_mdiodev() - create a DW xPCS instance with the MDIO @addr
> > + * @bus: pointer to the MDIO-bus descriptor for the device to be looked at
> > + * @addr: device MDIO-bus ID
> > + * @requested PHY interface
>
> An entry for @interface should go here.
Right.
>
> > + *
> > + * If successful, returns a pointer to the DW XPCS handle. Otherwise returns
> > + * -ENODEV if device couldn't be found on the bus, other negative errno related
> > + * to the data allocation and MDIO-bus communications.
>
> Please consider including this information as a Return: section of the
> Kernel doc. Likewise for xpcs_create_fwnode().
Sure.
>
> > + */
> > struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
> > phy_interface_t interface)
> > {
> > @@ -1529,6 +1541,44 @@ struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
> > }
> > EXPORT_SYMBOL_GPL(xpcs_create_mdiodev);
> >
> > +/**
> > + * xpcs_create_fwnode() - Create a DW xPCS instance from @fwnode
> > + * @node: fwnode handle poining to the DW XPCS device
>
> s/@node/@fwnode/
Holy mother, so many typos in the kdoc part. I should have been more
attentive. I'll fix all of them in v2. Thanks.
* Special thanks for mentioning the scripts/kernel-doc I'll be using
it from now on.
-Serge(y)
>
> > + * @interface: requested PHY interface
> > + *
> > + * If successful, returns a pointer to the DW XPCS handle. Otherwise returns
> > + * -ENODEV if the fwnode is marked unavailable or device couldn't be found on
> > + * the bus, -EPROBE_DEFER if the respective MDIO-device instance couldn't be
> > + * found, other negative errno related to the data allocations and MDIO-bus
> > + * communications.
> > + */
> > +struct dw_xpcs *xpcs_create_fwnode(struct fwnode_handle *fwnode,
> > + phy_interface_t interface)
>
> ...
Hi Simon
On Wed, Jun 05, 2024 at 06:48:17PM +0100, Simon Horman wrote:
> On Sun, Jun 02, 2024 at 05:36:21PM +0300, Serge Semin wrote:
>
> ...
>
> > diff --git a/drivers/net/pcs/pcs-xpcs-plat.c b/drivers/net/pcs/pcs-xpcs-plat.c
>
> ...
>
> > +const struct dev_pm_ops xpcs_plat_pm_ops = {
> > + SET_RUNTIME_PM_OPS(xpcs_plat_pm_runtime_suspend,
> > + xpcs_plat_pm_runtime_resume,
> > + NULL)
> > +};
>
> nit: xpcs_plat_pm_ops only seems to be used in this file.
> If so it should probably be static.
>
> Flagged by Sparse.
Right. I'll convert it to being static. Thanks.
-Serge(y)
>
> ...
>
> > +static struct platform_driver xpcs_plat_driver = {
> > + .probe = xpcs_plat_probe,
> > + .driver = {
> > + .name = "dwxpcs",
> > + .pm = &xpcs_plat_pm_ops,
> > + .of_match_table = xpcs_of_ids,
> > + },
> > +};
> > +module_platform_driver(xpcs_plat_driver);
>
> ...
On Thu, Jun 06, 2024 at 12:54:33PM +0300, Serge Semin wrote:
> On Wed, Jun 05, 2024 at 05:29:16PM -0600, Rob Herring wrote:
> > On Sun, Jun 02, 2024 at 05:36:20PM +0300, Serge Semin wrote:
> > > Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
> > > providing an interface between the Media Access Control (MAC) and Physical
> > > Medium Attachment Sublayer (PMA) through a Media independent interface.
> > > >From software point of view it exposes IEEE std. Clause 45 CSR space and
> > > can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
> > > case the PCS device is supposed to be defined under the respective MDIO
> > > bus DT-node. In the later case the DW xPCS will be just a normal IO
> > > memory-mapped device.
> > >
> > > Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
> > > source properties specified. The former one indicates the Clause 73/37
> > > auto-negotiation events like: negotiation page received, AN is completed
> > > or incompatible link partner. The clock DT-properties can describe up to
> > > three clock sources: peripheral bus clock source, internal reference clock
> > > and the externally connected reference clock.
> > >
> > > Finally the DW XPCS IP-core can be optionally synthesized with a
> > > vendor-specific interface connected to the Synopsys PMA (also called
> > > DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
> > > portable way. So if the DW XPCS device has the respective PMA attached
> > > then it should be reflected in the DT-node compatible string so the driver
> > > would be aware of the PMA-specific device capabilities (mainly connected
> > > with CSRs available for the fine-tunings).
> > >
> > > Signed-off-by: Serge Semin <[email protected]>
> > >
> > > ---
> > >
> > > Changelog v2:
> > > - Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
> > > interface is just a normal memory-mapped device.
> > > ---
> > > .../bindings/net/pcs/snps,dw-xpcs.yaml | 133 ++++++++++++++++++
> > > 1 file changed, 133 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > new file mode 100644
> > > index 000000000000..7927bceefbf3
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > @@ -0,0 +1,133 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Synopsys DesignWare Ethernet PCS
> > > +
> > > +maintainers:
> > > + - Serge Semin <[email protected]>
> > > +
> > > +description:
> > > + Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
> > > + between Media Access Control and Physical Medium Attachment Sublayer through
> > > + the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
> > > + controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
> > > + optionally synthesized with a vendor-specific interface connected to
> > > + Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
> > > + general it can be used to communicate with any compatible PHY.
> > > +
> > > + The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
> > > + by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
> > > + right to the system IO memory space.
> > > +
> > > +properties:
> > > + compatible:
> > > + oneOf:
> > > + - description: Synopsys DesignWare XPCS with none or unknown PMA
> > > + const: snps,dw-xpcs
> > > + - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
> > > + const: snps,dw-xpcs-gen1-3g
> > > + - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
> > > + const: snps,dw-xpcs-gen2-3g
> > > + - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
> > > + const: snps,dw-xpcs-gen2-6g
> > > + - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
> > > + const: snps,dw-xpcs-gen4-3g
> > > + - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
> > > + const: snps,dw-xpcs-gen4-6g
> > > + - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
> > > + const: snps,dw-xpcs-gen5-10g
> > > + - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
> > > + const: snps,dw-xpcs-gen5-12g
> > > +
> > > + reg:
> > > + items:
> > > + - description:
> > > + In case of the MDIO management interface this just a 5-bits ID
> > > + of the MDIO bus device. If DW XPCS CSRs space is accessed over the
> > > + MCI or APB3 management interfaces, then the space mapping can be
> > > + either 'direct' or 'indirect'. In the former case all Clause 45
> > > + registers are contiguously mapped within the address space
> > > + MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
> > > + to the multiple 256 register sets. There is a special viewport CSR
> > > + which is responsible for the set selection. The upper part of
> > > + the CSR address MMD+REG[20:8] is supposed to be written in there
> > > + so the corresponding subset would be mapped to the lowest 255 CSRs.
> > > +
> > > + reg-names:
> > > + items:
> > > + - enum: [ direct, indirect ]
> > > +
> > > + reg-io-width:
> > > + description:
> > > + The way the CSRs are mapped to the memory is platform depended. Since
> > > + each Clause 45 CSR is of 16-bits wide the access instructions must be
> > > + two bytes aligned at least.
> > > + default: 2
> > > + enum: [ 2, 4 ]
> > > +
> > > + interrupts:
> > > + description:
> > > + System interface interrupt output (sbd_intr_o) indicating Clause 73/37
> > > + auto-negotiation events':' Page received, AN is completed or incompatible
> > > + link partner.
> > > + maxItems: 1
> > > +
> > > + clocks:
> > > + description:
> > > + Both MCI and APB3 interfaces are supposed to be equipped with a clock
> > > + source connected via the clk_csr_i line.
> > > +
> > > + PCS/PMA layer can be clocked by an internal reference clock source
> > > + (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
> > > + generator. Both clocks can be supplied at a time.
> > > + minItems: 1
> > > + maxItems: 3
> > > +
> > > + clock-names:
> > > + minItems: 1
> > > + maxItems: 3
> > > + anyOf:
> > > + - items:
> > > + enum: [ core, pad ]
> >
>
> > This has no effect. If it is true, then the 2nd entry is too.
>
> Yeah, from the anyOf logic it's redundant indeed. But the idea was to
> signify that the DT-node may have one the next clock-names
> combination:
> clock-names = "pad";
> or clock-names = "core";
> or clock-names = "core", "pad";
> or clock-names = "pclk";
> or clock-names = "pclk", "core";
> or clock-names = "pclk", "pad";
> or clock-names = "pclk", "core", "pad";
That would be:
oneOf:
- minItems: 1
items:
- enum: [core, pad]
- const: pad
- minItems: 1
items:
- const: pclk
- enum: [core, pad]
- const: pad
*-names is enforced to be 'uniqueItems: true', so we don't have to worry
about repeated entries.
This also nicely splits between MMIO and MDIO.
> >
> > You are saying all the clocks are optional and any combination/order is
> > valid. Do we really need it so flexible? Doubtful the h/w is that
> > flexible.
>
> Well, I failed to figure out a more restrictive but still simple
> constraint. Here are the conditions which need to be taken into
> account:
> 1. "pclk" is specific for the memory-mapped DW XPCS only (DT-nodes
> found under normal system bus super-node). DT-nodes placed under the
> MDIO-bus super-node obviously have the MDIO-bus communication channel
> which is clocked by the internal clock generator.
> 2. "core" (also mentioned as "alt" in the HW-databooks) and "pad"
> clock sources can be found on XPCS with DW Enterprise Gen2, Gen4, Gen5
> and Gen6 PMAs. (At least that's what I managed to find in the DW XPCS
> v3.11a HW-manual.) Both of these clock sources can be specified at a
> time. So it's the software responsibility to choose which one to use.
>
> So based on the notes above it's still possible to have no clock
> source specified if it's an MDIO-based DW XPCS with a PMA/PHY with no
> ref-clock required.
>
> Any idea of how to implement the constraint with these conditions
> followed?
>
> -Serge(y)
Hi Rob
On Mon, Jun 10, 2024 at 03:49:16PM -0600, Rob Herring wrote:
> On Thu, Jun 06, 2024 at 12:54:33PM +0300, Serge Semin wrote:
> > On Wed, Jun 05, 2024 at 05:29:16PM -0600, Rob Herring wrote:
> > > On Sun, Jun 02, 2024 at 05:36:20PM +0300, Serge Semin wrote:
> > > > Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
> > > > providing an interface between the Media Access Control (MAC) and Physical
> > > > Medium Attachment Sublayer (PMA) through a Media independent interface.
> > > > >From software point of view it exposes IEEE std. Clause 45 CSR space and
> > > > can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
> > > > case the PCS device is supposed to be defined under the respective MDIO
> > > > bus DT-node. In the later case the DW xPCS will be just a normal IO
> > > > memory-mapped device.
> > > >
> > > > Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
> > > > source properties specified. The former one indicates the Clause 73/37
> > > > auto-negotiation events like: negotiation page received, AN is completed
> > > > or incompatible link partner. The clock DT-properties can describe up to
> > > > three clock sources: peripheral bus clock source, internal reference clock
> > > > and the externally connected reference clock.
> > > >
> > > > Finally the DW XPCS IP-core can be optionally synthesized with a
> > > > vendor-specific interface connected to the Synopsys PMA (also called
> > > > DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
> > > > portable way. So if the DW XPCS device has the respective PMA attached
> > > > then it should be reflected in the DT-node compatible string so the driver
> > > > would be aware of the PMA-specific device capabilities (mainly connected
> > > > with CSRs available for the fine-tunings).
> > > >
> > > > Signed-off-by: Serge Semin <[email protected]>
> > > >
> > > > ---
> > > >
> > > > Changelog v2:
> > > > - Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
> > > > interface is just a normal memory-mapped device.
> > > > ---
> > > > .../bindings/net/pcs/snps,dw-xpcs.yaml | 133 ++++++++++++++++++
> > > > 1 file changed, 133 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > new file mode 100644
> > > > index 000000000000..7927bceefbf3
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > @@ -0,0 +1,133 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Synopsys DesignWare Ethernet PCS
> > > > +
> > > > +maintainers:
> > > > + - Serge Semin <[email protected]>
> > > > +
> > > > +description:
> > > > + Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
> > > > + between Media Access Control and Physical Medium Attachment Sublayer through
> > > > + the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
> > > > + controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
> > > > + optionally synthesized with a vendor-specific interface connected to
> > > > + Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
> > > > + general it can be used to communicate with any compatible PHY.
> > > > +
> > > > + The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
> > > > + by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
> > > > + right to the system IO memory space.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + oneOf:
> > > > + - description: Synopsys DesignWare XPCS with none or unknown PMA
> > > > + const: snps,dw-xpcs
> > > > + - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
> > > > + const: snps,dw-xpcs-gen1-3g
> > > > + - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
> > > > + const: snps,dw-xpcs-gen2-3g
> > > > + - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
> > > > + const: snps,dw-xpcs-gen2-6g
> > > > + - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
> > > > + const: snps,dw-xpcs-gen4-3g
> > > > + - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
> > > > + const: snps,dw-xpcs-gen4-6g
> > > > + - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
> > > > + const: snps,dw-xpcs-gen5-10g
> > > > + - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
> > > > + const: snps,dw-xpcs-gen5-12g
> > > > +
> > > > + reg:
> > > > + items:
> > > > + - description:
> > > > + In case of the MDIO management interface this just a 5-bits ID
> > > > + of the MDIO bus device. If DW XPCS CSRs space is accessed over the
> > > > + MCI or APB3 management interfaces, then the space mapping can be
> > > > + either 'direct' or 'indirect'. In the former case all Clause 45
> > > > + registers are contiguously mapped within the address space
> > > > + MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
> > > > + to the multiple 256 register sets. There is a special viewport CSR
> > > > + which is responsible for the set selection. The upper part of
> > > > + the CSR address MMD+REG[20:8] is supposed to be written in there
> > > > + so the corresponding subset would be mapped to the lowest 255 CSRs.
> > > > +
> > > > + reg-names:
> > > > + items:
> > > > + - enum: [ direct, indirect ]
> > > > +
> > > > + reg-io-width:
> > > > + description:
> > > > + The way the CSRs are mapped to the memory is platform depended. Since
> > > > + each Clause 45 CSR is of 16-bits wide the access instructions must be
> > > > + two bytes aligned at least.
> > > > + default: 2
> > > > + enum: [ 2, 4 ]
> > > > +
> > > > + interrupts:
> > > > + description:
> > > > + System interface interrupt output (sbd_intr_o) indicating Clause 73/37
> > > > + auto-negotiation events':' Page received, AN is completed or incompatible
> > > > + link partner.
> > > > + maxItems: 1
> > > > +
> > > > + clocks:
> > > > + description:
> > > > + Both MCI and APB3 interfaces are supposed to be equipped with a clock
> > > > + source connected via the clk_csr_i line.
> > > > +
> > > > + PCS/PMA layer can be clocked by an internal reference clock source
> > > > + (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
> > > > + generator. Both clocks can be supplied at a time.
> > > > + minItems: 1
> > > > + maxItems: 3
> > > > +
> > > > + clock-names:
> > > > + minItems: 1
> > > > + maxItems: 3
> > > > + anyOf:
> > > > + - items:
> > > > + enum: [ core, pad ]
> > >
> >
> > > This has no effect. If it is true, then the 2nd entry is too.
> >
> > Yeah, from the anyOf logic it's redundant indeed. But the idea was to
> > signify that the DT-node may have one the next clock-names
> > combination:
> > clock-names = "pad";
> > or clock-names = "core";
> > or clock-names = "core", "pad";
> > or clock-names = "pclk";
> > or clock-names = "pclk", "core";
> > or clock-names = "pclk", "pad";
> > or clock-names = "pclk", "core", "pad";
>
> That would be:
>
> oneOf:
> - minItems: 1
> items:
> - enum: [core, pad]
> - const: pad
> - minItems: 1
> items:
> - const: pclk
> - enum: [core, pad]
> - const: pad
>
> *-names is enforced to be 'uniqueItems: true', so we don't have to worry
> about repeated entries.
>
> This also nicely splits between MMIO and MDIO.
I had such approach in mind, but it seemed to me more complicated and
weakly scaleable (should we need to add some more clocks). Isn't the
next constraint look more readable:
anyOf:
- description: DW XPCS accessible over MDIO-bus
minItems: 1
maxItems: 2
items:
enum: [core, pad]
- description: DW XPCS with the MCI/APB3 CSRs IO interface
minItems: 1
maxItems: 3
items:
enum: [pclk, core, pad]
contains:
const: pclk
?
AFAICS The only reason of using the pattern suggested by you would be
to define the ordered clock phandle settings. Is it so necessary
that we should sacrifice the readability in favor of the more strict
and less scalable solution?
-Serge(y)
>
> > >
> > > You are saying all the clocks are optional and any combination/order is
> > > valid. Do we really need it so flexible? Doubtful the h/w is that
> > > flexible.
> >
> > Well, I failed to figure out a more restrictive but still simple
> > constraint. Here are the conditions which need to be taken into
> > account:
> > 1. "pclk" is specific for the memory-mapped DW XPCS only (DT-nodes
> > found under normal system bus super-node). DT-nodes placed under the
> > MDIO-bus super-node obviously have the MDIO-bus communication channel
> > which is clocked by the internal clock generator.
> > 2. "core" (also mentioned as "alt" in the HW-databooks) and "pad"
> > clock sources can be found on XPCS with DW Enterprise Gen2, Gen4, Gen5
> > and Gen6 PMAs. (At least that's what I managed to find in the DW XPCS
> > v3.11a HW-manual.) Both of these clock sources can be specified at a
> > time. So it's the software responsibility to choose which one to use.
> >
> > So based on the notes above it's still possible to have no clock
> > source specified if it's an MDIO-based DW XPCS with a PMA/PHY with no
> > ref-clock required.
> >
> > Any idea of how to implement the constraint with these conditions
> > followed?
> >
> > -Serge(y)
On Tue, Jun 11, 2024 at 01:45:16PM +0300, Serge Semin wrote:
> Hi Rob
>
> On Mon, Jun 10, 2024 at 03:49:16PM -0600, Rob Herring wrote:
> > On Thu, Jun 06, 2024 at 12:54:33PM +0300, Serge Semin wrote:
> > > On Wed, Jun 05, 2024 at 05:29:16PM -0600, Rob Herring wrote:
> > > > On Sun, Jun 02, 2024 at 05:36:20PM +0300, Serge Semin wrote:
> > > > > Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
> > > > > providing an interface between the Media Access Control (MAC) and Physical
> > > > > Medium Attachment Sublayer (PMA) through a Media independent interface.
> > > > > >From software point of view it exposes IEEE std. Clause 45 CSR space and
> > > > > can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
> > > > > case the PCS device is supposed to be defined under the respective MDIO
> > > > > bus DT-node. In the later case the DW xPCS will be just a normal IO
> > > > > memory-mapped device.
> > > > >
> > > > > Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
> > > > > source properties specified. The former one indicates the Clause 73/37
> > > > > auto-negotiation events like: negotiation page received, AN is completed
> > > > > or incompatible link partner. The clock DT-properties can describe up to
> > > > > three clock sources: peripheral bus clock source, internal reference clock
> > > > > and the externally connected reference clock.
> > > > >
> > > > > Finally the DW XPCS IP-core can be optionally synthesized with a
> > > > > vendor-specific interface connected to the Synopsys PMA (also called
> > > > > DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
> > > > > portable way. So if the DW XPCS device has the respective PMA attached
> > > > > then it should be reflected in the DT-node compatible string so the driver
> > > > > would be aware of the PMA-specific device capabilities (mainly connected
> > > > > with CSRs available for the fine-tunings).
> > > > >
> > > > > Signed-off-by: Serge Semin <[email protected]>
> > > > >
> > > > > ---
> > > > >
> > > > > Changelog v2:
> > > > > - Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
> > > > > interface is just a normal memory-mapped device.
> > > > > ---
> > > > > .../bindings/net/pcs/snps,dw-xpcs.yaml | 133 ++++++++++++++++++
> > > > > 1 file changed, 133 insertions(+)
> > > > > create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..7927bceefbf3
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > > @@ -0,0 +1,133 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Synopsys DesignWare Ethernet PCS
> > > > > +
> > > > > +maintainers:
> > > > > + - Serge Semin <[email protected]>
> > > > > +
> > > > > +description:
> > > > > + Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
> > > > > + between Media Access Control and Physical Medium Attachment Sublayer through
> > > > > + the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
> > > > > + controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
> > > > > + optionally synthesized with a vendor-specific interface connected to
> > > > > + Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
> > > > > + general it can be used to communicate with any compatible PHY.
> > > > > +
> > > > > + The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
> > > > > + by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
> > > > > + right to the system IO memory space.
> > > > > +
> > > > > +properties:
> > > > > + compatible:
> > > > > + oneOf:
> > > > > + - description: Synopsys DesignWare XPCS with none or unknown PMA
> > > > > + const: snps,dw-xpcs
> > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
> > > > > + const: snps,dw-xpcs-gen1-3g
> > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
> > > > > + const: snps,dw-xpcs-gen2-3g
> > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
> > > > > + const: snps,dw-xpcs-gen2-6g
> > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
> > > > > + const: snps,dw-xpcs-gen4-3g
> > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
> > > > > + const: snps,dw-xpcs-gen4-6g
> > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
> > > > > + const: snps,dw-xpcs-gen5-10g
> > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
> > > > > + const: snps,dw-xpcs-gen5-12g
> > > > > +
> > > > > + reg:
> > > > > + items:
> > > > > + - description:
> > > > > + In case of the MDIO management interface this just a 5-bits ID
> > > > > + of the MDIO bus device. If DW XPCS CSRs space is accessed over the
> > > > > + MCI or APB3 management interfaces, then the space mapping can be
> > > > > + either 'direct' or 'indirect'. In the former case all Clause 45
> > > > > + registers are contiguously mapped within the address space
> > > > > + MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
> > > > > + to the multiple 256 register sets. There is a special viewport CSR
> > > > > + which is responsible for the set selection. The upper part of
> > > > > + the CSR address MMD+REG[20:8] is supposed to be written in there
> > > > > + so the corresponding subset would be mapped to the lowest 255 CSRs.
> > > > > +
> > > > > + reg-names:
> > > > > + items:
> > > > > + - enum: [ direct, indirect ]
> > > > > +
> > > > > + reg-io-width:
> > > > > + description:
> > > > > + The way the CSRs are mapped to the memory is platform depended. Since
> > > > > + each Clause 45 CSR is of 16-bits wide the access instructions must be
> > > > > + two bytes aligned at least.
> > > > > + default: 2
> > > > > + enum: [ 2, 4 ]
> > > > > +
> > > > > + interrupts:
> > > > > + description:
> > > > > + System interface interrupt output (sbd_intr_o) indicating Clause 73/37
> > > > > + auto-negotiation events':' Page received, AN is completed or incompatible
> > > > > + link partner.
> > > > > + maxItems: 1
> > > > > +
> > > > > + clocks:
> > > > > + description:
> > > > > + Both MCI and APB3 interfaces are supposed to be equipped with a clock
> > > > > + source connected via the clk_csr_i line.
> > > > > +
> > > > > + PCS/PMA layer can be clocked by an internal reference clock source
> > > > > + (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
> > > > > + generator. Both clocks can be supplied at a time.
> > > > > + minItems: 1
> > > > > + maxItems: 3
> > > > > +
> > > > > + clock-names:
> > > > > + minItems: 1
> > > > > + maxItems: 3
> > > > > + anyOf:
> > > > > + - items:
> > > > > + enum: [ core, pad ]
> > > >
> > >
> > > > This has no effect. If it is true, then the 2nd entry is too.
> > >
> > > Yeah, from the anyOf logic it's redundant indeed. But the idea was to
> > > signify that the DT-node may have one the next clock-names
> > > combination:
> > > clock-names = "pad";
> > > or clock-names = "core";
> > > or clock-names = "core", "pad";
> > > or clock-names = "pclk";
> > > or clock-names = "pclk", "core";
> > > or clock-names = "pclk", "pad";
> > > or clock-names = "pclk", "core", "pad";
> >
> > That would be:
> >
> > oneOf:
> > - minItems: 1
> > items:
> > - enum: [core, pad]
> > - const: pad
> > - minItems: 1
> > items:
> > - const: pclk
> > - enum: [core, pad]
> > - const: pad
> >
> > *-names is enforced to be 'uniqueItems: true', so we don't have to worry
> > about repeated entries.
> >
> > This also nicely splits between MMIO and MDIO.
>
> I had such approach in mind, but it seemed to me more complicated and
> weakly scaleable (should we need to add some more clocks). Isn't the
> next constraint look more readable:
Hardware is magically growing more clocks?
> anyOf:
> - description: DW XPCS accessible over MDIO-bus
> minItems: 1
> maxItems: 2
> items:
> enum: [core, pad]
> - description: DW XPCS with the MCI/APB3 CSRs IO interface
> minItems: 1
> maxItems: 3
> items:
> enum: [pclk, core, pad]
> contains:
> const: pclk
> ?
I don't see how that is much better in simplicity or scaleability. I
would just do this over the above:
minItems: 1
maxItems: 3
items:
enum: [pclk, core, pad]
Either you define the order or you don't. The former is strongly
preferred. The latter is done when it's too much a mess or we just don't
care to discuss it any more.
Rob
On Thu, Jun 13, 2024 at 09:41:25AM -0600, Rob Herring wrote:
> On Tue, Jun 11, 2024 at 01:45:16PM +0300, Serge Semin wrote:
> > Hi Rob
> >
> > On Mon, Jun 10, 2024 at 03:49:16PM -0600, Rob Herring wrote:
> > > On Thu, Jun 06, 2024 at 12:54:33PM +0300, Serge Semin wrote:
> > > > On Wed, Jun 05, 2024 at 05:29:16PM -0600, Rob Herring wrote:
> > > > > On Sun, Jun 02, 2024 at 05:36:20PM +0300, Serge Semin wrote:
> > > > > > Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
> > > > > > providing an interface between the Media Access Control (MAC) and Physical
> > > > > > Medium Attachment Sublayer (PMA) through a Media independent interface.
> > > > > > >From software point of view it exposes IEEE std. Clause 45 CSR space and
> > > > > > can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
> > > > > > case the PCS device is supposed to be defined under the respective MDIO
> > > > > > bus DT-node. In the later case the DW xPCS will be just a normal IO
> > > > > > memory-mapped device.
> > > > > >
> > > > > > Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
> > > > > > source properties specified. The former one indicates the Clause 73/37
> > > > > > auto-negotiation events like: negotiation page received, AN is completed
> > > > > > or incompatible link partner. The clock DT-properties can describe up to
> > > > > > three clock sources: peripheral bus clock source, internal reference clock
> > > > > > and the externally connected reference clock.
> > > > > >
> > > > > > Finally the DW XPCS IP-core can be optionally synthesized with a
> > > > > > vendor-specific interface connected to the Synopsys PMA (also called
> > > > > > DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
> > > > > > portable way. So if the DW XPCS device has the respective PMA attached
> > > > > > then it should be reflected in the DT-node compatible string so the driver
> > > > > > would be aware of the PMA-specific device capabilities (mainly connected
> > > > > > with CSRs available for the fine-tunings).
> > > > > >
> > > > > > Signed-off-by: Serge Semin <[email protected]>
> > > > > >
> > > > > > ---
> > > > > >
> > > > > > Changelog v2:
> > > > > > - Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
> > > > > > interface is just a normal memory-mapped device.
> > > > > > ---
> > > > > > .../bindings/net/pcs/snps,dw-xpcs.yaml | 133 ++++++++++++++++++
> > > > > > 1 file changed, 133 insertions(+)
> > > > > > create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..7927bceefbf3
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > > > @@ -0,0 +1,133 @@
> > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: Synopsys DesignWare Ethernet PCS
> > > > > > +
> > > > > > +maintainers:
> > > > > > + - Serge Semin <[email protected]>
> > > > > > +
> > > > > > +description:
> > > > > > + Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
> > > > > > + between Media Access Control and Physical Medium Attachment Sublayer through
> > > > > > + the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
> > > > > > + controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
> > > > > > + optionally synthesized with a vendor-specific interface connected to
> > > > > > + Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
> > > > > > + general it can be used to communicate with any compatible PHY.
> > > > > > +
> > > > > > + The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
> > > > > > + by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
> > > > > > + right to the system IO memory space.
> > > > > > +
> > > > > > +properties:
> > > > > > + compatible:
> > > > > > + oneOf:
> > > > > > + - description: Synopsys DesignWare XPCS with none or unknown PMA
> > > > > > + const: snps,dw-xpcs
> > > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
> > > > > > + const: snps,dw-xpcs-gen1-3g
> > > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
> > > > > > + const: snps,dw-xpcs-gen2-3g
> > > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
> > > > > > + const: snps,dw-xpcs-gen2-6g
> > > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
> > > > > > + const: snps,dw-xpcs-gen4-3g
> > > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
> > > > > > + const: snps,dw-xpcs-gen4-6g
> > > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
> > > > > > + const: snps,dw-xpcs-gen5-10g
> > > > > > + - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
> > > > > > + const: snps,dw-xpcs-gen5-12g
> > > > > > +
> > > > > > + reg:
> > > > > > + items:
> > > > > > + - description:
> > > > > > + In case of the MDIO management interface this just a 5-bits ID
> > > > > > + of the MDIO bus device. If DW XPCS CSRs space is accessed over the
> > > > > > + MCI or APB3 management interfaces, then the space mapping can be
> > > > > > + either 'direct' or 'indirect'. In the former case all Clause 45
> > > > > > + registers are contiguously mapped within the address space
> > > > > > + MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
> > > > > > + to the multiple 256 register sets. There is a special viewport CSR
> > > > > > + which is responsible for the set selection. The upper part of
> > > > > > + the CSR address MMD+REG[20:8] is supposed to be written in there
> > > > > > + so the corresponding subset would be mapped to the lowest 255 CSRs.
> > > > > > +
> > > > > > + reg-names:
> > > > > > + items:
> > > > > > + - enum: [ direct, indirect ]
> > > > > > +
> > > > > > + reg-io-width:
> > > > > > + description:
> > > > > > + The way the CSRs are mapped to the memory is platform depended. Since
> > > > > > + each Clause 45 CSR is of 16-bits wide the access instructions must be
> > > > > > + two bytes aligned at least.
> > > > > > + default: 2
> > > > > > + enum: [ 2, 4 ]
> > > > > > +
> > > > > > + interrupts:
> > > > > > + description:
> > > > > > + System interface interrupt output (sbd_intr_o) indicating Clause 73/37
> > > > > > + auto-negotiation events':' Page received, AN is completed or incompatible
> > > > > > + link partner.
> > > > > > + maxItems: 1
> > > > > > +
> > > > > > + clocks:
> > > > > > + description:
> > > > > > + Both MCI and APB3 interfaces are supposed to be equipped with a clock
> > > > > > + source connected via the clk_csr_i line.
> > > > > > +
> > > > > > + PCS/PMA layer can be clocked by an internal reference clock source
> > > > > > + (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
> > > > > > + generator. Both clocks can be supplied at a time.
> > > > > > + minItems: 1
> > > > > > + maxItems: 3
> > > > > > +
> > > > > > + clock-names:
> > > > > > + minItems: 1
> > > > > > + maxItems: 3
> > > > > > + anyOf:
> > > > > > + - items:
> > > > > > + enum: [ core, pad ]
> > > > >
> > > >
> > > > > This has no effect. If it is true, then the 2nd entry is too.
> > > >
> > > > Yeah, from the anyOf logic it's redundant indeed. But the idea was to
> > > > signify that the DT-node may have one the next clock-names
> > > > combination:
> > > > clock-names = "pad";
> > > > or clock-names = "core";
> > > > or clock-names = "core", "pad";
> > > > or clock-names = "pclk";
> > > > or clock-names = "pclk", "core";
> > > > or clock-names = "pclk", "pad";
> > > > or clock-names = "pclk", "core", "pad";
> > >
> > > That would be:
> > >
> > > oneOf:
> > > - minItems: 1
> > > items:
> > > - enum: [core, pad]
> > > - const: pad
> > > - minItems: 1
> > > items:
> > > - const: pclk
> > > - enum: [core, pad]
> > > - const: pad
> > >
> > > *-names is enforced to be 'uniqueItems: true', so we don't have to worry
> > > about repeated entries.
> > >
> > > This also nicely splits between MMIO and MDIO.
> >
> > I had such approach in mind, but it seemed to me more complicated and
> > weakly scaleable (should we need to add some more clocks). Isn't the
> > next constraint look more readable:
>
> Hardware is magically growing more clocks?
There is a non-zero probability I could have missed some additional
clocks defined in the DW XPCS IP-core databooks or there are some
vendor-specific clock sources we can't predict. Moreover the new
IP-core releases may have the clock sources list extended. So the
magic may happen.)
>
>
> > anyOf:
> > - description: DW XPCS accessible over MDIO-bus
> > minItems: 1
> > maxItems: 2
> > items:
> > enum: [core, pad]
> > - description: DW XPCS with the MCI/APB3 CSRs IO interface
> > minItems: 1
> > maxItems: 3
> > items:
> > enum: [pclk, core, pad]
> > contains:
> > const: pclk
> > ?
>
> I don't see how that is much better in simplicity or scaleability. I
> would just do this over the above:
>
> minItems: 1
> maxItems: 3
> items:
> enum: [pclk, core, pad]
>
> Either you define the order or you don't. The former is strongly
> preferred. The latter is done when it's too much a mess or we just don't
> care to discuss it any more.
Ok. Since the order-based constraint is strongly preferable, then
there is nothing to discuss. I'll make sure the order is defined.
Thanks for review.
-Serge(y)
>
> Rob