Wed, Mar 29, 2023 at 01:16:52PM +0530, Mukesh Ojha kirjoitti:
> Currently, scm driver only supports full dump when download
> mode is selected. Add support to enable minidump as well as
> enable it along with fulldump.
...
> #define QCOM_DOWNLOAD_MODE_MASK 0x30
> #define QCOM_DOWNLOAD_FULLDUMP 0x1
> +#define QCOM_DOWNLOAD_MINIDUMP 0x2
> +#define QCOM_DOWNLOAD_BOTHDUMP (QCOM_DOWNLOAD_FULLDUMP | QCOM_DOWNLOAD_MINIDUMP)
Now order is broken.
> #define QCOM_DOWNLOAD_NODUMP 0x0
...
> @@ -1420,13 +1422,16 @@ static irqreturn_t qcom_scm_irq_handler(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> -
Stray change and ping-pong style at the same time.
...
> if (download_mode == QCOM_DOWNLOAD_FULLDUMP)
> len = sysfs_emit(buffer, "full\n");
> + else if (download_mode == QCOM_DOWNLOAD_MINIDUMP)
> + len = sysfs_emit(buffer, "mini\n");
> + else if (download_mode == QCOM_DOWNLOAD_BOTHDUMP)
> + len = sysfs_emit(buffer, "full,mini\n");
Why not "both" ?
> else if (download_mode == QCOM_DOWNLOAD_NODUMP)
> len = sysfs_emit(buffer, "off\n");
With an array (for streq_match_string() call suggested earlier) this become as
simple as
if (mode >= ARRAY_SIZE(...))
return sysfs_emit("Oh heh!\n");
return sysfs_emit("%s\n", array[mode]);
...
> - if (sysfs_streq(val, "full")) {
Why changing this line?
> + if (sysfs_streq(val, "full,mini") || sysfs_streq(val, "mini,full")) {
> + download_mode = QCOM_DOWNLOAD_BOTHDUMP;
It's way too hard, esp. taking into account that once user enters wrong order,
user can't simply validate this by reading value back.
Use "both" and that's it.
> + } else if (sysfs_streq(val, "full")) {
> download_mode = QCOM_DOWNLOAD_FULLDUMP;
> + } else if (sysfs_streq(val, "mini")) {
> + download_mode = QCOM_DOWNLOAD_MINIDUMP;
...
> module_param_cb(download_mode, &download_mode_param_ops, NULL, 0644);
> MODULE_PARM_DESC(download_mode,
> - "Download mode: off/full or 0/1 for existing users");
> + "download mode: off/full/mini/full,mini or mini,full and 0/1 for existing users");
You really must be consistent with at least a couple of things:
1) capitalization;
2) indentation.
--
With Best Regards,
Andy Shevchenko
