2022-10-15 14:08:00

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 0/5] dma/arm64: qcom: use one compatible also for 0x10000 offset

Hi,

Dependencies
============
1. DT bindings and DMA driver change depends contextually on:
https://lore.kernel.org/all/[email protected]/

2. DTS patches are independent, although they will spark some dtbs_check
warnings (due to change in bindings).

Best regards,
Krzysztof

Krzysztof Kozlowski (5):
dt-bindings: dma: qcom: gpi: use sm6350 fallback
dmaengine: qcom: gpi: document preferred SM6350 binding
arm64: dts: qcom: sc7280: Add GPI DMA compatible fallback
arm64: dts: qcom: sm8350: Add GPI DMA compatible fallback
arm64: dts: qcom: sm8450: Add GPI DMA compatible fallback

Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 10 ++++++----
arch/arm64/boot/dts/qcom/sc7280.dtsi | 4 ++--
arch/arm64/boot/dts/qcom/sm8350.dtsi | 6 +++---
arch/arm64/boot/dts/qcom/sm8450.dtsi | 6 +++---
drivers/dma/qcom/gpi.c | 7 ++++---
5 files changed, 18 insertions(+), 15 deletions(-)

--
2.34.1


2022-10-15 14:08:48

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 1/5] dt-bindings: dma: qcom: gpi: use sm6350 fallback

Several devices like SM6350, SM8150 and SC7280 are actually compatible,
so use one compatible fallback for all of them.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
index 750b40c32213..0c2894498845 100644
--- a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
+++ b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
@@ -20,12 +20,14 @@ properties:
compatible:
oneOf:
- enum:
- - qcom,sc7280-gpi-dma
- qcom,sdm845-gpi-dma
- qcom,sm6350-gpi-dma
- - qcom,sm8350-gpi-dma
- - qcom,sm8450-gpi-dma
-
+ - items:
+ - enum:
+ - qcom,sc7280-gpi-dma
+ - qcom,sm8350-gpi-dma
+ - qcom,sm8450-gpi-dma
+ - const: qcom,sm6350-gpi-dma
- items:
- enum:
- qcom,sdm670-gpi-dma
--
2.34.1

2022-10-15 14:10:06

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 3/5] arm64: dts: qcom: sc7280: Add GPI DMA compatible fallback

Use SM6350 as fallback for GPI DMA, to indicate devices are compatible
and that drivers can bind with only one compatible.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 212580316d3e..2a167412fa6a 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -920,7 +920,7 @@ opp-384000000 {

gpi_dma0: dma-controller@900000 {
#dma-cells = <3>;
- compatible = "qcom,sc7280-gpi-dma";
+ compatible = "qcom,sc7280-gpi-dma", "qcom,sm6350-gpi-dma";
reg = <0 0x00900000 0 0x60000>;
interrupts = <GIC_SPI 244 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>,
@@ -1419,7 +1419,7 @@ uart7: serial@99c000 {

gpi_dma1: dma-controller@a00000 {
#dma-cells = <3>;
- compatible = "qcom,sc7280-gpi-dma";
+ compatible = "qcom,sc7280-gpi-dma", "qcom,sm6350-gpi-dma";
reg = <0 0x00a00000 0 0x60000>;
interrupts = <GIC_SPI 279 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 280 IRQ_TYPE_LEVEL_HIGH>,
--
2.34.1

2022-10-15 14:10:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 2/5] dmaengine: qcom: gpi: document preferred SM6350 binding

Devices with ee offset of 0x10000 should rather bind with SM6350
compatible, so the list will not unnecessarily grow for compatible
devices.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/dma/qcom/gpi.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
index f8e19e6e6117..061add832295 100644
--- a/drivers/dma/qcom/gpi.c
+++ b/drivers/dma/qcom/gpi.c
@@ -2286,13 +2286,14 @@ static int gpi_probe(struct platform_device *pdev)
}

static const struct of_device_id gpi_of_match[] = {
- { .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
{ .compatible = "qcom,sdm845-gpi-dma", .data = (void *)0x0 },
{ .compatible = "qcom,sm6350-gpi-dma", .data = (void *)0x10000 },
/*
- * Deprecated, devices with ee_offset = 0 should use sdm845-gpi-dma as
- * fallback and not need their own entries here.
+ * Do not grow the list for compatible devices. Instead use
+ * qcom,sdm845-gpi-dma (for ee_offset = 0x0) or qcom,sm6350-gpi-dma
+ * (for ee_offset = 0x10000).
*/
+ { .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
{ .compatible = "qcom,sm8150-gpi-dma", .data = (void *)0x0 },
{ .compatible = "qcom,sm8250-gpi-dma", .data = (void *)0x0 },
{ .compatible = "qcom,sm8350-gpi-dma", .data = (void *)0x10000 },
--
2.34.1

2022-10-15 14:13:26

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 4/5] arm64: dts: qcom: sm8350: Add GPI DMA compatible fallback

Use SM6350 as fallback for GPI DMA, to indicate devices are compatible
and that drivers can bind with only one compatible.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8350.dtsi | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index a86d9ea93b9d..aa08c0e065c7 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -678,7 +678,7 @@ ipcc: mailbox@408000 {
};

gpi_dma2: dma-controller@800000 {
- compatible = "qcom,sm8350-gpi-dma";
+ compatible = "qcom,sm8350-gpi-dma", "qcom,sm6350-gpi-dma";
reg = <0 0x00800000 0 0x60000>;
interrupts = <GIC_SPI 588 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 589 IRQ_TYPE_LEVEL_HIGH>,
@@ -904,7 +904,7 @@ spi19: spi@894000 {
};

gpi_dma0: dma-controller@900000 {
- compatible = "qcom,sm8350-gpi-dma";
+ compatible = "qcom,sm8350-gpi-dma", "qcom,sm6350-gpi-dma";
reg = <0 0x09800000 0 0x60000>;
interrupts = <GIC_SPI 244 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>,
@@ -1209,7 +1209,7 @@ spi7: spi@99c000 {
};

gpi_dma1: dma-controller@a00000 {
- compatible = "qcom,sm8350-gpi-dma";
+ compatible = "qcom,sm8350-gpi-dma", "qcom,sm6350-gpi-dma";
reg = <0 0x00a00000 0 0x60000>;
interrupts = <GIC_SPI 279 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 280 IRQ_TYPE_LEVEL_HIGH>,
--
2.34.1

2022-10-15 14:15:20

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 5/5] arm64: dts: qcom: sm8450: Add GPI DMA compatible fallback

Use SM6350 as fallback for GPI DMA, to indicate devices are compatible
and that drivers can bind with only one compatible.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8450.dtsi | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index d32f08df743d..e01a019d8b23 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -730,7 +730,7 @@ gcc: clock-controller@100000 {
};

gpi_dma2: dma-controller@800000 {
- compatible = "qcom,sm8450-gpi-dma";
+ compatible = "qcom,sm8450-gpi-dma", "qcom,sm6350-gpi-dma";
#dma-cells = <3>;
reg = <0 0x800000 0 0x60000>;
interrupts = <GIC_SPI 588 IRQ_TYPE_LEVEL_HIGH>,
@@ -1058,7 +1058,7 @@ spi21: spi@898000 {
};

gpi_dma0: dma-controller@900000 {
- compatible = "qcom,sm8450-gpi-dma";
+ compatible = "qcom,sm8450-gpi-dma", "qcom,sm6350-gpi-dma";
#dma-cells = <3>;
reg = <0 0x900000 0 0x60000>;
interrupts = <GIC_SPI 244 IRQ_TYPE_LEVEL_HIGH>,
@@ -1394,7 +1394,7 @@ uart7: serial@99c000 {
};

gpi_dma1: dma-controller@a00000 {
- compatible = "qcom,sm8450-gpi-dma";
+ compatible = "qcom,sm8450-gpi-dma", "qcom,sm6350-gpi-dma";
#dma-cells = <3>;
reg = <0 0xa00000 0 0x60000>;
interrupts = <GIC_SPI 279 IRQ_TYPE_LEVEL_HIGH>,
--
2.34.1

2022-10-17 10:42:59

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 1/5] dt-bindings: dma: qcom: gpi: use sm6350 fallback

On 15-10-22, 10:04, Krzysztof Kozlowski wrote:
> Several devices like SM6350, SM8150 and SC7280 are actually compatible,
> so use one compatible fallback for all of them.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
> index 750b40c32213..0c2894498845 100644
> --- a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
> +++ b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
> @@ -20,12 +20,14 @@ properties:
> compatible:
> oneOf:
> - enum:
> - - qcom,sc7280-gpi-dma
> - qcom,sdm845-gpi-dma
> - qcom,sm6350-gpi-dma
> - - qcom,sm8350-gpi-dma
> - - qcom,sm8450-gpi-dma
> -
> + - items:
> + - enum:
> + - qcom,sc7280-gpi-dma
> + - qcom,sm8350-gpi-dma
> + - qcom,sm8450-gpi-dma
> + - const: qcom,sm6350-gpi-dma

I think it makes sense but can we document this in binding as well that
why people should use these two compatibles. I am fine with this being
comments here..

> - items:
> - enum:
> - qcom,sdm670-gpi-dma
> --
> 2.34.1

--
~Vinod

2022-10-17 18:59:42

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/5] dt-bindings: dma: qcom: gpi: use sm6350 fallback

On Sat, 15 Oct 2022 10:04:43 -0400, Krzysztof Kozlowski wrote:
> Several devices like SM6350, SM8150 and SC7280 are actually compatible,
> so use one compatible fallback for all of them.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>

Acked-by: Rob Herring <[email protected]>

2022-10-17 21:32:46

by Richard Acayan

[permalink] [raw]
Subject: Re: [PATCH 1/5] dt-bindings: dma: qcom: gpi: use sm6350 fallback

> Several devices like SM6350, SM8150 and SC7280 are actually compatible,
> so use one compatible fallback for all of them.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
> index 750b40c32213..0c2894498845 100644
> --- a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
> +++ b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
> @@ -20,12 +20,14 @@ properties:
> compatible:
> oneOf:
> - enum:
> - - qcom,sc7280-gpi-dma
> - qcom,sdm845-gpi-dma
> - qcom,sm6350-gpi-dma
> - - qcom,sm8350-gpi-dma
> - - qcom,sm8450-gpi-dma
> -

If you don't want this empty line here, you can still ask for it to be removed
in the original patch. It doesn't look like it was applied yet.

> + - items:
> + - enum:
> + - qcom,sc7280-gpi-dma
> + - qcom,sm8350-gpi-dma
> + - qcom,sm8450-gpi-dma
> + - const: qcom,sm6350-gpi-dma
> - items:
> - enum:
> - qcom,sdm670-gpi-dma
> --
> 2.34.1
>

2022-10-17 21:40:11

by Richard Acayan

[permalink] [raw]
Subject: Re: [PATCH 2/5] dmaengine: qcom: gpi: document preferred SM6350 binding

> Devices with ee offset of 0x10000 should rather bind with SM6350
> compatible, so the list will not unnecessarily grow for compatible
> devices.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> drivers/dma/qcom/gpi.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
> index f8e19e6e6117..061add832295 100644
> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c
> @@ -2286,13 +2286,14 @@ static int gpi_probe(struct platform_device *pdev)
> }
>
> static const struct of_device_id gpi_of_match[] = {
> - { .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
> { .compatible = "qcom,sdm845-gpi-dma", .data = (void *)0x0 },
> { .compatible = "qcom,sm6350-gpi-dma", .data = (void *)0x10000 },
> /*
> - * Deprecated, devices with ee_offset = 0 should use sdm845-gpi-dma as
> - * fallback and not need their own entries here.

This comment is from the dependency series [1]. Why would we need to add it just
to remove it here? I was not notified that the dependency was applied anywhere
(except as a base for other series) so it's not set in stone. Let's just drop
the original patch that this comment originates from to prevent needlessly
adding and removing the same lines at once.

[1] https://lore.kernel.org/linux-arm-msm/[email protected]/

> + * Do not grow the list for compatible devices. Instead use
> + * qcom,sdm845-gpi-dma (for ee_offset = 0x0) or qcom,sm6350-gpi-dma
> + * (for ee_offset = 0x10000).
> */
> + { .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
> { .compatible = "qcom,sm8150-gpi-dma", .data = (void *)0x0 },
> { .compatible = "qcom,sm8250-gpi-dma", .data = (void *)0x0 },
> { .compatible = "qcom,sm8350-gpi-dma", .data = (void *)0x10000 },
> --
> 2.34.1
>

2022-10-17 21:46:00

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/5] dt-bindings: dma: qcom: gpi: use sm6350 fallback

On 17/10/2022 06:37, Vinod Koul wrote:
> On 15-10-22, 10:04, Krzysztof Kozlowski wrote:
>> Several devices like SM6350, SM8150 and SC7280 are actually compatible,
>> so use one compatible fallback for all of them.
>>
>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>> ---
>> Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> index 750b40c32213..0c2894498845 100644
>> --- a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> +++ b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> @@ -20,12 +20,14 @@ properties:
>> compatible:
>> oneOf:
>> - enum:
>> - - qcom,sc7280-gpi-dma
>> - qcom,sdm845-gpi-dma
>> - qcom,sm6350-gpi-dma
>> - - qcom,sm8350-gpi-dma
>> - - qcom,sm8450-gpi-dma
>> -
>> + - items:
>> + - enum:
>> + - qcom,sc7280-gpi-dma
>> + - qcom,sm8350-gpi-dma
>> + - qcom,sm8450-gpi-dma
>> + - const: qcom,sm6350-gpi-dma
>
> I think it makes sense but can we document this in binding as well that
> why people should use these two compatibles. I am fine with this being
> comments here..

It is kind of implied (and maybe obvious) from the bindings - a list of
two items, one enum and one fallback compatible.

We usually do not document such patterns in the bindings with comments
for that reason. If you insist, I can add it.

Best regards,
Krzysztof

2022-10-17 21:46:56

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/5] dt-bindings: dma: qcom: gpi: use sm6350 fallback

On 17/10/2022 17:10, Richard Acayan wrote:
>> Several devices like SM6350, SM8150 and SC7280 are actually compatible,
>> so use one compatible fallback for all of them.
>>
>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>> ---
>> Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> index 750b40c32213..0c2894498845 100644
>> --- a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> +++ b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> @@ -20,12 +20,14 @@ properties:
>> compatible:
>> oneOf:
>> - enum:
>> - - qcom,sc7280-gpi-dma
>> - qcom,sdm845-gpi-dma
>> - qcom,sm6350-gpi-dma
>> - - qcom,sm8350-gpi-dma
>> - - qcom,sm8450-gpi-dma
>> -
>
> If you don't want this empty line here, you can still ask for it to be removed
> in the original patch. It doesn't look like it was applied yet.
>

Sure.

Best regards,
Krzysztof

2022-10-17 22:01:01

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/5] dmaengine: qcom: gpi: document preferred SM6350 binding

On 17/10/2022 17:23, Richard Acayan wrote:
>> Devices with ee offset of 0x10000 should rather bind with SM6350
>> compatible, so the list will not unnecessarily grow for compatible
>> devices.
>>
>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>> ---
>> drivers/dma/qcom/gpi.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
>> index f8e19e6e6117..061add832295 100644
>> --- a/drivers/dma/qcom/gpi.c
>> +++ b/drivers/dma/qcom/gpi.c
>> @@ -2286,13 +2286,14 @@ static int gpi_probe(struct platform_device *pdev)
>> }
>>
>> static const struct of_device_id gpi_of_match[] = {
>> - { .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
>> { .compatible = "qcom,sdm845-gpi-dma", .data = (void *)0x0 },
>> { .compatible = "qcom,sm6350-gpi-dma", .data = (void *)0x10000 },
>> /*
>> - * Deprecated, devices with ee_offset = 0 should use sdm845-gpi-dma as
>> - * fallback and not need their own entries here.
>
> This comment is from the dependency series [1]. Why would we need to add it just
> to remove it here? I was not notified that the dependency was applied anywhere
> (except as a base for other series) so it's not set in stone. Let's just drop
> the original patch that this comment originates from to prevent needlessly
> adding and removing the same lines at once.

I don't remove the comment, I re-phrase it to be better suited for final
code.

Best regards,
Krzysztof

2022-10-17 22:27:02

by Richard Acayan

[permalink] [raw]
Subject: Re: [PATCH 2/5] dmaengine: qcom: gpi: document preferred SM6350 binding

> On 17/10/2022 17:23, Richard Acayan wrote:
>>> Devices with ee offset of 0x10000 should rather bind with SM6350
>>> compatible, so the list will not unnecessarily grow for compatible
>>> devices.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>> ---
>>> drivers/dma/qcom/gpi.c | 7 ++++---
>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
>>> index f8e19e6e6117..061add832295 100644
>>> --- a/drivers/dma/qcom/gpi.c
>>> +++ b/drivers/dma/qcom/gpi.c
>>> @@ -2286,13 +2286,14 @@ static int gpi_probe(struct platform_device *pdev)
>>> }
>>>
>>> static const struct of_device_id gpi_of_match[] = {
>>> - { .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
>>> { .compatible = "qcom,sdm845-gpi-dma", .data = (void *)0x0 },
>>> { .compatible = "qcom,sm6350-gpi-dma", .data = (void *)0x10000 },
>>> /*
>>> - * Deprecated, devices with ee_offset = 0 should use sdm845-gpi-dma as
>>> - * fallback and not need their own entries here.
>>
>> This comment is from the dependency series [1]. Why would we need to add it just
>> to remove it here? I was not notified that the dependency was applied anywhere
>> (except as a base for other series) so it's not set in stone. Let's just drop
>> the original patch that this comment originates from to prevent needlessly
>> adding and removing the same lines at once.
>
> I don't remove the comment, I re-phrase it to be better suited for final
> code.

Yes, I didn't word that exactly right. I still think the patch that adds this is
now useless if it's just going to be replaced. Do you think I should keep the
patch that this comment originates from, even though we already know exactly how
its substantial contents will be replaced?

We can't modify history and drop commits on kernel trees, but I can still send
a v6 series that drops the original comment.

>
> Best regards,
> Krzysztof
>

2022-10-17 23:01:09

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/5] dmaengine: qcom: gpi: document preferred SM6350 binding

On 17/10/2022 18:00, Richard Acayan wrote:
>> On 17/10/2022 17:23, Richard Acayan wrote:
>>>> Devices with ee offset of 0x10000 should rather bind with SM6350
>>>> compatible, so the list will not unnecessarily grow for compatible
>>>> devices.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>>> ---
>>>> drivers/dma/qcom/gpi.c | 7 ++++---
>>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
>>>> index f8e19e6e6117..061add832295 100644
>>>> --- a/drivers/dma/qcom/gpi.c
>>>> +++ b/drivers/dma/qcom/gpi.c
>>>> @@ -2286,13 +2286,14 @@ static int gpi_probe(struct platform_device *pdev)
>>>> }
>>>>
>>>> static const struct of_device_id gpi_of_match[] = {
>>>> - { .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
>>>> { .compatible = "qcom,sdm845-gpi-dma", .data = (void *)0x0 },
>>>> { .compatible = "qcom,sm6350-gpi-dma", .data = (void *)0x10000 },
>>>> /*
>>>> - * Deprecated, devices with ee_offset = 0 should use sdm845-gpi-dma as
>>>> - * fallback and not need their own entries here.
>>>
>>> This comment is from the dependency series [1]. Why would we need to add it just
>>> to remove it here? I was not notified that the dependency was applied anywhere
>>> (except as a base for other series) so it's not set in stone. Let's just drop
>>> the original patch that this comment originates from to prevent needlessly
>>> adding and removing the same lines at once.
>>
>> I don't remove the comment, I re-phrase it to be better suited for final
>> code.
>
> Yes, I didn't word that exactly right. I still think the patch that adds this is
> now useless if it's just going to be replaced. Do you think I should keep the
> patch that this comment originates from, even though we already know exactly how
> its substantial contents will be replaced?
>
> We can't modify history and drop commits on kernel trees, but I can still send
> a v6 series that drops the original comment.

Sure. You can make it then:

* Do not grow the list for compatible devices. Instead use
* qcom,sdm845-gpi-dma (for ee_offset = 0x0).

And my patch will just change one line.

We can also keep it like:

* Do not grow the list for compatible devices. Instead use
* proper fallback compatibles.

Best regards,
Krzysztof

2022-10-18 00:58:30

by Richard Acayan

[permalink] [raw]
Subject: Re: [PATCH 2/5] dmaengine: qcom: gpi: document preferred SM6350 binding

> On 17/10/2022 18:00, Richard Acayan wrote:
>>> On 17/10/2022 17:23, Richard Acayan wrote:
>>>>> Devices with ee offset of 0x10000 should rather bind with SM6350
>>>>> compatible, so the list will not unnecessarily grow for compatible
>>>>> devices.
>>>>>
>>>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>>>> ---
>>>>> drivers/dma/qcom/gpi.c | 7 ++++---
>>>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
>>>>> index f8e19e6e6117..061add832295 100644
>>>>> --- a/drivers/dma/qcom/gpi.c
>>>>> +++ b/drivers/dma/qcom/gpi.c
>>>>> @@ -2286,13 +2286,14 @@ static int gpi_probe(struct platform_device *pdev)
>>>>> }
>>>>>
>>>>> static const struct of_device_id gpi_of_match[] = {
>>>>> - { .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
>>>>> { .compatible = "qcom,sdm845-gpi-dma", .data = (void *)0x0 },
>>>>> { .compatible = "qcom,sm6350-gpi-dma", .data = (void *)0x10000 },
>>>>> /*
>>>>> - * Deprecated, devices with ee_offset = 0 should use sdm845-gpi-dma as
>>>>> - * fallback and not need their own entries here.
>>>>
>>>> This comment is from the dependency series [1]. Why would we need to add it just
>>>> to remove it here? I was not notified that the dependency was applied anywhere
>>>> (except as a base for other series) so it's not set in stone. Let's just drop
>>>> the original patch that this comment originates from to prevent needlessly
>>>> adding and removing the same lines at once.
>>>
>>> I don't remove the comment, I re-phrase it to be better suited for final
>>> code.
>>
>> Yes, I didn't word that exactly right. I still think the patch that adds this is
>> now useless if it's just going to be replaced. Do you think I should keep the
>> patch that this comment originates from, even though we already know exactly how
>> its substantial contents will be replaced?
>>
>> We can't modify history and drop commits on kernel trees, but I can still send
>> a v6 series that drops the original comment.
>
> Sure. You can make it then:
>
> * Do not grow the list for compatible devices. Instead use
> * qcom,sdm845-gpi-dma (for ee_offset = 0x0).

If you don't want me to drop the original patch completely, then there is no
need to make any changes at all to the driver patches IMHO. I originally thought
we only needed one patch for the driver (yours) but you seem to have a really
good reason not to drop the original patch. Sorry for asking.

I guess you can add this if you want:

Acked-by: Richard Acayan <[email protected]>

>
> And my patch will just change one line.
>
> We can also keep it like:
>
> * Do not grow the list for compatible devices. Instead use
> * proper fallback compatibles.
>
> Best regards,
> Krzysztof
>

2022-10-18 13:49:21

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH 4/5] arm64: dts: qcom: sm8350: Add GPI DMA compatible fallback

On 15/10/2022 16:04, Krzysztof Kozlowski wrote:
> Use SM6350 as fallback for GPI DMA, to indicate devices are compatible
> and that drivers can bind with only one compatible.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sm8350.dtsi | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>

Reviewed-by: Neil Armstrong <[email protected]>

2022-10-18 14:39:24

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm64: dts: qcom: sm8450: Add GPI DMA compatible fallback

On 15/10/2022 16:04, Krzysztof Kozlowski wrote:
> Use SM6350 as fallback for GPI DMA, to indicate devices are compatible
> and that drivers can bind with only one compatible.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sm8450.dtsi | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>

Reviewed-by: Neil Armstrong <[email protected]>

2022-10-18 14:53:57

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH 3/5] arm64: dts: qcom: sc7280: Add GPI DMA compatible fallback

On 15/10/2022 16:04, Krzysztof Kozlowski wrote:
> Use SM6350 as fallback for GPI DMA, to indicate devices are compatible
> and that drivers can bind with only one compatible.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>

Reviewed-by: Neil Armstrong <[email protected]>