Hi Ulf, On 2017/3/15 21:11, Ulf Hansson wrote: > On 14 February 2017 at 18:01, Gregory CLEMENT > <gregory.clement@xxxxxxxxxxxxxxxxxx> wrote: >> From: Hu Ziji <huziji@xxxxxxxxxxx> <snip> >> +config MMC_SDHCI_XENON >> + tristate "Marvell Xenon eMMC/SD/SDIO SDHCI driver" >> + depends on MMC_SDHCI_PLTFM >> + help >> + This selects Marvell Xenon eMMC/SD/SDIO SDHCI. >> + If you have a machine with integrated Marvell Xenon SDHC IP, > > /s/SDHC/SDHCI > Sorry. I don't get your point. Could you please give more detailed comments? >> + say Y or M here. >> + If unsure, say N. >> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile >> index ccc9c4cba154..b0a2ab4b256e 100644 >> --- a/drivers/mmc/host/Makefile >> +++ b/drivers/mmc/host/Makefile >> @@ -82,3 +82,6 @@ obj-$(CONFIG_MMC_SDHCI_BRCMSTB) += sdhci-brcmstb.o >> ifeq ($(CONFIG_CB710_DEBUG),y) >> CFLAGS-cb710-mmc += -DDEBUG >> endif >> + >> +obj-$(CONFIG_MMC_SDHCI_XENON) += sdhci-xenon-driver.o >> +sdhci-xenon-driver-y += sdhci-xenon.o > > Why not only this: > obj-$(CONFIG_MMC_SDHCI_XENON) += sdhci-xenon.o > Yes. Will try. >> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c >> new file mode 100644 <snip> > > ff --git a/drivers/mmc/host/sdhci-xenon.h b/drivers/mmc/host/sdhci-xenon.h >> new file mode 100644 >> index 000000000000..69de711db9eb >> --- /dev/null >> +++ b/drivers/mmc/host/sdhci-xenon.h > > You should probably put all this in the c-file instead. That is how > most other sdhci variants does it. > Unfortunately, we have some registers like XENON_SLOT_EMMC_CTRL which are accessed by both sdhci-xenon.c and sdhci-xenon-phy.c > [...] > > Overall this looks good to me, however I think Adrian needs to have a > quick look as well. > > One additional very minor nitpick. Perhaps you can align on the > function names prefix, as those currently varies between "whatever", > "xenon_" and "sdhci_xenon_". > Sure. Will fix them as you request. Thank you. Best regards, Hu Ziji > Kind regards > Uffe > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html