Re: [PATCH 2/2] memory: jz4780-nemc: driver for the NEMC on JZ4780 SoCs

From: Paul Bolle
Date: Thu Mar 05 2015 - 14:26:50 EST


On Thu, 2015-03-05 at 17:01 +0000, Zubair Lutfullah Kakakhel wrote:
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -83,6 +83,15 @@ config FSL_IFC
> bool
> depends on FSL_SOC
>
> +config JZ4780_NEMC
> + bool "Ingenic JZ4780 SoC NEMC driver"

A bool Kconfig symbol.

> + default y
> + depends on MACH_JZ4780
> + help
> + This driver is for the NAND/External Memory Controller (NEMC) in
> + the Ingenic JZ4780. This controller is used to handle external
> + memory devices such as NAND and SRAM.
> +
> source "drivers/memory/tegra/Kconfig"
>
> endif
> diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
> index 6b65481..b670441 100644
> --- a/drivers/memory/Makefile
> +++ b/drivers/memory/Makefile
> @@ -13,5 +13,6 @@ obj-$(CONFIG_FSL_CORENET_CF) += fsl-corenet-cf.o
> obj-$(CONFIG_FSL_IFC) += fsl_ifc.o
> obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o
> obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o
> +obj-$(CONFIG_JZ4780_NEMC) += jz4780-nemc.o

So jz4780-nemc.o will never be part of a module.

> obj-$(CONFIG_TEGRA_MC) += tegra/
> diff --git a/drivers/memory/jz4780-nemc.c b/drivers/memory/jz4780-nemc.c
> new file mode 100644
> index 0000000..370dc7b
> --- /dev/null
> +++ b/drivers/memory/jz4780-nemc.c
> @@ -0,0 +1,405 @@
> +/*
> + * JZ4780 NAND/external memory controller (NEMC)
> + *
> + * Copyright (c) 2015 Imagination Technologies
> + * Author: Alex Smith <alex@xxxxxxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */

> +#include <linux/module.h>

This is, I think, a red flag.

> +MODULE_DEVICE_TABLE(of, jz4780_nemc_dt_match);

This will be preprocessed away.

> +static struct platform_driver jz4780_nemc_driver = {
> + .probe = jz4780_nemc_probe,
> + .remove = jz4780_nemc_remove,
> + .driver = {
> + .name = "jz4780-nemc",
> + .owner = THIS_MODULE,

.owner will, basically, be NULL.

> + .of_match_table = of_match_ptr(jz4780_nemc_dt_match),
> + },
> +};
> +
> +static int __init jz4780_nemc_init(void)
> +{
> + return platform_driver_register(&jz4780_nemc_driver);
> +}
> +subsys_initcall(jz4780_nemc_init);

subsys_initcall() suggests, I guess, you don't actually expect this to
be modular.

> +static void __exit jz4780_nemc_exit(void)
> +{
> + platform_driver_unregister(&jz4780_nemc_driver);
> +}
> +module_exit(jz4780_nemc_exit);

This will never be called.

> +MODULE_AUTHOR("Alex Smith <alex@xxxxxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Ingenic JZ4780 NEMC driver");
> +MODULE_LICENSE("GPL v2");

These macros will be, effectively, preprocessed away.

And, by the way, "GPL v2" is at odds with the license in the comment
quoted above.

Thanks,


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/