On Wed Mar 26, 2025 at 9:51 PM CET, Rob Herring wrote:
> On Wed, Mar 26, 2025 at 06:13:40PM +0100, Remo Senekowitsch wrote:
>>
>> +impl Device {
>> + /// Obtain the fwnode corresponding to the device.
>> + fn fwnode(&self) -> &FwNode {
>> + // SAFETY: `self` is valid.
>> + let fwnode_handle = unsafe { bindings::dev_fwnode(self.as_raw()) };
>> + if fwnode_handle.is_null() {
>> + panic!("fwnode_handle cannot be null");
>
> It's usually not a good idea to panic the kernel especially with
> something a driver calls as that's probably recoverable.
>
> Users/drivers testing fwnode_handle/of_node for NULL is pretty common.
> Though often that's a legacy code path, so maybe not allowing NULL is
> fine for now.
Just to be clear on this, should I keep this as is, or return a result?
In the latter case, all the duplicated methods on `Device` that just
call `self.fwnode().same_method()` would have a result in their function
signatur as well. That includes `property_present`, `read_property`
and `children`.
>> + }
>> + // SAFETY: `fwnode_handle` is valid. Its lifetime is tied to `&self`. We
>> + // return a reference instead of an `ARef<FwNode>` because `dev_fwnode()`
>> + // doesn't increment the refcount.
>> + unsafe { &*fwnode_handle.cast() }
>> + }
>> +
>> + /// Checks if property is present or not.
>> + pub fn property_present(&self, name: &CStr) -> bool {
>> + self.fwnode().property_present(name)
>> + }
>> +}
>
> The C developer in me wants to put this after the FwNode stuff since
> this uses it.
Is that just a comment or a call to action? :-)
Remo
