* RE: [PATCH] DT: add MDIO node for FMan node
From: Shaohui Xie @ 2014-11-13 8:27 UTC (permalink / raw)
To: Scott Wood
Cc: Igal.Liberman@freescale.com, linuxppc-dev@lists.ozlabs.org,
Emilian Medve, devicetree@vger.kernel.org
In-Reply-To: <1415865867.15957.58.camel@freescale.com>
DQoNCkJlc3QgUmVnYXJkcywgDQpTaGFvaHVpIFhpZQ0KDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNz
YWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0MjENCj4gU2VudDogVGh1cnNkYXksIE5v
dmVtYmVyIDEzLCAyMDE0IDQ6MDQgUE0NCj4gVG86IFhpZSBTaGFvaHVpLUIyMTk4OQ0KPiBDYzog
TGliZXJtYW4gSWdhbC1CMzE5NTA7IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOw0KPiBk
ZXZpY2V0cmVlQHZnZXIua2VybmVsLm9yZzsgTWVkdmUgRW1pbGlhbi1FTU1FRFZFMQ0KPiBTdWJq
ZWN0OiBSZTogW1BBVENIXSBEVDogYWRkIE1ESU8gbm9kZSBmb3IgRk1hbiBub2RlDQo+IA0KPiBP
biBUaHUsIDIwMTQtMTEtMTMgYXQgMDI6MDIgLTA2MDAsIFhpZSBTaGFvaHVpLUIyMTk4OSB3cm90
ZToNCj4gPg0KPiA+DQo+ID4gQmVzdCBSZWdhcmRzLA0KPiA+IFNoYW9odWkgWGllDQo+ID4NCj4g
Pg0KPiA+ID4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPiA+IEZyb206IFdvb2QgU2Nv
dHQtQjA3NDIxDQo+ID4gPiBTZW50OiBUaHVyc2RheSwgTm92ZW1iZXIgMTMsIDIwMTQgMzoxNSBQ
TQ0KPiA+ID4gVG86IFhpZSBTaGFvaHVpLUIyMTk4OQ0KPiA+ID4gQ2M6IExpYmVybWFuIElnYWwt
QjMxOTUwOyBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsNCj4gPiA+IGRldmljZXRyZWVA
dmdlci5rZXJuZWwub3JnOyBNZWR2ZSBFbWlsaWFuLUVNTUVEVkUxDQo+ID4gPiBTdWJqZWN0OiBS
ZTogW1BBVENIXSBEVDogYWRkIE1ESU8gbm9kZSBmb3IgRk1hbiBub2RlDQo+ID4gPg0KPiA+ID4g
T24gVGh1LCAyMDE0LTExLTEzIGF0IDAxOjExIC0wNjAwLCBYaWUgU2hhb2h1aS1CMjE5ODkgd3Jv
dGU6DQo+ID4gPiA+ID4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPiA+ID4gPiBGcm9t
OiBXb29kIFNjb3R0LUIwNzQyMQ0KPiA+ID4gPiA+IFNlbnQ6IFRodXJzZGF5LCBOb3ZlbWJlciAx
MywgMjAxNCAyOjE3IFBNDQo+ID4gPiA+ID4gVG86IFhpZSBTaGFvaHVpLUIyMTk4OQ0KPiA+ID4g
PiA+IENjOiBMaWJlcm1hbiBJZ2FsLUIzMTk1MDsgbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5v
cmc7DQo+ID4gPiA+ID4gZGV2aWNldHJlZUB2Z2VyLmtlcm5lbC5vcmc7IE1lZHZlIEVtaWxpYW4t
RU1NRURWRTENCj4gPiA+ID4gPiBTdWJqZWN0OiBSZTogW1BBVENIXSBEVDogYWRkIE1ESU8gbm9k
ZSBmb3IgRk1hbiBub2RlDQo+ID4gPiA+ID4NCj4gPiA+ID4gPiBPbiBXZWQsIDIwMTQtMTEtMTIg
YXQgMDc6NDAgLTA2MDAsIFhpZSBTaGFvaHVpLUIyMTk4OSB3cm90ZToNCj4gPiA+ID4gPiA+ID4g
LS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPiA+ID4gPiA+ID4gRnJvbTogV29vZCBTY290
dC1CMDc0MjENCj4gPiA+ID4gPiA+ID4gU2VudDogV2VkbmVzZGF5LCBOb3ZlbWJlciAxMiwgMjAx
NCAxOjM4IEFNDQo+ID4gPiA+ID4gPiA+IFRvOiBYaWUgU2hhb2h1aS1CMjE5ODkNCj4gPiA+ID4g
PiA+ID4gQ2M6IExpYmVybWFuIElnYWwtQjMxOTUwOyBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJz
Lm9yZzsNCj4gPiA+ID4gPiA+ID4gZGV2aWNldHJlZUB2Z2VyLmtlcm5lbC5vcmc7IE1lZHZlIEVt
aWxpYW4tRU1NRURWRTENCj4gPiA+ID4gPiA+ID4gU3ViamVjdDogUmU6IFtQQVRDSF0gRFQ6IGFk
ZCBNRElPIG5vZGUgZm9yIEZNYW4gbm9kZQ0KPiA+ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gPiBP
biBUdWUsIDIwMTQtMTEtMTEgYXQgMDQ6MzIgLTA2MDAsIFhpZSBTaGFvaHVpLUIyMTk4OSB3cm90
ZToNCj4gPiA+ID4gPiA+ID4gPiA+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiA+
ID4gPiA+ID4gPiBGcm9tOiBXb29kIFNjb3R0LUIwNzQyMQ0KPiA+ID4gPiA+ID4gPiA+ID4gU2Vu
dDogVHVlc2RheSwgTm92ZW1iZXIgMTEsIDIwMTQgODoyMyBBTQ0KPiA+ID4gPiA+ID4gPiA+ID4g
VG86IHNoaC54aWVAZ21haWwuY29tDQo+ID4gPiA+ID4gPiA+ID4gPiBDYzogbGludXhwcGMtZGV2
QGxpc3RzLm96bGFicy5vcmc7DQo+ID4gPiA+ID4gPiA+ID4gPiBkZXZpY2V0cmVlQHZnZXIua2Vy
bmVsLm9yZzsgTWVkdmUgRW1pbGlhbi1FTU1FRFZFMTsgWGllDQo+ID4gPiA+ID4gPiA+ID4gPiBT
aGFvaHVpLUIyMTk4OQ0KPiA+ID4gPiA+ID4gPiA+ID4gU3ViamVjdDogUmU6IFtQQVRDSF0gRFQ6
IGFkZCBNRElPIG5vZGUgZm9yIEZNYW4gbm9kZQ0KPiA+ID4gPiA+ID4gPiA+ID4NCj4gPiA+ID4g
PiA+ID4gPiA+IE9uIFR1ZSwgMjAxNC0xMS0wNCBhdCAxOTo1NiArMDgwMCwgc2hoLnhpZUBnbWFp
bC5jb20NCj4gd3JvdGU6DQo+ID4gPiA+ID4gPiA+ID4gPiA+IEZyb206IFNoYW9odWkgWGllIDxT
aGFvaHVpLlhpZUBmcmVlc2NhbGUuY29tPg0KPiA+ID4gPiA+ID4gPiA+ID4gPg0KPiA+ID4gPiA+
ID4gPiA+ID4gPiBUaGlzIGJpbmRpbmcgaXMgZm9yIEZNYW4gTURJTywgaXQgY292ZXJzIEZNYW4g
djIgJiBGTWFuDQo+IHYzLg0KPiA+ID4gPiA+ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gPiA+ID4g
PiBTaWduZWQtb2ZmLWJ5OiBTaGFvaHVpIFhpZSA8U2hhb2h1aS5YaWVAZnJlZXNjYWxlLmNvbT4N
Cj4gPiA+ID4gPiA+ID4gPiA+ID4gLS0tDQo+ID4gPiA+ID4gPiA+ID4gPiA+IGJhc2VkIG9uIGh0
dHA6Ly9wYXRjaHdvcmsub3psYWJzLm9yZy9wYXRjaC8zOTAzNTEvDQo+ID4gPiA+ID4gPiA+ID4g
PiA+IGZvciAnbmV4dCcgb2YNCj4gPiA+ID4gPiA+ID4gPiA+ID4NCj4gPiA+IGdpdDovL2dpdC5r
ZXJuZWwub3JnL3B1Yi9zY20vbGludXgva2VybmVsL2dpdC9zY290dHdvb2QvbGludXguDQo+ID4g
PiA+ID4gPiA+ID4gPiA+IGdpdA0KPiA+ID4gPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiA+ID4gPiA+
IEFyZSB0aGVyZSBhbnkgb3RoZXIgRk1hbiBwaWVjZXMgdGhhdCBhcmUgbWlzc2luZyBmcm9tDQo+
ID4gPiA+ID4gPiA+ID4gPiB0aGUgYWJvdmUNCj4gPiA+ID4gPiBwYXRjaD8NCj4gPiA+ID4gPiA+
ID4gPiBbUy5IXSBJJ20gYWRkaW5nIElnYWwgZm9yIHRoaXMgY29tbWVudC4NCj4gPiA+ID4gPiA+
ID4gPg0KPiA+ID4gPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiA+ID4gPiA+ID4gKy0gYnVzLWZyZXF1
ZW5jeQ0KPiA+ID4gPiA+ID4gPiA+ID4gPiArCQlVc2FnZTogb3B0aW9uYWwNCj4gPiA+ID4gPiA+
ID4gPiA+ID4gKwkJVmFsdWUgdHlwZTogPHUzMj4NCj4gPiA+ID4gPiA+ID4gPiA+ID4gKwkJRGVm
aW5pdGlvbjogRGVmYXVsdCBNRElPIGJ1cyBjbG9jayBzcGVlZC4NCj4gPiA+ID4gPiA+ID4gPiA+
DQo+ID4gPiA+ID4gPiA+ID4gPiBVc2UgY2xvY2tzL2Nsb2NrLW5hbWVzDQo+ID4gPiA+ID4gPiA+
ID4gW1MuSF0gVGhlIE1ESU8gdXNlcyBGbWFuIGNsb2NrIGFuZCBkaXZpZGVzIGl0IHRvIGEgcHJv
cGVyDQo+ID4gPiA+ID4gPiA+ID4gdmFsdWUgd2hpY2gNCj4gPiA+ID4gPiA+ID4gaXMgc3BlY2lm
aWVkIGJ5IHRoaXMgcHJvcGVydHkuDQo+ID4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiA+IFVzZSBj
bG9ja3MvY2xvY2stbmFtZXMgdG8gZGVzY3JpYmUgdGhhdCByZWxhdGlvbnNoaXAuDQo+ID4gPiA+
ID4gPiA+DQo+ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gW1MuSF0gVGhlIE1ESU8gbm9kZSBpcyBz
dWItbm9kZSBhbmQgZW1iZWRkZWQgaW4gRm1hbiBub2RlLCB0aGUNCj4gPiA+ID4gPiA+IGNsb2Nr
cy9jbG9jay1uYW1lcyBpcyBwcm92aWRlZCBieSBGbWFuIG5vZGUsIHNob3VsZCByZXBlYXQNCj4g
PiA+ID4gPiA+IHRoZW0gaW4gTURJTyBub2RlPyBGb3IgdGhlIGRlZmF1bHQgTURJTyBidXMgY2xv
Y2sgc3BlZWQsIG1heWJlDQo+ID4gPiA+ID4gPiAiY2xvY2stDQo+ID4gPiByYW5nZXMiDQo+ID4g
PiA+ID4gPiBzaG91bGQgYmUgdXNlZD8NCj4gPiA+ID4gPg0KPiA+ID4gPiA+IEl0J3MgYSBkaWZm
ZXJlbnQgY2xvY2suICBZb3Ugd291bGRuJ3QgYmUgcmVwZWF0aW5nLiAgSWYgaXQncw0KPiA+ID4g
PiA+IGRlcml2ZWQgZnJvbSB0aGUgRk1hbiBjbG9jaywgdGhlbiBtYXliZSB5b3UgZG9uJ3QgbmVl
ZCBhbnl0aGluZw0KPiA+ID4gPiA+IGhlcmUgKGRvZXMgdGhlIGRyaXZlciBrbm93IHdoYXQgdGhl
IGRpdmlkZXIgaXMsIG9yIHdvdWxkIHRoYXQNCj4gPiA+ID4gPiBuZWVkIHRvIGJlIHNwZWNpZmll
ZCBpbiB0aGUgZGV2aWNlIHRyZWU/KSwgYnV0IG5vIG1vcmUNCj4gPiA+ID4gPiBjbG9jay1mcmVx
dWVuY3kvYnVzLQ0KPiA+ID4gZnJlcXVlbmN5IHByb3BlcnRpZXMuDQo+ID4gPiA+ID4NCj4gPiA+
ID4gW1MuSF0gVGhlIHB1cnBvc2UgaGVyZSBpcyB0byBnZXQgYSBzcGVjaWZpYyBjbG9jayBmcmVx
dWVuY3ksDQo+ID4gPiA+IGRyaXZlciB0bw0KPiA+ID4gdXNlIGl0IHRvIGNhbGN1bGF0ZSB0aGUg
ZGl2aWRlci4NCj4gPiA+ID4gVGhlbiB0aGUgRm1hbiBjbG9jayBjYW4gYmUgZGl2aWRlZCB0byB0
aGUgZnJlcXVlbmN5Lg0KPiA+ID4NCj4gPiA+IE9oLCBzbyB0aGlzIGlzIHN0YXRpbmcgYSBkZXNp
cmVkIGZyZXF1ZW5jeSBhbmQgbm90IHNvbWV0aGluZyB0aGF0DQo+ID4gPiBhbHJlYWR5IGV4aXN0
cz8gIFdoYXQgZGV0ZXJtaW5lcyB0aGlzIGZyZXF1ZW5jeT8gIElzIGl0IGJhc2VkIG9uDQo+ID4g
PiBib2FyZCBkZXNpZ24sIG9yIGp1c3Qgb24gdGhlIE1ESU8gc3RhbmRhcmQsIGV0Yz8gIEknbSB3
b25kZXJpbmcgaWYNCj4gPiA+IHRoZSBkZXZpY2UgdHJlZSBpcyB0aGUgcmlnaHQgcGxhY2UgZm9y
IGl0Lg0KPiA+IFtTLkhdIFllcywgYSBkZXNpcmVkIGZyZXF1ZW5jeSB3aGljaCBpcyBkaWZmZXJl
bnQgd2l0aCBNRElPIHN0YW5kYXJkLg0KPiANCj4gSSdtIG5vdCBzdXJlIHdoYXQgeW91IG1lYW4g
YnkgImRpZmZlcmVudCB3aXRoIi4gIERvIHlvdSBtZWFuICJkaWZmZXJlbnQNCj4gZnJvbSI/ICBX
aGF0IGRvZXMgdGhlIHN0YW5kYXJkIHNheSBhYm91dCBmcmVxdWVuY3k/DQpbUy5IXSBUaGUgc3Rh
bmRhcmQgTURJTyBmcmVxdWVuY3kgaXMgMi41TUh6LiAgQnV0IGEgZGlmZmVyZW50IG9uZSBpcyBk
ZXNpcmVkLg0KDQo+IA0KPiA+IFRoZSBGbWFuIGNsb2NrIGFuZCB0aGUgZGl2aWRlciBkZXRlcm1p
bmVzIHRoaXMgZnJlcXVlbmN5Lg0KPiA+IFNpbmNlIEZtYW4gY2xvY2sgaXMgZGlmZmVyZW50IG9u
IGRpZmZlcmVudCBTb0NzLCBzbyBzcGVjaWZ5IHRoZQ0KPiA+IGRlc2lyZWQgZnJlcXVlbmN5LCB0
aGVuIHRvIGdldCB0aGUgcHJvcGVyIGRpdmlkZXIuDQo+IA0KPiBXaHkgZG9lcyB0aGUgZm1hbiBj
bG9jayBiZWluZyBkaWZmZXJlbnQgbWVhbiB0aGUgbWRpbyBjbG9jayBzaG91bGQgYmUNCj4gZGlm
ZmVyZW50Pw0KW1MuSF0gVGhlIG1kaW8gY2xvY2sgc2hvdWxkIGJlIHNhbWUsIGEgZGlmZmVyZW50
IGRpdmlkZXIgc2hvdWxkIGJlIHVzZWQgDQp0byBtYXRjaCB0aGUgRm1hbiBjbG9jayB0byBtYWtl
IHN1cmUgdGhlIG1kaW8gY2xvY2sga2VwdCBzYW1lLiBTbyB0byBzcGVjaWZ5DQpUaGUgZGVzaXJl
ZCBtZGlvIGZyZXF1ZW5jeSwgdGhlbiB0byBnZXQgcHJvcGVyIGRpdmlkZXIuDQoNClRoYW5rcy4N
ClNoYW9odWkNCg==
^ permalink raw reply
* Re: [PATCH] DT: add MDIO node for FMan node
From: Scott Wood @ 2014-11-13 8:04 UTC (permalink / raw)
To: Xie Shaohui-B21989
Cc: Liberman Igal-B31950, linuxppc-dev@lists.ozlabs.org,
Medve Emilian-EMMEDVE1, devicetree@vger.kernel.org
In-Reply-To: <489ad06fda99436eb3307d613b1f8293@DM2PR0301MB0864.namprd03.prod.outlook.com>
On Thu, 2014-11-13 at 02:02 -0600, Xie Shaohui-B21989 wrote:
>
>
> Best Regards,
> Shaohui Xie
>
>
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Thursday, November 13, 2014 3:15 PM
> > To: Xie Shaohui-B21989
> > Cc: Liberman Igal-B31950; linuxppc-dev@lists.ozlabs.org;
> > devicetree@vger.kernel.org; Medve Emilian-EMMEDVE1
> > Subject: Re: [PATCH] DT: add MDIO node for FMan node
> >
> > On Thu, 2014-11-13 at 01:11 -0600, Xie Shaohui-B21989 wrote:
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Thursday, November 13, 2014 2:17 PM
> > > > To: Xie Shaohui-B21989
> > > > Cc: Liberman Igal-B31950; linuxppc-dev@lists.ozlabs.org;
> > > > devicetree@vger.kernel.org; Medve Emilian-EMMEDVE1
> > > > Subject: Re: [PATCH] DT: add MDIO node for FMan node
> > > >
> > > > On Wed, 2014-11-12 at 07:40 -0600, Xie Shaohui-B21989 wrote:
> > > > > > -----Original Message-----
> > > > > > From: Wood Scott-B07421
> > > > > > Sent: Wednesday, November 12, 2014 1:38 AM
> > > > > > To: Xie Shaohui-B21989
> > > > > > Cc: Liberman Igal-B31950; linuxppc-dev@lists.ozlabs.org;
> > > > > > devicetree@vger.kernel.org; Medve Emilian-EMMEDVE1
> > > > > > Subject: Re: [PATCH] DT: add MDIO node for FMan node
> > > > > >
> > > > > > On Tue, 2014-11-11 at 04:32 -0600, Xie Shaohui-B21989 wrote:
> > > > > > > > -----Original Message-----
> > > > > > > > From: Wood Scott-B07421
> > > > > > > > Sent: Tuesday, November 11, 2014 8:23 AM
> > > > > > > > To: shh.xie@gmail.com
> > > > > > > > Cc: linuxppc-dev@lists.ozlabs.org;
> > > > > > > > devicetree@vger.kernel.org; Medve Emilian-EMMEDVE1; Xie
> > > > > > > > Shaohui-B21989
> > > > > > > > Subject: Re: [PATCH] DT: add MDIO node for FMan node
> > > > > > > >
> > > > > > > > On Tue, 2014-11-04 at 19:56 +0800, shh.xie@gmail.com wrote:
> > > > > > > > > From: Shaohui Xie <Shaohui.Xie@freescale.com>
> > > > > > > > >
> > > > > > > > > This binding is for FMan MDIO, it covers FMan v2 & FMan v3.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> > > > > > > > > ---
> > > > > > > > > based on http://patchwork.ozlabs.org/patch/390351/
> > > > > > > > > for 'next' of
> > > > > > > > >
> > git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.
> > > > > > > > > git
> > > > > > > >
> > > > > > > > Are there any other FMan pieces that are missing from the
> > > > > > > > above
> > > > patch?
> > > > > > > [S.H] I'm adding Igal for this comment.
> > > > > > >
> > > > > > > >
> > > > > > > > > +- bus-frequency
> > > > > > > > > + Usage: optional
> > > > > > > > > + Value type: <u32>
> > > > > > > > > + Definition: Default MDIO bus clock speed.
> > > > > > > >
> > > > > > > > Use clocks/clock-names
> > > > > > > [S.H] The MDIO uses Fman clock and divides it to a proper
> > > > > > > value which
> > > > > > is specified by this property.
> > > > > >
> > > > > > Use clocks/clock-names to describe that relationship.
> > > > > >
> > > > >
> > > > > [S.H] The MDIO node is sub-node and embedded in Fman node, the
> > > > > clocks/clock-names is provided by Fman node, should repeat them in
> > > > > MDIO node? For the default MDIO bus clock speed, maybe "clock-
> > ranges"
> > > > > should be used?
> > > >
> > > > It's a different clock. You wouldn't be repeating. If it's derived
> > > > from the FMan clock, then maybe you don't need anything here (does
> > > > the driver know what the divider is, or would that need to be
> > > > specified in the device tree?), but no more clock-frequency/bus-
> > frequency properties.
> > > >
> > > [S.H] The purpose here is to get a specific clock frequency, driver to
> > use it to calculate the divider.
> > > Then the Fman clock can be divided to the frequency.
> >
> > Oh, so this is stating a desired frequency and not something that already
> > exists? What determines this frequency? Is it based on board design, or
> > just on the MDIO standard, etc? I'm wondering if the device tree is the
> > right place for it.
> [S.H] Yes, a desired frequency which is different with MDIO standard.
I'm not sure what you mean by "different with". Do you mean "different
from"? What does the standard say about frequency?
> The Fman clock and the divider determines this frequency.
> Since Fman clock is different on different SoCs, so specify the desired frequency,
> then to get the proper divider.
Why does the fman clock being different mean the mdio clock should be
different?
-Scott
^ permalink raw reply
* RE: [PATCH] DT: add MDIO node for FMan node
From: Shaohui Xie @ 2014-11-13 8:02 UTC (permalink / raw)
To: Scott Wood
Cc: Igal.Liberman@freescale.com, linuxppc-dev@lists.ozlabs.org,
Emilian Medve, devicetree@vger.kernel.org
In-Reply-To: <1415862913.15957.56.camel@freescale.com>
DQoNCkJlc3QgUmVnYXJkcywgDQpTaGFvaHVpIFhpZQ0KDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNz
YWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0MjENCj4gU2VudDogVGh1cnNkYXksIE5v
dmVtYmVyIDEzLCAyMDE0IDM6MTUgUE0NCj4gVG86IFhpZSBTaGFvaHVpLUIyMTk4OQ0KPiBDYzog
TGliZXJtYW4gSWdhbC1CMzE5NTA7IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOw0KPiBk
ZXZpY2V0cmVlQHZnZXIua2VybmVsLm9yZzsgTWVkdmUgRW1pbGlhbi1FTU1FRFZFMQ0KPiBTdWJq
ZWN0OiBSZTogW1BBVENIXSBEVDogYWRkIE1ESU8gbm9kZSBmb3IgRk1hbiBub2RlDQo+IA0KPiBP
biBUaHUsIDIwMTQtMTEtMTMgYXQgMDE6MTEgLTA2MDAsIFhpZSBTaGFvaHVpLUIyMTk4OSB3cm90
ZToNCj4gPiA+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiBGcm9tOiBXb29kIFNj
b3R0LUIwNzQyMQ0KPiA+ID4gU2VudDogVGh1cnNkYXksIE5vdmVtYmVyIDEzLCAyMDE0IDI6MTcg
UE0NCj4gPiA+IFRvOiBYaWUgU2hhb2h1aS1CMjE5ODkNCj4gPiA+IENjOiBMaWJlcm1hbiBJZ2Fs
LUIzMTk1MDsgbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmc7DQo+ID4gPiBkZXZpY2V0cmVl
QHZnZXIua2VybmVsLm9yZzsgTWVkdmUgRW1pbGlhbi1FTU1FRFZFMQ0KPiA+ID4gU3ViamVjdDog
UmU6IFtQQVRDSF0gRFQ6IGFkZCBNRElPIG5vZGUgZm9yIEZNYW4gbm9kZQ0KPiA+ID4NCj4gPiA+
IE9uIFdlZCwgMjAxNC0xMS0xMiBhdCAwNzo0MCAtMDYwMCwgWGllIFNoYW9odWktQjIxOTg5IHdy
b3RlOg0KPiA+ID4gPiA+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiA+ID4gRnJv
bTogV29vZCBTY290dC1CMDc0MjENCj4gPiA+ID4gPiBTZW50OiBXZWRuZXNkYXksIE5vdmVtYmVy
IDEyLCAyMDE0IDE6MzggQU0NCj4gPiA+ID4gPiBUbzogWGllIFNoYW9odWktQjIxOTg5DQo+ID4g
PiA+ID4gQ2M6IExpYmVybWFuIElnYWwtQjMxOTUwOyBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJz
Lm9yZzsNCj4gPiA+ID4gPiBkZXZpY2V0cmVlQHZnZXIua2VybmVsLm9yZzsgTWVkdmUgRW1pbGlh
bi1FTU1FRFZFMQ0KPiA+ID4gPiA+IFN1YmplY3Q6IFJlOiBbUEFUQ0hdIERUOiBhZGQgTURJTyBu
b2RlIGZvciBGTWFuIG5vZGUNCj4gPiA+ID4gPg0KPiA+ID4gPiA+IE9uIFR1ZSwgMjAxNC0xMS0x
MSBhdCAwNDozMiAtMDYwMCwgWGllIFNoYW9odWktQjIxOTg5IHdyb3RlOg0KPiA+ID4gPiA+ID4g
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+ID4gPiA+ID4gPiBGcm9tOiBXb29kIFNj
b3R0LUIwNzQyMQ0KPiA+ID4gPiA+ID4gPiBTZW50OiBUdWVzZGF5LCBOb3ZlbWJlciAxMSwgMjAx
NCA4OjIzIEFNDQo+ID4gPiA+ID4gPiA+IFRvOiBzaGgueGllQGdtYWlsLmNvbQ0KPiA+ID4gPiA+
ID4gPiBDYzogbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmc7DQo+ID4gPiA+ID4gPiA+IGRl
dmljZXRyZWVAdmdlci5rZXJuZWwub3JnOyBNZWR2ZSBFbWlsaWFuLUVNTUVEVkUxOyBYaWUNCj4g
PiA+ID4gPiA+ID4gU2hhb2h1aS1CMjE5ODkNCj4gPiA+ID4gPiA+ID4gU3ViamVjdDogUmU6IFtQ
QVRDSF0gRFQ6IGFkZCBNRElPIG5vZGUgZm9yIEZNYW4gbm9kZQ0KPiA+ID4gPiA+ID4gPg0KPiA+
ID4gPiA+ID4gPiBPbiBUdWUsIDIwMTQtMTEtMDQgYXQgMTk6NTYgKzA4MDAsIHNoaC54aWVAZ21h
aWwuY29tIHdyb3RlOg0KPiA+ID4gPiA+ID4gPiA+IEZyb206IFNoYW9odWkgWGllIDxTaGFvaHVp
LlhpZUBmcmVlc2NhbGUuY29tPg0KPiA+ID4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiA+ID4gVGhp
cyBiaW5kaW5nIGlzIGZvciBGTWFuIE1ESU8sIGl0IGNvdmVycyBGTWFuIHYyICYgRk1hbiB2My4N
Cj4gPiA+ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gPiA+IFNpZ25lZC1vZmYtYnk6IFNoYW9odWkg
WGllIDxTaGFvaHVpLlhpZUBmcmVlc2NhbGUuY29tPg0KPiA+ID4gPiA+ID4gPiA+IC0tLQ0KPiA+
ID4gPiA+ID4gPiA+IGJhc2VkIG9uIGh0dHA6Ly9wYXRjaHdvcmsub3psYWJzLm9yZy9wYXRjaC8z
OTAzNTEvDQo+ID4gPiA+ID4gPiA+ID4gZm9yICduZXh0JyBvZg0KPiA+ID4gPiA+ID4gPiA+DQo+
IGdpdDovL2dpdC5rZXJuZWwub3JnL3B1Yi9zY20vbGludXgva2VybmVsL2dpdC9zY290dHdvb2Qv
bGludXguDQo+ID4gPiA+ID4gPiA+ID4gZ2l0DQo+ID4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiA+
IEFyZSB0aGVyZSBhbnkgb3RoZXIgRk1hbiBwaWVjZXMgdGhhdCBhcmUgbWlzc2luZyBmcm9tIHRo
ZQ0KPiA+ID4gPiA+ID4gPiBhYm92ZQ0KPiA+ID4gcGF0Y2g/DQo+ID4gPiA+ID4gPiBbUy5IXSBJ
J20gYWRkaW5nIElnYWwgZm9yIHRoaXMgY29tbWVudC4NCj4gPiA+ID4gPiA+DQo+ID4gPiA+ID4g
PiA+DQo+ID4gPiA+ID4gPiA+ID4gKy0gYnVzLWZyZXF1ZW5jeQ0KPiA+ID4gPiA+ID4gPiA+ICsJ
CVVzYWdlOiBvcHRpb25hbA0KPiA+ID4gPiA+ID4gPiA+ICsJCVZhbHVlIHR5cGU6IDx1MzI+DQo+
ID4gPiA+ID4gPiA+ID4gKwkJRGVmaW5pdGlvbjogRGVmYXVsdCBNRElPIGJ1cyBjbG9jayBzcGVl
ZC4NCj4gPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiA+ID4gVXNlIGNsb2Nrcy9jbG9jay1uYW1lcw0K
PiA+ID4gPiA+ID4gW1MuSF0gVGhlIE1ESU8gdXNlcyBGbWFuIGNsb2NrIGFuZCBkaXZpZGVzIGl0
IHRvIGEgcHJvcGVyDQo+ID4gPiA+ID4gPiB2YWx1ZSB3aGljaA0KPiA+ID4gPiA+IGlzIHNwZWNp
ZmllZCBieSB0aGlzIHByb3BlcnR5Lg0KPiA+ID4gPiA+DQo+ID4gPiA+ID4gVXNlIGNsb2Nrcy9j
bG9jay1uYW1lcyB0byBkZXNjcmliZSB0aGF0IHJlbGF0aW9uc2hpcC4NCj4gPiA+ID4gPg0KPiA+
ID4gPg0KPiA+ID4gPiBbUy5IXSBUaGUgTURJTyBub2RlIGlzIHN1Yi1ub2RlIGFuZCBlbWJlZGRl
ZCBpbiBGbWFuIG5vZGUsIHRoZQ0KPiA+ID4gPiBjbG9ja3MvY2xvY2stbmFtZXMgaXMgcHJvdmlk
ZWQgYnkgRm1hbiBub2RlLCBzaG91bGQgcmVwZWF0IHRoZW0gaW4NCj4gPiA+ID4gTURJTyBub2Rl
PyBGb3IgdGhlIGRlZmF1bHQgTURJTyBidXMgY2xvY2sgc3BlZWQsIG1heWJlICJjbG9jay0NCj4g
cmFuZ2VzIg0KPiA+ID4gPiBzaG91bGQgYmUgdXNlZD8NCj4gPiA+DQo+ID4gPiBJdCdzIGEgZGlm
ZmVyZW50IGNsb2NrLiAgWW91IHdvdWxkbid0IGJlIHJlcGVhdGluZy4gIElmIGl0J3MgZGVyaXZl
ZA0KPiA+ID4gZnJvbSB0aGUgRk1hbiBjbG9jaywgdGhlbiBtYXliZSB5b3UgZG9uJ3QgbmVlZCBh
bnl0aGluZyBoZXJlIChkb2VzDQo+ID4gPiB0aGUgZHJpdmVyIGtub3cgd2hhdCB0aGUgZGl2aWRl
ciBpcywgb3Igd291bGQgdGhhdCBuZWVkIHRvIGJlDQo+ID4gPiBzcGVjaWZpZWQgaW4gdGhlIGRl
dmljZSB0cmVlPyksIGJ1dCBubyBtb3JlIGNsb2NrLWZyZXF1ZW5jeS9idXMtDQo+IGZyZXF1ZW5j
eSBwcm9wZXJ0aWVzLg0KPiA+ID4NCj4gPiBbUy5IXSBUaGUgcHVycG9zZSBoZXJlIGlzIHRvIGdl
dCBhIHNwZWNpZmljIGNsb2NrIGZyZXF1ZW5jeSwgZHJpdmVyIHRvDQo+IHVzZSBpdCB0byBjYWxj
dWxhdGUgdGhlIGRpdmlkZXIuDQo+ID4gVGhlbiB0aGUgRm1hbiBjbG9jayBjYW4gYmUgZGl2aWRl
ZCB0byB0aGUgZnJlcXVlbmN5Lg0KPiANCj4gT2gsIHNvIHRoaXMgaXMgc3RhdGluZyBhIGRlc2ly
ZWQgZnJlcXVlbmN5IGFuZCBub3Qgc29tZXRoaW5nIHRoYXQgYWxyZWFkeQ0KPiBleGlzdHM/ICBX
aGF0IGRldGVybWluZXMgdGhpcyBmcmVxdWVuY3k/ICBJcyBpdCBiYXNlZCBvbiBib2FyZCBkZXNp
Z24sIG9yDQo+IGp1c3Qgb24gdGhlIE1ESU8gc3RhbmRhcmQsIGV0Yz8gIEknbSB3b25kZXJpbmcg
aWYgdGhlIGRldmljZSB0cmVlIGlzIHRoZQ0KPiByaWdodCBwbGFjZSBmb3IgaXQuDQpbUy5IXSBZ
ZXMsIGEgZGVzaXJlZCBmcmVxdWVuY3kgd2hpY2ggaXMgZGlmZmVyZW50IHdpdGggTURJTyBzdGFu
ZGFyZC4NClRoZSBGbWFuIGNsb2NrIGFuZCB0aGUgZGl2aWRlciBkZXRlcm1pbmVzIHRoaXMgZnJl
cXVlbmN5Lg0KU2luY2UgRm1hbiBjbG9jayBpcyBkaWZmZXJlbnQgb24gZGlmZmVyZW50IFNvQ3Ms
IHNvIHNwZWNpZnkgdGhlIGRlc2lyZWQgZnJlcXVlbmN5LCANCnRoZW4gdG8gZ2V0IHRoZSBwcm9w
ZXIgZGl2aWRlci4NCg0KDQpUaGFua3MuDQpzaGFvaHVpDQo=
^ permalink raw reply
* Re: [PATCH] i2c: Driver to expose PowerNV platform i2c busses
From: Wolfram Sang @ 2014-11-13 7:58 UTC (permalink / raw)
To: Neelesh Gupta; +Cc: linuxppc-dev, linux-i2c
In-Reply-To: <20141110060424.9407.2498.stgit@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 10410 bytes --]
Hi,
I am basically fine if this goes via the powerpc-tree and I was hoping
that I could ack it right now. However, the driver looks a bit rushed
and definately needs updates before it is ready to go.
On Mon, Nov 10, 2014 at 11:35:39AM +0530, Neelesh Gupta wrote:
> The patch exposes the available i2c busses on the PowerNV platform
> to the kernel and implements the bus driver to support i2c and
> smbus commands.
> The driver uses the platform device infrastructure to probe the busses
> on the platform and registers them with the i2c driver framework.
>
> Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Review for the I2C parts:
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 78d56c5..350aa86 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -102,5 +102,6 @@ obj-$(CONFIG_I2C_ELEKTOR) += i2c-elektor.o
> obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o
> obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o
> obj-$(CONFIG_SCx200_ACB) += scx200_acb.o
> +obj-$(CONFIG_I2C_OPAL) += i2c-opal.o
Please sort it properly, not simply at the end.
>
> ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
> diff --git a/drivers/i2c/busses/i2c-opal.c b/drivers/i2c/busses/i2c-opal.c
> new file mode 100644
> index 0000000..3261716
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-opal.c
> @@ -0,0 +1,276 @@
> +/*
> + * IBM OPAL I2C driver
> + * Copyright (C) 2014 IBM
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/mm.h>
> +#include <asm/opal.h>
> +#include <asm/firmware.h>
Please sort the includes.
> +
> +static int i2c_opal_send_request(u32 bus_id, struct opal_i2c_request *req)
> +{
> + struct opal_msg msg;
> + int token, rc;
> +
> + token = opal_async_get_token_interruptible();
> + if (token < 0) {
> + if (token != -ERESTARTSYS)
> + pr_err("Failed to get the async token\n");
> +
> + return token;
> + }
> +
> + rc = opal_i2c_request(token, bus_id, req);
> + if (rc != OPAL_ASYNC_COMPLETION) {
> + rc = -EIO;
> + goto exit;
> + }
> +
> + rc = opal_async_wait_response(token, &msg);
> + if (rc) {
> + rc = -EIO;
> + goto exit;
Is it really -EIO? Maybe -ETIMEDOUT?
> + }
> +
> + rc = be64_to_cpu(msg.params[1]);
> + if (rc != OPAL_SUCCESS) {
> + rc = -EIO;
> + goto exit;
> + }
> +
> +exit:
> + opal_async_release_token(token);
> + return rc;
> +}
> +
> +static int i2c_opal_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> + int num)
> +{
> + unsigned long opal_id = (unsigned long)adap->algo_data;
> + struct opal_i2c_request req;
> + int rc, i;
> +
> + /* We only support fairly simple combinations here of one
> + * or two messages
> + */
I don't think you should offer I2C_FUNC_I2C with those limitations. Is
there a case you really needs this?
> + memset(&req, 0, sizeof(req));
> + switch (num) {
> + case 0:
> + return 0;
> + case 1:
> + req.type = (msgs[0].flags & I2C_M_RD) ?
> + OPAL_I2C_RAW_READ : OPAL_I2C_RAW_WRITE;
> + req.addr = cpu_to_be16(msgs[0].addr);
> + req.size = cpu_to_be32(msgs[0].len);
> + req.buffer_ra = cpu_to_be64(__pa(msgs[0].buf));
> + break;
> + case 2:
> + /* For two messages, we basically support only simple
> + * smbus transactions of a write plus a read. We might
> + * want to allow also two writes but we'd have to bounce
> + * the data into a single buffer.
> + */
> + if ((msgs[0].flags & I2C_M_RD) || !(msgs[1].flags & I2C_M_RD))
> + return -EIO;
> + if (msgs[0].len > 4)
> + return -EIO;
> + if (msgs[0].addr != msgs[1].addr)
> + return -EIO;
-EOPNOTSUPP? Please check Documentation/i2c/fault-codes for the error
codes we use.
> + req.type = OPAL_I2C_SM_READ;
> + req.addr = cpu_to_be16(msgs[0].addr);
> + req.subaddr_sz = msgs[0].len;
> + for (i = 0; i < msgs[0].len; i++)
> + req.subaddr = (req.subaddr << 8) | msgs[0].buf[i];
> + req.subaddr = cpu_to_be32(req.subaddr);
> + req.size = cpu_to_be32(msgs[1].len);
> + req.buffer_ra = cpu_to_be64(__pa(msgs[1].buf));
> + break;
> + default:
> + return -EIO;
> + }
> +
> + rc = i2c_opal_send_request(opal_id, &req);
> + if (rc)
> + return rc;
> +
> + return num;
> +}
> +
> +static int i2c_opal_smbus_xfer(struct i2c_adapter *adap, u16 addr,
> + unsigned short flags, char read_write,
> + u8 command, int size, union i2c_smbus_data *data)
> +{
> + unsigned long opal_id = (unsigned long)adap->algo_data;
> + struct opal_i2c_request req;
> + u8 local[2];
> + int rc;
> +
> + memset(&req, 0, sizeof(req));
> +
> + req.addr = cpu_to_be16(addr);
> + switch (size) {
> + case I2C_SMBUS_BYTE:
> + req.buffer_ra = cpu_to_be64(__pa(&data->byte));
> + req.size = cpu_to_be32(1);
> + /* Fall through */
> + case I2C_SMBUS_QUICK:
> + req.type = (read_write == I2C_SMBUS_READ) ?
> + OPAL_I2C_RAW_READ : OPAL_I2C_RAW_WRITE;
> + break;
> + case I2C_SMBUS_BYTE_DATA:
> + req.buffer_ra = cpu_to_be64(__pa(&data->byte));
> + req.size = cpu_to_be32(1);
> + req.subaddr = cpu_to_be32(command);
> + req.subaddr_sz = 1;
> + req.type = (read_write == I2C_SMBUS_READ) ?
> + OPAL_I2C_SM_READ : OPAL_I2C_SM_WRITE;
> + break;
> + case I2C_SMBUS_WORD_DATA:
> + if (!read_write) {
> + local[0] = data->word & 0xff;
> + local[1] = (data->word >> 8) & 0xff;
> + }
> + req.buffer_ra = cpu_to_be64(__pa(local));
> + req.size = cpu_to_be32(2);
> + req.subaddr = cpu_to_be32(command);
> + req.subaddr_sz = 1;
> + req.type = (read_write == I2C_SMBUS_READ) ?
> + OPAL_I2C_SM_READ : OPAL_I2C_SM_WRITE;
> + break;
> + case I2C_SMBUS_I2C_BLOCK_DATA:
> + req.buffer_ra = cpu_to_be64(__pa(&data->block[1]));
> + req.size = cpu_to_be32(data->block[0]);
> + req.subaddr = cpu_to_be32(command);
> + req.subaddr_sz = 1;
> + req.type = (read_write == I2C_SMBUS_READ) ?
> + OPAL_I2C_SM_READ : OPAL_I2C_SM_WRITE;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + rc = i2c_opal_send_request(opal_id, &req);
> + if (!rc && read_write && size == I2C_SMBUS_WORD_DATA) {
> + data->word = ((u16)local[1]) << 8;
> + data->word |= local[0];
> + }
> +
> + return rc;
> +}
> +
> +static u32 i2c_opal_func(struct i2c_adapter *adapter)
> +{
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
> + I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
> + I2C_FUNC_SMBUS_I2C_BLOCK;
> +}
See comment above about I2C_FUNC_I2C?
> +
> +static const struct i2c_algorithm i2c_opal_algo = {
> + .master_xfer = i2c_opal_master_xfer,
> + .smbus_xfer = i2c_opal_smbus_xfer,
> + .functionality = i2c_opal_func,
> +};
> +
> +static int i2c_opal_probe(struct platform_device *pdev)
> +{
> + struct i2c_adapter *adapter;
> + const char *pname;
> + u32 opal_id;
> + int rc;
> +
> + if (!pdev->dev.of_node)
> + return -ENODEV;
Can this happen? How would the match happen otherwise?
> + rc = of_property_read_u32(pdev->dev.of_node, "ibm,opal-id", &opal_id);
> + if (rc) {
> + dev_err(&pdev->dev, "Missing ibm,opal-id property !\n");
> + return -EIO;
> + }
> + adapter = kzalloc(sizeof(struct i2c_adapter), GFP_KERNEL);
devm_kzalloc?
> + if (!adapter)
> + return -ENOMEM;
> + adapter->algo = &i2c_opal_algo;
> + adapter->algo_data = (void *)(unsigned long)opal_id;
double cast?
> + adapter->dev.parent = &pdev->dev;
> + adapter->dev.of_node = of_node_get(pdev->dev.of_node);
> + pname = of_get_property(pdev->dev.of_node, "port-name", NULL);
I have never seen this binding before, it looks fishy. Where is it documented?
> + if (pname)
> + strlcpy(adapter->name, pname, sizeof(adapter->name));
> + else
> + strlcpy(adapter->name, "opal", sizeof(adapter->name));
> +
> + platform_set_drvdata(pdev, adapter);
> + rc = i2c_add_adapter(adapter);
> + if (rc)
> + dev_err(&pdev->dev, "Failed to register the i2c adapter\n");
Leaking 'adapter' here.
> +
> + return rc;
> +}
> +
> +static int i2c_opal_remove(struct platform_device *pdev)
> +{
> + struct i2c_adapter *adapter = platform_get_drvdata(pdev);
> +
> + i2c_del_adapter(adapter);
> +
> + kfree(adapter);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id i2c_opal_of_match[] = {
> + {
> + .compatible = "ibm,power8-i2c-port",
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, i2c_opal_of_match);
> +
> +static struct platform_driver i2c_opal_driver = {
> + .probe = i2c_opal_probe,
> + .remove = i2c_opal_remove,
> + .driver = {
> + .name = "i2c-opal",
> + .owner = THIS_MODULE,
Not needed.
> + .of_match_table = i2c_opal_of_match,
> + },
> +};
> +
> +static int __init i2c_opal_init(void)
> +{
> + if (!firmware_has_feature(FW_FEATURE_OPAL))
> + return -ENODEV;
> +
> + return platform_driver_register(&i2c_opal_driver);
> +}
> +
> +static void __exit i2c_opal_exit(void)
> +{
> + return platform_driver_unregister(&i2c_opal_driver);
> +}
> +
> +MODULE_AUTHOR("Neelesh Gupta <neelegup@linux.vnet.ibm.com>");
> +MODULE_DESCRIPTION("IBM OPAL I2C driver");
> +MODULE_LICENSE("GPL");
> +
> +module_init(i2c_opal_init);
> +module_exit(i2c_opal_exit);
Please put thos right below the functions it references.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] DT: add MDIO node for FMan node
From: Scott Wood @ 2014-11-13 7:15 UTC (permalink / raw)
To: Xie Shaohui-B21989
Cc: Liberman Igal-B31950, linuxppc-dev@lists.ozlabs.org,
Medve Emilian-EMMEDVE1, devicetree@vger.kernel.org
In-Reply-To: <59c6a8b7ad1d477aad9edd2152d02f8e@DM2PR0301MB0864.namprd03.prod.outlook.com>
On Thu, 2014-11-13 at 01:11 -0600, Xie Shaohui-B21989 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Thursday, November 13, 2014 2:17 PM
> > To: Xie Shaohui-B21989
> > Cc: Liberman Igal-B31950; linuxppc-dev@lists.ozlabs.org;
> > devicetree@vger.kernel.org; Medve Emilian-EMMEDVE1
> > Subject: Re: [PATCH] DT: add MDIO node for FMan node
> >
> > On Wed, 2014-11-12 at 07:40 -0600, Xie Shaohui-B21989 wrote:
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Wednesday, November 12, 2014 1:38 AM
> > > > To: Xie Shaohui-B21989
> > > > Cc: Liberman Igal-B31950; linuxppc-dev@lists.ozlabs.org;
> > > > devicetree@vger.kernel.org; Medve Emilian-EMMEDVE1
> > > > Subject: Re: [PATCH] DT: add MDIO node for FMan node
> > > >
> > > > On Tue, 2014-11-11 at 04:32 -0600, Xie Shaohui-B21989 wrote:
> > > > > > -----Original Message-----
> > > > > > From: Wood Scott-B07421
> > > > > > Sent: Tuesday, November 11, 2014 8:23 AM
> > > > > > To: shh.xie@gmail.com
> > > > > > Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org;
> > > > > > Medve Emilian-EMMEDVE1; Xie Shaohui-B21989
> > > > > > Subject: Re: [PATCH] DT: add MDIO node for FMan node
> > > > > >
> > > > > > On Tue, 2014-11-04 at 19:56 +0800, shh.xie@gmail.com wrote:
> > > > > > > From: Shaohui Xie <Shaohui.Xie@freescale.com>
> > > > > > >
> > > > > > > This binding is for FMan MDIO, it covers FMan v2 & FMan v3.
> > > > > > >
> > > > > > > Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> > > > > > > ---
> > > > > > > based on http://patchwork.ozlabs.org/patch/390351/
> > > > > > > for 'next' of
> > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.
> > > > > > > git
> > > > > >
> > > > > > Are there any other FMan pieces that are missing from the above
> > patch?
> > > > > [S.H] I'm adding Igal for this comment.
> > > > >
> > > > > >
> > > > > > > +- bus-frequency
> > > > > > > + Usage: optional
> > > > > > > + Value type: <u32>
> > > > > > > + Definition: Default MDIO bus clock speed.
> > > > > >
> > > > > > Use clocks/clock-names
> > > > > [S.H] The MDIO uses Fman clock and divides it to a proper value
> > > > > which
> > > > is specified by this property.
> > > >
> > > > Use clocks/clock-names to describe that relationship.
> > > >
> > >
> > > [S.H] The MDIO node is sub-node and embedded in Fman node, the
> > > clocks/clock-names is provided by Fman node, should repeat them in
> > > MDIO node? For the default MDIO bus clock speed, maybe "clock-ranges"
> > > should be used?
> >
> > It's a different clock. You wouldn't be repeating. If it's derived from
> > the FMan clock, then maybe you don't need anything here (does the driver
> > know what the divider is, or would that need to be specified in the
> > device tree?), but no more clock-frequency/bus-frequency properties.
> >
> [S.H] The purpose here is to get a specific clock frequency, driver to use it to calculate the divider.
> Then the Fman clock can be divided to the frequency.
Oh, so this is stating a desired frequency and not something that
already exists? What determines this frequency? Is it based on board
design, or just on the MDIO standard, etc? I'm wondering if the device
tree is the right place for it.
> > > [S.H] since Fman V2 & V3 can be differentiated by compatible, a
> > > boolean property "fsl,fman-internal-mdio" seems better, if defined, it
> > > indicates an internal MDIO. It looks like below:
> > >
> > > fsl,fman-internal-mdio
> > > Usage: required for internal MDIO
> > > Value type: Boolean
> > > Definition: Fman has internal MDIO for internal PCS(Physical
> > > Coding Sublayer) PHYs and external MDIO for external PHYs.
> > > The settings and programming routines for internal/external
> > > MDIO are different. Must be included for internal MDIO.
> > >
> > > How about this?
> >
> > This looks better, thanks.
> >
> > This would be set on fman v2 TBI mdio nodes as well, right?
> [S.H] Yes. But it's not used by driver. (I know I should not say this :) )
Saying it is fine as long as you still put it in the device tree. :-)
-Scott
^ permalink raw reply
* RE: [PATCH] DT: add MDIO node for FMan node
From: Shaohui Xie @ 2014-11-13 7:11 UTC (permalink / raw)
To: Scott Wood
Cc: Igal.Liberman@freescale.com, linuxppc-dev@lists.ozlabs.org,
Emilian Medve, devicetree@vger.kernel.org
In-Reply-To: <1415859399.15957.48.camel@freescale.com>
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBXb29kIFNjb3R0LUIwNzQyMQ0K
PiBTZW50OiBUaHVyc2RheSwgTm92ZW1iZXIgMTMsIDIwMTQgMjoxNyBQTQ0KPiBUbzogWGllIFNo
YW9odWktQjIxOTg5DQo+IENjOiBMaWJlcm1hbiBJZ2FsLUIzMTk1MDsgbGludXhwcGMtZGV2QGxp
c3RzLm96bGFicy5vcmc7DQo+IGRldmljZXRyZWVAdmdlci5rZXJuZWwub3JnOyBNZWR2ZSBFbWls
aWFuLUVNTUVEVkUxDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0hdIERUOiBhZGQgTURJTyBub2RlIGZv
ciBGTWFuIG5vZGUNCj4gDQo+IE9uIFdlZCwgMjAxNC0xMS0xMiBhdCAwNzo0MCAtMDYwMCwgWGll
IFNoYW9odWktQjIxOTg5IHdyb3RlOg0KPiA+ID4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0N
Cj4gPiA+IEZyb206IFdvb2QgU2NvdHQtQjA3NDIxDQo+ID4gPiBTZW50OiBXZWRuZXNkYXksIE5v
dmVtYmVyIDEyLCAyMDE0IDE6MzggQU0NCj4gPiA+IFRvOiBYaWUgU2hhb2h1aS1CMjE5ODkNCj4g
PiA+IENjOiBMaWJlcm1hbiBJZ2FsLUIzMTk1MDsgbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5v
cmc7DQo+ID4gPiBkZXZpY2V0cmVlQHZnZXIua2VybmVsLm9yZzsgTWVkdmUgRW1pbGlhbi1FTU1F
RFZFMQ0KPiA+ID4gU3ViamVjdDogUmU6IFtQQVRDSF0gRFQ6IGFkZCBNRElPIG5vZGUgZm9yIEZN
YW4gbm9kZQ0KPiA+ID4NCj4gPiA+IE9uIFR1ZSwgMjAxNC0xMS0xMSBhdCAwNDozMiAtMDYwMCwg
WGllIFNoYW9odWktQjIxOTg5IHdyb3RlOg0KPiA+ID4gPiA+IC0tLS0tT3JpZ2luYWwgTWVzc2Fn
ZS0tLS0tDQo+ID4gPiA+ID4gRnJvbTogV29vZCBTY290dC1CMDc0MjENCj4gPiA+ID4gPiBTZW50
OiBUdWVzZGF5LCBOb3ZlbWJlciAxMSwgMjAxNCA4OjIzIEFNDQo+ID4gPiA+ID4gVG86IHNoaC54
aWVAZ21haWwuY29tDQo+ID4gPiA+ID4gQ2M6IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3Jn
OyBkZXZpY2V0cmVlQHZnZXIua2VybmVsLm9yZzsNCj4gPiA+ID4gPiBNZWR2ZSBFbWlsaWFuLUVN
TUVEVkUxOyBYaWUgU2hhb2h1aS1CMjE5ODkNCj4gPiA+ID4gPiBTdWJqZWN0OiBSZTogW1BBVENI
XSBEVDogYWRkIE1ESU8gbm9kZSBmb3IgRk1hbiBub2RlDQo+ID4gPiA+ID4NCj4gPiA+ID4gPiBP
biBUdWUsIDIwMTQtMTEtMDQgYXQgMTk6NTYgKzA4MDAsIHNoaC54aWVAZ21haWwuY29tIHdyb3Rl
Og0KPiA+ID4gPiA+ID4gRnJvbTogU2hhb2h1aSBYaWUgPFNoYW9odWkuWGllQGZyZWVzY2FsZS5j
b20+DQo+ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gVGhpcyBiaW5kaW5nIGlzIGZvciBGTWFuIE1E
SU8sIGl0IGNvdmVycyBGTWFuIHYyICYgRk1hbiB2My4NCj4gPiA+ID4gPiA+DQo+ID4gPiA+ID4g
PiBTaWduZWQtb2ZmLWJ5OiBTaGFvaHVpIFhpZSA8U2hhb2h1aS5YaWVAZnJlZXNjYWxlLmNvbT4N
Cj4gPiA+ID4gPiA+IC0tLQ0KPiA+ID4gPiA+ID4gYmFzZWQgb24gaHR0cDovL3BhdGNod29yay5v
emxhYnMub3JnL3BhdGNoLzM5MDM1MS8NCj4gPiA+ID4gPiA+IGZvciAnbmV4dCcgb2YNCj4gPiA+
ID4gPiA+IGdpdDovL2dpdC5rZXJuZWwub3JnL3B1Yi9zY20vbGludXgva2VybmVsL2dpdC9zY290
dHdvb2QvbGludXguDQo+ID4gPiA+ID4gPiBnaXQNCj4gPiA+ID4gPg0KPiA+ID4gPiA+IEFyZSB0
aGVyZSBhbnkgb3RoZXIgRk1hbiBwaWVjZXMgdGhhdCBhcmUgbWlzc2luZyBmcm9tIHRoZSBhYm92
ZQ0KPiBwYXRjaD8NCj4gPiA+ID4gW1MuSF0gSSdtIGFkZGluZyBJZ2FsIGZvciB0aGlzIGNvbW1l
bnQuDQo+ID4gPiA+DQo+ID4gPiA+ID4NCj4gPiA+ID4gPiA+ICstIGJ1cy1mcmVxdWVuY3kNCj4g
PiA+ID4gPiA+ICsJCVVzYWdlOiBvcHRpb25hbA0KPiA+ID4gPiA+ID4gKwkJVmFsdWUgdHlwZTog
PHUzMj4NCj4gPiA+ID4gPiA+ICsJCURlZmluaXRpb246IERlZmF1bHQgTURJTyBidXMgY2xvY2sg
c3BlZWQuDQo+ID4gPiA+ID4NCj4gPiA+ID4gPiBVc2UgY2xvY2tzL2Nsb2NrLW5hbWVzDQo+ID4g
PiA+IFtTLkhdIFRoZSBNRElPIHVzZXMgRm1hbiBjbG9jayBhbmQgZGl2aWRlcyBpdCB0byBhIHBy
b3BlciB2YWx1ZQ0KPiA+ID4gPiB3aGljaA0KPiA+ID4gaXMgc3BlY2lmaWVkIGJ5IHRoaXMgcHJv
cGVydHkuDQo+ID4gPg0KPiA+ID4gVXNlIGNsb2Nrcy9jbG9jay1uYW1lcyB0byBkZXNjcmliZSB0
aGF0IHJlbGF0aW9uc2hpcC4NCj4gPiA+DQo+ID4NCj4gPiBbUy5IXSBUaGUgTURJTyBub2RlIGlz
IHN1Yi1ub2RlIGFuZCBlbWJlZGRlZCBpbiBGbWFuIG5vZGUsIHRoZQ0KPiA+IGNsb2Nrcy9jbG9j
ay1uYW1lcyBpcyBwcm92aWRlZCBieSBGbWFuIG5vZGUsIHNob3VsZCByZXBlYXQgdGhlbSBpbg0K
PiA+IE1ESU8gbm9kZT8gRm9yIHRoZSBkZWZhdWx0IE1ESU8gYnVzIGNsb2NrIHNwZWVkLCBtYXli
ZSAiY2xvY2stcmFuZ2VzIg0KPiA+IHNob3VsZCBiZSB1c2VkPw0KPiANCj4gSXQncyBhIGRpZmZl
cmVudCBjbG9jay4gIFlvdSB3b3VsZG4ndCBiZSByZXBlYXRpbmcuICBJZiBpdCdzIGRlcml2ZWQg
ZnJvbQ0KPiB0aGUgRk1hbiBjbG9jaywgdGhlbiBtYXliZSB5b3UgZG9uJ3QgbmVlZCBhbnl0aGlu
ZyBoZXJlIChkb2VzIHRoZSBkcml2ZXINCj4ga25vdyB3aGF0IHRoZSBkaXZpZGVyIGlzLCBvciB3
b3VsZCB0aGF0IG5lZWQgdG8gYmUgc3BlY2lmaWVkIGluIHRoZQ0KPiBkZXZpY2UgdHJlZT8pLCBi
dXQgbm8gbW9yZSBjbG9jay1mcmVxdWVuY3kvYnVzLWZyZXF1ZW5jeSBwcm9wZXJ0aWVzLg0KPiAN
CltTLkhdIFRoZSBwdXJwb3NlIGhlcmUgaXMgdG8gZ2V0IGEgc3BlY2lmaWMgY2xvY2sgZnJlcXVl
bmN5LCBkcml2ZXIgdG8gdXNlIGl0IHRvIGNhbGN1bGF0ZSB0aGUgZGl2aWRlci4NClRoZW4gdGhl
IEZtYW4gY2xvY2sgY2FuIGJlIGRpdmlkZWQgdG8gdGhlIGZyZXF1ZW5jeS4NCg0KPiA+IFtTLkhd
IHNpbmNlIEZtYW4gVjIgJiBWMyBjYW4gYmUgZGlmZmVyZW50aWF0ZWQgYnkgY29tcGF0aWJsZSwg
YQ0KPiA+IGJvb2xlYW4gcHJvcGVydHkgImZzbCxmbWFuLWludGVybmFsLW1kaW8iIHNlZW1zIGJl
dHRlciwgaWYgZGVmaW5lZCwgaXQNCj4gPiBpbmRpY2F0ZXMgYW4gaW50ZXJuYWwgTURJTy4gSXQg
bG9va3MgbGlrZSBiZWxvdzoNCj4gPg0KPiA+IGZzbCxmbWFuLWludGVybmFsLW1kaW8NCj4gPiAJ
CVVzYWdlOiByZXF1aXJlZCBmb3IgaW50ZXJuYWwgTURJTw0KPiA+IAkJVmFsdWUgdHlwZTogQm9v
bGVhbg0KPiA+IAkJRGVmaW5pdGlvbjogRm1hbiBoYXMgaW50ZXJuYWwgTURJTyBmb3IgaW50ZXJu
YWwgUENTKFBoeXNpY2FsDQo+ID4gCQlDb2RpbmcgU3VibGF5ZXIpIFBIWXMgYW5kIGV4dGVybmFs
IE1ESU8gZm9yIGV4dGVybmFsIFBIWXMuDQo+ID4gCQlUaGUgc2V0dGluZ3MgYW5kIHByb2dyYW1t
aW5nIHJvdXRpbmVzIGZvciBpbnRlcm5hbC9leHRlcm5hbA0KPiA+IAkJTURJTyBhcmUgZGlmZmVy
ZW50LiBNdXN0IGJlIGluY2x1ZGVkIGZvciBpbnRlcm5hbCBNRElPLg0KPiA+DQo+ID4gSG93IGFi
b3V0IHRoaXM/DQo+IA0KPiBUaGlzIGxvb2tzIGJldHRlciwgdGhhbmtzLg0KPiANCj4gVGhpcyB3
b3VsZCBiZSBzZXQgb24gZm1hbiB2MiBUQkkgbWRpbyBub2RlcyBhcyB3ZWxsLCByaWdodD8NCltT
LkhdIFllcy4gQnV0IGl0J3Mgbm90IHVzZWQgYnkgZHJpdmVyLiAoSSBrbm93IEkgc2hvdWxkIG5v
dCBzYXkgdGhpcyA6KSApDQoNClRoYW5rcy4NClNoYW9odWkNCg0K
^ permalink raw reply
* Re: powerpc/powernv: Support OPAL requested heartbeat
From: Jeremy Kerr @ 2014-11-13 6:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <1415860935.666.0.camel@kernel.crashing.org>
Hi Ben,
>> I'd assume "freq" with no units is in HZ, but looks like it's
>> milliseconds. I guess it's too late to rename it.
>
> We still can, we haven't cut an official build with that fw, jk ?
Not yet. You got in by a matter of minutes :)
Can you send me an updated skiboot patch too then?
Cheers,
Jeremy
^ permalink raw reply
* Re: [PATCH] usb: phy: fsl: Fix build errors
From: Peter Chen @ 2014-11-13 5:05 UTC (permalink / raw)
To: Felipe Balbi, leoli, linuxppc-dev, suresh.gupta
Cc: antoine.tenart, Linux USB Mailing List
In-Reply-To: <1415803146-6510-1-git-send-email-balbi@ti.com>
On Wed, Nov 12, 2014 at 08:39:06AM -0600, Felipe Balbi wrote:
> commit e47d925 (usb: move the OTG state from
> the USB PHY to the OTG structure) moved the
> OTG state from struct usb_phy to struct usb_otg.
>
> Unfortunately, even though I fixed quite a few
> build regressions with that patch already, this
> one was still missing.
>
> Note that this driver still has other randconfig
> build problems which I'll leave for driver author
> to fix, as that's less trivial.
Add more guys who may use this driver.
>
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>
> The following build error are left for Freescale folks
> since they seem to be a broken for quite a long time.
>
> drivers/usb/phy/phy-fsl-usb.c: In function ‘usb_otg_start’:
> drivers/usb/phy/phy-fsl-usb.c:918:3: error: ‘_fsl_readl’ undeclared (first use in this function)
> _fsl_readl = _fsl_readl_be;
> ^
> drivers/usb/phy/phy-fsl-usb.c:918:3: note: each undeclared identifier is reported only once for each function it appears in
> drivers/usb/phy/phy-fsl-usb.c:918:16: error: ‘_fsl_readl_be’ undeclared (first use in this function)
> _fsl_readl = _fsl_readl_be;
> ^
> drivers/usb/phy/phy-fsl-usb.c:919:3: error: ‘_fsl_writel’ undeclared (first use in this function)
> _fsl_writel = _fsl_writel_be;
> ^
> drivers/usb/phy/phy-fsl-usb.c:919:17: error: ‘_fsl_writel_be’ undeclared (first use in this function)
> _fsl_writel = _fsl_writel_be;
> ^
> drivers/usb/phy/phy-fsl-usb.c:921:16: error: ‘_fsl_readl_le’ undeclared (first use in this function)
> _fsl_readl = _fsl_readl_le;
> ^
> drivers/usb/phy/phy-fsl-usb.c:922:17: error: ‘_fsl_writel_le’ undeclared (first use in this function)
> _fsl_writel = _fsl_writel_le;
> ^
> drivers/usb/phy/phy-fsl-usb.c | 14 +++++++-------
> drivers/usb/phy/phy-fsl-usb.h | 2 +-
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
> index b7f36b2..ab38aa3 100644
> --- a/drivers/usb/phy/phy-fsl-usb.c
> +++ b/drivers/usb/phy/phy-fsl-usb.c
> @@ -274,7 +274,7 @@ void b_srp_end(unsigned long foo)
> fsl_otg_dischrg_vbus(0);
> srp_wait_done = 1;
>
> - if ((fsl_otg_dev->phy.state == OTG_STATE_B_SRP_INIT) &&
> + if ((fsl_otg_dev->phy.otg->state == OTG_STATE_B_SRP_INIT) &&
> fsl_otg_dev->fsm.b_sess_vld)
> fsl_otg_dev->fsm.b_srp_done = 1;
> }
> @@ -624,7 +624,7 @@ static int fsl_otg_set_host(struct usb_otg *otg, struct usb_bus *host)
> /* Mini-A cable connected */
> struct otg_fsm *fsm = &otg_dev->fsm;
>
> - otg.state = OTG_STATE_UNDEFINED;
> + otg->state = OTG_STATE_UNDEFINED;
> fsm->protocol = PROTO_UNDEF;
> }
> }
> @@ -682,7 +682,7 @@ static int fsl_otg_set_power(struct usb_phy *phy, unsigned mA)
> {
> if (!fsl_otg_dev)
> return -ENODEV;
> - if (phy->otg.state == OTG_STATE_B_PERIPHERAL)
> + if (phy->otg->state == OTG_STATE_B_PERIPHERAL)
> pr_info("FSL OTG: Draw %d mA\n", mA);
>
> return 0;
> @@ -715,7 +715,7 @@ static int fsl_otg_start_srp(struct usb_otg *otg)
> {
> struct fsl_otg *otg_dev;
>
> - if (!otg || otg.state != OTG_STATE_B_IDLE)
> + if (!otg || otg->state != OTG_STATE_B_IDLE)
> return -ENODEV;
>
> otg_dev = container_of(otg->usb_phy, struct fsl_otg, phy);
> @@ -990,10 +990,10 @@ int usb_otg_start(struct platform_device *pdev)
> * Also: record initial state of ID pin
> */
> if (fsl_readl(&p_otg->dr_mem_map->otgsc) & OTGSC_STS_USB_ID) {
> - p_otg->phy->otg.state = OTG_STATE_UNDEFINED;
> + p_otg->phy.otg->state = OTG_STATE_UNDEFINED;
> p_otg->fsm.id = 1;
> } else {
> - p_otg->phy->otg.state = OTG_STATE_A_IDLE;
> + p_otg->phy.otg->state = OTG_STATE_A_IDLE;
> p_otg->fsm.id = 0;
> }
>
> @@ -1048,7 +1048,7 @@ static int show_fsl_usb2_otg_state(struct device *dev,
> /* State */
> t = scnprintf(next, size,
> "OTG state: %s\n\n",
> - usb_otg_state_string(fsl_otg_dev->phy.state));
> + usb_otg_state_string(fsl_otg_dev->phy.otg->state));
> size -= t;
> next += t;
>
> diff --git a/drivers/usb/phy/phy-fsl-usb.h b/drivers/usb/phy/phy-fsl-usb.h
> index 5986c96..2314995 100644
> --- a/drivers/usb/phy/phy-fsl-usb.h
> +++ b/drivers/usb/phy/phy-fsl-usb.h
> @@ -298,7 +298,7 @@
> /* SE0 Time Before SRP */
> #define TB_SE0_SRP (2) /* b_idle,minimum 2 ms, section:5.3.2 */
>
> -#define SET_OTG_STATE(otg_ptr, newstate) ((otg_ptr)->state = newstate)
> +#define SET_OTG_STATE(phy, newstate) ((phy)->otg->state = newstate)
>
> struct usb_dr_mmap {
> /* Capability register */
> --
> 2.1.0.GIT
>
--
Best Regards,
Peter Chen
^ permalink raw reply
* Re: powerpc/powernv: Support OPAL requested heartbeat
From: Benjamin Herrenschmidt @ 2014-11-13 6:42 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Jeremy Kerr
In-Reply-To: <20141113052937.17DFF1400D2@ozlabs.org>
On Thu, 2014-11-13 at 16:29 +1100, Michael Ellerman wrote:
> I'd assume "freq" with no units is in HZ, but looks like it's
> milliseconds. I guess it's too late to rename it.
We still can, we haven't cut an official build with that fw, jk ?
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH V2] powerpc/TM: Disable/Enable TM looking at the ibm, pa-features device tree entry
From: Aneesh Kumar K.V @ 2014-11-13 6:19 UTC (permalink / raw)
To: Michael Ellerman; +Cc: paulus, Michael Neuling, linuxppc-dev
In-Reply-To: <1415857347.28703.8.camel@concordia>
Michael Ellerman <mpe@ellerman.id.au> writes:
> On Wed, 2014-11-12 at 11:09 +0530, Aneesh Kumar K.V wrote:
>> Michael Neuling <mikey@neuling.org> writes:
>>
>> > On Sun, 2014-11-02 at 20:02 +0530, Aneesh Kumar K.V wrote:
>> >> Runtime disable transactional memory feature looking at pa-features
>> >> device tree entry. We need to do this so that we can run a kernel
>> >> built with TM config in PR mode.
>> >
>> > I'm happy to turn this off but why do we need to do this in PR mode?
>> > Can you explain this in the commit message.
>>
>> Hmm, that commit message needs an update. I initially did the patch for
>> P8 PR support and wanted a mechanism to disable TM. Alex added basic TM
>> support for PR mode after that. So we can drop the PR part of the
>> commit message.
>>
>> Michael Ellerman,
>>
>> Let me know if you want me to send an updated version with the those
>> part of the commit message dropped
>
> How about:
>
> powerpc: Disable CPU_FTR_TM if TM is disabled by firmware
>
> Firmware is allowed to communicate to us via the "ibm,pa-features" property
> that TM (Transactional Memory) support is disabled.
>
> Currently this doesn't happen on any platform we're aware of, but we should
> honor it anyway.
>
Looks good.
-aneesh
^ permalink raw reply
* Re: [PATCH] DT: add MDIO node for FMan node
From: Scott Wood @ 2014-11-13 6:16 UTC (permalink / raw)
To: Xie Shaohui-B21989
Cc: Liberman Igal-B31950, linuxppc-dev@lists.ozlabs.org,
Medve Emilian-EMMEDVE1, devicetree@vger.kernel.org
In-Reply-To: <c4dd11a97a684be8b799c2f639e600ca@DM2PR0301MB0864.namprd03.prod.outlook.com>
On Wed, 2014-11-12 at 07:40 -0600, Xie Shaohui-B21989 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, November 12, 2014 1:38 AM
> > To: Xie Shaohui-B21989
> > Cc: Liberman Igal-B31950; linuxppc-dev@lists.ozlabs.org;
> > devicetree@vger.kernel.org; Medve Emilian-EMMEDVE1
> > Subject: Re: [PATCH] DT: add MDIO node for FMan node
> >
> > On Tue, 2014-11-11 at 04:32 -0600, Xie Shaohui-B21989 wrote:
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Tuesday, November 11, 2014 8:23 AM
> > > > To: shh.xie@gmail.com
> > > > Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org; Medve
> > > > Emilian-EMMEDVE1; Xie Shaohui-B21989
> > > > Subject: Re: [PATCH] DT: add MDIO node for FMan node
> > > >
> > > > On Tue, 2014-11-04 at 19:56 +0800, shh.xie@gmail.com wrote:
> > > > > From: Shaohui Xie <Shaohui.Xie@freescale.com>
> > > > >
> > > > > This binding is for FMan MDIO, it covers FMan v2 & FMan v3.
> > > > >
> > > > > Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> > > > > ---
> > > > > based on http://patchwork.ozlabs.org/patch/390351/
> > > > > for 'next' of
> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git
> > > >
> > > > Are there any other FMan pieces that are missing from the above patch?
> > > [S.H] I'm adding Igal for this comment.
> > >
> > > >
> > > > > +- bus-frequency
> > > > > + Usage: optional
> > > > > + Value type: <u32>
> > > > > + Definition: Default MDIO bus clock speed.
> > > >
> > > > Use clocks/clock-names
> > > [S.H] The MDIO uses Fman clock and divides it to a proper value which
> > is specified by this property.
> >
> > Use clocks/clock-names to describe that relationship.
> >
>
> [S.H] The MDIO node is sub-node and embedded in Fman node, the
> clocks/clock-names is provided by Fman node, should repeat them in MDIO
> node? For the default MDIO bus clock speed, maybe "clock-ranges" should
> be used?
It's a different clock. You wouldn't be repeating. If it's derived
from the FMan clock, then maybe you don't need anything here (does the
driver know what the divider is, or would that need to be specified in
the device tree?), but no more clock-frequency/bus-frequency properties.
> [S.H] since Fman V2 & V3 can be differentiated by compatible, a boolean
> property "fsl,fman-internal-mdio" seems better, if defined, it
> indicates an internal MDIO. It looks like below:
>
> fsl,fman-internal-mdio
> Usage: required for internal MDIO
> Value type: Boolean
> Definition: Fman has internal MDIO for internal PCS(Physical
> Coding Sublayer) PHYs and external MDIO for external PHYs.
> The settings and programming routines for internal/external
> MDIO are different. Must be included for internal MDIO.
>
> How about this?
This looks better, thanks.
This would be set on fman v2 TBI mdio nodes as well, right?
-Scott
^ permalink raw reply
* Re: [PATCH V2] powerpc/TM: Disable/Enable TM looking at the ibm, pa-features device tree entry
From: Michael Ellerman @ 2014-11-13 5:42 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: linuxppc-dev, Michael Neuling, paulus
In-Reply-To: <87d28tasfe.fsf@linux.vnet.ibm.com>
On Wed, 2014-11-12 at 11:09 +0530, Aneesh Kumar K.V wrote:
> Michael Neuling <mikey@neuling.org> writes:
>
> > On Sun, 2014-11-02 at 20:02 +0530, Aneesh Kumar K.V wrote:
> >> Runtime disable transactional memory feature looking at pa-features
> >> device tree entry. We need to do this so that we can run a kernel
> >> built with TM config in PR mode.
> >
> > I'm happy to turn this off but why do we need to do this in PR mode?
> > Can you explain this in the commit message.
>
> Hmm, that commit message needs an update. I initially did the patch for
> P8 PR support and wanted a mechanism to disable TM. Alex added basic TM
> support for PR mode after that. So we can drop the PR part of the
> commit message.
>
> Michael Ellerman,
>
> Let me know if you want me to send an updated version with the those
> part of the commit message dropped
How about:
powerpc: Disable CPU_FTR_TM if TM is disabled by firmware
Firmware is allowed to communicate to us via the "ibm,pa-features" property
that TM (Transactional Memory) support is disabled.
Currently this doesn't happen on any platform we're aware of, but we should
honor it anyway.
cheers
^ permalink raw reply
* Re: [PATCH] i2c: Driver to expose PowerNV platform i2c busses
From: Michael Ellerman @ 2014-11-13 5:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Neelesh Gupta, linuxppc-dev, wsa, linux-i2c, Jeremy Kerr
In-Reply-To: <1415772441.5124.39.camel@kernel.crashing.org>
On Wed, 2014-11-12 at 17:07 +1100, Benjamin Herrenschmidt wrote:
> On Mon, 2014-11-10 at 11:35 +0530, Neelesh Gupta wrote:
> > The patch exposes the available i2c busses on the PowerNV platform
> > to the kernel and implements the bus driver to support i2c and
> > smbus commands.
> > The driver uses the platform device infrastructure to probe the busses
> > on the platform and registers them with the i2c driver framework.
> >
> > Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com>
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
> This version slightly modified removes the unrelated gunk in opal.c
> (but needs to apply on top of some other patches in the powerpc tree)
>
> The driver is the same, it's only the
> arch/powerpc/platform/powernv/opal.c init bits that get cleaned up and
> simplified.
>
> From: Neelesh Gupta <neelegup@linux.vnet.ibm.com>
> Date: Fri, 7 Nov 2014 16:20:07 +1100
> Subject: [PATCH v2] i2c: Driver to expose PowerNV platform i2c busses
>
> The patch exposes the available i2c busses on the PowerNV platform
> to the kernel and implements the bus driver to support i2c and
> smbus commands.
>
> If the devices are found on the device tree for a given bus/adapter,
> the platform init code registers them to the core and binds them
> a static bus/adapter number, otherwise the driver registers the
> adapter to get the adapter number dynamically.
So the driver part depends on the arch/powerpc changes.
I added Wolfram the i2c maintainer to CC. Wolfram are you happy if we take this
through the powerpc tree with your ack, or I can do a topic branch?
cheers
^ permalink raw reply
* Re: powerpc/powernv: Support OPAL requested heartbeat
From: Michael Ellerman @ 2014-11-13 5:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, linuxppc-dev; +Cc: Jeremy Kerr
In-Reply-To: <1415772194.5124.37.camel@kernel.crashing.org>
On Wed, 2014-12-11 at 06:03:14 UTC, Benjamin Herrenschmidt wrote:
> If OPAL requests it, call it back via opal_poll_events() at a
> regular interval. Some versions of OPAL on some machines require
> this to operate some internal timeouts properly.
>
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index f1e0d8c..0153064 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -678,6 +680,49 @@ static int __init opal_init(void)
> " (0x%x)\n", rc, irq, hwirq);
> opal_irqs[i] = irq;
> }
> +}
> +
> +static int kopald(void *unused)
> +{
> + set_freezable();
> + do {
> + try_to_freeze();
> + opal_poll_events(NULL);
> + msleep_interruptible(opal_heartbeat);
> + } while (!kthread_should_stop());
> +
> + return 0;
> +}
> +
> +static void opal_init_heartbeat(void)
> +{
> + /* Old firwmware, we assume the HVC heartbeat is sufficient */
> + if (of_property_read_u32(opal_node, "ibm,heartbeat-freq",
> + &opal_heartbeat) != 0)
I'd assume "freq" with no units is in HZ, but looks like it's milliseconds. I
guess it's too late to rename it.
cheers
^ permalink raw reply
* Re: [PATCH v3 4/7] sound/radeon: Add quirk for broken 64-bit MSI
From: Michael Ellerman @ 2014-11-13 5:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Dave Airlie, Linux PCI, Brian King, Anton Blanchard, linuxppc-dev,
Yijing Wang, Takashi Iwai, Bjorn Helgaas, Alex Deucher
In-Reply-To: <1415765215.5124.30.camel@kernel.crashing.org>
On Wed, 2014-11-12 at 15:06 +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2014-11-12 at 13:23 +1100, Michael Ellerman wrote:
> > On Tue, 2014-11-11 at 14:12 -0700, Bjorn Helgaas wrote:
> > > On Thu, Oct 16, 2014 at 09:55:32AM +1100, Benjamin Herrenschmidt wrote:
> > > > On Wed, 2014-10-15 at 16:19 -0600, Bjorn Helgaas wrote:
> > > > > PCI/MSI: Add device flag indicating that 64-bit MSIs don't work
> > >
> > > I'm still assuming you're going to merge this series, but I don't see it in
> > > your tree (https://git.kernel.org/cgit/linux/kernel/git/benh/powerpc.git/)
> > > yet. Do you want me to do anything with it?
> >
> > I'm doing the powerpc tree this cycle, so it'd be in my tree, but it's not:
> >
> > https://git.kernel.org/cgit/linux/kernel/git/mpe/linux.git/
> >
> > Ben if you want me to take it let me know.
>
> Hrm, I might have dropped the ball accidentally here. Bjorn did you
> actually Ack the core changes ? In that case we should probably pick it
> up.
I tried piecing together this series with v2 of patch 3 as you described, I
think, but it didn't apply cleanly, and I'm not confident I won't screw it up.
So please resend the series the way you want it to go in.
You got an ack from Bjorn:
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
And he asked you change the subject on patch 2 to:
PCI/MSI: Add device flag indicating that 64-bit MSIs don't work
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: mitigate impact of decrementer reset
From: Michael Ellerman @ 2014-11-13 2:42 UTC (permalink / raw)
To: Paul Clarke; +Cc: paulmck, linuxppc-dev
In-Reply-To: <546126DC.6090909@us.ibm.com>
On Mon, 2014-11-10 at 14:58 -0600, Paul Clarke wrote:
> On 11/10/2014 04:08 AM, Benjamin Herrenschmidt wrote:
> > On Tue, 2014-10-07 at 14:13 -0500, Paul Clarke wrote:
> >> This patch short-circuits the reset of the decrementer, exiting after
> >> the decrementer reset, but before the housekeeping tasks if the only
> >> need for the interrupt is simply to reset it. After this patch,
> >> the latency spike was measured at about 150 nanoseconds.
> >
> > Doesn't this break the irq_work stuff ? We trigger it with a set_dec(1);
> > and your patch will probably cause it to be skipped...
>
> You're right.
Yeah, thanks Ben, that would have been bad.
So we'll need to come up with a different approach.
> I'm confused by the division between timer_interrupt() and
> __timer_interrupt(). The former is called with interrupts disabled (and
> enables them), but also calls irq_enter()/irq_exit(). Why are those
> calls not in __timer_interrupt()? (If they were, the short-circuit
> logic might be a bit easier to put directly in __timer_interrupt(),
> which would eliminate any duplicate code.)
>
> It looks like __timer_interrupt is only called directly by the broadcast
> timer IPI handler. (Why is __timer_interrupt not static?) Does this
> path not need irq_enter/irq_exit?
I think I answered most of this in the other mail I just sent, but let me know
if not.
And __timer_interrupt() is static, if you have a new enough kernel :)
cheers
^ permalink raw reply
* Re: powerpc: mitigate impact of decrementer reset
From: Michael Ellerman @ 2014-11-13 2:39 UTC (permalink / raw)
To: Paul Clarke; +Cc: linuxppc-dev
In-Reply-To: <545A591F.3080400@us.ibm.com>
On Wed, 2014-11-05 at 11:06 -0600, Paul Clarke wrote:
> Sorry it took me so long to get back to this...
>
> On 10/07/2014 09:52 PM, Michael Ellerman wrote:
> > On Tue, 2014-07-10 at 19:13:24 UTC, Paul Clarke wrote:
> >> This patch short-circuits the reset of the decrementer, exiting after
> >> the decrementer reset, but before the housekeeping tasks if the only
> >> need for the interrupt is simply to reset it. After this patch,
> >> the latency spike was measured at about 150 nanoseconds.
>
> > Thanks for the excellent changelog. But this patch makes me a bit nervous :)
> >
> > Do you know where the latency is coming from? Is it primarily the irq work?
>
> Yes, it is all under irq_enter (measured at ~10us) and irq_exit (~12us).
Hmm, OK. I actually meant irq_work_run().
AIUI irq_enter/exit() are just state tracking, they shouldn't be actually
running work.
How are you measuring it?
> > If so I'd prefer if we could move the short circuit into __timer_interrupt()
> > itself. That way we'd still have the trace points usable, and it would
> > hopefully result in less duplicated logic.
>
> But irq_enter and irq_exit are called in timer_interrupt, before
> __timer_interrupt is called. I don't see how that helps. The time
> spent in __timer_interrupt is minuscule by comparison.
Right, it won't help if it's irq_enter() that is causing the delay. But I was
assuming it was irq_work_run().
> Are you suggesting that irq_enter/exit be moved into __timer_interrupt
> as well? (I'm not sure how that would impact the existing call to
> __timer_interrupt from tick_broadcast_ipi_handler? And if there is no
> impact, what's the point of separating timer_interrupt and
> __timer_interrupt?)
The point is __timer_interrupt() is called from tick_broadcast_ipi_handler(),
which is called from smp_ipi_demux(), from icp_hv_ipi_action(), from
__do_irq(), which has already done irq_enter() (and will do irq_exit()).
cheers
^ permalink raw reply
* Re: [PATCH] i2c-qoriq: modified compatibility for correct prescaler
From: Wolfram Sang @ 2014-11-13 0:34 UTC (permalink / raw)
To: Valentin Longchamp
Cc: Linux device trees, Boschung, Rainer, Brunck, Holger, Linux I2C,
Scott Wood, Linux PowerPC Kernel
In-Reply-To: <5450AC85.40302@keymile.com>
[-- Attachment #1: Type: text/plain, Size: 333 bytes --]
> If we wanted to be on the safe side and strict (since we are not sure that the
> hardware is 100% compatible), we maybe should add a fsl,qoriq-i2c compatible to
> the driver that does the same as mpc8543-i2c.
Or you leave the driver as is and use both compatibles:
compatible = "fsl,qoriq-i2c", "fsl,mpc8543-i2c", "fsl-i2c";
?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] of/base: Fix PowerPC address parsing hack
From: Stephen Rothwell @ 2014-11-13 1:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: devicetree@vger.kernel.org, Arnd Bergmann, linuxppc-dev,
linux-kernel@vger.kernel.org, Olof Johansson, Rob Herring,
Grant Likely
In-Reply-To: <1415839522.5124.58.camel@kernel.crashing.org>
[-- Attachment #1: Type: text/plain, Size: 409 bytes --]
Hi Ben,
On Thu, 13 Nov 2014 11:45:22 +1100 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> What about this one instead ? I want to cache it because that function
> can be called quite a while and doing two additional property lookup
> and string compares every time might hurt some platforms.
Looks good to me.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] of/base: Fix PowerPC address parsing hack
From: Benjamin Herrenschmidt @ 2014-11-13 0:45 UTC (permalink / raw)
To: Stephen Rothwell
Cc: devicetree@vger.kernel.org, Arnd Bergmann, linuxppc-dev,
linux-kernel@vger.kernel.org, Olof Johansson, Rob Herring,
Grant Likely
In-Reply-To: <1415833725.5124.53.camel@kernel.crashing.org>
What about this one instead ? I want to cache it because that function
can be called quite a while and doing two additional property lookup
and string compares every time might hurt some platforms.
----
We have a historical hack that treats missing ranges properties as the
equivalent of an empty one. This is needed for ancient PowerMac "bad"
device-trees, and shouldn't be enabled for any other PowerPC platform,
otherwise we get some nasty layout of devices in sysfs or even
duplication when a set of otherwise identically named devices is
created multiple times under a different parent node with no ranges
property.
This fix is needed for the PowerNV i2c busses to be exposed properly
and will fix a number of other embedded cases.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <stable@vger.kernel.org>
diff --git a/drivers/of/address.c b/drivers/of/address.c
index e371825..5eae0cd 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -403,6 +403,17 @@ static struct of_bus *of_match_bus(struct device_node *np)
return NULL;
}
+static int of_empty_ranges_quirk(void)
+{
+ /* To save cycles, we cache the result */
+ static int quirk_state = -1;
+
+ if (quirk_state < 0)
+ quirk_state = of_machine_is_compatible("Power Macintosh") ||
+ of_machine_is_compatible("MacRISC");
+ return quirk_state;
+}
+
static int of_translate_one(struct device_node *parent, struct of_bus *bus,
struct of_bus *pbus, __be32 *addr,
int na, int ns, int pna, const char *rprop)
@@ -428,12 +439,10 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
* This code is only enabled on powerpc. --gcl
*/
ranges = of_get_property(parent, rprop, &rlen);
-#if !defined(CONFIG_PPC)
- if (ranges == NULL) {
+ if (ranges == NULL && !of_empty_ranges_quirk()) {
pr_err("OF: no ranges; cannot translate\n");
return 1;
}
-#endif /* !defined(CONFIG_PPC) */
if (ranges == NULL || rlen == 0) {
offset = of_read_number(addr, na);
memset(addr, 0, pna * 4);
^ permalink raw reply related
* [PATCH V3 1/4] kexec: Fix make headers_check
From: Geoff Levand @ 2014-11-13 0:19 UTC (permalink / raw)
To: Andrew Morton
Cc: linuxppc-dev, kexec, Eric Biederman, Vivek Goyal, linux-kernel
In-Reply-To: <cover.1415837218.git.geoff@infradead.org>
Remove the unneded declaration for a kexec_load() routine.
Fixes errors like these when running 'make headers_check':
include/uapi/linux/kexec.h: userspace cannot reference function or variable defined in the kernel
Signed-off-by: Geoff Levand <geoff@infradead.org>
Acked-by: Paul Bolle <pebolle@tiscali.nl>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
---
include/uapi/linux/kexec.h | 6 ------
1 file changed, 6 deletions(-)
diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
index 6925f5b..99048e5 100644
--- a/include/uapi/linux/kexec.h
+++ b/include/uapi/linux/kexec.h
@@ -55,12 +55,6 @@ struct kexec_segment {
size_t memsz;
};
-/* Load a new kernel image as described by the kexec_segment array
- * consisting of passed number of segments at the entry-point address.
- * The flags allow different useage types.
- */
-extern int kexec_load(void *, size_t, struct kexec_segment *,
- unsigned long int);
#endif /* __KERNEL__ */
#endif /* _UAPILINUX_KEXEC_H */
--
1.9.1
^ permalink raw reply related
* [PATCH V3 4/4] kexec: Add IND_FLAGS macro
From: Geoff Levand @ 2014-11-13 0:19 UTC (permalink / raw)
To: Andrew Morton
Cc: kexec, linux-kernel, Eric Biederman, linuxppc-dev, Vivek Goyal
In-Reply-To: <cover.1415837218.git.geoff@infradead.org>
Add a new kexec preprocessor macro IND_FLAGS, which is the bitwise OR of
all the possible kexec IND_ kimage_entry indirection flags. Having this
macro allows for simplified code in the prosessing of the kexec
kimage_entry items. Also, remove the local powerpc definition and use
the generic one.
Signed-off-by: Geoff Levand <geoff@infradead.org>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
---
arch/powerpc/kernel/machine_kexec_64.c | 2 --
include/linux/kexec.h | 1 +
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
index 879b3aa..75652a32 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -96,8 +96,6 @@ int default_machine_kexec_prepare(struct kimage *image)
return 0;
}
-#define IND_FLAGS (IND_DESTINATION | IND_INDIRECTION | IND_DONE | IND_SOURCE)
-
static void copy_segments(unsigned long ind)
{
unsigned long entry;
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 25e039c..b23412c 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -10,6 +10,7 @@
#define IND_INDIRECTION (1 << IND_INDIRECTION_BIT)
#define IND_DONE (1 << IND_DONE_BIT)
#define IND_SOURCE (1 << IND_SOURCE_BIT)
+#define IND_FLAGS (IND_DESTINATION | IND_INDIRECTION | IND_DONE | IND_SOURCE)
#if !defined(__ASSEMBLY__)
--
1.9.1
^ permalink raw reply related
* [PATCH V3 2/4] kexec: Simplify conditional
From: Geoff Levand @ 2014-11-13 0:19 UTC (permalink / raw)
To: Andrew Morton
Cc: linuxppc-dev, kexec, Eric Biederman, Vivek Goyal, linux-kernel
In-Reply-To: <cover.1415837218.git.geoff@infradead.org>
Simplify the code around one of the conditionals in the kexec_load
syscall routine.
The original code was confusing with a redundant check on KEXEC_ON_CRASH
and comments outside of the conditional block. This change switches the
order of the conditional check, and cleans up the comments for the
conditional. There is no functional change to the code.
Signed-off-by: Geoff Levand <geoff@infradead.org>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
---
kernel/kexec.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 2abf9f6..650fcba 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1288,19 +1288,22 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments,
if (nr_segments > 0) {
unsigned long i;
- /* Loading another kernel to reboot into */
- if ((flags & KEXEC_ON_CRASH) == 0)
- result = kimage_alloc_init(&image, entry, nr_segments,
- segments, flags);
- /* Loading another kernel to switch to if this one crashes */
- else if (flags & KEXEC_ON_CRASH) {
- /* Free any current crash dump kernel before
+ if (flags & KEXEC_ON_CRASH) {
+ /*
+ * Loading another kernel to switch to if this one
+ * crashes. Free any current crash dump kernel before
* we corrupt it.
*/
+
kimage_free(xchg(&kexec_crash_image, NULL));
result = kimage_alloc_init(&image, entry, nr_segments,
segments, flags);
crash_map_reserved_pages();
+ } else {
+ /* Loading another kernel to reboot into. */
+
+ result = kimage_alloc_init(&image, entry, nr_segments,
+ segments, flags);
}
if (result)
goto out;
--
1.9.1
^ permalink raw reply related
* [PATCH V3 0/4] kexec: minor fixups and enhancements
From: Geoff Levand @ 2014-11-13 0:19 UTC (permalink / raw)
To: Andrew Morton
Cc: linuxppc-dev, kexec, Eric Biederman, Vivek Goyal, linux-kernel
In-Reply-To: <cover.1408731991.git.geoff@infradead.org>
Hi Andrew,
This is essentially a resend of my patch set from October with the exception
of folding the last two patches of that series into the final patch here.
The patches here have been in review since first posted in August, and have
been acked by one or two kexec developers. Could you merge them through your
mm tree? Thanks.
-Geoff
The following changes since commit 206c5f60a3d902bc4b56dab2de3e88de5eb06108:
Linux 3.18-rc4 (2014-11-09 14:55:29 -0800)
are available in the git repository at:
git://git.linaro.org/people/geoff.levand/linux-kexec.git for-kexec
for you to fetch changes up to 916ef4b8abaa4202ea43731b9cd3f8e0ea95f05b:
kexec: Add IND_FLAGS macro (2014-11-12 15:27:54 -0800)
----------------------------------------------------------------
Geoff Levand (4):
kexec: Fix make headers_check
kexec: Simplify conditional
kexec: Add bit definitions for kimage entry flags
kexec: Add IND_FLAGS macro
arch/powerpc/kernel/machine_kexec_64.c | 2 --
include/linux/kexec.h | 20 ++++++++++++++++----
include/uapi/linux/kexec.h | 6 ------
kernel/kexec.c | 17 ++++++++++-------
4 files changed, 26 insertions(+), 19 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH V3 3/4] kexec: Add bit definitions for kimage entry flags
From: Geoff Levand @ 2014-11-13 0:19 UTC (permalink / raw)
To: Andrew Morton
Cc: linuxppc-dev, kexec, Eric Biederman, Vivek Goyal, linux-kernel
In-Reply-To: <cover.1415837218.git.geoff@infradead.org>
Define new kexec preprocessor macros IND_*_BIT that define the bit position of
the kimage entry flags. Change the existing IND_* flag macros to be defined as
bit shifts of the corresponding IND_*_BIT macros. Also wrap all C language code
in kexec.h with #if !defined(__ASSEMBLY__) so assembly files can include kexec.h
to get the IND_* and IND_*_BIT macros.
Some CPU instruction sets have tests for bit position which are convenient in
implementing routines that operate on the kimage entry list. The addition of
these bit position macros in a common location will avoid duplicate definitions
and the chance that changes to the IND_* flags will not be propagated to
assembly files.
Signed-off-by: Geoff Levand <geoff@infradead.org>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
---
include/linux/kexec.h | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 9d957b7..25e039c 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -1,6 +1,18 @@
#ifndef LINUX_KEXEC_H
#define LINUX_KEXEC_H
+#define IND_DESTINATION_BIT 0
+#define IND_INDIRECTION_BIT 1
+#define IND_DONE_BIT 2
+#define IND_SOURCE_BIT 3
+
+#define IND_DESTINATION (1 << IND_DESTINATION_BIT)
+#define IND_INDIRECTION (1 << IND_INDIRECTION_BIT)
+#define IND_DONE (1 << IND_DONE_BIT)
+#define IND_SOURCE (1 << IND_SOURCE_BIT)
+
+#if !defined(__ASSEMBLY__)
+
#include <uapi/linux/kexec.h>
#ifdef CONFIG_KEXEC
@@ -64,10 +76,6 @@
*/
typedef unsigned long kimage_entry_t;
-#define IND_DESTINATION 0x1
-#define IND_INDIRECTION 0x2
-#define IND_DONE 0x4
-#define IND_SOURCE 0x8
struct kexec_segment {
/*
@@ -313,4 +321,7 @@ struct task_struct;
static inline void crash_kexec(struct pt_regs *regs) { }
static inline int kexec_should_crash(struct task_struct *p) { return 0; }
#endif /* CONFIG_KEXEC */
+
+#endif /* !defined(__ASSEBMLY__) */
+
#endif /* LINUX_KEXEC_H */
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox