Re: [PATCH v2 1/2] MMC: add support for the Marvell Dove SDHCI controller

[Mike Rapoport <mike@compulab.co.il>]

On 10/27/10 13:56, Chris Ball wrote:
> Hi Mike, adding Wolfram to CC in case he has comments,
> 
> On Wed, Oct 27, 2010 at 01:21:58PM +0200, Mike Rapoport wrote:
>>
> 
> Let's add a little text here -- something like "Implement an sdhci-pltfm
> driver for the controller found on Marvell Dove and MMP SoCs" is fine.

Ok.

>> Signed-off-by: Mike Rapoport <mike@compulab.co.il>
>> CC: Saeed Bishara <saeed@marvell.com>
> 
>> ---
>>  drivers/mmc/host/Kconfig       |   13 +++++++
>>  drivers/mmc/host/Makefile      |    1 +
>>  drivers/mmc/host/sdhci-dove.c  |   72 ++++++++++++++++++++++++++++++++++++++++
>>  drivers/mmc/host/sdhci-pltfm.c |    3 ++
>>  drivers/mmc/host/sdhci-pltfm.h |    1 +
>>  5 files changed, 90 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/mmc/host/sdhci-dove.c
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 68d1279..4958839 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -130,6 +130,19 @@ config MMC_SDHCI_CNS3XXX
>>  
>>  	  If unsure, say N.
>>  
>> +config MMC_SDHCI_DOVE
>> +	bool "SDHCI support on Marvell's Dove SoC"
>> +	depends on ARCH_DOVE
> 
> The previous submission included ARCH_MMP, so I expect you'll want to
> resubmit with the extra dependency added once Saeed replies.

I've understood Saeed's comment that he prefers to limit this to Dove only. I'll
add MMP if it'll also work with this driver.

>> +	depends on MMC_SDHCI_PLTFM
>> +	select MMC_SDHCI_IO_ACCESSORS
>> +	help
>> +	  This selects the Secure Digital Host Controller Interface in
>> +	  Marvell's Dove SoC.
>> +
>> +	  If you have a controller with this interface, say Y or M here.
>> +
>> +	  If unsure, say N.
>> +
>>  config MMC_SDHCI_S3C
>>  	tristate "SDHCI support on Samsung S3C SoC"
>>  	depends on MMC_SDHCI && PLAT_SAMSUNG
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index 840bcb5..9e1c418 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -40,6 +40,7 @@ obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
>>  obj-$(CONFIG_MMC_SDHCI_PLTFM)			+= sdhci-platform.o
>>  sdhci-platform-y				:= sdhci-pltfm.o
>>  sdhci-platform-$(CONFIG_MMC_SDHCI_CNS3XXX)	+= sdhci-cns3xxx.o
>> +sdhci-platform-$(CONFIG_MMC_SDHCI_DOVE)		+= sdhci-dove.o
>>  
>>  obj-$(CONFIG_MMC_SDHCI_OF)	+= sdhci-of.o
>>  sdhci-of-y				:= sdhci-of-core.o
>> diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
>> new file mode 100644
>> index 0000000..ac844d1
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-dove.c
>> @@ -0,0 +1,72 @@
>> +/*
>> + * sdhci-dove.c Support for SDHCI on Marvell's Dove SoC
>> + *
>> + * Author: Saeed Bishara <saeed@marvell.com>
> 
> If Saeed is indeed the author of this code, the commit should be:
>    From: Saeed Bishara <saeed@marvell.com>
> rather than in your name.  (If it's not correct, it should be changed.)

Sure, indeed.
Saeed, shall I add your "Signed-off" as well?

>> + * Based on  sdhci-cns3xxx.c
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>> + */
>> +
>> +/* #include <linux/clk.h> */
>> +/* #include <linux/delay.h> */
> 
> Please don't submit commented out code to mainline -- if these aren't
> needed, they should just be dropped.

will fix.

> Thanks!
> 


-- 
Sincerely yours,
Mike.