lkml.org 
[lkml]   [2020]   [Feb]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [Intel PMC TGPIO Driver 5/5] drivers/ptp: Add PMC Time-Aware GPIO Driver
    Hi Christopher,

    thanks for your patch!

    On Fri, Jan 31, 2020 at 7:41 AM <christopher.s.hall@intel.com> wrote:

    > From: Christopher Hall <christopher.s.hall@intel.com>
    >
    > Add support for PMC Time-Aware GPIO (TGPIO) hardware that is present on
    > upcoming Intel platforms. The hardware logic is driven by the ART clock.
    > The current hardware has two GPIO pins. Input interrupts are not
    > implemented in hardware.
    >
    > The driver implements to the expanded PHC interface. Input requires use of
    > the user-polling interface. Also, since the ART clock can't be adjusted,
    > modulating the output frequency uses the edge timestamp interface
    > (EVENT_COUNT_TSTAMP2) and the PEROUT2 ioctl output frequency adjustment
    > interface.
    >
    > Acknowledgment: Portions of the driver code were authored by Felipe
    > Balbi <balbi@kernel.org>
    >
    > Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>

    This driver becomes a big confusion for the GPIO maintainer...

    > +config PTP_INTEL_PMC_TGPIO
    > + tristate "Intel PMC Timed GPIO"
    > + depends on X86
    > + depends on ACPI
    > + depends on PTP_1588_CLOCK
    (...)
    > +#include <linux/gpio.h>

    Don't use this header in new code, use <linux/gpio/driver.h>

    But it looks like you should just drop it because there is no GPIO
    of that generic type going on at all?

    > +/* Control Register */
    > +#define TGPIOCTL_EN BIT(0)
    > +#define TGPIOCTL_DIR BIT(1)
    > +#define TGPIOCTL_EP GENMASK(3, 2)
    > +#define TGPIOCTL_EP_RISING_EDGE (0 << 2)
    > +#define TGPIOCTL_EP_FALLING_EDGE (1 << 2)
    > +#define TGPIOCTL_EP_TOGGLE_EDGE (2 << 2)
    > +#define TGPIOCTL_PM BIT(4)

    OK this looks like some GPIO registers...

    Then there is a bunch of PTP stuff I don't understand I suppose
    related to the precision time protocol.

    Could you explain to a simple soul like me what is going on?
    Should I bother myself with this or is this "some other GPIO,
    not what you work on" or could it be that it's something I should
    review?

    I get the impression that this so-called "general purpose I/O"
    isn't very general purpose at all, it seems to be very PTP-purpose
    rather, so this confusion needs to be explained in the commit
    message and possibly in the code as well.

    What is it for really?

    Yours,
    Linus Walleij

    \
     
     \ /
      Last update: 2020-02-07 18:12    [W:4.739 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site