Nouveau - Sep 2016 - [PATCH 1/3] drm/nouveau/led: don't access led subdev if it wasn't initialized

From: Karol Herbst <karolherbst at gmail.com>

Fixes a kernel crash on suspend/resume.
---
 drm/nouveau/nouveau_led.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drm/nouveau/nouveau_led.c b/drm/nouveau/nouveau_led.c
index 9eed5a6..5e28b5f 100644
--- a/drm/nouveau/nouveau_led.c
+++ b/drm/nouveau/nouveau_led.c
@@ -107,7 +107,8 @@ nouveau_led_suspend(struct drm_device *dev)
 {
 	struct nouveau_drm *drm = nouveau_drm(dev);
 
-	led_classdev_suspend(&drm->led->led);
+	if (drm->led)
+		led_classdev_suspend(&drm->led->led);
 }
 
 void
@@ -115,8 +116,8 @@ nouveau_led_resume(struct drm_device *dev)
 {
 	struct nouveau_drm *drm = nouveau_drm(dev);
 
-	led_classdev_resume(&drm->led->led);
-
+	if (drm->led)
+		led_classdev_resume(&drm->led->led);
 }
 
 void
-- 
2.8.0

Signed-off-by: Martin Peres <martin.peres at free.fr>
---
 drm/nouveau/nouveau_led.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drm/nouveau/nouveau_led.c b/drm/nouveau/nouveau_led.c
index 5e28b5f..1f731da 100644
--- a/drm/nouveau/nouveau_led.c
+++ b/drm/nouveau/nouveau_led.c
@@ -44,7 +44,10 @@ nouveau_led_get_brightness(struct led_classdev *led)
 	div =  nvif_rd32(device, 0x61c880) & 0x00ffffff;
 	duty = nvif_rd32(device, 0x61c884) & 0x00ffffff;
 
-	return duty * LED_FULL / div;
+	if (div > 0)
+		return duty * LED_FULL / div;
+	else
+		return 0;
 }
 
 static void
-- 
2.8.0

Signed-off-by: Martin Peres <martin.peres at free.fr>
---
 drm/nouveau/nouveau_led.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drm/nouveau/nouveau_led.c b/drm/nouveau/nouveau_led.c
index 1f731da..3e2f1b6 100644
--- a/drm/nouveau/nouveau_led.c
+++ b/drm/nouveau/nouveau_led.c
@@ -82,6 +82,9 @@ nouveau_led_init(struct drm_device *dev)
 	struct dcb_gpio_func logo_led;
 	int ret;
 
+	if (!gpio)
+		return 0;
+
 	/* check that there is a GPIO controlling the logo LED */
 	if (nvkm_gpio_find(gpio, 0, DCB_GPIO_LOGO_LED_PWM, 0xff, &logo_led))
 		return 0;
-- 
2.8.0

On Fri, Sep 16, 2016 at 4:34 PM, Martin Peres <martin.peres at free.fr>
wrote:
> Signed-off-by: Martin Peres <martin.peres at free.fr>
Tested-by: Alexandre Courbot <acourbot at nvidia.com>

Fixes the crash I was seeing on GM206, thanks!

Reviewed-by: Karol Herbst <karolherbst at gmail.com>

2016-09-16 9:34 GMT+02:00 Martin Peres <martin.peres at
free.fr>:
> Signed-off-by: Martin Peres <martin.peres at free.fr>
> ---
>  drm/nouveau/nouveau_led.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drm/nouveau/nouveau_led.c b/drm/nouveau/nouveau_led.c
> index 5e28b5f..1f731da 100644
> --- a/drm/nouveau/nouveau_led.c
> +++ b/drm/nouveau/nouveau_led.c
> @@ -44,7 +44,10 @@ nouveau_led_get_brightness(struct led_classdev *led)
>         div =  nvif_rd32(device, 0x61c880) & 0x00ffffff;
>         duty = nvif_rd32(device, 0x61c884) & 0x00ffffff;
>
> -       return duty * LED_FULL / div;
> +       if (div > 0)
> +               return duty * LED_FULL / div;
> +       else
> +               return 0;
minor nitpick: you can drop the else, I don't mind though. Maybe it
would be clearer to do it the other way around though.

Or to do:

if (unlikely(div <= 0)) return 0;
return duty * LED_FULL / div;
>  }
>
>  static void
> --
> 2.8.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

On Fri, Sep 16, 2016 at 4:34 PM, Martin Peres <martin.peres at free.fr>
wrote:
> Signed-off-by: Martin Peres <martin.peres at free.fr>
Tested-by: Alexandre Courbot <acourbot at nvidia.com>

Fixes the crash I was seeing on Tegra, thanks!

Reviewed-by: Karol Herbst <karolherbst at gmail.com>

2016-09-16 9:34 GMT+02:00 Martin Peres <martin.peres at
free.fr>:
> Signed-off-by: Martin Peres <martin.peres at free.fr>
> ---
>  drm/nouveau/nouveau_led.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drm/nouveau/nouveau_led.c b/drm/nouveau/nouveau_led.c
> index 1f731da..3e2f1b6 100644
> --- a/drm/nouveau/nouveau_led.c
> +++ b/drm/nouveau/nouveau_led.c
> @@ -82,6 +82,9 @@ nouveau_led_init(struct drm_device *dev)
>         struct dcb_gpio_func logo_led;
>         int ret;
>
> +       if (!gpio)
> +               return 0;
> +
>         /* check that there is a GPIO controlling the logo LED */
>         if (nvkm_gpio_find(gpio, 0, DCB_GPIO_LOGO_LED_PWM, 0xff,
&logo_led))
>                 return 0;
> --
> 2.8.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

Suggested-by: Ilia Mirkin <imirkin at alum.mit.edu>
Signed-off-by: Martin Peres <martin.peres at free.fr>
---
 drm/nouveau/nouveau_led.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drm/nouveau/nouveau_led.h b/drm/nouveau/nouveau_led.h
index 750a0d9..187ecdb 100644
--- a/drm/nouveau/nouveau_led.h
+++ b/drm/nouveau/nouveau_led.h
@@ -49,9 +49,9 @@ void nouveau_led_resume(struct drm_device *dev);
 void nouveau_led_fini(struct drm_device *dev);
 #else
 static inline int  nouveau_led_init(struct drm_device *dev) { return 0; };
-void nouveau_led_suspend(struct drm_device *dev) { return; };
-void nouveau_led_resume(struct drm_device *dev) { return; };
-void nouveau_led_fini(struct drm_device *dev) { return; };
+static inline void nouveau_led_suspend(struct drm_device *dev) { };
+static inline void nouveau_led_resume(struct drm_device *dev) { };
+static inline void nouveau_led_fini(struct drm_device *dev) { };
 #endif
 
 #endif
-- 
2.8.0