Hi+AKA-Jaehoon, On Mon, 2016-03-28 at 19:34 +-0900, Jaehoon Chung wrote: +AD4- Hi, +AFs-snip+AF0- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- That said, I would rather prefer to see +ACI-snps,dw-mshc+ACI- prefix on description +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- of an MMC controller found on SoCFPGA series, +ACI-altr,socfpga-dw-mshc+ACI- seems +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- to be redundant. +AD4- Yes..it's redundant..i should be combined to +ACI-snps,dw-mshc+ACI-. So for socfpga platform compat string should be something like +ACI-snps,dw-mshc-socfpga+ACI- then? +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- According to drivers/mmc/host/dw+AF8-mmc-pltfm.c , the Altera SoCFPGA one +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +ACI-altr,socfpga-dw-mshc+ACI- and also Imagination Technology Pistacio one +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +ACI-img,pistachio-dw-mshc+ACI- need specialty bit (SDMMC+AF8-CMD+AF8-USE+AF8-HOLD+AF8-REG), +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- while the stock one +ACI-snps,dw-mshc+ACI- does not. I am not sure if the ARC +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- one needs it as well, but most likely yes. +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- I wonder if that bit is needed on some particular version of the DWMMC +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- core. In that case, should we have +ACI-snps,dw-mshc+ACI- and +ACI-snps,dw-mshc-vN+ACI- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- binding ? Or should we use DT property to discern the need for this bit ? +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- That's the most common way to take into account peculiarities, add +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- a property and handle it from the driver. +AD4- +AD4- +AD4- +AD4- +AD4- And by +ACI-that+ACI- you mean which of those two I listed , the +AD4- +AD4- +AD4- +AD4- +AD4- +ACI-snps,dw-mshc-vN+ACI- or adding new DT prop ? +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- I meant to add a new property, not a new compatible, but that's just +AD4- +AD4- +AD4- +AD4- my experience. +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- Let me say it +AF8AXw-might+AF8AXw- happen that a particular change you need is +AD4- +AD4- +AD4- +AD4- specific to a particular version of the DWMMC IP (query Synopsys +AD4- +AD4- +AD4- +AD4- by the way), but more probably it might be e.g. the same IP version with +AD4- +AD4- +AD4- +AD4- a different reduced or extended configuration or a minor fix/improvement +AD4- +AD4- +AD4- +AD4- to the IP block without resulting version number bump. +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- For example I don't remember that errata fixes in IP blocks result in +AD4- +AD4- +AD4- +AD4- a new compatible, instead there are quite common optional +ACI-quirk+ACI- +AD4- +AD4- +AD4- +AD4- properties for broken IPs -- e.g. check bindings/usb/dwc3.txt :) +AD4- +AD4- +AD4- Right, this very much matches how I see it as well. Thanks for confirming. +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- Alexey, can you tell us if the requirement for setting +AD4- +AD4- +AD4- SDMMC+AF8-CMD+AF8-USE+AF8-HOLD+AF8-REG came with some new revision of the core or +AD4- +AD4- +AD4- disappeared with some revision OR if this is some configuration +AD4- +AD4- +AD4- option of the core during synthesis ? +AD4- +AD4- Sorry for not following that discussion during my weekend but I'll try +AD4- +AD4- to address all questions now. +AD4- SDMMC+AF8-CMD+AF8-USE+AF8-HOLD+AF8-REG didn't come with new revision..It's using continuously. +AD4- But it's difficult to use the generic feature..because it's considered the below things. +AD4- +AD4- If Card is SDR50/SDR104/DDR50 mode.. +AD4- 1) and phase shift of cclk+AF8-in+AF8-drv is 0 then SDMMC+AF8-CMD+AF8-USE+AF8-HOLD+AF8-REG bit is set to 0, +AD4- 2) and phase shift of cclk+AF8-in+AF8-drv +AD4- 0 then SDMMC+AF8-CMD+AF8-USE+AF8-HOLD+AF8-REG bit is set to 1, +AD4- If Card is SDR12/SDR25 mode, then this bit is set to 1. So card type is also important here and for certain card type we don't need to set+AKA-SDMMC+AF8-CMD+AF8-USE+AF8-HOLD+AF8-REG, right? +AD4- We need to check phase shift scheme..but as i knew, each SoC have been implemented differently for phase shift. +AD4- (Phase shift have dependency to SoC.) Given my assumption above we need to check 2 things: +AKAAKg- Card type +AKAAKg- SoC-specific implementation detail (phase shift scheme) +AD4- And it have to check HCON register..there is IMPLEMENT+AF8-HOLD+AF8-REG(bit+AFs-22+AF0-). +AD4- (It described whether IP have hold register or not) Ah actually 3 things +AKAAKwCg-IMPLEMENT+AF8-HOLD+AF8-REG +AD4- I didn't read this thread entirely. +AD4- I'm not sure what you have discussed..but my understanding is right..i recommend to use +ACI-snps,dw-mshc+ACI- for ARC compat +AD4- string. +AD4- Otherwise it need to add +ACI-dw+AF8-mmc-+ADw-SoC+AD4-.c+ACI-. dw+AF8-mmc-pltfm.c should provide the basic dw-mmc controller functionality. Hm, interesting looks like you already made some changes here: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id+AD0-aaaaeb7a933471f6413ca44dd36efd57f2fa9429 So now driver checks if SoC has HOLD REG then SDMMC+AF8-CMD+AF8-USE+AF8-HOLD+AF8-REG will be set (regardless card type). And what's interesting and connected to this discussion since mentioned commit there's no point in having both+AKAAIg-altr,socfpga-dw-mshc+ACI- and+AKAAIg-img,pistachio-dw-mshc+ACI- compat strings because the do nothing now. I.e. it's time to replace both mentioned compat strings with generic+AKAAIg-snps,dw-mshc+ACI-. Anybody volunteers for that .dts+ACo- cleanup? -Alexey-- 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
