On Wed, 6 Sep 2023, Andy Shevchenko wrote:
> 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;
> }
I will have to respectfully disagree on this. Splitting the function into
two still adds additional redirection, however small, to the function. I
would also like to point out that the level of indent I am using is not
uncommon in 8250_exar.c and as such I do not find the styling of the
function to be out of place.
I will resubmit and try to address the other concerns raised by you and
Ilpo but unless Greg KH says otherwise I don't see any reason to change
the indent at this time.
> --
> With Best Regards,
> Andy Shevchenko
>
>
>
