* Re: [PATCH v9 0/4] Introduce ASPEED AST27xx BMC SoC
From: patchwork-bot+linux-riscv @ 2026-06-26 8:21 UTC (permalink / raw)
To: Ryan Chen
Cc: linux-riscv, robh, krzk+dt, conor+dt, joel, andrew,
catalin.marinas, will, arnd, krzk, alexandre.belloni, linusw,
fustini, pjw, palmer, aou, alex, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel, soc, conor.dooley
In-Reply-To: <20260609-upstream_ast2700-v9-0-f631752f0cb1@aspeedtech.com>
Hello:
This series was applied to riscv/linux.git (fixes)
by Arnd Bergmann <arnd@arndb.de>:
On Tue, 9 Jun 2026 10:47:17 +0800 you wrote:
> This introduces initial support for the Aspeed AST27xx SoC and the AST2700
> Evaluation Board (EVB) to the Linux kernel. The AST27xx is the 8th
> generation Baseboard Management Controller (BMC) SoC from Aspeed,
> featuring improved performance, enhanced security, and expanded I/O
> capabilities compared to previous generations.
>
> AST27xx SOC Family
> - https://www.aspeedtech.com/server_ast2700/
> - https://www.aspeedtech.com/server_ast2720/
> - https://www.aspeedtech.com/server_ast2750/
>
> [...]
Here is the summary with links:
- [v9,1/4] dt-bindings: arm: aspeed: Add AST2700 board compatible
https://git.kernel.org/riscv/c/34efd73379ff
- [v9,2/4] arm64: Kconfig: Add ASPEED SoC family Kconfig support
https://git.kernel.org/riscv/c/df6f379eb4ac
- [v9,3/4] arm64: dts: aspeed: Add initial AST27xx SoC device tree
https://git.kernel.org/riscv/c/e77bb5dc5759
- [v9,4/4] arm64: configs: Update defconfig for AST2700 platform support
https://git.kernel.org/riscv/c/512cef2af615
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v6 2/2] drm/bridge: Add Lontium LT9611C(EX/UXD) MIPI DSI to HDMI driver
From: Sunyun Yang @ 2026-06-26 8:13 UTC (permalink / raw)
To: Maxime Ripard
Cc: Krzysztof Kozlowski, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, dmitry.baryshkov, maarten.lankhorst, rfoss,
Laurent.pinchart, tzimmermann, jonas, jernej.skrabec, devicetree,
dri-devel, linux-kernel, xmzhu, xmzhu, rlyu, xbpeng
In-Reply-To: <20260626-zippy-affable-hamster-22101d@houat>
Maxime Ripard <mripard@kernel.org> 于2026年6月26日周五 15:49写道：
>
> On Fri, Jun 26, 2026 at 10:15:03AM +0800, Sunyun Yang wrote:
> > Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 21:51写道：
> > >
> > > On 25/06/2026 15:40, Sunyun Yang wrote:
> > > > Sunyun Yang <syyang@lontium.com> 于2026年6月25日周四 21:26写道：
> > > >>
> > > >> Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 21:17写道：
> > > >>>
> > > >>> On 25/06/2026 15:14, Sunyun Yang wrote:
> > > >>>> Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 20:54写道：
> > > >>>>>
> > > >>>>> On 08/05/2026 15:40, syyang@lontium.com wrote:
> > > >>>>>> +
> > > >>>>>> +static void lt9611c_reset(struct lt9611c *lt9611c)
> > > >>>>>> +{
> > > >>>>>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 1);
> > > >>>>>> + msleep(20);
> > > >>>>>> +
> > > >>>>>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 0);
> > > >>>>>> + msleep(20);
> > > >>>>>> +
> > > >>>>>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 1);
> > > >>>>>
> > > >>>>> This is just plain wrong. Why do you assert, then de-assert and then
> > > >>>>> finally assert AGAIN the reset leaving the device in powerdown stage?
> > > >>>>>
> > > >>>> I am using software to emulate the hardware RESET button on our EVB.
> > > >>>> When the hardware RESET button is pressed while our chip is running,
> > > >>>> the signal level changes from HIGH to LOW and then back to HIGH.
> > > >>>>
> > > >>>> Of course, we can also use the following:
> > > >>>> static void lt9611c_reset(struct lt9611c *lt9611c)
> > > >>>> {
> > > >>>> gpiod_set_value_cansleep(lt9611c->reset_gpio, 0);
> > > >>>> msleep(50);
> > > >>>> gpiod_set_value_cansleep(lt9611c->reset_gpio, 1);
> > > >>>> msleep(20);
> > > >>>> }
> > > >>>
> > > >>> Makes no sense either and you just did not get the point and did not
> > > >>> answer my question. I asked WHY you leave asserted. Answer "we emulate"
> > > >>> is just plain wrong.
> > > >>>
> > > >>> So again please answer:
> > > >>>
> > > >>> Why do you leave device with reset asserted?
> > > >>>
> > > >>
> > > >> devicetree: reset-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>;
> > > >>
> > > >> GPIO_ACTIVE_HIGH:
> > > >>
> > > >> gpiod_set_value_cansleep(lt9611c->reset_gpio, 0); ------ reset pin
> > > >> is Low level : Clear the register configuration in the chip to stop
> > > >> the chip from working.
> > > >>
> > > >> gpiod_set_value_cansleep(lt9611c->reset_gpio, 1); ------ reset pin
> > > >> is high level: The chip resumes operation.
> > > >>
> > > >>
> > > >
> > > > Our purpose is: pull the level low to clear the register configuration
> > > > in the chip, and then pull it high to allow the MCU inside the chip to
> > > > re‑initialize the registers.
> > >
> > >
> > > And you do completely opposite... so that confirms your code is just wrong.
> > >
> >
> > The lontium-lt9611.yaml uses GPIO_ACTIVE_HIGH. I am just following the
> > rule of this device tree. If I modify the device tree to use
> > GPIO_ACTIVE_LOW,
> > and use the following code in my driver, then my driver would be correct.
> > However, would the existing kernel drivers lontium-lt9611uxc.c and
> > lontium-lt9611.c be affected?
>
> It might, but then it's a DT problem. The GPIO API for drivers always
> considers the logical state of a GPIO, so if you need to assert a
> signal, you'll always need to set 1. That's what Krzysztof was trying to
> explain.
>
> The DT will provide with GPIO_ACTIVE_* how that logical state translates
> to a physical GPIO state.
>
> If the DT says that this particular GPIO is active-high, then it means
> that we need to set the GPIO to 1 to assert reset. Now of course, it
> might not make sense for the controller itself, but it might for the
> board if there's a GPIO inverter in the middle for example.
>
> Anyway, in the case you're raising, the issue definitely lies in the DT,
> and that's what would need to be fixed.
>
> I also wouldn't be too concerned about lontium-lt9611.yaml, it's just an
> example.
>
> Maxime
thanks Maxime, I will modify this code in the next version of the
driver, and I hope you can accept these changes.
Maxime:
I have another question I would like to ask you
regarding sashiko-bot@kernel.org. Since sashiko-bot sometimes has
opinions that differ from yours, whose advice should I follow?
If I do not adopt sashiko-bot's suggestions, will my patches still be
accepted into the upstream Linux kernel?
^ permalink raw reply
* Re: [PATCH 3/4] firmware: raspberrypi: Add reboot mode support
From: Gregor Herburger @ 2026-06-26 8:16 UTC (permalink / raw)
To: sashiko-reviews; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260626074334.8E6201F000E9@smtp.kernel.org>
Hi,
On Fri, Jun 26, 2026 at 07:43:33AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Mixing managed (devm) and manual API calls for reboot mode registration causes a double-free and a use-after-free on device removal.
> - [High] Ignoring the return value of `devm_reboot_mode_register` leads to a NULL pointer dereference or double free if registration fails.
> --
yes I will address this points in a v2. But first I want to wait if I get more
feedback on this series. In particular, whether this is the right approach from
a conceptual standpoint.
>
> commit c4eb702fd9dc0d615bc8a99fbb83befc50f8971f
> Author: Gregor Herburger <gregor.herburger@linutronix.de>
>
> firmware: raspberrypi: Add reboot mode support
>
> The Raspberry Pi firmware has a tryboot mode where it tries to boot from
> another partition. This patch adds support for this by registering a reboot
> mode driver.
>
> > diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> > --- a/drivers/firmware/raspberrypi.c
> > +++ b/drivers/firmware/raspberrypi.c
> > @@ -306,6 +323,12 @@ static int rpi_firmware_probe(struct platform_device *pdev)
> > rpi_register_hwmon_driver(dev, fw);
> > rpi_register_clk_driver(dev);
> >
> > + fw->reboot_mode.dev = dev;
> > + fw->reboot_mode.write = rpi_firmware_reboot_mode_write;
> > + ret = devm_reboot_mode_register(dev, &fw->reboot_mode);
> > + if (ret)
> > + dev_err(dev, "Failed to register reboot mode: %d\n", ret);
> > +
> > return 0;
> > }
>
> [Severity: High]
> Should we return the error code here instead of returning 0?
>
> If devm_reboot_mode_register() fails, the probe succeeds but the internal
> list head for the reboot mode might remain uninitialized.
>
> Later during device removal, rpi_firmware_remove() calls
> reboot_mode_unregister() unconditionally. This could lead to a null pointer
> dereference when iterating the uninitialized list, or a double free if the
> failure occurred inside the register function and it already freed the
> resources.
>
> [ ... ]
>
> > @@ -323,6 +346,8 @@ static void rpi_firmware_remove(struct platform_device *pdev)
> > {
> > struct rpi_firmware *fw = platform_get_drvdata(pdev);
> >
> > + reboot_mode_unregister(&fw->reboot_mode);
> > +
>
> [Severity: High]
> Could this manual unregister cause a double-free and use-after-free?
>
> Since devm_reboot_mode_register() is used in probe, the devres cleanup will
> automatically unregister the reboot mode when the device is removed.
>
> By manually calling reboot_mode_unregister() here:
> 1. It frees the strings in the list.
> 2. rpi_firmware_put(fw) drops the reference, potentially freeing the fw
> structure.
> 3. The devres cleanup runs devm_reboot_mode_release() upon returning,
> accessing the freed fw structure and attempting to unregister and free
> the strings a second time.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rpi-tryboot-v1-0-490b1c4c4970@linutronix.de?part=3
--
Gregor Herburger
Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen
Phone: +49 7556 25 999 35; Fax.: +49 7556 25 999 99
Hinweise zum Datenschutz finden Sie hier (Informations on data privacy
can be found here): https://linutronix.de/legal/data-protection.php
Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen |
Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700
806 | Geschäftsführer (Managing Directors): Dr. Wilfried Wessner,
Katharina Kopp, Alexander Gieringer
^ permalink raw reply
* [PATCH v4 2/2] riscv: dts: spacemit: Add cpu scaling for K1 SoC
From: Shuwei Wu @ 2026-06-26 8:10 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Yixun Lan, Yixun Lan
Cc: linux-pm, linux-kernel, linux-riscv, spacemit, devicetree,
Shuwei Wu
In-Reply-To: <20260626-shadow-deps-v4-0-bba9831f2f1d@mailbox.org>
Add CPU clock properties and OPP tables for the two CPU clusters in the
SpacemiT K1 SoC. The OPP entries use voltage ranges because the CPU
supply is shared by both clusters.
Enable CPU DVFS on Banana Pi BPI-F3 by including the OPP tables and
wiring the CPU nodes to the CPU regulator supply.
Signed-off-by: Shuwei Wu <shuwei.wu@mailbox.org>
---
Changes in v4:
- Use separate OPP tables for the two CPU clock clusters
- Use voltage ranges for the shared CPU supply
Changes in v3:
- Use one shared CPU OPP table for all CPUs
Changes in v2:
- Add k1-opp.dtsi with OPP tables for both CPU clusters
- Assign CPU supplies and include OPP table for Banana Pi BPI-F3
---
---
arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts | 35 +++++++-
arch/riscv/boot/dts/spacemit/k1-opp.dtsi | 105 ++++++++++++++++++++++++
arch/riscv/boot/dts/spacemit/k1.dtsi | 8 ++
3 files changed, 147 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
index 444c3b1e6f44..487179f7b9b9 100644
--- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
+++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
@@ -4,6 +4,7 @@
*/
#include "k1.dtsi"
+#include "k1-opp.dtsi"
#include "k1-pinctrl.dtsi"
/ {
@@ -86,6 +87,38 @@ &combo_phy {
status = "okay";
};
+&cpu_0 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_1 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_2 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_3 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_4 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_5 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_6 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_7 {
+ cpu-supply = <&buck1_0v9>;
+};
+
&emmc {
bus-width = <8>;
mmc-hs400-1_8v;
@@ -201,7 +234,7 @@ pmic@41 {
dldoin2-supply = <&buck5>;
regulators {
- buck1 {
+ buck1_0v9: buck1 {
regulator-min-microvolt = <500000>;
regulator-max-microvolt = <3450000>;
regulator-ramp-delay = <5000>;
diff --git a/arch/riscv/boot/dts/spacemit/k1-opp.dtsi b/arch/riscv/boot/dts/spacemit/k1-opp.dtsi
new file mode 100644
index 000000000000..4cebfcd87485
--- /dev/null
+++ b/arch/riscv/boot/dts/spacemit/k1-opp.dtsi
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+/ {
+ cluster0_opp_table: opp-table-cluster0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-614400000 {
+ opp-hz = /bits/ 64 <614400000>;
+ opp-microvolt = <950000 950000 1050000>;
+ clock-latency-ns = <200000>;
+ };
+
+ opp-819000000 {
+ opp-hz = /bits/ 64 <819000000>;
+ opp-microvolt = <950000 950000 1050000>;
+ clock-latency-ns = <200000>;
+ };
+
+ opp-1000000000 {
+ opp-hz = /bits/ 64 <1000000000>;
+ opp-microvolt = <950000 950000 1050000>;
+ clock-latency-ns = <200000>;
+ };
+
+ opp-1228800000 {
+ opp-hz = /bits/ 64 <1228800000>;
+ opp-microvolt = <950000 950000 1050000>;
+ clock-latency-ns = <200000>;
+ };
+
+ opp-1600000000 {
+ opp-hz = /bits/ 64 <1600000000>;
+ opp-microvolt = <1050000 1050000 1050000>;
+ clock-latency-ns = <200000>;
+ };
+ };
+
+ cluster1_opp_table: opp-table-cluster1 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-614400000 {
+ opp-hz = /bits/ 64 <614400000>;
+ opp-microvolt = <950000 950000 1050000>;
+ clock-latency-ns = <200000>;
+ };
+
+ opp-819000000 {
+ opp-hz = /bits/ 64 <819000000>;
+ opp-microvolt = <950000 950000 1050000>;
+ clock-latency-ns = <200000>;
+ };
+
+ opp-1000000000 {
+ opp-hz = /bits/ 64 <1000000000>;
+ opp-microvolt = <950000 950000 1050000>;
+ clock-latency-ns = <200000>;
+ };
+
+ opp-1228800000 {
+ opp-hz = /bits/ 64 <1228800000>;
+ opp-microvolt = <950000 950000 1050000>;
+ clock-latency-ns = <200000>;
+ };
+
+ opp-1600000000 {
+ opp-hz = /bits/ 64 <1600000000>;
+ opp-microvolt = <1050000 1050000 1050000>;
+ clock-latency-ns = <200000>;
+ };
+ };
+};
+
+&cpu_0 {
+ operating-points-v2 = <&cluster0_opp_table>;
+};
+
+&cpu_1 {
+ operating-points-v2 = <&cluster0_opp_table>;
+};
+
+&cpu_2 {
+ operating-points-v2 = <&cluster0_opp_table>;
+};
+
+&cpu_3 {
+ operating-points-v2 = <&cluster0_opp_table>;
+};
+
+&cpu_4 {
+ operating-points-v2 = <&cluster1_opp_table>;
+};
+
+&cpu_5 {
+ operating-points-v2 = <&cluster1_opp_table>;
+};
+
+&cpu_6 {
+ operating-points-v2 = <&cluster1_opp_table>;
+};
+
+&cpu_7 {
+ operating-points-v2 = <&cluster1_opp_table>;
+};
diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
index 529ec68e9c23..bdd109b81730 100644
--- a/arch/riscv/boot/dts/spacemit/k1.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
@@ -54,6 +54,7 @@ cpu_0: cpu@0 {
compatible = "spacemit,x60", "riscv";
device_type = "cpu";
reg = <0>;
+ clocks = <&syscon_apmu CLK_CPU_C0_CORE>;
riscv,isa = "rv64imafdcbv_zicbom_zicbop_zicboz_zicntr_zicond_zicsr_zifencei_zihintpause_zihpm_zfh_zba_zbb_zbc_zbs_zkt_zvfh_zvkt_sscofpmf_sstc_svinval_svnapot_svpbmt";
riscv,isa-base = "rv64i";
riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "b", "v", "zicbom",
@@ -84,6 +85,7 @@ cpu_1: cpu@1 {
compatible = "spacemit,x60", "riscv";
device_type = "cpu";
reg = <1>;
+ clocks = <&syscon_apmu CLK_CPU_C0_CORE>;
riscv,isa = "rv64imafdcbv_zicbom_zicbop_zicboz_zicntr_zicond_zicsr_zifencei_zihintpause_zihpm_zfh_zba_zbb_zbc_zbs_zkt_zvfh_zvkt_sscofpmf_sstc_svinval_svnapot_svpbmt";
riscv,isa-base = "rv64i";
riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "b", "v", "zicbom",
@@ -114,6 +116,7 @@ cpu_2: cpu@2 {
compatible = "spacemit,x60", "riscv";
device_type = "cpu";
reg = <2>;
+ clocks = <&syscon_apmu CLK_CPU_C0_CORE>;
riscv,isa = "rv64imafdcbv_zicbom_zicbop_zicboz_zicntr_zicond_zicsr_zifencei_zihintpause_zihpm_zfh_zba_zbb_zbc_zbs_zkt_zvfh_zvkt_sscofpmf_sstc_svinval_svnapot_svpbmt";
riscv,isa-base = "rv64i";
riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "b", "v", "zicbom",
@@ -144,6 +147,7 @@ cpu_3: cpu@3 {
compatible = "spacemit,x60", "riscv";
device_type = "cpu";
reg = <3>;
+ clocks = <&syscon_apmu CLK_CPU_C0_CORE>;
riscv,isa = "rv64imafdcbv_zicbom_zicbop_zicboz_zicntr_zicond_zicsr_zifencei_zihintpause_zihpm_zfh_zba_zbb_zbc_zbs_zkt_zvfh_zvkt_sscofpmf_sstc_svinval_svnapot_svpbmt";
riscv,isa-base = "rv64i";
riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "b", "v", "zicbom",
@@ -174,6 +178,7 @@ cpu_4: cpu@4 {
compatible = "spacemit,x60", "riscv";
device_type = "cpu";
reg = <4>;
+ clocks = <&syscon_apmu CLK_CPU_C1_CORE>;
riscv,isa = "rv64imafdcbv_zicbom_zicbop_zicboz_zicntr_zicond_zicsr_zifencei_zihintpause_zihpm_zfh_zba_zbb_zbc_zbs_zkt_zvfh_zvkt_sscofpmf_sstc_svinval_svnapot_svpbmt";
riscv,isa-base = "rv64i";
riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "b", "v", "zicbom",
@@ -204,6 +209,7 @@ cpu_5: cpu@5 {
compatible = "spacemit,x60", "riscv";
device_type = "cpu";
reg = <5>;
+ clocks = <&syscon_apmu CLK_CPU_C1_CORE>;
riscv,isa = "rv64imafdcbv_zicbom_zicbop_zicboz_zicntr_zicond_zicsr_zifencei_zihintpause_zihpm_zfh_zba_zbb_zbc_zbs_zkt_zvfh_zvkt_sscofpmf_sstc_svinval_svnapot_svpbmt";
riscv,isa-base = "rv64i";
riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "b", "v", "zicbom",
@@ -234,6 +240,7 @@ cpu_6: cpu@6 {
compatible = "spacemit,x60", "riscv";
device_type = "cpu";
reg = <6>;
+ clocks = <&syscon_apmu CLK_CPU_C1_CORE>;
riscv,isa = "rv64imafdcbv_zicbom_zicbop_zicboz_zicntr_zicond_zicsr_zifencei_zihintpause_zihpm_zfh_zba_zbb_zbc_zbs_zkt_zvfh_zvkt_sscofpmf_sstc_svinval_svnapot_svpbmt";
riscv,isa-base = "rv64i";
riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "b", "v", "zicbom",
@@ -264,6 +271,7 @@ cpu_7: cpu@7 {
compatible = "spacemit,x60", "riscv";
device_type = "cpu";
reg = <7>;
+ clocks = <&syscon_apmu CLK_CPU_C1_CORE>;
riscv,isa = "rv64imafdcbv_zicbom_zicbop_zicboz_zicntr_zicond_zicsr_zifencei_zihintpause_zihpm_zfh_zba_zbb_zbc_zbs_zkt_zvfh_zvkt_sscofpmf_sstc_svinval_svnapot_svpbmt";
riscv,isa-base = "rv64i";
riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "b", "v", "zicbom",
--
2.53.0
^ permalink raw reply related
* [PATCH v4 1/2] cpufreq: dt-platdev: Add SpacemiT K1 SoC to the allowlist
From: Shuwei Wu @ 2026-06-26 8:10 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Yixun Lan, Yixun Lan
Cc: linux-pm, linux-kernel, linux-riscv, spacemit, devicetree,
Shuwei Wu
In-Reply-To: <20260626-shadow-deps-v4-0-bba9831f2f1d@mailbox.org>
Add the compatible string for supporting the generic
cpufreq driver on the SpacemiT K1 SoC.
Signed-off-by: Shuwei Wu <shuwei.wu@mailbox.org>
---
Changes in v4:
- Drop the K1-specific cpufreq driver and use cpufreq-dt again
- Add spacemit,k1 back to the cpufreq-dt allowlist
Changes in v3:
- Add a K1-specific cpufreq driver for the shared-rail, dual-clock topology
- Add spacemit,k1 to the cpufreq-dt blocklist
---
---
drivers/cpufreq/cpufreq-dt-platdev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index 25fd3b191b7e..e262394b56f9 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -81,6 +81,8 @@ static const struct of_device_id allowlist[] __initconst = {
{ .have_governor_per_policy = true, },
},
+ { .compatible = "spacemit,k1", },
+
{ .compatible = "st-ericsson,u8500", },
{ .compatible = "st-ericsson,u8540", },
{ .compatible = "st-ericsson,u9500", },
--
2.53.0
^ permalink raw reply related
* [PATCH v4 0/2] cpufreq: spacemit: Add cpufreq support for K1 SoC
From: Shuwei Wu @ 2026-06-26 8:10 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Yixun Lan, Yixun Lan
Cc: linux-pm, linux-kernel, linux-riscv, spacemit, devicetree,
Shuwei Wu
This series enables CPU DVFS for the SpacemiT K1 SoC using the generic
cpufreq-dt driver.
K1 has two CPU clock clusters. The two clusters have separate CPU clocks,
so they are represented as two cpufreq policies: policy0 for CPUs 0-3 and
policy4 for CPUs 4-7.
The CPU voltage rail is shared between the clusters. To model this with two
policies, the OPP entries describe voltage ranges instead of a single fixed
voltage, so the shared regulator can keep the rail within a range acceptable
for the active OPP constraints.
Tested on Banana Pi BPI-F3:
~ # cat /sys/devices/system/cpu/online
0-7
~ # ls /sys/devices/system/cpu/cpufreq/
policy0 policy4
~ # cat /sys/devices/system/cpu/cpufreq/policy0/scaling_driver
cpufreq-dt
~ # cat /sys/devices/system/cpu/cpufreq/policy0/affected_cpus
0 1 2 3
~ # cat /sys/devices/system/cpu/cpufreq/policy4/scaling_driver
cpufreq-dt
~ # cat /sys/devices/system/cpu/cpufreq/policy4/affected_cpus
4 5 6 7
Both policies expose the same OPP frequencies:
~ # cat /sys/devices/system/cpu/cpufreq/policy0/scaling_available_frequencies
614400 819000 1000000 1228800 1600000
~ # cat /sys/devices/system/cpu/cpufreq/policy4/scaling_available_frequencies
614400 819000 1000000 1228800 1600000
For each policy, scaling_setspeed was set to each supported OPP and the
workload was pinned to one CPU covered by that policy with taskset.
CPU0 was used for policy0, and CPU4 was used for policy4. The clock rates below
are from /sys/kernel/debug/clk/clk_summary.
policy0 / CPU0:
----------------------------------------------------------
Frequency | cpu_c0_core_clk | Real (s) | User (s)
(kHz) | (Hz) | |
-------------+-------------------+------------+-----------
1,600,000 | 1,600,000,000 | 1.81 | 1.80
1,228,800 | 1,228,800,000 | 2.37 | 2.37
1,000,000 | 1,000,000,000 | 2.89 | 2.89
819,000 | 819,200,000 | 3.56 | 3.55
614,400 | 614,400,000 | 4.71 | 4.71
----------------------------------------------------------
policy4 / CPU4:
----------------------------------------------------------
Frequency | cpu_c1_core_clk | Real (s) | User (s)
(kHz) | (Hz) | |
-------------+-------------------+------------+-----------
1,600,000 | 1,600,000,000 | 1.81 | 1.80
1,228,800 | 1,228,800,000 | 2.36 | 2.36
1,000,000 | 1,000,000,000 | 2.89 | 2.89
819,000 | 819,200,000 | 3.55 | 3.55
614,400 | 614,400,000 | 4.71 | 4.70
----------------------------------------------------------
Signed-off-by: Shuwei Wu <shuwei.wu@mailbox.org>
---
Changes in v4:
- Represent K1 as two cpufreq-dt policies, one per CPU clock cluster
- Use OPP voltage ranges for the shared CPU supply
- Link to v3: https://lore.kernel.org/r/20260612-shadow-deps-v3-0-2f3ba88611ff@mailbox.org
Changes in v3:
- Add a K1-specific cpufreq driver for the shared-rail, dual-clock topology
- Use one shared CPU OPP table and one cpufreq policy for all CPUs
- Link to v2: https://lore.kernel.org/r/20260410-shadow-deps-v2-0-4e16b8c0f60e@mailbox.org
Changes in v2:
- Move OPP tables to dedicated k1-opp.dtsi
- Enable OPP only on BPI-F3 with cpu-supply present
- Link to v1: https://lore.kernel.org/r/20260308-shadow-deps-v1-0-0ceb5c7c07eb@mailbox.org
---
Shuwei Wu (2):
cpufreq: dt-platdev: Add SpacemiT K1 SoC to the allowlist
riscv: dts: spacemit: Add cpu scaling for K1 SoC
arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts | 35 +++++++-
arch/riscv/boot/dts/spacemit/k1-opp.dtsi | 105 ++++++++++++++++++++++++
arch/riscv/boot/dts/spacemit/k1.dtsi | 8 ++
drivers/cpufreq/cpufreq-dt-platdev.c | 2 +
4 files changed, 149 insertions(+), 1 deletion(-)
---
base-commit: 5164e95565d3fd508ca8a95351323f5716dfb695
change-id: 20260307-shadow-deps-3582a78aa756
prerequisite-patch-id: 154bd4f720ce5065d58b988de8f273207b44572e
prerequisite-message-id: <20260206-spacemit-p1-v4-0-8f695d93811e@riscstar.com>
prerequisite-patch-id: 5da3e75b18291a5540d4f66d7a0600fb8975ef62
prerequisite-patch-id: bcf41917414ecef8cf743095d130f6004c32f6a5
prerequisite-patch-id: cfe3800f8c791ec4c63e070af9628e88e0fc31b9
prerequisite-message-id: <20260305-k1-clk-fix-v1-1-abca85d6e266@mailbox.org>
prerequisite-patch-id: 7c7fb9f87dba019ece4c97c45750349a7cd28f3a
Best regards,
--
Shuwei Wu <shuwei.wu@mailbox.org>
^ permalink raw reply
* Re: [PATCH v6 2/2] drm/bridge: Add Lontium LT9611C(EX/UXD) MIPI DSI to HDMI driver
From: Sunyun Yang @ 2026-06-26 8:04 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: robh, krzk+dt, conor+dt, andrzej.hajda, neil.armstrong,
dmitry.baryshkov, maarten.lankhorst, rfoss, mripard,
Laurent.pinchart, tzimmermann, jonas, jernej.skrabec, devicetree,
dri-devel, linux-kernel, xmzhu, xmzhu, rlyu, xbpeng
In-Reply-To: <e8001fe1-684d-4de2-bde4-8b0c1ce1dd0d@kernel.org>
Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月26日周五 15:38写道：
>
> On 26/06/2026 04:15, Sunyun Yang wrote:
> > Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 21:51写道：
> >>
> >> On 25/06/2026 15:40, Sunyun Yang wrote:
> >>> Sunyun Yang <syyang@lontium.com> 于2026年6月25日周四 21:26写道：
> >>>>
> >>>> Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 21:17写道：
> >>>>>
> >>>>> On 25/06/2026 15:14, Sunyun Yang wrote:
> >>>>>> Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 20:54写道：
> >>>>>>>
> >>>>>>> On 08/05/2026 15:40, syyang@lontium.com wrote:
> >>>>>>>> +
> >>>>>>>> +static void lt9611c_reset(struct lt9611c *lt9611c)
> >>>>>>>> +{
> >>>>>>>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 1);
> >>>>>>>> + msleep(20);
> >>>>>>>> +
> >>>>>>>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 0);
> >>>>>>>> + msleep(20);
> >>>>>>>> +
> >>>>>>>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 1);
> >>>>>>>
> >>>>>>> This is just plain wrong. Why do you assert, then de-assert and then
> >>>>>>> finally assert AGAIN the reset leaving the device in powerdown stage?
> >>>>>>>
> >>>>>> I am using software to emulate the hardware RESET button on our EVB.
> >>>>>> When the hardware RESET button is pressed while our chip is running,
> >>>>>> the signal level changes from HIGH to LOW and then back to HIGH.
> >>>>>>
> >>>>>> Of course, we can also use the following:
> >>>>>> static void lt9611c_reset(struct lt9611c *lt9611c)
> >>>>>> {
> >>>>>> gpiod_set_value_cansleep(lt9611c->reset_gpio, 0);
> >>>>>> msleep(50);
> >>>>>> gpiod_set_value_cansleep(lt9611c->reset_gpio, 1);
> >>>>>> msleep(20);
> >>>>>> }
> >>>>>
> >>>>> Makes no sense either and you just did not get the point and did not
> >>>>> answer my question. I asked WHY you leave asserted. Answer "we emulate"
> >>>>> is just plain wrong.
> >>>>>
> >>>>> So again please answer:
> >>>>>
> >>>>> Why do you leave device with reset asserted?
> >>>>>
> >>>>
> >>>> devicetree: reset-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>;
> >>>>
> >>>> GPIO_ACTIVE_HIGH:
> >>>>
> >>>> gpiod_set_value_cansleep(lt9611c->reset_gpio, 0); ------ reset pin
> >>>> is Low level : Clear the register configuration in the chip to stop
> >>>> the chip from working.
> >>>>
> >>>> gpiod_set_value_cansleep(lt9611c->reset_gpio, 1); ------ reset pin
> >>>> is high level: The chip resumes operation.
> >>>>
> >>>>
> >>>
> >>> Our purpose is: pull the level low to clear the register configuration
> >>> in the chip, and then pull it high to allow the MCU inside the chip to
> >>> re‑initialize the registers.
> >>
> >>
> >> And you do completely opposite... so that confirms your code is just wrong.
> >>
> >
> > The lontium-lt9611.yaml uses GPIO_ACTIVE_HIGH. I am just following the
> > rule of this device tree. If I modify the device tree to use
> > GPIO_ACTIVE_LOW,
> > and use the following code in my driver, then my driver would be correct.
> > However, would the existing kernel drivers lontium-lt9611uxc.c and
> > lontium-lt9611.c be affected?
>
> DT has nothing to do here. 1 is assert, 0 is de-assert. Your code does
> things opposite to any logic, because you finish function with reset
> asserted.
>
I understand your point, and I will make the changes in the next version.
Additionally, I have another question I would like to ask you
regarding sashiko-bot@kernel.org. Since sashiko-bot sometimes has
opinions that differ from yours, whose advice should I follow?
If I do not adopt sashiko-bot's suggestions, will my patches still be
accepted into the upstream Linux kernel?
>
> Best regards,
> Krzysztof
^ permalink raw reply
* Re: [PATCH v5 0/7] drm/verisilicon: add Nuvoton MA35D1 DCU Lite support
From: Icenowy Zheng @ 2026-06-26 8:05 UTC (permalink / raw)
To: Joey Lu, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt
Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20260625094449.708386-1-a0987203069@gmail.com>
在 2026-06-25四的 17:44 +0800，Joey Lu写道：
> This series adds support for the Verisilicon DCUltraLite display
> controller as integrated in the Nuvoton MA35D1 SoC.
>
> The Verisilicon DC driver and its DT binding were originally written
> by
> Icenowy Zheng <zhengxingda@iscas.ac.cn> for the T-Head TH1520 SoC,
> which
> carries a DC8200 IP block. The present series builds on that
> foundation
> with gratitude to Icenowy for the original work.
>
> The DCUltraLite is a different variant in the DC IP family. While
> the two
> IPs share a broadly similar register layout, a number of differences
> prevent the existing driver from working on the MA35D1 without
> modification:
>
> - No CONFIG_EX commit path: the DC8200 staging registers
> (FB_CONFIG_EX, FB_TOP_LEFT, FB_BOTTOM_RIGHT, FB_BLEND_CONFIG,
> PANEL_CONFIG_EX) are absent. The DCUltraLite uses enable (bit 0)
> and
> reset (bit 4) bits in FB_CONFIG for direct framebuffer updates,
> and
> requires a per-frame VALID bit toggle (FB_CONFIG bit 3) to latch
> configuration changes.
>
> - No PANEL_START register: panel output begins when
> PANEL_CONFIG.RUNNING is set; the DC8200 multi-display sync start
> register at 0x1CCC does not exist.
>
> - Different IRQ registers: DISP_IRQ_STA at 0x147C / DISP_IRQ_EN at
> 0x1480, versus the DC8200's TOP_IRQ_ACK at 0x0010 / TOP_IRQ_EN at
> 0x0014.
>
> - Simpler clock topology: two clocks ("core" bus gate and "pix0"
> pixel
> divider); no axi or ahb clocks required.
>
> - Single display output: no per-output indexing beyond index 0 is
> needed.
>
> - Hardware-discoverable identity: the DCUltraLite exposes chip
> identity
> registers whose model field reads 0x0 (revision 0x5560,
> customer_id 0x305), allowing the existing vs_fill_chip_identity()
> path to identify the variant purely through register reads.
>
> Patch 1 generalises the verisilicon,dc DT binding to accommodate the
> Nuvoton MA35D1 SoC-specific compatible and the variant's two-clock,
> one-reset, single-port topology.
>
> Patch 2 adds the register-level macros needed by the DC8000 ops.
>
> Patches 3-5 introduce the driver changes in three logical steps: the
> vs_dc_funcs hardware ops vtable with DC8200 ops extracted into
> vs_dc8200.c; making axi/ahb clocks optional as a separate atomic
> change;
> and the DC8000 ops in vs_dc8000.c. Patch 6 adds the DCUltraLite HWDB
> entry that gates hardware recognition once all support is in place.
>
> Patch 7 adds the Kconfig dependency on ARCH_MA35, placed last because
> it
> is only meaningful after the HWDB entry is added.
>
> All patches have been tested on Nuvoton MA35D1 hardware.
I also tested these patches on TH1520, and it seems to show no
regression.
Thanks,
Icenowy
>
> Changes from v4:
> - [dt-bindings] Kept clock and reset item descriptions in the
> global
> clocks:/resets: properties; per-compatible sections only
> constrain
> minItems/maxItems and override clock-names items for
> nuvoton,ma35d1-dcu.
> - [dt-bindings] Dropped redundant global minItems/maxItems on
> clocks:
> and clock-names:.
> - [dt-bindings] Dropped the extra-space typo fix in port@0
> description
> to keep the patch atomic; left for a separate patch later.
> - [ops] Renamed crtc_enable/crtc_disable hooks to crtc_enable_ex/
> crtc_disable_ex.
> - [ops] Added unified IRQ bit definitions; each irq_ack()
> implementation
> now translates hardware-specific bits before returning.
> - [clocks] Split the axi/ahb optional-clock change into its own
> patch
> for atomicity.
> - [hwdb] Simplified the commit message for patch 6.
> - [kconfig] Simplified the commit message for patch 7.
>
> Joey Lu (7):
> dt-bindings: display: verisilicon,dc: generalize for single-output
> variants
> drm/verisilicon: add register-level macros for DC8000
> drm/verisilicon: introduce per-variant hardware ops table
> drm/verisilicon: make axi and ahb clocks optional
> drm/verisilicon: add DC8000 (DCUltraLite) display controller
> support
> drm/verisilicon: add DCUltraLite chip identity to HWDB
> drm/verisilicon: extend Kconfig to support ARCH_MA35 platforms
>
> .../bindings/display/verisilicon,dc.yaml | 57 +++++++++
> drivers/gpu/drm/verisilicon/Kconfig | 2 +-
> drivers/gpu/drm/verisilicon/Makefile | 2 +-
> drivers/gpu/drm/verisilicon/vs_bridge.c | 20 +--
> drivers/gpu/drm/verisilicon/vs_crtc.c | 38 +++++-
> drivers/gpu/drm/verisilicon/vs_crtc_regs.h | 1 +
> drivers/gpu/drm/verisilicon/vs_dc.c | 13 +-
> drivers/gpu/drm/verisilicon/vs_dc.h | 33 +++++
> drivers/gpu/drm/verisilicon/vs_dc8000.c | 86 +++++++++++++
> drivers/gpu/drm/verisilicon/vs_dc8200.c | 115
> ++++++++++++++++++
> drivers/gpu/drm/verisilicon/vs_drm.c | 5 +-
> drivers/gpu/drm/verisilicon/vs_drm.h | 8 ++
> drivers/gpu/drm/verisilicon/vs_hwdb.c | 14 +++
> drivers/gpu/drm/verisilicon/vs_hwdb.h | 6 +
> .../gpu/drm/verisilicon/vs_primary_plane.c | 32 +----
> .../drm/verisilicon/vs_primary_plane_regs.h | 3 +
> 16 files changed, 378 insertions(+), 57 deletions(-)
> create mode 100644 drivers/gpu/drm/verisilicon/vs_dc8000.c
> create mode 100644 drivers/gpu/drm/verisilicon/vs_dc8200.c
^ permalink raw reply
* Re: [PATCH v5 6/7] drm/verisilicon: add DCUltraLite chip identity to HWDB
From: Icenowy Zheng @ 2026-06-26 8:04 UTC (permalink / raw)
To: Joey Lu, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt
Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20260625094449.708386-7-a0987203069@gmail.com>
在 2026-06-25四的 17:44 +0800，Joey Lu写道：
> The Nuvoton MA35D1 chip contains a DCUltraLite display controller
> with
> model number 0x0 (sic, the model name contains no number either),
> revision 0x5560 and customer ID 0x305. It has a similar register map
> with DC8000, only one display output and only 32x32 cursor supported.
>
> Signed-off-by: Joey Lu <a0987203069@gmail.com>
> drivers/gpu/drm/verisilicon/vs_hwdb.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> index 91524d16f778..7d630a667a3f 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> @@ -129,6 +129,16 @@ static struct vs_chip_identity
> vs_chip_identities[] = {
> .max_cursor_size = 64,
> .formats = &vs_formats_no_yuv444,
> },
> + {
> + .model = 0x0, /* DCUltraLite */
> + .revision = 0x5560,
> + .customer_id = 0x305,
> +
> + .generation = VSDC_GEN_DC8000,
> + .display_count = 1,
> + .max_cursor_size = 32,
> + .formats = &vs_formats_no_yuv444,
> + },
Checked against the MA35D1 manual, and it looks okay.
```
Reviewed-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
```
Thanks,
Icenowy
> };
>
> int vs_fill_chip_identity(struct regmap *regs,
^ permalink raw reply
* Re: [PATCH v5 5/7] drm/verisilicon: add DC8000 (DCUltraLite) display controller support
From: Icenowy Zheng @ 2026-06-26 8:03 UTC (permalink / raw)
To: Joey Lu, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt
Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20260625094449.708386-6-a0987203069@gmail.com>
在 2026-06-25四的 17:44 +0800，Joey Lu写道：
> The Nuvoton MA35D1 SoC integrates a Verisilicon DCUltraLite display
> controller (DC8000 generation) whose register layout differs from
> the DC8200 in several important ways:
>
> 1. No CONFIG_EX commit path: framebuffer updates use the enable (bit
> 0)
> and reset (bit 4) bits in FB_CONFIG instead of the DC8200 staging
> registers (FB_CONFIG_EX, FB_TOP_LEFT, FB_BOTTOM_RIGHT,
> FB_BLEND_CONFIG, PANEL_CONFIG_EX).
>
> 2. No PANEL_START register: panel output starts when
> PANEL_CONFIG.RUNNING is set; there is no multi-display sync start
> register.
>
> 3. Different IRQ registers: DCUltraLite uses DISP_IRQ_STA (0x147C) /
> DISP_IRQ_EN (0x1480) versus DC8200's TOP_IRQ_ACK (0x0010) /
> TOP_IRQ_EN (0x0014).
>
> 4. Simpler clock topology: only 'core' (bus gate) and 'pix0' (pixel
> divider) clocks; no axi or ahb clocks required.
>
> Signed-off-by: Joey Lu <a0987203069@gmail.com>
> ---
> drivers/gpu/drm/verisilicon/Makefile | 2 +-
> drivers/gpu/drm/verisilicon/vs_dc.c | 5 +-
> drivers/gpu/drm/verisilicon/vs_dc.h | 1 +
> drivers/gpu/drm/verisilicon/vs_dc8000.c | 86
> +++++++++++++++++++++++++
> 4 files changed, 92 insertions(+), 2 deletions(-)
> create mode 100644 drivers/gpu/drm/verisilicon/vs_dc8000.c
>
> diff --git a/drivers/gpu/drm/verisilicon/Makefile
> b/drivers/gpu/drm/verisilicon/Makefile
> index 9d4cd16452fa..d2fd8e4dff24 100644
> --- a/drivers/gpu/drm/verisilicon/Makefile
> +++ b/drivers/gpu/drm/verisilicon/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-only
>
> -verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_dc8200.o
> vs_drm.o vs_hwdb.o \
> +verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_dc8200.o
> vs_dc8000.o vs_drm.o vs_hwdb.o \
> vs_plane.o vs_primary_plane.o vs_cursor_plane.o
>
> obj-$(CONFIG_DRM_VERISILICON_DC) += verisilicon-dc.o
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c
> b/drivers/gpu/drm/verisilicon/vs_dc.c
> index fd1f5fe67a68..9499fffbca58 100644
> --- a/drivers/gpu/drm/verisilicon/vs_dc.c
> +++ b/drivers/gpu/drm/verisilicon/vs_dc.c
> @@ -134,7 +134,10 @@ static int vs_dc_probe(struct platform_device
> *pdev)
> dev_info(dev, "Found DC%x rev %x customer %x\n", dc-
> >identity.model,
> dc->identity.revision, dc->identity.customer_id);
>
> - dc->funcs = &vs_dc8200_funcs;
> + if (dc->identity.generation == VSDC_GEN_DC8200)
> + dc->funcs = &vs_dc8200_funcs;
> + else
> + dc->funcs = &vs_dc8000_funcs;
>
> if (port_count > dc->identity.display_count) {
> dev_err(dev, "too many downstream ports than HW
> capability\n");
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.h
> b/drivers/gpu/drm/verisilicon/vs_dc.h
> index 825f5dd6bf17..ac96ad701199 100644
> --- a/drivers/gpu/drm/verisilicon/vs_dc.h
> +++ b/drivers/gpu/drm/verisilicon/vs_dc.h
> @@ -66,5 +66,6 @@ struct vs_dc {
> };
>
> extern const struct vs_dc_funcs vs_dc8200_funcs;
> +extern const struct vs_dc_funcs vs_dc8000_funcs;
>
> #endif /* _VS_DC_H_ */
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc8000.c
> b/drivers/gpu/drm/verisilicon/vs_dc8000.c
> new file mode 100644
> index 000000000000..fbe0fa516cac
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/vs_dc8000.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2026 Joey Lu <yclu4@nuvoton.com>
> + */
> +
> +#include <linux/regmap.h>
> +
> +#include "vs_crtc_regs.h"
> +#include "vs_dc.h"
> +#include "vs_drm.h"
> +#include "vs_primary_plane_regs.h"
> +
> +static void vs_dc8000_panel_enable_ex(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_RESET);
> +}
> +
> +static void vs_dc8000_panel_disable_ex(struct vs_dc *dc, unsigned
> int output)
> +{
> + regmap_clear_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_RESET);
> +}
> +
> +static void vs_dc8000_crtc_begin(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_VALID);
> +}
> +
> +static void vs_dc8000_crtc_flush(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_clear_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_VALID);
> +}
> +
> +static void vs_dc8000_crtc_enable_ex(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_ENABLE);
> +}
> +
> +static void vs_dc8000_crtc_disable_ex(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_clear_bits(dc->regs, VSDC_FB_CONFIG(output),
> + VSDC_FB_CONFIG_ENABLE);
> +}
> +
> +static void vs_dc8000_enable_vblank(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_set_bits(dc->regs, VSDC_DISP_IRQ_EN,
> + VSDC_DISP_IRQ_VSYNC(output));
> +}
> +
> +static void vs_dc8000_disable_vblank(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_clear_bits(dc->regs, VSDC_DISP_IRQ_EN,
> + VSDC_DISP_IRQ_VSYNC(output));
> +}
> +
> +static u32 vs_dc8000_irq_ack(struct vs_dc *dc)
> +{
> + u32 hw_irqs, unified = 0;
> + unsigned int i;
> +
> + regmap_read(dc->regs, VSDC_DISP_IRQ_STA, &hw_irqs);
> +
> + for (i = 0; i < VSDC_MAX_OUTPUTS; i++) {
> + if (hw_irqs & VSDC_DISP_IRQ_VSYNC(i))
> + unified |= VSDC_IRQ_VSYNC(i);
> + }
Maybe a warning for unknown IRQ bits should be added here.
Thanks,
Icenowy
> +
> + return unified;
> +}
> +
> +const struct vs_dc_funcs vs_dc8000_funcs = {
> + .panel_enable_ex = vs_dc8000_panel_enable_ex,
> + .panel_disable_ex = vs_dc8000_panel_disable_ex,
> + .crtc_begin = vs_dc8000_crtc_begin,
> + .crtc_flush = vs_dc8000_crtc_flush,
> + .crtc_enable_ex = vs_dc8000_crtc_enable_ex,
> + .crtc_disable_ex = vs_dc8000_crtc_disable_ex,
> + .enable_vblank = vs_dc8000_enable_vblank,
> + .disable_vblank = vs_dc8000_disable_vblank,
> + .irq_ack = vs_dc8000_irq_ack,
> +};
^ permalink raw reply
* Re: [PATCH v5 4/7] drm/verisilicon: make axi and ahb clocks optional
From: Icenowy Zheng @ 2026-06-26 8:03 UTC (permalink / raw)
To: Joey Lu, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt
Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20260625094449.708386-5-a0987203069@gmail.com>
在 2026-06-25四的 17:44 +0800，Joey Lu写道：
> The Nuvoton MA35D1 SoC integrates a DCUltraLite display controller
> whose
> AXI and AHB bus clocks share a single gate enable bit with the
> display
> core clock, so the clock driver does not expose them separately. This
> patch makes the axi and ahb clocks optional in the probe.
>
> Signed-off-by: Joey Lu <a0987203069@gmail.com>
```
Reviewed-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
```
Thanks,
Icenowy
> ---
> drivers/gpu/drm/verisilicon/vs_dc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c
> b/drivers/gpu/drm/verisilicon/vs_dc.c
> index 9729b693d360..fd1f5fe67a68 100644
> --- a/drivers/gpu/drm/verisilicon/vs_dc.c
> +++ b/drivers/gpu/drm/verisilicon/vs_dc.c
> @@ -90,13 +90,13 @@ static int vs_dc_probe(struct platform_device
> *pdev)
> return PTR_ERR(dc->core_clk);
> }
>
> - dc->axi_clk = devm_clk_get_enabled(dev, "axi");
> + dc->axi_clk = devm_clk_get_optional_enabled(dev, "axi");
> if (IS_ERR(dc->axi_clk)) {
> dev_err(dev, "can't get axi clock\n");
> return PTR_ERR(dc->axi_clk);
> }
>
> - dc->ahb_clk = devm_clk_get_enabled(dev, "ahb");
> + dc->ahb_clk = devm_clk_get_optional_enabled(dev, "ahb");
> if (IS_ERR(dc->ahb_clk)) {
> dev_err(dev, "can't get ahb clock\n");
> return PTR_ERR(dc->ahb_clk);
^ permalink raw reply
* Re: [PATCH v5 3/7] drm/verisilicon: introduce per-variant hardware ops table
From: Icenowy Zheng @ 2026-06-26 8:02 UTC (permalink / raw)
To: Joey Lu, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt
Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20260625094449.708386-4-a0987203069@gmail.com>
在 2026-06-25四的 17:44 +0800，Joey Lu写道：
> The DC8200 and DCUltraLite share a broadly similar register layout
> but
> differ in how the bridge, CRTC, primary plane and IRQ paths are
> driven.
> Introduce a vs_dc_funcs vtable so each variant can supply its own
> implementation without scattering conditionals across multiple files.
>
> Add a generation field to struct vs_chip_identity to distinguish
> variants.
> Extract the DC8200-specific hardware ops into vs_dc8200.c and add
> unified
> IRQ bit definitions so implementations can translate hardware-
> specific
> bits to a common set. Update the shared code to dispatch through
> dc->funcs.
>
> No behaviour change for existing DC8200 platforms.
>
> Signed-off-by: Joey Lu <a0987203069@gmail.com>
> ---
> drivers/gpu/drm/verisilicon/Makefile | 2 +-
> drivers/gpu/drm/verisilicon/vs_bridge.c | 20 +--
> drivers/gpu/drm/verisilicon/vs_crtc.c | 38 +++++-
> drivers/gpu/drm/verisilicon/vs_dc.c | 6 +-
> drivers/gpu/drm/verisilicon/vs_dc.h | 32 +++++
> drivers/gpu/drm/verisilicon/vs_dc8200.c | 115
> ++++++++++++++++++
> drivers/gpu/drm/verisilicon/vs_drm.c | 5 +-
> drivers/gpu/drm/verisilicon/vs_drm.h | 8 ++
> drivers/gpu/drm/verisilicon/vs_hwdb.c | 4 +
> drivers/gpu/drm/verisilicon/vs_hwdb.h | 6 +
> .../gpu/drm/verisilicon/vs_primary_plane.c | 32 +----
> 11 files changed, 214 insertions(+), 54 deletions(-)
> create mode 100644 drivers/gpu/drm/verisilicon/vs_dc8200.c
>
> diff --git a/drivers/gpu/drm/verisilicon/Makefile
> b/drivers/gpu/drm/verisilicon/Makefile
> index 426f4bcaa834..9d4cd16452fa 100644
> --- a/drivers/gpu/drm/verisilicon/Makefile
> +++ b/drivers/gpu/drm/verisilicon/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-only
>
> -verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_drm.o
> vs_hwdb.o \
> +verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_dc8200.o
> vs_drm.o vs_hwdb.o \
> vs_plane.o vs_primary_plane.o vs_cursor_plane.o
>
> obj-$(CONFIG_DRM_VERISILICON_DC) += verisilicon-dc.o
> diff --git a/drivers/gpu/drm/verisilicon/vs_bridge.c
> b/drivers/gpu/drm/verisilicon/vs_bridge.c
> index dc7c85b07fe3..3fbc8d57f8a1 100644
> --- a/drivers/gpu/drm/verisilicon/vs_bridge.c
> +++ b/drivers/gpu/drm/verisilicon/vs_bridge.c
> @@ -162,15 +162,8 @@ static void vs_bridge_enable_common(struct
> vs_crtc *crtc,
> VSDC_DISP_PANEL_CONFIG_DE_EN |
> VSDC_DISP_PANEL_CONFIG_DAT_EN |
> VSDC_DISP_PANEL_CONFIG_CLK_EN);
> - regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
> - VSDC_DISP_PANEL_CONFIG_RUNNING);
> - regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
> - VSDC_DISP_PANEL_START_MULTI_DISP_SYNC);
> - regmap_set_bits(dc->regs, VSDC_DISP_PANEL_START,
> - VSDC_DISP_PANEL_START_RUNNING(output));
> -
> - regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG_EX(crtc-
> >id),
> - VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
> +
> + dc->funcs->panel_enable_ex(dc, output);
> }
>
> static void vs_bridge_atomic_enable_dpi(struct drm_bridge *bridge,
> @@ -228,14 +221,7 @@ static void vs_bridge_atomic_disable(struct
> drm_bridge *bridge,
> struct vs_dc *dc = crtc->dc;
> unsigned int output = crtc->id;
>
> - regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
> - VSDC_DISP_PANEL_START_MULTI_DISP_SYNC |
> - VSDC_DISP_PANEL_START_RUNNING(output));
> - regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
> - VSDC_DISP_PANEL_CONFIG_RUNNING);
> -
> - regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG_EX(crtc-
> >id),
> - VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
> + dc->funcs->panel_disable_ex(dc, output);
> }
>
> static const struct drm_bridge_funcs vs_dpi_bridge_funcs = {
> diff --git a/drivers/gpu/drm/verisilicon/vs_crtc.c
> b/drivers/gpu/drm/verisilicon/vs_crtc.c
> index 0b8a35d09cd2..1c4aac708669 100644
> --- a/drivers/gpu/drm/verisilicon/vs_crtc.c
> +++ b/drivers/gpu/drm/verisilicon/vs_crtc.c
> @@ -16,10 +16,33 @@
> #include "vs_crtc_regs.h"
> #include "vs_crtc.h"
> #include "vs_dc.h"
> -#include "vs_dc_top_regs.h"
> #include "vs_drm.h"
> #include "vs_plane.h"
>
> +static void vs_crtc_atomic_begin(struct drm_crtc *crtc,
> + struct drm_atomic_commit *state)
> +{
> + struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
> + struct vs_dc *dc = vcrtc->dc;
> + unsigned int output = vcrtc->id;
> +
> + if (dc->funcs->crtc_begin)
> + dc->funcs->crtc_begin(dc, output);
> +}
> +
> +static void vs_crtc_atomic_flush(struct drm_crtc *crtc,
> + struct drm_atomic_commit *state)
> +{
> + struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
> + struct vs_dc *dc = vcrtc->dc;
> + unsigned int output = vcrtc->id;
> +
> + if (dc->funcs->crtc_flush)
> + dc->funcs->crtc_flush(dc, output);
> +
> + drm_crtc_vblank_atomic_flush(crtc, state);
> +}
> +
> static void vs_crtc_atomic_disable(struct drm_crtc *crtc,
> struct drm_atomic_commit *state)
> {
> @@ -30,6 +53,9 @@ static void vs_crtc_atomic_disable(struct drm_crtc
> *crtc,
> drm_crtc_vblank_off(crtc);
>
> clk_disable_unprepare(dc->pix_clk[output]);
> +
> + if (dc->funcs->crtc_disable_ex)
> + dc->funcs->crtc_disable_ex(dc, output);
> }
>
> static void vs_crtc_atomic_enable(struct drm_crtc *crtc,
> @@ -42,6 +68,9 @@ static void vs_crtc_atomic_enable(struct drm_crtc
> *crtc,
> drm_WARN_ON(&dc->drm_dev->base,
> clk_prepare_enable(dc->pix_clk[output]));
>
> + if (dc->funcs->crtc_enable_ex)
> + dc->funcs->crtc_enable_ex(dc, output);
> +
> drm_crtc_vblank_on(crtc);
> }
>
> @@ -119,7 +148,8 @@ static bool vs_crtc_mode_fixup(struct drm_crtc
> *crtc,
> }
>
> static const struct drm_crtc_helper_funcs vs_crtc_helper_funcs = {
> - .atomic_flush = drm_crtc_vblank_atomic_flush,
> + .atomic_begin = vs_crtc_atomic_begin,
> + .atomic_flush = vs_crtc_atomic_flush,
> .atomic_enable = vs_crtc_atomic_enable,
> .atomic_disable = vs_crtc_atomic_disable,
> .mode_set_nofb = vs_crtc_mode_set_nofb,
> @@ -132,7 +162,7 @@ static int vs_crtc_enable_vblank(struct drm_crtc
> *crtc)
> struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
> struct vs_dc *dc = vcrtc->dc;
>
> - regmap_set_bits(dc->regs, VSDC_TOP_IRQ_EN,
> VSDC_TOP_IRQ_VSYNC(vcrtc->id));
> + dc->funcs->enable_vblank(dc, vcrtc->id);
>
> return 0;
> }
> @@ -142,7 +172,7 @@ static void vs_crtc_disable_vblank(struct
> drm_crtc *crtc)
> struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
> struct vs_dc *dc = vcrtc->dc;
>
> - regmap_clear_bits(dc->regs, VSDC_TOP_IRQ_EN,
> VSDC_TOP_IRQ_VSYNC(vcrtc->id));
> + dc->funcs->disable_vblank(dc, vcrtc->id);
> }
>
> static const struct drm_crtc_funcs vs_crtc_funcs = {
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c
> b/drivers/gpu/drm/verisilicon/vs_dc.c
> index dad9967bc10b..9729b693d360 100644
> --- a/drivers/gpu/drm/verisilicon/vs_dc.c
> +++ b/drivers/gpu/drm/verisilicon/vs_dc.c
> @@ -8,9 +8,7 @@
> #include <linux/of.h>
> #include <linux/of_graph.h>
>
> -#include "vs_crtc.h"
> #include "vs_dc.h"
> -#include "vs_dc_top_regs.h"
> #include "vs_drm.h"
> #include "vs_hwdb.h"
>
> @@ -33,7 +31,7 @@ static irqreturn_t vs_dc_irq_handler(int irq, void
> *private)
> struct vs_dc *dc = private;
> u32 irqs;
>
> - regmap_read(dc->regs, VSDC_TOP_IRQ_ACK, &irqs);
> + irqs = dc->funcs->irq_ack(dc);
>
> vs_drm_handle_irq(dc, irqs);
>
> @@ -136,6 +134,8 @@ static int vs_dc_probe(struct platform_device
> *pdev)
> dev_info(dev, "Found DC%x rev %x customer %x\n", dc-
> >identity.model,
> dc->identity.revision, dc->identity.customer_id);
>
> + dc->funcs = &vs_dc8200_funcs;
> +
> if (port_count > dc->identity.display_count) {
> dev_err(dev, "too many downstream ports than HW
> capability\n");
> ret = -EINVAL;
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.h
> b/drivers/gpu/drm/verisilicon/vs_dc.h
> index ed1016f18758..825f5dd6bf17 100644
> --- a/drivers/gpu/drm/verisilicon/vs_dc.h
> +++ b/drivers/gpu/drm/verisilicon/vs_dc.h
> @@ -14,6 +14,7 @@
> #include <linux/reset.h>
>
> #include <drm/drm_device.h>
> +#include <drm/drm_plane.h>
>
> #include "vs_hwdb.h"
>
> @@ -22,6 +23,34 @@
>
> struct vs_drm_dev;
> struct vs_crtc;
> +struct vs_dc;
> +
> +struct vs_dc_funcs {
> + /* Bridge: atomic_enable, atomic_disable */
> + void (*panel_enable_ex)(struct vs_dc *dc, unsigned int
> output);
> + void (*panel_disable_ex)(struct vs_dc *dc, unsigned int
> output);
> +
> + /* CRTC: atomic_begin, atomic_flush */
> + void (*crtc_begin)(struct vs_dc *dc, unsigned int output);
> + void (*crtc_flush)(struct vs_dc *dc, unsigned int output);
> +
> + /* CRTC: atomic_enable, atomic_disable */
> + void (*crtc_enable_ex)(struct vs_dc *dc, unsigned int
> output);
> + void (*crtc_disable_ex)(struct vs_dc *dc, unsigned int
> output);
> +
> + /* CRTC: enable_vblank, disable_vblank */
> + void (*enable_vblank)(struct vs_dc *dc, unsigned int
> output);
> + void (*disable_vblank)(struct vs_dc *dc, unsigned int
> output);
> +
> + /* Primary plane: atomic_enable, atomic_disable,
> atomic_update */
> + void (*primary_plane_enable_ex)(struct vs_dc *dc, unsigned
> int output);
> + void (*primary_plane_disable_ex)(struct vs_dc *dc, unsigned
> int output);
> + void (*primary_plane_update_ex)(struct vs_dc *dc, unsigned
> int output,
> + struct drm_plane_state
> *state);
> +
> + /* IRQ acknowledge */
> + u32 (*irq_ack)(struct vs_dc *dc);
> +};
>
> struct vs_dc {
> struct regmap *regs;
> @@ -33,6 +62,9 @@ struct vs_dc {
>
> struct vs_drm_dev *drm_dev;
> struct vs_chip_identity identity;
> + const struct vs_dc_funcs *funcs;
> };
>
> +extern const struct vs_dc_funcs vs_dc8200_funcs;
> +
> #endif /* _VS_DC_H_ */
> diff --git a/drivers/gpu/drm/verisilicon/vs_dc8200.c
> b/drivers/gpu/drm/verisilicon/vs_dc8200.c
> new file mode 100644
> index 000000000000..17378f4ef96d
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/vs_dc8200.c
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2025 Icenowy Zheng <uwu@icenowy.me>
> + */
> +
> +#include <linux/regmap.h>
> +
> +#include "vs_bridge_regs.h"
> +#include "vs_dc.h"
> +#include "vs_dc_top_regs.h"
> +#include "vs_drm.h"
> +#include "vs_plane.h"
> +#include "vs_primary_plane_regs.h"
> +
> +static void vs_dc8200_panel_enable_ex(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
> + VSDC_DISP_PANEL_CONFIG_RUNNING);
> + regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
> + VSDC_DISP_PANEL_START_MULTI_DISP_SYNC);
> + regmap_set_bits(dc->regs, VSDC_DISP_PANEL_START,
> + VSDC_DISP_PANEL_START_RUNNING(output));
> +
> + regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG_EX(output),
> + VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
> +}
> +
> +static void vs_dc8200_panel_disable_ex(struct vs_dc *dc, unsigned
> int output)
> +{
> + regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
> + VSDC_DISP_PANEL_CONFIG_RUNNING);
> + regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
> + VSDC_DISP_PANEL_START_MULTI_DISP_SYNC |
> + VSDC_DISP_PANEL_START_RUNNING(output));
> +
> + regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG_EX(output),
> + VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
> +}
> +
> +static void vs_dc8200_enable_vblank(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_set_bits(dc->regs, VSDC_TOP_IRQ_EN,
> + VSDC_TOP_IRQ_VSYNC(output));
> +}
> +
> +static void vs_dc8200_disable_vblank(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_clear_bits(dc->regs, VSDC_TOP_IRQ_EN,
> + VSDC_TOP_IRQ_VSYNC(output));
> +}
> +
> +static void vs_dc8200_plane_commit(struct vs_dc *dc, unsigned int
> output)
> +{
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> + VSDC_FB_CONFIG_EX_COMMIT);
> +}
> +
> +static void vs_dc8200_primary_plane_enable_ex(struct vs_dc *dc,
> unsigned int output)
> +{
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> + VSDC_FB_CONFIG_EX_FB_EN);
> + regmap_update_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> + VSDC_FB_CONFIG_EX_DISPLAY_ID_MASK,
> + VSDC_FB_CONFIG_EX_DISPLAY_ID(output));
> +
> + vs_dc8200_plane_commit(dc, output);
> +}
> +
> +static void vs_dc8200_primary_plane_disable_ex(struct vs_dc *dc,
> unsigned int output)
> +{
> + regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> + VSDC_FB_CONFIG_EX_FB_EN);
> +
> + vs_dc8200_plane_commit(dc, output);
> +}
> +
> +static void vs_dc8200_primary_plane_update_ex(struct vs_dc *dc,
> unsigned int output,
> + struct drm_plane_state
> *state)
> +{
> + regmap_write(dc->regs, VSDC_FB_TOP_LEFT(output),
> + VSDC_MAKE_PLANE_POS(state->crtc_x, state-
> >crtc_y));
> + regmap_write(dc->regs, VSDC_FB_BOTTOM_RIGHT(output),
> + VSDC_MAKE_PLANE_POS(state->crtc_x + state-
> >crtc_w,
> + state->crtc_y + state-
> >crtc_h));
> + regmap_write(dc->regs, VSDC_FB_BLEND_CONFIG(output),
> + VSDC_FB_BLEND_CONFIG_BLEND_DISABLE);
> +
> + vs_dc8200_plane_commit(dc, output);
> +}
> +
> +static u32 vs_dc8200_irq_ack(struct vs_dc *dc)
> +{
> + u32 hw_irqs, unified = 0;
> + unsigned int i;
> +
> + regmap_read(dc->regs, VSDC_TOP_IRQ_ACK, &hw_irqs);
> +
> + for (i = 0; i < VSDC_MAX_OUTPUTS; i++) {
> + if (hw_irqs & VSDC_TOP_IRQ_VSYNC(i))
> + unified |= VSDC_IRQ_VSYNC(i);
> + }
Maybe add a drm_WARN_ONCE for unknown hardware IRQ bit?
Well, with this addressed,
```
Reviewed-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
```
Thanks,
Icenowy
> +
> + return unified;
> +}
> +
> +const struct vs_dc_funcs vs_dc8200_funcs = {
> + .panel_enable_ex = vs_dc8200_panel_enable_ex,
> + .panel_disable_ex =
> vs_dc8200_panel_disable_ex,
> + .enable_vblank = vs_dc8200_enable_vblank,
> + .disable_vblank =
> vs_dc8200_disable_vblank,
> + .primary_plane_enable_ex =
> vs_dc8200_primary_plane_enable_ex,
> + .primary_plane_disable_ex =
> vs_dc8200_primary_plane_disable_ex,
> + .primary_plane_update_ex =
> vs_dc8200_primary_plane_update_ex,
> + .irq_ack = vs_dc8200_irq_ack,
> +};
> diff --git a/drivers/gpu/drm/verisilicon/vs_drm.c
> b/drivers/gpu/drm/verisilicon/vs_drm.c
> index fd259d53f49f..24e9d0b008f3 100644
> --- a/drivers/gpu/drm/verisilicon/vs_drm.c
> +++ b/drivers/gpu/drm/verisilicon/vs_drm.c
> @@ -25,7 +25,6 @@
> #include "vs_bridge.h"
> #include "vs_crtc.h"
> #include "vs_dc.h"
> -#include "vs_dc_top_regs.h"
> #include "vs_drm.h"
>
> #define DRIVER_NAME "verisilicon"
> @@ -168,8 +167,8 @@ void vs_drm_handle_irq(struct vs_dc *dc, u32
> irqs)
> unsigned int i;
>
> for (i = 0; i < dc->identity.display_count; i++) {
> - if (irqs & VSDC_TOP_IRQ_VSYNC(i)) {
> - irqs &= ~VSDC_TOP_IRQ_VSYNC(i);
> + if (irqs & VSDC_IRQ_VSYNC(i)) {
> + irqs &= ~VSDC_IRQ_VSYNC(i);
> if (dc->drm_dev->crtcs[i])
> drm_crtc_handle_vblank(&dc->drm_dev-
> >crtcs[i]->base);
> }
> diff --git a/drivers/gpu/drm/verisilicon/vs_drm.h
> b/drivers/gpu/drm/verisilicon/vs_drm.h
> index 606338206a42..6a89c20879df 100644
> --- a/drivers/gpu/drm/verisilicon/vs_drm.h
> +++ b/drivers/gpu/drm/verisilicon/vs_drm.h
> @@ -6,6 +6,7 @@
> #ifndef _VS_DRM_H_
> #define _VS_DRM_H_
>
> +#include <linux/bits.h>
> #include <linux/platform_device.h>
> #include <linux/types.h>
>
> @@ -13,6 +14,13 @@
>
> struct vs_dc;
>
> +/*
> + * DC variants use different interrupt registers with diverging bit
> + * assignments; each irq_ack() implementation must translate its
> + * hardware-specific bits into these definitions.
> + */
> +#define VSDC_IRQ_VSYNC(n) BIT(n)
> +
> struct vs_drm_dev {
> struct drm_device base;
>
> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> index 2a0f7c59afa3..91524d16f778 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
> @@ -94,6 +94,7 @@ static struct vs_chip_identity vs_chip_identities[]
> = {
> .revision = 0x5720,
> .customer_id = ~0U,
>
> + .generation = VSDC_GEN_DC8200,
> .display_count = 2,
> .max_cursor_size = 64,
> .formats = &vs_formats_no_yuv444,
> @@ -103,6 +104,7 @@ static struct vs_chip_identity
> vs_chip_identities[] = {
> .revision = 0x5721,
> .customer_id = 0x30B,
>
> + .generation = VSDC_GEN_DC8200,
> .display_count = 2,
> .max_cursor_size = 64,
> .formats = &vs_formats_no_yuv444,
> @@ -112,6 +114,7 @@ static struct vs_chip_identity
> vs_chip_identities[] = {
> .revision = 0x5720,
> .customer_id = 0x310,
>
> + .generation = VSDC_GEN_DC8200,
> .display_count = 2,
> .max_cursor_size = 64,
> .formats = &vs_formats_with_yuv444,
> @@ -121,6 +124,7 @@ static struct vs_chip_identity
> vs_chip_identities[] = {
> .revision = 0x5720,
> .customer_id = 0x311,
>
> + .generation = VSDC_GEN_DC8200,
> .display_count = 2,
> .max_cursor_size = 64,
> .formats = &vs_formats_no_yuv444,
> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.h
> b/drivers/gpu/drm/verisilicon/vs_hwdb.h
> index 2065ecb73043..a15c8b565604 100644
> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.h
> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.h
> @@ -9,6 +9,11 @@
> #include <linux/regmap.h>
> #include <linux/types.h>
>
> +enum vs_dc_generation {
> + VSDC_GEN_DC8000,
> + VSDC_GEN_DC8200,
> +};
> +
> struct vs_formats {
> const u32 *array;
> unsigned int num;
> @@ -19,6 +24,7 @@ struct vs_chip_identity {
> u32 revision;
> u32 customer_id;
>
> + enum vs_dc_generation generation;
> u32 display_count;
> /*
> * The hardware only supports square cursor planes, so this
> field
> diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> index 1f2be41ae496..f992cb277f61 100644
> --- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> +++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> @@ -53,12 +53,6 @@ static int vs_primary_plane_atomic_check(struct
> drm_plane *plane,
> return 0;
> }
>
> -static void vs_primary_plane_commit(struct vs_dc *dc, unsigned int
> output)
> -{
> - regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> - VSDC_FB_CONFIG_EX_COMMIT);
> -}
> -
> static void vs_primary_plane_atomic_enable(struct drm_plane *plane,
> struct drm_atomic_commit
> *atomic_state)
> {
> @@ -69,13 +63,8 @@ static void vs_primary_plane_atomic_enable(struct
> drm_plane *plane,
> unsigned int output = vcrtc->id;
> struct vs_dc *dc = vcrtc->dc;
>
> - regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> - VSDC_FB_CONFIG_EX_FB_EN);
> - regmap_update_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> - VSDC_FB_CONFIG_EX_DISPLAY_ID_MASK,
> - VSDC_FB_CONFIG_EX_DISPLAY_ID(output));
> -
> - vs_primary_plane_commit(dc, output);
> + if (dc->funcs->primary_plane_enable_ex)
> + dc->funcs->primary_plane_enable_ex(dc, output);
> }
>
> static void vs_primary_plane_atomic_disable(struct drm_plane *plane,
> @@ -88,10 +77,8 @@ static void vs_primary_plane_atomic_disable(struct
> drm_plane *plane,
> unsigned int output = vcrtc->id;
> struct vs_dc *dc = vcrtc->dc;
>
> - regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> - VSDC_FB_CONFIG_EX_FB_EN);
> -
> - vs_primary_plane_commit(dc, output);
> + if (dc->funcs->primary_plane_disable_ex)
> + dc->funcs->primary_plane_disable_ex(dc, output);
> }
>
> static void vs_primary_plane_atomic_update(struct drm_plane *plane,
> @@ -133,18 +120,11 @@ static void
> vs_primary_plane_atomic_update(struct drm_plane *plane,
> regmap_write(dc->regs, VSDC_FB_STRIDE(output),
> fb->pitches[0]);
>
> - regmap_write(dc->regs, VSDC_FB_TOP_LEFT(output),
> - VSDC_MAKE_PLANE_POS(state->crtc_x, state-
> >crtc_y));
> - regmap_write(dc->regs, VSDC_FB_BOTTOM_RIGHT(output),
> - VSDC_MAKE_PLANE_POS(state->crtc_x + state-
> >crtc_w,
> - state->crtc_y + state-
> >crtc_h));
> regmap_write(dc->regs, VSDC_FB_SIZE(output),
> VSDC_MAKE_PLANE_SIZE(state->crtc_w, state-
> >crtc_h));
>
> - regmap_write(dc->regs, VSDC_FB_BLEND_CONFIG(output),
> - VSDC_FB_BLEND_CONFIG_BLEND_DISABLE);
> -
> - vs_primary_plane_commit(dc, output);
> + if (dc->funcs->primary_plane_update_ex)
> + dc->funcs->primary_plane_update_ex(dc, output,
> state);
> }
>
> static const struct drm_plane_helper_funcs
> vs_primary_plane_helper_funcs = {
^ permalink raw reply
* Re: [PATCH v1 2/2] reset: mediatek: add syscon-based reset controller driver
From: sashiko-bot @ 2026-06-26 8:00 UTC (permalink / raw)
To: peter.wang; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260626074820.2537772-3-peter.wang@mediatek.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The driver incorrectly uses read-modify-write operations (`regmap_write_bits`) on dedicated set/clear registers, causing spurious resets or clear operations on unrelated hardware lines.
- [Medium] The temporary array `arr` is allocated with `devm_kcalloc`, permanently wasting memory for the lifetime of the device.
--
commit 51d3326953762868e035e2bcec526fcc8e9e6083
Author: Peter Wang <peter.wang@mediatek.com>
reset: mediatek: add syscon-based reset controller driver
Add a new reset controller driver for MediaTek SoCs that expose reset
lines through memory-mapped registers managed by a syscon node. The
driver reads reset configurations from the mediatek,reset-bits property.
> diff --git a/drivers/reset/reset-mediatek-syscon.c b/drivers/reset/reset-mediatek-syscon.c
> new file mode 100644
> index 0000000000000..da8aa646cee76
> --- /dev/null
> +++ b/drivers/reset/reset-mediatek-syscon.c
[ ... ]
> +static int mediatek_syscon_reset_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct mediatek_syscon_reset_data *data =
> + container_of(rcdev, struct mediatek_syscon_reset_data, rcdev);
> + struct mediatek_syscon_reset_control *rstc = &data->mrstc[id];
> + unsigned int val = BIT(rstc->assert_bit);
> +
> + return regmap_write_bits(data->regmap, rstc->assert_offset, val, val);
[Severity: High]
Since the hardware uses separate set and clear registers, could using
regmap_write_bits() here cause an unintended read-modify-write cycle?
If the register is Write-1-to-Set, reading it might return the current
status, and writing that back will unintentionally write '1' to other
active lines in the same register. Should this use a direct regmap_write()
instead?
> +}
> +
> +/**
> + * mediatek_syscon_reset_deassert() - Deassert a specific reset line.
[ ... ]
> + */
> +static int mediatek_syscon_reset_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct mediatek_syscon_reset_data *data =
> + container_of(rcdev, struct mediatek_syscon_reset_data, rcdev);
> + struct mediatek_syscon_reset_control *rstc = &data->mrstc[id];
> + unsigned int val = BIT(rstc->deassert_bit);
> +
> + return regmap_write_bits(data->regmap, rstc->deassert_offset, val, val);
[Severity: High]
Similarly, for the deassert (Write-1-to-Clear) register, does using a
read-modify-write operation here risk unintentionally clearing other
unrelated reset lines that happen to read back as '1'?
> +}
[ ... ]
> +static int mediatek_syscon_reset_probe(struct platform_device *pdev)
> +{
[ ... ]
> + if (count % 5 != 0) {
> + dev_err(dev, "mediatek,reset-bits count (%d) must be a multiple of 5\n", count);
> + return -EINVAL;
> + }
> +
> + nr_resets = count / 5;
> +
> + arr = devm_kcalloc(dev, count, sizeof(u32), GFP_KERNEL);
[Severity: Medium]
Since arr is only used temporarily during probe to parse the device tree
properties and is not needed afterwards, does using devm_kcalloc()
unnecessarily keep this memory allocated for the lifetime of the device?
Would a standard kcalloc() followed by a kfree() at the end of the function
be more appropriate?
> + if (!arr)
> + return -ENOMEM;
> +
> + ret = of_property_read_u32_array(np, "mediatek,reset-bits", arr, count);
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626074820.2537772-1-peter.wang@mediatek.com?part=2
^ permalink raw reply
* Re: (subset) [PATCH v4 0/4] gpio: realtek: Add support for Realtek DHC RTD1625
From: Bartosz Golaszewski @ 2026-06-26 7:59 UTC (permalink / raw)
To: linusw, brgl, robh, krzk+dt, conor+dt, afaerber, mwalle,
andriy.shevchenko, tychang, Yu-Chun Lin
Cc: Bartosz Golaszewski, linux-gpio, devicetree, linux-kernel,
linux-arm-kernel, linux-realtek-soc, cy.huang, stanley_chang,
james.tai
In-Reply-To: <20260622092335.1166876-1-eleanor.lin@realtek.com>
On Mon, 22 Jun 2026 17:23:31 +0800, Yu-Chun Lin wrote:
> This series adds GPIO support for the Realtek DHC RTD1625 SoC.
>
> Unlike the existing driver (gpio-rtd.c) which uses shared bank registers,
> the RTD1625 features a per-pin register architecture where each GPIO line
> is managed by its own dedicated 32-bit control register. This distinct
> hardware design requires a new, separate driver.
>
> [...]
Applied, thanks!
[1/4] dt-bindings: gpio: realtek: Add realtek,rtd1625-gpio
https://git.kernel.org/brgl/c/8f32808e1530b2229e07695fb39c54fee910bd4a
[2/4] gpio: Replace "default y" with "default ARCH_REALTEK" in Kconfig
https://git.kernel.org/brgl/c/b5d23fcdb12972c522e96f90ab48be8a0d971b0e
[3/4] gpio: realtek: Add driver for Realtek DHC RTD1625 SoC
https://git.kernel.org/brgl/c/a57e27c43b0315ee86c6896510d69be5257e093e
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 4/5] media: i2c: vd55g1: Add support for vd55g4
From: Jacopo Mondi @ 2026-06-26 7:58 UTC (permalink / raw)
To: Benjamin Mugnier
Cc: Jacopo Mondi, Sylvain Petinot, Sakari Ailus,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hans Verkuil, linux-media, linux-kernel, devicetree
In-Reply-To: <68d276f9-e688-4736-a296-f56c5aaa4b77@foss.st.com>
Hi Benjamin
On Thu, Jun 25, 2026 at 01:41:56PM +0200, Benjamin Mugnier wrote:
> Hi Jacopo,
>
> Le 22/06/2026 à 12:16, Jacopo Mondi a écrit :
> > Hi Benjamin
> >
> > On Tue, Apr 28, 2026 at 10:40:58AM +0200, Benjamin Mugnier wrote:
> >> vd55g4 is the same device as vd65g4 but outputs in monochrome instead of
> >> RGB. Adapt the driver structure according to this new variant, and add
> >> its support.
> >>
> >> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
> >> ---
> >> drivers/media/i2c/vd55g1.c | 110 ++++++++++++++++++++++++++++++---------------
> >> 1 file changed, 74 insertions(+), 36 deletions(-)
> >>
> >> diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
> >> index 2c962fcb41d2..9f62fc0428a1 100644
> >> --- a/drivers/media/i2c/vd55g1.c
> >> +++ b/drivers/media/i2c/vd55g1.c
> >> @@ -29,11 +29,7 @@
> >>
> >> /* Register Map */
> >> #define VD55G1_REG_MODEL_ID CCI_REG32_LE(0x0000)
> >> -#define VD55G1_MODEL_ID_VD55G1 0x53354731 /* Mono */
> >> -#define VD55G1_MODEL_ID_VD65G4 0x53354733 /* RGB */
> >> -#define VD55G1_REG_REVISION CCI_REG16_LE(0x0004)
> >> -#define VD55G1_REVISION_CCB 0x2020
> >> -#define VD55G1_REVISION_BAYER 0x3030
> >> +#define VD55G1_REG_COLOR_VERSION CCI_REG32_LE(0x0670)
> >> #define VD55G1_REG_FWPATCH_REVISION CCI_REG16_LE(0x0012)
> >> #define VD55G1_REG_FWPATCH_START_ADDR CCI_REG8(0x2000)
> >> #define VD55G1_REG_SYSTEM_FSM CCI_REG8(0x001c)
> >> @@ -138,8 +134,39 @@
> >> #define VD55G1_MIPI_RATE_MIN (250 * MEGA)
> >> #define VD55G1_MIPI_RATE_MAX (1200 * MEGA)
> >>
> >> -#define VD55G1_MODEL_ID_NAME(id) \
> >> - ((id) == VD55G1_MODEL_ID_VD55G1 ? "vd55g1" : "vd65g4")
> >> +enum vd55g1_model_id {
> >> + VD55G1_MODEL_ID_2 = 0x53354731,
> >> + VD55G1_MODEL_ID_3 = 0x53354733,
> >> +};
> >> +
> >> +enum vd55g1_color_version {
> >> + VD55G1_COLOR_VERSION_MONO = 0x0,
> >> + VD55G1_COLOR_VERSION_BAYER = 0x1,
> >
> > nit: you don't need to initialize the enum members here
>
> Thank you, will do.
>
> >
> >> +};
> >> +
> >> +struct vd55g1_version {
> >> + char *name;
> >> + enum vd55g1_model_id id;
> >> + enum vd55g1_color_version color;
> >> +};
> >> +
> >> +static const struct vd55g1_version vd55g1_versions[] = {
> >> + {
> >> + .name = "vd55g1",
> >> + .id = VD55G1_MODEL_ID_2,
> >> + .color = VD55G1_COLOR_VERSION_MONO,
> >> + },
> >> + {
> >> + .name = "vd55g4",
> >> + .id = VD55G1_MODEL_ID_3,
> >> + .color = VD55G1_COLOR_VERSION_MONO,
> >> + },
> >> + {
> >> + .name = "vd65g4",
> >> + .id = VD55G1_MODEL_ID_3,
> >> + .color = VD55G1_COLOR_VERSION_BAYER,
> >> + },
> >> +};
> >>
> >> static const u8 vd55g1_patch_array[] = {
> >> 0x44, 0x03, 0x09, 0x02, 0xe6, 0x01, 0x42, 0x00, 0xea, 0x01, 0x42, 0x00,
> >> @@ -535,7 +562,7 @@ struct vd55g1_vblank_limits {
> >>
> >> struct vd55g1 {
> >> struct device *dev;
> >> - unsigned int id;
> >> + const struct vd55g1_version *version;
> >> struct v4l2_subdev sd;
> >> struct media_pad pad;
> >> struct regulator_bulk_data supplies[ARRAY_SIZE(vd55g1_supply_name)];
> >> @@ -628,7 +655,7 @@ static u32 vd55g1_get_fmt_code(struct vd55g1 *sensor, u32 code)
> >> {
> >> unsigned int i, j;
> >>
> >> - if (sensor->id == VD55G1_MODEL_ID_VD55G1)
> >> + if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER)
> >> return code;
> >
> > As pointed out in the previous patch, you seem to have 2 mono formats.
> > Is this still ok ?
> >
> >>
> >> for (i = 0; i < ARRAY_SIZE(vd55g1_mbus_formats_bayer); i++) {
> >> @@ -1183,8 +1210,8 @@ static int vd55g1_patch(struct vd55g1 *sensor)
> >> u64 patch;
> >> int ret = 0;
> >>
> >> - /* vd55g1 needs a patch while vd65g4 does not */
> >> - if (sensor->id == VD55G1_MODEL_ID_VD55G1) {
> >> + /* Version 2 needs a patch while version 3 does not */
> >> + if (sensor->version->id == VD55G1_MODEL_ID_2) {
> >> vd55g1_write_array(sensor, VD55G1_REG_FWPATCH_START_ADDR,
> >> sizeof(vd55g1_patch_array),
> >> vd55g1_patch_array, &ret);
> >
> > You might want to consider renaming vd55g1_patch_array ?
>
> It doesn't really patch an array, it writes an array of values into
My only point here was that it has 'vd55g1' in the name.
> continuous i2c registers. Just like before converting to cci_write() I
> used to have a vd55g1_write() function to write a register, I derived a
> vd55g1_write_array() function. Here it happens to be a firmware patch,
> but it could be anything you want ;)
>
> >
> >> @@ -1256,7 +1283,7 @@ static int vd55g1_enum_mbus_code(struct v4l2_subdev *sd,
> >> struct vd55g1 *sensor = to_vd55g1(sd);
> >> u32 base_code;
> >>
> >> - if (sensor->id == VD55G1_MODEL_ID_VD55G1) {
> >> + if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER) {
> >> if (code->index >= ARRAY_SIZE(vd55g1_mbus_formats_mono))
> >> return -EINVAL;
> >> base_code = vd55g1_mbus_formats_mono[code->index];
> >> @@ -1372,7 +1399,7 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
> >> if (ret)
> >> return ret;
> >>
> >> - if (sensor->id == VD55G1_MODEL_ID_VD55G1)
> >> + if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER)
> >> code = vd55g1_mbus_formats_mono[VD55G1_MBUS_CODE_IDX_DEF];
> >> else
> >> code = vd55g1_mbus_formats_bayer[VD55G1_MBUS_CODE_IDX_DEF][0];
> >> @@ -1659,38 +1686,48 @@ static int vd55g1_init_ctrls(struct vd55g1 *sensor)
> >> return ret;
> >> }
> >>
> >> +static const struct vd55g1_version *
> >> + vd55g1_get_version(enum vd55g1_model_id id,
> >> + enum vd55g1_color_version color)
> >
> > Should you indent one tab left ?
> >
>
> checkpatch.pl is fine with both. I can indent left, it looks cleaner.
>
> >> +{
> >> + unsigned int i;
> >> +
> >> + for (i = 0; i < ARRAY_SIZE(vd55g1_versions); i++) {
> >
> > You can declare i inside the for loop
> >
>
> Yes, thank you.
>
> >> + if (vd55g1_versions[i].id == id &&
> >> + vd55g1_versions[i].color == color)
> >> + return &vd55g1_versions[i];
> >> + }
> >> +
> >> + return NULL;
> >> +}
> >> +
> >> static int vd55g1_detect(struct vd55g1 *sensor)
> >> {
> >> - unsigned int dt_id = (uintptr_t)device_get_match_data(sensor->dev);
> >> - u64 rev, id;
> >> - int ret;
> >> + const struct vd55g1_version *dt_version =
> >> + device_get_match_data(sensor->dev);
> >> + const struct vd55g1_version *version;
> >> + u64 color, id;
> >> + int ret = 0;
> >>
> >> - ret = vd55g1_read(sensor, VD55G1_REG_MODEL_ID, &id, NULL);
> >> + vd55g1_read(sensor, VD55G1_REG_MODEL_ID, &id, &ret);
> >> + vd55g1_read(sensor, VD55G1_REG_COLOR_VERSION, &color, &ret);
> >> if (ret)
> >> return ret;
> >>
> >> - if (id != VD55G1_MODEL_ID_VD55G1 && id != VD55G1_MODEL_ID_VD65G4) {
> >> - dev_warn(sensor->dev, "Unsupported sensor id 0x%x\n",
> >> - (u32)id);
> >> + version = vd55g1_get_version(id, color);
> >> + if (!version) {
> >> + dev_warn(sensor->dev, "Unsupported sensor version, expected %s\n",
> >> + dt_version->name);
> >> return -ENODEV;
> >> }
> >> - if (id != dt_id) {
> >> - dev_err(sensor->dev, "Probed sensor %s and device tree definition (%s) mismatch",
> >> - VD55G1_MODEL_ID_NAME(id), VD55G1_MODEL_ID_NAME(dt_id));
> >> + if (version->id != dt_version->id ||
> >> + version->color != dt_version->color) {
> >> + dev_err(sensor->dev, "Probed sensor version %s and device tree definition %s mismatch",
> >> + version->name, dt_version->name);
> >> return -ENODEV;
> >> }
> >> - sensor->id = id;
> >>
> >> - ret = vd55g1_read(sensor, VD55G1_REG_REVISION, &rev, NULL);
> >> - if (ret)
> >> - return ret;
> >> -
> >> - if ((id == VD55G1_MODEL_ID_VD55G1 && rev != VD55G1_REVISION_CCB) &&
> >> - (id == VD55G1_MODEL_ID_VD65G4 && rev != VD55G1_REVISION_BAYER)) {
> >> - dev_err(sensor->dev, "Unsupported sensor revision 0x%x for sensor %s\n",
> >> - (u16)rev, VD55G1_MODEL_ID_NAME(id));
> >> - return -ENODEV;
> >> - }
> >> + sensor->version = version;
> >>
> >> return 0;
> >> }
> >> @@ -2048,8 +2085,9 @@ static void vd55g1_remove(struct i2c_client *client)
> >> }
> >>
> >> static const struct of_device_id vd55g1_dt_ids[] = {
> >> - { .compatible = "st,vd55g1", .data = (void *)VD55G1_MODEL_ID_VD55G1 },
> >> - { .compatible = "st,vd65g4", .data = (void *)VD55G1_MODEL_ID_VD65G4 },
> >> + { .compatible = "st,vd55g1", .data = (void *)&vd55g1_versions[0] },
> >> + { .compatible = "st,vd55g4", .data = (void *)&vd55g1_versions[1] },
> >> + { .compatible = "st,vd65g4", .data = (void *)&vd55g1_versions[2] },
> >> { /* sentinel */ }
> >> };
> >
> > All minors
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >
> > Thanks
> > j
> >
> >> MODULE_DEVICE_TABLE(of, vd55g1_dt_ids);
> >>
> >> --
> >> 2.43.0
> >>
> >>
>
> --
> Regards,
> Benjamin
>
^ permalink raw reply
* Re: [PATCH v5 1/7] dt-bindings: display: verisilicon,dc: generalize for single-output variants
From: Icenowy Zheng @ 2026-06-26 7:58 UTC (permalink / raw)
To: Conor Dooley, Conor Dooley
Cc: Joey Lu, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, ychuang3, schung, yclu4, dri-devel,
devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20260626-zit-amuck-e743e58d2e15@wendy>
在 2026-06-26五的 08:22 +0100，Conor Dooley写道：
> On Thu, Jun 25, 2026 at 05:33:37PM +0100, Conor Dooley wrote:
> > On Thu, Jun 25, 2026 at 05:44:43PM +0800, Joey Lu wrote:
> > > The verisilicon,dc binding was originally written for the T-Head
> > > TH1520
> > > SoC carrying a DC8200, and hard-codes five clocks, three resets
> > > and two
> > > output ports.
> > >
> > > Add the Nuvoton MA35D1 DCUltraLite (nuvoton,ma35d1-dcu) to the
> > > binding.
> > > The DCUltraLite uses only two clocks (core, pix0) and one reset
> > > (core),
> > > with a single output port.
> > >
> > > Use allOf/if blocks to express per-variant constraints rather
> > > than
> > > hard-coding the DC8200 topology at the top level. Each
> > > compatible's
> > > block constrains the clock and reset item counts; the nuvoton
> > > block
> > > additionally overrides clock-names to the two names it actually
> > > uses.
> > >
> > > Signed-off-by: Joey Lu <a0987203069@gmail.com>
> > > ---
> > > .../bindings/display/verisilicon,dc.yaml | 57
> > > +++++++++++++++++++
> > > 1 file changed, 57 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> > > b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> > > index 9dc35ab973f2..1e751f3c7ce8 100644
> > > ---
> > > a/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> > > +++
> > > b/Documentation/devicetree/bindings/display/verisilicon,dc.yaml
> > > @@ -17,6 +17,7 @@ properties:
> > > items:
> > > - enum:
> > > - thead,th1520-dc8200
> > > + - nuvoton,ma35d1-dcu
> > > - const: verisilicon,dc # DC IPs have discoverable
> > > ID/revision registers
> > >
> > > reg:
> > > @@ -77,6 +78,62 @@ required:
> > > - clock-names
> > > - ports
> > >
> > > +allOf:
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + const: thead,th1520-dc8200
> > > + then:
> > > + properties:
> > > + clocks:
> > > + minItems: 5
> > > + maxItems: 5
> > > +
> > > + clock-names:
> > > + minItems: 5
> > > + maxItems: 5
> >
> > All the maxItems here repeat the maximum constraint and do nothing.
> >
> > Since you didn't change the minimum constraint at the top level,
> > your
> > minItems also do nothing.
> >
> > > +
> > > + resets:
> > > + minItems: 3
> > > + maxItems: 3
> > > +
> > > + reset-names:
> > > + minItems: 3
> > > + maxItems: 3
> > > +
> > > + required:
> > > + - resets
> > > + - reset-names
> >
> > Both conditional sections have this, but the original binding
> > doesn't
> > require these for the thead device. This is a functional change
> > therefore and shouldn't be in a patch calling itself "generalise
> > for
> > single ended variants".
> >
> > FWIW, adding your new compatible shouldn't really be in a patch
> > with
> > that subject either, it really should say "add support for nuvoton
> > ma35d1" or something.
> >
> > > +
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + const: nuvoton,ma35d1-dcu
> > > + then:
> > > + properties:
> > > + clocks:
> > > + minItems: 2
> >
> > Anything that updates the minimum constraint should be done at the
> > top
> > level of this schema. The conditional section should then tighten
> > the
> > constraint, in this case that means only having maxItems.
> >
> > > + maxItems: 2
> > > +
> > > + clock-names:
> > > + items:
> > > + - const: core
> > > + - const: pix0
> >
> > Does this even work when the top level schema thinks clock 2 should
> > be
> > called axi?
>
> Additionally here, only have core and pix0 seems like it might be an
> oversimplification. I doubt removing the second output port means
> that
> the axi and ahb clocks are no longer needed.
> Is it the case that your device supplies the same clock to core, ahb
> and
> axi? If so, then you should fill those clocks in in your devicetree
> and
> this can just constrain the number of clocks/clock-names to 4.
The clock controller of that SoC is quite weird -- it has only a single
gate bit, but controlling 3 clock gates. All core, ahb and axi clocks
have gates controlled by this single bit, so it's why currently it's
modelled as only core clock supplied.
Well it might be worthful to supply the bus clock before the gate as
ahb/axi, especially axi, because both the AXI clock and the core clock
constraints the maximum pixel clock.
Thanks,
Icenowy
>
> >
> > > +
> > > + resets:
> > > + minItems: 1
> > > + maxItems: 1
> > > +
> > > + reset-names:
> > > + items:
> > > + - const: core
> >
> > This is just maxItems: 1.
> >
> > pw-bot: changes-requested
> >
> > Thanks,
> > Conor.
> >
> > > +
> > > + required:
> > > + - resets
> > > + - reset-names
> > > +
> > > additionalProperties: false
> > >
> > > examples:
> > > --
> > > 2.43.0
> > >
>
^ permalink raw reply
* Re: [PATCH 1/5] media: i2c: vd55g1: Fix media bus code initialization
From: Jacopo Mondi @ 2026-06-26 7:55 UTC (permalink / raw)
To: Benjamin Mugnier
Cc: Jacopo Mondi, Sylvain Petinot, Sakari Ailus,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hans Verkuil, linux-media, linux-kernel, devicetree
In-Reply-To: <f51d5d21-54fb-444a-9ae9-9b60e7a500f1@foss.st.com>
Hi Benjamin
On Thu, Jun 25, 2026 at 01:41:48PM +0200, Benjamin Mugnier wrote:
> Hi Jacopo,
>
> Thank you for your review.
>
> Le 22/06/2026 à 11:28, Jacopo Mondi a écrit :
> > Hi Benjamin
> >
> > On Tue, Apr 28, 2026 at 10:40:55AM +0200, Benjamin Mugnier wrote:
> >> In the driver initialization, the index of the default media bus code
> >> from the supported media bus code array is passed directly to the
> >> vd55g1_get_fmt_code() function instead of the proper media bus code.
> >>
> >> This works correctly as a proper media bus code is set after
> >> initialization but could not have been the case. This also resulted in
> >> mutliple "Unsupported mbus format" error messages.
> >>
> >> Retrieve the media bus code from the media bus code array, and pass this
> >> media bus code to vd55g1_get_fmt_code() instead of the code index.
> >>
> >> Rename VD55G1_MBUS_CODE_DEF to VD55G1_MBUS_CODE_IDX_DEF and
> >> VD55G1_MODE_DEF to VD55G1_MODE_IDX_DEF while at it to avoid future
> >> confusions. Display the guilty error code in warning message.
> >>
> >> Fixes: e138e7f00042 ("media: i2c: vd55g1: Add support for vd65g4 RGB variant")
> >>
> > You should cc stable for fixes
> >
> > Cc: stable@vger.kernel.org
> >
>
> We talked about this very recently and somehow I still forgot.
>
> >
> > The CI should have flagged that, but for some reason it didn't run
> > properly on your series
> > https://gitlab.freedesktop.org/linux-media/users/patchwork/-/pipelines/1655147
> >
> >> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
> >> ---
> >> drivers/media/i2c/vd55g1.c | 17 +++++++++++------
> >> 1 file changed, 11 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
> >> index 78d18c028154..1e9db21322e3 100644
> >> --- a/drivers/media/i2c/vd55g1.c
> >> +++ b/drivers/media/i2c/vd55g1.c
> >> @@ -114,9 +114,9 @@
> >>
> >> #define VD55G1_WIDTH 804
> >> #define VD55G1_HEIGHT 704
> >> -#define VD55G1_MODE_DEF 0
> >> +#define VD55G1_MODE_IDX_DEF 0
> >> #define VD55G1_NB_GPIOS 4
> >> -#define VD55G1_MBUS_CODE_DEF 0
> >> +#define VD55G1_MBUS_CODE_IDX_DEF 0
> >> #define VD55G1_DGAIN_DEF 256
> >> #define VD55G1_AGAIN_DEF 19
> >> #define VD55G1_EXPO_MAX_TERM 64
> >> @@ -634,7 +634,7 @@ static u32 vd55g1_get_fmt_code(struct vd55g1 *sensor, u32 code)
> >
> > Unrelated, but it seems you now have 2 codes for MONO. Does
> >
> > if (sensor->id == VD55G1_MODEL_ID_VD55G1)
> > return code;
> >
> > need an update ?>
>
> Not in this patch because it does not add the new MONO sensor, but in
Not in this patch ofc
> 4/5 I separated the model ID from the color code. Example for the vd55g4 :
>
> .name = "vd55g4",
> .id = VD55G1_MODEL_ID_3,
> .color = VD55G1_COLOR_VERSION_MONO,
>
> So the patch 4/5 updates the previous 'if' you mentioned to check the
> color member instead of the model :
>
> if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER)
>
> Which IMO is a good way to handle this problematic. Tell me if you're
> thinking about something else.
>
Yes, I was thinking about the unconditional return of 'code'.
You know have two mono codes, shouldn't you do the same as you do with
the bayer ones ?
> >> goto adapt_bayer_pattern;
> >> }
> >> }
> >> - dev_warn(sensor->dev, "Unsupported mbus format\n");
> >> + dev_warn(sensor->dev, "Unsupported mbus format: 0x%x\n", code);
> >>
> >> return code;
> >>
> >> @@ -1347,6 +1347,7 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
> >> {
> >> struct vd55g1 *sensor = to_vd55g1(sd);
> >> struct v4l2_subdev_format fmt = { 0 };
> >> + int code;
> >> struct v4l2_subdev_route routes[] = {
> >> { .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE }
> >> };
> >> @@ -1361,9 +1362,13 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
> >> if (ret)
> >> return ret;
> >>
> >> - vd55g1_update_pad_fmt(sensor, &vd55g1_supported_modes[VD55G1_MODE_DEF],
> >> - vd55g1_get_fmt_code(sensor, VD55G1_MBUS_CODE_DEF),
> >> - &fmt.format);
> >> + if (sensor->id == VD55G1_MODEL_ID_VD55G1)
> >> + code = vd55g1_mbus_formats_mono[VD55G1_MBUS_CODE_IDX_DEF];
> >> + else
> >> + code = vd55g1_mbus_formats_bayer[VD55G1_MBUS_CODE_IDX_DEF][0];
> >
> > Being this a multi-dimensional array, I don't seem much value in
> > defining VD55G1_MBUS_CODE_IDX_DEF if this is the only place where it
> > is used. What's the meaning of VD55G1_MBUS_CODE_IDX_DEF for
> > vd55g1_mbus_formats_bayer ? Does it represent the bitwidth or does it
> > represent the bayer pattern ?
>
> For vd55g1_mbus_formats_bayer, the first dimension of the array is the
> bitwidth, and the second one is the bayer pattern.
>
> >
> > I would rather define a
> > VD55G1_DEF_MBUS_CODE_MONO MEDIA_BUS_FMT_Y8_1X8
> > VD55G1_DEF_MBUS_CODE_BAYER MEDIA_BUS_FMT_SRGGB8_1X8
> >
> > Or maybe do
> >
> > code = vd55g1_mbus_formats_bayer[VD55G1_MBUS_CODE_IDX_DEF]
> > [VD55G1_MBUS_CODE_IDX_DEF];
> >
> > if easier.
> >
> > I understand it's a minor, so up to you.
>
> As you mentioned it's only used here. I won't mind removing
> VD55G1_MBUS_CODE_IDX_DEF entirely and do :
>
> code = vd55g1_mbus_formats_bayer[0][0];
>
> Does that sound okay ?
>
It does, thanks
> >
> >
> >
> >> + vd55g1_update_pad_fmt(sensor,
> >> + &vd55g1_supported_modes[VD55G1_MODE_IDX_DEF],
> >> + vd55g1_get_fmt_code(sensor, code), &fmt.format);
> >>
> >> return vd55g1_set_pad_fmt(sd, sd_state, &fmt);
> >> }
> >>
> >> --
> >> 2.43.0
> >>
> >>
>
> --
> Regards,
> Benjamin
>
^ permalink raw reply
* Re: [PATCH v1 1/2] Documentation: dt: reset: add mediatek,syscon-reset binding
From: sashiko-bot @ 2026-06-26 7:53 UTC (permalink / raw)
To: peter.wang; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260626074820.2537772-2-peter.wang@mediatek.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The YAML binding schema is missing the mandatory `$id` field.
--
commit 010ce9609c338d8282471debb7c3cd606956e201
Author: Peter Wang <peter.wang@mediatek.com>
Documentation: dt: reset: add mediatek,syscon-reset binding
This commit adds Device Tree binding documentation for the MediaTek SYSCON
reset controller. The binding describes a reset controller that is part of
a MediaTek SYSCON MFD block and manages multiple reset lines via the
mediatek,reset-bits property.
> diff --git a/Documentation/devicetree/bindings/reset/mediatek,syscon-reset.yaml b/Documentation/devicetree/bindings/reset/mediatek,syscon-reset.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/mediatek,syscon-reset.yaml
> @@ -0,0 +1,93 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
[Severity: Medium]
Is the mandatory $id field missing from this schema definition?
Without the $id field, the schema cross-reference system ($ref resolution)
can break, and dt_binding_check might silently skip validation or report
misleading errors. Should it be added to the top-level schema header?
> +
> +title: MediaTek SYSCON Reset Controller
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626074820.2537772-1-peter.wang@mediatek.com?part=1
^ permalink raw reply
* Re: [PATCH v2 9/9] hwmon: adm1275: Support ROHM BD12790
From: Matti Vaittinen @ 2026-06-26 7:49 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-hwmon, robh, conor+dt, devicetree
In-Reply-To: <20260626073347.A06981F000E9@smtp.kernel.org>
On 26/06/2026 10:33, sashiko-bot@kernel.org wrote:
> If we evaluate m = round(k / 10^(-3-R)) for a value of R=-2 and k=35119.94,
> it yields 351199.4. However, the actual code below correctly implements the
> value as 3512, which implies the formula should use multiplication rather
> than division.
Gah. I think Sashiko is right. The fixed comment should say:
m = round(k * 10^(-3-R))
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH v6 2/2] drm/bridge: Add Lontium LT9611C(EX/UXD) MIPI DSI to HDMI driver
From: Maxime Ripard @ 2026-06-26 7:49 UTC (permalink / raw)
To: Sunyun Yang
Cc: Krzysztof Kozlowski, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, dmitry.baryshkov, maarten.lankhorst, rfoss,
Laurent.pinchart, tzimmermann, jonas, jernej.skrabec, devicetree,
dri-devel, linux-kernel, xmzhu, xmzhu, rlyu, xbpeng
In-Reply-To: <CAFQXuNZtzBu+WiG8n0BeN47zagQmL-iz_6Af7prk-xHLAeRBwg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3937 bytes --]
On Fri, Jun 26, 2026 at 10:15:03AM +0800, Sunyun Yang wrote:
> Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 21:51写道：
> >
> > On 25/06/2026 15:40, Sunyun Yang wrote:
> > > Sunyun Yang <syyang@lontium.com> 于2026年6月25日周四 21:26写道：
> > >>
> > >> Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 21:17写道：
> > >>>
> > >>> On 25/06/2026 15:14, Sunyun Yang wrote:
> > >>>> Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 20:54写道：
> > >>>>>
> > >>>>> On 08/05/2026 15:40, syyang@lontium.com wrote:
> > >>>>>> +
> > >>>>>> +static void lt9611c_reset(struct lt9611c *lt9611c)
> > >>>>>> +{
> > >>>>>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 1);
> > >>>>>> + msleep(20);
> > >>>>>> +
> > >>>>>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 0);
> > >>>>>> + msleep(20);
> > >>>>>> +
> > >>>>>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 1);
> > >>>>>
> > >>>>> This is just plain wrong. Why do you assert, then de-assert and then
> > >>>>> finally assert AGAIN the reset leaving the device in powerdown stage?
> > >>>>>
> > >>>> I am using software to emulate the hardware RESET button on our EVB.
> > >>>> When the hardware RESET button is pressed while our chip is running,
> > >>>> the signal level changes from HIGH to LOW and then back to HIGH.
> > >>>>
> > >>>> Of course, we can also use the following:
> > >>>> static void lt9611c_reset(struct lt9611c *lt9611c)
> > >>>> {
> > >>>> gpiod_set_value_cansleep(lt9611c->reset_gpio, 0);
> > >>>> msleep(50);
> > >>>> gpiod_set_value_cansleep(lt9611c->reset_gpio, 1);
> > >>>> msleep(20);
> > >>>> }
> > >>>
> > >>> Makes no sense either and you just did not get the point and did not
> > >>> answer my question. I asked WHY you leave asserted. Answer "we emulate"
> > >>> is just plain wrong.
> > >>>
> > >>> So again please answer:
> > >>>
> > >>> Why do you leave device with reset asserted?
> > >>>
> > >>
> > >> devicetree: reset-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>;
> > >>
> > >> GPIO_ACTIVE_HIGH:
> > >>
> > >> gpiod_set_value_cansleep(lt9611c->reset_gpio, 0); ------ reset pin
> > >> is Low level : Clear the register configuration in the chip to stop
> > >> the chip from working.
> > >>
> > >> gpiod_set_value_cansleep(lt9611c->reset_gpio, 1); ------ reset pin
> > >> is high level: The chip resumes operation.
> > >>
> > >>
> > >
> > > Our purpose is: pull the level low to clear the register configuration
> > > in the chip, and then pull it high to allow the MCU inside the chip to
> > > re‑initialize the registers.
> >
> >
> > And you do completely opposite... so that confirms your code is just wrong.
> >
>
> The lontium-lt9611.yaml uses GPIO_ACTIVE_HIGH. I am just following the
> rule of this device tree. If I modify the device tree to use
> GPIO_ACTIVE_LOW,
> and use the following code in my driver, then my driver would be correct.
> However, would the existing kernel drivers lontium-lt9611uxc.c and
> lontium-lt9611.c be affected?
It might, but then it's a DT problem. The GPIO API for drivers always
considers the logical state of a GPIO, so if you need to assert a
signal, you'll always need to set 1. That's what Krzysztof was trying to
explain.
The DT will provide with GPIO_ACTIVE_* how that logical state translates
to a physical GPIO state.
If the DT says that this particular GPIO is active-high, then it means
that we need to set the GPIO to 1 to assert reset. Now of course, it
might not make sense for the controller itself, but it might for the
board if there's a GPIO inverter in the middle for example.
Anyway, in the case you're raising, the issue definitely lies in the DT,
and that's what would need to be fixed.
I also wouldn't be too concerned about lontium-lt9611.yaml, it's just an
example.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply
* Re: [PATCH v6 2/2] Input: isa1200 - new driver for Imagis ISA1200
From: Svyatoslav Ryhel @ 2026-06-26 7:45 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Linus Walleij,
linux-input, devicetree, linux-kernel
In-Reply-To: <aj28fvj34b4_VI3k@google.com>
пт, 26 черв. 2026 р. о 02:45 Dmitry Torokhov <dmitry.torokhov@gmail.com> пише:
>
> Hi Svyatoslav,
>
> On Wed, Jun 17, 2026 at 10:05:27AM +0300, Svyatoslav Ryhel wrote:
> > From: Linus Walleij <linusw@kernel.org>
> >
> > The ISA1200 is a haptic feedback unit from Imagis Technology using two
> > motors for haptic feedback in mobile phones. Used in many mobile devices
> > c. 2012 including Samsung Galxy S Advance GT-I9070 (Janice), Samsung Beam
> > GT-I8350 (Gavini), LG Optimus 4X P880 and LG Optimus Vu P895.
> >
> > The exact datasheet for the ISA1200 is not available; all data was modeled
> > based on available downstream kernel sources for various devices and
> > fragments of information scattered across the internet.
> >
> > Tested-by: Linus Walleij <linusw@kernel.org> # GT-I9070 Janice
> > Signed-off-by: Linus Walleij <linusw@kernel.org>
> > Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
>
>
> Sashiko convinced me that using mutex_trylock() in the playback work
> handler will result in dropping requests and that we are safe not taking
> the lock there at all. Can you please try the following modification?
>
> diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
> index c61adc4b605c..fb7f68fa0a2b 100644
> --- a/drivers/input/misc/isa1200.c
> +++ b/drivers/input/misc/isa1200.c
> @@ -253,15 +253,12 @@ static void isa1200_stop(struct isa1200 *isa)
> static void isa1200_play_work(struct work_struct *work)
> {
> struct isa1200 *isa = container_of(work, struct isa1200, play_work);
> - struct input_dev *input = isa->input;
> -
> - scoped_guard(mutex_try, &input->mutex) {
> - if (!isa->suspended) {
> - if (isa->level)
> - isa1200_start(isa);
> - else
> - isa1200_stop(isa);
> - }
> +
> + if (!READ_ONCE(isa->suspended)) {
> + if (isa->level)
> + isa1200_start(isa);
> + else
> + isa1200_stop(isa);
> }
> }
>
>
> If this works no need to resend, I'll fold on my side.
>
It does work fine. Thank you.
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH 4/4] arm64: dts: broadcom: bcm2712: Add reboot modes to firmware node
From: sashiko-bot @ 2026-06-26 7:44 UTC (permalink / raw)
To: Gregor Herburger; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260626-rpi-tryboot-v1-4-490b1c4c4970@linutronix.de>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Adding `mode-*` properties activates a dormant Use-After-Free and Double-Free vulnerability in the driver's unbind path.
--
commit cd92f6ce5b48afd036127289b2412bed6be015fc
Author: Gregor Herburger <gregor.herburger@linutronix.de>
arm64: dts: broadcom: bcm2712: Add reboot modes to firmware node
The raspberry pi firmware driver allows the tryboot reboot mode. Add
this mode and normal boot mode to the node.
> diff --git a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-base.dtsi b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-base.dtsi
> index b7a6bc34ae1ab..67095c7ff770d 100644
> --- a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-base.dtsi
> +++ b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-base.dtsi
> @@ -49,6 +49,8 @@ firmware: rpi-firmware {
> compatible = "raspberrypi,bcm2835-firmware", "simple-mfd";
>
> mboxes = <&mailbox>;
> + mode-normal = <0>;
> + mode-tryboot = <1>;
[Severity: Critical]
This is a pre-existing issue, but does adding these properties to the
device tree activate a dormant use-after-free and double-free vulnerability
in the driver's unbind path?
Looking at drivers/firmware/raspberrypi.c, the driver registers the reboot
mode using the devm variant during probe, but also manually unregisters it
during remove:
drivers/firmware/raspberrypi.c:rpi_firmware_remove() {
...
reboot_mode_unregister(&fw->reboot_mode);
rpi_firmware_put(fw);
...
}
The call to rpi_firmware_put() drops the reference and frees the fw object.
Because devm_reboot_mode_register() was used during probe, the devres cleanup
will eventually run devm_reboot_mode_release() which calls
reboot_mode_unregister() a second time. This passes the now-freed
fw->reboot_mode pointer.
Furthermore, the list strings are freed during the first unregister call:
drivers/power/reset/reboot-mode.c:reboot_mode_unregister() {
...
list_for_each_entry(info, &reboot->head, list)
kfree_const(info->mode);
...
}
Because the list pointers are not cleared and the list elements are preserved,
the second unregister call will iterate over the list again. Since this patch
populates that list by adding mode-normal and mode-tryboot, the second call
will try to free the already-freed info->mode pointers.
Should the manual reboot_mode_unregister() call be removed from
rpi_firmware_remove() before adding these device tree properties?
>
> firmware_clocks: clocks {
> compatible = "raspberrypi,firmware-clocks";
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rpi-tryboot-v1-0-490b1c4c4970@linutronix.de?part=4
^ permalink raw reply
* Re: [PATCH 3/4] firmware: raspberrypi: Add reboot mode support
From: sashiko-bot @ 2026-06-26 7:43 UTC (permalink / raw)
To: Gregor Herburger; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260626-rpi-tryboot-v1-3-490b1c4c4970@linutronix.de>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Mixing managed (devm) and manual API calls for reboot mode registration causes a double-free and a use-after-free on device removal.
- [High] Ignoring the return value of `devm_reboot_mode_register` leads to a NULL pointer dereference or double free if registration fails.
--
commit c4eb702fd9dc0d615bc8a99fbb83befc50f8971f
Author: Gregor Herburger <gregor.herburger@linutronix.de>
firmware: raspberrypi: Add reboot mode support
The Raspberry Pi firmware has a tryboot mode where it tries to boot from
another partition. This patch adds support for this by registering a reboot
mode driver.
> diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> --- a/drivers/firmware/raspberrypi.c
> +++ b/drivers/firmware/raspberrypi.c
> @@ -306,6 +323,12 @@ static int rpi_firmware_probe(struct platform_device *pdev)
> rpi_register_hwmon_driver(dev, fw);
> rpi_register_clk_driver(dev);
>
> + fw->reboot_mode.dev = dev;
> + fw->reboot_mode.write = rpi_firmware_reboot_mode_write;
> + ret = devm_reboot_mode_register(dev, &fw->reboot_mode);
> + if (ret)
> + dev_err(dev, "Failed to register reboot mode: %d\n", ret);
> +
> return 0;
> }
[Severity: High]
Should we return the error code here instead of returning 0?
If devm_reboot_mode_register() fails, the probe succeeds but the internal
list head for the reboot mode might remain uninitialized.
Later during device removal, rpi_firmware_remove() calls
reboot_mode_unregister() unconditionally. This could lead to a null pointer
dereference when iterating the uninitialized list, or a double free if the
failure occurred inside the register function and it already freed the
resources.
[ ... ]
> @@ -323,6 +346,8 @@ static void rpi_firmware_remove(struct platform_device *pdev)
> {
> struct rpi_firmware *fw = platform_get_drvdata(pdev);
>
> + reboot_mode_unregister(&fw->reboot_mode);
> +
[Severity: High]
Could this manual unregister cause a double-free and use-after-free?
Since devm_reboot_mode_register() is used in probe, the devres cleanup will
automatically unregister the reboot mode when the device is removed.
By manually calling reboot_mode_unregister() here:
1. It frees the strings in the list.
2. rpi_firmware_put(fw) drops the reference, potentially freeing the fw
structure.
3. The devres cleanup runs devm_reboot_mode_release() upon returning,
accessing the freed fw structure and attempting to unregister and free
the strings a second time.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rpi-tryboot-v1-0-490b1c4c4970@linutronix.de?part=3
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: raspberrypi,bcm2835-firmware: Include 'reboot-mode.yaml'
From: sashiko-bot @ 2026-06-26 7:43 UTC (permalink / raw)
To: Gregor Herburger; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260626-rpi-tryboot-v1-2-490b1c4c4970@linutronix.de>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing Device Tree Schema Restriction for reboot mode pattern properties
--
commit d367af481cae3dea3dd1bb270d9e4466fb383031
Author: Gregor Herburger <gregor.herburger@linutronix.de>
dt-bindings: raspberrypi,bcm2835-firmware: Include 'reboot-mode.yaml'
This commit updates the Raspberry Pi firmware binding to reference the
reboot-mode schema. This allows reboot mode properties to be used to set
reboot modes like tryboot.
> diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> index 983ea80eaec97..30b490e0d9fb3 100644
> --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> @@ -133,11 +133,14 @@ properties:
> required:
> - compatible
>
> +allOf:
> + - $ref: /schemas/power/reset/reboot-mode.yaml#
[Severity: Low]
Should this schema restrict the reboot mode pattern properties to a single
item?
The generic reboot-mode.yaml schema defines ^mode-.*$ as a uint32-array
without a length restriction. Because this binding doesn't override it with
maxItems: 1, the device tree schema will accept arrays of arbitrary length,
for example:
mode-tryboot = <1 2>;
Since the reboot-mode.c driver only evaluates a single 32-bit value using
of_property_read_u32(), this could lead to silent truncation of multi-cell
properties without any validation errors. Does it make sense to add a
patternProperties block here to restrict maxItems to 1?
> +
> required:
> - compatible
> - mboxes
>
> -additionalProperties: false
> +unevaluatedProperties: false
>
> examples:
> - |
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rpi-tryboot-v1-0-490b1c4c4970@linutronix.de?part=2
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-oneplus: Update compatible to include model
From: Krzysztof Kozlowski @ 2026-06-26 7:42 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: David Heidelberg, Krzysztof Kozlowski, Konrad Dybcio, Rob Herring,
Conor Dooley, Jason A. Donenfeld, Matthias Schiffer,
Vincent Huang, Bjorn Andersson, Konrad Dybcio, linux-input,
devicetree, linux-kernel, linux-arm-msm, phone-devel
In-Reply-To: <aj1OhZQjO5nNYlAo@google.com>
On 25/06/2026 18:57, Dmitry Torokhov wrote:
> Hi Krzysztof,
>
> On Thu, Jun 25, 2026 at 10:23:54AM +0200, Krzysztof Kozlowski wrote:
>> On 25/06/2026 06:53, Dmitry Torokhov wrote:
>>> On Wed, Jun 24, 2026 at 04:37:25PM +0200, David Heidelberg wrote:
>>>> On 24/06/2026 06:28, Dmitry Torokhov wrote:
>>>>> Hi David,
>>>>>
>>>>> On Sun, Jun 21, 2026 at 07:11:45PM +0200, David Heidelberg wrote:
>>>>>> On 28/05/2026 00:13, David Heidelberg wrote:
>>>>>>> On 27/05/2026 23:56, Dmitry Torokhov wrote:
>>>>>>>> Hi David,
>>>>>>>>
>>>>>>>> On Sat, May 23, 2026 at 11:45:35AM +0200, David Heidelberg via B4 Relay wrote:
>>>>>>>>> From: David Heidelberg <david@ixit.cz>
>>>>>>>>>
>>>>>>>>> We know the driver is reporting s3706b, introduce the compatible so we
>>>>>>>>> can more easily introduce quirks for weird touchscreen replacements in
>>>>>>>>> followup series.
>>>>>>>>>
>>>>>>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>>>>> Signed-off-by: David Heidelberg <david@ixit.cz>
>>>>>>>>> ---
>>>>>>>>> arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi | 2 +-
>>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>>>>>> b/arch/ arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>>>>>> index 6b7378cf4d493..148164d456a5a 100644
>>>>>>>>> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>>>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>>>>>> @@ -475,17 +475,17 @@ bq27441_fg: bq27441-battery@55 {
>>>>>>>>> };
>>>>>>>>> };
>>>>>>>>> &i2c12 {
>>>>>>>>> status = "okay";
>>>>>>>>> clock-frequency = <400000>;
>>>>>>>>> synaptics-rmi4-i2c@20 {
>>>>>>>>> - compatible = "syna,rmi4-i2c";
>>>>>>>>> + compatible = "syna,rmi4-s3706b", "syna,rmi4-i2c";
>>>>>>>>
>>>>>>>> So I believe we established that this device (s3706b) does not in fact
>>>>>>>> implement rmi4 protocol properly. Why do we have "syna,rmi4-i2c" as a
>>>>>>>> fallback? Shouldn't it be just "syna,rmi4-s3706b"?
>>>>>>>
>>>>>>> The vendor supplies s3706b which does implement the RMI4 properly.
>>>>>>>
>>>>>>> The 3rd party replacement impersonating original parts may not implement
>>>>>>> it properly, but I don't address this issue in this initial submission.
>>>>>>>
>>>>>>> With this compatible we know which original part is used by the vendor
>>>>>>> and installed in the phones, so later we can deduct specific sequences
>>>>>>> for the replacement aftermarket parts to keep phone touchscreen working
>>>>>>> same as they do on Android without affecting other devices.
>>>>>>
>>>>>> Hello Dmitry.
>>>>>>
>>>>>> May I ask what is currently preventing this series from moving forward?
>>>>>>
>>>>>> The first version was posted in 2023 [1]. I picked it up again in 2025 [2]
>>>>>> and am now on the 9th iteration (this patchset). At this point, the series
>>>>>> has been under discussion for well over a year, with relatively little
>>>>>> feedback and increasingly long gaps between review rounds.
>>>>>>
>>>>>> The current approach is based on the guidance I have received so far,
>>>>>> including suggestions from the device-tree maintainers. When concerns were
>>>>>> raised, I tried to address them and rework the series accordingly.
>>>>>>
>>>>>> What I am struggling with is understanding what specific issue still needs
>>>>>> to be resolved before these patches can be accepted. If there are remaining
>>>>>> requirements, objections to the approach, or technical concerns that I have
>>>>>> not addressed, I would appreciate having them stated explicitly so I can
>>>>>> work on them.
>>>>>>
>>>>>> I also split out the straightforward, self-contained changes in the hope
>>>>>> that at least those could progress independently while I continued working
>>>>>> on any follow-up requirements. However, even those patches do not appear to
>>>>>> be moving forward.
>>>>>>
>>>>>> Could you please clarify what outcome you would like to see from this
>>>>>> series, and what concrete changes would be required to get it accepted?
>>>>>
>>>>> I am still confused about how you want to differentiate between the full
>>>>> RMI4 support vs the OnePlus flavor. The "syna,rmi4-s3706b", as you
>>>>> mentioned, implements RMI4 protocol properly, so we do not need to
>>>>> actually have it documented neither in binding nor in DTS.
>>>>
>>>> --- part 1 ---
>>>>
>>>> This series addresses identification within device-tree. It's normal
>>>> recommended practice.
>>>>
>>>> If we know, the device ships specific, but **compliant** variant, we just
>>>> put it as compatible = "more-specific", "less-specific"; in this case
>>>> "syna,rmi4-s3706b", "syna,rmi4-i2c"
>>>>
>>>> This approach is used everywhere. This has nothing to do with after-market parts.
>>>
>>> We do this in many cases, sometimes when a part has different timings or
>>> maybe additional functionality compared to the base model.
>>
>> Generic expectation is to have always dedicated front compatible for
>> every device. rmi4-i2c is not really specific enough, more like a
>> family, thus a specific device compatible is essential by the DT rules.
>
> Essential in what way? What will break if such compatible is not there?
Essential by the rules we defined for DT and documented in writing-bindings.
> We have lived without it for many years and will continue live happily
> without it for years to come.
Does not matter if compatible is used or not. The rules are dictating that.
Best regards,
Krzysztof
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox