From: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
To: Christian Hartmann <cornogle@googlemail.com>
Cc: patches@opensource.wolfsonmicro.com,
linux-acpi <linux-acpi@vger.kernel.org>
Subject: Re: Fwd: ASoC : WM5102 Audio Codec needs ACPI support and a baytrail machine driver
Date: Tue, 16 Jun 2015 15:47:33 +0100 [thread overview]
Message-ID: <20150616144732.GN32730@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <CAKor6Tdg5yHaLVrCrWFVuydVcYUUtYr=oSj+k37Yd9O64+tg6A@mail.gmail.com>
On Tue, Jun 16, 2015 at 03:10:42PM +0200, Christian Hartmann wrote:
> ---------- Forwarded message ----------
> From: Christian Hartmann <cornogle@googlemail.com>
> Date: 2015-06-16 14:27 GMT+02:00
> Subject: Re: ASoC : WM5102 Audio Codec needs ACPI support and a
> baytrail machine driver
> To: alsa-devel@alsa-project.org
>
>
> Hi again,
>
>
> some notes to the NULL pointer dereference error
> I have changed the function arizona_spi_probe in arizona-spi.c and
> have looked into the code where the NULL is coming...
>
> in drivers/spi/spi.c the function spi_get_device_id() returns NULL,
> cause the called function spi_match_id() does never match an id string
> (there is no modalias string available I think).
> With the id = NULL the line
> type = id->driver_data; will throw the described exception.
>
> Going on further, it seems that "id" has no such structure element of
> "->driver_data", or I cannot find it.
>
> Can someone please explain me in some simple words where the element
> driver_data is coming or will be set
> in THIS function of arizona_spi_probe()
>
> The id->driver_data is not working for the acpi case...
> This tablet does not have openfirmware (OF) by the way. The code seems
> to be optimized for the Openfirmware API, but not for the ACPI case.
>
Alas you are correct this driver has no built in support for ACPI
at the moment. So you are going to need to add that to get it
working on an ACPI system.
I don't really have any experience with ACPI so can't really be of
much assistance here. However, most of these device match functions
OF or the SPI built in system contain two fields an ID and some
driver data. As you can see in the code:
if (spi->dev.of_node)
type = arizona_of_get_type(&spi->dev);
else
type = id->driver_data;
We extract the type in suitable ways for the SPI built in match
and for the OF match up, I presume you need to do something
similar that is specific to the ACPI case. I would suggest here
looking up other ACPI drivers and seeing how they handle this
situation.
What I would expect to see is an extra else if in this statement
that does something specific to the ACPI case.
>
>
> this is the currently local changed code to test and find null pointer
> inarizona-spi.c.
>
> diff --git a/drivers/mfd/arizona-spi.c b/drivers/mfd/arizona-spi.c
> index 1e845f6..de48cf6 100644
> --- a/drivers/mfd/arizona-spi.c
> +++ b/drivers/mfd/arizona-spi.c
> @@ -25,16 +25,26 @@
>
> static int arizona_spi_probe(struct spi_device *spi)
> {
> - const struct spi_device_id *id = spi_get_device_id(spi);
> + const struct spi_device_id *id ;
> struct arizona *arizona;
> const struct regmap_config *regmap_config;
> unsigned long type;
> int ret;
>
> - if (spi->dev.of_node)
> - type = arizona_of_get_type(&spi->dev);
> - else
> + /*
> + id = spi_get_device_id(spi);
> +
> + if (!id)
> + return -EINVAL;
> + type = id->driver_data;
> + */
> + id = WM5102;
> +
> + if (id->driver_data)
> type = id->driver_data;
> + else
> + type = WM5102;
> +
This just defaults to WM5102 whenever we arn't using the spi
matching, which will probably get you running but clearly isn't
going to be a suitable solution.
>
> switch (type) {
> #ifdef CONFIG_MFD_WM5102
> @@ -90,11 +100,23 @@ static const struct spi_device_id arizona_spi_ids[] = {
> };
> MODULE_DEVICE_TABLE(spi, arizona_spi_ids);
>
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id wm5102_acpi_match[] = {
> + { "WM510205", WM5102 },
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(acpi, wm5102_acpi_match);
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id wm5102_acpi_match[] = {
> + { "WM510205", WM5102 },
The second entry here should be the driver_data, the ACPI id does
look a bit odd but I guess this is what our Windows guys are
using.
Thanks,
Charles
next prev parent reply other threads:[~2015-06-16 14:47 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-16 10:00 ASoC : WM5102 Audio Codec needs ACPI support and a baytrail machine driver Christian Hartmann
2015-06-16 12:27 ` Christian Hartmann
2015-06-16 13:10 ` Fwd: " Christian Hartmann
2015-06-16 14:47 ` Charles Keepax [this message]
2015-06-22 9:55 ` Christian Hartmann
2015-06-22 12:00 ` Charles Keepax
2015-06-24 8:48 ` Charles Keepax
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150616144732.GN32730@opensource.wolfsonmicro.com \
--to=ckeepax@opensource.wolfsonmicro.com \
--cc=cornogle@googlemail.com \
--cc=linux-acpi@vger.kernel.org \
--cc=patches@opensource.wolfsonmicro.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.