2022-04-06 06:10:58

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v2 04/14] dt-bindings: arm: mediatek: document WED binding for MT7622

From: Lorenzo Bianconi <[email protected]>

Document the binding for the Wireless Ethernet Dispatch core on the MT7622
SoC, which is used for Ethernet->WLAN offloading
Add related info in mediatek-net bindings.

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

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml
new file mode 100644
index 000000000000..787d6673f952
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/arm/mediatek/mediatek,mt7622-wed.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: MediaTek Wireless Ethernet Dispatch Controller for MT7622
+
+maintainers:
+ - Lorenzo Bianconi <[email protected]>
+ - Felix Fietkau <[email protected]>
+
+description:
+ The mediatek wireless ethernet dispatch controller can be configured to
+ intercept and handle access to the WLAN DMA queues and PCIe interrupts
+ and implement hardware flow offloading from ethernet to WLAN.
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - mediatek,mt7622-wed
+ - const: syscon
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ wed0: wed@1020a000 {
+ compatible = "mediatek,mt7622-wed","syscon";
+ reg = <0 0x1020a000 0 0x1000>;
+ interrupts = <GIC_SPI 214 IRQ_TYPE_LEVEL_LOW>;
+ };
+ };
diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt b/Documentation/devicetree/bindings/net/mediatek-net.txt
index 13cb12ee4ed6..1c8dc44bbb52 100644
--- a/Documentation/devicetree/bindings/net/mediatek-net.txt
+++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
@@ -46,6 +46,8 @@ Optional properties:
- mediatek,cci-control: phandle to the cache coherent interconnect node
- mediatek,hifsys: phandle to the mediatek hifsys controller used to provide
various clocks and reset to the system.
+- mediatek,wed: a list of phandles to wireless ethernet dispatch nodes for
+ MT7622 SoC.

* Ethernet MAC node

--
2.35.1


2022-04-06 14:19:24

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v2 04/14] dt-bindings: arm: mediatek: document WED binding for MT7622

On Wed, Apr 6, 2022 at 10:18 AM Felix Fietkau <[email protected]> wrote:
> On 06.04.22 10:09, Krzysztof Kozlowski wrote:
> > On 05/04/2022 21:57, Felix Fietkau wrote:
> >> From: Lorenzo Bianconi <[email protected]>
> >>
> >> Document the binding for the Wireless Ethernet Dispatch core on the MT7622
> >> SoC, which is used for Ethernet->WLAN offloading
> >> Add related info in mediatek-net bindings.
> >>
> >> Signed-off-by: Lorenzo Bianconi <[email protected]>
> >> Signed-off-by: Felix Fietkau <[email protected]>
> >
> > Thank you for your patch. There is something to discuss/improve.
> >
> >> ---
> >> .../arm/mediatek/mediatek,mt7622-wed.yaml | 50 +++++++++++++++++++
> >> .../devicetree/bindings/net/mediatek-net.txt | 2 +
> >> 2 files changed, 52 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml
> >
> > Don't store drivers in arm directory. See:
> > https://lore.kernel.org/linux-devicetree/[email protected]/
> >
> > Isn't this a network offload engine? If yes, then probably it should be
> > in "net/".
> It's not a network offload engine by itself. It's a SoC component that
> connects to the offload engine and controls a MTK PCIe WLAN device,
> intercepting interrupts and DMA rings in order to be able to inject
> packets coming in from the offload engine.
> Do you think it still belongs in net, or maybe in soc instead?

I think it belongs into drivers/net/. Presumably this has some kind of
user interface to configure which packets are forwarded? I would not
want to maintain that in a SoC driver as this clearly needs to communicate
with both of the normal network devices in some form.

Arnd

2022-04-06 14:37:30

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 04/14] dt-bindings: arm: mediatek: document WED binding for MT7622

On 05/04/2022 21:57, Felix Fietkau wrote:
> From: Lorenzo Bianconi <[email protected]>
>
> Document the binding for the Wireless Ethernet Dispatch core on the MT7622
> SoC, which is used for Ethernet->WLAN offloading
> Add related info in mediatek-net bindings.
>
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> Signed-off-by: Felix Fietkau <[email protected]>

Thank you for your patch. There is something to discuss/improve.

> ---
> .../arm/mediatek/mediatek,mt7622-wed.yaml | 50 +++++++++++++++++++
> .../devicetree/bindings/net/mediatek-net.txt | 2 +
> 2 files changed, 52 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml

Don't store drivers in arm directory. See:
https://lore.kernel.org/linux-devicetree/[email protected]/

Isn't this a network offload engine? If yes, then probably it should be
in "net/".

>
> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml
> new file mode 100644
> index 000000000000..787d6673f952
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/arm/mediatek/mediatek,mt7622-wed.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: MediaTek Wireless Ethernet Dispatch Controller for MT7622
> +
> +maintainers:
> + - Lorenzo Bianconi <[email protected]>
> + - Felix Fietkau <[email protected]>
> +
> +description:
> + The mediatek wireless ethernet dispatch controller can be configured to
> + intercept and handle access to the WLAN DMA queues and PCIe interrupts
> + and implement hardware flow offloading from ethernet to WLAN.
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - mediatek,mt7622-wed
> + - const: syscon
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + wed0: wed@1020a000 {

Generic node name, "wed" is specific. Maybe "network-offload"? Or
"network-accelerator"? You probably know better what this device does,
so maybe come with some generic name?

The same in DTS patch.

The bindings themself look ok.

Best regards,
Krzysztof

2022-04-06 15:00:17

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v2 04/14] dt-bindings: arm: mediatek: document WED binding for MT7622


On 06.04.22 10:09, Krzysztof Kozlowski wrote:
> On 05/04/2022 21:57, Felix Fietkau wrote:
>> From: Lorenzo Bianconi <[email protected]>
>>
>> Document the binding for the Wireless Ethernet Dispatch core on the MT7622
>> SoC, which is used for Ethernet->WLAN offloading
>> Add related info in mediatek-net bindings.
>>
>> Signed-off-by: Lorenzo Bianconi <[email protected]>
>> Signed-off-by: Felix Fietkau <[email protected]>
>
> Thank you for your patch. There is something to discuss/improve.
>
>> ---
>> .../arm/mediatek/mediatek,mt7622-wed.yaml | 50 +++++++++++++++++++
>> .../devicetree/bindings/net/mediatek-net.txt | 2 +
>> 2 files changed, 52 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml
>
> Don't store drivers in arm directory. See:
> https://lore.kernel.org/linux-devicetree/[email protected]/
>
> Isn't this a network offload engine? If yes, then probably it should be
> in "net/".
It's not a network offload engine by itself. It's a SoC component that
connects to the offload engine and controls a MTK PCIe WLAN device,
intercepting interrupts and DMA rings in order to be able to inject
packets coming in from the offload engine.
Do you think it still belongs in net, or maybe in soc instead?

>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml
>> new file mode 100644
>> index 000000000000..787d6673f952
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml
>> @@ -0,0 +1,50 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/arm/mediatek/mediatek,mt7622-wed.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: MediaTek Wireless Ethernet Dispatch Controller for MT7622
>> +
>> +maintainers:
>> + - Lorenzo Bianconi <[email protected]>
>> + - Felix Fietkau <[email protected]>
>> +
>> +description:
>> + The mediatek wireless ethernet dispatch controller can be configured to
>> + intercept and handle access to the WLAN DMA queues and PCIe interrupts
>> + and implement hardware flow offloading from ethernet to WLAN.
>> +
>> +properties:
>> + compatible:
>> + items:
>> + - enum:
>> + - mediatek,mt7622-wed
>> + - const: syscon
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> + soc {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + wed0: wed@1020a000 {
>
> Generic node name, "wed" is specific. Maybe "network-offload"? Or
> "network-accelerator"? You probably know better what this device does,
> so maybe come with some generic name?
wed stands for "wireless ethernet dispatch". Both network-offload and
network-accelerator don't really fit. Would it make sense to spell it
out, or do you have any better naming ideas?

Thanks,

- Felix

2022-04-06 15:01:56

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v2 04/14] dt-bindings: arm: mediatek: document WED binding for MT7622

On 06.04.22 10:29, Arnd Bergmann wrote:
> On Wed, Apr 6, 2022 at 10:18 AM Felix Fietkau <[email protected]> wrote:
>> On 06.04.22 10:09, Krzysztof Kozlowski wrote:
>> > On 05/04/2022 21:57, Felix Fietkau wrote:
>> >> From: Lorenzo Bianconi <[email protected]>
>> >>
>> >> Document the binding for the Wireless Ethernet Dispatch core on the MT7622
>> >> SoC, which is used for Ethernet->WLAN offloading
>> >> Add related info in mediatek-net bindings.
>> >>
>> >> Signed-off-by: Lorenzo Bianconi <[email protected]>
>> >> Signed-off-by: Felix Fietkau <[email protected]>
>> >
>> > Thank you for your patch. There is something to discuss/improve.
>> >
>> >> ---
>> >> .../arm/mediatek/mediatek,mt7622-wed.yaml | 50 +++++++++++++++++++
>> >> .../devicetree/bindings/net/mediatek-net.txt | 2 +
>> >> 2 files changed, 52 insertions(+)
>> >> create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml
>> >
>> > Don't store drivers in arm directory. See:
>> > https://lore.kernel.org/linux-devicetree/[email protected]/
>> >
>> > Isn't this a network offload engine? If yes, then probably it should be
>> > in "net/".
>> It's not a network offload engine by itself. It's a SoC component that
>> connects to the offload engine and controls a MTK PCIe WLAN device,
>> intercepting interrupts and DMA rings in order to be able to inject
>> packets coming in from the offload engine.
>> Do you think it still belongs in net, or maybe in soc instead?
>
> I think it belongs into drivers/net/. Presumably this has some kind of
> user interface to configure which packets are forwarded? I would not
> want to maintain that in a SoC driver as this clearly needs to communicate
> with both of the normal network devices in some form.
The WLAN driver attaches to WED in order to deal with the intercepted
DMA rings, but other than that, WED itself has no user configuration.
Offload is controlled by the PPE code in the ethernet driver (which is
already upstream), and WED simply provides a destination port for PPE,
which allows packets to flow to the wireless device.

- Felix

2022-04-06 15:06:49

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 04/14] dt-bindings: arm: mediatek: document WED binding for MT7622

On 06/04/2022 10:32, Felix Fietkau wrote:
> On 06.04.22 10:29, Arnd Bergmann wrote:
>> On Wed, Apr 6, 2022 at 10:18 AM Felix Fietkau <[email protected]>
>> wrote:
>>> On 06.04.22 10:09, Krzysztof Kozlowski wrote:
>>>> On 05/04/2022 21:57, Felix Fietkau wrote:
>>>>> From: Lorenzo Bianconi <[email protected]>
>>>>>
>>>>> Document the binding for the Wireless Ethernet Dispatch core
>>>>> on the MT7622 SoC, which is used for Ethernet->WLAN
>>>>> offloading Add related info in mediatek-net bindings.
>>>>>
>>>>> Signed-off-by: Lorenzo Bianconi <[email protected]>
>>>>> Signed-off-by: Felix Fietkau <[email protected]>
>>>>
>>>> Thank you for your patch. There is something to
>>>> discuss/improve.
>>>>
>>>>> --- .../arm/mediatek/mediatek,mt7622-wed.yaml | 50
>>>>> +++++++++++++++++++
>>>>> .../devicetree/bindings/net/mediatek-net.txt | 2 + 2 files
>>>>> changed, 52 insertions(+) create mode 100644
>>>>> Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-wed.yaml
>>>>
>>>>
>>>>>
Don't store drivers in arm directory. See:
>>>> https://lore.kernel.org/linux-devicetree/[email protected]/
>>>>
>>>>
>>>>
Isn't this a network offload engine? If yes, then probably it should be
>>>> in "net/".
>>> It's not a network offload engine by itself. It's a SoC component
>>> that connects to the offload engine and controls a MTK PCIe WLAN
>>> device, intercepting interrupts and DMA rings in order to be able
>>> to inject packets coming in from the offload engine. Do you think
>>> it still belongs in net, or maybe in soc instead?
>>
>> I think it belongs into drivers/net/. Presumably this has some kind
>> of user interface to configure which packets are forwarded? I would
>> not want to maintain that in a SoC driver as this clearly needs to
>> communicate with both of the normal network devices in some form.
> The WLAN driver attaches to WED in order to deal with the intercepted
> DMA rings, but other than that, WED itself has no user
> configuration. Offload is controlled by the PPE code in the ethernet
> driver (which is already upstream), and WED simply provides a
> destination port for PPE, which allows packets to flow to the
> wireless device.

Thanks for clarification. I still wonder about the missing drivers as I
responded to your second bindings:
https://lore.kernel.org/all/[email protected]/T/#m6d108c644f0c05cd12c05e56abe2ef75760c6cef

Both of these compatibles - WED and PCIe - are not actually used. Now
everything is done inside your Ethernet driver which pokes WED and
PCIe-mirror address space via regmap/syscon.

Separate bindings might have sense if WED/PCIe mirror were ever
converted to real drivers.

Best regards,
Krzysztof

2022-04-07 19:41:26

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v2 04/14] dt-bindings: arm: mediatek: document WED binding for MT7622

On 07.04.22 17:50, Andrew Lunn wrote:
>> > Isn't this a network offload engine? If yes, then probably it should be
>> > in "net/".
>> It's not a network offload engine by itself. It's a SoC component that
>> connects to the offload engine and controls a MTK PCIe WLAN device,
>> intercepting interrupts and DMA rings in order to be able to inject packets
>> coming in from the offload engine.
>
> Hi Felix
>
> Maybe turn the question around. Can it be used for something other
> than networking? If not, then somewhere under net seems reasonable.
I'm fine with moving this to net.

- Felix

2022-04-07 20:27:33

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v2 04/14] dt-bindings: arm: mediatek: document WED binding for MT7622


On 06.04.22 10:57, Krzysztof Kozlowski wrote:
> Thanks for clarification. I still wonder about the missing drivers as I
> responded to your second bindings:
> https://lore.kernel.org/all/[email protected]/T/#m6d108c644f0c05cd12c05e56abe2ef75760c6cef
>
> Both of these compatibles - WED and PCIe - are not actually used. Now
> everything is done inside your Ethernet driver which pokes WED and
> PCIe-mirror address space via regmap/syscon.
>
> Separate bindings might have sense if WED/PCIe mirror were ever
> converted to real drivers.I think in terms of hardware description it makes more sense to have
separate nodes, even if the implementation uses them in one driver at
the moment.

- Felix

2022-04-07 21:10:17

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v2 04/14] dt-bindings: arm: mediatek: document WED binding for MT7622

> > Isn't this a network offload engine? If yes, then probably it should be
> > in "net/".
> It's not a network offload engine by itself. It's a SoC component that
> connects to the offload engine and controls a MTK PCIe WLAN device,
> intercepting interrupts and DMA rings in order to be able to inject packets
> coming in from the offload engine.

Hi Felix

Maybe turn the question around. Can it be used for something other
than networking? If not, then somewhere under net seems reasonable.

Andrew