Hi Gabriel, On Sat, Dec 4, 2021 at 9:41 PM Gabriel Somlo <gsomlo@xxxxxxxxx> 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! > > Co-developed-by: Kamil Rakoczy <krakoczy@xxxxxxxxxxxx> > Signed-off-by: Kamil Rakoczy <krakoczy@xxxxxxxxxxxx> > Co-developed-by: Maciej Dudek <mdudek@xxxxxxxxxxxxxxxxxxxxxxxx> > Signed-off-by: Maciej Dudek <mdudek@xxxxxxxxxxxxxxxxxxxxxxxx> > Co-developed-by: Paul Mackerras <paulus@xxxxxxxxxx> > Signed-off-by: Paul Mackerras <paulus@xxxxxxxxxx> > Signed-off-by: Gabriel Somlo <gsomlo@xxxxxxxxx> > --- /dev/null > +++ b/drivers/mmc/host/litex_mmc.c > +static void > +litex_set_clk(struct litex_mmc_host *host, unsigned int clk_freq) > +{ > + u32 div = clk_freq ? host->freq / clk_freq : 256; > + > + div = roundup_pow_of_two(div); > + div = min_t(u32, max_t(u32, div, 2), 256); > + dev_info(&host->dev->dev, "sdclk_freq=%d: set to %d via div=%d\n", > + clk_freq, host->freq / div, div); > + litex_write16(host->sdphy + LITEX_PHY_CLOCKERDIV, div); > +} > + > +static void > +litex_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 sdcard clock */ > + if (ios->clock != host->clock) { > + litex_set_clk(host, ios->clock); > + host->clock = ios->clock; It might be better to move the assignment to host->clock into the callee, to avoid it becoming out-of-sync when a second caller is introduced. > + } > +} > +static int > +litex_mmc_probe(struct platform_device *pdev) > +{ > + struct litex_mmc_host *host; > + struct mmc_host *mmc; > + struct device_node *cpu; > + 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; > + > + host->clock = 0; > + cpu = of_get_next_cpu_node(NULL); > + ret = of_property_read_u32(cpu, "clock-frequency", &host->freq); > + of_node_put(cpu); > + if (ret) { > + dev_err(&pdev->dev, "No \"clock-frequency\" property in DT\n"); > + goto err_free_host; > + } This looks fragile. Shouldn't the clock be obtained from a clock property in the mmc device node, pointing to a clock provider? How does the real clock tree look like? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx 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
