| Message ID | 20180910115935.163121-4-lkundrak@v3.sk (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Make SPI work on DT MMP2 | expand |
Lubomir Rintel <lkundrak@v3.sk> 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 <jarkko.nikula@linux.intel.com> - Mika Westerberg <mika.westerberg@linux.intel.com> Even if they are Intel, I think they have worked a lot on this driver for Intel platforms. Cheers.
On Fri, 2018-09-21 at 22:34 +0200, Robert Jarzmik wrote: > Lubomir Rintel <lkundrak@v3.sk> 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; 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 <jarkko.nikula@linux.intel.com> > - Mika Westerberg <mika.westerberg@linux.intel.com> > > Even if they are Intel, I think they have worked a lot on this driver > for Intel > platforms. Will do. > Cheers. Thanks, Lubo
On Fri, 2018-09-21 at 22:34 +0200, Robert Jarzmik wrote: > Lubomir Rintel <lkundrak@v3.sk> 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 <jarkko.nikula@linux.intel.com> > - Mika Westerberg <mika.westerberg@linux.intel.com> 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@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. >
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c index 14f4ea59caff..f674541675bb 100644 --- a/drivers/spi/spi-pxa2xx.c +++ b/drivers/spi/spi-pxa2xx.c @@ -1429,7 +1429,7 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev) struct resource *res; const struct acpi_device_id *adev_id = NULL; const struct pci_device_id *pcidev_id = NULL; - int type; + enum pxa_ssp_type type; adev = ACPI_COMPANION(&pdev->dev); @@ -1443,9 +1443,9 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev) return NULL; if (adev_id) - type = (int)adev_id->driver_data; + type = (enum pxa_ssp_type)adev_id->driver_data; else if (pcidev_id) - type = (int)pcidev_id->driver_data; + type = (enum pxa_ssp_type)pcidev_id->driver_data; else return NULL; diff --git a/include/linux/pxa2xx_ssp.h b/include/linux/pxa2xx_ssp.h index 13b4244d44c1..262e1f318836 100644 --- a/include/linux/pxa2xx_ssp.h +++ b/include/linux/pxa2xx_ssp.h @@ -217,7 +217,7 @@ struct ssp_device { const char *label; int port_id; - int type; + enum pxa_ssp_type type; int use_count; int irq;
That seems to be the correct type. Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> --- drivers/spi/spi-pxa2xx.c | 6 +++--- include/linux/pxa2xx_ssp.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)