Re: [PATCH v8 2/5] dt-bindings: soc: sophgo: Add Sophgo system control module

[Chen Wang]

On 2024/1/22 16:10, Krzysztof Kozlowski wrote:
> On 18/01/2024 06:29, Chen Wang wrote:
> > On 2024/1/16 19:37, Chen Wang wrote:
> > > On 2024/1/16 18:06, Krzysztof Kozlowski wrote:
> > > > On 16/01/2024 08:21, Chen Wang wrote:
> > > > > From: Chen Wang <unicorn_wang@xxxxxxxxxxx>
> > > > >
> > > > > Add documentation to describe Sophgo System Control for SG2042.
> > > > >
> > > > > Signed-off-by: Chen Wang <unicorn_wang@xxxxxxxxxxx>
> > > > > ---
> > > > >    .../soc/sophgo/sophgo,sg2042-sysctrl.yaml     | 46
> > > > > +++++++++++++++++++
> > > > >    1 file changed, 46 insertions(+)
> > > > >    create mode 100644
> > > > > Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml
> > > > > b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml
> > > > >
> > > > > new file mode 100644
> > > > > index 000000000000..7b50bb56b4cf
> > > > > --- /dev/null
> > > > > +++
> > > > > b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml
> > > > > @@ -0,0 +1,46 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id:
> > > > > http://devicetree.org/schemas/soc/sophgo/sophgo,sg2042-sysctrl.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Sophgo SG2042 SoC system control
> > > > > +
> > > > > +maintainers:
> > > > > +  - Chen Wang <unicorn_wang@xxxxxxxxxxx>
> > > > > +
> > > > > +description:
> > > > > +  The Sophgo system control is a registers block (SYS_CTRL),
> > > > > providing multiple
> > > > > +  low level platform functions like chip configuration, clock
> > > > > control, etc.
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    const: sophgo,sg2042-sysctrl
> > > > > +
> > > > > +  reg:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  clock-controller:
> > > > > +    # Child node
> > > > Drop the comment, it is obvious. It cannot be anything else.
> > > >
> > > > > +    $ref: /schemas/clock/sophgo,sg2042-sysclk.yaml#
> > > > > +    type: object
> > > > Why isn't this merged here? You do not need the child node really...
> > > > unless the clock inputs are specific to that clock controller and you
> > > > will have here more devices? But where are they in such case?
> > > I don't see more devices will be included later. It should be ok to
> > > merge them into one.
> > hi, Krzysztof,
> >
> > After some double check, I find we will have more devices in
> > system-control. For example, in the SYS_CTRL area, there is also a
> > section of registers used to control the "General Purpose Interrupt".
> > The pcie controller of sg2042 will use this interrupt controller which
> > is defined in SYS_CTRL, we will add it in later work.
> I expect then all devices to be documented.

hi, Krzysztof.

First, I'm very sorry for having double-checked with you for this system 
controller and child node issue, but this time I'm sure there should be 
no more child nodes except the clock and interrupt controllers, though 
there are some other registers in SYS_CTRL section, but we will not use 
them till now.

One question, when you say "to be documented", do you mean I need write 
binding/yaml files for other child node? But they exceed the scope of 
this patchset (this patchset is for clock support only). That's why I 
suggest just add clock-controller in this patchset and to add the 
interrupt controller in another patchset for pcie support. This 
mechanism should be suitable for our expansion.

Thanks,

Chen

> Best regards,
> Krzysztof