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

Samuel Holland
2022-04-01T00:10:25Z

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
>