diff mbox series

[V2] net: mhi : Add support to enable ethernet interface

Message ID 1689660928-12092-1-git-send-email-quic_vpernami@quicinc.com (mailing list archive)
State Superseded
Headers show
Series [V2] net: mhi : Add support to enable ethernet interface | expand

Commit Message

Vivek Pernamitta July 18, 2023, 6:15 a.m. UTC 
Add support to enable ethernet interface for MHI SWIP channels.

Signed-off-by: Vivek Pernamitta <quic_vpernami@quicinc.com>
Reviewed-by: Daniele Palmas <dnlplm@gmail.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
---

changes since v1:
	- Moved to net-next from linux-next	
	- moved to eth_hw_addr_random() to assign Ethernet MAC address
	  from eth_random_addr()
---
 drivers/net/mhi_net.c | 53 ++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 13 deletions(-)

Comments

Manivannan Sadhasivam July 18, 2023, 12:43 p.m. UTC | #1 
On Tue, Jul 18, 2023 at 11:45:28AM +0530, Vivek Pernamitta wrote:
> Add support to enable ethernet interface for MHI SWIP channels.
> 

Please add more info in the commit message i.e., why this interface is added and
how it is going to benefit the users etc..

Since you are modifying the existing mhi_swip interface, this isn't an ABI
change?

> Signed-off-by: Vivek Pernamitta <quic_vpernami@quicinc.com>
> Reviewed-by: Daniele Palmas <dnlplm@gmail.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> ---
> 
> changes since v1:
> 	- Moved to net-next from linux-next	
> 	- moved to eth_hw_addr_random() to assign Ethernet MAC address
> 	  from eth_random_addr()
> ---
>  drivers/net/mhi_net.c | 53 ++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 40 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/mhi_net.c b/drivers/net/mhi_net.c
> index 3d322ac..5bb8d99 100644
> --- a/drivers/net/mhi_net.c
> +++ b/drivers/net/mhi_net.c

[...]

> @@ -380,10 +405,12 @@ static void mhi_net_remove(struct mhi_device *mhi_dev)
>  
>  static const struct mhi_device_info mhi_hwip0 = {
>  	.netname = "mhi_hwip%d",
> +	.ethernet_if = false,
>  };
>  
>  static const struct mhi_device_info mhi_swip0 = {
>  	.netname = "mhi_swip%d",
> +	.ethernet_if = false,

false?

- Mani

>  };
>  
>  static const struct mhi_device_id mhi_net_id_table[] = {
> -- 
> 2.7.4
>
Bjorn Andersson July 18, 2023, 3:35 p.m. UTC | #2 
On Tue, Jul 18, 2023 at 11:45:28AM +0530, Vivek Pernamitta wrote:

Please drop the extra ' ' after "mhi" in $subject as well.

Thanks,
Bjorn
Vivek Pernamitta July 19, 2023, 10:38 a.m. UTC | #3 
Currently MHI NET driver does not support Ethernet interface,

we are adding Ethernet interface support to MHI NET driver, so new 
client can be configured to be Ethernet type over MHI by setting 
"mhi_device_info.ethernet_if = true"

currently we are not changing mhi_swip0/mhi_hwip0 to Ethernet. we are 
adding an provision to configure mhi client as  Ethernet type.


On 7/18/2023 6:13 PM, Manivannan Sadhasivam wrote:
> On Tue, Jul 18, 2023 at 11:45:28AM +0530, Vivek Pernamitta wrote:
>> Add support to enable ethernet interface for MHI SWIP channels.
>>
> Please add more info in the commit message i.e., why this interface is added and
> how it is going to benefit the users etc..
>
> Since you are modifying the existing mhi_swip interface, this isn't an ABI
> change?
>
>> Signed-off-by: Vivek Pernamitta <quic_vpernami@quicinc.com>
>> Reviewed-by: Daniele Palmas <dnlplm@gmail.com>
>> Reviewed-by: Simon Horman <simon.horman@corigine.com>
>> ---
>>
>> changes since v1:
>> 	- Moved to net-next from linux-next	
>> 	- moved to eth_hw_addr_random() to assign Ethernet MAC address
>> 	  from eth_random_addr()
>> ---
>>   drivers/net/mhi_net.c | 53 ++++++++++++++++++++++++++++++++++++++-------------
>>   1 file changed, 40 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/mhi_net.c b/drivers/net/mhi_net.c
>> index 3d322ac..5bb8d99 100644
>> --- a/drivers/net/mhi_net.c
>> +++ b/drivers/net/mhi_net.c
> [...]
>
>> @@ -380,10 +405,12 @@ static void mhi_net_remove(struct mhi_device *mhi_dev)
>>   
>>   static const struct mhi_device_info mhi_hwip0 = {
>>   	.netname = "mhi_hwip%d",
>> +	.ethernet_if = false,
>>   };
>>   
>>   static const struct mhi_device_info mhi_swip0 = {
>>   	.netname = "mhi_swip%d",
>> +	.ethernet_if = false,
> false?
yes ,it is false as by default mhi_swip/mhi_hwip interface will be 
normal net device.
>
> - Mani
>
>>   };
>>   
>>   static const struct mhi_device_id mhi_net_id_table[] = {
>> -- 
>> 2.7.4
>>
Daniele Palmas July 19, 2023, 11:17 a.m. UTC | #4 
Hello Vivek,

Il giorno mer 19 lug 2023 alle ore 12:39 Vivek Pernamitta
<quic_vpernami@quicinc.com> ha scritto:
>
>
> Currently MHI NET driver does not support Ethernet interface,
>
> we are adding Ethernet interface support to MHI NET driver, so new
> client can be configured to be Ethernet type over MHI by setting
> "mhi_device_info.ethernet_if = true"
>

how can this be set to "true"?

Is this something that can be changed dynamically at runtime (but I
can't see how currently) or is it "static" ?

In the latter case, not sure if the maintainer agrees, but maybe a new
binding (e.g. something like ETH_SW0) should be used ?

Thanks,
Daniele

> currently we are not changing mhi_swip0/mhi_hwip0 to Ethernet. we are
> adding an provision to configure mhi client as  Ethernet type.
>
>
> On 7/18/2023 6:13 PM, Manivannan Sadhasivam wrote:
> > On Tue, Jul 18, 2023 at 11:45:28AM +0530, Vivek Pernamitta wrote:
> >> Add support to enable ethernet interface for MHI SWIP channels.
> >>
> > Please add more info in the commit message i.e., why this interface is added and
> > how it is going to benefit the users etc..
> >
> > Since you are modifying the existing mhi_swip interface, this isn't an ABI
> > change?
> >
> >> Signed-off-by: Vivek Pernamitta <quic_vpernami@quicinc.com>
> >> Reviewed-by: Daniele Palmas <dnlplm@gmail.com>
> >> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> >> ---
> >>
> >> changes since v1:
> >>      - Moved to net-next from linux-next
> >>      - moved to eth_hw_addr_random() to assign Ethernet MAC address
> >>        from eth_random_addr()
> >> ---
> >>   drivers/net/mhi_net.c | 53 ++++++++++++++++++++++++++++++++++++++-------------
> >>   1 file changed, 40 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/net/mhi_net.c b/drivers/net/mhi_net.c
> >> index 3d322ac..5bb8d99 100644
> >> --- a/drivers/net/mhi_net.c
> >> +++ b/drivers/net/mhi_net.c
> > [...]
> >
> >> @@ -380,10 +405,12 @@ static void mhi_net_remove(struct mhi_device *mhi_dev)
> >>
> >>   static const struct mhi_device_info mhi_hwip0 = {
> >>      .netname = "mhi_hwip%d",
> >> +    .ethernet_if = false,
> >>   };
> >>
> >>   static const struct mhi_device_info mhi_swip0 = {
> >>      .netname = "mhi_swip%d",
> >> +    .ethernet_if = false,
> > false?
> yes ,it is false as by default mhi_swip/mhi_hwip interface will be
> normal net device.
> >
> > - Mani
> >
> >>   };
> >>
> >>   static const struct mhi_device_id mhi_net_id_table[] = {
> >> --
> >> 2.7.4
> >>
>
Manivannan Sadhasivam July 19, 2023, 11:42 a.m. UTC | #5 
On Wed, Jul 19, 2023 at 04:08:37PM +0530, Vivek Pernamitta wrote:
> 
> Currently MHI NET driver does not support Ethernet interface,
> 

Please do not top post. Read: https://people.kernel.org/tglx/notes-about-netiquette-qw89

> we are adding Ethernet interface support to MHI NET driver, so new client
> can be configured to be Ethernet type over MHI by setting
> "mhi_device_info.ethernet_if = true"
> 

Which is the new client you are referring to?

> currently we are not changing mhi_swip0/mhi_hwip0 to Ethernet. we are adding
> an provision to configure mhi client as  Ethernet type.
> 

If there are no users of the said provision, then it should not be added now.
You can only add interfaces to kernel if there is atleast one user.

- Mani

> 
> On 7/18/2023 6:13 PM, Manivannan Sadhasivam wrote:
> > On Tue, Jul 18, 2023 at 11:45:28AM +0530, Vivek Pernamitta wrote:
> > > Add support to enable ethernet interface for MHI SWIP channels.
> > > 
> > Please add more info in the commit message i.e., why this interface is added and
> > how it is going to benefit the users etc..
> > 
> > Since you are modifying the existing mhi_swip interface, this isn't an ABI
> > change?
> > 
> > > Signed-off-by: Vivek Pernamitta <quic_vpernami@quicinc.com>
> > > Reviewed-by: Daniele Palmas <dnlplm@gmail.com>
> > > Reviewed-by: Simon Horman <simon.horman@corigine.com>
> > > ---
> > > 
> > > changes since v1:
> > > 	- Moved to net-next from linux-next	
> > > 	- moved to eth_hw_addr_random() to assign Ethernet MAC address
> > > 	  from eth_random_addr()
> > > ---
> > >   drivers/net/mhi_net.c | 53 ++++++++++++++++++++++++++++++++++++++-------------
> > >   1 file changed, 40 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/net/mhi_net.c b/drivers/net/mhi_net.c
> > > index 3d322ac..5bb8d99 100644
> > > --- a/drivers/net/mhi_net.c
> > > +++ b/drivers/net/mhi_net.c
> > [...]
> > 
> > > @@ -380,10 +405,12 @@ static void mhi_net_remove(struct mhi_device *mhi_dev)
> > >   static const struct mhi_device_info mhi_hwip0 = {
> > >   	.netname = "mhi_hwip%d",
> > > +	.ethernet_if = false,
> > >   };
> > >   static const struct mhi_device_info mhi_swip0 = {
> > >   	.netname = "mhi_swip%d",
> > > +	.ethernet_if = false,
> > false?
> yes ,it is false as by default mhi_swip/mhi_hwip interface will be normal
> net device.
> > 
> > - Mani
> > 
> > >   };
> > >   static const struct mhi_device_id mhi_net_id_table[] = {
> > > -- 
> > > 2.7.4
> > >
diff mbox series

Patch

diff --git a/drivers/net/mhi_net.c b/drivers/net/mhi_net.c
index 3d322ac..5bb8d99 100644
--- a/drivers/net/mhi_net.c
+++ b/drivers/net/mhi_net.c
@@ -11,6 +11,7 @@ 
 #include <linux/netdevice.h>
 #include <linux/skbuff.h>
 #include <linux/u64_stats_sync.h>
+#include <linux/etherdevice.h>
 
 #define MHI_NET_MIN_MTU		ETH_MIN_MTU
 #define MHI_NET_MAX_MTU		0xffff
@@ -38,10 +39,12 @@  struct mhi_net_dev {
 	u32 rx_queue_sz;
 	int msg_enable;
 	unsigned int mru;
+	bool ethernet_if;
 };
 
 struct mhi_device_info {
 	const char *netname;
+	bool ethernet_if;
 };
 
 static int mhi_ndo_open(struct net_device *ndev)
@@ -140,6 +143,14 @@  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->min_mtu = ETH_MIN_MTU;
+	ndev->max_mtu = ETH_MAX_MTU;
+}
+
 static struct sk_buff *mhi_net_skb_agg(struct mhi_net_dev *mhi_netdev,
 				       struct sk_buff *skb)
 {
@@ -209,16 +220,22 @@  static void mhi_net_dl_callback(struct mhi_device *mhi_dev,
 			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->ethernet_if) {
+			skb_copy_to_linear_data(skb, skb->data,
+						mhi_res->bytes_xferd);
+			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);
@@ -301,11 +318,17 @@  static void mhi_net_rx_refill_work(struct work_struct *work)
 		schedule_delayed_work(&mhi_netdev->rx_refill, HZ / 2);
 }
 
-static int mhi_net_newlink(struct mhi_device *mhi_dev, struct net_device *ndev)
+static int mhi_net_newlink(struct mhi_device *mhi_dev, struct net_device *ndev, bool eth_dev)
 {
 	struct mhi_net_dev *mhi_netdev;
 	int err;
 
+	if (eth_dev) {
+		eth_hw_addr_random(ndev);
+		if (!is_valid_ether_addr(ndev->dev_addr))
+			return -EADDRNOTAVAIL;
+	}
+
 	mhi_netdev = netdev_priv(ndev);
 
 	dev_set_drvdata(&mhi_dev->dev, mhi_netdev);
@@ -313,6 +336,7 @@  static int mhi_net_newlink(struct mhi_device *mhi_dev, struct net_device *ndev)
 	mhi_netdev->mdev = mhi_dev;
 	mhi_netdev->skbagg_head = NULL;
 	mhi_netdev->mru = mhi_dev->mhi_cntrl->mru;
+	mhi_netdev->ethernet_if = eth_dev;
 
 	INIT_DELAYED_WORK(&mhi_netdev->rx_refill, mhi_net_rx_refill_work);
 	u64_stats_init(&mhi_netdev->stats.rx_syncp);
@@ -356,13 +380,14 @@  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_PREDICTABLE, info->ethernet_if ?
+			    mhi_ethernet_setup : mhi_net_setup);
 	if (!ndev)
 		return -ENOMEM;
 
 	SET_NETDEV_DEV(ndev, &mhi_dev->dev);
 
-	err = mhi_net_newlink(mhi_dev, ndev);
+	err = mhi_net_newlink(mhi_dev, ndev, info->ethernet_if);
 	if (err) {
 		free_netdev(ndev);
 		return err;
@@ -380,10 +405,12 @@  static void mhi_net_remove(struct mhi_device *mhi_dev)
 
 static const struct mhi_device_info mhi_hwip0 = {
 	.netname = "mhi_hwip%d",
+	.ethernet_if = false,
 };
 
 static const struct mhi_device_info mhi_swip0 = {
 	.netname = "mhi_swip%d",
+	.ethernet_if = false,
 };
 
 static const struct mhi_device_id mhi_net_id_table[] = {