Re: [PATCH v4 3/6] auxdisplay: Add TM16xx 7-segment LED matrix display controllers driver
From: Jean-François Lessard <hidden>
Date: 2025-08-27 18:37:48
Also in:
linux-leds, lkml
Le 25 août 2025 13 h 48 min 45 s HAE, "Jean-François Lessard" [off-list ref] a écrit :
Le 25 août 2025 11 h 14 min 21 s HAE, Andy Shevchenko [off-list ref] a écrit :quoted
On Sun, Aug 24, 2025 at 11:32:29PM -0400, Jean-François Lessard wrote:
...
quoted
quoted
+ fwnode_for_each_child_node(digits_node, child) + display->num_digits++;Don't we have a _count API for this?I'll use device_get_child_node_count() instead of manual counting loops.quoted
quoted
+ dev_dbg(dev, "Number of digits: %u\n", display->num_digits); + + if (display->num_digits) { + display->digits = devm_kcalloc(dev, display->num_digits, + sizeof(*display->digits), + GFP_KERNEL); + if (!display->digits) {quoted
+ fwnode_handle_put(digits_node);Use RAII instead, we have defined __free() method for this.quoted
+ return -ENOMEM; + } + + i = 0; + fwnode_for_each_child_node(digits_node, child) {Ditto. Use _scoped variant.Well received.
After further investigation, _scoped variant exists for device_for_each_child_node_scoped() but not for fwnode_for_each_child_node(). I suggest to include an additional patch in next submission to add to include/linux/property.h: #define fwnode_for_each_child_node_scoped(fwnode, child) \ for (struct fwnode_handle *child __free(fwnode_handle) = \ fwnode_get_next_child_node(fwnode, NULL); \ child; child = fwnode_get_next_child_node(fwnode, child)) #define fwnode_for_each_named_child_node_scoped(fwnode, child, name) \ fwnode_for_each_child_node_scoped(fwnode, child) \ for_each_if(fwnode_name_eq(child, name)) #define fwnode_for_each_available_child_node_scoped(fwnode, child) \ for (struct fwnode_handle *child __free(fwnode_handle) = \ fwnode_get_next_available_child_node(fwnode, NULL); \ child; child = fwnode_get_next_available_child_node(fwnode, child)) ...