On Mon, 14 Dec 2020 00:26:03 +0100
Denis 'GNUtoo' Carikli <[email protected]> wrote:

> On Wed,  9 Dec 2020 18:31:09 +0200
> belgin <[email protected]> wrote:
> 
> Thanks a lot for the patch.
> 
> I've not tested it yet (I'll do it after sending this mail).
> 
> Beside a potential issue with CMD_LIST, everything looks good so far.
> 
> The following has potential issue:
> >  enum CMD_LIST {
> > -   CMD_LIST_FW_UPDATE = 0,
> > -   CMD_LIST_FW_VER_BIN,
> > -   CMD_LIST_FW_VER_IC,
> >     CMD_LIST_CONFIG_VER,
> >     CMD_LIST_GET_THRESHOLD,
> >     CMD_LIST_POWER_OFF,
If you decide to do a v2 version of the patch, it might be a good idea
to keep the CMD_LIST_FW_UPDATE / CMD_LIST_FW_VER_BIN /
CMD_LIST_CONFIG_VER commands, and implement them with a function that
only does a printk. 

For instance you could define one function for each command. Here's a
crude example for synaptics_get_threshold:
> void synaptics_get_threshold(void)
> {
>         printk(KERN_DEBUG "[TSP] %s\n", __func__);
> }

and in in sec_sysfs_cmd:
> switch (cmd) {
> case CMD_LIST_FW_UPDATE:
>        synaptics_list_fw_update();
>        break;

Or:
> switch (cmd) {
> case CMD_LIST_FW_UPDATE:
>      printk(KERN_DEBUG 
>             "[TSP] Warning: CMD_LIST_FW_UPDATE called, skipping.\n"
>             "[TSP] A program might be trying to load a firmware\n"
>             "[TSP] In doubt, report it on the Replicant mailing list\n");
>      break;

While the above examples are not the quality expected by the official
Linux project (for instance it doesn't use dev_err and so on), we
don't need to upstream this patch in Linux, and it should work fine for
our needs here, as it enables us to check if any of these functions
have been called with 'dmesg | grep TSP' for instance.

Denis.

Attachment: pgpmH_8Lth0On.pgp
Description: OpenPGP digital signature

_______________________________________________
Replicant mailing list
[email protected]
https://lists.osuosl.org/mailman/listinfo/replicant

Reply via email to