Re: [RFC] soc: qcom: socinfo.rs: Add Rust Socinfo Driver implementation

[Wei Liu <wei.liu@kernel.org>]

On Mon, Aug 16, 2021 at 06:23:32PM -0700, Antonio Martorana wrote:
[...]
> new file mode 100644
> index 0000000..8af697a
> --- /dev/null
> +++ b/drivers/soc/qcom/socinfo_rust.rs
> @@ -0,0 +1,464 @@
> +//! Qualcomm support for socinfo.c
> +
> +#![no_std]
> +#![feature(allocator_api,global_asm)]

Not really related to this patch, but I wish there was a more ergonomic
way to specify these crate level features. I think these two lines are
c&p in a lot of places.

[...]
> +/*
> + * SoC version type with major number in the upper 16 bits and minor
> + * number in the lower 16 bits.
> + */
> +const fn socinfo_major(ver: u32) -> u32{
> +    let major: u32 = (ver >> 16) & 0xFFFF;
> +    return major;

This can be simplified as

    const fn socinfo_major(ver: u32) -> u32 {
        (ver >> 16) & 0xFFFF
    }

> +}
> +
> +const fn socinfo_minor(ver: u32) -> u32{
> +    let minor: u32 = ver & 0xFFFF;
> +    return minor;
> +}
> +
> +const fn socinfo_version(maj: u32, min: u32) -> u32{
> +    let version: u32 = ((maj & 0xFFFF) << 16) | (min & 0xFFFF);
> +    return version;
> +}

Same for these two functions. They can be simplified as well.

I have one further question: why aren't these helpers implemented as
methods of SocInfo? Like:

    impl SocInfo {
        fn major(&self) -> u32 {
	    (self.ver >> 16) & 0xFFFF
	}
    }

Thanks,
Wei.