* Re: [PATCH v3 1/3] spi: dt-bindings: snps,dw-apb-ssi: add 'power-domains' property
From: Wolfram Sang @ 2026-06-23 23:07 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-renesas-soc, Herve Codina, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-spi, devicetree
In-Reply-To: <20260623-banana-bettong-from-saturn-417cce@quoll>
[-- Attachment #1: Type: text/plain, Size: 385 bytes --]
> > + power-domains:
> > + maxItems: 1
>
> You should explain in the commit msg that likely other devices belong to
> power domain as well or likely can belong. Otherwise based on the commit
> msg I would suggest restricting it per variant... except that indeed
> these other variants could have a power domain.
Okay, if 'likely' is enough for you, I can reword it.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: PCI: qcom: Document the Hawi PCIe Controller
From: Matthew Leung @ 2026-06-23 22:47 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, linux-arm-msm, linux-pci,
devicetree, linux-kernel
In-Reply-To: <20260622-brainy-observant-crocodile-d57bef@quoll>
On Mon, Jun 22, 2026 at 12:12:04PM +0200, Krzysztof Kozlowski wrote:
> On Thu, Jun 18, 2026 at 10:00:32PM +0000, Matthew Leung wrote:
> > Add a dedicated schema for the PCIe controllers found on the Hawi
> > platform.
> >
> > Signed-off-by: Matthew Leung <matthew.leung@oss.qualcomm.com>
> > ---
> > .../devicetree/bindings/pci/qcom,hawi-pcie.yaml | 202 +++++++++++++++++++++
> > 1 file changed, 202 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,hawi-pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,hawi-pcie.yaml
> > new file mode 100644
> > index 000000000000..fb3145f89f7f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/qcom,hawi-pcie.yaml
> > @@ -0,0 +1,202 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/qcom,hawi-pcie.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm Hawi PCI Express Root Complex
> > +
> > +maintainers:
> > + - Bjorn Andersson <andersson@kernel.org>
> > + - Manivannan Sadhasivam <mani@kernel.org>
> > +
> > +description:
> > + Qualcomm Hawi SoC (and compatible) PCIe root complex controller is based on
> > + the Synopsys DesignWare PCIe IP.
> > +
> > +properties:
> > + compatible:
> > + const: qcom,hawi-pcie
> > +
> > + reg:
> > + minItems: 5
> > + items:
> > + - description: Qualcomm specific registers
> > + - description: DesignWare PCIe registers
> > + - description: External local bus interface registers
> > + - description: ATU address space
> > + - description: PCIe configuration space
> > + - description: MHI registers
>
> Why is MHI optional?
The optional properties were carried over from the previous binding
which this binding was derived from, but I see those need not be the
case here. Each of the properties are used in both instances.
I will update with making the optional properties required.
>
> > +
> > + reg-names:
> > + minItems: 5
> > + items:
> > + - const: parf
> > + - const: dbi
> > + - const: elbi
> > + - const: atu
> > + - const: config
> > + - const: mhi
> > +
> > + clocks:
> > + minItems: 6
> > + items:
> > + - description: PCIe Auxiliary clock
> > + - description: PCIe Configuration clock
> > + - description: PCIe Master AXI clock
> > + - description: PCIe Slave AXI clock
> > + - description: PCIe Slave Q2A AXI clock
> > + - description: PCIe Aggre NoC AXI clock
> > + - description: PCIe Config NoC AXI clock
>
> Same here - does that mean that once instance does not have this clock?
> If so, mention this in commit msg.
Same; will make required.
>
> > +
> > + clock-names:
> > + minItems: 6
> > + items:
> > + - const: aux
> > + - const: cfg
> > + - const: bus_master
> > + - const: bus_slave
> > + - const: slave_q2a
> > + - const: noc_aggr
> > + - const: cnoc_sf_axi
> > +
> > + interrupts:
> > + minItems: 8
> > + maxItems: 9
> > +
> > + interrupt-names:
> > + minItems: 8
> > + items:
> > + - const: msi0
> > + - const: msi1
> > + - const: msi2
> > + - const: msi3
> > + - const: msi4
> > + - const: msi5
> > + - const: msi6
> > + - const: msi7
> > + - const: global
>
> Here as well - why is global optional?
Same; will make required.
>
> > +
> > + resets:
> > + minItems: 1
>
> Same here?
Same; will make required.
>
> > + items:
> > + - description: PCIe core reset
> > + - description: PCIe link down reset
> > +
> > + reset-names:
> > + minItems: 1
> > + items:
> > + - const: pci
> > + - const: link_down
> > +
> > +required:
> > + - power-domains
> > + - resets
> > + - reset-names
> > +
> > +allOf:
> > + - $ref: qcom,pcie-common.yaml#
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH RFC 5/8] clk: sunxi-ng: a733: Add bus clocks support
From: Andre Przywara @ 2026-06-23 22:35 UTC (permalink / raw)
To: Junhui Liu, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Philipp Zabel, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Richard Cochran
Cc: linux-clk, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, linux-riscv, netdev
In-Reply-To: <20260310-a733-clk-v1-5-36b4e9b24457@pigmoral.tech>
Hi,
On 3/10/26 08:33, Junhui Liu wrote:
> Add the essential bus clocks in the Allwinner A733 CCU, including AHB,
> APB0, APB1, APB_UART, NSI, and MBUS. These buses are necessary for many
> other functional modules. Additionally clocks such as trace, gic and
> cpu_peri are also added as they fall within the register address range
> of the bus clocks, even though they are not strictly bus clocks.
>
> The MBUS clock is marked as critical to ensure the memory bus remains
> operational at all times. For the NSI and MBUS clocks, the hardware
> requires an update bit (bit 27) to be set so that the configuration
> takes effect and the updated parameters can be correctly read back.
>
> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
> ---
> drivers/clk/sunxi-ng/ccu-sun60i-a733.c | 131 +++++++++++++++++++++++++++++++++
> 1 file changed, 131 insertions(+)
>
> diff --git a/drivers/clk/sunxi-ng/ccu-sun60i-a733.c b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
> index cf819504c51f..68457813dbbb 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
> @@ -19,6 +19,7 @@
> #include "ccu_common.h"
>
> #include "ccu_div.h"
> +#include "ccu_mp.h"
> #include "ccu_mult.h"
> #include "ccu_nkmp.h"
> #include "ccu_nm.h"
> @@ -65,6 +66,16 @@ static const struct clk_hw *pll_ref_hws[] = {
> &pll_ref_clk.common.hw
> };
>
> +/*
> + * There is a non-software-configurable mux selecting between the DCXO and the
> + * PLL_REF in hardware, whose output is fed to the sys-24M clock. Although both
> + * sys-24M and pll-ref are fixed at 24 MHz, define a 1:1 fixed factor clock to
> + * provide logical separation:
> + * - pll-ref is dedicated to feeding other PLLs
> + * - sys-24M serves as reference clock for downstream functional modules
> + */
> +static CLK_FIXED_FACTOR_HWS(sys_24M_clk, "sys-24M", pll_ref_hws, 1, 1, 0);
> +
> #define SUN60I_A733_PLL_DDR_REG 0x020
> static struct ccu_nkmp pll_ddr_clk = {
> .enable = BIT(27),
> @@ -371,6 +382,107 @@ static SUNXI_CCU_M_HWS(pll_de_4x_clk, "pll-de-4x", pll_de_hws,
> static SUNXI_CCU_M_HWS(pll_de_3x_clk, "pll-de-3x", pll_de_hws,
> SUN60I_A733_PLL_DE_REG, 16, 3, 0);
>
> +/**************************************************************************
> + * bus clocks *
> + **************************************************************************/
> +
> +static const struct clk_parent_data ahb_apb_parents[] = {
> + { .hw = &sys_24M_clk.hw },
> + { .fw_name = "losc" },
> + { .fw_name = "iosc" },
> + { .hw = &pll_periph0_600M_clk.hw },
> +};
> +
> +static SUNXI_CCU_M_DATA_WITH_MUX(ahb_clk, "ahb", ahb_apb_parents, 0x500,
> + 0, 5, /* M */
> + 24, 2, /* mux */
> + 0);
> +
> +static SUNXI_CCU_M_DATA_WITH_MUX(apb0_clk, "apb0", ahb_apb_parents, 0x510,
> + 0, 5, /* M */
> + 24, 2, /* mux */
> + 0);
> +
> +static SUNXI_CCU_M_DATA_WITH_MUX(apb1_clk, "apb1", ahb_apb_parents, 0x518,
> + 0, 5, /* M */
> + 24, 2, /* mux */
> + 0);
> +
> +static const struct clk_parent_data apb_uart_parents[] = {
> + { .hw = &sys_24M_clk.hw },
> + { .fw_name = "losc" },
> + { .fw_name = "iosc" },
> + { .hw = &pll_periph0_600M_clk.hw },
> + { .hw = &pll_periph0_480M_clk.common.hw },
> +};
> +static SUNXI_CCU_M_DATA_WITH_MUX(apb_uart_clk, "apb-uart", apb_uart_parents, 0x538,
> + 0, 5, /* M */
> + 24, 3, /* mux */
> + 0);
> +
> +static const struct clk_parent_data trace_parents[] = {
> + { .hw = &sys_24M_clk.hw },
> + { .fw_name = "losc" },
> + { .fw_name = "iosc" },
> + { .hw = &pll_periph0_300M_clk.hw },
> + { .hw = &pll_periph0_400M_clk.hw },
> +};
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(trace_clk, "trace", trace_parents, 0x540,
> + 0, 5, /* M */
> + 24, 3, /* mux */
> + BIT(31), /* gate */
> + 0);
> +
> +static const struct clk_parent_data gic_cpu_peri_parents[] = {
> + { .hw = &sys_24M_clk.hw },
> + { .fw_name = "losc" },
> + { .hw = &pll_periph0_600M_clk.hw },
> + { .hw = &pll_periph0_480M_clk.common.hw },
> + { .hw = &pll_periph0_400M_clk.hw },
> +};
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(gic_clk, "gic", gic_cpu_peri_parents, 0x560,
Do we really want to model the GIC clock? The A523 has one as well, as
we don't describe it there. And while the GICv3 binding describes a
clock property, the Linux driver completely ignores that.
So if I see this correctly, this clock would become unused, and would be
turned off, killing the GIC? So we would at least need a CLK_IS_CRITICAL
flag?
But it's a good reminder to lift this clock to something PLL based, in
U-Boot's SPL, because I guess the 24MHz are rather slow.
> + 0, 5, /* M */
> + 24, 3, /* mux */
> + BIT(31), /* gate */
> + 0);
> +
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(cpu_peri_clk, "cpu-peri", gic_cpu_peri_parents, 0x568,
What is this clock about? I don't see it referenced by any peripheral in
the manual.
> + 0, 5, /* M */
> + 24, 3, /* mux */
> + BIT(31), /* gate */
> + 0);
> +
> +static const struct clk_parent_data nsi_parents[] = {
> + { .hw = &sys_24M_clk.hw },
> + { .hw = &pll_ddr_clk.common.hw },
> + { .hw = &pll_periph0_800M_clk.common.hw },
> + { .hw = &pll_periph0_600M_clk.hw },
> + { .hw = &pll_periph0_480M_clk.common.hw },
> + { .hw = &pll_de_3x_clk.common.hw },
> +};
> +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE_FEAT(nsi_clk, "nsi", nsi_parents, 0x580,
Similar question like for the GIC: do we need this in the kernel, and do
we need to prevent this from being turned off?
> + 0, 5, /* M */
> + 0, 0, /* no P */
> + 24, 3, /* mux */
> + BIT(31), /* gate */
> + 0, CCU_FEATURE_UPDATE_BIT);
> +
> +static const struct clk_parent_data mbus_parents[] = {
> + { .hw = &sys_24M_clk.hw },
> + { .hw = &pll_periph1_600M_clk.hw },
> + { .hw = &pll_ddr_clk.common.hw },
> + { .hw = &pll_periph1_480M_clk.common.hw },
> + { .hw = &pll_periph1_400M_clk.hw },
> + { .hw = &pll_npu_clk.common.hw },
> +};
> +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE_FEAT(mbus_clk, "mbus", mbus_parents, 0x588,
> + 0, 5, /* M */
> + 0, 0, /* no P */
> + 24, 3, /* mux */
> + BIT(31), /* gate */
> + CLK_IS_CRITICAL,
> + CCU_FEATURE_UPDATE_BIT);
> +
> /*
> * Contains all clocks that are controlled by a hardware register. They
> * have a (sunxi) .common member, which needs to be initialised by the common
> @@ -407,11 +519,21 @@ static struct ccu_common *sun60i_a733_ccu_clks[] = {
> &pll_de_clk.common,
> &pll_de_4x_clk.common,
> &pll_de_3x_clk.common,
> + &ahb_clk.common,
> + &apb0_clk.common,
> + &apb1_clk.common,
> + &apb_uart_clk.common,
> + &trace_clk.common,
> + &gic_clk.common,
> + &cpu_peri_clk.common,
> + &nsi_clk.common,
> + &mbus_clk.common,
> };
>
> static struct clk_hw_onecell_data sun60i_a733_hw_clks = {
> .hws = {
> [CLK_PLL_REF] = &pll_ref_clk.common.hw,
> + [CLK_SYS_24M] = &sys_24M_clk.hw,
> [CLK_PLL_DDR] = &pll_ddr_clk.common.hw,
> [CLK_PLL_PERIPH0_4X] = &pll_periph0_4x_clk.common.hw,
> [CLK_PLL_PERIPH0_2X] = &pll_periph0_2x_clk.common.hw,
> @@ -453,6 +575,15 @@ static struct clk_hw_onecell_data sun60i_a733_hw_clks = {
> [CLK_PLL_DE] = &pll_de_clk.common.hw,
> [CLK_PLL_DE_4X] = &pll_de_4x_clk.common.hw,
> [CLK_PLL_DE_3X] = &pll_de_3x_clk.common.hw,
> + [CLK_AHB] = &ahb_clk.common.hw,
> + [CLK_APB0] = &apb0_clk.common.hw,
> + [CLK_APB1] = &apb1_clk.common.hw,
> + [CLK_APB_UART] = &apb_uart_clk.common.hw,
> + [CLK_TRACE] = &trace_clk.common.hw,
> + [CLK_GIC] = &gic_clk.common.hw,
> + [CLK_CPU_PERI] = &cpu_peri_clk.common.hw,
> + [CLK_NSI] = &nsi_clk.common.hw,
> + [CLK_MBUS] = &mbus_clk.common.hw,
> },
> .num = CLK_FANOUT3 + 1,
> };
>
^ permalink raw reply
* Re: [PATCH 3/3] hwmon: (pmbus/fd5121): Add support FD5121, FD5123 and FD5125
From: Guenter Roeck @ 2026-06-23 22:07 UTC (permalink / raw)
To: Selvamani.Rajagopal, Jonathan Corbet, Shuah Khan, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-hwmon, linux-doc, linux-kernel, devicetree
In-Reply-To: <20260622-support-fd5121-from-onsemi-v1-3-b31767689c65@onsemi.com>
On 6/22/26 22:55, Selvamani Rajagopal via B4 Relay wrote:
> From: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
>
> FD5121 is a dual-rail, multi-phase, digital controller that offers
> full telemtry options including input/output voltage, current as
> well as fault handling and identifications.
>
> These controllers are compliant with PMBus specification.
>
> Signed-off-by: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
> ---
> MAINTAINERS | 8 +
> drivers/hwmon/pmbus/Kconfig | 9 +
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/fd5121.c | 1004 ++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 1022 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d95d3ef77773..c0664c33324a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20135,6 +20135,14 @@ L: linux-mips@vger.kernel.org
> S: Maintained
> F: arch/mips/boot/dts/ralink/omega2p.dts
>
> +ONSEMI HARDWARE MONITOR DRIVER
> +M: Selva Rajagopal <selvamani.rajagopal@onsemi.com>
> +L: linux-hwmon@vger.kernel.org
> +S: Supported
> +W: https://www.onsemi.com
> +F: Documentation/devicetree/bindings/hwmon/pmbus/onnn,fd5121.yaml
> +F: drivers/hwmon/pmbus/fd5121.c
> +
> ONSEMI ETHERNET PHY DRIVERS
> M: Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
> L: netdev@vger.kernel.org
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index c8cda160b5f8..3a06ed83539e 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -179,6 +179,15 @@ config SENSORS_E50SN12051
> This driver can also be built as a module. If so, the module will
> be called e50sn12051.
>
> +config SENSORS_FD5121
> + tristate "FD5121/FD5123/FD5125 controllers from onsemi"
> + help
> + If you say yes here, you get support for onsemi
> + controllers FD5121, FD5123, FD5125.
> +
> + This driver can also be built as a module. If so, the module will
> + be called fd5121.
> +
> config SENSORS_INA233
> tristate "Texas Instruments INA233 and compatibles"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index ffc05f493213..70f4afb41fe0 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_SENSORS_APS_379) += aps-379.o
> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o
> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o
> obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o
> +obj-$(CONFIG_SENSORS_FD5121) += fd5121.o
> obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o
> obj-$(CONFIG_SENSORS_HAC300S) += hac300s.o
> obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o
> diff --git a/drivers/hwmon/pmbus/fd5121.c b/drivers/hwmon/pmbus/fd5121.c
> new file mode 100644
> index 000000000000..e68c6d6cabbd
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/fd5121.c
> @@ -0,0 +1,1004 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2026 Semiconductor Components Industries, LLC ("onsemi").
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/unaligned.h>
> +
> +#include "pmbus.h"
> +
> +enum chips { chip_fd5121, chip_fd5123, chip_fd5125 };
> +
> +#define CTLR_ID_UNKNOWN 0
> +#define CTLR_ID_FD5121 0xFD5121
> +#define CTLR_ID_FD5123 0xFD5123
> +#define CTLR_ID_FD5125 0xFD5125
> +
> +#define FD5121_NUM_PAGES 2
> +
> +/* Custom PMBUS commands */
> +#define PMBUS_REG_VOUT_MIN 0x2B
> +#define PMBUS_REG_POWER_MODE 0x34
> +#define PMBUS_REG_VIN_ON 0x35
> +#define PMBUS_REG_VIN_OFF 0x36
> +#define PMBUS_REG_VIN_UV_FAULT_RESPONSE 0x5A
> +#define PMBUS_REG_IIN_OC_FAULT_RESPONSE 0x5C
> +#define PMBUS_REG_TON_DELAY 0x60
> +#define PMBUS_REG_POUT_OP_FAULT_RESPONSE 0x69
> +#define PMBUS_REG_READ_VAUX 0x85
> +
> +#define PMBUS_REG_IKNEE_SET 0x2D
> +#define PMBUS_REG_PIN_COUNTER 0x2E
> +#define PMBUS_REG_VMIN_AWARE 0x2F
> +#define PMBUS_REG_VAUX_UV_FAULT_LIMIT 0x6C
> +#define PMBUS_REG_VAUX_OV_FAULT_LIMIT 0x6D
> +#define PMBUS_REG_VAUX_UV_FAULT_RESPONSE 0x6E
> +#define PMBUS_REG_VAUX_OV_FAULT_RESPONSE 0x6F
> +#define PMBUS_REG_VAUX_UV_WARNING 0x75
> +#define PMBUS_REG_VAUX_OV_WARNING 0x76
> +#define PMBUS_REG_MFR_FREE_USER_CONFIG_TABLES 0xCF
> +#define PMBUS_REG_MFR_ADDRESS_TABLE 0xD0
> +#define PMBUS_REG_MFR_STATUS_ONSEMI 0xD1
> +#define PMBUS_REG_MFR_UNLOCK 0xD2
> +#define PMBUS_REG_MFR_FAULTY_SPS 0xD3
> +#define PMBUS_REG_TLVR_FAULTS 0xD4
> +#define PMBUS_REG_MFR_USER_STORE_CONFIG_TAB 0xD5
> +#define PMBUS_REG_MFR_USER_CONFIG_INDEX 0xD6
> +#define PMBUS_REG_MFR_PWM_DISCONNECTION 0xD7
> +#define PMBUS_REG_MFR_VR_DISCONNECTION 0xD8
> +#define PMBUS_REG_MFR_TON_SLEW 0xD9
> +#define PMBUS_REG_MFR_TOFF_SLEW 0xDA
> +#define PMBUS_REG_MFR_RAIL_NAME 0xDB
> +#define PMBUS_REG_MFR_VOUT_DROOP 0xDC
> +#define PMBUS_REG_MFR_USER_RESTORE_CONFIG_TAB 0xDD
> +#define PMBUS_REG_MFR_SVR_GO 0xDE
> +#define PMBUS_REG_MFR_SET_PWD 0xDF
> +#define PMBUS_REG_MFR_CONFIG_ACTIVATE 0xE0
> +#define PMBUS_REG_MFR_CONFIG_RECOVER 0xE1
> +#define PMBUS_REG_MFR_OTP_DUMP 0xE2
> +#define PMBUS_REG_MFR_BBR_EN 0xE3
> +#define PMBUS_REG_MFR_DPM_MIN 0xE4
> +#define PMBUS_REG_MFR_VBOOT 0xE5
> +#define PMBUS_REG_MFR_PRECLAMP_OFFSET 0xE6
> +#define PMBUS_REG_MFR_TLVR_DIAGN 0xE7
> +#define PMBUS_REG_MFR_READ_VSYS 0xE8
> +#define PMBUS_REG_MFR_SPECIFIC_E9 0xE9
> +#define PMBUS_REG_MFR_SPECIFIC_EA 0xEA
> +#define PMBUS_REG_MFR_SS_CBC 0xEB
> +#define PMBUS_REG_MFR_AMD_STATUS 0xEC
> +#define PMBUS_REG_MFR_CHECKSUM 0xEE
> +#define PMBUS_REG_CSE_INDEX 0xF0
> +#define PMBUS_REG_COUT_MEASURE 0xF1
> +#define PMBUS_REG_VR_COUT 0xF2
> +#define PMBUS_REG_BBR_RAM 0xF3
> +#define PMBUS_REG_BBR_OTP 0xF4
> +#define PMBUS_REG_READ_PSYS 0xF5
> +#define PMBUS_REG_POSTCLAMP_OFFSET 0xF6
> +#define PMBUS_REG_PGOOD_DELAY 0xF7
> +#define PMBUS_REG_MFR_SPECIFIC_F8 0xF8
> +#define PMBUS_REG_MFR_SPECIFIC_F9 0xF9
> +#define PMBUS_REG_MFR_PWD_PROGRAM_RAM 0xFA
> +#define PMBUS_REG_MFR_PWD_PROGRAM_I2C 0xFB
> +#define PMBUS_REG_MFR_PWD_ENABLE_OTP_STORE 0xFC
> +
> +/* List of recognized commands */
> +static const u8 cc_list[] = {
> + PMBUS_PAGE,
> + PMBUS_OPERATION,
> + PMBUS_ON_OFF_CONFIG,
> + PMBUS_CLEAR_FAULTS,
> + PMBUS_WRITE_PROTECT,
> + PMBUS_CAPABILITY,
> + PMBUS_VOUT_MODE,
> + PMBUS_VOUT_COMMAND,
> + PMBUS_VOUT_MAX,
> + PMBUS_VOUT_MARGIN_HIGH,
> + PMBUS_VOUT_MARGIN_LOW,
> + PMBUS_VOUT_TRANSITION_RATE,
> + PMBUS_REG_VOUT_MIN,
> + PMBUS_REG_IKNEE_SET,
> + PMBUS_REG_PIN_COUNTER,
> + PMBUS_REG_VMIN_AWARE,
> + PMBUS_REG_POWER_MODE,
> + PMBUS_REG_VIN_ON,
> + PMBUS_REG_VIN_OFF,
> + PMBUS_VOUT_OV_FAULT_LIMIT,
> + PMBUS_VOUT_OV_FAULT_RESPONSE,
> + PMBUS_VOUT_UV_FAULT_LIMIT,
> + PMBUS_VOUT_UV_FAULT_RESPONSE,
> + PMBUS_IOUT_OC_FAULT_LIMIT,
> + PMBUS_IOUT_OC_FAULT_RESPONSE,
> + PMBUS_IOUT_OC_WARN_LIMIT,
> + PMBUS_OT_FAULT_LIMIT,
> + PMBUS_OT_FAULT_RESPONSE,
> + PMBUS_OT_WARN_LIMIT,
> + PMBUS_VIN_OV_FAULT_LIMIT,
> + PMBUS_VIN_OV_FAULT_RESPONSE,
> + PMBUS_VIN_OV_WARN_LIMIT,
> + PMBUS_VIN_UV_WARN_LIMIT,
> + PMBUS_VIN_UV_FAULT_LIMIT,
> + PMBUS_REG_VIN_UV_FAULT_RESPONSE,
> + PMBUS_IIN_OC_FAULT_LIMIT,
> + PMBUS_REG_IIN_OC_FAULT_RESPONSE,
> + PMBUS_IIN_OC_WARN_LIMIT,
> + PMBUS_REG_TON_DELAY,
> + PMBUS_POUT_OP_FAULT_LIMIT,
> + PMBUS_REG_POUT_OP_FAULT_RESPONSE,
> + PMBUS_POUT_OP_WARN_LIMIT,
> + PMBUS_PIN_OP_WARN_LIMIT,
> + PMBUS_REG_VAUX_UV_FAULT_LIMIT,
> + PMBUS_REG_VAUX_OV_FAULT_LIMIT,
> + PMBUS_REG_VAUX_UV_FAULT_RESPONSE,
> + PMBUS_REG_VAUX_OV_FAULT_RESPONSE,
> + PMBUS_REG_VAUX_UV_WARNING,
> + PMBUS_REG_VAUX_OV_WARNING,
> + PMBUS_STATUS_BYTE,
> + PMBUS_STATUS_WORD,
> + PMBUS_STATUS_VOUT,
> + PMBUS_STATUS_IOUT,
> + PMBUS_STATUS_INPUT,
> + PMBUS_STATUS_TEMPERATURE,
> + PMBUS_STATUS_CML,
> + PMBUS_STATUS_OTHER,
> + PMBUS_STATUS_MFR_SPECIFIC,
> + PMBUS_REG_READ_VAUX,
> + PMBUS_READ_VIN,
> + PMBUS_READ_IIN,
> + PMBUS_READ_VOUT,
> + PMBUS_READ_IOUT,
> + PMBUS_READ_TEMPERATURE_1,
> + PMBUS_READ_POUT,
> + PMBUS_READ_PIN,
> + PMBUS_REVISION,
> + PMBUS_MFR_ID,
> + PMBUS_MFR_MODEL,
> + PMBUS_MFR_REVISION,
> + PMBUS_IC_DEVICE_ID,
> + PMBUS_REG_MFR_FREE_USER_CONFIG_TABLES,
> + PMBUS_REG_MFR_ADDRESS_TABLE,
> + PMBUS_REG_MFR_STATUS_ONSEMI,
> + PMBUS_REG_MFR_UNLOCK,
> + PMBUS_REG_MFR_FAULTY_SPS,
> + PMBUS_REG_TLVR_FAULTS,
> + PMBUS_REG_MFR_USER_STORE_CONFIG_TAB,
> + PMBUS_REG_MFR_USER_CONFIG_INDEX,
> + PMBUS_REG_MFR_PWM_DISCONNECTION,
> + PMBUS_REG_MFR_VR_DISCONNECTION,
> + PMBUS_REG_MFR_TON_SLEW,
> + PMBUS_REG_MFR_TOFF_SLEW,
> + PMBUS_REG_MFR_RAIL_NAME,
> + PMBUS_REG_MFR_VOUT_DROOP,
> + PMBUS_REG_MFR_USER_RESTORE_CONFIG_TAB,
> + PMBUS_REG_MFR_SVR_GO,
> + PMBUS_REG_MFR_SET_PWD,
> + PMBUS_REG_MFR_CONFIG_ACTIVATE,
> + PMBUS_REG_MFR_CONFIG_RECOVER,
> + PMBUS_REG_MFR_OTP_DUMP,
> + PMBUS_REG_MFR_BBR_EN,
> + PMBUS_REG_MFR_DPM_MIN,
> + PMBUS_REG_MFR_VBOOT,
> + PMBUS_REG_MFR_PRECLAMP_OFFSET,
> + PMBUS_REG_MFR_TLVR_DIAGN,
> + PMBUS_REG_MFR_READ_VSYS,
> + PMBUS_REG_MFR_SPECIFIC_E9,
> + PMBUS_REG_MFR_SPECIFIC_EA,
> + PMBUS_REG_MFR_SS_CBC,
> + PMBUS_REG_MFR_AMD_STATUS,
> + PMBUS_REG_MFR_CHECKSUM,
> + PMBUS_REG_CSE_INDEX,
> + PMBUS_REG_COUT_MEASURE,
> + PMBUS_REG_VR_COUT,
> + PMBUS_REG_BBR_RAM,
> + PMBUS_REG_BBR_OTP,
> + PMBUS_REG_READ_PSYS,
> + PMBUS_REG_POSTCLAMP_OFFSET,
> + PMBUS_REG_PGOOD_DELAY,
> + PMBUS_REG_MFR_SPECIFIC_F8,
> + PMBUS_REG_MFR_SPECIFIC_F9,
> + PMBUS_REG_MFR_PWD_PROGRAM_RAM,
> + PMBUS_REG_MFR_PWD_PROGRAM_I2C,
> + PMBUS_REG_MFR_PWD_ENABLE_OTP_STORE,
> +};
> +
> +/* Following registers expect block read */
> +static const u8 blk_rd_cc[] = {
> + PMBUS_SMBALERT_MASK,
> + PMBUS_MFR_DATE,
> + PMBUS_IC_DEVICE_REV,
> +};
> +
> +struct fd5121_data {
> + struct attribute_group *groups[3];
> + struct pmbus_driver_info info;
> + struct device *dev;
> + u32 id;
> +};
> +
> +static s32 fd5121_read_block_data(const struct i2c_client *client,
> + u8 cmd_code, u8 len, u8 *pbuf)
> +{
> + s32 ret = 0;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_READ_BLOCK_DATA)) {
> +
> + /* Payload length is in the first byte. */
> + ret = i2c_smbus_read_i2c_block_data(client, cmd_code,
> + len, pbuf);
> + if (ret < 0)
> + return ret;
> + ret = pbuf[0];
> + if (ret > len)
> + ret = len;
> + for (int idx = 0; idx < ret; idx++)
> + pbuf[idx] = pbuf[idx + 1];
> + return ret;
> + }
> + ret = i2c_smbus_read_block_data(client, cmd_code, pbuf);
> + if (ret < 0)
> + return ret;
> + return min_t(s32, ret, len);
> +}
> +
> +/* Command code that expects block read, not word read */
> +static bool fd5121_blk_rd_reg(u8 cmd_code)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(blk_rd_cc); i++) {
> + if (cmd_code == blk_rd_cc[i])
> + return true;
> + }
> + return false;
> +}
> +
> +static ssize_t fd5121_send_byte_store(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 val = 0;
> + int ret;
> +
> + ret = kstrtou8(buf, 10, &val);
> + if (ret < 0)
> + return ret;
> + ret = i2c_smbus_write_byte(client, val);
> + if (ret < 0)
> + return ret;
> + return count;
> +}
> +
> +static int fd5121_config_activate(struct i2c_client *client)
> +{
> + return i2c_smbus_write_byte_data(client,
> + PMBUS_REG_MFR_CONFIG_ACTIVATE, 0xAA);
> +}
> +
> +static ssize_t fd5121_byte_store(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + int ret = 0;
> + u8 val = 0;
> +
> + switch (reg) {
> + case PMBUS_REG_MFR_CONFIG_ACTIVATE:
> + ret = fd5121_config_activate(client);
> + if (ret < 0)
> + return ret;
> + return count;
> + default:
> + ret = kstrtou8(buf, 10, &val);
> + if (ret < 0)
> + return ret;
> + break;
> + }
> + if (reg == PMBUS_PAGE && ((val != 0 && val != 1 &&
> + val != GENMASK(7, 0))))
> + return -EINVAL;
> + ret = i2c_smbus_write_byte_data(client, reg, val);
> + if (ret < 0)
> + return ret;
> + return count;
> +}
> +
> +static ssize_t fd5121_byte_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + s32 ret;
> +
> + ret = i2c_smbus_read_byte_data(client, reg);
> + if (ret < 0)
> + return ret;
> + return scnprintf(buf, PAGE_SIZE, "%d\n", ret & 0xFF);
> +}
> +
> +static ssize_t fd5121_word_store(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + s16 val = 0;
> + int ret = 0;
> +
> + switch (reg) {
> + case PMBUS_REG_MFR_PWD_PROGRAM_RAM:
> + val = 0xC93F;
> + break;
> + default:
> + ret = kstrtos16(buf, 10, &val);
> + if (ret)
> + return ret;
> + break;
> + }
> + ret = i2c_smbus_write_word_data(client, reg, val);
> + if (ret < 0)
> + return ret;
> + return count;
> +}
> +
> +static ssize_t fd5121_word_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 data[I2C_SMBUS_BLOCK_MAX] = { 0 };
> + u8 reg = attr->index;
> + s32 ret = 0;
> +
> + if (fd5121_blk_rd_reg(reg)) {
> + ret = fd5121_read_block_data(client, reg, 2, data);
> + if (ret >= 0)
> + ret = get_unaligned_le16(data);
> + } else
> + ret = i2c_smbus_read_word_data(client, reg);
> + if (ret < 0)
> + return ret;
> + return scnprintf(buf, PAGE_SIZE, "%d\n", ret & 0xFFFF);
> +}
> +
> +static s32 fd5121_write_block_data(const struct i2c_client *client,
> + u8 cmd_code, u8 len, u8 *pbuf)
> +{
> + s32 ret = 0;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_WRITE_BLOCK_DATA))
> + ret = i2c_smbus_write_i2c_block_data(client, cmd_code,
> + len, pbuf);
> + else
> + ret = i2c_smbus_write_block_data(client, cmd_code,
> + len, pbuf);
> + return ret;
> +}
> +
> +static s32 fd5121_read_long(struct i2c_client *client, u8 cmd_code, u32 *pval)
> +{
> + u8 buffer[I2C_SMBUS_BLOCK_MAX] = { 0 };
> + s32 ret;
> +
> + ret = fd5121_read_block_data(client, cmd_code, 4, buffer);
> + if (ret < 0)
> + return ret;
> + if (ret < 4)
> + return -EIO;
> +
> + *pval = get_unaligned_le32(buffer);
> + return 0;
> +}
> +
> +static ssize_t fd5121_long_store(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + u8 buffer[4];
> + u32 val = 0;
> + int ret = 0;
> + u8 len;
> +
> + ret = kstrtou32(buf, 10, &val);
> + if (ret)
> + return ret;
> +
> + len = (u8) sizeof(buffer);
> + for (u8 i = 0; i < len; i++) {
> + buffer[i] = val & 0xFF;
> + val >>= 8;
> + }
> + ret = fd5121_write_block_data(client, reg, len, buffer);
> + if (ret < 0)
> + return ret;
> + return count;
> +}
> +
> +static ssize_t fd5121_long_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + u32 val = 0;
> + s32 ret = 0;
> +
> + ret = fd5121_read_long(client, reg, &val);
> + if (ret < 0)
> + return ret;
> + return scnprintf(buf, PAGE_SIZE, "%d\n", val);
> +}
> +
> +static ssize_t fd5121_block_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + u8 buffer[I2C_SMBUS_BLOCK_MAX] = { 0 };
> + u8 reg = attr->index;
> + int printed = 0;
> + s32 ret = 0;
> + u8 len = 0;
> + int i = 0;
> +
> + if (reg == PMBUS_REG_MFR_FAULTY_SPS) {
> + int to_print = 0;
> +
> + len = 7;
> + ret = fd5121_read_block_data(client, reg, len, buffer);
> + if (ret < 0)
> + return ret;
> + printed = 0;
> + to_print = (ret < len) ? ret : len;
> + for (i = 0; i < to_print; i++)
> + printed += scnprintf(buf + printed,
> + PAGE_SIZE - printed,
> + "%02x", buffer[i]);
> + printed += scnprintf(buf + printed,
> + PAGE_SIZE - printed, "\n");
> + return printed;
> + } else if (reg == PMBUS_REG_BBR_RAM ||
> + reg == PMBUS_REG_BBR_OTP) {
> + u32 len = (reg == PMBUS_REG_BBR_OTP) ? 165 : 164;
> +
> + /* Extra byte may be needed in case we need to store
> + * the length of the data
> + */
> + u8 *tmp_in = kcalloc(len+1, sizeof(u8), GFP_KERNEL);
> +
> + if (tmp_in == NULL)
> + return -ENOMEM;
> + ret = fd5121_read_block_data(client, reg, len, tmp_in);
> + if (ret < 0) {
> + kfree(tmp_in);
> + return ret;
> + }
> +
> + printed = 0;
> + for (i = 0; i < ret; i++)
> + printed += scnprintf(buf + printed,
> + PAGE_SIZE - printed, "%02x",
> + tmp_in[i]);
> + printed += scnprintf(buf + printed,
> + PAGE_SIZE - printed, "\n");
> +
> + kfree(tmp_in);
> + return printed;
> + } else
> + return -ENODATA;
> +}
> +
> +static SENSOR_DEVICE_ATTR_RW(page, fd5121_byte,
> + PMBUS_PAGE);
> +static SENSOR_DEVICE_ATTR_RO(vout_raw, fd5121_word,
> + PMBUS_READ_VOUT);
> +static SENSOR_DEVICE_ATTR_RW(operation, fd5121_byte,
> + PMBUS_OPERATION);
> +static SENSOR_DEVICE_ATTR_RW(on_off_config, fd5121_byte,
> + PMBUS_ON_OFF_CONFIG);
> +static SENSOR_DEVICE_ATTR_WO(clear_faults, fd5121_byte,
> + PMBUS_CLEAR_FAULTS);
> +static SENSOR_DEVICE_ATTR_RW(write_protect, fd5121_byte,
> + PMBUS_WRITE_PROTECT);
> +static SENSOR_DEVICE_ATTR_RO(capability, fd5121_byte,
> + PMBUS_CAPABILITY);
> +static SENSOR_DEVICE_ATTR_RW(smbalert_mask, fd5121_word,
> + PMBUS_SMBALERT_MASK);
> +static SENSOR_DEVICE_ATTR_RO(vout_mode, fd5121_byte,
> + PMBUS_VOUT_MODE);
> +static SENSOR_DEVICE_ATTR_RW(vout_command, fd5121_word,
> + PMBUS_VOUT_COMMAND);
> +static SENSOR_DEVICE_ATTR_RW(vout_max, fd5121_word,
> + PMBUS_VOUT_MAX);
> +static SENSOR_DEVICE_ATTR_RW(vout_margin_high, fd5121_word,
> + PMBUS_VOUT_MARGIN_HIGH);
> +static SENSOR_DEVICE_ATTR_RW(vout_margin_low, fd5121_word,
> + PMBUS_VOUT_MARGIN_LOW);
> +static SENSOR_DEVICE_ATTR_RW(vout_transition_rate, fd5121_word,
> + PMBUS_VOUT_TRANSITION_RATE);
> +static SENSOR_DEVICE_ATTR_RW(vout_min, fd5121_word,
> + PMBUS_REG_VOUT_MIN);
> +static SENSOR_DEVICE_ATTR_RW(power_mode, fd5121_byte,
> + PMBUS_REG_POWER_MODE);
> +static SENSOR_DEVICE_ATTR_RW(vin_on, fd5121_word,
> + PMBUS_REG_VIN_ON);
> +static SENSOR_DEVICE_ATTR_RW(vin_off, fd5121_word,
> + PMBUS_REG_VIN_OFF);
> +static SENSOR_DEVICE_ATTR_RW(vin_uv_fault_response, fd5121_byte,
> + PMBUS_REG_VIN_UV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(iin_oc_fault_response, fd5121_byte,
> + PMBUS_REG_IIN_OC_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(ton_delay, fd5121_word,
> + PMBUS_REG_TON_DELAY);
> +static SENSOR_DEVICE_ATTR_RW(pout_op_fault_response, fd5121_byte,
> + PMBUS_REG_POUT_OP_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RO(read_vaux, fd5121_word,
> + PMBUS_REG_READ_VAUX);
> +static SENSOR_DEVICE_ATTR_RW(iknee_set, fd5121_word,
> + PMBUS_REG_IKNEE_SET);
> +static SENSOR_DEVICE_ATTR_RW(pin_counter, fd5121_byte,
> + PMBUS_REG_PIN_COUNTER);
> +static SENSOR_DEVICE_ATTR_RW(vmin_aware, fd5121_word,
> + PMBUS_REG_VMIN_AWARE);
> +static SENSOR_DEVICE_ATTR_RW(vout_ov_fault_response, fd5121_byte,
> + PMBUS_VOUT_OV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vout_uv_fault_response, fd5121_byte,
> + PMBUS_VOUT_UV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(iout_oc_fault_response, fd5121_byte,
> + PMBUS_IOUT_OC_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(ot_fault_response, fd5121_byte,
> + PMBUS_OT_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vin_ov_fault_response, fd5121_byte,
> + PMBUS_VIN_OV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vaux_uv_fault_limit, fd5121_word,
> + PMBUS_REG_VAUX_UV_FAULT_LIMIT);
> +static SENSOR_DEVICE_ATTR_RW(vaux_ov_fault_limit, fd5121_word,
> + PMBUS_REG_VAUX_OV_FAULT_LIMIT);
> +static SENSOR_DEVICE_ATTR_RW(vaux_uv_fault_response, fd5121_byte,
> + PMBUS_REG_VAUX_UV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vaux_ov_fault_response, fd5121_byte,
> + PMBUS_REG_VAUX_OV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vaux_uv_warning, fd5121_word,
> + PMBUS_REG_VAUX_UV_WARNING);
> +static SENSOR_DEVICE_ATTR_RW(vaux_ov_warning, fd5121_word,
> + PMBUS_REG_VAUX_OV_WARNING);
> +static SENSOR_DEVICE_ATTR_RO(free_user_config_tables, fd5121_byte,
> + PMBUS_REG_MFR_FREE_USER_CONFIG_TABLES);
> +static SENSOR_DEVICE_ATTR_RW(address_table, fd5121_byte,
> + PMBUS_REG_MFR_ADDRESS_TABLE);
> +static SENSOR_DEVICE_ATTR_RW(status_onsemi, fd5121_word,
> + PMBUS_REG_MFR_STATUS_ONSEMI);
> +static SENSOR_DEVICE_ATTR_RO(status_byte, fd5121_byte,
> + PMBUS_STATUS_BYTE);
> +static SENSOR_DEVICE_ATTR_RO(status_cml, fd5121_byte,
> + PMBUS_STATUS_CML);
> +static SENSOR_DEVICE_ATTR_RO(status_other, fd5121_byte,
> + PMBUS_STATUS_OTHER);
> +static SENSOR_DEVICE_ATTR_RO(status_mfr_specific, fd5121_byte,
> + PMBUS_STATUS_MFR_SPECIFIC);
> +static SENSOR_DEVICE_ATTR_RO(revision, fd5121_byte,
> + PMBUS_REVISION);
> +static SENSOR_DEVICE_ATTR_RO(id, fd5121_long,
> + PMBUS_MFR_ID);
> +static SENSOR_DEVICE_ATTR_RO(model, fd5121_long,
> + PMBUS_MFR_MODEL);
> +static SENSOR_DEVICE_ATTR_RO(mfr_revision, fd5121_long,
> + PMBUS_MFR_REVISION);
> +static SENSOR_DEVICE_ATTR_RW(date, fd5121_word,
> + PMBUS_MFR_DATE);
> +static SENSOR_DEVICE_ATTR_RO(ic_device_id, fd5121_long,
> + PMBUS_IC_DEVICE_ID);
> +static SENSOR_DEVICE_ATTR_RO(ic_device_rev, fd5121_word,
> + PMBUS_IC_DEVICE_REV);
> +static SENSOR_DEVICE_ATTR_WO(unlock, fd5121_byte,
> + PMBUS_REG_MFR_UNLOCK);
> +static SENSOR_DEVICE_ATTR_RO(faulty_sps, fd5121_block,
> + PMBUS_REG_MFR_FAULTY_SPS);
> +static SENSOR_DEVICE_ATTR_RO(tlvr_faults, fd5121_word,
> + PMBUS_REG_TLVR_FAULTS);
> +static SENSOR_DEVICE_ATTR_RW(user_store_config_tab, fd5121_byte,
> + PMBUS_REG_MFR_USER_STORE_CONFIG_TAB);
> +static SENSOR_DEVICE_ATTR_RO(user_config_index, fd5121_byte,
> + PMBUS_REG_MFR_USER_CONFIG_INDEX);
> +static SENSOR_DEVICE_ATTR_RO(pwm_disconnection, fd5121_word,
> + PMBUS_REG_MFR_PWM_DISCONNECTION);
> +static SENSOR_DEVICE_ATTR_RO(vr_disconnection, fd5121_byte,
> + PMBUS_REG_MFR_VR_DISCONNECTION);
> +static SENSOR_DEVICE_ATTR_RW(ton_slew, fd5121_byte,
> + PMBUS_REG_MFR_TON_SLEW);
> +static SENSOR_DEVICE_ATTR_RW(toff_slew, fd5121_byte,
> + PMBUS_REG_MFR_TOFF_SLEW);
> +static SENSOR_DEVICE_ATTR_RW(rail_name, fd5121_word,
> + PMBUS_REG_MFR_RAIL_NAME);
> +static SENSOR_DEVICE_ATTR_RW(vout_droop, fd5121_byte,
> + PMBUS_REG_MFR_VOUT_DROOP);
> +static SENSOR_DEVICE_ATTR_WO(svr_go, fd5121_send_byte,
> + PMBUS_REG_MFR_SVR_GO);
> +static SENSOR_DEVICE_ATTR_RW(user_restore_config_tab, fd5121_byte,
> + PMBUS_REG_MFR_USER_RESTORE_CONFIG_TAB);
> +static SENSOR_DEVICE_ATTR_WO(set_pwd, fd5121_byte,
> + PMBUS_REG_MFR_SET_PWD);
> +static SENSOR_DEVICE_ATTR_RW(config_activate, fd5121_byte,
> + PMBUS_REG_MFR_CONFIG_ACTIVATE);
> +static SENSOR_DEVICE_ATTR_RW(config_recover, fd5121_byte,
> + PMBUS_REG_MFR_CONFIG_RECOVER);
> +static SENSOR_DEVICE_ATTR_RW(otp_dump, fd5121_byte,
> + PMBUS_REG_MFR_OTP_DUMP);
> +static SENSOR_DEVICE_ATTR_RW(bbr_en, fd5121_byte,
> + PMBUS_REG_MFR_BBR_EN);
> +static SENSOR_DEVICE_ATTR_RW(dpm_min, fd5121_byte,
> + PMBUS_REG_MFR_DPM_MIN);
> +static SENSOR_DEVICE_ATTR_RW(vboot, fd5121_word,
> + PMBUS_REG_MFR_VBOOT);
> +static SENSOR_DEVICE_ATTR_RW(preclamp_offset, fd5121_word,
> + PMBUS_REG_MFR_PRECLAMP_OFFSET);
> +static SENSOR_DEVICE_ATTR_RW(tlvr_diagn, fd5121_word,
> + PMBUS_REG_MFR_TLVR_DIAGN);
> +static SENSOR_DEVICE_ATTR_RO(vsys, fd5121_word,
> + PMBUS_REG_MFR_READ_VSYS);
> +static SENSOR_DEVICE_ATTR_RW(specific_e9, fd5121_word,
> + PMBUS_REG_MFR_SPECIFIC_E9);
> +static SENSOR_DEVICE_ATTR_RW(specific_ea, fd5121_long,
> + PMBUS_REG_MFR_SPECIFIC_EA);
> +static SENSOR_DEVICE_ATTR_RO(ss_cbc, fd5121_word,
> + PMBUS_REG_MFR_SS_CBC);
> +static SENSOR_DEVICE_ATTR_RO(amd_status, fd5121_byte,
> + PMBUS_REG_MFR_AMD_STATUS);
> +static SENSOR_DEVICE_ATTR_RO(checksum, fd5121_word,
> + PMBUS_REG_MFR_CHECKSUM);
> +static SENSOR_DEVICE_ATTR_RO(cse_index, fd5121_word,
> + PMBUS_REG_CSE_INDEX);
> +static SENSOR_DEVICE_ATTR_RW(cout_measure, fd5121_word,
> + PMBUS_REG_COUT_MEASURE);
> +static SENSOR_DEVICE_ATTR_RO(vr_cout, fd5121_word,
> + PMBUS_REG_VR_COUT);
> +static SENSOR_DEVICE_ATTR_RO(bbr_ram, fd5121_block,
> + PMBUS_REG_BBR_RAM);
> +static SENSOR_DEVICE_ATTR_RO(bbr_otp, fd5121_block,
> + PMBUS_REG_BBR_OTP);
> +static SENSOR_DEVICE_ATTR_RO(psys, fd5121_word,
> + PMBUS_REG_READ_PSYS);
> +static SENSOR_DEVICE_ATTR_RW(postclamp_offset, fd5121_word,
> + PMBUS_REG_POSTCLAMP_OFFSET);
> +static SENSOR_DEVICE_ATTR_RW(pgood_delay, fd5121_byte,
> + PMBUS_REG_PGOOD_DELAY);
> +static SENSOR_DEVICE_ATTR_RW(specific_f8, fd5121_word,
> + PMBUS_REG_MFR_SPECIFIC_F8);
> +static SENSOR_DEVICE_ATTR_RW(specific_f9, fd5121_long,
> + PMBUS_REG_MFR_SPECIFIC_F9);
> +static SENSOR_DEVICE_ATTR_RW(pwd_program_ram, fd5121_word,
> + PMBUS_REG_MFR_PWD_PROGRAM_RAM);
> +static SENSOR_DEVICE_ATTR_RW(pwd_program_i2c, fd5121_word,
> + PMBUS_REG_MFR_PWD_PROGRAM_I2C);
> +static SENSOR_DEVICE_ATTR_RW(pwd_enable_otp_store, fd5121_word,
> + PMBUS_REG_MFR_PWD_ENABLE_OTP_STORE);
> +
> +static struct attribute *fd5121_non_paged_attrs[] = {
> + &sensor_dev_attr_page.dev_attr.attr,
> + &sensor_dev_attr_capability.dev_attr.attr,
> + &sensor_dev_attr_pin_counter.dev_attr.attr,
> + &sensor_dev_attr_vaux_uv_fault_limit.dev_attr.attr,
> + &sensor_dev_attr_vaux_ov_fault_limit.dev_attr.attr,
> + &sensor_dev_attr_vaux_uv_warning.dev_attr.attr,
> + &sensor_dev_attr_vaux_ov_warning.dev_attr.attr,
> + &sensor_dev_attr_free_user_config_tables.dev_attr.attr,
> + &sensor_dev_attr_address_table.dev_attr.attr,
> + &sensor_dev_attr_unlock.dev_attr.attr,
> + &sensor_dev_attr_faulty_sps.dev_attr.attr,
> + &sensor_dev_attr_tlvr_faults.dev_attr.attr,
> + &sensor_dev_attr_user_store_config_tab.dev_attr.attr,
> + &sensor_dev_attr_user_config_index.dev_attr.attr,
> + &sensor_dev_attr_pwm_disconnection.dev_attr.attr,
> + &sensor_dev_attr_vr_disconnection.dev_attr.attr,
> + &sensor_dev_attr_user_restore_config_tab.dev_attr.attr,
> + &sensor_dev_attr_svr_go.dev_attr.attr,
> + &sensor_dev_attr_set_pwd.dev_attr.attr,
> + &sensor_dev_attr_config_activate.dev_attr.attr,
> + &sensor_dev_attr_config_recover.dev_attr.attr,
> + &sensor_dev_attr_otp_dump.dev_attr.attr,
> + &sensor_dev_attr_bbr_en.dev_attr.attr,
> + &sensor_dev_attr_vboot.dev_attr.attr,
> + &sensor_dev_attr_vsys.dev_attr.attr,
> + &sensor_dev_attr_specific_e9.dev_attr.attr,
> + &sensor_dev_attr_specific_ea.dev_attr.attr,
> + &sensor_dev_attr_ss_cbc.dev_attr.attr,
> + &sensor_dev_attr_checksum.dev_attr.attr,
> + &sensor_dev_attr_cse_index.dev_attr.attr,
> + &sensor_dev_attr_cout_measure.dev_attr.attr,
> + &sensor_dev_attr_vr_cout.dev_attr.attr,
> + &sensor_dev_attr_bbr_ram.dev_attr.attr,
> + &sensor_dev_attr_bbr_otp.dev_attr.attr,
> + &sensor_dev_attr_psys.dev_attr.attr,
> + &sensor_dev_attr_specific_f8.dev_attr.attr,
> + &sensor_dev_attr_specific_f9.dev_attr.attr,
> + &sensor_dev_attr_pwd_program_ram.dev_attr.attr,
> + &sensor_dev_attr_pwd_program_i2c.dev_attr.attr,
> + &sensor_dev_attr_pwd_enable_otp_store.dev_attr.attr,
> + &sensor_dev_attr_revision.dev_attr.attr,
> + &sensor_dev_attr_id.dev_attr.attr,
> + &sensor_dev_attr_model.dev_attr.attr,
> + &sensor_dev_attr_mfr_revision.dev_attr.attr,
> + &sensor_dev_attr_date.dev_attr.attr,
> + &sensor_dev_attr_ic_device_id.dev_attr.attr,
> + &sensor_dev_attr_ic_device_rev.dev_attr.attr,
> + NULL
> +};
> +
> +static struct attribute *fd5121_paged_attrs[] = {
> + &sensor_dev_attr_operation.dev_attr.attr,
> + &sensor_dev_attr_vout_raw.dev_attr.attr,
> + &sensor_dev_attr_on_off_config.dev_attr.attr,
> + &sensor_dev_attr_clear_faults.dev_attr.attr,
> + &sensor_dev_attr_write_protect.dev_attr.attr,
> + &sensor_dev_attr_smbalert_mask.dev_attr.attr,
> + &sensor_dev_attr_vout_mode.dev_attr.attr,
> + &sensor_dev_attr_vout_command.dev_attr.attr,
> + &sensor_dev_attr_vout_margin_high.dev_attr.attr,
> + &sensor_dev_attr_vout_margin_low.dev_attr.attr,
> + &sensor_dev_attr_vout_min.dev_attr.attr,
> + &sensor_dev_attr_vin_on.dev_attr.attr,
> + &sensor_dev_attr_vin_off.dev_attr.attr,
> + &sensor_dev_attr_vout_ov_fault_response.dev_attr.attr,
> + &sensor_dev_attr_vout_uv_fault_response.dev_attr.attr,
> + &sensor_dev_attr_iout_oc_fault_response.dev_attr.attr,
> + &sensor_dev_attr_ot_fault_response.dev_attr.attr,
> + &sensor_dev_attr_vin_ov_fault_response.dev_attr.attr,
> + &sensor_dev_attr_status_byte.dev_attr.attr,
> + &sensor_dev_attr_iknee_set.dev_attr.attr,
> + &sensor_dev_attr_vmin_aware.dev_attr.attr,
> + &sensor_dev_attr_power_mode.dev_attr.attr,
> + &sensor_dev_attr_vin_uv_fault_response.dev_attr.attr,
> + &sensor_dev_attr_iin_oc_fault_response.dev_attr.attr,
> + &sensor_dev_attr_ton_delay.dev_attr.attr,
> + &sensor_dev_attr_pout_op_fault_response.dev_attr.attr,
> + &sensor_dev_attr_vaux_uv_fault_response.dev_attr.attr,
> + &sensor_dev_attr_vaux_ov_fault_response.dev_attr.attr,
> + &sensor_dev_attr_status_onsemi.dev_attr.attr,
> + &sensor_dev_attr_status_cml.dev_attr.attr,
> + &sensor_dev_attr_status_other.dev_attr.attr,
> + &sensor_dev_attr_status_mfr_specific.dev_attr.attr,
> + &sensor_dev_attr_read_vaux.dev_attr.attr,
> + &sensor_dev_attr_ton_slew.dev_attr.attr,
> + &sensor_dev_attr_toff_slew.dev_attr.attr,
> + &sensor_dev_attr_rail_name.dev_attr.attr,
> + &sensor_dev_attr_vout_droop.dev_attr.attr,
> + &sensor_dev_attr_dpm_min.dev_attr.attr,
> + &sensor_dev_attr_preclamp_offset.dev_attr.attr,
> + &sensor_dev_attr_tlvr_diagn.dev_attr.attr,
> + &sensor_dev_attr_amd_status.dev_attr.attr,
> + &sensor_dev_attr_postclamp_offset.dev_attr.attr,
> + &sensor_dev_attr_pgood_delay.dev_attr.attr,
> + &sensor_dev_attr_vout_max.dev_attr.attr,
> + &sensor_dev_attr_vout_transition_rate.dev_attr.attr,
> + NULL
This is a complete no-go. We do not explose raw register data as sysfs
attributes. You may expose essential register data as debugfs files,
but only those deemed necessary. The above is just "let's blindly
expose everything". Most of the above should be programmed in manufacturing
and not be touched subsequently, much less as writeable attributes.
Writing bad/unexpected values into many of those attributes can turn
a board into a brick. It is bad enough that/if this is even possible,
but exposing it as sysfs attribute would be a terrible idea.
I am not going to review this driver any further at this point.
Guenter
> +};
> +
> +static struct attribute_group fd5121_groups[2] = {
> + { .name = "global", .attrs = fd5121_non_paged_attrs },
> + { .name = "paged", .attrs = fd5121_paged_attrs }
> +};
> +
> +/* Regulator descriptors for VOUT rails (VID encoded) */
> +static struct regulator_desc fd5121_reg_desc[] = {
> + PMBUS_REGULATOR_STEP_ONE("vout1", 3001, 1000, 200000),
> + PMBUS_REGULATOR_STEP_ONE("vout2", 3001, 1000, 200000),
> +};
> +
> +static int fd5121_valid_reg(struct i2c_client *client, int reg)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(cc_list); i++) {
> + if (reg == cc_list[i])
> + return 0;
> + }
> +
> + if (fd5121_blk_rd_reg(reg))
> + return 0;
> + return -ENXIO;
> +}
> +
> +static int fd5121_read_word_data(struct i2c_client *client, int page,
> + int phase, int reg)
> +{
> + int ret;
> +
> + ret = fd5121_valid_reg(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + ret = pmbus_read_word_data(client, page, phase, reg);
> + if (ret < 0)
> + return ret;
> +
> + /* Chip reports VOUT_MODE as vid. But gives raw value 1mV per bit.
> + * So, encode the READ_VOUT value so that it gets decoded and
> + * reported correctly.
> + */
> + if (reg == PMBUS_READ_VOUT)
> + ret = DIV_ROUND_CLOSEST(155000 - ret * 100, 625);
> + return ret;
> +}
> +
> +static int fd5121_read_byte_data(struct i2c_client *client, int page, int reg)
> +{
> + int ret;
> +
> + ret = fd5121_valid_reg(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + return pmbus_read_byte_data(client, page, reg);
> +}
> +
> +static int fd5121_write_byte_data(struct i2c_client *client, int page,
> + int reg, u8 value)
> +{
> + int ret;
> +
> + ret = fd5121_valid_reg(client, reg);
> + if (ret < 0)
> + return ret;
> + return pmbus_write_byte_data(client, page, reg, value);
> +}
> +
> +static int fd5121_write_byte(struct i2c_client *client, int page, u8 byte)
> +{
> + return pmbus_write_byte(client, page, byte);
> +}
> +
> +static int fd5121_write_word_data(struct i2c_client *client, int page,
> + int reg, u16 word)
> +{
> + int ret;
> +
> + ret = fd5121_valid_reg(client, reg);
> + if (ret < 0)
> + return ret;
> + ret = pmbus_write_word_data(client, page, reg, word);
> + return ret;
> +}
> +
> +static u32 fd5121_get_dev_id(struct i2c_client *client)
> +{
> + u32 dev_id = 0;
> + s32 ret = 0;
> +
> + ret = fd5121_read_long(client, PMBUS_IC_DEVICE_ID, &dev_id);
> + if (ret < 0)
> + return CTLR_ID_UNKNOWN;
> +
> + switch (dev_id) {
> + case CTLR_ID_FD5121:
> + case CTLR_ID_FD5123:
> + case CTLR_ID_FD5125:
> + break;
> + default:
> + if (dev_id != 0)
> + dev_err(&client->dev, "Unknown device 0x%x",
> + dev_id);
> + return CTLR_ID_UNKNOWN;
> + }
> + return dev_id;
> +}
> +
> +static int fd5121_probe(struct i2c_client *client)
> +{
> + struct pmbus_driver_info *info;
> + struct fd5121_data *pdata;
> + u32 id;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> + return -EOPNOTSUPP;
> +
> + pdata = devm_kzalloc(&client->dev, sizeof(struct fd5121_data),
> + GFP_KERNEL);
> + if (pdata == NULL)
> + return -ENOMEM;
> +
> + pdata->dev = &client->dev;
> + pdata->groups[0] = &fd5121_groups[0];
> + pdata->groups[1] = &fd5121_groups[1];
> +
> + id = fd5121_get_dev_id(client);
> + if (id == CTLR_ID_UNKNOWN)
> + return -ENODEV;
> +
> + pdata->id = id;
> +
> + switch (id) {
> + case CTLR_ID_FD5121:
> + case CTLR_ID_FD5123:
> + case CTLR_ID_FD5125:
> + break;
> + default:
> + dev_err(&client->dev, "Failed to read device ID");
> + return -ENODEV;
> + }
> +
> + info = &pdata->info;
> + info->groups = (const struct attribute_group **)&pdata->groups[0];
> + info->write_word_data = fd5121_write_word_data;
> + info->write_byte = fd5121_write_byte;
> + info->write_byte_data = fd5121_write_byte_data;
> + info->read_word_data = fd5121_read_word_data;
> + info->read_byte_data = fd5121_read_byte_data;
> +
> + info->pages = FD5121_NUM_PAGES;
> + info->format[PSC_VOLTAGE_IN] = linear;
> + info->format[PSC_VOLTAGE_OUT] = vid;
> +
> + fd5121_reg_desc[0].id = 0;
> + fd5121_reg_desc[1].id = 1;
> +
> + /* Device implements VID coding with 1 mV steps from 0.200 V
> + * up to 3.200 V
> + */
> + info->num_regulators = FD5121_NUM_PAGES;
> + info->reg_desc = fd5121_reg_desc;
> + info->format[PSC_CURRENT_IN] = linear;
> + info->format[PSC_CURRENT_OUT] = linear;
> + info->format[PSC_POWER] = linear;
> + info->format[PSC_TEMPERATURE] = linear;
> + for (u8 idx = 0; idx < info->pages; idx++) {
> + info->func[idx] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT;
> + info->func[idx] |= PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
> + info->func[idx] |= PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
> + info->func[idx] |= PMBUS_HAVE_PIN | PMBUS_HAVE_POUT;
> + info->func[idx] |= PMBUS_HAVE_VIN | PMBUS_HAVE_IIN;
> + info->func[idx] |= PMBUS_HAVE_STATUS_INPUT;
> + info->vrm_version[idx] = amd625mv;
> + }
> + return pmbus_do_probe(client, info);
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id fd5121_of_match[] = {
> + { .compatible = "onnn,fd5121" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, fd5121_of_match);
> +#endif
> +
> +static const struct i2c_device_id fd5121_id[] = {
> + { "fd5121", chip_fd5121 },
> + { "fd5123", chip_fd5123 },
> + { "fd5125", chip_fd5125 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, fd5121_id);
> +
> +static struct i2c_driver fd5121_driver = {
> + .driver = {
> + .name = "fd5121",
> + .of_match_table = of_match_ptr(fd5121_of_match),
> + },
> + .probe = fd5121_probe,
> + .id_table = fd5121_id,
> +};
> +
> +module_i2c_driver(fd5121_driver);
> +
> +MODULE_AUTHOR("Selva Rajagopal <selvamani.rajagopal@onsemi.com>");
> +MODULE_DESCRIPTION("PMBus driver for FD5121");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("PMBUS");
> +
>
^ permalink raw reply
* Re: [PATCH RFC] dt-bindings: sound: Convert cirrus,cs35l36 to DT schema
From: Rhodes, David @ 2026-06-23 22:00 UTC (permalink / raw)
To: David Heidelberg, David Rhodes, Richard Fitzgerald, Liam Girdwood,
Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
patches, Bjorn Helgaas
Cc: linux-sound, devicetree, linux-kernel, phone-devel
In-Reply-To: <513dd76c-586e-48e3-8542-5f42d35c7929@ixit.cz>
On 6/21/26 7:27 AM, David Heidelberg wrote:
> On 18/06/2026 23:38, Rhodes, David wrote:
>> On 6/18/26 9:33 AM, David Heidelberg via B4 Relay wrote:
>>> From: David Heidelberg <david@ixit.cz>
>>>
>>> Convert CS35L36 Speaker Amplifier.
>>>
>>> Changes:
>>> - maintainers email to the generic Cirrus email
>>> - Both the codec and downstream worked just fine without
>>> VP-supply provided. Align with datasheet for similar models.
>>> - add dai-common.yaml to cover for '#sound-dai-cells',
>>> 'sound-name-prefix'
>>>
>>> Co-developed-by: Rob Herring (Arm) <robh@kernel.org>
>>> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
>>> Signed-off-by: David Heidelberg <david@ixit.cz>
>>> ---
>>
>> Reviewed-by: David Rhodes <David.Rhodes@cirrus.com>
>
> Thanks for the review. btw. could you please verify against datasheet,
> if the
>
> - cirrus,classh-wk-fet-delay is also possible for cs35l36 (l35 has it,
> but l36 driver doesn't, thou DT isn't about driver, so checking)
> - cirrus,weak-fet-disable and cirrus,classh-weak-fet-thld-millivolt
> are also not implemented in any driver, but present in DT.
>
> If hardware is capable and the driver just doesn't have impl. it's fine,
> but otherwise I would remove these in v2.
>
> Thank you
> David :D
>
Hardware is capable and the driver does not have implementation.
This would be the register referred to as CS35L36_CLASSH_FET_DRV_CFG.
Thanks,
David
^ permalink raw reply
* Re: [PATCH v2 5/5] riscv: dts: spacemit: improve RTL8211F PHY configuration on K3 Pico-ITX board
From: sashiko-bot @ 2026-06-23 21:39 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260623204431.498700-6-aurelien@aurel32.net>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Adding vendor-specific properties (`realtek,aldps-enable` and `realtek,clkout-disable`) to a PHY node with a generic `compatible` string bypasses DT schema validation and perpetuates probing fragility.
--
commit 06c0a01ebf26666aceea3d45a75bbed3025600af
Author: Aurelien Jarno <aurelien@aurel32.net>
riscv: dts: spacemit: improve RTL8211F PHY configuration on K3 Pico-ITX board
This commit enables ALDPS and disables the clkout signal on the RTL8211F
PHY for the K3 Pico-ITX board to save power and match hardware schematics.
> diff --git a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
> index 532c72006ef1f..2476f5585a200 100644
> --- a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
> +++ b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
> @@ -226,6 +226,8 @@ phy0: phy@1 {
> reset-gpios = <&gpio 0 15 GPIO_ACTIVE_LOW>;
> reset-assert-us = <10000>;
> reset-deassert-us = <10000>;
> + realtek,aldps-enable;
> + realtek,clkout-disable;
[Severity: Medium]
Will adding these vendor-specific properties without specifying a Realtek
PHY ID in the compatible string bypass device tree schema validation?
The realtek,rtl82xx.yaml schema is only selected if a Realtek PHY ID is
present. Since the phy0 node only specifies ethernet-phy-ieee802.3-c22, the
validation tools will ignore the new Realtek-specific properties, meaning
typos or invalid types could pass silently during dtbs_check.
Also, since this node uses reset-gpios, if the PHY is kept in reset by
hardware or the bootloader, could the MDIO framework fail to read its ID
and probe it unless the exact PHY ID is provided in the compatible string?
Would it be better to update the compatible string to include the specific
PHY ID (such as ethernet-phy-id001c.c916) alongside the generic one to
ensure proper schema validation and reliable probing?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623204431.498700-1-aurelien@aurel32.net?part=5
^ permalink raw reply
* Re: [PATCH v2 3/4] drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support
From: Amit Barzilai @ 2026-06-23 21:34 UTC (permalink / raw)
To: markus.elfring
Cc: airlied, amit.barzilai22, andy, azuddinadam, chintanlike,
conor+dt, deller, devicetree, dri-devel, gregkh, javierm, krzk+dt,
linux-fbdev, linux-kernel, linux-staging, maarten.lankhorst,
mripard, robh, simona, tzimmermann
In-Reply-To: <16a86f3d-caf0-46d0-97a4-c9585bdaa06c@web.de>
Markus Elfring <Markus.Elfring@web.de> writes:
>> +++ b/drivers/gpu/drm/solomon/ssd130x.c
>> @@ -146,6 +146,33 @@
>> #define SSD133X_COLOR_DEPTH_256 0x0
>> #define SSD133X_COLOR_DEPTH_65K 0x1
>>
>> +/* ssd135x commands */
>> +#define SSD135X_SET_COL_RANGE 0x15
>> +#define SSD135X_WRITE_RAM 0x5c
>> +#define SSD135X_SET_ROW_RANGE 0x75
> [...]
>
> How do you think about to use an enumeration for such data?
> https://en.wikipedia.org/wiki/Enumerated_type#C_and_syntactically_similar_languages
Thank you for the suggestion.
I used #define to stay consistent with the rest of ssd130x.c, where the
command constants for the other families are all defined the same way.
In my opinion an enum could be a readable solution for these values, but I
don't think the switch should be included in this series.
--
Thanks,
Amit
^ permalink raw reply
* Re: [PATCH v3 3/3] iio: magnetometer: add driver for QST QMC5883L Sensor
From: Andy Shevchenko @ 2026-06-23 21:31 UTC (permalink / raw)
To: Siratul Islam
Cc: jic23, robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy,
linux-iio, devicetree, linux-kernel
In-Reply-To: <20260619104524.10172-4-siratul.islam@linux.dev>
On Fri, Jun 19, 2026 at 04:45:06PM +0600, Siratul Islam wrote:
> Add driver for the QST QMC5883L 3-Axis Magnetic Sensor
> connected via i2c.
LGTM now,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v5 2/3] iio: health: add MAX86150 ECG and PPG biosensor driver
From: sashiko-bot @ 2026-06-23 21:30 UTC (permalink / raw)
To: Md Shofiqul Islam; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260623201124.18271-3-shofiqtest@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Hardware FIFO overrun and data corruption due to incorrect burst read length.
- [High] Infinite IRQ storm on level-triggered interrupts due to improper IRQF_ONESHOT handling, and potential DoS via missing .validate_device.
- [Medium] IIO trigger reference leak on probe error path.
--
commit 9590a122a97de73064d5ca132b0611828fac557d
Author: Md Shofiqul Islam <shofiqtest@gmail.com>
iio: health: add MAX86150 ECG and PPG biosensor driver
Add a new IIO driver for the Analog Devices MAX86150 integrated
biosensor, which combines two PPG optical channels (Red/IR LED) and
one ECG biopotential channel in a single I2C device.
> diff --git a/drivers/iio/health/max86150.c b/drivers/iio/health/max86150.c
> new file mode 100644
> index 0000000000000..92bf31f0a7615
> --- /dev/null
> +++ b/drivers/iio/health/max86150.c
[ ... ]
> +static int max86150_read_one_sample(struct max86150_data *data,
> + u32 *ppg_red, u32 *ppg_ir, s32 *ecg)
> +{
> + int ret;
> +
> + /*
> + * Use data->fifo_raw (heap memory) not a local array so the buffer is
> + * DMA-mappable for I2C host controllers that use DMA for burst reads.
> + */
> + ret = regmap_noinc_read(data->regmap, MAX86150_REG_FIFO_DATA,
> + data->fifo_raw, sizeof(data->fifo_raw));
[Severity: High]
Since data->fifo_raw is padded to ARCH_DMA_MINALIGN (e.g., 64 or 128 bytes)
for DMA safety, will sizeof(data->fifo_raw) cause the I2C burst read to pop
64 or 128 bytes from the hardware FIFO instead of the intended 9 bytes?
Could this destroy the 3-byte slot alignment and permanently drop data?
> + if (ret)
> + return ret;
[ ... ]
> +static const struct iio_trigger_ops max86150_trigger_ops = {
> + .set_trigger_state = max86150_set_trigger_state,
> +};
[Severity: High]
Does the absence of .validate_device here allow external IIO devices to attach
to this trigger?
If they do attach, could that prevent the consumer thread from clearing this
device's interrupt, leading to a permanent IRQ storm?
[ ... ]
> + irq_trig = irq_get_trigger_type(client->irq);
> + if (!irq_trig)
> + irq_trig = IRQF_TRIGGER_FALLING;
> +
> + ret = devm_request_threaded_irq(&client->dev, client->irq,
> + NULL,
> + iio_trigger_generic_data_rdy_poll,
> + irq_trig | IRQF_ONESHOT,
> + "max86150", data->trig);
[Severity: High]
For level-triggered interrupts, will IRQF_ONESHOT correctly mask the line
until the hardware is cleared?
Looking at iio_trigger_generic_data_rdy_poll(), it acts as the primary
threaded handler and returns IRQ_HANDLED immediately after scheduling the
consumer thread.
Doesn't returning IRQ_HANDLED unmask the interrupt line before
max86150_trigger_handler() has a chance to read the hardware registers and
clear the source, causing an immediate IRQ storm?
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot request IRQ %d\n",
> + client->irq);
> +
> + ret = devm_iio_trigger_register(&client->dev, data->trig);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot register trigger\n");
> +
> + /*
> + * Set the default trigger so userspace can enable the buffer
> + * without a manual current_trigger write. iio_device_unregister()
> + * calls iio_trigger_put() to release this reference.
> + */
> + indio_dev->trig = iio_trigger_get(data->trig);
> + }
> +
> + ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev,
> + iio_pollfunc_store_time,
> + max86150_trigger_handler,
> + NULL);
[Severity: Medium]
If devm_iio_triggered_buffer_setup() fails here, is the trigger reference
acquired by iio_trigger_get() leaked?
Since INDIO_BUFFER_TRIGGERED is not set when buffer setup fails,
iio_dev_release() will skip calling iio_device_unregister_trigger_consumer().
Does this mean iio_trigger_put() is never called to clean up this reference?
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot setup triggered buffer\n");
> +
> + return devm_iio_device_register(&client->dev, indio_dev);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623201124.18271-1-shofiqtest@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v5 1/3] dt-bindings: iio: health: add adi,max86150
From: sashiko-bot @ 2026-06-23 21:17 UTC (permalink / raw)
To: Md Shofiqul Islam; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260623201124.18271-2-shofiqtest@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The description for the `interrupts` property explicitly states it is an 'Active-low interrupt line', but the example uses `IRQ_TYPE_EDGE_FALLING`.
--
commit 842884bb765a65ccd68dc98c1bab3a30ca22640a
Author: Md Shofiqul Islam <shofiqtest@gmail.com>
dt-bindings: iio: health: add adi,max86150
Add Device Tree binding schema for the Analog Devices MAX86150
integrated ECG and PPG biosensor.
> diff --git a/Documentation/devicetree/bindings/iio/health/adi,max86150.yaml b/Documentation/devicetree/bindings/iio/health/adi,max86150.yaml
> new file mode 100644
> index 0000000000000..c191f4f1525b1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/health/adi,max86150.yaml
> @@ -0,0 +1,78 @@
[ ... ]
> + interrupts:
> + maxItems: 1
> + description: |
> + Active-low interrupt line. Asserted when the FIFO almost-full
> + threshold is reached or when a new PPG sample is ready.
[ ... ]
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + heart-rate@5e {
> + compatible = "adi,max86150";
> + reg = <0x5e>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
[Severity: Low]
The property description states this is an "Active-low interrupt line",
which typically corresponds to IRQ_TYPE_LEVEL_LOW. Does the example use
IRQ_TYPE_EDGE_FALLING intentionally, or should it be updated to match the
description to avoid misleading developers who might copy this example?
> + vdd-supply = <&vdd_1v8>;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623201124.18271-1-shofiqtest@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
From: Conor Dooley @ 2026-06-23 21:14 UTC (permalink / raw)
To: Selvamani Rajagopal
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-hwmon@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <CYYPR02MB98280DF78A07EADACFD084EE83EE2@CYYPR02MB9828.namprd02.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 1717 bytes --]
On Tue, Jun 23, 2026 at 09:01:32PM +0000, Selvamani Rajagopal wrote:
>
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Subject: Re: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
> >
> >
> > > +
> > > +title: onsemi's multi-phase digital controllers
> >
> > Can someone explain to me what a "digital controller" actually is?
> > Seems very generi and that a word may have been left out, were it not
> > for the fact that this wording is used several times in the patch.
> >
>
> Thanks for reviewing.
>
> According to me, "digital controller" means the controller uses digital circuits to implement
> the features and functionality. We can remove "digital" and keep only controller. It won't make any
> difference for Linux documentation.
My point is that what's actually being controlled is missing. Maybe it
is obvious to you, but it is not to me. Your nodename in your example is
> + fd5121@50 {
which doesn't comply with node naming requirements and I wanted to come
up with a suggestion for what it should be.
I am assuming that its power or voltage that you're controlling so
either it should be hwmon@ or regulator@.
>
> > > +
> > > + enum:
> > > + - onnn,fd5121
> > > + - onnn,fd5123
> > > + - onnn,fd5125
> >
> > Your /OF/ match data in your driver suggests that you intended to permit
> > fallback compatibles here?
>
> Agree. Sorry about the discrepancy. Will fix it.
>
> >
> > |+#ifdef CONFIG_OF
> > |+static const struct of_device_id fd5121_of_match[] = {
> > |+ { .compatible = "onnn,fd5121" },
> > |+ { }
> > |+};
> > |+MODULE_DEVICE_TABLE(of, fd5121_of_match);
> > |+#endif
> >
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 07/12] rtc: rzn1: fix alarm range check truncation on 32-bit systems
From: Wolfram Sang @ 2026-06-23 21:08 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <CA+V-a8u2wt6623mYjhipOvJPo4va+bXs3qirQewocFr2QmUFhA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 423 bytes --]
> Can you please share the commands you tried, I'll try and replicate it
> on my side.
Sorry, can't give you the commands, just from my head: I tried to set an
alarm more than a week in the future, and the alarm was set to the next
day. But I was in a hurry, maybe I overlooked something, because that
handling used to work in the past IIRC. I can return to this topic on
Friday earliest, sadly. Maybe next week only...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 0/3] iio: health: add MAX86150 ECG and PPG biosensor driver
From: David Lechner @ 2026-06-23 21:04 UTC (permalink / raw)
To: Md Shofiqul Islam, linux-iio
Cc: jic23, nuno.sa, robh, krzk+dt, conor+dt, devicetree,
joshua.crofts1
In-Reply-To: <20260623155556.13701-1-shofiqtest@gmail.com>
On 6/23/26 10:55 AM, Md Shofiqul Islam wrote:
> The MAX86150 (Maxim/Analog Devices) combines two PPG optical channels
> (Red/IR LED) and one ECG biopotential channel in a single I2C device.
> Data is produced at up to 3200 Hz and buffered in a 32-entry hardware
> FIFO with a configurable almost-full interrupt.
>
> This series adds:
> 1. DT binding YAML schema for maxim,max86150
> 2. IIO driver with triggered buffer support
> 3. MAINTAINERS entry
>
> Changes in v2 (addressing Sashiko AI review, Joshua Crofts, Krzysztof Kozlowski):
Please don't send revisions in reply to previous series. It breaks
many workflows and tools that expect each series to be it's own
message thread.
Also, when submitting a series for a new driver, please wait at
least one week to gather feedback before sending the next revision.
^ permalink raw reply
* Re: [PATCH 8/9] drm/rockchip: dw_hdmi_qp: Enable YUV420 output format
From: sashiko-bot @ 2026-06-23 21:03 UTC (permalink / raw)
To: Cristian Ciocaltea; +Cc: dri-devel, robh, conor+dt, devicetree
In-Reply-To: <20260617-dw-hdmi-qp-yuv-v1-8-a665cfd06d7d@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Hardware State Mutation in Atomic Check
--
commit 88f697e7fc21ab15ca1c009f0f830ab90e6ec4e7
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
drm/rockchip: dw_hdmi_qp: Enable YUV420 output format
Both RK3576 and RK3588 SoCs are capable of driving the YUV420 output
color format, and the required bus-format handling and VOP2 support are
already in place. Advertise it via the platform supported formats so
the HDMI core can select it.
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 3a1c027aa90bb..e29522afc6f05 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -612,7 +612,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>
> plat_data.supported_formats = BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
> BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) |
> - BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422);
> + BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) |
> + BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420);
[Severity: High]
This is a pre-existing issue, but I noticed that adding new formats might
exercise the atomic check path more frequently.
Looking at dw_hdmi_qp_rockchip_encoder_atomic_check() in this same file,
it appears to mutate hardware state during the atomic validation phase:
drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c:dw_hdmi_qp_rockchip_encoder_atomic_check() {
...
ret = phy_configure(hdmi->phy, &phy_cfg);
if (!ret) {
hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
s->output_type = DRM_MODE_CONNECTOR_HDMIA;
s->output_bpc = conn_state->hdmi.output_bpc;
s->bus_format = ingest_fmt;
} else {
...
}
And in the PHY driver, phy_configure() mutates global state:
drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c:rk_hdptx_phy_configure() {
...
ret = rk_hdptx_phy_verify_hdmi_config(hdptx, &opts->hdmi, &hdptx->hdmi_cfg);
if (ret) {
dev_err(hdptx->dev, "invalid hdmi params for phy configure\n");
} else {
hdptx->restrict_rate_change = true;
...
}
Does this sequence cause problems when a userspace application (like a display
compositor) submits a test-only commit using the DRM_MODE_ATOMIC_TEST_ONLY
flag?
Since the atomic check runs for test-only commits, won't this corrupt the
active driver state and potentially break subsequent real commits if they skip
necessary hardware configurations due to the cached state already matching?
>
> encoder = &hdmi->encoder.encoder;
> encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-dw-hdmi-qp-yuv-v1-0-a665cfd06d7d@collabora.com?part=8
^ permalink raw reply
* RE: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
From: Selvamani Rajagopal @ 2026-06-23 21:01 UTC (permalink / raw)
To: Conor Dooley
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-hwmon@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <20260623-anybody-gutter-e6ca04f53bdb@spud>
> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Subject: Re: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
>
>
> > +
> > +title: onsemi's multi-phase digital controllers
>
> Can someone explain to me what a "digital controller" actually is?
> Seems very generi and that a word may have been left out, were it not
> for the fact that this wording is used several times in the patch.
>
Thanks for reviewing.
According to me, "digital controller" means the controller uses digital circuits to implement
the features and functionality. We can remove "digital" and keep only controller. It won't make any
difference for Linux documentation.
> > +
> > + enum:
> > + - onnn,fd5121
> > + - onnn,fd5123
> > + - onnn,fd5125
>
> Your /OF/ match data in your driver suggests that you intended to permit
> fallback compatibles here?
Agree. Sorry about the discrepancy. Will fix it.
>
> |+#ifdef CONFIG_OF
> |+static const struct of_device_id fd5121_of_match[] = {
> |+ { .compatible = "onnn,fd5121" },
> |+ { }
> |+};
> |+MODULE_DEVICE_TABLE(of, fd5121_of_match);
> |+#endif
>
^ permalink raw reply
* Re: [PATCH v3 10/10] drm/bridge: synopsys: dw-dp: Add audio support
From: sashiko-bot @ 2026-06-23 21:00 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: dri-devel, robh, devicetree, conor+dt
In-Reply-To: <20260612-synopsys-dw-dp-improvements-v3-10-dc61e6352508@collabora.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] The return value of `pm_runtime_get_sync()` is ignored in `dw_dp_audio_startup()`, violating PM runtime API contracts and risking hardware access while suspended.
- [High] Missing error checks on `clk_prepare_enable()` in `dw_dp_audio_prepare()` lead to hardware access failures and clock reference count underflows.
- [High] SDP register bank slots are permanently leaked because `dw_dp_audio_unprepare()` does not free them.
- [Medium] Prematurely updating `dp->audio_interface` before hardware validation causes an unbalanced clock disable in the teardown path on error.
- [Medium] `dw_dp_audio_prepare()` explicitly rejects valid intermediate channel counts, breaking multi-channel playback.
Pre-existing issues:
- [High] Concurrent accesses to the `dp->sdp_reg_bank` bitmap between DRM and ALSA threads lack locking, leading to SDP register corruption.
--
commit 91c310c87d4ddad349da89d89896e7fa9319465d
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
drm/bridge: synopsys: dw-dp: Add audio support
Implement audio support for the Synopsys DesignWare DisplayPort
controller.
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
index 7f4f36c614844..f1946f2c945da 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
[ ... ]
> +static int dw_dp_audio_startup(struct drm_bridge *bridge,
> + struct drm_connector *connector)
> +{
> + struct dw_dp *dp = bridge_to_dp(bridge);
> +
> + dev_dbg(dp->dev, "audio startup\n");
> + pm_runtime_get_sync(dp->dev);
[Severity: High]
Does this code need to check the return value of pm_runtime_get_sync()?
If the power domain or clock fails to resume, it returns a negative error
code. Ignoring this might lead to accessing suspended hardware registers when
ALSA triggers startup.
> +
> + return 0;
> +}
> +
> +static void dw_dp_audio_unprepare(struct drm_bridge *bridge,
> + struct drm_connector *connector)
> +{
> + struct dw_dp *dp = bridge_to_dp(bridge);
> +
> + /* Disable all audio streams */
> + regmap_update_bits(dp->regmap, DW_DP_AUD_CONFIG1, AUDIO_DATA_IN_EN,
> + FIELD_PREP(AUDIO_DATA_IN_EN, 0));
> +
> + if (dp->audio_interface == DW_DP_AUDIO_SPDIF)
> + clk_disable_unprepare(dp->spdif_clk);
> + else if (dp->audio_interface == DW_DP_AUDIO_I2S)
> + clk_disable_unprepare(dp->i2s_clk);
> +
> + dp->audio_interface = DW_DP_AUDIO_UNUSED;
> +}
[Severity: High]
Does dw_dp_audio_unprepare() leak SDP register bank slots?
dw_dp_audio_prepare() allocates a new SDP slot by calling dw_dp_send_sdp(),
which sets a bit in dp->sdp_reg_bank. However, dw_dp_audio_unprepare() never
clears the allocated bit.
Could this exhaust the register bank slots if userspace starts and stops audio
multiple times?
[ ... ]
> +static int dw_dp_audio_prepare(struct drm_bridge *bridge,
> + struct drm_connector *connector,
> + struct hdmi_codec_daifmt *daifmt,
> + struct hdmi_codec_params *params)
> +{
> + struct dw_dp *dp = bridge_to_dp(bridge);
> + u8 audio_data_in_en, supported_audio_interfaces;
> + u32 cfg1;
> + int ret;
> +
> + /*
> + * prepare might be called multiple times, so release the clocks
> + * from previous calls to keep the calls in balance.
> + */
> + if (dp->audio_interface != DW_DP_AUDIO_UNUSED)
> + dw_dp_audio_unprepare(bridge, connector);
> +
> + dp->audio_channels = params->cea.channels;
> + switch (params->cea.channels) {
> + case 1:
> + case 2:
> + audio_data_in_en = AUDIO_DATA_IN_EN_CHANNEL12;
> + break;
> + case 8:
> + audio_data_in_en = AUDIO_DATA_IN_EN_CHANNEL12 |
> + AUDIO_DATA_IN_EN_CHANNEL34 |
> + AUDIO_DATA_IN_EN_CHANNEL56 |
> + AUDIO_DATA_IN_EN_CHANNEL78;
> + break;
> + default:
> + dev_err(dp->dev, "invalid audio channels %d\n", dp->audio_channels);
> + return -EINVAL;
> + }
[Severity: Medium]
Will this explicit rejection break valid multi-channel playback?
The driver sets bridge->hdmi_audio_max_i2s_playback_channels = 8 indicating
hardware support, but this switch statement returns -EINVAL for intermediate
channel counts like 4 or 6. Is it intended to reject valid multi-channel
audio setups?
> +
> + switch (daifmt->fmt) {
> + case HDMI_SPDIF:
> + dp->audio_interface = DW_DP_AUDIO_SPDIF;
> + break;
> + case HDMI_I2S:
> + /*
> + * It is recommended to use SPDIF instead of I2S, since I2S mode requires
> + * manually inserting PCUV control bits from userspace and this is done
> + * automatically in hardware for SPDIF mode.
> + */
> + dp->audio_interface = DW_DP_AUDIO_I2S;
> + break;
> + default:
> + dev_err(dp->dev, "invalid DAI format %d\n", daifmt->fmt);
> + return -EINVAL;
> + }
> +
> + regmap_read(dp->regmap, DW_DP_CONFIG_REG1, &cfg1);
> + supported_audio_interfaces = FIELD_GET(AUDIO_SELECT, cfg1);
> +
> + if (supported_audio_interfaces != DW_DP_AUDIO_I2S_AND_SPDIF &&
> + supported_audio_interfaces != dp->audio_interface) {
> + dev_err(dp->dev, "unsupported DAI %d\n", daifmt->fmt);
> + return -EINVAL;
> + }
[Severity: Medium]
Could this leave dp->audio_interface in a corrupted state on failure?
If an unsupported DAI format is provided, dp->audio_interface is updated prior
to the validation check. If this function then returns -EINVAL, the ALSA core
later calls dw_dp_audio_shutdown() -> dw_dp_audio_unprepare(), which acts on
the corrupted state and attempts to disable a clock that was never enabled.
> +
> + clk_prepare_enable(dp->spdif_clk);
> + clk_prepare_enable(dp->i2s_clk);
[Severity: High]
Does this code need to check the return values of clk_prepare_enable()?
If clk_prepare_enable() fails, the driver continues to program hardware
registers, which could lead to hardware lockups. Furthermore, the teardown
path unconditionally disables the failed clock, causing reference count
underflows.
[ ... ]
> + ret = dw_dp_audio_infoframe_send(dp);
> + if (ret < 0)
> + dev_err(dp->dev, "failed to send audio infoframe\n");
[Severity: High]
This isn't a bug introduced by this patch, but adding audio support introduces
a concurrent access path that makes a pre-existing race possible.
dw_dp_audio_infoframe_send() calls dw_dp_send_sdp(), which allocates a slot
using non-atomic test and set:
nr = find_first_zero_bit(dp->sdp_reg_bank, SDP_REG_BANK_SIZE);
if (nr < SDP_REG_BANK_SIZE)
set_bit(nr, dp->sdp_reg_bank);
Can ALSA audio preparation running concurrently with DRM display updates lead
to slot allocation collisions and SDP hardware register corruption?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-synopsys-dw-dp-improvements-v3-0-dc61e6352508@collabora.com?part=10
^ permalink raw reply
* Re: [PATCH v4 2/3] iio: health: add MAX86150 ECG and PPG biosensor driver
From: Andy Shevchenko @ 2026-06-23 20:52 UTC (permalink / raw)
To: Md Shofiqul Islam
Cc: linux-iio, jic23, dlechner, nuno.sa, andy, robh, krzk+dt,
conor+dt, lars, devicetree, linux-kernel
In-Reply-To: <20260623174600.17100-3-shofiqtest@gmail.com>
On Tue, Jun 23, 2026 at 08:45:59PM +0300, Md Shofiqul Islam wrote:
> The MAX86150 (Maxim/Analog Devices) combines two PPG optical channels
> (Red/IR LED) and one ECG biopotential channel in a single I2C device with
> a 32-entry hardware FIFO.
>
> Driver features:
> - Three IIO channels: in_intensityred_raw, in_intensityir_raw,
> in_voltage0_raw
> - Triggered buffer path driven by the FIFO almost-full interrupt;
> set_trigger_state enables/disables the interrupt only while the buffer
> is active and FIFO is flushed before capture starts
> - validate_trigger = iio_trigger_validate_own_device prevents
> incompatible external triggers from being attached
> - IRQ trigger type taken from irq_get_trigger_type() to honour DT
> configuration; falls back to falling-edge if unspecified
> - fifo_raw burst-read buffer is heap-allocated in struct max86150_data
> and aligned to ARCH_DMA_MINALIGN to satisfy DMA mapping requirements
> of I2C host controllers that use DMA for burst reads
> - SYS_SHDN asserted at end of chip_init so LED drivers draw no current
> when capture is inactive; set_trigger_state() and read_raw() wake and
> sleep the device on demand
> - Per-sample timestamps anchored to the A_FULL IRQ capture time: the
> sample at index (A_FULL_SAMPLES - 1) maps to pf->timestamp, samples
> accumulated after the IRQ receive future timestamps, eliminating
> load-dependent jitter
> - FIFO empty vs exactly-full disambiguation: when wr_ptr == rd_ptr with
> OVF_COUNTER == 0, the A_FULL status bit distinguishes a pointer
> wrap-around (full) from a genuinely empty FIFO
> - devm_regulator_get_enable_optional() for the two optional supplies;
> -ENODEV is treated as success (supply absent, not an error)
> - Powerdown devm action disables interrupts and asserts SYS_SHDN
...
> endmenu
>
> +
Single blank line is enough.
> +config MAX86150
...
IWYU, please! (The list of missing headers below may not be comprehensive.)
+ array_size.h
> +#include <linux/bitfield.h>
+ bitops.h // GENMASK(), sign_extend32()
> +#include <linux/delay.h>
+ dev_printk.h // dev_err_probe()
+ device/devres.h // devm_add_action_or_reset()
+ err.h // IS_ERR(), -ENOMEM, ...
> +#include <linux/i2c.h>
> +#include <linux/irq.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
Can we move this group out...
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
+ types.h // uXX, sXX
> +
...to be here?
...
> +/* Scan element indices */
> +enum max86150_scan_idx {
> + MAX86150_IDX_PPG_RED = 0,
> + MAX86150_IDX_PPG_IR = 1,
> + MAX86150_IDX_ECG = 2,
> + MAX86150_IDX_TS,
Why no value for TS? Or why values for the rest? I assume it's HW/SW cases?
> +};
> +/**
> + * struct max86150_data - driver private state
> + * @regmap: register map for this device
> + * @dev: parent device (for dev_err logging)
> + * @trig: IIO hardware trigger backed by the device interrupt line
> + * @sample_period_ns: sample period in nanoseconds (set from configured rate)
> + * @fifo_raw: DMA-safe buffer for regmap_noinc_read() FIFO bursts;
> + * must be in struct (heap) not on the stack to satisfy
> + * DMA mapping requirements of some I2C host controllers
> + * @buf: IIO push buffer sized for worst-case (all 3 channels
> + * active): 3 x s32 (12 bytes) + 4-byte pad + s64
> + * timestamp = 24 bytes. __aligned(8) satisfies
> + * iio_push_to_buffers_with_timestamp().
> + */
> +struct max86150_data {
> + struct regmap *regmap;
> + struct device *dev;
Is regmap device and dev is the same? If so, drop one and derive the other.
> + struct iio_trigger *trig;
> + u32 sample_period_ns;
> + u8 fifo_raw[MAX86150_SAMPLE_BYTES] __aligned(ARCH_DMA_MINALIGN);
> + s32 buf[6] __aligned(8);
We have a macro for these. (For alignment.)
> +};
...
> +static int max86150_read_one_sample(struct max86150_data *data,
> + u32 *ppg_red, u32 *ppg_ir, s32 *ecg)
> +{
> + int ret;
> +
> + /*
> + * Use data->fifo_raw (heap memory) not a local array so the buffer is
> + * DMA-mappable for I2C host controllers that use DMA for burst reads.
> + */
> + ret = regmap_noinc_read(data->regmap, MAX86150_REG_FIFO_DATA,
> + data->fifo_raw, sizeof(data->fifo_raw));
> + if (ret)
> + return ret;
> +
> + /* Bytes [0..2]: PPG Red - 19-bit value in bits [18:0] */
> + *ppg_red = (u32)(data->fifo_raw[0] & 0x07) << 16 |
> + (u32)data->fifo_raw[1] << 8 | data->fifo_raw[2];
Casting to u32 makes a little sense. Why?
> + /* Bytes [3..5]: PPG IR - same format */
> + *ppg_ir = (u32)(data->fifo_raw[3] & 0x07) << 16 |
> + (u32)data->fifo_raw[4] << 8 | data->fifo_raw[5];
> +
> + /* Bytes [6..8]: ECG - 18-bit signed, sign-extend to s32 */
> + *ecg = sign_extend32((u32)(data->fifo_raw[6] & 0x03) << 16 |
> + (u32)data->fifo_raw[7] << 8 | data->fifo_raw[8], 17);
Ditto for all these...
> + return 0;
> +}
...
> +static int max86150_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct max86150_data *data = iio_priv(indio_dev);
> + u32 ppg_red, ppg_ir;
> + s32 ecg;
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + /*
> + * Claim direct mode to prevent concurrent sysfs reads from
> + * corrupting the FIFO pointers while a triggered buffer
> + * capture is active.
> + */
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> +
> + /*
> + * Single-shot path: wake the device, flush stale FIFO data,
> + * wait one sample period, read, then return to shutdown so
> + * the LEDs are not drawing current when idle.
> + */
> + ret = regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap,
> + MAX86150_REG_FIFO_WR_PTR, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap,
> + MAX86150_REG_OVF_COUNTER, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap,
> + MAX86150_REG_FIFO_RD_PTR, 0);
> + if (ret) {
> + regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN, MAX86150_SYS_SHDN);
> + iio_device_release_direct(indio_dev);
> + return ret;
No, use regular pattern, id est
if (ret)
return ret;
Ditto for other cases like this.
> + }
> +
> + /* Wait for one complete sample period at 100 Hz (<= 10 ms) */
> + usleep_range(11000, 13000);
> +
> + ret = max86150_read_one_sample(data, &ppg_red, &ppg_ir, &ecg);
> + regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN, MAX86150_SYS_SHDN);
> + iio_device_release_direct(indio_dev);
> + if (ret)
> + return ret;
> +
> + switch (chan->scan_index) {
> + case MAX86150_IDX_PPG_RED:
> + *val = ppg_red;
> + break;
> + case MAX86150_IDX_PPG_IR:
> + *val = ppg_ir;
> + break;
> + case MAX86150_IDX_ECG:
> + *val = ecg;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return IIO_VAL_INT;
> +
> + default:
> + return -EINVAL;
> + }
> +}
...
> +/*
> + * Control device power and the FIFO almost-full interrupt when the IIO
> + * triggered buffer is started (state=true) or stopped (state=false).
> + *
> + * On start: wake from shutdown, flush stale FIFO data so the first
> + * samples pushed to userspace are from after buffer enable, then
> + * unmask the A_FULL interrupt.
> + *
> + * On stop: mask the interrupt, then return to shutdown so the LED
> + * drivers do not draw current while capture is inactive.
> + */
> +static int max86150_set_trigger_state(struct iio_trigger *trig, bool state)
> +{
> + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> + struct max86150_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + if (!state) {
> + ret = regmap_write(data->regmap, MAX86150_REG_INT_ENABLE1, 0);
> + regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN, MAX86150_SYS_SHDN);
_set_bits(), but why no check?
> + return ret;
> + }
> +
> + ret = regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap, MAX86150_REG_FIFO_WR_PTR, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap, MAX86150_REG_OVF_COUNTER, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap, MAX86150_REG_FIFO_RD_PTR, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap, MAX86150_REG_INT_ENABLE1,
> + MAX86150_INT_A_FULL);
> + if (ret)
> + regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN, MAX86150_SYS_SHDN);
> + return ret;
Ditto.
...
Also why not to split to trigger_enable and trigger_disable?
_trigger_enable(max86150_data *data)
_trigger_disable(...)
_set_trigger_state()
if (enable)
return _trigger_enable()
return trigger_disable();
> +}
...
> +/**
> + * max86150_trigger_handler - threaded IRQ handler for FIFO almost-full
> + *
> + * Called by the IIO buffer infrastructure when the hardware trigger fires.
> + * Reads INT_STATUS1 to de-assert the interrupt, then drains all available
> + * FIFO samples into the IIO push buffer, packing only the channels that
> + * are currently enabled in active_scan_mask.
> + */
> +static irqreturn_t max86150_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *idev = pf->indio_dev;
> + struct max86150_data *data = iio_priv(idev);
> + unsigned int status, wr_ptr, rd_ptr, ovf;
> + u32 ppg_red, ppg_ir;
> + s32 ecg;
> + int ret, n_avail, i, j;
Usually it's not a good idea to mix ret with other variables that not
semantically related. Also, why those are signed?
> + /*
> + * Reading INT_STATUS1 clears the interrupt. Do this before touching
> + * the FIFO so the pin is de-asserted while we drain samples.
> + */
> + ret = regmap_read(data->regmap, MAX86150_REG_INT_STATUS1, &status);
> + if (ret)
> + goto done;
> +
> + ret = regmap_read(data->regmap, MAX86150_REG_FIFO_WR_PTR, &wr_ptr);
> + if (ret)
> + goto done;
> + ret = regmap_read(data->regmap, MAX86150_REG_FIFO_RD_PTR, &rd_ptr);
> + if (ret)
> + goto done;
> +
> + /*
> + * OVF_COUNTER: if non-zero the FIFO overflowed; drain all 32 slots.
> + * When wr_ptr == rd_ptr with no overflow the FIFO could be empty OR
> + * hold exactly MAX86150_FIFO_DEPTH entries (pointer wrap-around).
> + * Use the A_FULL status bit to disambiguate: if the IRQ fired for
> + * A_FULL but the computed count is zero, the FIFO wrapped to full.
> + */
> + ret = regmap_read(data->regmap, MAX86150_REG_OVF_COUNTER, &ovf);
> + if (ret)
> + goto done;
> +
> + if (ovf > 0) {
> + n_avail = MAX86150_FIFO_DEPTH;
> + } else {
> + n_avail = (wr_ptr - rd_ptr) & (MAX86150_FIFO_DEPTH - 1);
> + if (n_avail == 0 && (status & MAX86150_INT_A_FULL))
> + n_avail = MAX86150_FIFO_DEPTH;
> + }
> +
> + /*
> + * Anchor timestamps to the A_FULL IRQ capture time: sample index
> + * (MAX86150_A_FULL_SAMPLES - 1) corresponds to pf->timestamp.
> + * Samples that accumulated between the IRQ and handler execution
> + * receive timestamps in the future relative to the IRQ, eliminating
> + * load-dependent jitter in multi-sample drains.
> + */
> + for (i = 0; i < n_avail; i++) {
for (unsigned int i = 0; i < n_avail; i++) {
> + s64 ts = pf->timestamp +
> + (s64)(i - (MAX86150_A_FULL_SAMPLES - 1)) *
> + data->sample_period_ns;
This is unmaintainable. Split definition and assignment. Also, do you really
need a casting? I.o.w. isn't pf->timestamp already an s64?
> + ret = max86150_read_one_sample(data, &ppg_red, &ppg_ir, &ecg);
> + if (ret)
> + break;
> +
> + /*
> + * Zero the entire buffer before packing so padding bytes
> + * between enabled channels do not leak previous sample data
> + * to userspace when fewer than 3 channels are active.
> + */
> + memset(data->buf, 0, sizeof(data->buf));
> +
> + /*
> + * Pack only active channels at consecutive positions [0..j-1].
> + * iio_push_to_buffers_with_timestamp() uses scan_bytes (which
> + * accounts for the active channel count) to place the timestamp,
> + * so static indexing would misplace it when fewer than 3
> + * channels are enabled.
> + */
> + j = 0;
> + if (test_bit(MAX86150_IDX_PPG_RED, idev->active_scan_mask))
> + data->buf[j++] = ppg_red;
> + if (test_bit(MAX86150_IDX_PPG_IR, idev->active_scan_mask))
> + data->buf[j++] = ppg_ir;
> + if (test_bit(MAX86150_IDX_ECG, idev->active_scan_mask))
> + data->buf[j++] = ecg;
> +
> + iio_push_to_buffers_with_timestamp(idev, data->buf, ts);
> + }
> +
> +done:
> + iio_trigger_notify_done(idev->trig);
> + return IRQ_HANDLED;
> +}
> +
> +/* Chip initialisation / teardown */
> +
> +static void max86150_powerdown(void *arg)
> +{
> + struct max86150_data *data = arg;
> +
> + regmap_write(data->regmap, MAX86150_REG_INT_ENABLE1, 0);
> + regmap_write(data->regmap, MAX86150_REG_SYS_CTRL, MAX86150_SYS_SHDN);
> +}
...
> +static int max86150_chip_init(struct max86150_data *data)
> +{
> + int ret;
> +
> + /* Software reset; the bit self-clears within 1 ms */
> + ret = regmap_write(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_RESET);
> + if (ret)
> + return ret;
> + usleep_range(1000, 2000);
fsleep(). Also need a reference to datasheet section, table, et cetera.
> + /*
> + * FIFO: no sample averaging, rollover enabled, assert A_FULL when
> + * 17 samples are in the FIFO (32 - 15 = 17 available to read).
> + */
> + ret = regmap_write(data->regmap, MAX86150_REG_FIFO_CONFIG,
> + MAX86150_FIFO_ROLLOVER_EN |
> + FIELD_PREP(MAX86150_FIFO_A_FULL,
> + MAX86150_FIFO_A_FULL_VAL));
> + if (ret)
> + return ret;
> +
> + /* Slot 1 = PPG Red (LED1), Slot 2 = PPG IR (LED2) */
> + ret = regmap_write(data->regmap, MAX86150_REG_FIFO_DCTRL1,
> + FIELD_PREP(MAX86150_FIFO_FD1, MAX86150_FD_LED1) |
> + FIELD_PREP(MAX86150_FIFO_FD2, MAX86150_FD_LED2));
> + if (ret)
> + return ret;
> +
> + /* Slot 3 = ECG, Slot 4 = disabled */
> + ret = regmap_write(data->regmap, MAX86150_REG_FIFO_DCTRL2,
> + FIELD_PREP(MAX86150_FIFO_FD3, MAX86150_FD_ECG) |
> + FIELD_PREP(MAX86150_FIFO_FD4, MAX86150_FD_NONE));
> + if (ret)
> + return ret;
> +
> + /* PPG: 100 Hz sample rate, 16384 nA ADC full-scale range */
> + ret = regmap_write(data->regmap, MAX86150_REG_PPG_CONFIG1,
> + FIELD_PREP(MAX86150_PPG_ADC_RGE,
> + MAX86150_PPG_ADC_RGE_16384) |
> + FIELD_PREP(MAX86150_PPG_SR,
> + MAX86150_PPG_SR_100HZ));
> + if (ret)
> + return ret;
> +
> + /* LED pulse amplitudes (~50 mA) */
> + ret = regmap_write(data->regmap, MAX86150_REG_LED1_PA,
> + MAX86150_LED_PA_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(data->regmap, MAX86150_REG_LED2_PA,
> + MAX86150_LED_PA_DEFAULT);
> + if (ret)
> + return ret;
> +
> + /*
> + * Record sample period for timestamp reconstruction in the trigger
> + * handler. The PPG_SR field is fixed to 100 Hz in this driver.
> + */
> + data->sample_period_ns = 10000000; /* 100 Hz = 10 ms */
10 * NSEC_PER_MSEC
from time.h.
> + /*
> + * Assert SYS_SHDN so the LED drivers do not draw current while
> + * the driver is bound but no capture is active.
> + * set_trigger_state() clears SHDN when the IIO buffer is enabled
> + * and re-asserts it when disabled. read_raw() wakes and sleeps
> + * the device around each single-shot read.
> + */
> + return regmap_write(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN);
> +}
...
> +/* Probe */
These comments bring no value, please, drop all of a such.
...
> +static int max86150_probe(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev;
> + struct max86150_data *data;
> + unsigned int part_id;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + data = iio_priv(indio_dev);
> + data->dev = &client->dev;
A duplication. You have regmap with the same device.
> + /*
> + * Enable power supplies before any I2C access. Both supplies are
> + * optional in the device tree; use _optional variant so probing
> + * succeeds on boards that power the device from fixed rails with no
> + * DT regulator node.
> + */
> + ret = devm_regulator_get_enable_optional(&client->dev, "vdd");
> + if (ret && ret != -ENODEV)
> + return dev_err_probe(&client->dev, ret,
Add
struct device *dev = &client->dev;
to the top of the function and use everywhere.
> + "Failed to get/enable vdd supply\n");
> +
> + ret = devm_regulator_get_enable_optional(&client->dev, "leds");
> + if (ret && ret != -ENODEV)
> + return dev_err_probe(&client->dev, ret,
> + "Failed to get/enable leds supply\n");
> +
> + data->regmap = devm_regmap_init_i2c(client, &max86150_regmap_config);
> + if (IS_ERR(data->regmap))
> + return dev_err_probe(&client->dev, PTR_ERR(data->regmap),
> + "Failed to initialise regmap\n");
> +
> + ret = regmap_read(data->regmap, MAX86150_REG_PART_ID, &part_id);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot read part ID\n");
> +
> + if (part_id != MAX86150_PART_ID_VAL)
> + return dev_err_probe(&client->dev, -ENODEV,
> + "Unexpected part ID 0x%02x (expected 0x%02x)\n",
> + part_id, MAX86150_PART_ID_VAL);
> +
> + ret = max86150_chip_init(data);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Chip initialisation failed\n");
> +
> + ret = devm_add_action_or_reset(&client->dev, max86150_powerdown, data);
> + if (ret)
> + return ret;
> +
> + indio_dev->name = "max86150";
> + indio_dev->channels = max86150_channels;
> + indio_dev->num_channels = ARRAY_SIZE(max86150_channels);
> + indio_dev->info = &max86150_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + /*
> + * If the device tree provides an interrupt, set up a hardware
> + * trigger so userspace can use the FIFO almost-full signal to
> + * drive capture without polling.
> + */
> + if (client->irq > 0) {
> + unsigned long irq_trig;
> +
> + data->trig = devm_iio_trigger_alloc(&client->dev,
> + "%s-dev%d",
> + indio_dev->name,
> + iio_device_id(indio_dev));
> + if (!data->trig)
> + return -ENOMEM;
> +
> + data->trig->ops = &max86150_trigger_ops;
> + iio_trigger_set_drvdata(data->trig, indio_dev);
> +
> + /*
> + * Honour the interrupt trigger type from the device tree.
> + * Fall back to falling-edge if the DT does not specify one.
Why? Do we need to support broken DT?
> + */
> + irq_trig = irq_get_trigger_type(client->irq);
> + if (!irq_trig)
> + irq_trig = IRQF_TRIGGER_FALLING;
Simply drop this.
> + ret = devm_request_irq(&client->dev, client->irq,
> + iio_trigger_generic_data_rdy_poll,
> + irq_trig,
> + "max86150", data->trig);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot request IRQ %d\n",
> + client->irq);
No dup messages.
> +
> + ret = devm_iio_trigger_register(&client->dev, data->trig);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot register trigger\n");
> + }
> +
> + ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev,
> + iio_pollfunc_store_time,
> + max86150_trigger_handler,
> + NULL);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot setup triggered buffer\n");
So, it seems this message won't ever be printed. Drop it.
> + return devm_iio_device_register(&client->dev, indio_dev);
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH RFC v3 09/10] dt-bindings: display: rockchip: dw-dp: fix sound DAI cells
From: sashiko-bot @ 2026-06-23 20:51 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: conor+dt, devicetree, robh, dri-devel
In-Reply-To: <20260612-synopsys-dw-dp-improvements-v3-9-dc61e6352508@collabora.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Unconditionally changing `#sound-dai-cells` from `const: 0` to `const: 1` breaks backward compatibility with existing device tree binaries and causes `make dtbs_check` failures.
- [High] dw_dp_runtime_resume() fails to check the return value of clk_prepare_enable() for apb_clk and aux_clk, leading to unclocked hardware accesses.
- [High] The driver ignores the return value of pm_runtime_get_sync() and its scoped guard wrappers, leading to potential unclocked register accesses.
- [Medium] The `#sound-dai-cells = <1>` design is mathematically insufficient to support the RK3576's 6 DAI interfaces, ensuring future ABI breakage when MST is fully implemented.
--
commit dcfc80f29db8ade71eb0e49550ea25781716e785
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
dt-bindings: display: rockchip: dw-dp: fix sound DAI cells
This commit updates the #sound-dai-cells property to support the two
possible DAI interfaces (I2S and S/PDIF) on RK3588 and RK3576 DesignWare
DisplayPort controllers. It changes the const value to 1 and discusses
options for future Multi Stream Transport (MST) support.
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml
> index 2b0d9e23e9432..1303d0e2145a9 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml
> @@ -83,7 +83,8 @@ properties:
> maxItems: 1
>
> "#sound-dai-cells":
> - const: 0
> + const: 1
> + description: 0 for I2S, 1 for SPDIF
[Severity: Critical]
Does unconditionally changing #sound-dai-cells from const: 0 to const: 1
break backward compatibility with existing device tree binaries?
Existing device trees supplying 0 cells will now fail validation during
make dtbs_check, and older DTBs will no longer conform to the schema.
[Severity: Medium]
Is setting #sound-dai-cells = <1> mathematically sufficient to support the
RK3576 hardware?
The commit message mentions the RK3576 features MST with 6 DAIs, so a single
cell constraint only supports 2 states (I2S and S/PDIF). Will this necessitate
another breaking change when MST is implemented?
>
> required:
> - compatible
[ ... ]
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index 36ee6e027af52..7f4f36c614844 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
[ ... ]
> @@ -1651,6 +1657,8 @@ static void dw_dp_bridge_atomic_enable(struct drm_bridge *bridge,
> struct drm_connector_state *conn_state;
> int ret;
>
> + pm_runtime_get_sync(dp->dev);
> +
[Severity: High]
Does this code discard the return value of pm_runtime_get_sync()?
If the PM core fails to resume the device, the code proceeds with the enable
sequence, which could risk unclocked register accesses later on.
> connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
> if (!connector) {
> dev_err(dp->dev, "failed to get connector\n");
[ ... ]
> @@ -1725,6 +1734,8 @@ static enum drm_connector_status dw_dp_bridge_detect(struct drm_bridge *bridge,
> {
> struct dw_dp *dp = bridge_to_dp(bridge);
>
> + ACQUIRE(pm_runtime_active_auto, pm)(dp->dev);
> +
> if (!dw_dp_hpd_detect(dp))
[Severity: High]
Does using this unconditional PM scope guard discard the return value?
If the device failed to resume, the code will proceed to access hardware
registers in dw_dp_hpd_detect(), which could trigger a bus fault.
> return connector_status_disconnected;
>
> if (!dw_dp_hpd_detect_link(dp, connector))
[ ... ]
> @@ -2099,6 +2160,26 @@ void dw_dp_unbind(struct dw_dp *dp)
> }
> EXPORT_SYMBOL_GPL(dw_dp_unbind);
>
> +int dw_dp_runtime_suspend(struct dw_dp *dp)
> +{
> + clk_disable_unprepare(dp->aux_clk);
> + clk_disable_unprepare(dp->apb_clk);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(dw_dp_runtime_suspend);
> +
> +int dw_dp_runtime_resume(struct dw_dp *dp)
> +{
> + clk_prepare_enable(dp->apb_clk);
> + clk_prepare_enable(dp->aux_clk);
> +
[Severity: High]
Are we ignoring the return values of clk_prepare_enable() here?
If the clocks fail to enable, unconditionally calling dw_dp_init_hw() will
access hardware registers while unclocked, which could lead to a bus hang.
> + dw_dp_init_hw(dp);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(dw_dp_runtime_resume);
[ ... ]
> diff --git a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> index 0de822360c8db..1f53228e56d9f 100644
> --- a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
[ ... ]
> +static void dw_dp_rockchip_hpd_sw_sel(void *data, bool force_hpd_from_sw)
> +{
> + struct rockchip_dw_dp *dp = data;
> + u32 hpd_reg = dp->pdata->hpd_reg[dp->id];
> +
> + dp->hpd_sel = force_hpd_from_sw;
> +
> + ACQUIRE(pm_runtime_active_auto, pm)(dp->dev);
> +
> + regmap_write(dp->vo_grf, hpd_reg,
> + FIELD_PREP_WM16(ROCKCHIP_VO_GRF_DP_SINK_HPD_SEL, dp->hpd_sel));
[Severity: High]
Does this ignore the possibility that the power domain failed to resume?
If the unconditional PM guard fails, writing to the vo_grf syscon will
trigger a bus fault.
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-synopsys-dw-dp-improvements-v3-0-dc61e6352508@collabora.com?part=9
^ permalink raw reply
* Re: [PATCH 7/9] drm/rockchip: dw_hdmi_qp: Support 10-bit YUV422 output format
From: sashiko-bot @ 2026-06-23 20:51 UTC (permalink / raw)
To: Cristian Ciocaltea; +Cc: robh, dri-devel, devicetree, conor+dt
In-Reply-To: <20260617-dw-hdmi-qp-yuv-v1-7-a665cfd06d7d@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Mutating hardware state and global device tracking variables inside an atomic_check callback.
--
commit ee5f0003f61d5c3a78b5bc65346f39307471c74c
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
drm/rockchip: dw_hdmi_qp: Support 10-bit YUV422 output format
This commit adds support for the 10-bit YUV 4:2:2 output format
(MEDIA_BUS_FMT_UYVY10_1X20) to the Rockchip DW HDMI QP encoder. It also
forces 8 bpc when YUV 4:2:2 is in use to ensure the PHY PLL output remains
aligned with the TMDS character rate.
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
[ ... ]
> @@ -227,7 +228,14 @@ dw_hdmi_qp_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
> }
>
> phy_cfg.hdmi.tmds_char_rate = conn_state->hdmi.tmds_char_rate;
> - phy_cfg.hdmi.bpc = conn_state->hdmi.output_bpc;
> + /*
> + * YUV422 always transmits two 12-bit components per clock cycle,
> + * regardless of the color depth, which from a rate perspective is
> + * equivalent to three 8-bit RGB components. Force 8 bpc here to
> + * keep the PHY PLL output aligned with the TMDS character rate.
> + */
> + phy_cfg.hdmi.bpc = (s->output_mode == ROCKCHIP_OUT_MODE_YUV422 ?
> + 8 : conn_state->hdmi.output_bpc);
>
> ret = phy_configure(hdmi->phy, &phy_cfg);
> if (!ret) {
> hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
> s->output_type = DRM_MODE_CONNECTOR_HDMIA;
> s->output_bpc = conn_state->hdmi.output_bpc;
> s->bus_format = ingest_fmt;
> } else {
[Severity: High]
This is a pre-existing issue, but does
dw_hdmi_qp_rockchip_encoder_atomic_check() mutate hardware state and global
tracking variables during an atomic check?
In the DRM atomic framework, the atomic_check phase is expected to be
side-effect free. However, this code calls phy_configure(), which writes
directly to the PHY hardware registers, and then modifies the global
hdmi->tmds_char_rate variable.
Could this corrupt the active display configuration if userspace performs a
DRM_MODE_ATOMIC_TEST_ONLY commit or if an atomic commit fails a later
validation stage?
Furthermore, if the device is runtime-suspended, could accessing unclocked
hardware registers here trigger a synchronous abort and hang the system?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-dw-hdmi-qp-yuv-v1-0-a665cfd06d7d@collabora.com?part=7
^ permalink raw reply
* [PATCH v5 6/6] arm64: dts: allwinner: A133: add support for Baijie Helper A133 board
From: Alexander Sverdlin @ 2026-06-23 20:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input
In-Reply-To: <20260623204824.691832-1-alexander.sverdlin@gmail.com>
Baijie Helper A133 board is a development board around Baijie A133 Core
SBC. Features:
- 1/2/4GiB LPDDR4 DRAM
- 8/16/32GiB eMMC
- AXP707 PMIC
- USB-C OTG port in peripheral mode (via onboard hub)
- 2 USB 2.0 ports
- MicroSD slot and on-board eMMC module
- Gigabit Ethernet
- Bluetooth
- WiFi
Add initial support for both the Helper and Core boards, including UART,
PMU, eMMC, USB, Ethernet, LRADC-connected buttons.
UART1 can only be used for Bluetooth module, but BT-WiFi combo Allwinner
AW869A chip has no mainline driver currently.
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
Changelog:
v5:
- no changes
v4:
- renamed "sun50i-a133-baijie-helper.dtb" -> "sun50i-a133-helperboard.dtb"
- added "model" property into root of sun50i-a133-helperboard-core.dtsi
- added "cap-mmc-highspeed" and "max-frequency" into &mmc2
- added "x-powers,drive-vbus-en" and "*-supply" into &axp803
- dropped all "regulator-enable-ramp-delay" properties
- replaced ®_dcdc3 with a "polyphased" comment
- exact DRAM voltage in ®_dcdc5
- disabled ®_dcdc6 to avoid "[ 31.710641] dcdc6: disabling"
- added ®_vdd5v "root" regulator
- added "disable-wp" into &mmc0
- commented &usb_otg
- assigned usb1_vbus-supply in &usbphy
v3:
- added my copyrights into the newly introduced DTs
- all DT nodes sorted alphabetically
- all always-on regulators commented/propetly named
- all regulators got proper voltages (not default ranges)
- ADC-sensed buttons K1..K5 added
- re-labelled "eth_phy" -> "rgmii_phy"
- usbphy 0 switched from host into peripheral mode (downstream from an
onboard hub)
- typo sun50i-a133-baije-core.dtsi -> sun50i-a133-baijie-core.dtsi
v2:
- introduced baijie,helper-a133-core compatible for the Core (SoM) board
arch/arm64/boot/dts/allwinner/Makefile | 1 +
.../sun50i-a133-helperboard-core.dtsi | 197 ++++++++++++++++++
.../dts/allwinner/sun50i-a133-helperboard.dts | 148 +++++++++++++
3 files changed, 346 insertions(+)
create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard-core.dtsi
create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard.dts
diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
index 53e6b701e7d3..aa21f58a4be1 100644
--- a/arch/arm64/boot/dts/allwinner/Makefile
+++ b/arch/arm64/boot/dts/allwinner/Makefile
@@ -24,6 +24,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-sopine-baseboard.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-teres-i.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h64-remix-mini-pc.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a100-allwinner-perf1.dtb
+dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a133-helperboard.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a133-liontron-h-a133l.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-bananapi-m2-plus.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-bananapi-m2-plus-v1.2.dtb
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard-core.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard-core.dtsi
new file mode 100644
index 000000000000..545972d2324a
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard-core.dtsi
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2025 Arm Ltd.
+ * Copyright (c) 2026 Alexander Sverdlin <alexander.sverdlin@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "sun50i-a100.dtsi"
+#include "sun50i-a100-cpu-opp.dtsi"
+
+/{
+ model = "Baijie A133 HelperBoard Core";
+ compatible = "baijie,helperboard-a133-core",
+ "allwinner,sun50i-a100";
+
+ aliases {
+ serial1 = &uart1; /* BT module */
+ };
+};
+
+&cpu0 {
+ cpu-supply = <®_dcdc2>;
+};
+
+&lradc {
+ vref-supply = <®_aldo1>;
+};
+
+&mmc2 {
+ vmmc-supply = <®_dcdc1>;
+ vqmmc-supply = <®_eldo1>;
+ cap-mmc-highspeed;
+ cap-mmc-hw-reset;
+ max-frequency = <100000000>;
+ non-removable;
+ bus-width = <8>;
+ mmc-ddr-1_8v;
+ mmc-hs200-1_8v;
+ status = "okay";
+};
+
+&pio {
+ vcc-pb-supply = <®_dcdc1>;
+ vcc-pc-supply = <®_eldo1>;
+ vcc-pd-supply = <®_dcdc1>;
+ vcc-pe-supply = <®_dldo2>;
+ vcc-pf-supply = <®_dcdc1>;
+ vcc-pg-supply = <®_dldo1>;
+ vcc-ph-supply = <®_dcdc1>;
+ /*
+ * PL0/PL1 are the I2C connection to PMIC, but it would create a
+ * circular dependency:
+ * vcc-pl-supply = <®_aldo3>;
+ */
+};
+
+&r_i2c0 {
+ status = "okay";
+
+ axp803: pmic@34 {
+ compatible = "x-powers,axp803";
+ reg = <0x34>;
+ interrupt-parent = <&r_intc>;
+ interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+ x-powers,drive-vbus-en; /* set N_VBUSEN as output pin */
+ aldoin-supply = <®_vdd5v>;
+ dldoin-supply = <®_vdd5v>;
+ eldoin-supply = <®_vdd5v>;
+ fldoin-supply = <®_dcdc5>;
+ vin1-supply = <®_vdd5v>;
+ vin2-supply = <®_vdd5v>;
+ vin3-supply = <®_vdd5v>;
+ vin4-supply = <®_vdd5v>;
+ vin5-supply = <®_vdd5v>;
+ vin6-supply = <®_vdd5v>;
+ drivevbus-supply = <®_vdd5v>;
+ };
+};
+
+#include "axp803.dtsi"
+
+&ac_power_supply {
+ status = "okay";
+};
+
+®_aldo1 {
+ /* PLL + LRADC analog reference */
+ regulator-always-on;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "vcc-pll";
+};
+
+®_aldo2 {
+ /* LPDDR */
+ regulator-always-on;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "vdd18-lpddr";
+};
+
+®_aldo3 {
+ /*
+ * Port L, but linking it to &pio node would create a circular
+ * dependency because of PL0/PL1 I2C connection to PMIC
+ */
+ regulator-always-on;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "vcc-pl";
+};
+
+®_dcdc1 {
+ /* Besides Port D it also powers analog part of USB IP and SoC I/O */
+ regulator-always-on;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "vcc-3v3";
+};
+
+®_dcdc2 {
+ regulator-always-on;
+ regulator-min-microvolt = <810000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-name = "vdd-cpu";
+};
+
+/* DCDC3 is polyphased with DCDC2 */
+
+®_dcdc4 {
+ /* Digital part of USB IP, "System" SoC power rail */
+ regulator-always-on;
+ regulator-min-microvolt = <950000>;
+ regulator-max-microvolt = <950000>;
+ regulator-name = "vdd-sys";
+};
+
+®_dcdc5 {
+ regulator-always-on;
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1100000>;
+ regulator-name = "vcc-dram";
+};
+
+/* DCDC6 unused */
+®_dcdc6 {
+ status = "disabled";
+};
+
+®_dldo1 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "vcc-pg";
+};
+
+®_dldo2 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "vcc-pe";
+};
+
+®_dldo3 {
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ regulator-name = "avdd-csi";
+};
+
+®_dldo4 {
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ regulator-name = "afvcc-csi";
+};
+
+®_eldo1 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "vcc-pc";
+};
+
+®_eldo2 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-name = "dvdd-csi";
+};
+
+/* ELDO3 unused */
+
+®_fldo1 {
+ /* CPUS power rail */
+ regulator-always-on;
+ regulator-min-microvolt = <900000>;
+ regulator-max-microvolt = <900000>;
+ regulator-name = "vdd-cpus";
+};
+
+/* reg_drivevbus unused */
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard.dts
new file mode 100644
index 000000000000..694c0cacf906
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard.dts
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2025 Arm Ltd.
+ * Copyright (c) 2026 Alexander Sverdlin <alexander.sverdlin@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "sun50i-a133-helperboard-core.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/linux-event-codes.h>
+#include <dt-bindings/leds/common.h>
+
+/{
+ model = "Baijie HelperBoard A133";
+ compatible = "baijie,helperboard-a133",
+ "baijie,helperboard-a133-core",
+ "allwinner,sun50i-a100";
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ led {
+ function = LED_FUNCTION_INDICATOR;
+ color = <LED_COLOR_ID_GREEN>;
+ gpios = <&pio 7 13 GPIO_ACTIVE_LOW>; /* PH13 */
+ };
+ };
+
+ reg_vdd5v: vdd5v {
+ /* board wide 5V supply from a 12V->5V regulator */
+ compatible = "regulator-fixed";
+ regulator-name = "vdd-5v";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ regulator-always-on;
+ };
+};
+
+&ehci1 {
+ status = "okay";
+};
+
+&emac0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&rgmii0_pins>;
+ phy-handle = <&rgmii_phy>;
+ phy-mode = "rgmii-id";
+ allwinner,rx-delay-ps = <200>;
+ allwinner,tx-delay-ps = <200>;
+ status = "okay";
+};
+
+&lradc {
+ wakeup-source;
+ status = "okay";
+
+ button-115 {
+ label = "K1";
+ linux,code = <KEY_1>;
+ channel = <0>;
+ voltage = <114607>;
+ };
+
+ button-235 {
+ label = "K2";
+ linux,code = <KEY_2>;
+ channel = <0>;
+ voltage = <234783>;
+ };
+
+ button-360 {
+ label = "K3";
+ linux,code = <KEY_3>;
+ channel = <0>;
+ voltage = <360000>;
+ };
+
+ button-476 {
+ label = "K4";
+ linux,code = <KEY_4>;
+ channel = <0>;
+ voltage = <476471>;
+ };
+
+ button-592 {
+ label = "K5";
+ linux,code = <KEY_5>;
+ channel = <0>;
+ voltage = <591946>;
+ };
+};
+
+&mdio0 {
+ reset-gpios = <&pio 7 11 GPIO_ACTIVE_LOW>; /* PH11 */
+ reset-delay-us = <10000>;
+ reset-post-delay-us = <150000>;
+
+ rgmii_phy: ethernet-phy@1 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ };
+};
+
+&mmc0 {
+ vmmc-supply = <®_dcdc1>;
+ cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
+ bus-width = <4>;
+ disable-wp;
+ status = "okay";
+};
+
+&ohci1 {
+ status = "okay";
+};
+
+&rgmii0_pins {
+ drive-strength = <30>;
+};
+
+&uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart0_pb_pins>;
+ status = "okay";
+};
+
+&usb_otg {
+ /*
+ * Connected to a downstream port of an onboard hub, therefore only
+ * "peripheral" mode will work here.
+ */
+ dr_mode = "peripheral";
+ status = "okay";
+};
+
+&usbphy {
+ usb1_vbus-supply = <®_vdd5v>;
+ status = "okay";
+};
--
2.54.0
^ permalink raw reply related
* [PATCH v5 5/6] arm64: dts: allwinner: a100: Add LRADC node
From: Alexander Sverdlin @ 2026-06-23 20:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input
In-Reply-To: <20260623204824.691832-1-alexander.sverdlin@gmail.com>
A100/A133 SoCs feature a Low Rate ADC (LRADC) for Key application.
Specs:
- Power supply voltage: 1.8 V
- Reference voltage: 1.35 V
- Interrupt support
- Support Hold Key and General Key
- Support normal, continue and single work mode
- 6-bits resolution, sample rate up to 2 kHz
- Voltage input range between 0 and 1.35 V
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
Changelog:
v5:
- reflowed "compatible" property of lradc node
v4:
- added allwinner,sun50i-a100-lradc compatible
v3:
- new patch
arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
index b3fb1e0ee796..ba6020989ce9 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
@@ -466,6 +466,16 @@ ths: thermal-sensor@5070400 {
#thermal-sensor-cells = <1>;
};
+ lradc: lradc@5070800 {
+ compatible = "allwinner,sun50i-a100-lradc",
+ "allwinner,sun50i-r329-lradc";
+ reg = <0x05070800 0x400>;
+ interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_LRADC>;
+ resets = <&ccu RST_BUS_LRADC>;
+ status = "disabled";
+ };
+
usb_otg: usb@5100000 {
compatible = "allwinner,sun50i-a100-musb",
"allwinner,sun8i-a33-musb";
--
2.54.0
^ permalink raw reply related
* [PATCH v5 4/6] dt-bindings: input: sun4i-lradc-keys: Add A100/A133 compatible
From: Alexander Sverdlin @ 2026-06-23 20:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input
In-Reply-To: <20260623204824.691832-1-alexander.sverdlin@gmail.com>
The Allwinner A100/A133 SoCs have an LRADC which is compatible with the
versions in existing SoCs. Add a compatible string for A100, with the R329
fallback.
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
Changelog:
v5:
- no changes
v4:
- new patch
.../bindings/input/allwinner,sun4i-a10-lradc-keys.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/input/allwinner,sun4i-a10-lradc-keys.yaml b/Documentation/devicetree/bindings/input/allwinner,sun4i-a10-lradc-keys.yaml
index 6bdb8040be65..524c8b51f53f 100644
--- a/Documentation/devicetree/bindings/input/allwinner,sun4i-a10-lradc-keys.yaml
+++ b/Documentation/devicetree/bindings/input/allwinner,sun4i-a10-lradc-keys.yaml
@@ -23,6 +23,7 @@ properties:
- const: allwinner,sun50i-r329-lradc
- items:
- enum:
+ - allwinner,sun50i-a100-lradc
- allwinner,sun50i-h616-lradc
- allwinner,sun20i-d1-lradc
- const: allwinner,sun50i-r329-lradc
--
2.54.0
^ permalink raw reply related
* [PATCH v5 3/6] dt-bindings: arm: sunxi: Add Baijie HelperBoard A133 compatible
From: Alexander Sverdlin @ 2026-06-23 20:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input, Conor Dooley
In-Reply-To: <20260623204824.691832-1-alexander.sverdlin@gmail.com>
Baijie HelperBoard A133 is a development board around their A133 Core
board. Introduce a compatible for both the Core and the development
boards.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
Changelog:
v5:
- no changes
v4:
- renamed "Baijie Helper A133" -> "Baijie A133 HelperBoard"
- renamed "baijie,helper-a133" -> "baijie,helperboard-a133"
v3:
- no separate section for "core" .dtsi
v2:
- introduced baijie,helper-a133-core compatible for the Core (SoM) board
Documentation/devicetree/bindings/arm/sunxi.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
index e6443c266fa1..82dd58b95f8a 100644
--- a/Documentation/devicetree/bindings/arm/sunxi.yaml
+++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
@@ -96,6 +96,12 @@ properties:
- const: allwinner,ba10-tvbox
- const: allwinner,sun4i-a10
+ - description: Baijie A133 HelperBoard
+ items:
+ - const: baijie,helperboard-a133
+ - const: baijie,helperboard-a133-core
+ - const: allwinner,sun50i-a100
+
- description: BananaPi
items:
- const: lemaker,bananapi
--
2.54.0
^ permalink raw reply related
* [PATCH v5 2/6] dt-bindings: vendor-prefixes: Add Shenzhen Baijie Technology Co., Ltd.
From: Alexander Sverdlin @ 2026-06-23 20:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input, Conor Dooley, Paul Kocialkowski
In-Reply-To: <20260623204824.691832-1-alexander.sverdlin@gmail.com>
Shenzhen Baijie Technology Co., Ltd. focuses on R&D and production of
embedded products as well as customization of embedded solutions.
Link: https://szbaijie.com/
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Paul Kocialkowski <paulk@sys-base.io>
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
Changelog:
v2-v5:
- no changes
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 6b9fb6a6bf0b..88225786e216 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -229,6 +229,8 @@ patternProperties:
description: Azoteq (Pty) Ltd
"^azw,.*":
description: Shenzhen AZW Technology Co., Ltd.
+ "^baijie,.*":
+ description: Shenzhen Baijie Technology Co., Ltd.
"^baikal,.*":
description: BAIKAL ELECTRONICS, JSC
"^bananapi,.*":
--
2.54.0
^ permalink raw reply related
* [PATCH v5 1/6] arm64: defconfig: Enable Allwinner LRADC input driver
From: Alexander Sverdlin @ 2026-06-23 20:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input
In-Reply-To: <20260623204824.691832-1-alexander.sverdlin@gmail.com>
Enable Allwinner LRADC input driver as module to support buttons on Baijie
HelperBoard A133.
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
Changelog:
v4-v5:
- no changes
v3:
- new patch
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 654a102cb5bc..c267f0906460 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -527,6 +527,7 @@ CONFIG_KEYBOARD_GPIO=y
CONFIG_KEYBOARD_GPIO_POLLED=m
CONFIG_KEYBOARD_SNVS_PWRKEY=m
CONFIG_KEYBOARD_IMX_SC_KEY=m
+CONFIG_KEYBOARD_SUN4I_LRADC=m
CONFIG_KEYBOARD_CROS_EC=y
CONFIG_KEYBOARD_MTK_PMIC=m
CONFIG_MOUSE_ELAN_I2C=m
--
2.54.0
^ permalink raw reply related
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