linux-kernel - Re: [Intel PMC TGPIO Driver 5/5] drivers/ptp: Add PMC Time-Aware GPIO Driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACRpkdbi7q5Vr2Lt12eirs3Z8GLL2AuLLrAARCHkYEYgKbYkHg@mail.gmail.com>
Date:   Fri, 7 Feb 2020 18:10:46 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     christopher.s.hall@...el.com,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     netdev <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        jacob.e.keller@...el.com,
        Richard Cochran <richardcochran@...il.com>,
        "David S. Miller" <davem@...emloft.net>, sean.v.kelley@...el.com
Subject: Re: [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@...el.com> wrote:

> From: Christopher Hall <christopher.s.hall@...el.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@...nel.org>
>
> Signed-off-by: Christopher Hall <christopher.s.hall@...el.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