On Fri, Nov 25, 2022 at 05:27:24PM +0800, wangweidong.a@awinic.com wrote:
- check_sum = GET_32_DATA(*(p_check_sum + 3), *(p_check_sum + 2),
*(p_check_sum + 1), *(p_check_sum));
We've got the be32_to_cpu() and so on macros - I suspect that GET_32_DATA() should be one of those.
+static int aw_check_data_version(struct aw_bin *bin, int bin_num) +{
- int i = 0;
- for (i = DATA_VERSION_V1; i < DATA_VERSION_MAX; i++) {
if (bin->header_info[bin_num].bin_data_ver == i)return 0;- }
- pr_err("aw_bin_parse Unrecognized this bin data version\n");
- return -DATA_VER_ERR;
+}
This seems like an inefficient way of writing
if (bin->header_info[bin_num].bin_data_ver < DATA_VERSION_V1 || bin->header_info[bin_num].bin_data_ver > DATA_VERSION_MAX ||)
surely?
+static void aw_get_single_bin_header_1_0_0(struct aw_bin *bin) +{
- int i;
- bin->header_info[bin->all_bin_parse_num].header_len = 60;
- bin->header_info[bin->all_bin_parse_num].check_sum =
GET_32_DATA(*(bin->p_addr + 3), *(bin->p_addr + 2),*(bin->p_addr + 1), *(bin->p_addr));
The standard way of writing this would be with a packed struct with endianness annotations, that's a bit less error prone than this. I also didn't spot the size validation that ensures that we're not walking past the end of the binary image anywhere in the code, it might've been there but it could do with being rather more obvious. There are some size checks further down but it's not clear that they align with what's going on here.
+static int aw_parse_each_of_multi_bins_1_0_0(unsigned int bin_num, int bin_serial_num,
struct aw_bin *bin)+{
Given a function with an each_of name I'd expect to see a loop over multiple binaries? I see the loop in the caller but it's a bit confusing. Perhaps one_of.
- for (i = 0; i < cfg_hdr->a_ddt_num; ++i) {
if ((cfg_dde[i].data_type == ACF_SEC_TYPE_MUTLBIN) &&(aw_dev->chip_id == cfg_dde[i].chip_id) &&((aw_dev->i2c->adapter->nr == cfg_dde[i].dev_bus) &&(aw_dev->i2c->addr == cfg_dde[i].dev_addr))) {(*scene_num)++;}- }
Some of the indentation in this code is really hard to read - for example here it'd be better to align the if conditions with the brackets in the if statement to separate from the code that gets run if the condition is true.