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: [2/4] usb: dwc3: add dwc3 glue layer for UniPhier SoCs From: Felipe Balbi Message-Id: <87y3j1aabp.fsf@linux.intel.com> Date: Fri, 09 Mar 2018 11:04:58 +0200 To: Masahiro Yamada Cc: Kunihiko Hayashi , linux-usb@vger.kernel.org, Greg Kroah-Hartman , Rob Herring , Mark Rutland , devicetree@vger.kernel.org, linux-arm-kernel , Linux Kernel Mailing List , Jassi Brar , Masami Hiramatsu , Kishon Vijay Abraham I List-ID: SGksCgpNYXNhaGlybyBZYW1hZGEgPHlhbWFkYS5tYXNhaGlyb0Bzb2Npb25leHQuY29tPiB3cml0 ZXM6Cj4+Pj4gPiArc3RhdGljIHZvaWQgZHdjM3VfcmVzZXRfaW5pdChzdHJ1Y3QgZHdjM3VfcHJp diAqcHJpdikKPj4+PiA+ICt7Cj4+Pj4gPiArICBkd2MzdV9tYXNrd3JpdGUocHJpdiwgUkVTRVRf Q1RMLCBMSU5LX1JFU0VULCAwKTsKPj4+PiA+ICsgIHVzbGVlcF9yYW5nZSgxMDAwLCAyMDAwKTsK Pj4+PiA+ICsgIGR3YzN1X21hc2t3cml0ZShwcml2LCBSRVNFVF9DVEwsIExJTktfUkVTRVQsIExJ TktfUkVTRVQpOwo+Pj4+ID4gK30KPj4+PiA+ICsKPj4+PiA+ICtzdGF0aWMgdm9pZCBkd2MzdV9y ZXNldF9jbGVhcihzdHJ1Y3QgZHdjM3VfcHJpdiAqcHJpdikKPj4+PiA+ICt7Cj4+Pj4gPiArICBk d2MzdV9tYXNrd3JpdGUocHJpdiwgUkVTRVRfQ1RMLCBMSU5LX1JFU0VULCAwKTsKPj4+PiA+ICt9 Cj4+Pj4KPj4+PiBkcml2ZXJzL3Jlc2V0ID8KPj4+Cj4+PiBUaGUgcmVzZXQgZHJpdmVyIG1hbmFn ZXMgInN5c2N0cmwiIElPIG1hcCBhcmVhIGluIG91ciBTb0MuCj4+Pgo+Pj4gUkVTRVRfQ1RMIHJl Z2lzdGVyIGJlbG9uZ3MgdG8gImR3YzMtZ2x1ZSIgSU8gbWFwIGFyZWEsCj4+PiBhbmQgdGhlIGtl cm5lbCBjYW4ndCBhY2Nlc3MgdGhpcyBhcmVhIHVudGlsIGVuYWJsaW5nIHVzYiBjbG9ja3MgYW5k Cj4+PiBkZWFzc2VydGluZyB1c2IgcmVzZXRzIGluICJzeXNjdHJsIi4KPj4+Cj4+PiBJIHRoaW5r IHRoYXQgImR3YzMtZ2x1ZSIgcmVnaXN0ZXIgY29udHJvbCBzaG91bGQgYmUgc2VwYXJhdGVkIGZy b20KPj4+ICJzeXNjdHJsIi4KPj4KPj4gSnVzdCBzcGxpdCB5b3VyIGFkZHJlc3Mgc3BhY2UgYW5k IHRyZWF0IHlvdXIgZ2x1ZSBhcyBhIGRldmljZSB3aXRoCj4+IHNldmVyYWwgY2hpbGRyZW46Cj4+ Cj4+IGdsdWVANjViMDAwMDAgewo+PiAgICAgICAgIGNvbXBhdGlibGUgPSAiZm9vIgo+Pgo+PiAg ICAgICAgIHBoeUBiYXIgewo+PiAgICAgICAgICAgICAgICAgLi4uCj4+ICAgICAgICAgfTsKPj4K Pj4gICAgICAgICBzeXNjdHJsQGJheiB7Cj4+ICAgICAgICAgICAgICAgICAuLi4KPj4gICAgICAg ICB9Owo+Pgo+PiAgICAgICAgIGR3YzNAZm9vIHsKPj4gICAgICAgICAgICAgICAgIGNvbXBhdGli bGUgPSAic25wcywgZHdjMyI7Cj4+ICAgICAgICAgICAgICAgICAuLi4KPj4gICAgICAgICB9Owo+ PiB9Owo+Pgo+PiBUaGVuIHlvdSBrbm93IHRoYXQgeW91IGNhbiBsZXQgZHdjMy9jb3JlLmMgaGFu ZGxlIHRoZSBQSFkgZm9yIHlvdS4gSWYgd2UKPj4gbmVlZCB0byB0ZWFjaCBkd2MzL2NvcmUuYyBh Ym91dCByZWd1bGF0b3JzLCB3ZSBjYW4gZG8gdGhhdC4gQnV0IHdlIGRvbid0Cj4+IG5lZWQgU29D LXNwZWNpZmljIGhhY2tzIDstKQo+Pgo+PiAtLQo+PiBiYWxiaQo+Cj4KPiBTbGlnaHRseSByZWxh dGVkIHF1ZXN0aW9uLgo+Cj4KPiBXaHkgZG9uJ3Qgd2UgcHV0IGNsb2NrcyBhbmQgcmVzZXRzIHRv IGR3YzMvY29yZS5jPwoKV2UgY2FuIGRvIHRoYXQgZm9yIHRoZSBzaW1wbGVyIHBsYXRmb3Jtcywg bm8gd29ycmllcy4KCj4gZHdjMy1vZi1zaW1wbGUuYyBvbmx5IGhhbmRsZXMgY2xvY2tzIGFuZCBy ZXNldHMuCj4gVGhpcyBpcyBnZW5lcmljIGVub3VnaCB0byBiZSBhZGRlZCB0byBkd2MzL2NvcmUu YywgSSB0aGluay4KPgo+Cj4gSSBjaGVja2VkIHRoZSB0d28gaW5zdGFuY2VzIG9mIGR3YzMtb2Yt c2ltcGxlLgo+Cj4gInFjb20sZHdjMyIKPiBodHRwczovL2dpdGh1Yi5jb20vdG9ydmFsZHMvbGlu dXgvYmxvYi92NC4xNi1yYzMvYXJjaC9hcm02NC9ib290L2R0cy9xY29tL21zbTg5OTYuZHRzaSNM NzgwCj4KPiAicm9ja2NoaXAscmszMzk5LWR3YzMiCj4gaHR0cHM6Ly9naXRodWIuY29tL3RvcnZh bGRzL2xpbnV4L2Jsb2IvdjQuMTYtcmMzL2FyY2gvYXJtNjQvYm9vdC9kdHMvcm9ja2NoaXAvcmsz Mzk5LmR0c2kjTDM5NQo+Cj4KPiBUaGV5IGp1c3QgY29udGFpbiBjbG9ja3MsIHJlc2V0cywgYW5k ICJzbnBzLGR3YzMiIHN1Yi1ub2RlLgo+Cj4KPiBJZiB3ZSBkbyB0aGF0LAo+Cj4gdXNiQDc2MDAw MDAgewo+ICAgICAgICAgY29tcGF0aWJsZSA9ICJxY29tLGR3YzMiOwo+ICAgICAgICAgY2xvY2tz ID0gLi4uOwo+Cj4gICAgICAgICBkd2MzQDc2MDAwMDAgewo+ICAgICAgICAgICAgICAgICBjb21w YXRpYmxlID0gInNucHMsZHdjMyI7Cj4gICAgICAgICAgICAgICAgIHJlZyA9IC4uLjsKPiAgICAg ICAgICAgICAgICAgaW50ZXJydXB0cyA9IC4uLjsKPiAgICAgICAgICAgICAgICAgcGh5cyA9IC4u LjsKPiAgICAgICAgIH0KPiB9Owo+Cj4KPiB3aWxsIGJlIHR1cm5lZCBpbnRvCj4KPgo+IGR3YzNA NzYwMDAwMCB7Cj4gICAgICAgICBjb21wYXRpYmxlID0gInFjb20sZHdjMyIsICJzbnBzLGR3YzMi Owo+ICAgICAgICAgcmVnID0gLi4uOwo+ICAgICAgICAgY2xvY2tzID0gLi4uOwo+ICAgICAgICAg aW50ZXJydXB0cyA9IC4uLjsKPiAgICAgICAgIHBoeXMgPSAuLi47Cj4gfTsKPgo+Cj4gVGhpcyBs b29rcyBzaW1wbGVyLgoKeXVwLiBUaGlzIHdpbGwgb25seSB3b3JrIGZvciB0aGUgc2ltcGxlciBw bGF0Zm9ybXMsIHRob3VnaC4gVEkgcGxhdGZvcm1zCmFuZCBQQ0ktYmFzZWQgcGxhdGZvcm1zLCB0 aGlzIHJlYWxseSB3b24ndCB3b3JrIDotKQoKPiBBbHNvLCBkd2MzL2NvcmUuYyBhbmQgZHdjMy1v Zi1zaW1wbGUuYwo+IGR1cGxpY2F0ZSBydW50aW1lIFBNLgoKdGhleSAia2luZGEiIGR1cGxpY2F0 ZSA6LSkgU29tZSBwbGF0Zm9ybXMgaGF2ZSBwbGF0Zm9ybS1zcGVjaWZpYyBjbG9ja3MKd2hpY2gg YXJlIG5vdCBnZW5lcmljIGVub3VnaCB0byBiZSBzdHVmZmVkIGludG8gZHdjMy9jb3JlLmMuIFNv bWUgb2YKdGhvc2UgY2xvY2tzIG1heSBuZWVkIHNwZWNpYWwgaGFuZGxpbmcgb2Ygc29tZSBzb3J0 cy4gSXQncyBiZXN0IHRvIGtlZXAKdGhlIG9wdGlvbiBmb3IgcGVjdWxpYXIgY2xvY2sgdHJlZSBz ZXR1cHMgYXZhaWxhYmxlLgoKSWYgeW91ciBwbGF0Zm9ybSBpcyBzaW1wbGUgZW5vdWdoIHRoYXQg eW91IGNhbiBnZXQgYXdheSB3aXRob3V0IGEgZ2x1ZQpsYXllciwgc3VyZSB0aGluZy4gTW9yZSBw b3dlciBmb3IgeW91IDotKQo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: balbi@kernel.org (Felipe Balbi) Date: Fri, 09 Mar 2018 11:04:58 +0200 Subject: [PATCH 2/4] usb: dwc3: add dwc3 glue layer for UniPhier SoCs In-Reply-To: References: <1516712454-2915-3-git-send-email-hayashi.kunihiko@socionext.com> <87o9lklnwb.fsf@linux.intel.com> <20180124215228.ED43.4A936039@socionext.com> <87a7x3l8gr.fsf@linux.intel.com> Message-ID: <87y3j1aabp.fsf@linux.intel.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, Masahiro Yamada writes: >>>> > +static void dwc3u_reset_init(struct dwc3u_priv *priv) >>>> > +{ >>>> > + dwc3u_maskwrite(priv, RESET_CTL, LINK_RESET, 0); >>>> > + usleep_range(1000, 2000); >>>> > + dwc3u_maskwrite(priv, RESET_CTL, LINK_RESET, LINK_RESET); >>>> > +} >>>> > + >>>> > +static void dwc3u_reset_clear(struct dwc3u_priv *priv) >>>> > +{ >>>> > + dwc3u_maskwrite(priv, RESET_CTL, LINK_RESET, 0); >>>> > +} >>>> >>>> drivers/reset ? >>> >>> The reset driver manages "sysctrl" IO map area in our SoC. >>> >>> RESET_CTL register belongs to "dwc3-glue" IO map area, >>> and the kernel can't access this area until enabling usb clocks and >>> deasserting usb resets in "sysctrl". >>> >>> I think that "dwc3-glue" register control should be separated from >>> "sysctrl". >> >> Just split your address space and treat your glue as a device with >> several children: >> >> glue at 65b00000 { >> compatible = "foo" >> >> phy at bar { >> ... >> }; >> >> sysctrl at baz { >> ... >> }; >> >> dwc3 at foo { >> compatible = "snps, dwc3"; >> ... >> }; >> }; >> >> Then you know that you can let dwc3/core.c handle the PHY for you. If we >> need to teach dwc3/core.c about regulators, we can do that. But we don't >> need SoC-specific hacks ;-) >> >> -- >> balbi > > > Slightly related question. > > > Why don't we put clocks and resets to dwc3/core.c? We can do that for the simpler platforms, no worries. > dwc3-of-simple.c only handles clocks and resets. > This is generic enough to be added to dwc3/core.c, I think. > > > I checked the two instances of dwc3-of-simple. > > "qcom,dwc3" > https://github.com/torvalds/linux/blob/v4.16-rc3/arch/arm64/boot/dts/qcom/msm8996.dtsi#L780 > > "rockchip,rk3399-dwc3" > https://github.com/torvalds/linux/blob/v4.16-rc3/arch/arm64/boot/dts/rockchip/rk3399.dtsi#L395 > > > They just contain clocks, resets, and "snps,dwc3" sub-node. > > > If we do that, > > usb at 7600000 { > compatible = "qcom,dwc3"; > clocks = ...; > > dwc3 at 7600000 { > compatible = "snps,dwc3"; > reg = ...; > interrupts = ...; > phys = ...; > } > }; > > > will be turned into > > > dwc3 at 7600000 { > compatible = "qcom,dwc3", "snps,dwc3"; > reg = ...; > clocks = ...; > interrupts = ...; > phys = ...; > }; > > > This looks simpler. yup. This will only work for the simpler platforms, though. TI platforms and PCI-based platforms, this really won't work :-) > Also, dwc3/core.c and dwc3-of-simple.c > duplicate runtime PM. they "kinda" duplicate :-) Some platforms have platform-specific clocks which are not generic enough to be stuffed into dwc3/core.c. Some of those clocks may need special handling of some sorts. It's best to keep the option for peculiar clock tree setups available. If your platform is simple enough that you can get away without a glue layer, sure thing. More power for you :-) -- balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 832 bytes Desc: not available URL: From mboxrd@z Thu Jan 1 00:00:00 1970 From: Felipe Balbi Subject: Re: [PATCH 2/4] usb: dwc3: add dwc3 glue layer for UniPhier SoCs Date: Fri, 09 Mar 2018 11:04:58 +0200 Message-ID: <87y3j1aabp.fsf@linux.intel.com> References: <1516712454-2915-3-git-send-email-hayashi.kunihiko@socionext.com> <87o9lklnwb.fsf@linux.intel.com> <20180124215228.ED43.4A936039@socionext.com> <87a7x3l8gr.fsf@linux.intel.com> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Masahiro Yamada Cc: Kunihiko Hayashi , linux-usb@vger.kernel.org, Greg Kroah-Hartman , Rob Herring , Mark Rutland , devicetree@vger.kernel.org, linux-arm-kernel , Linux Kernel Mailing List , Jassi Brar , Masami Hiramatsu , Kishon Vijay Abraham I List-Id: devicetree@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Masahiro Yamada writes: >>>> > +static void dwc3u_reset_init(struct dwc3u_priv *priv) >>>> > +{ >>>> > + dwc3u_maskwrite(priv, RESET_CTL, LINK_RESET, 0); >>>> > + usleep_range(1000, 2000); >>>> > + dwc3u_maskwrite(priv, RESET_CTL, LINK_RESET, LINK_RESET); >>>> > +} >>>> > + >>>> > +static void dwc3u_reset_clear(struct dwc3u_priv *priv) >>>> > +{ >>>> > + dwc3u_maskwrite(priv, RESET_CTL, LINK_RESET, 0); >>>> > +} >>>> >>>> drivers/reset ? >>> >>> The reset driver manages "sysctrl" IO map area in our SoC. >>> >>> RESET_CTL register belongs to "dwc3-glue" IO map area, >>> and the kernel can't access this area until enabling usb clocks and >>> deasserting usb resets in "sysctrl". >>> >>> I think that "dwc3-glue" register control should be separated from >>> "sysctrl". >> >> Just split your address space and treat your glue as a device with >> several children: >> >> glue@65b00000 { >> compatible =3D "foo" >> >> phy@bar { >> ... >> }; >> >> sysctrl@baz { >> ... >> }; >> >> dwc3@foo { >> compatible =3D "snps, dwc3"; >> ... >> }; >> }; >> >> Then you know that you can let dwc3/core.c handle the PHY for you. If we >> need to teach dwc3/core.c about regulators, we can do that. But we don't >> need SoC-specific hacks ;-) >> >> -- >> balbi > > > Slightly related question. > > > Why don't we put clocks and resets to dwc3/core.c? We can do that for the simpler platforms, no worries. > dwc3-of-simple.c only handles clocks and resets. > This is generic enough to be added to dwc3/core.c, I think. > > > I checked the two instances of dwc3-of-simple. > > "qcom,dwc3" > https://github.com/torvalds/linux/blob/v4.16-rc3/arch/arm64/boot/dts/qcom= /msm8996.dtsi#L780 > > "rockchip,rk3399-dwc3" > https://github.com/torvalds/linux/blob/v4.16-rc3/arch/arm64/boot/dts/rock= chip/rk3399.dtsi#L395 > > > They just contain clocks, resets, and "snps,dwc3" sub-node. > > > If we do that, > > usb@7600000 { > compatible =3D "qcom,dwc3"; > clocks =3D ...; > > dwc3@7600000 { > compatible =3D "snps,dwc3"; > reg =3D ...; > interrupts =3D ...; > phys =3D ...; > } > }; > > > will be turned into > > > dwc3@7600000 { > compatible =3D "qcom,dwc3", "snps,dwc3"; > reg =3D ...; > clocks =3D ...; > interrupts =3D ...; > phys =3D ...; > }; > > > This looks simpler. yup. This will only work for the simpler platforms, though. TI platforms and PCI-based platforms, this really won't work :-) > Also, dwc3/core.c and dwc3-of-simple.c > duplicate runtime PM. they "kinda" duplicate :-) Some platforms have platform-specific clocks which are not generic enough to be stuffed into dwc3/core.c. Some of those clocks may need special handling of some sorts. It's best to keep the option for peculiar clock tree setups available. If your platform is simple enough that you can get away without a glue layer, sure thing. More power for you :-) =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlqiTjoACgkQzL64meEa mQYPCA/+P7rcXMx5aWldt2WPSal5A+KgDrwM7Bmw35toDIEgnMGzSHyuxVMEofIU tNiCKsZtmbk8X5NtZbXpjKHkKkCQp1fflFL0m4Lo901NJTB1oDTz5JdhlF8Bqjl4 ok7i6Ir7qG/hIgMqeR99EsXW+78VNN5AM96HNYRxZ9ZIQx4Fs+55Z6Q3U/z1UbMh ACQoXr9M+Bsk/bN9YMhoRJbyz4BMzm1UPFXutPsqgaMjXXJHCiiXDaQ1shn/ZDUd Z91KIF+KQCEB1RuSh/euU4x/JroeyJw39egeDXfGZjbGZAYnIJuwLEo7J9TJGQc2 Vzs0c4QYYOZZaGBdgNfX1Fp3Tu6qQb2CDhQLkRwACkD0ro2mlYHxM8zqy1c0jdVO ocW62XBWgPAXoGTtcr8G1f3QP/4Cmc5fKOeukAQ60nJ11NPzAZ9h0o0IEwlGllrp T/SDeGox+SdQ83tIhZ8fKwWJ+/N9uAQYbdffCzouL/nE5VIcpoG9x1LMspzVW7PM 3QdZGTs2ToQO5OpDTLlhwkZJwDrInLst8OWra1LpigIY0P06bLgG1Iakuta3BD+q bNKSa4/tGbdKcYQR0Z91p2QnLoGWjrymrZNWM5tecGlna7cbazhy3QgOvFckYSBE 9Y6IlyMOXvpq8J+2l9dF2+vFxl6h+4gPjlC2hCuOj6ccPb484KM= =kNAK -----END PGP SIGNATURE----- --=-=-=--