Re: [PATCH v6 00/18] Modernize rest of the krait drivers

[Rob Herring]

On Mon, Mar 21, 2022 at 6:45 PM Ansuel Smith <ansuelsmth@xxxxxxxxx> wrote:
>
> This is a follow-up to the ipq806x gcc modernize series. Manu cleanup
> changes and also some discoveries of wrong definition notice only with
> all these conversions.
>
> The first patch is an improvement of the clk_hw_get_parent_index. The
> original idea of clk_hw_get_parent_index was to give a way to access the
> parent index but for some reason the final version limited it to the
> current index. We change it to give the current parent if is not
> provided and to give the requested parent if provided. Any user of this
> function is updated to follow the new implementation.
>
> The patch 2 and 3 are some additional fixes for gcc.
> The first one is a fix that register the pxo and cxo fixed clock only if
> they are not defined in DTS.
> The patch 3 require some explaination. In short is a big HACK to prevent
> kernel panic with this series.
>
> The kpss-xcc driver is a mess.
> The Documentation declare that the clocks should be provided but for some
> reason it was never followed.
> In fact in the ipq8064 DTSI only the clocks for l2cc are declared but
> for cpu0 and cpu1 the clocks are not defined.
> The kpss-xcc driver use parent_names so the clks are ignored and never
> used so till now it wasn't a problem (ignoring the fact that they
> doesn't follow documentation at all)
> On top of that, the l2cc node declare the pxo clock in a really strange
> way. It's declared using the PXO_SRC gcc clock that is never defined in
> the gcc ipq8064 clock table. (the correct way was to declare a fixed
> clock in dts and reference that)
> To prevent any kind of problem we use the patch 3 and provide the clk
> for PXO_SRC in the gcc clock table. We manually provide the clk after
> gcc probe.
>
> Patch 4 is just a minor cleanup where we use the poll macro
>
> Patch 5 is the actually kpss-xcc conversion to parent data
>
> Patch 6-7 should be a fixup of a real conver case
>
> Patch 8 converts the krait-cc to parent_data
> Patch 9 give some love to the code with some minor fixup
> Patch 10 drop the hardcoded safe sel and use the new
> clk_hw_get_parent_index to get the safe parent index.
> (also I discovered that the parent order was wrong)
>
> Patch 11 is an additional fixup to force the reset of the muxes even
> more.
>
> Patch 12-13 are some additiona taken from the qsdk that were missing in
> the upstream driver
>
> Patch 14 converts krait-cc to yaml
>
> Patch 15 add to krait-cc Documentation the L2 clocks
>
> Patch 16 converts the kpss-acc driver to yaml and fix some Documentation
> error
>
> Patch 17 convets the kpss-gcc driver to yaml
>
> Patch 18 finally adds all this stuff to the ipq8064 dtsi (and fix the
> stupid PXO_SRC phandle)
>
> I tested this series on a ipq8064 SoC by running a cache benchmark test
> to make sure the changes are correct and we don't silently cause
> regressions. Also I compared the output of the clk_summary every time
> and we finally have a sane output where the mux are correctly placed in
> the correct parent. (till now we had the cpu aux clock all over the
> place, probably never cause problems but who knows.)
>
> v6:
> - Move dts patch as last patch
> - Address commencts from Rob
> - Fix warning from make dtbs_check
> v5:
> - Address comments from Krzysztof
> v4:
> - Fix more dt-bindings bog errors
> v3:
> - Split Documentation files for kpss and krait-cc
> v2:
> - introduce new API instead of fixing the existing one
> - do not reorganize variables in krait-cc
> - fix some comments error and improve it
> - return better error for patch 7
> - fix missing new line on patch 16

6 versions in a week is too many, especially with the merge window
starting. Please give some time for review of all the patches and by
more than one person. It doesn't look like any of the clk driver
patches have been reviewed since v1 for example.

Rob