| Message ID | 20230819023132.23082-2-rohan.g.thomas@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stmmac: Tx coe sw fallback | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 9 this patch: 9 |
| netdev/cc_maintainers | success | CCed 12 of 12 maintainers |
| netdev/build_clang | success | Errors and warnings before: 9 this patch: 9 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 9 this patch: 9 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 9 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Sat, Aug 19, 2023 at 10:31:31AM +0800, Rohan G Thomas wrote: > Add dt-bindings for the number of tx queues with coe support. Some > dwmac IPs support tx queues only for a few initial tx queues, > starting from tx queue 0. > > Signed-off-by: Rohan G Thomas <rohan.g.thomas@intel.com> > Acked-by: Conor Dooley <conor.dooley@microchip.com> Reviewed-by: Serge Semin <fancer.lancer@gmail.com> -Serge(y) > --- > Documentation/devicetree/bindings/net/snps,dwmac.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > index ddf9522a5dc2..0c6431c10cf9 100644 > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > @@ -313,6 +313,9 @@ properties: > snps,tx-queues-to-use: > $ref: /schemas/types.yaml#/definitions/uint32 > description: number of TX queues to be used in the driver > + snps,tx-queues-with-coe: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: number of TX queues that support TX checksum offloading > snps,tx-sched-wrr: > type: boolean > description: Weighted Round Robin > -- > 2.19.0 >
On Sat, 19 Aug 2023 10:31:31 +0800 Rohan G Thomas wrote: > + snps,tx-queues-with-coe: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: number of TX queues that support TX checksum offloading Is it going to be obvious that if not present all queues support checksum offload? I think we should document the default.
On Sat, 19 Aug 2023 10:31:31 +0800 Rohan G Thomas wrote: >> + snps,tx-queues-with-coe: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: number of TX queues that support TX checksum >> + offloading > >Is it going to be obvious that if not present all queues support checksum offload? I think we should document the default. Agreed. Will add this in the next version. BR, Rohan
On Tue, Aug 22, 2023 at 05:15:25PM -0700, Jakub Kicinski wrote: > On Sat, 19 Aug 2023 10:31:31 +0800 Rohan G Thomas wrote: > > + snps,tx-queues-with-coe: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: number of TX queues that support TX checksum offloading > > Is it going to be obvious that if not present all queues support > checksum offload? I think we should document the default. This question is debatable: 1. By default the DW xGMAC and DW QoS Eth IP-cores are synthesized with only the very first Tx queue having Tx COE enabled. 2. If TSO is disabled then the Tx COE can be individually enabled for each queue available on DW QoS Eth controller and for the very first N queues on DW xGMAC controller. 3. If TSO is enabled then the Tx COE will be automatically and always enabled for as many first queues as there are TSO-capable DMA-channels. 4. At the current state the STMMAC driver assumes that all Tx Queues support Tx COE. The entry 4 can't be changed since we'll risk to catch regressions on the platforms with no property specified. On the other hand it partly contradicts to the rest of the entries. I don't know what would be a correct way to specify the default value in this case. Most likely just keep the entry 4 and be done with it. BTW I just noticed that but the suggested "snps,tx-queues-with-coe" property semantic will only cover a DW XGMAC-part of the case 2. DW QoS Eth can be synthesized with Tx COE individually enabled for a particular queue if TSO is unavailable. -Serge(y) > -- > pw-bot: cr
>On Tue, Aug 22, 2023 at 05:15:25PM -0700, Jakub Kicinski wrote: >> On Sat, 19 Aug 2023 10:31:31 +0800 Rohan G Thomas wrote: >> > + snps,tx-queues-with-coe: >> > + $ref: /schemas/types.yaml#/definitions/uint32 >> > + description: number of TX queues that support TX checksum offloading >> > >> Is it going to be obvious that if not present all queues support >> checksum offload? I think we should document the default. > >This question is debatable: >1. By default the DW xGMAC and DW QoS Eth IP-cores are >synthesized with only the very first Tx queue having Tx COE enabled. >2. If TSO is disabled then the Tx COE can be individually enabled >for each queue available on DW QoS Eth controller and for the very >first N queues on DW xGMAC controller. >3. If TSO is enabled then the Tx COE will be automatically and always >enabled for as many first queues as there are TSO-capable >DMA-channels. >4. At the current state the STMMAC driver assumes that all Tx Queues >support Tx COE. > >The entry 4 can't be changed since we'll risk to catch regressions on >the platforms with no property specified. On the other hand it partly >contradicts to the rest of the entries. I don't know what would be a >correct way to specify the default value in this case. Most likely >just keep the entry 4 and be done with it. > >BTW I just noticed that but the suggested "snps,tx-queues-with-coe" >property semantic will only cover a DW XGMAC-part of the case 2. DW >QoS Eth can be synthesized with Tx COE individually enabled for a >particular queue if TSO is unavailable. Hi Serge, Didn't know about a different IP configuration supported by DW QoS Eth IP. If this is the case, I think we can have a flag 'coe-unsupported' for any TX queue subnode as below. + snps,coe-unsupported: + $ref: /schemas/types.yaml#/definitions/flag + description: + TX checksum offload is unsupported by the TX queue. If TX checksum + offload is requested for a packet to be transmitted through this + TX queue then have a software fallback in the driver for checksum + calculation. If this is okay, I can rework the patch based on this. Covers both DW QoS Eth IP and DW XGMAC IP cases. > >-Serge(y) > >> -- >> pw-bot: cr BR, Rohan
On Thu, Aug 24, 2023 at 01:10:04AM +0800, Rohan G Thomas wrote: > >On Tue, Aug 22, 2023 at 05:15:25PM -0700, Jakub Kicinski wrote: > >> On Sat, 19 Aug 2023 10:31:31 +0800 Rohan G Thomas wrote: > >> > + snps,tx-queues-with-coe: > >> > + $ref: /schemas/types.yaml#/definitions/uint32 > >> > + description: number of TX queues that support TX checksum offloading > >> > > > >> Is it going to be obvious that if not present all queues support > >> checksum offload? I think we should document the default. > > > >This question is debatable: > >1. By default the DW xGMAC and DW QoS Eth IP-cores are > >synthesized with only the very first Tx queue having Tx COE enabled. > >2. If TSO is disabled then the Tx COE can be individually enabled > >for each queue available on DW QoS Eth controller and for the very > >first N queues on DW xGMAC controller. > >3. If TSO is enabled then the Tx COE will be automatically and always > >enabled for as many first queues as there are TSO-capable > >DMA-channels. > >4. At the current state the STMMAC driver assumes that all Tx Queues > >support Tx COE. > > > >The entry 4 can't be changed since we'll risk to catch regressions on > >the platforms with no property specified. On the other hand it partly > >contradicts to the rest of the entries. I don't know what would be a > >correct way to specify the default value in this case. Most likely > >just keep the entry 4 and be done with it. > > > >BTW I just noticed that but the suggested "snps,tx-queues-with-coe" > >property semantic will only cover a DW XGMAC-part of the case 2. DW > >QoS Eth can be synthesized with Tx COE individually enabled for a > >particular queue if TSO is unavailable. > > Hi Serge, > > Didn't know about a different IP configuration supported by DW QoS Eth IP. If > this is the case, I think we can have a flag 'coe-unsupported' for any TX > queue subnode as below. > > + snps,coe-unsupported: > + $ref: /schemas/types.yaml#/definitions/flag AFAIR tKrzysztof preferred to use type: boolean for the flags. > + description: > + TX checksum offload is unsupported by the TX queue. > + If TX checksum > + offload is requested for a packet to be transmitted through this > + TX queue then have a software fallback in the driver for checksum > + calculation. This is redundant in the HW description. > > If this is okay, I can rework the patch based on this. Covers both DW QoS Eth IP > and DW XGMAC IP cases. I guess that's the only choice we have seeing the driver already expects all the Tx queues having the COE supported. -Serge(y) > > > > >-Serge(y) > > > >> -- > >> pw-bot: cr > > BR, > Rohan
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml index ddf9522a5dc2..0c6431c10cf9 100644 --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml @@ -313,6 +313,9 @@ properties: snps,tx-queues-to-use: $ref: /schemas/types.yaml#/definitions/uint32 description: number of TX queues to be used in the driver + snps,tx-queues-with-coe: + $ref: /schemas/types.yaml#/definitions/uint32 + description: number of TX queues that support TX checksum offloading snps,tx-sched-wrr: type: boolean description: Weighted Round Robin