lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3357e516-7a79-41ec-2e25-18eb39e6bf3f@gmail.com>
Date:   Tue, 5 Feb 2019 17:28:07 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Sowjanya Komatineni <skomatineni@...dia.com>,
        thierry.reding@...il.com, jonathanh@...dia.com,
        mkarthik@...dia.com, smohammed@...dia.com, talho@...dia.com
Cc:     linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-i2c@...r.kernel.org
Subject: Re: [PATCH V11 5/5] i2c: tegra: add i2c interface timing support

05.02.2019 11:56, Sowjanya Komatineni пишет:
> This patch adds I2C interface timing registers support for
> proper bus rate configuration along with meeting the i2c spec
> setup and hold times based on the tuning performed on Tegra210,
> Tegra186 and Tegra194 platforms.
> 
> I2C_INTERFACE_TIMING_0 register contains TLOW and THIGH field
> and Tegra I2C controller design uses them as a part of internal
> clock divisor.
> 
> I2C_INTERFACE_TIMING_1 register contains the setup and hold times
> for start and stop conditions.
> 
> Acked-by: Thierry Reding <treding@...dia.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@...dia.com>
> ---
>  [V9/V10/V11] : Same as V8
>  [V8] : Updated to handle timing implementation within tegra_i2c_init directly
>  [V7] : Minor updates to timing implementation
>  [V5/V6] : Added this Interface timing patch in V5 of the patch series.
> 
>  drivers/i2c/busses/i2c-tegra.c | 189 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 160 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index f7584f0ea7da..841cad6ccb57 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -129,6 +129,15 @@
>  #define I2C_MST_FIFO_STATUS_TX_MASK		0xff0000
>  #define I2C_MST_FIFO_STATUS_TX_SHIFT		16
>  
> +#define I2C_INTERFACE_TIMING_0			0x94
> +#define I2C_THIGH_SHIFT				8
> +#define I2C_INTERFACE_TIMING_1			0x98
> +
> +#define I2C_STANDARD_MODE			100000
> +#define I2C_FAST_MODE				400000
> +#define I2C_FAST_PLUS_MODE			1000000
> +#define I2C_HS_MODE				3500000
> +
>  /* Packet header size in bytes */
>  #define I2C_PACKET_HEADER_SIZE			12
>  
> @@ -166,7 +175,10 @@ enum msg_end_type {
>   * @has_config_load_reg: Has the config load register to load the new
>   *		configuration.
>   * @clk_divisor_hs_mode: Clock divisor in HS mode.
> - * @clk_divisor_std_fast_mode: Clock divisor in standard/fast mode. It is
> + * @clk_divisor_std_mode: Clock divisor in standard mode. It is
> + *		applicable if there is no fast clock source i.e. single clock
> + *		source.
> + * @clk_divisor_fast_mode: Clock divisor in fast mode. It is
>   *		applicable if there is no fast clock source i.e. single clock
>   *		source.
>   * @clk_divisor_fast_plus_mode: Clock divisor in fast mode plus. It is
> @@ -184,6 +196,16 @@ enum msg_end_type {
>   * @supports_bus_clear: Bus Clear support to recover from bus hang during
>   *		SDA stuck low from device for some unknown reasons.
>   * @has_apb_dma: Support of APBDMA on corresponding Tegra chip.
> + * @tlow_std_mode: Low period of the clock in standard mode.
> + * @thigh_std_mode: High period of the clock in standard mode.
> + * @tlow_fast_fastplus_mode: Low period of the clock in fast/fast-plus modes.
> + * @thigh_fast_fastplus_mode: High period of the clock in fast/fast-plus modes.
> + * @setup_hold_time_std_mode: Setup and hold time for start and stop conditions
> + *		in standard mode.
> + * @setup_hold_time_fast_fast_plus_mode: Setup and hold time for start and stop
> + *		conditions in fast/fast-plus modes.
> + * @setup_hold_time_hs_mode: Setup and hold time for start and stop conditions
> + *		in HS mode.
>   */
>  struct tegra_i2c_hw_feature {
>  	bool has_continue_xfer_support;
> @@ -191,7 +213,8 @@ struct tegra_i2c_hw_feature {
>  	bool has_single_clk_source;
>  	bool has_config_load_reg;
>  	int clk_divisor_hs_mode;
> -	int clk_divisor_std_fast_mode;
> +	int clk_divisor_std_mode;
> +	int clk_divisor_fast_mode;
>  	u16 clk_divisor_fast_plus_mode;
>  	bool has_multi_master_mode;
>  	bool has_slcg_override_reg;
> @@ -199,6 +222,13 @@ struct tegra_i2c_hw_feature {
>  	const struct i2c_adapter_quirks *quirks;
>  	bool supports_bus_clear;
>  	bool has_apb_dma;
> +	u8 tlow_std_mode;
> +	u8 thigh_std_mode;
> +	u8 tlow_fast_fastplus_mode;
> +	u8 thigh_fast_fastplus_mode;
> +	u32 setup_hold_time_std_mode;
> +	u32 setup_hold_time_fast_fast_plus_mode;
> +	u32 setup_hold_time_hs_mode;
>  };
>  
>  /**
> @@ -666,11 +696,13 @@ static int tegra_i2c_wait_for_config_load(struct tegra_i2c_dev *i2c_dev)
>  	return 0;
>  }
>  
> -static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
> +static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev, bool clk_reinit)
>  {
>  	u32 val;
>  	int err;
> -	u32 clk_divisor;
> +	u32 clk_divisor, clk_multiplier;
> +	u32 tsu_thd = 0;
> +	u8 tlow, thigh;
>  
>  	err = pm_runtime_get_sync(i2c_dev->dev);
>  	if (err < 0) {
> @@ -700,6 +732,36 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
>  					I2C_CLK_DIVISOR_STD_FAST_MODE_SHIFT;
>  	i2c_writel(i2c_dev, clk_divisor, I2C_CLK_DIVISOR);
>  
> +	if ((i2c_dev->bus_clk_rate > I2C_STANDARD_MODE) &&
> +	    (i2c_dev->bus_clk_rate <= I2C_FAST_PLUS_MODE)) {

CHECK: Unnecessary parentheses around 'i2c_dev->bus_clk_rate > I2C_STANDARD_MODE'
#1282: FILE: drivers/i2c/busses/i2c-tegra.c:735:
+       if ((i2c_dev->bus_clk_rate > I2C_STANDARD_MODE) &&
+           (i2c_dev->bus_clk_rate <= I2C_FAST_PLUS_MODE)) {

CHECK: Unnecessary parentheses around 'i2c_dev->bus_clk_rate <= I2C_FAST_PLUS_MODE'
#1282: FILE: drivers/i2c/busses/i2c-tegra.c:735:
+       if ((i2c_dev->bus_clk_rate > I2C_STANDARD_MODE) &&
+           (i2c_dev->bus_clk_rate <= I2C_FAST_PLUS_MODE)) {


> +		tlow = i2c_dev->hw->tlow_fast_fastplus_mode;
> +		thigh = i2c_dev->hw->thigh_fast_fastplus_mode;
> +		tsu_thd = i2c_dev->hw->setup_hold_time_fast_fast_plus_mode;
> +	} else {
> +		tlow = i2c_dev->hw->tlow_std_mode;
> +		thigh = i2c_dev->hw->thigh_std_mode;
> +		tsu_thd = i2c_dev->hw->setup_hold_time_std_mode;
> +	}
> +
> +	val = (thigh << I2C_THIGH_SHIFT) | tlow;
> +	if (val)
> +		i2c_writel(i2c_dev, val, I2C_INTERFACE_TIMING_0);
> +
> +	if (tsu_thd)
> +		i2c_writel(i2c_dev, tsu_thd, I2C_INTERFACE_TIMING_1);
> +
> +	if (!clk_reinit) {
> +		clk_multiplier = (tlow + thigh + 2);
> +		clk_multiplier *= (i2c_dev->clk_divisor_non_hs_mode + 1);
> +		err = clk_set_rate(i2c_dev->div_clk,
> +				   i2c_dev->bus_clk_rate * clk_multiplier);
> +		if (err) {
> +			dev_err(i2c_dev->dev,
> +				"failed changing clock rate: %d\n", err);
> +			goto err;
> +		}
> +	}
> +
>  	if (!i2c_dev->is_dvc) {
>  		u32 sl_cfg = i2c_readl(i2c_dev, I2C_SL_CNFG);
>  
> @@ -1106,7 +1168,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
>  			dmaengine_terminate_sync(i2c_dev->msg_read ?
>  						  i2c_dev->rx_dma_chan :
>  						  i2c_dev->tx_dma_chan);
> -			tegra_i2c_init(i2c_dev);
> +			tegra_i2c_init(i2c_dev, true);
>  			return -ETIMEDOUT;
>  		}
>  
> @@ -1133,7 +1195,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
>  	if (time_left == 0) {
>  		dev_err(i2c_dev->dev, "i2c transfer timed out\n");
>  
> -		tegra_i2c_init(i2c_dev);
> +		tegra_i2c_init(i2c_dev, true);
>  		return -ETIMEDOUT;
>  	}
>  
> @@ -1145,7 +1207,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
>  	if (likely(i2c_dev->msg_err == I2C_ERR_NONE))
>  		return 0;
>  
> -	tegra_i2c_init(i2c_dev);
> +	tegra_i2c_init(i2c_dev, true);
>  	/* start recovery upon arbitration loss in single master mode */
>  	if (i2c_dev->msg_err == I2C_ERR_ARBITRATION_LOST) {
>  		if (!i2c_dev->is_multimaster_mode)
> @@ -1240,7 +1302,8 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
>  	.has_per_pkt_xfer_complete_irq = false,
>  	.has_single_clk_source = false,
>  	.clk_divisor_hs_mode = 3,
> -	.clk_divisor_std_fast_mode = 0,
> +	.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,
> @@ -1249,6 +1312,13 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
>  	.quirks = &tegra_i2c_quirks,
>  	.supports_bus_clear = false,
>  	.has_apb_dma = true,
> +	.tlow_std_mode = 0x4,
> +	.thigh_std_mode = 0x2,
> +	.tlow_fast_fastplus_mode = 0x4,
> +	.thigh_fast_fastplus_mode = 0x2,
> +	.setup_hold_time_std_mode = 0x0,
> +	.setup_hold_time_fast_fast_plus_mode = 0x0,
> +	.setup_hold_time_hs_mode = 0x0,
>  };
>  
>  static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
> @@ -1256,7 +1326,8 @@ static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
>  	.has_per_pkt_xfer_complete_irq = false,
>  	.has_single_clk_source = false,
>  	.clk_divisor_hs_mode = 3,
> -	.clk_divisor_std_fast_mode = 0,
> +	.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,
> @@ -1265,6 +1336,13 @@ static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
>  	.quirks = &tegra_i2c_quirks,
>  	.supports_bus_clear = false,
>  	.has_apb_dma = true,
> +	.tlow_std_mode = 0x4,
> +	.thigh_std_mode = 0x2,
> +	.tlow_fast_fastplus_mode = 0x4,
> +	.thigh_fast_fastplus_mode = 0x2,
> +	.setup_hold_time_std_mode = 0x0,
> +	.setup_hold_time_fast_fast_plus_mode = 0x0,
> +	.setup_hold_time_hs_mode = 0x0,
>  };
>  
>  static const struct tegra_i2c_hw_feature tegra114_i2c_hw = {
> @@ -1272,7 +1350,8 @@ static const struct tegra_i2c_hw_feature tegra114_i2c_hw = {
>  	.has_per_pkt_xfer_complete_irq = true,
>  	.has_single_clk_source = true,
>  	.clk_divisor_hs_mode = 1,
> -	.clk_divisor_std_fast_mode = 0x19,
> +	.clk_divisor_std_mode = 0x19,
> +	.clk_divisor_fast_mode = 0x19,
>  	.clk_divisor_fast_plus_mode = 0x10,
>  	.has_config_load_reg = false,
>  	.has_multi_master_mode = false,
> @@ -1281,6 +1360,13 @@ static const struct tegra_i2c_hw_feature tegra114_i2c_hw = {
>  	.quirks = &tegra_i2c_quirks,
>  	.supports_bus_clear = true,
>  	.has_apb_dma = true,
> +	.tlow_std_mode = 0x4,
> +	.thigh_std_mode = 0x2,
> +	.tlow_fast_fastplus_mode = 0x4,
> +	.thigh_fast_fastplus_mode = 0x2,
> +	.setup_hold_time_std_mode = 0x0,
> +	.setup_hold_time_fast_fast_plus_mode = 0x0,
> +	.setup_hold_time_hs_mode = 0x0,
>  };
>  
>  static const struct tegra_i2c_hw_feature tegra124_i2c_hw = {
> @@ -1288,7 +1374,8 @@ static const struct tegra_i2c_hw_feature tegra124_i2c_hw = {
>  	.has_per_pkt_xfer_complete_irq = true,
>  	.has_single_clk_source = true,
>  	.clk_divisor_hs_mode = 1,
> -	.clk_divisor_std_fast_mode = 0x19,
> +	.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,
> @@ -1297,6 +1384,13 @@ static const struct tegra_i2c_hw_feature tegra124_i2c_hw = {
>  	.quirks = &tegra_i2c_quirks,
>  	.supports_bus_clear = true,
>  	.has_apb_dma = true,
> +	.tlow_std_mode = 0x4,
> +	.thigh_std_mode = 0x2,
> +	.tlow_fast_fastplus_mode = 0x4,
> +	.thigh_fast_fastplus_mode = 0x2,
> +	.setup_hold_time_std_mode = 0x0,
> +	.setup_hold_time_fast_fast_plus_mode = 0x0,
> +	.setup_hold_time_hs_mode = 0x0,
>  };
>  
>  static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
> @@ -1304,7 +1398,8 @@ static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
>  	.has_per_pkt_xfer_complete_irq = true,
>  	.has_single_clk_source = true,
>  	.clk_divisor_hs_mode = 1,
> -	.clk_divisor_std_fast_mode = 0x19,
> +	.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 = true,
> @@ -1313,27 +1408,67 @@ static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
>  	.quirks = &tegra_i2c_quirks,
>  	.supports_bus_clear = true,
>  	.has_apb_dma = true,
> +	.tlow_std_mode = 0x4,
> +	.thigh_std_mode = 0x2,
> +	.tlow_fast_fastplus_mode = 0x4,
> +	.thigh_fast_fastplus_mode = 0x2,
> +	.setup_hold_time_std_mode = 0,
> +	.setup_hold_time_fast_fast_plus_mode = 0,
> +	.setup_hold_time_hs_mode = 0,
>  };
>  
> -static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
> +static const struct tegra_i2c_hw_feature tegra186_i2c_hw = {
>  	.has_continue_xfer_support = true,
>  	.has_per_pkt_xfer_complete_irq = true,
>  	.has_single_clk_source = true,
>  	.clk_divisor_hs_mode = 1,
> -	.clk_divisor_std_fast_mode = 0x19,
> +	.clk_divisor_std_mode = 0x16,
> +	.clk_divisor_fast_mode = 0x19,
>  	.clk_divisor_fast_plus_mode = 0x10,
>  	.has_config_load_reg = true,
>  	.has_multi_master_mode = true,
>  	.has_slcg_override_reg = true,
>  	.has_mst_fifo = true,
> +	.quirks = &tegra_i2c_quirks,
> +	.supports_bus_clear = true,
> +	.has_apb_dma = false,
> +	.tlow_std_mode = 0x4,
> +	.thigh_std_mode = 0x3,
> +	.tlow_fast_fastplus_mode = 0x4,
> +	.thigh_fast_fastplus_mode = 0x2,
> +	.setup_hold_time_std_mode = 0,
> +	.setup_hold_time_fast_fast_plus_mode = 0,
> +	.setup_hold_time_hs_mode = 0,
> +};
> +
> +static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
> +	.has_continue_xfer_support = true,
> +	.has_per_pkt_xfer_complete_irq = true,
> +	.has_single_clk_source = true,
> +	.clk_divisor_hs_mode = 1,
> +	.clk_divisor_std_mode = 0x4f,
> +	.clk_divisor_fast_mode = 0x3c,
> +	.clk_divisor_fast_plus_mode = 0x16,
> +	.has_config_load_reg = true,
> +	.has_multi_master_mode = true,
> +	.has_slcg_override_reg = true,
> +	.has_mst_fifo = true,
>  	.quirks = &tegra194_i2c_quirks,
>  	.supports_bus_clear = true,
>  	.has_apb_dma = false,
> +	.tlow_std_mode = 0x8,
> +	.thigh_std_mode = 0x7,
> +	.tlow_fast_fastplus_mode = 0x2,
> +	.thigh_fast_fastplus_mode = 0x2,
> +	.setup_hold_time_std_mode = 0x08080808,
> +	.setup_hold_time_fast_fast_plus_mode = 0x02020202,
> +	.setup_hold_time_hs_mode = 0x090909,
>  };
>  
>  /* Match table for of_platform binding */
>  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, },
>  	{ .compatible = "nvidia,tegra210-i2c", .data = &tegra210_i2c_hw, },
>  	{ .compatible = "nvidia,tegra124-i2c", .data = &tegra124_i2c_hw, },
>  	{ .compatible = "nvidia,tegra114-i2c", .data = &tegra114_i2c_hw, },
> @@ -1354,7 +1489,6 @@ static int tegra_i2c_probe(struct platform_device *pdev)
>  	phys_addr_t base_phys;
>  	int irq;
>  	int ret = 0;
> -	int clk_multiplier = I2C_CLK_MULTIPLIER_STD_FAST_MODE;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	base_phys = res->start;
> @@ -1425,20 +1559,17 @@ static int tegra_i2c_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	i2c_dev->clk_divisor_non_hs_mode =
> -			i2c_dev->hw->clk_divisor_std_fast_mode;
> -	if (i2c_dev->hw->clk_divisor_fast_plus_mode &&
> -		(i2c_dev->bus_clk_rate == 1000000))
> +	if ((i2c_dev->bus_clk_rate > I2C_FAST_MODE) &&
> +	    (i2c_dev->bus_clk_rate <= I2C_FAST_PLUS_MODE))

CHECK: Unnecessary parentheses around 'i2c_dev->bus_clk_rate > I2C_FAST_MODE'
#1534: FILE: drivers/i2c/busses/i2c-tegra.c:1562:
+       if ((i2c_dev->bus_clk_rate > I2C_FAST_MODE) &&
+           (i2c_dev->bus_clk_rate <= I2C_FAST_PLUS_MODE))

CHECK: Unnecessary parentheses around 'i2c_dev->bus_clk_rate <= I2C_FAST_PLUS_MODE'
#1534: FILE: drivers/i2c/busses/i2c-tegra.c:1562:
+       if ((i2c_dev->bus_clk_rate > I2C_FAST_MODE) &&
+           (i2c_dev->bus_clk_rate <= I2C_FAST_PLUS_MODE))

[snip]

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