| Message ID | 20200903005300.7894-1-digetx@gmail.com |
|---|---|
| Headers | show |
| Series | Improvements for Tegra I2C driver | expand |
On Thu, Sep 3, 2020 at 3:53 AM Dmitry Osipenko <digetx@gmail.com> wrote: > > Use a single reset_control_reset() instead of assert/deasset couple in > order to make code cleaner a tad. > Note that the reset_control_reset() > uses 1 microsecond delay instead of 2 that was used previously, but this > shouldn't matter. What datasheet says about this delay? > In addition don't ignore potential error of the reset. > > Reviewed-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
03.09.2020 14:06, Andy Shevchenko пишет: > On Thu, Sep 3, 2020 at 3:53 AM Dmitry Osipenko <digetx@gmail.com> wrote: >> >> This patch unifies style of all messages in the driver by starting them >> with a lowercase letter and using consistent capitalization and wording >> for all messages. > > I didn't look at the rest (yet) but this series has a patch ordering issue. > Why do we first do some little, non-critical clean ups? > > The preferred way is to arrange like: > - fixes that may be backported > - fixes that are likely not going to be backported > - features > - cleanups > > In its turn cleanups go by severity: > - code affected ones (and maybe logical changers) > - ... > - commentary / indentation fixes > That's a good suggestion! All patches in this version are ordered by the time they were created ans since none of these patches should be worthwhile to backport (IMO) and because majority of patches do minor changes, it appeared to me that it should be okay as-is. I agree that it should be worthwhile to reorder the patches now, after the series grew up a tad in regards to amount of patches. I'll change the order in the next version, thanks!
03.09.2020 14:02, Andy Shevchenko пишет: > On Thu, Sep 3, 2020 at 3:53 AM Dmitry Osipenko <digetx@gmail.com> wrote: >> >> The tegra_i2c_flush_fifos() shouldn't sleep in atomic transfer and jiffies >> are not updating if interrupts are disabled. Hence let's use proper delay >> functions and use ktime API in order not to hang atomic transfer. Note >> that this patch doesn't fix any known problem because normally FIFO is >> flushed at the time of starting a new transfer. > >> + /* >> + * ktime_get() may take up to couple milliseconds in a worst case >> + * and normally FIFOs are flushed, hence let's check the state before >> + * proceeding to polling. >> + */ > > Everything, including above can be done by using macros from iopoll.h. Why not? Perhaps indeed it should be possible to use the common macros, at least I can't recall why I chose not to use them. Maybe because it appeared to me that the current variant is a bit nicer than: if (atomic) read_poll_atomic() else read_poll() I'll consider to use the common iopoll macros in v4, thanks!
Hello! This series performs a small refactoring of the Tegra I2C driver code and hardens the atomic-transfer mode. Changelog: 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 (22): i2c: tegra: Make tegra_i2c_flush_fifos() usable in atomic transfer i2c: tegra: Add missing newline before returns i2c: tegra: Clean up messages in the code i2c: tegra: Don't ignore tegra_i2c_flush_fifos() error i2c: tegra: Use reset_control_reset() i2c: tegra: Improve formatting of function variables i2c: tegra: Use dev_err_probe() i2c: tegra: Runtime PM always available on Tegra i2c: tegra: Clean up probe function i2c: tegra: Drop '_timeout' from wait/poll function names i2c: tegra: Remove likely/unlikely from the code i2c: tegra: Factor out error recovery from tegra_i2c_xfer_msg() i2c: tegra: Check errors for both positive and negative values i2c: tegra: Improve coding style of tegra_i2c_wait_for_config_load() i2c: tegra: Clean up whitespaces and newlines i2c: tegra: Rename variable in tegra_i2c_issue_bus_clear() i2c: tegra: Improve driver module description 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 drivers/i2c/busses/i2c-tegra.c | 1327 ++++++++++++++++---------------- 1 file changed, 684 insertions(+), 643 deletions(-)