On 17 September 2022 22:44:00 GMT+03:00, Christian Marangi [off-list ref] wrote:

On Sat, Sep 17, 2022 at 08:57:44PM +0200, Christian Marangi wrote:

quoted

On Sat, Sep 17, 2022 at 04:45:21PM +0300, Dmitry Baryshkov wrote:

quoted

On Sat, 17 Sept 2022 at 00:54, Christian Marangi [off-list ref] wrote:

quoted

On Fri, Sep 16, 2022 at 11:31:49PM +0300, Dmitry Baryshkov wrote:

quoted

On Fri, 16 Sept 2022 at 23:27, Christian Marangi [off-list ref] wrote:

quoted

On Fri, Sep 16, 2022 at 11:22:17PM +0300, Dmitry Baryshkov wrote:

quoted

On Fri, 16 Sept 2022 at 23:13, Christian Marangi [off-list ref] wrote:

quoted

On Fri, Sep 16, 2022 at 11:06:35PM +0300, Dmitry Baryshkov wrote:

quoted

On Fri, 16 Sept 2022 at 22:43, Christian Marangi [off-list ref] wrote:

quoted

On Fri, Sep 16, 2022 at 02:17:15PM -0500, Rob Herring wrote:

quoted

On Wed, Sep 14, 2022 at 04:22:53PM +0200, Christian Marangi wrote:

quoted

Convert kpss-acc driver Documentation to yaml.
The original Documentation was wrong all along. Fix it while we are
converting it.
The example was wrong as kpss-acc-v2 should only expose the regs but we
don't have any driver that expose additional clocks. The kpss-acc driver
is only specific to v1. For this exact reason, limit all the additional
bindings (clocks, clock-names, clock-output-names and #clock-cells) to
v1 and also flag that these bindings should NOT be used for v2.

Odd that a clock controller has no clocks, but okay.

As said in the commit v2 is only used for regs. v2 it's only used in
arch/arm/mach-qcom/platsmp.c to setup stuff cpu hotplug and bringup.

Should we split the 2 driver? To me the acc naming seems to be just
recycled for v2 and it's not really a clk controller.

So keeping v2 in arm/msm/qcom,kpss-acc-v2.yaml and v1 moved to clock?

I suspect that qcom,kpss-acc-v2 is misnamed as the "clock-controller".
According to msm-3.10, these regions are used by the Krait core
regulators.

Well we need to understand how to handle this... change the compatible
it's a nono for sure. In platsmp.c they are used for cpu power control
so could be that they are actually used to regulators. I would honestly
move v1 to clock and leave v2 to arm/msm but I'm not cetain on what name
to assign to the 2 yaml.

What do you think?

This is fine for me. If somebody gets better understanding of
underlying hardware and works on actually using these blocks, he will
update the bindings.

My only suggestion would be to rename kpss-acc-v2 nodes to
'power-controller@address' and document them so.

Ok so something like this?

    power-controller@f9088000 {
      compatible = "qcom,kpss-acc-v2";
      reg = <0xf9088000 0x1000>,
            <0xf9008000 0x1000>;
    };

(and I will have to fix dtbs warning as they will be unmatched I think.)
Yaml naming:
qcom,kpss-acc-v1.yaml
qcom,kpss-acc-v2.yaml
Right?

Sounds good to me.

I'd even say clock/qcom,kpss-acc-v1.yaml and
arm/msm/qcom,kpss-acc-v2.yaml or maybe power/qcom,kpss-acc-v2.yaml

Wonder if the gcc driver should have the same tretement? It's also a
clock-controller driver that doesn't use clock at all... Do you have
some info about it?

As far as I understand, the kpss-gcc is a normal clock controller,
isn't it? It provides clocks to other devices.

Hi again... Having acc-v2 as power-controller would require to set
#power-domain-cells = <0>;

Why? I don't think so. Rob/Krzysztof, please correct me if I'm wrong.

quoted

Would that be acceptable? Considering it wouldn't expose any PM domain?

About kpss-gcc we have some device that for some reason doesn't have the
required clocks defined in the dts. I checked the related gcc and no PXO
defined and no pll8_vote clock defined. (the affected dts are all listed
in the related Documentation)

No idea how they currently work with the kpss-gcc driver as these
parents are missing. Guess the driver just fails to probe?
So this was the question if you had more info about it... since to me it
seems just another gcc v2 that doesn't expose clocks but it's just a
power-controller just like acc-v2. 

-- 
	Ansuel

(Also sorry for the double email)
I'm checking the regs for apq8084 for example (from the dtsi)
Are we really sure they are power-controller?

It looks like it's a regularor on steroids. See krait-regulator.c and corresponding bindings in msm-3.10/3.14. So I'd use either the regulator or the power-controller (with significant bias towards controller)

Checking the regs it seems they just changed the location and they
placed clock-controller and right after the power-controller.
So one can get confused and say that 0xf9... can be all related to power
controller. I posted the regs for reference.

acc0 0xf9088000 0x1000
saw0 0xf9089000 0x1000

acc1 0xf9098000 0x1000
saw1 0xf9099000 0x1000

acc2 0xf90a8000 0x1000
saw2 0xf90b9000 0x1000

Anyway while at it there seems to be a bit of confusion about the naming
here... We have on ipq8064 and ipq4019 the saw node set as regulator and
with the regulator binding but on msm8974 and apq8084 the saw node set
as power-controller (with the l2 node with the regulator binding).

Think we should chose a name and fix every dts.
So the main question here is...
Should we keep acc as clock-controller or change it to power-controller
(for v2)?

Should we change saw node to regulator or power-controller?

From what I know acc are used to enable the cpu so it seems sane to keep
them as clock-controller (even if v2 doesn't export clock)
Saw node handle power (and in theory even low power state) so it seems
sane to change them to power-controller.

Currently we have no warning for saw node as they are not converted to
yaml but as soon as someone convert the txt to yaml then we will have
all sort of inconsistency so better take a decision now instead of
convert saw to yaml and then change acc node again to fix them for good.

The saw is definitely a bigger thing than just a regularor (or a set of them). It is used to control pmics, it handles low-power states, so `power-controller'.

-- 
With best wishes
Dmitry