introduce gsm48_lchan_and_pchan2chan_desc()

The function gsm48_lchan2chan_desc_as_configured() dups
gsm48_lchan2chan_desc() with merely a different pchan type
(ts->pchan_from_config instead of ts->pchan_is).

In an upcoming patch, I would like to do the same, just with yet another
pchan value (derived from lchan->type, because that reflects the channel
type even before a dynamic timeslot switched its pchan type).

So replace gsm48_lchan2chan_desc_as_configured() by
gsm48_lchan_and_pchan2chan_desc() with explicit pchan arg;
also call this from gsm48_lchan2chan_desc(), reducing code dup.

gsm48_lchan2chan_desc_as_configured() had more concise error logging.
Absorb that into the new gsm48_lchan_and_pchan2chan_desc().

Add gsm_lchan_and_pchan2chan_nr(), like gsm_lchan2chan_nr() just with
explicit pchan arg, to be able to pass the pchan down from the new
functions mentioned above.

Related: SYS#5559
Change-Id: I67f178c8160cdda1f2ab5513ac4f65c027d4012f

2 files changed, 24 insertions, 28 deletions

diff --git a/src/osmo-bsc/gsm_data.c b/src/osmo-bsc/gsm_data.c
index 4db70f060..e976a5b06 100644
--- a/src/osmo-bsc/gsm_data.c
+++ b/src/osmo-bsc/gsm_data.c
@@ -562,7 +562,7 @@ int gsm_pchan2chan_nr(enum gsm_phys_chan_config pchan,
 /* For RSL, to talk to osmo-bts, we introduce Osmocom specific channel number cbits to indicate VAMOS secondary lchans.
  * However, in RR, which is sent to the MS, these special cbits must not be sent, but their "normal" equivalent; for RR
  * messages, pass allow_osmo_cbits = false. */
-int gsm_lchan2chan_nr(const struct gsm_lchan *lchan, bool allow_osmo_cbits)
+int gsm_lchan_and_pchan2chan_nr(const struct gsm_lchan *lchan, enum gsm_phys_chan_config pchan, bool allow_osmo_cbits)
 {
 	int rc;
 	uint8_t lchan_nr = lchan->nr;
@@ -582,7 +582,7 @@ int gsm_lchan2chan_nr(const struct gsm_lchan *lchan, bool allow_osmo_cbits)
 	 * a primary ts->lchan[0] and a VAMOS ts->lchan[1]. Still, the VAMOS lchan should send chan_nr = 0. */
 	if (lchan->vamos.is_secondary)
 		lchan_nr -= lchan->ts->max_primary_lchans;
-	rc = gsm_pchan2chan_nr(lchan->ts->pchan_is, lchan->ts->nr, lchan_nr,
+	rc = gsm_pchan2chan_nr(pchan, lchan->ts->nr, lchan_nr,
 			       allow_osmo_cbits ? lchan->vamos.is_secondary : false);
 	/* Log an error so that we don't need to add logging to each caller of this function */
 	if (rc < 0)
@@ -593,6 +593,11 @@ int gsm_lchan2chan_nr(const struct gsm_lchan *lchan, bool allow_osmo_cbits)
 	return rc;
 }
 
+int gsm_lchan2chan_nr(const struct gsm_lchan *lchan, bool allow_osmo_cbits)
+{
+	return gsm_lchan_and_pchan2chan_nr(lchan, lchan->ts->pchan_is, allow_osmo_cbits);
+}
+
 static const uint8_t subslots_per_pchan[] = {
 	[GSM_PCHAN_NONE] = 0,
 	[GSM_PCHAN_CCCH] = 0,
@@ -712,35 +717,17 @@ static void _chan_desc_fill_tail(struct gsm48_chan_desc *cd, const struct gsm_lc
 	}
 }
 
-int gsm48_lchan2chan_desc(struct gsm48_chan_desc *cd,
-			  const struct gsm_lchan *lchan,
-			  uint8_t tsc, bool allow_osmo_cbits)
+int gsm48_lchan_and_pchan2chan_desc(struct gsm48_chan_desc *cd,
+				    const struct gsm_lchan *lchan,
+				    enum gsm_phys_chan_config pchan,
+				    uint8_t tsc, bool allow_osmo_cbits)
 {
-	int chan_nr = gsm_lchan2chan_nr(lchan, allow_osmo_cbits);
-	if (chan_nr < 0) {
-		/* Log an error so that we don't need to add logging to each caller of this function */
-		LOG_LCHAN(lchan, LOGL_ERROR, "Error encoding Channel Number\n");
-		return chan_nr;
-	}
-	cd->chan_nr = chan_nr;
-	_chan_desc_fill_tail(cd, lchan, tsc);
-	return 0;
-}
-
-/* like gsm48_lchan2chan_desc() above, but use ts->pchan_from_config to
- * return a channel description based on what is configured, rather than
- * what the current state of the pchan type is */
-int gsm48_lchan2chan_desc_as_configured(struct gsm48_chan_desc *cd,
-					const struct gsm_lchan *lchan,
-					uint8_t tsc)
-{
-	int chan_nr = gsm_pchan2chan_nr(lchan->ts->pchan_from_config, lchan->ts->nr, lchan->nr,
-					lchan->vamos.is_secondary);
+	int chan_nr = gsm_lchan_and_pchan2chan_nr(lchan, pchan, allow_osmo_cbits);
 	if (chan_nr < 0) {
 		/* Log an error so that we don't need to add logging to each caller of this function */
 		LOG_LCHAN(lchan, LOGL_ERROR,
 			  "Error encoding Channel Number: pchan %s ts %u ss %u%s (rc = %d)\n",
-			  gsm_pchan_name(lchan->ts->pchan_from_config), lchan->ts->nr, lchan->nr,
+			  gsm_pchan_name(pchan), lchan->ts->nr, lchan->nr,
 			  lchan->vamos.is_secondary ? " (VAMOS shadow)" : "", chan_nr);
 		return chan_nr;
 	}
@@ -749,6 +736,13 @@ int gsm48_lchan2chan_desc_as_configured(struct gsm48_chan_desc *cd,
 	return 0;
 }
 
+int gsm48_lchan2chan_desc(struct gsm48_chan_desc *cd,
+			  const struct gsm_lchan *lchan,
+			  uint8_t tsc, bool allow_osmo_cbits)
+{
+	return gsm48_lchan_and_pchan2chan_desc(cd, lchan, lchan->ts->pchan_is, tsc, allow_osmo_cbits);
+}
+
 uint8_t gsm_ts_tsc(const struct gsm_bts_trx_ts *ts)
 {
 	if (ts->tsc != -1)
diff --git a/src/osmo-bsc/system_information.c b/src/osmo-bsc/system_information.c
index e8b3b7d7d..974af3a84 100644
--- a/src/osmo-bsc/system_information.c
+++ b/src/osmo-bsc/system_information.c
@@ -1017,8 +1017,10 @@ static int generate_si4(enum osmo_sysinfo_type t, struct gsm_bts *bts)
 		const struct gsm_bts_trx_ts *ts = cbch_lchan->ts;
 		struct gsm48_chan_desc cd;
 
-		/* 10.5.2.5 (TV) CBCH Channel Description IE */
-		if (gsm48_lchan2chan_desc_as_configured(&cd, cbch_lchan, gsm_ts_tsc(cbch_lchan->ts)))
+		/* 10.5.2.5 (TV) CBCH Channel Description IE.
+		 * CBCH is never in VAMOS mode, so just pass allow_osmo_cbits == false. */
+		if (gsm48_lchan_and_pchan2chan_desc(&cd, cbch_lchan, cbch_lchan->ts->pchan_from_config,
+						    gsm_ts_tsc(cbch_lchan->ts), false))
 			return -EINVAL;
 		tail = tv_fixed_put(tail, GSM48_IE_CBCH_CHAN_DESC,
 				    sizeof(cd), (uint8_t *) &cd);