Re: [RESEND RFC PATCH 0/5] platform drivers for UP Board

From: Dan O'Donovan
Date: Tue Sep 13 2016 - 18:28:55 EST


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
>>