[PATCH v5 5/8] clk: Add dump operation to clk_ops

[Sean Anderson]

On 11/4/23 14:09, Igor Prusov wrote:
> On Sat, Nov 04, 2023 at 11:24:32AM -0400, Sean Anderson wrote:
>> On 11/2/23 08:20, Igor Prusov wrote:
>>> This adds dump function to struct clk_ops which should replace
>>> soc_clk_dump. It allows clock drivers to provide custom dump
>>> implementation without overriding generic CCF dump function.
>>>
>>> Signed-off-by: Igor Prusov <ivprusov at sberdevices.ru>
>>> Reviewed-by: Patrice Chotard <patrice.chotard at foss.st.com>
>>> Tested-by: Patrice Chotard <patrice.chotard at foss.st.com>
>>> ---
>>>    include/clk-uclass.h | 15 +++++++++++++++
>>>    1 file changed, 15 insertions(+)
>>>
>>> diff --git a/include/clk-uclass.h b/include/clk-uclass.h
>>> index a22f1a5d84..793bf14160 100644
>>> --- a/include/clk-uclass.h
>>> +++ b/include/clk-uclass.h
>>> @@ -25,6 +25,7 @@ struct ofnode_phandle_args;
>>>     * @set_parent: Set current clock parent
>>>     * @enable: Enable a clock.
>>>     * @disable: Disable a clock.
>>> + * @dump: Print clock information.
>>>     *
>>>     * The individual methods are described more fully below.
>>>     */
>>> @@ -39,6 +40,9 @@ struct clk_ops {
>>>    	int (*set_parent)(struct clk *clk, struct clk *parent);
>>>    	int (*enable)(struct clk *clk);
>>>    	int (*disable)(struct clk *clk);
>>> +#if IS_ENABLED(CONFIG_CMD_CLK)
>>> +	int (*dump)(struct udevice *dev);
>>> +#endif
>>>    };
>>>    #if 0 /* For documentation only */
>>> @@ -135,6 +139,17 @@ int enable(struct clk *clk);
>>>     * Return: zero on success, or -ve error code.
>>>     */
>>>    int disable(struct clk *clk);
>>> +
>>> +/**
>>> + * dump() - Print clock information.
>>> + * @clk:	The clock device to dump.
>>> + *
>>> + * If present, this function is called by "clk dump" command for each
>>> + * bound device.
>>> + *
>>> + * Return: zero on success, or -ve error code.
>>> + */
>>> +int dump(struct udevice *dev);
>>
>> Actually, this should return void, since we don't do anything with the return code.
> Good catch! Though there is, for example, zynqmp_clk_dump() that may
> return an error code. Wouldn't it be better to print an error message
> with the code in soc_clk_dump()? It might be convinient to have common
> code handling unexpected errors during dump.

Since this function is for printing, if the driver gets an error
it should just print the error itself. It can probably provide a better
error message than we can. And this command is mainly informational anyway,
so we don't really need to set the return code (e.g. $?).

--Sean