Hi Jennifer, On Tue, Nov 09, 2010 at 02:28:00PM +0800, Jennifer Li (TP) wrote: > Hi, > > This can make MMC/SD driver work on O2 chip. > > Best regards, > Jennifer Thanks for the patch submission. There are some changes that need to be made before this can be accepted into the kernel -- you can read more about the patch guidelines here: http://www.kernel.org/doc/Documentation/SubmittingPatches http://www.kernel.org/doc/Documentation/CodingStyle In particular: * The patch needs a "Signed-off-by" line. * Please submit a summary that explains how the patch works in general. Your directory name mentions "SD_ADMAissue_8220_8320" -- could you explain what that issue is, and how this patch fixes it? I guess I'm particularly interested in why you have to make so many PCI config writes, and what they achieve. * It would be ideal if you could generate the patch using Git, and if it could be based on 2.6.36 or newer instead of 2.6.35. * In lines like this: ret = pci_read_config_byte(chip->pdev, 0xD3, &scratch); please use symbolic names -- the name of the register -- instead of numbers like 0xD3. For example, from via-sdmmc.c: #define VIA_CRDR_PCI_WORK_MODE 0x40 #define VIA_CRDR_PCI_DBG_MODE 0x41 ... pci_write_config_byte(pcidev, VIA_CRDR_PCI_WORK_MODE, 0); pci_write_config_byte(pcidev, VIA_CRDR_PCI_DBG_MODE, 0); * Instead of comments like "// set E0 to 0x73", please describe what's actually happening to the hardware. For example, from the jmicron code: /* * Turn PMOS on [bit 0], set over current detection to 2.4 V * [bit 1:2] and enable over current debouncing [bit 6]. */ (Linux uses "/* */" comments rather than "//" comments.) * Commented-out code shouldn't be submitted: +// .quirks = SDHCI_QUICK_ADMA_TABLE_ENTRY, * The PCI device ID symbols should only be added to include/linux/pci_ids.h. There's already a space for other O2 devices in there. * I think it's generally okay not to check the return value on pci_write_config_byte(). Please fix these and resubmit the patch. Thanks! -- Chris Ball <cjb@xxxxxxxxxx> <http://printf.net/> One Laptop Per Child -- 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
