Hi Urja,
On Thu, Mar 29, 2012, Urja Rannikko wrote:
> I'm working on bluetooth on the pandora handheld console, and this
> functionality was missing.
> This is my first mail on this list, and I'm not 100% sure on all the
> customs here, but I think this is sufficient:
If possible please use git send-email instead of sending patches as
attachments.
> Signed-off-by: Urja Rannikko <urjaman@xxxxxxxxx>
This convention is only used for kernel-side patches.
> +static int poll_wait_reply(int fd, int ms) {
The { goes on its own line.
> + struct pollfd pfd;
> + pfd.fd = fd;
Empty line after the initial variable declarations.
> + pfd.events = POLLIN;
> + pfd.revents = 0;
> + return poll(&pfd, 1, ms);
> +}
Empty line before return. ...
> +static int texas_change_speed(int fd, uint32_t speed) {
Same thing here with the {
> + int i;
> + texas_speed_change_cmd_t cmd;
> + fprintf(stdout,"Sending speed change command..\n");
And again empty line after the variable declarations. Also missing space
after the comma (I'd have requested you to use printf instead of fprintf
since you're using stdout but I noticed that the rest of the file does
this too; feel free to send a separate cleanup patch for that if you
wish though).
> + for (i=0;i<3;i++) {
This should be:
for (i = 0; i < 3; i++) {
> + int n = write(fd,&cmd,sizeof(cmd));
The return value of write is ssize_t and not int (not that it makes much
difference but it doesn't hurt to be consistent here). Also, missing
space after each comma.
> + if (n < 0) {
> + perror("Failed to write speed change command");
> + return -1;
> + }
> + if (n < sizeof(cmd)) {
Add an empty line before the second if
> + fprintf(stderr, "Wanted to write %lu bytes, could only write %d. Stop\n", sizeof(cmd), n);
Keep your lines under 80 characters please.
> + return -1;
> + }
> + if (poll_wait_reply(fd,100)!=1) continue;
Empty line before the if and put the continue on its own line. Also,
missing space before and after !=
> + return read_command_complete(fd,cmd.hci_hdr.opcode,cmd.hci_hdr.plen);
Missing spaces after each comma.
> + }
> + fprintf(stderr,"Speed change command timeout.\n");
Add an empty line before the fprintf. Also, missing space after the
comma.
> + if (texas_change_speed(fd, speed)) return -1;
Put the return statement on its own line.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
