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

Heinrich Schuchardt
2022-03-20T07:18:10Z

On 3/18/22 04:54, Samuel Holland 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.
>
> 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);

Don't repeat yourself. Please, pull these 4 lines out of the if-else
construct.

Best regards

Heinrich

> + }
> +
> return sun8i_rsb_init(base);
> }
> #endif