Re: [PATCH v5 2/4] drm: Expose wedge recovery methods

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 19 Sep 2024, Raag Jadav <raag.jadav@intel.com> wrote:
> On Tue, Sep 17, 2024 at 10:49:07AM +0300, Jani Nikula wrote:
>> On Tue, 17 Sep 2024, Raag Jadav <raag.jadav@intel.com> wrote:
>> > Now that we have device wedged event in place, add wedge_recovery sysfs
>> > attribute which will expose recovery methods supported by the DRM device.
>> > This is useful for userspace consumers in cases where the device supports
>> > multiple recovery methods which can be used as fallbacks.
>> >
>> > $ cat /sys/class/drm/card0/wedge_recovery
>> > rebind
>> > bus-reset
>> > reboot
>> >
>> > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
>> > ---
>> >  drivers/gpu/drm/drm_sysfs.c | 23 +++++++++++++++++++++++
>> >  1 file changed, 23 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
>> > index fb3bbb6adcd1..b88cdbfa3b5e 100644
>> > --- a/drivers/gpu/drm/drm_sysfs.c
>> > +++ b/drivers/gpu/drm/drm_sysfs.c
>> > @@ -36,6 +36,8 @@
>> >  #define to_drm_minor(d) dev_get_drvdata(d)
>> >  #define to_drm_connector(d) dev_get_drvdata(d)
>> >  
>> > +extern const char *const wedge_recovery_opts[];
>> 
>> Data is not an interface. Please add a function for this.
>
> For a single user?

Yes.

Well, you kind of have two, and both places need to do bounds checking
on indexing the array. You also need to do bounds checking on the string
manipulation, you can't just strcat and assume it'll be all right.

BR,
Jani.


>
>> Side note, extern declarations for outside stuff don't belong in .c
>> files anyway.
>
> Sure.
>
> Raag

-- 
Jani Nikula, Intel