lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9a612b07-fe02-40d6-a1d4-7a6d1266ed18@ieee.org>
Date: Wed, 21 May 2025 21:46:47 -0500
From: Alex Elder <elder@...e.org>
To: Andrew Lunn <andrew@...n.ch>, Damien RiƩgel
 <damien.riegel@...abs.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Andrew Lunn <andrew+netdev@...n.ch>, "David S . Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Silicon Labs Kernel Team <linux-devel@...abs.com>, netdev@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 Johan Hovold <johan@...nel.org>, Alex Elder <elder@...nel.org>,
 greybus-dev@...ts.linaro.org
Subject: Re: [RFC net-next 00/15] Add support for Silicon Labs CPC

On 5/18/25 10:23 AM, Andrew Lunn wrote:
>> I think Andrew pulled Greybus in the discussion because there is some
>> overlap between Greybus and CPC:
>>    - Greybus has bundles and CPorts, CPC only has "endpoints", which
>>      would be the equivalent of a bundle with a single cport
>>    - discoverability of Greybus bundles/CPC endpoints by the host
>>    - multiple bundles/endpoints might coexist in the same
>>      module/CPC-enabled device
>>    - bundles/endpoints are independent from each other and each has its
>>      own dedicated driver
>>
>> Greybus goes a step further and specs some protocols like GPIO or UART.
>> CPC doesn't spec what goes over endpoints because it's geared towards
>> radio applications and as you said, it's very device/stack specific.
> 
> Is it device specific? Look at your Bluetooth implementation. I don't
> see anything device specific in it. That should work for any of the
> vendors of similar chips to yours.
> 
> For 802.15.4, Linux defines:
> 
> struct ieee802154_ops {
>          struct module   *owner;
>          int             (*start)(struct ieee802154_hw *hw);
>          void            (*stop)(struct ieee802154_hw *hw);
>          int             (*xmit_sync)(struct ieee802154_hw *hw,
>                                       struct sk_buff *skb);
>          int             (*xmit_async)(struct ieee802154_hw *hw,
>                                        struct sk_buff *skb);
>          int             (*ed)(struct ieee802154_hw *hw, u8 *level);
>          int             (*set_channel)(struct ieee802154_hw *hw, u8 page,
>                                         u8 channel);
>          int             (*set_hw_addr_filt)(struct ieee802154_hw *hw,
>                                              struct ieee802154_hw_addr_filt *filt,
>                                              unsigned long changed);
>          int             (*set_txpower)(struct ieee802154_hw *hw, s32 mbm);
>          int             (*set_lbt)(struct ieee802154_hw *hw, bool on);
>          int             (*set_cca_mode)(struct ieee802154_hw *hw,
>                                          const struct wpan_phy_cca *cca);
>          int             (*set_cca_ed_level)(struct ieee802154_hw *hw, s32 mbm);
>          int             (*set_csma_params)(struct ieee802154_hw *hw,
>                                             u8 min_be, u8 max_be, u8 retries);
>          int             (*set_frame_retries)(struct ieee802154_hw *hw,
>                                               s8 retries);
>          int             (*set_promiscuous_mode)(struct ieee802154_hw *hw,
>                                                  const bool on);
> };
> 
> Many of these are optional, but this gives an abstract representation
> of a device, which is should be possible to turn into a protocol
> talked over a transport bus like SPI or SDIO.

This is essentially how Greybus does things.  It sets
up drivers on the Linux side that translate callback
functions into Greybus operations that get performed
on target hardware on the remote module.

> This also comes back to my point of there being at least four vendors
> of devices like yours. Linux does not want four or more
> implementations of this, each 90% the same, just a different way of
> converting this structure of operations into messages over a transport
> bus.
> 
> You have to define the protocol. Mainline needs that so when the next
> vendor comes along, we can point at your protocol and say that is how
> it has to be implemented in Mainline. Make your firmware on the SoC
> understand it.  You have the advantage that you are here first, you
> get to define that protocol, but you do need to clearly define it.

I agree with all of this.

> You have listed how your implementation is similar to Greybus. You say
> what is not so great is streaming, i.e. the bulk data transfer needed
> to implement xmit_sync() and xmit_async() above. Greybus is too much
> RPC based. RPCs are actually what you want for most of the operations
> listed above, but i agree for data, in order to keep the transport
> fully loaded, you want double buffering. However, that appears to be
> possible with the current Greybus code.
> 
> gb_operation_unidirectional_timeout() says:

Yes, these are request messages that don't require a response.
The acknowledgement is about when the host *sent it*, not when
it got received.  They're rarely used but I could see them being
used this way.  Still, you might be limited to 255 or so in-flight
messages.

					-Alex

>   * Note that successful send of a unidirectional operation does not imply that
>   * the request as actually reached the remote end of the connection.
>   */
> 
> So long as you are doing your memory management correctly, i don't see
> why you cannot implement double buffering in the transport driver.
> 
> I also don't see why you cannot extend the Greybus upper API and add a
> true gb_operation_unidirectional_async() call.
> 
> You also said that lots of small transfers are inefficient, and you
> wanted to combine small high level messages into one big transport
> layer message. This is something you frequently see with USB Ethernet
> dongles. The Ethernet driver puts a number of small Ethernet packets
> into one USB URB. The USB layer itself has no idea this is going on. I
> don't see why the same cannot be done here, greybus itself does not
> need to be aware of the packet consolidation.
> 
> 	Andrew


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