* [PATCH v3 1/1] alix2: supplement driver to include GPIO button support
@ 2012-01-23 19:56 Philip Prindeville
2012-01-23 22:56 ` Andres Salomon
2012-01-25 22:57 ` Andrew Morton
0 siblings, 2 replies; 9+ messages in thread
From: Philip Prindeville @ 2012-01-23 19:56 UTC (permalink / raw)
To: platform-driver-x86, Alessandro Zummo, Richard Purdie,
Andres Salomon
Cc: Ed Wildgoose, David Woodhouse, Andrew Morton
From: "Philip A. Prindeville" <philipp@redfish-solutions.com>
GPIO 24 is used in reference designs as a soft-reset button, and
the alix2 is no exception. Add it as a gpio-button.
Use symbolic values to describe BIOS addresses.
Record the model number.
Redux: address Andres' review comments; make the model number exported;
add support for DMI detection of the board and manufacturer; change
detection return type to bool; add comment explaining that the model
number is used by downstream drivers to detect if we have RFKILL GPIO
line to mini-PCIe slots (alix 6 only).
Signed-off-by: Philip A. Prindeville <philipp@redfish-solutions.com>
Acked-by: Ed Wildgoose <kernel@wildgooses.com>
Cc: Andres Salomon <dilinger@queued.net>
---
arch/x86/platform/geode/alix.c | 80 +++++++++++++++++++++++++++++++++++++--
1 files changed, 75 insertions(+), 5 deletions(-)
diff --git a/arch/x86/platform/geode/alix.c b/arch/x86/platform/geode/alix.c
index dc5f1d3..cb97c13 100644
--- a/arch/x86/platform/geode/alix.c
+++ b/arch/x86/platform/geode/alix.c
@@ -6,6 +6,7 @@
*
* Copyright (C) 2008 Constantin Baranov <const@mimas.ru>
* Copyright (C) 2011 Ed Wildgoose <kernel@wildgooses.com>
+ * and Philip Prindeville <philipp@redfish-solutions.com>
*
* TODO: There are large similarities with leds-net5501.c
* by Alessandro Zummo <a.zummo@towertech.it>
@@ -24,14 +25,55 @@
#include <linux/leds.h>
#include <linux/platform_device.h>
#include <linux/gpio.h>
+#include <linux/input.h>
+#include <linux/gpio_keys.h>
+#include <linux/dmi.h>
#include <asm/geode.h>
+#define BIOS_SIGNATURE_TINYBIOS 0xf0000
+#define BIOS_SIGNATURE_COREBOOT 0x500
+#define BIOS_REGION_SIZE 0x10000
+
+/*
+ * the 6F2 has an RFKILL GPIO line to the mini-PCIe slots that isn't
+ * present on other Alix boards. Wifi drivers can use this symbol to
+ * detect if it's available.
+ */
+int alix_model;
+EXPORT_SYMBOL(alix_model);
+
static bool force = 0;
module_param(force, bool, 0444);
/* FIXME: Award bios is not automatically detected as Alix platform */
MODULE_PARM_DESC(force, "Force detection as ALIX.2/ALIX.3 platform");
+static struct gpio_keys_button alix_gpio_buttons[] = {
+ {
+ .code = KEY_RESTART,
+ .gpio = 24,
+ .active_low = 1,
+ .desc = "Reset button",
+ .type = EV_KEY,
+ .wakeup = 0,
+ .debounce_interval = 100,
+ .can_disable = 0,
+ }
+};
+static struct gpio_keys_platform_data alix_buttons_data = {
+ .buttons = alix_gpio_buttons,
+ .nbuttons = ARRAY_SIZE(alix_gpio_buttons),
+ .poll_interval = 20,
+};
+
+static struct platform_device alix_buttons_dev = {
+ .name = "gpio-keys-polled",
+ .id = 1,
+ .dev = {
+ .platform_data = &alix_buttons_data,
+ }
+};
+
static struct gpio_led alix_leds[] = {
{
.name = "alix:1",
@@ -64,17 +106,22 @@ static struct platform_device alix_leds_dev = {
.dev.platform_data = &alix_leds_data,
};
+static struct __initdata platform_device *alix_devs[] = {
+ &alix_buttons_dev,
+ &alix_leds_dev,
+};
+
static void __init register_alix(void)
{
/* Setup LED control through leds-gpio driver */
- platform_device_register(&alix_leds_dev);
+ platform_add_devices(alix_devs, ARRAY_SIZE(alix_devs));
}
static int __init alix_present(unsigned long bios_phys,
const char *alix_sig,
size_t alix_sig_len)
{
- const size_t bios_len = 0x00010000;
+ const size_t bios_len = BIOS_REGION_SIZE;
const char *bios_virt;
const char *scan_end;
const char *p;
@@ -109,7 +156,9 @@ static int __init alix_present(unsigned long bios_phys,
*a = '\0';
tail = p + alix_sig_len;
- if ((tail[0] == '2' || tail[0] == '3')) {
+ if ((tail[0] == '2' || tail[0] == '3' || tail[0] == '6')) {
+ alix_model = tail[0] - '0';
+
printk(KERN_INFO
"%s: system is recognized as \"%s\"\n",
KBUILD_MODNAME, name);
@@ -120,6 +169,26 @@ static int __init alix_present(unsigned long bios_phys,
return 0;
}
+static bool __init alix_present_dmi(void)
+{
+ char *vendor, *product;
+
+ vendor = dmi_get_system_info(DMI_SYS_VENDOR);
+ if (!vendor || strcmp(vendor, "PC Engines"))
+ return false;
+
+ product = dmi_get_system_info(DMI_PRODUCT_NAME);
+ if (!product || (strcmp(product, "ALIX.2D") && strcmp(product, "ALIX.6")))
+ return false;
+
+ alix_model = product[5] - '0';
+
+ printk(KERN_INFO "%s: system is recognized as \"%s %s\"\n",
+ KBUILD_MODNAME, vendor, product);
+
+ return true;
+}
+
static int __init alix_init(void)
{
const char tinybios_sig[] = "PC Engines ALIX.";
@@ -128,8 +197,9 @@ static int __init alix_init(void)
if (!is_geode())
return 0;
- if (alix_present(0xf0000, tinybios_sig, sizeof(tinybios_sig) - 1) ||
- alix_present(0x500, coreboot_sig, sizeof(coreboot_sig) - 1))
+ if (alix_present(BIOS_SIGNATURE_TINYBIOS, tinybios_sig, sizeof(tinybios_sig) - 1) ||
+ alix_present(BIOS_SIGNATURE_COREBOOT, coreboot_sig, sizeof(coreboot_sig) - 1) ||
+ alix_present_dmi())
register_alix();
return 0;
--
1.7.7.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/1] alix2: supplement driver to include GPIO button support
2012-01-23 19:56 [PATCH v3 1/1] alix2: supplement driver to include GPIO button support Philip Prindeville
@ 2012-01-23 22:56 ` Andres Salomon
2012-01-25 22:57 ` Andrew Morton
1 sibling, 0 replies; 9+ messages in thread
From: Andres Salomon @ 2012-01-23 22:56 UTC (permalink / raw)
To: Philip Prindeville
Cc: platform-driver-x86, Alessandro Zummo, Richard Purdie,
Ed Wildgoose, David Woodhouse, Andrew Morton
Thanks, looks good.
Acked-by: Andres Salomon <dilinger@queued.net>
On Mon, 23 Jan 2012 12:56:29 -0700
Philip Prindeville <philipp@redfish-solutions.com> wrote:
> From: "Philip A. Prindeville" <philipp@redfish-solutions.com>
>
> GPIO 24 is used in reference designs as a soft-reset button, and
> the alix2 is no exception. Add it as a gpio-button.
>
> Use symbolic values to describe BIOS addresses.
>
> Record the model number.
>
> Redux: address Andres' review comments; make the model number
> exported; add support for DMI detection of the board and
> manufacturer; change detection return type to bool; add comment
> explaining that the model number is used by downstream drivers to
> detect if we have RFKILL GPIO line to mini-PCIe slots (alix 6 only).
>
> Signed-off-by: Philip A. Prindeville <philipp@redfish-solutions.com>
> Acked-by: Ed Wildgoose <kernel@wildgooses.com>
> Cc: Andres Salomon <dilinger@queued.net>
> ---
> arch/x86/platform/geode/alix.c | 80
> +++++++++++++++++++++++++++++++++++++-- 1 files changed, 75
> insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/platform/geode/alix.c
> b/arch/x86/platform/geode/alix.c index dc5f1d3..cb97c13 100644
> --- a/arch/x86/platform/geode/alix.c
> +++ b/arch/x86/platform/geode/alix.c
> @@ -6,6 +6,7 @@
> *
> * Copyright (C) 2008 Constantin Baranov <const@mimas.ru>
> * Copyright (C) 2011 Ed Wildgoose <kernel@wildgooses.com>
> + * and Philip Prindeville
> <philipp@redfish-solutions.com> *
> * TODO: There are large similarities with leds-net5501.c
> * by Alessandro Zummo <a.zummo@towertech.it>
> @@ -24,14 +25,55 @@
> #include <linux/leds.h>
> #include <linux/platform_device.h>
> #include <linux/gpio.h>
> +#include <linux/input.h>
> +#include <linux/gpio_keys.h>
> +#include <linux/dmi.h>
>
> #include <asm/geode.h>
>
> +#define BIOS_SIGNATURE_TINYBIOS 0xf0000
> +#define BIOS_SIGNATURE_COREBOOT 0x500
> +#define BIOS_REGION_SIZE 0x10000
> +
> +/*
> + * the 6F2 has an RFKILL GPIO line to the mini-PCIe slots that isn't
> + * present on other Alix boards. Wifi drivers can use this symbol to
> + * detect if it's available.
> + */
> +int alix_model;
> +EXPORT_SYMBOL(alix_model);
> +
> static bool force = 0;
> module_param(force, bool, 0444);
> /* FIXME: Award bios is not automatically detected as Alix platform
> */ MODULE_PARM_DESC(force, "Force detection as ALIX.2/ALIX.3
> platform");
> +static struct gpio_keys_button alix_gpio_buttons[] = {
> + {
> + .code = KEY_RESTART,
> + .gpio = 24,
> + .active_low = 1,
> + .desc = "Reset button",
> + .type = EV_KEY,
> + .wakeup = 0,
> + .debounce_interval = 100,
> + .can_disable = 0,
> + }
> +};
> +static struct gpio_keys_platform_data alix_buttons_data = {
> + .buttons = alix_gpio_buttons,
> + .nbuttons = ARRAY_SIZE(alix_gpio_buttons),
> + .poll_interval = 20,
> +};
> +
> +static struct platform_device alix_buttons_dev = {
> + .name = "gpio-keys-polled",
> + .id = 1,
> + .dev = {
> + .platform_data = &alix_buttons_data,
> + }
> +};
> +
> static struct gpio_led alix_leds[] = {
> {
> .name = "alix:1",
> @@ -64,17 +106,22 @@ static struct platform_device alix_leds_dev = {
> .dev.platform_data = &alix_leds_data,
> };
>
> +static struct __initdata platform_device *alix_devs[] = {
> + &alix_buttons_dev,
> + &alix_leds_dev,
> +};
> +
> static void __init register_alix(void)
> {
> /* Setup LED control through leds-gpio driver */
> - platform_device_register(&alix_leds_dev);
> + platform_add_devices(alix_devs, ARRAY_SIZE(alix_devs));
> }
>
> static int __init alix_present(unsigned long bios_phys,
> const char *alix_sig,
> size_t alix_sig_len)
> {
> - const size_t bios_len = 0x00010000;
> + const size_t bios_len = BIOS_REGION_SIZE;
> const char *bios_virt;
> const char *scan_end;
> const char *p;
> @@ -109,7 +156,9 @@ static int __init alix_present(unsigned long
> bios_phys, *a = '\0';
>
> tail = p + alix_sig_len;
> - if ((tail[0] == '2' || tail[0] == '3')) {
> + if ((tail[0] == '2' || tail[0] == '3' || tail[0] ==
> '6')) {
> + alix_model = tail[0] - '0';
> +
> printk(KERN_INFO
> "%s: system is recognized as
> \"%s\"\n", KBUILD_MODNAME, name);
> @@ -120,6 +169,26 @@ static int __init alix_present(unsigned long
> bios_phys, return 0;
> }
>
> +static bool __init alix_present_dmi(void)
> +{
> + char *vendor, *product;
> +
> + vendor = dmi_get_system_info(DMI_SYS_VENDOR);
> + if (!vendor || strcmp(vendor, "PC Engines"))
> + return false;
> +
> + product = dmi_get_system_info(DMI_PRODUCT_NAME);
> + if (!product || (strcmp(product, "ALIX.2D") &&
> strcmp(product, "ALIX.6")))
> + return false;
> +
> + alix_model = product[5] - '0';
> +
> + printk(KERN_INFO "%s: system is recognized as \"%s %s\"\n",
> + KBUILD_MODNAME, vendor, product);
> +
> + return true;
> +}
> +
> static int __init alix_init(void)
> {
> const char tinybios_sig[] = "PC Engines ALIX.";
> @@ -128,8 +197,9 @@ static int __init alix_init(void)
> if (!is_geode())
> return 0;
>
> - if (alix_present(0xf0000, tinybios_sig, sizeof(tinybios_sig)
> - 1) ||
> - alix_present(0x500, coreboot_sig, sizeof(coreboot_sig) -
> 1))
> + if (alix_present(BIOS_SIGNATURE_TINYBIOS, tinybios_sig,
> sizeof(tinybios_sig) - 1) ||
> + alix_present(BIOS_SIGNATURE_COREBOOT, coreboot_sig,
> sizeof(coreboot_sig) - 1) ||
> + alix_present_dmi())
> register_alix();
>
> return 0;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/1] alix2: supplement driver to include GPIO button support
2012-01-23 19:56 [PATCH v3 1/1] alix2: supplement driver to include GPIO button support Philip Prindeville
2012-01-23 22:56 ` Andres Salomon
@ 2012-01-25 22:57 ` Andrew Morton
2012-01-25 23:04 ` Philip Prindeville
1 sibling, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2012-01-25 22:57 UTC (permalink / raw)
To: Philip Prindeville
Cc: platform-driver-x86, Alessandro Zummo, Richard Purdie,
Andres Salomon, Ed Wildgoose, David Woodhouse
On Mon, 23 Jan 2012 12:56:29 -0700
Philip Prindeville <philipp@redfish-solutions.com> wrote:
> From: "Philip A. Prindeville" <philipp@redfish-solutions.com>
>
> GPIO 24 is used in reference designs as a soft-reset button, and
> the alix2 is no exception. Add it as a gpio-button.
>
> Use symbolic values to describe BIOS addresses.
>
> Record the model number.
>
> Redux: address Andres' review comments; make the model number exported;
> add support for DMI detection of the board and manufacturer; change
> detection return type to bool; add comment explaining that the model
> number is used by downstream drivers to detect if we have RFKILL GPIO
> line to mini-PCIe slots (alix 6 only).
>
>
> ...
>
> arch/x86/platform/geode/alix.c | 80 +++++++++++++++++++++++++++++++++++++--
> 1 files changed, 75 insertions(+), 5 deletions(-)
>
> --- a/arch/x86/platform/geode/alix.c
> +++ b/arch/x86/platform/geode/alix.c
>
> ...
>
> @@ -24,14 +25,55 @@
> #include <linux/leds.h>
> #include <linux/platform_device.h>
> #include <linux/gpio.h>
> +#include <linux/input.h>
> +#include <linux/gpio_keys.h>
> +#include <linux/dmi.h>
>
> #include <asm/geode.h>
>
> +#define BIOS_SIGNATURE_TINYBIOS 0xf0000
> +#define BIOS_SIGNATURE_COREBOOT 0x500
> +#define BIOS_REGION_SIZE 0x10000
> +
> +/*
> + * the 6F2 has an RFKILL GPIO line to the mini-PCIe slots that isn't
> + * present on other Alix boards. Wifi drivers can use this symbol to
> + * detect if it's available.
> + */
> +int alix_model;
> +EXPORT_SYMBOL(alix_model);
This is odd. There are no references to this from outside this file
and it's hard to see how a wireless driver could use this - any such
driver would have to load this module on *all* machines (even non-x86)
simply to resolve this symbol.
>
> ...
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/1] alix2: supplement driver to include GPIO button support
2012-01-25 22:57 ` Andrew Morton
@ 2012-01-25 23:04 ` Philip Prindeville
2012-01-25 23:16 ` Andrew Morton
0 siblings, 1 reply; 9+ messages in thread
From: Philip Prindeville @ 2012-01-25 23:04 UTC (permalink / raw)
To: Andrew Morton
Cc: platform-driver-x86, Alessandro Zummo, Richard Purdie,
Andres Salomon, Ed Wildgoose, David Woodhouse
On 1/25/12 3:57 PM, Andrew Morton wrote:
> On Mon, 23 Jan 2012 12:56:29 -0700
> Philip Prindeville <philipp@redfish-solutions.com> wrote:
>
>> From: "Philip A. Prindeville" <philipp@redfish-solutions.com>
>>
>> GPIO 24 is used in reference designs as a soft-reset button, and
>> the alix2 is no exception. Add it as a gpio-button.
>>
>> Use symbolic values to describe BIOS addresses.
>>
>> Record the model number.
>>
>> Redux: address Andres' review comments; make the model number exported;
>> add support for DMI detection of the board and manufacturer; change
>> detection return type to bool; add comment explaining that the model
>> number is used by downstream drivers to detect if we have RFKILL GPIO
>> line to mini-PCIe slots (alix 6 only).
>>
>>
>> ...
>>
>> arch/x86/platform/geode/alix.c | 80 +++++++++++++++++++++++++++++++++++++--
>> 1 files changed, 75 insertions(+), 5 deletions(-)
>>
>> --- a/arch/x86/platform/geode/alix.c
>> +++ b/arch/x86/platform/geode/alix.c
>>
>> ...
>>
>> @@ -24,14 +25,55 @@
>> #include <linux/leds.h>
>> #include <linux/platform_device.h>
>> #include <linux/gpio.h>
>> +#include <linux/input.h>
>> +#include <linux/gpio_keys.h>
>> +#include <linux/dmi.h>
>>
>> #include <asm/geode.h>
>>
>> +#define BIOS_SIGNATURE_TINYBIOS 0xf0000
>> +#define BIOS_SIGNATURE_COREBOOT 0x500
>> +#define BIOS_REGION_SIZE 0x10000
>> +
>> +/*
>> + * the 6F2 has an RFKILL GPIO line to the mini-PCIe slots that isn't
>> + * present on other Alix boards. Wifi drivers can use this symbol to
>> + * detect if it's available.
>> + */
>> +int alix_model;
>> +EXPORT_SYMBOL(alix_model);
>
> This is odd. There are no references to this from outside this file
> and it's hard to see how a wireless driver could use this - any such
> driver would have to load this module on *all* machines (even non-x86)
> simply to resolve this symbol.
It's for an out-of-tree driver that's only ever built for Alix hardware.
Since it's only 4 bytes and one exported symbol, I figured it was acceptable...
I can remove it, resubmit, and use a patch locally in my tree if that's preferable.
-Philip
>
>>
>> ...
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/1] alix2: supplement driver to include GPIO button support
2012-01-25 23:04 ` Philip Prindeville
@ 2012-01-25 23:16 ` Andrew Morton
2012-01-25 23:47 ` Philip Prindeville
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2012-01-25 23:16 UTC (permalink / raw)
To: Philip Prindeville
Cc: platform-driver-x86, Alessandro Zummo, Richard Purdie,
Andres Salomon, Ed Wildgoose, David Woodhouse
On Wed, 25 Jan 2012 16:04:16 -0700
Philip Prindeville <philipp@redfish-solutions.com> wrote:
> > This is odd. There are no references to this from outside this file
> > and it's hard to see how a wireless driver could use this - any such
> > driver would have to load this module on *all* machines (even non-x86)
> > simply to resolve this symbol.
>
> It's for an out-of-tree driver that's only ever built for Alix hardware.
This should have been changelogged! And a code comment would be good,
too - if it confused me now, it will confused others later. And such a
code comment will help prevent others from coming in and "cleaning up"
the code later on.
Out-of-tree drivers are unpopular. Where is this driver, what is its
license and what are the prospects of making it in-tree?
I don't personally have problems with helping out-of-tree drivers but
making it EXPORT_SYMBOL_GPL() would set minds at rest.
> Since it's only 4 bytes and one exported symbol, I figured it was acceptable...
>
> I can remove it, resubmit, and use a patch locally in my tree if that's preferable
What we should do depends on the above issues...
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/1] alix2: supplement driver to include GPIO button support
2012-01-25 23:16 ` Andrew Morton
@ 2012-01-25 23:47 ` Philip Prindeville
2012-01-26 1:08 ` Andrew Morton
0 siblings, 1 reply; 9+ messages in thread
From: Philip Prindeville @ 2012-01-25 23:47 UTC (permalink / raw)
To: Andrew Morton
Cc: platform-driver-x86, Alessandro Zummo, Richard Purdie,
Andres Salomon, Ed Wildgoose, David Woodhouse
On 1/25/12 4:16 PM, Andrew Morton wrote:
> On Wed, 25 Jan 2012 16:04:16 -0700
> Philip Prindeville <philipp@redfish-solutions.com> wrote:
>
>>> This is odd. There are no references to this from outside this file
>>> and it's hard to see how a wireless driver could use this - any such
>>> driver would have to load this module on *all* machines (even non-x86)
>>> simply to resolve this symbol.
>>
>> It's for an out-of-tree driver that's only ever built for Alix hardware.
>
> This should have been changelogged! And a code comment would be good,
> too - if it confused me now, it will confused others later. And such a
> code comment will help prevent others from coming in and "cleaning up"
> the code later on.
Ok. There currently wasn't a ChangeLog in arch/x86/platform/geode so I can add one.
> Out-of-tree drivers are unpopular. Where is this driver, what is its
> license and what are the prospects of making it in-tree?
It's complicated. Generic GPIO supports polled-keys for input, and LEDs for outputs as you know.
There's no generic output mechanism for (say) an RFKILL line on the bus. If/when this materialized, I'll modify the alix driver to register that device in addition to the soft-reset button and the output LEDs (for the alix.6 device only).
There's also a certain amount of churn going on right now in coreboot about supporting the 'alix.6' as a variant of the 'alix.2' (the coreboot build machinery doesn't support this, and we need to hack kconfig to make this happen, i.e. have kconfig be queryable from shell scripts like coreboot's abuild)... so for now most people burn an alix.2 coreboot image onto their alix.6.
So there's a chain of dependencies that need to be resolved to get the ideal solution in place... but not wanting the perfect be the enemy of the good, I wanted to get what's available today out there with the caveat that something better is in the pipeline.
> I don't personally have problems with helping out-of-tree drivers but
> making it EXPORT_SYMBOL_GPL() would set minds at rest.
Ok. I'll make that patch and resubmit...
>> Since it's only 4 bytes and one exported symbol, I figured it was acceptable...
>>
>> I can remove it, resubmit, and use a patch locally in my tree if that's preferable
>
> What we should do depends on the above issues...
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/1] alix2: supplement driver to include GPIO button support
2012-01-25 23:47 ` Philip Prindeville
@ 2012-01-26 1:08 ` Andrew Morton
2012-01-26 1:37 ` Philip Prindeville
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2012-01-26 1:08 UTC (permalink / raw)
To: Philip Prindeville
Cc: platform-driver-x86, Alessandro Zummo, Richard Purdie,
Andres Salomon, Ed Wildgoose, David Woodhouse
On Wed, 25 Jan 2012 16:47:43 -0700
Philip Prindeville <philipp@redfish-solutions.com> wrote:
> On 1/25/12 4:16 PM, Andrew Morton wrote:
> > On Wed, 25 Jan 2012 16:04:16 -0700
> > Philip Prindeville <philipp@redfish-solutions.com> wrote:
> >
> >>> This is odd. There are no references to this from outside this file
> >>> and it's hard to see how a wireless driver could use this - any such
> >>> driver would have to load this module on *all* machines (even non-x86)
> >>> simply to resolve this symbol.
> >>
> >> It's for an out-of-tree driver that's only ever built for Alix hardware.
> >
> > This should have been changelogged! And a code comment would be good,
> > too - if it confused me now, it will confused others later. And such a
> > code comment will help prevent others from coming in and "cleaning up"
> > the code later on.
>
> Ok. There currently wasn't a ChangeLog in arch/x86/platform/geode so I can add one.
No, I'm referring to this patch's changelog - the metadata which gets
committed along with the code changes. Adding change info to the .c
file is lame - don't do that ;)
> > Out-of-tree drivers are unpopular. Where is this driver, what is its
> > license and what are the prospects of making it in-tree?
>
> It's complicated. Generic GPIO supports polled-keys for input, and LEDs for outputs as you know.
>
> There's no generic output mechanism for (say) an RFKILL line on the bus. If/when this materialized, I'll modify the alix driver to register that device in addition to the soft-reset button and the output LEDs (for the alix.6 device only).
>
> There's also a certain amount of churn going on right now in coreboot about supporting the 'alix.6' as a variant of the 'alix.2' (the coreboot build machinery doesn't support this, and we need to hack kconfig to make this happen, i.e. have kconfig be queryable from shell scripts like coreboot's abuild)... so for now most people burn an alix.2 coreboot image onto their alix.6.
>
> So there's a chain of dependencies that need to be resolved to get the ideal solution in place... but not wanting the perfect be the enemy of the good, I wanted to get what's available today out there with the caveat that something better is in the pipeline.
That didn't really answer my question about the whereabouts of the
mystery wireless driver. Oh well, it doesn't matter much.
>
> > I don't personally have problems with helping out-of-tree drivers but
> > making it EXPORT_SYMBOL_GPL() would set minds at rest.
>
> Ok. I'll make that patch and resubmit...
Add a comment there too, otherwise someone will come along and zap it.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/1] alix2: supplement driver to include GPIO button support
2012-01-26 1:08 ` Andrew Morton
@ 2012-01-26 1:37 ` Philip Prindeville
2012-01-26 10:15 ` Ed Wildgoose
0 siblings, 1 reply; 9+ messages in thread
From: Philip Prindeville @ 2012-01-26 1:37 UTC (permalink / raw)
To: Andrew Morton
Cc: platform-driver-x86, Alessandro Zummo, Richard Purdie,
Andres Salomon, Ed Wildgoose, David Woodhouse
On 1/25/12 6:08 PM, Andrew Morton wrote:
>>> Out-of-tree drivers are unpopular. Where is this driver, what is its
>>> license and what are the prospects of making it in-tree?
>>
>> It's complicated. Generic GPIO supports polled-keys for input, and LEDs for outputs as you know.
>>
>> There's no generic output mechanism for (say) an RFKILL line on the bus. If/when this materialized, I'll modify the alix driver to register that device in addition to the soft-reset button and the output LEDs (for the alix.6 device only).
>>
>> There's also a certain amount of churn going on right now in coreboot about supporting the 'alix.6' as a variant of the 'alix.2' (the coreboot build machinery doesn't support this, and we need to hack kconfig to make this happen, i.e. have kconfig be queryable from shell scripts like coreboot's abuild)... so for now most people burn an alix.2 coreboot image onto their alix.6.
>>
>> So there's a chain of dependencies that need to be resolved to get the ideal solution in place... but not wanting the perfect be the enemy of the good, I wanted to get what's available today out there with the caveat that something better is in the pipeline.
>
> That didn't really answer my question about the whereabouts of the
> mystery wireless driver. Oh well, it doesn't matter much.
Oh, yeah, sorry. The driver is currently only in my tree, though I had wanted to get it into OpenWRT but after having even linux-atm upstream patches not get retrofitted into the OpenWRT sources, I've given up on submitting additional patches.
The short answer was that in a perfect world, I won't need the out-of-tree driver and will be able to do everything in the alix2.c platform driver when GPIO support for non-button-or-LED-thingies gets added... at which point alix_model can be deprecated.
>>> I don't personally have problems with helping out-of-tree drivers but
>>> making it EXPORT_SYMBOL_GPL() would set minds at rest.
>>
>> Ok. I'll make that patch and resubmit...
>
> Add a comment there too, otherwise someone will come along and zap it.
>
Ok, will do.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/1] alix2: supplement driver to include GPIO button support
2012-01-26 1:37 ` Philip Prindeville
@ 2012-01-26 10:15 ` Ed Wildgoose
0 siblings, 0 replies; 9+ messages in thread
From: Ed Wildgoose @ 2012-01-26 10:15 UTC (permalink / raw)
To: Philip Prindeville
Cc: Andrew Morton, platform-driver-x86, Alessandro Zummo,
Richard Purdie, Andres Salomon, Ed Wildgoose, David Woodhouse
On 26/01/2012 01:37, Philip Prindeville wrote:
> On 1/25/12 6:08 PM, Andrew Morton wrote:
>
>> That didn't really answer my question about the whereabouts of the
>> mystery wireless driver. Oh well, it doesn't matter much.
> Oh, yeah, sorry. The driver is currently only in my tree, though I had wanted to get it into OpenWRT but after having even linux-atm upstream patches not get retrofitted into the OpenWRT sources, I've given up on submitting additional patches.
>
Any chance you would post the driver somewhere public so it's archived
for other hackers to play with?
Cheers for all your work on the Alix stuff - it still seems to have a
lot of life left, so thanks for working on it
Ed W
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-01-26 10:15 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-23 19:56 [PATCH v3 1/1] alix2: supplement driver to include GPIO button support Philip Prindeville
2012-01-23 22:56 ` Andres Salomon
2012-01-25 22:57 ` Andrew Morton
2012-01-25 23:04 ` Philip Prindeville
2012-01-25 23:16 ` Andrew Morton
2012-01-25 23:47 ` Philip Prindeville
2012-01-26 1:08 ` Andrew Morton
2012-01-26 1:37 ` Philip Prindeville
2012-01-26 10:15 ` Ed Wildgoose
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.