On Thu, Sep 14, 2023 at 08:32:40AM -0400, Matthew Howell wrote:
> From: Matthew Howell <matthew.howell@xxxxxxxxxxxx>
>
> Sealevel XR17V35X based cards utilize DTR to control RS-485 Enable, but the current implementation of 8250_exar uses RTS for the auto-RS485-Enable mode of the XR17V35X UARTs. This patch implements DTR Auto-RS485 on Sealevel cards.
Something went wrong with line wrapping, in the commit messages we usually ise
~72 characters long lines.
Same issue appears in the previous patch.
...
> +static int pci_sealevel_setup(struct exar8250 *priv, struct pci_dev *pcidev,
> + struct uart_8250_port *port, int idx)
> +{
> + int ret = pci_xr17v35x_setup(priv, pcidev, port, idx);
> +
> + if (ret)
> + return ret;
Hmm... Seems I need to elaborate the idea behind changing above to
int ret;
ret = pci_xr17v35x_setup(priv, pcidev, port, idx);
if (ret)
return ret;
With the long-term maintenance the additional code may appear in between
ret assignment and check. While it looks simple and impossible to happen,
with time we might end up in the situation like (hypothetical case):
int ret = foo();
... *baz;
...
baz = bar();
ret = PTR_ERR_OR_ZERO();
if (ret)
...
...
if (ret) // original one from your patch
That's why splitting is better than keeping it in definition block for this
kind of variable.
> + port->port.rs485_config = sealevel_rs485_config;
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
