I think it makes sense :-) There are some things I am not quite sure I get…
So I think the pointer approach like you describe below is the better way, with
the possible exception of gpio (I will explain that later). Having a
hal_spi_dev * or hal_adc_dev * passed to the hal api is a common way to do it
and I like it better than the sysid approach as you dont have the extra
overhead for each call.
The possible exception to this is gpio and is related to the part I dont get,
which is ram/code usage.
Consider a simple MCU with 3 spi ports and 64 gpio. For the SPI, the BSP would
have three hal_spi_dev structures (devid 0, 1, and 2) defined. These are in
.text and each take 8 bytes (stores a pointer and the device id). We need 4
bytes of RAM per SPI. We also need the driver structure (in flash). We only
need one of these and it is a set of N function pointers (N * 4 bytes in .text).
Now consider the gpio. There would be 64 of these 8 byte data structures (in.
text)? And we would need (worst-case), 64*4 bytes in RAM? And also N*4 bytes of
.text for each gpio API? I do realize you dont have to define all 64...
Folks probably wont like this idea; heck, I am not even sure I do… but I think
in most cases a simple gpio interface suffices. GPIO are logically ordered
(i.e. they have a sysid) and the code in hal_gpio.c in hw/mcu knows how to map
that logical gpio for the MCU. The BSP maps the sysid to something the
application can use. Basically, what we have today for gpio.
Maybe I just should not worry about the ram/code usage for gpio (but I do) :-)
> On Mar 23, 2016, at 7:58 PM, p...@wrada.com wrote:
>
> I was thinking the same as you. I had one thought for a way to get this
> functionality. Im actually pretty excited about it. Hope this makes sense.
>
> We could declare a structure in hal_xxx.h
>
> struct hal_xxx_device;
>
> This could be the ³this" argument to all the hal_xxx functions.
>
> Example
>
> int hal_adc_read( struct hal_adc_device *padc);
>
>
> In the hal_xxx_int.h we can define this like this.
>
> Struct hal_xxx_driver Š (same as we have now)..
>
> Struct hal_xxx_device {
> Struct hal_xxx_driver *pdrv;
> uint8_t devid; /* really only need 8 bits but
> structures PAD */
> uint8_t reserved[3];
> }
>
> And then define the shims as something like in hal_adc_int.c:
>
> inline int hal_adc_read(struct hal_xxx_device *pd) {
> If(pd && pd->pdrv && pd->pdrv->adc_read) {
> return pd->pdrv->adc_read(pd->pdrv, pd->devid);
> }
> Return -1;
> }
>
>
> Whether these are inline or not I don¹t care as long as the are NOT
> MACROS.
>
> Now suppose I am writing a library. The library needs some hal devices to
> do battery charging control.
>
> Typedef struct my_battery_library_state {
> Extern const struct hal_*gpio_device pon; // to set the
> LED status of
> the battery
> Extern const struct hal_*adc_device ptemp; // to read the
> battery
> temperature
> Extern const struct hal_*adc_device pvoltage; // to read the
> battery
> voltage
> Extern const struct hal_*pwm_device pcurrent; // to set the
> output
> current level
> }
>
> The library would have to get these values from the BSP via function call.
> Undefined variables won¹t consistently work in case the library author
> intends there to be multiple battery chargers.
>
> int bsp_get_power_library_led_gpio(int charger_instance, Struct
> hal_gpio_device **out);
>
> The bsp could typically implement as a const structure (since in most
> cases this is probably fixed by the board design).
>
>
>
> Const struct hal_adc_device temp_adc_for_battery_monitor = {
>
> _adc_controller, /* this type of driver */
> 0, /* device ID 0 */
> }
>
> int bsp_get_power_library_led_gpio(int instance, const struct
> hal_gpio_device **out) {
> /* This BSP only knows how to provision one battery monitor. */
> If(instance != 0) {
> Return -1;
> }
> *out = & temp_adc_for_battery_monitor;
> Return 0;
> }
>
>
>
>
> Analysis:
>
> For the sake of analysis, I¹ll consider two things using the HAL:
> 1) A library which doesn¹t know at compile time which hal_device it will
> use.
> 2) An application which does know at compile time.
>
> * the structure definition could be totally hidden from the application
> writer with the hal_shim. They just need to keep a pointer.
> * This structure API is probably faster than the sysid API since we don¹t
> have the BSP dispatch on every API call.
> * This would use 8 bytes of code space for each device (const) used and
> simple function to fetch the device from the library.
>
> * This uses way less code space as we don¹t have all the shim code for
> every hal. The hal shim could be a simple inline like above.
> * This would dispense with
