Hi Wolfram,
On Thu, Oct 21, 2021 at 11:59 AM Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
> > > + return clk_register_divider_table(NULL, name, parent_name, 0, sdnckcr,
> > > + 0, 2, 0, cpg_sd_div_table, &cpg_lock);
> >
> > So the SDn clock can no longer be disabled, as CPG_SD_STP_CK
> > handling is gone?
>
> Yes. I thought we can do it since we had 7f2c2f38c1c0 ("clk: renesas:
> rcar-gen3: Remove stp_ck handling for SDHI") anyhow.
OK.
> > > + /* Fallback for old DTs */
> > > + if (of_device_is_compatible(pdev->dev.of_node, "renesas,rcar-gen3-sdhi"))
> >
> > I think it would be cleaner to check a flag in struct
> > renesas_sdhi_of_data instead.
>
> Because new SoCs with the fallback compatible might show up?
Because it's frowned upon to sprinkle of_device_is_compatible()
calls all over the code, especially if you already use
of_device_get_match_data().
> > > * Some controllers provide a 2nd clock just to run the internal card
> > > * detection logic. Unfortunately, the existing driver architecture does
> >
> > The core looks good to me, but I have to admit I'm no expert on the
> > SDHn/SDn clock relations and the various SDHI transfer modes.
>
> I am really glad you like the changes in general. And you point to the
> reason for this change. All the clock relations of the SDHI transfer
> modes should go into the SDHI driver. Now, we can control SDnH and SDn
> seperately, so the SDHI driver can do the proper things depending on the
> mode and the quirks of the SDHI instance. I really think the clock
> driver part should be as simple as it is with this series.
Good!
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
