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.
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
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
** Lets nexus-devices.h contain all interfaces that exist on the
hardware, possibly applicable to Zynq7000 and versal as well
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
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'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.
Kinsey
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
