| Message ID | 1460460309-13619-6-git-send-email-adrian.hunter@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12 April 2016 at 13:25, Adrian Hunter <adrian.hunter@intel.com> 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@intel.com> > --- > 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? > { > u8 ctrl; > > @@ -255,7 +250,7 @@ static void sdhci_activate_led(struct sdhci_host *host) > sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); > } > > -static void sdhci_deactivate_led(struct sdhci_host *host) > +static void __sdhci_led_deactivate(struct sdhci_host *host) > { > u8 ctrl; > > @@ -264,9 +259,11 @@ static void sdhci_deactivate_led(struct sdhci_host *host) > sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); > } > > -#ifdef SDHCI_USE_LEDS_CLASS > +#if defined(CONFIG_LEDS_CLASS) || (defined(CONFIG_LEDS_CLASS_MODULE) && \ > + defined(CONFIG_MMC_SDHCI_MODULE)) > + > static void sdhci_led_control(struct led_classdev *led, > - enum led_brightness brightness) > + enum led_brightness brightness) > { > struct sdhci_host *host = container_of(led, struct sdhci_host, led); > unsigned long flags; > @@ -277,12 +274,62 @@ static void sdhci_led_control(struct led_classdev *led, > goto out; > > if (brightness == LED_OFF) > - sdhci_deactivate_led(host); > + __sdhci_led_deactivate(host); > else > - sdhci_activate_led(host); > + __sdhci_led_activate(host); > out: > spin_unlock_irqrestore(&host->lock, flags); > } > + > +static int sdhci_led_register(struct sdhci_host *host) > +{ > + struct mmc_host *mmc = host->mmc; > + > + snprintf(host->led_name, sizeof(host->led_name), > + "%s::", mmc_hostname(mmc)); > + > + host->led.name = host->led_name; > + host->led.brightness = LED_OFF; > + host->led.default_trigger = mmc_hostname(mmc); > + host->led.brightness_set = sdhci_led_control; > + > + return led_classdev_register(mmc_dev(mmc), &host->led); > +} > + > +static void sdhci_led_unregister(struct sdhci_host *host) > +{ > + led_classdev_unregister(&host->led); > +} > + > +static inline void sdhci_led_activate(struct sdhci_host *host) > +{ > +} > + > +static inline void sdhci_led_deactivate(struct sdhci_host *host) > +{ > +} This seems wrong. I assume you actually want to control the registered leds within this ifdef and not have the two functions above being stubs? > + > +#else > + > +static inline int sdhci_led_register(struct sdhci_host *host) > +{ > + return 0; > +} > + > +static inline void sdhci_led_unregister(struct sdhci_host *host) > +{ > +} > + > +static inline void sdhci_led_activate(struct sdhci_host *host) > +{ > + __sdhci_led_activate(host); > +} > + > +static inline void sdhci_led_deactivate(struct sdhci_host *host) > +{ > + __sdhci_led_deactivate(host); > +} See my comment above. Shouldn't these two functions be stubs? > + > #endif > > /*****************************************************************************\ > @@ -1320,9 +1367,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq) > > WARN_ON(host->mrq != NULL); > > -#ifndef SDHCI_USE_LEDS_CLASS > - sdhci_activate_led(host); > -#endif > + sdhci_led_activate(host); > > /* > * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED > @@ -2183,9 +2228,7 @@ static void sdhci_tasklet_finish(unsigned long param) > host->cmd = NULL; > host->data = NULL; > > -#ifndef SDHCI_USE_LEDS_CLASS > - sdhci_deactivate_led(host); > -#endif > + sdhci_led_deactivate(host); > > mmiowb(); > spin_unlock_irqrestore(&host->lock, flags); > @@ -3305,21 +3348,12 @@ int sdhci_add_host(struct sdhci_host *host) > sdhci_dumpregs(host); > #endif > > -#ifdef SDHCI_USE_LEDS_CLASS > - snprintf(host->led_name, sizeof(host->led_name), > - "%s::", mmc_hostname(mmc)); > - host->led.name = host->led_name; > - host->led.brightness = LED_OFF; > - host->led.default_trigger = mmc_hostname(mmc); > - host->led.brightness_set = sdhci_led_control; > - > - ret = led_classdev_register(mmc_dev(mmc), &host->led); > + ret = sdhci_led_register(host); > if (ret) { > pr_err("%s: Failed to register LED device: %d\n", > mmc_hostname(mmc), ret); > goto unirq; > } > -#endif > > mmiowb(); > > @@ -3338,10 +3372,8 @@ int sdhci_add_host(struct sdhci_host *host) > return 0; > > unled: > -#ifdef SDHCI_USE_LEDS_CLASS > - led_classdev_unregister(&host->led); > + sdhci_led_unregister(host); > unirq: > -#endif > sdhci_do_reset(host, SDHCI_RESET_ALL); > sdhci_writel(host, 0, SDHCI_INT_ENABLE); > sdhci_writel(host, 0, SDHCI_SIGNAL_ENABLE); > @@ -3389,9 +3421,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead) > > mmc_remove_host(mmc); > > -#ifdef SDHCI_USE_LEDS_CLASS > - led_classdev_unregister(&host->led); > -#endif > + sdhci_led_unregister(host); > > if (!dead) > sdhci_do_reset(host, SDHCI_RESET_ALL); > -- > 1.9.1 > Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 13/04/16 14:42, Ulf Hansson wrote: > On 12 April 2016 at 13:25, Adrian Hunter <adrian.hunter@intel.com> 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@intel.com> >> --- >> 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. > >> { >> u8 ctrl; >> >> @@ -255,7 +250,7 @@ static void sdhci_activate_led(struct sdhci_host *host) >> sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); >> } >> >> -static void sdhci_deactivate_led(struct sdhci_host *host) >> +static void __sdhci_led_deactivate(struct sdhci_host *host) >> { >> u8 ctrl; >> >> @@ -264,9 +259,11 @@ static void sdhci_deactivate_led(struct sdhci_host *host) >> sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); >> } >> >> -#ifdef SDHCI_USE_LEDS_CLASS >> +#if defined(CONFIG_LEDS_CLASS) || (defined(CONFIG_LEDS_CLASS_MODULE) && \ >> + defined(CONFIG_MMC_SDHCI_MODULE)) >> + >> static void sdhci_led_control(struct led_classdev *led, >> - enum led_brightness brightness) >> + enum led_brightness brightness) >> { >> struct sdhci_host *host = container_of(led, struct sdhci_host, led); >> unsigned long flags; >> @@ -277,12 +274,62 @@ static void sdhci_led_control(struct led_classdev *led, >> goto out; >> >> if (brightness == LED_OFF) >> - sdhci_deactivate_led(host); >> + __sdhci_led_deactivate(host); >> else >> - sdhci_activate_led(host); >> + __sdhci_led_activate(host); >> out: >> spin_unlock_irqrestore(&host->lock, flags); >> } >> + >> +static int sdhci_led_register(struct sdhci_host *host) >> +{ >> + struct mmc_host *mmc = host->mmc; >> + >> + snprintf(host->led_name, sizeof(host->led_name), >> + "%s::", mmc_hostname(mmc)); >> + >> + host->led.name = host->led_name; >> + host->led.brightness = LED_OFF; >> + host->led.default_trigger = mmc_hostname(mmc); >> + host->led.brightness_set = sdhci_led_control; >> + >> + return led_classdev_register(mmc_dev(mmc), &host->led); >> +} >> + >> +static void sdhci_led_unregister(struct sdhci_host *host) >> +{ >> + led_classdev_unregister(&host->led); >> +} >> + >> +static inline void sdhci_led_activate(struct sdhci_host *host) >> +{ >> +} >> + >> +static inline void sdhci_led_deactivate(struct sdhci_host *host) >> +{ >> +} > > This seems wrong. I assume you actually want to control the registered > leds within this ifdef and not have the two functions above being > stubs? No, because this is the case where mmc core controls the LED and these are the functions we call directly. > >> + >> +#else >> + >> +static inline int sdhci_led_register(struct sdhci_host *host) >> +{ >> + return 0; >> +} >> + >> +static inline void sdhci_led_unregister(struct sdhci_host *host) >> +{ >> +} >> + >> +static inline void sdhci_led_activate(struct sdhci_host *host) >> +{ >> + __sdhci_led_activate(host); >> +} >> + >> +static inline void sdhci_led_deactivate(struct sdhci_host *host) >> +{ >> + __sdhci_led_deactivate(host); >> +} > > See my comment above. Shouldn't these two functions be stubs? > >> + >> #endif >> >> /*****************************************************************************\ >> @@ -1320,9 +1367,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq) >> >> WARN_ON(host->mrq != NULL); >> >> -#ifndef SDHCI_USE_LEDS_CLASS >> - sdhci_activate_led(host); >> -#endif >> + sdhci_led_activate(host); >> >> /* >> * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED >> @@ -2183,9 +2228,7 @@ static void sdhci_tasklet_finish(unsigned long param) >> host->cmd = NULL; >> host->data = NULL; >> >> -#ifndef SDHCI_USE_LEDS_CLASS >> - sdhci_deactivate_led(host); >> -#endif >> + sdhci_led_deactivate(host); >> >> mmiowb(); >> spin_unlock_irqrestore(&host->lock, flags); >> @@ -3305,21 +3348,12 @@ int sdhci_add_host(struct sdhci_host *host) >> sdhci_dumpregs(host); >> #endif >> >> -#ifdef SDHCI_USE_LEDS_CLASS >> - snprintf(host->led_name, sizeof(host->led_name), >> - "%s::", mmc_hostname(mmc)); >> - host->led.name = host->led_name; >> - host->led.brightness = LED_OFF; >> - host->led.default_trigger = mmc_hostname(mmc); >> - host->led.brightness_set = sdhci_led_control; >> - >> - ret = led_classdev_register(mmc_dev(mmc), &host->led); >> + ret = sdhci_led_register(host); >> if (ret) { >> pr_err("%s: Failed to register LED device: %d\n", >> mmc_hostname(mmc), ret); >> goto unirq; >> } >> -#endif >> >> mmiowb(); >> >> @@ -3338,10 +3372,8 @@ int sdhci_add_host(struct sdhci_host *host) >> return 0; >> >> unled: >> -#ifdef SDHCI_USE_LEDS_CLASS >> - led_classdev_unregister(&host->led); >> + sdhci_led_unregister(host); >> unirq: >> -#endif >> sdhci_do_reset(host, SDHCI_RESET_ALL); >> sdhci_writel(host, 0, SDHCI_INT_ENABLE); >> sdhci_writel(host, 0, SDHCI_SIGNAL_ENABLE); >> @@ -3389,9 +3421,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead) >> >> mmc_remove_host(mmc); >> >> -#ifdef SDHCI_USE_LEDS_CLASS >> - led_classdev_unregister(&host->led); >> -#endif >> + sdhci_led_unregister(host); >> >> if (!dead) >> sdhci_do_reset(host, SDHCI_RESET_ALL); >> -- >> 1.9.1 >> > > Kind regards > Uffe > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 13 April 2016 at 13:45, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 13/04/16 14:42, Ulf Hansson wrote: >> On 12 April 2016 at 13:25, Adrian Hunter <adrian.hunter@intel.com> 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@intel.com> >>> --- >>> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 13/04/16 15:15, Ulf Hansson wrote: > On 13 April 2016 at 13:45, Adrian Hunter <adrian.hunter@intel.com> wrote: >> On 13/04/16 14:42, Ulf Hansson wrote: >>> On 12 April 2016 at 13:25, Adrian Hunter <adrian.hunter@intel.com> 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@intel.com> >>>> --- >>>> 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. How does it change the current behaviour? It was meant to be exactly the same. Perhaps looking closely at whether the original code was 'ifdef' or 'ifndef' will help. > > I am not sure why you want to use the leds in case of when the LED > subsystem isn't available? I am attempting to preserve the existing behaviour, so that is just how it was. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[...] >>>>> >>>>> -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. > > How does it change the current behaviour? It was meant to be exactly the > same. Perhaps looking closely at whether the original code was 'ifdef' or > 'ifndef' will help. Huh. Yes, indeed you are right! Sorry for noise, again. > >> >> I am not sure why you want to use the leds in case of when the LED >> subsystem isn't available? > > I am attempting to preserve the existing behaviour, so that is just how it was. > I have applied all five patches for next! Thanks! Still, perhaps a future clean-up can adopt to my proposal above!? Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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) { u8 ctrl; @@ -255,7 +250,7 @@ static void sdhci_activate_led(struct sdhci_host *host) sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); } -static void sdhci_deactivate_led(struct sdhci_host *host) +static void __sdhci_led_deactivate(struct sdhci_host *host) { u8 ctrl; @@ -264,9 +259,11 @@ static void sdhci_deactivate_led(struct sdhci_host *host) sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); } -#ifdef SDHCI_USE_LEDS_CLASS +#if defined(CONFIG_LEDS_CLASS) || (defined(CONFIG_LEDS_CLASS_MODULE) && \ + defined(CONFIG_MMC_SDHCI_MODULE)) + static void sdhci_led_control(struct led_classdev *led, - enum led_brightness brightness) + enum led_brightness brightness) { struct sdhci_host *host = container_of(led, struct sdhci_host, led); unsigned long flags; @@ -277,12 +274,62 @@ static void sdhci_led_control(struct led_classdev *led, goto out; if (brightness == LED_OFF) - sdhci_deactivate_led(host); + __sdhci_led_deactivate(host); else - sdhci_activate_led(host); + __sdhci_led_activate(host); out: spin_unlock_irqrestore(&host->lock, flags); } + +static int sdhci_led_register(struct sdhci_host *host) +{ + struct mmc_host *mmc = host->mmc; + + snprintf(host->led_name, sizeof(host->led_name), + "%s::", mmc_hostname(mmc)); + + host->led.name = host->led_name; + host->led.brightness = LED_OFF; + host->led.default_trigger = mmc_hostname(mmc); + host->led.brightness_set = sdhci_led_control; + + return led_classdev_register(mmc_dev(mmc), &host->led); +} + +static void sdhci_led_unregister(struct sdhci_host *host) +{ + led_classdev_unregister(&host->led); +} + +static inline void sdhci_led_activate(struct sdhci_host *host) +{ +} + +static inline void sdhci_led_deactivate(struct sdhci_host *host) +{ +} + +#else + +static inline int sdhci_led_register(struct sdhci_host *host) +{ + return 0; +} + +static inline void sdhci_led_unregister(struct sdhci_host *host) +{ +} + +static inline void sdhci_led_activate(struct sdhci_host *host) +{ + __sdhci_led_activate(host); +} + +static inline void sdhci_led_deactivate(struct sdhci_host *host) +{ + __sdhci_led_deactivate(host); +} + #endif /*****************************************************************************\ @@ -1320,9 +1367,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq) WARN_ON(host->mrq != NULL); -#ifndef SDHCI_USE_LEDS_CLASS - sdhci_activate_led(host); -#endif + sdhci_led_activate(host); /* * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED @@ -2183,9 +2228,7 @@ static void sdhci_tasklet_finish(unsigned long param) host->cmd = NULL; host->data = NULL; -#ifndef SDHCI_USE_LEDS_CLASS - sdhci_deactivate_led(host); -#endif + sdhci_led_deactivate(host); mmiowb(); spin_unlock_irqrestore(&host->lock, flags); @@ -3305,21 +3348,12 @@ int sdhci_add_host(struct sdhci_host *host) sdhci_dumpregs(host); #endif -#ifdef SDHCI_USE_LEDS_CLASS - snprintf(host->led_name, sizeof(host->led_name), - "%s::", mmc_hostname(mmc)); - host->led.name = host->led_name; - host->led.brightness = LED_OFF; - host->led.default_trigger = mmc_hostname(mmc); - host->led.brightness_set = sdhci_led_control; - - ret = led_classdev_register(mmc_dev(mmc), &host->led); + ret = sdhci_led_register(host); if (ret) { pr_err("%s: Failed to register LED device: %d\n", mmc_hostname(mmc), ret); goto unirq; } -#endif mmiowb(); @@ -3338,10 +3372,8 @@ int sdhci_add_host(struct sdhci_host *host) return 0; unled: -#ifdef SDHCI_USE_LEDS_CLASS - led_classdev_unregister(&host->led); + sdhci_led_unregister(host); unirq: -#endif sdhci_do_reset(host, SDHCI_RESET_ALL); sdhci_writel(host, 0, SDHCI_INT_ENABLE); sdhci_writel(host, 0, SDHCI_SIGNAL_ENABLE); @@ -3389,9 +3421,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead) mmc_remove_host(mmc); -#ifdef SDHCI_USE_LEDS_CLASS - led_classdev_unregister(&host->led); -#endif + sdhci_led_unregister(host); if (!dead) sdhci_do_reset(host, SDHCI_RESET_ALL);
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@intel.com> --- drivers/mmc/host/sdhci.c | 96 +++++++++++++++++++++++++++++++----------------- 1 file changed, 63 insertions(+), 33 deletions(-)