| Message ID | 20230606123119.57499-1-manivannan.sadhasivam@linaro.org |
|---|---|
| Headers | show |
| Series | Add MHI Endpoint network driver | expand |
On Tue, 6 Jun 2023 18:01:16 +0530 Manivannan Sadhasivam wrote: > This series adds a network driver for the Modem Host Interface (MHI) endpoint > devices that provides network interfaces to the PCIe based Qualcomm endpoint > devices supporting MHI bus (like Modems). This driver allows the MHI endpoint > devices to establish IP communication with the host machines (x86, ARM64) over > MHI bus. > > On the host side, the existing mhi_net driver provides the network connectivity > to the host. So the host can talk to the firmware over IP?
On Tue, Jun 06, 2023 at 02:59:00PM +0200, Andrew Lunn wrote: > On Tue, Jun 06, 2023 at 06:01:16PM +0530, Manivannan Sadhasivam wrote: > > Hi, > > > > This series adds a network driver for the Modem Host Interface (MHI) endpoint > > devices that provides network interfaces to the PCIe based Qualcomm endpoint > > devices supporting MHI bus (like Modems). This driver allows the MHI endpoint > > devices to establish IP communication with the host machines (x86, ARM64) over > > MHI bus. > > > > On the host side, the existing mhi_net driver provides the network connectivity > > to the host. > > > > - Mani > > > > Manivannan Sadhasivam (3): > > net: Add MHI Endpoint network driver > > MAINTAINERS: Add entry for MHI networking drivers under MHI bus > > net: mhi: Increase the default MTU from 16K to 32K > > > > MAINTAINERS | 1 + > > drivers/net/Kconfig | 9 ++ > > drivers/net/Makefile | 1 + > > drivers/net/mhi_ep_net.c | 331 +++++++++++++++++++++++++++++++++++++++ > > drivers/net/mhi_net.c | 2 +- > > Should we add a drivers/net/modem directory? Maybe modem is too > generic, we want something which represents GSM, LTE, UMTS, 3G, 4G, > 5G, ... XG etc. > The generic modem hierarchy sounds good to me because most of the times a single driver handles multiple technologies. The existing drivers supporting modems are already under different hierarchy like usb, wwan etc... So unifying them makes sense. But someone from networking community should take a call. - Mani > Andrew
On Tue, Jun 06, 2023 at 07:50:23AM -0600, Jeffrey Hugo wrote: > On 6/6/2023 6:31 AM, Manivannan Sadhasivam wrote: > > Most of the Qualcomm endpoint devices are supporting 32K MTU for the > > UL (Uplink) and DL (Downlink) channels. So let's use the same value > > in the MHI NET driver also. This gives almost 2x increase in the throughput > > for the UL channel. > > > > Below is the comparision: > > > > iperf on the UL channel with 16K MTU: > > > > [ ID] Interval Transfer Bandwidth > > [ 3] 0.0-10.0 sec 353 MBytes 296 Mbits/sec > > > > iperf on the UL channel with 32K MTU: > > > > [ ID] Interval Transfer Bandwidth > > [ 3] 0.0-10.0 sec 695 MBytes 583 Mbits/sec > > > > Cc: Loic Poulain <loic.poulain@linaro.org> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/net/mhi_net.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/mhi_net.c b/drivers/net/mhi_net.c > > index 3d322ac4f6a5..eddc2c701da4 100644 > > --- a/drivers/net/mhi_net.c > > +++ b/drivers/net/mhi_net.c > > @@ -14,7 +14,7 @@ > > #define MHI_NET_MIN_MTU ETH_MIN_MTU > > #define MHI_NET_MAX_MTU 0xffff > > -#define MHI_NET_DEFAULT_MTU 0x4000 > > +#define MHI_NET_DEFAULT_MTU 0x8000 > > Why not SZ_32K? Makes sense. Will change it in next iteration. - Mani
On Tue, Jun 06, 2023 at 02:22:27PM -0700, Jakub Kicinski wrote: > On Tue, 6 Jun 2023 18:01:16 +0530 Manivannan Sadhasivam wrote: > > This series adds a network driver for the Modem Host Interface (MHI) endpoint > > devices that provides network interfaces to the PCIe based Qualcomm endpoint > > devices supporting MHI bus (like Modems). This driver allows the MHI endpoint > > devices to establish IP communication with the host machines (x86, ARM64) over > > MHI bus. > > > > On the host side, the existing mhi_net driver provides the network connectivity > > to the host. > > So the host can talk to the firmware over IP? That's the typical usecase of these PCIe based modems. On the host, mhi_net driver creates the network interface that communicates with the endpoint over MHI host stack. On the endpoint, mhi_ep_net driver creates the network interface that communicates with the host over MHI endpoint stack. These drivers work on top of MHI channels like IP_SW0, IP_HW0 etc... IP_SW0 channel represents the IP communication between host and modem CPUs while IP_HW0 represents IP communication between host and modem DSP. - Mani
On Wed, 7 Jun 2023 at 08:56, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: > > On Tue, Jun 06, 2023 at 02:59:00PM +0200, Andrew Lunn wrote: > > On Tue, Jun 06, 2023 at 06:01:16PM +0530, Manivannan Sadhasivam wrote: > > > Hi, > > > > > > This series adds a network driver for the Modem Host Interface (MHI) endpoint > > > devices that provides network interfaces to the PCIe based Qualcomm endpoint > > > devices supporting MHI bus (like Modems). This driver allows the MHI endpoint > > > devices to establish IP communication with the host machines (x86, ARM64) over > > > MHI bus. > > > > > > On the host side, the existing mhi_net driver provides the network connectivity > > > to the host. > > > > > > - Mani > > > > > > Manivannan Sadhasivam (3): > > > net: Add MHI Endpoint network driver > > > MAINTAINERS: Add entry for MHI networking drivers under MHI bus > > > net: mhi: Increase the default MTU from 16K to 32K > > > > > > MAINTAINERS | 1 + > > > drivers/net/Kconfig | 9 ++ > > > drivers/net/Makefile | 1 + > > > drivers/net/mhi_ep_net.c | 331 +++++++++++++++++++++++++++++++++++++++ > > > drivers/net/mhi_net.c | 2 +- > > > > Should we add a drivers/net/modem directory? Maybe modem is too > > generic, we want something which represents GSM, LTE, UMTS, 3G, 4G, > > 5G, ... XG etc. > > > > The generic modem hierarchy sounds good to me because most of the times a > single driver handles multiple technologies. The existing drivers supporting > modems are already under different hierarchy like usb, wwan etc... So unifying > them makes sense. But someone from networking community should take a call. Yes, so there is already a drivers/net/wwan directory for this, in which there are drivers for control and data path, that together represent a given 'wwan' (modem) entity. So the generic mhi_net could be moved there, but the point is AFAIU, that MHI, despite his name, is not (more) used only for modem, but as a generic memory sharing based transport protocol, such as virtio. It would then not be necessarily true that a peripheral exposing MHI net channel is actually a modem? Regards, Loic
On Wed, Jun 07, 2023 at 09:12:00AM +0200, Loic Poulain wrote: > On Wed, 7 Jun 2023 at 08:56, Manivannan Sadhasivam > <manivannan.sadhasivam@linaro.org> wrote: > > > > On Tue, Jun 06, 2023 at 02:59:00PM +0200, Andrew Lunn wrote: > > > On Tue, Jun 06, 2023 at 06:01:16PM +0530, Manivannan Sadhasivam wrote: > > > > Hi, > > > > > > > > This series adds a network driver for the Modem Host Interface (MHI) endpoint > > > > devices that provides network interfaces to the PCIe based Qualcomm endpoint > > > > devices supporting MHI bus (like Modems). This driver allows the MHI endpoint > > > > devices to establish IP communication with the host machines (x86, ARM64) over > > > > MHI bus. > > > > > > > > On the host side, the existing mhi_net driver provides the network connectivity > > > > to the host. > > > > > > > > - Mani > > > > > > > > Manivannan Sadhasivam (3): > > > > net: Add MHI Endpoint network driver > > > > MAINTAINERS: Add entry for MHI networking drivers under MHI bus > > > > net: mhi: Increase the default MTU from 16K to 32K > > > > > > > > MAINTAINERS | 1 + > > > > drivers/net/Kconfig | 9 ++ > > > > drivers/net/Makefile | 1 + > > > > drivers/net/mhi_ep_net.c | 331 +++++++++++++++++++++++++++++++++++++++ > > > > drivers/net/mhi_net.c | 2 +- > > > > > > Should we add a drivers/net/modem directory? Maybe modem is too > > > generic, we want something which represents GSM, LTE, UMTS, 3G, 4G, > > > 5G, ... XG etc. > > > > > > > The generic modem hierarchy sounds good to me because most of the times a > > single driver handles multiple technologies. The existing drivers supporting > > modems are already under different hierarchy like usb, wwan etc... So unifying > > them makes sense. But someone from networking community should take a call. > > > Yes, so there is already a drivers/net/wwan directory for this, in > which there are drivers for control and data path, that together > represent a given 'wwan' (modem) entity. So the generic mhi_net could > be moved there, but the point is AFAIU, that MHI, despite his name, is > not (more) used only for modem, but as a generic memory sharing based > transport protocol, such as virtio. It would then not be necessarily > true that a peripheral exposing MHI net channel is actually a modem? > Agree, mhi_*_net drivers can be used by non-modem devices too as long as they support MHI protocol. - Mani > Regards, > Loic
On Tue, Jun 06, 2023 at 06:01:19PM +0530, Manivannan Sadhasivam wrote: > Most of the Qualcomm endpoint devices are supporting 32K MTU for the > UL (Uplink) and DL (Downlink) channels. So let's use the same value > in the MHI NET driver also. This gives almost 2x increase in the throughput > for the UL channel. > > Below is the comparision: Hi Manivannan, as it looks like there will be a v2: comparision -> comparison ...
On Wed, Jun 07, 2023 at 10:20:39AM +0200, Simon Horman wrote: > On Tue, Jun 06, 2023 at 06:01:17PM +0530, Manivannan Sadhasivam wrote: > > ... > > > +static void mhi_ep_net_dev_process_queue_packets(struct work_struct *work) > > +{ > > + struct mhi_ep_net_dev *mhi_ep_netdev = container_of(work, > > + struct mhi_ep_net_dev, xmit_work); > > + struct mhi_ep_device *mdev = mhi_ep_netdev->mdev; > > + struct sk_buff_head q; > > + struct sk_buff *skb; > > + int ret; > > + > > + if (mhi_ep_queue_is_empty(mdev, DMA_FROM_DEVICE)) { > > + netif_stop_queue(mhi_ep_netdev->ndev); > > + return; > > + } > > + > > + __skb_queue_head_init(&q); > > + > > + spin_lock_bh(&mhi_ep_netdev->tx_lock); > > + skb_queue_splice_init(&mhi_ep_netdev->tx_buffers, &q); > > + spin_unlock_bh(&mhi_ep_netdev->tx_lock); > > + > > + while ((skb = __skb_dequeue(&q))) { > > + ret = mhi_ep_queue_skb(mdev, skb); > > + if (ret) { > > Hi Manivannan, > > I wonder if this should be kfree_skb(skb); > Good catch! Will fix it. - Mani > > + kfree(skb); > > + goto exit_drop; > > + } > > ...
On Wed, Jun 07, 2023 at 12:28:09PM +0530, Manivannan Sadhasivam wrote: > On Tue, Jun 06, 2023 at 07:50:23AM -0600, Jeffrey Hugo wrote: > > On 6/6/2023 6:31 AM, Manivannan Sadhasivam wrote: > > > Most of the Qualcomm endpoint devices are supporting 32K MTU for the > > > UL (Uplink) and DL (Downlink) channels. So let's use the same value > > > in the MHI NET driver also. This gives almost 2x increase in the throughput > > > for the UL channel. You say here 'Most'. What happens on those which do not support 32K? Do the packets get dropped and it turns into a black hole? Andrew
> > Yes, so there is already a drivers/net/wwan directory for this, in > > which there are drivers for control and data path, that together > > represent a given 'wwan' (modem) entity. So the generic mhi_net could > > be moved there, but the point is AFAIU, that MHI, despite his name, is > > not (more) used only for modem, but as a generic memory sharing based > > transport protocol, such as virtio. It would then not be necessarily > > true that a peripheral exposing MHI net channel is actually a modem? > > > > Agree, mhi_*_net drivers can be used by non-modem devices too as long as they > support MHI protocol. O.K. I was just trying to avoid cluttering up the directory. But if this is shared code, not actual drivers, this is fine. Are there more features yet to be implemented? Would it make sense to create a mhi directory? Andrew
On 6/7/2023 1:41 AM, Manivannan Sadhasivam wrote: > On Wed, Jun 07, 2023 at 09:12:00AM +0200, Loic Poulain wrote: >> On Wed, 7 Jun 2023 at 08:56, Manivannan Sadhasivam >> <manivannan.sadhasivam@linaro.org> wrote: >>> >>> On Tue, Jun 06, 2023 at 02:59:00PM +0200, Andrew Lunn wrote: >>>> On Tue, Jun 06, 2023 at 06:01:16PM +0530, Manivannan Sadhasivam wrote: >>>>> Hi, >>>>> >>>>> This series adds a network driver for the Modem Host Interface (MHI) endpoint >>>>> devices that provides network interfaces to the PCIe based Qualcomm endpoint >>>>> devices supporting MHI bus (like Modems). This driver allows the MHI endpoint >>>>> devices to establish IP communication with the host machines (x86, ARM64) over >>>>> MHI bus. >>>>> >>>>> On the host side, the existing mhi_net driver provides the network connectivity >>>>> to the host. >>>>> >>>>> - Mani >>>>> >>>>> Manivannan Sadhasivam (3): >>>>> net: Add MHI Endpoint network driver >>>>> MAINTAINERS: Add entry for MHI networking drivers under MHI bus >>>>> net: mhi: Increase the default MTU from 16K to 32K >>>>> >>>>> MAINTAINERS | 1 + >>>>> drivers/net/Kconfig | 9 ++ >>>>> drivers/net/Makefile | 1 + >>>>> drivers/net/mhi_ep_net.c | 331 +++++++++++++++++++++++++++++++++++++++ >>>>> drivers/net/mhi_net.c | 2 +- >>>> >>>> Should we add a drivers/net/modem directory? Maybe modem is too >>>> generic, we want something which represents GSM, LTE, UMTS, 3G, 4G, >>>> 5G, ... XG etc. >>>> >>> >>> The generic modem hierarchy sounds good to me because most of the times a >>> single driver handles multiple technologies. The existing drivers supporting >>> modems are already under different hierarchy like usb, wwan etc... So unifying >>> them makes sense. But someone from networking community should take a call. >> >> >> Yes, so there is already a drivers/net/wwan directory for this, in >> which there are drivers for control and data path, that together >> represent a given 'wwan' (modem) entity. So the generic mhi_net could >> be moved there, but the point is AFAIU, that MHI, despite his name, is >> not (more) used only for modem, but as a generic memory sharing based >> transport protocol, such as virtio. It would then not be necessarily >> true that a peripheral exposing MHI net channel is actually a modem? >> > > Agree, mhi_*_net drivers can be used by non-modem devices too as long as they > support MHI protocol. I know of at-least 1 non-modem product in development that would benefit from these drivers.
On Wed, Jun 07, 2023 at 02:25:50PM +0200, Andrew Lunn wrote: > On Wed, Jun 07, 2023 at 12:28:09PM +0530, Manivannan Sadhasivam wrote: > > On Tue, Jun 06, 2023 at 07:50:23AM -0600, Jeffrey Hugo wrote: > > > On 6/6/2023 6:31 AM, Manivannan Sadhasivam wrote: > > > > Most of the Qualcomm endpoint devices are supporting 32K MTU for the > > > > UL (Uplink) and DL (Downlink) channels. So let's use the same value > > > > in the MHI NET driver also. This gives almost 2x increase in the throughput > > > > for the UL channel. > > You say here 'Most'. What happens on those which do not support 32K? > Do the packets get dropped and it turns into a black hole? > Yeah, and the host has to retransmit. But I checked again with Qcom on the MTU size and got a different answer that forced me to change "most" to "few". So this patch is not needed for now. I'll drop it. - Mani > Andrew >