Thread (12 messages) 12 messages, 3 authors, 2012-12-12

Re: [PATCH v3 2/4] leds: leds-pwm: Preparing the driver for device tree support

From: Thierry Reding <hidden>
Date: 2012-12-11 07:13:08
Also in: lkml

On Mon, Dec 10, 2012 at 11:00:35AM +0100, Peter Ujfalusi wrote:
quoted hunk ↗ jump to hunk
In order to be able to add device tree support for leds-pwm driver we need
to rearrange the data structures used by the drivers.

Signed-off-by: Peter Ujfalusi <redacted>
---
 drivers/leds/leds-pwm.c | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 351257c..02f0c0c 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -30,6 +30,11 @@ struct led_pwm_data {
 	unsigned int		period;
 };
 
+struct led_pwm_priv {
+	int num_leds;
+	struct led_pwm_data leds[];
+};
I think you want leds[0] here. Otherwise your structure is too large by
sizeof(struct led_pwm_data *).
quoted hunk ↗ jump to hunk
+
 static void led_pwm_set(struct led_classdev *led_cdev,
 	enum led_brightness brightness)
 {
@@ -47,25 +52,29 @@ static void led_pwm_set(struct led_classdev *led_cdev,
 	}
 }
 
+static inline int sizeof_pwm_leds_priv(int num_leds)
Perhaps this should return size_t?
+{
+	return sizeof(struct led_pwm_priv) +
+		      (sizeof(struct led_pwm_data) * num_leds);
+}
+
 static int led_pwm_probe(struct platform_device *pdev)
 {
 	struct led_pwm_platform_data *pdata = pdev->dev.platform_data;
-	struct led_pwm *cur_led;
-	struct led_pwm_data *leds_data, *led_dat;
+	struct led_pwm_priv *priv;
 	int i, ret = 0;
 
 	if (!pdata)
 		return -EBUSY;
 
-	leds_data = devm_kzalloc(&pdev->dev,
-			sizeof(struct led_pwm_data) * pdata->num_leds,
-				GFP_KERNEL);
-	if (!leds_data)
+	priv = devm_kzalloc(&pdev->dev, sizeof_pwm_leds_priv(pdata->num_leds),
+			    GFP_KERNEL);
I'm not sure if sizeof_pwm_leds_priv() requires to be a separate
function. You could make it shorter by doing something like:

	size_t extra = sizeof(*led_dat) * pdata->num_leds;

	priv = devm_kzalloc(&pdev->dev, sizeof(*priv) + extra, GFP_KERNEL);

But that's really just a matter of taste, so no further objections if
you want to keep the inline function.

Thierry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help