diff mbox series

mmc: core: Added support for LED trigger only when SD card is connected

Message ID 20210624061418.30361-1-tykwon@m2i.co.kr
State New
Headers show
Series mmc: core: Added support for LED trigger only when SD card is connected | expand

Commit Message

권태영 June 24, 2021, 6:14 a.m. UTC
led_trigger_event() is always called.
In this case, if the LED trigger is set to the SD Card, the trigger
will occur even when the SD card is not connected and the LED will blink.

In case of SD Card, it is judged based on Card Detection information and
changes to generate LED trigger only when SD Card is connected.

Board tested: NXP i.MX 8M board

Signed-off-by: Kwon Tae-young <tykwon@m2i.co.kr>
---
 drivers/mmc/core/core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Ulf Hansson June 24, 2021, 12:21 p.m. UTC | #1
On Thu, 24 Jun 2021 at 08:14, Kwon Tae-young <tykwon@m2i.co.kr> wrote:
>
> led_trigger_event() is always called.
> In this case, if the LED trigger is set to the SD Card, the trigger
> will occur even when the SD card is not connected and the LED will blink.
>
> In case of SD Card, it is judged based on Card Detection information and
> changes to generate LED trigger only when SD Card is connected.
>
> Board tested: NXP i.MX 8M board
>
> Signed-off-by: Kwon Tae-young <tykwon@m2i.co.kr>
> ---
>  drivers/mmc/core/core.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index f194940c5974..b3156f6c5cfa 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -352,7 +352,11 @@ int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
>         if (err)
>                 return err;
>
> -       led_trigger_event(host->led, LED_FULL);
> +       if (host->ops->get_cd)

No, this is not the right thing to do. Invoking the ->get_cd()
callback, for every request is suboptimal and would likely have
effects on performance.

Moreover, I wonder how big an issue it is to use the led here. If the
card is being removed, the request will fail anyway, so the led should
soon stop flashing anyway, right?

> +               host->ops->get_cd(host) ? led_trigger_event(host->led, LED_FULL) : NULL;
> +       else
> +               led_trigger_event(host->led, LED_FULL);
> +
>         __mmc_start_request(host, mrq);
>
>         return 0;
> --
> 2.17.1
>

Kind regards
Uffe
권태영 June 25, 2021, 5:43 a.m. UTC | #2
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index

> > f194940c5974..b3156f6c5cfa 100644

> > --- a/drivers/mmc/core/core.c

> > +++ b/drivers/mmc/core/core.c

> > @@ -352,7 +352,11 @@ int mmc_start_request(struct mmc_host *host,

> struct mmc_request *mrq)

> >         if (err)

> >                 return err;

> >

> > -       led_trigger_event(host->led, LED_FULL);

> > +       if (host->ops->get_cd)

> 

> No, this is not the right thing to do. Invoking the ->get_cd() callback,

> for every request is suboptimal and would likely have effects on

> performance.

> 

> Moreover, I wonder how big an issue it is to use the led here. If the

> card is being removed, the request will fail anyway, so the led should

> soon stop flashing anyway, right?


Thanks for the feedback.

When I think about it, it seems that an error should be returned from the mmc_card_removed() function when the SD card is removed.
However, in my current board, no error is returned from mmc_card_removed().

I'm guessing it's because of the NULL in the mmc_sd_remove() function in drivers/mmc/core/sd.c , but I'm not sure.
I think it was clumsy because I was a newbie unfamiliar with mmc drivers. :)
I'll take a closer look.

Regards,
Kwon
권태영 Sept. 3, 2021, 6:17 a.m. UTC | #3
> I'm guessing it's because of the NULL in the mmc_sd_remove() function in

> drivers/mmc/core/sd.c , but I'm not sure.

> I think it was clumsy because I was a newbie unfamiliar with mmc

> drivers. :) I'll take a closer look.


This was a hardware issue.
The CD pins on my hardware kept vibrating. :)
diff mbox series

Patch

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index f194940c5974..b3156f6c5cfa 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -352,7 +352,11 @@  int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 	if (err)
 		return err;
 
-	led_trigger_event(host->led, LED_FULL);
+	if (host->ops->get_cd)
+		host->ops->get_cd(host) ? led_trigger_event(host->led, LED_FULL) : NULL;
+	else
+		led_trigger_event(host->led, LED_FULL);
+
 	__mmc_start_request(host, mrq);
 
 	return 0;