Re: [linux-sunxi] Re: [PATCH 7/8] arm64: dts: allwinner: Add Allwinner H616 .dtsi file

2020-12-07 Thread André Przywara
On 03/12/2020 16:20, Chen-Yu Tsai wrote:

Hi,

> On Thu, Dec 3, 2020 at 11:45 PM André Przywara  wrote:
>>
>> On 03/12/2020 15:02, Chen-Yu Tsai wrote:
>>> On Thu, Dec 3, 2020 at 6:54 PM André Przywara  
>>> wrote:

 On 03/12/2020 03:16, Samuel Holland wrote:

 Hi,

> On 12/2/20 7:54 AM, Andre Przywara wrote:
> ...
>> +soc {
>> +compatible = "simple-bus";
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +ranges = <0x0 0x0 0x0 0x4000>;
>> +
>> +syscon: syscon@300 {
>> +compatible = "allwinner,sun50i-h616-system-control",
>> + "allwinner,sun50i-a64-system-control";
>> +reg = <0x0300 0x1000>;
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +ranges;
>> +
>> +sram_c: sram@28000 {
>> +compatible = "mmio-sram";
>> +reg = <0x00028000 0x3>;
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +ranges = <0 0x00028000 0x3>;
>> +};
>> +
>> +sram_c1: sram@1a0 {
>> +compatible = "mmio-sram";
>> +reg = <0x01a0 0x20>;
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +ranges = <0 0x01a0 0x20>;
>> +
>> +ve_sram: sram-section@0 {
>> +compatible = 
>> "allwinner,sun50i-h616-sram-c1",
>> + 
>> "allwinner,sun4i-a10-sram-c1";
>> +reg = <0x00 0x20>;
>> +};
>> +};
>> +};
>
> You mentioned that you could not find a SRAM A2. How were these SRAM 
> ranges
> verified? If you can load eGON.BT0 larger than 32 KiB, then presumably 
> NBROM
> uses SRAM C, and it is in the manual, but I see no mention of SRAM C1.

 The manual says that SRAM C *can* be used by "the system", at boot time,
 as long as it's configured correctly. I couldn't find any details on how
 to switch clock sources for SRAM C, and the manual stanza on this is
 quite gibberish. I presume it's configured either by BROM or by reset
 default this way. I think the idea is that the later users (VE, DE) take
 ownership at some point (which means we can't run any firmware in there).
 The BSP boot0 is 48KB already, so reaching into SRAM C, and the code
 itself heavily uses SRAM C (found by hacking boot0 to drop to FEL and
 inspecting the memory afterwards).

 For C1: I copied this name from the H6 .dtsi, the manual calls this
 "VE-SRAM", in both manuals, and the description looks identical there
 for both SoCs. I think this will be later used by the video engine, so I
 kept it in. The large size made me suspicious, and from former
 experiments it looks like being aliased to (parts of) SRAM C.
>>>
>>> I would just call it sram_ve or ve_sram. SRAM C1 would make more sense if
>>> it were part of SRAM C, not the other way around.
>>
>> But isn't that what we do? "sram_c1" is just the node name alias used
>> for the parent node. That is actually never referenced anywhere (in any
>> of the the H6 .dts), so we can actually remove it, I guess.
>> The actual SRAM section is called ve_sram already.
> 
> This is what I had in mind:
> 
> syscon: {
> sram_c: sram@28000 {
> compatible = "mmio-sram";
> reg = <0x00028000 0x3>;
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0 0x00028000 0x3>;
> 
> /* starting address might not be correct */
> sram_c_ve: sram-section@0 {
> compatible = "allwinner,sun50i-h616-ve-sram-c",
>  "allwinner,sun4i-a10-sram-c1";
>/* 64 kiB borrowed from ve_sram */
> reg = <0x0 0x1>;
> };
> };
> 
> ve_sram: sram@1a0 {
> compatible = "mmio-sram";
> reg = <0x01a0 0x20>;
> #address-cells = <1>;
> #size-cells = <1>;
> };
> };

OK, I am taking this version, but am dropping the sram_c_ve subnode,
until we know where it is, what we actually need and until we can test it.

> Another variant, trying to describe the aliasing, though it seems
> quite confusing:

I don't know 

Re: [linux-sunxi] Re: [PATCH 7/8] arm64: dts: allwinner: Add Allwinner H616 .dtsi file

2020-12-03 Thread Chen-Yu Tsai
On Thu, Dec 3, 2020 at 11:45 PM André Przywara  wrote:
>
> On 03/12/2020 15:02, Chen-Yu Tsai wrote:
> > On Thu, Dec 3, 2020 at 6:54 PM André Przywara  
> > wrote:
> >>
> >> On 03/12/2020 03:16, Samuel Holland wrote:
> >>
> >> Hi,
> >>
> >>> On 12/2/20 7:54 AM, Andre Przywara wrote:
> >>> ...
>  +soc {
>  +compatible = "simple-bus";
>  +#address-cells = <1>;
>  +#size-cells = <1>;
>  +ranges = <0x0 0x0 0x0 0x4000>;
>  +
>  +syscon: syscon@300 {
>  +compatible = "allwinner,sun50i-h616-system-control",
>  + "allwinner,sun50i-a64-system-control";
>  +reg = <0x0300 0x1000>;
>  +#address-cells = <1>;
>  +#size-cells = <1>;
>  +ranges;
>  +
>  +sram_c: sram@28000 {
>  +compatible = "mmio-sram";
>  +reg = <0x00028000 0x3>;
>  +#address-cells = <1>;
>  +#size-cells = <1>;
>  +ranges = <0 0x00028000 0x3>;
>  +};
>  +
>  +sram_c1: sram@1a0 {
>  +compatible = "mmio-sram";
>  +reg = <0x01a0 0x20>;
>  +#address-cells = <1>;
>  +#size-cells = <1>;
>  +ranges = <0 0x01a0 0x20>;
>  +
>  +ve_sram: sram-section@0 {
>  +compatible = 
>  "allwinner,sun50i-h616-sram-c1",
>  + 
>  "allwinner,sun4i-a10-sram-c1";
>  +reg = <0x00 0x20>;
>  +};
>  +};
>  +};
> >>>
> >>> You mentioned that you could not find a SRAM A2. How were these SRAM 
> >>> ranges
> >>> verified? If you can load eGON.BT0 larger than 32 KiB, then presumably 
> >>> NBROM
> >>> uses SRAM C, and it is in the manual, but I see no mention of SRAM C1.
> >>
> >> The manual says that SRAM C *can* be used by "the system", at boot time,
> >> as long as it's configured correctly. I couldn't find any details on how
> >> to switch clock sources for SRAM C, and the manual stanza on this is
> >> quite gibberish. I presume it's configured either by BROM or by reset
> >> default this way. I think the idea is that the later users (VE, DE) take
> >> ownership at some point (which means we can't run any firmware in there).
> >> The BSP boot0 is 48KB already, so reaching into SRAM C, and the code
> >> itself heavily uses SRAM C (found by hacking boot0 to drop to FEL and
> >> inspecting the memory afterwards).
> >>
> >> For C1: I copied this name from the H6 .dtsi, the manual calls this
> >> "VE-SRAM", in both manuals, and the description looks identical there
> >> for both SoCs. I think this will be later used by the video engine, so I
> >> kept it in. The large size made me suspicious, and from former
> >> experiments it looks like being aliased to (parts of) SRAM C.
> >
> > I would just call it sram_ve or ve_sram. SRAM C1 would make more sense if
> > it were part of SRAM C, not the other way around.
>
> But isn't that what we do? "sram_c1" is just the node name alias used
> for the parent node. That is actually never referenced anywhere (in any
> of the the H6 .dts), so we can actually remove it, I guess.
> The actual SRAM section is called ve_sram already.

This is what I had in mind:

syscon: {
sram_c: sram@28000 {
compatible = "mmio-sram";
reg = <0x00028000 0x3>;
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x00028000 0x3>;

/* starting address might not be correct */
sram_c_ve: sram-section@0 {
compatible = "allwinner,sun50i-h616-ve-sram-c",
 "allwinner,sun4i-a10-sram-c1";
   /* 64 kiB borrowed from ve_sram */
reg = <0x0 0x1>;
};
};

ve_sram: sram@1a0 {
compatible = "mmio-sram";
reg = <0x01a0 0x20>;
#address-cells = <1>;
#size-cells = <1>;
};
};

Another variant, trying to describe the aliasing, though it seems
quite confusing:

syscon: {
ve_sram: sram@1a0 {
compatible = "mmio-sram";
reg = <0x01a0 0x20>;
#address-cells = <1>;
#size-cells = <1>;
ranges;

ve_sram_c: sram-section@28000 {

Re: [linux-sunxi] Re: [PATCH 7/8] arm64: dts: allwinner: Add Allwinner H616 .dtsi file

2020-12-03 Thread André Przywara
On 03/12/2020 15:02, Chen-Yu Tsai wrote:
> On Thu, Dec 3, 2020 at 6:54 PM André Przywara  wrote:
>>
>> On 03/12/2020 03:16, Samuel Holland wrote:
>>
>> Hi,
>>
>>> On 12/2/20 7:54 AM, Andre Przywara wrote:
>>> ...
 +soc {
 +compatible = "simple-bus";
 +#address-cells = <1>;
 +#size-cells = <1>;
 +ranges = <0x0 0x0 0x0 0x4000>;
 +
 +syscon: syscon@300 {
 +compatible = "allwinner,sun50i-h616-system-control",
 + "allwinner,sun50i-a64-system-control";
 +reg = <0x0300 0x1000>;
 +#address-cells = <1>;
 +#size-cells = <1>;
 +ranges;
 +
 +sram_c: sram@28000 {
 +compatible = "mmio-sram";
 +reg = <0x00028000 0x3>;
 +#address-cells = <1>;
 +#size-cells = <1>;
 +ranges = <0 0x00028000 0x3>;
 +};
 +
 +sram_c1: sram@1a0 {
 +compatible = "mmio-sram";
 +reg = <0x01a0 0x20>;
 +#address-cells = <1>;
 +#size-cells = <1>;
 +ranges = <0 0x01a0 0x20>;
 +
 +ve_sram: sram-section@0 {
 +compatible = 
 "allwinner,sun50i-h616-sram-c1",
 + 
 "allwinner,sun4i-a10-sram-c1";
 +reg = <0x00 0x20>;
 +};
 +};
 +};
>>>
>>> You mentioned that you could not find a SRAM A2. How were these SRAM ranges
>>> verified? If you can load eGON.BT0 larger than 32 KiB, then presumably NBROM
>>> uses SRAM C, and it is in the manual, but I see no mention of SRAM C1.
>>
>> The manual says that SRAM C *can* be used by "the system", at boot time,
>> as long as it's configured correctly. I couldn't find any details on how
>> to switch clock sources for SRAM C, and the manual stanza on this is
>> quite gibberish. I presume it's configured either by BROM or by reset
>> default this way. I think the idea is that the later users (VE, DE) take
>> ownership at some point (which means we can't run any firmware in there).
>> The BSP boot0 is 48KB already, so reaching into SRAM C, and the code
>> itself heavily uses SRAM C (found by hacking boot0 to drop to FEL and
>> inspecting the memory afterwards).
>>
>> For C1: I copied this name from the H6 .dtsi, the manual calls this
>> "VE-SRAM", in both manuals, and the description looks identical there
>> for both SoCs. I think this will be later used by the video engine, so I
>> kept it in. The large size made me suspicious, and from former
>> experiments it looks like being aliased to (parts of) SRAM C.
> 
> I would just call it sram_ve or ve_sram. SRAM C1 would make more sense if
> it were part of SRAM C, not the other way around.

But isn't that what we do? "sram_c1" is just the node name alias used
for the parent node. That is actually never referenced anywhere (in any
of the the H6 .dts), so we can actually remove it, I guess.
The actual SRAM section is called ve_sram already.
And I can't change the compatible name, for the fallback, at least.

I can make the new compatible string read
"allwinner,sun50i-h616-ve-sram", if that helps, but that would mean
deviating from the H6 and other SoCs.

Cheers,
Andre


> 
> Also the sram-section node would make more sense if it were in sram_c, as
> that is the part that gets switched around, not the full region @ 1a0.
> 
> ChenYu
> 
>> Maybe some guys with more VE knowledge can shine some light on this?
>>
>> Cheers,
>> Andre
>>



Re: [linux-sunxi] Re: [PATCH 7/8] arm64: dts: allwinner: Add Allwinner H616 .dtsi file

2020-12-03 Thread Chen-Yu Tsai
On Thu, Dec 3, 2020 at 6:54 PM André Przywara  wrote:
>
> On 03/12/2020 03:16, Samuel Holland wrote:
>
> Hi,
>
> > On 12/2/20 7:54 AM, Andre Przywara wrote:
> > ...
> >> +soc {
> >> +compatible = "simple-bus";
> >> +#address-cells = <1>;
> >> +#size-cells = <1>;
> >> +ranges = <0x0 0x0 0x0 0x4000>;
> >> +
> >> +syscon: syscon@300 {
> >> +compatible = "allwinner,sun50i-h616-system-control",
> >> + "allwinner,sun50i-a64-system-control";
> >> +reg = <0x0300 0x1000>;
> >> +#address-cells = <1>;
> >> +#size-cells = <1>;
> >> +ranges;
> >> +
> >> +sram_c: sram@28000 {
> >> +compatible = "mmio-sram";
> >> +reg = <0x00028000 0x3>;
> >> +#address-cells = <1>;
> >> +#size-cells = <1>;
> >> +ranges = <0 0x00028000 0x3>;
> >> +};
> >> +
> >> +sram_c1: sram@1a0 {
> >> +compatible = "mmio-sram";
> >> +reg = <0x01a0 0x20>;
> >> +#address-cells = <1>;
> >> +#size-cells = <1>;
> >> +ranges = <0 0x01a0 0x20>;
> >> +
> >> +ve_sram: sram-section@0 {
> >> +compatible = 
> >> "allwinner,sun50i-h616-sram-c1",
> >> + 
> >> "allwinner,sun4i-a10-sram-c1";
> >> +reg = <0x00 0x20>;
> >> +};
> >> +};
> >> +};
> >
> > You mentioned that you could not find a SRAM A2. How were these SRAM ranges
> > verified? If you can load eGON.BT0 larger than 32 KiB, then presumably NBROM
> > uses SRAM C, and it is in the manual, but I see no mention of SRAM C1.
>
> The manual says that SRAM C *can* be used by "the system", at boot time,
> as long as it's configured correctly. I couldn't find any details on how
> to switch clock sources for SRAM C, and the manual stanza on this is
> quite gibberish. I presume it's configured either by BROM or by reset
> default this way. I think the idea is that the later users (VE, DE) take
> ownership at some point (which means we can't run any firmware in there).
> The BSP boot0 is 48KB already, so reaching into SRAM C, and the code
> itself heavily uses SRAM C (found by hacking boot0 to drop to FEL and
> inspecting the memory afterwards).
>
> For C1: I copied this name from the H6 .dtsi, the manual calls this
> "VE-SRAM", in both manuals, and the description looks identical there
> for both SoCs. I think this will be later used by the video engine, so I
> kept it in. The large size made me suspicious, and from former
> experiments it looks like being aliased to (parts of) SRAM C.

I would just call it sram_ve or ve_sram. SRAM C1 would make more sense if
it were part of SRAM C, not the other way around.

Also the sram-section node would make more sense if it were in sram_c, as
that is the part that gets switched around, not the full region @ 1a0.

ChenYu

> Maybe some guys with more VE knowledge can shine some light on this?
>
> Cheers,
> Andre
>
> --
> You received this message because you are subscribed to the Google Groups 
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-sunxi+unsubscr...@googlegroups.com.
> To view this discussion on the web, visit 
> https://groups.google.com/d/msgid/linux-sunxi/34e5618e-4a3d-9a46-5077-179c82592fce%40arm.com.