On 9/7/21 7:32 am, Kinsey Moore wrote: > On 7/8/2021 10:36, Kinsey Moore wrote: >> On 7/8/2021 02:48, Chris Johns wrote: >>> On 8/7/21 11:11 am, Kinsey Moore wrote: >>>> On 7/7/2021 19:28, Chris Johns wrote: >>>>> We can: >>>>> 1. Add FDT support. This is something I would like to avoid as it adds an >>>>> extra >>>>> layer of dependency and it complicates backwards compatibility for >>>>> existing >>>>> users. I however wonder if this is just me and it is something that will >>>>> be >>>>> needed more and more and cannot be avoided. >>>> It's not just you, I (and Joel) lean this direction as well. Having the >>>> option >>>> of FDT would be nice and would certainly cover this case, but making it a >>>> hard >>>> requirement seems excessive. >>> If we could contain a suitable default FDT in the BSP that achieves the >>> outcome >>> then I would be OK. >>> >>>>> 2. Add a weak call that is RTEMS and BSP specific to return a probe >>>>> state. The >>>>> default state could be enabled and the tests provide something from the >>>>> network >>>>> config. Not a great solution but it is simple and cheap to implement. The >>>>> driver >>>>> already has a weak call to set the reference clock. >>>> This patch (since omitted above) strikes a similar compromise but has the >>>> downside of the default application configuration not having network at >>>> all. >>>> The >>>> downside of this suggestion (option #2) is the addition of a second method >>>> to >>>> accomplish the same goal: define a new hook vs redefine or add to the >>>> existing >>>> definitions in nexus-devices.h. My initial patch addressed both, but added >>>> what >>>> appeared to be dead options to RTEMS and has since been reverted. The other >>>> current/alternate patch addresses both but leaks test configuration state >>>> into >>>> the application by installing the test network configuration. >>> I do not see them as being the same thing, yes they achieve the same result >>> however via different interfaces. Your patch creates a top level "user >>> interface", ie at the publicly exposed nexus bus interface while a weak >>> driver >>> interface is a private agreement between the BSP and this driver plus users >>> can >>> provide their own implementation to further change the mix without needing >>> to >>> configure or touch any released code. I am concerned the pattern of defining >>> mixes in the nexus header is adopted by others on a look and copy basis and >>> we >>> get more and more options that never get compiled and rot. >>> >>> A 2.1 alternative could be adding a global define such as .... >>> >>> RTEMS_BSD_CONFIG_NEXUS_BUS_DISABLE_DEFAULT_DRIVERS >>> >>> that disables all the provided definitions and the user can then provide >>> their >>> own? Maybe the network config could be extended to allow such defines? >>> >>> A global disable is needed so the BSD config header I posted before could be >>> used without nexus defines clashing. >>> >>>>> 3. Try and detect a PHY in the probe? I am not sure if that is easy or >>>>> hard. If >>>>> read works maybe that is something that may be suitable. >>>> That's what is currently happening. >>> Are you sure? I see the probe as passing and the device is consider >>> available >>> however there is no PHY and that creates the errors and that seems >>> reasonable. >>> The UKPHY driver is designed to inspect all PHY addresses however needing >>> 200 >>> reties does seem excessive. >>> >>> I was thinking about a simpler scan of the MII bus for a PHY in the probe >>> and if >>> none found disable return -1 from the probe. >> Sorry, I misunderstood what you were saying. You're correct that a PHY >> detection does not occur in the CGEM probe, but that wouldn't improve the >> current situation since PHYs are being detected on all available slots. If >> that weren't happening there wouldn't be PHY read/write errors, either. >>>> Unfortunately, the ukphy driver is there to >>>> detect braindead PHYs (I have a board which needs it, but could probably >>>> use a >>>> more specific PHY driver in its place) and will pick up certain bits being >>>> set >>>> as being a valid PHY. Perhaps the ukphy driver just needs more specificity >>>> or >>>> maybe the ukphy driver should never be used in nexus-devices.h when there >>>> are >>>> multiple interfaces provided for a BSP. >>> I think the UKPHY driver is OK, it is the cgem driver that has a crazy high >>> retry count and generates the error message. A failing probe with no error >>> messages generates no output. >> From above, the probe wouldn't fail in that case due to the spurious PHY >> detections unless you're trying to do a read/write transaction with the PHY >> and waiting for the timeout. This doesn't seem reasonable to do in a device >> probe. >>>>> I would prefer we avoid special build options for BSPs. It is easy for us >>>>> as >>>>> developers to make these changes and build a specific RTEMS plus we can >>>>> be a >>>>> little narrow in our focus when delivering something for a client in the >>>>> defaults we select. Specific builds of a BSP to configure options becomes >>>>> hard >>>>> for users where they have a few Zynq or ZynqMP designs all running RTEMS >>>>> and >>>>> each with a different mix of network interfaces. This creates a complex >>>>> set of >>>>> build options for common application code plus more configuration items to >>>>> control. >>>> It's possible that the ukphy driver could be >>>> improved and this entire problem just goes away or we ban that driver from >>>> the >>>> default configuration for multi-interface BSPs and the problem goes away. >>> The UKPHY is an important driver and we need it. The MVME2700 runs a rev 1 >>> Tulip >>> chip and the PHY on the board is so old there is no data on it and the >>> company >>> has long gone. The PHY does support the IEEE MII basic registers and the >>> UKPHY >>> works. I think our issue here is in the cgem driver and not else where. >> >> Ok, so the ukphy driver is too risky for behavioral changes and the CGEM >> driver needs to be tweaked to be a bit less verbose.
Yeap. >> >> The solutions to the remainder of the problem are: >> >> 1) Use a different, smarter PHY driver and avoid use of ukphy when multiple >> interfaces are present >> >> ** This could possibly solve the general problem for ethernet interfaces and >> replace the ukphy driver for new multi-interface hardware >> >> ** Lets nexus-devices.h contain all interfaces that exist on the hardware, >> possibly applicable to Zynq7000 and versal as well >> The problem here is some PHYs are covered by NDAs and a PHY that uses just the IEEE defined registers, ie UKPHY, is a good solution for a number of cases. I do not think the PHY drivers are the issue here. >> 2) Disable unused interfaces via weak-reference call in the CGEM probe >> >> ** This solves the CGEM problem and provides a template on how to solve the >> general problem for other BSPS/interfaces >> >> ** Allows the application to make arbitrarily different choices by >> implementing a relatively trivial function >> >> ** This feels like it's working toward a partial reimplementation of FDT with >> hardware configuration embedded in code Yes. Adding FDT and not breaking existing users is not easy. >> ** Lets nexus-devices.h contain all interfaces that exist on the hardware, >> possibly applicable to Zynq7000 and versal as well Yeah. The Versal MRMAC may be an exception here. I have not had time to examine all the detail that surrounds that MAC implementation. >> 3) Disable unused interfaces via header configuration conditionals >> >> ** Allows the application to define the interfaces it needs using existing >> mechanisms >> >> ** Doesn't provide any network interfaces by default to the application >> >> ** Only solves the problem one BSP at a time with extra preprocessor logic It is hard to remove once released and user depend on it. We spend a lot of time in RTEMS attempting to handle these cases further down the road. >> 4) Embed FDT somewhere that defines what interfaces get configured >> >> ** Pulls in extra dependencies >> >> ** Doesn't solve the test issue without generation/embedding of a FDT >> >> ** Allows the application to have exactly what it wants via a relatively >> standard mechanism >> >> ** Lets nexus-devices.h contain all interfaces that exist on the hardware, >> possibly applicable to Zynq7000 and versal as well >> I wonder if it is worth considering how a user would add and remove interfaces using this approach before we head down this path? >> I'm starting to lean toward option #1. It allows all interfaces to be defined >> and the primary interface for testing can be selected by the existing test >> configuration in config.inc. Applications can check the available interfaces >> for connectivity or they can prune the nexus devices in their own copy >> instead >> of pulling in nexus-devices.h. > > I looked a little further into what it would take to make a smarter version of > ukphy and ukphy isn't actually the issue here. The code in mii.c that detects > whether a PHY exists at all has a bug in it where PHY read timeouts aren't > detected and handled and so the 0xffffffff result of the failed read slips > through the bad-PHY check. I'm working up and verifying the patch for that > now. > Fixing that should hopefully allow for all interfaces to be enabled without > issue. The mii.c only comes into play when the probe passes and we are attaching the device. I was thinking of a MII probe call ... #if __rtems__ int mii_probe() { for (phy = 0; phy < 31; phy++) { WR4(sc, CGEM_PHY_MAINT, CGEM_PHY_MAINT_CLAUSE_22 | CGEM_PHY_MAINT_MUST_10 | CGEM_PHY_MAINT_OP_READ | (phy << CGEM_PHY_MAINT_PHY_ADDR_SHIFT) | (reg << CGEM_PHY_MAINT_REG_ADDR_SHIFT)); /* Wait for completion. */ tries=0; for (tries = 0; tries < 5; ++tries) { if ((RD4(sc, CGEM_NET_STAT) & CGEM_NET_STAT_PHY_MGMT_IDLE) != 0) { return OK; } DELAY(2); } } return NOT_FOUND; } #endif This only works if the MAC can be accessed this early in a boot and dos not need anything else to be enabled. Maybe the enables are easy as well, I have not looked. For an interface with no PHY and not being used the MII probe delay is 192msec which I think is just OK, maybe (??), but more than I would like. Chris _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel