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

[Krzysztof Kozlowski]

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@xxxxxxxxxxx>
>>>
>>> Add documentation to describe Sophgo System Controller for SG2042.
>>>
>>> Signed-off-by: Chen Wang <unicorn_wang@xxxxxxxxxxx>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
>>> ---
>>>   .../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@xxxxxxxxxxx>
>>> +
>>> +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