Re: [PATCH v3 3/3] iio: adc: Add support for QCOM PMIC5 Gen3 ADC

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

 



Hi Dmitry,

On 12/31/2023 11:16 PM, Dmitry Baryshkov wrote:
On Sun, 31 Dec 2023 at 19:13, Jishnu Prakash <quic_jprakash@xxxxxxxxxxx> wrote:
The ADC architecture on PMIC5 Gen3 is similar to that on PMIC5 Gen2,
with all SW communication to ADC going through PMK8550 which
communicates with other PMICs through PBS.

+static int adc_tm_register_tzd(struct adc5_chip *adc)
+{
+       unsigned int i, channel;
+       struct thermal_zone_device *tzd;
+
+       for (i = 0; i < adc->nchannels; i++) {
+               channel = V_CHAN(adc->chan_props[i]);
+
+               if (!adc->chan_props[i].adc_tm)
+                       continue;
+               tzd = devm_thermal_of_zone_register(adc->dev, channel,
+                       &adc->chan_props[i], &adc_tm_ops);
It is _very_ useful to register a hwmon too by calling
devm_thermal_add_hwmon_sysfs(). However this becomes tricky, as this
function is not defined in one of the global headers.

This actually points out an issue. You have the ADC driver fused
together with the thermal driver. Can I suggest using the aux device
to split the thermal functionality to the separate driver?

This way it would be possible to use the ADC without any thermal
monitoring in place.


There are a couple of issues which may make it harder to split the thermal functionality from this driver into an auxiliary driver as you mentioned.

For one, we use the same set of registers (offsets 0x4f-0x55) for both VADC function(requesting an immediate channel reading) and ADC_TM function (setting upper/lower thermal thresholds on a channel). To avoid any race conditions, we would need to share a mutex between the top-level ADC driver and the auxiliary ADC_TM thermal driver to avoid concurrently accessing these or any other shared registers.

In addition, the device has only one interrupt with one interrupt handler, and it gets triggered for both VADC and ADC_TM  events (end of conversion and threshold violation, respectively). The handler checks for both types of event and handles it as required.

For the shared interrupt, we may be able to keep the interrupt handler in the top-level driver and just notify the auxiliary TM driver if a threshold violation is detected. For the shared mutex, I think the auxiliary driver may be able to access the parent driver's mutex, but I'll need to check more for the implementation in both of these cases.

Please let me know if you see any problems with this kind of implementation or if you have any additional comments.

Thanks,

Jishnu

+
+               if (IS_ERR(tzd)) {
+                       if (PTR_ERR(tzd) == -ENODEV) {
+                               dev_warn(adc->dev, "thermal sensor on channel %d is not used\n",
+                                        channel);
+                               continue;
+                       }
+






[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