2018-07-19 17:55:13

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH 0/3] Enable smmu support on sdm845

This series enables apps-smmu (arm,mmu-500) and gpu-smmu (qcom,smmu-v2)
on sdm845. gpu-smmu needs one power domain from gpu clock controller
whose driver was sent by Amit [1].

This series also contains a patch to split the check for num_context_irqs
and num_context_banks in two. We now error out only if num_context_irqs
is greater than num_context_banks.
There are cases such as on sdm845, in which few of the context banks are
reserved by the bootloader (secure software), and thus kernel view
lesser number than the total available context banks.
So, we shouldn't error out simply when the num_context_irqs is not equal
to num_context_banks.

[1] https://lore.kernel.org/patchwork/cover/962208/

Vivek Gautam (3):
dts: arm64/sdm845: Add node for arm,mmu-500
dts: arm64/sdm845: Add node for qcom,smmu-v2
iommu/arm-smmu: Error out only if not enough context interrupts

arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 8 +++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 96 +++++++++++++++++++++++++++++++++
drivers/iommu/arm-smmu.c | 19 ++++---
3 files changed, 117 insertions(+), 6 deletions(-)

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation



2018-07-19 17:55:25

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH 1/3] dts: arm64/sdm845: Add node for arm,mmu-500

Add device node for arm,mmu-500 available on sdm845.
This MMU-500 with single TCU and multiple TBU architecture
is shared among all the peripherals except gpu on sdm845.

Signed-off-by: Vivek Gautam <[email protected]>
---
arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 4 ++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 73 +++++++++++++++++++++++++++++++++
2 files changed, 77 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
index 6d651f314193..13b50dff440f 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -58,3 +58,7 @@
bias-pull-up;
};
};
+
+&apps_smmu {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 00722b533a92..70ca18ae6cb3 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -980,6 +980,79 @@
cell-index = <0>;
};

+ apps_smmu: arm,smmu@15000000 {
+ compatible = "arm,mmu-500";
+ reg = <0x15000000 0x80000>;
+ #iommu-cells = <2>;
+ #global-interrupts = <1>;
+ interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 181 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 182 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 185 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 189 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 190 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 191 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 315 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 318 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 319 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 320 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 321 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 322 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 323 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 324 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 325 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 326 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 327 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 328 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 329 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 330 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 332 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 333 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 334 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 335 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 336 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 337 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 338 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 339 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 340 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 341 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 342 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ };
+
apss_shared: mailbox@17990000 {
compatible = "qcom,sdm845-apss-shared";
reg = <0x17990000 0x1000>;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


2018-07-19 17:55:54

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH 2/3] dts: arm64/sdm845: Add node for qcom,smmu-v2

Add device node for qcom,smmu-v2 available on sdm845.
This smmu is available only to GPU device.

Signed-off-by: Vivek Gautam <[email protected]>
---

One power domain required for this GPU smmu - GPU_CX_GDSC,
commented out in the node is coming from gpu clock controller [1].

[1] https://lore.kernel.org/patchwork/cover/962208/

arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 4 ++++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 23 +++++++++++++++++++++++
2 files changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
index 13b50dff440f..969aa76bdceb 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -62,3 +62,7 @@
&apps_smmu {
status = "okay";
};
+
+&gpu_smmu {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 70ca18ae6cb3..db9f9e84d162 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -980,6 +980,29 @@
cell-index = <0>;
};

+ gpu_smmu: arm,smmu@5040000 {
+ compatible = "qcom,sdm845-smmu-v2", "qcom,smmu-v2";
+ reg = <0x5040000 0x10000>;
+ #iommu-cells = <1>;
+ #global-interrupts = <2>;
+ interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 231 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 364 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 365 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 366 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 367 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 368 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 369 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 370 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 371 IRQ_TYPE_EDGE_RISING>;
+ clock-names = "bus", "iface";
+ clocks = <&gcc GCC_GPU_MEMNOC_GFX_CLK>,
+ <&gcc GCC_GPU_CFG_AHB_CLK>;
+
+ /* power-domains = <&gpucc GPU_CX_GDSC>; */
+ status = "disabled";
+ };
+
apps_smmu: arm,smmu@15000000 {
compatible = "arm,mmu-500";
reg = <0x15000000 0x80000>;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


2018-07-19 17:56:50

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH 3/3] iommu/arm-smmu: Error out only if not enough context interrupts

Currently we check if the number of context banks is not equal to
num_context_interrupts. However, there are booloaders such as, one
on sdm845 that reserves few context banks and thus kernel views
less than the total available context banks.
So, although the hardware definition in device tree would mention
the correct number of context interrupts, this number can be
greater than the number of context banks visible to smmu in kernel.
We should therefore error out only when the number of context banks
is greater than the available number of context interrupts.

Signed-off-by: Vivek Gautam <[email protected]>
Suggested-by: Tomasz Figa <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Will Deacon <[email protected]>
---
drivers/iommu/arm-smmu.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 7c69736a30f8..4cb53bf4f423 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2229,12 +2229,19 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
if (err)
return err;

- if (smmu->version == ARM_SMMU_V2 &&
- smmu->num_context_banks != smmu->num_context_irqs) {
- dev_err(dev,
- "found only %d context interrupt(s) but %d required\n",
- smmu->num_context_irqs, smmu->num_context_banks);
- return -ENODEV;
+ if (smmu->version == ARM_SMMU_V2) {
+ if (smmu->num_context_banks > smmu->num_context_irqs) {
+ dev_err(dev,
+ "found only %d context irq(s) but %d required\n",
+ smmu->num_context_irqs, smmu->num_context_banks);
+ return -ENODEV;
+ } else if (smmu->num_context_banks < smmu->num_context_irqs) {
+ /* loose extra context interrupts */
+ dev_notice(dev,
+ "found %d context irq(s) but only %d required\n",
+ smmu->num_context_irqs, smmu->num_context_banks);
+ smmu->num_context_irqs = smmu->num_context_banks;
+ }
}

for (i = 0; i < smmu->num_global_irqs; ++i) {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


2018-07-19 23:09:32

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 1/3] dts: arm64/sdm845: Add node for arm,mmu-500

On Thu, Jul 19, 2018 at 11:54 AM Vivek Gautam
<[email protected]> wrote:
>
> Add device node for arm,mmu-500 available on sdm845.
> This MMU-500 with single TCU and multiple TBU architecture
> is shared among all the peripherals except gpu on sdm845.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 4 ++
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 73 +++++++++++++++++++++++++++++++++
> 2 files changed, 77 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> index 6d651f314193..13b50dff440f 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -58,3 +58,7 @@
> bias-pull-up;
> };
> };
> +
> +&apps_smmu {
> + status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 00722b533a92..70ca18ae6cb3 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -980,6 +980,79 @@
> cell-index = <0>;
> };
>
> + apps_smmu: arm,smmu@15000000 {

iommu@...

> + compatible = "arm,mmu-500";

Really unmodified by QCom? Would be better to have SoC specific compatible.

Rob

2018-07-20 08:27:10

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH 1/3] dts: arm64/sdm845: Add node for arm,mmu-500

Hi Rob,

