On Thu, 2020-12-03 at 10:39 +0530, Srinivasan Raju wrote:
> This introduces the pureLiFi LiFi driver for LiFi-X, LiFi-XC
> and LiFi-XL USB devices.
The below is a trivial readability possible enhancement and should not
hinder this patch being applied.
> diff --git a/drivers/net/wireless/purelifi/dbgfs.c
> b/drivers/net/wireless/purelifi/dbgfs.c
[]
> +ssize_t purelifi_store_frequency(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t len)
> +{
> + int i, r, row, col, predivider, feedback_divider, output_div_0;
> + int output_div_1, output_div_2, clkout3_phase;
> + char values[6][4] = {{0}, {0} };
> + char usr_input[8] = {0};
> + bool valid = false;
> + int rv = 0;
> +
> + row = 0; col = 0;
> + for (i = 0; i < min(len, sizeof(values)); i++) {
> + values[row][col] = buf[i];
> + col++;
> + if (buf[i] == 0x20) {
> + row++;
> + col = 0;
> + } else if (buf[i] == 0x0A) {
> + if (row == 5 && col > 0) {
> + rv = sscanf((char *)&values[0][0],
> + "%d", &predivider);
Aren't the casts to (char *) redundant ?
> + if (rv != 1)
> + valid = false;
> + rv = sscanf((char *)&values[1][0],
> + "%d", &feedback_divider);
> + if (rv != 1)
> + valid = false;
> + rv = sscanf((char *)&values[2][0],
> + "%d", &output_div_0);
> + if (rv != 1)
> + valid = false;
> + rv = sscanf((char *)&values[3][0],
> + "%d", &output_div_1);
> + if (rv != 1)
> + valid = false;
> + rv = sscanf((char *)&values[4][0],
> + "%d", &output_div_2);
> + if (rv != 1)
> + valid = false;
> + rv = sscanf((char *)&values[5][0],
> + "%d", &clkout3_phase);
> + if (rv != 1)
> + valid = false;
> + valid = true;
trivia: this might be more intelligible with an array and a loop
int *index[] = {
&predivider,
&feedback_divider,
&output_div_0,
&output_div_1,
&output_div_2,
&clkout3_phase
};
valid = true;
i = 0;
while (valid && i < ARRAY_SIZE(index)) {
if (sscanf(&values[i][0], "%d", index[i]) != 1)
valid = false;
i++;
}
> +
> + break;
> + }
> + }
> + }
> +
> + if (valid) {
> + usr_input[0] = (char)predivider;
> + usr_input[1] = (char)feedback_divider;
> + usr_input[2] = (char)output_div_0;
> + usr_input[3] = (char)output_div_1;
> + usr_input[4] = (char)output_div_2;
> + usr_input[5] = (char)clkout3_phase;
> +
> + dev_info(dev, "options IP_DIV: %d VCO_MULT: %d OP_DIV_PHY: %d",
> + usr_input[0], usr_input[1], usr_input[2]);
> + dev_info(dev, "OP_DIV_CPU: %d OP_DIV_USB: %d CLK3_PH: %d\n",
> + usr_input[3], usr_input[4], usr_input[5]);
> +
> + r = usb_write_req((const u8 *)&usr_input[0],
> + sizeof(usr_input), USB_REQ_SET_FREQ);
> + if (r < 0)
> + dev_err(dev, "SET_FREQ failed (%d)\n", r);
> + } else {
> + dev_err(dev, "Your options are invalid\n");
> + }
> +
> + return len;
> +}
