RE: [PATCH v2 4/5] spi: Add OF binding support for SPI busses

[Chen Gong]

> -----Original Message-----
> From: linuxppc-dev-bounces+b11801=freescale.com@xxxxxxxxxx 
> [mailto:linuxppc-dev-bounces+b11801=freescale.com@xxxxxxxxxx] 
> On Behalf Of Grant Likely
> Sent: 2008?7?3? 9:03
> To: linuxppc-dev@xxxxxxxxxx; 
> spi-devel-general@xxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: david-b@xxxxxxxxxxx; fabrizio.garetto@xxxxxxxxx
> Subject: [PATCH v2 4/5] spi: Add OF binding support for SPI busses
> 
> From: Grant Likely <grant.likely@xxxxxxxxxxxx>
> 
> This patch adds support for populating an SPI bus based on data in the
> OF device tree.  This is useful for powerpc platforms which use the
> device tree instead of discrete code for describing platform layout.
> 
> Signed-off-by: Grant Likely <grant.likely@xxxxxxxxxxxx>
> ---
> 
>  drivers/of/Kconfig     |    6 +++
>  drivers/of/Makefile    |    1 +
>  drivers/of/of_spi.c    |   88 
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_spi.h |   18 ++++++++++
>  4 files changed, 113 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index 3a7a11a..edd6e92 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -13,3 +13,9 @@ config OF_I2C
>  	depends on PPC_OF && I2C
>  	help
>  	  OpenFirmware I2C accessors
> +
> +config OF_SPI
> +	def_tristate SPI
> +	depends on OF && PPC_OF && SPI
> +	help
> +	  OpenFirmware SPI accessors
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index 548772e..4c3c6f8 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -2,3 +2,4 @@ obj-y = base.o
>  obj-$(CONFIG_OF_DEVICE) += device.o platform.o
>  obj-$(CONFIG_OF_GPIO)   += gpio.o
>  obj-$(CONFIG_OF_I2C)	+= of_i2c.o
> +obj-$(CONFIG_OF_SPI)	+= of_spi.o
> diff --git a/drivers/of/of_spi.c b/drivers/of/of_spi.c
> new file mode 100644
> index 0000000..ed0c807
> --- /dev/null
> +++ b/drivers/of/of_spi.c
> @@ -0,0 +1,88 @@
> +/*
> + * SPI OF support routines
> + * Copyright (C) 2008 Secret Lab Technologies Ltd.
> + *
> + * Support routines for deriving SPI device attachments from 
> the device
> + * tree.
> + */
> +
> +#include <linux/of.h>
> +#include <linux/device.h>
> +#include <linux/spi/spi.h>
> +#include <linux/of_spi.h>
> +
> +/**
> + * of_register_spi_devices - Register child devices onto the SPI bus
> + * @master:	Pointer to spi_master device
> + * @np:		parent node of SPI device nodes
> + *
> + * Registers an spi_device for each child node of 'np' which 
> has a 'reg'
> + * property.
> + */
> +void of_register_spi_devices(struct spi_master *master, 
> struct device_node *np)
> +{
> +	struct spi_device *spi;
> +	struct device_node *nc;
> +	const u32 *prop;
> +	const char *sprop;
> +	int rc;
> +	int len;
> +
> +	for_each_child_of_node(np, nc) {
> +		/* Alloc an spi_device */
> +		spi = spi_alloc_device(master);
> +		if (!spi) {
> +			dev_err(&master->dev, "spi_device alloc 
> error for %s\n",
> +				nc->full_name);
> +			continue;
> +		}
> +
> +		/* Device address */
> +		prop = of_get_property(nc, "reg", &len);
> +		if (!prop || len < sizeof(*prop)) {
> +			dev_err(&master->dev, "%s has no 'reg' 
> property\n",
> +				nc->full_name);
> +			continue;
> +		}
> +		spi->chip_select = *prop;
> +
> +		/* Mode (clock phase/polarity/etc.) */
> +		if (of_find_property(nc, "spi,cpha", NULL))
> +			spi->mode |= SPI_CPHA;
> +		if (of_find_property(nc, "spi,cpol", NULL))
> +			spi->mode |= SPI_CPOL;

so becuase in function spi_alloc_deive, spi is allocated by kzalloc,
how about writing as follows:
		/* Mode (clock phase/polarity/etc.) */
		prop = of_get_property(nc, "spi,cpha", NULL))
                        if (prop)
			spi->mode |= *prop;
		prop = of_get_property(nc, "spi,cpol", NULL))
                        if (prop)
			spi->mode |= *prop;

> +
> +		/* Device speed */
> +		prop = of_get_property(nc, "max-speed", &len);
> +		if (!prop || len < sizeof(*prop)) {
> +			dev_err(&master->dev, "%s has no 
> 'max-speed' property\n",
> +				nc->full_name);
> +			continue;
> +		}
> +		spi->max_speed_hz = *prop;
> +
> +		/* IRQ */
> +		spi->irq = irq_of_parse_and_map(nc, 0);
> +
> +		/* Select device driver */
> +		sprop = of_get_property(nc, "linux,modalias", &len);
> +		if (sprop && len > 0)
> +			strncpy(spi->modalias, sprop, KOBJ_NAME_LEN);
> +		else
> +			strncpy(spi->modalias, "spidev", KOBJ_NAME_LEN);
> +
 how about writing as follows:

		if (sprop && len > 0)
			strncpy(spi->modalias, sprop, KOBJ_NAME_LEN -
1);
		else
			strncpy(spi->modalias, "spidev", KOBJ_NAME_LEN -
1);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/