[PATCH v7 0/3] net: Add Keystone NetCP ethernet driver support

[PATCH v7 0/3] net: Add Keystone NetCP ethernet driver support

[Murali Karicheri]

v6->v7
Fixed some minor documentation error and also modified the netcp driver
to fix the set* functions to include correct le/be macros.

v5->v6
updated version after incorporating comments [6] from David Miller,
David Laight & Geert Uytterhoeven on v5. I would like get this in
for v3.19 merge window if the latest version is acceptable.

v4->v5
Sorry to spin v5 quickly but I missed few check-patch warnings which
were pointed by Joe Perches(thanks). I folded his changes [5] along with
few more check-patch warning fixes. I would like get this in for v3.18
merge window if David is happy with this version.

v3->v4
Couple of fixes in in error path as pointed [4] out by David. Rest of
the patches are unchanged from v3. 

v2->v3
Update v3 after incorporating Jamal and David Miller's comment/suggestion
from earlier versions [1] [2].  After per the discussion here [3], the
controversial custom exports have been dropped now. And for future
future offload support additions, we will plug into generic frameworks
as an when they are available.

The network coprocessor (NetCP) is a hardware accelerator that processes
Ethernet packets. NetCP has a gigabit Ethernet (GbE) subsystem with a ethernet
switch sub-module to send and receive packets. NetCP also includes a packet
accelerator (PA) module to perform packet classification operations such as
header matching, and packet modification operations such as checksum
generation. NetCP can also optionally include a Security Accelerator(SA)
capable of performing IPSec operations on ingress/egress packets.
    
Keystone SoC's also have a 10 Gigabit Ethernet Subsystem (XGbE) which
includes a 3-port Ethernet switch sub-module capable of 10Gb/s and
1Gb/s rates per Ethernet port.
    
Both GBE and XGBE network processors supported using common driver. It
is also designed to handle future variants of NetCP.

Cc: David Miller <davem@davemloft.net>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Sandeep Nair <sandeepn02@gmail.com>
Cc: Santosh Shilimkar <santosh.shilimkar@kernel.org>

Murali Karicheri (1):
  MAINTAINER: net: Add TI NETCP Ethernet driver entry

Sandeep Nair (2):
  Documentation: dt: net: Add binding doc for Keystone NetCP ethernet
    driver
  net: Add Keystone NetCP ethernet driver

 .../devicetree/bindings/net/keystone-netcp.txt     |  197 ++
 MAINTAINERS                                        |    8 +
 drivers/net/ethernet/ti/Kconfig                    |   12 +-
 drivers/net/ethernet/ti/Makefile                   |    4 +
 drivers/net/ethernet/ti/netcp.h                    |  227 ++
 drivers/net/ethernet/ti/netcp_core.c               | 2262 ++++++++++++++++++++
 drivers/net/ethernet/ti/netcp_ethss.c              | 2159 +++++++++++++++++++
 drivers/net/ethernet/ti/netcp_sgmii.c              |  130 ++
 drivers/net/ethernet/ti/netcp_xgbepcsr.c           |  501 +++++
 9 files changed, 5499 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/keystone-netcp.txt
 create mode 100644 drivers/net/ethernet/ti/netcp.h
 create mode 100644 drivers/net/ethernet/ti/netcp_core.c
 create mode 100644 drivers/net/ethernet/ti/netcp_ethss.c
 create mode 100644 drivers/net/ethernet/ti/netcp_sgmii.c
 create mode 100644 drivers/net/ethernet/ti/netcp_xgbepcsr.c

[1] https://lkml.org/lkml/2014/4/22/805
[2] https://lkml.org/lkml/2014/8/15/218
[3] https://lkml.org/lkml/2014/9/11/691
[4] https://lkml.org/lkml/2014/9/15/403
[5] https://lkml.org/lkml/2014/9/25/332
[6] https://lkml.org/lkml/2014/9/29/639

-- 
1.7.9.5

[PATCH v7 1/3] Documentation: dt: net: Add binding doc for Keystone NetCP ethernet driver

[Murali Karicheri]

From: Sandeep Nair <sandeep_n@ti.com>

The network coprocessor (NetCP) is a hardware accelerator that processes
Ethernet packets. NetCP has a gigabit Ethernet (GbE) subsystem with a ethernet
switch sub-module to send and receive packets. NetCP also includes a packet
accelerator (PA) module to perform packet classification operations such as
header matching, and packet modification operations such as checksum
generation. NetCP can also optionally include a Security Accelerator(SA)
capable of performing IPSec operations on ingress/egress packets.

Keystone SoC's also have a 10 Gigabit Ethernet Subsystem (XGbE) which
includes a 3-port Ethernet switch sub-module capable of 10Gb/s and
1Gb/s rates per Ethernet port.

NetCP Subsystem device tree layout looks something like below:

-----------------------------
  NetCP subsystem(10G or 1G)
-----------------------------
	|
	|-> NetCP Devices ->	|
	|			|-> GBE/XGBE Switch
	|			|
	|			|-> Packet Accelerator
	|			|
	|			|-> Security Accelerator
	|
	|
	|
	|-> NetCP Interfaces ->	|
				|-> Ethernet Port 0
				|
				|-> Ethernet Port 1
				|
				|-> Ethernet Port 2
				|
				|-> Ethernet Port 3

Common driver supports GBE as well XGBE network processors.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: David Miller <davem@davemloft.net>
Cc: Santosh Shilimkar <santosh.shilimkar@kernel.org>

Signed-off-by: Sandeep Nair <sandeep_n@ti.com>
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 .../devicetree/bindings/net/keystone-netcp.txt     |  197 ++++++++++++++++++++
 1 file changed, 197 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/keystone-netcp.txt

diff --git a/Documentation/devicetree/bindings/net/keystone-netcp.txt b/Documentation/devicetree/bindings/net/keystone-netcp.txt
new file mode 100644
index 0000000..f9c0771
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt
@@ -0,0 +1,197 @@
+This document describes the device tree bindings associated with the
+keystone network coprocessor(NetCP) driver support.
+
+The network coprocessor (NetCP) is a hardware accelerator that processes
+Ethernet packets. NetCP has a gigabit Ethernet (GbE) subsytem with a ethernet
+switch sub-module to send and receive packets. NetCP also includes a packet
+accelerator (PA) module to perform packet classification operations such as
+header matching, and packet modification operations such as checksum
+generation. NetCP can also optionally include a Security Accelerator (SA)
+capable of performing IPSec operations on ingress/egress packets.
+
+Keystone II SoC's also have a 10 Gigabit Ethernet Subsystem (XGbE) which
+includes a 3-port Ethernet switch sub-module capable of 10Gb/s and 1Gb/s rates
+per Ethernet port.
+
+Keystone NetCP driver has a plug-in module architecture where each of the NetCP
+sub-modules exist as a loadable kernel module which plug in to the netcp core.
+These sub-modules are represented as "netcp-devices" in the dts bindings. It is
+mandatory to have the ethernet switch sub-module for the ethernet interface to
+be operational. Any other sub-module like the PA is optional.
+
+NetCP Ethernet SubSystem Layout:
+
+-----------------------------
+  NetCP subsystem(10G or 1G)
+-----------------------------
+	|
+	|-> NetCP Devices ->	|
+	|			|-> GBE/XGBE Switch
+	|			|
+	|			|-> Packet Accelerator
+	|			|
+	|			|-> Security Accelerator
+	|
+	|
+	|
+	|-> NetCP Interfaces ->	|
+				|-> Ethernet Port 0
+				|
+				|-> Ethernet Port 1
+				|
+				|-> Ethernet Port 2
+				|
+				|-> Ethernet Port 3
+
+
+NetCP subsystem properties:
+Required properties:
+- compatible:	Should be "ti,netcp-1.0"
+- clocks:	phandle to the reference clocks for the subsystem.
+- dma-id:	Navigator packet dma instance id.
+
+Optional properties:
+- reg:		register location and the size for the following register
+		regions in the specified order.
+		- Efuse MAC address register
+- dma-coherent:	Present if dma operations are coherent
+- big-endian:	Keystone devices can be operated in a mode where the DSP is in
+		the big endian mode. In such cases enable this option. This
+		option should also be enabled if the ARM is operated in
+		big endian mode with the DSP in little endian.
+
+NetCP device properties: Device specification for NetCP sub-modules.
+1Gb/10Gb (gbe/xgbe) ethernet switch sub-module specifications.
+Required properties:
+- label:	Must be "netcp-gbe" for 1Gb & "netcp-xgbe" for 10Gb.
+- reg:		register location and the size for the following register
+		regions in the specified order.
+		- subsystem registers
+		- serdes registers
+- tx-channel:	the navigator packet dma channel name for tx.
+- tx-queue:	the navigator queue number associated with the tx dma channel.
+- interfaces:	specification for each of the switch port to be registered as a
+		network interface in the stack.
+-- slave-port:	Switch port number, 0 based numbering.
+-- link-interface:	type of link interface, supported options are
+			- mac<->mac auto negotiate mode: 0
+			- mac<->phy mode: 1
+			- mac<->mac forced mode: 2
+			- mac<->fiber mode: 3
+			- mac<->phy mode with no mdio: 4
+			- 10Gb mac<->phy mode : 10
+			- 10Gb mac<->mac forced mode : 11
+----phy-handle:	phandle to PHY device
+
+Optional properties:
+- enable-ale:	NetCP driver keeps the address learning feature in the ethernet
+		switch module disabled. This attribute is to enable the address
+		learning.
+- secondary-slave-ports:	specification for each of the switch port not be
+				registered as a network interface. NetCP driver
+				will only initialize these ports and attach PHY
+				driver to them if needed.
+
+NetCP interface properties: Interface specification for NetCP sub-modules.
+Required properties:
+- rx-channel:	the navigator packet dma channel name for rx.
+- rx-queue:	the navigator queue number associated with rx dma channel.
+- rx-pool:	specifies the number of descriptors to be used & the region-id
+		for creating the rx descriptor pool.
+- tx-pool:	specifies the number of descriptors to be used & the region-id
+		for creating the tx descriptor pool.
+- rx-queue-depth:	number of descriptors in each of the free descriptor
+			queue (FDQ) for the pktdma Rx flow. There can be at
+			present a maximum of 4 queues per Rx flow.
+- rx-buffer-size:	the buffer size for each of the Rx flow FDQ.
+- tx-completion-queue:	the navigator queue number where the descriptors are
+			recycled after Tx DMA completion.
+
+Optional properties:
+- efuse-mac:	If this is 1, then the MAC address for the interface is
+		obtained from the device efuse mac address register
+- local-mac-address:	the driver is designed to use the of_get_mac_address api
+			only if efuse-mac is 0. When efuse-mac is 0, the MAC
+			address is obtained from local-mac-address. If this
+			attribute is not present, then the driver will use a
+			random MAC address.
+- "netcp-device label":	phandle to the device specification for each of NetCP
+			sub-module attached to this interface.
+
+Example binding:
+
+netcp: netcp at 2090000 {
+	reg = <0x2620110 0x8>;
+	reg-names = "efuse";
+	compatible = "ti,netcp-1.0";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+
+	clocks = <&papllclk>, <&clkcpgmac>, <&chipclk12>;
+	dma-coherent;
+	/* big-endian; */
+	dma-id = <0>;
+
+	netcp-devices {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+		gbe at 0x2090000 {
+			label = "netcp-gbe";
+			reg = <0x2090000 0xf00>;
+			/* enable-ale; */
+			tx-queue = <648>;
+			tx-channel = <8>;
+
+			interfaces {
+				gbe0: interface-0 {
+					slave-port = <0>;
+					link-interface	= <4>;
+				};
+				gbe1: interface-1 {
+					slave-port = <1>;
+					link-interface	= <4>;
+				};
+			};
+
+			secondary-slave-ports {
+				port-2 {
+					slave-port = <2>;
+					link-interface	= <2>;
+				};
+				port-3 {
+					slave-port = <3>;
+					link-interface	= <2>;
+				};
+			};
+		};
+	};
+
+	netcp-interfaces {
+		interface-0 {
+			rx-channel = <22>;
+			rx-pool = <1024 12>;
+			tx-pool = <1024 12>;
+			rx-queue-depth = <128 128 0 0>;
+			rx-buffer-size = <1518 4096 0 0>;
+			rx-queue = <8704>;
+			tx-completion-queue = <8706>;
+			efuse-mac = <1>;
+			netcp-gbe = <&gbe0>;
+
+		};
+		interface-1 {
+			rx-channel = <23>;
+			rx-pool = <1024 12>;
+			tx-pool = <1024 12>;
+			rx-queue-depth = <128 128 0 0>;
+			rx-buffer-size = <1518 4096 0 0>;
+			rx-queue = <8705>;
+			tx-completion-queue = <8707>;
+			efuse-mac = <0>;
+			local-mac-address = [02 18 31 7e 3e 6f];
+			netcp-gbe = <&gbe1>;
+		};
+	};
+};
-- 
1.7.9.5

[PATCH v7 3/3] MAINTAINER: net: Add TI NETCP Ethernet driver entry

[Murali Karicheri]

CC: David Miller <davem@davemloft.net>
Signed-off-by: Sandeep Nair <sandeep_n@ti.com>
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 MAINTAINERS |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ea4d005..bc876f2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9337,6 +9337,14 @@ F:	drivers/power/lp8788-charger.c
 F:	drivers/regulator/lp8788-*.c
 F:	include/linux/mfd/lp8788*.h
 
+TI NETCP ETHERNET DRIVER
+M:	Sandeep Nair <sandeepn02@gmail.com>
+M:	Murali Karicheri <m-karicheri2@ti.com>
+M:	Santosh Shilimkar <santosh.shilimkar@kernel.org>
+L:	netdev at vger.kernel.org
+S:	Maintained
+F:	drivers/net/ethernet/ti/netcp*
+
 TI TWL4030 SERIES SOC CODEC DRIVER
 M:	Peter Ujfalusi <peter.ujfalusi@ti.com>
 L:	alsa-devel at alsa-project.org (moderated for non-subscribers)
-- 
1.7.9.5

[PATCH v7 2/3] net: Add Keystone NetCP ethernet driver

[David Miller]

From: Murali Karicheri <m-karicheri2@ti.com>
Date: Tue, 2 Dec 2014 16:41:42 -0500

> +static void set_pkt_info_le(u32 buff, u32 buff_len, u32 ndesc,
> +			    struct knav_dma_desc *desc)
> +{
> +	desc->buff_len = cpu_to_le32(buff_len);
> +	desc->buff = cpu_to_le32(buff);
> +	desc->next_desc = cpu_to_le32(ndesc);
> +}

The members of knav_dma_desc are "u32", so you are going to get tons of
static checker warnings from trying to assign cpu_to_le32()'s result
(which is a le32) into them.

[PATCH v7 2/3] net: Add Keystone NetCP ethernet driver

[Murali Karicheri]

On 12/09/2014 01:10 PM, David Miller wrote:
> From: Murali Karicheri<m-karicheri2@ti.com>
> Date: Tue, 2 Dec 2014 16:41:42 -0500
>
>> +static void set_pkt_info_le(u32 buff, u32 buff_len, u32 ndesc,
>> +			    struct knav_dma_desc *desc)
>> +{
>> +	desc->buff_len = cpu_to_le32(buff_len);
>> +	desc->buff = cpu_to_le32(buff);
>> +	desc->next_desc = cpu_to_le32(ndesc);
>> +}
>
> The members of knav_dma_desc are "u32", so you are going to get tons of
> static checker warnings from trying to assign cpu_to_le32()'s result
> (which is a le32) into them.
David,

Thanks for your comment.

Are you referring to the static code analyser sparse that is invoked 
through?

make C=1 CHECK=sparse

I am running it now.

I started a build of v3.18-rc7 with this option and seeing a lot of
warnings. Am I using the right tool? A snapshot of the output is below

   CHECK   drivers/clk/clk.c
include/linux/err.h:40:16: warning: testing a 'safe expression'
include/linux/err.h:40:16: warning: testing a 'safe expression'
include/linux/err.h:40:16: warning: testing a 'safe expression'
include/linux/err.h:40:16: warning: testing a 'safe expression'
drivers/clk/clk.c:2263:5: warning: symbol '__clk_get' was not declared. 
Should it be static?
drivers/clk/clk.c:2274:6: warning: symbol '__clk_put' was not declared. 
Should it be static?
drivers/clk/clk.c:2647:37: warning: incorrect type in assignment 
(different modifiers)
drivers/clk/clk.c:2647:37:    expected void ( *[usertype] clk_init_cb )( 
... )
drivers/clk/clk.c:2647:37:    got void const *const data
drivers/clk/clk.c:67:22: warning: context imbalance in 'clk_enable_lock' 
- wrong count at exit
drivers/clk/clk.c:92:9: warning: context imbalance in 
'clk_enable_unlock' - unexpected unlock
   CC      drivers/clk/clk.o
   CHECK   drivers/clk/clk-divider.c
drivers/clk/clk-divider.c:362:9: warning: context imbalance in 
'clk_divider_set_rate' - different lock contexts for basic block
   CC      drivers/clk/clk-divider.o
   CHECK   drivers/clk/clk-fixed-factor.c
   CC      drivers/clk/clk-fixed-factor.o
   CHECK   drivers/clk/clk-fixed-rate.c
   CC      drivers/clk/clk-fixed-rate.o
   CHECK   drivers/clk/clk-gate.c
drivers/clk/clk-gate.c:71:9: warning: context imbalance in 
'clk_gate_endisable' - different lock contexts for basic block
   CC      drivers/clk/clk-gate.o
   CHECK   drivers/clk/clk-mux.c
drivers/clk/clk-mux.c:69:12: warning: context imbalance in 
'clk_mux_set_parent' - different lock contexts for basic block
   CC      drivers/clk/clk-mux.o
   CHECK   drivers/clk/clk-composite.c
   CC      drivers/clk/clk-composite.o
   CHECK   drivers/clk/clk-fractional-divider.c
drivers/clk/clk-fractional-divider.c:36:9: warning: context imbalance in 
'clk_fd_recalc_rate' - different lock contexts for basic block
drivers/clk/clk-fractional-divider.c:65:12: warning: context imbalance 
in 'clk_fd_set_rate' - different lock contexts for basic block
   CC      drivers/clk/clk-fractional-divider.o
   CHECK   drivers/clk/clk-gpio-gate.c
drivers/clk/clk-gpio-gate.c:189:13: warning: symbol 
'of_gpio_gate_clk_setup' was not declared. Should it be static?
   CC      drivers/clk/clk-gpio-gate.o
   CHECK   drivers/clk/clk-conf.c
   CC      drivers/clk/clk-conf.o
   CHECK   drivers/clk/keystone/pll.c
   CC      drivers/clk/keystone/pll.o
   CHECK   drivers/clk/keystone/gate.c
drivers/clk/keystone/gate.c:118:12: warning: context imbalance in 
'keystone_clk_enable' - different lock contexts for basic block
drivers/clk/keystone/gate.c:148:9: warning: context imbalance in 
'keystone_clk_disable' - different lock contexts for basic block
   CC      drivers/clk/keystone/gate.o
   LD      drivers/clk/keystone/built-in.o
   LD      drivers/clk/built-in.o
   CHECK   drivers/clocksource/clksrc-of.c
drivers/clocksource/clksrc-of.c:37:27: warning: incorrect type in 
assignment (different modifiers)
drivers/clocksource/clksrc-of.c:37:27:    expected void ( *[usertype] 
init_func )( ... )
drivers/clocksource/clksrc-of.c:37:27:    got void const *const data
   CC      drivers/clocksource/clksrc-of.o
   CHECK   drivers/clocksource/mmio.c
   CC      drivers/clocksource/mmio.o
   CHECK   drivers/clocksource/arm_arch_timer.c
   CC      drivers/clocksource/arm_arch_timer.o
   CHECK   drivers/clocksource/dummy_timer.c
   CC      drivers/clocksource/dummy_timer.o
   CHECK   drivers/clocksource/timer-keystone.c
   CC      drivers/clocksource/timer-keystone.o
   LD      drivers/clocksource/built-in.o
   LD      drivers/crypto/built-in.o
   CHECK   drivers/dma/dmaengine.c
   CC      drivers/dma/dmaengine.o
   CHECK   drivers/dma/virt-dma.c
drivers/dma/virt-dma.c:22:14: warning: symbol 'vchan_tx_submit' was not 
declared. Should it be static?
   CC      drivers/dma/virt-dma.o
   CHECK   drivers/dma/of-dma.c
   CC      drivers/dma/of-dma.o
   CHECK   drivers/dma/edma.c
drivers/dma/edma.c:560:32: warning: symbol 'edma_prep_dma_memcpy' was 
not declared. Should it be static?
drivers/dma/edma.c:1099:6: warning: symbol 'edma_filter_fn' was not 
declared. Should it be static?





-- 
Murali Karicheri
Linux Kernel, Texas Instruments

[PATCH v7 2/3] net: Add Keystone NetCP ethernet driver

[David Miller]

From: Murali Karicheri <m-karicheri2@ti.com>
Date: Wed, 10 Dec 2014 16:31:02 -0500

> Are you referring to the static code analyser sparse that is invoked
> through?

You have to explicitly enable endian checking, it's not on by
default.

default enable sparse __CHECK_ENDIAN__ (was: Re: [PATCH v7 2/3] net: Add Keystone NetCP ethernet driver)

[Joe Perches]

On Wed, 2014-12-10 at 20:41 -0500, David Miller wrote:
> From: Murali Karicheri <m-karicheri2@ti.com>
> > Are you referring to the static code analyser sparse that is invoked
> > through?
> You have to explicitly enable endian checking, it's not on by
> default.

There don't seem to be thousands of warnings anymore.

Maybe it's time to default enable it when using C=?

from: Documentation/sparse.txt:

The optional make variable CF can be used to pass arguments to sparse.  The
build system passes -Wbitwise to sparse automatically.  To perform endianness
checks, you may define __CHECK_ENDIAN__:

        make C=2 CF="-D__CHECK_ENDIAN__"

These checks are disabled by default as they generate a host of warnings.

default enable sparse __CHECK_ENDIAN__ (was: Re: [PATCH v7 2/3] net: Add Keystone NetCP ethernet driver)

[Marcel Holtmann]

Hi Joe,

>>> Are you referring to the static code analyser sparse that is invoked
>>> through?
>> You have to explicitly enable endian checking, it's not on by
>> default.
> 
> There don't seem to be thousands of warnings anymore.
> 
> Maybe it's time to default enable it when using C=?
> 
> from: Documentation/sparse.txt:
> 
> The optional make variable CF can be used to pass arguments to sparse.  The
> build system passes -Wbitwise to sparse automatically.  To perform endianness
> checks, you may define __CHECK_ENDIAN__:
> 
>        make C=2 CF="-D__CHECK_ENDIAN__"
> 
> These checks are disabled by default as they generate a host of warnings.

actually a few subsystems use this in their Makefile:

	subdir-ccflags-y += -D__CHECK_ENDIAN__

We could start with that to enable endian checks by default in various places.

Regards

Marcel

[PATCH v7 2/3] net: Add Keystone NetCP ethernet driver

[Murali Karicheri]

On 12/10/2014 08:41 PM, David Miller wrote:
> From: Murali Karicheri<m-karicheri2@ti.com>
> Date: Wed, 10 Dec 2014 16:31:02 -0500
>
>> Are you referring to the static code analyser sparse that is invoked
>> through?
>
> You have to explicitly enable endian checking, it's not on by
> default.
Thanks David and others who responded. Let me do this and resolve any 
warning before the next post.

BTW, could you provide any suggestions that would help us merge this 
series to upstream? This has been sitting on this list for a while now.

Thanks and regards,

-- 
Murali Karicheri
Linux Kernel, Texas Instruments

[PATCH v7 2/3] net: Add Keystone NetCP ethernet driver

[David Miller]

From: Murali Karicheri <m-karicheri2@ti.com>
Date: Thu, 11 Dec 2014 09:14:37 -0500

> BTW, could you provide any suggestions that would help us merge this
> series to upstream? This has been sitting on this list for a while
> now.

You simply have to continue going through the review and revision
process until people no longer find problems with your changes.

This could take several more weeks, you simply must be patient.

[PATCH v7 2/3] net: Add Keystone NetCP ethernet driver

[Murali Karicheri]

On 12/11/2014 12:01 PM, David Miller wrote:
> From: Murali Karicheri<m-karicheri2@ti.com>
> Date: Thu, 11 Dec 2014 09:14:37 -0500
>
>> BTW, could you provide any suggestions that would help us merge this
>> series to upstream? This has been sitting on this list for a while
>> now.
>
> You simply have to continue going through the review and revision
> process until people no longer find problems with your changes.
>
> This could take several more weeks, you simply must be patient.

Ok. Thanks. That is encouraging to hear!

Regards,
-- 
Murali Karicheri
Linux Kernel, Texas Instruments

[PATCH v7 2/3] net: Add Keystone NetCP ethernet driver

[Joe Perches]

On Thu, 2014-12-11 at 12:17 -0500, Murali Karicheri wrote:
> On 12/11/2014 12:01 PM, David Miller wrote:
> > From: Murali Karicheri<m-karicheri2@ti.com>
> > Date: Thu, 11 Dec 2014 09:14:37 -0500
> >
> >> BTW, could you provide any suggestions that would help us merge this
> >> series to upstream? This has been sitting on this list for a while
> >> now.
> >
> > You simply have to continue going through the review and revision
> > process until people no longer find problems with your changes.
> >
> > This could take several more weeks, you simply must be patient.
> 
> Ok. Thanks. That is encouraging to hear!

Perhaps these trivial things might be considered

(uncompiled)
---
 drivers/net/ethernet/ti/netcp_core.c     |  2 +-
 drivers/net/ethernet/ti/netcp_ethss.c    | 83 +++++++++++++++++---------------
 drivers/net/ethernet/ti/netcp_xgbepcsr.c | 48 +++++++++---------
 3 files changed, 71 insertions(+), 62 deletions(-)

diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index 60ad299..8f38fe8 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -1094,7 +1094,7 @@ static void netcp_tx_notify(void *arg)
 	napi_schedule(&netcp->tx_napi);
 }
 
-static struct knav_dma_desc*
+static struct knav_dma_desc *
 netcp_tx_map_skb(struct sk_buff *skb, struct netcp_intf *netcp)
 {
 	struct knav_dma_desc *desc, *ndesc, *pdesc;
diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index 036b886..3757957 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -486,21 +486,29 @@ struct netcp_ethtool_stat {
 	int offset;
 };
 
-#define GBE_STATSA_INFO(field)		"GBE_A:"#field, GBE_STATSA_MODULE,\
-				FIELD_SIZEOF(struct gbe_hw_stats, field), \
-				offsetof(struct gbe_hw_stats, field)
-
-#define GBE_STATSB_INFO(field)		"GBE_B:"#field, GBE_STATSB_MODULE,\
-				FIELD_SIZEOF(struct gbe_hw_stats, field), \
-				offsetof(struct gbe_hw_stats, field)
-
-#define GBE_STATSC_INFO(field)		"GBE_C:"#field, GBE_STATSC_MODULE,\
-				FIELD_SIZEOF(struct gbe_hw_stats, field), \
-				offsetof(struct gbe_hw_stats, field)
-
-#define GBE_STATSD_INFO(field)		"GBE_D:"#field, GBE_STATSD_MODULE,\
-				FIELD_SIZEOF(struct gbe_hw_stats, field), \
-				offsetof(struct gbe_hw_stats, field)
+#define GBE_STATSA_INFO(field)						\
+	.desc = "GBE_A:"#field,						\
+	.type = GBE_STATSA_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)
+
+#define GBE_STATSB_INFO(field)						\
+	.desc = "GBE_B:"#field,						\
+	.type = GBE_STATSB_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)
+
+#define GBE_STATSC_INFO(field)						\
+	.desc = "GBE_C:"#field,						\
+	.type = GBE_STATSC_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)
+
+#define GBE_STATSD_INFO(field)						\
+	.desc = "GBE_D:"#field,						\
+	.type = GBE_STATSD_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)
 
 static const struct netcp_ethtool_stat gbe13_et_stats[] = {
 	/* GBE module A */
@@ -645,17 +653,23 @@ static const struct netcp_ethtool_stat gbe13_et_stats[] = {
 	{GBE_STATSD_INFO(rx_dma_overruns)},
 };
 
-#define XGBE_STATS0_INFO(field)	"GBE_0:"#field, XGBE_STATS0_MODULE, \
-				FIELD_SIZEOF(struct xgbe_hw_stats, field), \
-				offsetof(struct xgbe_hw_stats, field)
+#define XGBE_STATS0_INFO(field)						\
+	.desc = "GBE_0:"#field,						\
+	.type = GBE_STATS0_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)
 
-#define XGBE_STATS1_INFO(field)	"GBE_1:"#field, XGBE_STATS1_MODULE, \
-				FIELD_SIZEOF(struct xgbe_hw_stats, field), \
-				offsetof(struct xgbe_hw_stats, field)
+#define XGBE_STATS1_INFO(field)						\
+	.desc = "GBE_1:"#field,						\
+	.type = GBE_STATS1_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)
 
-#define XGBE_STATS2_INFO(field)	"GBE_2:"#field, XGBE_STATS2_MODULE, \
-				FIELD_SIZEOF(struct xgbe_hw_stats, field), \
-				offsetof(struct xgbe_hw_stats, field)
+#define XGBE_STATS2_INFO(field)
+	.desc = "GBE_2:"#field,						\
+	.type = GBE_STATS2_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)
 
 static const struct netcp_ethtool_stat xgbe10_et_stats[] = {
 	/* GBE module 0 */
@@ -883,11 +897,11 @@ static void gbe_update_stats_ver14(struct gbe_priv *gbe_dev, uint64_t *data)
 			case GBE_STATSA_MODULE:
 			case GBE_STATSC_MODULE:
 				base = gbe_statsa;
-			break;
+				break;
 			case GBE_STATSB_MODULE:
 			case GBE_STATSD_MODULE:
 				base  = gbe_statsb;
-			break;
+				break;
 			}
 
 			p = base + gbe_dev->et_stats[j].offset;
@@ -1639,11 +1653,8 @@ static void init_secondary_ports(struct gbe_priv *gbe_dev,
 
 	for_each_child_of_node(node, port) {
 		slave = devm_kzalloc(dev, sizeof(*slave), GFP_KERNEL);
-		if (!slave) {
-			dev_err(dev, "memomry alloc failed for secondary port(%s), skipping...\n",
-				port->name);
+		if (!slave)
 			continue;
-		}
 
 		if (init_slave(gbe_dev, slave, port)) {
 			dev_err(dev, "Failed to initialize secondary port(%s), skipping...\n",
@@ -1763,10 +1774,8 @@ static int set_xgbe_ethss10_priv(struct gbe_priv *gbe_dev,
 					  XGBE10_NUM_STAT_ENTRIES *
 					  XGBE10_NUM_SLAVES * sizeof(u64),
 					  GFP_KERNEL);
-	if (!gbe_dev->hw_stats) {
-		dev_err(gbe_dev->dev, "hw_stats memory allocation failed\n");
+	if (!gbe_dev->hw_stats)
 		return -ENOMEM;
-	}
 
 	gbe_dev->ss_version = XGBE_SS_VERSION_10;
 	gbe_dev->sgmii_port_regs = gbe_dev->ss_regs +
@@ -1836,10 +1845,8 @@ static int set_gbe_ethss14_priv(struct gbe_priv *gbe_dev,
 					  GBE13_NUM_HW_STAT_ENTRIES *
 					  GBE13_NUM_SLAVES * sizeof(u64),
 					  GFP_KERNEL);
-	if (!gbe_dev->hw_stats) {
-		dev_err(gbe_dev->dev, "hw_stats memory allocation failed\n");
+	if (!gbe_dev->hw_stats)
 		return -ENOMEM;
-	}
 
 	gbe_dev->ss_version = GBE_SS_VERSION_14;
 	gbe_dev->sgmii_port_regs = regs + GBE13_SGMII_MODULE_OFFSET;
@@ -1995,10 +2002,10 @@ static int gbe_probe(struct netcp_device *netcp_device, struct device *dev,
 		dev_err(gbe_dev->dev, "error initializing ale engine\n");
 		ret = -ENODEV;
 		goto quit;
-	} else {
-		dev_dbg(gbe_dev->dev, "Created a gbe ale engine\n");
 	}
 
+	dev_dbg(gbe_dev->dev, "Created a gbe ale engine\n");
+
 	/* initialize host port */
 	gbe_init_host_port(gbe_dev);
 
diff --git a/drivers/net/ethernet/ti/netcp_xgbepcsr.c b/drivers/net/ethernet/ti/netcp_xgbepcsr.c
index 33571ac..d93a6a4 100644
--- a/drivers/net/ethernet/ti/netcp_xgbepcsr.c
+++ b/drivers/net/ethernet/ti/netcp_xgbepcsr.c
@@ -14,6 +14,9 @@
  * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  */
+
+#define pr_fmt(fmt) "XGBE: " fmt
+
 #include "netcp.h"
 
 /* XGBE registers */
@@ -43,7 +46,7 @@ struct serdes_cfg {
 	u32 mask;
 };
 
-static struct serdes_cfg cfg_phyb_1p25g_156p25mhz_cmu0[] = {
+static const struct serdes_cfg cfg_phyb_1p25g_156p25mhz_cmu0[] = {
 	{0x0000, 0x00800002, 0x00ff00ff},
 	{0x0014, 0x00003838, 0x0000ffff},
 	{0x0060, 0x1c44e438, 0xffffffff},
@@ -54,7 +57,7 @@ static struct serdes_cfg cfg_phyb_1p25g_156p25mhz_cmu0[] = {
 	{0x0000, 0x00000003, 0x000000ff},
 };
 
-static struct serdes_cfg cfg_phyb_10p3125g_156p25mhz_cmu1[] = {
+static const struct serdes_cfg cfg_phyb_10p3125g_156p25mhz_cmu1[] = {
 	{0x0c00, 0x00030002, 0x00ff00ff},
 	{0x0c14, 0x00005252, 0x0000ffff},
 	{0x0c28, 0x80000000, 0xff000000},
@@ -76,7 +79,7 @@ static struct serdes_cfg cfg_phyb_10p3125g_156p25mhz_cmu1[] = {
 	{0x0c00, 0x00000003, 0x000000ff},
 };
 
-static struct serdes_cfg cfg_phyb_10p3125g_16bit_lane[] = {
+static const struct serdes_cfg cfg_phyb_10p3125g_16bit_lane[] = {
 	{0x0204, 0x00000080, 0x000000ff},
 	{0x0208, 0x0000920d, 0x0000ffff},
 	{0x0204, 0xfc000000, 0xff000000},
@@ -106,7 +109,7 @@ static struct serdes_cfg cfg_phyb_10p3125g_16bit_lane[] = {
 	{0x03cc, 0x00000000, 0x000000ff},
 };
 
-static struct serdes_cfg cfg_phyb_10p3125g_comlane[] = {
+static const struct serdes_cfg cfg_phyb_10p3125g_comlane[] = {
 	{0x0a00, 0x00000800, 0x0000ff00},
 	{0x0a84, 0x00000000, 0x000000ff},
 	{0x0a8c, 0x00130000, 0x00ff0000},
@@ -124,7 +127,7 @@ static struct serdes_cfg cfg_phyb_10p3125g_comlane[] = {
 	{0x0ac0, 0x0000008b, 0x000000ff},
 };
 
-static struct serdes_cfg cfg_cm_c1_c2[] = {
+static const struct serdes_cfg cfg_cm_c1_c2[] = {
 	{0x0208, 0x00000000, 0x00000f00},
 	{0x0208, 0x00000000, 0x0000001f},
 	{0x0204, 0x00000000, 0x00040000},
@@ -185,8 +188,8 @@ static void netcp_xgbe_serdes_com_enable(void __iomem *serdes_regs)
 	}
 }
 
-static void netcp_xgbe_serdes_lane_enable(
-			void __iomem *serdes_regs, int lane)
+static void netcp_xgbe_serdes_lane_enable(void __iomem *serdes_regs,
+					  int lane)
 {
 	/* Set Lane Control Rate */
 	writel(0xe0e9e038, serdes_regs + 0x1fe0 + (4 * lane));
@@ -230,7 +233,7 @@ static int netcp_xgbe_wait_pll_locked(void __iomem *sw_regs)
 		cpu_relax();
 	} while (true);
 
-	pr_err("XGBE serdes not locked: time out.\n");
+	pr_err("serdes not locked: time out\n");
 	return ret;
 }
 
@@ -292,8 +295,7 @@ static void netcp_xgbe_serdes_reset_cdr(void __iomem *serdes_regs,
 	u32 tmp, dlpf, tbus;
 
 	/*Get the DLPF values */
-	tmp = netcp_xgbe_serdes_read_select_tbus(
-			serdes_regs, lane + 1, 5);
+	tmp = netcp_xgbe_serdes_read_select_tbus(serdes_regs, lane + 1, 5);
 
 	dlpf = tmp >> 2;
 
@@ -302,10 +304,10 @@ static void netcp_xgbe_serdes_reset_cdr(void __iomem *serdes_regs,
 		mdelay(1);
 		reg_rmw(sig_detect_reg, VAL_SH(0, 1), MASK_WID_SH(2, 1));
 	} else {
-		tbus = netcp_xgbe_serdes_read_select_tbus(serdes_regs, lane +
-							  1, 0xe);
+		tbus = netcp_xgbe_serdes_read_select_tbus(serdes_regs,
+							  lane + 1, 0xe);
 
-		pr_debug("XGBE: CDR centered, DLPF: %4d,%d,%d.\n",
+		pr_debug("CDR centered, DLPF: %4d,%d,%d\n",
 			 tmp >> 2, tmp & 3, (tbus >> 2) & 3);
 	}
 }
@@ -340,13 +342,13 @@ static int netcp_xgbe_check_link_status(void __iomem *serdes_regs,
 		case 0:
 			/* if good link lock the signal detect ON! */
 			if (!loss && blk_lock) {
-				pr_debug("XGBE PCSR Linked Lane: %d\n", i);
+				pr_debug("PCSR Linked Lane: %d\n", i);
 				reg_rmw(sig_detect_reg, VAL_SH(3, 1),
 					MASK_WID_SH(2, 1));
 				current_state[i] = 1;
 			} else if (!blk_lock) {
 				/* if no lock, then reset CDR */
-				pr_debug("XGBE PCSR Recover Lane: %d\n", i);
+				pr_debug("PCSR Recover Lane: %d\n", i);
 				netcp_xgbe_serdes_reset_cdr(serdes_regs,
 							    sig_detect_reg, i);
 			}
@@ -361,10 +363,10 @@ static int netcp_xgbe_check_link_status(void __iomem *serdes_regs,
 			break;
 
 		case 2:
-			if (blk_lock)
+			if (blk_lock) {
 				/* Nope just noise */
 				current_state[i] = 1;
-			else {
+			} else {
 				/* Lost the block lock, reset CDR if it is
 				 * not centered and go back to sync state
 				 */
@@ -375,7 +377,7 @@ static int netcp_xgbe_check_link_status(void __iomem *serdes_regs,
 			break;
 
 		default:
-			pr_err("XGBE: unknown current_state[%d] %d\n",
+			pr_err("unknown current_state[%d] %d\n",
 			       i, current_state[i]);
 			break;
 		}
@@ -417,19 +419,19 @@ static int netcp_xgbe_serdes_check_lane(void __iomem *serdes_regs,
 			break;
 
 		if (lane_down[0])
-			pr_debug("XGBE: detected link down on lane 0\n");
+			pr_debug("detected link down on lane 0\n");
 
 		if (lane_down[1])
-			pr_debug("XGBE: detected link down on lane 1\n");
+			pr_debug("detected link down on lane 1\n");
 
 		if (++retries > 1) {
-			pr_debug("XGBE: timeout waiting for serdes link up\n");
+			pr_debug("timeout waiting for serdes link up\n");
 			return -ETIMEDOUT;
 		}
 		mdelay(100);
 	} while (!link_up);
 
-	pr_debug("XGBE: PCSR link is up\n");
+	pr_debug("PCSR link is up\n");
 	return 0;
 }
 
@@ -494,7 +496,7 @@ int netcp_xgbe_serdes_init(void __iomem *serdes_regs, void __iomem *xgbe_regs)
 	/* read COMLANE bits 4:0 */
 	val = readl(serdes_regs + 0xa00);
 	if (val & 0x1f) {
-		pr_debug("XGBE: serdes already in operation - reset\n");
+		pr_debug("serdes already in operation - reset\n");
 		netcp_xgbe_reset_serdes(serdes_regs);
 	}
 	return netcp_xgbe_serdes_config(serdes_regs, xgbe_regs);

[PATCH v7 2/3] net: Add Keystone NetCP ethernet driver

[Murali Karicheri]

On 12/11/2014 01:34 PM, Joe Perches wrote:
> On Thu, 2014-12-11 at 12:17 -0500, Murali Karicheri wrote:
>> On 12/11/2014 12:01 PM, David Miller wrote:
>>> From: Murali Karicheri<m-karicheri2@ti.com>
>>> Date: Thu, 11 Dec 2014 09:14:37 -0500
>>>
>>>> BTW, could you provide any suggestions that would help us merge this
>>>> series to upstream? This has been sitting on this list for a while
>>>> now.
>>>
>>> You simply have to continue going through the review and revision
>>> process until people no longer find problems with your changes.
>>>
>>> This could take several more weeks, you simply must be patient.
>>
>> Ok. Thanks. That is encouraging to hear!
>
> Perhaps these trivial things might be considered
Joe, David,

I will address the comments and repost a new version.

Murali
>
> (uncompiled)
> ---
>   drivers/net/ethernet/ti/netcp_core.c     |  2 +-
>   drivers/net/ethernet/ti/netcp_ethss.c    | 83 +++++++++++++++++---------------
>   drivers/net/ethernet/ti/netcp_xgbepcsr.c | 48 +++++++++---------
>   3 files changed, 71 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
> index 60ad299..8f38fe8 100644
> --- a/drivers/net/ethernet/ti/netcp_core.c
> +++ b/drivers/net/ethernet/ti/netcp_core.c
> @@ -1094,7 +1094,7 @@ static void netcp_tx_notify(void *arg)
>   	napi_schedule(&netcp->tx_napi);
>   }
>
> -static struct knav_dma_desc*
> +static struct knav_dma_desc *
>   netcp_tx_map_skb(struct sk_buff *skb, struct netcp_intf *netcp)
>   {
>   	struct knav_dma_desc *desc, *ndesc, *pdesc;
> diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
> index 036b886..3757957 100644
> --- a/drivers/net/ethernet/ti/netcp_ethss.c
> +++ b/drivers/net/ethernet/ti/netcp_ethss.c
> @@ -486,21 +486,29 @@ struct netcp_ethtool_stat {
>   	int offset;
>   };
>
> -#define GBE_STATSA_INFO(field)		"GBE_A:"#field, GBE_STATSA_MODULE,\
> -				FIELD_SIZEOF(struct gbe_hw_stats, field), \
> -				offsetof(struct gbe_hw_stats, field)
> -
> -#define GBE_STATSB_INFO(field)		"GBE_B:"#field, GBE_STATSB_MODULE,\
> -				FIELD_SIZEOF(struct gbe_hw_stats, field), \
> -				offsetof(struct gbe_hw_stats, field)
> -
> -#define GBE_STATSC_INFO(field)		"GBE_C:"#field, GBE_STATSC_MODULE,\
> -				FIELD_SIZEOF(struct gbe_hw_stats, field), \
> -				offsetof(struct gbe_hw_stats, field)
> -
> -#define GBE_STATSD_INFO(field)		"GBE_D:"#field, GBE_STATSD_MODULE,\
> -				FIELD_SIZEOF(struct gbe_hw_stats, field), \
> -				offsetof(struct gbe_hw_stats, field)
> +#define GBE_STATSA_INFO(field)						\
> +	.desc = "GBE_A:"#field,						\
> +	.type = GBE_STATSA_MODULE,					\
> +	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
> +	.offset = offsetof(struct gbe_hw_stats, field)
> +
> +#define GBE_STATSB_INFO(field)						\
> +	.desc = "GBE_B:"#field,						\
> +	.type = GBE_STATSB_MODULE,					\
> +	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
> +	.offset = offsetof(struct gbe_hw_stats, field)
> +
> +#define GBE_STATSC_INFO(field)						\
> +	.desc = "GBE_C:"#field,						\
> +	.type = GBE_STATSC_MODULE,					\
> +	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
> +	.offset = offsetof(struct gbe_hw_stats, field)
> +
> +#define GBE_STATSD_INFO(field)						\
> +	.desc = "GBE_D:"#field,						\
> +	.type = GBE_STATSD_MODULE,					\
> +	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
> +	.offset = offsetof(struct gbe_hw_stats, field)
>
>   static const struct netcp_ethtool_stat gbe13_et_stats[] = {
>   	/* GBE module A */
> @@ -645,17 +653,23 @@ static const struct netcp_ethtool_stat gbe13_et_stats[] = {
>   	{GBE_STATSD_INFO(rx_dma_overruns)},
>   };
>
> -#define XGBE_STATS0_INFO(field)	"GBE_0:"#field, XGBE_STATS0_MODULE, \
> -				FIELD_SIZEOF(struct xgbe_hw_stats, field), \
> -				offsetof(struct xgbe_hw_stats, field)
> +#define XGBE_STATS0_INFO(field)						\
> +	.desc = "GBE_0:"#field,						\
> +	.type = GBE_STATS0_MODULE,					\
> +	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
> +	.offset = offsetof(struct gbe_hw_stats, field)
>
> -#define XGBE_STATS1_INFO(field)	"GBE_1:"#field, XGBE_STATS1_MODULE, \
> -				FIELD_SIZEOF(struct xgbe_hw_stats, field), \
> -				offsetof(struct xgbe_hw_stats, field)
> +#define XGBE_STATS1_INFO(field)						\
> +	.desc = "GBE_1:"#field,						\
> +	.type = GBE_STATS1_MODULE,					\
> +	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
> +	.offset = offsetof(struct gbe_hw_stats, field)
>
> -#define XGBE_STATS2_INFO(field)	"GBE_2:"#field, XGBE_STATS2_MODULE, \
> -				FIELD_SIZEOF(struct xgbe_hw_stats, field), \
> -				offsetof(struct xgbe_hw_stats, field)
> +#define XGBE_STATS2_INFO(field)
> +	.desc = "GBE_2:"#field,						\
> +	.type = GBE_STATS2_MODULE,					\
> +	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
> +	.offset = offsetof(struct gbe_hw_stats, field)
>
>   static const struct netcp_ethtool_stat xgbe10_et_stats[] = {
>   	/* GBE module 0 */
> @@ -883,11 +897,11 @@ static void gbe_update_stats_ver14(struct gbe_priv *gbe_dev, uint64_t *data)
>   			case GBE_STATSA_MODULE:
>   			case GBE_STATSC_MODULE:
>   				base = gbe_statsa;
> -			break;
> +				break;
>   			case GBE_STATSB_MODULE:
>   			case GBE_STATSD_MODULE:
>   				base  = gbe_statsb;
> -			break;
> +				break;
>   			}
>
>   			p = base + gbe_dev->et_stats[j].offset;
> @@ -1639,11 +1653,8 @@ static void init_secondary_ports(struct gbe_priv *gbe_dev,
>
>   	for_each_child_of_node(node, port) {
>   		slave = devm_kzalloc(dev, sizeof(*slave), GFP_KERNEL);
> -		if (!slave) {
> -			dev_err(dev, "memomry alloc failed for secondary port(%s), skipping...\n",
> -				port->name);
> +		if (!slave)
>   			continue;
> -		}
>
>   		if (init_slave(gbe_dev, slave, port)) {
>   			dev_err(dev, "Failed to initialize secondary port(%s), skipping...\n",
> @@ -1763,10 +1774,8 @@ static int set_xgbe_ethss10_priv(struct gbe_priv *gbe_dev,
>   					  XGBE10_NUM_STAT_ENTRIES *
>   					  XGBE10_NUM_SLAVES * sizeof(u64),
>   					  GFP_KERNEL);
> -	if (!gbe_dev->hw_stats) {
> -		dev_err(gbe_dev->dev, "hw_stats memory allocation failed\n");
> +	if (!gbe_dev->hw_stats)
>   		return -ENOMEM;
> -	}
>
>   	gbe_dev->ss_version = XGBE_SS_VERSION_10;
>   	gbe_dev->sgmii_port_regs = gbe_dev->ss_regs +
> @@ -1836,10 +1845,8 @@ static int set_gbe_ethss14_priv(struct gbe_priv *gbe_dev,
>   					  GBE13_NUM_HW_STAT_ENTRIES *
>   					  GBE13_NUM_SLAVES * sizeof(u64),
>   					  GFP_KERNEL);
> -	if (!gbe_dev->hw_stats) {
> -		dev_err(gbe_dev->dev, "hw_stats memory allocation failed\n");
> +	if (!gbe_dev->hw_stats)
>   		return -ENOMEM;
> -	}
>
>   	gbe_dev->ss_version = GBE_SS_VERSION_14;
>   	gbe_dev->sgmii_port_regs = regs + GBE13_SGMII_MODULE_OFFSET;
> @@ -1995,10 +2002,10 @@ static int gbe_probe(struct netcp_device *netcp_device, struct device *dev,
>   		dev_err(gbe_dev->dev, "error initializing ale engine\n");
>   		ret = -ENODEV;
>   		goto quit;
> -	} else {
> -		dev_dbg(gbe_dev->dev, "Created a gbe ale engine\n");
>   	}
>
> +	dev_dbg(gbe_dev->dev, "Created a gbe ale engine\n");
> +
>   	/* initialize host port */
>   	gbe_init_host_port(gbe_dev);
>
> diff --git a/drivers/net/ethernet/ti/netcp_xgbepcsr.c b/drivers/net/ethernet/ti/netcp_xgbepcsr.c
> index 33571ac..d93a6a4 100644
> --- a/drivers/net/ethernet/ti/netcp_xgbepcsr.c
> +++ b/drivers/net/ethernet/ti/netcp_xgbepcsr.c
> @@ -14,6 +14,9 @@
>    * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>    * GNU General Public License for more details.
>    */
> +
> +#define pr_fmt(fmt) "XGBE: " fmt
> +
>   #include "netcp.h"
>
>   /* XGBE registers */
> @@ -43,7 +46,7 @@ struct serdes_cfg {
>   	u32 mask;
>   };
>
> -static struct serdes_cfg cfg_phyb_1p25g_156p25mhz_cmu0[] = {
> +static const struct serdes_cfg cfg_phyb_1p25g_156p25mhz_cmu0[] = {
>   	{0x0000, 0x00800002, 0x00ff00ff},
>   	{0x0014, 0x00003838, 0x0000ffff},
>   	{0x0060, 0x1c44e438, 0xffffffff},
> @@ -54,7 +57,7 @@ static struct serdes_cfg cfg_phyb_1p25g_156p25mhz_cmu0[] = {
>   	{0x0000, 0x00000003, 0x000000ff},
>   };
>
> -static struct serdes_cfg cfg_phyb_10p3125g_156p25mhz_cmu1[] = {
> +static const struct serdes_cfg cfg_phyb_10p3125g_156p25mhz_cmu1[] = {
>   	{0x0c00, 0x00030002, 0x00ff00ff},
>   	{0x0c14, 0x00005252, 0x0000ffff},
>   	{0x0c28, 0x80000000, 0xff000000},
> @@ -76,7 +79,7 @@ static struct serdes_cfg cfg_phyb_10p3125g_156p25mhz_cmu1[] = {
>   	{0x0c00, 0x00000003, 0x000000ff},
>   };
>
> -static struct serdes_cfg cfg_phyb_10p3125g_16bit_lane[] = {
> +static const struct serdes_cfg cfg_phyb_10p3125g_16bit_lane[] = {
>   	{0x0204, 0x00000080, 0x000000ff},
>   	{0x0208, 0x0000920d, 0x0000ffff},
>   	{0x0204, 0xfc000000, 0xff000000},
> @@ -106,7 +109,7 @@ static struct serdes_cfg cfg_phyb_10p3125g_16bit_lane[] = {
>   	{0x03cc, 0x00000000, 0x000000ff},
>   };
>
> -static struct serdes_cfg cfg_phyb_10p3125g_comlane[] = {
> +static const struct serdes_cfg cfg_phyb_10p3125g_comlane[] = {
>   	{0x0a00, 0x00000800, 0x0000ff00},
>   	{0x0a84, 0x00000000, 0x000000ff},
>   	{0x0a8c, 0x00130000, 0x00ff0000},
> @@ -124,7 +127,7 @@ static struct serdes_cfg cfg_phyb_10p3125g_comlane[] = {
>   	{0x0ac0, 0x0000008b, 0x000000ff},
>   };
>
> -static struct serdes_cfg cfg_cm_c1_c2[] = {
> +static const struct serdes_cfg cfg_cm_c1_c2[] = {
>   	{0x0208, 0x00000000, 0x00000f00},
>   	{0x0208, 0x00000000, 0x0000001f},
>   	{0x0204, 0x00000000, 0x00040000},
> @@ -185,8 +188,8 @@ static void netcp_xgbe_serdes_com_enable(void __iomem *serdes_regs)
>   	}
>   }
>
> -static void netcp_xgbe_serdes_lane_enable(
> -			void __iomem *serdes_regs, int lane)
> +static void netcp_xgbe_serdes_lane_enable(void __iomem *serdes_regs,
> +					  int lane)
>   {
>   	/* Set Lane Control Rate */
>   	writel(0xe0e9e038, serdes_regs + 0x1fe0 + (4 * lane));
> @@ -230,7 +233,7 @@ static int netcp_xgbe_wait_pll_locked(void __iomem *sw_regs)
>   		cpu_relax();
>   	} while (true);
>
> -	pr_err("XGBE serdes not locked: time out.\n");
> +	pr_err("serdes not locked: time out\n");
>   	return ret;
>   }
>
> @@ -292,8 +295,7 @@ static void netcp_xgbe_serdes_reset_cdr(void __iomem *serdes_regs,
>   	u32 tmp, dlpf, tbus;
>
>   	/*Get the DLPF values */
> -	tmp = netcp_xgbe_serdes_read_select_tbus(
> -			serdes_regs, lane + 1, 5);
> +	tmp = netcp_xgbe_serdes_read_select_tbus(serdes_regs, lane + 1, 5);
>
>   	dlpf = tmp>>  2;
>
> @@ -302,10 +304,10 @@ static void netcp_xgbe_serdes_reset_cdr(void __iomem *serdes_regs,
>   		mdelay(1);
>   		reg_rmw(sig_detect_reg, VAL_SH(0, 1), MASK_WID_SH(2, 1));
>   	} else {
> -		tbus = netcp_xgbe_serdes_read_select_tbus(serdes_regs, lane +
> -							  1, 0xe);
> +		tbus = netcp_xgbe_serdes_read_select_tbus(serdes_regs,
> +							  lane + 1, 0xe);
>
> -		pr_debug("XGBE: CDR centered, DLPF: %4d,%d,%d.\n",
> +		pr_debug("CDR centered, DLPF: %4d,%d,%d\n",
>   			 tmp>>  2, tmp&  3, (tbus>>  2)&  3);
>   	}
>   }
> @@ -340,13 +342,13 @@ static int netcp_xgbe_check_link_status(void __iomem *serdes_regs,
>   		case 0:
>   			/* if good link lock the signal detect ON! */
>   			if (!loss&&  blk_lock) {
> -				pr_debug("XGBE PCSR Linked Lane: %d\n", i);
> +				pr_debug("PCSR Linked Lane: %d\n", i);
>   				reg_rmw(sig_detect_reg, VAL_SH(3, 1),
>   					MASK_WID_SH(2, 1));
>   				current_state[i] = 1;
>   			} else if (!blk_lock) {
>   				/* if no lock, then reset CDR */
> -				pr_debug("XGBE PCSR Recover Lane: %d\n", i);
> +				pr_debug("PCSR Recover Lane: %d\n", i);
>   				netcp_xgbe_serdes_reset_cdr(serdes_regs,
>   							    sig_detect_reg, i);
>   			}
> @@ -361,10 +363,10 @@ static int netcp_xgbe_check_link_status(void __iomem *serdes_regs,
>   			break;
>
>   		case 2:
> -			if (blk_lock)
> +			if (blk_lock) {
>   				/* Nope just noise */
>   				current_state[i] = 1;
> -			else {
> +			} else {
>   				/* Lost the block lock, reset CDR if it is
>   				 * not centered and go back to sync state
>   				 */
> @@ -375,7 +377,7 @@ static int netcp_xgbe_check_link_status(void __iomem *serdes_regs,
>   			break;
>
>   		default:
> -			pr_err("XGBE: unknown current_state[%d] %d\n",
> +			pr_err("unknown current_state[%d] %d\n",
>   			       i, current_state[i]);
>   			break;
>   		}
> @@ -417,19 +419,19 @@ static int netcp_xgbe_serdes_check_lane(void __iomem *serdes_regs,
>   			break;
>
>   		if (lane_down[0])
> -			pr_debug("XGBE: detected link down on lane 0\n");
> +			pr_debug("detected link down on lane 0\n");
>
>   		if (lane_down[1])
> -			pr_debug("XGBE: detected link down on lane 1\n");
> +			pr_debug("detected link down on lane 1\n");
>
>   		if (++retries>  1) {
> -			pr_debug("XGBE: timeout waiting for serdes link up\n");
> +			pr_debug("timeout waiting for serdes link up\n");
>   			return -ETIMEDOUT;
>   		}
>   		mdelay(100);
>   	} while (!link_up);
>
> -	pr_debug("XGBE: PCSR link is up\n");
> +	pr_debug("PCSR link is up\n");
>   	return 0;
>   }
>
> @@ -494,7 +496,7 @@ int netcp_xgbe_serdes_init(void __iomem *serdes_regs, void __iomem *xgbe_regs)
>   	/* read COMLANE bits 4:0 */
>   	val = readl(serdes_regs + 0xa00);
>   	if (val&  0x1f) {
> -		pr_debug("XGBE: serdes already in operation - reset\n");
> +		pr_debug("serdes already in operation - reset\n");
>   		netcp_xgbe_reset_serdes(serdes_regs);
>   	}
>   	return netcp_xgbe_serdes_config(serdes_regs, xgbe_regs);
>
>
>


-- 
Murali Karicheri
Linux Kernel, Texas Instruments