* Re: [PATCH v2 4/6] ARM: dts: imx6: Add support for phxBOARD-Mira i.MX 6 DualLight/Solo RDK
From: Stefan Riedmüller @ 2017-12-22 7:33 UTC (permalink / raw)
To: Lothar Waßmann
Cc: shawnguo-DgEjT+Ai2ygdnm+yROfE0A, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
fabio.estevam-3arQi8VN3Tc, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Christian Hemp, Stefan Christ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20171220151008.4aa4b5a5-AvR2QvxeiV7DiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
Hi,
On 20.12.2017 15:10, Lothar Waßmann wrote:
> Hi,
>
> On Wed, 20 Dec 2017 14:29:25 +0100 Stefan Riedmueller wrote:
>> From: Christian Hemp <c.hemp-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
>>
>> Add support for the PHYTEC phyBOARD-Mira Low-Cost Rapid Development Kit
>> with i.MX 6DualLight/Solo with NAND.
>>
> s/phxBOARD/phyBOARD/ in the subject line.
>
>
> Lothar Waßmann
I'll fix it, thanks.
Stefan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC 2/2] pci: dwc: pci-exynos: add the codes to support the exynos5433
From: Jingoo Han @ 2017-12-22 7:49 UTC (permalink / raw)
To: 'Jaehoon Chung', linux-pci-u79uwXL29TY76Z2rM5mHXA
Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, krzk-DgEjT+Ai2ygdnm+yROfE0A,
kgene-DgEjT+Ai2ygdnm+yROfE0A, lorenzo.pieralisi-5wv7dgnIgG8
In-Reply-To: <21cb6abf-5428-03cf-77f1-be762d96d156-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On Thursday, December 21, 2017 9:21 PM, Jaehoon Chung wrote:
> On 12/22/2017 01:12 AM, Jingoo Han wrote:
> > On Thursday, December 21, 2017 7:14 AM, Jaehoon Chung wrote:
> >>
> >> Exynos5433 has the PCIe for WiFi.
> >> Added the codes relevant to PCIe for supporting the exynos5433.
> >> Also changed the binding documentation name to
> >> 'samsung,exynos-pcie.txt'.
> >> (It's not only exynos5440 anymore.)
> >>
> >
> > I have no objection.
> > However, I added some comments about Exynos5440.
> >
> >> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >> ---
> >> ...exynos5440-pcie.txt => samsung,exynos-pcie.txt} | 2 +-
> >> drivers/pci/dwc/pci-exynos.c | 183
> > ++++++++++++++++-----
> >> 2 files changed, 144 insertions(+), 41 deletions(-)
> >> rename Documentation/devicetree/bindings/pci/{samsung,exynos5440-
> pcie.txt
> >> => samsung,exynos-pcie.txt} (97%)
> >>
> >> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-
> >> pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos-
> pcie.txt
> >> similarity index 97%
> >> rename from Documentation/devicetree/bindings/pci/samsung,exynos5440-
> >> pcie.txt
> >> rename to Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> >> index 34a11bfbfb60..958dcc150505 100644
> >> --- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> >> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> >> @@ -4,7 +4,7 @@ This PCIe host controller is based on the Synopsys
> >> DesignWare PCIe IP
> >> and thus inherits all the common properties defined in designware-
> >> pcie.txt.
> >>
> >> Required properties:
> >> -- compatible: "samsung,exynos5440-pcie"
> >> +- compatible: "samsung,exynos5440-pcie" or "samsung,exynos5433-pcie"
> >> - reg: base addresses and lengths of the PCIe controller,
> >> the PHY controller, additional register for the PHY controller.
> >> (Registers for the PHY controller are DEPRECATED.
> >> diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-
> exynos.c
> >> index 5596fdedbb94..8dee2e90347e 100644
> >> --- a/drivers/pci/dwc/pci-exynos.c
> >> +++ b/drivers/pci/dwc/pci-exynos.c
> >> @@ -40,6 +40,8 @@
> >> #define PCIE_IRQ_SPECIAL 0x008
> >> #define PCIE_IRQ_EN_PULSE 0x00c
> >> #define PCIE_IRQ_EN_LEVEL 0x010
> >> +#define PCIE_SW_WAKE 0x018
> >> +#define PCIE_BUS_EN BIT(1)
> >> #define IRQ_MSI_ENABLE BIT(2)
> >> #define PCIE_IRQ_EN_SPECIAL 0x014
> >> #define PCIE_PWR_RESET 0x018
> >> @@ -49,7 +51,8 @@
> >> #define PCIE_NONSTICKY_RESET 0x024
> >> #define PCIE_APP_INIT_RESET 0x028
> >> #define PCIE_APP_LTSSM_ENABLE 0x02c
> >> -#define PCIE_ELBI_RDLH_LINKUP 0x064
> >> +#define PCIE_ELBI_RDLH_LINKUP 0x074
> >
> > The address of this register should be 0x064 for exynos5440.
> > Howe about the following?
> >
> > +#define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP 0x064
> > +#define PCIE_ELBI_RDLH_LINKUP 0x074
> >
> > Or you can add the following.
> >
> > /* Exynos5440 PCIe ELBI registers */
> > #define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP 0x064
>
> Maybe, you're right. Because i didn't have Exynos5440 TRM, it's problem to
> me about updating other SoCs.
> I have checked almost all variants Exynos. They are using the LINKUP
> register as 0x74.
>
> If i can get the exynos5440 TRM, it's much helpful to me. Is it possible?
I don't have Exynos5440 TRM.
Please ask other guys.
Best regards,
Jingoo Han
>
> >
> >> +#define PCIE_ELBI_XMLH_LINKUP BIT(4)
> >> #define PCIE_ELBI_LTSSM_ENABLE 0x1
> >> #define PCIE_ELBI_SLV_AWMISC 0x11c
> >> #define PCIE_ELBI_SLV_ARMISC 0x120
> >> @@ -94,6 +97,10 @@
> >> #define PCIE_PHY_TRSV3_PD_TSV BIT(7)
> >> #define PCIE_PHY_TRSV3_LVCC 0x31c
> >>
> >> +/* DBI register */
> >> +#define PCIE_MISC_CONTROL_1_OFF 0x8BC
> >> +#define DBI_RO_WR_EN BIT(0)
> >> +
> >> struct exynos_pcie_mem_res {
> >> void __iomem *elbi_base; /* DT 0th resource: PCIe CTRL */
> >> void __iomem *phy_base; /* DT 1st resource: PHY CTRL */
> >> @@ -221,6 +228,96 @@ static const struct exynos_pcie_ops
> >> exynos5440_pcie_ops = {
> >> .deinit_clk_resources = exynos5440_pcie_deinit_clk_resources,
> >> };
> >>
> >> +static int exynos5433_pcie_get_mem_resources(struct platform_device
> > *pdev,
> >> + struct exynos_pcie *ep)
> >> +{
> >> + struct dw_pcie *pci = ep->pci;
> >> + struct device *dev = pci->dev;
> >> + struct resource *res;
> >> +
> >> + ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL);
> >> + if (!ep->mem_res)
> >> + return -ENOMEM;
> >> +
> >> + /* External Local Bus interface(ELBI) Register */
> >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
> >> + ep->mem_res->elbi_base = devm_ioremap_resource(&pdev->dev, res);
> >> + if (IS_ERR(ep->mem_res->elbi_base))
> >> + return PTR_ERR(ep->mem_res->elbi_base);
> >> +
> >> + /* Data Bus Interface(DBI) Register */
> >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> >> + pci->dbi_base = devm_ioremap_resource(&pdev->dev, res);
> >> + if (IS_ERR(pci->dbi_base))
> >> + return PTR_ERR(pci->dbi_base);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int exynos5433_pcie_get_clk_resources(struct exynos_pcie *ep)
> >> +{
> >> + struct dw_pcie *pci = ep->pci;
> >> + struct device *dev = pci->dev;
> >> +
> >> + ep->clk_res = devm_kzalloc(dev, sizeof(*ep->clk_res), GFP_KERNEL);
> >> + if (!ep->clk_res)
> >> + return -ENOMEM;
> >> +
> >> + ep->clk_res->clk = devm_clk_get(dev, "pcie");
> >> + if (IS_ERR(ep->clk_res->clk)) {
> >> + dev_err(dev, "Failed to get pcie rc clock\n");
> >> + return PTR_ERR(ep->clk_res->clk);
> >> + }
> >> +
> >> + ep->clk_res->bus_clk = devm_clk_get(dev, "pcie_bus");
> >> + if (IS_ERR(ep->clk_res->bus_clk)) {
> >> + dev_err(dev, "Failed to get pcie bus clock\n");
> >> + return PTR_ERR(ep->clk_res->bus_clk);
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static void exynos5433_pcie_deinit_clk_resources(struct exynos_pcie
> *ep)
> >> +{
> >> + clk_disable_unprepare(ep->clk_res->bus_clk);
> >> + clk_disable_unprepare(ep->clk_res->clk);
> >> +}
> >> +
> >> +
> >> +static int exynos5433_pcie_init_clk_resources(struct exynos_pcie *ep)
> >> +{
> >> + struct dw_pcie *pci = ep->pci;
> >> + struct device *dev = pci->dev;
> >> + int ret;
> >> +
> >> + ret = clk_prepare_enable(ep->clk_res->clk);
> >> + if (ret) {
> >> + dev_err(dev, "cannot enable pcie rc clock");
> >> + return ret;
> >> + }
> >> +
> >> + ret = clk_prepare_enable(ep->clk_res->bus_clk);
> >> + if (ret) {
> >> + dev_err(dev, "cannot enable pcie bus clock");
> >> + goto err_bus_clk;
> >> + }
> >> +
> >> + return 0;
> >> +
> >> +err_bus_clk:
> >> + clk_disable_unprepare(ep->clk_res->clk);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static const struct exynos_pcie_ops exynos5433_pcie_ops = {
> >> + .get_mem_resources = exynos5433_pcie_get_mem_resources,
> >> + .get_clk_resources = exynos5433_pcie_get_clk_resources,
> >> + .init_clk_resources = exynos5433_pcie_init_clk_resources,
> >> + .deinit_clk_resources = exynos5433_pcie_deinit_clk_resources,
> >> +};
> >> +
> >> static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
> >> {
> >> writel(val, base + reg);
> >> @@ -279,7 +376,9 @@ static void exynos_pcie_deassert_core_reset(struct
> >> exynos_pcie *ep)
> >> exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_NONSTICKY_RESET);
> >> exynos_pcie_writel(ep->mem_res->elbi_base, 1, PCIE_APP_INIT_RESET);
> >> exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_APP_INIT_RESET);
> >> - exynos_pcie_writel(ep->mem_res->block_base, 1, PCIE_PHY_MAC_RESET);
> >> + if (ep->mem_res->block_base)
> >> + exynos_pcie_writel(ep->mem_res->block_base, 1,
> >> + PCIE_PHY_MAC_RESET);
> >
> > Good.
> >
> >> }
> >>
> >> static void exynos_pcie_assert_phy_reset(struct exynos_pcie *ep)
> >> @@ -413,9 +512,6 @@ static int exynos_pcie_establish_link(struct
> >> exynos_pcie *ep)
> >> if (ep->using_phy) {
> >> phy_reset(ep->phy);
> >>
> >> - exynos_pcie_writel(ep->mem_res->elbi_base, 1,
> >> - PCIE_PWR_RESET);
> >> -
> >> phy_power_on(ep->phy);
> >> phy_init(ep->phy);
> >> } else {
> >> @@ -430,14 +526,16 @@ static int exynos_pcie_establish_link(struct
> >> exynos_pcie *ep)
> >> udelay(500);
> >> exynos_pcie_writel(ep->mem_res->block_base, 0,
> >> PCIE_PHY_COMMON_RESET);
> >> + exynos_pcie_deassert_core_reset(ep);
> >> }
> >>
> >> - /* pulse for common reset */
> >> - exynos_pcie_writel(ep->mem_res->block_base, 1,
> >> PCIE_PHY_COMMON_RESET);
> >> - udelay(500);
> >> - exynos_pcie_writel(ep->mem_res->block_base, 0,
> >> PCIE_PHY_COMMON_RESET);
> >
> > These codes are also necessary for Exyno5440.
> > How about moving these codes instead of removing them?
> >
> > @@ -430,14 +526,16 @@ static int exynos_pcie_establish_link(struct
> > exynos_pcie *ep)
> > udelay(500);
> > exynos_pcie_writel(ep->mem_res->block_base, 0,
> > PCIE_PHY_COMMON_RESET);
> > + /* pulse for common reset */
> > + exynos_pcie_writel(ep->mem_res->block_base, 1,
> > + PCIE_PHY_COMMON_RESET);
> > + udelay(500);
> > + exynos_pcie_writel(ep->mem_res->block_base, 0,
> > + PCIE_PHY_COMMON_RESET);
> > + exynos_pcie_deassert_core_reset(ep);
> > }
> >
> >
> >> + /*
> >> + * Enable DBI_RO_WR_EN bit.
> >> + * - When set to 1, some RO and HWinit bits are wriatble from
> >> + * the local application through the DBI.
> >> + */
> >> + dw_pcie_writel_dbi(pci, PCIE_MISC_CONTROL_1_OFF, DBI_RO_WR_EN);
> >>
> >> - exynos_pcie_deassert_core_reset(ep);
> >> dw_pcie_setup_rc(pp);
> >> exynos_pcie_assert_reset(ep);
> >>
> >> @@ -472,16 +570,6 @@ static void exynos_pcie_clear_irq_pulse(struct
> >> exynos_pcie *ep)
> >> exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_PULSE);
> >> }
> >>
> >> -static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
> >> -{
> >> - u32 val;
> >> -
> >> - /* enable INTX interrupt */
> >> - val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
> >> - IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
> >> - exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
> >> -}
> >> -
> >> static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
> >> {
> >> struct exynos_pcie *ep = arg;
> >> @@ -513,9 +601,16 @@ static void exynos_pcie_msi_init(struct
> exynos_pcie
> >> *ep)
> >> exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_LEVEL);
> >> }
> >>
> >> -static void exynos_pcie_enable_interrupts(struct exynos_pcie *ep)
> >> +static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
> >> {
> >> - exynos_pcie_enable_irq_pulse(ep);
> >> + u32 val;
> >> +
> >> + val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
> >> + IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
> >> + exynos_pcie_writel(ep->mem_res->elbi_base, val, PCIE_IRQ_EN_PULSE);
> >> +
> >> + exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_LEVEL);
> >> + exynos_pcie_writel(ep->mem_res->elbi_base, 0, PCIE_IRQ_EN_SPECIAL);
> >
> > Good.
> >
> >>
> >> if (IS_ENABLED(CONFIG_PCI_MSI))
> >> exynos_pcie_msi_init(ep);
> >> @@ -575,10 +670,8 @@ static int exynos_pcie_link_up(struct dw_pcie *pci)
> >> u32 val;
> >>
> >> val = exynos_pcie_readl(ep->mem_res->elbi_base,
> >> PCIE_ELBI_RDLH_LINKUP);
> >> - if (val == PCIE_ELBI_LTSSM_ENABLE)
> >> - return 1;
> >
> > Exynos5440 uses 'PCIE_ELBI_LTSSM_ENABLE'.
> > Can you keep this code for Exyno5440?
>
> It's possible to keep, but if it has to keep, then it needs to distinguish
> between exynos5440 and other exynos.
> Although I already mentioned, i needs to get Exynos5440 TRM. :)
>
> Best Regards,
> Jaehoon Chung
>
> >
> > This register can be added as below.
> >
> > /* Exynos5440 PCIe ELBI registers */
> > #define EXYNOS5440_PCIE_ELBI_RDLH_LINKUP 0x064
> > #define EXYNOS5440_PCIE_ELBI_LTSSM_ENABLE BIT(0)
> >
> > Best regards,
> > Jingoo Han
> >
> >>
> >> - return 0;
> >> + return (val & PCIE_ELBI_XMLH_LINKUP);
> >> }
> >>
> >> static int exynos_pcie_host_init(struct pcie_port *pp)
> >> @@ -587,7 +680,7 @@ static int exynos_pcie_host_init(struct pcie_port
> *pp)
> >> struct exynos_pcie *ep = to_exynos_pcie(pci);
> >>
> >> exynos_pcie_establish_link(ep);
> >> - exynos_pcie_enable_interrupts(ep);
> >> + exynos_pcie_enable_irq_pulse(ep);
> >>
> >> return 0;
> >> }
> >> @@ -608,8 +701,11 @@ static int __init exynos_add_pcie_port(struct
> >> exynos_pcie *ep,
> >>
> >> pp->irq = platform_get_irq(pdev, 1);
> >> if (pp->irq < 0) {
> >> - dev_err(dev, "failed to get irq\n");
> >> - return pp->irq;
> >> + pp->irq = platform_get_irq_byname(pdev, "intr");
> >> + if (pp->irq < 0) {
> >> + dev_err(dev, "failed to get irq\n");
> >> + return pp->irq;
> >> + }
> >> }
> >> ret = devm_request_irq(dev, pp->irq, exynos_pcie_irq_handler,
> >> IRQF_SHARED, "exynos-pcie", ep);
> >> @@ -678,13 +774,23 @@ static int __init exynos_pcie_probe(struct
> >> platform_device *pdev)
> >>
> >> ep->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> >>
> >> - /* Assume that controller doesn't use the PHY framework */
> >> - ep->using_phy = false;
> >> + /*
> >> + * In case of Exynos5440,
> >> + * Assume that controller doesn't use the PHY frameork.
> >> + * Other SoCs might be used the PHY framework.
> >> + */
> >> +
> >> + if (of_device_is_compatible(np, "samsung,exynos5440-pcie"))
> >> + ep->using_phy = false;
> >>
> >> - ep->phy = devm_of_phy_get(dev, np, NULL);
> >> + ep->phy = devm_of_phy_get(dev, np, "pcie-phy");
> >> if (IS_ERR(ep->phy)) {
> >> if (PTR_ERR(ep->phy) == -EPROBE_DEFER)
> >> return PTR_ERR(ep->phy);
> >> + if (!of_device_is_compatible(np, "samsung,exynos5440-pcie"))
> >> {
> >> + dev_err(dev, "Can't find the pcie-phy\n");
> >> + return PTR_ERR(ep->phy);
> >> + }
> >> dev_warn(dev, "Use the 'phy' property. Current DT of pci-
> >> exynos was deprecated!!\n");
> >> } else
> >> ep->using_phy = true;
> >> @@ -734,23 +840,20 @@ static int __exit exynos_pcie_remove(struct
> >> platform_device *pdev)
> >> static const struct of_device_id exynos_pcie_of_match[] = {
> >> {
> >> .compatible = "samsung,exynos5440-pcie",
> >> - .data = &exynos5440_pcie_ops
> >> + .data = &exynos5440_pcie_ops,
> >> + }, {
> >> + .compatible = "samsung,exynos5433-pcie",
> >> + .data = &exynos5433_pcie_ops,
> >> },
> >> {},
> >> };
> >>
> >> static struct platform_driver exynos_pcie_driver = {
> >> + .probe = exynos_pcie_probe,
> >> .remove = __exit_p(exynos_pcie_remove),
> >> .driver = {
> >> .name = "exynos-pcie",
> >> .of_match_table = exynos_pcie_of_match,
> >> },
> >> };
> >> -
> >> -/* Exynos PCIe driver does not allow module unload */
> >> -
> >> -static int __init exynos_pcie_init(void)
> >> -{
> >> - return platform_driver_probe(&exynos_pcie_driver,
> >> exynos_pcie_probe);
> >> -}
> >> -subsys_initcall(exynos_pcie_init);
> >> +builtin_platform_driver(exynos_pcie_driver);
> >> --
> >> 2.15.1
> >
> >
> >
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 2/3] PCI: mobiveil: Add Mobiveil PCIe Host Bridge IP driver
From: Subrahmanya Lingappa @ 2017-12-22 7:59 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-pci, Bjorn Helgaas, Marc Zyngier, robh, devicetree,
Mingkai Hu, Peter W Newton, M.h. Lian, Raj Raina, Rajan Kapoor,
Prabhjot Singh
In-Reply-To: <20171220170307.GC1709@red-moon>
Lorenzo,
On Wed, Dec 20, 2017 at 10:33 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Wed, Dec 13, 2017 at 11:08:37AM -0500, Subrahmanya Lingappa wrote:
> > Adds driver for Mobiveil AXI PCIe Host Bridge Soft IP -
> > GPEX 4.0, a PCIe gen4 IP. This IP has upto 8
> > outbound and inbound windows for the address translation.
> >
> > Signed-off-by: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > Cc: linux-pci@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> > ---
> > drivers/pci/host/Kconfig | 8 +
> > drivers/pci/host/Makefile | 1 +
> > drivers/pci/host/pcie-mobiveil.c | 653 +++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 662 insertions(+)
> > create mode 100644 drivers/pci/host/pcie-mobiveil.c
> >
> > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> > index 38d1298..09bf1d9 100644
> > --- a/drivers/pci/host/Kconfig
> > +++ b/drivers/pci/host/Kconfig
> > @@ -27,6 +27,14 @@ config PCIE_XILINX_NWL
> > or End Point. The current option selection will only
> > support root port enabling.
> >
> > +config PCIE_MOBIVEIL
> > + bool "Mobiveil AXI PCIe host bridge support"
> > + depends on ARCH_ZYNQMP || COMPILE_TEST
> > + depends on PCI_MSI_IRQ_DOMAIN
>
> There is no PCI_MSI_IRQ_DOMAIN dependency in this patch.
>
> > + help
> > + Say 'Y' here if you want kernel to support the Mobiveil AXI PCIe
> > + Host Bridge driver.
> > +
> > config PCI_FTPCI100
> > bool "Faraday Technology FTPCI100 PCI controller"
> > depends on OF
> > diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> > index 34ec1d8..d745d68 100644
> > --- a/drivers/pci/host/Makefile
> > +++ b/drivers/pci/host/Makefile
> > @@ -23,6 +23,7 @@ obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
> > obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o
> > obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o
> > obj-$(CONFIG_VMD) += vmd.o
> > +obj-$(CONFIG_PCIE_MOBIVEIL) += pcie-mobiveil.o
> >
> > # The following drivers are for devices that use the generic ACPI
> > # pci_root.c driver but don't support standard ECAM config access.
> > diff --git a/drivers/pci/host/pcie-mobiveil.c b/drivers/pci/host/pcie-mobiveil.c
> > new file mode 100644
> > index 0000000..8611aaa
> > --- /dev/null
> > +++ b/drivers/pci/host/pcie-mobiveil.c
> > @@ -0,0 +1,653 @@
> > +/*
> > + * PCIe host controller driver for Mobiveil PCIe Host controller
> > + *
> > + * SPDX-License-Identifier: GPL-2.0
> > + * Copyright (c) 2017 Mobiveil Inc.
> > + * Author: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/init.h>
> > +#include <linux/irq.h>
> > +#include <linux/irqchip/chained_irq.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/of_pci.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +
> > +/* register offsets and bit positions */
> > +
> > +/*
> > + * translation tables are grouped into windows, each window registers are
> > + * grouped into blocks of 4 or 16 registers each
> > + */
> > +#define PAB_REG_BLOCK_SIZE_4 4
> > +#define PAB_REG_BLOCK_SIZE_16 16
>
> What I wanted to say is that you can tag the block with a name
> instead of using a number.
>
> I can say that block size 4 is only for PAB_EXT_*, you can tag
> it as such.
>
> eg PAB_EXT_REG_BLOCK_SIZE
Ok, I'll rename them as below, and use them accordingly.
#define PAB_REG_BLOCK_SIZE 16
#define PAB_EXT_REG_BLOCK_SIZE 4
>
>
> > +#define PAB_REG_ADDR_4(offset, win) (offset + (win * PAB_REG_BLOCK_SIZE_4))
> > +#define PAB_REG_ADDR_16(offset, win) (offset + (win * PAB_REG_BLOCK_SIZE_16))
> > +
> > +#define LTSSM_STATUS 0x0404
> > +#define LTSSM_STATUS_L0_MASK 0x3f
> > +#define LTSSM_STATUS_L0 0x2d
> > +
> > +#define PAB_CTRL 0x0808
> > +#define AMBA_PIO_ENABLE_SHIFT 0
> > +#define PEX_PIO_ENABLE_SHIFT 1
> > +#define PAGE_SEL_SHIFT 13
> > +#define PAGE_SEL_MASK 0x3f
> > +#define PAGE_LO_MASK 0x3ff
> > +#define PAGE_SEL_EN 0xc00
> > +#define PAGE_SEL_OFFSET_SHIFT 10
> > +
> > +#define PAB_AXI_PIO_CTRL 0x0840
> > +#define APIO_EN_MASK 0xf
> > +
> > +#define PAB_PEX_PIO_CTRL 0x08c0
> > +#define PIO_ENABLE_SHIFT 0
> > +
> > +#define PAB_INTP_AMBA_MISC_ENB 0x0b0c
> > +#define PAB_INTP_AMBA_MISC_STAT 0x0b1c
> > +#define PAB_INTP_INTX_MASK 0x1e0
> > +
> > +#define PAB_AXI_AMAP_CTRL(win) PAB_REG_ADDR_16(0x0ba0, win)
> > +#define WIN_ENABLE_SHIFT 0
> > +#define WIN_TYPE_SHIFT 1
> > +#define WIN_SIZE_SHIFT 10
> > +
> > +#define PAB_EXT_AXI_AMAP_SIZE(win) PAB_REG_ADDR_4(0xbaf0, win)
> > +
> > +#define PAB_AXI_AMAP_AXI_WIN(win) PAB_REG_ADDR_16(0x0ba4, win)
> > +#define AXI_WINDOW_BASE_SHIFT 2
> > +
> > +#define PAB_AXI_AMAP_PEX_WIN_L(win) PAB_REG_ADDR_16(0x0ba8, win)
> > +#define PAB_BUS_SHIFT 24
> > +#define PAB_DEVICE_SHIFT 19
> > +#define PAB_FUNCTION_SHIFT 16
> > +
> > +#define PAB_AXI_AMAP_PEX_WIN_H(win) PAB_REG_ADDR_16(0x0bac, win)
> > +#define PAB_INTP_AXI_PIO_CLASS 0x474
>
> This one is a bit odd here, move it up in the file.
>
> > +#define PAB_PEX_AMAP_CTRL(win) PAB_REG_ADDR_16(0x4ba0, win)
> > +#define AMAP_CTRL_EN_SHIFT 0
> > +#define AMAP_CTRL_TYPE_SHIFT 1
> > +
> > +#define PAB_EXT_PEX_AMAP_SIZEN(win) PAB_REG_ADDR_4(0xbef0, win)
> > +#define PAB_PEX_AMAP_AXI_WIN(win) PAB_REG_ADDR_16(0x4ba4, win)
> > +#define PAB_PEX_AMAP_PEX_WIN_L(win) PAB_REG_ADDR_16(0x4ba8, win)
> > +#define PAB_PEX_AMAP_PEX_WIN_H(win) PAB_REG_ADDR_16(0x4bac, win)
>
> Here you have three base offsets:
>
> 0xb00
> 0x4000
> 0xb000
>
> you can create a macro for each of them, according to what they
> represent and then add an offset into each.
>
> > +
> > +/* supported number of interrupts */
> > +#define PCI_NUM_INTX 4
>
> It is already defined in linux/pci.h
>
> > +#define PAB_INTA_POS 5
> > +
> > +/* outbound and inbound window definitions */
> > +#define WIN_NUM_0 0
> > +#define WIN_NUM_1 1
> > +#define CFG_WINDOW_TYPE 0
> > +#define IO_WINDOW_TYPE 1
> > +#define MEM_WINDOW_TYPE 2
> > +#define IB_WIN_SIZE (256 * 1024 * 1024)
> > +#define MAX_PIO_WINDOWS 8
> > +
> > +/* Parameters for the waiting for link up routine */
> > +#define LINK_WAIT_MAX_RETRIES 10
> > +#define LINK_WAIT_MIN 90000
> > +#define LINK_WAIT_MAX 100000
> > +
> > +#ifndef UINT64_MAX
> > +#define UINT64_MAX (u64)(~((u64)0))
> > +#endif
> > +
> > +struct mobiveil_pcie {
> > + struct platform_device *pdev;
> > + struct list_head resources;
> > + void __iomem *config_axi_slave_base; /* endpoint config base */
> > + void __iomem *csr_axi_slave_base; /* root port config base */
> > + struct irq_domain *intx_domain;
> > + int irq;
> > + int apio_wins;
> > + int ppio_wins;
> > + int ob_wins_configured; /* configured outbound windows */
> > + int ib_wins_configured; /* configured inbound windows */
> > + struct resource *ob_io_res;
> > + char root_bus_nr;
> > +};
> > +
> > +static inline void csr_writel(struct mobiveil_pcie *pcie, const u32 value,
> > + const u32 reg)
> > +{
> > + writel_relaxed(value, pcie->csr_axi_slave_base + reg);
> > +}
> > +
> > +static inline u32 csr_readl(struct mobiveil_pcie *pcie, const u32 reg)
> > +{
> > + return readl_relaxed(pcie->csr_axi_slave_base + reg);
> > +}
> > +
> > +static bool mobiveil_pcie_link_up(struct mobiveil_pcie *pcie)
> > +{
> > + return (csr_readl(pcie, LTSSM_STATUS) &
> > + LTSSM_STATUS_L0_MASK) == LTSSM_STATUS_L0;
> > +}
>
> https://marc.info/?l=linux-pci&m=151329230814614&w=2
>
> Bjorn would like to remove it.
I do not have a solid explanation for this, as I followed the earlier
driver framework. I'll remove it for now.
>
>
> > +static bool mobiveil_pcie_valid_device(struct pci_bus *bus, unsigned int devfn)
> > +{
> > + struct mobiveil_pcie *pcie = bus->sysdata;
> > +
> > + /* Check if link is up when trying to access downstream ports */
> > + if (bus->number != pcie->root_bus_nr)
> > + if (!mobiveil_pcie_link_up(pcie))
> > + return false;
> > +
> > + /* Only one device down on each root port */
> > + if ((bus->number == pcie->root_bus_nr) && (devfn > 0))
> > + return false;
> > +
> > + /*
> > + * Do not read more than one device on the bus directly
> > + * attached to RC
> > + */
> > + if ((bus->primary == pcie->root_bus_nr) && (devfn > 0))
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +/*
> > + * mobiveil_pcie_map_bus - routine to get the configuration base of either
> > + * root port or endpoint
> > + */
> > +static void __iomem *mobiveil_pcie_map_bus(struct pci_bus *bus,
> > + unsigned int devfn, int where)
> > +{
> > + struct mobiveil_pcie *pcie = bus->sysdata;
> > +
> > + if (!mobiveil_pcie_valid_device(bus, devfn))
> > + return NULL;
> > +
> > + if (bus->number == pcie->root_bus_nr) {
> > + /* RC config access */
> > + return pcie->csr_axi_slave_base + where;
> > + } else {
> > + /*
> > + * EP config access (in Config/APIO space)
> > + * Program PEX Address base (31..16 bits) with appropriate value
> > + * (BDF) in PAB_AXI_AMAP_PEX_WIN_L0 Register.
> > + * Relies on pci_lock serialization
> > + */
> > + csr_writel(pcie,
> > + bus->number << PAB_BUS_SHIFT |
> > + PCI_SLOT(devfn) << PAB_DEVICE_SHIFT |
> > + PCI_FUNC(devfn) << PAB_FUNCTION_SHIFT,
> > + PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0));
> > + return pcie->config_axi_slave_base + where;
> > + }
> > +}
> > +
> > +static struct pci_ops mobiveil_pcie_ops = {
> > + .map_bus = mobiveil_pcie_map_bus,
> > + .read = pci_generic_config_read,
> > + .write = pci_generic_config_write,
> > +};
> > +
> > +static void mobiveil_pcie_isr(struct irq_desc *desc)
> > +{
> > + struct irq_chip *chip = irq_desc_get_chip(desc);
> > + struct mobiveil_pcie *pcie = irq_desc_get_handler_data(desc);
> > + struct device *dev = &pcie->pdev->dev;
> > + u32 intr_status;
> > + unsigned long shifted_status;
>
> Why not u32 ?
>
for_each_set_bit() api warns about the variable not being unsigned
long. So used the same to take out the warning.
> > + u32 bit, virq;
> > + u32 val, mask;
> > +
> > + chained_irq_enter(chip, desc);
> > +
> > + /* read INTx status */
> > + val = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
> > + mask = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> > + intr_status = val & mask;
> > +
> > + /*
> Handle INTx */
> > + if (intr_status & PAB_INTP_INTX_MASK) {
> > + shifted_status = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
>
> Why can't you reuse val to read the status and create shifted_status
> from it ?
>
> eg
>
> shifted_status = val << PAB_INTA_POS;
> csr_writel(pcie, shifted_status, PAB_INTP_AMBA_MISC_STAT);
>
> Just a hint to make the loop more readable. BTW, I do not think that
> writing shifted_status into that register is OK, see below.
>
> > + do {
> > + for_each_set_bit(bit, &shifted_status, PCI_NUM_INTX) {
> > +
> > + /* clear interrupts */
> > + csr_writel(pcie, shifted_status <<
> PAB_INTA_POS,
> > +
> PAB_INTP_AMBA_MISC_STAT);
>
> Should not you clear just the interrupt you are about to handle ?
PAB_INTP_AMBA_MISC_STAT register has INTA, INTB, INTC, INTD status
bits at positions 5,6,7 and 8,
they are RW1C bits, so writing 1 to this bit clears the status.
So here the status read was written back to it to clear.
>
>
> > + virq = irq_find_mapping(pcie->intx_domain,
> > + bit + 1);
> > + if (virq)
> > +
> generic_handle_irq(virq);
> > + else
> > + dev_err_ratelimited(dev,
> > + "unexpected IRQ, INT%d\n",
> > + bit);
> > +
> > + }
> > +
> > + shifted_status = csr_readl(pcie,
> > + PAB_INTP_AMBA_MISC_STAT);
> > +
> > + } while ((shifted_status >> PAB_INTA_POS) != 0);
>
> I do not understand this. Can you explain to me how the
> register at:
>
> PAB_INTP_AMBA_MISC_STAT
>
> works ?
>
just repeating what explained before, to ease the readablility.
PAB_INTP_AMBA_MISC_STAT register has INTA, INTB, INTC, INTD status
bits at positions 5,6,7 and 8,
they are RW1C bits, so writing 1 to this bit clears the status.
>
> A patch for mediatek has been posted:
>
> https://patchwork.ozlabs.org/patch/851181/
>
> It looks like this loop may suffer from the same issue, so please do
> have a look.
>
I will clear the the interrupt after
its hadled by generic_handle_irq()
.
right ?
>
> On top of that, most of the operations you are carrying out here
> can be done automatically by making them part of the struct irq_chip
> methods (ie acking IRQs, masking IRQs, etc, see below).
>
Any side effects of keeping this code as is ?
>
> > + }
> > +
> > + csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);
>
> > + chained_irq_exit(chip, desc);
> > +}
> > +
> > +static int mobiveil_pcie_parse_dt(struct mobiveil_pcie *pcie)
> > +{
> > + struct device *dev = &pcie->pdev->dev;
> > + struct platform_device *pdev = pcie->pdev;
> > + struct device_node *node = dev->of_node;
> > + struct resource *res;
> > + const char *type;
> > +
> > + type = of_get_property(node, "device_type", NULL);
> > + if (!type || strcmp(type, "pci")) {
> > + dev_err(dev, "invalid \"device_type\" %s\n", type);
> > + return -EINVAL;
> > + }
> > +
> > + /* map config resource */
> > + res = platform_get_resource_byname(pdev,
> > + IORESOURCE_MEM, "config_axi_slave");
> > + pcie->config_axi_slave_base = devm_pci_remap_cfg_resource(dev, res);
> > + if (IS_ERR(pcie->config_axi_slave_base))
> > + return PTR_ERR(pcie->config_axi_slave_base);
> > + pcie->ob_io_res = res;
> > +
> > + /* map csr resource */
> > + res = platform_get_resource_byname(pdev,
> > + IORESOURCE_MEM, "csr_axi_slave");
> > + pcie->csr_axi_slave_base = devm_pci_remap_cfg_resource(dev, res);
> > + if (IS_ERR(pcie->csr_axi_slave_base))
> > + return PTR_ERR(pcie->csr_axi_slave_base);
> > +
> > + /* read the number of windows requested */
> > + if (!pcie->apio_wins &&
> > + of_property_read_u32(node, "apio-wins", &pcie->apio_wins)) {
> > + pcie->apio_wins = MAX_PIO_WINDOWS;
> > + }
> > +
> > + if (!pcie->ppio_wins &&
>
> What I wanted to say in my previous review is that apio_wins and
> ppio_wins are zero here why would they be any different and why
> do you need to check that.
Understood. I will remove this redundant check.
>
>
> > + of_property_read_u32(node, "ppio-wins", &pcie->ppio_wins)) {
> > + pcie->ppio_wins = MAX_PIO_WINDOWS;
> > + }
> > +
> > + pcie->irq = platform_get_irq(pdev, 0);
> > + if (pcie->irq <= 0) {
> > + dev_err(dev, "failed to map IRQ: %d\n", pcie->irq);
> > + return -ENODEV;
> > + }
> > +
> > + irq_set_chained_handler_and_data(pcie->irq, mobiveil_pcie_isr, pcie);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * select_paged_register - routine to access paged register of root complex
> > + *
> > + * registers of RC are paged, for this scheme to work
> > + * extracted higher 6 bits of the offset will be written to pg_sel
> > + * field of PAB_CTRL register and rest of the lower 10 bits enabled with
> > + * PAGE_SEL_EN are used as offset of the register.
> > + *
> > + */
> > +static void select_paged_register(struct mobiveil_pcie *pcie, u32 offset)
> > +{
> > + int pab_ctrl_dw, pg_sel;
> > +
> > + /* clear pg_sel field */
> > + pab_ctrl_dw = csr_readl(pcie, PAB_CTRL);
> > + pab_ctrl_dw = (pab_ctrl_dw & ~(PAGE_SEL_MASK << PAGE_SEL_SHIFT));
> > +
> > + /* set pg_sel field */
> > + pg_sel = (offset >> PAGE_SEL_OFFSET_SHIFT) & PAGE_SEL_MASK;
> > + pab_ctrl_dw |= ((pg_sel << PAGE_SEL_SHIFT));
> > + csr_writel(pcie, pab_ctrl_dw, PAB_CTRL);
> > +}
> > +
> > +static void write_paged_register(struct mobiveil_pcie *pcie,
> > + u32 val, u32 offset)
> > +{
> > + u32 off = (offset & PAGE_LO_MASK) | PAGE_SEL_EN;
> > +
> > + select_paged_register(pcie, offset);
> > + csr_writel(pcie, val, off);
> > +}
> > +
> > +static u32 read_paged_register(struct mobiveil_pcie *pcie, u32 offset)
> > +{
> > + u32 off = (offset & PAGE_LO_MASK) | PAGE_SEL_EN;
> > +
> > + select_paged_register(pcie, offset);
> > + return csr_readl(pcie, off);
> > +}
> > +
> > +static void program_ib_windows(struct mobiveil_pcie *pcie, int win_num,
> > + int pci_addr, int type, int size_kb)
> > +{
> > + int pio_ctrl_val;
> > + int amap_ctrl_dw;
> > + u64 size64 = (UINT64_MAX << (WIN_SIZE_SHIFT + ilog2(size_kb)));
> > +
> > + if ((pcie->ib_wins_configured + 1) > pcie->ppio_wins) {
> > + dev_err(&pcie->pdev->dev,
> > + "ERROR: max inbound windows reached !\n");
> > + return;
> > + }
> > +
> > + pio_ctrl_val = csr_readl(pcie, PAB_PEX_PIO_CTRL);
> > + csr_writel(pcie,
> > + pio_ctrl_val | (1 << PIO_ENABLE_SHIFT), PAB_PEX_PIO_CTRL);
> > + amap_ctrl_dw = read_paged_register(pcie, PAB_PEX_AMAP_CTRL(win_num));
> > + amap_ctrl_dw = (amap_ctrl_dw | (type << AMAP_CTRL_TYPE_SHIFT));
> > + amap_ctrl_dw = (amap_ctrl_dw | (1 << AMAP_CTRL_EN_SHIFT));
> > +
> > + write_paged_register(pcie, amap_ctrl_dw | lower_32_bits(size64),
> > + PAB_PEX_AMAP_CTRL(win_num));
> > +
> > + write_paged_register(pcie, upper_32_bits(size64),
> > + PAB_EXT_PEX_AMAP_SIZEN(win_num));
> > +
> > + write_paged_register(pcie, pci_addr, PAB_PEX_AMAP_AXI_WIN(win_num));
> > + write_paged_register(pcie, pci_addr, PAB_PEX_AMAP_PEX_WIN_L(win_num));
> > + write_paged_register(pcie, 0, PAB_PEX_AMAP_PEX_WIN_H(win_num));
> > +}
> > +
> > +static void program_ob_windows(struct mobiveil_pcie *pcie, int win_num,
> > + u64 cpu_addr, u64 pci_addr, int config_io_bit,
> > + int size_kb)
> > +{
> > +
> > + u32 value, type;
> > + u64
> size64 = (UINT64_MAX << (WIN_SIZE_SHIFT +
> ilog2(size_kb)));
>
> Can you explain to me the sizing mechanism please ? It is just to make
> sure I got it right.
this register follows the PCI BAR sizing convention.
size64 is actually ~(size-1),
Looks like we can just do that instead of jugglery of bit shifting and ilog2().
>
>
> > + if ((pcie->ob_wins_configured + 1) > pcie->apio_wins) {
> > + dev_err(&pcie->pdev->dev,
> > + "ERROR: max outbound windows reached !\n");
> > + return;
> > + }
> > +
> > + /*
> > + * program Enable Bit to 1, Type Bit to (00) base 2, AXI Window Size Bit
> > + * to 4 KB in PAB_AXI_AMAP_CTRL register
> > + */
> > + type = config_io_bit;
> > + value = csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num));
> > + csr_writel(pcie,
> > + 1 << WIN_ENABLE_SHIFT |
> > + type << WIN_TYPE_SHIFT |
> > + lower_32_bits(size64),
> > + PAB_AXI_AMAP_CTRL(win_num));
> > +
> > + write_paged_register(pcie, upper_32_bits(size64),
> > + PAB_EXT_AXI_AMAP_SIZE(win_num));
> > +
> > + /*
> > + *
> program
> AXI window base with appropriate value in
> > + * PAB_AXI_AMAP_AXI_WIN0 register
> > + */
> > + value = csr_readl(pcie, PAB_AXI_AMAP_AXI_WIN(win_num));
> > + csr_writel(pcie,
> > + cpu_addr >> AXI_WINDOW_BASE_SHIFT <<
> > + AXI_WINDOW_BASE_SHIFT, PAB_AXI_AMAP_AXI_WIN(win_num));
> > +
> > + value = csr_readl(pcie, PAB_AXI_AMAP_PEX_WIN_H(win_num));
>
> What's "value" used for in this function ?
My bad, it was used in the previous versions and not cleaned up. I will now.
>
>
>
> > + csr_writel(pcie, lower_32_bits(pci_addr),
> > + PAB_AXI_AMAP_PEX_WIN_L(win_num));
> > + csr_writel(pcie, upper_32_bits(pci_addr),
> > + PAB_AXI_AMAP_PEX_WIN_H(win_num));
> > +
> > + pcie->ob_wins_configured++;
> > +}
> > +
> > +static int mobiveil_bringup_link(struct mobiveil_pcie *pcie)
> > +{
> > + int retries;
> > +
> > + /* check if the link is up or not */
> > + for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
> > + if (mobiveil_pcie_link_up(pcie))
> > + return 0;
> > +
> > + usleep_range(LINK_WAIT_MIN, LINK_WAIT_MAX);
> > + }
> > + dev_err(&pcie->pdev->dev, "link never came up\n");
> > + return -ETIMEDOUT;
> > +}
> > +
> > +static int
> mobiveil_host_init(struct mobiveil_pcie *pcie)
> > +{
> > + u32 value;
> > + int type = 0;
>
> You reinitialize it later and it should not be an int but an unsigned
> type.
>
> > + u32 pab_ctrl;
> > + int err;
> > + struct resource_entry *win, *tmp;
> > +
> > + /* setup the PCIe slot link power*/
>
> Comment is useless, remove it.
>
> > + err = mobiveil_bringup_link(pcie);
> > + if (err) {
> > + dev_info(&pcie->pdev->dev, "link bring-up failed\n");
> > + return err;
> > + }
> > +
> > + /*
> > + * program Bus Master Enable Bit in Command Register in PAB Config
> > + * Space
> > + */
> > + value = csr_readl(pcie, PCI_COMMAND);
> > + csr_writel(pcie,
> > + value |
>
> Why is this on a separate line ?
>
> > + PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER,
> > + PCI_COMMAND);
> > +
> > + /*
> > + * program PIO Enable Bit to 1 (and PEX PIO Enable to 1) in PAB_CTRL
> > + * register
> > + */
> > + pab_ctrl = csr_readl(pcie, PAB_CTRL);
> > + csr_writel(pcie, pab_ctrl | (1 << AMBA_PIO_ENABLE_SHIFT) |
> > + (1 << PEX_PIO_ENABLE_SHIFT),
> > + PAB_CTRL);
> > +
> > + csr_writel(pcie, PAB_INTP_INTX_MASK, PAB_INTP_AMBA_MISC_ENB);
> > +
> > + /* program PIO Enable Bit to 1 and Config Window Enable Bit to 1 in
> > + * PAB_AXI_PIO_CTRL Register
> > + */
> > + value = csr_readl(pcie, PAB_AXI_PIO_CTRL);
> > + csr_writel(pcie, value | APIO_EN_MASK, PAB_AXI_PIO_CTRL);
> > +
> > + /*
> > + * we'll program one outbound window for config reads and
> > + * another default inbound window for all the upstream traffic
> > + * rest of the outbound windows will be configured according to
> > + * the "ranges" field defined in device tree
> > + */
> > +
> > + /* config outbound translation window */
> > + program_ob_windows(pcie, pcie->ob_wins_configured,
> > + pcie->ob_io_res->start, 0, CFG_WINDOW_TYPE,
> > + resource_size(pcie->ob_io_res)/1024);
> > +
> > + /* memory inbound translation window */
> > + program_ib_windows(pcie, WIN_NUM_1, 0, MEM_WINDOW_TYPE, IB_WIN_SIZE);
> > +
> > + /* Get the I/O and memory ranges from DT */
> > + resource_list_for_each_entry_safe(win, tmp, &pcie->resources) {
> > + type = 0;
> > + if (resource_type(win->res) == IORESOURCE_MEM)
> > + type = MEM_WINDOW_TYPE;
> > + if (resource_type(win->res) == IORESOURCE_IO)
> > + type = IO_WINDOW_TYPE;
> > + if (type) {
> > + /* configure outbound translation window */
> > + program_ob_windows(pcie, pcie->ob_wins_configured,
> > + win->res->start, 0, type,
> > + resource_size(win->res)/1024);
> > + }
> > + }
> > +
> > + return err;
> > +}
> > +
> > +/* routine to setup the INTx related data */
> > +static int mobiveil_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
> > + irq_hw_number_t hwirq)
> > +{
> > + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
>
> IIUC, the host bridge acts as an INTX/MSI controller/multiplexer.
>
> So, instead of relying on
> dummy_irq_chip, you should create your own
> irq_chip with the methods that you require (eg irq_ack(), irq_mask(),
> irq_compose_msi_msg()) and use that as bottom irq_chip for both
> INTX and MSI domains.
>
> That's why I asked you to have a look at pcie-tango.c (except that there
> INTX aren't supported but the basic idea does not change) and implement
> IRQ domains handling like that same driver.
>
are there any disadvantages of keeping dummy_irq_chip, as I see quite
a few host bridge
drivers including
otehr two major soft IP drivers from altera and xilinx with similar
MSI implementaion.
>
> > + irq_set_chip_data(irq, domain->host_data);
> > + return 0;
> > +}
> > +
> > +/* INTx domain opeartions structure */
>
> s/opeartions/operations
>
> > +static const struct irq_domain_ops intx_domain_ops = {
> > + .map = mobiveil_pcie_intx_map,
> > +};
> > +
> > +static int mobiveil_pcie_init_irq_domain(struct mobiveil_pcie *pcie)
> > +{
> > + struct device *dev = &pcie->pdev->dev;
> > + struct device_node *node = dev->of_node;
> > + int ret;
>
> ret is unused
>
> > + /* setup INTx */
> > + pcie->intx_domain = irq_domain_add_linear(node,
> > + PCI_NUM_INTX + 1, &intx_domain_ops, pcie);
>
> You should use PCI_NUM_INTX and add pci_irqd_intx_xlate() as the
> domain ops .xlate.
>
> > + if (!pcie->intx_domain) {
> > + dev_err(dev, "Failed to get a INTx IRQ domain\n");
> > + return -ENODEV;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int mobiveil_pcie_probe(struct platform_device *pdev)
> > +{
> > + struct mobiveil_pcie *pcie;
> > + struct pci_bus *bus;
> > + struct pci_bus *child;
> > + struct pci_host_bridge *bridge;
> > + struct device *dev = &pdev->dev;
> > + struct device_node *node = dev->of_node;
> > + resource_size_t iobase;
> > + int ret;
> > +
> > + /* allocate the PCIe port */
> > + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
> > + if (!bridge)
> > + return -ENODEV;
> > +
> > + pcie = pci_host_bridge_priv(bridge);
> > + if (!pcie)
> > + return -ENOMEM;
> > +
> > + pcie->pdev = pdev;
> > +
> > + /* parse the device tree */
>
> Remove this comment, it does not add anything.
>
> > + ret = mobiveil_pcie_parse_dt(pcie);
> > + if (ret) {
> > + dev_err(dev, "Parsing DT failed, ret: %x\n", ret);
> > + return ret;
> > + }
> > +
> > + INIT_LIST_HEAD(&pcie->resources);
> > +
> > + /* parse the host bridge base addresses from the device tree file */
> > + ret = of_pci_get_host_bridge_resources(node,
> > + 0, 0xff, &pcie->resources, &iobase);
> > + if (ret) {
> > + dev_err(dev, "Getting bridge resources failed\n");
> > + return -ENOMEM;
> > + }
> > +
> > + /*
> > + * configure all inbound and outbound windows and prepare the RC for
> > + * config access
> > + */
> > + ret = mobiveil_host_init(pcie);
> > + if (ret) {
> > + dev_err(dev, "Failed to initialize host\n");
> > + goto error;
> > + }
> > +
> > + /* fixup for PCIe class register */
> > + csr_writel(pcie, 0x060402ab, PAB_INTP_AXI_PIO_CLASS);
> > +
> > + /* initialize the IRQ domains */
> > + ret = mobiveil_pcie_init_irq_domain(pcie);
> > + if (ret) {
> > + dev_err(dev, "Failed creating IRQ Domain\n");
> > + goto error;
> > + }
> > +
> > +
>
> Two empty lines, should be one.
>
> > + ret = devm_request_pci_bus_resources(dev, &pcie->resources);
> > + if (ret)
> > + goto error;
> > +
> > + /* Initialize bridge */
> > + list_splice_init(&pcie->resources, &bridge->windows);
> > + bridge->dev.parent = dev;
> > + bridge->sysdata = pcie;
> > + bridge->busnr = pcie->root_bus_nr;
> > + bridge->ops = &mobiveil_pcie_ops;
> > + bridge->map_irq = of_irq_parse_and_map_pci;
> > + bridge->swizzle_irq = pci_common_swizzle;
> > +
> > + /* setup the kernel resources for the newly added PCIe root bus */
> > + ret = pci_scan_root_bus_bridge(bridge);
> > + if (ret)
> > + goto error;
> > +
> > + bus = bridge->bus;
> > +
> > + pci_assign_unassigned_bus_resources(bus);
> > + list_for_each_entry(child, &bus->children, node)
> > + pcie_bus_configure_settings(child);
> > + pci_bus_add_devices(bus);
> > +
> > + platform_set_drvdata(pdev, pcie);
>
> Is this really needed ?
>
> Thanks,
> Lorenzo
>
> > +
> > + return 0;
> > +error:
> > + pci_free_resource_list(&pcie->resources);
> > + return ret;
> > +}
> > +
> > +static const struct of_device_id mobiveil_pcie_of_match[] = {
> > + {.compatible = "mbvl,gpex40-pcie",},
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, mobiveil_pcie_of_match);
> > +
> > +static struct platform_driver mobiveil_pcie_driver = {
> > + .probe = mobiveil_pcie_probe,
> > + .driver = {
> > + .name = "mobiveil-pcie",
> > + .of_match_table = mobiveil_pcie_of_match,
> > + .suppress_bind_attrs = true,
> > + },
> > +};
> > +
> > +builtin_platform_driver(mobiveil_pcie_driver);
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("Mobiveil PCIe host controller driver");
> > +MODULE_AUTHOR("Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>");
> > --
> > 1.8.3.1
> >
Thanks,
Subrahmanya
^ permalink raw reply
* Re: [PATCH] ASoC: rcar: tidyup simple-card example for CPU node
From: Geert Uytterhoeven @ 2017-12-22 8:02 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: Mark Rutland, devicetree, Linux-ALSA, Geert Uytterhoeven,
Mark Brown, Rob Herring, Simon, Yokoyama
In-Reply-To: <873743nxuj.wl%kuninori.morimoto.gx@renesas.com>
CC DT
On Fri, Dec 22, 2017 at 6:29 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> commit a5702e1cb3c ("ASoC: rsnd: Drop unit-addresses without reg
> properties") modifies simple-card multi CPU nodes.
> But, naming of "cpu-x" breaks probing.
> Let's add reg = <x>; instead of renaming node.
>
> Reported-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
> CC: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> Documentation/devicetree/bindings/sound/renesas,rsnd.txt | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.txt b/Documentation/devicetree/bindings/sound/renesas,rsnd.txt
> index 085bec3..51b7324 100644
> --- a/Documentation/devicetree/bindings/sound/renesas,rsnd.txt
> +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.txt
> @@ -197,12 +197,17 @@ Ex)
> [MEM] -> [SRC2] -> [CTU03] -+
>
> sound {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> compatible = "simple-scu-audio-card";
> ...
> - simple-audio-card,cpu-0 {
> + simple-audio-card,cpu@0 {
> + reg = <0>;
> sound-dai = <&rcar_sound 0>;
> };
> - simple-audio-card,cpu-1 {
> + simple-audio-card,cpu@1 {
> + reg = <1>;
> sound-dai = <&rcar_sound 1>;
> };
> simple-audio-card,codec {
> --
> 1.9.1
^ permalink raw reply
* Re: [PATCH v3 2/3] hwrng: exynos - add Samsung Exynos True RNG driver
From: Herbert Xu @ 2017-12-22 8:24 UTC (permalink / raw)
To: Łukasz Stelmach
Cc: Andrew F . Davis, PrasannaKumar Muralidharan, Rob Herring,
Matt Mackall, Krzysztof Kozlowski, Kukjin Kim,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Marek Szyprowski,
Bartlomiej Zolnierkiewicz
In-Reply-To: <20171204125351.26805-3-l.stelmach-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On Mon, Dec 04, 2017 at 01:53:50PM +0100, Łukasz Stelmach wrote:
> Add support for True Random Number Generator found in Samsung Exynos
> 5250+ SoCs.
>
> Signed-off-by: Łukasz Stelmach <l.stelmach-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
This doesn't build for me:
CC [M] drivers/char/hw_random/exynos-trng.o
../drivers/char/hw_random/exynos-trng.c:230:1: error: \u2018exynos_rng_dt_match\u2019 undeclared here (not in a function)
../drivers/char/hw_random/exynos-trng.c:230:1: error: \u2018__mod_of__exynos_rng_dt_match_device_table\u2019 aliased to undefined symbol \u2018exynos_rng_dt_match\u2019
make[2]: *** [drivers/char/hw_random/exynos-trng.o] Error 1
make[1]: *** [_module_drivers/char/hw_random] Error 2
Cheers,
--
Email: Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] ARM: dts: r8a7745: Add missing clock for secondary CA7 CPU core
From: Simon Horman @ 2017-12-22 8:24 UTC (permalink / raw)
To: Chris Paterson, Geert Uytterhoeven
Cc: Biju Das, Rob Herring, Mark Rutland, Magnus Damm, Fabrizio Castro,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <TY1PR06MB070295E987FDDC0093043924B70D0-/PRLmSCtZ14u2TXDOttQZW0DtJ1/0DrXvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
On Thu, Dec 21, 2017 at 02:58:24PM +0000, Chris Paterson wrote:
>
> > From: Biju Das [mailto:biju.das-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org]
> > Sent: 21 December 2017 14:52
> >
> > Add the missing clock to CA7 CPU1 node.
> >
> > Signed-off-by: Biju Das <biju.das-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
>
> Reviewed-by: Chris Paterson <Chris.Paterson2-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>
> Kind regards, Chris
>
On Thu, Dec 21, 2017 at 04:00:23PM +0100, Geert Uytterhoeven wrote:
> On Thu, Dec 21, 2017 at 3:52 PM, Biju Das <biju.das-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org> wrote:
> > Add the missing clock to CA7 CPU1 node.
> >
> > Signed-off-by: Biju Das <biju.das-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
>
> Gr{oetje,eeting}s,
Thanks, applied.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/2] arm64: dts: renesas: Remove renesas,no-ether-link property
From: Simon Horman @ 2017-12-22 8:28 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Sergei Shtylyov, Bogdan Mirea,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <39b80de4-84bd-916e-2ff9-580f7db720f2-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
On Thu, Dec 21, 2017 at 04:57:58PM +0200, Vladimir Zapolskiy wrote:
> Hi Simon,
>
> On 12/21/2017 01:28 PM, Simon Horman wrote:
> > On Wed, Dec 20, 2017 at 03:22:10PM +0200, Vladimir Zapolskiy wrote:
> >> The present change is a bug fix for AVB link iteratively up/down.
> >
> > If this is a bug please consider including Fixes tags in the patches.
> >
>
> would you prefer to get v2 with the Fixes tags added or short replies
> to v1 with the requested information?
Either way would be fine. I see you have gone ahead and posted v2,
lets roll with that.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 2/3] hwrng: exynos - add Samsung Exynos True RNG driver
From: Marek Szyprowski @ 2017-12-22 8:29 UTC (permalink / raw)
To: Herbert Xu, Łukasz Stelmach
Cc: Andrew F . Davis, PrasannaKumar Muralidharan, Rob Herring,
Matt Mackall, Krzysztof Kozlowski, Kukjin Kim, devicetree,
linux-crypto, linux-samsung-soc, linux-kernel,
Bartlomiej Zolnierkiewicz
In-Reply-To: <20171222082412.GB29663@gondor.apana.org.au>
Hi,
On 2017-12-22 09:24, Herbert Xu wrote:
> On Mon, Dec 04, 2017 at 01:53:50PM +0100, Łukasz Stelmach wrote:
>> Add support for True Random Number Generator found in Samsung Exynos
>> 5250+ SoCs.
>>
>> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
> This doesn't build for me:
>
> CC [M] drivers/char/hw_random/exynos-trng.o
> ../drivers/char/hw_random/exynos-trng.c:230:1: error: \u2018exynos_rng_dt_match\u2019 undeclared here (not in a function)
> ../drivers/char/hw_random/exynos-trng.c:230:1: error: \u2018__mod_of__exynos_rng_dt_match_device_table\u2019 aliased to undefined symbol \u2018exynos_rng_dt_match\u2019
> make[2]: *** [drivers/char/hw_random/exynos-trng.o] Error 1
> make[1]: *** [_module_drivers/char/hw_random] Error 2
This looks like a missing dependency on "OF" when "COMPILE_TEST" is
selected.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH v2 1/2] arm64: dts: renesas: salvator-x: Remove renesas,no-ether-link property
From: Simon Horman @ 2017-12-22 8:32 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Sergei Shtylyov, Bogdan Mirea, linux-renesas-soc, devicetree,
linux-arm-kernel
In-Reply-To: <1513869539-18803-2-git-send-email-vladimir_zapolskiy@mentor.com>
On Thu, Dec 21, 2017 at 05:18:58PM +0200, Vladimir Zapolskiy wrote:
> From: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
>
> The present change is a bug fix for AVB link iteratively up/down.
>
> Steps to reproduce:
> - start AVB TX stream (Using aplay via MSE),
> - disconnect+reconnect the eth cable,
> - after a reconnection the eth connection goes iteratively up/down
> without user interaction,
> - this may heal after some seconds or even stay for minutes.
>
> As the documentation specifies, the "renesas,no-ether-link" option
> should be used when a board does not provide a proper AVB_LINK signal.
> There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
> and ULCB starter kits since the AVB_LINK is correctly handled by HW.
>
> Choosing to keep or remove the "renesas,no-ether-link" option will
> have impact on the code flow in the following ways:
> - keeping this option enabled may lead to unexpected behavior since
> the RX & TX are enabled/disabled directly from adjust_link function
> without any HW interrogation,
> - removing this option, the RX & TX will only be enabled/disabled after
> HW interrogation. The HW check is made through the LMON pin in PSR
> register which specifies AVB_LINK signal value (0 - at low level;
> 1 - at high level).
>
> In conclusion, the present change is also a safety improvement because
> it removes the "renesas,no-ether-link" option leading to a proper way
> of detecting the link state based on HW interrogation and not on
> software heuristic.
>
> Fixes: d25e8ff0d5aa ("arm64: dts: renesas: Extract common Salvator-X board support")
The above shuffles the code around but does not introduce the problem
as far as I can see. Instead I think we should use:
Fixes: dc36965a8905 ("arm64: dts: r8a7796: salvator-x: Enable EthernetAVB")
Fixes: 6fa501c549aa ("arm64: dts: r8a7795: enable EthernetAVB on Salvator-X")
> Signed-off-by: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> ---
> arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> index 4e800e933944..c3095bd575d7 100644
> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> @@ -255,7 +255,6 @@
> &avb {
> pinctrl-0 = <&avb_pins>;
> pinctrl-names = "default";
> - renesas,no-ether-link;
> phy-handle = <&phy0>;
> status = "okay";
>
> --
> 2.8.1
>
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: renesas: ulcb: Remove renesas,no-ether-link property
From: Simon Horman @ 2017-12-22 8:32 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Sergei Shtylyov, Bogdan Mirea, linux-renesas-soc, devicetree,
linux-arm-kernel
In-Reply-To: <1513869539-18803-3-git-send-email-vladimir_zapolskiy@mentor.com>
On Thu, Dec 21, 2017 at 05:18:59PM +0200, Vladimir Zapolskiy wrote:
> From: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
>
> The present change is a bug fix for AVB link iteratively up/down.
>
> Steps to reproduce:
> - start AVB TX stream (Using aplay via MSE),
> - disconnect+reconnect the eth cable,
> - after a reconnection the eth connection goes iteratively up/down
> without user interaction,
> - this may heal after some seconds or even stay for minutes.
>
> As the documentation specifies, the "renesas,no-ether-link" option
> should be used when a board does not provide a proper AVB_LINK signal.
> There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
> and ULCB starter kits since the AVB_LINK is correctly handled by HW.
>
> Choosing to keep or remove the "renesas,no-ether-link" option will
> have impact on the code flow in the following ways:
> - keeping this option enabled may lead to unexpected behavior since
> the RX & TX are enabled/disabled directly from adjust_link function
> without any HW interrogation,
> - removing this option, the RX & TX will only be enabled/disabled after
> HW interrogation. The HW check is made through the LMON pin in PSR
> register which specifies AVB_LINK signal value (0 - at low level;
> 1 - at high level).
>
> In conclusion, the present change is also a safety improvement because
> it removes the "renesas,no-ether-link" option leading to a proper way
> of detecting the link state based on HW interrogation and not on
> software heuristic.
>
> Fixes: 253ed045a34d ("arm64: dts: renesas: Extract common ULCB board support")
The above shuffles the code around but does not introduce the problem
as far as I can see. Instead I think we should use:
Fixes: 144bf6ccb13f ("arm64: dts: h3ulcb: enable EthernetAVB")
Fixes: 883fae315a6a ("arm64: dts: m3ulcb: enable EthernetAVB")
> Signed-off-by: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> ---
> arch/arm64/boot/dts/renesas/ulcb.dtsi | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/renesas/ulcb.dtsi b/arch/arm64/boot/dts/renesas/ulcb.dtsi
> index be91016e0b48..3e7a6b94e9f8 100644
> --- a/arch/arm64/boot/dts/renesas/ulcb.dtsi
> +++ b/arch/arm64/boot/dts/renesas/ulcb.dtsi
> @@ -145,7 +145,6 @@
> &avb {
> pinctrl-0 = <&avb_pins>;
> pinctrl-names = "default";
> - renesas,no-ether-link;
> phy-handle = <&phy0>;
> status = "okay";
>
> --
> 2.8.1
>
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: sunxi: Add sid for a83t
From: Maxime Ripard @ 2017-12-22 8:35 UTC (permalink / raw)
To: Emmanuel Vadot
Cc: Kyle Evans, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Russell King,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Chen-Yu Tsai, Rob Herring,
Srinivas Kandagatla,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20171221190903.56ebae536acf51401c63802c-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1940 bytes --]
On Thu, Dec 21, 2017 at 07:09:03PM +0100, Emmanuel Vadot wrote:
>
> Hi Maxime,
>
> On Thu, 21 Dec 2017 16:26:30 +0100
> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>
> > Hi,
> >
> > On Thu, Dec 21, 2017 at 09:19:24AM -0600, Kyle Evans wrote:
> > > On Thu, Dec 21, 2017 at 8:55 AM, Maxime Ripard
> > > <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > > > Hi Kyle,
> > > >
> > > > On Tue, Dec 19, 2017 at 03:05:23PM -0600, kevans91-OYTqUY/oFF8@public.gmane.org wrote:
> > > >> Allwinner a83t has a 1 KB sid block with efuse for security rootkey and
> > > >> thermal calibration data, add node to describe it.
> > > >>
> > > >> a83t-sid is not currently supported by nvmem/sunxi-sid, but it is
> > > >> supported in an external driver for FreeBSD.
> > > >>
> > > >> Signed-off-by: Kyle Evans <kevans91-OYTqUY/oFF8@public.gmane.org>
> > > >
> > > > The patch looks fine in itself, but we've had a number of issues with
> > > > the register layout (and access patterns) in the past, so I'd rather
> > > > have something that works in Linux too if possible.
> > >
> > > I have a patch that I think should make it work fine on Linux [1], but
> > > I'm afraid I have little to no capability to test it myself and so I
> > > did not add it as well.
> > >
> > > I do know that the rootkey is offset 0x200 into the given space [2],
> > > as is the case with the H3, and that the readout quirk is not needed.
> > > I wasn't 100% sure that the a83t has 2Kbit worth of efuse space as the
> > > H3, but I do know that thermal data can be found at 0x34 and 0x38 in
> > > this space.
> >
> > Then maybe we should leave it aside until someone takes some time on
> > the A83t.
>
> Take some time on the Linux driver and do not apply this patch for
> now you mean ?
Yep.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v3 2/3] hwrng: exynos - add Samsung Exynos True RNG driver
From: Herbert Xu @ 2017-12-22 8:35 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Łukasz Stelmach, Andrew F . Davis,
PrasannaKumar Muralidharan, Rob Herring, Matt Mackall,
Krzysztof Kozlowski, Kukjin Kim, devicetree, linux-crypto,
linux-samsung-soc, linux-kernel, Bartlomiej Zolnierkiewicz
In-Reply-To: <916163bb-594b-779d-700c-de9fa15239dd@samsung.com>
On Fri, Dec 22, 2017 at 09:29:38AM +0100, Marek Szyprowski wrote:
> Hi,
>
> On 2017-12-22 09:24, Herbert Xu wrote:
> >On Mon, Dec 04, 2017 at 01:53:50PM +0100, Łukasz Stelmach wrote:
> >>Add support for True Random Number Generator found in Samsung Exynos
> >>5250+ SoCs.
> >>
> >>Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
> >This doesn't build for me:
> >
> > CC [M] drivers/char/hw_random/exynos-trng.o
> >../drivers/char/hw_random/exynos-trng.c:230:1: error: \u2018exynos_rng_dt_match\u2019 undeclared here (not in a function)
> >../drivers/char/hw_random/exynos-trng.c:230:1: error: \u2018__mod_of__exynos_rng_dt_match_device_table\u2019 aliased to undefined symbol \u2018exynos_rng_dt_match\u2019
> >make[2]: *** [drivers/char/hw_random/exynos-trng.o] Error 1
> >make[1]: *** [_module_drivers/char/hw_random] Error 2
>
> This looks like a missing dependency on "OF" when "COMPILE_TEST" is
> selected.
Actually it looks like a typo. The variable is actually called
exynos_trng_dt_match as opposed to exynos_rng_dt_match.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH v5] arm64: dts: ls1088a: Add USB support
From: yinbo.zhu @ 2017-12-22 8:38 UTC (permalink / raw)
To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Catalin Marinas ), Will Deacon ), Shawn Guo
Cc: Harninder Rai, Zhang Ying-22455, Yuantian Tang,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, Hou Zhiqiang, Prabhakar Kushwaha, Alison Wang,
xiaobo.xie, Ashish Kumar, jerry.huang, Raghav Dogra, Nipun Gupta,
Yangbo Lu, Amrita Kumari, yinbo . zhu, ran.wang_1,
Bogdan Purcareata, linux-arm-kernel, Laurentiu Tudor
From: yinbo.zhu <yinbo.zhu@nxp.com>
Add USB support on ls1088ardb
Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
---
Change in v5:
The usb node was sorted alphabetically in label name.
Remove the point in "yinbo.zhu".
arch/arm64/boot/dts/freescale/fsl-ls1088a-rdb.dts | 8 ++++++++
arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 20 ++++++++++++++++++++
2 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1088a-rdb.dts
index 0f6fcda..4f17601 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1088a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a-rdb.dts
@@ -125,3 +125,11 @@
&sata {
status = "okay";
};
+
+&usb0 {
+ status = "okay";
+};
+
+&usb1 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
index bd80e9a..caf2bdd 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
@@ -394,6 +394,26 @@
status = "disabled";
};
+ usb0: usb3@3100000 {
+ compatible = "snps,dwc3";
+ reg = <0x0 0x3100000 0x0 0x10000>;
+ interrupts = <0 80 IRQ_TYPE_LEVEL_HIGH>;
+ dr_mode = "host";
+ snps,quirk-frame-length-adjustment = <0x20>;
+ snps,dis_rxdet_inp3_quirk;
+ status = "disabled";
+ };
+
+ usb1: usb3@3110000 {
+ compatible = "snps,dwc3";
+ reg = <0x0 0x3110000 0x0 0x10000>;
+ interrupts = <0 81 IRQ_TYPE_LEVEL_HIGH>;
+ dr_mode = "host";
+ snps,quirk-frame-length-adjustment = <0x20>;
+ snps,dis_rxdet_inp3_quirk;
+ status = "disabled";
+ };
+
sata: sata@3200000 {
compatible = "fsl,ls1088a-ahci";
reg = <0x0 0x3200000 0x0 0x10000>,
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] ASoC: max98373: Added Amplifier Driver
From: Philippe Ombredanne @ 2017-12-22 8:39 UTC (permalink / raw)
To: Ryan Lee
Cc: Liam Girdwood, Mark, Rob Herring, Mark Rutland, Jaroslav Kysela,
Takashi Iwai, Arnd Bergmann, afd-l0cyMroinI0,
robert.jarzmik-GANU6spQydw, supercraig0719-Re5JQEeQqe8AvxtiuMwx3w,
jbrunet-rdvid1DuHRBWk0Htik3J/w, dannenberg-l0cyMroinI0,
Romain Perier, bryce.ferguson-lFk7bPDcGtkY5TsXZYaR1UEOCMrvLtNR,
kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ,
m-stecklein-l0cyMroinI0, ALSA,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
ryan.lee.maxim-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1513907030-18441-1-git-send-email-ryans.lee-zxKO94PEStzToO697jQleEEOCMrvLtNR@public.gmane.org>
Ryan,
On Fri, Dec 22, 2017 at 2:43 AM, Ryan Lee <ryans.lee-zxKO94PEStzToO697jQleEEOCMrvLtNR@public.gmane.org> wrote:
> Signed-off-by: Ryan Lee <ryans.lee-zxKO94PEStzToO697jQleEEOCMrvLtNR@public.gmane.org>
> ---
>
> Created max98373 amplifier driver.
>
<snip>
> --- /dev/null
> +++ b/sound/soc/codecs/max98373.c
> @@ -0,0 +1,996 @@
> +/*
> + * max98373.c -- MAX98373 ALSA Soc Audio driver
> + *
> + * Copyright (C) 2017 Maxim Integrated Products
> + * Author: Ryan Lee <ryans.lee-zxKO94PEStzToO697jQleEEOCMrvLtNR@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
Have you considered using the new SPDX tags instead of this fine but
long legalese?
And if other contributors in your team could follow suit and you could
spread the word that would be even better!
See Thomas doc patches [1] for details.
Thanks!
[1] https://lkml.org/lkml/2017/12/4/934
--
Cordially
Philippe Ombredanne
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/2] arm64: dts: renesas: salvator-x: Remove renesas,no-ether-link property
From: Simon Horman @ 2017-12-22 8:40 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Sergei Shtylyov, Bogdan Mirea, linux-renesas-soc, devicetree,
linux-arm-kernel
In-Reply-To: <20171222083203.h5a65uitv7yok5fz@verge.net.au>
On Fri, Dec 22, 2017 at 09:32:03AM +0100, Simon Horman wrote:
> On Thu, Dec 21, 2017 at 05:18:58PM +0200, Vladimir Zapolskiy wrote:
> > From: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
> >
> > The present change is a bug fix for AVB link iteratively up/down.
> >
> > Steps to reproduce:
> > - start AVB TX stream (Using aplay via MSE),
> > - disconnect+reconnect the eth cable,
> > - after a reconnection the eth connection goes iteratively up/down
> > without user interaction,
> > - this may heal after some seconds or even stay for minutes.
> >
> > As the documentation specifies, the "renesas,no-ether-link" option
> > should be used when a board does not provide a proper AVB_LINK signal.
> > There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
> > and ULCB starter kits since the AVB_LINK is correctly handled by HW.
> >
> > Choosing to keep or remove the "renesas,no-ether-link" option will
> > have impact on the code flow in the following ways:
> > - keeping this option enabled may lead to unexpected behavior since
> > the RX & TX are enabled/disabled directly from adjust_link function
> > without any HW interrogation,
> > - removing this option, the RX & TX will only be enabled/disabled after
> > HW interrogation. The HW check is made through the LMON pin in PSR
> > register which specifies AVB_LINK signal value (0 - at low level;
> > 1 - at high level).
> >
> > In conclusion, the present change is also a safety improvement because
> > it removes the "renesas,no-ether-link" option leading to a proper way
> > of detecting the link state based on HW interrogation and not on
> > software heuristic.
> >
> > Fixes: d25e8ff0d5aa ("arm64: dts: renesas: Extract common Salvator-X board support")
>
> The above shuffles the code around but does not introduce the problem
> as far as I can see. Instead I think we should use:
>
> Fixes: dc36965a8905 ("arm64: dts: r8a7796: salvator-x: Enable EthernetAVB")
> Fixes: 6fa501c549aa ("arm64: dts: r8a7795: enable EthernetAVB on Salvator-X")
>
> > Signed-off-by: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
> > Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
I have applied this a fix for v4.15 with the updated Fixes tags above.
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: renesas: ulcb: Remove renesas,no-ether-link property
From: Simon Horman @ 2017-12-22 8:40 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Sergei Shtylyov, Bogdan Mirea,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20171222083256.2jgts6mdmssr7y76-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
On Fri, Dec 22, 2017 at 09:32:56AM +0100, Simon Horman wrote:
> On Thu, Dec 21, 2017 at 05:18:59PM +0200, Vladimir Zapolskiy wrote:
> > From: Bogdan Mirea <Bogdan-Stefan_Mirea-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> >
> > The present change is a bug fix for AVB link iteratively up/down.
> >
> > Steps to reproduce:
> > - start AVB TX stream (Using aplay via MSE),
> > - disconnect+reconnect the eth cable,
> > - after a reconnection the eth connection goes iteratively up/down
> > without user interaction,
> > - this may heal after some seconds or even stay for minutes.
> >
> > As the documentation specifies, the "renesas,no-ether-link" option
> > should be used when a board does not provide a proper AVB_LINK signal.
> > There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
> > and ULCB starter kits since the AVB_LINK is correctly handled by HW.
> >
> > Choosing to keep or remove the "renesas,no-ether-link" option will
> > have impact on the code flow in the following ways:
> > - keeping this option enabled may lead to unexpected behavior since
> > the RX & TX are enabled/disabled directly from adjust_link function
> > without any HW interrogation,
> > - removing this option, the RX & TX will only be enabled/disabled after
> > HW interrogation. The HW check is made through the LMON pin in PSR
> > register which specifies AVB_LINK signal value (0 - at low level;
> > 1 - at high level).
> >
> > In conclusion, the present change is also a safety improvement because
> > it removes the "renesas,no-ether-link" option leading to a proper way
> > of detecting the link state based on HW interrogation and not on
> > software heuristic.
> >
> > Fixes: 253ed045a34d ("arm64: dts: renesas: Extract common ULCB board support")
>
> The above shuffles the code around but does not introduce the problem
> as far as I can see. Instead I think we should use:
>
> Fixes: 144bf6ccb13f ("arm64: dts: h3ulcb: enable EthernetAVB")
> Fixes: 883fae315a6a ("arm64: dts: m3ulcb: enable EthernetAVB")
>
> > Signed-off-by: Bogdan Mirea <Bogdan-Stefan_Mirea-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
I have applied this a fix for v4.15 with the updated Fixes tags above.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] ARM: dts: tango4: remove bogus interrupt-controller property
From: Marc Gonzalez @ 2017-12-22 8:41 UTC (permalink / raw)
To: Arnd Bergmann, Mans Rullgard
Cc: Rob Herring, Mark Rutland, Olof Johansson, Thomas Gleixner,
Greg Kroah-Hartman, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171221214831.3859194-1-arnd-r2nGTMty4D4@public.gmane.org>
On 21/12/2017 22:48, Arnd Bergmann wrote:
> dtc points out that the parent node of the interrupt controllers is not
> actually an interrupt controller itself, and lacks an #interrupt-cells
> property:
>
> arch/arm/boot/dts/tango4-vantage-1172.dtb: Warning (interrupts_property): Missing #interrupt-cells in interrupt-parent /soc/interrupt-controller@6e000
>
> This removes the annotation.
>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> ---
> arch/arm/boot/dts/tango4-common.dtsi | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/tango4-common.dtsi b/arch/arm/boot/dts/tango4-common.dtsi
> index 0ec1b0a317b4..ff72a8efb73d 100644
> --- a/arch/arm/boot/dts/tango4-common.dtsi
> +++ b/arch/arm/boot/dts/tango4-common.dtsi
> @@ -156,7 +156,6 @@
> reg = <0x6e000 0x400>;
> ranges = <0 0x6e000 0x400>;
> interrupt-parent = <&gic>;
> - interrupt-controller;
> #address-cells = <1>;
> #size-cells = <1>;
>
Acked-by: Marc Gonzalez <marc_gonzalez-y1yR0Z3OICC7zZZRDBGcUA@public.gmane.org>
Thanks Arnd.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] [media] Add Rockchip RK1608 driver
From: Philippe Ombredanne @ 2017-12-22 8:45 UTC (permalink / raw)
To: Leo Wen
Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, David S. Miller,
Greg Kroah-Hartman, rdunlap, Linux Media Mailing List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
open list:ARM/Rockchip SoC..., Eddie Cai, Jacob Chen
In-Reply-To: <CAFLEztTxXRQu-VJ2FzYbA_vkYZtkDur1nQ6goftdZdnbn63aQQ@mail.gmail.com>
Dear Leo,
On Fri, Dec 22, 2017 at 5:33 AM, Jacob Chen <jacobchen110@gmail.com> wrote:
> Hi leo,
>
>
> 2017-12-12 14:28 GMT+08:00 Leo Wen <leo.wen@rock-chips.com>:
>> Rk1608 is used as a PreISP to link on Soc, which mainly has two functions.
>> One is to download the firmware of RK1608, and the other is to match the
>> extra sensor such as camera and enable sensor by calling sensor's s_power.
>>
>> use below v4l2-ctl command to capture frames.
>>
>> v4l2-ctl --verbose -d /dev/video1 --stream-mmap=2
>> --stream-to=/tmp/stream.out --stream-count=60 --stream-poll
>>
>> use below command to playback the video on your PC.
>>
>> mplayer ./stream.out -loop 0 -demuxer rawvideo -rawvideo
>> w=640:h=480:size=$((640*480*3/2)):format=NV12
>>
>> Signed-off-by: Leo Wen <leo.wen@rock-chips.com>
<snip>
>> --- /dev/null
>> +++ b/drivers/media/spi/rk1608.c
>> @@ -0,0 +1,1165 @@
>> +/**
>> + * Rockchip rk1608 driver
>> + *
>> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
>> + *
>> + * This software is available to you under a choice of one of two
>> + * licenses. You may choose to be licensed under the terms of the GNU
>> + * General Public License (GPL) Version 2, available from the file
>> + * COPYING in the main directory of this source tree, or the
>> + * OpenIB.org BSD license below:
>> + *
>> + * Redistribution and use in source and binary forms, with or
>> + * without modification, are permitted provided that the following
>> + * conditions are met:
>> + *
>> + * - Redistributions of source code must retain the above
>> + * copyright notice, this list of conditions and the following
>> + * disclaimer.
>> + *
>> + * - Redistributions in binary form must reproduce the above
>> + * copyright notice, this list of conditions and the following
>> + * disclaimer in the documentation and/or other materials
>> + * provided with the distribution.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
>> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>> + * SOFTWARE.
>> + */
Have you considered using the new SPDX tags instead of this fine but
long legalese?
I know what you are about to say: everyone loves legalese so much that
it is hard to let go of so much of it and replace all this only with a
single SPDX tag line ;)
But then everyone loves code much more than legalese too! so you would
be making the world a service anyway.
And if other contributors in your team could follow suit and you could
spread the word that would be even better!
See Thomas doc patches [1] for details.
[1] https://lkml.org/lkml/2017/12/4/934
--
Cordially
Philippe Ombredanne
^ permalink raw reply
* Re: [PATCH v5 3/3] PCI: mobiveil: Add MSI support for Mobiveil PCIe Host
From: Subrahmanya Lingappa @ 2017-12-22 8:52 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-pci, Bjorn Helgaas, Lorenzo Pieralisi, robh, devicetree,
Mingkai Hu, Peter W Newton, M.h. Lian, Raj Raina, Rajan Kapoor,
Prabhjot Singh
In-Reply-To: <23b886bf-b21e-8e8c-460c-42316469f989@arm.com>
[-- Attachment #1: Type: text/plain, Size: 15889 bytes --]
Marc,
On Fri, Dec 15, 2017 at 9:43 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 13/12/17 16:08, Subrahmanya Lingappa wrote:
> > Adds MSI support for Mobiveil PCIe Host Bridge IP driver
> >
> > Signed-off-by: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > Cc: linux-pci@vger.kernel.org
> > ---
> > drivers/pci/host/pcie-mobiveil.c | 222 ++++++++++++++++++++++++++++++
> ++++++++-
> > 1 file changed, 220 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/host/pcie-mobiveil.c b/drivers/pci/host/pcie-
> mobiveil.c
> > index 8611aaa..39818d5 100644
> > --- a/drivers/pci/host/pcie-mobiveil.c
> > +++ b/drivers/pci/host/pcie-mobiveil.c
> > @@ -14,6 +14,7 @@
> > #include <linux/irqdomain.h>
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > +#include <linux/msi.h>
> > #include <linux/of_address.h>
> > #include <linux/of_irq.h>
> > #include <linux/of_platform.h>
> > @@ -55,6 +56,7 @@
> > #define PAB_INTP_AMBA_MISC_ENB 0x0b0c
> > #define PAB_INTP_AMBA_MISC_STAT 0x0b1c
> > #define PAB_INTP_INTX_MASK 0x1e0
> > +#define PAB_INTP_MSI_MASK 0x8
> >
> > #define PAB_AXI_AMAP_CTRL(win) PAB_REG_ADDR_16(0x0ba0, win)
> > #define WIN_ENABLE_SHIFT 0
> > @@ -85,8 +87,19 @@
> >
> > /* supported number of interrupts */
> > #define PCI_NUM_INTX 4
> > +#define PCI_NUM_MSI 16
> > #define PAB_INTA_POS 5
> >
> > +/* MSI registers */
> > +#define MSI_BASE_LO_OFFSET 0x04
> > +#define MSI_BASE_HI_OFFSET 0x08
> > +#define MSI_SIZE_OFFSET 0x0c
> > +#define MSI_ENABLE_OFFSET 0x14
> > +#define MSI_STATUS_OFFSET 0x18
> > +#define MSI_DATA_OFFSET 0x20
> > +#define MSI_ADDR_L_OFFSET 0x24
> > +#define MSI_ADDR_H_OFFSET 0x28
> > +
> > /* outbound and inbound window definitions */
> > #define WIN_NUM_0 0
> > #define WIN_NUM_1 1
> > @@ -105,11 +118,22 @@
> > #define UINT64_MAX (u64)(~((u64)0))
> > #endif
> >
> > +struct mobiveil_msi { /* MSI information */
> > + struct mutex lock; /* protect bitmap variable */
> > + struct irq_domain *msi_domain;
> > + struct irq_domain *dev_domain;
> > + phys_addr_t msi_pages_phys;
> > + int *msi_pages;
> > + int num_of_vectors;
> > + DECLARE_BITMAP(msi_irq_in_use, PCI_NUM_MSI);
> > +};
> > +
> > struct mobiveil_pcie {
> > struct platform_device *pdev;
> > struct list_head resources;
> > void __iomem *config_axi_slave_base; /* endpoint config base */
> > void __iomem *csr_axi_slave_base; /* root port config base */
> > + void __iomem *apb_csr_base; /* MSI register base */
> > struct irq_domain *intx_domain;
> > int irq;
> > int apio_wins;
> > @@ -118,6 +142,7 @@ struct mobiveil_pcie {
> > int ib_wins_configured; /* configured inbound windows */
> > struct resource *ob_io_res;
> > char root_bus_nr;
> > + struct mobiveil_msi msi;
> > };
> >
> > static inline void csr_writel(struct mobiveil_pcie *pcie, const u32
> value,
> > @@ -202,11 +227,18 @@ static void mobiveil_pcie_isr(struct irq_desc
> *desc)
> > struct irq_chip *chip = irq_desc_get_chip(desc);
> > struct mobiveil_pcie *pcie = irq_desc_get_handler_data(desc);
> > struct device *dev = &pcie->pdev->dev;
> > - u32 intr_status;
> > + struct mobiveil_msi *msi = &pcie->msi;
> > + u32 msi_data, msi_addr_lo, msi_addr_hi;
> > + u32 intr_status, msi_status;
> > unsigned long shifted_status;
> > u32 bit, virq;
> > u32 val, mask;
> >
> > + /*
> > + * The core provides a single interrupt for both INTx/MSI messages.
> > + * So we'll read both INTx and MSI status
> > + */
> > +
> > chained_irq_enter(chip, desc);
> >
> > /* read INTx status */
> > @@ -241,6 +273,41 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
> > } while ((shifted_status >> PAB_INTA_POS) != 0);
> > }
> >
> > + /* read extra MSI status register */
> > + msi_status = readl(pcie->apb_csr_base + MSI_STATUS_OFFSET);
>
> You are using the non-relaxed accessors here, while the rest of the
> driver used the _relaxed version. Why is that so?
>
> i
t was a slip, I will correct it to use relaxed order.
> > +
> > + /* handle MSI interrupts */
> > + if ((intr_status & PAB_INTP_MSI_MASK) || (msi_status & 1)) {
> > + do {
> > + msi_data = readl(pcie->apb_csr_base +
> MSI_DATA_OFFSET);
> > +
> > + /*
> > + * MSI_STATUS_OFFSET gets updated to zero once we
> have
> > + * popped not only the data but also address from
> MSI
> > + * hardware FIFO.so keeping these following two
> dummy
> > + * reads.
> > + */
> > + msi_addr_lo = readl(pcie->apb_csr_base +
> > + MSI_ADDR_L_OFFSET);
> > + msi_addr_hi = readl(pcie->apb_csr_base +
> > + MSI_ADDR_H_OFFSET);
>
> Is that really valid? Don't you have to issue a 64bit access?
>
Seems like its valid, because its just a pair of DWORD
accesses to config register space to read MSI data captured inside bridge
hardware FIFO.
is there any reason you think its invalid?
>
> > + dev_dbg(dev,
> > + "MSI registers, data: %08x, addr:
> %08x:%08x\n",
> > + msi_data, msi_addr_hi, msi_addr_lo);
> > +
> > + if (msi_data) {
> > + virq = irq_find_mapping(msi->dev_domain,
> > + msi_data);
> > + if (virq)
> > + generic_handle_irq(virq);
> > + } else
> > + dev_err_ratelimited(dev, "MSI data
> null\n");
>
> Braces on both sides of the else statement.
>
> Also, why is msi_data==0 not valid? You can definitely allocate it from
> your bitmap, and I'm expecting that there is a strong correlation
> between what you read from MSI_DATA_OFFSET and the payload that the
>
> endpoint writes.
>
> Agreed msi_data==0 should also be valid. will be fixed.
> +
> > + msi_status = readl(pcie->apb_csr_base +
> > + MSI_STATUS_OFFSET);
> > + } while (msi_status & 1);
> > + }
> > +
> > csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);
> > chained_irq_exit(chip, desc);
> > }
> > @@ -274,6 +341,12 @@ static int mobiveil_pcie_parse_dt(struct
> mobiveil_pcie *pcie)
> > if (IS_ERR(pcie->csr_axi_slave_base))
> > return PTR_ERR(pcie->csr_axi_slave_base);
> >
> > + /* map MSI config resource */
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "apb_csr");
> > + pcie->apb_csr_base = devm_pci_remap_cfg_resource(dev, res);
> > + if (IS_ERR(pcie->apb_csr_base))
> > + return PTR_ERR(pcie->apb_csr_base);
> > +
> > /* read the number of windows requested */
> > if (!pcie->apio_wins &&
> > of_property_read_u32(node, "apio-wins", &pcie->apio_wins))
> {
> > @@ -430,6 +503,27 @@ static int mobiveil_bringup_link(struct
> mobiveil_pcie *pcie)
> > return -ETIMEDOUT;
> > }
> >
> > +static void mobiveil_pcie_enable_msi(struct mobiveil_pcie *pcie)
> > +{
> > + phys_addr_t msg_addr;
> > + struct mobiveil_msi *msi = &pcie->msi;
> > +
> > +
> > + pcie->msi.num_of_vectors = PCI_NUM_MSI;
> > +
> > + msi->msi_pages = (void *)__get_free_pages(GFP_DMA, 0);
>
> That old trick again... Do you really need to carve out a RAM page for
> this? Can't you use just some dummy physical address instead? The base
> address of your PCIe RC, for example.
>
Due to the current hardware bug
we do need a real RAM location to write this MSI data into, as we cant use
any dummy address for now.
>
> > + msg_addr = virt_to_phys((void *)msi->msi_pages);
> > + msi->msi_pages_phys = (phys_addr_t)msg_addr;
> > +
> > + writel_relaxed(lower_32_bits(msg_addr),
> > + pcie->apb_csr_base + MSI_BASE_LO_OFFSET);
> > + writel_relaxed(upper_32_bits(msg_addr),
> > + pcie->apb_csr_base + MSI_BASE_HI_OFFSET);
> > + writel_relaxed(4096, pcie->apb_csr_base + MSI_SIZE_OFFSET);
> > + writel_relaxed(1,
> > + pcie->apb_csr_base + MSI_ENABLE_OFFSET);
>
> nit: No need to break all these writes, each of them can fit on a single
> line.
>
> > +}
> > +
> > static int mobiveil_host_init(struct mobiveil_pcie *pcie)
> > {
> > u32 value;
> > @@ -465,7 +559,8 @@ static int mobiveil_host_init(struct mobiveil_pcie
> *pcie)
> > (1 << PEX_PIO_ENABLE_SHIFT),
> > PAB_CTRL);
> >
> > - csr_writel(pcie, PAB_INTP_INTX_MASK, PAB_INTP_AMBA_MISC_ENB);
> > + csr_writel(pcie, (PAB_INTP_INTX_MASK | PAB_INTP_MSI_MASK),
> > + PAB_INTP_AMBA_MISC_ENB);
> >
> > /* program PIO Enable Bit to 1 and Config Window Enable Bit to 1 in
> > * PAB_AXI_PIO_CTRL Register
> > @@ -503,6 +598,10 @@ static int mobiveil_host_init(struct mobiveil_pcie
> *pcie)
> > }
> > }
> >
> > + /* setup MSI hardware registers */
> > + if (IS_ENABLED(CONFIG_PCI_MSI))
>
> Your driver already depends on
>
> PCI_MSI_IRQ_DOMAIN, which depends on
> PCI_MSI. So this IS_ENABLED() is pointless.
>
as Lorenzo pointed out on the other thread, I will take
out PCI_MSI_IRQ_DOMAIN dependency.
So looks like better to follow other tested drivers and continue to keep it
no ?
>
>
> > + mobiveil_pcie_enable_msi(pcie);
> > +
> > return err;
> > }
> >
> > @@ -520,6 +619,118 @@ static int mobiveil_pcie_intx_map(struct
> irq_domain *domain, unsigned int irq,
> > .map = mobiveil_pcie_intx_map,
> > };
> >
> > +static struct irq_chip mobiveil_msi_irq_chip = {
> > + .name = "Mobiveil PCIe MSI",
> > + .irq_mask = pci_msi_mask_irq,
> > + .irq_unmask = pci_msi_unmask_irq,
> > +};
> > +
> > +static struct msi_domain_info mobiveil_msi_domain_info = {
> > + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> > + MSI_FLAG_PCI_MSIX),
> > + .chip = &mobiveil_msi_irq_chip,
> > +};
> > +
> > +static void mobiveil_compose_msi_msg(struct irq_data *data, struct
> msi_msg *msg)
> > +{
> > + struct mobiveil_pcie *pcie = irq_data_get_irq_chip_data(data);
> > + phys_addr_t addr = virt_to_phys((void *)pcie->msi.msi_pages +
> > + (data->hwirq * sizeof(int)));
> > +
> > + msg->address_lo = lower_32_bits(addr);
> > + msg->address_hi = upper_32_bits(addr);
> > + msg->data = data->hwirq;
> > +
> > + dev_dbg(&pcie->pdev->dev, "msi#%d address_hi %#x address_lo %#x\n",
> > + (int)data->hwirq, msg->address_hi, msg->address_lo);
> > +}
> > +
> > +static int mobiveil_msi_set_affinity(struct irq_data *irq_data,
> > + const struct cpumask *mask, bool force)
> > +{
> > + return -EINVAL;
> > +}
> > +
> > +static struct irq_chip mobiveil_msi_bottom_irq_chip = {
> > + .name = "Mobiveil MSI",
> > + .irq_compose_msi_msg = mobiveil_compose_msi_msg,
> > + .irq_set_affinity = mobiveil_msi_set_affinity,
> > +};
> > +
> > +static int mobiveil_irq_msi_domain_alloc(struct irq_domain *domain,
> > + unsigned int virq, unsigned int nr_irqs, void *args)
> > +{
> > + struct mobiveil_pcie *pcie = domain->host_data;
> > + struct mobiveil_msi *msi = &pcie->msi;
> > + unsigned long bit;
> > +
> > + WARN_ON(nr_irqs != 1);
> > + mutex_lock(&msi->lock);
> > +
> > + bit = find_first_zero_bit(msi->msi_irq_in_use,
> msi->num_of_vectors);
> > + if (bit >= msi->num_of_vectors) {
> > + mutex_unlock(&msi->lock);
> > + return -ENOSPC;
> > + }
> > +
> > + set_bit(bit, msi->msi_irq_in_use);
> > +
> > + mutex_unlock(&msi->lock);
> > +
> > + irq_domain_set_info(domain, virq, bit,
> &mobiveil_msi_bottom_irq_chip,
> > + domain->host_data, handle_simple_irq,
> > + NULL, NULL);
> > + return 0;
> > +}
> > +
> > +static void mobiveil_irq_msi_domain_free(struct irq_domain *domain,
> > + unsigned int virq, unsigned int nr_irqs)
> > +{
> > + struct irq_data *d = irq_domain_get_irq_data(domain, virq);
> > + struct mobiveil_pcie *pcie = irq_data_get_irq_chip_data(d);
> > + struct mobiveil_msi *msi = &pcie->msi;
> > +
> > + mutex_lock(&msi->lock);
> > +
> > + if (!test_bit(d->hwirq, msi->msi_irq_in_use)) {
> > + dev_err(&pcie->pdev->dev, "trying to free unused
> MSI#%lu\n",
> > + d->hwirq);
> > + } else {
> > + __clear_bit(d->hwirq, msi->msi_irq_in_use);
> > + }
> > +
> > + mutex_unlock(&msi->lock);
> > +}
> > +
> > +static const struct irq_domain_ops msi_domain_ops = {
> > + .alloc = mobiveil_irq_msi_domain_alloc,
> > + .free = mobiveil_irq_msi_domain_free,
> > +};
> > +
> > +static int mobiveil_allocate_msi_domains(struct mobiveil_pcie *pcie)
> > +{
> > + struct device *dev = &pcie->pdev->dev;
> > + struct fwnode_handle *fwnode = of_node_to_fwnode(dev->of_node);
> > + struct mobiveil_msi *msi = &pcie->msi;
> > +
> > + mutex_init(&pcie->msi.lock);
> > + msi->dev_domain = irq_domain_add_linear(NULL, msi->num_of_vectors,
> > + &msi_domain_ops, pcie);
> > + if (!msi->dev_domain) {
> > + dev_err(dev, "failed to create IRQ domain\n");
> > + return -ENOMEM;
> > + }
> > +
> > + msi->msi_domain = pci_msi_create_irq_domain(fwnode,
> > + &mobiveil_msi_domain_info,
> msi->dev_domain);
> > + if (!msi->msi_domain) {
> > + dev_err(dev, "failed to create MSI domain\n");
> > + irq_domain_remove(msi->dev_domain);
> > + return -ENOMEM;
> > + }
> > + return 0;
> > +}
> > +
> > static int mobiveil_pcie_init_irq_domain(struct mobiveil_pcie *pcie)
> > {
> > struct device *dev = &pcie->pdev->dev;
> > @@ -535,6 +746,13 @@ static int mobiveil_pcie_init_irq_domain(struct
> mobiveil_pcie *pcie)
> > return -ENODEV;
> > }
> >
> > +#ifdef CONFIG_PCI_MSI
>
> Useless #ifdef
>
> > + /* setup MSI */
> > + ret = mobiveil_allocate_msi_domains(pcie);
> > + if (ret)
> > + return ret;
> > +#endif
> > +
> > return 0;
> > }
> >
> >
>
> Now, there is something that I find a bit annoying: This code looks very
> similar to drivers/pci/host/pcie-altera-msi.c, to the extent that I
> suspect this is the same IP.
>
>
Its a different IP.
> I suggest you investigate whether you can reuse the existing code
> instead of adding yet another MSI driver to the kernel.
>
> Yes, Since the hardware architecture is similar so they do look similar.
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
>
Thanks,
Subrahmanya
[-- Attachment #2: Type: text/html, Size: 23907 bytes --]
^ permalink raw reply
* Re: [PATCH v5 3/3] PCI: mobiveil: Add MSI support for Mobiveil PCIe Host
From: Subrahmanya Lingappa @ 2017-12-22 8:57 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA, Bjorn Helgaas,
Lorenzo Pieralisi, robh-DgEjT+Ai2ygdnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mingkai Hu, Peter W Newton,
M.h. Lian, Raj Raina, Rajan Kapoor, Prabhjot Singh
In-Reply-To: <23b886bf-b21e-8e8c-460c-42316469f989-5wv7dgnIgG8@public.gmane.org>
Re-sending the mail in Plain text mode.
Marc,
On Fri, Dec 15, 2017 at 9:43 PM, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote:
>
> On 13/12/17 16:08, Subrahmanya Lingappa wrote:
> > Adds MSI support for Mobiveil PCIe Host Bridge IP driver
> >
> > Signed-off-by: Subrahmanya Lingappa <l.subrahmanya-DTHOJn6Rh8lhmhkoCovsdw@public.gmane.org>
> > Cc: Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> > Cc: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
> > Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > ---
> > drivers/pci/host/pcie-mobiveil.c | 222 ++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 220 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/host/pcie-mobiveil.c b/drivers/pci/host/pcie-mobiveil.c
> > index 8611aaa..39818d5 100644
> > --- a/drivers/pci/host/pcie-mobiveil.c
> > +++ b/drivers/pci/host/pcie-mobiveil.c
> > @@ -14,6 +14,7 @@
> > #include <linux/irqdomain.h>
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > +#include <linux/msi.h>
> > #include <linux/of_address.h>
> > #include <linux/of_irq.h>
> > #include <linux/of_platform.h>
> > @@ -55,6 +56,7 @@
> > #define PAB_INTP_AMBA_MISC_ENB 0x0b0c
> > #define PAB_INTP_AMBA_MISC_STAT 0x0b1c
> > #define PAB_INTP_INTX_MASK 0x1e0
> > +#define PAB_INTP_MSI_MASK 0x8
> >
> > #define PAB_AXI_AMAP_CTRL(win) PAB_REG_ADDR_16(0x0ba0, win)
> > #define WIN_ENABLE_SHIFT 0
> > @@ -85,8 +87,19 @@
> >
> > /* supported number of interrupts */
> > #define PCI_NUM_INTX 4
> > +#define PCI_NUM_MSI 16
> > #define PAB_INTA_POS 5
> >
> > +/* MSI registers */
> > +#define MSI_BASE_LO_OFFSET 0x04
> > +#define MSI_BASE_HI_OFFSET 0x08
> > +#define MSI_SIZE_OFFSET 0x0c
> > +#define MSI_ENABLE_OFFSET 0x14
> > +#define MSI_STATUS_OFFSET 0x18
> > +#define MSI_DATA_OFFSET 0x20
> > +#define MSI_ADDR_L_OFFSET 0x24
> > +#define MSI_ADDR_H_OFFSET 0x28
> > +
> > /* outbound and inbound window definitions */
> > #define WIN_NUM_0 0
> > #define WIN_NUM_1 1
> > @@ -105,11 +118,22 @@
> > #define UINT64_MAX (u64)(~((u64)0))
> > #endif
> >
> > +struct mobiveil_msi { /* MSI information */
> > + struct mutex lock; /* protect bitmap variable */
> > + struct irq_domain *msi_domain;
> > + struct irq_domain *dev_domain;
> > + phys_addr_t msi_pages_phys;
> > + int *msi_pages;
> > + int num_of_vectors;
> > + DECLARE_BITMAP(msi_irq_in_use, PCI_NUM_MSI);
> > +};
> > +
> > struct mobiveil_pcie {
> > struct platform_device *pdev;
> > struct list_head resources;
> > void __iomem *config_axi_slave_base; /* endpoint config base */
> > void __iomem *csr_axi_slave_base; /* root port config base */
> > + void __iomem *apb_csr_base; /* MSI register base */
> > struct irq_domain *intx_domain;
> > int irq;
> > int apio_wins;
> > @@ -118,6 +142,7 @@ struct mobiveil_pcie {
> > int ib_wins_configured; /* configured inbound windows */
> > struct resource *ob_io_res;
> > char root_bus_nr;
> > + struct mobiveil_msi msi;
> > };
> >
> > static inline void csr_writel(struct mobiveil_pcie *pcie, const u32 value,
> > @@ -202,11 +227,18 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
> > struct irq_chip *chip = irq_desc_get_chip(desc);
> > struct mobiveil_pcie *pcie = irq_desc_get_handler_data(desc);
> > struct device *dev = &pcie->pdev->dev;
> > - u32 intr_status;
> > + struct mobiveil_msi *msi = &pcie->msi;
> > + u32 msi_data, msi_addr_lo, msi_addr_hi;
> > + u32 intr_status, msi_status;
> > unsigned long shifted_status;
> > u32 bit, virq;
> > u32 val, mask;
> >
> > + /*
> > + * The core provides a single interrupt for both INTx/MSI messages.
> > + * So we'll read both INTx and MSI status
> > + */
> > +
> > chained_irq_enter(chip, desc);
> >
> > /* read INTx status */
> > @@ -241,6 +273,41 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
> > } while ((shifted_status >> PAB_INTA_POS) != 0);
> > }
> >
> > + /* read extra MSI status register */
> > + msi_status = readl(pcie->apb_csr_base + MSI_STATUS_OFFSET);
>
> You are using the non-relaxed accessors here, while the rest of the
> driver used the _relaxed version. Why is that so?
>
i
t was a slip, I will correct it to use relaxed order.
>
> > +
> > + /* handle MSI interrupts */
> > + if ((intr_status & PAB_INTP_MSI_MASK) || (msi_status & 1)) {
> > + do {
> > + msi_data = readl(pcie->apb_csr_base + MSI_DATA_OFFSET);
> > +
> > + /*
> > + * MSI_STATUS_OFFSET gets updated to zero once we have
> > + * popped not only the data but also address from MSI
> > + * hardware FIFO.so keeping these following two dummy
> > + * reads.
> > + */
> > + msi_addr_lo = readl(pcie->apb_csr_base +
> > + MSI_ADDR_L_OFFSET);
> > + msi_addr_hi = readl(pcie->apb_csr_base +
> > + MSI_ADDR_H_OFFSET);
>
> Is that really valid? Don't you have to issue a 64bit access?
>
Seems like its valid, because its just a pair of DWORD
accesses to config register space to read MSI data captured inside
bridge hardware FIFO.
is there any reason you think its invalid?
>
> > + dev_dbg(dev,
> > + "MSI registers, data: %08x, addr: %08x:%08x\n",
> > + msi_data, msi_addr_hi, msi_addr_lo);
> > +
> > + if (msi_data) {
> > + virq = irq_find_mapping(msi->dev_domain,
> > + msi_data);
> > + if (virq)
> > + generic_handle_irq(virq);
> > + } else
> > + dev_err_ratelimited(dev, "MSI data null\n");
>
> Braces on both sides of the else statement.
>
> Also, why is msi_data==0 not valid? You can definitely allocate it from
> your bitmap, and I'm expecting that there is a strong correlation
> between what you read from MSI_DATA_OFFSET and the payload that the
> endpoint writes.
>
Agreed msi_data==0 should also be valid. will be fixed.
>
> > +
> > + msi_status = readl(pcie->apb_csr_base +
> > + MSI_STATUS_OFFSET);
> > + } while (msi_status & 1);
> > + }
> > +
> > csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);
> > chained_irq_exit(chip, desc);
> > }
> > @@ -274,6 +341,12 @@ static int mobiveil_pcie_parse_dt(struct mobiveil_pcie *pcie)
> > if (IS_ERR(pcie->csr_axi_slave_base))
> > return PTR_ERR(pcie->csr_axi_slave_base);
> >
> > + /* map MSI config resource */
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "apb_csr");
> > + pcie->apb_csr_base = devm_pci_remap_cfg_resource(dev, res);
> > + if (IS_ERR(pcie->apb_csr_base))
> > + return PTR_ERR(pcie->apb_csr_base);
> > +
> > /* read the number of windows requested */
> > if (!pcie->apio_wins &&
> > of_property_read_u32(node, "apio-wins", &pcie->apio_wins)) {
> > @@ -430,6 +503,27 @@ static int mobiveil_bringup_link(struct mobiveil_pcie *pcie)
> > return -ETIMEDOUT;
> > }
> >
> > +static void mobiveil_pcie_enable_msi(struct mobiveil_pcie *pcie)
> > +{
> > + phys_addr_t msg_addr;
> > + struct mobiveil_msi *msi = &pcie->msi;
> > +
> > +
> > + pcie->msi.num_of_vectors = PCI_NUM_MSI;
> > +
> > + msi->msi_pages = (void *)__get_free_pages(GFP_DMA, 0);
>
> That old trick again... Do you really need to carve out a RAM page for
> this? Can't you use just some dummy physical address instead? The base
> address of your PCIe RC, for example.
>
Due to the current hardware bug
we do need a real RAM location to write this MSI data into, as we cant
use any dummy address for now.
>
> > + msg_addr = virt_to_phys((void *)msi->msi_pages);
> > + msi->msi_pages_phys = (phys_addr_t)msg_addr;
> > +
> > + writel_relaxed(lower_32_bits(msg_addr),
> > + pcie->apb_csr_base + MSI_BASE_LO_OFFSET);
> > + writel_relaxed(upper_32_bits(msg_addr),
> > + pcie->apb_csr_base + MSI_BASE_HI_OFFSET);
> > + writel_relaxed(4096, pcie->apb_csr_base + MSI_SIZE_OFFSET);
> > + writel_relaxed(1,
> > + pcie->apb_csr_base + MSI_ENABLE_OFFSET);
>
> nit: No need to break all these writes, each of them can fit on a single
> line.
>
> > +}
> > +
> > static int mobiveil_host_init(struct mobiveil_pcie *pcie)
> > {
> > u32 value;
> > @@ -465,7 +559,8 @@ static int mobiveil_host_init(struct mobiveil_pcie *pcie)
> > (1 << PEX_PIO_ENABLE_SHIFT),
> > PAB_CTRL);
> >
> > - csr_writel(pcie, PAB_INTP_INTX_MASK, PAB_INTP_AMBA_MISC_ENB);
> > + csr_writel(pcie, (PAB_INTP_INTX_MASK | PAB_INTP_MSI_MASK),
> > + PAB_INTP_AMBA_MISC_ENB);
> >
> > /* program PIO Enable Bit to 1 and Config Window Enable Bit to 1 in
> > * PAB_AXI_PIO_CTRL Register
> > @@ -503,6 +598,10 @@ static int mobiveil_host_init(struct mobiveil_pcie *pcie)
> > }
> > }
> >
> > + /* setup MSI hardware registers */
> > + if (IS_ENABLED(CONFIG_PCI_MSI))
>
> Your driver already depends on PCI_MSI_IRQ_DOMAIN, which depends on
> PCI_MSI. So this IS_ENABLED() is pointless.
>
as Lorenzo pointed out on the other thread, I will take out
PCI_MSI_IRQ_DOMAIN dependency.
So looks like better to follow other tested drivers and continue to keep it no ?
>
> > + mobiveil_pcie_enable_msi(pcie);
> > +
> > return err;
> > }
> >
> > @@ -520,6 +619,118 @@ static int mobiveil_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
> > .map = mobiveil_pcie_intx_map,
> > };
> >
> > +static struct irq_chip mobiveil_msi_irq_chip = {
> > + .name = "Mobiveil PCIe MSI",
> > + .irq_mask = pci_msi_mask_irq,
> > + .irq_unmask = pci_msi_unmask_irq,
> > +};
> > +
> > +static struct msi_domain_info mobiveil_msi_domain_info = {
> > + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> > + MSI_FLAG_PCI_MSIX),
> > + .chip = &mobiveil_msi_irq_chip,
> > +};
> > +
> > +static void mobiveil_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> > +{
> > + struct mobiveil_pcie *pcie = irq_data_get_irq_chip_data(data);
> > + phys_addr_t addr = virt_to_phys((void *)pcie->msi.msi_pages +
> > + (data->hwirq * sizeof(int)));
> > +
> > + msg->address_lo = lower_32_bits(addr);
> > + msg->address_hi = upper_32_bits(addr);
> > + msg->data = data->hwirq;
> > +
> > + dev_dbg(&pcie->pdev->dev, "msi#%d address_hi %#x address_lo %#x\n",
> > + (int)data->hwirq, msg->address_hi, msg->address_lo);
> > +}
> > +
> > +static int mobiveil_msi_set_affinity(struct irq_data *irq_data,
> > + const struct cpumask *mask, bool force)
> > +{
> > + return -EINVAL;
> > +}
> > +
> > +static struct irq_chip mobiveil_msi_bottom_irq_chip = {
> > + .name = "Mobiveil MSI",
> > + .irq_compose_msi_msg = mobiveil_compose_msi_msg,
> > + .irq_set_affinity = mobiveil_msi_set_affinity,
> > +};
> > +
> > +static int mobiveil_irq_msi_domain_alloc(struct irq_domain *domain,
> > + unsigned int virq, unsigned int nr_irqs, void *args)
> > +{
> > + struct mobiveil_pcie *pcie = domain->host_data;
> > + struct mobiveil_msi *msi = &pcie->msi;
> > + unsigned long bit;
> > +
> > + WARN_ON(nr_irqs != 1);
> > + mutex_lock(&msi->lock);
> > +
> > + bit = find_first_zero_bit(msi->msi_irq_in_use, msi->num_of_vectors);
> > + if (bit >= msi->num_of_vectors) {
> > + mutex_unlock(&msi->lock);
> > + return -ENOSPC;
> > + }
> > +
> > + set_bit(bit, msi->msi_irq_in_use);
> > +
> > + mutex_unlock(&msi->lock);
> > +
> > + irq_domain_set_info(domain, virq, bit, &mobiveil_msi_bottom_irq_chip,
> > + domain->host_data, handle_simple_irq,
> > + NULL, NULL);
> > + return 0;
> > +}
> > +
> > +static void mobiveil_irq_msi_domain_free(struct irq_domain *domain,
> > + unsigned int virq, unsigned int nr_irqs)
> > +{
> > + struct irq_data *d = irq_domain_get_irq_data(domain, virq);
> > + struct mobiveil_pcie *pcie = irq_data_get_irq_chip_data(d);
> > + struct mobiveil_msi *msi = &pcie->msi;
> > +
> > + mutex_lock(&msi->lock);
> > +
> > + if (!test_bit(d->hwirq, msi->msi_irq_in_use)) {
> > + dev_err(&pcie->pdev->dev, "trying to free unused MSI#%lu\n",
> > + d->hwirq);
> > + } else {
> > + __clear_bit(d->hwirq, msi->msi_irq_in_use);
> > + }
> > +
> > + mutex_unlock(&msi->lock);
> > +}
> > +
> > +static const struct irq_domain_ops msi_domain_ops = {
> > + .alloc = mobiveil_irq_msi_domain_alloc,
> > + .free = mobiveil_irq_msi_domain_free,
> > +};
> > +
> > +static int mobiveil_allocate_msi_domains(struct mobiveil_pcie *pcie)
> > +{
> > + struct device *dev = &pcie->pdev->dev;
> > + struct fwnode_handle *fwnode = of_node_to_fwnode(dev->of_node);
> > + struct mobiveil_msi *msi = &pcie->msi;
> > +
> > + mutex_init(&pcie->msi.lock);
> > + msi->dev_domain = irq_domain_add_linear(NULL, msi->num_of_vectors,
> > + &msi_domain_ops, pcie);
> > + if (!msi->dev_domain) {
> > + dev_err(dev, "failed to create IRQ domain\n");
> > + return -ENOMEM;
> > + }
> > +
> > + msi->msi_domain = pci_msi_create_irq_domain(fwnode,
> > + &mobiveil_msi_domain_info, msi->dev_domain);
> > + if (!msi->msi_domain) {
> > + dev_err(dev, "failed to create MSI domain\n");
> > + irq_domain_remove(msi->dev_domain);
> > + return -ENOMEM;
> > + }
> > + return 0;
> > +}
> > +
> > static int mobiveil_pcie_init_irq_domain(struct mobiveil_pcie *pcie)
> > {
> > struct device *dev = &pcie->pdev->dev;
> > @@ -535,6 +746,13 @@ static int mobiveil_pcie_init_irq_domain(struct mobiveil_pcie *pcie)
> > return -ENODEV;
> > }
> >
> > +#ifdef CONFIG_PCI_MSI
>
> Useless #ifdef
>
> > + /* setup MSI */
> > + ret = mobiveil_allocate_msi_domains(pcie);
> > + if (ret)
> > + return ret;
> > +#endif
> > +
> > return 0;
> > }
> >
> >
>
> Now, there is something that I find a bit annoying: This code looks very
> similar to drivers/pci/host/pcie-altera-msi.c, to the extent that I
> suspect this is the same IP.
>
Its a different IP.
>
> I suggest you investigate whether you can reuse the existing code
> instead of adding yet another MSI driver to the kernel.
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
Thanks,
Subrahmanya
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 06/11] thermal: armada: Add support for Armada AP806
From: Miquel RAYNAL @ 2017-12-22 9:17 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Mark Rutland, devicetree, Baruch Siach, Nadav Haklai, linux-pm,
Antoine Tenart, Eduardo Valentin, David Sniatkiwicz, Rob Herring,
Zhang Rui, Thomas Petazzoni, linux-arm-kernel
In-Reply-To: <87zi6dg0e1.fsf@free-electrons.com>
Hello Gregory,
On Wed, 20 Dec 2017 11:27:18 +0100
Gregory CLEMENT <gregory.clement@free-electrons.com> wrote:
> Hi Miquel,
>
> On mar., déc. 19 2017, Miquel Raynal
> <miquel.raynal@free-electrons.com> wrote:
>
> > From: Baruch Siach <baruch@tkos.co.il>
> >
> > The AP806 component is integrated in the Armada 8K and 7K lines of
> > processors.
> >
> > The thermal sensor sample field on the status register is a signed
> > value. Extend armada_get_temp() and the driver structure to handle
> > signed values.
> >
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > [<miquel.raynal@free-electrons.com>: Changes when applying over the
> > previous patches, including the register names changes, also
> > switched the coefficients values to s64 instead of unsigned long to
> > deal with negative values and used do_div instead of the
> > traditionnal '/'] Signed-off-by: Miquel Raynal
> > <miquel.raynal@free-electrons.com> Reviewed-by: Gregory CLEMENT
> > <gregory.clement@free-electrons.com> ---
> > drivers/thermal/armada_thermal.c | 74
> > ++++++++++++++++++++++++++++++++-------- 1 file changed, 59
> > insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/thermal/armada_thermal.c
> > b/drivers/thermal/armada_thermal.c index ceebabf45c53..c7dcac39cbf9
> > 100644 --- a/drivers/thermal/armada_thermal.c
> > +++ b/drivers/thermal/armada_thermal.c
> > @@ -47,6 +47,11 @@
> > #define CONTROL0_OFFSET 0x0
> > #define CONTROL1_OFFSET 0x4
> >
> > +/* TSEN refers to the temperature sensors within the AP */
> > +#define CONTROL0_TSEN_START BIT(0)
> > +#define CONTROL0_TSEN_RESET BIT(1)
> > +#define CONTROL0_TSEN_ENABLE BIT(2)
> > +
> > struct armada_thermal_data;
> >
> > /* Marvell EBU Thermal Sensor Dev Structure */
> > @@ -66,10 +71,11 @@ struct armada_thermal_data {
> > bool (*is_valid)(struct armada_thermal_priv *);
> >
> > /* Formula coeficients: temp = (b - m * reg) / div */
> > - unsigned long coef_b;
> > - unsigned long coef_m;
> > - unsigned long coef_div;
> > + s64 coef_b;
> > + s64 coef_m;
> > + u32 coef_div;
> > bool inverted;
> > + bool signed_sample;
> >
> > /* Register shift and mask to access the sensor
> > temperature */ unsigned int temp_shift;
> > @@ -155,6 +161,18 @@ static void armada380_init_sensor(struct
> > platform_device *pdev, }
> > }
> >
> > +static void armada_ap806_init_sensor(struct platform_device *pdev,
> > + struct armada_thermal_priv
> > *priv) +{
> > + u32 reg;
> > +
> > + reg = readl_relaxed(priv->control0);
> > + reg &= ~CONTROL0_TSEN_RESET;
> > + reg |= CONTROL0_TSEN_START | CONTROL0_TSEN_ENABLE;
> > + writel(reg, priv->control0);
> > + msleep(10);
> > +}
> > +
> > static bool armada_is_valid(struct armada_thermal_priv *priv)
> > {
> > u32 reg = readl_relaxed(priv->status);
> > @@ -166,8 +184,8 @@ static int armada_get_temp(struct
> > thermal_zone_device *thermal, int *temp)
> > {
> > struct armada_thermal_priv *priv = thermal->devdata;
> > - unsigned long reg;
> > - unsigned long m, b, div;
> > + u32 reg, div;
> > + s64 sample, b, m;
> >
> > /* Valid check */
> > if (priv->data->is_valid && !priv->data->is_valid(priv)) {
> > @@ -178,6 +196,11 @@ static int armada_get_temp(struct
> > thermal_zone_device *thermal,
> > reg = readl_relaxed(priv->status);
> > reg = (reg >> priv->data->temp_shift) &
> > priv->data->temp_mask;
> > + if (priv->data->signed_sample)
> > + /* The most significant bit is the sign bit */
> > + sample = sign_extend32(reg,
> > fls(priv->data->temp_mask) - 1);
> > + else
> > + sample = reg;
> >
> > /* Get formula coeficients */
> > b = priv->data->coef_b;
> > @@ -185,9 +208,12 @@ static int armada_get_temp(struct
> > thermal_zone_device *thermal, div = priv->data->coef_div;
> >
> > if (priv->data->inverted)
> > - *temp = ((m * reg) - b) / div;
> > + *temp = (m * sample) - b;
> > else
> > - *temp = (b - (m * reg)) / div;
> > + *temp = b - (m * sample);
> > +
> > + do_div(*temp, div);
>
> I wanted to test in on ARMv7 and this line failed to compile:
> In file included from arch/arm/include/asm/div64.h:127:0,
> from include/linux/kernel.h:173,
> from include/linux/list.h:9,
> from include/linux/kobject.h:20,
> from include/linux/device.h:17,
> from drivers/thermal/armada_thermal.c:16:
> drivers/thermal/armada_thermal.c: In function ‘armada_get_temp’:
> include/asm-generic/div64.h:208:28: warning: comparison of distinct
> pointer types lacks a cast (void)(((typeof((n)) *)0) == ((uint64_t
> *)0)); \ ^
> drivers/thermal/armada_thermal.c:247:2: note: in expansion of macro
> ‘do_div’ do_div(*temp, div);
> ^~~~~~
> In file included from include/linux/ioport.h:13:0,
> from include/linux/device.h:16,
> from drivers/thermal/armada_thermal.c:16:
> include/asm-generic/div64.h:221:25: warning: right shift count >=
> width of type [-Wshift-count-overflow] } else if (likely(((n) >> 32)
> == 0)) { \ ^
> include/linux/compiler.h:175:40: note: in definition of macro ‘likely’
> # define likely(x) __builtin_expect(!!(x), 1)
> ^
> drivers/thermal/armada_thermal.c:247:2: note: in expansion of macro
> ‘do_div’ do_div(*temp, div);
> ^~~~~~
> In file included from arch/arm/include/asm/div64.h:127:0,
> from include/linux/kernel.h:173,
> from include/linux/list.h:9,
> from include/linux/kobject.h:20,
> from include/linux/device.h:17,
> from drivers/thermal/armada_thermal.c:16:
> include/asm-generic/div64.h:225:22: error: passing argument 1 of
> ‘__div64_32’ from incompatible pointer type
> [-Werror=incompatible-pointer-types] __rem = __div64_32(&(n),
> __base); \ ^ drivers/thermal/armada_thermal.c:247:2: note: in
> expansion of macro ‘do_div’ do_div(*temp, div);
> ^~~~~~
> In file included from include/linux/kernel.h:173:0,
> from include/linux/list.h:9,
> from include/linux/kobject.h:20,
> from include/linux/device.h:17,
> from drivers/thermal/armada_thermal.c:16:
> arch/arm/include/asm/div64.h:33:24: note: expected ‘uint64_t * {aka
> long long unsigned int *}’ but argument is of type ‘int *’ static
> inline uint32_t __div64_32(uint64_t *n, uint32_t base)
>
Indeed, I also have this compilation error with a 32-bit toolchain but
not with the 64-bit one. Anyway I fixed it and tested on a 32-bit
platform also.
Thanks for reporting it.
Miquèl
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ARM: dts: exynos: fix RTC interrupt for exynos5410
From: Krzysztof Kozlowski @ 2017-12-22 9:19 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rob Herring, Mark Rutland, Kukjin Kim, Marek Szyprowski,
devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel,
Sylwester Nawrocki, Bartłomiej Żołnierkiewicz
In-Reply-To: <20171221213020.3080361-1-arnd@arndb.de>
On Thu, Dec 21, 2017 at 10:30 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> According to the comment added to exynos_dt_pmu_match[] in commit
> 8b283c025443 ("ARM: exynos4/5: convert pmu wakeup to stacked domains"),
> the RTC is not able to wake up the system through the PMU on Exynos5410,
> unlike Exynos5420.
>
> However, when the RTC DT node got added, it was a straight copy of
> the Exynos5420 node, which now causes a warning from dtc.
>
> This removes the incorrect interrupt-parent, which should get the
> interrupt working and avoid the warning.
>
> Fixes: e1e146b1b062 ("ARM: dts: exynos: Add RTC and I2C to Exynos5410")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/arm/boot/dts/exynos5410.dtsi | 1 -
> 1 file changed, 1 deletion(-)
In my pull request thread [1] you mentioned that PMU should not be
interrupt controller for Exynos5410. Yes, I think you are right.
Regardless whether Exynos5410 really supports Suspend to RAM or not,
now it is not configured as IRQ controller so the RTC's should go
straight to GIC. I think your fix is correct and I should drop my
patch.
I do not have Exynos5410 anymore (Odroid XU was left at Samsung
Poland) so I cannot test it. Maybe guys in Poland can test it? Marek?
Sylwester? Bartlomiej?
BTW,
It annoys me so much that it is so difficult to get Samsung boards. I
was writing to Samsung many times - to HQ, Open Source Group, LSI,
Artik and nothing. They do not have any boards... or they do not want
to share. This is how Samsung's support for Open Source really looks
like.
>From the three boards I have, two of them I bought myself by my own
money, one was donated to me by Markus Reichl (Odroid U3, thanks!).
When I was working in Samsung Poland they provided me with boards
(thanks guys!) but only for the time of work there and I am not
Samsung employee anymore.
So except this board from Markus, all infrastructure I have was built
by my own money. Eh, it is not that expensive, one board costs like
50-70 USD with shipment, but come on! Seriously, damn big company,
with damn big marketing budget and plenty of people and they cannot
provide their own boards to open-source... It really pisses me off
sometimes...
Back to the topic - thanks for the patch!
Best regards,
Krzysztof
[1] https://www.spinics.net/lists/arm-kernel/msg624966.html
>
> diff --git a/arch/arm/boot/dts/exynos5410.dtsi b/arch/arm/boot/dts/exynos5410.dtsi
> index 06713ec86f0d..d2174727df9a 100644
> --- a/arch/arm/boot/dts/exynos5410.dtsi
> +++ b/arch/arm/boot/dts/exynos5410.dtsi
> @@ -333,7 +333,6 @@
> &rtc {
> clocks = <&clock CLK_RTC>;
> clock-names = "rtc";
> - interrupt-parent = <&pmu_system_controller>;
> status = "disabled";
> };
>
> --
> 2.9.0
>
^ permalink raw reply
* Re: [PATCH] devicetree: Add video bus switch
From: Pavel Machek @ 2017-12-22 9:24 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sakari Ailus, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA,
ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w,
sre-DgEjT+Ai2ygdnm+yROfE0A, pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
linux-media-u79uwXL29TY76Z2rM5mHXA, galak-sgV2jX0FEOL9JmXXK+q4OQ,
mchehab-JPH+aEBZ4P+UEJcrhfAQsw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <75694885.3PuLWzx4qN@avalon>
[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]
Hi!
> > > I don't really object using g_endpoint_config() as a temporary solution;
> > > I'd like to have Laurent's opinion on that though. Another option is to
> > > wait, but we've already waited a looong time (as in total).
> >
> > Laurent, do you have some input here? We have simple "2 cameras
> > connected to one signal processor" situation here. We need some way of
> > passing endpoint configuration from the sensors through the switch. I
> > did this:
>
> Could you give me a bit more information about the platform you're targeting:
> how the switch is connected, what kind of switch it is, and what endpoint
> configuration data you need ?
Platform is Nokia N900, Ivaylo already gave pointer to schematics.
Switch is controlled using GPIO, and basically there's CSI
configuration that would normally be in the device tree, but now we
have two CSI configurations to select from...
> > 9) Highly reconfigurable hardware - Julien Beraud
> >
> > - 44 sub-devices connected with an interconnect.
> > - As long as formats match, any sub-device could be connected to any
> > - other sub-device through a link.
> > - The result is 44 * 44 links at worst.
> > - A switch sub-device proposed as the solution to model the
> > - interconnect. The sub-devices are connected to the switch
> > - sub-devices through the hardware links that connect to the
> > - interconnect.
> > - The switch would be controlled through new IOCTLs S_ROUTING and
> > - G_ROUTING.
> > - Patches available:
> > http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip
> >
> > but the patches are from 2005. So I guess I'll need some guidance here...
>
> You made me feel very old for a moment. The patches are from 2015 :-)
Sorry about that :-).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* [PATCH v6 00/11] Armada thermal: improvements and A7K/A8K SoCs support
From: Miquel Raynal @ 2017-12-22 9:32 UTC (permalink / raw)
To: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Gregory Clement, Antoine Tenart, Nadav Haklai,
Miquel Raynal, Baruch Siach, David Sniatkiwicz
Hi,
This series takes over Baruch's series by integrating his patches about
supporting thermal on Armada 7K and 8K SoCs within a larger series with
several improvements on the armada_thermal.c driver.
For now, Armada 380 and CP110 compatibles share the same initialization
routine but this will probably change in the near future when adding
support for overheat interrupts.
DT bindings documentation is updated to match existing code.
Armada AP806 and CP110 DT are also updated with thermal nodes and have
already been accepted (thus absent in this series).
Thank you,
Miquèl
Changes since v5:
- Added Grogory's Tested-by tags for 64-bit platforms in patches:
"thermal: armada: Add support for Armada AP806"
"thermal: armada: Add support for Armada CP110"
- Fixed a compilation issue with 32-bit toolchains introduced in patch:
"thermal: armada: Add support for Armada AP806"
Baruch Siach (4):
dt-bindings: thermal: Describe Armada AP806 and CP110
thermal: armada: Use msleep for long delays
thermal: armada: Add support for Armada AP806
thermal: armada: Add support for Armada CP110
Miquel Raynal (7):
thermal: armada: Simplify the check of the validity bit
thermal: armada: Clarify control registers accesses
thermal: armada: Use real status register name
thermal: armada: Update Kconfig and module description
thermal: armada: Change sensors trim default value
thermal: armada: Wait sensors validity before exiting the init
callback
thermal: armada: Give meaningful names to the thermal zones
.../devicetree/bindings/thermal/armada-thermal.txt | 37 ++-
drivers/thermal/Kconfig | 4 +-
drivers/thermal/armada_thermal.c | 257 +++++++++++++++------
3 files changed, 218 insertions(+), 80 deletions(-)
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v6 01/11] dt-bindings: thermal: Describe Armada AP806 and CP110
From: Miquel Raynal @ 2017-12-22 9:32 UTC (permalink / raw)
To: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Gregory Clement, Antoine Tenart, Nadav Haklai,
Miquel Raynal, Baruch Siach, David Sniatkiwicz
In-Reply-To: <20171222093226.23456-1-miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
From: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
Add compatible strings for AP806 and CP110 that are part of the Armada
8k/7k line of SoCs.
Add a note on the differences in the size of the control area in
different bindings. This is an existing difference between the Armada
375 binding and the other boards already supported. The new AP806 and
CP110 bindings are similar to the existing Armada 375 in this regard.
Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
[<miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>: reword, additional details]
Signed-off-by: Miquel Raynal <miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
.../devicetree/bindings/thermal/armada-thermal.txt | 37 +++++++++++++++-------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/thermal/armada-thermal.txt b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
index 24aacf8948c5..e0d013a2e66d 100644
--- a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
@@ -2,22 +2,35 @@
Required properties:
-- compatible: Should be set to one of the following:
- marvell,armada370-thermal
- marvell,armada375-thermal
- marvell,armada380-thermal
- marvell,armadaxp-thermal
+- compatible: Should be set to one of the following:
+ * marvell,armada370-thermal
+ * marvell,armada375-thermal
+ * marvell,armada380-thermal
+ * marvell,armadaxp-thermal
+ * marvell,armada-ap806-thermal
+ * marvell,armada-cp110-thermal
-- reg: Device's register space.
- Two entries are expected, see the examples below.
- The first one is required for the sensor register;
- the second one is required for the control register
- to be used for sensor initialization (a.k.a. calibration).
+- reg: Device's register space.
+ Two entries are expected, see the examples below. The first one points
+ to the status register (4B). The second one points to the control
+ registers (8B).
+ Note: The compatibles marvell,armada370-thermal,
+ marvell,armada380-thermal, and marvell,armadaxp-thermal must point to
+ "control MSB/control 1", with size of 4 (deprecated binding), or point
+ to "control LSB/control 0" with size of 8 (current binding). All other
+ compatibles must point to "control LSB/control 0" with size of 8.
-Example:
+Examples:
+ /* Legacy bindings */
thermal@d0018300 {
compatible = "marvell,armada370-thermal";
- reg = <0xd0018300 0x4
+ reg = <0xd0018300 0x4
0xd0018304 0x4>;
};
+
+ ap_thermal: thermal@6f8084 {
+ compatible = "marvell,armada-ap806-thermal";
+ reg = <0x6f808C 0x4>,
+ <0x6f8084 0x8>;
+ };
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox