Hi Andy, Thanks for pointing all. We will fix it. Regards, Prabu On 5/28/2018 5:49 PM, Andy Shevchenko wrote: > On Tue, May 22, 2018 at 9:42 AM, Prabu Thangamuthu <Prabu.T@xxxxxxxxxxxx> wrote: >> To enable Synopsys DWC MSHC controller on HPAS-DX platform connected using >> PCIe interface. As Clock generation logic is implemented in MMCM module of >> HAPS-DX platform, we have separate functions to control the MMCM to >> generate required clocks with respect to speed mode. Also we have platform >> specific set_power function to support different VDD of eMMC devices. >> + * Authors: >> + * Prabu Thangamuthu <prabu.t@xxxxxxxxxxxx> >> + * Manjunath M B <manjumb@xxxxxxxxxxxx> >> + * > Redundant last line in above excerpt. > >> +#include <linux/pci.h> >> +#include <linux/delay.h> >> +#include <linux/module.h> >> +#include <linux/moduleparam.h> > This one is effectively included by module.h. > >> +/* Default emmc vdd is set to 3.3V */ >> +static unsigned int emmc_vdd = SDHC_EMMC_VDD_330V; >> +module_param(emmc_vdd, int, 0444); > This looks weird. Why do you need a module parameter? > >> + u16 clk = 0; >> + u32 reg = 0; >> + u32 vendor_ptr = 0; > Why do above have assignments? > >> +MODULE_PARM_DESC(emmc_vdd, "VDD to configure eMMC device supply voltage"); > This better to be near to the parameter declaration. Though, as I said > above, it feels wrong to have it in the first place. > > >> +/* Synopsys Vendor Specific Registers */ >> +#define SDHC_GPIO_OUT 0x34 >> +#define SDHC_AT_CTRL_R 0x40 >> + >> +#define SDHC_SW_TUNE_EN 0x00000010 >> + >> +/* MMCM DRP */ >> +#define SDHC_MMCM_DIV_REG 0x1020 >> +#define DIV_REG_100_MHZ 0x1145 >> + >> +#define SDHC_MMCM_CLKFBOUT 0x1024 >> +#define CLKFBOUT_100_MHZ 0x0000 >> + >> +#define SDHC_CCLK_MMCM_RST 0x00000001 > Do you need these all in the header file? Why? > > >> +#define PCI_DEVICE_ID_SYNOPSYS_DWC_MSHC 0xC202 > Absent blank line. > Broken style of the value. > > Who knows what else wrong with this patch? Please, pay attention to > the details and check twice before submit. > -- 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