| Message ID | 20220106174803.1773876-4-gsomlo@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: Add LiteSDCard mmc driver | expand |
On Thu, Jan 6, 2022 at 7:48 PM Gabriel Somlo <gsomlo@gmail.com> wrote: > > LiteX (https://github.com/enjoy-digital/litex) is a SoC framework > that targets FPGAs. LiteSDCard is a small footprint, configurable > SDCard core commonly used in LiteX designs. > > The driver was first written in May 2020 and has been maintained > cooperatively by the LiteX community. Thanks to all contributors! > +config MMC_LITEX > + tristate "LiteX MMC Host Controller support" > + depends on OF > + depends on PPC_MICROWATT || LITEX || COMPILE_TEST > + help > + This selects support for the MMC Host Controller found in LiteX SoCs. > + > + If unsure, say N. What would be the module name if built as a module? ... > +/* > + * LiteX LiteSDCard driver > + * > + * Copyright (C) 2019-2020 Antmicro <contact@antmicro.com> > + * Copyright (C) 2019-2020 Kamil Rakoczy <krakoczy@antmicro.com> > + * Copyright (C) 2019-2020 Maciej Dudek <mdudek@internships.antmicro.com> > + * Copyright (C) 2020 Paul Mackerras <paulus@ozlabs.org> > + * Copyright (C) 2020-2021 Gabriel Somlo <gsomlo@gmail.com> > + * Redundant blank line. > + */ ... > +#include <linux/module.h> > +#include <linux/litex.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/clk.h> > +#include <linux/interrupt.h> > +#include <linux/iopoll.h> > +#include <linux/mmc/sd.h> > +#include <linux/mmc/mmc.h> > +#include <linux/mmc/host.h> > +#include <linux/delay.h> > +#include <linux/dma-mapping.h> Perhaps keep it sorted? It will easily show, for example, absence of bits.h. ... > + ret = readx_poll_timeout(litex_read8, reg, evt, (evt & SD_BIT_DONE), Too many parentheses. > + SD_SLEEP_US, SD_TIMEOUT_US); > + if (ret || (evt & SD_BIT_TIMEOUT)) Redundant second condition. If you want +1 iteration, increase timeout. > + return -ETIMEDOUT; Why shadowed error code? ... > + pr_err("%s: unknown error evt=%x\n", __func__, evt); Use dev_err(). ... > + /* Wait for an interrupt if we have an interrupt and either there is > + * data to be transferred, or if the card can report busy via DAT0. > + */ This comment style is for the net subsystem, for others we use /* * Starting here... */ Fix it everywhere in your code. ... > + reg = host->sdcore + LITEX_CORE_CMDRSP; > + for (i = 0; i < 4; i++) { > + host->resp[i] = litex_read32(reg); > + reg += sizeof(u32); > + } Isn't it memcpy_fromio()? ... > + if (!host->app_cmd && cmd == SD_SEND_RELATIVE_ADDR) > + host->rca = (host->resp[3] >> 16) & 0xffff; Are you expecting a 32-bit value to be bigger than 2^32-1? ... > + div = min(max(div, 2U), 256U); clamp_t() / clamp_val() ? ... > + ret = platform_get_irq_optional(host->dev, 0); > + if (ret == -ENXIO || ret == 0) { > + dev_warn(dev, "Failed to get IRQ, using polling\n"); > + goto use_polling; > + } > + if (ret < 0) > + return ret; /* e.g., deferred probe */ > + host->irq = ret; Can it be rather written as ret = platform_get_irq_optional(host->dev, 0); if (ret < 0 && ret != -ENXIO) return ret; if (ret > 0) host->irq = ret; else { dev_warn(dev, "Failed to get IRQ, using polling\n"); goto use_polling; } ? ... > +use_polling: > + host->mmc->caps |= MMC_CAP_NEEDS_POLL; > + host->irq = 0; Isn't it 0 by default? ... > + mmc = mmc_alloc_host(sizeof(struct litex_mmc_host), &pdev->dev); > + /* NOTE: defaults to max_[req,seg]_size=PAGE_SIZE, max_blk_size=512, > + * and max_blk_count accordingly set to 8; > + * If for some reason we need to modify max_blk_count, we must also > + * re-calculate `max_[req,seg]_size = max_blk_size * max_blk_count;` > + */ Can you rather not split code by this comment. It makes sense to be above, no? > + if (!mmc) > + return -ENOMEM; ... > + /* set default sd_clk frequency range based on empirical observations > + * of LiteSDCard gateware behavior on typical SDCard media > + */ Start sentences from capital letters and keep proper style of multi-line comments. ... > +err: > + mmc_free_host(mmc); > + return ret; This... > +} > + > +static int litex_mmc_remove(struct platform_device *pdev) > +{ > + struct litex_mmc_host *host = dev_get_drvdata(&pdev->dev); > + > + if (host->irq > 0) > + free_irq(host->irq, host->mmc); > + mmc_remove_host(host->mmc); > + mmc_free_host(host->mmc); ...and this have ordering issues. You mixed devm_*() with non-devm_*() APIs in the wrong way. Also, I haven't noticed the free_irq() call in the error path of ->probe(). Isn't it missed? > + return 0; > +} ... > + .of_match_table = of_match_ptr(litex_match), Wrong usage of of_match_ptr().
Hi Andy, Thanks again, replies and follow-up questions inline below: On Thu, Jan 06, 2022 at 08:19:39PM +0200, Andy Shevchenko wrote: > On Thu, Jan 6, 2022 at 7:48 PM Gabriel Somlo <gsomlo@gmail.com> wrote: > > > > LiteX (https://github.com/enjoy-digital/litex) is a SoC framework > > that targets FPGAs. LiteSDCard is a small footprint, configurable > > SDCard core commonly used in LiteX designs. > > > > The driver was first written in May 2020 and has been maintained > > cooperatively by the LiteX community. Thanks to all contributors! > > > +config MMC_LITEX > > + tristate "LiteX MMC Host Controller support" > > + depends on OF > > + depends on PPC_MICROWATT || LITEX || COMPILE_TEST > > + help > > + This selects support for the MMC Host Controller found in LiteX SoCs. > > + > > + If unsure, say N. > > What would be the module name if built as a module? litex_mmc.ko -- why are you asking? I.e., should I mention that anywhere in the Kconfig blurb (I don't see other blurbs doing that, fwiw)? > ... > > > +/* > > + * LiteX LiteSDCard driver > > + * > > + * Copyright (C) 2019-2020 Antmicro <contact@antmicro.com> > > + * Copyright (C) 2019-2020 Kamil Rakoczy <krakoczy@antmicro.com> > > + * Copyright (C) 2019-2020 Maciej Dudek <mdudek@internships.antmicro.com> > > + * Copyright (C) 2020 Paul Mackerras <paulus@ozlabs.org> > > + * Copyright (C) 2020-2021 Gabriel Somlo <gsomlo@gmail.com> > > > + * > > Redundant blank line. OK, removed. > > > + */ > > ... > > > +#include <linux/module.h> > > +#include <linux/litex.h> > > +#include <linux/of.h> > > +#include <linux/platform_device.h> > > +#include <linux/clk.h> > > +#include <linux/interrupt.h> > > +#include <linux/iopoll.h> > > +#include <linux/mmc/sd.h> > > +#include <linux/mmc/mmc.h> > > +#include <linux/mmc/host.h> > > +#include <linux/delay.h> > > +#include <linux/dma-mapping.h> > > Perhaps keep it sorted? OK, sorted. Now looks like this: #include <linux/clk.h> #include <linux/delay.h> #include <linux/dma-mapping.h> #include <linux/interrupt.h> #include <linux/iopoll.h> #include <linux/litex.h> #include <linux/module.h> #include <linux/mmc/host.h> #include <linux/mmc/mmc.h> #include <linux/mmc/sd.h> #include <linux/mmc/slot-gpio.h> #include <linux/of.h> #include <linux/platform_device.h> > It will easily show, for example, absence of bits.h. I'm not getting any errors or warnings re. missing bits.h -- why are you mentioning it explicitly? (for the record, I try not to explicitly include anything unless I absolutely have to, so I try to avoid including headers included via other headers I already *have* to include explicitly!) I'm expecting you're going to tell me that's wrong, but then I'm much less confident of a clean canonical way of determining exactly what headers must be mentioned explicitly in a driver -- please advise! :) > > > + ret = readx_poll_timeout(litex_read8, reg, evt, (evt & SD_BIT_DONE), > > Too many parentheses. OK, removed (from both occurrences of `readx_poll_timeout()`). > > > + SD_SLEEP_US, SD_TIMEOUT_US); > > > + if (ret || (evt & SD_BIT_TIMEOUT)) > > Redundant second condition. If you want +1 iteration, increase timeout. Well, readx_poll_timeout() returns -ETIMEDOUT or 0, depending on whether or not it read the SD_BIT_DONE bit from the provided register reg. But even if it did return 0, the hardware might have returned its own timeout flag (SD_BIT_TIMEOUT) before readx_poll_timeout() decided to give up on polling the register. So we can time out in one of two different ways, and we need to check for both. > > + return -ETIMEDOUT; > > Why shadowed error code? See above -- if readx_poll_timeout returned 0, we should still return -ETIMEDOUT in case the hardware set the SD_BIT_TIMEOUT flag. I figured I'd deal with all timeout scenarios in one place, but we can definitely split that out to avoid confusion, something like this: ... ret = readx_poll_timeout(litex_read8, reg, evt, evt & SD_BIT_DONE, SD_SLEEP_US, SD_TIMEOUT_US); if (ret) return ret; if (evt == SD_BIT_DONE) return 0; if (evt & SD_BIT_WR_ERR) return -EIO; if (evt & SD_BIT_TIMEOUT) // <<< HERE return -ETIMEDOUT; if (evt & SD_BIT_CRC_ERR) return -EILSEQ; ... > ... > > > + pr_err("%s: unknown error evt=%x\n", __func__, evt); > > Use dev_err(). OK, done. > ... > > > + /* Wait for an interrupt if we have an interrupt and either there is > > + * data to be transferred, or if the card can report busy via DAT0. > > + */ > > This comment style is for the net subsystem, for others we use > /* > * Starting here... > */ > > Fix it everywhere in your code. OK, multi-line comment style updated across the entire file. > ... > > > + reg = host->sdcore + LITEX_CORE_CMDRSP; > > + for (i = 0; i < 4; i++) { > > + host->resp[i] = litex_read32(reg); > > + reg += sizeof(u32); > > + } > > Isn't it memcpy_fromio()? Yes, that appears to be the case. Looking at `litex_read32()` (in include/linux/litex.h), it's defined as le32_to_cpu((__le32 __force)readl(addr)) which counts as a "__raw_readl()", i.e., without the byteswap occurring on BE architectures. It looks like memcpy_fromio() doesn't play byteswap games on BE either, so they should be equivalent. With the caveat that I only have the ability to test on LE! For now, based on the above observations, I think it's safe to use memcpy_fromio() here. > ... > > > + if (!host->app_cmd && cmd == SD_SEND_RELATIVE_ADDR) > > + host->rca = (host->resp[3] >> 16) & 0xffff; > > Are you expecting a 32-bit value to be bigger than 2^32-1? Fair enough :) > ... > > > + div = min(max(div, 2U), 256U); > > clamp_t() / clamp_val() ? OK, changed to `div = clamp_val(div, 2U, 256U);` > ... > > > + ret = platform_get_irq_optional(host->dev, 0); > > + if (ret == -ENXIO || ret == 0) { > > + dev_warn(dev, "Failed to get IRQ, using polling\n"); > > + goto use_polling; > > + } > > + if (ret < 0) > > + return ret; /* e.g., deferred probe */ > > + host->irq = ret; > > Can it be rather written as > > ret = platform_get_irq_optional(host->dev, 0); > if (ret < 0 && ret != -ENXIO) > return ret; > if (ret > 0) > host->irq = ret; > else { > dev_warn(dev, "Failed to get IRQ, using polling\n"); > goto use_polling; > } > > ? OK, done. > ... > > > +use_polling: > > + host->mmc->caps |= MMC_CAP_NEEDS_POLL; > > > + host->irq = 0; > > Isn't it 0 by default? Yes, because of kzalloc() by way of mmc_alloc_host(). So I don't have to set it to 0 explicitly -- Done. > ... > > > + mmc = mmc_alloc_host(sizeof(struct litex_mmc_host), &pdev->dev); > > > + /* NOTE: defaults to max_[req,seg]_size=PAGE_SIZE, max_blk_size=512, > > + * and max_blk_count accordingly set to 8; > > + * If for some reason we need to modify max_blk_count, we must also > > + * re-calculate `max_[req,seg]_size = max_blk_size * max_blk_count;` > > + */ > > Can you rather not split code by this comment. It makes sense to be above, no? OK -- Done. > > + if (!mmc) > > + return -ENOMEM; > > ... > > > + /* set default sd_clk frequency range based on empirical observations > > + * of LiteSDCard gateware behavior on typical SDCard media > > + */ > > Start sentences from capital letters and keep proper style of > multi-line comments. Multi-line comment style fixed per your earlier observation. I also went and capitalized the first letter of each comment throughout the source. > ... > > > +err: > > + mmc_free_host(mmc); > > + return ret; > > This... > > > +} > > + > > +static int litex_mmc_remove(struct platform_device *pdev) > > +{ > > + struct litex_mmc_host *host = dev_get_drvdata(&pdev->dev); > > + > > + if (host->irq > 0) > > + free_irq(host->irq, host->mmc); > > + mmc_remove_host(host->mmc); > > + mmc_free_host(host->mmc); > > ...and this have ordering issues. You mixed devm_*() with non-devm_*() > APIs in the wrong way. > > Also, I haven't noticed the free_irq() call in the error path of > ->probe(). Isn't it missed? OK, I've reordered everything (and added the missing free_irq() call like so: static int litex_mmc_probe(struct platform_device *pdev) { ... ... return 0; err: if (host->irq > 0) free_irq(host->irq, host->mmc); mmc_free_host(mmc); return ret; } static int litex_mmc_remove(struct platform_device *pdev) { struct litex_mmc_host *host = dev_get_drvdata(&pdev->dev); mmc_remove_host(host->mmc); if (host->irq > 0) free_irq(host->irq, host->mmc); mmc_free_host(host->mmc); return 0; } Looking at other examples in drivers/mmc/host/* I'm not sure if/what other ordering issues or devm/non-devm mixing violations I'm perpetrating: if there are any left, could you please point them out in a bit more detail? > > + return 0; > > +} > > ... > > > + .of_match_table = of_match_ptr(litex_match), > > Wrong usage of of_match_ptr(). Huh? It's consistent with how a whole bunch of other files in drivers/mmc/host/*.c are using it, can you please elaborate? Thanks much for taking the time to review the driver, much appreciated! I'll send out a v7 as soon as I get some clarity on the portions above that I'm still confused about... :) Thanks again, --Gabriel
Hi Gabriel, On Thu, Jan 6, 2022 at 11:50 PM Gabriel L. Somlo <gsomlo@gmail.com> wrote: > On Thu, Jan 06, 2022 at 08:19:39PM +0200, Andy Shevchenko wrote: > > On Thu, Jan 6, 2022 at 7:48 PM Gabriel Somlo <gsomlo@gmail.com> wrote: > > > > > > LiteX (https://github.com/enjoy-digital/litex) is a SoC framework > > > that targets FPGAs. LiteSDCard is a small footprint, configurable > > > SDCard core commonly used in LiteX designs. > > > > > > The driver was first written in May 2020 and has been maintained > > > cooperatively by the LiteX community. Thanks to all contributors! > > > > > +config MMC_LITEX > > > + tristate "LiteX MMC Host Controller support" > > > + depends on OF > > > + depends on PPC_MICROWATT || LITEX || COMPILE_TEST > > > + help > > > + This selects support for the MMC Host Controller found in LiteX SoCs. > > > + > > > + If unsure, say N. > > > > What would be the module name if built as a module? > > litex_mmc.ko -- why are you asking? I.e., should I mention that anywhere > in the Kconfig blurb (I don't see other blurbs doing that, fwiw)? Many (most?) blurbs do mention the module name. > > > + div = min(max(div, 2U), 256U); > > > > clamp_t() / clamp_val() ? > > OK, changed to `div = clamp_val(div, 2U, 256U);` Please use clamp() instead of clamp_val(), as all three parameters have the same type (clamp_val() uses casts to align all parameters; casts are evil). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Fri, Jan 07, 2022 at 10:36:12AM +0100, Geert Uytterhoeven wrote: > Hi Gabriel, > > On Thu, Jan 6, 2022 at 11:50 PM Gabriel L. Somlo <gsomlo@gmail.com> wrote: > > On Thu, Jan 06, 2022 at 08:19:39PM +0200, Andy Shevchenko wrote: > > > On Thu, Jan 6, 2022 at 7:48 PM Gabriel Somlo <gsomlo@gmail.com> wrote: > > > > > > > > LiteX (https://github.com/enjoy-digital/litex) is a SoC framework > > > > that targets FPGAs. LiteSDCard is a small footprint, configurable > > > > SDCard core commonly used in LiteX designs. > > > > > > > > The driver was first written in May 2020 and has been maintained > > > > cooperatively by the LiteX community. Thanks to all contributors! > > > > > > > +config MMC_LITEX > > > > + tristate "LiteX MMC Host Controller support" > > > > + depends on OF > > > > + depends on PPC_MICROWATT || LITEX || COMPILE_TEST > > > > + help > > > > + This selects support for the MMC Host Controller found in LiteX SoCs. > > > > + > > > > + If unsure, say N. > > > > > > What would be the module name if built as a module? > > > > litex_mmc.ko -- why are you asking? I.e., should I mention that anywhere > > in the Kconfig blurb (I don't see other blurbs doing that, fwiw)? > > Many (most?) blurbs do mention the module name. I was doubting this as well, but I searched and its true. The text 'module will be called' shows up many times, there is also different text. $ grep -r 'module will be called' drivers/ | wc 1347 9023 9086 $ grep -r 'tristate \"' drivers/ | wc 7169 47486 521795 So maybe >10% have module name in the blurb. Example: To compile this driver as a module, choose M here: the module will be called tifm_sd. Thanks, -Stafford
On Fri, Jan 7, 2022 at 12:12 PM Stafford Horne <shorne@gmail.com> wrote: > On Fri, Jan 07, 2022 at 10:36:12AM +0100, Geert Uytterhoeven wrote: > > On Thu, Jan 6, 2022 at 11:50 PM Gabriel L. Somlo <gsomlo@gmail.com> wrote: ... > > Many (most?) blurbs do mention the module name. > > I was doubting this as well, but I searched and its true. The text 'module will > be called' shows up many times, there is also different text. > > $ grep -r 'module will be called' drivers/ | wc > 1347 9023 9086 > > $ grep -r 'tristate \"' drivers/ | wc > 7169 47486 521795 Just a side note: `git grep ...` is much faster in the Git trees. And for this particular case I dare to advertise a script I wrote [1] to help with recursive searches. [1]: https://github.com/andy-shev/home-bin-tools/blob/master/gl4func.sh > So maybe >10% have module name in the blurb. Example: > > To compile this driver as a module, choose M here: the > module will be called tifm_sd.
Hi Andy, On Fri, Jan 7, 2022 at 11:25 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Fri, Jan 7, 2022 at 12:12 PM Stafford Horne <shorne@gmail.com> wrote: > > On Fri, Jan 07, 2022 at 10:36:12AM +0100, Geert Uytterhoeven wrote: > > > On Thu, Jan 6, 2022 at 11:50 PM Gabriel L. Somlo <gsomlo@gmail.com> wrote: > > ... > > > > Many (most?) blurbs do mention the module name. > > > > I was doubting this as well, but I searched and its true. The text 'module will > > be called' shows up many times, there is also different text. > > > > $ grep -r 'module will be called' drivers/ | wc > > 1347 9023 9086 > > > > $ grep -r 'tristate \"' drivers/ | wc > > 7169 47486 521795 > > Just a side note: `git grep ...` is much faster in the Git trees. Indeed. > And for this particular case I dare to advertise a script I wrote [1] > to help with recursive searches. > > [1]: https://github.com/andy-shev/home-bin-tools/blob/master/gl4func.sh Cool! My fingers are used to type git grep -w <pat1> -- $(git grep -lw <pat2> -- ...) ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Fri, Jan 7, 2022 at 11:29 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Fri, Jan 7, 2022 at 11:25 AM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Fri, Jan 7, 2022 at 12:12 PM Stafford Horne <shorne@gmail.com> wrote: > > > On Fri, Jan 07, 2022 at 10:36:12AM +0100, Geert Uytterhoeven wrote: > > > > On Thu, Jan 6, 2022 at 11:50 PM Gabriel L. Somlo <gsomlo@gmail.com> wrote: > > > > ... > > > > > > Many (most?) blurbs do mention the module name. > > > > > > I was doubting this as well, but I searched and its true. The text 'module will > > > be called' shows up many times, there is also different text. > > > > > > $ grep -r 'module will be called' drivers/ | wc > > > 1347 9023 9086 > > > > > > $ grep -r 'tristate \"' drivers/ | wc > > > 7169 47486 521795 > > > > Just a side note: `git grep ...` is much faster in the Git trees. > > Indeed. > > > And for this particular case I dare to advertise a script I wrote [1] > > to help with recursive searches. > > > > [1]: https://github.com/andy-shev/home-bin-tools/blob/master/gl4func.sh > > Cool! > > My fingers are used to type > > git grep -w <pat1> -- $(git grep -lw <pat2> -- ...) > > ;-) Actually you can just use git grep --and? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Fri, Jan 7, 2022 at 12:30 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Fri, Jan 7, 2022 at 11:29 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Fri, Jan 7, 2022 at 11:25 AM Andy Shevchenko > > <andy.shevchenko@gmail.com> wrote: > > > On Fri, Jan 7, 2022 at 12:12 PM Stafford Horne <shorne@gmail.com> wrote: ... > > > And for this particular case I dare to advertise a script I wrote [1] > > > to help with recursive searches. > > > > > > [1]: https://github.com/andy-shev/home-bin-tools/blob/master/gl4func.sh > > > > Cool! > > > > My fingers are used to type > > > > git grep -w <pat1> -- $(git grep -lw <pat2> -- ...) > > > > ;-) > > Actually you can just use git grep --and? Thanks for the hint (and patches are welcome :-). The last time I have read the man of git grep was a long time ago, and I definitely missed any new features.
On Fri, Jan 07, 2022 at 12:24:23PM +0200, Andy Shevchenko wrote: > On Fri, Jan 7, 2022 at 12:12 PM Stafford Horne <shorne@gmail.com> wrote: > > On Fri, Jan 07, 2022 at 10:36:12AM +0100, Geert Uytterhoeven wrote: > > > On Thu, Jan 6, 2022 at 11:50 PM Gabriel L. Somlo <gsomlo@gmail.com> wrote: > > ... > > > > Many (most?) blurbs do mention the module name. > > > > I was doubting this as well, but I searched and its true. The text 'module will > > be called' shows up many times, there is also different text. > > > > $ grep -r 'module will be called' drivers/ | wc > > 1347 9023 9086 > > > > $ grep -r 'tristate \"' drivers/ | wc > > 7169 47486 521795 > > Just a side note: `git grep ...` is much faster in the Git trees. Yes, it is quite a lot faster, I always wondered why one would use it rather than just grep. Thanks for the tip. < shorne@antec ~/work/linux > time grep -r 'module will be called' drivers/ >/dev/null real 0m0.338s user 0m0.220s sys 0m0.113s < shorne@antec ~/work/linux > time git grep 'module will be called' -- drivers/ >/dev/null real 0m0.153s user 0m0.205s sys 0m0.659s > And for this particular case I dare to advertise a script I wrote [1] > to help with recursive searches. > > [1]: https://github.com/andy-shev/home-bin-tools/blob/master/gl4func.sh Neat script. > > So maybe >10% have module name in the blurb. Example: > > > > To compile this driver as a module, choose M here: the > > module will be called tifm_sd. > > > > -- > With Best Regards, > Andy Shevchenko
On Fri, Jan 07, 2022 at 07:12:43PM +0900, Stafford Horne wrote: > On Fri, Jan 07, 2022 at 10:36:12AM +0100, Geert Uytterhoeven wrote: > > Hi Gabriel, > > > > On Thu, Jan 6, 2022 at 11:50 PM Gabriel L. Somlo <gsomlo@gmail.com> wrote: > > > On Thu, Jan 06, 2022 at 08:19:39PM +0200, Andy Shevchenko wrote: > > > > On Thu, Jan 6, 2022 at 7:48 PM Gabriel Somlo <gsomlo@gmail.com> wrote: > > > > > > > > > > LiteX (https://github.com/enjoy-digital/litex) is a SoC framework > > > > > that targets FPGAs. LiteSDCard is a small footprint, configurable > > > > > SDCard core commonly used in LiteX designs. > > > > > > > > > > The driver was first written in May 2020 and has been maintained > > > > > cooperatively by the LiteX community. Thanks to all contributors! > > > > > > > > > +config MMC_LITEX > > > > > + tristate "LiteX MMC Host Controller support" > > > > > + depends on OF > > > > > + depends on PPC_MICROWATT || LITEX || COMPILE_TEST > > > > > + help > > > > > + This selects support for the MMC Host Controller found in LiteX SoCs. > > > > > + > > > > > + If unsure, say N. > > > > > > > > What would be the module name if built as a module? > > > > > > litex_mmc.ko -- why are you asking? I.e., should I mention that anywhere > > > in the Kconfig blurb (I don't see other blurbs doing that, fwiw)? > > > > Many (most?) blurbs do mention the module name. > > I was doubting this as well, but I searched and its true. The text 'module will > be called' shows up many times, there is also different text. > > $ grep -r 'module will be called' drivers/ | wc > 1347 9023 9086 > > $ grep -r 'tristate \"' drivers/ | wc > 7169 47486 521795 OK, so initially I quickly looked at a few entries before LiteX in the drivers/mmc/host/Kconfig file, and saw nothing useful, and asked a follow-up question... :) Knowing the magic "module will be called" incantation to grep for helped, and now I created a similar one for LiteSDCard's Kconfig entry, and lined it up for v7. Thanks to everyone who chimed in! Best, --Gabriel
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index 5af8494c31b5..c1b66d06d1c9 100644 --- a/drivers/mmc/host/Kconfig +++ b/drivers/mmc/host/Kconfig @@ -1093,3 +1093,12 @@ config MMC_OWL config MMC_SDHCI_EXTERNAL_DMA bool + +config MMC_LITEX + tristate "LiteX MMC Host Controller support" + depends on OF + depends on PPC_MICROWATT || LITEX || COMPILE_TEST + help + This selects support for the MMC Host Controller found in LiteX SoCs. + + If unsure, say N. diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile index ea36d379bd3c..4e4ceb32c4b4 100644 --- a/drivers/mmc/host/Makefile +++ b/drivers/mmc/host/Makefile @@ -101,6 +101,7 @@ obj-$(CONFIG_MMC_CQHCI) += cqhci.o cqhci-y += cqhci-core.o cqhci-$(CONFIG_MMC_CRYPTO) += cqhci-crypto.o obj-$(CONFIG_MMC_HSQ) += mmc_hsq.o +obj-$(CONFIG_MMC_LITEX) += litex_mmc.o ifeq ($(CONFIG_CB710_DEBUG),y) CFLAGS-cb710-mmc += -DDEBUG diff --git a/drivers/mmc/host/litex_mmc.c b/drivers/mmc/host/litex_mmc.c new file mode 100644 index 000000000000..cbed97ca70d0 --- /dev/null +++ b/drivers/mmc/host/litex_mmc.c @@ -0,0 +1,659 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * LiteX LiteSDCard driver + * + * Copyright (C) 2019-2020 Antmicro <contact@antmicro.com> + * Copyright (C) 2019-2020 Kamil Rakoczy <krakoczy@antmicro.com> + * Copyright (C) 2019-2020 Maciej Dudek <mdudek@internships.antmicro.com> + * Copyright (C) 2020 Paul Mackerras <paulus@ozlabs.org> + * Copyright (C) 2020-2021 Gabriel Somlo <gsomlo@gmail.com> + * + */ + +#include <linux/module.h> +#include <linux/litex.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/clk.h> +#include <linux/interrupt.h> +#include <linux/iopoll.h> +#include <linux/mmc/sd.h> +#include <linux/mmc/mmc.h> +#include <linux/mmc/host.h> +#include <linux/delay.h> +#include <linux/dma-mapping.h> + +#define LITEX_PHY_CARDDETECT 0x00 +#define LITEX_PHY_CLOCKERDIV 0x04 +#define LITEX_PHY_INITIALIZE 0x08 +#define LITEX_PHY_WRITESTATUS 0x0C +#define LITEX_CORE_CMDARG 0x00 +#define LITEX_CORE_CMDCMD 0x04 +#define LITEX_CORE_CMDSND 0x08 +#define LITEX_CORE_CMDRSP 0x0C +#define LITEX_CORE_CMDEVT 0x1C +#define LITEX_CORE_DATAEVT 0x20 +#define LITEX_CORE_BLKLEN 0x24 +#define LITEX_CORE_BLKCNT 0x28 +#define LITEX_BLK2MEM_BASE 0x00 +#define LITEX_BLK2MEM_LEN 0x08 +#define LITEX_BLK2MEM_ENA 0x0C +#define LITEX_BLK2MEM_DONE 0x10 +#define LITEX_BLK2MEM_LOOP 0x14 +#define LITEX_MEM2BLK_BASE 0x00 +#define LITEX_MEM2BLK_LEN 0x08 +#define LITEX_MEM2BLK_ENA 0x0C +#define LITEX_MEM2BLK_DONE 0x10 +#define LITEX_MEM2BLK_LOOP 0x14 +#define LITEX_MEM2BLK 0x18 +#define LITEX_IRQ_STATUS 0x00 +#define LITEX_IRQ_PENDING 0x04 +#define LITEX_IRQ_ENABLE 0x08 + +#define SD_CTL_DATA_XFER_NONE 0 +#define SD_CTL_DATA_XFER_READ 1 +#define SD_CTL_DATA_XFER_WRITE 2 + +#define SD_CTL_RESP_NONE 0 +#define SD_CTL_RESP_SHORT 1 +#define SD_CTL_RESP_LONG 2 +#define SD_CTL_RESP_SHORT_BUSY 3 + +#define SD_BIT_DONE BIT(0) +#define SD_BIT_WR_ERR BIT(1) +#define SD_BIT_TIMEOUT BIT(2) +#define SD_BIT_CRC_ERR BIT(3) + +#define SD_SLEEP_US 5 +#define SD_TIMEOUT_US 20000 + +#define SDIRQ_CARD_DETECT 1 +#define SDIRQ_SD_TO_MEM_DONE 2 +#define SDIRQ_MEM_TO_SD_DONE 4 +#define SDIRQ_CMD_DONE 8 + +#define LITEX_MMC_OCR (MMC_VDD_27_28 | MMC_VDD_28_29 | MMC_VDD_29_30 | \ + MMC_VDD_30_31 | MMC_VDD_31_32 | MMC_VDD_32_33 | \ + MMC_VDD_33_34 | MMC_VDD_34_35 | MMC_VDD_35_36) + +struct litex_mmc_host { + struct mmc_host *mmc; + struct platform_device *dev; + + void __iomem *sdphy; + void __iomem *sdcore; + void __iomem *sdreader; + void __iomem *sdwriter; + void __iomem *sdirq; + + void *buffer; + size_t buf_size; + dma_addr_t dma; + + struct completion cmd_done; + int irq; + + unsigned int ref_clk; + unsigned int sd_clk; + + u32 resp[4]; + u16 rca; + + bool is_bus_width_set; + bool app_cmd; +}; + +static int litex_mmc_sdcard_wait_done(void __iomem *reg) +{ + u8 evt; + int ret; + + ret = readx_poll_timeout(litex_read8, reg, evt, (evt & SD_BIT_DONE), + SD_SLEEP_US, SD_TIMEOUT_US); + if (ret || (evt & SD_BIT_TIMEOUT)) + return -ETIMEDOUT; + if (evt == SD_BIT_DONE) + return 0; + if (evt & SD_BIT_WR_ERR) + return -EIO; + if (evt & SD_BIT_CRC_ERR) + return -EILSEQ; + pr_err("%s: unknown error evt=%x\n", __func__, evt); + return -EINVAL; +} + +static int litex_mmc_send_cmd(struct litex_mmc_host *host, + u8 cmd, u32 arg, u8 response_len, u8 transfer) +{ + struct device *dev = mmc_dev(host->mmc); + void __iomem *reg; + int ret; + u8 evt; + + litex_write32(host->sdcore + LITEX_CORE_CMDARG, arg); + litex_write32(host->sdcore + LITEX_CORE_CMDCMD, + cmd << 8 | transfer << 5 | response_len); + litex_write8(host->sdcore + LITEX_CORE_CMDSND, 1); + + /* Wait for an interrupt if we have an interrupt and either there is + * data to be transferred, or if the card can report busy via DAT0. + */ + if (host->irq > 0 && + (transfer != SD_CTL_DATA_XFER_NONE || + response_len == SD_CTL_RESP_SHORT_BUSY)) { + reinit_completion(&host->cmd_done); + litex_write32(host->sdirq + LITEX_IRQ_ENABLE, + SDIRQ_CMD_DONE | SDIRQ_CARD_DETECT); + wait_for_completion(&host->cmd_done); + } + + ret = litex_mmc_sdcard_wait_done(host->sdcore + LITEX_CORE_CMDEVT); + if (ret) { + dev_err(dev, "Command (cmd %d) error, status %d\n", cmd, ret); + return ret; + } + + if (response_len != SD_CTL_RESP_NONE) { + int i; + + reg = host->sdcore + LITEX_CORE_CMDRSP; + for (i = 0; i < 4; i++) { + host->resp[i] = litex_read32(reg); + reg += sizeof(u32); + } + } + + if (!host->app_cmd && cmd == SD_SEND_RELATIVE_ADDR) + host->rca = (host->resp[3] >> 16) & 0xffff; + + host->app_cmd = (cmd == MMC_APP_CMD); + + if (transfer == SD_CTL_DATA_XFER_NONE) + return ret; /* OK from prior litex_mmc_sdcard_wait_done() */ + + ret = litex_mmc_sdcard_wait_done(host->sdcore + LITEX_CORE_DATAEVT); + if (ret) { + dev_err(dev, "Data xfer (cmd %d) error, status %d\n", cmd, ret); + return ret; + } + + /* wait for completion of (read or write) DMA transfer */ + reg = (transfer == SD_CTL_DATA_XFER_READ) ? + host->sdreader + LITEX_BLK2MEM_DONE : + host->sdwriter + LITEX_MEM2BLK_DONE; + ret = readx_poll_timeout(litex_read8, reg, evt, (evt & SD_BIT_DONE), + SD_SLEEP_US, SD_TIMEOUT_US); + if (ret) + dev_err(dev, "DMA timeout (cmd %d)\n", cmd); + + return ret; +} + +static int litex_mmc_send_app_cmd(struct litex_mmc_host *host) +{ + return litex_mmc_send_cmd(host, MMC_APP_CMD, host->rca << 16, + SD_CTL_RESP_SHORT, SD_CTL_DATA_XFER_NONE); +} + +static int litex_mmc_send_set_bus_w_cmd(struct litex_mmc_host *host, u32 width) +{ + return litex_mmc_send_cmd(host, SD_APP_SET_BUS_WIDTH, width, + SD_CTL_RESP_SHORT, SD_CTL_DATA_XFER_NONE); +} + +static int litex_mmc_set_bus_width(struct litex_mmc_host *host) +{ + bool app_cmd_sent; + int ret; + + if (host->is_bus_width_set) + return 0; + + /* ensure 'app_cmd' precedes 'app_set_bus_width_cmd' */ + app_cmd_sent = host->app_cmd; /* was preceding command app_cmd? */ + if (!app_cmd_sent) { + ret = litex_mmc_send_app_cmd(host); + if (ret) + return ret; + } + + /* litesdcard only supports 4-bit bus width */ + ret = litex_mmc_send_set_bus_w_cmd(host, MMC_BUS_WIDTH_4); + if (ret) + return ret; + + /* re-send 'app_cmd' if necessary */ + if (app_cmd_sent) { + ret = litex_mmc_send_app_cmd(host); + if (ret) + return ret; + } + + host->is_bus_width_set = true; + + return 0; +} + +static int litex_mmc_get_cd(struct mmc_host *mmc) +{ + struct litex_mmc_host *host = mmc_priv(mmc); + int ret; + + if (!mmc_card_is_removable(mmc)) + return 1; + + ret = !litex_read8(host->sdphy + LITEX_PHY_CARDDETECT); + + /* ensure bus width will be set (again) upon card (re)insertion */ + if (ret == 0) + host->is_bus_width_set = false; + + return ret; +} + +static irqreturn_t litex_mmc_interrupt(int irq, void *arg) +{ + struct mmc_host *mmc = arg; + struct litex_mmc_host *host = mmc_priv(mmc); + u32 pending = litex_read32(host->sdirq + LITEX_IRQ_PENDING); + + /* Check for card change interrupt */ + if (pending & SDIRQ_CARD_DETECT) { + litex_write32(host->sdirq + LITEX_IRQ_PENDING, + SDIRQ_CARD_DETECT); + mmc_detect_change(mmc, msecs_to_jiffies(10)); + } + + /* Check for command completed */ + if (pending & SDIRQ_CMD_DONE) { + /* Disable it so it doesn't keep interrupting */ + litex_write32(host->sdirq + LITEX_IRQ_ENABLE, + SDIRQ_CARD_DETECT); + complete(&host->cmd_done); + } + + return IRQ_HANDLED; +} + +static u32 litex_mmc_response_len(struct mmc_command *cmd) +{ + if (cmd->flags & MMC_RSP_136) + return SD_CTL_RESP_LONG; + if (!(cmd->flags & MMC_RSP_PRESENT)) + return SD_CTL_RESP_NONE; + if (cmd->flags & MMC_RSP_BUSY) + return SD_CTL_RESP_SHORT_BUSY; + return SD_CTL_RESP_SHORT; +} + +static void litex_mmc_do_dma(struct litex_mmc_host *host, struct mmc_data *data, + unsigned int *len, bool *direct, u8 *transfer) +{ + struct device *dev = mmc_dev(host->mmc); + dma_addr_t dma; + int sg_count; + + /* Try to DMA directly to/from the data buffer. + * We can do that if the buffer can be mapped for DMA + * in one contiguous chunk. + */ + dma = host->dma; + *len = data->blksz * data->blocks; + sg_count = dma_map_sg(dev, data->sg, data->sg_len, + mmc_get_dma_dir(data)); + if (sg_count == 1) { + dma = sg_dma_address(data->sg); + *len = sg_dma_len(data->sg); + *direct = true; + } else if (*len > host->buf_size) + *len = host->buf_size; + + if (data->flags & MMC_DATA_READ) { + litex_write8(host->sdreader + LITEX_BLK2MEM_ENA, 0); + litex_write64(host->sdreader + LITEX_BLK2MEM_BASE, dma); + litex_write32(host->sdreader + LITEX_BLK2MEM_LEN, *len); + litex_write8(host->sdreader + LITEX_BLK2MEM_ENA, 1); + *transfer = SD_CTL_DATA_XFER_READ; + } else if (data->flags & MMC_DATA_WRITE) { + if (!*direct) + sg_copy_to_buffer(data->sg, data->sg_len, + host->buffer, *len); + litex_write8(host->sdwriter + LITEX_MEM2BLK_ENA, 0); + litex_write64(host->sdwriter + LITEX_MEM2BLK_BASE, dma); + litex_write32(host->sdwriter + LITEX_MEM2BLK_LEN, *len); + litex_write8(host->sdwriter + LITEX_MEM2BLK_ENA, 1); + *transfer = SD_CTL_DATA_XFER_WRITE; + } else { + dev_warn(dev, "Data present w/o read or write flag.\n"); + /* Continue: set cmd status, mark req done */ + } + + litex_write16(host->sdcore + LITEX_CORE_BLKLEN, data->blksz); + litex_write32(host->sdcore + LITEX_CORE_BLKCNT, data->blocks); +} + +static void litex_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq) +{ + struct litex_mmc_host *host = mmc_priv(mmc); + struct device *dev = mmc_dev(mmc); + struct mmc_command *cmd = mrq->cmd; + struct mmc_command *sbc = mrq->sbc; + struct mmc_data *data = mrq->data; + struct mmc_command *stop = mrq->stop; + unsigned int retries = cmd->retries; + unsigned int len = 0; + bool direct = false; + u32 response_len = litex_mmc_response_len(cmd); + u8 transfer = SD_CTL_DATA_XFER_NONE; + + /* First check that the card is still there */ + if (!litex_mmc_get_cd(mmc)) { + cmd->error = -ENOMEDIUM; + mmc_request_done(mmc, mrq); + return; + } + + /* Send set-block-count command if needed */ + if (sbc) { + sbc->error = litex_mmc_send_cmd(host, sbc->opcode, sbc->arg, + litex_mmc_response_len(sbc), + SD_CTL_DATA_XFER_NONE); + if (sbc->error) { + host->is_bus_width_set = false; + mmc_request_done(mmc, mrq); + return; + } + } + + if (data) { + /* LiteSDCard only supports 4-bit bus width; therefore, we MUST + * inject a SET_BUS_WIDTH (acmd6) before the very first data + * transfer, earlier than when the mmc subsystem would normally + * get around to it! + */ + cmd->error = litex_mmc_set_bus_width(host); + if (cmd->error) { + dev_err(dev, "Can't set bus width!\n"); + mmc_request_done(mmc, mrq); + return; + } + + litex_mmc_do_dma(host, data, &len, &direct, &transfer); + } + + do { + cmd->error = litex_mmc_send_cmd(host, cmd->opcode, cmd->arg, + response_len, transfer); + } while (cmd->error && retries-- > 0); + + if (cmd->error) { + /* card may be gone; don't assume bus width is still set */ + host->is_bus_width_set = false; + } + + if (response_len == SD_CTL_RESP_SHORT) { + /* pull short response fields from appropriate host registers */ + cmd->resp[0] = host->resp[3]; + cmd->resp[1] = host->resp[2] & 0xFF; + } else if (response_len == SD_CTL_RESP_LONG) { + cmd->resp[0] = host->resp[0]; + cmd->resp[1] = host->resp[1]; + cmd->resp[2] = host->resp[2]; + cmd->resp[3] = host->resp[3]; + } + + /* Send stop-transmission command if required */ + if (stop && (cmd->error || !sbc)) { + stop->error = litex_mmc_send_cmd(host, stop->opcode, stop->arg, + litex_mmc_response_len(stop), + SD_CTL_DATA_XFER_NONE); + if (stop->error) + host->is_bus_width_set = false; + } + + if (data) { + dma_unmap_sg(dev, data->sg, data->sg_len, + mmc_get_dma_dir(data)); + } + + if (!cmd->error && transfer != SD_CTL_DATA_XFER_NONE) { + data->bytes_xfered = min(len, mmc->max_req_size); + if (transfer == SD_CTL_DATA_XFER_READ && !direct) { + sg_copy_from_buffer(data->sg, sg_nents(data->sg), + host->buffer, data->bytes_xfered); + } + } + + mmc_request_done(mmc, mrq); +} + +static void litex_mmc_setclk(struct litex_mmc_host *host, unsigned int freq) +{ + struct device *dev = mmc_dev(host->mmc); + u32 div; + + div = freq ? host->ref_clk / freq : 256U; + div = roundup_pow_of_two(div); + div = min(max(div, 2U), 256U); + dev_dbg(dev, "sd_clk_freq=%d: set to %d via div=%d\n", + freq, host->ref_clk / div, div); + litex_write16(host->sdphy + LITEX_PHY_CLOCKERDIV, div); + host->sd_clk = freq; +} + +static void litex_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) +{ + struct litex_mmc_host *host = mmc_priv(mmc); + + /* NOTE: Ignore any ios->bus_width updates; they occur right after + * the mmc core sends its own acmd6 bus-width change notification, + * which is redundant since we snoop on the command flow and inject + * an early acmd6 before the first data transfer command is sent! + */ + + /* update sd_clk */ + if (ios->clock != host->sd_clk) + litex_mmc_setclk(host, ios->clock); +} + +static const struct mmc_host_ops litex_mmc_ops = { + .get_cd = litex_mmc_get_cd, + .request = litex_mmc_request, + .set_ios = litex_mmc_set_ios, +}; + +static int litex_mmc_irq_init(struct litex_mmc_host *host) +{ + struct device *dev = mmc_dev(host->mmc); + int ret; + + ret = platform_get_irq_optional(host->dev, 0); + if (ret == -ENXIO || ret == 0) { + dev_warn(dev, "Failed to get IRQ, using polling\n"); + goto use_polling; + } + if (ret < 0) + return ret; /* e.g., deferred probe */ + host->irq = ret; + + host->sdirq = devm_platform_ioremap_resource_byname(host->dev, "irq"); + if (IS_ERR(host->sdirq)) + return PTR_ERR(host->sdirq); + + ret = request_irq(host->irq, litex_mmc_interrupt, 0, + "litex-mmc", host->mmc); + if (ret < 0) { + dev_warn(dev, "IRQ request error %d, using polling\n", ret); + goto use_polling; + } + + /* clear & enable card-change interrupts */ + litex_write32(host->sdirq + LITEX_IRQ_PENDING, SDIRQ_CARD_DETECT); + litex_write32(host->sdirq + LITEX_IRQ_ENABLE, SDIRQ_CARD_DETECT); + + return 0; + +use_polling: + host->mmc->caps |= MMC_CAP_NEEDS_POLL; + host->irq = 0; + return 0; +} + +static int litex_mmc_probe(struct platform_device *pdev) +{ + struct litex_mmc_host *host; + struct mmc_host *mmc; + struct clk *clk; + int ret; + + mmc = mmc_alloc_host(sizeof(struct litex_mmc_host), &pdev->dev); + /* NOTE: defaults to max_[req,seg]_size=PAGE_SIZE, max_blk_size=512, + * and max_blk_count accordingly set to 8; + * If for some reason we need to modify max_blk_count, we must also + * re-calculate `max_[req,seg]_size = max_blk_size * max_blk_count;` + */ + if (!mmc) + return -ENOMEM; + + host = mmc_priv(mmc); + host->mmc = mmc; + host->dev = pdev; + + /* initialize clock source */ + clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(clk)) { + ret = dev_err_probe(&pdev->dev, + PTR_ERR(clk), "can't get clock\n"); + goto err; + } + host->ref_clk = clk_get_rate(clk); + host->sd_clk = 0; + + /* LiteSDCard only supports 4-bit bus width; therefore, we MUST inject + * a SET_BUS_WIDTH (acmd6) before the very first data transfer, earlier + * than when the mmc subsystem would normally get around to it! + */ + host->is_bus_width_set = false; + host->app_cmd = false; + + /* LiteSDCard can support 64-bit DMA addressing */ + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)); + if (ret) + goto err; + + host->buf_size = mmc->max_req_size * 2; + host->buffer = dmam_alloc_coherent(&pdev->dev, host->buf_size, + &host->dma, GFP_DMA); + if (host->buffer == NULL) { + ret = -ENOMEM; + goto err; + } + + host->sdphy = devm_platform_ioremap_resource_byname(pdev, "phy"); + if (IS_ERR(host->sdphy)) { + ret = PTR_ERR(host->sdphy); + goto err; + } + + host->sdcore = devm_platform_ioremap_resource_byname(pdev, "core"); + if (IS_ERR(host->sdcore)) { + ret = PTR_ERR(host->sdcore); + goto err; + } + + host->sdreader = devm_platform_ioremap_resource_byname(pdev, "reader"); + if (IS_ERR(host->sdreader)) { + ret = PTR_ERR(host->sdreader); + goto err; + } + + host->sdwriter = devm_platform_ioremap_resource_byname(pdev, "writer"); + if (IS_ERR(host->sdwriter)) { + ret = PTR_ERR(host->sdwriter); + goto err; + } + + /* ensure DMA bus masters are disabled */ + litex_write8(host->sdreader + LITEX_BLK2MEM_ENA, 0); + litex_write8(host->sdwriter + LITEX_MEM2BLK_ENA, 0); + + init_completion(&host->cmd_done); + ret = litex_mmc_irq_init(host); + if (ret) + goto err; + + /* allow full generic 2.7-3.6V range; no software tuning available */ + mmc->ocr_avail = LITEX_MMC_OCR; + + mmc->ops = &litex_mmc_ops; + + /* set default sd_clk frequency range based on empirical observations + * of LiteSDCard gateware behavior on typical SDCard media + */ + mmc->f_min = 12.5e6; + mmc->f_max = 50e6; + + ret = mmc_of_parse(mmc); + if (ret) + goto err; + + /* force 4-bit bus_width (only width supported by hardware) */ + mmc->caps &= ~MMC_CAP_8_BIT_DATA; + mmc->caps |= MMC_CAP_4_BIT_DATA; + + /* set default capabilities */ + mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | + MMC_CAP_DRIVER_TYPE_D | + MMC_CAP_CMD23; + mmc->caps2 |= MMC_CAP2_NO_WRITE_PROTECT | + MMC_CAP2_NO_SDIO | + MMC_CAP2_NO_MMC; + + platform_set_drvdata(pdev, host); + + ret = mmc_add_host(mmc); + if (ret < 0) + goto err; + + return 0; + +err: + mmc_free_host(mmc); + return ret; +} + +static int litex_mmc_remove(struct platform_device *pdev) +{ + struct litex_mmc_host *host = dev_get_drvdata(&pdev->dev); + + if (host->irq > 0) + free_irq(host->irq, host->mmc); + mmc_remove_host(host->mmc); + mmc_free_host(host->mmc); + + return 0; +} + +static const struct of_device_id litex_match[] = { + { .compatible = "litex,mmc" }, + { } +}; +MODULE_DEVICE_TABLE(of, litex_match); + +static struct platform_driver litex_mmc_driver = { + .probe = litex_mmc_probe, + .remove = litex_mmc_remove, + .driver = { + .name = "litex-mmc", + .of_match_table = of_match_ptr(litex_match), + }, +}; +module_platform_driver(litex_mmc_driver); + +MODULE_DESCRIPTION("LiteX SDCard driver"); +MODULE_AUTHOR("Antmicro <contact@antmicro.com>"); +MODULE_AUTHOR("Kamil Rakoczy <krakoczy@antmicro.com>"); +MODULE_AUTHOR("Maciej Dudek <mdudek@internships.antmicro.com>"); +MODULE_AUTHOR("Paul Mackerras <paulus@ozlabs.org>"); +MODULE_AUTHOR("Gabriel Somlo <gsomlo@gmail.com>"); +MODULE_LICENSE("GPL v2");