laforge has submitted this change. ( 
https://gerrit.osmocom.org/c/osmocom-bb/+/33532?usp=email )

Change subject: mobile: Fix PCS ARFCN handling: PCS can only be ARFCN 512..810
......................................................................

mobile: Fix PCS ARFCN handling: PCS can only be ARFCN 512..810

While it is correct to use the band indicator from SI1 rest octets,
it may only be applied for ARFCN values in the range 512..810.

The function gsm_refer_pcs() is used to determine, if the cell (which
'talks' about ARFCNs) refers to them PCS or DCS channels. It returns
true, if it refers to PCS, but this only means that ARFCNs in the range
512..810 are PCS channels, not all ARFCNs.

The new function gsm_arfcn_refer_pcs() is used to add the PCS flag to an
ARFCN, if the given cell refers to PCS and the given ARFCN is in the PCS
range 512..810.

Change-Id: Id99c8534bf853f4f24f99364790c1ac1df6cc007
Related: OS#6078
---
M src/host/layer23/include/osmocom/bb/common/sysinfo.h
M src/host/layer23/src/common/sysinfo.c
M src/host/layer23/src/mobile/gsm322.c
M src/host/layer23/src/mobile/gsm48_rr.c
4 files changed, 71 insertions(+), 41 deletions(-)

Approvals:
  fixeria: Looks good to me, but someone else must approve
  laforge: Looks good to me, but someone else must approve
  Jenkins Builder: Verified
  jolly: Looks good to me, approved




diff --git a/src/host/layer23/include/osmocom/bb/common/sysinfo.h 
b/src/host/layer23/include/osmocom/bb/common/sysinfo.h
index 9efb248..c31cf9d 100644
--- a/src/host/layer23/include/osmocom/bb/common/sysinfo.h
+++ b/src/host/layer23/include/osmocom/bb/common/sysinfo.h
@@ -71,7 +71,7 @@
        /* si1 rest */
        uint8_t                         nch;
        uint8_t                         nch_position;
-       uint8_t                         band_ind; /* set for DCS */
+       bool                            band_ind; /* set for DCS */

        /* si3 rest */
        uint8_t                         sp;
@@ -181,7 +181,8 @@
 };

 char *gsm_print_arfcn(uint16_t arfcn);
-uint8_t gsm_refer_pcs(uint16_t arfcn, const struct gsm48_sysinfo *s);
+bool gsm_refer_pcs(uint16_t cell_arfcn, const struct gsm48_sysinfo *cell_s);
+uint16_t gsm_arfcn_refer_pcs(uint16_t cell_arfcn, const struct gsm48_sysinfo 
*cell_s, uint16_t arfcn);
 int gsm48_sysinfo_dump(const struct gsm48_sysinfo *s, uint16_t arfcn,
                       void (*print)(void *, const char *, ...),
                       void *priv, uint8_t *freq_map);
diff --git a/src/host/layer23/src/common/sysinfo.c 
b/src/host/layer23/src/common/sysinfo.c
index f541c85..7fdc6ff 100644
--- a/src/host/layer23/src/common/sysinfo.c
+++ b/src/host/layer23/src/common/sysinfo.c
@@ -50,20 +50,35 @@
        return text;
 }

