diff mbox series

[RFC,12/16] clk: hifiberry-dacpro: add ACPI support

Message ID 20200409195841.18901-13-pierre-louis.bossart@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series ASoC/SOF/clk/gpio/dt: add Hifiberry DAC+ PRO support | expand

Commit Message

Pierre-Louis Bossart April 9, 2020, 7:58 p.m. UTC
On ACPI platforms the of_ functions are irrelevant, conditionally
compile them out and add devm_clk_hw_register_clkdev() call instead.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/clk/clk-hifiberry-dacpro.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Stephen Boyd April 22, 2020, 9:32 a.m. UTC | #1
Quoting Pierre-Louis Bossart (2020-04-09 12:58:37)
> On ACPI platforms the of_ functions are irrelevant, conditionally
> compile them out and add devm_clk_hw_register_clkdev() call instead.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  drivers/clk/clk-hifiberry-dacpro.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/clk/clk-hifiberry-dacpro.c b/drivers/clk/clk-hifiberry-dacpro.c
> index bf0616c959da..d01a90fed51b 100644
> --- a/drivers/clk/clk-hifiberry-dacpro.c
> +++ b/drivers/clk/clk-hifiberry-dacpro.c
> @@ -114,15 +114,22 @@ static int clk_hifiberry_dacpro_probe(struct platform_device *pdev)
>                 return ret;
>         }
>  
> +#ifndef CONFIG_ACPI

Use if (!IS_ENABLED(CONFIG_ACPI)) instead?

>         ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
>                                      &proclk->hw);
> +#else
> +       ret = devm_clk_hw_register_clkdev(dev, &proclk->hw,
> +                                         init.name, NULL);
> +#endif
>  
>         return ret;
>  }
>  
>  static int clk_hifiberry_dacpro_remove(struct platform_device *pdev)
>  {
> +#ifndef CONFIG_ACPI
>         of_clk_del_provider(pdev->dev.of_node);
> +#endif
Andy Shevchenko April 22, 2020, 9:47 a.m. UTC | #2
On Wed, Apr 22, 2020 at 02:32:15AM -0700, Stephen Boyd wrote:
> Quoting Pierre-Louis Bossart (2020-04-09 12:58:37)
> > On ACPI platforms the of_ functions are irrelevant, conditionally
> > compile them out and add devm_clk_hw_register_clkdev() call instead.

...

> Use if (!IS_ENABLED(CONFIG_ACPI)) instead?
> 
> >         ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
> >                                      &proclk->hw);

I'm rather wondering if we have OF stuff integrated properly to CLK framework
to avoid first branch completely.

> > +#else
> > +       ret = devm_clk_hw_register_clkdev(dev, &proclk->hw,
> > +                                         init.name, NULL);
> > +#endif
Pierre-Louis Bossart April 22, 2020, 9:54 a.m. UTC | #3
On 4/22/20 4:32 AM, Stephen Boyd wrote:
> Quoting Pierre-Louis Bossart (2020-04-09 12:58:37)
>> On ACPI platforms the of_ functions are irrelevant, conditionally
>> compile them out and add devm_clk_hw_register_clkdev() call instead.
>>
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> ---
>>   drivers/clk/clk-hifiberry-dacpro.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/clk/clk-hifiberry-dacpro.c b/drivers/clk/clk-hifiberry-dacpro.c
>> index bf0616c959da..d01a90fed51b 100644
>> --- a/drivers/clk/clk-hifiberry-dacpro.c
>> +++ b/drivers/clk/clk-hifiberry-dacpro.c
>> @@ -114,15 +114,22 @@ static int clk_hifiberry_dacpro_probe(struct platform_device *pdev)
>>                  return ret;
>>          }
>>   
>> +#ifndef CONFIG_ACPI
> 
> Use if (!IS_ENABLED(CONFIG_ACPI)) instead?

