| Message ID | acebc88db3e5fcd2a2607b56842af7443a6e1289.1704694903.git.unicorn_wang@outlook.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | riscv: sophgo: add clock support for sg2042 | expand |
On 08/01/2024 07:48, Chen Wang wrote: > From: Chen Wang <unicorn_wang@outlook.com> > > Add documentation to describe Sophgo System Controller for SG2042. > > Signed-off-by: Chen Wang <unicorn_wang@outlook.com> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > .../soc/sophgo/sophgo,sg2042-sysctrl.yaml | 34 +++++++++++++++++++ > 1 file changed, 34 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..1ec1eaa55598 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml > @@ -0,0 +1,34 @@ > +# 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 controller > + > +maintainers: > + - Chen Wang <unicorn_wang@outlook.com> > + > +description: > + The Sophgo SG2042 SoC system controller provides register information such > + as offset, mask and shift that can be used by other modules, such as clocks. "offset, mask and shift" is not a register information stored in syscons. Are you really sure, that your system controller hardware stores offsets of some other registers? Show as some example of such offsets, masks and shifts provided by this hardware. Best regards, Krzysztof
On 2024/1/8 15:03, Krzysztof Kozlowski wrote: > On 08/01/2024 07:48, Chen Wang wrote: >> From: Chen Wang <unicorn_wang@outlook.com> >> >> Add documentation to describe Sophgo System Controller for SG2042. >> >> Signed-off-by: Chen Wang <unicorn_wang@outlook.com> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> --- >> .../soc/sophgo/sophgo,sg2042-sysctrl.yaml | 34 +++++++++++++++++++ >> 1 file changed, 34 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..1ec1eaa55598 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml >> @@ -0,0 +1,34 @@ >> +# 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 controller >> + >> +maintainers: >> + - Chen Wang <unicorn_wang@outlook.com> >> + >> +description: >> + The Sophgo SG2042 SoC system controller provides register information such >> + as offset, mask and shift that can be used by other modules, such as clocks. > "offset, mask and shift" is not a register information stored in > syscons. Are you really sure, that your system controller hardware > stores offsets of some other registers? > > Show as some example of such offsets, masks and shifts provided by this > hardware. The system control module is defined here: https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst. It contains some registers related to pll and gates. Some other clocks registars are defined in https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst. memory-map is defined in https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/mmap.rst > > > Best regards, > Krzysztof >
On 08/01/2024 08:20, Chen Wang wrote: > > On 2024/1/8 15:03, Krzysztof Kozlowski wrote: >> On 08/01/2024 07:48, Chen Wang wrote: >>> From: Chen Wang <unicorn_wang@outlook.com> >>> >>> Add documentation to describe Sophgo System Controller for SG2042. >>> >>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>> --- >>> .../soc/sophgo/sophgo,sg2042-sysctrl.yaml | 34 +++++++++++++++++++ >>> 1 file changed, 34 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..1ec1eaa55598 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml >>> @@ -0,0 +1,34 @@ >>> +# 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 controller >>> + >>> +maintainers: >>> + - Chen Wang <unicorn_wang@outlook.com> >>> + >>> +description: >>> + The Sophgo SG2042 SoC system controller provides register information such >>> + as offset, mask and shift that can be used by other modules, such as clocks. >> "offset, mask and shift" is not a register information stored in >> syscons. Are you really sure, that your system controller hardware >> stores offsets of some other registers? >> >> Show as some example of such offsets, masks and shifts provided by this >> hardware. > > The system control module is defined here: > https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst. > It contains some registers related to pll and gates. I do not see there registers providing shifts and offsets... just values. > > Some other clocks registars are defined in > https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst. > > memory-map is defined in > https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/mmap.rst Please fix the wording because it does not make sense. System controller does not provide register information. Your datasheet provides register information. Best regards, Krzysztof
On 2024/1/9 3:36, Krzysztof Kozlowski wrote: > On 08/01/2024 08:20, Chen Wang wrote: >> On 2024/1/8 15:03, Krzysztof Kozlowski wrote: >>> On 08/01/2024 07:48, Chen Wang wrote: >>>> From: Chen Wang <unicorn_wang@outlook.com> >>>> >>>> Add documentation to describe Sophgo System Controller for SG2042. >>>> >>>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com> >>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>> --- >>>> .../soc/sophgo/sophgo,sg2042-sysctrl.yaml | 34 +++++++++++++++++++ >>>> 1 file changed, 34 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..1ec1eaa55598 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml >>>> @@ -0,0 +1,34 @@ >>>> +# 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 controller >>>> + >>>> +maintainers: >>>> + - Chen Wang <unicorn_wang@outlook.com> >>>> + >>>> +description: >>>> + The Sophgo SG2042 SoC system controller provides register information such >>>> + as offset, mask and shift that can be used by other modules, such as clocks. >>> "offset, mask and shift" is not a register information stored in >>> syscons. Are you really sure, that your system controller hardware >>> stores offsets of some other registers? >>> >>> Show as some example of such offsets, masks and shifts provided by this >>> hardware. >> The system control module is defined here: >> https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst. >> It contains some registers related to pll and gates. > I do not see there registers providing shifts and offsets... just values. Let me first clarify more what the "offset"/"shift"/"mask" I meant, Use https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst#mpll_control-offset-0x0e8 as example, this register is used to control Main PLL: - Offset: 0x0E8, to my understand, it is the offest between this MPLL_CONTROL register and the start of system-control base - Shift: the conlumn "LSB", for example, to locate the field MPLL_CONTROL.MPLL_FBDIV, we can first use system-control base + offset to get the address of MPLL_CONTROL, then use LSB(16) as shift to get the start position of this field. - Mask: still use MPLL_CONTROL.MPLL_FBDIV as example, use MSB(27) and LSB(16), this means the width of this field is 12 and with this we can get bit-mask for this field. For SG2042, IC define clock related registers in two parts, one is in https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst, and another in https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst. I define the system control node in DTS and just treat it as a block of registers array and after regmap I can get some registers address such as MPLL_CONTROL to access it from my driver code. >> Some other clocks registars are defined in >> https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst. >> >> memory-map is defined in >> https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/mmap.rst > > Please fix the wording because it does not make sense. System controller > does not provide register information. Your datasheet provides register > information. Sorry, I don't understand why you say "System controller does not provide register information."? As I explained above, I did see the information about these clock-related registers from the system control module, such as the offset/shift/mask I mentioned above. That's why I wrote "that can be used by other modules, such as clocks". How should I express, please enlighten me. Thanks, Chen > Best regards, > Krzysztof >
On 2024/1/9 3:36, Krzysztof Kozlowski wrote: > On 08/01/2024 08:20, Chen Wang wrote: >> On 2024/1/8 15:03, Krzysztof Kozlowski wrote: >>> On 08/01/2024 07:48, Chen Wang wrote: >>>> From: Chen Wang <unicorn_wang@outlook.com> >>>> >>>> Add documentation to describe Sophgo System Controller for SG2042. >>>> >>>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com> >>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>> --- >>>> .../soc/sophgo/sophgo,sg2042-sysctrl.yaml | 34 +++++++++++++++++++ >>>> 1 file changed, 34 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..1ec1eaa55598 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml >>>> @@ -0,0 +1,34 @@ >>>> +# 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 controller >>>> + >>>> +maintainers: >>>> + - Chen Wang <unicorn_wang@outlook.com> >>>> + >>>> +description: >>>> + The Sophgo SG2042 SoC system controller provides register information such >>>> + as offset, mask and shift that can be used by other modules, such as clocks. >>> "offset, mask and shift" is not a register information stored in >>> syscons. Are you really sure, that your system controller hardware >>> stores offsets of some other registers? >>> >>> Show as some example of such offsets, masks and shifts provided by this >>> hardware. >> The system control module is defined here: >> https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst. >> It contains some registers related to pll and gates. > I do not see there registers providing shifts and offsets... just values. > >> Some other clocks registars are defined in >> https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst. >> >> memory-map is defined in >> https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/mmap.rst > > Please fix the wording because it does not make sense. System controller > does not provide register information. Your datasheet provides register > information. Could it be that what I said "that can be used by other modules, such as clocks." may cause misunderstanding. I plan to change it to "The Sophgo SG2042 SoC system controller provides register information such as offset, mask and shift to configure related modules such as clock." Is this better? > > Best regards, > Krzysztof >
On 09/01/2024 09:52, Chen Wang wrote: > > On 2024/1/9 3:36, Krzysztof Kozlowski wrote: >> On 08/01/2024 08:20, Chen Wang wrote: >>> On 2024/1/8 15:03, Krzysztof Kozlowski wrote: >>>> On 08/01/2024 07:48, Chen Wang wrote: >>>>> From: Chen Wang <unicorn_wang@outlook.com> >>>>> >>>>> Add documentation to describe Sophgo System Controller for SG2042. >>>>> >>>>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com> >>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>>> --- >>>>> .../soc/sophgo/sophgo,sg2042-sysctrl.yaml | 34 +++++++++++++++++++ >>>>> 1 file changed, 34 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..1ec1eaa55598 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml >>>>> @@ -0,0 +1,34 @@ >>>>> +# 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 controller >>>>> + >>>>> +maintainers: >>>>> + - Chen Wang <unicorn_wang@outlook.com> >>>>> + >>>>> +description: >>>>> + The Sophgo SG2042 SoC system controller provides register information such >>>>> + as offset, mask and shift that can be used by other modules, such as clocks. >>>> "offset, mask and shift" is not a register information stored in >>>> syscons. Are you really sure, that your system controller hardware >>>> stores offsets of some other registers? >>>> >>>> Show as some example of such offsets, masks and shifts provided by this >>>> hardware. >>> The system control module is defined here: >>> https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst. >>> It contains some registers related to pll and gates. >> I do not see there registers providing shifts and offsets... just values. >> >>> Some other clocks registars are defined in >>> https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst. >>> >>> memory-map is defined in >>> https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/mmap.rst >> >> Please fix the wording because it does not make sense. System controller >> does not provide register information. Your datasheet provides register >> information. > > Could it be that what I said "that can be used by other modules, such as modules as Linux modules should not be involved in this description. > clocks." may cause misunderstanding. I plan to change it to "The Sophgo > SG2042 SoC system controller provides register information such as > offset, mask and shift to configure related modules such as clock." Is > this better? > Still does not make sense. To provide "offset" means that some other hardware reads sophgo module to get the value of offset. That's not the case here. Best regards, Krzysztof
On 2024/1/9 16:56, Krzysztof Kozlowski wrote: > On 09/01/2024 09:52, Chen Wang wrote: >> On 2024/1/9 3:36, Krzysztof Kozlowski wrote: >>> On 08/01/2024 08:20, Chen Wang wrote: >>>> On 2024/1/8 15:03, Krzysztof Kozlowski wrote: >>>>> On 08/01/2024 07:48, Chen Wang wrote: >>>>>> From: Chen Wang <unicorn_wang@outlook.com> >>>>>> >>>>>> Add documentation to describe Sophgo System Controller for SG2042. >>>>>> >>>>>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com> >>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>>>> --- >>>>>> .../soc/sophgo/sophgo,sg2042-sysctrl.yaml | 34 +++++++++++++++++++ >>>>>> 1 file changed, 34 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..1ec1eaa55598 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml >>>>>> @@ -0,0 +1,34 @@ >>>>>> +# 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 controller >>>>>> + >>>>>> +maintainers: >>>>>> + - Chen Wang <unicorn_wang@outlook.com> >>>>>> + >>>>>> +description: >>>>>> + The Sophgo SG2042 SoC system controller provides register information such >>>>>> + as offset, mask and shift that can be used by other modules, such as clocks. >>>>> "offset, mask and shift" is not a register information stored in >>>>> syscons. Are you really sure, that your system controller hardware >>>>> stores offsets of some other registers? >>>>> >>>>> Show as some example of such offsets, masks and shifts provided by this >>>>> hardware. >>>> The system control module is defined here: >>>> https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst. >>>> It contains some registers related to pll and gates. >>> I do not see there registers providing shifts and offsets... just values. >>> >>>> Some other clocks registars are defined in >>>> https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst. >>>> >>>> memory-map is defined in >>>> https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/mmap.rst >>> Please fix the wording because it does not make sense. System controller >>> does not provide register information. Your datasheet provides register >>> information. >> Could it be that what I said "that can be used by other modules, such as > modules as Linux modules should not be involved in this description. > >> clocks." may cause misunderstanding. I plan to change it to "The Sophgo >> SG2042 SoC system controller provides register information such as >> offset, mask and shift to configure related modules such as clock." Is >> this better? >> > Still does not make sense. To provide "offset" means that some other > hardware reads sophgo module to get the value of offset. That's not the > case here. I'm probably starting to understand what you mean. How about changing it to the following? The Sophgo system controller is a registers block, providing multiple low level platform functions like chip configuration, clock control, etc. > > Best regards, > Krzysztof >
On 10/01/2024 01:44, Chen Wang wrote: >>> clocks." may cause misunderstanding. I plan to change it to "The Sophgo >>> SG2042 SoC system controller provides register information such as >>> offset, mask and shift to configure related modules such as clock." Is >>> this better? >>> >> Still does not make sense. To provide "offset" means that some other >> hardware reads sophgo module to get the value of offset. That's not the >> case here. > > I'm probably starting to understand what you mean. How about changing it > to the following? > > The Sophgo system controller is a registers block, providing multiple > low level platform functions like chip configuration, clock control, etc. Yes, that sounds good. Best regards, Krzysztof
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..1ec1eaa55598 --- /dev/null +++ b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml @@ -0,0 +1,34 @@ +# 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 controller + +maintainers: + - Chen Wang <unicorn_wang@outlook.com> + +description: + The Sophgo SG2042 SoC system controller provides register information such + as offset, mask and shift that can be used by other modules, such as clocks. + +properties: + compatible: + const: sophgo,sg2042-sysctrl + + reg: + maxItems: 1 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + system-controller@30010000 { + compatible = "sophgo,sg2042-sysctrl"; + reg = <0x30010000 0x1000>; + };