diff mbox series

leds: call of_node_put() when breaking out of for_each_available_child_of_node()

Message ID 20221122065002.2327772-1-zhangpeng362@huawei.com
State New
Headers
Series leds: call of_node_put() when breaking out of for_each_available_child_of_node() |

Commit Message

zhangpeng (AS) Nov. 22, 2022, 6:50 a.m. UTC 
  From: ZhangPeng <zhangpeng362@huawei.com>

Since for_each_available_child_of_node() will increase the refcount of
node, we need to call of_node_put() manually when breaking out of the
iteration.

Fixes: 24e2d05d1b68 ("leds: Add driver for Qualcomm LPG")
Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
---
 drivers/leds/rgb/leds-qcom-lpg.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

Comments

kernel test robot Nov. 23, 2022, 5:30 a.m. UTC | #1 
Hi Peng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.1-rc6 next-20221122]
[cannot apply to pavel-leds/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Peng-Zhang/leds-call-of_node_put-when-breaking-out-of-for_each_available_child_of_node/20221122-142530
patch link:    https://lore.kernel.org/r/20221122065002.2327772-1-zhangpeng362%40huawei.com
patch subject: [PATCH] leds: call of_node_put() when breaking out of for_each_available_child_of_node()
config: arc-randconfig-r013-20221121
compiler: arceb-elf-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/013aed0a135439317bd90a015567b9c955e50445
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Peng-Zhang/leds-call-of_node_put-when-breaking-out-of-for_each_available_child_of_node/20221122-142530
        git checkout 013aed0a135439317bd90a015567b9c955e50445
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/leds/rgb/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/leds/rgb/leds-qcom-lpg.c: In function 'lpg_add_led':
>> drivers/leds/rgb/leds-qcom-lpg.c:1098:17: error: expected ';' before 'goto'
    1098 |                 goto err_put_np_node;
         |                 ^~~~
   drivers/leds/rgb/leds-qcom-lpg.c:1108:25: error: expected ';' before 'goto'
    1108 |                         goto err_put_np_node;
         |                         ^~~~


vim +1098 drivers/leds/rgb/leds-qcom-lpg.c

  1070	
  1071	static int lpg_add_led(struct lpg *lpg, struct device_node *np)
  1072	{
  1073		struct led_init_data init_data = {};
  1074		struct led_classdev *cdev;
  1075		struct device_node *child;
  1076		struct mc_subled *info;
  1077		struct lpg_led *led;
  1078		const char *state;
  1079		int num_channels;
  1080		u32 color = 0;
  1081		int ret;
  1082		int i;
  1083	
  1084		ret = of_property_read_u32(np, "color", &color);
  1085		if (ret < 0 && ret != -EINVAL) {
  1086			dev_err(lpg->dev, "failed to parse \"color\" of %pOF\n", np);
  1087			goto err_put_np_node;
  1088		}
  1089	
  1090		if (color == LED_COLOR_ID_RGB)
  1091			num_channels = of_get_available_child_count(np);
  1092		else
  1093			num_channels = 1;
  1094	
  1095		led = devm_kzalloc(lpg->dev, struct_size(led, channels, num_channels), GFP_KERNEL);
  1096		if (!led) {
  1097			ret = -ENOMEM
> 1098			goto err_put_np_node;
  1099		}
  1100	
  1101		led->lpg = lpg;
  1102		led->num_channels = num_channels;
  1103	
  1104		if (color == LED_COLOR_ID_RGB) {
  1105			info = devm_kcalloc(lpg->dev, num_channels, sizeof(*info), GFP_KERNEL);
  1106			if (!info) {
  1107				ret = -ENOMEM
  1108				goto err_put_np_node;
  1109			}
  1110			i = 0;
  1111			for_each_available_child_of_node(np, child) {
  1112				ret = lpg_parse_channel(lpg, child, &led->channels[i]);
  1113				if (ret < 0)
  1114					goto err_put_child_node;
  1115	
  1116				info[i].color_index = led->channels[i]->color;
  1117				info[i].intensity = 0;
  1118				i++;
  1119			}
  1120	
  1121			led->mcdev.subled_info = info;
  1122			led->mcdev.num_colors = num_channels;
  1123	
  1124			cdev = &led->mcdev.led_cdev;
  1125			cdev->brightness_set = lpg_brightness_mc_set;
  1126			cdev->blink_set = lpg_blink_mc_set;
  1127	
  1128			/* Register pattern accessors only if we have a LUT block */
  1129			if (lpg->lut_base) {
  1130				cdev->pattern_set = lpg_pattern_mc_set;
  1131				cdev->pattern_clear = lpg_pattern_mc_clear;
  1132			}
  1133		} else {
  1134			ret = lpg_parse_channel(lpg, np, &led->channels[0]);
  1135			if (ret < 0)
  1136				goto err_put_np_node;
  1137	
  1138			cdev = &led->cdev;
  1139			cdev->brightness_set = lpg_brightness_single_set;
  1140			cdev->blink_set = lpg_blink_single_set;
  1141	
  1142			/* Register pattern accessors only if we have a LUT block */
  1143			if (lpg->lut_base) {
  1144				cdev->pattern_set = lpg_pattern_single_set;
  1145				cdev->pattern_clear = lpg_pattern_single_clear;
  1146			}
  1147		}
  1148	
  1149		cdev->default_trigger = of_get_property(np, "linux,default-trigger", NULL);
  1150		cdev->max_brightness = LPG_RESOLUTION - 1;
  1151	
  1152		if (!of_property_read_string(np, "default-state", &state) &&
  1153		    !strcmp(state, "on"))
  1154			cdev->brightness = cdev->max_brightness;
  1155		else
  1156			cdev->brightness = LED_OFF;
  1157	
  1158		cdev->brightness_set(cdev, cdev->brightness);
  1159	
  1160		init_data.fwnode = of_fwnode_handle(np);
  1161	
  1162		if (color == LED_COLOR_ID_RGB)
  1163			ret = devm_led_classdev_multicolor_register_ext(lpg->dev, &led->mcdev, &init_data);
  1164		else
  1165			ret = devm_led_classdev_register_ext(lpg->dev, &led->cdev, &init_data);
  1166		if (ret)
  1167			dev_err(lpg->dev, "unable to register %s\n", cdev->name);
  1168		else
  1169			return ret;
  1170	
  1171	err_put_np_node:
  1172		of_node_put(np);
  1173		return ret;
  1174	
  1175	err_put_child_node:
  1176		of_node_put(child);
  1177		return ret;
  1178	}
  1179
kernel test robot Nov. 23, 2022, 6:20 a.m. UTC | #2 
Hi Peng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.1-rc6 next-20221122]
[cannot apply to pavel-leds/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Peng-Zhang/leds-call-of_node_put-when-breaking-out-of-for_each_available_child_of_node/20221122-142530
patch link:    https://lore.kernel.org/r/20221122065002.2327772-1-zhangpeng362%40huawei.com
patch subject: [PATCH] leds: call of_node_put() when breaking out of for_each_available_child_of_node()
config: riscv-randconfig-r014-20221120
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project af8c49dc1ec44339d915d988ffe0f38da68ca0e7)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/013aed0a135439317bd90a015567b9c955e50445
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Peng-Zhang/leds-call-of_node_put-when-breaking-out-of-for_each_available_child_of_node/20221122-142530
        git checkout 013aed0a135439317bd90a015567b9c955e50445
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/leds/rgb/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/leds/rgb/leds-qcom-lpg.c:1097:16: error: expected ';' after expression
                   ret = -ENOMEM
                                ^
                                ;
   drivers/leds/rgb/leds-qcom-lpg.c:1107:17: error: expected ';' after expression
                           ret = -ENOMEM
                                        ^
                                        ;
   2 errors generated.


