Received-SPF: pass (google.com: domain of linux-kernel+bounces-207050-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <0a45a87f-84d1-4fb7-adaf-c612af68cd40@gmail.com>
Date: Sat, 8 Jun 2024 15:57:36 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/1] arm: rust: Enable Rust support for ARMv7
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
 Jamie Cunliffe <jamie.cunliffe@arm.com>
Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
 rust-for-linux@vger.kernel.org, Miguel Ojeda <ojeda@kernel.org>,
 Sven Van Asbroeck <thesven73@gmail.com>
References: <4e0f5932-c7bc-4878-862c-1186cbecd71d@gmail.com>
 <CANiq72kaVw_t=hCJ-qEr=9ybuOngAR6isnCK717dBEm7CFm2NQ@mail.gmail.com>
Content-Language: en-US
From: Christian Schrefl <chrisi.schrefl@gmail.com>
In-Reply-To: <CANiq72kaVw_t=hCJ-qEr=9ybuOngAR6isnCK717dBEm7CFm2NQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On 08.06.24 2:16 PM, Miguel Ojeda wrote:
> On Sat, Jun 8, 2024 at 12:30 AM Christian Schrefl
> <chrisi.schrefl@gmail.com> wrote:
>>
>> This is based on the code by Sven Van Asbroeck from the original
>> rust branch and inspired by the AArch version by Jamie Cunliffe.
> 
> Thanks for mentioning that!
> 
> (AArch64 perhaps?)

Ah yes I meant AArch64. Fixed it locally.

>> I have tested the rust samples and a custom simple MMIO module
>> on on hardware (De1SoC FPGA + Arm A9 CPU).
> 
> Very nice to have tested it in real hardware, thanks!
> 
> (Duplicate "on on").

I fixed that locally, but forgot to run format-patch again, sorry! 

>> +``arm``        Maintained        ARMv7 Little Endian only.
> 
> How hard would it be to test and maintain other versions, eventually?
> I met Jamie recently, he may have some ideas here.

I think it would just be setting the correct rust target,
but I have no hardware to test it and I think its best to
only do v7 for the initial support.

>> -BINDGEN_TARGET_arm64   := aarch64-linux-gnu
>> +BINDGEN_TARGET_arm64:= aarch64-linux-gnu
> 
> Spurious change?

I've changed that so the := aligns with the others, but I can
remove that for v2 if you want

>> +# Depending on how the architecute defines ARCH_SLAB_MINALIGN, bindgen might generate a binding.
>> +# Disable this here as there is a const that will always be generated in bindings_helper.c
> 
> Typo in "architecute". Also, I guess you mean `.h` instead. Please use
> Markdown formatting for consistency with the rest of the file.

Fixed.

>> +        panic!("arm uses the builtin rustc target");
> 
> Since ARM64 puts the name of the target in the message, it would be
> nice to do the same (or perhaps it can be removed from the ARM64 one
> otherwise).

I can add the target here but then the message would have to depend
on the sub architecture if we support v6 in the future. 

> Cheers,
> Miguel