[PATCH net-next 0/3] Add support for TI DP83826 configuration

[PATCH net-next 0/3] Add support for TI DP83826 configuration

[Jérémie Dautheribes]

Hi everyone,

This short patch series introduces the possibility of overriding
some parameters which are latched by default by hardware straps on the
TI DP83826 PHY.

The settings that can be overridden include:
  - Configuring the PHY in either MII mode or RMII mode.
  - When in RMII mode, configuring the PHY in RMII slave mode or RMII
  master mode.

The RMII master/slave mode is TI-specific and determines whether the PHY
operates from a 25MHz reference clock (master mode) or from a 50MHz
reference clock (slave mode).

While these features should be supported by all the TI DP8382x family,
I have only been able to test them on TI DP83826 hardware.
Therefore, support has been added specifically for this PHY in this patch
series.

Jérémie Dautheribes (3):
  dt-bindings: net: dp83822: support configuring RMII master/slave mode
  net: phy: dp83826: Add support for phy-mode configuration
  net: phy: dp83826: support configuring RMII master/slave operation
    mode

 .../devicetree/bindings/net/ti,dp83822.yaml   | 16 +++++++
 drivers/net/phy/dp83822.c                     | 44 +++++++++++++++++++
 2 files changed, 60 insertions(+)

-- 
2.34.1

[PATCH net-next 1/3] dt-bindings: net: dp83822: support configuring RMII master/slave mode

[Jérémie Dautheribes]

Add property ti,rmii-mode to support selecting the RMII operation mode
between:
	- master mode (PHY operates from a 25MHz clock reference)
	- slave mode (PHY operates from a 50MHz clock reference)

If not set, the operation mode is configured by hardware straps.

Signed-off-by: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
---
 .../devicetree/bindings/net/ti,dp83822.yaml      | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ti,dp83822.yaml b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
index 8f4350be689c..8f23254c0458 100644
--- a/Documentation/devicetree/bindings/net/ti,dp83822.yaml
+++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
@@ -80,6 +80,22 @@ properties:
            10625, 11250, 11875, 12500, 13125, 13750, 14375, 15000]
     default: 10000
 
+  ti,rmii-mode:
+    description: |
+       If present, select the RMII operation mode. Two modes are
+       available:
+         - RMII master, where the PHY operates from a 25MHz clock reference,
+         provided by a crystal or a CMOS-level oscillator
+         - RMII slave, where the PHY operates from a 50MHz clock reference,
+         provided by a CMOS-level oscillator
+       The RMII operation mode can also be configured by its straps.
+       If the strap pin is not set correctly or not set at all, then this can be
+       used to configure it.
+    $ref: /schemas/types.yaml#/definitions/string
+    enum:
+      - master
+      - slave
+
 required:
   - reg
 
-- 
2.34.1

Re: [PATCH net-next 1/3] dt-bindings: net: dp83822: support configuring RMII master/slave mode

[Krzysztof Kozlowski]

On 22/02/2024 11:31, Jérémie Dautheribes wrote:
> Add property ti,rmii-mode to support selecting the RMII operation mode
> between:
> 	- master mode (PHY operates from a 25MHz clock reference)
> 	- slave mode (PHY operates from a 50MHz clock reference)
> 
> If not set, the operation mode is configured by hardware straps.
> 
> Signed-off-by: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH net-next 1/3] dt-bindings: net: dp83822: support configuring RMII master/slave mode

[Andrew Lunn]

