| Message ID | 1546465721-12108-1-git-send-email-skomatineni@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V1] i2c: tegra: Fix Maximum transfer size | expand |
On 2019-01-02 22:48, Sowjanya Komatineni wrote: > 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@nvidia.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? > }; > > /** > @@ -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? > + .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. 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); > >
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@nvidia.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); >> >>
On 2019-01-02 23:39, Sowjanya Komatineni wrote: >>> +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 *blush* I was comparing with old sources. Sorry! Cheers, Peter
Hi Sowjanya,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tegra/for-next]
[also build test WARNING on v4.20 next-20190102]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Sowjanya-Komatineni/i2c-tegra-Fix-Maximum-transfer-size/20190103-082727
base: https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git for-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm
All warnings (new ones prefixed by >>):
>> drivers/i2c/busses/i2c-tegra.c:839:18: warning: large integer implicitly truncated to unsigned type [-Woverflow]
.max_read_len = 65536,
^~~~~
drivers/i2c/busses/i2c-tegra.c:840:19: warning: large integer implicitly truncated to unsigned type [-Woverflow]
.max_write_len = 65536,
^~~~~
>> drivers/i2c/busses/i2c-tegra.c:854:12: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
.quirks = &tegra_i2c_quirks,
^
drivers/i2c/busses/i2c-tegra.c:868:12: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
.quirks = &tegra_i2c_quirks,
^
drivers/i2c/busses/i2c-tegra.c:882:12: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
.quirks = &tegra_i2c_quirks,
^
drivers/i2c/busses/i2c-tegra.c:896:12: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
.quirks = &tegra_i2c_quirks,
^
drivers/i2c/busses/i2c-tegra.c:910:12: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
.quirks = &tegra_i2c_quirks,
^
drivers/i2c/busses/i2c-tegra.c:924:12: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
.quirks = &tegra194_i2c_quirks,
^
vim +839 drivers/i2c/busses/i2c-tegra.c
836
837 static const struct i2c_adapter_quirks tegra194_i2c_quirks = {
838 .flags = I2C_AQ_NO_ZERO_LEN,
> 839 .max_read_len = 65536,
840 .max_write_len = 65536,
841 };
842
843 static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
844 .has_continue_xfer_support = false,
845 .has_per_pkt_xfer_complete_irq = false,
846 .has_single_clk_source = false,
847 .clk_divisor_hs_mode = 3,
848 .clk_divisor_std_fast_mode = 0,
849 .clk_divisor_fast_plus_mode = 0,
850 .has_config_load_reg = false,
851 .has_multi_master_mode = false,
852 .has_slcg_override_reg = false,
853 .has_mst_fifo = false,
> 854 .quirks = &tegra_i2c_quirks,
855 };
856
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
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; }; /** @@ -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, + .max_read_len = 65536, + .max_write_len = 65536, +}; + 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);
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@nvidia.com> --- drivers/i2c/busses/i2c-tegra.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)