I saw theres a PR started last week https://github.com/apache/incubator-mynewt-core/pull/258
But looks like its still waiting on discussion? On Mon, May 8, 2017 at 12:12 PM, Vipul Rahane <vi...@runtime.io> wrote: > Hello, > > > On May 6, 2017, at 9:29 PM, marko kiiskila <ma...@runtime.io> wrote: > > > > Hi Vipul, > > > >> On May 5, 2017, at 7:10 PM, Vipul Rahane <vi...@runtime.io> wrote: > >> > >> I agree with Marko and I did try initializing the devices using the > init callback in os_dev_create(). This kind of helps in removing the dummy > functions. > >> The fix required changing the data structure for the sensor_mgr sensor > list to be a singly linked list. I am going to open a PR soon with the > changes, after we have consensus on the point below: > >> > >> If we want to separate out device driver creation of board specific > peripherals vs sensor chips, I have couple of suggestions: > >> > >> 1. hal_bsp_ext() which gets called after hal_bsp_init() probably based > on priority. > > > > This I don’t understand. Could you expand how this would work? > > Here is what I was thinking: > > Have sysinit control the flow so that hal_bsp_init() gets called before > hal_bsp_ext_init() but looking at it in detail it seems that would require > a lot of changes including using sysinit for OS initialization which does > not happen currently. > > Another way of doing what I was thinking about doing above is: We already > have _KERNEL stage of devices which get initialized in os_pkt_init() which > happens at the beginning of sysinit() which is always the first thing we do > in main(). We can make hal_bsp_ext_pkg_init() which gets executed at stage > 0 instead of os_pkt_init() and change the stage of os_pkg_init() to be 1. > > This approach would be similar to what I was thinking below. I guess this > is the only way to do it without changing a lot of code in the OS. > > > > >> 2. make sensor_dev_create() part of a separate independent pkg by > itself which would have a priority that makes it get called after > hal_bap_init(). This is because the sensor library might not be the right > place to do the device creation for boards like the Ruuvi which do it in > the bsp cause they have them onboard. > > > > I do understand the convenience of having just a single package where > > you could declare os_dev’s for all plug-in sensors for which a driver > > exists in -core. I think I’d prefer this option (I’m assuming 1 & 2 are > > alternative approaches?) > > Yes, I was thinking of alternative approaches but that is not possible. > > > > >> > >> Regards, > >> Vipul Rahane > >> > >>> On May 5, 2017, at 6:55 PM, marko kiiskila <ma...@runtime.io> wrote: > >>> > >>> Hi, > >>> > >>> I did find it a bit odd to see these device initializations in nrf52dk > BSP, > >>> without matching dependency/syscfg variable declarations in there. > >>> > >>> I think what I’d want is to have BSP have the device creation only > >>> if the board has the particular device on it. And if it has the device > >>> creation, then it also should have the matching dependency to driver, > as well > >>> as the syscfg knob declaration to enable it. > >>> Example of such a board is the Ruuvi Tag. It has a sensor on-board; > >>> BSP should have a knob to enable it’s initialization. BSP also knows > >>> how to reach that sensor. So it should be passing this data to driver > >>> either with a syscfg setting, cfg structure in init or define in a > header > >>> file. One of those. > >>> > >>> nrf52dk does not have this hardware on it, so I feel that in that case > >>> the OS device should be created somewhere else (in the app, > >>> or separate package). Preferably this should happen relatively early, > >>> so package initialized from sysinit() would already be able to > >>> access that device. I don’t know what the right answer is here, > >>> but I don’t think hardware added with jumper wires should be > >>> initialized in the BSP itself. > >>> > >>> Also, it is not great to have empty init routines present for these > >>> sensor devices. I did talk with Vipul about what kind of changes he’d > >>> have to do to allow the driver _init routines to register the sensor > >>> with the framework before the generic sensor framework is initialized. > >>> I think he now has a handle on how to fix this. > >>> > >>> At least this is my current thinking. Anyone else with comments? > >>> > >>>> On May 5, 2017, at 3:18 PM, Jacob Rosenthal <jakerosent...@gmail.com> > wrote: > >>>> > >>>> Cool. So Ill let the bsp do the init. > >>>> > >>>> I am having one issue. For lis2dh, I need to know the current > accel_mode to > >>>> know if the data read back is 8, 10, or 12 bits. From > lis2dh.c:sensor_read > >>>> we get a sensor object and can lookup the config with > SENSOR_GET_DEVICE and > >>>> pass to lis2dh_get_vector_data. However when lis2dh_shell.c attemps to > >>>> reuse that function it doesnt get passed a device, sensor, or a > config ... > >>>> > >>>> I attempted (with my half ass c) to just grab the struct from the > bsp, but > >>>> its not currently in the header or global so Im getting a new > instance and > >>>> breaking my code. I could make the lis2dh device global in the bsp, > but > >>>> thats probably not preferred anyway. Is there an api Im missing, or > one > >>>> that fits here? > >>>> > >>>> relevant code in github > >>>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/ > c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/ > lis2dh/src/lis2dh.c#L372 > >>>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/ > c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/ > lis2dh/src/lis2dh_shell.c#L98 > >>>> > >>>> > >>>> On Fri, May 5, 2017 at 11:42 AM, Kevin Townsend <ke...@adafruit.com> > wrote: > >>>> > >>>>> This is my fault most likely ... the goal at the driver level was > indeed > >>>>> to init with a known default config, but obviously if people think a > >>>>> different solution is better there's no reason not to change it! > >>>>> > >>>>> K. > >>>>> > >>>>> > >>>>> > >>>>> On 05/05/17 20:39, Vipul Rahane wrote: > >>>>> > >>>>>> I do not know the reason why it is done the way it is but I think > you are > >>>>>> right, the reason might have been to initialize with a default > config. > >>>>>> > >>>>>> > >>>>>> On May 4, 2017, at 3:45 PM, Jacob Rosenthal < > jakerosent...@gmail.com> > >>>>>>> wrote: > >>>>>>> > >>>>>>> On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <vi...@runtime.io> > wrote: > >>>>>>> > >>>>>>> I have been thinking about calling the driver initialization > function > >>>>>>>> directly from the bsp > >>>>>>>> > >>>>>>> > >>>>>>> Can you comment on why you're not yet init in the BSP? Im thinking > power > >>>>>>> issues, which is another conversation I started in another thread. > Maybe > >>>>>>> if > >>>>>>> the goal was to init with a config that was as low power as > possible > >>>>>>> maybe > >>>>>>> its ok to do so in the BSP? > >>>>>>> > >>>>>>> May I know what do you mean by the following ? Isn’t it currently > that > >>>>>> way. > >>>>>> > >>>>>> Shouldnt these BSPs have dependencies on the drivers either way, if > only > >>>>>>> when the _PRESENT flag is on > >>>>>>> > >>>>>> Regards, > >>>>>> Vipul Rahane > >>>>>> > >>>>> > >>>>> > >>> > >> > > > > Regards, > Vipul Rahane
