Re: [net-next v4 2/8] net: wwan: core: split port creation and registration

[Sergey Ryazanov <ryazanov.s.a@gmail.com>]

On 1/7/26 09:21, Slark Xiao wrote:
> At 2026-01-07 00:49:50, "Sai Krishna Gajula" <saikrishnag@marvell.com> wrote:
>>> -----Original Message-----
>>> From: Slark Xiao <slark_xiao@163.com>
>>> Sent: Monday, January 5, 2026 3:50 PM
>>> To: loic.poulain@oss.qualcomm.com; ryazanov.s.a@gmail.com;
>>> johannes@sipsolutions.net; andrew+netdev@lunn.ch;
>>> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
>>> pabeni@redhat.com; mani@kernel.org
>>> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org
>>> Subject: [net-next v4 2/8] net: wwan: core: split port creation and
>>> registration
>>>
>>> From: Sergey Ryazanov <ryazanov. s. a@ gmail. com> Upcoming GNSS (NMEA)
>>> port type support requires exporting it via the GNSS subsystem. On another
>>> hand, we still need to do basic WWAN core work: find or allocate the WWAN
>>> device, make it the
>>> From: Sergey Ryazanov <ryazanov.s.a@gmail.com>
>>>
>>> Upcoming GNSS (NMEA) port type support requires exporting it via the GNSS
>>> subsystem. On another hand, we still need to do basic WWAN core
>>> work: find or allocate the WWAN device, make it the port parent, etc. To reuse
>>> as much code as possible, split the port creation function into the registration
>>> of a regular WWAN port device, and basic port struct initialization.
>>>
>>> To be able to use put_device() uniformly, break the device_register() call into
>>> device_initialize() and device_add() and call device initialization earlier.

[skipped]

>>> +	struct wwan_device *wwandev = to_wwan_dev(port->dev.parent);
>>> +	char namefmt[0x20];
>>> +	int minor, err;
>>> +
>>> +	/* A port is exposed as character device, get a minor */
>>> +	minor = ida_alloc_range(&minors, 0, WWAN_MAX_MINORS - 1,
>>> GFP_KERNEL);
>>> +	if (minor < 0)
>>> +		return minor;
>>> +
>>> +	port->dev.class = &wwan_class;
>>> +	port->dev.devt = MKDEV(wwan_major, minor);
>>> +
>>> +	/* allocate unique name based on wwan device id, port type and
>>> number */
>>> +	snprintf(namefmt, sizeof(namefmt), "wwan%u%s%%d", wwandev-
>>>> id,
>>> +		 wwan_port_types[port->type].devsuf);
>>> +
>>> +	/* Serialize ports registration */
>>> +	mutex_lock(&wwan_register_lock);
>>> +
>>> +	__wwan_port_dev_assign_name(port, namefmt);
>>> +	err = device_add(&port->dev);
>>> +
>>> +	mutex_unlock(&wwan_register_lock);
>>> +
>>> +	if (err)
>>> +		return err;
>> Please check, if freeing with ida_free is required before returning err.
>> if (err) {
>>     ida_free(&minors, minor);
>>     return err;
>> }
> Yes, you are right.
> And patch 7/8 modifies this file as well. We need to align with that changes
> since there are some issues(we still allocates the minor even the cdev is
> false). This would lead to the release function can't release the correct
> devt.

Sai, nice catch! It should free the minor number on error exactly like this.

--
Sergey