Re: [PATCH v2 3/7] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs

["Marek Behún" <kabel@kernel.org>]

On Tue, 19 Sep 2023 16:00:39 +0300
Andy Shevchenko <andy@kernel.org> wrote:

> > +	mutex_lock(&mcu->lock);
> > +
> > +	if (ctl_mask)
> > +		err = omnia_ctl_cmd_unlocked(mcu, CMD_GENERAL_CONTROL, ctl,
> > +					     ctl_mask);  
> 
> > +	if (!err && ext_ctl_mask)
> > +		err = omnia_ctl_cmd_unlocked(mcu, CMD_EXT_CONTROL, ext_ctl,
> > +					     ext_ctl_mask);  
> 
> Can it be
> 
> 	if (err)
> 		goto out_unlock;
> 
> 	if (_mask)
> 		...
> 
> ?

Hi Andy,

so I am refactoring this to use guard(mutex), but now I have this:

	guard(mutex, &mcu->lock);

	if (ctl_mask) {
		err = ...;
		if (err)
			goto out_err;
	}

	if (ext_ctl_mask) {
		err = ...;
		if (err)
			goto out_err;
	}

	return;
out_err:
	dev_err(dev, "Cannot set GPIOs: %d\n", err);



which clearly is not any better... or at least the original
  if (!err && ext_ctl_mask)
is better IMO.

Compare with:

	guard(mutex, &mcu->lock);

	if (ctl_mask)
		err = ...;

	if (!err && ext_ctl_mask)
		err = ...;

	if (err)
		dev_err(dev, "Cannot set GPIOs: %d\n", err);


Do you have a better suggestion?