git grep CONFIG_ACPI shows most of the kernel code uses #if(n)def 
CONFIG_ACPI. It's equivalent, it's a boolean.
Stephen Boyd April 22, 2020, 8:52 p.m. UTC | #4
Quoting Pierre-Louis Bossart (2020-04-22 02:54:38)
> 
> 
> On 4/22/20 4:32 AM, Stephen Boyd wrote:
> > Quoting Pierre-Louis Bossart (2020-04-09 12:58:37)
> >> On ACPI platforms the of_ functions are irrelevant, conditionally
> >> compile them out and add devm_clk_hw_register_clkdev() call instead.
> >>
> >> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >> ---
> >>   drivers/clk/clk-hifiberry-dacpro.c | 7 +++++++
> >>   1 file changed, 7 insertions(+)
> >>
> >> diff --git a/drivers/clk/clk-hifiberry-dacpro.c b/drivers/clk/clk-hifiberry-dacpro.c
> >> index bf0616c959da..d01a90fed51b 100644
> >> --- a/drivers/clk/clk-hifiberry-dacpro.c
> >> +++ b/drivers/clk/clk-hifiberry-dacpro.c
> >> @@ -114,15 +114,22 @@ static int clk_hifiberry_dacpro_probe(struct platform_device *pdev)
> >>                  return ret;
> >>          }
> >>   
> >> +#ifndef CONFIG_ACPI
> > 
> > Use if (!IS_ENABLED(CONFIG_ACPI)) instead?
> 
> git grep CONFIG_ACPI shows most of the kernel code uses #if(n)def 
> CONFIG_ACPI. It's equivalent, it's a boolean.

It's not equivalent. It is a pre-processor directive vs. an if statement
that evaluates to 0 or 1 and lets the compiler see both sides of the
code to check types.
Pierre-Louis Bossart April 22, 2020, 9:08 p.m. UTC | #5
On 4/22/20 3:52 PM, Stephen Boyd wrote:
> Quoting Pierre-Louis Bossart (2020-04-22 02:54:38)
>>
>>
>> On 4/22/20 4:32 AM, Stephen Boyd wrote:
>>> Quoting Pierre-Louis Bossart (2020-04-09 12:58:37)
>>>> On ACPI platforms the of_ functions are irrelevant, conditionally
>>>> compile them out and add devm_clk_hw_register_clkdev() call instead.
>>>>
>>>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>>> ---
>>>>    drivers/clk/clk-hifiberry-dacpro.c | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/clk-hifiberry-dacpro.c b/drivers/clk/clk-hifiberry-dacpro.c
>>>> index bf0616c959da..d01a90fed51b 100644
>>>> --- a/drivers/clk/clk-hifiberry-dacpro.c
>>>> +++ b/drivers/clk/clk-hifiberry-dacpro.c
>>>> @@ -114,15 +114,22 @@ static int clk_hifiberry_dacpro_probe(struct platform_device *pdev)
>>>>                   return ret;
>>>>           }
>>>>    
>>>> +#ifndef CONFIG_ACPI
>>>
>>> Use if (!IS_ENABLED(CONFIG_ACPI)) instead?
>>
>> git grep CONFIG_ACPI shows most of the kernel code uses #if(n)def
>> CONFIG_ACPI. It's equivalent, it's a boolean.
> 
> It's not equivalent. It is a pre-processor directive vs. an if statement
> that evaluates to 0 or 1 and lets the compiler see both sides of the
> code to check types.

Ah, yes I misunderstood your point.
diff mbox series

Patch

diff --git a/drivers/clk/clk-hifiberry-dacpro.c b/drivers/clk/clk-hifiberry-dacpro.c
index bf0616c959da..d01a90fed51b 100644
--- a/drivers/clk/clk-hifiberry-dacpro.c
+++ b/drivers/clk/clk-hifiberry-dacpro.c
@@ -114,15 +114,22 @@  static int clk_hifiberry_dacpro_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+#ifndef CONFIG_ACPI
 	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
 				     &proclk->hw);
+#else
+	ret = devm_clk_hw_register_clkdev(dev, &proclk->hw,
+					  init.name, NULL);
+#endif
 
 	return ret;
 }
 
 static int clk_hifiberry_dacpro_remove(struct platform_device *pdev)
 {
+#ifndef CONFIG_ACPI
 	of_clk_del_provider(pdev->dev.of_node);
+#endif
 	return 0;
 }