From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752261AbbDOI1S (ORCPT ); Wed, 15 Apr 2015 04:27:18 -0400 Received: from mail-by2on0110.outbound.protection.outlook.com ([207.46.100.110]:52096 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751276AbbDOI1H (ORCPT ); Wed, 15 Apr 2015 04:27:07 -0400 From: "Shengzhou.Liu@freescale.com" To: Florian Fainelli , "netdev@vger.kernel.org" , "davem@davemloft.net" CC: "linux-kernel@vger.kernel.org" Subject: RE: [PATCH v2] net/phy: tune get_phy_c45_ids to support more c45 phy Thread-Topic: [PATCH v2] net/phy: tune get_phy_c45_ids to support more c45 phy Thread-Index: AQHQdpucZY0bwhXwQkWOrAM0PKeak51MyESAgADkifA= Date: Wed, 15 Apr 2015 08:27:05 +0000 Message-ID: References: <1429006192-21403-1-git-send-email-Shengzhou.Liu@freescale.com> <552D5221.1030703@gmail.com> In-Reply-To: <552D5221.1030703@gmail.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: gmail.com; dkim=none (message not signed) header.d=none; x-originating-ip: [192.158.241.86] x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM2PR03MB399; x-forefront-antispam-report: BMV:1;SFV:NSPM;SFS:(10019020)(6009001)(51704005)(479174004)(24454002)(377454003)(77096005)(102836002)(62966003)(77156002)(2501003)(99286002)(4001410100001)(66066001)(87936001)(2656002)(2201001)(2950100001)(40100003)(2900100001)(33656002)(92566002)(122556002)(86362001)(76576001)(19580395003)(19580405001)(76176999)(50986999)(54356999)(46102003)(106116001)(74316001);DIR:OUT;SFP:1102;SCL:1;SRVR:DM2PR03MB399;H:DM2PR03MB398.namprd03.prod.outlook.com;FPR:;SPF:None;MLV:sfv;LANG:en; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(5002010)(5005006);SRVR:DM2PR03MB399;BCL:0;PCL:0;RULEID:;SRVR:DM2PR03MB399; x-forefront-prvs: 0547116B72 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Apr 2015 08:27:05.1571 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR03MB399 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by nfs id t3F8RfoS014932 > -----Original Message----- > From: Florian Fainelli [mailto:f.fainelli@gmail.com] > Sent: Wednesday, April 15, 2015 1:45 AM > To: Liu Shengzhou-B36685; netdev@vger.kernel.org; davem@davemloft.net > Cc: linux-kernel@vger.kernel.org > Subject: Re: [PATCH v2] net/phy: tune get_phy_c45_ids to support more c45 phy > > On 14/04/15 03:09, Shengzhou Liu wrote: > > As some C45 10G PHYs(e.g. Cortina CS4315/CS4340 PHY) have zero Devices > > In package, current driver can't get correct devices_in_package value > > by non-zero Devices In package. > > so let's probe more with zero Devices In package to support more C45 > > PHYs. > > I cannot remember exactly how many times this patch has been posted, but it > still is not clear to me what you are doing here is helping with these > Cortina PHYs. > > Could you post a dump of the mdiobus_read() arguments and values for the old > code and the new code you are proposing? That way it might be clearer what is > the goal here? > The key point is that standard C45 PHYs use non-zero Devices in package i(reg_addr = MII_ADDR_C45 | i << 16 | MDIO_DEVS) to read devices_in_package, device zero is reserved, but Cortina CS4315/CS4340 PHY use zero Devices in package(reg_addr = MII_ADDR_C45 | 0 << 16 | MDIO_DEVS) to read devices_in_package. This is caused by Cortina non-standard PHY, e.g. standard PHY has MII_PHYSID1=0x02, MII_PHYSID2=0x03, but CS4315/CS4340 PHY has non-standard offset of PHY ID registers(MII_PHYSID1=0x00, MII_PHYSID2=0x01). > > > > Signed-off-by: Shengzhou Liu > > --- > > v2: use MDIO_DEVS1 and MDIO_DEVS2 instead of constant '6', '5' > > > > drivers/net/phy/phy_device.c | 25 +++++++++++++++++++++---- > > 1 file changed, 21 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/net/phy/phy_device.c > > b/drivers/net/phy/phy_device.c index bdfe51f..c4f836f 100644 > > --- a/drivers/net/phy/phy_device.c > > +++ b/drivers/net/phy/phy_device.c > > @@ -242,12 +242,29 @@ static int get_phy_c45_ids(struct mii_bus *bus, int > addr, u32 *phy_id, > > return -EIO; > > c45_ids->devices_in_package |= (phy_reg & 0xffff); > > > > - /* If mostly Fs, there is no device there, > > - * let's get out of here. > > + /* If mostly Fs, let's continue to probe more > > + * as some c45 PHYs have zero Devices In package, > > + * e.g. Cortina CS4315/CS4340 PHY. > > */ > > if ((c45_ids->devices_in_package & 0x1fffffff) == 0x1fffffff) { > > - *phy_id = 0xffffffff; > > - return 0; > > + reg_addr = MII_ADDR_C45 | 0 << 16 | MDIO_DEVS2; > > + phy_reg = mdiobus_read(bus, addr, reg_addr); > > + if (phy_reg < 0) > > + return -EIO; > > + c45_ids->devices_in_package = (phy_reg & 0xffff) << 16; > > + reg_addr = MII_ADDR_C45 | 0 << 16 | MDIO_DEVS1; > > + phy_reg = mdiobus_read(bus, addr, reg_addr); > > + if (phy_reg < 0) > > + return -EIO; > > + c45_ids->devices_in_package |= (phy_reg & 0xffff); > > + /* If mostly Fs, there is no device there, > > + * let's get out of here. > > + */ > > + if ((c45_ids->devices_in_package & 0x1fffffff) == > > + 0x1fffffff) { > > + *phy_id = 0xffffffff; > > + return 0; > > + } > > Could not we somehow be a bit more clever and utilize the loop, with an > adjusted i = 0 during this condition? Some something like this (untested): > > Florian I had done it to utilize the loop with i = 0 as your thought, but David Miller said that the way of utilizing the loop makes no sense to test 'i' for zero vs. non-zero until the looping construct it is contained within can actually hit a zero value, adding such a check here makes the code confusing. So I dropped the way of utilizing the loop to make code readable. {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Shengzhou.Liu@freescale.com" Subject: RE: [PATCH v2] net/phy: tune get_phy_c45_ids to support more c45 phy Date: Wed, 15 Apr 2015 08:27:05 +0000 Message-ID: References: <1429006192-21403-1-git-send-email-Shengzhou.Liu@freescale.com> <552D5221.1030703@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Cc: "linux-kernel@vger.kernel.org" To: Florian Fainelli , "netdev@vger.kernel.org" , "davem@davemloft.net" Return-path: In-Reply-To: <552D5221.1030703@gmail.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBGbG9yaWFuIEZhaW5lbGxpIFtt YWlsdG86Zi5mYWluZWxsaUBnbWFpbC5jb21dDQo+IFNlbnQ6IFdlZG5lc2RheSwgQXByaWwgMTUs IDIwMTUgMTo0NSBBTQ0KPiBUbzogTGl1IFNoZW5nemhvdS1CMzY2ODU7IG5ldGRldkB2Z2VyLmtl cm5lbC5vcmc7IGRhdmVtQGRhdmVtbG9mdC5uZXQNCj4gQ2M6IGxpbnV4LWtlcm5lbEB2Z2VyLmtl cm5lbC5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRDSCB2Ml0gbmV0L3BoeTogdHVuZSBnZXRfcGh5 X2M0NV9pZHMgdG8gc3VwcG9ydCBtb3JlIGM0NSBwaHkNCj4gDQo+IE9uIDE0LzA0LzE1IDAzOjA5 LCBTaGVuZ3pob3UgTGl1IHdyb3RlOg0KPiA+IEFzIHNvbWUgQzQ1IDEwRyBQSFlzKGUuZy4gQ29y dGluYSBDUzQzMTUvQ1M0MzQwIFBIWSkgaGF2ZSB6ZXJvIERldmljZXMNCj4gPiBJbiBwYWNrYWdl LCBjdXJyZW50IGRyaXZlciBjYW4ndCBnZXQgY29ycmVjdCBkZXZpY2VzX2luX3BhY2thZ2UgdmFs dWUNCj4gPiBieSBub24temVybyBEZXZpY2VzIEluIHBhY2thZ2UuDQo+ID4gc28gbGV0J3MgcHJv YmUgbW9yZSB3aXRoIHplcm8gRGV2aWNlcyBJbiBwYWNrYWdlIHRvIHN1cHBvcnQgbW9yZSBDNDUN Cj4gPiBQSFlzLg0KPiANCj4gSSBjYW5ub3QgcmVtZW1iZXIgZXhhY3RseSBob3cgbWFueSB0aW1l cyB0aGlzIHBhdGNoIGhhcyBiZWVuIHBvc3RlZCwgYnV0IGl0DQo+IHN0aWxsIGlzIG5vdCBjbGVh ciB0byBtZSB3aGF0IHlvdSBhcmUgZG9pbmcgaGVyZSBpcyBoZWxwaW5nIHdpdGggdGhlc2UNCj4g Q29ydGluYSBQSFlzLg0KPiANCj4gQ291bGQgeW91IHBvc3QgYSBkdW1wIG9mIHRoZSBtZGlvYnVz X3JlYWQoKSBhcmd1bWVudHMgYW5kIHZhbHVlcyBmb3IgdGhlIG9sZA0KPiBjb2RlIGFuZCB0aGUg bmV3IGNvZGUgeW91IGFyZSBwcm9wb3Npbmc/IFRoYXQgd2F5IGl0IG1pZ2h0IGJlIGNsZWFyZXIg d2hhdCBpcw0KPiB0aGUgZ29hbCBoZXJlPw0KPiANClRoZSBrZXkgcG9pbnQgaXMgdGhhdCBzdGFu ZGFyZCBDNDUgUEhZcyB1c2Ugbm9uLXplcm8gRGV2aWNlcyBpbiBwYWNrYWdlIGkocmVnX2FkZHIg PSBNSUlfQUREUl9DNDUgfCBpIDw8IDE2IHwgTURJT19ERVZTKSB0byByZWFkICBkZXZpY2VzX2lu X3BhY2thZ2UsIGRldmljZSB6ZXJvIGlzIHJlc2VydmVkLCBidXQgQ29ydGluYSBDUzQzMTUvQ1M0 MzQwIFBIWSB1c2UgemVybyBEZXZpY2VzIGluIHBhY2thZ2UocmVnX2FkZHIgPSBNSUlfQUREUl9D NDUgfCAwIDw8IDE2IHwgTURJT19ERVZTKSB0byByZWFkIGRldmljZXNfaW5fcGFja2FnZS4gDQoN ClRoaXMgaXMgY2F1c2VkIGJ5IENvcnRpbmEgbm9uLXN0YW5kYXJkIFBIWSwgZS5nLiBzdGFuZGFy ZCBQSFkgaGFzIE1JSV9QSFlTSUQxPTB4MDIsIE1JSV9QSFlTSUQyPTB4MDMsIGJ1dCBDUzQzMTUv Q1M0MzQwIFBIWSBoYXMgbm9uLXN0YW5kYXJkIG9mZnNldCBvZiBQSFkgSUQgcmVnaXN0ZXJzKE1J SV9QSFlTSUQxPTB4MDAsIE1JSV9QSFlTSUQyPTB4MDEpLg0KICANCj4gPg0KPiA+IFNpZ25lZC1v ZmYtYnk6IFNoZW5nemhvdSBMaXUgPFNoZW5nemhvdS5MaXVAZnJlZXNjYWxlLmNvbT4NCj4gPiAt LS0NCj4gPiB2MjogdXNlIE1ESU9fREVWUzEgYW5kIE1ESU9fREVWUzIgaW5zdGVhZCBvZiBjb25z dGFudCAnNicsICc1Jw0KPiA+DQo+ID4gIGRyaXZlcnMvbmV0L3BoeS9waHlfZGV2aWNlLmMgfCAy NSArKysrKysrKysrKysrKysrKysrKystLS0tDQo+ID4gIDEgZmlsZSBjaGFuZ2VkLCAyMSBpbnNl cnRpb25zKCspLCA0IGRlbGV0aW9ucygtKQ0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMv bmV0L3BoeS9waHlfZGV2aWNlLmMNCj4gPiBiL2RyaXZlcnMvbmV0L3BoeS9waHlfZGV2aWNlLmMg aW5kZXggYmRmZTUxZi4uYzRmODM2ZiAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL25ldC9waHkv cGh5X2RldmljZS5jDQo+ID4gKysrIGIvZHJpdmVycy9uZXQvcGh5L3BoeV9kZXZpY2UuYw0KPiA+ IEBAIC0yNDIsMTIgKzI0MiwyOSBAQCBzdGF0aWMgaW50IGdldF9waHlfYzQ1X2lkcyhzdHJ1Y3Qg bWlpX2J1cyAqYnVzLCBpbnQNCj4gYWRkciwgdTMyICpwaHlfaWQsDQo+ID4gIAkJCXJldHVybiAt RUlPOw0KPiA+ICAJCWM0NV9pZHMtPmRldmljZXNfaW5fcGFja2FnZSB8PSAocGh5X3JlZyAmIDB4 ZmZmZik7DQo+ID4NCj4gPiAtCQkvKiBJZiBtb3N0bHkgRnMsIHRoZXJlIGlzIG5vIGRldmljZSB0 aGVyZSwNCj4gPiAtCQkgKiBsZXQncyBnZXQgb3V0IG9mIGhlcmUuDQo+ID4gKwkJLyogSWYgbW9z dGx5IEZzLCBsZXQncyBjb250aW51ZSB0byBwcm9iZSBtb3JlDQo+ID4gKwkJICogYXMgc29tZSBj NDUgUEhZcyBoYXZlIHplcm8gRGV2aWNlcyBJbiBwYWNrYWdlLA0KPiA+ICsJCSAqIGUuZy4gQ29y dGluYSBDUzQzMTUvQ1M0MzQwIFBIWS4NCj4gPiAgCQkgKi8NCj4gPiAgCQlpZiAoKGM0NV9pZHMt PmRldmljZXNfaW5fcGFja2FnZSAmIDB4MWZmZmZmZmYpID09IDB4MWZmZmZmZmYpIHsNCj4gPiAt CQkJKnBoeV9pZCA9IDB4ZmZmZmZmZmY7DQo+ID4gLQkJCXJldHVybiAwOw0KPiA+ICsJCQlyZWdf YWRkciA9IE1JSV9BRERSX0M0NSB8IDAgPDwgMTYgfCBNRElPX0RFVlMyOw0KPiA+ICsJCQlwaHlf cmVnID0gbWRpb2J1c19yZWFkKGJ1cywgYWRkciwgcmVnX2FkZHIpOw0KPiA+ICsJCQlpZiAocGh5 X3JlZyA8IDApDQo+ID4gKwkJCQlyZXR1cm4gLUVJTzsNCj4gPiArCQkJYzQ1X2lkcy0+ZGV2aWNl c19pbl9wYWNrYWdlID0gKHBoeV9yZWcgJiAweGZmZmYpIDw8IDE2Ow0KPiA+ICsJCQlyZWdfYWRk ciA9IE1JSV9BRERSX0M0NSB8IDAgPDwgMTYgfCBNRElPX0RFVlMxOw0KPiA+ICsJCQlwaHlfcmVn ID0gbWRpb2J1c19yZWFkKGJ1cywgYWRkciwgcmVnX2FkZHIpOw0KPiA+ICsJCQlpZiAocGh5X3Jl ZyA8IDApDQo+ID4gKwkJCQlyZXR1cm4gLUVJTzsNCj4gPiArCQkJYzQ1X2lkcy0+ZGV2aWNlc19p bl9wYWNrYWdlIHw9IChwaHlfcmVnICYgMHhmZmZmKTsNCj4gPiArCQkJLyogSWYgbW9zdGx5IEZz LCB0aGVyZSBpcyBubyBkZXZpY2UgdGhlcmUsDQo+ID4gKwkJCSAqIGxldCdzIGdldCBvdXQgb2Yg aGVyZS4NCj4gPiArCQkJICovDQo+ID4gKwkJCWlmICgoYzQ1X2lkcy0+ZGV2aWNlc19pbl9wYWNr YWdlICYgMHgxZmZmZmZmZikgPT0NCj4gPiArCQkJCQkJCTB4MWZmZmZmZmYpIHsNCj4gPiArCQkJ CSpwaHlfaWQgPSAweGZmZmZmZmZmOw0KPiA+ICsJCQkJcmV0dXJuIDA7DQo+ID4gKwkJCX0NCj4g DQo+IENvdWxkIG5vdCB3ZSBzb21laG93IGJlIGEgYml0IG1vcmUgY2xldmVyIGFuZCB1dGlsaXpl IHRoZSBsb29wLCB3aXRoIGFuDQo+IGFkanVzdGVkIGkgPSAwIGR1cmluZyB0aGlzIGNvbmRpdGlv bj8gU29tZSBzb21ldGhpbmcgbGlrZSB0aGlzICh1bnRlc3RlZCk6DQo+IA0KPiBGbG9yaWFuDQoN CkkgaGFkIGRvbmUgaXQgdG8gdXRpbGl6ZSB0aGUgbG9vcCB3aXRoIGkgPSAwIGFzIHlvdXIgdGhv dWdodCwgYnV0IERhdmlkIE1pbGxlciBzYWlkIHRoYXQgdGhlIHdheSBvZiB1dGlsaXppbmcgdGhl IGxvb3AgbWFrZXMgbm8gc2Vuc2UgdG8gdGVzdCAnaScgZm9yIHplcm8gdnMuIG5vbi16ZXJvIHVu dGlsIHRoZSBsb29waW5nIGNvbnN0cnVjdCBpdCBpcyBjb250YWluZWQgd2l0aGluIGNhbiBhY3R1 YWxseSBoaXQgYSB6ZXJvIHZhbHVlLCBhZGRpbmcgc3VjaCBhIGNoZWNrIGhlcmUgbWFrZXMgdGhl IGNvZGUgY29uZnVzaW5nLg0KU28gSSBkcm9wcGVkIHRoZSB3YXkgb2YgdXRpbGl6aW5nIHRoZSBs b29wIHRvIG1ha2UgY29kZSByZWFkYWJsZS4NCg0K