On Wed, 2015-07-22 at 19:16 +0300, Madalin Bucur wrote:

This introduces the Freescale Data Path Acceleration Architecture
(DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan,
BMan, PAMU and FMan drivers to deliver Ethernet connectivity on
the Freescale DPAA QorIQ platforms.

trivia:

+static void __hot _dpa_tx_conf(struct net_device	*net_dev,
+			       const struct dpa_priv_s	*priv,
+			       struct dpa_percpu_priv_s	*percpu_priv,
+			       const struct qm_fd	*fd,
+			       u32			fqid)
+{

[]

+static struct dpa_bp * __cold
+dpa_priv_bp_probe(struct device *dev)

Do the __hot and __cold markings really matter?
Some of them may be questionable.

+static int __init dpa_load(void)
+{

[]

+	err = platform_driver_register(&dpa_driver);
+	if (unlikely(err < 0)) {
+		pr_err(KBUILD_MODNAME
+			": %s:%hu:%s(): platform_driver_register() = %d\n",
+			KBUILD_BASENAME ".c", __LINE__, __func__, err);
+	}
+
+	pr_debug(KBUILD_MODNAME ": %s:%s() ->\n",
+		 KBUILD_BASENAME ".c", __func__);

Perhaps these should use pr_fmt

+static void __exit dpa_unload(void)
+{
+	pr_debug(KBUILD_MODNAME ": -> %s:%s()\n",
+		 KBUILD_BASENAME ".c", __func__);

dynamic debug has __func__ available and perhaps
the function tracer might be used instead.

[]

+#define __hot

curious.

Maybe it'd be good to add a real __hot to compiler.h

+struct dpa_buffer_layout_s {
+	u16	priv_data_size;
+	bool		parse_results;
+	bool		time_stamp;
+	bool		hash_results;
+	u16	data_align;
+};

+struct dpa_fq {
+	struct qman_fq		 fq_base;
+	struct list_head	 list;
+	struct net_device	*net_dev;

some inconsistent indentation here and there

+struct dpa_bp {
+	struct bman_pool		*pool;
+	u8				bpid;
+	struct device			*dev;
+	union {
+		/* The buffer pools used for the private ports are initialized
+		 * with target_count buffers for each CPU; at runtime the
+		 * number of buffers per CPU is constantly brought back to this
+		 * level
+		 */
+		int target_count;
+		/* The configured value for the number of buffers in the pool,
+		 * used for shared port buffer pools
+		 */
+		int config_count;
+	};

Anonymous unions are relatively rare

+	struct {
+		/**

Maybe the /** style should be avoided

+		 * All egress queues to a given net device belong to one
+		 * (and the same) congestion group.
+		 */
+		struct qman_cgr cgr;
+	} cgr_data;

[]

+int dpa_stop(struct net_device *net_dev)
+{

[]

+	err = mac_dev->stop(mac_dev);
+	if (unlikely(err < 0))
+		netif_err(priv, ifdown, net_dev, "mac_dev->stop() = %d\n",
+			  err);

Some of the likely/unlikely uses may not
be useful/necessary.

+
+	for_each_port_device(i, mac_dev->port_dev) {
+		error = fm_port_disable(
+				fm_port_drv_handle(mac_dev->port_dev[i]));
+		err = error ? error : err;

		if (error)
			err = error;

is more obvious to me.