Re: [PATCH 1/2] rust: Add initial interconnect framework abstractions

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 7/23/25 1:41 PM, Miguel Ojeda wrote:
> On Wed, Jul 23, 2025 at 1:32 PM Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> The C framework makes no effort to check for that, so panicking is at
>> least something.. That said, what would you suggest to do here?
> 
> If you want to mimic the C side, then you will need to use one of the
> non-panicking operations, such as e.g. `wrapping_mul()`.
> 
> Otherwise, you could make it a fallible method, i.e. return `Result`.
> 
> Otherwise, I think the panic should be documented in the docs of the
> methods (because callers then really need to be careful).
> 
> Which option to takes depends a bit on the use case and what C
> maintainers consider best for a particular operation.
> 
> For instance, sometimes people have used `build_assert!` because they
> expect that the value is always known at compile-time (after
> optimizations).

build_assert sounds nice, but the value is often retrieved dynamically
(e.g. from the device tree itself). Result seems like the best option.
I'll also add a verse in the docs about the return variants.

> 
>> I was debating that. icc_path represents the interconnect consumer part
>> (i.e. used in device drivers that just need to toggle a bus endpoint),
>> whereas the corresponding provider part (which manages said bus) is not
>> yet abstracted.
>>
>> It would make logical sense to split these two.. with the latter going
>> to icc_provider.rs, perhaps?
> 
> Ah, so I just meant that you could have the `icc_path` as a `mod
> icc_path;`, and move it to its own file, rather than inline. Other
> reorganizations makes sense, but I was only suggesting that :)

So, IccBwUnit stays in icc.rs and icc_path -> icc_path.rs?

> 
>> No, it compiles fine here.. Strangely, I didn't get any warnings or
>> errors with this patch. Maybe because the struct is pub and within the
>> same file?
> 
> It likely happens if `CONFIG_INTERCONNECT` is not set, because then
> the entire module above is gone.

OK that makes sense!

> 
>> I almost wanna say `make rustfmt` produced slightly different results
>> (one or two lines of difference) than make rust-analyzer + vscode
>> extension.. hmm.. Perhaps PEBKAC..
> 
> In mainline we currently enforce that code is formatted with `make ...
> rustfmt` (there is `make ... rustfmtcheck` to check, too), so if some
> extension gives you different formatting, please double-check before
> submitting the code.

I assumed that since the extension name is the same as the make target,
they were meant to be ;)

Konrad