linux-kernel - RE: [PATCH V1] i2c: tegra: Fix Maximum transfer size

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: <BN7PR12MB283656FC1AA704474BC52728C28C0@BN7PR12MB2836.namprd12.prod.outlook.com>
Date:   Wed, 2 Jan 2019 22:39:56 +0000
From:   Sowjanya Komatineni <skomatineni@...dia.com>
To:     Peter Rosin <peda@...ntia.se>,
        "thierry.reding@...il.com" <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Timo Alho <talho@...dia.com>
CC:     "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>
Subject: RE: [PATCH V1] i2c: tegra: Fix Maximum transfer size

Thanks Peter.

>> Tegra194 supports maximum 64K Bytes transfer per packet.
>> Tegra186 and prior supports maximum 4K Bytes transfer per packet.
>> 
>> This patch fixes this payload difference between Tegra194 and prior 
>> tegra chipsets.
>> 
>> Signed-off-by: Sowjanya Komatineni <skomatineni@...dia.com>
>> ---
>>  drivers/i2c/busses/i2c-tegra.c | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/i2c/busses/i2c-tegra.c 
>> b/drivers/i2c/busses/i2c-tegra.c index 437294ea2f0a..0c08ed8b6c94 
>> 100644
>> --- a/drivers/i2c/busses/i2c-tegra.c
>> +++ b/drivers/i2c/busses/i2c-tegra.c
>> @@ -167,6 +167,7 @@ struct tegra_i2c_hw_feature {
>>  	bool has_multi_master_mode;
>>  	bool has_slcg_override_reg;
>>  	bool has_mst_fifo;
>> +	struct i2c_adapter_quirks *quirks;
>
>This could perhaps be a pointer to const?
>
Yes, corrected in V2.
>>  };
>>  
>>  /**
>> @@ -833,6 +834,12 @@ static const struct i2c_adapter_quirks tegra_i2c_quirks = {
>>  	.max_write_len = 4096,
>>  };
>>  
>> +static const struct i2c_adapter_quirks tegra194_i2c_quirks = {
>> +	.flags = I2C_AQ_NO_ZERO_LEN,
>
>This is not mentioned in the commit message and the flag is not present in the old struct tegra_i2c_quirks above. Are you trying to sneak changes past the maintainers, or was the change simply not intentional?
Flag of NO_ZERO_LEN is present in tegra_i2c_quirks as well. I have added separate quirk with same flag and payload change for Tegra194
>> +	.max_read_len = 65536,
>> +	.max_write_len = 65536,
>
>Those are highly dubious when .max_read_len and .max_write_len are of type u16. I bet you could just remove them.
Yes, corrected in V2.
>
>Cheers,
>Peter
>
>> +};
>> +
>>  static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
>>  	.has_continue_xfer_support = false,
>>  	.has_per_pkt_xfer_complete_irq = false, @@ -844,6 +851,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,
>> +	.quirks = &tegra_i2c_quirks,
>>  };
>>  
>>  static const struct tegra_i2c_hw_feature tegra30_i2c_hw = { @@ -857,6 
>> +865,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,
>> +	.quirks = &tegra_i2c_quirks,
>>  };
>>  
>>  static const struct tegra_i2c_hw_feature tegra114_i2c_hw = { @@ 
>> -870,6 +879,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,
>> +	.quirks = &tegra_i2c_quirks,
>>  };
>>  
>>  static const struct tegra_i2c_hw_feature tegra124_i2c_hw = { @@ 
>> -883,6 +893,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,
>> +	.quirks = &tegra_i2c_quirks,
>>  };
>>  
>>  static const struct tegra_i2c_hw_feature tegra210_i2c_hw = { @@ 
>> -896,6 +907,7 @@ static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
>>  	.has_multi_master_mode = true,
>>  	.has_slcg_override_reg = true,
>>  	.has_mst_fifo = false,
>> +	.quirks = &tegra_i2c_quirks,
>>  };
>>  
>>  static const struct tegra_i2c_hw_feature tegra194_i2c_hw = { @@ 
>> -909,6 +921,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,
>> +	.quirks = &tegra194_i2c_quirks,
>>  };
>>  
>>  /* Match table for of_platform binding */ @@ -960,7 +973,6 @@ static 
>> int tegra_i2c_probe(struct platform_device *pdev)
>>  	i2c_dev->base = base;
>>  	i2c_dev->div_clk = div_clk;
>>  	i2c_dev->adapter.algo = &tegra_i2c_algo;
>> -	i2c_dev->adapter.quirks = &tegra_i2c_quirks;
>>  	i2c_dev->irq = irq;
>>  	i2c_dev->cont_id = pdev->id;
>>  	i2c_dev->dev = &pdev->dev;
>> @@ -976,6 +988,8 @@ static int tegra_i2c_probe(struct platform_device *pdev)
>>  	i2c_dev->hw = of_device_get_match_data(&pdev->dev);
>>  	i2c_dev->is_dvc = of_device_is_compatible(pdev->dev.of_node,
>>  						  "nvidia,tegra20-i2c-dvc");
>> +	i2c_dev->adapter.quirks = i2c_dev->hw->quirks;
>> +
>>  	init_completion(&i2c_dev->msg_complete);
>>  	spin_lock_init(&i2c_dev->xfer_lock);
>>  
>>