Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <bc79f3dd-425f-8f91-bccb-69bf263ed8b2@amd.com>
Date:   Mon, 19 Dec 2022 16:35:11 -0600
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.0
Subject: Re: [PATCH v8 13/13] Documentation: amd-pstate: introduce new global
 sysfs attributes
Content-Language: en-US
To:     Perry Yuan <perry.yuan@amd.com>, rafael.j.wysocki@intel.com,
        ray.huang@amd.com, viresh.kumar@linaro.org
Cc:     Deepak.Sharma@amd.com, Nathan.Fontenot@amd.com,
        Alexander.Deucher@amd.com, Shimmer.Huang@amd.com,
        Xiaojian.Du@amd.com, Li.Meng@amd.com, wyes.karny@amd.com,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20221219064042.661122-1-perry.yuan@amd.com>
 <20221219064042.661122-14-perry.yuan@amd.com>
From:   "Limonciello, Mario" <mario.limonciello@amd.com>
In-Reply-To: <20221219064042.661122-14-perry.yuan@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MmovVDJUeXJjZ0FxS2ZLOFZwZ1dZY0NJSGxaTFhkMHluaTJnSXVuSks0V3By?=
 =?utf-8?B?Y2x1VUZUNUZuanN1ZTl6d3JMaHBZZ3hscmRlYVM1MDIwd0hpN0pJK0Q3UmRh?=
 =?utf-8?B?Z3pxT3FjWkNZODVKTjROc3lyaG5vbWxNdVBFMFdEOTA5VytPNnpaREZsQk1z?=
 =?utf-8?B?WTltaWx3QUg3N1hyYnpYM2VMNEJDanJsby9IdmorVVVReDNXZ2FENjhVQjAr?=
 =?utf-8?B?d0V2WU0rV25tWnFGRVB3Qk9UdDhWb1JtZE0wekZMTlY1aHQvc1U4UDdzaEFp?=
 =?utf-8?B?ckN5MnZ5ZDY4WndLa0RNN1VseHkvL2VRZ2gwbVBqNHpPc1d6ZU1pY3RVUkdp?=
 =?utf-8?B?bVIrUDlFMXdMRWwzOExiTHo3R1ZHMWtHUjN3dTN3dGhabXBDNGlyVGlrZTdx?=
 =?utf-8?B?NzNOa3hTN3drSDhzWWlhanAvVGtsbXRGWnRjd0JSSk5aS3R5ZjBobnVjVHVa?=
 =?utf-8?B?bGJiZTBRc0pCdGtPdEhKMjlEdmFBMUtHamFkNG1vQnU1NTEva3Q3RjlKRDNO?=
 =?utf-8?B?djVTREwrRUp2c0xwSVllQ3QrallvWWMrK1g1QTczcnVxUUl0Vm81Tkd4c2lv?=
 =?utf-8?B?M0ZzUGxYSDEvQ0svTERFRFIxU0k0eVovdHU5WGRpMGh4LzNlWnkzTitwRUcr?=
 =?utf-8?B?QTZEQnRQUzBvUEwvMFVZZC9MZ1pPbW1UTDVqSFM3TjNxTThLNUwrZTh2TFpU?=
 =?utf-8?B?K1I4ODlqY2xLTHZpWEM4eGtxd0FBcVZXdW5DUk9EeFFWSVExMUNocEpMOEpt?=
 =?utf-8?B?bWM1a2FvcDBQcjdRMTFBRzdYZ2NacWNHTWFVV1ZUcTdSZHk4MWhPU1ZrbWFq?=
 =?utf-8?B?U2NPRmJtUFRDTXRUMzNOaE9hcmNlNlpBS0p2NEtDQmRpN1l4cUI4WG1Kc3ZQ?=
 =?utf-8?B?QXNPQktpTStCL0lOU2VGcmFTcDAxVVFQeERMLzlDbFdrUUx1ZzZuay9YTHdO?=
 =?utf-8?B?SjRjcTJ1cS8raTQzZ0J0U2JWUW0zWGFyWU5COEdKWURJeFRIWCtnWVBXRTJB?=
 =?utf-8?B?SlJ5SzdzV2owU0lZOUxxRTVkSWtHMmRGcjg5amx5dXFueXF6UldaM0E4RmpC?=
 =?utf-8?B?YitPUUcybWRwL2UrMWU0UlRBUVFmR28vUSsyWkp6cmhoaUVKZnpqQXpkdXJt?=
 =?utf-8?B?NmlqT1YyQkdNYkxEVDJyQXRWYlVLUVNMbC9hSTExZTlzS3cyL1E0MTBZaUwy?=
 =?utf-8?B?Mzg1WlhyOFhiZ002aTRPbUwxNmJnNVpoaDJGN21RSTZSZDQ2NG1LbmlGc1U1?=
 =?utf-8?B?K3VaYTV2VlVrV2hDLzgrMmdlU3NGUmtCRVp4ZUhQaXJnUWJYb2tzUFUyQkFD?=
 =?utf-8?B?b3lCQUc5TS9XVUVXWDBTK21JbDZPWng1Ukw4VWNmRXFSSjEzYzRUU05DUHpM?=
 =?utf-8?B?VS9oS1BWQ01IcUdHZ0hDc0Jjdlgxdk5aRHBoeVAwSXdoVXFCSXJIeWdGcWVw?=
 =?utf-8?B?OStnc1NZRmtBeVFrY3QrNGpyY1BFMmZqdjV0aEU1S1YwSlc5K2Y0eVYvYTh3?=
 =?utf-8?B?aUw1RzNIeFlCbUR5WEZGWXNoalBaczAxUEVXT2Q1aWV6R3ErenEyVFhxMVRD?=
 =?utf-8?B?Q0NOOTcxaHV6bmpJakNORmRndjN2NEg2UlNnMzYxbGlRQmorOSt5cUdlbnB0?=
 =?utf-8?B?dys4bDc5QmZUd1lDOFd6ZFVWNHdmOTVRb0xnczBzT1R5d0t6MmMwZm91NkZL?=
 =?utf-8?B?bmdhbWpudkpiaitOSlpIUHZ1K3BhaDBGUk5qS090RTMyNUNnNE4zMUd5c2ZT?=
 =?utf-8?B?TzRsdGRxSVF2MHRMNWRVUUhvMmZQdisxenpJUnFqNnJ0MFpGS202NHF6QUdq?=
 =?utf-8?B?RmlSYUZiMTgxUHNuS0lZdGkvNTlyejRrK0VvSm5rK2s1aTk5cko3Uy9FVHFr?=
 =?utf-8?B?U3lNMFd4YWlaWlY4eVhnQzdmYVRvKzJRYy82Z2JMOEZMdVpTc3JhRFRzRXll?=
 =?utf-8?B?UEcrTmpnSm9kY2hwb0d6bmRhMHhXVklhb3Flbkc2eTZKUVJycTZLM21LN1Q1?=
 =?utf-8?B?NXdrdjdnZ1dFY2lCSkVGdTVrdnZZazd2elhWYUNLbzNKdW5wMzI3T3l1d2g2?=
 =?utf-8?B?bm9nWXd6MVBDU0pDcStJU3d0OFJJbmUzaStWSDRMRFJPdFliak1tczFzREFm?=
 =?utf-8?Q?EHwk+Xl1pm759ogRw1skhekt2?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 830f872d-0beb-4770-459c-08dae2114c31
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Dec 2022 22:35:14.8839
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 6fmnH7lEZ4o90n1TWebgPozoyf/W8ztwi4K39QyPSLRUUHGMic7fW3dOCz6JtZSBbV3jAuZbfwzrF9TAclwXeQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR12MB5981
X-Spam-Status: No, score=-3.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 12/19/2022 00:40, Perry Yuan wrote:
> The new amd-pstate driver support to switch the driver working mode and

Words like "new" don't age well.

> use can switch the driver mode within the sysfs attributes in the below
> path and check current mode

Perhaps "The amd-pstate driver supports switching working modes at runtime.
Users can view and change modes by interacting with the "status" sysfs 
attribute.

> 
> $ cd /sys/devices/system/cpu/amd-pstate
> 

No need to change directories; you're demonstrating below using full paths.

> check driver mode:
> $ cat /sys/devices/system/cpu/amd-pstate/status
> 
> switch mode:
> $ sudo bash -c "echo passive  > /sys/devices/system/cpu/amd-pstate/status"
> or
> $ sudo bash -c "echo active  > /sys/devices/system/cpu/amd-pstate/status"

Another way you could suggest this:

# echo "passive" | sudo tee /sys/devices/system/cpu/amd-pstate/status
or
# echo "active" | sudo tee /sys/devices/system/cpu/amd-pstate/status

I don't feel strongly which way to suggest though.

> 
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> ---
>   Documentation/admin-guide/pm/amd-pstate.rst | 28 +++++++++++++++++++++
>   1 file changed, 28 insertions(+)
> 
> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
> index 62744dae3c5f..f3a8f8a66783 100644
> --- a/Documentation/admin-guide/pm/amd-pstate.rst
> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> @@ -339,6 +339,34 @@ processor must provide at least nominal performance requested and go higher if c
>   operating conditions allow.
>   
>   
> +User Space Interface in ``sysfs``
> +=================================
> +
> +Global Attributes
> +-----------------
> +
> +``amd-pstate`` exposes several global attributes (files) in ``sysfs`` to
> +control its functionality at the system level.  They are located in the
> +``/sys/devices/system/cpu/amd-pstate/`` directory and affect all CPUs.
> +
> +``status``
> +	Operation mode of the driver: "active", "passive" or "off".
> +
> +	"active"
> +		The driver is functional and in the ``active mode``
> +
> +	"passive"
> +		The driver is functional and in the ``passive mode``
> +
> +	"off"
> +		The driver is unregistered and not functional now.
> +
> +	This attribute can be written to in order to change the driver's
> +	operation mode or to unregister it.  The string written to it must be
> +	one of the possible values of it and, if successful, the write will

I think this is implied that you wrote a possible value and if it 
returns success something happens.  That's how all sysfs files work and 
it's needlessly wordy.

> +	cause the driver to switch over to the operation mode represented by
> +	that string - or to be unregistered in the "off" case.
Considering the implication of my above comment I think you can reword 
this as:

"Writing one of these values to the sysfs file will cause the driver to..."

> +
>   ``cpupower`` tool support for ``amd-pstate``
>   ===============================================
>