Add documentation of the algorithm used to perform scatter/gather
of the requested lines and values in linereq_get_values() and
linereq_set_values_unlocked() to improve maintainability.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 73262305de0f..27cfed748b0a 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1391,9 +1391,18 @@ static long linereq_get_values(struct linereq *lr, void __user *ip)
 	if (copy_from_user(&lv, ip, sizeof(lv)))
 		return -EFAULT;
 
+	/*
+	 * gpiod_get_array_value_complex() requires compacted desc and val
+	 * arrays, rather than the sparse ones in lv.
+	 * Calculation of num_get and construction of the desc array is
+	 * optimized to avoid allocation for the desc array for the common
+	 * num_get == 1 case.
+	 */
+	/* scan requested lines to calculate the subset to get */
 	for (num_get = 0, i = 0; i < lr->num_lines; i++) {
 		if (lv.mask & BIT_ULL(i)) {
 			num_get++;
+			/* capture desc for the num_get == 1 case */
 			descs = &lr->lines[i].desc;
 		}
 	}
@@ -1402,6 +1411,7 @@ static long linereq_get_values(struct linereq *lr, void __user *ip)
 		return -EINVAL;
 
 	if (num_get != 1) {
+		/* build compacted desc array */
 		descs = kmalloc_array(num_get, sizeof(*descs), GFP_KERNEL);
 		if (!descs)
 			return -ENOMEM;
@@ -1422,6 +1432,7 @@ static long linereq_get_values(struct linereq *lr, void __user *ip)
 
 	lv.bits = 0;
 	for (didx = 0, i = 0; i < lr->num_lines; i++) {
+		/* unpack compacted vals for the response */
 		if (lv.mask & BIT_ULL(i)) {
 			if (lr->lines[i].sw_debounced)
 				val = debounced_value(&lr->lines[i]);
@@ -1447,14 +1458,25 @@ static long linereq_set_values_unlocked(struct linereq *lr,
 	unsigned int i, didx, num_set;
 	int ret;
 
+	/*
+	 * gpiod_set_array_value_complex() requires compacted desc and val
+	 * arrays, rather than the sparse ones in lv.
+	 * Calculation of num_set and construction of the descs and vals arrays
+	 * is optimized to minimize scanning the lv->mask, and to avoid
+	 * allocation for the desc array for the common num_set == 1 case.
+	 */
 	bitmap_zero(vals, GPIO_V2_LINES_MAX);
+	/* scan requested lines to determine the subset to be set */
 	for (num_set = 0, i = 0; i < lr->num_lines; i++) {
 		if (lv->mask & BIT_ULL(i)) {
+			/* setting inputs is not allowed */
 			if (!test_bit(FLAG_IS_OUT, &lr->lines[i].desc->flags))
 				return -EPERM;
+			/* add to compacted values */
 			if (lv->bits & BIT_ULL(i))
 				__set_bit(num_set, vals);
 			num_set++;
+			/* capture desc for the num_set == 1 case */
 			descs = &lr->lines[i].desc;
 		}
 	}
@@ -1462,7 +1484,7 @@ static long linereq_set_values_unlocked(struct linereq *lr,
 		return -EINVAL;
 
 	if (num_set != 1) {
-		/* build compacted desc array and values */
+		/* build compacted desc array */
 		descs = kmalloc_array(num_set, sizeof(*descs), GFP_KERNEL);
 		if (!descs)
 			return -ENOMEM;
-- 
2.39.2

On Thu, Dec 14, 2023 at 05:58:14PM +0800, Kent Gibson wrote:
> Add documentation of the algorithm used to perform scatter/gather
> of the requested lines and values in linereq_get_values() and
> linereq_set_values_unlocked() to improve maintainability.

I believe this is based on the old discussion in the thread where I proposed
the patch to convert this (unreadable in my opinion) code to use bitmap APIs.


-- 
With Best Regards,
Andy Shevchenko

On Thu, Dec 14, 2023 at 05:12:36PM +0200, Andy Shevchenko wrote:
> On Thu, Dec 14, 2023 at 05:58:14PM +0800, Kent Gibson wrote:
> > Add documentation of the algorithm used to perform scatter/gather
> > of the requested lines and values in linereq_get_values() and
> > linereq_set_values_unlocked() to improve maintainability.
>
> I believe this is based on the old discussion in the thread where I proposed
> the patch to convert this (unreadable in my opinion) code to use bitmap APIs.
>

I believe you are correct - that the code is unreadable in your
opinion.  And I noted that you were the origin of this change in the
cover letter.

I'm not usually comforable with this level of hand holding, but I made
an exception in your case.

Cheers,
Kent.

On Thu, Dec 14, 2023 at 11:23:01PM +0800, Kent Gibson wrote:
> On Thu, Dec 14, 2023 at 05:12:36PM +0200, Andy Shevchenko wrote:
> > On Thu, Dec 14, 2023 at 05:58:14PM +0800, Kent Gibson wrote:
> > > Add documentation of the algorithm used to perform scatter/gather
> > > of the requested lines and values in linereq_get_values() and
> > > linereq_set_values_unlocked() to improve maintainability.
> >
> > I believe this is based on the old discussion in the thread where I proposed
> > the patch to convert this (unreadable in my opinion) code to use bitmap APIs.
> 
> I believe you are correct - that the code is unreadable in your
> opinion.  And I noted that you were the origin of this change in the
> cover letter.
> 
> I'm not usually comforable with this level of hand holding, but I made
> an exception in your case.

At least it helps somebody to understand that, thank you!

-- 
With Best Regards,
Andy Shevchenko