Subject: Re: [PATCH v3 2/2] powerpc/mpc5121: add initial support for PDM360NG board

From: Anatolij Gustschin <[email-masked]>
Date: Tue, 27 Jul 2010 19:28:56 +0200

On Tue, 27 Jul 2010 10:58:33 -0600
Grant Likely <[email-masked]> wrote:
...
> >> > +               spi@11900 {
> >> > +                       compatible = "fsl,mpc5121-psc-spi", "fsl,mpc5121-psc";
> >> > +                       cell-index = <9>;
> >>
> >> Try to drop the cell-index properties.  They are almost always misused.
> >
> > Removing cell-index would require changing the spi driver's probe.
> > Currently cell-index is used to set spi bus number. What could be used
> > for bus enumeration instead? Is it okay to use part of the spi node
> > address? e.g. obtaining the offset 0x11900, masking out the unrelated
> > bits and shifting by 8 would deliver unique index 9 for PSC9 in SPI
> > mode. This would work for all 12 PSC SPI controllers of mpc5121.
> 
> Does the spi bus number really matter?  The device tree context gives
> you a firm association between spi masters and devices which doesn't
> require assigning a specific bus number.  The core spi code can
> dynamically assign a bus number for the bus by setting bus_num to -1.

The bus number is used in the mpc5121 psc spi driver to obtain correct
clock for PSC in question (0 to 11) and to enable the PSC clock at probe
time. Therefore using dynamically assigned bus number would require another
change to the spi driver.

...
> > This requires fixing the mpc5121 psc spi driver to create spi child
> > nodes of the spi master node. I have already send the appropriate
> > patch to spi-devel list, but it is not the right approach to call
> > of_register_spi_devices() in each driver.
> 
> It's not wrong; but it isn't ideal either.
> 
> > Do you plan to fix it in
> > core spi code in v2.6.36?
> 
> Apparently I no longer have to because you've gone ahead and done it
> for me anyway.  :-)  I'll take a look at that patch and send you my
> comments.

Okay, thanks,
Anatolij