[PATCH rtems-libbsd] rtemsbsd: Use a separate header for test devices

[Kinsey Moore]

On 7/7/2021 19:28, Chris Johns wrote:
> On 7/7/21 11:26 pm, Kinsey Moore wrote:
>> On 7/6/2021 21:20, Chris Johns wrote:
>>> On 7/7/21 12:03 pm, Kinsey Moore wrote:
>>>> On 7/6/2021 20:57, Chris Johns wrote:
>>>>> On 7/7/21 11:05 am, Kinsey Moore wrote:
>>>>>> The need for the difference on ZynqMP is that there are 4 different CGEM
>>>>>> interfaces of which dev boards primarily make use of CGEM3.
>>>>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM0(ZYNQMP_IRQ_ETHERNET_0);
>>>>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM1(ZYNQMP_IRQ_ETHERNET_1);
>>>>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM2(ZYNQMP_IRQ_ETHERNET_2);
>>>>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM3(ZYNQMP_IRQ_ETHERNET_3);
>>>>>
>>>>> ?
>>>> Yes, this does technically work
>>> Hmm, I suggest this is what we should support as a default.
>>>
>>>> if you can read the shell output past the log
>>>> spam. The other interfaces trying and failing to come up throw a gargantuan
>>>> amount of messages to the console.
>>> Why do these interfaces fail to initialise? What are the errors?
>>>
>>> The removed probe check is based around FDT and so I suspect is the reason
>>> Sebastian suggested FDT support. Needing FDT support would break existing Zynq
>>> users.
>> The devices themselves are detected just fine and are likely operational. The
>> MII busses are probably unterminated or pulled high/low which yields a ukphy
>> detection on every available slot and constant PHY read/write timeouts. A small
>> sample from enabling both CGEM2 and CGEM3 on this custom board:
>>
>> ukphy13:  none, 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 100baseT4,
>> 1000baseSX, 1000baseSX-FDX, 1000baseT, 1000baseT-master, 1000baseT-FDX,
>> 1000baseT-FDX-master, auto
>> cgem3: phy read timeout: 0
>> ukphy14: <Generic IEEE 802.3u media interface> PHY 14 on miibus0
>> cgem3: phy write timeout: 0
>> cgem3: phy read timeout: 0
>> cgem3: phy read timeout: 0
>> cgem3: phy read timeout: 0
>> cgem3: phy read timeout: 0
>> cgem3: phy read timeout: 0
>> cgem3: phy read timeout: 0
>>
>> ...
>>
>> cgem3: phy read timeout: 0
> That is noisy and painful.
>
>> This type of output reached ukphy31 before I was able to run the shutdown
>> command with 50+ PHY read/write errors and more continuing afterward. I imagine
>> it's worse if I enable all 4 interfaces.
> The output is from the cgem driver which is good because it is localised. I
> agree we need something to control what is enabled and I prefer we avoid special
> build options, I will explain why I think this later. 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.

> 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.
> 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. 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 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.

I wonder why this never came up with Zynq or QorIQ. Maybe no one wanted 
to run network tests on the alternate interfaces because dev boards with 
those interfaces configured didn't exist? 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.


Kinsey