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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Mon, May 23, 2016 at 09:23:05AM +0200, Petr Kulhavy wrote:
> On 20 May 2016 at 18:21, Charles Keepax
> <ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> > On Tue, May 17, 2016 at 02:49:49PM +0200, Petr Kulhavy wrote:
> >> +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.

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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux