> 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
