diff mbox series

[v2,7/8] du-bindings: mips: cpu: Add img,mips compatible

Message ID 20240612-boston-syscon-v2-7-9f8e1a07fa63@flygoat.com (mailing list archive)
State Superseded
Headers show
Series MIPS: Boston: Fix syscon devicetree binding and node | expand

Commit Message

Jiaxun Yang June 12, 2024, 11:56 a.m. UTC
This compatible is used by boston.dts.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
note: This is a wildcard compatible for all MIPS CPUs,
      I think we should use something like "riscv" for riscv.
      Should I add something like "mips" or "mips,cpu"?
---
 Documentation/devicetree/bindings/mips/cpus.yaml | 1 +
 1 file changed, 1 insertion(+)

Comments

Conor Dooley June 12, 2024, 4:39 p.m. UTC | #1
On Wed, Jun 12, 2024 at 12:56:26PM +0100, Jiaxun Yang wrote:
> This compatible is used by boston.dts.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> note: This is a wildcard compatible for all MIPS CPUs,
>       I think we should use something like "riscv" for riscv.

riscv systems, other than simulators etc are not meant to use the
"riscv" compatible. All of the real CPUs use "vendor,cpu", "riscv".
I'd suggest you add specific compatibles for your CPUs.

Thanks
Conor.

>       Should I add something like "mips" or "mips,cpu"?
> ---
>  Documentation/devicetree/bindings/mips/cpus.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/mips/cpus.yaml b/Documentation/devicetree/bindings/mips/cpus.yaml
> index a85137add668..7f9513f2a540 100644
> --- a/Documentation/devicetree/bindings/mips/cpus.yaml
> +++ b/Documentation/devicetree/bindings/mips/cpus.yaml
> @@ -24,6 +24,7 @@ properties:
>        - brcm,bmips5000
>        - brcm,bmips5200
>        - img,i6500
> +      - img,mips
>        - ingenic,xburst-fpu1.0-mxu1.1
>        - ingenic,xburst-fpu2.0-mxu2.0
>        - ingenic,xburst-mxu1.0
> 
> -- 
> 2.43.0
>
Jiaxun Yang June 12, 2024, 4:59 p.m. UTC | #2
在2024年6月12日六月 下午5:39,Conor Dooley写道:
> On Wed, Jun 12, 2024 at 12:56:26PM +0100, Jiaxun Yang wrote:
>> This compatible is used by boston.dts.
>> 
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>> note: This is a wildcard compatible for all MIPS CPUs,
>>       I think we should use something like "riscv" for riscv.
>
> riscv systems, other than simulators etc are not meant to use the
> "riscv" compatible. All of the real CPUs use "vendor,cpu", "riscv".
> I'd suggest you add specific compatibles for your CPUs.

Boston can be combined with many different CPUs, thus we need to have
such compatibles.

Thanks
>
> Thanks
> Conor.
>
[...]
Rob Herring (Arm) June 13, 2024, 6:59 p.m. UTC | #3
On Wed, Jun 12, 2024 at 05:59:24PM +0100, Jiaxun Yang wrote:
> 
> 
> 在2024年6月12日六月 下午5:39,Conor Dooley写道:
> > On Wed, Jun 12, 2024 at 12:56:26PM +0100, Jiaxun Yang wrote:
> >> This compatible is used by boston.dts.
> >> 
> >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> >> ---
> >> note: This is a wildcard compatible for all MIPS CPUs,
> >>       I think we should use something like "riscv" for riscv.
> >
> > riscv systems, other than simulators etc are not meant to use the
> > "riscv" compatible. All of the real CPUs use "vendor,cpu", "riscv".
> > I'd suggest you add specific compatibles for your CPUs.
> 
> Boston can be combined with many different CPUs, thus we need to have
> such compatibles.

Then you'll need different DTs. Different h/w, different DT.

No way we're taking a generic compatible like this.

Rob
Jiaxun Yang June 13, 2024, 7:40 p.m. UTC | #4
在2024年6月13日六月 下午7:59,Rob Herring写道:
> On Wed, Jun 12, 2024 at 05:59:24PM +0100, Jiaxun Yang wrote:
>> 
>> 
>> 在2024年6月12日六月 下午5:39,Conor Dooley写道:
>> > On Wed, Jun 12, 2024 at 12:56:26PM +0100, Jiaxun Yang wrote:
>> >> This compatible is used by boston.dts.
>> >> 
>> >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> >> ---
>> >> note: This is a wildcard compatible for all MIPS CPUs,
>> >>       I think we should use something like "riscv" for riscv.
>> >
>> > riscv systems, other than simulators etc are not meant to use the
>> > "riscv" compatible. All of the real CPUs use "vendor,cpu", "riscv".
>> > I'd suggest you add specific compatibles for your CPUs.
>> 
>> Boston can be combined with many different CPUs, thus we need to have
>> such compatibles.
>
> Then you'll need different DTs. Different h/w, different DT.

The board have 9 CPU types in total, with hundreds of different possible
CPU topologies. Maintaining separate DT for them seems impossible in kernel.

We can potentially patch this in bootloader, but for existing firmware it's
being doing like this for years. I can see for RISC-V QEMU generated DTB is
using a single "riscv" compatible and I do think it's a similar problem.

I think it's better to document it and warn people only to use it in limited
circumstances, instead of keeping such usage in grey area.

Thanks

>
> No way we're taking a generic compatible like this.
>
> Rob
Conor Dooley June 15, 2024, 12:28 p.m. UTC | #5
On Thu, Jun 13, 2024 at 08:40:18PM +0100, Jiaxun Yang wrote:
> 
> 
> 在2024年6月13日六月 下午7:59,Rob Herring写道:
> > On Wed, Jun 12, 2024 at 05:59:24PM +0100, Jiaxun Yang wrote:
> >> 
> >> 
> >> 在2024年6月12日六月 下午5:39,Conor Dooley写道:
> >> > On Wed, Jun 12, 2024 at 12:56:26PM +0100, Jiaxun Yang wrote:
> >> >> This compatible is used by boston.dts.
> >> >> 
> >> >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> >> >> ---
> >> >> note: This is a wildcard compatible for all MIPS CPUs,
> >> >>       I think we should use something like "riscv" for riscv.
> >> >
> >> > riscv systems, other than simulators etc are not meant to use the
> >> > "riscv" compatible. All of the real CPUs use "vendor,cpu", "riscv".
> >> > I'd suggest you add specific compatibles for your CPUs.
> >> 
> >> Boston can be combined with many different CPUs, thus we need to have
> >> such compatibles.
> >
> > Then you'll need different DTs. Different h/w, different DT.
> 
> The board have 9 CPU types in total, with hundreds of different possible
> CPU topologies. Maintaining separate DT for them seems impossible in kernel.

But you could definitely add 9 different compatibles for each of these
different CPUs.

> We can potentially patch this in bootloader, but for existing firmware it's
> being doing like this for years. I can see for RISC-V QEMU generated DTB is
> using a single "riscv" compatible and I do think it's a similar problem.

That "riscv" compatible is only supposed to be used for
simulators/software models. Real CPUs are not meant to use it. AFAICT,
your boston is a real platform, even if the CPUs are implemented on an
FPGA they should still have one. If you take the OpenC906 RISC-V CPU and
put it on an FPGA, you're still meant to put "thead,c906" in your DT.

> I think it's better to document it and warn people only to use it in limited
> circumstances, instead of keeping such usage in grey area.
> 
> >
> > No way we're taking a generic compatible like this.
Jiaxun Yang June 15, 2024, 7:16 p.m. UTC | #6
在2024年6月15日六月 下午1:28,Conor Dooley写道:
> On Thu, Jun 13, 2024 at 08:40:18PM +0100, Jiaxun Yang wrote:
>> 
>> 
>> 在2024年6月13日六月 下午7:59,Rob Herring写道:
>> > On Wed, Jun 12, 2024 at 05:59:24PM +0100, Jiaxun Yang wrote:
>> >> 
>> >> 
>> >> 在2024年6月12日六月 下午5:39,Conor Dooley写道:
>> >> > On Wed, Jun 12, 2024 at 12:56:26PM +0100, Jiaxun Yang wrote:
>> >> >> This compatible is used by boston.dts.
>> >> >> 
>> >> >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> >> >> ---
>> >> >> note: This is a wildcard compatible for all MIPS CPUs,
>> >> >>       I think we should use something like "riscv" for riscv.
>> >> >
>> >> > riscv systems, other than simulators etc are not meant to use the
>> >> > "riscv" compatible. All of the real CPUs use "vendor,cpu", "riscv".
>> >> > I'd suggest you add specific compatibles for your CPUs.
>> >> 
>> >> Boston can be combined with many different CPUs, thus we need to have
>> >> such compatibles.
>> >
>> > Then you'll need different DTs. Different h/w, different DT.
>> 
>> The board have 9 CPU types in total, with hundreds of different possible
>> CPU topologies. Maintaining separate DT for them seems impossible in kernel.
>
> But you could definitely add 9 different compatibles for each of these
> different CPUs.

They are already in current bindings, but we need a default one to fill
in kernel dts.

>
>> We can potentially patch this in bootloader, but for existing firmware it's
>> being doing like this for years. I can see for RISC-V QEMU generated DTB is
>> using a single "riscv" compatible and I do think it's a similar problem.
>
> That "riscv" compatible is only supposed to be used for
> simulators/software models. Real CPUs are not meant to use it. AFAICT,
> your boston is a real platform, even if the CPUs are implemented on an
> FPGA they should still have one. If you take the OpenC906 RISC-V CPU and
> put it on an FPGA, you're still meant to put "thead,c906" in your DT.
>

And sadly, boston is also the platform used by MIPS internal emulators :-(

Thanks
- Jiaxun
>
> 附件:
> * signature.asc
Conor Dooley June 19, 2024, 6:13 p.m. UTC | #7
On Sat, Jun 15, 2024 at 08:16:11PM +0100, Jiaxun Yang wrote:
> 
> 
> 在2024年6月15日六月 下午1:28,Conor Dooley写道:
> > On Thu, Jun 13, 2024 at 08:40:18PM +0100, Jiaxun Yang wrote:
> >> 
> >> 
> >> 在2024年6月13日六月 下午7:59,Rob Herring写道:
> >> > On Wed, Jun 12, 2024 at 05:59:24PM +0100, Jiaxun Yang wrote:
> >> >> 
> >> >> 
> >> >> 在2024年6月12日六月 下午5:39,Conor Dooley写道:
> >> >> > On Wed, Jun 12, 2024 at 12:56:26PM +0100, Jiaxun Yang wrote:
> >> >> >> This compatible is used by boston.dts.
> >> >> >> 
> >> >> >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> >> >> >> ---
> >> >> >> note: This is a wildcard compatible for all MIPS CPUs,
> >> >> >>       I think we should use something like "riscv" for riscv.
> >> >> >
> >> >> > riscv systems, other than simulators etc are not meant to use the
> >> >> > "riscv" compatible. All of the real CPUs use "vendor,cpu", "riscv".
> >> >> > I'd suggest you add specific compatibles for your CPUs.
> >> >> 
> >> >> Boston can be combined with many different CPUs, thus we need to have
> >> >> such compatibles.
> >> >
> >> > Then you'll need different DTs. Different h/w, different DT.
> >> 
> >> The board have 9 CPU types in total, with hundreds of different possible
> >> CPU topologies. Maintaining separate DT for them seems impossible in kernel.
> >
> > But you could definitely add 9 different compatibles for each of these
> > different CPUs.
> 
> They are already in current bindings, but we need a default one to fill
> in kernel dts.

IMO, no you don't. I don't think some "default" dts does anything other
than promote not correctly documenting what the CPU actually is and just
using the default. I'd reject the "riscv" compatibles in isolation if
they arrived now & demand a "qemu,riscv" or w/e compatible.

> 
> >
> >> We can potentially patch this in bootloader, but for existing firmware it's
> >> being doing like this for years. I can see for RISC-V QEMU generated DTB is
> >> using a single "riscv" compatible and I do think it's a similar problem.
> >
> > That "riscv" compatible is only supposed to be used for
> > simulators/software models. Real CPUs are not meant to use it. AFAICT,
> > your boston is a real platform, even if the CPUs are implemented on an
> > FPGA they should still have one. If you take the OpenC906 RISC-V CPU and
> > put it on an FPGA, you're still meant to put "thead,c906" in your DT.
> >
> 
> And sadly, boston is also the platform used by MIPS internal emulators :-(
> 
> Thanks
> - Jiaxun
> >
> > 附件:
> > * signature.asc
> 
> -- 
> - Jiaxun
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mips/cpus.yaml b/Documentation/devicetree/bindings/mips/cpus.yaml
index a85137add668..7f9513f2a540 100644
--- a/Documentation/devicetree/bindings/mips/cpus.yaml
+++ b/Documentation/devicetree/bindings/mips/cpus.yaml
@@ -24,6 +24,7 @@  properties:
       - brcm,bmips5000
       - brcm,bmips5200
       - img,i6500
+      - img,mips
       - ingenic,xburst-fpu1.0-mxu1.1
       - ingenic,xburst-fpu2.0-mxu2.0
       - ingenic,xburst-mxu1.0