Re: [PATCH v1 2/2] drm/xe/xe_ras: Add RAS support for GPU health indicator

[Andi Shyti <andi.shyti@linux.intel.com>]

Hi Rodrigo,

> > > +static const char * const gpu_health_states[] = { "ok", "warning", "critical" };
> > > +static const char * const gpu_health_fmt[] = {
> > > +	"[%s] %s %s\n",
> > > +	"%s [%s] %s\n",
> > > +	"%s %s [%s]\n",
> > > +};
> > 
> > Please, don't use complex sentences in sysfs outputs. Use a
> > single string/character/value
> 
> I like this one better. So we don't need to have an uAPI entry to define
> the meaning of 0, 1, 2.
> 
> Regarding the sysfs rules, as long as it is one entry per sysfs we should
> be compliant with the rule. So, we should be good here.
> 
> This style is consistent with the style used in /sys/power/ entries for instance:

Well, if it's a sysfs interface, it's a uAPI entry and needs to
be documented (something that I don't see in this series, BTW).

The sysfs interfaces don't really need to be 1, 2, etc, they can
also be strings, as long as userspace applications are able to
easily parse the content and know what to expect from them.

If we need two strings, then we create two files.

If we just need to provide information to system administrators,
then we can either use the debugfs or procfs that don't need to
be considered as uAPI.

Andi

> $ cat /sys/power/mem_sleep
> [s2idle] deep
> 
> $ cat /sys/power/state
> freeze mem disk