[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <95e8f8fd-d7ea-4d78-a794-72f73a3e19f3@whut.edu.cn>
Date: Wed, 14 May 2025 20:00:28 +0800
From: Ze Huang <huangze@...t.edu.cn>
To: Vinod Koul <vkoul@...nel.org>
Cc: Kishon Vijay Abraham I <kishon@...nel.org>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Yixun Lan <dlan@...too.org>,
Philipp Zabel <p.zabel@...gutronix.de>, linux-phy@...ts.infradead.org,
devicetree@...r.kernel.org, linux-riscv@...ts.infradead.org,
spacemit@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/4] phy: spacemit: add USB3 support for K1 PCIe/USB3
combo PHY
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@...t.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.
Powered by blists - more mailing lists