On Thu, Feb 22, 2024 at 11:31:15AM +0100, Jérémie Dautheribes wrote:
> Add property ti,rmii-mode to support selecting the RMII operation mode
> between:
> 	- master mode (PHY operates from a 25MHz clock reference)
> 	- slave mode (PHY operates from a 50MHz clock reference)
> 
> If not set, the operation mode is configured by hardware straps.
> 
> Signed-off-by: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
> ---
>  .../devicetree/bindings/net/ti,dp83822.yaml      | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ti,dp83822.yaml b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> index 8f4350be689c..8f23254c0458 100644
> --- a/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> +++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> @@ -80,6 +80,22 @@ properties:
>             10625, 11250, 11875, 12500, 13125, 13750, 14375, 15000]
>      default: 10000
>  
> +  ti,rmii-mode:
> +    description: |
> +       If present, select the RMII operation mode. Two modes are
> +       available:
> +         - RMII master, where the PHY operates from a 25MHz clock reference,
> +         provided by a crystal or a CMOS-level oscillator
> +         - RMII slave, where the PHY operates from a 50MHz clock reference,
> +         provided by a CMOS-level oscillator

What has master and slave got to do with this?

Sometimes, the MAC provides a clock to the PHY, and all data transfer
over the RMII bus is timed by that.

Sometimes, the PHY provides a clock to the MAC, and all data transfer
over the RMII bus is timed by that.

Here there is a clear master/slave relationship, who is providing the
clock, who is consuming the clock. However, what you describe does not
fit that. Maybe look at other PHY bindings, and copy what they do for
clocks.

	Andrew

Re: [PATCH net-next 1/3] dt-bindings: net: dp83822: support configuring RMII master/slave mode

[Jérémie Dautheribes]

Hi Andrew,

On 26/02/2024 16:28, Andrew Lunn wrote:
> On Thu, Feb 22, 2024 at 11:31:15AM +0100, Jérémie Dautheribes wrote:
>> Add property ti,rmii-mode to support selecting the RMII operation mode
>> between:
>> 	- master mode (PHY operates from a 25MHz clock reference)
>> 	- slave mode (PHY operates from a 50MHz clock reference)
>>
>> If not set, the operation mode is configured by hardware straps.
>>
>> Signed-off-by: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
>> ---
>>   .../devicetree/bindings/net/ti,dp83822.yaml      | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/ti,dp83822.yaml b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
>> index 8f4350be689c..8f23254c0458 100644
>> --- a/Documentation/devicetree/bindings/net/ti,dp83822.yaml
>> +++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
>> @@ -80,6 +80,22 @@ properties:
>>              10625, 11250, 11875, 12500, 13125, 13750, 14375, 15000]
>>       default: 10000
>>   
>> +  ti,rmii-mode:
>> +    description: |
>> +       If present, select the RMII operation mode. Two modes are
>> +       available:
>> +         - RMII master, where the PHY operates from a 25MHz clock reference,
>> +         provided by a crystal or a CMOS-level oscillator
>> +         - RMII slave, where the PHY operates from a 50MHz clock reference,
>> +         provided by a CMOS-level oscillator
> 
> What has master and slave got to do with this?
> 
> Sometimes, the MAC provides a clock to the PHY, and all data transfer
> over the RMII bus is timed by that.
> 
> Sometimes, the PHY provides a clock to the MAC, and all data transfer
> over the RMII bus is timed by that.
> 
> Here there is a clear master/slave relationship, who is providing the
> clock, who is consuming the clock. However, what you describe does not
> fit that. Maybe look at other PHY bindings, and copy what they do for
> clocks.

In fact, I hesitated a lot before choosing this master/slave designation 
because of the same reasoning as you. But the TI DP83826 datasheet [1] 
uses this name for two orthogonal yet connected meanings, here's a copy 
of the corresponding § (in section 9.3.10):

"The DP83826 offers two types of RMII operations: RMII Slave and RMII 
Master. In RMII Master operation, the DP83826 operates from either a 
25-MHz CMOS-level oscillator connected to XI pin, a 25-MHz crystal 
connected across XI and XO pins. A 50-MHz output clock referenced from 
DP83826 can be connected to the MAC. In RMII Slave operation, the 
DP83826 operates from a 50-MHz CMOS-level oscillator connected to the XI 
pin and shares the same clock as the MAC. Alternatively, in RMII slave 
mode, the PHY can operate from a 50-MHz clock provided by the Host MAC."

