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

[Dario Binacchi]

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.
IMHO you can submit the patch. Why not, add a test too.

Thanks and regards,
Dario

> 
> Jan
> 
> > +	if (addr == FDT_ADDR_T_NONE) {
> > +		dev_err(dev, "failed to get base register size\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	pdata->offset = size - pdata->width / BITS_PER_BYTE;
> >
> >  	addr = dev_read_addr(dev);
> >  	if (addr == FDT_ADDR_T_NONE) {
> >