Re: [PATCH RESEND 2/5] clk: call log_debug() instead to avoid console log printing
On 11/2/2023 2:01 AM, Sean Anderson wrote:
> On 11/1/23 13:55, Sean Anderson wrote:
>> On 8/17/23 13:04, Yang Xiwen via B4 Relay wrote:
>>> From: Yang Xiwen
>>>
>>> it's a very common case to register a clock without a parent, such as
>>> clk_register_fixed_rate().
>>
>> Actually, that seems like the only place this is done.
>>
>>> Replace log_error() with log_debug() to avoid
>>> useless console log if not debugging.
>>>
>>> Signed-off-by: Yang Xiwen
>>> ---
>>> drivers/clk/clk.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>>> index a5a3461b66..a38daaac0c 100644
>>> --- a/drivers/clk/clk.c
>>> +++ b/drivers/clk/clk.c
>>> @@ -24,8 +24,8 @@ int clk_register(struct clk *clk, const char
>>> *drv_name,
>>> ret = uclass_get_device_by_name(UCLASS_CLK, parent_name, );
>>> if (ret) {
>>> - log_err("%s: failed to get %s device (parent of %s)\n",
>>> - __func__, parent_name, name);
>>> + log_debug("%s: failed to get %s device (parent of %s)\n",
>>> + __func__, parent_name, name);
>>> } else {
>>> log_debug("%s: name: %s parent: %s [0x%p]\n", __func__, name,
>>> parent->name, parent);
>>>
>>
>> I think a correct fix would be
>>
>> diff --git i/drivers/clk/clk.c w/drivers/clk/clk.c
>> index a5a3461b66c..cb333c83f66 100644
>> --- i/drivers/clk/clk.c
>> +++ w/drivers/clk/clk.c
>> @@ -18,17 +18,19 @@
>> int clk_register(struct clk *clk, const char *drv_name,
>> const char *name, const char *parent_name)
>> {
>> - struct udevice *parent;
>> + struct udevice *parent = NULL;
>> struct driver *drv;
>> int ret;
>>
>> - ret = uclass_get_device_by_name(UCLASS_CLK, parent_name,
>> );
>> - if (ret) {
>> - log_err("%s: failed to get %s device (parent of %s)\n",
>> - __func__, parent_name, name);
>> - } else {
>> - log_debug("%s: name: %s parent: %s [0x%p]\n",
>> __func__, name,
>> - parent->name, parent);
>> + if (parent_name) {
>> + ret = uclass_get_device_by_name(UCLASS_CLK, parent_name,
>> + );
>> + if (ret)
>> + log_err("%s: failed to get %s device (parent
>> of %s)\n",
>> + __func__, parent_name, name);
>> + else
>> + log_debug("%s: name: %s parent: %s [0x%p]\n",
>> __func__,
>> + name, parent->name, parent);
>> }
>>
>> drv = lists_driver_lookup_name(drv_name);
>>
>> --Sean
>
> or you could modify the condition to be `if (ret && parent_name)` with
> appropriate
> modification of the second debug message.
>
Thanks for your review. I'll use your patch with your SoB and drop mine
in next release.
> --Sean
--
Best regards,
Yang Xiwen
Re: [PATCH RESEND 2/5] clk: call log_debug() instead to avoid console log printing
On 11/1/23 13:55, Sean Anderson wrote:
On 8/17/23 13:04, Yang Xiwen via B4 Relay wrote:
From: Yang Xiwen
it's a very common case to register a clock without a parent, such as
clk_register_fixed_rate().
Actually, that seems like the only place this is done.
Replace log_error() with log_debug() to avoid
useless console log if not debugging.
Signed-off-by: Yang Xiwen
---
drivers/clk/clk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index a5a3461b66..a38daaac0c 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -24,8 +24,8 @@ int clk_register(struct clk *clk, const char *drv_name,
ret = uclass_get_device_by_name(UCLASS_CLK, parent_name, );
if (ret) {
- log_err("%s: failed to get %s device (parent of %s)\n",
- __func__, parent_name, name);
+ log_debug("%s: failed to get %s device (parent of %s)\n",
+ __func__, parent_name, name);
} else {
log_debug("%s: name: %s parent: %s [0x%p]\n", __func__, name,
parent->name, parent);
I think a correct fix would be
diff --git i/drivers/clk/clk.c w/drivers/clk/clk.c
index a5a3461b66c..cb333c83f66 100644
--- i/drivers/clk/clk.c
+++ w/drivers/clk/clk.c
@@ -18,17 +18,19 @@
int clk_register(struct clk *clk, const char *drv_name,
const char *name, const char *parent_name)
{
- struct udevice *parent;
+ struct udevice *parent = NULL;
struct driver *drv;
int ret;
- ret = uclass_get_device_by_name(UCLASS_CLK, parent_name, );
- if (ret) {
- log_err("%s: failed to get %s device (parent of %s)\n",
- __func__, parent_name, name);
- } else {
- log_debug("%s: name: %s parent: %s [0x%p]\n", __func__, name,
- parent->name, parent);
+ if (parent_name) {
+ ret = uclass_get_device_by_name(UCLASS_CLK, parent_name,
+ );
+ if (ret)
+ log_err("%s: failed to get %s device (parent of %s)\n",
+ __func__, parent_name, name);
+ else
+ log_debug("%s: name: %s parent: %s [0x%p]\n", __func__,
+ name, parent->name, parent);
}
drv = lists_driver_lookup_name(drv_name);
--Sean
or you could modify the condition to be `if (ret && parent_name)` with
appropriate
modification of the second debug message.
--Sean
Re: [PATCH RESEND 2/5] clk: call log_debug() instead to avoid console log printing
On 8/17/23 13:04, Yang Xiwen via B4 Relay wrote:
From: Yang Xiwen
it's a very common case to register a clock without a parent, such as
clk_register_fixed_rate().
Actually, that seems like the only place this is done.
Replace log_error() with log_debug() to avoid
useless console log if not debugging.
Signed-off-by: Yang Xiwen
---
drivers/clk/clk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index a5a3461b66..a38daaac0c 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -24,8 +24,8 @@ int clk_register(struct clk *clk, const char *drv_name,
ret = uclass_get_device_by_name(UCLASS_CLK, parent_name, );
if (ret) {
- log_err("%s: failed to get %s device (parent of %s)\n",
- __func__, parent_name, name);
+ log_debug("%s: failed to get %s device (parent of %s)\n",
+ __func__, parent_name, name);
} else {
log_debug("%s: name: %s parent: %s [0x%p]\n", __func__, name,
parent->name, parent);
I think a correct fix would be
diff --git i/drivers/clk/clk.c w/drivers/clk/clk.c
index a5a3461b66c..cb333c83f66 100644
--- i/drivers/clk/clk.c
+++ w/drivers/clk/clk.c
@@ -18,17 +18,19 @@
int clk_register(struct clk *clk, const char *drv_name,
const char *name, const char *parent_name)
{
- struct udevice *parent;
+ struct udevice *parent = NULL;
struct driver *drv;
int ret;
- ret = uclass_get_device_by_name(UCLASS_CLK, parent_name, );
- if (ret) {
- log_err("%s: failed to get %s device (parent of %s)\n",
- __func__, parent_name, name);
- } else {
- log_debug("%s: name: %s parent: %s [0x%p]\n", __func__, name,
- parent->name, parent);
+ if (parent_name) {
+ ret = uclass_get_device_by_name(UCLASS_CLK, parent_name,
+ );
+ if (ret)
+ log_err("%s: failed to get %s device (parent of %s)\n",
+ __func__, parent_name, name);
+ else
+ log_debug("%s: name: %s parent: %s [0x%p]\n", __func__,
+ name, parent->name, parent);
}
drv = lists_driver_lookup_name(drv_name);
--Sean
[PATCH RESEND 2/5] clk: call log_debug() instead to avoid console log printing
From: Yang Xiwen
it's a very common case to register a clock without a parent, such as
clk_register_fixed_rate(). Replace log_error() with log_debug() to avoid
useless console log if not debugging.
Signed-off-by: Yang Xiwen
---
drivers/clk/clk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index a5a3461b66..a38daaac0c 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -24,8 +24,8 @@ int clk_register(struct clk *clk, const char *drv_name,
ret = uclass_get_device_by_name(UCLASS_CLK, parent_name, );
if (ret) {
- log_err("%s: failed to get %s device (parent of %s)\n",
- __func__, parent_name, name);
+ log_debug("%s: failed to get %s device (parent of %s)\n",
+ __func__, parent_name, name);
} else {
log_debug("%s: name: %s parent: %s [0x%p]\n", __func__, name,
parent->name, parent);
--
2.34.1
