* Re: [PATCH v2 3/5] PCI: dwc: rcar-gen4: Split .start_link into ltssm_control and speed_control
From: sashiko-bot @ 2026-07-20 2:10 UTC (permalink / raw)
To: Marek Vasut; +Cc: devicetree, linux-pci, conor+dt, wsa+renesas, robh
In-Reply-To: <20260720020505.315335-4-marek.vasut+renesas@mailbox.org>
> Introduce new .speed_control callback and implement it on R-Car Gen4.
> The callback implements the second half of what is currently present
> in rcar_gen4_pcie_start_link(), and rcar_gen4_pcie_start_link() does
> call the .speed_control callback. This is a preparatory patch for
> R-Car Gen5 support, where the .speed_control implementation is
> different.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720020505.315335-1-marek.vasut+renesas@mailbox.org?part=3
^ permalink raw reply
* Re: [PATCH v8 3/3] thermal/drivers/imx: Add calibration offset support
From: CHENG Haoning (BCSC/ENG1) @ 2026-07-20 2:10 UTC (permalink / raw)
To: Frieder Schrempf
Cc: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <PAWPR10MB741520A95E4822E935A0B7C0C2C62@PAWPR10MB7415.EURPRD10.PROD.OUTLOOK.COM>
On 2026-07-17 06:23:44+00:00, CHENG Haoning (BCSC/ENG1) wrote:
> On Wed, Jul 15, 2026 at 11:06:07AM +0200, Frieder Schrempf wrote:
>
> > On 14.07.26 12:28, Haoning CHENG via B4 Relay wrote:
> >
> > Sorry to chime in so late. I just want to understand what this
> > calibration offset is about. Why would there be a need of a
> > board-specific offset? The sensor is in the SoC and if you add a
> > board-specific offset, you no longer measure the SoC core temperature,
> > right?
> >
> > How would you determine the offset in the first place? How would I know
> > what fsl,temp-calibration-offset-millicelsius should be set to? I could
> > put a sensor on the SoC case and use the delta as offset, but then I
> > would just account for the thermal resistance of the casing and not
> > measure the SoC core temperature anymore, right?
>
> Hi Frieder,
>
> Thanks for pointing this out. Your understanding is correct: after
> applying the offset, the reported value no longer represents the raw
> SoC die or junction temperature.
>
> For this board, the required "SoC temperature" is the package-surface
> temperature. The offset was derived by comparing the TEMPMON reading
> against a calibrated external sensor placed near the SoC package
> surface under steady-state thermal conditions, and is used to
> approximate the package-surface temperature from the internal TEMPMON
> reading.
>
> The Linux thermal framework does not require a thermal zone to use a
> specific temperature reference. It only requires the reported
> temperature and trip points to use the same temperature domain.
>
> In this driver, the offset is added in get_temp() and subtracted in
> set_alarm() and set_panic() when programming the hardware thresholds.
> This keeps the reported temperature and trip points in the same
> package-surface temperature domain. Doing this in the driver is
> necessary because the hardware alarm thresholds must also be offset-
> corrected; applying the offset only in userspace would cause the
> TEMPMON IRQ to fire at the wrong die temperature.
>
> I agree that "calibration offset" is misleading, since this does not
> calibrate TEMPMON to a more accurate junction temperature. In the next
> revision, the commit messages and DT binding have been updated to
> describe this as a board-specific conversion offset from the internal
> sensor reading to an estimated package-surface temperature.
>
> Thanks,
> Haoning
Hi Frieder,
I need to correct my previous reply. After further discussion with our hardware team, the offset does NOT convert die temperature to package-surface temperature.
Their thermal characterization shows that the raw TEMPMON sensor reading itself deviates from the theoretical junction temperature - this deviation exists even at the die level, before any board or package influence. The offset corrects the sensor reading toward the actual junction temperature, so it is a genuine sensor calibration.
Board-level effects may contribute additionally, but they are not the primary reason for this feature. I apologize for the confusion - my earlier description was inaccurate. The v10 series reflects this corrected understanding.
Thanks,
Haoning
^ permalink raw reply
* Re: [PATCH v2 3/5] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support
From: Guenter Roeck @ 2026-07-20 2:10 UTC (permalink / raw)
To: Torreno, Alexis Czezar, sashiko-reviews@lists.linux.dev
Cc: linux-hwmon@vger.kernel.org, conor+dt@kernel.org,
devicetree@vger.kernel.org, robh@kernel.org
In-Reply-To: <PH0PR03MB6351F16AE2D15684260D2930F1C32@PH0PR03MB6351.namprd03.prod.outlook.com>
On 7/19/26 18:58, Torreno, Alexis Czezar wrote:
>>> +
>>> +/*
>>> + * Driver currently only supports READ_VOUT for monitoring that needs
>>> +scaling to
>>> + * convert from feedback voltage to actual output voltage.
>>> + *
>>> + * vout_actual = vout_fb × (1 + RFB1/RFB2)
>>> + *
>>> + * VOUT_COMMAND and VOUT_MAX are currently not in use, but work with
>>> +feedback
>>> + * voltage. If regulator support is added, VOUT_COMMAND values would
>>> +need
>>> + * to be adjusted accordingly.
>>> + */
>>> +static int max20830_read_word_data(struct i2c_client *client, int page,
>>> + int phase, int reg)
>>> +{
>>> + const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
>>> + const struct max20830_data *data = container_of(info, struct
>> max20830_data, info);
>>> + int ret;
>>> +
>>> + switch (reg) {
>>> + case PMBUS_READ_VOUT:
>>> + ret = pmbus_read_word_data(client, page, phase, reg);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + /* Apply voltage divider scaling if resistors are non-zero */
>>> + if (data->vout_rfb1 && data->vout_rfb2)
>>> + ret = DIV_ROUND_CLOSEST_ULL((u64)ret * (data-
>>> vout_rfb1 +
>>> + data->vout_rfb2), data-
>>> vout_rfb2);
>>> + return ret;
>>
>> [Severity: High]
>> Does this returned value need bounds checking?
>>
>> If a large resistor divider is used, the scaled PMBus word calculated here can
>> exceed 0xFFFF. The generic PMBus core function pmbus_reg2data_linear() will
>> cast this returned value to a u16, silently wrapping the value modulo
>> 65536 and resulting in wildly incorrect telemetry (e.g., reporting 2V instead of
>> 12V).
>>
>> Should this be clamped using clamp_val(ret, 0, 0xffff)?
>>
>
> Good Call, will clamp this as suggested.
>
>>> + default:
>>> + return -ENODATA;
>>> + }
>>> +}
>>
>> [Severity: High]
>> Will returning -ENODATA bypass software scaling for VOUT limit attributes?
>>
>> The driver sets PMBUS_HAVE_VOUT below, which exposes hardware VOUT
>> limits (like inX_max or inX_lcrit). Because only PMBUS_READ_VOUT is
>> intercepted here, the core will read those limits unscaled from the hardware.
>>
>
> Not applicable - no limit registers exist or are exposed.
>
>>> +
>>> static struct pmbus_driver_info max20830_info = {
>>> .pages = 1,
>>> .format[PSC_VOLTAGE_IN] = linear,
>>> @@ -24,13 +64,25 @@ static struct pmbus_driver_info max20830_info = {
>>> PMBUS_HAVE_TEMP |
>>> PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT |
>>> PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,
>>> + .read_word_data = max20830_read_word_data,
>>
>> [Severity: High]
>> Does this struct also need a .write_word_data callback?
>>
>> Since PMBUS_HAVE_VOUT is enabled, user writes to limit attributes will fall
>> through to the PMBus core, which writes the scaled PMBus word directly into
>> the hardware's limit register. Because the hardware expects limits in the
>> unscaled feedback voltage range, the written value will be completely out of
>> bounds.
>>
>
> Not applicable. No writable VOUT registers are exposed. MAX20830 has no
> limit registers to write to. also regulator support is not implemented.
>
Please add a comment to the code explaining this (if you dod not do that already),
to remind patch authors that there is additional work to do if regulator support
is added.
Thanks,
Guenter
>> --
>> Sashiko AI review ·
>> https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260706-dev-
>> max20830c-v2-0-
>> 37761e89bb5f@analog.com?part=3__;Iw!!A3Ni8CS0y2Y!9BBLoE4ditcREIvsDD7
>> _VDDKjsS7XgrV0udXIgvBZeCRChzDWWsu7TY8ybr5VBPG4VQlLnDS6EQmu6fhfLx
>> CpKUl28ZSE04$
^ permalink raw reply
* [PATCH v2 5/5] PCI: dwc: rcar-gen4: Add support for R-Car X5H PCIe4
From: Marek Vasut @ 2026-07-20 2:02 UTC (permalink / raw)
To: linux-pci
Cc: Marek Vasut, Krzysztof Wilczyński, Bjorn Helgaas,
Conor Dooley, Geert Uytterhoeven, Krzysztof Kozlowski,
Lorenzo Pieralisi, Manivannan Sadhasivam, Rob Herring,
Yoshihiro Shimoda, devicetree, linux-kernel, linux-renesas-soc
In-Reply-To: <20260720020505.315335-1-marek.vasut+renesas@mailbox.org>
Add support for R8A78000 (R-Car X5H) PCIe4.
This driver previously supported R-Car Gen4 S4/V4H/V4M. PCIe features
of R-Car X5H PCIe4 are almost all the same.
The controller initialization sequence is slightly different and is
factored out into controller specific callbacks, in a manner similar
to previous R-Car Gen4 handling.
The controller does have a PHY attached to it, but the PHY is operated
by a separate PHY driver, the PHY driver instance binding is handled
in rcar_gen4_pcie_get_resources() and controlled in the aforementioned
controller specific callbacks.
The controller driver is deliberately using "renesas,rcar-gen5-pcie4"
DT compatible string to discern R-Car X5H PCIe4 controller supported
by this driver, from R-Car X5H PCIe6 controller which will most likely
use a separate driver.
The R-Car X5H PCIe4 controller embeds HDMA instead of EDMA embedded
in the R-Car Gen4 PCIe controller, "dw-edma" driver supports both
DMA variants.
Endpoint mode is currently not implemented for R-Car Gen5 PCIe4.
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V2: No change
---
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 115 +++++++++++++++++++-
1 file changed, 113 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 2ee8b344ec541..f403e11c884a4 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -19,6 +19,7 @@
#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/pci.h>
+#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/reset.h>
@@ -36,6 +37,7 @@
/* MSI Capability */
#define MSICAP0 0x0050
+#define MSICAP0_MMESCAP_MASK GENMASK(19, 17)
#define MSICAP0_MSIE BIT(16)
/* PCIe Interrupt Status 0 */
@@ -74,6 +76,11 @@
#define PCIEPWRMNGCTRL 0x0070
#define APP_CLK_REQ_N BIT(11)
#define APP_CLK_PM_EN BIT(10)
+#define APP_READY_ENTR_L23 BIT(6)
+#define APP_REQ_ENTR_L1 BIT(5)
+
+/* PCI Express capability */
+#define EXPCAP(x) (0x0070 + (x))
#define RCAR_NUM_SPEED_CHANGE_RETRIES 10
#define RCAR_MAX_LINK_SPEED 4
@@ -97,6 +104,7 @@ struct rcar_gen4_pcie {
struct dw_pcie dw;
void __iomem *base;
void __iomem *phy_base;
+ struct phy *phy;
struct platform_device *pdev;
struct reset_control *perst;
const struct rcar_gen4_pcie_drvdata *drvdata;
@@ -169,6 +177,35 @@ static int rcar_gen4_pcie_speed_control(struct rcar_gen4_pcie *rcar)
return 0;
}
+static int rcar_gen5_pcie_speed_control(struct rcar_gen4_pcie *rcar)
+{
+ struct dw_pcie *dw = &rcar->dw;
+ u32 lnkcap = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
+ u32 lnksta = dw_pcie_readw_dbi(dw, EXPCAP(PCI_EXP_LNKSTA));
+ u32 val, retries;
+
+ if ((lnksta & PCI_EXP_LNKSTA_CLS) == (lnkcap & PCI_EXP_LNKCAP_SLS))
+ return 0;
+
+ /* Retrain link */
+ val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCTL));
+ val |= PCI_EXP_LNKCTL_RL;
+ dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCTL), val);
+
+ /* Wait for link retrain */
+ for (retries = 0; retries <= 10; retries++) {
+ lnksta = dw_pcie_readw_dbi(dw, EXPCAP(PCI_EXP_LNKSTA));
+
+ /* Check retrain flag */
+ if (!(lnksta & PCI_EXP_LNKSTA_LT))
+ break;
+
+ usleep_range(1000, 1100);
+ }
+
+ return 0;
+}
+
/*
* Enable LTSSM of this controller and manually initiate the speed change.
* Always return 0.
@@ -284,6 +321,44 @@ static int rcar_gen4_v4h_v4m_pcie_init(struct rcar_gen4_pcie *rcar)
return 0;
}
+static int rcar_gen5_pcie_init(struct rcar_gen4_pcie *rcar)
+{
+ struct dw_pcie *dw = &rcar->dw;
+ int ret;
+ u32 val;
+
+ /* R-Car Gen4 and Gen5 common initialization. */
+ ret = rcar_gen4_pcie_common_init(rcar);
+ if (ret)
+ return ret;
+
+ /* R-Car Gen5 specific additional initialization. */
+ ret = phy_init(rcar->phy);
+ if (ret)
+ return ret;
+
+ dw_pcie_dbi_ro_wr_en(dw);
+
+ val = dw_pcie_readl_dbi(dw, PCIE_PORT_LANE_SKEW);
+ val &= ~PORT_LANE_SKEW_INSERT_MASK;
+ if (dw->num_lanes < 8)
+ val |= BIT(6);
+ dw_pcie_writel_dbi(dw, PCIE_PORT_LANE_SKEW, val);
+
+ val = dw_pcie_readl_dbi(dw, MSICAP0);
+ FIELD_MODIFY(MSICAP0_MMESCAP_MASK, &val, 4);
+ dw_pcie_writel_dbi(dw, MSICAP0, val);
+
+ dw_pcie_dbi_ro_wr_dis(dw);
+
+ val = readl(rcar->base + PCIEPWRMNGCTRL);
+ val |= APP_CLK_REQ_N | APP_CLK_PM_EN |
+ APP_READY_ENTR_L23 | APP_REQ_ENTR_L1;
+ writel(val, rcar->base + PCIEPWRMNGCTRL);
+
+ return 0;
+}
+
static void rcar_gen4_pcie_common_deinit(struct rcar_gen4_pcie *rcar)
{
struct dw_pcie *dw = &rcar->dw;
@@ -321,8 +396,11 @@ static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar)
struct device_node *root_port;
rcar->phy_base = devm_platform_ioremap_resource_byname(rcar->pdev, "phy");
- if (IS_ERR(rcar->phy_base))
- return PTR_ERR(rcar->phy_base);
+ if (IS_ERR(rcar->phy_base)) {
+ rcar->phy = devm_phy_get(dev, NULL);
+ if (IS_ERR(rcar->phy))
+ return PTR_ERR(rcar->phy);
+ }
root_port = of_get_next_available_child(dev->of_node, NULL);
rcar->perst = of_reset_control_get_optional_exclusive(root_port, "perst");
@@ -743,6 +821,28 @@ static int r8a779f0_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable)
return 0;
}
+static int rcar_gen5_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable)
+{
+ u32 val;
+
+ val = readl(rcar->base + PCIERSTCTRL1);
+ if (enable) {
+ val |= APP_LTSSM_ENABLE;
+ val &= ~APP_HOLD_PHY_RST;
+ } else {
+ val &= ~APP_LTSSM_ENABLE;
+ val |= APP_HOLD_PHY_RST;
+ }
+ writel(val, rcar->base + PCIERSTCTRL1);
+
+ if (enable)
+ phy_power_on(rcar->phy);
+ else
+ phy_power_off(rcar->phy);
+
+ return 0;
+}
+
static void rcar_gen4_pcie_phy_reg_update_bits(struct rcar_gen4_pcie *rcar,
u32 offset, u32 mask, u32 val)
{
@@ -921,6 +1021,13 @@ static struct rcar_gen4_pcie_drvdata drvdata_rcar_gen4_pcie_ep = {
.mode = DW_PCIE_EP_TYPE,
};
+static struct rcar_gen4_pcie_drvdata drvdata_rcar_gen5_pcie = {
+ .init = rcar_gen5_pcie_init,
+ .ltssm_control = rcar_gen5_pcie_ltssm_control,
+ .speed_control = rcar_gen5_pcie_speed_control,
+ .mode = DW_PCIE_RC_TYPE,
+};
+
static const struct of_device_id rcar_gen4_pcie_of_match[] = {
{
.compatible = "renesas,r8a779f0-pcie",
@@ -938,6 +1045,10 @@ static const struct of_device_id rcar_gen4_pcie_of_match[] = {
.compatible = "renesas,rcar-gen4-pcie-ep",
.data = &drvdata_rcar_gen4_pcie_ep,
},
+ {
+ .compatible = "renesas,rcar-gen5-pcie4",
+ .data = &drvdata_rcar_gen5_pcie,
+ },
{},
};
MODULE_DEVICE_TABLE(of, rcar_gen4_pcie_of_match);
--
2.53.0
^ permalink raw reply related
* [PATCH v2 4/5] PCI: dwc: rcar-gen4: Handle PERST via reset subsystem
From: Marek Vasut @ 2026-07-20 2:02 UTC (permalink / raw)
To: linux-pci
Cc: Marek Vasut, Krzysztof Wilczyński, Bjorn Helgaas,
Conor Dooley, Geert Uytterhoeven, Krzysztof Kozlowski,
Lorenzo Pieralisi, Manivannan Sadhasivam, Rob Herring,
Yoshihiro Shimoda, devicetree, linux-kernel, linux-renesas-soc
In-Reply-To: <20260720020505.315335-1-marek.vasut+renesas@mailbox.org>
Handle PERST via both GPIO and reset subsystem. On R-Car Gen4, the
PERST signal is operated as a GPIO, on R-Car Gen5 it might only be
accessible via SCMI reset via reset subsystem. Support both options.
This is a preparatory patch for R-Car Gen5 support.
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V2: - Use dev_err_probe() for perst in rcar_gen4_pcie_get_resources()
- Use of_get_next_available_child() to obtain root port OF node
- Switch to of_reset_control_get_optional_exclusive() to deal with
R-Car Gen4, where the PERST is handled as GPIO instead of reset
- Rename rcar_gen4_pcie_host_perst() to rcar_gen4_pcie_host_perst_assert()
and use bool type for assert and deassert selection
- Add missing reset_control_put() into rcar_gen4_pcie_probe() fail path
---
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 39 ++++++++++++++++++---
1 file changed, 35 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 73b01e74b42cb..2ee8b344ec541 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -98,6 +98,7 @@ struct rcar_gen4_pcie {
void __iomem *base;
void __iomem *phy_base;
struct platform_device *pdev;
+ struct reset_control *perst;
const struct rcar_gen4_pcie_drvdata *drvdata;
};
#define to_rcar_gen4_pcie(_dw) container_of(_dw, struct rcar_gen4_pcie, dw)
@@ -316,12 +317,23 @@ static void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar)
{
+ struct device *dev = rcar->dw.dev;
+ struct device_node *root_port;
+
rcar->phy_base = devm_platform_ioremap_resource_byname(rcar->pdev, "phy");
if (IS_ERR(rcar->phy_base))
return PTR_ERR(rcar->phy_base);
+ root_port = of_get_next_available_child(dev->of_node, NULL);
+ rcar->perst = of_reset_control_get_optional_exclusive(root_port, "perst");
+ of_node_put(root_port);
+ if (IS_ERR(rcar->perst))
+ return dev_err_probe(dev, PTR_ERR(rcar->perst), "Failed to get PERST#\n");
+
/* Renesas-specific registers */
rcar->base = devm_platform_ioremap_resource_byname(rcar->pdev, "app");
+ if (IS_ERR(rcar->base))
+ reset_control_put(rcar->perst);
return PTR_ERR_OR_ZERO(rcar->base);
}
@@ -442,6 +454,22 @@ static int rcar_gen4_pcie_host_msi_init(struct dw_pcie_rp *pp)
return ret;
}
+static void rcar_gen4_pcie_host_perst_assert(struct dw_pcie_rp *pp, bool assert)
+{
+ struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
+ struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+
+ gpiod_set_value_cansleep(dw->pe_rst, assert);
+
+ if (!rcar->perst)
+ return;
+
+ if (assert)
+ reset_control_assert(rcar->perst);
+ else
+ reset_control_deassert(rcar->perst);
+}
+
/* Host mode */
static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
{
@@ -449,7 +477,7 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
int ret;
- gpiod_set_value_cansleep(dw->pe_rst, 1);
+ rcar_gen4_pcie_host_perst_assert(pp, true);
ret = rcar->drvdata->init(rcar);
if (ret)
@@ -470,7 +498,7 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
msleep(PCIE_T_PVPERL_MS); /* pe_rst requires 100msec delay */
- gpiod_set_value_cansleep(dw->pe_rst, 0);
+ rcar_gen4_pcie_host_perst_assert(pp, false);
return 0;
@@ -484,7 +512,7 @@ static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp)
struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
- gpiod_set_value_cansleep(dw->pe_rst, 1);
+ rcar_gen4_pcie_host_perst_assert(pp, true);
rcar_gen4_pcie_common_deinit(rcar);
}
@@ -654,7 +682,7 @@ static int rcar_gen4_pcie_probe(struct platform_device *pdev)
err = rcar_gen4_pcie_prepare(rcar);
if (err)
- return err;
+ goto err_prepare;
err = rcar_gen4_add_dw_pcie(rcar);
if (err)
@@ -662,6 +690,8 @@ static int rcar_gen4_pcie_probe(struct platform_device *pdev)
return 0;
+err_prepare:
+ reset_control_put(rcar->perst);
err_unprepare:
rcar_gen4_pcie_unprepare(rcar);
@@ -688,6 +718,7 @@ static void rcar_gen4_pcie_remove(struct platform_device *pdev)
rcar_gen4_remove_dw_pcie(rcar);
rcar_gen4_pcie_unprepare(rcar);
+ reset_control_put(rcar->perst);
}
static int r8a779f0_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable)
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/5] PCI: dwc: rcar-gen4: Rework rcar_gen4_pcie_drvdata .additional_common_init into .init
From: Marek Vasut @ 2026-07-20 2:02 UTC (permalink / raw)
To: linux-pci
Cc: Marek Vasut, Krzysztof Wilczyński, Bjorn Helgaas,
Conor Dooley, Geert Uytterhoeven, Krzysztof Kozlowski,
Lorenzo Pieralisi, Manivannan Sadhasivam, Rob Herring,
Yoshihiro Shimoda, devicetree, linux-kernel, linux-renesas-soc
In-Reply-To: <20260720020505.315335-1-marek.vasut+renesas@mailbox.org>
Rework the current complicated initialization callchain that invokes
host_init->common_init->additional_common_init() for host and
ep_init->common_init->additional_common_init() for endpoint into
simpler host_init->init() and ep_init->init() .
The init callback is now always implmented for all R-Car Gen4 SoCs,
therefore there is no need to check whether the callback is not NULL.
For R-Car S4 the .init callback is rcar_gen4_pcie_common_init(),
for R-Car V4H and V4M the .init callback is implemented using a
new function rcar_gen4_v4h_v4m_pcie_init() which calls the common
rcar_gen4_pcie_common_init() followed by R-Car V4H and V4M specific
hardware initialization. The hardware initialization sequence remains
unchanged.
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V2: Rename additional_common_init to init, reworked from previous patch
PCI: dwc: rcar-gen4: Return error code from .additional_common_init
---
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 56 ++++++++++++---------
1 file changed, 32 insertions(+), 24 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 5f7211b91ee5b..5724f64eadfad 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -87,7 +87,7 @@ MODULE_FIRMWARE(RCAR_GEN4_PCIE_FIRMWARE_NAME);
struct rcar_gen4_pcie;
struct rcar_gen4_pcie_drvdata {
- void (*additional_common_init)(struct rcar_gen4_pcie *rcar);
+ int (*init)(struct rcar_gen4_pcie *rcar);
int (*ltssm_control)(struct rcar_gen4_pcie *rcar, bool enable);
enum dw_pcie_device_mode mode;
};
@@ -241,9 +241,6 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar)
reset_control_status(dw->core_rsts[DW_PCIE_PWR_RST].rstc);
fsleep(1000);
- if (rcar->drvdata->additional_common_init)
- rcar->drvdata->additional_common_init(rcar);
-
return 0;
err_unprepare:
@@ -252,6 +249,31 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar)
return ret;
}
+static int rcar_gen4_v4h_v4m_pcie_init(struct rcar_gen4_pcie *rcar)
+{
+ struct dw_pcie *dw = &rcar->dw;
+ u32 val;
+ int ret;
+
+ /* R-Car Gen4 common initialization. */
+ ret = rcar_gen4_pcie_common_init(rcar);
+ if (ret)
+ return ret;
+
+ /* R-Car V4H and V4M specific additional initialization. */
+ val = dw_pcie_readl_dbi(dw, PCIE_PORT_LANE_SKEW);
+ val &= ~PORT_LANE_SKEW_INSERT_MASK;
+ if (dw->num_lanes < 4)
+ val |= BIT(6);
+ dw_pcie_writel_dbi(dw, PCIE_PORT_LANE_SKEW, val);
+
+ val = readl(rcar->base + PCIEPWRMNGCTRL);
+ val |= APP_CLK_REQ_N | APP_CLK_PM_EN;
+ writel(val, rcar->base + PCIEPWRMNGCTRL);
+
+ return 0;
+}
+
static void rcar_gen4_pcie_common_deinit(struct rcar_gen4_pcie *rcar)
{
struct dw_pcie *dw = &rcar->dw;
@@ -420,7 +442,7 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
gpiod_set_value_cansleep(dw->pe_rst, 1);
- ret = rcar_gen4_pcie_common_init(rcar);
+ ret = rcar->drvdata->init(rcar);
if (ret)
return ret;
@@ -487,7 +509,7 @@ static void rcar_gen4_pcie_ep_pre_init(struct dw_pcie_ep *ep)
struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
int ret;
- ret = rcar_gen4_pcie_common_init(rcar);
+ ret = rcar->drvdata->init(rcar);
if (ret)
return;
@@ -681,22 +703,6 @@ static int r8a779f0_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable)
return 0;
}
-static void rcar_gen4_pcie_additional_common_init(struct rcar_gen4_pcie *rcar)
-{
- struct dw_pcie *dw = &rcar->dw;
- u32 val;
-
- val = dw_pcie_readl_dbi(dw, PCIE_PORT_LANE_SKEW);
- val &= ~PORT_LANE_SKEW_INSERT_MASK;
- if (dw->num_lanes < 4)
- val |= BIT(6);
- dw_pcie_writel_dbi(dw, PCIE_PORT_LANE_SKEW, val);
-
- val = readl(rcar->base + PCIEPWRMNGCTRL);
- val |= APP_CLK_REQ_N | APP_CLK_PM_EN;
- writel(val, rcar->base + PCIEPWRMNGCTRL);
-}
-
static void rcar_gen4_pcie_phy_reg_update_bits(struct rcar_gen4_pcie *rcar,
u32 offset, u32 mask, u32 val)
{
@@ -848,23 +854,25 @@ static int rcar_gen4_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable
}
static struct rcar_gen4_pcie_drvdata drvdata_r8a779f0_pcie = {
+ .init = rcar_gen4_pcie_common_init,
.ltssm_control = r8a779f0_pcie_ltssm_control,
.mode = DW_PCIE_RC_TYPE,
};
static struct rcar_gen4_pcie_drvdata drvdata_r8a779f0_pcie_ep = {
+ .init = rcar_gen4_pcie_common_init,
.ltssm_control = r8a779f0_pcie_ltssm_control,
.mode = DW_PCIE_EP_TYPE,
};
static struct rcar_gen4_pcie_drvdata drvdata_rcar_gen4_pcie = {
- .additional_common_init = rcar_gen4_pcie_additional_common_init,
+ .init = rcar_gen4_v4h_v4m_pcie_init,
.ltssm_control = rcar_gen4_pcie_ltssm_control,
.mode = DW_PCIE_RC_TYPE,
};
static struct rcar_gen4_pcie_drvdata drvdata_rcar_gen4_pcie_ep = {
- .additional_common_init = rcar_gen4_pcie_additional_common_init,
+ .init = rcar_gen4_v4h_v4m_pcie_init,
.ltssm_control = rcar_gen4_pcie_ltssm_control,
.mode = DW_PCIE_EP_TYPE,
};
--
2.53.0
^ permalink raw reply related
* [PATCH v2 3/5] PCI: dwc: rcar-gen4: Split .start_link into ltssm_control and speed_control
From: Marek Vasut @ 2026-07-20 2:02 UTC (permalink / raw)
To: linux-pci
Cc: Marek Vasut, Krzysztof Wilczyński, Bjorn Helgaas,
Conor Dooley, Geert Uytterhoeven, Krzysztof Kozlowski,
Lorenzo Pieralisi, Manivannan Sadhasivam, Rob Herring,
Yoshihiro Shimoda, devicetree, linux-kernel, linux-renesas-soc
In-Reply-To: <20260720020505.315335-1-marek.vasut+renesas@mailbox.org>
Introduce new .speed_control callback and implement it on R-Car Gen4.
The callback implements the second half of what is currently present
in rcar_gen4_pcie_start_link(), and rcar_gen4_pcie_start_link() does
call the .speed_control callback. This is a preparatory patch for
R-Car Gen5 support, where the .speed_control implementation is
different.
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V2: No change
---
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 39 ++++++++++++++-------
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 5724f64eadfad..73b01e74b42cb 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -89,6 +89,7 @@ struct rcar_gen4_pcie;
struct rcar_gen4_pcie_drvdata {
int (*init)(struct rcar_gen4_pcie *rcar);
int (*ltssm_control)(struct rcar_gen4_pcie *rcar, bool enable);
+ int (*speed_control)(struct rcar_gen4_pcie *rcar);
enum dw_pcie_device_mode mode;
};
@@ -140,20 +141,10 @@ static int rcar_gen4_pcie_speed_change(struct dw_pcie *dw)
return -ETIMEDOUT;
}
-/*
- * Enable LTSSM of this controller and manually initiate the speed change.
- * Always return 0.
- */
-static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
+static int rcar_gen4_pcie_speed_control(struct rcar_gen4_pcie *rcar)
{
- struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
- int i, changes, ret;
-
- if (rcar->drvdata->ltssm_control) {
- ret = rcar->drvdata->ltssm_control(rcar, true);
- if (ret)
- return ret;
- }
+ struct dw_pcie *dw = &rcar->dw;
+ int i, changes;
/*
* Require direct speed change with retrying here if the max_link_speed
@@ -177,6 +168,24 @@ static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
return 0;
}
+/*
+ * Enable LTSSM of this controller and manually initiate the speed change.
+ * Always return 0.
+ */
+static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
+{
+ struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+ int ret;
+
+ if (rcar->drvdata->ltssm_control) {
+ ret = rcar->drvdata->ltssm_control(rcar, true);
+ if (ret)
+ return ret;
+ }
+
+ return rcar->drvdata->speed_control(rcar);
+}
+
static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
{
struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
@@ -856,24 +865,28 @@ static int rcar_gen4_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable
static struct rcar_gen4_pcie_drvdata drvdata_r8a779f0_pcie = {
.init = rcar_gen4_pcie_common_init,
.ltssm_control = r8a779f0_pcie_ltssm_control,
+ .speed_control = rcar_gen4_pcie_speed_control,
.mode = DW_PCIE_RC_TYPE,
};
static struct rcar_gen4_pcie_drvdata drvdata_r8a779f0_pcie_ep = {
.init = rcar_gen4_pcie_common_init,
.ltssm_control = r8a779f0_pcie_ltssm_control,
+ .speed_control = rcar_gen4_pcie_speed_control,
.mode = DW_PCIE_EP_TYPE,
};
static struct rcar_gen4_pcie_drvdata drvdata_rcar_gen4_pcie = {
.init = rcar_gen4_v4h_v4m_pcie_init,
.ltssm_control = rcar_gen4_pcie_ltssm_control,
+ .speed_control = rcar_gen4_pcie_speed_control,
.mode = DW_PCIE_RC_TYPE,
};
static struct rcar_gen4_pcie_drvdata drvdata_rcar_gen4_pcie_ep = {
.init = rcar_gen4_v4h_v4m_pcie_init,
.ltssm_control = rcar_gen4_pcie_ltssm_control,
+ .speed_control = rcar_gen4_pcie_speed_control,
.mode = DW_PCIE_EP_TYPE,
};
--
2.53.0
^ permalink raw reply related
* [PATCH v2 1/5] dt-bindings: PCI: rcar-gen4-pci-host: Add R-Car X5H PCIe4 compatible
From: Marek Vasut @ 2026-07-20 2:02 UTC (permalink / raw)
To: linux-pci
Cc: Marek Vasut, Krzysztof Kozlowski, Krzysztof Wilczyński,
Bjorn Helgaas, Conor Dooley, Geert Uytterhoeven,
Krzysztof Kozlowski, Lorenzo Pieralisi, Manivannan Sadhasivam,
Rob Herring, Yoshihiro Shimoda, devicetree, linux-kernel,
linux-renesas-soc
In-Reply-To: <20260720020505.315335-1-marek.vasut+renesas@mailbox.org>
Document bindings for R-Car X5H (R8A78000) PCIe4 host module.
The binding document is deliberately using "renesas,rcar-gen5-pcie4"
DT compatible string to discern R-Car X5H PCIe4 controller supported
by this binding, from R-Car X5H PCIe6 controller which will use a
separate binding.
The R-Car X5H PCIe4 controller does no longer include PHY register
range, the PHY is now a separate IP and referenced via the 'phy' DT
property. The 'reg' and 'reg-names' DT properties therefore differ
between R-Car Gen4 PCIe and R-Car X5H PCIe4, and the difference is
handled in the allOf section.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V2: Add RB from Krzysztof
---
.../bindings/pci/rcar-gen4-pci-host.yaml | 74 +++++++++++++------
1 file changed, 52 insertions(+), 22 deletions(-)
diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
index 64f456c6a75a1..9767a52f3904e 100644
--- a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
+++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
@@ -10,30 +10,20 @@ title: Renesas R-Car Gen4 PCIe Host
maintainers:
- Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
-allOf:
- - $ref: snps,dw-pcie.yaml#
-
properties:
compatible:
- items:
- - enum:
- - renesas,r8a779f0-pcie # R-Car S4-8
- - renesas,r8a779g0-pcie # R-Car V4H
- - renesas,r8a779h0-pcie # R-Car V4M
- - const: renesas,rcar-gen4-pcie # R-Car Gen4
-
- reg:
- maxItems: 7
-
- reg-names:
- items:
- - const: dbi
- - const: dbi2
- - const: atu
- - const: dma
- - const: app
- - const: phy
- - const: config
+ oneOf:
+ - items:
+ - enum:
+ - renesas,r8a779f0-pcie # R-Car S4-8
+ - renesas,r8a779g0-pcie # R-Car V4H
+ - renesas,r8a779h0-pcie # R-Car V4M
+ - const: renesas,rcar-gen4-pcie # R-Car Gen4
+
+ - items:
+ - enum:
+ - renesas,r8a78000-pcie4 # R-Car X5H PCIe4
+ - const: renesas,rcar-gen5-pcie4 # R-Car Gen5 PCIe4
interrupts:
maxItems: 4
@@ -84,6 +74,46 @@ required:
- resets
- reset-names
+allOf:
+ - $ref: snps,dw-pcie.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: renesas,rcar-gen4-pcie
+ then:
+ properties:
+ reg:
+ maxItems: 7
+
+ reg-names:
+ items:
+ - const: dbi
+ - const: dbi2
+ - const: atu
+ - const: dma
+ - const: app
+ - const: phy
+ - const: config
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: renesas,rcar-gen5-pcie4
+ then:
+ properties:
+ reg:
+ maxItems: 6
+
+ reg-names:
+ items:
+ - const: dbi
+ - const: dbi2
+ - const: atu
+ - const: dma
+ - const: app
+ - const: config
+
unevaluatedProperties: false
examples:
--
2.53.0
^ permalink raw reply related
* [PATCH v2 0/5] PCI: dwc: rcar-gen4: Add support for R-Car X5H PCIe4
From: Marek Vasut @ 2026-07-20 2:02 UTC (permalink / raw)
To: linux-pci
Cc: Marek Vasut, Krzysztof Wilczyński, Bjorn Helgaas,
Conor Dooley, Geert Uytterhoeven, Krzysztof Kozlowski,
Lorenzo Pieralisi, Manivannan Sadhasivam, Rob Herring,
Yoshihiro Shimoda, devicetree, linux-kernel, linux-renesas-soc
Add support for R8A78000 (R-Car X5H) PCIe4.
This driver previously supported R-Car Gen4 S4/V4H/V4M. PCIe features
of R-Car X5H PCIe4 are almost all the same, except for different PHY
which has its own driver, slightly different initialization code and
the DMA is no longer eDMA but HDMA.
Endpoint mode is currently not implemented for R-Car Gen5 PCIe4.
Marek Vasut (5):
dt-bindings: PCI: rcar-gen4-pci-host: Add R-Car X5H PCIe4 compatible
PCI: dwc: rcar-gen4: Rework rcar_gen4_pcie_drvdata
.additional_common_init into .init
PCI: dwc: rcar-gen4: Split .start_link into ltssm_control and
speed_control
PCI: dwc: rcar-gen4: Handle PERST via reset subsystem
PCI: dwc: rcar-gen4: Add support for R-Car X5H PCIe4
.../bindings/pci/rcar-gen4-pci-host.yaml | 74 ++++--
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 237 +++++++++++++++---
2 files changed, 252 insertions(+), 59 deletions(-)
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
--
2.53.0
^ permalink raw reply
* Re: [PATCH v6 12/14] media: mediatek: vcodec: clean xpc status
From: sashiko-bot @ 2026-07-20 2:03 UTC (permalink / raw)
To: Kyrie Wu; +Cc: robh, devicetree, conor+dt, media-ci
In-Reply-To: <20260720012056.1026551-13-kyrie.wu@mediatek.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Low] Enum type mismatch when indexing parent device's register array.
Pre-existing issues:
- [High] Null pointer dereference on spurious hardware interrupts.
- [High] Use-after-free and dirty state on probe error path.
- [High] Unconditional NULL pointer dereference in the hardware IRQ handler on platforms using syscon for VDECSYS.
--
--- Patch [12]: [PATCH v6 12/14] media: mediatek: vcodec: clean xpc status ---
commit ff6c78cd026b1536229c06cdffd12b2498c8aeb9
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: vcodec: clean xpc status
The driver need to clean xpc status when receive decoder hardware
interrupt for mt8196 platform.
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c
> index 881d5de41e050..46ac3e41c401c 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c
> @@ -88,6 +116,8 @@ static irqreturn_t mtk_vdec_hw_irq_handler(int irq, void *priv)
[Severity: High]
This is a pre-existing issue, but does mtk_vdec_hw_irq_handler() in
mtk_vcodec_dec_hw.c properly handle spurious interrupts where
mtk_vcodec_get_curr_ctx() returns NULL?
If there is no active context when the interrupt fires, ctx will be NULL.
The handler subsequently passes this NULL context to mtk_v4l2_vdec_err(),
which dereferences ctx->dev directly:
mtk_vdec_hw_irq_handler() {
...
ctx = mtk_vcodec_get_curr_ctx(dev->main_dev, dev->hw_idx);
/* check if HW active or not */
cg_status = readl(dev->reg_base[VDEC_HW_SYS] + VDEC_HW_ACTIVE_ADDR);
if (cg_status & VDEC_HW_ACTIVE_MASK) {
mtk_v4l2_vdec_err(ctx, "vdec active is not 0x0 (0x%08x)", cg_status);
return IRQ_HANDLED;
}
...
}
Could this trigger a kernel panic on spurious interrupts?
[Severity: High]
This is also a pre-existing issue, but will dev->reg_base[VDEC_HW_SYS] be
NULL in mtk_vdec_hw_irq_handler() on platforms using syscon for VDECSYS
(like MT8195 and MT8196)?
On platforms utilizing the mediatek,vdecsys syscon, main_dev->reg_base[0] is
left NULL. The subdevice probe copies this NULL pointer to the subdevice.
When an interrupt fires, the handler performs an unconditional readl() from
this NULL address in the snippet shown above, instead of using the
vdecsys_regmap fallback.
Could this cause an unconditional panic when an interrupt fires on affected
platforms?
> writel(dec_done_status | VDEC_IRQ_CFG, vdec_misc_addr);
> writel(dec_done_status & ~VDEC_IRQ_CLR, vdec_misc_addr);
>
> + mtk_vdec_hw_clean_xpc(dev);
> +
> wake_up_dec_ctx(ctx, MTK_INST_IRQ_RECEIVED, dev->hw_idx);
>
> mtk_v4l2_vdec_dbg(3, ctx, "wake up ctx %d, dec_done_status=%x",
[ ... ]
> @@ -166,6 +196,7 @@ static int mtk_vdec_hw_probe(struct platform_device *pdev)
> subdev_dev->hw_idx = hw_idx;
> subdev_dev->main_dev = main_dev;
> subdev_dev->reg_base[VDEC_HW_SYS] = main_dev->reg_base[VDEC_HW_SYS];
> + subdev_dev->reg_base[VDEC_HW_XPC] = main_dev->reg_base[VDEC_HW_MISC];
[Severity: Low]
Is mtk_vdec_hw_probe() indexing the main_dev->reg_base array with the
correct enum?
It looks like it is using VDEC_HW_MISC (from enum mtk_vdec_hw_reg_idx)
instead of the expected VDEC_MISC (from enum mtk_hw_reg_idx). While they
both currently evaluate to 1, could this cause out-of-bounds access or
incorrect address mappings if the enums drift?
> set_bit(subdev_dev->hw_idx, main_dev->subdev_bitmap);
>
> if (IS_SUPPORT_VDEC_HW_IRQ(hw_idx)) {
[Severity: High]
This is a pre-existing issue, but does the mtk_vdec_hw_probe() error path
leave dangling pointers in the parent device?
If mtk_vdec_hw_init_irq() fails here, mtk_vdec_hw_probe() returns an error:
mtk_vdec_hw_probe() {
...
if (IS_SUPPORT_VDEC_HW_IRQ(hw_idx)) {
ret = mtk_vdec_hw_init_irq(subdev_dev);
if (ret)
return ret;
}
...
}
Since subdev_dev was allocated with devres, it will be automatically freed.
However, the parent (main_dev) will retain the freed pointer in
main_dev->subdev_dev[hw_idx] and the bit in main_dev->subdev_bitmap remains
set.
Could this lead to a use-after-free if another component iterates over the
subdevices?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=12
^ permalink raw reply
* Re: [PATCH] dt-bindings: PCI: rcar-gen4-pci-host: Document optional msi-parent
From: sashiko-bot @ 2026-07-20 2:01 UTC (permalink / raw)
To: Marek Vasut; +Cc: devicetree, linux-pci, wsa+renesas, robh, conor+dt
In-Reply-To: <20260720015505.314957-1-marek.vasut+renesas@mailbox.org>
> The R-Car Gen4 PCIe controller can optionally use GIC ITS as an
> MSI controller. This configuration is described in DT by setting
> msi-parent DT property in the PCIe controller node. Document this
> configuration.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720015505.314957-1-marek.vasut+renesas@mailbox.org?part=1
^ permalink raw reply
* RE: [PATCH v2 3/5] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support
From: Torreno, Alexis Czezar @ 2026-07-20 1:58 UTC (permalink / raw)
To: sashiko-reviews@lists.linux.dev
Cc: linux-hwmon@vger.kernel.org, conor+dt@kernel.org,
devicetree@vger.kernel.org, robh@kernel.org
In-Reply-To: <20260706022155.C5B251F000E9@smtp.kernel.org>
> > +
> > +/*
> > + * Driver currently only supports READ_VOUT for monitoring that needs
> > +scaling to
> > + * convert from feedback voltage to actual output voltage.
> > + *
> > + * vout_actual = vout_fb × (1 + RFB1/RFB2)
> > + *
> > + * VOUT_COMMAND and VOUT_MAX are currently not in use, but work with
> > +feedback
> > + * voltage. If regulator support is added, VOUT_COMMAND values would
> > +need
> > + * to be adjusted accordingly.
> > + */
> > +static int max20830_read_word_data(struct i2c_client *client, int page,
> > + int phase, int reg)
> > +{
> > + const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> > + const struct max20830_data *data = container_of(info, struct
> max20830_data, info);
> > + int ret;
> > +
> > + switch (reg) {
> > + case PMBUS_READ_VOUT:
> > + ret = pmbus_read_word_data(client, page, phase, reg);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* Apply voltage divider scaling if resistors are non-zero */
> > + if (data->vout_rfb1 && data->vout_rfb2)
> > + ret = DIV_ROUND_CLOSEST_ULL((u64)ret * (data-
> >vout_rfb1 +
> > + data->vout_rfb2), data-
> >vout_rfb2);
> > + return ret;
>
> [Severity: High]
> Does this returned value need bounds checking?
>
> If a large resistor divider is used, the scaled PMBus word calculated here can
> exceed 0xFFFF. The generic PMBus core function pmbus_reg2data_linear() will
> cast this returned value to a u16, silently wrapping the value modulo
> 65536 and resulting in wildly incorrect telemetry (e.g., reporting 2V instead of
> 12V).
>
> Should this be clamped using clamp_val(ret, 0, 0xffff)?
>
Good Call, will clamp this as suggested.
> > + default:
> > + return -ENODATA;
> > + }
> > +}
>
> [Severity: High]
> Will returning -ENODATA bypass software scaling for VOUT limit attributes?
>
> The driver sets PMBUS_HAVE_VOUT below, which exposes hardware VOUT
> limits (like inX_max or inX_lcrit). Because only PMBUS_READ_VOUT is
> intercepted here, the core will read those limits unscaled from the hardware.
>
Not applicable - no limit registers exist or are exposed.
> > +
> > static struct pmbus_driver_info max20830_info = {
> > .pages = 1,
> > .format[PSC_VOLTAGE_IN] = linear,
> > @@ -24,13 +64,25 @@ static struct pmbus_driver_info max20830_info = {
> > PMBUS_HAVE_TEMP |
> > PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT |
> > PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,
> > + .read_word_data = max20830_read_word_data,
>
> [Severity: High]
> Does this struct also need a .write_word_data callback?
>
> Since PMBUS_HAVE_VOUT is enabled, user writes to limit attributes will fall
> through to the PMBus core, which writes the scaled PMBus word directly into
> the hardware's limit register. Because the hardware expects limits in the
> unscaled feedback voltage range, the written value will be completely out of
> bounds.
>
Not applicable. No writable VOUT registers are exposed. MAX20830 has no
limit registers to write to. also regulator support is not implemented.
> --
> Sashiko AI review ·
> https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260706-dev-
> max20830c-v2-0-
> 37761e89bb5f@analog.com?part=3__;Iw!!A3Ni8CS0y2Y!9BBLoE4ditcREIvsDD7
> _VDDKjsS7XgrV0udXIgvBZeCRChzDWWsu7TY8ybr5VBPG4VQlLnDS6EQmu6fhfLx
> CpKUl28ZSE04$
^ permalink raw reply
* [PATCH] dt-bindings: PCI: rcar-gen4-pci-host: Document optional msi-parent
From: Marek Vasut @ 2026-07-20 1:54 UTC (permalink / raw)
To: linux-pci
Cc: Marek Vasut, Krzysztof Wilczyński, Bjorn Helgaas,
Conor Dooley, Geert Uytterhoeven, Koichiro Den,
Krzysztof Kozlowski, Lorenzo Pieralisi, Manivannan Sadhasivam,
Rob Herring, Yoshihiro Shimoda, devicetree, linux-kernel,
linux-renesas-soc
The R-Car Gen4 PCIe controller can optionally use GIC ITS as an
MSI controller. This configuration is described in DT by setting
msi-parent DT property in the PCIe controller node. Document this
configuration.
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Koichiro Den <den@valinux.co.jp>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
index bb3f843c59d91..64f456c6a75a1 100644
--- a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
+++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
@@ -66,6 +66,9 @@ properties:
max-link-speed:
maximum: 4
+ msi-parent:
+ maxItems: 1
+
num-lanes:
maximum: 4
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v6 14/14] media: mediatek: decoder: support av1 extend vsi
From: sashiko-bot @ 2026-07-20 1:50 UTC (permalink / raw)
To: Kyrie Wu; +Cc: conor+dt, robh, devicetree, media-ci
In-Reply-To: <20260720012056.1026551-15-kyrie.wu@mediatek.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] Buffer overflow into firmware shared memory due to unconditional sizeof() in vdec_av1_slice_vsi_to_remote()
Pre-existing issues:
- [High] Use-After-Free (UAF) due to lockless access to dynamic V4L2 control array in vdec_av1_slice_setup_tile_group()
--
--- Patch [14]: [PATCH v6 14/14] media: mediatek: decoder: support av1 extend vsi ---
commit ed7b2651547150bfaed8f1355f43d152991a7bc1
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: decoder: support av1 extend vsi
This commit introduces support for AV1 extend VSI architecture in the
Mediatek decoder. It adds tile group information to the VPU shared memory
structure for extended architectures.
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> index 14639098fd463..1ef561dae18c7 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
[ ... ]
> @@ -641,6 +644,8 @@ struct vdec_av1_slice_fb {
> * @frame: current frame info
> * @state: status after decode done
> * @cur_lst_tile_id: tile id for large scale
> + * @tile_group: tile group info
> + * @reserved: reserved
> */
> struct vdec_av1_slice_vsi {
> /* lat */
> @@ -665,6 +670,8 @@ struct vdec_av1_slice_vsi {
> struct vdec_av1_slice_frame frame;
> struct vdec_av1_slice_state state;
> u32 cur_lst_tile_id;
> + struct vdec_av1_slice_tile_group tile_group;
> + unsigned int reserved[4];
> };
[Severity: Critical]
Does the addition of this large struct cause a buffer overflow on
non-extended architectures?
The firmware allocates only 3408 bytes (AV1_NON_EXT_VSI_SIZE) for this
memory mapping on non-extended architectures. However, since
vdec_av1_slice_vsi_to_remote() unconditionally copies the entire struct
using sizeof(*vsi), won't this write out-of-bounds data into the adjacent
VPU shared memory?
[ ... ]
> @@ -1402,17 +1409,29 @@ static void vdec_av1_slice_setup_uh(struct vdec_av1_slice_instance *instance,
> vdec_av1_slice_setup_tile(frame, &ctrl_fh->tile_info);
> }
>
> +static
> +struct vdec_av1_slice_tile_group *vdec_av1_get_tile_group(struct vdec_av1_slice_instance *instance,
> + struct vdec_av1_slice_vsi *vsi)
> +{
> + if (IS_VDEC_SUPPORT_EXT(instance->ctx->dev->dec_capability))
> + return &vsi->tile_group;
> + else
> + return &instance->tile_group;
> +}
> +
> static int vdec_av1_slice_setup_tile_group(struct vdec_av1_slice_instance *instance,
> struct vdec_av1_slice_vsi *vsi)
> {
> struct v4l2_ctrl_av1_tile_group_entry *ctrl_tge;
> - struct vdec_av1_slice_tile_group *tile_group = &instance->tile_group;
> + struct vdec_av1_slice_tile_group *tile_group;
> struct vdec_av1_slice_uncompressed_header *uh = &vsi->frame.uh;
> struct vdec_av1_slice_tile *tile = &uh->tile;
> struct v4l2_ctrl *ctrl;
> u32 tge_size;
> int i;
>
> + tile_group = vdec_av1_get_tile_group(instance, vsi);
> +
> ctrl = v4l2_ctrl_find(&instance->ctx->ctrl_hdl, V4L2_CID_STATELESS_AV1_TILE_GROUP_ENTRY);
> if (!ctrl)
> return -EINVAL;
[Severity: High]
This is a pre-existing issue, but does this function access the dynamically
allocated V4L2 control payload locklessly?
vdec_av1_slice_setup_tile_group() fetches the control and later accesses
ctrl->p_cur.p without holding the control handler lock (ctrl_hdl->lock).
Since this executes in a workqueue, could a concurrent VIDIOC_S_EXT_CTRLS
ioctl cause the V4L2 core to reallocate the array and free the old memory?
This would result in a use-after-free read of the old array payload, potentially
tricking the hardware into performing out-of-bounds DMA.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=14
^ permalink raw reply
* Re: [PATCH v6 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo
From: sashiko-bot @ 2026-07-20 1:48 UTC (permalink / raw)
To: Kyrie Wu; +Cc: conor+dt, devicetree, robh, media-ci
In-Reply-To: <20260720012056.1026551-14-kyrie.wu@mediatek.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but this patch incompletely fixes a subsystem-wide issue by fixing AV1 while leaving the exact same bug in the VP9 stateless decoder. The C plane offset for reference frames is incorrectly calculated during dynamic resolution changes.
--
--- Patch [13]: [PATCH v6 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo ---
commit 6fb6f33cb9871d45e294575fbf5e9c61dfa39742
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: decoder: fill av1 buffer size with picinfo
The buffer size of y and c plane has been calculated in vcp/scp,
can fill each frame buffer size with picinfo directly.
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
[ ... ]
> @@ -1845,8 +1846,10 @@ static int vdec_av1_slice_setup_core_buffer(struct vdec_av1_slice_instance *inst
> }
>
> vref->y.dma_addr = vb2_dma_contig_plane_dma_addr(vb, 0);
> + vref->y.size = vsi->fb.y.size;
> + vref->c.size = vsi->fb.c.size;
> if (plane == 1)
> - vref->c.dma_addr = vref->y.dma_addr + size;
> + vref->c.dma_addr = vref->y.dma_addr + vsi->fb.y.size;
[Severity: High]
This is a pre-existing issue, but this fix for AV1 appears to leave the exact
same bug unfixed in the VP9 stateless decoder.
When a VP9 stream changes resolution mid-stream, the driver calculates the
C plane offset based on the current frame's width and height. Does this
erroneously apply the current-frame offset to all previously decoded
reference frames?
vdec_vp9_req_lat_if.c:vdec_vp9_slice_setup_core_buffer() {
...
size = ALIGN(w, 64) * ALIGN(h, 64);
...
vsi->ref[i].c.dma_addr =
vsi->ref[i].y.dma_addr + size;
...
}
If the current frame is smaller than the reference frame, would the hardware
read corrupted chroma data from the middle of the reference frame's Y plane?
If the current frame is larger, could the calculated offset applied to the
reference frame point beyond its actual chroma offset, potentially triggering
a hardware fault via the IOMMU?
Could a similar fix using ctx->picinfo.fb_sz[0] be applied to VP9?
> else
> vref->c.dma_addr = vb2_dma_contig_plane_dma_addr(vb, 1);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=13
^ permalink raw reply
* Re: [PATCH v6 03/14] media: mediatek: vcodec: add driver to support vcp encoder
From: sashiko-bot @ 2026-07-20 1:45 UTC (permalink / raw)
To: Kyrie Wu; +Cc: devicetree, media-ci, robh, conor+dt
In-Reply-To: <20260720012056.1026551-4-kyrie.wu@mediatek.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing `.get_venc_capa` callback in `mtk_vcodec_vcp_msg` causes a guaranteed kernel panic on encoder device open.
- [High] Late IPI acknowledgments from the firmware can falsely satisfy subsequent IPI requests after a timeout, leading to data corruption or desynchronization.
- [Medium] Early return on `deregister_feature` failure leaks the `rproc` remote processor reference.
--
--- Patch [3]: [PATCH v6 03/14] media: mediatek: vcodec: add driver to support vcp encoder ---
commit 9d12e27cbb34067860fe1673f4f8bfcc5549ebf1
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: vcodec: add driver to support vcp encoder
This patch adds VCP encoder support to the mediatek vcodec driver. It enables
the encoder to call the VCP interface to communicate with the coprocessor.
> 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
> index 150d842c7d226..ece4e3e87e62d 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
[ ... ]
> @@ -223,8 +225,13 @@ static int mtk_vcodec_vcp_msg_ipi_send(struct mtk_vcodec_fw *fw, int id, void *b
> }
>
> mutex_lock(&vcp->ipi_mutex);
> - feature_id = VDEC_FEATURE_ID;
> - mailbox_id = IPI_OUT_VDEC_1;
> + if (fw->fw_use == ENCODER) {
> + feature_id = VENC_FEATURE_ID;
> + mailbox_id = IPI_OUT_VENC_0;
> + } else {
> + 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(vcp_device, feature_id)) {
[Severity: High]
Can late IPI acknowledgments from the firmware falsely satisfy subsequent IPI
requests after a timeout?
If wait_event_interruptible_timeout() in mtk_vcodec_vcp_msg_ipi_send() times
out, it aborts and unlocks the IPI mutex. If a new request is submitted reusing
the same ID, and the firmware finally sends a delayed ACK for the original
timed-out request, the background msg_thread could blindly accept it:
mtk_vcodec_vcp_msg_process_thread() {
...
fw->vcp->msg_signaled[obj->id] = true;
wake_up(&fw->vcp->msg_wq[obj->id]);
...
}
Would this cause the new request to immediately return success while reading
stale responses belonging to the timed-out request, leading to protocol
desynchronization?
[ ... ]
> @@ -530,7 +562,11 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_vcp_init(void *priv, enum mtk_vcodec_fw_use
> struct mtk_vcodec_fw *fw;
> int i;
>
> - if (fw_use == DECODER) {
> + if (fw_use == ENCODER) {
> + struct mtk_vcodec_enc_dev *enc_dev = priv;
> +
> + plat_dev = enc_dev->plat_dev;
> + } else if (fw_use == DECODER) {
> struct mtk_vcodec_dec_dev *dec_dev = priv;
>
> plat_dev = dec_dev->plat_dev;
[Severity: High]
Does this patch miss implementing the get_venc_capa callback in the
mtk_vcodec_vcp_msg operations struct?
When the V4L2 encoder device is opened, fops_vcodec_open() unconditionally
calls mtk_vcodec_fw_get_venc_capa(), which attempts to dereference:
mtk_vcodec_fw_get_venc_capa() {
return fw->ops->get_venc_capa(fw);
}
If the mtk_vcodec_vcp_msg struct does not have this callback registered,
could this result in a NULL pointer dereference when a userspace application
opens the encoder video device node?
[Severity: Medium]
In mtk_vcodec_vcp_release(), if the coprocessor's deregister_feature operation
fails, the function emits an error and returns early:
mtk_vcodec_vcp_release() {
...
ret = vcp_device->ops->deregister_feature(vcp_device, fw->vcp->feature_id);
if (ret < 0) {
dev_err(dev, "deregister feature_id(%d) fail(%d)\n", fw->vcp->feature_id, ret);
return;
}
...
}
Will this early return skip the put_device label that calls
mtk_vcodec_vcp_put_device(), leading to a leaked rproc reference since
rproc_put() would be bypassed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=3
^ permalink raw reply
* Re: [PATCH v6 04/14] media: mediatek: vcodec: get different firmware ipi id
From: sashiko-bot @ 2026-07-20 1:44 UTC (permalink / raw)
To: Kyrie Wu; +Cc: robh, media-ci, conor+dt, devicetree
In-Reply-To: <20260720012056.1026551-5-kyrie.wu@mediatek.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing check on `mtk_vcodec_fw_get_ipi()` return value leads to out-of-bounds memory write during VPU IPI registration
--
--- Patch [4]: [PATCH v6 04/14] media: mediatek: vcodec: get different firmware ipi id ---
commit 74021a635ff4b0a5bb38e1b68df4fa4d276cadb5
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: vcodec: get different firmware ipi id
This commit introduces a helper function to get the IPI ID based on
firmware type and hardware index for different architectures. It replaces
hardcoded SCP IPI IDs with this helper in various decoder interfaces.
> 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 552d0d8a84444..8630e871d78e2 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> @@ -3,6 +3,20 @@
> #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"
> +
> +int mtk_vcodec_fw_get_ipi(enum mtk_vcodec_fw_type type, int hw_id)
> +{
> + switch (type) {
> + case SCP:
> + return hw_id == MTK_VDEC_LAT0 ? SCP_IPI_VDEC_LAT : SCP_IPI_VDEC_CORE;
> + case VCP:
> + return hw_id == MTK_VDEC_LAT0 ? VCP_IPI_LAT_DECODER : VCP_IPI_CORE_DECODER;
> + default:
> + return -EINVAL;
> + }
> +}
[ ... ]
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
> index 10359ce9b934e..69d60717181a6 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
> @@ -1214,8 +1215,8 @@ static int vdec_h264_slice_init(struct mtk_vcodec_dec_ctx *ctx)
>
> inst->ctx = ctx;
>
> - inst->vpu.id = SCP_IPI_VDEC_LAT;
> - inst->vpu.core_id = SCP_IPI_VDEC_CORE;
> + inst->vpu.id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_LAT0);
> + inst->vpu.core_id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_CORE);
[Severity: High]
Does this code need to check the return value of mtk_vcodec_fw_get_ipi()
before assigning it to inst->vpu.id?
If the firmware type is not SCP or VCP, mtk_vcodec_fw_get_ipi() returns
-EINVAL. This assigns -22 to inst->vpu.id, which then gets passed down
through vpu_dec_init() into vpu_ipi_register():
drivers/media/platform/mediatek/vpu/mtk_vpu.c:vpu_ipi_register() {
...
if (id < IPI_MAX && handler) {
ipi_desc = vpu->ipi_desc;
ipi_desc[id].name = name;
ipi_desc[id].handler = handler;
ipi_desc[id].priv = priv;
return 0;
}
...
}
Since the bounds check uses a signed comparison, -22 < 7 evaluates to true,
which could lead to an out-of-bounds write to ipi_desc[-22] and corrupt
memory. This same pattern also appears in the slice initialization routines
for AV1, HEVC, VP8, and VP9.
Could this negative ID assignment be prevented by validating the return
value of mtk_vcodec_fw_get_ipi()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=4
^ permalink raw reply
* Re: [PATCH v3 2/4] media: synopsys: hdmirx: add HDMI audio capture support
From: Dmitry Osipenko @ 2026-07-20 1:42 UTC (permalink / raw)
To: Igor Paunovic, Mauro Carvalho Chehab
Cc: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, linux-media, linux-rockchip, linux-arm-kernel,
kernel, devicetree, linux-kernel
In-Reply-To: <20260718085728.6797-3-royalnet026@gmail.com>
On 7/18/26 11:57, Igor Paunovic wrote:
> The Synopsys DesignWare HDMI RX controller extracts the audio stream
> embedded in the incoming HDMI signal and feeds it to an on-SoC I2S
> controller. Expose it as an ALSA capture device by registering the
> generic hdmi-codec as a child of the controller, so that a
> simple-audio-card in the device tree can bind the HDMI RX audio DAI.
>
> The sample rate is recovered from the ACR N/CTS values together with the
> measured TMDS character rate. A periodic worker keeps the local audio
> reference clock locked to the source by nudging it in small ppm steps to
> hold the audio FIFO fill level near its target, which avoids FIFO
> under/overflow and the resulting dropped samples.
>
> Signed-off-by: Igor Paunovic <royalnet026@gmail.com>
> ---
> Changes in v3:
> - restore the v1 audio teardown in remove(): audio_shutdown() already
> stops the worker when the stream closes, so the extra flag clear
> and trailing cancel added in v2 were redundant (Dmitry Osipenko)
> - rename the ACR read locals and add a comment documenting the
> register byte packing
> - drop the get_dai_id stub so OF-graph cards resolve the DAI index
> from the reg property
>
> Changes in v2:
> - register the S/PDIF DAI so the indexes match the binding and reject
> it with -EOPNOTSUPP until wired up (Sebastian Reichel)
> - use platform_device_register_data() and drop the fixed 32-bit DMA
> mask (Dmitry Osipenko)
> - don't leave an ERR_PTR in audio_pdev on registration failure
> - fix teardown ordering in remove()
> - stop the worker before reprogramming shared state in hw_params()
> - look up the "audio" clock by name instead of indexing clks[1]
> - keep the worker on system_unbound_wq when re-arming
>
> .../platform/synopsys/hdmirx/snps_hdmirx.c | 271 ++++++++++++++++++
> .../platform/synopsys/hdmirx/snps_hdmirx.h | 8 +
> 2 files changed, 279 insertions(+)
Works well on 5b, thanks. Only one additional review comment:
Please update hdmirx_suspend() to return -EBUSY if audio_streaming=true.
```
static __maybe_unused int hdmirx_suspend(struct device *dev)
{
struct snps_hdmirx_dev *hdmirx_dev = dev_get_drvdata(dev);
if (READ_ONCE((hdmirx_dev->audio_streaming)))
return -EBUSY;
```
--
Best regards,
Dmitry
^ permalink raw reply
* Re: [PATCH v6 05/14] media: mediatek: vcodec: get share memory address
From: sashiko-bot @ 2026-07-20 1:40 UTC (permalink / raw)
To: Kyrie Wu; +Cc: media-ci, devicetree, robh, conor+dt
In-Reply-To: <20260720012056.1026551-6-kyrie.wu@mediatek.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] mtk_vcodec_fw_map_dm_addr() has inconsistent error semantics across firmware types, leading to NULL or ERR_PTR dereferences.
- [High] mtk_vcodec_vcp_dm_addr() calculates shared memory offsets blindly without verifying they fit within the allocated firmware memory (vsi_size).
Pre-existing issues:
- [High] Firmware-provided sizes vsi->cdf_table_size and vsi->iq_table_size are used for memory allocation and memcpy without validation.
--
--- Patch [5]: [PATCH v6 05/14] media: mediatek: vcodec: get share memory address ---
commit 4c9f9286cbe2e7417003bc83c03e8482665fcca4
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: vcodec: get share memory address
This commit adds support to get shared memory addresses for the VCP
architecture by dividing a single shared memory block into different
functions.
> 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
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
[ ... ]
> @@ -483,8 +483,31 @@ static unsigned int mtk_vcodec_vcp_get_vdec_capa(struct mtk_vcodec_fw *fw)
> 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)
> +static unsigned int mtk_vcodec_vcp_get_venc_capa(struct mtk_vcodec_fw *fw)
> {
> + return 0;
> +}
> +
> +static void *mtk_vcodec_vcp_dm_addr(struct mtk_vcodec_fw *fw, u32 mem_type)
> +{
> + unsigned char *vsi_core = fw->vcp->vsi_core_addr;
> +
> + switch (mem_type) {
> + case ENCODER_MEM:
> + case VCODEC_LAT_MEM:
> + return fw->vcp->vsi_addr;
> + case VCODEC_CORE_MEM:
> + return vsi_core;
> + case VP9_FRAME_MEM:
> + return vsi_core + VCODEC_VSI_LEN;
> + case AV1_CDF_MEM:
> + return vsi_core + VCODEC_VSI_LEN + VP9_FRAME_SIZE;
> + case AV1_IQ_MEM:
> + return vsi_core + VCODEC_VSI_LEN + VP9_FRAME_SIZE + AV1_CDF_SIZE;
[Severity: High]
Are these offsets guaranteed to fit within the memory provided by the
hardware?
When fw->vcp->vsi_size is initialized via get_mem_size() in
mtk_vcodec_vcp_load_firmware(), the driver doesn't seem to validate that
the allocated block is large enough to contain these offsets. If a
malformed device tree or mismatched firmware provides a smaller memory
block, could this lead to out-of-bounds accesses into adjacent memory?
> + default:
> + break;
> + }
> +
> + return NULL;
> +}
[Severity: High]
Is returning NULL here safe when other firmware types return an ERR_PTR?
In drivers like vpu and scp, mapping functions return ERR_PTR(-EINVAL)
on failure. Because this VCP function returns NULL instead, callers have
mixed expectations.
For example, in vdec_av1_slice_init_cdf_table(), the code checks:
remote_cdf_table = mtk_vcodec_fw_map_dm_addr(...);
if (IS_ERR(remote_cdf_table)) {
...
}
If VCP mapping fails and returns NULL, it passes the IS_ERR() check and
can cause a NULL pointer dereference during the subsequent memcpy().
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
[ ... ]
> @@ -775,7 +784,7 @@ static int vdec_av1_slice_init_cdf_table(struct vdec_av1_slice_instance *instanc
> ctx = instance->ctx;
> vsi = instance->vpu.vsi;
> remote_cdf_table = mtk_vcodec_fw_map_dm_addr(ctx->dev->fw_handler,
> - (u32)vsi->cdf_table_addr);
> + vdec_av1_get_cdf_table_addr(ctx, vsi));
> if (IS_ERR(remote_cdf_table)) {
> mtk_vdec_err(ctx, "failed to map cdf table\n");
> return PTR_ERR(remote_cdf_table);
[Severity: High]
This is a pre-existing issue, but does vsi->cdf_table_size need to be
validated against the maximum allowed size before it is used?
Shortly after this mapping, the code does:
memcpy(instance->cdf_table.va, remote_cdf_table, vsi->cdf_table_size);
Since vsi->cdf_table_size is provided by the firmware via shared memory, a
compromised or malformed firmware could provide a size larger than the
AV1_CDF_SIZE (0xFE80) memory region. Could this result in an out-of-bounds
read of adjacent kernel memory?
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
[ ... ]
> @@ -514,7 +523,7 @@ static int vdec_vp9_slice_init_default_frame_ctx(struct vdec_vp9_slice_instance
> return -EINVAL;
>
> remote_frame_ctx = mtk_vcodec_fw_map_dm_addr(ctx->dev->fw_handler,
> - (u32)vsi->default_frame_ctx);
> + vdec_vp9_get_frame_ctx_addr(ctx, vsi));
> if (!remote_frame_ctx) {
> mtk_vdec_err(ctx, "failed to map default frame ctx\n");
> return -EINVAL;
[Severity: High]
Does this !remote_frame_ctx check properly handle the ERR_PTR(-EINVAL) that
can be returned by mtk_vcodec_fw_map_dm_addr() when using VPU or SCP?
Since the VPU and SCP mapping paths return an ERR_PTR on failure, this check
will bypass them, resulting in kmemdup() attempting to dereference the
error pointer:
vdec_vp9_slice_init_default_frame_ctx() {
...
if (!remote_frame_ctx) {
return -EINVAL;
}
...
frame_ctx = kmemdup(remote_frame_ctx, sizeof(*frame_ctx), GFP_KERNEL);
}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=5
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: iio: adc: rockchip-saradc: Add RV1106 compatible
From: Jonathan Cameron @ 2026-07-20 1:39 UTC (permalink / raw)
To: Heiko Stuebner
Cc: Simon Glass, Andy Shevchenko, David Lechner, linux-iio,
devicetree, Conor Dooley, linux-rockchip, Krzysztof Kozlowski,
Nuno Sá, Rob Herring, Jonas Karlman, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel
In-Reply-To: <5684233.Sb9uPGUboI@phil>
On Wed, 15 Jul 2026 21:21:17 +0200
Heiko Stuebner <heiko@sntech.de> wrote:
> Am Dienstag, 14. Juli 2026, 21:16:32 Mitteleuropäische Sommerzeit schrieb Simon Glass:
> > Add the compatible for the SARADC of the Rockchip RV1106, which is
> > compatible with the RK3588 variant.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>
>
Applied to the testing branch of iio.git. Thanks,
J
>
>
^ permalink raw reply
* Re: [PATCH 2/2] iio: magnetometer: add support for QST QMC6308
From: Jonathan Cameron @ 2026-07-20 1:38 UTC (permalink / raw)
To: Jorijn van der Graaf
Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Siratul Islam, Luca Weiss,
linux-iio, devicetree, linux-kernel
In-Reply-To: <20260714202842.340293-3-jorijnvdgraaf@catcrafts.net>
On Tue, 14 Jul 2026 22:28:42 +0200
Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net> wrote:
> The QST QMC6308 is a 3-axis AMR magnetometer on I2C, a single-supply
> 4-pin WLCSP part with no interrupt/DRDY pin, found e.g. in the
> Fairphone 6. Its register map differs from the QMC5883L (chip ID at
> 0x00 instead of 0x0D, data at 0x01..0x06, and the range field living
> in control register 2), so add a separate driver rather than extending
> the QMC5883L driver.
Trim this description down for brevity. Drop the package, and I don't
think we need to mention anything it doesn't have.
>
> Support raw X/Y/Z reads, output data rates 10/50/100/200 Hz, field
> ranges +-30/12/8/2 Gauss, filter oversampling ratios (OSR1) 8/4/2/1,
> the mount matrix, and runtime PM. The second-stage decimation filter
> (OSR2) is left at its power-on default. The package has no DRDY pin,
> so there is no trigger support.
As above I'd not mention what we aren't doing. Also not sure there
is particular value in listing the values each parameter can take.
>
> Run measurements in the chip's periodic "normal" mode paced by the
> DRDY flag rather than in its one-shot "single" mode: the datasheet
> specifies no conversion time that could bound a one-shot wait, while
> normal mode is paced by the specified output data rates, which also
> keeps the sampling_frequency ABI meaningful.
This is a little unusual. Unless single mode is really bad we'd
normally just use that for sysfs reads. Still I don't really mind if
continuous makes more sense here.
>
> Runtime PM puts the chip into its suspend mode after 500 ms without a
> reading, dropping supply current from tens-to-hundreds of microamps to
> 2-3 uA (datasheet Table 2). The suspended chip retains its registers
> and keeps responding on I2C, so resuming only rewrites the mode field
> and discards one stale sample, and configuration changes can be
> applied even while suspended. VDD is left enabled across runtime
> suspend: the on-chip suspend draw is already negligible, and register
> retention is what keeps the resume path trivial.
This is a bit verbose but I suppose some useful info for reviewers in
there. Maybe take another look at cutting it down.
I'm going to assume fable was busy here. Try asking it to be brief
in the patch intro, or just edit it after!
>
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
https://sashiko.dev/#/patchset/20260714202842.340293-1-jorijnvdgraaf%40catcrafts.net
Has a few comments. I may or may not remember to call them out in
this review as well!
Nice driver in general
Just a few minor things from me.
Jonathan
> diff --git a/drivers/iio/magnetometer/qmc6308.c b/drivers/iio/magnetometer/qmc6308.c
> new file mode 100644
> index 000000000000..ceb4b98402bb
> --- /dev/null
> +++ b/drivers/iio/magnetometer/qmc6308.c
> +static int qmc6308_take_measurement(struct iio_dev *indio_dev, int index,
> + int *val)
> +{
> + struct qmc6308_data *data = iio_priv(indio_dev);
> + struct regmap *map = data->regmap;
> + struct device *dev = regmap_get_device(map);
> + unsigned int status;
> + __le16 buf[3];
> + int ret;
> +
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret) {
> + /* EACCES means a read raced runtime PM disable on suspend */
> + if (ret != -EACCES)
> + dev_err(dev, "Failed to power on (%d)\n", ret);
Whilst not a permanent issue, why do we care about hiding the error message? I'd
just print it whatever.
> + return ret;
> + }
> +
> + scoped_guard(mutex, &data->mutex) {
> + /* 50ms headroom over the slowest ODR (10Hz) */
> + ret = regmap_read_poll_timeout(map, QMC6308_REG_STATUS,
> + status,
> + (status & QMC6308_STATUS_DRDY),
> + 2 * USEC_PER_MSEC,
> + 150 * USEC_PER_MSEC);
> + if (ret)
> + goto out_rpm_put;
whilst this is technically not a bug, cleanup.h has some guidance that
was written with maintainability in mind (and a few pointed comments from
Linus Torvalds - no one likes making him grumpy ;)
Upshot (and sashiko has this right) is don't mix goto and anything from
that header.
Here, take a look at:
pm_runtime.h and in particular
PM_RUNTIME_ACQUIRE_AUTOSUSPEND()
Don't worry about keeping it powered up for a tiny bit long than strictly
necessary.
Added bonus is that then you can use guard() rather than scoped guard
for the mutex and return directly on errors.
> +
> + ret = regmap_bulk_read(map, QMC6308_REG_X_LSB, buf,
> + sizeof(buf));
> + if (ret)
> + goto out_rpm_put;
> +
> + if (status & QMC6308_STATUS_OVFL)
> + ret = -ERANGE;
> + }
> +
> +out_rpm_put:
> + pm_runtime_put_autosuspend(dev);
> + if (ret)
> + return ret;
> +
> + *val = (s16)le16_to_cpu(buf[index]);
> +
> + return 0;
> +}
> +
> +static int qmc6308_init(struct qmc6308_data *data)
> +{
> + struct regmap *map = data->regmap;
> + unsigned int reg;
> + int ret;
> +
> + ret = regmap_read(map, QMC6308_REG_ID, ®);
> + if (ret)
> + return ret;
> +
> + /* Allow unknown IDs so that fallback compatibles work */
> + if (reg != QMC6308_CHIP_ID)
> + dev_warn(regmap_get_device(map),
> + "Unknown chip id: 0x%02x, continuing\n", reg);
> +
> + /* The SOFT_RST bit is not auto-cleared and must be written back 0 */
> + ret = regmap_write(map, QMC6308_REG_CTRL2, QMC6308_SOFT_RST);
> + if (ret)
> + return ret;
> +
> + fsleep(QMC6308_POR_US);
> +
> + data->range = QMC6308_RNG_30G;
> +
> + ret = regmap_write(map, QMC6308_REG_CTRL2,
> + FIELD_PREP(QMC6308_SET_RESET_MASK,
> + QMC6308_SET_RESET_ON) |
> + FIELD_PREP(QMC6308_RNG_MASK, data->range));
> + if (ret)
> + return ret;
> +
> + data->odr = QMC6308_ODR_50HZ;
> + data->osr = QMC6308_OSR1_8;
> +
> + return regmap_write(map, QMC6308_REG_CTRL1,
> + FIELD_PREP(QMC6308_MODE_MASK,
> + QMC6308_MODE_NORMAL) |
> + FIELD_PREP(QMC6308_ODR_MASK, data->odr) |
> + FIELD_PREP(QMC6308_OSR1_MASK, data->osr));
I think you mentioned in your discussion with Siratul that you'll explicitly
set all the fields. Sashiko noted that you say you are leaving OSR2 alone
(in the patch description) but then set it to 0.
> +}
> +static int qmc6308_probe(struct i2c_client *client)
> +{
...
> + ret = qmc6308_init(data);
> + if (ret)
> + return dev_err_probe(dev, ret, "qmc6308 init failed\n");
> +
> + pm_runtime_set_active(dev);
> +
> + ret = devm_add_action_or_reset(dev, qmc6308_power_down_action, data);
> + if (ret)
> + return ret;
> +
> + pm_runtime_get_noresume(dev);
We've had a few cases of this recently and there are plenty in tree.
However, it's unnecessary - follow through what happens in dd.c
after probe() is called. Ultimately it checks the counter and if 0
I believe it will power it down without needing this increment / decrement.
Dropping this tends to get sashiko confused, but will resolve the compaint
it has right now about raised reference count on exit.
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_set_autosuspend_delay(dev, QMC6308_AUTOSUSPEND_DELAY_MS);
> + ret = devm_pm_runtime_enable(dev);
> + if (ret)
> + return ret;
> +
> + pm_runtime_put_autosuspend(dev);
As above, this should be unnecessary.
> +
> + return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static int qmc6308_runtime_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct qmc6308_data *data = iio_priv(indio_dev);
> +
> + return qmc6308_set_mode(data, QMC6308_MODE_SUSPEND);
> +}
> +
> +static int qmc6308_runtime_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct qmc6308_data *data = iio_priv(indio_dev);
> + unsigned int status;
> + int ret;
> +
> + ret = qmc6308_set_mode(data, QMC6308_MODE_NORMAL);
> + if (ret)
> + return ret;
> +
> + /*
> + * DRDY may still be set for a sample converted before the last
> + * suspend; clear it so the next read waits for fresh data.
> + */
> + return regmap_read(data->regmap, QMC6308_REG_STATUS, &status);
This feels like a solution in the wrong place. Why not move it
to the read_raw() path after the runtime resume call?
Sure it will add a delay on repeated reads, but they are coming from
sysfs so we don't care that much.
It is fine to do it here because nothing else resumes but none
the less it does feel rather unintuitive. If you really want it
here maybe add a note in the read_raw() code path that says it will
have been cleared if resume occurred.
Also if this fails, you should put device back into suspend mode
so as to end up in a consistent state.
> +}
> +static struct i2c_driver qmc6308_driver = {
> + .driver = {
> + .name = "qmc6308",
> + .of_match_table = qmc6308_match,
> + .pm = pm_ptr(&qmc6308_pm_ops),
> + },
> + .id_table = qmc6308_id,
> + .probe = qmc6308_probe,
> +};
> +module_i2c_driver(qmc6308_driver);
> +
> +MODULE_DESCRIPTION("QST QMC6308 3-Axis Magnetic Sensor driver");
> +MODULE_AUTHOR("Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>");
> +MODULE_LICENSE("Dual BSD/GPL");
Why BSD? Coming from somewhere else or you have particular reason to
prefer that. It is relatively unusual for kernel drivers.
Jonathan
^ permalink raw reply
* Re: [PATCH v6 02/14] media: mediatek: vcodec: add driver to support vcp
From: sashiko-bot @ 2026-07-20 1:36 UTC (permalink / raw)
To: Kyrie Wu; +Cc: conor+dt, robh, devicetree, media-ci
In-Reply-To: <20260720012056.1026551-3-kyrie.wu@mediatek.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Destructive `request_module` polling loop causes a massive modprobe fork bomb and breaks deferred probe.
- [High] A timeout in `mtk_vcodec_vcp_msg_ipi_send()` leaves the system vulnerable to late ACKs, causing host-firmware state desynchronization.
- [High] `ipi_mutex` serializes the entire `wait_event_interruptible_timeout()`, breaking IPI concurrency and risking deadlocks.
- [Medium] Early return in `mtk_vcodec_vcp_release()` leaks remoteproc device reference and breaks teardown state.
- [Medium] Missing IPC message length validation enables potential out-of-bounds reads from untrusted firmware.
- [Low] Unused variable `fw_type` in encoder and decoder probe functions.
--
--- Patch [2]: [PATCH v6 02/14] media: mediatek: vcodec: add driver to support vcp ---
commit 06d72e77d54f36511b46df20abe564b636b8dc79
Author: Kyrie Wu <kyrie.wu@mediatek.com>
Subject: media: mediatek: vcodec: add driver to support vcp
This patch changes the processor from scp to vcp for the mt8196 platform.
It adds a new firmware interface to handle communication between the kernel
and the vcp.
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
[ ... ]
> +static int mtk_vcodec_vcp_msg_process_thread(void *arg)
> +{
[ ... ]
> + 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);
> + continue;
> + }
> +
> + handler(msg, obj->len, fw->vcp->ipi_desc[obj->id].priv);
[Severity: Medium]
Could this pass an arbitrarily large length to the handler?
The firmware-provided obj->len does not appear to be validated against the
boundaries of VCP_SHARE_BUF_SIZE (64 bytes). If the firmware provides an
out-of-bounds length, it might lead to out-of-bounds reads in the receiving
handlers.
> + mtk_vcodec_vcp_ipi_unlock(fw->vcp, obj->id);
[ ... ]
> +static int mtk_vcodec_vcp_msg_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf,
> + unsigned int len, unsigned int wait)
> +{
[ ... ]
> + mutex_lock(&vcp->ipi_mutex);
> + feature_id = VDEC_FEATURE_ID;
> + mailbox_id = IPI_OUT_VDEC_1;
[ ... ]
> +wait_ack:
> + /* wait for VCP's ACK */
> + ret = wait_event_interruptible_timeout(*msg_wq, *msg_signaled,
> + msecs_to_jiffies(IPI_TIMEOUT_MS));
[Severity: High]
Does holding ipi_mutex across this sleep serialize all VCP communication?
Acquiring the mutex before sending the IPC message and holding it while
waiting for the ACK seems to single-thread all communication, which might
defeat the purpose of the per-ID waitqueues.
Also, if any registered IPI handler executing in the receiving msg_thread
needs to send a followup command via mtk_vcodec_vcp_msg_ipi_send(), could
it deadlock trying to acquire ipi_mutex while this thread holds it?
> + if (!ret) {
> + 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]);
[Severity: High]
Can a timeout here lead to state desynchronization with the firmware?
If the wait times out, the function returns an error and releases ipi_mutex
without invalidating the transaction (e.g., via a sequence number).
If the firmware later completes the abandoned task and sends a delayed ACK,
msg_thread will set msg_signaled[id] = true and call wake_up(). If another
thread subsequently reuses this id for a new command, could it be prematurely
awakened by the stale ACK, causing the kernel to reuse or free memory buffers
while the firmware is still accessing them?
[ ... ]
> +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(mtk_vcodec_vcp_driver_loaded(fw), "mtk-vcp")) {
> + if (++retry > retry_cnt) {
> + dev_err(dev, "failed to load mtk-vcp module");
> + return -EPROBE_DEFER;
> + }
> + usleep_range(1000, 2000);
> + }
[Severity: High]
Could this polling loop cause a modprobe fork bomb?
If the mtk-vcp module is missing or unable to load, this loop executes up to
10,000 times, synchronously calling request_module() on each iteration. This
will spawn an excessive number of user-space modprobe processes.
[ ... ]
> +static int mtk_vcodec_vcp_load_firmware(struct mtk_vcodec_fw *fw)
> +{
[ ... ]
> + if (mtk_vcodec_vcp_get_vcp_device(fw) < 0) {
> + dev_err(&fw->pdev->dev, "vcp device is null.\n");
> + return -EINVAL;
> + }
[Severity: High]
Is it intentional to overwrite the -EPROBE_DEFER return value from
mtk_vcodec_vcp_get_vcp_device() with -EINVAL here? This will permanently
break the deferred probe mechanism for this device if the module is simply
not loaded yet.
[ ... ]
> +static void mtk_vcodec_vcp_release(struct mtk_vcodec_fw *fw)
> +{
[ ... ]
> + ret = vcp_device->ops->deregister_feature(vcp_device, fw->vcp->feature_id);
> + if (ret < 0) {
> + dev_err(dev, "deregister feature_id(%d) fail(%d)\n", fw->vcp->feature_id, ret);
> + return;
> + }
> +
> + fw->vcp->is_register_done = false;
> + fw->vcp->is_init_done = false;
> +
> +put_device:
> + mtk_vcodec_vcp_put_device(fw);
> +}
[Severity: Medium]
Will this early return leak the remoteproc device reference?
If deregister_feature fails, returning early skips mtk_vcodec_vcp_put_device(),
which leaves the rproc reference elevated and prevents the remoteproc module
from being cleanly unloaded.
> --- 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,19 +376,22 @@ 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 {
[Severity: Low]
It looks like the fw_type variable is assigned here, but is never used anywhere
else in the function since the firmware selection was refactored to use
fw_init. Is there any remaining need for fw_type?
> --- 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,16 +259,18 @@ 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;
[Severity: Low]
Similarly, fw_type appears to be dead code here as well, since it is no longer
passed to mtk_vcodec_fw_select().
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=2
^ permalink raw reply
* Re: [PATCH v6 01/14] media: mediatek: vcodec: support vcp architecture
From: sashiko-bot @ 2026-07-20 1:36 UTC (permalink / raw)
To: Kyrie Wu; +Cc: conor+dt, media-ci, devicetree, robh
In-Reply-To: <20260720012056.1026551-2-kyrie.wu@mediatek.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] The patch adds support for the VCP architecture but fails to handle the VCP enum in mtk_vcodec_fw_select(), resulting in guaranteed probe failure.
Pre-existing issues:
- [Critical] The driver destroys decode_workqueue before unregistering the video device, allowing concurrent ioctls to queue work to a destroyed workqueue.
- [Critical] The driver state structure (dev) is managed by devm_kzalloc but is accessed by file operations that outlive the device binding, leading to a Use-After-Free.
- [High] The video device allocated via video_device_alloc() leaks on multiple probe error paths.
- [High] Child platform devices created by of_platform_populate() are never cleaned up.
- [High] The ordered workqueue 'dev->core_workqueue' leaks in mtk_vcodec_dec_remove().
--
--- Patch [1]: [PATCH v6 01/14] media: mediatek: vcodec: support vcp architecture ---
commit 86e187a5e23e9a4483af39ff82e14ad66126f95a
Author: Kyrie Wu <kyrie.wu@mediatek.com>
Subject: media: mediatek: vcodec: support vcp architecture
This commit adds support for the VCP architecture for the video codec on
some platforms. It detects the VCP coprocessor in the device tree and sets
the VCP architecture type.
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
> index 300363a40158c..c1642fb09b42a 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
> @@ -14,6 +14,7 @@ struct mtk_vcodec_enc_dev;
> enum mtk_vcodec_fw_type {
> VPU,
> SCP,
> + VCP,
> };
>
> enum mtk_vcodec_fw_use {
[ ... ]
> 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 e936ed8dffbaf..d220b645e4556 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
> @@ -379,6 +379,9 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> } else if (!of_property_read_u32(pdev->dev.of_node, "mediatek,scp",
> &rproc_phandle)) {
> fw_type = SCP;
> + } else if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vcp",
> + &rproc_phandle)) {
> + fw_type = VCP;
[Severity: High]
If the mediatek,vcp property is present, probe sets fw_type = VCP and later
calls mtk_vcodec_fw_select(). However, since mtk_vcodec_fw_select() does not
have a case for VCP, won't it hit the default case and return ERR_PTR(-EINVAL),
causing probe to unconditionally fail?
mtk_vcodec_fw.c:mtk_vcodec_fw_select() {
...
default:
dev_err(&plat_dev->dev, "Invalid vcodec fw type");
return ERR_PTR(-EINVAL);
}
[Severity: High]
This is a pre-existing issue, but does the video device allocated by
video_device_alloc() leak on multiple error paths in mtk_vcodec_probe()?
If initialization steps like v4l2_m2m_init(), alloc_ordered_workqueue(), or
of_platform_populate() fail, the code jumps to error labels like
err_dec_alloc, err_event_workq, or err_reg_cont, which skip calling
video_device_release(vfd_dec):
err_dec_alloc:
v4l2_device_unregister(&dev->v4l2_dev);
[Severity: High]
This isn't a bug introduced by this patch, but are the child platform devices
created by of_platform_populate() in mtk_vcodec_probe() ever cleaned up?
If probe fails after of_platform_populate(), or when the driver is unbound,
mtk_vcodec_dec_remove() executes but appears to never call
of_platform_depopulate() to unregister the child devices. Can this cause state
corruption and resource leaks on subsequent probes?
[Severity: High]
This is a pre-existing issue, but does the ordered workqueue
dev->core_workqueue leak when the driver is unbound?
In mtk_vcodec_probe(), alloc_ordered_workqueue() allocates dev->core_workqueue
for the VDEC_LAT architecture. However, mtk_vcodec_dec_remove() destroys
decode_workqueue but seems to completely omit destroying core_workqueue:
mtk_vcodec_dec_drv.c:mtk_vcodec_dec_remove() {
...
destroy_workqueue(dev->decode_workqueue);
...
}
[Severity: Critical]
This is a pre-existing issue, but is there a use-after-free window during
driver removal?
In mtk_vcodec_dec_remove(), destroy_workqueue(dev->decode_workqueue) is
executed before video_unregister_device(dev->vfd_dec). During this window,
could userspace invoke V4L2 ioctls that call m2mops_vdec_device_run(), which
executes queue_work() on the already freed workqueue?
mtk_vcodec_dec.c:m2mops_vdec_device_run() {
...
queue_work(dev->decode_workqueue, &ctx->decode_work);
}
[Severity: Critical]
This is a pre-existing issue, but can the dev state structure be accessed
after it is freed by devres?
The driver state dev is allocated with devm_kzalloc in mtk_vcodec_probe() and
is automatically freed when mtk_vcodec_dec_remove() returns. If a process
holds /dev/videoX open during unbind, the file descriptor remains valid.
When the application later closes it, fops_vcodec_release() executes and
dereferences the freed dev pointer to acquire dev->dev_mutex:
mtk_vcodec_dec_drv.c:fops_vcodec_release() {
struct mtk_vcodec_dec_dev *dev = video_drvdata(file);
...
mutex_lock(&dev->dev_mutex);
}
> } else {
> dev_dbg(&pdev->dev, "Could not get vdec IPI device");
> return -ENODEV;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=1
^ permalink raw reply
* Re: [PATCH v6 09/14] media: mediatek: vcodec: add decoder compatible to support mt8196
From: sashiko-bot @ 2026-07-20 1:36 UTC (permalink / raw)
To: Kyrie Wu; +Cc: devicetree, robh, media-ci, conor+dt
In-Reply-To: <20260720012056.1026551-10-kyrie.wu@mediatek.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added MT8196 chip was added to the `mtk_vcodec_dec_chip_name` enum, but its V4L2 capability limits were not updated in the stateless decoder's control setup, causing the driver to advertise artificially limited fallback capabilities to userspace.
--
--- Patch [9]: [PATCH v6 09/14] media: mediatek: vcodec: add decoder compatible to support mt8196 ---
commit 33685e9d98f41bca9dad07f1a7aa64db22f658cd
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: vcodec: add decoder compatible to support mt8196
MT8196 is lat single core architecture. Support its compatible and
use `mtk_lat_sig_core_pdata` to initialize platform data.
> 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 15e437323af5a..53d1708b012b7 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
> @@ -29,6 +29,7 @@ enum mtk_vcodec_dec_chip_name {
> MTK_VDEC_MT8188 = 8188,
> MTK_VDEC_MT8192 = 8192,
> MTK_VDEC_MT8195 = 8195,
> + MTK_VDEC_MT8196 = 8196,
[Severity: High]
Does adding this new chip name without updating the stateless decoder's
control limits restrict the MT8196 capabilities to their lowest defaults?
For example, in drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c:
mtk_vcodec_dec_fill_h265_profile() {
switch (ctx->dev->chip_name) {
case MTK_VDEC_MT8188:
case MTK_VDEC_MT8195:
cfg->max = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10;
break;
default:
cfg->max = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE;
break;
}
}
Will this cause the HEVC max profile for MT8196 to fall to the default
MAIN_STILL_PICTURE limit, preventing standard Main profile and 10-bit decoding?
A similar fallback to defaults seems to occur for MT8196 in:
- mtk_vcodec_dec_fill_h265_level() (limits HEVC level to 4 instead of 5.1/5.2)
- mtk_vcodec_dec_fill_vp9_profile() (limits VP9 to Profile 1)
- mtk_vcodec_dec_fill_h264_level() (limits H.264 to Level 4.1)
Should the MT8196 chip be added to the capability limit setups for the stateless
decoder controls?
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=9
^ permalink raw reply
* Re: [PATCH v6 07/14] media: mediatek: vcodec: send share memory address to vcp
From: sashiko-bot @ 2026-07-20 1:32 UTC (permalink / raw)
To: Kyrie Wu; +Cc: conor+dt, devicetree, media-ci, robh
In-Reply-To: <20260720012056.1026551-8-kyrie.wu@mediatek.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unconditional increase in firmware IPC message size may break backwards compatibility with older SCP/VPU firmware.
--
--- Patch [7]: [PATCH v6 07/14] media: mediatek: vcodec: send share memory address to vcp ---
commit 84b64e642819aa5202009e5076e47823645bcbc0
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: vcodec: send share memory address to vcp
This commit updates the AP to VPU initialization message by adding a shared
memory address field. It is used specifically for the VCP architecture where
shared memory is allocated in the kernel rather than reserved in the VPU.
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h b/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
> index 47070be2a9914..097561a1efdcf 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
[ ... ]
> struct vdec_ap_ipi_init {
> uint32_t msg_id;
> u32 codec_type;
> uint64_t ap_inst_addr;
> + u64 shared_iova;
> };
[ ... ]
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> index cdb673e6b4770..3a10b32be0945 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> @@ -236,6 +236,8 @@ int vpu_dec_init(struct vdec_vpu_inst *vpu)
> msg.msg_id = AP_IPIMSG_DEC_INIT;
> msg.ap_inst_addr = (unsigned long)vpu;
> msg.codec_type = vpu->codec_type;
> + if (mtk_vcodec_fw_get_type(vpu->ctx->dev->fw_handler) == VCP)
> + msg.shared_iova = vpu->ctx->dev->fw_handler->vcp->iova_addr;
>
> mtk_vdec_debug(vpu->ctx, "vdec_inst=%p", vpu);
[Severity: Medium]
Does this unconditional size increase break backwards compatibility with older
SCP/VPU firmware?
By adding shared_iova to struct vdec_ap_ipi_init, the struct size increases
from 16 bytes to 24 bytes. Directly after this diff hunk in vpu_dec_init(),
the message is sent to the firmware using the full sizeof(msg):
err = vcodec_vpu_send_msg(vpu, (void *)&msg, sizeof(msg));
If older firmware expects a strictly sized 16-byte initialization message,
could sending 24 bytes unconditionally cause the initialization to fail and
render the video decoder unusable on existing non-VCP devices?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=7
^ 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