* Re: [PATCH v2] dt-bindings: pinctrl: qcom,pmic-gpio: Add Qualcomm PMK7750
From: Krzysztof Kozlowski @ 2026-06-11 12:24 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260611111232.EF6541F00893@smtp.kernel.org>
On 11/06/2026 13:12, sashiko-bot@kernel.org wrote:
>>
>> properties:
>> compatible:
>> - items:
>> - - enum:
> [ ... ]
>> + oneOf:
>> + - items:
>> + - enum:
> [ ... ]
>> + - qcom,pmiv0104-gpio
>> + - qcom,pmk8350-gpio
>> + - qcom,pmk7750-gpio
>
> [Severity: Medium]
> Since qcom,pmk7750-gpio is added here to the standalone enum list, won't
> standalone usage bypass the generation-specific constraints (such as the
> gpio-line-names limit) defined in the allOf conditional block for the
> PMK8550 generation?
I should have removed pmk7750 here :(
Oh, I hate these YAML block re-indentations.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/2] clk: amlogic: Add A9 AO clock controller driver
From: Jian Hu @ 2026-06-11 12:24 UTC (permalink / raw)
To: Jerome Brunet
Cc: Jian Hu via B4 Relay, Neil Armstrong, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Xianwei Zhao, Kevin Hilman, Martin Blumenstingl, linux-amlogic,
linux-clk, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <1jpl1yfunu.fsf@starbuckisacylon.baylibre.com>
On 6/10/2026 8:26 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On mer. 10 juin 2026 at 12:18, Jian Hu <jian.hu@amlogic.com> wrote:
>
>> Hi Jerome,
>>
>> Thanks for your review
>>
>> On 6/3/2026 10:29 PM, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> On Wed 03 Jun 2026 at 20:17, Jian Hu via B4 Relay <devnull+jian.hu.amlogic.com@kernel.org> wrote:
>>>
>>>> From: Jian Hu <jian.hu@amlogic.com>
>>>>
>>>> Add the Always-on clock controller driver for the Amlogic A9 SoC family.
>>>>
>>>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>>>> ---
>>>> drivers/clk/meson/Kconfig | 13 ++
>>>> drivers/clk/meson/Makefile | 1 +
>>>> drivers/clk/meson/a9-aoclk.c | 419 +++++++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 433 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>>> index cf8cf3f9e4ee..625e6788b940 100644
>>>> --- a/drivers/clk/meson/Kconfig
>>>> +++ b/drivers/clk/meson/Kconfig
>>>> @@ -132,6 +132,19 @@ config COMMON_CLK_A1_PERIPHERALS
>>>> device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>>>> controller to work.
>>>>
>>>> +config COMMON_CLK_A9_AO
>>>> + tristate "Amlogic A9 SoC AO clock controller support"
>>>> + depends on ARM64
>>>> + default ARCH_MESON || COMPILE_TEST
>>>> + select COMMON_CLK_MESON_REGMAP
>>>> + select COMMON_CLK_MESON_CLKC_UTILS
>>>> + select COMMON_CLK_MESON_DUALDIV
>>>> + imply COMMON_CLK_SCMI
>>>> + help
>>>> + Support for the AO clock controller on Amlogic A311Y3 based
>>>> + device, AKA A9.
>>>> + Say Y if you want A9 AO clock controller to work.
>>>> +
>>>> config COMMON_CLK_C3_PLL
>>>> tristate "Amlogic C3 PLL clock controller"
>>>> depends on ARM64
>>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>>> index c6719694a242..f89d027c282c 100644
>>>> --- a/drivers/clk/meson/Makefile
>>>> +++ b/drivers/clk/meson/Makefile
>>>> @@ -19,6 +19,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>>>> obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>>>> obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>>>> obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
>>>> +obj-$(CONFIG_COMMON_CLK_A9_AO) += a9-aoclk.o
>>>> obj-$(CONFIG_COMMON_CLK_C3_PLL) += c3-pll.o
>>>> obj-$(CONFIG_COMMON_CLK_C3_PERIPHERALS) += c3-peripherals.o
>>>> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>>>> diff --git a/drivers/clk/meson/a9-aoclk.c b/drivers/clk/meson/a9-aoclk.c
>>>> new file mode 100644
>>>> index 000000000000..b7b3ca231a42
>>>> --- /dev/null
>>>> +++ b/drivers/clk/meson/a9-aoclk.c
>>>> @@ -0,0 +1,419 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
>>>> +/*
>>>> + * Copyright (C) 2026 Amlogic, Inc. All rights reserved
>>>> + */
>>>> +
>>>> +#include <dt-bindings/clock/amlogic,a9-aoclkc.h>
>>>> +#include <linux/clk-provider.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include "clk-regmap.h"
>>>> +#include "clk-dualdiv.h"
>>>> +#include "meson-clkc-utils.h"
>>>> +
>>>> +#define AO_OSCIN_CTRL 0x00
>>>> +#define AO_SYS_CLK0 0x04
>>>> +#define AO_PWM_CLK_A_CTRL 0x1c
>>>> +#define AO_PWM_CLK_B_CTRL 0x20
>>>> +#define AO_PWM_CLK_C_CTRL 0x24
>>>> +#define AO_PWM_CLK_D_CTRL 0x28
>>>> +#define AO_PWM_CLK_E_CTRL 0x2c
>>>> +#define AO_PWM_CLK_F_CTRL 0x30
>>>> +#define AO_PWM_CLK_G_CTRL 0x34
>>>> +#define AO_CEC_CTRL0 0x38
>>>> +#define AO_CEC_CTRL1 0x3c
>>>> +#define AO_RTC_BY_OSCIN_CTRL0 0x50
>>>> +#define AO_RTC_BY_OSCIN_CTRL1 0x54
>>>> +
>>>> +#define A9_COMP_SEL(_name, _reg, _shift, _mask, _pdata) \
>>>> + MESON_COMP_SEL(a9_ao_, _name, _reg, _shift, _mask, _pdata, NULL, 0, 0)
>>>> +
>>>> +#define A9_COMP_DIV(_name, _reg, _shift, _width) \
>>>> + MESON_COMP_DIV(a9_ao_, _name, _reg, _shift, _width, 0, CLK_SET_RATE_PARENT)
>>>> +
>>>> +#define A9_COMP_GATE(_name, _reg, _bit) \
>>>> + MESON_COMP_GATE(a9_ao_, _name, _reg, _bit, CLK_SET_RATE_PARENT)
>>>> +
>>>> +static struct clk_regmap a9_ao_xtal_in = {
>>>> + .data = &(struct clk_regmap_gate_data){
>>>> + .offset = AO_OSCIN_CTRL,
>>>> + .bit_idx = 3,
>>>> + },
>>>> + /*
>>>> + * It may be ao_sys's parent clock, its child clocks mark
>>>> + * CLK_IS_CRITICAL, So mark CLK_IS_CRITICAL for it.
>>>> + */
>>> I don't really get what you mean ... Could you rephrase ?
>>
>> The AO sys gate clock chain may be:
>>
>> ao_xtal_in->ao_xtal->ao_sys-> AO sys gate clocks
>>
>> "ao_xtal_in" is part of the parent chain of the AO sys gate clocks.
>>
>> Some of its downstream clocks are marked with CLK_IS_CRITICAL. To ensure
>> those clocks remain functional, ao_xtal_in must not be disabled and is
>> therefore marked as CLK_IS_CRITICAL as well.
> If any of the downstream clocks are critical and marked as such, there is not
> need to mark this one as well.
>
> You should only mark the clocks that are actually critical with the flag
> and let CCF figure out the dependencies.
Thanks for the clarification.
Understood. CCF already keeps the parent clocks of critical clocks enabled
during __clk_core_init(), so the CLK_IS_CRITICAL flag is not needed here.
I'll drop it in the next revision.
>>
>> I will rephrase it like this in the next version:
>>
>> /*
>> * ao_sys can select different clock sources. One possible clock
>> path is:
>> * ao_xtal_in->ao_xtal->ao_sys-> ao sys gate clocks
>> *
>> * ao_xtal_in is in the parent chain of AO sys gate clocks.
>> * Since some downstream clocks are marked CLK_IS_CRITICAL,
>> * ao_xtal_in must remain enabled and is therefore marked
>> * CLK_IS_CRITICAL as well.
>> */
>>
>>>> + .hw.init = CLK_HW_INIT_FW_NAME("ao_xtal_in", "xtal",
>>>> + &clk_regmap_gate_ops, CLK_IS_CRITICAL),
>>> I'm honestly not sure about this. It is correct, sure and the macro exist to be
>>> used but ... It does not really help readability here, does it ?
>>>
>>> (I know that was a feedback you've got on v1)
>>>
>>> Other than that, this looks good to me.
>>>
>> Ok, I will use the original clk_init_data for this one.
> Well my comment applies to whole thing really.
>
> There are surely ways in which the macro but the way we statically
> declare things, it adds a level of indirection that makes things harder
> to review IMO.
Understood. The same reasoning applies to the PLL and peripheral clock
controllers too.
I'll switch back to the explicit clk_init_data initialization and drop
CLK_HW_INIT_FW_NAME in the next revision.
>>
>> [ ... ]
>>
>>> --
>>> Jerome
> --
> Jerome
--
Jian
^ permalink raw reply
* Re: [PATCH 2/2] MAINTAINERS: Add myself as maintainer for PMS7003
From: Maxwell Doose @ 2026-06-11 12:24 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Krzysztof Kozlowski, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, open list:IIO SUBSYSTEM AND DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, Tomasz Duszynski
In-Reply-To: <aipfxL3ttO1-7W1T@ashevche-desk.local>
On Thu, Jun 11, 2026 at 2:12 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Thu, Jun 11, 2026 at 10:09:57AM +0300, Andy Shevchenko wrote:
> > On Thu, Jun 11, 2026 at 08:58:47AM +0200, Krzysztof Kozlowski wrote:
> > > On 11/06/2026 08:50, Andy Shevchenko wrote:
> > > > On Thu, Jun 11, 2026 at 08:37:33AM +0200, Krzysztof Kozlowski wrote:
> > > >> On 11/06/2026 00:24, Maxwell Doose wrote:
> > > >>> On Wed, Jun 10, 2026 at 4:09 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > >>>> On Tue, Jun 09, 2026 at 11:03:26AM -0500, Maxwell Doose wrote:
> > > >>>>> Tomasz's entry is no longer valid, as he is not active anymore. Add
> > > >>>>
> > > >>>> Why is not longer valid? I see activity in Feb...
> > > >>>
> > > >>> Strange. According to git log --author="Tomasz Duszynski" last commit
> > > >>> I have from him is 2023. We also did have an RFC open for a month on
> > > >>> linux-iio with Tomasz Cced with no response.
> > > >>
> > > >> So you did not check enough... and no one needs to read RFC :/
> > > >
> > > > Hmm... lore.kernel.org shows last activity November last year (07-11-2025).
> > > > What other sources do you suggest to check?
> > >
> > > No, only lore.
> >
> > I used this request:
> > https://lore.kernel.org/all/?q=f%3A%22Tomasz+Duszynski%22
> >
> > > As I said, February this year.
> > >
> > > https://lore.kernel.org/all/CAObtm8zKUAWNS23nRMhc9ZR-zn7xeVOFPiV4ai_x7Bkd5puiyA@mail.gmail.com/
> >
> > Okay, you used UTF-8 name, Where did you get it from? MAINTAINERS has no
> > diacritics.
>
> OTOH, you may have used simply email approach. With
> https://lore.kernel.org/all/?q=f%3Atduszyns%40gmail.com
> I got it as well.
Ok, now I see it. I will say though that we've emailed Tomasz at least
10 times at this point for various things and hasn't responded to even
one :/ if we need to we can allow Jonathan to chime in.
^ permalink raw reply
* Re: [PATCH v3 1/8] dt-bindings: remoteproc: qcom,pas: add thermal mitigation properties
From: Krzysztof Kozlowski @ 2026-06-11 12:23 UTC (permalink / raw)
To: Gaurav Kohli
Cc: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Daniel Lezcano, Amit Kucheria,
Manivannan Sadhasivam, Konrad Dybcio, Kees Cook,
Gustavo A. R. Silva, cros-qcom-dts-watchers, linux-arm-msm,
linux-remoteproc, devicetree, linux-kernel, linux-pm,
linux-hardening, Manaf Meethalavalappu Pallikunhi
In-Reply-To: <03d863ee-2caa-41f2-94b5-7332fc930b42@oss.qualcomm.com>
On 11/06/2026 13:12, Gaurav Kohli wrote:
>> Why? And where is this generic property defined? You cannot just
>> sprinkle generic properties in random bindings.
>>
>
> Ack, will add why part.
> These names are matched with the thermal mitigation device identifiers
> populated by remote firmware over QMI and define mitigation devices are
> exposed as cooling devices.
No, -names correspond to values passed via DT, not some remote firmware.
The remote firmware should give you interface which is explicit and does
not need such properties.
>
> Without tmd-names in DT, it is unclear which index corresponds to which
> binding. With #cooling-cells, tmd-names in DT provides an explicit
> name-to-index mapping. Please suggest.
I don't get what is the index here. Index of what?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v11 6/6] arm64: dts: qcom: lemans: Add OPP-table for ICE UFS device node
From: Kuldeep Singh @ 2026-06-11 12:23 UTC (permalink / raw)
To: Abhinaba Rakshit, Bjorn Andersson, Konrad Dybcio,
Manivannan Sadhasivam, James E.J. Bottomley, Martin K. Petersen,
Adrian Hunter, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neeraj Soni, Harshal Dev
Cc: linux-arm-msm, linux-kernel, linux-scsi, linux-mmc, devicetree
In-Reply-To: <20260609-enable-ice-clock-scaling-v11-6-1cebc8b3275b@oss.qualcomm.com>
On 09-06-2026 03:17, Abhinaba Rakshit wrote:
> Qualcomm Inline Crypto Engine (ICE) platform driver now supports
> an optional OPP-table.
>
> Add OPP-table for ICE UFS device nodes for LeMans platform.
>
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
Reviewed-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
--
Regards
Kuldeep
^ permalink raw reply
* [PATCH] dt-bindings: i2c: mux-gpio: name correct maintainer
From: Wolfram Sang @ 2026-06-11 12:20 UTC (permalink / raw)
To: linux-i2c
Cc: Peter Korsgaard, Wolfram Sang, Peter Rosin, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree
The YAML conversion added me as maintainer but I can't recall being
asked nor do I want to maintain it. Add Peter as maintainer for the
binding as he is maintainer of the driver.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Documentation/devicetree/bindings/i2c/i2c-mux-gpio.yaml | 2 +-
MAINTAINERS | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.yaml
index 4a93d1f78f93..6e44510aaef6 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.yaml
@@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: GPIO-based I2C Bus Mux
maintainers:
- - Wolfram Sang <wsa@kernel.org>
+ - Peter Korsgaard <peter.korsgaard@barco.com>
description: |
This binding describes an I2C bus multiplexer that uses GPIOs to route the I2C signals.
diff --git a/MAINTAINERS b/MAINTAINERS
index e035a3be797c..a616abfce644 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10772,6 +10772,7 @@ GENERIC GPIO I2C MULTIPLEXER DRIVER
M: Peter Korsgaard <peter.korsgaard@barco.com>
L: linux-i2c@vger.kernel.org
S: Supported
+F: Documentation/devicetree/bindings/i2c/i2c-mux-gpio.yaml
F: Documentation/i2c/muxes/i2c-mux-gpio.rst
F: drivers/i2c/muxes/i2c-mux-gpio.c
F: include/linux/platform_data/i2c-mux-gpio.h
--
2.51.0
^ permalink raw reply related
* Re: [PATCH 2/2] hwmon: (pmbus/lm25066) add SMBus current limit configuration support
From: Guenter Roeck @ 2026-06-11 12:20 UTC (permalink / raw)
To: Potin Lai, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Zev Weiss
Cc: linux-hwmon, devicetree, linux-kernel, Cosmo Chou, Mike Hsieh,
Potin Lai
In-Reply-To: <20260611-lm25066-cl-config-v1-2-02e567bf3d91@gmail.com>
On 6/11/26 02:58, Potin Lai wrote:
> Add support for the mutually exclusive 'ti,cl-smbus-high' and
> 'ti,cl-smbus-low' devicetree properties. When present, these properties
> override the hardware configuration pins via the DEVICE_SETUP (0xD9)
> register to set the Current Limit Configuration bit (bit 2) and
> Current Limit Setting bit (bit 4) to SMBus settings.
>
> The Bit 4 mapping to High/Low current limit is handled dynamically on
> probe because it is swapped for lm25056 and lm25066 compared to other
> supported chips (lm5064, lm5066, and lm5066i).
>
> Signed-off-by: Potin Lai <potin.lai.pt@gmail.com>
> ---
> drivers/hwmon/pmbus/lm25066.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/hwmon/pmbus/lm25066.c b/drivers/hwmon/pmbus/lm25066.c
> index dd7275a67a0a..20e114bdc882 100644
> --- a/drivers/hwmon/pmbus/lm25066.c
> +++ b/drivers/hwmon/pmbus/lm25066.c
> @@ -34,6 +34,7 @@ enum chips { lm25056, lm25066, lm5064, lm5066, lm5066i };
> #define LM25066_READ_AVG_PIN 0xdf
>
> #define LM25066_DEV_SETUP_CL BIT(4) /* Current limit */
> +#define LM25066_DEV_SETUP_CL_CFG BIT(2) /* Current limit configuration */
>
> #define LM25066_SAMPLES_FOR_AVG_MAX 4096
>
> @@ -464,6 +465,8 @@ MODULE_DEVICE_TABLE(of, lm25066_of_match);
> static int lm25066_probe(struct i2c_client *client)
> {
> int config;
> + int config_new;
> + int ret;
> u32 shunt;
> struct lm25066_data *data;
> struct pmbus_driver_info *info;
> @@ -484,6 +487,28 @@ static int lm25066_probe(struct i2c_client *client)
>
> data->id = (enum chips)(unsigned long)i2c_get_match_data(client);
>
> + config_new = config;
> + if (of_property_read_bool(client->dev.of_node, "ti,cl-smbus-high")) {
> + config_new |= LM25066_DEV_SETUP_CL_CFG;
> + if (data->id == lm25056 || data->id == lm25066)
LM25056 does not support setting the gain via software, and bit 2 of this
register is reserved. These properties need to be disabled for that chip.
That will have to be reflected both here and in the devicetree file.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v11 5/6] arm64: dts: qcom: monaco: Add OPP-table for ICE UFS and ICE eMMC nodes
From: Kuldeep Singh @ 2026-06-11 12:20 UTC (permalink / raw)
To: Abhinaba Rakshit, Bjorn Andersson, Konrad Dybcio,
Manivannan Sadhasivam, James E.J. Bottomley, Martin K. Petersen,
Adrian Hunter, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neeraj Soni, Harshal Dev
Cc: linux-arm-msm, linux-kernel, linux-scsi, linux-mmc, devicetree
In-Reply-To: <20260609-enable-ice-clock-scaling-v11-5-1cebc8b3275b@oss.qualcomm.com>
On 09-06-2026 03:17, Abhinaba Rakshit wrote:
> Qualcomm Inline Crypto Engine (ICE) platform driver now, supports
> an optional OPP-table.
>
> Add OPP-table for ICE UFS and ICE eMMC device nodes for Monaco
> platform.
>
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/monaco.dtsi | 37 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/monaco.dtsi b/arch/arm64/boot/dts/qcom/monaco.dtsi
> index a1b6e6211b84d0d5008231c55613a0ccd61b9450..d9298d8b7874b8669b2cded2a28a99dce6eadbda 100644
> --- a/arch/arm64/boot/dts/qcom/monaco.dtsi
> +++ b/arch/arm64/boot/dts/qcom/monaco.dtsi
> @@ -2742,6 +2742,27 @@ ice: crypto@1d88000 {
> clock-names = "core",
> "iface";
> power-domains = <&gcc GCC_UFS_PHY_GDSC>;
> +
> + operating-points-v2 = <&ice_opp_table>;
> +
> + ice_opp_table: opp-table {
> + compatible = "operating-points-v2";
> +
> + opp-75000000 {
> + opp-hz = /bits/ 64 <75000000>;
> + required-opps = <&rpmhpd_opp_svs_l1>;
> + };
> +
> + opp-201600000 {
> + opp-hz = /bits/ 64 <201600000>;
> + required-opps = <&rpmhpd_opp_svs_l1>;
> + };
> +
> + opp-403200000 {
> + opp-hz = /bits/ 64 <403200000>;
> + required-opps = <&rpmhpd_opp_nom>;
> + };
> + };
> };
>
> crypto: crypto@1dfa000 {
> @@ -4878,6 +4899,22 @@ sdhc_ice: crypto@87c8000 {
> clock-names = "core",
> "iface";
> power-domains = <&rpmhpd RPMHPD_CX>;
> +
> + operating-points-v2 = <&ice_mmc_opp_table>;
s/ice_mmc_opp_table/sdhc_ice_opp_table?
--
Regards
Kuldeep
^ permalink raw reply
* Re: [PATCH v8 2/7] mfd: Add driver for ASUS Transformer embedded controller
From: Svyatoslav Ryhel @ 2026-06-11 12:16 UTC (permalink / raw)
To: Lee Jones
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <20260611111732.GN4151951@google.com>
чт, 11 черв. 2026 р. о 14:17 Lee Jones <lee@kernel.org> пише:
>
> On Thu, 28 May 2026, Svyatoslav Ryhel wrote:
> > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> >
> > Support Nuvoton NPCE795-based ECs as used in Asus Transformer TF201,
> > TF300T, TF300TG, TF300TL and TF700T pad and dock, as well as TF101 dock
> > and TF600T, P1801-T and TF701T pad. This is a glue driver handling
> > detection and common operations for EC's functions.
> >
> > Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > ---
> > drivers/mfd/Kconfig | 16 +
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/asus-transformer-ec.c | 542 ++++++++++++++++++++++++
> > include/linux/mfd/asus-transformer-ec.h | 92 ++++
> > 4 files changed, 651 insertions(+)
> > create mode 100644 drivers/mfd/asus-transformer-ec.c
> > create mode 100644 include/linux/mfd/asus-transformer-ec.h
> >
...
> > +
> > + memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> > + ret = i2c_smbus_read_i2c_block_data(client, ASUSEC_DOCKRAM_CONTROL,
> > + ASUSEC_ENTRY_SIZE, buf);
> > + if (ret < ASUSEC_ENTRY_SIZE) {
> > + dev_err(&client->dev, "failed to access control buffer: %d\n",
> > + ret);
> > + return ret;
>
> Should we return a negative error code here if the read is shorter than
> expected, rather than propagating the positive byte count?
>
Yes, I have adjusted it already locally for the next iteration. It
will return ret if negative and -EIO if ret is pos but less then
ASUSEC_ENTRY_SIZE (return ret < 0 ? ret : -EIO)
> > + }
> > +
> > + if (buf[0] != ASUSEC_CTL_SIZE) {
> > + dev_err(&client->dev, "buffer size exceeds %d: %d\n",
> > + ASUSEC_CTL_SIZE, buf[0]);
> > + return -EPROTO;
> > + }
> > +
> > + val = get_unaligned_le64(buf + 1);
> > +
> > + if (out)
> > + *out = val;
> > +
> > + if (mask || xor) {
> > + put_unaligned_le64((val & ~mask) ^ xor, buf + 1);
> > + ret = i2c_smbus_write_i2c_block_data(client,
> > + ASUSEC_DOCKRAM_CONTROL,
> > + ASUSEC_ENTRY_SIZE, buf);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(asus_dockram_access_ctl);
> > +
> > +static int asus_ec_signal_request(struct asus_ec_data *ddata)
> > +{
> > + guard(mutex)(&ddata->ecreq_lock);
> > +
> > + gpiod_set_value_cansleep(ddata->ecreq_gpio, 1);
> > + msleep(50);
> > +
> > + gpiod_set_value_cansleep(ddata->ecreq_gpio, 0);
> > + msleep(200);
>
> Do these numbers come from the datasheet or were they arbitrarily chosen?
>
There is no datasheet. These delays come from downstream driver and
with lower values or removed delays request fails.
> > +
> > + return 0;
> > +}
> > +
> > +static void asus_ec_clear_buffer(struct asus_ec_data *ddata)
> > +{
> > + int ret, retry = ASUSEC_RSP_BUFFER_SIZE;
> > +
> > + /*
> > + * Read the buffer till we get valid data by checking ASUSEC_OBF_MASK
> > + * of the status byte or till we reach end of the 256 byte buffer.
> > + */
> > + while (retry--) {
> > + ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> > + ASUSEC_ENTRY_SIZE,
> > + ddata->ec_buf);
> > + if (ret < ASUSEC_ENTRY_SIZE)
> > + continue;
> > +
> > + if (ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK)
> > + continue;
> > +
> > + break;
> > + }
> > +}
> > +
> > +static int asus_ec_log_info(struct asus_ec_data *ddata, unsigned int reg,
> > + const char *name, const char **out)
> > +{
> > + struct device *dev = &ddata->client->dev;
> > + u8 buf[ASUSEC_ENTRY_BUFSIZE];
> > + int ret;
> > +
> > + memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> > + ret = i2c_smbus_read_i2c_block_data(ddata->ec.dockram, reg,
> > + ASUSEC_ENTRY_SIZE, buf);
> > + if (ret < ASUSEC_ENTRY_SIZE)
> > + return ret;
>
> Same here. These should be negative.
>
return ret < 0 ? ret : -EIO same as above
> > +
> > + if (buf[0] > ASUSEC_ENTRY_SIZE) {
> > + dev_err(dev, "bad data len; buffer: %*ph; ret: %d\n",
> > + ASUSEC_ENTRY_BUFSIZE, buf, ret);
> > + return -EPROTO;
> > + }
> > +
> > + if (!ddata->logging_disabled) {
> > + dev_info(dev, "%-14s: %.*s\n", name, buf[0], buf + 1);
> > +
> > + if (out) {
> > + *out = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
> > + buf[0], buf + 1);
> > + if (!*out)
> > + return -ENOMEM;
> > + }
> > + }
>
> FWIW, I hate this! What does it give you now that development is done?
>
We have already discussed this, and you agreed that EC and firmware
prints may stay! This prints EC model and firmware info as well as EC
firmware behavior. It allows identify possible new revisions of EC -
Firmware combo and address possible regressions (check if it is chip
malfunction or firmware needs a new programming model) without
rebuilding kernel and digging downstream kernel for needed bits of
code.
> > + return 0;
> > +}
> > +
> > +static int asus_ec_reset(struct asus_ec_data *ddata)
> > +{
> > + int retry, ret;
> > +
> > + guard(mutex)(&ddata->ecreq_lock);
> > +
> > + for (retry = 0; retry < ASUSEC_RETRY_MAX; retry++) {
>
> for (int return = ... is generally preferred for throwaway variables.
>
Not that I care too much, but I am defining ret anyway, why not add
retry too there?
>
> > + ret = i2c_smbus_write_word_data(ddata->client, ASUSEC_WRITE_BUF,
> > + ASUSEC_RESET);
> > + if (!ret)
> > + return 0;
> > +
> > + msleep(ASUSEC_ACCESS_TIMEOUT);
>
> I like that this is defined, can we do that with the others please?
>
I don't see any benefits of defining delays, they are all arbitrary
and derive from downstream kernel, removing or altering them caused
malfunction.
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int asus_ec_susb_on_status(struct asus_ec_data *ddata)
> > +{
> > + u64 flag;
> > + int ret;
> > +
> > + ret = asus_dockram_access_ctl(ddata->ec.dockram, &flag, 0, 0);
> > + if (ret)
> > + return ret;
> > +
> > + flag &= ASUSEC_CTL_SUSB_MODE;
> > + dev_info(&ddata->client->dev, "EC FW behaviour: %s\n",
> > + flag ? "susb on when receive ec_req" :
> > + "susb on when system wakeup");
> > +
> > + return 0;
> > +}
> > +
> > +static int asus_ec_set_factory_mode(struct asus_ec_data *ddata,
> > + enum asusec_mode fmode)
> > +{
> > + dev_info(&ddata->client->dev, "Entering %s mode.\n",
> > + fmode == ASUSEC_MODE_FACTORY ? "factory" : "normal");
> > +
> > + return asus_dockram_access_ctl(ddata->ec.dockram, NULL,
> > + ASUSEC_CTL_FACTORY_MODE,
> > + fmode == ASUSEC_MODE_FACTORY ?
> > + ASUSEC_CTL_FACTORY_MODE : 0);
>
> Why not create make:
>
> ASUSEC_MODE_FACTORY == ASUSEC_CTL_FACTORY_MODE
>
> What happens to NORMAL?
>
ASUSEC_CTL_FACTORY_MODE is a bit in the ctl register. For NORMAL mode
bit is cleared,
for FACTORY bit it set, for NONE bit is ignored.
> > +}
> > +
> > +static int asus_ec_detect(struct asus_ec_data *ddata)
> > +{
> > + int ret;
> > +
> > + ret = asus_ec_reset(ddata);
> > + if (ret)
> > + goto err_exit;
> > +
> > + asus_ec_clear_buffer(ddata);
> > +
> > + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_MODEL, "Model",
> > + &ddata->ec.model);
>
> You can use 100-chars and make the code look beautiful! :)
>
Not every subsystem permits 100 chars, some stick to 80 as a strict
rule, so it is better be safe.
> > + if (ret)
> > + goto err_exit;
> > +
> > + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_FW, "FW version",
> > + NULL);
> > + if (ret)
> > + goto err_exit;
> > +
> > + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_CFGFMT, "Config format",
> > + NULL);
> > + if (ret)
> > + goto err_exit;
> > +
> > + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_HW, "HW version",
> > + NULL);
> > + if (ret)
> > + goto err_exit;
> > +
> > + /* Disable logging on next EC request */
>
> Why, but why?
>
Cause EC requests are frequent (handshake/reset) and constant logging
same data is not acceptable.
> > + ddata->logging_disabled = true;
> > +
> > + /* Check and inform about EC firmware behavior */
> > + ret = asus_ec_susb_on_status(ddata);
> > + if (ret)
> > + goto err_exit;
> > +
> > + ddata->ec.name = ddata->info->name;
> > +
> > + /* Some EC require factory mode to be set normal on each request */
> > + if (ddata->info->fmode)
> > + ret = asus_ec_set_factory_mode(ddata, ddata->info->fmode);
> > +
> > +err_exit:
> > + if (ret)
> > + dev_err(&ddata->client->dev, "failed to access EC: %d\n", ret);
> > +
> > + return ret;
> > +}
> > +
> > +static void asus_ec_handle_smi(struct asus_ec_data *ddata, unsigned int code)
> > +{
> > + switch (code) {
> > + case ASUSEC_SMI_HANDSHAKE:
> > + case ASUSEC_SMI_RESET:
> > + asus_ec_detect(ddata);
> > + break;
> > + }
> > +}
> > +
> > +static irqreturn_t asus_ec_interrupt(int irq, void *dev_id)
> > +{
> > + struct asus_ec_data *ddata = dev_id;
> > + unsigned long notify_action;
> > + int ret;
> > +
> > + ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> > + ASUSEC_ENTRY_SIZE, ddata->ec_buf);
> > + if (ret < ASUSEC_ENTRY_SIZE ||
> > + !(ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK))
>
> Unwrap for readability.
>
> Also, I think a comment would be helpful.
>
if (ret < ASUSEC_ENTRY_SIZE)
return IRQ_NONE;
ret = ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK;
if (!ret)
return IRQ_NONE;
This would be acceptable? (I will add comments later on)
> > + return IRQ_NONE;
> > +
> > + notify_action = ddata->ec_buf[ASUSEC_IRQ_STATUS];
> > + if (notify_action & ASUSEC_SMI_MASK) {
> > + unsigned int code = ddata->ec_buf[ASUSEC_SMI_CODE];
> > +
> > + asus_ec_handle_smi(ddata, code);
> > +
> > + notify_action |= code << 8;
> > + }
> > +
> > + blocking_notifier_call_chain(&ddata->ec.notify_list,
> > + notify_action, ddata->ec_buf);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static void asus_ec_release_dockram_dev(void *client)
> > +{
> > + i2c_unregister_device(client);
> > +}
> > +
> > +static struct i2c_client *devm_asus_dockram_get(struct device *dev)
> > +{
> > + struct i2c_client *parent = to_i2c_client(dev);
> > + struct i2c_client *dockram;
> > + struct dockram_ec_data *ddata;
> > + int ret;
> > +
> > + dockram = i2c_new_ancillary_device(parent, "dockram",
> > + parent->addr + 2);
>
> Could we define a macro for the address offset '2' here to avoid using a magic
> number?
>
It seems that you are excessively concerned with "magic numbers".
> > + if (IS_ERR(dockram))
> > + return dockram;
> > +
> > + ret = devm_add_action_or_reset(dev, asus_ec_release_dockram_dev,
> > + dockram);
> > + if (ret)
> > + return ERR_PTR(ret);
> > +
> > + ddata = devm_kzalloc(&dockram->dev, sizeof(*ddata), GFP_KERNEL);
> > + if (!ddata)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + i2c_set_clientdata(dockram, ddata);
> > + mutex_init(&ddata->ctl_lock);
> > +
> > + return dockram;
> > +}
> > +
> > +static const struct mfd_cell asus_ec_sl101_dock_mfd_devices[] = {
> > + MFD_CELL_NAME("asus-transformer-ec-kbc"),
> > +};
> > +
> > +static const struct mfd_cell asus_ec_tf101_dock_mfd_devices[] = {
> > + MFD_CELL_BASIC("asus-transformer-ec-battery", NULL, NULL, 0, 1),
> > + MFD_CELL_BASIC("asus-transformer-ec-charger", NULL, NULL, 0, 1),
> > + MFD_CELL_BASIC("asus-transformer-ec-led", NULL, NULL, 0, 1),
> > + MFD_CELL_NAME("asus-transformer-ec-keys"),
> > + MFD_CELL_NAME("asus-transformer-ec-kbc"),
> > +};
> > +
> > +static const struct mfd_cell asus_ec_tf201_pad_mfd_devices[] = {
> > + MFD_CELL_NAME("asus-transformer-ec-battery"),
> > + MFD_CELL_NAME("asus-transformer-ec-led"),
> > +};
> > +
> > +static const struct mfd_cell asus_ec_tf600t_pad_mfd_devices[] = {
> > + MFD_CELL_NAME("asus-transformer-ec-battery"),
> > + MFD_CELL_NAME("asus-transformer-ec-charger"),
> > + MFD_CELL_NAME("asus-transformer-ec-led"),
> > +};
> > +
> > +static int asus_ec_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct asus_ec_data *ddata;
> > + const struct mfd_cell *cells;
> > + unsigned int num_cells;
> > + unsigned long irqflags;
> > + int ret;
> > +
> > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
> > + return dev_err_probe(dev, -ENXIO,
> > + "I2C bus is missing required SMBus block mode support\n");
> > +
> > + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> > + if (!ddata)
> > + return -ENOMEM;
> > +
> > + ddata->info = device_get_match_data(dev);
> > + if (!ddata->info)
> > + return -ENODEV;
> > +
> > + switch (ddata->info->variant) {
> > + case ASUSEC_SL101_DOCK:
> > + cells = asus_ec_sl101_dock_mfd_devices;
> > + num_cells = ARRAY_SIZE(asus_ec_sl101_dock_mfd_devices);
> > + break;
> > + case ASUSEC_TF101_DOCK:
> > + cells = asus_ec_tf101_dock_mfd_devices;
> > + num_cells = ARRAY_SIZE(asus_ec_tf101_dock_mfd_devices);
> > + break;
> > + case ASUSEC_TF201_PAD:
> > + cells = asus_ec_tf201_pad_mfd_devices;
> > + num_cells = ARRAY_SIZE(asus_ec_tf201_pad_mfd_devices);
> > + break;
> > + case ASUSEC_TF600T_PAD:
> > + cells = asus_ec_tf600t_pad_mfd_devices;
> > + num_cells = ARRAY_SIZE(asus_ec_tf600t_pad_mfd_devices);
> > + break;
> > + default:
> > + return dev_err_probe(dev, -EINVAL,
> > + "unknown device variant %d\n",
> > + ddata->info->variant);
> > + }
> > +
> > + i2c_set_clientdata(client, ddata);
> > + ddata->client = client;
> > +
> > + ddata->ec.dockram = devm_asus_dockram_get(dev);
> > + if (IS_ERR(ddata->ec.dockram))
> > + return dev_err_probe(dev, PTR_ERR(ddata->ec.dockram),
> > + "failed to get dockram\n");
> > +
> > + ddata->ecreq_gpio = devm_gpiod_get(dev, "request", GPIOD_OUT_LOW);
> > + if (IS_ERR(ddata->ecreq_gpio))
> > + return dev_err_probe(dev, PTR_ERR(ddata->ecreq_gpio),
> > + "failed to get EC request GPIO\n");
> > +
> > + BLOCKING_INIT_NOTIFIER_HEAD(&ddata->ec.notify_list);
> > + mutex_init(&ddata->ecreq_lock);
> > +
> > + asus_ec_signal_request(ddata);
> > +
> > + ret = asus_ec_detect(ddata);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to detect EC version\n");
> > +
> > + /*
> > + * Systems using device tree should set up interrupt via DTS,
> > + * the rest will use the default low interrupt.
> > + */
> > + irqflags = dev->of_node ? 0 : IRQF_TRIGGER_LOW;
> > +
> > + ret = devm_request_threaded_irq(dev, client->irq, NULL,
> > + &asus_ec_interrupt,
> > + IRQF_ONESHOT | irqflags,
> > + client->name, ddata);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to register IRQ\n");
> > +
> > + /* Parent I2C controller uses DMA, ASUS EC and child devices do not */
> > + client->dev.coherent_dma_mask = 0;
> > + client->dev.dma_mask = &client->dev.coherent_dma_mask;
> > +
> > + return devm_mfd_add_devices(dev, 0, cells, num_cells, NULL, 0, NULL);
> > +}
> > +
> > +static const struct asus_ec_chip_info asus_ec_sl101_dock_data = {
> > + .name = "dock",
> > + .variant = ASUSEC_SL101_DOCK,
> > + .fmode = ASUSEC_MODE_NONE,
> > +};
> > +
> > +static const struct asus_ec_chip_info asus_ec_tf101_dock_data = {
> > + .name = "dock",
> > + .variant = ASUSEC_TF101_DOCK,
> > + .fmode = ASUSEC_MODE_NONE,
> > +};
> > +
> > +static const struct asus_ec_chip_info asus_ec_tf201_pad_data = {
> > + .name = "pad",
> > + .variant = ASUSEC_TF201_PAD,
> > + .fmode = ASUSEC_MODE_NORMAL,
> > +};
> > +
> > +static const struct asus_ec_chip_info asus_ec_tf600t_pad_data = {
> > + .name = "pad",
> > + .variant = ASUSEC_TF600T_PAD,
> > + .fmode = ASUSEC_MODE_NORMAL,
> > +};
>
> Any reason not to just pass the identifier (variant) and add the name
> and fmode attribues to the switch() above?
Why not set it here, I am not passing any mfd or any other API via of data.
> > +
> > +static const struct of_device_id asus_ec_match[] = {
> > + {
> > + .compatible = "asus,sl101-ec-dock",
> > + .data = &asus_ec_sl101_dock_data
> > + }, {
> > + .compatible = "asus,tf101-ec-dock",
> > + .data = &asus_ec_tf101_dock_data
> > + }, {
> > + .compatible = "asus,tf201-ec-pad",
> > + .data = &asus_ec_tf201_pad_data
> > + }, {
> > + .compatible = "asus,tf600t-ec-pad",
> > + .data = &asus_ec_tf600t_pad_data
> > + },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, asus_ec_match);
> > +
> > +static struct i2c_driver asus_ec_driver = {
> > + .driver = {
> > + .name = "asus-transformer-ec",
> > + .of_match_table = asus_ec_match,
> > + },
> > + .probe = asus_ec_probe,
> > +};
> > +module_i2c_driver(asus_ec_driver);
> > +
> > +MODULE_AUTHOR("Michał Mirosław <mirq-linux@rere.qmqm.pl>");
> > +MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@gmail.com>");
> > +MODULE_DESCRIPTION("ASUS Transformer's EC driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/mfd/asus-transformer-ec.h b/include/linux/mfd/asus-transformer-ec.h
> > new file mode 100644
> > index 000000000000..f085eea2193e
> > --- /dev/null
> > +++ b/include/linux/mfd/asus-transformer-ec.h
> > @@ -0,0 +1,92 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __MFD_ASUS_TRANSFORMER_EC_H
> > +#define __MFD_ASUS_TRANSFORMER_EC_H
> > +
> > +#include <linux/notifier.h>
> > +#include <linux/platform_device.h>
> > +
> > +struct i2c_client;
> > +
> > +/**
> > + * struct asusec_core - public part shared with all cells
> > + *
> > + * @model: firmware version running on the EC
> > + * @name: prefix associated with the EC
> > + * @dockram: pointer to Dockram's i2c_client
> > + * @notify_list: notify list used by cells
> > + */
> > +struct asusec_core {
> > + const char *model;
> > + const char *name;
> > + struct i2c_client *dockram;
> > + struct blocking_notifier_head notify_list;
> > +};
> > +
> > +#define ASUSEC_ENTRIES 0x100
> > +#define ASUSEC_ENTRY_SIZE 32
> > +#define ASUSEC_ENTRY_BUFSIZE (ASUSEC_ENTRY_SIZE + 1)
> > +
> > +/* interrupt sources */
> > +#define ASUSEC_IRQ_STATUS 1
> > +#define ASUSEC_OBF_MASK BIT(0)
> > +#define ASUSEC_KEY_MASK BIT(2)
> > +#define ASUSEC_KBC_MASK BIT(3)
> > +#define ASUSEC_AUX_MASK BIT(5)
> > +#define ASUSEC_SCI_MASK BIT(6)
> > +#define ASUSEC_SMI_MASK BIT(7)
> > +
> > +/* SMI notification codes */
> > +#define ASUSEC_SMI_CODE 2
> > +#define ASUSEC_SMI_POWER_NOTIFY 0x31 /* USB cable plug event */
> > +#define ASUSEC_SMI_HANDSHAKE 0x50 /* response to ec_req edge */
> > +#define ASUSEC_SMI_WAKE 0x53
> > +#define ASUSEC_SMI_RESET 0x5f
> > +#define ASUSEC_SMI_ADAPTER_EVENT 0x60 /* charger to dock plug event */
> > +#define ASUSEC_SMI_BACKLIGHT_ON 0x63
> > +#define ASUSEC_SMI_AUDIO_DOCK_IN 0x70
> > +
> > +#define ASUSEC_SMI_ACTION(code) (ASUSEC_SMI_MASK | ASUSEC_OBF_MASK | \
> > + (ASUSEC_SMI_##code << 8))
> > +
> > +/* control register [0x0a] layout */
> > +#define ASUSEC_CTL_SIZE 8
> > +
> > +/*
> > + * EC reports power from 40-pin connector in the LSB of the control
> > + * register. The following values have been observed (xor 0x02):
> > + *
> > + * PAD-ec no-plug 0x40 / PAD-ec DOCK 0x20 / DOCK-ec no-plug 0x40
> > + * PAD-ec AC 0x25 / PAD-ec DOCK+AC 0x24 / DOCK-ec AC 0x25
> > + * PAD-ec USB 0x45 / PAD-ec DOCK+USB 0x24 / DOCK-ec USB 0x41
> > + */
> > +
> > +#define ASUSEC_CTL_DIRECT_POWER_SOURCE BIT_ULL(0)
> > +#define ASUSEC_STAT_CHARGING BIT_ULL(2)
> > +#define ASUSEC_CTL_FULL_POWER_SOURCE BIT_ULL(5)
> > +#define ASUSEC_CTL_SUSB_MODE BIT_ULL(9)
> > +#define ASUSEC_CMD_SUSPEND_S3 BIT_ULL(33)
> > +#define ASUSEC_CTL_TEST_DISCHARGE BIT_ULL(35)
> > +#define ASUSEC_CMD_SUSPEND_INHIBIT BIT_ULL(37)
> > +#define ASUSEC_CTL_FACTORY_MODE BIT_ULL(38)
> > +#define ASUSEC_CTL_KEEP_AWAKE BIT_ULL(39)
> > +#define ASUSEC_CTL_USB_CHARGE BIT_ULL(40)
> > +#define ASUSEC_CTL_LED_BLINK BIT_ULL(40)
> > +#define ASUSEC_CTL_LED_AMBER BIT_ULL(41)
> > +#define ASUSEC_CTL_LED_GREEN BIT_ULL(42)
> > +#define ASUSEC_CMD_SWITCH_HDMI BIT_ULL(56)
> > +#define ASUSEC_CMD_WIN_SHUTDOWN BIT_ULL(62)
> > +
> > +#define ASUSEC_DOCKRAM_INFO_MODEL 0x01
> > +#define ASUSEC_DOCKRAM_INFO_FW 0x02
> > +#define ASUSEC_DOCKRAM_INFO_CFGFMT 0x03
> > +#define ASUSEC_DOCKRAM_INFO_HW 0x04
> > +#define ASUSEC_DOCKRAM_CONTROL 0x0a
> > +#define ASUSEC_DOCKRAM_BATT_CTL 0x14
> > +
> > +#define ASUSEC_WRITE_BUF 0x64
> > +#define ASUSEC_READ_BUF 0x6a
> > +
> > +int asus_dockram_access_ctl(struct i2c_client *client,
> > + u64 *out, u64 mask, u64 xor);
> > +
> > +#endif /* __MFD_ASUS_TRANSFORMER_EC_H */
> > --
> > 2.51.0
> >
>
> --
> Lee Jones
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add TI ADS1220
From: Jonathan Cameron @ 2026-06-11 12:15 UTC (permalink / raw)
To: Nguyen Minh Tien
Cc: linux-iio, devicetree, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, David Lechner, Nuno Sá, Andy Shevchenko,
linux-kernel
In-Reply-To: <20260610151342.44274-2-zizuzacker@gmail.com>
On Wed, 10 Jun 2026 22:13:41 +0700
Nguyen Minh Tien <zizuzacker@gmail.com> wrote:
> The ADS1220 is a 24-bit, 2-kSPS, 4-channel delta-sigma ADC from Texas
> Instruments with an SPI (mode 1) interface, a programmable gain amplifier,
> an internal 2.048V reference and a dedicated DRDY data-ready output.
>
> Add a device tree binding describing the SPI device and its per-input
> channel child nodes (single-ended AINx or the multiplexer's differential
> pairs), the optional external/AVDD reference selection and the DRDY
> interrupt.
>
> Signed-off-by: Nguyen Minh Tien <zizuzacker@gmail.com>
Given Andy asked the big questions, I'll focus in on what is here
with assumption there is a good reason this can't be merged with existing
driver and / or binding. Note that it is fine to combine bindings but not
drivers, or indeed the other way around.
There are some interesting complex corners on this device - such as the RTD
configurations (current outputs on what would otherwise be input channels).
It might be worth looking at some of the other RTD supporting sensors
in tree and how their bindings are done. The bridge setup is also
a little interesting as we need to know to enable the bypass of AIN3 to
AVSS.
Normally we strongly push for a binding to be complete as possible from
the start, even if the driver doesn't support some features yet but
here that may make it more complex.
Just a couple of comments below
Thanks,
Jonathan
> ---
> .../bindings/iio/adc/ti,ads1220.yaml | 146 ++++++++++++++++++
> 1 file changed, 146 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml
> new file mode 100644
> index 000000000..1fedffc2a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml
> @@ -0,0 +1,146 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,ads1220.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments ADS1220 ADC
> +
> +maintainers:
> + - Nguyen Minh Tien <zizuzacker@gmail.com>
> +
> +description:
> + The TI ADS1220 is a precision 24-bit, 2-kSPS, delta-sigma ADC with an SPI
> + (mode 1) interface. It provides two differential or four single-ended inputs
> + through a multiplexer, a programmable gain amplifier (gain 1 to 128), an
> + internal 2.048V reference and oscillator, two programmable excitation current
> + sources and a 50/60Hz rejection filter. A dedicated DRDY output signals when a
> + new conversion result is available.
Ideally put a datasheet reference in this description.
> +
> +properties:
> + compatible:
> + const: ti,ads1220
> +
> + reg:
> + maxItems: 1
> +
> + spi-cpha: true
> +
> + interrupts:
> + description: DRDY pin, signals that a new conversion result is ready.
> + maxItems: 1
> +
> + avdd-supply:
> + description: Analog power supply (AVDD/AVSS).
> +
> + dvdd-supply:
> + description: Digital power supply (DVDD/DGND).
> +
> + vref-supply:
> + description:
> + External reference voltage (REFP0/REFN0). If omitted, the internal
> + 2.048V reference is used unless ti,vref-avdd is set.
> +
> + ti,vref-avdd:
> + type: boolean
> + description:
> + Use the analog supply (AVDD/AVSS) as the conversion reference instead of
> + the internal 2.048V reference. Suited to ratiometric single-supply
> + measurements (for example a potentiometer wired across AVDD), giving a
> + full 0..AVDD input range without an external reference. Ignored when
> + vref-supply is present.
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + "#io-channel-cells":
> + const: 1
> +
> +required:
> + - compatible
> + - reg
> + - "#address-cells"
> + - "#size-cells"
> + - avdd-supply
> + - dvdd-supply
> +
> +patternProperties:
> + "^channel@[0-6]$":
> + $ref: adc.yaml
> + type: object
> + description: Represents one ADC input configuration (channel).
> +
> + properties:
> + reg:
> + minimum: 0
> + maximum: 6
Why 6? Is the idea that you might have both differential and single ended
inputs from the same physical wires, or overlapping differential
pairs? In practice that rarely reflects how boards are wired.
However I can't immediately figure out a combination that ends up with 7 channels.
> +
> + diff-channels:
> + description:
> + Differential input pair routable by the ADS1220 multiplexer.
> + oneOf:
> + - items: [const: 0, const: 1]
> + - items: [const: 0, const: 2]
> + - items: [const: 0, const: 3]
> + - items: [const: 1, const: 2]
> + - items: [const: 1, const: 3]
> + - items: [const: 2, const: 3]
> + - items: [const: 1, const: 0]
> + - items: [const: 3, const: 2]
> +
> + single-channel:
> + description:
> + Single-ended input channel AINx measured against AVSS.
> + minimum: 0
> + maximum: 3
> +
> + oneOf:
> + - required: [diff-channels]
> + - required: [single-channel]
> +
> + required:
> + - reg
> +
> + unevaluatedProperties: false
> +
> +allOf:
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@0 {
> + compatible = "ti,ads1220";
> + reg = <0>;
> + spi-max-frequency = <2500000>;
> + spi-cpha;
> + interrupt-parent = <&pio>;
> + interrupts = <4 4 IRQ_TYPE_EDGE_FALLING>;
> + avdd-supply = <®_vcc3v3>;
> + dvdd-supply = <®_vcc3v3>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #io-channel-cells = <1>;
> +
> + channel@0 {
> + reg = <0>;
> + single-channel = <0>;
> + };
> +
> + channel@1 {
> + reg = <1>;
> + diff-channels = <0 1>;
> + };
> + };
> + };
> +...
^ permalink raw reply
* Re: [PATCH v11 4/6] arm64: dts: qcom: kodiak: Add OPP-table for ICE UFS and ICE eMMC nodes
From: Kuldeep Singh @ 2026-06-11 12:12 UTC (permalink / raw)
To: Abhinaba Rakshit, Bjorn Andersson, Konrad Dybcio,
Manivannan Sadhasivam, James E.J. Bottomley, Martin K. Petersen,
Adrian Hunter, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neeraj Soni, Harshal Dev
Cc: linux-arm-msm, linux-kernel, linux-scsi, linux-mmc, devicetree
In-Reply-To: <20260609-enable-ice-clock-scaling-v11-4-1cebc8b3275b@oss.qualcomm.com>
On 09-06-2026 03:17, Abhinaba Rakshit wrote:
> Qualcomm Inline Crypto Engine (ICE) platform driver now, supports
> an optional OPP-table.
>
> Add OPP-table for ICE UFS and ICE eMMC device nodes for Kodiak
> platform.
s/eMMC/sdhc
>
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/kodiak.dtsi | 42 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
> index ecf4790f3415c46781c8e790d7892a41300ee7a0..cd76da7e49d8c664df6a60b5c18418c4e97a3ba4 100644
> --- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
> @@ -1087,6 +1087,27 @@ sdhc_ice: crypto@7c8000 {
> clock-names = "core",
> "iface";
> power-domains = <&rpmhpd SC7280_CX>;
> +
> + operating-points-v2 = <&ice_mmc_opp_table>;
To align with sdhc_ice(as label name), can we rename to ice_sdhc_opp_table?
With this udpate,
Reviewed-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
--
Regards
Kuldeep
^ permalink raw reply
* Re: [PATCH 7/7] arm64: dts: qcom: sm8350-hdk: describe WiFi/BT chip
From: Konrad Dybcio @ 2026-06-11 12:09 UTC (permalink / raw)
To: Dmitry Baryshkov, Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Qiang Yu,
Jeff Johnson, Liam Girdwood, Mark Brown, Krzysztof Kozlowski,
Conor Dooley, Bartosz Golaszewski, Marcel Holtmann,
Luiz Augusto von Dentz, Balakrishna Godavarthi, Rocky Liao,
Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-pci, linux-kernel, linux-wireless, ath11k,
devicetree, Bartosz Golaszewski, linux-bluetooth
In-Reply-To: <20260601-sm8350-wifi-v1-7-242917d88031@oss.qualcomm.com>
On 6/1/26 11:46 AM, Dmitry Baryshkov wrote:
> The SM8350 HDK has onboard WiFi/BT chip, WCN6851. It is an earlier
> version of well-known WCN6855 WiFI/BT SoC. Describe the PMU, BT and WiFI
> parts of the device.
[...]
> + wcn6855-pmu {
> + compatible = "qcom,wcn6851-pmu", "qcom,wcn6855-pmu";
> +
> + pinctrl-0 = <&bt_en>, <&wlan_en>, <&swctrl>;
> + pinctrl-names = "default";
> +
> + wlan-enable-gpios = <&tlmm 64 GPIO_ACTIVE_HIGH>;
> + bt-enable-gpios = <&tlmm 65 GPIO_ACTIVE_HIGH>;
> + swctrl-gpios = <&tlmm 153 GPIO_ACTIVE_HIGH>;
> +
> + vddio-supply = <&vreg_s10b_1p8>;
> + vddaon-supply = <&vreg_s11b_0p95>;
> + vddpmu-supply = <&vreg_s11b_0p95>;
> + vddpmumx-supply = <&vreg_s2e_0p85>;
> + vddpmucx-supply = <&vreg_s11b_0p95>;
> + vddrfa0p95-supply = <&vreg_s11b_0p95>;
> + vddrfa1p3-supply = <&vreg_s12b_1p25>;
> + vddrfa1p9-supply = <&vreg_s1c_1p86>;
> + vddpcie1p3-supply = <&vreg_s12b_1p25>;
> + vddpcie1p9-supply = <&vreg_s1c_1p86>;
[...]
> @@ -373,6 +437,13 @@ vreg_l7e_2p8: ldo7 {
> regulator-name = "vreg_l7e_2p8";
> regulator-min-microvolt = <2800000>;
> regulator-max-microvolt = <2800000>;
> +
> + /*
> + * This is used by the RF front-end for which there is
> + * no way to represent it in DT (yet?).
> + */
> + regulator-boot-on;
> + regulator-always-on;
msm-5.4 maps this to bt-vdd-asd-supply (asd being a keyboard smash,
perhaps?) - what is its actual use?
Konrad
^ permalink raw reply
* Re: [PATCH v3 2/3] hwmon: pmbus: Add support for Silergy SQ24860
From: Guenter Roeck @ 2026-06-11 12:06 UTC (permalink / raw)
To: sashiko-reviews, Ziming Zhu; +Cc: robh, linux-hwmon, conor+dt, devicetree
In-Reply-To: <20260611075433.325CB1F00893@smtp.kernel.org>
On 6/11/26 00:54, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Missing signedness check on PMBus limits causes negative inputs to silently set maximum hardware limits.
> - [Medium] Returns -EINVAL instead of clamping for out-of-bounds lower limits in PMBUS_IIN_OC_FAULT_LIMIT.
> --
>
> commit 1edf6322da1b629fce9b16a96496a18863aacef8
> Author: Ziming Zhu <ziming.zhu@silergycorp.com>
>
> hwmon: pmbus: Add support for Silergy SQ24860
>
> Add PMBus hwmon support for the Silergy SQ24860 eFuse. The driver reports
> voltage, current, power, and temperature, along with history attributes
> and over-current fault limits.
>
>> diff --git a/drivers/hwmon/pmbus/sq24860.c b/drivers/hwmon/pmbus/sq24860.c
>> new file mode 100644
>> index 0000000000000..f16f650ff7ba2
>> --- /dev/null
>> +++ b/drivers/hwmon/pmbus/sq24860.c
> [ ... ]
>> +static int sq24860_write_word_data(struct i2c_client *client,
>> + int page, int reg, u16 value)
>> +{
>> + int ret;
>> +
>> + switch (reg) {
>> + case PMBUS_VIN_UV_WARN_LIMIT:
>> + case PMBUS_VIN_UV_FAULT_LIMIT:
>> + case PMBUS_VIN_OV_WARN_LIMIT:
>> + case PMBUS_VIN_OV_FAULT_LIMIT:
>> + case PMBUS_VOUT_UV_WARN_LIMIT:
>> + case PMBUS_IIN_OC_WARN_LIMIT:
>> + case PMBUS_OT_WARN_LIMIT:
>> + case PMBUS_OT_FAULT_LIMIT:
>> + case PMBUS_PIN_OP_WARN_LIMIT:
>> + value >>= SQ24860_8B_SHIFT;
>> + value = clamp_val(value, 0, 0xff);
>
> [Severity: High]
> Does this handle negative user inputs correctly?
>
> The PMBus core converts limit values into 16-bit 2's complement integers
> (s16), but passes them as u16 to sq24860_write_word_data(). Because this
> driver treats value as an unsigned u16, a negative user limit written via
> sysfs will become a large positive value (e.g., 0xFFFF).
>
> When this large value is logically right-shifted and clamped here, it
> results in setting the maximum possible hardware limit (0xff) instead of
> clamping to the minimum limit (0).
>
Without a datasheet it is all but impossible to determine how to handle
this. Is the 8-bit value signed or not ? Who knows. The read function
suggests that it is unsigned, given that the sign bit is not extended,
but that is just a guess. If the value is unsigned, negative values
(0x8000 and higher) would have to be set to 0 prior to shifting and
clamping. However, that really depends on the chip.
>> + ret = pmbus_write_word_data(client, page, reg, value);
>> + break;
>> +
>> + case PMBUS_IIN_OC_FAULT_LIMIT:
>> + if (value < SQ24860_IIN_OCF_OFF)
>> + return -EINVAL;
>
> [Severity: High]
> Could a negative user limit bypass this bounds check?
>
> Since value is unsigned, a negative limit (e.g., 0xFFFF) will evaluate
> as greater than SQ24860_IIN_OCF_OFF (165). This bypasses the check below
> and overflows the 32-bit unsigned calculation, effectively disabling
> over-current protection by incorrectly setting the hardware limit to its
> maximum.
>
Same as above.
> [Severity: Medium]
> Should this clamp the value instead of returning an error?
>
> The hwmon sysfs ABI convention requires drivers to silently clamp
> out-of-range user limits to the nearest supported hardware value rather
> than returning an error.
>
> If the requested limit translates to a value below SQ24860_IIN_OCF_OFF,
> returning -EINVAL violates this convention. Would it be better to clamp
> the limit to SQ24860_IIN_OCF_OFF so the underlying register is set to 0?
>
Yes.
Thanks,
Guenter
>> + value -= SQ24860_IIN_OCF_OFF;
>> + value = DIV_ROUND_CLOSEST(((unsigned int)value) * SQ24860_IIN_OCF_DIV,
>> + SQ24860_IIN_OCF_NUM);
>> + value = clamp_val(value, 0, 0x3f);
>> + ret = pmbus_write_byte_data(client, page, SQ24860_VIREF, value);
>> + break;
>
^ permalink raw reply
* Re: [PATCH v3 00/11] kdump: reduce vmcore size and capture time
From: Baoquan He @ 2026-06-11 12:03 UTC (permalink / raw)
To: Wandun
Cc: linux-arm-kernel, linux-kernel, loongarch, linux-riscv,
devicetree, kexec, iommu, zhaomeijing, Rob Herring, saravanak,
bhe, rppt, pjw, palmer, aou, chenhuacai, kernel, catalin.marinas,
will, alex, akpm, pasha.tatashin, pratyush, ruirui.yang,
m.szyprowski, robin.murphy
In-Reply-To: <a3993db0-6975-455d-9674-4fd7cfcf80fc@gmail.com>
On 06/11/26 at 11:09am, Wandun wrote:
>
>
> On 6/11/26 10:09, Wandun wrote:
> >
> >
> > On 5/27/26 11:29, Wandun Chen wrote:
> >> From: Wandun Chen <chenwandun@lixiang.com>
> >>
> >> On SoCs that carve out large firmware-owned reserved memory (GPU
> >> firmware, DSP, modem, camera ISP, NPU, ...), kdump currently dumps
> >> those carveouts as part of system RAM even though their contents are
> >> firmware state that is not useful for kernel crash analysis.
> >>
> >> This series introduces an opt-in 'dumpable' flag [1] on struct
> >> reserved_mem and uses it to filter the elfcorehdr PT_LOAD ranges on
> >> DT-based architectures (arm64, riscv, loongarch). By default reserved
> >> regions are treated as non-dumpable; CMA regions are explicitly opted
> >> in because their pages are returned to the buddy allocator and may
> >> carry key crash-analysis data.
> >>
> >> The series is organized as follows:
> >> Patches 1-3: Pre-existing fixes and a small prep change.
> >> Patches 4-5: Restructure to allow appending /memreserve/ entries.
> >> Patches 6-7: Add a dumpable flag and append /memreserve/ entries.
> >> Patch 8: Add generic kdump helpers.
> >> Patches 9-11: Wire the helpers into arm64, riscv and loongarch kdump
> >> elfcorehdr preparation.
> > Hi,
> >
> > Gentle ping on this series.
> >
> > Status summary:
> > -patch 03: respun separately per Rob's suggestion, picked up for 7.2
> > -patch 06: Acked-by: Marek Szyprowski -patch 09: Acked-by: Will Deacon
> > The remaining patches (01, 02, 04, 05, 07, 08, 10, 11) are still
> > awaiting review. your feedback would be greately appreciated. I know we
> > are at the end of 7.1 -rc cycle, I don't want to rush this series, just
> > collecting more feedback, and will send next version based on 7.2-rc1.
> > If spliting the series into smaller logical group would make review
> > easier, please let me know. Best regards, Wandun
>
> Apologies for the formatting issue in my previous email.
> Here is the properly formatted version.
>
> Gentle ping on this series.
Thanks for the effort, the overral looks good to me at 1st glance. I will
check if there's concern on generic part. And meanwhile, I am wondering
if there's any chance x86 or other ARCH-es w/o OF/FDT can also choose to
not dump some areas, e.g GPU stolen memory. Surely, that's another story.
>
> Status summary:
> - patch 03: respun separately per Rob's suggestion, picked up for 7.2
> - patch 06: Acked-by: Marek Szyprowski
> - patch 09: Acked-by: Will Deacon
>
> The remaining patches (01, 02, 04, 05, 07, 08, 10, 11) are still
> awaiting review. Your feedback would be greatly appreciated.
>
> I know we are at the end of 7.1-rc cycle, I don't want to rush this
> series, just collecting more feedback, and will send next version based
> on 7.2-rc1.
>
> If splitting the series into smaller logical groups would make review
> easier, please let me know.
>
> Best regards,
> Wandun
>
>
> >>
> >> v2 --> v3:
> >> 1. Fix out-of-bounds issue if device tree lacks /reserved-memory node.[2]
> >> 2. Fix UAF issue when alloc_reserved_mem_array() fails.
> >> 3. Add some prepare patches.
> >>
> >> v1 --> v2:
> >> 1. v1 added an opt-out DT property ('linux,no-dump'). Per Rob's
> >> feedback [1], v2 drop that property and exclude reserve memory
> >> by default.
> >> 2. Split some prepared patches from the original patches.
> >> 3. Address coding-style comments on patch 5 from Rob.
> >>
> >> [1] https://lore.kernel.org/lkml/20260506144542.GA2072596-
> >> robh@kernel.org/
> >> [2] https://sashiko.dev/#/patchset/20260520091844.592753-1-
> >> chenwandun%40lixiang.com?part=4
> >>
> >> Wandun Chen (11):
> >> of: reserved_mem: handle NULL name in of_reserved_mem_lookup()
> >> kexec/crash: provide crash_exclude_mem_range() stub when
> >> CONFIG_CRASH_DUMP=n
> >> of: reserved_mem: avoid post-init UAF when alloc_reserved_mem_array()
> >> fails
> >> of: reserved_mem: zero total_reserved_mem_cnt if no valid
> >> /reserved-memory entry
> >> of: reserved_mem: split alloc_reserved_mem_array() from
> >> fdt_scan_reserved_mem_late()
> >> of: reserved_mem: add dumpable flag to opt-in vmcore
> >> of: reserved_mem: save /memreserve/ entries into the reserved_mem
> >> array
> >> of: reserved_mem: add kdump helpers to exclude non-dumpable regions
> >> arm64: kdump: exclude non-dumpable reserved memory regions from vmcore
> >> riscv: kdump: exclude non-dumpable reserved memory regions from vmcore
> >> loongarch: kdump: exclude non-dumpable reserved memory regions from
> >> vmcore
> >>
> >> arch/arm64/kernel/machine_kexec_file.c | 6 ++
> >> arch/loongarch/kernel/machine_kexec_file.c | 6 ++
> >> arch/riscv/kernel/machine_kexec_file.c | 4 +
> >> drivers/of/fdt.c | 11 +-
> >> drivers/of/of_private.h | 3 +
> >> drivers/of/of_reserved_mem.c | 117 +++++++++++++++++++--
> >> include/linux/crash_core.h | 6 ++
> >> include/linux/of_reserved_mem.h | 15 +++
> >> kernel/dma/contiguous.c | 1 +
> >> 9 files changed, 157 insertions(+), 12 deletions(-)
> >>
> >
>
^ permalink raw reply
* Re: [PATCH 2/2] pinctrl: tegra238: add missing AON pin groups
From: Linus Walleij @ 2026-06-11 12:04 UTC (permalink / raw)
To: Prathamesh Shete
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thierry Reding,
Jonathan Hunter, Arnd Bergmann, linux-gpio, devicetree,
linux-tegra, linux-kernel
In-Reply-To: <20260608094122.1245189-2-pshete@nvidia.com>
On Mon, Jun 8, 2026 at 11:42 AM Prathamesh Shete <pshete@nvidia.com> wrote:
> Add 24 pin groups on ports EE, FF, GG and HH to the AON pin controller
> group table (tegra238_aon_groups[]). Their pin arrays, drive-group
> macros and pin descriptors were already defined, but the matching
> PINGROUP() entries were not present, so these pins could not be muxed
> or configured through the AON pin controller.
>
> The pin arrays were not referenced, so the build emitted
> -Wunused-const-variable warnings, and commit 119de2c33d96 ("pinctrl:
> tegra238: remove unused entries") removed three of them. Restore those
> arrays and add the full set of PINGROUP() entries to make the pins
> usable.
>
> Fixes: 25cac7292d49 ("pinctrl: tegra: Add Tegra238 pinmux driver")
> Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
Patches applied.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 5/7] arm64: dts: qcom: sm8350: expand UART18 to 4 pins config
From: Konrad Dybcio @ 2026-06-11 12:04 UTC (permalink / raw)
To: Dmitry Baryshkov, Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Qiang Yu,
Jeff Johnson, Liam Girdwood, Mark Brown, Krzysztof Kozlowski,
Conor Dooley, Bartosz Golaszewski, Marcel Holtmann,
Luiz Augusto von Dentz, Balakrishna Godavarthi, Rocky Liao,
Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-pci, linux-kernel, linux-wireless, ath11k,
devicetree, Bartosz Golaszewski, linux-bluetooth
In-Reply-To: <20260601-sm8350-wifi-v1-5-242917d88031@oss.qualcomm.com>
On 6/1/26 11:46 AM, Dmitry Baryshkov wrote:
> On SM8350 platforms the primary use of UART18 is a 4-pin UART (targeting
> Bluetooth or other similar applications). Add all 4 pins to the default
> pinctrl entry for the UART.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: pinctrl: tegra238: add missing AON pin groups
From: Linus Walleij @ 2026-06-11 12:03 UTC (permalink / raw)
To: Prathamesh Shete
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thierry Reding,
Jonathan Hunter, Arnd Bergmann, linux-gpio, devicetree,
linux-tegra, linux-kernel
In-Reply-To: <20260608094122.1245189-1-pshete@nvidia.com>
On Mon, Jun 8, 2026 at 11:41 AM Prathamesh Shete <pshete@nvidia.com> wrote:
> Add 24 pin groups, and their matching drive groups, on ports EE, FF,
> GG and HH to the Tegra238 AON pinmux binding. These groups are present
> on the AON pin controller, so device trees that mux these pins through
> it validate against the schema.
>
> Fixes: 9323f8a0e12c ("dt-bindings: pinctrl: Document Tegra238 pin controllers")
> Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply
* RE: [PATCH v1 3/7] ASoC: codecs: ES8389: Fix the issue about mclk_src
From: Zhang Yi @ 2026-06-11 12:01 UTC (permalink / raw)
To: krzk
Cc: broonie, conor+dt, devicetree, krzk+dt, linux-sound, robh, tiwai,
zhangyi
In-Reply-To: <20260611-likable-ultraviolet-moth-fdf2f6@quoll>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]
> > Fix the issue with incorrect modifications to mclk_src When the system
> > needs to be configured to use the MCLK from the SCLK pin, the code
> > still sets the relevant registers to use the MCLK from the MCLK pin
> > And setting `mclk_src` to `u8` is inappropriate, because the purpose
> > of `mclk_src` is to determine whether to use SCLK as MCLK.
> > Therefore, we will change the `mclk_src` member from `u8` to `bool`.
> >
> > Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
>
> NAK, same comments as before. You just ignore feedback.
I thought I had responded to your feedback, but I haven't received a reply since then.
I¡¯d like to know if I need to respond to all of the AI¡¯s feedback.
Much of the AI¡¯s feedback concerns pre-existing issues that are unrelated to my patch.
At the same time, I assumed that when you saw a ¡°High¡± severity level on Sashiko,
you wouldn't be able to view the patches, which is why I sent the following patches and assumed they were all v0.
https://lore.kernel.org/all/20260608083540.12581-1-zhangyi@everest-semi.com/
https://lore.kernel.org/all/20260609025605.16945-1-zhangyi@everest-semi.com/
https://lore.kernel.org/all/20260609030623.17404-1-zhangyi@everest-semi.com/
If you can see my reply, should the patch I send next be v2 or v5? I think it should be v2.
I'm sorry for the trouble I've caused you
^ permalink raw reply
* Re: [PATCH 4/4] arm64: dts: qcom: glymur: Add EVA clock controller node
From: Konrad Dybcio @ 2026-06-11 12:00 UTC (permalink / raw)
To: Taniya Das, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Brian Masney, Dmitry Baryshkov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Konrad Dybcio
Cc: Ajit Pandey, Imran Shaik, Jagadeesh Kona, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260526-evacc_glymur-v1-4-b61c7755c403@oss.qualcomm.com>
On 5/26/26 7:29 AM, Taniya Das wrote:
> Add the device node for the EVA clock controller (evacc) for Qualcomm
> Glymur SoC. The EVACC provides clocks and resets to the EVA hardware block.
>
> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/glymur.dtsi | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
> index 20b49af7298e9549d126aa50a0dc7a90943a3249..66948808d197bd17ffe65190b472bb845cba0eb8 100644
> --- a/arch/arm64/boot/dts/qcom/glymur.dtsi
> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
> @@ -4,6 +4,7 @@
> */
>
> #include <dt-bindings/clock/qcom,glymur-dispcc.h>
> +#include <dt-bindings/clock/qcom,glymur-evacc.h>
> #include <dt-bindings/clock/qcom,glymur-gcc.h>
> #include <dt-bindings/clock/qcom,glymur-gpucc.h>
> #include <dt-bindings/clock/qcom,glymur-tcsr.h>
> @@ -4804,6 +4805,24 @@ videocc: clock-controller@aaf0000 {
> #power-domain-cells = <1>;
> };
>
> + evacc: clock-controller@abf0000 {
> + compatible = "qcom,glymur-evacc";
> + reg = <0x0 0x0abf0000 0x0 0x10000>;
> + clocks = <&gcc GCC_EVA_AHB_CLK>,
> + <&rpmhcc RPMH_CXO_CLK>,
> + <&rpmhcc RPMH_CXO_CLK_A>,
With the XO_A situation resolved:
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 1/4] clk: qcom: gcc-glymur: Move EVA clocks to critical clock list
From: Konrad Dybcio @ 2026-06-11 12:01 UTC (permalink / raw)
To: Dmitry Baryshkov, Taniya Das
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio,
Ajit Pandey, Imran Shaik, Jagadeesh Kona, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <61353034-51a8-4be9-8f7f-b15f5d60fcb3@oss.qualcomm.com>
On 5/26/26 9:12 AM, Dmitry Baryshkov wrote:
> On 26/05/2026 09:07, Taniya Das wrote:
>>
>>
>> On 5/26/2026 11:16 AM, Dmitry Baryshkov wrote:
>>> On Tue, May 26, 2026 at 10:59:44AM +0530, Taniya Das wrote:
>>>> The gcc_eva_ahb_clk and gcc_eva_xo_clk branch clocks should not be
>>>> registered as standalone GCC branch clocks.
>>>
>>> , otherwise .... what?
>>
>> If registered as normal branch clocks, they may be gated, which
>> breaks access to the EVA clock controller during clock controller probe.
>
> At least for the gcc_eva_ahb_clk I'd expect platforms actually reference that clock (as well as they do for GCC_VIDEO_AHB_CLK). For the XO clk it's fine as it follows other XO clocks, but please add it to the commit message.
+1
Konrad
^ permalink raw reply
* Re: [PATCH 3/4] clk: qcom: Add EVA clock controller driver for Glymur SoC
From: Konrad Dybcio @ 2026-06-11 12:00 UTC (permalink / raw)
To: Taniya Das, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Brian Masney, Dmitry Baryshkov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Konrad Dybcio
Cc: Ajit Pandey, Imran Shaik, Jagadeesh Kona, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260526-evacc_glymur-v1-3-b61c7755c403@oss.qualcomm.com>
On 5/26/26 7:29 AM, Taniya Das wrote:
> Add the Enhanced Video Analytics (EVA) clock controller driver for
> the Glymur SoC. The EVACC manages the PLL, RCGs, branch clocks, GDSCs
> and resets for the EVA subsystem which handles vision processing
> workloads.
>
> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
> ---
[...]
> +enum {
> + DT_AHB_CLK,
> + DT_BI_TCXO,
> + DT_BI_TCXO_AO,
DT_BI_TCXO_AO is unused, will it ever be?
[...]
> +static void clk_glymur_regs_configure(struct device *dev, struct regmap *regmap)
> +{
> + /* Update CTRL_IN register */
Is there any better comment we could share here?
> + regmap_update_bits(regmap, 0x9f24, BIT(0), BIT(0));
regmap_set_bits()
otherwise lgtm
Konrad
^ permalink raw reply
* Re: [PATCH v5 4/8] dt-bindings: can: fsl,flexcan: add NXP S32N79 SoC support
From: Ciprian Marian Costea @ 2026-06-11 11:57 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Marc Kleine-Budde, Vincent Mailhol, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Frank Li, Sascha Hauer,
Fabio Estevam, Pengutronix Kernel Team, linux-can, devicetree,
linux-kernel, imx, linux-arm-kernel, NXP S32 Linux Team,
Christophe Lizzi, Alberto Ruiz, Enric Balletbo, Eric Chanudet,
Andra-Teodora Ilie, Larisa Grigore, Conor Dooley, Haibo Chen
In-Reply-To: <20260610-crouching-wild-mushroom-c8bf6a@quoll>
On 6/10/2026 9:37 AM, Krzysztof Kozlowski wrote:
> On Tue, Jun 09, 2026 at 04:29:50PM +0200, Ciprian Costea wrote:
>> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>
>> Add NXP S32N79 SoC compatible string and interrupt properties.
>>
>> On S32N79, FlexCAN IP is integrated with two interrupt lines:
>> one for the mailbox interrupts (0-127) and one for signaling
>> bus errors and device state changes.
>>
>> Co-developed-by: Andra-Teodora Ilie <andra.ilie@nxp.com>
>> Signed-off-by: Andra-Teodora Ilie <andra.ilie@nxp.com>
>> Co-developed-by: Larisa Grigore <larisa.grigore@nxp.com>
>> Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
>> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>> Acked-by: Conor Dooley <conor.dooley@microchip.com>
>> Reviewed-and-tested-by: Haibo Chen <haibo.chen@nxp.com>
>
> You cannot test a binding (in a meaning what "testing" means). Building
> code is not testing.
>
>> Tested-by: Enric Balletbo i Serra <eballetb@redhat.com>
>
> Not possible.
>
> Best regards,
> Krzysztof
>
Hello Krzysztof,
Yes, my bad. It makes total sense.
I presume I shouldn't send a new version just for this 'Tested-by'
removal - right ?
Best Regards,
Ciprian
^ permalink raw reply
* RE: [PATCH v3 3/5] dt-bindings: clock: cix,sky1-audss-clock: add audss clock controller
From: Joakim Zhang @ 2026-06-11 11:57 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: mturquette@baylibre.com, sboyd@kernel.org, bmasney@redhat.com,
robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
p.zabel@pengutronix.de, Gary Yang, cix-kernel-upstream,
linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20260611-numbat-of-unmistakable-excitement-f6cfed@quoll>
Hi,
[...]
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Thursday, June 11, 2026 3:42 PM
> To: Joakim Zhang <joakim.zhang@cixtech.com>
> Cc: mturquette@baylibre.com; sboyd@kernel.org; bmasney@redhat.com;
> robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> p.zabel@pengutronix.de; Gary Yang <gary.yang@cixtech.com>; cix-kernel-
> upstream <cix-kernel-upstream@cixtech.com>; linux-clk@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v3 3/5] dt-bindings: clock: cix,sky1-audss-clock: add audss
> clock controller
>
> EXTERNAL EMAIL
>
> > diff --git a/include/dt-bindings/clock/cix,sky1-audss.h b/include/dt-
> bindings/clock/cix,sky1-audss.h
> > new file mode 100644
> > index 000000000000..033046407dee
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/cix,sky1-audss.h
>
> Filename must match the compatible.
Will rename to include/dt-bindings/clock/cix,sky1-audss-clock.h to match the compatible.
Thanks,
Joakim
^ permalink raw reply
* RE: [PATCH v2 3/5] dt-bindings: clock: cix,sky1-audss-clock: add audss clock controller
From: Joakim Zhang @ 2026-06-11 11:57 UTC (permalink / raw)
To: Krzysztof Kozlowski, mturquette@baylibre.com, sboyd@kernel.org,
bmasney@redhat.com, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, p.zabel@pengutronix.de, Gary Yang
Cc: cix-kernel-upstream, linux-clk@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <992261bb-6e2e-4662-96f2-c5b18d513b32@kernel.org>
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Thursday, June 11, 2026 3:41 PM
> To: Joakim Zhang <joakim.zhang@cixtech.com>; mturquette@baylibre.com;
> sboyd@kernel.org; bmasney@redhat.com; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de; Gary Yang
> <Gary.Yang@cixtech.com>
> Cc: cix-kernel-upstream <cix-kernel-upstream@cixtech.com>; linux-
> clk@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v2 3/5] dt-bindings: clock: cix,sky1-audss-clock: add audss
> clock controller
>
> EXTERNAL EMAIL
>
> On 09/06/2026 08:27, Joakim Zhang wrote:
> >
> > Hi Krzysztof,
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: Friday, June 5, 2026 5:24 PM
> >> To: Joakim Zhang <joakim.zhang@cixtech.com>; mturquette@baylibre.com;
> >> sboyd@kernel.org; bmasney@redhat.com; robh@kernel.org;
> >> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de; Gary
> >> krzk+Yang
> >> <gary.yang@cixtech.com>
> >> Cc: cix-kernel-upstream <cix-kernel-upstream@cixtech.com>; linux-
> >> clk@vger.kernel.org; devicetree@vger.kernel.org;
> >> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> >> Subject: Re: [PATCH v2 3/5] dt-bindings: clock: cix,sky1-audss-clock:
> >> add audss clock controller
> >>
> >> EXTERNAL EMAIL
> >>
> >> On 05/06/2026 05:22, joakim.zhang@cixtech.com wrote:
> >>> +description: |
> >>> + Clock provider for the Cix Sky1 audio subsystem (AUDSS).
> >>> +
> >>> + This node is a child of a cix,sky1-audss-system-control
> >>> + MFD/syscon node (see cix,sky1-system-control.yaml). It does not
> >>> + have a reg property; clock mux, divider and gate fields are
> >>> + accessed through the parent
> >> register block.
> >>> +
> >>> + Software reset lines for AUDSS blocks are exposed on the parent
> >>> + syscon via #reset-cells. Reset indices are defined in
> >>> + include/dt-bindings/reset/cix,sky1-audss-system-control.h.
> >>> +
> >>> + Six SoC-level reference clocks listed in clocks/clock-names feed
> >>> + the AUDSS clock tree. The provider exposes the internal AUDSS
> >>> + clocks to other devices via #clock-cells; indices are defined in
> >>> + cix,sky1-
> >> audss.h.
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + const: cix,sky1-audss-clock
> >>> +
> >>> + '#clock-cells':
> >>> + const: 1
> >>> + description:
> >>> + Clock indices are defined in include/dt-bindings/clock/cix,sky1-audss.h.
> >>> +
> >>> + clocks:
> >>> + minItems: 6
> >>
> >> Drop
> > OK
> >
> >>> + maxItems: 6
> >>> + description:
> >>> + Six SoC-level audio reference clocks that feed the audio subsystem,
> >>> + in the same order as clock-names.
> >>> +
> >>> + clock-names:
> >>> + items:
> >>> + - const: audio_clk0
> >>> + - const: audio_clk1
> >>> + - const: audio_clk2
> >>> + - const: audio_clk3
> >>> + - const: audio_clk4
> >>> + - const: audio_clk5
> >>
> >> Pretty pointless names. Names matching indexes have no benefits, drop
> >> all of them and instead list items in "clocks" with description.
> > Yes, you are right, I will describe these more meaningful.
> >
> >>> +
> >>> + resets:
> >>> + maxItems: 1
> >>> + description: Audio subsystem NoC (or bus) reset line.
> >>> +
> >>> + power-domains:
> >>> + maxItems: 1
> >>> + description: Audio subsystem power domain.
> >>
> >> So the clock part has power domain but reset part does not? This is odd.
> >> Especially that parent is audss (right?) and here you describe that
> >> this is audss poer domain.
> >>
> >> Same question about resets.
> >
> > The reset and power domain takes effect on the entire subsystem, i.e., audss
> can be accessed only after powered on and reset released, including the CRU
> registers which contains clock/reset/control bits for all device within the audss.
> >
> > Because the reset controller probe does not access the hardware, while the
> clock controller does, so at that time, the power domain and reset were placed
> in the clock driver. At present, it does not seem very reasonable either.
> >
> > Linking the "reset" and "power domain" to the parent node requires us to
> ensure the order of the probes. We need to perform deferred probes within the
> child nodes until the parent node has been probed.
> >
>
> Please wrap your replies.
>
> You refer here to probe, so driver design, but I did not ask about that.
> I asked about hardware design.
Just as you understand, power domain and noc reset for the whole audss.
Thanks,
Joakim
^ permalink raw reply
* RE: [PATCH v3 1/5] dt-bindings: soc: cix,sky1-system-control: add audss system control
From: Joakim Zhang @ 2026-06-11 11:56 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: mturquette@baylibre.com, sboyd@kernel.org, bmasney@redhat.com,
robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
p.zabel@pengutronix.de, Gary Yang, cix-kernel-upstream,
linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20260611-gorgeous-macho-cricket-f1b78c@quoll>
Hi,
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Thursday, June 11, 2026 3:40 PM
> To: Joakim Zhang <joakim.zhang@cixtech.com>
> Cc: mturquette@baylibre.com; sboyd@kernel.org; bmasney@redhat.com;
> robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> p.zabel@pengutronix.de; Gary Yang <gary.yang@cixtech.com>; cix-kernel-
> upstream <cix-kernel-upstream@cixtech.com>; linux-clk@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v3 1/5] dt-bindings: soc: cix,sky1-system-control: add audss
> system control
>
> EXTERNAL EMAIL
>
> On Wed, Jun 10, 2026 at 03:56:41PM +0800, joakim.zhang@cixtech.com wrote:
> > From: Joakim Zhang <joakim.zhang@cixtech.com>
> >
> > The Cix Sky1 Audio Subsystem (AUDSS) groups audio-related clock, reset
> > and control registers in a dedicated CRU block. Software reset lines
> > are exposed on the syscon parent via #reset-cells, following the same
> > model as the existing Sky1 FCH and S5 system control bindings.
> >
> > Add the cix,sky1-audss-system-control compatible to
> > cix,sky1-system-control.yaml for the MFD/syscon parent node, and
> > define AUDSS software reset indices in
> > include/dt-bindings/reset/cix,sky1-audss-system-control.h for I2S,
> > HDA, DMAC, mailbox, watchdog and timer blocks.
>
> All this is pretty pointless - you explained the binding, which answers nothing
> why you did it that way. Instead you must explain the hardware design.
>
> >
> > Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
> > ---
> > .../soc/cix/cix,sky1-system-control.yaml | 52 +++++++++++++++++--
> > .../reset/cix,sky1-audss-system-control.h | 25 +++++++++
> > 2 files changed, 72 insertions(+), 5 deletions(-) create mode 100644
> > include/dt-bindings/reset/cix,sky1-audss-system-control.h
> >
> > diff --git
> > a/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.ya
> > ml
> > b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.ya
> > ml index a01a515222c6..61d26a69fd44 100644
> > ---
> > a/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.ya
> > ml
> > +++ b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-contro
> > +++ l.yaml
> > @@ -15,11 +15,16 @@ description:
> >
> > properties:
> > compatible:
> > - items:
> > - - enum:
> > - - cix,sky1-system-control
> > - - cix,sky1-s5-system-control
> > - - const: syscon
> > + oneOf:
> > + - items:
> > + - enum:
> > + - cix,sky1-system-control
> > + - cix,sky1-s5-system-control
> > + - const: syscon
> > + - items:
> > + - const: cix,sky1-audss-system-control
> > + - const: simple-mfd
>
> Just so you are aware - this means children do not depend on the parent for
> operation. You will not be able to fix it later, if it turns out that children do
> depend...
Understood. simple-mfd is intentional: the clock child only accesses the parent MMIO via syscon and external resets/clocks via phandles. No parent driver coordination is needed today. We attached all resources audss needed from child node now.
> > + - const: syscon
> >
> > reg:
> > maxItems: 1
> > @@ -27,6 +32,28 @@ properties:
> > '#reset-cells':
> > const: 1
> >
> > + clock-controller:
> > + type: object
> > + properties:
> > + compatible:
> > + const: cix,sky1-audss-clock
> > + required:
> > + - compatible
> > + additionalProperties: true
> > +
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: cix,sky1-audss-system-control
> > + then:
> > + required:
> > + - clock-controller
> > + else:
> > + properties:
> > + clock-controller: false
> > +
> > required:
> > - compatible
> > - reg
> > @@ -40,3 +67,18 @@ examples:
> > reg = <0x4160000 0x100>;
> > #reset-cells = <1>;
> > };
> > + - |
> > + audss_syscon: system-controller@7110000 {
> > + compatible = "cix,sky1-audss-system-control", "simple-mfd", "syscon";
> > + reg = <0x7110000 0x10000>;
> > + #reset-cells = <1>;
> > +
> > + clock-controller {
> > + compatible = "cix,sky1-audss-clock";
> > + power-domains = <&smc_devpd 0>;
>
> My questions from v2 from the other patch are still valid - why audss system
> clock controller is outside of the power domain? Why the audss reset is outside,
> but audss clock not?
>
> This does not feel like correct hardware representation.
Yes, I agree with your point. This does not really reflect the hardware well. Both noc reset and power-domain takes effect on audss, should move to parent node.
Thanks,
Joakim
^ 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