diff mbox series

[net-next,v2,1/3] net: mhi: Add RX/TX fixup callbacks

Message ID 1612213542-17257-1-git-send-email-loic.poulain@linaro.org
State New
Headers show
Series [net-next,v2,1/3] net: mhi: Add RX/TX fixup callbacks | expand

Commit Message

Loic Poulain Feb. 1, 2021, 9:05 p.m. UTC
MHI can transport different protocols, some are handled at upper level,
like IP and QMAP(rmnet/netlink), but others will need to be inside MHI
net driver, like mbim. This change adds support for protocol rx/tx
fixup callbacks registration, that can be used to encode/decode the
targeted protocol.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

---
 v2: mhi_net_dev as rx/tx_fixup parameter

 drivers/net/mhi_net.c | 70 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 58 insertions(+), 12 deletions(-)

-- 
2.7.4

Comments

Jakub Kicinski Feb. 3, 2021, 11:08 p.m. UTC | #1
Please put the maintainers or the list thru which you expect the patch
to be applied in the To: field of your emails.

On Mon,  1 Feb 2021 22:05:40 +0100 Loic Poulain wrote:
> +	if (proto && proto->tx_fixup) {

> +		skb = proto->tx_fixup(mhi_netdev, skb);

> +		if (unlikely(!skb))

> +			goto exit_drop;

> +	}


> @@ -170,7 +193,11 @@ static void mhi_net_dl_callback(struct mhi_device *mhi_dev,

>  		}

>  

>  		skb_put(skb, mhi_res->bytes_xferd);

> -		netif_rx(skb);

> +

> +		if (proto && proto->rx_fixup)

> +			proto->rx_fixup(mhi_netdev, skb);

> +		else

> +			netif_rx(skb);

>  	}


There us a slight asymmetry between tx_fixup and rx_fixup.
tx_fixup just massages the frame and then mhi_net still takes
care of transmission. On Rx side rx_fixup actually does the
netif_rx(skb). Maybe s/rx_fixup/rx/ ?
Loic Poulain Feb. 4, 2021, 7:54 a.m. UTC | #2
Hi Jakub,


On Thu, 4 Feb 2021 at 00:08, Jakub Kicinski <kuba@kernel.org> wrote:
>

> Please put the maintainers or the list thru which you expect the patch

> to be applied in the To: field of your emails.

>

> On Mon,  1 Feb 2021 22:05:40 +0100 Loic Poulain wrote:

> > +     if (proto && proto->tx_fixup) {

> > +             skb = proto->tx_fixup(mhi_netdev, skb);

> > +             if (unlikely(!skb))

> > +                     goto exit_drop;

> > +     }

>

> > @@ -170,7 +193,11 @@ static void mhi_net_dl_callback(struct mhi_device *mhi_dev,

> >               }

> >

> >               skb_put(skb, mhi_res->bytes_xferd);

> > -             netif_rx(skb);

> > +

> > +             if (proto && proto->rx_fixup)

> > +                     proto->rx_fixup(mhi_netdev, skb);

> > +             else

> > +                     netif_rx(skb);

> >       }

>

> There us a slight asymmetry between tx_fixup and rx_fixup.

> tx_fixup just massages the frame and then mhi_net still takes

> care of transmission. On Rx side rx_fixup actually does the

> netif_rx(skb). Maybe s/rx_fixup/rx/ ?


Yes, that makes sense.

Loic
diff mbox series

Patch

