Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Tue, 7 Dec 2021 15:16:22 +0100 | Subject | Re: [PATCH v2 3/3] mmc: Add driver for LiteX's LiteSDCard interface |
| |
Hi Gabriel,
On Tue, Dec 7, 2021 at 3:10 PM Gabriel L. Somlo <gsomlo@gmail.com> wrote: > On Mon, Dec 06, 2021 at 01:24:49PM +0100, Geert Uytterhoeven wrote: > > On Sat, Dec 4, 2021 at 9:41 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! > > > > > > Co-developed-by: Kamil Rakoczy <krakoczy@antmicro.com> > > > Signed-off-by: Kamil Rakoczy <krakoczy@antmicro.com> > > > Co-developed-by: Maciej Dudek <mdudek@internships.antmicro.com> > > > Signed-off-by: Maciej Dudek <mdudek@internships.antmicro.com> > > > Co-developed-by: Paul Mackerras <paulus@ozlabs.org> > > > Signed-off-by: Paul Mackerras <paulus@ozlabs.org> > > > Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
> > > + 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? > > In a full LiteX SoC, the main sys_clock is used for cpu, buses, and as a > input source for peripherals such as LiteSDCard (which then further > subdivides it to obtain a 12.5--50.0 MHz sd_clock. > > But since we're considering supporting LiteSDCard as an independent IP > block, the "source clock" frequency should indeed be specified as a DT > property in the MMC device node. (I'll have to add that to the list of > updates for litex_json2dts_linux.py as well, once we settle on what it > will look like -- I'll try to make the change and corresponding update > to the devicetree bindings doc for v3). > > LMK what you think.
Ideally there should be a "clocks" property with a phandle pointing to a clock controller node (compatible with "litex,clk").
How does drivers/tty/serial/liteuart.c handle this? Oh, it doesn't ;-)
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
| |