Re: [PATCH net-next v9 03/15] dpll: add basic SiTime SiT9531x support
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-17 15:01:48
Also in:
linux-devicetree, lkml
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 0 · Low: 1 - [Low] The user-visible Kconfig help for SIT9531X_DPLL states the driver "registers each on-chip PLL as a DPLL device and exposes input/output… -- dpll: add basic SiTime SiT9531x support Adds the first piece of a SiTime SiT9531x driver: an I2C driver with a paged regmap configuration, page/offset register helpers, XO rate lookup, an optional reset line and variant detection from the ID byte at page 0 offset 0x02. No DPLL objects are registered yet.
quoted hunk ↗ jump to hunk
diff --git a/drivers/dpll/sit9531x/Kconfig b/drivers/dpll/sit9531x/Kconfig new file mode 100644 index 0000000000000..47aea8674327c --- /dev/null +++ b/drivers/dpll/sit9531x/Kconfig@@ -0,0 +1,17 @@ +# SPDX-License-Identifier: GPL-2.0-only + +config SIT9531X_DPLL + tristate "SiTime SiT9531x DPLL driver" + depends on I2C && NET + select DPLL + select REGMAP_I2C + help + Driver for SiTime SiT9531x family clock generators + (SiT95317, SiT95316). + + This driver registers each on-chip PLL as a DPLL device + and exposes input/output clocks as DPLL pins, providing + runtime configuration via Generic Netlink.
[Severity: Low] This isn't a bug, but the help text describes behaviour that does not exist yet at this commit. Nothing in this patch includes a DPLL header or calls a dpll_*() function, and select DPLL pulls the DPLL core in for a module that does not use it. Would it be worth moving the "registers each on-chip PLL as a DPLL device ... via Generic Netlink" paragraph and select DPLL to the patch that actually performs the registration, so the visible config option matches the code at every step of the series? For the record, this resolves later in the series: the final tree adds dpll.c and prop.c, sit9531x_dpll_device_register() calls dpll_device_register(), and sit9531x_dev_probe() ends with sit9531x_devm_dpll_init(), which makes the help text accurate again.
quoted hunk ↗ jump to hunk
+ + To compile this driver as a module, choose M here: the + module will be called sit9531x.diff --git a/drivers/dpll/sit9531x/Makefile b/drivers/dpll/sit9531x/Makefile new file mode 100644 index 0000000000000..a221fe55386a9 --- /dev/null +++ b/drivers/dpll/sit9531x/Makefile@@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0-only + +obj-$(CONFIG_SIT9531X_DPLL) += sit9531x.o +sit9531x-y := core.odiff --git a/drivers/dpll/sit9531x/core.c b/drivers/dpll/sit9531x/core.c new file mode 100644 index 0000000000000..b0920f76362a1 --- /dev/null +++ b/drivers/dpll/sit9531x/core.c@@ -0,0 +1,272 @@
[ ... ]
+ dev_info(sitdev->dev, "%s detected, %u inputs, %u outputs\n", + sitdev->info->name, sitdev->info->num_inputs, + sitdev->info->num_outputs); + + return 0; +}
Related to the Kconfig comment above: sit9531x_dev_probe() returns right after the dev_info(), so a device tree node describing dpll-types, input-pins and output-pins binds successfully with no DPLL device and no netlink interface, and without any message saying so. Is a note in the help text or the commit log about this intermediate state wanted? [ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915000015.80480-1-arouhi%40sitime.com