Hi Wolfram,
On Thu, Nov 2, 2017 at 1:47 PM, Wolfram Sang
<[email protected]> wrote:
> Those two functions are very short and only called once. The code
> becomes easier to understand if the code is directly put into the main
> xfer function.
>
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
> drivers/i2c/busses/i2c-sh_mobile.c | 28 +++++++++-------------------
> 1 file changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-sh_mobile.c
> b/drivers/i2c/busses/i2c-sh_mobile.c
> index a1253df9574594..cdd7a746b9d1ed 100644
> --- a/drivers/i2c/busses/i2c-sh_mobile.c
> +++ b/drivers/i2c/busses/i2c-sh_mobile.c
> @@ -298,23 +298,6 @@ static int sh_mobile_i2c_init(struct sh_mobile_i2c_data
> *pd)
> return 0;
> }
>
> -static void activate_ch(struct sh_mobile_i2c_data *pd)
> -{
> - /* Wake up device and enable clock */
> - pm_runtime_get_sync(pd->dev);
> - clk_prepare_enable(pd->clk);
> -}
> -
> -static void deactivate_ch(struct sh_mobile_i2c_data *pd)
> -{
> - /* Disable channel */
> - iic_set_clr(pd, ICCR, 0, ICCR_ICE);
> -
> - /* Disable clock and mark device as idle */
> - clk_disable_unprepare(pd->clk);
> - pm_runtime_put_sync(pd->dev);
> -}
> -
> static unsigned char i2c_op(struct sh_mobile_i2c_data *pd,
> enum sh_mobile_i2c_op op, unsigned char data)
> {
> @@ -717,7 +700,9 @@ static int sh_mobile_i2c_xfer(struct i2c_adapter *adapter,
> int i;
> long timeout;
>
> - activate_ch(pd);
> + /* Wake up device and enable clock */
> + pm_runtime_get_sync(pd->dev);
> + clk_prepare_enable(pd->clk);
Suggestion for further cleanup: the call to clk_prepare_enable() should
not be needed, due to Runtime PM taking care of that.
This is also the case on SH, which uses the legacy clock domain
(drivers/sh/pm_runtime.c).
>
> /* Process all messages */
> for (i = 0; i < num; i++) {
> @@ -754,7 +739,12 @@ static int sh_mobile_i2c_xfer(struct i2c_adapter
> *adapter,
> break;
> }
>
> - deactivate_ch(pd);
> + /* Disable channel */
> + iic_set_clr(pd, ICCR, 0, ICCR_ICE);
> +
> + /* Disable clock and mark device as idle */
> + clk_disable_unprepare(pd->clk);
Likewise for clk_disable_unprepare().
> + pm_runtime_put_sync(pd->dev);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
