Re: [PATCH v3 3/7] rust: property: Introduce PropertyGuard

Remo Senekowitsch
2025-04-26T21:50:45Z

On Sat Apr 26, 2025 at 5:02 PM CEST, Danilo Krummrich wrote:
> On Sat, Apr 26, 2025 at 04:35:07PM +0200, Dirk Behme wrote:
>> On 26.04.25 16:19, Danilo Krummrich wrote:
>> > On Sat, Apr 26, 2025 at 01:08:39PM +0200, Remo Senekowitsch wrote:
>> >> On Sat Apr 26, 2025 at 12:15 PM CEST, Danilo Krummrich wrote:
>> >>> If it'd be possible to use dev_err!() instead I wouldn't object in this specific
>> >>> case. But this code is used by drivers from probe(), hence printing the error
>> >>> without saying for which device it did occur is a bit pointless.
>> >>>
>> >>> Drivers can still decide to properly print the error if the returned Result
>> >>> indicates one.
>> >>
>> >> One alternative would be to store a reference count to the device in
>> >> `FwNode`. At that point we'd be guaranteed to have a valid reference
>> >> whenever we want to log something.
>> >
>> > Yes, that would work. However, I'm not convinced that it's worth to store an
>> > ARef<Device> (i.e. take a device reference) in each FwNode structure *only* to
>> > be able to force an error print if a required device property isn't available.
>> >
>> > Why do you think it is important to force this error print by having it in
>> > PropertyGuard::required() and even take an additional device reference for this
>> > purpose, rather than leaving it to the driver when to print a message for an
>> > error condition that makes it fail to probe()?
>>
>> To my understanding doing the error print in "core" was proposed by
>> Rob [1]:
>
> That is fine, though it doesn't answer my question above. :)

If the question is addressed to me, I don't think it is important.
I don't have a particular preference either way. I'm just trying to
come up with a solution that is satisfactory to everyone. We should
hear from Rob if he's ok with removing the logging entirely given the
limitations.