On 13 April 2016 at 13:45, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > On 13/04/16 14:42, Ulf Hansson wrote: >> On 12 April 2016 at 13:25, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: >>> ifdef's make the code more complicated and harder to read. >>> Move all the LED code together to reduce the ifdef's to >>> one place. >>> >>> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx> >>> --- >>> drivers/mmc/host/sdhci.c | 96 +++++++++++++++++++++++++++++++----------------- >>> 1 file changed, 63 insertions(+), 33 deletions(-) >>> >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >>> index be5b5c95138c..13f3dd8992d5 100644 >>> --- a/drivers/mmc/host/sdhci.c >>> +++ b/drivers/mmc/host/sdhci.c >>> @@ -38,11 +38,6 @@ >>> #define DBG(f, x...) \ >>> pr_debug(DRIVER_NAME " [%s()]: " f, __func__,## x) >>> >>> -#if defined(CONFIG_LEDS_CLASS) || (defined(CONFIG_LEDS_CLASS_MODULE) && \ >>> - defined(CONFIG_MMC_SDHCI_MODULE)) >>> -#define SDHCI_USE_LEDS_CLASS >>> -#endif >>> - >>> #define MAX_TUNING_LOOP 40 >>> >>> static unsigned int debug_quirks = 0; >>> @@ -246,7 +241,7 @@ static void sdhci_reinit(struct sdhci_host *host) >>> sdhci_enable_card_detection(host); >>> } >>> >>> -static void sdhci_activate_led(struct sdhci_host *host) >>> +static void __sdhci_led_activate(struct sdhci_host *host) >> >> The renaming here seems a bit unnecessary, but more importantly why >> can't you move these functions within the "if def sdhci leds" instead? > > They get used either way. Either we use the LEDS subsystem (and mmc core > controls them) or we turn them on/off directly from sdhci_request() etc. That seems wrong. Moreover it changes the current behaviour. I am not sure why you want to use the leds in case of when the LED subsystem isn't available? [...] Kind regards Uffe -- 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