[PATCH 02/10] spi: s3c64xx: move controller information into driver data
Olof Johansson
olof at lixom.net
Wed May 30 17:23:49 EST 2012
Hi,
Some comments below.
On Tue, May 8, 2012 at 3:04 PM, Thomas Abraham
<thomas.abraham at linaro.org> 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 at linaro.org>
> Signed-off-by: Thomas Abraham <thomas.abraham at linaro.org>
> ---
> 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
More information about the devicetree-discuss
mailing list