[PATCH v10 0/2] Introduce ICSSG based ethernet Driver

[PATCH v10 0/2] Introduce ICSSG based ethernet Driver

[MD Danish Anwar]

The Programmable Real-time Unit and Industrial Communication Subsystem
Gigabit (PRU_ICSSG) is a low-latency microcontroller subsystem in the TI
SoCs. This subsystem is provided for the use cases like the implementation
of custom peripheral interfaces, offloading of tasks from the other
processor cores of the SoC, etc.

The subsystem includes many accelerators for data processing like
multiplier and multiplier-accumulator. It also has peripherals like
UART, MII/RGMII, MDIO, etc. Every ICSSG core includes two 32-bit
load/store RISC CPU cores called PRUs.

The above features allow it to be used for implementing custom firmware
based peripherals like ethernet.

This series adds the YAML documentation and the driver with basic EMAC
support for TI AM654 Silicon Rev 2 SoC with the PRU_ICSSG Sub-system.
running dual-EMAC firmware.
This currently supports basic EMAC with 1Gbps and 100Mbps link. 10M and
half-duplex modes are not yet supported because they require the support
of an IEP, which will be added later.
Advanced features like switch-dev and timestamping will be added later. 

This is the v10 of the patch series [v1]. This version of the patchset 
addresses comments made on v9.

There series doesn't have any dependency.

Changes from v9 to v10 :
*) Rebased the series on latest net-next.
*) Moved 'ndev prueth->emac[mac] == emac' assignment to the end of function
   prueth_netdev_init().
*) In unsupported phy_mode switch case instead of returning -EINVAL, store
   the error code in ret and 'goto free'

Changes from v8 to v9 :
*) Rebased the series on latest net-next.
*) Fixed smatch and sparse warnings as pointed by Simon.
*) Fixed leaky ndev in prueth_netdev_init() as asked by Simon.

Changes from v7 to v8 :
*) Rebased the series on 6.5-rc1.
*) Fixed few formattings. 

Changes from v6 to v7 :
*) Added RB tag of Rob in patch 1 of this series.
*) Addressed Simon's comment on patch 2 of the series.
*) Rebased patchset on next-20230428 linux-next.

Changes from v5 to v6 :
*) Added RB tag of Andrew Lunn in patch 2 of this series.
*) Addressed Rob's comment on patch 1 of the series.
*) Rebased patchset on next-20230421 linux-next.

Changes from v4 to v5 :
*) Re-arranged properties section in ti,icssg-prueth.yaml file.
*) Added requirement for minimum one ethernet port.
*) Fixed some minor formatting errors as asked by Krzysztof.
*) Dropped SGMII mode from enum mii_mode as SGMII mode is not currently
   supported by the driver.
*) Added switch-case block to handle different phy modes by ICSSG driver.

Changes from v3 to v4 :
*) Addressed Krzysztof's comments and fixed dt_binding_check errors in 
   patch 1/2.
*) Added interrupt-extended property in ethernet-ports properties section.
*) Fixed comments in file icssg_switch_map.h according to the Linux coding
   style in patch 2/2. Added Documentation of structures in patch 2/2.

Changes from v2 to v3 :
*) Addressed Rob and Krzysztof's comments on patch 1 of this series.
   Fixed indentation. Removed description and pinctrl section from 
   ti,icssg-prueth.yaml file.
*) Addressed Krzysztof, Paolo, Randy, Andrew and Christophe's comments on 
   patch 2 of this seires.
*) Fixed blanklines in Kconfig and Makefile. Changed structures to const 
   as suggested by Krzysztof.
*) Fixed while loop logic in emac_tx_complete_packets() API as suggested 
   by Paolo. Previously in the loop's last iteration 'budget' was 0 and 
   napi_consume_skb would wrongly assume the caller is not in NAPI context
   Now, budget won't be zero in last iteration of loop. 
*) Removed inline functions addr_to_da1() and addr_to_da0() as asked by 
   Andrew.
