Re: [PATCH v2 4/4] phy: spacemit: add USB3 support for K1 PCIe/USB3 combo PHY

[Ze Huang <huangze@whut.edu.cn>]

On 5/14/25 4:53 PM, Vinod Koul wrote:
> On 18-04-25, 21:19, Ze Huang wrote:
>> Add support for USB 3.0 mode on the K1 PCIe/USB3 combo PHY. Currently,
>> only USB mode is supported; PCIe support is not included in this change.
>>
>> Signed-off-by: Ze Huang <huangze@whut.edu.cn>
>> ---
...
>> +#define COMBPHY_USB_LFPS_REG		0x58
>> +#define  COMBPHY_USB_LFPS_MASK		0x700
>> +#define  COMBPHY_USB_LFPS_THRES_DEFAULT	0x03
> Same comment as other patch

>> +
>> +#define COMBPHY_MODE_SEL	BIT(3)
>> +#define COMBPHY_WAIT_TIMEOUT	1000
>> +
>> +struct spacemit_combphy_priv {
>> +	struct device *dev;
>> +	struct phy *phy;
>> +	struct reset_control *phy_rst;
>> +	void __iomem *phy_ctrl;
>> +	void __iomem *phy_sel;
>> +	bool rx_always_on;
>> +	u8 lfps_threshold;
>> +	u8 type;
>> +};
>> +
>> +static void spacemit_reg_update(void __iomem *reg, u32 offset, u32 mask, u32 val)
>> +{
>> +	u32 tmp;
>> +
>> +	tmp = readl(reg + offset);
>> +	tmp = (tmp & ~(mask)) | val;
>> +	writel(tmp, reg + offset);
>> +}
>> +
>> +static int spacemit_combphy_wait_ready(struct spacemit_combphy_priv *priv,
>> +				       u32 offset, u32 mask, u32 val)
>> +{
>> +	u32 reg_val;
>> +	int ret = 0;
> Superfluous init, drop it pls

OK

>
>> +
>> +	ret = read_poll_timeout(readl, reg_val, (reg_val & mask) == val,
>> +				1000, COMBPHY_WAIT_TIMEOUT * 1000, false,
>> +				priv->phy_ctrl + offset);
>> +
>> +	return ret;
> why use local variable?

Will drop it.

>
>> +}
>> +
>> +static int spacemit_combphy_set_mode(struct spacemit_combphy_priv *priv)
>> +{
>> +	int ret = 0;
>> +
>> +	switch (priv->type) {
>> +	case PHY_TYPE_USB3:
>> +		spacemit_reg_update(priv->phy_sel, 0, 0, COMBPHY_MODE_SEL);
>> +		break;
>> +	default:
>> +		dev_err(priv->dev, "PHY type %x not supported\n", priv->type);
>> +		ret = -EINVAL;
>> +		break;
>> +	}
>> +
>> +	return ret;
>> +}
...
>> +static const struct of_device_id spacemit_combphy_of_match[] = {
>> +	{ .compatible = "spacemit,k1-combphy", },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, spacemit_combphy_of_match);
>> +
>> +static struct platform_driver spacemit_combphy_driver = {
>> +	.probe	= spacemit_combphy_probe,
>> +	.driver = {
>> +		.name = "spacemit-k1-combphy",
>> +		.of_match_table = spacemit_combphy_of_match,
>> +	},
>> +};
>> +module_platform_driver(spacemit_combphy_driver);
>> +
>> +MODULE_DESCRIPTION("Spacemit PCIE/USB3.0 COMBO PHY driver");
>> +MODULE_LICENSE("GPL");
> Could this be single driver with different init register sequences?

Yes, on K1 SoC, the USB3 and PCIe Port A share the same COMBO PHY hardware.

PHY can be initialized for PCIe as well via a different init sequence.
In the future, we can extend the driver to support both modes based on 
the DT property.