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

[Sergey Ryazanov]

On 1/7/26 09:21, Slark Xiao wrote:
> At 2026-01-07 00:49:50, "Sai Krishna Gajula" <saikrishnag@xxxxxxxxxxx> wrote:
> > > -----Original Message-----
> > > From: Slark Xiao <slark_xiao@xxxxxxx>
> > > Sent: Monday, January 5, 2026 3:50 PM
> > > To: loic.poulain@xxxxxxxxxxxxxxxx; ryazanov.s.a@xxxxxxxxx;
> > > johannes@xxxxxxxxxxxxxxxx; andrew+netdev@xxxxxxx;
> > > davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> > > pabeni@xxxxxxxxxx; mani@xxxxxxxxxx
> > > Cc: netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > > 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@xxxxxxxxx>
> > >
> > > 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