Re: [PATCH v3 3/5] phy: cix: Add CIX Sky1 USB3 PHY driver
From: sashiko-bot@kernel.org
Date: 2026-09-03 02:48:52
Also in:
linux-devicetree
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [Low] Incorrect format specifier for printing `device_node` pointer. - [Low] Missing `devm_mutex_init()` for devm-allocated mutex. - [Low] Missing explicit inclusion of `<linux/property.h>`. - [Low] Unused data structures and enumerations in the header file. -- commit 5cad48211eb5758f9a40687789544cb07846cb4a Author: Hongliang Yang [off-list ref] phy: cix: Add CIX Sky1 USB3 PHY driver Add PHY driver for the USB3 PHY found on CIX Sky1 SoCs. The driver handles initialization, reset, and clock management for the multi-port USB3 PHY.
quoted hunk ↗ jump to hunk
diff --git a/drivers/phy/cix/phy-cix-usb3.c b/drivers/phy/cix/phy-cix-usb3.c new file mode 100644 index 0000000000000..0f074af84e909 --- /dev/null +++ b/drivers/phy/cix/phy-cix-usb3.c@@ -0,0 +1,410 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * phy driver for cdn_sd0803_t7g_typea + * + * Author: Hongliang Yang <hongliang.yang@cixtech.com> + */ + +#include <linux/clk.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of.h> +#include <linux/phy/phy.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/reset.h> +#include "phy-cix-usbdp.h"
[Severity: Low] Since the driver uses device_for_each_child_node_scoped(), should <linux/property.h> be explicitly included here to avoid relying on implicit dependencies from other headers? [ ... ]
+static int cix_u3phy_probe(struct platform_device *pdev)
+{
+ struct cix_u3phy *u3phy;
+ struct device *dev = &pdev->dev;
+ struct phy_provider *phy_provider;
+ struct device_node *np = dev->of_node;
+ struct device_node *child_np;
+ struct phy *phy;
+ int ret;
+
+ u3phy = devm_kzalloc(dev, sizeof(*u3phy), GFP_KERNEL);
+ if (!u3phy)
+ return -ENOMEM;
+ dev_set_drvdata(dev, u3phy);
+ u3phy->dev = dev;
+ u3phy->cfg = device_get_match_data(dev);
+ if (!u3phy->cfg) {
+ dev_err(dev, "no OF data can be matched with %p node\n", np);[Severity: Low] Will this print the raw pointer address instead of the device node name? Since np is a device_node pointer, should the %pOF format specifier be used instead?
+ return -EINVAL; + } + u3phy->reset = devm_reset_control_get_exclusive(dev, "core");
[ ... ]
+ u3phy->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(u3phy->base))
+ return PTR_ERR(u3phy->base);
+ u3phy->phy_regmap = devm_regmap_init(dev, NULL, u3phy, &cix_u3phy_regmap_cfg);
+ if (IS_ERR(u3phy->phy_regmap)) {
+ dev_err(dev, "failed to remap phy register\n");
+ return PTR_ERR(u3phy->phy_regmap);
+ }
+ mutex_init(&u3phy->mutex);[Severity: Low] Since u3phy is allocated using devm_kzalloc(), the memory will be automatically freed on driver unbind. When initialized with mutex_init(), mutex_destroy() won't be called automatically, which skips cleanup steps expected by the resource management API for devm-allocated mutexes. Should devm_mutex_init() be used here instead?
+ device_for_each_child_node_scoped(dev, child_fn) {
+ const char *name = fwnode_get_name(child_fn);[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/phy/cix/phy-cix-usbdp.h b/drivers/phy/cix/phy-cix-usbdp.h new file mode 100644 index 0000000000000..6ec7eb5c81aa9 --- /dev/null +++ b/drivers/phy/cix/phy-cix-usbdp.h@@ -0,0 +1,310 @@
[ ... ]
+/* gop status address */
+#define GOP_STATUS_ADDRESS 0x83E05000
+#define GOP_STATUS_SIZE 0x04
+
+struct gop_status {
+ unsigned char phy_status[4];
+};
+
+/*
+ * 0: usb
+ * 1: 2 lane usb+ 2 lane dp
+ * 2: usb device
+ * 3: usb2.0 + 4 lane dp
+ */
+enum phy_role {
+ USB_ROLE_NONE,
+ USB_ROLE_HOST,
+ USB_ROLE_DEVICE,
+ USB_ROLE_HOST_20,
+};[Severity: Low] Are this struct and enum needed? Neither gop_status nor phy_role appear to be used anywhere in the provided code.
+ +#endif
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260903023521.2064986-1-hongliang.yang@cixtech.com?part=3 -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy