All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5] arm64: dts: qcom: qcom6490-idp: Add IPA node
@ 2026-07-22  8:23 Pavan Kumar
  2026-07-22  8:33 ` sashiko-bot
  2026-07-22 12:17 ` Dmitry Baryshkov
  0 siblings, 2 replies; 3+ messages in thread
From: Pavan Kumar @ 2026-07-22  8:23 UTC (permalink / raw)
  To: andersson, konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm,
	devicetree, linux-kernel
  Cc: Pavan Kumar

Enable the IPA block and provide the firmware name
required for firmware loading on qcm6490-idp.

Signed-off-by: Pavan Kumar <pavan.m@oss.qualcomm.com>
---
Changes in v5:
- Reworked commit message to describe the functionality being enabled.
- Removed IPA property consolidation explanation.
- Removed obsolete Link tag to IPA loader series.
- Clarified that the change applies to qcm6490-idp.
Changes in v4:
- Removed empty line between Link and Signed-off-by tags.
- Added empty line before the status property in the IPA node.
- Sent as a new thread instead of replying to the previous revision.
Changes in v3:
- Rebased on top of IPA property consolidation series.
- Dropped qcom,gsi-loader and memory-region properties as they are now
  provided by kodiak.dtsi.
- Added Link tag to the dependent IPA loader series.
Changes in v2:
- Modified firmware file extension from .mdt to .mbn

v4: https://lore.kernel.org/all/20260703085809.2573214-1-sarat.addepalli@oss.qualcomm.com/
v3: https://lore.kernel.org/all/20260702105509.1360340-1-sarat.addepalli@oss.qualcomm.com/
v2: https://lore.kernel.org/all/20260618124700.2001191-1-sarat.addepalli@oss.qualcomm.com/
v1: https://lore.kernel.org/all/20250304152133.GA2763820@hu-kapandey-hyd.qualcomm.com/
---
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
index bdc02260f902..7242695aed97 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -619,6 +619,12 @@ &gpu_zap_shader {
 	firmware-name = "qcom/qcm6490/a660_zap.mbn";
 };
 
+&ipa {
+	firmware-name = "qcom/qcm6490/ipa_fws.mbn";
+
+	status = "okay";
+};
+
 &lpass_rx_macro {
 	status = "okay";
 };

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v5] arm64: dts: qcom: qcom6490-idp: Add IPA node
  2026-07-22  8:23 [PATCH v5] arm64: dts: qcom: qcom6490-idp: Add IPA node Pavan Kumar
@ 2026-07-22  8:33 ` sashiko-bot
  2026-07-22 12:17 ` Dmitry Baryshkov
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-07-22  8:33 UTC (permalink / raw)
  To: Pavan Kumar; +Cc: conor+dt, devicetree, robh

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The IPA device will fail to probe unconditionally due to a missing `memory-region` property, contradicting the commit message claims.
--

--- Patch [1]: [PATCH v5] arm64: dts: qcom: qcom6490-idp: Add IPA node ---
commit 1670b1c514088d32ff0dcb8120b44156a4500dd0
Author: Pavan Kumar <pavan.m@oss.qualcomm.com>

arm64: dts: qcom: qcom6490-idp: Add IPA node

Enable the IPA block and provide the firmware name required for firmware
loading on qcm6490-idp.

> Changes in v3:
> - Rebased on top of IPA property consolidation series.
> - Dropped qcom,gsi-loader and memory-region properties as they are now
>   provided by kodiak.dtsi.

[Severity: High]
Is this description accurate regarding kodiak.dtsi providing these properties?

During system boot, the IPA driver in drivers/net/ipa/ipa_main.c probes the
ipa node. Without a qcom,gsi-loader property, ipa_firmware_loader() defaults
to IPA_LOADER_SELF. This triggers a call to ipa_firmware_load(), which
attempts to fetch the memory-region property via
of_reserved_mem_region_to_resource().

If memory-region is missing in both qcm6490-idp.dts and the included
kodiak.dtsi, the fetch will fail and abort the probe.

> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> index bdc02260f902b..7242695aed978 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts

[ ... ]

> @@ -619,6 +619,12 @@ &gpu_zap_shader {
>  	firmware-name = "qcom/qcm6490/a660_zap.mbn";
>  };
>  
> +&ipa {
> +	firmware-name = "qcom/qcm6490/ipa_fws.mbn";
> +
> +	status = "okay";
> +};
> +

[Severity: High]
Will this node fail to probe unconditionally on the qcm6490-idp board due
to the missing memory-region property?

Additionally, looking earlier in arch/arm64/boot/dts/qcom/qcm6490-idp.dts,
there is an explicit deletion of the firmware memory region:

/delete-node/ &ipa_fw_mem;

If kodiak.dtsi did provide the memory-region property pointing to this
node, wouldn't deleting its target cause a Device Tree Compiler error?
Does the memory-region property need to be explicitly defined here to
allow successful probing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722082314.1502072-1-pavan.m@oss.qualcomm.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v5] arm64: dts: qcom: qcom6490-idp: Add IPA node
  2026-07-22  8:23 [PATCH v5] arm64: dts: qcom: qcom6490-idp: Add IPA node Pavan Kumar
  2026-07-22  8:33 ` sashiko-bot
@ 2026-07-22 12:17 ` Dmitry Baryshkov
  1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Baryshkov @ 2026-07-22 12:17 UTC (permalink / raw)
  To: Pavan Kumar
  Cc: andersson, konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm,
	devicetree, linux-kernel

> [PATCH v5] arm64: dts: qcom: qcom6490-idp: Add IPA node

-ENOSUCHBOARD, it's qcm6490-idp.


On Wed, Jul 22, 2026 at 01:53:14PM +0530, Pavan Kumar wrote:
> Enable the IPA block and provide the firmware name
> required for firmware loading on qcm6490-idp.
> 
> Signed-off-by: Pavan Kumar <pavan.m@oss.qualcomm.com>
> ---
> Changes in v5:
> - Reworked commit message to describe the functionality being enabled.
> - Removed IPA property consolidation explanation.
> - Removed obsolete Link tag to IPA loader series.
> - Clarified that the change applies to qcm6490-idp.
> Changes in v4:
> - Removed empty line between Link and Signed-off-by tags.
> - Added empty line before the status property in the IPA node.
> - Sent as a new thread instead of replying to the previous revision.
> Changes in v3:
> - Rebased on top of IPA property consolidation series.
> - Dropped qcom,gsi-loader and memory-region properties as they are now
>   provided by kodiak.dtsi.
> - Added Link tag to the dependent IPA loader series.
> Changes in v2:
> - Modified firmware file extension from .mdt to .mbn
> 
> v4: https://lore.kernel.org/all/20260703085809.2573214-1-sarat.addepalli@oss.qualcomm.com/
> v3: https://lore.kernel.org/all/20260702105509.1360340-1-sarat.addepalli@oss.qualcomm.com/
> v2: https://lore.kernel.org/all/20260618124700.2001191-1-sarat.addepalli@oss.qualcomm.com/
> v1: https://lore.kernel.org/all/20250304152133.GA2763820@hu-kapandey-hyd.qualcomm.com/
> ---
>  arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Other than the subject:


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>



-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-22 12:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-22  8:23 [PATCH v5] arm64: dts: qcom: qcom6490-idp: Add IPA node Pavan Kumar
2026-07-22  8:33 ` sashiko-bot
2026-07-22 12:17 ` Dmitry Baryshkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.