Messages in this thread | | | From | Andy Shevchenko <> | Date | Thu, 4 Mar 2021 11:50:52 +0200 | Subject | Re: [PATCH 4/4] platform/x86: pmc_atom: improve critclk_systems matching for Siemens PCs |
| |
On Thu, Mar 4, 2021 at 9:27 AM Henning Schild <henning.schild@siemens.com> wrote:
> Siemens industrial PCs unfortunately can not always be properly > identified the way we used to. An earlier commit introduced code that > allows proper identification without looking at DMI strings that could > differ based on product branding. > Switch over to that proper way and revert commits that used to collect > the machines based on unstable strings.
> +#include <linux/platform_data/x86/simatic-ipc.h>
> +static int pmc_clk_is_critical(const struct dmi_system_id *d) > +{ > + int ret = true; > + u32 station_id; > + > + if (!strcmp(d->ident, "SIEMENS AG")) { > + if (dmi_walk(simatic_ipc_find_dmi_entry_helper, &station_id)) > + ret = false; > + else > + ret = (station_id == SIMATIC_IPC_IPC227E || > + station_id == SIMATIC_IPC_IPC277E); > + } > + > + return ret;
Much easier to rewrite it as
if (strcmp(...)) // BTW, do we have a dmi_* helper for that? return true;
if (dmi_walk) return false;
return station_id == || ...;
> +}
Maybe instead you can rewrite it as a callback in DMI table which changes a (global, yeah) variable that you simply reassign...
> if (d) { > - clk_data->critical = true; > - pr_info("%s critclks quirk enabled\n", d->ident); > + clk_data->critical = pmc_clk_is_critical(d); > + if (clk_data->critical) > + pr_info("%s critclks quirk enabled\n", d->ident); > }
...somewhere here?
Like clk_data->critical = global_var; if (...) pr_info();
It seems it will reduce burden on a callback by dropping strcmp() call.
-- With Best Regards, Andy Shevchenko
| |