All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Pawel Moll <pawel.moll@arm.com>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Samuel Ortiz <sameo@linux.intel.com>,
	Arnd Bergmann <arnd@arndb.de>, Jon Medhurst <tixy@linaro.org>,
	arm@kernel.org, Olof Johansson <olof@lixom.net>,
	Jean Delvare <khali@linux-fr.org>,
	lm-sensors@lm-sensors.org
Subject: Re: [lm-sensors] [RFC 08/18] hwmon: vexpress: Use regmap instead of custom interface
Date: Mon, 23 Dec 2013 17:31:39 +0000	[thread overview]
Message-ID: <20131223173139.GA1678@roeck-us.net> (raw)
In-Reply-To: <1387815830-8794-9-git-send-email-pawel.moll@arm.com>

On Mon, Dec 23, 2013 at 04:23:40PM +0000, Pawel Moll wrote:
> This patch makes the Versatile Express hwmon driver
> use regmap interface, instead of custom vexpress config
> one. It will request the regmap resource associated
> with the device, which makes it pretty much hardware
> agnostic.
> 
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> Cc: Jean Delvare <khali@linux-fr.org>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: lm-sensors@lm-sensors.org
> ---
>  drivers/hwmon/Kconfig    |  3 ++-
>  drivers/hwmon/vexpress.c | 29 +++++++++++------------------
>  2 files changed, 13 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 52d548f..7747a47 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1324,7 +1324,8 @@ config SENSORS_TWL4030_MADC
>  
>  config SENSORS_VEXPRESS
>  	tristate "Versatile Express"
> -	depends on VEXPRESS_CONFIG
> +	depends on ARM || ARM64
> +	depends on REGMAP
>  	help
>  	  This driver provides support for hardware sensors available on
>  	  the ARM Ltd's Versatile Express platform. It can provide wide
> diff --git a/drivers/hwmon/vexpress.c b/drivers/hwmon/vexpress.c
> index d867e6b..b58cf1c 100644
> --- a/drivers/hwmon/vexpress.c
> +++ b/drivers/hwmon/vexpress.c
> @@ -22,11 +22,11 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
> -#include <linux/vexpress.h>
> +#include <linux/regmap.h>
>  
>  struct vexpress_hwmon_data {
>  	struct device *hwmon_dev;
> -	struct vexpress_config_func *func;
> +	struct regmap *reg;
>  };
>  
>  static ssize_t vexpress_hwmon_name_show(struct device *dev,
> @@ -56,7 +56,7 @@ static ssize_t vexpress_hwmon_u32_show(struct device *dev,
>  	int err;
>  	u32 value;
>  
> -	err = vexpress_config_read(data->func, 0, &value);
> +	err = regmap_read(data->reg, 0, &value);
>  	if (err)
>  		return err;
>  
> @@ -69,13 +69,13 @@ static ssize_t vexpress_hwmon_u64_show(struct device *dev,
>  {
>  	struct vexpress_hwmon_data *data = dev_get_drvdata(dev);
>  	int err;
> -	u32 value_hi, value_lo;
> +	unsigned int value_hi, value_lo;
>  
> -	err = vexpress_config_read(data->func, 0, &value_lo);
> +	err = regmap_read(data->reg, 0, &value_lo);
>  	if (err)
>  		return err;
>  
> -	err = vexpress_config_read(data->func, 1, &value_hi);
> +	err = regmap_read(data->reg, 1, &value_hi);
>  	if (err)
>  		return err;
>  
> @@ -175,26 +175,21 @@ static int vexpress_hwmon_probe(struct platform_device *pdev)
>  	if (!match)
>  		return -ENODEV;
>  
> -	data->func = vexpress_config_func_get_by_dev(&pdev->dev);
> -	if (!data->func)
> +	data->reg = dev_get_regmap(&pdev->dev, NULL);
> +	if (!data->reg)
>  		return -ENODEV;
>  
>  	err = sysfs_create_group(&pdev->dev.kobj, match->data);
>  	if (err)
> -		goto error;
> +		return err;
>  
>  	data->hwmon_dev = hwmon_device_register(&pdev->dev);
>  	if (IS_ERR(data->hwmon_dev)) {
> -		err = PTR_ERR(data->hwmon_dev);
> -		goto error;
> +		sysfs_remove_group(&pdev->dev.kobj, match->data);
> +		return PTR_ERR(data->hwmon_dev);

This change is unrelated and violates coding style. If the goto disturbs you,
I would suggest to convert the driver to use devm_hwmon_device_register_with_groups() 
in a separate patch and get rid of the error handling and the entire remove() function.

Thanks,
Guenter

>  	}
>  
>  	return 0;
> -
> -error:
> -	sysfs_remove_group(&pdev->dev.kobj, match->data);
> -	vexpress_config_func_put(data->func);
> -	return err;
>  }
>  
>  static int vexpress_hwmon_remove(struct platform_device *pdev)
> @@ -207,8 +202,6 @@ static int vexpress_hwmon_remove(struct platform_device *pdev)
>  	match = of_match_device(vexpress_hwmon_of_match, &pdev->dev);
>  	sysfs_remove_group(&pdev->dev.kobj, match->data);
>  
> -	vexpress_config_func_put(data->func);
> -
>  	return 0;
>  }
>  
> -- 
> 1.8.3.2
> 
> 
> 

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

WARNING: multiple messages have this Message-ID (diff)
From: linux@roeck-us.net (Guenter Roeck)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 08/18] hwmon: vexpress: Use regmap instead of custom interface
Date: Mon, 23 Dec 2013 09:31:39 -0800	[thread overview]
Message-ID: <20131223173139.GA1678@roeck-us.net> (raw)
In-Reply-To: <1387815830-8794-9-git-send-email-pawel.moll@arm.com>

On Mon, Dec 23, 2013 at 04:23:40PM +0000, Pawel Moll wrote:
> This patch makes the Versatile Express hwmon driver
> use regmap interface, instead of custom vexpress config
> one. It will request the regmap resource associated
> with the device, which makes it pretty much hardware
> agnostic.
> 
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> Cc: Jean Delvare <khali@linux-fr.org>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: lm-sensors at lm-sensors.org
> ---
>  drivers/hwmon/Kconfig    |  3 ++-
>  drivers/hwmon/vexpress.c | 29 +++++++++++------------------
>  2 files changed, 13 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 52d548f..7747a47 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1324,7 +1324,8 @@ config SENSORS_TWL4030_MADC
>  
>  config SENSORS_VEXPRESS
>  	tristate "Versatile Express"
> -	depends on VEXPRESS_CONFIG
> +	depends on ARM || ARM64
> +	depends on REGMAP
>  	help
>  	  This driver provides support for hardware sensors available on
>  	  the ARM Ltd's Versatile Express platform. It can provide wide
> diff --git a/drivers/hwmon/vexpress.c b/drivers/hwmon/vexpress.c
> index d867e6b..b58cf1c 100644
> --- a/drivers/hwmon/vexpress.c
> +++ b/drivers/hwmon/vexpress.c
> @@ -22,11 +22,11 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
> -#include <linux/vexpress.h>
> +#include <linux/regmap.h>
>  
>  struct vexpress_hwmon_data {
>  	struct device *hwmon_dev;
> -	struct vexpress_config_func *func;
> +	struct regmap *reg;
>  };
>  
>  static ssize_t vexpress_hwmon_name_show(struct device *dev,
> @@ -56,7 +56,7 @@ static ssize_t vexpress_hwmon_u32_show(struct device *dev,
>  	int err;
>  	u32 value;
>  
> -	err = vexpress_config_read(data->func, 0, &value);
> +	err = regmap_read(data->reg, 0, &value);
>  	if (err)
>  		return err;
>  
> @@ -69,13 +69,13 @@ static ssize_t vexpress_hwmon_u64_show(struct device *dev,
>  {
>  	struct vexpress_hwmon_data *data = dev_get_drvdata(dev);
>  	int err;
> -	u32 value_hi, value_lo;
> +	unsigned int value_hi, value_lo;
>  
> -	err = vexpress_config_read(data->func, 0, &value_lo);
> +	err = regmap_read(data->reg, 0, &value_lo);
>  	if (err)
>  		return err;
>  
> -	err = vexpress_config_read(data->func, 1, &value_hi);
> +	err = regmap_read(data->reg, 1, &value_hi);
>  	if (err)
>  		return err;
>  
> @@ -175,26 +175,21 @@ static int vexpress_hwmon_probe(struct platform_device *pdev)
>  	if (!match)
>  		return -ENODEV;
>  
> -	data->func = vexpress_config_func_get_by_dev(&pdev->dev);
> -	if (!data->func)
> +	data->reg = dev_get_regmap(&pdev->dev, NULL);
> +	if (!data->reg)
>  		return -ENODEV;
>  
>  	err = sysfs_create_group(&pdev->dev.kobj, match->data);
>  	if (err)
> -		goto error;
> +		return err;
>  
>  	data->hwmon_dev = hwmon_device_register(&pdev->dev);
>  	if (IS_ERR(data->hwmon_dev)) {
> -		err = PTR_ERR(data->hwmon_dev);
> -		goto error;
> +		sysfs_remove_group(&pdev->dev.kobj, match->data);
> +		return PTR_ERR(data->hwmon_dev);

This change is unrelated and violates coding style. If the goto disturbs you,
I would suggest to convert the driver to use devm_hwmon_device_register_with_groups() 
in a separate patch and get rid of the error handling and the entire remove() function.

Thanks,
Guenter

>  	}
>  
>  	return 0;
> -
> -error:
> -	sysfs_remove_group(&pdev->dev.kobj, match->data);
> -	vexpress_config_func_put(data->func);
> -	return err;
>  }
>  
>  static int vexpress_hwmon_remove(struct platform_device *pdev)
> @@ -207,8 +202,6 @@ static int vexpress_hwmon_remove(struct platform_device *pdev)
>  	match = of_match_device(vexpress_hwmon_of_match, &pdev->dev);
>  	sysfs_remove_group(&pdev->dev.kobj, match->data);
>  
> -	vexpress_config_func_put(data->func);
> -
>  	return 0;
>  }
>  
> -- 
> 1.8.3.2
> 
> 
> 

WARNING: multiple messages have this Message-ID (diff)
From: Guenter Roeck <linux@roeck-us.net>
To: Pawel Moll <pawel.moll@arm.com>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Samuel Ortiz <sameo@linux.intel.com>,
	Arnd Bergmann <arnd@arndb.de>, Jon Medhurst <tixy@linaro.org>,
	arm@kernel.org, Olof Johansson <olof@lixom.net>,
	Jean Delvare <khali@linux-fr.org>,
	lm-sensors@lm-sensors.org
Subject: Re: [RFC 08/18] hwmon: vexpress: Use regmap instead of custom interface
Date: Mon, 23 Dec 2013 09:31:39 -0800	[thread overview]
Message-ID: <20131223173139.GA1678@roeck-us.net> (raw)
In-Reply-To: <1387815830-8794-9-git-send-email-pawel.moll@arm.com>

On Mon, Dec 23, 2013 at 04:23:40PM +0000, Pawel Moll wrote:
> This patch makes the Versatile Express hwmon driver
> use regmap interface, instead of custom vexpress config
> one. It will request the regmap resource associated
> with the device, which makes it pretty much hardware
> agnostic.
> 
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> Cc: Jean Delvare <khali@linux-fr.org>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: lm-sensors@lm-sensors.org
> ---
>  drivers/hwmon/Kconfig    |  3 ++-
>  drivers/hwmon/vexpress.c | 29 +++++++++++------------------
>  2 files changed, 13 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 52d548f..7747a47 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1324,7 +1324,8 @@ config SENSORS_TWL4030_MADC
>  
>  config SENSORS_VEXPRESS
>  	tristate "Versatile Express"
> -	depends on VEXPRESS_CONFIG
> +	depends on ARM || ARM64
> +	depends on REGMAP
>  	help
>  	  This driver provides support for hardware sensors available on
>  	  the ARM Ltd's Versatile Express platform. It can provide wide
> diff --git a/drivers/hwmon/vexpress.c b/drivers/hwmon/vexpress.c
> index d867e6b..b58cf1c 100644
> --- a/drivers/hwmon/vexpress.c
> +++ b/drivers/hwmon/vexpress.c
> @@ -22,11 +22,11 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
> -#include <linux/vexpress.h>
> +#include <linux/regmap.h>
>  
>  struct vexpress_hwmon_data {
>  	struct device *hwmon_dev;
> -	struct vexpress_config_func *func;
> +	struct regmap *reg;
>  };
>  
>  static ssize_t vexpress_hwmon_name_show(struct device *dev,
> @@ -56,7 +56,7 @@ static ssize_t vexpress_hwmon_u32_show(struct device *dev,
>  	int err;
>  	u32 value;
>  
> -	err = vexpress_config_read(data->func, 0, &value);
> +	err = regmap_read(data->reg, 0, &value);
>  	if (err)
>  		return err;
>  
> @@ -69,13 +69,13 @@ static ssize_t vexpress_hwmon_u64_show(struct device *dev,
>  {
>  	struct vexpress_hwmon_data *data = dev_get_drvdata(dev);
>  	int err;
> -	u32 value_hi, value_lo;
> +	unsigned int value_hi, value_lo;
>  
> -	err = vexpress_config_read(data->func, 0, &value_lo);
> +	err = regmap_read(data->reg, 0, &value_lo);
>  	if (err)
>  		return err;
>  
> -	err = vexpress_config_read(data->func, 1, &value_hi);
> +	err = regmap_read(data->reg, 1, &value_hi);
>  	if (err)
>  		return err;
>  
> @@ -175,26 +175,21 @@ static int vexpress_hwmon_probe(struct platform_device *pdev)
>  	if (!match)
>  		return -ENODEV;
>  
> -	data->func = vexpress_config_func_get_by_dev(&pdev->dev);
> -	if (!data->func)
> +	data->reg = dev_get_regmap(&pdev->dev, NULL);
> +	if (!data->reg)
>  		return -ENODEV;
>  
>  	err = sysfs_create_group(&pdev->dev.kobj, match->data);
>  	if (err)
> -		goto error;
> +		return err;
>  
>  	data->hwmon_dev = hwmon_device_register(&pdev->dev);
>  	if (IS_ERR(data->hwmon_dev)) {
> -		err = PTR_ERR(data->hwmon_dev);
> -		goto error;
> +		sysfs_remove_group(&pdev->dev.kobj, match->data);
> +		return PTR_ERR(data->hwmon_dev);

This change is unrelated and violates coding style. If the goto disturbs you,
I would suggest to convert the driver to use devm_hwmon_device_register_with_groups() 
in a separate patch and get rid of the error handling and the entire remove() function.

Thanks,
Guenter

>  	}
>  
>  	return 0;
> -
> -error:
> -	sysfs_remove_group(&pdev->dev.kobj, match->data);
> -	vexpress_config_func_put(data->func);
> -	return err;
>  }
>  
>  static int vexpress_hwmon_remove(struct platform_device *pdev)
> @@ -207,8 +202,6 @@ static int vexpress_hwmon_remove(struct platform_device *pdev)
>  	match = of_match_device(vexpress_hwmon_of_match, &pdev->dev);
>  	sysfs_remove_group(&pdev->dev.kobj, match->data);
>  
> -	vexpress_config_func_put(data->func);
> -
>  	return 0;
>  }
>  
> -- 
> 1.8.3.2
> 
> 
> 

  reply	other threads:[~2013-12-23 17:31 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-23 16:23 [RFC 00/18] Versatile Express config rework Pawel Moll
2013-12-23 16:23 ` Pawel Moll
     [not found] ` < 1387815830-8794-7-git-send-email-pawel.moll@arm.com>
     [not found] ` < 1387815830-8794-5-git-send-email-pawel.moll@arm.com>
2013-12-23 16:23 ` [RFC 01/18] mfd: syscon: Consider platform data a regmap config name Pawel Moll
2014-01-06  9:48   ` Lee Jones
2014-01-06  9:48     ` Lee Jones
2014-01-06 10:20     ` Lee Jones
2014-01-06 10:20       ` Lee Jones
2013-12-23 16:23 ` [RFC 02/18] power/reset: vexpress: Use sched_clock as the time source Pawel Moll
2013-12-23 19:28   ` John Stultz
2013-12-23 19:28     ` John Stultz
2014-01-08 16:01     ` Pawel Moll
2014-01-08 16:01       ` Pawel Moll
2013-12-23 16:23 ` [RFC 03/18] GPIO: gpio-generic: Add label to platform data Pawel Moll
2013-12-23 17:26   ` Linus Walleij
2013-12-23 17:26     ` Linus Walleij
2014-01-08 15:57     ` Pawel Moll
2014-01-08 15:57       ` Pawel Moll
2014-01-14 10:30       ` Linus Walleij
2014-01-14 10:30         ` Linus Walleij
2014-01-14 10:44         ` Pawel Moll
2014-01-14 10:44           ` Pawel Moll
     [not found] ` <1387815830-8794-1-git-send-email-pawel.moll-5wv7dgnIgG8@public.gmane.org>
2013-12-23 16:23   ` [RFC 04/18] driver core & of: Mark of_nodes of added device as populated Pawel Moll
2013-12-23 16:23     ` Pawel Moll
2014-01-08 17:28     ` Rob Herring
2014-01-08 17:28       ` Rob Herring
2014-01-16 17:03       ` Grant Likely
2014-01-16 17:03         ` Grant Likely
2014-01-16 17:03         ` Grant Likely
2013-12-23 16:23 ` [RFC 05/18] driver core: Do not WARN when devres list is not empty at probe time Pawel Moll
2013-12-23 16:23   ` Pawel Moll
2013-12-23 16:23 ` [RFC 06/18] regmap: Formalise use of non-bus context Pawel Moll
2013-12-24 12:45   ` Mark Brown
2013-12-24 12:45     ` Mark Brown
2014-01-09 13:08     ` Pawel Moll
2014-01-09 13:08       ` Pawel Moll
2014-01-09 13:34       ` Mark Brown
2014-01-09 13:34         ` Mark Brown
2014-01-09 15:47         ` Pawel Moll
2014-01-09 15:47           ` Pawel Moll
2014-01-16 17:09       ` Grant Likely
2014-01-16 17:09         ` Grant Likely
2014-01-16 17:20         ` Pawel Moll
2014-01-16 17:20           ` Pawel Moll
2013-12-23 16:23 ` [RFC 07/18] regmap: debugfs: Always create "registers" & "access" files Pawel Moll
2013-12-24 12:19   ` Mark Brown
2013-12-24 12:19     ` Mark Brown
2014-01-08 17:31     ` Pawel Moll
2014-01-08 17:31       ` Pawel Moll
2014-01-08 18:00       ` Mark Brown
2014-01-08 18:00         ` Mark Brown
2013-12-23 16:23 ` [lm-sensors] [RFC 08/18] hwmon: vexpress: Use regmap instead of custom interface Pawel Moll
2013-12-23 16:23   ` Pawel Moll
2013-12-23 17:31   ` Guenter Roeck [this message]
2013-12-23 17:31     ` Guenter Roeck
2013-12-23 17:31     ` Guenter Roeck
2014-01-08 15:57     ` [lm-sensors] " Pawel Moll
2014-01-08 15:57       ` Pawel Moll
2014-01-08 15:57       ` Pawel Moll
2013-12-23 16:23 ` [RFC 09/18] power/reset: " Pawel Moll
2013-12-23 16:23 ` [RFC 10/18] regulator: " Pawel Moll
2013-12-24 12:24   ` Mark Brown
2013-12-24 12:24     ` Mark Brown
2014-01-08 18:25     ` Pawel Moll
2014-01-08 18:25       ` Pawel Moll
2014-01-09 13:35       ` Mark Brown
2014-01-09 13:35         ` Mark Brown
2013-12-23 16:23 ` [RFC 11/18] clocksource: Sched clock source for Versatile Express Pawel Moll
2013-12-23 16:23 ` [RFC 12/18] clk: versatile: Split config options for sp810 and vexpress_osc Pawel Moll
2013-12-23 20:05   ` Mike Turquette
2013-12-23 20:05     ` Mike Turquette
2014-01-08 16:06     ` Pawel Moll
2014-01-08 16:06       ` Pawel Moll
2013-12-23 16:23 ` [RFC 13/18] clk: versatile: Use regmap instead of custom interface Pawel Moll
2013-12-23 16:23 ` [RFC 14/18] ARM: vexpress: remove redundant vexpress_dt_cpus_num to get cpu count Pawel Moll
2013-12-23 16:23 ` [RFC 15/18] ARM: vexpress: Simplify SMP operations for DT-powered system Pawel Moll
2013-12-23 16:23 ` [RFC 16/18] bus: Versatile Express configuration bus Pawel Moll
2013-12-23 16:23 ` [RFC 17/18] misc: Versatile Express System Config interface driver Pawel Moll
2013-12-23 16:23 ` [RFC 18/18] mfd: vexpress: Split sysreg functions into MFD cells Pawel Moll
2014-01-06 10:40   ` Lee Jones
2014-01-06 10:40     ` Lee Jones
2014-01-08 16:17     ` Pawel Moll
2014-01-08 16:17       ` Pawel Moll

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131223173139.GA1678@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=arm@kernel.org \
    --cc=arnd@arndb.de \
    --cc=khali@linux-fr.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lm-sensors@lm-sensors.org \
    --cc=olof@lixom.net \
    --cc=pawel.moll@arm.com \
    --cc=sameo@linux.intel.com \
    --cc=tixy@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.