Re: [PATCH v2] clk: keystone: sci-clk: Adding support for non contiguous clocks

From: Kumar, Udit
Date: Wed Feb 07 2024 - 00:34:39 EST



On 2/6/2024 9:24 PM, Kamlesh Gurudasani wrote:
"Kumar, Udit" <u-kumar1@xxxxxx> writes:

get_freq is a bit expensive as it has to walk the clock tree to find
the clock frequency (at least the first time?). just wondering if
there is lighter alternative here?

How about get_clock? Doesn't read the registers at least.
Said API needs, some flags to be passed,

Can those flag be set to zero, Chandru ?

get_clock doesn't require any flags to be passed.

May be firmware does not need it but  I was referring to

https://elixir.bootlin.com/linux/latest/source/drivers/clk/keystone/sci-clk.c#L78
Just took a look,

I now understand the reason for confusion,

#define TI_SCI_MSG_SET_CLOCK_STATE 0x0100
#define TI_SCI_MSG_GET_CLOCK_STATE 0x0101

cops->get_clock = ti_sci_cmd_get_clock; --> refers to
TI_SCI_MSG_SET_CLOCK_STATE
That's why we are passing the flag from linux for get_clock

Linux is using terminology of get/put.

As Chandru pointed, we don't have to pass flags, cause he is refering
to TI_SCI_MSG_GET_CLOCK_STATE

Below functions passes TI_SCI_MSG_GET_CLOCK_STATE to DM, which is what
we actually want.
cops->is_auto = ti_sci_cmd_clk_is_auto;
cops->is_on = ti_sci_cmd_clk_is_on;
cops->is_off = ti_sci_cmd_clk_is_off;


I think calling ti_sci_cmd_clk_is_auto should be good . other functions needs current state and requested state.

Chandru ?


Which should be safe to call, Chandru can confirm.

Regards,
Kamlesh




Regards,
Kamlesh