-/* check if the cell 'talks' about DCS (0) or PCS (1) */
-uint8_t gsm_refer_pcs(uint16_t arfcn, const struct gsm48_sysinfo *s)
+/* Check if the cell 'talks' about DCS (false) or PCS (true) */
+bool gsm_refer_pcs(uint16_t cell_arfcn, const struct gsm48_sysinfo *cell_s)
 {
        /* If ARFCN is PCS band, the cell refers to PCS */
-       if ((arfcn & ARFCN_PCS))
-               return 1;
+       if ((cell_arfcn & ARFCN_PCS))
+               return true;

        /* If no SI1 is available, we assume DCS. Be sure to call this
         * function only if SI 1 is available. */
-       if (!s->si1)
+       if (!cell_s->si1)
                return 0;

        /* If band indicator indicates PCS band, the cell refers to PCSThe  */
-       return s->band_ind;
+       return cell_s->band_ind;
+}
+
+/* Change the given ARFCN to PCS ARFCN, if it is in the PCS channel range and 
the cell refers to PCS band. */
+uint16_t gsm_arfcn_refer_pcs(uint16_t cell_arfcn, const struct gsm48_sysinfo 
*cell_s, uint16_t arfcn)
+{
+       /* If ARFCN is not one of the overlapping channel of PCS and DCS. */
+       if (arfcn < 512 && arfcn > 810)
+               return arfcn;
+
+       /* If the 'cell' does not refer to PCS. */
+       if (!gsm_refer_pcs(cell_arfcn, cell_s))
+               return arfcn;
+
+       /* The ARFCN is PCS, because the ARFCN is in the PCS range and the cell 
refers to it. */
+       return arfcn | ARFCN_PCS;
 }

 int gsm48_sysinfo_dump(const struct gsm48_sysinfo *s, uint16_t arfcn,
@@ -541,10 +556,7 @@
                s->nch_position = bitvec_get_uint(&bv, 5);
        } else
                s->nch = 0;
-       if (bitvec_get_bit_high(&bv) == H)
-               s->band_ind = 1;
-       else
-               s->band_ind = 0;
+       s->band_ind = (bitvec_get_bit_high(&bv) == H);

        return 0;
 }
diff --git a/src/host/layer23/src/mobile/gsm322.c 
b/src/host/layer23/src/mobile/gsm322.c
index 35811d3..3d0cc77 100644
--- a/src/host/layer23/src/mobile/gsm322.c
+++ b/src/host/layer23/src/mobile/gsm322.c
@@ -2419,7 +2419,8 @@
        struct gsm322_cellsel *cs = &ms->cellsel;
        struct gsm48_sysinfo *s;
        struct gsm322_ba_list *ba = NULL;
-       int i, refer_pcs;
+       int i;
+       bool refer_pcs;
        uint8_t freq[128+38];

        if (!cs) {
@@ -2472,7 +2473,8 @@
        struct gsm48_sysinfo *s)
 {
        struct gsm322_ba_list *ba;
-       int i, refer_pcs;
+       int i;
+       bool refer_pcs;
        uint8_t freq[128+38];

        /* find or create ba list */
@@ -3408,8 +3410,7 @@
 }

 /* create temporary ba range with given frequency ranges */
-struct gsm322_ba_list *gsm322_cs_ba_range(struct osmocom_ms *ms,
-       uint32_t *range, uint8_t ranges, uint8_t refer_pcs)
+static struct gsm322_ba_list *gsm322_cs_ba_range(struct osmocom_ms *ms, 
uint32_t *range, uint8_t ranges, bool refer_pcs)
 {
        static struct gsm322_ba_list ba;
        int lower, higher;
@@ -4403,7 +4404,8 @@
        uint8_t map[128];
        uint16_t nc[32];
        uint8_t changed = 0;
-       int refer_pcs, index;
+       bool refer_pcs;
+       int index;
        uint16_t arfcn;

        if (cs->ms->settings.no_neighbour)
diff --git a/src/host/layer23/src/mobile/gsm48_rr.c 
b/src/host/layer23/src/mobile/gsm48_rr.c
index 97ba173..6ba57e3 100644
--- a/src/host/layer23/src/mobile/gsm48_rr.c
+++ b/src/host/layer23/src/mobile/gsm48_rr.c
@@ -2600,7 +2600,8 @@
         && s->si5
         && (!s->nb_ext_ind_si5 || s->si5bis)) {
                struct gsm48_rr_meas *rrmeas = &ms->rrlayer.meas;
-               int i, refer_pcs;
+               int i;
+               bool refer_pcs;
                int16_t arfcn;

                LOGP(DRR, LOGL_NOTICE, "Complete set of SI5* for BA(%d)\n",
@@ -3385,8 +3386,7 @@
        } else {
                cd.h = 0;
                gsm48_decode_chan_h0(&ia->chan_desc, &cd.tsc, &cd.arfcn);
-               if (gsm_refer_pcs(cs->arfcn, s))
-                       cd.arfcn |= ARFCN_PCS;
+               cd.arfcn = gsm_arfcn_refer_pcs(cs->arfcn, s, cd.arfcn);
                LOGP(DRR, LOGL_INFO, " (ta %d/%dm ra 0x%02x chan_nr 0x%02x "
                        "ARFCN %s TS %u SS %u TSC %u)\n",
                        ia->timing_advance,
@@ -3504,8 +3504,7 @@
        } else {
                cd1.h = 0;
                gsm48_decode_chan_h0(&ia->chan_desc1, &cd1.tsc, &cd1.arfcn);
-               if (gsm_refer_pcs(cs->arfcn, s))
-                       cd1.arfcn |= ARFCN_PCS;
+               cd1.arfcn = gsm_arfcn_refer_pcs(cs->arfcn, s, cd1.arfcn);
                LOGP(DRR, LOGL_INFO, " assignment 1 (ta %d/%dm ra 0x%02x "
                        "chan_nr 0x%02x ARFCN %s TS %u SS %u TSC %u)\n",
                        ia->timing_advance1,
@@ -3533,8 +3532,7 @@
        } else {
                cd2.h = 0;
                gsm48_decode_chan_h0(&ia->chan_desc2, &cd2.tsc, &cd2.arfcn);
-               if (gsm_refer_pcs(cs->arfcn, s))
-                       cd2.arfcn |= ARFCN_PCS;
+               cd2.arfcn = gsm_arfcn_refer_pcs(cs->arfcn, s, cd2.arfcn);
                LOGP(DRR, LOGL_INFO, " assignment 2 (ta %d/%dm ra 0x%02x "
                        "chan_nr 0x%02x ARFCN %s TS %u SS %u TSC %u)\n",
                        ia->timing_advance2,
@@ -3998,8 +3996,8 @@
        struct gsm322_cellsel *cs = &ms->cellsel;
        struct gsm48_sysinfo *s = cs->si;
        struct gsm_settings *set = &ms->settings;
-       int i, pcs, index;
-       uint16_t arfcn;
+       int i, index;
+       uint16_t arfcn, pcs;

        pcs = gsm_refer_pcs(cs->arfcn, s) ? ARFCN_PCS : 0;

@@ -4102,7 +4100,9 @@

        /* convert to band_arfcn and check for unsported frequency */
        for (i = 0; i < *ma_len; i++) {
-               arfcn = ma[i] | pcs;
+               arfcn = ma[i];
+               if (arfcn >= 512 && arfcn <= 810)
+                       arfcn |= pcs;
                ma[i] = arfcn;
                index = arfcn2index(arfcn);
                if (!(set->freq_map[index >> 3] & (1 << (index & 7)))) {
@@ -4490,8 +4490,7 @@
        } else {
                cd.h = 0;
                gsm48_decode_chan_h0(&fr->chan_desc, &cd.tsc, &cd.arfcn);
-               if (gsm_refer_pcs(cs->arfcn, s))
-                       cd.arfcn |= ARFCN_PCS;
+               cd.arfcn = gsm_arfcn_refer_pcs(cs->arfcn, s, cd.arfcn);
                LOGP(DRR, LOGL_INFO, " (ARFCN %s TS %u SS %u TSC %u)\n",
                        gsm_print_arfcn(cd.arfcn), ch_ts, ch_subch, cd.tsc);
        }
@@ -4600,8 +4599,7 @@
        } else {
                cd->h = 0;
                gsm48_decode_chan_h0(&cm->chan_desc, &cd->tsc, &cd->arfcn);
-               if (gsm_refer_pcs(cs->arfcn, s))
-                       cd->arfcn |= ARFCN_PCS;
+               cd->arfcn = gsm_arfcn_refer_pcs(cs->arfcn, s, cd->arfcn);
                LOGP(DRR, LOGL_INFO, " (chan_nr 0x%02x ARFCN %s TS %u SS %u "
                        "TSC %u mode %u)\n", cm->chan_desc.chan_nr,
                        gsm_print_arfcn(cd->arfcn), ch_ts, ch_subch, cd->tsc,
@@ -4744,8 +4742,7 @@
                } else {
                        cdb->h = 0;
                        gsm48_decode_chan_h0(ccd, &cdb->tsc, &cdb->arfcn);
-                       if (gsm_refer_pcs(cs->arfcn, s))
-                               cdb->arfcn |= ARFCN_PCS;
+                       cdb->arfcn = gsm_arfcn_refer_pcs(cs->arfcn, s, 
cdb->arfcn);
                        LOGP(DRR, LOGL_INFO, " before: (chan_nr 0x%02x "
                                "ARFCN %s TS %u SS %u TSC %u)\n", ccd->chan_nr,
                                gsm_print_arfcn(cdb->arfcn),
@@ -4772,8 +4769,7 @@
        } else {
                cda->h = 0;
                gsm48_decode_chan_h0(&ac->chan_desc, &cda->tsc, &cda->arfcn);
-               if (gsm_refer_pcs(cs->arfcn, s))
-                       cda->arfcn |= ARFCN_PCS;
+               cda->arfcn = gsm_arfcn_refer_pcs(cs->arfcn, s, cda->arfcn);
                LOGP(DRR, LOGL_INFO, " after: (chan_nr 0x%02x ARFCN %s TS %u "
                        "SS %u TSC %u)\n", ac->chan_desc.chan_nr,
                        gsm_print_arfcn(cda->arfcn), ch_ts, ch_subch, cda->tsc);
@@ -5149,8 +5145,7 @@
                } else {
                        cdb->h = 0;
                        gsm48_decode_chan_h0(ccd, &cdb->tsc, &cdb->arfcn);
-                       if (gsm_refer_pcs(cs->arfcn, s))
-                               cdb->arfcn |= ARFCN_PCS;
+                       cdb->arfcn = gsm_arfcn_refer_pcs(cs->arfcn, s, 
cdb->arfcn);
                        LOGP(DRR, LOGL_INFO, " before: (chan_nr 0x%02x "
                                "ARFCN %s TS %u SS %u TSC %u)\n", ccd->chan_nr,
                                gsm_print_arfcn(cdb->arfcn),
@@ -5177,8 +5172,7 @@
        } else {
                cda->h = 0;
                gsm48_decode_chan_h0(&ho->chan_desc, &cda->tsc, &cda->arfcn);
-               if (gsm_refer_pcs(cs->arfcn, s))
-                       cda->arfcn |= ARFCN_PCS;
+               cda->arfcn = gsm_arfcn_refer_pcs(cs->arfcn, s, cda->arfcn);
                LOGP(DRR, LOGL_INFO, " after: (chan_nr 0x%02x ARFCN %s TS %u "
                        "SS %u TSC %u)\n", ho->chan_desc.chan_nr,
                        gsm_print_arfcn(cda->arfcn), ch_ts, ch_subch, cda->tsc);
@@ -5737,8 +5731,7 @@
        } else {
                cd.h = 0;
                gsm48_decode_chan_h0(ch_desc, &cd.tsc, &cd.arfcn);
-               if (gsm_refer_pcs(cs->arfcn, s))
-                       cd.arfcn |= ARFCN_PCS;
+               cd.arfcn = gsm_arfcn_refer_pcs(cs->arfcn, s, cd.arfcn);
                LOGP(DRR, LOGL_INFO, " (chan_nr 0x%02x ARFCN %s TS %u SS %u TSC 
%u)\n",
                     ch_desc->chan_nr, gsm_print_arfcn(cd.arfcn), ch_ts, 
ch_subch, cd.tsc);
        }

--
To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33532?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Change-Id: Id99c8534bf853f4f24f99364790c1ac1df6cc007
Gerrit-Change-Number: 33532
Gerrit-PatchSet: 4
Gerrit-Owner: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de>
Gerrit-Reviewer: jolly <andr...@eversberg.eu>
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-MessageType: merged

Reply via email to