Messages in this thread | | | From | Linus Walleij <> | Date | Fri, 7 Feb 2020 18:10:46 +0100 | 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@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
| |