Chris and Vipul: That does indeed seem to be a decent option! I will see if others chime in and if not, add that change.
Thanks!!! > On Jun 6, 2018, at 1:32 PM, Vipul Rahane <vi...@runtime.io> wrote: > > >> On Jun 6, 2018, at 1:01 PM, Christopher Collins <ch...@runtime.io> wrote: >> >> On Wed, Jun 06, 2018 at 11:57:34AM -0700, will sanfilippo wrote: >>> Chris: >>> >>> I might be missing something here, but given that os_dev already has a >>> reference count and that handles multiple folks opening/closing the device, >>> does the underlying adc driver need a reference count itself? If it just >>> returned no error if opened again this would be fine. >>> >>> I do note that os_dev_open() and os_dev_close() always call the open/close >>> handlers regardless of reference count. I wonder if that should be changed >>> (meaning only call open/close once)? >> >> No, you aren't missing anything; I just misunderstood the os_dev >> reference counting. Thanks for setting me straight :). >> >> Another option: the ADC open function checks its os_dev's reference >> count. If the value is greater than zero, then return without doing >> anything. >> > > I was going to suggest that as well. Seems like a simple solution :-) > >> Chris >> >>> >>> >>>> On Jun 6, 2018, at 10:13 AM, Christopher Collins <ch...@runtime.io> wrote: >>>> >>>> On Wed, Jun 06, 2018 at 08:50:34AM -0700, will sanfilippo wrote: >>>>> Hello: >>>>> >>>>> I am not the most familiar with the ADC device so it is possible that it >>>>> was being used incorrectly but in any event I ran across something I >>>>> wanted to discuss. The call to os_dev_open() allows a device to be opened >>>>> multiple times (there is a reference count there). However, the call to >>>>> nrf52_adc_open() returns an error (OS_EBUSY) if the device has already >>>>> been opened. >>>>> >>>>> This presented a problem in the following case: consider two independent >>>>> packages both of which want to use ADC_0. Each package is going to >>>>> attempt to open the ADC device (since it has no idea if it was already >>>>> opened) but the second attempt to open the device will result in an error >>>>> code returned. Depending on how the code is written in the package, this >>>>> could be a problem. Given that an ADC is almost always a mutli-channel >>>>> peripheral (one adc device has multple channels) I would suspect the >>>>> above case to be common: multiple packages wanting an ADC channel from a >>>>> single device. >>>>> >>>>> I am not sure if anything needs to be done here; just wanted to see if >>>>> folks thought there should different behavior with regards to the >>>>> function returning an error if the device was already opened. If not, >>>>> folks are going to have to be careful when they write code using the adc >>>>> device. Seems to me if nothing is going to change we have two options: >>>>> >>>>> 1) The device gets created and opened in some place and handed to the >>>>> packages that need it. >>>>> 2) The device gets created (say by the bsp) and each package can attempt >>>>> to open the device. If os_dev_lookup() returns !NULL but os_dev_open() >>>>> returns NULL it means that the device has already been opened. >>>>> >>>>> Something about #2 just sort of bothers me. I do not like ambiguous stuff >>>>> like that; how do you know if there was an error for another reason? >>>> >>>> Why not: >>>> >>>> 3) Make the ADC driver consistent with other drivers by adding a >>>> reference count. >>>> >>>> ? >>>> >>>> I know something less than nothing about the ADC code, so I could >>>> certainly be missing something. >>>> >>>> Chris >>> > > - Vipul
