diff mbox series

[04/10] dt-bindings: memory: mt7621: add syscon as compatible string

Message ID 20220914085451.11723-5-arinc.unal@arinc9.com
State Superseded, archived
Headers show
Series dt-bindings and mt7621 devicetree changes | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 1 warnings, 17 lines checked
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Arınç ÜNAL Sept. 14, 2022, 8:54 a.m. UTC
Add syscon as a constant string on the compatible property as it's required
for the SoC to work. Update the example accordingly.

Fixes: 5278e4a181ff ("dt-bindings: memory: add binding for Mediatek's MT7621 SDRAM memory controller")
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../bindings/memory-controllers/mediatek,mt7621-memc.yaml   | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Sergio Paracuellos Sept. 14, 2022, 9:03 a.m. UTC | #1
On Wed, Sep 14, 2022 at 10:55 AM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
>
> Add syscon as a constant string on the compatible property as it's required
> for the SoC to work. Update the example accordingly.
>
> Fixes: 5278e4a181ff ("dt-bindings: memory: add binding for Mediatek's MT7621 SDRAM memory controller")
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../bindings/memory-controllers/mediatek,mt7621-memc.yaml   | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Acked-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>

Thanks,
    Sergio Paracuellos
Rob Herring (Arm) Sept. 14, 2022, 3:14 p.m. UTC | #2
On Wed, Sep 14, 2022 at 11:54:45AM +0300, Arınç ÜNAL wrote:
> Add syscon as a constant string on the compatible property as it's required
> for the SoC to work. Update the example accordingly.

It's not required. It's required to automagically create a regmap. That 
can be done yourself as well. The downside to adding 'syscon' is it 
requires a DT update. Maybe that's fine for this platform? I don't know.

> 
> Fixes: 5278e4a181ff ("dt-bindings: memory: add binding for Mediatek's MT7621 SDRAM memory controller")
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../bindings/memory-controllers/mediatek,mt7621-memc.yaml   | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,mt7621-memc.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,mt7621-memc.yaml
> index 85e02854f083..6ccdaf99c778 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,mt7621-memc.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,mt7621-memc.yaml
> @@ -11,7 +11,9 @@ maintainers:
>  
>  properties:
>    compatible:
> -    const: mediatek,mt7621-memc
> +    items:
> +      - const: mediatek,mt7621-memc
> +      - const: syscon
>  
>    reg:
>      maxItems: 1
> @@ -25,6 +27,6 @@ additionalProperties: false
>  examples:
>    - |
>      memory-controller@5000 {
> -        compatible = "mediatek,mt7621-memc";
> +        compatible = "mediatek,mt7621-memc", "syscon";
>          reg = <0x5000 0x1000>;
>      };
> -- 
> 2.34.1
> 
>
Arınç ÜNAL Sept. 14, 2022, 3:18 p.m. UTC | #3
On 14.09.2022 18:14, Rob Herring wrote:
> On Wed, Sep 14, 2022 at 11:54:45AM +0300, Arınç ÜNAL wrote:
>> Add syscon as a constant string on the compatible property as it's required
>> for the SoC to work. Update the example accordingly.
> 
> It's not required. It's required to automagically create a regmap. That
> can be done yourself as well. The downside to adding 'syscon' is it
> requires a DT update. Maybe that's fine for this platform? I don't know.

My GB-PC2 won't boot without syscon on mt7621.dtsi. This string was 
always there on the memory controller node on mt7621.dtsi.

Arınç
Sergio Paracuellos Sept. 15, 2022, 3:21 a.m. UTC | #4
On Wed, Sep 14, 2022 at 5:19 PM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
>
> On 14.09.2022 18:14, Rob Herring wrote:
> > On Wed, Sep 14, 2022 at 11:54:45AM +0300, Arınç ÜNAL wrote:
> >> Add syscon as a constant string on the compatible property as it's required
> >> for the SoC to work. Update the example accordingly.
> >
> > It's not required. It's required to automagically create a regmap. That
> > can be done yourself as well. The downside to adding 'syscon' is it
> > requires a DT update. Maybe that's fine for this platform? I don't know.
>
> My GB-PC2 won't boot without syscon on mt7621.dtsi. This string was
> always there on the memory controller node on mt7621.dtsi.

The string was introduced because the mt7621 clock driver needs to
read some registers creating a regmap from the syscon. The bindings
were added before the clock driver was properly mainlined and at first
the clock driver was using ralink architecture dependent operations
rt_memc_* defined in
'arch/mips/include/asm/mach-ralink/ralink_regs.h'. I forgot to update
the mem controller binding when memc became a syscon so I think this
patch is correct. I also think the sample should use 'syscon' in the
node name instead of memory-controller.

Best regards,
    Sergio Paracuellos
>
> Arınç
Arınç ÜNAL Sept. 15, 2022, 6:24 a.m. UTC | #5
On 15.09.2022 06:21, Sergio Paracuellos wrote:
> On Wed, Sep 14, 2022 at 5:19 PM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
>>
>> On 14.09.2022 18:14, Rob Herring wrote:
>>> On Wed, Sep 14, 2022 at 11:54:45AM +0300, Arınç ÜNAL wrote:
>>>> Add syscon as a constant string on the compatible property as it's required
>>>> for the SoC to work. Update the example accordingly.
>>>
>>> It's not required. It's required to automagically create a regmap. That
>>> can be done yourself as well. The downside to adding 'syscon' is it
>>> requires a DT update. Maybe that's fine for this platform? I don't know.
>>
>> My GB-PC2 won't boot without syscon on mt7621.dtsi. This string was
>> always there on the memory controller node on mt7621.dtsi.
> 
> The string was introduced because the mt7621 clock driver needs to
> read some registers creating a regmap from the syscon. The bindings
> were added before the clock driver was properly mainlined and at first
> the clock driver was using ralink architecture dependent operations
> rt_memc_* defined in
> 'arch/mips/include/asm/mach-ralink/ralink_regs.h'. I forgot to update
> the mem controller binding when memc became a syscon so I think this
> patch is correct. I also think the sample should use 'syscon' in the
> node name instead of memory-controller.

Will change to syscon in v2.

Arınç
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,mt7621-memc.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,mt7621-memc.yaml
index 85e02854f083..6ccdaf99c778 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,mt7621-memc.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,mt7621-memc.yaml
@@ -11,7 +11,9 @@  maintainers:
 
 properties:
   compatible:
-    const: mediatek,mt7621-memc
+    items:
+      - const: mediatek,mt7621-memc
+      - const: syscon
 
   reg:
     maxItems: 1
@@ -25,6 +27,6 @@  additionalProperties: false
 examples:
   - |
     memory-controller@5000 {
-        compatible = "mediatek,mt7621-memc";
+        compatible = "mediatek,mt7621-memc", "syscon";
         reg = <0x5000 0x1000>;
     };