| Message ID | 1394789757-40732-2-git-send-email-B48286@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 14, 2014 at 05:35:57PM +0800, Hou Zhiqiang wrote: > Get the spi_master's bus_num from DTS to make the spi_master's name > static. So "mtdparts=spi.bus_num.chip_select:..." in cmdline can be > used to asign mtd partions of spi flash. If we are going to do this it shouldn't be device specific (it should be done in the framework since nothing is specific to the controller there) but I'm not convinced that we should be doing it - this is all very Linux specific. The DT already has support for specifying flash layouts, can't those be used (for example via chosen if they're not fixed for the board)? Or if it's just picking the correct filesystem then UUIDs and labels are the standard way to do things.
> -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: Saturday, March 15, 2014 3:19 AM > To: Hou Zhiqiang-B48286 > Cc: linux-spi@vger.kernel.org; devicetree@vger.kernel.org; > rob.herring@calxeda.com; pawel.moll@arm.com; mark.rutland@arm.com; > ijc+devicetree@hellion.org.uk; galak@codeaurora.org; > grant.likely@secretlab.ca; Wood Scott-B07421; Hu Mingkai-B21284 > Subject: Re: [PATCH 2/2] spi/fsl-lib: Get the SPI controller bus number > from DTS > > On Fri, Mar 14, 2014 at 05:35:57PM +0800, Hou Zhiqiang wrote: > > Get the spi_master's bus_num from DTS to make the spi_master's name > > static. So "mtdparts=spi.bus_num.chip_select:..." in cmdline can be > > used to asign mtd partions of spi flash. > > If we are going to do this it shouldn't be device specific (it should be > done in the framework since nothing is specific to the controller there) > but I'm not convinced that we should be doing it - this is all very Linux > specific. This patch just assign a bus number to the controller. It is driver's responsibility to distribute a bus number to spi_master and the definition of bus_num is used to distinguish controllers. So, it is specific for the controller and doesn't affect the framework. > > The DT already has support for specifying flash layouts, can't those be > used (for example via chosen if they're not fixed for the board)? Or if > it's just picking the correct filesystem then UUIDs and labels are the > standard way to do things. The DT specifying flash layouts is ok. There is another way to make the flash layouts using command line, but it is not safe because of the dynamic bus_num. It is not the reason that the way of DT is supported flash layouts, to live the other way unsafe, right? Thanks, Zhiqiang -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 17, 2014 at 05:11:11AM +0000, B48286@freescale.com wrote: > > On Fri, Mar 14, 2014 at 05:35:57PM +0800, Hou Zhiqiang wrote: > > > Get the spi_master's bus_num from DTS to make the spi_master's name > > > static. So "mtdparts=spi.bus_num.chip_select:..." in cmdline can be > > > used to asign mtd partions of spi flash. > > If we are going to do this it shouldn't be device specific (it should be > > done in the framework since nothing is specific to the controller there) > > but I'm not convinced that we should be doing it - this is all very Linux > > specific. > This patch just assign a bus number to the controller. It is driver's > responsibility to distribute a bus number to spi_master and the definition > of bus_num is used to distinguish controllers. So, it is specific for the > controller and doesn't affect the framework. You are adding a property to specify a bus number. There is no reason why such a property should be specific to a single controller, every controller in every system has a bus number assigned to it so if we have a way to specify one controller via the device tree we should have a way to specify it for all. > > The DT already has support for specifying flash layouts, can't those be > > used (for example via chosen if they're not fixed for the board)? Or if > > it's just picking the correct filesystem then UUIDs and labels are the > > standard way to do things. > The DT specifying flash layouts is ok. There is another way to make the > flash layouts using command line, but it is not safe because of the dynamic > bus_num. It is not the reason that the way of DT is supported flash layouts, > to live the other way unsafe, right? This sounds to me like we need a better way of talking about flash device names on the Linux command line rather than a way to fix the bus number - for example, being able to refer to them using a fixed property like the physical address. Being able to refer to devices via an alias assigned in the DT would also be useful (and more readable), I think there may already be a mechanism for doing that which would need to be plumbed in but I'm not 100% sure.
> -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: Monday, March 17, 2014 9:01 PM > To: Hou Zhiqiang-B48286 > Cc: linux-spi@vger.kernel.org; devicetree@vger.kernel.org; > rob.herring@calxeda.com; pawel.moll@arm.com; mark.rutland@arm.com; > ijc+devicetree@hellion.org.uk; galak@codeaurora.org; > grant.likely@secretlab.ca; Wood Scott-B07421; Hu Mingkai-B21284 > Subject: Re: [PATCH 2/2] spi/fsl-lib: Get the SPI controller bus number > from DTS > > On Mon, Mar 17, 2014 at 05:11:11AM +0000, B48286@freescale.com wrote: > > > On Fri, Mar 14, 2014 at 05:35:57PM +0800, Hou Zhiqiang wrote: > > > > Get the spi_master's bus_num from DTS to make the spi_master's > > > > name static. So "mtdparts=spi.bus_num.chip_select:..." in cmdline > > > > can be used to asign mtd partions of spi flash. > > > > If we are going to do this it shouldn't be device specific (it > > > should be done in the framework since nothing is specific to the > > > controller there) but I'm not convinced that we should be doing it - > > > this is all very Linux specific. > > > This patch just assign a bus number to the controller. It is driver's > > responsibility to distribute a bus number to spi_master and the > > definition of bus_num is used to distinguish controllers. So, it is > > specific for the controller and doesn't affect the framework. > > You are adding a property to specify a bus number. There is no reason > why such a property should be specific to a single controller, every > controller in every system has a bus number assigned to it so if we have > a way to specify one controller via the device tree we should have a way > to specify it for all. > The reason to specify a bus number to a single controller is to avoid allocating one dynamically, so we know the bus number before booting up the kernel. If there are several controllers, you should add this property to all of them. But it is unnecessary to add this property, if you do not care about the bus number, and it will be allocated dynamically. > > > The DT already has support for specifying flash layouts, can't those > > > be used (for example via chosen if they're not fixed for the board)? > > > Or if it's just picking the correct filesystem then UUIDs and labels > > > are the standard way to do things. > > > The DT specifying flash layouts is ok. There is another way to make > > the flash layouts using command line, but it is not safe because of > > the dynamic bus_num. It is not the reason that the way of DT is > > supported flash layouts, to live the other way unsafe, right? > > This sounds to me like we need a better way of talking about flash device > names on the Linux command line rather than a way to fix the bus number - > for example, being able to refer to them using a fixed property like the > physical address. Being able to refer to devices via an alias assigned > in the DT would also be useful (and more readable), I think there may > already be a mechanism for doing that which would need to be plumbed in > but I'm not 100% sure. The bus number is the variable designed to distinguish one spi controller from others. Why spi controller's physical address must be use instead of bus number? -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 18, 2014 at 8:40 AM, B48286@freescale.com <B48286@freescale.com> wrote: >> > > The DT already has support for specifying flash layouts, can't those >> > > be used (for example via chosen if they're not fixed for the board)? >> > > Or if it's just picking the correct filesystem then UUIDs and labels >> > > are the standard way to do things. >> >> > The DT specifying flash layouts is ok. There is another way to make >> > the flash layouts using command line, but it is not safe because of >> > the dynamic bus_num. It is not the reason that the way of DT is >> > supported flash layouts, to live the other way unsafe, right? >> >> This sounds to me like we need a better way of talking about flash device >> names on the Linux command line rather than a way to fix the bus number - >> for example, being able to refer to them using a fixed property like the >> physical address. Being able to refer to devices via an alias assigned >> in the DT would also be useful (and more readable), I think there may >> already be a mechanism for doing that which would need to be plumbed in >> but I'm not 100% sure. > > The bus number is the variable designed to distinguish one spi controller > from others. Why spi controller's physical address must be use instead of > bus number? Because the bus number is dynamic, while the physical address doesn't change, so it can be used to uniqely identify the device before booting the kernel. Cfr. "spi1" vs. "e6e20000.spi". Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogZ2VlcnQudXl0dGVyaG9l dmVuQGdtYWlsLmNvbSBbbWFpbHRvOmdlZXJ0LnV5dHRlcmhvZXZlbkBnbWFpbC5jb21dDQo+IE9u IEJlaGFsZiBPZiBHZWVydCBVeXR0ZXJob2V2ZW4NCj4gU2VudDogVHVlc2RheSwgTWFyY2ggMTgs IDIwMTQgNDo1NiBQTQ0KPiBUbzogSG91IFpoaXFpYW5nLUI0ODI4Ng0KPiBDYzogTWFyayBCcm93 bjsgbGludXgtc3BpQHZnZXIua2VybmVsLm9yZzsgZGV2aWNldHJlZUB2Z2VyLmtlcm5lbC5vcmc7 DQo+IHJvYi5oZXJyaW5nQGNhbHhlZGEuY29tOyBwYXdlbC5tb2xsQGFybS5jb207IG1hcmsucnV0 bGFuZEBhcm0uY29tOw0KPiBpamMrZGV2aWNldHJlZUBoZWxsaW9uLm9yZy51azsgZ2FsYWtAY29k ZWF1cm9yYS5vcmc7DQo+IGdyYW50Lmxpa2VseUBzZWNyZXRsYWIuY2E7IFdvb2QgU2NvdHQtQjA3 NDIxOyBIdSBNaW5na2FpLUIyMTI4NA0KPiBTdWJqZWN0OiBSZTogW1BBVENIIDIvMl0gc3BpL2Zz bC1saWI6IEdldCB0aGUgU1BJIGNvbnRyb2xsZXIgYnVzIG51bWJlcg0KPiBmcm9tIERUUw0KPiAN Cj4gT24gVHVlLCBNYXIgMTgsIDIwMTQgYXQgODo0MCBBTSwgQjQ4Mjg2QGZyZWVzY2FsZS5jb20N Cj4gPEI0ODI4NkBmcmVlc2NhbGUuY29tPiB3cm90ZToNCj4gPj4gPiA+IFRoZSBEVCBhbHJlYWR5 IGhhcyBzdXBwb3J0IGZvciBzcGVjaWZ5aW5nIGZsYXNoIGxheW91dHMsIGNhbid0DQo+ID4+ID4g PiB0aG9zZSBiZSB1c2VkIChmb3IgZXhhbXBsZSB2aWEgY2hvc2VuIGlmIHRoZXkncmUgbm90IGZp eGVkIGZvciB0aGUNCj4gYm9hcmQpPw0KPiA+PiA+ID4gT3IgaWYgaXQncyBqdXN0IHBpY2tpbmcg dGhlIGNvcnJlY3QgZmlsZXN5c3RlbSB0aGVuIFVVSURzIGFuZA0KPiA+PiA+ID4gbGFiZWxzIGFy ZSB0aGUgc3RhbmRhcmQgd2F5IHRvIGRvIHRoaW5ncy4NCj4gPj4NCj4gPj4gPiBUaGUgRFQgc3Bl Y2lmeWluZyBmbGFzaCBsYXlvdXRzIGlzIG9rLiBUaGVyZSBpcyBhbm90aGVyIHdheSB0byBtYWtl DQo+ID4+ID4gdGhlIGZsYXNoIGxheW91dHMgdXNpbmcgY29tbWFuZCBsaW5lLCBidXQgaXQgaXMg bm90IHNhZmUgYmVjYXVzZSBvZg0KPiA+PiA+IHRoZSBkeW5hbWljIGJ1c19udW0uIEl0IGlzIG5v dCB0aGUgcmVhc29uIHRoYXQgdGhlIHdheSBvZiBEVCBpcw0KPiA+PiA+IHN1cHBvcnRlZCBmbGFz aCBsYXlvdXRzLCB0byBsaXZlIHRoZSBvdGhlciB3YXkgdW5zYWZlLCByaWdodD8NCj4gPj4NCj4g Pj4gVGhpcyBzb3VuZHMgdG8gbWUgbGlrZSB3ZSBuZWVkIGEgYmV0dGVyIHdheSBvZiB0YWxraW5n IGFib3V0IGZsYXNoDQo+ID4+IGRldmljZSBuYW1lcyBvbiB0aGUgTGludXggY29tbWFuZCBsaW5l IHJhdGhlciB0aGFuIGEgd2F5IHRvIGZpeCB0aGUNCj4gPj4gYnVzIG51bWJlciAtIGZvciBleGFt cGxlLCBiZWluZyBhYmxlIHRvIHJlZmVyIHRvIHRoZW0gdXNpbmcgYSBmaXhlZA0KPiA+PiBwcm9w ZXJ0eSBsaWtlIHRoZSBwaHlzaWNhbCBhZGRyZXNzLiAgQmVpbmcgYWJsZSB0byByZWZlciB0byBk ZXZpY2VzDQo+ID4+IHZpYSBhbiBhbGlhcyBhc3NpZ25lZCBpbiB0aGUgRFQgd291bGQgYWxzbyBi ZSB1c2VmdWwgKGFuZCBtb3JlDQo+ID4+IHJlYWRhYmxlKSwgSSB0aGluayB0aGVyZSBtYXkgYWxy ZWFkeSBiZSBhIG1lY2hhbmlzbSBmb3IgZG9pbmcgdGhhdA0KPiA+PiB3aGljaCB3b3VsZCBuZWVk IHRvIGJlIHBsdW1iZWQgaW4gYnV0IEknbSBub3QgMTAwJSBzdXJlLg0KPiA+DQo+ID4gVGhlIGJ1 cyBudW1iZXIgaXMgdGhlIHZhcmlhYmxlIGRlc2lnbmVkIHRvIGRpc3Rpbmd1aXNoIG9uZSBzcGkN Cj4gPiBjb250cm9sbGVyIGZyb20gb3RoZXJzLiBXaHkgc3BpIGNvbnRyb2xsZXIncyBwaHlzaWNh bCBhZGRyZXNzIG11c3QgYmUNCj4gPiB1c2UgaW5zdGVhZCBvZiBidXMgbnVtYmVyPw0KPiANCj4g QmVjYXVzZSB0aGUgYnVzIG51bWJlciBpcyBkeW5hbWljLCB3aGlsZSB0aGUgcGh5c2ljYWwgYWRk cmVzcyBkb2Vzbid0DQo+IGNoYW5nZSwgc28gaXQgY2FuIGJlIHVzZWQgdG8gdW5pcWVseSBpZGVu dGlmeSB0aGUgZGV2aWNlIGJlZm9yZSBib290aW5nDQo+IHRoZSBrZXJuZWwuDQo+IENmci4gInNw aTEiIHZzLiAiZTZlMjAwMDAuc3BpIi4NCj4gDQoNClRoZSBwcmVjb25kaXRpb24gb2YgZHluYW1p YyBidXMgbnVtYmVyIGlzIGluaXRpYWwgaXQgd2l0aCAtMSBpbiB0aGUgY29udHJvbGxlcg0KZHJp dmVyLiBCdXQgbm93IEkgbmVlZCBhIHJlYXNvbmFibGUgYnVzIG51bWJlciwgSSBkb24ndCB3YW50 IGEgZHluYW1pYyBvbmUuDQpXaHkgZG9lcyB1c2UgdGhlIGNvbnRyb2xsZXIncyBwaHlzaWNhbCBh ZGRyZXNzIHRvIHRha2UgdGhlIHJvbGUgb2YgYnVzIG51bWJlcg0KdG8gZGlzdGluZ3Vpc2ggY29u dHJvbGxlcnMuIA0KDQo+IEdye29ldGplLGVldGluZ31zLA0KPiANCj4gICAgICAgICAgICAgICAg ICAgICAgICAgR2VlcnQNCj4gDQo+IC0tDQo+IEdlZXJ0IFV5dHRlcmhvZXZlbiAtLSBUaGVyZSdz IGxvdHMgb2YgTGludXggYmV5b25kIGlhMzIgLS0gZ2VlcnRAbGludXgtDQo+IG02OGsub3JnDQo+ IA0KPiBJbiBwZXJzb25hbCBjb252ZXJzYXRpb25zIHdpdGggdGVjaG5pY2FsIHBlb3BsZSwgSSBj YWxsIG15c2VsZiBhIGhhY2tlci4NCj4gQnV0IHdoZW4gSSdtIHRhbGtpbmcgdG8gam91cm5hbGlz dHMgSSBqdXN0IHNheSAicHJvZ3JhbW1lciIgb3Igc29tZXRoaW5nDQo+IGxpa2UgdGhhdC4NCj4g ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAtLSBMaW51cyBUb3J2YWxkcw0KPiANCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2014-03-18 at 04:34 -0500, Hou Zhiqiang-B48286 wrote: > > > -----Original Message----- > > From: geert.uytterhoeven@gmail.com [mailto:geert.uytterhoeven@gmail.com] > > On Behalf Of Geert Uytterhoeven > > Sent: Tuesday, March 18, 2014 4:56 PM > > To: Hou Zhiqiang-B48286 > > Cc: Mark Brown; linux-spi@vger.kernel.org; devicetree@vger.kernel.org; > > rob.herring@calxeda.com; pawel.moll@arm.com; mark.rutland@arm.com; > > ijc+devicetree@hellion.org.uk; galak@codeaurora.org; > > grant.likely@secretlab.ca; Wood Scott-B07421; Hu Mingkai-B21284 > > Subject: Re: [PATCH 2/2] spi/fsl-lib: Get the SPI controller bus number > > from DTS > > > > On Tue, Mar 18, 2014 at 8:40 AM, B48286@freescale.com > > <B48286@freescale.com> wrote: > > >> > > The DT already has support for specifying flash layouts, can't > > >> > > those be used (for example via chosen if they're not fixed for the > > board)? > > >> > > Or if it's just picking the correct filesystem then UUIDs and > > >> > > labels are the standard way to do things. > > >> > > >> > The DT specifying flash layouts is ok. There is another way to make > > >> > the flash layouts using command line, but it is not safe because of > > >> > the dynamic bus_num. It is not the reason that the way of DT is > > >> > supported flash layouts, to live the other way unsafe, right? > > >> > > >> This sounds to me like we need a better way of talking about flash > > >> device names on the Linux command line rather than a way to fix the > > >> bus number - for example, being able to refer to them using a fixed > > >> property like the physical address. Being able to refer to devices > > >> via an alias assigned in the DT would also be useful (and more > > >> readable), I think there may already be a mechanism for doing that > > >> which would need to be plumbed in but I'm not 100% sure. > > > > > > The bus number is the variable designed to distinguish one spi > > > controller from others. Why spi controller's physical address must be > > > use instead of bus number? > > > > Because the bus number is dynamic, while the physical address doesn't > > change, so it can be used to uniqely identify the device before booting > > the kernel. > > Cfr. "spi1" vs. "e6e20000.spi". > > > > The precondition of dynamic bus number is initial it with -1 in the controller > driver. But now I need a reasonable bus number, I don't want a dynamic one. > Why does use the controller's physical address to take the role of bus number > to distinguish controllers. Where are you going to get this "reasonable" non-dynamic number from? How are you going to ensure there are no conflicts with other SPI controllers (e.g. on a dynamic add-on card)? Physical addresses work well because they are tied to something real, rather than an arbitrary enumeration. Our NAND controllers use the physical address for the MTD name. Device tree NOR flash allows the device tree to set the mtd name[1], and otherwise falls back on the platform device name, which contains the physical address. -Scott [1] This violates the "device tree describes hardware rather than configures Linux" rule... -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Wood Scott-B07421 > Sent: Wednesday, March 19, 2014 6:09 AM > To: Hou Zhiqiang-B48286 > Cc: 'Geert Uytterhoeven'; Mark Brown; linux-spi@vger.kernel.org; > devicetree@vger.kernel.org; rob.herring@calxeda.com; pawel.moll@arm.com; > mark.rutland@arm.com; ijc+devicetree@hellion.org.uk; galak@codeaurora.org; > grant.likely@secretlab.ca; Hu Mingkai-B21284 > Subject: Re: [PATCH 2/2] spi/fsl-lib: Get the SPI controller bus number > from DTS > > On Tue, 2014-03-18 at 04:34 -0500, Hou Zhiqiang-B48286 wrote: > > > > > -----Original Message----- > > > From: geert.uytterhoeven@gmail.com > > > [mailto:geert.uytterhoeven@gmail.com] > > > On Behalf Of Geert Uytterhoeven > > > Sent: Tuesday, March 18, 2014 4:56 PM > > > To: Hou Zhiqiang-B48286 > > > Cc: Mark Brown; linux-spi@vger.kernel.org; > > > devicetree@vger.kernel.org; rob.herring@calxeda.com; > > > pawel.moll@arm.com; mark.rutland@arm.com; > > > ijc+devicetree@hellion.org.uk; galak@codeaurora.org; > > > grant.likely@secretlab.ca; Wood Scott-B07421; Hu Mingkai-B21284 > > > Subject: Re: [PATCH 2/2] spi/fsl-lib: Get the SPI controller bus > > > number from DTS > > > > > > On Tue, Mar 18, 2014 at 8:40 AM, B48286@freescale.com > > > <B48286@freescale.com> wrote: > > > >> > > The DT already has support for specifying flash layouts, > > > >> > > can't those be used (for example via chosen if they're not > > > >> > > fixed for the > > > board)? > > > >> > > Or if it's just picking the correct filesystem then UUIDs and > > > >> > > labels are the standard way to do things. > > > >> > > > >> > The DT specifying flash layouts is ok. There is another way to > > > >> > make the flash layouts using command line, but it is not safe > > > >> > because of the dynamic bus_num. It is not the reason that the > > > >> > way of DT is supported flash layouts, to live the other way > unsafe, right? > > > >> > > > >> This sounds to me like we need a better way of talking about > > > >> flash device names on the Linux command line rather than a way to > > > >> fix the bus number - for example, being able to refer to them > > > >> using a fixed property like the physical address. Being able to > > > >> refer to devices via an alias assigned in the DT would also be > > > >> useful (and more readable), I think there may already be a > > > >> mechanism for doing that which would need to be plumbed in but I'm > not 100% sure. > > > > > > > > The bus number is the variable designed to distinguish one spi > > > > controller from others. Why spi controller's physical address must > > > > be use instead of bus number? > > > > > > Because the bus number is dynamic, while the physical address > > > doesn't change, so it can be used to uniqely identify the device > > > before booting the kernel. > > > Cfr. "spi1" vs. "e6e20000.spi". > > > > > > > The precondition of dynamic bus number is initial it with -1 in the > > controller driver. But now I need a reasonable bus number, I don't want > a dynamic one. > > Why does use the controller's physical address to take the role of bus > > number to distinguish controllers. > > Where are you going to get this "reasonable" non-dynamic number from? > How are you going to ensure there are no conflicts with other SPI > controllers (e.g. on a dynamic add-on card)? > "other than negative (== assign one dynamically), bus_num is fully board-specific. usually that simplifies to being SOC-specific. example: one SOC has three SPI controllers, numbered 0..2, and one board's schematics might show it using SPI-2. software would normally use bus_num=2 for that controller." The above paragraph is description of bus_num in spi.h. The "reasonable" is from it. Other controllers should also include this property, otherwise it will be dynamic. So there is not conflict. > Physical addresses work well because they are tied to something real, > rather than an arbitrary enumeration. Our NAND controllers use the > physical address for the MTD name. Device tree NOR flash allows the > device tree to set the mtd name[1], and otherwise falls back on the > platform device name, which contains the physical address. > I know the physical work well, but there is a mechanism of bus number. As the description say above, isn't it reasonable? > -Scott > > [1] This violates the "device tree describes hardware rather than > configures Linux" rule... >
On Tue, 2014-03-18 at 21:49 -0500, Hou Zhiqiang-B48286 wrote: > > > -----Original Message----- > > From: Wood Scott-B07421 > > Sent: Wednesday, March 19, 2014 6:09 AM > > To: Hou Zhiqiang-B48286 > > Cc: 'Geert Uytterhoeven'; Mark Brown; linux-spi@vger.kernel.org; > > devicetree@vger.kernel.org; rob.herring@calxeda.com; pawel.moll@arm.com; > > mark.rutland@arm.com; ijc+devicetree@hellion.org.uk; galak@codeaurora.org; > > grant.likely@secretlab.ca; Hu Mingkai-B21284 > > Subject: Re: [PATCH 2/2] spi/fsl-lib: Get the SPI controller bus number > > from DTS > > > > On Tue, 2014-03-18 at 04:34 -0500, Hou Zhiqiang-B48286 wrote: > > > > > > > -----Original Message----- > > > > From: geert.uytterhoeven@gmail.com > > > > [mailto:geert.uytterhoeven@gmail.com] > > > > On Behalf Of Geert Uytterhoeven > > > > Sent: Tuesday, March 18, 2014 4:56 PM > > > > To: Hou Zhiqiang-B48286 > > > > Cc: Mark Brown; linux-spi@vger.kernel.org; > > > > devicetree@vger.kernel.org; rob.herring@calxeda.com; > > > > pawel.moll@arm.com; mark.rutland@arm.com; > > > > ijc+devicetree@hellion.org.uk; galak@codeaurora.org; > > > > grant.likely@secretlab.ca; Wood Scott-B07421; Hu Mingkai-B21284 > > > > Subject: Re: [PATCH 2/2] spi/fsl-lib: Get the SPI controller bus > > > > number from DTS > > > > > > > > On Tue, Mar 18, 2014 at 8:40 AM, B48286@freescale.com > > > > <B48286@freescale.com> wrote: > > > > >> > > The DT already has support for specifying flash layouts, > > > > >> > > can't those be used (for example via chosen if they're not > > > > >> > > fixed for the > > > > board)? > > > > >> > > Or if it's just picking the correct filesystem then UUIDs and > > > > >> > > labels are the standard way to do things. > > > > >> > > > > >> > The DT specifying flash layouts is ok. There is another way to > > > > >> > make the flash layouts using command line, but it is not safe > > > > >> > because of the dynamic bus_num. It is not the reason that the > > > > >> > way of DT is supported flash layouts, to live the other way > > unsafe, right? > > > > >> > > > > >> This sounds to me like we need a better way of talking about > > > > >> flash device names on the Linux command line rather than a way to > > > > >> fix the bus number - for example, being able to refer to them > > > > >> using a fixed property like the physical address. Being able to > > > > >> refer to devices via an alias assigned in the DT would also be > > > > >> useful (and more readable), I think there may already be a > > > > >> mechanism for doing that which would need to be plumbed in but I'm > > not 100% sure. > > > > > > > > > > The bus number is the variable designed to distinguish one spi > > > > > controller from others. Why spi controller's physical address must > > > > > be use instead of bus number? > > > > > > > > Because the bus number is dynamic, while the physical address > > > > doesn't change, so it can be used to uniqely identify the device > > > > before booting the kernel. > > > > Cfr. "spi1" vs. "e6e20000.spi". > > > > > > > > > > The precondition of dynamic bus number is initial it with -1 in the > > > controller driver. But now I need a reasonable bus number, I don't want > > a dynamic one. > > > Why does use the controller's physical address to take the role of bus > > > number to distinguish controllers. > > > > Where are you going to get this "reasonable" non-dynamic number from? > > How are you going to ensure there are no conflicts with other SPI > > controllers (e.g. on a dynamic add-on card)? > > > "other than negative (== assign one dynamically), bus_num is fully > board-specific. usually that simplifies to being SOC-specific. > example: one SOC has three SPI controllers, numbered 0..2, > and one board's schematics might show it using SPI-2. software > would normally use bus_num=2 for that controller." > The above paragraph is description of bus_num in spi.h. The "reasonable" > is from it. > Other controllers should also include this property, otherwise it will be > dynamic. So there is not conflict. So instead of using something concrete like a physical address, you want to use a number from a manual. Again, what happens in a system where SPI controllers can be added dynamically (forget about whether this is possible on the systems you care about), or even statically from a different source (e.g. board logic)? How does the driver know what number the manual assigns to a given controller? Why would you even want to deal with this when using the physical address is so easy? > > Physical addresses work well because they are tied to something real, > > rather than an arbitrary enumeration. Our NAND controllers use the > > physical address for the MTD name. Device tree NOR flash allows the > > device tree to set the mtd name[1], and otherwise falls back on the > > platform device name, which contains the physical address. > > > I know the physical work well, but there is a mechanism of bus number. > As the description say above, isn't it reasonable? No. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 18, 2014 at 07:40:28AM +0000, B48286@freescale.com wrote: > > You are adding a property to specify a bus number. There is no reason > > why such a property should be specific to a single controller, every > > controller in every system has a bus number assigned to it so if we have > > a way to specify one controller via the device tree we should have a way > > to specify it for all. > The reason to specify a bus number to a single controller is to avoid > allocating one dynamically, so we know the bus number before booting up the > kernel. If there are several controllers, you should add this property to all > of them. But it is unnecessary to add this property, if you do not care about > the bus number, and it will be allocated dynamically. Please re-read what I wrote above, you're completely missing the point. The problem above is that you are adding this in driver specific code, even if this is a good idea there's nothing specific to this device about it so we shouldn't be doing it in a driver.
diff --git a/drivers/spi/spi-fsl-lib.c b/drivers/spi/spi-fsl-lib.c index 0b75f26..f5f0307b 100644 --- a/drivers/spi/spi-fsl-lib.c +++ b/drivers/spi/spi-fsl-lib.c @@ -198,7 +198,7 @@ int of_mpc8xxx_spi_probe(struct platform_device *ofdev) struct mpc8xxx_spi_probe_info *pinfo; struct fsl_spi_platform_data *pdata; const void *prop; - int ret = -ENOMEM; + int ret = -ENOMEM, bus_num; pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL); if (!pinfo) @@ -207,8 +207,12 @@ int of_mpc8xxx_spi_probe(struct platform_device *ofdev) pdata = &pinfo->pdata; dev->platform_data = pdata; - /* Allocate bus num dynamically. */ - pdata->bus_num = -1; + ret = of_property_read_u32(np, "bus-num", &bus_num); + if (ret < 0) { + /* Allocate bus num dynamically. */ + bus_num = -1; + } + pdata->bus_num = bus_num; #ifdef CONFIG_FSL_SOC /* SPI controller is either clocked from QE or SoC clock. */
Get the spi_master's bus_num from DTS to make the spi_master's name static. So "mtdparts=spi.bus_num.chip_select:..." in cmdline can be used to asign mtd partions of spi flash. Signed-off-by: Hou Zhiqiang <B48286@freescale.com> --- drivers/spi/spi-fsl-lib.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)