Re: [PATCH net-next v2 2/3] net: wwan: Add RPMSG WWAN CTRL driver

From: Aleksander Morgado <aleksander@aleksander.es>
To: Stephan Gerhold <stephan@gerhold.net>
Cc: "David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Loic Poulain <loic.poulain@linaro.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Sergey Ryazanov <ryazanov.s.a@gmail.com>,
	Johannes Berg <johannes.berg@intel.com>,
	Leon Romanovsky <leon@kernel.org>,
	M Chetan Kumar <m.chetan.kumar@intel.com>,
	linuxwwan@intel.com, Ohad Ben-Cohen <ohad@wizery.com>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Network Development <netdev@vger.kernel.org>,
	linux-remoteproc@vger.kernel.org,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	phone-devel@vger.kernel.org,
	open list <linux-kernel@vger.kernel.org>,
	~postmarketos/upstreaming@lists.sr.ht
Subject: Re: [PATCH net-next v2 2/3] net: wwan: Add RPMSG WWAN CTRL driver
Date: Fri, 18 Jun 2021 10:21:18 +0200	[thread overview]
Message-ID: <CAAP7ucKHXv_Wu7dpSmPpy1utMZV5iXGOjGg87AbcR4j+Xcz=WA@mail.gmail.com> (raw)
In-Reply-To: <20210618075243.42046-3-stephan@gerhold.net>

Hey Stephan,

> +static const struct rpmsg_device_id rpmsg_wwan_ctrl_id_table[] = {
> +       /* RPMSG channels for Qualcomm SoCs with integrated modem */
> +       { .name = "DATA5_CNTL", .driver_data = WWAN_PORT_QMI },
> +       { .name = "DATA4", .driver_data = WWAN_PORT_AT },
> +       {},
> +};

If I understand this properly, now these rpmsg backed control ports
would be automatically exposed without the need of a userspace CLI
tool to do that (rpmsgexport).

And if I recall correctly, DATA5_CNTL and DATA4 were the only channels
actively exported with udev actions using rpmsgexport in postmarketos,
but that didn't mean someone could add additional rules to export
other channels (i.e. as per the ModemManager port type hint rules,
DATA[0-9]*_CNTL as QMI and DATA[0-9]* as AT, except for DATA40_CNTL
and DATA_40 which are the USB tethering related ones).

So, does this mean we're limiting the amount of channels exported to
only one QMI control port and one AT control port? Not saying that's
wrong, but maybe it makes sense to add a comment somewhere specifying
that explicitly.

Also, would it make sense to have some way to trigger the export of
additional channels somehow via userspace? e.g. something like
rpmsgexport but using the wwan subsystem. I'm not sure if that's a
true need anywhere or just over-engineering the solution, truth be
told.

-- 
Aleksander
https://aleksander.es