mbox series

[v7,00/34] Improvements for Tegra I2C driver

Message ID 20200908224006.25636-1-digetx@gmail.com
Headers show
Series Improvements for Tegra I2C driver | expand

Message

Dmitry Osipenko Sept. 8, 2020, 10:39 p.m. UTC
Hello!

This series performs refactoring of the Tegra I2C driver code and hardens
the atomic-transfer mode.

Changelog:

v7: - Reworked the "Clean up probe function" patch by moving out all
      variable renamings into the "Clean up variable names" patch.
      This results in a nicer diff, which was asked by Andy Shevchenko.

    - Squashed "Improve coding style of tegra_i2c_wait_for_config_load()"
      patch into "Factor out register polling into separate function" in
      order avoid unnecessary ping-pong changes, which was asked by
      Andy Shevchenko.

    - Added more indentation improvements, it should be ideal now.

    - I haven't changed order of the "Clean up variable types" patch,
      which was suggested by Andy Shevchenko, because I already moved
      that patch multiple times and we decided to sort patches starting
      with more important cleanups and down to less important. The type
      changes are more important than shuffling code around, IMO.

v6: - Added new patch that adds missing RPM puts, thanks to Andy Shevchenko
      for the suggestion.

    - Improved commit messages by extending them with more a more detailed
      explanation of the changes.

    - Added clarifying comment to the "Use reset_control_reset()" change,
      which was asked by Andy Shevchenko.

    - Refactored the "Clean up probe function" patch by moving the
      dev_err_probe() change into the "Use clk-bulk helpers" patch,
      which was suggested by Andy Shevchenko.

    - Improved ordering of the patches like it was suggested by
      Andy Shevchenko.

    - Added Andy Shevchenko to suggested-by of the "Use clk-bulk helpers"
      patch.

    - Improved "Remove i2c_dev.clk_divisor_non_hs_mode member" patch by
      making the case-switch to use "fast plus mode" timing if clock rate
      is out-of-range. Just to make it more consistent.

    - The "Improve tegra_i2c_dev structure" patch is squashed into
     "Improve formatting of variables" and "Clean up types/names" patches.

    - All variable-renaming changes are squashed into a single "Clean up
      variable names" patch.

    - Made extra minor improvement to various patches, like more comments
      and indentations improved.

v5: - Dropped the "Factor out runtime PM and hardware initialization"
      patch, like it was suggested by Michał Mirosław. Instead a less
      invasive "Factor out hardware initialization into separate function"
      patch added, it doesn't touch the RPM initialization.

    - The "Remove outdated barrier()" patch now removes outdated comments.

    - Updated commit description of the "Remove "dma" variable" patch,
      saying that the transfer mode may be changed by a callee. This was
      suggested by Michał Mirosław.

    - Reworked the "Clean up and improve comments" patch. Couple more
      comments are corrected and reworded now.

    - Added r-b's from Michał Mirosław.

    - New patches:

        i2c: tegra: Mask interrupt in tegra_i2c_issue_bus_clear()
        i2c: tegra: Remove redundant check in tegra_i2c_issue_bus_clear()
        i2c: tegra: Don't fall back to PIO mode if DMA configuration fails
        i2c: tegra: Clean up variable types
        i2c: tegra: Improve tegra_i2c_dev structure

v4: - Reordered patches in the fixes/features/cleanups order like it was
      suggested by Andy Shevchenko.

    - Now using clk-bulk API, which was suggested by Andy Shevchenko.

    - Reworked "Make tegra_i2c_flush_fifos() usable in atomic transfer"
      patch to use iopoll API, which was suggested by Andy Shevchenko.

    - Separated "Clean up probe function" into several smaller patches.

    - Squashed "Add missing newline before returns" patch into
      "Clean up whitespaces, newlines and indentation".

    - The "Drop '_timeout' from wait/poll function names" is renamed to
      "Rename wait/poll functions".

    - The "Use reset_control_reset()" is changed to not fail tegra_i2c_init(),
      but only emit warning. This should be more friendly behaviour in oppose
      to having a non-bootable machine if reset-control fails.

    - New patches:

        i2c: tegra: Remove error message used for devm_request_irq() failure
        i2c: tegra: Use devm_platform_get_and_ioremap_resource()
        i2c: tegra: Use platform_get_irq()
        i2c: tegra: Use clk-bulk helpers
        i2c: tegra: Remove bogus barrier()
        i2c: tegra: Factor out register polling into separate function
        i2c: tegra: Consolidate error handling in tegra_i2c_xfer_msg()
        i2c: tegra: Clean up and improve comments
        i2c: tegra: Rename couple "ret" variables to "err"

v3: - Optimized "Make tegra_i2c_flush_fifos() usable in atomic transfer"
      patch by pre-checking FIFO state before starting to poll using
      ktime API, which may be expensive under some circumstances.

    - The "Clean up messages in the code" patch now makes all messages
      to use proper capitalization of abbreviations. Thanks to Andy Shevchenko
      and Michał Mirosław for the suggestion.

    - The "Remove unnecessary whitespaces and newlines" patch is transformed
      into "Clean up whitespaces and newlines", it now also adds missing
      newlines and spaces.

    - Reworked the "Clean up probe function" patch in accordance to
      suggestion from Michał Mirosław by factoring out only parts of
      the code that make error unwinding cleaner.

    - Added r-b from Michał Mirosław.

    - Added more patches:

        i2c: tegra: Reorder location of functions in the code
        i2c: tegra: Factor out packet header setup from tegra_i2c_xfer_msg()
        i2c: tegra: Remove "dma" variable
        i2c: tegra: Initialization div-clk rate unconditionally
        i2c: tegra: Remove i2c_dev.clk_divisor_non_hs_mode member

v2: - Cleaned more messages in the "Clean up messages in the code" patch.

    - The error code of reset_control_reset() is checked now.

    - Added these new patches to clean up couple more things:

        i2c: tegra: Check errors for both positive and negative values
        i2c: tegra: Improve coding style of tegra_i2c_wait_for_config_load()
        i2c: tegra: Remove unnecessary whitespaces and newlines
        i2c: tegra: Rename variable in tegra_i2c_issue_bus_clear()
        i2c: tegra: Improve driver module description

Dmitry Osipenko (34):
  i2c: tegra: Make tegra_i2c_flush_fifos() usable in atomic transfer
  i2c: tegra: Add missing pm_runtime_put()
  i2c: tegra: Handle potential error of tegra_i2c_flush_fifos()
  i2c: tegra: Mask interrupt in tegra_i2c_issue_bus_clear()
  i2c: tegra: Initialize div-clk rate unconditionally
  i2c: tegra: Remove i2c_dev.clk_divisor_non_hs_mode member
  i2c: tegra: Runtime PM always available on Tegra
  i2c: tegra: Remove error message used for devm_request_irq() failure
  i2c: tegra: Use reset_control_reset()
  i2c: tegra: Use devm_platform_get_and_ioremap_resource()
  i2c: tegra: Use platform_get_irq()
  i2c: tegra: Use clk-bulk helpers
  i2c: tegra: Move out all device-tree parsing into tegra_i2c_parse_dt()
  i2c: tegra: Clean up probe function
  i2c: tegra: Reorder location of functions in the code
  i2c: tegra: Clean up variable types
  i2c: tegra: Remove outdated barrier()
  i2c: tegra: Remove likely/unlikely from the code
  i2c: tegra: Remove redundant check in tegra_i2c_issue_bus_clear()
  i2c: tegra: Remove "dma" variable from tegra_i2c_xfer_msg()
  i2c: tegra: Don't fall back to PIO mode if DMA configuration fails
  i2c: tegra: Rename wait/poll functions
  i2c: tegra: Factor out error recovery from tegra_i2c_xfer_msg()
  i2c: tegra: Factor out packet header setup from tegra_i2c_xfer_msg()
  i2c: tegra: Factor out register polling into separate function
  i2c: tegra: Factor out hardware initialization into separate function
  i2c: tegra: Check errors for both positive and negative values
  i2c: tegra: Consolidate error handling in tegra_i2c_xfer_msg()
  i2c: tegra: Improve formatting of variables
  i2c: tegra: Clean up variable names
  i2c: tegra: Clean up printk messages
  i2c: tegra: Clean up and improve comments
  i2c: tegra: Clean up whitespaces, newlines and indentation
  i2c: tegra: Improve driver module description

 drivers/i2c/busses/i2c-tegra.c | 1435 ++++++++++++++++----------------
 1 file changed, 701 insertions(+), 734 deletions(-)

Comments

Andy Shevchenko Sept. 9, 2020, 9:11 a.m. UTC | #1
On Wed, Sep 9, 2020 at 1:40 AM Dmitry Osipenko <digetx@gmail.com> wrote:
>
> Hello!
>
> This series performs refactoring of the Tegra I2C driver code and hardens
> the atomic-transfer mode.

I think there is still room for improvement, but let not block it, FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Changelog:
>
> v7: - Reworked the "Clean up probe function" patch by moving out all
>       variable renamings into the "Clean up variable names" patch.
>       This results in a nicer diff, which was asked by Andy Shevchenko.
>
>     - Squashed "Improve coding style of tegra_i2c_wait_for_config_load()"
>       patch into "Factor out register polling into separate function" in
>       order avoid unnecessary ping-pong changes, which was asked by
>       Andy Shevchenko.
>
>     - Added more indentation improvements, it should be ideal now.
>
>     - I haven't changed order of the "Clean up variable types" patch,
>       which was suggested by Andy Shevchenko, because I already moved
>       that patch multiple times and we decided to sort patches starting
>       with more important cleanups and down to less important. The type
>       changes are more important than shuffling code around, IMO.
>
> v6: - Added new patch that adds missing RPM puts, thanks to Andy Shevchenko
>       for the suggestion.
>
>     - Improved commit messages by extending them with more a more detailed
>       explanation of the changes.
>
>     - Added clarifying comment to the "Use reset_control_reset()" change,
>       which was asked by Andy Shevchenko.
>
>     - Refactored the "Clean up probe function" patch by moving the
>       dev_err_probe() change into the "Use clk-bulk helpers" patch,
>       which was suggested by Andy Shevchenko.
>
>     - Improved ordering of the patches like it was suggested by
>       Andy Shevchenko.
>
>     - Added Andy Shevchenko to suggested-by of the "Use clk-bulk helpers"
>       patch.
>
>     - Improved "Remove i2c_dev.clk_divisor_non_hs_mode member" patch by
>       making the case-switch to use "fast plus mode" timing if clock rate
>       is out-of-range. Just to make it more consistent.
>
>     - The "Improve tegra_i2c_dev structure" patch is squashed into
>      "Improve formatting of variables" and "Clean up types/names" patches.
>
>     - All variable-renaming changes are squashed into a single "Clean up
>       variable names" patch.
>
>     - Made extra minor improvement to various patches, like more comments
>       and indentations improved.
>
> v5: - Dropped the "Factor out runtime PM and hardware initialization"
>       patch, like it was suggested by Michał Mirosław. Instead a less
>       invasive "Factor out hardware initialization into separate function"
>       patch added, it doesn't touch the RPM initialization.
>
>     - The "Remove outdated barrier()" patch now removes outdated comments.
>
>     - Updated commit description of the "Remove "dma" variable" patch,
>       saying that the transfer mode may be changed by a callee. This was
>       suggested by Michał Mirosław.
>
>     - Reworked the "Clean up and improve comments" patch. Couple more
>       comments are corrected and reworded now.
>
>     - Added r-b's from Michał Mirosław.
>
>     - New patches:
>
>         i2c: tegra: Mask interrupt in tegra_i2c_issue_bus_clear()
>         i2c: tegra: Remove redundant check in tegra_i2c_issue_bus_clear()
>         i2c: tegra: Don't fall back to PIO mode if DMA configuration fails
>         i2c: tegra: Clean up variable types
>         i2c: tegra: Improve tegra_i2c_dev structure
>
> v4: - Reordered patches in the fixes/features/cleanups order like it was
>       suggested by Andy Shevchenko.
>
>     - Now using clk-bulk API, which was suggested by Andy Shevchenko.
>
>     - Reworked "Make tegra_i2c_flush_fifos() usable in atomic transfer"
>       patch to use iopoll API, which was suggested by Andy Shevchenko.
>
>     - Separated "Clean up probe function" into several smaller patches.
>
>     - Squashed "Add missing newline before returns" patch into
>       "Clean up whitespaces, newlines and indentation".
>
>     - The "Drop '_timeout' from wait/poll function names" is renamed to
>       "Rename wait/poll functions".
>
>     - The "Use reset_control_reset()" is changed to not fail tegra_i2c_init(),
>       but only emit warning. This should be more friendly behaviour in oppose
>       to having a non-bootable machine if reset-control fails.
>
>     - New patches:
>
>         i2c: tegra: Remove error message used for devm_request_irq() failure
>         i2c: tegra: Use devm_platform_get_and_ioremap_resource()
>         i2c: tegra: Use platform_get_irq()
>         i2c: tegra: Use clk-bulk helpers
>         i2c: tegra: Remove bogus barrier()
>         i2c: tegra: Factor out register polling into separate function
>         i2c: tegra: Consolidate error handling in tegra_i2c_xfer_msg()
>         i2c: tegra: Clean up and improve comments
>         i2c: tegra: Rename couple "ret" variables to "err"
>
> v3: - Optimized "Make tegra_i2c_flush_fifos() usable in atomic transfer"
>       patch by pre-checking FIFO state before starting to poll using
>       ktime API, which may be expensive under some circumstances.
>
>     - The "Clean up messages in the code" patch now makes all messages
>       to use proper capitalization of abbreviations. Thanks to Andy Shevchenko
>       and Michał Mirosław for the suggestion.
>
>     - The "Remove unnecessary whitespaces and newlines" patch is transformed
>       into "Clean up whitespaces and newlines", it now also adds missing
>       newlines and spaces.
>
>     - Reworked the "Clean up probe function" patch in accordance to
>       suggestion from Michał Mirosław by factoring out only parts of
>       the code that make error unwinding cleaner.
>
>     - Added r-b from Michał Mirosław.
>
>     - Added more patches:
>
>         i2c: tegra: Reorder location of functions in the code
>         i2c: tegra: Factor out packet header setup from tegra_i2c_xfer_msg()
>         i2c: tegra: Remove "dma" variable
>         i2c: tegra: Initialization div-clk rate unconditionally
>         i2c: tegra: Remove i2c_dev.clk_divisor_non_hs_mode member
>
> v2: - Cleaned more messages in the "Clean up messages in the code" patch.
>
>     - The error code of reset_control_reset() is checked now.
>
>     - Added these new patches to clean up couple more things:
>
>         i2c: tegra: Check errors for both positive and negative values
>         i2c: tegra: Improve coding style of tegra_i2c_wait_for_config_load()
>         i2c: tegra: Remove unnecessary whitespaces and newlines
>         i2c: tegra: Rename variable in tegra_i2c_issue_bus_clear()
>         i2c: tegra: Improve driver module description
>
> Dmitry Osipenko (34):
>   i2c: tegra: Make tegra_i2c_flush_fifos() usable in atomic transfer
>   i2c: tegra: Add missing pm_runtime_put()
>   i2c: tegra: Handle potential error of tegra_i2c_flush_fifos()
>   i2c: tegra: Mask interrupt in tegra_i2c_issue_bus_clear()
>   i2c: tegra: Initialize div-clk rate unconditionally
>   i2c: tegra: Remove i2c_dev.clk_divisor_non_hs_mode member
>   i2c: tegra: Runtime PM always available on Tegra
>   i2c: tegra: Remove error message used for devm_request_irq() failure
>   i2c: tegra: Use reset_control_reset()
>   i2c: tegra: Use devm_platform_get_and_ioremap_resource()
>   i2c: tegra: Use platform_get_irq()
>   i2c: tegra: Use clk-bulk helpers
>   i2c: tegra: Move out all device-tree parsing into tegra_i2c_parse_dt()
>   i2c: tegra: Clean up probe function
>   i2c: tegra: Reorder location of functions in the code
>   i2c: tegra: Clean up variable types
>   i2c: tegra: Remove outdated barrier()
>   i2c: tegra: Remove likely/unlikely from the code
>   i2c: tegra: Remove redundant check in tegra_i2c_issue_bus_clear()
>   i2c: tegra: Remove "dma" variable from tegra_i2c_xfer_msg()
>   i2c: tegra: Don't fall back to PIO mode if DMA configuration fails
>   i2c: tegra: Rename wait/poll functions
>   i2c: tegra: Factor out error recovery from tegra_i2c_xfer_msg()
>   i2c: tegra: Factor out packet header setup from tegra_i2c_xfer_msg()
>   i2c: tegra: Factor out register polling into separate function
>   i2c: tegra: Factor out hardware initialization into separate function
>   i2c: tegra: Check errors for both positive and negative values
>   i2c: tegra: Consolidate error handling in tegra_i2c_xfer_msg()
>   i2c: tegra: Improve formatting of variables
>   i2c: tegra: Clean up variable names
>   i2c: tegra: Clean up printk messages
>   i2c: tegra: Clean up and improve comments
>   i2c: tegra: Clean up whitespaces, newlines and indentation
>   i2c: tegra: Improve driver module description
>
>  drivers/i2c/busses/i2c-tegra.c | 1435 ++++++++++++++++----------------
>  1 file changed, 701 insertions(+), 734 deletions(-)
>
> --
> 2.27.0
>
Dmitry Osipenko Sept. 9, 2020, 3:36 p.m. UTC | #2
09.09.2020 12:11, Andy Shevchenko пишет:
> On Wed, Sep 9, 2020 at 1:40 AM Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> Hello!
>>
>> This series performs refactoring of the Tegra I2C driver code and hardens
>> the atomic-transfer mode.
> 
> I think there is still room for improvement, but let not block it, FWIW,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Thank you and Michał for helping with the review! Very appreciate this!
Wolfram Sang Sept. 9, 2020, 3:49 p.m. UTC | #3
On Wed, Sep 09, 2020 at 06:36:50PM +0300, Dmitry Osipenko wrote:
> 09.09.2020 12:11, Andy Shevchenko пишет:
> > On Wed, Sep 9, 2020 at 1:40 AM Dmitry Osipenko <digetx@gmail.com> wrote:
> >>
> >> Hello!
> >>
> >> This series performs refactoring of the Tegra I2C driver code and hardens
> >> the atomic-transfer mode.
> > 
> > I think there is still room for improvement, but let not block it, FWIW,
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> Thank you and Michał for helping with the review! Very appreciate this!

Yes, thanks everyone so far!

Is there some internal testfarm where this should be regression tested?
Otherwise, I'd trust Dmitry, Andy, and Michał here and would apply it
this week after some generic high-level review.
Dmitry Osipenko Sept. 9, 2020, 5:39 p.m. UTC | #4
09.09.2020 18:49, Wolfram Sang пишет:
> On Wed, Sep 09, 2020 at 06:36:50PM +0300, Dmitry Osipenko wrote:
>> 09.09.2020 12:11, Andy Shevchenko пишет:
>>> On Wed, Sep 9, 2020 at 1:40 AM Dmitry Osipenko <digetx@gmail.com> wrote:
>>>>
>>>> Hello!
>>>>
>>>> This series performs refactoring of the Tegra I2C driver code and hardens
>>>> the atomic-transfer mode.
>>>
>>> I think there is still room for improvement, but let not block it, FWIW,
>>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>
>> Thank you and Michał for helping with the review! Very appreciate this!
> 
> Yes, thanks everyone so far!
> 
> Is there some internal testfarm where this should be regression tested?
> Otherwise, I'd trust Dmitry, Andy, and Michał here and would apply it
> this week after some generic high-level review.
> 

Thierry and Jon, would be awesome if you could give this series a test
on T210+. Thanks in advance!
Thierry Reding Sept. 17, 2020, 12:44 p.m. UTC | #5
On Wed, Sep 09, 2020 at 05:49:02PM +0200, Wolfram Sang wrote:
> On Wed, Sep 09, 2020 at 06:36:50PM +0300, Dmitry Osipenko wrote:
> > 09.09.2020 12:11, Andy Shevchenko пишет:
> > > On Wed, Sep 9, 2020 at 1:40 AM Dmitry Osipenko <digetx@gmail.com> wrote:
> > >>
> > >> Hello!
> > >>
> > >> This series performs refactoring of the Tegra I2C driver code and hardens
> > >> the atomic-transfer mode.
> > > 
> > > I think there is still room for improvement, but let not block it, FWIW,
> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > 
> > Thank you and Michał for helping with the review! Very appreciate this!
> 
> Yes, thanks everyone so far!
> 
> Is there some internal testfarm where this should be regression tested?
> Otherwise, I'd trust Dmitry, Andy, and Michał here and would apply it
> this week after some generic high-level review.

I'll queue this for a run on the test farm. I had a couple of minor
comments, but after going through the full series I'm pretty happy
overall with the result, so I'll go over my comments again and will
reevaluate.

Thierry
Wolfram Sang Sept. 21, 2020, 9:12 a.m. UTC | #6
On Thu, Sep 17, 2020 at 02:44:18PM +0200, Thierry Reding wrote:
> On Wed, Sep 09, 2020 at 05:49:02PM +0200, Wolfram Sang wrote:
> > On Wed, Sep 09, 2020 at 06:36:50PM +0300, Dmitry Osipenko wrote:
> > > 09.09.2020 12:11, Andy Shevchenko пишет:
> > > > On Wed, Sep 9, 2020 at 1:40 AM Dmitry Osipenko <digetx@gmail.com> wrote:
> > > >>
> > > >> Hello!
> > > >>
> > > >> This series performs refactoring of the Tegra I2C driver code and hardens
> > > >> the atomic-transfer mode.
> > > > 
> > > > I think there is still room for improvement, but let not block it, FWIW,
> > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > 
> > > Thank you and Michał for helping with the review! Very appreciate this!
> > 
> > Yes, thanks everyone so far!
> > 
> > Is there some internal testfarm where this should be regression tested?
> > Otherwise, I'd trust Dmitry, Andy, and Michał here and would apply it
> > this week after some generic high-level review.
> 
> I'll queue this for a run on the test farm. I had a couple of minor
> comments, but after going through the full series I'm pretty happy
> overall with the result, so I'll go over my comments again and will
> reevaluate.

Cool, thanks! You guys just let me know if v7 is fine please. Otherwise
I will surely notice if a v8 hits the list ;)
Thierry Reding Sept. 21, 2020, 10:18 a.m. UTC | #7
On Wed, 09 Sep 2020 01:39:32 +0300, Dmitry Osipenko wrote:
> Hello!
> 
> This series performs refactoring of the Tegra I2C driver code and hardens
> the atomic-transfer mode.
> 
> Changelog:
> 
> v7: - Reworked the "Clean up probe function" patch by moving out all
>       variable renamings into the "Clean up variable names" patch.
>       This results in a nicer diff, which was asked by Andy Shevchenko.
> 
>     - Squashed "Improve coding style of tegra_i2c_wait_for_config_load()"
>       patch into "Factor out register polling into separate function" in
>       order avoid unnecessary ping-pong changes, which was asked by
>       Andy Shevchenko.
> 
>     - Added more indentation improvements, it should be ideal now.
> 
>     - I haven't changed order of the "Clean up variable types" patch,
>       which was suggested by Andy Shevchenko, because I already moved
>       that patch multiple times and we decided to sort patches starting
>       with more important cleanups and down to less important. The type
>       changes are more important than shuffling code around, IMO.
> 
> v6: - Added new patch that adds missing RPM puts, thanks to Andy Shevchenko
>       for the suggestion.
> 
>     - Improved commit messages by extending them with more a more detailed
>       explanation of the changes.
> 
>     - Added clarifying comment to the "Use reset_control_reset()" change,
>       which was asked by Andy Shevchenko.
> 
>     - Refactored the "Clean up probe function" patch by moving the
>       dev_err_probe() change into the "Use clk-bulk helpers" patch,
>       which was suggested by Andy Shevchenko.
> 
>     - Improved ordering of the patches like it was suggested by
>       Andy Shevchenko.
> 
>     - Added Andy Shevchenko to suggested-by of the "Use clk-bulk helpers"
>       patch.
> 
>     - Improved "Remove i2c_dev.clk_divisor_non_hs_mode member" patch by
>       making the case-switch to use "fast plus mode" timing if clock rate
>       is out-of-range. Just to make it more consistent.
> 
>     - The "Improve tegra_i2c_dev structure" patch is squashed into
>      "Improve formatting of variables" and "Clean up types/names" patches.
> 
>     - All variable-renaming changes are squashed into a single "Clean up
>       variable names" patch.
> 
>     - Made extra minor improvement to various patches, like more comments
>       and indentations improved.
> 
> v5: - Dropped the "Factor out runtime PM and hardware initialization"
>       patch, like it was suggested by Michał Mirosław. Instead a less
>       invasive "Factor out hardware initialization into separate function"
>       patch added, it doesn't touch the RPM initialization.
> 
>     - The "Remove outdated barrier()" patch now removes outdated comments.
> 
>     - Updated commit description of the "Remove "dma" variable" patch,
>       saying that the transfer mode may be changed by a callee. This was
>       suggested by Michał Mirosław.
> 
>     - Reworked the "Clean up and improve comments" patch. Couple more
>       comments are corrected and reworded now.
> 
>     - Added r-b's from Michał Mirosław.
> 
>     - New patches:
> 
>         i2c: tegra: Mask interrupt in tegra_i2c_issue_bus_clear()
>         i2c: tegra: Remove redundant check in tegra_i2c_issue_bus_clear()
>         i2c: tegra: Don't fall back to PIO mode if DMA configuration fails
>         i2c: tegra: Clean up variable types
>         i2c: tegra: Improve tegra_i2c_dev structure
> 
> v4: - Reordered patches in the fixes/features/cleanups order like it was
>       suggested by Andy Shevchenko.
> 
>     - Now using clk-bulk API, which was suggested by Andy Shevchenko.
> 
>     - Reworked "Make tegra_i2c_flush_fifos() usable in atomic transfer"
>       patch to use iopoll API, which was suggested by Andy Shevchenko.
> 
>     - Separated "Clean up probe function" into several smaller patches.
> 
>     - Squashed "Add missing newline before returns" patch into
>       "Clean up whitespaces, newlines and indentation".
> 
>     - The "Drop '_timeout' from wait/poll function names" is renamed to
>       "Rename wait/poll functions".
> 
>     - The "Use reset_control_reset()" is changed to not fail tegra_i2c_init(),
>       but only emit warning. This should be more friendly behaviour in oppose
>       to having a non-bootable machine if reset-control fails.
> 
>     - New patches:
> 
>         i2c: tegra: Remove error message used for devm_request_irq() failure
>         i2c: tegra: Use devm_platform_get_and_ioremap_resource()
>         i2c: tegra: Use platform_get_irq()
>         i2c: tegra: Use clk-bulk helpers
>         i2c: tegra: Remove bogus barrier()
>         i2c: tegra: Factor out register polling into separate function
>         i2c: tegra: Consolidate error handling in tegra_i2c_xfer_msg()
>         i2c: tegra: Clean up and improve comments
>         i2c: tegra: Rename couple "ret" variables to "err"
> 
> v3: - Optimized "Make tegra_i2c_flush_fifos() usable in atomic transfer"
>       patch by pre-checking FIFO state before starting to poll using
>       ktime API, which may be expensive under some circumstances.
> 
>     - The "Clean up messages in the code" patch now makes all messages
>       to use proper capitalization of abbreviations. Thanks to Andy Shevchenko
>       and Michał Mirosław for the suggestion.
> 
>     - The "Remove unnecessary whitespaces and newlines" patch is transformed
>       into "Clean up whitespaces and newlines", it now also adds missing
>       newlines and spaces.
> 
>     - Reworked the "Clean up probe function" patch in accordance to
>       suggestion from Michał Mirosław by factoring out only parts of
>       the code that make error unwinding cleaner.
> 
>     - Added r-b from Michał Mirosław.
> 
>     - Added more patches:
> 
>         i2c: tegra: Reorder location of functions in the code
>         i2c: tegra: Factor out packet header setup from tegra_i2c_xfer_msg()
>         i2c: tegra: Remove "dma" variable
>         i2c: tegra: Initialization div-clk rate unconditionally
>         i2c: tegra: Remove i2c_dev.clk_divisor_non_hs_mode member
> 
> v2: - Cleaned more messages in the "Clean up messages in the code" patch.
> 
>     - The error code of reset_control_reset() is checked now.
> 
>     - Added these new patches to clean up couple more things:
> 
>         i2c: tegra: Check errors for both positive and negative values
>         i2c: tegra: Improve coding style of tegra_i2c_wait_for_config_load()
>         i2c: tegra: Remove unnecessary whitespaces and newlines
>         i2c: tegra: Rename variable in tegra_i2c_issue_bus_clear()
>         i2c: tegra: Improve driver module description
> 
> Dmitry Osipenko (34):
>   i2c: tegra: Make tegra_i2c_flush_fifos() usable in atomic transfer
>   i2c: tegra: Add missing pm_runtime_put()
>   i2c: tegra: Handle potential error of tegra_i2c_flush_fifos()
>   i2c: tegra: Mask interrupt in tegra_i2c_issue_bus_clear()
>   i2c: tegra: Initialize div-clk rate unconditionally
>   i2c: tegra: Remove i2c_dev.clk_divisor_non_hs_mode member
>   i2c: tegra: Runtime PM always available on Tegra
>   i2c: tegra: Remove error message used for devm_request_irq() failure
>   i2c: tegra: Use reset_control_reset()
>   i2c: tegra: Use devm_platform_get_and_ioremap_resource()
>   i2c: tegra: Use platform_get_irq()
>   i2c: tegra: Use clk-bulk helpers
>   i2c: tegra: Move out all device-tree parsing into tegra_i2c_parse_dt()
>   i2c: tegra: Clean up probe function
>   i2c: tegra: Reorder location of functions in the code
>   i2c: tegra: Clean up variable types
>   i2c: tegra: Remove outdated barrier()
>   i2c: tegra: Remove likely/unlikely from the code
>   i2c: tegra: Remove redundant check in tegra_i2c_issue_bus_clear()
>   i2c: tegra: Remove "dma" variable from tegra_i2c_xfer_msg()
>   i2c: tegra: Don't fall back to PIO mode if DMA configuration fails
>   i2c: tegra: Rename wait/poll functions
>   i2c: tegra: Factor out error recovery from tegra_i2c_xfer_msg()
>   i2c: tegra: Factor out packet header setup from tegra_i2c_xfer_msg()
>   i2c: tegra: Factor out register polling into separate function
>   i2c: tegra: Factor out hardware initialization into separate function
>   i2c: tegra: Check errors for both positive and negative values
>   i2c: tegra: Consolidate error handling in tegra_i2c_xfer_msg()
>   i2c: tegra: Improve formatting of variables
>   i2c: tegra: Clean up variable names
>   i2c: tegra: Clean up printk messages
>   i2c: tegra: Clean up and improve comments
>   i2c: tegra: Clean up whitespaces, newlines and indentation
>   i2c: tegra: Improve driver module description
> 
>  drivers/i2c/busses/i2c-tegra.c | 1435 ++++++++++++++++----------------
>  1 file changed, 701 insertions(+), 734 deletions(-)
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Test results:
  14 builds: 14 pass, 0 fail
   9 boots:  9 pass, 0 fail
  47 tests:  47 pass, 0 fail

Boards tested: tegra20-ventana, tegra30-cardhu-a04, tegra124-jetson-tk1,
               tegra186-p2771-0000, tegra194-p2972-0000
Thierry Reding Sept. 21, 2020, 10:42 a.m. UTC | #8
On Mon, Sep 21, 2020 at 10:18:27AM +0000, Thierry Reding wrote:
> On Wed, 09 Sep 2020 01:39:32 +0300, Dmitry Osipenko wrote:
> > Hello!
> > 
> > This series performs refactoring of the Tegra I2C driver code and hardens
> > the atomic-transfer mode.
> > 
> > Changelog:
> > 
> > v7: - Reworked the "Clean up probe function" patch by moving out all
> >       variable renamings into the "Clean up variable names" patch.
> >       This results in a nicer diff, which was asked by Andy Shevchenko.
> > 
> >     - Squashed "Improve coding style of tegra_i2c_wait_for_config_load()"
> >       patch into "Factor out register polling into separate function" in
> >       order avoid unnecessary ping-pong changes, which was asked by
> >       Andy Shevchenko.
> > 
> >     - Added more indentation improvements, it should be ideal now.
> > 
> >     - I haven't changed order of the "Clean up variable types" patch,
> >       which was suggested by Andy Shevchenko, because I already moved
> >       that patch multiple times and we decided to sort patches starting
> >       with more important cleanups and down to less important. The type
> >       changes are more important than shuffling code around, IMO.
> > 
> > v6: - Added new patch that adds missing RPM puts, thanks to Andy Shevchenko
> >       for the suggestion.
> > 
> >     - Improved commit messages by extending them with more a more detailed
> >       explanation of the changes.
> > 
> >     - Added clarifying comment to the "Use reset_control_reset()" change,
> >       which was asked by Andy Shevchenko.
> > 
> >     - Refactored the "Clean up probe function" patch by moving the
> >       dev_err_probe() change into the "Use clk-bulk helpers" patch,
> >       which was suggested by Andy Shevchenko.
> > 
> >     - Improved ordering of the patches like it was suggested by
> >       Andy Shevchenko.
> > 
> >     - Added Andy Shevchenko to suggested-by of the "Use clk-bulk helpers"
> >       patch.
> > 
> >     - Improved "Remove i2c_dev.clk_divisor_non_hs_mode member" patch by
> >       making the case-switch to use "fast plus mode" timing if clock rate
> >       is out-of-range. Just to make it more consistent.
> > 
> >     - The "Improve tegra_i2c_dev structure" patch is squashed into
> >      "Improve formatting of variables" and "Clean up types/names" patches.
> > 
> >     - All variable-renaming changes are squashed into a single "Clean up
> >       variable names" patch.
> > 
> >     - Made extra minor improvement to various patches, like more comments
> >       and indentations improved.
> > 
> > v5: - Dropped the "Factor out runtime PM and hardware initialization"
> >       patch, like it was suggested by Michał Mirosław. Instead a less
> >       invasive "Factor out hardware initialization into separate function"
> >       patch added, it doesn't touch the RPM initialization.
> > 
> >     - The "Remove outdated barrier()" patch now removes outdated comments.
> > 
> >     - Updated commit description of the "Remove "dma" variable" patch,
> >       saying that the transfer mode may be changed by a callee. This was
> >       suggested by Michał Mirosław.
> > 
> >     - Reworked the "Clean up and improve comments" patch. Couple more
> >       comments are corrected and reworded now.
> > 
> >     - Added r-b's from Michał Mirosław.
> > 
> >     - New patches:
> > 
> >         i2c: tegra: Mask interrupt in tegra_i2c_issue_bus_clear()
> >         i2c: tegra: Remove redundant check in tegra_i2c_issue_bus_clear()
> >         i2c: tegra: Don't fall back to PIO mode if DMA configuration fails
> >         i2c: tegra: Clean up variable types
> >         i2c: tegra: Improve tegra_i2c_dev structure
> > 
> > v4: - Reordered patches in the fixes/features/cleanups order like it was
> >       suggested by Andy Shevchenko.
> > 
> >     - Now using clk-bulk API, which was suggested by Andy Shevchenko.
> > 
> >     - Reworked "Make tegra_i2c_flush_fifos() usable in atomic transfer"
> >       patch to use iopoll API, which was suggested by Andy Shevchenko.
> > 
> >     - Separated "Clean up probe function" into several smaller patches.
> > 
> >     - Squashed "Add missing newline before returns" patch into
> >       "Clean up whitespaces, newlines and indentation".
> > 
> >     - The "Drop '_timeout' from wait/poll function names" is renamed to
> >       "Rename wait/poll functions".
> > 
> >     - The "Use reset_control_reset()" is changed to not fail tegra_i2c_init(),
> >       but only emit warning. This should be more friendly behaviour in oppose
> >       to having a non-bootable machine if reset-control fails.
> > 
> >     - New patches:
> > 
> >         i2c: tegra: Remove error message used for devm_request_irq() failure
> >         i2c: tegra: Use devm_platform_get_and_ioremap_resource()
> >         i2c: tegra: Use platform_get_irq()
> >         i2c: tegra: Use clk-bulk helpers
> >         i2c: tegra: Remove bogus barrier()
> >         i2c: tegra: Factor out register polling into separate function
> >         i2c: tegra: Consolidate error handling in tegra_i2c_xfer_msg()
> >         i2c: tegra: Clean up and improve comments
> >         i2c: tegra: Rename couple "ret" variables to "err"
> > 
> > v3: - Optimized "Make tegra_i2c_flush_fifos() usable in atomic transfer"
> >       patch by pre-checking FIFO state before starting to poll using
> >       ktime API, which may be expensive under some circumstances.
> > 
> >     - The "Clean up messages in the code" patch now makes all messages
> >       to use proper capitalization of abbreviations. Thanks to Andy Shevchenko
> >       and Michał Mirosław for the suggestion.
> > 
> >     - The "Remove unnecessary whitespaces and newlines" patch is transformed
> >       into "Clean up whitespaces and newlines", it now also adds missing
> >       newlines and spaces.
> > 
> >     - Reworked the "Clean up probe function" patch in accordance to
> >       suggestion from Michał Mirosław by factoring out only parts of
> >       the code that make error unwinding cleaner.
> > 
> >     - Added r-b from Michał Mirosław.
> > 
> >     - Added more patches:
> > 
> >         i2c: tegra: Reorder location of functions in the code
> >         i2c: tegra: Factor out packet header setup from tegra_i2c_xfer_msg()
> >         i2c: tegra: Remove "dma" variable
> >         i2c: tegra: Initialization div-clk rate unconditionally
> >         i2c: tegra: Remove i2c_dev.clk_divisor_non_hs_mode member
> > 
> > v2: - Cleaned more messages in the "Clean up messages in the code" patch.
> > 
> >     - The error code of reset_control_reset() is checked now.
> > 
> >     - Added these new patches to clean up couple more things:
> > 
> >         i2c: tegra: Check errors for both positive and negative values
> >         i2c: tegra: Improve coding style of tegra_i2c_wait_for_config_load()
> >         i2c: tegra: Remove unnecessary whitespaces and newlines
> >         i2c: tegra: Rename variable in tegra_i2c_issue_bus_clear()
> >         i2c: tegra: Improve driver module description
> > 
> > Dmitry Osipenko (34):
> >   i2c: tegra: Make tegra_i2c_flush_fifos() usable in atomic transfer
> >   i2c: tegra: Add missing pm_runtime_put()
> >   i2c: tegra: Handle potential error of tegra_i2c_flush_fifos()
> >   i2c: tegra: Mask interrupt in tegra_i2c_issue_bus_clear()
> >   i2c: tegra: Initialize div-clk rate unconditionally
> >   i2c: tegra: Remove i2c_dev.clk_divisor_non_hs_mode member
> >   i2c: tegra: Runtime PM always available on Tegra
> >   i2c: tegra: Remove error message used for devm_request_irq() failure
> >   i2c: tegra: Use reset_control_reset()
> >   i2c: tegra: Use devm_platform_get_and_ioremap_resource()
> >   i2c: tegra: Use platform_get_irq()
> >   i2c: tegra: Use clk-bulk helpers
> >   i2c: tegra: Move out all device-tree parsing into tegra_i2c_parse_dt()
> >   i2c: tegra: Clean up probe function
> >   i2c: tegra: Reorder location of functions in the code
> >   i2c: tegra: Clean up variable types
> >   i2c: tegra: Remove outdated barrier()
> >   i2c: tegra: Remove likely/unlikely from the code
> >   i2c: tegra: Remove redundant check in tegra_i2c_issue_bus_clear()
> >   i2c: tegra: Remove "dma" variable from tegra_i2c_xfer_msg()
> >   i2c: tegra: Don't fall back to PIO mode if DMA configuration fails
> >   i2c: tegra: Rename wait/poll functions
> >   i2c: tegra: Factor out error recovery from tegra_i2c_xfer_msg()
> >   i2c: tegra: Factor out packet header setup from tegra_i2c_xfer_msg()
> >   i2c: tegra: Factor out register polling into separate function
> >   i2c: tegra: Factor out hardware initialization into separate function
> >   i2c: tegra: Check errors for both positive and negative values
> >   i2c: tegra: Consolidate error handling in tegra_i2c_xfer_msg()
> >   i2c: tegra: Improve formatting of variables
> >   i2c: tegra: Clean up variable names
> >   i2c: tegra: Clean up printk messages
> >   i2c: tegra: Clean up and improve comments
> >   i2c: tegra: Clean up whitespaces, newlines and indentation
> >   i2c: tegra: Improve driver module description
> > 
> >  drivers/i2c/busses/i2c-tegra.c | 1435 ++++++++++++++++----------------
> >  1 file changed, 701 insertions(+), 734 deletions(-)
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Hm... not sure how this ended up here. My reporting script is parsing
the mailbox from patchwork, so perhaps this is patchwork injecting any
stored tags into the mailbox?

> Test results:
>   14 builds: 14 pass, 0 fail
>    9 boots:  9 pass, 0 fail
>   47 tests:  47 pass, 0 fail
> 
> Boards tested: tegra20-ventana, tegra30-cardhu-a04, tegra124-jetson-tk1,
>                tegra186-p2771-0000, tegra194-p2972-0000

Looks like something went wrong here. Some additional boards were
tested, but the reporting script seems to have failed to retrieve
some of the logs and then decided not to include the boards here.

Anyway, I don't see any failures with this set of patches applied
so I think it's all good.

Thierry