[PATCH] net: phy: marvell: Add soft reset for 88E1510

[PATCH] net: phy: marvell: Add soft reset for 88E1510

[Ben Brown]

When bringing down then up the link on a 88e1512 phy a link is not
getting established. This is because the phy is coming out of reset then
immediately getting configured. During configuration the page is
unsuccessfully updated causing writes to the wrong registers.

Add the soft reset function that does a reset then polling read waiting
for the phy to come back online, at which stage the page register can be
updated successfully.

This was tested on a 88E1512 phy, using ip link to bring up/down the
link.

Signed-off-by: Ben Brown <ben.brown@alliedtelesis.co.nz>
---
 drivers/net/phy/marvell.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 7a578b5aa2ed..1ae75141408a 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -3922,6 +3922,7 @@ static struct phy_driver marvell_drivers[] = {
 		.features = PHY_GBIT_FIBRE_FEATURES,
 		.flags = PHY_POLL_CABLE_TEST,
 		.probe = marvell_probe,
+		.soft_reset = genphy_soft_reset,
 		.config_init = m88e1510_config_init,
 		.config_aneg = m88e1510_config_aneg,
 		.read_status = marvell_read_status,
-- 
2.54.0

Re: [PATCH] net: phy: marvell: Add soft reset for 88E1510

[Andrew Lunn]

On Fri, Jul 03, 2026 at 10:50:34AM +1200, Ben Brown wrote:
> When bringing down then up the link on a 88e1512 phy a link is not
> getting established. This is because the phy is coming out of reset then
> immediately getting configured. During configuration the page is
> unsuccessfully updated causing writes to the wrong registers.
> 
> Add the soft reset function that does a reset then polling read waiting
> for the phy to come back online, at which stage the page register can be
> updated successfully.
> 
> This was tested on a 88E1512 phy, using ip link to bring up/down the
> link.

What makes the 88E1512 special that it needs this, but no other
Marvell PHY does?

	Andrew

Re: [PATCH] net: phy: marvell: Add soft reset for 88E1510

[Ben Brown]

On 7/3/26 11:04, Andrew Lunn wrote:
> On Fri, Jul 03, 2026 at 10:50:34AM +1200, Ben Brown wrote:
>> When bringing down then up the link on a 88e1512 phy a link is not
>> getting established. This is because the phy is coming out of reset then
>> immediately getting configured. During configuration the page is
>> unsuccessfully updated causing writes to the wrong registers.
>>
>> Add the soft reset function that does a reset then polling read waiting
>> for the phy to come back online, at which stage the page register can be
>> updated successfully.
>>
>> This was tested on a 88E1512 phy, using ip link to bring up/down the
>> link.
> 
> What makes the 88E1512 special that it needs this, but no other
> Marvell PHY does?
> 
> 	Andrew

This may be needed on the other marvell phys, but I only have access to 
a 88E1512 phy so I am only updating what I have seen this on.

Re: [PATCH] net: phy: marvell: Add soft reset for 88E1510

[Andrew Lunn]

On Fri, Jul 03, 2026 at 12:14:26AM +0000, Ben Brown wrote:
> 
> 
> On 7/3/26 11:04, Andrew Lunn wrote:
> > On Fri, Jul 03, 2026 at 10:50:34AM +1200, Ben Brown wrote:
> >> When bringing down then up the link on a 88e1512 phy a link is not
> >> getting established. This is because the phy is coming out of reset then
> >> immediately getting configured. During configuration the page is
> >> unsuccessfully updated causing writes to the wrong registers.
> >>
> >> Add the soft reset function that does a reset then polling read waiting
> >> for the phy to come back online, at which stage the page register can be
> >> updated successfully.
> >>
> >> This was tested on a 88E1512 phy, using ip link to bring up/down the
> >> link.
> > 
> > What makes the 88E1512 special that it needs this, but no other
> > Marvell PHY does?
> > 
> > 	Andrew
> 
> This may be needed on the other marvell phys, but I only have access to 
> a 88E1512 phy so I am only updating what I have seen this on.

The Marvell driver is used a lot, so i would of expected somebody else
to of noticed.

Lets take a step back.

What sort of reset are we talking about? Software or hardware?

	Andrew

Re: [PATCH] net: phy: marvell: Add soft reset for 88E1510

[Ben Brown]

On 7/4/26 00:53, Andrew Lunn wrote:
> On Fri, Jul 03, 2026 at 12:14:26AM +0000, Ben Brown wrote:
>>
>>
>> On 7/3/26 11:04, Andrew Lunn wrote:
>>> On Fri, Jul 03, 2026 at 10:50:34AM +1200, Ben Brown wrote:
>>>> When bringing down then up the link on a 88e1512 phy a link is not
>>>> getting established. This is because the phy is coming out of reset then
>>>> immediately getting configured. During configuration the page is
>>>> unsuccessfully updated causing writes to the wrong registers.
>>>>
>>>> Add the soft reset function that does a reset then polling read waiting
>>>> for the phy to come back online, at which stage the page register can be
>>>> updated successfully.
>>>>
>>>> This was tested on a 88E1512 phy, using ip link to bring up/down the
>>>> link.
>>>
>>> What makes the 88E1512 special that it needs this, but no other
>>> Marvell PHY does?
>>>
>>> 	Andrew
>>
>> This may be needed on the other marvell phys, but I only have access to
>> a 88E1512 phy so I am only updating what I have seen this on.
> 
> The Marvell driver is used a lot, so i would of expected somebody else
> to of noticed.
> 
> Lets take a step back.
> 
> What sort of reset are we talking about? Software or hardware?
> 
> 	Andrew

It is doing a hardware reset using a GPIO line.

When linked down the phy gets put into reset using a hardware GPIO line,
during link up the phy initializes the hardware using phy_init_hw(),
which de-asserts that reset GPIO. Then trivial setup is done before
the driver specific config_init().

When we are doing the marvell m88e1510_config_init() the first page
write is not applying so it ends up writing configuration to the wrong 
registers. When testing fixes adding a 15us sleep before changing the 
page also meant the page was updated correctly.

Re: [PATCH] net: phy: marvell: Add soft reset for 88E1510

[Andrew Lunn]

> > Lets take a step back.
> > 
> > What sort of reset are we talking about? Software or hardware?
> > 
> > 	Andrew
> 
> It is doing a hardware reset using a GPIO line.
> 
> When linked down the phy gets put into reset using a hardware GPIO line,
> during link up the phy initializes the hardware using phy_init_hw(),
> which de-asserts that reset GPIO. Then trivial setup is done before
> the driver specific config_init().
> 
> When we are doing the marvell m88e1510_config_init() the first page
> write is not applying so it ends up writing configuration to the wrong 
> registers. When testing fixes adding a 15us sleep before changing the 
> page also meant the page was updated correctly.

  reset-gpios:
    maxItems: 1
    description:
      The GPIO phandle and specifier for the PHY reset signal.

  reset-assert-us:
    description:
      Delay after the reset was asserted in microseconds. If this
      property is missing the delay will be skipped.

  reset-deassert-us:
    description:
      Delay after the reset was deasserted in microseconds. If
      this property is missing the delay will be skipped.


What delays are you using?

     Andrew

Re: [PATCH] net: phy: marvell: Add soft reset for 88E1510

[Ben Brown]

On 7/9/26 01:04, Andrew Lunn wrote:
>>> Lets take a step back.
>>>
>>> What sort of reset are we talking about? Software or hardware?
>>>
>>> 	Andrew
>>
>> It is doing a hardware reset using a GPIO line.
>>
>> When linked down the phy gets put into reset using a hardware GPIO line,
>> during link up the phy initializes the hardware using phy_init_hw(),
>> which de-asserts that reset GPIO. Then trivial setup is done before
>> the driver specific config_init().
>>
>> When we are doing the marvell m88e1510_config_init() the first page
>> write is not applying so it ends up writing configuration to the wrong
>> registers. When testing fixes adding a 15us sleep before changing the
>> page also meant the page was updated correctly.
> 
>    reset-gpios:
>      maxItems: 1
>      description:
>        The GPIO phandle and specifier for the PHY reset signal.
> 
>    reset-assert-us:
>      description:
>        Delay after the reset was asserted in microseconds. If this
>        property is missing the delay will be skipped.
> 
>    reset-deassert-us:
>      description:
>        Delay after the reset was deasserted in microseconds. If
>        this property is missing the delay will be skipped.
> 
> 
> What delays are you using?
> 
>       Andrew

We were not using any of those delays as I didn't see those options.
Adding a delay fixes our issue, so this patch is not needed.

Thank you for your time,
Ben