[PATCH v9 2/4] i2c: tegra: Move variant to tegra_i2c_hw_feature

Kartik Rajput posted 4 patches 2 weeks, 4 days ago
There is a newer version of this series
[PATCH v9 2/4] i2c: tegra: Move variant to tegra_i2c_hw_feature
Posted by Kartik Rajput 2 weeks, 4 days ago
Move the variant field into tegra_i2c_hw_feature and populate it for all
SoCs. Add dedicated SoC data for "nvidia,tegra20-i2c-dvc" and
"nvidia,tegra210-i2c-vi" compatibles. Drop the compatible-string checks
from tegra_i2c_parse_dt to initialize the Tegra I2C variant.

Signed-off-by: Kartik Rajput <kkartik@nvidia.com>
---
Changes in v5:
	* Updated commit message.
Changes in v4:
	* Reverted the change to remove config checks from IS_DVC and
	  IS_VI macros.
---
 drivers/i2c/busses/i2c-tegra.c | 98 ++++++++++++++++++++++++++++------
 1 file changed, 81 insertions(+), 17 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index c2c60ba4fe5e..2ef5fba66b0f 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -235,6 +235,7 @@ enum tegra_i2c_variant {
  *		timing settings.
  * @enable_hs_mode_support: Enable support for high speed (HS) mode transfers.
  * @has_mutex: Has mutex register for mutual exclusion with other firmwares or VMs.
+ * @variant: This represents the I2C controller variant.
  */
 struct tegra_i2c_hw_feature {
 	bool has_continue_xfer_support;
@@ -266,6 +267,7 @@ struct tegra_i2c_hw_feature {
 	bool has_interface_timing_reg;
 	bool enable_hs_mode_support;
 	bool has_mutex;
+	enum tegra_i2c_variant variant;
 };
 
 /**
@@ -280,7 +282,6 @@ struct tegra_i2c_hw_feature {
  * @base_phys: physical base address of the I2C controller
  * @cont_id: I2C controller ID, used for packet header
  * @irq: IRQ number of transfer complete interrupt
- * @variant: This represents the I2C controller variant.
  * @msg_complete: transfer completion notifier
  * @msg_buf_remaining: size of unsent data in the message buffer
  * @msg_len: length of message in current transfer
@@ -332,13 +333,12 @@ struct tegra_i2c_dev {
 	bool atomic_mode;
 	bool dma_mode;
 	bool msg_read;
-	enum tegra_i2c_variant variant;
 };
 
 #define IS_DVC(dev) (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) && \
-		     (dev)->variant == TEGRA_I2C_VARIANT_DVC)
+		     (dev)->hw->variant == TEGRA_I2C_VARIANT_DVC)
 #define IS_VI(dev)  (IS_ENABLED(CONFIG_ARCH_TEGRA_210_SOC) && \
-		     (dev)->variant == TEGRA_I2C_VARIANT_VI)
+		     (dev)->hw->variant == TEGRA_I2C_VARIANT_VI)
 
 static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val,
 		       unsigned int reg)
@@ -1647,8 +1647,42 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
 	.has_interface_timing_reg = false,
 	.enable_hs_mode_support = false,
 	.has_mutex = false,
+	.variant = TEGRA_I2C_VARIANT_DEFAULT,
 };
 
+#if IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC)
+static const struct tegra_i2c_hw_feature tegra20_dvc_i2c_hw = {
+	.has_continue_xfer_support = false,
+	.has_per_pkt_xfer_complete_irq = false,
+	.clk_divisor_hs_mode = 3,
+	.clk_divisor_std_mode = 0,
+	.clk_divisor_fast_mode = 0,
+	.clk_divisor_fast_plus_mode = 0,
+	.has_config_load_reg = false,
+	.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,
+	.tlow_std_mode = 0x4,
+	.thigh_std_mode = 0x2,
+	.tlow_fast_mode = 0x4,
+	.thigh_fast_mode = 0x2,
+	.tlow_fastplus_mode = 0x4,
+	.thigh_fastplus_mode = 0x2,
+	.setup_hold_time_std_mode = 0x0,
+	.setup_hold_time_fast_mode = 0x0,
+	.setup_hold_time_fastplus_mode = 0x0,
+	.setup_hold_time_hs_mode = 0x0,
+	.has_interface_timing_reg = false,
+	.enable_hs_mode_support = false,
+	.has_mutex = false,
+	.variant = TEGRA_I2C_VARIANT_DVC,
+};
+#endif
+
 static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
 	.has_continue_xfer_support = true,
 	.has_per_pkt_xfer_complete_irq = false,
@@ -1677,6 +1711,7 @@ static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
 	.has_interface_timing_reg = false,
 	.enable_hs_mode_support = false,
 	.has_mutex = false,
+	.variant = TEGRA_I2C_VARIANT_DEFAULT,
 };
 
 static const struct tegra_i2c_hw_feature tegra114_i2c_hw = {
@@ -1707,6 +1742,7 @@ static const struct tegra_i2c_hw_feature tegra114_i2c_hw = {
 	.has_interface_timing_reg = false,
 	.enable_hs_mode_support = false,
 	.has_mutex = false,
+	.variant = TEGRA_I2C_VARIANT_DEFAULT,
 };
 
 static const struct tegra_i2c_hw_feature tegra124_i2c_hw = {
@@ -1737,6 +1773,7 @@ static const struct tegra_i2c_hw_feature tegra124_i2c_hw = {
 	.has_interface_timing_reg = true,
 	.enable_hs_mode_support = false,
 	.has_mutex = false,
+	.variant = TEGRA_I2C_VARIANT_DEFAULT,
 };
 
 static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
@@ -1767,8 +1804,42 @@ static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
 	.has_interface_timing_reg = true,
 	.enable_hs_mode_support = false,
 	.has_mutex = false,
+	.variant = TEGRA_I2C_VARIANT_DEFAULT,
 };
 
+#if IS_ENABLED(CONFIG_ARCH_TEGRA_210_SOC)
+static const struct tegra_i2c_hw_feature tegra210_vi_i2c_hw = {
+	.has_continue_xfer_support = true,
+	.has_per_pkt_xfer_complete_irq = true,
+	.clk_divisor_hs_mode = 1,
+	.clk_divisor_std_mode = 0x19,
+	.clk_divisor_fast_mode = 0x19,
+	.clk_divisor_fast_plus_mode = 0x10,
+	.has_config_load_reg = true,
+	.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,
+	.tlow_std_mode = 0x4,
+	.thigh_std_mode = 0x2,
+	.tlow_fast_mode = 0x4,
+	.thigh_fast_mode = 0x2,
+	.tlow_fastplus_mode = 0x4,
+	.thigh_fastplus_mode = 0x2,
+	.setup_hold_time_std_mode = 0,
+	.setup_hold_time_fast_mode = 0,
+	.setup_hold_time_fastplus_mode = 0,
+	.setup_hold_time_hs_mode = 0,
+	.has_interface_timing_reg = true,
+	.enable_hs_mode_support = false,
+	.has_mutex = false,
+	.variant = TEGRA_I2C_VARIANT_VI,
+};
+#endif
+
 static const struct tegra_i2c_hw_feature tegra186_i2c_hw = {
 	.has_continue_xfer_support = true,
 	.has_per_pkt_xfer_complete_irq = true,
@@ -1797,6 +1868,7 @@ static const struct tegra_i2c_hw_feature tegra186_i2c_hw = {
 	.has_interface_timing_reg = true,
 	.enable_hs_mode_support = false,
 	.has_mutex = false,
+	.variant = TEGRA_I2C_VARIANT_DEFAULT,
 };
 
 static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
@@ -1829,6 +1901,7 @@ static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
 	.has_interface_timing_reg = true,
 	.enable_hs_mode_support = true,
 	.has_mutex = false,
+	.variant = TEGRA_I2C_VARIANT_DEFAULT,
 };
 
 static const struct tegra_i2c_hw_feature tegra256_i2c_hw = {
@@ -1861,6 +1934,7 @@ static const struct tegra_i2c_hw_feature tegra256_i2c_hw = {
 	.has_interface_timing_reg = true,
 	.enable_hs_mode_support = true,
 	.has_mutex = true,
+	.variant = TEGRA_I2C_VARIANT_DEFAULT,
 };
 
 static const struct tegra_i2c_hw_feature tegra264_i2c_hw = {
@@ -1893,6 +1967,7 @@ static const struct tegra_i2c_hw_feature tegra264_i2c_hw = {
 	.has_interface_timing_reg = true,
 	.enable_hs_mode_support = true,
 	.has_mutex = true,
+	.variant = TEGRA_I2C_VARIANT_DEFAULT,
 };
 
 static const struct of_device_id tegra_i2c_of_match[] = {
@@ -1901,7 +1976,7 @@ static const struct of_device_id tegra_i2c_of_match[] = {
 	{ .compatible = "nvidia,tegra194-i2c", .data = &tegra194_i2c_hw, },
 	{ .compatible = "nvidia,tegra186-i2c", .data = &tegra186_i2c_hw, },
 #if IS_ENABLED(CONFIG_ARCH_TEGRA_210_SOC)
-	{ .compatible = "nvidia,tegra210-i2c-vi", .data = &tegra210_i2c_hw, },
+	{ .compatible = "nvidia,tegra210-i2c-vi", .data = &tegra210_vi_i2c_hw, },
 #endif
 	{ .compatible = "nvidia,tegra210-i2c", .data = &tegra210_i2c_hw, },
 	{ .compatible = "nvidia,tegra124-i2c", .data = &tegra124_i2c_hw, },
@@ -1909,7 +1984,7 @@ static const struct of_device_id tegra_i2c_of_match[] = {
 	{ .compatible = "nvidia,tegra30-i2c", .data = &tegra30_i2c_hw, },
 	{ .compatible = "nvidia,tegra20-i2c", .data = &tegra20_i2c_hw, },
 #if IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC)
-	{ .compatible = "nvidia,tegra20-i2c-dvc", .data = &tegra20_i2c_hw, },
+	{ .compatible = "nvidia,tegra20-i2c-dvc", .data = &tegra20_dvc_i2c_hw, },
 #endif
 	{},
 };
@@ -1917,23 +1992,12 @@ MODULE_DEVICE_TABLE(of, tegra_i2c_of_match);
 
 static void tegra_i2c_parse_dt(struct tegra_i2c_dev *i2c_dev)
 {
-	struct device_node *np = i2c_dev->dev->of_node;
 	bool multi_mode;
 
 	i2c_parse_fw_timings(i2c_dev->dev, &i2c_dev->timings, true);
 
 	multi_mode = device_property_read_bool(i2c_dev->dev, "multi-master");
 	i2c_dev->multimaster_mode = multi_mode;
-
-	i2c_dev->variant = TEGRA_I2C_VARIANT_DEFAULT;
-
-	if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
-	    of_device_is_compatible(np, "nvidia,tegra20-i2c-dvc"))
-		i2c_dev->variant = TEGRA_I2C_VARIANT_DVC;
-
-	if (IS_ENABLED(CONFIG_ARCH_TEGRA_210_SOC) &&
-	    of_device_is_compatible(np, "nvidia,tegra210-i2c-vi"))
-		i2c_dev->variant = TEGRA_I2C_VARIANT_VI;
 }
 
 static int tegra_i2c_init_clocks(struct tegra_i2c_dev *i2c_dev)
-- 
2.43.0
Re: [PATCH v9 2/4] i2c: tegra: Move variant to tegra_i2c_hw_feature
Posted by Andi Shyti 2 weeks, 2 days ago
Hi Kartik,

> @@ -280,7 +282,6 @@ struct tegra_i2c_hw_feature {
>   * @base_phys: physical base address of the I2C controller
>   * @cont_id: I2C controller ID, used for packet header
>   * @irq: IRQ number of transfer complete interrupt
> - * @variant: This represents the I2C controller variant.
>   * @msg_complete: transfer completion notifier
>   * @msg_buf_remaining: size of unsent data in the message buffer
>   * @msg_len: length of message in current transfer
> @@ -332,13 +333,12 @@ struct tegra_i2c_dev {
>  	bool atomic_mode;
>  	bool dma_mode;
>  	bool msg_read;
> -	enum tegra_i2c_variant variant;
>  };
>  
>  #define IS_DVC(dev) (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) && \
> -		     (dev)->variant == TEGRA_I2C_VARIANT_DVC)
> +		     (dev)->hw->variant == TEGRA_I2C_VARIANT_DVC)
>  #define IS_VI(dev)  (IS_ENABLED(CONFIG_ARCH_TEGRA_210_SOC) && \
> -		     (dev)->variant == TEGRA_I2C_VARIANT_VI)
> +		     (dev)->hw->variant == TEGRA_I2C_VARIANT_VI)
>  
>  static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val,
>  		       unsigned int reg)
> @@ -1647,8 +1647,42 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
>  	.has_interface_timing_reg = false,
>  	.enable_hs_mode_support = false,
>  	.has_mutex = false,
> +	.variant = TEGRA_I2C_VARIANT_DEFAULT,
>  };

in v8 I made a question at this point, could you please answer?

Thanks,
Andi
Re: [PATCH v9 2/4] i2c: tegra: Move variant to tegra_i2c_hw_feature
Posted by Kartik Rajput 2 weeks, 2 days ago
Hi Andi,

On 22/01/26 23:02, Andi Shyti wrote:
> External email: Use caution opening links or attachments
> 
> 
> Hi Kartik,
> 
>> @@ -280,7 +282,6 @@ struct tegra_i2c_hw_feature {
>>    * @base_phys: physical base address of the I2C controller
>>    * @cont_id: I2C controller ID, used for packet header
>>    * @irq: IRQ number of transfer complete interrupt
>> - * @variant: This represents the I2C controller variant.
>>    * @msg_complete: transfer completion notifier
>>    * @msg_buf_remaining: size of unsent data in the message buffer
>>    * @msg_len: length of message in current transfer
>> @@ -332,13 +333,12 @@ struct tegra_i2c_dev {
>>        bool atomic_mode;
>>        bool dma_mode;
>>        bool msg_read;
>> -     enum tegra_i2c_variant variant;
>>   };
>>
>>   #define IS_DVC(dev) (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) && \
>> -                  (dev)->variant == TEGRA_I2C_VARIANT_DVC)
>> +                  (dev)->hw->variant == TEGRA_I2C_VARIANT_DVC)
>>   #define IS_VI(dev)  (IS_ENABLED(CONFIG_ARCH_TEGRA_210_SOC) && \
>> -                  (dev)->variant == TEGRA_I2C_VARIANT_VI)
>> +                  (dev)->hw->variant == TEGRA_I2C_VARIANT_VI)
>>
>>   static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val,
>>                       unsigned int reg)
>> @@ -1647,8 +1647,42 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
>>        .has_interface_timing_reg = false,
>>        .enable_hs_mode_support = false,
>>        .has_mutex = false,
>> +     .variant = TEGRA_I2C_VARIANT_DEFAULT,
>>   };
> 
> in v8 I made a question at this point, could you please answer?
> 
> Thanks,
> Andi

Are you referring to the following question on v7 of this series, which I've already addressed?
https://lore.kernel.org/linux-tegra/c3cdece5-1bf6-46eb-a668-2ee2a4358a88@nvidia.com/#t


>> why do we need this extra step in this patch and we don't add
>> "variant" directly in tegra_i2c_dev->hw?
>>
> Since we are also introducing match data for DVC and VI variant, I
> decided to split this into two patches.
> 
> If you prefer, I can squash following into a single patch:
>   * "i2c: tegra: Introduce tegra_i2c_variant to identify DVC and VI"
>   * "i2c: tegra: Move variant to tegra_i2c_hw_feature"


Thanks,
Kartik
Re: [PATCH v9 2/4] i2c: tegra: Move variant to tegra_i2c_hw_feature
Posted by Andi Shyti 1 week, 3 days ago
Hi Kartik,

> > > @@ -280,7 +282,6 @@ struct tegra_i2c_hw_feature {
> > >    * @base_phys: physical base address of the I2C controller
> > >    * @cont_id: I2C controller ID, used for packet header
> > >    * @irq: IRQ number of transfer complete interrupt
> > > - * @variant: This represents the I2C controller variant.
> > >    * @msg_complete: transfer completion notifier
> > >    * @msg_buf_remaining: size of unsent data in the message buffer
> > >    * @msg_len: length of message in current transfer
> > > @@ -332,13 +333,12 @@ struct tegra_i2c_dev {
> > >        bool atomic_mode;
> > >        bool dma_mode;
> > >        bool msg_read;
> > > -     enum tegra_i2c_variant variant;
> > >   };
> > > 
> > >   #define IS_DVC(dev) (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) && \
> > > -                  (dev)->variant == TEGRA_I2C_VARIANT_DVC)
> > > +                  (dev)->hw->variant == TEGRA_I2C_VARIANT_DVC)
> > >   #define IS_VI(dev)  (IS_ENABLED(CONFIG_ARCH_TEGRA_210_SOC) && \
> > > -                  (dev)->variant == TEGRA_I2C_VARIANT_VI)
> > > +                  (dev)->hw->variant == TEGRA_I2C_VARIANT_VI)
> > > 
> > >   static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val,
> > >                       unsigned int reg)
> > > @@ -1647,8 +1647,42 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
> > >        .has_interface_timing_reg = false,
> > >        .enable_hs_mode_support = false,
> > >        .has_mutex = false,
> > > +     .variant = TEGRA_I2C_VARIANT_DEFAULT,
> > >   };
> > 
> > in v8 I made a question at this point, could you please answer?

...

> Are you referring to the following question on v7 of this series, which I've already addressed?
> https://lore.kernel.org/linux-tegra/c3cdece5-1bf6-46eb-a668-2ee2a4358a88@nvidia.com/#t

I'm sorry, what have you addressed. It doesn't look addressed to
me. In the same series, I want to avoid adding something in one
patch and removing it in the next. It just adds noise.

Andi
Re: [PATCH v9 2/4] i2c: tegra: Move variant to tegra_i2c_hw_feature
Posted by Kartik Rajput 1 week, 3 days ago
Hi Andi,

On 29/01/26 03:47, Andi Shyti wrote:
> External email: Use caution opening links or attachments
> 
> 
> Hi Kartik,
> 
>>>> @@ -280,7 +282,6 @@ struct tegra_i2c_hw_feature {
>>>>     * @base_phys: physical base address of the I2C controller
>>>>     * @cont_id: I2C controller ID, used for packet header
>>>>     * @irq: IRQ number of transfer complete interrupt
>>>> - * @variant: This represents the I2C controller variant.
>>>>     * @msg_complete: transfer completion notifier
>>>>     * @msg_buf_remaining: size of unsent data in the message buffer
>>>>     * @msg_len: length of message in current transfer
>>>> @@ -332,13 +333,12 @@ struct tegra_i2c_dev {
>>>>         bool atomic_mode;
>>>>         bool dma_mode;
>>>>         bool msg_read;
>>>> -     enum tegra_i2c_variant variant;
>>>>    };
>>>>
>>>>    #define IS_DVC(dev) (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) && \
>>>> -                  (dev)->variant == TEGRA_I2C_VARIANT_DVC)
>>>> +                  (dev)->hw->variant == TEGRA_I2C_VARIANT_DVC)
>>>>    #define IS_VI(dev)  (IS_ENABLED(CONFIG_ARCH_TEGRA_210_SOC) && \
>>>> -                  (dev)->variant == TEGRA_I2C_VARIANT_VI)
>>>> +                  (dev)->hw->variant == TEGRA_I2C_VARIANT_VI)
>>>>
>>>>    static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val,
>>>>                        unsigned int reg)
>>>> @@ -1647,8 +1647,42 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
>>>>         .has_interface_timing_reg = false,
>>>>         .enable_hs_mode_support = false,
>>>>         .has_mutex = false,
>>>> +     .variant = TEGRA_I2C_VARIANT_DEFAULT,
>>>>    };
>>>
>>> in v8 I made a question at this point, could you please answer?
> 
> ...
> 
>> Are you referring to the following question on v7 of this series, which I've already addressed?
>> https://lore.kernel.org/linux-tegra/c3cdece5-1bf6-46eb-a668-2ee2a4358a88@nvidia.com/#t
> 
> I'm sorry, what have you addressed. It doesn't look addressed to
> me. In the same series, I want to avoid adding something in one
> patch and removing it in the next. It just adds noise.
> 
> Andi

Ack. I will squash these two patches into one to avoid this.

Thanks,
Kartik