Re: [PATCH 1/2 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Naveen,

Not a full review, but a few quick things I happened to notice:

On Tue, Jun 10, 2014 at 3:08 AM, Naveen Krishna Chatradhi
<ch.naveen@xxxxxxxxxxx> wrote:
> @@ -94,7 +93,6 @@ Example:
>                         spi-max-frequency = <10000>;
>
>                         controller-data {
> -                               cs-gpio = <&gpa2 5 1 0 3>;
>                                 samsung,spi-feedback-delay = <0>;

In a future patch (not this one!) it might make sense to also move
spi-feedback-delay out.


> +static struct s3c64xx_spi_csinfo *s3c64xx_get_cs_gpios(struct spi_device *spi)

I believe that you can make use of the fact that the SPI core already
got these GPIOs for you.  See of_spi_register_master().  Everything
you need ought to be in master->num_chipselect and master->cs_gpios.

Strangely, it doesn't look like any other drivers use this, so
possibly I'm confused...


> @@ -806,9 +815,14 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
>         struct s3c64xx_spi_info *sci;
>         int err;
>
> +       if (!spi->dev.of_node) {
> +               dev_err(&spi->dev, "device node not found\n");
> +               return -EINVAL;
> +       }

This seems incredibly broken.  You're saying that the SPI driver no
longer works without the device tree?  I don't think that's desirable,
but I suppose I could be wrong.

Also note that you still left an "if" test below for trying to deal
with the fact that there is no "of_node".


-Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux