Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1482309 > unrolled thread

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

Started byAndy Shevchenko <andriy.shevchenko@linux.intel.com>
First post2016-09-13 11:50 +0200
Last post2016-09-14 00:30 +0200
Articles 3 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [RESEND RFC PATCH 0/5] platform drivers for UP Board Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-09-13 11:50 +0200
    Re: [RESEND RFC PATCH 0/5] platform drivers for UP Board Mika Westerberg <mika.westerberg@linux.intel.com> - 2016-09-13 12:00 +0200
    Re: [RESEND RFC PATCH 0/5] platform drivers for UP Board Dan O'Donovan <dan@emutex.com> - 2016-09-14 00:30 +0200

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

FromAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Date2016-09-13 11:50 +0200
SubjectRe: [RESEND RFC PATCH 0/5] platform drivers for UP Board
Message-ID<sgSvM-8uI-11@gated-at.bofh.it>
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!

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.

Basically most (everything?) you are trying to do in C code may and
should be done in ASL.

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
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[toc] | [next] | [standalone]


#1482317

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2016-09-13 12:00 +0200
Message-ID<sgSFr-6J-7@gated-at.bofh.it>
In reply to#1482309
On Tue, Sep 13, 2016 at 12:42:52PM +0300, 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!
> 
> 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.
> 
> Basically most (everything?) you are trying to do in C code may and
> should be done in ASL.
> 
> Mika, can you correct me if I'm wrong?

Sounds correct to me.

We have been working on a Yocto layer which allows one to ship your own
peripherals as ACPI ASL fragments which targets all Intel boards which
have ACPI BIOS:

  https://github.com/westeri/meta-acpi

For example adding GPIO leds is just matter of adding:

  ACPI_FRAGMENTS = "leds.asl"
  MACHINE = "minnowboard"

to your Yocto local.conf. The resulting image has these fragments
compiled and included in initrd where the kernel is able to pick them
up.

It is still very much work in progress, though :)

[toc] | [prev] | [next] | [standalone]


#1482818

FromDan O'Donovan <dan@emutex.com>
Date2016-09-14 00:30 +0200
Message-ID<sh4ng-823-21@gated-at.bofh.it>
In reply to#1482309
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
>>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web