[PATCH] common: boards: wolfvision: use state_by_alias instead of opencoding

[PATCH] common: boards: wolfvision: use state_by_alias instead of opencoding

[Ahmad Fatoum]

This introduces no functional change, but makes code a bit more compact.

Cc: Michael Riesch <michael.riesch@wolfvision.net>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/boards/wolfvision/common.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/common/boards/wolfvision/common.c b/common/boards/wolfvision/common.c
index f483918cecfe..5484a8ac6b06 100644
--- a/common/boards/wolfvision/common.c
+++ b/common/boards/wolfvision/common.c
@@ -62,11 +62,7 @@ int wolfvision_register_ethaddr(void)
 	char mac[ETH_ALEN];
 	int ret;
 
-	ret = of_device_ensure_probed_by_alias("state");
-	if (ret)
-		return ret;
-
-	state = state_by_name("state");
+	state = state_by_alias("state");
 	if (!state)
 		return -ENOENT;
 
-- 
2.39.2

Re: [PATCH] common: boards: wolfvision: use state_by_alias instead of opencoding

[Michael Riesch]

Hi Ahmad,

Thanks a lot for your patch!

On 5/15/24 08:07, Ahmad Fatoum wrote:
> This introduces no functional change, but makes code a bit more compact.
> 
> Cc: Michael Riesch <michael.riesch@wolfvision.net>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  common/boards/wolfvision/common.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/common/boards/wolfvision/common.c b/common/boards/wolfvision/common.c
> index f483918cecfe..5484a8ac6b06 100644
> --- a/common/boards/wolfvision/common.c
> +++ b/common/boards/wolfvision/common.c
> @@ -62,11 +62,7 @@ int wolfvision_register_ethaddr(void)
>  	char mac[ETH_ALEN];
>  	int ret;
>  
> -	ret = of_device_ensure_probed_by_alias("state");

Just to be on the safe side: of_device_ensure_probed_by_alias makes sure
that the underlying drivers are probed, right?

> -	if (ret)
> -		return ret;
> -
> -	state = state_by_name("state");
> +	state = state_by_alias("state");

state_by_alias, on the other hand, calls only of_find_node_by_alias,
which (as I presume) does not ensure that.

IIRC the of_device_ensure_... magic was necessary in our setup, but I
can give your patch a test during the next round of barebox board code
cleanups.

Best regards,
Michael

>  	if (!state)
>  		return -ENOENT;
>  

Re: [PATCH] common: boards: wolfvision: use state_by_alias instead of opencoding

[Ahmad Fatoum]

Hello Michael,

On 15.05.24 08:29, Michael Riesch wrote:
> Hi Ahmad,
> 
> Thanks a lot for your patch!
> 
> On 5/15/24 08:07, Ahmad Fatoum wrote:
>> This introduces no functional change, but makes code a bit more compact.
>>
>> Cc: Michael Riesch <michael.riesch@wolfvision.net>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  common/boards/wolfvision/common.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/common/boards/wolfvision/common.c b/common/boards/wolfvision/common.c
>> index f483918cecfe..5484a8ac6b06 100644
>> --- a/common/boards/wolfvision/common.c
>> +++ b/common/boards/wolfvision/common.c
>> @@ -62,11 +62,7 @@ int wolfvision_register_ethaddr(void)
>>  	char mac[ETH_ALEN];
>>  	int ret;
>>  
>> -	ret = of_device_ensure_probed_by_alias("state");
> 
> Just to be on the safe side: of_device_ensure_probed_by_alias makes sure
> that the underlying drivers are probed, right?

Yes.

> 
>> -	if (ret)
>> -		return ret;
>> -
>> -	state = state_by_name("state");
>> +	state = state_by_alias("state");
> 
> state_by_alias, on the other hand, calls only of_find_node_by_alias,
> which (as I presume) does not ensure that.

Yes, but afterwards it calls state_by_node(), which calls of_device_ensure_probed().

> IIRC the of_device_ensure_... magic was necessary in our setup, but I
> can give your patch a test during the next round of barebox board code
> cleanups.

Yes, I ran into these problems before too on a deep probe system, which is
why state_by_alias was added.

Cheers,
Ahmad

> 
> Best regards,
> Michael
> 
>>  	if (!state)
>>  		return -ENOENT;
>>  
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] common: boards: wolfvision: use state_by_alias instead of opencoding

[Michael Riesch]

Hi Ahmad,

On 5/15/24 08:35, Ahmad Fatoum wrote:
> Hello Michael,
> 
> On 15.05.24 08:29, Michael Riesch wrote:
>> Hi Ahmad,
>>
>> Thanks a lot for your patch!
>>
>> On 5/15/24 08:07, Ahmad Fatoum wrote:
>>> This introduces no functional change, but makes code a bit more compact.
>>>
>>> Cc: Michael Riesch <michael.riesch@wolfvision.net>
>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>> ---
>>>  common/boards/wolfvision/common.c | 6 +-----
>>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/common/boards/wolfvision/common.c b/common/boards/wolfvision/common.c
>>> index f483918cecfe..5484a8ac6b06 100644
>>> --- a/common/boards/wolfvision/common.c
>>> +++ b/common/boards/wolfvision/common.c
>>> @@ -62,11 +62,7 @@ int wolfvision_register_ethaddr(void)
>>>  	char mac[ETH_ALEN];
>>>  	int ret;
>>>  
>>> -	ret = of_device_ensure_probed_by_alias("state");
>>
>> Just to be on the safe side: of_device_ensure_probed_by_alias makes sure
>> that the underlying drivers are probed, right?
> 
> Yes.
> 
>>
>>> -	if (ret)
>>> -		return ret;
>>> -
>>> -	state = state_by_name("state");
>>> +	state = state_by_alias("state");
>>
>> state_by_alias, on the other hand, calls only of_find_node_by_alias,
>> which (as I presume) does not ensure that.
> 
> Yes, but afterwards it calls state_by_node(), which calls of_device_ensure_probed().

Ah, nice!

>> IIRC the of_device_ensure_... magic was necessary in our setup, but I
>> can give your patch a test during the next round of barebox board code
>> cleanups.
> 
> Yes, I ran into these problems before too on a deep probe system, which is
> why state_by_alias was added.

OK, this sounds good.

Reviewed-by: Michael Riesch <michael.riesch@wolfvision.net>

Thanks and regards,
Michael

> 
> Cheers,
> Ahmad
> 
>>
>> Best regards,
>> Michael
>>
>>>  	if (!state)
>>>  		return -ENOENT;
>>>  
>>
> 

Re: [PATCH] common: boards: wolfvision: use state_by_alias instead of opencoding

[Sascha Hauer]

On Wed, 15 May 2024 08:07:36 +0200, Ahmad Fatoum wrote:
> This introduces no functional change, but makes code a bit more compact.
> 
> 

Applied, thanks!

[1/1] common: boards: wolfvision: use state_by_alias instead of opencoding
      https://git.pengutronix.de/cgit/barebox/commit/?id=6b1df8b8219d (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>