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

[David Wu]

Hi Philipp,

在 2019/4/4 下午3:19, Philipp Tomsich 写道:
> 
> 
>> On 04.04.2019, at 05:51, David Wu <david.wu at rock-chips.com> 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 <david.wu at rock-chips.com>
>> ---
>>
>> 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);
> 
> 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
>>
>>
>>
> 
> 
>