| Message ID | 1393221265-13057-2-git-send-email-lho@apm.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hello, Loc. Almost there. Just one more thing. On Sun, Feb 23, 2014 at 10:54:24PM -0700, Loc Ho wrote: .... > +static int xgene_ahci_init_memram(struct xgene_ahci_context *ctx) > +{ > + void __iomem *diagcsr = ctx->csr_base + SATA_DIAG_OFFSET; > + int try; > + u32 val; > + > + val = readl(diagcsr + CFG_MEM_RAM_SHUTDOWN); > + if (val == 0) { > + dev_dbg(ctx->dev, "memory already released from shutdown\n"); > + return 0; > + } > + dev_dbg(ctx->dev, "Release memory from shutdown\n"); > + /* SATA controller memory in shutdown. Remove from shutdown. */ > + writel(0x0, diagcsr + CFG_MEM_RAM_SHUTDOWN); > + readl(diagcsr + CFG_MEM_RAM_SHUTDOWN); /* Force a barrier */ > + > + /* Check for at least ~1ms */ > + try = 1000; > + do { > + val = readl(diagcsr + BLOCK_MEM_RDY); > + if (val != 0xFFFFFFFF) > + usleep_range(1, 100); > + } while (val != 0xFFFFFFFF && try-- > 0); > + if (try <= 0) { > + dev_err(ctx->dev, "failed to release memory from shutdown\n"); > + return -ENODEV; > + } > + return 0; > +} Hmm... ISTR raising this issue before but the above is way more elaborate than necessary. This isn't in any sense a hot path and 1ms is short enough to handle it simply. If the only thing being addressed here is that the init may take upto 1ms, you might as well just do writel(0x0, diagcsr + CFG_MEM_RAM_SHUTDOWN); readl(diagcsr + CFG_MEM_RAM_SHUTDOWN); /* Force a barrier */ msleep(1); /* reset may take upto 1ms */ if (readl(diagcsr + BLOCK_MEM_RDY) != 0xFFFFFFFF) { dev_err(...); return -ENODEV; } return 0; Thanks.
On Sun, Feb 23, 2014 at 10:54:25PM -0700, Loc Ho wrote: > Signed-off-by: Loc Ho <lho@apm.com> > Signed-off-by: Tuan Phan <tphan@apm.com> > Signed-off-by: Suman Tripathi <stripathi@apm.com> This doesn't apply cleanly to libata/for-3.15. How should this be routed? Thanks.
Hey, On Mon, Feb 24, 2014 at 05:02:52PM -0800, Loc Ho wrote: > The completion of the RAM removal from shutdown is quite fast. As per > spec, the max time is 1ms but from the run-time code, it only take one > (1us) or two (2us) read for this to completed. An 1 ms hard delay is more > than 100 time slower. The code isn't that complex. If you strongly advise > that I should remove the while loop and use 1 ms, then okay. Otherwise, I > would prefer to keep the original code. The code is rather silly because as it's written the delay may be upto 100ms. If it's sitting in the hot path, using things like usleep_range() makes sense. In probe / shutdown paths, it's just an unnecessary distraction. If the timeout is as short as 1ms, it doesn't really matter whether the hardware can do it faster or not. It's just irrelevant. Please just write simple code. Thanks.
Hi, On Tuesday 25 February 2014 06:44 AM, Loc Ho wrote: > Hi Tejun, > > On Sun, Feb 23, 2014 at 10:54:25PM -0700, Loc Ho wrote: > > Signed-off-by: Loc Ho <lho@apm.com <mailto:lho@apm.com>> > > Signed-off-by: Tuan Phan <tphan@apm.com <mailto:tphan@apm.com>> > > Signed-off-by: Suman Tripathi <stripathi@apm.com <mailto:stripathi@apm.com>> > > This doesn't apply cleanly to libata/for-3.15. How should this be > routed? > > > You need to apply the corresponding PHY driver as well. Are you taken the > corresponding PHY driver into your libata tree? If not, we will need to route > to Kishon generic PHY framework tree. I include Kishon Vijay Abraham to this > email thread. I prefer dt entry go via dt maintainer. However I can add my Acked-by for the dt patches if you can copy me in those patches. > > Kishon, > > Do I need to re-send my SATA PHY driver to you as I had being posting them to > the scsi/arm mailing list? Yes please. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Kishon/Tejun, >> > Signed-off-by: Loc Ho <lho@apm.com <mailto:lho@apm.com>> >> > Signed-off-by: Tuan Phan <tphan@apm.com <mailto:tphan@apm.com>> >> > Signed-off-by: Suman Tripathi <stripathi@apm.com <mailto:stripathi@apm.com>> >> >> This doesn't apply cleanly to libata/for-3.15. How should this be >> routed? >> >> >> You need to apply the corresponding PHY driver as well. Are you taken the >> corresponding PHY driver into your libata tree? If not, we will need to route >> to Kishon generic PHY framework tree. I include Kishon Vijay Abraham to this >> email thread. > > I prefer dt entry go via dt maintainer. However I can add my Acked-by for the > dt patches if you can copy me in those patches. The dt email is cc'ed in the patch. Do I need to do anything more here? >> Kishon, >> >> Do I need to re-send my SATA PHY driver to you as I had being posting them to >> the scsi/arm mailing list? > > Yes please. Okay... I will re-post the PHY to you and linux-kernel@vger.kernel.org. -Loc -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/ata/apm-xgene.txt b/Documentation/devicetree/bindings/ata/apm-xgene.txt new file mode 100644 index 0000000..633eb3b --- /dev/null +++ b/Documentation/devicetree/bindings/ata/apm-xgene.txt @@ -0,0 +1,70 @@ +* APM X-Gene 6.0 Gb/s SATA host controller nodes + +SATA host controller nodes are defined to describe on-chip Serial ATA +controllers. Each SATA controller (pair of ports) have its own node. + +Required properties: +- compatible : Shall contain: + * "apm,xgene-ahci-sgmii" if mux'ed with SGMII + * "apm,xgene-ahci-pcie" if mux'ed with PCIe +- reg : First memory resource shall be the AHCI memory + resource. + Second memory resource shall be the host controller + memory resource. +- interrupts : Interrupt-specifier for SATA host controller IRQ. +- clocks : Reference to the clock entry. +- phys : A list of phandles + phy-specifiers, one for each + entry in phy-names. +- phy-names : Should contain: + * "sata-6g" for the SATA 6.0Gbps PHY + +Optional properties: +- status : Shall be "ok" if enabled or "disabled" if disabled. + Default is "ok". +- interrupt-parent : Interrupt controller. + +Example: + sataclk: sataclk { + compatible = "fixed-clock"; + #clock-cells = <1>; + clock-frequency = <100000000>; + clock-output-names = "sataclk"; + }; + + phy2: phy@1f22a000 { + compatible = "apm,xgene-phy"; + reg = <0x0 0x1f22a000 0x0 0x100>, + <0x0 0x1f22c000 0x0 0x100>; + #phy-cells = <1>; + }; + + phy3: phy@1f23a000 { + compatible = "apm,xgene-phy"; + reg = <0x0 0x1f23a000 0x0 0x100>, + <0x0 0x1f23c000 0x0 0x100>; + #phy-cells = <1>; + }; + + sata2: sata@1a400000 { + compatible = "apm,xgene-ahci-sgmii"; + reg = <0x0 0x1a400000 0x0 0x1000>, + <0x0 0x1f220000 0x0 0x10000>; + interrupt-parent = <&gic>; + interrupts = <0x0 0x87 0x4>; + status = "ok"; + clocks = <&sataclk 0>; + phys = <&phy2 0>; + phy-names = "sata-6g"; + }; + + sata3: sata@1a800000 { + compatible = "apm,xgene-ahci-pcie"; + reg = <0x0 0x1a800000 0x0 0x1000>, + <0x0 0x1f230000 0x0 0x10000>; + interrupt-parent = <&gic>; + interrupts = <0x0 0x88 0x4>; + status = "ok"; + clocks = <&sataclk 0>; + phys = <&phy3 0>; + phy-names = "sata-6g"; + };