Hi,
On 06-07-17 19:11, Andy Shevchenko wrote:
On Thu, 2017-07-06 at 18:49 +0200, Hans de Goede wrote:
At least on the UP board SBC both PWMs are enabled leading to us
trying to add the same pwm_lookup twice, which leads to the following:
[ 0.902224] list_add double add: new=ffffffffb8efd400,
prev=ffffffffb8efd400, next=ffffffffb8eeede0.
[ 0.912466] ------------[ cut here ]------------
[ 0.917624] kernel BUG at lib/list_debug.c:31!
[ 0.922588] invalid opcode: 0000 [#1] SMP
...
[ 1.027450] Call Trace:
[ 1.030185] pwm_add_table+0x4c/0x90
[ 1.034181] bsw_pwm_setup+0x1a/0x20
[ 1.038175] acpi_lpss_create_device+0xfe/0x420
...
This commit fixes this by only calling pwm_add_table for the first
PWM controller (which is the one used for the backlight).
Thanks, my comment below.
For the quick fix I agree on this:
Acked-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Thank you.
Rafael, can we get this added to 4.13 soonish please ? One of the 2
commits this fixes has been causing trouble for some users since 4.11.3.
By the way, do you need a shell script that allows to setup pin muxing
via external CPLD?
Thanks, but no thanks I don't want to make physical alterations to
my boards.
Cc: stable@xxxxxxxxxxxxxxx
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1458599
Fixes: bf7696a12071 ("acpi: lpss: call pwm_add_table() for BSW...")
Fixes: 04434ab5120a ("ACPI / LPSS: Call pwm_add_table() for Bay
Trail...")
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
drivers/acpi/acpi_lpss.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index 10347e3d73ad..5bd58bd4ab05 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -85,6 +85,7 @@ static const struct lpss_device_desc lpss_dma_desc =
{
};
struct lpss_private_data {
+ struct acpi_device *adev;
void __iomem *mmio_base;
resource_size_t mmio_size;
unsigned int fixed_clk_rate;
@@ -155,6 +156,12 @@ static struct pwm_lookup byt_pwm_lookup[] = {
static void byt_pwm_setup(struct lpss_private_data *pdata)
{
+ struct acpi_device *adev = pdata->adev;
+
+ /* Only call pwm_add_table for the first PWM controller */
+ if (!adev->pnp.unique_id || strcmp(adev->pnp.unique_id, "1"))
+ return;
+
It would be nice to have a separate mapping between UID and lookup
table.
If we ever have the need for mappings for more then the first
PWM controller, then yes we should add something like that.
Regards,
Hans
Though, for now it's only one case, perhaps we may do this later.
if (!acpi_dev_present("INT33FD", NULL, -1))
pwm_add_table(byt_pwm_lookup,
ARRAY_SIZE(byt_pwm_lookup));
}
@@ -180,6 +187,12 @@ static struct pwm_lookup bsw_pwm_lookup[] = {
static void bsw_pwm_setup(struct lpss_private_data *pdata)
{
+ struct acpi_device *adev = pdata->adev;
+
+ /* Only call pwm_add_table for the first PWM controller */
+ if (!adev->pnp.unique_id || strcmp(adev->pnp.unique_id, "1"))
+ return;
+
pwm_add_table(bsw_pwm_lookup, ARRAY_SIZE(bsw_pwm_lookup));
}
@@ -456,6 +469,7 @@ static int acpi_lpss_create_device(struct
acpi_device *adev,
goto err_out;
}
+ pdata->adev = adev;
pdata->dev_desc = dev_desc;
if (dev_desc->setup)