From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-fx0-f11.google.com (mail-fx0-f11.google.com [209.85.220.11]) by ozlabs.org (Postfix) with ESMTP id 84144DDDB4 for ; Sat, 14 Feb 2009 02:55:09 +1100 (EST) Received: by fxm4 with SMTP id 4so5776558fxm.9 for ; Fri, 13 Feb 2009 07:55:07 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: Date: Fri, 13 Feb 2009 16:52:49 +0100 Message-ID: Subject: Re: Chipselect in SPI binding with mpc5200-psc-spi From: Henk Stegeman To: linuxppc-dev@ozlabs.org Content-Type: multipart/alternative; boundary=001636457c722e6cba0462ced4e4 List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , --001636457c722e6cba0462ced4e4 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Thanks Greg, I'm having success more success with the driver now, The device gets nicely probed and the probe function gets the properties from the device tree, The following patch from Anton Vorontsov: http://kerneltrap.org/mailarchive/linux-kernel/2008/5/23/1922744 Was very useful as example for adding the of support to my 'own' spi slave driver. (Thanks Anton,) Henk. On Fri, Feb 13, 2009 at 4:19 PM, Grant Likely wrote: > On Fri, Feb 13, 2009 at 3:40 AM, Henk Stegeman > wrote: > > I'm busy adding support for slave deviced behind mpc52xx-psc-spi. > > One complication I have is that my SPI slave device has an interrupt > output > > to the CPU. > > My idea is to add it as a gpios property in the slave device's > > configuration: > > > > spi@2400 { // PSC3 (SPI IF to the IO-controller ) > > device_type = "spi"; > > #address-cells = <1>; > > #size-cells = <0>; > > compatible = "fsl,mpc5200-psc-spi","fsl,mpc5200b-psc-spi"; > > cell-index = <2>; > > reg = <0x2400 0x100>; > > interrupts = <2 3 0>; > > interrupt-parent = <&mpc5200_pic>; > > gpios = <&gpt4 0 0>; > > > > io-controller@0 { > > compatible = "microkey,smc4000io"; > > spi-max-frequency = <1000000>; > > reg = <0>; > > // gpios: first is IRQ to cpu > > gpios = <&gpt6 0 0>; > > }; > > There is a better way to do this, and driver support for it is > currently merged into Ben Herrenschmidt's -next tree. > > Do this instead: > io-controller@0 { > compatible = "microkey,smc4000io"; > spi-max-frequency = <1000000>; > reg = <0>; > interrupt-controller = < &gpt6 >; // Use GPT6 as > the IRQ controller > interrupts = < 1 >; // And make it rising edge. > }; > > Then add these two properties to the GPT node: > > interrupt-controller; > #interrupt-cells = <1>; > > Then you can use normal irq_of_parse_and_map() to set up your handler. > > > How should I then register my spi slave driver? My smc4000io_probe > function > > gets called correctly by of_spi support but when I register as follows: > > > > static struct spi_driver smc4000io_driver = { > > .driver = { > > .name = "smc4000io", > > .bus = &spi_bus_type, > > .owner = THIS_MODULE, > > }, > > .probe = smc4000io_probe, > > .remove = __devexit_p(smc4000io_remove), > > }; > > > > static int __init smc4000io_init(void) > > { > > return spi_register_driver(&smc4000io_driver); > > } > > > > static void __exit smc4000io_exit(void) > > { > > spi_unregister_driver(&smc4000io_driver); > > } > > > > module_init(smc4000io_init); > > Yes, this is right. The psc_spi driver automatically registers all > spi children that it finds in the device tree onto the SPI bus. > Therefore registering an spi_driver() is the right thing to do. > > > But when I do: > > > > static struct of_platform_driver smc4000_spi_of_driver = { > > .name = "smc4000io", > > .match_table = smc4000io_of_match, > > .probe = smc4000io_of_probe, > > .remove = __devexit_p(smc4000io_of_remove), > > }; > > > > static int __init smc4000io_init(void) > > { > > return of_register_platform_driver(&smc4000_spi_of_driver); > > } > > module_init(smc4000io_init); > > > > Then my smc4000io_of_probe function never gets called. > > Correct. of_platform_driver isn't useful in this case because the > device cannot exist independently of the SPI bus. Plus an > of_platform_device doesn't provide any information about the SPI bus > itself. > > g. > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd. > --001636457c722e6cba0462ced4e4 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Thanks Greg,

I'm having success more success with the driver now= ,

The device gets nicely probed and the probe function gets the prop= erties from the device tree,

The following patch from Anton Vorontso= v:

http://kerneltrap.org/mailarchive/linux-kernel/2008/5/23/1922744<= br>
Was very useful  as example for adding the of support to my = 9;own' spi slave driver.

(Thanks Anton,)

Henk.

On Fri, = Feb 13, 2009 at 4:19 PM, Grant Likely <grant.likely@secretlab.ca> wrot= e:
On Fri, Feb 13, 2009 at 3:40 AM, Henk Stegeman <henk.stegeman@gmail.com> wrote:
> I'm busy adding support for slave deviced behind mpc52xx-psc-spi.<= br> > One complication I have is that my SPI slave device has an interrupt o= utput
> to the CPU.
> My idea is to add it as a gpios property in the slave device's
> configuration:
>
>         spi@2400 {        // P= SC3 (SPI IF to the IO-controller )
>             device_type =3D "spi&qu= ot;;
>             #address-cells =3D <1>= ;
>             #size-cells =3D <0>; >             compatible =3D "fsl,mpc= 5200-psc-spi","fsl,mpc5200b-psc-spi";
>             cell-index =3D <2>; >             reg =3D <0x2400 0x100>= ;
>             interrupts =3D <2 3 0>= ;
>             interrupt-parent =3D <&am= p;mpc5200_pic>;
>             gpios =3D <&gpt4 0 0&= gt;;
>
>             io-controller@0 {
>                 compatible =3D= "microkey,smc4000io";
>                 spi-max-freque= ncy =3D <1000000>;
>                 reg =3D <0&= gt;;
>                 // gpios: firs= t is IRQ to cpu
>                 gpios =3D <= &gpt6 0 0>;
>             };

There is a better way to do this, and driver support for it is
currently merged into Ben Herrenschmidt's -next tree.

Do this instead:
       io-controller@0 {
               compatible =3D &quo= t;microkey,smc4000io";
               spi-max-frequency = =3D <1000000>;
               reg =3D <0>;<= br>
               interrupt-con= troller =3D < &gpt6 >;     // Use GPT6 as
the IRQ controller
               interrupts =3D <= 1 >;    // And make it rising edge.
       };

Then add these two properties to the GPT node:

       interrupt-controller;
       #interrupt-cells =3D <1>;

Then you can use normal irq_of_parse_and_map() to set up your handler.

> How should I then register my spi slave driver? My smc4000io_probe fun= ction
> gets called correctly by of_spi support but when I register as follows= :
>
> static struct spi_driver smc4000io_driver =3D {
>     .driver =3D {
>         .name    =3D "smc4000io&quo= t;,
>         .bus    =3D &spi_bus_type, >         .owner    =3D THIS_MODULE,
>     },
>     .probe        =3D smc4000io_probe, >     .remove        =3D __devexit_p(smc40= 00io_remove),
> };
>
> static int __init smc4000io_init(void)
> {
>     return spi_register_driver(&smc4000io_driver);
> }
>
> static void __exit smc4000io_exit(void)
> {
>     spi_unregister_driver(&smc4000io_driver);
> }
>
> module_init(smc4000io_init);

Yes, this is right.  The psc_spi driver automatically registers = all
spi children that it finds in the device tree onto the SPI bus.
Therefore registering an spi_driver() is the right thing to do.

> But when I do:
>
> static struct of_platform_driver smc4000_spi_of_driver =3D {
>     .name =3D "smc4000io",
>     .match_table =3D smc4000io_of_match,
>     .probe =3D smc4000io_of_probe,
>     .remove        =3D __devexit_p(smc40= 00io_of_remove),
> };
>
> static int __init smc4000io_init(void)
> {
>     return of_register_platform_driver(&smc4000_spi_of_d= river);
> }
> module_init(smc4000io_init);
>
> Then my smc4000io_of_probe function never gets called.

Correct.  of_platform_driver isn't useful in this case becau= se the
device cannot exist independently of the SPI bus.  Plus an
of_platform_device doesn't provide any information about the SPI bus itself.

g.

--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

--001636457c722e6cba0462ced4e4--