On Fri, Jul 20, 2018 at 4:38 AM, Rob Herring <[email protected]> wrote:
> On Thu, Jul 19, 2018 at 11:54 AM Vivek Gautam
> <[email protected]> wrote:
>>
>> Add device node for arm,mmu-500 available on sdm845.
>> This MMU-500 with single TCU and multiple TBU architecture
>> is shared among all the peripherals except gpu on sdm845.
>>
>> Signed-off-by: Vivek Gautam <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 4 ++
>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 73 +++++++++++++++++++++++++++++++++
>> 2 files changed, 77 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> index 6d651f314193..13b50dff440f 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> @@ -58,3 +58,7 @@
>> bias-pull-up;
>> };
>> };
>> +
>> +&apps_smmu {
>> + status = "okay";
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> index 00722b533a92..70ca18ae6cb3 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> @@ -980,6 +980,79 @@
>> cell-index = <0>;
>> };
>>
>> + apps_smmu: arm,smmu@15000000 {
>
> iommu@...

Thanks for the review.
Sure, will modify this.

>
>> + compatible = "arm,mmu-500";
>
> Really unmodified by QCom? Would be better to have SoC specific compatible.

Yes, at this point we are able to use the driver as is on 845.
But, will add a SoC specific compatible as suggested to make bindings
future proof.

Best regards
Vivek

>
> Rob
> _______________________________________________
> iommu mailing list
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/iommu



--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2018-07-24 08:37:51

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH 3/3] iommu/arm-smmu: Error out only if not enough context interrupts

On Thu, Jul 19, 2018 at 11:23:56PM +0530, Vivek Gautam wrote:
> Currently we check if the number of context banks is not equal to
> num_context_interrupts. However, there are booloaders such as, one
> on sdm845 that reserves few context banks and thus kernel views
> less than the total available context banks.
> So, although the hardware definition in device tree would mention
> the correct number of context interrupts, this number can be
> greater than the number of context banks visible to smmu in kernel.
> We should therefore error out only when the number of context banks
> is greater than the available number of context interrupts.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> Suggested-by: Tomasz Figa <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Will Deacon <[email protected]>
> ---
> drivers/iommu/arm-smmu.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 7c69736a30f8..4cb53bf4f423 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -2229,12 +2229,19 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> if (err)
> return err;
>
> - if (smmu->version == ARM_SMMU_V2 &&
> - smmu->num_context_banks != smmu->num_context_irqs) {
> - dev_err(dev,
> - "found only %d context interrupt(s) but %d required\n",
> - smmu->num_context_irqs, smmu->num_context_banks);
> - return -ENODEV;
> + if (smmu->version == ARM_SMMU_V2) {
> + if (smmu->num_context_banks > smmu->num_context_irqs) {
> + dev_err(dev,
> + "found only %d context irq(s) but %d required\n",
> + smmu->num_context_irqs, smmu->num_context_banks);
> + return -ENODEV;
> + } else if (smmu->num_context_banks < smmu->num_context_irqs) {
> + /* loose extra context interrupts */
> + dev_notice(dev,
> + "found %d context irq(s) but only %d required\n",
> + smmu->num_context_irqs, smmu->num_context_banks);
> + smmu->num_context_irqs = smmu->num_context_banks;
> + }

I don't see the utility in the new message. Can you simplify with the patch
below on top? It's a bit weird that we only decide to ignore the extra irqs
after calling platform_get_irq() on them, but that seems to be harmless.

Will

--->8

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index aa46c1ed5bf9..5349e22b5c78 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2109,13 +2109,10 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
"found only %d context irq(s) but %d required\n",
smmu->num_context_irqs, smmu->num_context_banks);
return -ENODEV;
- } else if (smmu->num_context_banks < smmu->num_context_irqs) {
- /* loose extra context interrupts */
- dev_notice(dev,
- "found %d context irq(s) but only %d required\n",
- smmu->num_context_irqs, smmu->num_context_banks);
- smmu->num_context_irqs = smmu->num_context_banks;
}
+
+ /* Ignore superfluous interrupts */
+ smmu->num_context_irqs = smmu->num_context_banks;
}

for (i = 0; i < smmu->num_global_irqs; ++i) {

2018-07-24 09:41:09

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH 3/3] iommu/arm-smmu: Error out only if not enough context interrupts

Hi Will,


On 7/24/2018 2:06 PM, Will Deacon wrote:
> On Thu, Jul 19, 2018 at 11:23:56PM +0530, Vivek Gautam wrote:
>> Currently we check if the number of context banks is not equal to
>> num_context_interrupts. However, there are booloaders such as, one
>> on sdm845 that reserves few context banks and thus kernel views
>> less than the total available context banks.
>> So, although the hardware definition in device tree would mention
>> the correct number of context interrupts, this number can be
>> greater than the number of context banks visible to smmu in kernel.
>> We should therefore error out only when the number of context banks
>> is greater than the available number of context interrupts.
>>
>> Signed-off-by: Vivek Gautam <[email protected]>
>> Suggested-by: Tomasz Figa <[email protected]>
>> Cc: Robin Murphy <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> ---
>> drivers/iommu/arm-smmu.c | 19 +++++++++++++------
>> 1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 7c69736a30f8..4cb53bf4f423 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -2229,12 +2229,19 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>> if (err)
>> return err;
>>
>> - if (smmu->version == ARM_SMMU_V2 &&
>> - smmu->num_context_banks != smmu->num_context_irqs) {
>> - dev_err(dev,
>> - "found only %d context interrupt(s) but %d required\n",
>> - smmu->num_context_irqs, smmu->num_context_banks);
>> - return -ENODEV;
>> + if (smmu->version == ARM_SMMU_V2) {
>> + if (smmu->num_context_banks > smmu->num_context_irqs) {
>> + dev_err(dev,
>> + "found only %d context irq(s) but %d required\n",
>> + smmu->num_context_irqs, smmu->num_context_banks);
>> + return -ENODEV;
>> + } else if (smmu->num_context_banks < smmu->num_context_irqs) {
>> + /* loose extra context interrupts */
>> + dev_notice(dev,
>> + "found %d context irq(s) but only %d required\n",
>> + smmu->num_context_irqs, smmu->num_context_banks);
>> + smmu->num_context_irqs = smmu->num_context_banks;
>> + }
> I don't see the utility in the new message. Can you simplify with the patch
> below on top? It's a bit weird that we only decide to ignore the extra irqs
> after calling platform_get_irq() on them, but that seems to be harmless.

Thanks. I will modify as suggested below and respin.

Best regards
Vivek

>
> Will
>
> --->8
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index aa46c1ed5bf9..5349e22b5c78 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -2109,13 +2109,10 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> "found only %d context irq(s) but %d required\n",
> smmu->num_context_irqs, smmu->num_context_banks);
> return -ENODEV;
> - } else if (smmu->num_context_banks < smmu->num_context_irqs) {
> - /* loose extra context interrupts */
> - dev_notice(dev,
> - "found %d context irq(s) but only %d required\n",
> - smmu->num_context_irqs, smmu->num_context_banks);
> - smmu->num_context_irqs = smmu->num_context_banks;
> }
> +
> + /* Ignore superfluous interrupts */
> + smmu->num_context_irqs = smmu->num_context_banks;
> }
>
> for (i = 0; i < smmu->num_global_irqs; ++i) {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2018-07-25 11:59:45

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH 3/3] iommu/arm-smmu: Error out only if not enough context interrupts

On Tue, Jul 24, 2018 at 03:09:41PM +0530, Vivek Gautam wrote:
> On 7/24/2018 2:06 PM, Will Deacon wrote:
> >On Thu, Jul 19, 2018 at 11:23:56PM +0530, Vivek Gautam wrote:
> >>diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >>index 7c69736a30f8..4cb53bf4f423 100644
> >>--- a/drivers/iommu/arm-smmu.c
> >>+++ b/drivers/iommu/arm-smmu.c
> >>@@ -2229,12 +2229,19 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> >> if (err)
> >> return err;
> >>- if (smmu->version == ARM_SMMU_V2 &&
> >>- smmu->num_context_banks != smmu->num_context_irqs) {
> >>- dev_err(dev,
> >>- "found only %d context interrupt(s) but %d required\n",
> >>- smmu->num_context_irqs, smmu->num_context_banks);
> >>- return -ENODEV;
> >>+ if (smmu->version == ARM_SMMU_V2) {
> >>+ if (smmu->num_context_banks > smmu->num_context_irqs) {
> >>+ dev_err(dev,
> >>+ "found only %d context irq(s) but %d required\n",
> >>+ smmu->num_context_irqs, smmu->num_context_banks);
> >>+ return -ENODEV;
> >>+ } else if (smmu->num_context_banks < smmu->num_context_irqs) {
> >>+ /* loose extra context interrupts */
> >>+ dev_notice(dev,
> >>+ "found %d context irq(s) but only %d required\n",
> >>+ smmu->num_context_irqs, smmu->num_context_banks);
> >>+ smmu->num_context_irqs = smmu->num_context_banks;
> >>+ }
> >I don't see the utility in the new message. Can you simplify with the patch
> >below on top? It's a bit weird that we only decide to ignore the extra irqs
> >after calling platform_get_irq() on them, but that seems to be harmless.
>
> Thanks. I will modify as suggested below and respin.

It's ok, I can make the change locally.

Will

2018-07-27 09:59:15

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH 3/3] iommu/arm-smmu: Error out only if not enough context interrupts

On Wed, Jul 25, 2018 at 5:27 PM, Will Deacon <[email protected]> wrote:
> On Tue, Jul 24, 2018 at 03:09:41PM +0530, Vivek Gautam wrote:
>> On 7/24/2018 2:06 PM, Will Deacon wrote:
>> >On Thu, Jul 19, 2018 at 11:23:56PM +0530, Vivek Gautam wrote:
>> >>diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> >>index 7c69736a30f8..4cb53bf4f423 100644
>> >>--- a/drivers/iommu/arm-smmu.c
>> >>+++ b/drivers/iommu/arm-smmu.c
>> >>@@ -2229,12 +2229,19 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>> >> if (err)
>> >> return err;
>> >>- if (smmu->version == ARM_SMMU_V2 &&
>> >>- smmu->num_context_banks != smmu->num_context_irqs) {
>> >>- dev_err(dev,
>> >>- "found only %d context interrupt(s) but %d required\n",
>> >>- smmu->num_context_irqs, smmu->num_context_banks);
>> >>- return -ENODEV;
>> >>+ if (smmu->version == ARM_SMMU_V2) {
>> >>+ if (smmu->num_context_banks > smmu->num_context_irqs) {
>> >>+ dev_err(dev,
>> >>+ "found only %d context irq(s) but %d required\n",
>> >>+ smmu->num_context_irqs, smmu->num_context_banks);
>> >>+ return -ENODEV;
>> >>+ } else if (smmu->num_context_banks < smmu->num_context_irqs) {
>> >>+ /* loose extra context interrupts */
>> >>+ dev_notice(dev,
>> >>+ "found %d context irq(s) but only %d required\n",
>> >>+ smmu->num_context_irqs, smmu->num_context_banks);
>> >>+ smmu->num_context_irqs = smmu->num_context_banks;
>> >>+ }
>> >I don't see the utility in the new message. Can you simplify with the patch
>> >below on top? It's a bit weird that we only decide to ignore the extra irqs
>> >after calling platform_get_irq() on them, but that seems to be harmless.
>>
>> Thanks. I will modify as suggested below and respin.
>
> It's ok, I can make the change locally.

Thanks Will for making the changes, and picking this.

Best regards
Vivek

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2018-08-10 22:19:21

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH 1/3] dts: arm64/sdm845: Add node for arm,mmu-500

Hi,

On Thu, Jul 19, 2018 at 10:53 AM, Vivek Gautam
<[email protected]> wrote:
> Add device node for arm,mmu-500 available on sdm845.
> This MMU-500 with single TCU and multiple TBU architecture
> is shared among all the peripherals except gpu on sdm845.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 4 ++
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 73 +++++++++++++++++++++++++++++++++
> 2 files changed, 77 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> index 6d651f314193..13b50dff440f 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -58,3 +58,7 @@
> bias-pull-up;
> };
> };
> +
> +&apps_smmu {
> + status = "okay";
> +};

When you spin this patch please put the above in the correct place.
Since "a" sorts alphabetically before "i" then this should be just
before the line:

&i2c10 {

Thanks!

-Doug

2018-08-10 22:31:40

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH 1/3] dts: arm64/sdm845: Add node for arm,mmu-500

Hi,

On Fri, Aug 10, 2018 at 3:18 PM, Doug Anderson <[email protected]> wrote:
> Hi,
>
> On Thu, Jul 19, 2018 at 10:53 AM, Vivek Gautam
> <[email protected]> wrote:
>> Add device node for arm,mmu-500 available on sdm845.
>> This MMU-500 with single TCU and multiple TBU architecture
>> is shared among all the peripherals except gpu on sdm845.
>>
>> Signed-off-by: Vivek Gautam <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 4 ++
>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 73 +++++++++++++++++++++++++++++++++
>> 2 files changed, 77 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> index 6d651f314193..13b50dff440f 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> @@ -58,3 +58,7 @@
>> bias-pull-up;
>> };
>> };
>> +
>> +&apps_smmu {
>> + status = "okay";
>> +};
>
> When you spin this patch please put the above in the correct place.
> Since "a" sorts alphabetically before "i" then this should be just
> before the line:
>
> &i2c10 {

Sorry--one more thing I thought of after I sent this out...

Possibly you can drop this part of the patch completely and get rid of
the 'status = "disabled";' in sdm845.dtsi. As I understand it you
really only want to mark things as disabled in the SoC dtsi file if
some boards might use this device and other boards wouldn't. For
instance not all boards will have the SD card controller hooked up /
enabled so having that set to "disabled" in the SoC device tree file
makes sense. ...but it's not a board-level question about whether the
SMMU is present--it's always there. You don't gain anything by
forcing all boards to set status to "okay".


-Doug

2018-08-13 05:34:19

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH 1/3] dts: arm64/sdm845: Add node for arm,mmu-500

Hi Doug,


On 8/11/2018 4:00 AM, Doug Anderson wrote:
> Hi,
>
> On Fri, Aug 10, 2018 at 3:18 PM, Doug Anderson <[email protected]> wrote:
>> Hi,
>>
>> On Thu, Jul 19, 2018 at 10:53 AM, Vivek Gautam
>> <[email protected]> wrote:
>>> Add device node for arm,mmu-500 available on sdm845.
>>> This MMU-500 with single TCU and multiple TBU architecture
>>> is shared among all the peripherals except gpu on sdm845.
>>>
>>> Signed-off-by: Vivek Gautam <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 4 ++
>>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 73 +++++++++++++++++++++++++++++++++
>>> 2 files changed, 77 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>>> index 6d651f314193..13b50dff440f 100644
>>> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>>> @@ -58,3 +58,7 @@
>>> bias-pull-up;
>>> };
>>> };
>>> +
>>> +&apps_smmu {
>>> + status = "okay";
>>> +};
>> When you spin this patch please put the above in the correct place.
>> Since "a" sorts alphabetically before "i" then this should be just
>> before the line:
>>
>> &i2c10 {
> Sorry--one more thing I thought of after I sent this out...
>
> Possibly you can drop this part of the patch completely and get rid of
> the 'status = "disabled";' in sdm845.dtsi. As I understand it you
> really only want to mark things as disabled in the SoC dtsi file if
> some boards might use this device and other boards wouldn't. For
> instance not all boards will have the SD card controller hooked up /
> enabled so having that set to "disabled" in the SoC device tree file
> makes sense. ...but it's not a board-level question about whether the
> SMMU is present--it's always there. You don't gain anything by
> forcing all boards to set status to "okay".

Thanks for reviewing the patches.
Will sort the node as per alphabetical order.
Also as you pointed, it makes sense to not have the 'status' property
in SMMU. Will remove that. Thanks.

Best regards
Vivek
>
>
> -Doug