vim +1097 drivers/leds/rgb/leds-qcom-lpg.c

  1070	
  1071	static int lpg_add_led(struct lpg *lpg, struct device_node *np)
  1072	{
  1073		struct led_init_data init_data = {};
  1074		struct led_classdev *cdev;
  1075		struct device_node *child;
  1076		struct mc_subled *info;
  1077		struct lpg_led *led;
  1078		const char *state;
  1079		int num_channels;
  1080		u32 color = 0;
  1081		int ret;
  1082		int i;
  1083	
  1084		ret = of_property_read_u32(np, "color", &color);
  1085		if (ret < 0 && ret != -EINVAL) {
  1086			dev_err(lpg->dev, "failed to parse \"color\" of %pOF\n", np);
  1087			goto err_put_np_node;
  1088		}
  1089	
  1090		if (color == LED_COLOR_ID_RGB)
  1091			num_channels = of_get_available_child_count(np);
  1092		else
  1093			num_channels = 1;
  1094	
  1095		led = devm_kzalloc(lpg->dev, struct_size(led, channels, num_channels), GFP_KERNEL);
  1096		if (!led) {
> 1097			ret = -ENOMEM
  1098			goto err_put_np_node;
  1099		}
  1100	
  1101		led->lpg = lpg;
  1102		led->num_channels = num_channels;
  1103	
  1104		if (color == LED_COLOR_ID_RGB) {
  1105			info = devm_kcalloc(lpg->dev, num_channels, sizeof(*info), GFP_KERNEL);
  1106			if (!info) {
  1107				ret = -ENOMEM
  1108				goto err_put_np_node;
  1109			}
  1110			i = 0;
  1111			for_each_available_child_of_node(np, child) {
  1112				ret = lpg_parse_channel(lpg, child, &led->channels[i]);
  1113				if (ret < 0)
  1114					goto err_put_child_node;
  1115	
  1116				info[i].color_index = led->channels[i]->color;
  1117				info[i].intensity = 0;
  1118				i++;
  1119			}
  1120	
  1121			led->mcdev.subled_info = info;
  1122			led->mcdev.num_colors = num_channels;
  1123	
  1124			cdev = &led->mcdev.led_cdev;
  1125			cdev->brightness_set = lpg_brightness_mc_set;
  1126			cdev->blink_set = lpg_blink_mc_set;
  1127	
  1128			/* Register pattern accessors only if we have a LUT block */
  1129			if (lpg->lut_base) {
  1130				cdev->pattern_set = lpg_pattern_mc_set;
  1131				cdev->pattern_clear = lpg_pattern_mc_clear;
  1132			}
  1133		} else {
  1134			ret = lpg_parse_channel(lpg, np, &led->channels[0]);
  1135			if (ret < 0)
  1136				goto err_put_np_node;
  1137	
  1138			cdev = &led->cdev;
  1139			cdev->brightness_set = lpg_brightness_single_set;
  1140			cdev->blink_set = lpg_blink_single_set;
  1141	
  1142			/* Register pattern accessors only if we have a LUT block */
  1143			if (lpg->lut_base) {
  1144				cdev->pattern_set = lpg_pattern_single_set;
  1145				cdev->pattern_clear = lpg_pattern_single_clear;
  1146			}
  1147		}
  1148	
  1149		cdev->default_trigger = of_get_property(np, "linux,default-trigger", NULL);
  1150		cdev->max_brightness = LPG_RESOLUTION - 1;
  1151	
  1152		if (!of_property_read_string(np, "default-state", &state) &&
  1153		    !strcmp(state, "on"))
  1154			cdev->brightness = cdev->max_brightness;
  1155		else
  1156			cdev->brightness = LED_OFF;
  1157	
  1158		cdev->brightness_set(cdev, cdev->brightness);
  1159	
  1160		init_data.fwnode = of_fwnode_handle(np);
  1161	
  1162		if (color == LED_COLOR_ID_RGB)
  1163			ret = devm_led_classdev_multicolor_register_ext(lpg->dev, &led->mcdev, &init_data);
  1164		else
  1165			ret = devm_led_classdev_register_ext(lpg->dev, &led->cdev, &init_data);
  1166		if (ret)
  1167			dev_err(lpg->dev, "unable to register %s\n", cdev->name);
  1168		else
  1169			return ret;
  1170	
  1171	err_put_np_node:
  1172		of_node_put(np);
  1173		return ret;
  1174	
  1175	err_put_child_node:
  1176		of_node_put(child);
  1177		return ret;
  1178	}
  1179
