| Message ID | 20181107003250.5832-1-elder@linaro.org |
|---|---|
| Headers | show |
| Series | net: introduce Qualcomm IPA driver | expand |
Hi- On 11/6/18 4:32 PM, Alex Elder wrote: > diff --git a/drivers/net/ipa/Kconfig b/drivers/net/ipa/Kconfig > new file mode 100644 > index 000000000000..f8ea9363f532 > --- /dev/null > +++ b/drivers/net/ipa/Kconfig > @@ -0,0 +1,30 @@ > +config IPA > + tristate "Qualcomm IPA support" > + depends on NET > + select QCOM_QMI_HELPERS > + select QCOM_MDT_LOADER > + default n > + help > + Choose Y here to include support for the Qualcomm IP > + Accelerator (IPA), a hardware block present in some > + Qualcomm SoCs. The IPA is a programmable protocol > + processor that is capable of generic hardware handling > + of IP packets, including routing, filtering, and NAT. > + Currently the IPA driver supports only basic transport > + of network traffic between the AP and modem, on the > + Qualcomm SDM845 SoC. > + > + If unsure, say N. > + > +config IPA_ASSERT > + bool "Enable IPA assertions" > + depends on IPA > + default y > + help > + Incorporate IPA assertion verification in the build. This > + cause various design assumptions to be checked at runtime, causes > + generating a report (and a crash) if any assumed condition > + does not hold. You may wish to disable this to avoid the > + overhead of checking. > + > + If unsure doubt, say "Y" here. -- ~Randy
On Wed, Nov 7, 2018 at 1:33 AM Alex Elder <elder@linaro.org> wrote: > > This patch includes code implementing the IPA DMA module, which > defines a structure to represent a DMA allocation for the IPA device. > It's used throughout the IPA code. > > Signed-off-by: Alex Elder <elder@linaro.org> I looked through all the users of this and couldn't fine one that actually benefits from it. I'd say better drop this patch entirely and open-code the contents in the callers. That will help readability since the dma API is well understood by many people. Generally speaking, try not to wrap Linux interfaces into driver specific helper functions. Arnd
On Wed, Nov 7, 2018 at 1:33 AM Alex Elder <elder@linaro.org> wrote: > +config IPA_ASSERT > + bool "Enable IPA assertions" > + depends on IPA > + default y > + help > + Incorporate IPA assertion verification in the build. This > + cause various design assumptions to be checked at runtime, > + generating a report (and a crash) if any assumed condition > + does not hold. You may wish to disable this to avoid the > + overhead of checking. Maybe remove this from the submission. > +#define ipa_debug(fmt, args...) dev_dbg(ipa_ctx->dev, fmt, ## args) > +#define ipa_err(fmt, args...) dev_err(ipa_ctx->dev, fmt, ## args) These macros refer to variables in the caller that are not passed as arguments, which is generally a bad idea. They also trivially wrap a standard kernel interface, so better just that directly. > +#define ipa_bug() \ > + do { \ > + ipa_err("an unrecoverable error has occurred\n"); \ > + BUG(); \ > + } while (0) > + > +#define ipa_bug_on(condition) \ > + do { \ > + if (condition) { \ > + ipa_err("ipa_bug_on(%s) failed!\n", #condition); \ > + ipa_bug(); \ > + } \ > + } while (0) According to a discussion at the kernel summit, we should generally try to avoid BUG() as it rarely does anything useful: it crashes the current task, but in a network driver that usually means killing the entire kernel since you are not in process context. Try questioning each one to see if it can possibly happen, or if the code can be rewritten in a way to guarantee that it cannot. If continuing after the bug was detected does not cause a security hole or permanent data corruption, you can also use WARN_ON() or WARN_ONCE() (without a wrapper). > +int ipa_wwan_init(void); > +void ipa_wwan_cleanup(void); > + > +int ipa_stop_gsi_channel(u32 ep_id); > + > +void ipa_cfg_ep(u32 ep_id); > + > +int ipa_tx_dp(enum ipa_client_type dst, struct sk_buff *skb); > + > +bool ipa_endp_aggr_support(u32 ep_id); > +enum ipa_seq_type ipa_endp_seq_type(u32 ep_id); > + > +void ipa_endp_init_hdr_cons(u32 ep_id, u32 header_size, > + u32 metadata_offset, u32 length_offset); > +void ipa_endp_init_hdr_prod(u32 ep_id, u32 header_size, > + u32 metadata_offset, u32 length_offset); I'm surprised to see many functions that don't take a pointer to an instance as the first argument, which often indicates that you have global state variables and the driver won't work with multiple hardware instances. Arnd
On Wed, Nov 7, 2018 at 1:33 AM Alex Elder <elder@linaro.org> wrote: > Note: This portion of the driver will be heavily affected by > planned rework on the data path code. Ok. I don't think the ioctl interface has a real chance of getting merged into the kernel. You should generally not require any custom user space tools for a driver like this. > diff --git a/drivers/net/ipa/msm_rmnet.h b/drivers/net/ipa/msm_rmnet.h > new file mode 100644 > index 000000000000..042380fd53fb > --- /dev/null > +++ b/drivers/net/ipa/msm_rmnet.h Just for the record: if we really wanted to define ioctls, this would go into 'include/linux/uapi/msm_rmnet.h' and get installed into the /usr/include hierarchy on all machines. > + > +#define RMNET_IOCTL_SET_LLP_ETHERNET 0x000089f1 /* Set Ethernet protocol */ > +#define RMNET_IOCTL_SET_LLP_IP 0x000089f2 /* Set RAWIP protocol */ > +#define RMNET_IOCTL_GET_LLP 0x000089f3 /* Get link protocol */ > +#define RMNET_IOCTL_SET_QOS_ENABLE 0x000089f4 /* Set QoS header enabled */ > +#define RMNET_IOCTL_SET_QOS_DISABLE 0x000089f5 /* Set QoS header disabled*/ > +#define RMNET_IOCTL_GET_QOS 0x000089f6 /* Get QoS header state */ > +#define RMNET_IOCTL_GET_OPMODE 0x000089f7 /* Get operation mode */ And the commands would be defined using _IOC/_IOR/_IOW macros that document which arguments they take > +#define RMNET_IOCTL_OPEN 0x000089f8 /* Open transport port */ > +#define RMNET_IOCTL_CLOSE 0x000089f9 /* Close transport port */ > +#define RMNET_IOCTL_FLOW_ENABLE 0x000089fa /* Flow enable */ > +#define RMNET_IOCTL_FLOW_DISABLE 0x000089fb /* Flow disable */ > +#define RMNET_IOCTL_FLOW_SET_HNDL 0x000089fc /* Set flow handle */ > +#define RMNET_IOCTL_EXTENDED 0x000089fd /* Extended IOCTLs */ 'extended' interfaces are obviously out of the question entirely, those would all need to be separate commands. > +/* User space may not have this defined. */ > +#ifndef IFNAMSIZ > +#define IFNAMSIZ 16 > +#endif This is in <linux/if.h> > +struct rmnet_ioctl_extended_s { > + u32 extended_ioctl; > + union { And unions in the ioctl interfaces also wouldn't work. > +static bool initialized; /* Avoid duplicate initialization */ > + > +static struct rmnet_ipa_context rmnet_ipa_ctx_struct; > +static struct rmnet_ipa_context *rmnet_ipa_ctx = &rmnet_ipa_ctx_struct; Global variables like these should be removed. > +/** ipa_wwan_xmit() - Transmits an skb. > + * > + * @skb: skb to be transmitted > + * @dev: network device > + * > + * Return codes: > + * NETDEV_TX_OK: Success > + * NETDEV_TX_BUSY: Error while transmitting the skb. Try again later > + */ > +static int ipa_wwan_xmit(struct sk_buff *skb, struct net_device *dev) > +{ > + struct ipa_wwan_private *wwan_ptr = netdev_priv(dev); > + unsigned int skb_len; > + int outstanding; > + > + if (skb->protocol != htons(ETH_P_MAP)) { > + dev_kfree_skb_any(skb); > + dev->stats.tx_dropped++; > + return NETDEV_TX_OK; > + } > + > + /* Control packets are sent even if queue is stopped. We > + * always honor the data and control high-water marks. > + */ > + outstanding = atomic_read(&wwan_ptr->outstanding_pkts); > + if (!RMNET_MAP_GET_CD_BIT(skb)) { /* Data packet? */ > + if (netif_queue_stopped(dev)) > + return NETDEV_TX_BUSY; > + if (outstanding >= wwan_ptr->outstanding_high) > + return NETDEV_TX_BUSY; > + } else if (outstanding >= wwan_ptr->outstanding_high_ctl) { > + return NETDEV_TX_BUSY; > + } This seems to be a poor reimplementation of BQL. Better use netdev_sent_queue() and netdev_completed_queue() to do the same thing better. > +/** apps_ipa_packet_receive_notify() - Rx notify > + * > + * @priv: driver context > + * @evt: event type > + * @data: data provided with event > + * > + * IPA will pass a packet to the Linux network stack with skb->data > + */ > +static void apps_ipa_packet_receive_notify(void *priv, enum ipa_dp_evt_type evt, > + unsigned long data) > +{ > + struct ipa_wwan_private *wwan_ptr; > + struct net_device *dev = priv; > + > + wwan_ptr = netdev_priv(dev); > + if (evt == IPA_RECEIVE) { > + struct sk_buff *skb = (struct sk_buff *)data; > + int ret; > + unsigned int packet_len = skb->len; > + > + skb->dev = rmnet_ipa_ctx->dev; > + skb->protocol = htons(ETH_P_MAP); > + > + ret = netif_receive_skb(skb); > + if (ret) { > + pr_err_ratelimited("fail on netif_receive_skb\n"); > + dev->stats.rx_dropped++; > + } > + dev->stats.rx_packets++; > + dev->stats.rx_bytes += packet_len; > + } else if (evt == IPA_CLIENT_START_POLL) { > + napi_schedule(&wwan_ptr->napi); > + } else if (evt == IPA_CLIENT_COMP_NAPI) { > + napi_complete(&wwan_ptr->napi); > + } else { > + ipa_err("Invalid evt %d received in wan_ipa_receive\n", evt); > + } > +} I don't understand the logic here. Why is this a callback function? You normally want the data path to be as fast as possible, and the indirection seems like it would get in the way of that. Since the function doesn't do much interesting work, could it be moved into the caller? > +/** handle_ingress_format() - Ingress data format configuration */ > +static int handle_ingress_format(struct net_device *dev, > + struct rmnet_ioctl_extended_s *in) > +{ Can you describe how this would be called from user space? I.e. what is the reason we have to configure anything here? > + > + /* Unsupported requests */ > + case RMNET_IOCTL_SET_MRU: /* Set MRU */ > + case RMNET_IOCTL_GET_MRU: /* Get MRU */ > + case RMNET_IOCTL_GET_AGGREGATION_COUNT: /* Get agg count */ > + case RMNET_IOCTL_SET_AGGREGATION_COUNT: /* Set agg count */ > + case RMNET_IOCTL_GET_AGGREGATION_SIZE: /* Get agg size */ > + case RMNET_IOCTL_SET_AGGREGATION_SIZE: /* Set agg size */ > + case RMNET_IOCTL_FLOW_CONTROL: /* Do flow control */ > + case RMNET_IOCTL_GET_DFLT_CONTROL_CHANNEL: /* For legacy use */ > + case RMNET_IOCTL_GET_HWSW_MAP: /* Get HW/SW map */ > + case RMNET_IOCTL_SET_RX_HEADROOM: /* Set RX Headroom */ > + case RMNET_IOCTL_SET_QOS_VERSION: /* Set 8/6 byte QoS */ > + case RMNET_IOCTL_GET_QOS_VERSION: /* Get 8/6 byte QoS */ > + case RMNET_IOCTL_GET_SUPPORTED_QOS_MODES: /* Get QoS modes */ > + case RMNET_IOCTL_SET_SLEEP_STATE: /* Set sleep state */ > + case RMNET_IOCTL_SET_XLAT_DEV_INFO: /* xlat dev name */ > + case RMNET_IOCTL_DEREGISTER_DEV: /* Deregister netdev */ > + return -ENOTSUPP; /* Defined, but unsupported command */ > + > + default: > + return -EINVAL; /* Invalid (unrecognized) command */ > + } > + > +copy_out: > + return copy_to_user(data, &edata, size) ? -EFAULT : 0; > +} > + > +/** ipa_wwan_ioctl() - I/O control for wwan network driver */ > +static int ipa_wwan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd) > +{ > + struct rmnet_ioctl_data_s ioctl_data = { }; > + void __user *data; > + size_t size; > + > + data = ifr->ifr_ifru.ifru_data; > + size = sizeof(ioctl_data); > + > + switch (cmd) { > + /* These features are implied; alternatives are not supported */ > + case RMNET_IOCTL_SET_LLP_IP: /* RAW IP protocol */ > + case RMNET_IOCTL_SET_QOS_DISABLE: /* QoS header disabled */ > + return 0; > + > + /* These features are not supported; use alternatives */ > + case RMNET_IOCTL_SET_LLP_ETHERNET: /* Ethernet protocol */ > + case RMNET_IOCTL_SET_QOS_ENABLE: /* QoS header enabled */ > + case RMNET_IOCTL_GET_OPMODE: /* Get operation mode */ > + case RMNET_IOCTL_FLOW_ENABLE: /* Flow enable */ > + case RMNET_IOCTL_FLOW_DISABLE: /* Flow disable */ > + case RMNET_IOCTL_FLOW_SET_HNDL: /* Set flow handle */ > + return -ENOTSUPP; > + > + case RMNET_IOCTL_GET_LLP: /* Get link protocol */ > + ioctl_data.u.operation_mode = RMNET_MODE_LLP_IP; > + goto copy_out; > + > + case RMNET_IOCTL_GET_QOS: /* Get QoS header state */ > + ioctl_data.u.operation_mode = RMNET_MODE_NONE; > + goto copy_out; > + > + case RMNET_IOCTL_OPEN: /* Open transport port */ > + case RMNET_IOCTL_CLOSE: /* Close transport port */ > + return 0; > + > + case RMNET_IOCTL_EXTENDED: /* Extended IOCTLs */ > + return ipa_wwan_ioctl_extended(dev, data); > + > + default: > + return -EINVAL; > + } It would help to remove everything that is a nop or not implemented or that returns a constant value here, those are clearly not relevant for the submission here. > + > +static const struct of_device_id rmnet_ipa_dt_match[] = { > + {.compatible = "qcom,rmnet-ipa"}, > + {}, > +}; The match string looks overly generic, surely there must be plans to have future versions of this that might require identification. Arnd
On Wed, Nov 7, 2018 at 1:33 AM Alex Elder <elder@linaro.org> wrote: > +static void ipa_client_remove_deferred(struct work_struct *work); Try to avoid forward declarations by reordering the code in call order, it will also make it easier to read. > +static DECLARE_WORK(ipa_client_remove_work, ipa_client_remove_deferred); > + > +static struct ipa_context ipa_ctx_struct; > +struct ipa_context *ipa_ctx = &ipa_ctx_struct; Global state variables should generally be removed as well, and passed around as function arguments. > +static int hdr_init_local_cmd(u32 offset, u32 size) > +{ > + struct ipa_desc desc = { }; > + struct ipa_dma_mem mem; > + void *payload; > + int ret; > + > + if (ipa_dma_alloc(&mem, size, GFP_KERNEL)) > + return -ENOMEM; > + > + offset += ipa_ctx->smem_offset; > + > + payload = ipahal_hdr_init_local_pyld(&mem, offset); > + if (!payload) { > + ret = -ENOMEM; > + goto err_dma_free; > + } > + > + desc.type = IPA_IMM_CMD_DESC; > + desc.len_opcode = IPA_IMM_CMD_HDR_INIT_LOCAL; > + desc.payload = payload; > + > + ret = ipa_send_cmd(&desc); You have a bunch of dynamic allocations in here, which you then immediately tear down again after the command is complete. I can't see at all what you do with the DMA address, since you seem to not use the virtual address at all but only store the physical address in some kind of descriptor without ever writing to it. Am I missing something here? > +/* Remoteproc callbacks for SSR events: prepare, start, stop, unprepare */ > +int ipa_ssr_prepare(struct rproc_subdev *subdev) > +{ > + printk("======== SSR prepare received ========\n"); I think you mean dev_dbg() here. A plain printk() without a level is not correct and we probably don't want those messages to arrive on the console for normal users. > +static int ipa_firmware_load(struct de > + > +err_clear_dev: > + ipa_ctx->lan_cons_ep_id = 0; > + ipa_ctx->cmd_prod_ep_id = 0; > + ipahal_exit(); > +err_dma_exit: > + ipa_dma_exit(); > +err_clear_gsi: > + ipa_ctx->gsi = NULL; > + ipa_ctx->ipa_phys = 0; > + ipa_reg_exit(); > +err_clear_ipa_irq: > + ipa_ctx->ipa_irq = 0; > +err_clear_filter_bitmap: > + ipa_ctx->filter_bitmap = 0; > +err_interconnect_exit: > + ipa_interconnect_exit(); > +err_clock_exit: > + ipa_clock_exit(); > + ipa_ctx->dev = NULL; > +out_smp2p_exit: > + ipa_smp2p_exit(dev); > + No need to initialize members to zero when you are about to free the structure. > +static struct platform_driver ipa_plat_drv = { > + .probe = ipa_plat_drv_probe, > + .remove = ipa_plat_drv_remove, > + .driver = { > + .name = "ipa", > + .owner = THIS_MODULE, > + .pm = &ipa_pm_ops, > + .of_match_table = ipa_plat_drv_match, > + }, > +}; > + > +builtin_platform_driver(ipa_plat_drv); This should be module_platform_driver(), and allow unloading the driver. Arnd
On Tue, Nov 6, 2018 at 6:33 PM Alex Elder <elder@linaro.org> wrote: > > Add the binding definitions for the "qcom,ipa" and "qcom,rmnet-ipa" > device tree nodes. > > Signed-off-by: Alex Elder <elder@linaro.org> > --- > .../devicetree/bindings/soc/qcom/qcom,ipa.txt | 136 ++++++++++++++++++ > .../bindings/soc/qcom/qcom,rmnet-ipa.txt | 15 ++ > 2 files changed, 151 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt > create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt > new file mode 100644 > index 000000000000..d4d3d37df029 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt > @@ -0,0 +1,136 @@ > +Qualcomm IPA (IP Accelerator) Driver Bindings are for h/w not drivers. > + > +This binding describes the Qualcomm IPA. The IPA is capable of offloading > +certain network processing tasks (e.g. filtering, routing, and NAT) from > +the main processor. The IPA currently serves only as a network interface, > +providing access to an LTE network available via a modem. > + > +The IPA sits between multiple independent "execution environments," > +including the AP subsystem (APSS) and the modem. The IPA presents > +a Generic Software Interface (GSI) to each execution environment. > +The GSI is an integral part of the IPA, but it is logically isolated > +and has a distinct interrupt and a separately-defined address space. > + > + ---------- ------------- --------- > + | | |G| |G| | | > + | APSS |===|S| IPA |S|===| Modem | > + | | |I| |I| | | > + ---------- ------------- --------- > + > +See also: > + bindings/interrupt-controller/interrupts.txt > + bindings/interconnect/interconnect.txt > + bindings/soc/qcom/qcom,smp2p.txt > + bindings/reserved-memory/reserved-memory.txt > + bindings/clock/clock-bindings.txt > + > +All properties defined below are required. > + > +- compatible: > + Must be one of the following compatible strings: > + "qcom,ipa-sdm845-modem_init" > + "qcom,ipa-sdm845-tz_init" Normal order is <vendor>,<soc>-<ipblock>. Don't use '_'. What's the difference between these 2? It can't be detected somehow? This might be better expressed as a property. Then if Trustzone initializes things, it can just add a property. > + > +-reg: > + Resources specyfing the physical address spaces of the IPA and GSI. typo > + > +-reg-names: > + The names of the address space ranges defined by the "reg" property. > + Must be "ipa" and "gsi". > + > +- interrupts-extended: Use 'interrupts' here and describe what they are and the order. What they are connected to (and the need for interrupts-extended) is outside the scope of this binding. > + Specifies the IRQs used by the IPA. Four cells are required, > + specifying: the IPA IRQ; the GSI IRQ; the clock query interrupt > + from the modem; and the "ready for stage 2 initialization" > + interrupt from the modem. The first two are hardware IRQs; the > + third and fourth are SMP2P input interrupts. > + > +- interrupt-names: > + The names of the interrupts defined by the "interrupts-extended" > + property. Must be "ipa", "gsi", "ipa-clock-query", and > + "ipa-post-init". Format as one per line. > + > +- clocks: > + Resource that defines the IPA core clock. > + > +- clock-names: > + The name used for the IPA core clock. Must be "core". > + > +- interconnects: > + Specifies the interconnects used by the IPA. Three cells are > + required, specifying: the path from the IPA to memory; from > + IPA to internal (SoC resident) memory; and between the AP > + subsystem and IPA for register access. > + > +- interconnect-names: > + The names of the interconnects defined by the "interconnects" > + property. Must be "memory", "imem", and "config". > + > +- qcom,smem-states > + The state bits used for SMP2P output. Two cells must be specified. > + The first indicates whether the value in the second bit is valid > + (1 means valid). The second, if valid, defines whether the IPA > + clock is enabled (1 means enabled). > + > +- qcom,smem-state-names > + The names of the state bits used for SMP2P output. These must be > + "ipa-clock-enabled-valid" and "ipa-clock-enabled". > + > +- memory-region > + A phandle for a reserved memory area that holds the firmware passed > + to Trust Zone for authentication. (Note, this is required > + only for "qcom,ipa-sdm845-tz_init".) > + > += EXAMPLE > + > +The following example represents the IPA present in the SDM845 SoC. It > +shows portions of the "modem-smp2p" node to indicate its relationship > +with the interrupts and SMEM states used by the IPA. > + > + modem-smp2p { > + compatible = "qcom,smp2p"; > + . . . > + ipa_smp2p_out: ipa-ap-to-modem { > + qcom,entry-name = "ipa"; > + #qcom,smem-state-cells = <1>; > + }; > + > + ipa_smp2p_in: ipa-modem-to-ap { > + qcom,entry-name = "ipa"; > + interrupt-controller; > + #interrupt-cells = <2>; > + }; > + }; > + > + ipa@1e00000 { ipa@1e40000 > + compatible = "qcom,ipa-sdm845-modem_init"; > + > + reg = <0x1e40000 0x34000>, > + <0x1e04000 0x2c000>; > + reg-names = "ipa", > + "gsi"; > + > + interrupts-extended = <&intc 0 311 IRQ_TYPE_LEVEL_HIGH>, > + <&intc 0 432 IRQ_TYPE_LEVEL_HIGH>, > + <&ipa_smp2p_in 0 IRQ_TYPE_EDGE_RISING>, > + <&ipa_smp2p_in 1 IRQ_TYPE_EDGE_RISING>; > + interrupt-names = "ipa", > + "gsi", > + "ipa-clock-query", > + "ipa-post-init"; > + > + clocks = <&rpmhcc RPMH_IPA_CLK>; > + clock-names = "core"; > + > + interconnects = <&qnoc MASTER_IPA &qnoc SLAVE_EBI1>, > + <&qnoc MASTER_IPA &qnoc SLAVE_IMEM>, > + <&qnoc MASTER_APPSS_PROC &qnoc SLAVE_IPA_CFG>; > + interconnect-names = "memory", > + "imem", > + "config"; > + > + qcom,smem-states = <&ipa_smp2p_out 0>, > + <&ipa_smp2p_out 1>; > + qcom,smem-state-names = "ipa-clock-enabled-valid", > + "ipa-clock-enabled"; > + }; > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt > new file mode 100644 > index 000000000000..3d0b2aabefc7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt > @@ -0,0 +1,15 @@ > +Qualcomm IPA RMNet Driver > + > +This binding describes the IPA RMNet driver, which is used to > +represent virtual interfaces available on the modem accessed via > +the IPA. Other than the compatible string there are no properties > +associated with this device. Only a compatible string is a sure sign this is not a h/w device and you are just abusing DT to instantiate drivers. Make the IPA driver instantiate any sub drivers it needs. Rob
On Tue, 2018-11-06 at 18:32 -0600, Alex Elder wrote: > The IPA uses "rmnet" as a way to present remote network resources as > if they were local to the AP. IPA interfaces representing networks > accessible via the modem are represented as rmnet interfaces, > implemented by the "rmnet data driver" found here: > drivers/net/ethernet/qualcomm/rmnet/ It looks like there's a lot of overlap between this driver and that one. Ideally they would be a single driver which could automatically select the IPA mode for appropriate hardware, or the IPA mode would be a "subdriver" that bases itself heavily on the existing rmnet driver even if it doesn't use the same packet movement functions. Or something like that. But as Arnd stated, the ioctls won't fly. They were also proposed for the rmnet driver but they are better done as netlink, or via sysfs as the existing qmi_wwan driver does for some of the same values. Half the non-extended ioctls aren't even supported/used and should simply be removed. The extended ioctls should be evaluated as to whether they are really needed (eg RMNET_IOCTL_GET_DRIVER_NAME). I think most of the rest have either been handled already via the 'rmnet' driver itself (like the MUX channels using the vlan netlink attributes) or should be added via netlink-type mechansims if they are really required. In general, it would be good to get to a single 'rmnet' driver that has a single API and supports as much hardware as possible. There's too much duplication currently. Dan > The IPA is able to perform aggregation of packets, as well as > checksum offload. These options (plus others, such as configuring > MTU size) are configurable using an ioctl interface. In addition, > rmnet devices support multiplexing. > > TX packets are handed to the data path layer, and when their > transmission is complete the notification callback will be > called. The data path code posts RX packets to the hardware, > and when they are filled they are supplied here by a receive > notification callback. > > The IPA driver currently does not support the modem shutting down > (or crashing). But the rmnet_ipa device roughly represents the > availability of networks reachable by the modem. If the modem is > operational, an ipa_rmnet network device will be available. Modem > operation is managed by the remoteproc subsystem. > > Note: This portion of the driver will be heavily affected by > planned rework on the data path code. > > Signed-off-by: Alex Elder <elder@linaro.org> > --- > drivers/net/ipa/msm_rmnet.h | 120 +++++ > drivers/net/ipa/rmnet_config.h | 31 ++ > drivers/net/ipa/rmnet_ipa.c | 805 > +++++++++++++++++++++++++++++++++ > 3 files changed, 956 insertions(+) > create mode 100644 drivers/net/ipa/msm_rmnet.h > create mode 100644 drivers/net/ipa/rmnet_config.h > create mode 100644 drivers/net/ipa/rmnet_ipa.c > > diff --git a/drivers/net/ipa/msm_rmnet.h > b/drivers/net/ipa/msm_rmnet.h > new file mode 100644 > index 000000000000..042380fd53fb > --- /dev/null > +++ b/drivers/net/ipa/msm_rmnet.h > @@ -0,0 +1,120 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* Copyright (c) 2018, The Linux Foundation. All rights reserved. > + * Copyright (C) 2018 Linaro Ltd. > + */ > +#ifndef _MSM_RMNET_H_ > +#define _MSM_RMNET_H_ > + > +/* Bitmap macros for RmNET driver operation mode. */ > +#define RMNET_MODE_NONE 0x00 > +#define RMNET_MODE_LLP_ETH 0x01 > +#define RMNET_MODE_LLP_IP 0x02 > +#define RMNET_MODE_QOS 0x04 > + > +/* IOCTL commands > + * Values chosen to not conflict with other drivers in the ecosystem > + */ > + > +#define RMNET_IOCTL_SET_LLP_ETHERNET 0x000089f1 /* Set Ethernet > protocol */ > +#define RMNET_IOCTL_SET_LLP_IP 0x000089f2 /* Set RAWIP > protocol */ > +#define RMNET_IOCTL_GET_LLP 0x000089f3 /* Get link > protocol */ > +#define RMNET_IOCTL_SET_QOS_ENABLE 0x000089f4 /* Set QoS header > enabled */ > +#define RMNET_IOCTL_SET_QOS_DISABLE 0x000089f5 /* Set QoS header > disabled*/ > +#define RMNET_IOCTL_GET_QOS 0x000089f6 /* Get QoS header > state */ > +#define RMNET_IOCTL_GET_OPMODE 0x000089f7 /* Get > operation mode */ > +#define RMNET_IOCTL_OPEN 0x000089f8 /* Open transport > port */ > +#define RMNET_IOCTL_CLOSE 0x000089f9 /* Close transport > port */ > +#define RMNET_IOCTL_FLOW_ENABLE 0x000089fa /* Flow > enable */ > +#define RMNET_IOCTL_FLOW_DISABLE 0x000089fb /* Flow disable > */ > +#define RMNET_IOCTL_FLOW_SET_HNDL 0x000089fc /* Set flow handle > */ > +#define RMNET_IOCTL_EXTENDED 0x000089fd /* Extended > IOCTLs */ > + > +/* RmNet Data Required IOCTLs */ > +#define RMNET_IOCTL_GET_SUPPORTED_FEATURES 0x0000 /* Get > features */ > +#define RMNET_IOCTL_SET_MRU 0x0001 /* > Set MRU */ > +#define RMNET_IOCTL_GET_MRU 0x0002 /* > Get MRU */ > +#define RMNET_IOCTL_GET_EPID 0x0003 /* > Get endpoint ID */ > +#define RMNET_IOCTL_GET_DRIVER_NAME 0x0004 /* > Get driver name */ > +#define RMNET_IOCTL_ADD_MUX_CHANNEL 0x0005 /* > Add MUX ID */ > +#define RMNET_IOCTL_SET_EGRESS_DATA_FORMAT 0x0006 /* Set > EDF */ > +#define RMNET_IOCTL_SET_INGRESS_DATA_FORMAT 0x0007 /* Set > IDF */ > +#define RMNET_IOCTL_SET_AGGREGATION_COUNT 0x0008 /* Set > agg count */ > +#define RMNET_IOCTL_GET_AGGREGATION_COUNT 0x0009 /* Get > agg count */ > +#define RMNET_IOCTL_SET_AGGREGATION_SIZE 0x000a /* Set > agg size */ > +#define RMNET_IOCTL_GET_AGGREGATION_SIZE 0x000b /* Get > agg size */ > +#define RMNET_IOCTL_FLOW_CONTROL 0x000c /* Do > flow control */ > +#define RMNET_IOCTL_GET_DFLT_CONTROL_CHANNEL 0x000d /* For > legacy use */ > +#define RMNET_IOCTL_GET_HWSW_MAP 0x000e /* Get > HW/SW map */ > +#define RMNET_IOCTL_SET_RX_HEADROOM 0x000f /* > RX Headroom */ > +#define RMNET_IOCTL_GET_EP_PAIR 0x0010 > /* Endpoint pair */ > +#define RMNET_IOCTL_SET_QOS_VERSION 0x0011 /* > 8/6 byte QoS hdr*/ > +#define RMNET_IOCTL_GET_QOS_VERSION 0x0012 /* > 8/6 byte QoS hdr*/ > +#define RMNET_IOCTL_GET_SUPPORTED_QOS_MODES 0x0013 /* Get > QoS modes */ > +#define RMNET_IOCTL_SET_SLEEP_STATE 0x0014 /* > Set sleep state */ > +#define RMNET_IOCTL_SET_XLAT_DEV_INFO 0x0015 /* > xlat dev name */ > +#define RMNET_IOCTL_DEREGISTER_DEV 0x0016 /* > Dereg a net dev */ > +#define RMNET_IOCTL_GET_SG_SUPPORT 0x0017 /* > Query sg support*/ > + > +/* Return values for the RMNET_IOCTL_GET_SUPPORTED_FEATURES IOCTL */ > +#define RMNET_IOCTL_FEAT_NOTIFY_MUX_CHANNEL BIT(0) > +#define RMNET_IOCTL_FEAT_SET_EGRESS_DATA_FORMAT BIT(1 > ) > +#define RMNET_IOCTL_FEAT_SET_INGRESS_DATA_FORMAT BIT(2) > + > +/* Input values for the RMNET_IOCTL_SET_EGRESS_DATA_FORMAT IOCTL */ > +#define RMNET_IOCTL_EGRESS_FORMAT_AGGREGATION BIT(2) > +#define RMNET_IOCTL_EGRESS_FORMAT_CHECKSUM BIT(4) > + > +/* Input values for the RMNET_IOCTL_SET_INGRESS_DATA_FORMAT IOCTL */ > +#define RMNET_IOCTL_INGRESS_FORMAT_CHECKSUM BIT(4) > +#define RMNET_IOCTL_INGRESS_FORMAT_AGG_DATA BIT(5) > + > +/* User space may not have this defined. */ > +#ifndef IFNAMSIZ > +#define IFNAMSIZ 16 > +#endif > + > +struct rmnet_ioctl_extended_s { > + u32 extended_ioctl; > + union { > + u32 data; /* Generic data field for most > extended IOCTLs */ > + > + /* Return values for > + * RMNET_IOCTL_GET_DRIVER_NAME > + * RMNET_IOCTL_GET_DFLT_CONTROL_CHANNEL > + */ > + char if_name[IFNAMSIZ]; > + > + /* Input values for the RMNET_IOCTL_ADD_MUX_CHANNEL > IOCTL */ > + struct { > + u32 mux_id; > + char vchannel_name[IFNAMSIZ]; > + } rmnet_mux_val; > + > + /* Input values for the RMNET_IOCTL_FLOW_CONTROL > IOCTL */ > + struct { > + u8 flow_mode; > + u8 mux_id; > + } flow_control_prop; > + > + /* Return values for RMNET_IOCTL_GET_EP_PAIR */ > + struct { > + u32 consumer_pipe_num; > + u32 producer_pipe_num; > + } ipa_ep_pair; > + > + struct { > + u32 __data; /* Placeholder for legacy > data*/ > + u32 agg_size; > + u32 agg_count; > + } ingress_format; > + } u; > +}; > + > +struct rmnet_ioctl_data_s { > + union { > + u32 operation_mode; > + u32 tcm_handle; > + } u; > +}; > +#endif /* _MSM_RMNET_H_ */ > diff --git a/drivers/net/ipa/rmnet_config.h > b/drivers/net/ipa/rmnet_config.h > new file mode 100644 > index 000000000000..3b9a549ca1bd > --- /dev/null > +++ b/drivers/net/ipa/rmnet_config.h > @@ -0,0 +1,31 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* Copyright (c) 2016-2018, The Linux Foundation. All rights > reserved. > + * Copyright (C) 2018 Linaro Ltd. > + */ > +#ifndef _RMNET_CONFIG_H_ > +#define _RMNET_CONFIG_H_ > + > +#include <linux/types.h> > + > +/* XXX We want to use struct rmnet_map_header, but that's currently > defined in > + * XXX drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h > + * XXX We also want to use RMNET_MAP_GET_CD_BIT(Y), defined in the > same file. > + */ > +struct rmnet_map_header_s { > +#ifndef RMNET_USE_BIG_ENDIAN_STRUCTS > + u8 pad_len : 6, > + reserved_bit : 1, > + cd_bit : 1; > +#else > + u8 cd_bit : 1, > + reserved_bit : 1, > + pad_len : 6; > +#endif /* RMNET_USE_BIG_ENDIAN_STRUCTS */ > + u8 mux_id; > + u16 pkt_len; > +} __aligned(1); > + > +#define RMNET_MAP_GET_CD_BIT(Y) (((struct rmnet_map_header_s *)Y- > >data)->cd_bit) > + > +#endif /* _RMNET_CONFIG_H_ */ > diff --git a/drivers/net/ipa/rmnet_ipa.c > b/drivers/net/ipa/rmnet_ipa.c > new file mode 100644 > index 000000000000..7006afe3a5ea > --- /dev/null > +++ b/drivers/net/ipa/rmnet_ipa.c > @@ -0,0 +1,805 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* Copyright (c) 2014-2018, The Linux Foundation. All rights > reserved. > + * Copyright (C) 2018 Linaro Ltd. > + */ > + > +/* WWAN Transport Network Driver. */ > + > +#include <linux/completion.h> > +#include <linux/errno.h> > +#include <linux/if_arp.h> > +#include <linux/interrupt.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/netdevice.h> > +#include <linux/of_device.h> > +#include <linux/string.h> > +#include <linux/skbuff.h> > +#include <linux/version.h> > +#include <linux/workqueue.h> > +#include <net/pkt_sched.h> > + > +#include "msm_rmnet.h" > +#include "rmnet_config.h" > +#include "ipa_qmi.h" > +#include "ipa_i.h" > + > +#define DRIVER_NAME "wwan_ioctl" > +#define IPA_WWAN_DEV_NAME "rmnet_ipa%d" > + > +#define MUX_CHANNEL_MAX 10 /* max mux channels > */ > + > +#define NAPI_WEIGHT 60 > + > +#define WWAN_DATA_LEN 2000 > +#define HEADROOM_FOR_QMAP 8 /* for mux header */ > +#define TAILROOM 0 /* for padding by mux layer > */ > + > +#define DEFAULT_OUTSTANDING_HIGH 128 > +#define DEFAULT_OUTSTANDING_HIGH_CTL (DEFAULT_OUTSTANDING_HIG > H + 32) > +#define DEFAULT_OUTSTANDING_LOW 64 > + > +#define IPA_APPS_WWAN_CONS_RING_COUNT 256 > +#define IPA_APPS_WWAN_PROD_RING_COUNT 512 > + > +static int ipa_rmnet_poll(struct napi_struct *napi, int budget); > + > +/** struct ipa_wwan_private - WWAN private data > + * @net: network interface struct implemented by this driver > + * @stats: iface statistics > + * @outstanding_high: number of outstanding packets allowed > + * @outstanding_low: number of outstanding packets which shall cause > + * > + * WWAN private - holds all relevant info about WWAN driver > + */ > +struct ipa_wwan_private { > + struct net_device_stats stats; > + atomic_t outstanding_pkts; > + int outstanding_high_ctl; > + int outstanding_high; > + int outstanding_low; > + struct napi_struct napi; > +}; > + > +struct rmnet_ipa_context { > + struct net_device *dev; > + struct mutex mux_id_mutex; /* protects > mux_id[] */ > + u32 mux_id_count; > + u32 mux_id[MUX_CHANNEL_MAX]; > + u32 wan_prod_ep_id; > + u32 wan_cons_ep_id; > + struct mutex ep_setup_mutex; /* endpoint > setup/teardown */ > +}; > + > +static bool initialized; /* Avoid duplicate initialization */ > + > +static struct rmnet_ipa_context rmnet_ipa_ctx_struct; > +static struct rmnet_ipa_context *rmnet_ipa_ctx = > &rmnet_ipa_ctx_struct; > + > +/** wwan_open() - Opens the wwan network interface */ > +static int ipa_wwan_open(struct net_device *dev) > +{ > + struct ipa_wwan_private *wwan_ptr = netdev_priv(dev); > + > + napi_enable(&wwan_ptr->napi); > + netif_start_queue(dev); > + > + return 0; > +} > + > +/** ipa_wwan_stop() - Stops the wwan network interface. */ > +static int ipa_wwan_stop(struct net_device *dev) > +{ > + netif_stop_queue(dev); > + > + return 0; > +} > + > +/** ipa_wwan_xmit() - Transmits an skb. > + * > + * @skb: skb to be transmitted > + * @dev: network device > + * > + * Return codes: > + * NETDEV_TX_OK: Success > + * NETDEV_TX_BUSY: Error while transmitting the skb. Try again later > + */ > +static int ipa_wwan_xmit(struct sk_buff *skb, struct net_device > *dev) > +{ > + struct ipa_wwan_private *wwan_ptr = netdev_priv(dev); > + unsigned int skb_len; > + int outstanding; > + > + if (skb->protocol != htons(ETH_P_MAP)) { > + dev_kfree_skb_any(skb); > + dev->stats.tx_dropped++; > + return NETDEV_TX_OK; > + } > + > + /* Control packets are sent even if queue is stopped. We > + * always honor the data and control high-water marks. > + */ > + outstanding = atomic_read(&wwan_ptr->outstanding_pkts); > + if (!RMNET_MAP_GET_CD_BIT(skb)) { /* Data packet? */ > + if (netif_queue_stopped(dev)) > + return NETDEV_TX_BUSY; > + if (outstanding >= wwan_ptr->outstanding_high) > + return NETDEV_TX_BUSY; > + } else if (outstanding >= wwan_ptr->outstanding_high_ctl) { > + return NETDEV_TX_BUSY; > + } > + > + /* both data packets and commands will be routed to > + * IPA_CLIENT_Q6_WAN_CONS based on status configuration. > + */ > + skb_len = skb->len; > + if (ipa_tx_dp(IPA_CLIENT_APPS_WAN_PROD, skb)) > + return NETDEV_TX_BUSY; > + > + atomic_inc(&wwan_ptr->outstanding_pkts); > + dev->stats.tx_packets++; > + dev->stats.tx_bytes += skb_len; > + > + return NETDEV_TX_OK; > +} > + > +/** apps_ipa_tx_complete_notify() - Rx notify > + * > + * @priv: driver context > + * @evt: event type > + * @data: data provided with event > + * > + * Check that the packet is the one we sent and release it > + * This function will be called in defered context in IPA wq. > + */ > +static void apps_ipa_tx_complete_notify(void *priv, enum > ipa_dp_evt_type evt, > + unsigned long data) > +{ > + struct ipa_wwan_private *wwan_ptr; > + struct net_device *dev = priv; > + struct sk_buff *skb; > + > + skb = (struct sk_buff *)data; > + > + if (dev != rmnet_ipa_ctx->dev) { > + dev_kfree_skb_any(skb); > + return; > + } > + > + if (evt != IPA_WRITE_DONE) { > + ipa_err("unsupported evt on Tx callback, Drop the > packet\n"); > + dev_kfree_skb_any(skb); > + dev->stats.tx_dropped++; > + return; > + } > + > + wwan_ptr = netdev_priv(dev); > + atomic_dec(&wwan_ptr->outstanding_pkts); > + __netif_tx_lock_bh(netdev_get_tx_queue(dev, 0)); > + if (netif_queue_stopped(dev) && > + atomic_read(&wwan_ptr->outstanding_pkts) < > + wwan_ptr->outstanding_low) { > + netif_wake_queue(dev); > + } > + > + __netif_tx_unlock_bh(netdev_get_tx_queue(dev, 0)); > + dev_kfree_skb_any(skb); > +} > + > +/** apps_ipa_packet_receive_notify() - Rx notify > + * > + * @priv: driver context > + * @evt: event type > + * @data: data provided with event > + * > + * IPA will pass a packet to the Linux network stack with skb->data > + */ > +static void apps_ipa_packet_receive_notify(void *priv, enum > ipa_dp_evt_type evt, > + unsigned long data) > +{ > + struct ipa_wwan_private *wwan_ptr; > + struct net_device *dev = priv; > + > + wwan_ptr = netdev_priv(dev); > + if (evt == IPA_RECEIVE) { > + struct sk_buff *skb = (struct sk_buff *)data; > + int ret; > + unsigned int packet_len = skb->len; > + > + skb->dev = rmnet_ipa_ctx->dev; > + skb->protocol = htons(ETH_P_MAP); > + > + ret = netif_receive_skb(skb); > + if (ret) { > + pr_err_ratelimited("fail on > netif_receive_skb\n"); > + dev->stats.rx_dropped++; > + } > + dev->stats.rx_packets++; > + dev->stats.rx_bytes += packet_len; > + } else if (evt == IPA_CLIENT_START_POLL) { > + napi_schedule(&wwan_ptr->napi); > + } else if (evt == IPA_CLIENT_COMP_NAPI) { > + napi_complete(&wwan_ptr->napi); > + } else { > + ipa_err("Invalid evt %d received in > wan_ipa_receive\n", evt); > + } > +} > + > +/** handle_ingress_format() - Ingress data format configuration */ > +static int handle_ingress_format(struct net_device *dev, > + struct rmnet_ioctl_extended_s *in) > +{ > + enum ipa_cs_offload_en offload_type; > + enum ipa_client_type client; > + u32 metadata_offset; > + u32 rx_buffer_size; > + u32 channel_count; > + u32 length_offset; > + u32 header_size; > + bool aggr_active; > + u32 aggr_bytes; > + u32 aggr_count; > + u32 aggr_size; /* in KB */ > + u32 ep_id; > + int ret; > + > + client = IPA_CLIENT_APPS_WAN_CONS; > + channel_count = IPA_APPS_WWAN_CONS_RING_COUNT; > + header_size = sizeof(struct rmnet_map_header_s); > + metadata_offset = offsetof(struct rmnet_map_header_s, > mux_id); > + length_offset = offsetof(struct rmnet_map_header_s, > pkt_len); > + offload_type = IPA_CS_OFFLOAD_NONE; > + aggr_bytes = IPA_GENERIC_AGGR_BYTE_LIMIT; > + aggr_count = IPA_GENERIC_AGGR_PKT_LIMIT; > + aggr_active = false; > + > + if (in->u.data & RMNET_IOCTL_INGRESS_FORMAT_CHECKSUM) > + offload_type = IPA_CS_OFFLOAD_DL; > + > + if (in->u.data & RMNET_IOCTL_INGRESS_FORMAT_AGG_DATA) { > + aggr_bytes = in->u.ingress_format.agg_size; > + aggr_count = in->u.ingress_format.agg_count; > + aggr_active = true; > + } > + > + if (aggr_bytes > ipa_reg_aggr_max_byte_limit()) > + return -EINVAL; > + > + if (aggr_count > ipa_reg_aggr_max_packet_limit()) > + return -EINVAL; > + > + /* Compute the buffer size required to handle the requested > + * aggregation byte limit. The aggr_byte_limit value is > + * expressed as a number of KB, but we derive that value > + * after computing the buffer size to use (in bytes). The > + * buffer must be sufficient to hold one IPA_MTU-sized > + * packet *after* the limit is reached. > + * > + * (Note that the rx_buffer_size value reflects only the > + * space for data, not any standard metadata or headers.) > + */ > + rx_buffer_size = ipa_aggr_byte_limit_buf_size(aggr_bytes); > + > + /* Account for the extra IPA_MTU past the limit in the > + * buffer, and convert the result to the KB units the > + * aggr_byte_limit uses. > + */ > + aggr_size = (rx_buffer_size - IPA_MTU) / SZ_1K; > + > + mutex_lock(&rmnet_ipa_ctx->ep_setup_mutex); > + > + if (rmnet_ipa_ctx->wan_cons_ep_id != IPA_EP_ID_BAD) { > + ret = -EBUSY; > + goto out_unlock; > + } > + > + ret = ipa_ep_alloc(client); > + if (ret < 0) > + goto out_unlock; > + ep_id = ret; > + > + /* Record our endpoint configuration parameters */ > + ipa_endp_init_hdr_cons(ep_id, header_size, metadata_offset, > + length_offset); > + ipa_endp_init_hdr_ext_cons(ep_id, 0, true); > + ipa_endp_init_aggr_cons(ep_id, aggr_size, aggr_count, true); > + ipa_endp_init_cfg_cons(ep_id, offload_type); > + ipa_endp_init_hdr_metadata_mask_cons(ep_id, 0xff000000); > + ipa_endp_status_cons(ep_id, !aggr_active); > + > + ipa_ctx->ipa_client_apps_wan_cons_agg_gro = aggr_active; > + > + ret = ipa_ep_setup(ep_id, channel_count, 1, rx_buffer_size, > + apps_ipa_packet_receive_notify, dev); > + if (ret) > + ipa_ep_free(ep_id); > + else > + rmnet_ipa_ctx->wan_cons_ep_id = ep_id; > +out_unlock: > + mutex_unlock(&rmnet_ipa_ctx->ep_setup_mutex); > + > + return ret; > +} > + > +/** handle_egress_format() - Egress data format configuration */ > +static int handle_egress_format(struct net_device *dev, > + struct rmnet_ioctl_extended_s *e) > +{ > + enum ipa_cs_offload_en offload_type; > + enum ipa_client_type dst_client; > + enum ipa_client_type client; > + enum ipa_aggr_type aggr_type; > + enum ipa_aggr_en aggr_en; > + u32 channel_count; > + u32 length_offset; > + u32 header_align; > + u32 header_offset; > + u32 header_size; > + u32 ep_id; > + int ret; > + > + client = IPA_CLIENT_APPS_WAN_PROD; > + dst_client = IPA_CLIENT_APPS_LAN_CONS; > + channel_count = IPA_APPS_WWAN_PROD_RING_COUNT; > + header_size = sizeof(struct rmnet_map_header_s); > + offload_type = IPA_CS_OFFLOAD_NONE; > + aggr_en = IPA_BYPASS_AGGR; > + aggr_type = 0; /* ignored if BYPASS */ > + header_offset = 0; > + length_offset = 0; > + header_align = 0; > + > + if (e->u.data & RMNET_IOCTL_EGRESS_FORMAT_CHECKSUM) { > + offload_type = IPA_CS_OFFLOAD_UL; > + header_offset = sizeof(struct rmnet_map_header_s) / > 4; > + header_size += sizeof(u32); > + } > + > + if (e->u.data & RMNET_IOCTL_EGRESS_FORMAT_AGGREGATION) { > + aggr_en = IPA_ENABLE_DEAGGR; > + aggr_type = IPA_QCMAP; > + length_offset = offsetof(struct rmnet_map_header_s, > pkt_len); > + header_align = ilog2(sizeof(u32)); > + } > + > + mutex_lock(&rmnet_ipa_ctx->ep_setup_mutex); > + > + if (rmnet_ipa_ctx->wan_prod_ep_id != IPA_EP_ID_BAD) { > + ret = -EBUSY; > + goto out_unlock; > + } > + > + ret = ipa_ep_alloc(client); > + if (ret < 0) > + goto out_unlock; > + ep_id = ret; > + > + if (aggr_en == IPA_ENABLE_DEAGGR && > !ipa_endp_aggr_support(ep_id)) { > + ret = -ENOTSUPP; > + goto out_unlock; > + } > + > + /* We really do want 0 metadata offset */ > + ipa_endp_init_hdr_prod(ep_id, header_size, 0, > length_offset); > + ipa_endp_init_hdr_ext_prod(ep_id, header_align); > + ipa_endp_init_mode_prod(ep_id, IPA_BASIC, dst_client); > + ipa_endp_init_aggr_prod(ep_id, aggr_en, aggr_type); > + ipa_endp_init_cfg_prod(ep_id, offload_type, header_offset); > + ipa_endp_init_seq_prod(ep_id); > + ipa_endp_init_deaggr_prod(ep_id); > + /* Enable source notification status for exception packets > + * (i.e. QMAP commands) to be routed to modem. > + */ > + ipa_endp_status_prod(ep_id, true, IPA_CLIENT_Q6_WAN_CONS); > + > + /* Use a deferred interrupting no-op to reduce completion > interrupts */ > + ipa_no_intr_init(ep_id); > + > + ret = ipa_ep_setup(ep_id, channel_count, 1, 0, > + apps_ipa_tx_complete_notify, dev); > + if (ret) > + ipa_ep_free(ep_id); > + else > + rmnet_ipa_ctx->wan_prod_ep_id = ep_id; > + > +out_unlock: > + mutex_unlock(&rmnet_ipa_ctx->ep_setup_mutex); > + > + return ret; > +} > + > +/** ipa_wwan_add_mux_channel() - add a mux_id */ > +static int ipa_wwan_add_mux_channel(u32 mux_id) > +{ > + int ret; > + u32 i; > + > + mutex_lock(&rmnet_ipa_ctx->mux_id_mutex); > + > + if (rmnet_ipa_ctx->mux_id_count >= MUX_CHANNEL_MAX) { > + ret = -EFAULT; > + goto out; > + } > + > + for (i = 0; i < rmnet_ipa_ctx->mux_id_count; i++) > + if (mux_id == rmnet_ipa_ctx->mux_id[i]) > + break; > + > + /* Record the mux_id if it hasn't already been seen */ > + if (i == rmnet_ipa_ctx->mux_id_count) > + rmnet_ipa_ctx->mux_id[rmnet_ipa_ctx->mux_id_count++] > = mux_id; > + ret = 0; > +out: > + mutex_unlock(&rmnet_ipa_ctx->mux_id_mutex); > + > + return ret; > +} > + > +/** ipa_wwan_ioctl_extended() - rmnet extended I/O control */ > +static int ipa_wwan_ioctl_extended(struct net_device *dev, void > __user *data) > +{ > + struct rmnet_ioctl_extended_s edata = { }; > + size_t size = sizeof(edata); > + > + if (copy_from_user(&edata, data, size)) > + return -EFAULT; > + > + switch (edata.extended_ioctl) { > + case RMNET_IOCTL_GET_SUPPORTED_FEATURES: /* Get > features */ > + edata.u.data = RMNET_IOCTL_FEAT_NOTIFY_MUX_CHANNEL; > + edata.u.data |= > RMNET_IOCTL_FEAT_SET_EGRESS_DATA_FORMAT; > + edata.u.data |= > RMNET_IOCTL_FEAT_SET_INGRESS_DATA_FORMAT; > + goto copy_out; > + > + case RMNET_IOCTL_GET_EPID: /* Get > endpoint ID */ > + edata.u.data = 1; > + goto copy_out; > + > + case RMNET_IOCTL_GET_DRIVER_NAME: /* Get > driver name */ > + memcpy(&edata.u.if_name, rmnet_ipa_ctx->dev->name, > IFNAMSIZ); > + goto copy_out; > + > + case RMNET_IOCTL_ADD_MUX_CHANNEL: /* Add MUX > ID */ > + return > ipa_wwan_add_mux_channel(edata.u.rmnet_mux_val.mux_id); > + > + case RMNET_IOCTL_SET_EGRESS_DATA_FORMAT: /* Egress > data format */ > + return handle_egress_format(dev, &edata) ? -EFAULT : > 0; > + > + case RMNET_IOCTL_SET_INGRESS_DATA_FORMAT: /* Ingress > format */ > + return handle_ingress_format(dev, &edata) ? -EFAULT > : 0; > + > + case RMNET_IOCTL_GET_EP_PAIR: /* Get > endpoint pair */ > + edata.u.ipa_ep_pair.consumer_pipe_num = > + ipa_client_ep_id(IPA_CLIENT_APPS_WAN > _PROD); > + edata.u.ipa_ep_pair.producer_pipe_num = > + ipa_client_ep_id(IPA_CLIENT_APPS_WAN > _CONS); > + goto copy_out; > + > + case RMNET_IOCTL_GET_SG_SUPPORT: /* Get SG > support */ > + edata.u.data = 1; /* Scatter/gather is always > supported */ > + goto copy_out; > + > + /* Unsupported requests */ > + case RMNET_IOCTL_SET_MRU: /* Set MRU > */ > + case RMNET_IOCTL_GET_MRU: /* Get MRU > */ > + case RMNET_IOCTL_GET_AGGREGATION_COUNT: /* > Get agg count */ > + case RMNET_IOCTL_SET_AGGREGATION_COUNT: /* > Set agg count */ > + case RMNET_IOCTL_GET_AGGREGATION_SIZE: /* Get > agg size */ > + case RMNET_IOCTL_SET_AGGREGATION_SIZE: /* Set > agg size */ > + case RMNET_IOCTL_FLOW_CONTROL: /* Do > flow control */ > + case RMNET_IOCTL_GET_DFLT_CONTROL_CHANNEL: /* For > legacy use */ > + case RMNET_IOCTL_GET_HWSW_MAP: /* Get > HW/SW map */ > + case RMNET_IOCTL_SET_RX_HEADROOM: /* Set RX > Headroom */ > + case RMNET_IOCTL_SET_QOS_VERSION: /* Set 8/6 > byte QoS */ > + case RMNET_IOCTL_GET_QOS_VERSION: /* Get 8/6 > byte QoS */ > + case RMNET_IOCTL_GET_SUPPORTED_QOS_MODES: /* Get QoS > modes */ > + case RMNET_IOCTL_SET_SLEEP_STATE: /* Set > sleep state */ > + case RMNET_IOCTL_SET_XLAT_DEV_INFO: /* xlat > dev name */ > + case RMNET_IOCTL_DEREGISTER_DEV: /* > Deregister netdev */ > + return -ENOTSUPP; /* Defined, but unsupported > command */ > + > + default: > + return -EINVAL; /* Invalid > (unrecognized) command */ > + } > + > +copy_out: > + return copy_to_user(data, &edata, size) ? -EFAULT : 0; > +} > + > +/** ipa_wwan_ioctl() - I/O control for wwan network driver */ > +static int ipa_wwan_ioctl(struct net_device *dev, struct ifreq *ifr, > int cmd) > +{ > + struct rmnet_ioctl_data_s ioctl_data = { }; > + void __user *data; > + size_t size; > + > + data = ifr->ifr_ifru.ifru_data; > + size = sizeof(ioctl_data); > + > + switch (cmd) { > + /* These features are implied; alternatives are not > supported */ > + case RMNET_IOCTL_SET_LLP_IP: /* RAW IP > protocol */ > + case RMNET_IOCTL_SET_QOS_DISABLE: /* QoS header > disabled */ > + return 0; > + > + /* These features are not supported; use alternatives */ > + case RMNET_IOCTL_SET_LLP_ETHERNET: /* Ethernet > protocol */ > + case RMNET_IOCTL_SET_QOS_ENABLE: /* QoS header > enabled */ > + case RMNET_IOCTL_GET_OPMODE: /* Get operation > mode */ > + case RMNET_IOCTL_FLOW_ENABLE: /* Flow enable > */ > + case RMNET_IOCTL_FLOW_DISABLE: /* Flow > disable */ > + case RMNET_IOCTL_FLOW_SET_HNDL: /* Set flow > handle */ > + return -ENOTSUPP; > + > + case RMNET_IOCTL_GET_LLP: /* Get link > protocol */ > + ioctl_data.u.operation_mode = RMNET_MODE_LLP_IP; > + goto copy_out; > + > + case RMNET_IOCTL_GET_QOS: /* Get QoS header > state */ > + ioctl_data.u.operation_mode = RMNET_MODE_NONE; > + goto copy_out; > + > + case RMNET_IOCTL_OPEN: /* Open > transport port */ > + case RMNET_IOCTL_CLOSE: /* Close > transport port */ > + return 0; > + > + case RMNET_IOCTL_EXTENDED: /* Extended IOCTLs > */ > + return ipa_wwan_ioctl_extended(dev, data); > + > + default: > + return -EINVAL; > + } > + > +copy_out: > + return copy_to_user(data, &ioctl_data, size) ? -EFAULT : 0; > +} > + > +static const struct net_device_ops ipa_wwan_ops_ip = { > + .ndo_open = ipa_wwan_open, > + .ndo_stop = ipa_wwan_stop, > + .ndo_start_xmit = ipa_wwan_xmit, > + .ndo_do_ioctl = ipa_wwan_ioctl, > +}; > + > +/** wwan_setup() - Setup the wwan network driver */ > +static void ipa_wwan_setup(struct net_device *dev) > +{ > + dev->netdev_ops = &ipa_wwan_ops_ip; > + ether_setup(dev); > + dev->header_ops = NULL; /* No header (override > ether_setup() value) */ > + dev->type = ARPHRD_RAWIP; > + dev->hard_header_len = 0; > + dev->max_mtu = WWAN_DATA_LEN; > + dev->mtu = dev->max_mtu; > + dev->addr_len = 0; > + dev->flags &= ~(IFF_BROADCAST | IFF_MULTICAST); > + dev->needed_headroom = HEADROOM_FOR_QMAP; > + dev->needed_tailroom = TAILROOM; > + dev->watchdog_timeo = msecs_to_jiffies(10 * MSEC_PER_SEC); > +} > + > +/** ipa_wwan_probe() - Network probe function */ > +static int ipa_wwan_probe(struct platform_device *pdev) > +{ > + struct ipa_wwan_private *wwan_ptr; > + struct net_device *dev; > + int ret; > + > + mutex_init(&rmnet_ipa_ctx->ep_setup_mutex); > + mutex_init(&rmnet_ipa_ctx->mux_id_mutex); > + > + /* Mark client handles bad until we initialize them */ > + rmnet_ipa_ctx->wan_prod_ep_id = IPA_EP_ID_BAD; > + rmnet_ipa_ctx->wan_cons_ep_id = IPA_EP_ID_BAD; > + > + ret = ipa_modem_smem_init(); > + if (ret) > + goto err_clear_ctx; > + > + /* start A7 QMI service/client */ > + ipa_qmi_init(); > + > + /* initialize wan-driver netdev */ > + dev = alloc_netdev(sizeof(struct ipa_wwan_private), > + IPA_WWAN_DEV_NAME, > + NET_NAME_UNKNOWN, > + ipa_wwan_setup); > + if (!dev) { > + ipa_err("no memory for netdev\n"); > + ret = -ENOMEM; > + goto err_clear_ctx; > + } > + rmnet_ipa_ctx->dev = dev; > + wwan_ptr = netdev_priv(dev); > + wwan_ptr->outstanding_high_ctl = > DEFAULT_OUTSTANDING_HIGH_CTL; > + wwan_ptr->outstanding_high = DEFAULT_OUTSTANDING_HIGH; > + wwan_ptr->outstanding_low = DEFAULT_OUTSTANDING_LOW; > + atomic_set(&wwan_ptr->outstanding_pkts, 0); > + > + /* Enable SG support in netdevice. */ > + dev->hw_features |= NETIF_F_SG; > + > + netif_napi_add(dev, &wwan_ptr->napi, ipa_rmnet_poll, > NAPI_WEIGHT); > + ret = register_netdev(dev); > + if (ret) { > + ipa_err("unable to register ipa_netdev %d rc=%d\n", > 0, ret); > + goto err_napi_del; > + } > + > + /* offline charging mode */ > + ipa_proxy_clk_unvote(); > + > + /* Till the system is suspended, we keep the clock open */ > + ipa_client_add(); > + > + initialized = true; > + > + return 0; > + > +err_napi_del: > + netif_napi_del(&wwan_ptr->napi); > + free_netdev(dev); > +err_clear_ctx: > + memset(&rmnet_ipa_ctx_struct, 0, > sizeof(rmnet_ipa_ctx_struct)); > + > + return ret; > +} > + > +static int ipa_wwan_remove(struct platform_device *pdev) > +{ > + struct ipa_wwan_private *wwan_ptr = > netdev_priv(rmnet_ipa_ctx->dev); > + > + dev_info(&pdev->dev, "rmnet_ipa started > deinitialization\n"); > + > + mutex_lock(&rmnet_ipa_ctx->ep_setup_mutex); > + > + ipa_client_add(); > + > + if (rmnet_ipa_ctx->wan_cons_ep_id != IPA_EP_ID_BAD) { > + ipa_ep_teardown(rmnet_ipa_ctx->wan_cons_ep_id); > + rmnet_ipa_ctx->wan_cons_ep_id = IPA_EP_ID_BAD; > + } > + > + if (rmnet_ipa_ctx->wan_prod_ep_id != IPA_EP_ID_BAD) { > + ipa_ep_teardown(rmnet_ipa_ctx->wan_prod_ep_id); > + rmnet_ipa_ctx->wan_prod_ep_id = IPA_EP_ID_BAD; > + } > + > + ipa_client_remove(); > + > + netif_napi_del(&wwan_ptr->napi); > + mutex_unlock(&rmnet_ipa_ctx->ep_setup_mutex); > + unregister_netdev(rmnet_ipa_ctx->dev); > + > + if (rmnet_ipa_ctx->dev) > + free_netdev(rmnet_ipa_ctx->dev); > + rmnet_ipa_ctx->dev = NULL; > + > + mutex_destroy(&rmnet_ipa_ctx->mux_id_mutex); > + mutex_destroy(&rmnet_ipa_ctx->ep_setup_mutex); > + > + initialized = false; > + > + dev_info(&pdev->dev, "rmnet_ipa completed > deinitialization\n"); > + > + return 0; > +} > + > +/** rmnet_ipa_ap_suspend() - suspend callback for runtime_pm > + * @dev: pointer to device > + * > + * This callback will be invoked by the runtime_pm framework when an > AP suspend > + * operation is invoked, usually by pressing a suspend button. > + * > + * Returns -EAGAIN to runtime_pm framework in case there are pending > packets > + * in the Tx queue. This will postpone the suspend operation until > all the > + * pending packets will be transmitted. > + * > + * In case there are no packets to send, releases the WWAN0_PROD > entity. > + * As an outcome, the number of IPA active clients should be > decremented > + * until IPA clocks can be gated. > + */ > +static int rmnet_ipa_ap_suspend(struct device *dev) > +{ > + struct net_device *netdev = rmnet_ipa_ctx->dev; > + struct ipa_wwan_private *wwan_ptr; > + int ret; > + > + if (!netdev) { > + ipa_err("netdev is NULL.\n"); > + ret = 0; > + goto bail; > + } > + > + netif_tx_lock_bh(netdev); > + wwan_ptr = netdev_priv(netdev); > + if (!wwan_ptr) { > + ipa_err("wwan_ptr is NULL.\n"); > + ret = 0; > + goto unlock_and_bail; > + } > + > + /* Do not allow A7 to suspend in case there are outstanding > packets */ > + if (atomic_read(&wwan_ptr->outstanding_pkts) != 0) { > + ret = -EAGAIN; > + goto unlock_and_bail; > + } > + > + /* Make sure that there is no Tx operation ongoing */ > + netif_stop_queue(netdev); > + > + ret = 0; > + ipa_client_remove(); > + > +unlock_and_bail: > + netif_tx_unlock_bh(netdev); > +bail: > + > + return ret; > +} > + > +/** rmnet_ipa_ap_resume() - resume callback for runtime_pm > + * @dev: pointer to device > + * > + * This callback will be invoked by the runtime_pm framework when an > AP resume > + * operation is invoked. > + * > + * Enables the network interface queue and returns success to the > + * runtime_pm framework. > + */ > +static int rmnet_ipa_ap_resume(struct device *dev) > +{ > + struct net_device *netdev = rmnet_ipa_ctx->dev; > + > + ipa_client_add(); > + if (netdev) > + netif_wake_queue(netdev); > + > + return 0; > +} > + > +static const struct of_device_id rmnet_ipa_dt_match[] = { > + {.compatible = "qcom,rmnet-ipa"}, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, rmnet_ipa_dt_match); > + > +static const struct dev_pm_ops rmnet_ipa_pm_ops = { > + .suspend_noirq = rmnet_ipa_ap_suspend, > + .resume_noirq = rmnet_ipa_ap_resume, > +}; > + > +static struct platform_driver rmnet_ipa_driver = { > + .driver = { > + .name = "rmnet_ipa", > + .owner = THIS_MODULE, > + .pm = &rmnet_ipa_pm_ops, > + .of_match_table = rmnet_ipa_dt_match, > + }, > + .probe = ipa_wwan_probe, > + .remove = ipa_wwan_remove, > +}; > + > +int ipa_wwan_init(void) > +{ > + if (initialized) > + return 0; > + > + return platform_driver_register(&rmnet_ipa_driver); > +} > + > +void ipa_wwan_cleanup(void) > +{ > + platform_driver_unregister(&rmnet_ipa_driver); > + memset(&rmnet_ipa_ctx_struct, 0, > sizeof(rmnet_ipa_ctx_struct)); > +} > + > +static int ipa_rmnet_poll(struct napi_struct *napi, int budget) > +{ > + return ipa_rx_poll(rmnet_ipa_ctx->wan_cons_ep_id, budget); > +} > + > +MODULE_DESCRIPTION("WWAN Network Interface"); > +MODULE_LICENSE("GPL v2");
On 11/7/18 5:50 AM, Arnd Bergmann wrote: > On Wed, Nov 7, 2018 at 1:33 AM Alex Elder <elder@linaro.org> wrote: >> >> Add the binding definitions for the "qcom,ipa" and "qcom,rmnet-ipa" >> device tree nodes. >> >> Signed-off-by: Alex Elder <elder@linaro.org> >> --- >> .../devicetree/bindings/soc/qcom/qcom,ipa.txt | 136 ++++++++++++++++++ >> .../bindings/soc/qcom/qcom,rmnet-ipa.txt | 15 ++ >> 2 files changed, 151 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt >> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt > > I think this should go into bindings/net instead of bindings/soc, since it's > mostly about networking rather than a specific detail of managing the SoC > itself. Done (in my tree--and will be reflected next time I send something out). >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt >> new file mode 100644 >> index 000000000000..d4d3d37df029 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt >> @@ -0,0 +1,136 @@ >> +Qualcomm IPA (IP Accelerator) Driver >> + >> +This binding describes the Qualcomm IPA. The IPA is capable of offloading >> +certain network processing tasks (e.g. filtering, routing, and NAT) from >> +the main processor. The IPA currently serves only as a network interface, >> +providing access to an LTE network available via a modem. > > That doesn't belong into the binding. Say what the hardware can do here, > not what a specific implementation of the driver does at this moment. > The binding should be written in an OS independent way after all. OK. >> +- interrupts-extended: >> + Specifies the IRQs used by the IPA. Four cells are required, >> + specifying: the IPA IRQ; the GSI IRQ; the clock query interrupt >> + from the modem; and the "ready for stage 2 initialization" >> + interrupt from the modem. The first two are hardware IRQs; the >> + third and fourth are SMP2P input interrupts. > > You mean 'four interrupts', not 'four cells' -- each interrupt specifier > already consists of at least two cells (one for the phandle to the > irqchip, plus one or more cells to describe that interrupt). OK. >> +- interconnects: >> + Specifies the interconnects used by the IPA. Three cells are >> + required, specifying: the path from the IPA to memory; from >> + IPA to internal (SoC resident) memory; and between the AP >> + subsystem and IPA for register access. > > Same here and in the rest. OK, I've fixed all of these. Thank you. -Alex > > Arnd >
On 11/7/18 8:59 AM, Rob Herring wrote: > On Tue, Nov 6, 2018 at 6:33 PM Alex Elder <elder@linaro.org> wrote: >> >> Add the binding definitions for the "qcom,ipa" and "qcom,rmnet-ipa" >> device tree nodes. >> >> Signed-off-by: Alex Elder <elder@linaro.org> >> --- >> .../devicetree/bindings/soc/qcom/qcom,ipa.txt | 136 ++++++++++++++++++ >> .../bindings/soc/qcom/qcom,rmnet-ipa.txt | 15 ++ >> 2 files changed, 151 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt >> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt >> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt >> new file mode 100644 >> index 000000000000..d4d3d37df029 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt >> @@ -0,0 +1,136 @@ >> +Qualcomm IPA (IP Accelerator) Driver > > Bindings are for h/w not drivers. OK. I'll drop " Driver". >> + >> +This binding describes the Qualcomm IPA. The IPA is capable of offloading >> +certain network processing tasks (e.g. filtering, routing, and NAT) from >> +the main processor. The IPA currently serves only as a network interface, >> +providing access to an LTE network available via a modem. >> + >> +The IPA sits between multiple independent "execution environments," >> +including the AP subsystem (APSS) and the modem. The IPA presents >> +a Generic Software Interface (GSI) to each execution environment. >> +The GSI is an integral part of the IPA, but it is logically isolated >> +and has a distinct interrupt and a separately-defined address space. >> + >> + ---------- ------------- --------- >> + | | |G| |G| | | >> + | APSS |===|S| IPA |S|===| Modem | >> + | | |I| |I| | | >> + ---------- ------------- --------- >> + >> +See also: >> + bindings/interrupt-controller/interrupts.txt >> + bindings/interconnect/interconnect.txt >> + bindings/soc/qcom/qcom,smp2p.txt >> + bindings/reserved-memory/reserved-memory.txt >> + bindings/clock/clock-bindings.txt >> + >> +All properties defined below are required. >> + >> +- compatible: >> + Must be one of the following compatible strings: >> + "qcom,ipa-sdm845-modem_init" >> + "qcom,ipa-sdm845-tz_init" > > Normal order is <vendor>,<soc>-<ipblock>." I'll use "qcom,sdm845-ipa-modem-init" and "qcom,sdm845-ipa-tz-init". (Or just "qcom,sdm845-ipa", depending on the outcome of the discussion below.) > Don't use '_'. OK. > What's the difference between these 2? It can't be detected somehow? There is some early initialization, including loading some firmware, that must be done by trusted code. That can be done by either Trust Zone or the modem. If it's done by the modem, there is an additional step required during initialization so the modem can tell the AP that it has done its part, and the AP can finish IPA initialization. There is no way of detecting (e.g. by probing hardware) which is in effect so we use DT. I discussed this with Bjorn, who said that this was a situation seen elsewhere and that using compatible strings was the way he suggested to address it. > This might be better expressed as a property. Then if Trustzone > initializes things, it can just add a property. A Boolean property to distinguish them would be fine as well, but I would like to address this "common" problem consistently. Bjorn, would you please weigh in? >> + >> +-reg: >> + Resources specyfing the physical address spaces of the IPA and GSI. > > typo > >> + >> +-reg-names: >> + The names of the address space ranges defined by the "reg" property. >> + Must be "ipa" and "gsi". >> + >> +- interrupts-extended: > > Use 'interrupts' here and describe what they are and the order. What > they are connected to (and the need for interrupts-extended) is > outside the scope of this binding. I used interrupts-extended because there were two interrupt parents (a "normal" interrupt controller and the interrupt controller implemented for SMP2P input). A paragraph here: bindings/interrupt-controller/interrupts.txt recommends "interrupts-extended" in that case. I have no objection to using just "interrupts" but can you tell me what I misunderstood? It seems like I need to do "interrupts-extended". >> + Specifies the IRQs used by the IPA. Four cells are required, >> + specifying: the IPA IRQ; the GSI IRQ; the clock query interrupt >> + from the modem; and the "ready for stage 2 initialization" >> + interrupt from the modem. The first two are hardware IRQs; the >> + third and fourth are SMP2P input interrupts. >> + >> +- interrupt-names: >> + The names of the interrupts defined by the "interrupts-extended" >> + property. Must be "ipa", "gsi", "ipa-clock-query", and >> + "ipa-post-init". > > Format as one per line. Done. And I did this throughout the file where there was more than one name. One per line, no comma, no "and". >> + >> +- clocks: >> + Resource that defines the IPA core clock. >> + >> +- clock-names: >> + The name used for the IPA core clock. Must be "core". >> + >> +- interconnects: >> + Specifies the interconnects used by the IPA. Three cells are >> + required, specifying: the path from the IPA to memory; from >> + IPA to internal (SoC resident) memory; and between the AP >> + subsystem and IPA for register access. >> + >> +- interconnect-names: >> + The names of the interconnects defined by the "interconnects" >> + property. Must be "memory", "imem", and "config". >> + >> +- qcom,smem-states >> + The state bits used for SMP2P output. Two cells must be specified. >> + The first indicates whether the value in the second bit is valid >> + (1 means valid). The second, if valid, defines whether the IPA >> + clock is enabled (1 means enabled). >> + >> +- qcom,smem-state-names >> + The names of the state bits used for SMP2P output. These must be >> + "ipa-clock-enabled-valid" and "ipa-clock-enabled". >> + >> +- memory-region >> + A phandle for a reserved memory area that holds the firmware passed >> + to Trust Zone for authentication. (Note, this is required >> + only for "qcom,ipa-sdm845-tz_init".) >> + >> += EXAMPLE >> + >> +The following example represents the IPA present in the SDM845 SoC. It >> +shows portions of the "modem-smp2p" node to indicate its relationship >> +with the interrupts and SMEM states used by the IPA. >> + >> + modem-smp2p { >> + compatible = "qcom,smp2p"; >> + . . . >> + ipa_smp2p_out: ipa-ap-to-modem { >> + qcom,entry-name = "ipa"; >> + #qcom,smem-state-cells = <1>; >> + }; >> + >> + ipa_smp2p_in: ipa-modem-to-ap { >> + qcom,entry-name = "ipa"; >> + interrupt-controller; >> + #interrupt-cells = <2>; >> + }; >> + }; >> + >> + ipa@1e00000 { > > ipa@1e40000 Oops. Fixed. >> + compatible = "qcom,ipa-sdm845-modem_init"; >> + >> + reg = <0x1e40000 0x34000>, >> + <0x1e04000 0x2c000>; >> + reg-names = "ipa", >> + "gsi"; >> + >> + interrupts-extended = <&intc 0 311 IRQ_TYPE_LEVEL_HIGH>, >> + <&intc 0 432 IRQ_TYPE_LEVEL_HIGH>, >> + <&ipa_smp2p_in 0 IRQ_TYPE_EDGE_RISING>, >> + <&ipa_smp2p_in 1 IRQ_TYPE_EDGE_RISING>; >> + interrupt-names = "ipa", >> + "gsi", >> + "ipa-clock-query", >> + "ipa-post-init"; >> + >> + clocks = <&rpmhcc RPMH_IPA_CLK>; >> + clock-names = "core"; >> + >> + interconnects = <&qnoc MASTER_IPA &qnoc SLAVE_EBI1>, >> + <&qnoc MASTER_IPA &qnoc SLAVE_IMEM>, >> + <&qnoc MASTER_APPSS_PROC &qnoc SLAVE_IPA_CFG>; >> + interconnect-names = "memory", >> + "imem", >> + "config"; >> + >> + qcom,smem-states = <&ipa_smp2p_out 0>, >> + <&ipa_smp2p_out 1>; >> + qcom,smem-state-names = "ipa-clock-enabled-valid", >> + "ipa-clock-enabled"; >> + }; >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt >> new file mode 100644 >> index 000000000000..3d0b2aabefc7 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt >> @@ -0,0 +1,15 @@ >> +Qualcomm IPA RMNet Driver >> + >> +This binding describes the IPA RMNet driver, which is used to >> +represent virtual interfaces available on the modem accessed via >> +the IPA. Other than the compatible string there are no properties >> +associated with this device. > > Only a compatible string is a sure sign this is not a h/w device and > you are just abusing DT to instantiate drivers. Make the IPA driver > instantiate any sub drivers it needs. Yeah I have been thinking this but hadn't followed through on doing anything about it yet. I'll remove this node entirely. It's possible it had other properties at one time, but in the end this represents a soft interface and can be implemented within the IPA driver. Thanks a lot for the review. -Alex > > Rob >
On 11/7/18 8:59 AM, Rob Herring wrote: > On Tue, Nov 6, 2018 at 6:33 PM Alex Elder <elder@linaro.org> wrote: >> >> Add the binding definitions for the "qcom,ipa" and "qcom,rmnet-ipa" >> device tree nodes. >> >> Signed-off-by: Alex Elder <elder@linaro.org> Rob, I'm just following up to let you know that I have now addressed all of your suggestions in my current code. This includes the removal of the "qcom,rmnet-ipa" DT node. The driver that was set up to match that node is now gone from the code, and this entity is no longer represented by a "real" device. It implements a network device that represents the modem, and that is now set up and torn down as needed by the "main" IPA code. The one thing that might need to be addressed is whether a Boolean flag should be used rather than a different compatible string to select whether the modem or TZ is responsible for GSI firmware load and launch. If you and Bjorn agree it should be done with a flag instead I'll fix the binding and code accordingly. -Alex >> --- >> .../devicetree/bindings/soc/qcom/qcom,ipa.txt | 136 ++++++++++++++++++ >> .../bindings/soc/qcom/qcom,rmnet-ipa.txt | 15 ++ >> 2 files changed, 151 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt >> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt >> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt >> new file mode 100644 >> index 000000000000..d4d3d37df029 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt >> @@ -0,0 +1,136 @@ >> +Qualcomm IPA (IP Accelerator) Driver > > Bindings are for h/w not drivers. > >> + >> +This binding describes the Qualcomm IPA. The IPA is capable of offloading >> +certain network processing tasks (e.g. filtering, routing, and NAT) from >> +the main processor. The IPA currently serves only as a network interface, >> +providing access to an LTE network available via a modem. >> + >> +The IPA sits between multiple independent "execution environments," >> +including the AP subsystem (APSS) and the modem. The IPA presents >> +a Generic Software Interface (GSI) to each execution environment. >> +The GSI is an integral part of the IPA, but it is logically isolated >> +and has a distinct interrupt and a separately-defined address space. >> + >> + ---------- ------------- --------- >> + | | |G| |G| | | >> + | APSS |===|S| IPA |S|===| Modem | >> + | | |I| |I| | | >> + ---------- ------------- --------- >> + >> +See also: >> + bindings/interrupt-controller/interrupts.txt >> + bindings/interconnect/interconnect.txt >> + bindings/soc/qcom/qcom,smp2p.txt >> + bindings/reserved-memory/reserved-memory.txt >> + bindings/clock/clock-bindings.txt >> + >> +All properties defined below are required. >> + >> +- compatible: >> + Must be one of the following compatible strings: >> + "qcom,ipa-sdm845-modem_init" >> + "qcom,ipa-sdm845-tz_init" > > Normal order is <vendor>,<soc>-<ipblock>. > > Don't use '_'. > > What's the difference between these 2? It can't be detected somehow? > This might be better expressed as a property. Then if Trustzone > initializes things, it can just add a property. > >> + >> +-reg: >> + Resources specyfing the physical address spaces of the IPA and GSI. > > typo > >> + >> +-reg-names: >> + The names of the address space ranges defined by the "reg" property. >> + Must be "ipa" and "gsi". >> + >> +- interrupts-extended: > > Use 'interrupts' here and describe what they are and the order. What > they are connected to (and the need for interrupts-extended) is > outside the scope of this binding. > >> + Specifies the IRQs used by the IPA. Four cells are required, >> + specifying: the IPA IRQ; the GSI IRQ; the clock query interrupt >> + from the modem; and the "ready for stage 2 initialization" >> + interrupt from the modem. The first two are hardware IRQs; the >> + third and fourth are SMP2P input interrupts. >> + >> +- interrupt-names: >> + The names of the interrupts defined by the "interrupts-extended" >> + property. Must be "ipa", "gsi", "ipa-clock-query", and >> + "ipa-post-init". > > Format as one per line. > >> + >> +- clocks: >> + Resource that defines the IPA core clock. >> + >> +- clock-names: >> + The name used for the IPA core clock. Must be "core". >> + >> +- interconnects: >> + Specifies the interconnects used by the IPA. Three cells are >> + required, specifying: the path from the IPA to memory; from >> + IPA to internal (SoC resident) memory; and between the AP >> + subsystem and IPA for register access. >> + >> +- interconnect-names: >> + The names of the interconnects defined by the "interconnects" >> + property. Must be "memory", "imem", and "config". >> + >> +- qcom,smem-states >> + The state bits used for SMP2P output. Two cells must be specified. >> + The first indicates whether the value in the second bit is valid >> + (1 means valid). The second, if valid, defines whether the IPA >> + clock is enabled (1 means enabled). >> + >> +- qcom,smem-state-names >> + The names of the state bits used for SMP2P output. These must be >> + "ipa-clock-enabled-valid" and "ipa-clock-enabled". >> + >> +- memory-region >> + A phandle for a reserved memory area that holds the firmware passed >> + to Trust Zone for authentication. (Note, this is required >> + only for "qcom,ipa-sdm845-tz_init".) >> + >> += EXAMPLE >> + >> +The following example represents the IPA present in the SDM845 SoC. It >> +shows portions of the "modem-smp2p" node to indicate its relationship >> +with the interrupts and SMEM states used by the IPA. >> + >> + modem-smp2p { >> + compatible = "qcom,smp2p"; >> + . . . >> + ipa_smp2p_out: ipa-ap-to-modem { >> + qcom,entry-name = "ipa"; >> + #qcom,smem-state-cells = <1>; >> + }; >> + >> + ipa_smp2p_in: ipa-modem-to-ap { >> + qcom,entry-name = "ipa"; >> + interrupt-controller; >> + #interrupt-cells = <2>; >> + }; >> + }; >> + >> + ipa@1e00000 { > > ipa@1e40000 > >> + compatible = "qcom,ipa-sdm845-modem_init"; >> + >> + reg = <0x1e40000 0x34000>, >> + <0x1e04000 0x2c000>; >> + reg-names = "ipa", >> + "gsi"; >> + >> + interrupts-extended = <&intc 0 311 IRQ_TYPE_LEVEL_HIGH>, >> + <&intc 0 432 IRQ_TYPE_LEVEL_HIGH>, >> + <&ipa_smp2p_in 0 IRQ_TYPE_EDGE_RISING>, >> + <&ipa_smp2p_in 1 IRQ_TYPE_EDGE_RISING>; >> + interrupt-names = "ipa", >> + "gsi", >> + "ipa-clock-query", >> + "ipa-post-init"; >> + >> + clocks = <&rpmhcc RPMH_IPA_CLK>; >> + clock-names = "core"; >> + >> + interconnects = <&qnoc MASTER_IPA &qnoc SLAVE_EBI1>, >> + <&qnoc MASTER_IPA &qnoc SLAVE_IMEM>, >> + <&qnoc MASTER_APPSS_PROC &qnoc SLAVE_IPA_CFG>; >> + interconnect-names = "memory", >> + "imem", >> + "config"; >> + >> + qcom,smem-states = <&ipa_smp2p_out 0>, >> + <&ipa_smp2p_out 1>; >> + qcom,smem-state-names = "ipa-clock-enabled-valid", >> + "ipa-clock-enabled"; >> + }; >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt >> new file mode 100644 >> index 000000000000..3d0b2aabefc7 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt >> @@ -0,0 +1,15 @@ >> +Qualcomm IPA RMNet Driver >> + >> +This binding describes the IPA RMNet driver, which is used to >> +represent virtual interfaces available on the modem accessed via >> +the IPA. Other than the compatible string there are no properties >> +associated with this device. > > Only a compatible string is a sure sign this is not a h/w device and > you are just abusing DT to instantiate drivers. Make the IPA driver > instantiate any sub drivers it needs. > > Rob >
On 11/7/18 9:00 AM, Arnd Bergmann wrote: > On Wed, Nov 7, 2018 at 1:33 AM Alex Elder <elder@linaro.org> wrote: >> diff --git a/drivers/net/ipa/ipa_reg.c b/drivers/net/ipa/ipa_reg.c >> new file mode 100644 >> index 000000000000..5e0aa6163235 >> --- /dev/null >> +++ b/drivers/net/ipa/ipa_reg.c >> @@ -0,0 +1,972 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> + >> +/* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved. >> + * Copyright (C) 2018 Linaro Ltd. >> + */ >> + >> +#include <linux/types.h> >> +#include <linux/io.h> >> +#include <linux/bitfield.h> >> + >> +#include "ipa_reg.h" >> + >> +/* I/O remapped base address of IPA register space */ >> +static void __iomem *ipa_reg_virt; > > This should of course be part of the device structure. Yes, this should have been be in that structure to begin with. I'm working through doing a comprehensive replacement of global variables like this with values passed around as arguments. I've intended to do it all along but your nudge pushed it to the top of my list. It's a *lot* of work, much more than I realized. But I'm making rapid progress. >> +/* struct ipa_reg_desc - descriptor for an abstracted hardware register >> + * >> + * @construct - fn to construct the register value from its field structure >> + * @parse - function to parse register field values into its field structure >> + * @offset - register offset relative to base address >> + * @n_ofst - size multiplier for "N-parameterized" registers >> + */ >> +struct ipa_reg_desc { >> + u32 (*construct)(enum ipa_reg reg, const void *fields); >> + void (*parse)(enum ipa_reg reg, void *fields, u32 val); >> + u32 offset; >> + u16 n_ofst; >> +}; > > Indirect function pointers can be a bit expensive in the post-spectre > days. It's probably not overly important if these are always part of > an MMIO access function, but you should be careful about using > these in the data path. OK. There used to be a more elaborate scheme for supporting lots of versions, and I have tried to preserve at least part of the underlying mechanism (the parse and construct functions). Not all of these registers use the indirect functions, and it looks to me like none of them are in the data path. The most important registers for the fast path are found in the GSI code. And that doesn't use this construct--it only reads and writes 32-bit registers. (I think it differs because it was originally developed by a different team.) > How many different versions do we have to support in practice I don't know for sure how many versions really were used, but the original code had about 10 distinct version values, many of which shared most (but not all) register definitions (offset and bit field widths) with older versions. With the upstream code the decision was made to ignore anything older than IPA version 3 (and 3.5.1 in particular, which is found in the SDM845 SoC). It may be that this parse/construct mechanism isn't justified at this point. I thought the way it presented a generic interface was useful, but with just one (initial) hardware target we don't (yet) realize its potential benefit. It could be added back later, as support for new versions is added. As of now I don't plan to change this, but if you or someone else feels it would be better without it I can do that. -Alex > Arnd >
On 11/7/18 8:08 AM, Arnd Bergmann wrote: > On Wed, Nov 7, 2018 at 1:33 AM Alex Elder <elder@linaro.org> wrote: > >> +static void ipa_client_remove_deferred(struct work_struct *work); > > Try to avoid forward declarations by reordering the code in call order, > it will also make it easier to read. > >> +static DECLARE_WORK(ipa_client_remove_work, ipa_client_remove_deferred); Done. I've actually reworked this a lot, and pulled all the clock (and interconnect) related code into a separate source file. No more forward declarations (there anyway), and the work structure is now embedded in the top-level IPA structure so I can derive it again in the work function (rather than using the global). >> +static struct ipa_context ipa_ctx_struct; >> +struct ipa_context *ipa_ctx = &ipa_ctx_struct; > > Global state variables should generally be removed as well, and > passed around as function arguments. Working on this. >> +static int hdr_init_local_cmd(u32 offset, u32 size) >> +{ >> + struct ipa_desc desc = { }; >> + struct ipa_dma_mem mem; >> + void *payload; >> + int ret; >> + >> + if (ipa_dma_alloc(&mem, size, GFP_KERNEL)) >> + return -ENOMEM; >> + >> + offset += ipa_ctx->smem_offset; >> + >> + payload = ipahal_hdr_init_local_pyld(&mem, offset); >> + if (!payload) { >> + ret = -ENOMEM; >> + goto err_dma_free; >> + } >> + >> + desc.type = IPA_IMM_CMD_DESC; >> + desc.len_opcode = IPA_IMM_CMD_HDR_INIT_LOCAL; >> + desc.payload = payload; >> + >> + ret = ipa_send_cmd(&desc); > > You have a bunch of dynamic allocations in here, which you > then immediately tear down again after the command is complete. > I can't see at all what you do with the DMA address, since you > seem to not use the virtual address at all but only store > the physical address in some kind of descriptor without ever > writing to it. I should probably have added at least a comment here. The DMA memory was zeroed at the time of allocation. That zero buffer is then referred to in the payload to the HDR_INIT_LOCAL immediate command. So that command, when executing in the IPA hardware, uses the contents of the buffer whose physical address it's supplied, which in this case is full of zeroes. We don't use the virtual address because the buffer came pre-zeroed. Based on your comment elsewhere I will be putting the command payload in a structure on the stack rather than allocating it dynamically. > Am I missing something here? > >> +/* Remoteproc callbacks for SSR events: prepare, start, stop, unprepare */ >> +int ipa_ssr_prepare(struct rproc_subdev *subdev) >> +{ >> + printk("======== SSR prepare received ========\n"); > > I think you mean dev_dbg() here. A plain printk() without a level > is not correct and we probably don't want those messages to arrive > on the console for normal users. Yes, this was obviously a debug message in some code I should have removed before sending... >> +static int ipa_firmware_load(struct de >> + >> +err_clear_dev: >> + ipa_ctx->lan_cons_ep_id = 0; >> + ipa_ctx->cmd_prod_ep_id = 0; >> + ipahal_exit(); >> +err_dma_exit: >> + ipa_dma_exit(); >> +err_clear_gsi: >> + ipa_ctx->gsi = NULL; >> + ipa_ctx->ipa_phys = 0; >> + ipa_reg_exit(); >> +err_clear_ipa_irq: >> + ipa_ctx->ipa_irq = 0; >> +err_clear_filter_bitmap: >> + ipa_ctx->filter_bitmap = 0; >> +err_interconnect_exit: >> + ipa_interconnect_exit(); >> +err_clock_exit: >> + ipa_clock_exit(); >> + ipa_ctx->dev = NULL; >> +out_smp2p_exit: >> + ipa_smp2p_exit(dev); >> + > > No need to initialize members to zero when you are about > to free the structure. The IPA context is in fact a global, static structure at the moment. All of this bookkeeping (zeroing out things) is a habitual practice, basically. Regardless your point is good and I'll remove these kinds of things as part of converting to not using globals. >> +static struct platform_driver ipa_plat_drv = { >> + .probe = ipa_plat_drv_probe, >> + .remove = ipa_plat_drv_remove, >> + .driver = { >> + .name = "ipa", >> + .owner = THIS_MODULE, >> + .pm = &ipa_pm_ops, >> + .of_match_table = ipa_plat_drv_match, >> + }, >> +}; >> + >> +builtin_platform_driver(ipa_plat_drv); > > This should be module_platform_driver(), and allow unloading > the driver. Yes. I've done this for my own use, but the code is not currently able to shut down cleanly. I've been fixing a *lot* of the things that don't clean up after themselves today but there's more work before I can say this can be safely built as a module. But it's a requirement. -Alex > > Arnd >
This series presents the driver for the Qualcomm IP Accelerator (IPA). The IPA is a hardware component present in some Qualcomm SoCs that allows network functions--such as routing, filtering, network address translation and aggregation--to be performed without active involvement of the main application processor (AP). Initially, these advanced features are not supported; the IPA driver simply provides a network interface that makes the modem's LTE network available to the AP. In addition, support is only provided for the IPA found in the Qualcomm SDM845 SoC. This code is derived from a driver developed internally by Qualcomm. A version of the original source can be seen here: https://source.codeaurora.org/quic/la/kernel/msm-4.9/tree in the "drivers/platform/msm/ipa" directory. Many were involved in developing this, but the following individuals deserve explicit acknowledgement for their substantial contributions: Abhishek Choubey Ady Abraham Chaitanya Pratapa David Arinzon Ghanim Fodi Gidon Studinski Ravi Gummadidala Shihuan Liu Skylar Chang The code has undergone considerable rework to prepare it for incorporation into upstream Linux. Parts of it bear little resemblance to the original driver. Still, some work remains to be done. The current code and its design had a preliminary review, and some changes to the data path implementation were recommended. These have not yet been addressed: - Use NAPI for all interfaces, not just RX (and WAN data) endpoints. - Do more work in the NAPI poll function, including collecting completed TX requests and posting buffers for RX. - Do not use periodic NOP requests as a way to avoid TX interrupts. - The NAPI context should be associated with the hardware interrupt (it is now associated with something abstracted from the hardware). - Use threaded interrupts, to avoid the need for using spinlocks and atomic variables for synchronizing between workqueue and interrupt context. - Have runtime power management enable and disable IPA clock and interconnects. Many thanks to Arnd Bergmann, Ilias Apalodimas, and Bjorn Andersson for their early feedback. While there clearly remains work to do on this, we felt that things are far enough along that it would be helpful to solicit broader input on the code. Major issues are best addressed as soon as possible, and even minor issues when identified help in setting priorities. This code is dependent on the following two sets of code, which have been posted for review but are not yet accepted upstream: - Interconnect framework: https://lkml.org/lkml/2018/8/31/444 - SDM845 interconnect provider driver: https://lkml.org/lkml/2018/8/24/25 In addition, it depends on four more bits of code that have not yet been posted for upstream review, but are expected to be available soon: - clk-rpmh support for IPA from David Dai <daidavid1@codeaurora.org> - SDM845 reserved memory from Bjorn Andersson <bjorn.andersson@linaro.org> - list_cut_end() from Alex Elder <elder@linaro.org> - FIELD_MAX() in "bitfield.h" from Alex Elder <elder@linaro.org> This code (including its dependencies) is available in buildable form here, based on kernel v4.19: remote: ssh://git@git.linaro.org/people/alex.elder/linux.git branch: qualcomm_ipa-v1 59562facd61a arm64: dts: sdm845: add IPA information -Alex Alex Elder (12): dt-bindings: soc: qcom: add IPA bindings soc: qcom: ipa: DMA helpers soc: qcom: ipa: generic software interface soc: qcom: ipa: immediate commands soc: qcom: ipa: IPA interrupts and the microcontroller soc: qcom: ipa: QMI modem communication soc: qcom: ipa: IPA register abstraction soc: qcom: ipa: utility functions soc: qcom: ipa: main IPA source file soc: qcom: ipa: data path soc: qcom: ipa: IPA rmnet interface soc: qcom: ipa: build and "ipa_i.h" .../devicetree/bindings/soc/qcom/qcom,ipa.txt | 136 ++ .../bindings/soc/qcom/qcom,rmnet-ipa.txt | 15 + drivers/net/ipa/Kconfig | 30 + drivers/net/ipa/Makefile | 7 + drivers/net/ipa/gsi.c | 1685 ++++++++++++++ drivers/net/ipa/gsi.h | 195 ++ drivers/net/ipa/gsi_reg.h | 563 +++++ drivers/net/ipa/ipa_dma.c | 61 + drivers/net/ipa/ipa_dma.h | 61 + drivers/net/ipa/ipa_dp.c | 1994 +++++++++++++++++ drivers/net/ipa/ipa_i.h | 573 +++++ drivers/net/ipa/ipa_interrupts.c | 307 +++ drivers/net/ipa/ipa_main.c | 1400 ++++++++++++ drivers/net/ipa/ipa_qmi.c | 406 ++++ drivers/net/ipa/ipa_qmi.h | 12 + drivers/net/ipa/ipa_qmi_msg.c | 587 +++++ drivers/net/ipa/ipa_qmi_msg.h | 233 ++ drivers/net/ipa/ipa_reg.c | 972 ++++++++ drivers/net/ipa/ipa_reg.h | 614 +++++ drivers/net/ipa/ipa_uc.c | 336 +++ drivers/net/ipa/ipa_utils.c | 1035 +++++++++ drivers/net/ipa/ipahal.c | 541 +++++ drivers/net/ipa/ipahal.h | 253 +++ drivers/net/ipa/msm_rmnet.h | 120 + drivers/net/ipa/rmnet_config.h | 31 + drivers/net/ipa/rmnet_ipa.c | 805 +++++++ 26 files changed, 12972 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt create mode 100644 drivers/net/ipa/Kconfig create mode 100644 drivers/net/ipa/Makefile create mode 100644 drivers/net/ipa/gsi.c create mode 100644 drivers/net/ipa/gsi.h create mode 100644 drivers/net/ipa/gsi_reg.h create mode 100644 drivers/net/ipa/ipa_dma.c create mode 100644 drivers/net/ipa/ipa_dma.h create mode 100644 drivers/net/ipa/ipa_dp.c create mode 100644 drivers/net/ipa/ipa_i.h create mode 100644 drivers/net/ipa/ipa_interrupts.c create mode 100644 drivers/net/ipa/ipa_main.c create mode 100644 drivers/net/ipa/ipa_qmi.c create mode 100644 drivers/net/ipa/ipa_qmi.h create mode 100644 drivers/net/ipa/ipa_qmi_msg.c create mode 100644 drivers/net/ipa/ipa_qmi_msg.h create mode 100644 drivers/net/ipa/ipa_reg.c create mode 100644 drivers/net/ipa/ipa_reg.h create mode 100644 drivers/net/ipa/ipa_uc.c create mode 100644 drivers/net/ipa/ipa_utils.c create mode 100644 drivers/net/ipa/ipahal.c create mode 100644 drivers/net/ipa/ipahal.h create mode 100644 drivers/net/ipa/msm_rmnet.h create mode 100644 drivers/net/ipa/rmnet_config.h create mode 100644 drivers/net/ipa/rmnet_ipa.c -- 2.17.1