* Re: [PATCH 2/2] mmc: core: Add post-power-off-delay-ms support
From: sashiko-bot @ 2026-07-16 23:37 UTC (permalink / raw)
To: Judith Mendez; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260716232641.2659710-3-jm@ti.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `post-power-off-delay-ms` hardware constraint is only enforced in `mmc_power_cycle()`, leaving decoupled power-off/power-on sequences vulnerable to hardware timing violations.
- [Low] Explicitly configuring a 0 ms post-power-off delay in the device tree is ignored, falling back to the 1 ms default.
--
--- Patch [2]: [PATCH 2/2] mmc: core: Add post-power-off-delay-ms support ---
commit 36ce7de56b9b7e7608c56353aede4cb3de73df5b
Author: Judith Mendez <jm@ti.com>
mmc: core: Add post-power-off-delay-ms support
This commit adds support for post-power-off-delay-ms, allowing a
configurable delay after an MMC power off.
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 29e80e5f928e9..f9049aaf44394 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1394,8 +1394,12 @@ void mmc_power_off(struct mmc_host *host)
> void mmc_power_cycle(struct mmc_host *host, u32 ocr)
> {
> mmc_power_off(host);
> - /* Wait at least 1 ms according to SD spec */
> - mmc_delay(1);
> + if (host->post_power_off_delay_ms) {
> + mmc_delay(host->post_power_off_delay_ms);
> + } else {
> + /* Wait at least 1 ms according to SD spec */
> + mmc_delay(1);
> + }
[Severity: Medium]
Will this delay be missed for decoupled power-off and power-on sequences?
If a device relies on post-power-off-delay-ms (e.g., 100ms) but goes through a
runtime PM suspend or system suspend, mmc_power_off() will be called, followed
later by mmc_power_up(). Because the delay is only enforced here in
mmc_power_cycle(), will the decoupled off/on path violate the power-off time
constraint?
Could mmc_power_off() record a timestamp, and mmc_power_up() calculate the
elapsed time to only delay the remaining required duration?
[Severity: Low]
Does this ignore an explicitly configured 0 ms delay?
If a developer explicitly specifies post-power-off-delay-ms = <0> in the
device tree, this condition will evaluate to false and unexpectedly fall
back to the 1 ms default.
Could this be resolved by initializing the default value in mmc_alloc_host()
and removing the if/else condition here?
> mmc_power_up(host, ocr);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260716232641.2659710-1-jm@ti.com?part=2
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy
From: Thinh Nguyen @ 2026-07-16 23:53 UTC (permalink / raw)
To: Peter Chen
Cc: Thinh Nguyen, Krzysztof Kozlowski, Dmitry Baryshkov,
Krishna Kurapati, Greg Kroah-Hartman, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Wesley Cheng,
linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <alhDjZ0gJZq_7G6z@hu-petche-lv.qualcomm.com>
On Wed, Jul 15, 2026, Peter Chen wrote:
> On 26-07-16 00:09:47, Thinh Nguyen wrote:
> > >
> > > Only the new user for adding phy_type="eusb2" at dts needs to check the
> > > controller logic, and this is what the user needs to do that check the
> > > new added device tree property logic at the driver.
> >
> > To clarify, are you suggesting to introduce a new phy_type property to
> > indicate phy type? If so, that sounds similar to qcom,has-eusb2-phy but
> > more generic. Also, where do you plan to define it (dwc3 or phy node)?
> >
>
> No, I meant just adding new entry at below usbphy_modes array, that's the
> easier way. At the current code at of.c, it indeed mixes description of
> the phy interface (between controller and phy) and phy type, the array
> names phy interface, and dt property names "phy_type".
The naming of the property is unfortunate, but the usb.yaml explicitly
documents "phy_type" is for configuring the controller base on
phy-controller interface. Repurposing it to also mean phy type would be
a semantic change and may be harder to convince the DT maintainers to
accept.
>
> diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c
> index 1ab134f45d67..5cbf17d493ad 100644
> --- a/drivers/usb/phy/of.c
> +++ b/drivers/usb/phy/of.c
> @@ -16,6 +16,7 @@ static const char *const usbphy_modes[] = {
> [USBPHY_INTERFACE_MODE_ULPI] = "ulpi",
> [USBPHY_INTERFACE_MODE_SERIAL] = "serial",
> [USBPHY_INTERFACE_MODE_HSIC] = "hsic",
> + [USBPHY_INTERFACE_MODE_EUSB2] = "eusb2",
> };
>
> line34: err = of_property_read_string(np, "phy_type", &phy_type);
>
> In fact, HSIC is not the interface between controller and PHY, it is
> after UTMI(+) or ULPI, and similar with eUSB2.
>
> The API of_usb_get_phy_mode is mainly used by controller driver
> to know what kinds of PHY it is connected, and do corresponding
> configuration, the configuration may not limit to interface between
> controller and PHY. Do you agree we just take "phy mode" and
> "phy type" are same thing, and delete "INTERFACE" in this file
> and add "eusb2" as a new "phy_type"?
>
So just deleting "INTERFACE" isn't enough. We'd also need to update the
documentation description and its enum list.
Also, for dwc3 core, phy_type = "eusb2" doesn't tell whether the UTMI
interface is 8-bit or 16-bit, which may be needed to configure
GUSB2PHYCFG.
BR,
Thinh
^ permalink raw reply
* Re: [PATCH 0/2] qcom_battmgr: Add batteryless DC-adapter MAINS support
From: Bjorn Andersson @ 2026-07-16 23:54 UTC (permalink / raw)
To: Rakesh Kota
Cc: Krzysztof Kozlowski, Konrad Dybcio, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sebastian Reichel,
linux-arm-msm, devicetree, linux-kernel, linux-pm, jishnu.prakash,
kamal.wadhwa
In-Reply-To: <20260716084850.gssw67izy6vq36tg@hu-kotarake-hyd.qualcomm.com>
On Thu, Jul 16, 2026 at 02:18:50PM +0530, Rakesh Kota wrote:
> On Fri, Jul 03, 2026 at 11:16:59AM +0200, Krzysztof Kozlowski wrote:
> > On 03/07/2026 11:13, Rakesh Kota wrote:
> > > On Thu, Jul 02, 2026 at 01:28:35PM +0200, Krzysztof Kozlowski wrote:
> > >> On 02/07/2026 13:17, Rakesh Kota wrote:
> > >>> On Tue, Jun 30, 2026 at 04:16:13PM +0200, Konrad Dybcio wrote:
> > >>>> On 5/22/26 12:56 AM, Bjorn Andersson wrote:
> > >>>>> On Tue, May 19, 2026 at 12:29:38PM +0530, Rakesh Kota wrote:
> > >>>>>> On Mon, May 18, 2026 at 04:36:32PM +0200, Konrad Dybcio wrote:
> > >>>>>>> On 5/18/26 3:49 PM, Rakesh Kota wrote:
> > >>>>>>>> On batteryless boards powered by 12V DC adapters, registering the
> > >>>>>>>> power supply as BATTERY causes userspace to incorrectly trigger
> > >>>>>>>> battery power-saving sequences.
> > >>>>>>>
> > >>>>>>> Does battman really offer no way of differentiating whether a battery
> > >>>>>>> is *actually* present in such cases?
> > >>>>>>>
> > >>>>>>> What boards are affected?
> > >>>>>>>
> > >>>>>> Currently, batteryless support is only implemented for the
> > >>>>>> qcs6490-rb3gen2(Kodiak) board.
> > >>>>>>
> > >>>>>
> > >>>>> What do you mean?
> > >>>>>
> > >>>>> Are you saying that the pmic_glink firmware in Kodiak has a one-off hack
> > >>>>> that no other implementation of this firmware has?
> > >>>>>
> > >>>>> My Lenovo IdeaCentre (hamoa) doesn't have battery, what should I do now?
> > >>>>
> > >>>> I was going through my inbox.
> > >>>> This remains unanswered, and I don't see the discussion progressing
> > >>>> without resolving this.
> > >>>>
> > >>> To clarify — there is no platform-side hack involved. The Kodiak board
> > >>> has a batteryless variant, but the pmic_glink firmware provides no way
> > >>> to distinguish whether a battery is physically present or not. Since
> > >>> this can't be determined from firmware, we introduced a DT-based
> > >>> property to describe the hardware configuration explicitly.
> > >>>
> > >>> This is not Kodiak-specific — any batteryless board, including hamoa,
> > >>> can use the same DT property to avoid incorrect BATTERY registration
> > >>
> > >> Unfortunately the answer might be: fix your firmware. Since your
> > >> firmware is the interface and we do not expose battery in DT, then you
> > >> do not get additional properties. You made conscious design choice of
> > >> abstracting some things in the firmware, so you do control that part of
> > >> the software stack. If you make one choice, it might lead to other, like
> > >> rejecting such properties.
> > >>
> > >
> > > This is a hardware limitation — both the debug board and batteryless
> > > platform use the same Batt_ID resistor (10K), so the firmware has no way
> > > to distinguish between the two at runtime. There is nothing to fix on
> > > the firmware side.
> > >
> > > Given this, would a new compatible string (e.g.,qcom,pmic-glink-batteryless)
> > > be preferred over a DT property, or do you have another suggestion?
> >
> > Debug board is not a end device, thus you basically have only one real
> > use case: battery less. Treat everything that.
>
>
> can we have 2 supplies added - (fake) battery and mains? if we detect 10K bat id?
> since mobile/production platforms share the same firmware and driver code,
> the debug board still requires a fake battery registration to keep the common
> code path functional. In that scenario, should we register both the main power
> supply (DC adapter) and a fake battery, so the shared code continues to work
> correctly without requiring platform-specific divergence?
>
My Hamoa-based PC (desktop) currently reports: ac, battery, usb, and
wireless. I would expect that it reports on AC (or no battmgr
interfaces?).
If I understand what you're saying, the pmic-glink firmware is generic
across all Hamoa targets, so there is no configuration in that firmware
to differentiate between this and a laptop usecase?
Similarly, my Hamoa laptop doesn't have wireless charging, but you're
saying the pmic-glink firmware doesn't know about this - it just don't
get any input power on those pins?
I think this is similar to if we had a discrete charger block that we
describe in DeviceTree, we would somehow have to describe the
"configuration" of that component.
But I don't think you considered those other use cases. Perhaps this
should be an optional qcom,battmgr-interfaces = "ac", "usb" ...; taking
the list of interfaces that is actually implemented.
Regards,
Bjorn
> regards
> Rakesh Kota
^ permalink raw reply
* Re: [PATCH v9 2/4] clk: cix: add sky1 audss clock controller
From: Brian Masney @ 2026-07-16 23:56 UTC (permalink / raw)
To: joakim.zhang
Cc: mturquette, sboyd, robh, krzk+dt, conor+dt, p.zabel,
cix-kernel-upstream, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <20260709120125.3997078-3-joakim.zhang@cixtech.com>
Hi Joakim,
Thanks for the patch.
On Thu, Jul 09, 2026 at 08:01:23PM +0800, joakim.zhang@cixtech.com wrote:
> From: Joakim Zhang <joakim.zhang@cixtech.com>
>
> Add a platform driver for the Cix Sky1 AUDSS CRU. The driver maps
> the CRU registers and registers mux, divider and gate clocks for
> DSP, SRAM, HDA, DMAC, I2S, mailbox, watchdog and timer blocks.
>
> Four SoC-level audio reference clocks are enabled as inputs to the
> internal clock tree. The driver releases the AUDSS NOC reset, enables
> runtime PM and instantiates the auxiliary reset device.
>
> Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
> ---
> drivers/clk/Kconfig | 1 +
> drivers/clk/Makefile | 1 +
> drivers/clk/cix/Kconfig | 16 +
> drivers/clk/cix/Makefile | 3 +
> drivers/clk/cix/clk-sky1-audss.c | 1205 ++++++++++++++++++++++++++++++
> 5 files changed, 1226 insertions(+)
> create mode 100644 drivers/clk/cix/Kconfig
> create mode 100644 drivers/clk/cix/Makefile
> create mode 100644 drivers/clk/cix/clk-sky1-audss.c
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 1717ce75a907..cfcaab39068a 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -509,6 +509,7 @@ source "drivers/clk/actions/Kconfig"
> source "drivers/clk/analogbits/Kconfig"
> source "drivers/clk/aspeed/Kconfig"
> source "drivers/clk/bcm/Kconfig"
> +source "drivers/clk/cix/Kconfig"
> source "drivers/clk/eswin/Kconfig"
> source "drivers/clk/hisilicon/Kconfig"
> source "drivers/clk/imgtec/Kconfig"
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index cc108a75a900..87c992f0df54 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -119,6 +119,7 @@ obj-$(CONFIG_ARCH_ARTPEC) += axis/
> obj-$(CONFIG_ARC_PLAT_AXS10X) += axs10x/
> obj-y += bcm/
> obj-$(CONFIG_ARCH_BERLIN) += berlin/
> +obj-y += cix/
> obj-$(CONFIG_ARCH_DAVINCI) += davinci/
> obj-$(CONFIG_COMMON_CLK_ESWIN) += eswin/
> obj-$(CONFIG_ARCH_HISI) += hisilicon/
> diff --git a/drivers/clk/cix/Kconfig b/drivers/clk/cix/Kconfig
> new file mode 100644
> index 000000000000..c92a9a873893
> --- /dev/null
> +++ b/drivers/clk/cix/Kconfig
> @@ -0,0 +1,16 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Audio subsystem clock support for Cixtech SoC family
> +menu "Clock support for Cixtech audss"
> +
> +config CLK_SKY1_AUDSS
> + tristate "Cixtech Sky1 Audio Subsystem Clock Driver"
> + depends on ARCH_CIX || COMPILE_TEST
> + select AUXILIARY_BUS
> + select REGMAP_MMIO
> + select RESET_CONTROLLER
> + help
> + Support for the Audio Subsystem clock controller present on
> + Cixtech Sky1 SoC. This driver provides mux, divider and gate
> + clocks for DSP, I2S, HDA and related blocks in the audio
> + subsystem. Say M or Y here if you want to build this driver.
> +endmenu
> diff --git a/drivers/clk/cix/Makefile b/drivers/clk/cix/Makefile
> new file mode 100644
> index 000000000000..bc612f1d08b2
> --- /dev/null
> +++ b/drivers/clk/cix/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_CLK_SKY1_AUDSS) += clk-sky1-audss.o
> diff --git a/drivers/clk/cix/clk-sky1-audss.c b/drivers/clk/cix/clk-sky1-audss.c
> new file mode 100644
> index 000000000000..5f9133232eb1
> --- /dev/null
> +++ b/drivers/clk/cix/clk-sky1-audss.c
> @@ -0,0 +1,1205 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// Copyright 2026 Cix Technology Group Co., Ltd.
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
You can drop this include since you are only using of_device_id. You
will get it via two different chains:
<linux/platform_device.h>
-> <linux/device.h>
-> <linux/device/driver.h>
-> <linux/device-id/of.h>
linux/clk-provider.h -> linux/of.h -> linux/device-id/of.h
(Pasted from a review on a different series a little bit ago.)
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +
> +#include <dt-bindings/clock/cix,sky1-audss-cru.h>
> +
> +#define INFO_HIFI0 0x00
> +#define INFO_CLK_GATE 0x10
> +#define INFO_CLK_DIV 0x14
> +#define INFO_CLK_MUX 0x18
> +#define INFO_MCLK 0x70
> +
> +#define SKY1_AUDSS_CLK_PARENTS_CNT 4
> +#define SKY1_AUDSS_NUM_CLKS (CLK_MCLK4 + 1)
> +
> +static u32 sky1_reg_save[][2] = {
> + { INFO_HIFI0, 0 },
> + { INFO_CLK_GATE, 0 },
> + { INFO_CLK_DIV, 0 },
> + { INFO_CLK_MUX, 0 },
> + { INFO_MCLK, 0 },
> +};
> +
> +static const char * const sky1_audss_clk_names[SKY1_AUDSS_CLK_PARENTS_CNT] = {
> + "x8k", "x11k", "sys", "48m",
> +};
> +
> +static const u32 sky1_clk_rate_default[SKY1_AUDSS_CLK_PARENTS_CNT] = {
> + 294912000,
> + 270950400,
> + 800000000,
> + 48000000,
> +};
> +
> +static const char * const dsp_clk_parent[] = {
> + "audio_clk4"
> +};
> +
> +static const char * const dsp_bclk_parent[] = {
> + "audio_clk4_div2"
> +};
> +
> +static const char * const dsp_pbclk_parent[] = {
> + "audio_clk4_div4"
> +};
> +
> +static const char * const sram_axi_parent[] = {
> + "audio_clk4_div2"
> +};
> +
> +static const char * const hda_sys_parent[] = {
> + "audio_clk4_div2"
> +};
> +
> +static const char * const hda_hda_parent[] = {
> + "audio_clk5"
> +};
> +
> +static const char * const dmac_axi_parent[] = {
> + "audio_clk4_div2"
> +};
> +
> +static const char * const wdg_apb_parent[] = {
> + "audio_clk5_div2"
> +};
> +
> +static const char * const wdg_wdg_parent[] = {
> + "audio_clk5_div2"
> +};
> +
> +static const char * const timer_apb_parent[] = {
> + "audio_clk4_div4"
> +};
> +
> +static const char * const timer_timer_parent[] = {
> + "audio_clk5_div2"
> +};
> +
> +static const char * const mailbox_apb_parent[] = {
> + "audio_clk4_div4"
> +};
> +
> +static const char * const i2s_apb_parent[] = {
> + "audio_clk4_div4"
> +};
> +
> +static const char * const i2s0_parents[] = {
> + "audio_clk0", "audio_clk2"
> +};
> +
> +static const char * const i2s1_parents[] = {
> + "audio_clk0", "audio_clk2"
> +};
> +
> +static const char * const i2s2_parents[] = {
> + "audio_clk0", "audio_clk2"
> +};
> +
> +static const char * const i2s3_parents[] = {
> + "audio_clk0", "audio_clk2"
> +};
> +
> +static const char * const i2s4_parents[] = {
> + "audio_clk0", "audio_clk2"
> +};
> +
> +static const char * const i2s5_parents[] = {
> + "audio_clk0", "audio_clk2"
> +};
> +
> +static const char * const i2s6_parents[] = {
> + "audio_clk0", "audio_clk2"
> +};
> +
> +static const char * const i2s7_parents[] = {
> + "audio_clk0", "audio_clk2"
> +};
> +
> +static const char * const i2s8_parents[] = {
> + "audio_clk0", "audio_clk2"
> +};
> +
> +static const char * const i2s9_parents[] = {
> + "audio_clk0", "audio_clk2"
> +};
> +
> +static const char * const mclk_parents[] = {
> + "audio_clk0", "audio_clk2"
> +};
> +
> +static const u32 i2s3_mux_table[] = { 0, 2 };
> +static const u32 i2s4_mux_table[] = { 0, 2 };
> +
> +/*
> + * audss composite clock definition
> + */
> +struct muxdiv_cfg {
> + int offset;
> + u8 shift;
> + u8 width;
> + u8 flags;
> +};
> +
> +struct gate_cfg {
> + int offset;
> + u8 shift;
> + u8 flags;
> +};
> +
> +struct composite_clk_cfg {
> + u32 id;
> + const char * const name;
> + const char * const *parent_names;
> + int num_parents;
> + const u32 *mux_table;
> + struct muxdiv_cfg *mux_cfg;
> + struct muxdiv_cfg *div_cfg;
> + struct gate_cfg *gate_cfg;
> + unsigned long flags;
> +};
> +
> +#define CFG(_id,\
> + _name,\
> + _parent_names,\
> + _mux_table,\
> + _mux_offset, _mux_shift, _mux_width, _mux_flags,\
> + _div_offset, _div_shift, _div_width, _div_flags,\
> + _gate_offset, _gate_shift, _gate_flags,\
> + _flags)\
> +{\
> + .id = _id,\
> + .name = _name,\
> + .parent_names = _parent_names,\
> + .num_parents = ARRAY_SIZE(_parent_names),\
> + .mux_table = _mux_table,\
> + .mux_cfg = &(struct muxdiv_cfg) { _mux_offset, _mux_shift, _mux_width, _mux_flags },\
> + .div_cfg = &(struct muxdiv_cfg) { _div_offset, _div_shift, _div_width, _div_flags },\
> + .gate_cfg = &(struct gate_cfg) { _gate_offset, _gate_shift, _gate_flags },\
> + .flags = _flags,\
> +}
> +
> +static const struct composite_clk_cfg sky1_audss_clks[] = {
> + /* dsp */
> + CFG(CLK_DSP_CLK,
> + "audss_dsp_clk",
> + dsp_clk_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_HIFI0, 0, 0,
> + 0),
> + CFG(CLK_DSP_BCLK,
> + "audss_dsp_bclk",
> + dsp_bclk_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + -1, 0, 0,
> + 0),
> + CFG(CLK_DSP_PBCLK,
> + "audss_dsp_pbclk",
> + dsp_pbclk_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + -1, 0, 0,
> + 0),
> + /* sram */
> + CFG(CLK_SRAM_AXI,
> + "audss_sram_axi",
> + sram_axi_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 16, 0,
> + 0),
> + /* hda */
> + CFG(CLK_HDA_SYS,
> + "audss_hda_sys",
> + hda_sys_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 14, 0,
> + 0),
> + CFG(CLK_HDA_HDA,
> + "audss_hda_hda",
> + hda_hda_parent,
> + NULL,
> + -1, 0, 0, 0,
> + -1, 0, 0, 0,
> + INFO_CLK_GATE, 14, 0,
> + 0),
> + /* dmac */
> + CFG(CLK_DMAC_AXI,
> + "audss_dmac_axi",
> + dmac_axi_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 15, 0,
> + 0),
> + /* wdg */
> + CFG(CLK_WDG_APB,
> + "audss_wdg_apb",
> + wdg_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + -1, 0, 0, 0,
> + INFO_CLK_GATE, 10, 0,
> + 0),
> + CFG(CLK_WDG_WDG,
> + "audss_wdg_wdg",
> + wdg_wdg_parent,
> + NULL,
> + -1, 0, 0, 0,
> + -1, 0, 0, 0,
> + INFO_CLK_GATE, 10, 0,
> + 0),
> + /* timer */
> + CFG(CLK_TIMER_APB,
> + "audss_timer_apb",
> + timer_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 11, 0,
> + 0),
> + CFG(CLK_TIMER_TIMER,
> + "audss_timer_timer",
> + timer_timer_parent,
> + NULL,
> + -1, 0, 0, 0,
> + -1, 0, 0, 0,
> + INFO_CLK_GATE, 11, 0,
> + 0),
> + /* mailbox: mb0(ap->dsp), mb1(dsp->ap) */
> + CFG(CLK_MB_0_APB,
> + "audss_mb_0_apb",
> + mailbox_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + -1, 0, 0, 0,
> + INFO_CLK_GATE, 12, 0,
> + 0),
> + CFG(CLK_MB_1_APB,
> + "audss_mb_1_apb",
> + mailbox_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + -1, 0, 0, 0,
> + INFO_CLK_GATE, 13, 0,
> + 0),
> + /* i2s */
> + CFG(CLK_I2S0_APB,
> + "audss_i2s0_apb",
> + i2s_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 0, 0,
> + 0),
> + CFG(CLK_I2S1_APB,
> + "audss_i2s1_apb",
> + i2s_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 1, 0,
> + 0),
> + CFG(CLK_I2S2_APB,
> + "audss_i2s2_apb",
> + i2s_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 2, 0,
> + 0),
> + CFG(CLK_I2S3_APB,
> + "audss_i2s3_apb",
> + i2s_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 3, 0,
> + 0),
> + CFG(CLK_I2S4_APB,
> + "audss_i2s4_apb",
> + i2s_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 4, 0,
> + 0),
> + CFG(CLK_I2S5_APB,
> + "audss_i2s5_apb",
> + i2s_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 5, 0,
> + 0),
> + CFG(CLK_I2S6_APB,
> + "audss_i2s6_apb",
> + i2s_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 6, 0,
> + 0),
> + CFG(CLK_I2S7_APB,
> + "audss_i2s7_apb",
> + i2s_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 7, 0,
> + 0),
> + CFG(CLK_I2S8_APB,
> + "audss_i2s8_apb",
> + i2s_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 8, 0,
> + 0),
> + CFG(CLK_I2S9_APB,
> + "audss_i2s9_apb",
> + i2s_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 9, 0,
> + 0),
> + CFG(CLK_I2S0,
> + "audss_i2s0",
> + i2s0_parents,
> + NULL,
> + INFO_CLK_MUX, 0, 2, 0,
> + INFO_CLK_DIV, 2, 2, 0,
> + INFO_CLK_GATE, 0, 0,
> + 0),
> + CFG(CLK_I2S1,
> + "audss_i2s1",
> + i2s1_parents,
> + NULL,
> + INFO_CLK_MUX, 2, 2, 0,
> + INFO_CLK_DIV, 4, 2, 0,
> + INFO_CLK_GATE, 1, 0,
> + 0),
> + CFG(CLK_I2S2,
> + "audss_i2s2",
> + i2s2_parents,
> + NULL,
> + INFO_CLK_MUX, 4, 2, 0,
> + INFO_CLK_DIV, 6, 2, 0,
> + INFO_CLK_GATE, 2, 0,
> + 0),
> + CFG(CLK_I2S3,
> + "audss_i2s3",
> + i2s3_parents,
> + i2s3_mux_table,
> + INFO_CLK_MUX, 6, 2, 0,
> + INFO_CLK_DIV, 8, 2, 0,
> + INFO_CLK_GATE, 3, 0,
> + 0),
> + CFG(CLK_I2S4,
> + "audss_i2s4",
> + i2s4_parents,
> + i2s4_mux_table,
> + INFO_CLK_MUX, 8, 2, 0,
> + INFO_CLK_DIV, 10, 2, 0,
> + INFO_CLK_GATE, 4, 0,
> + 0),
> + CFG(CLK_I2S5,
> + "audss_i2s5",
> + i2s5_parents,
> + NULL,
> + INFO_CLK_MUX, 10, 2, 0,
> + INFO_CLK_DIV, 12, 2, 0,
> + INFO_CLK_GATE, 5, 0,
> + 0),
> + CFG(CLK_I2S6,
> + "audss_i2s6",
> + i2s6_parents,
> + NULL,
> + INFO_CLK_MUX, 12, 2, 0,
> + INFO_CLK_DIV, 14, 2, 0,
> + INFO_CLK_GATE, 6, 0,
> + 0),
> + CFG(CLK_I2S7,
> + "audss_i2s7",
> + i2s7_parents,
> + NULL,
> + INFO_CLK_MUX, 14, 2, 0,
> + INFO_CLK_DIV, 16, 2, 0,
> + INFO_CLK_GATE, 7, 0,
> + 0),
> + CFG(CLK_I2S8,
> + "audss_i2s8",
> + i2s8_parents,
> + NULL,
> + INFO_CLK_MUX, 16, 2, 0,
> + INFO_CLK_DIV, 18, 2, 0,
> + INFO_CLK_GATE, 8, 0,
> + 0),
> + CFG(CLK_I2S9,
> + "audss_i2s9",
> + i2s9_parents,
> + NULL,
> + INFO_CLK_MUX, 18, 2, 0,
> + INFO_CLK_DIV, 20, 2, 0,
> + INFO_CLK_GATE, 9, 0,
> + 0),
> + /* mclk */
> + CFG(CLK_MCLK0,
> + "audss_mclk0",
> + mclk_parents,
> + NULL,
> + INFO_MCLK, 5, 1, 0,
> + -1, 0, 0, 0,
> + INFO_MCLK, 0, 0,
> + 0),
> + CFG(CLK_MCLK1,
> + "audss_mclk1",
> + mclk_parents,
> + NULL,
> + INFO_MCLK, 6, 1, 0,
> + -1, 0, 0, 0,
> + INFO_MCLK, 1, 0,
> + 0),
> + CFG(CLK_MCLK2,
> + "audss_mclk2",
> + mclk_parents,
> + NULL,
> + INFO_MCLK, 7, 1, 0,
> + -1, 0, 0, 0,
> + INFO_MCLK, 2, 0,
> + 0),
> + CFG(CLK_MCLK3,
> + "audss_mclk3",
> + mclk_parents,
> + NULL,
> + INFO_MCLK, 8, 1, 0,
> + -1, 0, 0, 0,
> + INFO_MCLK, 3, 0,
> + 0),
> + CFG(CLK_MCLK4,
> + "audss_mclk4",
> + mclk_parents,
> + NULL,
> + INFO_MCLK, 9, 1, 0,
> + -1, 0, 0, 0,
> + INFO_MCLK, 4, 0,
> + 0),
> +};
> +
> +struct sky1_audss_clks_devtype_data {
> + u32 (*reg_save)[2];
> + size_t reg_save_size;
> + const char * const *clk_names;
> + size_t clk_num;
> + const u32 *clk_rate_default;
> + const struct composite_clk_cfg *clk_cfg;
> + size_t clk_cfg_size;
> +};
> +
> +static const struct regmap_config sky1_audss_regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> +};
> +
> +struct sky1_audss_clks_priv {
> + struct device *dev;
> + struct regmap *regmap_cru;
> + struct reset_control *rst_noc;
> + struct clk *clks[SKY1_AUDSS_CLK_PARENTS_CNT];
> + const struct sky1_audss_clks_devtype_data *devtype_data;
> + spinlock_t lock;
> + struct clk_hw_onecell_data *clk_data;
> +};
> +
> +#if IS_ENABLED(CONFIG_RESET_SKY1_AUDSS)
> +
> +static int sky1_audss_reset_controller_register(struct device *dev)
> +{
> + struct auxiliary_device *adev;
> +
> + if (!of_property_present(dev->of_node, "#reset-cells"))
> + return 0;
> +
> + adev = devm_auxiliary_device_create(dev, "reset", NULL);
> + if (!adev)
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +#else
> +
> +static int sky1_audss_reset_controller_register(struct device *dev)
> +{
> + return 0;
> +}
> +
> +#endif
> +
> +/*
> + * clk_ops for audss clock mux/divider/gate
> + */
> +struct sky1_clk_divider {
> + struct clk_divider div;
> + struct regmap *regmap;
> + int offset;
> +};
> +
> +struct sky1_clk_gate {
> + struct clk_gate gate;
> + struct regmap *regmap;
> + int offset;
> +};
> +
> +struct sky1_clk_mux {
> + struct clk_mux mux;
> + struct regmap *regmap;
> + int offset;
> +};
> +
> +static inline struct sky1_clk_mux *to_sky1_clk_mux(struct clk_mux *mux)
> +{
> + return container_of(mux, struct sky1_clk_mux, mux);
> +}
> +
> +static u8 sky1_audss_clk_mux_get_parent(struct clk_hw *hw)
> +{
> + struct clk_mux *mux = to_clk_mux(hw);
> + struct sky1_clk_mux *sky1_mux = to_sky1_clk_mux(mux);
> + u32 val;
> +
> + regmap_read(sky1_mux->regmap, sky1_mux->offset, &val);
> + val = val >> mux->shift;
> + val &= mux->mask;
> +
> + return clk_mux_val_to_index(hw, mux->table, mux->flags, val);
> +}
> +
> +static int sky1_audss_clk_mux_set_parent(struct clk_hw *hw, u8 index)
> +{
> + struct clk_mux *mux = to_clk_mux(hw);
> + u32 val = clk_mux_index_to_val(mux->table, mux->flags, index);
> + struct sky1_clk_mux *sky1_mux = to_sky1_clk_mux(mux);
> + unsigned long flags = 0;
> + u32 reg;
> +
> + if (mux->lock)
> + spin_lock_irqsave(mux->lock, flags);
> + else
> + __acquire(mux->lock);
> +
> + if (mux->flags & CLK_MUX_HIWORD_MASK) {
> + reg = mux->mask << (mux->shift + 16);
> + } else {
> + regmap_read(sky1_mux->regmap, sky1_mux->offset, ®);
> + reg &= ~(mux->mask << mux->shift);
> + }
> + val = val << mux->shift;
> + reg |= val;
> + regmap_write(sky1_mux->regmap, sky1_mux->offset, reg);
> +
> + if (mux->lock)
> + spin_unlock_irqrestore(mux->lock, flags);
> + else
> + __release(mux->lock);
> +
> + return 0;
> +}
> +
> +static int sky1_audss_clk_mux_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + struct clk_mux *mux = to_clk_mux(hw);
> +
> + return clk_mux_determine_rate_flags(hw, req, mux->flags);
> +}
> +
> +static const struct clk_ops sky1_audss_clk_mux_ops = {
> + .get_parent = sky1_audss_clk_mux_get_parent,
> + .set_parent = sky1_audss_clk_mux_set_parent,
> + .determine_rate = sky1_audss_clk_mux_determine_rate,
> +};
> +
> +static inline struct sky1_clk_divider *to_sky1_clk_divider(struct clk_divider *div)
> +{
> + return container_of(div, struct sky1_clk_divider, div);
> +}
> +
> +static unsigned long sky1_audss_clk_divider_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct clk_divider *divider = to_clk_divider(hw);
> + struct sky1_clk_divider *sky1_div = to_sky1_clk_divider(divider);
> + unsigned int val;
> +
> + regmap_read(sky1_div->regmap, sky1_div->offset, &val);
> + val = val >> divider->shift;
> + val &= clk_div_mask(divider->width);
> +
> + return divider_recalc_rate(hw, parent_rate, val, divider->table,
> + divider->flags, divider->width);
> +}
> +
> +static int sky1_audss_clk_divider_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + struct clk_divider *divider = to_clk_divider(hw);
> + struct sky1_clk_divider *sky1_div = to_sky1_clk_divider(divider);
> +
> + /* if read only, just return current value */
> + if (divider->flags & CLK_DIVIDER_READ_ONLY) {
> + u32 val;
> +
> + regmap_read(sky1_div->regmap, sky1_div->offset, &val);
> + val = val >> divider->shift;
> + val &= clk_div_mask(divider->width);
> +
> + return divider_ro_determine_rate(hw, req, divider->table,
> + divider->width,
> + divider->flags, val);
> + }
> +
> + return divider_determine_rate(hw, req, divider->table, divider->width,
> + divider->flags);
> +}
> +
> +static int sky1_audss_clk_divider_set_rate(struct clk_hw *hw,
> + unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct clk_divider *divider = to_clk_divider(hw);
> + struct sky1_clk_divider *sky1_div = to_sky1_clk_divider(divider);
> + int value;
> + unsigned long flags = 0;
> + u32 val;
> +
> + value = divider_get_val(rate, parent_rate, divider->table,
> + divider->width, divider->flags);
> + if (value < 0)
> + return value;
> +
> + if (divider->lock)
> + spin_lock_irqsave(divider->lock, flags);
> + else
> + __acquire(divider->lock);
> +
> + if (divider->flags & CLK_DIVIDER_HIWORD_MASK) {
> + val = clk_div_mask(divider->width) << (divider->shift + 16);
> + } else {
> + regmap_read(sky1_div->regmap, sky1_div->offset, &val);
> + val &= ~(clk_div_mask(divider->width) << divider->shift);
> + }
> + val |= (u32)value << divider->shift;
> + regmap_write(sky1_div->regmap, sky1_div->offset, val);
> +
> + if (divider->lock)
> + spin_unlock_irqrestore(divider->lock, flags);
> + else
> + __release(divider->lock);
> +
> + return 0;
> +}
> +
> +static const struct clk_ops sky1_audss_clk_divider_ops = {
> + .recalc_rate = sky1_audss_clk_divider_recalc_rate,
> + .determine_rate = sky1_audss_clk_divider_determine_rate,
> + .set_rate = sky1_audss_clk_divider_set_rate,
> +};
> +
> +static inline struct sky1_clk_gate *to_sky1_clk_gate(struct clk_gate *gate)
> +{
> + return container_of(gate, struct sky1_clk_gate, gate);
> +}
> +
> +static void sky1_audss_clk_gate_endisable(struct clk_hw *hw, int enable)
> +{
> + struct clk_gate *gate = to_clk_gate(hw);
> + struct sky1_clk_gate *sky1_gate = to_sky1_clk_gate(gate);
> + int set = gate->flags & CLK_GATE_SET_TO_DISABLE ? 1 : 0;
> + unsigned long flags = 0;
> + u32 reg;
> +
> + set ^= enable;
> +
> + if (gate->lock)
> + spin_lock_irqsave(gate->lock, flags);
> + else
> + __acquire(gate->lock);
> +
> + if (gate->flags & CLK_GATE_HIWORD_MASK) {
> + reg = BIT(gate->bit_idx + 16);
> + if (set)
> + reg |= BIT(gate->bit_idx);
> + } else {
> + regmap_read(sky1_gate->regmap, sky1_gate->offset, ®);
> +
> + if (set)
> + reg |= BIT(gate->bit_idx);
> + else
> + reg &= ~BIT(gate->bit_idx);
> + }
> +
> + regmap_write(sky1_gate->regmap, sky1_gate->offset, reg);
> +
> + if (gate->lock)
> + spin_unlock_irqrestore(gate->lock, flags);
> + else
> + __release(gate->lock);
> +}
> +
> +static int sky1_audss_clk_gate_enable(struct clk_hw *hw)
> +{
> + sky1_audss_clk_gate_endisable(hw, 1);
> +
> + return 0;
> +}
> +
> +static void sky1_audss_clk_gate_disable(struct clk_hw *hw)
> +{
> + sky1_audss_clk_gate_endisable(hw, 0);
> +}
> +
> +static int sky1_audss_clk_gate_is_enabled(struct clk_hw *hw)
> +{
> + struct clk_gate *gate = to_clk_gate(hw);
> + struct sky1_clk_gate *sky1_gate = to_sky1_clk_gate(gate);
> + u32 reg;
> +
> + regmap_read(sky1_gate->regmap, sky1_gate->offset, ®);
> +
> + /* if a set bit disables this clk, flip it before masking */
> + if (gate->flags & CLK_GATE_SET_TO_DISABLE)
> + reg ^= BIT(gate->bit_idx);
> +
> + reg &= BIT(gate->bit_idx);
> +
> + return !!reg;
> +}
> +
> +static const struct clk_ops sky1_audss_clk_gate_ops = {
> + .enable = sky1_audss_clk_gate_enable,
> + .disable = sky1_audss_clk_gate_disable,
> + .is_enabled = sky1_audss_clk_gate_is_enabled,
> +};
> +
> +static struct clk_hw *sky1_audss_clk_register(struct device *dev,
> + const char *name,
> + const char * const *parent_names,
> + int num_parents,
> + struct regmap *regmap,
> + const u32 *mux_table,
> + struct muxdiv_cfg *mux_cfg,
> + struct muxdiv_cfg *div_cfg,
> + struct gate_cfg *gate_cfg,
> + unsigned long flags,
> + spinlock_t *lock)
> +{
> + const struct clk_ops *sky1_gate_ops = NULL;
> + const struct clk_ops *sky1_mux_ops = NULL;
> + const struct clk_ops *sky1_div_ops = NULL;
> + struct sky1_clk_divider *sky1_div = NULL;
> + struct sky1_clk_gate *sky1_gate = NULL;
> + struct sky1_clk_mux *sky1_mux = NULL;
> + struct clk_hw *hw = ERR_PTR(-ENOMEM);
> + struct clk_parent_data *parent_data;
> + int i;
> +
> + parent_data = devm_kcalloc(dev, num_parents, sizeof(*parent_data), GFP_KERNEL);
> + if (!parent_data)
> + return ERR_PTR(-ENOMEM);
> +
> + for (i = 0; i < num_parents; i++)
> + parent_data[i].name = parent_names[i];
> +
> + if (mux_cfg->offset >= 0) {
> + sky1_mux = devm_kzalloc(dev, sizeof(*sky1_mux), GFP_KERNEL);
> + if (!sky1_mux)
> + return ERR_PTR(-ENOMEM);
> +
> + sky1_mux->mux.reg = NULL;
> + sky1_mux->mux.shift = mux_cfg->shift;
> + sky1_mux->mux.mask = BIT(mux_cfg->width) - 1;
> + sky1_mux->mux.flags = mux_cfg->flags;
> + sky1_mux->mux.table = mux_table;
> + sky1_mux->mux.lock = lock;
> + sky1_mux_ops = &sky1_audss_clk_mux_ops;
> + sky1_mux->regmap = regmap;
> + sky1_mux->offset = mux_cfg->offset;
> + }
> +
> + if (div_cfg->offset >= 0) {
> + sky1_div = devm_kzalloc(dev, sizeof(*sky1_div), GFP_KERNEL);
> + if (!sky1_div)
> + return ERR_PTR(-ENOMEM);
> +
> + sky1_div->div.reg = NULL;
> + sky1_div->div.shift = div_cfg->shift;
> + sky1_div->div.width = div_cfg->width;
> + sky1_div->div.flags = div_cfg->flags | CLK_DIVIDER_POWER_OF_TWO;
> + sky1_div->div.lock = lock;
> + sky1_div_ops = &sky1_audss_clk_divider_ops;
> + sky1_div->regmap = regmap;
> + sky1_div->offset = div_cfg->offset;
> + }
> +
> + if (gate_cfg->offset >= 0) {
> + sky1_gate = devm_kzalloc(dev, sizeof(*sky1_gate), GFP_KERNEL);
> + if (!sky1_gate)
> + return ERR_PTR(-ENOMEM);
> +
> + sky1_gate->gate.reg = NULL;
> + sky1_gate->gate.bit_idx = gate_cfg->shift;
> + sky1_gate->gate.flags = gate_cfg->flags;
> + sky1_gate->gate.lock = lock;
> + sky1_gate_ops = &sky1_audss_clk_gate_ops;
> + sky1_gate->regmap = regmap;
> + sky1_gate->offset = gate_cfg->offset;
> + }
> +
> + hw = devm_clk_hw_register_composite_pdata(dev, name, parent_data, num_parents,
> + sky1_mux ? &sky1_mux->mux.hw : NULL, sky1_mux_ops,
> + sky1_div ? &sky1_div->div.hw : NULL, sky1_div_ops,
> + sky1_gate ? &sky1_gate->gate.hw : NULL, sky1_gate_ops,
> + flags);
> + if (IS_ERR(hw)) {
> + dev_err(dev, "register %s clock failed with err = %ld\n",
> + name, PTR_ERR(hw));
> + return hw;
> + }
> +
> + return hw;
> +}
> +
> +static int sky1_audss_clks_get(struct sky1_audss_clks_priv *priv)
> +{
> + const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
> + int i;
> +
> + for (i = 0; i < devtype_data->clk_num; i++) {
> + priv->clks[i] = devm_clk_get(priv->dev, devtype_data->clk_names[i]);
> + if (IS_ERR(priv->clks[i]))
> + return dev_err_probe(priv->dev, PTR_ERR(priv->clks[i]),
> + "failed to get clock %s", devtype_data->clk_names[i]);
> + }
> +
> + return 0;
> +}
> +
> +static int sky1_audss_clks_enable(struct sky1_audss_clks_priv *priv)
> +{
> + const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
> + int i, err;
> +
> + for (i = 0; i < devtype_data->clk_num; i++) {
> + err = clk_prepare_enable(priv->clks[i]);
> + if (err) {
> + dev_err(priv->dev, "failed to enable clock %s\n",
> + devtype_data->clk_names[i]);
> + goto err_clks;
> + }
> + }
> +
> + return 0;
> +
> +err_clks:
> + while (--i >= 0)
> + clk_disable_unprepare(priv->clks[i]);
> +
> + return err;
> +}
> +
> +static void sky1_audss_clks_disable(struct sky1_audss_clks_priv *priv)
> +{
> + const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
> + int i;
> +
> + for (i = 0; i < devtype_data->clk_num; i++)
> + clk_disable_unprepare(priv->clks[i]);
> +}
> +
> +static int sky1_audss_clks_set_rate(struct sky1_audss_clks_priv *priv)
> +{
> + const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
> + int i, err;
> +
> + for (i = 0; i < devtype_data->clk_num; i++) {
> + err = clk_set_rate(priv->clks[i], devtype_data->clk_rate_default[i]);
> + if (err) {
> + dev_err(priv->dev, "failed to set clock rate %s\n",
> + devtype_data->clk_names[i]);
> + return err;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/* register sky1 audio subsystem clocks */
> +static int sky1_audss_clk_probe(struct platform_device *pdev)
> +{
> + const struct sky1_audss_clks_devtype_data *devtype_data;
> + struct sky1_audss_clks_priv *priv;
> + struct device *dev = &pdev->dev;
> + struct clk_hw **clk_table;
> + void __iomem *base;
> + int i, ret;
> +
> + devtype_data = device_get_match_data(dev);
> + if (!devtype_data)
> + return -ENODEV;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + spin_lock_init(&priv->lock);
> +
> + priv->clk_data = devm_kzalloc(dev,
> + struct_size(priv->clk_data, hws, SKY1_AUDSS_NUM_CLKS),
> + GFP_KERNEL);
> + if (!priv->clk_data)
> + return -ENOMEM;
> +
> + priv->clk_data->num = SKY1_AUDSS_NUM_CLKS;
> + clk_table = priv->clk_data->hws;
> +
> + base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + priv->regmap_cru = devm_regmap_init_mmio(dev, base, &sky1_audss_regmap_config);
> + if (IS_ERR(priv->regmap_cru))
> + return dev_err_probe(dev, PTR_ERR(priv->regmap_cru),
> + "failed to initialize regmap\n");
> +
> + priv->dev = dev;
> + priv->devtype_data = devtype_data;
> +
> + priv->rst_noc = devm_reset_control_get_exclusive(dev, NULL);
> + if (IS_ERR(priv->rst_noc))
> + return dev_err_probe(dev, PTR_ERR(priv->rst_noc),
> + "failed to get audss noc reset");
> +
> + reset_control_assert(priv->rst_noc);
> +
> + reset_control_deassert(priv->rst_noc);
> +
> + pm_runtime_get_noresume(dev);
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> +
> + platform_set_drvdata(pdev, priv);
> +
> + ret = sky1_audss_clks_get(priv);
> + if (ret)
> + goto err_pm;
> +
> + ret = sky1_audss_clks_enable(priv);
> + if (ret) {
> + dev_err(dev, "failed to enable clocks\n");
> + goto err_pm;
> + }
> +
> + ret = sky1_audss_clks_set_rate(priv);
> + if (ret) {
> + dev_err(dev, "failed to set clocks rate\n");
> + goto fail_clks_set;
So starting here... Is there a double put below? I'll flag it below.
> + }
> +
> + /* audio_clk4 clock fixed divider */
> + clk_table[CLK_AUD_CLK4_DIV2] =
> + devm_clk_hw_register_fixed_factor(dev,
> + "audio_clk4_div2",
> + "audio_clk4",
> + 0,
> + 1, 2);
> + if (IS_ERR(clk_table[CLK_AUD_CLK4_DIV2])) {
> + ret = PTR_ERR(clk_table[CLK_AUD_CLK4_DIV2]);
> + dev_err(dev, "failed to register clock %d, ret:%d\n", CLK_AUD_CLK4_DIV2, ret);
> + goto fail_fixed_clk;
> + }
> +
> + clk_table[CLK_AUD_CLK4_DIV4] =
> + devm_clk_hw_register_fixed_factor(dev,
> + "audio_clk4_div4",
> + "audio_clk4",
> + 0,
> + 1, 4);
> + if (IS_ERR(clk_table[CLK_AUD_CLK4_DIV4])) {
> + ret = PTR_ERR(clk_table[CLK_AUD_CLK4_DIV4]);
> + dev_err(dev, "failed to register clock %d, ret:%d\n", CLK_AUD_CLK4_DIV4, ret);
> + goto fail_fixed_clk;
> + }
> +
> + /* audio_clk5 clock fixed divider */
> + clk_table[CLK_AUD_CLK5_DIV2] =
> + devm_clk_hw_register_fixed_factor(dev,
> + "audio_clk5_div2",
> + "audio_clk5",
> + 0,
> + 1, 2);
> + if (IS_ERR(clk_table[CLK_AUD_CLK5_DIV2])) {
> + ret = PTR_ERR(clk_table[CLK_AUD_CLK5_DIV2]);
> + dev_err(dev, "failed to register clock %d, ret:%d\n", CLK_AUD_CLK5_DIV2, ret);
> + goto fail_fixed_clk;
> + }
> +
> + for (i = 0; i < devtype_data->clk_cfg_size; i++) {
> + clk_table[devtype_data->clk_cfg[i].id] =
> + sky1_audss_clk_register(dev,
> + devtype_data->clk_cfg[i].name,
> + devtype_data->clk_cfg[i].parent_names,
> + devtype_data->clk_cfg[i].num_parents,
> + priv->regmap_cru,
> + devtype_data->clk_cfg[i].mux_table,
> + devtype_data->clk_cfg[i].mux_cfg,
> + devtype_data->clk_cfg[i].div_cfg,
> + devtype_data->clk_cfg[i].gate_cfg,
> + devtype_data->clk_cfg[i].flags,
> + &priv->lock);
> + if (IS_ERR(clk_table[devtype_data->clk_cfg[i].id])) {
> + ret = PTR_ERR(clk_table[devtype_data->clk_cfg[i].id]);
> + dev_err(dev, "failed to register clock %d, ret:%d\n",
> + devtype_data->clk_cfg[i].id, ret);
> + goto fail_fixed_clk;
> + }
> + }
> +
> + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, priv->clk_data);
> + if (ret) {
> + dev_err(dev, "failed to add clock provider: %d\n", ret);
> + goto fail_register;
> + }
> +
> + ret = sky1_audss_reset_controller_register(dev);
> + if (ret)
> + goto fail_register;
> +
> + pm_runtime_put_sync(dev);
> +
> + return 0;
> +
> +fail_register:
> +fail_fixed_clk:
> +fail_clks_set:
Collapse these 3 labels into 1.
> + pm_runtime_put_sync(dev);
> +err_pm:
> + pm_runtime_put_noidle(dev);
So above where I flagged the 'goto fail_clks_set', is this a double put?
> + pm_runtime_disable(dev);
> + return ret;
> +}
> +
> +static void sky1_audss_clk_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> +
> + if (!pm_runtime_status_suspended(dev))
> + pm_runtime_force_suspend(dev);
> +
> + pm_runtime_disable(dev);
I am fairly certain that remove() is called before the devm handlers are
invoked. The clocks will be shutdown, but there's a window where they
are still visible to consumers. I think you should use
devm_add_action_or_reset() so that the cleanup is called in the expected
order.
> +}
> +
> +static int __maybe_unused sky1_audss_clk_runtime_suspend(struct device *dev)
> +{
> + struct sky1_audss_clks_priv *priv = dev_get_drvdata(dev);
> + const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
> + unsigned long flags;
> + int i;
> +
> + spin_lock_irqsave(&priv->lock, flags);
> + for (i = 0; i < devtype_data->reg_save_size; i++)
> + regmap_read(priv->regmap_cru,
> + devtype_data->reg_save[i][0], &devtype_data->reg_save[i][1]);
> + spin_unlock_irqrestore(&priv->lock, flags);
> +
> + sky1_audss_clks_disable(priv);
> +
> + return reset_control_assert(priv->rst_noc);
> +}
> +
> +static int __maybe_unused sky1_audss_clk_runtime_resume(struct device *dev)
> +{
> + struct sky1_audss_clks_priv *priv = dev_get_drvdata(dev);
> + const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
> + unsigned long flags;
> + int i, ret;
> +
> + ret = reset_control_deassert(priv->rst_noc);
> + if (ret)
> + return ret;
> +
> + ret = sky1_audss_clks_enable(priv);
> + if (ret) {
> + dev_err(dev, "failed to enable clocks\n");
> + reset_control_assert(priv->rst_noc);
> + return ret;
> + }
> +
> + spin_lock_irqsave(&priv->lock, flags);
> + for (i = 0; i < devtype_data->reg_save_size; i++)
> + regmap_write(priv->regmap_cru,
> + devtype_data->reg_save[i][0], devtype_data->reg_save[i][1]);
Does it matter what order the registers are written on resume?
Brian
^ permalink raw reply
* Re: [PATCH v7 01/13] dt-bindings: phy: Add zx297520v3 USB phy documentation
From: Rob Herring (Arm) @ 2026-07-17 0:05 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Philipp Zabel, Russell King, linux-phy, Stephen Boyd,
linux-kernel, linux-arm-kernel, devicetree, Michael Turquette,
Brian Masney, Conor Dooley, Neil Armstrong, Vinod Koul,
Krzysztof Kozlowski, linux-clk
In-Reply-To: <20260717-zx29clk-v7-1-408411cfcf36@gmail.com>
On Fri, 17 Jul 2026 00:35:37 +0300, Stefan Dösinger wrote:
> This binding will be used as a subnode of topcrm, which will be added in
> the next patch.
>
> Why is this a child node and not #phy-cells added to topcrm itself,
> like clocks and resets as writing-bindings.rst suggests? Because
> of_phy_provider_register checks if the of_node the PHY provider is added
> to belongs to the device or is a child of the device, so I can't put the
> PHY driver into an MFD (or AUX) child without its own node.
>
> Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
>
> ---
>
> How do I handle examples? It would be identical to the example in
> topcrm. The example needs the reset defines from the next patch in
> either case.
> ---
> .../bindings/phy/zte,zx297520v3-usb-phy.yaml | 96 ++++++++++++++++++++++
> MAINTAINERS | 2 +
> include/dt-bindings/phy/phy-zte-zx297520v3-usb.h | 12 +++
> 3 files changed, 110 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.example.dtb: /example-0/clock-controller: failed to match any schema with compatible: ['zte,zx297520v3-topcrm', 'syscon']
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.example.dtb: clock-controller (zte,zx297520v3-topcrm): 'reg' is a required property
from schema $id: http://devicetree.org/schemas/mfd/syscon-common.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.kernel.org/project/devicetree/patch/20260717-zx29clk-v7-1-408411cfcf36@gmail.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* Re: [PATCH v7 02/13] dt-bindings: clk: zte: Add zx297520v3 top clock and reset controller
From: Rob Herring (Arm) @ 2026-07-17 0:05 UTC (permalink / raw)
To: Stefan Dösinger
Cc: linux-phy, Conor Dooley, Neil Armstrong, Michael Turquette,
linux-kernel, Brian Masney, Vinod Koul, Russell King, linux-clk,
devicetree, Krzysztof Kozlowski, Philipp Zabel, linux-arm-kernel,
Stephen Boyd
In-Reply-To: <20260717-zx29clk-v7-2-408411cfcf36@gmail.com>
On Fri, 17 Jul 2026 00:35:38 +0300, Stefan Dösinger wrote:
> These SoCs have 3 clock and reset controllers: Top, Matrix and LSP.
> Clocks go from oscillator -> top -> matrix -> LSP, with a register in
> top controlling most clocks that get passed to matrix and a register in
> matrix controlling all the clocks that get passed to LSP.
>
> Generally every device has two clocks (one work clock, and one that
> connects it to the bus, I call it PCLK), two reset bits (I don't know
> what the difference is - sometimes asserting one is enough to reset the
> device, sometimes both need to be asserted). PCLK and WCLK are
> controlled by individual gates. Some devices have a mux and/or a
> divider for their work clock. Some devices, like the GPIO controller,
> only have reset bits and no clocks.
>
> The top clock controller is fed by a 26mhz external oscillator and has 4
> PLLs to generate other clock rates. ZTE's kernel mostly relies on the
> boot ROM to set up PLLs, but one LTE-Related PLL is not configured
> on some boards. Therefore my driver contains code to program PLLs. It
> produces identical settings as the boot ROM for the pre-programmed
> frequencies.
>
> Not all clocks will have an explicit user in the end. I am defining a
> lot of them simply to shut them off. The boot loader sets up a few of
> the proprietary timers, which will send regular IRQs (although the
> kernel of course doesn't need to listen to them). I don't plan to add a
> driver for the proprietary timer as I see no use for them - the ARM arch
> timer works just fine. I will add a driver for the very similar
> proprietary watchdog though.
>
> The clock list in this patch is pretty complete but not exhaustive.
> There are other bits that are enabled, but I couldn't deduce what they
> are controlling by trial and error. Some of them seem to do nothing.
> Others cause an instant hang of the board when disabled. It is quite
> likely that a handful more clocks will be added in the future, but not a
> large number.
>
> Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
>
> ---
>
> Changes v6->v7:
> *) Moved the bingings back into clock/ (Krzysztof). use clock-controller@
> in example and in the later DTSI patch.
>
> *) With as lucky find in strings in LTE's cpko.ko blob I could make sense
> of the register that controls which clocks get passed from PLLs into
> the clock distribution inside topcrm. Importantly that also narrows
> which clocks can possibly be passed to Matrix, so I could slim down the
> bindings considerably.
>
> *) Add a usb phy child node with the phy schema added in the previous
> patch. I don't expect the USB status register to ever be in another
> place, so the binding expresses the @84 expectation.
>
> Changes v5->v6:
> Set value for syscon-reboot example (Sashiko). It was my intention to
> set only the lowest bit, and I think Sashiko is right that without
> 'value' being set, all other bits are actively set to 0. It shouldn't
> matter given my understanding of the hardware (afaics all other bits are
> ignored), but actively clearing bits was not my intention.
>
> I haven't changed the name match for "syscon-reboot". I see plenty of
> examples of hardcoding this string as opposed to having a regex for
> syscon-reboot@12345678 in other bindings.
>
> Changes v4->v5:
>
> Rename from zte,zx297520v3-topclk to zte,zx297520v3-topcrm and move to
> soc/zte
> Fix path in MAINTAINERS
> Add syscon-reboot node to the binding
> Give the USB and HSIC PHY resets their own reset control
> ---
> .../bindings/clock/zte,zx297520v3-topcrm.yaml | 123 +++++++++++++++++++++
> MAINTAINERS | 3 +
> include/dt-bindings/clock/zte,zx297520v3-clk.h | 66 +++++++++++
> include/dt-bindings/reset/zte,zx297520v3-reset.h | 32 ++++++
> 4 files changed, 224 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.example.dtb: clock-controller (zte,zx297520v3-topcrm): 'clocks' is a required property
from schema $id: http://devicetree.org/schemas/clock/zte,zx297520v3-topcrm.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.example.dtb: clock-controller (zte,zx297520v3-topcrm): 'clock-names' is a required property
from schema $id: http://devicetree.org/schemas/clock/zte,zx297520v3-topcrm.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.example.dtb: clock-controller (zte,zx297520v3-topcrm): 'syscon-reboot' is a required property
from schema $id: http://devicetree.org/schemas/clock/zte,zx297520v3-topcrm.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.example.dtb: clock-controller (zte,zx297520v3-topcrm): '#clock-cells' is a required property
from schema $id: http://devicetree.org/schemas/clock/zte,zx297520v3-topcrm.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.kernel.org/project/devicetree/patch/20260717-zx29clk-v7-2-408411cfcf36@gmail.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy
From: Thinh Nguyen @ 2026-07-17 0:06 UTC (permalink / raw)
To: Peter Chen
Cc: Krishna Kurapati, Thinh Nguyen, Krzysztof Kozlowski,
Dmitry Baryshkov, Greg Kroah-Hartman, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Wesley Cheng,
linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <ali0dEMbSh4STLmi@hu-petche-lv.qualcomm.com>
On Thu, Jul 16, 2026, Peter Chen wrote:
> On 26-07-16 09:42:35, Krishna Kurapati wrote:
> >
> >
> > >
> > > At minimum, dwc3 needs to be reviewed and updated for dwc->hsphy_mode
> > > check when configuring GUSB2PHYCFG. Regardless, phy type should not be a
> > > usbphy_modes enum as I noted above.
> > >
> > We can just add the enum for eusb2 in of.c as pointed by Peter in [1], read
> > that in dwc3-qcom.c and use it during suspend resume for interrupt handling
> > and to modify the DWC3_GUSB2PHYCFG_EUSB2OPMODE bit once we are modifying it
> > in core.
> >
>
> I have seen fewer platforms use "phy_type" at dts for arm64, dwc3
> core uses it only for special cases and the code was added for
> ten years ago. For the default situation, we may not need to
> change DWC3_GUSB2PHYCFG_EUSB2OPMODE value for UTMI+, Thinh, is it
> correct?
>
The GUSB2PHYCFG.eUSB2OPMODE is only relevant for host mode and mainly
for electrical compliance. Usually by default, the CoreConsultant
setting should have this set correctly. So not explicitly setting it
should be functionally fine (IIRC).
That said, this is separate from the GUSB2PHYCFG.PHYIF, which the core
uses dwc->hsphy_mode to indicate whether the UTMI interface is 8-bit or
16-bit.
BR,
Thinh
^ permalink raw reply
* Re: [PATCH 2/2] power: supply: qcom_battmgr: Add support batteryless boards as MAINS
From: Bjorn Andersson @ 2026-07-17 0:12 UTC (permalink / raw)
To: Rakesh Kota
Cc: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Sebastian Reichel, linux-arm-msm, devicetree, linux-kernel,
linux-pm
In-Reply-To: <20260518-add_dc_in_support-v1-2-31fbaa329879@oss.qualcomm.com>
On Mon, May 18, 2026 at 07:19:40PM +0530, Rakesh Kota wrote:
> Add support for the "qcom,batteryless" device tree property.
> When this boolean property is present, the driver registers the power
> supply as POWER_SUPPLY_TYPE_MAINS instead of BATTERY. This prevents
> userspace from triggering battery power-saving sequences when using
> powered by 12V adapters.
>
> Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> ---
> drivers/power/supply/qcom_battmgr.c | 47 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
> index 490137a23d00e97a9e6ced21d1e66fe637db6c9d..35d266375d8c46b161a64f9c2a8c6054dc2211de 100644
> --- a/drivers/power/supply/qcom_battmgr.c
> +++ b/drivers/power/supply/qcom_battmgr.c
> @@ -332,6 +332,7 @@ struct qcom_battmgr {
> struct qcom_battmgr_wireless wireless;
>
> struct work_struct enable_work;
> + bool batteryless;
This is a local variable used only within qcom_battmgr_probe()
>
> /*
> * @lock is used to prevent concurrent power supply requests to the
> @@ -930,6 +931,47 @@ static const struct power_supply_desc sm8550_bat_psy_desc = {
> .property_is_writeable = qcom_battmgr_bat_is_writeable,
> };
>
> +static int qcom_battmgr_dcin_get_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct qcom_battmgr *battmgr = power_supply_get_drvdata(psy);
> + int ret;
> +
> + if (!battmgr->service_up)
> + return -EAGAIN;
> +
> + ret = qcom_battmgr_bat_sm8350_update(battmgr, psp);
> + if (ret < 0)
> + return ret;
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_STATUS:
> + val->intval = battmgr->status.status;
> + break;
> + case POWER_SUPPLY_PROP_PRESENT:
> + val->intval = battmgr->info.present;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static const enum power_supply_property dcin_props[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_PRESENT,
> +};
> +
> +static const struct power_supply_desc dcin_psy_desc = {
Everything else in this driver is prefixed based on which firmware
interface it's operating against, but you decided to ignore that
convention...
Don't do that.
> + .name = "qcom-battmgr-dcin",
Isn't this what we call qcom-battmgr-ac for the compute targets?
Is there any reason to not unify the naming?
> + .type = POWER_SUPPLY_TYPE_MAINS,
> + .properties = dcin_props,
> + .num_properties = ARRAY_SIZE(dcin_props),
> + .get_property = qcom_battmgr_dcin_get_property,
> +};
> +
> static int qcom_battmgr_ac_get_property(struct power_supply *psy,
> enum power_supply_property psp,
> union power_supply_propval *val)
> @@ -1652,6 +1694,7 @@ static int qcom_battmgr_probe(struct auxiliary_device *adev,
> mutex_init(&battmgr->lock);
> init_completion(&battmgr->ack);
>
> + battmgr->batteryless = device_property_read_bool(dev, "qcom,batteryless");
> match = of_match_device(qcom_battmgr_of_variants, dev->parent);
> if (match)
> battmgr->variant = (unsigned long)match->data;
> @@ -1690,7 +1733,9 @@ static int qcom_battmgr_probe(struct auxiliary_device *adev,
> return dev_err_probe(dev, PTR_ERR(battmgr->wls_psy),
> "failed to register wireless charing power supply\n");
> } else {
> - if (battmgr->variant == QCOM_BATTMGR_SM8550)
> + if (battmgr->batteryless)
> + psy_desc = &dcin_psy_desc;
The psy_desc here is what is used to register the bat_psy on the line
below. Why would you register "dcin" as your bat_psy?
Is there an actual reason or are you just lazy?
Regards,
Bjorn
> + else if (battmgr->variant == QCOM_BATTMGR_SM8550)
> psy_desc = &sm8550_bat_psy_desc;
> else
> psy_desc = &sm8350_bat_psy_desc;
>
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: soc: qcom: pmic-glink: Document batteryless property
From: Bjorn Andersson @ 2026-07-17 0:14 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rakesh Kota, Konrad Dybcio, Krzysztof Kozlowski, Kamal Wadhwa,
jishnu.prakash, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sebastian Reichel, linux-arm-msm, devicetree,
linux-kernel, linux-pm
In-Reply-To: <dqe3ntmtmjqixp2l5n7qm6zpcwh4sizr7jd5g3cl3tzpvl6tvo@casnjg3orldd>
On Thu, Jul 02, 2026 at 02:49:37PM +0300, Dmitry Baryshkov wrote:
> On Thu, Jul 02, 2026 at 05:00:10PM +0530, Rakesh Kota wrote:
> > On Mon, May 25, 2026 at 05:31:39PM +0530, Rakesh Kota wrote:
> > > On Fri, May 22, 2026 at 11:24:19AM +0200, Konrad Dybcio wrote:
> > > > On 5/21/26 11:58 AM, Krzysztof Kozlowski wrote:
> > > > > On 21/05/2026 10:46, Konrad Dybcio wrote:
> > > > >> On 5/21/26 9:20 AM, Krzysztof Kozlowski wrote:
> > > > >>>> Since firmware does not have a way to dynamically tell if it on a
> > > > >>>> debug-board powered device or a DCIN powered device, We are required to
> > > > >>>> add this new DT property.
> > > > >>>
> > > > >>> Neither debug-board powered device nor battery-less will have
> > > > >>> monitored-battery, thus again, why lack of that property cannot tell you
> > > > >>> what you need?
> > > > >>
> > > > >> A device with a battery will not have a monitored-battery either
> > > > > But why? If for such device property "no battery" is suitable, then for
> > > > > me "monitored-battery" is suitable as well. IOW, if you say that having
> > > > > a property describing batter is not a accurate hardware property here,
> > > > > then neither saying "no battery" is, because no batter is basically some
> > > > > sort of battery (just like empty set is still a set, empty array is
> > > > > still an array).
> > > >
> > > > The battmgr service running on one of the remoteprocs already has all
> > > > the information about the battery and it also handles all the type-c,
> > > > PD and charger configuration, only letting the OS know about the
> > > > results.
> > > >
> > > > Hence, unless there's some other hardware at play (e.g. for custom
> > > > 200 W charging), which wasn't fully implemented in the QC firmware,
> > > > there is no reason to describe a battery separately, since the OS
> > > > can't do anything useful with that information
> > > >
> > > > In some abstract way, perhaps monitored_battery = <&pmic_glink> could
> > > > be thought of as valid (since that's the data source the OS gets to
> > > > see)
> > >
> > > Should we use monitored-battery = <&pmic_glink>; If so, would it be
> > > incorrect to mark pmic_glink as simple-battery, as that may cause the
> > > schema validation to fail?
> >
> > Hi @Konrad Dybcio, @Krzysztof Kozlowski ,
> >
> > Following up on the earlier discussion — is monitored-battery =
> > <&pmic_glink>; a valid pattern? And if so, would marking pmic_glink as
> > simple-battery cause DT schema validation failures?
>
> I'd say, no, monitored-battery should be pointing to the battery device.
> Please fix this on the firmware side, so that the firmware can report
> having no battery at all.
>
This won't be fixed in the firmware on my devices.
Regards,
Bjorn
> --
> With best wishes
> Dmitry
^ permalink raw reply
* Re: [PATCH v7 07/17] iio: test: add kunit tests for channel prefix naming generation
From: Jonathan Cameron @ 2026-07-17 0:27 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Rodrigo Alencar, Jonathan Cameron, Rodrigo Alencar via B4 Relay,
linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Jonathan Corbet, Shuah Khan, Kees Cook,
Gustavo A. R. Silva
In-Reply-To: <alcpb-7GVSfk6jFB@ashevche-desk.local>
On Wed, 15 Jul 2026 09:32:15 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Tue, Jul 14, 2026 at 06:08:12PM -0700, Jonathan Cameron wrote:
> > On Mon, 13 Jul 2026 10:52:56 +0100
> > Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> > > On 12/07/26 02:09, Jonathan Cameron wrote:
> > > > On Tue, 07 Jul 2026 15:04:28 +0100
> > > > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
>
> ...
>
> > > > > Because __iio_chan_prefix_emit() is static, the test translation unit
> > > > > is pulled into industrialio-core.c.
>
> KUnit also has static/non-static automation via a macro (defined in the
> kunit/visibility.h) and I see that's used in the below example.
>
> > > > Isn't there some magic route cases like this that makes it non static
> > > > only when self tests are enabled?
> > > > Claude tells me to look at include/kunit/visibility.h
> > >
> > > There is, Although I think that using
> > >
> > > #if IS_ENABLED(CONFIG_IIO_CHANNEL_PREFIX_KUNIT_TEST)
> > > #include "test/iio-test-channel-prefix.c"
> > > #endif
> > >
> > > was more straight forward, less invasive and easier to change than..
>
> Maybe, but thanks to this thread, I fixed other modules that use their own
> approach to use the standard KUnit infra for this (as below).
>
> > > /* In "drivers/iio/industrialio-core.c" */
> > >
> > > #include <kunit/visibility.h>
> > > ...
> > > VISIBLE_IF_KUNIT ssize_t __iio_chan_prefix_emit(...)
> > > {
> > > ...
> > > }
> > > EXPORT_SYMBOL_IF_KUNIT(__iio_chan_prefix_emit);
> > >
> > > /* In "iio_core.h" */
> > >
> > > #if IS_ENABLED(CONFIG_KUNIT)
> > > ssize_t __iio_chan_prefix_emit(...);
> > > #endif
> > >
> > > /* In "drivers/iio/test/iio-test-channel-prefix.c" */
> > >
> > > #include <kunit/visibility.h>
> > > #include <iio_core.h>
> > > ...
> > > MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
> > > ...
> > > // Use __iio_chan_prefix_emit() in tests
> >
> > I'd rather this wasn't built into the core module. So prefer you jump
> > though those hoops.
>
> Hmm... The above (while being verbose) is the standard way of how we export
> symbols for KUnit tests. Do you have a better alternative that everyone can
> use? (Not only IIO subsystem.)
Ah. I put that comment in an unhelpful place. I meant the include above not this bit.
Absolutely fine with the exports being there.
Jonathan
>
^ permalink raw reply
* Re: [PATCH v3 4/4] arm64: dts: qcom: Enable USB controllers on Shikra IQS platform
From: Krishna Kurapati @ 2026-07-17 0:30 UTC (permalink / raw)
To: Konrad Dybcio
Cc: linux-arm-msm, devicetree, linux-kernel, Bjorn Andersson,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley
In-Reply-To: <606db992-446c-4601-805a-d86d0fb74110@oss.qualcomm.com>
On 7/16/2026 11:10 PM, Konrad Dybcio wrote:
> On 7/16/26 6:29 PM, Krishna Kurapati wrote:
>> Enable both usb controllers in device mode on Shikra IQS platform.
>>
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>> ---
>> arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts | 35 +++++++++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts b/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts
>> index 3003a47bd759..2d4e34ec3b82 100644
>> --- a/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts
>> +++ b/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts
>> @@ -38,3 +38,38 @@ &sdhc_1 {
>>
>> status = "okay";
>> };
>> +
>> +&usb_1 {
>> + dr_mode = "peripheral";
>
> Why does this one have to be peripheral if the other boards are otg?
>
Actually I missed adding dr_mode as peripheral for other boards. I put
in commit message that I want them to work in device mode but missed
adding dr_mode property. Since the default behavior is peripheral mode,
i saw no functional issue while testing. Shall I send v2 with either
removing dr mode here in iqs or adding it in cqs/cqm ?
> [...]
>
>> +&usb_2 {
>> + dr_mode = "peripheral";
>> +
>> + status = "okay";
>
> And why do the other boards lack the secondary USB?
As mentioned in cover letter, on pure upstream build, I validated both
controllers on iqs platform and only primary controller on cqs/cqm
platform on a remote access setup.
Regards,
Krishna,
^ permalink raw reply
* Re: [PATCH v4 2/2] pwm: Add Nuvoton MA35D1 PWM controller support
From: Chi-Wen Weng @ 2026-07-17 0:33 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: robh, krzk+dt, conor+dt, linux-arm-kernel, linux-pwm, devicetree,
linux-kernel, cwweng, Trevor Gamblin
In-Reply-To: <alj1UapHAq9f_MiF@monoceros>
Uwe Kleine-König 於 2026/7/16 下午 11:29 寫道:
> Hello,
>
> On Wed, Jun 17, 2026 at 10:59:25AM +0800, Chi-Wen Weng wrote:
>> +#include <linux/bits.h>
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/math64.h>
>> +#include <linux/mod_devicetable.h>
> Please don't include that file, <linux/platform_device.h> should pull in
> the things you need from that file.
>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pwm.h>
>> +
>> +#define MA35D1_REG_PWM_CTL0 0x00
>> +#define MA35D1_REG_PWM_CTL1 0x04
>> +#define MA35D1_REG_PWM_CNTEN 0x20
>> +#define MA35D1_REG_PWM_PERIOD(ch) (0x30 + 4 * (ch))
>> +#define MA35D1_REG_PWM_CMPDAT(ch) (0x50 + 4 * (ch))
>> +#define MA35D1_REG_PWM_WGCTL0 0xb0
>> +#define MA35D1_REG_PWM_WGCTL1 0xb4
>> +#define MA35D1_REG_PWM_POLCTL 0xd4
>> +#define MA35D1_REG_PWM_POEN 0xd8
>> +
>> +#define MA35D1_PWM_CTL1_CNTMODE_MASK(ch) BIT(16 + (ch))
>> +#define MA35D1_PWM_CTL1_OUTMODE_MASK(ch) BIT(24 + ((ch) / 2))
>> +
>> +#define MA35D1_PWM_WGCTL_ACTION_MASK 0x3
>> +#define MA35D1_PWM_WGCTL_ACTION_LOW 1
>> +#define MA35D1_PWM_WGCTL_ACTION_HIGH 2
> If you make this:
>
> #define MA35D1_PWM_WGCTL_ACTION(ch) GENMASK(2 * (ch) + 2, 2 * (ch))
> #define MA35D1_PWM_WGCTL_ACTION_LOW 1
> #define MA35D1_PWM_WGCTL_ACTION_HIGH 2
>
> you can drop the static inlines below.
>
>> +
>> +#define MA35D1_PWM_WGCTL_ZERO_HIGH(ch) \
>> + (MA35D1_PWM_WGCTL_ACTION_HIGH << (2 * (ch)))
>> +#define MA35D1_PWM_WGCTL_CMP_UP_LOW(ch) \
>> + (MA35D1_PWM_WGCTL_ACTION_LOW << (2 * (ch)))
>> +
>> +#define MA35D1_PWM_CNTEN_EN(ch) BIT(ch)
>> +#define MA35D1_PWM_POEN_EN(ch) BIT(ch)
>> +#define MA35D1_PWM_POLCTL_INV(ch) BIT(ch)
>> +
>> +#define MA35D1_PWM_MAX_CMPDAT 0xffff
>> +#define MA35D1_PWM_MAX_PERIOD 0xfffe
>> +#define MA35D1_PWM_MAX_PERIOD_CYCLES (MA35D1_PWM_MAX_PERIOD + 1)
> This is irritating with similar names and different values/semantic.
>
>> +#define MA35D1_PWM_NUM_CHANNELS 6
>> +
>> [...]
>> +static int nuvoton_pwm_probe(struct platform_device *pdev)
>> +{
>> [...]
>> + nuvoton_pwm_init(nvtpwm);
> This clobbers what the hardware is doing. The idea here is to not modify
> the hardware settings at probe time to keep e.g. a backlight configured
> as it was setup by the bootloader and only modify on explicit calls to
> .apply().
>
>> +
>> + chip->ops = &nuvoton_pwm_ops;
>> + chip->atomic = true;
>> +
>> + ret = devm_pwmchip_add(dev, chip);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Unable to add PWM chip\n");
>> +
>> + return 0;
>> +}
> Best regards
> Uwe
Hi Uwe,
Thanks for the review.
On the include, I will drop <linux/mod_devicetable.h> as
<linux/platform_device.h> already provides what is needed here.
For the WGCTL helpers, I will rework this to avoid the extra static inline
helpers and compute the 2-bit action field mask/value locally when
configuring
a channel. The WGCTL action field is 2 bits wide, so I will use the bit
range
[2 * ch + 1 : 2 * ch] for the mask.
I will also rename the maximum value definitions to make the semantics
clearer.
The intent is to keep PERIOD below the 16-bit register field maximum so that
CMPDAT can be programmed greater than PERIOD to generate a 100% duty cycle.
Most importantly, you are right about the probe-time initialization. The
driver
should not reconfigure CTL1/WGCTL for all channels during probe, as that can
disturb a PWM output already configured and enabled by firmware, such as a
backlight.
In the next version I will remove the controller initialization from
probe().
Instead, the driver will configure only the channel being changed from the
.apply() callback. The disable path will only clear POENn and CNTENn and
will
not touch the waveform/control configuration.
Best regards,
Chi-Wen
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: mmc: Add post-power-off-delay-ms property
From: Rob Herring (Arm) @ 2026-07-17 0:35 UTC (permalink / raw)
To: Judith Mendez
Cc: linux-mmc, devicetree, Conor Dooley, linux-kernel, Ulf Hansson,
Krzysztof Kozlowski
In-Reply-To: <20260716232641.2659710-2-jm@ti.com>
On Thu, 16 Jul 2026 18:26:40 -0500, Judith Mendez wrote:
> Add post-power-off-delay-ms property to MMC controller common.
>
> This property shall be used to specify delay if needed after
> deasserting power during MMC power cycles.
>
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
> .../devicetree/bindings/mmc/mmc-controller-common.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml: properties:post-power-off-delay-ms: '$ref' should not be valid under {'const': '$ref'}
hint: Standard unit suffix properties don't need a type $ref
from schema $id: http://devicetree.org/meta-schemas/core.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.kernel.org/project/devicetree/patch/20260716232641.2659710-2-jm@ti.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: iio: adc: Add Nuvoton MA35D1 EADC
From: Jonathan Cameron @ 2026-07-17 0:42 UTC (permalink / raw)
To: Chi-Wen Weng
Cc: jic23, robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy,
linux-arm-kernel, linux-iio, devicetree, linux-kernel, cwweng
In-Reply-To: <20260713081127.115197-2-cwweng.linux@gmail.com>
On Mon, 13 Jul 2026 16:11:26 +0800
Chi-Wen Weng <cwweng.linux@gmail.com> wrote:
> From: Chi-Wen Weng <cwweng@nuvoton.com>
>
> Add devicetree binding for the Enhanced ADC controller found on
> Nuvoton MA35D1 SoCs.
>
> The controller has one register region, up to four end-of-conversion
> interrupts, one functional clock, and an optional reset line. The
> binding also describes the optional external reference supply and the
> optional DMA request supported by the hardware.
>
> ADC input pins are described using standard channel child nodes. The
> external ADC input channels are 0 to 7. Differential inputs are described
> with diff-channels and are limited to the fixed hardware pairs.
>
> Signed-off-by: Chi-Wen Weng <cwweng@nuvoton.com>
Hi, A few comments below,
> ---
> .../bindings/iio/adc/nuvoton,ma35d1-eadc.yaml | 163 ++++++++++++++++++
> 1 file changed, 163 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
> new file mode 100644
> index 000000000000..1be5470643d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
...
> + interrupts:
> + minItems: 1
> + maxItems: 4
> + items:
> + - description: ADINT0 end-of-conversion interrupt
> + - description: ADINT1 end-of-conversion interrupt
> + - description: ADINT2 end-of-conversion interrupt
> + - description: ADINT3 end-of-conversion interrupt
Not sure this doc helps. What are these interrupts for?
The driver only uses one of them so why are there 4?
May well need interrupt-names to allow gaps in the list to
work but hard to tell without more information.
> +
> +patternProperties:
> + '^channel@[0-7]$':
> + type: object
> + $ref: adc.yaml
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + maximum: 7
> +
> + diff-channels: true
Do you need that explicitly. I think adc.yaml already brings that in.
> +
> + allOf:
> + - if:
> + properties:
> + reg:
> + const: 0
> + then:
> + properties:
> + diff-channels:
> + items:
> + - const: 0
> + - const: 4
> + - if:
> + properties:
> + reg:
> + const: 1
> + then:
> + properties:
> + diff-channels:
> + items:
> + - const: 1
> + - const: 5
> + - if:
> + properties:
> + reg:
> + const: 2
> + then:
> + properties:
> + diff-channels:
> + items:
> + - const: 2
> + - const: 6
> + - if:
> + properties:
> + reg:
> + const: 3
> + then:
> + properties:
> + diff-channels:
For these take a look at the description of diff-channels.
It states that where the pairs are hard wired you can just use reg
and not specify diff-channels. Seems to apply here and will greatly
simplify this binding.
> + items:
> + - const: 3
> + - const: 7
> + - if:
> + properties:
> + reg:
> + enum: [4, 5, 6, 7]
What are these channels? There is a reference to one
internally wired one so what are the other 3?
> + then:
> + not:
> + required:
> + - diff-channels
> +
> + required:
> + - reg
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: iio: adc: Add Nuvoton MA35D1 EADC
From: Chi-Wen Weng @ 2026-07-17 0:59 UTC (permalink / raw)
To: Jonathan Cameron
Cc: jic23, robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy,
linux-arm-kernel, linux-iio, devicetree, linux-kernel, cwweng
In-Reply-To: <20260716174237.00004b79@oss.qualcomm.com>
Hi Jonathan,
Thank you for the review.
> Not sure this doc helps. What are these interrupts for?
> The driver only uses one of them so why are there 4?
>
> May well need interrupt-names to allow gaps in the list to
> work but hard to tell without more information.
The hardware has four EADC interrupt outputs, ADINT0 to ADINT3. The
conversion-complete source for each interrupt can be selected through
the EADC interrupt source registers.
However, this initial driver only uses sample module 0 and routes its
end-of-conversion event to ADINT0. So the binding does not need to
describe the unused interrupt outputs at this stage.
I will simplify this in v3 and document only one interrupt entry for
ADINT0. If support for the other ADINT lines is added later, the binding
can be extended with interrupt-names at that point.
I will also drop the redundant maxItems from the interrupts property, as
reported by Rob's bot.
> Do you need that explicitly. I think adc.yaml already brings that in.
No, it is not needed. I will drop the explicit diff-channels property
from the binding.
> For these take a look at the description of diff-channels.
> It states that where the pairs are hard wired you can just use reg
> and not specify diff-channels. Seems to apply here and will greatly
> simplify this binding.
Yes, the differential pairs are fixed in hardware. The valid pairs are
0-4, 1-5, 2-6 and 3-7.
I will rework the channel binding to avoid diff-channels for these fixed
pairs. For fixed differential channels, reg will identify the hardware
pair. For single-ended inputs, I will use single-channel so that the
single-ended input number is explicit and not confused with a fixed
differential pair index.
That should remove the large allOf block and make the binding much
simpler.
> What are these channels? There is a reference to one
> internally wired one so what are the other 3?
Channels 4 to 7 are external ADC input pins. They can be used as
single-ended inputs, and in differential mode they are the fixed negative
inputs for channels 0 to 3.
The internally wired channel is channel 8, which is the VBAT input. This
binding only describes the external ADC input pins, so I will clarify the
description to avoid confusion.
Thanks again. I will address these issues in the next revision.
Best regards,
Chi-Wen
^ permalink raw reply
* [PATCH 1/3] arm64: dts: rockchip: Fix rk3399-roc-pc-plus analog audio
From: Fabio Estevam @ 2026-07-17 1:07 UTC (permalink / raw)
To: heiko
Cc: robh, krzk+dt, conor+dt, devicetree, linux-arm-kernel,
linux-rockchip, Fabio Estevam, stable
From: Fabio Estevam <festevam@nabladev.com>
The ES8388 sound card on the rk3399-roc-pc-plus fails to probe because
i2s1 cannot claim its MCLK pin:
pinctrl: pin gpio4-0 already requested by ff880000.i2s; cannot claim for ff890000.i2s
pinctrl: error -EINVAL: pin-128 (ff890000.i2s)
pinctrl: error -EINVAL: could not request pin 128 (gpio4-0) from group i2s-8ch-mclk-pin
on device rockchip-pinctrl
GPIO4_A0 is routed as SCLK_I2S_8CH_OUT and is used by i2s1 as the
external MCLK for the ES8388 codec. The board dts already removes
GPIO4_A0 from the i2s0_8ch_bus pin group, but i2s0 still claims the
same pin through its bclk_off state.
Since the i2s driver requests both states, this blocks i2s1 pinctrl
setup and leaves the simple-audio-card deferred with a parse error.
Override i2s0_8ch_bus_bclk_off as well, matching the existing
i2s0_8ch_bus override, so GPIO4_A0 is left for i2s1/ES8388 audio.
Cc: stable@vger.kernel.org
Fixes: 6d9a7bd6a13c ("arm64: dts: rockchip: add support for Firefly ROC-RK3399-PC-PLUS")
Signed-off-by: Fabio Estevam <festevam@nabladev.com>
---
arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
index 4f2831097624..f6d62ee51f1b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
@@ -136,6 +136,18 @@ &i2s0_8ch_bus {
<3 RK_PD7 1 &pcfg_pull_none>;
};
+&i2s0_8ch_bus_bclk_off {
+ rockchip,pins =
+ <3 RK_PD0 RK_FUNC_GPIO &pcfg_pull_none>,
+ <3 RK_PD1 1 &pcfg_pull_none>,
+ <3 RK_PD2 1 &pcfg_pull_none>,
+ <3 RK_PD3 1 &pcfg_pull_none>,
+ <3 RK_PD4 1 &pcfg_pull_none>,
+ <3 RK_PD5 1 &pcfg_pull_none>,
+ <3 RK_PD6 1 &pcfg_pull_none>,
+ <3 RK_PD7 1 &pcfg_pull_none>;
+};
+
&i2s1 {
pinctrl-names = "default";
pinctrl-0 = <&i2s_8ch_mclk_pin>, <&i2s1_2ch_bus>;
--
2.43.0
^ permalink raw reply related
* [PATCH 2/3] arm64: dts: rockchip: Enable wireless on rk3399-roc-pc-plus
From: Fabio Estevam @ 2026-07-17 1:07 UTC (permalink / raw)
To: heiko
Cc: robh, krzk+dt, conor+dt, devicetree, linux-arm-kernel,
linux-rockchip, Fabio Estevam
In-Reply-To: <20260717010736.578419-1-festevam@gmail.com>
From: Fabio Estevam <festevam@nabladev.com>
The rk3399-roc-pc-plus board has an Ampak AP6256 Wi-Fi/Bluetooth
module.
Describe the Wi-Fi function on SDIO0 and the Bluetooth function on
UART0. Add the Bluetooth wake and shutdown pinctrl entries, enable
SDIO0 as a non-removable SDIO device, and add the power sequencing
delays needed by the module.
The module uses the RK808 CLKOUT2 output as its 32 kHz low-power
clock. Drop the duplicate HYM8563 clock-output-names property and
remove the same clock from the SDIO power sequencer so the Bluetooth
node can request the shared LPO clock directly.
Signed-off-by: Fabio Estevam <festevam@nabladev.com>
---
.../boot/dts/rockchip/rk3399-roc-pc-plus.dts | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
index f6d62ee51f1b..f87bc7b6afc1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
@@ -123,6 +123,10 @@ es8388: es8388@11 {
};
};
+&hym8563 {
+ /delete-property/ clock-output-names;
+};
+
/* <4 RK_PA0 1 &pcfg_pull_none> is used as i2s_8ch_mclk_pin */
&i2s0_8ch_bus {
rockchip,pins =
@@ -157,6 +161,20 @@ &i2s1 {
};
&pinctrl {
+ bt {
+ bt_enable_h: bt-enable-h {
+ rockchip,pins = <0 RK_PB1 RK_FUNC_GPIO &pcfg_pull_none>;
+ };
+
+ bt_host_wake_l: bt-host-wake-l {
+ rockchip,pins = <0 RK_PA4 RK_FUNC_GPIO &pcfg_pull_none>;
+ };
+
+ bt_wake_l: bt-wake-l {
+ rockchip,pins = <2 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
+ };
+ };
+
es8388 {
ear_ctl_pin: ear-ctl-pin {
rockchip,pins = <0 RK_PA2 RK_FUNC_GPIO &pcfg_output_high>;
@@ -180,6 +198,28 @@ i2s_8ch_mclk_pin: i2s-8ch-mclk-pin {
};
};
+&sdio_pwrseq {
+ /delete-property/ clocks;
+ /delete-property/ clock-names;
+ post-power-on-delay-ms = <100>;
+ power-off-delay-us = <500000>;
+};
+
+&sdio0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ bus-width = <4>;
+ cap-sd-highspeed;
+ cap-sdio-irq;
+ keep-power-in-suspend;
+ mmc-pwrseq = <&sdio_pwrseq>;
+ non-removable;
+ pinctrl-names = "default";
+ pinctrl-0 = <&sdio0_bus4 &sdio0_cmd &sdio0_clk>;
+ sd-uhs-sdr104;
+ status = "okay";
+};
+
&u2phy0 {
status = "okay";
@@ -212,6 +252,17 @@ &uart0 {
pinctrl-names = "default";
pinctrl-0 = <&uart0_xfer &uart0_cts &uart0_rts>;
status = "okay";
+
+ bluetooth {
+ compatible = "brcm,bcm43438-bt";
+ clocks = <&rk808 1>;
+ clock-names = "lpo";
+ device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
+ host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
+ shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&bt_host_wake_l &bt_wake_l &bt_enable_h>;
+ };
};
&usbdrd_dwc3_0 {
--
2.43.0
^ permalink raw reply related
* [PATCH 3/3] arm64: dts: rockchip: Keep rk3399-roc-pc-plus work LED on
From: Fabio Estevam @ 2026-07-17 1:07 UTC (permalink / raw)
To: heiko
Cc: robh, krzk+dt, conor+dt, devicetree, linux-arm-kernel,
linux-rockchip, Fabio Estevam
In-Reply-To: <20260717010736.578419-1-festevam@gmail.com>
From: Fabio Estevam <festevam@nabladev.com>
The rk3399-roc-pc-plus inherits the work LED heartbeat trigger from
the common rk3399-roc-pc.dtsi.
On the rk3399-roc-pc-plus this LED is the prominent blue front-panel
status LED. Blinking it continuously is distracting for a PC-style
board. The usual default is a steady power/status indication while
the system is running.
Use the default-on trigger for this board instead. This keeps the LED
useful as a simple running indicator and still lets userspace select a
different trigger or turn it off after boot.
Signed-off-by: Fabio Estevam <festevam@nabladev.com>
---
arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
index f87bc7b6afc1..7e8039b19be7 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
@@ -93,6 +93,10 @@ leds {
};
};
+&work_led {
+ linux,default-trigger = "default-on";
+};
+
&fusb0 {
vbus-supply = <&vcc_vbus_typec1>;
};
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 3/3] arm64: dts: rockchip: Keep rk3399-roc-pc-plus work LED on
From: sashiko-bot @ 2026-07-17 1:12 UTC (permalink / raw)
To: Fabio Estevam; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260717010736.578419-3-festevam@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message describes the work LED as a "prominent blue front-panel status LED", but the patch fails to override the inherited "green:work" label, resulting in a code and commit message mismatch.
--
--- Patch [3]: [PATCH 3/3] arm64: dts: rockchip: Keep rk3399-roc-pc-plus work LED on ---
commit 7bd9a09829c15f03ac88e155bb13da9235dada8c
Author: Fabio Estevam <festevam@nabladev.com>
arm64: dts: rockchip: Keep rk3399-roc-pc-plus work LED on
This commit overrides the default LED trigger for the rk3399-roc-pc-plus
work LED to default-on. It changes the behavior from a distracting continuous
blink to a steady status indication.
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
> index f87bc7b6afc14..7e8039b19be7e 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
> @@ -93,6 +93,10 @@ leds {
> };
> };
>
> +&work_led {
> + linux,default-trigger = "default-on";
> +};
[Severity: Medium]
The commit message mentions this is the prominent blue front-panel status LED.
Since the board inherits from rk3399-roc-pc.dtsi where the work LED is
typically configured with the green:work label, does this node also need a
color or label override?
If the inherited label is not overridden here, the sysfs interface might
misrepresent the physical blue LED as green, causing a mismatch between the
commit message's description and the resulting device tree.
> +
> &fusb0 {
> vbus-supply = <&vcc_vbus_typec1>;
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717010736.578419-1-festevam@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH v3 1/3] dt-bindings: clock: qcom: glymur-gcc: Add missing CX power domain
From: Bjorn Andersson @ 2026-07-17 1:15 UTC (permalink / raw)
To: Taniya Das
Cc: Abel Vesa, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio, Maulik Shah,
Dmitry Baryshkov, Jyothi Kumar Seerapu, Konrad Dybcio,
Brian Masney, Krzysztof Kozlowski, Sibi Sankar, Pankaj Patil,
Akhil P Oommen, linux-arm-msm, linux-clk, devicetree,
linux-kernel, Jishnu Prakash, Raviteja Laggyshetty, Kamal Wadhwa,
Qiang Yu, Manaf Meethalavalappu Pallikunhi, Abel Vesa
In-Reply-To: <316fd37b-9936-4bca-8521-9a11ee65cbd6@oss.qualcomm.com>
On Thu, Jul 16, 2026 at 04:42:11PM +0530, Taniya Das wrote:
>
>
> On 7/16/2026 12:52 PM, Abel Vesa wrote:
> > On 26-07-16 10:13:48, Taniya Das wrote:
> >>
> >>
> >> On 7/15/2026 6:59 PM, Abel Vesa wrote:
> >>> The GDSCs provided by the Glymur GCC are supplied by the RPMh CX power
> >>> domain. Model that parent domain in the GCC binding so the provider can
> >>> describe the dependency in devicetree.
> >>>
> >>> Add a single CX power-domain entry to the binding and make it required,
> >>> matching the hardware description needed by the GCC node.
> >>>
> >>> Fixes: ee2d967030fe ("dt-bindings: clock: qcom: document the Glymur Global Clock Controller")
> >>> Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> >>> ---
> >>> Documentation/devicetree/bindings/clock/qcom,glymur-gcc.yaml | 8 ++++++++
> >>> 1 file changed, 8 insertions(+)
> >>
> >>
> >> Abel, I feel we should add the 'required-opps' as well which will ensure
> >> the clock controllers minimum voltage requirement.
> >
> > But it was agreed in v1 that we should not add the required-opps. So we
> > dropped it in v2.
> >
>
> Okay, sorry, may be I missed that comment.
>
>
> > Can you give more details to why you think it is needed ?
>
> My reasoning is that if a clock controller requires a rail, it is
> preferable to explicitly specify the required operating level rather
> than rely on system/client vote. This makes the dependency
> self-contained and easier to reason about. Clients that require a higher
> operating level than the clock controller's minimum requirement remain
> free to vote for a higher level as needed.
>
You're expressing this in generic terms, can you please provide the
concrete example of what this would look for in the Glymur GCC case?
What is the required-opps for the Glymur GCC?
Regards,
Bjorn
>
> --
> Thanks,
> Taniya Das
>
^ permalink raw reply
* Re: [PATCH 2/3] arm64: dts: rockchip: Enable wireless on rk3399-roc-pc-plus
From: sashiko-bot @ 2026-07-17 1:16 UTC (permalink / raw)
To: Fabio Estevam; +Cc: devicetree, conor+dt, robh
In-Reply-To: <20260717010736.578419-2-festevam@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the shared 32kHz LPO clock from the SDIO power sequencer breaks Wi-Fi initialization when Bluetooth is disabled or probes later.
--
--- Patch [2]: [PATCH 2/3] arm64: dts: rockchip: Enable wireless on rk3399-roc-pc-plus ---
commit d6496a6030e5f440c56fb715094eff4f9078ed81
Author: Fabio Estevam <festevam@nabladev.com>
arm64: dts: rockchip: Enable wireless on rk3399-roc-pc-plus
This commit enables the Ampak AP6256 Wi-Fi/Bluetooth module on the
rk3399-roc-pc-plus board. It describes the SDIO and UART interfaces, adds
power sequencing delays, and removes the clock from the SDIO power sequencer
to allow the Bluetooth node to request it directly.
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
> index f6d62ee51f1b9..f87bc7b6afc14 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-plus.dts
[ ... ]
> @@ -180,6 +198,28 @@ i2s_8ch_mclk_pin: i2s-8ch-mclk-pin {
> };
> };
>
> +&sdio_pwrseq {
> + /delete-property/ clocks;
> + /delete-property/ clock-names;
[Severity: High]
Will removing the shared 32kHz LPO clock from the SDIO power sequencer break
Wi-Fi initialization when Bluetooth is disabled or probes later?
If the MMC/SDIO subsystem initiates its power-on sequence before the Bluetooth
driver probes, or if Bluetooth is disabled, the LPO clock will not be enabled.
Without this clock, the Wi-Fi module cannot start its state machine and will
fail to respond to the initial CMD5 enumeration from the MMC core.
Additionally, if Bluetooth enters runtime suspend and gates the clock, could
this crash the active Wi-Fi firmware?
> + post-power-on-delay-ms = <100>;
> + power-off-delay-us = <500000>;
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717010736.578419-1-festevam@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy
From: Peter Chen @ 2026-07-17 1:20 UTC (permalink / raw)
To: Thinh Nguyen
Cc: Krishna Kurapati, Krzysztof Kozlowski, Dmitry Baryshkov,
Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Wesley Cheng, linux-arm-msm@vger.kernel.org,
linux-usb@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <allvBYQN_0QkI835@vbox>
On 26-07-17 00:06:37, Thinh Nguyen wrote:
> >
> > I have seen fewer platforms use "phy_type" at dts for arm64, dwc3
> > core uses it only for special cases and the code was added for
> > ten years ago. For the default situation, we may not need to
> > change DWC3_GUSB2PHYCFG_EUSB2OPMODE value for UTMI+, Thinh, is it
> > correct?
> >
>
> The GUSB2PHYCFG.eUSB2OPMODE is only relevant for host mode and mainly
> for electrical compliance. Usually by default, the CoreConsultant
> setting should have this set correctly. So not explicitly setting it
> should be functionally fine (IIRC).
>
> That said, this is separate from the GUSB2PHYCFG.PHYIF, which the core
> uses dwc->hsphy_mode to indicate whether the UTMI interface is 8-bit or
> 16-bit.
>
Why only rockchip uses this "phy_type" property, why other SoC vendors
no this requirement for UTMI width setting?
--
Thanks,
Peter Chen
^ permalink raw reply
* Re: [PATCH v5 02/14] media: mediatek: vcodec: add driver to support vcp
From: Kyrie Wu (吴晗) @ 2026-07-17 1:53 UTC (permalink / raw)
To: linux-mediatek@lists.infradead.org,
Yunfei Dong (董云飞), ribalda@chromium.org,
robh@kernel.org, fanwu01@zju.edu.cn, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, haoxiang_li2024@163.com,
nicolas.dufresne@collabora.com,
Andrew-CT Chen (陳智迪),
AngeloGioacchino Del Regno, mchehab@kernel.org,
conor+dt@kernel.org, Tiffany Lin (林慧珊),
sebastian.fricke@collabora.com, benjamin.gaignard@collabora.com,
rongqianfeng@vivo.com, linux-kernel@vger.kernel.org,
fshao@chromium.org, kees@kernel.org, Kyrie Wu (吴晗),
krzk+dt@kernel.org, laurent.pinchart+renesas@ideasonboard.com,
wenst@chromium.org, Irui Wang (王瑞),
linux-arm-kernel@lists.infradead.org, matthias.bgg@gmail.com,
hverkuil+cisco@kernel.org, jacopo.mondi@ideasonboard.com,
p.zabel@pengutronix.de
Cc: tfiga@chromium.org, tzungbi@kernel.org,
sakari.ailus@linux.intel.com
In-Reply-To: <2dad1d32f59b271e9cf53f0da71b584e2e04a889.camel@collabora.com>
On Wed, 2026-07-15 at 20:50 -0400, Nicolas Dufresne wrote:
> Le mercredi 03 juin 2026 à 16:40 +0800, Kyrie Wu a écrit :
> > From: Yunfei Dong <yunfei.dong@mediatek.com>
> >
> > The processor is changed from scp to vcp in mt8196 platform.
> > Adding new firmware interface to communicate kernel with vcp
> > for the communication method is changed.
> >
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > ---
> > .../media/platform/mediatek/vcodec/Kconfig | 4 +
> > .../platform/mediatek/vcodec/common/Makefile | 4 +
> > .../mediatek/vcodec/common/mtk_vcodec_fw.c | 26 +-
> > .../vcodec/common/mtk_vcodec_fw_priv.h | 12 +
> > .../vcodec/common/mtk_vcodec_fw_scp.c | 1 +
> > .../vcodec/common/mtk_vcodec_fw_vcp.c | 505
> > ++++++++++++++++++
> > .../vcodec/common/mtk_vcodec_fw_vcp.h | 142 +++++
> > .../vcodec/common/mtk_vcodec_fw_vpu.c | 1 +
> > .../vcodec/decoder/mtk_vcodec_dec_drv.c | 3 +
> > .../vcodec/decoder/mtk_vcodec_dec_drv.h | 2 +
> > .../vcodec/encoder/mtk_vcodec_enc_drv.c | 2 +
> > .../vcodec/encoder/mtk_vcodec_enc_drv.h | 2 +
> > 12 files changed, 691 insertions(+), 13 deletions(-)
> > create mode 100644
> > drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> > create mode 100644
> > drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
> >
> > diff --git a/drivers/media/platform/mediatek/vcodec/Kconfig
> > b/drivers/media/platform/mediatek/vcodec/Kconfig
> > index bc8292232530..d23dad5c78ce 100644
> > --- a/drivers/media/platform/mediatek/vcodec/Kconfig
> > +++ b/drivers/media/platform/mediatek/vcodec/Kconfig
> > @@ -1,4 +1,7 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > +config VIDEO_MEDIATEK_VCODEC_VCP
> > + bool
> > +
> > config VIDEO_MEDIATEK_VCODEC_SCP
> > bool
> >
> > @@ -21,6 +24,7 @@ config VIDEO_MEDIATEK_VCODEC
> > select V4L2_MEM2MEM_DEV
> > select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU
> > select VIDEO_MEDIATEK_VCODEC_SCP if MTK_SCP
> > + select VIDEO_MEDIATEK_VCODEC_VCP if MTK_VCP_RPROC
> > select V4L2_H264
> > select V4L2_VP9
> > select MEDIA_CONTROLLER
> > diff --git a/drivers/media/platform/mediatek/vcodec/common/Makefile
> > b/drivers/media/platform/mediatek/vcodec/common/Makefile
> > index d0479914dfb3..2f68692e8c98 100644
> > --- a/drivers/media/platform/mediatek/vcodec/common/Makefile
> > +++ b/drivers/media/platform/mediatek/vcodec/common/Makefile
> > @@ -14,6 +14,10 @@ ifneq ($(CONFIG_VIDEO_MEDIATEK_VCODEC_SCP),)
> > mtk-vcodec-common-y += mtk_vcodec_fw_scp.o
> > endif
> >
> > +ifneq ($(CONFIG_VIDEO_MEDIATEK_VCODEC_VCP),)
> > +mtk-vcodec-common-y += mtk_vcodec_fw_vcp.o
> > +endif
> > +
> > ifneq ($(CONFIG_DEBUG_FS),)
> > obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += mtk-vcodec-dbgfs.o
> >
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> > index 08949b08fbc6..64977bcd6abd 100644
> > --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> > +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> > @@ -3,26 +3,26 @@
> > #include "../decoder/mtk_vcodec_dec_drv.h"
> > #include "../encoder/mtk_vcodec_enc_drv.h"
> > #include "mtk_vcodec_fw_priv.h"
> > +#include "mtk_vcodec_fw_vcp.h"
> >
> > struct mtk_vcodec_fw *mtk_vcodec_fw_select(void *priv, enum
> > mtk_vcodec_fw_type type,
> > enum mtk_vcodec_fw_use
> > fw_use)
> > {
> > - struct platform_device *plat_dev;
> > + if (fw_use == ENCODER) {
> > + struct mtk_vcodec_enc_dev *enc_dev = priv;
> >
> > - if (fw_use == ENCODER)
> > - plat_dev = ((struct mtk_vcodec_enc_dev *)priv)-
> > >plat_dev;
> > - else
> > - plat_dev = ((struct mtk_vcodec_dec_dev *)priv)-
> > >plat_dev;
> > + return enc_dev->fw_init(priv, fw_use);
> > + }
> > +
> > + if (fw_use == DECODER) {
>
> Should we construct this a little safer and use some if/else if/else
> with some
> bug_on in the last entry, or same with a switch ?
Dear Nicolas,
Thanks for the suggestion.
I agree that the current helper should not infer the device type by
casting priv based on fw_use. In the next version I will rework this so
that the encoder/decoder probe code passes the selected firmware init
callback into mtk_vcodec_fw_select() directly.
With that change, mtk_vcodec_fw_select() only validates fw_use and the
callback pointer, then calls the selected callback:
dev->fw_handler = mtk_vcodec_fw_select(dev, DECODER, ev->fw_init);
function definition:
struct mtk_vcodec_fw *mtk_vcodec_fw_select(void *priv, enum
mtk_vcodec_fw_use fw_use, mtk_vcodec_fw_init_func fw_init);
This removes the encoder/decoder casts from mtk_vcodec_fw_select() and
keeps the invalid fw_use path explicit.
I will fold this into v6.
Thanks,
Kyrie
>
> > + struct mtk_vcodec_dec_dev *dec_dev = priv;
> >
> > - switch (type) {
> > - case VPU:
> > - return mtk_vcodec_fw_vpu_init(priv, fw_use);
> > - case SCP:
> > - return mtk_vcodec_fw_scp_init(priv, fw_use);
> > - default:
> > - dev_err(&plat_dev->dev, "Invalid vcodec fw type");
> > - return ERR_PTR(-EINVAL);
> > + return dec_dev->fw_init(priv, fw_use);
> > }
> > +
> > + pr_err("Error in selecting firmware, fw_use is not set");
> > +
> > + return ERR_PTR(-EINVAL);
> > }
> > EXPORT_SYMBOL_GPL(mtk_vcodec_fw_select);
> >
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.
> > h
> > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.
> > h
> > index 99603accd82e..0a2a9b010244 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.
> > h
> > +++
> > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.
> > h
> > @@ -4,6 +4,7 @@
> > #define _MTK_VCODEC_FW_PRIV_H_
> >
> > #include "mtk_vcodec_fw.h"
> > +#include "mtk_vcodec_fw_vcp.h"
> >
> > struct mtk_vcodec_dec_dev;
> > struct mtk_vcodec_enc_dev;
> > @@ -13,6 +14,7 @@ struct mtk_vcodec_fw {
> > const struct mtk_vcodec_fw_ops *ops;
> > struct platform_device *pdev;
> > struct mtk_scp *scp;
> > + struct mtk_vcp *vcp;
> > enum mtk_vcodec_fw_use fw_use;
> > };
> >
> > @@ -49,4 +51,14 @@ mtk_vcodec_fw_scp_init(void *priv, enum
> > mtk_vcodec_fw_use fw_use)
> > }
> > #endif /* CONFIG_VIDEO_MEDIATEK_VCODEC_SCP */
> >
> > +#if IS_ENABLED(CONFIG_VIDEO_MEDIATEK_VCODEC_VCP)
> > +struct mtk_vcodec_fw *mtk_vcodec_fw_vcp_init(void *priv, enum
> > mtk_vcodec_fw_use fw_use);
> > +#else
> > +static inline struct mtk_vcodec_fw *
> > +mtk_vcodec_fw_vcp_init(void *priv, enum mtk_vcodec_fw_use fw_use)
> > +{
> > + return ERR_PTR(-ENODEV);
> > +}
> > +#endif /* CONFIG_VIDEO_MEDIATEK_VCODEC_VCP */
> > +
> > #endif /* _MTK_VCODEC_FW_PRIV_H_ */
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_scp.c
> > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_scp.c
> > index 1b0bc47355c0..1aad5c3ef990 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_scp.c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_scp.c
> > @@ -90,3 +90,4 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_scp_init(void
> > *priv, enum mtk_vcodec_fw_use
> >
> > return fw;
> > }
> > +EXPORT_SYMBOL_GPL(mtk_vcodec_fw_scp_init);
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> > new file mode 100644
> > index 000000000000..2f3bfcd97958
> > --- /dev/null
> > +++
> > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> > @@ -0,0 +1,505 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2025 MediaTek Inc.
> > + * Author: Yunfei Dong <yunfei.dong@mediatek.com>
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/dma-direction.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/iommu.h>
> > +#include <linux/remoteproc/mtk_vcp_public.h>
> > +#include <linux/firmware/mediatek/mtk-vcp-ipc.h>
> > +
> > +#include "../decoder/mtk_vcodec_dec_drv.h"
> > +#include "../decoder/vdec_ipi_msg.h"
> > +#include "mtk_vcodec_fw_priv.h"
> > +
> > +#define IPI_SEND_TIMEOUT_MS 100U
> > +#define IPI_TIMEOUT_MS 100U
> > +
> > +#define VCP_IPI_HEADER_SIZE (sizeof(u32) * 2)
> > +#define VCP_IPI_ALIGN (4)
> > +
> > +static struct mutex *mtk_vcodec_vcp_get_ipi_lock(struct mtk_vcp
> > *vcp, u32 ipi_id)
> > +{
> > + return &vcp->ipi_desc[ipi_id].lock;
>
> Maybe add sound bound checks ?
>
> > +}
> > +
> > +static void mtk_vcodec_vcp_ipi_lock(struct mtk_vcp *vcp, u32
> > ipi_id)
> > +{
> > + struct mutex *lock = mtk_vcodec_vcp_get_ipi_lock(vcp,
> > ipi_id);
> > +
> > + if (!lock)
> > + return;
>
> mtk_vcodec_vcp_get_ipi_lock() nevers return null at the minute.
>
> > +
> > + mutex_lock(lock);
> > +}
> > +
> > +static void mtk_vcodec_vcp_ipi_unlock(struct mtk_vcp *vcp, u32
> > ipi_id)
> > +{
> > + struct mutex *lock = mtk_vcodec_vcp_get_ipi_lock(vcp,
> > ipi_id);
> > +
> > + if (!lock)
> > + return;
> > +
> > + lockdep_assert_held(lock);
> > + mutex_unlock(lock);
> > +}
> > +
> > +static spinlock_t *mtk_vcodec_vcp_get_msg_queue_lock(struct
> > mtk_vcodec_fw *fw)
> > +{
> > + return &fw->vcp->msg_queue.lock;
> > +}
> > +
> > +static void mtk_vcodec_vcp_msq_queue_lock(struct mtk_vcodec_fw
> > *fw, unsigned long *flags)
> > +{
> > + spinlock_t *lock = mtk_vcodec_vcp_get_msg_queue_lock(fw);
> > +
> > + if (!lock)
> > + return;
>
> mtk_vcodec_vcp_get_msg_queue_lock() never returns null.
>
> > +
> > + spin_lock_irqsave(lock, *flags);
> > +}
> > +
> > +static void mtk_vcodec_vcp_msq_queue_unlock(struct mtk_vcodec_fw
> > *fw, unsigned long *flags)
> > +{
> > + spinlock_t *lock = mtk_vcodec_vcp_get_msg_queue_lock(fw);
> > +
> > + if (!lock)
> > + return;
> > +
> > + spin_unlock_irqrestore(lock, *flags);
> > +}
> > +
> > +static int mtk_vcodec_vcp_notifier(struct notifier_block *nb,
> > unsigned long event, void *ptr)
> > +{
> > + struct mtk_vcp *vcp = container_of(nb, struct mtk_vcp,
> > vcp_notify);
> > +
> > + switch (event) {
> > + case VCP_EVENT_SUSPEND:
> > + case VCP_EVENT_STOP:
> > + dev_dbg(&vcp->pdev->dev, "vcp notifier suspend");
> > + break;
> > + case VCP_EVENT_READY:
> > + case VCP_EVENT_RESUME:
> > + dev_dbg(&vcp->pdev->dev, "vcp notifier ready");
> > + break;
> > + }
> > +
> > + return NOTIFY_DONE;
> > +}
> > +
> > +static void mtk_vcodec_vcp_free_msg_node(struct mtk_vcodec_fw *fw,
> > + struct mtk_vcp_msg_node
> > *msg_node)
> > +{
> > + unsigned long flags;
> > +
> > + mtk_vcodec_vcp_msq_queue_lock(fw, &flags);
> > + list_add(&msg_node->list, &fw->vcp->msg_queue.node_list);
> > + mtk_vcodec_vcp_msq_queue_unlock(fw, &flags);
> > +}
> > +
> > +static int mtk_vcodec_vcp_ipi_register(struct mtk_vcp *vcp, u32
> > ipi_id, vcp_ipi_handler_t handler,
> > + void *priv)
> > +{
> > + if (!vcp)
> > + return -EPROBE_DEFER;
> > +
> > + if (WARN_ON(ipi_id >= VCP_IPI_MAX) || WARN_ON(!handler))
> > + return -EINVAL;
>
> Ok, fair enough as a bound check I guess, thought the get/null-check
> construct
> still make no sense.
>
> Stopping there, I'm on a time crunch, but considering the
> dependencies aren't
> met, feel free to ping me again for more reviews.
>
> Nicolas
Thanks. I will fix it.
>
> > +
> > + mtk_vcodec_vcp_ipi_lock(vcp, ipi_id);
> > + vcp->ipi_desc[ipi_id].handler = handler;
> > + vcp->ipi_desc[ipi_id].priv = priv;
> > + mtk_vcodec_vcp_ipi_unlock(vcp, ipi_id);
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_vcodec_vcp_msg_process_thread(void *arg)
> > +{
> > + struct mtk_vcodec_fw *fw = arg;
> > + struct vdec_vpu_ipi_ack *msg = NULL;
> > + struct mtk_vcp_share_obj *obj;
> > + struct mtk_vcp_msg_node *msg_node;
> > + vcp_ipi_handler_t handler;
> > + unsigned long flags;
> > + int ret = 0;
> > +
> > + do {
> > + ret = wait_event_interruptible(fw->vcp-
> > >msg_queue.wq,
> > + atomic_read(&fw-
> > >vcp->msg_queue.cnt) > 0);
> > + if (ret < 0) {
> > + dev_err(&fw->pdev->dev, "wait msg queue
> > ack timeout %d %d\n",
> > + ret, atomic_read(&fw->vcp-
> > >msg_queue.cnt));
> > + continue;
> > + }
> > +
> > + mtk_vcodec_vcp_msq_queue_lock(fw, &flags);
> > + msg_node = list_entry(fw->vcp-
> > >msg_queue.msg_list.next,
> > + struct mtk_vcp_msg_node,
> > list);
> > + list_del(&msg_node->list);
> > + atomic_dec(&fw->vcp->msg_queue.cnt);
> > + mtk_vcodec_vcp_msq_queue_unlock(fw, &flags);
> > +
> > + obj = &msg_node->ipi_data;
> > + msg = (struct vdec_vpu_ipi_ack *)obj->share_buf;
> > +
> > + if (!msg->ap_inst_addr) {
> > + dev_err(&fw->pdev->dev, "invalid message
> > address\n");
> > + mtk_vcodec_vcp_free_msg_node(fw,
> > msg_node);
> > + continue;
> > + }
> > +
> > + dev_dbg(&fw->pdev->dev, "msg ack id %d len %d
> > msg_id 0x%x\n", obj->id, obj->len,
> > + msg->msg_id);
> > +
> > + mtk_vcodec_vcp_ipi_lock(fw->vcp, obj->id);
> > + handler = fw->vcp->ipi_desc[obj->id].handler;
> > + if (!handler) {
> > + dev_err(&fw->pdev->dev, "invalid ack ipi
> > handler id = %d\n", obj->id);
> > + mtk_vcodec_vcp_ipi_unlock(fw->vcp, obj-
> > >id);
> > + mtk_vcodec_vcp_free_msg_node(fw,
> > msg_node);
> > + return -EINVAL;
> > + }
> > +
> > + handler(msg, obj->len, fw->vcp->ipi_desc[obj-
> > >id].priv);
> > + mtk_vcodec_vcp_ipi_unlock(fw->vcp, obj->id);
> > +
> > + fw->vcp->msg_signaled[obj->id] = true;
> > + wake_up(&fw->vcp->msg_wq[obj->id]);
> > +
> > + mtk_vcodec_vcp_free_msg_node(fw, msg_node);
> > + } while (!kthread_should_stop());
> > +
> > + return ret;
> > +}
> > +
> > +static int mtk_vcodec_vcp_msg_ack_isr(unsigned int id, void
> > *prdata, void *data, unsigned int len)
> > +{
> > + struct mtk_vcodec_fw *fw = prdata;
> > + struct mtk_vcp_msg_queue *msg_queue = &fw->vcp->msg_queue;
> > + struct mtk_vcp_msg_node *msg_node;
> > + struct vdec_vpu_ipi_ack *msg = NULL;
> > + struct mtk_vcp_share_obj *obj = data;
> > + unsigned long flags;
> > +
> > + msg = (struct vdec_vpu_ipi_ack *)obj->share_buf;
> > +
> > + mtk_vcodec_vcp_msq_queue_lock(fw, &flags);
> > + if (!list_empty(&msg_queue->node_list)) {
> > + msg_node = list_entry(msg_queue->node_list.next,
> > struct mtk_vcp_msg_node, list);
> > +
> > + memcpy(&msg_node->ipi_data, obj, sizeof(*obj));
> > + list_move_tail(&msg_node->list, &msg_queue-
> > >msg_list);
> > + atomic_inc(&msg_queue->cnt);
> > + mtk_vcodec_vcp_msq_queue_unlock(fw, &flags);
> > +
> > + dev_dbg(&fw->pdev->dev, "push ipi_id %x msg_id %x,
> > msg cnt %d\n",
> > + obj->id, msg->msg_id,
> > atomic_read(&msg_queue->cnt));
> > +
> > + wake_up(&msg_queue->wq);
> > + } else {
> > + mtk_vcodec_vcp_msq_queue_unlock(fw, &flags);
> > + dev_err(&fw->pdev->dev, "no free nodes in msg
> > queue\n");
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_vcodec_vcp_msg_ipi_send(struct mtk_vcodec_fw *fw,
> > int id, void *buf,
> > + unsigned int len, unsigned
> > int wait)
> > +{
> > + struct mtk_vcp *vcp = fw->vcp;
> > + struct mtk_vcp_device *vcp_device = vcp->vcp_device;
> > + bool *msg_signaled = &vcp->msg_signaled[id];
> > + wait_queue_head_t *msg_wq = &vcp->msg_wq[id];
> > + int ret, ipi_size, feature_id, mailbox_id, retry_cnt = 0;
> > + unsigned long timeout_jiffies = 0;
> > + struct mtk_vcp_share_obj obj = {0};
> > + unsigned int *data;
> > +
> > + if (!vcp_device) {
> > + dev_dbg(&fw->pdev->dev, "vcp device is null\n");
> > + return -EINVAL;
> > + }
> > +
> > + mutex_lock(&vcp->ipi_mutex);
> > + feature_id = VDEC_FEATURE_ID;
> > + mailbox_id = IPI_OUT_VDEC_1;
> > +
> > + timeout_jiffies = jiffies +
> > msecs_to_jiffies(VCP_SYNC_TIMEOUT_MS);
> > + while (!vcp_device->ops->vcp_is_ready(feature_id)) {
> > + if (time_after(jiffies, timeout_jiffies)) {
> > + vcp->ipi_id_ack[id] = -EINVAL;
> > + ret = -EINVAL;
> > + goto error;
> > + }
> > + mdelay(1);
> > + }
> > +
> > + if (len > VCP_SHARE_BUF_SIZE) {
> > + vcp->ipi_id_ack[id] = -EINVAL;
> > + ret = -EINVAL;
> > + goto error;
> > + }
> > +
> > + obj.id = id;
> > + obj.len = len;
> > + memcpy(obj.share_buf, buf, len);
> > +
> > + ipi_size = round_up(VCP_IPI_HEADER_SIZE + len,
> > VCP_IPI_ALIGN);
> > + data = (unsigned int *)obj.share_buf;
> > + dev_dbg(&fw->pdev->dev, "vcp send message: id %d len %d
> > data 0x%x\n",
> > + obj.id, obj.len, data[0]);
> > +
> > + ret = mtk_vcp_ipc_send(vcp_get_ipidev(vcp_device),
> > mailbox_id, &obj, ipi_size);
> > + if (ret != IPI_ACTION_DONE) {
> > + vcp->ipi_id_ack[id] = -EIO;
> > + ret = -EIO;
> > + goto error;
> > + }
> > +
> > +wait_ack:
> > + /* wait for VCP's ACK */
> > + ret = wait_event_timeout(*msg_wq, *msg_signaled,
> > msecs_to_jiffies(IPI_TIMEOUT_MS));
> > + if (!ret || retry_cnt > 5) {
> > + vcp->ipi_id_ack[id] = VCODEC_IPI_MSG_STATUS_FAIL;
> > + dev_err(&fw->pdev->dev, "wait ipi ack timeout! %d
> > %d\n", ret, vcp->ipi_id_ack[id]);
> > + } else if (ret == -ERESTARTSYS) {
> > + dev_err(&fw->pdev->dev, "wait ipi ack err (%d)\n",
> > vcp->ipi_id_ack[id]);
> > + retry_cnt++;
> > + goto wait_ack;
> > + } else if (ret < 0) {
> > + dev_err(&fw->pdev->dev, "wait ipi ack fail ret %d
> > %d\n", ret, vcp->ipi_id_ack[id]);
> > + vcp->ipi_id_ack[id] = VCODEC_IPI_MSG_STATUS_FAIL;
> > + }
> > +
> > + dev_dbg(&fw->pdev->dev, "receive message: id %d len %d
> > data 0x%x\n",
> > + obj.id, obj.len, data[0]);
> > +
> > + *msg_signaled = false;
> > + mutex_unlock(&vcp->ipi_mutex);
> > +
> > + return vcp->ipi_id_ack[id];
> > +
> > +error:
> > + mutex_unlock(&vcp->ipi_mutex);
> > + dev_err(&fw->pdev->dev, "send msg error type:%d msg:%d >
> > %d ret:%d\n", fw->type, len,
> > + VCP_SHARE_BUF_SIZE, ret);
> > +
> > + return ret;
> > +}
> > +
> > +static int check_vcp_loaded(struct mtk_vcodec_fw *fw)
> > +{
> > + struct device *dev = &fw->pdev->dev;
> > + struct device_driver *drv;
> > +
> > + drv = driver_find("mtk-vcp", &platform_bus_type);
> > + if (!drv) {
> > + dev_err(dev, "find mtk-vcp driver failed, need to
> > reload.");
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_vcodec_vcp_get_vcp_device(struct mtk_vcodec_fw *fw)
> > +{
> > + struct device *dev = &fw->pdev->dev;
> > + int retry = 0, retry_cnt = 10000;
> > + phandle vcp_phandle;
> > +
> > + while (try_then_request_module(check_vcp_loaded(fw), "mtk-
> > vcp")) {
> > + if (++retry > retry_cnt) {
> > + dev_err(dev, "failed to load mtk-vcp
> > module");
> > + return -EPROBE_DEFER;
> > + }
> > + usleep_range(1000, 2000);
> > + }
> > +
> > + if (of_property_read_u32(dev->of_node, "mediatek,vcp",
> > &vcp_phandle)) {
> > + dev_err(dev, "can't get vcp handle.\n");
> > + return -ENODEV;
> > + }
> > +
> > + fw->vcp->vcp_device = mtk_vcp_get_by_phandle(vcp_phandle);
> > + if (!fw->vcp->vcp_device) {
> > + dev_err(dev, "get vcp device failed\n");
> > + return -ENODEV;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_vcodec_vcp_load_firmware(struct mtk_vcodec_fw *fw)
> > +{
> > + struct mtk_vcp_device *vcp_device;
> > + int ret, feature_id, mem_id, mailbox_id, ipi_id;
> > +
> > + if (fw->vcp->is_init_done) {
> > + dev_dbg(&fw->pdev->dev, "vcp has already been
> > initialized done.\n");
> > + return 0;
> > + }
> > +
> > + if (mtk_vcodec_vcp_get_vcp_device(fw) < 0) {
> > + dev_err(&fw->pdev->dev, "vcp device is null.\n");
> > + return -EINVAL;
> > + }
> > +
> > + vcp_device = fw->vcp->vcp_device;
> > +
> > + feature_id = VDEC_FEATURE_ID;
> > + mem_id = VDEC_MEM_ID;
> > + mailbox_id = IPI_IN_VDEC_1;
> > + ipi_id = VCP_IPI_LAT_DECODER;
> > +
> > + ret =
> > mtk_vcp_mbox_ipc_register(vcp_get_ipidev(vcp_device), mailbox_id,
> > + mtk_vcodec_vcp_msg_ack_isr
> > , fw, &fw->vcp->share_data);
> > + if (ret) {
> > + dev_dbg(&fw->pdev->dev, "ipi register fail %d %d
> > %d %d\n", ret, feature_id,
> > + mem_id, mailbox_id);
> > + return -EINVAL;
> > + }
> > +
> > + fw->vcp->vcp_notify.notifier_call =
> > mtk_vcodec_vcp_notifier;
> > + fw->vcp->vcp_notify.priority = 1;
> > + vcp_device->ops->vcp_register_notify(feature_id, &fw->vcp-
> > >vcp_notify);
> > +
> > + if (!fw->vcp->is_register_done) {
> > + ret = vcp_device->ops-
> > >vcp_register_feature(vcp_device, feature_id);
> > + if (ret < 0) {
> > + dev_err(&fw->pdev->dev, "%d register to
> > vcp fail(%d)\n", feature_id, ret);
> > + return -EINVAL;
> > + }
> > +
> > + fw->vcp->is_register_done = true;
> > + }
> > +
> > + fw->vcp->is_init_done = true;
> > +
> > + mutex_init(&fw->vcp->ipi_desc[ipi_id].lock);
> > + mutex_init(&fw->vcp->ipi_mutex);
> > +
> > + kthread_run(mtk_vcodec_vcp_msg_process_thread, fw,
> > "vcp_vdec_msq_thread");
> > +
> > + fw->vcp->vsi_addr = vcp_device->ops-
> > >vcp_get_mem_virt(mem_id);
> > + fw->vcp->vsi_core_addr = fw->vcp->vsi_addr +
> > VCODEC_VSI_LEN;
> > + fw->vcp->vsi_size = vcp_device->ops-
> > >vcp_get_mem_size(mem_id);
> > + fw->vcp->iova_addr = vcp_device->ops-
> > >vcp_get_mem_iova(mem_id);
> > +
> > + init_waitqueue_head(&fw->vcp-
> > >msg_wq[VCP_IPI_LAT_DECODER]);
> > + init_waitqueue_head(&fw->vcp-
> > >msg_wq[VCP_IPI_CORE_DECODER]);
> > +
> > + dev_dbg(&fw->pdev->dev, "vdec vcp init done => va: %p
> > size:0x%x iova:%p.\n",
> > + fw->vcp->vsi_addr, fw->vcp->vsi_size, &fw->vcp-
> > >iova_addr);
> > +
> > + return 0;
> > +}
> > +
> > +static unsigned int mtk_vcodec_vcp_get_vdec_capa(struct
> > mtk_vcodec_fw *fw)
> > +{
> > + return MTK_VDEC_FORMAT_MM21 | MTK_VDEC_FORMAT_H264_SLICE |
> > MTK_VDEC_FORMAT_VP9_FRAME |
> > + MTK_VDEC_FORMAT_AV1_FRAME |
> > MTK_VDEC_FORMAT_HEVC_FRAME |
> > + MTK_VDEC_IS_SUPPORT_10BIT |
> > MTK_VDEC_IS_SUPPORT_EXT;
> > +}
> > +
> > +static void *mtk_vcodec_vcp_dm_addr(struct mtk_vcodec_fw *fw, u32
> > dtcm_dmem_addr)
> > +{
> > + return NULL;
> > +}
> > +
> > +static int mtk_vcodec_vcp_set_ipi_register(struct mtk_vcodec_fw
> > *fw, int id,
> > + mtk_vcodec_ipi_handler
> > handler,
> > + const char *name, void
> > *priv)
> > +{
> > + return mtk_vcodec_vcp_ipi_register(fw->vcp, id, handler,
> > priv);
> > +}
> > +
> > +static int mtk_vcodec_vcp_ipi_send(struct mtk_vcodec_fw *fw, int
> > id, void *buf,
> > + unsigned int len, unsigned int
> > wait)
> > +{
> > + return mtk_vcodec_vcp_msg_ipi_send(fw, id, buf, len,
> > wait);
> > +}
> > +
> > +static void mtk_vcodec_vcp_release(struct mtk_vcodec_fw *fw)
> > +{
> > + struct mtk_vcp_device *vcp_device = fw->vcp->vcp_device;
> > + struct device *dev = &fw->pdev->dev;
> > + int ret, feature_id;
> > +
> > + if (!fw->vcp->vcp_device) {
> > + dev_err(dev, "vcp device is null\n");
> > + return;
> > + }
> > +
> > + if (!fw->vcp->is_register_done)
> > + return;
> > +
> > + feature_id = VDEC_FEATURE_ID;
> > + ret = vcp_device->ops->vcp_deregister_feature(vcp_device,
> > VDEC_FEATURE_ID);
> > + if (ret < 0) {
> > + dev_err(dev, "deregister feature_id(%d)
> > fail(%d)\n", feature_id, ret);
> > + return;
> > + }
> > +
> > + fw->vcp->is_register_done = false;
> > +}
> > +
> > +static const struct mtk_vcodec_fw_ops mtk_vcodec_vcp_msg = {
> > + .load_firmware = mtk_vcodec_vcp_load_firmware,
> > + .get_vdec_capa = mtk_vcodec_vcp_get_vdec_capa,
> > + .map_dm_addr = mtk_vcodec_vcp_dm_addr,
> > + .ipi_register = mtk_vcodec_vcp_set_ipi_register,
> > + .ipi_send = mtk_vcodec_vcp_ipi_send,
> > + .release = mtk_vcodec_vcp_release,
> > +};
> > +
> > +struct mtk_vcodec_fw *mtk_vcodec_fw_vcp_init(void *priv, enum
> > mtk_vcodec_fw_use fw_use)
> > +{
> > + struct mtk_vcp_msg_node *msg_node;
> > + struct platform_device *plat_dev;
> > + struct mtk_vcodec_fw *fw;
> > + int i;
> > +
> > + if (fw_use == DECODER) {
> > + struct mtk_vcodec_dec_dev *dec_dev = priv;
> > +
> > + plat_dev = dec_dev->plat_dev;
> > + } else {
> > + pr_err("Invalid fw_use %d (use a reasonable fw id
> > here)\n", fw_use);
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + fw = devm_kzalloc(&plat_dev->dev, sizeof(*fw),
> > GFP_KERNEL);
> > + if (!fw)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + fw->type = VCP;
> > + fw->pdev = plat_dev;
> > + fw->fw_use = fw_use;
> > + fw->ops = &mtk_vcodec_vcp_msg;
> > + fw->vcp = devm_kzalloc(&plat_dev->dev, sizeof(*fw->vcp),
> > GFP_KERNEL);
> > + if (!fw->vcp)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + INIT_LIST_HEAD(&fw->vcp->msg_queue.msg_list);
> > + INIT_LIST_HEAD(&fw->vcp->msg_queue.node_list);
> > + spin_lock_init(&fw->vcp->msg_queue.lock);
> > + init_waitqueue_head(&fw->vcp->msg_queue.wq);
> > + atomic_set(&fw->vcp->msg_queue.cnt, 0);
> > + fw->vcp->pdev = plat_dev;
> > +
> > + for (i = 0; i < VCP_MAX_MQ_NODE_CNT; i++) {
> > + msg_node = devm_kzalloc(&plat_dev->dev,
> > sizeof(*msg_node), GFP_KERNEL);
> > + if (!msg_node)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + list_add(&msg_node->list, &fw->vcp-
> > >msg_queue.node_list);
> > + }
> > +
> > + return fw;
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_vcodec_fw_vcp_init);
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
> > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
> > new file mode 100644
> > index 000000000000..1072c37078f4
> > --- /dev/null
> > +++
> > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
> > @@ -0,0 +1,142 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2025 MediaTek Inc.
> > + * Author: Yunfei Dong <yunfei.dong@mediatek.com>
> > + */
> > +
> > +#ifndef _MTK_VCODEC_FW_VCP_H_
> > +#define _MTK_VCODEC_FW_VCP_H_
> > +
> > +typedef void (*vcp_ipi_handler_t) (void *data, unsigned int len,
> > void *priv);
> > +
> > +#define VCP_MAX_MQ_NODE_CNT 6
> > +#define VCP_SHARE_BUF_SIZE 64
> > +
> > +#define VCODEC_VSI_LEN (0x2000)
> > +
> > +/* enum mtk_vcp_ipi_index - index used to separate different
> > hardware */
> > +enum mtk_vcp_ipi_index {
> > + VCP_IPI_LAT_DECODER,
> > + VCP_IPI_CORE_DECODER,
> > + VCP_IPI_MAX,
> > +};
> > +
> > +/**
> > + * struct mtk_vcp_msg_queue - process the vcp message between
> > kernel with vcp
> > + *
> > + * @msg_list: store share buffer list which from vcp to kernel
> > + * @wq: waitqueue that can be used to wait for vcp message
> > + * @lock: protect msg list
> > + * @cnt: the count of share obj in msg list
> > + * @node_list: share obj list
> > + */
> > +struct mtk_vcp_msg_queue {
> > + struct list_head msg_list;
> > + wait_queue_head_t wq;
> > + /* protect msg_list and node_list */
> > + spinlock_t lock;
> > + atomic_t cnt;
> > + struct list_head node_list;
> > +};
> > +
> > +/**
> > + * struct mtk_vcp_ipi_desc - store the ack handler
> > + *
> > + * @lock: protect ack handler data
> > + * @handler: calling this handler when kernel receive ack
> > + * @priv: private data when calling handler to process
> > + */
> > +struct mtk_vcp_ipi_desc {
> > + /* protect handler and priv */
> > + struct mutex lock;
> > + vcp_ipi_handler_t handler;
> > + void *priv;
> > +};
> > +
> > +/**
> > + * struct mtk_vcp_share_obj - share buffer used to send data to
> > vcp
> > + *
> > + * @id: message index
> > + * @len: message size
> > + * @share_buf: message data
> > + */
> > +struct mtk_vcp_share_obj {
> > + unsigned int id;
> > + unsigned int len;
> > + unsigned char share_buf[VCP_SHARE_BUF_SIZE];
> > +};
> > +
> > +/* enum mtk_vcp_ipi_msg_status - the status when send message to
> > vcp */
> > +enum mtk_vcp_ipi_msg_status {
> > + VCODEC_IPI_MSG_STATUS_OK = 0,
> > + VCODEC_IPI_MSG_STATUS_FAIL = -1,
> > + VCODEC_IPI_MSG_STATUS_MAX_INST = -2,
> > + VCODEC_IPI_MSG_STATUS_ILSEQ = -3,
> > + VCODEC_IPI_MSG_STATUS_INVALID_ID = -4,
> > + VCODEC_IPI_MSG_STATUS_DMA_FAIL = -5,
> > +};
> > +
> > +/**
> > + * struct mtk_vcp_msg_node - share buffer used to send data to vcp
> > + *
> > + * @ipi_data: share obj data
> > + * @list: list to store msg node
> > + */
> > +struct mtk_vcp_msg_node {
> > + struct mtk_vcp_share_obj ipi_data;
> > + struct list_head list;
> > +};
> > +
> > +/**
> > + * struct mtk_vcp - vcp firmware private data
> > + *
> > + * @is_init_done: vcp is ready to use
> > + *
> > + * @ipi_mutex: used to protect ipi data
> > + * @msg_signaled: whether receive ack from vcp
> > + * @msg_wq: wake message queue
> > + *
> > + * @ipi_desc: store ack handler
> > + * @ipi_id_ack: the ack handler status
> > + *
> > + * @msg_queue: process vcp message
> > + * @share_data: temp share obj data
> > + *
> > + * @vcp_notify: register notifier to vcp
> > + *
> > + * @vsi_addr: vsi virtual data address
> > + * @vsi_core_addr: vsi core virtual data address
> > + * @iova_addr: vsi iova address
> > + * @vsi_size: vsi size
> > + *
> > + * @pdev: platform device
> > + * @vcp_device: vcp private data
> > + * @is_register_done: register vcodec to vcp
> > + */
> > +struct mtk_vcp {
> > + bool is_init_done;
> > +
> > + /* serialize ipi message send/receive */
> > + struct mutex ipi_mutex;
> > + bool msg_signaled[VCP_IPI_MAX];
> > + wait_queue_head_t msg_wq[VCP_IPI_MAX];
> > +
> > + struct mtk_vcp_ipi_desc ipi_desc[VCP_IPI_MAX];
> > + bool ipi_id_ack[VCP_IPI_MAX];
> > +
> > + struct mtk_vcp_msg_queue msg_queue;
> > + struct mtk_vcp_share_obj share_data;
> > +
> > + struct notifier_block vcp_notify;
> > +
> > + void *vsi_addr;
> > + void *vsi_core_addr;
> > + dma_addr_t iova_addr;
> > + int vsi_size;
> > +
> > + struct platform_device *pdev;
> > + struct mtk_vcp_device *vcp_device;
> > + bool is_register_done;
> > +};
> > +
> > +#endif
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
> > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
> > index 3632037f78f5..41643db9f4eb 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
> > @@ -130,3 +130,4 @@ struct mtk_vcodec_fw
> > *mtk_vcodec_fw_vpu_init(void *priv, enum mtk_vcodec_fw_use
> >
> > return fw;
> > }
> > +EXPORT_SYMBOL_GPL(mtk_vcodec_fw_vpu_init);
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .c
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .c
> > index d220b645e455..ce17b649022f 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .c
> > @@ -376,12 +376,15 @@ static int mtk_vcodec_probe(struct
> > platform_device *pdev)
> > if (!of_property_read_u32(pdev->dev.of_node,
> > "mediatek,vpu",
> > &rproc_phandle)) {
> > fw_type = VPU;
> > + dev->fw_init = mtk_vcodec_fw_vpu_init;
> > } else if (!of_property_read_u32(pdev->dev.of_node,
> > "mediatek,scp",
> > &rproc_phandle)) {
> > fw_type = SCP;
> > + dev->fw_init = mtk_vcodec_fw_scp_init;
> > } else if (!of_property_read_u32(pdev->dev.of_node,
> > "mediatek,vcp",
> > &rproc_phandle)) {
> > fw_type = VCP;
> > + dev->fw_init = mtk_vcodec_fw_vcp_init;
> > } else {
> > dev_dbg(&pdev->dev, "Could not get vdec IPI
> > device");
> > return -ENODEV;
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .h
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .h
> > index c9d27534c63e..15e437323af5 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .h
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .h
> > @@ -251,6 +251,7 @@ struct mtk_vcodec_dec_ctx {
> > * @vdecsys_regmap: VDEC_SYS register space passed through syscon
> > *
> > * @fw_handler: used to communicate with the firmware.
> > + * @fw_init: firmware-specific init callback selected at probe
> > time
> > * @id_counter: used to identify current opened instance
> > *
> > * @dec_mutex: decoder hardware lock
> > @@ -292,6 +293,7 @@ struct mtk_vcodec_dec_dev {
> > struct regmap *vdecsys_regmap;
> >
> > struct mtk_vcodec_fw *fw_handler;
> > + struct mtk_vcodec_fw *(*fw_init)(void *priv, enum
> > mtk_vcodec_fw_use fw_use);
> > u64 id_counter;
> >
> > /* decoder hardware mutex lock */
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv
> > .c
> > b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv
> > .c
> > index 4e4541b2fc8e..dc54d445d98d 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv
> > .c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv
> > .c
> > @@ -259,9 +259,11 @@ static int mtk_vcodec_probe(struct
> > platform_device *pdev)
> > if (!of_property_read_u32(pdev->dev.of_node,
> > "mediatek,vpu",
> > &rproc_phandle)) {
> > fw_type = VPU;
> > + dev->fw_init = mtk_vcodec_fw_vpu_init;
> > } else if (!of_property_read_u32(pdev->dev.of_node,
> > "mediatek,scp",
> > &rproc_phandle)) {
> > fw_type = SCP;
> > + dev->fw_init = mtk_vcodec_fw_scp_init;
> > } else {
> > dev_err(&pdev->dev, "[MTK VCODEC] Could not get
> > venc IPI device");
> > return -ENODEV;
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv
> > .h
> > b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv
> > .h
> > index 0cddfa13594f..934ff648125d 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv
> > .h
> > +++
> > b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv
> > .h
> > @@ -174,6 +174,7 @@ struct mtk_vcodec_enc_ctx {
> > * @venc_pdata: encoder IC-specific data
> > *
> > * @fw_handler: used to communicate with the firmware.
> > + * @fw_init: firmware-specific init callback selected at probe
> > time
> > * @id_counter: used to identify current opened instance
> > *
> > * @enc_mutex: encoder hardware lock.
> > @@ -201,6 +202,7 @@ struct mtk_vcodec_enc_dev {
> > const struct mtk_vcodec_enc_pdata *venc_pdata;
> >
> > struct mtk_vcodec_fw *fw_handler;
> > + struct mtk_vcodec_fw *(*fw_init)(void *priv, enum
> > mtk_vcodec_fw_use fw_use);
> > u64 id_counter;
> >
> > /* encoder hardware mutex lock */
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy
From: Thinh Nguyen @ 2026-07-17 1:56 UTC (permalink / raw)
To: Peter Chen
Cc: Thinh Nguyen, Krishna Kurapati, Krzysztof Kozlowski,
Dmitry Baryshkov, Greg Kroah-Hartman, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Wesley Cheng,
linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <almDYCfHf4WV-giY@hu-petche-lv.qualcomm.com>
On Thu, Jul 16, 2026, Peter Chen wrote:
> On 26-07-17 00:06:37, Thinh Nguyen wrote:
> > >
> > > I have seen fewer platforms use "phy_type" at dts for arm64, dwc3
> > > core uses it only for special cases and the code was added for
> > > ten years ago. For the default situation, we may not need to
> > > change DWC3_GUSB2PHYCFG_EUSB2OPMODE value for UTMI+, Thinh, is it
> > > correct?
> > >
> >
> > The GUSB2PHYCFG.eUSB2OPMODE is only relevant for host mode and mainly
> > for electrical compliance. Usually by default, the CoreConsultant
> > setting should have this set correctly. So not explicitly setting it
> > should be functionally fine (IIRC).
> >
> > That said, this is separate from the GUSB2PHYCFG.PHYIF, which the core
> > uses dwc->hsphy_mode to indicate whether the UTMI interface is 8-bit or
> > 16-bit.
> >
>
> Why only rockchip uses this "phy_type" property, why other SoC vendors
> no this requirement for UTMI width setting?
>
Not just rockchip, some tegra and hikey also use it. Some old qcom dts
files also have it for ulpi.
It is typically set when the coreConsultant default differs from what
the platform needs, so the driver can override it with the correct
setting.
My point is that changing "phy_type" definition is more involved than it
looks. I'm open to alternatives.
Thanks,
Thinh
^ permalink raw reply
* Re: [PATCH v3 2/3] arm64: dts: qcom: purwa: Drop the Hamoa workaround for PDC
From: Bjorn Andersson @ 2026-07-17 2:08 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Maulik Shah, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Konrad Dybcio, linux-arm-msm, linux-kernel,
devicetree, Konrad Dybcio
In-Reply-To: <20260716-starling-of-heavenly-symmetry-09a73a@quoll>
On Thu, Jul 16, 2026 at 09:59:44AM +0200, Krzysztof Kozlowski wrote:
> On Wed, Jul 15, 2026 at 06:52:01PM +0530, Maulik Shah wrote:
> > X1P42100 (Purwa) shares the X1E80100 (Hamoa) PDC device, but the hardware
> > register bug addressed in commit e9a48ea4d90b ("irqchip/qcom-pdc:
> > Workaround hardware register bug on X1E80100") is already fixed in
> > X1P42100 silicon.
> >
> > X1E80100 compatible forces the software workaround. Use the X1P42100
> > specific compatible string for the PDC node to remove the workaround.
> >
> > Fixes: f08edb529916 ("arm64: dts: qcom: Add X1P42100 SoC and CRD")
> > Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> > ---
> > arch/arm64/boot/dts/qcom/purwa.dtsi | 5 +++++
> > 1 file changed, 5 insertions(+)
>
> Why does the DT change appear in the middle of the patchset? Please read
> submitting patches documents - both of them - and maintainer-soc
> profile.
>
I thought I had figured it out, but I'm not sure anymore.
The claim from the cover letter is that patch 1 and 2 are completely
independent, but patch 3 depends on Bartosz's thank you letter [2] that
arrived a week before this series was sent out.
We're not merging the three changes through the same tree and there's no
expressed dependency between patch 2 and 3 (only implicitly by the order
in the series). But as Konrad points out, in-between patch 2 and 3 we
would not enable the secondary GPIO in the PDC driver, so Purwa would
have broken GPIOs for a while (not ok). I think merging them in the
opposite order would be what we want (i.e. 1, 3, then 2)
But this series implies that Purwa has been broken from the start - that
the PDC driver has always operated on the wrong registers.
Perhaps the impact of this was limited as there's not that many direct
&pdc references in the DT, but the patch that Bartosz's thank-you email
was sent for got merged as 77fbc756d9cb ("Revert "pinctrl: qcom:
x1e80100: Bypass PDC wakeup parent for now""), and that would make a lot
more use of the PDC.
So while nothing in this series states it, it sounds like Purwa might be
completely broken right now and this series aims to fix it?
It's not clear to me why the driver change doesn't have a Fixes tag, it
seems like the patch that introduced x1e_quirk was broken and should be
marked as Fixes.
[2] https://lore.kernel.org/linux-arm-msm/CAMRc=MeU0QuRozMscv02M59+a66S05Jm18CyvNE-qSYrY=S7hQ@mail.gmail.com/
Regards,
Bjorn
> Best regards,
> Krzysztof
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox