Shawn Guo <shawn.guo@xxxxxxxxxxxxx> writes: > On Sat, Jun 11, 2011 at 11:30:42AM +0200, Arnaud Patard wrote: >> Shawn Guo <shawn.guo@xxxxxxxxxx> writes: >> >> Hi, >> >> > The patch extends card_detect and write_protect support to get mx5 >> > family and more scenarios supported. The changes include: >> > >> > * Turn platform_data from optional to mandatory >> > * Add cd_types and wp_types into platform_data to cover more use >> > cases >> > * Remove the use of flag ESDHC_FLAG_GPIO_FOR_CD >> > * Adjust machine codes to adopt the platform_data changes >> >> Before I go and test theses patches, I'd like to get some >> clarification. From what I see, you've modified all over the place the >> code to provide a platform_data, setting wp/cd type to type "NONE", as >> if it was the default you choose. Why this default and not considerer >> the "SIGNAL" type being the default ? Is this choice the safest one when >> one doesn't know what type to choose or can it have some bad side >> effects ? > > The mx51_babbage is the only board support I'm concerned about in > this patch. For other boards, I chose to translate the NULL pdata > into "NONE" for both wp/cd types as the safest one, because I do not > have (or care to check) the board schematics telling how wp/cd are > routed on those boards. The patch ensures there is no regression > for those boards, and let people who have schematics to set up wp/cd > types later. ok. Thanks for making things clear. I see some changes for loco/imx53qsb. Do you need testers for it too or you've tested it ? > >> Also, why didn't you modify the imx*_add_sdhci_esdhc_imx() functions to >> provide the default platform_data by themselves that if the 2nd argument >> was NULL instead of modifying all theses machines files ? >> > As I said above, the wp/cd "NONE" types translated from NULL pdata > will be set up properly later by people who have schematics. You're not answering my question about moving the NULL-> "NONE" type from *all* modified machine file into the imx*add_sdhci_esdhc_imx(). If it's the default, why all machines file have to be modified to set it ? Moreover, *nothing* AFAICS is preventing to call theses functions will NULL. What will happen ? An oops ? To me, a default is the value set when nothing is set, and clearly modifying all functions call site due to having to provide the default seems imho wrong. Thanks, Arnaud -- 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
