Re: [PATCH v8 02/11] mfd: Add max7360 support

["Mathieu Dubois-Briand" <mathieu.dubois-briand@bootlin.com>]

On Fri May 9, 2025 at 11:29 AM CEST, Christophe JAILLET wrote:
> Le 09/05/2025 à 11:14, mathieu.dubois-briand@bootlin.com a écrit :
>> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
>> 
>> Add core driver to support MAX7360 i2c chip, multi function device
>> with keypad, GPIO, PWM, GPO and rotary encoder submodules.
>> 
>> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
>> Co-developed-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
>> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
>> ---
>
> Hi,
>
> ...
>
>> +static int max7360_mask_irqs(struct regmap *regmap)
>> +{
>> +	struct device *dev = regmap_get_device(regmap);
>> +	unsigned int val;
>> +	int ret;
>> +
>> +	/*
>> +	 * GPIO/PWM interrupts are not masked on reset: as the MAX7360 "INTI"
>> +	 * interrupt line is shared between GPIOs and rotary encoder, this could
>> +	 * result in repeated spurious interrupts on the rotary encoder driver
>> +	 * if the GPIO driver is not loaded. Mask them now to avoid this
>> +	 * situation.
>> +	 */
>> +	for (unsigned int i = 0; i < MAX7360_PORT_PWM_COUNT; i++) {
>> +		ret = regmap_write_bits(regmap, MAX7360_REG_PWMCFG(i),
>> +					MAX7360_PORT_CFG_INTERRUPT_MASK,
>> +					MAX7360_PORT_CFG_INTERRUPT_MASK);
>> +		if (ret)
>> +			return dev_err_probe(dev, ret,
>> +					     "Failed to write MAX7360 port configuration");
>
> Nitpick: Missing \n
>
>> +	}
>> +
>> +	/* Read GPIO in register, to ACK any pending IRQ. */
>> +	ret = regmap_read(regmap, MAX7360_REG_GPIOIN, &val);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Failed to read GPIO values: %d\n", ret);
>
> Nitpick: ret is not needed in the error message.
>
>> +
>> +	return 0;
>> +}
>
> ...
>
> CJ

Hi Christophe,

Thanks, I'm fixing the two messages.

Thanks for your review.
Mathieu

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com