* [PATCH v1 1/2] mfd: intel_soc_pmic_chtwc: Use dev_err_probe()
@ 2022-06-16 17:10 Andy Shevchenko
2022-06-16 17:10 ` [PATCH v1 2/2] mfd: intel_soc_pmic_chtwc: Switch from __maybe_unused to pm_sleep_ptr() etc Andy Shevchenko
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Andy Shevchenko @ 2022-06-16 17:10 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede, linux-kernel; +Cc: Andy Shevchenko, Lee Jones
Simplify the mux error path a bit by using dev_err_probe().
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/mfd/intel_soc_pmic_chtwc.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/mfd/intel_soc_pmic_chtwc.c b/drivers/mfd/intel_soc_pmic_chtwc.c
index 4eab191e053a..1cbe2bcce530 100644
--- a/drivers/mfd/intel_soc_pmic_chtwc.c
+++ b/drivers/mfd/intel_soc_pmic_chtwc.c
@@ -179,18 +179,13 @@ static int cht_wc_probe(struct i2c_client *client)
int ret;
status = acpi_evaluate_integer(ACPI_HANDLE(dev), "_HRV", NULL, &hrv);
- if (ACPI_FAILURE(status)) {
- dev_err(dev, "Failed to get PMIC hardware revision\n");
- return -ENODEV;
- }
- if (hrv != CHT_WC_HRV) {
- dev_err(dev, "Invalid PMIC hardware revision: %llu\n", hrv);
- return -ENODEV;
- }
- if (client->irq < 0) {
- dev_err(dev, "Invalid IRQ\n");
- return -EINVAL;
- }
+ if (ACPI_FAILURE(status))
+ return dev_err_probe(dev, -ENODEV, "Failed to get PMIC hardware revision\n");
+ if (hrv != CHT_WC_HRV)
+ return dev_err_probe(dev, -ENODEV, "Invalid PMIC hardware revision: %llu\n", hrv);
+
+ if (client->irq < 0)
+ return dev_err_probe(dev, -EINVAL, "Invalid IRQ\n");
pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
if (!pmic)
--
2.35.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v1 2/2] mfd: intel_soc_pmic_chtwc: Switch from __maybe_unused to pm_sleep_ptr() etc
2022-06-16 17:10 [PATCH v1 1/2] mfd: intel_soc_pmic_chtwc: Use dev_err_probe() Andy Shevchenko
@ 2022-06-16 17:10 ` Andy Shevchenko
2022-06-20 8:38 ` Hans de Goede
` (2 more replies)
2022-06-20 8:33 ` [PATCH v1 1/2] mfd: intel_soc_pmic_chtwc: Use dev_err_probe() Hans de Goede
2022-06-27 13:05 ` Lee Jones
2 siblings, 3 replies; 9+ messages in thread
From: Andy Shevchenko @ 2022-06-16 17:10 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede, linux-kernel; +Cc: Andy Shevchenko, Lee Jones
Letting the compiler remove these functions when the kernel is built
without CONFIG_PM_SLEEP support is simpler and less heavier for builds
than the use of __maybe_unused attributes.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/mfd/intel_soc_pmic_chtwc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mfd/intel_soc_pmic_chtwc.c b/drivers/mfd/intel_soc_pmic_chtwc.c
index 1cbe2bcce530..9216f0d34206 100644
--- a/drivers/mfd/intel_soc_pmic_chtwc.c
+++ b/drivers/mfd/intel_soc_pmic_chtwc.c
@@ -222,7 +222,7 @@ static void cht_wc_shutdown(struct i2c_client *client)
disable_irq(pmic->irq);
}
-static int __maybe_unused cht_wc_suspend(struct device *dev)
+static int cht_wc_suspend(struct device *dev)
{
struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
@@ -231,7 +231,7 @@ static int __maybe_unused cht_wc_suspend(struct device *dev)
return 0;
}
-static int __maybe_unused cht_wc_resume(struct device *dev)
+static int cht_wc_resume(struct device *dev)
{
struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
@@ -239,7 +239,7 @@ static int __maybe_unused cht_wc_resume(struct device *dev)
return 0;
}
-static SIMPLE_DEV_PM_OPS(cht_wc_pm_ops, cht_wc_suspend, cht_wc_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(cht_wc_pm_ops, cht_wc_suspend, cht_wc_resume);
static const struct i2c_device_id cht_wc_i2c_id[] = {
{ }
@@ -253,7 +253,7 @@ static const struct acpi_device_id cht_wc_acpi_ids[] = {
static struct i2c_driver cht_wc_driver = {
.driver = {
.name = "CHT Whiskey Cove PMIC",
- .pm = &cht_wc_pm_ops,
+ .pm = pm_sleep_ptr(&cht_wc_pm_ops),
.acpi_match_table = cht_wc_acpi_ids,
},
.probe_new = cht_wc_probe,
--
2.35.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v1 2/2] mfd: intel_soc_pmic_chtwc: Switch from __maybe_unused to pm_sleep_ptr() etc
2022-06-16 17:10 ` [PATCH v1 2/2] mfd: intel_soc_pmic_chtwc: Switch from __maybe_unused to pm_sleep_ptr() etc Andy Shevchenko
@ 2022-06-20 8:38 ` Hans de Goede
2022-06-20 9:39 ` Andy Shevchenko
2022-06-20 8:48 ` Hans de Goede
2022-06-27 13:05 ` Lee Jones
2 siblings, 1 reply; 9+ messages in thread
From: Hans de Goede @ 2022-06-20 8:38 UTC (permalink / raw)
To: Andy Shevchenko, linux-kernel; +Cc: Andy Shevchenko, Lee Jones
Hi,
On 6/16/22 19:10, Andy Shevchenko wrote:
> Letting the compiler remove these functions when the kernel is built
> without CONFIG_PM_SLEEP support is simpler and less heavier for builds
> than the use of __maybe_unused attributes.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
I don't see how this helps, __maybe_unused only means the compiler should not
warn AFAIK it is still allowed to remove static globals which are unused
even of they are marked __maybe_unused ?
And DEFINE_SIMPLE_DEV_PM_OPS does not mark the dev_pm_ops as
__maybe_unused where as pm_sleep_ptr() will not reference the struct
when CONFIG_PM is not set.
But I guess that the new PTR_IF magic:
#define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
Still counts as referencing the struct so we don't get
an unused warning and since cond is a const 0 value the compiler
can optimize things away ?
Regards,
Hans
> ---
> drivers/mfd/intel_soc_pmic_chtwc.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mfd/intel_soc_pmic_chtwc.c b/drivers/mfd/intel_soc_pmic_chtwc.c
> index 1cbe2bcce530..9216f0d34206 100644
> --- a/drivers/mfd/intel_soc_pmic_chtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_chtwc.c
> @@ -222,7 +222,7 @@ static void cht_wc_shutdown(struct i2c_client *client)
> disable_irq(pmic->irq);
> }
>
> -static int __maybe_unused cht_wc_suspend(struct device *dev)
> +static int cht_wc_suspend(struct device *dev)
> {
> struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
>
> @@ -231,7 +231,7 @@ static int __maybe_unused cht_wc_suspend(struct device *dev)
> return 0;
> }
>
> -static int __maybe_unused cht_wc_resume(struct device *dev)
> +static int cht_wc_resume(struct device *dev)
> {
> struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
>
> @@ -239,7 +239,7 @@ static int __maybe_unused cht_wc_resume(struct device *dev)
>
> return 0;
> }
> -static SIMPLE_DEV_PM_OPS(cht_wc_pm_ops, cht_wc_suspend, cht_wc_resume);
> +static DEFINE_SIMPLE_DEV_PM_OPS(cht_wc_pm_ops, cht_wc_suspend, cht_wc_resume);
>
> static const struct i2c_device_id cht_wc_i2c_id[] = {
> { }
> @@ -253,7 +253,7 @@ static const struct acpi_device_id cht_wc_acpi_ids[] = {
> static struct i2c_driver cht_wc_driver = {
> .driver = {
> .name = "CHT Whiskey Cove PMIC",
> - .pm = &cht_wc_pm_ops,
> + .pm = pm_sleep_ptr(&cht_wc_pm_ops),
> .acpi_match_table = cht_wc_acpi_ids,
> },
> .probe_new = cht_wc_probe,
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 2/2] mfd: intel_soc_pmic_chtwc: Switch from __maybe_unused to pm_sleep_ptr() etc
2022-06-20 8:38 ` Hans de Goede
@ 2022-06-20 9:39 ` Andy Shevchenko
0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2022-06-20 9:39 UTC (permalink / raw)
To: Hans de Goede; +Cc: linux-kernel, Andy Shevchenko, Lee Jones
On Mon, Jun 20, 2022 at 10:38:34AM +0200, Hans de Goede wrote:
> On 6/16/22 19:10, Andy Shevchenko wrote:
> > Letting the compiler remove these functions when the kernel is built
> > without CONFIG_PM_SLEEP support is simpler and less heavier for builds
> > than the use of __maybe_unused attributes.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> I don't see how this helps, __maybe_unused only means the compiler should not
> warn AFAIK it is still allowed to remove static globals which are unused
> even of they are marked __maybe_unused ?
__maybe_unused will force compiler to compile and linker to drop the section.
The proposed approach saves resources on build, i.e. it's already compiler that
decides not to compile the code.
> And DEFINE_SIMPLE_DEV_PM_OPS does not mark the dev_pm_ops as
> __maybe_unused where as pm_sleep_ptr() will not reference the struct
> when CONFIG_PM is not set.
>
> But I guess that the new PTR_IF magic:
>
> #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
>
> Still counts as referencing the struct so we don't get
> an unused warning and since cond is a const 0 value the compiler
> can optimize things away ?
See above, it's not olny about warning and compiler.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] mfd: intel_soc_pmic_chtwc: Switch from __maybe_unused to pm_sleep_ptr() etc
2022-06-16 17:10 ` [PATCH v1 2/2] mfd: intel_soc_pmic_chtwc: Switch from __maybe_unused to pm_sleep_ptr() etc Andy Shevchenko
2022-06-20 8:38 ` Hans de Goede
@ 2022-06-20 8:48 ` Hans de Goede
2022-06-20 9:40 ` Andy Shevchenko
2022-06-27 13:05 ` Lee Jones
2 siblings, 1 reply; 9+ messages in thread
From: Hans de Goede @ 2022-06-20 8:48 UTC (permalink / raw)
To: Andy Shevchenko, linux-kernel; +Cc: Andy Shevchenko, Lee Jones
Hi,
On 6/16/22 19:10, Andy Shevchenko wrote:
> Letting the compiler remove these functions when the kernel is built
> without CONFIG_PM_SLEEP support is simpler and less heavier for builds
> than the use of __maybe_unused attributes.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
I see that this basically converts the code to the new way of dealing
with pm_ops, so after a second look:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
> drivers/mfd/intel_soc_pmic_chtwc.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mfd/intel_soc_pmic_chtwc.c b/drivers/mfd/intel_soc_pmic_chtwc.c
> index 1cbe2bcce530..9216f0d34206 100644
> --- a/drivers/mfd/intel_soc_pmic_chtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_chtwc.c
> @@ -222,7 +222,7 @@ static void cht_wc_shutdown(struct i2c_client *client)
> disable_irq(pmic->irq);
> }
>
> -static int __maybe_unused cht_wc_suspend(struct device *dev)
> +static int cht_wc_suspend(struct device *dev)
> {
> struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
>
> @@ -231,7 +231,7 @@ static int __maybe_unused cht_wc_suspend(struct device *dev)
> return 0;
> }
>
> -static int __maybe_unused cht_wc_resume(struct device *dev)
> +static int cht_wc_resume(struct device *dev)
> {
> struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
>
> @@ -239,7 +239,7 @@ static int __maybe_unused cht_wc_resume(struct device *dev)
>
> return 0;
> }
> -static SIMPLE_DEV_PM_OPS(cht_wc_pm_ops, cht_wc_suspend, cht_wc_resume);
> +static DEFINE_SIMPLE_DEV_PM_OPS(cht_wc_pm_ops, cht_wc_suspend, cht_wc_resume);
>
> static const struct i2c_device_id cht_wc_i2c_id[] = {
> { }
> @@ -253,7 +253,7 @@ static const struct acpi_device_id cht_wc_acpi_ids[] = {
> static struct i2c_driver cht_wc_driver = {
> .driver = {
> .name = "CHT Whiskey Cove PMIC",
> - .pm = &cht_wc_pm_ops,
> + .pm = pm_sleep_ptr(&cht_wc_pm_ops),
> .acpi_match_table = cht_wc_acpi_ids,
> },
> .probe_new = cht_wc_probe,
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 2/2] mfd: intel_soc_pmic_chtwc: Switch from __maybe_unused to pm_sleep_ptr() etc
2022-06-20 8:48 ` Hans de Goede
@ 2022-06-20 9:40 ` Andy Shevchenko
0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2022-06-20 9:40 UTC (permalink / raw)
To: Hans de Goede; +Cc: linux-kernel, Andy Shevchenko, Lee Jones
On Mon, Jun 20, 2022 at 10:48:01AM +0200, Hans de Goede wrote:
> On 6/16/22 19:10, Andy Shevchenko wrote:
> > Letting the compiler remove these functions when the kernel is built
> > without CONFIG_PM_SLEEP support is simpler and less heavier for builds
> > than the use of __maybe_unused attributes.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> I see that this basically converts the code to the new way of dealing
> with pm_ops, so after a second look:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Thanks!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] mfd: intel_soc_pmic_chtwc: Switch from __maybe_unused to pm_sleep_ptr() etc
2022-06-16 17:10 ` [PATCH v1 2/2] mfd: intel_soc_pmic_chtwc: Switch from __maybe_unused to pm_sleep_ptr() etc Andy Shevchenko
2022-06-20 8:38 ` Hans de Goede
2022-06-20 8:48 ` Hans de Goede
@ 2022-06-27 13:05 ` Lee Jones
2 siblings, 0 replies; 9+ messages in thread
From: Lee Jones @ 2022-06-27 13:05 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Hans de Goede, linux-kernel, Andy Shevchenko
On Thu, 16 Jun 2022, Andy Shevchenko wrote:
> Letting the compiler remove these functions when the kernel is built
> without CONFIG_PM_SLEEP support is simpler and less heavier for builds
> than the use of __maybe_unused attributes.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/mfd/intel_soc_pmic_chtwc.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Applied, thanks.
--
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] mfd: intel_soc_pmic_chtwc: Use dev_err_probe()
2022-06-16 17:10 [PATCH v1 1/2] mfd: intel_soc_pmic_chtwc: Use dev_err_probe() Andy Shevchenko
2022-06-16 17:10 ` [PATCH v1 2/2] mfd: intel_soc_pmic_chtwc: Switch from __maybe_unused to pm_sleep_ptr() etc Andy Shevchenko
@ 2022-06-20 8:33 ` Hans de Goede
2022-06-27 13:05 ` Lee Jones
2 siblings, 0 replies; 9+ messages in thread
From: Hans de Goede @ 2022-06-20 8:33 UTC (permalink / raw)
To: Andy Shevchenko, linux-kernel; +Cc: Andy Shevchenko, Lee Jones
Hi,
On 6/16/22 19:10, Andy Shevchenko wrote:
> Simplify the mux error path a bit by using dev_err_probe().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
> drivers/mfd/intel_soc_pmic_chtwc.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mfd/intel_soc_pmic_chtwc.c b/drivers/mfd/intel_soc_pmic_chtwc.c
> index 4eab191e053a..1cbe2bcce530 100644
> --- a/drivers/mfd/intel_soc_pmic_chtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_chtwc.c
> @@ -179,18 +179,13 @@ static int cht_wc_probe(struct i2c_client *client)
> int ret;
>
> status = acpi_evaluate_integer(ACPI_HANDLE(dev), "_HRV", NULL, &hrv);
> - if (ACPI_FAILURE(status)) {
> - dev_err(dev, "Failed to get PMIC hardware revision\n");
> - return -ENODEV;
> - }
> - if (hrv != CHT_WC_HRV) {
> - dev_err(dev, "Invalid PMIC hardware revision: %llu\n", hrv);
> - return -ENODEV;
> - }
> - if (client->irq < 0) {
> - dev_err(dev, "Invalid IRQ\n");
> - return -EINVAL;
> - }
> + if (ACPI_FAILURE(status))
> + return dev_err_probe(dev, -ENODEV, "Failed to get PMIC hardware revision\n");
> + if (hrv != CHT_WC_HRV)
> + return dev_err_probe(dev, -ENODEV, "Invalid PMIC hardware revision: %llu\n", hrv);
> +
> + if (client->irq < 0)
> + return dev_err_probe(dev, -EINVAL, "Invalid IRQ\n");
>
> pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
> if (!pmic)
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 1/2] mfd: intel_soc_pmic_chtwc: Use dev_err_probe()
2022-06-16 17:10 [PATCH v1 1/2] mfd: intel_soc_pmic_chtwc: Use dev_err_probe() Andy Shevchenko
2022-06-16 17:10 ` [PATCH v1 2/2] mfd: intel_soc_pmic_chtwc: Switch from __maybe_unused to pm_sleep_ptr() etc Andy Shevchenko
2022-06-20 8:33 ` [PATCH v1 1/2] mfd: intel_soc_pmic_chtwc: Use dev_err_probe() Hans de Goede
@ 2022-06-27 13:05 ` Lee Jones
2 siblings, 0 replies; 9+ messages in thread
From: Lee Jones @ 2022-06-27 13:05 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Hans de Goede, linux-kernel, Andy Shevchenko
On Thu, 16 Jun 2022, Andy Shevchenko wrote:
> Simplify the mux error path a bit by using dev_err_probe().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/mfd/intel_soc_pmic_chtwc.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
Applied, thanks.
--
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-06-27 13:06 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-16 17:10 [PATCH v1 1/2] mfd: intel_soc_pmic_chtwc: Use dev_err_probe() Andy Shevchenko
2022-06-16 17:10 ` [PATCH v1 2/2] mfd: intel_soc_pmic_chtwc: Switch from __maybe_unused to pm_sleep_ptr() etc Andy Shevchenko
2022-06-20 8:38 ` Hans de Goede
2022-06-20 9:39 ` Andy Shevchenko
2022-06-20 8:48 ` Hans de Goede
2022-06-20 9:40 ` Andy Shevchenko
2022-06-27 13:05 ` Lee Jones
2022-06-20 8:33 ` [PATCH v1 1/2] mfd: intel_soc_pmic_chtwc: Use dev_err_probe() Hans de Goede
2022-06-27 13:05 ` Lee Jones
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.