On 18/11/14 08:23, Ivan T. Ivanov wrote: > > On Mon, 2014-11-17 at 23:12 +0100, Hartmut Knaack wrote: >> Ivan T. Ivanov schrieb am 12.11.2014 09:55: >>> On Tue, 2014-11-11 at 23:39 +0100, Hartmut Knaack wrote: >>>> Ivan T. Ivanov schrieb am 11.11.2014 09:21: >>>>> Hi Hartmut, >>>>> >>>>> On Mon, 2014-11-10 at 22:11 +0100, Hartmut Knaack wrote: >>>>>> Ivan T. Ivanov schrieb am 03.11.2014 16:24: >>>>>>> From: Stanimir Varbanov <svarbanov@xxxxxxxxxx> >>>>>>> >>>>>>> The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has >>>>>>> 15 bits resolution and register space inside PMIC accessible across >>>>>>> SPMI bus. >>>>>>> >>>>>>> The vadc driver registers itself through IIO interface. >>>>>> Reviewing again, I got the feeling that due to the complexity of adc reads (writing to >>>>>> register >>>>>> to start conversion, waiting a decent time for the conversion to complete, reading the >>>>>> result), >>>>>> it would be beneficial to use a mutex in vadc_read_raw or its depending functions. >>>>> >>>>> Hm, yes, but there is such a nice info_exist_lock :-) in core functions, >>>>> which in practice serve the same purpose. >>>> I seem to miss that. Please point me in the right direction. >>> >>> I am referring to info_exist_lock mutex part of struct iio_dev. >>> It protects all operations inkern.c, no? >>> >> Good point, thanks for helping me there. > > I was wondering, is there a plan to improve this part of the code? No one is working on it that I know of. All patches welcome! > I mean to remove per device lock and use something like try_module_get(), > when clients are acquiring reference to iio channel? That might indeed provide a more elegant solution... (the things I don't know exist never fail to surprise me!) > >>>>>>> + >>>>>>> + ret = of_property_read_u32(node, "reg", &res); >>>>>> For u16, there would be of_property_read_u16(). >>>>>>> + if (ret < 0) >>>>>>> + return -ENODEV; >>>>>> Just return ret here? >>>>> >>>>> I am usually trying to follow these recommendations[1]. In practice driver >>>>> core cares only for EPROBE_DEFER, ENODEV and ENXIO, while of_property_read_u32() >>>>> can return ENODATA and EOVERFLOW, which did't not make sense for the core. >>>> Please point me in the right direction on this one, too. It is pretty common to pass error >>>> codes >>>> up, as it is also mentioned in [1]. >>> >>> Yes, I know that is common to just pass error codes up, but in this case it did't >>> make too much sense, I think. Also take a look at realy_probe() and line 343. >> This doesn't convince me. Actually, within the probe_failed part, it just doesn't care about >> ENODEV and ENXIO as long as debug messages are disabled (which apart from some developers, is >> default for the vast majority of devices). For all other error codes, it will at least print an >> info or warning about what's going wrong (and that can be a lot of help for debugging). > > Well, if you insist... will change it. > > Thanks, > Ivan > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
