Hi, Some comments below. On Tue, May 8, 2012 at 3:04 PM, Thomas Abraham <thomas.abraham@xxxxxxxxxx> wrote: > Platform data is used to specify controller hardware specific information > such as the tx/rx fifo level mask and bit offset of rx fifo level. Such > information is not suitable to be supplied from device tree. Instead, > it can be moved into the driver data and removed from platform data. > > Cc: Jaswinder Singh <jaswinder.singh@xxxxxxxxxx> > Signed-off-by: Thomas Abraham <thomas.abraham@xxxxxxxxxx> > --- > drivers/spi/spi-s3c64xx.c | 180 ++++++++++++++++++++++++++++++++++++++------- > 1 files changed, 153 insertions(+), 27 deletions(-) > > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > index 6a3d51a..f6bc0e3 100644 > --- a/drivers/spi/spi-s3c64xx.c > +++ b/drivers/spi/spi-s3c64xx.c > @@ -171,6 +198,8 @@ struct s3c64xx_spi_driver_data { > struct s3c64xx_spi_dma_data rx_dma; > struct s3c64xx_spi_dma_data tx_dma; > struct samsung_dma_ops *ops; > + struct s3c64xx_spi_port_config *port_conf; > + unsigned port_id; unsigned int > @@ -942,6 +964,13 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int channel) > flush_fifo(sdd); > } > > +static inline struct s3c64xx_spi_port_config *s3c64xx_spi_get_port_config( > + struct platform_device *pdev) > +{ > + return (struct s3c64xx_spi_port_config *) > + platform_get_device_id(pdev)->driver_data; > +} > + > static int __init s3c64xx_spi_probe(struct platform_device *pdev) > { > struct resource *mem_res, *dmatx_res, *dmarx_res; > @@ -1000,6 +1029,7 @@ static int __init s3c64xx_spi_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, master); > > sdd = spi_master_get_devdata(master); > + sdd->port_conf = s3c64xx_spi_get_port_config(pdev); > sdd->master = master; > sdd->cntrlr_info = sci; > sdd->pdev = pdev; Single-use helper? Might as well open code it in this case. > @@ -1227,6 +1258,100 @@ static const struct dev_pm_ops s3c64xx_spi_pm = { > s3c64xx_spi_runtime_resume, NULL) > }; > > +#if defined(CONFIG_CPU_S3C2416) || defined(CONFIG_CPU_S3C2443) > +struct s3c64xx_spi_port_config s3c2443_spi_port_config = { > + .fifo_lvl_mask = { 0x7f }, > + .rx_lvl_offset = 13, > + .tx_st_done = 21, > + .high_speed = true, > +}; > +#define S3C2443_SPI_PORT_CONFIG ((kernel_ulong_t)&s3c2443_spi_port_config) > +#else > +#define S3C2443_SPI_PORT_CONFIG ((kernel_ulong_t)NULL) > +#endif Is it really worth it to do the ifdefs here for just 16 bytes of data per entry? The table itself below takes more space. [..] > +#ifdef CONFIG_ARCH_S5PV210 > +struct s3c64xx_spi_port_config s5pv210_spi_port_config = { > + .fifo_lvl_mask = { 0x1ff, 0x7F }, > + .rx_lvl_offset = 15, > + .tx_st_done = 25, > + .high_speed = 1, high_speed = true > +}; > +#define S5PV210_SPI_PORT_CONFIG ((kernel_ulong_t)&s5pv210_spi_port_config) > +#else > +#define S5PV210_SPI_PORT_CONFIG ((kernel_ulong_t)NULL) > +#endif /* CONFIG_ARCH_S5PV210 */ > + > +#ifdef CONFIG_ARCH_EXYNOS4 > +struct s3c64xx_spi_port_config exynos4_spi_port_config = { > + .fifo_lvl_mask = { 0x1ff, 0x7F, 0x7F }, > + .rx_lvl_offset = 15, > + .tx_st_done = 25, > + .high_speed = 1, high_speed = true -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html