Thanks for the feedback. The difficult part here is that these are not my drivers, but are both GPL - and I feel should be included upstream. While I'm not a C coder, I may be able to do minor changes, but this was my best effort of putting something together that does actually work - but may benefit from the expertise of the associated lists to improve. On 30/10/16 23:06, Jacek Anaszewski wrote: > Hi Steven, > > Thanks for the patches. Few initial notes: > > - generally it is preferred to submit patches with "git send-email", > which simplifies review, as the remarks can be added directly in the > code, in a reply to the patch, > - if you want do add some overall description of the patch set then you > can add --cover-letter switch which will generate template for this > purpose. > - please also use scripts/checkpatch.pl before submission > - make sure that the patches are based on the master branch of > git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git, > - always add on CC the maintainers of the affected kernel subsystems - > this information can be found in the MAINTAINERS file > - in case of LED drivers cc also linux-kernel@xxxxxxxxxxxxxxx, > - subscribe to the mailing list related to the subsystem the patches > are dedicated to, and skim through some recent patches and reviews - > it can allow for eliminating some common problems from your driver > and reducing the number of review iterations. > > Regarding the patches - gpio-nct5104d.c seems to be targeted for > GPIO subsystem - you should add Linus Walleij and > linux-gpio@xxxxxxxxxxxxxxx on cc. Correct, the nct5104 is required for the leds-apu2 module. They both co-exist to address the LEDs on the PC Engines APU2 board. There are requirements for the leds_gpio module on leds-apu2 as well - but I couldn't figure out how to reference this. > For leds-apu2.c - you shouldn't register gpio driver in the LED > subsystem but add the relevant dependency in the Kconfig. Nonetheless > I can't find gpio-apu2.c driver in the mainline kernel. > > You should also use devm_led_classdev_register() for registering LED > class device. Please follow the design of the other LED class drivers > using platform_device_register_simple() for probing. You've gone above my head here :P As mentioned, I was hoping it would be possible to get someone to pick it up and run with it based on my initial throwing of things together. If that's not possible, I can keep hunting for someone to clean things up a little bit and try again at a later stage? > Best regards, > Jacek Anaszewski > > On 10/29/2016 06:38 PM, Steven Haigh wrote: >> Patches attached. >> >> Changes: >> * Updated leds_apu driver. Included an older version in my previous >> email. >> * Added required nct5104d GPIO driver support. >> >> These are all against 4.4.28 right now - but should probably apply >> cleanly to any future builds with minimal (or nil) changes. >> >> Please include me in the reply as I'm not part of this list. >> > -- Steven Haigh Email: netwiz@xxxxxxxxx Web: https://www.crc.id.au Phone: (03) 9001 6090 - 0412 935 897
Attachment:
signature.asc
Description: OpenPGP digital signature
