Hei hei,
for leds-gpio you can use the properties 'function' and 'color' in the
devicetree node and omit 'label', the label is constructed
automatically. This is a common feature supposed to be working for all
LED drivers. However it did not yet work for the 'leds-pwm' driver.
This series removes platform_data support for the leds-pwm driver and
takes the opportunity to update the leds-pwm dt-bindings accordingly.
v5 was tested on a at91 sama5d2 based platform with LEDs connected to
GPIO and PWM.
Greets
Alex
v5:
- replaced patch 1/3 by a new patch removing platform_data support for
the leds-pwm driver
- little rewording of commit message in patch 2/3
- updated patch 3/3 based on feedback by Rob Herring
- added Marek Behún to Cc, because he also works on removing
platform_data support
- rebased series on pavel/for-next
v4:
- added led-class patch handling fwnode passing differently (patch 1/3)
- adapted leds-pwm patch to new led-class (patch 2/3)
- contacted original author of leds-pwm dt binding on license issue
(patch 3/3)
v3:
- series rebased on v5.9-rc4
- changed license of .yaml file to recommended one (patch 2/2)
- added Acked-by to both patches
v2:
- series rebased on v5.9-rc3
- added the dt-bindings update patch (2/2)
v1:
- based on v5.9-rc2
- backport on v5.4.59 tested and working
Alexander Dahl (3):
leds: pwm: Remove platform_data support
leds: pwm: Allow automatic labels for DT based devices
dt-bindings: leds: Convert pwm to yaml
.../devicetree/bindings/leds/leds-pwm.txt | 50 -----------
.../devicetree/bindings/leds/leds-pwm.yaml | 82 +++++++++++++++++++
drivers/leds/leds-pwm.c | 33 ++------
3 files changed, 89 insertions(+), 76 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt
create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml
base-commit: 03eb2ca44a95105d1482d5e7471016cf8b383f97
--
2.20.1
Since commit 141f15c66d94 ("leds: pwm: remove header") that platform
interface is not usable from outside and there seems to be no in tree
user anymore. All in-tree users of the leds-pwm driver seem to use DT
currently. Getting rid of the old platform interface will allow the
leds-pwm driver to switch over from 'devm_led_classdev_register()' to
'devm_led_classdev_register_ext()' later.
Signed-off-by: Alexander Dahl <[email protected]>
Cc: Denis Osterland-Heim <[email protected]>
Cc: Marek Behún <[email protected]>
---
Notes:
v5:
* added this patch to series (replacing another patch with a not
working, different approach)
drivers/leds/leds-pwm.c | 30 +++++-------------------------
1 file changed, 5 insertions(+), 25 deletions(-)
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index e35a97c1d828..4e9954f8f7eb 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -25,11 +25,6 @@ struct led_pwm {
unsigned int max_brightness;
};
-struct led_pwm_platform_data {
- int num_leds;
- struct led_pwm *leds;
-};
-
struct led_pwm_data {
struct led_classdev cdev;
struct pwm_device *pwm;
@@ -61,6 +56,7 @@ static int led_pwm_set(struct led_classdev *led_cdev,
return pwm_apply_state(led_dat->pwm, &led_dat->pwmstate);
}
+__attribute__((nonnull))
static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
struct led_pwm *led, struct fwnode_handle *fwnode)
{
@@ -74,10 +70,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
led_data->cdev.max_brightness = led->max_brightness;
led_data->cdev.flags = LED_CORE_SUSPENDRESUME;
- if (fwnode)
- led_data->pwm = devm_fwnode_pwm_get(dev, fwnode, NULL);
- else
- led_data->pwm = devm_pwm_get(dev, led->name);
+ led_data->pwm = devm_fwnode_pwm_get(dev, fwnode, NULL);
if (IS_ERR(led_data->pwm))
return dev_err_probe(dev, PTR_ERR(led_data->pwm),
"unable to request PWM for %s\n",
@@ -143,15 +136,11 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
static int led_pwm_probe(struct platform_device *pdev)
{
- struct led_pwm_platform_data *pdata = dev_get_platdata(&pdev->dev);
struct led_pwm_priv *priv;
- int count, i;
int ret = 0;
+ int count;
- if (pdata)
- count = pdata->num_leds;
- else
- count = device_get_child_node_count(&pdev->dev);
+ count = device_get_child_node_count(&pdev->dev);
if (!count)
return -EINVAL;
@@ -161,16 +150,7 @@ static int led_pwm_probe(struct platform_device *pdev)
if (!priv)
return -ENOMEM;
- if (pdata) {
- for (i = 0; i < count; i++) {
- ret = led_pwm_add(&pdev->dev, priv, &pdata->leds[i],
- NULL);
- if (ret)
- break;
- }
- } else {
- ret = led_pwm_create_fwnode(&pdev->dev, priv);
- }
+ ret = led_pwm_create_fwnode(&pdev->dev, priv);
if (ret)
return ret;
--
2.20.1
Hi!
> Since commit 141f15c66d94 ("leds: pwm: remove header") that platform
> interface is not usable from outside and there seems to be no in tree
> user anymore. All in-tree users of the leds-pwm driver seem to use DT
> currently. Getting rid of the old platform interface will allow the
> leds-pwm driver to switch over from 'devm_led_classdev_register()' to
> 'devm_led_classdev_register_ext()' later.
> @@ -61,6 +56,7 @@ static int led_pwm_set(struct led_classdev *led_cdev,
> return pwm_apply_state(led_dat->pwm, &led_dat->pwmstate);
> }
>
> +__attribute__((nonnull))
> static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
> struct led_pwm *led, struct fwnode_handle *fwnode)
> {
This normally goes elsewhere -- right? I'd expect:
static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
struct led_pwm *led, struct fwnode_handle *fwnode)
__attribute__((nonnull))
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Besides Pavel's note about the __attribute__((nonnull)) position
Reviewed-by: Marek Behún <[email protected]>
Hello Pavel,
Am Samstag, 19. September 2020, 11:44:18 CEST schrieb Pavel Machek:
> > Since commit 141f15c66d94 ("leds: pwm: remove header") that platform
> > interface is not usable from outside and there seems to be no in tree
> > user anymore. All in-tree users of the leds-pwm driver seem to use DT
> > currently. Getting rid of the old platform interface will allow the
> > leds-pwm driver to switch over from 'devm_led_classdev_register()' to
> > 'devm_led_classdev_register_ext()' later.
> >
> > @@ -61,6 +56,7 @@ static int led_pwm_set(struct led_classdev *led_cdev,
> >
> > return pwm_apply_state(led_dat->pwm, &led_dat->pwmstate);
> >
> > }
> >
> > +__attribute__((nonnull))
> >
> > static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
> >
> > struct led_pwm *led, struct fwnode_handle *fwnode)
> >
> > {
>
> This normally goes elsewhere -- right? I'd expect:
>
>
> static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
> struct led_pwm *led, struct fwnode_handle *fwnode)
> __attribute__((nonnull))
I found both variants in kernel code. I can live with both variants and have
no strong preference.
My initial intention to add it was to get a compiler warning in case someone
does not pass a fwnode here, e.g. when using that old platform_data approach
(which is supposed to be removed with this patch). You might call it a self
check on my own changes. I can also drop that attribute if you don't want
that kind of stuff in linux-leds.
Greets
Alex
Hi!
> > > +__attribute__((nonnull))
> > >
> > > static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
> > >
> > > struct led_pwm *led, struct fwnode_handle *fwnode)
> > >
> > > {
> >
> > This normally goes elsewhere -- right? I'd expect:
> >
> >
> > static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
> > struct led_pwm *led, struct fwnode_handle *fwnode)
> > __attribute__((nonnull))
>
> I found both variants in kernel code. I can live with both variants and have
> no strong preference.
>
> My initial intention to add it was to get a compiler warning in case someone
> does not pass a fwnode here, e.g. when using that old platform_data approach
> (which is supposed to be removed with this patch). You might call it a self
> check on my own changes. I can also drop that attribute if you don't want
> that kind of stuff in linux-leds.
I'm okay with it at the second place :-).
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hello Pavel,
On Wed, Sep 30, 2020 at 07:24:41PM +0200, Pavel Machek wrote:
> Hi!
>
> > > > +__attribute__((nonnull))
> > > >
> > > > static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
> > > >
> > > > struct led_pwm *led, struct fwnode_handle *fwnode)
> > > >
> > > > {
> > >
> > > This normally goes elsewhere -- right? I'd expect:
> > >
> > >
> > > static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
> > > struct led_pwm *led, struct fwnode_handle *fwnode)
> > > __attribute__((nonnull))
> >
> > I found both variants in kernel code. I can live with both variants and have
> > no strong preference.
> >
> > My initial intention to add it was to get a compiler warning in case someone
> > does not pass a fwnode here, e.g. when using that old platform_data approach
> > (which is supposed to be removed with this patch). You might call it a self
> > check on my own changes. I can also drop that attribute if you don't want
> > that kind of stuff in linux-leds.
>
> I'm okay with it at the second place :-).
Should have tried this before, but I actually did now. O:-)
If I move the attribute behind, I get this on a W=1 build:
CC drivers/leds/leds-pwm.o
/home/alex/src/linux/leds/drivers/leds/leds-pwm.c:58:1: error: attributes should be specified before the declarator in a function definition
static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
^~~~~~
Because it won't build then, I'll keep it where it is. Meanwhile I
worked on all the DT remarks by Rob and I will send v6 soon.
Greets
Alex
--
/"\ ASCII RIBBON | ?With the first link, the chain is forged. The first
\ / CAMPAIGN | speech censured, the first thought forbidden, the
X AGAINST | first freedom denied, chains us all irrevocably.?
/ \ HTML MAIL | (Jean-Luc Picard, quoting Judge Aaron Satie)