On 04/28/2018 04:56 PM, Alexander Monakov wrote:
Low-level controller enable function __i2c_dw_enable is overloaded to also handle disabling. What's worse, even though the documentation requires polling the IC_ENABLE_STATUS register when disabling, this is not done: polling needs to be requested specifically by calling __i2c_dw_enable_and_wait, which can also poll on enabling, but that doesn't work if the IC_ENABLE_STATUS register is not implemented. This is quite confusing if not in fact backwards. Especially since the documentation says that disabling should be followed by polling, the driver should be using a separate function where it does one-shot disables to make the optimization stand out. This refactors the two functions so that requested status is given in the name rather than in a boolean argument. Specifically: - __i2c_dw_enable: enable without polling (in accordance with docs) - __i2c_dw_disable: disable and do poll (also as suggested by docs) - __i2c_dw_disable_nowait: disable without polling (Linux-specific) No functional change. Cc: Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx> Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx> Signed-off-by: Alexander Monakov <amonakov@xxxxxxxxx> --- drivers/i2c/busses/i2c-designware-common.c | 20 +++++++++----------- drivers/i2c/busses/i2c-designware-core.h | 13 +++++++++++-- drivers/i2c/busses/i2c-designware-master.c | 8 ++++---- drivers/i2c/busses/i2c-designware-slave.c | 6 +++--- 4 files changed, 27 insertions(+), 20 deletions(-)
I'm neutral with this change. Acked-by: Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx>
