Re: [PATCH 15/18] state: factor device path lookup into helper function

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 31.05.23 19:54, Marco Felsch wrote:
> Hi Ahmad,
> 
> On 23-05-31, Ahmad Fatoum wrote:
>> The #ifdef __BAREBOX__ is meant for easier synchronization with
>> dt-utils. We'll keep that intact, but move it out of the function to not
>> break reading flow. After sync, dt-utils would now need to implement
>>
>>   of_cdev_find
>>   cdev_to_devpath
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  common/state/state.c | 30 +++++++++++++++++++++++-------
>>  1 file changed, 23 insertions(+), 7 deletions(-)
>>
>> diff --git a/common/state/state.c b/common/state/state.c
>> index 11cc86ff73be..88e246198fb8 100644
>> --- a/common/state/state.c
>> +++ b/common/state/state.c
>> @@ -581,6 +581,20 @@ void state_release(struct state *state)
>>  	free(state);
>>  }
>>  
>> +#ifdef __BAREBOX__
>> +static char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size)
>> +{
>> +	/*
>> +	 * We only accept partitions exactly mapping the barebox-state,
>> +	 * but dt-utils may need to set non-zero values here
>> +	 */
>> +	*offset = 0;
>> +	*size = 0;
>> +
>> +	return basprintf("/dev/%s", cdev->name);
>> +}
>> +#endif
> 
> We could get rid of the #ifdef if we move this function to some barebox
> internal code not shared with dt-utils.

Setting offset and size to zero makes no sense elsewhere, that's why I left it here.

> Regards,
>   Marco
> 
>> +
>>  /*
>>   * state_new_from_node - create a new state instance from a device_node
>>   *
>> @@ -597,8 +611,9 @@ struct state *state_new_from_node(struct device_node *node, bool readonly)
>>  	const char *alias;
>>  	uint32_t stridesize;
>>  	struct device_node *partition_node;
>> -	off_t offset = 0;
>> -	size_t size = 0;
>> +	struct cdev *cdev;
>> +	off_t offset;
>> +	size_t size;
>>  
>>  	alias = of_alias_get(node);
>>  	if (!alias) {
>> @@ -617,11 +632,8 @@ struct state *state_new_from_node(struct device_node *node, bool readonly)
>>  		goto out_release_state;
>>  	}
>>  
>> -#ifdef __BAREBOX__
>> -	ret = of_find_path_by_node(partition_node, &state->backend_path, 0);
>> -#else
>> -	ret = of_get_devicepath(partition_node, &state->backend_path, &offset, &size);
>> -#endif
>> +	cdev = of_cdev_find(partition_node);
>> +	ret = PTR_ERR_OR_ZERO(cdev);
>>  	if (ret) {
>>  		if (ret != -EPROBE_DEFER)
>>  			dev_err(&state->dev, "state failed to parse path to backend: %s\n",
>> @@ -629,6 +641,10 @@ struct state *state_new_from_node(struct device_node *node, bool readonly)
>>  		goto out_release_state;
>>  	}
>>  
>> +	state->backend_path = cdev_to_devpath(cdev, &offset, &size);
>> +
>> +	pr_debug("%s: backend resolved to %s\n", node->full_name, state->backend_path);
>> +
>>  	state->backend_reproducible_name = of_get_reproducible_name(partition_node);
>>  
>>  	ret = of_property_read_string(node, "backend-type", &backend_type);
>> -- 
>> 2.39.2
>>
>>
>>
> 

-- 
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 |