lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e3222caa-ec69-2e90-ef81-666b03da656d@mediatek.com>
Date:   Tue, 15 Feb 2022 14:47:33 +0800
From:   Macpaul Lin <macpaul.lin@...iatek.com>
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        "thierry.reding@...il.com" <thierry.reding@...il.com>
CC:     "u.kleine-koenig@...gutronix.de" <u.kleine-koenig@...gutronix.de>,
        "lee.jones@...aro.org" <lee.jones@...aro.org>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
        "linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "kernel@...labora.com" <kernel@...labora.com>
Subject: Re: [PATCH v2 3/3] pwm: pwm-mediatek: Beautify error messages text

On 2/14/22 10:03 PM, AngeloGioacchino Del Regno wrote:
> As a cherry-on-top cleanup, make error messages clearer to read
> by changing instances of "clock: XXXX failed" to a more readable
> "Failed to get XXXX clock". Also add "of" to unsupported period
> error.
> 
> This is purely a cosmetic change; no "real" functional changes.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> ---
>   drivers/pwm/pwm-mediatek.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
> index 6b39f3d69e41..568b13a48717 100644
> --- a/drivers/pwm/pwm-mediatek.c
> +++ b/drivers/pwm/pwm-mediatek.c
> @@ -146,7 +146,7 @@ static int pwm_mediatek_config(struct pwm_chip *chip, struct pwm_device *pwm,
>   
>   	if (clkdiv > PWM_CLK_DIV_MAX) {
>   		pwm_mediatek_clk_disable(chip, pwm);
> -		dev_err(chip->dev, "period %d not supported\n", period_ns);
> +		dev_err(chip->dev, "period of %d ns not supported\n", period_ns);
>   		return -EINVAL;
>   	}
>   
> @@ -229,12 +229,12 @@ static int pwm_mediatek_probe(struct platform_device *pdev)
>   	pc->clk_top = devm_clk_get(&pdev->dev, "top");
>   	if (IS_ERR(pc->clk_top))
>   		return dev_err_probe(&pdev->dev, PTR_ERR(pc->clk_top),
> -				     "clock: top failed\n");
> +				     "Failed to get top clock\n");
>   
>   	pc->clk_main = devm_clk_get(&pdev->dev, "main");
>   	if (IS_ERR(pc->clk_main))
>   		return dev_err_probe(&pdev->dev, PTR_ERR(pc->clk_main),
> -				     "clock: main failed\n");
> +				     "Failed to get main clock\n");
>   
>   	for (i = 0; i < pc->soc->num_pwms; i++) {
>   		char name[8];
> @@ -244,7 +244,7 @@ static int pwm_mediatek_probe(struct platform_device *pdev)
>   		pc->clk_pwms[i] = devm_clk_get(&pdev->dev, name);
>   		if (IS_ERR(pc->clk_pwms[i]))
>   			return dev_err_probe(&pdev->dev, PTR_ERR(pc->clk_pwms[i]),
> -					     "clock: %s failed\n", name);
> +					     "Failed to get %s clock\n", name);
>   	}
>   
>   	pc->chip.dev = &pdev->dev;
> 

The format of these debug messages "clock: top" or "clock: main" is  
meant to keep both human and machine's readability at the same time.
This kind of format is much more easier to parse by scripts, which the  
driver's category and sub nodes are separated by delimiters . If a fail  
log has been provided, the script could indicated where the issue might  
exists probably. Device vender, field application engineer, and driver  
maintainer could be able to write and use the error log parser before  
debugging.

I'm not sure if this kind of format will be better. Like, "Failed to get  
clock: %s".

If most people like this kind of solution ("Failed to get clock: %s\n"),  
then you can have the reviewed-by tag.
Thanks!

Reviewed-by: Macpaul Lin <macpaul.lin@...iatek.com>

Regards, :)
Macpaul Lin

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