| Message ID | 20200409195841.18901-13-pierre-louis.bossart@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | ASoC/SOF/clk/gpio/dt: add Hifiberry DAC+ PRO support | expand |
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
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
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.
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.
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 --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; }
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(+)