[PATCH] serial: 8250: dw: enable using pdata with ACPI

Marcin Wojtas posted 1 patch 3 years, 10 months ago
There is a newer version of this series
drivers/tty/serial/8250/8250_dw.c | 24 ++++++++++----------
1 file changed, 12 insertions(+), 12 deletions(-)
[PATCH] serial: 8250: dw: enable using pdata with ACPI
Posted by Marcin Wojtas 3 years, 10 months ago
Commit 3242fe805b52 ("serial: 8250: dw: Move the USR register to pdata")
caused NULL-pointer dereference when booting with ACPI by unconditional
usage of the recently added pdata.

In order to fix that and prevent similar issues in future, hook the
default version of this structure in dw8250_acpi_match table.

Fixes: 3242fe805b52 ("serial: 8250: dw: Move the USR register to pdata")
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 drivers/tty/serial/8250/8250_dw.c | 24 ++++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index f57bbd32ef11..3b79bd5c9c9d 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -773,18 +773,18 @@ static const struct of_device_id dw8250_of_match[] = {
 MODULE_DEVICE_TABLE(of, dw8250_of_match);
 
 static const struct acpi_device_id dw8250_acpi_match[] = {
-	{ "INT33C4", 0 },
-	{ "INT33C5", 0 },
-	{ "INT3434", 0 },
-	{ "INT3435", 0 },
-	{ "80860F0A", 0 },
-	{ "8086228A", 0 },
-	{ "APMC0D08", 0},
-	{ "AMD0020", 0 },
-	{ "AMDI0020", 0 },
-	{ "AMDI0022", 0 },
-	{ "BRCM2032", 0 },
-	{ "HISI0031", 0 },
+	{ "INT33C4", (kernel_ulong_t)&dw8250_dw_apb },
+	{ "INT33C5", (kernel_ulong_t)&dw8250_dw_apb },
+	{ "INT3434", (kernel_ulong_t)&dw8250_dw_apb },
+	{ "INT3435", (kernel_ulong_t)&dw8250_dw_apb },
+	{ "80860F0A", (kernel_ulong_t)&dw8250_dw_apb },
+	{ "8086228A", (kernel_ulong_t)&dw8250_dw_apb },
+	{ "APMC0D08", (kernel_ulong_t)&dw8250_dw_apb},
+	{ "AMD0020", (kernel_ulong_t)&dw8250_dw_apb },
+	{ "AMDI0020", (kernel_ulong_t)&dw8250_dw_apb },
+	{ "AMDI0022", (kernel_ulong_t)&dw8250_dw_apb },
+	{ "BRCM2032", (kernel_ulong_t)&dw8250_dw_apb },
+	{ "HISI0031", (kernel_ulong_t)&dw8250_dw_apb },
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, dw8250_acpi_match);
-- 
2.29.0
Re: [PATCH] serial: 8250: dw: enable using pdata with ACPI
Posted by Andy Shevchenko 3 years, 10 months ago
On Sun, Jun 19, 2022 at 9:43 AM Marcin Wojtas <mw@semihalf.com> wrote:
>
> Commit 3242fe805b52 ("serial: 8250: dw: Move the USR register to pdata")
> caused NULL-pointer dereference when booting with ACPI by unconditional
> usage of the recently added pdata.
>
> In order to fix that and prevent similar issues in future, hook the
> default version of this structure in dw8250_acpi_match table.

Thanks for the report and fix! One proposal below.

...

>  static const struct acpi_device_id dw8250_acpi_match[] = {
> -       { "INT33C4", 0 },
> -       { "INT33C5", 0 },
> -       { "INT3434", 0 },
> -       { "INT3435", 0 },
> -       { "80860F0A", 0 },
> -       { "8086228A", 0 },
> -       { "APMC0D08", 0},
> -       { "AMD0020", 0 },
> -       { "AMDI0020", 0 },
> -       { "AMDI0022", 0 },
> -       { "BRCM2032", 0 },
> -       { "HISI0031", 0 },
> +       { "INT33C4", (kernel_ulong_t)&dw8250_dw_apb },
> +       { "INT33C5", (kernel_ulong_t)&dw8250_dw_apb },
> +       { "INT3434", (kernel_ulong_t)&dw8250_dw_apb },
> +       { "INT3435", (kernel_ulong_t)&dw8250_dw_apb },
> +       { "80860F0A", (kernel_ulong_t)&dw8250_dw_apb },
> +       { "8086228A", (kernel_ulong_t)&dw8250_dw_apb },
> +       { "APMC0D08", (kernel_ulong_t)&dw8250_dw_apb},
> +       { "AMD0020", (kernel_ulong_t)&dw8250_dw_apb },
> +       { "AMDI0020", (kernel_ulong_t)&dw8250_dw_apb },
> +       { "AMDI0022", (kernel_ulong_t)&dw8250_dw_apb },
> +       { "BRCM2032", (kernel_ulong_t)&dw8250_dw_apb },
> +       { "HISI0031", (kernel_ulong_t)&dw8250_dw_apb },

Since you are touching all of them, please keep the order
alphanumerically sorted by the HID.

>         { },
>  };


-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] serial: 8250: dw: enable using pdata with ACPI
Posted by Marcin Wojtas 3 years, 10 months ago
Hi,

pon., 20 cze 2022 o 10:01 Andy Shevchenko <andy.shevchenko@gmail.com>
napisał(a):
>
> On Sun, Jun 19, 2022 at 9:43 AM Marcin Wojtas <mw@semihalf.com> wrote:
> >
> > Commit 3242fe805b52 ("serial: 8250: dw: Move the USR register to pdata")
> > caused NULL-pointer dereference when booting with ACPI by unconditional
> > usage of the recently added pdata.
> >
> > In order to fix that and prevent similar issues in future, hook the
> > default version of this structure in dw8250_acpi_match table.
>
> Thanks for the report and fix! One proposal below.
>
> ...
>
> >  static const struct acpi_device_id dw8250_acpi_match[] = {
> > -       { "INT33C4", 0 },
> > -       { "INT33C5", 0 },
> > -       { "INT3434", 0 },
> > -       { "INT3435", 0 },
> > -       { "80860F0A", 0 },
> > -       { "8086228A", 0 },
> > -       { "APMC0D08", 0},
> > -       { "AMD0020", 0 },
> > -       { "AMDI0020", 0 },
> > -       { "AMDI0022", 0 },
> > -       { "BRCM2032", 0 },
> > -       { "HISI0031", 0 },
> > +       { "INT33C4", (kernel_ulong_t)&dw8250_dw_apb },
> > +       { "INT33C5", (kernel_ulong_t)&dw8250_dw_apb },
> > +       { "INT3434", (kernel_ulong_t)&dw8250_dw_apb },
> > +       { "INT3435", (kernel_ulong_t)&dw8250_dw_apb },
> > +       { "80860F0A", (kernel_ulong_t)&dw8250_dw_apb },
> > +       { "8086228A", (kernel_ulong_t)&dw8250_dw_apb },
> > +       { "APMC0D08", (kernel_ulong_t)&dw8250_dw_apb},
> > +       { "AMD0020", (kernel_ulong_t)&dw8250_dw_apb },
> > +       { "AMDI0020", (kernel_ulong_t)&dw8250_dw_apb },
> > +       { "AMDI0022", (kernel_ulong_t)&dw8250_dw_apb },
> > +       { "BRCM2032", (kernel_ulong_t)&dw8250_dw_apb },
> > +       { "HISI0031", (kernel_ulong_t)&dw8250_dw_apb },
>
> Since you are touching all of them, please keep the order
> alphanumerically sorted by the HID.

Sure.

Thanks,
Marcin
Re: [PATCH] serial: 8250: dw: enable using pdata with ACPI
Posted by Andy Shevchenko 3 years, 10 months ago
On Mon, Jun 20, 2022 at 11:14 AM Marcin Wojtas <mw@semihalf.com> wrote:
> pon., 20 cze 2022 o 10:01 Andy Shevchenko <andy.shevchenko@gmail.com>
> napisał(a):
> > On Sun, Jun 19, 2022 at 9:43 AM Marcin Wojtas <mw@semihalf.com> wrote:

...

> > Since you are touching all of them, please keep the order
> > alphanumerically sorted by the HID.
>
> Sure.

With that addressed,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>


-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] serial: 8250: dw: enable using pdata with ACPI
Posted by Ilpo Järvinen 3 years, 10 months ago
On Sun, 19 Jun 2022, Marcin Wojtas wrote:

> Commit 3242fe805b52 ("serial: 8250: dw: Move the USR register to pdata")

That commit id is not correct, please fix.

Other than that,
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

> caused NULL-pointer dereference when booting with ACPI by unconditional
> usage of the recently added pdata.
> 
> In order to fix that and prevent similar issues in future, hook the
> default version of this structure in dw8250_acpi_match table.
> 
> Fixes: 3242fe805b52 ("serial: 8250: dw: Move the USR register to pdata")
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c | 24 ++++++++++----------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index f57bbd32ef11..3b79bd5c9c9d 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -773,18 +773,18 @@ static const struct of_device_id dw8250_of_match[] = {
>  MODULE_DEVICE_TABLE(of, dw8250_of_match);
>  
>  static const struct acpi_device_id dw8250_acpi_match[] = {
> -	{ "INT33C4", 0 },
> -	{ "INT33C5", 0 },
> -	{ "INT3434", 0 },
> -	{ "INT3435", 0 },
> -	{ "80860F0A", 0 },
> -	{ "8086228A", 0 },
> -	{ "APMC0D08", 0},
> -	{ "AMD0020", 0 },
> -	{ "AMDI0020", 0 },
> -	{ "AMDI0022", 0 },
> -	{ "BRCM2032", 0 },
> -	{ "HISI0031", 0 },
> +	{ "INT33C4", (kernel_ulong_t)&dw8250_dw_apb },
> +	{ "INT33C5", (kernel_ulong_t)&dw8250_dw_apb },
> +	{ "INT3434", (kernel_ulong_t)&dw8250_dw_apb },
> +	{ "INT3435", (kernel_ulong_t)&dw8250_dw_apb },
> +	{ "80860F0A", (kernel_ulong_t)&dw8250_dw_apb },
> +	{ "8086228A", (kernel_ulong_t)&dw8250_dw_apb },
> +	{ "APMC0D08", (kernel_ulong_t)&dw8250_dw_apb},
> +	{ "AMD0020", (kernel_ulong_t)&dw8250_dw_apb },
> +	{ "AMDI0020", (kernel_ulong_t)&dw8250_dw_apb },
> +	{ "AMDI0022", (kernel_ulong_t)&dw8250_dw_apb },
> +	{ "BRCM2032", (kernel_ulong_t)&dw8250_dw_apb },
> +	{ "HISI0031", (kernel_ulong_t)&dw8250_dw_apb },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(acpi, dw8250_acpi_match);
> 

-- 
 i.
Re: [PATCH] serial: 8250: dw: enable using pdata with ACPI
Posted by Marcin Wojtas 3 years, 10 months ago
Hi,

pon., 20 cze 2022 o 09:31 Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> napisał(a):
>
> On Sun, 19 Jun 2022, Marcin Wojtas wrote:
>
> > Commit 3242fe805b52 ("serial: 8250: dw: Move the USR register to pdata")
>
> That commit id is not correct, please fix.
>

Will do, thanks for spotting.

Best regards,
Marcin

> Other than that,
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>
> > caused NULL-pointer dereference when booting with ACPI by unconditional
> > usage of the recently added pdata.
> >
> > In order to fix that and prevent similar issues in future, hook the
> > default version of this structure in dw8250_acpi_match table.
> >
> > Fixes: 3242fe805b52 ("serial: 8250: dw: Move the USR register to pdata")
> > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > ---
> >  drivers/tty/serial/8250/8250_dw.c | 24 ++++++++++----------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> > index f57bbd32ef11..3b79bd5c9c9d 100644
> > --- a/drivers/tty/serial/8250/8250_dw.c
> > +++ b/drivers/tty/serial/8250/8250_dw.c
> > @@ -773,18 +773,18 @@ static const struct of_device_id dw8250_of_match[] = {
> >  MODULE_DEVICE_TABLE(of, dw8250_of_match);
> >
> >  static const struct acpi_device_id dw8250_acpi_match[] = {
> > -     { "INT33C4", 0 },
> > -     { "INT33C5", 0 },
> > -     { "INT3434", 0 },
> > -     { "INT3435", 0 },
> > -     { "80860F0A", 0 },
> > -     { "8086228A", 0 },
> > -     { "APMC0D08", 0},
> > -     { "AMD0020", 0 },
> > -     { "AMDI0020", 0 },
> > -     { "AMDI0022", 0 },
> > -     { "BRCM2032", 0 },
> > -     { "HISI0031", 0 },
> > +     { "INT33C4", (kernel_ulong_t)&dw8250_dw_apb },
> > +     { "INT33C5", (kernel_ulong_t)&dw8250_dw_apb },
> > +     { "INT3434", (kernel_ulong_t)&dw8250_dw_apb },
> > +     { "INT3435", (kernel_ulong_t)&dw8250_dw_apb },
> > +     { "80860F0A", (kernel_ulong_t)&dw8250_dw_apb },
> > +     { "8086228A", (kernel_ulong_t)&dw8250_dw_apb },
> > +     { "APMC0D08", (kernel_ulong_t)&dw8250_dw_apb},
> > +     { "AMD0020", (kernel_ulong_t)&dw8250_dw_apb },
> > +     { "AMDI0020", (kernel_ulong_t)&dw8250_dw_apb },
> > +     { "AMDI0022", (kernel_ulong_t)&dw8250_dw_apb },
> > +     { "BRCM2032", (kernel_ulong_t)&dw8250_dw_apb },
> > +     { "HISI0031", (kernel_ulong_t)&dw8250_dw_apb },
> >       { },
> >  };
> >  MODULE_DEVICE_TABLE(acpi, dw8250_acpi_match);
> >
>
> --
>  i.