[PATCH 00/22] net: phy: refactoring and Micrel features

[PATCH 00/22] net: phy: refactoring and Micrel features

[Johan Hovold]

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(-)

-- 
2.0.4

[PATCH 01/22] dt/bindings: fix documentation of ethernet-phy compatible property

[Johan Hovold]

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: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/net/phy.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index 5b8c58903077..40831fbaff72 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -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 {
-- 
2.0.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/22] net: phy: add module_phy_driver macro

[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 <johan@kernel.org>
---
 include/linux/phy.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index ed39956b5613..c50e1f1f46e0 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -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 */
-- 
2.0.4

[PATCH 03/22] net: phy: replace phy_driver_register calls

[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 <johan@kernel.org>
---
 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(-)

diff --git a/drivers/net/phy/amd.c b/drivers/net/phy/amd.c
index a3fb5ceb6487..65a488f82eb8 100644
--- a/drivers/net/phy/amd.c
+++ b/drivers/net/phy/amd.c
@@ -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 },
diff --git a/drivers/net/phy/et1011c.c b/drivers/net/phy/et1011c.c
index a8eb19ec3183..a907743816a8 100644
--- a/drivers/net/phy/et1011c.c
+++ b/drivers/net/phy/et1011c.c
@@ -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 },
diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c
index 9a5f234d95b0..0a7b9c7f09a2 100644
--- a/drivers/net/phy/national.c
+++ b/drivers/net/phy/national.c
@@ -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 },
 	{ }
diff --git a/drivers/net/phy/qsemi.c b/drivers/net/phy/qsemi.c
index fe0d0a15d5e1..be4c6f7c3645 100644
--- a/drivers/net/phy/qsemi.c
+++ b/drivers/net/phy/qsemi.c
@@ -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 },
-- 
2.0.4

[PATCH 04/22] net: phy: replace phy_drivers_register calls

[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 <johan@kernel.org>
---
 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(-)

diff --git a/drivers/net/phy/amd-xgbe-phy.c b/drivers/net/phy/amd-xgbe-phy.c
index f3230eef41fd..52e7427c0a38 100644
--- a/drivers/net/phy/amd-xgbe-phy.c
+++ b/drivers/net/phy/amd-xgbe-phy.c
@@ -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 },
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index fdc1b418fa6a..f80e19ac6704 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -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 },
diff --git a/drivers/net/phy/bcm63xx.c b/drivers/net/phy/bcm63xx.c
index ac55b0807853..830ec31f952f 100644
--- a/drivers/net/phy/bcm63xx.c
+++ b/drivers/net/phy/bcm63xx.c
@@ -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 },
diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index fdce1ea28790..f9de20f93cb8 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -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);
 
diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c
index 799789518e87..1eca20452f03 100644
--- a/drivers/net/phy/bcm87xx.c
+++ b/drivers/net/phy/bcm87xx.c
@@ -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");
diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 34088d60da74..4e2abfb8552c 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -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 },
diff --git a/drivers/net/phy/cicada.c b/drivers/net/phy/cicada.c
index b57ce0cc9657..27f5464899d4 100644
--- a/drivers/net/phy/cicada.c
+++ b/drivers/net/phy/cicada.c
@@ -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 },
diff --git a/drivers/net/phy/davicom.c b/drivers/net/phy/davicom.c
index d2c08f625a41..0d16c7d9e1bf 100644
--- a/drivers/net/phy/davicom.c
+++ b/drivers/net/phy/davicom.c
@@ -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 },
diff --git a/drivers/net/phy/icplus.c b/drivers/net/phy/icplus.c
index 97bf58bf4939..8644f039d922 100644
--- a/drivers/net/phy/icplus.c
+++ b/drivers/net/phy/icplus.c
@@ -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 },
diff --git a/drivers/net/phy/lxt.c b/drivers/net/phy/lxt.c
index 9108f3191701..a3a5a703635b 100644
--- a/drivers/net/phy/lxt.c
+++ b/drivers/net/phy/lxt.c
@@ -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 },
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index bd37e45c89c0..708facd78612 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -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 },
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 8c2a29a9bd7f..bcc6c0ea75fa 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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");
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 45483fdfbe06..96a0f0fab3ca 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -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 },
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index a4b08198fb9f..c0f6479e19d4 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -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 },
diff --git a/drivers/net/phy/ste10Xp.c b/drivers/net/phy/ste10Xp.c
index 5e1eb138916f..3fc199b773e6 100644
--- a/drivers/net/phy/ste10Xp.c
+++ b/drivers/net/phy/ste10Xp.c
@@ -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 },
diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c
index 5dc0935da99c..76cad712ddb2 100644
--- a/drivers/net/phy/vitesse.c
+++ b/drivers/net/phy/vitesse.c
@@ -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 },
-- 
2.0.4

[PATCH 05/22] net: phy: micrel: fix config_intr error handling

[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 <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index bcc6c0ea75fa..62ca9613a514 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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);
-- 
2.0.4

[PATCH 06/22] net: phy: micrel: use BIT macro

[Johan Hovold]

Use BIT macro for bitmask definitions.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 62ca9613a514..d962a2866bba 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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;
-- 
2.0.4

[PATCH 07/22] net: phy: micrel: refactor broadcast disable

[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 <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index d962a2866bba..21abe5ade3df 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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;
 }
 
-- 
2.0.4

[PATCH 08/22] net: phy: micrel: disable broadcast for KSZ8081/KSZ8091

[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 <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 21abe5ade3df..16135ac18bfe 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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,
-- 
2.0.4

[PATCH 09/22] net: phy: micrel: add led-mode sanity check

[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 <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 16135ac18bfe..1b3985cdc64c 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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;
-- 
2.0.4

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

[Johan Hovold]

Refactor led-mode error handling.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 1b3985cdc64c..ec9ce35e934b 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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;
-- 
2.0.4

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

[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 <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index ec9ce35e934b..12e18f7273ce 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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;
-- 
2.0.4

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

[Johan Hovold]

Enable led-mode configuration for KSZ8081 and KSZ8091.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 Documentation/devicetree/bindings/net/micrel.txt | 2 ++
 drivers/net/phy/micrel.c                         | 1 +
 2 files changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
index e1d99b95c4ec..30062fae5623 100644
--- a/Documentation/devicetree/bindings/net/micrel.txt
+++ b/Documentation/devicetree/bindings/net/micrel.txt
@@ -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.
 
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 12e18f7273ce..30e894d6ffbd 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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;
 }
-- 
2.0.4

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

[Johan Hovold]

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

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 include/linux/phy.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index c50e1f1f46e0..29c260add1f6 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -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
-- 
2.0.4

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

[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 <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 82 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 69 insertions(+), 13 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 30e894d6ffbd..189fddb829bc 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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,
-- 
2.0.4

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

[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 <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 189fddb829bc..928248187a11 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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;
 }
 
-- 
2.0.4

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

[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 <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 928248187a11..6467bcc5a39d 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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,
-- 
2.0.4

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

[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 <johan@kernel.org>
---
 drivers/net/phy/micrel.c   | 94 +++++++++++++++++++++++++---------------------
 include/linux/micrel_phy.h |  1 -
 2 files changed, 51 insertions(+), 44 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 6467bcc5a39d..d2e790cd3651 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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,
diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index 53d33dee70e1..2e5b194b9b19 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -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
-- 
2.0.4

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

[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 <johan@kernel.org>
---
 Documentation/devicetree/bindings/net/micrel.txt |  4 ++--
 drivers/net/phy/micrel.c                         | 11 ++++++-----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
index 30062fae5623..a1bab5eaae02 100644
--- a/Documentation/devicetree/bindings/net/micrel.txt
+++ b/Documentation/devicetree/bindings/net/micrel.txt
@@ -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.
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index d2e790cd3651..591190384497 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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;
-- 
2.0.4

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

[Johan Hovold]

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

Cc: devicetree@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
index a1bab5eaae02..9b08dd6551dd 100644
--- a/Documentation/devicetree/bindings/net/micrel.txt
+++ b/Documentation/devicetree/bindings/net/micrel.txt
@@ -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:
-- 
2.0.4

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

[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 <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 71 ++++++++++++++++++++----------------------------
 1 file changed, 29 insertions(+), 42 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 591190384497..743d5b52d8db 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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, },
-- 
2.0.4

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

[Johan Hovold]

Add myself to the list of copyright holders.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 743d5b52d8db..3b89548e6063 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -6,6 +6,7 @@
  * Author: David J. Choi
  *
  * Copyright (c) 2010-2013 Micrel, Inc.
+ * Copyright (c) 2014 Johan Hovold <johan@kernel.org>
  *
  * 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
-- 
2.0.4

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

[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 <bth@kamstrup.dk>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 3b89548e6063..bbd1c9ea0cbf 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -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,
-- 
2.0.4

Re: [PATCH 00/22] net: phy: refactoring and Micrel features

[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.

Thanks.

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

[Mark Rutland]

On Tue, Nov 11, 2014 at 05:37:37PM +0000, Johan Hovold wrote:
> Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding
> documentation.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
> index a1bab5eaae02..9b08dd6551dd 100644
> --- a/Documentation/devicetree/bindings/net/micrel.txt
> +++ b/Documentation/devicetree/bindings/net/micrel.txt
> @@ -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.

Re: [PATCH 00/22] net: phy: refactoring and Micrel features

[Johan Hovold]

On Tue, Nov 11, 2014 at 12:49:36PM -0500, David Miller wrote:
> 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

Re: [PATCH 00/22] net: phy: refactoring and Micrel features

[Florian Fainelli]

On 11/11/2014 10:00 AM, Johan Hovold wrote:
> On Tue, Nov 11, 2014 at 12:49:36PM -0500, David Miller wrote:
>> 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:

Sounds like a reasonable plan to me. Thanks!

> 
> 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
> 

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

[Johan Hovold]

On Tue, Nov 11, 2014 at 05:57:42PM +0000, Mark Rutland wrote:
> On Tue, Nov 11, 2014 at 05:37:37PM +0000, Johan Hovold wrote:
> > Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding
> > documentation.
> > 
> > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---
> >  Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
> > index a1bab5eaae02..9b08dd6551dd 100644
> > --- a/Documentation/devicetree/bindings/net/micrel.txt
> > +++ b/Documentation/devicetree/bindings/net/micrel.txt
> > @@ -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.

Ouch, copied from variable name, sorry.

> 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.

> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Sascha Hauer]

On Tue, Nov 11, 2014 at 07:18:25PM +0100, Johan Hovold wrote:
> On Tue, Nov 11, 2014 at 05:57:42PM +0000, Mark Rutland wrote:
> > On Tue, Nov 11, 2014 at 05:37:37PM +0000, Johan Hovold wrote:
> > > Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding
> > > documentation.
> > > 
> > > Cc: devicetree@vger.kernel.org
> > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > ---
> > >  Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
> > > index a1bab5eaae02..9b08dd6551dd 100644
> > > --- a/Documentation/devicetree/bindings/net/micrel.txt
> > > +++ b/Documentation/devicetree/bindings/net/micrel.txt
> > > @@ -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.
> 
> Ouch, copied from variable name, sorry.
> 
> > 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.

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

-- 
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 |

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

[Johan Hovold]

On Wed, Nov 12, 2014 at 08:01:27AM +0100, Sascha Hauer wrote:
> On Tue, Nov 11, 2014 at 07:18:25PM +0100, Johan Hovold wrote:
> > On Tue, Nov 11, 2014 at 05:57:42PM +0000, Mark Rutland wrote:
> > > On Tue, Nov 11, 2014 at 05:37:37PM +0000, Johan Hovold wrote:
> > > > Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding
> > > > documentation.
> > > > 
> > > > Cc: devicetree@vger.kernel.org
> > > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > > ---
> > > >  Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
> > > > index a1bab5eaae02..9b08dd6551dd 100644
> > > > --- a/Documentation/devicetree/bindings/net/micrel.txt
> > > > +++ b/Documentation/devicetree/bindings/net/micrel.txt
> > > > @@ -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.
> > 
> > Ouch, copied from variable name, sorry.
> > 
> > > 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.
> 
> 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.

> 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

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

[Sascha Hauer]

On Wed, Nov 12, 2014 at 10:19:20AM +0100, Johan Hovold wrote:
> On Wed, Nov 12, 2014 at 08:01:27AM +0100, Sascha Hauer wrote:
> > On Tue, Nov 11, 2014 at 07:18:25PM +0100, Johan Hovold wrote:
> > > On Tue, Nov 11, 2014 at 05:57:42PM +0000, Mark Rutland wrote:
> > > > On Tue, Nov 11, 2014 at 05:37:37PM +0000, Johan Hovold wrote:
> > > > > Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding
> > > > > documentation.
> > > > > 
> > > > > Cc: devicetree@vger.kernel.org
> > > > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > > > ---
> > > > >  Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
> > > > >  1 file changed, 5 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
> > > > > index a1bab5eaae02..9b08dd6551dd 100644
> > > > > --- a/Documentation/devicetree/bindings/net/micrel.txt
> > > > > +++ b/Documentation/devicetree/bindings/net/micrel.txt
> > > > > @@ -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.
> > > 
> > > Ouch, copied from variable name, sorry.
> > > 
> > > > 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.
> > 
> > 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.

"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


-- 
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 |

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

[Johan Hovold]

On Thu, Nov 13, 2014 at 09:09:27AM +0100, Sascha Hauer wrote:
> On Wed, Nov 12, 2014 at 10:19:20AM +0100, Johan Hovold wrote:
> > On Wed, Nov 12, 2014 at 08:01:27AM +0100, Sascha Hauer wrote:
> > > On Tue, Nov 11, 2014 at 07:18:25PM +0100, Johan Hovold wrote:
> > > > On Tue, Nov 11, 2014 at 05:57:42PM +0000, Mark Rutland wrote:
> > > > > On Tue, Nov 11, 2014 at 05:37:37PM +0000, Johan Hovold wrote:
> > > > > > Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding
> > > > > > documentation.
> > > > > > 
> > > > > > Cc: devicetree@vger.kernel.org
> > > > > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
> > > > > >  1 file changed, 5 insertions(+)
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
> > > > > > index a1bab5eaae02..9b08dd6551dd 100644
> > > > > > --- a/Documentation/devicetree/bindings/net/micrel.txt
> > > > > > +++ b/Documentation/devicetree/bindings/net/micrel.txt
> > > > > > @@ -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.
> > > > 
> > > > Ouch, copied from variable name, sorry.
> > > > 
> > > > > 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.
> > > 
> > > 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.
> 
> "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