[PATCH 1/2] phy: rockchip-inno-usb2: Write to correct GRF

[Kever Yang]

On 2024/2/26 06:10, Jonas Karlman wrote:
> On RK3399 the USB2PHY regs are located in the common GRF, remaining SoCs
> that is supported by this driver have the USB2PHY regs in a different
> GRF.
>
> When support for RK356x, RK3588 and RK3328 was added this driver was
> never updated to use correct GRF and have instead incorrectly written
> to wrong GRF for these SoCs.
>
> The default reset values for the USB2PHY have made USB mostly working
> even when wrong GRF was used, however, following have been observed:
>
>    scanning bus usb at fd840000 for devices...
>    ERROR:  USB-error: DEVICENOTRESPONDING: Device did not respond to token (IN) or did
>    not provide a handshake (OUT) (5)
>    ERROR: USB-error: DEVICENOTRESPONDING: Device did not respond to token (IN) or did
>    not provide a handshake (OUT) (5)
>    unable to get device descriptor (error=-1)
>
> Fix this by using a regmap from rockchip,usbgrf prop and fall back to
> getting a regmap for parent udevice instead of always getting the
> common GRF.
>
> Also protect against accidental clear of bit 0 in a reg with offset 0,
> only bind driver to enabled otg/host-ports and remove unused headers.
>
> Fixes: 3da15f0b49a2 ("phy: rockchip-inno-usb2: Add USB2 PHY for rk3568")
> Fixes: cdf9010f6e17 ("phy: rockchip-inno-usb2: add initial support for rk3588 PHY")
> Fixes: 9aa93d84038b ("phy: rockchip-inno-usb2: Add USB2 PHY for RK3328")
> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
Reviewed-by: Kever Yang <kever.yang at rock-chips.com>

Thanks,
- Kever
> ---
>   drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 41 ++++++++++---------
>   1 file changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> index 70e61eccb79a..7317128d135e 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> @@ -6,23 +6,16 @@
>    * Copyright (C) 2020 Amarula Solutions(India)
>    */
>   
> -#include <common.h>
>   #include <clk-uclass.h>
>   #include <dm.h>
> -#include <asm/global_data.h>
>   #include <dm/device_compat.h>
>   #include <dm/device-internal.h>
>   #include <dm/lists.h>
>   #include <generic-phy.h>
> -#include <reset.h>
> +#include <regmap.h>
>   #include <syscon.h>
> -#include <asm/gpio.h>
> -#include <asm/io.h>
> -#include <linux/iopoll.h>
>   #include <asm/arch-rockchip/clock.h>
>   
> -DECLARE_GLOBAL_DATA_PTR;
> -
>   #define usleep_range(a, b) udelay((b))
>   #define BIT_WRITEABLE_SHIFT	16
>   
> @@ -61,30 +54,39 @@ struct rockchip_usb2phy_cfg {
>   };
>   
>   struct rockchip_usb2phy {
> -	void *reg_base;
> +	struct regmap *reg_base;
>   	struct clk phyclk;
>   	const struct rockchip_usb2phy_cfg *phy_cfg;
>   };
>   
> -static inline int property_enable(void *reg_base,
> +static inline int property_enable(struct regmap *base,
>   				  const struct usb2phy_reg *reg, bool en)
>   {
>   	unsigned int val, mask, tmp;
>   
> +	if (!reg->offset && !reg->enable && !reg->disable)
> +		return 0;
> +
>   	tmp = en ? reg->enable : reg->disable;
>   	mask = GENMASK(reg->bitend, reg->bitstart);
>   	val = (tmp << reg->bitstart) | (mask << BIT_WRITEABLE_SHIFT);
>   
> -	return writel(val, reg_base + reg->offset);
> +	return regmap_write(base, reg->offset, val);
>   }
>   
> -static inline bool property_enabled(void *reg_base,
> +static inline bool property_enabled(struct regmap *base,
>   				    const struct usb2phy_reg *reg)
>   {
> +	int ret;
>   	unsigned int tmp, orig;
>   	unsigned int mask = GENMASK(reg->bitend, reg->bitstart);
>   
> -	orig = readl(reg_base + reg->offset);
> +	if (!reg->offset && !reg->enable && !reg->disable)
> +		return false;
> +
> +	ret = regmap_read(base, reg->offset, &orig);
> +	if (ret)
> +		return false;
>   
>   	tmp = (orig & mask) >> reg->bitstart;
>   	return tmp != reg->disable;
> @@ -248,7 +250,11 @@ static int rockchip_usb2phy_probe(struct udevice *dev)
>   	unsigned int reg;
>   	int index, ret;
>   
> -	priv->reg_base = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
> +	if (dev_read_bool(dev, "rockchip,usbgrf"))
> +		priv->reg_base =
> +			syscon_regmap_lookup_by_phandle(dev, "rockchip,usbgrf");
> +	else
> +		priv->reg_base = syscon_get_regmap(dev_get_parent(dev));
>   	if (IS_ERR(priv->reg_base))
>   		return PTR_ERR(priv->reg_base);
>   
> @@ -305,11 +311,8 @@ static int rockchip_usb2phy_bind(struct udevice *dev)
>   	int ret = 0;
>   
>   	dev_for_each_subnode(node, dev) {
> -		if (!ofnode_valid(node)) {
> -			dev_info(dev, "subnode %s not found\n", dev->name);
> -			ret = -ENXIO;
> -			goto bind_fail;
> -		}
> +		if (!ofnode_is_enabled(node))
> +			continue;
>   
>   		name = ofnode_get_name(node);
>   		dev_dbg(dev, "subnode %s\n", name);