On Thu, Sep 10, 2026 at 3:00 PM Javier Martinez Canillas <[email protected]> wrote: > > static int ssd130x_write_cmds(struct ssd130x_device *ssd130x, const u8 > > *cmd, > > size_t len) > > @@ -271,6 +310,17 @@ static int ssd130x_write_cmds(struct ssd130x_device > > *ssd130x, const u8 *cmd, > > unsigned int i; > > int ret; > > > > + if (ssd130x->device_info->cmd_params_are_data) { > > + if (!len) > > + return 0; > > + > > > Can len even be 0? If that's the case then I guess that makes more sense > to check and bail out early regardless if cmd_params_are_data is true ? > > For the !cmd_params_are_data case, the for loop will be a no-op anyways > but still I think is cleaner to check as the first thing in this function.
I leaned on the no-op loop in the second branch, but I agree it would be cleaner to check this and remove the edge case regardless if parameters are data or not. Addressed in v6. > > + ret = regmap_write(ssd130x->regmap, SSD13XX_COMMAND, cmd[0]); > > + if (ret || len == 1) > > + return ret; > > + > > The len == 1 case is for commands that do not have parameters right? I > think that adding some comments explaining this to make it clear why > there is an early return. Added a comment in v6. > I'm happy with the implementation now, thanks a lot for bearing with > me and your patience iterating over this series. > > Reviewed-by: Javier Martinez Canillas <[email protected]> No problem at all, happy to work with you. Thank you for the review. -- Thanks, Amit