diff mbox series

Patch

diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
index 02f51cc61837..8075115cef58 100644
--- a/drivers/leds/rgb/leds-qcom-lpg.c
+++ b/drivers/leds/rgb/leds-qcom-lpg.c
@@ -1084,7 +1084,7 @@  static int lpg_add_led(struct lpg *lpg, struct device_node *np)
 	ret = of_property_read_u32(np, "color", &color);
 	if (ret < 0 && ret != -EINVAL) {
 		dev_err(lpg->dev, "failed to parse \"color\" of %pOF\n", np);
-		return ret;
+		goto err_put_np_node;
 	}
 
 	if (color == LED_COLOR_ID_RGB)
@@ -1093,21 +1093,25 @@  static int lpg_add_led(struct lpg *lpg, struct device_node *np)
 		num_channels = 1;
 
 	led = devm_kzalloc(lpg->dev, struct_size(led, channels, num_channels), GFP_KERNEL);
-	if (!led)
-		return -ENOMEM;
+	if (!led) {
+		ret = -ENOMEM
+		goto err_put_np_node;
+	}
 
 	led->lpg = lpg;
 	led->num_channels = num_channels;
 
 	if (color == LED_COLOR_ID_RGB) {
 		info = devm_kcalloc(lpg->dev, num_channels, sizeof(*info), GFP_KERNEL);
-		if (!info)
-			return -ENOMEM;
+		if (!info) {
+			ret = -ENOMEM
+			goto err_put_np_node;
+		}
 		i = 0;
 		for_each_available_child_of_node(np, child) {
 			ret = lpg_parse_channel(lpg, child, &led->channels[i]);
 			if (ret < 0)
-				return ret;
+				goto err_put_child_node;
 
 			info[i].color_index = led->channels[i]->color;
 			info[i].intensity = 0;
@@ -1129,7 +1133,7 @@  static int lpg_add_led(struct lpg *lpg, struct device_node *np)
 	} else {
 		ret = lpg_parse_channel(lpg, np, &led->channels[0]);
 		if (ret < 0)
-			return ret;
+			goto err_put_np_node;
 
 		cdev = &led->cdev;
 		cdev->brightness_set = lpg_brightness_single_set;
@@ -1161,7 +1165,15 @@  static int lpg_add_led(struct lpg *lpg, struct device_node *np)
 		ret = devm_led_classdev_register_ext(lpg->dev, &led->cdev, &init_data);
 	if (ret)
 		dev_err(lpg->dev, "unable to register %s\n", cdev->name);
+	else
+		return ret;
+
+err_put_np_node:
+	of_node_put(np);
+	return ret;
 
+err_put_child_node:
+	of_node_put(child);
 	return ret;
 }