[PATCH phy-fixes] phy: marvell: mvebu-a3700-utmi: fix incorrect USB2_PHY_CTRL register access

[PATCH phy-fixes] phy: marvell: mvebu-a3700-utmi: fix incorrect USB2_PHY_CTRL register access

[Gabor Juhos]

The mvebu_a3700_utmi_phy_power_off() function tries to modify the
USB2_PHY_CTRL register by using the IO address of the PHY IP block along
with the readl/writel IO accessors. However, the register exist in the
USB miscellaneous register space, and as such it must be accessed via
regmap like it is done in the mvebu_a3700_utmi_phy_power_on() function.

Change the code to use regmap_update_bits() for modífying the register
to fix this.

Fixes: cc8b7a0ae866 ("phy: add A3700 UTMI PHY driver")
Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
 drivers/phy/marvell/phy-mvebu-a3700-utmi.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
index 04f4fb4bed7024c933190bbd9353615d54ac172c..f882bc57649c77343c3cb9c5111db82812993c3e 100644
--- a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
+++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
@@ -168,9 +168,8 @@ static int mvebu_a3700_utmi_phy_power_off(struct phy *phy)
 	u32 reg;
 
 	/* Disable PHY pull-up and enable USB2 suspend */
-	reg = readl(utmi->regs + USB2_PHY_CTRL(usb32));
-	reg &= ~(RB_USB2PHY_PU | RB_USB2PHY_SUSPM(usb32));
-	writel(reg, utmi->regs + USB2_PHY_CTRL(usb32));
+	regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
+			   RB_USB2PHY_PU | RB_USB2PHY_SUSPM(usb32), 0);
 
 	/* Power down OTG module */
 	if (usb32) {

---
base-commit: 81af9e40e2e4e1aa95f09fb34811760be6742c58
change-id: 20260321-a3700-utmi-fix-usb2_phy_ctrl-access-759ad1d7c935

Best regards,
-- 
Gabor Juhos <j4g8y7@gmail.com>

Re: [PATCH phy-fixes] phy: marvell: mvebu-a3700-utmi: fix incorrect USB2_PHY_CTRL register access

[Miquel Raynal]

Hi Gabor,

On 21/03/2026 at 15:42:32 +01, Gabor Juhos <j4g8y7@gmail.com> wrote:

> The mvebu_a3700_utmi_phy_power_off() function tries to modify the
> USB2_PHY_CTRL register by using the IO address of the PHY IP block along
> with the readl/writel IO accessors. However, the register exist in the
> USB miscellaneous register space, and as such it must be accessed via
> regmap like it is done in the mvebu_a3700_utmi_phy_power_on() function.
>
> Change the code to use regmap_update_bits() for modífying the register

Spurious accent here :-)                             ^

Do you imply that the register access was not working? Or that it was
not using the correct API? (hence potential issues with locking might arise)


Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Miquèl

Re: [PATCH phy-fixes] phy: marvell: mvebu-a3700-utmi: fix incorrect USB2_PHY_CTRL register access

[Gabor Juhos]

Hi Miquel,

> Hi Gabor,
> 
> On 21/03/2026 at 15:42:32 +01, Gabor Juhos <j4g8y7@gmail.com> wrote:
> 
>> The mvebu_a3700_utmi_phy_power_off() function tries to modify the
>> USB2_PHY_CTRL register by using the IO address of the PHY IP block along
>> with the readl/writel IO accessors. However, the register exist in the
>> USB miscellaneous register space, and as such it must be accessed via
>> regmap like it is done in the mvebu_a3700_utmi_phy_power_on() function.
>>
>> Change the code to use regmap_update_bits() for modífying the register
> 
> Spurious accent here :-)                             ^
> 
> Do you imply that the register access was not working? Or that it was
> not using the correct API? ...

It was using the wrong API with wrong base address.

The USB2_PHY_CTRL(x) macro gives back an offset relative to the base address of
the USB miscellaneous registers. However the current code uses that offset with
the base address of the UTMI PHY registers.

So, instead of modifying the 'USB2 Host PHY Control' register at 0xd005f804 it
changes the 'USB2 UTMI PHY PLL Control 1' register at 0xd005f004.

Since the miscellaneous registers can be accessed only via the regmap obtained
from a syscon node we have to use the regmap API instead of the generic IO
accessors.

> ... (hence potential issues with locking might arise)

The regmap API uses built-in locking so there should be no locking issues.

Regards,
Gabor

Re: [PATCH phy-fixes] phy: marvell: mvebu-a3700-utmi: fix incorrect USB2_PHY_CTRL register access

[Miquel Raynal]

On 01/04/2026 at 09:13:57 +02, Gabor Juhos <j4g8y7@gmail.com> wrote:

> Hi Miquel,
>
>> Hi Gabor,
>> 
>> On 21/03/2026 at 15:42:32 +01, Gabor Juhos <j4g8y7@gmail.com> wrote:
>> 
>>> The mvebu_a3700_utmi_phy_power_off() function tries to modify the
>>> USB2_PHY_CTRL register by using the IO address of the PHY IP block along
>>> with the readl/writel IO accessors. However, the register exist in the
>>> USB miscellaneous register space, and as such it must be accessed via
>>> regmap like it is done in the mvebu_a3700_utmi_phy_power_on() function.
>>>
>>> Change the code to use regmap_update_bits() for modífying the register
>> 
>> Spurious accent here :-)                             ^
>> 
>> Do you imply that the register access was not working? Or that it was
>> not using the correct API? ...
>
> It was using the wrong API with wrong base address.
>
> The USB2_PHY_CTRL(x) macro gives back an offset relative to the base address of
> the USB miscellaneous registers. However the current code uses that offset with
> the base address of the UTMI PHY registers.
>
> So, instead of modifying the 'USB2 Host PHY Control' register at 0xd005f804 it
> changes the 'USB2 UTMI PHY PLL Control 1' register at 0xd005f004.

Ok, that's what I was asking.

> Since the miscellaneous registers can be accessed only via the regmap obtained
> from a syscon node we have to use the regmap API instead of the generic IO
> accessors.

Yeah yeah, I was asking to clarify the problem, whether it was a
register access issue or an API mismatch in the first place.

>> ... (hence potential issues with locking might arise)
>
> The regmap API uses built-in locking so there should be no locking
> issues.

Yes, that's what I was implying, I was talking about the bare readl()
call here (aka the "incorrect" API).

Thanks,
Miquèl