Re: [PATCH v2 17/23] i2c: sun8i_rsb: Only do non-DM pin setup for non-DM I2C

[Samuel Holland <samuel@sholland.org>]

On 3/31/22 6:20 PM, Andre Przywara wrote:
> On Thu, 17 Mar 2022 22:54:14 -0500
> Samuel Holland <samuel@sholland.org> wrote:
> 
>> When the DM_I2C driver is loaded, the pin setup is done automatically
>> from the device tree by the pinctrl driver.
>>
>> Clean up the code in the process: remove #ifdefs and recognize that the
>> pin configuration is the same for all sun8i/sun50i SoCs, not just those
>> which select CONFIG_MACH_SUN8I.
> 
> Indeed, even though the F1C100s uses mux 4, but we don't use the RSB
> there, and can fix that when we need it.
> 
> So this means we could enable RSB for the H616 SPL?

Not in SPL, because the DM clock driver is not yet available there, and
prcm_apb0_enable() does not support the H6 and newer CCU gate/reset bits layout.
It should work for H616 in U-Boot proper.

Regards,
Samuel

> Regardless this looks alright:
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> 
> Cheers,
> Andre
> 
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>>
>> (no changes since v1)
>>
>>  drivers/i2c/sun8i_rsb.c | 46 +++++++++++++++++------------------------
>>  1 file changed, 19 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/i2c/sun8i_rsb.c b/drivers/i2c/sun8i_rsb.c
>> index 716b245a00..0dea8f7a92 100644
>> --- a/drivers/i2c/sun8i_rsb.c
>> +++ b/drivers/i2c/sun8i_rsb.c
>> @@ -95,27 +95,6 @@ static int sun8i_rsb_set_device_address(struct sunxi_rsb_reg *base,
>>  	return sun8i_rsb_do_trans(base);
>>  }
>>  
>> -static void sun8i_rsb_cfg_io(void)
>> -{
>> -#ifdef CONFIG_MACH_SUN8I
>> -	sunxi_gpio_set_cfgpin(SUNXI_GPL(0), SUN8I_GPL_R_RSB);
>> -	sunxi_gpio_set_cfgpin(SUNXI_GPL(1), SUN8I_GPL_R_RSB);
>> -	sunxi_gpio_set_pull(SUNXI_GPL(0), 1);
>> -	sunxi_gpio_set_pull(SUNXI_GPL(1), 1);
>> -	sunxi_gpio_set_drv(SUNXI_GPL(0), 2);
>> -	sunxi_gpio_set_drv(SUNXI_GPL(1), 2);
>> -#elif defined CONFIG_MACH_SUN9I
>> -	sunxi_gpio_set_cfgpin(SUNXI_GPN(0), SUN9I_GPN_R_RSB);
>> -	sunxi_gpio_set_cfgpin(SUNXI_GPN(1), SUN9I_GPN_R_RSB);
>> -	sunxi_gpio_set_pull(SUNXI_GPN(0), 1);
>> -	sunxi_gpio_set_pull(SUNXI_GPN(1), 1);
>> -	sunxi_gpio_set_drv(SUNXI_GPN(0), 2);
>> -	sunxi_gpio_set_drv(SUNXI_GPN(1), 2);
>> -#else
>> -#error unsupported MACH_SUNXI
>> -#endif
>> -}
>> -
>>  static void sun8i_rsb_set_clk(struct sunxi_rsb_reg *base)
>>  {
>>  	u32 div = 0;
>> @@ -147,12 +126,6 @@ static int sun8i_rsb_set_device_mode(struct sunxi_rsb_reg *base)
>>  
>>  static int sun8i_rsb_init(struct sunxi_rsb_reg *base)
>>  {
>> -	/* Enable RSB and PIO clk, and de-assert their resets */
>> -	prcm_apb0_enable(PRCM_APB0_GATE_PIO | PRCM_APB0_GATE_RSB);
>> -
>> -	/* Setup external pins */
>> -	sun8i_rsb_cfg_io();
>> -
>>  	writel(RSB_CTRL_SOFT_RST, &base->ctrl);
>>  	sun8i_rsb_set_clk(base);
>>  
>> @@ -185,6 +158,25 @@ int rsb_init(void)
>>  {
>>  	struct sunxi_rsb_reg *base = (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
>>  
>> +	/* Enable RSB and PIO clk, and de-assert their resets */
>> +	prcm_apb0_enable(PRCM_APB0_GATE_PIO | PRCM_APB0_GATE_RSB);
>> +
>> +	if (IS_ENABLED(CONFIG_MACH_SUN9I)) {
>> +		sunxi_gpio_set_cfgpin(SUNXI_GPN(0), SUN9I_GPN_R_RSB);
>> +		sunxi_gpio_set_cfgpin(SUNXI_GPN(1), SUN9I_GPN_R_RSB);
>> +		sunxi_gpio_set_pull(SUNXI_GPN(0), 1);
>> +		sunxi_gpio_set_pull(SUNXI_GPN(1), 1);
>> +		sunxi_gpio_set_drv(SUNXI_GPN(0), 2);
>> +		sunxi_gpio_set_drv(SUNXI_GPN(1), 2);
>> +	} else {
>> +		sunxi_gpio_set_cfgpin(SUNXI_GPL(0), SUN8I_GPL_R_RSB);
>> +		sunxi_gpio_set_cfgpin(SUNXI_GPL(1), SUN8I_GPL_R_RSB);
>> +		sunxi_gpio_set_pull(SUNXI_GPL(0), 1);
>> +		sunxi_gpio_set_pull(SUNXI_GPL(1), 1);
>> +		sunxi_gpio_set_drv(SUNXI_GPL(0), 2);
>> +		sunxi_gpio_set_drv(SUNXI_GPL(1), 2);
>> +	}
>> +
>>  	return sun8i_rsb_init(base);
>>  }
>>  #endif
>