[v2] leds: max8997: Don't error if there is no pdata

Message ID 20221020114442.22215-1-paul@crapouillou.net
State New
Headers
Series [v2] leds: max8997: Don't error if there is no pdata |

Commit Message

Paul Cercueil Oct. 20, 2022, 11:44 a.m. UTC
  The driver works just fine if no platform data is supplied.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 v2: Add Andy's reviewed-by and reword the commit description.

 drivers/leds/leds-max8997.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)
  

Comments

Pavel Machek Oct. 21, 2022, 1:11 p.m. UTC | #1
On Thu 2022-10-20 12:44:42, Paul Cercueil wrote:
> The driver works just fine if no platform data is supplied.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Does it? Bad Paul, bad Andy.

> +++ b/drivers/leds/leds-max8997.c
> @@ -238,11 +238,6 @@ static int max8997_led_probe(struct platform_device *pdev)
>  	char name[20];
>  	int ret = 0;
>  
> -	if (pdata == NULL) {
> -		dev_err(&pdev->dev, "no platform data\n");
> -		return -ENODEV;
> -	}
> -
>  	led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL);
>  	if (led == NULL)
>  		return -ENOMEM;
> @@ -258,7 +253,7 @@ static int max8997_led_probe(struct platform_device *pdev)
>  	led->iodev = iodev;
>  
>  	/* initialize mode and brightness according to platform_data */
> -	if (pdata->led_pdata) {
> +	if (pdata && pdata->led_pdata) {
>  		u8 mode = 0, brightness = 0;
>  
>  		mode = pdata->led_pdata->mode[led->id];

I see pdata being dereferenced here.
									Pavel
  
Paul Cercueil Oct. 21, 2022, 1:19 p.m. UTC | #2
Le ven. 21 oct. 2022 à 15:11:45 +0200, Pavel Machek <pavel@ucw.cz> a 
écrit :
> On Thu 2022-10-20 12:44:42, Paul Cercueil wrote:
>>  The driver works just fine if no platform data is supplied.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> Does it? Bad Paul, bad Andy.

Yes, it does.

>>  +++ b/drivers/leds/leds-max8997.c
>>  @@ -238,11 +238,6 @@ static int max8997_led_probe(struct 
>> platform_device *pdev)
>>   	char name[20];
>>   	int ret = 0;
>> 
>>  -	if (pdata == NULL) {
>>  -		dev_err(&pdev->dev, "no platform data\n");
>>  -		return -ENODEV;
>>  -	}
>>  -
>>   	led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL);
>>   	if (led == NULL)
>>   		return -ENOMEM;
>>  @@ -258,7 +253,7 @@ static int max8997_led_probe(struct 
>> platform_device *pdev)
>>   	led->iodev = iodev;
>> 
>>   	/* initialize mode and brightness according to platform_data */
>>  -	if (pdata->led_pdata) {
>>  +	if (pdata && pdata->led_pdata) {
>>   		u8 mode = 0, brightness = 0;
>> 
>>   		mode = pdata->led_pdata->mode[led->id];
> 
> I see pdata being dereferenced here.

Oh really. Look again.

-Paul
  
Pavel Machek Oct. 22, 2022, 9:55 a.m. UTC | #3
Hi!

> > >  The driver works just fine if no platform data is supplied.
> > > 
> > >  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > >  Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > 
> > Does it? Bad Paul, bad Andy.
> 
> Yes, it does.

Sorry, I misread the patch. Applied now.

Best regards,
							Pavel
  

Patch

diff --git a/drivers/leds/leds-max8997.c b/drivers/leds/leds-max8997.c
index c0bddb33888d..c8d7f55c9dec 100644
--- a/drivers/leds/leds-max8997.c
+++ b/drivers/leds/leds-max8997.c
@@ -238,11 +238,6 @@  static int max8997_led_probe(struct platform_device *pdev)
 	char name[20];
 	int ret = 0;
 
-	if (pdata == NULL) {
-		dev_err(&pdev->dev, "no platform data\n");
-		return -ENODEV;
-	}
-
 	led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL);
 	if (led == NULL)
 		return -ENOMEM;
@@ -258,7 +253,7 @@  static int max8997_led_probe(struct platform_device *pdev)
 	led->iodev = iodev;
 
 	/* initialize mode and brightness according to platform_data */
-	if (pdata->led_pdata) {
+	if (pdata && pdata->led_pdata) {
 		u8 mode = 0, brightness = 0;
 
 		mode = pdata->led_pdata->mode[led->id];