[U-Boot] [PATCH v4 1/1] ARM: kirkwood: add mvsdio driver

Stefan Roese stefan.roese at gmail.com
Fri Jul 25 09:15:42 CEST 2014


Hi Gerald,

On 24.07.2014 21:07, Gerald Kerma wrote:
> Signed-off-by: Gerald Kerma <drEagle at doukki.net>
> ---
>   Changes in v4:
>   - rename drivers files to MVSDIO
>   - fix MMC clock init which now use dev ID
>   - clean debug strings
>   - remove MVSDIO_TWEAK_NOSDHS quirk
>   - remove dead code

Thanks. But still some more comments below.

>   Changes in v3:
>   - Add MVSDIO_TWEAK_NOSDHS quirk
>   - Minor clean
>
>   Changes in v2:
>   - Fix some typo and missing lines from patch import
>
>   arch/arm/cpu/arm926ejs/kirkwood/cpu.c          |  58 ++++
>   arch/arm/include/asm/arch-kirkwood/config.h    |   2 +
>   arch/arm/include/asm/arch-kirkwood/cpu.h       |   2 +
>   arch/arm/include/asm/arch-kirkwood/kirkwood.h  |  24 ++
>   arch/arm/include/asm/arch-kirkwood/kw88f6282.h |  33 +++
>   board/Marvell/openrd/openrd.c                  |  11 +
>   board/Marvell/sheevaplug/sheevaplug.c          |  11 +
>   drivers/mmc/Makefile                           |   1 +
>   drivers/mmc/mvsdio.c                           | 384 +++++++++++++++++++++++++
>   include/configs/openrd.h                       |   8 +
>   include/configs/sheevaplug.h                   |  19 +-
>   include/mvsdio.h                               | 279 ++++++++++++++++++
>   12 files changed, 830 insertions(+), 2 deletions(-)
>   create mode 100644 arch/arm/include/asm/arch-kirkwood/kw88f6282.h
>   create mode 100644 drivers/mmc/mvsdio.c
>   create mode 100644 include/mvsdio.h
>
> diff --git a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c
> index da80240..472aa8a 100644
> --- a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c
> +++ b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c
> @@ -333,6 +333,64 @@ int arch_cpu_init(void)
>   }
>   #endif /* CONFIG_ARCH_CPU_INIT */
>
> +/*****************************************************************************
> + * General
> + ****************************************************************************/
> +#if defined(CONFIG_ARCH_DEV_ID)
> +
> +void kirkwood_pcie_id(u32 *dev, u32 *rev)
> +{
> +	*dev = (readl(KW_REG_PCIE_DEVID) >> 16) & 0xffff;
> +	*rev = readl(KW_REG_PCIE_REVID) & 0xff;
> +}
> +
> +/*
> + * Identify device ID and revision.
> + */
> +char *kirkwood_id(void)
> +{
> +	u32 dev, rev;
> +
> +	kirkwood_pcie_id(&dev, &rev);
> +
> +	if (dev == MV88F6281_DEV_ID) {
> +		if (rev == MV88F6281_REV_Z0)
> +			return "MV88F6281-Z0";
> +		else if (rev == MV88F6281_REV_A0)
> +			return "MV88F6281-A0";
> +		else if (rev == MV88F6281_REV_A1)
> +			return "MV88F6281-A1";
> +		else
> +			return "MV88F6281-Rev-Unsupported";
> +	} else if (dev == MV88F6192_DEV_ID) {
> +		if (rev == MV88F6192_REV_Z0)
> +			return "MV88F6192-Z0";
> +		else if (rev == MV88F6192_REV_A0)
> +			return "MV88F6192-A0";
> +		else if (rev == MV88F6192_REV_A1)
> +			return "MV88F6192-A1";
> +		else
> +			return "MV88F6192-Rev-Unsupported";
> +	} else if (dev == MV88F6180_DEV_ID) {
> +		if (rev == MV88F6180_REV_A0)
> +			return "MV88F6180-Rev-A0";
> +		else if (rev == MV88F6180_REV_A1)
> +			return "MV88F6180-Rev-A1";
> +		else
> +			return "MV88F6180-Rev-Unsupported";
> +	} else if (dev == MV88F6282_DEV_ID) {
> +		if (rev == MV88F6282_REV_A0)
> +			return "MV88F6282-Rev-A0";
> +		else if (rev == MV88F6282_REV_A1)
> +			return "MV88F6282-Rev-A1";
> +		else
> +			return "MV88F6282-Rev-Unsupported";
> +	} else {
> +		return "Device-Unknown";
> +	}
> +}
> +#endif /* CONFIG_ARCH_DEV_ID */

I still don't see why this function is needed for the MMC driver. It 
seems to only be used from a debug() output. I suggest to remove it from 
this patch to not touch the general kirkwood stuff.

Also this MMC driver should be able to support multiple Marvells SoC's, 
right? So calling some "kirkwood" functions doesn't seem to be generic.

> +
>   /*
>    * SOC specific misc init
>    */
> diff --git a/arch/arm/include/asm/arch-kirkwood/config.h b/arch/arm/include/asm/arch-kirkwood/config.h
> index 7a688e4..a0563a3 100644
> --- a/arch/arm/include/asm/arch-kirkwood/config.h
> +++ b/arch/arm/include/asm/arch-kirkwood/config.h
> @@ -19,6 +19,8 @@
>   #include <asm/arch/kw88f6281.h>
>   #elif defined (CONFIG_KW88F6192)
>   #include <asm/arch/kw88f6192.h>
> +#elif defined(CONFIG_KW88F6182)
> +#include <asm/arch/kw88f6182.h>
>   #else
>   #error "SOC Name not defined"
>   #endif /* CONFIG_KW88F6281 */
> diff --git a/arch/arm/include/asm/arch-kirkwood/cpu.h b/arch/arm/include/asm/arch-kirkwood/cpu.h
> index 97daa40..e7b6448 100644
> --- a/arch/arm/include/asm/arch-kirkwood/cpu.h
> +++ b/arch/arm/include/asm/arch-kirkwood/cpu.h
> @@ -151,5 +151,7 @@ int kw_config_mpp(unsigned int mpp0_7, unsigned int mpp8_15,
>   		unsigned int mpp32_39, unsigned int mpp40_47,
>   		unsigned int mpp48_55);
>   unsigned int kw_winctrl_calcsize(unsigned int sizeval);
> +void kirkwood_pcie_id(u32 *dev, u32 *rev);
> +char *kirkwood_id(void);
>   #endif /* __ASSEMBLY__ */
>   #endif /* _KWCPU_H */
> diff --git a/arch/arm/include/asm/arch-kirkwood/kirkwood.h b/arch/arm/include/asm/arch-kirkwood/kirkwood.h
> index bc207f5..489517f 100644
> --- a/arch/arm/include/asm/arch-kirkwood/kirkwood.h
> +++ b/arch/arm/include/asm/arch-kirkwood/kirkwood.h
> @@ -39,6 +39,7 @@
>   #define KW_EGIGA0_BASE			(KW_REGISTER(0x72000))
>   #define KW_EGIGA1_BASE			(KW_REGISTER(0x76000))
>   #define KW_SATA_BASE			(KW_REGISTER(0x80000))
> +#define KW_SDIO_BASE			(KW_REGISTER(0x90000))
>
>   /* Kirkwood Sata controller has two ports */
>   #define KW_SATA_PORT0_OFFSET		0x2000
> @@ -61,10 +62,33 @@
>   #define MVCPU_WIN_ENABLE	KWCPU_WIN_ENABLE
>   #define MVCPU_WIN_DISABLE	KWCPU_WIN_DISABLE
>
> +/*
> + * Supported devices and revisions.
> + */
> +#define MV88F6281_DEV_ID	0x6281
> +#define MV88F6281_REV_Z0	0
> +#define MV88F6281_REV_A0	2
> +#define MV88F6281_REV_A1	3
> +
> +#define MV88F6192_DEV_ID	0x6192
> +#define MV88F6192_REV_Z0	0
> +#define MV88F6192_REV_A0	2
> +#define MV88F6192_REV_A1	3
> +
> +#define MV88F6180_DEV_ID	0x6180
> +#define MV88F6180_REV_A0	2
> +#define MV88F6180_REV_A1	3
> +
> +#define MV88F6282_DEV_ID	0x6282
> +#define MV88F6282_REV_A0	0
> +#define MV88F6282_REV_A1	1
> +
>   #if defined (CONFIG_KW88F6281)
>   #include <asm/arch/kw88f6281.h>
>   #elif defined (CONFIG_KW88F6192)
>   #include <asm/arch/kw88f6192.h>
> +#elif defined(CONFIG_KW88F6182)
> +#include <asm/arch/kw88f6182.h>
>   #else
>   #error "SOC Name not defined"
>   #endif /* CONFIG_KW88F6281 */
> diff --git a/arch/arm/include/asm/arch-kirkwood/kw88f6282.h b/arch/arm/include/asm/arch-kirkwood/kw88f6282.h
> new file mode 100644
> index 0000000..5310da2
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-kirkwood/kw88f6282.h
> @@ -0,0 +1,33 @@
> +/*
> + * (C) Copyright 2009
> + * Marvell Semiconductor <www.marvell.com>
> + * Written-by: Prafulla Wadaskar <prafulla at marvell.com>
> + *
> + * Header file for Feroceon CPU core 88FR131 Based KW88F6281 SOC.
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */

Again, please use the SPDX license identifier as done in all U-Boot 
sources now. This is mandatory!

BTW: You seem to be adding a new Kirkwood SoC version with this MMC 
patch. This really is not MMC related and should be split into a 
separate patch.

Thanks,
Stefan



More information about the U-Boot mailing list