Re: [PATCH v7 1/4] dt-bindings: soc: sophgo: Add Sophgo system control module

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Chen Wang <unicorn_wang@outlook.com>,
	Chen Wang <unicornxw@gmail.com>,
	aou@eecs.berkeley.edu, chao.wei@sophgo.com, conor@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, mturquette@baylibre.com,
	palmer@dabbelt.com, paul.walmsley@sifive.com,
	richardcochran@gmail.com, robh+dt@kernel.org, sboyd@kernel.org,
	devicetree@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
	haijiao.liu@sophgo.com, xiaoguang.xing@sophgo.com,
	guoren@kernel.org, jszhang@kernel.org, inochiama@outlook.com,
	samuel.holland@sifive.com
Subject: Re: [PATCH v7 1/4] dt-bindings: soc: sophgo: Add Sophgo system control module
Date: Mon, 8 Jan 2024 20:36:33 +0100	[thread overview]
Message-ID: <1e1ef0a0-6639-4a4d-9b4e-50bcee3fb3c5@linaro.org> (raw)
In-Reply-To: <MA0P287MB2822E54A6DD36F914DB56E98FE6B2@MA0P287MB2822.INDP287.PROD.OUTLOOK.COM>

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv