ARM-software/CMSIS_5

Missing I/O Control support in Driver_ETH_PHY interface

lzo2 opened this issue · 7 comments

lzo2 commented

Hello,
I am wondering why CMSIS-Driver Ethernet PHY API is missing the widespread “Control” function available in other CMSIS Driver APIs. I am currently implementing a T1S PHY driver, which also provides a PLCA (Physical Layer Collision Avoidance) layer with the PHY. Providing such control interface "int32_t (*Control) (uint32_t control, uint32_t arg)" would be greatly helpful to pass ioctl configuration requests to that PLCA hardware.
Can you tell me, if there is some reason for not providing that generic "Control" method in ETH_PHY interface ? or do I have to consider PLCA as a separated peripheral with its own driver interface ?
Thanks and regards

Hello,
as you noticed, CMSIS-Driver Ethernet PHY API is as simple as it gets, therefore only basic functionality is supported.
Would it be an option to set configuration in PowerControl function? Can you share the PHY device name so that we can take a look at the datasheet?

lzo2 commented

Hi,
The PHY device is the LAN8670 from Microchip with the following datasheet
Internally my driver is implementing the "Clause 45 Register Access" with the rw function pointers passed during the Initialize call (ARM_ETH_PHY_Read_t fn_read, ARM_ETH_PHY_Write_t fn_write).
My concerns is about providing I/O control to let applicative layer, changing PLCA setup, like for example enable/disable PLCA, configuring the nodeID or the nodeCount, or other specific PLCA settings.

Many thanks for the additional info!
In the short term I think it is best using approach with the rw function pointers and implement extension separately.
Changing API is a long process and in this case it would also break existing implementations. Therefore we can take this into account as an option for future versions.
What is the network stack used in your case? Where is PLCA handling implemented currently?

lzo2 commented

The network stack currently used is emNet from Segger, and the integration with CMSIS-Driver PHY Api works fine for physical layer communication, state, init, etc...
The PLCA handling is partly done in that network stack: initialization and default configuration. But PLCA is also handled by application to change node configuration, enable/disable. For example, the LAN8670 is provided by Microchip in a "10BASE-T1S USB Adapter", and you can see in their application note , the kind of PLCA ethernet settings you can configure at application level:
Capture 10BASE-T1S USB Adapter Properties
We need to provide same kind of control within the PHY/PLCA driver, at application level. And a generic int32_t Control (uint32_t control, uint32_t arg) method would be enough to do the job.
I understand that such kind of API break takes time and is long process. How can I follow-up that such enhancement is taken into account by the community and approved for a future version ?

Hi @lzo2, @VladimirUmek,

I think the question here is: What is the purpose of the CMSIS-Driver API?

So far, all the CMSIS-Driver APIs (not only the PHY one) focus on the functionality required by middleware components, such as the IP Stack or the likes. It is assumed that the application is specific to the selected hardware (i.e., the PHY in this case) but the middleware of course is not.

If one wants to write an application that is agnostic to the concret hardware, it should not be too complicated to extend the API. In a later step we could review the API extensions and discuss if these are generic enough to be added into the CMSIS-Driver API. My minimum expectation would be to review at least two different implementations in order to get a good feeling for the abstraction.

One remark to POSIX ioctl-like approaches: While this indeed has a "common" function signature across all kinds of hardware and implementation it is not really an abstraction. Without knowing what concret hardware is behind, knowing which control values are supported and what type of arg it expects, one cannot really make use of it. Even worse, the compile cannot be much of a help as well. One will only know at runtime if the function call is correct or not. In CMSIS-Driver APIs we want to be more strict, more typesafe and concrete.

I.e., a call like control(5, 7) shall have an exact meaning that has the very same effect for all implementations (while it may be optional of course). Instead of hiding that effect behind arbitrary numbers, one could provide a specific API function such as setSomethingSpecific(type_t value).

lzo2 commented

Hi @JonatanAntoni ,

I agree with you that good abstraction should not rely on a single implementation and two is a minimal.
I understand the need for CMSIS-Driver API to be more strict and provide a common set of functional controls shared by most of the implementations. Setting in CMSIS driver headers, the definition of the supported controls concretely formalize the call behaviors -not like ioctl where everything is possible and sometimes the worst. Using enum instead of uint32_t could also increased this typesafe evaluation. An API proposing uint32_t control, uint32_t arg made me think it was open to some specific vendor extensions. I admit that reserving a range for specific controls in the API would be better to avoid conflicts.

As the time for adopting new functions or new control typedef may be long, I find that current API architecture leaves no room for some specific driver extensions. For example, if I want to add new specific API function ( setSomethingSpecific(type_t value) ) as you suggested, there is no mean in the current access structure of the driver to provide an opaque vendor specific pointer containing that new API function, waiting for its adoption

typedef struct _ARM_DRIVER_USART {
  ARM_DRIVER_VERSION     (*GetVersion)      (void); 
  ...
  ARM_USART_MODEM_STATUS (*GetModemStatus)  (void);
  void * VendorExtension;
} const ARM_DRIVER_USART;

But understanding what you have said about the purpose of the CMSIS-Driver API and philosophy, I feel that extendibility is not the main driver compared to conformity. BTW, do you know if some conformance test suites exist to validate our driver implementation ?

Hi @lzo2,

There are some ongoing discussions about how to add vendor specific calls. The easiest but not well abstracted way would be accessing the native driver implementation directly without going through the CMSIS-Driver abstraction.

Another possibility might be using "inheritance" like this:

typedef _ARM_DRIVER_USART_EX {
  struct _ARM_DRIVER_USART drv;
  void (*VendorExtension)(void);
} ARM_DRIVER_USART_EX;

ARM_DRIVER_USART_EX usartEx = {
  <API functions>
  ...
  VendorExtensionImpl
};

Having the original driver api structure as the first member in the extended structure allows some "dirty hack" on a driver structure:

  ARM_DRIVER_USART* usart1 = &usartEx.drv;
  ...
  ARM_DRIVER_USART_EX* usart1ex = (ARM_DRIVER_USART_EX*)usart1;

The cast is not type-safe, of course. I.e., one needs to be sure that the retrieved pointer actually points to an instance of an extended driver.

For validating your driver implementation please take a look into https://github.com/ARM-software/CMSIS-Driver_Validation.

Cheers,
Jonatan