On Tue, May 22, 2018 at 1:35 AM, Evan Green <evgreen@xxxxxxxxxxxx> wrote: Some style concerns. > int mmc_gpio_get_cd(struct mmc_host *host) > { > + int can_sleep; > struct mmc_gpio *ctx = host->slot.handler_priv; I would rather name it int cansleep; and put after ctx definition. > > if (!ctx || !ctx->cd_gpio) > return -ENOSYS; > > - if (ctx->override_cd_active_level) > - return !gpiod_get_raw_value_cansleep(ctx->cd_gpio) ^ > - !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); > + can_sleep = gpiod_cansleep(ctx->cd_gpio); > + if (ctx->override_cd_active_level) { > + int value = can_sleep ? > + gpiod_get_raw_value_cansleep(ctx->cd_gpio) : > + gpiod_get_raw_value(ctx->cd_gpio); > + return !value ^ !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); > + } > > - return gpiod_get_value_cansleep(ctx->cd_gpio); > + return can_sleep ? gpiod_get_value_cansleep(ctx->cd_gpio) : > + gpiod_get_value(ctx->cd_gpio); Perhaps same style as you use above woule be better, i.e. retrun cansleep ? true_path: false_path; > } > EXPORT_SYMBOL(mmc_gpio_get_cd); -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html