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. 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 >
