Re: ARC dw-mshc binding compat string

[Alexey Brodkin]

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