Cc: Grant Likely <grant.likely@...>, Richard Purdie <rpurdie@...>, Stephen Rothwell <sfr@...>, Kumar Gala <galak@...>, <linux-kernel@...>, <linuxppc-dev@...>
Your new driver was 194 lines, not counting docs or Kconfig. My patch
added about 104 lines to the existing leds-gpio driver. So yes, about half
the code.
I guess, in terms of compiled size, the combined platform + of_platform
driver is bigger than the of_platform only driver. Though it's much
smaller than having both the platform only and of_platform only drivers.
In terms of source code, there's less with the combined driver.
I don't see why the combined leds-gpio driver can't have an ifdef for the
platform part. All the platform driver specific code is already in one
contiguous block. In fact, I just did it and it works fine. My LEDs from
the dts were created and the LED I have as a platform device wasn't, as
expected. Here's the patch, pretty simple:
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -108,2 +108,3 @@ static int create_gpio_led(struct gpio_led *cur_led,
+#ifdef CONFIG_LEDS_GPIO_PLATFORM
static int gpio_led_probe(struct platform_device *pdev)
@@ -222,2 +223,3 @@ module_init(gpio_led_init);
module_exit(gpio_led_exit);
+#endif /* CONFIG_LEDS_GPIO_PLATFORM */
It creates a "thing" from a template passed a pointer to a struct. This is
very common, there must be hundreds of functions in the kernel that work
this way. The difference is instead of allocating and returning the
created thing, you pass it a blank pre-allocated thing to fill in and
register. I know there is other code that works this way too. It's
usually used, like it is here, so you can allocate a bunch of things at
once and then register them one at a time.
The struct gpio_led is just used to pass the stats to the led creation
function. It doesn't stick around. You could use local variables for the
gpio and name and pass them to the create led function. Bundling them into
a struct is an tiny change that lets more code be shared.
Obviously the OF binding code should be conditional, selectable from
kconfig if the platform has OF support. It's all in one contiguous block
and that shows where the ifdef would go. I didn't think it was necessary
to include the obvious kconfig patch.
--
