Re: [PATCH v5 2/2] i2c: hisi: Add I2C bus recovery support

[Andi Shyti]

Hi Bowen,

the patch looks good and it can be taken without 1/2.

Just few nitpicks,

...

> +static int hisi_i2c_get_bus_recovery_res(struct hisi_i2c_controller *ctlr,
> +					 struct platform_device *pdev)
> +{
> +	struct resource *res0;

/res0/res/

> +
> +	res0 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +
> +	if (!res0 || resource_size(res0) != HISI_I2C_RECOVERY_REG_SIZE)
> +		return -ENODEV;
> +
> +	ctlr->sctrl_addr = devm_ioremap_resource(&pdev->dev, res0);
> +	if (IS_ERR(ctlr->sctrl_addr)) {
> +		ctlr->sctrl_addr = NULL;
> +		return -ENOMEM;

return PTR_ERR(ctlr->sctrl_addr);

> +	}
> +
> +	return 0;
> +}

...

>  static int hisi_i2c_start_xfer(struct hisi_i2c_controller *ctlr)
>  {
>  	struct i2c_msg *msg = ctlr->msgs;
> @@ -496,6 +634,10 @@ static int hisi_i2c_probe(struct platform_device *pdev)
>  
>  	hisi_i2c_configure_bus(ctlr);
>  
> +	ret = hisi_i2c_recovery_init(ctlr);
> +	if (ret)
> +		dev_info(ctlr->dev, "I2C bus recovery not available\n");

I would use here dev_dbg(...);

If it's OK with you, I can change the ones above and apply,
otherwise, please send a v6.

Andi

> +
>  	adapter = &ctlr->adapter;
>  	snprintf(adapter->name, sizeof(adapter->name),
>  		 "HiSilicon I2C Controller %s", dev_name(dev));
> -- 
> 2.33.0
>