mbox series

[00/12] i2c-pxa cleanups

Message ID 20191215160444.GB25745@shell.armlinux.org.uk
Headers show
Series i2c-pxa cleanups | expand

Message

Russell King (Oracle) Dec. 15, 2019, 4:04 p.m. UTC
Hi,

This series cleans up the i2c-pxa code via the following changes:

1. replace i2c_pxa_addr_byte() with the functional equivalent
   i2c_8bit_addr_from_msg().

2. removing unnecessary headers, and rearranging those that remain
   in alphabetical order.

3. rearranging functions in the file to flow better; particularly
   placing the PIO specific functions next to the PIO algorithm
   structure, so all the PIO mode related code is together.  This
   eliminates the forward declaration of i2c_pxa_handler().

4. group the register bitfield definitions, which were split over two
   separate locations in the file, into a single location, and add
   some definitions for the IBMR register.

5. always set the 'fm' and 'hs' members for each hardware type; the
   storage for these members is always allocated, we don't need to
   bloat the code (neither runtime, nor in the source) for this.

6. move definitions private to i2c-pxa out of the platform data
   header; platforms have no business knowing these details.

7. group all driver-based IDs match (platform and OF) to one common
   location rather than at either end of the file.

8. fix i2c_pxa_scream_blue_murder()'s log output to be printed on a
   single line as it was intended, rather than being printed one
   entry per line - which makes it difficult to read particularly
   when it has been enabled and you're getting lots of them.  Also
   fix decode_bits() output in the same way.

9. fix i2c_pxa_wait_bus_not_busy() boundary condition, so that a
   coincidental success and timeout results in the function being
   successful rather than failing. (This has never been seen in
   practice, but was spotted while reviewing the code.)

All in all, these changes should have (and have had so far) no
observable impact on the driver; therefore, I do not see any reason
to backport any of these changes to stable trees.

 drivers/i2c/busses/i2c-pxa.c          | 595 ++++++++++++++++++----------------
 include/linux/platform_data/i2c-pxa.h |  48 ---
 2 files changed, 318 insertions(+), 325 deletions(-)

Comments

Wolfram Sang Jan. 6, 2020, 1:39 p.m. UTC | #1
Russell,

> All in all, these changes should have (and have had so far) no
> observable impact on the driver; therefore, I do not see any reason
> to backport any of these changes to stable trees.

Thanks for the detailed explanations. The series looks good to me. There
are some checkpatch reports about using "BIT()" and multiline comment
style, but I am not pedantic about these. Especially since the overall
readabililty of the driver definately gets improved.

Where Murphy's Law kicks in, though, is that after 2 years of silence,
we had a change in the pxa driver applied in Nov 2019. So, this series
doesn't apply on my for-next. Checking against v5.4, the merge conflicts
do not look super hard, but also not trivial, so I'd like to ask you if
you can rebase the series to v5.5-rc1 or later? I'd rather see this done
by someone who has more indepth experience with the driver and HW to
test.

Thanks,

   Wolfram