On Wed Jun 4, 2025 at 7:23 PM JST, Danilo Krummrich wrote:
> On Wed, May 21, 2025 at 03:45:12PM +0900, Alexandre Courbot wrote:
>> +impl Chipset {
>> +    /// Returns the HAL corresponding to this chipset.
>> +    pub(super) fn get_fb_fal(self) -> &'static dyn FbHal {
>
> Please don't use the 'get' prefix here.

Yup, I've fixed that after your other comment.

>
> Also, I feel like it's a bit random to have this on Chipset. I think the
> standalone function to create a HAL was fine. (Same for falcon/hal.rs, where I
> missed this.)

Works just as well, also probably a bit more consistent if we need to
create HALs that depend in things other than the chipset.

Reply via email to