Re: [PATCH v2 2/2] ASoC: wm8985: add device-tree support

On 23 May 2016 at 10:31, Charles Keepax
[off-list ref] wrote:

On Mon, May 23, 2016 at 09:23:05AM +0200, Petr Kulhavy wrote:

quoted

On 20 May 2016 at 18:21, Charles Keepax
[off-list ref] wrote:

quoted

On Tue, May 17, 2016 at 02:49:49PM +0200, Petr Kulhavy wrote:

quoted

+static enum wm8985_type wm8985_data = WM8985;
+static enum wm8985_type wm8758_data = WM8758;
+
+static const struct of_device_id wm8985_of_match[] = {
+     { .compatible = "wlf,wm8985", .data = &wm8985_data},
+     { .compatible = "wlf,wm8758", .data = &wm8758_data},

You can probably just use (void *)WM8985 instead here.

I know this pretty safe with integers but can you do this with enums?
Doesn't it lead to undefined behaviour?

Hmm... its not uncommon that it is done in the kernel, but
that is rarely an indicator of if something is actually
technically valid or not. Casting from an integer to a pointer
is technically implementation defined, but we clearly rely on
gcc's implementation in the kernel. However, without actually
reading the spec in more detail I am unclear on if an enum counts
as an integer here, my suspicion is that it would since it
generally always implicitly casts to an int.

I guess the correct fix is that .data should probably be a
kernel_ulong_t in of_device_id. But if no one else objects I am
happy for you to leave your current code as is.

If I understand this Stack Overflow answer right:
http://stackoverflow.com/a/2331327
The enum should be casted to an integer of size <= size of a pointer
for the architecture.
So this should be more or less ok?
.data = (void*)(int)WM8985

Thanks
Petr
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html