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.
Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
Fixes: e8796c6c69d1 ("platform/x86: pmc_atom: Add Siemens CONNECT ...")
Fixes: f110d252ae79 ("platform/x86: pmc_atom: Add Siemens SIMATIC ...")
Fixes: ad0d315b4d4e ("platform/x86: pmc_atom: Add Siemens SIMATIC ...")
Tested-by: Michael Haener <[email protected]>
Signed-off-by: Henning Schild <[email protected]>
---
drivers/platform/x86/pmc_atom.c | 47 +++++++++++++++++++--------------
1 file changed, 27 insertions(+), 20 deletions(-)
diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index ca684ed760d1..38542d547f29 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -13,6 +13,7 @@
#include <linux/io.h>
#include <linux/platform_data/x86/clk-pmc-atom.h>
#include <linux/platform_data/x86/pmc_atom.h>
+#include <linux/platform_data/x86/simatic-ipc.h>
#include <linux/platform_device.h>
#include <linux/pci.h>
#include <linux/seq_file.h>
@@ -362,6 +363,23 @@ static void pmc_dbgfs_register(struct pmc_dev *pmc)
}
#endif /* CONFIG_DEBUG_FS */
+static bool pmc_clk_is_critical = true;
+
+static int siemens_clk_is_critical(const struct dmi_system_id *d)
+{
+ u32 st_id;
+
+ if (dmi_walk(simatic_ipc_find_dmi_entry_helper, &st_id))
+ goto out;
+
+ if (st_id == SIMATIC_IPC_IPC227E || st_id == SIMATIC_IPC_IPC277E)
+ return 1;
+
+out:
+ pmc_clk_is_critical = false;
+ return 1;
+}
+
/*
* Some systems need one or more of their pmc_plt_clks to be
* marked as critical.
@@ -424,24 +442,10 @@ static const struct dmi_system_id critclk_systems[] = {
},
},
{
- .ident = "SIMATIC IPC227E",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
- DMI_MATCH(DMI_PRODUCT_VERSION, "6ES7647-8B"),
- },
- },
- {
- .ident = "SIMATIC IPC277E",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
- DMI_MATCH(DMI_PRODUCT_VERSION, "6AV7882-0"),
- },
- },
- {
- .ident = "CONNECT X300",
+ .callback = siemens_clk_is_critical,
+ .ident = "SIEMENS AG",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
- DMI_MATCH(DMI_PRODUCT_VERSION, "A5E45074588"),
},
},
@@ -453,7 +457,7 @@ static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
{
struct platform_device *clkdev;
struct pmc_clk_data *clk_data;
- const struct dmi_system_id *d = dmi_first_match(critclk_systems);
+ const struct dmi_system_id *d;
clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
if (!clk_data)
@@ -461,9 +465,12 @@ static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
clk_data->base = pmc_regmap; /* offset is added by client */
clk_data->clks = pmc_data->clks;
- if (d) {
- clk_data->critical = true;
- pr_info("%s critclks quirk enabled\n", d->ident);
+ if (dmi_check_system(critclk_systems)) {
+ clk_data->critical = pmc_clk_is_critical;
+ if (clk_data->critical) {
+ d = dmi_first_match(critclk_systems);
+ pr_info("%s critclks quirk enabled\n", d->ident);
+ }
}
clkdev = platform_device_register_data(&pdev->dev, "clk-pmc-atom",
--
2.26.2
Am Mon, 15 Mar 2021 10:57:10 +0100
schrieb Henning Schild <[email protected]>:
> 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.
>
> Fixes: 648e921888ad ("clk: x86: Stop marking clocks as
> CLK_IS_CRITICAL") Fixes: e8796c6c69d1 ("platform/x86: pmc_atom: Add
> Siemens CONNECT ...") Fixes: f110d252ae79 ("platform/x86: pmc_atom:
> Add Siemens SIMATIC ...") Fixes: ad0d315b4d4e ("platform/x86:
> pmc_atom: Add Siemens SIMATIC ...") Tested-by: Michael Haener
> <[email protected]> Signed-off-by: Henning Schild
> <[email protected]> ---
> drivers/platform/x86/pmc_atom.c | 47
> +++++++++++++++++++-------------- 1 file changed, 27 insertions(+),
> 20 deletions(-)
>
> diff --git a/drivers/platform/x86/pmc_atom.c
> b/drivers/platform/x86/pmc_atom.c index ca684ed760d1..38542d547f29
> 100644 --- a/drivers/platform/x86/pmc_atom.c
> +++ b/drivers/platform/x86/pmc_atom.c
> @@ -13,6 +13,7 @@
> #include <linux/io.h>
> #include <linux/platform_data/x86/clk-pmc-atom.h>
> #include <linux/platform_data/x86/pmc_atom.h>
> +#include <linux/platform_data/x86/simatic-ipc.h>
> #include <linux/platform_device.h>
> #include <linux/pci.h>
> #include <linux/seq_file.h>
> @@ -362,6 +363,23 @@ static void pmc_dbgfs_register(struct pmc_dev
> *pmc) }
> #endif /* CONFIG_DEBUG_FS */
>
> +static bool pmc_clk_is_critical = true;
> +
> +static int siemens_clk_is_critical(const struct dmi_system_id *d)
> +{
> + u32 st_id;
> +
> + if (dmi_walk(simatic_ipc_find_dmi_entry_helper, &st_id))
> + goto out;
> +
> + if (st_id == SIMATIC_IPC_IPC227E || st_id ==
> SIMATIC_IPC_IPC277E)
> + return 1;
> +
> +out:
> + pmc_clk_is_critical = false;
> + return 1;
> +}
> +
> /*
> * Some systems need one or more of their pmc_plt_clks to be
> * marked as critical.
> @@ -424,24 +442,10 @@ static const struct dmi_system_id
> critclk_systems[] = { },
> },
> {
> - .ident = "SIMATIC IPC227E",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
> - DMI_MATCH(DMI_PRODUCT_VERSION, "6ES7647-8B"),
> - },
> - },
> - {
> - .ident = "SIMATIC IPC277E",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
> - DMI_MATCH(DMI_PRODUCT_VERSION, "6AV7882-0"),
> - },
> - },
> - {
> - .ident = "CONNECT X300",
> + .callback = siemens_clk_is_critical,
> + .ident = "SIEMENS AG",
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
> - DMI_MATCH(DMI_PRODUCT_VERSION,
> "A5E45074588"), },
> },
>
> @@ -453,7 +457,7 @@ static int pmc_setup_clks(struct pci_dev *pdev,
> void __iomem *pmc_regmap, {
> struct platform_device *clkdev;
> struct pmc_clk_data *clk_data;
> - const struct dmi_system_id *d =
> dmi_first_match(critclk_systems);
> + const struct dmi_system_id *d;
>
> clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
> if (!clk_data)
> @@ -461,9 +465,12 @@ static int pmc_setup_clks(struct pci_dev *pdev,
> void __iomem *pmc_regmap,
> clk_data->base = pmc_regmap; /* offset is added by client */
> clk_data->clks = pmc_data->clks;
> - if (d) {
> - clk_data->critical = true;
> - pr_info("%s critclks quirk enabled\n", d->ident);
> + if (dmi_check_system(critclk_systems)) {
Had to switch to check_system to get the callback to work.
> + clk_data->critical = pmc_clk_is_critical;
> + if (clk_data->critical) {
> + d = dmi_first_match(critclk_systems);
> + pr_info("%s critclks quirk enabled\n",
> d->ident);
Now need a double match here just to print the ident. Not too happy
with that but proposing it like this to keep the ident printing.
I guess it could be improved by not printing the ident or having a
global variable and global callback to remember the ident to print
later. I would propose to not print the ident if the double-match does
not find traction.
Henning
> + }
> }
>
> clkdev = platform_device_register_data(&pdev->dev,
> "clk-pmc-atom",
Hi,
On 3/15/21 11:14 AM, Henning Schild wrote:
> Am Mon, 15 Mar 2021 10:57:10 +0100
> schrieb Henning Schild <[email protected]>:
>
>> 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.
>>
>> Fixes: 648e921888ad ("clk: x86: Stop marking clocks as
>> CLK_IS_CRITICAL") Fixes: e8796c6c69d1 ("platform/x86: pmc_atom: Add
>> Siemens CONNECT ...") Fixes: f110d252ae79 ("platform/x86: pmc_atom:
>> Add Siemens SIMATIC ...") Fixes: ad0d315b4d4e ("platform/x86:
>> pmc_atom: Add Siemens SIMATIC ...") Tested-by: Michael Haener
>> <[email protected]> Signed-off-by: Henning Schild
>> <[email protected]> ---
>> drivers/platform/x86/pmc_atom.c | 47
>> +++++++++++++++++++-------------- 1 file changed, 27 insertions(+),
>> 20 deletions(-)
>>
>> diff --git a/drivers/platform/x86/pmc_atom.c
>> b/drivers/platform/x86/pmc_atom.c index ca684ed760d1..38542d547f29
>> 100644 --- a/drivers/platform/x86/pmc_atom.c
>> +++ b/drivers/platform/x86/pmc_atom.c
>> @@ -13,6 +13,7 @@
>> #include <linux/io.h>
>> #include <linux/platform_data/x86/clk-pmc-atom.h>
>> #include <linux/platform_data/x86/pmc_atom.h>
>> +#include <linux/platform_data/x86/simatic-ipc.h>
>> #include <linux/platform_device.h>
>> #include <linux/pci.h>
>> #include <linux/seq_file.h>
>> @@ -362,6 +363,23 @@ static void pmc_dbgfs_register(struct pmc_dev
>> *pmc) }
>> #endif /* CONFIG_DEBUG_FS */
>>
>> +static bool pmc_clk_is_critical = true;
>> +
>> +static int siemens_clk_is_critical(const struct dmi_system_id *d)
>> +{
>> + u32 st_id;
>> +
>> + if (dmi_walk(simatic_ipc_find_dmi_entry_helper, &st_id))
>> + goto out;
>> +
>> + if (st_id == SIMATIC_IPC_IPC227E || st_id ==
>> SIMATIC_IPC_IPC277E)
>> + return 1;
>> +
>> +out:
>> + pmc_clk_is_critical = false;
>> + return 1;
>> +}
>> +
>> /*
>> * Some systems need one or more of their pmc_plt_clks to be
>> * marked as critical.
>> @@ -424,24 +442,10 @@ static const struct dmi_system_id
>> critclk_systems[] = { },
>> },
>> {
>> - .ident = "SIMATIC IPC227E",
>> - .matches = {
>> - DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
>> - DMI_MATCH(DMI_PRODUCT_VERSION, "6ES7647-8B"),
>> - },
>> - },
>> - {
>> - .ident = "SIMATIC IPC277E",
>> - .matches = {
>> - DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
>> - DMI_MATCH(DMI_PRODUCT_VERSION, "6AV7882-0"),
>> - },
>> - },
>> - {
>> - .ident = "CONNECT X300",
>> + .callback = siemens_clk_is_critical,
>> + .ident = "SIEMENS AG",
>> .matches = {
>> DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
>> - DMI_MATCH(DMI_PRODUCT_VERSION,
>> "A5E45074588"), },
>> },
>>
>> @@ -453,7 +457,7 @@ static int pmc_setup_clks(struct pci_dev *pdev,
>> void __iomem *pmc_regmap, {
>> struct platform_device *clkdev;
>> struct pmc_clk_data *clk_data;
>> - const struct dmi_system_id *d =
>> dmi_first_match(critclk_systems);
>> + const struct dmi_system_id *d;
>>
>> clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
>> if (!clk_data)
>> @@ -461,9 +465,12 @@ static int pmc_setup_clks(struct pci_dev *pdev,
>> void __iomem *pmc_regmap,
>> clk_data->base = pmc_regmap; /* offset is added by client */
>> clk_data->clks = pmc_data->clks;
>> - if (d) {
>> - clk_data->critical = true;
>> - pr_info("%s critclks quirk enabled\n", d->ident);
>> + if (dmi_check_system(critclk_systems)) {
>
> Had to switch to check_system to get the callback to work.
>
>> + clk_data->critical = pmc_clk_is_critical;
>> + if (clk_data->critical) {
>> + d = dmi_first_match(critclk_systems);
>> + pr_info("%s critclks quirk enabled\n",
>> d->ident);
>
> Now need a double match here just to print the ident. Not too happy
> with that but proposing it like this to keep the ident printing.
>
> I guess it could be improved by not printing the ident or having a
> global variable and global callback to remember the ident to print
> later. I would propose to not print the ident if the double-match does
> not find traction.
IMHO it would be best to add another callback for the non Siemens
entries which just prints the ideent and returns 1 to avoid needsly
looping over the rest of the array.
And then just set the callback member of all the non Siemens entries
to this new callback. The space for the callback pointer is already
reserved in the struct anyways, so actually setting it does not take
up any space.
Regards,
Hans
Am Mon, 15 Mar 2021 11:19:24 +0100
schrieb Hans de Goede <[email protected]>:
> Hi,
>
> On 3/15/21 11:14 AM, Henning Schild wrote:
> > Am Mon, 15 Mar 2021 10:57:10 +0100
> > schrieb Henning Schild <[email protected]>:
> >
> >> 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.
> >>
> >> Fixes: 648e921888ad ("clk: x86: Stop marking clocks as
> >> CLK_IS_CRITICAL") Fixes: e8796c6c69d1 ("platform/x86: pmc_atom: Add
> >> Siemens CONNECT ...") Fixes: f110d252ae79 ("platform/x86: pmc_atom:
> >> Add Siemens SIMATIC ...") Fixes: ad0d315b4d4e ("platform/x86:
> >> pmc_atom: Add Siemens SIMATIC ...") Tested-by: Michael Haener
> >> <[email protected]> Signed-off-by: Henning Schild
> >> <[email protected]> ---
> >> drivers/platform/x86/pmc_atom.c | 47
> >> +++++++++++++++++++-------------- 1 file changed, 27 insertions(+),
> >> 20 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/pmc_atom.c
> >> b/drivers/platform/x86/pmc_atom.c index ca684ed760d1..38542d547f29
> >> 100644 --- a/drivers/platform/x86/pmc_atom.c
> >> +++ b/drivers/platform/x86/pmc_atom.c
> >> @@ -13,6 +13,7 @@
> >> #include <linux/io.h>
> >> #include <linux/platform_data/x86/clk-pmc-atom.h>
> >> #include <linux/platform_data/x86/pmc_atom.h>
> >> +#include <linux/platform_data/x86/simatic-ipc.h>
> >> #include <linux/platform_device.h>
> >> #include <linux/pci.h>
> >> #include <linux/seq_file.h>
> >> @@ -362,6 +363,23 @@ static void pmc_dbgfs_register(struct pmc_dev
> >> *pmc) }
> >> #endif /* CONFIG_DEBUG_FS */
> >>
> >> +static bool pmc_clk_is_critical = true;
> >> +
> >> +static int siemens_clk_is_critical(const struct dmi_system_id *d)
> >> +{
> >> + u32 st_id;
> >> +
> >> + if (dmi_walk(simatic_ipc_find_dmi_entry_helper, &st_id))
> >> + goto out;
> >> +
> >> + if (st_id == SIMATIC_IPC_IPC227E || st_id ==
> >> SIMATIC_IPC_IPC277E)
> >> + return 1;
> >> +
> >> +out:
> >> + pmc_clk_is_critical = false;
> >> + return 1;
> >> +}
> >> +
> >> /*
> >> * Some systems need one or more of their pmc_plt_clks to be
> >> * marked as critical.
> >> @@ -424,24 +442,10 @@ static const struct dmi_system_id
> >> critclk_systems[] = { },
> >> },
> >> {
> >> - .ident = "SIMATIC IPC227E",
> >> - .matches = {
> >> - DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
> >> - DMI_MATCH(DMI_PRODUCT_VERSION,
> >> "6ES7647-8B"),
> >> - },
> >> - },
> >> - {
> >> - .ident = "SIMATIC IPC277E",
> >> - .matches = {
> >> - DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
> >> - DMI_MATCH(DMI_PRODUCT_VERSION,
> >> "6AV7882-0"),
> >> - },
> >> - },
> >> - {
> >> - .ident = "CONNECT X300",
> >> + .callback = siemens_clk_is_critical,
> >> + .ident = "SIEMENS AG",
> >> .matches = {
> >> DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
> >> - DMI_MATCH(DMI_PRODUCT_VERSION,
> >> "A5E45074588"), },
> >> },
> >>
> >> @@ -453,7 +457,7 @@ static int pmc_setup_clks(struct pci_dev *pdev,
> >> void __iomem *pmc_regmap, {
> >> struct platform_device *clkdev;
> >> struct pmc_clk_data *clk_data;
> >> - const struct dmi_system_id *d =
> >> dmi_first_match(critclk_systems);
> >> + const struct dmi_system_id *d;
> >>
> >> clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
> >> if (!clk_data)
> >> @@ -461,9 +465,12 @@ static int pmc_setup_clks(struct pci_dev
> >> *pdev, void __iomem *pmc_regmap,
> >> clk_data->base = pmc_regmap; /* offset is added by client
> >> */ clk_data->clks = pmc_data->clks;
> >> - if (d) {
> >> - clk_data->critical = true;
> >> - pr_info("%s critclks quirk enabled\n", d->ident);
> >> + if (dmi_check_system(critclk_systems)) {
> >
> > Had to switch to check_system to get the callback to work.
> >
> >> + clk_data->critical = pmc_clk_is_critical;
> >> + if (clk_data->critical) {
> >> + d = dmi_first_match(critclk_systems);
> >> + pr_info("%s critclks quirk enabled\n",
> >> d->ident);
> >
> > Now need a double match here just to print the ident. Not too happy
> > with that but proposing it like this to keep the ident printing.
> >
> > I guess it could be improved by not printing the ident or having a
> > global variable and global callback to remember the ident to print
> > later. I would propose to not print the ident if the double-match
> > does not find traction.
>
> IMHO it would be best to add another callback for the non Siemens
> entries which just prints the ideent and returns 1 to avoid needsly
> looping over the rest of the array.
>
> And then just set the callback member of all the non Siemens entries
> to this new callback. The space for the callback pointer is already
> reserved in the struct anyways, so actually setting it does not take
> up any space.
Sounds good. I think i will make that another patch on top of the
series and send it in reply to "v2 4/4". Maybe squash it in a v3.
regards,
Henning
> Regards,
>
> Hans
>
Hi Henning,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on pavel-linux-leds/for-next]
[also build test WARNING on tip/master linux/master linus/master v5.12-rc3 next-20210315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Henning-Schild/add-device-drivers-for-Siemens-Industrial-PCs/20210315-181441
base: git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git for-next
config: x86_64-randconfig-s022-20210315 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-277-gc089cd2d-dirty
# https://github.com/0day-ci/linux/commit/7b3ce3514b5f314b15ab6e2898104fa3e8e76d59
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Henning-Schild/add-device-drivers-for-Siemens-Industrial-PCs/20210315-181441
git checkout 7b3ce3514b5f314b15ab6e2898104fa3e8e76d59
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
"sparse warnings: (new ones prefixed by >>)"
drivers/platform/x86/pmc_atom.c: note: in included file:
>> include/linux/platform_data/x86/simatic-ipc.h:50:30: sparse: sparse: cast to restricted __le32
vim +50 include/linux/platform_data/x86/simatic-ipc.h
051f5729fd6fb4 Henning Schild 2021-03-15 30
051f5729fd6fb4 Henning Schild 2021-03-15 31 static inline u32 simatic_ipc_get_station_id(u8 *data, int max_len)
051f5729fd6fb4 Henning Schild 2021-03-15 32 {
051f5729fd6fb4 Henning Schild 2021-03-15 33 u32 station_id = SIMATIC_IPC_INVALID_STATION_ID;
051f5729fd6fb4 Henning Schild 2021-03-15 34 int i;
051f5729fd6fb4 Henning Schild 2021-03-15 35 struct {
051f5729fd6fb4 Henning Schild 2021-03-15 36 u8 type; /* type (0xff = binary) */
051f5729fd6fb4 Henning Schild 2021-03-15 37 u8 len; /* len of data entry */
051f5729fd6fb4 Henning Schild 2021-03-15 38 u8 reserved[3];
051f5729fd6fb4 Henning Schild 2021-03-15 39 u32 station_id; /* station id (LE) */
051f5729fd6fb4 Henning Schild 2021-03-15 40 } __packed
051f5729fd6fb4 Henning Schild 2021-03-15 41 *data_entry = (void *)data + sizeof(struct dmi_header);
051f5729fd6fb4 Henning Schild 2021-03-15 42
051f5729fd6fb4 Henning Schild 2021-03-15 43 /* find 4th entry in OEM data */
051f5729fd6fb4 Henning Schild 2021-03-15 44 for (i = 0; i < 3; i++)
051f5729fd6fb4 Henning Schild 2021-03-15 45 data_entry = (void *)((u8 *)(data_entry) + data_entry->len);
051f5729fd6fb4 Henning Schild 2021-03-15 46
051f5729fd6fb4 Henning Schild 2021-03-15 47 /* decode station id */
051f5729fd6fb4 Henning Schild 2021-03-15 48 if (data_entry && (u8 *)data_entry < data + max_len &&
051f5729fd6fb4 Henning Schild 2021-03-15 49 data_entry->type == 0xff && data_entry->len == 9)
051f5729fd6fb4 Henning Schild 2021-03-15 @50 station_id = le32_to_cpu(data_entry->station_id);
051f5729fd6fb4 Henning Schild 2021-03-15 51
051f5729fd6fb4 Henning Schild 2021-03-15 52 return station_id;
051f5729fd6fb4 Henning Schild 2021-03-15 53 }
051f5729fd6fb4 Henning Schild 2021-03-15 54
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Introduce a global variable to remember the matching entry for later
printing. Also having a callback allows to stop matching after the first
hit.
Signed-off-by: Henning Schild <[email protected]>
---
drivers/platform/x86/pmc_atom.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index 38542d547f29..d0f74856cd8b 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -364,8 +364,16 @@ static void pmc_dbgfs_register(struct pmc_dev *pmc)
#endif /* CONFIG_DEBUG_FS */
static bool pmc_clk_is_critical = true;
+static const struct dmi_system_id *dmi_critical;
-static int siemens_clk_is_critical(const struct dmi_system_id *d)
+static int dmi_callback(const struct dmi_system_id *d)
+{
+ dmi_critical = d;
+
+ return 1;
+}
+
+static int dmi_callback_siemens(const struct dmi_system_id *d)
{
u32 st_id;
@@ -373,7 +381,7 @@ static int siemens_clk_is_critical(const struct dmi_system_id *d)
goto out;
if (st_id == SIMATIC_IPC_IPC227E || st_id == SIMATIC_IPC_IPC277E)
- return 1;
+ return dmi_callback(d);
out:
pmc_clk_is_critical = false;
@@ -388,6 +396,7 @@ static const struct dmi_system_id critclk_systems[] = {
{
/* pmc_plt_clk0 is used for an external HSIC USB HUB */
.ident = "MPL CEC1x",
+ .callback = dmi_callback,
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "MPL AG"),
DMI_MATCH(DMI_PRODUCT_NAME, "CEC10 Family"),
@@ -396,6 +405,7 @@ static const struct dmi_system_id critclk_systems[] = {
{
/* pmc_plt_clk0 - 3 are used for the 4 ethernet controllers */
.ident = "Lex 3I380D",
+ .callback = dmi_callback,
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
DMI_MATCH(DMI_PRODUCT_NAME, "3I380D"),
@@ -404,6 +414,7 @@ static const struct dmi_system_id critclk_systems[] = {
{
/* pmc_plt_clk* - are used for ethernet controllers */
.ident = "Lex 2I385SW",
+ .callback = dmi_callback,
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
DMI_MATCH(DMI_PRODUCT_NAME, "2I385SW"),
@@ -412,6 +423,7 @@ static const struct dmi_system_id critclk_systems[] = {
{
/* pmc_plt_clk* - are used for ethernet controllers */
.ident = "Beckhoff CB3163",
+ .callback = dmi_callback,
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff Automation"),
DMI_MATCH(DMI_BOARD_NAME, "CB3163"),
@@ -420,6 +432,7 @@ static const struct dmi_system_id critclk_systems[] = {
{
/* pmc_plt_clk* - are used for ethernet controllers */
.ident = "Beckhoff CB4063",
+ .callback = dmi_callback,
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff Automation"),
DMI_MATCH(DMI_BOARD_NAME, "CB4063"),
@@ -428,6 +441,7 @@ static const struct dmi_system_id critclk_systems[] = {
{
/* pmc_plt_clk* - are used for ethernet controllers */
.ident = "Beckhoff CB6263",
+ .callback = dmi_callback,
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff Automation"),
DMI_MATCH(DMI_BOARD_NAME, "CB6263"),
@@ -436,13 +450,14 @@ static const struct dmi_system_id critclk_systems[] = {
{
/* pmc_plt_clk* - are used for ethernet controllers */
.ident = "Beckhoff CB6363",
+ .callback = dmi_callback,
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff Automation"),
DMI_MATCH(DMI_BOARD_NAME, "CB6363"),
},
},
{
- .callback = siemens_clk_is_critical,
+ .callback = dmi_callback_siemens,
.ident = "SIEMENS AG",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
@@ -457,7 +472,6 @@ static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
{
struct platform_device *clkdev;
struct pmc_clk_data *clk_data;
- const struct dmi_system_id *d;
clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
if (!clk_data)
@@ -468,8 +482,8 @@ static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
if (dmi_check_system(critclk_systems)) {
clk_data->critical = pmc_clk_is_critical;
if (clk_data->critical) {
- d = dmi_first_match(critclk_systems);
- pr_info("%s critclks quirk enabled\n", d->ident);
+ pr_info("%s critclks quirk enabled\n",
+ dmi_critical->ident);
}
}
--
2.26.2
Hi,
On 3/15/21 3:58 PM, Henning Schild wrote:
> Introduce a global variable to remember the matching entry for later
> printing. Also having a callback allows to stop matching after the first
> hit.
>
> Signed-off-by: Henning Schild <[email protected]>
> ---
> drivers/platform/x86/pmc_atom.c | 26 ++++++++++++++++++++------
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
> index 38542d547f29..d0f74856cd8b 100644
> --- a/drivers/platform/x86/pmc_atom.c
> +++ b/drivers/platform/x86/pmc_atom.c
> @@ -364,8 +364,16 @@ static void pmc_dbgfs_register(struct pmc_dev *pmc)
> #endif /* CONFIG_DEBUG_FS */
>
> static bool pmc_clk_is_critical = true;
> +static const struct dmi_system_id *dmi_critical;
>
> -static int siemens_clk_is_critical(const struct dmi_system_id *d)
> +static int dmi_callback(const struct dmi_system_id *d)
> +{
> + dmi_critical = d;
Don't introduce a global variable for this please. Instead just directly
print the ident of the matching dmi_system_id here.
Regards,
Hans
> +
> + return 1;
> +}
> +
> +static int dmi_callback_siemens(const struct dmi_system_id *d)
> {
> u32 st_id;
>
> @@ -373,7 +381,7 @@ static int siemens_clk_is_critical(const struct dmi_system_id *d)
> goto out;
>
> if (st_id == SIMATIC_IPC_IPC227E || st_id == SIMATIC_IPC_IPC277E)
> - return 1;
> + return dmi_callback(d);
>
> out:
> pmc_clk_is_critical = false;
> @@ -388,6 +396,7 @@ static const struct dmi_system_id critclk_systems[] = {
> {
> /* pmc_plt_clk0 is used for an external HSIC USB HUB */
> .ident = "MPL CEC1x",
> + .callback = dmi_callback,
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "MPL AG"),
> DMI_MATCH(DMI_PRODUCT_NAME, "CEC10 Family"),
> @@ -396,6 +405,7 @@ static const struct dmi_system_id critclk_systems[] = {
> {
> /* pmc_plt_clk0 - 3 are used for the 4 ethernet controllers */
> .ident = "Lex 3I380D",
> + .callback = dmi_callback,
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
> DMI_MATCH(DMI_PRODUCT_NAME, "3I380D"),
> @@ -404,6 +414,7 @@ static const struct dmi_system_id critclk_systems[] = {
> {
> /* pmc_plt_clk* - are used for ethernet controllers */
> .ident = "Lex 2I385SW",
> + .callback = dmi_callback,
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
> DMI_MATCH(DMI_PRODUCT_NAME, "2I385SW"),
> @@ -412,6 +423,7 @@ static const struct dmi_system_id critclk_systems[] = {
> {
> /* pmc_plt_clk* - are used for ethernet controllers */
> .ident = "Beckhoff CB3163",
> + .callback = dmi_callback,
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff Automation"),
> DMI_MATCH(DMI_BOARD_NAME, "CB3163"),
> @@ -420,6 +432,7 @@ static const struct dmi_system_id critclk_systems[] = {
> {
> /* pmc_plt_clk* - are used for ethernet controllers */
> .ident = "Beckhoff CB4063",
> + .callback = dmi_callback,
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff Automation"),
> DMI_MATCH(DMI_BOARD_NAME, "CB4063"),
> @@ -428,6 +441,7 @@ static const struct dmi_system_id critclk_systems[] = {
> {
> /* pmc_plt_clk* - are used for ethernet controllers */
> .ident = "Beckhoff CB6263",
> + .callback = dmi_callback,
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff Automation"),
> DMI_MATCH(DMI_BOARD_NAME, "CB6263"),
> @@ -436,13 +450,14 @@ static const struct dmi_system_id critclk_systems[] = {
> {
> /* pmc_plt_clk* - are used for ethernet controllers */
> .ident = "Beckhoff CB6363",
> + .callback = dmi_callback,
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff Automation"),
> DMI_MATCH(DMI_BOARD_NAME, "CB6363"),
> },
> },
> {
> - .callback = siemens_clk_is_critical,
> + .callback = dmi_callback_siemens,
> .ident = "SIEMENS AG",
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
> @@ -457,7 +472,6 @@ static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
> {
> struct platform_device *clkdev;
> struct pmc_clk_data *clk_data;
> - const struct dmi_system_id *d;
>
> clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
> if (!clk_data)
> @@ -468,8 +482,8 @@ static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
> if (dmi_check_system(critclk_systems)) {
> clk_data->critical = pmc_clk_is_critical;
> if (clk_data->critical) {
> - d = dmi_first_match(critclk_systems);
> - pr_info("%s critclks quirk enabled\n", d->ident);
> + pr_info("%s critclks quirk enabled\n",
> + dmi_critical->ident);
> }
> }
>
>
Am Mon, 15 Mar 2021 17:31:49 +0100
schrieb Hans de Goede <[email protected]>:
> Hi,
>
> On 3/15/21 3:58 PM, Henning Schild wrote:
> > Introduce a global variable to remember the matching entry for later
> > printing. Also having a callback allows to stop matching after the
> > first hit.
> >
> > Signed-off-by: Henning Schild <[email protected]>
> > ---
> > drivers/platform/x86/pmc_atom.c | 26 ++++++++++++++++++++------
> > 1 file changed, 20 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/platform/x86/pmc_atom.c
> > b/drivers/platform/x86/pmc_atom.c index 38542d547f29..d0f74856cd8b
> > 100644 --- a/drivers/platform/x86/pmc_atom.c
> > +++ b/drivers/platform/x86/pmc_atom.c
> > @@ -364,8 +364,16 @@ static void pmc_dbgfs_register(struct pmc_dev
> > *pmc) #endif /* CONFIG_DEBUG_FS */
> >
> > static bool pmc_clk_is_critical = true;
> > +static const struct dmi_system_id *dmi_critical;
> >
> > -static int siemens_clk_is_critical(const struct dmi_system_id *d)
> > +static int dmi_callback(const struct dmi_system_id *d)
> > +{
> > + dmi_critical = d;
>
> Don't introduce a global variable for this please. Instead just
> directly print the ident of the matching dmi_system_id here.
Sorry, missed that part. Result looks nice and clean, thanks. I think i
will squash it into 4/4 in v3 and not follow up here for now.
Henning
> Regards,
>
> Hans
>
>
> > +
> > + return 1;
> > +}
> > +
> > +static int dmi_callback_siemens(const struct dmi_system_id *d)
> > {
> > u32 st_id;
> >
> > @@ -373,7 +381,7 @@ static int siemens_clk_is_critical(const struct
> > dmi_system_id *d) goto out;
> >
> > if (st_id == SIMATIC_IPC_IPC227E || st_id ==
> > SIMATIC_IPC_IPC277E)
> > - return 1;
> > + return dmi_callback(d);
> >
> > out:
> > pmc_clk_is_critical = false;
> > @@ -388,6 +396,7 @@ static const struct dmi_system_id
> > critclk_systems[] = { {
> > /* pmc_plt_clk0 is used for an external HSIC USB
> > HUB */ .ident = "MPL CEC1x",
> > + .callback = dmi_callback,
> > .matches = {
> > DMI_MATCH(DMI_SYS_VENDOR, "MPL AG"),
> > DMI_MATCH(DMI_PRODUCT_NAME, "CEC10
> > Family"), @@ -396,6 +405,7 @@ static const struct dmi_system_id
> > critclk_systems[] = { {
> > /* pmc_plt_clk0 - 3 are used for the 4 ethernet
> > controllers */ .ident = "Lex 3I380D",
> > + .callback = dmi_callback,
> > .matches = {
> > DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
> > DMI_MATCH(DMI_PRODUCT_NAME, "3I380D"),
> > @@ -404,6 +414,7 @@ static const struct dmi_system_id
> > critclk_systems[] = { {
> > /* pmc_plt_clk* - are used for ethernet
> > controllers */ .ident = "Lex 2I385SW",
> > + .callback = dmi_callback,
> > .matches = {
> > DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
> > DMI_MATCH(DMI_PRODUCT_NAME, "2I385SW"),
> > @@ -412,6 +423,7 @@ static const struct dmi_system_id
> > critclk_systems[] = { {
> > /* pmc_plt_clk* - are used for ethernet
> > controllers */ .ident = "Beckhoff CB3163",
> > + .callback = dmi_callback,
> > .matches = {
> > DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
> > Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB3163"),
> > @@ -420,6 +432,7 @@ static const struct dmi_system_id
> > critclk_systems[] = { {
> > /* pmc_plt_clk* - are used for ethernet
> > controllers */ .ident = "Beckhoff CB4063",
> > + .callback = dmi_callback,
> > .matches = {
> > DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
> > Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB4063"),
> > @@ -428,6 +441,7 @@ static const struct dmi_system_id
> > critclk_systems[] = { {
> > /* pmc_plt_clk* - are used for ethernet
> > controllers */ .ident = "Beckhoff CB6263",
> > + .callback = dmi_callback,
> > .matches = {
> > DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
> > Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB6263"),
> > @@ -436,13 +450,14 @@ static const struct dmi_system_id
> > critclk_systems[] = { {
> > /* pmc_plt_clk* - are used for ethernet
> > controllers */ .ident = "Beckhoff CB6363",
> > + .callback = dmi_callback,
> > .matches = {
> > DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
> > Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB6363"),
> > },
> > },
> > {
> > - .callback = siemens_clk_is_critical,
> > + .callback = dmi_callback_siemens,
> > .ident = "SIEMENS AG",
> > .matches = {
> > DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
> > @@ -457,7 +472,6 @@ static int pmc_setup_clks(struct pci_dev *pdev,
> > void __iomem *pmc_regmap, {
> > struct platform_device *clkdev;
> > struct pmc_clk_data *clk_data;
> > - const struct dmi_system_id *d;
> >
> > clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
> > if (!clk_data)
> > @@ -468,8 +482,8 @@ static int pmc_setup_clks(struct pci_dev *pdev,
> > void __iomem *pmc_regmap, if (dmi_check_system(critclk_systems)) {
> > clk_data->critical = pmc_clk_is_critical;
> > if (clk_data->critical) {
> > - d = dmi_first_match(critclk_systems);
> > - pr_info("%s critclks quirk enabled\n",
> > d->ident);
> > + pr_info("%s critclks quirk enabled\n",
> > + dmi_critical->ident);
> > }
> > }
> >
> >
>
Hi,
On 3/15/21 6:00 PM, Henning Schild wrote:
> Am Mon, 15 Mar 2021 17:31:49 +0100
> schrieb Hans de Goede <[email protected]>:
>
>> Hi,
>>
>> On 3/15/21 3:58 PM, Henning Schild wrote:
>>> Introduce a global variable to remember the matching entry for later
>>> printing. Also having a callback allows to stop matching after the
>>> first hit.
>>>
>>> Signed-off-by: Henning Schild <[email protected]>
>>> ---
>>> drivers/platform/x86/pmc_atom.c | 26 ++++++++++++++++++++------
>>> 1 file changed, 20 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/pmc_atom.c
>>> b/drivers/platform/x86/pmc_atom.c index 38542d547f29..d0f74856cd8b
>>> 100644 --- a/drivers/platform/x86/pmc_atom.c
>>> +++ b/drivers/platform/x86/pmc_atom.c
>>> @@ -364,8 +364,16 @@ static void pmc_dbgfs_register(struct pmc_dev
>>> *pmc) #endif /* CONFIG_DEBUG_FS */
>>>
>>> static bool pmc_clk_is_critical = true;
>>> +static const struct dmi_system_id *dmi_critical;
>>>
>>> -static int siemens_clk_is_critical(const struct dmi_system_id *d)
>>> +static int dmi_callback(const struct dmi_system_id *d)
>>> +{
>>> + dmi_critical = d;
>>
>> Don't introduce a global variable for this please. Instead just
>> directly print the ident of the matching dmi_system_id here.
>
> Sorry, missed that part. Result looks nice and clean, thanks. I think i
> will squash it into 4/4 in v3 and not follow up here for now.
Ack, that sounds good to me.
Regards,
Hans
>>> +
>>> + return 1;
>>> +}
>>> +
>>> +static int dmi_callback_siemens(const struct dmi_system_id *d)
>>> {
>>> u32 st_id;
>>>
>>> @@ -373,7 +381,7 @@ static int siemens_clk_is_critical(const struct
>>> dmi_system_id *d) goto out;
>>>
>>> if (st_id == SIMATIC_IPC_IPC227E || st_id ==
>>> SIMATIC_IPC_IPC277E)
>>> - return 1;
>>> + return dmi_callback(d);
>>>
>>> out:
>>> pmc_clk_is_critical = false;
>>> @@ -388,6 +396,7 @@ static const struct dmi_system_id
>>> critclk_systems[] = { {
>>> /* pmc_plt_clk0 is used for an external HSIC USB
>>> HUB */ .ident = "MPL CEC1x",
>>> + .callback = dmi_callback,
>>> .matches = {
>>> DMI_MATCH(DMI_SYS_VENDOR, "MPL AG"),
>>> DMI_MATCH(DMI_PRODUCT_NAME, "CEC10
>>> Family"), @@ -396,6 +405,7 @@ static const struct dmi_system_id
>>> critclk_systems[] = { {
>>> /* pmc_plt_clk0 - 3 are used for the 4 ethernet
>>> controllers */ .ident = "Lex 3I380D",
>>> + .callback = dmi_callback,
>>> .matches = {
>>> DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
>>> DMI_MATCH(DMI_PRODUCT_NAME, "3I380D"),
>>> @@ -404,6 +414,7 @@ static const struct dmi_system_id
>>> critclk_systems[] = { {
>>> /* pmc_plt_clk* - are used for ethernet
>>> controllers */ .ident = "Lex 2I385SW",
>>> + .callback = dmi_callback,
>>> .matches = {
>>> DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
>>> DMI_MATCH(DMI_PRODUCT_NAME, "2I385SW"),
>>> @@ -412,6 +423,7 @@ static const struct dmi_system_id
>>> critclk_systems[] = { {
>>> /* pmc_plt_clk* - are used for ethernet
>>> controllers */ .ident = "Beckhoff CB3163",
>>> + .callback = dmi_callback,
>>> .matches = {
>>> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
>>> Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB3163"),
>>> @@ -420,6 +432,7 @@ static const struct dmi_system_id
>>> critclk_systems[] = { {
>>> /* pmc_plt_clk* - are used for ethernet
>>> controllers */ .ident = "Beckhoff CB4063",
>>> + .callback = dmi_callback,
>>> .matches = {
>>> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
>>> Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB4063"),
>>> @@ -428,6 +441,7 @@ static const struct dmi_system_id
>>> critclk_systems[] = { {
>>> /* pmc_plt_clk* - are used for ethernet
>>> controllers */ .ident = "Beckhoff CB6263",
>>> + .callback = dmi_callback,
>>> .matches = {
>>> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
>>> Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB6263"),
>>> @@ -436,13 +450,14 @@ static const struct dmi_system_id
>>> critclk_systems[] = { {
>>> /* pmc_plt_clk* - are used for ethernet
>>> controllers */ .ident = "Beckhoff CB6363",
>>> + .callback = dmi_callback,
>>> .matches = {
>>> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
>>> Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB6363"),
>>> },
>>> },
>>> {
>>> - .callback = siemens_clk_is_critical,
>>> + .callback = dmi_callback_siemens,
>>> .ident = "SIEMENS AG",
>>> .matches = {
>>> DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
>>> @@ -457,7 +472,6 @@ static int pmc_setup_clks(struct pci_dev *pdev,
>>> void __iomem *pmc_regmap, {
>>> struct platform_device *clkdev;
>>> struct pmc_clk_data *clk_data;
>>> - const struct dmi_system_id *d;
>>>
>>> clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
>>> if (!clk_data)
>>> @@ -468,8 +482,8 @@ static int pmc_setup_clks(struct pci_dev *pdev,
>>> void __iomem *pmc_regmap, if (dmi_check_system(critclk_systems)) {
>>> clk_data->critical = pmc_clk_is_critical;
>>> if (clk_data->critical) {
>>> - d = dmi_first_match(critclk_systems);
>>> - pr_info("%s critclks quirk enabled\n",
>>> d->ident);
>>> + pr_info("%s critclks quirk enabled\n",
>>> + dmi_critical->ident);
>>> }
>>> }
>>>
>>>
>>
>
Hoi Hans,
on a slighly different but also related topic. Did you ever come across
SMSC SCH5347? Seems to be pretty similar to 56xx, only with spec non
public ... and probably less often in use
Maybe you happen to have code, or know the differences. We already have
it working with a modified copy of sch56xx but that is still rough and
i thought i ask before we potentially duplicate work.
groetjes,
Henning
Am Mon, 15 Mar 2021 19:01:13 +0100
schrieb Hans de Goede <[email protected]>:
> Hi,
>
> On 3/15/21 6:00 PM, Henning Schild wrote:
> > Am Mon, 15 Mar 2021 17:31:49 +0100
> > schrieb Hans de Goede <[email protected]>:
> >
> >> Hi,
> >>
> >> On 3/15/21 3:58 PM, Henning Schild wrote:
> >>> Introduce a global variable to remember the matching entry for
> >>> later printing. Also having a callback allows to stop matching
> >>> after the first hit.
> >>>
> >>> Signed-off-by: Henning Schild <[email protected]>
> >>> ---
> >>> drivers/platform/x86/pmc_atom.c | 26 ++++++++++++++++++++------
> >>> 1 file changed, 20 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/platform/x86/pmc_atom.c
> >>> b/drivers/platform/x86/pmc_atom.c index 38542d547f29..d0f74856cd8b
> >>> 100644 --- a/drivers/platform/x86/pmc_atom.c
> >>> +++ b/drivers/platform/x86/pmc_atom.c
> >>> @@ -364,8 +364,16 @@ static void pmc_dbgfs_register(struct pmc_dev
> >>> *pmc) #endif /* CONFIG_DEBUG_FS */
> >>>
> >>> static bool pmc_clk_is_critical = true;
> >>> +static const struct dmi_system_id *dmi_critical;
> >>>
> >>> -static int siemens_clk_is_critical(const struct dmi_system_id *d)
> >>> +static int dmi_callback(const struct dmi_system_id *d)
> >>> +{
> >>> + dmi_critical = d;
> >>
> >> Don't introduce a global variable for this please. Instead just
> >> directly print the ident of the matching dmi_system_id here.
> >
> > Sorry, missed that part. Result looks nice and clean, thanks. I
> > think i will squash it into 4/4 in v3 and not follow up here for
> > now.
>
> Ack, that sounds good to me.
>
> Regards,
>
> Hans
>
>
> >>> +
> >>> + return 1;
> >>> +}
> >>> +
> >>> +static int dmi_callback_siemens(const struct dmi_system_id *d)
> >>> {
> >>> u32 st_id;
> >>>
> >>> @@ -373,7 +381,7 @@ static int siemens_clk_is_critical(const
> >>> struct dmi_system_id *d) goto out;
> >>>
> >>> if (st_id == SIMATIC_IPC_IPC227E || st_id ==
> >>> SIMATIC_IPC_IPC277E)
> >>> - return 1;
> >>> + return dmi_callback(d);
> >>>
> >>> out:
> >>> pmc_clk_is_critical = false;
> >>> @@ -388,6 +396,7 @@ static const struct dmi_system_id
> >>> critclk_systems[] = { {
> >>> /* pmc_plt_clk0 is used for an external HSIC USB
> >>> HUB */ .ident = "MPL CEC1x",
> >>> + .callback = dmi_callback,
> >>> .matches = {
> >>> DMI_MATCH(DMI_SYS_VENDOR, "MPL AG"),
> >>> DMI_MATCH(DMI_PRODUCT_NAME, "CEC10
> >>> Family"), @@ -396,6 +405,7 @@ static const struct dmi_system_id
> >>> critclk_systems[] = { {
> >>> /* pmc_plt_clk0 - 3 are used for the 4 ethernet
> >>> controllers */ .ident = "Lex 3I380D",
> >>> + .callback = dmi_callback,
> >>> .matches = {
> >>> DMI_MATCH(DMI_SYS_VENDOR, "Lex
> >>> BayTrail"), DMI_MATCH(DMI_PRODUCT_NAME, "3I380D"),
> >>> @@ -404,6 +414,7 @@ static const struct dmi_system_id
> >>> critclk_systems[] = { {
> >>> /* pmc_plt_clk* - are used for ethernet
> >>> controllers */ .ident = "Lex 2I385SW",
> >>> + .callback = dmi_callback,
> >>> .matches = {
> >>> DMI_MATCH(DMI_SYS_VENDOR, "Lex
> >>> BayTrail"), DMI_MATCH(DMI_PRODUCT_NAME, "2I385SW"),
> >>> @@ -412,6 +423,7 @@ static const struct dmi_system_id
> >>> critclk_systems[] = { {
> >>> /* pmc_plt_clk* - are used for ethernet
> >>> controllers */ .ident = "Beckhoff CB3163",
> >>> + .callback = dmi_callback,
> >>> .matches = {
> >>> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
> >>> Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB3163"),
> >>> @@ -420,6 +432,7 @@ static const struct dmi_system_id
> >>> critclk_systems[] = { {
> >>> /* pmc_plt_clk* - are used for ethernet
> >>> controllers */ .ident = "Beckhoff CB4063",
> >>> + .callback = dmi_callback,
> >>> .matches = {
> >>> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
> >>> Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB4063"),
> >>> @@ -428,6 +441,7 @@ static const struct dmi_system_id
> >>> critclk_systems[] = { {
> >>> /* pmc_plt_clk* - are used for ethernet
> >>> controllers */ .ident = "Beckhoff CB6263",
> >>> + .callback = dmi_callback,
> >>> .matches = {
> >>> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
> >>> Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB6263"),
> >>> @@ -436,13 +450,14 @@ static const struct dmi_system_id
> >>> critclk_systems[] = { {
> >>> /* pmc_plt_clk* - are used for ethernet
> >>> controllers */ .ident = "Beckhoff CB6363",
> >>> + .callback = dmi_callback,
> >>> .matches = {
> >>> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
> >>> Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB6363"),
> >>> },
> >>> },
> >>> {
> >>> - .callback = siemens_clk_is_critical,
> >>> + .callback = dmi_callback_siemens,
> >>> .ident = "SIEMENS AG",
> >>> .matches = {
> >>> DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
> >>> @@ -457,7 +472,6 @@ static int pmc_setup_clks(struct pci_dev
> >>> *pdev, void __iomem *pmc_regmap, {
> >>> struct platform_device *clkdev;
> >>> struct pmc_clk_data *clk_data;
> >>> - const struct dmi_system_id *d;
> >>>
> >>> clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
> >>> if (!clk_data)
> >>> @@ -468,8 +482,8 @@ static int pmc_setup_clks(struct pci_dev
> >>> *pdev, void __iomem *pmc_regmap, if
> >>> (dmi_check_system(critclk_systems)) { clk_data->critical =
> >>> pmc_clk_is_critical; if (clk_data->critical) {
> >>> - d = dmi_first_match(critclk_systems);
> >>> - pr_info("%s critclks quirk enabled\n",
> >>> d->ident);
> >>> + pr_info("%s critclks quirk enabled\n",
> >>> + dmi_critical->ident);
> >>> }
> >>> }
> >>>
> >>>
> >>
> >
>
Hi Henning,
On 3/16/21 6:47 AM, Henning Schild wrote:
> Hoi Hans,
>
> on a slighly different but also related topic. Did you ever come across
> SMSC SCH5347? Seems to be pretty similar to 56xx, only with spec non
> public ... and probably less often in use
> Maybe you happen to have code, or know the differences. We already have
> it working with a modified copy of sch56xx but that is still rough and
> i thought i ask before we potentially duplicate work.
The history of FSC (Fujitsu-Siemens Computers) sensor support goes something
like this:
1. The university which I worked at had picked a FSC machine for the
replacement of all workstations, the FSCXXX sensor chip it had was i2c based,
so I could relatively easy reverse-engineer it and wrote a driver.
2. Around the time I stopped working for the university and started working
for Red Hat (2008) FSC contacted me and asked me if I wanted to help them
with officially supporting the FSC and later the SCH devices. They provided
test hardware and documentation at this time.
3. This continued for a while when FSC became just "Fujitsu" and things
moved from the FSC i2c based chips to the super-io based SCH chips. After
a while Fujitsu stopped contacting me about new chips and I stopped working
on this.
So ATM I'm not working no SCH56xx support and I've not worked on that for
quite some time now. So it is good to hear that you will be contributing
SCH5347 support to the kernel.
Regards,
Hans
> Am Mon, 15 Mar 2021 19:01:13 +0100
> schrieb Hans de Goede <[email protected]>:
>
>> Hi,
>>
>> On 3/15/21 6:00 PM, Henning Schild wrote:
>>> Am Mon, 15 Mar 2021 17:31:49 +0100
>>> schrieb Hans de Goede <[email protected]>:
>>>
>>>> Hi,
>>>>
>>>> On 3/15/21 3:58 PM, Henning Schild wrote:
>>>>> Introduce a global variable to remember the matching entry for
>>>>> later printing. Also having a callback allows to stop matching
>>>>> after the first hit.
>>>>>
>>>>> Signed-off-by: Henning Schild <[email protected]>
>>>>> ---
>>>>> drivers/platform/x86/pmc_atom.c | 26 ++++++++++++++++++++------
>>>>> 1 file changed, 20 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/platform/x86/pmc_atom.c
>>>>> b/drivers/platform/x86/pmc_atom.c index 38542d547f29..d0f74856cd8b
>>>>> 100644 --- a/drivers/platform/x86/pmc_atom.c
>>>>> +++ b/drivers/platform/x86/pmc_atom.c
>>>>> @@ -364,8 +364,16 @@ static void pmc_dbgfs_register(struct pmc_dev
>>>>> *pmc) #endif /* CONFIG_DEBUG_FS */
>>>>>
>>>>> static bool pmc_clk_is_critical = true;
>>>>> +static const struct dmi_system_id *dmi_critical;
>>>>>
>>>>> -static int siemens_clk_is_critical(const struct dmi_system_id *d)
>>>>> +static int dmi_callback(const struct dmi_system_id *d)
>>>>> +{
>>>>> + dmi_critical = d;
>>>>
>>>> Don't introduce a global variable for this please. Instead just
>>>> directly print the ident of the matching dmi_system_id here.
>>>
>>> Sorry, missed that part. Result looks nice and clean, thanks. I
>>> think i will squash it into 4/4 in v3 and not follow up here for
>>> now.
>>
>> Ack, that sounds good to me.
>>
>> Regards,
>>
>> Hans
>>
>>
>>>>> +
>>>>> + return 1;
>>>>> +}
>>>>> +
>>>>> +static int dmi_callback_siemens(const struct dmi_system_id *d)
>>>>> {
>>>>> u32 st_id;
>>>>>
>>>>> @@ -373,7 +381,7 @@ static int siemens_clk_is_critical(const
>>>>> struct dmi_system_id *d) goto out;
>>>>>
>>>>> if (st_id == SIMATIC_IPC_IPC227E || st_id ==
>>>>> SIMATIC_IPC_IPC277E)
>>>>> - return 1;
>>>>> + return dmi_callback(d);
>>>>>
>>>>> out:
>>>>> pmc_clk_is_critical = false;
>>>>> @@ -388,6 +396,7 @@ static const struct dmi_system_id
>>>>> critclk_systems[] = { {
>>>>> /* pmc_plt_clk0 is used for an external HSIC USB
>>>>> HUB */ .ident = "MPL CEC1x",
>>>>> + .callback = dmi_callback,
>>>>> .matches = {
>>>>> DMI_MATCH(DMI_SYS_VENDOR, "MPL AG"),
>>>>> DMI_MATCH(DMI_PRODUCT_NAME, "CEC10
>>>>> Family"), @@ -396,6 +405,7 @@ static const struct dmi_system_id
>>>>> critclk_systems[] = { {
>>>>> /* pmc_plt_clk0 - 3 are used for the 4 ethernet
>>>>> controllers */ .ident = "Lex 3I380D",
>>>>> + .callback = dmi_callback,
>>>>> .matches = {
>>>>> DMI_MATCH(DMI_SYS_VENDOR, "Lex
>>>>> BayTrail"), DMI_MATCH(DMI_PRODUCT_NAME, "3I380D"),
>>>>> @@ -404,6 +414,7 @@ static const struct dmi_system_id
>>>>> critclk_systems[] = { {
>>>>> /* pmc_plt_clk* - are used for ethernet
>>>>> controllers */ .ident = "Lex 2I385SW",
>>>>> + .callback = dmi_callback,
>>>>> .matches = {
>>>>> DMI_MATCH(DMI_SYS_VENDOR, "Lex
>>>>> BayTrail"), DMI_MATCH(DMI_PRODUCT_NAME, "2I385SW"),
>>>>> @@ -412,6 +423,7 @@ static const struct dmi_system_id
>>>>> critclk_systems[] = { {
>>>>> /* pmc_plt_clk* - are used for ethernet
>>>>> controllers */ .ident = "Beckhoff CB3163",
>>>>> + .callback = dmi_callback,
>>>>> .matches = {
>>>>> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
>>>>> Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB3163"),
>>>>> @@ -420,6 +432,7 @@ static const struct dmi_system_id
>>>>> critclk_systems[] = { {
>>>>> /* pmc_plt_clk* - are used for ethernet
>>>>> controllers */ .ident = "Beckhoff CB4063",
>>>>> + .callback = dmi_callback,
>>>>> .matches = {
>>>>> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
>>>>> Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB4063"),
>>>>> @@ -428,6 +441,7 @@ static const struct dmi_system_id
>>>>> critclk_systems[] = { {
>>>>> /* pmc_plt_clk* - are used for ethernet
>>>>> controllers */ .ident = "Beckhoff CB6263",
>>>>> + .callback = dmi_callback,
>>>>> .matches = {
>>>>> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
>>>>> Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB6263"),
>>>>> @@ -436,13 +450,14 @@ static const struct dmi_system_id
>>>>> critclk_systems[] = { {
>>>>> /* pmc_plt_clk* - are used for ethernet
>>>>> controllers */ .ident = "Beckhoff CB6363",
>>>>> + .callback = dmi_callback,
>>>>> .matches = {
>>>>> DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff
>>>>> Automation"), DMI_MATCH(DMI_BOARD_NAME, "CB6363"),
>>>>> },
>>>>> },
>>>>> {
>>>>> - .callback = siemens_clk_is_critical,
>>>>> + .callback = dmi_callback_siemens,
>>>>> .ident = "SIEMENS AG",
>>>>> .matches = {
>>>>> DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
>>>>> @@ -457,7 +472,6 @@ static int pmc_setup_clks(struct pci_dev
>>>>> *pdev, void __iomem *pmc_regmap, {
>>>>> struct platform_device *clkdev;
>>>>> struct pmc_clk_data *clk_data;
>>>>> - const struct dmi_system_id *d;
>>>>>
>>>>> clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
>>>>> if (!clk_data)
>>>>> @@ -468,8 +482,8 @@ static int pmc_setup_clks(struct pci_dev
>>>>> *pdev, void __iomem *pmc_regmap, if
>>>>> (dmi_check_system(critclk_systems)) { clk_data->critical =
>>>>> pmc_clk_is_critical; if (clk_data->critical) {
>>>>> - d = dmi_first_match(critclk_systems);
>>>>> - pr_info("%s critclks quirk enabled\n",
>>>>> d->ident);
>>>>> + pr_info("%s critclks quirk enabled\n",
>>>>> + dmi_critical->ident);
>>>>> }
>>>>> }
>>>>>
>>>>>
>>>>
>>>
>>
>