* [PATCH] gpio: sch: Fix Oops on module load on Asus Eee PC 1201
@ 2016-05-06 21:49 Colin Pitrat
2016-05-07 19:37 ` Colin Pitrat
2016-05-09 7:02 ` Mika Westerberg
0 siblings, 2 replies; 5+ messages in thread
From: Colin Pitrat @ 2016-05-06 21:49 UTC (permalink / raw)
To: linus.walleij, gnurou
Cc: rebecca.swee.fun.chang, mika.westerberg, linux-gpio, linux-kernel
This fixes the issue descirbe in bug 117531.
It's a regression introduced in linux 4.5 that causes a Oops at load of
gpio_sch and prevents powering off the computer.
The patch consist in reverting commit 737c8fccf1c5b2aae3c6d9a66dce17e35fc39b71
(a.k.a 'gpio: sch: use gpiochip data pointer') that causes this regression.
However, although it does work for me, I'm not sure of the impact of reverting
only this part of the patch.
Signed-off-by: Colin Pitrat <colin.pitrat@gmail.com>
---
drivers/gpio/gpio-sch.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index e85e753..a8a333ad 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -41,6 +41,8 @@ struct sch_gpio {
unsigned short resume_base;
};
+#define to_sch_gpio(gc) container_of(gc, struct sch_gpio, chip)
+
static unsigned sch_gpio_offset(struct sch_gpio *sch, unsigned gpio,
unsigned reg)
{
@@ -63,7 +65,7 @@ static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned gpio)
static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
{
- struct sch_gpio *sch = gpiochip_get_data(gc);
+ struct sch_gpio *sch = to_sch_gpio(gc);
unsigned short offset, bit;
u8 reg_val;
@@ -78,7 +80,7 @@ static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
static void sch_gpio_reg_set(struct gpio_chip *gc, unsigned gpio, unsigned reg,
int val)
{
- struct sch_gpio *sch = gpiochip_get_data(gc);
+ struct sch_gpio *sch = to_sch_gpio(gc);
unsigned short offset, bit;
u8 reg_val;
@@ -95,7 +97,7 @@ static void sch_gpio_reg_set(struct gpio_chip *gc, unsigned gpio, unsigned reg,
static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
{
- struct sch_gpio *sch = gpiochip_get_data(gc);
+ struct sch_gpio *sch = to_sch_gpio(gc);
spin_lock(&sch->lock);
sch_gpio_reg_set(gc, gpio_num, GIO, 1);
@@ -110,7 +112,7 @@ static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
{
- struct sch_gpio *sch = gpiochip_get_data(gc);
+ struct sch_gpio *sch = to_sch_gpio(gc);
spin_lock(&sch->lock);
sch_gpio_reg_set(gc, gpio_num, GLV, val);
@@ -120,7 +122,7 @@ static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
int val)
{
- struct sch_gpio *sch = gpiochip_get_data(gc);
+ struct sch_gpio *sch = to_sch_gpio(gc);
spin_lock(&sch->lock);
sch_gpio_reg_set(gc, gpio_num, GIO, 0);
@@ -215,7 +217,15 @@ static int sch_gpio_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, sch);
- return devm_gpiochip_add_data(&pdev->dev, &sch->chip, sch);
+ return gpiochip_add(&sch->chip);
+}
+
+static int sch_gpio_remove(struct platform_device *pdev)
+{
+ struct sch_gpio *sch = platform_get_drvdata(pdev);
+
+ gpiochip_remove(&sch->chip);
+ return 0;
}
static struct platform_driver sch_gpio_driver = {
@@ -223,6 +233,7 @@ static struct platform_driver sch_gpio_driver = {
.name = "sch_gpio",
},
.probe = sch_gpio_probe,
+ .remove = sch_gpio_remove,
};
module_platform_driver(sch_gpio_driver);
--
2.8.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio: sch: Fix Oops on module load on Asus Eee PC 1201
2016-05-06 21:49 [PATCH] gpio: sch: Fix Oops on module load on Asus Eee PC 1201 Colin Pitrat
@ 2016-05-07 19:37 ` Colin Pitrat
2016-05-09 7:02 ` Mika Westerberg
1 sibling, 0 replies; 5+ messages in thread
From: Colin Pitrat @ 2016-05-07 19:37 UTC (permalink / raw)
To: linus.walleij, gnurou
Cc: rebecca.swee.fun.chang, mika.westerberg, linux-gpio, linux-kernel
On Fri, May 06, 2016 at 10:49:23PM +0100, Colin Pitrat wrote:
> This fixes the issue descirbe in bug 117531.
> It's a regression introduced in linux 4.5 that causes a Oops at load of
> gpio_sch and prevents powering off the computer.
>
> The patch consist in reverting commit 737c8fccf1c5b2aae3c6d9a66dce17e35fc39b71
> (a.k.a 'gpio: sch: use gpiochip data pointer') that causes this regression.
> However, although it does work for me, I'm not sure of the impact of reverting
> only this part of the patch.
>
> Signed-off-by: Colin Pitrat <colin.pitrat@gmail.com>
>
> ---
> drivers/gpio/gpio-sch.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
> index e85e753..a8a333ad 100644
> --- a/drivers/gpio/gpio-sch.c
> +++ b/drivers/gpio/gpio-sch.c
> @@ -41,6 +41,8 @@ struct sch_gpio {
> unsigned short resume_base;
> };
>
> +#define to_sch_gpio(gc) container_of(gc, struct sch_gpio, chip)
> +
> static unsigned sch_gpio_offset(struct sch_gpio *sch, unsigned gpio,
> unsigned reg)
> {
> @@ -63,7 +65,7 @@ static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned gpio)
>
> static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
> {
> - struct sch_gpio *sch = gpiochip_get_data(gc);
> + struct sch_gpio *sch = to_sch_gpio(gc);
> unsigned short offset, bit;
> u8 reg_val;
>
> @@ -78,7 +80,7 @@ static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
> static void sch_gpio_reg_set(struct gpio_chip *gc, unsigned gpio, unsigned reg,
> int val)
> {
> - struct sch_gpio *sch = gpiochip_get_data(gc);
> + struct sch_gpio *sch = to_sch_gpio(gc);
> unsigned short offset, bit;
> u8 reg_val;
>
> @@ -95,7 +97,7 @@ static void sch_gpio_reg_set(struct gpio_chip *gc, unsigned gpio, unsigned reg,
>
> static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
> {
> - struct sch_gpio *sch = gpiochip_get_data(gc);
> + struct sch_gpio *sch = to_sch_gpio(gc);
>
> spin_lock(&sch->lock);
> sch_gpio_reg_set(gc, gpio_num, GIO, 1);
> @@ -110,7 +112,7 @@ static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
>
> static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
> {
> - struct sch_gpio *sch = gpiochip_get_data(gc);
> + struct sch_gpio *sch = to_sch_gpio(gc);
>
> spin_lock(&sch->lock);
> sch_gpio_reg_set(gc, gpio_num, GLV, val);
> @@ -120,7 +122,7 @@ static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
> static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
> int val)
> {
> - struct sch_gpio *sch = gpiochip_get_data(gc);
> + struct sch_gpio *sch = to_sch_gpio(gc);
>
> spin_lock(&sch->lock);
> sch_gpio_reg_set(gc, gpio_num, GIO, 0);
> @@ -215,7 +217,15 @@ static int sch_gpio_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, sch);
>
> - return devm_gpiochip_add_data(&pdev->dev, &sch->chip, sch);
> + return gpiochip_add(&sch->chip);
> +}
> +
> +static int sch_gpio_remove(struct platform_device *pdev)
> +{
> + struct sch_gpio *sch = platform_get_drvdata(pdev);
> +
> + gpiochip_remove(&sch->chip);
> + return 0;
> }
>
> static struct platform_driver sch_gpio_driver = {
> @@ -223,6 +233,7 @@ static struct platform_driver sch_gpio_driver = {
> .name = "sch_gpio",
> },
> .probe = sch_gpio_probe,
> + .remove = sch_gpio_remove,
> };
>
> module_platform_driver(sch_gpio_driver);
> --
> 2.8.2
>
Oups sorry, this patch is wrong !
It reverts two changes instead of just the one causing the issue.
I'll send a v2 with only the required change reverted.
Sorry for the inconvenience.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio: sch: Fix Oops on module load on Asus Eee PC 1201
2016-05-06 21:49 [PATCH] gpio: sch: Fix Oops on module load on Asus Eee PC 1201 Colin Pitrat
2016-05-07 19:37 ` Colin Pitrat
@ 2016-05-09 7:02 ` Mika Westerberg
2016-05-09 19:37 ` Colin Pitrat
1 sibling, 1 reply; 5+ messages in thread
From: Mika Westerberg @ 2016-05-09 7:02 UTC (permalink / raw)
To: Colin Pitrat
Cc: linus.walleij, gnurou, rebecca.swee.fun.chang, linux-gpio, linux-kernel
On Fri, May 06, 2016 at 10:49:25PM +0100, Colin Pitrat wrote:
> This fixes the issue descirbe in bug 117531.
Can you include the oops log in the changelog as well?
> It's a regression introduced in linux 4.5 that causes a Oops at load of
> gpio_sch and prevents powering off the computer.
>
> The patch consist in reverting commit 737c8fccf1c5b2aae3c6d9a66dce17e35fc39b71
> (a.k.a 'gpio: sch: use gpiochip data pointer') that causes this regression.
> However, although it does work for me, I'm not sure of the impact of reverting
> only this part of the patch.
What happens here is that after the change we expect that
gpiochip_get_data() returns the correct pointer but that only happens
after devm_gpiochip_add_data() is called, which is the last call in
sch_gpio_probe(). Before that we call sch_gpio_reg_set() couple of times
if the device id is PCI_DEVICE_ID_INTEL_SCH_LPC and that causes NULL
pointer dereference.
I think better fix is to make sch_gpio_reg_get() and sch_gpio_reg_set()
take pointer to struct sch_gpio and use that directly like:
static void sch_gpio_reg_set(struct sch_gpio *sch, unsigned gpio,
unsigned reg, int val)
{
...
}
Neither function actually need gc for anything anyway.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio: sch: Fix Oops on module load on Asus Eee PC 1201
2016-05-09 7:02 ` Mika Westerberg
@ 2016-05-09 19:37 ` Colin Pitrat
[not found] ` <CABy3NjSJTygdP3POxQQqxDA6=Jx4DeQ47KONASfm46kei7wBDQ@mail.gmail.com>
0 siblings, 1 reply; 5+ messages in thread
From: Colin Pitrat @ 2016-05-09 19:37 UTC (permalink / raw)
To: Mika Westerberg
Cc: linus.walleij, gnurou, rebecca.swee.fun.chang, linux-gpio, linux-kernel
On Mon, May 09, 2016 at 10:02:45AM +0300, Mika Westerberg wrote:
> Can you include the oops log in the changelog as well?
The oops is the following:
[ 13.753278] BUG: unable to handle kernel NULL pointer dereference at 00000094
[ 13.755532] IP: [<f834c241>] sch_gpio_probe+0x161/0x280 [gpio_sch]
[ 13.755532] *pde = 00000000
[ 13.755532] Oops: 0000 [#1] PREEMPT SMP
[ 13.755532] Modules linked in: gpio_sch(+) atl1c snd_hda_intel snd_hda_codec sparse_keymap led_class thermal rfkill snd_hda_core snd_hwdep i8042 fjes snd_pcm serio wmi acpi_cpufreq evdev snd_timer battery snd soundcore mac_hid button ac shpchp lpc_sch tpm_tis tpm processor sch_fq_codel ip_tables x_tables reiserfs uvesafb sd_mod ata_generic pata_acpi pata_sch libata scsi_mod ehci_pci uhci_hcd ehci_hcd usbcore usb_common gma500_gfx video i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm agpgart
[ 13.755532] CPU: 1 PID: 240 Comm: systemd-udevd Not tainted 4.5.1-1-ARCH #1
[ 13.755532] Hardware name: ASUSTeK Computer INC. 1201HA/1201HA, BIOS 0302 02/05/2010
[ 13.755532] task: f4cef500 ti: f4db0000 task.ti: f4db0000
[ 13.755532] EIP: 0060:[<f834c241>] EFLAGS: 00010246 CPU: 1
[ 13.755532] EIP is at sch_gpio_probe+0x161/0x280 [gpio_sch]
[ 13.755532] EAX: 0000000a EBX: f55609cc ECX: 00000000 EDX: 0000000e
[ 13.755532] ESI: f834d0cc EDI: 00000000 EBP: f4db1cb4 ESP: f4db1c98
[ 13.755532] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[ 13.755532] CR0: 80050033 CR2: 00000094 CR3: 34da5000 CR4: 000006d0
[ 13.755532] Stack:
[ 13.755532] 00000040 c008d3b0 c008d20c c008d200 c008d20c f834e014 c008d20c f4db1cd0
[ 13.755532] c1387b13 f4db1cd0 c13850e2 c008d20c 00000000 f834e014 f4db1d04 c1385b49
[ 13.755532] 00000000 f4db1cf0 c130a6ac c008d20c f834e014 00000000 f4db1d04 00000022
[ 13.755532] Call Trace:
[ 13.755532] [<c1387b13>] platform_drv_probe+0x43/0xb0
[ 13.755532] [<c13850e2>] ? driver_sysfs_add+0x62/0x80
[ 13.755532] [<c1385b49>] driver_probe_device+0x1f9/0x450
[ 13.755532] [<c130a6ac>] ? acpi_driver_match_device+0x31/0x4b
[ 13.755532] [<c1385e19>] __driver_attach+0x79/0x80
[ 13.755532] [<c1385da0>] ? driver_probe_device+0x450/0x450
[ 13.755532] [<c13839a7>] bus_for_each_dev+0x57/0xa0
[ 13.755532] [<c13852be>] driver_attach+0x1e/0x20
[ 13.755532] [<c1385da0>] ? driver_probe_device+0x450/0x450
[ 13.755532] [<c1384dcf>] bus_add_driver+0x1ef/0x290
[ 13.755532] [<f815d000>] ? 0xf815d000
[ 13.755532] [<f815d000>] ? 0xf815d000
[ 13.755532] [<c138670d>] driver_register+0x5d/0xf0
[ 13.755532] [<c1387a7f>] __platform_driver_register+0x2f/0x40
[ 13.755532] [<f815d012>] sch_gpio_driver_init+0x12/0x1000 [gpio_sch]
[ 13.755532] [<c100047a>] do_one_initcall+0xaa/0x200
[ 13.755532] [<f815d000>] ? 0xf815d000
[ 13.755532] [<c115f525>] ? kvfree+0x45/0x50
[ 13.755532] [<c11938be>] ? kmem_cache_alloc_trace+0x7e/0x1f0
[ 13.755532] [<c1140fe9>] ? do_init_module+0x21/0x1a8
[ 13.755532] [<c1140fe9>] ? do_init_module+0x21/0x1a8
[ 13.755532] [<c1141018>] do_init_module+0x50/0x1a8
[ 13.755532] [<c10e1cfc>] load_module+0x204c/0x25b0
[ 13.755532] [<c10e2387>] SyS_init_module+0x127/0x180
[ 13.755532] [<c1001b3d>] do_fast_syscall_32+0x8d/0x150
[ 13.755532] [<c150b187>] sysenter_past_esp+0x40/0x61
[ 13.755532] Code: ff ff 5b 5e 5f 5d c3 8d 76 00 8b 7b 10 31 c0 ba 0e 00 00 00 66 89 83 92 00 00 00 b8 0a 00 00 00 66 89 53 4c 66 89 83 94 00 00 00 <0f> b7 b7 94 00 00 00 83 fe 08 0f 87 df 00 00 00 b9 08 00 00 00
[ 13.755532] EIP: [<f834c241>] sch_gpio_probe+0x161/0x280 [gpio_sch] SS:ESP 0068:f4db1c98
[ 13.755532] CR2: 0000000000000094
[ 14.100430] ---[ end trace 6e7a6fbbc3457766 ]---
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio: sch: Fix Oops on module load on Asus Eee PC 1201
[not found] ` <CABy3NjSJTygdP3POxQQqxDA6=Jx4DeQ47KONASfm46kei7wBDQ@mail.gmail.com>
@ 2016-06-17 8:10 ` Mika Westerberg
0 siblings, 0 replies; 5+ messages in thread
From: Mika Westerberg @ 2016-06-17 8:10 UTC (permalink / raw)
To: Colin Pitrat
Cc: Linus Walleij, Alexandre Courbot, rebecca.swee.fun.chang,
linux-gpio, linux-kernel
On Thu, Jun 16, 2016 at 08:20:15PM +0100, Colin Pitrat wrote:
> Hello,
> do you have news on this issue ?
> Is there any way I can help ?
Yeah, rework the patch according:
https://lkml.org/lkml/2016/5/9/61
:-)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-06-17 8:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-06 21:49 [PATCH] gpio: sch: Fix Oops on module load on Asus Eee PC 1201 Colin Pitrat
2016-05-07 19:37 ` Colin Pitrat
2016-05-09 7:02 ` Mika Westerberg
2016-05-09 19:37 ` Colin Pitrat
[not found] ` <CABy3NjSJTygdP3POxQQqxDA6=Jx4DeQ47KONASfm46kei7wBDQ@mail.gmail.com>
2016-06-17 8:10 ` Mika Westerberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).