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

[Heinrich Schuchardt]

On 3/20/22 08:17, Heinrich Schuchardt wrote:
> 
> 
> 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 at 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.

Sorry missied N vs L.

> 
> Best regards
> 
> Heinrich
> 
>> +    }
>> +
>>       return sun8i_rsb_init(base);
>>   }
>>   #endif