| Message ID | 20220223223326.28021-7-ricardo.martinez@linux.intel.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: wwan: t7xx: PCIe driver for MediaTek M.2 modem | expand |
On Wed, 23 Feb 2022, Ricardo Martinez wrote: > From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com> > > Adds AT and MBIM ports to the port proxy infrastructure. > The initialization method is responsible for creating the corresponding > ports using the WWAN framework infrastructure. The implemented WWAN port > operations are start, stop, and TX. > > Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com> > Co-developed-by: Ricardo Martinez <ricardo.martinez@linux.intel.com> > Signed-off-by: Ricardo Martinez <ricardo.martinez@linux.intel.com> > > >From a WWAN framework perspective: > Reviewed-by: Loic Poulain <loic.poulain@linaro.org> > --- > +static int t7xx_port_ctrl_tx(struct wwan_port *port, struct sk_buff *skb) > +{ ... > + if (!len || !port_private->rx_length_th || !port_private->chan_enable) It raises eyebrows to see rx_xx being used on something called "tx". Is it trying to tests port not inited?
On Thu, Feb 24, 2022 at 1:35 AM Ricardo Martinez <ricardo.martinez@linux.intel.com> wrote: > From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com> > > Adds AT and MBIM ports to the port proxy infrastructure. > The initialization method is responsible for creating the corresponding > ports using the WWAN framework infrastructure. The implemented WWAN port > operations are start, stop, and TX. [skipped] > +static int t7xx_port_ctrl_tx(struct wwan_port *port, struct sk_buff *skb) > +{ > + struct t7xx_port *port_private = wwan_port_get_drvdata(port); > + size_t actual_len, alloc_size, txq_mtu = CLDMA_MTU; > + struct t7xx_port_static *port_static; > + unsigned int len, i, packets; > + struct t7xx_fsm_ctl *ctl; > + enum md_state md_state; > + > + len = skb->len; > + if (!len || !port_private->rx_length_th || !port_private->chan_enable) > + return -EINVAL; > + > + port_static = port_private->port_static; > + ctl = port_private->t7xx_dev->md->fsm_ctl; > + md_state = t7xx_fsm_get_md_state(ctl); > + if (md_state == MD_STATE_WAITING_FOR_HS1 || md_state == MD_STATE_WAITING_FOR_HS2) { > + dev_warn(port_private->dev, "Cannot write to %s port when md_state=%d\n", > + port_static->name, md_state); > + return -ENODEV; > + } > + > + alloc_size = min_t(size_t, txq_mtu, len + CCCI_HEADROOM); > + actual_len = alloc_size - CCCI_HEADROOM; > + packets = DIV_ROUND_UP(len, txq_mtu - CCCI_HEADROOM); > + > + for (i = 0; i < packets; i++) { > + struct ccci_header *ccci_h; > + struct sk_buff *skb_ccci; > + int ret; > + > + if (packets > 1 && packets == i + 1) { > + actual_len = len % (txq_mtu - CCCI_HEADROOM); > + alloc_size = actual_len + CCCI_HEADROOM; > + } Why do you track the packet number? Why not track the offset in the passed data? E.g.: for (off = 0; off < len; off += chunklen) { chunklen = min(len - off, CLDMA_MTU - sizeof(struct ccci_header); skb_ccci = alloc_skb(chunklen + sizeof(struct ccci_header), ...); skb_put_data(skb_ccci, skb->data + off, chunklen); /* Send skb_ccci */ } > + skb_ccci = __dev_alloc_skb(alloc_size, GFP_KERNEL); > + if (!skb_ccci) > + return -ENOMEM; > + > + ccci_h = skb_put(skb_ccci, sizeof(*ccci_h)); > + t7xx_ccci_header_init(ccci_h, 0, actual_len + sizeof(*ccci_h), > + port_static->tx_ch, 0); > + skb_put_data(skb_ccci, skb->data + i * (txq_mtu - CCCI_HEADROOM), actual_len); > + t7xx_port_proxy_set_tx_seq_num(port_private, ccci_h); > + > + ret = t7xx_port_send_skb_to_md(port_private, skb_ccci); > + if (ret) { > + dev_kfree_skb_any(skb_ccci); > + dev_err(port_private->dev, "Write error on %s port, %d\n", > + port_static->name, ret); > + return ret; > + } > + > + port_private->seq_nums[MTK_TX]++; Sequence number tracking as well as CCCI header construction are common operations, so why not move them to t7xx_port_send_skb_to_md()? > + } > + > + dev_kfree_skb(skb); > + return 0; > +} -- Sergey
On 3/6/2022 6:56 PM, Sergey Ryazanov wrote: > On Thu, Feb 24, 2022 at 1:35 AM Ricardo Martinez > <ricardo.martinez@linux.intel.com> wrote: >> From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com> >> >> Adds AT and MBIM ports to the port proxy infrastructure. >> The initialization method is responsible for creating the corresponding >> ports using the WWAN framework infrastructure. The implemented WWAN port >> operations are start, stop, and TX. > [skipped] > >> +static int t7xx_port_ctrl_tx(struct wwan_port *port, struct sk_buff *skb) >> +{ >> + struct t7xx_port *port_private = wwan_port_get_drvdata(port); >> + size_t actual_len, alloc_size, txq_mtu = CLDMA_MTU; >> + struct t7xx_port_static *port_static; >> + unsigned int len, i, packets; >> + struct t7xx_fsm_ctl *ctl; >> + enum md_state md_state; >> + >> + len = skb->len; >> + if (!len || !port_private->rx_length_th || !port_private->chan_enable) >> + return -EINVAL; >> + >> + port_static = port_private->port_static; >> + ctl = port_private->t7xx_dev->md->fsm_ctl; >> + md_state = t7xx_fsm_get_md_state(ctl); >> + if (md_state == MD_STATE_WAITING_FOR_HS1 || md_state == MD_STATE_WAITING_FOR_HS2) { >> + dev_warn(port_private->dev, "Cannot write to %s port when md_state=%d\n", >> + port_static->name, md_state); >> + return -ENODEV; >> + } >> + >> + alloc_size = min_t(size_t, txq_mtu, len + CCCI_HEADROOM); >> + actual_len = alloc_size - CCCI_HEADROOM; >> + packets = DIV_ROUND_UP(len, txq_mtu - CCCI_HEADROOM); >> + >> + for (i = 0; i < packets; i++) { >> + struct ccci_header *ccci_h; >> + struct sk_buff *skb_ccci; >> + int ret; >> + >> + if (packets > 1 && packets == i + 1) { >> + actual_len = len % (txq_mtu - CCCI_HEADROOM); >> + alloc_size = actual_len + CCCI_HEADROOM; >> + } > Why do you track the packet number? Why not track the offset in the > passed data? E.g.: > > for (off = 0; off < len; off += chunklen) { > chunklen = min(len - off, CLDMA_MTU - sizeof(struct ccci_header); > skb_ccci = alloc_skb(chunklen + sizeof(struct ccci_header), ...); > skb_put_data(skb_ccci, skb->data + off, chunklen); > /* Send skb_ccci */ > } Sure, I'll make that change. >> + skb_ccci = __dev_alloc_skb(alloc_size, GFP_KERNEL); >> + if (!skb_ccci) >> + return -ENOMEM; >> + >> + ccci_h = skb_put(skb_ccci, sizeof(*ccci_h)); >> + t7xx_ccci_header_init(ccci_h, 0, actual_len + sizeof(*ccci_h), >> + port_static->tx_ch, 0); >> + skb_put_data(skb_ccci, skb->data + i * (txq_mtu - CCCI_HEADROOM), actual_len); >> + t7xx_port_proxy_set_tx_seq_num(port_private, ccci_h); >> + >> + ret = t7xx_port_send_skb_to_md(port_private, skb_ccci); >> + if (ret) { >> + dev_kfree_skb_any(skb_ccci); >> + dev_err(port_private->dev, "Write error on %s port, %d\n", >> + port_static->name, ret); >> + return ret; >> + } >> + >> + port_private->seq_nums[MTK_TX]++; > Sequence number tracking as well as CCCI header construction are > common operations, so why not move them to t7xx_port_send_skb_to_md()? Sequence number should be set as part of CCCI header construction. I think it's a bit more readable to initialize the CCCI header right after the corresponding skb_put(). Not a big deal, any thoughts? Note that the upcoming fw update feature doesn't require a CCCI header, so we could rename the TX function as t7xx_port_send_ccci_skb_to_md(), this would give a hint that it is taking care of the CCCI header. >> + } >> + >> + dev_kfree_skb(skb); >> + return 0; >> +} > -- > Sergey
On Wed, Mar 9, 2022 at 3:02 AM Martinez, Ricardo <ricardo.martinez@linux.intel.com> wrote: > On 3/6/2022 6:56 PM, Sergey Ryazanov wrote: >> On Thu, Feb 24, 2022 at 1:35 AM Ricardo Martinez >> <ricardo.martinez@linux.intel.com> wrote: >>> From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com> >>> >>> Adds AT and MBIM ports to the port proxy infrastructure. >>> The initialization method is responsible for creating the corresponding >>> ports using the WWAN framework infrastructure. The implemented WWAN port >>> operations are start, stop, and TX. >> [skipped] >> >>> +static int t7xx_port_ctrl_tx(struct wwan_port *port, struct sk_buff *skb) >>> +{ >>> + struct t7xx_port *port_private = wwan_port_get_drvdata(port); >>> + size_t actual_len, alloc_size, txq_mtu = CLDMA_MTU; >>> + struct t7xx_port_static *port_static; >>> + unsigned int len, i, packets; >>> + struct t7xx_fsm_ctl *ctl; >>> + enum md_state md_state; >>> + >>> + len = skb->len; >>> + if (!len || !port_private->rx_length_th || !port_private->chan_enable) >>> + return -EINVAL; >>> + >>> + port_static = port_private->port_static; >>> + ctl = port_private->t7xx_dev->md->fsm_ctl; >>> + md_state = t7xx_fsm_get_md_state(ctl); >>> + if (md_state == MD_STATE_WAITING_FOR_HS1 || md_state == MD_STATE_WAITING_FOR_HS2) { >>> + dev_warn(port_private->dev, "Cannot write to %s port when md_state=%d\n", >>> + port_static->name, md_state); >>> + return -ENODEV; >>> + } >>> + >>> + alloc_size = min_t(size_t, txq_mtu, len + CCCI_HEADROOM); >>> + actual_len = alloc_size - CCCI_HEADROOM; >>> + packets = DIV_ROUND_UP(len, txq_mtu - CCCI_HEADROOM); >>> + >>> + for (i = 0; i < packets; i++) { >>> + struct ccci_header *ccci_h; >>> + struct sk_buff *skb_ccci; >>> + int ret; >>> + >>> + if (packets > 1 && packets == i + 1) { >>> + actual_len = len % (txq_mtu - CCCI_HEADROOM); >>> + alloc_size = actual_len + CCCI_HEADROOM; >>> + } >> >> Why do you track the packet number? Why not track the offset in the >> passed data? E.g.: >> >> for (off = 0; off < len; off += chunklen) { >> chunklen = min(len - off, CLDMA_MTU - sizeof(struct ccci_header); >> skb_ccci = alloc_skb(chunklen + sizeof(struct ccci_header), ...); >> skb_put_data(skb_ccci, skb->data + off, chunklen); >> /* Send skb_ccci */ >> } > > Sure, I'll make that change. > >>> + skb_ccci = __dev_alloc_skb(alloc_size, GFP_KERNEL); >>> + if (!skb_ccci) >>> + return -ENOMEM; >>> + >>> + ccci_h = skb_put(skb_ccci, sizeof(*ccci_h)); >>> + t7xx_ccci_header_init(ccci_h, 0, actual_len + sizeof(*ccci_h), >>> + port_static->tx_ch, 0); >>> + skb_put_data(skb_ccci, skb->data + i * (txq_mtu - CCCI_HEADROOM), actual_len); >>> + t7xx_port_proxy_set_tx_seq_num(port_private, ccci_h); >>> + >>> + ret = t7xx_port_send_skb_to_md(port_private, skb_ccci); >>> + if (ret) { >>> + dev_kfree_skb_any(skb_ccci); >>> + dev_err(port_private->dev, "Write error on %s port, %d\n", >>> + port_static->name, ret); >>> + return ret; >>> + } >>> + >>> + port_private->seq_nums[MTK_TX]++; >> >> Sequence number tracking as well as CCCI header construction are >> common operations, so why not move them to t7xx_port_send_skb_to_md()? > > Sequence number should be set as part of CCCI header construction. > > I think it's a bit more readable to initialize the CCCI header right > after the corresponding skb_put(). Not a big deal, any thoughts? I do not _think_ creating the CCCI header in the WWAN or CTRL port functions is any good idea. In case of stacked protocols, each layer should create its own header, pass the packet down the stack, and then a next layer will create a next header. In case of the CTRL port, this means that the control port code should take an opaque data block from an upper layer (e.g. features request), prepend it with a control msg header, and pass it down the stack to the port proxy layer, where the CCCI header will be prepended. In case a WWAN port, all headers are passed from user space, so there шы nothing to prepend. And the only remaining function is to fragment a user input, and then pass all the fragments to the port proxy layer, where the CCCI header will be prepended. This way, you do not overload the CTRL/WWAN port with code of other protocols (i.e. CCCI), reduce code duplication. Which in itself improves the code maintainability and future development. Creating a CCCI header at the WWAN port layer is like forcing a user to manually create IP and UDP headers before writing a data block into a network socket :) Anyway, it is up to you to decide exactly how to create headers and assign sequence numbers. I just wanted to point out the code inconsistency. It does not make the code wrong, it just makes the code look stranger. > Note that the upcoming fw update feature doesn't require a CCCI header, > so we could rename the TX function as t7xx_port_send_ccci_skb_to_md(), > this would give a hint that it is taking care of the CCCI header. Does this mean the firmware upgrade does not utilize the channel id, and just pushes data directly to a specific CLDMA queue? In that case it looks like the firmware upgrade code needs to entirely bypass the port proxy layer and communicate directly with CLDMA. Isn't it? >>> + } >>> + >>> + dev_kfree_skb(skb); >>> + return 0; >>> +}
On 3/9/2022 4:13 PM, Sergey Ryazanov wrote: > On Wed, Mar 9, 2022 at 3:02 AM Martinez, Ricardo > <ricardo.martinez@linux.intel.com> wrote: >> On 3/6/2022 6:56 PM, Sergey Ryazanov wrote: >>> On Thu, Feb 24, 2022 at 1:35 AM Ricardo Martinez >>> <ricardo.martinez@linux.intel.com> wrote: >>>> From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com> >>>> >>>> Adds AT and MBIM ports to the port proxy infrastructure. >>>> The initialization method is responsible for creating the corresponding >>>> ports using the WWAN framework infrastructure. The implemented WWAN port >>>> operations are start, stop, and TX. >>> [skipped] >>> >>>> +static int t7xx_port_ctrl_tx(struct wwan_port *port, struct sk_buff *skb) >>>> +{ >>>> + struct t7xx_port *port_private = wwan_port_get_drvdata(port); >>>> + size_t actual_len, alloc_size, txq_mtu = CLDMA_MTU; >>>> + struct t7xx_port_static *port_static; >>>> + unsigned int len, i, packets; >>>> + struct t7xx_fsm_ctl *ctl; >>>> + enum md_state md_state; >>>> + >>>> + len = skb->len; >>>> + if (!len || !port_private->rx_length_th || !port_private->chan_enable) >>>> + return -EINVAL; >>>> + >>>> + port_static = port_private->port_static; >>>> + ctl = port_private->t7xx_dev->md->fsm_ctl; >>>> + md_state = t7xx_fsm_get_md_state(ctl); >>>> + if (md_state == MD_STATE_WAITING_FOR_HS1 || md_state == MD_STATE_WAITING_FOR_HS2) { >>>> + dev_warn(port_private->dev, "Cannot write to %s port when md_state=%d\n", >>>> + port_static->name, md_state); >>>> + return -ENODEV; >>>> + } >>>> + >>>> + alloc_size = min_t(size_t, txq_mtu, len + CCCI_HEADROOM); >>>> + actual_len = alloc_size - CCCI_HEADROOM; >>>> + packets = DIV_ROUND_UP(len, txq_mtu - CCCI_HEADROOM); >>>> + >>>> + for (i = 0; i < packets; i++) { >>>> + struct ccci_header *ccci_h; >>>> + struct sk_buff *skb_ccci; >>>> + int ret; >>>> + >>>> + if (packets > 1 && packets == i + 1) { >>>> + actual_len = len % (txq_mtu - CCCI_HEADROOM); >>>> + alloc_size = actual_len + CCCI_HEADROOM; >>>> + } >>> Why do you track the packet number? Why not track the offset in the >>> passed data? E.g.: >>> >>> for (off = 0; off < len; off += chunklen) { >>> chunklen = min(len - off, CLDMA_MTU - sizeof(struct ccci_header); >>> skb_ccci = alloc_skb(chunklen + sizeof(struct ccci_header), ...); >>> skb_put_data(skb_ccci, skb->data + off, chunklen); >>> /* Send skb_ccci */ >>> } >> Sure, I'll make that change. >> >>>> + skb_ccci = __dev_alloc_skb(alloc_size, GFP_KERNEL); >>>> + if (!skb_ccci) >>>> + return -ENOMEM; >>>> + >>>> + ccci_h = skb_put(skb_ccci, sizeof(*ccci_h)); >>>> + t7xx_ccci_header_init(ccci_h, 0, actual_len + sizeof(*ccci_h), >>>> + port_static->tx_ch, 0); >>>> + skb_put_data(skb_ccci, skb->data + i * (txq_mtu - CCCI_HEADROOM), actual_len); >>>> + t7xx_port_proxy_set_tx_seq_num(port_private, ccci_h); >>>> + >>>> + ret = t7xx_port_send_skb_to_md(port_private, skb_ccci); >>>> + if (ret) { >>>> + dev_kfree_skb_any(skb_ccci); >>>> + dev_err(port_private->dev, "Write error on %s port, %d\n", >>>> + port_static->name, ret); >>>> + return ret; >>>> + } >>>> + >>>> + port_private->seq_nums[MTK_TX]++; >>> Sequence number tracking as well as CCCI header construction are >>> common operations, so why not move them to t7xx_port_send_skb_to_md()? >> Sequence number should be set as part of CCCI header construction. >> >> I think it's a bit more readable to initialize the CCCI header right >> after the corresponding skb_put(). Not a big deal, any thoughts? > I do not _think_ creating the CCCI header in the WWAN or CTRL port > functions is any good idea. In case of stacked protocols, each layer > should create its own header, pass the packet down the stack, and then > a next layer will create a next header. > > In case of the CTRL port, this means that the control port code should > take an opaque data block from an upper layer (e.g. features request), > prepend it with a control msg header, and pass it down the stack to > the port proxy layer, where the CCCI header will be prepended. > > In case a WWAN port, all headers are passed from user space, so there > шы nothing to prepend. And the only remaining function is to fragment > a user input, and then pass all the fragments to the port proxy > layer, where the CCCI header will be prepended. > > This way, you do not overload the CTRL/WWAN port with code of other > protocols (i.e. CCCI), reduce code duplication. Which in itself > improves the code maintainability and future development. Creating a > CCCI header at the WWAN port layer is like forcing a user to manually > create IP and UDP headers before writing a data block into a network > socket :) > > Anyway, it is up to you to decide exactly how to create headers and > assign sequence numbers. I just wanted to point out the code > inconsistency. It does not make the code wrong, it just makes the code > look stranger. Agree, the next iteration will implement a layered approach. >> Note that the upcoming fw update feature doesn't require a CCCI header, >> so we could rename the TX function as t7xx_port_send_ccci_skb_to_md(), >> this would give a hint that it is taking care of the CCCI header. > Does this mean the firmware upgrade does not utilize the channel id, > and just pushes data directly to a specific CLDMA queue? In that case > it looks like the firmware upgrade code needs to entirely bypass the > port proxy layer and communicate directly with CLDMA. Isn't it? It could bypass port proxy, or it could use a new helper function implemented for the layered approach, this function (t7xx_port_send_raw_skb) sends an skb to the right CLDMA instance and queue based on the port configuration. > >>>> + } >>>> + >>>> + dev_kfree_skb(skb); >>>> + return 0; >>>> +}
diff --git a/drivers/net/wwan/t7xx/Makefile b/drivers/net/wwan/t7xx/Makefile index 63e1c67b82b5..9eec2e2472fb 100644 --- a/drivers/net/wwan/t7xx/Makefile +++ b/drivers/net/wwan/t7xx/Makefile @@ -12,3 +12,4 @@ mtk_t7xx-y:= t7xx_pci.o \ t7xx_hif_cldma.o \ t7xx_port_proxy.o \ t7xx_port_ctrl_msg.o \ + t7xx_port_wwan.o \ diff --git a/drivers/net/wwan/t7xx/t7xx_port_proxy.c b/drivers/net/wwan/t7xx/t7xx_port_proxy.c index 256442a60cc2..9a5cc64904d3 100644 --- a/drivers/net/wwan/t7xx/t7xx_port_proxy.c +++ b/drivers/net/wwan/t7xx/t7xx_port_proxy.c @@ -51,6 +51,30 @@ static struct t7xx_port_static t7xx_md_ports[] = { { + .tx_ch = PORT_CH_UART2_TX, + .rx_ch = PORT_CH_UART2_RX, + .txq_index = Q_IDX_AT_CMD, + .rxq_index = Q_IDX_AT_CMD, + .txq_exp_index = 0xff, + .rxq_exp_index = 0xff, + .path_id = CLDMA_ID_MD, + .flags = 0, + .ops = &wwan_sub_port_ops, + .name = "AT", + .port_type = WWAN_PORT_AT, + }, { + .tx_ch = PORT_CH_MBIM_TX, + .rx_ch = PORT_CH_MBIM_RX, + .txq_index = Q_IDX_MBIM, + .rxq_index = Q_IDX_MBIM, + .txq_exp_index = 0, + .rxq_exp_index = 0, + .path_id = CLDMA_ID_MD, + .flags = 0, + .ops = &wwan_sub_port_ops, + .name = "MBIM", + .port_type = WWAN_PORT_MBIM, + }, { .tx_ch = PORT_CH_CONTROL_TX, .rx_ch = PORT_CH_CONTROL_RX, .txq_index = Q_IDX_CTRL, diff --git a/drivers/net/wwan/t7xx/t7xx_port_proxy.h b/drivers/net/wwan/t7xx/t7xx_port_proxy.h index b23750f78d55..1c9608987728 100644 --- a/drivers/net/wwan/t7xx/t7xx_port_proxy.h +++ b/drivers/net/wwan/t7xx/t7xx_port_proxy.h @@ -80,6 +80,7 @@ struct port_msg { #define PORT_ENUM_VER_MISMATCH 0x00657272 /* Port operations mapping */ +extern struct port_ops wwan_sub_port_ops; extern struct port_ops ctl_port_ops; int t7xx_port_proxy_send_skb(struct t7xx_port *port, struct sk_buff *skb); diff --git a/drivers/net/wwan/t7xx/t7xx_port_wwan.c b/drivers/net/wwan/t7xx/t7xx_port_wwan.c new file mode 100644 index 000000000000..ac9144021431 --- /dev/null +++ b/drivers/net/wwan/t7xx/t7xx_port_wwan.c @@ -0,0 +1,210 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2021, MediaTek Inc. + * Copyright (c) 2021-2022, Intel Corporation. + * + * Authors: + * Amir Hanania <amir.hanania@intel.com> + * Chandrashekar Devegowda <chandrashekar.devegowda@intel.com> + * Haijun Liu <haijun.liu@mediatek.com> + * Moises Veleta <moises.veleta@intel.com> + * Ricardo Martinez<ricardo.martinez@linux.intel.com> + * + * Contributors: + * Andy Shevchenko <andriy.shevchenko@linux.intel.com> + * Chiranjeevi Rapolu <chiranjeevi.rapolu@intel.com> + * Eliot Lee <eliot.lee@intel.com> + * Sreehari Kancharla <sreehari.kancharla@intel.com> + */ + +#include <linux/atomic.h> +#include <linux/bitfield.h> +#include <linux/dev_printk.h> +#include <linux/err.h> +#include <linux/gfp.h> +#include <linux/minmax.h> +#include <linux/netdevice.h> +#include <linux/skbuff.h> +#include <linux/spinlock.h> +#include <linux/string.h> +#include <linux/wwan.h> + +#include "t7xx_common.h" +#include "t7xx_port.h" +#include "t7xx_port_proxy.h" +#include "t7xx_state_monitor.h" + +#define CCCI_HEADROOM 128 + +static int t7xx_port_ctrl_start(struct wwan_port *port) +{ + struct t7xx_port *port_mtk = wwan_port_get_drvdata(port); + + if (atomic_read(&port_mtk->usage_cnt)) + return -EBUSY; + + atomic_inc(&port_mtk->usage_cnt); + return 0; +} + +static void t7xx_port_ctrl_stop(struct wwan_port *port) +{ + struct t7xx_port *port_mtk = wwan_port_get_drvdata(port); + + atomic_dec(&port_mtk->usage_cnt); +} + +static int t7xx_port_ctrl_tx(struct wwan_port *port, struct sk_buff *skb) +{ + struct t7xx_port *port_private = wwan_port_get_drvdata(port); + size_t actual_len, alloc_size, txq_mtu = CLDMA_MTU; + struct t7xx_port_static *port_static; + unsigned int len, i, packets; + struct t7xx_fsm_ctl *ctl; + enum md_state md_state; + + len = skb->len; + if (!len || !port_private->rx_length_th || !port_private->chan_enable) + return -EINVAL; + + port_static = port_private->port_static; + ctl = port_private->t7xx_dev->md->fsm_ctl; + md_state = t7xx_fsm_get_md_state(ctl); + if (md_state == MD_STATE_WAITING_FOR_HS1 || md_state == MD_STATE_WAITING_FOR_HS2) { + dev_warn(port_private->dev, "Cannot write to %s port when md_state=%d\n", + port_static->name, md_state); + return -ENODEV; + } + + alloc_size = min_t(size_t, txq_mtu, len + CCCI_HEADROOM); + actual_len = alloc_size - CCCI_HEADROOM; + packets = DIV_ROUND_UP(len, txq_mtu - CCCI_HEADROOM); + + for (i = 0; i < packets; i++) { + struct ccci_header *ccci_h; + struct sk_buff *skb_ccci; + int ret; + + if (packets > 1 && packets == i + 1) { + actual_len = len % (txq_mtu - CCCI_HEADROOM); + alloc_size = actual_len + CCCI_HEADROOM; + } + + skb_ccci = __dev_alloc_skb(alloc_size, GFP_KERNEL); + if (!skb_ccci) + return -ENOMEM; + + ccci_h = skb_put(skb_ccci, sizeof(*ccci_h)); + t7xx_ccci_header_init(ccci_h, 0, actual_len + sizeof(*ccci_h), + port_static->tx_ch, 0); + skb_put_data(skb_ccci, skb->data + i * (txq_mtu - CCCI_HEADROOM), actual_len); + t7xx_port_proxy_set_tx_seq_num(port_private, ccci_h); + + ret = t7xx_port_send_skb_to_md(port_private, skb_ccci); + if (ret) { + dev_kfree_skb_any(skb_ccci); + dev_err(port_private->dev, "Write error on %s port, %d\n", + port_static->name, ret); + return ret; + } + + port_private->seq_nums[MTK_TX]++; + } + + dev_kfree_skb(skb); + return 0; +} + +static const struct wwan_port_ops wwan_ops = { + .start = t7xx_port_ctrl_start, + .stop = t7xx_port_ctrl_stop, + .tx = t7xx_port_ctrl_tx, +}; + +static int t7xx_port_wwan_init(struct t7xx_port *port) +{ + struct t7xx_port_static *port_static = port->port_static; + + port->rx_length_th = RX_QUEUE_MAXLEN; + port->flags |= PORT_F_RX_ADJUST_HEADER; + + if (port_static->rx_ch == PORT_CH_UART2_RX) + port->flags |= PORT_F_RX_CH_TRAFFIC; + + if (!port->chan_enable) + port->flags |= PORT_F_RX_ALLOW_DROP; + + return 0; +} + +static void t7xx_port_wwan_uninit(struct t7xx_port *port) +{ + unsigned long flags; + + if (!port->wwan_port) + return; + + spin_lock_irqsave(&port->rx_wq.lock, flags); + port->rx_length_th = 0; + wwan_remove_port(port->wwan_port); + port->wwan_port = NULL; + spin_unlock_irqrestore(&port->rx_wq.lock, flags); +} + +static int t7xx_port_wwan_recv_skb(struct t7xx_port *port, struct sk_buff *skb) +{ + struct t7xx_port_static *port_static = port->port_static; + + if (!atomic_read(&port->usage_cnt)) { + dev_kfree_skb_any(skb); + dev_err_ratelimited(port->dev, "Port %s is not opened, drop packets\n", + port_static->name); + return 0; + } + + return t7xx_port_recv_skb(port, skb); +} + +static int t7xx_port_wwan_enable_chl(struct t7xx_port *port) +{ + spin_lock(&port->port_update_lock); + port->chan_enable = true; + port->flags &= ~PORT_F_RX_ALLOW_DROP; + spin_unlock(&port->port_update_lock); + + return 0; +} + +static int t7xx_port_wwan_disable_chl(struct t7xx_port *port) +{ + spin_lock(&port->port_update_lock); + port->chan_enable = false; + port->flags |= PORT_F_RX_ALLOW_DROP; + spin_unlock(&port->port_update_lock); + + return 0; +} + +static void t7xx_port_wwan_md_state_notify(struct t7xx_port *port, unsigned int state) +{ + struct t7xx_port_static *port_static = port->port_static; + + if (state != MD_STATE_READY) + return; + + if (!port->wwan_port) { + port->wwan_port = wwan_create_port(port->dev, port_static->port_type, + &wwan_ops, port); + if (IS_ERR(port->wwan_port)) + dev_err(port->dev, "Unable to create WWWAN port %s", port_static->name); + } +} + +struct port_ops wwan_sub_port_ops = { + .init = t7xx_port_wwan_init, + .recv_skb = t7xx_port_wwan_recv_skb, + .uninit = t7xx_port_wwan_uninit, + .enable_chl = t7xx_port_wwan_enable_chl, + .disable_chl = t7xx_port_wwan_disable_chl, + .md_state_notify = t7xx_port_wwan_md_state_notify, +};