So it seems that in some cases this also fits the master/slave 
relationship you describe.

That said, would you like me to include this description (or some parts) 
in the binding in addition to what I've already written? Or would you 
prefer me to use a more meaningful property name?

BTW, this series has already been merged into the net-next tree, I'm not 
sure what procedure to follow in such cases.


Best regards,

Jérémie

[1] 
https://www.ti.com/lit/ds/symlink/dp83826i.pdf?ts=1708075771406&ref_url=https%253A%252F%252Fwww.ti.com%252Fproduct%252FDP83826I

Re: [PATCH net-next 1/3] dt-bindings: net: dp83822: support configuring RMII master/slave mode

[Andrew Lunn]

> > > --- a/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> > > +++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> > > @@ -80,6 +80,22 @@ properties:
> > >              10625, 11250, 11875, 12500, 13125, 13750, 14375, 15000]
> > >       default: 10000
> > > +  ti,rmii-mode:
> > > +    description: |
> > > +       If present, select the RMII operation mode. Two modes are
> > > +       available:
> > > +         - RMII master, where the PHY operates from a 25MHz clock reference,
> > > +         provided by a crystal or a CMOS-level oscillator
> > > +         - RMII slave, where the PHY operates from a 50MHz clock reference,
> > > +         provided by a CMOS-level oscillator
> > 
> > What has master and slave got to do with this?
> > 
> > Sometimes, the MAC provides a clock to the PHY, and all data transfer
> > over the RMII bus is timed by that.
> > 
> > Sometimes, the PHY provides a clock to the MAC, and all data transfer
> > over the RMII bus is timed by that.
> > 
> > Here there is a clear master/slave relationship, who is providing the
> > clock, who is consuming the clock. However, what you describe does not
> > fit that. Maybe look at other PHY bindings, and copy what they do for
> > clocks.
> 
> In fact, I hesitated a lot before choosing this master/slave designation
> because of the same reasoning as you. But the TI DP83826 datasheet [1] uses
> this name for two orthogonal yet connected meanings, here's a copy of the
> corresponding § (in section 9.3.10):
> 
> "The DP83826 offers two types of RMII operations: RMII Slave and RMII
> Master. In RMII Master operation, the DP83826 operates from either a 25-MHz
> CMOS-level oscillator connected to XI pin, a 25-MHz crystal connected across
> XI and XO pins. A 50-MHz output clock referenced from DP83826 can be
> connected to the MAC. In RMII Slave operation, the DP83826 operates from a
> 50-MHz CMOS-level oscillator connected to the XI pin and shares the same
> clock as the MAC. Alternatively, in RMII slave mode, the PHY can operate
> from a 50-MHz clock provided by the Host MAC."
> 
> So it seems that in some cases this also fits the master/slave relationship
> you describe.

We are normally interested in this 50Mhz reference clock. So i would
drop all references to 25Mhz. It is not relevant to the binding, since
it is nothing to do with connecting the PHY to the MAC, and it has a
fixed value.

So you can simplify this down to:

RMII Master: Outputs a 50Mhz Reference clock which can be connected to the MAC.

RMII Slave: Expects a 50MHz Reference clock input, shared with the
MAC.

> That said, would you like me to include this description (or some parts) in
> the binding in addition to what I've already written? Or would you prefer me
> to use a more meaningful property name?

We don't really have any vendor agnostic consistent naming. dp83867
and dp83869 seems to call this ti,clk-output-sel. Since this is
another dp83xxx device, it would be nice if there was consistency
between all these TI devices. So could you check if the concept is the
same, and if so, change dp83826 to follow what other TI devices do.

> BTW, this series has already been merged into the net-next tree, I'm not
> sure what procedure to follow in such cases.

KAPI don't become fixed until published as a release kernel. We can
rework bindings until then. So just submit patches on top of what is
already in net-next.

	Andrew

