OsmocomBB

On Sun, Mar 05, 2017 at 08:08:08AM +0000, fixeria [REDMINE] wrote:

The following information we can provide to layer23 applications:

pleae provide some more background on the usage of those fields

struct l1ctl_l1_info_ind {
/* HW info */
uint8_t hw_dev_id;
uint8_t hw_dev_ver;
uint8_t hw_dev_arm_ver;
uint8_t hw_cdsp_ver;

is it well-defined what the above fields are used for? Are we sure that
uint8_t is sufficient for those? Are all of the values directly read
from hardware? If so, where from?

/* Supported features */
uint8_t tx_support;
uint8_t sim_support;
uint8_t burst_ind_support;
uint8_t transceiver_support;

I would structure this in a way that it can be extended in the future.
One option would be to do something similar to the 'classmark' feature
on GSM, where the phone indicates a variable-length bitmask to the
network, and new bits can be added at the end. Old software will simply
only read the bits it understands and ignore the bits at the end. As we
don't care about the size of the structure, we could do it with bytes
instead of bits, but that's a matter of taste.

so generally I would counter-propose:

struct l1ctl_l1_info_ind {
    /* version of this structure, if we ever have to change it */
    uint16_t version;
    struct {
        uint8_t dev_id;
        uint8_t dev_ver;
        uint8_t dev_arm_ver;
        uint8_t cdsp_ver;
    } hw;
    struct {
        uint16_t len;
        uint8_t data[0];
    } features;
} __attribute__((packed));

so then the code can have a variable-length list of features, and you
can add more over time, with forward/backward compatibility

Any ideas? Maybe something else?

• build target (e88/e99/...)
• git version of firmware compile
• whether code is running from flash or ram