From: Guenter Roeck <linux@roeck-us.net>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Jean Delvare <jdelvare@suse.com>,
linux-hwmon@vger.kernel.org, Nishanth Menon <nm@ti.com>,
Simon Guinot <simon.guinot@sequanux.org>,
Jamie Lentin <jm@lentin.co.uk>
Subject: Re: [6/9] hwmon: gpio-fan: Get rid of platform data struct
Date: Sun, 8 Oct 2017 07:32:26 -0700 [thread overview]
Message-ID: <20171008143226.GA5379@roeck-us.net> (raw)
In-Reply-To: <20170925230911.20824-7-linus.walleij@linaro.org>
On Tue, Sep 26, 2017 at 01:09:08AM +0200, Linus Walleij wrote:
> We are not passing the platform data struct into the driver from the
> outside, there is no point of having it around separately so instead
> of first populating the platform data struct and assigning the result
> into the same variables in the state container (struct gpio_fan_data)
> just assign the configuration from the device tree directly into the
> state container members.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Applied to hwmon-next.
Thanks,
Guenter
> ---
> drivers/hwmon/gpio-fan.c | 88 +++++++++++++++++-------------------------------
> 1 file changed, 30 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
> index 55dbdb223e02..000c8d2e0987 100644
> --- a/drivers/hwmon/gpio-fan.c
> +++ b/drivers/hwmon/gpio-fan.c
> @@ -45,18 +45,6 @@ struct gpio_fan_speed {
> int ctrl_val;
> };
>
> -struct gpio_fan_platform_data {
> - int num_ctrl;
> - unsigned int *ctrl; /* fan control GPIOs. */
> - struct gpio_fan_alarm *alarm; /* fan alarm GPIO. */
> - /*
> - * Speed conversion array: rpm from/to GPIO bit field.
> - * This array _must_ be sorted in ascending rpm order.
> - */
> - int num_speed;
> - struct gpio_fan_speed *speed;
> -};
> -
> struct gpio_fan_data {
> struct device *dev;
> struct device *hwmon_dev;
> @@ -113,14 +101,12 @@ static ssize_t fan1_alarm_show(struct device *dev,
>
> static DEVICE_ATTR_RO(fan1_alarm);
>
> -static int fan_alarm_init(struct gpio_fan_data *fan_data,
> - struct gpio_fan_alarm *alarm)
> +static int fan_alarm_init(struct gpio_fan_data *fan_data)
> {
> int err;
> int alarm_irq;
> struct device *dev = fan_data->dev;
> -
> - fan_data->alarm = alarm;
> + struct gpio_fan_alarm *alarm = fan_data->alarm;
>
> err = devm_gpio_request(dev, alarm->gpio, "GPIO fan alarm");
> if (err)
> @@ -379,12 +365,11 @@ static const struct attribute_group *gpio_fan_groups[] = {
> NULL
> };
>
> -static int fan_ctrl_init(struct gpio_fan_data *fan_data,
> - struct gpio_fan_platform_data *pdata)
> +static int fan_ctrl_init(struct gpio_fan_data *fan_data)
> {
> struct device *dev = fan_data->dev;
> - int num_ctrl = pdata->num_ctrl;
> - unsigned *ctrl = pdata->ctrl;
> + int num_ctrl = fan_data->num_ctrl;
> + unsigned int *ctrl = fan_data->ctrl;
> int i, err;
>
> for (i = 0; i < num_ctrl; i++) {
> @@ -399,10 +384,6 @@ static int fan_ctrl_init(struct gpio_fan_data *fan_data,
> return err;
> }
>
> - fan_data->num_ctrl = num_ctrl;
> - fan_data->ctrl = ctrl;
> - fan_data->num_speed = pdata->num_speed;
> - fan_data->speed = pdata->speed;
> fan_data->pwm_enable = true; /* Enable manual fan speed control. */
> fan_data->speed_index = get_fan_speed_index(fan_data);
> if (fan_data->speed_index < 0)
> @@ -456,21 +437,19 @@ static const struct thermal_cooling_device_ops gpio_fan_cool_ops = {
> /*
> * Translate OpenFirmware node properties into platform_data
> */
> -static int gpio_fan_get_of_pdata(struct device *dev,
> - struct gpio_fan_platform_data *pdata)
> +static int gpio_fan_get_of_data(struct gpio_fan_data *fan_data)
> {
> - struct device_node *node;
> struct gpio_fan_speed *speed;
> + struct device *dev = fan_data->dev;
> + struct device_node *np = dev->of_node;
> unsigned *ctrl;
> unsigned i;
> u32 u;
> struct property *prop;
> const __be32 *p;
>
> - node = dev->of_node;
> -
> /* Alarm GPIO if one exists */
> - if (of_gpio_named_count(node, "alarm-gpios") > 0) {
> + if (of_gpio_named_count(np, "alarm-gpios") > 0) {
> struct gpio_fan_alarm *alarm;
> int val;
> enum of_gpio_flags flags;
> @@ -480,39 +459,39 @@ static int gpio_fan_get_of_pdata(struct device *dev,
> if (!alarm)
> return -ENOMEM;
>
> - val = of_get_named_gpio_flags(node, "alarm-gpios", 0, &flags);
> + val = of_get_named_gpio_flags(np, "alarm-gpios", 0, &flags);
> if (val < 0)
> return val;
> alarm->gpio = val;
> alarm->active_low = flags & OF_GPIO_ACTIVE_LOW;
>
> - pdata->alarm = alarm;
> + fan_data->alarm = alarm;
> }
>
> /* Fill GPIO pin array */
> - pdata->num_ctrl = of_gpio_count(node);
> - if (pdata->num_ctrl <= 0) {
> - if (pdata->alarm)
> + fan_data->num_ctrl = of_gpio_count(np);
> + if (fan_data->num_ctrl <= 0) {
> + if (fan_data->alarm)
> return 0;
> dev_err(dev, "DT properties empty / missing");
> return -ENODEV;
> }
> - ctrl = devm_kzalloc(dev, pdata->num_ctrl * sizeof(unsigned),
> - GFP_KERNEL);
> + ctrl = devm_kzalloc(dev, fan_data->num_ctrl * sizeof(unsigned int),
> + GFP_KERNEL);
> if (!ctrl)
> return -ENOMEM;
> - for (i = 0; i < pdata->num_ctrl; i++) {
> + for (i = 0; i < fan_data->num_ctrl; i++) {
> int val;
>
> - val = of_get_gpio(node, i);
> + val = of_get_gpio(np, i);
> if (val < 0)
> return val;
> ctrl[i] = val;
> }
> - pdata->ctrl = ctrl;
> + fan_data->ctrl = ctrl;
>
> /* Get number of RPM/ctrl_val pairs in speed map */
> - prop = of_find_property(node, "gpio-fan,speed-map", &i);
> + prop = of_find_property(np, "gpio-fan,speed-map", &i);
> if (!prop) {
> dev_err(dev, "gpio-fan,speed-map DT property missing");
> return -ENODEV;
> @@ -522,7 +501,7 @@ static int gpio_fan_get_of_pdata(struct device *dev,
> dev_err(dev, "gpio-fan,speed-map contains zero/odd number of entries");
> return -ENODEV;
> }
> - pdata->num_speed = i / 2;
> + fan_data->num_speed = i / 2;
>
> /*
> * Populate speed map
> @@ -530,12 +509,12 @@ static int gpio_fan_get_of_pdata(struct device *dev,
> * this needs splitting into pairs to create gpio_fan_speed structs
> */
> speed = devm_kzalloc(dev,
> - pdata->num_speed * sizeof(struct gpio_fan_speed),
> + fan_data->num_speed * sizeof(struct gpio_fan_speed),
> GFP_KERNEL);
> if (!speed)
> return -ENOMEM;
> p = NULL;
> - for (i = 0; i < pdata->num_speed; i++) {
> + for (i = 0; i < fan_data->num_speed; i++) {
> p = of_prop_next_u32(prop, p, &u);
> if (!p)
> return -ENODEV;
> @@ -545,7 +524,7 @@ static int gpio_fan_get_of_pdata(struct device *dev,
> return -ENODEV;
> speed[i].ctrl_val = u;
> }
> - pdata->speed = speed;
> + fan_data->speed = speed;
>
> return 0;
> }
> @@ -562,20 +541,13 @@ static int gpio_fan_probe(struct platform_device *pdev)
> struct gpio_fan_data *fan_data;
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node;
> - struct gpio_fan_platform_data *pdata;
>
> fan_data = devm_kzalloc(dev, sizeof(struct gpio_fan_data),
> GFP_KERNEL);
> if (!fan_data)
> return -ENOMEM;
>
> - pdata = devm_kzalloc(dev,
> - sizeof(struct gpio_fan_platform_data),
> - GFP_KERNEL);
> - if (!pdata)
> - return -ENOMEM;
> -
> - err = gpio_fan_get_of_pdata(dev, pdata);
> + err = gpio_fan_get_of_data(fan_data);
> if (err)
> return err;
>
> @@ -584,17 +556,17 @@ static int gpio_fan_probe(struct platform_device *pdev)
> mutex_init(&fan_data->lock);
>
> /* Configure alarm GPIO if available. */
> - if (pdata->alarm) {
> - err = fan_alarm_init(fan_data, pdata->alarm);
> + if (fan_data->alarm) {
> + err = fan_alarm_init(fan_data);
> if (err)
> return err;
> }
>
> /* Configure control GPIOs if available. */
> - if (pdata->ctrl && pdata->num_ctrl > 0) {
> - if (!pdata->speed || pdata->num_speed <= 1)
> + if (fan_data->ctrl && fan_data->num_ctrl > 0) {
> + if (!fan_data->speed || fan_data->num_speed <= 1)
> return -EINVAL;
> - err = fan_ctrl_init(fan_data, pdata);
> + err = fan_ctrl_init(fan_data);
> if (err)
> return err;
> }
next prev parent reply other threads:[~2017-10-08 14:32 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-25 23:09 [PATCH 0/9] GPIO controlled fans refactoring Linus Walleij
2017-09-25 23:09 ` [PATCH 1/9] hwmon: gpio-fan: Move DT bindings to the right place Linus Walleij
2017-10-05 20:31 ` Rob Herring
2017-10-08 14:21 ` [1/9] " Guenter Roeck
2017-09-25 23:09 ` [PATCH 2/9] hwmon: gpio-fan: Use local variable pointers Linus Walleij
2017-10-08 14:26 ` [2/9] " Guenter Roeck
2017-09-25 23:09 ` [PATCH 3/9] hwmon: gpio-fan: Localize platform data Linus Walleij
2017-09-25 23:09 ` [PATCH 4/9] hwmon: gpio-fan: Send around device pointer Linus Walleij
2017-10-08 14:28 ` [4/9] " Guenter Roeck
2017-09-25 23:09 ` [PATCH 5/9] hwmon: gpio-fan: Mandate OF_GPIO and cut pdata path Linus Walleij
2017-10-08 14:29 ` [5/9] " Guenter Roeck
2017-09-25 23:09 ` [PATCH 6/9] hwmon: gpio-fan: Get rid of platform data struct Linus Walleij
2017-10-08 14:32 ` Guenter Roeck [this message]
2017-09-25 23:09 ` [PATCH 7/9] hwmon: gpio-fan: Get rid of the gpio alarm struct Linus Walleij
2017-10-08 14:33 ` [7/9] " Guenter Roeck
2017-09-25 23:09 ` [PATCH 8/9] hwmon: gpio-fan: Rename GPIO line state variables Linus Walleij
2017-10-08 14:35 ` [8/9] " Guenter Roeck
2017-09-25 23:09 ` [PATCH 9/9] hwmon: gpio-fan: Convert to use GPIO descriptors Linus Walleij
2017-10-08 14:39 ` [9/9] " Guenter Roeck
2017-10-08 23:05 ` Linus Walleij
2017-10-08 16:20 ` Guenter Roeck
2017-10-08 23:12 ` Linus Walleij
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=20171008143226.GA5379@roeck-us.net \
--to=linux@roeck-us.net \
--cc=jdelvare@suse.com \
--cc=jm@lentin.co.uk \
--cc=linus.walleij@linaro.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=nm@ti.com \
--cc=simon.guinot@sequanux.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox