[...] > >> > >> > >>> In regards to the mmc data part, I suggest to drop the > >>> ->set_tap_delay() callback, but rather use a boolean flag to indicate > >>> whether clock phases needs to be changed for the variant. Potentially > >>> that could even be skipped and instead call clk_set_phase() > >>> unconditionally, as the clock core deals fine with clock providers > >>> that doesn't support the ->set_phase() callback. > >> > >> In connection to another version of this driver for latest Xilinx chip > >> it would be better to keep set_tap_delay callback in the driver. The > >> reason is that new chip/ip is capable to setup tap delays directly > >> without asking firmware to do it. That's why for versal IP there is a > >> need to call different setup_tap_delay function. > > > > The ->set_tap_delay() callback is for ZyncMp pointing to > > sdhci_arasan_zynqmp_set_tap_delay(). This function calls the > > clk_set_phase() API. > > > > What does ->set_tap_delay() do for the latest version? > > There is different set of default tap delays which should be programmed > and it is done just via writing to registers which are the part of > controller address space. Okay, I see. Not sure what makes most sense to do here, but it sounds to me like another ->set_phase() callback should be implemented for the clock provider. In other words, calling clk_set_phase() should continue to works just fine for this case as well. If it turns out to be inconvenient, we can always add the ->set_tap_delay() at a later point when it makes more sense. [...] Kind regards Uffe