Re: [PATCH v4 3/9] phy: starfive: Add Starfive JH7110 PCIe 2.0 PHY driver

[Marek Vasut <marex@denx.de>]

On 8/29/24 3:30 AM, Minda Chen wrote:

[...]

> +static int phy_pcie_mode_set(struct jh7110_pcie_phy *data, bool usb_mode)
> +{
> +	unsigned int phy_mode, width, usb3_phy, ss_mode, split;
> +
> +	/* default is PCIe mode */
> +	if (!data->stg_syscon || !data->sys_syscon) {
> +		if (usb_mode) {
> +			dev_err(data->phy->dev, "doesn't support usb3 mode\n");

USB3 in capitals , USB is an abbreviation.

[...]

> +static int jh7110_pcie_phy_set_mode(struct phy *_phy,

Use plain 'phy' variable name, drop the leading underscore .

> +				    enum phy_mode mode, int submode)
> +{
> +	struct udevice *dev = _phy->dev;
> +	struct jh7110_pcie_phy *phy = dev_get_priv(dev);
> +	int ret;
> +
> +	if (mode == phy->mode)
> +		return 0;
> +
> +	switch (mode) {
> +	case PHY_MODE_USB_HOST:
> +	case PHY_MODE_USB_DEVICE:
> +	case PHY_MODE_USB_OTG:
> +		ret = phy_pcie_mode_set(phy, 1);
> +		if (ret)
> +			return ret;
> +		break;
> +	case PHY_MODE_PCIE:
> +		phy_pcie_mode_set(phy, 0);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	dev_dbg(_phy->dev, "Changing phy mode to %d\n", mode);

PHY in capitals.

> +	phy->mode = mode;
> +
> +	return 0;
> +}
Looks pretty good, thanks !