Hello, > On May 6, 2017, at 9:29 PM, marko kiiskila <[email protected]> wrote: > > Hi Vipul, > >> On May 5, 2017, at 7:10 PM, Vipul Rahane <[email protected]> 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 <[email protected]> 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 <[email protected]> >>>> 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 <[email protected]> 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 <[email protected]> >>>>>>> wrote: >>>>>>> >>>>>>> On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <[email protected]> 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
