On Mon, Jan 10, 2022 at 4:51 PM Doug Anderson <dianders@xxxxxxxxxxxx> wrote: > > Hi, > > On Mon, Jan 10, 2022 at 3:15 PM Abhishek Kumar <kuabhs@xxxxxxxxxxxx> wrote: > > > > +int ath10k_core_parse_default_bdf_dt(struct ath10k *ar) > > +{ > > + struct device_node *node; > > + const char *board_name = NULL; > > + > > + ar->id.default_bdf[0] = '\0'; > > + > > + node = ar->dev->of_node; > > + if (!node) > > + return -ENOENT; > > + > > + of_property_read_string(node, "qcom,ath10k-default-bdf", > > + &board_name); > > + if (!board_name) > > + return -ENODATA; > > + > > + if (strscpy(ar->id.default_bdf, > > + board_name, sizeof(ar->id.default_bdf)) < 0) > > + ath10k_warn(ar, > > + "default board name is longer than allocated buffer, board_name: %s; allocated size: %ld\n", > > + board_name, sizeof(ar->id.default_bdf)); > > I suspect, but don't know for sure, that you're going to get another > builder splat here. Just like sizeof() isn't guaranteed to return an > "unsigned int", it's also not guaranteed to return an "unsigned long". > I believe you want %zu. See Documentation/core-api/printk-formats.rst Thanks for the tip, I will make this fix in V3. > > > + > > + return 0; > > +} > > +EXPORT_SYMBOL(ath10k_core_parse_default_bdf_dt); > > Boy, that function seems like overkill for something that you need > once at init time. ...and I also suspect that the lifetime of the > string returned by of_property_read_string() is valid for as long as > your "of_node" is held and thus probably you could use it directly (it > likely has a longer lifetime than the location you're storing it). > > ...but I guess it matches the ath10k_core_check_dt() function above > it, so I guess it's fine? Ya, that was my idea to match it with ath10k_core_check_dt, initially, I was planning to remodify ath10k_core_check_dt to parse the new property, but looks it is used it multiple places, so I thought having a separate parser function would be cleaner, however, I am open to new ideas. - Abhishek