Re: [PATCH v5 net-next,mips 6/7] netdev: octeon-ethernet: Add Cavium Octeon III support.

From: David Miller
Date: Sun Dec 03 2017 - 11:27:02 EST


From: David Daney <david.daney@xxxxxxxxxx>
Date: Fri, 1 Dec 2017 15:18:06 -0800

> +static char *mix_port;
> +module_param(mix_port, charp, 0444);
> +MODULE_PARM_DESC(mix_port, "Specifies which ports connect to MIX interfaces.");
> +
> +static char *pki_port;
> +module_param(pki_port, charp, 0444);
> +MODULE_PARM_DESC(pki_port, "Specifies which ports connect to the PKI.");

Please no module parameters.

Please instead find a way to determine or configure these elements
at run time with generic configuration interfaces.
> +
> +static int bgx_probe(struct platform_device *pdev)
> +{
> + struct mac_platform_data platform_data;
> + const __be32 *reg;
> + u32 port;
> + u64 addr;
> + struct device_node *child;
> + struct platform_device *new_dev;
> + struct platform_device *pki_dev;
> + int numa_node, interface;
> + int i;
> + int r = 0;
> + char id[64];
> + u64 data;

Please use reverse-christmas-tree ordering (longest to shortest line) for
local variable declarations.

Please fix this in your entire submission.

> +static int bgx_mix_init_from_fdt(void)
> +{
> + struct device_node *node;
> + struct device_node *parent = NULL;
> + int mix = 0;
> +

Please do not use tabs like this when declaring local variables.
Much worse, some functions use this style whereas others do not,
be consistent otherwise your code is very hard to read.

Please fix this for your entire submission not just this specific
case I am pointing out.

Thanks.