xaptum / xaptum-buildroot

External Buildroot tree for Xaptum hardware
GNU General Public License v2.0
0 stars 0 forks source link

Add enftun to boardconfig #7

Closed peterhaijen closed 6 years ago

peterhaijen commented 6 years ago

Hi @drbild this are the changes I've made to add enftun to buildroot:

peterhaijen commented 6 years ago

I am working on your kernel config and board config request. This will take some time to complete.

peterhaijen commented 6 years ago

Hi @drbild this should do it, kernel changes split into several commits, and made sama5d2 config more generic.

drbild commented 6 years ago
  • You asked to rename the samadd2 config, replacing xaprc001 by xaprc_dev, but there is a directory called sama5d2_xplained_xaprc001 - do you want that renamed as well?

Exactly. That is intended to be a config for using the Xplained board to do development, without infecting the production config with unnecessary options. Feel free to add to that as needed.

  • You asked to split up the kernel changes in different commits. If you want me to commit a kernel config without (network) support for my board (sama5d2 xplained), then you're asking me to commit a kernel config which I can't test on my board, so that will be an untested commit.

Good catch. I think if the commit adding network support comes first in the series, its ok though. Because then the subsequent commits will have network support and be testable.

  • These kernel commits will still depend on each other, as adding one kernel option may result in other options being commented out as they are now implied (just like enabling support for enftun removes xtt from the buildroot config as enftun requires xtt), or a list un 'not set' options appearing.

Absolutely. But the "semantic" changes will be independent.

drbild commented 6 years ago

Looks good, thanks.

I'm confused by the commit "changes from doing savedefconfig". Shouldn't the prior commits adding various features also have come from savedefconfig. I envision the following workflow:

1) make menuconfig to set the new config 2) make to build, test, etc. 3) make savedefconfig to generate the new config 4) git diff to confirm that the changes to the defconfig look reasonable 5) git commit to create the commit.

So every commited defconfig is first output by savedefconfig.

There's no need to re-do this series. I'll merge as is. I'm just curious, since in this series I can't tell why the options added in the savedefconfig commit were added.

peterhaijen commented 6 years ago

I'm confused by the commit "changes from doing savedefconfig". Shouldn't the prior commits adding various features also have come from savedefconfig. I envision the following workflow:

Yes that's what I expected as well. Unfortunately, that was apparently not the case.

One explanation is that the kernel config was manually edited at some point. Another I can think of is that a switch to a different kernel results in a slighly changed savedefconfig.

I see, thanks. That makes sense. I must have manually edited at some point...

peterhaijen commented 6 years ago
  • You asked to rename the samadd2 config, replacing xaprc001 by xaprc_dev, but there is a directory called sama5d2_xplained_xaprc001 - do you want that renamed as well?

Exactly. That is intended to be a config for using the Xplained board to do development, without infecting the production config with unnecessary options. Feel free to add to that as needed.

My question was more about renaming the directory here, rather than modifying it's contents.

  • You asked to split up the kernel changes in different commits. If you want me to commit a kernel config without (network) support for my board (sama5d2 xplained), then you're asking me to commit a kernel config which I can't test on my board, so that will be an untested commit.

Good catch. I think if the commit adding network support comes first in the series, its ok though. Because then the subsequent commits will have network support and be testable.

I think we have a misunderstanding here. I was referring to the config files NOT specifically for the sama5d2 board - the config files for the actual production boards. You wanted to keep the config for these boards minimal. I was just saying that, since I currently do not have access to such a production board, I cannot test the kernel I am preparing (configuring) for these boards. The kernel prepared for the production board does not include the network drivers necessary for my sama5d2 board, and so running the production kernel on my sama5d2 leaves me with a board without any network, and I am unable to test enftun on this.

  • These kernel commits will still depend on each other, as adding one kernel option may result in other options being commented out as they are now implied (just like enabling support for enftun removes xtt from the buildroot config as enftun requires xtt), or a list un 'not set' options appearing.

Absolutely. But the "semantic" changes will be independent.

This was actually much less of a problem than I had anticipated - after I had isolated the changes introduced by a simple make menuconfig followed by a make savedefconfig (with no options altered) on the at-the-time latest config in Git. With this out of the way, the actual changes I did to support enftun are all really straightforward, clean and as expected.

drbild commented 6 years ago
  • You asked to rename the samadd2 config, replacing xaprc001 by xaprc_dev, but there is a directory called sama5d2_xplained_xaprc001 - do you want that renamed as well?

Exactly. That is intended to be a config for using the Xplained board to do development, without infecting the production config with unnecessary options. Feel free to add to that as needed.

My question was more about renaming the directory here, rather than modifying it's contents.

Yes, renaming is good. Other changes are good to though; whatever helps you.

If you want me to commit a kernel config without (network) support for my board (sama5d2 xplained), then you're asking me to commit a kernel config which I can't test on my board, so that will be an untested commit.

Good catch. I think if the commit adding network support comes first in the series, its ok though. Because then the subsequent commits will have network support and be testable.

I think we have a misunderstanding here. I was referring to the config files NOT specifically for the sama5d2 board - the config files for the actual production boards.

I see. I'll take those as untested for now.

When do you expect to have production boards to test with?