2024-02-20 19:51:44

by Christian Marangi

[permalink] [raw]
Subject: [net-next RFC PATCH v2 0/3] net: phy: support multi PHY in phy_driver Was: net: phy: detach PHY driver OPs from phy_driver struct

This is an alternative implementation of "net: phy: detach PHY driver OPs
from phy_driver struct" with the same object in mind.
v2 is used to keep track of the similar attempts but this is the 3rd try
to accomplish the same object.

As was pointed out in the previous series, deatching OPs is a way too big
change (although IMHO needed, but I understand the problem with downstream
and ugly code). As suggested and was already an idea discussed privately,
a more easier approach is introduce an alternative way in phy_driver
struct to declare PHY with the use of an array of IDs.

The second attempt to this had a fundamental problem, as pointed out by
Florian, it did cause an ABI change in sysfs. This was caused by the fact
that sysfs entry are created dased on the first name the PHY driver is
registreted and changing the dev name after (although wrong) also doesn't
update the sysfs name.

The only solution to this problem is to register one driver for each PHY
ID like it's done currently.

This was the case for attempt 1 (detached OPs) and is implemented here in
the 3rd attempt.

To accomplish this, the mdiodrv has to be moved in a separate struct and
defined for each PHY the phy_driver supports (this is already the case
for each phy_driver struct). With this change, we can keep the current
phy_driver struct and support defining multi PHY.

Each PHY will be registered as a separate driver, (even if they are defined
in the same phy_driver struct) permitting to register it directly
with the right name.

For single PHY implementation, the phy_driver is internally converted to
ids implementation by dynamically allocating the table with only one
entry.

This is needed to handle the move of mdiodrv from the phy_driver struct
to a more specific one for each PHY ID.

Changes v2:
- Drop c45 patch
- Complete rework to handle specifi names for PHYs (no ABI
regression)

Christian Marangi (3):
net: phy: add support for defining multiple PHY IDs in PHY driver
net: phy: aquantia: group common OPs for PHYs where possible
net: phy: bcm7xxx: rework phy_driver table to new multiple PHY ID
format

drivers/net/phy/aquantia/aquantia_main.c | 170 +++++++++--------------
drivers/net/phy/bcm7xxx.c | 140 +++++++++++--------
drivers/net/phy/phy_device.c | 127 ++++++++++++-----
include/linux/phy.h | 38 ++++-
4 files changed, 275 insertions(+), 200 deletions(-)

--
2.43.0



2024-02-20 19:52:16

by Christian Marangi

[permalink] [raw]
Subject: [net-next RFC PATCH v2 2/3] net: phy: aquantia: group common OPs for PHYs where possible

Group common OPS for PHY where possible by defining multiple PHYs for
similar PHY drivers instead of duplicating them for each PHY.

Signed-off-by: Christian Marangi <[email protected]>
---
drivers/net/phy/aquantia/aquantia_main.c | 170 +++++++++--------------
1 file changed, 64 insertions(+), 106 deletions(-)

diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index a6a7980585f5..e8c89fdbda94 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -770,16 +770,26 @@ static int aqr111_config_init(struct phy_device *phydev)

static struct phy_driver aqr_driver[] = {
{
- PHY_ID_MATCH_MODEL(PHY_ID_AQ1202),
- .name = "Aquantia AQ1202",
- .config_aneg = aqr_config_aneg,
- .config_intr = aqr_config_intr,
- .handle_interrupt = aqr_handle_interrupt,
- .read_status = aqr_read_status,
-},
-{
- PHY_ID_MATCH_MODEL(PHY_ID_AQ2104),
- .name = "Aquantia AQ2104",
+ .name = "Aquantia AQ1202/AQ2104/AQR106/AQR405",
+ .ids = (struct phy_driver_id []){
+ {
+ PHY_ID_MATCH_MODEL(PHY_ID_AQ1202),
+ .name = "Aquantia AQ1202"
+ },
+ {
+ PHY_ID_MATCH_MODEL(PHY_ID_AQ2104),
+ .name = "Aquantia AQ2104",
+ },
+ {
+ PHY_ID_MATCH_MODEL(PHY_ID_AQR106),
+ .name = "Aquantia AQR106",
+ },
+ {
+ PHY_ID_MATCH_MODEL(PHY_ID_AQR405),
+ .name = "Aquantia AQR405",
+ },
+ },
+ .ids_count = 4,
.config_aneg = aqr_config_aneg,
.config_intr = aqr_config_intr,
.handle_interrupt = aqr_handle_interrupt,
@@ -796,16 +806,22 @@ static struct phy_driver aqr_driver[] = {
.resume = aqr107_resume,
},
{
- PHY_ID_MATCH_MODEL(PHY_ID_AQR106),
- .name = "Aquantia AQR106",
- .config_aneg = aqr_config_aneg,
- .config_intr = aqr_config_intr,
- .handle_interrupt = aqr_handle_interrupt,
- .read_status = aqr_read_status,
-},
-{
- PHY_ID_MATCH_MODEL(PHY_ID_AQR107),
- .name = "Aquantia AQR107",
+ .name = "Aquantia AQR107/AQR112/AQR412",
+ .ids = (struct phy_driver_id []){
+ {
+ PHY_ID_MATCH_MODEL(PHY_ID_AQR107),
+ .name = "Aquantia AQR107"
+ },
+ {
+ PHY_ID_MATCH_MODEL(PHY_ID_AQR112),
+ .name = "Aquantia AQR112",
+ },
+ {
+ PHY_ID_MATCH_MODEL(PHY_ID_AQR412),
+ .name = "Aquantia AQR412",
+ },
+ },
+ .ids_count = 3,
.probe = aqr107_probe,
.get_rate_matching = aqr107_get_rate_matching,
.config_init = aqr107_config_init,
@@ -842,27 +858,22 @@ static struct phy_driver aqr_driver[] = {
.link_change_notify = aqr107_link_change_notify,
},
{
- PHY_ID_MATCH_MODEL(PHY_ID_AQR111),
- .name = "Aquantia AQR111",
- .probe = aqr107_probe,
- .get_rate_matching = aqr107_get_rate_matching,
- .config_init = aqr111_config_init,
- .config_aneg = aqr_config_aneg,
- .config_intr = aqr_config_intr,
- .handle_interrupt = aqr_handle_interrupt,
- .read_status = aqr107_read_status,
- .get_tunable = aqr107_get_tunable,
- .set_tunable = aqr107_set_tunable,
- .suspend = aqr107_suspend,
- .resume = aqr107_resume,
- .get_sset_count = aqr107_get_sset_count,
- .get_strings = aqr107_get_strings,
- .get_stats = aqr107_get_stats,
- .link_change_notify = aqr107_link_change_notify,
-},
-{
- PHY_ID_MATCH_MODEL(PHY_ID_AQR111B0),
- .name = "Aquantia AQR111B0",
+ .name = "Aquantia AQR111/AQR111B0",
+ .ids = (struct phy_driver_id []){
+ {
+ PHY_ID_MATCH_MODEL(PHY_ID_AQR111),
+ .name = "Aquantia AQR111"
+ },
+ {
+ PHY_ID_MATCH_MODEL(PHY_ID_AQR111B0),
+ .name = "Aquantia AQR111B0",
+ },
+ {
+ PHY_ID_MATCH_MODEL(PHY_ID_AQR106),
+ .name = "Aquantia AQR106",
+ },
+ },
+ .ids_count = 3,
.probe = aqr107_probe,
.get_rate_matching = aqr107_get_rate_matching,
.config_init = aqr111_config_init,
@@ -880,71 +891,18 @@ static struct phy_driver aqr_driver[] = {
.link_change_notify = aqr107_link_change_notify,
},
{
- PHY_ID_MATCH_MODEL(PHY_ID_AQR405),
- .name = "Aquantia AQR405",
- .config_aneg = aqr_config_aneg,
- .config_intr = aqr_config_intr,
- .handle_interrupt = aqr_handle_interrupt,
- .read_status = aqr_read_status,
-},
-{
- PHY_ID_MATCH_MODEL(PHY_ID_AQR112),
- .name = "Aquantia AQR112",
- .probe = aqr107_probe,
- .config_aneg = aqr_config_aneg,
- .config_intr = aqr_config_intr,
- .handle_interrupt = aqr_handle_interrupt,
- .get_tunable = aqr107_get_tunable,
- .set_tunable = aqr107_set_tunable,
- .suspend = aqr107_suspend,
- .resume = aqr107_resume,
- .read_status = aqr107_read_status,
- .get_rate_matching = aqr107_get_rate_matching,
- .get_sset_count = aqr107_get_sset_count,
- .get_strings = aqr107_get_strings,
- .get_stats = aqr107_get_stats,
- .link_change_notify = aqr107_link_change_notify,
-},
-{
- PHY_ID_MATCH_MODEL(PHY_ID_AQR412),
- .name = "Aquantia AQR412",
- .probe = aqr107_probe,
- .config_aneg = aqr_config_aneg,
- .config_intr = aqr_config_intr,
- .handle_interrupt = aqr_handle_interrupt,
- .get_tunable = aqr107_get_tunable,
- .set_tunable = aqr107_set_tunable,
- .suspend = aqr107_suspend,
- .resume = aqr107_resume,
- .read_status = aqr107_read_status,
- .get_rate_matching = aqr107_get_rate_matching,
- .get_sset_count = aqr107_get_sset_count,
- .get_strings = aqr107_get_strings,
- .get_stats = aqr107_get_stats,
- .link_change_notify = aqr107_link_change_notify,
-},
-{
- PHY_ID_MATCH_MODEL(PHY_ID_AQR113),
- .name = "Aquantia AQR113",
- .probe = aqr107_probe,
- .get_rate_matching = aqr107_get_rate_matching,
- .config_init = aqr113c_config_init,
- .config_aneg = aqr_config_aneg,
- .config_intr = aqr_config_intr,
- .handle_interrupt = aqr_handle_interrupt,
- .read_status = aqr107_read_status,
- .get_tunable = aqr107_get_tunable,
- .set_tunable = aqr107_set_tunable,
- .suspend = aqr107_suspend,
- .resume = aqr107_resume,
- .get_sset_count = aqr107_get_sset_count,
- .get_strings = aqr107_get_strings,
- .get_stats = aqr107_get_stats,
- .link_change_notify = aqr107_link_change_notify,
-},
-{
- PHY_ID_MATCH_MODEL(PHY_ID_AQR113C),
- .name = "Aquantia AQR113C",
+ .name = "Aquantia AQR113/AQR113C",
+ .ids = (struct phy_driver_id []){
+ {
+ PHY_ID_MATCH_MODEL(PHY_ID_AQR113),
+ .name = "Aquantia AQR113"
+ },
+ {
+ PHY_ID_MATCH_MODEL(PHY_ID_AQR113C),
+ .name = "Aquantia AQR113C",
+ },
+ },
+ .ids_count = 2,
.probe = aqr107_probe,
.get_rate_matching = aqr107_get_rate_matching,
.config_init = aqr113c_config_init,
--
2.43.0


2024-02-20 19:52:27

by Christian Marangi

[permalink] [raw]
Subject: [net-next RFC PATCH v2 1/3] net: phy: add support for defining multiple PHY IDs in PHY driver

Some PHY driver might implement the same OPs for different PHY ID and
using a mask is not enough to match similar PHYs.

To reduce code duplication, add support for defining multiple PHY IDs in
PHY driver struct.

Introduce a new variable in phy_driver struct, .ids, where a table array of
phy_driver_id can be defined to reference multiple PHY IDs (with their
own masks) supporting the same group of OPs and flags. PHY driver also
require to declare .ids_count to signal the amount of multiple PHYs the
phy_driver supports.

Introduce a new variable in phy_device, .drv_id, where the matching
phy_driver_id is stored.

To make this transparent and don't cause regression in ABI, mdiodrv
registration logic is changed. Each element in .ids is registreted as a
separate driver (mimicking them defined as an entire phy_driver), each
element then points to the common phy_driver struct to reduce code
duplication, permitting the correct driver name applied. Name of each
driver is taken by the .name in phy_driver_id.

If defined .name defined in phy_driver will be used for early PHY driver
registration.

Both implementation are mutually exclusive, phy_driver with .phy_id or
phy_id_mask and .ids are not supported and are rejected by the kernel
with a WARN as this is a bugged/wrong PHY driver.

to_phy_driver macro is updated to this new structure.

Single PHY ID implementation is still supported and ID is internally
converted to the .ids table with a single element.

Signed-off-by: Christian Marangi <[email protected]>
---
drivers/net/phy/phy_device.c | 127 ++++++++++++++++++++++++++---------
include/linux/phy.h | 38 +++++++++--
2 files changed, 129 insertions(+), 36 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index d63dca535746..7125e16f4c70 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -524,12 +524,13 @@ static int phy_scan_fixups(struct phy_device *phydev)

static int phy_bus_match(struct device *dev, struct device_driver *drv)
{
+ struct phy_driver_id *phydrv_id = to_phy_driver_id(drv);
struct phy_device *phydev = to_phy_device(dev);
- struct phy_driver *phydrv = to_phy_driver(drv);
+ struct phy_driver *phydrv = phydrv_id->driver;
const int num_ids = ARRAY_SIZE(phydev->c45_ids.device_ids);
int i;

- if (!(phydrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY))
+ if (!(phydrv_id->mdiodrv.flags & MDIO_DEVICE_IS_PHY))
return 0;

if (phydrv->match_phy_device)
@@ -541,13 +542,14 @@ static int phy_bus_match(struct device *dev, struct device_driver *drv)
continue;

if (phy_id_compare(phydev->c45_ids.device_ids[i],
- phydrv->phy_id, phydrv->phy_id_mask))
+ phydrv_id->phy_id, phydrv_id->phy_id_mask))
return 1;
}
+
return 0;
} else {
- return phy_id_compare(phydev->phy_id, phydrv->phy_id,
- phydrv->phy_id_mask);
+ return phy_id_compare(phydev->phy_id, phydrv_id->phy_id,
+ phydrv_id->phy_id_mask);
}
}

@@ -1465,9 +1467,9 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
*/
if (!d->driver) {
if (phydev->is_c45)
- d->driver = &genphy_c45_driver.mdiodrv.driver;
+ d->driver = &genphy_c45_driver.ids->mdiodrv.driver;
else
- d->driver = &genphy_driver.mdiodrv.driver;
+ d->driver = &genphy_driver.ids->mdiodrv.driver;

using_genphy = true;
}
@@ -1649,14 +1651,14 @@ static bool phy_driver_is_genphy_kind(struct phy_device *phydev,
bool phy_driver_is_genphy(struct phy_device *phydev)
{
return phy_driver_is_genphy_kind(phydev,
- &genphy_driver.mdiodrv.driver);
+ &genphy_driver.ids->mdiodrv.driver);
}
EXPORT_SYMBOL_GPL(phy_driver_is_genphy);

bool phy_driver_is_genphy_10g(struct phy_device *phydev)
{
return phy_driver_is_genphy_kind(phydev,
- &genphy_c45_driver.mdiodrv.driver);
+ &genphy_c45_driver.ids->mdiodrv.driver);
}
EXPORT_SYMBOL_GPL(phy_driver_is_genphy_10g);

@@ -3409,9 +3411,11 @@ static int phy_probe(struct device *dev)
{
struct phy_device *phydev = to_phy_device(dev);
struct device_driver *drv = phydev->mdio.dev.driver;
- struct phy_driver *phydrv = to_phy_driver(drv);
+ struct phy_driver_id *id = to_phy_driver_id(drv);
+ struct phy_driver *phydrv = id->driver;
int err = 0;

+ phydev->drv_id = id;
phydev->drv = phydrv;

/* Disable the interrupt if the PHY doesn't support it
@@ -3551,6 +3555,42 @@ static int phy_remove(struct device *dev)
return 0;
}

+static int phy_driver_mdiodrv_register(struct mdio_driver_common *mdiodrv, char *name,
+ struct module *owner)
+{
+ int retval;
+
+ /* PHYLIB device drivers must not match using a DT compatible table
+ * as this bypasses our checks that the mdiodev that is being matched
+ * is backed by a struct phy_device. If such a case happens, we will
+ * make out-of-bounds accesses and lockup in phydev->lock.
+ */
+ if (WARN(mdiodrv->driver.of_match_table,
+ "%s: driver must not provide a DT match table\n",
+ name))
+ return -EINVAL;
+
+ mdiodrv->flags |= MDIO_DEVICE_IS_PHY;
+ mdiodrv->driver.name = name;
+ mdiodrv->driver.bus = &mdio_bus_type;
+ mdiodrv->driver.probe = phy_probe;
+ mdiodrv->driver.remove = phy_remove;
+ mdiodrv->driver.owner = owner;
+ mdiodrv->driver.probe_type = PROBE_FORCE_SYNCHRONOUS;
+
+ retval = driver_register(&mdiodrv->driver);
+ if (retval) {
+ pr_err("%s: Error %d in registering driver\n",
+ name, retval);
+
+ return retval;
+ }
+
+ pr_debug("%s: Registered new driver\n", name);
+
+ return 0;
+}
+
/**
* phy_driver_register - register a phy_driver with the PHY layer
* @new_driver: new phy_driver to register
@@ -3558,7 +3598,7 @@ static int phy_remove(struct device *dev)
*/
int phy_driver_register(struct phy_driver *new_driver, struct module *owner)
{
- int retval;
+ int i, retval;

/* Either the features are hard coded, or dynamically
* determined. It cannot be both.
@@ -3569,33 +3609,47 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner)
return -EINVAL;
}

- /* PHYLIB device drivers must not match using a DT compatible table
- * as this bypasses our checks that the mdiodev that is being matched
- * is backed by a struct phy_device. If such a case happens, we will
- * make out-of-bounds accesses and lockup in phydev->lock.
+ /* Either PHY driver define multiple PHY IDs, or a single one.
+ * It cannot be both.
*/
- if (WARN(new_driver->mdiodrv.driver.of_match_table,
- "%s: driver must not provide a DT match table\n",
- new_driver->name))
+ if (WARN_ON((new_driver->phy_id || new_driver->phy_id_mask) &&
+ new_driver->ids)) {
+ pr_err("%s: phy_id or phy_id_mask and ids table must not both be set\n",
+ new_driver->name);
return -EINVAL;
+ }

- new_driver->mdiodrv.flags |= MDIO_DEVICE_IS_PHY;
- new_driver->mdiodrv.driver.name = new_driver->name;
- new_driver->mdiodrv.driver.bus = &mdio_bus_type;
- new_driver->mdiodrv.driver.probe = phy_probe;
- new_driver->mdiodrv.driver.remove = phy_remove;
- new_driver->mdiodrv.driver.owner = owner;
- new_driver->mdiodrv.driver.probe_type = PROBE_FORCE_SYNCHRONOUS;
+ /* Dynamically allocate ids table for PHY driver that define
+ * a single PHY ID.
+ */
+ if (!new_driver->ids) {
+ struct phy_driver_id *id;

- retval = driver_register(&new_driver->mdiodrv.driver);
- if (retval) {
- pr_err("%s: Error %d in registering driver\n",
- new_driver->name, retval);
+ id = kzalloc(sizeof(*new_driver->ids), GFP_KERNEL);
+ if (!id)
+ return -ENOMEM;

- return retval;
+ id->name = new_driver->name;
+ id->phy_id = new_driver->phy_id;
+ id->phy_id_mask = new_driver->phy_id_mask;
+ new_driver->ids = id;
+ new_driver->ids_count = 1;
}

- pr_debug("%s: Registered new driver\n", new_driver->name);
+ for (i = 0; i < new_driver->ids_count; i++) {
+ struct phy_driver_id *ids = new_driver->ids;
+
+ /* Attach the phy_driver to the phy_driver_id */
+ ids[i].driver = new_driver;
+ retval = phy_driver_mdiodrv_register(&ids[i].mdiodrv,
+ ids[i].name, owner);
+ if (retval) {
+ while (i-- > 0)
+ driver_unregister(&ids[i].mdiodrv.driver);
+
+ return retval;
+ }
+ }

return 0;
}
@@ -3620,7 +3674,16 @@ EXPORT_SYMBOL(phy_drivers_register);

void phy_driver_unregister(struct phy_driver *drv)
{
- driver_unregister(&drv->mdiodrv.driver);
+ int i;
+
+ for (i = 0; i < drv->ids_count; i++)
+ driver_unregister(&drv->ids[i].mdiodrv.driver);
+
+ /* With phy_id or phy_id_mask set in phy_driver, assume
+ * dynamically allocated ids table.
+ */
+ if (drv->phy_id || drv->phy_id_mask)
+ kfree(drv->ids);
}
EXPORT_SYMBOL(phy_driver_unregister);

diff --git a/include/linux/phy.h b/include/linux/phy.h
index c2dda21b39e1..f341a9867f19 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -537,6 +537,7 @@ struct phy_c45_device_ids {
u32 device_ids[MDIO_MMD_NUM];
};

+struct phy_driver_id;
struct macsec_context;
struct macsec_ops;

@@ -544,6 +545,7 @@ struct macsec_ops;
* struct phy_device - An instance of a PHY
*
* @mdio: MDIO bus this PHY is on
+ * @drv_id: Pointer to the driver ID for this PHY instance
* @drv: Pointer to the driver for this PHY instance
* @devlink: Create a link between phy dev and mac dev, if the external phy
* used by current mac interface is managed by another mac interface.
@@ -641,6 +643,7 @@ struct phy_device {

/* Information about the PHY type */
/* And management functions */
+ const struct phy_driver_id *drv_id;
const struct phy_driver *drv;

struct device_link *devlink;
@@ -881,9 +884,32 @@ struct phy_led {

#define to_phy_led(d) container_of(d, struct phy_led, led_cdev)

+/**
+ * struct phy_driver_id - Driver structure id for a particular PHY type
+ * @mdiodrv: Data common to all MDIO devices
+ * @phy_id: The result of reading the UID registers of this PHY
+ * type, and ANDing them with the phy_id_mask. This driver
+ * only works for PHYs with IDs which match this field
+ * @name: The friendly name of this PHY type
+ * @phy_id_mask: Defines the important bits of the phy_id
+ *
+ * @driver: Pointer to the associated PHY driver
+ */
+struct phy_driver_id {
+ struct mdio_driver_common mdiodrv;
+ u32 phy_id;
+ char *name;
+ u32 phy_id_mask;
+
+ struct phy_driver *driver;
+};
+
/**
* struct phy_driver - Driver structure for a particular PHY type
*
+ * @ids: array of mdio device IDs to match this driver (terminated with
+ * zero phy_id_mask)
+ * @ids_count: count of mdio device IDs (not counting the sentinel)
* @mdiodrv: Data common to all MDIO devices
* @phy_id: The result of reading the UID registers of this PHY
* type, and ANDing them with the phy_id_mask. This driver
@@ -905,7 +931,8 @@ struct phy_led {
* though it is not currently supported in the driver).
*/
struct phy_driver {
- struct mdio_driver_common mdiodrv;
+ struct phy_driver_id *ids;
+ u32 ids_count;
u32 phy_id;
char *name;
u32 phy_id_mask;
@@ -1171,8 +1198,10 @@ struct phy_driver {
int (*led_polarity_set)(struct phy_device *dev, int index,
unsigned long modes);
};
-#define to_phy_driver(d) container_of(to_mdio_common_driver(d), \
- struct phy_driver, mdiodrv)
+
+#define to_phy_driver_id(d) container_of(to_mdio_common_driver(d), \
+ struct phy_driver_id, mdiodrv)
+#define to_phy_driver(d) to_phy_driver_id(d)->driver

#define PHY_ANY_ID "MATCH ANY PHY"
#define PHY_ANY_UID 0xffffffff
@@ -1206,7 +1235,8 @@ static inline bool phy_id_compare(u32 id1, u32 id2, u32 mask)
*/
static inline bool phydev_id_compare(struct phy_device *phydev, u32 id)
{
- return phy_id_compare(id, phydev->phy_id, phydev->drv->phy_id_mask);
+ return phy_id_compare(id, phydev->phy_id,
+ phydev->drv_id->phy_id_mask);
}

/* A Structure for boards to register fixups with the PHY Lib */
--
2.43.0


2024-02-20 19:52:30

by Christian Marangi

[permalink] [raw]
Subject: [net-next RFC PATCH v2 3/3] net: phy: bcm7xxx: rework phy_driver table to new multiple PHY ID format

Rework bcm7xxx PHY driver table to new multiple PHY format
implementation to reduce code duplication and final size of the compiled
module.

Signed-off-by: Christian Marangi <[email protected]>
---
drivers/net/phy/bcm7xxx.c | 140 ++++++++++++++++++++++----------------
1 file changed, 82 insertions(+), 58 deletions(-)

diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index 97638ba7ae85..6ff09c92f7fa 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -845,16 +845,6 @@ static int bcm7xxx_28nm_probe(struct phy_device *phydev)
.phy_id = (_oui), \
.phy_id_mask = 0xfffffff0, \
.name = _name, \
- /* PHY_GBIT_FEATURES */ \
- .flags = PHY_IS_INTERNAL, \
- .config_init = bcm7xxx_28nm_config_init, \
- .resume = bcm7xxx_28nm_resume, \
- .get_tunable = bcm7xxx_28nm_get_tunable, \
- .set_tunable = bcm7xxx_28nm_set_tunable, \
- .get_sset_count = bcm_phy_get_sset_count, \
- .get_strings = bcm_phy_get_strings, \
- .get_stats = bcm7xxx_28nm_get_phy_stats, \
- .probe = bcm7xxx_28nm_probe, \
}

#define BCM7XXX_28NM_EPHY(_oui, _name) \
@@ -862,16 +852,6 @@ static int bcm7xxx_28nm_probe(struct phy_device *phydev)
.phy_id = (_oui), \
.phy_id_mask = 0xfffffff0, \
.name = _name, \
- /* PHY_BASIC_FEATURES */ \
- .flags = PHY_IS_INTERNAL, \
- .config_init = bcm7xxx_28nm_ephy_config_init, \
- .resume = bcm7xxx_28nm_ephy_resume, \
- .get_sset_count = bcm_phy_get_sset_count, \
- .get_strings = bcm_phy_get_strings, \
- .get_stats = bcm7xxx_28nm_get_phy_stats, \
- .probe = bcm7xxx_28nm_probe, \
- .read_mmd = bcm7xxx_28nm_ephy_read_mmd, \
- .write_mmd = bcm7xxx_28nm_ephy_write_mmd, \
}

#define BCM7XXX_40NM_EPHY(_oui, _name) \
@@ -879,12 +859,6 @@ static int bcm7xxx_28nm_probe(struct phy_device *phydev)
.phy_id = (_oui), \
.phy_id_mask = 0xfffffff0, \
.name = _name, \
- /* PHY_BASIC_FEATURES */ \
- .flags = PHY_IS_INTERNAL, \
- .soft_reset = genphy_soft_reset, \
- .config_init = bcm7xxx_config_init, \
- .suspend = bcm7xxx_suspend, \
- .resume = bcm7xxx_config_init, \
}

#define BCM7XXX_16NM_EPHY(_oui, _name) \
@@ -892,41 +866,91 @@ static int bcm7xxx_28nm_probe(struct phy_device *phydev)
.phy_id = (_oui), \
.phy_id_mask = 0xfffffff0, \
.name = _name, \
- /* PHY_BASIC_FEATURES */ \
- .flags = PHY_IS_INTERNAL, \
- .get_sset_count = bcm_phy_get_sset_count, \
- .get_strings = bcm_phy_get_strings, \
- .get_stats = bcm7xxx_28nm_get_phy_stats, \
- .probe = bcm7xxx_28nm_probe, \
- .config_init = bcm7xxx_16nm_ephy_config_init, \
- .config_aneg = genphy_config_aneg, \
- .read_status = genphy_read_status, \
- .resume = bcm7xxx_16nm_ephy_resume, \
}

static struct phy_driver bcm7xxx_driver[] = {
- BCM7XXX_28NM_EPHY(PHY_ID_BCM72113, "Broadcom BCM72113"),
- BCM7XXX_28NM_EPHY(PHY_ID_BCM72116, "Broadcom BCM72116"),
- BCM7XXX_16NM_EPHY(PHY_ID_BCM72165, "Broadcom BCM72165"),
- BCM7XXX_28NM_GPHY(PHY_ID_BCM7250, "Broadcom BCM7250"),
- BCM7XXX_28NM_EPHY(PHY_ID_BCM7255, "Broadcom BCM7255"),
- BCM7XXX_28NM_EPHY(PHY_ID_BCM7260, "Broadcom BCM7260"),
- BCM7XXX_28NM_EPHY(PHY_ID_BCM7268, "Broadcom BCM7268"),
- BCM7XXX_28NM_EPHY(PHY_ID_BCM7271, "Broadcom BCM7271"),
- BCM7XXX_28NM_GPHY(PHY_ID_BCM7278, "Broadcom BCM7278"),
- BCM7XXX_28NM_GPHY(PHY_ID_BCM7364, "Broadcom BCM7364"),
- BCM7XXX_28NM_GPHY(PHY_ID_BCM7366, "Broadcom BCM7366"),
- BCM7XXX_16NM_EPHY(PHY_ID_BCM74165, "Broadcom BCM74165"),
- BCM7XXX_28NM_GPHY(PHY_ID_BCM74371, "Broadcom BCM74371"),
- BCM7XXX_28NM_GPHY(PHY_ID_BCM7439, "Broadcom BCM7439"),
- BCM7XXX_28NM_GPHY(PHY_ID_BCM7439_2, "Broadcom BCM7439 (2)"),
- BCM7XXX_28NM_GPHY(PHY_ID_BCM7445, "Broadcom BCM7445"),
- BCM7XXX_40NM_EPHY(PHY_ID_BCM7346, "Broadcom BCM7346"),
- BCM7XXX_40NM_EPHY(PHY_ID_BCM7362, "Broadcom BCM7362"),
- BCM7XXX_40NM_EPHY(PHY_ID_BCM7425, "Broadcom BCM7425"),
- BCM7XXX_40NM_EPHY(PHY_ID_BCM7429, "Broadcom BCM7429"),
- BCM7XXX_40NM_EPHY(PHY_ID_BCM7435, "Broadcom BCM7435"),
- BCM7XXX_16NM_EPHY(PHY_ID_BCM7712, "Broadcom BCM7712"),
+{
+ .name = "Broadcom BCM7XXX 16NM EPHY",
+ .ids = (struct phy_driver_id []){
+ BCM7XXX_16NM_EPHY(PHY_ID_BCM72165, "Broadcom BCM72165"),
+ BCM7XXX_16NM_EPHY(PHY_ID_BCM74165, "Broadcom BCM74165"),
+ BCM7XXX_16NM_EPHY(PHY_ID_BCM7712, "Broadcom BCM7712"),
+ },
+ .ids_count = 3,
+ /* PHY_BASIC_FEATURES */
+ .flags = PHY_IS_INTERNAL,
+ .get_sset_count = bcm_phy_get_sset_count,
+ .get_strings = bcm_phy_get_strings,
+ .get_stats = bcm7xxx_28nm_get_phy_stats,
+ .probe = bcm7xxx_28nm_probe,
+ .config_init = bcm7xxx_16nm_ephy_config_init,
+ .config_aneg = genphy_config_aneg,
+ .read_status = genphy_read_status,
+ .resume = bcm7xxx_16nm_ephy_resume,
+},
+{
+ .name = "Broadcom BCM7XXX 28NM GPHY",
+ .ids = (struct phy_driver_id []){
+ BCM7XXX_28NM_GPHY(PHY_ID_BCM7250, "Broadcom BCM7250"),
+ BCM7XXX_28NM_GPHY(PHY_ID_BCM7278, "Broadcom BCM7278"),
+ BCM7XXX_28NM_GPHY(PHY_ID_BCM7364, "Broadcom BCM7364"),
+ BCM7XXX_28NM_GPHY(PHY_ID_BCM7366, "Broadcom BCM7366"),
+ BCM7XXX_28NM_GPHY(PHY_ID_BCM74371, "Broadcom BCM74371"),
+ BCM7XXX_28NM_GPHY(PHY_ID_BCM7439, "Broadcom BCM7439"),
+ BCM7XXX_28NM_GPHY(PHY_ID_BCM7439_2, "Broadcom BCM7439 (2)"),
+ BCM7XXX_28NM_GPHY(PHY_ID_BCM7445, "Broadcom BCM7445"),
+ },
+ .ids_count = 8,
+ /* PHY_GBIT_FEATURES */
+ .flags = PHY_IS_INTERNAL,
+ .config_init = bcm7xxx_28nm_config_init,
+ .resume = bcm7xxx_28nm_resume,
+ .get_tunable = bcm7xxx_28nm_get_tunable,
+ .set_tunable = bcm7xxx_28nm_set_tunable,
+ .get_sset_count = bcm_phy_get_sset_count,
+ .get_strings = bcm_phy_get_strings,
+ .get_stats = bcm7xxx_28nm_get_phy_stats,
+ .probe = bcm7xxx_28nm_probe,
+},
+{
+ .name = "Broadcom BCM7XXX 28NM EPHY",
+ .ids = (struct phy_driver_id []){
+ BCM7XXX_28NM_EPHY(PHY_ID_BCM72113, "Broadcom BCM72113"),
+ BCM7XXX_28NM_EPHY(PHY_ID_BCM72116, "Broadcom BCM72116"),
+ BCM7XXX_28NM_EPHY(PHY_ID_BCM7255, "Broadcom BCM7255"),
+ BCM7XXX_28NM_EPHY(PHY_ID_BCM7260, "Broadcom BCM7260"),
+ BCM7XXX_28NM_EPHY(PHY_ID_BCM7268, "Broadcom BCM7268"),
+ BCM7XXX_28NM_EPHY(PHY_ID_BCM7271, "Broadcom BCM7271"),
+ },
+ .ids_count = 6,
+ /* PHY_BASIC_FEATURES */
+ .flags = PHY_IS_INTERNAL,
+ .config_init = bcm7xxx_28nm_ephy_config_init,
+ .resume = bcm7xxx_28nm_ephy_resume,
+ .get_sset_count = bcm_phy_get_sset_count,
+ .get_strings = bcm_phy_get_strings,
+ .get_stats = bcm7xxx_28nm_get_phy_stats,
+ .probe = bcm7xxx_28nm_probe,
+ .read_mmd = bcm7xxx_28nm_ephy_read_mmd,
+ .write_mmd = bcm7xxx_28nm_ephy_write_mmd,
+},
+{
+ .name = "Broadcom BCM7XXX 40NM EPHY",
+ .ids = (struct phy_driver_id []){
+ BCM7XXX_40NM_EPHY(PHY_ID_BCM7346, "Broadcom BCM7346"),
+ BCM7XXX_40NM_EPHY(PHY_ID_BCM7362, "Broadcom BCM7362"),
+ BCM7XXX_40NM_EPHY(PHY_ID_BCM7425, "Broadcom BCM7425"),
+ BCM7XXX_40NM_EPHY(PHY_ID_BCM7429, "Broadcom BCM7429"),
+ BCM7XXX_40NM_EPHY(PHY_ID_BCM7435, "Broadcom BCM7435"),
+ },
+ .ids_count = 5,
+ /* PHY_BASIC_FEATURES */
+ .flags = PHY_IS_INTERNAL,
+ .soft_reset = genphy_soft_reset,
+ .config_init = bcm7xxx_config_init,
+ .suspend = bcm7xxx_suspend,
+ .resume = bcm7xxx_config_init,
+},
};

static struct mdio_device_id __maybe_unused bcm7xxx_tbl[] = {
--
2.43.0