[PATCH v1 2/7] phy: starfive: Add Starfive JH7110 USB 2.0 PHY driver

[Marek Vasut]

On 5/4/24 5:03 PM, Minda Chen wrote:

[...]

> diff --git a/drivers/phy/starfive/Makefile b/drivers/phy/starfive/Makefile
> new file mode 100644
> index 0000000000..a405a75e34
> --- /dev/null
> +++ b/drivers/phy/starfive/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright (C) 2023 Starfive

2024 instead of 2023, please fix globally.

> +#
> +
> +obj-$(CONFIG_PHY_STARFIVE_JH7110_USB2)	+= phy-jh7110-usb2.o
> diff --git a/drivers/phy/starfive/phy-jh7110-usb2.c b/drivers/phy/starfive/phy-jh7110-usb2.c
> new file mode 100644
> index 0000000000..ffbd96d721
> --- /dev/null
> +++ b/drivers/phy/starfive/phy-jh7110-usb2.c

[...]

> +static void usb2_set_ls_keepalive(struct jh7110_usb2_phy *phy, bool set)
> +{
> +	unsigned int val;
> +
> +	/* Host mode enable the LS speed keep-alive signal */
> +	val = readl(phy->regs + USB_LS_KEEPALIVE_OFF);
> +	if (set)
> +		val |= USB_LS_KEEPALIVE_ENABLE;
> +	else
> +		val &= ~USB_LS_KEEPALIVE_ENABLE;
> +
> +	writel(val, phy->regs + USB_LS_KEEPALIVE_OFF);

This is clrsetbits_le32(), use it.

> +}
> +
> +static int usb2_phy_set_mode(struct phy *_phy,
> +			     enum phy_mode mode, int submode)
> +{
> +	struct udevice *dev = _phy->dev;
> +	struct jh7110_usb2_phy *phy = dev_get_priv(dev);
> +
> +	switch (mode) {
> +	case PHY_MODE_USB_HOST:
> +	case PHY_MODE_USB_DEVICE:
> +	case PHY_MODE_USB_OTG:
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (mode != phy->mode) {

Reduce indent this way:

if (mode == phy->mode)
   return 0;

... do mode switch stuff ...
return 0;

> +		dev_dbg(dev, "Changing phy to %d\n", mode);
> +		phy->mode = mode;
> +		usb2_set_ls_keepalive(phy, (mode != PHY_MODE_USB_DEVICE));
> +	}
> +
> +	return 0;
> +}
> +
> +static int jh7110_usb2_phy_init(struct phy *_phy)
> +{
> +	struct udevice *dev = _phy->dev;
> +	struct jh7110_usb2_phy *phy = dev_get_priv(dev);
> +	int ret;
> +
> +	ret = clk_prepare_enable(phy->app_125m);

return clk_prepare_...(); is just fine

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int jh7110_usb2_phy_exit(struct phy *_phy)
> +{
> +	struct udevice *dev = _phy->dev;
> +	struct jh7110_usb2_phy *phy = dev_get_priv(dev);
> +
> +	clk_disable_unprepare(phy->app_125m);
> +
> +	return 0;
> +}
> +
> +struct phy_ops jh7110_usb2_phy_ops = {
> +	.init     = jh7110_usb2_phy_init,
> +	.exit     = jh7110_usb2_phy_exit,
> +	.set_mode = usb2_phy_set_mode,
> +};
> +
> +int jh7110_usb2_phy_probe(struct udevice *dev)
> +{
> +	struct jh7110_usb2_phy *phy = dev_get_priv(dev);
> +
> +	phy->regs = dev_read_addr_ptr(dev);
> +

Drop extra newline.

> +	if (!phy->regs)
> +		return -EINVAL;

[...]