linux-kernel - Re: [PATCH v12 3/9] mmc: cavium: Add MMC platform driver for Octeon SOCs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 17 Mar 2017 14:35:34 +0100
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Jan Glauber <jglauber@...ium.com>
Cc:     "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        David Daney <ddaney@...iumnetworks.com>,
        "Steven J . Hill" <Steven.Hill@...ium.com>,
        David Daney <david.daney@...ium.com>
Subject: Re: [PATCH v12 3/9] mmc: cavium: Add MMC platform driver for Octeon SOCs

On 10 March 2017 at 14:25, Jan Glauber <jglauber@...ium.com> wrote:
> Add a platform driver for Octeon MIPS SOCs.
>
> Signed-off-by: Jan Glauber <jglauber@...ium.com>
> Signed-off-by: David Daney <david.daney@...ium.com>
> Signed-off-by: Steven J. Hill <steven.hill@...ium.com>
> ---
>  drivers/mmc/host/Kconfig               |  10 ++
>  drivers/mmc/host/Makefile              |   2 +
>  drivers/mmc/host/cavium-pltfm-octeon.c | 183 +++++++++++++++++++++++++++++++++
>  3 files changed, 195 insertions(+)
>  create mode 100644 drivers/mmc/host/cavium-pltfm-octeon.c

Please rename the file to cavium-octeon.c

>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index f08691a..68cc811 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -622,6 +622,16 @@ config SDH_BFIN_MISSING_CMD_PULLUP_WORKAROUND
>         help
>           If you say yes here SD-Cards may work on the EZkit.
>
> +config MMC_CAVIUM_OCTEON
> +       tristate "Cavium OCTEON SD/MMC Card Interface support"
> +       depends on CAVIUM_OCTEON_SOC
> +       help
> +         This selects Cavium OCTEON SD/MMC card Interface.
> +         If you have an OCTEON board with a Multimedia Card slot,
> +         say Y or M here.
> +
> +         If unsure, say N.
> +

Adding more new cavium variants becomes a bit messy in this approach.
May I suggest something similar we are using for SDHCI mmc driver.
That is:

For the core mmc cavium driver:
config MMC_CAVIUM (to build cavium-mmc.o)
I would also appreciate to rename that file to cavium.c
This also means you need to export the functions you provide from the
header cavium-mmc.h (rename to cavium.h)

For the octeon variant:
config MMC_CAVIUM_OCTEON (to build cavium-octeon.o)
depends on MMC_CAVIUM && CAVIUM_OCTEON_SOC

>  config MMC_DW
>         tristate "Synopsys DesignWare Memory Card Interface"
>         depends on HAS_DMA
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 6d548c4..c7f0ccf 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -42,6 +42,8 @@ obj-$(CONFIG_MMC_SDHI)                += sh_mobile_sdhi.o
>  obj-$(CONFIG_MMC_CB710)                += cb710-mmc.o
>  obj-$(CONFIG_MMC_VIA_SDMMC)    += via-sdmmc.o
>  obj-$(CONFIG_SDH_BFIN)         += bfin_sdh.o
> +octeon-mmc-objs := cavium-mmc.o cavium-pltfm-octeon.o
> +obj-$(CONFIG_MMC_CAVIUM_OCTEON) += octeon-mmc.o

By changing according to above, we get two build objects instead of
two. Nice an clean.

>  obj-$(CONFIG_MMC_DW)           += dw_mmc.o
>  obj-$(CONFIG_MMC_DW_PLTFM)     += dw_mmc-pltfm.o
>  obj-$(CONFIG_MMC_DW_EXYNOS)    += dw_mmc-exynos.o
> diff --git a/drivers/mmc/host/cavium-pltfm-octeon.c b/drivers/mmc/host/cavium-pltfm-octeon.c
> new file mode 100644
> index 0000000..e83d143

[...]

Changing the name of the files, may also lead to that you perhaps want
to change the prefix of the functions.

Otherwise this looks good to me.

Kind regards
Uffe