netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] net: phy: dp83867: add binding and support for io_impedance_ctrl nvmem cell
@ 2022-06-06 20:22 Rasmus Villemoes
  2022-06-06 20:22 ` [PATCH net-next 1/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Rasmus Villemoes @ 2022-06-06 20:22 UTC (permalink / raw)
  To: netdev, Andrew Lunn, Heiner Kallweit, Russell King
  Cc: devicetree, Rob Herring, Jakub Kicinski, David S. Miller,
	Dan Murphy, Rasmus Villemoes, linux-kernel

We have a board where measurements indicate that the current three
options - leaving IO_IMPEDANCE_CTRL at the (factory calibrated) reset
value or using one of the two boolean properties to set it to the
min/max value - are too coarse.

This series adds a device tree binding for an nvmem cell which can be
populated during production with a suitable value calibrated for each
board, and corresponding support in the driver. The second patch adds
a trivial phy wrapper for dev_err_probe(), used in the third.


Rasmus Villemoes (3):
  dt-bindings: dp83867: add binding for io_impedance_ctrl nvmem cell
  linux/phy.h: add phydev_err_probe() wrapper for dev_err_probe()
  net: phy: dp83867: implement support for io_impedance_ctrl nvmem cell

 .../devicetree/bindings/net/ti,dp83867.yaml   | 18 +++++-
 drivers/net/phy/dp83867.c                     | 55 +++++++++++++++++--
 include/linux/phy.h                           |  3 +
 3 files changed, 67 insertions(+), 9 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH net-next 1/3] dt-bindings: dp83867: add binding for io_impedance_ctrl nvmem cell
  2022-06-06 20:22 [PATCH net-next 0/3] net: phy: dp83867: add binding and support for io_impedance_ctrl nvmem cell Rasmus Villemoes
@ 2022-06-06 20:22 ` Rasmus Villemoes
  2022-06-06 21:58   ` Andrew Lunn
                     ` (2 more replies)
  2022-06-06 20:22 ` [PATCH net-next 2/3] linux/phy.h: add phydev_err_probe() wrapper for dev_err_probe() Rasmus Villemoes
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 14+ messages in thread
From: Rasmus Villemoes @ 2022-06-06 20:22 UTC (permalink / raw)
  To: netdev, Andrew Lunn, Heiner Kallweit, Russell King
  Cc: devicetree, Rob Herring, Jakub Kicinski, David S. Miller,
	Dan Murphy, Rasmus Villemoes, linux-kernel

We have a board where measurements indicate that the current three
options - leaving IO_IMPEDANCE_CTRL at the (factory calibrated) reset
value or using one of the two boolean properties to set it to the
min/max value - are too coarse.

There is no documented mapping from the 32 possible values of the
IO_IMPEDANCE_CTRL field to values in the range 35-70 ohms, and the
exact mapping is likely to vary from chip to chip. So add a DT binding
for an nvmem cell which can be populated during production with a
value suitable for each specific board.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 .../devicetree/bindings/net/ti,dp83867.yaml    | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.yaml b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
index 047d757e8d82..76ff08a477ba 100644
--- a/Documentation/devicetree/bindings/net/ti,dp83867.yaml
+++ b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
@@ -31,6 +31,16 @@ properties:
   reg:
     maxItems: 1
 
+  nvmem-cells:
+    maxItems: 1
+    description:
+      Nvmem data cell containing the value to write to the
+      IO_IMPEDANCE_CTRL field of the IO_MUX_CFG register.
+
+  nvmem-cell-names:
+    items:
+      - const: io_impedance_ctrl
+
   ti,min-output-impedance:
     type: boolean
     description: |
@@ -42,9 +52,11 @@ properties:
     description: |
       MAC Interface Impedance control to set the programmable output impedance
       to a maximum value (70 ohms).
-      Note: ti,min-output-impedance and ti,max-output-impedance are mutually
-        exclusive. When both properties are present ti,max-output-impedance
-        takes precedence.
+      Note: Specifying an io_impedance_ctrl nvmem cell or one of the
+        ti,min-output-impedance, ti,max-output-impedance properties
+        are mutually exclusive. If more than one is present, an nvmem
+        cell takes precedence over ti,max-output-impedance, which in
+        turn takes precedence over ti,min-output-impedance.
 
   tx-fifo-depth:
     $ref: /schemas/types.yaml#/definitions/uint32
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 2/3] linux/phy.h: add phydev_err_probe() wrapper for dev_err_probe()
  2022-06-06 20:22 [PATCH net-next 0/3] net: phy: dp83867: add binding and support for io_impedance_ctrl nvmem cell Rasmus Villemoes
  2022-06-06 20:22 ` [PATCH net-next 1/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
@ 2022-06-06 20:22 ` Rasmus Villemoes
  2022-06-06 20:22 ` [PATCH net-next 3/3] net: phy: dp83867: implement support for io_impedance_ctrl nvmem cell Rasmus Villemoes
  2022-06-14  8:46 ` [PATCH net-next v2 0/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
  3 siblings, 0 replies; 14+ messages in thread
From: Rasmus Villemoes @ 2022-06-06 20:22 UTC (permalink / raw)
  To: netdev, Andrew Lunn, Heiner Kallweit, Russell King
  Cc: devicetree, Rob Herring, Jakub Kicinski, David S. Miller,
	Dan Murphy, Rasmus Villemoes, linux-kernel

The dev_err_probe() function is quite useful to avoid boilerplate
related to -EPROBE_DEFER handling. Add a phydev_err_probe() helper to
simplify making use of that from phy drivers which otherwise use the
phydev_* helpers.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/phy.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 508f1149665b..bed9a347481b 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1539,6 +1539,9 @@ static inline void phy_device_reset(struct phy_device *phydev, int value)
 #define phydev_err(_phydev, format, args...)	\
 	dev_err(&_phydev->mdio.dev, format, ##args)
 
+#define phydev_err_probe(_phydev, err, format, args...)	\
+	dev_err_probe(&_phydev->mdio.dev, err, format, ##args)
+
 #define phydev_info(_phydev, format, args...)	\
 	dev_info(&_phydev->mdio.dev, format, ##args)
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 3/3] net: phy: dp83867: implement support for io_impedance_ctrl nvmem cell
  2022-06-06 20:22 [PATCH net-next 0/3] net: phy: dp83867: add binding and support for io_impedance_ctrl nvmem cell Rasmus Villemoes
  2022-06-06 20:22 ` [PATCH net-next 1/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
  2022-06-06 20:22 ` [PATCH net-next 2/3] linux/phy.h: add phydev_err_probe() wrapper for dev_err_probe() Rasmus Villemoes
@ 2022-06-06 20:22 ` Rasmus Villemoes
  2022-06-14  8:46 ` [PATCH net-next v2 0/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
  3 siblings, 0 replies; 14+ messages in thread
From: Rasmus Villemoes @ 2022-06-06 20:22 UTC (permalink / raw)
  To: netdev, Andrew Lunn, Heiner Kallweit, Russell King
  Cc: devicetree, Rob Herring, Jakub Kicinski, David S. Miller,
	Dan Murphy, Rasmus Villemoes, linux-kernel

We have a board where measurements indicate that the current three
options - leaving IO_IMPEDANCE_CTRL at the (factory calibrated) reset
value or using one of the two boolean properties to set it to the
min/max value - are too coarse.

Implement support for the newly added binding allowing device tree to
specify an nvmem cell containing an appropriate value for this
specific board.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/net/phy/dp83867.c | 55 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 8561f2d4443b..45d8a9298251 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -14,6 +14,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/bitfield.h>
+#include <linux/nvmem-consumer.h>
 
 #include <dt-bindings/net/ti-dp83867.h>
 
@@ -521,6 +522,51 @@ static int dp83867_verify_rgmii_cfg(struct phy_device *phydev)
 }
 
 #if IS_ENABLED(CONFIG_OF_MDIO)
+static int dp83867_of_init_io_impedance(struct phy_device *phydev)
+{
+	struct dp83867_private *dp83867 = phydev->priv;
+	struct device *dev = &phydev->mdio.dev;
+	struct device_node *of_node = dev->of_node;
+	struct nvmem_cell *cell;
+	u8 *buf, val;
+	int ret;
+
+	cell = of_nvmem_cell_get(of_node, "io_impedance_ctrl");
+	if (IS_ERR(cell)) {
+		ret = PTR_ERR(cell);
+		if (ret != -ENOENT)
+			return phydev_err_probe(phydev, ret,
+						"failed to get nvmem cell io_impedance_ctrl\n");
+
+		/* If no nvmem cell, check for the boolean properties. */
+		if (of_property_read_bool(of_node, "ti,max-output-impedance"))
+			dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX;
+		else if (of_property_read_bool(of_node, "ti,min-output-impedance"))
+			dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN;
+		else
+			dp83867->io_impedance = -1; /* leave at default */
+
+		return 0;
+	}
+
+	buf = nvmem_cell_read(cell, NULL);
+	nvmem_cell_put(cell);
+
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
+
+	val = *buf;
+	kfree(buf);
+
+	if ((val & DP83867_IO_MUX_CFG_IO_IMPEDANCE_MASK) != val) {
+		phydev_err(phydev, "nvmem cell 'io_impedance_ctrl' contents out of range\n");
+		return -ERANGE;
+	}
+	dp83867->io_impedance = val;
+
+	return 0;
+}
+
 static int dp83867_of_init(struct phy_device *phydev)
 {
 	struct dp83867_private *dp83867 = phydev->priv;
@@ -548,12 +594,9 @@ static int dp83867_of_init(struct phy_device *phydev)
 		}
 	}
 
-	if (of_property_read_bool(of_node, "ti,max-output-impedance"))
-		dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX;
-	else if (of_property_read_bool(of_node, "ti,min-output-impedance"))
-		dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN;
-	else
-		dp83867->io_impedance = -1; /* leave at default */
+	ret = dp83867_of_init_io_impedance(phydev);
+	if (ret)
+		return ret;
 
 	dp83867->rxctrl_strap_quirk = of_property_read_bool(of_node,
 							    "ti,dp83867-rxctrl-strap-quirk");
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next 1/3] dt-bindings: dp83867: add binding for io_impedance_ctrl nvmem cell
  2022-06-06 20:22 ` [PATCH net-next 1/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
@ 2022-06-06 21:58   ` Andrew Lunn
  2022-06-07 11:54     ` Rasmus Villemoes
  2022-06-09 20:04   ` Rob Herring
  2022-06-14 18:46   ` Andrew Lunn
  2 siblings, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2022-06-06 21:58 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: netdev, Heiner Kallweit, Russell King, devicetree, Rob Herring,
	Jakub Kicinski, David S. Miller, Dan Murphy, linux-kernel

> There is no documented mapping from the 32 possible values of the
> IO_IMPEDANCE_CTRL field to values in the range 35-70 ohms

There have been a few active TI engineers submitting patches to TI PHY
drivers. Please could you reach out to them and ask if they can
provide documentation.

Having magic values in DT is not the preferred why to use it. Ideally
you should store Ohms in the cell and convert to the register value.

    Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next 1/3] dt-bindings: dp83867: add binding for io_impedance_ctrl nvmem cell
  2022-06-06 21:58   ` Andrew Lunn
@ 2022-06-07 11:54     ` Rasmus Villemoes
  2022-06-14  5:40       ` Jakub Kicinski
  0 siblings, 1 reply; 14+ messages in thread
From: Rasmus Villemoes @ 2022-06-07 11:54 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, Heiner Kallweit, Russell King, devicetree, Rob Herring,
	Jakub Kicinski, David S. Miller, Dan Murphy, linux-kernel

On 06/06/2022 23.58, Andrew Lunn wrote:
>> There is no documented mapping from the 32 possible values of the
>> IO_IMPEDANCE_CTRL field to values in the range 35-70 ohms
> 
> There have been a few active TI engineers submitting patches to TI PHY
> drivers. Please could you reach out to them and ask if they can
> provide documentation.
>
> Having magic values in DT is not the preferred why to use it. Ideally
> you should store Ohms in the cell and convert to the register value.

We've already asked TI for more detailed information, but apparently the
data sheet already says all there is to know. I should have worded the
commit message differently. Something like

  There is no fixed mapping from register values to values in the range
  35-70 ohms; it varies from chip to chip, and even that target range is
  approximate.

So AFAICS the only meaningful thing to store in an nvmem cell is an
appropriate (per-board) raw value of that field.

I would think this would be very similar to how various sensors have
nvmem cells defining calibration data.

Rasmus


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next 1/3] dt-bindings: dp83867: add binding for io_impedance_ctrl nvmem cell
  2022-06-06 20:22 ` [PATCH net-next 1/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
  2022-06-06 21:58   ` Andrew Lunn
@ 2022-06-09 20:04   ` Rob Herring
  2022-06-14 18:46   ` Andrew Lunn
  2 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2022-06-09 20:04 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Jakub Kicinski, Heiner Kallweit, devicetree, Rob Herring,
	Andrew Lunn, Dan Murphy, netdev, linux-kernel, Russell King,
	David S. Miller

On Mon, 06 Jun 2022 22:22:18 +0200, Rasmus Villemoes wrote:
> We have a board where measurements indicate that the current three
> options - leaving IO_IMPEDANCE_CTRL at the (factory calibrated) reset
> value or using one of the two boolean properties to set it to the
> min/max value - are too coarse.
> 
> There is no documented mapping from the 32 possible values of the
> IO_IMPEDANCE_CTRL field to values in the range 35-70 ohms, and the
> exact mapping is likely to vary from chip to chip. So add a DT binding
> for an nvmem cell which can be populated during production with a
> value suitable for each specific board.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  .../devicetree/bindings/net/ti,dp83867.yaml    | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next 1/3] dt-bindings: dp83867: add binding for io_impedance_ctrl nvmem cell
  2022-06-07 11:54     ` Rasmus Villemoes
@ 2022-06-14  5:40       ` Jakub Kicinski
  0 siblings, 0 replies; 14+ messages in thread
From: Jakub Kicinski @ 2022-06-14  5:40 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Andrew Lunn, netdev, Heiner Kallweit, Russell King, devicetree,
	Rob Herring, David S. Miller, Dan Murphy, linux-kernel

On Tue, 7 Jun 2022 13:54:30 +0200 Rasmus Villemoes wrote:
> On 06/06/2022 23.58, Andrew Lunn wrote:
> >> There is no documented mapping from the 32 possible values of the
> >> IO_IMPEDANCE_CTRL field to values in the range 35-70 ohms  
> > 
> > There have been a few active TI engineers submitting patches to TI PHY
> > drivers. Please could you reach out to them and ask if they can
> > provide documentation.
> >
> > Having magic values in DT is not the preferred why to use it. Ideally
> > you should store Ohms in the cell and convert to the register value.  
> 
> We've already asked TI for more detailed information, but apparently the
> data sheet already says all there is to know. I should have worded the
> commit message differently. Something like
> 
>   There is no fixed mapping from register values to values in the range
>   35-70 ohms; it varies from chip to chip, and even that target range is
>   approximate.

The series was waiting for Andrew to come back but it ended up getting
marked as Changes Requested in PW. Would you mind reposting with the
modification to the commit message?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH net-next v2 0/3] dt-bindings: dp83867: add binding for io_impedance_ctrl nvmem cell
  2022-06-06 20:22 [PATCH net-next 0/3] net: phy: dp83867: add binding and support for io_impedance_ctrl nvmem cell Rasmus Villemoes
                   ` (2 preceding siblings ...)
  2022-06-06 20:22 ` [PATCH net-next 3/3] net: phy: dp83867: implement support for io_impedance_ctrl nvmem cell Rasmus Villemoes
@ 2022-06-14  8:46 ` Rasmus Villemoes
  2022-06-14  8:46   ` [PATCH net-next v2 1/3] " Rasmus Villemoes
                     ` (3 more replies)
  3 siblings, 4 replies; 14+ messages in thread
From: Rasmus Villemoes @ 2022-06-14  8:46 UTC (permalink / raw)
  To: netdev, Andrew Lunn, Heiner Kallweit, Russell King
  Cc: devicetree, Rob Herring, Jakub Kicinski, David S. Miller,
	Grygorii Strashko, Praneeth Bajjuri, linux-kernel,
	Rasmus Villemoes

We have a board where measurements indicate that the current three
options - leaving IO_IMPEDANCE_CTRL at the reset value (which is
factory calibrated to a value corresponding to approximately 50 ohms)
or using one of the two boolean properties to set it to the min/max
value - are too coarse.

This series adds a device tree binding for an nvmem cell which can be
populated during production with a suitable value calibrated for each
board, and corresponding support in the driver. The second patch adds
a trivial phy wrapper for dev_err_probe(), used in the third.

v2: Improve commit message for patch 1; add Rob's R-b to patch 1. No
changes in the patches themselves.

Since Dan Murphy's email address bounces, cc'ing two other @ti.com
addresses that have touched the driver in recent years.

Rasmus Villemoes (3):
  dt-bindings: dp83867: add binding for io_impedance_ctrl nvmem cell
  linux/phy.h: add phydev_err_probe() wrapper for dev_err_probe()
  net: phy: dp83867: implement support for io_impedance_ctrl nvmem cell

 .../devicetree/bindings/net/ti,dp83867.yaml   | 18 +++++-
 drivers/net/phy/dp83867.c                     | 55 +++++++++++++++++--
 include/linux/phy.h                           |  3 +
 3 files changed, 67 insertions(+), 9 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH net-next v2 1/3] dt-bindings: dp83867: add binding for io_impedance_ctrl nvmem cell
  2022-06-14  8:46 ` [PATCH net-next v2 0/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
@ 2022-06-14  8:46   ` Rasmus Villemoes
  2022-06-14  8:46   ` [PATCH net-next v2 2/3] linux/phy.h: add phydev_err_probe() wrapper for dev_err_probe() Rasmus Villemoes
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Rasmus Villemoes @ 2022-06-14  8:46 UTC (permalink / raw)
  To: netdev, Andrew Lunn, Heiner Kallweit, Russell King
  Cc: devicetree, Rob Herring, Jakub Kicinski, David S. Miller,
	Grygorii Strashko, Praneeth Bajjuri, linux-kernel,
	Rasmus Villemoes, Rob Herring

We have a board where measurements indicate that the current three
options - leaving IO_IMPEDANCE_CTRL at the reset value (which is
factory calibrated to a value corresponding to approximately 50 ohms)
or using one of the two boolean properties to set it to the min/max
value - are too coarse.

There is no fixed mapping from register values to values in the range
35-70 ohms; it varies from chip to chip, and even that target range is
approximate. So add a DT binding for an nvmem cell which can be
populated during production with a value suitable for each specific
board.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 .../devicetree/bindings/net/ti,dp83867.yaml    | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.yaml b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
index 047d757e8d82..76ff08a477ba 100644
--- a/Documentation/devicetree/bindings/net/ti,dp83867.yaml
+++ b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
@@ -31,6 +31,16 @@ properties:
   reg:
     maxItems: 1
 
+  nvmem-cells:
+    maxItems: 1
+    description:
+      Nvmem data cell containing the value to write to the
+      IO_IMPEDANCE_CTRL field of the IO_MUX_CFG register.
+
+  nvmem-cell-names:
+    items:
+      - const: io_impedance_ctrl
+
   ti,min-output-impedance:
     type: boolean
     description: |
@@ -42,9 +52,11 @@ properties:
     description: |
       MAC Interface Impedance control to set the programmable output impedance
       to a maximum value (70 ohms).
-      Note: ti,min-output-impedance and ti,max-output-impedance are mutually
-        exclusive. When both properties are present ti,max-output-impedance
-        takes precedence.
+      Note: Specifying an io_impedance_ctrl nvmem cell or one of the
+        ti,min-output-impedance, ti,max-output-impedance properties
+        are mutually exclusive. If more than one is present, an nvmem
+        cell takes precedence over ti,max-output-impedance, which in
+        turn takes precedence over ti,min-output-impedance.
 
   tx-fifo-depth:
     $ref: /schemas/types.yaml#/definitions/uint32
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v2 2/3] linux/phy.h: add phydev_err_probe() wrapper for dev_err_probe()
  2022-06-14  8:46 ` [PATCH net-next v2 0/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
  2022-06-14  8:46   ` [PATCH net-next v2 1/3] " Rasmus Villemoes
@ 2022-06-14  8:46   ` Rasmus Villemoes
  2022-06-14  8:46   ` [PATCH net-next v2 3/3] net: phy: dp83867: implement support for io_impedance_ctrl nvmem cell Rasmus Villemoes
  2022-06-17  3:50   ` [PATCH net-next v2 0/3] dt-bindings: dp83867: add binding " patchwork-bot+netdevbpf
  3 siblings, 0 replies; 14+ messages in thread
From: Rasmus Villemoes @ 2022-06-14  8:46 UTC (permalink / raw)
  To: netdev, Andrew Lunn, Heiner Kallweit, Russell King
  Cc: devicetree, Rob Herring, Jakub Kicinski, David S. Miller,
	Grygorii Strashko, Praneeth Bajjuri, linux-kernel,
	Rasmus Villemoes

The dev_err_probe() function is quite useful to avoid boilerplate
related to -EPROBE_DEFER handling. Add a phydev_err_probe() helper to
simplify making use of that from phy drivers which otherwise use the
phydev_* helpers.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/phy.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 508f1149665b..bed9a347481b 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1539,6 +1539,9 @@ static inline void phy_device_reset(struct phy_device *phydev, int value)
 #define phydev_err(_phydev, format, args...)	\
 	dev_err(&_phydev->mdio.dev, format, ##args)
 
+#define phydev_err_probe(_phydev, err, format, args...)	\
+	dev_err_probe(&_phydev->mdio.dev, err, format, ##args)
+
 #define phydev_info(_phydev, format, args...)	\
 	dev_info(&_phydev->mdio.dev, format, ##args)
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v2 3/3] net: phy: dp83867: implement support for io_impedance_ctrl nvmem cell
  2022-06-14  8:46 ` [PATCH net-next v2 0/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
  2022-06-14  8:46   ` [PATCH net-next v2 1/3] " Rasmus Villemoes
  2022-06-14  8:46   ` [PATCH net-next v2 2/3] linux/phy.h: add phydev_err_probe() wrapper for dev_err_probe() Rasmus Villemoes
@ 2022-06-14  8:46   ` Rasmus Villemoes
  2022-06-17  3:50   ` [PATCH net-next v2 0/3] dt-bindings: dp83867: add binding " patchwork-bot+netdevbpf
  3 siblings, 0 replies; 14+ messages in thread
From: Rasmus Villemoes @ 2022-06-14  8:46 UTC (permalink / raw)
  To: netdev, Andrew Lunn, Heiner Kallweit, Russell King
  Cc: devicetree, Rob Herring, Jakub Kicinski, David S. Miller,
	Grygorii Strashko, Praneeth Bajjuri, linux-kernel,
	Rasmus Villemoes

We have a board where measurements indicate that the current three
options - leaving IO_IMPEDANCE_CTRL at the (factory calibrated) reset
value or using one of the two boolean properties to set it to the
min/max value - are too coarse.

Implement support for the newly added binding allowing device tree to
specify an nvmem cell containing an appropriate value for this
specific board.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/net/phy/dp83867.c | 55 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 8561f2d4443b..45d8a9298251 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -14,6 +14,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/bitfield.h>
+#include <linux/nvmem-consumer.h>
 
 #include <dt-bindings/net/ti-dp83867.h>
 
@@ -521,6 +522,51 @@ static int dp83867_verify_rgmii_cfg(struct phy_device *phydev)
 }
 
 #if IS_ENABLED(CONFIG_OF_MDIO)
+static int dp83867_of_init_io_impedance(struct phy_device *phydev)
+{
+	struct dp83867_private *dp83867 = phydev->priv;
+	struct device *dev = &phydev->mdio.dev;
+	struct device_node *of_node = dev->of_node;
+	struct nvmem_cell *cell;
+	u8 *buf, val;
+	int ret;
+
+	cell = of_nvmem_cell_get(of_node, "io_impedance_ctrl");
+	if (IS_ERR(cell)) {
+		ret = PTR_ERR(cell);
+		if (ret != -ENOENT)
+			return phydev_err_probe(phydev, ret,
+						"failed to get nvmem cell io_impedance_ctrl\n");
+
+		/* If no nvmem cell, check for the boolean properties. */
+		if (of_property_read_bool(of_node, "ti,max-output-impedance"))
+			dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX;
+		else if (of_property_read_bool(of_node, "ti,min-output-impedance"))
+			dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN;
+		else
+			dp83867->io_impedance = -1; /* leave at default */
+
+		return 0;
+	}
+
+	buf = nvmem_cell_read(cell, NULL);
+	nvmem_cell_put(cell);
+
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
+
+	val = *buf;
+	kfree(buf);
+
+	if ((val & DP83867_IO_MUX_CFG_IO_IMPEDANCE_MASK) != val) {
+		phydev_err(phydev, "nvmem cell 'io_impedance_ctrl' contents out of range\n");
+		return -ERANGE;
+	}
+	dp83867->io_impedance = val;
+
+	return 0;
+}
+
 static int dp83867_of_init(struct phy_device *phydev)
 {
 	struct dp83867_private *dp83867 = phydev->priv;
@@ -548,12 +594,9 @@ static int dp83867_of_init(struct phy_device *phydev)
 		}
 	}
 
-	if (of_property_read_bool(of_node, "ti,max-output-impedance"))
-		dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX;
-	else if (of_property_read_bool(of_node, "ti,min-output-impedance"))
-		dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN;
-	else
-		dp83867->io_impedance = -1; /* leave at default */
+	ret = dp83867_of_init_io_impedance(phydev);
+	if (ret)
+		return ret;
 
 	dp83867->rxctrl_strap_quirk = of_property_read_bool(of_node,
 							    "ti,dp83867-rxctrl-strap-quirk");
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next 1/3] dt-bindings: dp83867: add binding for io_impedance_ctrl nvmem cell
  2022-06-06 20:22 ` [PATCH net-next 1/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
  2022-06-06 21:58   ` Andrew Lunn
  2022-06-09 20:04   ` Rob Herring
@ 2022-06-14 18:46   ` Andrew Lunn
  2 siblings, 0 replies; 14+ messages in thread
From: Andrew Lunn @ 2022-06-14 18:46 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: netdev, Heiner Kallweit, Russell King, devicetree, Rob Herring,
	Jakub Kicinski, David S. Miller, Dan Murphy, linux-kernel

On Mon, Jun 06, 2022 at 10:22:18PM +0200, Rasmus Villemoes wrote:
> We have a board where measurements indicate that the current three
> options - leaving IO_IMPEDANCE_CTRL at the (factory calibrated) reset
> value or using one of the two boolean properties to set it to the
> min/max value - are too coarse.
> 
> There is no documented mapping from the 32 possible values of the
> IO_IMPEDANCE_CTRL field to values in the range 35-70 ohms, and the
> exact mapping is likely to vary from chip to chip. So add a DT binding
> for an nvmem cell which can be populated during production with a
> value suitable for each specific board.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 0/3] dt-bindings: dp83867: add binding for io_impedance_ctrl nvmem cell
  2022-06-14  8:46 ` [PATCH net-next v2 0/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
                     ` (2 preceding siblings ...)
  2022-06-14  8:46   ` [PATCH net-next v2 3/3] net: phy: dp83867: implement support for io_impedance_ctrl nvmem cell Rasmus Villemoes
@ 2022-06-17  3:50   ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 14+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-06-17  3:50 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: netdev, andrew, hkallweit1, linux, devicetree, robh+dt, kuba,
	davem, grygorii.strashko, praneeth, linux-kernel

Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 14 Jun 2022 10:46:09 +0200 you wrote:
> We have a board where measurements indicate that the current three
> options - leaving IO_IMPEDANCE_CTRL at the reset value (which is
> factory calibrated to a value corresponding to approximately 50 ohms)
> or using one of the two boolean properties to set it to the min/max
> value - are too coarse.
> 
> This series adds a device tree binding for an nvmem cell which can be
> populated during production with a suitable value calibrated for each
> board, and corresponding support in the driver. The second patch adds
> a trivial phy wrapper for dev_err_probe(), used in the third.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/3] dt-bindings: dp83867: add binding for io_impedance_ctrl nvmem cell
    https://git.kernel.org/netdev/net-next/c/ab1e9de84aff
  - [net-next,v2,2/3] linux/phy.h: add phydev_err_probe() wrapper for dev_err_probe()
    https://git.kernel.org/netdev/net-next/c/a793679827a8
  - [net-next,v2,3/3] net: phy: dp83867: implement support for io_impedance_ctrl nvmem cell
    https://git.kernel.org/netdev/net-next/c/5c2d0a6a0701

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-06-17  3:50 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-06 20:22 [PATCH net-next 0/3] net: phy: dp83867: add binding and support for io_impedance_ctrl nvmem cell Rasmus Villemoes
2022-06-06 20:22 ` [PATCH net-next 1/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
2022-06-06 21:58   ` Andrew Lunn
2022-06-07 11:54     ` Rasmus Villemoes
2022-06-14  5:40       ` Jakub Kicinski
2022-06-09 20:04   ` Rob Herring
2022-06-14 18:46   ` Andrew Lunn
2022-06-06 20:22 ` [PATCH net-next 2/3] linux/phy.h: add phydev_err_probe() wrapper for dev_err_probe() Rasmus Villemoes
2022-06-06 20:22 ` [PATCH net-next 3/3] net: phy: dp83867: implement support for io_impedance_ctrl nvmem cell Rasmus Villemoes
2022-06-14  8:46 ` [PATCH net-next v2 0/3] dt-bindings: dp83867: add binding " Rasmus Villemoes
2022-06-14  8:46   ` [PATCH net-next v2 1/3] " Rasmus Villemoes
2022-06-14  8:46   ` [PATCH net-next v2 2/3] linux/phy.h: add phydev_err_probe() wrapper for dev_err_probe() Rasmus Villemoes
2022-06-14  8:46   ` [PATCH net-next v2 3/3] net: phy: dp83867: implement support for io_impedance_ctrl nvmem cell Rasmus Villemoes
2022-06-17  3:50   ` [PATCH net-next v2 0/3] dt-bindings: dp83867: add binding " patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).