Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RGMIIResource #149

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add RGMIIResource #149

wants to merge 1 commit into from

Conversation

twam
Copy link
Contributor

@twam twam commented Mar 27, 2021

Preparation for PR #148

@twam
Copy link
Contributor Author

twam commented Feb 21, 2022

Rebased on latest main.

@TechnoMancer
Copy link

Because the MDIO bus can support multiple PHYs it is often shared between PHYs (this is the case on all the colorlight boards as far as I know).

Thus I think it is unwise to bundle it in with the RGMII signals as you can only request a given pin once.

Adding it to the RGMIIResource for each PHY will result in only being able to use one of them at a time.

@ghost
Copy link

ghost commented Apr 20, 2022

I agree with @TechnoMancer that MDIO poses a problem if it is shared, however the current resource definition system is already extremely prone to this problem and solutions for it will require more of the redesign and rethink on how potentially shared resources are requested. In the meantime I think that defining a separate MDIOResource would be better than only being able to use a single PHY at a time (this would not actually happen as the resource system does not check for underlying pin conflicts, only top-level Resource conflicts, but such an interface would be unsafe and unwise to expose, which is almost certainly worse).

For future designs we should think about how we might want to indicate that one resource is tangentially related to another (say, this MDIO applies to these PHYs), mostly for a sort of documentary function.

@@ -142,3 +142,20 @@ def PS2Resource(*args, clk, dat, conn=None, attrs=None):
ios.append(attrs)

return Resource.family(*args, default_name="ps2", ios=ios)


def RGMIIResource(*args, txc, txd, tx_ctl, rxc, rxd, rx_ctl, mdc, mdio, attrs=None, conn=None):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Should mdc and mdio be optional pins?
  • Should there be an optional rst_n pin?

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

Successfully merging this pull request may close these issues.

None yet

3 participants