*) Added dev_err_probe() instead of dev_err() as suggested by Christophe.
*) In ti,icssg-prueth.yaml file, in the patternProperties section of 
   ethernet-ports, kept the port name as "port" instead of "ethernet-port" 
   as all other drivers were using "port". Will change it if is compulsory 
   to use "ethernet-port".

[v1] https://lore.kernel.org/all/20220506052433.28087-1-p-mohan@ti.com/
[v2] https://lore.kernel.org/all/20220531095108.21757-1-p-mohan@ti.com/
[v3] https://lore.kernel.org/all/20221223110930.1337536-1-danishanwar@ti.com/
[v4] https://lore.kernel.org/all/20230206060708.3574472-1-danishanwar@ti.com/
[v5] https://lore.kernel.org/all/20230210114957.2667963-1-danishanwar@ti.com/
[v6] https://lore.kernel.org/all/20230424053233.2338782-1-danishanwar@ti.com/
[v7] https://lore.kernel.org/all/20230502061650.2716736-1-danishanwar@ti.com/
[v8] https://lore.kernel.org/all/20230710053550.89160-1-danishanwar@ti.com/
[v9] https://lore.kernel.org/all/20230714094432.1834489-1-danishanwar@ti.com/

Thanks and Regards,
Md Danish Anwar

MD Danish Anwar (1):
  dt-bindings: net: Add ICSSG Ethernet

Roger Quadros (1):
  net: ti: icssg-prueth: Add ICSSG ethernet driver

 .../bindings/net/ti,icssg-prueth.yaml         |  184 ++
 drivers/net/ethernet/ti/Kconfig               |   13 +
 drivers/net/ethernet/ti/Makefile              |    2 +
 drivers/net/ethernet/ti/icssg_classifier.c    |  367 ++++
 drivers/net/ethernet/ti/icssg_config.c        |  450 ++++
 drivers/net/ethernet/ti/icssg_config.h        |  200 ++
 drivers/net/ethernet/ti/icssg_ethtool.c       |  326 +++
 drivers/net/ethernet/ti/icssg_mii_cfg.c       |  120 ++
 drivers/net/ethernet/ti/icssg_mii_rt.h        |  151 ++
 drivers/net/ethernet/ti/icssg_prueth.c        | 1890 +++++++++++++++++
 drivers/net/ethernet/ti/icssg_prueth.h        |  252 +++
 drivers/net/ethernet/ti/icssg_queues.c        |   38 +
 drivers/net/ethernet/ti/icssg_switch_map.h    |  234 ++
 13 files changed, 4227 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
 create mode 100644 drivers/net/ethernet/ti/icssg_classifier.c
 create mode 100644 drivers/net/ethernet/ti/icssg_config.c
 create mode 100644 drivers/net/ethernet/ti/icssg_config.h
 create mode 100644 drivers/net/ethernet/ti/icssg_ethtool.c
 create mode 100644 drivers/net/ethernet/ti/icssg_mii_cfg.c
 create mode 100644 drivers/net/ethernet/ti/icssg_mii_rt.h
 create mode 100644 drivers/net/ethernet/ti/icssg_prueth.c
 create mode 100644 drivers/net/ethernet/ti/icssg_prueth.h
 create mode 100644 drivers/net/ethernet/ti/icssg_queues.c
 create mode 100644 drivers/net/ethernet/ti/icssg_switch_map.h

-- 
2.34.1

[PATCH v10 1/2] dt-bindings: net: Add ICSSG Ethernet

[MD Danish Anwar]

Add a YAML binding document for the ICSSG Programmable real time unit
based Ethernet hardware. The ICSSG driver uses the PRU and PRUSS consumer
APIs to interface the PRUs and load/run the firmware for supporting
ethernet functionality.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 .../bindings/net/ti,icssg-prueth.yaml         | 184 ++++++++++++++++++
 1 file changed, 184 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml

diff --git a/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
new file mode 100644
index 000000000000..8ec30b3eb760
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
@@ -0,0 +1,184 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/ti,icssg-prueth.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments ICSSG PRUSS Ethernet
+
+maintainers:
+  - Md Danish Anwar <danishanwar@ti.com>
+
+description:
+  Ethernet based on the Programmable Real-Time Unit and Industrial
+  Communication Subsystem.
+
+allOf:
+  - $ref: /schemas/remoteproc/ti,pru-consumer.yaml#
+
+properties:
+  compatible:
+    enum:
+      - ti,am654-icssg-prueth  # for AM65x SoC family
+
+  sram:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle to MSMC SRAM node
+
+  dmas:
+    maxItems: 10
+
+  dma-names:
+    items:
+      - const: tx0-0
+      - const: tx0-1
+      - const: tx0-2
+      - const: tx0-3
+      - const: tx1-0
+      - const: tx1-1
+      - const: tx1-2
+      - const: tx1-3
+      - const: rx0
+      - const: rx1
+
+  ti,mii-g-rt:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle to MII_G_RT module's syscon regmap.
+
+  ti,mii-rt:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle to MII_RT module's syscon regmap
+
+  interrupts:
+    maxItems: 2
+    description:
+      Interrupt specifiers to TX timestamp IRQ.
+
+  interrupt-names:
+    items:
+      - const: tx_ts0
+      - const: tx_ts1
+
+  ethernet-ports:
+    type: object
+    additionalProperties: false
+
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      ^port@[0-1]$:
+        type: object
+        description: ICSSG PRUETH external ports
+        $ref: ethernet-controller.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          reg:
+            items:
+              - enum: [0, 1]
+            description: ICSSG PRUETH port number
+
+          interrupts:
+            maxItems: 1
+
+          ti,syscon-rgmii-delay:
+            items:
+              - items:
+                  - description: phandle to system controller node
+                  - description: The offset to ICSSG control register
+            $ref: /schemas/types.yaml#/definitions/phandle-array
+            description:
+              phandle to system controller node and register offset
+              to ICSSG control register for RGMII transmit delay
+
+        required:
+          - reg
+    anyOf:
+      - required:
+          - port@0
+      - required:
+          - port@1
+
+required:
+  - compatible
+  - sram
+  - dmas
+  - dma-names
+  - ethernet-ports
+  - ti,mii-g-rt
+  - interrupts
+  - interrupt-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    /* Example k3-am654 base board SR2.0, dual-emac */
+    pruss2_eth: ethernet {
+        compatible = "ti,am654-icssg-prueth";
+        pinctrl-names = "default";
+        pinctrl-0 = <&icssg2_rgmii_pins_default>;
+        sram = <&msmc_ram>;
+
+        ti,prus = <&pru2_0>, <&rtu2_0>, <&tx_pru2_0>,
+                  <&pru2_1>, <&rtu2_1>, <&tx_pru2_1>;
+        firmware-name = "ti-pruss/am65x-pru0-prueth-fw.elf",
+                        "ti-pruss/am65x-rtu0-prueth-fw.elf",
+                        "ti-pruss/am65x-txpru0-prueth-fw.elf",
+                        "ti-pruss/am65x-pru1-prueth-fw.elf",
+                        "ti-pruss/am65x-rtu1-prueth-fw.elf",
+                        "ti-pruss/am65x-txpru1-prueth-fw.elf";
+        ti,pruss-gp-mux-sel = <2>,      /* MII mode */
+                              <2>,
+                              <2>,
+                              <2>,      /* MII mode */
+                              <2>,
+                              <2>;
+        dmas = <&main_udmap 0xc300>, /* egress slice 0 */
+               <&main_udmap 0xc301>, /* egress slice 0 */
+               <&main_udmap 0xc302>, /* egress slice 0 */
+               <&main_udmap 0xc303>, /* egress slice 0 */
+               <&main_udmap 0xc304>, /* egress slice 1 */
+               <&main_udmap 0xc305>, /* egress slice 1 */
+               <&main_udmap 0xc306>, /* egress slice 1 */
+               <&main_udmap 0xc307>, /* egress slice 1 */
+               <&main_udmap 0x4300>, /* ingress slice 0 */
+               <&main_udmap 0x4301>; /* ingress slice 1 */
+        dma-names = "tx0-0", "tx0-1", "tx0-2", "tx0-3",
+                    "tx1-0", "tx1-1", "tx1-2", "tx1-3",
+                    "rx0", "rx1";
+        ti,mii-g-rt = <&icssg2_mii_g_rt>;
+        interrupt-parent = <&icssg2_intc>;
+        interrupts = <24 0 2>, <25 1 3>;
+        interrupt-names = "tx_ts0", "tx_ts1";
+        ethernet-ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            pruss2_emac0: port@0 {
+                reg = <0>;
+                phy-handle = <&pruss2_eth0_phy>;
+                phy-mode = "rgmii-id";
+                interrupts-extended = <&icssg2_intc 24>;
+                ti,syscon-rgmii-delay = <&scm_conf 0x4120>;
+                /* Filled in by bootloader */
+                local-mac-address = [00 00 00 00 00 00];
+            };
+
+            pruss2_emac1: port@1 {
+                reg = <1>;
+                phy-handle = <&pruss2_eth1_phy>;
+                phy-mode = "rgmii-id";
+                interrupts-extended = <&icssg2_intc 25>;
+                ti,syscon-rgmii-delay = <&scm_conf 0x4124>;
+                /* Filled in by bootloader */
+                local-mac-address = [00 00 00 00 00 00];
+            };
+        };
+    };
-- 
2.34.1

Re: [PATCH v10 2/2] net: ti: icssg-prueth: Add ICSSG ethernet driver

[Jakub Kicinski]

The patch is too big to review.

Please break it apart separating into individual features, targeting
around 10 patches in the series. That will make it easier for reviewers
to take a look at the features in which they have expertise.

See two things which jumped out at me immediately below:

On Wed, 19 Jul 2023 13:57:55 +0530 MD Danish Anwar wrote:
> +	ICSSG_STATS(rx_crc_error_frames),

> +	ICSSG_STATS(rx_max_size_error_frames),
> +	ICSSG_STATS(rx_frame_min_size),
> +	ICSSG_STATS(rx_min_size_error_frames),
> +	ICSSG_STATS(rx_overrun_frames),

> +	ICSSG_STATS(rx_64B_frames),
> +	ICSSG_STATS(rx_bucket1_frames),
> +	ICSSG_STATS(rx_bucket2_frames),
> +	ICSSG_STATS(rx_bucket3_frames),
> +	ICSSG_STATS(rx_bucket4_frames),
> +	ICSSG_STATS(rx_bucket5_frames),
> +	ICSSG_STATS(rx_total_bytes),
> +	ICSSG_STATS(rx_tx_total_bytes),
> +	/* Tx */
> +	ICSSG_STATS(tx_good_frames),
> +	ICSSG_STATS(tx_broadcast_frames),
> +	ICSSG_STATS(tx_multicast_frames),
> +	ICSSG_STATS(tx_odd_nibble_frames),
> +	ICSSG_STATS(tx_underflow_errors),
> +	ICSSG_STATS(tx_frame_max_size),
> +	ICSSG_STATS(tx_max_size_error_frames),
> +	ICSSG_STATS(tx_frame_min_size),
> +	ICSSG_STATS(tx_min_size_error_frames),
> +	ICSSG_STATS(tx_bucket1_size),
> +	ICSSG_STATS(tx_bucket2_size),
> +	ICSSG_STATS(tx_bucket3_size),
> +	ICSSG_STATS(tx_bucket4_size),
> +	ICSSG_STATS(tx_64B_frames),
> +	ICSSG_STATS(tx_bucket1_frames),
> +	ICSSG_STATS(tx_bucket2_frames),
> +	ICSSG_STATS(tx_bucket3_frames),
> +	ICSSG_STATS(tx_bucket4_frames),
> +	ICSSG_STATS(tx_bucket5_frames),
> +	ICSSG_STATS(tx_total_bytes),

Please use standard stats:
https://docs.kernel.org/next/networking/statistics.html

And do not duplicate those stats in the ethool -S output.

> +static const char emac_ethtool_priv_flags[][ETH_GSTRING_LEN] = {
> +	"iet-frame-preemption",
> +	"iet-mac-verify",
> +};

What are these? We have a proper ethtool API for frame preemption.
-- 
pw-bot: cr

Re: [EXTERNAL] Re: [PATCH v10 2/2] net: ti: icssg-prueth: Add ICSSG ethernet driver

[Md Danish Anwar]

Hi Jakub,

On 20/07/23 10:05 am, Jakub Kicinski wrote:
> The patch is too big to review.
> 
> Please break it apart separating into individual features, targeting
> around 10 patches in the series. That will make it easier for reviewers
> to take a look at the features in which they have expertise.
> 

Sure Jakub. I will try to break this patch in multiple patches as below.

Patch 1: Introduce Firmware mapping for the driver (icss_switch_map.h)

Patch 2: Introduce mii helper APIs. (icssg_mii_rt.h and icssg_mii_cfg.h). This
patch will also introduce basic prueth and emac structures in icssg_prueth.h as
these structures will be used by the helper APIs.

Patch 3: Introduce firmware configuration and classification APIs.
(icssg_classifier.c, icssg_config.h and icssg_config.c)

Patch 4: Introduce APIs for ICSSG Queues (icssg_queues.c)

Patch 5: Introduce ICSSG Ethernet driver. (icssg_prueth.c and icssg_prueth.h)
This patch will enable the driver and basic functionality can work after this
patch. This patch will be using all the APIs introduced earlier. This patch
will also include Kconfig and Makefile changes.

Patch 6: Enable standard statistics via ndo_get_stats64

Patch 7: Introduce ethtool ops for ICSSG

Patch 8: Introduce power management support (suspend / resume APIs)

However this structure of patches will introduce some APIs earlier (in patch
2,3 and 4) which will be used later by patch 5. I hope it will be OK to
introduce APIs and macros earlier and use them later.

This restructuring will shorten all the individual patches. However patch 5
will still be a bit large as patch 5 introduces all the neccessary APIs as
driver probe / remove, ndo open / close, tx/rx etc.

Currnetly this single patch has close to 4000 insertion and is touching 12
files. After restructring patch 5 will have around 1800 insertions and will
touch only 4 files (icssg_prueth.c, icssg_prueth.h, Kconfig, Makefile). This is
still significant improvement.

Please let me know if this is OK.

Also this patch has Reviewed-By tag of Andrew. Can I carry forward his
Reviewed-By tag in all patches or do I need to drop it?

> See two things which jumped out at me immediately below:
> 
> On Wed, 19 Jul 2023 13:57:55 +0530 MD Danish Anwar wrote:
>> +	ICSSG_STATS(rx_crc_error_frames),
> 
>> +	ICSSG_STATS(rx_max_size_error_frames),
>> +	ICSSG_STATS(rx_frame_min_size),
>> +	ICSSG_STATS(rx_min_size_error_frames),
>> +	ICSSG_STATS(rx_overrun_frames),
> 
>> +	ICSSG_STATS(rx_64B_frames),
>> +	ICSSG_STATS(rx_bucket1_frames),
>> +	ICSSG_STATS(rx_bucket2_frames),
>> +	ICSSG_STATS(rx_bucket3_frames),
>> +	ICSSG_STATS(rx_bucket4_frames),
>> +	ICSSG_STATS(rx_bucket5_frames),
>> +	ICSSG_STATS(rx_total_bytes),
>> +	ICSSG_STATS(rx_tx_total_bytes),
>> +	/* Tx */
>> +	ICSSG_STATS(tx_good_frames),
>> +	ICSSG_STATS(tx_broadcast_frames),
>> +	ICSSG_STATS(tx_multicast_frames),
>> +	ICSSG_STATS(tx_odd_nibble_frames),
>> +	ICSSG_STATS(tx_underflow_errors),
>> +	ICSSG_STATS(tx_frame_max_size),
>> +	ICSSG_STATS(tx_max_size_error_frames),
>> +	ICSSG_STATS(tx_frame_min_size),
>> +	ICSSG_STATS(tx_min_size_error_frames),
>> +	ICSSG_STATS(tx_bucket1_size),
>> +	ICSSG_STATS(tx_bucket2_size),
>> +	ICSSG_STATS(tx_bucket3_size),
>> +	ICSSG_STATS(tx_bucket4_size),
>> +	ICSSG_STATS(tx_64B_frames),
>> +	ICSSG_STATS(tx_bucket1_frames),
>> +	ICSSG_STATS(tx_bucket2_frames),
>> +	ICSSG_STATS(tx_bucket3_frames),
>> +	ICSSG_STATS(tx_bucket4_frames),
>> +	ICSSG_STATS(tx_bucket5_frames),
>> +	ICSSG_STATS(tx_total_bytes),
> 
> Please use standard stats:
> https://docs.kernel.org/next/networking/statistics.html
> 

Sure. I will use standard stats in patch 6.

> And do not duplicate those stats in the ethool -S output.
> 

Sure I will make sure to not duplicate standard stats in driver specific stats
of ethtool -S output.

>> +static const char emac_ethtool_priv_flags[][ETH_GSTRING_LEN] = {
>> +	"iet-frame-preemption",
>> +	"iet-mac-verify",
>> +};
> 
> What are these? We have a proper ethtool API for frame preemption.

I will drop this.

Please let me know if this approach looks ok. I will go ahead and start working
on it. I Will send next revision at the earliest.

-- 
Thanks and Regards,
Danish.

Re: [EXTERNAL] Re: [PATCH v10 2/2] net: ti: icssg-prueth: Add ICSSG ethernet driver

[Jakub Kicinski]

On Thu, 20 Jul 2023 17:12:50 +0530 Md Danish Anwar wrote:
> Patch 1: Introduce Firmware mapping for the driver (icss_switch_map.h)
> 
> Patch 2: Introduce mii helper APIs. (icssg_mii_rt.h and icssg_mii_cfg.h). This
> patch will also introduce basic prueth and emac structures in icssg_prueth.h as
> these structures will be used by the helper APIs.
> 
> Patch 3: Introduce firmware configuration and classification APIs.
> (icssg_classifier.c, icssg_config.h and icssg_config.c)
> 
> Patch 4: Introduce APIs for ICSSG Queues (icssg_queues.c)
> 
> Patch 5: Introduce ICSSG Ethernet driver. (icssg_prueth.c and icssg_prueth.h)
> This patch will enable the driver and basic functionality can work after this
> patch. This patch will be using all the APIs introduced earlier. This patch
> will also include Kconfig and Makefile changes.
> 
> Patch 6: Enable standard statistics via ndo_get_stats64
> 
> Patch 7: Introduce ethtool ops for ICSSG
> 
> Patch 8: Introduce power management support (suspend / resume APIs)
> 
> However this structure of patches will introduce some APIs earlier (in patch
> 2,3 and 4) which will be used later by patch 5. I hope it will be OK to
> introduce APIs and macros earlier and use them later.
> 
> This restructuring will shorten all the individual patches. However patch 5
> will still be a bit large as patch 5 introduces all the neccessary APIs as
> driver probe / remove, ndo open / close, tx/rx etc.
> 
> Currnetly this single patch has close to 4000 insertion and is touching 12
> files. After restructring patch 5 will have around 1800 insertions and will
> touch only 4 files (icssg_prueth.c, icssg_prueth.h, Kconfig, Makefile). This is
> still significant improvement.
> 
> Please let me know if this is OK.

SGTM, thanks! One patch still being larger than others is a bit
inevitable.

> Also this patch has Reviewed-By tag of Andrew. Can I carry forward his
> Reviewed-By tag in all patches or do I need to drop it?

If the code is identical I reckon you can carry it.

Re: [EXTERNAL] Re: [PATCH v10 2/2] net: ti: icssg-prueth: Add ICSSG ethernet driver

[Roger Quadros]

Hi Danish,

On 20/07/2023 14:42, Md Danish Anwar wrote:
> Hi Jakub,
> 
> On 20/07/23 10:05 am, Jakub Kicinski wrote:
>> The patch is too big to review.
>>
>> Please break it apart separating into individual features, targeting
>> around 10 patches in the series. That will make it easier for reviewers
>> to take a look at the features in which they have expertise.
>>
> 
> Sure Jakub. I will try to break this patch in multiple patches as below.
> 
> Patch 1: Introduce Firmware mapping for the driver (icss_switch_map.h)
> 
> Patch 2: Introduce mii helper APIs. (icssg_mii_rt.h and icssg_mii_cfg.h). This
> patch will also introduce basic prueth and emac structures in icssg_prueth.h as
> these structures will be used by the helper APIs.
> 
> Patch 3: Introduce firmware configuration and classification APIs.
> (icssg_classifier.c, icssg_config.h and icssg_config.c)
> 
> Patch 4: Introduce APIs for ICSSG Queues (icssg_queues.c)
> 
> Patch 5: Introduce ICSSG Ethernet driver. (icssg_prueth.c and icssg_prueth.h)
> This patch will enable the driver and basic functionality can work after this
> patch. This patch will be using all the APIs introduced earlier. This patch
> will also include Kconfig and Makefile changes.

DT binding documentation patch can come here.

> 
> Patch 6: Enable standard statistics via ndo_get_stats64
> 
> Patch 7: Introduce ethtool ops for ICSSG
> 
> Patch 8: Introduce power management support (suspend / resume APIs)
> 

<snip>

-- 
cheers,
-roger

Re: [EXTERNAL] Re: [EXTERNAL] Re: [PATCH v10 2/2] net: ti: icssg-prueth: Add ICSSG ethernet driver

[Md Danish Anwar]

On 21/07/23 1:11 am, Roger Quadros wrote:
> Hi Danish,
> 
> On 20/07/2023 14:42, Md Danish Anwar wrote:
>> Hi Jakub,
>>
>> On 20/07/23 10:05 am, Jakub Kicinski wrote:
>>> The patch is too big to review.
>>>
>>> Please break it apart separating into individual features, targeting
>>> around 10 patches in the series. That will make it easier for reviewers
>>> to take a look at the features in which they have expertise.
>>>
>>
>> Sure Jakub. I will try to break this patch in multiple patches as below.
>>
>> Patch 1: Introduce Firmware mapping for the driver (icss_switch_map.h)
>>
>> Patch 2: Introduce mii helper APIs. (icssg_mii_rt.h and icssg_mii_cfg.h). This
>> patch will also introduce basic prueth and emac structures in icssg_prueth.h as
>> these structures will be used by the helper APIs.
>>
>> Patch 3: Introduce firmware configuration and classification APIs.
>> (icssg_classifier.c, icssg_config.h and icssg_config.c)
>>
>> Patch 4: Introduce APIs for ICSSG Queues (icssg_queues.c)
>>
>> Patch 5: Introduce ICSSG Ethernet driver. (icssg_prueth.c and icssg_prueth.h)
>> This patch will enable the driver and basic functionality can work after this
>> patch. This patch will be using all the APIs introduced earlier. This patch
>> will also include Kconfig and Makefile changes.
> 
> DT binding documentation patch can come here.
> 

Sure, Roger. I will add DT binding documentation patch here.

>>
>> Patch 6: Enable standard statistics via ndo_get_stats64
>>
>> Patch 7: Introduce ethtool ops for ICSSG
>>
>> Patch 8: Introduce power management support (suspend / resume APIs)
>>
> 
> <snip>
> 

-- 
Thanks and Regards,
Danish.