Re: [PATCH V8 2/2] serial: exar: Add RS-485 support for Sealevel XR17V35X based cards

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2023-09-22 at 17:48 +0300, andriy.shevchenko@xxxxxxxxx wrote:
> ⚠Caution: External email. Exercise extreme caution with links or attachments.⚠
> 
> 
> Bingo, you have threaded emails!
> 
> On Fri, Sep 22, 2023 at 02:22:11PM +0000, 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.
> 
> Btw, few ideas for further improvements / questions.
> 
> ...
> 
> > +static int pci_sealevel_setup(struct exar8250 *priv, struct pci_dev *pcidev,
> > +                struct uart_8250_port *port, int idx)
> > +{
> > +     int ret;
> > +
> > +     ret = pci_xr17v35x_setup(priv, pcidev, port, idx);
> > +     if (ret)
> > +             return ret;
> > +
> > +     port->port.rs485_config = sealevel_rs485_config;
> > +
> > +     return 0;
> > +}
> 
> This actually can be embedded into original pci_xr17v35x_setup() as
> 
>         if (pdev->subsystem_vendor == PCI_VENDOR_ID_SEALEVEL)
>                 port->port.rs485_config = sealevel_rs485_config;
> 
> ...
> 

That was my original thought prior to the first submission, but I wasn't
sure about adding vendor-specific changes into pci_xr17v35x_setup()
since it appears that the convention in 8250_exar.c has been for vendors
to have their own setup function when they needed to change the
init/setup behaviour.

If that is not the case though and having this in pci_xr17v35x_setup()
is more appropriate I'll be happy to put it there instead of having yet
another setup function.

> > +     SEALEVEL_DEVICE(XR17V4358, pbn_sealevel_16),
> 
> This is kinda worries me. Original Exar card has12 port, why 16 is in use for this one?
> 

Ah, good catch. I had actually forgotten about the 12 port version. Will
either fix or make redundant with move to pci_xr17v35x_setup() depending
on feedback from my statement above about what is most appropriate.

> > +     SEALEVEL_DEVICE(XR17V8358, pbn_sealevel_16),
> With the above suggestion this will be fixed automatically.
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux