Re: [RFC PATCH v2 4/5] rust/hw/core: Provide some interfaces for the GPIO device

["Chen Miao" <chenmiao@openatom.club>]

On 10/28/2025 7:30 PM, Paolo Bonzini wrote:
> On Tue, Oct 28, 2025 at 12:24 PM Chen Miao <chenmiao@openatom.club> wrote:
>> On 10/28/2025 6:49 PM, Paolo Bonzini wrote:
>>> On Tue, Oct 28, 2025 at 11:18 AM chenmiao <chenmiao@openatom.club> wrote:
>>>> In irq.rs, we added a new get method for the InterruptSource type to determine
>>>> whether an InterruptSource is null. This eliminates the need to repeatedly
>>>> call self.cell.get().is_null() for null checks during comparisons.
>>>> Additionally, we exposed the slice_as_ptrmethod to support external usage with
>>>> the &[InterruptSource]type.
>>>>
>>>> In qdev.rs, we implemented the init_gpio_out_namedfunction, which corresponds
>>>> to the C function qdev_init_gpio_out_named. We also refactored the
>>>> init_gpio_outfunction to reuse the init_gpio_out_namedinterface.
>>>>
>>>> Signed-off-by: chenmiao <chenmiao@openatom.club>
>>>> ---
>>>>    rust/hw/core/src/irq.rs  |  6 +++++-
>>>>    rust/hw/core/src/qdev.rs | 12 +++++++++---
>>>>    2 files changed, 14 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/rust/hw/core/src/irq.rs b/rust/hw/core/src/irq.rs
>>>> index e0d7784d97..dd5d0cadbc 100644
>>>> --- a/rust/hw/core/src/irq.rs
>>>> +++ b/rust/hw/core/src/irq.rs
>>>> @@ -71,6 +71,10 @@ pub fn pulse(&self) {
>>>>        pub fn raise(&self) {
>>>>            self.set(true);
>>>>        }
>>>> +
>>>> +    pub fn get(&self) -> bool {
>>>> +        !self.cell.get().is_null()
>>>> +    }
>>> This should not be get(), but "is_connected()". Also it should be
>>> implemented for any T, therefore in the "impl<T> InterruptSource<T>"
>>> block below.
>> I'll fix it later.
> You can drop it actually (see review of 5/5).
Ok!
>>>>    }
>>>>
>>>>    impl<T> InterruptSource<T>
>>>> @@ -91,7 +95,7 @@ pub(crate) const fn as_ptr(&self) -> *mut *mut bindings::IRQState {
>>>>            self.cell.as_ptr()
>>>>        }
>>>>
>>>> -    pub(crate) const fn slice_as_ptr(slice: &[Self]) -> *mut *mut bindings::IRQState {
>>>> +    pub const fn slice_as_ptr(slice: &[Self]) -> *mut *mut bindings::IRQState {
>>>>            assert!(!slice.is_empty());
>>>>            slice[0].as_ptr()
>>>>        }
>>> Since you are not using this, you don't need to expose it outside the crate.
>> I have used this function in the next patch.
> Isn't it commented out? The code that is used is:
>
> +        self.init_gpio_in(self.handler_size(), PCF8574State::gpio_set);
> +        self.init_gpio_out(from_ref(&self.handler[0]));
> +        self.init_gpio_out_named(from_ref(&self.intrq), "nINT", 1);

Oh, sorry, I was referring to my initial version. Initially, this function was 
used externally, but later I implemented init_gpio_out_named, and now it's 
internal again.

You're right, this function should not be exposed.

Chen Miao

>