Quoting Doug Anderson (2018-09-20 13:22:37) > Hi, > On Thu, Sep 20, 2018 at 11:03 AM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > > > We never really look at the 'ret' local variable in these functions, so > > let's remove it to make way for shorter and simpler code. Furthermore, > > we can shorten some lines by adding two local variables for the SE and > > the message length so that everything fits in 80 columns. And kernel > > style is to leave the return statement by itself, detached from the rest > > of the function. > > > > Cc: Karthikeyan Ramasubramanian <kramasub@xxxxxxxxxxxxxx> > > Cc: Sagar Dharia <sdharia@xxxxxxxxxxxxxx> > > Cc: Girish Mahadevan <girishm@xxxxxxxxxxxxxx> > > Signed-off-by: Stephen Boyd <swboyd@xxxxxxxxxxxx> > > --- > > drivers/i2c/busses/i2c-qcom-geni.c | 54 ++++++++++++++---------------- > > 1 file changed, 25 insertions(+), 29 deletions(-) > > I didn't hate the old code, but you're right that this looks a little better. > > Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > > NOTE: now that you're cleaning it up, I'd personally go even further > and totally get rid of the "mode" local variable. Basically, just: > > dma_buf = i2c_get_dma_safe_msg_buf(msg, 32); > if (dma_buf) > geni_se_select_mode(se, GENI_SE_DMA); > else > geni_se_select_mode(se, GENI_SE_FIFO); Sure that's fine. I'll resend this one patch with that squashed in.
