| Message ID | 20240205082434.36531-1-m.shams@samsung.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] serial: samsung: honor fifosize from dts at first | expand |
On 05/02/2024 09:24, Tamseel Shams wrote: > Currently for platforms which passes UART fifosize from DT gets > override by local driver structure "s3c24xx_serial_drv_data", > which is not intended. Change the code to honor fifosize from > device tree at first. > > Signed-off-by: Tamseel Shams <m.shams@samsung.com> > --- > Change Log: > v1 -> v2: > Acknowledged Krzysztof's comments > Initialized "ret" variable > > drivers/tty/serial/samsung_tty.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > index 71d17d804fda..e5dc2c32b1bd 100644 > --- a/drivers/tty/serial/samsung_tty.c > +++ b/drivers/tty/serial/samsung_tty.c > @@ -1952,7 +1952,7 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) > struct device_node *np = pdev->dev.of_node; > struct s3c24xx_uart_port *ourport; > int index = probe_index; > - int ret, prop = 0; > + int ret = 1, prop = 0; I am sorry, but return of probe function cannot be positive. > > if (np) { > ret = of_alias_get_id(np, "serial"); > @@ -1990,8 +1990,7 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) > } > > if (np) { > - of_property_read_u32(np, > - "samsung,uart-fifosize", &ourport->port.fifosize); > + ret = of_property_read_u32(np, "samsung,uart-fifosize", &ourport->port.fifosize); > > if (of_property_read_u32(np, "reg-io-width", &prop) == 0) { > switch (prop) { > @@ -2009,10 +2008,13 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) > } > } > > - if (ourport->drv_data->fifosize[index]) > - ourport->port.fifosize = ourport->drv_data->fifosize[index]; > - else if (ourport->info->fifosize) > - ourport->port.fifosize = ourport->info->fifosize; > + if (ret) { Are you sure that you are checking correct ret? Best regards, Krzysztof
Hi Krzysztof, > -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] > Sent: 06 February 2024 15:42 > To: Tamseel Shams <m.shams@samsung.com>; alim.akhtar@samsung.com; > krzysztof.kozlowski+dt@linaro.org; gregkh@linuxfoundation.org; > jirislaby@kernel.org > Cc: linux-arm-kernel@lists.infradead.org; linux-samsung- > soc@vger.kernel.org; linux-kernel@vger.kernel.org; linux- > serial@vger.kernel.org > Subject: Re: [PATCH v2] serial: samsung: honor fifosize from dts at first > > On 05/02/2024 09:24, Tamseel Shams wrote: > > Currently for platforms which passes UART fifosize from DT gets > > override by local driver structure "s3c24xx_serial_drv_data", which is > > not intended. Change the code to honor fifosize from device tree at > > first. > > > > Signed-off-by: Tamseel Shams <m.shams@samsung.com> > > --- > > Change Log: > > v1 -> v2: > > Acknowledged Krzysztof's comments > > Initialized "ret" variable > > > > drivers/tty/serial/samsung_tty.c | 16 +++++++++------- > > 1 file changed, 9 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/tty/serial/samsung_tty.c > > b/drivers/tty/serial/samsung_tty.c > > index 71d17d804fda..e5dc2c32b1bd 100644 > > --- a/drivers/tty/serial/samsung_tty.c > > +++ b/drivers/tty/serial/samsung_tty.c > > @@ -1952,7 +1952,7 @@ static int s3c24xx_serial_probe(struct > platform_device *pdev) > > struct device_node *np = pdev->dev.of_node; > > struct s3c24xx_uart_port *ourport; > > int index = probe_index; > > - int ret, prop = 0; > > + int ret = 1, prop = 0; > > I am sorry, but return of probe function cannot be positive. > Thanks for the review. I am reusing the "ret" variable to check whether fifosize property is present in DT or not. It will be overwritten at later stage in probe function before returning. Also, "ret" variable is used to return from probe function only in case of failure i.e. "ret" is negative. Currently, probe function always returns 0 in case of success. For better readability, I am thinking of introducing a new variable for checking the return value from function "of_property_read_u32" while getting "samsung,uart-fifosize" property. What are your thoughts on that? > > > > if (np) { > > ret = of_alias_get_id(np, "serial"); @@ -1990,8 +1990,7 @@ > static > > int s3c24xx_serial_probe(struct platform_device *pdev) > > } > > > > if (np) { > > - of_property_read_u32(np, > > - "samsung,uart-fifosize", &ourport->port.fifosize); > > + ret = of_property_read_u32(np, "samsung,uart-fifosize", > > +&ourport->port.fifosize); > > > > if (of_property_read_u32(np, "reg-io-width", &prop) == 0) { > > switch (prop) { > > @@ -2009,10 +2008,13 @@ static int s3c24xx_serial_probe(struct > platform_device *pdev) > > } > > } > > > > - if (ourport->drv_data->fifosize[index]) > > - ourport->port.fifosize = ourport->drv_data->fifosize[index]; > > - else if (ourport->info->fifosize) > > - ourport->port.fifosize = ourport->info->fifosize; > > + if (ret) { > > Are you sure that you are checking correct ret? Explanation same as above. Thanks & Regards, Tamseel Shams
diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index 71d17d804fda..e5dc2c32b1bd 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -1952,7 +1952,7 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node; struct s3c24xx_uart_port *ourport; int index = probe_index; - int ret, prop = 0; + int ret = 1, prop = 0; if (np) { ret = of_alias_get_id(np, "serial"); @@ -1990,8 +1990,7 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) } if (np) { - of_property_read_u32(np, - "samsung,uart-fifosize", &ourport->port.fifosize); + ret = of_property_read_u32(np, "samsung,uart-fifosize", &ourport->port.fifosize); if (of_property_read_u32(np, "reg-io-width", &prop) == 0) { switch (prop) { @@ -2009,10 +2008,13 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) } } - if (ourport->drv_data->fifosize[index]) - ourport->port.fifosize = ourport->drv_data->fifosize[index]; - else if (ourport->info->fifosize) - ourport->port.fifosize = ourport->info->fifosize; + if (ret) { + if (ourport->drv_data->fifosize[index]) + ourport->port.fifosize = ourport->drv_data->fifosize[index]; + else if (ourport->info->fifosize) + ourport->port.fifosize = ourport->info->fifosize; + } + ourport->port.has_sysrq = IS_ENABLED(CONFIG_SERIAL_SAMSUNG_CONSOLE); /*
Currently for platforms which passes UART fifosize from DT gets override by local driver structure "s3c24xx_serial_drv_data", which is not intended. Change the code to honor fifosize from device tree at first. Signed-off-by: Tamseel Shams <m.shams@samsung.com> --- Change Log: v1 -> v2: Acknowledged Krzysztof's comments Initialized "ret" variable drivers/tty/serial/samsung_tty.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)