tnt has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-e1-hardware/+/26850 )
Change subject: icE1usb fw: Use helper functions to derive config value from struct ......................................................................
icE1usb fw: Use helper functions to derive config value from struct
Just from this patch, it's not clear why this is better, but it makes things a bit cleaner in upcoming patches
Signed-off-by: Sylvain Munaut tnt@246tNt.com Change-Id: I4c75bb4eb20c1d1aaa1695e95bdf0417bbf3bf76 --- M firmware/ice40-riscv/icE1usb/usb_e1.c 1 file changed, 13 insertions(+), 18 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-e1-hardware refs/changes/50/26850/1
diff --git a/firmware/ice40-riscv/icE1usb/usb_e1.c b/firmware/ice40-riscv/icE1usb/usb_e1.c index 08b192b..9dc6f1d 100644 --- a/firmware/ice40-riscv/icE1usb/usb_e1.c +++ b/firmware/ice40-riscv/icE1usb/usb_e1.c @@ -236,25 +236,19 @@ return USB_FND_SUCCESS; }
-static void _perform_tx_config(int port) +static uint32_t +_tx_config_reg(const struct ice1usb_tx_config *cfg) { - struct usb_e1_state *usb_e1 = _get_state(port); - const struct ice1usb_tx_config *cfg = &usb_e1->tx_cfg; - e1_tx_config(port, - ((cfg->mode & 3) << 1) | + return ((cfg->mode & 3) << 1) | ((cfg->timing & 1) << 3) | ((cfg->alarm & 1) << 4) | - ((cfg->ext_loopback & 3) << 5) - ); + ((cfg->ext_loopback & 3) << 5); }
-static void _perform_rx_config(int port) +static uint32_t +_rx_config_reg(const struct ice1usb_rx_config *cfg) { - struct usb_e1_state *usb_e1 = _get_state(port); - const struct ice1usb_rx_config *cfg = &usb_e1->rx_cfg; - e1_rx_config(port, - (cfg->mode << 1) - ); + return (cfg->mode << 1); }
static enum usb_fnd_resp @@ -300,9 +294,10 @@
case true: /* Reset and Re-Enable E1 */ - e1_init(port, 0, 0); - _perform_rx_config(port); - _perform_tx_config(port); + e1_init(port, + _rx_config_reg(&rx_cfg_default), + _tx_config_reg(&tx_cfg_default) + );
/* Reset BDI */ usb_e1->in_bdi = 0; @@ -352,7 +347,7 @@ printf("set_tx_mode[%d] %02x%02x%02x%02x\r\n", port, xfer->data[0], xfer->data[1], xfer->data[2], xfer->data[3]); usb_e1->tx_cfg = *cfg; - _perform_tx_config(port); + e1_tx_config(port, _tx_config_reg(cfg)); return true; }
@@ -365,7 +360,7 @@ struct usb_e1_state *usb_e1 = _get_state(port); printf("set_rx_mode[%d] %02x\r\n", port, xfer->data[0]); usb_e1->rx_cfg = *cfg; - _perform_rx_config(port); + e1_rx_config(port, _rx_config_reg(cfg)); return true; }
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-e1-hardware/+/26850 )
Change subject: icE1usb fw: Use helper functions to derive config value from struct ......................................................................
Patch Set 1: Code-Review+2
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-e1-hardware/+/26850 )
Change subject: icE1usb fw: Use helper functions to derive config value from struct ......................................................................
icE1usb fw: Use helper functions to derive config value from struct
Just from this patch, it's not clear why this is better, but it makes things a bit cleaner in upcoming patches
Signed-off-by: Sylvain Munaut tnt@246tNt.com Change-Id: I4c75bb4eb20c1d1aaa1695e95bdf0417bbf3bf76 --- M firmware/ice40-riscv/icE1usb/usb_e1.c 1 file changed, 13 insertions(+), 18 deletions(-)
Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved
diff --git a/firmware/ice40-riscv/icE1usb/usb_e1.c b/firmware/ice40-riscv/icE1usb/usb_e1.c index 08b192b..9dc6f1d 100644 --- a/firmware/ice40-riscv/icE1usb/usb_e1.c +++ b/firmware/ice40-riscv/icE1usb/usb_e1.c @@ -236,25 +236,19 @@ return USB_FND_SUCCESS; }
-static void _perform_tx_config(int port) +static uint32_t +_tx_config_reg(const struct ice1usb_tx_config *cfg) { - struct usb_e1_state *usb_e1 = _get_state(port); - const struct ice1usb_tx_config *cfg = &usb_e1->tx_cfg; - e1_tx_config(port, - ((cfg->mode & 3) << 1) | + return ((cfg->mode & 3) << 1) | ((cfg->timing & 1) << 3) | ((cfg->alarm & 1) << 4) | - ((cfg->ext_loopback & 3) << 5) - ); + ((cfg->ext_loopback & 3) << 5); }
-static void _perform_rx_config(int port) +static uint32_t +_rx_config_reg(const struct ice1usb_rx_config *cfg) { - struct usb_e1_state *usb_e1 = _get_state(port); - const struct ice1usb_rx_config *cfg = &usb_e1->rx_cfg; - e1_rx_config(port, - (cfg->mode << 1) - ); + return (cfg->mode << 1); }
static enum usb_fnd_resp @@ -300,9 +294,10 @@
case true: /* Reset and Re-Enable E1 */ - e1_init(port, 0, 0); - _perform_rx_config(port); - _perform_tx_config(port); + e1_init(port, + _rx_config_reg(&rx_cfg_default), + _tx_config_reg(&tx_cfg_default) + );
/* Reset BDI */ usb_e1->in_bdi = 0; @@ -352,7 +347,7 @@ printf("set_tx_mode[%d] %02x%02x%02x%02x\r\n", port, xfer->data[0], xfer->data[1], xfer->data[2], xfer->data[3]); usb_e1->tx_cfg = *cfg; - _perform_tx_config(port); + e1_tx_config(port, _tx_config_reg(cfg)); return true; }
@@ -365,7 +360,7 @@ struct usb_e1_state *usb_e1 = _get_state(port); printf("set_rx_mode[%d] %02x\r\n", port, xfer->data[0]); usb_e1->rx_cfg = *cfg; - _perform_rx_config(port); + e1_rx_config(port, _rx_config_reg(cfg)); return true; }