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

[Guenter Roeck]

On 4/18/23 08:28, nick.hawkins@xxxxxxx wrote:
> From: Nick Hawkins <nick.hawkins@xxxxxxx>
>
> 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@xxxxxxx>
> ---
>   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,