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

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

[Martin Peres]

From: Karol Herbst <karolherbst@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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH 2/3] drm/nouveau/led: guard against a division by 0

[Martin Peres]

Signed-off-by: Martin Peres <martin.peres@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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH 3/3] drm/nouveau/led: abort early if the device does not have GPIOs

[Martin Peres]

Signed-off-by: Martin Peres <martin.peres@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH 2/3] drm/nouveau/led: guard against a division by 0

[Alexandre Courbot]

On Fri, Sep 16, 2016 at 4:34 PM, Martin Peres <martin.peres@free.fr> wrote:
> Signed-off-by: Martin Peres <martin.peres@free.fr>

Tested-by: Alexandre Courbot <acourbot@nvidia.com>

Fixes the crash I was seeing on GM206, thanks!
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH 2/3] drm/nouveau/led: guard against a division by 0

[Karol Herbst]

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

2016-09-16 9:34 GMT+02:00 Martin Peres <martin.peres@free.fr>:
> Signed-off-by: Martin Peres <martin.peres@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH 3/3] drm/nouveau/led: abort early if the device does not have GPIOs

[Alexandre Courbot]

On Fri, Sep 16, 2016 at 4:34 PM, Martin Peres <martin.peres@free.fr> wrote:
> Signed-off-by: Martin Peres <martin.peres@free.fr>

Tested-by: Alexandre Courbot <acourbot@nvidia.com>

Fixes the crash I was seeing on Tegra, thanks!
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH 3/3] drm/nouveau/led: abort early if the device does not have GPIOs

[Karol Herbst]

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

2016-09-16 9:34 GMT+02:00 Martin Peres <martin.peres@free.fr>:
> Signed-off-by: Martin Peres <martin.peres@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH 2/3] drm/nouveau/led: guard against a division by 0

[Martin Peres]

On 16/09/16 10:42, Karol Herbst wrote:
> Reviewed-by: Karol Herbst <karolherbst@gmail.com>
>
> 2016-09-16 9:34 GMT+02:00 Martin Peres <martin.peres@free.fr>:
>> Signed-off-by: Martin Peres <martin.peres@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;

The unlikely is a good idea, not a fan of single lines if conditions 
though :s
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH 2/3] drm/nouveau/led: guard against a division by 0

[Karol Herbst]

2016-09-16 11:14 GMT+02:00 Martin Peres <martin.peres@free.fr>:
> On 16/09/16 10:42, Karol Herbst wrote:
>>
>> Reviewed-by: Karol Herbst <karolherbst@gmail.com>
>>
>> 2016-09-16 9:34 GMT+02:00 Martin Peres <martin.peres@free.fr>:
>>>
>>> Signed-off-by: Martin Peres <martin.peres@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;
>
>
> The unlikely is a good idea, not a fan of single lines if conditions though
> :s

the single line was me being lazy.

if (unlikely(div <= 0))
        return 0;
return duty * LED_FULL / div;

:p
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH] drm/nouveau/led: make all the stub functions static inline

[Martin Peres]

Suggested-by: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Martin Peres <martin.peres@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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau