Paul,
thanks for your feedback. While developing this I looked at other drivers in
the tree and many of your comments would apply to them, too?
On Wed, Mar 11, 2015 at 2:31 AM, Paul Bolle <pebolle@tiscali
>> +config POWER_RESET_SYSCON_POWEROFF
>> + bool "Generic SYSCON regmap poweroff driver"
>
> This adds a bool symbol.
Is that an issue or is it just to substantiate the rest of your comments below?
>> +obj-$(CONFIG_POWER_RESET_SYSCON_POWEROFF) += syscon-poweroff.o
>
> So this objectfile can never be part of a module.
Same comment would apply for power/reset/qnap-poweroff.c, right?
>> +#include <linux/module.h>
>
> Is that include needed?
Wouldn't that be needed if MODULE_DEVICE_TABLE etc, is used?
>> +MODULE_DEVICE_TABLE(of, syscon_poweroff_of_match);
>
> This will be preprocessed away.
In my experiments it didn't work without this, are you sure it's not needed?
>
>> +module_platform_driver(syscon_poweroff_driver);
>
> I think the built-in equivalent of this would be adding a wrapper that
> only does
> platform_driver_register(&syscon_poweroff_driver);
I couldn't find other examples in the tree doing this. Any pointers?
>> +MODULE_LICENSE("GPL v2");
>
> You probably meant
> MODULE_LICENSE("GPL");
Sorry about that. Will fix.
>
>> +MODULE_AUTHOR("Moritz Fischer <moritz.fischer@xxxxxxxxx>");
>> +MODULE_DESCRIPTION("Generic SYSCON poweroff driver");
>> +MODULE_ALIAS("platform:syscon-poweroff");
>
> But these four macros will all be effectively preprocessed away, anyhow.
I'll remove them.
Thanks,
Moritz
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
