Skip to contentSkip to conversation

net: phy: refactoring and Micrel features

PATCH In review Johan Hovold 5 participants 32 messages Nov 11, 2014 – Nov 14, 2014

Subsystems (5)
Networking Network drivers Device tree bindings Ethernet PHY +1 more subsystems
lore
Share & tools
lore

Johan Hovold

author
#0 lore raw

cover net: phy: refactoring and Micrel features

This series fixes a couple of minor issues, refactors phy-driver registration, adds device and device-type abstractions to the micrel driver, and adds support for broadcast disable, led-mode and RMII-reference clock selection for KSZ8081 and KSZ8091 devices.

While adding support for more features for the Micrel PHYs mentioned above, it became apparent that the configuration space is much too large and that adding type-specific callbacks will simply not scale. Instead I added a driver_data field to struct phy_device, which can be used to store static device type data that can be parsed and acted on in generic driver callbacks instead. This allows a lot of duplicated code to be removed, and should make it much easier to add new features or deal with device-type quirks in the future.

This series has been tested on a dual KSZ8081 setup (which wasn't supported until now). Further testing on other Micrel PHYs would be much appreciated.

Note that the final patch effectively reverts the recent commit a95a18afe4c8 ("phy/micrel: KSZ8031RNL RMII clock reconfiguration bug"), which claims that some KSZ8031 PHY become unresponsive if the broadcast disable flag is set before configuring the clock mode, even though disabling the broadcast address is generally the first thing you want to do (e.g. to avoid unnecessary reconfigurations).

Hopefully, the author of that commit (on CC) can confirm whether this is indeed the case, and if so the final patch can be dropped unless it can be worked around in a different way (which I suspect).

Thanks,
Johan

Johan Hovold (22):
dt/bindings: fix documentation of ethernet-phy compatible property
net: phy: add module_phy_driver macro
net: phy: replace phy_driver_register calls
net: phy: replace phy_drivers_register calls
net: phy: micrel: fix config_intr error handling
net: phy: micrel: use BIT macro
net: phy: micrel: refactor broadcast disable
net: phy: micrel: disable broadcast for KSZ8081/KSZ8091
net: phy: micrel: add led-mode sanity check
net: phy: micrel: refactor led-mode error handling
net: phy: micrel: clean up led-mode setup
net: phy: micrel: enable led-mode for KSZ8081/KSZ8091
net: phy: add static data field to struct phy_driver
net: phy: micrel: add device-type abstraction
net: phy: micrel: parse of nodes at probe
net: phy: micrel: add has-broadcast-disable flag to type data
net: phy: micrel: add generic rmii-ref-clk-sel support
net: phy: micrel: add support for rmii_ref_clk_sel to KSZ8081/KSZ8091 dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding
net: phy: micrel: refactor interrupt config
net: phy: micrel: add copyright entry
net: phy: micrel: use generic config_init for KSZ8021/KSZ8031

 Documentation/devicetree/bindings/net/micrel.txt |  11 +-
 Documentation/devicetree/bindings/net/phy.txt    |   3 +-
 drivers/net/phy/amd-xgbe-phy.c                   |  15 +-
 drivers/net/phy/amd.c                            |  17 +-
 drivers/net/phy/at803x.c                         |  14 +-
 drivers/net/phy/bcm63xx.c                        |  15 +-
 drivers/net/phy/bcm7xxx.c                        |  15 +-
 drivers/net/phy/bcm87xx.c                        |  14 +-
 drivers/net/phy/broadcom.c                       |  15 +-
 drivers/net/phy/cicada.c                         |  15 +-
 drivers/net/phy/davicom.c                        |  15 +-
 drivers/net/phy/et1011c.c                        |  17 +-
 drivers/net/phy/icplus.c                         |  15 +-
 drivers/net/phy/lxt.c                            |  15 +-
 drivers/net/phy/marvell.c                        |  15 +-
 drivers/net/phy/micrel.c                         | 346 ++++++++++++++---------
 drivers/net/phy/national.c                       |  17 +-
 drivers/net/phy/qsemi.c                          |  17 +-
 drivers/net/phy/realtek.c                        |  13 +-
 drivers/net/phy/smsc.c                           |  14 +-
 drivers/net/phy/ste10Xp.c                        |  15 +-
 drivers/net/phy/vitesse.c                        |  14 +-
 include/linux/micrel_phy.h                       |   1 -
 include/linux/phy.h                              |  26 ++
 24 files changed, 278 insertions(+), 396 deletions(-)
Signature · 1 line
2.0.4
author
#1 lore raw

PATCH 1/22 dt/bindings: fix documentation of ethernet-phy compatible property

A recent commit extended the documentation of the ethernet-phy compatible property, but placed the new paragraph under the max-speed property.

Fixes f00e756ed12d ("dt: Document a compatible entry for MDIO ethernet

Phys")

Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/phy.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/net/phy.txt +2 -1
index 5b8c58903077..40831fbaff72 100644
@@ -19,7 +19,6 @@ Optional Properties:
   specifications. If neither of these are specified, the default is to
   assume clause 22. The compatible list may also contain other
   elements.
-- max-speed: Maximum PHY supported speed (10, 100, 1000...)
 
   If the phy's identifier is known then the list may contain an entry
   of the form: "ethernet-phy-idAAAA.BBBB" where

@@ -29,6 +28,8 @@ Optional Properties:
             4 hex digits. This is the chip vendor OUI bits 19:24,
             followed by 10 bits of a vendor specific ID.
 
+- max-speed: Maximum PHY supported speed (10, 100, 1000...)
+
 Example:
 
 ethernet-phy@0 {
Signature · 6 lines
2.0.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#2 lore raw

PATCH 2/22 net: phy: add module_phy_driver macro

In reply to Johan Hovold

Add helper macro for PHY drivers which do not do anything special in module init/exit. This will allow us to eliminate a lot of boilerplate code.

Signed-off-by Johan Hovold <[email protected]>
---
 include/linux/phy.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
include/linux/phy.h +24 -0
index ed39956b5613..c50e1f1f46e0 100644
@@ -745,4 +745,28 @@ int __init mdio_bus_init(void);
 void mdio_bus_exit(void);
 
 extern struct bus_type mdio_bus_type;
+
+/**
+ * module_phy_driver() - Helper macro for registering PHY drivers
+ * @__phy_drivers: array of PHY drivers to register
+ *
+ * Helper macro for PHY drivers which do not do anything special in module
+ * init/exit. Each module may only use this macro once, and calling it
+ * replaces module_init() and module_exit().
+ */
+#define phy_module_driver(__phy_drivers, __count)			\
+static int __init phy_module_init(void)					\
+{									\
+	return phy_drivers_register(__phy_drivers, __count);		\
+}									\
+module_init(phy_module_init);						\
+static void __exit phy_module_exit(void)				\
+{									\
+	phy_drivers_unregister(__phy_drivers, __count);			\
+}									\
+module_exit(phy_module_exit)
+
+#define module_phy_driver(__phy_drivers)				\
+	phy_module_driver(__phy_drivers, ARRAY_SIZE(__phy_drivers))
+
 #endif /* __PHY_H */
Signature · 1 line
2.0.4
author
#3 lore raw

PATCH 3/22 net: phy: replace phy_driver_register calls

In reply to Johan Hovold

Replace module init/exit which only calls phy_driver_register with module_phy_driver macro.

Compile tested only.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/amd.c      | 17 +++--------------
 drivers/net/phy/et1011c.c  | 17 +++--------------
 drivers/net/phy/national.c | 17 +++--------------
 drivers/net/phy/qsemi.c    | 17 +++--------------
 4 files changed, 12 insertions(+), 56 deletions(-)
drivers/net/phy/amd.c +3 -14
index a3fb5ceb6487..65a488f82eb8 100644
@@ -61,7 +61,7 @@ static int am79c_config_intr(struct phy_device *phydev)
 	return err;
 }
 
-static struct phy_driver am79c_driver = {
+static struct phy_driver am79c_driver[] = { {
 	.phy_id		= PHY_ID_AM79C874,
 	.name		= "AM79C874",
 	.phy_id_mask	= 0xfffffff0,

@@ -73,20 +73,9 @@ static struct phy_driver am79c_driver = {
 	.ack_interrupt	= am79c_ack_interrupt,
 	.config_intr	= am79c_config_intr,
 	.driver		= { .owner = THIS_MODULE,},
-};
-
-static int __init am79c_init(void)
-{
-	return phy_driver_register(&am79c_driver);
-}
-
-static void __exit am79c_exit(void)
-{
-	phy_driver_unregister(&am79c_driver);
-}
+} };
 
-module_init(am79c_init);
-module_exit(am79c_exit);
+module_phy_driver(am79c_driver);
 
 static struct mdio_device_id __maybe_unused amd_tbl[] = {
 	{ PHY_ID_AM79C874, 0xfffffff0 },
drivers/net/phy/et1011c.c +3 -14
index a8eb19ec3183..a907743816a8 100644
@@ -87,7 +87,7 @@ static int et1011c_read_status(struct phy_device *phydev)
 	return ret;
 }
 
-static struct phy_driver et1011c_driver = {
+static struct phy_driver et1011c_driver[] = { {
 	.phy_id		= 0x0282f014,
 	.name		= "ET1011C",
 	.phy_id_mask	= 0xfffffff0,

@@ -96,20 +96,9 @@ static struct phy_driver et1011c_driver = {
 	.config_aneg	= et1011c_config_aneg,
 	.read_status	= et1011c_read_status,
 	.driver 	= { .owner = THIS_MODULE,},
-};
-
-static int __init et1011c_init(void)
-{
-	return phy_driver_register(&et1011c_driver);
-}
-
-static void __exit et1011c_exit(void)
-{
-	phy_driver_unregister(&et1011c_driver);
-}
+} };
 
-module_init(et1011c_init);
-module_exit(et1011c_exit);
+module_phy_driver(et1011c_driver);
 
 static struct mdio_device_id __maybe_unused et1011c_tbl[] = {
 	{ 0x0282f014, 0xfffffff0 },
drivers/net/phy/national.c +3 -14
index 9a5f234d95b0..0a7b9c7f09a2 100644
@@ -129,7 +129,7 @@ static int ns_config_init(struct phy_device *phydev)
 	return ns_ack_interrupt(phydev);
 }
 
-static struct phy_driver dp83865_driver = {
+static struct phy_driver dp83865_driver[] = { {
 	.phy_id = DP83865_PHY_ID,
 	.phy_id_mask = 0xfffffff0,
 	.name = "NatSemi DP83865",

@@ -141,25 +141,14 @@ static struct phy_driver dp83865_driver = {
 	.ack_interrupt = ns_ack_interrupt,
 	.config_intr = ns_config_intr,
 	.driver = {.owner = THIS_MODULE,}
-};
+} };
 
-static int __init ns_init(void)
-{
-	return phy_driver_register(&dp83865_driver);
-}
-
-static void __exit ns_exit(void)
-{
-	phy_driver_unregister(&dp83865_driver);
-}
+module_phy_driver(dp83865_driver);
 
 MODULE_DESCRIPTION("NatSemi PHY driver");
 MODULE_AUTHOR("Stuart Menefy");
 MODULE_LICENSE("GPL");
 
-module_init(ns_init);
-module_exit(ns_exit);
-
 static struct mdio_device_id __maybe_unused ns_tbl[] = {
 	{ DP83865_PHY_ID, 0xfffffff0 },
 	{ }
drivers/net/phy/qsemi.c +3 -14
index fe0d0a15d5e1..be4c6f7c3645 100644
@@ -111,7 +111,7 @@ static int qs6612_config_intr(struct phy_device *phydev)
 
 }
 
-static struct phy_driver qs6612_driver = {
+static struct phy_driver qs6612_driver[] = { {
 	.phy_id		= 0x00181440,
 	.name		= "QS6612",
 	.phy_id_mask	= 0xfffffff0,

@@ -123,20 +123,9 @@ static struct phy_driver qs6612_driver = {
 	.ack_interrupt	= qs6612_ack_interrupt,
 	.config_intr	= qs6612_config_intr,
 	.driver 	= { .owner = THIS_MODULE,},
-};
-
-static int __init qs6612_init(void)
-{
-	return phy_driver_register(&qs6612_driver);
-}
-
-static void __exit qs6612_exit(void)
-{
-	phy_driver_unregister(&qs6612_driver);
-}
+} };
 
-module_init(qs6612_init);
-module_exit(qs6612_exit);
+module_phy_driver(qs6612_driver);
 
 static struct mdio_device_id __maybe_unused qs6612_tbl[] = {
 	{ 0x00181440, 0xfffffff0 },
Signature · 1 line
2.0.4
author
#4 lore raw

PATCH 4/22 net: phy: replace phy_drivers_register calls

In reply to Johan Hovold

Replace module init/exit which only calls phy_drivers_register with module_phy_driver macro.

Tested using Micrel driver, and otherwise compile-tested only.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/amd-xgbe-phy.c | 15 +--------------
 drivers/net/phy/at803x.c       | 14 +-------------
 drivers/net/phy/bcm63xx.c      | 15 +--------------
 drivers/net/phy/bcm7xxx.c      | 15 +--------------
 drivers/net/phy/bcm87xx.c      | 14 +-------------
 drivers/net/phy/broadcom.c     | 15 +--------------
 drivers/net/phy/cicada.c       | 15 +--------------
 drivers/net/phy/davicom.c      | 15 +--------------
 drivers/net/phy/icplus.c       | 15 +--------------
 drivers/net/phy/lxt.c          | 15 +--------------
 drivers/net/phy/marvell.c      | 15 +--------------
 drivers/net/phy/micrel.c       | 15 +--------------
 drivers/net/phy/realtek.c      | 13 +------------
 drivers/net/phy/smsc.c         | 14 +-------------
 drivers/net/phy/ste10Xp.c      | 15 +--------------
 drivers/net/phy/vitesse.c      | 14 +-------------
 16 files changed, 16 insertions(+), 218 deletions(-)
drivers/net/phy/amd-xgbe-phy.c +1 -14
index f3230eef41fd..52e7427c0a38 100644
@@ -1435,20 +1435,7 @@ static struct phy_driver amd_xgbe_phy_driver[] = {
 	},
 };
 
-static int __init amd_xgbe_phy_init(void)
-{
-	return phy_drivers_register(amd_xgbe_phy_driver,
-				    ARRAY_SIZE(amd_xgbe_phy_driver));
-}
-
-static void __exit amd_xgbe_phy_exit(void)
-{
-	phy_drivers_unregister(amd_xgbe_phy_driver,
-			       ARRAY_SIZE(amd_xgbe_phy_driver));
-}
-
-module_init(amd_xgbe_phy_init);
-module_exit(amd_xgbe_phy_exit);
+module_phy_driver(amd_xgbe_phy_driver);
 
 static struct mdio_device_id __maybe_unused amd_xgbe_phy_ids[] = {
 	{ XGBE_PHY_ID, XGBE_PHY_MASK },
drivers/net/phy/at803x.c +1 -13
index fdc1b418fa6a..f80e19ac6704 100644
@@ -352,19 +352,7 @@ static struct phy_driver at803x_driver[] = {
 	},
 } };
 
-static int __init atheros_init(void)
-{
-	return phy_drivers_register(at803x_driver,
-				    ARRAY_SIZE(at803x_driver));
-}
-
-static void __exit atheros_exit(void)
-{
-	phy_drivers_unregister(at803x_driver, ARRAY_SIZE(at803x_driver));
-}
-
-module_init(atheros_init);
-module_exit(atheros_exit);
+module_phy_driver(at803x_driver);
 
 static struct mdio_device_id __maybe_unused atheros_tbl[] = {
 	{ ATH8030_PHY_ID, 0xffffffef },
drivers/net/phy/bcm63xx.c +1 -14
index ac55b0807853..830ec31f952f 100644
@@ -100,20 +100,7 @@ static struct phy_driver bcm63xx_driver[] = {
 	.driver		= { .owner = THIS_MODULE },
 } };
 
-static int __init bcm63xx_phy_init(void)
-{
-	return phy_drivers_register(bcm63xx_driver,
-		ARRAY_SIZE(bcm63xx_driver));
-}
-
-static void __exit bcm63xx_phy_exit(void)
-{
-	phy_drivers_unregister(bcm63xx_driver,
-		ARRAY_SIZE(bcm63xx_driver));
-}
-
-module_init(bcm63xx_phy_init);
-module_exit(bcm63xx_phy_exit);
+module_phy_driver(bcm63xx_driver);
 
 static struct mdio_device_id __maybe_unused bcm63xx_tbl[] = {
 	{ 0x00406000, 0xfffffc00 },
drivers/net/phy/bcm7xxx.c +1 -14
index fdce1ea28790..f9de20f93cb8 100644
@@ -337,20 +337,7 @@ static struct mdio_device_id __maybe_unused bcm7xxx_tbl[] = {
 	{ }
 };
 
-static int __init bcm7xxx_phy_init(void)
-{
-	return phy_drivers_register(bcm7xxx_driver,
-			ARRAY_SIZE(bcm7xxx_driver));
-}
-
-static void __exit bcm7xxx_phy_exit(void)
-{
-	phy_drivers_unregister(bcm7xxx_driver,
-			ARRAY_SIZE(bcm7xxx_driver));
-}
-
-module_init(bcm7xxx_phy_init);
-module_exit(bcm7xxx_phy_exit);
+module_phy_driver(bcm7xxx_driver);
 
 MODULE_DEVICE_TABLE(mdio, bcm7xxx_tbl);
drivers/net/phy/bcm87xx.c +1 -13
index 799789518e87..1eca20452f03 100644
@@ -216,18 +216,6 @@ static struct phy_driver bcm87xx_driver[] = {
 	.driver		= { .owner = THIS_MODULE },
 } };
 
-static int __init bcm87xx_init(void)
-{
-	return phy_drivers_register(bcm87xx_driver,
-		ARRAY_SIZE(bcm87xx_driver));
-}
-module_init(bcm87xx_init);
-
-static void __exit bcm87xx_exit(void)
-{
-	phy_drivers_unregister(bcm87xx_driver,
-		ARRAY_SIZE(bcm87xx_driver));
-}
-module_exit(bcm87xx_exit);
+module_phy_driver(bcm87xx_driver);
 
 MODULE_LICENSE("GPL");
drivers/net/phy/broadcom.c +1 -14
index 34088d60da74..4e2abfb8552c 100644
@@ -776,20 +776,7 @@ static struct phy_driver broadcom_drivers[] = {
 	.driver		= { .owner = THIS_MODULE },
 } };
 
-static int __init broadcom_init(void)
-{
-	return phy_drivers_register(broadcom_drivers,
-		ARRAY_SIZE(broadcom_drivers));
-}
-
-static void __exit broadcom_exit(void)
-{
-	phy_drivers_unregister(broadcom_drivers,
-		ARRAY_SIZE(broadcom_drivers));
-}
-
-module_init(broadcom_init);
-module_exit(broadcom_exit);
+module_phy_driver(broadcom_drivers);
 
 static struct mdio_device_id __maybe_unused broadcom_tbl[] = {
 	{ PHY_ID_BCM5411, 0xfffffff0 },
drivers/net/phy/cicada.c +1 -14
index b57ce0cc9657..27f5464899d4 100644
@@ -129,20 +129,7 @@ static struct phy_driver cis820x_driver[] = {
 	.driver		= { .owner = THIS_MODULE,},
 } };
 
-static int __init cicada_init(void)
-{
-	return phy_drivers_register(cis820x_driver,
-		ARRAY_SIZE(cis820x_driver));
-}
-
-static void __exit cicada_exit(void)
-{
-	phy_drivers_unregister(cis820x_driver,
-		ARRAY_SIZE(cis820x_driver));
-}
-
-module_init(cicada_init);
-module_exit(cicada_exit);
+module_phy_driver(cis820x_driver);
 
 static struct mdio_device_id __maybe_unused cicada_tbl[] = {
 	{ 0x000fc410, 0x000ffff0 },
drivers/net/phy/davicom.c +1 -14
index d2c08f625a41..0d16c7d9e1bf 100644
@@ -182,20 +182,7 @@ static struct phy_driver dm91xx_driver[] = {
 	.driver		= { .owner = THIS_MODULE,},
 } };
 
-static int __init davicom_init(void)
-{
-	return phy_drivers_register(dm91xx_driver,
-		ARRAY_SIZE(dm91xx_driver));
-}
-
-static void __exit davicom_exit(void)
-{
-	phy_drivers_unregister(dm91xx_driver,
-		ARRAY_SIZE(dm91xx_driver));
-}
-
-module_init(davicom_init);
-module_exit(davicom_exit);
+module_phy_driver(dm91xx_driver);
 
 static struct mdio_device_id __maybe_unused davicom_tbl[] = {
 	{ 0x0181b880, 0x0ffffff0 },
drivers/net/phy/icplus.c +1 -14
index 97bf58bf4939..8644f039d922 100644
@@ -253,20 +253,7 @@ static struct phy_driver icplus_driver[] = {
 	.driver		= { .owner = THIS_MODULE,},
 } };
 
-static int __init icplus_init(void)
-{
-	return phy_drivers_register(icplus_driver,
-		ARRAY_SIZE(icplus_driver));
-}
-
-static void __exit icplus_exit(void)
-{
-	phy_drivers_unregister(icplus_driver,
-		ARRAY_SIZE(icplus_driver));
-}
-
-module_init(icplus_init);
-module_exit(icplus_exit);
+module_phy_driver(icplus_driver);
 
 static struct mdio_device_id __maybe_unused icplus_tbl[] = {
 	{ 0x02430d80, 0x0ffffff0 },
drivers/net/phy/lxt.c +1 -14
index 9108f3191701..a3a5a703635b 100644
@@ -312,20 +312,7 @@ static struct phy_driver lxt97x_driver[] = {
 	.driver		= { .owner = THIS_MODULE,},
 } };
 
-static int __init lxt_init(void)
-{
-	return phy_drivers_register(lxt97x_driver,
-		ARRAY_SIZE(lxt97x_driver));
-}
-
-static void __exit lxt_exit(void)
-{
-	phy_drivers_unregister(lxt97x_driver,
-		ARRAY_SIZE(lxt97x_driver));
-}
-
-module_init(lxt_init);
-module_exit(lxt_exit);
+module_phy_driver(lxt97x_driver);
 
 static struct mdio_device_id __maybe_unused lxt_tbl[] = {
 	{ 0x78100000, 0xfffffff0 },
drivers/net/phy/marvell.c +1 -14
index bd37e45c89c0..708facd78612 100644
@@ -1052,20 +1052,7 @@ static struct phy_driver marvell_drivers[] = {
 	},
 };
 
-static int __init marvell_init(void)
-{
-	return phy_drivers_register(marvell_drivers,
-		 ARRAY_SIZE(marvell_drivers));
-}
-
-static void __exit marvell_exit(void)
-{
-	phy_drivers_unregister(marvell_drivers,
-		 ARRAY_SIZE(marvell_drivers));
-}
-
-module_init(marvell_init);
-module_exit(marvell_exit);
+module_phy_driver(marvell_drivers);
 
 static struct mdio_device_id __maybe_unused marvell_tbl[] = {
 	{ MARVELL_PHY_ID_88E1101, MARVELL_PHY_ID_MASK },
drivers/net/phy/micrel.c +1 -14
index 8c2a29a9bd7f..bcc6c0ea75fa 100644
@@ -657,20 +657,7 @@ static struct phy_driver ksphy_driver[] = {
 	.driver		= { .owner = THIS_MODULE, },
 } };
 
-static int __init ksphy_init(void)
-{
-	return phy_drivers_register(ksphy_driver,
-		ARRAY_SIZE(ksphy_driver));
-}
-
-static void __exit ksphy_exit(void)
-{
-	phy_drivers_unregister(ksphy_driver,
-		ARRAY_SIZE(ksphy_driver));
-}
-
-module_init(ksphy_init);
-module_exit(ksphy_exit);
+module_phy_driver(ksphy_driver);
 
 MODULE_DESCRIPTION("Micrel PHY driver");
 MODULE_AUTHOR("David J. Choi");
drivers/net/phy/realtek.c +1 -12
index 45483fdfbe06..96a0f0fab3ca 100644
@@ -101,18 +101,7 @@ static struct phy_driver realtek_drvs[] = {
 	},
 };
 
-static int __init realtek_init(void)
-{
-	return phy_drivers_register(realtek_drvs, ARRAY_SIZE(realtek_drvs));
-}
-
-static void __exit realtek_exit(void)
-{
-	phy_drivers_unregister(realtek_drvs, ARRAY_SIZE(realtek_drvs));
-}
-
-module_init(realtek_init);
-module_exit(realtek_exit);
+module_phy_driver(realtek_drvs);
 
 static struct mdio_device_id __maybe_unused realtek_tbl[] = {
 	{ 0x001cc912, 0x001fffff },
drivers/net/phy/smsc.c +1 -13
index a4b08198fb9f..c0f6479e19d4 100644
@@ -250,24 +250,12 @@ static struct phy_driver smsc_phy_driver[] = {
 	.driver		= { .owner = THIS_MODULE, }
 } };
 
-static int __init smsc_init(void)
-{
-	return phy_drivers_register(smsc_phy_driver,
-		ARRAY_SIZE(smsc_phy_driver));
-}
-
-static void __exit smsc_exit(void)
-{
-	phy_drivers_unregister(smsc_phy_driver, ARRAY_SIZE(smsc_phy_driver));
-}
+module_phy_driver(smsc_phy_driver);
 
 MODULE_DESCRIPTION("SMSC PHY driver");
 MODULE_AUTHOR("Herbert Valerio Riedel");
 MODULE_LICENSE("GPL");
 
-module_init(smsc_init);
-module_exit(smsc_exit);
-
 static struct mdio_device_id __maybe_unused smsc_tbl[] = {
 	{ 0x0007c0a0, 0xfffffff0 },
 	{ 0x0007c0b0, 0xfffffff0 },
drivers/net/phy/ste10Xp.c +1 -14
index 5e1eb138916f..3fc199b773e6 100644
@@ -112,20 +112,7 @@ static struct phy_driver ste10xp_pdriver[] = {
 	.driver = {.owner = THIS_MODULE,}
 } };
 
-static int __init ste10Xp_init(void)
-{
-	return phy_drivers_register(ste10xp_pdriver,
-		ARRAY_SIZE(ste10xp_pdriver));
-}
-
-static void __exit ste10Xp_exit(void)
-{
-	phy_drivers_unregister(ste10xp_pdriver,
-		ARRAY_SIZE(ste10xp_pdriver));
-}
-
-module_init(ste10Xp_init);
-module_exit(ste10Xp_exit);
+module_phy_driver(ste10xp_pdriver);
 
 static struct mdio_device_id __maybe_unused ste10Xp_tbl[] = {
 	{ STE101P_PHY_ID, 0xfffffff0 },
drivers/net/phy/vitesse.c +1 -13
index 5dc0935da99c..76cad712ddb2 100644
@@ -311,19 +311,7 @@ static struct phy_driver vsc82xx_driver[] = {
 	.driver		= { .owner = THIS_MODULE,},
 } };
 
-static int __init vsc82xx_init(void)
-{
-	return phy_drivers_register(vsc82xx_driver,
-		ARRAY_SIZE(vsc82xx_driver));
-}
-
-static void __exit vsc82xx_exit(void)
-{
-	phy_drivers_unregister(vsc82xx_driver, ARRAY_SIZE(vsc82xx_driver));
-}
-
-module_init(vsc82xx_init);
-module_exit(vsc82xx_exit);
+module_phy_driver(vsc82xx_driver);
 
 static struct mdio_device_id __maybe_unused vitesse_tbl[] = {
 	{ PHY_ID_VSC8234, 0x000ffff0 },
Signature · 1 line
2.0.4
author
#5 lore raw

PATCH 5/22 net: phy: micrel: fix config_intr error handling

In reply to Johan Hovold

Make sure never to update the control register with random data (an error code) by checking the return value after reading it.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/phy/micrel.c +6 -0
index bcc6c0ea75fa..62ca9613a514 100644
@@ -122,6 +122,8 @@ static int kszphy_config_intr(struct phy_device *phydev)
 
 	/* set the interrupt pin active low */
 	temp = phy_read(phydev, MII_KSZPHY_CTRL);
+	if (temp < 0)
+		return temp;
 	temp &= ~KSZPHY_CTRL_INT_ACTIVE_HIGH;
 	phy_write(phydev, MII_KSZPHY_CTRL, temp);
 	rc = kszphy_set_interrupt(phydev);

@@ -134,6 +136,8 @@ static int ksz9021_config_intr(struct phy_device *phydev)
 
 	/* set the interrupt pin active low */
 	temp = phy_read(phydev, MII_KSZPHY_CTRL);
+	if (temp < 0)
+		return temp;
 	temp &= ~KSZ9021_CTRL_INT_ACTIVE_HIGH;
 	phy_write(phydev, MII_KSZPHY_CTRL, temp);
 	rc = kszphy_set_interrupt(phydev);

@@ -146,6 +150,8 @@ static int ks8737_config_intr(struct phy_device *phydev)
 
 	/* set the interrupt pin active low */
 	temp = phy_read(phydev, MII_KSZPHY_CTRL);
+	if (temp < 0)
+		return temp;
 	temp &= ~KS8737_CTRL_INT_ACTIVE_HIGH;
 	phy_write(phydev, MII_KSZPHY_CTRL, temp);
 	rc = kszphy_set_interrupt(phydev);
Signature · 1 line
2.0.4
author
#6 lore raw

PATCH 6/22 net: phy: micrel: use BIT macro

In reply to Johan Hovold

Use BIT macro for bitmask definitions.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)
drivers/net/phy/micrel.c +17 -17
index 62ca9613a514..d962a2866bba 100644
@@ -30,30 +30,30 @@
 
 /* Operation Mode Strap Override */
 #define MII_KSZPHY_OMSO				0x16
-#define KSZPHY_OMSO_B_CAST_OFF			(1 << 9)
-#define KSZPHY_OMSO_RMII_OVERRIDE		(1 << 1)
-#define KSZPHY_OMSO_MII_OVERRIDE		(1 << 0)
+#define KSZPHY_OMSO_B_CAST_OFF			BIT(9)
+#define KSZPHY_OMSO_RMII_OVERRIDE		BIT(1)
+#define KSZPHY_OMSO_MII_OVERRIDE		BIT(0)
 
 /* general Interrupt control/status reg in vendor specific block. */
 #define MII_KSZPHY_INTCS			0x1B
-#define	KSZPHY_INTCS_JABBER			(1 << 15)
-#define	KSZPHY_INTCS_RECEIVE_ERR		(1 << 14)
-#define	KSZPHY_INTCS_PAGE_RECEIVE		(1 << 13)
-#define	KSZPHY_INTCS_PARELLEL			(1 << 12)
-#define	KSZPHY_INTCS_LINK_PARTNER_ACK		(1 << 11)
-#define	KSZPHY_INTCS_LINK_DOWN			(1 << 10)
-#define	KSZPHY_INTCS_REMOTE_FAULT		(1 << 9)
-#define	KSZPHY_INTCS_LINK_UP			(1 << 8)
+#define	KSZPHY_INTCS_JABBER			BIT(15)
+#define	KSZPHY_INTCS_RECEIVE_ERR		BIT(14)
+#define	KSZPHY_INTCS_PAGE_RECEIVE		BIT(13)
+#define	KSZPHY_INTCS_PARELLEL			BIT(12)
+#define	KSZPHY_INTCS_LINK_PARTNER_ACK		BIT(11)
+#define	KSZPHY_INTCS_LINK_DOWN			BIT(10)
+#define	KSZPHY_INTCS_REMOTE_FAULT		BIT(9)
+#define	KSZPHY_INTCS_LINK_UP			BIT(8)
 #define	KSZPHY_INTCS_ALL			(KSZPHY_INTCS_LINK_UP |\
 						KSZPHY_INTCS_LINK_DOWN)
 
 /* general PHY control reg in vendor specific block. */
 #define	MII_KSZPHY_CTRL			0x1F
 /* bitmap of PHY register to set interrupt mode */
-#define KSZPHY_CTRL_INT_ACTIVE_HIGH		(1 << 9)
-#define KSZ9021_CTRL_INT_ACTIVE_HIGH		(1 << 14)
-#define KS8737_CTRL_INT_ACTIVE_HIGH		(1 << 14)
-#define KSZ8051_RMII_50MHZ_CLK			(1 << 7)
+#define KSZPHY_CTRL_INT_ACTIVE_HIGH		BIT(9)
+#define KSZ9021_CTRL_INT_ACTIVE_HIGH		BIT(14)
+#define KS8737_CTRL_INT_ACTIVE_HIGH		BIT(14)
+#define KSZ8051_RMII_50MHZ_CLK			BIT(7)
 
 /* Write/read to/from extended registers */
 #define MII_KSZPHY_EXTREG                       0x0b

@@ -400,8 +400,8 @@ static int ksz9031_config_init(struct phy_device *phydev)
 }
 
 #define KSZ8873MLL_GLOBAL_CONTROL_4	0x06
-#define KSZ8873MLL_GLOBAL_CONTROL_4_DUPLEX	(1 << 6)
-#define KSZ8873MLL_GLOBAL_CONTROL_4_SPEED	(1 << 4)
+#define KSZ8873MLL_GLOBAL_CONTROL_4_DUPLEX	BIT(6)
+#define KSZ8873MLL_GLOBAL_CONTROL_4_SPEED	BIT(4)
 static int ksz8873mll_read_status(struct phy_device *phydev)
 {
 	int regval;
Signature · 1 line
2.0.4
author
#7 lore raw

PATCH 7/22 net: phy: micrel: refactor broadcast disable

In reply to Johan Hovold

Refactor and clean up broadcast disable.

Some Micrel PHYs have a broadcast-off bit in the Operation Mode Strap Override register which can be used to disable PHY address 0 as the broadcast address, so that it can be used as a unique (non-broadcast) address on a shared bus.

Note that the KSZPHY_OMSO_RMII_OVERRIDE bit is set by default on KSZ8021/8031.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)
drivers/net/phy/micrel.c +22 -2
index d962a2866bba..21abe5ade3df 100644
@@ -184,6 +184,25 @@ static int kszphy_setup_led(struct phy_device *phydev,
 	return rc < 0 ? rc : 0;
 }
 
+/* Disable PHY address 0 as the broadcast address, so that it can be used as a
+ * unique (non-broadcast) address on a shared bus.
+ */
+static int kszphy_broadcast_disable(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_read(phydev, MII_KSZPHY_OMSO);
+	if (ret < 0)
+		goto out;
+
+	ret = phy_write(phydev, MII_KSZPHY_OMSO, ret | KSZPHY_OMSO_B_CAST_OFF);
+out:
+	if (ret)
+		dev_err(&phydev->dev, "failed to disable broadcast address\n");
+
+	return ret;
+}
+
 static int kszphy_config_init(struct phy_device *phydev)
 {
 	return 0;

@@ -197,7 +216,6 @@ static int kszphy_config_init_led8041(struct phy_device *phydev)
 
 static int ksz8021_config_init(struct phy_device *phydev)
 {
-	const u16 val = KSZPHY_OMSO_B_CAST_OFF | KSZPHY_OMSO_RMII_OVERRIDE;
 	int rc;
 
 	rc = kszphy_setup_led(phydev, 0x1f, 4);

@@ -207,7 +225,9 @@ static int ksz8021_config_init(struct phy_device *phydev)
 	rc = ksz_config_flags(phydev);
 	if (rc < 0)
 		return rc;
-	rc = phy_write(phydev, MII_KSZPHY_OMSO, val);
+
+	rc = kszphy_broadcast_disable(phydev);
+
 	return rc < 0 ? rc : 0;
 }
Signature · 1 line
2.0.4
author
#8 lore raw

PATCH 8/22 net: phy: micrel: disable broadcast for KSZ8081/KSZ8091

In reply to Johan Hovold

Disable PHY address 0 as the broadcast address, so that it can be used as a unique (non-broadcast) address on a shared bus.

Note that this can also be configured using the B-CAST_OFF pin on KSZ9091, but that KSZ8081 lacks this pin and is also limited to addresses 0 and 3.

Specifically, this allows for dual KSZ8081 setups.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/net/phy/micrel.c +8 -1
index 21abe5ade3df..16135ac18bfe 100644
@@ -243,6 +243,13 @@ static int ks8051_config_init(struct phy_device *phydev)
 	return rc < 0 ? rc : 0;
 }
 
+static int ksz8081_config_init(struct phy_device *phydev)
+{
+	kszphy_broadcast_disable(phydev);
+
+	return 0;
+}
+
 static int ksz9021_load_values_from_of(struct phy_device *phydev,
 				       struct device_node *of_node, u16 reg,
 				       char *field1, char *field2,

@@ -605,7 +612,7 @@ static struct phy_driver ksphy_driver[] = {
 	.phy_id_mask	= 0x00fffff0,
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init,
+	.config_init	= ksz8081_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
Signature · 1 line
2.0.4
author
#9 lore raw

PATCH 9/22 net: phy: micrel: add led-mode sanity check

In reply to Johan Hovold

Make sure never to update more than two bits when setting the led mode, something which could for example change the reference-clock setting.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/net/phy/micrel.c +5 -0
index 16135ac18bfe..1b3985cdc64c 100644
@@ -173,6 +173,11 @@ static int kszphy_setup_led(struct phy_device *phydev,
 	if (of_property_read_u32(of_node, "micrel,led-mode", &val))
 		return 0;
 
+	if (val > 3) {
+		dev_err(&phydev->dev, "invalid led mode: 0x%02x\n", val);
+		return -EINVAL;
+	}
+
 	temp = phy_read(phydev, reg);
 	if (temp < 0)
 		return temp;
Signature · 1 line
2.0.4
author
#11 lore raw

PATCH 11/22 net: phy: micrel: clean up led-mode setup

In reply to Johan Hovold

Clean up led-mode setup by introducing proper defines for PHY Control registers 1 and 2 and only passing the register to the setup function.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)
drivers/net/phy/micrel.c +22 -9
index ec9ce35e934b..12e18f7273ce 100644
@@ -47,8 +47,12 @@
 #define	KSZPHY_INTCS_ALL			(KSZPHY_INTCS_LINK_UP |\
 						KSZPHY_INTCS_LINK_DOWN)
 
-/* general PHY control reg in vendor specific block. */
-#define	MII_KSZPHY_CTRL			0x1F
+/* PHY Control 1 */
+#define	MII_KSZPHY_CTRL_1			0x1e
+
+/* PHY Control 2 / PHY Control (if no PHY Control 1) */
+#define	MII_KSZPHY_CTRL_2			0x1f
+#define	MII_KSZPHY_CTRL				MII_KSZPHY_CTRL_2
 /* bitmap of PHY register to set interrupt mode */
 #define KSZPHY_CTRL_INT_ACTIVE_HIGH		BIT(9)
 #define KSZ9021_CTRL_INT_ACTIVE_HIGH		BIT(14)

@@ -158,13 +162,12 @@ static int ks8737_config_intr(struct phy_device *phydev)
 	return rc < 0 ? rc : 0;
 }
 
-static int kszphy_setup_led(struct phy_device *phydev,
-			    unsigned int reg, unsigned int shift)
+static int kszphy_setup_led(struct phy_device *phydev, u32 reg)
 {
 
 	struct device *dev = &phydev->dev;
 	struct device_node *of_node = dev->of_node;
-	int rc, temp;
+	int rc, temp, shift;
 	u32 val;
 
 	if (!of_node && dev->parent->of_node)

@@ -178,6 +181,17 @@ static int kszphy_setup_led(struct phy_device *phydev,
 		return -EINVAL;
 	}
 
+	switch (reg) {
+	case MII_KSZPHY_CTRL_1:
+		shift = 14;
+		break;
+	case MII_KSZPHY_CTRL_2:
+		shift = 4;
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	temp = phy_read(phydev, reg);
 	if (temp < 0) {
 		rc = temp;

@@ -220,15 +234,14 @@ static int kszphy_config_init(struct phy_device *phydev)
 
 static int kszphy_config_init_led8041(struct phy_device *phydev)
 {
-	/* single led control, register 0x1e bits 15..14 */
-	return kszphy_setup_led(phydev, 0x1e, 14);
+	return kszphy_setup_led(phydev, MII_KSZPHY_CTRL_1);
 }
 
 static int ksz8021_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	kszphy_setup_led(phydev, 0x1f, 4);
+	kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
 
 	rc = ksz_config_flags(phydev);
 	if (rc < 0)

@@ -243,7 +256,7 @@ static int ks8051_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	kszphy_setup_led(phydev, 0x1f, 4);
+	kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
 
 	rc = ksz_config_flags(phydev);
 	return rc < 0 ? rc : 0;
Signature · 1 line
2.0.4
author
#12 lore raw

PATCH 12/22 net: phy: micrel: enable led-mode for KSZ8081/KSZ8091

In reply to Johan Hovold

Enable led-mode configuration for KSZ8081 and KSZ8091.

Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt | 2 ++
 drivers/net/phy/micrel.c                         | 1 +
 2 files changed, 3 insertions(+)
Documentation/devicetree/bindings/net/micrel.txt +2 -0
index e1d99b95c4ec..30062fae5623 100644
@@ -14,6 +14,8 @@ Optional properties:
 	      KSZ8021: register 0x1f, bits 5..4
 	      KSZ8031: register 0x1f, bits 5..4
 	      KSZ8051: register 0x1f, bits 5..4
+	      KSZ8081: register 0x1f, bits 5..4
+	      KSZ8091: register 0x1f, bits 5..4
 
               See the respective PHY datasheet for the mode values.
drivers/net/phy/micrel.c +1 -0
index 12e18f7273ce..30e894d6ffbd 100644
@@ -265,6 +265,7 @@ static int ks8051_config_init(struct phy_device *phydev)
 static int ksz8081_config_init(struct phy_device *phydev)
 {
 	kszphy_broadcast_disable(phydev);
+	kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
 
 	return 0;
 }
Signature · 1 line
2.0.4
author
#13 lore raw

PATCH 13/22 net: phy: add static data field to struct phy_driver

In reply to Johan Hovold

Add static data field to struct phy_driver that be used to store structured device-type information.

Signed-off-by Johan Hovold <[email protected]>
---
 include/linux/phy.h | 2 ++
 1 file changed, 2 insertions(+)
include/linux/phy.h +2 -0
index c50e1f1f46e0..29c260add1f6 100644
@@ -433,6 +433,7 @@ struct phy_device {
  *   by this PHY
  * flags: A bitfield defining certain other features this PHY
  *   supports (like interrupts)
+ * driver_data: static driver data
  *
  * The drivers must implement config_aneg and read_status.  All
  * other functions are optional. Note that none of these

@@ -448,6 +449,7 @@ struct phy_driver {
 	unsigned int phy_id_mask;
 	u32 features;
 	u32 flags;
+	const void *driver_data;
 
 	/*
 	 * Called to issue a PHY software reset
Signature · 1 line
2.0.4
author
#14 lore raw

PATCH 14/22 net: phy: micrel: add device-type abstraction

In reply to Johan Hovold

Add structured device-type information and support for generic led-mode setup to the generic config_init callback.

This is a first step in ultimately getting rid of device-type specific callbacks.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 82 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 69 insertions(+), 13 deletions(-)
drivers/net/phy/micrel.c +69 -13
index 30e894d6ffbd..189fddb829bc 100644
@@ -73,6 +73,30 @@
 
 #define PS_TO_REG				200
 
+struct kszphy_type {
+	u32 led_mode_reg;
+};
+
+struct kszphy_priv {
+	const struct kszphy_type *type;
+};
+
+static const struct kszphy_type ksz8021_type = {
+	.led_mode_reg		= MII_KSZPHY_CTRL_2,
+};
+
+static const struct kszphy_type ksz8041_type = {
+	.led_mode_reg		= MII_KSZPHY_CTRL_1,
+};
+
+static const struct kszphy_type ksz8051_type = {
+	.led_mode_reg		= MII_KSZPHY_CTRL_2,
+};
+
+static const struct kszphy_type ksz8081_type = {
+	.led_mode_reg		= MII_KSZPHY_CTRL_2,
+};
+
 static int ksz_config_flags(struct phy_device *phydev)
 {
 	int regval;

@@ -229,19 +253,25 @@ out:
 
 static int kszphy_config_init(struct phy_device *phydev)
 {
-	return 0;
-}
+	struct kszphy_priv *priv = phydev->priv;
+	const struct kszphy_type *type;
 
-static int kszphy_config_init_led8041(struct phy_device *phydev)
-{
-	return kszphy_setup_led(phydev, MII_KSZPHY_CTRL_1);
+	if (!priv)
+		return 0;
+
+	type = priv->type;
+
+	if (type->led_mode_reg)
+		kszphy_setup_led(phydev, type->led_mode_reg);
+
+	return 0;
 }
 
 static int ksz8021_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
+	kszphy_config_init(phydev);
 
 	rc = ksz_config_flags(phydev);
 	if (rc < 0)

@@ -256,7 +286,7 @@ static int ks8051_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
+	kszphy_config_init(phydev);
 
 	rc = ksz_config_flags(phydev);
 	return rc < 0 ? rc : 0;

@@ -265,9 +295,8 @@ static int ks8051_config_init(struct phy_device *phydev)
 static int ksz8081_config_init(struct phy_device *phydev)
 {
 	kszphy_broadcast_disable(phydev);
-	kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
 
-	return 0;
+	return kszphy_config_init(phydev);
 }
 
 static int ksz9021_load_values_from_of(struct phy_device *phydev,

@@ -499,6 +528,22 @@ ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
 {
 }
 
+static int kszphy_probe(struct phy_device *phydev)
+{
+	const struct kszphy_type *type = phydev->drv->driver_data;
+	struct kszphy_priv *priv;
+
+	priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	phydev->priv = priv;
+
+	priv->type = type;
+
+	return 0;
+}
+
 static int ksz8021_probe(struct phy_device *phydev)
 {
 	struct clk *clk;

@@ -517,7 +562,7 @@ static int ksz8021_probe(struct phy_device *phydev)
 		}
 	}
 
-	return 0;
+	return kszphy_probe(phydev);
 }
 
 static struct phy_driver ksphy_driver[] = {

@@ -542,6 +587,7 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause |
 			   SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ksz8021_type,
 	.probe		= ksz8021_probe,
 	.config_init	= ksz8021_config_init,
 	.config_aneg	= genphy_config_aneg,

@@ -558,6 +604,7 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause |
 			   SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ksz8021_type,
 	.probe		= ksz8021_probe,
 	.config_init	= ksz8021_config_init,
 	.config_aneg	= genphy_config_aneg,

@@ -574,7 +621,9 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause
 				| SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init_led8041,
+	.driver_data	= &ksz8041_type,
+	.probe		= kszphy_probe,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,

@@ -589,7 +638,9 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= PHY_BASIC_FEATURES |
 			  SUPPORTED_Pause | SUPPORTED_Asym_Pause,
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init_led8041,
+	.driver_data	= &ksz8041_type,
+	.probe		= kszphy_probe,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,

@@ -604,6 +655,7 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause
 				| SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ksz8051_type,
 	.config_init	= ks8051_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,

@@ -618,7 +670,9 @@ static struct phy_driver ksphy_driver[] = {
 	.phy_id_mask	= 0x00ffffff,
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init_led8041,
+	.driver_data	= &ksz8041_type,
+	.probe		= kszphy_probe,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,

@@ -632,6 +686,8 @@ static struct phy_driver ksphy_driver[] = {
 	.phy_id_mask	= 0x00fffff0,
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ksz8081_type,
+	.probe		= kszphy_probe,
 	.config_init	= ksz8081_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
Signature · 1 line
2.0.4
author
#15 lore raw

PATCH 15/22 net: phy: micrel: parse of nodes at probe

In reply to Johan Hovold

Parse the "micrel,led-mode" property at probe, rather than at config_init time in the led-setup helper itself.

Note that the bogus parent->of_node bit is removed.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)
drivers/net/phy/micrel.c +21 -18
index 189fddb829bc..928248187a11 100644
@@ -79,6 +79,7 @@ struct kszphy_type {
 
 struct kszphy_priv {
 	const struct kszphy_type *type;
+	int led_mode;
 };
 
 static const struct kszphy_type ksz8021_type = {

@@ -186,24 +187,9 @@ static int ks8737_config_intr(struct phy_device *phydev)
 	return rc < 0 ? rc : 0;
 }
 
-static int kszphy_setup_led(struct phy_device *phydev, u32 reg)
+static int kszphy_setup_led(struct phy_device *phydev, u32 reg, int val)
 {
-
-	struct device *dev = &phydev->dev;
-	struct device_node *of_node = dev->of_node;
 	int rc, temp, shift;
-	u32 val;
-
-	if (!of_node && dev->parent->of_node)
-		of_node = dev->parent->of_node;
-
-	if (of_property_read_u32(of_node, "micrel,led-mode", &val))
-		return 0;
-
-	if (val > 3) {
-		dev_err(&phydev->dev, "invalid led mode: 0x%02x\n", val);
-		return -EINVAL;
-	}
 
 	switch (reg) {
 	case MII_KSZPHY_CTRL_1:

@@ -261,8 +247,8 @@ static int kszphy_config_init(struct phy_device *phydev)
 
 	type = priv->type;
 
-	if (type->led_mode_reg)
-		kszphy_setup_led(phydev, type->led_mode_reg);
+	if (priv->led_mode >= 0)
+		kszphy_setup_led(phydev, type->led_mode_reg, priv->led_mode);
 
 	return 0;
 }

@@ -531,7 +517,9 @@ ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
 static int kszphy_probe(struct phy_device *phydev)
 {
 	const struct kszphy_type *type = phydev->drv->driver_data;
+	struct device_node *np = phydev->dev.of_node;
 	struct kszphy_priv *priv;
+	int ret;
 
 	priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)

@@ -541,6 +529,21 @@ static int kszphy_probe(struct phy_device *phydev)
 
 	priv->type = type;
 
+	if (type->led_mode_reg) {
+		ret = of_property_read_u32(np, "micrel,led-mode",
+				&priv->led_mode);
+		if (ret)
+			priv->led_mode = -1;
+
+		if (priv->led_mode > 3) {
+			dev_err(&phydev->dev, "invalid led mode: 0x%02x\n",
+					priv->led_mode);
+			priv->led_mode = -1;
+		}
+	} else {
+		priv->led_mode = -1;
+	}
+
 	return 0;
 }
Signature · 1 line
2.0.4
author
#16 lore raw

PATCH 16/22 net: phy: micrel: add has-broadcast-disable flag to type data

In reply to Johan Hovold

Add has_broadcast_disable flag to type-data and generic config_init.

This allows us to remove the ksz8081 config_init callback.

Note that ksz8021_config_init is kept for now due to a95a18afe4c8 ("phy/micrel: KSZ8031RNL RMII clock reconfiguration bug").

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
drivers/net/phy/micrel.c +6 -8
index 928248187a11..6467bcc5a39d 100644
@@ -75,6 +75,7 @@
 
 struct kszphy_type {
 	u32 led_mode_reg;
+	bool has_broadcast_disable;
 };
 
 struct kszphy_priv {

@@ -96,6 +97,7 @@ static const struct kszphy_type ksz8051_type = {
 
 static const struct kszphy_type ksz8081_type = {
 	.led_mode_reg		= MII_KSZPHY_CTRL_2,
+	.has_broadcast_disable	= true,
 };
 
 static int ksz_config_flags(struct phy_device *phydev)

@@ -247,6 +249,9 @@ static int kszphy_config_init(struct phy_device *phydev)
 
 	type = priv->type;
 
+	if (type->has_broadcast_disable)
+		kszphy_broadcast_disable(phydev);
+
 	if (priv->led_mode >= 0)
 		kszphy_setup_led(phydev, type->led_mode_reg, priv->led_mode);
 

@@ -278,13 +283,6 @@ static int ks8051_config_init(struct phy_device *phydev)
 	return rc < 0 ? rc : 0;
 }
 
-static int ksz8081_config_init(struct phy_device *phydev)
-{
-	kszphy_broadcast_disable(phydev);
-
-	return kszphy_config_init(phydev);
-}
-
 static int ksz9021_load_values_from_of(struct phy_device *phydev,
 				       struct device_node *of_node, u16 reg,
 				       char *field1, char *field2,

@@ -691,7 +689,7 @@ static struct phy_driver ksphy_driver[] = {
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.driver_data	= &ksz8081_type,
 	.probe		= kszphy_probe,
-	.config_init	= ksz8081_config_init,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
Signature · 1 line
2.0.4
author
#17 lore raw

PATCH 17/22 net: phy: micrel: add generic rmii-ref-clk-sel support

In reply to Johan Hovold

Add generic RMII-reference-clock-select support.

Several Micrel PHY have a RMII-reference-clock-select bit to select 25 MHz or 50 MHz clock mode. Recently, support for configuring this through device tree for KSZ8021 and KSZ8031 was added.

Generalise this support so that it can be configured for other PHY types as well.

Note that some PHY revisions (of the same type) has this bit inverted. This should be either configurable through a new device-tree property, or preferably, determined based on PHY ID if possible.

Also note that this removes support for setting 25 MHz mode from board files which was also added by the above mentioned commit 45f56cb82e45 ("net/phy: micrel: Add clock support for KSZ8021/KSZ8031").

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c   | 94 +++++++++++++++++++++++++---------------------
 include/linux/micrel_phy.h |  1 -
 2 files changed, 51 insertions(+), 44 deletions(-)
drivers/net/phy/micrel.c +51 -43
index 6467bcc5a39d..d2e790cd3651 100644
@@ -57,7 +57,7 @@
 #define KSZPHY_CTRL_INT_ACTIVE_HIGH		BIT(9)
 #define KSZ9021_CTRL_INT_ACTIVE_HIGH		BIT(14)
 #define KS8737_CTRL_INT_ACTIVE_HIGH		BIT(14)
-#define KSZ8051_RMII_50MHZ_CLK			BIT(7)
+#define KSZPHY_RMII_REF_CLK_SEL			BIT(7)
 
 /* Write/read to/from extended registers */
 #define MII_KSZPHY_EXTREG                       0x0b

@@ -76,15 +76,19 @@
 struct kszphy_type {
 	u32 led_mode_reg;
 	bool has_broadcast_disable;
+	bool has_rmii_ref_clk_sel;
 };
 
 struct kszphy_priv {
 	const struct kszphy_type *type;
 	int led_mode;
+	bool rmii_ref_clk_sel;
+	bool rmii_ref_clk_sel_val;
 };
 
 static const struct kszphy_type ksz8021_type = {
 	.led_mode_reg		= MII_KSZPHY_CTRL_2,
+	.has_rmii_ref_clk_sel	= true,
 };
 
 static const struct kszphy_type ksz8041_type = {

@@ -100,21 +104,6 @@ static const struct kszphy_type ksz8081_type = {
 	.has_broadcast_disable	= true,
 };
 
-static int ksz_config_flags(struct phy_device *phydev)
-{
-	int regval;
-
-	if (phydev->dev_flags & (MICREL_PHY_50MHZ_CLK | MICREL_PHY_25MHZ_CLK)) {
-		regval = phy_read(phydev, MII_KSZPHY_CTRL);
-		if (phydev->dev_flags & MICREL_PHY_50MHZ_CLK)
-			regval |= KSZ8051_RMII_50MHZ_CLK;
-		else
-			regval &= ~KSZ8051_RMII_50MHZ_CLK;
-		return phy_write(phydev, MII_KSZPHY_CTRL, regval);
-	}
-	return 0;
-}
-
 static int kszphy_extended_write(struct phy_device *phydev,
 				u32 regnum, u16 val)
 {

@@ -189,6 +178,22 @@ static int ks8737_config_intr(struct phy_device *phydev)
 	return rc < 0 ? rc : 0;
 }
 
+static int kszphy_rmii_clk_sel(struct phy_device *phydev, bool val)
+{
+	int ctrl;
+
+	ctrl = phy_read(phydev, MII_KSZPHY_CTRL);
+	if (ctrl < 0)
+		return ctrl;
+
+	if (val)
+		ctrl |= KSZPHY_RMII_REF_CLK_SEL;
+	else
+		ctrl &= ~KSZPHY_RMII_REF_CLK_SEL;
+
+	return phy_write(phydev, MII_KSZPHY_CTRL, ctrl);
+}
+
 static int kszphy_setup_led(struct phy_device *phydev, u32 reg, int val)
 {
 	int rc, temp, shift;

@@ -243,6 +248,7 @@ static int kszphy_config_init(struct phy_device *phydev)
 {
 	struct kszphy_priv *priv = phydev->priv;
 	const struct kszphy_type *type;
+	int ret;
 
 	if (!priv)
 		return 0;

@@ -252,6 +258,14 @@ static int kszphy_config_init(struct phy_device *phydev)
 	if (type->has_broadcast_disable)
 		kszphy_broadcast_disable(phydev);
 
+	if (priv->rmii_ref_clk_sel) {
+		ret = kszphy_rmii_clk_sel(phydev, priv->rmii_ref_clk_sel_val);
+		if (ret) {
+			dev_err(&phydev->dev, "failed to set rmii reference clock\n");
+			return ret;
+		}
+	}
+
 	if (priv->led_mode >= 0)
 		kszphy_setup_led(phydev, type->led_mode_reg, priv->led_mode);
 

@@ -262,10 +276,8 @@ static int ksz8021_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	kszphy_config_init(phydev);
-
-	rc = ksz_config_flags(phydev);
-	if (rc < 0)
+	rc = kszphy_config_init(phydev);
+	if (rc)
 		return rc;
 
 	rc = kszphy_broadcast_disable(phydev);

@@ -273,16 +285,6 @@ static int ksz8021_config_init(struct phy_device *phydev)
 	return rc < 0 ? rc : 0;
 }
 
-static int ks8051_config_init(struct phy_device *phydev)
-{
-	int rc;
-
-	kszphy_config_init(phydev);
-
-	rc = ksz_config_flags(phydev);
-	return rc < 0 ? rc : 0;
-}
-
 static int ksz9021_load_values_from_of(struct phy_device *phydev,
 				       struct device_node *of_node, u16 reg,
 				       char *field1, char *field2,

@@ -517,6 +519,7 @@ static int kszphy_probe(struct phy_device *phydev)
 	const struct kszphy_type *type = phydev->drv->driver_data;
 	struct device_node *np = phydev->dev.of_node;
 	struct kszphy_priv *priv;
+	struct clk *clk;
 	int ret;
 
 	priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);

@@ -542,28 +545,32 @@ static int kszphy_probe(struct phy_device *phydev)
 		priv->led_mode = -1;
 	}
 
-	return 0;
-}
-
-static int ksz8021_probe(struct phy_device *phydev)
-{
-	struct clk *clk;
-
 	clk = devm_clk_get(&phydev->dev, "rmii-ref");
 	if (!IS_ERR(clk)) {
 		unsigned long rate = clk_get_rate(clk);
 
+		priv->rmii_ref_clk_sel = type->has_rmii_ref_clk_sel;
+
+		/* FIXME: add support for PHY revisions that have this bit
+		 * inverted (e.g. through new property or based on PHY ID).
+		 */
 		if (rate > 24500000 && rate < 25500000) {
-			phydev->dev_flags |= MICREL_PHY_25MHZ_CLK;
+			priv->rmii_ref_clk_sel_val = false;
 		} else if (rate > 49500000 && rate < 50500000) {
-			phydev->dev_flags |= MICREL_PHY_50MHZ_CLK;
+			priv->rmii_ref_clk_sel_val = true;
 		} else {
 			dev_err(&phydev->dev, "Clock rate out of range: %ld\n", rate);
 			return -EINVAL;
 		}
 	}
 
-	return kszphy_probe(phydev);
+	/* Support legacy board-file configuration */
+	if (phydev->dev_flags & MICREL_PHY_50MHZ_CLK) {
+		priv->rmii_ref_clk_sel = true;
+		priv->rmii_ref_clk_sel_val = true;
+	}
+
+	return 0;
 }
 
 static struct phy_driver ksphy_driver[] = {

@@ -589,7 +596,7 @@ static struct phy_driver ksphy_driver[] = {
 			   SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.driver_data	= &ksz8021_type,
-	.probe		= ksz8021_probe,
+	.probe		= kszphy_probe,
 	.config_init	= ksz8021_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,

@@ -606,7 +613,7 @@ static struct phy_driver ksphy_driver[] = {
 			   SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.driver_data	= &ksz8021_type,
-	.probe		= ksz8021_probe,
+	.probe		= kszphy_probe,
 	.config_init	= ksz8021_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,

@@ -657,7 +664,8 @@ static struct phy_driver ksphy_driver[] = {
 				| SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.driver_data	= &ksz8051_type,
-	.config_init	= ks8051_config_init,
+	.probe		= kszphy_probe,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
include/linux/micrel_phy.h +0 -1
index 53d33dee70e1..2e5b194b9b19 100644
@@ -37,7 +37,6 @@
 
 /* struct phy_device dev_flags definitions */
 #define MICREL_PHY_50MHZ_CLK	0x00000001
-#define MICREL_PHY_25MHZ_CLK	0x00000002
 
 #define MICREL_KSZ9021_EXTREG_CTRL	0xB
 #define MICREL_KSZ9021_EXTREG_DATA_WRITE	0xC
Signature · 1 line
2.0.4
author
#18 lore raw

PATCH 18/22 net: phy: micrel: add support for rmii_ref_clk_sel to KSZ8081/KSZ8091

In reply to Johan Hovold

Micrel KSZ8081 and KSZ8091 has the rmii_ref_clk_sel bit that is used to select 25 or 50 MHz clock mode.

Note that on some revisions of the PHY (e.g. KSZ8081RND) the function of this bit is inverted so that setting it enables 25 MHz mode. Add a new device property "micrel,rmii_ref_clk_sel_25_mhz" to be able to configure this.

Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt |  4 ++--
 drivers/net/phy/micrel.c                         | 11 ++++++-----
 2 files changed, 8 insertions(+), 7 deletions(-)
Documentation/devicetree/bindings/net/micrel.txt +2 -2
index 30062fae5623..a1bab5eaae02 100644
@@ -22,5 +22,5 @@ Optional properties:
  - clocks, clock-names: contains clocks according to the common clock bindings.
 
               supported clocks:
-	      - KSZ8021, KSZ8031: "rmii-ref": The RMII refence input clock. Used
-		to determine the XI input clock.
+	      - KSZ8021, KSZ8031, KSZ8081, KSZ8091: "rmii-ref": The RMII
+		refence input clock. Used to determine the XI input clock.
drivers/net/phy/micrel.c +6 -5
index d2e790cd3651..591190384497 100644
@@ -102,6 +102,7 @@ static const struct kszphy_type ksz8051_type = {
 static const struct kszphy_type ksz8081_type = {
 	.led_mode_reg		= MII_KSZPHY_CTRL_2,
 	.has_broadcast_disable	= true,
+	.has_rmii_ref_clk_sel	= true,
 };
 
 static int kszphy_extended_write(struct phy_device *phydev,

@@ -548,16 +549,16 @@ static int kszphy_probe(struct phy_device *phydev)
 	clk = devm_clk_get(&phydev->dev, "rmii-ref");
 	if (!IS_ERR(clk)) {
 		unsigned long rate = clk_get_rate(clk);
+		bool rmii_ref_clk_sel_25_mhz;
 
 		priv->rmii_ref_clk_sel = type->has_rmii_ref_clk_sel;
+		rmii_ref_clk_sel_25_mhz = of_property_read_bool(np,
+				"micrel,rmii_ref_clk_sel_25_mhz");
 
-		/* FIXME: add support for PHY revisions that have this bit
-		 * inverted (e.g. through new property or based on PHY ID).
-		 */
 		if (rate > 24500000 && rate < 25500000) {
-			priv->rmii_ref_clk_sel_val = false;
+			priv->rmii_ref_clk_sel_val = rmii_ref_clk_sel_25_mhz;
 		} else if (rate > 49500000 && rate < 50500000) {
-			priv->rmii_ref_clk_sel_val = true;
+			priv->rmii_ref_clk_sel_val = !rmii_ref_clk_sel_25_mhz;
 		} else {
 			dev_err(&phydev->dev, "Clock rate out of range: %ld\n", rate);
 			return -EINVAL;
Signature · 1 line
2.0.4
author
#19 lore raw

PATCH 19/22 dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding

In reply to Johan Hovold

Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding documentation.

Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/net/micrel.txt +5 -0
index a1bab5eaae02..9b08dd6551dd 100644
@@ -19,6 +19,11 @@ Optional properties:
 
               See the respective PHY datasheet for the mode values.
 
+ - micrel,rmii_ref_clk_sel_25_mhz: rmii_ref_clk_sel bit selects 25 MHz mode
+
+		Whether 25 MHz (rather than 50 Mhz) clock mode is selected
+		when the rmii_ref_clk_sel bit is set.
+
  - clocks, clock-names: contains clocks according to the common clock bindings.
 
               supported clocks:
Signature · 1 line
2.0.4
author
#26 lore raw

PATCH 20/22 net: phy: micrel: refactor interrupt config

In reply to Johan Hovold

Add generic interrupt-config callback and store interrupt-level bitmask in type data for PHY types not using bit 9.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 71 ++++++++++++++++++++----------------------------
 1 file changed, 29 insertions(+), 42 deletions(-)
drivers/net/phy/micrel.c +29 -42
index 591190384497..743d5b52d8db 100644
@@ -55,8 +55,6 @@
 #define	MII_KSZPHY_CTRL				MII_KSZPHY_CTRL_2
 /* bitmap of PHY register to set interrupt mode */
 #define KSZPHY_CTRL_INT_ACTIVE_HIGH		BIT(9)
-#define KSZ9021_CTRL_INT_ACTIVE_HIGH		BIT(14)
-#define KS8737_CTRL_INT_ACTIVE_HIGH		BIT(14)
 #define KSZPHY_RMII_REF_CLK_SEL			BIT(7)
 
 /* Write/read to/from extended registers */

@@ -75,6 +73,7 @@
 
 struct kszphy_type {
 	u32 led_mode_reg;
+	u16 interrupt_level_mask;
 	bool has_broadcast_disable;
 	bool has_rmii_ref_clk_sel;
 };

@@ -105,6 +104,14 @@ static const struct kszphy_type ksz8081_type = {
 	.has_rmii_ref_clk_sel	= true,
 };
 
+static const struct kszphy_type ks8737_type = {
+	.interrupt_level_mask	= BIT(14),
+};
+
+static const struct kszphy_type ksz9021_type = {
+	.interrupt_level_mask	= BIT(14),
+};
+
 static int kszphy_extended_write(struct phy_device *phydev,
 				u32 regnum, u16 val)
 {

@@ -129,54 +136,31 @@ static int kszphy_ack_interrupt(struct phy_device *phydev)
 	return (rc < 0) ? rc : 0;
 }
 
-static int kszphy_set_interrupt(struct phy_device *phydev)
-{
-	int temp;
-	temp = (PHY_INTERRUPT_ENABLED == phydev->interrupts) ?
-		KSZPHY_INTCS_ALL : 0;
-	return phy_write(phydev, MII_KSZPHY_INTCS, temp);
-}
-
 static int kszphy_config_intr(struct phy_device *phydev)
 {
-	int temp, rc;
-
-	/* set the interrupt pin active low */
-	temp = phy_read(phydev, MII_KSZPHY_CTRL);
-	if (temp < 0)
-		return temp;
-	temp &= ~KSZPHY_CTRL_INT_ACTIVE_HIGH;
-	phy_write(phydev, MII_KSZPHY_CTRL, temp);
-	rc = kszphy_set_interrupt(phydev);
-	return rc < 0 ? rc : 0;
-}
+	const struct kszphy_type *type = phydev->drv->driver_data;
+	int temp;
+	u16 mask;
 
-static int ksz9021_config_intr(struct phy_device *phydev)
-{
-	int temp, rc;
+	if (type && type->interrupt_level_mask)
+		mask = type->interrupt_level_mask;
+	else
+		mask = KSZPHY_CTRL_INT_ACTIVE_HIGH;
 
 	/* set the interrupt pin active low */
 	temp = phy_read(phydev, MII_KSZPHY_CTRL);
 	if (temp < 0)
 		return temp;
-	temp &= ~KSZ9021_CTRL_INT_ACTIVE_HIGH;
+	temp &= ~mask;
 	phy_write(phydev, MII_KSZPHY_CTRL, temp);
-	rc = kszphy_set_interrupt(phydev);
-	return rc < 0 ? rc : 0;
-}
 
-static int ks8737_config_intr(struct phy_device *phydev)
-{
-	int temp, rc;
+	/* enable / disable interrupts */
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+		temp = KSZPHY_INTCS_ALL;
+	else
+		temp = 0;
 
-	/* set the interrupt pin active low */
-	temp = phy_read(phydev, MII_KSZPHY_CTRL);
-	if (temp < 0)
-		return temp;
-	temp &= ~KS8737_CTRL_INT_ACTIVE_HIGH;
-	phy_write(phydev, MII_KSZPHY_CTRL, temp);
-	rc = kszphy_set_interrupt(phydev);
-	return rc < 0 ? rc : 0;
+	return phy_write(phydev, MII_KSZPHY_INTCS, temp);
 }
 
 static int kszphy_rmii_clk_sel(struct phy_device *phydev, bool val)

@@ -581,11 +565,12 @@ static struct phy_driver ksphy_driver[] = {
 	.name		= "Micrel KS8737",
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ks8737_type,
 	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= ks8737_config_intr,
+	.config_intr	= kszphy_config_intr,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE,},

@@ -726,11 +711,12 @@ static struct phy_driver ksphy_driver[] = {
 	.name		= "Micrel KSZ9021 Gigabit PHY",
 	.features	= (PHY_GBIT_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ksz9021_type,
 	.config_init	= ksz9021_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= ksz9021_config_intr,
+	.config_intr	= kszphy_config_intr,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.read_mmd_indirect = ksz9021_rd_mmd_phyreg,

@@ -742,11 +728,12 @@ static struct phy_driver ksphy_driver[] = {
 	.name		= "Micrel KSZ9031 Gigabit PHY",
 	.features	= (PHY_GBIT_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.driver_data	= &ksz9021_type,
 	.config_init	= ksz9031_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= ksz9021_config_intr,
+	.config_intr	= kszphy_config_intr,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE, },
Signature · 1 line
2.0.4
author
#10 lore raw

PATCH 10/22 net: phy: micrel: refactor led-mode error handling

In reply to Johan Hovold

Refactor led-mode error handling.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)
drivers/net/phy/micrel.c +10 -9
index 1b3985cdc64c..ec9ce35e934b 100644
@@ -179,14 +179,19 @@ static int kszphy_setup_led(struct phy_device *phydev,
 	}
 
 	temp = phy_read(phydev, reg);
-	if (temp < 0)
-		return temp;
+	if (temp < 0) {
+		rc = temp;
+		goto out;
+	}
 
 	temp &= ~(3 << shift);
 	temp |= val << shift;
 	rc = phy_write(phydev, reg, temp);
+out:
+	if (rc < 0)
+		dev_err(&phydev->dev, "failed to set led mode\n");
 
-	return rc < 0 ? rc : 0;
+	return rc;
 }
 
 /* Disable PHY address 0 as the broadcast address, so that it can be used as a

@@ -223,9 +228,7 @@ static int ksz8021_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	rc = kszphy_setup_led(phydev, 0x1f, 4);
-	if (rc)
-		dev_err(&phydev->dev, "failed to set led mode\n");
+	kszphy_setup_led(phydev, 0x1f, 4);
 
 	rc = ksz_config_flags(phydev);
 	if (rc < 0)

@@ -240,9 +243,7 @@ static int ks8051_config_init(struct phy_device *phydev)
 {
 	int rc;
 
-	rc = kszphy_setup_led(phydev, 0x1f, 4);
-	if (rc)
-		dev_err(&phydev->dev, "failed to set led mode\n");
+	kszphy_setup_led(phydev, 0x1f, 4);
 
 	rc = ksz_config_flags(phydev);
 	return rc < 0 ? rc : 0;
Signature · 1 line
2.0.4
author
#28 lore raw

RFC 22/22 net: phy: micrel: use generic config_init for KSZ8021/KSZ8031

In reply to Johan Hovold

Use generic config_init callback also for KSZ8021 and KSZ8031.

This has been avoided this far due to commit a95a18afe4c8 ("phy/micrel:
KSZ8031RNL RMII clock reconfiguration bug"), which claims that the PHY becomes unresponsive if the broadcast disable flag is set before configuring the clock mode.

Disabling the broadcast address in generally the first think you want to do, and if PHY 0 is probed first, this will disable the broadcast address for all (Micrel) PHYs on the bus and prevent unnecessary reconfigurations. Note that if parallel probing is ever allowed, this will be a requirement.

Cc Bruno Thomsen <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)
drivers/net/phy/micrel.c +3 -15
index 3b89548e6063..bbd1c9ea0cbf 100644
@@ -88,6 +88,7 @@ struct kszphy_priv {
 
 static const struct kszphy_type ksz8021_type = {
 	.led_mode_reg		= MII_KSZPHY_CTRL_2,
+	.has_broadcast_disable	= true,
 	.has_rmii_ref_clk_sel	= true,
 };
 

@@ -258,19 +259,6 @@ static int kszphy_config_init(struct phy_device *phydev)
 	return 0;
 }
 
-static int ksz8021_config_init(struct phy_device *phydev)
-{
-	int rc;
-
-	rc = kszphy_config_init(phydev);
-	if (rc)
-		return rc;
-
-	rc = kszphy_broadcast_disable(phydev);
-
-	return rc < 0 ? rc : 0;
-}
-
 static int ksz9021_load_values_from_of(struct phy_device *phydev,
 				       struct device_node *of_node, u16 reg,
 				       char *field1, char *field2,

@@ -584,7 +572,7 @@ static struct phy_driver ksphy_driver[] = {
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.driver_data	= &ksz8021_type,
 	.probe		= kszphy_probe,
-	.config_init	= ksz8021_config_init,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,

@@ -601,7 +589,7 @@ static struct phy_driver ksphy_driver[] = {
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.driver_data	= &ksz8021_type,
 	.probe		= kszphy_probe,
-	.config_init	= ksz8021_config_init,
+	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
Signature · 1 line
2.0.4
author
#27 lore raw

PATCH 21/22 net: phy: micrel: add copyright entry

In reply to Johan Hovold

Add myself to the list of copyright holders.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/phy/micrel.c | 1 +
 1 file changed, 1 insertion(+)
drivers/net/phy/micrel.c +1 -0
index 743d5b52d8db..3b89548e6063 100644
@@ -6,6 +6,7 @@
  * Author: David J. Choi
  *
  * Copyright (c) 2010-2013 Micrel, Inc.
+ * Copyright (c) 2014 Johan Hovold <[email protected]>
  *
  * This program is free software; you can redistribute  it and/or modify it
  * under  the terms of  the GNU General  Public License as published by the
Signature · 1 line
2.0.4
maintainer
#29 lore raw
In reply to Johan Hovold

22 patches is just too much, sorry.

Please reduce this down to something closer to ~10 or so patches.

If necessary you can do two series, submit the first 10 and then when that's reviewed and accepted submit the second set.

Thanks.

#20 lore raw

Re: [PATCH 19/22] dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding

In reply to Johan Hovold
Quoted 14 lines · Johan Hovold

Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding documentation.

Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/net/micrel.txt +0 -0
index a1bab5eaae02..9b08dd6551dd 100644

Documentation/devicetree/bindings/net/micrel.txt Optional properties: · line 25

Quoted 7 lines
Documentation/devicetree/bindings/net/micrel.txt +4 -0
@@ -19,6 +19,11 @@ Optional properties:
 
               See the respective PHY datasheet for the mode values.
 
+ - micrel,rmii_ref_clk_sel_25_mhz: rmii_ref_clk_sel bit selects 25 MHz mode
+
+		Whether 25 MHz (rather than 50 Mhz) clock mode is selected
+		when the rmii_ref_clk_sel bit is set.

s/_/-/ in property names please.

That said, I don't follow the meaning. Does this cause the kernel to do something different, or is is simply that a 25MHz ref clock is wired up?

Surely that should be described via the common clock bindings? Or if internal through a clock-frequency property?

Mark.

author
#30 lore raw
In reply to David Miller
Quoted 6 lines · David Miller

22 patches is just too much, sorry.

Please reduce this down to something closer to ~10 or so patches.

If necessary you can do two series, submit the first 10 and then when that's reviewed and accepted submit the second set.

No problem. The three driver-registration patches are really self-contained and could be reviewed and applied separately, and then I split the remaining ones where I add the device-type abstraction, that is:

Series 1:
net: phy: add module_phy_driver macro
net: phy: replace phy_driver_register calls
net: phy: replace phy_drivers_register calls

Series 2:
dt/bindings: fix documentation of ethernet-phy compatible property
net: phy: micrel: fix config_intr error handling
net: phy: micrel: use BIT macro
net: phy: micrel: refactor broadcast disable
net: phy: micrel: disable broadcast for KSZ8081/KSZ8091
net: phy: micrel: add led-mode sanity check
net: phy: micrel: refactor led-mode error handling
net: phy: micrel: clean up led-mode setup
net: phy: micrel: enable led-mode for KSZ8081/KSZ8091

Series 3 (submitted after review of Series 2):
net: phy: add static data field to struct phy_driver
net: phy: micrel: add device-type abstraction
net: phy: micrel: parse of nodes at probe
net: phy: micrel: add has-broadcast-disable flag to type data
net: phy: micrel: add generic rmii-ref-clk-sel support
net: phy: micrel: add support for rmii_ref_clk_sel to KSZ8081/KSZ8091 dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding
net: phy: micrel: refactor interrupt config
net: phy: micrel: add copyright entry
net: phy: micrel: use generic config_init for KSZ8021/KSZ8031

Thanks,
Johan

#31 lore raw
Quoted 5 lines · Johan Hovold
Quoted 4 lines · David Miller

22 patches is just too much, sorry.

Please reduce this down to something closer to ~10 or so patches.

Quoted 7 lines
Quoted 2 lines

If necessary you can do two series, submit the first 10 and then when that's reviewed and accepted submit the second set.

No problem. The three driver-registration patches are really self-contained and could be reviewed and applied separately, and then I split the remaining ones where I add the device-type abstraction, that is:

Sounds like a reasonable plan to me. Thanks!

Quoted 32 lines

Series 1:
net: phy: add module_phy_driver macro
net: phy: replace phy_driver_register calls
net: phy: replace phy_drivers_register calls

Series 2:
dt/bindings: fix documentation of ethernet-phy compatible property
net: phy: micrel: fix config_intr error handling
net: phy: micrel: use BIT macro
net: phy: micrel: refactor broadcast disable
net: phy: micrel: disable broadcast for KSZ8081/KSZ8091
net: phy: micrel: add led-mode sanity check
net: phy: micrel: refactor led-mode error handling
net: phy: micrel: clean up led-mode setup
net: phy: micrel: enable led-mode for KSZ8081/KSZ8091

Series 3 (submitted after review of Series 2):
net: phy: add static data field to struct phy_driver
net: phy: micrel: add device-type abstraction
net: phy: micrel: parse of nodes at probe
net: phy: micrel: add has-broadcast-disable flag to type data
net: phy: micrel: add generic rmii-ref-clk-sel support
net: phy: micrel: add support for rmii_ref_clk_sel to KSZ8081/KSZ8091 dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding
net: phy: micrel: refactor interrupt config
net: phy: micrel: add copyright entry
net: phy: micrel: use generic config_init for KSZ8021/KSZ8031

Thanks,
Johan

author
#21 lore raw

Re: [PATCH 19/22] dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding

In reply to Mark Rutland
Quoted 16 lines · Mark Rutland
Quoted 15 lines · Johan Hovold

Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding documentation.

Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/net/micrel.txt +0 -0
index a1bab5eaae02..9b08dd6551dd 100644
@@ -19,6 +19,11 @@ Optional properties:
Quoted 8 lines

Documentation/devicetree/bindings/net/micrel.txt Optional properties: · line 25

Quoted 6 lines
Documentation/devicetree/bindings/net/micrel.txt +4 -0
               See the respective PHY datasheet for the mode values.
 
+ - micrel,rmii_ref_clk_sel_25_mhz: rmii_ref_clk_sel bit selects 25 MHz mode
+
+		Whether 25 MHz (rather than 50 Mhz) clock mode is selected
+		when the rmii_ref_clk_sel bit is set.

s/_/-/ in property names please.

Ouch, copied from variable name, sorry.

Quoted 2 lines

That said, I don't follow the meaning. Does this cause the kernel to do something different, or is is simply that a 25MHz ref clock is wired up?

Yes, the driver currently sets this configuration bit based on a common clock binding.

However, it turns out the meaning of the bit is reversed on some PHY variants. On most PHYs 50 MHz mode is selected by setting this bit, whereas on the PHYs that need this new property, setting it selects 25 MHz mode instead.

Quoted 2 lines

Surely that should be described via the common clock bindings? Or if internal through a clock-frequency property?

The driver currently selects the mode using the common clock bindings, but this new property is needed to properly handle those PHY variants on which the clock configuration bit has the reverse meaning.

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#22 lore raw

Re: [PATCH 19/22] dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding

Quoted 31 lines · Johan Hovold
Quoted 16 lines · Mark Rutland
Quoted 15 lines · Johan Hovold

Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding documentation.

Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/net/micrel.txt +0 -0
index a1bab5eaae02..9b08dd6551dd 100644
@@ -19,6 +19,11 @@ Optional properties:
Quoted 8 lines

Documentation/devicetree/bindings/net/micrel.txt Optional properties: · line 25

Quoted 6 lines
Documentation/devicetree/bindings/net/micrel.txt +4 -0
               See the respective PHY datasheet for the mode values.
 
+ - micrel,rmii_ref_clk_sel_25_mhz: rmii_ref_clk_sel bit selects 25 MHz mode
+
+		Whether 25 MHz (rather than 50 Mhz) clock mode is selected
+		when the rmii_ref_clk_sel bit is set.

s/_/-/ in property names please.

Ouch, copied from variable name, sorry.

Quoted 2 lines

That said, I don't follow the meaning. Does this cause the kernel to do something different, or is is simply that a 25MHz ref clock is wired up?

Quoted 7 lines

Yes, the driver currently sets this configuration bit based on a common clock binding.

However, it turns out the meaning of the bit is reversed on some PHY variants. On most PHYs 50 MHz mode is selected by setting this bit, whereas on the PHYs that need this new property, setting it selects 25 MHz mode instead.

Maybe rename the property to something like rmii-ref-clk-25mhz-active-high then? Also you should probably make it more explicit that this is a hardware property and not for adjusting the clock.

It's not very nice from Micrel to create Phys with exactly the same device id but the meaning of this bit reversed.

Sascha

Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
author
#23 lore raw

Re: [PATCH 19/22] dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding

Quoted 35 lines · Sascha Hauer
Quoted 34 lines · Johan Hovold
Quoted 16 lines · Mark Rutland
Quoted 15 lines · Johan Hovold

Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding documentation.

Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/net/micrel.txt +0 -0
index a1bab5eaae02..9b08dd6551dd 100644
@@ -19,6 +19,11 @@ Optional properties:
Quoted 8 lines

Documentation/devicetree/bindings/net/micrel.txt Optional properties: · line 25

Quoted 6 lines
Documentation/devicetree/bindings/net/micrel.txt +4 -0
               See the respective PHY datasheet for the mode values.
 
+ - micrel,rmii_ref_clk_sel_25_mhz: rmii_ref_clk_sel bit selects 25 MHz mode
+
+		Whether 25 MHz (rather than 50 Mhz) clock mode is selected
+		when the rmii_ref_clk_sel bit is set.

s/_/-/ in property names please.

Ouch, copied from variable name, sorry.

Quoted 2 lines

That said, I don't follow the meaning. Does this cause the kernel to do something different, or is is simply that a 25MHz ref clock is wired up?

Yes, the driver currently sets this configuration bit based on a common clock binding.

Quoted 8 lines
Quoted 4 lines

However, it turns out the meaning of the bit is reversed on some PHY variants. On most PHYs 50 MHz mode is selected by setting this bit, whereas on the PHYs that need this new property, setting it selects 25 MHz mode instead.

Maybe rename the property to something like rmii-ref-clk-25mhz-active-high then? Also you should probably make it more explicit that this is a hardware property and not for adjusting the clock.

You're right, but how about calling it

micrel,rmii-reference-clock-select-inverted

Then no one will set it believing it will change the clock mode and without reading the binding doc first.

The description could then read something like

micrel,rmii-reference-clock-select-inverted: RMII Reference
Clock Select bit is inverted

The RMII Reference Clock Select bit is inverted so that setting it selects 25 MHz rather than 50 MHz clock mode.

Note that this is only needed for PHY variants that has this bit inverted and that a clock reference ("rmii-ref" below) is always needed to select the actual mode.

Quoted 2 lines

It's not very nice from Micrel to create Phys with exactly the same device id but the meaning of this bit reversed.

Not very nice at all.

Thanks,
Johan

#24 lore raw

Re: [PATCH 19/22] dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding

Quoted 57 lines · Johan Hovold
Quoted 35 lines · Sascha Hauer
Quoted 34 lines · Johan Hovold
Quoted 16 lines · Mark Rutland
Quoted 15 lines · Johan Hovold

Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding documentation.

Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/net/micrel.txt +0 -0
index a1bab5eaae02..9b08dd6551dd 100644
@@ -19,6 +19,11 @@ Optional properties:
Quoted 8 lines

Documentation/devicetree/bindings/net/micrel.txt Optional properties: · line 25

Quoted 6 lines
Documentation/devicetree/bindings/net/micrel.txt +4 -0
               See the respective PHY datasheet for the mode values.
 
+ - micrel,rmii_ref_clk_sel_25_mhz: rmii_ref_clk_sel bit selects 25 MHz mode
+
+		Whether 25 MHz (rather than 50 Mhz) clock mode is selected
+		when the rmii_ref_clk_sel bit is set.

s/_/-/ in property names please.

Ouch, copied from variable name, sorry.

Quoted 2 lines

That said, I don't follow the meaning. Does this cause the kernel to do something different, or is is simply that a 25MHz ref clock is wired up?

Yes, the driver currently sets this configuration bit based on a common clock binding.

Quoted 8 lines
Quoted 4 lines

However, it turns out the meaning of the bit is reversed on some PHY variants. On most PHYs 50 MHz mode is selected by setting this bit, whereas on the PHYs that need this new property, setting it selects 25 MHz mode instead.

Maybe rename the property to something like rmii-ref-clk-25mhz-active-high then? Also you should probably make it more explicit that this is a hardware property and not for adjusting the clock.

You're right, but how about calling it

micrel,rmii-reference-clock-select-inverted

Then no one will set it believing it will change the clock mode and without reading the binding doc first.

The description could then read something like

micrel,rmii-reference-clock-select-inverted: RMII Reference
Clock Select bit is inverted

Quoted 6 lines

The RMII Reference Clock Select bit is inverted so that setting it selects 25 MHz rather than 50 MHz clock mode.

Note that this is only needed for PHY variants that has this bit inverted and that a clock reference ("rmii-ref" below) is always needed to select the actual mode.

"Inverted" only has a meaning when everybody agrees what's the normal case. Since that not the case I really prefer talking about "active-high" or "active-low".

Sascha

Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
author
#25 lore raw

Re: [PATCH 19/22] dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding

Quoted 61 lines · Sascha Hauer
Quoted 60 lines · Johan Hovold
Quoted 35 lines · Sascha Hauer
Quoted 34 lines · Johan Hovold
Quoted 16 lines · Mark Rutland
Quoted 15 lines · Johan Hovold

Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding documentation.

Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/net/micrel.txt +0 -0
index a1bab5eaae02..9b08dd6551dd 100644
@@ -19,6 +19,11 @@ Optional properties:
Quoted 8 lines

Documentation/devicetree/bindings/net/micrel.txt Optional properties: · line 25

Quoted 6 lines
Documentation/devicetree/bindings/net/micrel.txt +4 -0
               See the respective PHY datasheet for the mode values.
 
+ - micrel,rmii_ref_clk_sel_25_mhz: rmii_ref_clk_sel bit selects 25 MHz mode
+
+		Whether 25 MHz (rather than 50 Mhz) clock mode is selected
+		when the rmii_ref_clk_sel bit is set.

s/_/-/ in property names please.

Ouch, copied from variable name, sorry.

Quoted 2 lines

That said, I don't follow the meaning. Does this cause the kernel to do something different, or is is simply that a 25MHz ref clock is wired up?

Yes, the driver currently sets this configuration bit based on a common clock binding.

Quoted 8 lines
Quoted 4 lines

However, it turns out the meaning of the bit is reversed on some PHY variants. On most PHYs 50 MHz mode is selected by setting this bit, whereas on the PHYs that need this new property, setting it selects 25 MHz mode instead.

Maybe rename the property to something like rmii-ref-clk-25mhz-active-high then? Also you should probably make it more explicit that this is a hardware property and not for adjusting the clock.

You're right, but how about calling it

micrel,rmii-reference-clock-select-inverted

Then no one will set it believing it will change the clock mode and without reading the binding doc first.

The description could then read something like

micrel,rmii-reference-clock-select-inverted: RMII Reference
Clock Select bit is inverted

The RMII Reference Clock Select bit is inverted so that setting it selects 25 MHz rather than 50 MHz clock mode.

Quoted 7 lines
Quoted 3 lines

Note that this is only needed for PHY variants that has this bit inverted and that a clock reference ("rmii-ref" below) is always needed to select the actual mode.

"Inverted" only has a meaning when everybody agrees what's the normal case. Since that not the case I really prefer talking about "active-high" or "active-low".

Yes, but I'm reluctant to using "active-high" and "active-low" as this is not a signal (or IO pin) we're dealing with.

It's a configuration bit, which (if set) selects 25 MHz mode. Hence I still think something like

	micrel,rmii_ref_clk_sel_25_mhz

or

micrel,rmii_reference_clock_select_selects_25_mhz

is preferred. The binding documentation will still make it clear that a clocks reference is also needed.

Johan