Re: [PATCH v2 4/8] Add Advantech EIO I2C driver

Hi Ramiro,

I had a fast look and I'm limiting to few comments this round.

On Tue, Jul 14, 2026 at 05:54:18PM +0200, Ramiro Oliveira wrote:
> This commit adds the driver to control the Advantech EIO I2C block, this
> block is included in the Advantech EIO MFD.

Can you please read the SubmittingPatches documentation for
properly formatting the commit log? Please ask if you have
questions.

...

> +static inline int eio_reg_set_bits(const struct eio_i2c_chan *chan,
> +				   unsigned int reg, unsigned int mask)
> +{
> +	return regmap_update_bits(chan->parent->regmap, reg, mask, mask);
> +}
> +
> +static inline int eio_reg_clear_bits(const struct eio_i2c_chan *chan,
> +				     unsigned int reg, unsigned int mask)
> +{
> +	return regmap_update_bits(chan->parent->regmap, reg, mask, 0);

should the two regmap_update_bits above use chan->base + reg?

> +}

...

> +static int smb_access(struct eio_i2c_chan *i2c_chan, u8 addr, bool is_read, u8 cmd,
> +		      int size, union i2c_smbus_data *data)
> +{
> +	int i, tmp, ret = 0;
> +	unsigned int st1, st2;
> +	int len = 0;

...

> +		for (i = 1; i <= len; i++)
> +			eio_reg_read(i2c_chan, SMB_REG_HBLOCK,
> +				     (unsigned int *)&data->block[i]);

mmmhhhh... how many bytes are you sending to eio_reg_read()
starting from &data_block[i]?

> +		break;

...

> +static int eio_i2c_probe(struct platform_device *pdev)
> +{
> +	static const char * const names[] = { "i2c0", "i2c1", "smb0", "smb1" };
> +	struct device *dev = &pdev->dev;
> +	struct eio_i2c_dev *eio_i2c;
> +	struct eio_dev *eio_dev = dev_get_drvdata(dev->parent);
> +	int ret = 0;
> +	enum eio_chan_id ch;
> +
> +	if (!eio_dev) {
> +		dev_err(dev, "Error contact eio_core\n");

please use dev_err_probe()

Andi

> +		return -ENODEV;
> +	}