Re: [PATCH 3/5] spi: pxa2xx: use an enum for type

3 messages, 2 authors, 2018-10-10 · open the first message on its own page

Re: [PATCH 3/5] spi: pxa2xx: use an enum for type

From: Robert Jarzmik <robert.jarzmik@free.fr>
Date: 2018-09-21 20:34:11

Lubomir Rintel [off-list ref] writes:
That seems to be the correct type.
Okay, but what happens here when adev_id->driver_data is a value out of enum
range ? Does the following assignment make sense ?
+		type = (enum pxa_ssp_type)adev_id->driver_data;
As a side note, could you join for the next throw to the review :
 - Jarkko Nikula [off-list ref]
 - Mika Westerberg [off-list ref]

Even if they are Intel, I think they have worked a lot on this driver for Intel
platforms.

Cheers.

-- 
Robert

Re: [PATCH 3/5] spi: pxa2xx: use an enum for type

From: Lubomir Rintel <lkundrak@v3.sk>
Date: 2018-10-09 16:43:52

On Fri, 2018-09-21 at 22:34 +0200, Robert Jarzmik wrote:
Lubomir Rintel [off-list ref] writes:
quoted
That seems to be the correct type.
Okay, but what happens here when adev_id->driver_data is a value out
of enum
range ? Does the following assignment make sense ?
quoted
+		type = (enum pxa_ssp_type)adev_id->driver_data;
No change in behavior. In standard C the enum type is compatible with
an integer type (char or larger), thus type would just hold a value
outside the rank of an enum.

But why would that happen? What we can get here is just the constants
from pxa2xx_spi_acpi_match[], all of them of enum pxa_ssp_type type.
As a side note, could you join for the next throw to the review :
 - Jarkko Nikula [off-list ref]
 - Mika Westerberg [off-list ref]

Even if they are Intel, I think they have worked a lot on this driver
for Intel
platforms.
Will do.
Cheers.
Thanks,
Lubo

Re: [PATCH 3/5] spi: pxa2xx: use an enum for type

From: Lubomir Rintel <lkundrak@v3.sk>
Date: 2018-10-10 17:27:06

On Fri, 2018-09-21 at 22:34 +0200, Robert Jarzmik wrote:
Lubomir Rintel [off-list ref] writes:
quoted
That seems to be the correct type.
Okay, but what happens here when adev_id->driver_data is a value out
of enum
range ? Does the following assignment make sense ?
quoted
+		type = (enum pxa_ssp_type)adev_id->driver_data;
As a side note, could you join for the next throw to the review :
 - Jarkko Nikula [off-list ref]
 - Mika Westerberg [off-list ref]
Argh, of course I forgot. My apologies.

Jarkko & Mika, I'm wondering if you could take a look at the [1] thread
and tell me what you think. (I can forward it to you via e-mail too, if
you're not subscribed to lists it has been Cc'd to, if that would be
more convenient).

[1] [PATCH 0/11] spi: pxa2xx: add DT and slave mode support"
    https://lore.kernel.org/lkml/20181010170936.316862-1-lkundrak at v3.sk/T/#t

Thanks
Lubo
Even if they are Intel, I think they have worked a lot on this driver
for Intel
platforms.

Cheers.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help