diff mbox series

[v1,2/9] hwmon: (gxp_fan_ctrl) Give GPIO access to fan data

Message ID 20230418152824.110823-3-nick.hawkins@hpe.com
State New
Headers show
Series ARM: Add GPIO and PSU Support | expand

Commit Message

Hawkins, Nick April 18, 2023, 3:28 p.m. UTC 
From: Nick Hawkins <nick.hawkins@hpe.com>

The fan driver has access to the IO that reports fan presence.
Add the ability for other drivers such as GPIO to call in
to get fan information. Also remove the system power check as
the GPIO driver needs it to report power state.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
---
 drivers/hwmon/gxp-fan-ctrl.c | 58 +++++++++++++++---------------------
 1 file changed, 24 insertions(+), 34 deletions(-)

Comments

Guenter Roeck April 18, 2023, 5:10 p.m. UTC | #1 
On 4/18/23 08:28, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> The fan driver has access to the IO that reports fan presence.
> Add the ability for other drivers such as GPIO to call in
> to get fan information. Also remove the system power check as
> the GPIO driver needs it to report power state.
> 

Sorry, I am lost right here. Why would "gpio" want to know about
or report fan information ?

Guenter

> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> ---
>   drivers/hwmon/gxp-fan-ctrl.c | 58 +++++++++++++++---------------------
>   1 file changed, 24 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/hwmon/gxp-fan-ctrl.c b/drivers/hwmon/gxp-fan-ctrl.c
> index 0014b8b0fd41..a8fcea98cc55 100644
> --- a/drivers/hwmon/gxp-fan-ctrl.c
> +++ b/drivers/hwmon/gxp-fan-ctrl.c
> @@ -1,5 +1,5 @@
>   // SPDX-License-Identifier: GPL-2.0-only
> -/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P. */
> +/* Copyright (C) 2023 Hewlett-Packard Enterprise Development Company, L.P. */
>   
>   #include <linux/bits.h>
>   #include <linux/err.h>
> @@ -11,15 +11,14 @@
>   
>   #define OFS_FAN_INST	0 /* Is 0 because plreg base will be set at INST */
>   #define OFS_FAN_FAIL	2 /* Is 2 bytes after base */
> -#define OFS_SEVSTAT	0 /* Is 0 because fn2 base will be set at SEVSTAT */
> -#define POWER_BIT	24
>   
>   struct gxp_fan_ctrl_drvdata {
>   	void __iomem	*base;
>   	void __iomem	*plreg;
> -	void __iomem	*fn2;
>   };
>   
> +struct gxp_fan_ctrl_drvdata *drvdata;
> +
>   static bool fan_installed(struct device *dev, int fan)
>   {
>   	struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
> @@ -30,6 +29,16 @@ static bool fan_installed(struct device *dev, int fan)
>   	return !!(val & BIT(fan));
>   }
>   
> +u8 get_fans_installed(void)
> +{
> +	static u8 val;
> +
> +	val = readb(drvdata->plreg + OFS_FAN_INST);
> +
> +	return val;
> +}
> +EXPORT_SYMBOL(get_fans_installed);
> +
>   static long fan_failed(struct device *dev, int fan)
>   {
>   	struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
> @@ -40,19 +49,19 @@ static long fan_failed(struct device *dev, int fan)
>   	return !!(val & BIT(fan));
>   }
>   
> -static long fan_enabled(struct device *dev, int fan)
> +u8 get_fans_failed(void)
>   {
> -	struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
> -	u32 val;
> +	static u8 val;
>   
> -	/*
> -	 * Check the power status as if the platform is off the value
> -	 * reported for the PWM will be incorrect. Report fan as
> -	 * disabled.
> -	 */
> -	val = readl(drvdata->fn2 + OFS_SEVSTAT);
> +	val = readb(drvdata->plreg + OFS_FAN_FAIL);
> +
> +	return val;
> +}
> +EXPORT_SYMBOL(get_fans_failed);
>   
> -	return !!((val & BIT(POWER_BIT)) && fan_installed(dev, fan));
> +static long fan_enabled(struct device *dev, int fan)
> +{
> +	return !!(fan_installed(dev, fan));
>   }
>   
>   static int gxp_pwm_write(struct device *dev, u32 attr, int channel, long val)
> @@ -98,20 +107,8 @@ static int gxp_fan_read(struct device *dev, u32 attr, int channel, long *val)
>   static int gxp_pwm_read(struct device *dev, u32 attr, int channel, long *val)
>   {
>   	struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
> -	u32 reg;
> -
> -	/*
> -	 * Check the power status of the platform. If the platform is off
> -	 * the value reported for the PWM will be incorrect. In this case
> -	 * report a PWM of zero.
> -	 */
>   
> -	reg = readl(drvdata->fn2 + OFS_SEVSTAT);
> -
> -	if (reg & BIT(POWER_BIT))
> -		*val = fan_installed(dev, channel) ? readb(drvdata->base + channel) : 0;
> -	else
> -		*val = 0;
> +	*val = fan_installed(dev, channel) ? readb(drvdata->base + channel) : 0;
>   
>   	return 0;
>   }
> @@ -198,7 +195,6 @@ static const struct hwmon_chip_info gxp_fan_ctrl_chip_info = {
>   
>   static int gxp_fan_ctrl_probe(struct platform_device *pdev)
>   {
> -	struct gxp_fan_ctrl_drvdata *drvdata;
>   	struct device *dev = &pdev->dev;
>   	struct device *hwmon_dev;
>   
> @@ -218,12 +214,6 @@ static int gxp_fan_ctrl_probe(struct platform_device *pdev)
>   		return dev_err_probe(dev, PTR_ERR(drvdata->plreg),
>   				     "failed to map plreg\n");
>   
> -	drvdata->fn2 = devm_platform_ioremap_resource_byname(pdev,
> -							     "fn2");
> -	if (IS_ERR(drvdata->fn2))
> -		return dev_err_probe(dev, PTR_ERR(drvdata->fn2),
> -				     "failed to map fn2\n");
> -
>   	hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
>   							 "hpe_gxp_fan_ctrl",
>   							 drvdata,
kernel test robot April 19, 2023, 3:01 p.m. UTC | #2 
Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on robh/for-next linus/master v6.3-rc7]
[cannot apply to brgl/gpio/for-next next-20230418]
[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/nick-hawkins-hpe-com/gpio-gxp-Add-HPE-GXP-GPIO/20230418-233513
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20230418152824.110823-3-nick.hawkins%40hpe.com
patch subject: [PATCH v1 2/9] hwmon: (gxp_fan_ctrl) Give GPIO access to fan data
config: hexagon-randconfig-r045-20230419 (https://download.01.org/0day-ci/archive/20230419/202304192243.9hwJ1Cad-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 437b7602e4a998220871de78afcb020b9c14a661)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/be3722d7f32fea1ea375090d05cbfdd3dd4e04d3
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review nick-hawkins-hpe-com/gpio-gxp-Add-HPE-GXP-GPIO/20230418-233513
        git checkout be3722d7f32fea1ea375090d05cbfdd3dd4e04d3
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash block/partitions/ drivers/hwmon/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304192243.9hwJ1Cad-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/hwmon/gxp-fan-ctrl.c:7:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from drivers/hwmon/gxp-fan-ctrl.c:7:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from drivers/hwmon/gxp-fan-ctrl.c:7:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
>> drivers/hwmon/gxp-fan-ctrl.c:32:4: warning: no previous prototype for function 'get_fans_installed' [-Wmissing-prototypes]
   u8 get_fans_installed(void)
      ^
   drivers/hwmon/gxp-fan-ctrl.c:32:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   u8 get_fans_installed(void)
   ^
   static 
>> drivers/hwmon/gxp-fan-ctrl.c:52:4: warning: no previous prototype for function 'get_fans_failed' [-Wmissing-prototypes]
   u8 get_fans_failed(void)
      ^
   drivers/hwmon/gxp-fan-ctrl.c:52:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   u8 get_fans_failed(void)
   ^
   static 
   8 warnings generated.


vim +/get_fans_installed +32 drivers/hwmon/gxp-fan-ctrl.c

    31	
  > 32	u8 get_fans_installed(void)
    33	{
    34		static u8 val;
    35	
    36		val = readb(drvdata->plreg + OFS_FAN_INST);
    37	
    38		return val;
    39	}
    40	EXPORT_SYMBOL(get_fans_installed);
    41	
    42	static long fan_failed(struct device *dev, int fan)
    43	{
    44		struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
    45		u8 val;
    46	
    47		val = readb(drvdata->plreg + OFS_FAN_FAIL);
    48	
    49		return !!(val & BIT(fan));
    50	}
    51	
  > 52	u8 get_fans_failed(void)
    53	{
    54		static u8 val;
    55	
    56		val = readb(drvdata->plreg + OFS_FAN_FAIL);
    57	
    58		return val;
    59	}
    60	EXPORT_SYMBOL(get_fans_failed);
    61
diff mbox series

Patch

diff --git a/drivers/hwmon/gxp-fan-ctrl.c b/drivers/hwmon/gxp-fan-ctrl.c
index 0014b8b0fd41..a8fcea98cc55 100644
--- a/drivers/hwmon/gxp-fan-ctrl.c
+++ b/drivers/hwmon/gxp-fan-ctrl.c
@@ -1,5 +1,5 @@ 
 // SPDX-License-Identifier: GPL-2.0-only
-/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P. */
+/* Copyright (C) 2023 Hewlett-Packard Enterprise Development Company, L.P. */
 
 #include <linux/bits.h>
 #include <linux/err.h>
@@ -11,15 +11,14 @@ 
 
 #define OFS_FAN_INST	0 /* Is 0 because plreg base will be set at INST */
 #define OFS_FAN_FAIL	2 /* Is 2 bytes after base */
-#define OFS_SEVSTAT	0 /* Is 0 because fn2 base will be set at SEVSTAT */
-#define POWER_BIT	24
 
 struct gxp_fan_ctrl_drvdata {
 	void __iomem	*base;
 	void __iomem	*plreg;
-	void __iomem	*fn2;
 };
 
+struct gxp_fan_ctrl_drvdata *drvdata;
+
 static bool fan_installed(struct device *dev, int fan)
 {
 	struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
@@ -30,6 +29,16 @@  static bool fan_installed(struct device *dev, int fan)
 	return !!(val & BIT(fan));
 }
 
+u8 get_fans_installed(void)
+{
+	static u8 val;
+
+	val = readb(drvdata->plreg + OFS_FAN_INST);
+
+	return val;
+}
+EXPORT_SYMBOL(get_fans_installed);
+
 static long fan_failed(struct device *dev, int fan)
 {
 	struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
@@ -40,19 +49,19 @@  static long fan_failed(struct device *dev, int fan)
 	return !!(val & BIT(fan));
 }
 
-static long fan_enabled(struct device *dev, int fan)
+u8 get_fans_failed(void)
 {
-	struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
-	u32 val;
+	static u8 val;
 
-	/*
-	 * Check the power status as if the platform is off the value
-	 * reported for the PWM will be incorrect. Report fan as
-	 * disabled.
-	 */
-	val = readl(drvdata->fn2 + OFS_SEVSTAT);
+	val = readb(drvdata->plreg + OFS_FAN_FAIL);
+
+	return val;
+}
+EXPORT_SYMBOL(get_fans_failed);
 
-	return !!((val & BIT(POWER_BIT)) && fan_installed(dev, fan));
+static long fan_enabled(struct device *dev, int fan)
+{
+	return !!(fan_installed(dev, fan));
 }
 
 static int gxp_pwm_write(struct device *dev, u32 attr, int channel, long val)
@@ -98,20 +107,8 @@  static int gxp_fan_read(struct device *dev, u32 attr, int channel, long *val)
 static int gxp_pwm_read(struct device *dev, u32 attr, int channel, long *val)
 {
 	struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
-	u32 reg;
-
-	/*
-	 * Check the power status of the platform. If the platform is off
-	 * the value reported for the PWM will be incorrect. In this case
-	 * report a PWM of zero.
-	 */
 
-	reg = readl(drvdata->fn2 + OFS_SEVSTAT);
-
-	if (reg & BIT(POWER_BIT))
-		*val = fan_installed(dev, channel) ? readb(drvdata->base + channel) : 0;
-	else
-		*val = 0;
+	*val = fan_installed(dev, channel) ? readb(drvdata->base + channel) : 0;
 
 	return 0;
 }
@@ -198,7 +195,6 @@  static const struct hwmon_chip_info gxp_fan_ctrl_chip_info = {
 
 static int gxp_fan_ctrl_probe(struct platform_device *pdev)
 {
-	struct gxp_fan_ctrl_drvdata *drvdata;
 	struct device *dev = &pdev->dev;
 	struct device *hwmon_dev;
 
@@ -218,12 +214,6 @@  static int gxp_fan_ctrl_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(drvdata->plreg),
 				     "failed to map plreg\n");
 
-	drvdata->fn2 = devm_platform_ioremap_resource_byname(pdev,
-							     "fn2");
-	if (IS_ERR(drvdata->fn2))
-		return dev_err_probe(dev, PTR_ERR(drvdata->fn2),
-				     "failed to map fn2\n");
-
 	hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
 							 "hpe_gxp_fan_ctrl",
 							 drvdata,