On Thu, Feb 27, 2025 at 11:58:24AM +0000, Ding, Shenghao wrote:
> > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > Sent: Tuesday, February 25, 2025 10:41 PM
> > On Tue, Feb 25, 2025 at 10:03:16PM +0800, Shenghao Ding wrote:
...
> > > +
> > > + for (i = 0; i < 20; i++) {
> > > + const unsigned char *dat = &p->data[24 * i];
> >
> > Okay, you have 24-byte records.
> >
> > > + if (dat[23] != 1)
> > > + break;
> >
> > > + if (strstr(dat, "umg_SsmKEGCye") != NULL) {
> >
> > These calls basically suggest that the data may be not at the same offset.
> > But at the same time they don't have boundary checks and there is a chance
> > that the tail of one of these string comparisons may trigger if the value appears
> > to be the same, like "Cye".
> >
> > Instead, better to introduce a special data type or even data types, where you
> > put string with limited length and respective data, like u24/i8 in your
> > terminology. With that this code becomes much more clearer.
>
> Calibration parameters locations and data schema in dsp firmware
> The number of items are flexible, but not more than 20. The dsp tool
> will reserve 20*24-byte space for fct params. In some cases, the
> number of fct param is less than 20, the data will be saved from the
> beginning, the rest part will be stuffed with zero.
> fct_param_num (not more than 20)
> for (i = 0; i < fct_param_num; i++) {
> Alias of fct param (20 bytes)
> Book (1 byte)
> Page (1 byte)
> Offset (1 byte)
> CoeffLength (1 byte) = 0x1
> }
> if (20 - fct_param_num)
> 24*(20 - fct_param_num) pieces of '0' as stuffing
> As follow
> umg_SsmKEGCye = Book, Page, Offset, CoeffLength
> iks_E0 = Book, Page, Offset, CoeffLength
> yep_LsqM0 = Book, Page, Offset, CoeffLength
> oyz_U0_ujx = Book, Page, Offset, CoeffLength
> iks_GC_GMgq = Book, Page, Offset, CoeffLength
> gou_Yao = Book, Page, Offset, CoeffLength
> kgd_Wsc_Qsbp = Book, Page, Offset, CoeffLength
> yec_CqseSsqs = Book, Page, Offset, CoeffLength
> iks_SogkGgog2 = Book, Page, Offset, CoeffLength
> yec_Sae_Y = Book, Page, Offset, CoeffLength
> Re_Int = Book, Page, Offset, CoeffLength
> SigFlag = Book, Page, Offset, CoeffLength
> a1_Int = Book, Page, Offset, CoeffLength
> a2_Int = Book, Page, Offset, CoeffLength
>
> Do you mean to use strncmp instead of strstr?
Yes,
> Due to the different length of the alias of fct params,
> I have to pass the max len, 20 bytes, as the str len.
Yes, for each alias you need just to define a data type:
struct fct_param_alias_u24 {
const char name[20];
u8 book;
u8 page;
u8 offset;
u8 coeff_len;
};
struct fct_param_alias_u8 {
...
};
Then in the loop you do the following (just an example, can be done
differently):
union {
const char name[20];
struct fct_param_alias_u8 *u8;
struct fct_param_alias_u24 *u24;
...
const void *data;
} a;
a.data = data;
if (!strncmp(a.name, ..., sizeof(a.name)))
foo = ...(a.u24->book, a.u24->page, a.u24->offset);
else if (...)
...
Also parameters can be split to a table which will have the need to use method
as a callback
> > > + hex_parse_u24(&r->pow_reg, &dat[20]);
> > > + continue;
> > > + }
> > > + if (strstr(dat, "iks_E0") != NULL) {
> > > + hex_parse_u24(&r->r0_reg, &dat[20]);
> > > + continue;
> > > + }
> > > + if (strstr(dat, "yep_LsqM0") != NULL) {
> > > + hex_parse_u24(&r->invr0_reg, &dat[20]);
> > > + continue;
> > > + }
> > > + if (strstr(dat, "oyz_U0_ujx") != NULL) {
> > > + hex_parse_u24(&r->r0_low_reg, &dat[20]);
> > > + continue;
> > > + }
> > > + if (strstr(dat, "iks_GC_GMgq") != NULL) {
> > > + hex_parse_u24(&r->tlimit_reg, &dat[20]);
> > > + continue;
> > > + }
> > > + if (strstr(dat, "gou_Yao") != NULL) {
> > > + hex_parse_u8(p->thr, &dat[20], 3);
> > > + continue;
> > > + }
> > > + if (strstr(dat, "kgd_Wsc_Qsbp") != NULL) {
> > > + hex_parse_u8(p->plt_flg, &dat[20], 3);
> > > + continue;
> > > + }
> > > + if (strstr(dat, "yec_CqseSsqs") != NULL) {
> > > + hex_parse_u8(p->sin_gn, &dat[20], 3);
> > > + continue;
> > > + }
> > > + if (strstr(dat, "iks_SogkGgog2") != NULL) {
> > > + hex_parse_u8(p->sin_gn2, &dat[20], 3);
> > > + continue;
> > > + }
> > > + if (strstr(dat, "yec_Sae_Y") != NULL) {
> > > + hex_parse_u8(p->thr2, &dat[20], 3);
> > > + continue;
> > > + }
> > > + if (strstr(dat, "Re_Int") != NULL) {
> > > + hex_parse_u8(p->r0_reg, &dat[20], 3);
> > > + continue;
> > > + }
> > > + /* Check whether the spk connection is open */
> > > + if (strstr(dat, "SigFlag") != NULL) {
> > > + hex_parse_u8(p->tf_reg, &dat[20], 3);
> > > + continue;
> > > + }
> > > + if (strstr(dat, "a1_Int") != NULL) {
> > > + hex_parse_u8(p->a1_reg, &dat[20], 3);
> > > + continue;
> > > + }
> > > + if (strstr(dat, "a2_Int") != NULL) {
> > > + hex_parse_u8(p->a2_reg, &dat[20], 3);
> > > + continue;
> > > + }
> > > + }
> > > +}
--
With Best Regards,
Andy Shevchenko
