Re: [PATCH v3 09/19] hwmon: (mr75203) add VM active channel support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/31/2022 2:48 PM, Andy Shevchenko wrote:
On Tue, Aug 30, 2022 at 07:22:02PM +0000, Eliav Farber wrote:
Add active channel support per voltage monitor.
The number of active channels is read from the device-tree.
When absent in device-tree, all channels are assumed to be used.

This shall be useful to expose sysfs only for inputs that are connected
to a voltage source.

Setting number of active channels to 0, means that entire VM sensor is
not used.

...

+struct voltage_device {
+     u32 vm_map;     /* Map channel number to VM index */
+     u32 ch_map;     /* Map channel number to channel index */
+};
+
+struct voltage_channels {
+     u32 total;      /* Total number of channels in all VMs */
+     u8 max;         /* Maximum number of channels among all VMs */
+};

Why not convert them to kernel doc?

Fixed in v4.


+     ret = device_property_read_u8_array(dev, "moortec,vm-active-channels",
+                                         vm_active_ch, vm_num);
+     if (ret) {
+             /*
+              * Incase vm-active-channels property is not defined,
+              * we assume each VM sensor has all of its channels
+              * active.
+              */
+             for (i = 0; i < vm_num; i++)
+                     vm_active_ch[i] = ch_num;

NIH memset().

Fixed in v4.


+             pvt->vm_channels.max = ch_num;
+             pvt->vm_channels.total = ch_num * vm_num;
+     } else {
+             for (i = 0; i < vm_num; i++) {
+                     if (vm_active_ch[i] > ch_num) {
+                             dev_err(dev, "invalid active channels: %u\n",
+                                     vm_active_ch[i]);
+                             return -EINVAL;
+                     }
+
+                     pvt->vm_channels.total += vm_active_ch[i];
+
+                     if (vm_active_ch[i] > pvt->vm_channels.max)
+                             pvt->vm_channels.max = vm_active_ch[i];
+             }
+     }

...

+     k = 0;
+     for (i = 0; i < vm_num; i++)
+             for (j = 0; j < vm_active_ch[i]; j++) {
+                     pvt->vd[k].vm_map = vm_idx[i];
+                     pvt->vd[k].ch_map = j;

+                     k++;

How is it different from moving this outside the inner loop as

       k += vm_active_ch[i];

?

k is used inside the inner loop, so increasing it outside the inner loop
will result in a different incorrect setting of vm_map and ch_map.

+             }

Missed outer {}.

Fixed in v4.


--
Regards, Eliav




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux