* Re: [PATCH v2 2/3] drm/msm/adreno: Add support for A704 GPU
From: Dmitry Baryshkov @ 2026-06-16 0:56 UTC (permalink / raw)
To: Akhil P Oommen
Cc: Rob Clark, Sean Paul, Konrad Dybcio, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Marijn Suijten, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Will Deacon, Robin Murphy, Joerg Roedel (AMD), Bibek Kumar Patro,
linux-arm-msm, dri-devel, freedreno, devicetree, linux-kernel,
linux-arm-kernel, iommu, Aditya Sherawat, Konrad Dybcio
In-Reply-To: <20260615-shikra-gpu-v2-2-2f2d1347c3fb@oss.qualcomm.com>
On Mon, Jun 15, 2026 at 03:02:58AM +0530, Akhil P Oommen wrote:
> From: Aditya Sherawat <asherawa@qti.qualcomm.com>
>
> Adreno A704 GPU found in Shikra is an IP reuse of A702 GPU with very
> minimal changes. The only KMD facing difference is the chipid and the
> zap firmware which is specified via devicetree.
>
> Just add the new chipid to enable support for A704 GPU in Shikra.
>
> Signed-off-by: Aditya Sherawat <asherawa@qti.qualcomm.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v2 2/2] pinctrl: qcom: Add the tlmm driver for Maili platform
From: Dmitry Baryshkov @ 2026-06-16 0:55 UTC (permalink / raw)
To: Jingyi Wang, g
Cc: Bjorn Andersson, Linus Walleij, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, aiqun.yu, tingwei.zhang, trilok.soni, yijie.yang,
linux-arm-msm, linux-gpio, devicetree, linux-kernel
In-Reply-To: <20260614-maili-pinctrl-v2-2-0db5bfc23d64@oss.qualcomm.com>
On Sun, Jun 14, 2026 at 11:55:04PM -0700, Jingyi Wang wrote:
> Add support for Maili TLMM configuration and control via the pinctrl
> framework.
>
> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> ---
> drivers/pinctrl/qcom/Kconfig.msm | 10 +
> drivers/pinctrl/qcom/Makefile | 1 +
> drivers/pinctrl/qcom/pinctrl-maili.c | 1630 ++++++++++++++++++++++++++++++++++
> 3 files changed, 1641 insertions(+)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH V1 0/2] arm64: dts: qcom: Shikra SD Card support
From: Dmitry Baryshkov @ 2026-06-16 0:54 UTC (permalink / raw)
To: Monish Chunara
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Vinod Koul, Neil Armstrong, Wesley Cheng,
Ulf Hansson, Kernel Team, linux-arm-msm, devicetree, linux-kernel,
linux-phy, linux-mmc, Nitin Rawat, Pradeep Pragallapati,
Komal Bajaj, Konrad Dybcio
In-Reply-To: <20260604122045.494712-1-monish.chunara@oss.qualcomm.com>
On Thu, Jun 04, 2026 at 05:50:43PM +0530, Monish Chunara wrote:
> This series adds SD card support for the Shikra platform.
>
> The first patch adds the SDHC2 controller node and the necessary pinctrl
> configurations to the base Shikra SoC dtsi. The second patch enables
> this support on the Shikra EVK (CQS, CQM, and IQS variants) by defining
> the regulator supplies and the card detection GPIO.
>
> Testing:
> - Validated on Shikra EVK variants.
>
> This series depends on:
> - https://lore.kernel.org/all/20260527-shikra-dt-v4-0-b5ca1fa0b392@oss.qualcomm.com/
> - https://lore.kernel.org/all/20260521-shikra-rproc-v3-0-2fca0bbe1ad7@oss.qualcomm.com/
> - https://lore.kernel.org/linux-devicetree/20260513-tsens_binding-v1-1-1780c6a6caf2@oss.qualcomm.com/
If the SD card depends on remote proc, tsens or cpufreq, then something
is wrong. Maybe, the way the serieas are organized and sent.
> - https://lore.kernel.org/all/20260524-shikra_epss_l3-v1-0-b1528a436134@oss.qualcomm.com/
> - https://lore.kernel.org/all/20260522-shikra-cpufreq-scaling-v4-0-f042a25896c5@oss.qualcomm.com/
> - https://lore.kernel.org/all/20260530-shikra-dt-m1-v2-0-6bb581035d13@oss.qualcomm.com/
>
> Monish Chunara (2):
> arm64: dts: qcom: Add SD Card support for Shikra SoC
> arm64: dts: qcom: Enable SD card for Shikra EVK
>
> arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts | 18 ++++
> arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts | 18 ++++
> arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts | 18 ++++
> arch/arm64/boot/dts/qcom/shikra.dtsi | 93 +++++++++++++++++++++
> 4 files changed, 147 insertions(+)
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v5 12/16] media: iris: implement support for the Agatti platform
From: sashiko-bot @ 2026-06-16 0:40 UTC (permalink / raw)
To: Dmitry Baryshkov; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260616-iris-ar50lt-v5-12-583b42770b6a@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Logic Error: Invalid hardware frequency calculation due to missing `mb_cycles_vsp` parameter
- [High] Hardware Configuration: The watchdog interrupt is defined but never unmasked, breaking watchdog recovery
- [Low] Build Failure: Initialization of non-existent struct members in qcm2290_data
--
commit eabd242734902d122adf5442b6b1946c0a61380b
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
media: iris: implement support for the Agatti platform
Port support for the AR50Lt video codec core (present for example on the
Agatti platform) to the Iris driver. Unlike more recent cores this
generation doesn't have the PIPE property (as it always has only one
pipe). Also, unlike newer platforms, buffer sizes are requested from the
firmware instead of being calculated by the driver.
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c b/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c
> new file mode 100644
> index 0000000000000..393256f39112b
> --- /dev/null
> +++ b/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c
[ ... ]
> +static struct platform_inst_caps platform_inst_cap_ar50lt = {
> + .min_frame_width = 128,
> + .max_frame_width = 1920,
> + .min_frame_height = 128,
> + .max_frame_height = 1920,
> + .max_mbpf = (1920 * 1088) / 256,
> + .mb_cycles_vpp = 440,
> + .mb_cycles_fw = 733003,
> + .mb_cycles_fw_vpp = 225975,
[Severity: High]
Does this code need to initialize .mb_cycles_vsp?
It looks like .mb_cycles_vsp is omitted here, so it will default to 0. If
the .calc_freq callback in iris_vpu_ar50lt_ops uses
iris_vpu2_calculate_frequency(), that function calculates vsp_freq by
multiplying the macroblocks-per-second by caps->mb_cycles_vsp. Because
mb_cycles_vsp is 0, the resulting hardware clock frequency might be severely
undercalculated, potentially causing performance degradation or hardware
timeouts.
> + .max_frame_rate = 120,
> + .max_operating_rate = 120,
> +};
> +
> +const struct iris_platform_data qcm2290_data = {
> + .firmware_desc_gen1 = &iris_vpu_ar50lt_p1_gen1_s6_desc,
[Severity: Low]
Does struct iris_platform_data have a member named firmware_desc_gen1?
It appears that struct iris_platform_data (defined in
iris_platform_common.h) only contains a generic firmware_desc member. Using
.firmware_desc_gen1 here might result in a compilation error.
> + .vpu_ops = &iris_vpu_ar50lt_ops,
> + .icc_tbl = iris_icc_info_ar50lt,
> + .icc_tbl_size = ARRAY_SIZE(iris_icc_info_ar50lt),
[ ... ]
> + .tz_cp_config_data_size = ARRAY_SIZE(tz_cp_config_ar50lt),
> + .num_vpp_pipe = 1,
> + .no_rpmh = true,
> + .wd_intr_mask = WRAPPER_INTR_STATUS_A2HWD_BMSK,
[Severity: High]
Will the watchdog interrupt actually be unmasked by the hardware?
This defines the platform's watchdog status bit mask as
WRAPPER_INTR_STATUS_A2HWD_BMSK (0x10). However, if
iris_vpu_ar50lt_interrupt_init() in iris_vpu_ar50lt.c overwrites
WRAPPER_INTR_MASK with a hardcoded mask that omits the watchdog bit, the
watchdog interrupt will not be unmasked, preventing the driver from
detecting and recovering from VPU firmware hangs.
> + .icc_ib_multiplier = 2,
> + .max_session_count = 8,
> + .max_core_mbpf = ((1920 * 1088) / 256) * 4,
> + /* Concurrency: 1080p@30 decode + 1080p@30 encode */
> + /* Concurrency: 3 * 1080p@30 decode */
> + .max_core_mbps = (((1920 * 1088) / 256) * 90),
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-iris-ar50lt-v5-0-583b42770b6a@oss.qualcomm.com?part=12
^ permalink raw reply
* Re: [PATCH net-next 8/8] net: mdio: realtek-rtl9300: Add support for RTL839x
From: Jakub Kicinski @ 2026-06-16 0:37 UTC (permalink / raw)
To: markus.stockhausen
Cc: Jakub Kicinski, andrew, hkallweit1, linux, davem, edumazet,
pabeni, netdev, chris.packham, daniel, robh, krzk+dt, conor+dt,
devicetree
In-Reply-To: <20260613112946.1071411-9-markus.stockhausen@gmx.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: mdio: realtek-rtl9300: Add support for RTL839x
This patch adds RTL839x (cypress) support to the realtek-rtl9300 MDIO
driver, reusing the multi-device infrastructure that was prepared for
this purpose. It introduces a new RTL8390 register set, four c22/c45
read/write helpers, and a new otto_emdio_8390_info compatible table
entry.
> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index d9ff0b0aecbb..2ab1aeb85eed 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -139,6 +139,29 @@
> #define RTL8380_SMI_POLL_CTRL 0xa17c
> #define RTL8380_SMI_PORT0_5_ADDR_CTRL 0xa1c8
>
> +#define RTL8390_NUM_BUSES 2
> +#define RTL8390_NUM_PAGES 8192
> +#define RTL8390_NUM_PORTS 52
> +#define RTL8390_BCAST_PHYID_CTRL 0x03ec
> +#define RTL8390_PHYREG_ACCESS_CTRL 0x03dc
> +#define RTL8390_PHY_CTRL_REG_ADDR GENMASK(9, 5)
> +#define RTL8390_PHY_CTRL_PARK_PAGE GENMASK(27, 23)
[Low]
Is RTL8390_PHY_CTRL_PARK_PAGE used anywhere? It does not appear to
be referenced in the rest of the patch, while the analogous masks for
the 8380 and 9300 variants are populated with 0x1f in their c22 helpers.
For example, otto_emdio_9300_read_c22() does:
.c22_data = FIELD_PREP(RTL9300_PHY_CTRL_REG_ADDR, regnum) |
FIELD_PREP(RTL9300_PHY_CTRL_PARK_PAGE, 0x1f) |
FIELD_PREP(RTL9300_PHY_CTRL_MAIN_PAGE, priv->page[port]),
while otto_emdio_8390_read_c22() below only sets REG_ADDR and MAIN_PAGE,
leaving bits 27..23 of c22_data at 0:
> +#define RTL8390_PHY_CTRL_MAIN_PAGE GENMASK(22, 10)
> +#define RTL8390_PHY_CTRL_FAIL BIT(1)
> +#define RTL8390_PHY_CTRL_WRITE BIT(3)
> +#define RTL8390_PHY_CTRL_READ 0
> +#define RTL8390_PHY_CTRL_TYPE_C45 BIT(2)
> +#define RTL8390_PHY_CTRL_TYPE_C22 0
[ ... ]
> @@ -457,6 +480,62 @@ static int otto_emdio_8380_write_c45(struct mii_bus *bus, int port,
> return otto_emdio_write_cmd(bus, RTL8380_PHY_CTRL_TYPE_C45, &cmd_data);
> }
>
> +static int otto_emdio_8390_read_c22(struct mii_bus *bus, int port, int regnum, u32 *value)
> +{
> + struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
> + struct otto_emdio_cmd_regs cmd_data = {
> + .c22_data = FIELD_PREP(RTL8390_PHY_CTRL_REG_ADDR, regnum) |
> + FIELD_PREP(RTL8390_PHY_CTRL_MAIN_PAGE, priv->page[port]),
> + .ext_page = FIELD_PREP(RTL8390_PHY_CTRL_EXT_PAGE, 0x1ff),
> + .io_data = FIELD_PREP(RTL8390_PHY_CTRL_INDATA, port),
> + };
> +
> + return otto_emdio_read_cmd(bus, RTL8390_PHY_CTRL_TYPE_C22, &cmd_data,
> + RTL8390_PHY_CTRL_DATA, value);
> +}
> +
> +static int otto_emdio_8390_write_c22(struct mii_bus *bus, int port, int regnum, u16 value)
> +{
> + struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
> + struct otto_emdio_cmd_regs cmd_data = {
> + .c22_data = FIELD_PREP(RTL8390_PHY_CTRL_REG_ADDR, regnum) |
> + FIELD_PREP(RTL8390_PHY_CTRL_MAIN_PAGE, priv->page[port]),
Should these helpers also set RTL8390_PHY_CTRL_PARK_PAGE to 0x1f, matching
the 8380/9300 convention, or is the macro itself intended to be removed
because the 8390 hardware does not need a parking page?
> + .ext_page = FIELD_PREP(RTL8390_PHY_CTRL_EXT_PAGE, 0x1ff),
> + .io_data = FIELD_PREP(RTL8390_PHY_CTRL_INDATA, value),
> + .port_mask_high = (u32)(BIT_ULL(port) >> 32),
> + .port_mask_low = (u32)(BIT_ULL(port)),
> + };
> +
> + return otto_emdio_write_cmd(bus, RTL8390_PHY_CTRL_TYPE_C22, &cmd_data);
> +}
[ ... ]
^ permalink raw reply
* Re: [PATCH net-next 5/8] net: mdio: realtek-rtl9300: Add c45 over c22 mitigation
From: Jakub Kicinski @ 2026-06-16 0:37 UTC (permalink / raw)
To: markus.stockhausen
Cc: Jakub Kicinski, andrew, hkallweit1, linux, davem, edumazet,
pabeni, netdev, chris.packham, daniel, robh, krzk+dt, conor+dt,
devicetree
In-Reply-To: <20260613112946.1071411-6-markus.stockhausen@gmx.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: mdio: realtek-rtl9300: Add c45 over c22 mitigation
This patch adds a per-port state machine that disables hardware MAC polling
around a register 13/14/13/14 c45-over-c22 MMD access sequence so the
sequence cannot be interrupted by a parallel polling read. Polling is
disabled when the first MII_MMD_CTRL access is seen and re-enabled when
the four-step sequence completes or unexpected input is detected.
A few questions on the new state-machine corner cases below.
> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index a7fd075947b6..e206ee3e2b1c 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
[ ... ]
> @@ -254,6 +257,43 @@ static int otto_emdio_set_port_polling(struct otto_emdio_priv *priv, int port, b
> BIT(port % 32), active);
> }
>
> +static int otto_emdio_mmd_prefix(struct otto_emdio_priv *priv, int port, int regnum)
> +{
> + u8 newstate, *state = &priv->mmd_state[port];
> + int expected, ret = 0;
> +
> + if (!priv->init_done)
> + return 0;
> + /*
> + * Disabled polling might produce link flapping and false notification interrupts on the
> + * MAC layer. In this case disable c45 over c22 MMD access because chances are high that
> + * the register 13/14/13/14 sequence is intercepted by a parallel hardware access. As
> + * a workaround the PHY must provide its own mmd read/write() callbacks and redirect to
> + * normal c22 registers. See rtlgen_read_mmd().
> + */
> + if (priv->info->link_flap)
> + return (regnum == MII_MMD_DATA || regnum == MII_MMD_CTRL) ? -EIO : 0;
> +
> + expected = (*state & 1) ? MII_MMD_DATA : MII_MMD_CTRL;
> + newstate = regnum == expected ? *state + 1 : 0;
> +
> + if (newstate == 1 || newstate < *state)
> + ret = otto_emdio_set_port_polling(priv, port, !newstate);
> + *state = newstate;
> +
> + return ret;
> +}
> +
> +static int otto_emdio_mmd_postfix(struct otto_emdio_priv *priv, int port, int regnum)
> +{
> + if (priv->mmd_state[port] != 4)
> + return 0;
> +
> + priv->mmd_state[port] = 0;
> +
> + return otto_emdio_set_port_polling(priv, port, true);
> +}
[High]
Can otto_emdio_mmd_prefix() and otto_emdio_mmd_postfix() get out of sync
on a transient MDIO failure?
The prefix transitions state 0->1 on the first MII_MMD_CTRL access and
calls otto_emdio_set_port_polling(port, false). In the c22 read/write
helpers below, if priv->info->read_c22() / write_c22() then returns an
error (otto_emdio_run_cmd() can return -ETIMEDOUT from
regmap_read_poll_timeout(), -ENXIO via cmd_fail, or any regmap_write()
error), the function returns immediately and never calls
otto_emdio_mmd_postfix().
Polling is left disabled, and *state stays non-zero.
On subsequent retries with regnums that still match the parity-based
expected value (the natural retry shape for an EEE 13/14/13/14
sequence), mmd_prefix keeps doing *state + 1 unbounded (state walks
through 5, 6, 7, ...). mmd_postfix only re-enables polling when
mmd_state == 4 exactly:
if (priv->mmd_state[port] != 4)
return 0;
so once the counter overshoots 4 it stays disabled until either an
unrelated regnum forces newstate < *state in mmd_prefix, or the u8
wraps at 255->0.
The commit message says polling is re-enabled "as soon as the sequence
is finished or any other unexpected input is detected", but a transient
MDIO failure mid-sequence is neither.
Should the error paths in otto_emdio_read_c22() / otto_emdio_write_c22()
reset mmd_state and re-enable polling on a failed transfer?
> @@ -463,7 +503,15 @@ static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
> if (regnum == 31)
> return priv->page[port];
>
> + ret = otto_emdio_mmd_prefix(priv, port, regnum);
> + if (ret)
> + return ret;
> +
> ret = priv->info->read_c22(bus, port, regnum, &value);
> + if (ret)
> + return ret;
> +
> + ret = otto_emdio_mmd_postfix(priv, port, regnum);
> }
>
> return ret ? ret : value;
[Medium]
The regnum == 31 page-select path returns from inside the scoped_guard
before reaching mmd_prefix() / mmd_postfix():
if (regnum == 31)
return priv->page[port];
If a page-register access occurs while mmd_state is mid-sequence (1, 2,
or 3), the counter is not reset and the polling-disable bit is not
restored, even though a page write between MMD steps is exactly the
"unexpected input" the commit message describes.
Recovery only happens on the next non-13/14 access that falls through
mmd_prefix and triggers the newstate < *state branch.
Should the regnum == 31 path also reconcile mmd_state and re-enable
polling when a sequence is in progress? The same question applies in
otto_emdio_write_c22().
[Low]
A separate observation about the read return path:
> return ret ? ret : value;
After a successful priv->info->read_c22() populates value, ret is
overwritten by mmd_postfix()'s return. If
otto_emdio_set_port_polling(priv, port, true) inside mmd_postfix()
fails, the function returns that error even though the data transfer
succeeded, so the PHY layer discards correctly read data. For
read-clear / latched status registers the hardware-side bits have
already been consumed.
Would it be preferable to treat a postfix failure as a polling-
bookkeeping issue (log and proceed) and still return the read value?
> @@ -487,7 +535,15 @@ static int otto_emdio_write_c22(struct mii_bus *bus, int phy_id, int regnum, u16
> return 0;
> }
>
> - return priv->info->write_c22(bus, port, regnum, value);
> + ret = otto_emdio_mmd_prefix(priv, port, regnum);
> + if (ret)
> + return ret;
> +
> + ret = priv->info->write_c22(bus, port, regnum, value);
> + if (ret)
> + return ret;
> +
> + return otto_emdio_mmd_postfix(priv, port, regnum);
> }
> }
The same shape exists for the write path: a postfix failure here turns
a successful write into a reported error and may cause an upper-layer
retry of an already-applied write.
^ permalink raw reply
* Re: [PATCH net-next 2/8] net: mdio: realtek-rtl9300: Add polling documentation
From: Jakub Kicinski @ 2026-06-16 0:37 UTC (permalink / raw)
To: markus.stockhausen
Cc: Jakub Kicinski, andrew, hkallweit1, linux, davem, edumazet,
pabeni, netdev, chris.packham, daniel, robh, krzk+dt, conor+dt,
devicetree
In-Reply-To: <20260613112946.1071411-3-markus.stockhausen@gmx.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: mdio: realtek-rtl9300: Add polling documentation
This commit adds a comment block to the rtl9300 MDIO driver describing
how the Realtek Otto switches' hardware polling unit interacts with the
MDIO bus and what this implies for kernel-initiated PHY accesses.
> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 892ed3780a65..da2864c94d2c 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -35,6 +35,72 @@
[ ... ]
> + * phy_read(phy, 5); phy_read(phy, 14);
> + * phy_read(phy, 6);
> + *
> + * The c22 over c45 register 13/14 sequences read MDIO_AN_EEE_ADV and MDIO_AN_EEE_LPABLE. As soon
> + * as one PHY status is read, the polling engine goes over to the next PHY. Basically the bus is
> + * always busy and the MAC status is updated in realtime.
[ ... ]
> + * - A c45 over c22 kernel access sequence is most likely to fail because chances are high that
> + * the polling engine overwrites registers 13/14 in between.
[Low]
The two paragraphs above describe the same mechanism — using Clause 22
transactions on registers 13 (MII_MMD_CTRL) and 14 (MII_MMD_DATA) to
indirectly access Clause 45 MMD registers — but use opposite wordings:
"c22 over c45" in the first paragraph and "c45 over c22" in the second.
The example writes shown earlier in the same comment (phy_write(phy, 13,
0x7); phy_write(phy, 14, 0x3c); ...) are the standard C22 indirect access
to C45 MMD registers, conventionally referred to in the kernel as
"C45 over C22".
Since the stated goal of this comment is to simplify future development
and review, would it make sense to use the same phrasing in both places
(for instance "C45 over C22" in both)?
^ permalink raw reply
* Re: [PATCH net-next 1/8] dt-bindings: net: realtek,rtl9301-mdio: Add RTL83xx series
From: Jakub Kicinski @ 2026-06-16 0:37 UTC (permalink / raw)
To: markus.stockhausen
Cc: Jakub Kicinski, andrew, hkallweit1, linux, davem, edumazet,
pabeni, netdev, chris.packham, daniel, robh, krzk+dt, conor+dt,
devicetree
In-Reply-To: <20260613112946.1071411-2-markus.stockhausen@gmx.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: dt-bindings: net: realtek,rtl9301-mdio: Add RTL83xx series
This commit extends the realtek,rtl9301-mdio binding to cover the
lower-end Realtek Otto switch families: Maple (RTL8380/8381/8382) and
Cypress (RTL8391/8392/8393). New compatible strings are added for these
SoCs alongside the existing RTL9300/RTL9311 entries.
> diff --git a/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml
> index 271e05bae9c5..de33364b67ef 100644
> --- a/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml
> +++ b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml
> @@ -12,6 +12,16 @@ maintainers:
> properties:
> compatible:
> oneOf:
> + - items:
> + - enum:
> + - realtek,rtl8381-mdio
> + - realtek,rtl8382-mdio
> + - const: realtek,rtl8380-mdio
> + - items:
> + - enum:
> + - realtek,rtl8392-mdio
> + - realtek,rtl8393-mdio
> + - const: realtek,rtl8391-mdio
[Low]
With this change the binding now describes three distinct SoC families
(RTL83xx Maple, RTL839x Cypress, and the existing RTL93xx), but the
file still carries:
title: Realtek RTL9300 MDIO Controller
Should the title (and the top-level description, if applicable) be
updated to reflect the broader scope now that RTL8380/8381/8382 and
RTL8391/8392/8393 are covered by the same schema?
> @@ -24,6 +34,8 @@ properties:
> - realtek,rtl9313-mdio
> - const: realtek,rtl9311-mdio
> - enum:
> + - realtek,rtl8380-mdio
> + - realtek,rtl8391-mdio
> - realtek,rtl9301-mdio
> - realtek,rtl9311-mdio
>
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH v5 02/16] media: iris: Filter UBWC raw formats based on hardware capabilities
From: Dmitry Baryshkov @ 2026-06-16 0:32 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260616001733.5FC251F000E9@smtp.kernel.org>
On Tue, Jun 16, 2026 at 12:17:32AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The `iris_vdec_formats_noubwc_cap` array contains a zero-initialized gap at index 1, which prematurely terminates V4L2 format enumeration and improperly validates 0 as a valid pixel format.
I will check later and respond or fix the issues.
> --
>
> commit 1ff6b6e464175bc1ccf6878e0aa89ffec126608b
> Author: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v5 14/16] media: iris: add Gen2 firmware support on the Agatti platform
From: Dmitry Baryshkov @ 2026-06-16 0:31 UTC (permalink / raw)
To: sashiko-reviews; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260616002607.0BABA1F000E9@smtp.kernel.org>
On Tue, Jun 16, 2026 at 12:26:06AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Incomplete dual firmware refactoring leads to build breakage and NULL pointer dereference during device probe.
> --
>
> commit 56abb89242d476eadcfa7c399ffc9ee1dfc1fca4
> Author: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
>
> media: iris: add Gen2 firmware support on the Agatti platform
>
> This patch extends the iris driver to support both HFI Gen1 and Gen2
> firmwares for the Agatti platform.
>
> > diff --git a/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c b/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c
> > index 393256f39112b..d9de7dcb59e3a 100644
> > --- a/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c
> > +++ b/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c
>
> [ ... ]
>
> > @@ -78,7 +84,8 @@ static struct platform_inst_caps platform_inst_cap_ar50lt = {
> > };
> >
> > const struct iris_platform_data qcm2290_data = {
> > - .firmware_desc_gen1 = &iris_vpu_ar50lt_p1_gen1_s6_desc,
> > + .firmware_desc_gen1 = &iris_vpu_ar50lt_p1_gen1_desc,
> > + .firmware_desc_gen2 = &iris_vpu_ar50lt_p1_gen2_s6_desc,
> > .vpu_ops = &iris_vpu_ar50lt_ops,
>
> [Severity: High]
> Does struct iris_platform_data actually contain the firmware_desc_gen2 field?
>
> It looks like this might cause a build failure if struct iris_platform_data
> hasn't been updated to include these specific gen1/gen2 fields.
Huh? It's listed as a dependency patch.
>
> Additionally, does this introduce a NULL pointer dereference during device
> probe? If iris_probe() in drivers/media/platform/qcom/iris/iris_probe.c
> still unconditionally reads the legacy firmware_desc field:
>
> iris_probe() {
> ...
> core->iris_firmware_desc = core->iris_platform_data->firmware_desc;
> core->iris_firmware_data = core->iris_firmware_desc->firmware_data;
> ...
> }
>
> Won't this dereference a NULL pointer since the legacy firmware_desc is left
> uninitialized for this platform? It seems a prerequisite patch refactoring
> struct iris_platform_data and the probe function might be missing from this
> series.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260616-iris-ar50lt-v5-0-583b42770b6a@oss.qualcomm.com?part=14
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v5 13/16] media: iris: Introduce buffer size calculations for AR50LT
From: Dmitry Baryshkov @ 2026-06-16 0:30 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260616002148.EE7CB1F000E9@smtp.kernel.org>
On Tue, Jun 16, 2026 at 12:21:48AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [High] The AR50LT decoder line buffer size calculations accept an `is_opb` parameter but completely ignore it, failing to allocate memory for the VPSS line buffer when OPB (split mode) is enabled.
> - [High] The AR50LT BIN buffer size calculations for H.264 and HEVC ignore `num_vpp_pipes`, preventing proper buffer partitioning and alignment across hardware pipelines.
> - [High] The VP9 AR50LT line buffer calculation omits critical component sizes for DMA metadata and QP maps.
> - [High] Missing SEI user data allocation in the H.264 AR50LT persist buffer calculation.
Vikash, Visnu, could you pleae check whether buffer sizes for HFI Gen2
are calculated correctly or not?
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v2] arm64: dts: qcom: lemans-evk: Describe the PCIe M.2 Key E connector
From: Dmitry Baryshkov @ 2026-06-16 0:28 UTC (permalink / raw)
To: Wei Deng
Cc: andersson, konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm,
devicetree, linux-kernel, manivannan.sadhasivam,
bartosz.golaszewski, quic_chezhou, cheng.jiang, shuai.zhang,
jinwang.li, xiuzhuo.shang, mengshi.wu
In-Reply-To: <20260615103228.3104083-1-wei.deng@oss.qualcomm.com>
On Mon, Jun 15, 2026 at 04:02:28PM +0530, Wei Deng wrote:
> The lemans EVK has the PCIe M.2 Mechanical Key E connector to connect
> wireless connectivity cards over PCIe and UART interfaces. Hence,
> describe the connector node and link it with the PCIe 0 Root Port and
> UART17 nodes through graph port/endpoint.
>
> Also add 'compatible = "pciclass,0604"' to the pcieport0 node in
> lemans.dtsi to allow the PCI subsystem to associate the DT node with
> the PCI-to-PCI bridge device.
>
> The M.2 Key E connector is powered by a 3.3V fixed regulator
> (vreg_wcn_3p3) which is sourced from the board's 12V DC input rail
> (vreg_dcin_12v). Both regulators are always-on and are required by the
> pcie-m2-e-connector binding.
>
> Also add the serial1 = &uart17 alias, which is required for the
> Bluetooth serdev device to be enumerated on the UART17 interface.
>
> Signed-off-by: Wei Deng <wei.deng@oss.qualcomm.com>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> Changes in v2:
> - Collected Reviewed-by tag and reorganized the patch
Please don't send the patches as replies to the previous iterations.
Each new revision should be in a separate thread.
>
> Link: https://lore.kernel.org/linux-arm-msm/20260608091702.3797437-2-wei.deng@oss.qualcomm.com/ [v1]
>
> arch/arm64/boot/dts/qcom/lemans-evk.dts | 75 +++++++++++++++++++++++++
> arch/arm64/boot/dts/qcom/lemans.dtsi | 1 +
> 2 files changed, 76 insertions(+)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14
From: Kurt Borja @ 2026-06-16 0:26 UTC (permalink / raw)
To: David Lechner (TI), Jonathan Cameron, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Kurt Borja, Nguyen Minh Tien, linux-iio, devicetree, linux-kernel
In-Reply-To: <20260615-iio-adc-ti-ads122c14-v1-1-e6bdadf7cb2b@baylibre.com>
On Mon Jun 15, 2026 at 4:59 PM -05, David Lechner (TI) wrote:
> Add new bindings for ti,ads122c14 and similar devices.
>
> This is an ADC that is primarily intended for use with temperature
> sensors. There are a few unusual properties because of this. In
> particular, the reference voltage source and current output requirements
> can be different for each measurement, so these are included in the
> channel bindings.
>
> The REFP/REFN reference voltage is usually just connected to a resistor
> that is being driven by the ADC's current outputs, so there is special
> property for this case rather than requiring a regulator to be defined
> to represent that.
>
> ti,vref-source is reused from ti,tlv320adcx140.yaml (otherwise might
> have preferred an enum of strings).
>
> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
> ---
> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++++++++++++++++++
> MAINTAINERS | 7 +
> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
> 3 files changed, 242 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
> new file mode 100644
> index 000000000000..dc7f37cad772
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
> @@ -0,0 +1,224 @@
[...]
> +patternProperties:
> + ^channel@[0-7]$:
> + $ref: adc.yaml
> +
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + maximum: 16 # arbitrary limit, channel@ can be any combination of AIN0-AIN7
> +
> + single-channel:
> + maximum: 7
> +
> + diff-channels:
> + items:
> + maximum: 7
> +
> + bipolar:
> + description:
> + Set this flag if the differential input can be negative.
> +
> + excitation-channels:
I noticed this doesn't have the "ti," prefix. Is your plan to add this
to adc.yaml?
Also, do you think excitation-pins might be a better name?
> + description: AINx pins used as current output.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 1
> + maxItems: 2
> + items:
> + maximum: 7
> +
> + excitation-current-microamp:
> + description: The current output of the excitation channels in microamps.
> + minimum: 1
> + maximum: 1000
> +
> + current-chopping:
If you agree with the above comment, I think this too should be added.
In that case, can we call this something like
excitation-current-rotation for less ambiguity?
I say this because my device has an "IDAC rotation mode", but it also has
a "Chop Mode" which rotates analog inputs and averages consecutive
conversions.
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + If provided, the two excitation channels are to be used with current
> + chopping enabled.
> +
> + ti,vref-source:
> + description: |
> + Indicates the source for the reference voltage for this channel.
> + 0 - Internal 2.5V reference
> + 1 - Internal 1.25V reference
> + 2 - External reference (REFP-REFN)
> + 3 - AVDD as reference
> +
> + For convenience, macros for these values are available in
> + dt-bindings/iio/adc/ti,ads112c14.h.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + maximum: 3
> + default: 0
> +
> + dependencies:
> + excitation-channels: [ excitation-current-microamp ]
> + excitation-current-microamp: [ excitation-channels ]
> + current-chopping: [ excitation-channels ]
> +
> + oneOf:
> + - required: [ single-channel ]
> + - required: [ diff-channels ]
[...]
--
Thanks,
~ Kurt
^ permalink raw reply
* Re: [PATCH 1/2] power: sequencing: pcie-m2: Add PCI ID 0x1103 for WCN6855 Bluetooth
From: Dmitry Baryshkov @ 2026-06-16 0:26 UTC (permalink / raw)
To: Wei Deng
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Manivannan Sadhasivam, Bartosz Golaszewski,
linux-arm-msm, devicetree, linux-kernel, linux-pci, linux-pm,
quic_chezhou, cheng.jiang, shuai.zhang, jinwang.li, xiuzhuo.shang,
mengshi.wu
In-Reply-To: <20260608091702.3797437-2-wei.deng@oss.qualcomm.com>
On Mon, Jun 08, 2026 at 02:47:01PM +0530, Wei Deng wrote:
> WCN6855 is a Qualcomm Wi-Fi/BT combo chip that uses PCI device ID
> 0x1103. Add it to pwrseq_m2_pci_ids[] alongside the existing 0x1107
> (WCN7850) entry, so that the pwrseq-pcie-m2 driver creates a Bluetooth
> serdev device for WCN6855 cards inserted into PCIe M.2 Key E connectors.
>
> Signed-off-by: Wei Deng <wei.deng@oss.qualcomm.com>
> ---
> drivers/power/sequencing/pwrseq-pcie-m2.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
> index efeb25ba9c79..b3af14464314 100644
> --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> @@ -188,6 +188,8 @@ static int pwrseq_pcie_m2_match(struct pwrseq_device *pwrseq,
> static const struct pci_device_id pwrseq_m2_pci_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x1107),
> .driver_data = (kernel_ulong_t)"qcom,wcn7850-bt" },
> + { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x1103),
> + .driver_data = (kernel_ulong_t)"qcom,wcn6855-bt" },
> { } /* Sentinel */
Please keep the list sorted. I saw that Bartosz has applied the patch.
Would you please send a followup?
> };
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v5 14/16] media: iris: add Gen2 firmware support on the Agatti platform
From: sashiko-bot @ 2026-06-16 0:26 UTC (permalink / raw)
To: Dmitry Baryshkov; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260616-iris-ar50lt-v5-14-583b42770b6a@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete dual firmware refactoring leads to build breakage and NULL pointer dereference during device probe.
--
commit 56abb89242d476eadcfa7c399ffc9ee1dfc1fca4
Author: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
media: iris: add Gen2 firmware support on the Agatti platform
This patch extends the iris driver to support both HFI Gen1 and Gen2
firmwares for the Agatti platform.
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c b/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c
> index 393256f39112b..d9de7dcb59e3a 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c
> +++ b/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c
[ ... ]
> @@ -78,7 +84,8 @@ static struct platform_inst_caps platform_inst_cap_ar50lt = {
> };
>
> const struct iris_platform_data qcm2290_data = {
> - .firmware_desc_gen1 = &iris_vpu_ar50lt_p1_gen1_s6_desc,
> + .firmware_desc_gen1 = &iris_vpu_ar50lt_p1_gen1_desc,
> + .firmware_desc_gen2 = &iris_vpu_ar50lt_p1_gen2_s6_desc,
> .vpu_ops = &iris_vpu_ar50lt_ops,
[Severity: High]
Does struct iris_platform_data actually contain the firmware_desc_gen2 field?
It looks like this might cause a build failure if struct iris_platform_data
hasn't been updated to include these specific gen1/gen2 fields.
Additionally, does this introduce a NULL pointer dereference during device
probe? If iris_probe() in drivers/media/platform/qcom/iris/iris_probe.c
still unconditionally reads the legacy firmware_desc field:
iris_probe() {
...
core->iris_firmware_desc = core->iris_platform_data->firmware_desc;
core->iris_firmware_data = core->iris_firmware_desc->firmware_data;
...
}
Won't this dereference a NULL pointer since the legacy firmware_desc is left
uninitialized for this platform? It seems a prerequisite patch refactoring
struct iris_platform_data and the probe function might be missing from this
series.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-iris-ar50lt-v5-0-583b42770b6a@oss.qualcomm.com?part=14
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: phy: qcom,usb-hs-phy: add qcom,vendor-init-seq
From: Dmitry Baryshkov @ 2026-06-16 0:24 UTC (permalink / raw)
To: github.com
Cc: Konrad Dybcio, linux-phy, devicetree, linux-arm-msm, Vinod Koul,
Neil Armstrong, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson
In-Reply-To: <0298e09a5b4497910c226269daa2586b@herrie.org>
On Mon, Jun 15, 2026 at 02:17:08PM +0200, me@herrie.org wrote:
> On 2026-06-12 02:25, Dmitry Baryshkov wrote:
> > On Thu, Jun 11, 2026 at 12:39:45PM +0200, Konrad Dybcio wrote:
> > > On 6/4/26 1:02 AM, Dmitry Baryshkov wrote:
> > > > On Wed, Jun 03, 2026 at 06:09:18PM +0200, me@herrie.org wrote:
> > > >> On 2026-06-03 15:57, Dmitry Baryshkov wrote:
> > > >>> On Wed, Jun 03, 2026 at 07:48:08AM +0200, Herman van Hazendonk wrote:
> > > >>>> Add an optional "qcom,vendor-init-seq" property carrying raw ULPI
> > > >>>> (address, value) pairs that are written after PHY reset.
> > > >>>>
> > > >>>> Unlike the existing "qcom,init-seq" property, the address field is
> > > >>>> NOT offset by ULPI_EXT_VENDOR_SPECIFIC, so the new property can
> > > >>>> reach the standard ULPI vendor register range (0x30-0x3f). MSM8x60-
> > > >>>> class hardware needs this range to programme pre-emphasis, HS driver
> > > >>>> slope and CDR auto-reset bits the legacy msm_otg driver used to set
> > > >>>> via platform data.
> > > >>>
> > > >>> Are those register writes specific to the device or to the whole
> > > >>> platform? In the latter case please extend the driver to write them.
> > > >>
> > > >> Looking at every MSM8x60 reference kernel I could find (Qualcomm's own
> > > >> msm8x60 board, HP TouchPad / APQ8060, and some HTC/Saumsung MSM8660
> > > >> devices), the writes split into two groups:
> > > >>
> > > >> Platform-level (same across all MSM8x60 hardware):
> > > >> - reg 0x36 bits 1+2: CDR auto-reset disabled, SE1 gating disabled
> > > >> - reg 0x32 bits [5:4]: pre-emphasis at 20%
> > > >>
> > > >> Board-specific:
> > > >> - reg 0x32 bits [3:0]: HS driver slope — HP TouchPad uses 5, HTC
> > > >> devices use 1. This clearly depends on board layout (trace length,
> > > >> connector loading, etc.).
> > > >>
> > > >> So the platform-level writes should move unconditionally into the driver
> > > >> behind a match-data flag for the MSM8x60-class compatible, and only the
> > > >> HS driver slope value belongs in DT.
> > > >
> > > > Looks like it. Please hardcode the value for your platform in the driver
> > > > (with the comment), meanwhile we can try looking up the actual values.
> > >
> > > Do we have the values for a MTP/QRD (or whatever they used to be
> > > called
> > > back then..), like we would usually put in there?
> >
> > As far as I can understand msm-3.0 and msm-3.4 most of the boards were
> > writing 0 here (although it might have been unexpected). None of the
> > board files set the hsdrvslope value (which means 0).
> >
> > Please correct me if I'm wrong. I see that for tenderloin kernels change
> > that to 0x5, but I can't find a sensible commit message.
> >
> > I could not find the documentation for vendor ULPI registers for those
> > chips, so I don't think we can identify, how to make sense of those
> > values. In such a case and having different board-specific values, we
> > don't have a better option than having a qcom,hsdrvslope (or similarly
> > named) property in DT.
> Dmitry/Konrad,
>
> The bit-level meaning *is* documented byte-for-byte across
> every MSM8x60 downstream tree I could find: Code Aurora is the
> canonical source:
>
> arch/arm/mach-msm/include/mach/msm_hsusb_hw.h
>
> #define ULPI_CONFIG_REG3 0x32
> #define ULPI_DIGOUT_CTRL 0x36 /* on MSM7x30 / MSM8x60 */
> #define ULPI_CDR_AUTORESET (1 << 1)
> #define ULPI_SE1_GATE (1 << 2)
> #define ULPI_PRE_EMPHASIS_MASK (3 << 4)
> #define ULPI_HSDRVSLOPE_MASK (0x0F)
>
> reg 0x32 [5:4]: pre-emphasis level (00 disabled, 11 = 20%, 10 = 10%)
> reg 0x32 [3:0]: HS driver slope
> reg 0x36 bit 1: CDR auto-reset enable
> reg 0x36 bit 2: SE1 gating enable
>
> The legacy `msm72k_otg.c` semantics for the 0x36 bits are inverted:
> setting the bit disables the function (see set_cdr_auto_reset() and
> set_se1_gating()). Every MSM8x60 reference board sets both bits,
> i.e. CDR auto-reset disabled + SE1 gating disabled.
>
> Surveying every MSM8x60-class board file I can reach: Qualcomm's own
> reference, Samsung, Sony, HTC, and HP: the platform-level values
> are unanimous:
>
> pre-emphasis = 20%
> CDR auto-reset = disabled
> SE1 gating = disabled
>
> | Vendor / board | hsdrvslope |
> | -----------------------------------------------|------------|
> | Qualcomm reference (SURF / FFA / Fluid / | |
> | Dragon / Fusion -- board-msm8x60.c) | unset (0) |
> | Samsung Galaxy S2 family (Q1 / Celox / Dali / | |
> | generic 8x60 MTP) | unset (0) |
> | Sony MSM8660 (sony-kernel-msm8660) | unset (0) |
> | HTC MSM8660 (shooter / holiday / pyramid / | |
> | doubleshot / shooter_u / ruby) | 1 |
> | HP TouchPad (board-tenderloin.c) | 5 |
>
> So Qualcomm's own MTP, the closest thing to a reference platform we
> have, leaves hsdrvslope at the silicon default (0). Two OEMs (HTC,
> HP) override it to non-zero values that match their board layout.
> Neither override has a commit message. HTC's value is buried in an
> init_seq array, HP's appears as `.hsdrvslope = 0x05` in
> board-tenderloin.c with no comment. I've kept TouchPad's 5 because
> that's the value the device shipped with and our HS link is happy
> with it; changing it risks regressing eye margin on the silicon we
> still have to test against.
>
> So the plan is exactly what you both already converged on:
>
> 1. Drop qcom,vendor-init-seq from the binding (this patch goes away
> entirely).
> 2. In the driver, hardcode the three platform-level writes behind
> the qcom,usb-hs-phy-msm8660 compatible match-data, with a
> comment citing msm_hsusb_hw.h as the source for the bit names.
> 3. Add a single qcom,hs-drv-slope property (u8, 0..15) for the
> board-varying value. TouchPad DT sets 5. Absent ⇒ leave silicon
> default in place, matching Qualcomm/Samsung/Sony reference
> behaviour.
>
> I'll send the new v in that shape: single dt-bindings patch
> (qcom,hs-drv-slope) plus the driver patch that hardcodes the platform
> writes and consumes the new property.
Sounds good. Please explain in the commit messages that you can't decode
how the values correspond to the actual slope values (I couldn't find
corresponding doc inside Qualcomm).
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v2 2/6] iommu/arm-smmu: Add interconnect bandwidth voting support
From: Dmitry Baryshkov @ 2026-06-16 0:22 UTC (permalink / raw)
To: Bibek Kumar Patro
Cc: Will Deacon, Robin Murphy, Joerg Roedel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
linux-arm-kernel, iommu, devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <8518a085-b8b7-4ee6-b08c-8dde3971a6f1@oss.qualcomm.com>
On Mon, Jun 15, 2026 at 06:55:45PM +0530, Bibek Kumar Patro wrote:
>
>
> On 6/8/2026 7:25 PM, Dmitry Baryshkov wrote:
> > On Tue, May 26, 2026 at 08:12:03PM +0530, Bibek Kumar Patro wrote:
> > > On some SoCs the SMMU registers require an active interconnect
> > > bandwidth vote to be accessible. While other clients typically
> > > satisfy this requirement implicitly, certain corner cases (e.g.
> > > during sleep/wakeup transitions) can leave the SMMU without a
> > > vote, causing intermittent register access failures.
> > >
> > > Add support for an optional interconnect path to the arm-smmu
> > > driver and vote for bandwidth while the SMMU is active. The path
> > > is acquired from DT if present and ignored otherwise.
> > >
> > > The bandwidth vote is enabled before accessing SMMU registers
> > > during probe and runtime resume, and released during runtime
> > > suspend and on error paths.
> > >
> > > Generally, from an architectural perspective, GEM_NOC and DDR are
> > > expected to have an active vote whenever the adreno_smmu block is
> > > powered on. In most common use cases, this requirement is implicitly
> > > satisfied because other GPU-related clients (for example, the GMU
> > > device) already hold a GEM_NOC vote when adreno_smmu is enabled.
> > >
> > > However, there are certain corner cases, such as during sleep/wakeup
> > > transitions, where the GEM_NOC vote can be removed before adreno_smmu
> > > is powered down. If adreno_smmu is then accessed while the interconnect
> > > vote is missing, it can lead to the observed failures. Because of the
> > > precise ordering involved, this scenario is difficult to reproduce
> > > consistently.
> > > (also GDSC is involved in adreno usecases can have an independent vote)
> > >
> > > Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
> > > ---
> > > drivers/iommu/arm/arm-smmu/arm-smmu.c | 57 +++++++++++++++++++++++++++++++++--
> > > drivers/iommu/arm/arm-smmu/arm-smmu.h | 2 ++
> > > 2 files changed, 57 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > > index 0bd21d206eb3e75c3b9fb1364cdc92e82c5aa499..07c7e44ec6a5bd1488f00f87d859a20495e46601 100644
> > > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > > @@ -53,6 +53,11 @@
> > > #define MSI_IOVA_BASE 0x8000000
> > > #define MSI_IOVA_LENGTH 0x100000
> > > +/* Interconnect bandwidth vote values for the SMMU register access path */
> > > +#define ARM_SMMU_ICC_AVG_BW 0
> > > +#define ARM_SMMU_ICC_PEAK_BW_HIGH 1000
> >
> > totally random numbers, which might be different for non-Qualcomm platform.
> >
> > > +#define ARM_SMMU_ICC_PEAK_BW_LOW 0
> > > +
> > > static int force_stage;
> > > module_param(force_stage, int, S_IRUGO);
> > > MODULE_PARM_DESC(force_stage,
> > > @@ -86,6 +91,36 @@ static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
> > > }
> > > }
> > > +static int arm_smmu_icc_get(struct arm_smmu_device *smmu)
> > > +{
> > > + smmu->icc_path = devm_of_icc_get(smmu->dev, NULL);
> >
> > Is there always only one bus / path in question?
> >
>
> <Apologies, missed to respond to this query>
> Yes for TCU, it needs to only have a vote on GEM_NOC interconnect
> while accessing the DDR in downstream path (client->TCU->DDR), which we are
> addressing here.
> Hence it's only one icc path in question here.
Again, you are describing Qualcomm platform, while the code part is
generic.
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v5 13/16] media: iris: Introduce buffer size calculations for AR50LT
From: sashiko-bot @ 2026-06-16 0:21 UTC (permalink / raw)
To: Dmitry Baryshkov; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260616-iris-ar50lt-v5-13-583b42770b6a@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The AR50LT decoder line buffer size calculations accept an `is_opb` parameter but completely ignore it, failing to allocate memory for the VPSS line buffer when OPB (split mode) is enabled.
- [High] The AR50LT BIN buffer size calculations for H.264 and HEVC ignore `num_vpp_pipes`, preventing proper buffer partitioning and alignment across hardware pipelines.
- [High] The VP9 AR50LT line buffer calculation omits critical component sizes for DMA metadata and QP maps.
- [High] Missing SEI user data allocation in the H.264 AR50LT persist buffer calculation.
--
commit 41b73900b6c3face3194c0a45272d5dc37396dab
Author: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
media: iris: Introduce buffer size calculations for AR50LT
Introduces AR50LT buffer size calculation for both encoder and
decoder. Reuse the buffer size calculation which are common, while
adding the AR50LT specific ones separately.
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_buffer.c b/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
> index 4a39b8fef52b0..ca03d65705136 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
> @@ -50,6 +50,32 @@ static u32 hfi_buffer_bin_h264d(u32 frame_width, u32 frame_height, u32 num_vpp_p
> return size_h264d_hw_bin_buffer(n_aligned_w, n_aligned_h, num_vpp_pipes);
> }
>
> +static u32 size_h264d_hw_bin_buffer_ar50lt(u32 frame_width, u32 frame_height, u32 num_vpp_pipes)
> +{
> + u32 size_yuv, size_bin_hdr, size_bin_res;
> +
> + size_yuv = ((frame_width * frame_height * 3) >> 1);
> + if (size_yuv <= 1920 * 1088 * 3 / 2) {
> + size_bin_hdr = size_yuv * H264_CABAC_HDR_RATIO_SM_TOT;
> + size_bin_res = size_yuv * H264_CABAC_RES_RATIO_SM_TOT;
> + } else {
> + size_bin_hdr = (size_yuv * 3) / 5;
> + size_bin_res = (size_yuv * 3) / 2;
> + }
> + size_bin_hdr = ALIGN(size_bin_hdr, DMA_ALIGNMENT);
> + size_bin_res = ALIGN(size_bin_res, DMA_ALIGNMENT);
> +
> + return size_bin_hdr + size_bin_res;
> +}
[Severity: High]
Will this cause incorrect buffer sizing when multiple VPP pipelines are active?
The num_vpp_pipes argument is accepted but doesn't seem to be used to
partition and align the buffer components like it is in the generic
size_h264d_hw_bin_buffer() calculation.
[ ... ]
> @@ -111,6 +152,32 @@ static u32 hfi_buffer_bin_h265d(u32 frame_width, u32 frame_height, u32 num_vpp_p
> return size_h265d_hw_bin_buffer(n_aligned_w, n_aligned_h, num_vpp_pipes);
> }
>
> +static u32 size_h265d_hw_bin_buffer_ar50lt(u32 frame_width, u32 frame_height, u32 num_vpp_pipes)
> +{
> + u32 size_yuv, size_bin_hdr, size_bin_res;
> +
> + size_yuv = ((frame_width * frame_height * 3) >> 1);
> + if (size_yuv <= ((BIN_BUFFER_THRESHOLD * 3) >> 1)) {
> + size_bin_hdr = size_yuv * H265_CABAC_HDR_RATIO_SM_TOT;
> + size_bin_res = size_yuv * H265_CABAC_RES_RATIO_SM_TOT;
> + } else {
> + size_bin_hdr = (size_yuv * 41) / 50;
> + size_bin_res = (size_yuv * 59) / 50;
> + }
> + size_bin_hdr = ALIGN(size_bin_hdr, DMA_ALIGNMENT);
> + size_bin_res = ALIGN(size_bin_res, DMA_ALIGNMENT);
> +
> + return size_bin_hdr + size_bin_res;
> +}
[Severity: High]
Similar to the H.264 function above, does this missing num_vpp_pipes
partitioning logic lead to internal misalignment for secondary pipes?
[ ... ]
> @@ -230,6 +341,11 @@ static u32 hfi_buffer_persist_h264d(void)
> DMA_ALIGNMENT);
> }
>
> +static u32 hfi_buffer_persist_h264d_ar50lt(void)
> +{
> + return ALIGN((SIZE_SLIST_BUF_H264 * NUM_SLIST_BUF_H264), DMA_ALIGNMENT);
> +}
[Severity: High]
Does this calculation inadvertently omit the SEI user data allocation?
The original hfi_buffer_persist_h264d() and the new AR50LT HEVC function both
include NUM_HW_PIC_BUF * SIZE_SEI_USERDATA, so leaving it out here might
cause the hardware to write out of bounds during SEI metadata extraction.
[ ... ]
> @@ -404,6 +584,29 @@ u32 hfi_buffer_line_h265d(u32 frame_width, u32 frame_height, bool is_opb, u32 nu
> return ALIGN((_size + vpss_lb_size), DMA_ALIGNMENT);
> }
>
> +static inline
> +u32 hfi_buffer_line_h265d_ar50lt(u32 frame_width, u32 frame_height, bool is_opb, u32 num_vpp_pipes)
> +{
> + u32 size;
> +
> + size = ALIGN(size_h265d_lb_fe_top_data_ar50lt(frame_width, frame_height), DMA_ALIGNMENT) +
> + ALIGN(size_h265d_lb_fe_top_ctrl(frame_width, frame_height), DMA_ALIGNMENT) +
> + ALIGN(size_h265d_lb_fe_left_ctrl(frame_width, frame_height),
> + DMA_ALIGNMENT) * num_vpp_pipes +
> + ALIGN(size_h265d_lb_se_left_ctrl_ar50lt(frame_width, frame_height),
> + DMA_ALIGNMENT) * num_vpp_pipes +
> + ALIGN(size_h265d_lb_se_top_ctrl(frame_width, frame_height), DMA_ALIGNMENT) +
> + ALIGN(size_h265d_lb_pe_top_data_ar50lt(frame_width, frame_height), DMA_ALIGNMENT) +
> + ALIGN(size_h265d_lb_vsp_top(frame_width, frame_height), DMA_ALIGNMENT) +
> + ALIGN(size_h265d_lb_vsp_left(frame_width, frame_height),
> + DMA_ALIGNMENT) * num_vpp_pipes +
> + ALIGN(size_h265d_lb_recon_dma_metadata_wr(frame_width, frame_height),
> + DMA_ALIGNMENT) * 4 +
> + ALIGN(size_h265d_qp(frame_width, frame_height), DMA_ALIGNMENT);
> +
> + return ALIGN(size, DMA_ALIGNMENT);
> +}
[Severity: High]
Is the is_opb parameter intentionally ignored here?
Without conditionally adding size_vpss_lb() when is_opb is true, could the
returned line buffer size be significantly undersized during split mode
operations, potentially leading to memory corruption?
[ ... ]
> @@ -492,6 +706,19 @@ u32 hfi_iris3_vp9d_lb_size(u32 frame_width, u32 frame_height, u32 num_vpp_pipes)
> ALIGN(size_vp9d_qp(frame_width, frame_height), DMA_ALIGNMENT);
> }
>
> +static inline
> +u32 hfi_ar50lt_vp9d_lb_size(u32 frame_width, u32 frame_height, u32 num_vpp_pipes)
> +{
> + return ALIGN(size_vpxd_lb_fe_left_ctrl(frame_width, frame_height), DMA_ALIGNMENT) *
> + num_vpp_pipes +
> + ALIGN(size_vpxd_lb_se_left_ctrl_ar50lt(frame_width, frame_height), DMA_ALIGNMENT) *
> + num_vpp_pipes +
> + ALIGN(size_vp9d_lb_vsp_top(frame_width, frame_height), DMA_ALIGNMENT) +
> + ALIGN(size_vpxd_lb_se_top_ctrl(frame_width, frame_height), DMA_ALIGNMENT) +
> + ALIGN(size_vp9d_lb_pe_top_data(frame_width, frame_height), DMA_ALIGNMENT) +
> + ALIGN(size_vp9d_lb_fe_top_data(frame_width, frame_height), DMA_ALIGNMENT);
> +}
[Severity: High]
Are we missing the DMA metadata, top control, and QP map allocations here?
The generic hfi_iris3_vp9d_lb_size() explicitly adds
size_vpxd_lb_fe_top_ctrl(), size_vpxd_lb_recon_dma_metadata_wr(), and
size_vp9d_qp(). Omitting these could result in an undersized buffer.
> +
> static inline
> u32 hfi_buffer_line_vp9d(u32 frame_width, u32 frame_height, u32 _yuv_bufcount_min, bool is_opb,
> u32 num_vpp_pipes)
> @@ -507,6 +734,13 @@ u32 hfi_buffer_line_vp9d(u32 frame_width, u32 frame_height, u32 _yuv_bufcount_mi
> return _lb_size + vpss_lb_size + 4096;
> }
>
> +static inline
> +u32 hfi_buffer_line_vp9d_ar50lt(u32 frame_width, u32 frame_height, u32 _yuv_bufcount_min,
> + bool is_opb, u32 num_vpp_pipes)
> +{
> + return hfi_ar50lt_vp9d_lb_size(frame_width, frame_height, num_vpp_pipes);
> +}
[Severity: High]
Does this function also need to check is_opb and add the VPSS line buffer
size, similarly to hfi_buffer_line_vp9d()?
[ ... ]
> @@ -529,6 +763,25 @@ static u32 hfi_buffer_line_h264d(u32 frame_width, u32 frame_height,
> return ALIGN((size + vpss_lb_size), DMA_ALIGNMENT);
> }
>
> +static u32 hfi_buffer_line_h264d_ar50lt(u32 frame_width, u32 frame_height,
> + bool is_opb, u32 num_vpp_pipes)
> +{
> + u32 size;
> +
> + size = ALIGN(size_h264d_lb_fe_top_data_ar50lt(frame_width), DMA_ALIGNMENT) +
> + ALIGN(size_h264d_lb_fe_top_ctrl_ar50lt(frame_width), DMA_ALIGNMENT) +
> + ALIGN(size_h264d_lb_fe_left_ctrl(frame_height), DMA_ALIGNMENT) * num_vpp_pipes +
> + ALIGN(size_h264d_lb_se_top_ctrl_ar50lt(frame_width), DMA_ALIGNMENT) +
> + ALIGN(size_h264d_lb_se_left_ctrl_ar50lt(frame_height), DMA_ALIGNMENT) *
> + num_vpp_pipes +
> + ALIGN(size_h264d_lb_pe_top_data_ar50lt(frame_width), DMA_ALIGNMENT) +
> + ALIGN(size_h264d_lb_vsp_top(frame_width), DMA_ALIGNMENT) +
> + ALIGN(size_h264d_lb_recon_dma_metadata_wr(frame_height), DMA_ALIGNMENT) * 2 +
> + ALIGN(size_h264d_qp(frame_width, frame_height), DMA_ALIGNMENT);
> +
> + return ALIGN(size, DMA_ALIGNMENT);
> +}
[Severity: High]
As with the HEVC and VP9 functions above, the is_opb argument is entirely
ignored. Could this lead to out-of-bounds DMA writes when split mode is
enabled?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-iris-ar50lt-v5-0-583b42770b6a@oss.qualcomm.com?part=13
^ permalink raw reply
* Re: [PATCH v2 2/6] iommu/arm-smmu: Add interconnect bandwidth voting support
From: Dmitry Baryshkov @ 2026-06-16 0:21 UTC (permalink / raw)
To: Bibek Kumar Patro
Cc: Will Deacon, Robin Murphy, Joerg Roedel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
linux-arm-kernel, iommu, devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <d25f24ca-5bb3-4276-ac8f-8340e8fb4ce8@oss.qualcomm.com>
On Mon, Jun 15, 2026 at 06:36:51PM +0530, Bibek Kumar Patro wrote:
>
>
> On 6/8/2026 7:25 PM, Dmitry Baryshkov wrote:
> > On Tue, May 26, 2026 at 08:12:03PM +0530, Bibek Kumar Patro wrote:
> > > On some SoCs the SMMU registers require an active interconnect
> > > bandwidth vote to be accessible. While other clients typically
> > > satisfy this requirement implicitly, certain corner cases (e.g.
> > > during sleep/wakeup transitions) can leave the SMMU without a
> > > vote, causing intermittent register access failures.
> > >
> > > Add support for an optional interconnect path to the arm-smmu
> > > driver and vote for bandwidth while the SMMU is active. The path
> > > is acquired from DT if present and ignored otherwise.
> > >
> > > The bandwidth vote is enabled before accessing SMMU registers
> > > during probe and runtime resume, and released during runtime
> > > suspend and on error paths.
> > >
> > > Generally, from an architectural perspective, GEM_NOC and DDR are
> > > expected to have an active vote whenever the adreno_smmu block is
> > > powered on. In most common use cases, this requirement is implicitly
> > > satisfied because other GPU-related clients (for example, the GMU
> > > device) already hold a GEM_NOC vote when adreno_smmu is enabled.
> > >
> > > However, there are certain corner cases, such as during sleep/wakeup
> > > transitions, where the GEM_NOC vote can be removed before adreno_smmu
> > > is powered down. If adreno_smmu is then accessed while the interconnect
> > > vote is missing, it can lead to the observed failures. Because of the
> > > precise ordering involved, this scenario is difficult to reproduce
> > > consistently.
> > > (also GDSC is involved in adreno usecases can have an independent vote)
> > >
> > > Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
> > > ---
> > > drivers/iommu/arm/arm-smmu/arm-smmu.c | 57 +++++++++++++++++++++++++++++++++--
> > > drivers/iommu/arm/arm-smmu/arm-smmu.h | 2 ++
> > > 2 files changed, 57 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > > index 0bd21d206eb3e75c3b9fb1364cdc92e82c5aa499..07c7e44ec6a5bd1488f00f87d859a20495e46601 100644
> > > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > > @@ -53,6 +53,11 @@
> > > #define MSI_IOVA_BASE 0x8000000
> > > #define MSI_IOVA_LENGTH 0x100000
> > > +/* Interconnect bandwidth vote values for the SMMU register access path */
> > > +#define ARM_SMMU_ICC_AVG_BW 0
> > > +#define ARM_SMMU_ICC_PEAK_BW_HIGH 1000
> >
> > totally random numbers, which might be different for non-Qualcomm platform.
> >
>
> Ideally, any non-zero value would be enough to keep the path active.
This is true for Qualcomm devices. However, you are adding this to a
generic code.
> Here 1 Would be enough to keep the path active, but might be too small to
> reliably keep the bus active.
> Other is UINT_MAX, which will reliably keep the bus active but might cause a
> power penalty.
>
> #define ARM_SMMU_ICC_PEAK_BW_HIGH UINT_MAX
>
> seems to be suitable here to reliably keep the bus active by BCM
> for both Qualcomm and non-Qualcomm platforms (with some power penalty).
>
> LMK, if you feel otherwise.
Shift it to the qcom instance or provide platform-specific values? (My
preference would be towards the first solution).
>
>
> > > +#define ARM_SMMU_ICC_PEAK_BW_LOW 0
> > > +
> > > static int force_stage;
> > > module_param(force_stage, int, S_IRUGO);
> > > MODULE_PARM_DESC(force_stage,
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v5 10/16] media: iris: add minimal GET_PROPERTY implementation
From: sashiko-bot @ 2026-06-16 0:20 UTC (permalink / raw)
To: Dmitry Baryshkov; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260616-iris-ar50lt-v5-10-583b42770b6a@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Use-After-Free due to missing reference counting in instance lookup.
- [High] Unsafe release of instance lock during firmware wait allows concurrent IOCTLs to clobber completion state.
--
commit d766418e0dabcbcbb8dc9a3f16b8a80a51ec61f4
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
media: iris: add minimal GET_PROPERTY implementation
This commit adds minimal support for querying properties from the firmware
to satisfy Gen1 firmware requirements for AR50Lt. It implements the
session_get_property operation and handles the resulting property info
messages.
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> index 7674b47ad6c49..99e82e5510abe 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> @@ -1117,10 +1117,31 @@ static int iris_hfi_gen1_session_set_config_params(struct iris_inst *inst, u32 p
> return 0;
> }
>
> +static int iris_hfi_gen1_session_get_property(struct iris_inst *inst, u32 packet_type)
> +{
> + struct hfi_session_get_property_pkt pkt;
> + int ret;
> +
> + pkt.shdr.hdr.size = sizeof(pkt);
> + pkt.shdr.hdr.pkt_type = HFI_CMD_SESSION_GET_PROPERTY;
> + pkt.shdr.session_id = inst->session_id;
> + pkt.num_properties = 1;
> + pkt.data = packet_type;
> +
> + reinit_completion(&inst->completion);
> +
> + ret = iris_hfi_queue_cmd_write(inst->core, &pkt, pkt.shdr.hdr.size);
> + if (ret)
> + return ret;
> +
> + return iris_wait_for_session_response(inst, false);
> +}
[Severity: High]
This is a pre-existing issue, but the newly added function utilizes a pattern
that might be unsafe.
In iris_wait_for_session_response(), inst->lock is explicitly dropped while
waiting for inst->completion. Because the driver does not appear to use
vdev->lock to serialize V4L2 ioctls, dropping inst->lock could expose the
driver to concurrent ioctls.
Could a second thread acquire inst->lock, issue another firmware command,
and call reinit_completion(&inst->completion), erasing the state the first
thread is waiting on?
If so, when the firmware finally responds, complete(&inst->completion) might
arbitrarily wake up the wrong thread and desynchronize the driver state.
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> index bfd7495bf44f0..23fc7194b1e3a 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
[ ... ]
> @@ -652,6 +656,8 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response
> iris_hfi_gen1_session_etb_done(inst, hdr);
> } else if (hdr->pkt_type == HFI_MSG_SESSION_FILL_BUFFER) {
> iris_hfi_gen1_session_ftb_done(inst, hdr);
> + } else if (hdr->pkt_type == HFI_MSG_SESSION_PROPERTY_INFO) {
> + complete(&inst->completion);
[Severity: Critical]
This isn't a bug introduced by this patch, but there appears to be a
critical structural flaw with how the instance is handled here.
Earlier in iris_hfi_gen1_handle_response(), iris_get_instance() looks up an
iris_inst from the core->instances list while holding core->lock, but returns
the raw pointer without incrementing any reference count, and then drops
core->lock.
The response handler then subsequently attempts to acquire
mutex_lock(&inst->lock). If a concurrent thread executing iris_close()
acquires core->lock via iris_remove_session(), removes the instance from
the list, and then calls kfree(inst), could the IRQ handler end up
executing a locking operation on freed memory?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-iris-ar50lt-v5-0-583b42770b6a@oss.qualcomm.com?part=10
^ permalink raw reply
* Re: [PATCH v5 11/16] media: iris: update buffer requirements based on received info
From: sashiko-bot @ 2026-06-16 0:20 UTC (permalink / raw)
To: Dmitry Baryshkov; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260616-iris-ar50lt-v5-11-583b42770b6a@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent handling of HFI response packets and instance teardown can result in a Use-After-Free (UAF) of `inst`.
--
commit 39194b5e5ad26925b8c03d02889401ec6e1fa21d
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
media: iris: update buffer requirements based on received info
Upon receiving data for HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS the
driver should update buffer sizes and counts from the received data.
This will be used for upcoming support of AR50Lt platforms.
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> index 23fc7194b1e3a..896953ea62ea4 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
[ ... ]
> @@ -657,7 +730,7 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response
> } else if (hdr->pkt_type == HFI_MSG_SESSION_FILL_BUFFER) {
> iris_hfi_gen1_session_ftb_done(inst, hdr);
> } else if (hdr->pkt_type == HFI_MSG_SESSION_PROPERTY_INFO) {
> - complete(&inst->completion);
> + iris_hfi_gen1_session_property_info(inst, hdr);
> } else {
> struct hfi_msg_session_hdr_pkt *shdr;
[Severity: High]
This is a pre-existing issue, but can concurrent handling of HFI response
packets and instance teardown result in a use-after-free of the inst pointer?
Looking at the start of iris_hfi_gen1_handle_response(), it calls
iris_get_instance() which retrieves the inst pointer from the core list
without incrementing any reference count:
drivers/media/platform/qcom/iris/iris_utils.c:iris_get_instance() {
list_for_each_entry(inst, &core->instances, list) {
if (inst->session_id == session_id) {
mutex_unlock(&core->lock);
return inst;
}
}
}
The asynchronous response handler then blocks trying to acquire
mutex_lock(&inst->lock).
Concurrently, if iris_close() executes and calls iris_session_close(), it
can temporarily unlock inst->lock while waiting for the final
HFI_MSG_SYS_SESSION_END response. This allows iris_close() to eventually
finish, remove the session, unlock and destroy the mutex, and free the
memory:
drivers/media/platform/qcom/iris/iris_vidc.c:iris_close() {
iris_remove_session(inst);
mutex_unlock(&inst->lock);
mutex_destroy(&inst->ctx_q_lock);
mutex_destroy(&inst->lock);
...
kfree(inst);
}
If a spurious or trailing firmware message is processed for the same session,
the response handler could wake up when the lock is released by iris_close(),
successfully acquire the destroyed lock, and dereference the freed inst memory.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-iris-ar50lt-v5-0-583b42770b6a@oss.qualcomm.com?part=11
^ permalink raw reply
* Re: [PATCH v2 3/6] arm64: dts: qcom: kodiak: Add GEM_NOC interconnect for adreno SMMU
From: Dmitry Baryshkov @ 2026-06-16 0:19 UTC (permalink / raw)
To: Bibek Kumar Patro
Cc: Will Deacon, Robin Murphy, Joerg Roedel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
linux-arm-kernel, iommu, devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <26d51cbb-2d87-4564-b3c6-cc61ab900e19@oss.qualcomm.com>
On Mon, Jun 08, 2026 at 08:07:07PM +0530, Bibek Kumar Patro wrote:
>
>
> On 6/8/2026 7:27 PM, Dmitry Baryshkov wrote:
> > On Tue, May 26, 2026 at 08:12:04PM +0530, Bibek Kumar Patro wrote:
> > > On Kodiak platforms, the Adreno SMMU requires a bandwidth vote on
> > > the GEM_NOC path (MASTER_GPU_TCU -> SLAVE_EBI1) before its registers
> > > are accessible. Without this vote, the SMMU may become unreachable,
> > > leading to intermittent probe failures and runtime issues.
> > >
> > > Add the required interconnect to ensure reliable register access.
> >
> > Does it only concern the GPU SMMU? What about the APPS SMMU? Should it
> > be voting on other interconnects too? I guess so, because currently I
> > see that TBUs vote for various interconnects. BTW: should apps_smmu also
> > vote on the power domains?
> >
>
> This race mainly occurs in GPU SMMU, where the GDSC can have an
> independent vote on the Adreno SMMU. However, the GEM_NOC vote may
> already have been removed by the GPU (or any consumer of adreno_smmu,
> e.g gmu), unless it is explicitly voted by the GPU SMMU (which acts as a
> supplier for the GPU). This mismatch can lead to SHUB timeouts or NoC
> errors.
>
> Mostly this race reported in suspend/resume cycle (when gpu/gmu devices
> moves to slumber/suspend state before adreno_smmu powers down
> and the later doesn't have explicit interconnect voting).
>
> In the case of APPS SMMU, such a race is not expected for any known
> use case. APPS SMMU is part of a shared infrastructure block, and its
> power is typically kept enabled as long as attached master devices are
> active. Therefore, explicit power-domain voting from APPS SMMU may not
> be required.
This looks like a good part of the commit message. Please add it where
it belongs.
>
> Thanks,
> Bibek
>
>
> > >
> > > Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
> > > ---
> > > arch/arm64/boot/dts/qcom/kodiak.dtsi | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
> > > index fa540d8c2615dc02d941eb16bc7253204c2750bd..eefa4b836a81374ff437ab4bbcbc3fecc1590ab6 100644
> > > --- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
> > > @@ -3386,6 +3386,8 @@ adreno_smmu: iommu@3da0000 {
> > > power-domains = <&gpucc GPU_CC_CX_GDSC>;
> > > dma-coherent;
> > > + interconnects = <&gem_noc MASTER_GPU_TCU QCOM_ICC_TAG_ALWAYS
> > > + &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
> > > };
> > > gfx_0_tbu: tbu@3dd9000 {
> > >
> > > --
> > > 2.34.1
> > >
> >
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 0/4] iio: adc: new ti-ads112c14 driver
From: Kurt Borja @ 2026-06-16 0:18 UTC (permalink / raw)
To: David Lechner (TI), Jonathan Cameron, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Kurt Borja, Nguyen Minh Tien, linux-iio, devicetree, linux-kernel
In-Reply-To: <20260615-iio-adc-ti-ads122c14-v1-0-e6bdadf7cb2b@baylibre.com>
Hi David,
On Mon Jun 15, 2026 at 4:59 PM -05, David Lechner (TI) wrote:
> This adds support for TI ADS112C14 and ADS122C14 ADC chips.
>
> The closest thing we've seen to this in the kernel already is ads124s08.
> However, that has a completely different register map and the DT
> bindings are incomplete and the driver is extremely basic. So I've just
> started from scratch here.
>
> We've also had a similar submission recently for ADS1220 [1]. That chip
> is in a similar situation to ads124s08 in that it has a different
> register map (but the submitted DT bindings are better than the ones for
> ads124s08, even if still a bit incomplete). And literally as I was
> writing the previous sentence, another series [2] was sent for yet
> another similar family of chips (ADS1262). That one is even more complex
> in the feature set than the ones I am working on. I was going to polish
> up the driver a bit more before submitting it, but now it seems more
> urgent to coordinate with the other two series to align on how we would
> like to handle all of these.
>
> [1]: https://lore.kernel.org/linux-iio/20260610151342.44274-1-zizuzacker@gmail.com/
> [2]: https://lore.kernel.org/linux-iio/20260612-ads126x-v1-0-894c788d03ed@gmail.com/
>
> All of these chips have in common that they are designed for use with
> RTDs and thermocouples and so they look very similar to each other in
> terms of wiring and feature set, even if the register maps are
> different. They are in the gray area where we could either keep them
> separate because they are just different enough, or we could do like
> we've done before with ad_sigma_delta and have a bit of an abstraction
> layer for the register differences and otherwise try to share as much
> code as possible. Normally, I would lean towards keeping them separate,
> but in this case, I'm considering trying to share code because the
> devicetree bindings for the inputs is complex and is going to be mostly
> the same across all of these chips.
The channel configuration is indeed very similar for the three chips.
All three have IDAC, BOC and VREF configurations.
>
> If we decide to go the route of sharing code, we could still merge this
> series as-is and then do the refactoring to add the abstraction layer in
> a follow-up series that also adds support for the first of the other
> chips.
Do you have a proposal of how such an abstraction would look like? I do
like the idea of abstracting the firmware parsing, scales and shared
calculations.
>
> This series includes just basic support for reading single measurements
> from the ADC and gain selection via the scale attribute. I plan to
> follow this up with additional series to add support for buffered reads,
> filtering/oversampling configuration, event support, gpio controller
> support and perhaps a few other things that are slipping my mind right
> now.
>
> The most interesting part about this (that I alluded to above) is the
> way channels are handled. These are multipling ADCs with differential
> and single-ended inputs. But what sets them apart from other similar
> chips is that since they are designed for use with RTDs, there can also
> be a current output required to excite the RTD and this current output
> might be different for different channels. So the way I conceptualized
> the channels is that the devicetree specifies the conditions needed
> to take a particular measurement rather than being purely a physical
> channel.
>
> This makes things more flexible, but does make the driver a bit more
> complex. For example, knowing when the current output needs to be
> enabled or disabled. For now, I have chosen a lazy-enable where they
> are not turned on until the first measurement is taken that requires
> them, but then they stay on until another measurement is taken that
> doesn't require them. This can lead to some oddness with the diagnostic
> channels that may be measuring something that indirectly requires the
> current output (i.e. the external reference voltage when it is connected
> to a resistor rather than a power supply). This means you need to take
> a measurement that requires the current output to be enabled before the
> diagnostic channels will give accurate readings.
This is the same approach I took around the BOC, it feels kinda hacky
but it makes sense. Just an idea I thought about just now: What if we
have an additional write-only "_enable" sysfs attribute for these
channels?
The approach I took for the IDAC was to have a single configuration that
it's enabled for all channels. This makes some sense in my device when
thinking about optimal software sequencial reads, because of the
register layout, but I also see the value in having per-channel IDAC
configuration. I think I will take your approach, so we have the same
channel configuration around this.
Have you thought about how to implement the BOC? In the ADS1262 the
feature can be found "Sensor Bias". What I did was add per channel DT
properties for this too.
Another question. When you implement power management in the future,
will you enable autosuspend? IDAC currents will be lost if autosuspend
is enabled. Is this acceptable? In my case I did enable autosuspend, but
I have some doubts about this.
>
> I have also pushed a branch to [3] that contains the start of some
> documentation for this driver that can give some more insight into how
> the implementation works. It still needs some work and also documents
> some things that haven't been implemented yet, so I haven't included it
> in this series yet.
>
> [3]: https://github.com/dlech/linux/blob/b4/iio-adc-ti-ads122c14/Documentation/iio/ads112c14.rst
>
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
> David Lechner (TI) (4):
> dt-bindings: iio: adc: add ti,ads122c14
> iio: adc: add ti-ads112c14 driver
> iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel
> iio: adc: ti-ads112c14: add measurement channel support
>
> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++
> MAINTAINERS | 8 +
> drivers/iio/adc/Kconfig | 12 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ti-ads112c14.c | 1053 ++++++++++++++++++++
> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
> 6 files changed, 1309 insertions(+)
> ---
> base-commit: ec039126b7fac4e3af35ebccaa7c6f9b6875ba81
> change-id: 20260514-iio-adc-ti-ads122c14-d0b92479334e
>
> Best regards,
> --
> David Lechner (TI) <dlechner@baylibre.com>
--
Thanks,
~ Kurt
^ permalink raw reply
* Re: [PATCH v2 1/6] dt-bindings: iommu: arm,smmu: Document interconnects property
From: Dmitry Baryshkov @ 2026-06-16 0:18 UTC (permalink / raw)
To: Bibek Kumar Patro
Cc: Konrad Dybcio, Will Deacon, Robin Murphy, Joerg Roedel,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, linux-arm-kernel, iommu, devicetree, linux-kernel,
linux-arm-msm
In-Reply-To: <a1db573b-bcb4-44a5-89b6-6d1c76f4a18a@oss.qualcomm.com>
On Mon, Jun 08, 2026 at 07:32:45PM +0530, Bibek Kumar Patro wrote:
>
>
> On 6/8/2026 7:19 PM, Dmitry Baryshkov wrote:
> > On Mon, Jun 08, 2026 at 07:14:43PM +0530, Bibek Kumar Patro wrote:
> > >
> > >
> > > On 6/8/2026 3:22 PM, Konrad Dybcio wrote:
> > > > On 5/26/26 4:42 PM, Bibek Kumar Patro wrote:
> > > > > Some SoC implementations require a bandwidth vote on an interconnect
> > > > > path before the SMMU register space is accessible. Add the optional
> > > > > 'interconnects' property to the binding to allow platform DT nodes
> > > > > to describe this path.
> > > > >
> > > > > Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
> > > > > ---
> > > > > .../devicetree/bindings/iommu/arm,smmu.yaml | 27 ++++++++++++++++++++++
> > > > > 1 file changed, 27 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > > > index 06fb5c8e7547cb7a92823adc2772b94f747376a6..3a677ff1a18fcdf5c0ca9ec8a017d41f9eb5ff09 100644
> > > > > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > > > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > > > @@ -243,6 +243,13 @@ properties:
> > > > > minItems: 1
> > > > > maxItems: 3
> > > > > + interconnects:
> > > > > + maxItems: 1
> > > > > + description:
> > > > > + Interconnect path to the SMMU register space. Required on SoCs
> > > > > + where the SMMU registers are only accessible after a bandwidth
> > > > > + vote has been placed on the interconnect fabric.
> > > > > +
> > > > > nvidia,memory-controller:
> > > > > description: |
> > > > > A phandle to the memory controller on NVIDIA Tegra186 and later SoCs.
> > > > > @@ -602,6 +609,26 @@ allOf:
> > > > > clock-names: false
> > > > > clocks: false
> > > > > + - if:
> > > > > + properties:
> > > > > + compatible:
> > > > > + items:
> > > > > + - enum:
> > > > > + - qcom,qcs615-smmu-500
> > > > > + - qcom,qcs8300-smmu-500
> > > > > + - qcom,sa8775p-smmu-500
> > > > > + - qcom,sc7280-smmu-500
> > > >
> > > > This is a list of targets that happen to be supported by QLI.. but should
> > > > this list not contain _all_ Qualcomm SoCs, or at least a much broader range?
> > > >
> > > > Perhaps
> > > >
> > > > if: properties: compatible: contains: qcom,adreno-smmu
> > > >
> > > > ?
> > > >
> > >
> > > As of now platforms where the issues [1] getting reported are added, the
> > > list will grow.
> > > <We still have to evaluate and test on other non-QLI platforms hosted in
> > > upstream [2]>
> >
> > Do you really need to test, which platforms have an interconnect, or can
> > you predict it by checking the SoC documentation? I strongly belive, the
> > latter is the case.
> >
>
> Agree, for interconnect path we can surely predict from the SoC
> documentation,
> But for the corresponding FLAGS/values (MASTER_GPU_TCU, SLAVE_EBI1), it
> would need some testing before finalizing the change on corresponding
> platforms.
I'd assume that corresponding values should already be known. The
platforms are not new.
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v5 02/16] media: iris: Filter UBWC raw formats based on hardware capabilities
From: sashiko-bot @ 2026-06-16 0:17 UTC (permalink / raw)
To: Dmitry Baryshkov; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260616-iris-ar50lt-v5-2-583b42770b6a@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `iris_vdec_formats_noubwc_cap` array contains a zero-initialized gap at index 1, which prematurely terminates V4L2 format enumeration and improperly validates 0 as a valid pixel format.
--
commit 1ff6b6e464175bc1ccf6878e0aa89ffec126608b
Author: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
media: iris: Filter UBWC raw formats based on hardware capabilities
The raw formats supported by Iris were previously advertised
unconditionally, assuming UBWC support on all platforms. Use the UBWC
configuration provided by the platform to dynamically filter raw formats
at runtime, keeping linear formats available when UBWC is not supported.
> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
> index 9e228b70420e4..63b7c9dec60a0 100644
> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
[ ... ]
> @@ -69,8 +70,14 @@ static const u32 iris_vdec_formats_cap[] = {
> [IRIS_FMT_QC10C] = V4L2_PIX_FMT_QC10C,
> };
>
> +static const u32 iris_vdec_formats_noubwc_cap[] = {
> + [IRIS_FMT_NV12] = V4L2_PIX_FMT_NV12,
> + [IRIS_FMT_TP10] = V4L2_PIX_FMT_P010,
> +};
[Severity: High]
Does this create a sparse array with a zero-initialized gap at index 1?
Because IRIS_FMT_NV12 is 0 and IRIS_FMT_TP10 is 2, skipping index 1
(IRIS_FMT_QC08C) implicitly initializes it to 0.
When check_format() in iris_vdec.c iterates through this array:
for (i = 0; i < size; i++) {
if (fmt[i] == pixfmt)
break;
}
Won't it incorrectly accept 0 as a valid pixel format by matching against the
uninitialized gap?
[ ... ]
> @@ -110,6 +122,7 @@ static bool check_format(struct iris_inst *inst, u32 pixfmt, u32 type)
>
> static u32 find_format_by_index(struct iris_inst *inst, u32 index, u32 type)
> {
> + const struct qcom_ubwc_cfg_data *ubwc = inst->core->ubwc_cfg;
> unsigned int size;
> const u32 *fmt;
>
> @@ -119,8 +132,13 @@ static u32 find_format_by_index(struct iris_inst *inst, u32 index, u32 type)
> size = inst->core->iris_platform_data->inst_iris_fmts_size;
> break;
> case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> - fmt = iris_vdec_formats_cap;
> - size = ARRAY_SIZE(iris_vdec_formats_cap);
> + if (ubwc->ubwc_enc_version) {
> + fmt = iris_vdec_formats_cap;
> + size = ARRAY_SIZE(iris_vdec_formats_cap);
> + } else {
> + fmt = iris_vdec_formats_noubwc_cap;
> + size = ARRAY_SIZE(iris_vdec_formats_noubwc_cap);
> + }
> break;
> default:
> return 0;
[Severity: High]
Will the V4L2 enumeration stop prematurely when encountering the gap?
Since the V4L2 framework queries formats using a sequential index, querying
index 1 will hit the zero-initialized hole, causing find_format_by_index()
in iris_vdec.c to return 0.
When 0 is returned, V4L2 typically stops enumeration, meaning the valid 10-bit
linear format (V4L2_PIX_FMT_P010) at index 2 will be completely hidden.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-iris-ar50lt-v5-0-583b42770b6a@oss.qualcomm.com?part=2
^ 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