Re: [PATCH v9 1/2] net: mhi: Enable Ethernet interface support

[vivek pernamitta <vivek.pernamitta@oss.qualcomm.com>]

On 3/24/2026 3:35 PM, Loic Poulain wrote:
> Hi Vivek,
> 
> On Tue, Mar 24, 2026 at 10:02 AM Vivek Pernamitta
> <vivek.pernamitta@oss.qualcomm.com> wrote:
>>
>> Add support to configure a new client as Ethernet type over MHI by
>> setting "mhi_device_info.ethernet_if = true". Create a new Ethernet
>> interface named eth%d. This complements existing NET driver support.
>>
>> Allocate MHI netdevs using NET_NAME_ENUM to reflect kernel-enumerated
>> naming. This updates the reported name_assign_type for MHI net
>> interfaces created by this driver, aligning naming semantics across
>> existing and new devices. No functional or interface naming changes
>> are introduced
>>
>> Introduce ETH0 and ETH1 network interfaces required for NETCONF
>> and S-plane components.
>>
>> IP_ETH channels represent the Ethernet interface exposed by the
>> MHI device. The driver creates a corresponding netdev instance,
>> allowing the host to access and operate the device’s Ethernet
>> interface.
>>
>> NETCONF:
>> Use NETCONF protocol for configuration operations such as fetching,
>> modifying, and deleting network device configurations.
>>
>> S-plane:
>> Support frequency and time synchronization between O-DUs and O-RUs
>> using Synchronous Ethernet and IEEE 1588. Assume PTP transport over
>> L2 Ethernet (ITU-T G.8275.1) for full timing support; allow PTP over
>> UDP/IP (ITU-T G.8275.2) with reduced reliability. as per ORAN spec
>> O-RAN.WG4.CUS.0-R003-v12.00.
>>
>> Signed-off-by: Vivek Pernamitta <vivek.pernamitta@oss.qualcomm.com>
>> ---
>>   drivers/net/mhi_net.c | 65 +++++++++++++++++++++++++++++++++++++++++----------
>>   1 file changed, 53 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/mhi_net.c b/drivers/net/mhi_net.c
>> index ae169929a9d8e449b5a427993abf68e8d032fae2..87d8c61fab983005eaea99603d74a557d1483b6e 100644
>> --- a/drivers/net/mhi_net.c
>> +++ b/drivers/net/mhi_net.c
>> @@ -4,6 +4,7 @@
>>    * Copyright (C) 2020 Linaro Ltd <loic.poulain@linaro.org>
>>    */
>>
>> +#include <linux/etherdevice.h>
>>   #include <linux/if_arp.h>
>>   #include <linux/mhi.h>
>>   #include <linux/mod_devicetable.h>
>> @@ -42,6 +43,7 @@ struct mhi_net_dev {
>>
>>   struct mhi_device_info {
>>          const char *netname;
>> +       bool ethernet_if;
>>   };
>>
>>   static int mhi_ndo_open(struct net_device *ndev)
>> @@ -119,11 +121,29 @@ static void mhi_ndo_get_stats64(struct net_device *ndev,
>>          } while (u64_stats_fetch_retry(&mhi_netdev->stats.tx_syncp, start));
>>   }
>>
>> +static int mhi_mac_address(struct net_device *dev, void *p)
>> +{
>> +       if (dev->type == ARPHRD_ETHER)
>> +               return eth_mac_addr(dev, p);
>> +
>> +       return 0;
> 
> return -EOPNOTSUPP ?

Sure , will make changes
> 
>> +}
>> +
>> +static int mhi_validate_address(struct net_device *dev)
>> +{
>> +       if (dev->type == ARPHRD_ETHER)
>> +               return eth_validate_addr(dev);
>> +
>> +       return 0;
>> +}
>> +
>>   static const struct net_device_ops mhi_netdev_ops = {
>>          .ndo_open               = mhi_ndo_open,
>>          .ndo_stop               = mhi_ndo_stop,
>>          .ndo_start_xmit         = mhi_ndo_xmit,
>>          .ndo_get_stats64        = mhi_ndo_get_stats64,
>> +       .ndo_set_mac_address    = mhi_mac_address,
>> +       .ndo_validate_addr      = mhi_validate_address,
>>   };
>>
>>   static void mhi_net_setup(struct net_device *ndev)
>> @@ -140,6 +160,13 @@ static void mhi_net_setup(struct net_device *ndev)
>>          ndev->tx_queue_len = 1000;
>>   }
>>
>> +static void mhi_ethernet_setup(struct net_device *ndev)
>> +{
>> +       ndev->netdev_ops = &mhi_netdev_ops;
>> +       ether_setup(ndev);
>> +       ndev->max_mtu = ETH_MAX_MTU;
>> +}
>> +
>>   static struct sk_buff *mhi_net_skb_agg(struct mhi_net_dev *mhi_netdev,
>>                                         struct sk_buff *skb)
>>   {
>> @@ -208,17 +235,20 @@ static void mhi_net_dl_callback(struct mhi_device *mhi_dev,
>>                          skb = mhi_net_skb_agg(mhi_netdev, skb);
>>                          mhi_netdev->skbagg_head = NULL;
>>                  }
>> -
>> -               switch (skb->data[0] & 0xf0) {
>> -               case 0x40:
>> -                       skb->protocol = htons(ETH_P_IP);
>> -                       break;
>> -               case 0x60:
>> -                       skb->protocol = htons(ETH_P_IPV6);
>> -                       break;
>> -               default:
>> -                       skb->protocol = htons(ETH_P_MAP);
>> -                       break;
>> +               if (mhi_netdev->ndev->type == ARPHRD_ETHER) {
>> +                       skb->protocol = eth_type_trans(skb, mhi_netdev->ndev);
>> +               } else {
>> +                       switch (skb->data[0] & 0xf0) {
>> +                       case 0x40:
>> +                               skb->protocol = htons(ETH_P_IP);
>> +                               break;
>> +                       case 0x60:
>> +                               skb->protocol = htons(ETH_P_IPV6);
>> +                               break;
>> +                       default:
>> +                               skb->protocol = htons(ETH_P_MAP);
>> +                               break;
>> +                       }
>>                  }
>>
>>                  u64_stats_update_begin(&mhi_netdev->stats.rx_syncp);
>> @@ -306,6 +336,9 @@ static int mhi_net_newlink(struct mhi_device *mhi_dev, struct net_device *ndev)
>>          struct mhi_net_dev *mhi_netdev;
>>          int err;
>>
>> +       if (!!ndev->header_ops)
> 
> The double negation does nothing useful here, you don't need it.

OK sure , will correct it

>> +               eth_hw_addr_random(ndev);
>> +
>>          mhi_netdev = netdev_priv(ndev);
>>
>>          dev_set_drvdata(&mhi_dev->dev, mhi_netdev);
>> @@ -356,7 +389,8 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
>>          int err;
>>
>>          ndev = alloc_netdev(sizeof(struct mhi_net_dev), info->netname,
>> -                           NET_NAME_PREDICTABLE, mhi_net_setup);
>> +                           NET_NAME_ENUM, info->ethernet_if ?
>> +                           mhi_ethernet_setup : mhi_net_setup);
>>          if (!ndev)
>>                  return -ENOMEM;
>>
>> @@ -386,11 +420,18 @@ static const struct mhi_device_info mhi_swip0 = {
>>          .netname = "mhi_swip%d",
>>   };
>>
>> +static const struct mhi_device_info mhi_eth0 = {
>> +       .netname = "eth%d",
> 
> We named other interfaces `mhi_hwipX/mhi_swipX`, would it make sense
> to have similar naming here as well?
> 
Thanks for suggestion.
Incorporated the recommendation to use eth%d for Ethernet interface naming.
https://lore.kernel.org/all/20250725171053.03c0c44d@kernel.org/

>> +       .ethernet_if = true,
>> +};
>> +
>>   static const struct mhi_device_id mhi_net_id_table[] = {
>>          /* Hardware accelerated data PATH (to modem IPA), protocol agnostic */
>>          { .chan = "IP_HW0", .driver_data = (kernel_ulong_t)&mhi_hwip0 },
>>          /* Software data PATH (to modem CPU) */
>>          { .chan = "IP_SW0", .driver_data = (kernel_ulong_t)&mhi_swip0 },
>> +       { .chan = "IP_ETH0", .driver_data = (kernel_ulong_t)&mhi_eth0 },
>> +       { .chan = "IP_ETH1", .driver_data = (kernel_ulong_t)&mhi_eth0 },
>>          {}
>>   };
>>   MODULE_DEVICE_TABLE(mhi, mhi_net_id_table);
>>
>> --
>> 2.34.1
>>
>>