| Message ID | 20211011141733.3999-1-stephan@gerhold.net |
|---|---|
| Headers | show |
| Series | net: wwan: Add Qualcomm BAM-DMUX WWAN network driver | expand |
On Mon, Oct 11, 2021 at 04:17:36PM +0200, Stephan Gerhold wrote: > The BAM Data Multiplexer provides access to the network data channels of > modems integrated into many older Qualcomm SoCs, e.g. Qualcomm MSM8916 or > MSM8974. It is built using a simple protocol layer on top of a DMA engine > (Qualcomm BAM) and bidirectional interrupts to coordinate power control. > > The modem announces a fixed set of channels by sending an OPEN command. > The driver exports each channel as separate network interface so that > a connection can be established via QMI from userspace. The network > interface can work either in Ethernet or Raw-IP mode (configurable via > QMI). However, Ethernet mode seems to be broken with most firmwares > (network packets are actually received as Raw-IP), therefore the driver > only supports Raw-IP mode. > > Note that the control channel (QMI/AT) is entirely separate from > BAM-DMUX and is already supported by the RPMSG_WWAN_CTRL driver. > > The driver uses runtime PM to coordinate power control with the modem. > TX/RX buffers are put in a kind of "ring queue" and submitted via > the bam_dma driver of the DMAEngine subsystem. > > The basic architecture looks roughly like this: > > +------------+ +-------+ > [IPv4/6] | BAM-DMUX | | | > [Data...] | | | | > ---------->|wwan0 | [DMUX chan: x] | | > [IPv4/6] | (chan: 0) | [IPv4/6] | | > [Data...] | | [Data...] | | > ---------->|wwan1 |--------------->| Modem | > | (chan: 1) | BAM | | > [IPv4/6] | ... | (DMA Engine) | | > [Data...] | | | | > ---------->|wwan7 | | | > | (chan: 7) | | | > +------------+ +-------+ > > However, on newer SoCs/firmware versions Qualcomm began gradually moving > to QMAP (rmnet driver) as backend-independent protocol for multiplexing > and data aggegration. Some firmware versions allow using QMAP on top of > BAM-DMUX (effectively resulting in a second multiplexing layer plus data > aggregation). The architecture with QMAP would look roughly like this: > > +-------------+ +------------+ +-------+ > [IPv4/6] | RMNET | | BAM-DMUX | | | > [Data...] | | | | [DMUX chan: 0] | | > ---------->|rmnet_data1 | ----->|wwan0 | [QMAP mux-id: x] | | > | (mux-id: 1) | | | (chan: 0) | [IPv4/6] | | > | | | | | [Data...] | | > [IPv4/6] | ... |------ | |----------------->| Modem | > [Data...] | | | | BAM | | > ---------->|rmnet_data42 | [QMAP: x] |[wwan1] | (DMA Engine) | | > | (mux-id: 42)| [IPv4/6] |... unused! | | | > | | [Data...] |[wwan7] | | | > | | | | | | > +-------------+ +------------+ +-------+ > > In this case, wwan1-7 would remain unused. The firmware used on the most > recent SoCs with BAM-DMUX even seems to announce only a single BAM-DMUX > channel (wwan0), which makes QMAP the only option for multiplexing there. > > However, so far the driver is mainly tested without QMAP, on various > smartphones/tablets based on Qualcomm MSM8916/MSM8974. It looks like QMAP > depends on a MTU negotiation feature in BAM-DMUX which is not supported yet. > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > --- > Note that this is my first network driver, so I apologize in advance > if I made some obvious mistakes. :) > > Changes since RFC: > - Rebase on net-next and fix conflicts > - Rename network interfaces from "rmnet%d" -> "wwan%d" > - Fix wrong file name in MAINTAINERS entry > - Clarify control channel in commit message. (It is entirely independent > of BAM-DMUX and is already supported by the RPMSG WWAN CTRL driver.) > > Like in the RFC version [1], the driver does not currently use the link > management of the WWAN subsystem. Instead, it simply exposes one network > interface for each of the up to 8 channels. > > This setup works out of the box with all available open-source userspace > WWAN implementations, especially ModemManager (no changes needed). > oFono works too although it requires minor changes to support WWAN control > ports (/dev/wwan0qmi0) which are independent of BAM-DMUX (already provided > by the "rpmsg_wwan_ctrl" driver). > It was easy to support because the setup is very similar to ones already > supported for USB modems. Some of them provide multiple network interfaces > and ModemManager can bundle them together to a single modem. > > I believe it is best to keep this setup as-is for now and not add even > more complexity to userspace with another setup that works only in this > particular configuration. I will reply to this patch separately to explain > that a bit more clearly. This patch is already long enough as-is. :) > > [1]: https://lore.kernel.org/netdev/20210719145317.79692-5-stephan@gerhold.net/ > The main goal of the WWAN link management is to make the multiplexing setup transparent to userspace. Unfortunately it's still unclear to me how or even if this can be achieved for the many different different setups that exist for Qualcomm modems. To show that more clearly I'll "briefly" list the various currently supported setups in ModemManager (there might be even more that I am not even aware of). The details are not too important, this list only exists to show the complexity that is already handled in ModemManager: Control ports (QMI/AT/MBIM): *1. Preferred: WWAN subsystem control port (chardev)* - cdc-wdm - rpmsg_wwan_ctrl (most common setup for BAM-DMUX) - mhi_wwan_ctrl 2. QRTR network sockets (net/qrtr) on newer Qualcomm SoCs (most common setup for IPA (drivers/net/ipa) (3. Legacy: Driver-specific char devices) - cdc-wdm usbmisc chardev - rpmsg-char Network interfaces: *1. Preferred: WWAN subsystem link management* - mhi_wwan_mbim (only for MHI+MBIM, not MHI+QMI) - (iosm, but that's not Qualcomm) 2. Single/multiple exposed network interfaces - USB modems - qcom_bam_dmux (this patch) 3. qmimux (built-in multiplexing of qmi_wwan driver) - qmi_wwan 4. "rmnet" links created via netlink, works on top of: Note: Various different "rmnet" versions and configurations exist that need to be configured appropriately. - qmi_wwan, optional - IPA (drivers/net/ipa), always required - qcom_bam_dmux, optional (supported only on very recent firmware versions/SoCs) ModemManager already provides an unified API for all these in userspace. The goal for the WWAN subsystem would be to unify all these approaches in the kernel, to simplify userspace. We have *partially* achieved this for the QMI/AT control ports where there are multiple backends with the same frontend now (USB cdc-wdm, RPMSG, MHI). But not fully, new Qualcomm SoCs require controlling the modem via QRTR network sockets and I'm not sure if this can be mapped to the WWAN subsystem chardevs. The "partially" means that userspace still needs to support multiple approaches, and usually needs to keep supporting old approaches for compatibility with old kernels anyway. And for the network interfaces it is even more unclear to me if there is a good way to abstract those transparently to userspace. The QMI configuration that is currently done in userspace is quite specific to hardware/firmware version [2]. Sergey suggested to address some of these points by moving the QMI setup to the the kernel [3]. Unfortunately, this comes with a lot of complexity, which is why this was purposefully left up to userspace when the qmi_wwan driver (for USB modems) was added to the kernel [4]. All in all, I believe finding a solution that can cover all the setups listed above is a huge undertaking, for both kernel and userspace. It goes way beyond the scope of this patch and is therefore better addressed separately. This is why I think it's best to keep the current setup in this patch for now (which is already supported by userspace!), and investigate unifying the interface separately. Thanks for reading. :) Stephan [2]: https://lore.kernel.org/netdev/YPmRcBXpRtKKSDl8@gerhold.net/ [3]: https://lore.kernel.org/netdev/CAHNKnsQr4Ys8q3Ctru-H=L3ZDwb__2D3E08mMZchDLAs1KetAg@mail.gmail.com/ [4]: https://lore.kernel.org/netdev/CAAP7ucLDEoJzwNvWLCWyCNE+kKBDn4aBU-9XT_Uv_yetnX4h-g@mail.gmail.com/
On Mon 11 Oct 07:17 PDT 2021, Stephan Gerhold wrote: > In some configurations, the BAM DMA controller is set up by a remote > processor and the local processor can simply start making use of it > without setting up the BAM. This is already supported using the > "qcom,controlled-remotely" property. > > However, for some reason another possible configuration is that the > remote processor is responsible for powering up the BAM, but we are > still responsible for initializing it (e.g. resetting it etc). > > This configuration is quite challenging to handle properly because > the power control is handled through separate channels > (e.g. device-specific SMSM interrupts / smem-states). Great care > must be taken to ensure the BAM registers are not accessed while > the BAM is powered off since this results in a bus stall. > > Attempt to support this configuration with minimal device-specific > code in the bam_dma driver by tracking the number of requested > channels. Consumers of DMA channels are responsible to only request > DMA channels when the BAM was powered on by the remote processor, > and to release them before the BAM is powered off. > > When the first channel is requested the BAM is initialized (reset) > and it is also put into reset when the last channel was released. > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > --- > Changes since RFC: > - Drop qcom-specific terminology "power collapse", instead rename > "qcom,remote-power-collapse" -> "qcom,powered-remotely" > > NOTE: This is *not* a compile-time requirement for the BAM-DMUX driver > so this could also go through the dmaengine tree. > > See original RFC for a discussion of alternative approaches to handle > this configuration: > https://lore.kernel.org/netdev/20210719145317.79692-3-stephan@gerhold.net/ > --- > drivers/dma/qcom/bam_dma.c | 88 ++++++++++++++++++++++++-------------- > 1 file changed, 56 insertions(+), 32 deletions(-) > > diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c > index c8a77b428b52..1b33a3ebbfec 100644 > --- a/drivers/dma/qcom/bam_dma.c > +++ b/drivers/dma/qcom/bam_dma.c > @@ -388,6 +388,8 @@ struct bam_device { > /* execution environment ID, from DT */ > u32 ee; > bool controlled_remotely; > + bool powered_remotely; > + u32 active_channels; > > const struct reg_offset_data *layout; > > @@ -415,6 +417,44 @@ static inline void __iomem *bam_addr(struct bam_device *bdev, u32 pipe, > r.ee_mult * bdev->ee; > } > > +/** > + * bam_reset - reset and initialize BAM registers Please include a set of () after the function name. > + * @bdev: bam device > + */ > +static void bam_reset(struct bam_device *bdev) > +{ > + u32 val; > + > + /* s/w reset bam */ > + /* after reset all pipes are disabled and idle */ > + val = readl_relaxed(bam_addr(bdev, 0, BAM_CTRL)); > + val |= BAM_SW_RST; > + writel_relaxed(val, bam_addr(bdev, 0, BAM_CTRL)); > + val &= ~BAM_SW_RST; > + writel_relaxed(val, bam_addr(bdev, 0, BAM_CTRL)); Seems odd to me that we assert and deassert the reset in back-to-back writes, without any delay etc. That said, this is unrelated to your patch as you just moved this hunk from below. > + > + /* make sure previous stores are visible before enabling BAM */ > + wmb(); > + > + /* enable bam */ > + val |= BAM_EN; > + writel_relaxed(val, bam_addr(bdev, 0, BAM_CTRL)); > + > + /* set descriptor threshhold, start with 4 bytes */ > + writel_relaxed(DEFAULT_CNT_THRSHLD, > + bam_addr(bdev, 0, BAM_DESC_CNT_TRSHLD)); > + > + /* Enable default set of h/w workarounds, ie all except BAM_FULL_PIPE */ > + writel_relaxed(BAM_CNFG_BITS_DEFAULT, bam_addr(bdev, 0, BAM_CNFG_BITS)); > + > + /* enable irqs for errors */ > + writel_relaxed(BAM_ERROR_EN | BAM_HRESP_ERR_EN, > + bam_addr(bdev, 0, BAM_IRQ_EN)); > + > + /* unmask global bam interrupt */ > + writel_relaxed(BAM_IRQ_MSK, bam_addr(bdev, 0, BAM_IRQ_SRCS_MSK_EE)); > +} > + > /** > * bam_reset_channel - Reset individual BAM DMA channel > * @bchan: bam channel > @@ -512,6 +552,9 @@ static int bam_alloc_chan(struct dma_chan *chan) > return -ENOMEM; > } > > + if (bdev->active_channels++ == 0 && bdev->powered_remotely) > + bam_reset(bdev); > + > return 0; > } > > @@ -565,6 +608,13 @@ static void bam_free_chan(struct dma_chan *chan) > /* disable irq */ > writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_IRQ_EN)); > > + if (--bdev->active_channels == 0 && bdev->powered_remotely) { > + /* s/w reset bam */ > + val = readl_relaxed(bam_addr(bdev, 0, BAM_CTRL)); > + val |= BAM_SW_RST; > + writel_relaxed(val, bam_addr(bdev, 0, BAM_CTRL)); > + } > + > err: > pm_runtime_mark_last_busy(bdev->dev); > pm_runtime_put_autosuspend(bdev->dev); > @@ -1164,38 +1214,10 @@ static int bam_init(struct bam_device *bdev) > bdev->num_channels = val & BAM_NUM_PIPES_MASK; > } > > - if (bdev->controlled_remotely) > + if (bdev->controlled_remotely || bdev->powered_remotely) > return 0; I think the resulting code would be cleaner if you flipped it around as: /* Reset BAM now if fully controlled locally */ if (!bdev->controlled_remotely && !bdev->powered_remotely) bam_reset(bdev); return 0; Regards, Bjorn > > - /* s/w reset bam */ > - /* after reset all pipes are disabled and idle */ > - val = readl_relaxed(bam_addr(bdev, 0, BAM_CTRL)); > - val |= BAM_SW_RST; > - writel_relaxed(val, bam_addr(bdev, 0, BAM_CTRL)); > - val &= ~BAM_SW_RST; > - writel_relaxed(val, bam_addr(bdev, 0, BAM_CTRL)); > - > - /* make sure previous stores are visible before enabling BAM */ > - wmb(); > - > - /* enable bam */ > - val |= BAM_EN; > - writel_relaxed(val, bam_addr(bdev, 0, BAM_CTRL)); > - > - /* set descriptor threshhold, start with 4 bytes */ > - writel_relaxed(DEFAULT_CNT_THRSHLD, > - bam_addr(bdev, 0, BAM_DESC_CNT_TRSHLD)); > - > - /* Enable default set of h/w workarounds, ie all except BAM_FULL_PIPE */ > - writel_relaxed(BAM_CNFG_BITS_DEFAULT, bam_addr(bdev, 0, BAM_CNFG_BITS)); > - > - /* enable irqs for errors */ > - writel_relaxed(BAM_ERROR_EN | BAM_HRESP_ERR_EN, > - bam_addr(bdev, 0, BAM_IRQ_EN)); > - > - /* unmask global bam interrupt */ > - writel_relaxed(BAM_IRQ_MSK, bam_addr(bdev, 0, BAM_IRQ_SRCS_MSK_EE)); > - > + bam_reset(bdev); > return 0; > }
On Mon, Oct 11, 2021 at 10:39:20AM -0700, Bjorn Andersson wrote: > On Mon 11 Oct 07:17 PDT 2021, Stephan Gerhold wrote: > > > In some configurations, the BAM DMA controller is set up by a remote > > processor and the local processor can simply start making use of it > > without setting up the BAM. This is already supported using the > > "qcom,controlled-remotely" property. > > > > However, for some reason another possible configuration is that the > > remote processor is responsible for powering up the BAM, but we are > > still responsible for initializing it (e.g. resetting it etc). > > > > This configuration is quite challenging to handle properly because > > the power control is handled through separate channels > > (e.g. device-specific SMSM interrupts / smem-states). Great care > > must be taken to ensure the BAM registers are not accessed while > > the BAM is powered off since this results in a bus stall. > > > > Attempt to support this configuration with minimal device-specific > > code in the bam_dma driver by tracking the number of requested > > channels. Consumers of DMA channels are responsible to only request > > DMA channels when the BAM was powered on by the remote processor, > > and to release them before the BAM is powered off. > > > > When the first channel is requested the BAM is initialized (reset) > > and it is also put into reset when the last channel was released. > > > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > > --- > > Changes since RFC: > > - Drop qcom-specific terminology "power collapse", instead rename > > "qcom,remote-power-collapse" -> "qcom,powered-remotely" > > > > NOTE: This is *not* a compile-time requirement for the BAM-DMUX driver > > so this could also go through the dmaengine tree. > > > > See original RFC for a discussion of alternative approaches to handle > > this configuration: > > https://lore.kernel.org/netdev/20210719145317.79692-3-stephan@gerhold.net/ > > --- > > drivers/dma/qcom/bam_dma.c | 88 ++++++++++++++++++++++++-------------- > > 1 file changed, 56 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c > > index c8a77b428b52..1b33a3ebbfec 100644 > > --- a/drivers/dma/qcom/bam_dma.c > > +++ b/drivers/dma/qcom/bam_dma.c > > @@ -388,6 +388,8 @@ struct bam_device { > > /* execution environment ID, from DT */ > > u32 ee; > > bool controlled_remotely; > > + bool powered_remotely; > > + u32 active_channels; > > > > const struct reg_offset_data *layout; > > > > @@ -415,6 +417,44 @@ static inline void __iomem *bam_addr(struct bam_device *bdev, u32 pipe, > > r.ee_mult * bdev->ee; > > } > > > > +/** > > + * bam_reset - reset and initialize BAM registers > > Please include a set of () after the function name. > Thanks, will fix this in v3. > > + * @bdev: bam device > > + */ > > +static void bam_reset(struct bam_device *bdev) > > +{ > > + u32 val; > > + > > + /* s/w reset bam */ > > + /* after reset all pipes are disabled and idle */ > > + val = readl_relaxed(bam_addr(bdev, 0, BAM_CTRL)); > > + val |= BAM_SW_RST; > > + writel_relaxed(val, bam_addr(bdev, 0, BAM_CTRL)); > > + val &= ~BAM_SW_RST; > > + writel_relaxed(val, bam_addr(bdev, 0, BAM_CTRL)); > > Seems odd to me that we assert and deassert the reset in back-to-back > writes, without any delay etc. That said, this is unrelated to your > patch as you just moved this hunk from below. > It seems to work fine though. :) I'm pretty sure I checked that this actually resets at some point, but perhaps I was lucky there. But yeah, seems better to adjust this in some future patch instead of here. > > + > > + /* make sure previous stores are visible before enabling BAM */ > > + wmb(); > > + > > + /* enable bam */ > > + val |= BAM_EN; > > + writel_relaxed(val, bam_addr(bdev, 0, BAM_CTRL)); > > + > > + /* set descriptor threshhold, start with 4 bytes */ > > + writel_relaxed(DEFAULT_CNT_THRSHLD, > > + bam_addr(bdev, 0, BAM_DESC_CNT_TRSHLD)); > > + > > + /* Enable default set of h/w workarounds, ie all except BAM_FULL_PIPE */ > > + writel_relaxed(BAM_CNFG_BITS_DEFAULT, bam_addr(bdev, 0, BAM_CNFG_BITS)); > > + > > + /* enable irqs for errors */ > > + writel_relaxed(BAM_ERROR_EN | BAM_HRESP_ERR_EN, > > + bam_addr(bdev, 0, BAM_IRQ_EN)); > > + > > + /* unmask global bam interrupt */ > > + writel_relaxed(BAM_IRQ_MSK, bam_addr(bdev, 0, BAM_IRQ_SRCS_MSK_EE)); > > +} > > + > > /** > > * bam_reset_channel - Reset individual BAM DMA channel > > * @bchan: bam channel > > @@ -512,6 +552,9 @@ static int bam_alloc_chan(struct dma_chan *chan) > > return -ENOMEM; > > } > > > > + if (bdev->active_channels++ == 0 && bdev->powered_remotely) > > + bam_reset(bdev); > > + > > return 0; > > } > > > > @@ -565,6 +608,13 @@ static void bam_free_chan(struct dma_chan *chan) > > /* disable irq */ > > writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_IRQ_EN)); > > > > + if (--bdev->active_channels == 0 && bdev->powered_remotely) { > > + /* s/w reset bam */ > > + val = readl_relaxed(bam_addr(bdev, 0, BAM_CTRL)); > > + val |= BAM_SW_RST; > > + writel_relaxed(val, bam_addr(bdev, 0, BAM_CTRL)); > > + } > > + > > err: > > pm_runtime_mark_last_busy(bdev->dev); > > pm_runtime_put_autosuspend(bdev->dev); > > @@ -1164,38 +1214,10 @@ static int bam_init(struct bam_device *bdev) > > bdev->num_channels = val & BAM_NUM_PIPES_MASK; > > } > > > > - if (bdev->controlled_remotely) > > + if (bdev->controlled_remotely || bdev->powered_remotely) > > return 0; > > I think the resulting code would be cleaner if you flipped it around as: > > /* Reset BAM now if fully controlled locally */ > if (!bdev->controlled_remotely && !bdev->powered_remotely) > bam_reset(bdev); > > return 0; > Hmm, you are probably right, I can flip it in v3. Thanks! Stephan
Hi Stephan, On Mon, 11 Oct 2021 at 16:51, Stephan Gerhold <stephan@gerhold.net> wrote: > > Like in the RFC version [1], the driver does not currently use the link > > management of the WWAN subsystem. Instead, it simply exposes one network > > interface for each of the up to 8 channels. > > > > This setup works out of the box with all available open-source userspace > > WWAN implementations, especially ModemManager (no changes needed). > > oFono works too although it requires minor changes to support WWAN control > > ports (/dev/wwan0qmi0) which are independent of BAM-DMUX (already provided > > by the "rpmsg_wwan_ctrl" driver). > > It was easy to support because the setup is very similar to ones already > > supported for USB modems. Some of them provide multiple network interfaces > > and ModemManager can bundle them together to a single modem. > > > > I believe it is best to keep this setup as-is for now and not add even > > more complexity to userspace with another setup that works only in this > > particular configuration. I will reply to this patch separately to explain > > that a bit more clearly. This patch is already long enough as-is. :) > > > > [1]: https://lore.kernel.org/netdev/20210719145317.79692-5-stephan@gerhold.net/ > > > > The main goal of the WWAN link management is to make the multiplexing > setup transparent to userspace. Unfortunately it's still unclear to me > how or even if this can be achieved for the many different different > setups that exist for Qualcomm modems. To show that more clearly I'll > "briefly" list the various currently supported setups in ModemManager > (there might be even more that I am not even aware of). The goal is also to have a common hierarchy, with the network link being a child of the WWAN device, as for the control ports. Making it easier for the user side to find the relation between all these devices. Moreover, it allows having a common set of attributes, like the LINK ID, and possibly new ones in the future. I mean it's probably fine if you create a static set of network devices and do not support dynamic link creation, but I think they should be created in some way via the WWAN subsystem, and get the same attributes (link id), we can have special meaning link ids (-1) for e.g. non context specific netdevs (e.g. for rmnet/qmap transport iface). Regards, Loic
On Tue, Oct 12, 2021 at 09:55:48AM +0200, Loic Poulain wrote: > Hi Stephan, > > On Mon, 11 Oct 2021 at 16:51, Stephan Gerhold <stephan@gerhold.net> wrote: > > > Like in the RFC version [1], the driver does not currently use the link > > > management of the WWAN subsystem. Instead, it simply exposes one network > > > interface for each of the up to 8 channels. > > > > > > This setup works out of the box with all available open-source userspace > > > WWAN implementations, especially ModemManager (no changes needed). > > > oFono works too although it requires minor changes to support WWAN control > > > ports (/dev/wwan0qmi0) which are independent of BAM-DMUX (already provided > > > by the "rpmsg_wwan_ctrl" driver). > > > It was easy to support because the setup is very similar to ones already > > > supported for USB modems. Some of them provide multiple network interfaces > > > and ModemManager can bundle them together to a single modem. > > > > > > I believe it is best to keep this setup as-is for now and not add even > > > more complexity to userspace with another setup that works only in this > > > particular configuration. I will reply to this patch separately to explain > > > that a bit more clearly. This patch is already long enough as-is. :) > > > > > > [1]: https://lore.kernel.org/netdev/20210719145317.79692-5-stephan@gerhold.net/ > > > > > > > The main goal of the WWAN link management is to make the multiplexing > > setup transparent to userspace. Unfortunately it's still unclear to me > > how or even if this can be achieved for the many different different > > setups that exist for Qualcomm modems. To show that more clearly I'll > > "briefly" list the various currently supported setups in ModemManager > > (there might be even more that I am not even aware of). > > The goal is also to have a common hierarchy, with the network link > being a child of the WWAN device, as for the control ports. Making it > easier for the user side to find the relation between all these > devices. Moreover, it allows having a common set of attributes, like > the LINK ID, and possibly new ones in the future. I mean it's probably > fine if you create a static set of network devices and do not support > dynamic link creation, but I think they should be created in some way > via the WWAN subsystem, and get the same attributes (link id), we can > have special meaning link ids (-1) for e.g. non context specific > netdevs (e.g. for rmnet/qmap transport iface). > At the moment, my driver makes the link IDs available via "dev_port". I think this was also suggested for the WWAN subsystem at some point. [1] If we skip the dynamic link creation as a first step, but want to create the network device below the WWAN parent device, the main problem that remains is that there is currently no good way to get the driver that provides the network interfaces. The common WWAN parent device in my case is the device that represents the modem remote processor, but this is not enough to identify "bam-dmux". Userspace needs to know which driver it is dealing with to set up the multiplexing correctly via QMI. (The QMI message is different for BAM-DMUX and e.g. rmnet). I guess if the goal is only to have a common hierarchy (and not necessarily to have multiplexing entirely transparent to userspace), it is not too bad to make the driver that provides the ports somehow available to userspace. Perhaps via some extra sysfs attribute. What do you think? Also note that a common hierarchy for all configurations is not possible unless someone finds a solution to integrate the QRTR network sockets into the WWAN subsystem. This is primarily relevant for the IPA driver, but there are some SoCs with QRTR + BAM-DMUX as well. This will only work in my case because I only work on "older" SoCs where QMI can still go via the RPMSG_WWAN_CTRL driver. Thanks, Stephan [1]: https://lore.kernel.org/netdev/CAMZdPi_e+ibRQiytAYkjo1A9GzLm6Np6Tma-6KMHuWfFcaFsCg@mail.gmail.com/