Re: [PATCH] Module argument to control whether intel-spi-pci attempts to turn the SPI flash chip writeable
On Mon, 3 Aug 2020 at 10:57, Mika Westerberg wrote: > I think instead of this we should simply make it so that the driver > never tries to make the chip writable. I think this is a good idea, but I wasn't sure if it was an acceptable behaviour change. Should the driver still try to set BCR_WPD when writing an image (i.e. defer the setting of write enable until later), or just not set the BCR register at all? I think your last comment was the latter, but wanted to check. Richard.
Re: [PATCH] SPI LPC information kernel module
On Tue, 30 Jun 2020 at 09:56, Greg Kroah-Hartman wrote: > Again, which makes it seem like securityfs is not the thing for this, as > it describes the hardware, not a security model which is what securityfs > has been for in the past, right? It describes the hardware platform. From a fwupd perspective I don't mind if the BC attributes are read from securityfs, sysfs or even read from an offset in a file from /proc... I guess sysfs makes sense if securityfs is defined for things like the LSM or lockdown status, although I also thought sysfs was for devices *in* the platform, not the platform itself. I guess exposing the platform registers in sysfs is no more weird than exposing things like the mei device and rfkill. Richard
Re: [PATCH] Ability to read the MKTME status from userspace
On Fri, 19 Jun 2020 at 20:41, Andy Lutomirski wrote: > I don't object in principle to Linux giving userspace more visibility > into what's going on, but I'm not convinced that adding a new > must-support-for-a-long-time interface that only solves 5% of your > problem is worth it. At the moment the only visibility we have is "the CPU supports TME" and "the kernel printed a message in the journal". The sysfs/procfs file read allows us to notify the admin if the firmware is deliberately disabling TME for some reason, without resorting to `grep` on dmesg. I don't think perfect should be the enemy of the good. Richard.
Re: [PATCH] Ability to read the MKTME status from userspace
On Fri, 19 Jun 2020 at 17:41, Greg Kroah-Hartman wrote: > > Yes. I want to show the user *why* TME is not available. > So even if it is "available" that's fine, even if it is not being used? No, it's just one more thing we can check and report. For instance, "Full memory encryption: NO [firmware-disabled, unencrypted-swap, EFI memory map incomplete] > And how can you ever tell if a BIOS disables a CPU feature, yet the chip > still has it? Isn't that what the "x86/tme: enabled by BIOS" kernel log entry is for? Richard.
Re: [PATCH] Ability to read the MKTME status from userspace
On Fri, 19 Jun 2020 at 17:10, Borislav Petkov wrote: > - do you just want to display feature support? Yes. I want to show the user *why* TME is not available. Richard.
Re: [PATCH] Ability to read the MKTME status from userspace
On Fri, 19 Jun 2020 at 17:17, Borislav Petkov wrote: > Well, I believe all the kernel can do is supply bits of information - > just like MSRs - and depending on the settings of those bits, userspace > can decide what the situation is. For example: > bit 0 - CPUID support > bit 1 - BIOS enabled Yes, something like this would be entirely fine for my purpose -- I don't mind where or how I get the data out of the kernel, but parsing the entire systemd journal for a magic string is something I really want to avoid. Richard.
Re: [PATCH] Ability to read the MKTME status from userspace
On Fri, 19 Jun 2020 at 15:48, Dave Hansen wrote: > You cut out the important part. The "pretty sure" involves a bunch of > preconditions and knowing what your hardware configuration is in the > first place. Totally agree. > Let's take a step back. We add read-only ABIs so that decisions can be > made. What decision will somebody make from the ABI being proposed here? The question of "is my memory encrypted" is what I'm trying to decide. To the end user (or the person marking a compliance ticksheet for a government contract) all they want to know is the end result. At the moment for AMD SME this seems much simpler as there are less "preconditions". > Someone does 'cat /proc/mktme' (or whatever) and it says "1" or > whatever, which means yay, encryption is on. What do they do? I think "is my memory encrypted" for Intel has to be a superset of: 1. TME in CPU info 2. not disabled by the platform 3. not using unencrypted swap 4. not using a memory accelerator 5. entire DRAM area is marked with EFI_MEMORY_CPU_CRYPTO It seems the only way to answer the questions and make it easy for the consumer to know the answer is to ask the kernel for each of the 5 different questions. At the moment we can only get 1, 3, maybe 4, soon to be 5, but not 2. Richard.
Re: [PATCH] Ability to read the MKTME status from userspace
On Fri, 19 Jun 2020 at 15:23, Dave Hansen wrote: > Last night, I asked my kids if they brushed their teeth. They said: > "Dad, my toothbrush was available." They argued that mere availability > was a better situation than not *having* a toothbrush. They were > logically right, of course, but they still got cavities. I don't see how that's comparable, sorry. Surely Intel wants to sell hardware advertising TME as a security feature? > > So my take-away from that is that it's currently impossible to > > actually say if your system is *actually* using TME. > Not in a generic way, and it can't be derived from cpuid or MSRs alone. Well, it seems not in any way at the moment. > I'm pretty sure I'm using TME, but I didn't become sure from > poking at sysfs. How do you know that Lenovo didn't disable TME without looking at dmesg? I don't think "pretty sure" is good enough when TME is considered a security feature. Richard
Re: [PATCH] Ability to read the MKTME status from userspace
On Fri, 19 Jun 2020 at 14:58, Dave Hansen wrote: > > Right, but for the most part you'd agree that a machine with > > functioning TME and encrypted swap partition is more secure than a > > machine without TME? > > Nope. There might be zero memory connected to the memory controller > that supports TME. So you're saying that a machine with TME available and enabled is not considered more secure than a machine without TME? What I want to do is have a sliding scale of TME not available < TME available but disabled < TME available and enabled < TME available, enabled and being used. The extra nugget of data gets me from state 2 to state 3. > > Can I use TME if the CPU supports it, but the platform has disabled > > it? How do I know that my system is actually *using* the benefits the > > TME feature provides? > > You must have a system with UEFI 2.8, ensure TME is enabled, then make > sure the OS parses EFI_MEMORY_CPU_CRYPTO, then ensure you request that > you data be placed in a region marked with EFI_MEMORY_CPU_CRYPTO, and > that it be *kept* there (hint: NUMA APIs don't do this). So my take-away from that is that it's currently impossible to actually say if your system is *actually* using TME. Richard.
Re: [PATCH] Ability to read the MKTME status from userspace
On Fri, 19 Jun 2020 at 14:44, Borislav Petkov wrote: > Yes, this is what I'm proposing with clearing the flag in /proc/cpuinfo. > The needed information is there: > 1. TME in CPUID > 2. TME *not* in /proc/cpuinfo No, it's not a boolean at all. If the platform disable is a BIOS configuration we don't know if TME isn't available because the CPU doesn't support it or because the firmware has disabled it. In the latter case, a firmware update or firmware configuration change might actually enable it. If the user installs a CPU with TME support and then we tell the user "your system doesn't support TME" then we're going to have some very confused users unless we can differentiate the two cases. > Along with proper ABI definition, design, > documentation and all that belongs to a proper interface with userspace. I don't think Daniels patch was a "final version" and I'm sure follow-ups can add this kind of thing. At the moment it's just people telling him "you don't need this" when as a potential consumer I'm saying we really do. Richard.
Re: [PATCH] Ability to read the MKTME status from userspace
On Fri, 19 Jun 2020 at 14:33, Dave Hansen wrote: > On top of that, the kernel can just swap data out to unencrypted storage. Right, but for the most part you'd agree that a machine with functioning TME and encrypted swap partition is more secure than a machine without TME? > So, I really wonder what folks want from this flag in the first place. > It really tells you _nothing_. Can I use TME if the CPU supports it, but the platform has disabled it? How do I know that my system is actually *using* the benefits the TME feature provides? Richard.
Re: [PATCH] Ability to read the MKTME status from userspace
On Fri, 19 Jun 2020 at 14:22, Borislav Petkov wrote: > And how is the user going to know from your "module"? AFAICT, your > module loads on any system - not only on ones which have MKTME in CPUID. I maintain fwupd, which would be one consumer of this information. At the moment we already look at the CPUID for the TME flag, which successfully recognises CPU systems which support the feature. What we don't know is if the firmware platform has disabled the MKTME feature. Ideally we would export two things: 1. that the CPU supports TME (->cpuid, already done) 2. that the platform has not disabled TME in some way The only way we have at the moment to see if TME is supported on the platform (rather than just the CPU) is by grepping the entire systemd journal at boot time, grepping for the "x86/tme: enabled by BIOS" string. With a securityfs/sysfs/procfs file we don't have to do this expensive operation for reading one tiny bit of data. Richard
Re: [PATCH] Ability to read the MKTME status from userspace
On Fri, 19 Jun 2020 at 00:52, Dave Hansen wrote: > It doesn't tell you if your data is encrypted. Sorry for the perhaps naive question, but I thought MKTME was essentially full physical memory encryption? Richard
Re: [PATCH] mfd: Export LPC attributes for the system SPI chip
On Thu, 14 May 2020 at 13:15, Mika Westerberg
wrote:
> > +What:/sys/kernel/security/firmware/bioswe
> Should this still be "firmware_protections" or similar. Plain "firmware"
> sounds again too generic. Maybe its just me..
It's not always going to be protections provided by the firmware; it
might also be restrictions put on the firmware. My first choice was
/sys/kernel/security/firmware_security/ but having the double
'security' just looked redundant.
> > + LPC_SPT,/* Sunrise Point */
> > + LPC_KBL,/* Kaby Lake */
> > + LPC_TGL,/* Tiger Lake */
>
> These all have the SPI-NOR controller as separate PCI device (as ICL and
> others).
For Sunrise Point I see:
00:1f.0 ISA bridge [0601]: Intel Corporation CM236 Chipset LPC/eSPI
Controller [8086:a150] (rev 31)
00:1f.0 ISA bridge [0601]: Intel Corporation Sunrise Point LPC
Controller/eSPI Controller [8086:9d4e] (rev 21)
For Kaby Lake I see:
00:1f.0 ISA bridge [0601]: Intel Corporation HM175 Chipset LPC/eSPI
Controller [8086:a152] (rev 31)",
You're indeed correct about Tiger Lake, my apologies.
> > + [LPC_SPT] = {
> > + .name = "Sunrise Point",
> > + .spi_type = INTEL_SPI_LPC,
> > + },
>
> So all of these have LCP/eSPI controller but the SPI-NOR controller is
> not accessible through it - it is a separate PCI device.
I have a Sunrise Point system here -- the lspci is here:
https://people.freedesktop.org/~hughsient/temp/lspci.txt
Is the SPI-NOR controller perhaps hidden? If I read the BCR @ 0xdc
from the 00:1f.0 ISB bridge I get the expected BIOS_WE, BLE and
SMM_BWP results.
> Like you said, Evolution seems to mangle these.
I'll use git for future patches, thanks.
> > + pci_read_config_dword(dev, BCR, );
> > + info->writeable = !!(bcr & BCR_WPD);
> > + break;
> > +
> > + case INTEL_SPI_LPC:
>
> So instead of this, you can add the security attributes to the existing
> entries where we are sure there is SPI-NOR controller behind LPC. Here
> it is not the case and further..
Sooo I'd use INTEL_SPI_LPT? On my system RCBA isn't set, and so "if
(!res->start)" bails out with return -ENODEV;"
> Otherwise this looks good, nice work :)
I thank you for your patience so far, what I've got the hang of this I
promise to start being more useful.
Richard.
Re: [PATCH] mfd: Export LPC attributes for the system SPI chip
On Wed, 2020-05-13 at 19:25 +0300, Mika Westerberg wrote:
> This may be problematic if there is driver bound to the device and
> accessing the hardware simultaneusly. Although this is just read side
> and I don't think these registers have any side effects when you read
> them, so should not be an issue.
Right, agreed.
> > So Cannon Lake, Cannon Point and Ice Lake would go into
> > drivers/mtd/spi-nor/controllers/intel-spi-pci.c and the systems
> > like
> > Sunrise Point using an ISA bridge would use drivers/mfd/lpc_ich.c?
>
> Yes, something like that.
Okay, lets get lpc_ich.c sorted, then I'll look at implementing the
same securityfs interface for intel-spi-pci.c
> I would like to keep it (the label) there but I think we can label
> SPI_INTEL_SPI with that instead and then make the -pci.c to work
> standalone if CONFIG_SPI_INTEL_SPI is not enabled.
Okay, good idea. I'll have a look at this next week after we get the
ISB bridges agreed on.
New patch here, if you want me to split it up a bit please just ask how
you would like it split. I fear Evolution is mangling the patch so I
might have to indeed start using git-send-email. :(
Thanks,
Richard.
>From 0b395efde8da7d5099c87945def473ff164d1c4c Mon Sep 17 00:00:00 2001
From: Richard Hughes
Date: Mon, 11 May 2020 14:19:27 +0100
Subject: [PATCH] mfd: Export LPC attributes for the system SPI chip
Export standard SPI-specific config values from various LPC
controllers.
This allows userspace components such as fwupd to verify the most basic
SPI
protections are set correctly. For instance, checking BIOSWE is
disabled
and BLE is enabled.
Exporting these values from the kernel allows us to report the security
level of the platform without rebooting and running an EFI binary like
chipsec.
Signed-off-by: Richard Hughes
---
Documentation/ABI/testing/sysfs-security-spi | 23 +++
drivers/mfd/lpc_ich.c| 169 ++-
include/linux/platform_data/intel-spi.h | 1 +
3 files changed, 187 insertions(+), 6 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-security-spi
diff --git a/Documentation/ABI/testing/sysfs-security-spi
b/Documentation/ABI/testing/sysfs-security-spi
new file mode 100644
index ..6805b74d7036
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-security-spi
@@ -0,0 +1,23 @@
+What: /sys/kernel/security/firmware/bioswe
+Date: May 2020
+KernelVersion: 5.7.0
+Contact: rich...@hughsie.com
+Description: If the system firmware set BIOS Write Enable.
+ 0: writes disabled, 1: writes enabled.
+Users: https://github.com/fwupd/fwupd
+
+What: /sys/kernel/security/firmware/ble
+Date: May 2020
+KernelVersion: 5.7.0
+Contact: rich...@hughsie.com
+Description: If the system firmware set Bios Lock Enable.
+ 0: SMM lock disabled, 1: SMM lock enabled.
+Users: https://github.com/fwupd/fwupd
+
+What: /sys/kernel/security/firmware/smm_bwp
+Date: May 2020
+KernelVersion: 5.7.0
+Contact: rich...@hughsie.com
+Description: If the system firmware set SMM Bios Write Protect.
+ 0: writes disabled unless in SMM, 1: writes enabled.
+Users: https://github.com/fwupd/fwupd
diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
index 3bbb29a7e7a5..fab017efdb9d 100644
--- a/drivers/mfd/lpc_ich.c
+++ b/drivers/mfd/lpc_ich.c
@@ -33,6 +33,7 @@
* document number 322169-001, 322170-003: 5 Series, 3400 Series
(PCH)
* document number 320066-003, 320257-008: EP80597 (IICH)
* document number 324645-001, 324646-001: Cougar Point (CPT)
+ * document number 332690-006, 332691-003: C230 (CPT)
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -40,6 +41,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -68,6 +70,8 @@
#define SPIBASE_LPT_SZ 512
#define BCR0xdc
#define BCR_WPDBIT(0)
+#define BCR_BLEBIT(1)
+#define BCR_SMM_BWPBIT(5)
#define SPIBASE_APL_SZ 4096
@@ -93,6 +97,11 @@ struct lpc_ich_priv {
int abase_save; /* Cached ACPI base value */
int actrl_pbase_save; /* Cached ACPI control or PMC
base value */
int gctrl_save; /* Cached GPIO control value */
+
+ struct dentry *firmware_dir;/* SecurityFS entries */
+ struct dentry *firmware_bioswe;
+ struct dentry *firmware_ble;
+ struct dentry *firmware_smm_bwp;
};
static struct resource wdt_ich_res[] = {
@@ -221,6 +230,9 @@ enum lpc_chipsets {
LPC_APL,/* Apollo Lake SoC */
LPC_GLK,/* Gemini Lake SoC */
LPC_COUGARMOUNTAIN,/* Cougar Mountain SoC*/
+ LPC_SPT,/* Sunrise Point */
+ LPC_KBL,/* Kaby Lake */
+ LPC_TGL,/* Tiger Lake */
};
static struct
Re: [PATCH] mfd: Export LPC attributes for the system SPI chip
On Wed, 13 May 2020 at 10:11, Mika Westerberg wrote: > > I can fix up all those, but out of interest how did you "know" the > > right three digit identifier to use? > I work for Intel ;-) Hah, okay, thanks :) > > I'm really wondering if drivers/mfd/lpc_ich.c is the right place for > > this kind of "just expose one byte of PCI config space" functionality. > Ideally there is one driver per device. My idea in https://github.com/hughsie/spi_lpc was to not actually register a pci_driver. > If this is touching the 00:1f.5 PCI device (SPI-NOR controller) then the > right place is the intel-spi-pci.c as that's the driver for this > controller. So Cannon Lake, Cannon Point and Ice Lake would go into drivers/mtd/spi-nor/controllers/intel-spi-pci.c and the systems like Sunrise Point using an ISA bridge would use drivers/mfd/lpc_ich.c? > We can put this there so that it does not enable the SPI-NOR > functionality itself and the mark the SPI-NOR functionality only as > being dangerous or something like that. I think getting the distros to enable SPI_INTEL_SPI_PCI might be a tough sell. Could we perhaps remove the DANGEROUS label as it's not writeable without a module option? > > > > + char tmp[2]; > > > Wouldn't this need to account the '\0' as well? > You sprint() there "%d\n", so that includes a number, '\n' and '\0' unless > I'm missing something. Doh, of course you're right. Will fix, thanks. Richard
Re: [PATCH] mfd: Export LPC attributes for the system SPI chip
On Wed, 13 May 2020 at 08:08, Mika Westerberg
wrote:
> I think this one should contain KernelVersion as well, see
> Documentation/ABI/README.
Thanks, I'll fix that up.
> I think you can always include this header without #ifs
Thanks.
> > static struct resource wdt_ich_res[] = {
> > @@ -221,6 +236,16 @@ enum lpc_chipsets {
> > LPC_APL,/* Apollo Lake SoC */
> > LPC_GLK,/* Gemini Lake SoC */
> > LPC_COUGARMOUNTAIN,/* Cougar Mountain SoC*/
> > + LPC_SPT,/* Sunrise Point */
> > + LPC_KLK,/* Kaby Lake */
> KBL for Kaby Lake
I can fix up all those, but out of interest how did you "know" the
right three digit identifier to use?
> This is not PCH, Cactus Ridge is Thunderbolt host controller AFAIK.
This was suggested from someone testing the original spi_lpc.c code on
a macbook, I can remove it for now and work out if it's incorrect
later.
> For example these PCI IDs are for the SPI-NOR controller (not LPC
> controller) so this causes this driver to try to bind to a completely
> different device which it cannot handle.
I'm really wondering if drivers/mfd/lpc_ich.c is the right place for
this kind of "just expose one byte of PCI config space" functionality.
Certainly drivers/platform/x86/intel_spi_lpc.c is much simpler, and
would also allow me to do some of the chipsec tests in the future --
for instance if BIOSWE is unset but BLE is set, try setting BIOSWE and
check that SMM clears it back.
> > + char tmp[2];
>
> Wouldn't this need to account the '\0' as well?
It's one char ('1' or '0') plus '`\0` -- no?
> I think "spi" is bit too general name here. I would expect "spi" to
> actually refer to something connected to spi bus and possibly coming
> from drivers/spi/*.
> Perhaps "bios_protections" or something like that.
Sure, that's a good idea. I know BIOS is a badword now, so how about
just "firmware"? so /sys/kernel/security/firmware/bioswe
> > + securityfs_remove(priv->spi_dir);
> > + return -1;
> I don't know securityfs well enought but I think -1 is not correct here
> and if you want that then maybe -EPERM instead.
I will look, I don't think the actual value is terribly important. The
only time I can trigger this is forgetting to remove the securityfs
file in module unload, and then trying to re-insert the module --
which failed with -EEXIST from memory.
> I wonder if you can simply call
> securityfs_remove(priv->spi_dir);
> and that removes the children automatically? I'm do not know securityfs
> so it may not be the case.
No, that doesn't work.
> > struct intel_spi_boardinfo {
> > enum intel_spi_type type;
> > bool writeable;
> > + bool ble;
> > + bool smm_bwp;
>
> I don't think these belong here. They should be part of the lpc private
> structure instead (lpc_ich_priv).
Can fix, thanks.
Richard.
[PATCH] mfd: Export LPC attributes for the system SPI chip
Export standard SPI-specific config values from various LPC
controllers.
This allows userspace components such as fwupd to verify the most basic
SPI
protections are set correctly. For instance, checking BIOSWE is
disabled
and BLE is enabled.
Exporting these values from the kernel allows us to report the security
level of the platform without rebooting and running an EFI binary like
chipsec.
Signed-off-by: Richard Hughes
---
Documentation/ABI/testing/sysfs-security-spi | 17 ++
drivers/mfd/lpc_ich.c| 225 ++-
include/linux/platform_data/intel-spi.h | 7 +-
3 files changed, 242 insertions(+), 7 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-security-spi
diff --git a/Documentation/ABI/testing/sysfs-security-spi
b/Documentation/ABI/testing/sysfs-security-spi
new file mode 100644
index ..ee867b1366f9
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-security-spi
@@ -0,0 +1,17 @@
+What: /sys/kernel/security/spi/bioswe
+Date: May 2020
+Contact:platform-driver-...@vger.kernel.org
+Description:If the system firmware set BIOS Write Enable.
+ 0: writes disabled, 1: writes enabled.
+
+What: /sys/kernel/security/spi/ble
+Date: May 2020
+Contact:platform-driver-...@vger.kernel.org
+Description:If the system firmware set Bios Lock Enable.
+ 0: SMM lock disabled, 1: SMM lock enabled.
+
+What: /sys/kernel/security/spi/smm_bwp
+Date: May 2020
+Contact:platform-driver-...@vger.kernel.org
+Description:If the system firmware set SMM Bios Write Protect.
+ 0: writes disabled unless in SMM, 1: writes enabled.
diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
index 3bbb29a7e7a5..e9a97c461d9a 100644
--- a/drivers/mfd/lpc_ich.c
+++ b/drivers/mfd/lpc_ich.c
@@ -33,6 +33,8 @@
* document number 322169-001, 322170-003: 5 Series, 3400 Series
(PCH)
* document number 320066-003, 320257-008: EP80597 (IICH)
* document number 324645-001, 324646-001: Cougar Point (CPT)
+ * document number 332690-006, 332691-003: C230 (CPT)
+ * document number 337867-003, 337868-002: Cannon Point (PCH)
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -46,6 +48,10 @@
#include
#include
+#if IS_ENABLED(CONFIG_SECURITY)
+#include
+#endif
+
#define ACPIBASE 0x40
#define ACPIBASE_GPE_OFF 0x28
#define ACPIBASE_GPE_END 0x2f
@@ -68,6 +74,8 @@
#define SPIBASE_LPT_SZ 512
#define BCR0xdc
#define BCR_WPDBIT(0)
+#define BCR_BLEBIT(1)
+#define BCR_SMM_BWPBIT(5)
#define SPIBASE_APL_SZ 4096
@@ -93,6 +101,13 @@ struct lpc_ich_priv {
int abase_save; /* Cached ACPI base value */
int actrl_pbase_save; /* Cached ACPI control or PMC
base value */
int gctrl_save; /* Cached GPIO control value */
+
+#if IS_ENABLED(CONFIG_SECURITY)
+ struct dentry *spi_dir; /* SecurityFS entries */
+ struct dentry *spi_bioswe;
+ struct dentry *spi_ble;
+ struct dentry *spi_smm_bwp;
+#endif
};
static struct resource wdt_ich_res[] = {
@@ -221,6 +236,16 @@ enum lpc_chipsets {
LPC_APL,/* Apollo Lake SoC */
LPC_GLK,/* Gemini Lake SoC */
LPC_COUGARMOUNTAIN,/* Cougar Mountain SoC*/
+ LPC_SPT,/* Sunrise Point */
+ LPC_KLK,/* Kaby Lake */
+ LPC_CNPT, /* Cannon Point */
+ LPC_CLK,/* Comet Lake */
+ LPC_ILK,/* Ice Lake */
+ LPC_ELK,/* Elkhart Lake */
+ LPC_JLK,/* Jasper Lake */
+ LPC_TLK,/* Tiger Lake */
+ LPC_CNLK, /* Cannon Lake */
+ LPC_CRDG, /* Cactus Ridge */
};
static struct lpc_ich_info lpc_chipset_info[] = {
@@ -557,6 +582,46 @@ static struct lpc_ich_info lpc_chipset_info[] = {
.name = "Cougar Mountain SoC",
.iTCO_version = 3,
},
+ [LPC_SPT] = {
+ .name = "Sunrise Point",
+ .spi_type = INTEL_SPI_LPC,
+ },
+ [LPC_KLK] = {
+ .name = "Kaby Lake-H",
+ .spi_type = INTEL_SPI_LPC,
+ },
+ [LPC_CNPT] = {
+ .name = "Cannon Point",
+ .spi_type = INTEL_SPI_LPC,
+ },
+ [LPC_CLK] = {
+ .name = "Comet Lake",
+ .spi_type = INTEL_SPI_LPC,
+ },
+ [LPC_ILK] = {
+ .name = "Ice Lake",
+ .spi_type = INTEL_SPI_LPC,
+ },
+ [LPC_ELK] = {
+ .name = "Elkhart Lake",
+ .spi_type = INTEL_SPI_LPC,
+ },
+ [LPC_JLK] = {
+ .name = "Jasper Lake",
+ .spi_type = INTEL_SPI_LPC,
+
Re: [PATCH 2/2] Introduce CONFIG_SUSPEND
On Sun, 2007-07-29 at 23:36 +0200, Rafael J. Wysocki wrote: > On Sunday, 29 July 2007 23:18, Adrian Bunk wrote: > > On Sun, Jul 29, 2007 at 11:17:20PM +0200, Rafael J. Wysocki wrote: > > > On Sunday, 29 July 2007 22:40, Adrian Bunk wrote: > > > > On Sun, Jul 29, 2007 at 02:38:05PM +0200, Rafael J. Wysocki wrote: > > > > >... > > > > > +config SUSPEND > > > > > + bool "Suspend" > > > > "Suspend to RAM" > > > > > > Not only. This also includes "standby". > > > > Whatever it includes - please tell it to the user in the prompt. > > > > Technical issues are important, but it's often forgotten how many > > problems people run into because the description of a kconfig option > > could have been better. > > Sure. Please see the updated patch I've just sent. :-) So are you guys using: "standby" = idle state, ~0.5 seconds "suspend" = sleep to ram, ~10 seconds "hibernate" = sleep to disk, ~30 seconds If so - you rock. This is the common nomenclature I've been pushing for a few months now in GNOME, KDE and general userspace. I've written up a spec here: http://cvs.gnome.org/viewcvs/*checkout*/gnome-power-manager/docs/sleep-names.html Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] Introduce CONFIG_SUSPEND
On Sun, 2007-07-29 at 23:36 +0200, Rafael J. Wysocki wrote: On Sunday, 29 July 2007 23:18, Adrian Bunk wrote: On Sun, Jul 29, 2007 at 11:17:20PM +0200, Rafael J. Wysocki wrote: On Sunday, 29 July 2007 22:40, Adrian Bunk wrote: On Sun, Jul 29, 2007 at 02:38:05PM +0200, Rafael J. Wysocki wrote: ... +config SUSPEND + bool Suspend Suspend to RAM Not only. This also includes standby. Whatever it includes - please tell it to the user in the prompt. Technical issues are important, but it's often forgotten how many problems people run into because the description of a kconfig option could have been better. Sure. Please see the updated patch I've just sent. :-) So are you guys using: standby = idle state, ~0.5 seconds suspend = sleep to ram, ~10 seconds hibernate = sleep to disk, ~30 seconds If so - you rock. This is the common nomenclature I've been pushing for a few months now in GNOME, KDE and general userspace. I've written up a spec here: http://cvs.gnome.org/viewcvs/*checkout*/gnome-power-manager/docs/sleep-names.html Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH][ACPI][BUTTON] remove procfs-interface
On Thu, 2007-07-12 at 17:46 +0800, Zhang, Rui wrote: > > I'm not sure if the button sysfs I/F is already finished. > We'd better make a double check. :) We need a button sysfs interface? What's wrong with just using input? Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] the scheduled ACPI_PROCFS removal
On Thu, 2007-07-12 at 09:32 +0400, Alexey Starikovskiy wrote:
> >> [*] Does someone have an alternative for
> >> /proc/acpi/battery/BAT1/{state,info}?
> I'm working on it. Should have proto by the end of week.
If you are using the power_supply class (i hope you are ;-) then a HAL
from freedesktop git should make userspace continue to just work. Please
yell if you notice any differences.
Richard.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] the scheduled ACPI_PROCFS removal
On Thu, 2007-07-12 at 09:32 +0400, Alexey Starikovskiy wrote:
[*] Does someone have an alternative for
/proc/acpi/battery/BAT1/{state,info}?
I'm working on it. Should have proto by the end of week.
If you are using the power_supply class (i hope you are ;-) then a HAL
from freedesktop git should make userspace continue to just work. Please
yell if you notice any differences.
Richard.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH][ACPI][BUTTON] remove procfs-interface
On Thu, 2007-07-12 at 17:46 +0800, Zhang, Rui wrote: I'm not sure if the button sysfs I/F is already finished. We'd better make a double check. :) We need a button sysfs interface? What's wrong with just using input? Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch] Reporting the lid status using INPUT
Len,
* user shuts lid, initiates suspend
{lid is reported down}
* user opens lid, resume completes
But the lid INPUT up event is not sent. This works if you don't initiate
suspend when you shut the lid (blank screen for instance) -- so I know
my hardware is okay.
I see:
input_report_switch(input, SW_LID, !state);
in response to an event, but in a resume hook we should probably do
acpi_evaluate_integer(handle, "_LID", NULL, ) and then send an
event, just so userspace is aware of what the state of the panel is.
If switch state matches to what input layer thinks it is the event will
not be sent.
Signed-off-by: Richard Hughes <[EMAIL PROTECTED]>
--- Begin Message ---
On resume we need to refresh the lid status as we will not get an event if
the lid opening was what triggered the suspend.
This manifests itself in users never getting a "lid open" event when a
suspend happens because of lid close on hardware that supports wake on
lid open. This makes userspace gets very confused indeed.
Patch attached forces a check of the lid status in the resume handler.
Signed-off-by: Richard Hughes <[EMAIL PROTECTED]>
---
diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index cb4110b..fd3473b 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -68,6 +68,7 @@ MODULE_LICENSE("GPL");
static int acpi_button_add(struct acpi_device *device);
static int acpi_button_remove(struct acpi_device *device, int type);
+static int acpi_button_resume(struct acpi_device *device);
static int acpi_button_info_open_fs(struct inode *inode, struct file *file);
static int acpi_button_state_open_fs(struct inode *inode, struct file *file);
@@ -77,6 +78,7 @@ static struct acpi_driver acpi_button_driver = {
.ids = "button_power,button_sleep,PNP0C0D,PNP0C0C,PNP0C0E",
.ops = {
.add = acpi_button_add,
+ .resume = acpi_button_resume,
.remove = acpi_button_remove,
},
};
@@ -487,6 +489,29 @@ static int acpi_button_remove(struct acpi_device *device,
int type)
return 0;
}
+/* this is needed to learn about changes made in suspended state */
+static int acpi_button_resume(struct acpi_device *device)
+{
+ struct acpi_button *button;
+ struct acpi_handle *handle;
+ struct input_dev *input;
+ unsigned long state;
+
+ button = device->driver_data;
+ handle = button->device->handle;
+ input = button->input;
+
+ /*
+* On resume we send the state; if it matches to what input layer
+* thinks then the event will not even reach userspace.
+*/
+ if (!ACPI_FAILURE(acpi_evaluate_integer(handle, "_LID",
+ NULL, )))
+ input_report_switch(input, SW_LID, !state);
+
+ return 0;
+}
+
static int __init acpi_button_init(void)
{
int result;
--- End Message ---
[patch] Reporting the lid status using INPUT
Len,
* user shuts lid, initiates suspend
{lid is reported down}
* user opens lid, resume completes
But the lid INPUT up event is not sent. This works if you don't initiate
suspend when you shut the lid (blank screen for instance) -- so I know
my hardware is okay.
I see:
input_report_switch(input, SW_LID, !state);
in response to an event, but in a resume hook we should probably do
acpi_evaluate_integer(handle, _LID, NULL, state) and then send an
event, just so userspace is aware of what the state of the panel is.
If switch state matches to what input layer thinks it is the event will
not be sent.
Signed-off-by: Richard Hughes [EMAIL PROTECTED]
---BeginMessage---
On resume we need to refresh the lid status as we will not get an event if
the lid opening was what triggered the suspend.
This manifests itself in users never getting a lid open event when a
suspend happens because of lid close on hardware that supports wake on
lid open. This makes userspace gets very confused indeed.
Patch attached forces a check of the lid status in the resume handler.
Signed-off-by: Richard Hughes [EMAIL PROTECTED]
---
diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index cb4110b..fd3473b 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -68,6 +68,7 @@ MODULE_LICENSE(GPL);
static int acpi_button_add(struct acpi_device *device);
static int acpi_button_remove(struct acpi_device *device, int type);
+static int acpi_button_resume(struct acpi_device *device);
static int acpi_button_info_open_fs(struct inode *inode, struct file *file);
static int acpi_button_state_open_fs(struct inode *inode, struct file *file);
@@ -77,6 +78,7 @@ static struct acpi_driver acpi_button_driver = {
.ids = button_power,button_sleep,PNP0C0D,PNP0C0C,PNP0C0E,
.ops = {
.add = acpi_button_add,
+ .resume = acpi_button_resume,
.remove = acpi_button_remove,
},
};
@@ -487,6 +489,29 @@ static int acpi_button_remove(struct acpi_device *device,
int type)
return 0;
}
+/* this is needed to learn about changes made in suspended state */
+static int acpi_button_resume(struct acpi_device *device)
+{
+ struct acpi_button *button;
+ struct acpi_handle *handle;
+ struct input_dev *input;
+ unsigned long state;
+
+ button = device-driver_data;
+ handle = button-device-handle;
+ input = button-input;
+
+ /*
+* On resume we send the state; if it matches to what input layer
+* thinks then the event will not even reach userspace.
+*/
+ if (!ACPI_FAILURE(acpi_evaluate_integer(handle, _LID,
+ NULL, state)))
+ input_report_switch(input, SW_LID, !state);
+
+ return 0;
+}
+
static int __init acpi_button_init(void)
{
int result;
---End Message---
Re: [patch] Move led attributes out of device name and into sysfs attributes, was Re: LED devices
On Tue, 2007-06-26 at 11:02 +0100, Richard Purdie wrote: > There were some other opinions voiced including one from the person > who started this discussion. > > So no, the people who write the tools that parse sysfs (like HAL.) > don't appreciate this. > > People who write tools that parse sysfs like shell scripts don't > appreciate it either, as I illustrated. >From a hal point of view, we don't care if the device name is 'led01' or 'light_to_dance_the_fandango' and from a shell point of view it's probably best for the latter. I think the point Greg tried to make is that it shouldn't matter, and HAL shouldn't export (nor parse) the device name as anything sensible. > You've yet to give any technical reason why we can't have meaningful > busids rather than random numbers. Your entire argument seems to be > that its wrong because its a bit different and nobody else does it... If it's a trivial name then I think led_thinklight0 is perfectly okay, I think Kay was talking more about the attribute vs. name-in-device encoding. Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Add INPUT support to toshiba_acpi
On Tue, 2007-06-26 at 07:03 +0200, Rolf Eike Beer wrote: > Richard Hughes wrote: > > On Sat, 2007-06-23 at 16:56 +0200, Rolf Eike Beer wrote: > > > None of the above keys generated a key event. Neither does > "Brightness > > > down", but it still works. "Brightness up" generates an event and > works. > > > Kpowersave tells me it can't do brightness switching in software > (which > > > works in WinXtraPain). > Yes, I need a special process running for this. Would it be of some > use if I take a look with IRPtracker on it or is that all you need to > know? Yes, although this is out of my area or expertise, sorry. Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Add INPUT support to toshiba_acpi
On Tue, 2007-06-26 at 07:03 +0200, Rolf Eike Beer wrote: Richard Hughes wrote: On Sat, 2007-06-23 at 16:56 +0200, Rolf Eike Beer wrote: None of the above keys generated a key event. Neither does Brightness down, but it still works. Brightness up generates an event and works. Kpowersave tells me it can't do brightness switching in software (which works in WinXtraPain). Yes, I need a special process running for this. Would it be of some use if I take a look with IRPtracker on it or is that all you need to know? Yes, although this is out of my area or expertise, sorry. Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] Move led attributes out of device name and into sysfs attributes, was Re: LED devices
On Tue, 2007-06-26 at 11:02 +0100, Richard Purdie wrote: There were some other opinions voiced including one from the person who started this discussion. So no, the people who write the tools that parse sysfs (like HAL.) don't appreciate this. People who write tools that parse sysfs like shell scripts don't appreciate it either, as I illustrated. From a hal point of view, we don't care if the device name is 'led01' or 'light_to_dance_the_fandango' and from a shell point of view it's probably best for the latter. I think the point Greg tried to make is that it shouldn't matter, and HAL shouldn't export (nor parse) the device name as anything sensible. You've yet to give any technical reason why we can't have meaningful busids rather than random numbers. Your entire argument seems to be that its wrong because its a bit different and nobody else does it... If it's a trivial name then I think led_thinklight0 is perfectly okay, I think Kay was talking more about the attribute vs. name-in-device encoding. Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] Reporting the lid status using INPUT
On Mon, 2007-06-25 at 22:49 +0200, Bartók Albert wrote: > GMail On 16/06/07, Dmitry Torokhov <[EMAIL PROTECTED]> wrote: > > On Saturday 16 June 2007 13:11, Richard Hughes wrote: > > > On Fri, 2007-06-15 at 15:53 +0100, Richard Hughes wrote: > > > > On Fri, 2007-06-15 at 10:29 -0400, Dmitry Torokhov wrote: > > > > > On 6/15/07, Richard Hughes <[EMAIL PROTECTED]> wrote: > > > > > > On Fri, 2007-06-15 at 13:29 +0100, Richard Hughes wrote: > > > > > > > in response to an event, but I'm thinking in a resume hook we > > > > > > > should > > > > > > > probably do acpi_evaluate_integer(handle, "_LID", NULL, ) > > > > > > > and then > > > > > > > send an event, just so userspace is aware of what the state of > > > > > > > the panel > > > > > > > is. > > > > > > > > > > > > Attached patch fixed the issue for me. Comments? > > > > > > > > > > > > > > > > The patch makes perfect sense. The only issue I have is this: > > > > > > > > > > > + /* on resume we send the state; it might be the same, but > > > > > > userspace > > > > > > +* should handle duplicated events */ > > > > > > > > > > If switch state matches to what input layer thinks it is the event > > > > > will not even reach userspace. > > > > > > > > Okay, new patch attached, thanks for the speedy review. > > > > > > This fix is also confirmed by somebody else, see > > > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=243030 > > > > > > It would be great if this could go into .22, although I appreciate it's > > > quite late in the day. > > > > > > > This is of course Len's call but in my book this is a bugfix and as such > > is appropriate for -rc4/rc5. > > > Guys? Any ack-nak? > > works perfectly for me so far. Thanks. Len, can you add this to your tree please. Thanks. Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Add INPUT support to toshiba_acpi
On Sat, 2007-06-23 at 16:56 +0200, Rolf Eike Beer wrote: > None of the above keys generated a key event. Neither does "Brightness down", > but it still works. "Brightness up" generates an event and works. Kpowersave > tells me it can't do brightness switching in software (which works in > WinXtraPain). Do you know how windows does this? Do you have to load a special system-try thing to make the keys work? Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Add INPUT support to toshiba_acpi
On Sat, 2007-06-23 at 16:56 +0200, Rolf Eike Beer wrote: None of the above keys generated a key event. Neither does Brightness down, but it still works. Brightness up generates an event and works. Kpowersave tells me it can't do brightness switching in software (which works in WinXtraPain). Do you know how windows does this? Do you have to load a special system-try thing to make the keys work? Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] Reporting the lid status using INPUT
On Mon, 2007-06-25 at 22:49 +0200, Bartók Albert wrote: GMail On 16/06/07, Dmitry Torokhov [EMAIL PROTECTED] wrote: On Saturday 16 June 2007 13:11, Richard Hughes wrote: On Fri, 2007-06-15 at 15:53 +0100, Richard Hughes wrote: On Fri, 2007-06-15 at 10:29 -0400, Dmitry Torokhov wrote: On 6/15/07, Richard Hughes [EMAIL PROTECTED] wrote: On Fri, 2007-06-15 at 13:29 +0100, Richard Hughes wrote: in response to an event, but I'm thinking in a resume hook we should probably do acpi_evaluate_integer(handle, _LID, NULL, state) and then send an event, just so userspace is aware of what the state of the panel is. Attached patch fixed the issue for me. Comments? The patch makes perfect sense. The only issue I have is this: + /* on resume we send the state; it might be the same, but userspace +* should handle duplicated events */ If switch state matches to what input layer thinks it is the event will not even reach userspace. Okay, new patch attached, thanks for the speedy review. This fix is also confirmed by somebody else, see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=243030 It would be great if this could go into .22, although I appreciate it's quite late in the day. This is of course Len's call but in my book this is a bugfix and as such is appropriate for -rc4/rc5. Guys? Any ack-nak? works perfectly for me so far. Thanks. Len, can you add this to your tree please. Thanks. Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] Reporting the lid status using INPUT
On Fri, 2007-06-15 at 15:53 +0100, Richard Hughes wrote: > On Fri, 2007-06-15 at 10:29 -0400, Dmitry Torokhov wrote: > > On 6/15/07, Richard Hughes <[EMAIL PROTECTED]> wrote: > > > On Fri, 2007-06-15 at 13:29 +0100, Richard Hughes wrote: > > > > in response to an event, but I'm thinking in a resume hook we should > > > > probably do acpi_evaluate_integer(handle, "_LID", NULL, ) and then > > > > send an event, just so userspace is aware of what the state of the panel > > > > is. > > > > > > Attached patch fixed the issue for me. Comments? > > > > > > > The patch makes perfect sense. The only issue I have is this: > > > > > + /* on resume we send the state; it might be the same, but userspace > > > + * should handle duplicated events */ > > > > If switch state matches to what input layer thinks it is the event > > will not even reach userspace. > > Okay, new patch attached, thanks for the speedy review. This fix is also confirmed by somebody else, see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=243030 It would be great if this could go into .22, although I appreciate it's quite late in the day. Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] Reporting the lid status using INPUT
On Fri, 2007-06-15 at 15:53 +0100, Richard Hughes wrote: On Fri, 2007-06-15 at 10:29 -0400, Dmitry Torokhov wrote: On 6/15/07, Richard Hughes [EMAIL PROTECTED] wrote: On Fri, 2007-06-15 at 13:29 +0100, Richard Hughes wrote: in response to an event, but I'm thinking in a resume hook we should probably do acpi_evaluate_integer(handle, _LID, NULL, state) and then send an event, just so userspace is aware of what the state of the panel is. Attached patch fixed the issue for me. Comments? The patch makes perfect sense. The only issue I have is this: + /* on resume we send the state; it might be the same, but userspace + * should handle duplicated events */ If switch state matches to what input layer thinks it is the event will not even reach userspace. Okay, new patch attached, thanks for the speedy review. This fix is also confirmed by somebody else, see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=243030 It would be great if this could go into .22, although I appreciate it's quite late in the day. Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] Reporting the lid status using INPUT
On Fri, 2007-06-15 at 10:29 -0400, Dmitry Torokhov wrote:
> On 6/15/07, Richard Hughes <[EMAIL PROTECTED]> wrote:
> > On Fri, 2007-06-15 at 13:29 +0100, Richard Hughes wrote:
> > > in response to an event, but I'm thinking in a resume hook we should
> > > probably do acpi_evaluate_integer(handle, "_LID", NULL, ) and then
> > > send an event, just so userspace is aware of what the state of the panel
> > > is.
> >
> > Attached patch fixed the issue for me. Comments?
> >
>
> The patch makes perfect sense. The only issue I have is this:
>
> > + /* on resume we send the state; it might be the same, but userspace
> > +* should handle duplicated events */
>
> If switch state matches to what input layer thinks it is the event
> will not even reach userspace.
Okay, new patch attached, thanks for the speedy review.
Signed-off-by: Richard Hughes <[EMAIL PROTECTED]>
--- Begin Message ---
On resume we need to refresh the lid status as we will not get an event if
the lid opening was what triggered the suspend.
This manifests itself in users never getting a "lid open" event when a
suspend happens because of lid close on hardware that supports wake on
lid open. This makes userspace gets very confused indeed.
Patch attached forces a check of the lid status in the resume handler.
Signed-off-by: Richard Hughes <[EMAIL PROTECTED]>
---
diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index cb4110b..fd3473b 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -68,6 +68,7 @@ MODULE_LICENSE("GPL");
static int acpi_button_add(struct acpi_device *device);
static int acpi_button_remove(struct acpi_device *device, int type);
+static int acpi_button_resume(struct acpi_device *device);
static int acpi_button_info_open_fs(struct inode *inode, struct file *file);
static int acpi_button_state_open_fs(struct inode *inode, struct file *file);
@@ -77,6 +78,7 @@ static struct acpi_driver acpi_button_driver = {
.ids = "button_power,button_sleep,PNP0C0D,PNP0C0C,PNP0C0E",
.ops = {
.add = acpi_button_add,
+ .resume = acpi_button_resume,
.remove = acpi_button_remove,
},
};
@@ -487,6 +489,29 @@ static int acpi_button_remove(struct acpi_device *device,
int type)
return 0;
}
+/* this is needed to learn about changes made in suspended state */
+static int acpi_button_resume(struct acpi_device *device)
+{
+ struct acpi_button *button;
+ struct acpi_handle *handle;
+ struct input_dev *input;
+ unsigned long state;
+
+ button = device->driver_data;
+ handle = button->device->handle;
+ input = button->input;
+
+ /*
+* On resume we send the state; if it matches to what input layer
+* thinks then the event will not even reach userspace.
+*/
+ if (!ACPI_FAILURE(acpi_evaluate_integer(handle, "_LID",
+ NULL, )))
+ input_report_switch(input, SW_LID, !state);
+
+ return 0;
+}
+
static int __init acpi_button_init(void)
{
int result;
--- End Message ---
Re: [2.6.22-rc3][ACPI?] Resume from s2r doesn't work.
On Thu, 2007-06-14 at 19:56 +, Pavel Machek wrote: > > noapic/nolapic will not help with video issue. try s2ram from > suspend.sf.net. Also there is (unashamed plug) http://people.freedesktop.org/~hughsient/quirk/ Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6.22-rc3][ACPI?] Resume from s2r doesn't work.
On Thu, 2007-06-14 at 19:56 +, Pavel Machek wrote: noapic/nolapic will not help with video issue. try s2ram from suspend.sf.net. Also there is (unashamed plug) http://people.freedesktop.org/~hughsient/quirk/ Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] Reporting the lid status using INPUT
On Fri, 2007-06-15 at 10:29 -0400, Dmitry Torokhov wrote:
On 6/15/07, Richard Hughes [EMAIL PROTECTED] wrote:
On Fri, 2007-06-15 at 13:29 +0100, Richard Hughes wrote:
in response to an event, but I'm thinking in a resume hook we should
probably do acpi_evaluate_integer(handle, _LID, NULL, state) and then
send an event, just so userspace is aware of what the state of the panel
is.
Attached patch fixed the issue for me. Comments?
The patch makes perfect sense. The only issue I have is this:
+ /* on resume we send the state; it might be the same, but userspace
+* should handle duplicated events */
If switch state matches to what input layer thinks it is the event
will not even reach userspace.
Okay, new patch attached, thanks for the speedy review.
Signed-off-by: Richard Hughes [EMAIL PROTECTED]
---BeginMessage---
On resume we need to refresh the lid status as we will not get an event if
the lid opening was what triggered the suspend.
This manifests itself in users never getting a lid open event when a
suspend happens because of lid close on hardware that supports wake on
lid open. This makes userspace gets very confused indeed.
Patch attached forces a check of the lid status in the resume handler.
Signed-off-by: Richard Hughes [EMAIL PROTECTED]
---
diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index cb4110b..fd3473b 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -68,6 +68,7 @@ MODULE_LICENSE(GPL);
static int acpi_button_add(struct acpi_device *device);
static int acpi_button_remove(struct acpi_device *device, int type);
+static int acpi_button_resume(struct acpi_device *device);
static int acpi_button_info_open_fs(struct inode *inode, struct file *file);
static int acpi_button_state_open_fs(struct inode *inode, struct file *file);
@@ -77,6 +78,7 @@ static struct acpi_driver acpi_button_driver = {
.ids = button_power,button_sleep,PNP0C0D,PNP0C0C,PNP0C0E,
.ops = {
.add = acpi_button_add,
+ .resume = acpi_button_resume,
.remove = acpi_button_remove,
},
};
@@ -487,6 +489,29 @@ static int acpi_button_remove(struct acpi_device *device,
int type)
return 0;
}
+/* this is needed to learn about changes made in suspended state */
+static int acpi_button_resume(struct acpi_device *device)
+{
+ struct acpi_button *button;
+ struct acpi_handle *handle;
+ struct input_dev *input;
+ unsigned long state;
+
+ button = device-driver_data;
+ handle = button-device-handle;
+ input = button-input;
+
+ /*
+* On resume we send the state; if it matches to what input layer
+* thinks then the event will not even reach userspace.
+*/
+ if (!ACPI_FAILURE(acpi_evaluate_integer(handle, _LID,
+ NULL, state)))
+ input_report_switch(input, SW_LID, !state);
+
+ return 0;
+}
+
static int __init acpi_button_init(void)
{
int result;
---End Message---
Re: [patch] Move led attributes out of device name and into sysfs attributes, was Re: LED devices
On Sun, 2007-06-10 at 12:11 +0200, Pavel Machek wrote: > Can we keep the original naming? spitz:disk is as unique as led02, and > it is _way_ easier to use. > Come on, I want to use the led subsystem from the scripts... I don't see a problem with spitz_disk, which is just as easy to use in scripts. Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] Move led attributes out of device name and into sysfs attributes, was Re: LED devices
On Sun, 2007-06-10 at 12:11 +0200, Pavel Machek wrote: Can we keep the original naming? spitz:disk is as unique as led02, and it is _way_ easier to use. Come on, I want to use the led subsystem from the scripts... I don't see a problem with spitz_disk, which is just as easy to use in scripts. Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Add INPUT support to toshiba_acpi
On Fri, 2007-06-08 at 10:23 -0400, Dmitry Torokhov wrote:
> On 6/7/07, Richard Hughes <[EMAIL PROTECTED]> wrote:
> > On Sun, 2007-06-03 at 00:48 -0400, Dmitry Torokhov wrote:
> > >
> > >
> > > It looks like KEY_COFFE comes from 0x0c/0x19e - AL Terminal
> > > Lock/Screensaver
> > > so your interpretation is indeed correct. I guess I better add an
> > > alias to
> > > input.h
> >
> > What's the status of this patch? Good for merging?
>
> Not my call - John is the official maintainer of the driver. However
> from input layer POV it looks good.
Good, thanks.
> > Do you want me to
> > redo the current patch using input-polldev
>
> The patch I send to you earlier already does this. I am attaching a
> refreshed version that used KEY_COFFEE instead of KEY_BREAK.
Brilliant, these look great.
> > and the new setkeycode stuff?
>
> And I have the patch doing this as well (attached). If you could
> please give it a try I'd appreciate it.
You beat me to it {hastily removes item from TODO...} :-)
I'll give it a try as soon as possible, thanks.
Richard.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Add INPUT support to toshiba_acpi
On Fri, 2007-06-08 at 10:23 -0400, Dmitry Torokhov wrote:
On 6/7/07, Richard Hughes [EMAIL PROTECTED] wrote:
On Sun, 2007-06-03 at 00:48 -0400, Dmitry Torokhov wrote:
It looks like KEY_COFFE comes from 0x0c/0x19e - AL Terminal
Lock/Screensaver
so your interpretation is indeed correct. I guess I better add an
alias to
input.h
What's the status of this patch? Good for merging?
Not my call - John is the official maintainer of the driver. However
from input layer POV it looks good.
Good, thanks.
Do you want me to
redo the current patch using input-polldev
The patch I send to you earlier already does this. I am attaching a
refreshed version that used KEY_COFFEE instead of KEY_BREAK.
Brilliant, these look great.
and the new setkeycode stuff?
And I have the patch doing this as well (attached). If you could
please give it a try I'd appreciate it.
You beat me to it {hastily removes item from TODO...} :-)
I'll give it a try as soon as possible, thanks.
Richard.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Add INPUT support to toshiba_acpi
On Sun, 2007-06-03 at 00:48 -0400, Dmitry Torokhov wrote: > > > It looks like KEY_COFFE comes from 0x0c/0x19e - AL Terminal > Lock/Screensaver > so your interpretation is indeed correct. I guess I better add an > alias to > input.h What's the status of this patch? Good for merging? Do you want me to redo the current patch using input-polldev and the new setkeycode stuff? Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Add INPUT support to toshiba_acpi
On Sun, 2007-06-03 at 00:48 -0400, Dmitry Torokhov wrote: It looks like KEY_COFFE comes from 0x0c/0x19e - AL Terminal Lock/Screensaver so your interpretation is indeed correct. I guess I better add an alias to input.h What's the status of this patch? Good for merging? Do you want me to redo the current patch using input-polldev and the new setkeycode stuff? Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Add INPUT support to toshiba_acpi
On Fri, 2007-06-01 at 12:45 -0400, Dmitry Torokhov wrote: > The patch was pretty good, I did not quite like the driver > registration code so I tried to clean it up. Cool, cheers. > What do you think about > the attached patch (not tested due to the lack of hardware). If you > are OK with it please add your signed-off-by because version of the > patch I grabbed did not have it. Attached patch looks good - I can't test it until next week, but looks logically correct to me. I would also like to add the oddball proc event notification system to the removal-schedule document, but this can be a fight for another day. > > > Also I don't think you want to use > > > KEY_BREAK. What is the expected function of that key? To lock the screen. We probably want to replace HCI_HKEY_BREAK with HCI_HKEY_LOCK, as the picture on the keys is a little padlock. > > It's a "lock" key, I really want KEY_LOCK added to input.h, but that > > might prove difficult. For now I've used KEY_CLEAR, yell if you think > > there's a better one to substitute or if you guys want me to add get a > > constant added to input.h. > > Iam still struggling with the purpose of the key. What would you > extect to happen when youser presses this key? Screen gets locked? > KEY_SCREENLOCK? KEY_SCREENSAVER? Either of these keys would be good to add. So yes, patch looks good, cheers for the improvements. Signed-off-by: Richard Hughes <[EMAIL PROTECTED]> Thanks again, Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Add INPUT support to toshiba_acpi
On Fri, 2007-06-01 at 12:45 -0400, Dmitry Torokhov wrote: The patch was pretty good, I did not quite like the driver registration code so I tried to clean it up. Cool, cheers. What do you think about the attached patch (not tested due to the lack of hardware). If you are OK with it please add your signed-off-by because version of the patch I grabbed did not have it. Attached patch looks good - I can't test it until next week, but looks logically correct to me. I would also like to add the oddball proc event notification system to the removal-schedule document, but this can be a fight for another day. Also I don't think you want to use KEY_BREAK. What is the expected function of that key? To lock the screen. We probably want to replace HCI_HKEY_BREAK with HCI_HKEY_LOCK, as the picture on the keys is a little padlock. It's a lock key, I really want KEY_LOCK added to input.h, but that might prove difficult. For now I've used KEY_CLEAR, yell if you think there's a better one to substitute or if you guys want me to add get a constant added to input.h. Iam still struggling with the purpose of the key. What would you extect to happen when youser presses this key? Screen gets locked? KEY_SCREENLOCK? KEY_SCREENSAVER? Either of these keys would be good to add. So yes, patch looks good, cheers for the improvements. Signed-off-by: Richard Hughes [EMAIL PROTECTED] Thanks again, Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] Move led attributes out of device name and into sysfs attributes, was Re: LED devices
On Fri, 2007-06-01 at 16:43 +0100, Richard Purdie wrote: > On Fri, 2007-06-01 at 16:04 +0100, Richard Hughes wrote: > > Patch attached corrects all the brokenness with the led class (encoding > > some attributes in the device handle). > > For the benefit of LKML, there has been some discussion of this > elsewhere. My arguments do not particularly come across in Richard's > choice of quoting and I'm a little annoyed he's chosen to do things this > way, particularly before any further feedback from Greg/Kay but so be > it. To be honest, I didn't want to do this, but you would not accept my argument - and you wanted to add _more_ properties into the device handle. > Greg said that the LEDs class should export any property data as a class > attribute rather than this just being available in the device name. I > added the patch in > http://git.o-hand.com/?p=linux-rpurdie-leds;a=shortlog;h=for-mm to deal > with this, without breaking the existing LED naming and documented API. Yes, but as you'll notice with my patch, lots of the users that use the led class do not use the handle:colour name, and so stripping off second : parameter for the color attribute was just wrong. And ugly. > Greg said that the only requirement on bus id naming was that it needed > to be unique so this should therefore be compatible. HAL could then go > ahead and ignore the device name, all the attributes are there in the > fashion it needs which was the original complaint. But when I was investigating adding led class support to thinkpad_acpi I couldn't use the existing LED class, as some of the LED's did not have a pre-defined colour, but did have a description. Again, adding support into HAL was made more difficult until you proposed screenscaping the led colour from the device name. This isn't very nice. > I accept this is basically out of my hands now. Greg/Kay have the > appropriate emails and if they'll let me know which approach they want > to take, I'll apply an appropriate patch. I'd suggest not applying this > patch directly as it introduces a race in the error path it alters. Sure, I really wanted to convert the class to struct device (which I'm more familiar with) but just tried to do minimal changes. What changes need to be made to avoid a race? Thanks. Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch] Move led attributes out of device name and into sysfs attributes, was Re: LED devices
On Thu, 2007-05-31 at 00:24 -0700, Greg KH wrote: > On Wed, May 30, 2007 at 05:19:31PM +0100, Richard Purdie wrote: > > On Wed, 2007-05-30 at 16:34 +0100, Richard Hughes wrote: > > > I've talked with other kernel guys here at red hat and they don't think > > > putting properties in the handle is a good idea. > > > > > > I've cc'd Kay, Greg KH and a few other developers for comments. > > > > > > To summarize, the LED device class creates a device with a ":" delimited > > > handle, where properties that are not going to changed get included in > > > the handle name for userspace to parse with sscanf. For > > > example, /sys/class/leds/thinklight:blue:keyboard_backlight/brightness > > > > > > I think this breaks the one-value-per sysfs file rule and sure makes it > > > more difficult to extract information in HAL. The backlight class should > > > have an attribute "color" and "function" so new properties can be added > > > (or ones that make no sense removed) without breaking API, but the > > > maintainer doesn't like that idea. > > > > I will put my side (as the above maintainer) across. When deciding on > > the naming for the LEDs in /sys/class/leds/ we had several choices. > > Taking my handheld as an example, some choices were: > > > > /sys/class/leds/led0/ > > /sys/class/leds/led1/ > > Yes. > > > /sys/class/leds/spitz0/ > > /sys/class/leds/spitz1/ > > Yes. > > > /sys/class/leds/spitz:amber/ > > /sys/class/leds/spitz:green/ > > No way! Don't do that. > > > The first contains no useful information, the second one is only > > fractionally better, the third is actually quite useful. Faced with the > > third name, a person can actually point to the right LED on the device. > > So? You need to read the attributes in the sysfs device directory to > find out exactly what type of device this is, what it does, and all > sorts of other information. > > The first two examples above are correct. They use a "bus id" type > naming scheme, like ALL OTHER DEVICES IN THE KERNEL. The only > requirement is that it is unique. > > Userspace programs, like HAL, can handle the fact that spitz0 is really > a gree LED and it means that the device is charging. That all comes > from the individual sysfs files. > > So please, don't imbend sysfs attributes into the bus id for the device, > that's just wrong on so many levels... > > > As far as the class is concerned, LED colour is a static property (it > > could handle multi coloured through multiple LED devices). > > Yes, and as a property, it needs to be an attribute, not the bus id. > > Do you really want to see /sys/bus/usb/devices/usb:hub23 or > /sys/bus/usb/devices/usb:nerf_rocket_launcher3 ? No, just read the > attributes to determine the type of the device, like all other devices. > > Please, if this is the way that the code is currently working, change it > now. Kay, Patch attached corrects all the brokenness with the led class (encoding some attributes in the device handle). Documentation/leds-class.txt| 13 drivers/leds/led-class.c| 62 ++-- drivers/leds/leds-ams-delta.c | 18 +++ drivers/leds/leds-corgi.c |8 +++-- drivers/leds/leds-h1940.c | 12 +-- drivers/leds/leds-locomo.c | 8 +++-- drivers/leds/leds-net48xx.c |3 + drivers/leds/leds-spitz.c |8 +++-- drivers/leds/leds-tosa.c|8 +++-- drivers/leds/leds-wrap.c|6 ++- drivers/macintosh/via-pmu-led.c |1 drivers/misc/asus-laptop.c |3 + include/linux/leds.h|2 + 13 files changed, 115 insertions(+), 37 deletions(-) Signed-off-by: Richard Hughes <[EMAIL PROTECTED]> Please review attached patch, thanks. diff --git a/Documentation/leds-class.txt b/Documentation/leds-class.txt index 8c35c04..083222b 100644 --- a/Documentation/leds-class.txt +++ b/Documentation/leds-class.txt @@ -34,19 +34,6 @@ and the aim is to keep a small amount of code giving as much functionality as possible. Please keep this in mind when suggesting enhancements. -LED Device Naming -= - -Is currently of the form: - -"devicename:colour" - -There have been calls for LED properties such as colour to be exported as -individual led class attributes. As a solution which doesn't incur as much -overhead, I suggest these become part of the device name. The naming scheme -above leaves scope for further attributes should they be needed. - - Known Issues diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c index 3
[patch] Move led attributes out of device name and into sysfs attributes, was Re: LED devices
On Thu, 2007-05-31 at 00:24 -0700, Greg KH wrote:
On Wed, May 30, 2007 at 05:19:31PM +0100, Richard Purdie wrote:
On Wed, 2007-05-30 at 16:34 +0100, Richard Hughes wrote:
I've talked with other kernel guys here at red hat and they don't think
putting properties in the handle is a good idea.
I've cc'd Kay, Greg KH and a few other developers for comments.
To summarize, the LED device class creates a device with a : delimited
handle, where properties that are not going to changed get included in
the handle name for userspace to parse with sscanf. For
example, /sys/class/leds/thinklight:blue:keyboard_backlight/brightness
I think this breaks the one-value-per sysfs file rule and sure makes it
more difficult to extract information in HAL. The backlight class should
have an attribute color and function so new properties can be added
(or ones that make no sense removed) without breaking API, but the
maintainer doesn't like that idea.
I will put my side (as the above maintainer) across. When deciding on
the naming for the LEDs in /sys/class/leds/ we had several choices.
Taking my handheld as an example, some choices were:
/sys/class/leds/led0/
/sys/class/leds/led1/
Yes.
/sys/class/leds/spitz0/
/sys/class/leds/spitz1/
Yes.
/sys/class/leds/spitz:amber/
/sys/class/leds/spitz:green/
No way! Don't do that.
The first contains no useful information, the second one is only
fractionally better, the third is actually quite useful. Faced with the
third name, a person can actually point to the right LED on the device.
So? You need to read the attributes in the sysfs device directory to
find out exactly what type of device this is, what it does, and all
sorts of other information.
The first two examples above are correct. They use a bus id type
naming scheme, like ALL OTHER DEVICES IN THE KERNEL. The only
requirement is that it is unique.
Userspace programs, like HAL, can handle the fact that spitz0 is really
a gree LED and it means that the device is charging. That all comes
from the individual sysfs files.
So please, don't imbend sysfs attributes into the bus id for the device,
that's just wrong on so many levels...
As far as the class is concerned, LED colour is a static property (it
could handle multi coloured through multiple LED devices).
Yes, and as a property, it needs to be an attribute, not the bus id.
Do you really want to see /sys/bus/usb/devices/usb:hub23 or
/sys/bus/usb/devices/usb:nerf_rocket_launcher3 ? No, just read the
attributes to determine the type of the device, like all other devices.
Please, if this is the way that the code is currently working, change it
now.
Kay,
Patch attached corrects all the brokenness with the led class (encoding
some attributes in the device handle).
Documentation/leds-class.txt| 13
drivers/leds/led-class.c| 62 ++--
drivers/leds/leds-ams-delta.c | 18 +++
drivers/leds/leds-corgi.c |8 +++--
drivers/leds/leds-h1940.c | 12 +--
drivers/leds/leds-locomo.c |8 +++--
drivers/leds/leds-net48xx.c |3 +
drivers/leds/leds-spitz.c |8 +++--
drivers/leds/leds-tosa.c|8 +++--
drivers/leds/leds-wrap.c|6 ++-
drivers/macintosh/via-pmu-led.c |1
drivers/misc/asus-laptop.c |3 +
include/linux/leds.h|2 +
13 files changed, 115 insertions(+), 37 deletions(-)
Signed-off-by: Richard Hughes [EMAIL PROTECTED]
Please review attached patch, thanks.
diff --git a/Documentation/leds-class.txt b/Documentation/leds-class.txt
index 8c35c04..083222b 100644
--- a/Documentation/leds-class.txt
+++ b/Documentation/leds-class.txt
@@ -34,19 +34,6 @@ and the aim is to keep a small amount of code giving as much functionality
as possible. Please keep this in mind when suggesting enhancements.
-LED Device Naming
-=
-
-Is currently of the form:
-
-devicename:colour
-
-There have been calls for LED properties such as colour to be exported as
-individual led class attributes. As a solution which doesn't incur as much
-overhead, I suggest these become part of the device name. The naming scheme
-above leaves scope for further attributes should they be needed.
-
-
Known Issues
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 3c17112..42b7355 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -36,6 +36,30 @@ static ssize_t led_brightness_show(struct class_device *dev, char *buf)
return ret;
}
+static ssize_t led_colour_show(struct class_device *dev, char *buf)
+{
+ struct led_classdev *led_cdev = class_get_devdata(dev);
+ ssize_t ret = 0;
+
+ /* no lock needed for this */
+ sprintf(buf, %s\n, led_cdev-colour);
+ ret = strlen(buf) + 1;
+
+ return ret;
+}
+
+static ssize_t led_description_show(struct class_device *dev, char
Re: [patch] Move led attributes out of device name and into sysfs attributes, was Re: LED devices
On Fri, 2007-06-01 at 16:43 +0100, Richard Purdie wrote: On Fri, 2007-06-01 at 16:04 +0100, Richard Hughes wrote: Patch attached corrects all the brokenness with the led class (encoding some attributes in the device handle). For the benefit of LKML, there has been some discussion of this elsewhere. My arguments do not particularly come across in Richard's choice of quoting and I'm a little annoyed he's chosen to do things this way, particularly before any further feedback from Greg/Kay but so be it. To be honest, I didn't want to do this, but you would not accept my argument - and you wanted to add _more_ properties into the device handle. Greg said that the LEDs class should export any property data as a class attribute rather than this just being available in the device name. I added the patch in http://git.o-hand.com/?p=linux-rpurdie-leds;a=shortlog;h=for-mm to deal with this, without breaking the existing LED naming and documented API. Yes, but as you'll notice with my patch, lots of the users that use the led class do not use the handle:colour name, and so stripping off second : parameter for the color attribute was just wrong. And ugly. Greg said that the only requirement on bus id naming was that it needed to be unique so this should therefore be compatible. HAL could then go ahead and ignore the device name, all the attributes are there in the fashion it needs which was the original complaint. But when I was investigating adding led class support to thinkpad_acpi I couldn't use the existing LED class, as some of the LED's did not have a pre-defined colour, but did have a description. Again, adding support into HAL was made more difficult until you proposed screenscaping the led colour from the device name. This isn't very nice. I accept this is basically out of my hands now. Greg/Kay have the appropriate emails and if they'll let me know which approach they want to take, I'll apply an appropriate patch. I'd suggest not applying this patch directly as it introduces a race in the error path it alters. Sure, I really wanted to convert the class to struct device (which I'm more familiar with) but just tried to do minimal changes. What changes need to be made to avoid a race? Thanks. Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Add INPUT support to toshiba_acpi
On Thu, 2007-05-31 at 18:46 +0200, Andreas Mohr wrote:
> HCI_EMPTY is *by far* the most frequent state to occur I think
> (users won't press keys all the time), thus it's probably better(?)
> for branch prediction to have this placed first, right?
> Not that it matters too much instruction-wise, but still...
Sure.
> Apart from that I'm very happy to see progress on this front
> (speaking as a "proud" owner of an old Toshiba notebook requiring
> this stuff).
Good - this stuff should just work :-)
> And I'd definitely move the multiple identical "Re-enabled hotkeys" parts
> into one single non-inlined(!) function for the same reason.
> Not to mention that it's BUTT UGLY to have the *same* fat
> multi-line comment duplicated bazillion times.
Agree. New patch (untested) attached.
Thanks for review,
Richard.
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 139f41f..38835b6 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -222,6 +222,7 @@ config ACPI_TOSHIBA
tristate "Toshiba Laptop Extras"
depends on X86
select BACKLIGHT_CLASS_DEVICE
+ select INPUT_POLLDEV
---help---
This driver adds support for access to certain system settings
on "legacy free" Toshiba laptops. These laptops can be recognized by
diff --git a/drivers/acpi/toshiba_acpi.c b/drivers/acpi/toshiba_acpi.c
index 3906d47..f802609 100644
--- a/drivers/acpi/toshiba_acpi.c
+++ b/drivers/acpi/toshiba_acpi.c
@@ -27,13 +27,13 @@
* engineering the Windows drivers
* Yasushi Nagato - changes for linux kernel 2.4 -> 2.5
* Rob Miller - TV out and hotkeys help
- *
+ * Richard Hughes - emit INPUT events for hotkeys
*
* TODO
*
*/
-#define TOSHIBA_ACPI_VERSION "0.18"
+#define TOSHIBA_ACPI_VERSION "0.19"
#define PROC_INTERFACE_VERSION 1
#include
@@ -42,6 +42,7 @@
#include
#include
#include
+#include
#include
@@ -99,6 +100,16 @@ MODULE_LICENSE("GPL");
#define HCI_VIDEO_OUT_CRT 0x2
#define HCI_VIDEO_OUT_TV 0x4
+/* key definitions */
+#define HCI_HKEY_MUTE 0x0101
+#define HCI_HKEY_LOCK 0x013b
+#define HCI_HKEY_SEARCH 0x013c
+#define HCI_HKEY_SUSPEND 0x013d
+#define HCI_HKEY_HIBERNATE 0x013e
+#define HCI_HKEY_BRIGHTNESSDOWN 0x0140
+#define HCI_HKEY_BRIGHTNESSUP 0x0141
+#define HCI_HKEY_WLAN 0x0142
+
/* utility
*/
@@ -213,9 +224,15 @@ static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result)
static struct proc_dir_entry *toshiba_proc_dir /*= 0*/ ;
static struct backlight_device *toshiba_backlight_device;
+static struct input_polled_dev *toshiba_poll_dev;
static int force_fan;
static int last_key_event;
static int key_event_valid;
+static int hotkeys_over_input = 1;
+static int hotkeys_check_per_sec = 2;
+
+module_param(hotkeys_over_input, uint, 0400);
+module_param(hotkeys_check_per_sec, uint, 0400);
typedef struct _ProcItem {
const char *name;
@@ -438,34 +455,60 @@ static unsigned long write_fan(const char *buffer, unsigned long count)
return count;
}
-static char *read_keys(char *p)
+/* returns zero if hci event is invalid */
+static u32 get_hci_system_event_value(void)
{
u32 hci_result;
u32 value;
+ u32 retval = 0;
+
+ /* read a system event from the queue */
+ hci_read1(HCI_SYSTEM_EVENT, , _result);
+
+ switch (hci_result) {
+ case HCI_EMPTY:
+ /* better luck next time */
+ break;
+ case HCI_SUCCESS:
+ /* value is the data in the queue */
+ retval = value;
+ break;
+ case HCI_NOT_SUPPORTED:
+ /* This is a workaround for an unresolved issue on
+ * some machines where system events sporadically
+ * become disabled. */
+ hci_write1(HCI_SYSTEM_EVENT, 1, _result);
+ printk(MY_NOTICE "Re-enabled hotkeys\n");
+ break;
+ default:
+ /* there was an unknown error */
+ printk(MY_ERR "Error reading hotkey status\n");
+ }
+ return retval;
+}
- if (!key_event_valid) {
- hci_read1(HCI_SYSTEM_EVENT, , _result);
- if (hci_result == HCI_SUCCESS) {
- key_event_valid = 1;
- last_key_event = value;
- } else if (hci_result == HCI_EMPTY) {
- /* better luck next time */
- } else if (hci_result == HCI_NOT_SUPPORTED) {
- /* This is a workaround for an unresolved issue on
- * some machines where system events sporadically
- * become disabled. */
- hci_write1(HCI_SYSTEM_EVENT, 1, _result);
- printk(MY_NOTICE "Re-enabled hotkeys\n");
- } else {
- printk(MY_ERR "Error reading hotkey status\n");
- goto end;
+/* deprecate this interface... */
+static char *read_keys(char *p)
+{
+ u32 value;
+
+ if (hotkeys_over_input) {
+ key_event_valid = 0;
+ last_key_event = 0;
+ } else {
+ if (!key_event_valid) {
+ value = get_hci_system_event_value();
+ if (value != 0) {
+key_event_valid = 1;
+last_key_event = value;
+ }
}
}
p += sprintf(p, "hotkey_ready:%d\n", key_event_valid);
p += sprintf(p, "hotkey: 0x%04x\n&qu
Re: Add INPUT support to toshiba_acpi
On Thu, 2007-05-31 at 14:44 +0100, Richard Hughes wrote:
> Nope, impossible AFAICS. The hardware is just broken. Windows XP has an
> toshiba supplied daemon that polls, so I think we have to just bite the
> bullet.
... adding in reply in different thread...
On Thu, 2007-05-31 at 10:37 -0400, Dmitry Torokhov wrote:
> Please use input-polldev to set up polled devices. It
> will create work queue for you and will make sure that polling is
> stopped when device is closed.
Okay, I had never heard of this. I've done the suggested changes in the
attached patch. Please can you have a look and make sure I'm being sane.
> Also I don't think you want to use
> KEY_BREAK. What is the expected function of that key?
It's a "lock" key, I really want KEY_LOCK added to input.h, but that
might prove difficult. For now I've used KEY_CLEAR, yell if you think
there's a better one to substitute or if you guys want me to add get a
constant added to input.h.
Thanks for the review.
Richard.
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 139f41f..38835b6 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -222,6 +222,7 @@ config ACPI_TOSHIBA
tristate "Toshiba Laptop Extras"
depends on X86
select BACKLIGHT_CLASS_DEVICE
+ select INPUT_POLLDEV
---help---
This driver adds support for access to certain system settings
on "legacy free" Toshiba laptops. These laptops can be recognized by
diff --git a/drivers/acpi/toshiba_acpi.c b/drivers/acpi/toshiba_acpi.c
index 3906d47..2b1a3d9 100644
--- a/drivers/acpi/toshiba_acpi.c
+++ b/drivers/acpi/toshiba_acpi.c
@@ -27,13 +27,13 @@
* engineering the Windows drivers
* Yasushi Nagato - changes for linux kernel 2.4 -> 2.5
* Rob Miller - TV out and hotkeys help
- *
+ * Richard Hughes - emit INPUT events for hotkeys
*
* TODO
*
*/
-#define TOSHIBA_ACPI_VERSION "0.18"
+#define TOSHIBA_ACPI_VERSION "0.19"
#define PROC_INTERFACE_VERSION 1
#include
@@ -42,6 +42,7 @@
#include
#include
#include
+#include
#include
@@ -99,6 +100,16 @@ MODULE_LICENSE("GPL");
#define HCI_VIDEO_OUT_CRT 0x2
#define HCI_VIDEO_OUT_TV 0x4
+/* key definitions */
+#define HCI_HKEY_MUTE 0x0101
+#define HCI_HKEY_BREAK 0x013b
+#define HCI_HKEY_SEARCH 0x013c
+#define HCI_HKEY_SUSPEND 0x013d
+#define HCI_HKEY_HIBERNATE 0x013e
+#define HCI_HKEY_BRIGHTNESSDOWN 0x0140
+#define HCI_HKEY_BRIGHTNESSUP 0x0141
+#define HCI_HKEY_WLAN 0x0142
+
/* utility
*/
@@ -213,9 +224,15 @@ static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result)
static struct proc_dir_entry *toshiba_proc_dir /*= 0*/ ;
static struct backlight_device *toshiba_backlight_device;
+static struct input_polled_dev *toshiba_poll_dev;
static int force_fan;
static int last_key_event;
static int key_event_valid;
+static int hotkeys_over_input = 1;
+static int hotkeys_check_per_sec = 2;
+
+module_param(hotkeys_over_input, uint, 0400);
+module_param(hotkeys_check_per_sec, uint, 0400);
typedef struct _ProcItem {
const char *name;
@@ -443,27 +460,33 @@ static char *read_keys(char *p)
u32 hci_result;
u32 value;
- if (!key_event_valid) {
- hci_read1(HCI_SYSTEM_EVENT, , _result);
- if (hci_result == HCI_SUCCESS) {
- key_event_valid = 1;
- last_key_event = value;
- } else if (hci_result == HCI_EMPTY) {
- /* better luck next time */
- } else if (hci_result == HCI_NOT_SUPPORTED) {
- /* This is a workaround for an unresolved issue on
- * some machines where system events sporadically
- * become disabled. */
- hci_write1(HCI_SYSTEM_EVENT, 1, _result);
- printk(MY_NOTICE "Re-enabled hotkeys\n");
- } else {
- printk(MY_ERR "Error reading hotkey status\n");
- goto end;
+ if (!hotkeys_over_input) {
+ if (!key_event_valid) {
+ hci_read1(HCI_SYSTEM_EVENT, , _result);
+ if (hci_result == HCI_SUCCESS) {
+key_event_valid = 1;
+last_key_event = value;
+ } else if (hci_result == HCI_EMPTY) {
+/* better luck next time */
+ } else if (hci_result == HCI_NOT_SUPPORTED) {
+/* This is a workaround for an unresolved issue on
+ * some machines where system events sporadically
+ * become disabled. */
+hci_write1(HCI_SYSTEM_EVENT, 1, _result);
+printk(MY_NOTICE "Re-enabled hotkeys\n");
+ } else {
+printk(MY_ERR "Error reading hotkey status\n");
+goto end;
+ }
}
+ } else {
+ key_event_valid = 0;
+ last_key_event = 0;
}
p += sprintf(p, "hotkey_ready:%d\n", key_event_valid);
p += sprintf(p, "hotkey: 0x%04x\n", last_key_event);
+ p += sprintf(p, "hotkeys_via_input: %d\n", hotkeys_over_input);
end:
return p;
@@ -534,15 +557,127 @@ static acpi_status __exit remove_device(void)
}
static struct backlight_ops toshiba_backlight_data = {
-.get_brightness = get_lcd,
-.update_status =
Re: Add INPUT support to toshiba_acpi
On Thu, 2007-05-31 at 13:53 +0100, Bastien Nocera wrote:
> On Thu, 2007-05-31 at 13:36 +0100, Richard Hughes wrote:
> > Attached patch adds a kernel thread to do polling on Toshiba hardware.
> >
> > Toshiba hardware is a little oddball, and does not provide ACPI events
> > on some key presses, typically Fn hotkey buttons. The key interface is
> > now polled, and events now matched to a list of toshiba specific
> > scancodes, and are squirted to userspace using the INPUT subsystem.
> >
> > This means that toshiba laptops buttons "just work" without any
> > userspace daemon (using uinput) such as fnfx or bodges such as using a
> > userspace hal addon. Doing the polling in kernel is more efficient, and
> > makes stuff just work out of the box. You can assign the keys using
> > standard X keymaps, or using tools such as gnome-keybinding-properties.
> >
> > This is similar to other patches sent for the thinkpad_acpi driver, and
> > is part of the "Unf*ck my keyboard" initiative to make multimedia keys
> > just work.
> >
> > Changes from the first patch involve switching to a workqueue for the
> > polling, not breaking the spaces in "hotkeys_via_input" and also masking
> > out the fn key button up.
>
> A couple of things:
> - use a switch statement in toshiba_deliver_button_event(), would look a
> bit cleaner.
Agree, done.
> - make the magic number #defines
Agree, done.
> - not sure if it's possible, but you should disable the workqueue
> altogether if nothing has the input device opened.
Do we get an event when the [input]->users value changes? If not, we
could do 1Hz (users > 0) checking when the input device is unclaimed,
and then switch to 4Hz polling when the input device is open.
> Hopefully we'll find a way to receive an interrupt, or some kind of
> event when the keys are pressed in the future, and completely avoid the
> polling. In the meantime, it should be minimised.
Nope, impossible AFAICS. The hardware is just broken. Windows XP has an
toshiba supplied daemon that polls, so I think we have to just bite the
bullet.
New patch attached.
Richard.
diff --git a/drivers/acpi/toshiba_acpi.c b/drivers/acpi/toshiba_acpi.c
index 3906d47..cf4ab76 100644
--- a/drivers/acpi/toshiba_acpi.c
+++ b/drivers/acpi/toshiba_acpi.c
@@ -27,13 +27,13 @@
* engineering the Windows drivers
* Yasushi Nagato - changes for linux kernel 2.4 -> 2.5
* Rob Miller - TV out and hotkeys help
- *
+ * Richard Hughes - emit INPUT events, based on a patch from Daniel Silverstone
*
* TODO
*
*/
-#define TOSHIBA_ACPI_VERSION "0.18"
+#define TOSHIBA_ACPI_VERSION "0.19"
#define PROC_INTERFACE_VERSION 1
#include
@@ -42,6 +42,8 @@
#include
#include
#include
+#include
+#include
#include
@@ -99,6 +101,16 @@ MODULE_LICENSE("GPL");
#define HCI_VIDEO_OUT_CRT 0x2
#define HCI_VIDEO_OUT_TV 0x4
+/* key definitions */
+#define HCI_HKEY_MUTE 0x0101
+#define HCI_HKEY_BREAK 0x013b
+#define HCI_HKEY_SEARCH 0x013c
+#define HCI_HKEY_SUSPEND 0x013d
+#define HCI_HKEY_HIBERNATE 0x013e
+#define HCI_HKEY_BRIGHTNESSDOWN 0x0140
+#define HCI_HKEY_BRIGHTNESSUP 0x0141
+#define HCI_HKEY_WLAN 0x0142
+
/* utility
*/
@@ -213,9 +225,15 @@ static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result)
static struct proc_dir_entry *toshiba_proc_dir /*= 0*/ ;
static struct backlight_device *toshiba_backlight_device;
+static struct input_dev *toshiba_input;
static int force_fan;
static int last_key_event;
static int key_event_valid;
+static int hotkeys_over_input = 1;
+static int hotkeys_check_per_sec = 2;
+
+module_param(hotkeys_over_input, uint, 0400);
+module_param(hotkeys_check_per_sec, uint, 0400);
typedef struct _ProcItem {
const char *name;
@@ -443,27 +461,33 @@ static char *read_keys(char *p)
u32 hci_result;
u32 value;
- if (!key_event_valid) {
- hci_read1(HCI_SYSTEM_EVENT, , _result);
- if (hci_result == HCI_SUCCESS) {
- key_event_valid = 1;
- last_key_event = value;
- } else if (hci_result == HCI_EMPTY) {
- /* better luck next time */
- } else if (hci_result == HCI_NOT_SUPPORTED) {
- /* This is a workaround for an unresolved issue on
- * some machines where system events sporadically
- * become disabled. */
- hci_write1(HCI_SYSTEM_EVENT, 1, _result);
- printk(MY_NOTICE "Re-enabled hotkeys\n");
- } else {
- printk(MY_ERR "Error reading hotkey status\n");
- goto end;
+ if (!hotkeys_over_input) {
+ if (!key_event_valid) {
+ hci_read1(HCI_SYSTEM_EVENT, , _result);
+ if (hci_result == HCI_SUCCESS) {
+key_event_valid = 1;
+last_key_event = value;
+ } else if (hci_result == HCI_EMPTY) {
+/* better luck next time */
+ } else if (hci_result == HCI_NOT_SUPPORTED) {
+/* This is a workaround for an u
Add INPUT support to toshiba_acpi
Attached patch adds a kernel thread to do polling on Toshiba hardware.
Toshiba hardware is a little oddball, and does not provide ACPI events
on some key presses, typically Fn hotkey buttons. The key interface is
now polled, and events now matched to a list of toshiba specific
scancodes, and are squirted to userspace using the INPUT subsystem.
This means that toshiba laptops buttons "just work" without any
userspace daemon (using uinput) such as fnfx or bodges such as using a
userspace hal addon. Doing the polling in kernel is more efficient, and
makes stuff just work out of the box. You can assign the keys using
standard X keymaps, or using tools such as gnome-keybinding-properties.
This is similar to other patches sent for the thinkpad_acpi driver, and
is part of the "Unf*ck my keyboard" initiative to make multimedia keys
just work.
Changes from the first patch involve switching to a workqueue for the
polling, not breaking the spaces in "hotkeys_via_input" and also masking
out the fn key button up.
toshiba_acpi.c | 228 +++--
1 file changed, 206 insertions(+), 22 deletions(-)
Signed-off-by: Richard Hughes <[EMAIL PROTECTED]>
--- origin/toshiba_acpi.c 2007-05-18 09:56:03.0 +0100
+++ toshiba_acpi.c 2007-05-31 13:24:02.0 +0100
@@ -27,13 +27,13 @@
* engineering the Windows drivers
* Yasushi Nagato - changes for linux kernel 2.4 -> 2.5
* Rob Miller - TV out and hotkeys help
- *
+ * Richard Hughes - emit INPUT events, based on a patch from Daniel Silverstone
*
* TODO
*
*/
-#define TOSHIBA_ACPI_VERSION "0.18"
+#define TOSHIBA_ACPI_VERSION "0.19"
#define PROC_INTERFACE_VERSION 1
#include
@@ -42,6 +42,7 @@
#include
#include
#include
+#include
#include
@@ -213,9 +214,15 @@ static acpi_status hci_read1(u32 reg, u3
static struct proc_dir_entry *toshiba_proc_dir /*= 0*/ ;
static struct backlight_device *toshiba_backlight_device;
+static struct input_dev *toshiba_input;
static int force_fan;
static int last_key_event;
static int key_event_valid;
+static int hotkeys_over_input = 1;
+static int hotkeys_check_per_sec = 2;
+
+module_param(hotkeys_over_input, uint, 0400);
+module_param(hotkeys_check_per_sec, uint, 0400);
typedef struct _ProcItem {
const char *name;
@@ -443,27 +450,33 @@ static char *read_keys(char *p)
u32 hci_result;
u32 value;
- if (!key_event_valid) {
- hci_read1(HCI_SYSTEM_EVENT, , _result);
- if (hci_result == HCI_SUCCESS) {
- key_event_valid = 1;
- last_key_event = value;
- } else if (hci_result == HCI_EMPTY) {
- /* better luck next time */
- } else if (hci_result == HCI_NOT_SUPPORTED) {
- /* This is a workaround for an unresolved issue on
- * some machines where system events sporadically
- * become disabled. */
- hci_write1(HCI_SYSTEM_EVENT, 1, _result);
- printk(MY_NOTICE "Re-enabled hotkeys\n");
- } else {
- printk(MY_ERR "Error reading hotkey status\n");
- goto end;
+ if (!hotkeys_over_input) {
+ if (!key_event_valid) {
+ hci_read1(HCI_SYSTEM_EVENT, , _result);
+ if (hci_result == HCI_SUCCESS) {
+key_event_valid = 1;
+last_key_event = value;
+ } else if (hci_result == HCI_EMPTY) {
+/* better luck next time */
+ } else if (hci_result == HCI_NOT_SUPPORTED) {
+/* This is a workaround for an unresolved issue on
+ * some machines where system events sporadically
+ * become disabled. */
+hci_write1(HCI_SYSTEM_EVENT, 1, _result);
+printk(MY_NOTICE "Re-enabled hotkeys\n");
+ } else {
+printk(MY_ERR "Error reading hotkey status\n");
+goto end;
+ }
}
+ } else {
+ key_event_valid = 0;
+ last_key_event = 0;
}
p += sprintf(p, "hotkey_ready:%d\n", key_event_valid);
p += sprintf(p, "hotkey: 0x%04x\n", last_key_event);
+ p += sprintf(p, "hotkeys_via_input: %d\n", hotkeys_over_input);
end:
return p;
@@ -534,15 +547,121 @@ static acpi_status __exit remove_device(
}
static struct backlight_ops toshiba_backlight_data = {
-.get_brightness = get_lcd,
-.update_status = set_lcd_status,
+ .get_brightness = get_lcd,
+ .update_status = set_lcd_status,
};
+static void toshiba_keys_fire(struct work_struct *work);
+static struct workqueue_struct *toshiba_keys_wq;
+static DECLARE_DELAYED_WORK(toshiba_keys_work, toshiba_keys_fire);
+
+static void toshiba_deliver_button_event(u32 value)
+{
+ int keycode = KEY_UNKNOWN;
+ int key_down = 1;
+
+ if (!toshiba_input)
+ return;
+
+ /* translate MSB to key up */
+ if (value & 0x80) {
+ value &= ~0x80;
+ key_down = 0;
+ }
+
+ /* ignore FN on its own */
+ if (value == 0x0100)
+ return;
+
+ /* translate keys to keycodes */
+ if (value == 0x101)
+ keycode = KEY_MUTE;
+ else if (value == 0x13b)
+ keycode = KEY_BREAK;
+ else if (value == 0x13c)
+ keycode = KEY_
Add INPUT support to toshiba_acpi
Attached patch adds a kernel thread to do polling on Toshiba hardware.
Toshiba hardware is a little oddball, and does not provide ACPI events
on some key presses, typically Fn hotkey buttons. The key interface is
now polled, and events now matched to a list of toshiba specific
scancodes, and are squirted to userspace using the INPUT subsystem.
This means that toshiba laptops buttons just work without any
userspace daemon (using uinput) such as fnfx or bodges such as using a
userspace hal addon. Doing the polling in kernel is more efficient, and
makes stuff just work out of the box. You can assign the keys using
standard X keymaps, or using tools such as gnome-keybinding-properties.
This is similar to other patches sent for the thinkpad_acpi driver, and
is part of the Unf*ck my keyboard initiative to make multimedia keys
just work.
Changes from the first patch involve switching to a workqueue for the
polling, not breaking the spaces in hotkeys_via_input and also masking
out the fn key button up.
toshiba_acpi.c | 228 +++--
1 file changed, 206 insertions(+), 22 deletions(-)
Signed-off-by: Richard Hughes [EMAIL PROTECTED]
--- origin/toshiba_acpi.c 2007-05-18 09:56:03.0 +0100
+++ toshiba_acpi.c 2007-05-31 13:24:02.0 +0100
@@ -27,13 +27,13 @@
* engineering the Windows drivers
* Yasushi Nagato - changes for linux kernel 2.4 - 2.5
* Rob Miller - TV out and hotkeys help
- *
+ * Richard Hughes - emit INPUT events, based on a patch from Daniel Silverstone
*
* TODO
*
*/
-#define TOSHIBA_ACPI_VERSION 0.18
+#define TOSHIBA_ACPI_VERSION 0.19
#define PROC_INTERFACE_VERSION 1
#include linux/kernel.h
@@ -42,6 +42,7 @@
#include linux/types.h
#include linux/proc_fs.h
#include linux/backlight.h
+#include linux/input.h
#include asm/uaccess.h
@@ -213,9 +214,15 @@ static acpi_status hci_read1(u32 reg, u3
static struct proc_dir_entry *toshiba_proc_dir /*= 0*/ ;
static struct backlight_device *toshiba_backlight_device;
+static struct input_dev *toshiba_input;
static int force_fan;
static int last_key_event;
static int key_event_valid;
+static int hotkeys_over_input = 1;
+static int hotkeys_check_per_sec = 2;
+
+module_param(hotkeys_over_input, uint, 0400);
+module_param(hotkeys_check_per_sec, uint, 0400);
typedef struct _ProcItem {
const char *name;
@@ -443,27 +450,33 @@ static char *read_keys(char *p)
u32 hci_result;
u32 value;
- if (!key_event_valid) {
- hci_read1(HCI_SYSTEM_EVENT, value, hci_result);
- if (hci_result == HCI_SUCCESS) {
- key_event_valid = 1;
- last_key_event = value;
- } else if (hci_result == HCI_EMPTY) {
- /* better luck next time */
- } else if (hci_result == HCI_NOT_SUPPORTED) {
- /* This is a workaround for an unresolved issue on
- * some machines where system events sporadically
- * become disabled. */
- hci_write1(HCI_SYSTEM_EVENT, 1, hci_result);
- printk(MY_NOTICE Re-enabled hotkeys\n);
- } else {
- printk(MY_ERR Error reading hotkey status\n);
- goto end;
+ if (!hotkeys_over_input) {
+ if (!key_event_valid) {
+ hci_read1(HCI_SYSTEM_EVENT, value, hci_result);
+ if (hci_result == HCI_SUCCESS) {
+key_event_valid = 1;
+last_key_event = value;
+ } else if (hci_result == HCI_EMPTY) {
+/* better luck next time */
+ } else if (hci_result == HCI_NOT_SUPPORTED) {
+/* This is a workaround for an unresolved issue on
+ * some machines where system events sporadically
+ * become disabled. */
+hci_write1(HCI_SYSTEM_EVENT, 1, hci_result);
+printk(MY_NOTICE Re-enabled hotkeys\n);
+ } else {
+printk(MY_ERR Error reading hotkey status\n);
+goto end;
+ }
}
+ } else {
+ key_event_valid = 0;
+ last_key_event = 0;
}
p += sprintf(p, hotkey_ready:%d\n, key_event_valid);
p += sprintf(p, hotkey: 0x%04x\n, last_key_event);
+ p += sprintf(p, hotkeys_via_input: %d\n, hotkeys_over_input);
end:
return p;
@@ -534,15 +547,121 @@ static acpi_status __exit remove_device(
}
static struct backlight_ops toshiba_backlight_data = {
-.get_brightness = get_lcd,
-.update_status = set_lcd_status,
+ .get_brightness = get_lcd,
+ .update_status = set_lcd_status,
};
+static void toshiba_keys_fire(struct work_struct *work);
+static struct workqueue_struct *toshiba_keys_wq;
+static DECLARE_DELAYED_WORK(toshiba_keys_work, toshiba_keys_fire);
+
+static void toshiba_deliver_button_event(u32 value)
+{
+ int keycode = KEY_UNKNOWN;
+ int key_down = 1;
+
+ if (!toshiba_input)
+ return;
+
+ /* translate MSB to key up */
+ if (value 0x80) {
+ value = ~0x80;
+ key_down = 0;
+ }
+
+ /* ignore FN on its own */
+ if (value == 0x0100)
+ return;
+
+ /* translate keys to keycodes */
+ if (value == 0x101)
+ keycode = KEY_MUTE;
+ else if (value == 0x13b)
+ keycode = KEY_BREAK;
+ else if (value == 0x13c)
+ keycode = KEY_SEARCH;
+ else if (value == 0x13d)
+ keycode = KEY_SLEEP
Re: Add INPUT support to toshiba_acpi
On Thu, 2007-05-31 at 13:53 +0100, Bastien Nocera wrote:
On Thu, 2007-05-31 at 13:36 +0100, Richard Hughes wrote:
Attached patch adds a kernel thread to do polling on Toshiba hardware.
Toshiba hardware is a little oddball, and does not provide ACPI events
on some key presses, typically Fn hotkey buttons. The key interface is
now polled, and events now matched to a list of toshiba specific
scancodes, and are squirted to userspace using the INPUT subsystem.
This means that toshiba laptops buttons just work without any
userspace daemon (using uinput) such as fnfx or bodges such as using a
userspace hal addon. Doing the polling in kernel is more efficient, and
makes stuff just work out of the box. You can assign the keys using
standard X keymaps, or using tools such as gnome-keybinding-properties.
This is similar to other patches sent for the thinkpad_acpi driver, and
is part of the Unf*ck my keyboard initiative to make multimedia keys
just work.
Changes from the first patch involve switching to a workqueue for the
polling, not breaking the spaces in hotkeys_via_input and also masking
out the fn key button up.
A couple of things:
- use a switch statement in toshiba_deliver_button_event(), would look a
bit cleaner.
Agree, done.
- make the magic number #defines
Agree, done.
- not sure if it's possible, but you should disable the workqueue
altogether if nothing has the input device opened.
Do we get an event when the [input]-users value changes? If not, we
could do 1Hz (users 0) checking when the input device is unclaimed,
and then switch to 4Hz polling when the input device is open.
Hopefully we'll find a way to receive an interrupt, or some kind of
event when the keys are pressed in the future, and completely avoid the
polling. In the meantime, it should be minimised.
Nope, impossible AFAICS. The hardware is just broken. Windows XP has an
toshiba supplied daemon that polls, so I think we have to just bite the
bullet.
New patch attached.
Richard.
diff --git a/drivers/acpi/toshiba_acpi.c b/drivers/acpi/toshiba_acpi.c
index 3906d47..cf4ab76 100644
--- a/drivers/acpi/toshiba_acpi.c
+++ b/drivers/acpi/toshiba_acpi.c
@@ -27,13 +27,13 @@
* engineering the Windows drivers
* Yasushi Nagato - changes for linux kernel 2.4 - 2.5
* Rob Miller - TV out and hotkeys help
- *
+ * Richard Hughes - emit INPUT events, based on a patch from Daniel Silverstone
*
* TODO
*
*/
-#define TOSHIBA_ACPI_VERSION 0.18
+#define TOSHIBA_ACPI_VERSION 0.19
#define PROC_INTERFACE_VERSION 1
#include linux/kernel.h
@@ -42,6 +42,8 @@
#include linux/types.h
#include linux/proc_fs.h
#include linux/backlight.h
+#include linux/input.h
+#include linux/workqueue.h
#include asm/uaccess.h
@@ -99,6 +101,16 @@ MODULE_LICENSE(GPL);
#define HCI_VIDEO_OUT_CRT 0x2
#define HCI_VIDEO_OUT_TV 0x4
+/* key definitions */
+#define HCI_HKEY_MUTE 0x0101
+#define HCI_HKEY_BREAK 0x013b
+#define HCI_HKEY_SEARCH 0x013c
+#define HCI_HKEY_SUSPEND 0x013d
+#define HCI_HKEY_HIBERNATE 0x013e
+#define HCI_HKEY_BRIGHTNESSDOWN 0x0140
+#define HCI_HKEY_BRIGHTNESSUP 0x0141
+#define HCI_HKEY_WLAN 0x0142
+
/* utility
*/
@@ -213,9 +225,15 @@ static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result)
static struct proc_dir_entry *toshiba_proc_dir /*= 0*/ ;
static struct backlight_device *toshiba_backlight_device;
+static struct input_dev *toshiba_input;
static int force_fan;
static int last_key_event;
static int key_event_valid;
+static int hotkeys_over_input = 1;
+static int hotkeys_check_per_sec = 2;
+
+module_param(hotkeys_over_input, uint, 0400);
+module_param(hotkeys_check_per_sec, uint, 0400);
typedef struct _ProcItem {
const char *name;
@@ -443,27 +461,33 @@ static char *read_keys(char *p)
u32 hci_result;
u32 value;
- if (!key_event_valid) {
- hci_read1(HCI_SYSTEM_EVENT, value, hci_result);
- if (hci_result == HCI_SUCCESS) {
- key_event_valid = 1;
- last_key_event = value;
- } else if (hci_result == HCI_EMPTY) {
- /* better luck next time */
- } else if (hci_result == HCI_NOT_SUPPORTED) {
- /* This is a workaround for an unresolved issue on
- * some machines where system events sporadically
- * become disabled. */
- hci_write1(HCI_SYSTEM_EVENT, 1, hci_result);
- printk(MY_NOTICE Re-enabled hotkeys\n);
- } else {
- printk(MY_ERR Error reading hotkey status\n);
- goto end;
+ if (!hotkeys_over_input) {
+ if (!key_event_valid) {
+ hci_read1(HCI_SYSTEM_EVENT, value, hci_result);
+ if (hci_result == HCI_SUCCESS) {
+key_event_valid = 1;
+last_key_event = value;
+ } else if (hci_result == HCI_EMPTY) {
+/* better luck next time */
+ } else if (hci_result == HCI_NOT_SUPPORTED) {
+/* This is a workaround for an unresolved issue on
+ * some machines where system events sporadically
+ * become disabled. */
+hci_write1(HCI_SYSTEM_EVENT, 1, hci_result);
+printk(MY_NOTICE Re-enabled hotkeys\n
Re: Add INPUT support to toshiba_acpi
On Thu, 2007-05-31 at 14:44 +0100, Richard Hughes wrote:
Nope, impossible AFAICS. The hardware is just broken. Windows XP has an
toshiba supplied daemon that polls, so I think we have to just bite the
bullet.
... adding in reply in different thread...
On Thu, 2007-05-31 at 10:37 -0400, Dmitry Torokhov wrote:
Please use input-polldev to set up polled devices. It
will create work queue for you and will make sure that polling is
stopped when device is closed.
Okay, I had never heard of this. I've done the suggested changes in the
attached patch. Please can you have a look and make sure I'm being sane.
Also I don't think you want to use
KEY_BREAK. What is the expected function of that key?
It's a lock key, I really want KEY_LOCK added to input.h, but that
might prove difficult. For now I've used KEY_CLEAR, yell if you think
there's a better one to substitute or if you guys want me to add get a
constant added to input.h.
Thanks for the review.
Richard.
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 139f41f..38835b6 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -222,6 +222,7 @@ config ACPI_TOSHIBA
tristate Toshiba Laptop Extras
depends on X86
select BACKLIGHT_CLASS_DEVICE
+ select INPUT_POLLDEV
---help---
This driver adds support for access to certain system settings
on legacy free Toshiba laptops. These laptops can be recognized by
diff --git a/drivers/acpi/toshiba_acpi.c b/drivers/acpi/toshiba_acpi.c
index 3906d47..2b1a3d9 100644
--- a/drivers/acpi/toshiba_acpi.c
+++ b/drivers/acpi/toshiba_acpi.c
@@ -27,13 +27,13 @@
* engineering the Windows drivers
* Yasushi Nagato - changes for linux kernel 2.4 - 2.5
* Rob Miller - TV out and hotkeys help
- *
+ * Richard Hughes - emit INPUT events for hotkeys
*
* TODO
*
*/
-#define TOSHIBA_ACPI_VERSION 0.18
+#define TOSHIBA_ACPI_VERSION 0.19
#define PROC_INTERFACE_VERSION 1
#include linux/kernel.h
@@ -42,6 +42,7 @@
#include linux/types.h
#include linux/proc_fs.h
#include linux/backlight.h
+#include linux/input-polldev.h
#include asm/uaccess.h
@@ -99,6 +100,16 @@ MODULE_LICENSE(GPL);
#define HCI_VIDEO_OUT_CRT 0x2
#define HCI_VIDEO_OUT_TV 0x4
+/* key definitions */
+#define HCI_HKEY_MUTE 0x0101
+#define HCI_HKEY_BREAK 0x013b
+#define HCI_HKEY_SEARCH 0x013c
+#define HCI_HKEY_SUSPEND 0x013d
+#define HCI_HKEY_HIBERNATE 0x013e
+#define HCI_HKEY_BRIGHTNESSDOWN 0x0140
+#define HCI_HKEY_BRIGHTNESSUP 0x0141
+#define HCI_HKEY_WLAN 0x0142
+
/* utility
*/
@@ -213,9 +224,15 @@ static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result)
static struct proc_dir_entry *toshiba_proc_dir /*= 0*/ ;
static struct backlight_device *toshiba_backlight_device;
+static struct input_polled_dev *toshiba_poll_dev;
static int force_fan;
static int last_key_event;
static int key_event_valid;
+static int hotkeys_over_input = 1;
+static int hotkeys_check_per_sec = 2;
+
+module_param(hotkeys_over_input, uint, 0400);
+module_param(hotkeys_check_per_sec, uint, 0400);
typedef struct _ProcItem {
const char *name;
@@ -443,27 +460,33 @@ static char *read_keys(char *p)
u32 hci_result;
u32 value;
- if (!key_event_valid) {
- hci_read1(HCI_SYSTEM_EVENT, value, hci_result);
- if (hci_result == HCI_SUCCESS) {
- key_event_valid = 1;
- last_key_event = value;
- } else if (hci_result == HCI_EMPTY) {
- /* better luck next time */
- } else if (hci_result == HCI_NOT_SUPPORTED) {
- /* This is a workaround for an unresolved issue on
- * some machines where system events sporadically
- * become disabled. */
- hci_write1(HCI_SYSTEM_EVENT, 1, hci_result);
- printk(MY_NOTICE Re-enabled hotkeys\n);
- } else {
- printk(MY_ERR Error reading hotkey status\n);
- goto end;
+ if (!hotkeys_over_input) {
+ if (!key_event_valid) {
+ hci_read1(HCI_SYSTEM_EVENT, value, hci_result);
+ if (hci_result == HCI_SUCCESS) {
+key_event_valid = 1;
+last_key_event = value;
+ } else if (hci_result == HCI_EMPTY) {
+/* better luck next time */
+ } else if (hci_result == HCI_NOT_SUPPORTED) {
+/* This is a workaround for an unresolved issue on
+ * some machines where system events sporadically
+ * become disabled. */
+hci_write1(HCI_SYSTEM_EVENT, 1, hci_result);
+printk(MY_NOTICE Re-enabled hotkeys\n);
+ } else {
+printk(MY_ERR Error reading hotkey status\n);
+goto end;
+ }
}
+ } else {
+ key_event_valid = 0;
+ last_key_event = 0;
}
p += sprintf(p, hotkey_ready:%d\n, key_event_valid);
p += sprintf(p, hotkey: 0x%04x\n, last_key_event);
+ p += sprintf(p, hotkeys_via_input: %d\n, hotkeys_over_input);
end:
return p;
@@ -534,15 +557,127 @@ static acpi_status __exit remove_device(void)
}
static struct backlight_ops toshiba_backlight_data = {
-.get_brightness = get_lcd,
-.update_status = set_lcd_status,
+ .get_brightness = get_lcd,
+ .update_status = set_lcd_status
Re: Add INPUT support to toshiba_acpi
On Thu, 2007-05-31 at 18:46 +0200, Andreas Mohr wrote:
HCI_EMPTY is *by far* the most frequent state to occur I think
(users won't press keys all the time), thus it's probably better(?)
for branch prediction to have this placed first, right?
Not that it matters too much instruction-wise, but still...
Sure.
Apart from that I'm very happy to see progress on this front
(speaking as a proud owner of an old Toshiba notebook requiring
this stuff).
Good - this stuff should just work :-)
And I'd definitely move the multiple identical Re-enabled hotkeys parts
into one single non-inlined(!) function for the same reason.
Not to mention that it's BUTT UGLY to have the *same* fat
multi-line comment duplicated bazillion times.
Agree. New patch (untested) attached.
Thanks for review,
Richard.
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 139f41f..38835b6 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -222,6 +222,7 @@ config ACPI_TOSHIBA
tristate Toshiba Laptop Extras
depends on X86
select BACKLIGHT_CLASS_DEVICE
+ select INPUT_POLLDEV
---help---
This driver adds support for access to certain system settings
on legacy free Toshiba laptops. These laptops can be recognized by
diff --git a/drivers/acpi/toshiba_acpi.c b/drivers/acpi/toshiba_acpi.c
index 3906d47..f802609 100644
--- a/drivers/acpi/toshiba_acpi.c
+++ b/drivers/acpi/toshiba_acpi.c
@@ -27,13 +27,13 @@
* engineering the Windows drivers
* Yasushi Nagato - changes for linux kernel 2.4 - 2.5
* Rob Miller - TV out and hotkeys help
- *
+ * Richard Hughes - emit INPUT events for hotkeys
*
* TODO
*
*/
-#define TOSHIBA_ACPI_VERSION 0.18
+#define TOSHIBA_ACPI_VERSION 0.19
#define PROC_INTERFACE_VERSION 1
#include linux/kernel.h
@@ -42,6 +42,7 @@
#include linux/types.h
#include linux/proc_fs.h
#include linux/backlight.h
+#include linux/input-polldev.h
#include asm/uaccess.h
@@ -99,6 +100,16 @@ MODULE_LICENSE(GPL);
#define HCI_VIDEO_OUT_CRT 0x2
#define HCI_VIDEO_OUT_TV 0x4
+/* key definitions */
+#define HCI_HKEY_MUTE 0x0101
+#define HCI_HKEY_LOCK 0x013b
+#define HCI_HKEY_SEARCH 0x013c
+#define HCI_HKEY_SUSPEND 0x013d
+#define HCI_HKEY_HIBERNATE 0x013e
+#define HCI_HKEY_BRIGHTNESSDOWN 0x0140
+#define HCI_HKEY_BRIGHTNESSUP 0x0141
+#define HCI_HKEY_WLAN 0x0142
+
/* utility
*/
@@ -213,9 +224,15 @@ static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result)
static struct proc_dir_entry *toshiba_proc_dir /*= 0*/ ;
static struct backlight_device *toshiba_backlight_device;
+static struct input_polled_dev *toshiba_poll_dev;
static int force_fan;
static int last_key_event;
static int key_event_valid;
+static int hotkeys_over_input = 1;
+static int hotkeys_check_per_sec = 2;
+
+module_param(hotkeys_over_input, uint, 0400);
+module_param(hotkeys_check_per_sec, uint, 0400);
typedef struct _ProcItem {
const char *name;
@@ -438,34 +455,60 @@ static unsigned long write_fan(const char *buffer, unsigned long count)
return count;
}
-static char *read_keys(char *p)
+/* returns zero if hci event is invalid */
+static u32 get_hci_system_event_value(void)
{
u32 hci_result;
u32 value;
+ u32 retval = 0;
+
+ /* read a system event from the queue */
+ hci_read1(HCI_SYSTEM_EVENT, value, hci_result);
+
+ switch (hci_result) {
+ case HCI_EMPTY:
+ /* better luck next time */
+ break;
+ case HCI_SUCCESS:
+ /* value is the data in the queue */
+ retval = value;
+ break;
+ case HCI_NOT_SUPPORTED:
+ /* This is a workaround for an unresolved issue on
+ * some machines where system events sporadically
+ * become disabled. */
+ hci_write1(HCI_SYSTEM_EVENT, 1, hci_result);
+ printk(MY_NOTICE Re-enabled hotkeys\n);
+ break;
+ default:
+ /* there was an unknown error */
+ printk(MY_ERR Error reading hotkey status\n);
+ }
+ return retval;
+}
- if (!key_event_valid) {
- hci_read1(HCI_SYSTEM_EVENT, value, hci_result);
- if (hci_result == HCI_SUCCESS) {
- key_event_valid = 1;
- last_key_event = value;
- } else if (hci_result == HCI_EMPTY) {
- /* better luck next time */
- } else if (hci_result == HCI_NOT_SUPPORTED) {
- /* This is a workaround for an unresolved issue on
- * some machines where system events sporadically
- * become disabled. */
- hci_write1(HCI_SYSTEM_EVENT, 1, hci_result);
- printk(MY_NOTICE Re-enabled hotkeys\n);
- } else {
- printk(MY_ERR Error reading hotkey status\n);
- goto end;
+/* deprecate this interface... */
+static char *read_keys(char *p)
+{
+ u32 value;
+
+ if (hotkeys_over_input) {
+ key_event_valid = 0;
+ last_key_event = 0;
+ } else {
+ if (!key_event_valid) {
+ value = get_hci_system_event_value();
+ if (value != 0) {
+key_event_valid = 1;
+last_key_event = value;
+ }
}
}
p += sprintf(p, hotkey_ready:%d\n, key_event_valid);
p += sprintf(p, hotkey: 0x%04x\n, last_key_event);
+ p += sprintf(p, hotkeys_via_input: %d\n, hotkeys_over_input
Re: sonypc with Sony Vaio VGN-SZ1VP
On Thu, 2007-01-04 at 13:28 -0800, Andrew Morton wrote: > On Thu, 4 Jan 2007 22:18:30 +0100 > Mattia Dongili <[EMAIL PROTECTED]> wrote: > > > > The place to start (please) is the patches in -mm: > > > > > > 2.6-sony_acpi4.patch > > > sony_apci-resume.patch > > > sony_apci-resume-fix.patch > > > acpi-add-backlight-support-to-the-sony_acpi.patch > > > acpi-add-backlight-support-to-the-sony_acpi-v2.patch > > > video-sysfs-support-take-2-add-dev-argument-for-backlight_device_register-sony_acpi-fix.patch > > > > > > It presently has both the /proc and /sys/.../backlight/.. interfaces, so > > > the first > > > job would be to chop out the /proc stuff. > > > > Ok, I'll import all of them and start from there. > > Is it ok to wipe all the /proc stuff without notice? > > spose so. I don't know if any apps are dependent upon the /proc file, > but the driver isn't in mainline yet so it's unlikely that there's a > mountain of software depending upon existing interfaces. Well, HAL has used it for changing the brightness for the last year or so: /proc/acpi/sony/brightness Although if you use a new enough HAL (CVS), the laptop will be supported via the shiny new backlight class. Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: sonypc with Sony Vaio VGN-SZ1VP
On Thu, 2007-01-04 at 13:28 -0800, Andrew Morton wrote: On Thu, 4 Jan 2007 22:18:30 +0100 Mattia Dongili [EMAIL PROTECTED] wrote: The place to start (please) is the patches in -mm: 2.6-sony_acpi4.patch sony_apci-resume.patch sony_apci-resume-fix.patch acpi-add-backlight-support-to-the-sony_acpi.patch acpi-add-backlight-support-to-the-sony_acpi-v2.patch video-sysfs-support-take-2-add-dev-argument-for-backlight_device_register-sony_acpi-fix.patch It presently has both the /proc and /sys/.../backlight/.. interfaces, so the first job would be to chop out the /proc stuff. Ok, I'll import all of them and start from there. Is it ok to wipe all the /proc stuff without notice? spose so. I don't know if any apps are dependent upon the /proc file, but the driver isn't in mainline yet so it's unlikely that there's a mountain of software depending upon existing interfaces. Well, HAL has used it for changing the brightness for the last year or so: /proc/acpi/sony/brightness Although if you use a new enough HAL (CVS), the laptop will be supported via the shiny new backlight class. Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: status of the USB ov511.c driver in kernel 2.6?
On Mon, 28 Feb 2005 23:48:13 +0100, Adrian Bunk wrote: > - there's no *_decomp.c module in the kernel sources If I remember correctly people are bit uneasy about putting decompression of proprietary codecs into to the kernel. See http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg04778.html Every kernel release I end up building an external module just to get my OV518 usb webcam to work. (It requires decompression support) Not cool. > Are there any reasons why the upstream driver can't be resynced with the > kernel? I think ovcamchip is from the development branch, but ov511 is from the stable branch. Not sure about that. Best bet is contact Mark McClelland (maintainer) here: mmcclell calpoly.edu or mark alpha.dyndns.org Richard Hughes -- http://www.hughsie.com/PUBLIC-KEY - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: status of the USB ov511.c driver in kernel 2.6?
On Mon, 28 Feb 2005 23:48:13 +0100, Adrian Bunk wrote: - there's no *_decomp.c module in the kernel sources If I remember correctly people are bit uneasy about putting decompression of proprietary codecs into to the kernel. See http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg04778.html Every kernel release I end up building an external module just to get my OV518 usb webcam to work. (It requires decompression support) Not cool. Are there any reasons why the upstream driver can't be resynced with the kernel? I think ovcamchip is from the development branch, but ov511 is from the stable branch. Not sure about that. Best bet is contact Mark McClelland (maintainer) here: mmcclell atsign calpoly.edu or mark atsign alpha.dyndns.org Richard Hughes -- http://www.hughsie.com/PUBLIC-KEY - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Linux hangs during IDE initialization at boot for 30 sec
Con Kolivas kolivas.org> writes: > >>I'm not sure how the list of intefaces is probed on this machine, that's > >>probably where the problem is. > > I've read that people have had this problem go away if they disable this > option: > > < > generic/default IDE chipset support Okay I'll try this tonight. I'm running a stock Fedora kernel at the moment (rawhide) so this issue might affect more people than we think. > If you have chipset support for your IDE controller this isn't needed, > and I'd recommend disabling it. The "why" it made the problem go away is > something I can't answer. Richard. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Linux hangs during IDE initialization at boot for 30 sec
Benjamin Herrenschmidt kernel.crashing.org> writes: > This looks like bogus HW, or bogus list of IDE interfaces ... How can I test to see if this is the case? > > The IDE layer waits up to 30 seconds for a device to drop it's busy bit, > which is necessary for some drives that aren't fully initialized yet. Sure, make sense. > I suspect in your case, it's reading "ff", which indicates either that > there is no hardware where the kernel tries to probe, or that there is > bogus IDE interfaces which don't properly have the D7 line pulled low so > that BUSY appears not set in absence of a drive. Right. How do I find the value of D7? > I'm not sure how the list of intefaces is probed on this machine, that's > probably where the problem is. Thanks, Richard Hughes - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Linux hangs during IDE initialization at boot for 30 sec
Benjamin Herrenschmidt benh at kernel.crashing.org writes: This looks like bogus HW, or bogus list of IDE interfaces ... How can I test to see if this is the case? The IDE layer waits up to 30 seconds for a device to drop it's busy bit, which is necessary for some drives that aren't fully initialized yet. Sure, make sense. I suspect in your case, it's reading ff, which indicates either that there is no hardware where the kernel tries to probe, or that there is bogus IDE interfaces which don't properly have the D7 line pulled low so that BUSY appears not set in absence of a drive. Right. How do I find the value of D7? I'm not sure how the list of intefaces is probed on this machine, that's probably where the problem is. Thanks, Richard Hughes - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Linux hangs during IDE initialization at boot for 30 sec
Con Kolivas kernel at kolivas.org writes: I'm not sure how the list of intefaces is probed on this machine, that's probably where the problem is. I've read that people have had this problem go away if they disable this option: generic/default IDE chipset support Okay I'll try this tonight. I'm running a stock Fedora kernel at the moment (rawhide) so this issue might affect more people than we think. If you have chipset support for your IDE controller this isn't needed, and I'd recommend disabling it. The why it made the problem go away is something I can't answer. Richard. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Linux hangs during IDE initialization at boot for 30 sec
On Tue, 01 Feb 2005 12:57:33 +0100, Michael Brade wrote: > Hi, > > since at least kernel 2.6.9 I'm having a problem booting linux - it hangs > after this > > Probing IDE interface ide0... > hda: HITACHI_DK23DA-30, ATA DISK drive > elevator: using anticipatory as default io scheduler > ide0 at 0x1f0-0x1f7,0x3f6 on irq 14 > Probing IDE interface ide1... > hdc: TOSHIBA DVD-ROM SD-R2212, ATAPI CD/DVD-ROM drive > ide1 at 0x170-0x177,0x376 on irq 15 > > After about 30 seconds everything continues fine with > > hda: max request size: 128KiB > > I found additional lines in the log just before the line above: Same here on 2.6.11-rc2-bk3 using a *Toshiba* Satellite Pro A10. messages can be found here: http://hughsie.no-ip.com/write/kernel/messages please ask if you need any more info. Richard Hughes - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Linux hangs during IDE initialization at boot for 30 sec
On Tue, 01 Feb 2005 12:57:33 +0100, Michael Brade wrote: Hi, since at least kernel 2.6.9 I'm having a problem booting linux - it hangs after this Probing IDE interface ide0... hda: HITACHI_DK23DA-30, ATA DISK drive elevator: using anticipatory as default io scheduler ide0 at 0x1f0-0x1f7,0x3f6 on irq 14 Probing IDE interface ide1... hdc: TOSHIBA DVD-ROM SD-R2212, ATAPI CD/DVD-ROM drive ide1 at 0x170-0x177,0x376 on irq 15 After about 30 seconds everything continues fine with hda: max request size: 128KiB I found additional lines in the log just before the line above: Same here on 2.6.11-rc2-bk3 using a *Toshiba* Satellite Pro A10. messages can be found here: http://hughsie.no-ip.com/write/kernel/messages please ask if you need any more info. Richard Hughes - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: OpenOffice crashes due to incorrect access permissions on /dev/dri/card*
On Sat, 29 Jan 2005 12:49:16 +, Richard Hughes wrote: > Note, that strace glxgears gives exactly the same output, going from 0 to > 14 and then seg-faulting, so it's *not just a oo problem*. I know it's bad to answer your own post, but here goes. I changed my /etc/udev/permissions.d/50-udev.permissions config to read: dri/*:root:root:0666 changing it from dri/*:root:root:0660 And oowriter and glxgears work from bootup. Shall I file a bug with udev? Richard Hughes -- http://www.hughsie.com/PUBLIC-KEY - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
OpenOffice crashes due to incorrect access permissions on /dev/dri/card*
On Sat, 29 Jan 2005 13:32:47 +0100, Bernd Eckenfels wrote:
> it is not a permission thing, it tells you, that you have not card14 - and i
> dont know the dri interface but it looks unlikely that there ever will be
> one .)
I figured (maybe incorrectly) that oo was just going thru the dri card*
files:
stat64("/dev/dri", {st_mode=S_IFDIR|0755, st_size=80, ...}) = 0
stat64("/dev/dri/card8", 0xbfec2e8c)= -1 ENOENT (No such file or directory)
geteuid32() = 500
stat64("/dev/dri", {st_mode=S_IFDIR|0755, st_size=80, ...}) = 0
stat64("/dev/dri/card9", 0xbfec2e8c)= -1 ENOENT (No such file or directory)
geteuid32() = 500
stat64("/dev/dri", {st_mode=S_IFDIR|0755, st_size=80, ...}) = 0
stat64("/dev/dri/card10", 0xbfec2e8c) = -1 ENOENT (No such file or directory)
geteuid32() = 500
stat64("/dev/dri", {st_mode=S_IFDIR|0755, st_size=80, ...}) = 0
stat64("/dev/dri/card11", 0xbfec2e8c) = -1 ENOENT (No such file or directory)
geteuid32() = 500
stat64("/dev/dri", {st_mode=S_IFDIR|0755, st_size=80, ...}) = 0
stat64("/dev/dri/card12", 0xbfec2e8c) = -1 ENOENT (No such file or directory)
geteuid32() = 500
stat64("/dev/dri", {st_mode=S_IFDIR|0755, st_size=80, ...}) = 0
stat64("/dev/dri/card13", 0xbfec2e8c) = -1 ENOENT (No such file or directory)
geteuid32() = 500
stat64("/dev/dri", {st_mode=S_IFDIR|0755, st_size=80, ...}) = 0
stat64("/dev/dri/card14", 0xbfec2e8c) = -1 ENOENT (No such file or directory)
munmap(0x9b4838, 8192) = -1 EINVAL (Invalid argument)
munmap(0x9d55248, 3219928560) = -1 EINVAL (Invalid argument)
--- SIGSEGV (Segmentation fault) @ 0 (0) ---
+++ killed by SIGSEGV +++
from 0 to 14 until it found a card that it could use. The root cause
has to be that it can't use dri/card* because of access permissions.
Note, that strace glxgears gives exactly the same output, going from 0 to
14 and then seg-faulting, so it's *not just a oo problem*.
Richard Hughes
--
http://www.hughsie.com/PUBLIC-KEY
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Bug 4081] New: OpenOffice crashes while starting due to a threading error
On Fri, 28 Jan 2005 15:57:13 -0800, Stephen Hemminger wrote:
> Note: on 2.6.10
> /dev/dri/card0 crw-rw-rw-
> on 2.6.11-rc2
> /dev/dri/card0 crw-rw
> /dev/dri/card1 crw-rw
>
> Changing permissions seems to fix (it for startup), will try more and see
> if udev remembers not to turn them back.
Me too. (2.6.11-rc2-bk3, openoffice.org-1.1.3) I straced soffice and found
it hanging on /dev/dri:
geteuid32() = 500
stat64("/dev/dri", {st_mode=S_IFDIR|0755, st_size=80, ...}) = 0
stat64("/dev/dri/card14", 0xbff9c8bc) = -1 ENOENT (No such file or directory)
munmap(0x9b4838, 8192) = -1 EINVAL (Invalid argument)
munmap(0x949e248, 322082) = -1 EINVAL (Invalid argument)
--- SIGSEGV (Segmentation fault) @ 0 (0) ---
+++ killed by SIGSEGV +++
and with a chmod a+rw /dev/dri/card* everything is okay until I reboot,
and it defaults back to crw-rw
What is at fault? Certainly oo shouldn't just seg-fault, but should the
permissions on /dev/dri/card* be crw-rw or crw-rw-rw-?
Richard Hughes
--
http://www.hughsie.com/PUBLIC-KEY
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Bug 4081] New: OpenOffice crashes while starting due to a threading error
On Fri, 28 Jan 2005 15:57:13 -0800, Stephen Hemminger wrote:
Note: on 2.6.10
/dev/dri/card0 crw-rw-rw-
on 2.6.11-rc2
/dev/dri/card0 crw-rw
/dev/dri/card1 crw-rw
Changing permissions seems to fix (it for startup), will try more and see
if udev remembers not to turn them back.
Me too. (2.6.11-rc2-bk3, openoffice.org-1.1.3) I straced soffice and found
it hanging on /dev/dri:
geteuid32() = 500
stat64(/dev/dri, {st_mode=S_IFDIR|0755, st_size=80, ...}) = 0
stat64(/dev/dri/card14, 0xbff9c8bc) = -1 ENOENT (No such file or directory)
munmap(0x9b4838, 8192) = -1 EINVAL (Invalid argument)
munmap(0x949e248, 322082) = -1 EINVAL (Invalid argument)
--- SIGSEGV (Segmentation fault) @ 0 (0) ---
+++ killed by SIGSEGV +++
and with a chmod a+rw /dev/dri/card* everything is okay until I reboot,
and it defaults back to crw-rw
What is at fault? Certainly oo shouldn't just seg-fault, but should the
permissions on /dev/dri/card* be crw-rw or crw-rw-rw-?
Richard Hughes
--
http://www.hughsie.com/PUBLIC-KEY
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: OpenOffice crashes due to incorrect access permissions on /dev/dri/card*
On Sat, 29 Jan 2005 12:49:16 +, Richard Hughes wrote: Note, that strace glxgears gives exactly the same output, going from 0 to 14 and then seg-faulting, so it's *not just a oo problem*. I know it's bad to answer your own post, but here goes. I changed my /etc/udev/permissions.d/50-udev.permissions config to read: dri/*:root:root:0666 changing it from dri/*:root:root:0660 And oowriter and glxgears work from bootup. Shall I file a bug with udev? Richard Hughes -- http://www.hughsie.com/PUBLIC-KEY - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
