Re: [PATCH 2/2] i2c: cadence: Add reset controller support

[Michal Simek <michal.simek@amd.com>]

On 3/30/23 20:04, Lars-Peter Clausen wrote:
> The Cadence I2C controller has an external reset signal that needs to be
> de-asserted before the I2C controller can be used.
> 
> Add support to the driver to be able to take the peripheral out of reset
> using the reset controller API. The reset is optional in the driver for
> compatibility to systems where the reset managed by the bootloader.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>   drivers/i2c/busses/i2c-cadence.c | 26 ++++++++++++++++++++++----
>   1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
> index 8f61a633c42c..102774ab2497 100644
> --- a/drivers/i2c/busses/i2c-cadence.c
> +++ b/drivers/i2c/busses/i2c-cadence.c
> @@ -16,6 +16,7 @@
>   #include <linux/of.h>
>   #include <linux/pm_runtime.h>
>   #include <linux/pinctrl/consumer.h>
> +#include <linux/reset.h>
>   
>   /* Register offsets for the I2C device. */
>   #define CDNS_I2C_CR_OFFSET		0x00 /* Control Register, RW */
> @@ -178,6 +179,7 @@ enum cdns_i2c_slave_state {
>    * @bus_hold_flag:	Flag used in repeated start for clearing HOLD bit
>    * @clk:		Pointer to struct clk
>    * @clk_rate_change_nb:	Notifier block for clock rate changes
> + * @reset:		Reset control for the device
>    * @quirks:		flag for broken hold bit usage in r1p10
>    * @ctrl_reg:		Cached value of the control register.
>    * @ctrl_reg_diva_divb: value of fields DIV_A and DIV_B from CR register
> @@ -204,6 +206,7 @@ struct cdns_i2c {
>   	unsigned int bus_hold_flag;
>   	struct clk *clk;
>   	struct notifier_block clk_rate_change_nb;
> +	struct reset_control *reset;
>   	u32 quirks;
>   	u32 ctrl_reg;
>   	struct i2c_bus_recovery_info rinfo;
> @@ -1325,10 +1328,22 @@ static int cdns_i2c_probe(struct platform_device *pdev)
>   		return dev_err_probe(&pdev->dev, PTR_ERR(id->clk),
>   				     "input clock not found.\n");
>   
> +	id->reset = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
> +	if (IS_ERR(id->reset))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(id->reset),
> +							 "Failed to request reset.\n");

incorrect alignment.

> +
>   	ret = clk_prepare_enable(id->clk);
>   	if (ret)
>   		dev_err(&pdev->dev, "Unable to enable clock.\n");
>   
> +	ret = reset_control_deassert(id->reset);
> +	if (ret) {
> +		dev_err_probe(&pdev->dev, ret,
> +					  "Failed to de-assert reset.\n");

incorrect alignment

The rest looks good to me.

M