Messages in this thread | | | Subject | Re: Adding a .platform_init callback to sdhci_arasan_ops | From | Sebastian Frias <> | Date | Tue, 6 Dec 2016 14:42:21 +0100 |
| |
Hi,
On 05/12/16 17:30, Doug Anderson wrote: <snip> > > AKA: you are setting up various "corecfg" stuff that's documented in > the generic Arasan docs. Others SDHCI-Arasan implementations might > want to set the same things, but those values may be stored elsewhere > for them.
Exactly, that is what I'm trying to find out. To me, one good place to store this would be DT.
> > So if _all_ Arasan implementations need these same values or need the > same logic to figure out these values, then you should do something > that's not chip-specific but something generic. >
It depends on where this needs to be set, and why am I the first to need to set this up.
> If you've got a specific weird quirk that's specific to your platform, > then you could do that in a chip-specific init.
Yes, or in the set_ios you talked earlier.
> > Presumably many of the above could just be hardcoded on some > implementations, so they might not be available in a memory-mapped > implementation... >
Exactly.
> >> which seems much easier to handle (and portable). >> >> At any rate, one thing to note from this is that many of these >> bits should probably be initialised based on DT, right? > > Probably, or by proving the voltage value of regulations. Note that I > think DT already gets parsed and sets up caps. I'm not really an > expert here and I'd let someone who actually knows / maintains SDMMC > comment. I know for sure that dw_mmc (which I'm way more familiar > with) does things very differently than sdhci (which I'm barely > familiar with).
Thanks. Could somebody else comment please?
Best regards,
Sebastian
> > >> For example, the DT has a "non-removable" property, which I think >> should be used to setup SLOT_TYPE_EMBEDDED (if the property is >> present) or SLOT_TYPE_REMOVABLE (if the property is not present) >> >> Looking at Documentation/devicetree/bindings/mmc/mmc.txt we can see >> more related properties: >> >> Optional properties: >> - bus-width: Number of data lines, can be <1>, <4>, or <8>. The default >> will be <1> if the property is absent. >> - wp-gpios: Specify GPIOs for write protection, see gpio binding >> - cd-inverted: when present, polarity on the CD line is inverted. See the note >> below for the case, when a GPIO is used for the CD line >> - wp-inverted: when present, polarity on the WP line is inverted. See the note >> below for the case, when a GPIO is used for the WP line >> - disable-wp: When set no physical WP line is present. This property should >> only be specified when the controller has a dedicated write-protect >> detection logic. If a GPIO is always used for the write-protect detection >> logic it is sufficient to not specify wp-gpios property in the absence of a WP >> line. >> - max-frequency: maximum operating clock frequency >> - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on >> this system, even if the controller claims it is. >> - cap-sd-highspeed: SD high-speed timing is supported >> - cap-mmc-highspeed: MMC high-speed timing is supported >> - sd-uhs-sdr12: SD UHS SDR12 speed is supported >> - sd-uhs-sdr25: SD UHS SDR25 speed is supported >> - sd-uhs-sdr50: SD UHS SDR50 speed is supported >> - sd-uhs-sdr104: SD UHS SDR104 speed is supported >> - sd-uhs-ddr50: SD UHS DDR50 speed is supported >> ... >> >> which makes me wonder, what is the recommended way of dealing with this? >> - Should I use properties on the DT? If so, then I need to add code to set >> up the register properly. >> - Should I hardcode these values use a minimal DT? If so, then I need an >> init function to put all this. >> - Should I hardcode stuff at u-Boot level? If so, nothing is required and >> should work without any modifications to the Arasan Linux driver. >> >> It appears that the Linux driver is expecting most of these fields to be >> hardcoded and "pre-set" before (maybe by the bootloader) it starts, hence >> the lack of any "init" function so far. >> >>> >>> In your platform-specific init you're proposing you could set >>> tango_pad_mode to 0. That seems tango-specific. >>> >>> You'd want to hook into "set_ios" for setting sel_sdio or not. That's >>> important if anyone ever wants to plug in an external SDIO card to >>> your slot. This one good argument for putting this in >>> sdhci_arasan_soc_ctl_map, since you wouldn't want to do a >>> compatibility matching every time set_ios is called. >> >> Thanks for the advice, I will look into that. >> >>> >>> I'd have to look more into the whole SD/WP polarity issue. There are >>> already so many levels of inversion for these signals in Linux that >>> it's confusing. It seems like you might just want to hardcode them to >>> "0" and let users use all the existing ways to invert things... You >>> could either put that hardcoding into your platform init code or (if >>> you're using sdhci_arasan_soc_ctl_map) put it in the main init code so >>> that if anyone else needs to init similar signals then they can take >>> advantage of it. >> >> Yes, I think I will leave them to 0. >> >>> >>> -- >>> >>> One random side note is that as currently documented you need to >>> specify a "shift" of -1 for any sdhci_arasan_soc_ctl_map fields you >>> aren't using. That seems stupid--not sure why I did that. It seems >>> better to clue off "width = 0" so that you could just freely not init >>> any fields you don't need. >> >> I see. >> So far I'm not really convinced about using "soc_ctl_map" because what I >> have so far is more portable, and can easily be put as is somewhere else >> (i.e.: in different flavors of bootloaders) > > Well, most of your parameters are generic corecfg parameters for > Asasan. Seems like they would fit into the map nicely... > > -Doug >
| |