Re: AT86RF212 based USB-Stick (HUL)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

On 08/17/2017 01:14 PM, Josef Filzmaier wrote:
Hello!

First i want to thank you for taking your time to look at my code and all the
useful comments. I will go through them step by step.

No problem. I'm very happy to see someone working on adding support for a new board to the firmware and driver. Glad to review and test it. Sometimes it just takes a few days to come to it.

I have some general questions:

My thought was to squash all the commits into a single one once the code is in
good enough shape. Do you prefer to keep the commit history?

In general I'm not so much interested in the commit history. The interest is more to have big changes split into a useful, always buildable and easy to review series of commits.

We also want to have new features and bug fixes as well as cleanups in separate commits. This all could sound a bit overloading when doing a first series of patches (also the git send mail thing I comment on below). I try to not be to pedantic but some things I really have.

If I look over your changes a potential series would be:
0001: Update atusb header with hw board define to sync up with firmware
0002: Basic refactor of atusb to enable easy support for a new board (hw_name and chip_data changes for example) [This would need to be done for rzusb and atusb only and need to build and work without the last patch]
0003: Finally add support for hulusb with all remaining needed changes.

If it turns out that having 2 & 3 as split commits it to complicated and the merged patch is not really that big we can also go for a merged version.

If you are going to have some generic bug fixes in there they would need to be separated out into signle commits and put in the beginning of the series though.

I did debug the firmware over UART. I thought it might be a useful option to
other developers (using the DEBUG flag) working on this firmware, but i can of
course also remove my debug code completely if it is not needed?

Good question. I can see the debugging functionality being useful during bringup, just not during the normal lifetime. I'm happy to get the uart.c and additional changes as a patch to the firmware as long as it is off by default and we do not have to many debug statements sprinkled all over the code. :)

As for review I can go through your changes on the github links and do
some initial review there but the final patches and review to get
applied to the tree need to come as git patches to this mailing list.

I have seen the git send-email tutorial on the linux-wpan homepage (To change
the homepage). Never used git send-email before but i guess this is how it
should be done with the code as well, once the code is in good shape.

Yes, git send mail to this list is the final step to get review and the patches applied in the end. It can be a bit steep for first time users. If you have trouble asking here is fine but there should also be some good tips in the kernel Documentation folder or on the internet.

What I normally do is something like this:

git format-patch --subject-prefix="PATCH bluetooth-next" -s -3 --cover-letter

git send-email --to=linux-wpan@xxxxxxxxxxxxxxx --smtp-server=XXX --smtp-user="XXX --smtp-server-port=587 --smtp-encryption=tls 000*

It would take your latest three commits, adjust the subject prefix, make sure they are signed off and finally send them to the list through your smtp server.

I also send you the address from my private mail (datenfreihafen.org).
Did this one end up in your SPAM folder? That would be odd.

It actually did. No clue why :)

Odd, I did never really had trouble with my private address before. Thanks for letting me know.

regards
Stefan Schmidt
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux