Re: [PATCH v3 1/2] serial: sprd: getting port index via serial aliases only

[Baolin Wang]

On Wed, Mar 18, 2020 at 6:51 PM Chunyan Zhang <zhang.lyra@xxxxxxxxx> wrote:
>
> From: Chunyan Zhang <chunyan.zhang@xxxxxxxxxx>
>
> This patch simplifies the process of getting serial port number, with
> this patch, serial devices must have aliases configured in devicetree.
>
> The serial port searched out via sprd_port array maybe wrong if we don't
> have serial alias defined in devicetree, and specify console with command
> line, we would get the wrong port number if other serial ports probe
> failed before console's. So using aliases is mandatory.
>
> Signed-off-by: Chunyan Zhang <chunyan.zhang@xxxxxxxxxx>

Looks good to me.
Reviewed-by: Baolin Wang <baolin.wang7@xxxxxxxxx>

> ---
>  drivers/tty/serial/sprd_serial.c | 36 +++++---------------------------
>  1 file changed, 5 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
> index 914862844790..e9d148d47bec 100644
> --- a/drivers/tty/serial/sprd_serial.c
> +++ b/drivers/tty/serial/sprd_serial.c
> @@ -1102,29 +1102,6 @@ static struct uart_driver sprd_uart_driver = {
>         .cons = SPRD_CONSOLE,
>  };
>
> -static int sprd_probe_dt_alias(int index, struct device *dev)
> -{
> -       struct device_node *np;
> -       int ret = index;
> -
> -       if (!IS_ENABLED(CONFIG_OF))
> -               return ret;
> -
> -       np = dev->of_node;
> -       if (!np)
> -               return ret;
> -
> -       ret = of_alias_get_id(np, "serial");
> -       if (ret < 0)
> -               ret = index;
> -       else if (ret >= ARRAY_SIZE(sprd_port) || sprd_port[ret] != NULL) {
> -               dev_warn(dev, "requested serial port %d not available.\n", ret);
> -               ret = index;
> -       }
> -
> -       return ret;
> -}
> -
>  static int sprd_remove(struct platform_device *dev)
>  {
>         struct sprd_uart_port *sup = platform_get_drvdata(dev);
> @@ -1204,14 +1181,11 @@ static int sprd_probe(struct platform_device *pdev)
>         int index;
>         int ret;
>
> -       for (index = 0; index < ARRAY_SIZE(sprd_port); index++)
> -               if (sprd_port[index] == NULL)
> -                       break;
> -
> -       if (index == ARRAY_SIZE(sprd_port))
> -               return -EBUSY;
> -
> -       index = sprd_probe_dt_alias(index, &pdev->dev);
> +       index = of_alias_get_id(pdev->dev.of_node, "serial");
> +       if (index < 0 || index >= ARRAY_SIZE(sprd_port)) {
> +               dev_err(&pdev->dev, "got a wrong serial alias id %d\n", index);
> +               return -EINVAL;
> +       }
>
>         sprd_port[index] = devm_kzalloc(&pdev->dev, sizeof(*sprd_port[index]),
>                                         GFP_KERNEL);
> --
> 2.20.1
>


-- 
Baolin Wang