Re: [PATCH 1/3] usb: orion-echi: Add support for the Armada 3700
Hello, On Wed, 8 Mar 2017 17:24:21 +0100, Gregory CLEMENT wrote: > Signed-off-by: jinghuaI think you need a full first name + last name for this Signed-off-by. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH 1/3] usb: orion-echi: Add support for the Armada 3700
Hello, On Wed, 8 Mar 2017 17:24:21 +0100, Gregory CLEMENT wrote: > Signed-off-by: jinghua I think you need a full first name + last name for this Signed-off-by. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH 1/3] usb: orion-echi: Add support for the Armada 3700
Hi Andrew, On mer., mars 08 2017, Andrew Lunnwrote: > Hi Gregory [...] Thanks for your comments I will fix the typos and the wording. >> +#define USB_SBUSCFG 0x90 >> +#define USB_SBUSCFG_BAWR0x6 >> +#define USB_SBUSCFG_BARD0x3 >> +#define USB_SBUSCFG_AHBBRST 0x0 > > These three are all shifts. So i would suggest adding _SHIFT to the > end. Actually I removed it to fit in the 80 character... > >> + >> +/* BAWR = BARD = 3 : Align read/write bursts packets larger than 128 bytes >> */ >> +#define USB_SBUSCFG_BAWR_ALIGN_128B 0x3 >> +#define USB_SBUSCFG_BARD_ALIGN_128B 0x3 >> +/* AHBBRST = 3 : Align AHB Burst to INCR16 (64 bytes) */ >> +#define USB_SBUSCFG_AHBBRST_INCR16 0x3 > > You can then apply the shift here. ... but I didn't think to this trick. > >> + >> +#define USB_SBUSCFG_DEF_VAL ((USB_SBUSCFG_BAWR_ALIGN_128B << >> USB_SBUSCFG_BAWR) \ >> + | (USB_SBUSCFG_BARD_ALIGN_128B << USB_SBUSCFG_BARD) \ >> + | (USB_SBUSCFG_AHBBRST_INCR16 << USB_SBUSCFG_AHBBRST)) > > and this is then shorted. So I will do it in the v2. Thanks Gregory > > Andrew -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
Re: [PATCH 1/3] usb: orion-echi: Add support for the Armada 3700
Hi Andrew, On mer., mars 08 2017, Andrew Lunn wrote: > Hi Gregory [...] Thanks for your comments I will fix the typos and the wording. >> +#define USB_SBUSCFG 0x90 >> +#define USB_SBUSCFG_BAWR0x6 >> +#define USB_SBUSCFG_BARD0x3 >> +#define USB_SBUSCFG_AHBBRST 0x0 > > These three are all shifts. So i would suggest adding _SHIFT to the > end. Actually I removed it to fit in the 80 character... > >> + >> +/* BAWR = BARD = 3 : Align read/write bursts packets larger than 128 bytes >> */ >> +#define USB_SBUSCFG_BAWR_ALIGN_128B 0x3 >> +#define USB_SBUSCFG_BARD_ALIGN_128B 0x3 >> +/* AHBBRST = 3 : Align AHB Burst to INCR16 (64 bytes) */ >> +#define USB_SBUSCFG_AHBBRST_INCR16 0x3 > > You can then apply the shift here. ... but I didn't think to this trick. > >> + >> +#define USB_SBUSCFG_DEF_VAL ((USB_SBUSCFG_BAWR_ALIGN_128B << >> USB_SBUSCFG_BAWR) \ >> + | (USB_SBUSCFG_BARD_ALIGN_128B << USB_SBUSCFG_BARD) \ >> + | (USB_SBUSCFG_AHBBRST_INCR16 << USB_SBUSCFG_AHBBRST)) > > and this is then shorted. So I will do it in the v2. Thanks Gregory > > Andrew -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
Re: [PATCH 1/3] usb: orion-echi: Add support for the Armada 3700
Hi Gregory > - Add a new compatoble string for the Armada 3700 SoCs compatible > > - add sbuscfg support for orion usb controller driver. For the SoCs > without hlock, need to program BAWR/BARD/AHBBRST fields in the sbuscfg > register to guarantee the AHB master's burst would not overrun or > underrun the FIFO. > > - the sbuscfg register has to be set after the usb controller reset, > otherwise the value would be overridden to 0. In order to do this, the > reset callback is registered. > > [gregory.clem...@free-electrons.com: - reword commit and comments >- fix checkpatch warning] > Signed-off-by: jinghua> Signed-off-by: Gregory CLEMENT > --- > .../devicetree/bindings/usb/ehci-orion.txt | 4 ++- > drivers/usb/host/ehci-orion.c | 39 > ++ > 2 files changed, 42 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/usb/ehci-orion.txt > b/Documentation/devicetree/bindings/usb/ehci-orion.txt > index 17c3bc858b86..9dfffc9dffec 100644 > --- a/Documentation/devicetree/bindings/usb/ehci-orion.txt > +++ b/Documentation/devicetree/bindings/usb/ehci-orion.txt > @@ -1,7 +1,9 @@ > * EHCI controller, Orion Marvell variants > > Required properties: > -- compatible: must be "marvell,orion-ehci" > +- compatible: could be one of the following must, not could. > + "marvell,orion-ehci" > + "marvell,armada-3700-ehci" > - reg: physical base address of the controller and length of memory mapped >region. > - interrupts: The EHCI interrupt > diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c > index ee8d5faa0194..cf778e166b90 100644 > --- a/drivers/usb/host/ehci-orion.c > +++ b/drivers/usb/host/ehci-orion.c > @@ -47,6 +47,21 @@ > #define USB_PHY_IVREF_CTRL 0x440 > #define USB_PHY_TST_GRP_CTRL 0x450 > > +#define USB_SBUSCFG 0x90 > +#define USB_SBUSCFG_BAWR0x6 > +#define USB_SBUSCFG_BARD0x3 > +#define USB_SBUSCFG_AHBBRST 0x0 These three are all shifts. So i would suggest adding _SHIFT to the end. > + > +/* BAWR = BARD = 3 : Align read/write bursts packets larger than 128 bytes */ > +#define USB_SBUSCFG_BAWR_ALIGN_128B 0x3 > +#define USB_SBUSCFG_BARD_ALIGN_128B 0x3 > +/* AHBBRST = 3 : Align AHB Burst to INCR16 (64 bytes) */ > +#define USB_SBUSCFG_AHBBRST_INCR16 0x3 You can then apply the shift here. > + > +#define USB_SBUSCFG_DEF_VAL ((USB_SBUSCFG_BAWR_ALIGN_128B << > USB_SBUSCFG_BAWR) \ > + | (USB_SBUSCFG_BARD_ALIGN_128B << USB_SBUSCFG_BARD) \ > + | (USB_SBUSCFG_AHBBRST_INCR16 << USB_SBUSCFG_AHBBRST)) and this is then shorted. Andrew
Re: [PATCH 1/3] usb: orion-echi: Add support for the Armada 3700
Hi Gregory > - Add a new compatoble string for the Armada 3700 SoCs compatible > > - add sbuscfg support for orion usb controller driver. For the SoCs > without hlock, need to program BAWR/BARD/AHBBRST fields in the sbuscfg > register to guarantee the AHB master's burst would not overrun or > underrun the FIFO. > > - the sbuscfg register has to be set after the usb controller reset, > otherwise the value would be overridden to 0. In order to do this, the > reset callback is registered. > > [gregory.clem...@free-electrons.com: - reword commit and comments >- fix checkpatch warning] > Signed-off-by: jinghua > Signed-off-by: Gregory CLEMENT > --- > .../devicetree/bindings/usb/ehci-orion.txt | 4 ++- > drivers/usb/host/ehci-orion.c | 39 > ++ > 2 files changed, 42 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/usb/ehci-orion.txt > b/Documentation/devicetree/bindings/usb/ehci-orion.txt > index 17c3bc858b86..9dfffc9dffec 100644 > --- a/Documentation/devicetree/bindings/usb/ehci-orion.txt > +++ b/Documentation/devicetree/bindings/usb/ehci-orion.txt > @@ -1,7 +1,9 @@ > * EHCI controller, Orion Marvell variants > > Required properties: > -- compatible: must be "marvell,orion-ehci" > +- compatible: could be one of the following must, not could. > + "marvell,orion-ehci" > + "marvell,armada-3700-ehci" > - reg: physical base address of the controller and length of memory mapped >region. > - interrupts: The EHCI interrupt > diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c > index ee8d5faa0194..cf778e166b90 100644 > --- a/drivers/usb/host/ehci-orion.c > +++ b/drivers/usb/host/ehci-orion.c > @@ -47,6 +47,21 @@ > #define USB_PHY_IVREF_CTRL 0x440 > #define USB_PHY_TST_GRP_CTRL 0x450 > > +#define USB_SBUSCFG 0x90 > +#define USB_SBUSCFG_BAWR0x6 > +#define USB_SBUSCFG_BARD0x3 > +#define USB_SBUSCFG_AHBBRST 0x0 These three are all shifts. So i would suggest adding _SHIFT to the end. > + > +/* BAWR = BARD = 3 : Align read/write bursts packets larger than 128 bytes */ > +#define USB_SBUSCFG_BAWR_ALIGN_128B 0x3 > +#define USB_SBUSCFG_BARD_ALIGN_128B 0x3 > +/* AHBBRST = 3 : Align AHB Burst to INCR16 (64 bytes) */ > +#define USB_SBUSCFG_AHBBRST_INCR16 0x3 You can then apply the shift here. > + > +#define USB_SBUSCFG_DEF_VAL ((USB_SBUSCFG_BAWR_ALIGN_128B << > USB_SBUSCFG_BAWR) \ > + | (USB_SBUSCFG_BARD_ALIGN_128B << USB_SBUSCFG_BARD) \ > + | (USB_SBUSCFG_AHBBRST_INCR16 << USB_SBUSCFG_AHBBRST)) and this is then shorted. Andrew
[PATCH 1/3] usb: orion-echi: Add support for the Armada 3700
From: jinghua- Add a new compatoble string for the Armada 3700 SoCs - add sbuscfg support for orion usb controller driver. For the SoCs without hlock, need to program BAWR/BARD/AHBBRST fields in the sbuscfg register to guarantee the AHB master's burst would not overrun or underrun the FIFO. - the sbuscfg register has to be set after the usb controller reset, otherwise the value would be overridden to 0. In order to do this, the reset callback is registered. [gregory.clem...@free-electrons.com: - reword commit and comments - fix checkpatch warning] Signed-off-by: jinghua Signed-off-by: Gregory CLEMENT --- .../devicetree/bindings/usb/ehci-orion.txt | 4 ++- drivers/usb/host/ehci-orion.c | 39 ++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/usb/ehci-orion.txt b/Documentation/devicetree/bindings/usb/ehci-orion.txt index 17c3bc858b86..9dfffc9dffec 100644 --- a/Documentation/devicetree/bindings/usb/ehci-orion.txt +++ b/Documentation/devicetree/bindings/usb/ehci-orion.txt @@ -1,7 +1,9 @@ * EHCI controller, Orion Marvell variants Required properties: -- compatible: must be "marvell,orion-ehci" +- compatible: could be one of the following + "marvell,orion-ehci" + "marvell,armada-3700-ehci" - reg: physical base address of the controller and length of memory mapped region. - interrupts: The EHCI interrupt diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c index ee8d5faa0194..cf778e166b90 100644 --- a/drivers/usb/host/ehci-orion.c +++ b/drivers/usb/host/ehci-orion.c @@ -47,6 +47,21 @@ #define USB_PHY_IVREF_CTRL 0x440 #define USB_PHY_TST_GRP_CTRL 0x450 +#define USB_SBUSCFG0x90 +#defineUSB_SBUSCFG_BAWR0x6 +#defineUSB_SBUSCFG_BARD0x3 +#defineUSB_SBUSCFG_AHBBRST 0x0 + +/* BAWR = BARD = 3 : Align read/write bursts packets larger than 128 bytes */ +#define USB_SBUSCFG_BAWR_ALIGN_128B0x3 +#define USB_SBUSCFG_BARD_ALIGN_128B0x3 +/* AHBBRST = 3: Align AHB Burst to INCR16 (64 bytes) */ +#define USB_SBUSCFG_AHBBRST_INCR16 0x3 + +#define USB_SBUSCFG_DEF_VAL ((USB_SBUSCFG_BAWR_ALIGN_128B << USB_SBUSCFG_BAWR) \ +| (USB_SBUSCFG_BARD_ALIGN_128B << USB_SBUSCFG_BARD) \ +| (USB_SBUSCFG_AHBBRST_INCR16 << USB_SBUSCFG_AHBBRST)) + #define DRIVER_DESC "EHCI orion driver" #define hcd_to_orion_priv(h) ((struct orion_ehci_hcd *)hcd_to_ehci(h)->priv) @@ -151,8 +166,31 @@ ehci_orion_conf_mbus_windows(struct usb_hcd *hcd, } } +static int ehci_orion_drv_reset(struct usb_hcd *hcd) +{ + struct device *dev = hcd->self.controller; + int retval; + + retval = ehci_setup(hcd); + if (retval) + dev_err(dev, "ehci_setup failed %d\n", retval); + + /* +* For SoC without hlock, need to program sbuscfg value to guarantee +* AHB master's burst would not overrun or underrun FIFO. +* +* sbuscfg reg has to be set after usb controller reset, otherwise +* the value would be override to 0. +*/ + if (of_device_is_compatible(dev->of_node, "marvell,armada-3700-ehci")) + wrl(USB_SBUSCFG, USB_SBUSCFG_DEF_VAL); + + return retval; +} + static const struct ehci_driver_overrides orion_overrides __initconst = { .extra_priv_size = sizeof(struct orion_ehci_hcd), + .reset = ehci_orion_drv_reset, }; static int ehci_orion_drv_probe(struct platform_device *pdev) @@ -310,6 +348,7 @@ static int ehci_orion_drv_remove(struct platform_device *pdev) static const struct of_device_id ehci_orion_dt_ids[] = { { .compatible = "marvell,orion-ehci", }, + { .compatible = "marvell,armada-3700-ehci", }, {}, }; MODULE_DEVICE_TABLE(of, ehci_orion_dt_ids); -- 2.11.0
[PATCH 1/3] usb: orion-echi: Add support for the Armada 3700
From: jinghua - Add a new compatoble string for the Armada 3700 SoCs - add sbuscfg support for orion usb controller driver. For the SoCs without hlock, need to program BAWR/BARD/AHBBRST fields in the sbuscfg register to guarantee the AHB master's burst would not overrun or underrun the FIFO. - the sbuscfg register has to be set after the usb controller reset, otherwise the value would be overridden to 0. In order to do this, the reset callback is registered. [gregory.clem...@free-electrons.com: - reword commit and comments - fix checkpatch warning] Signed-off-by: jinghua Signed-off-by: Gregory CLEMENT --- .../devicetree/bindings/usb/ehci-orion.txt | 4 ++- drivers/usb/host/ehci-orion.c | 39 ++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/usb/ehci-orion.txt b/Documentation/devicetree/bindings/usb/ehci-orion.txt index 17c3bc858b86..9dfffc9dffec 100644 --- a/Documentation/devicetree/bindings/usb/ehci-orion.txt +++ b/Documentation/devicetree/bindings/usb/ehci-orion.txt @@ -1,7 +1,9 @@ * EHCI controller, Orion Marvell variants Required properties: -- compatible: must be "marvell,orion-ehci" +- compatible: could be one of the following + "marvell,orion-ehci" + "marvell,armada-3700-ehci" - reg: physical base address of the controller and length of memory mapped region. - interrupts: The EHCI interrupt diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c index ee8d5faa0194..cf778e166b90 100644 --- a/drivers/usb/host/ehci-orion.c +++ b/drivers/usb/host/ehci-orion.c @@ -47,6 +47,21 @@ #define USB_PHY_IVREF_CTRL 0x440 #define USB_PHY_TST_GRP_CTRL 0x450 +#define USB_SBUSCFG0x90 +#defineUSB_SBUSCFG_BAWR0x6 +#defineUSB_SBUSCFG_BARD0x3 +#defineUSB_SBUSCFG_AHBBRST 0x0 + +/* BAWR = BARD = 3 : Align read/write bursts packets larger than 128 bytes */ +#define USB_SBUSCFG_BAWR_ALIGN_128B0x3 +#define USB_SBUSCFG_BARD_ALIGN_128B0x3 +/* AHBBRST = 3: Align AHB Burst to INCR16 (64 bytes) */ +#define USB_SBUSCFG_AHBBRST_INCR16 0x3 + +#define USB_SBUSCFG_DEF_VAL ((USB_SBUSCFG_BAWR_ALIGN_128B << USB_SBUSCFG_BAWR) \ +| (USB_SBUSCFG_BARD_ALIGN_128B << USB_SBUSCFG_BARD) \ +| (USB_SBUSCFG_AHBBRST_INCR16 << USB_SBUSCFG_AHBBRST)) + #define DRIVER_DESC "EHCI orion driver" #define hcd_to_orion_priv(h) ((struct orion_ehci_hcd *)hcd_to_ehci(h)->priv) @@ -151,8 +166,31 @@ ehci_orion_conf_mbus_windows(struct usb_hcd *hcd, } } +static int ehci_orion_drv_reset(struct usb_hcd *hcd) +{ + struct device *dev = hcd->self.controller; + int retval; + + retval = ehci_setup(hcd); + if (retval) + dev_err(dev, "ehci_setup failed %d\n", retval); + + /* +* For SoC without hlock, need to program sbuscfg value to guarantee +* AHB master's burst would not overrun or underrun FIFO. +* +* sbuscfg reg has to be set after usb controller reset, otherwise +* the value would be override to 0. +*/ + if (of_device_is_compatible(dev->of_node, "marvell,armada-3700-ehci")) + wrl(USB_SBUSCFG, USB_SBUSCFG_DEF_VAL); + + return retval; +} + static const struct ehci_driver_overrides orion_overrides __initconst = { .extra_priv_size = sizeof(struct orion_ehci_hcd), + .reset = ehci_orion_drv_reset, }; static int ehci_orion_drv_probe(struct platform_device *pdev) @@ -310,6 +348,7 @@ static int ehci_orion_drv_remove(struct platform_device *pdev) static const struct of_device_id ehci_orion_dt_ids[] = { { .compatible = "marvell,orion-ehci", }, + { .compatible = "marvell,armada-3700-ehci", }, {}, }; MODULE_DEVICE_TABLE(of, ehci_orion_dt_ids); -- 2.11.0