| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <kt3envdgn7kxjjvu6mm5hozb3ml64d4s54ssozljmr7qttvxij@hnuzfawjspoy>
Date: Fri, 27 Jun 2025 14:30:25 +0800
From: Inochi Amaoto <inochiama@...il.com>
To: Vinod Koul <vkoul@...nel.org>, Inochi Amaoto <inochiama@...il.com>
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>, Chen Wang <unicorn_wang@...look.com>,
Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>, linux-phy@...ts.infradead.org,
devicetree@...r.kernel.org, sophgo@...ts.linux.dev, linux-kernel@...r.kernel.org,
linux-riscv@...ts.infradead.org, Yixun Lan <dlan@...too.org>, Longbin Li <looong.bin@...il.com>
Subject: Re: [PATCH v4 2/2] phy: sophgo: Add USB 2.0 PHY driver for Sophgo
CV18XX/SG200X
On Thu, Jun 26, 2025 at 05:16:28PM -0700, Vinod Koul wrote:
> On 11-06-25, 16:18, Inochi Amaoto wrote:
> > Add USB 2.0 PHY driver for Sophgo CV18XX/SG200X. Currently
> > this driver does not support OTG mode as lack of document.
> >
> > Signed-off-by: Inochi Amaoto <inochiama@...il.com>
> > ---
> > drivers/phy/Kconfig | 1 +
> > drivers/phy/Makefile | 1 +
> > drivers/phy/sophgo/Kconfig | 19 +++
> > drivers/phy/sophgo/Makefile | 2 +
> > drivers/phy/sophgo/phy-cv1800-usb2.c | 222 +++++++++++++++++++++++++++
> > 5 files changed, 245 insertions(+)
> > create mode 100644 drivers/phy/sophgo/Kconfig
> > create mode 100644 drivers/phy/sophgo/Makefile
> > create mode 100644 drivers/phy/sophgo/phy-cv1800-usb2.c
> >
> > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> > index 58c911e1b2d2..678dd0452f0a 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -122,6 +122,7 @@ source "drivers/phy/renesas/Kconfig"
> > source "drivers/phy/rockchip/Kconfig"
> > source "drivers/phy/samsung/Kconfig"
> > source "drivers/phy/socionext/Kconfig"
> > +source "drivers/phy/sophgo/Kconfig"
> > source "drivers/phy/st/Kconfig"
> > source "drivers/phy/starfive/Kconfig"
> > source "drivers/phy/sunplus/Kconfig"
> > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> > index c670a8dac468..bfb27fb5a494 100644
> > --- a/drivers/phy/Makefile
> > +++ b/drivers/phy/Makefile
> > @@ -35,6 +35,7 @@ obj-y += allwinner/ \
> > rockchip/ \
> > samsung/ \
> > socionext/ \
> > + sophgo/ \
> > st/ \
> > starfive/ \
> > sunplus/ \
> > diff --git a/drivers/phy/sophgo/Kconfig b/drivers/phy/sophgo/Kconfig
> > new file mode 100644
> > index 000000000000..2c943bbe1f81
> > --- /dev/null
> > +++ b/drivers/phy/sophgo/Kconfig
> > @@ -0,0 +1,19 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +#
> > +# Phy drivers for Sophgo platforms
> > +#
> > +
> > +if ARCH_SOPHGO || COMPILE_TEST
> > +
> > +config PHY_SOPHGO_CV1800_USB2
> > + tristate "Sophgo CV18XX/SG200X USB 2.0 PHY support"
> > + depends on MFD_SYSCON
> > + depends on USB_SUPPORT
> > + select GENERIC_PHY
> > + help
> > + Enable this to support the USB 2.0 PHY used with
> > + the DWC2 USB controller in Sophgo CV18XX/SG200X
> > + series SoC.
> > + If unsure, say N.
> > +
> > +endif # ARCH_SOPHGO || COMPILE_TEST
> > diff --git a/drivers/phy/sophgo/Makefile b/drivers/phy/sophgo/Makefile
> > new file mode 100644
> > index 000000000000..318060661759
> > --- /dev/null
> > +++ b/drivers/phy/sophgo/Makefile
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +obj-$(CONFIG_PHY_SOPHGO_CV1800_USB2) += phy-cv1800-usb2.o
> > diff --git a/drivers/phy/sophgo/phy-cv1800-usb2.c b/drivers/phy/sophgo/phy-cv1800-usb2.c
> > new file mode 100644
> > index 000000000000..1d21db7f875b
> > --- /dev/null
> > +++ b/drivers/phy/sophgo/phy-cv1800-usb2.c
> > @@ -0,0 +1,222 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2025 Inochi Amaoto <inochiama@...look.com>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/kernel.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/regmap.h>
> > +#include <linux/spinlock.h>
> > +
> > +#define REG_USB_PHY_CTRL 0x048
> > +
> > +#define PHY_ID_OVERWRITE_EN BIT(6)
> > +#define PHY_ID_OVERWRITE_MODE BIT(7)
> > +#define PHY_ID_OVERWRITE_MODE_HOST FIELD_PREP(BIT(7), 0)
> > +#define PHY_ID_OVERWRITE_MODE_DEVICE FIELD_PREP(BIT(7), 1)
> > +
> > +#define PHY_APP_CLK_RATE 125000000
> > +#define PHY_LPM_CLK_RATE 12000000
> > +#define PHY_STB_CLK_RATE 333334
> > +
> > +struct cv1800_usb_phy {
> > + struct phy *phy;
> > + struct regmap *syscon;
> > + spinlock_t lock;
> > + struct clk *usb_app_clk;
> > + struct clk *usb_lpm_clk;
> > + struct clk *usb_stb_clk;
> > + bool support_otg;
> > +};
> > +
> > +static int cv1800_usb_phy_set_mode(struct phy *_phy,
> > + enum phy_mode mode, int submode)
> > +{
> > + struct cv1800_usb_phy *phy = phy_get_drvdata(_phy);
> > + unsigned int regval = 0;
> > + int ret;
> > +
> > + switch (mode) {
> > + case PHY_MODE_USB_DEVICE:
> > + regval = PHY_ID_OVERWRITE_EN | PHY_ID_OVERWRITE_MODE_DEVICE;
> > + break;
> > + case PHY_MODE_USB_HOST:
> > + regval = PHY_ID_OVERWRITE_EN | PHY_ID_OVERWRITE_MODE_HOST;
> > + break;
> > + case PHY_MODE_USB_OTG:
> > + if (!phy->support_otg)
> > + return 0;
> > +
> > + ret = regmap_read(phy->syscon, REG_USB_PHY_CTRL, ®val);
> > + if (ret)
> > + return ret;
> > +
> > + regval = FIELD_GET(PHY_ID_OVERWRITE_MODE, regval);
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return regmap_update_bits(phy->syscon, REG_USB_PHY_CTRL,
> > + PHY_ID_OVERWRITE_EN | PHY_ID_OVERWRITE_MODE,
> > + regval);
> > +}
> > +
> > +static ssize_t dr_mode_write(struct file *file, const char __user *_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + struct seq_file *seq = file->private_data;
> > + struct cv1800_usb_phy *phy = seq->private;
> > + enum phy_mode mode;
> > + char buf[16];
> > +
> > + if (copy_from_user(&buf, _buf, min_t(size_t, sizeof(buf) - 1, count)))
> > + return -EFAULT;
> > +
> > + if (sysfs_streq(buf, "host"))
> > + mode = PHY_MODE_USB_DEVICE;
> > + else if (sysfs_streq(buf, "peripheral"))
> > + mode = PHY_MODE_USB_DEVICE;
> > + else if (sysfs_streq(buf, "otg"))
> > + mode = PHY_MODE_USB_OTG;
> > + else
> > + return -EINVAL;
> > +
> > + return cv1800_usb_phy_set_mode(phy->phy, mode, 0);
> > +}
> > +
> > +static int dr_mode_show(struct seq_file *seq, void *v)
> > +{
> > + struct cv1800_usb_phy *phy = seq->private;
> > + unsigned long flags;
> > + unsigned int regval;
> > + bool is_host = true;
> > + int ret;
> > +
> > + spin_lock_irqsave(&phy->lock, flags);
> > + ret = regmap_read(phy->syscon, REG_USB_PHY_CTRL, ®val);
> > + spin_unlock_irqrestore(&phy->lock, flags);
> > +
> > + if (ret)
> > + return ret;
> > +
> > + if (regval & PHY_ID_OVERWRITE_MODE)
> > + is_host = false;
> > +
> > + if (!(regval & PHY_ID_OVERWRITE_EN))
> > + seq_puts(seq, "otg: ");
> > +
> > + seq_puts(seq, is_host ? "host\n" : "peripheral\n");
> > +
> > + return 0;
> > +}
>
> This should be done by host controller and not phy and then use apis to
> set the mode for phy from controller, pls see other driver on how they
> do this
>
Cool, I will remove this thing and let the controller do this.
> > +
> > +DEFINE_SHOW_STORE_ATTRIBUTE(dr_mode);
> > +
> > +static int cv1800_usb_phy_set_clock(struct cv1800_usb_phy *phy)
> > +{
> > + int ret;
> > +
> > + ret = clk_set_rate(phy->usb_app_clk, PHY_APP_CLK_RATE);
> > + if (ret)
> > + return ret;
> > +
> > + ret = clk_set_rate(phy->usb_lpm_clk, PHY_LPM_CLK_RATE);
> > + if (ret)
> > + return ret;
> > +
> > + ret = clk_set_rate(phy->usb_stb_clk, PHY_STB_CLK_RATE);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
>
> Should this not be return ret here? or just do return clk_set_rate()
> here
>
I think return ret is the same as return 0. And it is a good
idea to just do return clk_set_rate.
>
> > +}
> > +
> > +static const struct phy_ops cv1800_usb_phy_ops = {
> > + .set_mode = cv1800_usb_phy_set_mode,
> > + .owner = THIS_MODULE,
> > +};
> > +
> > +static int cv1800_usb_phy_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct device *parent = dev->parent;
> > + struct cv1800_usb_phy *phy;
> > + struct phy_provider *phy_provider;
> > + int ret;
> > +
> > + if (!parent)
> > + return -ENODEV;
> > +
> > + phy = devm_kmalloc(dev, sizeof(*phy), GFP_KERNEL);
> > + if (!phy)
> > + return -ENOMEM;
> > +
> > + phy->syscon = syscon_node_to_regmap(parent->of_node);
> > + if (IS_ERR_OR_NULL(phy->syscon))
> > + return -ENODEV;
> > +
> > + phy->support_otg = false;
> > +
> > + spin_lock_init(&phy->lock);
> > +
> > + phy->usb_app_clk = devm_clk_get_enabled(dev, "app");
> > + if (IS_ERR(phy->usb_app_clk))
> > + return dev_err_probe(dev, PTR_ERR(phy->usb_app_clk),
> > + "Failed to get app clock\n");
> > +
> > + phy->usb_lpm_clk = devm_clk_get_enabled(dev, "lpm");
> > + if (IS_ERR(phy->usb_lpm_clk))
> > + return dev_err_probe(dev, PTR_ERR(phy->usb_lpm_clk),
> > + "Failed to get lpm clock\n");
> > +
> > + phy->usb_stb_clk = devm_clk_get_enabled(dev, "stb");
> > + if (IS_ERR(phy->usb_stb_clk))
> > + return dev_err_probe(dev, PTR_ERR(phy->usb_stb_clk),
> > + "Failed to get stb clock\n");
> > +
> > + phy->phy = devm_phy_create(dev, NULL, &cv1800_usb_phy_ops);
> > + if (IS_ERR(phy->phy))
> > + return dev_err_probe(dev, PTR_ERR(phy->phy),
> > + "Failed to create phy\n");
> > +
> > + ret = cv1800_usb_phy_set_clock(phy);
> > + if (ret)
> > + return ret;
> > +
> > + debugfs_create_file("dr_mode", 0644, phy->phy->debugfs,
> > + phy, &dr_mode_fops);
> > +
> > + phy_set_drvdata(phy->phy, phy);
> > + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> > +
> > + return PTR_ERR_OR_ZERO(phy_provider);
> > +}
> > +
> > +static const struct of_device_id cv1800_usb_phy_ids[] = {
> > + { .compatible = "sophgo,cv1800b-usb2-phy" },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, cv1800_usb_phy_ids);
> > +
> > +static struct platform_driver cv1800_usb_phy_driver = {
> > + .probe = cv1800_usb_phy_probe,
> > + .driver = {
> > + .name = "cv1800-usb2-phy",
> > + .of_match_table = cv1800_usb_phy_ids,
> > + },
> > +};
> > +module_platform_driver(cv1800_usb_phy_driver);
> > +
> > +MODULE_AUTHOR("Inochi Amaoto <inochiama@...look.com>");
> > +MODULE_DESCRIPTION("CV1800/SG2000 SoC USB 2.0 PHY driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.49.0
>
> --
> ~Vinod
Powered by blists - more mailing lists