Hi Ulf,
Am 30.10.2019 um 16:51 schrieb Ulf Hansson ulf.hansson@linaro.org:
np = of_get_compatible_child(np, "ti,wl1251");if (np) {/** We have TI wl1251 attached to MMC3. Pass this information to* SDIO core because it can't be probed by normal methods.*/dev_info(host->dev, "found wl1251\n");card->quirks |= MMC_QUIRK_NONSTD_SDIO;card->cccr.wide_bus = 1;card->cis.vendor = 0x104c;card->cis.device = 0x9066;card->cis.blksize = 512;card->cis.max_dtr = 24000000;card->ocr = 0x80;These things should really be figured out by the mmc core during SDIO card initialization itself, not via the host ops ->init_card() callback. That is just poor hack, which in the long run should go away.
Yes, I agree.
But I am just the poor guy who is trying to fix really broken code with as low effort as possible.
I don't even have a significant clue what this code is exactly doing and what the magic values mean. They were setup by pandora_wl1251_init_card() in the same way so that I have just moved the code here and make it called in (almost) the same situation.
Moreover, I think we should add a subnode to the host node in the DT, to describe the embedded SDIO card, rather than parsing the subnode for the SDIO func - as that seems wrong to me.
You mean a second subnode?
The wl1251 is the child node of the mmc node and describes the SDIO card. We just check if it is a wl1251 or e.g. wl1837 or something else or even no child.
To add a subnode for the SDIO card, we already have a binding that I think we should extend. Please have a look at Documentation/devicetree/bindings/mmc/mmc-card.txt.
If you want an example of how to implement this for your case, do a git grep "broken-hpi" in the driver/mmc/core/, I think it will tell you more of what I have in mind.
So while I agree that it should be improved in the long run, we should IMHO fix the hack first (broken since v4.9!), even if it remains a hack for now. Improving this part seems to be quite independent and focussed on the mmc subsystem, while the other patches involve other subsystems.
Maybe should we make a REVISIT note in the code? Or add something to the commit message about the idea how it should be done right?
of_node_put(np);}}}
static void omap_hsmmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
2.19.1
Kind regards Uffe
BR and thanks, Nikolaus