diff mbox series

platform/x86: pmc_atom: use callback for all dmi quirk entries

Message ID 20210315145855.17174-1-henning.schild@siemens.com
State New
Headers show
Series platform/x86: pmc_atom: use callback for all dmi quirk entries | expand

Commit Message

Henning Schild March 15, 2021, 2:58 p.m. UTC
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 <henning.schild@siemens.com>
---
 drivers/platform/x86/pmc_atom.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

Comments

Hans de Goede March 15, 2021, 4:31 p.m. UTC | #1
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 <henning.schild@siemens.com>
> ---
>  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);
>  		}
>  	}
>  
>
Hans de Goede March 15, 2021, 6:01 p.m. UTC | #2
Hi,

On 3/15/21 6:00 PM, Henning Schild wrote:
> Am Mon, 15 Mar 2021 17:31:49 +0100
> schrieb Hans de Goede <hdegoede@redhat.com>:
> 
>> 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 <henning.schild@siemens.com>
>>> ---
>>>  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);
>>>  		}
>>>  	}
>>>  
>>>   
>>
>
Henning Schild March 16, 2021, 5:47 a.m. UTC | #3
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 <hdegoede@redhat.com>:

> Hi,
> 
> On 3/15/21 6:00 PM, Henning Schild wrote:
> > Am Mon, 15 Mar 2021 17:31:49 +0100
> > schrieb Hans de Goede <hdegoede@redhat.com>:
> >   
> >> 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 <henning.schild@siemens.com>
> >>> ---
> >>>  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);
> >>>  		}
> >>>  	}
> >>>  
> >>>     
> >>  
> >   
>
Hans de Goede March 16, 2021, 9:43 a.m. UTC | #4
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 <hdegoede@redhat.com>:
> 
>> Hi,
>>
>> On 3/15/21 6:00 PM, Henning Schild wrote:
>>> Am Mon, 15 Mar 2021 17:31:49 +0100
>>> schrieb Hans de Goede <hdegoede@redhat.com>:
>>>   
>>>> 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 <henning.schild@siemens.com>
>>>>> ---
>>>>>  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);
>>>>>  		}
>>>>>  	}
>>>>>  
>>>>>     
>>>>  
>>>   
>>
>
diff mbox series

Patch

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);
 		}
 	}