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

From: Andre Przywara <andre.przywara@arm.com>
To: Samuel Holland <samuel@sholland.org>
Cc: u-boot@lists.denx.de, Jagan Teki <jagan@amarulasolutions.com>,
	Sean Anderson <seanga2@gmail.com>, Simon Glass <sjg@chromium.org>,
	Heinrich Schuchardt <heinrich.schuchardt@canonical.com>,
	Heiko Schocher <hs@denx.de>,
	Joe Hershberger <joe.hershberger@ni.com>
Subject: Re: [PATCH v2 17/23] i2c: sun8i_rsb: Only do non-DM pin setup for non-DM I2C
Date: Fri, 1 Apr 2022 00:20:51 +0100	[thread overview]
Message-ID: <20220401002051.53792af3@slackpad.lan> (raw)
In-Reply-To: <20220318035420.15058-18-samuel@sholland.org>

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?

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