[PATCH] HID: wacom: Fix invalid power_supply_powers calls

power_supply_powers calls added in 35b4c01e29bdd9632dabf9784ed3486333f00427
have to be called after power device is created. This patch also fixes the
second call - it has to be "ac" instead of "battery"

Signed-off-by: Przemo Firszt <redacted>
Signed-off-by: Chris Bagwell <redacted>
---
 drivers/hid/hid-wacom.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index b47e58b..acab74c 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c

@@ -531,7 +531,6 @@ static int wacom_probe(struct hid_device *hdev,
 	wdata->battery.type = POWER_SUPPLY_TYPE_BATTERY;
 	wdata->battery.use_for_apm = 0;
 
-	power_supply_powers(&wdata->battery, &hdev->dev);
 
 	ret = power_supply_register(&hdev->dev, &wdata->battery);
 	if (ret) {

@@ -540,6 +539,8 @@ static int wacom_probe(struct hid_device *hdev,
 		goto err_battery;
 	}
 
+	power_supply_powers(&wdata->battery, &hdev->dev);
+
 	wdata->ac.properties = wacom_ac_props;
 	wdata->ac.num_properties = ARRAY_SIZE(wacom_ac_props);
 	wdata->ac.get_property = wacom_ac_get_property;

@@ -547,14 +548,14 @@ static int wacom_probe(struct hid_device *hdev,
 	wdata->ac.type = POWER_SUPPLY_TYPE_MAINS;
 	wdata->ac.use_for_apm = 0;
 
-	power_supply_powers(&wdata->battery, &hdev->dev);
-
 	ret = power_supply_register(&hdev->dev, &wdata->ac);
 	if (ret) {
 		hid_warn(hdev,
 			 "can't create ac battery attribute, err: %d\n", ret);
 		goto err_ac;
 	}
+
+	power_supply_powers(&wdata->ac, &hdev->dev);
 #endif
 	return 0;
 

-- 
1.7.6.4

On Sun, 5 Feb 2012, Przemo Firszt wrote:

power_supply_powers calls added in 35b4c01e29bdd9632dabf9784ed3486333f00427
have to be called after power device is created. This patch also fixes the
second call - it has to be "ac" instead of "battery"

Signed-off-by: Przemo Firszt <redacted>
Signed-off-by: Chris Bagwell <redacted>

[ adding Jeremy to CC ]

Hmm, seems valid. How did you notice? Have you seen crashes because of 
wild pointers?

Thanks,

-- 
Jiri Kosina
SUSE Labs

On Sun, 5 Feb 2012, Przemo Firszt wrote:

[..]

Hmm, seems valid. How did you notice? Have you seen crashes because of
wild pointers?

Hi Jiri,
Yes, the driver was unusable - 100% crashes during connection.

Chris came up with the solution, I did coding & testing.

The crash details are here:  http://pastebin.com/ZVNZWaPs

regards,
Przemo

On Mon, 6 Feb 2012, przemo@firszt.eu wrote:

quoted

Hmm, seems valid. How did you notice? Have you seen crashes because of
wild pointers?

Hi Jiri,
Yes, the driver was unusable - 100% crashes during connection.

Okay, I thought that'd be the case.

Chris came up with the solution, I did coding & testing.

The crash details are here:  http://pastebin.com/ZVNZWaPs

Thank you. Will be pushing to Linus soon.

-- 
Jiri Kosina
SUSE Labs

On 02/06/2012 04:42 AM, Jiri Kosina wrote:

On Mon, 6 Feb 2012, przemo@firszt.eu wrote:

quoted

quoted

Hmm, seems valid. How did you notice? Have you seen crashes because of
wild pointers?

Hi Jiri,
Yes, the driver was unusable - 100% crashes during connection.

Okay, I thought that'd be the case.

Very sorry about that.  I don't have a device to test with, so I should
have reviewed the code extra carefully.

Does the same bug apply to the Wii changes, which were of the same form?

    J

On 02/06/2012 04:42 AM, Jiri Kosina wrote:

quoted

On Mon, 6 Feb 2012, przemo@firszt.eu wrote:

quoted

quoted

Hmm, seems valid. How did you notice? Have you seen crashes because of
wild pointers?

Hi Jiri,
Yes, the driver was unusable - 100% crashes during connection.

Okay, I thought that'd be the case.

Very sorry about that.  I don't have a device to test with, so I should
have reviewed the code extra carefully.

Does the same bug apply to the Wii changes, which were of the same form?

Hi Jeremy,
The wii code looks the same, so probably it's affected as well. Can you
make a patch?

power_supply_powers call in wiimote driver:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/hid/hid-wiimote-core.c#l1229

regards,
Przemo Firszt

On Mon, 6 Feb 2012, przemo@firszt.eu wrote:

quoted

quoted

quoted

quoted

Hmm, seems valid. How did you notice? Have you seen crashes because of
wild pointers?

Hi Jiri,
Yes, the driver was unusable - 100% crashes during connection.

Okay, I thought that'd be the case.

Very sorry about that.  I don't have a device to test with, so I should
have reviewed the code extra carefully.

Does the same bug apply to the Wii changes, which were of the same form?

Hi Jeremy,
The wii code looks the same, so probably it's affected as well. Can you
make a patch?

power_supply_powers call in wiimote driver:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/hid/hid-wiimote-core.c#l1229

I have now queued the patch below for the same pile as well.
Thanks for spotting it.


From: Jiri Kosina <redacted>
Subject: [PATCH] HID: wiimote: fix invalid power_supply_powers call

Analogically to d7cb3dbd1 ("HID: wacom: Fix invalid power_supply_powers
calls"), fix also the same occurence in wiimote driver.

Reported-by: przemo@firszt.eu
Signed-off-by: Jiri Kosina <redacted>
---
 drivers/hid/hid-wiimote-core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
index fc253b4..cac3589 100644
--- a/drivers/hid/hid-wiimote-core.c
+++ b/drivers/hid/hid-wiimote-core.c

@@ -1226,14 +1226,14 @@ static int wiimote_hid_probe(struct hid_device *hdev,
 	wdata->battery.type = POWER_SUPPLY_TYPE_BATTERY;
 	wdata->battery.use_for_apm = 0;
 
-	power_supply_powers(&wdata->battery, &hdev->dev);
-
 	ret = power_supply_register(&wdata->hdev->dev, &wdata->battery);
 	if (ret) {
 		hid_err(hdev, "Cannot register battery device\n");
 		goto err_battery;
 	}
 
+	power_supply_powers(&wdata->battery, &hdev->dev);
+
 	ret = wiimote_leds_create(wdata);
 	if (ret)
 		goto err_free;

-- 
Jiri Kosina
SUSE Labs