From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4 From: Tony Lindgren Message-Id: <20180218175027.GW6364@atomide.com> Date: Sun, 18 Feb 2018 09:50:27 -0800 To: Sebastian Reichel Cc: Kishon Vijay Abraham I , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, linux-omap@vger.kernel.org, devicetree@vger.kernel.org, Mark Rutland , Marcel Partap , Michael Scott , Rob Herring List-ID: KiBTZWJhc3RpYW4gUmVpY2hlbCA8c3JlQGtlcm5lbC5vcmc+IFsxODAyMTggMDA6MzJdOgo+IE9u IFNhdCwgRmViIDE3LCAyMDE4IGF0IDAxOjA3OjIzUE0gLTA4MDAsIFRvbnkgTGluZGdyZW4gd3Jv dGU6Cj4gPiBGb3IgcmVmZXJlbmNlIGhlcmUgaXMgd2hhdCBJIG1lYXN1cmVkIGZvciB0b3RhbCBw b3dlciBjb25zdW1wdGlvbiBvbgo+ID4gYW4gaWRsZSBEcm9pZCA0IHdpdGggYW5kIHdpdGhvdXQg VVNCIHJlbGF0ZWQgTURNNjYwMCBtb2R1bGVzOgo+ID4gCj4gPiBpZGxlIGxjZCBvZmYJcGh5LW1h cHBob25lLW1kbTY2MDAJb2hjaS1wbGF0Zm9ybQo+ID4gMTUzbVcJCTI4NG1XCQkJMzQ0bVcKPiAK PiBTbyBtb3JlIHRoYW4gdHdpY2UgdGhlIGlkbGUgcG93ZXIuLi4gV2UgcmVhbGx5IHdhbnQgdG8g Z2V0IHJ1bnRpbWUKPiBQTSB3b3JraW5nIDovCgpZZXMgYW5kIHdlIHdhbnQnIG1vZGVtIHRvIGlk bGUgdG9vIDopCgo+ID4gKy8qCj4gPiArICogTURNNjYwMCBzdGF0dXMgY29kZXMuIFRoZXNlIGFy ZSBjb3BpZWQgZnJvbSBNb3Rvcm9sYSBNYXBwaG9uZSBMaW51eAo+ID4gKyAqIGtlcm5lbCB0cmVl LiBUaGUgQkIgbmFtaW5nIGhlcmUgcmVmZXJzIHRvICJCYXNlQmFuZCIgZm9yIG1vZGVtLgo+ID4g KyAqLwo+IAo+IEFjdHVhbGx5IHlvdXIgc3RhdHVzIGNvZGVzIGFyZSBCUF8gKGJhc2ViYW5kIHBy b2Nlc3NvcikgcHJlZml4ZWQuCgpJJ2xsIGp1c3QgZ2V0IHJpZCBvZiB0aGUgbmFtaW5nIGFuZCBz dGljayB0byBNRE02NjAwIHByZWZpeGllcy4KTm8gbmVlZCB0byBrZWVwIHRoZSBjb25mdXNpbmcg QlAvQVAgcHJlZml4ZXMuCgo+ID4gK3N0YXRpYyB2b2lkIHBoeV9tZG02NjAwX2luaXRfaXJxKHN0 cnVjdCBwaHlfbWRtNjYwMCAqZGRhdGEpCj4gPiArewo+ID4gKwlzdHJ1Y3QgZGV2aWNlICpkZXYg PSBkZGF0YS0+ZGV2Owo+ID4gKwlpbnQgaSwgZXJyb3IsIGlycTsKPiA+ICsKPiA+ICsJZm9yIChp ID0gUEhZX01ETTY2MDBfU1RBVFVTMDsKPiA+ICsJICAgICBpIDw9IFBIWV9NRE02NjAwX1NUQVRV UzI7IGkrKykgewo+ID4gKwkJaWYgKElTX0VSUihkZGF0YS0+Z3Bpb1tpXSkpCj4gPiArCQkJY29u dGludWU7Cj4gCj4gVGhpcyBjYW4gYmUgZHJvcHBlZCwgc2luY2UgdGhlIGRyaXZlciBlcnJvcnMg b3V0IG9mIHByb2JlCj4gd2hlbiB0aGVyZSBhcmUgZ3BpbyBlcnJvcnMuCgpUcnVlIHdpbGwgZHJv cC4KCj4gPiArc3RhdGljIGludCBwaHlfbWRtNjYwMF9pbml0X2xpbmVzKHN0cnVjdCBwaHlfbWRt NjYwMCAqZGRhdGEpCj4gPiArewo+ID4gKwlzdHJ1Y3QgZGV2aWNlICpkZXYgPSBkZGF0YS0+ZGV2 Owo+ID4gKwlpbnQgaSwgaiwgbnJfZ3BpbyA9IDA7Cj4gPiArCj4gPiArCWZvciAoaSA9IDA7IGkg PCBBUlJBWV9TSVpFKHBoeV9tZG02NjAwX2xpbmVfbWFwKTsgaSsrKSB7Cj4gPiArCQljb25zdCBz dHJ1Y3QgcGh5X21kbTY2MDBfbWFwICptYXAgPQo+ID4gKwkJCSZwaHlfbWRtNjYwMF9saW5lX21h cFtpXTsKPiA+ICsKPiA+ICsJCWZvciAoaiA9IDA7IGogPCBtYXAtPm5yX2dwaW9zOyBqKyspIHsK PiA+ICsJCQlzdHJ1Y3QgZ3Bpb19kZXNjICoqZ3BpbyA9ICZkZGF0YS0+Z3Bpb1tucl9ncGlvXTsK PiA+ICsKPiA+ICsJCQkqZ3BpbyA9IGRldm1fZ3Bpb2RfZ2V0X2luZGV4KGRldiwKPiA+ICsJCQkJ CQkgICAgIG1hcC0+bmFtZSwgaiwKPiA+ICsJCQkJCQkgICAgIG1hcC0+ZGlyZWN0aW9uKTsKPiA+ ICsJCQlpZiAoSVNfRVJSKCpncGlvKSkgewo+ID4gKwkJCQlkZXZfaW5mbyhkZXYsCj4gPiArCQkJ CQkgImdwaW8gJXMgZXJyb3IgJWxpLCBhbHJlYWR5IHRha2VuP1xuIiwKPiA+ICsJCQkJCSBtYXAt Pm5hbWUsIFBUUl9FUlIoKmdwaW8pKTsKPiA+ICsJCQkJcmV0dXJuIFBUUl9FUlIoKmdwaW8pOwo+ ID4gKwkJCX0KPiA+ICsJCQlucl9ncGlvKys7Cj4gPiArCQl9Cj4gCj4gSSB0aGluayB0aGUgY29k ZSBzaG91bGQgdXNlIHRoZSBncGlvZF9nZXRfYXJyYXkoKSBBUEkuCgpPSyB0aGFua3Mgd2lsbCB0 YWtlIGEgbG9vay4KCj4gPiArCS8qIEdpdmUgdXAgc2hhcmVkIEdQSU9zIG5vdywgdGhleSB3aWxs IGJlIHVzZWQgZm9yIE9PQiB3YWtlICovCj4gPiArCWRldm1fZ3Bpb2RfcHV0KGRkYXRhLT5kZXYs IG1vZGVfZ3BpbzApOwo+ID4gKwlkZGF0YS0+Z3Bpb1tQSFlfTURNNjYwMF9NT0RFMF0gPSBFUlJf UFRSKC1FTk9ERVYpOwo+ID4gKwlkZXZtX2dwaW9kX3B1dChkZGF0YS0+ZGV2LCBtb2RlX2dwaW8x KTsKPiA+ICsJZGRhdGEtPmdwaW9bUEhZX01ETTY2MDBfTU9ERTBdID0gRVJSX1BUUigtRU5PREVW KTsKPiAKPiBZb3Ugd2FudCBQSFlfTURNNjYwMF9NT0RFMSBpbnN0ZWFkLiBBbHNvIEkgd291bGQg anVzdCB1c2UgTlVMTC4KPiBOVUxMIGlzIHVzZWQgYnkgZ3Bpb2RfZ2V0X29wdGlvbmFsIGFuZCBp cyBoYW5kbGVkIGJ5IHRoZSBncGlvZAo+IGZ1bmN0aW9ucywgc28geW91IGRvbid0IG5lZWQgdG8g Y2hlY2sgZm9yIGdwaW8gZXJyb3JzIGV2ZXJ5d2hlcmUuCgpPb3BzIHl1cCBsZXQncyBqdXN0IGRy b3AgdGhpcyB1bnRpbCB3ZSBrbm93IHdoYXQgdG8gZG8gaW4gdGhlCmZ1cnRoZXIgcGF0Y2hlcy4K Cj4gPiArI2lmZGVmIENPTkZJR19PRgo+ID4gK3N0YXRpYyBjb25zdCBzdHJ1Y3Qgb2ZfZGV2aWNl X2lkIHBoeV9tZG02NjAwX2lkX3RhYmxlW10gPSB7Cj4gPiArCXsgLmNvbXBhdGlibGUgPSAibW90 b3JvbGEsbWFwcGhvbmUtbWRtNjYwMCIsIH0sCj4gPiArCXt9LAo+ID4gK307Cj4gPiArTU9EVUxF X0RFVklDRV9UQUJMRShvZiwgcGh5X21kbTY2MDBfaWRfdGFibGUpOwo+ID4gKyNlbmRpZgo+IAo+ IEkgc3VnZ2VzdCB0byBqdXN0IGRlcGVuZCBvbiBDT05GSUdfT0YgaW4gS2NvbmZpZyBhbmQgZHJv cCB0aGUgaWZkZWYKPiBhbmQgb2ZfbWF0Y2hfcHRyKCkgcGFydHMuIEl0J3MgdmVyeSB1bmxpa2Vs eSwgdGhhdCB0aGlzIHdpbGwgYmUKPiB1c2VkIHdpdGhvdXQgRFQgYW5kIHdvdWxkIG5lZWQgcXVp dGUgc29tZSByZXdvcmsgYW55d2F5cy4KCk9LCgo+ID4gK3N0YXRpYyBpbnQgcGh5X21kbTY2MDBf cHJvYmUoc3RydWN0IHBsYXRmb3JtX2RldmljZSAqcGRldikKPiA+ICt7Cj4gPiArCXN0cnVjdCBw aHlfbWRtNjYwMCAqZGRhdGE7Cj4gPiArCXN0cnVjdCB1c2Jfb3RnICpvdGc7Cj4gPiArCWNvbnN0 IHN0cnVjdCBvZl9kZXZpY2VfaWQgKm9mX2lkOwo+ID4gKwlpbnQgZXJyb3I7Cj4gPiArCj4gPiAr CW9mX2lkID0gb2ZfbWF0Y2hfZGV2aWNlKG9mX21hdGNoX3B0cihwaHlfbWRtNjYwMF9pZF90YWJs ZSksCj4gPiArCQkJCSZwZGV2LT5kZXYpOwo+ID4gKwlpZiAoIW9mX2lkKQo+ID4gKwkJcmV0dXJu IC1FSU5WQUw7Cj4gCj4gSSBzdWdnZXN0IHRvIGRyb3AgdGhlIG9mX21hdGNoX2RldmljZSgpLiBU aGUgZHJpdmVyIHdpbGwgZXJyb3IKPiBvdXQgYW55d2F5cyB3aGVuIGl0IGNhbid0IGdldCB0aGUg Z3Bpb3MuCgpPSwoKPiBHZW5lcmFsbHkgSSdtIGEgYml0IHdvcnJpZWQgYWJvdXQgaGFuZGxpbmcg dGhlIG1vZGUgZ3Bpb3MgaW4gdHdvCj4gZGlmZmVyZW50IGRyaXZlcnMuIEl0IGxvb2tzIGxpa2Ug aXQgbWlnaHQgYmVjb21lIGEgZGVwZW5kZW5jeSBoZWxsLgoKWWVhaCB5b3UncmUgcmlnaHQuIFRo YXQgd2lsbCByZXF1aXJlIHRoZSBtb2R1bGVzIHRvIGJlIGxvYWRlZAppbiB0aGUgcmlnaHQgb3Jk ZXIuIEl0J3MgcHJvYmFibHkgYmVzdCB0aGF0IHdlIGhhbmRsZSB0aGUgbWRtNjYwMAp0byBTb0Mg d2FrZS11cCBpbiB0aGlzIGRyaXZlci4gQW5kIHRoZW4gbWF5YmUgZXhwb3J0IGEgZnVuY3Rpb24g Zm9yCnRvZ2dsaW5nIHRoZSBTb0MgdG8gbWRtNjYwIHdha2UgdG8gbWFrZSBzdXJlIHRoZSBkZXBl bmRlbmNpZXMgYXJlCmNsZWFyIGZvciB0aGUgbW9kdWxlcy4KClJlZ2FyZHMsCgpUb255Ci0tLQpU byB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUg bGludXgtdXNiIiBpbgp0aGUgYm9keSBvZiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2Vy bmVsLm9yZwpNb3JlIG1ham9yZG9tbyBpbmZvIGF0ICBodHRwOi8vdmdlci5rZXJuZWwub3JnL21h am9yZG9tby1pbmZvLmh0bWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Sun, 18 Feb 2018 09:50:27 -0800 From: Tony Lindgren Subject: Re: [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4 Message-ID: <20180218175027.GW6364@atomide.com> References: <20180217210723.7013-1-tony@atomide.com> <20180218003139.qiojzvfnbb5vdmrj@earth.universe> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <20180218003139.qiojzvfnbb5vdmrj@earth.universe> To: Sebastian Reichel Cc: Kishon Vijay Abraham I , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, linux-omap@vger.kernel.org, devicetree@vger.kernel.org, Mark Rutland , Marcel Partap , Michael Scott , Rob Herring List-ID: * Sebastian Reichel [180218 00:32]: > On Sat, Feb 17, 2018 at 01:07:23PM -0800, Tony Lindgren wrote: > > For reference here is what I measured for total power consumption on > > an idle Droid 4 with and without USB related MDM6600 modules: > >=20 > > idle lcd off phy-mapphone-mdm6600 ohci-platform > > 153mW 284mW 344mW >=20 > So more than twice the idle power... We really want to get runtime > PM working :/ Yes and we want' modem to idle too :) > > +/* > > + * MDM6600 status codes. These are copied from Motorola Mapphone Linux > > + * kernel tree. The BB naming here refers to "BaseBand" for modem. > > + */ >=20 > Actually your status codes are BP_ (baseband processor) prefixed. I'll just get rid of the naming and stick to MDM6600 prefixies. No need to keep the confusing BP/AP prefixes. > > +static void phy_mdm6600_init_irq(struct phy_mdm6600 *ddata) > > +{ > > + struct device *dev =3D ddata->dev; > > + int i, error, irq; > > + > > + for (i =3D PHY_MDM6600_STATUS0; > > + i <=3D PHY_MDM6600_STATUS2; i++) { > > + if (IS_ERR(ddata->gpio[i])) > > + continue; >=20 > This can be dropped, since the driver errors out of probe > when there are gpio errors. True will drop. > > +static int phy_mdm6600_init_lines(struct phy_mdm6600 *ddata) > > +{ > > + struct device *dev =3D ddata->dev; > > + int i, j, nr_gpio =3D 0; > > + > > + for (i =3D 0; i < ARRAY_SIZE(phy_mdm6600_line_map); i++) { > > + const struct phy_mdm6600_map *map =3D > > + &phy_mdm6600_line_map[i]; > > + > > + for (j =3D 0; j < map->nr_gpios; j++) { > > + struct gpio_desc **gpio =3D &ddata->gpio[nr_gpio]; > > + > > + *gpio =3D devm_gpiod_get_index(dev, > > + map->name, j, > > + map->direction); > > + if (IS_ERR(*gpio)) { > > + dev_info(dev, > > + "gpio %s error %li, already taken?\n", > > + map->name, PTR_ERR(*gpio)); > > + return PTR_ERR(*gpio); > > + } > > + nr_gpio++; > > + } >=20 > I think the code should use the gpiod_get_array() API. OK thanks will take a look. > > + /* Give up shared GPIOs now, they will be used for OOB wake */ > > + devm_gpiod_put(ddata->dev, mode_gpio0); > > + ddata->gpio[PHY_MDM6600_MODE0] =3D ERR_PTR(-ENODEV); > > + devm_gpiod_put(ddata->dev, mode_gpio1); > > + ddata->gpio[PHY_MDM6600_MODE0] =3D ERR_PTR(-ENODEV); >=20 > You want PHY_MDM6600_MODE1 instead. Also I would just use NULL. > NULL is used by gpiod_get_optional and is handled by the gpiod > functions, so you don't need to check for gpio errors everywhere. Oops yup let's just drop this until we know what to do in the further patches. > > +#ifdef CONFIG_OF > > +static const struct of_device_id phy_mdm6600_id_table[] =3D { > > + { .compatible =3D "motorola,mapphone-mdm6600", }, > > + {}, > > +}; > > +MODULE_DEVICE_TABLE(of, phy_mdm6600_id_table); > > +#endif >=20 > I suggest to just depend on CONFIG_OF in Kconfig and drop the ifdef > and of_match_ptr() parts. It's very unlikely, that this will be > used without DT and would need quite some rework anyways. OK > > +static int phy_mdm6600_probe(struct platform_device *pdev) > > +{ > > + struct phy_mdm6600 *ddata; > > + struct usb_otg *otg; > > + const struct of_device_id *of_id; > > + int error; > > + > > + of_id =3D of_match_device(of_match_ptr(phy_mdm6600_id_table), > > + &pdev->dev); > > + if (!of_id) > > + return -EINVAL; >=20 > I suggest to drop the of_match_device(). The driver will error > out anyways when it can't get the gpios. OK > Generally I'm a bit worried about handling the mode gpios in two > different drivers. It looks like it might become a dependency hell. Yeah you're right. That will require the modules to be loaded in the right order. It's probably best that we handle the mdm6600 to SoC wake-up in this driver. And then maybe export a function for toggling the SoC to mdm660 wake to make sure the dependencies are clear for the modules. Regards, Tony