diff --git a/drivers/net/mhi_net.c b/drivers/net/mhi_net.c
index 4f512531..34d4bcf 100644
--- a/drivers/net/mhi_net.c
+++ b/drivers/net/mhi_net.c
@@ -32,11 +32,24 @@  struct mhi_net_stats {
 struct mhi_net_dev {
 	struct mhi_device *mdev;
 	struct net_device *ndev;
+	const struct mhi_net_proto *proto;
+	void *proto_data;
 	struct delayed_work rx_refill;
 	struct mhi_net_stats stats;
 	u32 rx_queue_sz;
 };
 
+struct mhi_net_proto {
+	int (*init)(struct mhi_net_dev *mhi_netdev);
+	struct sk_buff * (*tx_fixup)(struct mhi_net_dev *mhi_netdev, struct sk_buff *skb);
+	int (*rx_fixup)(struct mhi_net_dev *mhi_netdev, struct sk_buff *skb);
+};
+
+struct mhi_device_info {
+	const char *netname;
+	const struct mhi_net_proto *proto;
+};
+
 static int mhi_ndo_open(struct net_device *ndev)
 {
 	struct mhi_net_dev *mhi_netdev = netdev_priv(ndev);
@@ -66,26 +79,35 @@  static int mhi_ndo_stop(struct net_device *ndev)
 static int mhi_ndo_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
 	struct mhi_net_dev *mhi_netdev = netdev_priv(ndev);
+	const struct mhi_net_proto *proto = mhi_netdev->proto;
 	struct mhi_device *mdev = mhi_netdev->mdev;
 	int err;
 
+	if (proto && proto->tx_fixup) {
+		skb = proto->tx_fixup(mhi_netdev, skb);
+		if (unlikely(!skb))
+			goto exit_drop;
+	}
+
 	err = mhi_queue_skb(mdev, DMA_TO_DEVICE, skb, skb->len, MHI_EOT);
 	if (unlikely(err)) {
 		net_err_ratelimited("%s: Failed to queue TX buf (%d)\n",
 				    ndev->name, err);
-
-		u64_stats_update_begin(&mhi_netdev->stats.tx_syncp);
-		u64_stats_inc(&mhi_netdev->stats.tx_dropped);
-		u64_stats_update_end(&mhi_netdev->stats.tx_syncp);
-
-		/* drop the packet */
 		dev_kfree_skb_any(skb);
+		goto exit_drop;
 	}
 
 	if (mhi_queue_is_full(mdev, DMA_TO_DEVICE))
 		netif_stop_queue(ndev);
 
 	return NETDEV_TX_OK;
+
+exit_drop:
+	u64_stats_update_begin(&mhi_netdev->stats.tx_syncp);
+	u64_stats_inc(&mhi_netdev->stats.tx_dropped);
+	u64_stats_update_end(&mhi_netdev->stats.tx_syncp);
+
+	return NETDEV_TX_OK;
 }
 
 static void mhi_ndo_get_stats64(struct net_device *ndev,
@@ -136,6 +158,7 @@  static void mhi_net_dl_callback(struct mhi_device *mhi_dev,
 				struct mhi_result *mhi_res)
 {
 	struct mhi_net_dev *mhi_netdev = dev_get_drvdata(&mhi_dev->dev);
+	const struct mhi_net_proto *proto = mhi_netdev->proto;
 	struct sk_buff *skb = mhi_res->buf_addr;
 	int free_desc_count;
 
@@ -170,7 +193,11 @@  static void mhi_net_dl_callback(struct mhi_device *mhi_dev,
 		}
 
 		skb_put(skb, mhi_res->bytes_xferd);
-		netif_rx(skb);
+
+		if (proto && proto->rx_fixup)
+			proto->rx_fixup(mhi_netdev, skb);
+		else
+			netif_rx(skb);
 	}
 
 	/* Refill if RX buffers queue becomes low */
@@ -252,14 +279,14 @@  static struct device_type wwan_type = {
 static int mhi_net_probe(struct mhi_device *mhi_dev,
 			 const struct mhi_device_id *id)
 {
-	const char *netname = (char *)id->driver_data;
+	const struct mhi_device_info *info = (struct mhi_device_info *)id->driver_data;
 	struct device *dev = &mhi_dev->dev;
 	struct mhi_net_dev *mhi_netdev;
 	struct net_device *ndev;
 	int err;
 
-	ndev = alloc_netdev(sizeof(*mhi_netdev), netname, NET_NAME_PREDICTABLE,
-			    mhi_net_setup);
+	ndev = alloc_netdev(sizeof(*mhi_netdev), info->netname,
+			    NET_NAME_PREDICTABLE, mhi_net_setup);
 	if (!ndev)
 		return -ENOMEM;
 
@@ -267,6 +294,7 @@  static int mhi_net_probe(struct mhi_device *mhi_dev,
 	dev_set_drvdata(dev, mhi_netdev);
 	mhi_netdev->ndev = ndev;
 	mhi_netdev->mdev = mhi_dev;
+	mhi_netdev->proto = info->proto;
 	SET_NETDEV_DEV(ndev, &mhi_dev->dev);
 	SET_NETDEV_DEVTYPE(ndev, &wwan_type);
 
@@ -286,8 +314,16 @@  static int mhi_net_probe(struct mhi_device *mhi_dev,
 	if (err)
 		goto out_err;
 
+	if (mhi_netdev->proto) {
+		err = mhi_netdev->proto->init(mhi_netdev);
+		if (err)
+			goto out_err_proto;
+	}
+
 	return 0;
 
+out_err_proto:
+	unregister_netdev(ndev);
 out_err:
 	free_netdev(ndev);
 	return err;
@@ -304,9 +340,19 @@  static void mhi_net_remove(struct mhi_device *mhi_dev)
 	free_netdev(mhi_netdev->ndev);
 }
 
+static const struct mhi_device_info mhi_hwip0 = {
+	.netname = "mhi_hwip%d",
+};
+
+static const struct mhi_device_info mhi_swip0 = {
+	.netname = "mhi_swip%d",
+};
+
 static const struct mhi_device_id mhi_net_id_table[] = {
-	{ .chan = "IP_HW0", .driver_data = (kernel_ulong_t)"mhi_hwip%d" },
-	{ .chan = "IP_SW0", .driver_data = (kernel_ulong_t)"mhi_swip%d" },
+	/* 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 },
 	{}
 };
 MODULE_DEVICE_TABLE(mhi, mhi_net_id_table);