Re: [RFC PATCH v3 25/35] Documentation/devicetree/bindings/sh/cpus.yaml: Add SH CPU.

["D. Jeff Dionne" <djeffdionne@gmail.com>]

Hi Sato-san,

We must not imply that Renesas is responsible for J2, or that it is a sanctioned SH core.

J-Core has the responsibility for maintenance of those SH ISA compatible cores.

J.

> On Oct 25, 2023, at 20:14, Yoshinori Sato <ysato@users.sourceforge.jp> wrote:
> 
> On Wed, 18 Oct 2023 23:27:43 +0900,
> Geert Uytterhoeven wrote:
>> 
>> Hi Sato-san,
>> 
>> On Sat, Oct 14, 2023 at 4:54 PM Yoshinori Sato
>> <ysato@users.sourceforge.jp> wrote:
>>> Renesas SuperH binding definition.
>>> 
>>> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
>> 
>> Thanks for your patch!
>> 
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/sh/cpus.yaml
>>> @@ -0,0 +1,45 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/sh/cpus.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Renesas SuperH CPUs
>>> +
>>> +maintainers:
>>> +  - Yoshinori Sato <ysato@users.sourceforge.jp>
>>> +
>>> +description: |+
>>> +  The device tree allows to describe the layout of CPUs in a system through
>>> +  the "cpus" node, which in turn contains a number of subnodes (ie "cpu")
>>> +  defining properties for every cpu.
>>> +
>>> +  Bindings for CPU nodes follow the Devicetree Specification, available from:
>>> +
>>> +  https://www.devicetree.org/specifications/
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>> 
>> Missing
>> 
>>    - jcore,j2
>> 
>>> +          - renesas,sh4
>> 
>> 
>>> +      - const: renesas,sh
>> 
>> I see arch/sh/boot/dts/j2_mimas_v2.dts lacks the fallback to
>> "renesas,sh", though.
>> Is there a common base of instructions that are available on all SH cores?
> 
> The base instruction set is sh2.
> Before that, there is sh1, but this is not compatible with Linux.
> I think it would be a good idea to change this to "renesas,sh2",
> but the SH7619 (SH2 CPU) would look like the following.
> cpus {
> 	cpu: cpu@0 {
>        	compatible = "renesas,sh2", "renesas,sh2";
>        };
> };
> 
>> Missing reg property.
>> Missing "device_type: true".
>> 
>>> +
>>> +  clock-frequency:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description: |
>>> +      CPU core clock freqency.
>> 
>> Perhaps a "clocks" property instead, or as an alternative?
>> 
>> On sh7750, you do have
>> 
>>    clocks = <&cpg SH7750_CPG_ICK>;
>> 
>>> +
>>> +required:
>>> +  - compatible
>>> +
>>> +additionalProperties: true
>>> +
>>> +examples:
>>> +  - |
>>> +        cpus {
>> 
>> make dt_binding_check
>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/sh/cpus.yaml:
>> 
>> Documentation/devicetree/bindings/sh/cpus.example.dtb: cpus:
>> '#address-cells' is a required property
>>        from schema $id: http://devicetree.org/schemas/cpus.yaml#
>>        Documentation/devicetree/bindings/sh/cpus.example.dtb: cpus:
>> '#size-cells' is a required property
>> from schema $id: http://devicetree.org/schemas/cpus.yaml#
>> 
>>> +                cpu: cpu@0 {
>>> +                      compatible = "renesas,sh4", "renesas,sh";
>> 
>> Documentation/devicetree/bindings/sh/cpus.example.dts:19.28-21.19:
>> Warning (unit_address_vs_reg): /example-0/cpus/cpu@0: node has a unit
>> name, but no reg or ranges property
>> Documentation/devicetree/bindings/sh/cpus.example.dtb: cpus: cpu@0:
>> 'cache-level' is a required property
>>        from schema $id: http://devicetree.org/schemas/cpus.yaml#
>> 
>>> +                };
>>> +        };
>>> +...
>> 
>> Gr{oetje,eeting}s,
>> 
>>                        Geert
>> 
>> -- 
>> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>> 
>> In personal conversations with technical people, I call myself a hacker. But
>> when I'm talking to journalists I just say "programmer" or something like that.
>>                                -- Linus Torvalds
> 
> -- 
> Yosinori Sato