linux-kernel - Re: [PATCH V3 1/3] dt-bindings: clk: sprd: Add bindings for ums512 clock controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 24 Apr 2022 23:12:51 +0800
From:   Cixi Geng <gengcixi@...il.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Michael Turquette <mturquette@...libre.com>, sboyd@...nel.org,
        Rob Herring <robh+dt@...nel.org>,
        krzysztof.kozlowski+dt@...aro.org,
        Orson Zhai <orsonzhai@...il.com>,
        "baolin.wang7@...il.com" <baolin.wang7@...il.com>,
        Chunyan Zhang <zhang.lyra@...il.com>,
        linux-clk@...r.kernel.org,
        Devicetree List <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V3 1/3] dt-bindings: clk: sprd: Add bindings for ums512
 clock controller

Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> 于2022年4月24日周日 22:30写道：
>
> On 24/04/2022 16:22, Cixi Geng wrote:
> >>>>
> >>>> Neither here nor later you did not answer the question - why do you need
> >>>> such complex construction, instead of adding syscon to the clock controller?
> >>>>
> >>>> Let me paste again my concerns:
> >>>>
> >>>>   You have nodes with reg but without unit address ("rpll"). These nodes
> >>>>   are modeled as children but they are not children - it's a workaround
> >>>>   for exposing syscon, isn't it? The sc9863a looks like broken design,
> >>>>   so please do not duplicate it here.
> >>>>
> >>>> IOW, sc9863a uses similar pattern as here and the DTS is made wrong.
> >>>> Because of this you need to create complex ways to get the regmap for
> >>>> the clock controller... Why not making it simple? Clock controller with
> >>>> syscon?
> >>>
> >>> I find the history discuss about the sp9863 clock[1] and last
> >>> ums512-clk dt-bindings patch[2] which from chunyan.
> >>> please refer to the reasons below.
> >>>
> >>> These clocks are at the same register range with global registers.
> >>> the registers shared with more than one devices  which  basically
> >>> are multimedia devices. You may noticed that these are all gate
> >>> clocks which are in the global registers  ranges and are used to
> >>> controll the enable status of some devices or some part of devices.
> >>>
> >>> [1] https://lore.kernel.org/all/CAAfSe-s0gcehu0ZDj=FTe5S7CzAHC5mahXBH2fJm7mXS7Xys1Q@mail.gmail.com/#r
> >>> [2] https://lore.kernel.org/all/163425295208.1688384.11023187625793114662@swboyd.mtv.corp.google.com/#r
> >>
> >> Which looks like discussion about different bindings. You had there a
> >> clock controller and additional clock device using "sprd,syscon". Why
> >> the rpll is a subdevice and not a part of clock controller. The same as
> >> all other clocks coming from that clock-controller, right? What is so
> >> special about rpll that is is a separate device, not part of the clock
> >> controller? It's the same address space, isn't it?
> > The hardware spec design these clocks are not belonged to the syscon,
> > the phandle is only used to get virtual  map address for clocks which
> > have the same phsical address base with one syscon.(I don't know the
> > historical reason for this design) It also can wroten a clock sperated from
> > syscon by add the reg which same as syscon. but will lead to a duplicate
> > mapping--one is from the clock,and one is from syscon. which make difficulty
> >  in analyzing some panic problems.
>
> I don't understand still. You said that they do not belong to same
> address space, right? But the sprd,ums512-apahb-gate in this patch or
> mentioned rpll
> (https://elixir.bootlin.com/linux/v5.18-rc3/source/arch/arm64/boot/dts/sprd/sharkl3.dtsi#L106)
> does not reference any other address space. It's entire address space is
> the same as address space of glbregs.
Maybe I didn't describe clearly, what I said is these clocks isn't the
syscom sub-clock.
from chunyan's explain:
 they  are at the same register range with global registers. in
originally we put them
directly onto the bus indeed when submitting the patches for SC9863A
clocks last year,
and it had a private property named 'sprd,syscon' which could provide
regmap for these clocks.
after follow Rob's suggetion we make them a child of the syscon. these
are all gate clocks which
are in the global registers ranges and are used to controll the enable
status of some devices
or some part of devices.
>
> So if it does not belong to the same address space, where is this space
> defined?
>
> Best regards,
> Krzysztof