From: Felipe Balbi <balbi-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Kunihiko Hayashi
<hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Greg Kroah-Hartman
<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
Masahiro Yamada
<yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>,
Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jassi Brar
<jaswinder.singh-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
Masami Hiramatsu
<masami.hiramatsu-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
Subject: Re: [PATCH 2/4] usb: dwc3: add dwc3 glue layer for UniPhier SoCs
Date: Wed, 24 Jan 2018 14:58:12 +0200 [thread overview]
Message-ID: <87a7x3l8gr.fsf@linux.intel.com> (raw)
In-Reply-To: <20180124215228.ED43.4A936039-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4072 bytes --]
Hi,
Kunihiko Hayashi <hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> writes:
> Hello Felipe,
>
> Thank you for your comments.
>
> On Tue, 23 Jan 2018 15:12:36 +0200 <balbi-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
>>
>> Hi,
>>
>> Kunihiko Hayashi <hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> writes:
>> > Add a specific glue layer for UniPhier SoC platform to support
>> > USB host mode. It manages hardware operating sequences to enable multiple
>> > clock gates and assert resets, and to prepare to use dwc3 controller
>> > on the SoC.
>> >
>> > This patch also handles the physical layer that has same register space
>> > as the glue layer, because it needs to integrate initialziation sequence
>> > between glue and phy.
>> >
>> > In case of some SoCs, since some initialization values for PHY are
>> > included in nvmem, this patch includes the way to get the values from nvmem.
>> >
>> > It supports PXs2 and LD20 SoCs.
>> >
>> > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
>> > Signed-off-by: Motoya Tanigawa <tanigawa.motoya-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
>> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> > ---
>> > drivers/usb/dwc3/Kconfig | 9 +
>> > drivers/usb/dwc3/Makefile | 1 +
>> > drivers/usb/dwc3/dwc3-uniphier.c | 554 +++++++++++++++++++++++++++++++++++++++
>> > 3 files changed, 564 insertions(+)
>> > create mode 100644 drivers/usb/dwc3/dwc3-uniphier.c
>
> ...snip...
>
>> > +
>> > +static void dwc3u_ssphy_testio_write(struct dwc3u_priv *priv, int port,
>> > + u32 data)
>>
>> anything with sshphy or hsphy in the name should probably be part of a
>> PHY driver using drivers/phy/ framework.
>
> I can try to separate phy control from this driver.
> However, phy registers belongs to "dwc3-glue" IO map area (65b00000),
> and this area also contains a reset bit for "dwc3-core" hardware.
>
> Although the phy driver is called from dwc3-core driver,
> we need to deassert the reset bit before probing dwc3-core driver.
>
> As shown later, I think that it's difficut to determine the order of
> initializing the registers in this area.
>
>> > +static void dwc3u_vbus_disable(struct dwc3u_priv *priv)
>> > +{
>> > + int i;
>> > +
>> > + for (i = 0; i < priv->nvbus; i++) {
>> > + dwc3u_maskwrite(priv, VBUS_CONTROL(i),
>> > + DRVVBUS_REG_EN | DRVVBUS_REG,
>> > + DRVVBUS_REG_EN | 0);
>> > + }
>> > +}
>>
>> drivers/regulator maybe?
>
> VBUS_CONTROL register is used for determing whether "dwc3-glue" hardware
> enables vbus connected with "regulator" hardware.
>
> The regulator driver should manage "regulator" hardware, and
> I don't think that the driver should manage this register.
>
>> > +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 = "foo"
phy@bar {
...
};
sysctrl@baz {
...
};
dwc3@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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
next prev parent reply other threads:[~2018-01-24 12:58 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-23 13:00 [PATCH 0/4] usb: dwc3: add UniPhier dwc3 glue layer support Kunihiko Hayashi
2018-01-23 13:00 ` [PATCH 1/4] dt-bindings: dwc3: add binding documentation for UniPhier dwc3 glue driver Kunihiko Hayashi
[not found] ` <1516712454-2915-2-git-send-email-hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
2018-01-30 0:06 ` Rob Herring
2018-01-30 12:14 ` Kunihiko Hayashi
2018-01-23 13:00 ` [PATCH 2/4] usb: dwc3: add dwc3 glue layer for UniPhier SoCs Kunihiko Hayashi
2018-01-23 13:12 ` Felipe Balbi
2018-01-24 12:52 ` Kunihiko Hayashi
[not found] ` <20180124215228.ED43.4A936039-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
2018-01-24 12:58 ` Felipe Balbi [this message]
2018-01-25 2:09 ` Kunihiko Hayashi
2018-01-26 7:28 ` Kunihiko Hayashi
2018-02-26 14:48 ` Masahiro Yamada
2018-03-09 9:04 ` Felipe Balbi
[not found] ` <1516712454-2915-3-git-send-email-hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
2018-01-25 21:16 ` kbuild test robot
[not found] ` <1516712454-2915-1-git-send-email-hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
2018-01-23 13:00 ` [PATCH 3/4] ARM: dts: uniphier: add dwc3 usb node for PXs2 Kunihiko Hayashi
2018-01-23 13:00 ` [PATCH 4/4] arm64: dts: uniphier: add dwc3 usb node for LD20 Kunihiko Hayashi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87a7x3l8gr.fsf@linux.intel.com \
--to=balbi-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
--cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
--cc=hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org \
--cc=jaswinder.singh-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
--cc=kishon-l0cyMroinI0@public.gmane.org \
--cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
--cc=masami.hiramatsu-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
--cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).