Skip to content

Conversation

@ryanhaus
Copy link

See #424.
This PR takes the '--local-ip' argument into consideration when generating the SoC.

New 'netboot' command output:

litex> netboot

Booting from network...
Local IP: 10.138.242.37
Remote IP: 10.138.242.38
...

@trabucayre
Copy link
Contributor

Hi and sorry for the delay.
Most of the target with ethernet have an eth_ip and remote_ip parameter at constructor level.
These two variables are used by add_ethernet and constant filled accordingly.

So something like:

@@ -109,7 +114,11 @@ def main():
         if "leds" in board.soc_capabilities:
             soc_kwargs.update(with_led_chaser=True)
         if "ethernet" in board.soc_capabilities:
-            soc_kwargs.update(with_ethernet=True)
+            soc_kwargs.update(
+                with_ethernet = True,
+                eth_ip        = args.local_ip,
+                remote_ip     = args.remote_ip,
+            )
         if "pcie" in board.soc_capabilities:
             soc_kwargs.update(with_pcie=True)
         if "spiflash" in board.soc_capabilities:

Looks easier and uses already existing code

@ryanhaus
Copy link
Author

@trabucayre Thank you for the suggestion, I have changed it to be implemented that way & tested it out. I also removed now-unused code due to this change.

@trabucayre
Copy link
Contributor

Looks good, the only thing (but maybe too time consuming) is to check if all targets have correct add_ethernet call.
I will check it quickly

@ryanhaus
Copy link
Author

@trabucayre I did a quick scan through of the targets in litex-boards, looks like many of them don't use eth_ip/local_ip or remote_ip when configuring ethernet. I only have a Digilent Arty to test with, so I don't want to modify other boards in case it breaks something in a board I can't test.

From my understanding, the current approach in this repo is to allow the board to setup ethernet with default values, and then manually overriding the LOCALIPx and REMOTEIPx constants after configuration (see here). Although I agree using soc_kwargs to set the IP constants beforehand is probably the better solution, it may be more work than it's worth. My recently merged PR enjoy-digital/litex#2259 does make it more readable to manually override those constants. What do you think?

@trabucayre
Copy link
Contributor

I'm agree this PR improves readability, but I think I will trying to modify all targets to be coherent and creating a PR to discuss this point with @enjoy-digital
The idea behind this is to have an homogeneous approach for ethernet, otherwise some targets supports eth_ip/remote_ip, some others no.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants