On do, 2015-07-09 at 16:21 -0400, Roy Pledge wrote:

+config FSL_DPA_PIRQ_FAST
+	bool
+	default y

First used in 04/11.

+config FSL_DPA_PIRQ_SLOW
+	bool
+	default y
+
+config FSL_DPA_PORTAL_SHARE
+	bool
+	default y

As in 02/11: these three symbols function as aliases for FSL_DPA. Why
are they needed? 

 config FSL_BMAN
 	tristate "BMan device management"
 	default n
 	help
 		FSL DPAA BMan driver
 
+config FSL_BMAN_PORTAL
+	tristate "BMan portal(s)"
+	default n
+	help
+		FSL BMan portal driver

+struct bman_portal {
+	[...]
+#ifdef CONFIG_FSL_DPA_CAN_WAIT_SYNC

This check will always evaluate to true, right? (I'll only report this
once.)

+	struct bman_pool *rcri_owned; /* only 1 release WAIT_SYNC at 
a time */
+#endif
+#ifdef CONFIG_FSL_DPA_PORTAL_SHARE

Ditto.

+	raw_spinlock_t sharing_lock; /* only used if is_shared */
+	int is_shared;
+	struct bman_portal *sharing_redirect;
+#endif
+	[...]
+};

+const struct bman_portal_config *bman_get_portal_config(void)
+{
+	[...]
+}

I couldn't find callers of this function.

+EXPORT_SYMBOL(bman_get_portal_config);

Nor users of this export, obviously.

+
+u32 bman_irqsource_get(void)
+{
+	[...]
+}

Ditto.

+EXPORT_SYMBOL(bman_irqsource_get);

Ditto.

+int bman_p_irqsource_add(struct bman_portal *p, __maybe_unused u32 bits)
+{
+	[...]
+}
+EXPORT_SYMBOL(bman_p_irqsource_add);

There seem to be no users of this export.

+int bman_irqsource_add(__maybe_unused u32 bits)
+{
+	[...]
+}

Unused.

+EXPORT_SYMBOL(bman_irqsource_add);

Ditto.

+int bman_irqsource_remove(u32 bits)
+{
+	[...]
+}

Ditto. 

+EXPORT_SYMBOL(bman_irqsource_remove);

Ditto.

+u32 bman_poll_slow(void)
+{
+	[...]
+}

Ditto.

+EXPORT_SYMBOL(bman_poll_slow);

Ditto.

+void bman_poll(void)
+{
+	[...]
+}

Ditto.

+EXPORT_SYMBOL(bman_poll);

Ditto.

+static inline struct bm_rcr_entry *try_rel_start(struct bman_portal **p,
+#ifdef CONFIG_FSL_DPA_CAN_WAIT

Always true, right?

+					__maybe_unused struct bman_pool *pool,
+#endif
+					__maybe_unused unsigned long *irqflags,
+					__maybe_unused u32 flags)

And this is a, well, novel way to declare a function.

+{
+	[...]
+}

+int bman_flush_stockpile(struct bman_pool *pool, u32 flags)
+{
+	[...]
+}

Unused function.

+EXPORT_SYMBOL(bman_flush_stockpile);

So unused export too.

+#ifdef CONFIG_FSL_BMAN
+u32 bman_query_free_buffers(struct bman_pool *pool)
+{
+	return bm_pool_free_buffers(pool->params.bpid);
+}
+EXPORT_SYMBOL(bman_query_free_buffers);
+
+int bman_update_pool_thresholds(struct bman_pool *pool, const u32 *thresholds)
+{
+	u32 bpid;
+
+	bpid = bman_get_params(pool)->bpid;
+
+	return bm_pool_set(bpid, thresholds);
+}
+EXPORT_SYMBOL(bman_update_pool_thresholds);

More of the same.

+#endif

No MODULE_LICENSE() here, nor in the other files that make up this
module. So loading this module will trigger a warning and taint the
kernel.

+EXPORT_SYMBOL(bman_alloc_bpid_range);

Unused export.

+EXPORT_SYMBOL(bman_release_bpid_range);

Ditto.

+EXPORT_SYMBOL(bman_seed_bpid_range);

Ditto.

+int bman_reserve_bpid_range(u32 bpid, u32 count)
+{
+	return dpaa_resource_reserve(&bpalloc, bpid, count);
+}
+EXPORT_SYMBOL(bman_reserve_bpid_range);

Because bman_reserve_bpid() is unused, see below, this function and this
export are unused too.

#if IS_ENABLED(CONFIG_FSL_BMAN_PORTAL)

+#ifdef DPAA_RESOURCE_DEBUG

Never defined. So DUMP() is dead code.

Please replace the CONFIG_ prefix with something else.

+#ifdef CONFIG_TRY_BETTER_MEMCPY

This will always be true, right?

[...]
+#else
+#define copy_words memcpy
+#define copy_shorts memcpy
+#define copy_bytes memcpy
+#endif

+static inline int bman_reserve_bpid(u32 bpid)
+{
+	return bman_reserve_bpid_range(bpid, 1);
+}

Unused.

Thanks,


Paul Bolle