Re: [PATCH net-next 1/3] dt-bindings: net: dp83822: support configuring RMII master/slave mode

[Jérémie Dautheribes]

Hi Andrew,

On 29/02/2024 22:23, Andrew Lunn wrote:
>>>> --- a/Documentation/devicetree/bindings/net/ti,dp83822.yaml
>>>> +++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
>>>> @@ -80,6 +80,22 @@ properties:
>>>>               10625, 11250, 11875, 12500, 13125, 13750, 14375, 15000]
>>>>        default: 10000
>>>> +  ti,rmii-mode:
>>>> +    description: |
>>>> +       If present, select the RMII operation mode. Two modes are
>>>> +       available:
>>>> +         - RMII master, where the PHY operates from a 25MHz clock reference,
>>>> +         provided by a crystal or a CMOS-level oscillator
>>>> +         - RMII slave, where the PHY operates from a 50MHz clock reference,
>>>> +         provided by a CMOS-level oscillator
>>>
>>> What has master and slave got to do with this?
>>>
>>> Sometimes, the MAC provides a clock to the PHY, and all data transfer
>>> over the RMII bus is timed by that.
>>>
>>> Sometimes, the PHY provides a clock to the MAC, and all data transfer
>>> over the RMII bus is timed by that.
>>>
>>> Here there is a clear master/slave relationship, who is providing the
>>> clock, who is consuming the clock. However, what you describe does not
>>> fit that. Maybe look at other PHY bindings, and copy what they do for
>>> clocks.
>>
>> In fact, I hesitated a lot before choosing this master/slave designation
>> because of the same reasoning as you. But the TI DP83826 datasheet [1] uses
>> this name for two orthogonal yet connected meanings, here's a copy of the
>> corresponding § (in section 9.3.10):
>>
>> "The DP83826 offers two types of RMII operations: RMII Slave and RMII
>> Master. In RMII Master operation, the DP83826 operates from either a 25-MHz
>> CMOS-level oscillator connected to XI pin, a 25-MHz crystal connected across
>> XI and XO pins. A 50-MHz output clock referenced from DP83826 can be
>> connected to the MAC. In RMII Slave operation, the DP83826 operates from a
>> 50-MHz CMOS-level oscillator connected to the XI pin and shares the same
>> clock as the MAC. Alternatively, in RMII slave mode, the PHY can operate
>> from a 50-MHz clock provided by the Host MAC."
>>
>> So it seems that in some cases this also fits the master/slave relationship
>> you describe.
> 
> We are normally interested in this 50Mhz reference clock. So i would
> drop all references to 25Mhz. It is not relevant to the binding, since
> it is nothing to do with connecting the PHY to the MAC, and it has a
> fixed value.
> 
> So you can simplify this down to:
> 
> RMII Master: Outputs a 50Mhz Reference clock which can be connected to the MAC.
> 
> RMII Slave: Expects a 50MHz Reference clock input, shared with the
> MAC.
> 
>> That said, would you like me to include this description (or some parts) in
>> the binding in addition to what I've already written? Or would you prefer me
>> to use a more meaningful property name?
> 
> We don't really have any vendor agnostic consistent naming. dp83867
> and dp83869 seems to call this ti,clk-output-sel. Since this is
> another dp83xxx device, it would be nice if there was consistency
> between all these TI devices. So could you check if the concept is the
> same, and if so, change dp83826 to follow what other TI devices do.


So I had a look at this ti,clk-output-sel property on the TI DP8386x 
bindings, but unfortunately it does not correspond to our use case. In 
their case, it is used to select one of the various internal clocks to 
output on the CLK_OUT pin.
In our case, we would prefer to describe the direction of the clock (OUT 
in master mode, IN in slave mode).

Given this, should we stick to "ti,rmii-mode" which is consistent with 
the datasheet terminology, or consider perhaps something like 
ti,clock-dir-sel (with possible values of in/out)?"

Best regards,
Jérémie

Re: [PATCH net-next 1/3] dt-bindings: net: dp83822: support configuring RMII master/slave mode

[Andrew Lunn]

> > We are normally interested in this 50Mhz reference clock. So i would
> > drop all references to 25Mhz. It is not relevant to the binding, since
> > it is nothing to do with connecting the PHY to the MAC, and it has a
> > fixed value.
> > 
> > So you can simplify this down to:
> > 
> > RMII Master: Outputs a 50Mhz Reference clock which can be connected to the MAC.
> > 
> > RMII Slave: Expects a 50MHz Reference clock input, shared with the
> > MAC.
> > 
> > > That said, would you like me to include this description (or some parts) in
> > > the binding in addition to what I've already written? Or would you prefer me
> > > to use a more meaningful property name?
> > 
> > We don't really have any vendor agnostic consistent naming. dp83867
> > and dp83869 seems to call this ti,clk-output-sel. Since this is
> > another dp83xxx device, it would be nice if there was consistency
> > between all these TI devices. So could you check if the concept is the
> > same, and if so, change dp83826 to follow what other TI devices do.
> 
> 
> So I had a look at this ti,clk-output-sel property on the TI DP8386x
> bindings, but unfortunately it does not correspond to our use case. In their
> case, it is used to select one of the various internal clocks to output on
> the CLK_OUT pin.
> In our case, we would prefer to describe the direction of the clock (OUT in
> master mode, IN in slave mode).

I would suggest we keep with the current property name, but simplify
the description. Focus on the reference clock, and ignore the crystal.

    Andrew

Re: [PATCH net-next 1/3] dt-bindings: net: dp83822: support configuring RMII master/slave mode

[Jérémie Dautheribes]

>>> We are normally interested in this 50Mhz reference clock. So i would
>>> drop all references to 25Mhz. It is not relevant to the binding, since
>>> it is nothing to do with connecting the PHY to the MAC, and it has a
>>> fixed value.
>>>
>>> So you can simplify this down to:
>>>
>>> RMII Master: Outputs a 50Mhz Reference clock which can be connected to the MAC.
>>>
>>> RMII Slave: Expects a 50MHz Reference clock input, shared with the
>>> MAC.
>>>
>>>> That said, would you like me to include this description (or some parts) in
>>>> the binding in addition to what I've already written? Or would you prefer me
>>>> to use a more meaningful property name?
>>>
>>> We don't really have any vendor agnostic consistent naming. dp83867
>>> and dp83869 seems to call this ti,clk-output-sel. Since this is
>>> another dp83xxx device, it would be nice if there was consistency
>>> between all these TI devices. So could you check if the concept is the
>>> same, and if so, change dp83826 to follow what other TI devices do.
>>
>>
>> So I had a look at this ti,clk-output-sel property on the TI DP8386x
>> bindings, but unfortunately it does not correspond to our use case. In their
>> case, it is used to select one of the various internal clocks to output on
>> the CLK_OUT pin.
>> In our case, we would prefer to describe the direction of the clock (OUT in
>> master mode, IN in slave mode).
> 
> I would suggest we keep with the current property name, but simplify
> the description. Focus on the reference clock, and ignore the crystal.


Ok noted, thanks for your feedback! I will send a v2 containing a 
simplified description + implement your suggested changes on patch 2.

Jérémie

[PATCH net-next 2/3] net: phy: dp83826: Add support for phy-mode configuration

[Jérémie Dautheribes]

The TI DP83826 PHY can operate in either MII mode or RMII mode.
By default, it is configured by straps.
It can also be configured by writing to the bit 5 of register 0x17 - RMII
and Status Register (RCSR).

When phydev->interface is rmii, rmii mode must be enabled, otherwise
mii mode must be set.
This prevents misconfiguration of hw straps.

