| Message ID | 20181115145013.3378-1-paul.kocialkowski@bootlin.com |
|---|---|
| Headers | show |
| Series | Cedrus support for the Allwinner H5 and A64 platforms | expand |
On Thu, Nov 15, 2018 at 10:51 PM Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote: > > This adds nodes for the Video Engine and the associated reserved memory > for the H5. Up to 96 MiB of memory are dedicated to the CMA pool. > > The pool is located at the end of the first 256 MiB of RAM so that the > VPU can access it. It is unclear whether this is still a hard > requirement for this platform, but it seems safer that way. I think we can actually test this. You could move the reserved memory pool beyond 256 MiB, and have cedrus decode stuff, and try to display the results. If it's gibberish, or the system crashes, it's likely the memory access wrapped around at 256 MiB. What do you think? ChenYu
On Thu, Nov 15, 2018 at 10:50 PM Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote: > > This cosmetic change removes the heading 0 in the video-codec unit > address, as it's done for other nodes. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Nit: I'd prefer the subject prefix format be "<family>: <soc>: ... ", or "sun8i: a33:" in this case. This format seems to be used more often than your alternative format. I can fix it up when applying. Acked-by: Chen-Yu Tsai <wens@csie.org> ChenYu
On Thu, Nov 15, 2018 at 10:50 PM Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote: > > This cosmetic change removes the heading 0 in the video-codec unit > address, as it's done for other nodes. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Other than the subject format we can fix when applying, Acked-by: Chen-Yu Tsai <wens@csie.org>
On Thu, Nov 15, 2018 at 10:50 PM Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote: > > Unlike in previous generations, the system-control register range is not > limited to a size of 0x30 on the H3. In particular, the EMAC clock > configuration register (accessed through syscon) is at offset 0x30 in > that range. > > Extend the register size to its full range (0x1000) as a result. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Other than the subject format, Acked-by: Chen-Yu Tsai <wens@csie.org>
On Thu, Nov 15, 2018 at 10:50 PM Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote: > > Just like the A64 and H5, the H3 SoC uses the system control block > to enable the EMAC clock. > > Add a variant structure definition for the H3 and use it over the A10 > one. This will allow using the H3-specific binding for the syscon node > attached to the EMAC instead of the generic syscon binding. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
On Thu, Nov 15, 2018 at 10:50 PM Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote: > > Add the description for the SRAM C1 section to the A64 device-tree. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > index f3a66f888205..88b3e9110833 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > @@ -277,6 +277,20 @@ > reg = <0x0000 0x28000>; > }; > }; > + > + sram_c1: sram@1d00000 { > + compatible = "mmio-sram"; > + reg = <0x01d00000 0x80000>; I can confirm that this SRAM region is indeed at this address. However the size is only 0x40000, not 0x80000. The address ranges should be fixed. One hiccup is that the VE reset has to be de-asserted and the VE bus clock has to be ungated for the CPU to access this region when it's mapped to the CPU. One other thing I find interesting is that in the previous SoCs, the bits that control this mapping says 50K, but in reality it is 512K for the older SoCs, and 256K for this one. ChenYu > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0 0x01d00000 0x80000>; > + > + ve_sram: sram-section@0 { > + compatible = "allwinner,sun50i-a64-sram-c1", > + "allwinner,sun4i-a10-sram-c1"; > + reg = <0x000000 0x80000>; > + }; > + }; > }; > > dma: dma-controller@1c02000 { > -- > 2.19.1 >
On Thu, Nov 15, 2018 at 10:50 PM Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote: > > Add the H5-specific system control node description to its device-tree > with support for the SRAM C1 section, that will be used by the video > codec node later on. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 22 ++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi > index b41dc1aab67d..c2d14b22b8c1 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi > @@ -94,6 +94,28 @@ > }; > > soc { > + system-control@1c00000 { > + compatible = "allwinner,sun50i-h5-system-control"; > + reg = <0x01c00000 0x1000>; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + sram_c1: sram@1d00000 { > + compatible = "mmio-sram"; > + reg = <0x01d00000 0x80000>; I'll try to check this one tomorrow. I did find something interesting on the H3: there also seems to be SRAM at 0x01dc0000 to 0x01dcfeff , again mapped by the same bits as SRAM C1. And on the A33, the SRAM C1 range is 0x01d00000 to 0x01d478ff. This was found by mapping the SRAM to the CPU, then using devmem to poke around the register range. If there's SRAM, the first read would typically return random data, and a subsequent write to it would set some value that would be read back correctly. If there's no SRAM, a read either returns 0x0 or some random data that can't be overwritten. You might want to check the other SoCs. ChenYu > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0 0x01d00000 0x80000>; > + > + ve_sram: sram-section@0 { > + compatible = "allwinner,sun50i-h5-sram-c1", > + "allwinner,sun4i-a10-sram-c1"; > + reg = <0x000000 0x80000>; > + }; > + }; > + }; > + > mali: gpu@1e80000 { > compatible = "allwinner,sun50i-h5-mali", "arm,mali-450"; > reg = <0x01e80000 0x30000>; > -- > 2.19.1 >
On Thu, Nov 15, 2018 at 03:50:06PM +0100, Paul Kocialkowski wrote: > Now that we have specific nodes for the H3 and H5 system-controller > that allow proper access to the EMAC clock configuration register, > we no longer need a common dummy syscon node. > > Switch the syscon label over to each platform's dtsi file. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > arch/arm/boot/dts/sun8i-h3.dtsi | 2 +- > arch/arm/boot/dts/sunxi-h3-h5.dtsi | 6 ------ > arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 2 +- > 3 files changed, 2 insertions(+), 8 deletions(-) > > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi > index 7157d954fb8c..b337a9282783 100644 > --- a/arch/arm/boot/dts/sun8i-h3.dtsi > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi > @@ -134,7 +134,7 @@ > }; > > soc { > - system-control@1c00000 { > + syscon: system-control@1c00000 { > compatible = "allwinner,sun8i-h3-system-control"; > reg = <0x01c00000 0x1000>; > #address-cells = <1>; > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi > index 4b1530ebe427..9175ff0fb59a 100644 > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi > @@ -152,12 +152,6 @@ > }; > }; > > - syscon: syscon@1c00000 { > - compatible = "allwinner,sun8i-h3-system-controller", > - "syscon"; > - reg = <0x01c00000 0x1000>; > - }; > - You're also dropping the syscon compatible there. But I'm not sure how it could work with the H3 EMAC driver that would overwrite the compatible already. Maxime
On Thu, Nov 15, 2018 at 03:50:13PM +0100, Paul Kocialkowski wrote: > This adds nodes for the Video Engine and the associated reserved memory > for the A64. Up to 96 MiB of memory are dedicated to the CMA pool. > > The pool is located at the end of the first 256 MiB of RAM so that the > VPU can access it. It is unclear whether this is still a hard > requirement for this platform, but it seems safer that way. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 25 +++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > index 88b3e9110833..a35a5c9ffbbe 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > @@ -185,6 +185,20 @@ > (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>; > }; > > + reserved-memory { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + cma_pool: cma@4a000000 { > + compatible = "shared-dma-pool"; > + size = <0x6000000>; > + alloc-ranges = <0x4a000000 0x6000000>; This introduces a DTC warning, since you're using a unit-address, but don't have a reg register. I've fixed it in the other DT by renaming that node to default-pool. You can also drop the label, it's not used anywhere. Maxime
On Fri, Nov 16, 2018 at 5:39 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Thu, Nov 15, 2018 at 03:50:06PM +0100, Paul Kocialkowski wrote: > > Now that we have specific nodes for the H3 and H5 system-controller > > that allow proper access to the EMAC clock configuration register, > > we no longer need a common dummy syscon node. > > > > Switch the syscon label over to each platform's dtsi file. > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > --- > > arch/arm/boot/dts/sun8i-h3.dtsi | 2 +- > > arch/arm/boot/dts/sunxi-h3-h5.dtsi | 6 ------ > > arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 2 +- > > 3 files changed, 2 insertions(+), 8 deletions(-) > > > > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi > > index 7157d954fb8c..b337a9282783 100644 > > --- a/arch/arm/boot/dts/sun8i-h3.dtsi > > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi > > @@ -134,7 +134,7 @@ > > }; > > > > soc { > > - system-control@1c00000 { > > + syscon: system-control@1c00000 { > > compatible = "allwinner,sun8i-h3-system-control"; > > reg = <0x01c00000 0x1000>; > > #address-cells = <1>; > > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi > > index 4b1530ebe427..9175ff0fb59a 100644 > > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi > > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi > > @@ -152,12 +152,6 @@ > > }; > > }; > > > > - syscon: syscon@1c00000 { > > - compatible = "allwinner,sun8i-h3-system-controller", > > - "syscon"; > > - reg = <0x01c00000 0x1000>; > > - }; > > - > > You're also dropping the syscon compatible there. But I'm not sure how > it could work with the H3 EMAC driver that would overwrite the > compatible already. I assume you are referring to the previous patch? The node names are not the same, hence the previous patch is adding another node for the system controller, and this patch removes the old one with the "syscon" compatible. We already patched the EMAC driver to support the new SRAM controller based regmap, so other than making people unhappy about having to update their DT, I don't think there would be any problems. This also means H3 in -next currently has _two_ syscon nodes. ChenYu
Hi, Le vendredi 16 novembre 2018 à 17:47 +0800, Chen-Yu Tsai a écrit : > On Fri, Nov 16, 2018 at 5:39 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Thu, Nov 15, 2018 at 03:50:06PM +0100, Paul Kocialkowski wrote: > > > Now that we have specific nodes for the H3 and H5 system-controller > > > that allow proper access to the EMAC clock configuration register, > > > we no longer need a common dummy syscon node. > > > > > > Switch the syscon label over to each platform's dtsi file. > > > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > > --- > > > arch/arm/boot/dts/sun8i-h3.dtsi | 2 +- > > > arch/arm/boot/dts/sunxi-h3-h5.dtsi | 6 ------ > > > arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 2 +- > > > 3 files changed, 2 insertions(+), 8 deletions(-) > > > > > > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi > > > index 7157d954fb8c..b337a9282783 100644 > > > --- a/arch/arm/boot/dts/sun8i-h3.dtsi > > > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi > > > @@ -134,7 +134,7 @@ > > > }; > > > > > > soc { > > > - system-control@1c00000 { > > > + syscon: system-control@1c00000 { > > > compatible = "allwinner,sun8i-h3-system-control"; > > > reg = <0x01c00000 0x1000>; > > > #address-cells = <1>; > > > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi > > > index 4b1530ebe427..9175ff0fb59a 100644 > > > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi > > > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi > > > @@ -152,12 +152,6 @@ > > > }; > > > }; > > > > > > - syscon: syscon@1c00000 { > > > - compatible = "allwinner,sun8i-h3-system-controller", > > > - "syscon"; > > > - reg = <0x01c00000 0x1000>; > > > - }; > > > - > > > > You're also dropping the syscon compatible there. But I'm not sure how > > it could work with the H3 EMAC driver that would overwrite the > > compatible already. > > I assume you are referring to the previous patch? The node names are not > the same, hence the previous patch is adding another node for the system > controller, and this patch removes the old one with the "syscon" compatible. > > We already patched the EMAC driver to support the new SRAM controller based > regmap, so other than making people unhappy about having to update their > DT, I don't think there would be any problems. This also means H3 in -next > currently has _two_ syscon nodes. Yes, the point is indeed to only have a single node per platform (in the platform dtsi) instead of two (one in the common h3-h5 dtsi and one in the platform dtsi). I guess updating the dt is not even a hard requirement after this series: things will keep working with the dummy syscon node for giving the EMAC driver access to the syscon registers. Cheers, Paul
Hi, Le jeudi 15 novembre 2018 à 23:50 +0800, Chen-Yu Tsai a écrit : > On Thu, Nov 15, 2018 at 10:50 PM Paul Kocialkowski > <paul.kocialkowski@bootlin.com> wrote: > > This cosmetic change removes the heading 0 in the video-codec unit > > address, as it's done for other nodes. > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > Nit: I'd prefer the subject prefix format be "<family>: <soc>: ... ", > or "sun8i: a33:" in this case. This format seems to be used more often > than your alternative format. > > I can fix it up when applying. Understood, I will make sure to follow this convention next time. Cheers, Paul > Acked-by: Chen-Yu Tsai <wens@csie.org> > > ChenYu
Hi, On Fri, Nov 16, 2018 at 05:47:50PM +0800, Chen-Yu Tsai wrote: > On Fri, Nov 16, 2018 at 5:39 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > On Thu, Nov 15, 2018 at 03:50:06PM +0100, Paul Kocialkowski wrote: > > > Now that we have specific nodes for the H3 and H5 system-controller > > > that allow proper access to the EMAC clock configuration register, > > > we no longer need a common dummy syscon node. > > > > > > Switch the syscon label over to each platform's dtsi file. > > > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > > --- > > > arch/arm/boot/dts/sun8i-h3.dtsi | 2 +- > > > arch/arm/boot/dts/sunxi-h3-h5.dtsi | 6 ------ > > > arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 2 +- > > > 3 files changed, 2 insertions(+), 8 deletions(-) > > > > > > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi > > > index 7157d954fb8c..b337a9282783 100644 > > > --- a/arch/arm/boot/dts/sun8i-h3.dtsi > > > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi > > > @@ -134,7 +134,7 @@ > > > }; > > > > > > soc { > > > - system-control@1c00000 { > > > + syscon: system-control@1c00000 { > > > compatible = "allwinner,sun8i-h3-system-control"; > > > reg = <0x01c00000 0x1000>; > > > #address-cells = <1>; > > > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi > > > index 4b1530ebe427..9175ff0fb59a 100644 > > > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi > > > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi > > > @@ -152,12 +152,6 @@ > > > }; > > > }; > > > > > > - syscon: syscon@1c00000 { > > > - compatible = "allwinner,sun8i-h3-system-controller", > > > - "syscon"; > > > - reg = <0x01c00000 0x1000>; > > > - }; > > > - > > > > You're also dropping the syscon compatible there. But I'm not sure how > > it could work with the H3 EMAC driver that would overwrite the > > compatible already. > > I assume you are referring to the previous patch? The node names are not > the same, hence the previous patch is adding another node for the system > controller, and this patch removes the old one with the "syscon" compatible. > > We already patched the EMAC driver to support the new SRAM controller based > regmap, so other than making people unhappy about having to update their > DT, I don't think there would be any problems. This also means H3 in -next > currently has _two_ syscon nodes. Ah, indeed, I missed that. Pointing that out in the commit log could be nice though. Thanks! Maxime
On Fri, Nov 16, 2018 at 12:52 AM Chen-Yu Tsai <wens@csie.org> wrote: > > On Thu, Nov 15, 2018 at 10:50 PM Paul Kocialkowski > <paul.kocialkowski@bootlin.com> wrote: > > > > Add the H5-specific system control node description to its device-tree > > with support for the SRAM C1 section, that will be used by the video > > codec node later on. > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > --- > > arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 22 ++++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi > > index b41dc1aab67d..c2d14b22b8c1 100644 > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi > > @@ -94,6 +94,28 @@ > > }; > > > > soc { > > + system-control@1c00000 { > > + compatible = "allwinner,sun50i-h5-system-control"; > > + reg = <0x01c00000 0x1000>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges; > > + > > + sram_c1: sram@1d00000 { > > + compatible = "mmio-sram"; > > + reg = <0x01d00000 0x80000>; > > I'll try to check this one tomorrow. > > I did find something interesting on the H3: there also seems to be SRAM at > 0x01dc0000 to 0x01dcfeff , again mapped by the same bits as SRAM C1. > > And on the A33, the SRAM C1 range is 0x01d00000 to 0x01d478ff. > > This was found by mapping the SRAM to the CPU, then using devmem to poke > around the register range. If there's SRAM, the first read would typically > return random data, and a subsequent write to it would set some value that > would be read back correctly. If there's no SRAM, a read either returns 0x0 > or some random data that can't be overwritten. > > You might want to check the other SoCs. This range seems to contain stuff other than SRAM, possibly fixed lookup tables. Since this is entirely unknown, lets just stick to the known full range instead. ChenYu > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges = <0 0x01d00000 0x80000>; > > + > > + ve_sram: sram-section@0 { > > + compatible = "allwinner,sun50i-h5-sram-c1", > > + "allwinner,sun4i-a10-sram-c1"; > > + reg = <0x000000 0x80000>; > > + }; > > + }; > > + }; > > + > > mali: gpu@1e80000 { > > compatible = "allwinner,sun50i-h5-mali", "arm,mali-450"; > > reg = <0x01e80000 0x30000>; > > -- > > 2.19.1 > >
Hi, On Thu, 2018-11-15 at 23:35 +0800, Chen-Yu Tsai wrote: > On Thu, Nov 15, 2018 at 10:51 PM Paul Kocialkowski > <paul.kocialkowski@bootlin.com> wrote: > > This adds nodes for the Video Engine and the associated reserved memory > > for the H5. Up to 96 MiB of memory are dedicated to the CMA pool. > > > > The pool is located at the end of the first 256 MiB of RAM so that the > > VPU can access it. It is unclear whether this is still a hard > > requirement for this platform, but it seems safer that way. > > I think we can actually test this. You could move the reserved memory > pool beyond 256 MiB, and have cedrus decode stuff, and try to display > the results. If it's gibberish, or the system crashes, it's likely the > memory access wrapped around at 256 MiB. > > What do you think? I did the test on various platforms and found that starting with the A33, the VPU can map any address in RAM! SO we shouldn't need reserved memory nodes for these devices after all. Cheers, Paul
Hi, On Fri, 2018-11-30 at 11:38 +0800, Chen-Yu Tsai wrote: > On Fri, Nov 16, 2018 at 12:52 AM Chen-Yu Tsai <wens@csie.org> wrote: > > On Thu, Nov 15, 2018 at 10:50 PM Paul Kocialkowski > > <paul.kocialkowski@bootlin.com> wrote: > > > Add the H5-specific system control node description to its device-tree > > > with support for the SRAM C1 section, that will be used by the video > > > codec node later on. > > > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > > --- > > > arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 22 ++++++++++++++++++++ > > > 1 file changed, 22 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi > > > index b41dc1aab67d..c2d14b22b8c1 100644 > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi > > > @@ -94,6 +94,28 @@ > > > }; > > > > > > soc { > > > + system-control@1c00000 { > > > + compatible = "allwinner,sun50i-h5-system-control"; > > > + reg = <0x01c00000 0x1000>; > > > + #address-cells = <1>; > > > + #size-cells = <1>; > > > + ranges; > > > + > > > + sram_c1: sram@1d00000 { > > > + compatible = "mmio-sram"; > > > + reg = <0x01d00000 0x80000>; > > > > I'll try to check this one tomorrow. > > > > I did find something interesting on the H3: there also seems to be SRAM at > > 0x01dc0000 to 0x01dcfeff , again mapped by the same bits as SRAM C1. > > > > And on the A33, the SRAM C1 range is 0x01d00000 to 0x01d478ff. > > > > This was found by mapping the SRAM to the CPU, then using devmem to poke > > around the register range. If there's SRAM, the first read would typically > > return random data, and a subsequent write to it would set some value that > > would be read back correctly. If there's no SRAM, a read either returns 0x0 > > or some random data that can't be overwritten. > > > > You might want to check the other SoCs. > > This range seems to contain stuff other than SRAM, possibly fixed lookup > tables. Since this is entirely unknown, lets just stick to the known full > range instead. Thanks for investigating all this! I also conducted some tests and found that the H5 has its SRAM C1 (marked as SRAM C in the manual) at 0x18000. However for the A64, SRAM C1 gets mapped to 0x1D00000. There is also SRAM C at 0x18000 but this one seems unrelated to the VPU and only used by DE2 (as already described in the A64 dt). I share your conclusion that there seems to be more than SRAM in there. Testing with devmem write/read on the start address was reliable as a test, but some chunks in the range did not behave like SRAM (not the same value read). I agree that we should keep the known full range as there are lots of unknowns here. Cheers, Paul > ChenYu > > > > + #address-cells = <1>; > > > + #size-cells = <1>; > > > + ranges = <0 0x01d00000 0x80000>; > > > + > > > + ve_sram: sram-section@0 { > > > + compatible = "allwinner,sun50i-h5-sram-c1", > > > + "allwinner,sun4i-a10-sram-c1"; > > > + reg = <0x000000 0x80000>; > > > + }; > > > + }; > > > + }; > > > + > > > mali: gpu@1e80000 { > > > compatible = "allwinner,sun50i-h5-mali", "arm,mali-450"; > > > reg = <0x01e80000 0x30000>; > > > -- > > > 2.19.1 > > >