[PATCH v4 05/12] pinctrl: single: get register area size by device API

[Jan Kiszka]

On 01.05.21 17:39, Dario Binacchi wrote:
> Hi Jan,
>
>> Il 01/05/2021 14:29 Jan Kiszka <jan.kiszka at web.de> ha scritto:
>>
>>
>> On 11.04.21 09:39, Dario Binacchi wrote:
>>> Use dev_read_addr_size to get size of the controller's register area.
>>>
>>> Signed-off-by: Dario Binacchi <dariobin at libero.it>
>>> Reviewed-by: Pratyush Yadav <p.yadav at ti.com>
>>>
>>> ---
>>>
>>> (no changes since v3)
>>>
>>> Changes in v3:
>>> - Added Pratyush Yadav review tag.
>>>
>>> Changes in v2:
>>> - Check dev_read_addr_size return value.
>>>
>>>  drivers/pinctrl/pinctrl-single.c | 14 ++++++++------
>>>  1 file changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
>>> index cec00e289c..d5656de8e8 100644
>>> --- a/drivers/pinctrl/pinctrl-single.c
>>> +++ b/drivers/pinctrl/pinctrl-single.c
>>> @@ -182,17 +182,19 @@ static int single_set_state(struct udevice *dev,
>>>  static int single_of_to_plat(struct udevice *dev)
>>>  {
>>>  	fdt_addr_t addr;
>>> -	u32 of_reg[2];
>>> -	int res;
>>> +	fdt_size_t size;
>>>  	struct single_pdata *pdata = dev_get_plat(dev);
>>>
>>>  	pdata->width =
>>>  		dev_read_u32_default(dev, "pinctrl-single,register-width", 0);
>>>
>>> -	res = dev_read_u32_array(dev, "reg", of_reg, 2);
>>> -	if (res)
>>> -		return res;
>>> -	pdata->offset = of_reg[1] - pdata->width / 8;
>>> +	addr = dev_read_addr_size(dev, "reg", &size);
>>
>> Looks to me as if this has to be
>>
>> 	addr = dev_read_addr_size_index(dev, 0, &size);
>>
>> because dev_read_addr_size() assumes address and size width of
>> fdt_addr_t, ie. 2 cells, rather than what is actually configured in the DT.
>>
>> I don't understand the use cases of dev_read_addr_size() vs.
>> dev_read_addr_size_index(), if the former should also respect what the
>> parent node configured size-wise. However, this patch breaks e.g.
>> k3-am65 DTs, namely pinctrl at 4301c000 which uses only one cell for
>> address and size (according to bus at 42040000).
>
> If dev_read_addr_size() breaks k3-am65 DTs, the use of dev_read_addr_size_index()
> doesn't break am335x DTs. Sandbox and beaglebone tests are ok.

fdt_addr_t is 32 bits on the am335x, it's 64 on the am65xx while the
address/size widths used in that branch is only 32.

> IMHO you can submit the patch. Why not, add a test too.

I will - I'm just not yet sure if I should patch the function above or
rather ofnode_get_addr_size.

Jan