* Re: [PATCH 2/2] arm64: dts: socfpga: agilex7-gen2: Add initial device tree
From: Nazle Asmade, Muhammad Nazim Amirul @ 2026-06-24 11:22 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: dinguyen@kernel.org, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <fa79f704-6c04-4052-9625-d371976c14f7@kernel.org>
On 24/6/2026 6:26 pm, Krzysztof Kozlowski wrote:
> On 24/06/2026 12:17, Nazle Asmade, Muhammad Nazim Amirul wrote:
>>> SoC without any interface, serial or storage or network, is close to
>>> useless one.
>>>
>>> I don't see a point in having it in mainline. Serial is usually ABSOLUTE
>>> minimum.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> Hi Krzysztof,
>>
>> Thank you for the review and fast response!
>>
>> I ran both dt_binding_check and dtbs_check (with CHECK_DTBS=y) locally —
>> both passed without errors. Could you clarify which specific test you
>> believe is failing?
>
> I would expect simple-bus schema warning or W=1, because node is placed
> outside of soc@, but maybe there is no such.
> >>
>> Regarding "MMIO goes to MMIO" — are you referring to the GIC
>
> Comments are placed in very specific and intentional place. Please read
> guides how mailing list in-line review works before posting patches.
Apologies on this matter.>
>> (interrupt-controller@7000000) being placed at the root level instead of
>> under the soc bus node?
>>
>> Regarding the serial console — the platform clock driver is not yet
>> upstream, so the UART depends on clkmgr. Would adding the UART with
>> clock-frequency be acceptable as an interim solution?
>
> Add complete working serial. Why can't you use fixed placeholder clock?
> There are probably multiple ways to solve it, not necessary
> clock-frequency and I do not even remember if clock-frequency is allowed.
Will add a UART node with a fixed-clock placeholder>
> But if you cannot bring serial, then my comment stays valid: this is
> unusable upstream thus is not ready to be posted and merged.
>
> Best regards,
> Krzysztof
Sorry I had to reply again to this thread due to not including all
recipient just now. Also one thing to mention, this patch actually a V1
version and latest should be v2 but it is just a device renaming change
on the latest one so the comment still valid. Will send out v3 once it
is ready! Thanks Krzysztof!
BR,
Nazim
^ permalink raw reply
* Re: [PATCH v3] dt-bindings: interrupt-controller: ti,irq-crossbar: Convert to DT schema
From: Konrad Dybcio @ 2026-06-24 11:22 UTC (permalink / raw)
To: Rob Herring (Arm), Bhargav Joshi
Cc: simona.toaca, Krzysztof Kozlowski, m-chawdhry, daniel.baluta,
Thomas Gleixner, Sricharan R, linux-kernel, devicetree,
Conor Dooley, goledhruva
In-Reply-To: <178154975475.1640860.7016352484406221018.robh@kernel.org>
On 6/15/26 11:01 PM, Rob Herring (Arm) wrote:
>
> On Fri, 12 Jun 2026 02:42:29 +0530, Bhargav Joshi wrote:
>> Convert TI irq-crossbar binding from text format to DT schema.
>>
>> As part of conversion following changes are made:
>> - Add '#interrupt-cells' as a required property which was missing in
>> text binding
>> - As irq-crossbar is interrupt-controller. Move binding from
>> bindings/arm/omap to bindings/interrupt-controller
>>
>> Signed-off-by: Bhargav Joshi <j.bhargav.u@gmail.com>
>> ---
>> Changes in v3:
>> - Fixed typo in property description
>> - Link to v2: https://lore.kernel.org/r/20260611-crossbar-v2-1-231d4f88298e@gmail.com
>>
>> Changes in v2:
>> - Dropped property name change and driver updates.
>> - Link to v1: https://lore.kernel.org/r/20260606-crossbar-v1-0-f67f7cb9ee50@gmail.com
>> ---
>> .../devicetree/bindings/arm/omap/crossbar.txt | 55 -------------
>> .../interrupt-controller/ti,irq-crossbar.yaml | 96 ++++++++++++++++++++++
>> 2 files changed, 96 insertions(+), 55 deletions(-)
>>
>
> Applied, thanks!
With:
$ dt-validate --version
2026.4
on next-20260623
I'm getting:
$ LC_ALL=C make ARCH=arm64 LLVM=1 -j24 dt_binding_check
SCHEMA Documentation/devicetree/bindings/processed-schema.json
Traceback (most recent call last):
File "/home/konrad/.local/bin/dt-mk-schema", line 6, in <module>
sys.exit(main())
~~~~^^
File "/home/konrad/.local/share/pipx/venvs/dtschema/lib/python3.14/site-packages/dtschema/mk_schema.py", line 28, in main
schemas = dtschema.DTValidator(args.schemas).schemas
~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
File "/home/konrad/.local/share/pipx/venvs/dtschema/lib/python3.14/site-packages/dtschema/validator.py", line 399, in __init__
self.make_property_type_cache()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
File "/home/konrad/.local/share/pipx/venvs/dtschema/lib/python3.14/site-packages/dtschema/validator.py", line 528, in make_property_type_cache
self.check_duplicate_property_types()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
File "/home/konrad/.local/share/pipx/venvs/dtschema/lib/python3.14/site-packages/dtschema/validator.py", line 522, in check_duplicate_property_types
print(f"{self.schemas[sch_id]['$filename']}: {p}: multiple incompatible types: {v['type']}", file=sys.stderr)
~~~~~~~~~~~~^^^^^^^^
KeyError: 'http://devicetree.org/schemas/interrupt-controller/ti,irq-crossbar.yaml#'
make[2]: *** [Documentation/devicetree/bindings/Makefile:75: Documentation/devicetree/bindings/processed-schema.json] Błąd 1
make[2]: *** Kasuję plik 'Documentation/devicetree/bindings/processed-schema.json'
make[1]: *** [<snip>/linux/Makefile:1672: dt_binding_schemas] Błąd 2
make: *** [Makefile:248: __sub-make] Error 2
Konrad
^ permalink raw reply
* Re: [PATCH 1/2] arm64: dts: qcom: sm8250-xiaomi-elish: Add pm8008 PMIC
From: Konrad Dybcio @ 2026-06-24 11:00 UTC (permalink / raw)
To: Xin Xu; +Cc: andersson, devicetree, konradybcio, linux-arm-msm, linux-kernel
In-Reply-To: <tencent_A1865527015C7AD1B52C56A46AD106234508@qq.com>
On 6/22/26 8:07 PM, Xin Xu wrote:
> On Mon, 2026-06-22 at 13:40 +0200, Konrad Dybcio wrote:
>> On 6/19/26 6:07 PM, Xin Xu wrote:
>>> Add the pm8008 PMIC node on i2c15 with seven LDOs,
>>> using GPIO84 as interrupt and GPIO76 as reset.
>>>
>>> Signed-off-by: Xin Xu <xxsemail@qq.com>
>>> ---
[...]
>>> + pm8008_default: pm8008-default-state {
>>> + int-pins {
>>> + pins = "gpio84";
>>> + function = "gpio";
>>> + bias-disable;
>>> + drive-strength = <2>;
>>> + input-enable;
>>> + };
>>> +
>>> + reset-pins {
>>> + pins = "gpio76";
>>> + function = "gpio";
>>> + bias-pull-up;
>>> + drive-strength = <2>;
>>> + output-high;
>>
>> Drop output-high, the driver will take care of setting the output
>> state
>>
>> Konrad
>
> Thank you for your review!
>
> I will fix the coding style issues (blank line before status,
> interrupts-extended, property order, and dropping output-high)
> in the next version.
>
> I have verified all LDO voltages against the downstream device tree:
> https://github.com/MiCode/kernel_devicetree/tree/elish-r-oss/
> The definitions can be found around lines 209–244 in
> qcom/elish-sm8250-camera-board.dtsi
Thanks. It's best if you grab the DTB from a running device (which
has all the overlays applied etc.), which you can grab from
/sys/firmware/fdt
You can then pass that to DTC directly
Konrad
^ permalink raw reply
* Re: [PATCH v3 12/15] drm/tidss: oldi: Convert OLDI to an aux driver
From: Swamil Jain @ 2026-06-24 10:49 UTC (permalink / raw)
To: Tomi Valkeinen, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Aradhya Bhatia,
Nishanth Menon, Vignesh Raghavendra, Devarsh Thakkar,
Louis Chauvet
Cc: devicetree, dri-devel, linux-kernel, linux-arm-kernel
In-Reply-To: <20260529-beagley-ai-display-v3-12-7fefdc5d1adf@ideasonboard.com>
On 29-05-2026 14:15, Tomi Valkeinen wrote:
> Currently in the DT, OLDI is defined in child nodes under the DSS node.
> The tidss driver will parse the DT, and create DRM bridges for the
> OLDIs, and there are no Linux devices for the OLDIs.
>
> On new SoCs the OLDIs have their own power-domains which we need to
> control. The cleanest way to do this in DT is to add the PDs to the OLDI
> nodes. But this means the OLDI bridge code would somehow have to
> manually manage the PDs, the PDs not being under a Linux device, and
> there isn't much support for that kind of setup in the PD framework.
>
> A solution to this is to convert the OLDI to an auxiliary device/driver,
> created by tidss:
>
> - At module load time the tidss module will, in addition to registering
> the tidss DRM driver, register an oldi auxiliary driver.
> - At probe time tidss will parse the DT, and create an auxiliary device
> for each OLDI.
>
> The aux driver will probe, and as its of_node points to the OLDI node
> containing the PD, the driver framework will take care of enabling and
> disabling the PD when OLDI is used ("used" as in pm_runtime context).
>
> Tested-by: Swamil Jain <s-jain1@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
Reviewed-by: Swamil Jain <s-jain1@ti.com>
> drivers/gpu/drm/tidss/tidss_drv.c | 51 +++-
> drivers/gpu/drm/tidss/tidss_drv.h | 5 +-
> drivers/gpu/drm/tidss/tidss_oldi.c | 502 ++++++++++++++++++++++++++-----------
> drivers/gpu/drm/tidss/tidss_oldi.h | 7 +-
> 4 files changed, 405 insertions(+), 160 deletions(-)
>
> diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
> index 5cb3e746aeb3..aef945101be4 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.c
> +++ b/drivers/gpu/drm/tidss/tidss_drv.c
> @@ -133,10 +133,6 @@ static int tidss_probe(struct platform_device *pdev)
> return ret;
> }
>
> - ret = tidss_oldi_init(tidss);
> - if (ret)
> - return dev_err_probe(dev, ret, "failed to init OLDI\n");
> -
> pm_runtime_enable(dev);
>
> pm_runtime_set_autosuspend_delay(dev, 1000);
> @@ -147,24 +143,30 @@ static int tidss_probe(struct platform_device *pdev)
> dispc_runtime_resume(tidss->dispc);
> #endif
>
> + ret = tidss_oldi_create_devices(tidss);
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to create OLDI devices\n");
> + goto err_runtime_suspend;
> + }
> +
> ret = tidss_modeset_init(tidss);
> if (ret < 0) {
> if (ret != -EPROBE_DEFER)
> dev_err(dev, "failed to init DRM/KMS (%d)\n", ret);
> - goto err_runtime_suspend;
> + goto err_destroy_oldis;
> }
>
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> ret = irq;
> - goto err_runtime_suspend;
> + goto err_destroy_oldis;
> }
> tidss->irq = irq;
>
> ret = tidss_irq_install(ddev, irq);
> if (ret) {
> dev_err(dev, "tidss_irq_install failed: %d\n", ret);
> - goto err_runtime_suspend;
> + goto err_destroy_oldis;
> }
>
> drm_kms_helper_poll_init(ddev);
> @@ -194,6 +196,9 @@ static int tidss_probe(struct platform_device *pdev)
> err_irq_uninstall:
> tidss_irq_uninstall(ddev);
>
> +err_destroy_oldis:
> + tidss_oldi_destroy_devices(tidss);
> +
> err_runtime_suspend:
> #ifndef CONFIG_PM
> dispc_runtime_suspend(tidss->dispc);
> @@ -201,8 +206,6 @@ static int tidss_probe(struct platform_device *pdev)
> pm_runtime_dont_use_autosuspend(dev);
> pm_runtime_disable(dev);
>
> - tidss_oldi_deinit(tidss);
> -
> return ret;
> }
>
> @@ -218,6 +221,8 @@ static void tidss_remove(struct platform_device *pdev)
>
> tidss_irq_uninstall(ddev);
>
> + tidss_oldi_destroy_devices(tidss);
> +
> #ifndef CONFIG_PM
> /* If we don't have PM, we need to call suspend manually */
> dispc_runtime_suspend(tidss->dispc);
> @@ -225,8 +230,6 @@ static void tidss_remove(struct platform_device *pdev)
> pm_runtime_dont_use_autosuspend(dev);
> pm_runtime_disable(dev);
>
> - tidss_oldi_deinit(tidss);
> -
> /* devm allocated dispc goes away with the dev so mark it NULL */
> dispc_remove(tidss);
>
> @@ -262,7 +265,31 @@ static struct platform_driver tidss_platform_driver = {
> },
> };
>
> -drm_module_platform_driver(tidss_platform_driver);
> +static int __init tidss_platform_driver_init(void)
> +{
> + int ret;
> +
> + ret = tidss_oldi_register_driver();
> + if (ret)
> + return ret;
> +
> + ret = drm_platform_driver_register(&tidss_platform_driver);
> + if (ret) {
> + tidss_oldi_unregister_driver();
> + return ret;
> + }
> +
> + return 0;
> +}
> +module_init(tidss_platform_driver_init);
> +
> +static void __exit tidss_platform_driver_exit(void)
> +{
> + platform_driver_unregister(&tidss_platform_driver);
> + tidss_oldi_unregister_driver();
> +}
> +module_exit(tidss_platform_driver_exit);
> +
>
> MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@ti.com>");
> MODULE_DESCRIPTION("TI Keystone DSS Driver");
> diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/tidss/tidss_drv.h
> index e1c1f41d8b4b..d3dba639b278 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.h
> +++ b/drivers/gpu/drm/tidss/tidss_drv.h
> @@ -16,7 +16,8 @@
> #define TIDSS_MAX_OLDI_TXES 2
>
> typedef u32 dispc_irq_t;
> -struct tidss_oldi;
> +
> +struct auxiliary_device;
>
> struct tidss_device {
> struct drm_device ddev; /* DRM device for DSS */
> @@ -34,7 +35,7 @@ struct tidss_device {
> struct drm_plane *planes[TIDSS_MAX_PLANES];
>
> unsigned int num_oldis;
> - struct tidss_oldi *oldis[TIDSS_MAX_OLDI_TXES];
> + struct auxiliary_device *oldis[TIDSS_MAX_OLDI_TXES];
>
> unsigned int irq;
>
> diff --git a/drivers/gpu/drm/tidss/tidss_oldi.c b/drivers/gpu/drm/tidss/tidss_oldi.c
> index e925ddaa4fd6..188f31b57c6a 100644
> --- a/drivers/gpu/drm/tidss/tidss_oldi.c
> +++ b/drivers/gpu/drm/tidss/tidss_oldi.c
> @@ -5,11 +5,13 @@
> * Aradhya Bhatia <a-bhatia1@ti.com>
> */
>
> +#include <linux/auxiliary_bus.h>
> #include <linux/clk.h>
> #include <linux/of.h>
> #include <linux/of_graph.h>
> #include <linux/mfd/syscon.h>
> #include <linux/media-bus-format.h>
> +#include <linux/pm_runtime.h>
> #include <linux/regmap.h>
>
> #include <drm/drm_atomic_helper.h>
> @@ -20,6 +22,12 @@
> #include "tidss_dispc_regs.h"
> #include "tidss_oldi.h"
>
> +static DEFINE_IDA(tidss_oldi_ida);
> +
> +struct tidss_oldi_platform_data {
> + struct tidss_device *tidss;
> +};
> +
> struct tidss_oldi {
> struct tidss_device *tidss;
> struct device *dev;
> @@ -251,6 +259,8 @@ static void tidss_oldi_atomic_pre_enable(struct drm_bridge *bridge,
> if (oldi->link_type == OLDI_MODE_SECONDARY_CLONE_SINGLE_LINK)
> return;
>
> + WARN_ON(pm_runtime_get_sync(oldi->dev) < 0);
> +
> connector = drm_atomic_get_new_connector_for_encoder(state,
> bridge->encoder);
> if (WARN_ON(!connector))
> @@ -296,6 +306,8 @@ static void tidss_oldi_atomic_post_disable(struct drm_bridge *bridge,
>
> /* Clear OLDI Config */
> tidss_disable_oldi(oldi->tidss, oldi->parent_vp);
> +
> + pm_runtime_put_autosuspend(oldi->dev);
> }
>
> #define MAX_INPUT_SEL_FORMATS 1
> @@ -383,12 +395,16 @@ static int get_oldi_mode(struct device_node *oldi_tx, int *companion_instance)
> */
> return OLDI_MODE_SINGLE_LINK;
>
> - if (of_property_read_u32(companion, "reg", &companion_reg))
> + if (of_property_read_u32(companion, "reg", &companion_reg)) {
> + of_node_put(companion);
> return OLDI_MODE_UNSUPPORTED;
> + }
>
> - if (companion_reg > (TIDSS_MAX_OLDI_TXES - 1))
> + if (companion_reg > (TIDSS_MAX_OLDI_TXES - 1)) {
> /* Invalid companion OLDI reg value. */
> + of_node_put(companion);
> return OLDI_MODE_UNSUPPORTED;
> + }
>
> *companion_instance = (int)companion_reg;
>
> @@ -464,174 +480,372 @@ static int get_parent_dss_vp(struct device_node *oldi_tx, u32 *parent_vp)
> return -ENODEV;
> }
>
> -void tidss_oldi_deinit(struct tidss_device *tidss)
> +static int tidss_oldi_probe(struct auxiliary_device *auxdev,
> + const struct auxiliary_device_id *id)
> {
> - for (int i = 0; i < tidss->num_oldis; i++) {
> - if (tidss->oldis[i]) {
> - drm_bridge_remove(&tidss->oldis[i]->bridge);
> - tidss->is_ext_vp_clk[tidss->oldis[i]->parent_vp] = false;
> - tidss->oldis[i] = NULL;
> + struct device *dev = &auxdev->dev;
> + struct tidss_oldi_platform_data *oldi_pdata = dev_get_platdata(dev);
> + struct tidss_device *tidss = oldi_pdata->tidss;
> + struct device_node *node = auxdev->dev.of_node;
> + enum tidss_oldi_link_type link_type = OLDI_MODE_UNSUPPORTED;
> + int companion_instance = -1;
> + struct drm_bridge *bridge;
> + struct device_link *link;
> + struct tidss_oldi *oldi;
> + u32 oldi_instance;
> + u32 parent_vp;
> + int ret;
> +
> + ret = of_property_read_u32(node, "reg", &oldi_instance);
> + if (ret)
> + return ret;
> +
> + ret = get_parent_dss_vp(node, &parent_vp);
> + if (ret)
> + return ret;
> +
> + /*
> + * Now that it's confirmed that OLDI is connected with DSS,
> + * let's continue getting the OLDI sinks ahead and other OLDI
> + * properties.
> + */
> + bridge = devm_drm_of_get_bridge(dev, node, OLDI_OUTPUT_PORT, 0);
> + if (IS_ERR(bridge)) {
> + /*
> + * Either there was no OLDI sink in the devicetree, or the OLDI
> + * sink has not been added yet. In any case, return.
> + */
> + return dev_err_probe(dev, PTR_ERR(bridge),
> + "no panel/bridge for OLDI%u.\n",
> + oldi_instance);
> + }
> +
> + link_type = get_oldi_mode(node, &companion_instance);
> + if (link_type == OLDI_MODE_UNSUPPORTED) {
> + return dev_err_probe(dev, -EINVAL,
> + "OLDI%u: Unsupported OLDI connection.\n",
> + oldi_instance);
> + } else if ((link_type == OLDI_MODE_SECONDARY_CLONE_SINGLE_LINK) ||
> + (link_type == OLDI_MODE_CLONE_SINGLE_LINK)) {
> + /*
> + * The OLDI driver cannot support OLDI clone mode properly at
> + * present. The clone mode requires 2 working encoder-bridge
> + * pipelines, generating from the same crtc. The DRM framework
> + * does not support this at present. If there were to be, say, 2
> + * OLDI sink bridges each connected to an OLDI TXes, they
> + * couldn't both be supported simultaneously. This driver still
> + * has some code pertaining to OLDI clone mode configuration in
> + * DSS hardware for future, when there is a better
> + * infrastructure in the DRM framework to support 2
> + * encoder-bridge pipelines simultaneously. Till that time, this
> + * driver shall error out if it detects a clone mode
> + * configuration.
> + */
> + return dev_err_probe(dev, -EOPNOTSUPP,
> + "The OLDI driver does not support Clone Mode at present.\n");
> + }
> +
> + oldi = devm_drm_bridge_alloc(dev, struct tidss_oldi, bridge,
> + &tidss_oldi_bridge_funcs);
> + if (IS_ERR(oldi))
> + return PTR_ERR(oldi);
> +
> + oldi->parent_vp = parent_vp;
> + oldi->oldi_instance = oldi_instance;
> + oldi->companion_instance = companion_instance;
> + oldi->link_type = link_type;
> + oldi->dev = dev;
> + oldi->next_bridge = bridge;
> + oldi->tidss = tidss;
> +
> + auxiliary_set_drvdata(auxdev, oldi);
> +
> + /*
> + * Only the primary OLDI needs to reference the io-ctrl system
> + * registers, and the serial clock.
> + * We don't require a check for secondary OLDI in dual-link mode
> + * because the driver will not create a drm_bridge instance.
> + * But the driver will need to create a drm_bridge instance,
> + * for secondary OLDI in clone mode (once it is supported).
> + */
> + if (link_type != OLDI_MODE_SECONDARY_DUAL_LINK &&
> + link_type != OLDI_MODE_SECONDARY_CLONE_SINGLE_LINK) {
> + oldi->io_ctrl = syscon_regmap_lookup_by_phandle(node,
> + "ti,oldi-io-ctrl");
> + if (IS_ERR(oldi->io_ctrl)) {
> + return dev_err_probe(oldi->dev, PTR_ERR(oldi->io_ctrl),
> + "OLDI%u: syscon_regmap_lookup_by_phandle failed.\n",
> + oldi_instance);
> + }
> +
> + oldi->serial = of_clk_get_by_name(node, "serial");
> + if (IS_ERR(oldi->serial)) {
> + return dev_err_probe(oldi->dev, PTR_ERR(oldi->serial),
> + "OLDI%u: Failed to get serial clock.\n",
> + oldi_instance);
> }
> }
> +
> + if (link_type != OLDI_MODE_SECONDARY_DUAL_LINK) {
> + /* Register the bridge. */
> + oldi->bridge.of_node = node;
> + oldi->bridge.driver_private = oldi;
> +
> + tidss->is_ext_vp_clk[oldi->parent_vp] = true;
> +
> + drm_bridge_add(&oldi->bridge);
> + }
> +
> + link = device_link_add(&auxdev->dev, tidss->dev,
> + DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_CONSUMER);
> + if (!link) {
> + ret = -EINVAL;
> + goto err_bridge_remove;
> + }
> +
> + pm_runtime_enable(dev);
> + pm_runtime_set_autosuspend_delay(dev, 500);
> + pm_runtime_use_autosuspend(dev);
> +
> + return 0;
> +
> +err_bridge_remove:
> + if (link_type != OLDI_MODE_SECONDARY_DUAL_LINK) {
> + drm_bridge_remove(&oldi->bridge);
> + tidss->is_ext_vp_clk[oldi->parent_vp] = false;
> + }
> +
> + clk_put(oldi->serial);
> +
> + return ret;
> }
>
> -int tidss_oldi_init(struct tidss_device *tidss)
> +static void tidss_oldi_remove(struct auxiliary_device *auxdev)
> {
> - struct tidss_oldi *oldi;
> - struct device_node *child;
> - struct drm_bridge *bridge;
> - u32 parent_vp, oldi_instance;
> - int companion_instance = -1;
> - enum tidss_oldi_link_type link_type = OLDI_MODE_UNSUPPORTED;
> - struct device_node *oldi_parent;
> - int ret = 0;
> + struct tidss_oldi *oldi = auxiliary_get_drvdata(auxdev);
> + struct tidss_device *tidss = oldi->tidss;
> + struct device *dev = &auxdev->dev;
>
> - tidss->num_oldis = 0;
> + pm_runtime_dont_use_autosuspend(dev);
> + pm_runtime_disable(dev);
>
> - oldi_parent = of_get_child_by_name(tidss->dev->of_node, "oldi-transmitters");
> - if (!oldi_parent)
> - /* Return gracefully */
> - return 0;
> + if (oldi->link_type != OLDI_MODE_SECONDARY_DUAL_LINK) {
> + drm_bridge_remove(&oldi->bridge);
> +
> + tidss->is_ext_vp_clk[oldi->parent_vp] = false;
> + }
> +
> + clk_put(oldi->serial);
> +}
> +
> +static const struct auxiliary_device_id tidss_oldi_aux_id_table[] = {
> + { .name = "tidss.oldi" },
> + {}
> +};
> +
> +static struct auxiliary_driver oldi_aux_driver = {
> + .name = "oldi",
> + .probe = tidss_oldi_probe,
> + .remove = tidss_oldi_remove,
> + .id_table = tidss_oldi_aux_id_table,
> +};
> +
> +static void tidss_oldi_aux_device_release(struct device *dev)
> +{
> + struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
> +
> + ida_free(&tidss_oldi_ida, auxdev->id);
> +
> + of_node_put(auxdev->dev.of_node);
>
> - for_each_available_child_of_node(oldi_parent, child) {
> - ret = get_parent_dss_vp(child, &parent_vp);
> - if (ret) {
> - if (ret == -ENODEV) {
> - /*
> - * ENODEV means that this particular OLDI node
> - * is not connected with the DSS, which is not
> - * a harmful case. There could be another OLDI
> - * which may still be connected.
> - * Continue to search for that.
> - */
> - continue;
> + kfree(auxdev->dev.platform_data);
> + kfree(auxdev);
> +}
> +
> +static struct auxiliary_device *
> +tidss_oldi_create_device(struct tidss_device *tidss,
> + struct device_node *oldi_tx)
> +{
> + struct tidss_oldi_platform_data *oldi_pdata;
> + struct auxiliary_device *companion_auxdev;
> + struct auxiliary_device *auxdev;
> + struct device_link *link = NULL;
> + u32 oldi_aux_id;
> + int ret;
> +
> + /*
> + * Allocate the ID first, so that we get a lower ID for the primary
> + * OLDI, instead of the companion grabbing it in the call below. Note
> + * that the ID allocated here often matches the OLDI hardware index,
> + * but not always.
> + * The OLDI hardware index cannot be used as an ID, as, say, OLDI 1 on
> + * two DSS instances would produce the exact same device name
> + * ("tidss.oldi.1").
> + */
> + ret = ida_alloc(&tidss_oldi_ida, GFP_KERNEL);
> + if (ret < 0)
> + return ERR_PTR(ret);
> +
> + oldi_aux_id = ret;
> +
> + companion_auxdev = NULL;
> +
> + /*
> + * If this is the primary OLDI and there is a companion, create the
> + * auxdev for the secondary OLDI first, as the secondary will act as
> + * a supplier for the primary OLDI.
> + */
> + if (!of_property_read_bool(oldi_tx, "ti,secondary-oldi")) {
> + struct device_node *companion_node;
> +
> + companion_node = of_parse_phandle(oldi_tx, "ti,companion-oldi", 0);
> + if (companion_node) {
> + companion_auxdev =
> + tidss_oldi_create_device(tidss, companion_node);
> +
> + of_node_put(companion_node);
> +
> + if (IS_ERR(companion_auxdev)) {
> + dev_err(tidss->dev,
> + "Failed to create secondary oldi device\n");
> + ret = PTR_ERR(companion_auxdev);
> + goto err_free_ida;
> }
> - goto err_put_node;
> }
> + }
>
> - ret = of_property_read_u32(child, "reg", &oldi_instance);
> - if (ret)
> - goto err_put_node;
> + oldi_pdata = kzalloc_obj(*oldi_pdata);
> + if (!oldi_pdata) {
> + ret = -ENOMEM;
> + goto err_free_ida;
> + }
>
> - /*
> - * Now that it's confirmed that OLDI is connected with DSS,
> - * let's continue getting the OLDI sinks ahead and other OLDI
> - * properties.
> - */
> - bridge = devm_drm_of_get_bridge(tidss->dev, child,
> - OLDI_OUTPUT_PORT, 0);
> - if (IS_ERR(bridge)) {
> - /*
> - * Either there was no OLDI sink in the devicetree, or
> - * the OLDI sink has not been added yet. In any case,
> - * return.
> - * We don't want to have an OLDI node connected to DSS
> - * but not to any sink.
> - */
> - ret = dev_err_probe(tidss->dev, PTR_ERR(bridge),
> - "no panel/bridge for OLDI%u.\n",
> - oldi_instance);
> - goto err_put_node;
> - }
> + oldi_pdata->tidss = tidss;
>
> - link_type = get_oldi_mode(child, &companion_instance);
> - if (link_type == OLDI_MODE_UNSUPPORTED) {
> - ret = dev_err_probe(tidss->dev, -EINVAL,
> - "OLDI%u: Unsupported OLDI connection.\n",
> - oldi_instance);
> - goto err_put_node;
> - } else if ((link_type == OLDI_MODE_SECONDARY_CLONE_SINGLE_LINK) ||
> - (link_type == OLDI_MODE_CLONE_SINGLE_LINK)) {
> - /*
> - * The OLDI driver cannot support OLDI clone mode
> - * properly at present.
> - * The clone mode requires 2 working encoder-bridge
> - * pipelines, generating from the same crtc. The DRM
> - * framework does not support this at present. If
> - * there were to be, say, 2 OLDI sink bridges each
> - * connected to an OLDI TXes, they couldn't both be
> - * supported simultaneously.
> - * This driver still has some code pertaining to OLDI
> - * clone mode configuration in DSS hardware for future,
> - * when there is a better infrastructure in the DRM
> - * framework to support 2 encoder-bridge pipelines
> - * simultaneously.
> - * Till that time, this driver shall error out if it
> - * detects a clone mode configuration.
> - */
> - ret = dev_err_probe(tidss->dev, -EOPNOTSUPP,
> - "The OLDI driver does not support Clone Mode at present.\n");
> - goto err_put_node;
> - } else if (link_type == OLDI_MODE_SECONDARY_DUAL_LINK) {
> - /*
> - * This is the secondary OLDI node, which serves as a
> - * companion to the primary OLDI, when it is configured
> - * for the dual-link mode. Since the primary OLDI will
> - * be a part of bridge chain, no need to put this one
> - * too. Continue onto the next OLDI node.
> - */
> - continue;
> - }
> + auxdev = kzalloc_obj(*auxdev);
> + if (!auxdev) {
> + ret = -ENOMEM;
> + goto err_free_pdata;
> + }
> +
> + *auxdev = (struct auxiliary_device) {
> + .name = "oldi",
> + .id = oldi_aux_id,
> + .dev = {
> + .parent = tidss->dev,
> + .of_node = of_node_get(oldi_tx),
> + .release = tidss_oldi_aux_device_release,
> + .platform_data = oldi_pdata,
> + },
> + };
> +
> + ret = auxiliary_device_init(auxdev);
> + if (ret) {
> + dev_err(tidss->dev, "OLDI auxiliary_device_init failed: %d\n",
> + ret);
> + goto err_free_auxdev;
> + }
>
> - oldi = devm_drm_bridge_alloc(tidss->dev, struct tidss_oldi, bridge,
> - &tidss_oldi_bridge_funcs);
> - if (IS_ERR(oldi)) {
> - ret = PTR_ERR(oldi);
> - goto err_put_node;
> + /*
> + * Create a device-link between the primary and the secondary, so that
> + * the secondary will be powered on when the primary is used.
> + */
> + if (companion_auxdev) {
> + link = device_link_add(&auxdev->dev, &companion_auxdev->dev,
> + DL_FLAG_PM_RUNTIME |
> + DL_FLAG_AUTOREMOVE_CONSUMER |
> + DL_FLAG_AUTOREMOVE_SUPPLIER);
> + if (!link) {
> + dev_err(tidss->dev,
> + "device_link_add failed between primary and secondary OLDI\n");
> + ret = -EINVAL;
> + goto err_uninit_auxdev;
> }
> + }
>
> - oldi->parent_vp = parent_vp;
> - oldi->oldi_instance = oldi_instance;
> - oldi->companion_instance = companion_instance;
> - oldi->link_type = link_type;
> - oldi->dev = tidss->dev;
> - oldi->next_bridge = bridge;
> + ret = auxiliary_device_add(auxdev);
> + if (ret) {
> + dev_err(tidss->dev, "OLDI auxiliary_device_add failed: %d\n",
> + ret);
> + goto err_link_del;
> + }
>
> - /*
> - * Only the primary OLDI needs to reference the io-ctrl system
> - * registers, and the serial clock.
> - * We don't require a check for secondary OLDI in dual-link mode
> - * because the driver will not create a drm_bridge instance.
> - * But the driver will need to create a drm_bridge instance,
> - * for secondary OLDI in clone mode (once it is supported).
> - */
> - if (link_type != OLDI_MODE_SECONDARY_CLONE_SINGLE_LINK) {
> - oldi->io_ctrl = syscon_regmap_lookup_by_phandle(child,
> - "ti,oldi-io-ctrl");
> - if (IS_ERR(oldi->io_ctrl)) {
> - ret = dev_err_probe(oldi->dev, PTR_ERR(oldi->io_ctrl),
> - "OLDI%u: syscon_regmap_lookup_by_phandle failed.\n",
> - oldi_instance);
> - goto err_put_node;
> - }
> + tidss->oldis[tidss->num_oldis++] = auxdev;
>
> - oldi->serial = of_clk_get_by_name(child, "serial");
> - if (IS_ERR(oldi->serial)) {
> - ret = dev_err_probe(oldi->dev, PTR_ERR(oldi->serial),
> - "OLDI%u: Failed to get serial clock.\n",
> - oldi_instance);
> - goto err_put_node;
> - }
> - }
> + return auxdev;
>
> - /* Register the bridge. */
> - oldi->bridge.of_node = child;
> - oldi->bridge.driver_private = oldi;
> +err_link_del:
> + if (link)
> + device_link_del(link);
> +err_uninit_auxdev:
> + auxiliary_device_uninit(auxdev);
> + /* return here, as the rest are done in auxdev's release */
> + return ERR_PTR(ret);
>
> - tidss->oldis[tidss->num_oldis++] = oldi;
> - tidss->is_ext_vp_clk[oldi->parent_vp] = true;
> - oldi->tidss = tidss;
> +err_free_auxdev:
> + kfree(auxdev);
> +err_free_pdata:
> + kfree(oldi_pdata);
> +err_free_ida:
> + ida_free(&tidss_oldi_ida, oldi_aux_id);
>
> - drm_bridge_add(&oldi->bridge);
> + return ERR_PTR(ret);
> +}
> +
> +int tidss_oldi_create_devices(struct tidss_device *tidss)
> +{
> + struct device_node *oldi_txes;
> + struct device_node *oldi_tx;
> + int ret;
> +
> + oldi_txes = of_get_child_by_name(tidss->dev->of_node,
> + "oldi-transmitters");
> + if (!oldi_txes)
> + return 0;
> +
> + /*
> + * Look for primary OLDIs and create devices for them. For dual-link
> + * cases, the primary's create_device call will also create the
> + * secondary device.
> + */
> + for_each_available_child_of_node(oldi_txes, oldi_tx) {
> + struct auxiliary_device *auxdev;
> +
> + if (of_property_read_bool(oldi_tx, "ti,secondary-oldi"))
> + continue;
> +
> + auxdev = tidss_oldi_create_device(tidss, oldi_tx);
> + if (IS_ERR(auxdev)) {
> + ret = PTR_ERR(auxdev);
> + goto err_destroy_oldis;
> + }
> }
>
> - of_node_put(child);
> - of_node_put(oldi_parent);
> + of_node_put(oldi_txes);
>
> return 0;
>
> -err_put_node:
> - of_node_put(child);
> - of_node_put(oldi_parent);
> +err_destroy_oldis:
> + tidss_oldi_destroy_devices(tidss);
> +
> + of_node_put(oldi_tx);
> + of_node_put(oldi_txes);
> +
> return ret;
> }
> +
> +void tidss_oldi_destroy_devices(struct tidss_device *tidss)
> +{
> + for (unsigned int i = 0; i < tidss->num_oldis; ++i)
> + auxiliary_device_destroy(tidss->oldis[i]);
> +}
> +
> +int tidss_oldi_register_driver(void)
> +{
> + return auxiliary_driver_register(&oldi_aux_driver);
> +}
> +
> +void tidss_oldi_unregister_driver(void)
> +{
> + auxiliary_driver_unregister(&oldi_aux_driver);
> +}
> diff --git a/drivers/gpu/drm/tidss/tidss_oldi.h b/drivers/gpu/drm/tidss/tidss_oldi.h
> index a361e6dbfce3..2069bd46aaae 100644
> --- a/drivers/gpu/drm/tidss/tidss_oldi.h
> +++ b/drivers/gpu/drm/tidss/tidss_oldi.h
> @@ -36,7 +36,10 @@ enum tidss_oldi_link_type {
> OLDI_MODE_SECONDARY_DUAL_LINK,
> };
>
> -int tidss_oldi_init(struct tidss_device *tidss);
> -void tidss_oldi_deinit(struct tidss_device *tidss);
> +int tidss_oldi_create_devices(struct tidss_device *tidss);
> +void tidss_oldi_destroy_devices(struct tidss_device *tidss);
> +
> +int tidss_oldi_register_driver(void);
> +void tidss_oldi_unregister_driver(void);
>
> #endif /* __TIDSS_OLDI_H__ */
>
^ permalink raw reply
* Re: [PATCH v5 11/12] hwmon: spd5118: Add I3C support
From: sashiko-bot @ 2026-06-24 10:49 UTC (permalink / raw)
To: Akhil R; +Cc: devicetree, linux-i3c, linux-hwmon, robh, Frank.Li, conor+dt
In-Reply-To: <20260624102153.1770072-12-akhilrajeev@nvidia.com>
> Add a regmap config and a probe function to support I3C-based
> communication with SPD5118 devices.
>
> On an I3C bus, SPD5118 devices are enumerated via SETAASA and always
> require an ACPI or device tree entry. Device matching is hence through
> the OF match tables only and does not need an I3C class match table. The
> device identity is verified in the type registers before proceeding to
> the common probe function.
>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624102153.1770072-1-akhilrajeev@nvidia.com?part=11
^ permalink raw reply
* Re: [PATCH v5 02/12] i3c: master: Use unified device property interface
From: sashiko-bot @ 2026-06-24 10:48 UTC (permalink / raw)
To: Akhil R; +Cc: devicetree, linux-hwmon, Frank.Li, linux-i3c, conor+dt, robh
In-Reply-To: <20260624102153.1770072-3-akhilrajeev@nvidia.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Fwnode references acquired for I2C and I3C boardinfo entries are permanently leaked.
Pre-existing issues:
- [High] Dangling `desc->dev` pointer causes Use-After-Free and Double Free when device registration fails.
--
commit cf59bb0f706cf267c9ca158815835766a81fc214
Author: Akhil R <akhilrajeev@nvidia.com>
i3c: master: Use unified device property interface
Replace all OF-specific functions with unified device property functions
as a prerequisite to support both ACPI and device tree.
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index f1be38a640ca1..b20f56f7b68e8 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
[ ... ]
> @@ -1928,7 +1930,8 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
> desc->info.pid);
>
> if (desc->boardinfo)
> - desc->dev->dev.of_node = desc->boardinfo->of_node;
> + device_set_node(&desc->dev->dev,
> + fwnode_handle_get(desc->boardinfo->fwnode));
[Severity: High]
Does this take an extra reference to the fwnode that ends up leaking?
The original code simply assigned the pointer, but the new code calls
fwnode_handle_get(), acquiring an additional reference. When the device is
subsequently released via i3c_device_release(), it will drop this second
reference.
Since the original reference stored in desc->boardinfo->fwnode does not appear
to be dropped during master teardown, does this cause the initial reference to
permanently leak on the normal success path?
> ret = device_register(&desc->dev->dev);
> if (ret) {
[Severity: High]
This isn't a bug introduced by this patch, but does this error path leave a
dangling pointer in desc->dev?
If device_register() fails, put_device(&desc->dev->dev) is called, which drops
the reference count to 0 and invokes i3c_device_release(). The release
function frees desc->dev (the i3cdev structure). However, desc->dev is not set
to NULL.
Later, when i3c_master_unregister_i3c_devs() iterates through the bus,
it will dereference the dangling pointer:
drivers/i3c/master.c:i3c_master_unregister_i3c_devs() {
...
i3c_bus_for_each_i3cdev(&master->bus, i3cdev) {
if (!i3cdev->dev)
continue;
i3cdev->dev->desc = NULL;
if (device_is_registered(&i3cdev->dev->dev))
device_unregister(&i3cdev->dev->dev);
else
put_device(&i3cdev->dev->dev);
i3cdev->dev = NULL;
}
...
}
Could this lead to a use-after-free and double-free when it evaluates
i3cdev->dev and attempts to call put_device() again?
[ ... ]
> @@ -2649,14 +2656,14 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
> boardinfo->lvr = reg[2];
>
> list_add_tail(&boardinfo->node, &master->boardinfo.i2c);
> - of_node_get(node);
> + fwnode_handle_get(fwnode);
>
> return 0;
> }
[Severity: High]
Does this code leak the fwnode reference?
The fwnode_handle_get(fwnode) call increments the reference count, but the
resulting reference is neither stored in boardinfo nor dropped before the
function returns. This appears to leak the reference immediately on the
success path.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624102153.1770072-1-akhilrajeev@nvidia.com?part=2
^ permalink raw reply
* Re: [PATCH v3 11/15] drm/tidss: oldi: Fix OLDI signal polarities
From: Swamil Jain @ 2026-06-24 10:47 UTC (permalink / raw)
To: Tomi Valkeinen, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Aradhya Bhatia,
Nishanth Menon, Vignesh Raghavendra, Devarsh Thakkar,
Louis Chauvet
Cc: devicetree, dri-devel, linux-kernel, linux-arm-kernel
In-Reply-To: <20260529-beagley-ai-display-v3-11-7fefdc5d1adf@ideasonboard.com>
On 29-05-2026 14:15, Tomi Valkeinen wrote:
> OLDI has a few issues with how it handles the signal polarities:
>
> - It always sets OLDI_DEPOL, which means DE active low
> - It sets DRM_BUS_FLAG_DE_HIGH in struct drm_bridge_timings, i.e.
> reverse to the OLDI_DEPOL
> - It sets DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE in struct drm_bridge_timings,
> but the TRM says "The DSS interface is clocked on the rising edge of
> OLDI_FWD_P_CLK pixel clock", which I read as "OLDI samples on rising
> edge".
> - But the defined drm_bridge_timings is not actually used anywhere, even
> if it is set to bridge->timings, so the bus flags are just ignored.
>
> However, based on my testing, OLDI_DEPOL bit or the edge on which data
> and syncs are driven doesn't seem to affect the OLDI output. Possibly
> it's not as robust, but I did not see any effect with an oscilloscope.
>
> However, the code is still quite broken, so let's fix it:
> - Remove drm_bridge_timings
> - Set the correct input_bus_cfg.flags in tidss_oldi_atomic_check()
> - Set OLDI_DEPOL based on the DE bus flag
>
> Tested-by: Swamil Jain <s-jain1@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
Reviewed-by: Swamil Jain <s-jain1@ti.com>
> drivers/gpu/drm/tidss/tidss_oldi.c | 38 ++++++++++++++++++++++++++++----------
> 1 file changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/tidss/tidss_oldi.c b/drivers/gpu/drm/tidss/tidss_oldi.c
> index 17c535bfa057..e925ddaa4fd6 100644
> --- a/drivers/gpu/drm/tidss/tidss_oldi.c
> +++ b/drivers/gpu/drm/tidss/tidss_oldi.c
> @@ -164,7 +164,8 @@ static void tidss_oldi_tx_power(struct tidss_oldi *oldi, bool enable)
> regmap_update_bits(oldi->io_ctrl, OLDI_PD_CTRL, mask, enable ? 0 : mask);
> }
>
> -static int tidss_oldi_config(struct tidss_oldi *oldi)
> +static int tidss_oldi_config(struct tidss_oldi *oldi,
> + struct drm_bridge_state *bridge_state)
> {
> const struct oldi_bus_format *bus_fmt = NULL;
> u32 oldi_cfg = 0;
> @@ -183,7 +184,8 @@ static int tidss_oldi_config(struct tidss_oldi *oldi)
> "OLDI%u: DSS port width %d not supported\n",
> oldi->oldi_instance, bus_fmt->data_width);
>
> - oldi_cfg |= OLDI_DEPOL;
> + if (bridge_state->input_bus_cfg.flags & DRM_BUS_FLAG_DE_LOW)
> + oldi_cfg |= OLDI_DEPOL; /* 1 = active low */
>
> oldi_cfg = (oldi_cfg & (~OLDI_MAP)) | (bus_fmt->oldi_mode_reg_val << 1);
>
> @@ -220,6 +222,22 @@ static int tidss_oldi_config(struct tidss_oldi *oldi)
> return ret;
> }
>
> +static int tidss_oldi_atomic_check(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state)
> +{
> + bridge_state->input_bus_cfg.flags &=
> + ~(DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
> + DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE);
> +
> + bridge_state->input_bus_cfg.flags |=
> + DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE |
> + DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE;
> +
> + return 0;
> +}
> +
> static void tidss_oldi_atomic_pre_enable(struct drm_bridge *bridge,
> struct drm_atomic_state *state)
> {
> @@ -228,6 +246,7 @@ static void tidss_oldi_atomic_pre_enable(struct drm_bridge *bridge,
> struct drm_connector_state *conn_state;
> struct drm_crtc_state *crtc_state;
> struct drm_display_mode *mode;
> + struct drm_bridge_state *bridge_state;
>
> if (oldi->link_type == OLDI_MODE_SECONDARY_CLONE_SINGLE_LINK)
> return;
> @@ -245,10 +264,14 @@ static void tidss_oldi_atomic_pre_enable(struct drm_bridge *bridge,
> if (WARN_ON(!crtc_state))
> return;
>
> + bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
> + if (WARN_ON(!bridge_state))
> + return;
> +
> mode = &crtc_state->adjusted_mode;
>
> /* Configure the OLDI params*/
> - tidss_oldi_config(oldi);
> + tidss_oldi_config(oldi, bridge_state);
>
> /* Set the OLDI serial clock (7 times the pixel clock) */
> tidss_oldi_set_serial_clk(oldi, mode->clock * 7 * 1000);
> @@ -329,7 +352,8 @@ tidss_oldi_mode_valid(struct drm_bridge *bridge,
> }
>
> static const struct drm_bridge_funcs tidss_oldi_bridge_funcs = {
> - .attach = tidss_oldi_bridge_attach,
> + .attach = tidss_oldi_bridge_attach,
> + .atomic_check = tidss_oldi_atomic_check,
> .atomic_pre_enable = tidss_oldi_atomic_pre_enable,
> .atomic_post_disable = tidss_oldi_atomic_post_disable,
> .atomic_get_input_bus_fmts = tidss_oldi_atomic_get_input_bus_fmts,
> @@ -440,11 +464,6 @@ static int get_parent_dss_vp(struct device_node *oldi_tx, u32 *parent_vp)
> return -ENODEV;
> }
>
> -static const struct drm_bridge_timings default_tidss_oldi_timings = {
> - .input_bus_flags = DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE
> - | DRM_BUS_FLAG_DE_HIGH,
> -};
> -
> void tidss_oldi_deinit(struct tidss_device *tidss)
> {
> for (int i = 0; i < tidss->num_oldis; i++) {
> @@ -598,7 +617,6 @@ int tidss_oldi_init(struct tidss_device *tidss)
> /* Register the bridge. */
> oldi->bridge.of_node = child;
> oldi->bridge.driver_private = oldi;
> - oldi->bridge.timings = &default_tidss_oldi_timings;
>
> tidss->oldis[tidss->num_oldis++] = oldi;
> tidss->is_ext_vp_clk[oldi->parent_vp] = true;
>
^ permalink raw reply
* Re: [PATCH v3 10/15] drm/tidss: Add support for DPIENABLE bit
From: Swamil Jain @ 2026-06-24 10:46 UTC (permalink / raw)
To: Tomi Valkeinen, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Aradhya Bhatia,
Nishanth Menon, Vignesh Raghavendra, Devarsh Thakkar,
Louis Chauvet
Cc: devicetree, dri-devel, linux-kernel, linux-arm-kernel
In-Reply-To: <20260529-beagley-ai-display-v3-10-7fefdc5d1adf@ideasonboard.com>
On 29-05-2026 14:15, Tomi Valkeinen wrote:
> Many (or even all?) K3 SoCs have DSS VP_CONTROL.DPIENABLE bit described
> in their documentation. This bit controls whether the DPI block is
> enabled, and is set to 1 by default (i.e. DPI is enabled at HW reset).
>
> However, in almost all SoCs the setting does not actually do anything,
> and at the moment the bit is not managed by the driver.
>
> The exception is AM62L, which does have DPIENABLE connected, and
> disabling the DPI block when it is not in use provides power savings.
>
> Let's add a new feature flag for this, 'has_vp_control_dpienable', and
> implement the support. Disable DPIENABLE for all videoports at resume
> time, so that it is 0 by default. Specifically enable and disable it in
> dispc_vp_enable() and dispc_vp_disable() for DPI output.
>
> Tested-by: Swamil Jain <s-jain1@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
Reviewed-by: Swamil Jain <s-jain1@ti.com>
> drivers/gpu/drm/tidss/tidss_dispc.c | 23 +++++++++++++++++++++--
> drivers/gpu/drm/tidss/tidss_dispc.h | 2 ++
> drivers/gpu/drm/tidss/tidss_dispc_regs.h | 1 +
> 3 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index 08342a9a5e8c..1b8d52f10673 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -442,6 +442,8 @@ const struct dispc_features dispc_am62l_feats = {
> },
>
> .vid_order = {0},
> +
> + .has_vp_control_dpienable = true,
> };
>
> static const u16 *dispc_common_regmap;
> @@ -1210,6 +1212,11 @@ void dispc_vp_prepare(struct dispc_device *dispc, u32 hw_videoport,
> (!ipc ? DPI0_CLK_CTRL_DATA_CLK_INVDIS : 0) |
> (rf ? DPI0_CLK_CTRL_SYNC_CLK_INVDIS : 0));
> }
> +
> + if (dispc->feat->has_vp_control_dpienable &&
> + dispc->vp_data[hw_videoport].dpi_output)
> + VP_REG_FLD_MOD(dispc, hw_videoport, DISPC_VP_CONTROL, 1,
> + DISPC_VP_CONTROL_DPIENABLE_MASK);
> }
>
> void dispc_vp_enable(struct dispc_device *dispc, u32 hw_videoport)
> @@ -1226,6 +1233,11 @@ void dispc_vp_disable(struct dispc_device *dispc, u32 hw_videoport)
>
> void dispc_vp_unprepare(struct dispc_device *dispc, u32 hw_videoport)
> {
> + if (dispc->feat->has_vp_control_dpienable &&
> + dispc->vp_data[hw_videoport].dpi_output)
> + VP_REG_FLD_MOD(dispc, hw_videoport, DISPC_VP_CONTROL, 0,
> + DISPC_VP_CONTROL_DPIENABLE_MASK);
> +
> if (dispc->feat->vp_bus_type[hw_videoport] == DISPC_VP_OLDI_AM65X) {
> dispc_vp_write(dispc, hw_videoport, DISPC_VP_DSS_OLDI_CFG, 0);
>
> @@ -2445,10 +2457,17 @@ static void dispc_vp_init(struct dispc_device *dispc)
>
> dev_dbg(dispc->dev, "%s()\n", __func__);
>
> - /* Enable the gamma Shadow bit-field for all VPs*/
> - for (i = 0; i < dispc->feat->num_vps; i++)
> + for (i = 0; i < dispc->feat->num_vps; i++) {
> + /* Enable the gamma Shadow bit-field for all VPs*/
> VP_REG_FLD_MOD(dispc, i, DISPC_VP_CONFIG, 1,
> DISPC_VP_CONFIG_GAMMAENABLE_MASK);
> +
> + if (dispc->feat->has_vp_control_dpienable) {
> + /* Disable DPIENABLE for all VPs */
> + VP_REG_FLD_MOD(dispc, i, DISPC_VP_CONTROL, 0,
> + DISPC_VP_CONTROL_DPIENABLE_MASK);
> + }
> + }
> }
>
> static void dispc_initial_config(struct dispc_device *dispc)
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
> index 6f53d554259c..0fbfb86adfbf 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
> @@ -92,6 +92,8 @@ struct dispc_features {
> u32 num_vids;
> struct dispc_vid_info vid_info[TIDSS_MAX_PLANES];
> u32 vid_order[TIDSS_MAX_PLANES];
> + /* The DSS has VP_CONTROL.DPIENABLE bit */
> + bool has_vp_control_dpienable;
> };
>
> extern const struct dispc_features dispc_k2g_feats;
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc_regs.h b/drivers/gpu/drm/tidss/tidss_dispc_regs.h
> index 4cdde24d8372..4246c72efdd5 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc_regs.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc_regs.h
> @@ -230,6 +230,7 @@ enum dispc_common_regs {
>
> #define DISPC_VP_CONTROL 0x4
> #define DISPC_VP_CONTROL_DATALINES_MASK GENMASK(10, 8)
> +#define DISPC_VP_CONTROL_DPIENABLE_MASK GENMASK(6, 6)
> #define DISPC_VP_CONTROL_GOBIT_MASK GENMASK(5, 5)
> #define DISPC_VP_CONTROL_ENABLE_MASK GENMASK(0, 0)
>
>
^ permalink raw reply
* Re: [PATCH v5 05/12] i3c: master: Add support for devices without PID
From: sashiko-bot @ 2026-06-24 10:45 UTC (permalink / raw)
To: Akhil R; +Cc: Frank.Li, robh, conor+dt, linux-i3c, devicetree, linux-hwmon
In-Reply-To: <20260624102153.1770072-6-akhilrajeev@nvidia.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] Devices returning a PID of 0 during Dynamic Address Assignment (DAA) bypass duplicate detection, leading to sysfs naming collisions that reliably trigger a preexisting Use-After-Free.
- [Medium] The driver incorrectly allows non-SETAASA I3C devices to be defined in the device tree with a PID of 0, permanently breaking device matching upon Hot-Join re-attachment.
- [Low] The newly added static address matching logic in `i3c_master_attach_boardinfo()` will never successfully match any device, rendering the `else` branch dead code.
- [Low] Redundant boolean check for `i3cdev->info.pid` inside the duplicate search loop.
Pre-existing issues:
- [High] Dangling pointer in `desc->dev` leads to a Use-After-Free and Double Free if `device_register()` fails.
- [High] Calling `device_register()` while holding a read lock on the bus rwsem can lead to a deadlock if the driver's probe function attempts an I3C transfer.
--
commit 76e7c85069f65f75e5476a13e6f4ab2ae705589f
Author: Akhil R <akhilrajeev@nvidia.com>
i3c: master: Add support for devices without PID
Devices using SETAASA for address assignment are not required to have
a 48-bit PID according to the I3C specification. Allow such devices to
register and use the static address where PID was required.
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 3deae8cdae525..bd0dc76c7ba1e 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1989,8 +1989,17 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
> desc->dev->dev.type = &i3c_device_type;
> desc->dev->dev.bus = &i3c_bus_type;
> desc->dev->dev.release = i3c_device_release;
> - dev_set_name(&desc->dev->dev, "%d-%llx", master->bus.id,
> - desc->info.pid);
> +
> + /*
> + * For devices without PID (e.g., SETAASA devices), use
> + * static address for naming instead.
> + */
> + if (desc->info.pid)
> + dev_set_name(&desc->dev->dev, "%d-%llx", master->bus.id,
> + desc->info.pid);
> + else
> + dev_set_name(&desc->dev->dev, "%d-%02x", master->bus.id,
> + desc->info.static_addr);
>
> if (desc->boardinfo)
> device_set_node(&desc->dev->dev,
[Severity: High]
This is a pre-existing issue, but does failing to clear desc->dev when
device_register() fails lead to a double free and use-after-free later?
Looking at i3c_master_register_new_i3c_devs():
ret = device_register(&desc->dev->dev);
if (ret) {
dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
put_device(&desc->dev->dev);
}
If device_register() fails, put_device() is called which frees the i3c_device
structure, but desc->dev is left dangling. Later, when the master is
unregistered in i3c_master_unregister_i3c_devs(), the loop checks:
if (!i3cdev->dev)
continue;
i3cdev->dev->desc = NULL;
if (device_is_registered(&i3cdev->dev->dev))
device_unregister(&i3cdev->dev->dev);
else
put_device(&i3cdev->dev->dev);
Since desc->dev is not NULL, this writes to the freed memory and then calls
put_device() again, resulting in a use-after-free and double free.
[Severity: High]
This is also a pre-existing issue, but can calling device_register() while
holding a read lock on the bus rwsem lead to a deadlock?
In i3c_master_reg_work_fn():
i3c_bus_normaluse_lock(&master->bus);
if (!master->shutting_down)
i3c_master_register_new_i3c_devs(master);
The device_register() call inside i3c_master_register_new_i3c_devs()
synchronously triggers the driver's probe function. If the probe attempts an
I3C transfer, it calls i3c_device_do_priv_xfers(), which recursively requests
the read lock on the bus.
If a concurrent thread, such as a Hot Join, requests a write lock between
these two read lock attempts, the rwsem blocks the second read lock to prevent
writer starvation. The writer blocks waiting for the first read lock to be
released, which won't happen until device_register() returns, resulting in an
ABBA deadlock.
[ ... ]
> @@ -2383,8 +2392,18 @@ static void i3c_master_attach_boardinfo(struct i3c_dev_desc *i3cdev)
> struct i3c_dev_boardinfo *i3cboardinfo;
>
> list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) {
> - if (i3cdev->info.pid != i3cboardinfo->pid)
> - continue;
> + /*
> + * For devices without PID (e.g., SETAASA devices), match by
> + * static address. For devices with PID, match by PID.
> + */
> + if (i3cboardinfo->pid) {
> + if (i3cdev->info.pid != i3cboardinfo->pid)
> + continue;
> + } else {
> + if (!i3cboardinfo->static_addr ||
> + i3cdev->info.static_addr != i3cboardinfo->static_addr)
> + continue;
> + }
[Severity: Low]
Will this newly added else block ever successfully match a device?
i3c_master_attach_boardinfo() is called from
__i3c_master_add_i3c_dev_locked() to handle devices discovered dynamically via
DAA or Hot-Join. For these devices, the hardware does not provide a static
address during DAA, so i3cdev->info.static_addr is strictly 0.
Because of this, the condition
i3cdev->info.static_addr != i3cboardinfo->static_addr
will evaluate to 0 != non_zero, which is always true. This causes the loop
to unconditionally continue, making this fallback dead code.
>
> i3cdev->boardinfo = i3cboardinfo;
> i3cdev->info.static_addr = i3cboardinfo->static_addr;
> @@ -2398,8 +2417,12 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
> struct i3c_master_controller *master = i3c_dev_get_master(refdev);
> struct i3c_dev_desc *i3cdev;
>
> + if (!refdev->info.pid)
> + return NULL;
> +
[Severity: High]
Does returning NULL when PID is 0 allow devices to bypass duplicate detection
and trigger the pre-existing use-after-free bug?
If a dynamically discovered device reports a PID of 0, this check
unconditionally bypasses duplicate rejection, so the device is repeatedly
added.
Because static_addr is also 0 for DAA-discovered devices, the new naming
fallback in i3c_master_register_new_i3c_devs():
dev_set_name(&desc->dev->dev, "%d-%02x", master->bus.id,
desc->info.static_addr);
will name multiple devices the same (e.g., %d-00). The first instance will
register successfully, but subsequent ones will fail device_register() due to
sysfs name collisions. This perfectly triggers the desc->dev use-after-free
on the error path.
> i3c_bus_for_each_i3cdev(&master->bus, i3cdev) {
> - if (i3cdev != refdev && i3cdev->info.pid == refdev->info.pid)
> + if (i3cdev != refdev && i3cdev->info.pid &&
> + i3cdev->info.pid == refdev->info.pid)
[Severity: Low]
Is the check for i3cdev->info.pid redundant here?
Since refdev->info.pid was explicitly checked for being non-zero right before
this loop, the equality check i3cdev->info.pid == refdev->info.pid already
guarantees that i3cdev->info.pid is non-zero. The boolean check isn't strictly
necessary.
> return i3cdev;
> }
[ ... ]
> @@ -2832,9 +2855,15 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
>
> boardinfo->pid = ((u64)reg[1] << 32) | reg[2];
>
> - if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
> - I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
> - return -EINVAL;
> + /* For SETAASA devices, validate the static address instead of PID */
> + if (boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
> + if (!boardinfo->static_addr)
> + return -EINVAL;
> + } else {
> + if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
> + I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
> + return -EINVAL;
> + }
[Severity: Medium]
Could this logic incorrectly allow non-SETAASA I3C devices in the device tree
to have a PID of 0, permanently breaking device matching upon Hot-Join?
If a device is configured with mipi-i3c-static-method = SETDASA, it will reach
this else block. If its PID is 0, both boardinfo->pid & GENMASK_ULL(63, 48)
and I3C_PID_RND_LOWER_32BITS(boardinfo->pid) evaluate to 0, which bypasses
validation, and the device registers with PID 0.
If the device later Hot-Joins, DAA will retrieve its real, non-zero PID. When
i3c_master_attach_boardinfo() runs, it sees i3cboardinfo->pid == 0 and falls
back to matching by static_addr. Because static_addr is 0 during DAA, it
fails to match and the device permanently loses its DT properties.
>
> boardinfo->init_dyn_addr = init_dyn_addr;
> boardinfo->fwnode = fwnode_handle_get(fwnode);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624102153.1770072-1-akhilrajeev@nvidia.com?part=5
^ permalink raw reply
* Re: [PATCH v3 08/15] drm/tidss: Add mechanism to detect DPI output
From: Swamil Jain @ 2026-06-24 10:44 UTC (permalink / raw)
To: Tomi Valkeinen, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Aradhya Bhatia,
Nishanth Menon, Vignesh Raghavendra, Devarsh Thakkar,
Louis Chauvet
Cc: devicetree, dri-devel, linux-kernel, linux-arm-kernel
In-Reply-To: <20260529-beagley-ai-display-v3-8-7fefdc5d1adf@ideasonboard.com>
On 29-05-2026 14:15, Tomi Valkeinen wrote:
> There are situations where the driver needs to know if the output is
> going to the DPI output or not. There is no trivial way to get this
> information, as there is no "DPI bridge". We can only find this out in
> reverse: check if the output is NOT DPI, and if that is negative, then
> it must be DPI.
>
> At the moment we have two non-DPI outputs: DSI and OLDI. DSI always has
> "ti,j721e-dsi" DSI bridge connected to the DSI, so we can use that for
> checking. OLDI doesn't have a compatible property, but we can check if
> the DT node has "oldi-transmitters" node as a parent, and the dss node
> itself as a grand-parent.
>
> If the output is not connected to either of the above, it must be DPI.
>
> Tested-by: Swamil Jain <s-jain1@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
Reviewed-by: Swamil Jain <s-jain1@ti.com>
> drivers/gpu/drm/tidss/tidss_crtc.c | 10 +++++--
> drivers/gpu/drm/tidss/tidss_crtc.h | 4 ++-
> drivers/gpu/drm/tidss/tidss_dispc.c | 5 +++-
> drivers/gpu/drm/tidss/tidss_dispc.h | 3 +-
> drivers/gpu/drm/tidss/tidss_kms.c | 55 ++++++++++++++++++++++++++++++++++++-
> 5 files changed, 70 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
> index a31c21c5f855..dfdf61b01dcd 100644
> --- a/drivers/gpu/drm/tidss/tidss_crtc.c
> +++ b/drivers/gpu/drm/tidss/tidss_crtc.c
> @@ -192,7 +192,8 @@ static void tidss_crtc_atomic_flush(struct drm_crtc *crtc,
> return;
>
> /* Write vp properties to HW if needed. */
> - dispc_vp_setup(tidss->dispc, tcrtc->hw_videoport, crtc->state, false);
> + dispc_vp_setup(tidss->dispc, tcrtc->hw_videoport, crtc->state, false,
> + tcrtc->dpi_output);
>
> /* Update plane positions if needed. */
> tidss_crtc_position_planes(tidss, crtc, old_crtc_state, false);
> @@ -235,7 +236,8 @@ static void tidss_crtc_atomic_enable(struct drm_crtc *crtc,
> if (r != 0)
> return;
>
> - dispc_vp_setup(tidss->dispc, tcrtc->hw_videoport, crtc->state, true);
> + dispc_vp_setup(tidss->dispc, tcrtc->hw_videoport, crtc->state, true,
> + tcrtc->dpi_output);
> tidss_crtc_position_planes(tidss, crtc, old_state, true);
>
> /* Turn vertical blanking interrupt reporting on. */
> @@ -417,7 +419,8 @@ static const struct drm_crtc_funcs tidss_crtc_funcs = {
>
> struct tidss_crtc *tidss_crtc_create(struct tidss_device *tidss,
> u32 hw_videoport,
> - struct drm_plane *primary)
> + struct drm_plane *primary,
> + bool dpi_output)
> {
> struct tidss_crtc *tcrtc;
> struct drm_crtc *crtc;
> @@ -430,6 +433,7 @@ struct tidss_crtc *tidss_crtc_create(struct tidss_device *tidss,
> return ERR_PTR(-ENOMEM);
>
> tcrtc->hw_videoport = hw_videoport;
> + tcrtc->dpi_output = dpi_output;
> init_completion(&tcrtc->framedone_completion);
>
> crtc = &tcrtc->crtc;
> diff --git a/drivers/gpu/drm/tidss/tidss_crtc.h b/drivers/gpu/drm/tidss/tidss_crtc.h
> index 040d1205496b..65df220698f6 100644
> --- a/drivers/gpu/drm/tidss/tidss_crtc.h
> +++ b/drivers/gpu/drm/tidss/tidss_crtc.h
> @@ -20,6 +20,7 @@ struct tidss_crtc {
> struct drm_crtc crtc;
>
> u32 hw_videoport;
> + bool dpi_output;
>
> struct drm_pending_vblank_event *event;
>
> @@ -44,5 +45,6 @@ void tidss_crtc_error_irq(struct drm_crtc *crtc, u64 irqstatus);
>
> struct tidss_crtc *tidss_crtc_create(struct tidss_device *tidss,
> u32 hw_videoport,
> - struct drm_plane *primary);
> + struct drm_plane *primary,
> + bool dpi_output);
> #endif
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index 58d5eb033bdb..c21ac3f51720 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -448,6 +448,7 @@ static const u16 *dispc_common_regmap;
>
> struct dss_vp_data {
> u32 *gamma_table;
> + bool dpi_output;
> };
>
> struct dispc_device {
> @@ -2770,8 +2771,10 @@ static void dispc_vp_set_color_mgmt(struct dispc_device *dispc,
> }
>
> void dispc_vp_setup(struct dispc_device *dispc, u32 hw_videoport,
> - const struct drm_crtc_state *state, bool newmodeset)
> + const struct drm_crtc_state *state, bool newmodeset,
> + bool dpi_output)
> {
> + dispc->vp_data[hw_videoport].dpi_output = dpi_output;
> dispc_vp_set_default_color(dispc, hw_videoport, 0);
> dispc_vp_set_color_mgmt(dispc, hw_videoport, state, newmodeset);
> }
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
> index 739d211d0018..6f53d554259c 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
> @@ -131,7 +131,8 @@ void dispc_vp_disable_clk(struct dispc_device *dispc, u32 hw_videoport);
> int dispc_vp_set_clk_rate(struct dispc_device *dispc, u32 hw_videoport,
> unsigned long rate);
> void dispc_vp_setup(struct dispc_device *dispc, u32 hw_videoport,
> - const struct drm_crtc_state *state, bool newmodeset);
> + const struct drm_crtc_state *state, bool newmodeset,
> + bool dpi_output);
>
> int dispc_runtime_suspend(struct dispc_device *dispc);
> int dispc_runtime_resume(struct dispc_device *dispc);
> diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c
> index 8bb93194e5ac..bc8b10af9a48 100644
> --- a/drivers/gpu/drm/tidss/tidss_kms.c
> +++ b/drivers/gpu/drm/tidss/tidss_kms.c
> @@ -122,6 +122,50 @@ static const struct drm_mode_config_funcs mode_config_funcs = {
> .atomic_commit = drm_atomic_helper_commit,
> };
>
> +static const char * const tidss_internal_bridge_compatibles[] = {
> + "ti,j721e-dsi",
> +};
> +
> +/*
> + * Detect whether the bridge is internal to the SoC or not. This is needed
> + * to find out whether we are using DPI output (thus no internal bridge).
> + * We detect this via two means:
> + * - If the bridge's of_node has a compatible, compare to known internal values.
> + * - If the bridge is a grand-child of DSS, and has "oldi-transmitters" parent.
> + */
> +static bool tidss_is_bridge_internal(struct tidss_device *tidss,
> + struct drm_bridge *bridge)
> +{
> + struct device_node *parent, *grand_parent;
> + struct property *prop;
> + bool is_internal;
> +
> + if (WARN_ON(!bridge->of_node))
> + return false;
> +
> + prop = of_find_property(bridge->of_node, "compatible", NULL);
> + for (const char *cp = of_prop_next_string(prop, NULL); cp;
> + cp = of_prop_next_string(prop, cp)) {
> + for (unsigned int i = 0;
> + i < ARRAY_SIZE(tidss_internal_bridge_compatibles); ++i) {
> + if (strcmp(cp, tidss_internal_bridge_compatibles[i]) == 0)
> + return true;
> + }
> + }
> +
> + parent = of_get_parent(bridge->of_node);
> + grand_parent = of_get_parent(parent);
> +
> + is_internal = parent && grand_parent &&
> + tidss->dev->of_node == grand_parent &&
> + of_node_name_eq(parent, "oldi-transmitters");
> +
> + of_node_put(grand_parent);
> + of_node_put(parent);
> +
> + return is_internal;
> +}
> +
> static int tidss_dispc_modeset_init(struct tidss_device *tidss)
> {
> struct device *dev = tidss->dev;
> @@ -133,6 +177,7 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
> u32 hw_videoport;
> struct drm_bridge *bridge;
> u32 enc_type;
> + bool dpi_output;
> };
>
> const struct dispc_features *feat = tidss->feat;
> @@ -149,6 +194,7 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
> struct drm_panel *panel;
> struct drm_bridge *bridge;
> u32 enc_type = DRM_MODE_ENCODER_NONE;
> + bool dpi_output;
> int ret;
>
> ret = drm_of_find_panel_or_bridge(dev->of_node, i, 0,
> @@ -160,6 +206,11 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
> return dev_err_probe(dev, ret, "port %d probe failed\n", i);
> }
>
> + if (bridge)
> + dpi_output = !tidss_is_bridge_internal(tidss, bridge);
> + else
> + dpi_output = true;
> +
> if (panel) {
> u32 conn_type;
>
> @@ -199,6 +250,7 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
> pipes[num_pipes].hw_videoport = i;
> pipes[num_pipes].bridge = bridge;
> pipes[num_pipes].enc_type = enc_type;
> + pipes[num_pipes].dpi_output = dpi_output;
> num_pipes++;
> }
>
> @@ -224,7 +276,8 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
> tidss->planes[tidss->num_planes++] = &tplane->plane;
>
> tcrtc = tidss_crtc_create(tidss, pipes[i].hw_videoport,
> - &tplane->plane);
> + &tplane->plane,
> + pipes[i].dpi_output);
> if (IS_ERR(tcrtc)) {
> dev_err(tidss->dev, "crtc create failed\n");
> return PTR_ERR(tcrtc);
>
^ permalink raw reply
* Re: [PATCH v5 08/12] i3c: dw-i3c-master: Add ACPI core clock frequency quirk
From: sashiko-bot @ 2026-06-24 10:45 UTC (permalink / raw)
To: Akhil R; +Cc: robh, Frank.Li, linux-i3c, devicetree, linux-hwmon, conor+dt
In-Reply-To: <20260624102153.1770072-9-akhilrajeev@nvidia.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Changing `core_clk` from mandatory to optional delays missing clock validation, leading to unclocked hardware register accesses that can trigger a synchronous external abort (system crash) on misconfigured platforms.
Pre-existing issues:
- [High] The workqueue `master->wq` is destroyed during unregistration, but the devres-managed hardware IRQ remains active without being masked, allowing a late interrupt to queue work on the destroyed workqueue.
- [High] The driver unconditionally casts the pointer returned by `device_get_match_data()` to an `unsigned long` when binding an ACPI companion, leading to garbage quirk flags if the device is bound via the PRP0001 ACPI-OF fallback.
--
commit d5fbc1910ed540d2be9a4cb57886399f9c4f9b04
Author: Akhil R <akhilrajeev@nvidia.com>
i3c: dw-i3c-master: Add ACPI core clock frequency quirk
Some ACPI-enumerated devices like Tegra410 do not expose the controller
core clock through the clk framework. Unlike device tree, ACPI on Arm does
not model clock providers. The hardware is expected to have its clocks
enabled by firmware before the OS takes over.
Make the core clock optional and allow selected ACPI devices to provide the
core clock rate through the "clock-frequency" _DSD property when the core
clock is absent.
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 29030fd9594aa..8e40d178d500a 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
[ ... ]
> @@ -1577,7 +1598,7 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
> if (IS_ERR(master->regs))
> return PTR_ERR(master->regs);
>
> - master->core_clk = devm_clk_get_enabled(&pdev->dev, NULL);
> + master->core_clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
[Severity: High]
If the device tree is missing the core clock, this will now return NULL
instead of an error. Could this lead to unclocked MMIO accesses?
In dw_i3c_common_probe():
master->core_clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
...
writel(INTR_ALL, master->regs + INTR_STATUS);
If the clock is missing, the MMIO access to INTR_STATUS will happen
unconditionally, which can trigger a synchronous external abort on some
systems.
The missing clock is checked later in dw_i3c_master_get_core_rate():
if (!(master->quirks & DW_I3C_ACPI_SKIP_CLK_RST)) {
dev_err(master->dev, "missing core clock\n");
return 0;
}
Is it possible to validate the missing clock earlier to prevent the kernel
panic during the probe sequence?
> if (IS_ERR(master->core_clk))
> return PTR_ERR(master->core_clk);
>
[Severity: High]
This isn't a bug introduced by this patch, but does the removal path
destroy the workqueue while hardware interrupts are still active?
In drivers/i3c/master/dw-i3c-master.c:dw_i3c_common_remove():
i3c_master_unregister(&master->base);
This unregisters the device and eventually triggers:
drivers/i3c/master.c:i3c_masterdev_release():
if (master->wq)
destroy_workqueue(master->wq);
Since INTR_STATUS_EN is never cleared to mask interrupts and the devres
managed IRQ is not yet freed, could a delayed IBI or Hot-Join interrupt
still fire?
If it does, the still-active IRQ handler will attempt to queue work on
the destroyed workqueue:
drivers/i3c/master/dw-i3c-master.c:dw_i3c_master_irq_handle_ibis():
} else if (IBI_TYPE_HJ(reg)) {
i3c_master_queue_hotjoin(&master->base);
}
Can this result in a use-after-free?
[Severity: High]
This is a pre-existing issue, but does this unconditionally cast the match
data pointer to an integer?
In drivers/i3c/master/dw-i3c-master.c:dw_i3c_common_probe():
if (has_acpi_companion(&pdev->dev)) {
quirks = (unsigned long)device_get_match_data(&pdev->dev);
} else if (pdev->dev.of_node) {
drvdata = device_get_match_data(&pdev->dev);
if (drvdata)
quirks = drvdata->flags;
}
If the device uses the PRP0001 ACPI-OF fallback, has_acpi_companion() will
be true, and device_get_match_data() will return a pointer to the OF table
struct dw_i3c_drvdata.
Does this cast the pointer address directly into the quirks bitmask, causing
unpredictable feature toggles?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624102153.1770072-1-akhilrajeev@nvidia.com?part=8
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: power: limits: Describe Qualcomm SPEL hardware
From: Krzysztof Kozlowski @ 2026-06-24 10:45 UTC (permalink / raw)
To: Manaf Meethalavalappu Pallikunhi
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Rafael J. Wysocki,
Bjorn Andersson, Konrad Dybcio, Daniel Lezcano, Gaurav Kohli,
linux-arm-msm, devicetree, linux-kernel, linux-pm
In-Reply-To: <db8f06fa-0dda-4a22-baaf-8a708d43e113@oss.qualcomm.com>
On 23/06/2026 11:47, Manaf Meethalavalappu Pallikunhi wrote:
> Hi Krzysztof,
>
>
> On 6/22/2026 5:58 PM, Krzysztof Kozlowski wrote:
>> On Sat, Jun 20, 2026 at 02:09:08AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
>>> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
>>> based power monitoring and limiting capabilities for various domains.
>>>
>>> Add a DeviceTree binding to describe the SPEL block on Qualcomm's SoC.
>>>
>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>> ---
>>> .../bindings/power/limits/qcom,spel.yaml | 47 ++++++++++++++++++++++
>>> MAINTAINERS | 6 +++
>>> 2 files changed, 53 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml b/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
>>
>> What is "limits" directory for? What sort of class of devices fit there?
>
> Added for devices that integrate with the powercap framework (exposed
> via sys/class/powercap). These devices are responsible for enforcing and
That's a driver answer. I asked about class of devices. powercap
framework is Linux thing, not a class of devices.
Please describe hardware, not Linux frameworks.
> monitoring power consumption limits across different domains, such as
> the system, SoC, or specific subsystems. Any other better directory ?
I don't know what is this hardware doing and commit msg is quite short
on explanation. Power monitoring is usually hwmon, but probably this is
not a hwmon.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 07/15] drm/tidss: oldi: Remove define for unused register OLDI_LB_CTRL
From: Swamil Jain @ 2026-06-24 10:43 UTC (permalink / raw)
To: Tomi Valkeinen, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Aradhya Bhatia,
Nishanth Menon, Vignesh Raghavendra, Devarsh Thakkar,
Louis Chauvet
Cc: devicetree, dri-devel, linux-kernel, linux-arm-kernel
In-Reply-To: <20260529-beagley-ai-display-v3-7-7fefdc5d1adf@ideasonboard.com>
On 29-05-2026 14:15, Tomi Valkeinen wrote:
> OLDI_LB_CTRL define is not used, and doesn't seem to exist at least on
> some SoCs. Let's remove the define.
>
> Tested-by: Swamil Jain <s-jain1@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
Reviewed-by: Swamil Jain <s-jain1@ti.com>
> drivers/gpu/drm/tidss/tidss_oldi.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tidss/tidss_oldi.h b/drivers/gpu/drm/tidss/tidss_oldi.h
> index 8cd535c5ee65..a361e6dbfce3 100644
> --- a/drivers/gpu/drm/tidss/tidss_oldi.h
> +++ b/drivers/gpu/drm/tidss/tidss_oldi.h
> @@ -20,7 +20,6 @@ struct tidss_oldi;
>
> /* Register offsets */
> #define OLDI_PD_CTRL 0x100
> -#define OLDI_LB_CTRL 0x104
>
> /* Power control bits */
> #define OLDI_PWRDOWN_TX(n) BIT(n)
>
^ permalink raw reply
* Re: [PATCH v5 04/12] i3c: master: Add support for devices using SETAASA
From: sashiko-bot @ 2026-06-24 10:43 UTC (permalink / raw)
To: Akhil R; +Cc: linux-hwmon, devicetree, robh, linux-i3c, conor+dt, Frank.Li
In-Reply-To: <20260624102153.1770072-5-akhilrajeev@nvidia.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] SETAASA broadcast is sent before SETDASA direct assignments, which breaks initialization for devices that natively support SETAASA but are configured for SETDASA.
Pre-existing issues:
- [High] Missing RSTDAA on error path due to premature address slot clearing leads to bus address collision.
- [High] Double free and Use-After-Free of `desc->dev` when `device_register()` fails.
--
commit 08b7cd00b36fcaa4fe6292af27e273df6dc64b5a
Author: Akhil R <akhilrajeev@nvidia.com>
i3c: master: Add support for devices using SETAASA
Add support for devices using SETAASA attached to DDR5 memory modules that
do not support ENTDAA. Devices using SETAASA assign their pre-defined static
addresses as dynamic addresses during DAA.
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 4bba2bad897a7..3deae8cdae525 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
[ ... ]
> @@ -1878,6 +1924,22 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
> if (ret)
> goto err_free_dev;
>
> + /*
> + * For devices using SETAASA instead of ENTDAA, the address is statically
> + * assigned. Update the dynamic address to the provided static address.
> + * Reattach the I3C device after updating the dynamic address with the same
> + * static address. It is not mandatory for such devices to implement CCC
> + * commands like GETPID, GETDCR etc. Hence, we can return after reattaching.
> + */
> + if (i3cdev->boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
> + i3cdev->info.dyn_addr = i3cdev->boardinfo->static_addr;
> + ret = i3c_master_reattach_i3c_dev_locked(i3cdev, 0);
> + if (ret)
> + goto err_rstdaa;
> +
> + return 0;
> + }
> +
[Severity: High]
This is a pre-existing issue, but missing RSTDAA on the error path due to
premature address slot clearing can lead to bus address collisions.
In i3c_master_early_i3c_dev_add(), the driver assigns the dynamic address
with SETDASA and reattaches the device. If reattaching fails, the error
path internally calls i3c_master_put_i3c_addrs(), which clears the
init_dyn_addr slot status back to I3C_ADDR_SLOT_FREE.
When the code jumps to err_rstdaa:
err_rstdaa:
i3c_master_rstdaa_locked(master, i3cdev->boardinfo->init_dyn_addr);
It calls i3c_master_rstdaa_locked() to reset the device's dynamic address.
However, i3c_master_rstdaa_locked() verifies the slot status:
drivers/i3c/master.c:i3c_master_rstdaa_locked() {
...
if (addr != I3C_BROADCAST_ADDR && addrstat != I3C_ADDR_SLOT_I3C_DEV)
return -EINVAL;
...
}
Because the slot was just freed, the function silently returns without
sending the RSTDAA CCC command.
Will this allow the hardware device to continue squatting on the dynamic
address while the master considers the slot free, causing collisions later?
> ret = i3c_master_setdasa_locked(master, i3cdev->info.static_addr,
> i3cdev->boardinfo->init_dyn_addr);
[ ... ]
> @@ -2232,6 +2294,12 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> if (ret)
> goto err_bus_cleanup;
>
> + if (master->addr_method & I3C_ADDR_METHOD_SETAASA) {
> + ret = i3c_master_setaasa_locked(master);
> + if (ret)
> + goto err_bus_cleanup;
> + }
> +
[Severity: High]
Is it possible that sending the SETAASA broadcast before direct SETDASA
assignments breaks initialization for devices that natively support SETAASA
but are configured for SETDASA?
According to the I3C specification, any device on the bus natively supporting
SETAASA will respond to this broadcast by adopting its static address as its
dynamic address.
After this broadcast, the driver iterates through devices and attempts to
assign custom dynamic addresses via direct SETDASA commands:
drivers/i3c/master.c:i3c_master_early_i3c_dev_add() {
...
ret = i3c_master_setdasa_locked(master, i3cdev->info.static_addr,
i3cdev->boardinfo->init_dyn_addr);
...
}
Since the target device already adopted its dynamic address during the
SETAASA broadcast, it is no longer in the unassigned state and will NACK
the subsequent SETDASA command.
Could this cause device initialization to abort entirely?
Should SETDASA direct assignments be executed before broadcasting SETAASA?
> /*
> * Reserve init_dyn_addr first, and then try to pre-assign dynamic
> * address and retrieve device information if needed.
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but there is a potential double
free and use-after-free of desc->dev when device_register() fails.
In i3c_master_register_new_i3c_devs(), if device_register() fails,
put_device() is called:
drivers/i3c/master.c:i3c_master_register_new_i3c_devs() {
...
ret = device_register(&desc->dev->dev);
if (ret) {
dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
put_device(&desc->dev->dev);
}
...
}
This drops the reference count to 0 and triggers i3c_device_release(), which
frees desc->dev via kfree(i3cdev). However, the pointer desc->dev is not set
to NULL and remains dangling in the master->bus.devs.i3c list.
Later, when the master module is unloaded or unregistered, the cleanup
iterates over the list:
drivers/i3c/master.c:i3c_master_unregister_i3c_devs() {
...
if (!i3cdev->dev)
continue;
i3cdev->dev->desc = NULL;
if (device_is_registered(&i3cdev->dev->dev))
device_unregister(&i3cdev->dev->dev);
else
put_device(&i3cdev->dev->dev);
i3cdev->dev = NULL;
...
}
Because i3cdev->dev is a dangling pointer, the check passes. This leads to
writing to i3cdev->dev->desc (use-after-free) and calling put_device() again
(double free).
Could we set desc->dev to NULL after put_device() in the registration error
path?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624102153.1770072-1-akhilrajeev@nvidia.com?part=4
^ permalink raw reply
* Re: [PATCH v5 06/12] i3c: master: match I3C device through DT and ACPI
From: sashiko-bot @ 2026-06-24 10:42 UTC (permalink / raw)
To: Akhil R; +Cc: robh, conor+dt, Frank.Li, linux-i3c, linux-hwmon, devicetree
In-Reply-To: <20260624102153.1770072-7-akhilrajeev@nvidia.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Suppressing the native I3C modalias breaks module autoloading for standard (non-SETAASA) I3C devices that possess an OF or ACPI node.
Pre-existing issues:
- [Critical] Kernel stack memory disclosure in `i3c_device_uevent`, `modalias_show`, and `i3c_device_match_id` when `i3cdev->desc` is NULL.
- [High] Lockless access to `i3cdev->desc` in `i3c_device_match()` can lead to a Use-After-Free during concurrent DAA or Hot-Join operations.
--
commit 027ebb9082e984584613be936bd42a0829ebd9c6
Author: Akhil R <akhilrajeev@nvidia.com>
i3c: master: match I3C device through DT and ACPI
This patch allows SETAASA-based I3C devices to match through Device Tree
or ACPI when they cannot be identified by PID or DCR. It emits OF and ACPI
modaliases so firmware-matched devices can autoload their drivers.
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -320,6 +321,15 @@ static int i3c_device_uevent(const struct device *dev, struct kobj_uevent_env *e
> const struct i3c_device *i3cdev = dev_to_i3cdev(dev);
> struct i3c_device_info devinfo;
> u16 manuf, part, ext;
> + int rc;
> +
> + rc = of_device_uevent_modalias(dev, env);
> + if (rc != -ENODEV)
> + return rc;
[Severity: High]
Does returning early here break module autoloading for standard I3C devices
that have an OF or ACPI node?
If a standard non-SETAASA device has an OF node, it will only emit the
of: modalias and skip generating the standard i3c: modalias.
If the device's driver only provides an I3C id_table, udev might fail to
find a matching module. Furthermore, since i3c_device_match() restricts
of_driver_match_device() strictly to devices with the
I3C_ADDR_METHOD_SETAASA flag, standard I3C devices wouldn't be able to match
via Device Tree either.
> +
> + rc = acpi_device_uevent_modalias(dev, env);
> + if (rc != -ENODEV)
> + return rc;
>
> if (i3cdev->desc)
> devinfo = i3cdev->desc->info;
[Severity: Critical]
This is a pre-existing issue, but can this leak uninitialized stack memory
to userspace?
When a device is unregistered, i3c_master_unregister_i3c_devs() sets
i3cdev->dev->desc = NULL before calling device_unregister(). This triggers
a uevent that calls i3c_device_uevent().
If i3cdev->desc is NULL, the devinfo stack variable is never initialized,
but its contents (like devinfo.pid and devinfo.dcr) are still used to
construct the modalias string right below this block.
Could this expose kernel stack memory via netlink messages, or via sysfs
reads in modalias_show()? A similar uninitialized read also occurs in
i3c_device_match_id() when i3c_device_get_info() is called with a NULL
descriptor.
[ ... ]
> @@ -345,15 +355,32 @@ static int i3c_device_match(struct device *dev, const struct device_driver *drv)
> {
> struct i3c_device *i3cdev;
> const struct i3c_driver *i3cdrv;
> + u8 static_addr_method = 0;
>
> if (dev->type != &i3c_device_type)
> return 0;
>
> i3cdev = dev_to_i3cdev(dev);
> i3cdrv = drv_to_i3cdrv(drv);
> - if (i3c_device_match_id(i3cdev, i3cdrv->id_table))
> +
> + if (i3cdev->desc && i3cdev->desc->boardinfo)
> + static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
[Severity: High]
This is a pre-existing issue, but is it safe to access i3cdev->desc here
without holding the bus read lock?
During dynamic address assignment or hot-join operations,
__i3c_master_add_i3c_dev_locked() may find a duplicate device, reassign
a new descriptor, and free the old descriptor.
Since i3c_device_match() is called by the driver core without acquiring
the bus lock, might a concurrent hot-join thread free i3cdev->desc while
it is being dereferenced here, leading to a use-after-free? This same race
could happen when i3cdev->desc->info is accessed by i3c_device_get_info()
during matching.
> +
> + /*
> + * SETAASA-based devices need not always have a matching ID since
> + * it is not mandatory for such devices to implement deviceinfo
> + * CCC commands. Allow them to register through DT or ACPI.
> + */
> + if (i3cdrv->id_table && i3c_device_match_id(i3cdev, i3cdrv->id_table))
> return 1;
>
> + if (static_addr_method & I3C_ADDR_METHOD_SETAASA) {
> + if (of_driver_match_device(dev, drv))
> + return 1;
> + if (acpi_driver_match_device(dev, drv))
> + return 1;
> + }
> +
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624102153.1770072-1-akhilrajeev@nvidia.com?part=6
^ permalink raw reply
* Re: [PATCH v3 1/8] dt-bindings: remoteproc: qcom,pas: add thermal mitigation properties
From: Krzysztof Kozlowski @ 2026-06-24 10:42 UTC (permalink / raw)
To: Daniel Lezcano, Gaurav Kohli
Cc: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Amit Kucheria,
Manivannan Sadhasivam, Konrad Dybcio, Kees Cook,
Gustavo A. R. Silva, cros-qcom-dts-watchers, linux-arm-msm,
linux-remoteproc, devicetree, linux-kernel, linux-pm,
linux-hardening, Manaf Meethalavalappu Pallikunhi
In-Reply-To: <61765401-3397-497d-a0ca-e9bf9d76cc6a@oss.qualcomm.com>
On 18/06/2026 09:17, Daniel Lezcano wrote:
> On 6/16/26 06:21, Krzysztof Kozlowski wrote:
>> On 15/06/2026 14:30, Daniel Lezcano wrote:
>>> Hi Gaurav,
>>>>>> thanks for review, shall i use driver data, which is basically pas
>>>>>> data structure like below:
>>>>>>
>>>>>> static const struct qcom_pas_data {
>>>>>> .crash_reason_smem = 601,
>>>>>> .firmware_name = "cdsp.mdt",
>>>>>> .tmd_names = (const char *[]){"xyz", NULL},
>>>>>> .num_tmds = 1,
>>>>>>
>>>>>> Is something like above acceptable? and this will also help to filter
>>>>>> tmd names as well?
>>>>>
>>>>>
>>>>> How the thermal framework will bind the thermal zone with the TMD ?
>>>>> (node pointer, id) ?
>>>>>
>>>>
>>>> Hi Daniel,
>>>>
>>>> thanks for review.
>>>>
>>>> With id only, in this case instead of taking tmd names from device tree,
>>>> qmi_tmd will take tmd name from pas_data(driver) and register with the
>>>> cooling framework with id only. Please let us know if this looks fine.
>>> May be I'm missing something but:
>>>
>>> - The QMI TMD returns a list of names, not ids
>>> - The QMI TMD may return the list in different order than assumed
>>> - The cooling map index points to the name of the TMD in the DT
>>> - This name is used to match the name in the aformentionned list
>>> - The index in the list and the id in the DT can differ
>>>
>>> Krzysztof , I don't get why having the TMD names as properties is wrong,
>>> they describes the existing TMDs on the system and the cooling maps
>>> index points to the one to be connected with thermal zone.
>>
>>
>> 'xxx-names' have a fixed meaning in DT by convention - assign
>> identifiable strings to the 'xxx'. I miss the property 'tmd' in such
>> case - its definition and meaning. Where is it?
>>
>> But maybe you just want list of strings, so I am open to discuss it - I
>> don't understand the need for this property and commit and property
>> description tell me nothing.
>>
>> Really, this commit message is basically non-existing. It explains what
>> it did and provides that much explanation WHY:
>>
>> "- tmd-names (thermal mitigation device names)"
>>
>> Really? This is the explanation why this change is being made, why this
>> property is needed?
>>
>> So sure, describe the problem being solved and WHY this problem is being
>> solved that way. Maybe it will fit DT.
>
> Ok, I understand
>
> Let me try to clarify.
>
> When the QMI TMD protocol is initialized, the list of available TMDs
> provided by the service is requested. They are identified by a *string*
> not a numerical id.
>
> We can not assume the list is always in the same order because the QMI
> TMD is a separate subsystem and the interface is the protocol. The
> protocol does not refer to any TMD with an index but its name.
> Hardcoding an index here is not possible.
Still given device (remoteproc) will provide a fixed, one name for TMD.
That name is fully deducible from the compatible.
>
> The name identifies the TMD we connect to and in return we receive a
> handler to use when sending the QMI messages.
>
> That is for the driver part.
>
> I understand for the DT, it is possible to not give the tmd-names
> because the it can go into the driver's data structure.
>
> But the thermal framework won't be able to associate a cooling device
> (one TMD) with a thermal zone because the cooling mapping must point to
> a cooling device in the DT.
>
> Initially, Gaurav sent a description where each TMD was a child node of
> the remote proc node. And you rightfully told us it is no longer the way
> to go as stated in the documentation. And you suggested to replace the
> child nodes with an array with the tmd-names and add an index in the
> cooling map pointing to this array.
There was only one child node. It was one-to-one mapping, thus I don't
think I suggested using any tmd-names.
Here is the code:
https://lore.kernel.org/linux-devicetree/20251223123227.1317244-4-gaurav.kohli@oss.qualcomm.com/
and I clearly see one cooling only:
+ cooling:
+ $ref: /schemas/thermal/qcom,qmi-cooling.yaml#
Therefore I still do not see the need of tmd-names. You know the name of
cooling device, because you have strict one-to-one mapping.
>
> The thermal framework has been extended to support this new format and
> associate the cooling device with the thermal zone. This change is now
> upstream for v7.2 [1]
>
> The resulting implementation is the driver gets the tmd-names array
> property. For each name, it connects to the QMI TMD and register the
> cooling device with the index corresponding to the name position on the
> tmd-names array.
>
> On the other side, the thermal framework parses the cooling-map, gets
> the index and do the association (binding).
>
> The tmd-names array property replaces the child nodes and allows to do
> the mapping between the string based identifier with a numerical id.
>
> I hope that clarifies the approach.
>
> -- Daniel
>
> [1]
> https://lore.kernel.org/all/20260526140802.1059293-12-daniel.lezcano@oss.qualcomm.com/
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v5 12/12] arm64: defconfig: Enable I3C and SPD5118 hwmon
From: sashiko-bot @ 2026-06-24 10:40 UTC (permalink / raw)
To: Akhil R; +Cc: conor+dt, linux-hwmon, robh, devicetree, linux-i3c, Frank.Li
In-Reply-To: <20260624102153.1770072-13-akhilrajeev@nvidia.com>
> Enable I3C subsystem (I3C), Synopsys DesignWare I3C master controller
> (DW_I3C_MASTER), and SPD5118 hwmon temperature sensor (SENSORS_SPD5118)
> as modules.
>
> The NVIDIA Vera CPU uses SOCAMM LPDDR5X memory module, which contains
> SPD5118 (JEDEC JESD300) compliant temperature sensor. This sensor is
> accessible over the I3C bus through the DesignWare I3C controller present
> on the SoC. Enabling these configs allows monitoring memory module
> temperatures on platforms such as Vera Rubin. Vera is an ACPI-based
> platform and does not use device tree.
>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624102153.1770072-1-akhilrajeev@nvidia.com?part=12
^ permalink raw reply
* Re: [PATCH v3 06/15] drm/tidss: Remove extra pm_runtime_mark_last_busy
From: Swamil Jain @ 2026-06-24 10:39 UTC (permalink / raw)
To: Tomi Valkeinen, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Aradhya Bhatia,
Nishanth Menon, Vignesh Raghavendra, Devarsh Thakkar,
Louis Chauvet
Cc: devicetree, dri-devel, linux-kernel, linux-arm-kernel
In-Reply-To: <20260529-beagley-ai-display-v3-6-7fefdc5d1adf@ideasonboard.com>
On 29-05-2026 14:15, Tomi Valkeinen wrote:
> pm_runtime_put_autosuspend() calls pm_runtime_mark_last_busy(), so no
> need to call pm_runtime_mark_last_busy() explicitly in the driver.
>
> Tested-by: Swamil Jain <s-jain1@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
Reviewed-by: Swamil Jain <s-jain1@ti.com>
> drivers/gpu/drm/tidss/tidss_drv.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
> index 1c8cc18bc53c..5cb3e746aeb3 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.c
> +++ b/drivers/gpu/drm/tidss/tidss_drv.c
> @@ -42,8 +42,6 @@ void tidss_runtime_put(struct tidss_device *tidss)
> {
> int r;
>
> - pm_runtime_mark_last_busy(tidss->dev);
> -
> r = pm_runtime_put_autosuspend(tidss->dev);
> WARN_ON(r < 0);
> }
>
^ permalink raw reply
* Re: [PATCH v5 03/12] i3c: master: Support ACPI enumeration of child devices
From: sashiko-bot @ 2026-06-24 10:38 UTC (permalink / raw)
To: Akhil R; +Cc: robh, devicetree, conor+dt, linux-hwmon, linux-i3c, Frank.Li
In-Reply-To: <20260624102153.1770072-4-akhilrajeev@nvidia.com>
> Although the existing subsystem allows host controllers to register
> through the ACPI table, it was not possible to describe I3C or I2C
> devices when using ACPI. This is because the driver relied on the reg
> property to retrieve the PID, static address, etc., whereas ACPI uses
> _ADR or serial resources to describe such devices.
>
> Read _ADR and LVR from ACPI resources and extract the data as per the
> ACPI specification for an I3C bus. Also read mipi-i3c-static-address as
> per the MIPI DISCO specifications [1] to get the static address to be
> used.
>
> Enable describing I3C or I2C devices in the ACPI table. This is required
> if the device uses a static address or if it needs device-specific
> properties.
>
> [1] https://www.mipi.org/mipi-disco-for-i3c-download
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624102153.1770072-1-akhilrajeev@nvidia.com?part=3
^ permalink raw reply
* Re: [PATCH v5 07/12] i3c: dw-i3c-master: Add SETAASA as supported CCC
From: sashiko-bot @ 2026-06-24 10:34 UTC (permalink / raw)
To: Akhil R; +Cc: linux-i3c, robh, Frank.Li, conor+dt, devicetree, linux-hwmon
In-Reply-To: <20260624102153.1770072-8-akhilrajeev@nvidia.com>
> Add SETAASA and SETHID to the supported list of CCC commands for
> DesignWare I3C host controller.
>
> SETAASA is a broadcast command that assigns predefined static addresses
> to all I3C devices on the bus.
>
> SETHID is to stop HID bit flipping by the SPD Hub to which the SPD devices
> are connected. It is a prerequisite command to be sent before SETAASA as
> recommended by JESD300-5 and JESD403 sideband bus specifications.
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624102153.1770072-1-akhilrajeev@nvidia.com?part=7
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: axiado: Add initial support for AX3005 SoC and eval board
From: sashiko-bot @ 2026-06-24 10:34 UTC (permalink / raw)
To: Swark Yang; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260624-upstream-axiado-ax3005-upstream-v1-2-c05bd0bc9124@axiado.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] All secondary CPUs share the exact same `cpu-release-addr` for the SMP spin-table.
- [Medium] The ARMv8 architectural timer and GIC maintenance interrupts are incorrectly configured as active-high.
- [Low] Mismatch between the memory node's unit address and its `reg` base address.
- [Low] Missing `clock-frequency` property in the `timer` node despite indicating firmware did not configure registers.
--
commit 9c7369abb8e810cca148d4bf6cc3f8266fdfad15
Author: Swark Yang <syang@axiado.com>
Subject: arm64: dts: axiado: Add initial support for AX3005 SoC and eval board
This commit adds initial device tree support for the AX3005 SoC and its
evaluation board. The AX3005 is a multi-core SoC featuring 4 Cortex-A53
cores, and this adds the CPUs, timer, GPIO, UART, I2C, I3C, SPI and USB
controllers.
Link: None
> diff --git a/arch/arm64/boot/dts/axiado/ax3005-evk.dts b/arch/arm64/boot/dts/axiado/ax3005-evk.dts
> new file mode 100644
> --- /dev/null
> +++ b/arch/arm64/boot/dts/axiado/ax3005-evk.dts
[ ... ]
> + memory@0 {
> + device_type = "memory";
> + /* Cortex-A53 will use following memory map */
> + reg = <0x0 0x81000000 0x0 0x7f000000>;
> + };
[Severity: Low]
Should this unit address match the base address? The memory node specifies a
base address of 0x81000000 in its reg property, but the unit address is @0.
This will cause a dtc compilation warning for unit_address_vs_reg.
[ ... ]
> diff --git a/arch/arm64/boot/dts/axiado/ax3005.dtsi b/arch/arm64/boot/dts/axiado/ax3005.dtsi
> new file mode 100644
> --- /dev/null
> +++ b/arch/arm64/boot/dts/axiado/ax3005.dtsi
[ ... ]
> + cpu1: cpu@1 {
> + compatible = "arm,cortex-a53";
> + device_type = "cpu";
> + reg = <0x0 0x1>;
> + enable-method = "spin-table";
> + cpu-release-addr = <0x0 0x80002fa0>;
[Severity: High]
Are all secondary CPUs supposed to share the exact same cpu-release-addr?
If cpu1, cpu2, and cpu3 all poll 0x80002fa0, the first wake event will
trigger all of them simultaneously. Will this cause concurrent jumps into
secondary_startup and corrupt the stack during SMP bring-up since they would
all attempt to consume the single cpu_boot_task?
[ ... ]
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupt-parent = <&gic500>;
> + interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_PPI 14 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_PPI 10 IRQ_TYPE_LEVEL_HIGH>;
[Severity: Medium]
Should the ARMv8 architectural timer PPIs be active-low? The generic timer
output signals usually generate active-low level-sensitive interrupts
(IRQ_TYPE_LEVEL_LOW), but these are defined as IRQ_TYPE_LEVEL_HIGH here.
> + arm,cpu-registers-not-fw-configured;
[Severity: Low]
Does this timer node need a clock-frequency property?
The arm,cpu-registers-not-fw-configured property indicates firmware did not
initialize the timer frequency register. If CNTFRQ_EL0 is actually
unconfigured, omitting the fallback frequency can cause boot warnings or
failure in the timer subsystem.
> + };
[ ... ]
> + gic500: interrupt-controller@40400000 {
> + compatible = "arm,gic-v3";
> + reg = <0x0 0x40400000 0x0 0x10000>,
> + <0x0 0x40500000 0x0 0xc0000>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + #interrupt-cells = <3>;
> + interrupt-controller;
> + #redistributor-regions = <1>;
> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
[Severity: Medium]
Does the GICv3 VGIC maintenance interrupt need to be active-low? Similar to
the timer interrupts, the architecture specifies this as an active-low
level-sensitive interrupt, but it is declared as IRQ_TYPE_LEVEL_HIGH here.
> + };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-upstream-axiado-ax3005-upstream-v1-0-c05bd0bc9124@axiado.com?part=2
^ permalink raw reply
* Re: [PATCH v5 10/12] hwmon: spd5118: Remove 16-bit addressing
From: sashiko-bot @ 2026-06-24 10:33 UTC (permalink / raw)
To: Akhil R; +Cc: linux-i3c, devicetree, conor+dt, robh, linux-hwmon, Frank.Li
In-Reply-To: <20260624102153.1770072-11-akhilrajeev@nvidia.com>
> The intent of introducing 16-bit addressing was to support I3C, but it
> turns out that I3C does not require reading the Legacy Mode register,
> nor any specific encoding for page translation. The testing of 16-bit
> code was limited and there are no known users for this feature. Remove
> the sections that support 16-bit addressing and prepare the driver to
> support I3C appropriately.
>
> Suggested-by: Guenter Roeck <linux@roeck-us.net>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624102153.1770072-1-akhilrajeev@nvidia.com?part=10
^ permalink raw reply
* Re: [PATCH v5 09/12] i3c: dw-i3c-master: Add ACPI ID for Tegra410
From: sashiko-bot @ 2026-06-24 10:32 UTC (permalink / raw)
To: Akhil R; +Cc: robh, conor+dt, devicetree, linux-i3c, Frank.Li, linux-hwmon
In-Reply-To: <20260624102153.1770072-10-akhilrajeev@nvidia.com>
> Update variable names to generic names and add Tegra410 ACPI ID to
> support the I3C controller in Tegra410, which is a DesignWare I3C host
> controller.
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624102153.1770072-1-akhilrajeev@nvidia.com?part=9
^ permalink raw reply
* Re: [PATCH v5 01/12] dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA
From: sashiko-bot @ 2026-06-24 10:30 UTC (permalink / raw)
To: Akhil R; +Cc: linux-hwmon, linux-i3c, Frank.Li, devicetree, conor+dt, robh
In-Reply-To: <20260624102153.1770072-2-akhilrajeev@nvidia.com>
> Add the 'mipi-i3c-static-method' property mentioned in the MIPI I3C
> Discovery and Configuration Specification [1] to specify which discovery
> method an I3C device supports during bus initialization. The property is
> a bitmap, where a bit value of 1 indicates support for that method, and 0
> indicates lack of support.
>
> Bit 0: SETDASA CCC (Direct)
> Bit 1: SETAASA CCC (Broadcast)
> Bit 2: Other CCC (vendor / standards extension)
> All other bits are reserved.
>
> It is specifically needed when an I3C device requires SETAASA for the
> address assignment. SETDASA will be supported by default if this property
> is absent, which means for now the property just serves as a flag to
> enable SETAASA, but keep the property as a bitmap to align with the
> specifications.
>
> [1] https://www.mipi.org/mipi-disco-for-i3c-download
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624102153.1770072-1-akhilrajeev@nvidia.com?part=1
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: socfpga: agilex7-gen2: Add initial device tree
From: Krzysztof Kozlowski @ 2026-06-24 10:26 UTC (permalink / raw)
To: Nazle Asmade, Muhammad Nazim Amirul
Cc: dinguyen@kernel.org, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <4acdd3ee-c7eb-4871-bb59-c2e3eeedda28@altera.com>
On 24/06/2026 12:17, Nazle Asmade, Muhammad Nazim Amirul wrote:
>> SoC without any interface, serial or storage or network, is close to
>> useless one.
>>
>> I don't see a point in having it in mainline. Serial is usually ABSOLUTE
>> minimum.
>>
>> Best regards,
>> Krzysztof
>>
> Hi Krzysztof,
>
> Thank you for the review and fast response!
>
> I ran both dt_binding_check and dtbs_check (with CHECK_DTBS=y) locally —
> both passed without errors. Could you clarify which specific test you
> believe is failing?
I would expect simple-bus schema warning or W=1, because node is placed
outside of soc@, but maybe there is no such.
>
> Regarding "MMIO goes to MMIO" — are you referring to the GIC
Comments are placed in very specific and intentional place. Please read
guides how mailing list in-line review works before posting patches.
> (interrupt-controller@7000000) being placed at the root level instead of
> under the soc bus node?
>
> Regarding the serial console — the platform clock driver is not yet
> upstream, so the UART depends on clkmgr. Would adding the UART with
> clock-frequency be acceptable as an interim solution?
Add complete working serial. Why can't you use fixed placeholder clock?
There are probably multiple ways to solve it, not necessary
clock-frequency and I do not even remember if clock-frequency is allowed.
But if you cannot bring serial, then my comment stays valid: this is
unusable upstream thus is not ready to be posted and merged.
Best regards,
Krzysztof
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox