On Tue, Mar 16, 2021 at 08:37:53PM +0200, Cristian Ciocaltea wrote:

On Tue, Mar 16, 2021 at 09:47:39AM +0530, Manivannan Sadhasivam wrote:

quoted

On Mon, Mar 08, 2021 at 07:18:28PM +0200, Cristian Ciocaltea wrote:

quoted

The following clocks of the Actions Semi Owl S500 SoC have been defined
to use a shared clock factor table 'bisp_factor_table[]': DE[1-2], VCE,
VDE, BISP, SENSOR[0-1]

There are several issues involved in this approach:

* 'bisp_factor_table[]' describes the configuration of a regular 8-rates
  divider, so its usage is redundant. Additionally, judging by the BISP
  clock context, it is incomplete since it maps only 8 out of 12
  possible entries.

* The clocks mentioned above are not identical in terms of the available
  rates, therefore cannot rely on the same factor table. Specifically,
  BISP and SENSOR* are standard 12-rate dividers so their configuration
  should rely on a proper clock div table, while VCE and VDE require a
  factor table that is a actually a subset of the one needed for DE[1-2]
  clocks.

Let's fix this by implementing the following:

* Add new factor tables 'de_factor_table' and 'hde_factor_table' to
  properly handle DE[1-2], VCE and VDE clocks.

* Add a common div table 'std12rate_div_table' for BISP and SENSOR[0-1]
  clocks converted to OWL_COMP_DIV.

* Drop the now unused 'bisp_factor_table[]'.

Nice!

quoted

Additionally, since SENSOR[0-1] are not gated, unset the OWL_GATE_HW
configuration and drop the CLK_IGNORE_UNUSED flag in their definitions.

No. You should not screen the functionality exposed by the hw, that's what the
purpose of these CLK_ flags.

I'm not sure I get this, or maybe I wasn't clear enough with my
explanation regarding the changes to SENSOR clocks: they are not gated
in hardware, hence the statement 'OWL_GATE_HW(CMU_DEVCLKEN0, 14, 0)' 
was invalid and I replaced it with '{ 0 }'.

This clock is gated in hw as per the datasheet. Again, please don't make
judgements based on the vendor code as it is not upto date with HW. I
know it is silly but that's how things are...

Additionally, I assumed the 'CLK_IGNORE_UNUSED' flag makes sense only
for the gated clocks. Do I miss something?

CLK_IGNORE_UNUSED is used by the clk framework to essentially skip
gating the clocks which are turned ON by the bootloader and there is no
other driver using it. But I think you can remove this flag because
there is no reason to leave this specific clock to be ON always.

Thanks,
Mani

quoted

Other than that, this patch looks good to me.

Thanks,
Cristi

[...]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel