| Message ID | 1997643.hzLDnYTZoH@vostro.rjw.lan (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Tuesday, October 07, 2014 02:18:45 AM Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Make use of device property API in this driver so that both OF and ACPI > based system can use the same driver. > > This change contains material from Max Eliaser and Mika Westerberg. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Bryan, can you please tell me if you are OK with this patch? You said you were for one of the previous versions if I remember correctly, but it's changed quite a bit since then, so can you please check this new version too? > --- > drivers/leds/leds-gpio.c | 67 +++++++++++++++++++++-------------------------- > 1 file changed, 30 insertions(+), 37 deletions(-) > > Index: linux-pm/drivers/leds/leds-gpio.c > =================================================================== > --- linux-pm.orig/drivers/leds/leds-gpio.c > +++ linux-pm/drivers/leds/leds-gpio.c > @@ -15,13 +15,11 @@ > #include <linux/gpio.h> > #include <linux/gpio/consumer.h> > #include <linux/leds.h> > -#include <linux/of.h> > -#include <linux/of_platform.h> > -#include <linux/of_gpio.h> > #include <linux/slab.h> > #include <linux/workqueue.h> > #include <linux/module.h> > #include <linux/err.h> > +#include <linux/property.h> > > struct gpio_led_data { > struct led_classdev cdev; > @@ -171,40 +169,41 @@ static inline int sizeof_gpio_leds_priv( > (sizeof(struct gpio_led_data) * num_leds); > } > > -/* Code to create from OpenFirmware platform devices */ > -#ifdef CONFIG_OF_GPIO > -static struct gpio_leds_priv *gpio_leds_create_of(struct platform_device *pdev) > +static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev) > { > - struct device_node *np = pdev->dev.of_node, *child; > + struct device *dev = &pdev->dev; > struct gpio_leds_priv *priv; > int count, ret; > + void *child; > > - /* count LEDs in this device, so we know how much to allocate */ > - count = of_get_available_child_count(np); > + count = device_get_child_node_count(dev); > if (!count) > return ERR_PTR(-ENODEV); > > - for_each_available_child_of_node(np, child) > - if (of_get_gpio(child, 0) == -EPROBE_DEFER) > - return ERR_PTR(-EPROBE_DEFER); > - > - priv = devm_kzalloc(&pdev->dev, sizeof_gpio_leds_priv(count), > - GFP_KERNEL); > + priv = devm_kzalloc(dev, sizeof_gpio_leds_priv(count), GFP_KERNEL); > if (!priv) > return ERR_PTR(-ENOMEM); > > - for_each_available_child_of_node(np, child) { > + device_for_each_child_node(dev, child) { > struct gpio_led led = {}; > - enum of_gpio_flags flags; > - const char *state; > + const char *state = NULL; > + > + led.gpiod = devm_get_named_gpiod_from_child(dev, child, > + "gpios", 0); > + if (IS_ERR(led.gpiod)) { > + device_put_child_node(dev, child); > + goto err; > + } > > - led.gpio = of_get_gpio_flags(child, 0, &flags); > - led.active_low = flags & OF_GPIO_ACTIVE_LOW; > - led.name = of_get_property(child, "label", NULL) ? : child->name; > - led.default_trigger = > - of_get_property(child, "linux,default-trigger", NULL); > - state = of_get_property(child, "default-state", NULL); > - if (state) { > + device_child_property_read_string(dev, child, > + "label", &led.name); > + device_child_property_read_string(dev, child, > + "linux,default-trigger", > + &led.default_trigger); > + > + if (!device_child_property_read_string(dev, child, > + "linux,default_state", > + &state)) { > if (!strcmp(state, "keep")) > led.default_state = LEDS_GPIO_DEFSTATE_KEEP; > else if (!strcmp(state, "on")) > @@ -213,13 +212,14 @@ static struct gpio_leds_priv *gpio_leds_ > led.default_state = LEDS_GPIO_DEFSTATE_OFF; > } > > - if (of_get_property(child, "retain-state-suspended", NULL)) > + if (!device_get_child_property(dev, child, > + "retain-state-suspended", NULL)) > led.retain_state_suspended = 1; > > ret = create_gpio_led(&led, &priv->leds[priv->num_leds++], > - &pdev->dev, NULL); > + dev, NULL); > if (ret < 0) { > - of_node_put(child); > + device_put_child_node(dev, child); > goto err; > } > } > @@ -238,13 +238,6 @@ static const struct of_device_id of_gpio > }; > > MODULE_DEVICE_TABLE(of, of_gpio_leds_match); > -#else /* CONFIG_OF_GPIO */ > -static struct gpio_leds_priv *gpio_leds_create_of(struct platform_device *pdev) > -{ > - return ERR_PTR(-ENODEV); > -} > -#endif /* CONFIG_OF_GPIO */ > - > > static int gpio_led_probe(struct platform_device *pdev) > { > @@ -273,7 +266,7 @@ static int gpio_led_probe(struct platfor > } > } > } else { > - priv = gpio_leds_create_of(pdev); > + priv = gpio_leds_create(pdev); > if (IS_ERR(priv)) > return PTR_ERR(priv); > } > @@ -300,7 +293,7 @@ static struct platform_driver gpio_led_d > .driver = { > .name = "leds-gpio", > .owner = THIS_MODULE, > - .of_match_table = of_match_ptr(of_gpio_leds_match), > + .of_match_table = of_gpio_leds_match, > }, > }; > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 8, 2014 at 7:04 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Tuesday, October 07, 2014 02:18:45 AM Rafael J. Wysocki wrote: >> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> >> Make use of device property API in this driver so that both OF and ACPI >> based system can use the same driver. >> >> This change contains material from Max Eliaser and Mika Westerberg. >> >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> >> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Bryan, can you please tell me if you are OK with this patch? > > You said you were for one of the previous versions if I remember correctly, > but it's changed quite a bit since then, so can you please check this new > version too? > Sure, the new one is even better than before. Please go ahead with my Ack Acked-by: Bryan Wu <cooloney@gmail.com> Thanks, -Bryan >> --- >> drivers/leds/leds-gpio.c | 67 +++++++++++++++++++++-------------------------- >> 1 file changed, 30 insertions(+), 37 deletions(-) >> >> Index: linux-pm/drivers/leds/leds-gpio.c >> =================================================================== >> --- linux-pm.orig/drivers/leds/leds-gpio.c >> +++ linux-pm/drivers/leds/leds-gpio.c >> @@ -15,13 +15,11 @@ >> #include <linux/gpio.h> >> #include <linux/gpio/consumer.h> >> #include <linux/leds.h> >> -#include <linux/of.h> >> -#include <linux/of_platform.h> >> -#include <linux/of_gpio.h> >> #include <linux/slab.h> >> #include <linux/workqueue.h> >> #include <linux/module.h> >> #include <linux/err.h> >> +#include <linux/property.h> >> >> struct gpio_led_data { >> struct led_classdev cdev; >> @@ -171,40 +169,41 @@ static inline int sizeof_gpio_leds_priv( >> (sizeof(struct gpio_led_data) * num_leds); >> } >> >> -/* Code to create from OpenFirmware platform devices */ >> -#ifdef CONFIG_OF_GPIO >> -static struct gpio_leds_priv *gpio_leds_create_of(struct platform_device *pdev) >> +static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev) >> { >> - struct device_node *np = pdev->dev.of_node, *child; >> + struct device *dev = &pdev->dev; >> struct gpio_leds_priv *priv; >> int count, ret; >> + void *child; >> >> - /* count LEDs in this device, so we know how much to allocate */ >> - count = of_get_available_child_count(np); >> + count = device_get_child_node_count(dev); >> if (!count) >> return ERR_PTR(-ENODEV); >> >> - for_each_available_child_of_node(np, child) >> - if (of_get_gpio(child, 0) == -EPROBE_DEFER) >> - return ERR_PTR(-EPROBE_DEFER); >> - >> - priv = devm_kzalloc(&pdev->dev, sizeof_gpio_leds_priv(count), >> - GFP_KERNEL); >> + priv = devm_kzalloc(dev, sizeof_gpio_leds_priv(count), GFP_KERNEL); >> if (!priv) >> return ERR_PTR(-ENOMEM); >> >> - for_each_available_child_of_node(np, child) { >> + device_for_each_child_node(dev, child) { >> struct gpio_led led = {}; >> - enum of_gpio_flags flags; >> - const char *state; >> + const char *state = NULL; >> + >> + led.gpiod = devm_get_named_gpiod_from_child(dev, child, >> + "gpios", 0); >> + if (IS_ERR(led.gpiod)) { >> + device_put_child_node(dev, child); >> + goto err; >> + } >> >> - led.gpio = of_get_gpio_flags(child, 0, &flags); >> - led.active_low = flags & OF_GPIO_ACTIVE_LOW; >> - led.name = of_get_property(child, "label", NULL) ? : child->name; >> - led.default_trigger = >> - of_get_property(child, "linux,default-trigger", NULL); >> - state = of_get_property(child, "default-state", NULL); >> - if (state) { >> + device_child_property_read_string(dev, child, >> + "label", &led.name); >> + device_child_property_read_string(dev, child, >> + "linux,default-trigger", >> + &led.default_trigger); >> + >> + if (!device_child_property_read_string(dev, child, >> + "linux,default_state", >> + &state)) { >> if (!strcmp(state, "keep")) >> led.default_state = LEDS_GPIO_DEFSTATE_KEEP; >> else if (!strcmp(state, "on")) >> @@ -213,13 +212,14 @@ static struct gpio_leds_priv *gpio_leds_ >> led.default_state = LEDS_GPIO_DEFSTATE_OFF; >> } >> >> - if (of_get_property(child, "retain-state-suspended", NULL)) >> + if (!device_get_child_property(dev, child, >> + "retain-state-suspended", NULL)) >> led.retain_state_suspended = 1; >> >> ret = create_gpio_led(&led, &priv->leds[priv->num_leds++], >> - &pdev->dev, NULL); >> + dev, NULL); >> if (ret < 0) { >> - of_node_put(child); >> + device_put_child_node(dev, child); >> goto err; >> } >> } >> @@ -238,13 +238,6 @@ static const struct of_device_id of_gpio >> }; >> >> MODULE_DEVICE_TABLE(of, of_gpio_leds_match); >> -#else /* CONFIG_OF_GPIO */ >> -static struct gpio_leds_priv *gpio_leds_create_of(struct platform_device *pdev) >> -{ >> - return ERR_PTR(-ENODEV); >> -} >> -#endif /* CONFIG_OF_GPIO */ >> - >> >> static int gpio_led_probe(struct platform_device *pdev) >> { >> @@ -273,7 +266,7 @@ static int gpio_led_probe(struct platfor >> } >> } >> } else { >> - priv = gpio_leds_create_of(pdev); >> + priv = gpio_leds_create(pdev); >> if (IS_ERR(priv)) >> return PTR_ERR(priv); >> } >> @@ -300,7 +293,7 @@ static struct platform_driver gpio_led_d >> .driver = { >> .name = "leds-gpio", >> .owner = THIS_MODULE, >> - .of_match_table = of_match_ptr(of_gpio_leds_match), >> + .of_match_table = of_gpio_leds_match, >> }, >> }; >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > I speak only for myself. > Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, October 08, 2014 10:47:16 AM Bryan Wu wrote: > On Wed, Oct 8, 2014 at 7:04 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Tuesday, October 07, 2014 02:18:45 AM Rafael J. Wysocki wrote: > >> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > >> > >> Make use of device property API in this driver so that both OF and ACPI > >> based system can use the same driver. > >> > >> This change contains material from Max Eliaser and Mika Westerberg. > >> > >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > >> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Bryan, can you please tell me if you are OK with this patch? > > > > You said you were for one of the previous versions if I remember correctly, > > but it's changed quite a bit since then, so can you please check this new > > version too? > > > > Sure, the new one is even better than before. Please go ahead with my Ack > Acked-by: Bryan Wu <cooloney@gmail.com> Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: linux-pm/drivers/leds/leds-gpio.c =================================================================== --- linux-pm.orig/drivers/leds/leds-gpio.c +++ linux-pm/drivers/leds/leds-gpio.c @@ -15,13 +15,11 @@ #include <linux/gpio.h> #include <linux/gpio/consumer.h> #include <linux/leds.h> -#include <linux/of.h> -#include <linux/of_platform.h> -#include <linux/of_gpio.h> #include <linux/slab.h> #include <linux/workqueue.h> #include <linux/module.h> #include <linux/err.h> +#include <linux/property.h> struct gpio_led_data { struct led_classdev cdev; @@ -171,40 +169,41 @@ static inline int sizeof_gpio_leds_priv( (sizeof(struct gpio_led_data) * num_leds); } -/* Code to create from OpenFirmware platform devices */ -#ifdef CONFIG_OF_GPIO -static struct gpio_leds_priv *gpio_leds_create_of(struct platform_device *pdev) +static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev) { - struct device_node *np = pdev->dev.of_node, *child; + struct device *dev = &pdev->dev; struct gpio_leds_priv *priv; int count, ret; + void *child; - /* count LEDs in this device, so we know how much to allocate */ - count = of_get_available_child_count(np); + count = device_get_child_node_count(dev); if (!count) return ERR_PTR(-ENODEV); - for_each_available_child_of_node(np, child) - if (of_get_gpio(child, 0) == -EPROBE_DEFER) - return ERR_PTR(-EPROBE_DEFER); - - priv = devm_kzalloc(&pdev->dev, sizeof_gpio_leds_priv(count), - GFP_KERNEL); + priv = devm_kzalloc(dev, sizeof_gpio_leds_priv(count), GFP_KERNEL); if (!priv) return ERR_PTR(-ENOMEM); - for_each_available_child_of_node(np, child) { + device_for_each_child_node(dev, child) { struct gpio_led led = {}; - enum of_gpio_flags flags; - const char *state; + const char *state = NULL; + + led.gpiod = devm_get_named_gpiod_from_child(dev, child, + "gpios", 0); + if (IS_ERR(led.gpiod)) { + device_put_child_node(dev, child); + goto err; + } - led.gpio = of_get_gpio_flags(child, 0, &flags); - led.active_low = flags & OF_GPIO_ACTIVE_LOW; - led.name = of_get_property(child, "label", NULL) ? : child->name; - led.default_trigger = - of_get_property(child, "linux,default-trigger", NULL); - state = of_get_property(child, "default-state", NULL); - if (state) { + device_child_property_read_string(dev, child, + "label", &led.name); + device_child_property_read_string(dev, child, + "linux,default-trigger", + &led.default_trigger); + + if (!device_child_property_read_string(dev, child, + "linux,default_state", + &state)) { if (!strcmp(state, "keep")) led.default_state = LEDS_GPIO_DEFSTATE_KEEP; else if (!strcmp(state, "on")) @@ -213,13 +212,14 @@ static struct gpio_leds_priv *gpio_leds_ led.default_state = LEDS_GPIO_DEFSTATE_OFF; } - if (of_get_property(child, "retain-state-suspended", NULL)) + if (!device_get_child_property(dev, child, + "retain-state-suspended", NULL)) led.retain_state_suspended = 1; ret = create_gpio_led(&led, &priv->leds[priv->num_leds++], - &pdev->dev, NULL); + dev, NULL); if (ret < 0) { - of_node_put(child); + device_put_child_node(dev, child); goto err; } } @@ -238,13 +238,6 @@ static const struct of_device_id of_gpio }; MODULE_DEVICE_TABLE(of, of_gpio_leds_match); -#else /* CONFIG_OF_GPIO */ -static struct gpio_leds_priv *gpio_leds_create_of(struct platform_device *pdev) -{ - return ERR_PTR(-ENODEV); -} -#endif /* CONFIG_OF_GPIO */ - static int gpio_led_probe(struct platform_device *pdev) { @@ -273,7 +266,7 @@ static int gpio_led_probe(struct platfor } } } else { - priv = gpio_leds_create_of(pdev); + priv = gpio_leds_create(pdev); if (IS_ERR(priv)) return PTR_ERR(priv); } @@ -300,7 +293,7 @@ static struct platform_driver gpio_led_d .driver = { .name = "leds-gpio", .owner = THIS_MODULE, - .of_match_table = of_match_ptr(of_gpio_leds_match), + .of_match_table = of_gpio_leds_match, }, };