Re: [U-Boot] [PATCH v2 04/10] pinctrl: rockchip: Special treatment for RK3288 gpio0 pins' iomux

2019-04-04 Thread Heiko Stuebner
Am Donnerstag, 4. April 2019, 10:16:02 CEST schrieb David Wu:
> Hi Philipp,
> 
> 在 2019/4/4 下午3:19, Philipp Tomsich 写道:
> > 
> > 
> >> On 04.04.2019, at 05:51, David Wu  wrote:
> >>
> >> RK3288 pmu_gpio0 iomux setting have no higher 16 writing corresponding
> >> bits, need to read before write the register.
> >>
> >> Signed-off-by: David Wu 
> >> ---
> >>
> >> drivers/pinctrl/rockchip/pinctrl-rk3288.c | 8 +++-
> >> 1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/pinctrl/rockchip/pinctrl-rk3288.c 
> >> b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
> >> index 1fa601d954..d66ffdf24b 100644
> >> --- a/drivers/pinctrl/rockchip/pinctrl-rk3288.c
> >> +++ b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
> >> @@ -54,7 +54,13 @@ static int rk3288_set_mux(struct rockchip_pin_bank 
> >> *bank, int pin, int mux)
> >>}
> >>}
> >>
> >> -  data = (mask << (bit + 16));
> >> +  if (bank->bank_num == 0) {
> >> +  regmap_read(regmap, reg, );
> > 
> > Could you pull the regmap_read out of the if and make it common for all 
> > cases, so the differences between the paths are in data-manipulation only?
> 
> Yes, the difference between the gpio0 and other pins is the 
> data-manipulation, and i think the others don't need the regmap_read,
> so it is not a common case.

yep ... the other pinmuxes are using hiword-mask registers
while only gpio0 (in the pmu-area) needs the to get the
read-modify-write scheme.




___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v2 04/10] pinctrl: rockchip: Special treatment for RK3288 gpio0 pins' iomux

2019-04-04 Thread David Wu

Hi Philipp,

在 2019/4/4 下午3:19, Philipp Tomsich 写道:




On 04.04.2019, at 05:51, David Wu  wrote:

RK3288 pmu_gpio0 iomux setting have no higher 16 writing corresponding
bits, need to read before write the register.

Signed-off-by: David Wu 
---

drivers/pinctrl/rockchip/pinctrl-rk3288.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/rockchip/pinctrl-rk3288.c 
b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
index 1fa601d954..d66ffdf24b 100644
--- a/drivers/pinctrl/rockchip/pinctrl-rk3288.c
+++ b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
@@ -54,7 +54,13 @@ static int rk3288_set_mux(struct rockchip_pin_bank *bank, 
int pin, int mux)
}
}

-   data = (mask << (bit + 16));
+   if (bank->bank_num == 0) {
+   regmap_read(regmap, reg, );


Could you pull the regmap_read out of the if and make it common for all cases, 
so the differences between the paths are in data-manipulation only?


Yes, the difference between the gpio0 and other pins is the 
data-manipulation, and i think the others don't need the regmap_read,

so it is not a common case.




+   data &= ~(mask << bit);
+   } else {
+   data = (mask << (bit + 16));
+   }
+


Please add a comment, so readers will be able to understand what is happening 
(and why) without referring to the TRM


data |= (mux & mask) << bit;
ret = regmap_write(regmap, reg, data);

--
2.19.1










___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v2 04/10] pinctrl: rockchip: Special treatment for RK3288 gpio0 pins' iomux

2019-04-04 Thread Philipp Tomsich


> On 04.04.2019, at 05:51, David Wu  wrote:
> 
> RK3288 pmu_gpio0 iomux setting have no higher 16 writing corresponding
> bits, need to read before write the register.
> 
> Signed-off-by: David Wu 
> ---
> 
> drivers/pinctrl/rockchip/pinctrl-rk3288.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/rockchip/pinctrl-rk3288.c 
> b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
> index 1fa601d954..d66ffdf24b 100644
> --- a/drivers/pinctrl/rockchip/pinctrl-rk3288.c
> +++ b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
> @@ -54,7 +54,13 @@ static int rk3288_set_mux(struct rockchip_pin_bank *bank, 
> int pin, int mux)
>   }
>   }
> 
> - data = (mask << (bit + 16));
> + if (bank->bank_num == 0) {
> + regmap_read(regmap, reg, );

Could you pull the regmap_read out of the if and make it common for all cases, 
so the differences between the paths are in data-manipulation only?

> + data &= ~(mask << bit);
> + } else {
> + data = (mask << (bit + 16));
> + }
> +

Please add a comment, so readers will be able to understand what is happening 
(and why) without referring to the TRM.

>   data |= (mux & mask) << bit;
>   ret = regmap_write(regmap, reg, data);
> 
> -- 
> 2.19.1
> 
> 
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH v2 04/10] pinctrl: rockchip: Special treatment for RK3288 gpio0 pins' iomux

2019-04-03 Thread David Wu
RK3288 pmu_gpio0 iomux setting have no higher 16 writing corresponding
bits, need to read before write the register.

Signed-off-by: David Wu 
---

 drivers/pinctrl/rockchip/pinctrl-rk3288.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/rockchip/pinctrl-rk3288.c 
b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
index 1fa601d954..d66ffdf24b 100644
--- a/drivers/pinctrl/rockchip/pinctrl-rk3288.c
+++ b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
@@ -54,7 +54,13 @@ static int rk3288_set_mux(struct rockchip_pin_bank *bank, 
int pin, int mux)
}
}
 
-   data = (mask << (bit + 16));
+   if (bank->bank_num == 0) {
+   regmap_read(regmap, reg, );
+   data &= ~(mask << bit);
+   } else {
+   data = (mask << (bit + 16));
+   }
+
data |= (mux & mask) << bit;
ret = regmap_write(regmap, reg, data);
 
-- 
2.19.1



___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot