[PATCH v1 3/3] pinctrl: cherryview: Replace ifdeffery by pm_sleep_ptr() macro

Andy Shevchenko posted 3 patches 1 year, 3 months ago
[PATCH v1 3/3] pinctrl: cherryview: Replace ifdeffery by pm_sleep_ptr() macro
Posted by Andy Shevchenko 1 year, 3 months ago
Explicit ifdeffery is ugly and theoretically might be not synchronised
with the rest of functions that are assigned via pm_sleep_ptr() macro.
Replace ifdeffery by pm_sleep_ptr() macro to improve this.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 2f0e29c78dfb..9cdffd73e345 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1608,6 +1608,16 @@ static acpi_status chv_pinctrl_mmio_access_handler(u32 function,
 	return AE_OK;
 }
 
+static int chv_pinctrl_pm_init(struct intel_pinctrl *pctrl)
+{
+	pctrl->context.pads = devm_kcalloc(pctrl->dev, pctrl->soc->npins,
+					   sizeof(*pctrl->context.pads), GFP_KERNEL);
+	if (!pctrl->context.pads)
+		return -ENOMEM;
+
+	return 0;
+}
+
 static int chv_pinctrl_probe(struct platform_device *pdev)
 {
 	const struct intel_pinctrl_soc_data *soc_data;
@@ -1648,13 +1658,9 @@ static int chv_pinctrl_probe(struct platform_device *pdev)
 
 	community->pad_regs = community->regs + FAMILY_PAD_REGS_OFF;
 
-#ifdef CONFIG_PM_SLEEP
-	pctrl->context.pads = devm_kcalloc(dev, pctrl->soc->npins,
-					   sizeof(*pctrl->context.pads),
-					   GFP_KERNEL);
-	if (!pctrl->context.pads)
-		return -ENOMEM;
-#endif
+	ret = pm_sleep_ptr(chv_pinctrl_pm_init) ? chv_pinctrl_pm_init(pctrl) : 0;
+	if (ret)
+		return ret;
 
 	pctrl->context.communities = devm_kcalloc(dev, pctrl->soc->ncommunities,
 						  sizeof(*pctrl->context.communities),
-- 
2.43.0.rc1.1336.g36b5255a03ac
Re: [PATCH v1 3/3] pinctrl: cherryview: Replace ifdeffery by pm_sleep_ptr() macro
Posted by kernel test robot 1 year, 3 months ago
Hi Andy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linusw-pinctrl/devel]
[also build test WARNING on linusw-pinctrl/for-next linus/master v6.11-rc6 next-20240904]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/pinctrl-intel-Replace-ifdeffery-by-pm_sleep_ptr-macro/20240904-011041
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
patch link:    https://lore.kernel.org/r/20240903170752.3564538-4-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v1 3/3] pinctrl: cherryview: Replace ifdeffery by pm_sleep_ptr() macro
config: x86_64-randconfig-r073-20240904 (https://download.01.org/0day-ci/archive/20240904/202409041939.VrJGEW4H-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240904/202409041939.VrJGEW4H-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409041939.VrJGEW4H-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/pinctrl/intel/pinctrl-cherryview.c: In function 'chv_pinctrl_probe':
>> drivers/pinctrl/intel/pinctrl-cherryview.c:1657:49: warning: the address of 'chv_pinctrl_pm_init' will always evaluate as 'true' [-Waddress]
    1657 |         ret = pm_sleep_ptr(chv_pinctrl_pm_init) ? chv_pinctrl_pm_init(pctrl) : 0;
         |                                                 ^


vim +1657 drivers/pinctrl/intel/pinctrl-cherryview.c

  1620	
  1621	static int chv_pinctrl_probe(struct platform_device *pdev)
  1622	{
  1623		const struct intel_pinctrl_soc_data *soc_data;
  1624		struct intel_community_context *cctx;
  1625		struct intel_community *community;
  1626		struct device *dev = &pdev->dev;
  1627		struct intel_pinctrl *pctrl;
  1628		acpi_status status;
  1629		unsigned int i;
  1630		int ret, irq;
  1631	
  1632		soc_data = intel_pinctrl_get_soc_data(pdev);
  1633		if (IS_ERR(soc_data))
  1634			return PTR_ERR(soc_data);
  1635	
  1636		pctrl = devm_kzalloc(dev, sizeof(*pctrl), GFP_KERNEL);
  1637		if (!pctrl)
  1638			return -ENOMEM;
  1639	
  1640		pctrl->dev = dev;
  1641		pctrl->soc = soc_data;
  1642	
  1643		pctrl->ncommunities = pctrl->soc->ncommunities;
  1644		pctrl->communities = devm_kmemdup(dev, pctrl->soc->communities,
  1645						  pctrl->ncommunities * sizeof(*pctrl->communities),
  1646						  GFP_KERNEL);
  1647		if (!pctrl->communities)
  1648			return -ENOMEM;
  1649	
  1650		community = &pctrl->communities[0];
  1651		community->regs = devm_platform_ioremap_resource(pdev, 0);
  1652		if (IS_ERR(community->regs))
  1653			return PTR_ERR(community->regs);
  1654	
  1655		community->pad_regs = community->regs + FAMILY_PAD_REGS_OFF;
  1656	
> 1657		ret = pm_sleep_ptr(chv_pinctrl_pm_init) ? chv_pinctrl_pm_init(pctrl) : 0;
  1658		if (ret)
  1659			return ret;
  1660	
  1661		pctrl->context.communities = devm_kcalloc(dev, pctrl->soc->ncommunities,
  1662							  sizeof(*pctrl->context.communities),
  1663							  GFP_KERNEL);
  1664		if (!pctrl->context.communities)
  1665			return -ENOMEM;
  1666	
  1667		cctx = &pctrl->context.communities[0];
  1668		for (i = 0; i < ARRAY_SIZE(cctx->intr_lines); i++)
  1669			cctx->intr_lines[i] = CHV_INVALID_HWIRQ;
  1670	
  1671		irq = platform_get_irq(pdev, 0);
  1672		if (irq < 0)
  1673			return irq;
  1674	
  1675		pctrl->pctldesc = chv_pinctrl_desc;
  1676		pctrl->pctldesc.name = dev_name(dev);
  1677		pctrl->pctldesc.pins = pctrl->soc->pins;
  1678		pctrl->pctldesc.npins = pctrl->soc->npins;
  1679	
  1680		pctrl->pctldev = devm_pinctrl_register(dev, &pctrl->pctldesc, pctrl);
  1681		if (IS_ERR(pctrl->pctldev)) {
  1682			dev_err(dev, "failed to register pinctrl driver\n");
  1683			return PTR_ERR(pctrl->pctldev);
  1684		}
  1685	
  1686		ret = chv_gpio_probe(pctrl, irq);
  1687		if (ret)
  1688			return ret;
  1689	
  1690		status = acpi_install_address_space_handler(ACPI_HANDLE(dev),
  1691						community->acpi_space_id,
  1692						chv_pinctrl_mmio_access_handler,
  1693						NULL, pctrl);
  1694		if (ACPI_FAILURE(status))
  1695			dev_err(dev, "failed to install ACPI addr space handler\n");
  1696	
  1697		platform_set_drvdata(pdev, pctrl);
  1698	
  1699		return 0;
  1700	}
  1701	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v1 3/3] pinctrl: cherryview: Replace ifdeffery by pm_sleep_ptr() macro
Posted by Mika Westerberg 1 year, 3 months ago
On Tue, Sep 03, 2024 at 08:04:51PM +0300, Andy Shevchenko wrote:
> Explicit ifdeffery is ugly and theoretically might be not synchronised
> with the rest of functions that are assigned via pm_sleep_ptr() macro.
> Replace ifdeffery by pm_sleep_ptr() macro to improve this.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-cherryview.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
> index 2f0e29c78dfb..9cdffd73e345 100644
> --- a/drivers/pinctrl/intel/pinctrl-cherryview.c
> +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
> @@ -1608,6 +1608,16 @@ static acpi_status chv_pinctrl_mmio_access_handler(u32 function,
>  	return AE_OK;
>  }
>  
> +static int chv_pinctrl_pm_init(struct intel_pinctrl *pctrl)
> +{
> +	pctrl->context.pads = devm_kcalloc(pctrl->dev, pctrl->soc->npins,
> +					   sizeof(*pctrl->context.pads), GFP_KERNEL);
> +	if (!pctrl->context.pads)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
>  static int chv_pinctrl_probe(struct platform_device *pdev)
>  {
>  	const struct intel_pinctrl_soc_data *soc_data;
> @@ -1648,13 +1658,9 @@ static int chv_pinctrl_probe(struct platform_device *pdev)
>  
>  	community->pad_regs = community->regs + FAMILY_PAD_REGS_OFF;
>  
> -#ifdef CONFIG_PM_SLEEP
> -	pctrl->context.pads = devm_kcalloc(dev, pctrl->soc->npins,
> -					   sizeof(*pctrl->context.pads),
> -					   GFP_KERNEL);
> -	if (!pctrl->context.pads)
> -		return -ENOMEM;
> -#endif
> +	ret = pm_sleep_ptr(chv_pinctrl_pm_init) ? chv_pinctrl_pm_init(pctrl) : 0;

and here

> +	if (ret)
> +		return ret;
>  
>  	pctrl->context.communities = devm_kcalloc(dev, pctrl->soc->ncommunities,
>  						  sizeof(*pctrl->context.communities),
> -- 
> 2.43.0.rc1.1336.g36b5255a03ac