Thanks Greg! I'll do a V2 incorporating all of your feedback.
Steve
On Thu, Sep 20, 2018 at 1:30 AM Greg KH <greg@xxxxxxxxx> wrote:
>
> On Wed, Sep 19, 2018 at 07:16:12AM -1000, Steve Sakoman wrote:
> > Add a "pps_4wire" file to serial ports in sysfs in case the kernel is
> > configured with CONFIG_PPS_CLIENT_LDISC. Writing 1 to the file enables
> > the use of CTS instead of DCD for PPS signal input. This is necessary
> > in case a serial port is not completely wired.
> > Though this affects PPS processing the patch is against the serial core
> > as the source of the serial port PPS event dispatching has to be
> > modified. Furthermore it should be possible to modify the source of
> > serial port PPS event dispatching before changing the line discipline.
> >
> > Signed-off-by: Andreas Steinmetz <ast@xxxxxxxx>
> > Signed-off-by: Steve Sakoman <steve@xxxxxxxxxxx>
> > Tested-by: Steve Sakoman <steve@xxxxxxxxxxx>
> > ---
> > Documentation/ABI/testing/sysfs-tty | 9 +++++
> > drivers/tty/serial/serial_core.c | 71 ++++++++++++++++++++++++++++++++++++-
> > include/linux/serial_core.h | 3 +-
> > 3 files changed, 81 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-tty b/Documentation/ABI/testing/sysfs-tty
> > index 9eb3c2b..441105a 100644
> > --- a/Documentation/ABI/testing/sysfs-tty
> > +++ b/Documentation/ABI/testing/sysfs-tty
> > @@ -154,3 +154,12 @@ Description:
> > device specification. For example, when user sets 7bytes on
> > 16550A, which has 1/4/8/14 bytes trigger, the RX trigger is
> > automatically changed to 4 bytes.
> > +
> > +What: /sys/class/tty/ttyS0/pps_4wire
> > +Date: September 2018
> > +Contact: Steve Sakoman <steve@xxxxxxxxxxx>
> > +Description:
> > + Shows/sets "4 wire" mode for the PPS input to the serial driver.
> > + For fully implemented serial ports PPS is normally provided
> > + on the DCD line. For partial "4 wire" implementations CTS is
> > + used instead of DCD.
> > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> > index 80bb56f..5839bed 100644
> > --- a/drivers/tty/serial/serial_core.c
> > +++ b/drivers/tty/serial/serial_core.c
> > @@ -2664,6 +2664,56 @@ static ssize_t uart_get_attr_iomem_reg_shift(struct device *dev,
> > return snprintf(buf, PAGE_SIZE, "%d\n", tmp.iomem_reg_shift);
> > }
> >
> > +static ssize_t uart_get_attr_pps_4wire(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct tty_port *port = dev_get_drvdata(dev);
> > + struct uart_state *state = container_of(port, struct uart_state, port);
> > + struct uart_port *uport;
> > + int mode = 0;
> > +
> > + mutex_lock(&port->mutex);
> > + uport = uart_port_check(state);
> > + if (!uport)
> > + goto out;
> > +
> > + mode = uport->pps_4wire;
> > +
> > +out:
> > + mutex_unlock(&port->mutex);
> > + return snprintf(buf, PAGE_SIZE, mode ? "yes" : "no");
>
> No need to care about the size of the buffer when only dealing with a
> single page. sprintf() is fine here.
>
> > +}
> > +
> > +static ssize_t uart_set_attr_pps_4wire(struct device *dev,
> > + struct device_attribute *attr, const char *buf, size_t count)
> > +{
> > + struct tty_port *port = dev_get_drvdata(dev);
> > + struct uart_state *state = container_of(port, struct uart_state, port);
> > + struct uart_port *uport;
> > + bool mode;
> > + int ret;
> > +
> > + if (!count)
> > + return -EINVAL;
> > +
> > + ret = kstrtobool(buf, &mode);
> > + if (ret < 0)
> > + return ret;
> > +
> > + mutex_lock(&port->mutex);
> > + uport = uart_port_check(state);
> > + if (!uport)
> > + goto out;
> > +
> > + spin_lock_irq(&uport->lock);
> > + uport->pps_4wire = mode;
> > + spin_unlock_irq(&uport->lock);
> > +
> > +out:
> > + mutex_unlock(&port->mutex);
> > + return count;
> > +}
> > +
> > static DEVICE_ATTR(type, S_IRUSR | S_IRGRP, uart_get_attr_type, NULL);
> > static DEVICE_ATTR(line, S_IRUSR | S_IRGRP, uart_get_attr_line, NULL);
> > static DEVICE_ATTR(port, S_IRUSR | S_IRGRP, uart_get_attr_port, NULL);
> > @@ -2677,6 +2727,8 @@ static DEVICE_ATTR(custom_divisor, S_IRUSR | S_IRGRP, uart_get_attr_custom_divis
> > static DEVICE_ATTR(io_type, S_IRUSR | S_IRGRP, uart_get_attr_io_type, NULL);
> > static DEVICE_ATTR(iomem_base, S_IRUSR | S_IRGRP, uart_get_attr_iomem_base, NULL);
> > static DEVICE_ATTR(iomem_reg_shift, S_IRUSR | S_IRGRP, uart_get_attr_iomem_reg_shift, NULL);
> > +static DEVICE_ATTR(pps_4wire, S_IRUSR | S_IWUSR | S_IRGRP,
> > + uart_get_attr_pps_4wire, uart_set_attr_pps_4wire);
>
> DEVICE_ATTR_RW()? All of these should get fixed up eventually as well,
> but let's not add to the bad style if possible :)
>
> Also, it's good to use the output of scripts/get_maintainer.pl to figure
> out who to cc: for patch review.
>
> thanks,
>
> greg k-h
