2022-04-06 13:46:34

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v2 05/14] dt-bindings: arm: mediatek: document the pcie mirror node on MT7622

From: Lorenzo Bianconi <[email protected]>

This patch adds the pcie mirror document bindings for MT7622 SoC.
The feature is used for intercepting PCIe MMIO access for the WED core
Add related info in mediatek-net bindings.

Signed-off-by: Lorenzo Bianconi <[email protected]>
Signed-off-by: Felix Fietkau <[email protected]>
---
.../mediatek/mediatek,mt7622-pcie-mirror.yaml | 42 +++++++++++++++++++
.../devicetree/bindings/net/mediatek-net.txt | 2 +
2 files changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml
new file mode 100644
index 000000000000..9fbeb626ab23
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: MediaTek PCIE Mirror Controller for MT7622
+
+maintainers:
+ - Lorenzo Bianconi <[email protected]>
+ - Felix Fietkau <[email protected]>
+
+description:
+ The mediatek PCIE mirror provides a configuration interface for PCIE
+ controller on MT7622 soc.
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - mediatek,mt7622-pcie-mirror
+ - const: syscon
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ pcie_mirror: pcie-mirror@10000400 {
+ compatible = "mediatek,mt7622-pcie-mirror", "syscon";
+ reg = <0 0x10000400 0 0x10>;
+ };
+ };
diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt b/Documentation/devicetree/bindings/net/mediatek-net.txt
index 1c8dc44bbb52..f18d70189375 100644
--- a/Documentation/devicetree/bindings/net/mediatek-net.txt
+++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
@@ -48,6 +48,8 @@ Optional properties:
various clocks and reset to the system.
- mediatek,wed: a list of phandles to wireless ethernet dispatch nodes for
MT7622 SoC.
+- mediatek,pcie-mirror: phandle to the mediatek pcie-mirror controller for
+ MT7622 SoC.

* Ethernet MAC node

--
2.35.1


2022-04-06 14:17:20

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 05/14] dt-bindings: arm: mediatek: document the pcie mirror node on MT7622

On 05/04/2022 21:57, Felix Fietkau wrote:
> From: Lorenzo Bianconi <[email protected]>
>
> This patch adds the pcie mirror document bindings for MT7622 SoC.
> The feature is used for intercepting PCIe MMIO access for the WED core
> Add related info in mediatek-net bindings.
>
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> Signed-off-by: Felix Fietkau <[email protected]>
> ---
> .../mediatek/mediatek,mt7622-pcie-mirror.yaml | 42 +++++++++++++++++++

Eh, I wanted to ask to not put it inside arm/, but judging by your usage
- you did not create drivers for both of these (WED and PCIe mirror).

You only need them to expose address spaces via syscon.

This actually looks hacky. Either WED and PCIe mirror are part of
network driver, then add the address spaces via "reg". If they are not,
but instead they are separate blocks, why you don't have drivers for them?


Best regards,
Krzysztof

2022-04-06 16:13:01

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v2 05/14] dt-bindings: arm: mediatek: document the pcie mirror node on MT7622


On 06.04.22 10:20, Krzysztof Kozlowski wrote:
> On 05/04/2022 21:57, Felix Fietkau wrote:
>> From: Lorenzo Bianconi <[email protected]>
>>
>> This patch adds the pcie mirror document bindings for MT7622 SoC.
>> The feature is used for intercepting PCIe MMIO access for the WED core
>> Add related info in mediatek-net bindings.
>>
>> Signed-off-by: Lorenzo Bianconi <[email protected]>
>> Signed-off-by: Felix Fietkau <[email protected]>
>> ---
>> .../mediatek/mediatek,mt7622-pcie-mirror.yaml | 42 +++++++++++++++++++
>
> Eh, I wanted to ask to not put it inside arm/, but judging by your usage
> - you did not create drivers for both of these (WED and PCIe mirror).
>
> You only need them to expose address spaces via syscon.
>
> This actually looks hacky. Either WED and PCIe mirror are part of
> network driver, then add the address spaces via "reg". If they are not,
> but instead they are separate blocks, why you don't have drivers for them?
The code that uses the WED block is built into the Ethernet driver, but
not all SoCs that use this ethernet core have it. Also, there are two
WED blocks, and I'm not sure if future SoCs might have a different
number of them at some point.
The WED code also needs to access registers of the ethernet MAC.
One reason for having a separate device is this:
As long as WED is not in use, ethernet supports coherent DMA for
increased performance. When the first wireless device attaches to WED,
IO coherency gets disabled and the ethernet DMA rings are cleaned up and
allocated again, this time with the struct device of WED (which doesn't
have the dma-coherent property).

- Felix

2022-04-07 19:37:27

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 05/14] dt-bindings: arm: mediatek: document the pcie mirror node on MT7622

On Tue, Apr 05, 2022 at 09:57:46PM +0200, Felix Fietkau wrote:
> From: Lorenzo Bianconi <[email protected]>
>
> This patch adds the pcie mirror document bindings for MT7622 SoC.
> The feature is used for intercepting PCIe MMIO access for the WED core
> Add related info in mediatek-net bindings.
>
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> Signed-off-by: Felix Fietkau <[email protected]>
> ---
> .../mediatek/mediatek,mt7622-pcie-mirror.yaml | 42 +++++++++++++++++++
> .../devicetree/bindings/net/mediatek-net.txt | 2 +
> 2 files changed, 44 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml
>
> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml
> new file mode 100644
> index 000000000000..9fbeb626ab23
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: MediaTek PCIE Mirror Controller for MT7622
> +
> +maintainers:
> + - Lorenzo Bianconi <[email protected]>
> + - Felix Fietkau <[email protected]>
> +
> +description:
> + The mediatek PCIE mirror provides a configuration interface for PCIE
> + controller on MT7622 soc.
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - mediatek,mt7622-pcie-mirror
> + - const: syscon

This doesn't sound like a syscon to me. Are there multiple clients or
functions in this block? A 'syscon' property is not the only way to
create a regmap if that's what you need.

Rob

2022-04-07 20:21:46

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 05/14] dt-bindings: arm: mediatek: document the pcie mirror node on MT7622

On Wed, Apr 06, 2022 at 01:01:06PM +0200, Felix Fietkau wrote:
>
> On 06.04.22 10:20, Krzysztof Kozlowski wrote:
> > On 05/04/2022 21:57, Felix Fietkau wrote:
> > > From: Lorenzo Bianconi <[email protected]>
> > >
> > > This patch adds the pcie mirror document bindings for MT7622 SoC.
> > > The feature is used for intercepting PCIe MMIO access for the WED core
> > > Add related info in mediatek-net bindings.
> > >
> > > Signed-off-by: Lorenzo Bianconi <[email protected]>
> > > Signed-off-by: Felix Fietkau <[email protected]>
> > > ---
> > > .../mediatek/mediatek,mt7622-pcie-mirror.yaml | 42 +++++++++++++++++++
> >
> > Eh, I wanted to ask to not put it inside arm/, but judging by your usage
> > - you did not create drivers for both of these (WED and PCIe mirror).
> >
> > You only need them to expose address spaces via syscon.
> >
> > This actually looks hacky. Either WED and PCIe mirror are part of
> > network driver, then add the address spaces via "reg". If they are not,
> > but instead they are separate blocks, why you don't have drivers for them?
> The code that uses the WED block is built into the Ethernet driver, but not
> all SoCs that use this ethernet core have it. Also, there are two WED
> blocks, and I'm not sure if future SoCs might have a different number of
> them at some point.
> The WED code also needs to access registers of the ethernet MAC.
> One reason for having a separate device is this:
> As long as WED is not in use, ethernet supports coherent DMA for increased
> performance. When the first wireless device attaches to WED, IO coherency
> gets disabled and the ethernet DMA rings are cleaned up and allocated again,
> this time with the struct device of WED (which doesn't have the dma-coherent
> property).

I'm pretty sure there are assumptions in the driver core that coherency
is not changing on the fly. In any case, if it is, using 'dma-coherent'
is not appropriate. You obviously have another method to determine
whether you are coherent or not.

Rob

2022-04-07 20:25:40

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v2 05/14] dt-bindings: arm: mediatek: document the pcie mirror node on MT7622


On 07.04.22 19:16, Rob Herring wrote:
> On Wed, Apr 06, 2022 at 01:01:06PM +0200, Felix Fietkau wrote:
>>
>> On 06.04.22 10:20, Krzysztof Kozlowski wrote:
>> > On 05/04/2022 21:57, Felix Fietkau wrote:
>> > > From: Lorenzo Bianconi <[email protected]>
>> > >
>> > > This patch adds the pcie mirror document bindings for MT7622 SoC.
>> > > The feature is used for intercepting PCIe MMIO access for the WED core
>> > > Add related info in mediatek-net bindings.
>> > >
>> > > Signed-off-by: Lorenzo Bianconi <[email protected]>
>> > > Signed-off-by: Felix Fietkau <[email protected]>
>> > > ---
>> > > .../mediatek/mediatek,mt7622-pcie-mirror.yaml | 42 +++++++++++++++++++
>> >
>> > Eh, I wanted to ask to not put it inside arm/, but judging by your usage
>> > - you did not create drivers for both of these (WED and PCIe mirror).
>> >
>> > You only need them to expose address spaces via syscon.
>> >
>> > This actually looks hacky. Either WED and PCIe mirror are part of
>> > network driver, then add the address spaces via "reg". If they are not,
>> > but instead they are separate blocks, why you don't have drivers for them?
>> The code that uses the WED block is built into the Ethernet driver, but not
>> all SoCs that use this ethernet core have it. Also, there are two WED
>> blocks, and I'm not sure if future SoCs might have a different number of
>> them at some point.
>> The WED code also needs to access registers of the ethernet MAC.
>> One reason for having a separate device is this:
>> As long as WED is not in use, ethernet supports coherent DMA for increased
>> performance. When the first wireless device attaches to WED, IO coherency
>> gets disabled and the ethernet DMA rings are cleaned up and allocated again,
>> this time with the struct device of WED (which doesn't have the dma-coherent
>> property).
>
> I'm pretty sure there are assumptions in the driver core that coherency
> is not changing on the fly. In any case, if it is, using 'dma-coherent'
> is not appropriate. You obviously have another method to determine
> whether you are coherent or not.
It's not really on the fly. Before changing coherency, all DMA memory is
freed, and the subsequent reallocation uses the struct device of the WED
core, which does not have the dma-coherent property.

- Felix

2022-04-09 01:30:08

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v2 05/14] dt-bindings: arm: mediatek: document the pcie mirror node on MT7622

On 07.04.22 19:19, Rob Herring wrote:
> On Tue, Apr 05, 2022 at 09:57:46PM +0200, Felix Fietkau wrote:
>> From: Lorenzo Bianconi <[email protected]>
>>
>> This patch adds the pcie mirror document bindings for MT7622 SoC.
>> The feature is used for intercepting PCIe MMIO access for the WED core
>> Add related info in mediatek-net bindings.
>>
>> Signed-off-by: Lorenzo Bianconi <[email protected]>
>> Signed-off-by: Felix Fietkau <[email protected]>
>> ---
>> .../mediatek/mediatek,mt7622-pcie-mirror.yaml | 42 +++++++++++++++++++
>> .../devicetree/bindings/net/mediatek-net.txt | 2 +
>> 2 files changed, 44 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml
>> new file mode 100644
>> index 000000000000..9fbeb626ab23
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml
>> @@ -0,0 +1,42 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/arm/mediatek/mediatek,mt7622-pcie-mirror.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: MediaTek PCIE Mirror Controller for MT7622
>> +
>> +maintainers:
>> + - Lorenzo Bianconi <[email protected]>
>> + - Felix Fietkau <[email protected]>
>> +
>> +description:
>> + The mediatek PCIE mirror provides a configuration interface for PCIE
>> + controller on MT7622 soc.
>> +
>> +properties:
>> + compatible:
>> + items:
>> + - enum:
>> + - mediatek,mt7622-pcie-mirror
>> + - const: syscon
>
> This doesn't sound like a syscon to me. Are there multiple clients or
> functions in this block? A 'syscon' property is not the only way to
> create a regmap if that's what you need.
It's used only by the WED code in the ethernet driver, but there are
multiple WED instances and only a single pcie-mirror block containing
configuration for them.

- Felix