On 09/13/2016 10:42 AM, Andy Shevchenko wrote: > On Mon, 2016-07-04 at 17:07 +0100, Dan O'Donovan wrote: >> [Re-sending to a wider audience suggested by Darren Hart] >> >> The UP Board is a new SBC based on the Intel Atom X5-Z8350 "Cherry >> Trail" SoC and features a 40-pin I/O pin header and form-factor >> inspired by the Raspberry Pi 2. >> >> It utilises a CPLD between the SoC and the external 40-pin header >> to provide buffered voltage level-shifting of the I/O signals, mux >> switching and LED control, and programmable pin mapping between the >> SoC and the external pin header. >> >> The gpio, pinctrl and led drivers provided in this patch series >> enable and manage the functions provided by that CPLD. >> >> I have some open questions about this patch series: >> * Is it ok to place all of these various UP board drivers together >> in drivers/platform/x86/, or would it be preferable to place them >> in the respective sub-system directories (gpio, pinctrl, etc.)? >> My rationale for keeping them together here is that they are all >> specific to this UP Board platform and not expected to be >> generally useful on any other platforms (except variants of UP). >> * Is it acceptable to include hard-coded references to ACPI device >> IDs (representing devices integrated on the SoC devices) for the >> purpose of pin map and gpio references? Or is it required to >> use only named gpio pins? >> >> Any feedback/suggestions on the questions above, and the patch series >> in general, would be greatly appreciated! Firstly, many thanks to everyone for the valuable feedback provided on this patch set so far, and apologies for the very delayed follow-up. I haven't abandoned it, but unfortunately haven't been able to get near it for the past few weeks for various reasons. I'm looking forward to addressing all comments received as soon as I can. > Looking closer to this and taking into account what is going on with > ACPI support for open connected boards I think this patch set is not > needed at all. I do agree with your point about shifting a lot of this into ASL. I do think a driver is still needed though to manage the pin/led controller (implemented on a CPLD). Some previous comments have suggested an MFD driver structure for representing the CPLD functions, which seems to make sense. > Basically most (everything?) you are trying to do in C code may and > should be done in ASL. Among other things, one primary concept was to create a "virtual" gpio-chip on top of this pin controller to represent the external header pins, so that I could catch the run-time pin configuration updates (e.g. gpio direction changes) and reconfigure the pin controller accordingly. It also had some other benefits, from my perspective, such as allowing a more logical pin numbering scheme for those external pins. I haven't seen ASL examples yet that would indicate that it is possible to do it all in ASL, but I'll do some more research on that front now to see what I might be missing there. > > Mika, can you correct me if I'm wrong? > >> Further information on the UP board can be obtained from [1] and [2]. >> >> [1] https://www.up-board.org >> [2] https://up-community.org >> >> Dan O'Donovan (5): >> platform: x86: add driver for UP Board I/O CPLD >> platform: x86: add UP Board I/O pinctrl driver >> platform: x86: add UP Board I/O gpio driver >> platform: x86: add UP Board CPLD LED driver >> platform: x86: add platform driver for UP Board >> >> drivers/platform/x86/Kconfig | 13 + >> drivers/platform/x86/Makefile | 5 + >> drivers/platform/x86/up_board.c | 167 ++++++++++ >> drivers/platform/x86/up_board_cpld.c | 560 >> ++++++++++++++++++++++++++++++++ >> drivers/platform/x86/up_board_cpld.h | 38 +++ >> drivers/platform/x86/up_board_gpio.c | 254 +++++++++++++++ >> drivers/platform/x86/up_board_gpio.h | 59 ++++ >> drivers/platform/x86/up_board_leds.c | 85 +++++ >> drivers/platform/x86/up_board_leds.h | 50 +++ >> drivers/platform/x86/up_board_pinctrl.c | 285 ++++++++++++++++ >> drivers/platform/x86/up_board_pinctrl.h | 102 ++++++ >> 11 files changed, 1618 insertions(+) >> create mode 100644 drivers/platform/x86/up_board.c >> create mode 100644 drivers/platform/x86/up_board_cpld.c >> create mode 100644 drivers/platform/x86/up_board_cpld.h >> create mode 100644 drivers/platform/x86/up_board_gpio.c >> create mode 100644 drivers/platform/x86/up_board_gpio.h >> create mode 100644 drivers/platform/x86/up_board_leds.c >> create mode 100644 drivers/platform/x86/up_board_leds.h >> create mode 100644 drivers/platform/x86/up_board_pinctrl.c >> create mode 100644 drivers/platform/x86/up_board_pinctrl.h >> -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html