Re: [PATCH RFT 3/5] i2c: sh_mobile: manually "inline" two short functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Wolfram,

On Thu, Nov 2, 2017 at 1:47 PM, Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> 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 <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> ---
>  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 -- geert@xxxxxxxxxxxxxx

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



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux