| Message ID | MEYP282MB26978032980360EBBB1DAFF9BB752@MEYP282MB2697.AUSP282.PROD.OUTLOOK.COM (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: wwan: t7xx: Add fastboot interface | expand |
On Mon, 2024-01-22 at 17:09 +0800, Jinjian Song wrote: > From: Jinjian Song <jinjian.song@fibocom.com> > > On early detection of wwan device in fastboot mode, driver sets > up CLDMA0 HW tx/rx queues for raw data transfer and then create > fastboot port to userspace. > > Application can use this port to flash firmware and collect > core dump by fastboot protocol commands. > > Signed-off-by: Jinjian Song <jinjian.song@fibocom.com> > --- > v5: > * no change > v4: > * change function prefix to t7xx_port_fastboot > * change the name 'FASTBOOT' to fastboot in struct t7xx_early_port_conf > v3: > * no change > v2: > * no change Minor nit: you could/should omit the 'no change' entries [...] > +static int t7xx_port_fastboot_tx(struct wwan_port *port, struct sk_buff *skb) > +{ > + struct t7xx_port *port_private = wwan_port_get_drvdata(port); > + struct sk_buff *cur = skb, *cloned; > + size_t actual, len, offset = 0; > + int ret; > + int txq_mtu; > + > + if (!port_private->chan_enable) > + return -EINVAL; > + > + txq_mtu = t7xx_get_port_mtu(port_private); > + if (txq_mtu < 0) > + return -EINVAL; > + > + actual = cur->len; > + while (actual) { > + len = min_t(size_t, actual, txq_mtu); > + cloned = __dev_alloc_skb(len, GFP_KERNEL); Minor nit: the variable name is misleading, as the skb is not cloned. > + if (!cloned) > + return -ENOMEM; > + > + skb_put_data(cloned, cur->data + offset, len); > + > + ret = t7xx_port_send_raw_skb(port_private, cloned); > + if (ret) { > + dev_kfree_skb(cloned); > + dev_err(port_private->dev, "Write error on fastboot port, %d\n", ret); > + break; > + } > + offset += len; > + actual -= len; > + } > + > + dev_kfree_skb(skb); > + return 0; > +} > [...] > ++static void t7xx_port_fastboot_uninit(struct t7xx_port *port) > +{ > + if (!port->wwan.wwan_port) > + return; > + > + port->rx_length_th = 0; > + wwan_remove_port(port->wwan.wwan_port); > + port->wwan.wwan_port = NULL; > +} > + > +static int t7xx_port_fastboot_recv_skb(struct t7xx_port *port, struct sk_buff *skb) > +{ > + if (!atomic_read(&port->usage_cnt) || !port->chan_enable) { > + const struct t7xx_port_conf *port_conf = port->port_conf; > + > + dev_kfree_skb_any(skb); > + dev_err_ratelimited(port->dev, "Port %s is not opened, drop packets\n", > + port_conf->name); > + /* Dropping skb, caller should not access skb.*/ > + return 0; > + } > + > + wwan_port_rx(port->wwan.wwan_port, skb); > + > + return 0; > +} > + > +static int t7xx_port_fastboot_enable_chl(struct t7xx_port *port) > +{ > + spin_lock(&port->port_update_lock); > + port->chan_enable = true; > + spin_unlock(&port->port_update_lock); > + > + return 0; > +} > + > +static int t7xx_port_fastboot_disable_chl(struct t7xx_port *port) > +{ > + spin_lock(&port->port_update_lock); > + port->chan_enable = false; > + spin_unlock(&port->port_update_lock); > + > + return 0; > +} The above 4 functions implementation are exact copies of t7xx_port_wwan_*() functions in drivers/net/wwan/t7xx/t7xx_port_wwan.c Please reorganize the code to avoid such duplication, e.g. renaming the exiting function to something more generic, making them non static, and declaring them in some t7xx specific header. Cheers, Paolo
From: Paolo Abeni <pabeni@redhat.com> >On Mon, 2024-01-22 at 17:09 +0800, Jinjian Song wrote: >> From: Jinjian Song <jinjian.song@fibocom.com> >> >> On early detection of wwan device in fastboot mode, driver sets >> up CLDMA0 HW tx/rx queues for raw data transfer and then create >> fastboot port to userspace. >> >> Application can use this port to flash firmware and collect >> core dump by fastboot protocol commands. >> >> Signed-off-by: Jinjian Song <jinjian.song@fibocom.com> >> --- >> v5: >> * no change >> v4: >> * change function prefix to t7xx_port_fastboot >> * change the name 'FASTBOOT' to fastboot in struct t7xx_early_port_conf >> v3: >> * no change >> v2: >> * no change > >Minor nit: you could/should omit the 'no change' entries Let me delete the info. >[...] > > >> +static int t7xx_port_fastboot_tx(struct wwan_port *port, struct sk_buff >*skb) >> +{ >> + struct t7xx_port *port_private = wwan_port_get_drvdata(port); >> + struct sk_buff *cur = skb, *cloned; >> + size_t actual, len, offset = 0; >> + int ret; >> + int txq_mtu; >> + >> + if (!port_private->chan_enable) >> + return -EINVAL; >> + >> + txq_mtu = t7xx_get_port_mtu(port_private); >> + if (txq_mtu < 0) >> + return -EINVAL; >> + >> + actual = cur->len; >> + while (actual) { >> + len = min_t(size_t, actual, txq_mtu); >> + cloned = __dev_alloc_skb(len, GFP_KERNEL); > >Minor nit: the variable name is misleading, as the skb is not cloned. Let me rename it to tx_skb. >> + if (!cloned) >> + return -ENOMEM; >> + >> + skb_put_data(cloned, cur->data + offset, len); >> + >> + ret = t7xx_port_send_raw_skb(port_private, cloned); >> + if (ret) { >> + dev_kfree_skb(cloned); >> + dev_err(port_private->dev, "Write error on fastboot port, %d\n", ret)>; >> + break; >> + } >> + offset += len; >> + actual -= len; >> + } >> + >> + dev_kfree_skb(skb); >> + return 0; >> +} >> > >[...] >> ++static void t7xx_port_fastboot_uninit(struct t7xx_port *port) >> +{ >> + if (!port->wwan.wwan_port) >> + return; >> + >> + port->rx_length_th = 0; >> + wwan_remove_port(port->wwan.wwan_port); >> + port->wwan.wwan_port = NULL; >> +} >> + >> +static int t7xx_port_fastboot_recv_skb(struct t7xx_port *port, struct sk>_buff *skb) >> +{ >> + if (!atomic_read(&port->usage_cnt) || !port->chan_enable) { >> + const struct t7xx_port_conf *port_conf = port->port_conf; >> + >> + dev_kfree_skb_any(skb); >> + dev_err_ratelimited(port->dev, "Port %s is not opened, drop packets\n">, >> + port_conf->name); >> + /* Dropping skb, caller should not access skb.*/ >> + return 0; >> + } >> + >> + wwan_port_rx(port->wwan.wwan_port, skb); >> + >> + return 0; >> +} >> + >> +static int t7xx_port_fastboot_enable_chl(struct t7xx_port *port) >> +{ >> + spin_lock(&port->port_update_lock); >> + port->chan_enable = true; >> + spin_unlock(&port->port_update_lock); >> + >> + return 0; >> +} >> + >> +static int t7xx_port_fastboot_disable_chl(struct t7xx_port *port) >> +{ >> + spin_lock(&port->port_update_lock); >> + port->chan_enable = false; >> + spin_unlock(&port->port_update_lock); >> + >> + return 0; >> +} > >The above 4 functions implementation are exact copies of >t7xx_port_wwan_*() functions in drivers/net/wwan/t7xx/t7xx_port_wwan.c > >Please reorganize the code to avoid such duplication, e.g. renaming the >exiting function to something more generic, making them non static, and >declaring them in some t7xx specific header. Let me reorganize the code, I think I should merge fastboot codes to t7xx_port_wwan.c. Best Regards, JinJian
diff --git a/Documentation/networking/device_drivers/wwan/t7xx.rst b/Documentation/networking/device_drivers/wwan/t7xx.rst index d13624a52d8b..7257ede90152 100644 --- a/Documentation/networking/device_drivers/wwan/t7xx.rst +++ b/Documentation/networking/device_drivers/wwan/t7xx.rst @@ -125,6 +125,20 @@ The driver exposes an AT port by implementing AT WWAN Port. The userspace end of the control port is a /dev/wwan0at0 character device. Application shall use this interface to issue AT commands. +fastboot port userspace ABI +--------------------------- + +/dev/wwan0fastboot0 character device +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +The driver exposes a fastboot protocol interface by implementing +fastboot WWAN Port. The userspace end of the fastboot channel pipe is a +/dev/wwan0fastboot0 character device. Application shall use this interface for +fastboot protocol communication. + +Please note that driver needs to be reloaded to export /dev/wwan0fastboot0 +port, because device needs a cold reset after enter ``FASTBOOT_DL_SWITCHING`` +mode. + The MediaTek's T700 modem supports the 3GPP TS 27.007 [4] specification. References diff --git a/drivers/net/wwan/t7xx/Makefile b/drivers/net/wwan/t7xx/Makefile index 2652cd00504e..ddf03efe388a 100644 --- a/drivers/net/wwan/t7xx/Makefile +++ b/drivers/net/wwan/t7xx/Makefile @@ -11,6 +11,7 @@ mtk_t7xx-y:= t7xx_pci.o \ t7xx_port_proxy.o \ t7xx_port_ctrl_msg.o \ t7xx_port_wwan.o \ + t7xx_port_fastboot.o \ t7xx_hif_dpmaif.o \ t7xx_hif_dpmaif_tx.o \ t7xx_hif_dpmaif_rx.o \ diff --git a/drivers/net/wwan/t7xx/t7xx_port_fastboot.c b/drivers/net/wwan/t7xx/t7xx_port_fastboot.c new file mode 100644 index 000000000000..880931af3433 --- /dev/null +++ b/drivers/net/wwan/t7xx/t7xx_port_fastboot.c @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2023, Fibocom Wireless Inc. + * + * Authors: + * Jinjian Song <jinjian.song@fibocom.com> + */ + +#include <linux/atomic.h> +#include <linux/dev_printk.h> +#include <linux/err.h> +#include <linux/minmax.h> +#include <linux/skbuff.h> +#include <linux/spinlock.h> +#include <linux/wwan.h> + +#include "t7xx_port.h" +#include "t7xx_port_proxy.h" +#include "t7xx_state_monitor.h" + +static int t7xx_port_fastboot_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_fastboot_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_fastboot_tx(struct wwan_port *port, struct sk_buff *skb) +{ + struct t7xx_port *port_private = wwan_port_get_drvdata(port); + struct sk_buff *cur = skb, *cloned; + size_t actual, len, offset = 0; + int ret; + int txq_mtu; + + if (!port_private->chan_enable) + return -EINVAL; + + txq_mtu = t7xx_get_port_mtu(port_private); + if (txq_mtu < 0) + return -EINVAL; + + actual = cur->len; + while (actual) { + len = min_t(size_t, actual, txq_mtu); + cloned = __dev_alloc_skb(len, GFP_KERNEL); + if (!cloned) + return -ENOMEM; + + skb_put_data(cloned, cur->data + offset, len); + + ret = t7xx_port_send_raw_skb(port_private, cloned); + if (ret) { + dev_kfree_skb(cloned); + dev_err(port_private->dev, "Write error on fastboot port, %d\n", ret); + break; + } + offset += len; + actual -= len; + } + + dev_kfree_skb(skb); + return 0; +} + +static const struct wwan_port_ops wwan_ops = { + .start = t7xx_port_fastboot_start, + .stop = t7xx_port_fastboot_stop, + .tx = t7xx_port_fastboot_tx, +}; + +static int t7xx_port_fastboot_init(struct t7xx_port *port) +{ + const struct t7xx_port_conf *port_conf = port->port_conf; + unsigned int header_len = sizeof(struct ccci_header), mtu; + struct wwan_port_caps caps; + + port->rx_length_th = RX_QUEUE_MAXLEN; + + if (!port->wwan.wwan_port) { + mtu = t7xx_get_port_mtu(port); + caps.frag_len = mtu - header_len; + caps.headroom_len = header_len; + port->wwan.wwan_port = wwan_create_port(port->dev, port_conf->port_type, + &wwan_ops, &caps, port); + if (IS_ERR(port->wwan.wwan_port)) + dev_err(port->dev, "Unable to create WWWAN port %s", port_conf->name); + } + + return 0; +} + +static void t7xx_port_fastboot_uninit(struct t7xx_port *port) +{ + if (!port->wwan.wwan_port) + return; + + port->rx_length_th = 0; + wwan_remove_port(port->wwan.wwan_port); + port->wwan.wwan_port = NULL; +} + +static int t7xx_port_fastboot_recv_skb(struct t7xx_port *port, struct sk_buff *skb) +{ + if (!atomic_read(&port->usage_cnt) || !port->chan_enable) { + const struct t7xx_port_conf *port_conf = port->port_conf; + + dev_kfree_skb_any(skb); + dev_err_ratelimited(port->dev, "Port %s is not opened, drop packets\n", + port_conf->name); + /* Dropping skb, caller should not access skb.*/ + return 0; + } + + wwan_port_rx(port->wwan.wwan_port, skb); + + return 0; +} + +static int t7xx_port_fastboot_enable_chl(struct t7xx_port *port) +{ + spin_lock(&port->port_update_lock); + port->chan_enable = true; + spin_unlock(&port->port_update_lock); + + return 0; +} + +static int t7xx_port_fastboot_disable_chl(struct t7xx_port *port) +{ + spin_lock(&port->port_update_lock); + port->chan_enable = false; + spin_unlock(&port->port_update_lock); + + return 0; +} + +struct port_ops fastboot_port_ops = { + .init = t7xx_port_fastboot_init, + .recv_skb = t7xx_port_fastboot_recv_skb, + .uninit = t7xx_port_fastboot_uninit, + .enable_chl = t7xx_port_fastboot_enable_chl, + .disable_chl = t7xx_port_fastboot_disable_chl, +}; diff --git a/drivers/net/wwan/t7xx/t7xx_port_proxy.c b/drivers/net/wwan/t7xx/t7xx_port_proxy.c index e53a152faee4..7200d2d210fc 100644 --- a/drivers/net/wwan/t7xx/t7xx_port_proxy.c +++ b/drivers/net/wwan/t7xx/t7xx_port_proxy.c @@ -112,6 +112,9 @@ static const struct t7xx_port_conf t7xx_early_port_conf[] = { .txq_exp_index = CLDMA_Q_IDX_DUMP, .rxq_exp_index = CLDMA_Q_IDX_DUMP, .path_id = CLDMA_ID_AP, + .ops = &fastboot_port_ops, + .name = "fastboot", + .port_type = WWAN_PORT_FASTBOOT, }, }; diff --git a/drivers/net/wwan/t7xx/t7xx_port_proxy.h b/drivers/net/wwan/t7xx/t7xx_port_proxy.h index 7f5706811445..0f40b4884dc0 100644 --- a/drivers/net/wwan/t7xx/t7xx_port_proxy.h +++ b/drivers/net/wwan/t7xx/t7xx_port_proxy.h @@ -98,6 +98,8 @@ extern struct port_ops ctl_port_ops; extern struct port_ops t7xx_trace_port_ops; #endif +extern struct port_ops fastboot_port_ops; + void t7xx_port_proxy_reset(struct port_proxy *port_prox); void t7xx_port_proxy_uninit(struct port_proxy *port_prox); int t7xx_port_proxy_init(struct t7xx_modem *md); diff --git a/drivers/net/wwan/t7xx/t7xx_state_monitor.c b/drivers/net/wwan/t7xx/t7xx_state_monitor.c index 2bcd061617e2..60bc8d635ade 100644 --- a/drivers/net/wwan/t7xx/t7xx_state_monitor.c +++ b/drivers/net/wwan/t7xx/t7xx_state_monitor.c @@ -229,6 +229,7 @@ static void t7xx_lk_stage_event_handling(struct t7xx_fsm_ctl *ctl, unsigned int struct cldma_ctrl *md_ctrl; enum lk_event_id lk_event; struct device *dev; + struct t7xx_port *port; dev = &md->t7xx_dev->pdev->dev; lk_event = FIELD_GET(MISC_LK_EVENT_MASK, status); @@ -244,6 +245,9 @@ static void t7xx_lk_stage_event_handling(struct t7xx_fsm_ctl *ctl, unsigned int t7xx_cldma_stop(md_ctrl); t7xx_cldma_switch_cfg(md_ctrl, CLDMA_DEDICATED_Q_CFG); + port = &ctl->md->port_prox->ports[0]; + port->port_conf->ops->enable_chl(port); + t7xx_cldma_start(md_ctrl); if (lk_event == LK_EVENT_CREATE_POST_DL_PORT)