Message-ID: <YjnSg1NixXzjyXX9@Ansuel-xps.localdomain>
Date:   Tue, 22 Mar 2022 14:43:31 +0100
From:   Ansuel Smith <ansuelsmth@...il.com>
To:     Rob Herring <robh+dt@...nel.org>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        Andy Gross <agross@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, devicetree@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        linux-clk <linux-clk@...r.kernel.org>
Subject: Re: [PATCH v6 00/18] Modernize rest of the krait drivers

On Mon, Mar 21, 2022 at 08:56:13PM -0500, Rob Herring wrote:
> On Mon, Mar 21, 2022 at 6:45 PM Ansuel Smith <ansuelsmth@...il.com> 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

Yes sorry. There was an initial review for the clk driver from v1 to
v2 but nothing else. I was trying to make the Documentation ready while
I wait for a second review of the clk code.

Will wait for clk code review to send v7 hoping it will be the final
version.

-- 
	Ansuel

Powered by blists - more mailing lists