On 03/28/2016 12:34 PM, Jaehoon Chung wrote: > Hi, Hi, [...] >>>>>>>> That said, I would rather prefer to see "snps,dw-mshc" prefix on description >>>>>>>> of an MMC controller found on SoCFPGA series, "altr,socfpga-dw-mshc" seems >>>>>>>> to be redundant. > > Yes..it's redundant..i should be combined to "snps,dw-mshc". Should the compat string be compatible = "altr,socfpga-dw-mshc", "snps,dw-mshc"; or just compatible = "snps,dw-mshc"; ? I am under the impression that a soc-specific identifier in addition to a generic one (used by the driver compat table) is a good idea, because it can help discerning the IP block from a generic one if needed at some future point in time. It will also not break the DT for systems which may depend on the non-generic compat, like *BSDs and such. What do you think ? (btw this is very much my question in this thread) >>>>>>> According to drivers/mmc/host/dw_mmc-pltfm.c , the Altera SoCFPGA one >>>>>>> "altr,socfpga-dw-mshc" and also Imagination Technology Pistacio one >>>>>>> "img,pistachio-dw-mshc" need specialty bit (SDMMC_CMD_USE_HOLD_REG), >>>>>>> while the stock one "snps,dw-mshc" does not. I am not sure if the ARC >>>>>>> one needs it as well, but most likely yes. >>>>>>> >>>>>>> I wonder if that bit is needed on some particular version of the DWMMC >>>>>>> core. In that case, should we have "snps,dw-mshc" and "snps,dw-mshc-vN" >>>>>>> binding ? Or should we use DT property to discern the need for this bit ? >>>>>>> >>>>>> That's the most common way to take into account peculiarities, add >>>>>> a property and handle it from the driver. >>>>> And by "that" you mean which of those two I listed , the >>>>> "snps,dw-mshc-vN" or adding new DT prop ? >>>>> >>>> I meant to add a new property, not a new compatible, but that's just >>>> my experience. >>>> >>>> Let me say it __might__ happen that a particular change you need is >>>> specific to a particular version of the DWMMC IP (query Synopsys >>>> by the way), but more probably it might be e.g. the same IP version with >>>> a different reduced or extended configuration or a minor fix/improvement >>>> to the IP block without resulting version number bump. >>>> >>>> For example I don't remember that errata fixes in IP blocks result in >>>> a new compatible, instead there are quite common optional "quirk" >>>> properties for broken IPs -- e.g. check bindings/usb/dwc3.txt :) >>> Right, this very much matches how I see it as well. Thanks for confirming. >>> >>> Alexey, can you tell us if the requirement for setting >>> SDMMC_CMD_USE_HOLD_REG came with some new revision of the core or >>> disappeared with some revision OR if this is some configuration >>> option of the core during synthesis ? >> >> Sorry for not following that discussion during my weekend but I'll try >> to address all questions now. > > SDMMC_CMD_USE_HOLD_REG didn't come with new revision..It's using continuously. > But it's difficult to use the generic feature..because it's considered the below things. > > If Card is SDR50/SDR104/DDR50 mode.. > 1) and phase shift of cclk_in_drv is 0 then SDMMC_CMD_USE_HOLD_REG bit is set to 0, > 2) and phase shift of cclk_in_drv > 0 then SDMMC_CMD_USE_HOLD_REG bit is set to 1, > If Card is SDR12/SDR25 mode, then this bit is set to 1. > > We need to check phase shift scheme..but as i knew, each SoC have been implemented differently for phase shift. > (Phase shift have dependency to SoC.) > > And it have to check HCON register..there is IMPLEMENT_HOLD_REG(bit[22]). > (It described whether IP have hold register or not) > > I didn't read this thread entirely. > I'm not sure what you have discussed..but my understanding is right..i recommend to use "snps,dw-mshc" for ARC compat string. > Otherwise it need to add "dw_mmc-<SoC>.c". dw_mmc-pltfm.c should provide the basic dw-mmc controller functionality. > > After read this thread entirely, i will check more detailed what you discussed. > If i missed something, let me know, plz. Thanks for the clarification, linux-next indeed contains changes which make snps,dw-mshc and altr,socfpga-dw-mshc equal. > Best Regards, > Jaehoon Chung > >> >> DW Mobile Storage databook says: >> --------------------->8----------------------- >> To meet the relatively high Input Hold Time requirement for SDR12, SDR25, >> and other MMC speed modes, you should program bit[29]use_hold_Reg of the >> CMD register to 1'b1. >> --------------------->8----------------------- >> >> So I'd say this specific setting has nothing to do with a particular IP block >> but instead it is related to card's mode of operation. More precisely bus clock. >> SDR12 stands for 12.5 MByte/s, SDR25 stands for 25 MByte/s. I.e. we probably need >> so set that bit just for certain cases and regardless board that uses DW MMC. >> >> I'm adding DW MMC maintainer as well as linux-mmc mailing list so people who >> understands that stuff better may comment here as well. >> >> -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 >> >> > -- Best regards, Marek Vasut -- 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
