[bug report] i3c: master: Add basic driver for the Renesas I3C controller

[bug report] i3c: master: Add basic driver for the Renesas I3C controller

[Dan Carpenter]

Hello Wolfram Sang,

Commit d028219a9f14 ("i3c: master: Add basic driver for the Renesas
I3C controller") from Jul 24, 2025 (linux-next), leads to the
following (unpublished) Smatch static checker warning:

	drivers/i3c/master/renesas-i3c.c:682 renesas_i3c_daa()
	warn: duplicate check 'ret < 0' (previous on line 660)

drivers/i3c/master/renesas-i3c.c
    622 static int renesas_i3c_daa(struct i3c_master_controller *m)
    623 {
    624         struct renesas_i3c *i3c = to_renesas_i3c(m);
    625         struct renesas_i3c_cmd *cmd;
    626         u32 olddevs, newdevs;
    627         u8 last_addr = 0, pos;
    628         int ret;
    629 
    630         struct renesas_i3c_xfer *xfer __free(kfree) = renesas_i3c_alloc_xfer(i3c, 1);
    631         if (!xfer)
    632                 return -ENOMEM;
    633 
    634         /* Enable I3C bus. */
    635         renesas_i3c_bus_enable(m, true);
    636 
    637         olddevs = ~(i3c->free_pos);
    638         i3c->internal_state = I3C_INTERNAL_STATE_CONTROLLER_ENTDAA;
    639 
    640         /* Setting DATBASn registers for target devices. */
    641         for (pos = 0; pos < i3c->maxdevs; pos++) {
    642                 if (olddevs & BIT(pos))
    643                         continue;
    644 
    645                 ret = i3c_master_get_free_addr(m, last_addr + 1);
    646                 if (ret < 0)
    647                         return -ENOSPC;
    648 
    649                 i3c->addrs[pos] = ret;
    650                 last_addr = ret;
    651 
    652                 renesas_writel(i3c->regs, DATBAS(pos), datbas_dvdyad_with_parity(ret));
    653         }
    654 
    655         init_completion(&xfer->comp);
    656         cmd = xfer->cmds;
    657         cmd->rx_count = 0;
    658 
    659         ret = renesas_i3c_get_free_pos(i3c);
    660         if (ret < 0)
    661                 return ret;
    662 
    663         /*
    664          * Setup the command descriptor to start the ENTDAA command
    665          * and starting at the selected device index.
    666          */
    667         cmd->cmd0 = NCMDQP_CMD_ATTR(NCMDQP_ADDR_ASSGN) | NCMDQP_ROC |
    668                     NCMDQP_TID(I3C_COMMAND_ADDRESS_ASSIGNMENT) |
    669                     NCMDQP_CMD(I3C_CCC_ENTDAA) | NCMDQP_DEV_INDEX(ret) |
    670                     NCMDQP_DEV_COUNT(i3c->maxdevs - ret) | NCMDQP_TOC;
    671 
    672         renesas_i3c_wait_xfer(i3c, xfer);
    673 
    674         newdevs = GENMASK(i3c->maxdevs - cmd->rx_count - 1, 0);
    675         newdevs &= ~olddevs;
    676 
    677         for (pos = 0; pos < i3c->maxdevs; pos++) {
    678                 if (newdevs & BIT(pos))
    679                         i3c_master_add_i3c_dev_locked(m, i3c->addrs[pos]);

Should we add checking for i3c_master_add_i3c_dev_locked()?

    680         }
    681 
--> 682         return ret < 0 ? ret : 0;

Otherwise this could just be "return 0;"

    683 }

regards,
dan carpenter

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [bug report] i3c: master: Add basic driver for the Renesas I3C controller

[Wolfram Sang]

Hi Dan,

thanks for the report. I'd argue it is not a bug, though.

>     677         for (pos = 0; pos < i3c->maxdevs; pos++) {
>     678                 if (newdevs & BIT(pos))
>     679                         i3c_master_add_i3c_dev_locked(m, i3c->addrs[pos]);
> 
> Should we add checking for i3c_master_add_i3c_dev_locked()?

No, I think we should continue to add the remaining devices even if one
fails. Other driver do it also like above.

>     680         }
>     681 
> --> 682         return ret < 0 ? ret : 0;
> 
> Otherwise this could just be "return 0;"

True that, I will send a patch.

Happy hacking,

   Wolfram


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c