On Wed, Sep 06, 2023 at 11:05:20AM -0400, Matthew Howell wrote:
> On Wed, 6 Sep 2023, Andy Shevchenko wrote:
> > On Tue, Sep 05, 2023 at 12:06:20PM -0400, Matthew Howell wrote:
...
> > > + generic_rs485_config(port, termios, rs485);
> >
> > > + if (rs485->flags & SER_RS485_ENABLED) {
> >
> > What I meant is to have
> >
> > if (!)rs485->flags & SER_RS485_ENABLED))
> > return 0;
> >
> > here, which allows you to reduce indentation level in the below block.
> >
> > > + /* Set EFR[4]=1 to enable enhanced feature registers */
> > > + writeb(readb(p + UART_XR_EFR) | UART_EFR_ECB, p + UART_XR_EFR);
> > > +
> > > + /* Set MCR to use DTR as Auto-RS485 Enable signal */
> > > + writeb(UART_MCR_OUT1, p + UART_MCR);
> > > +
> > > + /* Store original LCR and set LCR[7]=1 to enable access to DLD register */
> > > + old_lcr = readb(p + UART_LCR);
> > > + writeb(old_lcr | UART_LCR_DLAB, p + UART_LCR);
> > > +
> > > + /* Set DLD[7]=1 for inverted RS485 Enable logic */
> > > + writeb(readb(p + UART_EXAR_DLD) | UART_EXAR_DLD_485_POLARITY, p + UART_EXAR_DLD);
> > > +
> > > + writeb(old_lcr, p + UART_LCR);
> > > + }
> > > +
> > > + return 0;
>
> I see where you are coming from now, but I find that slightly less clear
> than having the 'main action' within the conditional statement. And since
> the code is not heavily indented I don't see much benefit of removing the
> indent.
In that case it might make sense to split to two functions:
func1()
{
...
}
func2()
{
if (...)
return func1()
return 0;
}
--
With Best Regards,
Andy Shevchenko
