* [PATCH 0/3] Grab IPA IMEM slice through DT
@ 2025-05-22 23:08 Konrad Dybcio
2025-05-22 23:08 ` [PATCH 1/3] dt-bindings: sram: qcom,imem: Allow modem-tables Konrad Dybcio
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Konrad Dybcio @ 2025-05-22 23:08 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alex Elder
Cc: Marijn Suijten, linux-arm-msm, devicetree, linux-kernel, netdev,
Konrad Dybcio
This adds the necessary driver change to migrate over from
hardcoded-per-IPA-version-but-varying-per-implementation numbers, while
unfortunately keeping them in there for backwards compatibility.
The DT changes will be submitted in a separate series, this one is OK
to merge independently.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Konrad Dybcio (3):
dt-bindings: sram: qcom,imem: Allow modem-tables
dt-bindings: net: qcom,ipa: Add sram property for describing IMEM slice
net: ipa: Grab IMEM slice base/size from DTS
Documentation/devicetree/bindings/net/qcom,ipa.yaml | 7 +++++++
Documentation/devicetree/bindings/sram/qcom,imem.yaml | 3 +++
drivers/net/ipa/ipa_data.h | 3 +++
drivers/net/ipa/ipa_mem.c | 18 ++++++++++++++++++
4 files changed, 31 insertions(+)
---
base-commit: 460178e842c7a1e48a06df684c66eb5fd630bcf7
change-id: 20250523-topic-ipa_imem-def66cca88e5
Best regards,
--
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] dt-bindings: sram: qcom,imem: Allow modem-tables
2025-05-22 23:08 [PATCH 0/3] Grab IPA IMEM slice through DT Konrad Dybcio
@ 2025-05-22 23:08 ` Konrad Dybcio
2025-05-23 17:59 ` Alex Elder
2025-05-22 23:08 ` [PATCH 2/3] dt-bindings: net: qcom,ipa: Add sram property for describing IMEM slice Konrad Dybcio
2025-05-22 23:08 ` [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS Konrad Dybcio
2 siblings, 1 reply; 12+ messages in thread
From: Konrad Dybcio @ 2025-05-22 23:08 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alex Elder
Cc: Marijn Suijten, linux-arm-msm, devicetree, linux-kernel, netdev,
Konrad Dybcio
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
The IP Accelerator hardware/firmware owns a sizeable region within the
IMEM, ominously named 'modem-tables', presumably having to do with some
internal IPA-modem specifics.
It's not actually accessed by the OS, although we have to IOMMU-map it
with the IPA device, so that presumably the firmware can act upon it.
Allow it as a subnode of IMEM.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Documentation/devicetree/bindings/sram/qcom,imem.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/sram/qcom,imem.yaml b/Documentation/devicetree/bindings/sram/qcom,imem.yaml
index 2711f90d9664b70fcd1e2f7e2dfd3386ed5c1952..7c882819222dc04190db357ac6f9a3a35137cc9e 100644
--- a/Documentation/devicetree/bindings/sram/qcom,imem.yaml
+++ b/Documentation/devicetree/bindings/sram/qcom,imem.yaml
@@ -51,6 +51,9 @@ properties:
$ref: /schemas/power/reset/syscon-reboot-mode.yaml#
patternProperties:
+ "^modem-tables@[0-9a-f]+$":
+ description: Region reserved for the IP Accelerator
+
"^pil-reloc@[0-9a-f]+$":
$ref: /schemas/remoteproc/qcom,pil-info.yaml#
description: Peripheral image loader relocation region
--
2.49.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] dt-bindings: net: qcom,ipa: Add sram property for describing IMEM slice
2025-05-22 23:08 [PATCH 0/3] Grab IPA IMEM slice through DT Konrad Dybcio
2025-05-22 23:08 ` [PATCH 1/3] dt-bindings: sram: qcom,imem: Allow modem-tables Konrad Dybcio
@ 2025-05-22 23:08 ` Konrad Dybcio
2025-05-23 17:59 ` Alex Elder
2025-05-22 23:08 ` [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS Konrad Dybcio
2 siblings, 1 reply; 12+ messages in thread
From: Konrad Dybcio @ 2025-05-22 23:08 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alex Elder
Cc: Marijn Suijten, linux-arm-msm, devicetree, linux-kernel, netdev,
Konrad Dybcio
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
The IPA driver currently grabs a slice of IMEM through hardcoded
addresses. Not only is that ugly and against the principles of DT,
but it also creates a situation where two distinct platforms
implementing the same version of IPA would need to be hardcoded
together and matched at runtime.
Instead, do the sane thing and accept a handle to said region directly.
Don't make it required on purpose, as a) it's not there on ancient
implementations (currently unsupported) and we're not yet done with
filling the data across al DTs.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Documentation/devicetree/bindings/net/qcom,ipa.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/qcom,ipa.yaml b/Documentation/devicetree/bindings/net/qcom,ipa.yaml
index b4a79912d4739bec33933cdd7bb5e720eb41c814..1109f4d170af7178b998c6b7d415cc60de1c58c5 100644
--- a/Documentation/devicetree/bindings/net/qcom,ipa.yaml
+++ b/Documentation/devicetree/bindings/net/qcom,ipa.yaml
@@ -166,6 +166,13 @@ properties:
initializing IPA hardware. Optional, and only used when
Trust Zone performs early initialization.
+ sram:
+ maxItems: 1
+ description:
+ A reference to an additional region residing in IMEM (special
+ on-chip SRAM), which is accessed by the IPA firmware and needs
+ to be IOMMU-mapped from the OS.
+
required:
- compatible
- iommus
--
2.49.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS
2025-05-22 23:08 [PATCH 0/3] Grab IPA IMEM slice through DT Konrad Dybcio
2025-05-22 23:08 ` [PATCH 1/3] dt-bindings: sram: qcom,imem: Allow modem-tables Konrad Dybcio
2025-05-22 23:08 ` [PATCH 2/3] dt-bindings: net: qcom,ipa: Add sram property for describing IMEM slice Konrad Dybcio
@ 2025-05-22 23:08 ` Konrad Dybcio
2025-05-23 5:49 ` Dmitry Baryshkov
` (3 more replies)
2 siblings, 4 replies; 12+ messages in thread
From: Konrad Dybcio @ 2025-05-22 23:08 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alex Elder
Cc: Marijn Suijten, linux-arm-msm, devicetree, linux-kernel, netdev,
Konrad Dybcio
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
This is a detail that differ per chip, and not per IPA version (and
there are cases of the same IPA versions being implemented across very
very very different SoCs).
This region isn't actually used by the driver, but we most definitely
want to iommu-map it, so that IPA can poke at the data within.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
drivers/net/ipa/ipa_data.h | 3 +++
drivers/net/ipa/ipa_mem.c | 18 ++++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h
index 2fd03f0799b207833f9f2b421ce043534720d718..a384df91b5ee3ed2db9c7812ad43d03424b82a6f 100644
--- a/drivers/net/ipa/ipa_data.h
+++ b/drivers/net/ipa/ipa_data.h
@@ -185,8 +185,11 @@ struct ipa_resource_data {
struct ipa_mem_data {
u32 local_count;
const struct ipa_mem *local;
+
+ /* DEPRECATED (now passed via DT) fallback data, varies per chip and not per IPA version */
u32 imem_addr;
u32 imem_size;
+
u32 smem_size;
};
diff --git a/drivers/net/ipa/ipa_mem.c b/drivers/net/ipa/ipa_mem.c
index 835a3c9c1fd47167da3396424a1653ebcae81d40..020508ab47d92b5cca9d5b467e3fef46936b4a82 100644
--- a/drivers/net/ipa/ipa_mem.c
+++ b/drivers/net/ipa/ipa_mem.c
@@ -7,6 +7,7 @@
#include <linux/dma-mapping.h>
#include <linux/io.h>
#include <linux/iommu.h>
+#include <linux/of_address.h>
#include <linux/platform_device.h>
#include <linux/types.h>
@@ -617,7 +618,9 @@ static void ipa_smem_exit(struct ipa *ipa)
int ipa_mem_init(struct ipa *ipa, struct platform_device *pdev,
const struct ipa_mem_data *mem_data)
{
+ struct device_node *ipa_slice_np;
struct device *dev = &pdev->dev;
+ u32 imem_base, imem_size;
struct resource *res;
int ret;
@@ -656,6 +659,21 @@ int ipa_mem_init(struct ipa *ipa, struct platform_device *pdev,
ipa->mem_addr = res->start;
ipa->mem_size = resource_size(res);
+ ipa_slice_np = of_parse_phandle(dev->of_node, "sram", 0);
+ if (ipa_slice_np) {
+ ret = of_address_to_resource(ipa_slice_np, 0, res);
+ of_node_put(ipa_slice_np);
+ if (ret)
+ return ret;
+
+ imem_base = res->start;
+ imem_size = resource_size(res);
+ } else {
+ /* Backwards compatibility for DTs lacking an explicit reference */
+ imem_base = mem_data->imem_addr;
+ imem_size = mem_data->imem_size;
+ }
+
ret = ipa_imem_init(ipa, mem_data->imem_addr, mem_data->imem_size);
if (ret)
goto err_unmap;
--
2.49.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS
2025-05-22 23:08 ` [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS Konrad Dybcio
@ 2025-05-23 5:49 ` Dmitry Baryshkov
2025-05-23 13:17 ` Simon Horman
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Dmitry Baryshkov @ 2025-05-23 5:49 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alex Elder, Marijn Suijten, linux-arm-msm,
devicetree, linux-kernel, netdev, Konrad Dybcio
On Fri, May 23, 2025 at 01:08:34AM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> This is a detail that differ per chip, and not per IPA version (and
> there are cases of the same IPA versions being implemented across very
> very very different SoCs).
>
> This region isn't actually used by the driver, but we most definitely
> want to iommu-map it, so that IPA can poke at the data within.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> drivers/net/ipa/ipa_data.h | 3 +++
> drivers/net/ipa/ipa_mem.c | 18 ++++++++++++++++++
> 2 files changed, 21 insertions(+)
>
Acked-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS
2025-05-22 23:08 ` [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS Konrad Dybcio
2025-05-23 5:49 ` Dmitry Baryshkov
@ 2025-05-23 13:17 ` Simon Horman
2025-05-23 18:16 ` Konrad Dybcio
2025-05-23 17:59 ` Alex Elder
2025-05-23 18:59 ` kernel test robot
3 siblings, 1 reply; 12+ messages in thread
From: Simon Horman @ 2025-05-23 13:17 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alex Elder, Marijn Suijten, linux-arm-msm,
devicetree, linux-kernel, netdev, Konrad Dybcio
On Fri, May 23, 2025 at 01:08:34AM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> This is a detail that differ per chip, and not per IPA version (and
> there are cases of the same IPA versions being implemented across very
> very very different SoCs).
>
> This region isn't actually used by the driver, but we most definitely
> want to iommu-map it, so that IPA can poke at the data within.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
It looks like these patches are for net-next. For future reference,
it's best to note that in the subject.
Subject: [PATCH net-next 3/3 v2] ...
> ---
> drivers/net/ipa/ipa_data.h | 3 +++
> drivers/net/ipa/ipa_mem.c | 18 ++++++++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h
> index 2fd03f0799b207833f9f2b421ce043534720d718..a384df91b5ee3ed2db9c7812ad43d03424b82a6f 100644
> --- a/drivers/net/ipa/ipa_data.h
> +++ b/drivers/net/ipa/ipa_data.h
> @@ -185,8 +185,11 @@ struct ipa_resource_data {
> struct ipa_mem_data {
> u32 local_count;
> const struct ipa_mem *local;
> +
> + /* DEPRECATED (now passed via DT) fallback data, varies per chip and not per IPA version */
For Networking code, please restrict lines to 80 columns wide or less where
that can be done without reducing readability (which is the case here).
/* DEPRECATED (now passed via DT) fallback data,
* varies per chip and not per IPA version */
> u32 imem_addr;
> u32 imem_size;
> +
> u32 smem_size;
> };
>
> diff --git a/drivers/net/ipa/ipa_mem.c b/drivers/net/ipa/ipa_mem.c
> index 835a3c9c1fd47167da3396424a1653ebcae81d40..020508ab47d92b5cca9d5b467e3fef46936b4a82 100644
> --- a/drivers/net/ipa/ipa_mem.c
> +++ b/drivers/net/ipa/ipa_mem.c
> @@ -7,6 +7,7 @@
> #include <linux/dma-mapping.h>
> #include <linux/io.h>
> #include <linux/iommu.h>
> +#include <linux/of_address.h>
> #include <linux/platform_device.h>
> #include <linux/types.h>
>
> @@ -617,7 +618,9 @@ static void ipa_smem_exit(struct ipa *ipa)
> int ipa_mem_init(struct ipa *ipa, struct platform_device *pdev,
> const struct ipa_mem_data *mem_data)
> {
> + struct device_node *ipa_slice_np;
> struct device *dev = &pdev->dev;
> + u32 imem_base, imem_size;
> struct resource *res;
> int ret;
>
> @@ -656,6 +659,21 @@ int ipa_mem_init(struct ipa *ipa, struct platform_device *pdev,
> ipa->mem_addr = res->start;
> ipa->mem_size = resource_size(res);
>
> + ipa_slice_np = of_parse_phandle(dev->of_node, "sram", 0);
> + if (ipa_slice_np) {
> + ret = of_address_to_resource(ipa_slice_np, 0, res);
> + of_node_put(ipa_slice_np);
> + if (ret)
> + return ret;
> +
> + imem_base = res->start;
> + imem_size = resource_size(res);
> + } else {
> + /* Backwards compatibility for DTs lacking an explicit reference */
Ditto.
> + imem_base = mem_data->imem_addr;
> + imem_size = mem_data->imem_size;
> + }
> +
> ret = ipa_imem_init(ipa, mem_data->imem_addr, mem_data->imem_size);
I think you also need to update this line to use the local
variables imem_addr and imem_size.
> if (ret)
> goto err_unmap;
Please do observe the 24h rule [1] if you post a v2 of this patchset.
[1] https://docs.kernel.org/process/maintainer-netdev.html#resending-after-review
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] dt-bindings: sram: qcom,imem: Allow modem-tables
2025-05-22 23:08 ` [PATCH 1/3] dt-bindings: sram: qcom,imem: Allow modem-tables Konrad Dybcio
@ 2025-05-23 17:59 ` Alex Elder
2025-05-23 18:13 ` Konrad Dybcio
0 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2025-05-23 17:59 UTC (permalink / raw)
To: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alex Elder
Cc: Marijn Suijten, linux-arm-msm, devicetree, linux-kernel, netdev,
Konrad Dybcio
On 5/22/25 6:08 PM, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> The IP Accelerator hardware/firmware owns a sizeable region within the
> IMEM, ominously named 'modem-tables', presumably having to do with some
> internal IPA-modem specifics.
>
> It's not actually accessed by the OS, although we have to IOMMU-map it
> with the IPA device, so that presumably the firmware can act upon it.
>
> Allow it as a subnode of IMEM.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
So this will just show up as a subnode of an sram@... node,
the way "qcom,pil-reloc-info" does. This is great.
Is it called "modem-tables" in internal documentation? Or
did you choose this ominous name?
Reviewed-by: Alex Elder <elder@riscstar.com>
> ---
> Documentation/devicetree/bindings/sram/qcom,imem.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/sram/qcom,imem.yaml b/Documentation/devicetree/bindings/sram/qcom,imem.yaml
> index 2711f90d9664b70fcd1e2f7e2dfd3386ed5c1952..7c882819222dc04190db357ac6f9a3a35137cc9e 100644
> --- a/Documentation/devicetree/bindings/sram/qcom,imem.yaml
> +++ b/Documentation/devicetree/bindings/sram/qcom,imem.yaml
> @@ -51,6 +51,9 @@ properties:
> $ref: /schemas/power/reset/syscon-reboot-mode.yaml#
>
> patternProperties:
> + "^modem-tables@[0-9a-f]+$":
> + description: Region reserved for the IP Accelerator
> +
> "^pil-reloc@[0-9a-f]+$":
> $ref: /schemas/remoteproc/qcom,pil-info.yaml#
> description: Peripheral image loader relocation region
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: net: qcom,ipa: Add sram property for describing IMEM slice
2025-05-22 23:08 ` [PATCH 2/3] dt-bindings: net: qcom,ipa: Add sram property for describing IMEM slice Konrad Dybcio
@ 2025-05-23 17:59 ` Alex Elder
0 siblings, 0 replies; 12+ messages in thread
From: Alex Elder @ 2025-05-23 17:59 UTC (permalink / raw)
To: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alex Elder
Cc: Marijn Suijten, linux-arm-msm, devicetree, linux-kernel, netdev,
Konrad Dybcio
On 5/22/25 6:08 PM, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> The IPA driver currently grabs a slice of IMEM through hardcoded
> addresses. Not only is that ugly and against the principles of DT,
> but it also creates a situation where two distinct platforms
> implementing the same version of IPA would need to be hardcoded
> together and matched at runtime.
>
> Instead, do the sane thing and accept a handle to said region directly.
>
> Don't make it required on purpose, as a) it's not there on ancient
> implementations (currently unsupported) and we're not yet done with
> filling the data across al DTs.
We have to support "ancient" DTBs, right? So unfortunately
the fallback can't go away.
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Looks good.
Reviewed-by: Alex Elder <elder@riscstar.com>
> ---
> Documentation/devicetree/bindings/net/qcom,ipa.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/qcom,ipa.yaml b/Documentation/devicetree/bindings/net/qcom,ipa.yaml
> index b4a79912d4739bec33933cdd7bb5e720eb41c814..1109f4d170af7178b998c6b7d415cc60de1c58c5 100644
> --- a/Documentation/devicetree/bindings/net/qcom,ipa.yaml
> +++ b/Documentation/devicetree/bindings/net/qcom,ipa.yaml
> @@ -166,6 +166,13 @@ properties:
> initializing IPA hardware. Optional, and only used when
> Trust Zone performs early initialization.
>
> + sram:
> + maxItems: 1
> + description:
> + A reference to an additional region residing in IMEM (special
> + on-chip SRAM), which is accessed by the IPA firmware and needs
> + to be IOMMU-mapped from the OS.
> +
> required:
> - compatible
> - iommus
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS
2025-05-22 23:08 ` [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS Konrad Dybcio
2025-05-23 5:49 ` Dmitry Baryshkov
2025-05-23 13:17 ` Simon Horman
@ 2025-05-23 17:59 ` Alex Elder
2025-05-23 18:59 ` kernel test robot
3 siblings, 0 replies; 12+ messages in thread
From: Alex Elder @ 2025-05-23 17:59 UTC (permalink / raw)
To: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alex Elder
Cc: Marijn Suijten, linux-arm-msm, devicetree, linux-kernel, netdev,
Konrad Dybcio
On 5/22/25 6:08 PM, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> This is a detail that differ per chip, and not per IPA version (and
> there are cases of the same IPA versions being implemented across very
> very very different SoCs).
>
> This region isn't actually used by the driver, but we most definitely
> want to iommu-map it, so that IPA can poke at the data within.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
You need to fix something here, but it otherwise look good.
Please fix, and assuming you do:
Reviewed-by: Alex Elder <elder@riscstar.com>
> ---
> drivers/net/ipa/ipa_data.h | 3 +++
> drivers/net/ipa/ipa_mem.c | 18 ++++++++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h
> index 2fd03f0799b207833f9f2b421ce043534720d718..a384df91b5ee3ed2db9c7812ad43d03424b82a6f 100644
> --- a/drivers/net/ipa/ipa_data.h
> +++ b/drivers/net/ipa/ipa_data.h
> @@ -185,8 +185,11 @@ struct ipa_resource_data {
> struct ipa_mem_data {
> u32 local_count;
> const struct ipa_mem *local;
> +
> + /* DEPRECATED (now passed via DT) fallback data, varies per chip and not per IPA version */
> u32 imem_addr;
> u32 imem_size;
> +
> u32 smem_size;
> };
>
> diff --git a/drivers/net/ipa/ipa_mem.c b/drivers/net/ipa/ipa_mem.c
> index 835a3c9c1fd47167da3396424a1653ebcae81d40..020508ab47d92b5cca9d5b467e3fef46936b4a82 100644
> --- a/drivers/net/ipa/ipa_mem.c
> +++ b/drivers/net/ipa/ipa_mem.c
> @@ -7,6 +7,7 @@
> #include <linux/dma-mapping.h>
> #include <linux/io.h>
> #include <linux/iommu.h>
> +#include <linux/of_address.h>
> #include <linux/platform_device.h>
> #include <linux/types.h>
>
> @@ -617,7 +618,9 @@ static void ipa_smem_exit(struct ipa *ipa)
> int ipa_mem_init(struct ipa *ipa, struct platform_device *pdev,
> const struct ipa_mem_data *mem_data)
> {
> + struct device_node *ipa_slice_np;
> struct device *dev = &pdev->dev;
> + u32 imem_base, imem_size;
> struct resource *res;
> int ret;
>
> @@ -656,6 +659,21 @@ int ipa_mem_init(struct ipa *ipa, struct platform_device *pdev,
> ipa->mem_addr = res->start;
> ipa->mem_size = resource_size(res);
>
> + ipa_slice_np = of_parse_phandle(dev->of_node, "sram", 0);
> + if (ipa_slice_np) {
> + ret = of_address_to_resource(ipa_slice_np, 0, res);
> + of_node_put(ipa_slice_np);
> + if (ret)
> + return ret;
> +
> + imem_base = res->start;
> + imem_size = resource_size(res);
> + } else {
> + /* Backwards compatibility for DTs lacking an explicit reference */
> + imem_base = mem_data->imem_addr;
> + imem_size = mem_data->imem_size;
> + }
> +
> ret = ipa_imem_init(ipa, mem_data->imem_addr, mem_data->imem_size);
You want to pass imem_base and imem_size here?
-Alex
> if (ret)
> goto err_unmap;
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] dt-bindings: sram: qcom,imem: Allow modem-tables
2025-05-23 17:59 ` Alex Elder
@ 2025-05-23 18:13 ` Konrad Dybcio
0 siblings, 0 replies; 12+ messages in thread
From: Konrad Dybcio @ 2025-05-23 18:13 UTC (permalink / raw)
To: Alex Elder, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alex Elder
Cc: Marijn Suijten, linux-arm-msm, devicetree, linux-kernel, netdev
On 5/23/25 7:59 PM, Alex Elder wrote:
> On 5/22/25 6:08 PM, Konrad Dybcio wrote:
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>
>> The IP Accelerator hardware/firmware owns a sizeable region within the
>> IMEM, ominously named 'modem-tables', presumably having to do with some
>> internal IPA-modem specifics.
>>
>> It's not actually accessed by the OS, although we have to IOMMU-map it
>> with the IPA device, so that presumably the firmware can act upon it.
>>
>> Allow it as a subnode of IMEM.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> So this will just show up as a subnode of an sram@... node,
> the way "qcom,pil-reloc-info" does. This is great.
>
> Is it called "modem-tables" in internal documentation? Or
> did you choose this ominous name?
Downstream. It's hard to find accurate information on this.
Konrad
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS
2025-05-23 13:17 ` Simon Horman
@ 2025-05-23 18:16 ` Konrad Dybcio
0 siblings, 0 replies; 12+ messages in thread
From: Konrad Dybcio @ 2025-05-23 18:16 UTC (permalink / raw)
To: Simon Horman, Konrad Dybcio
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alex Elder, Marijn Suijten, linux-arm-msm,
devicetree, linux-kernel, netdev
On 5/23/25 3:17 PM, Simon Horman wrote:
> On Fri, May 23, 2025 at 01:08:34AM +0200, Konrad Dybcio wrote:
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>
>> This is a detail that differ per chip, and not per IPA version (and
>> there are cases of the same IPA versions being implemented across very
>> very very different SoCs).
>>
>> This region isn't actually used by the driver, but we most definitely
>> want to iommu-map it, so that IPA can poke at the data within.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> It looks like these patches are for net-next. For future reference,
> it's best to note that in the subject.
>
> Subject: [PATCH net-next 3/3 v2] ...
>
>> ---
ah, the networking guys and their customs ;)
[...]
>> ret = ipa_imem_init(ipa, mem_data->imem_addr, mem_data->imem_size);
>
> I think you also need to update this line to use the local
> variables imem_addr and imem_size.
I paid great attention to validate that the data I got was good and printed
the value inside the first if branch.. but failed to change it here. Thanks
for catching it!
Konrad
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS
2025-05-22 23:08 ` [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS Konrad Dybcio
` (2 preceding siblings ...)
2025-05-23 17:59 ` Alex Elder
@ 2025-05-23 18:59 ` kernel test robot
3 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2025-05-23 18:59 UTC (permalink / raw)
To: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alex Elder
Cc: oe-kbuild-all, netdev, Marijn Suijten, linux-arm-msm, devicetree,
linux-kernel, Konrad Dybcio
Hi Konrad,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 460178e842c7a1e48a06df684c66eb5fd630bcf7]
url: https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/dt-bindings-sram-qcom-imem-Allow-modem-tables/20250523-071359
base: 460178e842c7a1e48a06df684c66eb5fd630bcf7
patch link: https://lore.kernel.org/r/20250523-topic-ipa_imem-v1-3-b5d536291c7f%40oss.qualcomm.com
patch subject: [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS
config: xtensa-randconfig-002-20250524 (https://download.01.org/0day-ci/archive/20250524/202505240200.w0D4DdAY-lkp@intel.com/config)
compiler: xtensa-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250524/202505240200.w0D4DdAY-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505240200.w0D4DdAY-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/net/ipa/ipa_mem.c: In function 'ipa_mem_init':
>> drivers/net/ipa/ipa_mem.c:623:17: warning: variable 'imem_size' set but not used [-Wunused-but-set-variable]
u32 imem_base, imem_size;
^~~~~~~~~
>> drivers/net/ipa/ipa_mem.c:623:6: warning: variable 'imem_base' set but not used [-Wunused-but-set-variable]
u32 imem_base, imem_size;
^~~~~~~~~
vim +/imem_size +623 drivers/net/ipa/ipa_mem.c
616
617 /* Perform memory region-related initialization */
618 int ipa_mem_init(struct ipa *ipa, struct platform_device *pdev,
619 const struct ipa_mem_data *mem_data)
620 {
621 struct device_node *ipa_slice_np;
622 struct device *dev = &pdev->dev;
> 623 u32 imem_base, imem_size;
624 struct resource *res;
625 int ret;
626
627 /* Make sure the set of defined memory regions is valid */
628 if (!ipa_mem_valid(ipa, mem_data))
629 return -EINVAL;
630
631 ipa->mem_count = mem_data->local_count;
632 ipa->mem = mem_data->local;
633
634 /* Check the route and filter table memory regions */
635 if (!ipa_table_mem_valid(ipa, false))
636 return -EINVAL;
637 if (!ipa_table_mem_valid(ipa, true))
638 return -EINVAL;
639
640 ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
641 if (ret) {
642 dev_err(dev, "error %d setting DMA mask\n", ret);
643 return ret;
644 }
645
646 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ipa-shared");
647 if (!res) {
648 dev_err(dev,
649 "DT error getting \"ipa-shared\" memory property\n");
650 return -ENODEV;
651 }
652
653 ipa->mem_virt = memremap(res->start, resource_size(res), MEMREMAP_WC);
654 if (!ipa->mem_virt) {
655 dev_err(dev, "unable to remap \"ipa-shared\" memory\n");
656 return -ENOMEM;
657 }
658
659 ipa->mem_addr = res->start;
660 ipa->mem_size = resource_size(res);
661
662 ipa_slice_np = of_parse_phandle(dev->of_node, "sram", 0);
663 if (ipa_slice_np) {
664 ret = of_address_to_resource(ipa_slice_np, 0, res);
665 of_node_put(ipa_slice_np);
666 if (ret)
667 return ret;
668
669 imem_base = res->start;
670 imem_size = resource_size(res);
671 } else {
672 /* Backwards compatibility for DTs lacking an explicit reference */
673 imem_base = mem_data->imem_addr;
674 imem_size = mem_data->imem_size;
675 }
676
677 ret = ipa_imem_init(ipa, mem_data->imem_addr, mem_data->imem_size);
678 if (ret)
679 goto err_unmap;
680
681 ret = ipa_smem_init(ipa, mem_data->smem_size);
682 if (ret)
683 goto err_imem_exit;
684
685 return 0;
686
687 err_imem_exit:
688 ipa_imem_exit(ipa);
689 err_unmap:
690 memunmap(ipa->mem_virt);
691
692 return ret;
693 }
694
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-05-23 19:00 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-22 23:08 [PATCH 0/3] Grab IPA IMEM slice through DT Konrad Dybcio
2025-05-22 23:08 ` [PATCH 1/3] dt-bindings: sram: qcom,imem: Allow modem-tables Konrad Dybcio
2025-05-23 17:59 ` Alex Elder
2025-05-23 18:13 ` Konrad Dybcio
2025-05-22 23:08 ` [PATCH 2/3] dt-bindings: net: qcom,ipa: Add sram property for describing IMEM slice Konrad Dybcio
2025-05-23 17:59 ` Alex Elder
2025-05-22 23:08 ` [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS Konrad Dybcio
2025-05-23 5:49 ` Dmitry Baryshkov
2025-05-23 13:17 ` Simon Horman
2025-05-23 18:16 ` Konrad Dybcio
2025-05-23 17:59 ` Alex Elder
2025-05-23 18:59 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).