Re: [PATCH 2/4] i2c: tegra: make reset an optional property

[Philipp Zabel <p.zabel@pengutronix.de>]

On Di, 2025-05-06 at 15:29 +0530, Akhil R wrote:
> For controllers that has an internal software reset, make the reset
> property optional. This is useful in systems that choose to restrict
> reset control from Linux.
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 35 ++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 87976e99e6d0..49b77dcef184 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -134,6 +134,8 @@
>  #define I2C_MST_FIFO_STATUS_TX			GENMASK(23, 16)
>  #define I2C_MST_FIFO_STATUS_RX			GENMASK(7, 0)
>  
> +#define I2C_MASTER_RESET_CNTRL			0x0a8
> +
>  /* configuration load timeout in microseconds */
>  #define I2C_CONFIG_LOAD_TIMEOUT			1000000
>  
> @@ -184,6 +186,9 @@ enum msg_end_type {
>   * @has_mst_fifo: The I2C controller contains the new MST FIFO interface that
>   *		provides additional features and allows for longer messages to
>   *		be transferred in one go.
> + * @has_mst_reset: The I2C controller contains MASTER_RESET_CTRL register which
> + *		provides an alternative to controller reset when configured as
> + *		I2C master
>   * @quirks: I2C adapter quirks for limiting write/read transfer size and not
>   *		allowing 0 length transfers.
>   * @supports_bus_clear: Bus Clear support to recover from bus hang during
> @@ -213,6 +218,7 @@ struct tegra_i2c_hw_feature {
>  	bool has_multi_master_mode;
>  	bool has_slcg_override_reg;
>  	bool has_mst_fifo;
> +	bool has_mst_reset;
>  	const struct i2c_adapter_quirks *quirks;
>  	bool supports_bus_clear;
>  	bool has_apb_dma;
> @@ -604,6 +610,18 @@ static int tegra_i2c_wait_for_config_load(struct tegra_i2c_dev *i2c_dev)
>  	return 0;
>  }
>  
> +static int tegra_i2c_master_reset(struct tegra_i2c_dev *i2c_dev)
> +{
> +	if (!i2c_dev->hw->has_mst_reset)
> +		return -EOPNOTSUPP;
> +
> +	i2c_writel(i2c_dev, 0x1, I2C_MASTER_RESET_CNTRL);
> +	udelay(1);
> +	i2c_writel(i2c_dev, 0x0, I2C_MASTER_RESET_CNTRL);
> +
> +	return 0;
> +}
> +
>  static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
>  {
>  	u32 val, clk_divisor, clk_multiplier, tsu_thd, tlow, thigh, non_hs_mode;
> @@ -621,8 +639,10 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
>  	 */
>  	if (handle)
>  		err = acpi_evaluate_object(handle, "_RST", NULL, NULL);
> -	else
> +	else if (i2c_dev->rst)
>  		err = reset_control_reset(i2c_dev->rst);
> +	else
> +		err = tegra_i2c_master_reset(i2c_dev);
>  
>  	WARN_ON_ONCE(err);
>  
> @@ -1467,6 +1487,7 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
>  	.has_multi_master_mode = false,
>  	.has_slcg_override_reg = false,
>  	.has_mst_fifo = false,
> +	.has_mst_reset = false,
>  	.quirks = &tegra_i2c_quirks,
>  	.supports_bus_clear = false,
>  	.has_apb_dma = true,
> @@ -1491,6 +1512,7 @@ static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
>  	.has_multi_master_mode = false,
>  	.has_slcg_override_reg = false,
>  	.has_mst_fifo = false,
> +	.has_mst_reset = false,
>  	.quirks = &tegra_i2c_quirks,
>  	.supports_bus_clear = false,
>  	.has_apb_dma = true,
> @@ -1515,6 +1537,7 @@ static const struct tegra_i2c_hw_feature tegra114_i2c_hw = {
>  	.has_multi_master_mode = false,
>  	.has_slcg_override_reg = false,
>  	.has_mst_fifo = false,
> +	.has_mst_reset = false,
>  	.quirks = &tegra_i2c_quirks,
>  	.supports_bus_clear = true,
>  	.has_apb_dma = true,
> @@ -1539,6 +1562,7 @@ static const struct tegra_i2c_hw_feature tegra124_i2c_hw = {
>  	.has_multi_master_mode = false,
>  	.has_slcg_override_reg = true,
>  	.has_mst_fifo = false,
> +	.has_mst_reset = false,
>  	.quirks = &tegra_i2c_quirks,
>  	.supports_bus_clear = true,
>  	.has_apb_dma = true,
> @@ -1563,6 +1587,7 @@ static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
>  	.has_multi_master_mode = false,
>  	.has_slcg_override_reg = true,
>  	.has_mst_fifo = false,
> +	.has_mst_reset = false,
>  	.quirks = &tegra_i2c_quirks,
>  	.supports_bus_clear = true,
>  	.has_apb_dma = true,
> @@ -1587,6 +1612,7 @@ static const struct tegra_i2c_hw_feature tegra186_i2c_hw = {
>  	.has_multi_master_mode = false,
>  	.has_slcg_override_reg = true,
>  	.has_mst_fifo = false,
> +	.has_mst_reset = false,
>  	.quirks = &tegra_i2c_quirks,
>  	.supports_bus_clear = true,
>  	.has_apb_dma = false,
> @@ -1611,6 +1637,7 @@ static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
>  	.has_multi_master_mode = true,
>  	.has_slcg_override_reg = true,
>  	.has_mst_fifo = true,
> +	.has_mst_reset = true,
>  	.quirks = &tegra194_i2c_quirks,
>  	.supports_bus_clear = true,
>  	.has_apb_dma = false,
> @@ -1666,7 +1693,11 @@ static int tegra_i2c_init_reset(struct tegra_i2c_dev *i2c_dev)
>  	if (ACPI_HANDLE(i2c_dev->dev))
>  		return 0;
>  
> -	i2c_dev->rst = devm_reset_control_get_exclusive(i2c_dev->dev, "i2c");
> +	if (i2c_dev->hw->has_mst_reset)
> +		i2c_dev->rst = devm_reset_control_get_optional_exclusive(i2c_dev->dev, "i2c");
> +	else
> +		i2c_dev->rst = devm_reset_control_get_exclusive(i2c_dev->dev, "i2c");

This could just use devm_reset_control_get_optional_exclusive()
unconditionally. If the device tree correctly marked the required
resets as non-optional, DT checks would guarantee that required resets
are present in the device tree.

regards
Philipp