Signed-off-by: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
---
 drivers/net/phy/dp83822.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 30f2616ab1c2..2d8275e59dcc 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -100,6 +100,7 @@
 #define DP83822_WOL_CLR_INDICATION BIT(11)
 
 /* RCSR bits */
+#define DP83822_RMII_MODE_EN	BIT(5)
 #define DP83822_RGMII_MODE_EN	BIT(9)
 #define DP83822_RX_CLK_SHIFT	BIT(12)
 #define DP83822_TX_CLK_SHIFT	BIT(11)
@@ -500,6 +501,16 @@ static int dp83826_config_init(struct phy_device *phydev)
 	u16 val, mask;
 	int ret;
 
+	if (phydev->interface == PHY_INTERFACE_MODE_RMII)
+		ret = phy_set_bits_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RCSR,
+				       DP83822_RMII_MODE_EN);
+	else
+		ret = phy_clear_bits_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RCSR,
+					 DP83822_RMII_MODE_EN);
+
+	if (ret)
+		return ret;
+
 	if (dp83822->cfg_dac_minus != DP83826_CFG_DAC_MINUS_DEFAULT) {
 		val = FIELD_PREP(DP83826_VOD_CFG1_MINUS_MDI_MASK, dp83822->cfg_dac_minus) |
 		      FIELD_PREP(DP83826_VOD_CFG1_MINUS_MDIX_MASK,
-- 
2.34.1

Re: [PATCH net-next 2/3] net: phy: dp83826: Add support for phy-mode configuration

[Maxime Chevallier]

Hello Jérémie,

On Thu, 22 Feb 2024 11:31:16 +0100
Jérémie Dautheribes <jeremie.dautheribes@bootlin.com> wrote:

> The TI DP83826 PHY can operate in either MII mode or RMII mode.
> By default, it is configured by straps.
> It can also be configured by writing to the bit 5 of register 0x17 - RMII
> and Status Register (RCSR).
> 
> When phydev->interface is rmii, rmii mode must be enabled, otherwise
> mii mode must be set.
> This prevents misconfiguration of hw straps.
> 
> Signed-off-by: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>

This looks good to me,

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Re: [PATCH net-next 2/3] net: phy: dp83826: Add support for phy-mode configuration

[Andrew Lunn]

On Thu, Feb 22, 2024 at 11:31:16AM +0100, Jérémie Dautheribes wrote:
> The TI DP83826 PHY can operate in either MII mode or RMII mode.
> By default, it is configured by straps.
> It can also be configured by writing to the bit 5 of register 0x17 - RMII
> and Status Register (RCSR).
> 
> When phydev->interface is rmii, rmii mode must be enabled, otherwise
> mii mode must be set.
> This prevents misconfiguration of hw straps.
> 
> Signed-off-by: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
> ---
>  drivers/net/phy/dp83822.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> index 30f2616ab1c2..2d8275e59dcc 100644
> --- a/drivers/net/phy/dp83822.c
> +++ b/drivers/net/phy/dp83822.c
> @@ -100,6 +100,7 @@
>  #define DP83822_WOL_CLR_INDICATION BIT(11)
>  
>  /* RCSR bits */
> +#define DP83822_RMII_MODE_EN	BIT(5)
>  #define DP83822_RGMII_MODE_EN	BIT(9)
>  #define DP83822_RX_CLK_SHIFT	BIT(12)
>  #define DP83822_TX_CLK_SHIFT	BIT(11)
> @@ -500,6 +501,16 @@ static int dp83826_config_init(struct phy_device *phydev)
>  	u16 val, mask;
>  	int ret;
>  
> +	if (phydev->interface == PHY_INTERFACE_MODE_RMII)
> +		ret = phy_set_bits_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RCSR,
> +				       DP83822_RMII_MODE_EN);
> +	else
> +		ret = phy_clear_bits_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RCSR,
> +					 DP83822_RMII_MODE_EN);

I would probably add a test for MII and return -EINVAL if asked to do
something else altogether.

	  Andrew

[PATCH net-next 3/3] net: phy: dp83826: support configuring RMII master/slave operation mode

[Jérémie Dautheribes]

The TI DP83826 PHY can operate between two RMII modes:
	- master mode (PHY operates from a 25MHz clock reference)
        - slave mode (PHY operates from a 50MHz clock reference)

By default, the operation mode is configured by hardware straps.

Add support to configure the operation mode from within the driver.

Signed-off-by: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
---
 drivers/net/phy/dp83822.c | 43 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 2d8275e59dcc..edc39ae4c241 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -101,6 +101,7 @@
 
 /* RCSR bits */
 #define DP83822_RMII_MODE_EN	BIT(5)
+#define DP83822_RMII_MODE_SEL	BIT(7)
 #define DP83822_RGMII_MODE_EN	BIT(9)
 #define DP83822_RX_CLK_SHIFT	BIT(12)
 #define DP83822_TX_CLK_SHIFT	BIT(11)
@@ -495,21 +496,53 @@ static int dp83822_config_init(struct phy_device *phydev)
 	return dp8382x_disable_wol(phydev);
 }
 
+static int dp83826_config_rmii_mode(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	const char *of_val;
+	int ret;
+
+	if (!device_property_read_string(dev, "ti,rmii-mode", &of_val)) {
+		if (strcmp(of_val, "master") == 0) {
+			ret = phy_clear_bits_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RCSR,
+						 DP83822_RMII_MODE_SEL);
+		} else if (strcmp(of_val, "slave") == 0) {
+			ret = phy_set_bits_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RCSR,
+					       DP83822_RMII_MODE_SEL);
+		} else {
+			phydev_err(phydev, "Invalid value for ti,rmii-mode property (%s)\n",
+				   of_val);
+			ret = -EINVAL;
+		}
+
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int dp83826_config_init(struct phy_device *phydev)
 {
 	struct dp83822_private *dp83822 = phydev->priv;
 	u16 val, mask;
 	int ret;
 
-	if (phydev->interface == PHY_INTERFACE_MODE_RMII)
+	if (phydev->interface == PHY_INTERFACE_MODE_RMII) {
 		ret = phy_set_bits_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RCSR,
 				       DP83822_RMII_MODE_EN);
-	else
+		if (ret)
+			return ret;
+
+		ret = dp83826_config_rmii_mode(phydev);
+		if (ret)
+			return ret;
+	} else {
 		ret = phy_clear_bits_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RCSR,
 					 DP83822_RMII_MODE_EN);
-
-	if (ret)
-		return ret;
+		if (ret)
+			return ret;
+	}
 
 	if (dp83822->cfg_dac_minus != DP83826_CFG_DAC_MINUS_DEFAULT) {
 		val = FIELD_PREP(DP83826_VOD_CFG1_MINUS_MDI_MASK, dp83822->cfg_dac_minus) |
-- 
2.34.1

Re: [PATCH net-next 0/3] Add support for TI DP83826 configuration

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu, 22 Feb 2024 11:31:14 +0100 you wrote:
> Hi everyone,
> 
> This short patch series introduces the possibility of overriding
> some parameters which are latched by default by hardware straps on the
> TI DP83826 PHY.
> 
> The settings that can be overridden include:
>   - Configuring the PHY in either MII mode or RMII mode.
>   - When in RMII mode, configuring the PHY in RMII slave mode or RMII
>   master mode.
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] dt-bindings: net: dp83822: support configuring RMII master/slave mode
    https://git.kernel.org/netdev/net-next/c/95f4fa1f459a
  - [net-next,2/3] net: phy: dp83826: Add support for phy-mode configuration
    https://git.kernel.org/netdev/net-next/c/d2ed0774b633
  - [net-next,3/3] net: phy: dp83826: support configuring RMII master/slave operation mode
    https://git.kernel.org/netdev/net-next/c/2844a0d7cffe

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