| Message ID | 20250116-cluster-hci-broken-v2-2-fc52cfb7a19e@bootlin.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | MIPS: Allow using multi-cluster with a broken HCI. | expand |
On Thu, Jan 16, 2025 at 11:59:20AM +0100, Gregory CLEMENT wrote: > The CM3.5 used on EyeQ6 reports that Hardware Cache Initialization is > complete, but in reality it's not the case. It also incorrectly > indicates that Hardware Cache Initialization is supported. This new > compatible string allows warning about this broken feature that cannot > be detected at runtime. > > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > --- > .../devicetree/bindings/mips/mti,mips-cm.yaml | 24 ++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml > index 9f500804737d23e19f50a9326168686c05d3a54e..4713673f0cfc7785bb183917ee382a815ebfe9e1 100644 > --- a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml > +++ b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml > @@ -14,7 +14,12 @@ maintainers: > > properties: > compatible: > - const: mti,mips-cm > + oneOf: > + - const: mti,mips-cm > + - const: mti,eyeq6-cm Being a mobileye device, the vendor prefix should be mobileye. > + description: > + On EyeQ6 the HCI (Hardware Cache Initialization) information for > + the L2 cache in multi-cluster configuration is broken. > > reg: > description: > @@ -25,14 +30,29 @@ properties: > > required: > - compatible > - - reg > > additionalProperties: false > > +if: > + properties: > + compatible: > + contains: > + const: mti,eyeq6-cm > +then: > + properties: > + reg: false > +else: > + required: > + - reg How does one access this block with no registers? Is this some subset of a larger block? If so, need to define that block first. These 2 blocks don't look related and the only property shared is 'compatible'. This should be a separate doc. Rob
Hello Rob, > On Thu, Jan 16, 2025 at 11:59:20AM +0100, Gregory CLEMENT wrote: >> The CM3.5 used on EyeQ6 reports that Hardware Cache Initialization is >> complete, but in reality it's not the case. It also incorrectly >> indicates that Hardware Cache Initialization is supported. This new >> compatible string allows warning about this broken feature that cannot >> be detected at runtime. >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> >> --- >> .../devicetree/bindings/mips/mti,mips-cm.yaml | 24 ++++++++++++++++++++-- >> 1 file changed, 22 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml >> index 9f500804737d23e19f50a9326168686c05d3a54e..4713673f0cfc7785bb183917ee382a815ebfe9e1 100644 >> --- a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml >> +++ b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml >> @@ -14,7 +14,12 @@ maintainers: >> >> properties: >> compatible: >> - const: mti,mips-cm >> + oneOf: >> + - const: mti,mips-cm >> + - const: mti,eyeq6-cm > > Being a mobileye device, the vendor prefix should be mobileye. I chose mti because actually this block is part of the I6500 and provided as is by MIPS. > >> + description: >> + On EyeQ6 the HCI (Hardware Cache Initialization) information for >> + the L2 cache in multi-cluster configuration is broken. >> >> reg: >> description: >> @@ -25,14 +30,29 @@ properties: >> >> required: >> - compatible >> - - reg >> >> additionalProperties: false >> >> +if: >> + properties: >> + compatible: >> + contains: >> + const: mti,eyeq6-cm >> +then: >> + properties: >> + reg: false >> +else: >> + required: >> + - reg > > How does one access this block with no registers? Is this some subset of > a larger block? If so, need to define that block first. CM stands for Coherence Manager. This component is mandatory when you want to do SMP across MIPS core. This is part of the MIPS architecture, and the address of the CM is provided by the Coprocessor 0. "CP0 is incorporated on the CPU chip and supports the virtual memory system and exception handling. CP0 is also referred to as the System Control Coprocessor." So to summarize, in a functional system, this information doesn't have to be exposed through the device tree, as it is available at runtime from any MIPS CPU. > > These 2 blocks don't look related and the only property shared is > 'compatible'. This should be a separate doc. As mentioned in the cover letter, I reused the work from Jiaxun, who needed to deal with bogus CM but in a different way. In his use case, the issue with the CM was that the address in CP0 was wrong. In my case, this address is correct; it is only one piece of information reported by the CM that is wrong. I don't mind creating a separate doc if you still think it is the right thing to do. Gregory > > Rob
在2025年1月17日一月 上午9:46，Gregory CLEMENT写道： [...] Hi all, >> >> These 2 blocks don't look related and the only property shared is >> 'compatible'. This should be a separate doc. > > As mentioned in the cover letter, I reused the work from Jiaxun, who > needed to deal with bogus CM but in a different way. In his use case, > the issue with the CM was that the address in CP0 was wrong. In my case, > this address is correct; it is only one piece of information reported by > the CM that is wrong. I don't mind creating a separate doc if you > still think it is the right thing to do. Precisely I'm dealing with two kind of systems, the first is systems doesn't come with CP0.CMCGRBase, and thus rely on DeviceTree for probing the CM. The second is systems mapping CMGCR at inappropriate locations and we want kernel to remap it. We don't want reg property to be mandatory as we are dealing with a huge amount of legacy systems which mapping CM registers at different locations, while we have to use a uniformed built-in DT and probe mapping at runtime. Thanks - Jiaxun > > Gregory > >> >> Rob > > -- > Grégory CLEMENT, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
On Fri, Jan 17, 2025 at 10:46:00AM +0100, Gregory CLEMENT wrote: > >> - const: mti,mips-cm > >> + oneOf: > >> + - const: mti,mips-cm > >> + - const: mti,eyeq6-cm > > > > Being a mobileye device, the vendor prefix should be mobileye. > > I chose mti because actually this block is part of the I6500 and > provided as is by MIPS. But MIPS or MTI did not create eyeq6, so then product does not fit vendor. > > > > >> + description: > >> + On EyeQ6 the HCI (Hardware Cache Initialization) information for > >> + the L2 cache in multi-cluster configuration is broken. > >> > >> reg: > >> description: > >> @@ -25,14 +30,29 @@ properties: > >> > >> required: > >> - compatible > >> - - reg > >> > >> additionalProperties: false > >> > >> +if: > >> + properties: > >> + compatible: > >> + contains: > >> + const: mti,eyeq6-cm > >> +then: > >> + properties: > >> + reg: false > >> +else: > >> + required: > >> + - reg > > > > How does one access this block with no registers? Is this some subset of > > a larger block? If so, need to define that block first. > > CM stands for Coherence Manager. This component is mandatory when you > want to do SMP across MIPS core. This is part of the MIPS architecture, > and the address of the CM is provided by the Coprocessor 0. > > "CP0 is incorporated on the CPU chip and supports the virtual memory > system and exception handling. CP0 is also referred to as the System > Control Coprocessor." > > So to summarize, in a functional system, this information doesn't have > to be exposed through the device tree, as it is available at runtime > from any MIPS CPU. > > > > > These 2 blocks don't look related and the only property shared is > > 'compatible'. This should be a separate doc. > > As mentioned in the cover letter, I reused the work from Jiaxun, who > needed to deal with bogus CM but in a different way. In his use case, > the issue with the CM was that the address in CP0 was wrong. In my case, > this address is correct; it is only one piece of information reported by > the CM that is wrong. I don't mind creating a separate doc if you > still think it is the right thing to do. So the programming interface in general is the same, but in one case the reg/address detection does not work reliably? I guess could stay the same doc, but all this should be explained in binding description. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml index 9f500804737d23e19f50a9326168686c05d3a54e..4713673f0cfc7785bb183917ee382a815ebfe9e1 100644 --- a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml +++ b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml @@ -14,7 +14,12 @@ maintainers: properties: compatible: - const: mti,mips-cm + oneOf: + - const: mti,mips-cm + - const: mti,eyeq6-cm + description: + On EyeQ6 the HCI (Hardware Cache Initialization) information for + the L2 cache in multi-cluster configuration is broken. reg: description: @@ -25,14 +30,29 @@ properties: required: - compatible - - reg additionalProperties: false +if: + properties: + compatible: + contains: + const: mti,eyeq6-cm +then: + properties: + reg: false +else: + required: + - reg + examples: - | coherency-manager@1fbf8000 { compatible = "mti,mips-cm"; reg = <0x1bde8000 0x8000>; }; + + coherency-manager { + compatible = "mti,eyeq6-cm"; + }; ...
The CM3.5 used on EyeQ6 reports that Hardware Cache Initialization is complete, but in reality it's not the case. It also incorrectly indicates that Hardware Cache Initialization is supported. This new compatible string allows warning about this broken feature that cannot be detected at runtime. Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> --- .../devicetree/bindings/mips/mti,mips-cm.yaml | 24 ++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)