[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20210122165725.1dcdbb5b@coco.lan>
Date: Fri, 22 Jan 2021 16:57:25 +0100
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Rob Herring <robh@...nel.org>
Cc: Vinod Koul <vkoul@...nel.org>, Alex Dewar <alex.dewar90@...il.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Kishon Vijay Abraham I <kishon@...com>,
Yu Chen <chenyu56@...wei.com>,
"open list:STAGING SUBSYSTEM" <devel@...verdev.osuosl.org>,
devicetree@...r.kernel.org,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 4/4] phy: phy-hi3670-usb3: move driver from staging
into phy
Em Fri, 22 Jan 2021 08:51:37 -0600
Rob Herring <robh@...nel.org> escreveu:
> On Tue, Jan 19, 2021 at 4:26 AM Mauro Carvalho Chehab
> <mchehab+huawei@...nel.org> wrote:
> >
> > Em Thu, 14 Jan 2021 19:47:31 -0600
> > Rob Herring <robh@...nel.org> escreveu:
> >
> > > On Thu, Jan 14, 2021 at 06:35:44PM +0100, Mauro Carvalho Chehab wrote:
> > > > The phy USB3 driver for Hisilicon 970 (hi3670) is ready
> > > > for mainstream. Mode it from staging into the main driver's
> > > > phy/ directory.
> > > >
> > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> > > > ---
> > > > .../bindings/phy/phy-hi3670-usb3.yaml | 72 ++
> > > > MAINTAINERS | 9 +-
> > > > drivers/phy/hisilicon/Kconfig | 10 +
> > > > drivers/phy/hisilicon/Makefile | 1 +
> > > > drivers/phy/hisilicon/phy-hi3670-usb3.c | 668 ++++++++++++++++++
> > > > drivers/staging/hikey9xx/Kconfig | 11 -
> > > > drivers/staging/hikey9xx/Makefile | 2 -
> > > > drivers/staging/hikey9xx/phy-hi3670-usb3.c | 668 ------------------
> > > > drivers/staging/hikey9xx/phy-hi3670-usb3.yaml | 72 --
> > > > 9 files changed, 759 insertions(+), 754 deletions(-)
> > > > create mode 100644 Documentation/devicetree/bindings/phy/phy-hi3670-usb3.yaml
> > > > create mode 100644 drivers/phy/hisilicon/phy-hi3670-usb3.c
> > > > delete mode 100644 drivers/staging/hikey9xx/phy-hi3670-usb3.c
> > > > delete mode 100644 drivers/staging/hikey9xx/phy-hi3670-usb3.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.yaml b/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.yaml
> > > > new file mode 100644
> > > > index 000000000000..125a5d6546ae
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.yaml
> > > > @@ -0,0 +1,72 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/phy/hisilicon,hi3670-usb3.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Hisilicon Kirin970 USB PHY
> > > > +
> > > > +maintainers:
> > > > + - Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> > >
> > > Blank line.
> >
> > Ok.
> >
> > >
> > > > +description: |+
> > > > + Bindings for USB3 PHY on HiSilicon Kirin 970.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + const: hisilicon,hi3670-usb-phy
> > > > +
> > > > + "#phy-cells":
> > > > + const: 0
> > > > +
> > > > + hisilicon,pericrg-syscon:
> > > > + $ref: '/schemas/types.yaml#/definitions/phandle'
> > > > + description: phandle of syscon used to control iso refclk.
> > > > +
> > > > + hisilicon,pctrl-syscon:
> > > > + $ref: '/schemas/types.yaml#/definitions/phandle'
> > > > + description: phandle of syscon used to control usb tcxo.
> > > > +
> > > > + hisilicon,sctrl-syscon:
> > > > + $ref: '/schemas/types.yaml#/definitions/phandle'
> > > > + description: phandle of syscon used to control phy deep sleep.
> > > > +
> > > > + hisilicon,eye-diagram-param:
> > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > + description: Eye diagram for phy.
> > > > +
> > > > + hisilicon,tx-vboost-lvl:
> > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > + description: TX level vboost for phy.
> > > > +
> > > > +required:
> > > > + - compatible
> > > > + - hisilicon,pericrg-syscon
> > > > + - hisilicon,pctrl-syscon
> > > > + - hisilicon,sctrl-syscon
> > > > + - hisilicon,eye-diagram-param
> > > > + - hisilicon,tx-vboost-lvl
> > > > + - "#phy-cells"
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > + - |
> > > > + bus {
> > > > + #address-cells = <2>;
> > > > + #size-cells = <2>;
> > > > +
> > > > + usb3_otg_bc: usb3_otg_bc@...00000 {
> > > > + compatible = "syscon", "simple-mfd";
> > > > + reg = <0x0 0xff200000 0x0 0x1000>;
> > > > +
> > > > + usb_phy {
> > >
> > > Is there a contiguous register region for this sub-block? If so, add
> > > 'reg' even though Linux doesn't need it currently.
> >
> > No. The driver uses 4 syscon regions in order to access the needed
> > registers:
>
> I meant just for the parent device node. I assume these are the 'main'
> registers? If not, then maybe it should be a child of one of the other
> syscons.
>
> 'reg' would just be for documentation ATM. However, if the subblock
> was reused on another chip, but at a different offset then reg would
> become useful. You could handle that with a fixed offset when 'reg' is
> missing, but adding it later would be too late.
Hmm... You meaning something like this:
examples:
- |
bus {
#address-cells = <2>;
#size-cells = <2>;
reg = <0>;
right?
If so, I don't see any problem on doing that, but that fictional
"bus" was added there just to be able to set #address-cells and
#size-cells, in order to avoid warnings when checking the DT
validity, as recommended by a past review from your side.
The actual DT for USB is (from hi3670.dtsi):
/ {
compatible = "hisilicon,hi3670";
interrupt-parent = <&gic>;
#address-cells = <2>;
#size-cells = <2>;
soc {
compatible = "simple-bus";
#address-cells = <2>;
#size-cells = <2>;
ranges;
crg_ctrl: crg_ctrl@...35000 {
compatible = "hisilicon,hi3670-crgctrl", "syscon";
reg = <0x0 0xfff35000 0x0 0x1000>;
#clock-cells = <1>;
};
...
usb3_otg_bc: usb3_otg_bc@...00000 {
compatible = "syscon", "simple-mfd";
reg = <0x0 0xff200000 0x0 0x1000>;
usb_phy: usbphy {
compatible = "hisilicon,hi3670-usb-phy";
#phy-cells = <0>;
hisilicon,pericrg-syscon = <&crg_ctrl>;
hisilicon,pctrl-syscon = <&pctrl>;
hisilicon,sctrl-syscon = <&sctrl>;
hisilicon,eye-diagram-param = <0xFDFEE4>;
hisilicon,tx-vboost-lvl = <0x5>;
phy-supply = <&ldo17>;
};
};
...
};
};
Thanks,
Mauro
Powered by blists - more mailing lists