On Tue, Mar 04, 2025 at 12:44:23PM +0530, Viken Dadhaniya wrote: > Remove the dependency on aliases in the device tree configuration for the > qcom serial driver. Currently, the absence of an alias results in an > invalid line number, causing the driver probe to fail for geni serial. > > To prevent probe failures, implement logic to dynamically assign line > numbers if an alias is not present in the device tree for non-console > ports. > Please read and follow https://docs.kernel.org/process/submitting-patches.html#describe-your-changes Start with your problem description, then a description of your proposed solution. > Signed-off-by: Viken Dadhaniya <quic_vdadhani@xxxxxxxxxxx> > --- > drivers/tty/serial/qcom_geni_serial.c | 26 +++++++++++++++++++++++--- > 1 file changed, 23 insertions(+), 3 deletions(-) > > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c > index a80ce7aaf309..2457f39dfc84 100644 > --- a/drivers/tty/serial/qcom_geni_serial.c > +++ b/drivers/tty/serial/qcom_geni_serial.c > @@ -98,6 +98,8 @@ > > #define DMA_RX_BUF_SIZE 2048 > > +static DEFINE_IDR(port_idr); You're just looking for a index allocator, so DEFINE_IDA() is probably what you want to use. That said, theoretically I think we could get 24 GENI serial instances in a system. Making this a huge waste of memory and CPU cycles. An empty idr takes 88 bytes, if you then allocate 1 entry it grows with at least one xa_array node which is 576 bytes. > + > struct qcom_geni_device_data { > bool console; > enum geni_se_xfer_mode mode; > @@ -253,10 +255,25 @@ static struct qcom_geni_serial_port *get_port_from_line(int line, bool console) > struct qcom_geni_serial_port *port; > int nr_ports = console ? GENI_UART_CONS_PORTS : GENI_UART_PORTS; > > - if (line < 0 || line >= nr_ports) > - return ERR_PTR(-ENXIO); > + if (console) { > + if (line < 0 || line >= nr_ports) > + return ERR_PTR(-ENXIO); > + > + port = &qcom_geni_console_port; > + } else { > + int max_alias_num = of_alias_get_highest_id("serial"); > + > + if (line < 0 || line >= nr_ports) > + line = idr_alloc(&port_idr, (void *)port, max_alias_num + 1, nr_ports, > + GFP_KERNEL); > + else > + line = idr_alloc(&port_idr, (void *)port, line, nr_ports, GFP_KERNEL); > + > + if (line < 0) > + return ERR_PTR(-ENXIO); > > - port = console ? &qcom_geni_console_port : &qcom_geni_uart_ports[line]; > + port = &qcom_geni_uart_ports[line]; Plus qcom_geni_uart_ports[] is GENI_UART_PORTS long. So you will actually only have a maximum of 3 ports. I like the change, but please replace port_idr with a u32 and use linux/bitmap.h to implement the port allocation scheme. Regards, Bjorn > + } > return port; > } > > @@ -1761,6 +1778,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > port->wakeup_irq); > if (ret) { > device_init_wakeup(&pdev->dev, false); > + idr_remove(&port_idr, uport->line); > uart_remove_one_port(drv, uport); > return ret; > } > @@ -1772,10 +1790,12 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > static void qcom_geni_serial_remove(struct platform_device *pdev) > { > struct qcom_geni_serial_port *port = platform_get_drvdata(pdev); > + struct uart_port *uport = &port->uport; > struct uart_driver *drv = port->private_data.drv; > > dev_pm_clear_wake_irq(&pdev->dev); > device_init_wakeup(&pdev->dev, false); > + idr_remove(&port_idr, uport->line); > uart_remove_one_port(drv, &port->uport); > } > > -- > 2.34.1 > >
