Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Michael Ellerman <mpe@ellerman.id.au>
To:     Fabiano Rosas <farosas@linux.ibm.com>,
        Pratik Sampat <psampat@linux.ibm.com>,
        benh@kernel.crashing.org, paulus@samba.org,
        linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org,
        linux-kernel@vger.kernel.org, pratik.r.sampat@gmail.com
Subject: Re: [RFC] powerpc/pseries: Interface to represent PAPR firmware
 attributes
In-Reply-To: <87tum6vb58.fsf@linux.ibm.com>
References: <20210604163501.51511-1-psampat@linux.ibm.com>
 <87wnr4uhs9.fsf@linux.ibm.com>
 <5c9cb57b-e9d8-0361-8be7-60dc9618db34@linux.ibm.com>
 <87tum6vb58.fsf@linux.ibm.com>
Date:   Wed, 16 Jun 2021 10:03:11 +1000
Message-ID: <875yyeu14w.fsf@mpe.ellerman.id.au>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Fabiano Rosas <farosas@linux.ibm.com> writes:
> Pratik Sampat <psampat@linux.ibm.com> writes:
...
>>>
>>>> The new H_CALL exports information in direct string value format, hence
>>>> a new interface has been introduced in /sys/firmware/papr to export
>>> Hm.. Maybe this should be something less generic than "papr"?
>>
>> The interface naming was inspired from /sys/firmware/opal's naming convention.
>> We believed the name PAPR could serve as more generic name to be used by both
>> Linux running on PHYP and linux on KVM.
>
> Right, I agree with that rationale, but /opal has identifiable elements
> in it whereas /papr would have the generic "attr_X_name", which does not
> give much hint about what they are.
>
> We also expect people to iterate the "attr_X_*" files, so if we decide
> to add something else under /papr in the future, that would potentially
> cause issues with any tool that just lists the content of the directory.
>
> So maybe we should be proactive and put the hcall stuff inside a
> subdirectory already. /papr/energy_scale_attrs comes to mind, but I
> don't have a strong opinion on the particular name.

Maybe we should use the descriptive part of the hcall.

So H_GET_ENERGY_SCALE_INFO -> ../papr/energy_scale_info/

That should help avoid any naming confusion, because every hcall should
have a unique name.

In future if there's ever a H_GET_ENERGY_SCALE_INFO_2 we would then have
to decide if we expose that as a separate directory, or more likely we
would handle that in the kernel and continue to use the existing sysfs
name.

...

> Based on all the new information you provided, I'd say present all the
> data and group it under the ID:
>
> /sys/firmware/papr/energy_scale_attrs/
>    |-- <id>/
>      |-- desc
>      |-- value
>      |-- value_desc
>    |-- <id>/
>      |-- desc
>      |-- value
>      |-- value_desc

Yeah that seems reasonable.

I'd think we should just omit the value_desc if it's empty.

cheers