[PATCH] pinctrl/rockchip: re-fix RK3308 pinmux bits

[jay.xu at rock-chips.com]

Hi Luca:

BR
--------------
jay.xu at rock-chips.com
>Hello Jianqun Xu,
>
>On Fri, 30 Sep 2022 18:26:20 +0800
>Jianqun Xu <jay.xu at rock-chips.com> wrote:
>
>> Part of pins from RK3308 SoCs have two registers to do pinmux, one is
>> the origin register with 2bits named by gpioxx_sel, and another with
>> 3bits and named by gpioxx_sel_plus.
>
>Are the "plus" registers documented anywhere? 

At RK3308 TRM CH5 GRF, 

GRF_SOC_CON13 

3  RW  0x0 gpio2a2_sel_src_ctrl
                  IOMUX control source selection.
                 1'b0: use basic GPIO2A_IOMUX[gpio2a2_sel]
                 1'b1: use gpio2a2_sel_plus instead of GPIO2A_IOMUX[gpio2a2_sel]

2:0  RW  0x0 gpio2a2_sel_plus
                     3'b000: GPIO2_A2
                     3'b001: UART0_CTSN
                     3'b010: SPI0_CLK
                     3'b011: I2C2_SDA
                     3'b100: Reserved
                     3'b101: OWIRE_M2

Sorry I don't know about the trm you got from our company, if you cannot find this part,
may require a new version ?

>
>The reference manual I have does not mention them.
>
>> The default value is 2bits. But Rockchip downstream pinctrl driver has a
>> soc init for RK3308 to switch to the 3bits path. The first patch
>> upstream the support for RK3308 pinctrl but drop the soc init codes.
>>
>> The commit 1f3e25a06883 ("pinctrl: rockchip: fix RK3308 pinmux bits") try
>> to fix back to 2 bits path, but that will makes some iomux not be
>> supported.
>
>Sorry about that, of course I could not know because my documentation
>does not mention those registers. 

Never be mind, that is my mistask when I upstream the support fo rk3308.
The upstream version will lack of function 4-7, but it works well for the function 0-3

Problem happens when I backport these patches to our driver and the uart work bad since our
driver will set it to 3bits path.

Anyway thanks for your patch and could help to do a verify and review for this patch  ?

>
>> @@ -3014,6 +3014,50 @@ static int __maybe_unused rockchip_pinctrl_resume(struct device *dev)
>>  static SIMPLE_DEV_PM_OPS(rockchip_pinctrl_dev_pm_ops, rockchip_pinctrl_suspend,
>>  rockchip_pinctrl_resume);
>> 
>> +
>> +static int rk3308_soc_data_init(struct rockchip_pinctrl *info)
>> +{
>> +	int ret;
>> +
>> +	#define RK3308_GRF_SOC_CON13	(0x608)
>> +	#define RK3308_GRF_SOC_CON15	(0x610)
>> +
>> +	/* RK3308_GRF_SOC_CON13 */
>> +	#define RK3308_GRF_I2C3_IOFUNC_SRC_CTRL	(BIT(16 + 10) | BIT(10))
>> +	#define RK3308_GRF_GPIO2A3_SEL_SRC_CTRL	(BIT(16 + 7)  | BIT(7))
>> +	#define RK3308_GRF_GPIO2A2_SEL_SRC_CTRL	(BIT(16 + 3)  | BIT(3))
>> +
>> +	/* RK3308_GRF_SOC_CON15 */
>> +	#define RK3308_GRF_GPIO2C0_SEL_SRC_CTRL	(BIT(16 + 11) | BIT(11))
>> +	#define RK3308_GRF_GPIO3B3_SEL_SRC_CTRL	(BIT(16 + 7)  | BIT(7))
>> +	#define RK3308_GRF_GPIO3B2_SEL_SRC_CTRL	(BIT(16 + 3)  | BIT(3))
>> +
>> +	/*
>> +	* Enable the special ctrl of selected sources.
>> +	*
>> +	* Example reference to GRF_SOC_CON13 description:
>> +	*
>> +	* gpio2a2_sel_src_ctrl
>> +	* IOMUX control source selection.
>> +	* 1'b0: use basic GPIO2A_IOMUX[gpio2a2_sel]
>> +	* 1'b1: use gpio2a2_sel_plus instead of GPIO2A_IOMUX[gpio2a2_sel]
>> +	*/
>> +
>> +	ret = regmap_write(info->regmap_base, RK3308_GRF_SOC_CON13,
>> +	   RK3308_GRF_I2C3_IOFUNC_SRC_CTRL |
>> +	   RK3308_GRF_GPIO2A3_SEL_SRC_CTRL |
>> +	   RK3308_GRF_GPIO2A2_SEL_SRC_CTRL);
>> +	if (ret)
>> +	return ret;
>> +
>> +	ret = regmap_write(info->regmap_base, RK3308_GRF_SOC_CON15,
>> +	   RK3308_GRF_GPIO2C0_SEL_SRC_CTRL |
>> +	   RK3308_GRF_GPIO3B3_SEL_SRC_CTRL |
>> +	   RK3308_GRF_GPIO3B2_SEL_SRC_CTRL);
>> +
>> +	return ret;
>> +}
>
>This is new code, not code that my patch has removed. Is it needed? How
>did the driver work before without this code?
> 
I set these codes before pinctrl to register. the driver could work with 2bit or 3bit,
but a problem is that if the uboot change it to 3bit, but the kernel not to do any set
then it works bad, that what happend when I backport it.

>I think we need to clarify those registers before applying a fix that
>might create other problems. 

Reference uppper note, part of trm for gpio2_a2

>
>Best regards.
>--
>Luca Ceresoli, Bootlin
>Embedded Linux and Kernel engineering
>https://bootlin.com