* Re: Subject: [PATCH v10 1/2] PCI: microchip: Add host driver for Microchip PCIe controller
From: Daire.McNamara @ 2020-05-26 14:51 UTC (permalink / raw)
To: amurray, lorenzo.pieralisi, linux-pci, bhelgaas, robh+dt,
devicetree
Cc: david.abdurachmanov
In-Reply-To: <930a4f34fd11be89bc66cfa35b249c9b685aa8c2.camel@microchip.com>
^ permalink raw reply
* Re: Subject: [PATCH v10 2/2] PCI: microchip: Add host driver for Microchip PCIe controller
From: Daire.McNamara @ 2020-05-26 14:52 UTC (permalink / raw)
To: amurray, lorenzo.pieralisi, linux-pci, bhelgaas, robh+dt,
devicetree
Cc: david.abdurachmanov
In-Reply-To: <c0b1e55f7fae0998458d6df7e4f601f5e797750d.camel@microchip.com>
add support for the Microchip PCIe PolarFire PCIe controller when
configured in host (Root Complex) mode.
Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
---
drivers/pci/controller/Kconfig | 9 +
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-microchip-host.c | 664 +++++++++++++++++++
3 files changed, 674 insertions(+)
create mode 100644 drivers/pci/controller/pcie-microchip-host.c
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 91bfdb784829..5ec760466bd9 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -258,6 +258,15 @@ config PCI_HYPERV_INTERFACE
The Hyper-V PCI Interface is a helper driver allows other drivers to
have a common interface with the Hyper-V PCI frontend driver.
+config PCIE_MICROCHIP_HOST
+ bool "Microchip AXI PCIe host bridge support"
+ depends on PCI_MSI && OF
+ select PCI_MSI_IRQ_DOMAIN
+ select GENERIC_MSI_IRQ_DOMAIN
+ help
+ Say Y here if you want kernel to support the Microchip AXI PCIe
+ Host Bridge driver.
+
source "drivers/pci/controller/dwc/Kconfig"
source "drivers/pci/controller/mobiveil/Kconfig"
source "drivers/pci/controller/cadence/Kconfig"
diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
index 158c59771824..416528d8553e 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_PCIE_ROCKCHIP_EP) += pcie-rockchip-ep.o
obj-$(CONFIG_PCIE_ROCKCHIP_HOST) += pcie-rockchip-host.o
obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o
obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o
+obj-$(CONFIG_PCIE_MICROCHIP_HOST) += pcie-microchip-host.o
obj-$(CONFIG_VMD) += vmd.o
obj-$(CONFIG_PCIE_BRCMSTB) += pcie-brcmstb.o
# pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
new file mode 100644
index 000000000000..bdea416aff79
--- /dev/null
+++ b/drivers/pci/controller/pcie-microchip-host.c
@@ -0,0 +1,664 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Microchip AXI PCIe Bridge host controller driver
+ *
+ * Copyright (c) 2018 - 2019 Microchip Corporation. All rights reserved.
+ *
+ * Author: Daire McNamara <daire.mcnamara@microchip.com>
+ *
+ * Based on:
+ * pcie-rcar.c
+ * pcie-xilinx.c
+ * pcie-altera.c
+ */
+#include <linux/irqchip/chained_irq.h>
+#include <linux/module.h>
+#include <linux/msi.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_pci.h>
+#include <linux/pci-ecam.h>
+#include <linux/platform_device.h>
+
+#include "../pci.h"
+
+/* Number of MSI IRQs */
+#define MC_NUM_MSI_IRQS 32
+#define MC_NUM_MSI_IRQS_CODED 5
+
+/* PCIe Bridge Phy and Controller Phy offsets */
+#define MC_PCIE1_BRIDGE_ADDR 0x00008000u
+#define MC_PCIE1_CTRL_ADDR 0x0000a000u
+
+/* PCIe Controller Phy Regs */
+#define MC_SEC_ERROR_INT 0x28
+#define SEC_ERROR_INT_TX_RAM_SEC_ERR_INT GENMASK(3, 0)
+#define SEC_ERROR_INT_RX_RAM_SEC_ERR_INT GENMASK(7, 4)
+#define SEC_ERROR_INT_PCIE2AXI_RAM_SEC_ERR_INT GENMASK(11, 8)
+#define SEC_ERROR_INT_AXI2PCIE_RAM_SEC_ERR_INT GENMASK(15, 12)
+#define MC_SEC_ERROR_INT_MASK 0x2c
+#define MC_DED_ERROR_INT 0x30
+#define DED_ERROR_INT_TX_RAM_DED_ERR_INT GENMASK(3, 0)
+#define DED_ERROR_INT_RX_RAM_DED_ERR_INT GENMASK(7, 4)
+#define DED_ERROR_INT_PCIE2AXI_RAM_DED_ERR_INT GENMASK(11, 8)
+#define DED_ERROR_INT_AXI2PCIE_RAM_DED_ERR_INT GENMASK(15, 12)
+#define MC_DED_ERROR_INT_MASK 0x34
+#define MC_ECC_CONTROL 0x38
+#define ECC_CONTROL_AXI2PCIE_RAM_ECC_BYPASS BIT(27)
+#define ECC_CONTROL_PCIE2AXI_RAM_ECC_BYPASS BIT(26)
+#define ECC_CONTROL_RX_RAM_ECC_BYPASS BIT(25)
+#define ECC_CONTROL_TX_RAM_ECC_BYPASS BIT(24)
+#define MC_LTSSM_STATE 0x5c
+#define LTSSM_L0_STATE 0x10
+#define MC_PCIE_EVENT_INT 0x14c
+#define PCIE_EVENT_INT_L2_EXIT_INT BIT(0)
+#define PCIE_EVENT_INT_HOTRST_EXIT_INT BIT(1)
+#define PCIE_EVENT_INT_DLUP_EXIT_INT BIT(2)
+#define PCIE_EVENT_INT_L2_EXIT_INT_MASK BIT(16)
+#define PCIE_EVENT_INT_HOTRST_EXIT_INT_MASK BIT(17)
+#define PCIE_EVENT_INT_DLUP_EXIT_INT_MASK BIT(18)
+
+/* PCIe Bridge Phy Regs */
+#define MC_PCIE_PCI_IDS_DW1 0x9c
+
+/* PCIe Config space MSI capability structure */
+#define MC_MSI_CAP_CTRL 0xe0u
+#define MSI_ENABLE (0x01u << 16)
+#define MSI_CAP_MULTI (MC_NUM_MSI_IRQS_CODED << 17)
+#define MSI_ENABLE_MULTI (MC_NUM_MSI_IRQS_CODED << 20)
+#define MC_MSI_MSG_LO_ADDR 0xe4u
+#define MC_MSI_MSG_HI_ADDR 0xe8u
+#define MC_MSI_MSG_DATA 0xf0u
+
+#define MC_IMASK_LOCAL 0x180
+#define PCIE_LOCAL_INT_ENABLE 0x0f000000u
+#define PCI_INTS 0x0f000000u
+#define PM_MSI_INT_SHIFT 24
+#define PCIE_ENABLE_MSI 0x10000000u
+#define MSI_INT 0x10000000u
+#define MSI_INT_SHIFT 28
+#define MC_ISTATUS_LOCAL 0x184
+#define MC_IMASK_HOST 0x188
+#define MC_ISTATUS_HOST 0x18c
+#define MC_MSI_ADDR 0x190
+#define MC_ISTATUS_MSI 0x194
+
+/* PCIe Master table init defines */
+#define MC_ATR0_PCIE_WIN0_SRCADDR_PARAM 0x600u
+#define ATR0_PCIE_ATR_SIZE 0x1f
+#define ATR0_PCIE_ATR_SIZE_SHIFT 1
+#define MC_ATR0_PCIE_WIN0_SRC_ADDR 0x604u
+#define MC_ATR0_PCIE_WIN0_TRSL_ADDR_LSB 0x608u
+#define MC_ATR0_PCIE_WIN0_TRSL_ADDR_UDW 0x60cu
+#define MC_ATR0_PCIE_WIN0_TRSL_PARAM 0x610u
+
+/* PCIe AXI slave table init defines */
+#define MC_ATR0_AXI4_SLV0_SRCADDR_PARAM 0x800u
+#define ATR_SIZE_SHIFT 1
+#define ATR_IMPL_ENABLE 1
+#define MC_ATR0_AXI4_SLV0_SRC_ADDR 0x804u
+#define MC_ATR0_AXI4_SLV0_TRSL_ADDR_LSB 0x808u
+#define MC_ATR0_AXI4_SLV0_TRSL_ADDR_UDW 0x80cu
+#define MC_ATR0_AXI4_SLV0_TRSL_PARAM 0x810u
+#define PCIE_TX_RX_INTERFACE 0x00000000u
+#define PCIE_CONFIG_INTERFACE 0x00000001u
+
+#define ATT_ENTRY_SIZE 32
+
+struct mc_msi {
+ struct mutex lock; /* Protect used bitmap */
+ struct irq_domain *msi_domain;
+ struct irq_domain *dev_domain;
+ u32 num_vectors;
+ u64 vector_phy;
+ DECLARE_BITMAP(used, MC_NUM_MSI_IRQS);
+};
+
+struct mc_pcie {
+ struct platform_device *pdev;
+ void __iomem *cfg_base_addr;
+ void __iomem *bridge_base_addr;
+ void __iomem *ctrl_base_addr;
+ u64 cfghw_base_addr;
+ u64 cfg_size;
+ u32 atr_sz;
+ u32 irq;
+ struct irq_domain *intx_domain;
+ raw_spinlock_t intx_mask_lock;
+ struct mc_msi msi;
+};
+
+static inline u32 cfg_readl(struct mc_pcie *pcie, const u32 reg)
+{
+ return readl_relaxed(pcie->cfg_base_addr + reg);
+}
+
+static inline void cfg_writel(struct mc_pcie *pcie, const u32 val,
+ const u32 reg)
+{
+ writel_relaxed(val, pcie->cfg_base_addr + reg);
+}
+
+static void mc_pcie_enable(struct mc_pcie *pcie)
+{
+ u32 enb;
+
+ enb = readl(pcie->bridge_base_addr + MC_LTSSM_STATE);
+ enb |= LTSSM_L0_STATE;
+ writel(enb, pcie->bridge_base_addr + MC_LTSSM_STATE);
+}
+
+static void mc_pcie_isr(struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct mc_pcie *pcie = irq_desc_get_handler_data(desc);
+ struct device *dev = &pcie->pdev->dev;
+ struct mc_msi *msi = &pcie->msi;
+ unsigned long status;
+ unsigned long intx_status;
+ unsigned long msi_status;
+ u32 bit;
+ u32 virq;
+
+ /*
+ * 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);
+
+ status = readl(pcie->bridge_base_addr + MC_ISTATUS_LOCAL);
+ while (status & (PCI_INTS | MSI_INT)) {
+ intx_status = (status & PCI_INTS) >> PM_MSI_INT_SHIFT;
+ for_each_set_bit(bit, &intx_status, PCI_NUM_INTX) {
+ virq = irq_find_mapping(pcie->intx_domain, bit + 1);
+ if (virq)
+ generic_handle_irq(virq);
+ else
+ dev_err_ratelimited(dev, "bad INTx IRQ %d\n",
+ bit);
+
+ /* Clear that interrupt bit */
+ writel(1 << (bit + PM_MSI_INT_SHIFT),
+ pcie->bridge_base_addr + MC_ISTATUS_LOCAL);
+ }
+
+ msi_status = (status & MSI_INT);
+ if (msi_status) {
+ msi_status = readl(pcie->bridge_base_addr +
+ MC_ISTATUS_MSI);
+ for_each_set_bit(bit, &msi_status, msi->num_vectors) {
+ virq = irq_find_mapping(msi->dev_domain, bit);
+ if (virq)
+ generic_handle_irq(virq);
+ else
+ dev_err_ratelimited(dev,
+ "bad MSI IRQ %d\n",
+ bit);
+
+ /* Clear that MSI interrupt bit */
+ writel((1 << bit), pcie->bridge_base_addr +
+ MC_ISTATUS_MSI);
+ }
+ /* Clear the ISTATUS MSI bit */
+ writel(1 << MSI_INT_SHIFT, pcie->bridge_base_addr +
+ MC_ISTATUS_LOCAL);
+ }
+
+ status = readl(pcie->bridge_base_addr + MC_ISTATUS_LOCAL);
+ }
+
+ chained_irq_exit(chip, desc);
+}
+
+static int mc_pcie_parse_dt(struct mc_pcie *pcie)
+{
+ struct platform_device *pdev = pcie->pdev;
+ struct device *dev = &pcie->pdev->dev;
+ struct mc_msi *msi = &pcie->msi;
+ struct resource *res;
+ void __iomem *axi_base_addr;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ axi_base_addr = devm_ioremap_resource(dev, res);
+ if (IS_ERR(axi_base_addr))
+ return PTR_ERR(axi_base_addr);
+
+ pcie->bridge_base_addr = axi_base_addr + MC_PCIE1_BRIDGE_ADDR;
+ pcie->ctrl_base_addr = axi_base_addr + MC_PCIE1_CTRL_ADDR;
+
+ msi->vector_phy = MC_MSI_ADDR;
+
+ msi->num_vectors = MC_NUM_MSI_IRQS;
+
+ pcie->irq = platform_get_irq(pdev, 0);
+ if (pcie->irq < 0) {
+ dev_err(dev, "unable to request IRQ%d\n", pcie->irq);
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+static void mc_pcie_enable_msi(struct mc_pcie *pcie)
+{
+ struct mc_msi *msi = &pcie->msi;
+ u32 msg_ctrl = cfg_readl(pcie, MC_MSI_CAP_CTRL);
+
+ msg_ctrl |= MSI_ENABLE_MULTI | MSI_CAP_MULTI | MSI_ENABLE;
+ cfg_writel(pcie, msg_ctrl, MC_MSI_CAP_CTRL);
+
+ cfg_writel(pcie, lower_32_bits(msi->vector_phy), MC_MSI_MSG_LO_ADDR);
+ cfg_writel(pcie, upper_32_bits(msi->vector_phy), MC_MSI_MSG_HI_ADDR);
+}
+
+static int mc_host_init(struct mc_pcie *pcie)
+{
+ u32 val;
+
+ mc_pcie_enable(pcie);
+
+ val = ECC_CONTROL_AXI2PCIE_RAM_ECC_BYPASS |
+ ECC_CONTROL_PCIE2AXI_RAM_ECC_BYPASS |
+ ECC_CONTROL_RX_RAM_ECC_BYPASS | ECC_CONTROL_TX_RAM_ECC_BYPASS;
+ writel(val, pcie->ctrl_base_addr + MC_ECC_CONTROL);
+
+ val = PCIE_EVENT_INT_L2_EXIT_INT | PCIE_EVENT_INT_HOTRST_EXIT_INT |
+ PCIE_EVENT_INT_DLUP_EXIT_INT | PCIE_EVENT_INT_L2_EXIT_INT_MASK |
+ PCIE_EVENT_INT_HOTRST_EXIT_INT_MASK |
+ PCIE_EVENT_INT_DLUP_EXIT_INT_MASK;
+ writel(val, pcie->ctrl_base_addr + MC_PCIE_EVENT_INT);
+
+ val = SEC_ERROR_INT_TX_RAM_SEC_ERR_INT |
+ SEC_ERROR_INT_RX_RAM_SEC_ERR_INT |
+ SEC_ERROR_INT_PCIE2AXI_RAM_SEC_ERR_INT |
+ SEC_ERROR_INT_AXI2PCIE_RAM_SEC_ERR_INT;
+ writel(val, pcie->ctrl_base_addr + MC_SEC_ERROR_INT);
+ writel(val, pcie->ctrl_base_addr + MC_SEC_ERROR_INT_MASK);
+
+ val = DED_ERROR_INT_TX_RAM_DED_ERR_INT |
+ DED_ERROR_INT_RX_RAM_DED_ERR_INT |
+ DED_ERROR_INT_PCIE2AXI_RAM_DED_ERR_INT |
+ DED_ERROR_INT_AXI2PCIE_RAM_DED_ERR_INT;
+ writel(val, pcie->ctrl_base_addr + MC_DED_ERROR_INT);
+ writel(val, pcie->ctrl_base_addr + MC_DED_ERROR_INT_MASK);
+
+ writel(0, pcie->bridge_base_addr + MC_IMASK_LOCAL);
+ writel(GENMASK(31, 0), pcie->bridge_base_addr + MC_ISTATUS_LOCAL);
+ writel(0, pcie->bridge_base_addr + MC_IMASK_HOST);
+ writel(GENMASK(31, 0), pcie->bridge_base_addr + MC_ISTATUS_HOST);
+
+ /* Configure Address Translation Table 0 for PCIe config space */
+ writel(PCIE_CONFIG_INTERFACE, pcie->bridge_base_addr +
+ MC_ATR0_AXI4_SLV0_TRSL_PARAM);
+
+ val = lower_32_bits(pcie->cfghw_base_addr) |
+ (pcie->atr_sz << ATR_SIZE_SHIFT) | ATR_IMPL_ENABLE;
+ writel(val, pcie->bridge_base_addr + MC_ATR0_AXI4_SLV0_SRCADDR_PARAM);
+ writel(0, pcie->bridge_base_addr + MC_ATR0_AXI4_SLV0_SRC_ADDR);
+
+ val = lower_32_bits(pcie->cfghw_base_addr);
+ writel(val, pcie->bridge_base_addr + MC_ATR0_AXI4_SLV0_TRSL_ADDR_LSB);
+ val = upper_32_bits(pcie->cfghw_base_addr);
+ writel(val, pcie->bridge_base_addr + MC_ATR0_AXI4_SLV0_TRSL_ADDR_UDW);
+
+ val = readl(pcie->bridge_base_addr + MC_ATR0_PCIE_WIN0_SRCADDR_PARAM);
+ val |= (ATR0_PCIE_ATR_SIZE << ATR0_PCIE_ATR_SIZE_SHIFT);
+ writel(val, pcie->bridge_base_addr + MC_ATR0_PCIE_WIN0_SRCADDR_PARAM);
+ writel(0, pcie->bridge_base_addr + MC_ATR0_PCIE_WIN0_SRC_ADDR);
+
+ return 0;
+}
+
+static void mc_mask_intx_irq(struct irq_data *data)
+{
+ struct irq_desc *desc = irq_to_desc(data->irq);
+ struct mc_pcie *pcie;
+ unsigned long flags;
+ u32 mask, val;
+
+ pcie = irq_desc_get_chip_data(desc);
+ mask = PCIE_LOCAL_INT_ENABLE;
+ raw_spin_lock_irqsave(&pcie->intx_mask_lock, flags);
+ val = readl(pcie->bridge_base_addr + MC_IMASK_LOCAL);
+ val &= ~mask;
+ writel(val, pcie->bridge_base_addr + MC_IMASK_LOCAL);
+ raw_spin_unlock_irqrestore(&pcie->intx_mask_lock, flags);
+}
+
+static void mc_unmask_intx_irq(struct irq_data *data)
+{
+ struct irq_desc *desc = irq_to_desc(data->irq);
+ struct mc_pcie *pcie;
+ unsigned long flags;
+ u32 mask, val;
+
+ pcie = irq_desc_get_chip_data(desc);
+ mask = PCIE_LOCAL_INT_ENABLE;
+ raw_spin_lock_irqsave(&pcie->intx_mask_lock, flags);
+ val = readl(pcie->bridge_base_addr + MC_IMASK_LOCAL);
+ val |= mask;
+ writel(val, pcie->bridge_base_addr + MC_IMASK_LOCAL);
+ raw_spin_unlock_irqrestore(&pcie->intx_mask_lock, flags);
+}
+
+static struct irq_chip mc_intx_irq_chip = {
+ .name = "microchip_pcie:intx",
+ .irq_enable = mc_unmask_intx_irq,
+ .irq_disable = mc_mask_intx_irq,
+ .irq_mask = mc_mask_intx_irq,
+ .irq_unmask = mc_unmask_intx_irq,
+};
+
+static int mc_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ irq_set_chip_and_handler(irq, &mc_intx_irq_chip, handle_simple_irq);
+ irq_set_chip_data(irq, domain->host_data);
+
+ return 0;
+}
+
+static const struct irq_domain_ops intx_domain_ops = {
+ .map = mc_pcie_intx_map,
+};
+
+static struct irq_chip mc_msi_irq_chip = {
+ .name = "Microchip PCIe MSI",
+ .irq_mask = pci_msi_mask_irq,
+ .irq_unmask = pci_msi_unmask_irq,
+};
+
+static struct msi_domain_info mc_msi_domain_info = {
+ .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+ MSI_FLAG_PCI_MSIX),
+ .chip = &mc_msi_irq_chip,
+};
+
+static void mc_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
+{
+ struct mc_pcie *pcie = irq_data_get_irq_chip_data(data);
+ phys_addr_t addr = pcie->msi.vector_phy;
+
+ msg->address_lo = lower_32_bits(addr);
+ msg->address_hi = upper_32_bits(addr);
+ msg->data = data->hwirq;
+
+ dev_dbg(&pcie->pdev->dev, "msi#%x address_hi %#x address_lo %#x\n",
+ (int)data->hwirq, msg->address_hi, msg->address_lo);
+}
+
+static int mc_msi_set_affinity(struct irq_data *irq_data,
+ const struct cpumask *mask, bool force)
+{
+ return -EINVAL;
+}
+
+static struct irq_chip mc_msi_bottom_irq_chip = {
+ .name = "Microchip MSI",
+ .irq_compose_msi_msg = mc_compose_msi_msg,
+ .irq_set_affinity = mc_msi_set_affinity,
+};
+
+static int mc_irq_msi_domain_alloc(struct irq_domain *domain,
+ unsigned int virq, unsigned int nr_irqs,
+ void *args)
+{
+ struct mc_pcie *pcie = domain->host_data;
+ struct mc_msi *msi = &pcie->msi;
+ unsigned long bit;
+ u32 reg;
+
+ WARN_ON(nr_irqs != 1);
+ mutex_lock(&msi->lock);
+ bit = find_first_zero_bit(msi->used, msi->num_vectors);
+ if (bit >= msi->num_vectors) {
+ mutex_unlock(&msi->lock);
+ return -ENOSPC;
+ }
+
+ set_bit(bit, msi->used);
+ mutex_unlock(&msi->lock);
+
+ irq_domain_set_info(domain, virq, bit, &mc_msi_bottom_irq_chip,
+ domain->host_data, handle_simple_irq, NULL, NULL);
+
+ /* Enable MSI interrupts */
+ reg = readl(pcie->bridge_base_addr + MC_IMASK_LOCAL);
+ reg |= PCIE_ENABLE_MSI;
+ writel(reg, pcie->bridge_base_addr + MC_IMASK_LOCAL);
+
+ return 0;
+}
+
+static void mc_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 mc_pcie *pcie = irq_data_get_irq_chip_data(d);
+ struct mc_msi *msi = &pcie->msi;
+
+ mutex_lock(&msi->lock);
+
+ if (test_bit(d->hwirq, msi->used))
+ __clear_bit(d->hwirq, msi->used);
+ else
+ dev_err(&pcie->pdev->dev, "trying to free unused MSI%lu\n",
+ d->hwirq);
+
+ mutex_unlock(&msi->lock);
+}
+
+static const struct irq_domain_ops msi_domain_ops = {
+ .alloc = mc_irq_msi_domain_alloc,
+ .free = mc_irq_msi_domain_free,
+};
+
+static int mc_allocate_msi_domains(struct mc_pcie *pcie)
+{
+ struct device *dev = &pcie->pdev->dev;
+ struct fwnode_handle *fwnode = of_node_to_fwnode(dev->of_node);
+ struct mc_msi *msi = &pcie->msi;
+
+ mutex_init(&pcie->msi.lock);
+
+ msi->dev_domain = irq_domain_add_linear(NULL, msi->num_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,
+ &mc_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 mc_pcie_init_irq_domains(struct mc_pcie *pcie)
+{
+ struct device *dev = &pcie->pdev->dev;
+ struct device_node *node = dev->of_node;
+
+ pcie->intx_domain = irq_domain_add_linear(node, PCI_NUM_INTX,
+ &intx_domain_ops, pcie);
+ if (!pcie->intx_domain) {
+ dev_err(dev, "failed to get an INTx IRQ domain\n");
+ return -ENOMEM;
+ }
+ raw_spin_lock_init(&pcie->intx_mask_lock);
+
+ return mc_allocate_msi_domains(pcie);
+}
+
+static void mc_pci_unmap_cfg(void *ptr)
+{
+ pci_ecam_free((struct pci_config_window *)ptr);
+}
+
+static int mc_pcie_probe(struct platform_device *pdev)
+{
+ struct mc_pcie *pcie;
+ struct pci_host_bridge *bridge;
+ struct pci_config_window *cfg;
+ struct device *dev = &pdev->dev;
+ struct resource_entry *win;
+ struct resource *bus_range = NULL;
+ struct resource *cfgres;
+ int ret;
+ resource_size_t size;
+ u32 index;
+ u32 atr_sz;
+ u32 val;
+
+ if (!dev->of_node)
+ return -ENODEV;
+
+ /* Allocate the PCIe port */
+ bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
+ if (!bridge)
+ return -ENOMEM;
+
+ pcie = pci_host_bridge_priv(bridge);
+
+ pcie->pdev = pdev;
+
+ cfgres = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ pcie->cfghw_base_addr = cfgres->start;
+ size = resource_size(cfgres);
+ pcie->cfg_size = size;
+ pcie->atr_sz = find_first_bit((const unsigned long *)&size, 64) - 1;
+
+ ret = mc_pcie_parse_dt(pcie);
+ if (ret) {
+ dev_err(dev, "parsing devicetree failed, ret %x\n", ret);
+ return ret;
+ }
+
+ irq_set_chained_handler_and_data(pcie->irq, mc_pcie_isr, pcie);
+
+ ret = mc_host_init(pcie);
+ if (ret) {
+ dev_err(dev, "failed to initialize host\n");
+ return ret;
+ }
+
+ /*
+ * Configure all inbound and outbound windows and prepare
+ * for config access
+ */
+ ret = pci_parse_request_of_pci_ranges(dev, &bridge->windows,
+ &bridge->dma_ranges, &bus_range);
+ if (ret) {
+ dev_err(dev, "failed to parse PCI ranges\n");
+ return ret;
+ }
+
+ index = 1;
+ resource_list_for_each_entry(win, &bridge->windows) {
+ if ((resource_type(win->res) != IORESOURCE_MEM) &&
+ (resource_type(win->res) != IORESOURCE_IO))
+ continue;
+
+ size = resource_size(win->res);
+ atr_sz = find_first_bit((const unsigned long *)&size, 64) - 1;
+
+ /*
+ * Configure Address Translation Table index for PCI
+ * mem space
+ */
+ writel(PCIE_TX_RX_INTERFACE, pcie->bridge_base_addr +
+ (index * ATT_ENTRY_SIZE) +
+ MC_ATR0_AXI4_SLV0_TRSL_PARAM);
+
+ val = lower_32_bits(win->res->start) |
+ (atr_sz << ATR_SIZE_SHIFT) |
+ ATR_IMPL_ENABLE;
+
+ writel(val, pcie->bridge_base_addr +
+ (index * ATT_ENTRY_SIZE) +
+ MC_ATR0_AXI4_SLV0_SRCADDR_PARAM);
+
+ val = upper_32_bits(win->res->start);
+ writel(val, pcie->bridge_base_addr + (index * ATT_ENTRY_SIZE) +
+ MC_ATR0_AXI4_SLV0_SRC_ADDR);
+
+ val = lower_32_bits(win->res->start - win->offset);
+ writel(val, pcie->bridge_base_addr +
+ (index * ATT_ENTRY_SIZE) +
+ MC_ATR0_AXI4_SLV0_TRSL_ADDR_LSB);
+
+ val = upper_32_bits(win->res->start);
+ writel(val, pcie->bridge_base_addr + (index * ATT_ENTRY_SIZE) +
+ MC_ATR0_AXI4_SLV0_TRSL_ADDR_UDW);
+
+ index++;
+ }
+
+ ret = mc_pcie_init_irq_domains(pcie);
+ if (ret) {
+ dev_err(dev, "failed creating IRQ domains\n");
+ return ret;
+ }
+
+ /* Parse and map our Configuration Space windows */
+ cfg = pci_ecam_create(dev, cfgres, bus_range, &pci_generic_ecam_ops);
+ if (IS_ERR(cfg)) {
+ dev_err(dev, "failed creating Configuration Space\n");
+ return PTR_ERR(cfg);
+ }
+
+ pcie->cfg_base_addr = cfg->win;
+
+ val = cfg_readl(pcie, PCI_PRIMARY_BUS);
+ val &= 0xff000000;
+ val |= 0x00ff0100;
+ cfg_writel(pcie, val, PCI_PRIMARY_BUS);
+
+ /* Hardware doesn't setup MSI by default */
+ mc_pcie_enable_msi(pcie);
+
+ val = PCIE_ENABLE_MSI | PCIE_LOCAL_INT_ENABLE;
+ writel(val, pcie->bridge_base_addr + MC_IMASK_LOCAL);
+
+ ret = devm_add_action_or_reset(dev, mc_pci_unmap_cfg, cfg);
+ if (ret)
+ return ret;
+
+ /* Initialize bridge */
+ bridge->dev.parent = dev;
+ bridge->sysdata = cfg;
+ bridge->busnr = cfg->busr.start;
+ bridge->ops = &pci_generic_ecam_ops.pci_ops;
+ bridge->map_irq = of_irq_parse_and_map_pci;
+ bridge->swizzle_irq = pci_common_swizzle;
+
+ return pci_host_probe(bridge);
+}
+
+static const struct of_device_id mc_pcie_of_match[] = {
+ { .compatible = "microchip,pf-axi-pcie-host-1.0" },
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, mc_pcie_of_match)
+
+static struct platform_driver mc_pcie_driver = {
+ .probe = mc_pcie_probe,
+ .driver = {
+ .name = "microchip-pcie",
+ .of_match_table = mc_pcie_of_match,
+ .suppress_bind_attrs = true,
+ },
+};
+
+builtin_platform_driver(mc_pcie_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Microchip PCIe host controller driver");
+MODULE_AUTHOR("Daire McNamara <daire.mcnamara@microchip.com>");
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v10 1/2] PCI: microchip: Add host driver for Microchip PCIe controller
From: Daire.McNamara @ 2020-05-26 14:54 UTC (permalink / raw)
To: amurray, lorenzo.pieralisi, linux-pci, bhelgaas, robh+dt,
devicetree
Cc: david.abdurachmanov
In-Reply-To: <930a4f34fd11be89bc66cfa35b249c9b685aa8c2.camel@microchip.com>
add device tree bindings for the Microchip PCIe PolarFire PCIe controller
when configured in host (Root Complex) mode.
Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
---
.../bindings/pci/microchip,pcie-host.yaml | 93 +++++++++++++++++++
1 file changed, 93 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
diff --git a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
new file mode 100644
index 000000000000..26b8cf94a746
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/microchip,pcie-host.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PCIe Root Port Bridge Controller Device Tree Bindings
+
+maintainers:
+ - Daire McNamara <daire.mcnamara@microchip.com>
+
+allOf:
+ - $ref: /schemas/pci/pci-bus.yaml#
+
+properties:
+ compatible:
+ const: microchip,pcie-host-1.0 # PolarFire
+
+ reg:
+ maxItems: 2
+
+ reg-names:
+ items:
+ - const: cfg
+ - const: apb
+
+ interrupts:
+ minItems: 1
+ maxItems: 2
+ items:
+ - description: PCIe host controller
+ - description: builtin MSI controller
+
+ interrupt-names:
+ minItems: 1
+ maxItems: 2
+ items:
+ - const: pcie
+ - const: msi
+
+ ranges:
+ maxItems: 1
+
+ dma-ranges:
+ maxItems: 1
+
+ msi-controller:
+ description: Identifies the node as an MSI controller.
+
+ msi-parent:
+ description: MSI controller the device is capable of using.
+
+required:
+ - reg
+ - reg-names
+ - dma-ranges
+ - "#interrupt-cells"
+ - interrupts
+ - interrupt-map-mask
+ - interrupt-map
+ - msi-controller
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ soc {
+ #address-cells = <2>;
+ #size-cells = <1>;
+ pcie0: pcie@2030000000 {
+ compatible = "microchip,pcie-host-1.0";
+ reg = <0x20 0x30000000 0x0 0x4000000>,
+ <0x20 0x0 0x0 0x100000>;
+ reg-names = "cfg", "apb";
+ device_type = "pci";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+ interrupts = <32>;
+ interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+ interrupt-map = <0 0 0 1 &pcie0 0>,
+ <0 0 0 2 &pcie0 1>,
+ <0 0 0 3 &pcie0 2>,
+ <0 0 0 4 &pcie0 3>;
+ interrupt-parent = <&plic0>;
+ interrupt-controller;
+ msi-parent = <&pcie0>;
+ msi-controller;
+ bus-range = <0x00 0x7f>;
+ ranges = <0x03000000 0x0 0x40000000 0x0 0x40000000 0x0 0x20000000>;
+ dma-ranges = <0x02000000 0x0 0x00000000 0x0 0x00000000 0x1 0x00000000>;
+ };
+ };
--
2.17.1
^ permalink raw reply related
* [PATCHv2 1/2] spi: dw: add reset control
From: Dinh Nguyen @ 2020-05-26 15:12 UTC (permalink / raw)
To: linux-kernel
Cc: dinguyen, devicetree, broonie, robh+dt, linux-spi, Sergey.Semin,
fancer.lancer, andriy.shevchenko, lars.povlsen, Liang Jin J
Add mechanism to get the reset control and deassert it in order to bring
the IP out of reset.
Signed-off-by: Liang Jin J <liang.j.jin@ericsson.com>
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
v2: use _get_optional_exclusive
put IP back into reset if there was an error in probe function
---
drivers/spi/spi-dw-mmio.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 384a3ab6dc2d..07e015b6d03d 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -20,6 +20,7 @@
#include <linux/acpi.h>
#include <linux/property.h>
#include <linux/regmap.h>
+#include <linux/reset.h>
#include "spi-dw.h"
@@ -30,6 +31,7 @@ struct dw_spi_mmio {
struct clk *clk;
struct clk *pclk;
void *priv;
+ struct reset_control *rstc;
};
#define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL 0x24
@@ -175,6 +177,14 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
if (ret)
goto out_clk;
+ /* find an optional reset controller */
+ dwsmmio->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, "spi");
+ if (IS_ERR(dwsmmio->rstc)) {
+ if (PTR_ERR(dwsmmio->rstc) == -EPROBE_DEFER)
+ return PTR_ERR(dwsmmio->rstc);
+ }
+ reset_control_deassert(dwsmmio->rstc);
+
dws->bus_num = pdev->id;
dws->max_freq = clk_get_rate(dwsmmio->clk);
@@ -208,6 +218,8 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
clk_disable_unprepare(dwsmmio->pclk);
out_clk:
clk_disable_unprepare(dwsmmio->clk);
+ reset_control_assert(dwsmmio->rstc);
+
return ret;
}
@@ -219,6 +231,7 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
clk_disable_unprepare(dwsmmio->pclk);
clk_disable_unprepare(dwsmmio->clk);
+ reset_control_assert(dwsmmio->rstc);
return 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCHv2 2/2] dt-bindings: snps,dw-apb-ssi: add optional reset property
From: Dinh Nguyen @ 2020-05-26 15:12 UTC (permalink / raw)
To: linux-kernel
Cc: dinguyen, devicetree, broonie, robh+dt, linux-spi, Sergey.Semin,
fancer.lancer, andriy.shevchenko, lars.povlsen
In-Reply-To: <20200526151218.6186-1-dinguyen@kernel.org>
Add optional reset property.
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
v2: actually document the "resets" and "reset-names" optional properties
---
Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
index 3ed08ee9feba..c679778612f3 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
@@ -22,6 +22,9 @@ Optional properties:
- num-cs : The number of chipselects. If omitted, this will default to 4.
- reg-io-width : The I/O register width (in bytes) implemented by this
device. Supported values are 2 or 4 (the default).
+- resets : contains an entry for each entry in reset-names.
+ See ../reset/reset.txt for details.
+- reset-names : must contain "spi"
Child nodes as per the generic SPI binding.
@@ -37,5 +40,7 @@ Example:
num-cs = <2>;
cs-gpios = <&gpio0 13 0>,
<&gpio0 14 0>;
+ resets = <&rst SPIM0_RST>;
+ reset-names = "spi";
};
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v5 03/14] PCI: cadence: Convert all r/w accessors to perform only 32-bit accesses
From: Rob Herring @ 2020-05-26 15:12 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Tom Joseph, Lorenzo Pieralisi, Bjorn Helgaas, PCI,
linux-kernel@vger.kernel.org, Arnd Bergmann, Greg Kroah-Hartman,
devicetree, linux-omap,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <df29309d-8401-4040-eb1e-90bb3af93a82@ti.com>
On Sun, May 24, 2020 at 9:30 PM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
> Hi Rob,
>
> On 5/22/2020 9:24 PM, Rob Herring wrote:
> > On Thu, May 21, 2020 at 9:37 PM Kishon Vijay Abraham I <kishon@ti.com> wrote:
> >>
> >> Certain platforms like TI's J721E using Cadence PCIe IP can perform only
> >> 32-bit accesses for reading or writing to Cadence registers. Convert all
> >> read and write accesses to 32-bit in Cadence PCIe driver in preparation
> >> for adding PCIe support in TI's J721E SoC.
> >
> > Looking more closely I don't think cdns_pcie_ep_assert_intx is okay
> > with this and never can be given the PCI_COMMAND and PCI_STATUS
> > registers are in the same word (IIRC, that's the main reason 32-bit
> > config space accesses are broken). So this isn't going to work at
>
> right, PCI_STATUS has write '1' to clear bits and there's a chance that it
> could be reset while raising legacy interrupt. While this cannot be avoided for
> TI's J721E, other platforms doesn't have to have this limitation.
> > least for EP accesses. And maybe you need a custom .raise_irq() hook
> > to minimize any problems (such as making the RMW atomic at least from
> > the endpoint's perspective).
>
> This is to make sure EP doesn't update in-consistent state when RC is updating
> the PCI_STATUS register? Since this involves two different systems, how do we
> make this atomic?
You can't make it atomic WRT both systems, but is there locking around
each RMW? Specifically, are preemption and interrupts disabled to
ensure time between a read and write are minimized? You wouldn't want
interrupts disabled during the delay too though (i.e. around
.raise_irq()).
BTW, I've asked this question before, but aren't PCI legacy interrupts
level triggered? If so, isn't generating a pulse wrong?
Rob
^ permalink raw reply
* Re: [PATCHv2 1/2] spi: dw: add reset control
From: Andy Shevchenko @ 2020-05-26 15:21 UTC (permalink / raw)
To: Dinh Nguyen
Cc: linux-kernel, devicetree, broonie, robh+dt, linux-spi,
Sergey.Semin, fancer.lancer, lars.povlsen, Liang Jin J
In-Reply-To: <20200526151218.6186-1-dinguyen@kernel.org>
On Tue, May 26, 2020 at 10:12:17AM -0500, Dinh Nguyen wrote:
> Add mechanism to get the reset control and deassert it in order to bring
> the IP out of reset.
...
> struct clk *clk;
> struct clk *pclk;
> void *priv;
> + struct reset_control *rstc;
I think either you make it one space or you align all the rest members
accordingly. I would rather go with former.
...
> + /* find an optional reset controller */
> + dwsmmio->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, "spi");
> + if (IS_ERR(dwsmmio->rstc)) {
> + if (PTR_ERR(dwsmmio->rstc) == -EPROBE_DEFER)
> + return PTR_ERR(dwsmmio->rstc);
If it's other type of errors when reset control, we ignore them...
Why?!
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: sram: add documentation for reserved-only flag
From: Mian Yousaf Kaukab @ 2020-05-26 15:28 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Stephen Warren, robin.murphy, devicetree, talho,
jonathanh, linux-tegra, linux-kernel, linux-arm-kernel, afaerber,
arnd, gregkh
In-Reply-To: <20200520085558.GB2136208@ulmo>
On Wed, May 20, 2020 at 10:55:58AM +0200, Thierry Reding wrote:
> On Tue, May 19, 2020 at 05:03:26PM -0600, Rob Herring wrote:
> > On Tue, May 19, 2020 at 10:16:43AM -0600, Stephen Warren wrote:
> > > On 5/13/20 4:41 AM, Mian Yousaf Kaukab wrote:
> > > > On Tue, May 12, 2020 at 01:45:28PM -0600, Stephen Warren wrote:
> > > >> On 5/12/20 8:48 AM, Mian Yousaf Kaukab wrote:
> > > >>> Add documentation for the new optional flag added for SRAM driver.
> > > >>
> > > >>> diff --git a/Documentation/devicetree/bindings/sram/sram.yaml b/Documentation/devicetree/bindings/sram/sram.yaml
> > > >>
> > > >>> + reserved-only:
> > > >>> + description:
> > > >>> + The flag indicating, that only SRAM reserved regions have to be remapped.
> > > >>> + remapping type is selected depending upon no-memory-wc as usual.
> > > >>> + type: boolean
> > > >>
> > > >> This feels a bit like a SW flag rather than a HW description, so I'm not
> > > >> sure it's appropriate to put it into DT.
> > > >
> > > > Reserved regions themselves are software descriptions, no? Then we have 'pool'
> > > > flag which is again a software flag and so on. This flag falls into same
> > > > category and nothing out of ordinary.
> > >
> > > I suppose that's true to some extent. This is indeed a description of
> > > the system environment presented to the SW that consumes the DT, which
> > > is a bit more than pure HW description but still a description of
> > > something imposed externally rather than describing something that's up
> > > to the discretion of the consuming SW. So, go ahead.
> > >
> > > >> Are there any cases where the SW should map all of the SRAM, i.e. where
> > > >> we wouldn't expect to set reserved-only? [...]
> > > >
> > > > Yes, here are a few examples:
> > > > arch/arm/boot/dts/aspeed-g*.dtsi
> > > > arch/arm/boot/dts/at91*.dtsi
> > > > arch/arm/boot/dts/bcm7445.dtsi
> > > > Then arch/arm/boot/dts/dra7.dtsi is an example where we should map everything
> > > > except the reserved region.
> > > >
> > > >> [...] I'd expect reserved-only to be
> > > >> the default, and perhaps only, mode of operation for the SRAM driver.
> > > >
> > > > It will break compatibility with existing dtbs.
> > > >
> > > >> If we can't do that because some SW currently expects to be able to map
> > > >> arbitrary portions of the SRAM, shouldn't that SW be fixed to tell the
> > > >> SRAM driver which parts it's using, hence still allowing the driver to
> > > >> only map in-use portions?
> > > >
> > > > User doesn’t need sram driver in that case. It can use genalloc api directly.
> > >
> > > This sounds a bit odd. Without a driver for the reserved region, nothing
> > > should be touching it, since otherwise there's no code that owns an
> > > manages the region. If any code needs to consume the region, it should
> > > obtain info about the region from some form of provider code that can
> > > handle both the allocation and mapping. Anything else sounds like some
> > > consumer code directly making use of DT nodes it doesn't own. But since
> > > I'm not familiar enough with the SRAM driver and genalloc code that you
> > > mention to fully understand the allocation paths I guess I won't object
> > > for now, although it does still sound fishy.
> >
> > I'm fine with the concept, but I don't think a single flag is adequate.
> > If there are reserved regions within the SRAM, then define child nodes
> > to mark those regions reserved. I don't think you need a new flag. Just
> > a 'reg' property and nothing else.
>
> It sounds to me like there are two different interpretations of SRAM and
> reserved regions. On one hand, as you suggest, we have one SRAM that's
> made available as genalloc pool and then individual regions can be
> marked as reserved so that they aren't added to that pool.
>
> At the same time, each reserved region is also exposed as a separate
> pool and that's in fact used by many consumers as a way of getting a
> specific chunk of the SRAM for their own use (via phandle to the region
> from the consumer's device tree node).
>
> In addition to that, the reserved region code doesn't actually fully do
> its job because while the reserved region isn't actually added to the
> "top-level" SRAM pool, the memory is still mapped. At the same time this
> is something that we actually want because, like I mentioned, some of
> the consumers do want to get at their SRAM chunks via references to the
> partitions.
>
> The problem that this patch series is really trying to solve is another
> still: the complete SRAM is always mapped to kernel memory, irrespective
> of whether any regions are marked reserved or not and that can cause
> speculative accesses to memory outside of the defined regions.
>
> Stephen's suggestion is to default to only mapping memory for which a
> partition has been defined in the SRAM and assuming that all SRAM
> outside of those partitions is off limits. I think that's a sensible
> default and it's unambiguous.
>
> But as Yousaf points out that would break compatibility with existing
> device trees. Depending on how you interpret the bindings one could
> argue that those device trees are buggy and should have partitions
> defined (in the cases I've looked at they end up using a fixed region
> anyway, so that could've just been made explicit in the device tree).
>
> However, it also looks like all of the users that rely on the original
> behaviour where they can just access the full pool are those that don't
> define any reserved regions, whereas all users that do reserve regions
> will actually use those reserved regions.
>
> So I think we can make use of this by differentiating in the driver
> between SRAM nodes with or without children and change the behaviour
> accordingly. I think that has the big advantage that it makes things
> work as (I think) most people would expect and doesn't further
> complicate the binding with extra flags.
I tend to agree on mapping partitions only if they exist. So far I could
only find one exception. It is arch/arm/boot/dts/armada-370.dtsi which is
using the top level pool as well as a partition to reserve 32 bytes at the
bottom of sram. This can be fixed along with the sram driver change, by
adding another partition for the rest of the sram and using its handle in
the crypto@90000 instead of top-level sram node handle. Do you see anymore
exceptions where both top level pool and the partitions both are being used?
Then on the backward compatibility topic, another issue is that boot code
could add sram nodes dynamically. For example arch/arm/mach-k3/common.c in
u-boot does it. This particular case will not break after the suggested change
because it is not adding any partitions. However, there could be other
boot-loaders which are not this lucky.
>
> Thierry
/Yousaf
^ permalink raw reply
* Re: [PATCH v8 3/3] PM / AVS: SVS: Introduce SVS engine
From: Matthias Brugger @ 2020-05-26 15:29 UTC (permalink / raw)
To: Roger Lu
Cc: Enric Balletbo Serra, Kevin Hilman, Rob Herring, Nicolas Boichat,
Stephen Boyd, Mark Rutland, Nishanth Menon, Angus Lin,
devicetree@vger.kernel.org, Linux PM list, linux-kernel,
Xiaoqing Liu, YT Lee, Fan Chen,
moderated list:ARM/Mediatek SoC support, HenryC Chen,
Charles Yang, Linux ARM
In-Reply-To: <1590484328.4392.44.camel@mtksdaap41>
On 26/05/2020 11:12, Roger Lu wrote:
> Hi Matthias,
>
> Thanks for the feedback.
>
> On Fri, 2020-05-22 at 17:38 +0200, Matthias Brugger wrote:
>>
>> On 22/05/2020 11:40, Roger Lu wrote:
>>>
>>> Hi Enric,
>>>
>>> On Tue, 2020-05-19 at 17:30 +0200, Enric Balletbo Serra wrote:
>>>> Hi Roger,
>>>>
>>>> Thank you for your patch. I have the feeling that this driver is
>>>> complex and difficult to follow and I am wondering if it wouldn't be
>>>> better if you can send a version that simply adds basic functionality
>>>> for now. Some comments below.
>>>
>>> Thanks for the advices. I'll submit SVS v9 with basic functionality
>>> patch + step by step functionalities' patches.
>>>
>>>>
>>>> Missatge de Roger Lu <roger.lu@mediatek.com> del dia dl., 18 de maig
>>>> 2020 a les 11:25:
>>>>>
>>>>> The SVS (Smart Voltage Scaling) engine is a piece
>>>>> of hardware which is used to calculate optimized
>>>>> voltage values of several power domains,
>>>>> e.g. CPU/GPU/CCI, according to chip process corner,
>>>>> temperatures, and other factors. Then DVFS driver
>>>>> could apply those optimized voltage values to reduce
>>>>> power consumption.
>>>>>
>>>>> Signed-off-by: Roger Lu <roger.lu@mediatek.com>
>>>>> ---
>>>>> drivers/power/avs/Kconfig | 10 +
>>>>> drivers/power/avs/Makefile | 1 +
>>>>> drivers/power/avs/mtk_svs.c | 2119 +++++++++++++++++++++++++++++++++
>>>>> include/linux/power/mtk_svs.h | 23 +
>>>>> 4 files changed, 2153 insertions(+)
>>>>> create mode 100644 drivers/power/avs/mtk_svs.c
>>>>> create mode 100644 include/linux/power/mtk_svs.h
>>>>>
>>>>> diff --git a/drivers/power/avs/Kconfig b/drivers/power/avs/Kconfig
>>>>> index cdb4237bfd02..67089ac6040e 100644
>>>>> --- a/drivers/power/avs/Kconfig
>>>>> +++ b/drivers/power/avs/Kconfig
>>>>> @@ -35,3 +35,13 @@ config ROCKCHIP_IODOMAIN
>>>>> Say y here to enable support io domains on Rockchip SoCs. It is
>>>>> necessary for the io domain setting of the SoC to match the
>>>>> voltage supplied by the regulators.
>>>>> +
>>>>> +config MTK_SVS
>>>>> + bool "MediaTek Smart Voltage Scaling(SVS)"
>>>>
>>>> Can't be this a module? Why? In such case, you should use tristate option
>>>
>>> Generally, MTK_SVS is needed in MTK SoC(mt81xx) products. So, we don't provide
>>> module option in config. If, somehow, SVS isn't needed, we suggest
>>> CONFIG_MTK_SVS=n to be set.
>>>
>>
>> The question here is if it needs to be probed before we probe the modules. If
>> not, we should add a Kconfig option for MT81xx SoCs to select MTK_SVS.
>
> Excuse me to make you confuse. MT81xx SoCs is the subset MTK ICs that
> will use CONFIG_MTK_SVS. In other words, CONFIG_MTK_SVS will be used
> with other MTK ICs as well. So, MTK_SVS is the general naming for MTK IC
> to enable SVS power feature. Anyway, back to Enric's question, I'll make
> MTK_SVS become a tristate feature in the next patch. Thanks.
>
>>
[...]
>>>>> +
>>>>> +static const u32 svs_regs_v2[] = {
>>>>
>>>> Is this SoC specific or shared between SoCs?
>>>
>>> Shared between SoCs. Some SVS in MTK SoCs use v2 register map.
>>>
>>
>> And which silicon uses v1 then? Is v2 a MediaTek internal naming you want to keep?
>
> 1. MT8173 IC uses v1 register map.
> 2. Yes, I'll keep v2 postfix.
>
Sounds good, thanks for clarification.
Regards,
Matthias
^ permalink raw reply
* [PATCH v3 2/7] dt-bindings: watchdog: dw-wdt: Support devices with asynch clocks
From: Serge Semin @ 2020-05-26 15:41 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Rob Herring
Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
Thomas Bogendoerfer, Arnd Bergmann, linux-mips, linux-watchdog,
devicetree, linux-kernel
In-Reply-To: <20200526154123.24402-1-Sergey.Semin@baikalelectronics.ru>
DW Watchdog IP core can be synthesised with asynchronous timer/APB
clocks support (WDT_ASYNC_CLK_MODE_ENABLE == 1). In this case
separate clock signals are supposed to be used to feed watchdog timer
and APB interface of the device. Let's update the DW Watchdog DT node
schema so it would support the optional APB3 bus clock specified along
with the mandatory watchdog timer reference clock.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org
---
Changelog v2:
- It's a new patch unpinned from the previous one.
---
.../devicetree/bindings/watchdog/snps,dw-wdt.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
index 4f6944756ab4..5bf6dc6377f3 100644
--- a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
@@ -24,8 +24,16 @@ properties:
maxItems: 1
clocks:
+ minItems: 1
items:
- description: Watchdog timer reference clock
+ - description: APB3 interface clock
+
+ clock-names:
+ minItems: 1
+ items:
+ - const: tclk
+ - const: pclk
resets:
description: Phandle to the DW Watchdog reset lane
--
2.26.2
^ permalink raw reply related
* [PATCH v3 3/7] dt-bindings: watchdog: dw-wdt: Add watchdog TOPs array property
From: Serge Semin @ 2020-05-26 15:41 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Rob Herring
Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
Thomas Bogendoerfer, Arnd Bergmann, linux-mips, linux-watchdog,
devicetree, linux-kernel
In-Reply-To: <20200526154123.24402-1-Sergey.Semin@baikalelectronics.ru>
In case if DW Watchdog IP core is built with WDT_USE_FIX_TOP == false,
a custom timeout periods are used to preset the timer counter. In
this case that periods should be specified in a new "snps,watchdog-tops"
property of the DW watchdog dts node.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org
---
Changelog v2:
- Rearrange SoBs.
- Move $ref to the root level of the "snps,watchdog-tops" property
so does the constraints.
- Add default TOP values array.
- Discard the label definition from the new bindings example.
Changelog v3:
- Remove items property and move the minItems and maxItems constraints to
the root level of the snps,watchdog-tops property.
---
.../bindings/watchdog/snps,dw-wdt.yaml | 32 +++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
index 5bf6dc6377f3..d9fc7bb851b1 100644
--- a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
@@ -39,6 +39,23 @@ properties:
description: Phandle to the DW Watchdog reset lane
maxItems: 1
+ snps,watchdog-tops:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ DW APB Watchdog custom timer intervals - Timeout Period ranges (TOPs).
+ Each TOP is a number loaded into the watchdog counter at the moment of
+ the timer restart. The counter decrementing happens each tick of the
+ reference clock. Therefore the TOPs array is equivalent to an array of
+ the timer expiration intervals supported by the DW APB Watchdog. Note
+ DW APB Watchdog IP-core might be synthesized with fixed TOP values,
+ in which case this property is unnecessary with default TOPs utilized.
+ default: [0x0001000 0x0002000 0x0004000 0x0008000
+ 0x0010000 0x0020000 0x0040000 0x0080000
+ 0x0100000 0x0200000 0x0400000 0x0800000
+ 0x1000000 0x2000000 0x4000000 0x8000000]
+ minItems: 16
+ maxItems: 16
+
unevaluatedProperties: false
required:
@@ -55,4 +72,19 @@ examples:
clocks = <&per_base_clk>;
resets = <&wdt_rst>;
};
+
+ - |
+ watchdog@ffd02000 {
+ compatible = "snps,dw-wdt";
+ reg = <0xffd02000 0x1000>;
+ interrupts = <0 171 4>;
+ clocks = <&per_base_clk>;
+ clock-names = "tclk";
+ snps,watchdog-tops = <0x000000FF 0x000001FF 0x000003FF
+ 0x000007FF 0x0000FFFF 0x0001FFFF
+ 0x0003FFFF 0x0007FFFF 0x000FFFFF
+ 0x001FFFFF 0x003FFFFF 0x007FFFFF
+ 0x00FFFFFF 0x01FFFFFF 0x03FFFFFF
+ 0x07FFFFFF>;
+ };
...
--
2.26.2
^ permalink raw reply related
* [PATCH v3 7/7] watchdog: dw_wdt: Add DebugFS files
From: Serge Semin @ 2020-05-26 15:41 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Arnd Bergmann, Rob Herring, linux-mips, devicetree,
linux-watchdog, linux-kernel
In-Reply-To: <20200526154123.24402-1-Sergey.Semin@baikalelectronics.ru>
For the sake of the easier device-driver debug procedure, we added a
DebugFS file with the controller registers state. It's available only if
kernel is configured with DebugFS support.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v2:
- Rearrange SoBs.
- Discard timeout/pretimeout/ping/enable DebugFS nodes. Registers state
dump node is only left.
---
drivers/watchdog/dw_wdt.c | 68 +++++++++++++++++++++++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 3cd7c485cd70..012681baaa6d 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -28,6 +28,7 @@
#include <linux/platform_device.h>
#include <linux/reset.h>
#include <linux/watchdog.h>
+#include <linux/debugfs.h>
#define WDOG_CONTROL_REG_OFFSET 0x00
#define WDOG_CONTROL_REG_WDT_EN_MASK 0x01
@@ -39,8 +40,14 @@
#define WDOG_COUNTER_RESTART_KICK_VALUE 0x76
#define WDOG_INTERRUPT_STATUS_REG_OFFSET 0x10
#define WDOG_INTERRUPT_CLEAR_REG_OFFSET 0x14
+#define WDOG_COMP_PARAMS_5_REG_OFFSET 0xe4
+#define WDOG_COMP_PARAMS_4_REG_OFFSET 0xe8
+#define WDOG_COMP_PARAMS_3_REG_OFFSET 0xec
+#define WDOG_COMP_PARAMS_2_REG_OFFSET 0xf0
#define WDOG_COMP_PARAMS_1_REG_OFFSET 0xf4
#define WDOG_COMP_PARAMS_1_USE_FIX_TOP BIT(6)
+#define WDOG_COMP_VERSION_REG_OFFSET 0xf8
+#define WDOG_COMP_TYPE_REG_OFFSET 0xfc
/* There are sixteen TOPs (timeout periods) that can be set in the watchdog. */
#define DW_WDT_NUM_TOPS 16
@@ -85,6 +92,10 @@ struct dw_wdt {
/* Save/restore */
u32 control;
u32 timeout;
+
+#ifdef CONFIG_DEBUG_FS
+ struct dentry *dbgfs_dir;
+#endif
};
#define to_dw_wdt(wdd) container_of(wdd, struct dw_wdt, wdd)
@@ -484,6 +495,59 @@ static int dw_wdt_init_timeouts(struct dw_wdt *dw_wdt, struct device *dev)
return 0;
}
+#ifdef CONFIG_DEBUG_FS
+
+#define DW_WDT_DBGFS_REG(_name, _off) \
+{ \
+ .name = _name, \
+ .offset = _off \
+}
+
+static const struct debugfs_reg32 dw_wdt_dbgfs_regs[] = {
+ DW_WDT_DBGFS_REG("cr", WDOG_CONTROL_REG_OFFSET),
+ DW_WDT_DBGFS_REG("torr", WDOG_TIMEOUT_RANGE_REG_OFFSET),
+ DW_WDT_DBGFS_REG("ccvr", WDOG_CURRENT_COUNT_REG_OFFSET),
+ DW_WDT_DBGFS_REG("crr", WDOG_COUNTER_RESTART_REG_OFFSET),
+ DW_WDT_DBGFS_REG("stat", WDOG_INTERRUPT_STATUS_REG_OFFSET),
+ DW_WDT_DBGFS_REG("param5", WDOG_COMP_PARAMS_5_REG_OFFSET),
+ DW_WDT_DBGFS_REG("param4", WDOG_COMP_PARAMS_4_REG_OFFSET),
+ DW_WDT_DBGFS_REG("param3", WDOG_COMP_PARAMS_3_REG_OFFSET),
+ DW_WDT_DBGFS_REG("param2", WDOG_COMP_PARAMS_2_REG_OFFSET),
+ DW_WDT_DBGFS_REG("param1", WDOG_COMP_PARAMS_1_REG_OFFSET),
+ DW_WDT_DBGFS_REG("version", WDOG_COMP_VERSION_REG_OFFSET),
+ DW_WDT_DBGFS_REG("type", WDOG_COMP_TYPE_REG_OFFSET)
+};
+
+static void dw_wdt_dbgfs_init(struct dw_wdt *dw_wdt)
+{
+ struct device *dev = dw_wdt->wdd.parent;
+ struct debugfs_regset32 *regset;
+
+ regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
+ if (!regset)
+ return;
+
+ regset->regs = dw_wdt_dbgfs_regs;
+ regset->nregs = ARRAY_SIZE(dw_wdt_dbgfs_regs);
+ regset->base = dw_wdt->regs;
+
+ dw_wdt->dbgfs_dir = debugfs_create_dir(dev_name(dev), NULL);
+
+ debugfs_create_regset32("registers", 0444, dw_wdt->dbgfs_dir, regset);
+}
+
+static void dw_wdt_dbgfs_clear(struct dw_wdt *dw_wdt)
+{
+ debugfs_remove_recursive(dw_wdt->dbgfs_dir);
+}
+
+#else /* !CONFIG_DEBUG_FS */
+
+static void dw_wdt_dbgfs_init(struct dw_wdt *dw_wdt) {}
+static void dw_wdt_dbgfs_clear(struct dw_wdt *dw_wdt) {}
+
+#endif /* !CONFIG_DEBUG_FS */
+
static int dw_wdt_drv_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -607,6 +671,8 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
if (ret)
goto out_disable_pclk;
+ dw_wdt_dbgfs_init(dw_wdt);
+
return 0;
out_disable_pclk:
@@ -621,6 +687,8 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
{
struct dw_wdt *dw_wdt = platform_get_drvdata(pdev);
+ dw_wdt_dbgfs_clear(dw_wdt);
+
watchdog_unregister_device(&dw_wdt->wdd);
reset_control_assert(dw_wdt->rst);
clk_disable_unprepare(dw_wdt->pclk);
--
2.26.2
^ permalink raw reply related
* [PATCH v3 6/7] watchdog: dw_wdt: Add pre-timeouts support
From: Serge Semin @ 2020-05-26 15:41 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Arnd Bergmann, Rob Herring, linux-mips, devicetree,
linux-watchdog, linux-kernel
In-Reply-To: <20200526154123.24402-1-Sergey.Semin@baikalelectronics.ru>
DW Watchdog can rise an interrupt in case if IRQ request mode is enabled
and timer reaches the zero value. In this case the IRQ lane is left
pending until either the next watchdog kick event (watchdog restart) or
until the WDT_EOI register is read or the device/system reset. This
interface can be used to implement the pre-timeout functionality
optionally provided by the Linux kernel watchdog devices.
IRQ mode provides a two stages timeout interface. It means the IRQ is
raised when the counter reaches zero, while the system reset occurs only
after subsequent timeout if the timer restart is not performed. Due to
this peculiarity the pre-timeout value is actually set to the achieved
hardware timeout, while the real watchdog timeout is considered to be
twice as much of it. This applies a significant limitation on the
pre-timeout values, so current implementation supports either zero value,
which disables the pre-timeout events, or non-zero values, which imply
the pre-timeout to be at least half of the current watchdog timeout.
Note that we ask the interrupt controller to detect the rising-edge
pre-timeout interrupts to prevent the high-level-IRQs flood, since
if the pre-timeout happens, the IRQ lane will be left pending until
it's cleared by the timer restart.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v2:
- Rearrange SoBs.
- Make the Pre-timeout IRQ being optionally supported.
---
drivers/watchdog/dw_wdt.c | 138 +++++++++++++++++++++++++++++++++++---
1 file changed, 130 insertions(+), 8 deletions(-)
diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index efbc36872670..3cd7c485cd70 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -22,6 +22,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
+#include <linux/interrupt.h>
#include <linux/of.h>
#include <linux/pm.h>
#include <linux/platform_device.h>
@@ -36,6 +37,8 @@
#define WDOG_CURRENT_COUNT_REG_OFFSET 0x08
#define WDOG_COUNTER_RESTART_REG_OFFSET 0x0c
#define WDOG_COUNTER_RESTART_KICK_VALUE 0x76
+#define WDOG_INTERRUPT_STATUS_REG_OFFSET 0x10
+#define WDOG_INTERRUPT_CLEAR_REG_OFFSET 0x14
#define WDOG_COMP_PARAMS_1_REG_OFFSET 0xf4
#define WDOG_COMP_PARAMS_1_USE_FIX_TOP BIT(6)
@@ -59,6 +62,11 @@ module_param(nowayout, bool, 0);
MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "
"(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+enum dw_wdt_rmod {
+ DW_WDT_RMOD_RESET = 1,
+ DW_WDT_RMOD_IRQ = 2
+};
+
struct dw_wdt_timeout {
u32 top_val;
unsigned int sec;
@@ -70,6 +78,7 @@ struct dw_wdt {
struct clk *clk;
struct clk *pclk;
unsigned long rate;
+ enum dw_wdt_rmod rmod;
struct dw_wdt_timeout timeouts[DW_WDT_NUM_TOPS];
struct watchdog_device wdd;
struct reset_control *rst;
@@ -86,6 +95,20 @@ static inline int dw_wdt_is_enabled(struct dw_wdt *dw_wdt)
WDOG_CONTROL_REG_WDT_EN_MASK;
}
+static void dw_wdt_update_mode(struct dw_wdt *dw_wdt, enum dw_wdt_rmod rmod)
+{
+ u32 val;
+
+ val = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
+ if (rmod == DW_WDT_RMOD_IRQ)
+ val |= WDOG_CONTROL_REG_RESP_MODE_MASK;
+ else
+ val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
+ writel(val, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
+
+ dw_wdt->rmod = rmod;
+}
+
static unsigned int dw_wdt_find_best_top(struct dw_wdt *dw_wdt,
unsigned int timeout, u32 *top_val)
{
@@ -145,7 +168,11 @@ static unsigned int dw_wdt_get_timeout(struct dw_wdt *dw_wdt)
break;
}
- return dw_wdt->timeouts[idx].sec;
+ /*
+ * In IRQ mode due to the two stages counter, the actual timeout is
+ * twice greater than the TOP setting.
+ */
+ return dw_wdt->timeouts[idx].sec * dw_wdt->rmod;
}
static int dw_wdt_ping(struct watchdog_device *wdd)
@@ -164,7 +191,20 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
unsigned int timeout;
u32 top_val;
- timeout = dw_wdt_find_best_top(dw_wdt, top_s, &top_val);
+ /*
+ * Note IRQ mode being enabled means having a non-zero pre-timeout
+ * setup. In this case we try to find a TOP as close to the half of the
+ * requested timeout as possible since DW Watchdog IRQ mode is designed
+ * in two stages way - first timeout rises the pre-timeout interrupt,
+ * second timeout performs the system reset. So basically the effective
+ * watchdog-caused reset happens after two watchdog TOPs elapsed.
+ */
+ timeout = dw_wdt_find_best_top(dw_wdt, DIV_ROUND_UP(top_s, dw_wdt->rmod),
+ &top_val);
+ if (dw_wdt->rmod == DW_WDT_RMOD_IRQ)
+ wdd->pretimeout = timeout;
+ else
+ wdd->pretimeout = 0;
/*
* Set the new value in the watchdog. Some versions of dw_wdt
@@ -175,25 +215,47 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
writel(top_val | top_val << WDOG_TIMEOUT_RANGE_TOPINIT_SHIFT,
dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
+ /* Kick new TOP value into the watchdog counter if activated. */
+ if (watchdog_active(wdd))
+ dw_wdt_ping(wdd);
+
/*
* In case users set bigger timeout value than HW can support,
* kernel(watchdog_dev.c) helps to feed watchdog before
* wdd->max_hw_heartbeat_ms
*/
if (top_s * 1000 <= wdd->max_hw_heartbeat_ms)
- wdd->timeout = timeout;
+ wdd->timeout = timeout * dw_wdt->rmod;
else
wdd->timeout = top_s;
return 0;
}
+static int dw_wdt_set_pretimeout(struct watchdog_device *wdd, unsigned int req)
+{
+ struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
+
+ /*
+ * We ignore actual value of the timeout passed from user-space
+ * using it as a flag whether the pretimeout functionality is intended
+ * to be activated.
+ */
+ dw_wdt_update_mode(dw_wdt, req ? DW_WDT_RMOD_IRQ : DW_WDT_RMOD_RESET);
+ dw_wdt_set_timeout(wdd, wdd->timeout);
+
+ return 0;
+}
+
static void dw_wdt_arm_system_reset(struct dw_wdt *dw_wdt)
{
u32 val = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
- /* Disable interrupt mode; always perform system reset. */
- val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
+ /* Disable/enable interrupt mode depending on the RMOD flag. */
+ if (dw_wdt->rmod == DW_WDT_RMOD_IRQ)
+ val |= WDOG_CONTROL_REG_RESP_MODE_MASK;
+ else
+ val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
/* Enable watchdog. */
val |= WDOG_CONTROL_REG_WDT_EN_MASK;
writel(val, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
@@ -231,6 +293,7 @@ static int dw_wdt_restart(struct watchdog_device *wdd,
struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
writel(0, dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
+ dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
if (dw_wdt_is_enabled(dw_wdt))
writel(WDOG_COUNTER_RESTART_KICK_VALUE,
dw_wdt->regs + WDOG_COUNTER_RESTART_REG_OFFSET);
@@ -246,9 +309,19 @@ static int dw_wdt_restart(struct watchdog_device *wdd,
static unsigned int dw_wdt_get_timeleft(struct watchdog_device *wdd)
{
struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
+ unsigned int sec;
+ u32 val;
+
+ val = readl(dw_wdt->regs + WDOG_CURRENT_COUNT_REG_OFFSET);
+ sec = val / dw_wdt->rate;
- return readl(dw_wdt->regs + WDOG_CURRENT_COUNT_REG_OFFSET) /
- dw_wdt->rate;
+ if (dw_wdt->rmod == DW_WDT_RMOD_IRQ) {
+ val = readl(dw_wdt->regs + WDOG_INTERRUPT_STATUS_REG_OFFSET);
+ if (!val)
+ sec += wdd->pretimeout;
+ }
+
+ return sec;
}
static const struct watchdog_info dw_wdt_ident = {
@@ -257,16 +330,41 @@ static const struct watchdog_info dw_wdt_ident = {
.identity = "Synopsys DesignWare Watchdog",
};
+static const struct watchdog_info dw_wdt_pt_ident = {
+ .options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT |
+ WDIOF_PRETIMEOUT | WDIOF_MAGICCLOSE,
+ .identity = "Synopsys DesignWare Watchdog",
+};
+
static const struct watchdog_ops dw_wdt_ops = {
.owner = THIS_MODULE,
.start = dw_wdt_start,
.stop = dw_wdt_stop,
.ping = dw_wdt_ping,
.set_timeout = dw_wdt_set_timeout,
+ .set_pretimeout = dw_wdt_set_pretimeout,
.get_timeleft = dw_wdt_get_timeleft,
.restart = dw_wdt_restart,
};
+static irqreturn_t dw_wdt_irq(int irq, void *devid)
+{
+ struct dw_wdt *dw_wdt = devid;
+ u32 val;
+
+ /*
+ * We don't clear the IRQ status. It's supposed to be done by the
+ * following ping operations.
+ */
+ val = readl(dw_wdt->regs + WDOG_INTERRUPT_STATUS_REG_OFFSET);
+ if (!val)
+ return IRQ_NONE;
+
+ watchdog_notify_pretimeout(&dw_wdt->wdd);
+
+ return IRQ_HANDLED;
+}
+
#ifdef CONFIG_PM_SLEEP
static int dw_wdt_suspend(struct device *dev)
{
@@ -447,6 +545,31 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
goto out_disable_pclk;
}
+ /* Enable normal reset without pre-timeout by default. */
+ dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
+
+ /*
+ * Pre-timeout IRQ is optional, since some hardware may lack support
+ * of it. Note we must request rising-edge IRQ, since the lane is left
+ * pending either until the next watchdog kick event or up to the
+ * system reset.
+ */
+ ret = platform_get_irq_optional(pdev, 0);
+ if (ret >= 0) {
+ ret = devm_request_irq(dev, ret, dw_wdt_irq,
+ IRQF_SHARED | IRQF_TRIGGER_RISING,
+ pdev->name, dw_wdt);
+ if (ret)
+ goto out_disable_pclk;
+
+ dw_wdt->wdd.info = &dw_wdt_pt_ident;
+ } else {
+ if (ret == -EPROBE_DEFER)
+ goto out_disable_pclk;
+
+ dw_wdt->wdd.info = &dw_wdt_ident;
+ }
+
reset_control_deassert(dw_wdt->rst);
ret = dw_wdt_init_timeouts(dw_wdt, dev);
@@ -454,7 +577,6 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
goto out_disable_clk;
wdd = &dw_wdt->wdd;
- wdd->info = &dw_wdt_ident;
wdd->ops = &dw_wdt_ops;
wdd->min_timeout = dw_wdt_get_min_timeout(dw_wdt);
wdd->max_hw_heartbeat_ms = dw_wdt_get_max_timeout_ms(dw_wdt);
--
2.26.2
^ permalink raw reply related
* [PATCH v3 5/7] watchdog: dw_wdt: Support devices with asynch clocks
From: Serge Semin @ 2020-05-26 15:41 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Arnd Bergmann, Rob Herring, linux-mips, devicetree,
linux-watchdog, linux-kernel
In-Reply-To: <20200526154123.24402-1-Sergey.Semin@baikalelectronics.ru>
DW Watchdog IP core can be synthesised with asynchronous timer/APB
clocks support (WDT_ASYNC_CLK_MODE_ENABLE == 1). In this case
separate clock signals are supposed to be used to feed watchdog timer
and APB interface of the device. Currently the driver supports
the synchronous mode only. Since there is no way to determine which
mode was actually activated for device from its registers, we have to
rely on the platform device configuration data. If optional "pclk"
clock source is supplied, we consider the device working in asynchronous
mode, otherwise the driver falls back to the synchronous configuration.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v2:
- Rearrange SoBs.
---
drivers/watchdog/dw_wdt.c | 48 +++++++++++++++++++++++++++++++++++----
1 file changed, 43 insertions(+), 5 deletions(-)
diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 693c0d1fd796..efbc36872670 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -68,6 +68,7 @@ struct dw_wdt_timeout {
struct dw_wdt {
void __iomem *regs;
struct clk *clk;
+ struct clk *pclk;
unsigned long rate;
struct dw_wdt_timeout timeouts[DW_WDT_NUM_TOPS];
struct watchdog_device wdd;
@@ -274,6 +275,7 @@ static int dw_wdt_suspend(struct device *dev)
dw_wdt->control = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
dw_wdt->timeout = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
+ clk_disable_unprepare(dw_wdt->pclk);
clk_disable_unprepare(dw_wdt->clk);
return 0;
@@ -287,6 +289,12 @@ static int dw_wdt_resume(struct device *dev)
if (err)
return err;
+ err = clk_prepare_enable(dw_wdt->pclk);
+ if (err) {
+ clk_disable_unprepare(dw_wdt->clk);
+ return err;
+ }
+
writel(dw_wdt->timeout, dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
writel(dw_wdt->control, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
@@ -393,9 +401,18 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
if (IS_ERR(dw_wdt->regs))
return PTR_ERR(dw_wdt->regs);
- dw_wdt->clk = devm_clk_get(dev, NULL);
- if (IS_ERR(dw_wdt->clk))
- return PTR_ERR(dw_wdt->clk);
+ /*
+ * Try to request the watchdog dedicated timer clock source. It must
+ * be supplied if asynchronous mode is enabled. Otherwise fallback
+ * to the common timer/bus clocks configuration, in which the very
+ * first found clock supply both timer and APB signals.
+ */
+ dw_wdt->clk = devm_clk_get(dev, "tclk");
+ if (IS_ERR(dw_wdt->clk)) {
+ dw_wdt->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(dw_wdt->clk))
+ return PTR_ERR(dw_wdt->clk);
+ }
ret = clk_prepare_enable(dw_wdt->clk);
if (ret)
@@ -407,10 +424,27 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
goto out_disable_clk;
}
+ /*
+ * Request APB clock if device is configured with async clocks mode.
+ * In this case both tclk and pclk clocks are supposed to be specified.
+ * Alas we can't know for sure whether async mode was really activated,
+ * so the pclk phandle reference is left optional. If it couldn't be
+ * found we consider the device configured in synchronous clocks mode.
+ */
+ dw_wdt->pclk = devm_clk_get_optional(dev, "pclk");
+ if (IS_ERR(dw_wdt->pclk)) {
+ ret = PTR_ERR(dw_wdt->pclk);
+ goto out_disable_clk;
+ }
+
+ ret = clk_prepare_enable(dw_wdt->pclk);
+ if (ret)
+ goto out_disable_clk;
+
dw_wdt->rst = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
if (IS_ERR(dw_wdt->rst)) {
ret = PTR_ERR(dw_wdt->rst);
- goto out_disable_clk;
+ goto out_disable_pclk;
}
reset_control_deassert(dw_wdt->rst);
@@ -449,10 +483,13 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
ret = watchdog_register_device(wdd);
if (ret)
- goto out_disable_clk;
+ goto out_disable_pclk;
return 0;
+out_disable_pclk:
+ clk_disable_unprepare(dw_wdt->pclk);
+
out_disable_clk:
clk_disable_unprepare(dw_wdt->clk);
return ret;
@@ -464,6 +501,7 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
watchdog_unregister_device(&dw_wdt->wdd);
reset_control_assert(dw_wdt->rst);
+ clk_disable_unprepare(dw_wdt->pclk);
clk_disable_unprepare(dw_wdt->clk);
return 0;
--
2.26.2
^ permalink raw reply related
* [PATCH v3 4/7] watchdog: dw_wdt: Support devices with non-fixed TOP values
From: Serge Semin @ 2020-05-26 15:41 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Arnd Bergmann, Rob Herring, linux-mips, devicetree,
linux-watchdog, linux-kernel
In-Reply-To: <20200526154123.24402-1-Sergey.Semin@baikalelectronics.ru>
In case if the DW Watchdog IP core is synthesised with
WDT_USE_FIX_TOP == false, the TOP interval indexes make the device
to load a custom periods to the counter. These periods are hardwired
at the IP synthesis stage and can be within [2^8, 2^(WDT_CNT_WIDTH - 1)].
Alas their values can't be detected at runtime and must be somehow
supplied to the driver so one could properly determine the watchdog
timeout intervals. For this purpose we suggest to have a vendor-
specific dts property "snps,watchdog-tops" utilized, which would
provide an array of sixteen counter values. At device probe stage they
will be used to initialize the watchdog device timeouts determined
from the array values and current clocks source rate.
In order to have custom TOP values supported the driver must be
altered in the following way. First of all the fixed-top values
ready-to-use array must be determined for compatibility with currently
supported devices, which were synthesised with WDT_USE_FIX_TOP == true.
It will be used if either fixed TOP feature is detected being enabled or
no custom TOPs are fetched from the device dt node. Secondly at the probe
stage we must initialize an array of the watchdog timeouts corresponding
to the detected TOPs list and the reference clock rate. For generality the
procedure of initialization is designed in a way to support the TOPs array
with no limitations on the items order or value. Finally the watchdog
period search methods should be altered to support the new timeouts data
structure.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v2:
- Rearrange SoBs.
- Add "ms" suffix to the methods returning msec and convert the methods
with no "ms" suffix to return a timeout in sec.
- Make sure minimum timeout is at least 1 sec.
- Refactor the timeouts calculation procedure to retain the timeouts in
the ascending order.
- Make sure there is no integer overflow in milliseconds calculation. It
is saved in a dedicated uint field of the timeout structure.
---
drivers/watchdog/dw_wdt.c | 191 +++++++++++++++++++++++++++++++++-----
1 file changed, 167 insertions(+), 24 deletions(-)
diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index fba21de2bbad..693c0d1fd796 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -13,6 +13,8 @@
*/
#include <linux/bitops.h>
+#include <linux/limits.h>
+#include <linux/kernel.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/err.h>
@@ -34,21 +36,40 @@
#define WDOG_CURRENT_COUNT_REG_OFFSET 0x08
#define WDOG_COUNTER_RESTART_REG_OFFSET 0x0c
#define WDOG_COUNTER_RESTART_KICK_VALUE 0x76
+#define WDOG_COMP_PARAMS_1_REG_OFFSET 0xf4
+#define WDOG_COMP_PARAMS_1_USE_FIX_TOP BIT(6)
-/* The maximum TOP (timeout period) value that can be set in the watchdog. */
-#define DW_WDT_MAX_TOP 15
+/* There are sixteen TOPs (timeout periods) that can be set in the watchdog. */
+#define DW_WDT_NUM_TOPS 16
+#define DW_WDT_FIX_TOP(_idx) (1U << (16 + _idx))
#define DW_WDT_DEFAULT_SECONDS 30
+static const u32 dw_wdt_fix_tops[DW_WDT_NUM_TOPS] = {
+ DW_WDT_FIX_TOP(0), DW_WDT_FIX_TOP(1), DW_WDT_FIX_TOP(2),
+ DW_WDT_FIX_TOP(3), DW_WDT_FIX_TOP(4), DW_WDT_FIX_TOP(5),
+ DW_WDT_FIX_TOP(6), DW_WDT_FIX_TOP(7), DW_WDT_FIX_TOP(8),
+ DW_WDT_FIX_TOP(9), DW_WDT_FIX_TOP(10), DW_WDT_FIX_TOP(11),
+ DW_WDT_FIX_TOP(12), DW_WDT_FIX_TOP(13), DW_WDT_FIX_TOP(14),
+ DW_WDT_FIX_TOP(15)
+};
+
static bool nowayout = WATCHDOG_NOWAYOUT;
module_param(nowayout, bool, 0);
MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "
"(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+struct dw_wdt_timeout {
+ u32 top_val;
+ unsigned int sec;
+ unsigned int msec;
+};
+
struct dw_wdt {
void __iomem *regs;
struct clk *clk;
unsigned long rate;
+ struct dw_wdt_timeout timeouts[DW_WDT_NUM_TOPS];
struct watchdog_device wdd;
struct reset_control *rst;
/* Save/restore */
@@ -64,20 +85,66 @@ static inline int dw_wdt_is_enabled(struct dw_wdt *dw_wdt)
WDOG_CONTROL_REG_WDT_EN_MASK;
}
-static inline int dw_wdt_top_in_seconds(struct dw_wdt *dw_wdt, unsigned top)
+static unsigned int dw_wdt_find_best_top(struct dw_wdt *dw_wdt,
+ unsigned int timeout, u32 *top_val)
{
+ int idx;
+
/*
- * There are 16 possible timeout values in 0..15 where the number of
- * cycles is 2 ^ (16 + i) and the watchdog counts down.
+ * Find a TOP with timeout greater or equal to the requested number.
+ * Note we'll select a TOP with maximum timeout if the requested
+ * timeout couldn't be reached.
*/
- return (1U << (16 + top)) / dw_wdt->rate;
+ for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
+ if (dw_wdt->timeouts[idx].sec >= timeout)
+ break;
+ }
+
+ if (idx == DW_WDT_NUM_TOPS)
+ --idx;
+
+ *top_val = dw_wdt->timeouts[idx].top_val;
+
+ return dw_wdt->timeouts[idx].sec;
}
-static int dw_wdt_get_top(struct dw_wdt *dw_wdt)
+static unsigned int dw_wdt_get_min_timeout(struct dw_wdt *dw_wdt)
{
- int top = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET) & 0xF;
+ int idx;
+
+ /*
+ * We'll find a timeout greater or equal to one second anyway because
+ * the driver probe would have failed if there was none.
+ */
+ for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
+ if (dw_wdt->timeouts[idx].sec)
+ break;
+ }
- return dw_wdt_top_in_seconds(dw_wdt, top);
+ return dw_wdt->timeouts[idx].sec;
+}
+
+static unsigned int dw_wdt_get_max_timeout_ms(struct dw_wdt *dw_wdt)
+{
+ struct dw_wdt_timeout *timeout = &dw_wdt->timeouts[DW_WDT_NUM_TOPS - 1];
+ u64 msec;
+
+ msec = (u64)timeout->sec * MSEC_PER_SEC + timeout->msec;
+
+ return msec < UINT_MAX ? msec : UINT_MAX;
+}
+
+static unsigned int dw_wdt_get_timeout(struct dw_wdt *dw_wdt)
+{
+ int top_val = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET) & 0xF;
+ int idx;
+
+ for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
+ if (dw_wdt->timeouts[idx].top_val == top_val)
+ break;
+ }
+
+ return dw_wdt->timeouts[idx].sec;
}
static int dw_wdt_ping(struct watchdog_device *wdd)
@@ -93,17 +160,10 @@ static int dw_wdt_ping(struct watchdog_device *wdd)
static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
{
struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
- int i, top_val = DW_WDT_MAX_TOP;
+ unsigned int timeout;
+ u32 top_val;
- /*
- * Iterate over the timeout values until we find the closest match. We
- * always look for >=.
- */
- for (i = 0; i <= DW_WDT_MAX_TOP; ++i)
- if (dw_wdt_top_in_seconds(dw_wdt, i) >= top_s) {
- top_val = i;
- break;
- }
+ timeout = dw_wdt_find_best_top(dw_wdt, top_s, &top_val);
/*
* Set the new value in the watchdog. Some versions of dw_wdt
@@ -120,7 +180,7 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
* wdd->max_hw_heartbeat_ms
*/
if (top_s * 1000 <= wdd->max_hw_heartbeat_ms)
- wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
+ wdd->timeout = timeout;
else
wdd->timeout = top_s;
@@ -238,6 +298,86 @@ static int dw_wdt_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(dw_wdt_pm_ops, dw_wdt_suspend, dw_wdt_resume);
+/*
+ * In case if DW WDT IP core is synthesized with fixed TOP feature disabled the
+ * TOPs array can be arbitrary ordered with nearly any sixteen uint numbers
+ * depending on the system engineer imagination. The next method handles the
+ * passed TOPs array to pre-calculate the effective timeouts and to sort the
+ * TOP items out in the ascending order with respect to the timeouts.
+ */
+
+static void dw_wdt_handle_tops(struct dw_wdt *dw_wdt, const u32 *tops)
+{
+ struct dw_wdt_timeout tout, *dst;
+ int val, tidx;
+ u64 msec;
+
+ /*
+ * We walk over the passed TOPs array and calculate corresponding
+ * timeouts in seconds and milliseconds. The milliseconds granularity
+ * is needed to distinguish the TOPs with very close timeouts and to
+ * set the watchdog max heartbeat setting further.
+ */
+ for (val = 0; val < DW_WDT_NUM_TOPS; ++val) {
+ tout.top_val = val;
+ tout.sec = tops[val] / dw_wdt->rate;
+ msec = (u64)tops[val] * MSEC_PER_SEC;
+ do_div(msec, dw_wdt->rate);
+ tout.msec = msec - ((u64)tout.sec * MSEC_PER_SEC);
+
+ /*
+ * Find a suitable place for the current TOP in the timeouts
+ * array so that the list is remained in the ascending order.
+ */
+ for (tidx = 0; tidx < val; ++tidx) {
+ dst = &dw_wdt->timeouts[tidx];
+ if (tout.sec > dst->sec || (tout.sec == dst->sec &&
+ tout.msec >= dst->msec))
+ continue;
+ else
+ swap(*dst, tout);
+ }
+
+ dw_wdt->timeouts[val] = tout;
+ }
+}
+
+static int dw_wdt_init_timeouts(struct dw_wdt *dw_wdt, struct device *dev)
+{
+ u32 data, of_tops[DW_WDT_NUM_TOPS];
+ const u32 *tops;
+ int ret;
+
+ /*
+ * Retrieve custom or fixed counter values depending on the
+ * WDT_USE_FIX_TOP flag found in the component specific parameters
+ * #1 register.
+ */
+ data = readl(dw_wdt->regs + WDOG_COMP_PARAMS_1_REG_OFFSET);
+ if (data & WDOG_COMP_PARAMS_1_USE_FIX_TOP) {
+ tops = dw_wdt_fix_tops;
+ } else {
+ ret = of_property_read_variable_u32_array(dev_of_node(dev),
+ "snps,watchdog-tops", of_tops, DW_WDT_NUM_TOPS,
+ DW_WDT_NUM_TOPS);
+ if (ret < 0) {
+ dev_warn(dev, "No valid TOPs array specified\n");
+ tops = dw_wdt_fix_tops;
+ } else {
+ tops = of_tops;
+ }
+ }
+
+ /* Convert the specified TOPs into an array of watchdog timeouts. */
+ dw_wdt_handle_tops(dw_wdt, tops);
+ if (!dw_wdt->timeouts[DW_WDT_NUM_TOPS - 1].sec) {
+ dev_err(dev, "No any valid TOP detected\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int dw_wdt_drv_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -275,12 +415,15 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
reset_control_deassert(dw_wdt->rst);
+ ret = dw_wdt_init_timeouts(dw_wdt, dev);
+ if (ret)
+ goto out_disable_clk;
+
wdd = &dw_wdt->wdd;
wdd->info = &dw_wdt_ident;
wdd->ops = &dw_wdt_ops;
- wdd->min_timeout = 1;
- wdd->max_hw_heartbeat_ms =
- dw_wdt_top_in_seconds(dw_wdt, DW_WDT_MAX_TOP) * 1000;
+ wdd->min_timeout = dw_wdt_get_min_timeout(dw_wdt);
+ wdd->max_hw_heartbeat_ms = dw_wdt_get_max_timeout_ms(dw_wdt);
wdd->parent = dev;
watchdog_set_drvdata(wdd, dw_wdt);
@@ -293,7 +436,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
* devicetree.
*/
if (dw_wdt_is_enabled(dw_wdt)) {
- wdd->timeout = dw_wdt_get_top(dw_wdt);
+ wdd->timeout = dw_wdt_get_timeout(dw_wdt);
set_bit(WDOG_HW_RUNNING, &wdd->status);
} else {
wdd->timeout = DW_WDT_DEFAULT_SECONDS;
--
2.26.2
^ permalink raw reply related
* [PATCH v3 0/7] watchdog: dw_wdt: Take Baikal-T1 DW WDT peculiarities into account
From: Serge Semin @ 2020-05-26 15:41 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Serge Semin, Serge Semin, Alexey Malahov, Maxim Kaurkin,
Pavel Parkhomenko, Ramil Zaripov, Ekaterina Skachko, Vadim Vlasov,
Alexey Kolotnikov, Thomas Bogendoerfer, Arnd Bergmann,
Rob Herring, linux-mips, linux-watchdog, devicetree, linux-kernel
Merge window is upon us. Please review/merge in/whatever the rest of the
patches.
There were a few features enabled at the time of the Baikal-T1 SoC DW WDT
IP synthesis, which weren't taken into account in the DW WDT driver available
in the kernel. First of all the SoC engineers synthesized the watchdog core
with WDT_USE_FIX_TOP set to false (don't really know why, but they did).
Due to this the timer reset values weren't fixed as the driver expected
but were initialized with a pre-defined values selected by the engineers.
Secondly the driver expected that the watchdog APB bus and the timer had
synchronous reference clocks, while Baikal-T1 SoC DW WDT was created with
asynchronous ones. So the driver should enable two clock devices: APB bus
clocks and a separate timer reference clock. Finally DW Watchdog Timer is
capable of generating a pre-timeout interrupt if corresponding config is
enabled. The problem was that the pre-timeout IRQ happens when the set
timeout elapses, while the actual WDT expiration and subsequent reboot take
place in the next timeout. This makes the pre-timeout functionality
implementation a bit tricky, since in this case we would have to find a
WDT timeout twice smaller the requested timeout. All of the changes described
above are provided by the patches in this patchset.
In addition traditionally we replaced the legacy plain text-based dt-binding
file with yaml-based one and added the controller registers dump DebugFS node
to ease the driver debug procedure.
This patchset is rebased and tested on the mainline Linux kernel 5.6-rc4:
base-commit: 0e698dfa2822 ("Linux 5.7-rc4")
tag: v5.7-rc4
Changelog v2:
- Rearrange SoBs.
- Discard BE copyright header from the binding file.
- Replace "additionalProperties: false" with "unevaluatedProperties: false"
property in the binding.
- Move the APB3 clocks support declared in the dt binding file into a
dedicated patch.
- Move $ref to the root level of the "snps,watchdog-tops" property
so does the constraints.
- Make Pre-timeout IRQs support being optional.
- Add "ms" suffix to the methods returning msec and convert the methods
with no "ms" suffix to return a timeout in sec.
- Make sure minimum timeout is at least 1 sec.
- Refactor the timeouts calculation procedure to to retain the timeouts in
the ascending order.
- Make sure there is no integer overflow in milliseconds calculation. It
is saved in a dedicated uint field of the timeout structure.
- Discard timeout/pretimeout/ping/enable DebugFS nodes. Registers state
dump node is only left.
Link: https://lore.kernel.org/linux-watchdog/20200510105807.880-1-Sergey.Semin@baikalelectronics.ru/
Changelog v3:
- Add Rob's Reviewed-by tag to the DT-related patches.
- Remove items from the "snps,watchdog-tops" property and move the
minItems and maxItems constraints to the root level of it.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-watchdog@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Serge Semin (7):
dt-bindings: watchdog: Convert DW WDT binding to DT schema
dt-bindings: watchdog: dw-wdt: Support devices with asynch clocks
dt-bindings: watchdog: dw-wdt: Add watchdog TOPs array property
watchdog: dw_wdt: Support devices with non-fixed TOP values
watchdog: dw_wdt: Support devices with asynch clocks
watchdog: dw_wdt: Add pre-timeouts support
watchdog: dw_wdt: Add DebugFS files
.../devicetree/bindings/watchdog/dw_wdt.txt | 24 -
.../bindings/watchdog/snps,dw-wdt.yaml | 90 ++++
drivers/watchdog/dw_wdt.c | 437 ++++++++++++++++--
3 files changed, 494 insertions(+), 57 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt
create mode 100644 Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
--
2.26.2
^ permalink raw reply
* [PATCH v3 1/7] dt-bindings: watchdog: Convert DW WDT binding to DT schema
From: Serge Semin @ 2020-05-26 15:41 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Rob Herring
Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
Thomas Bogendoerfer, Arnd Bergmann, linux-mips, linux-watchdog,
devicetree, linux-kernel
In-Reply-To: <20200526154123.24402-1-Sergey.Semin@baikalelectronics.ru>
Modern device tree bindings are supposed to be created as YAML-files
in accordance with dt-schema. This commit replaces the DW Watchdog
legacy bare text bindings with YAML file. As before the binding states
that the corresponding dts node is supposed to have a registers
range, a watchdog timer references clock source, optional reset line and
pre-timeout interrupt.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org
---
Changelog v2:
- Rearrange SoBs.
- Discard BE copyright header.
- Replace "additionalProperties: false" with "unevaluatedProperties: false"
property.
- Discard interrupts property from the required properties list.
- Remove a label definition from the binding example.
- Move the asynchronous APB3 clock support into a dedicated patch.
---
.../devicetree/bindings/watchdog/dw_wdt.txt | 24 ---------
.../bindings/watchdog/snps,dw-wdt.yaml | 50 +++++++++++++++++++
2 files changed, 50 insertions(+), 24 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt
create mode 100644 Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
diff --git a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
deleted file mode 100644
index eb0914420c7c..000000000000
--- a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
+++ /dev/null
@@ -1,24 +0,0 @@
-Synopsys Designware Watchdog Timer
-
-Required Properties:
-
-- compatible : Should contain "snps,dw-wdt"
-- reg : Base address and size of the watchdog timer registers.
-- clocks : phandle + clock-specifier for the clock that drives the
- watchdog timer.
-
-Optional Properties:
-
-- interrupts : The interrupt used for the watchdog timeout warning.
-- resets : phandle pointing to the system reset controller with
- line index for the watchdog.
-
-Example:
-
- watchdog0: wd@ffd02000 {
- compatible = "snps,dw-wdt";
- reg = <0xffd02000 0x1000>;
- interrupts = <0 171 4>;
- clocks = <&per_base_clk>;
- resets = <&rst WDT0_RESET>;
- };
diff --git a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
new file mode 100644
index 000000000000..4f6944756ab4
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/snps,dw-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys Designware Watchdog Timer
+
+allOf:
+ - $ref: "watchdog.yaml#"
+
+maintainers:
+ - Jamie Iles <jamie@jamieiles.com>
+
+properties:
+ compatible:
+ const: snps,dw-wdt
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ description: DW Watchdog pre-timeout interrupt
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: Watchdog timer reference clock
+
+ resets:
+ description: Phandle to the DW Watchdog reset lane
+ maxItems: 1
+
+unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - clocks
+
+examples:
+ - |
+ watchdog@ffd02000 {
+ compatible = "snps,dw-wdt";
+ reg = <0xffd02000 0x1000>;
+ interrupts = <0 171 4>;
+ clocks = <&per_base_clk>;
+ resets = <&wdt_rst>;
+ };
+...
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v3 01/20] dt-bindings: arm: gic: Allow combining arm,gic-400 compatible strings
From: Rob Herring @ 2020-05-26 15:49 UTC (permalink / raw)
To: André Przywara
Cc: Geert Uytterhoeven, Liviu Dudau, Sudeep Holla, Lorenzo Pieralisi,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Mark Rutland, Marc Zyngier
In-Reply-To: <6e2cc3e9-b14f-2b50-0390-addcc82389e0@arm.com>
On Tue, May 19, 2020 at 3:20 AM André Przywara <andre.przywara@arm.com> wrote:
>
> On 19/05/2020 08:39, Geert Uytterhoeven wrote:
>
> Hi Geert,
>
>
> > On Wed, May 13, 2020 at 12:31 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >> The arm,gic-400 compatible is probably the best matching string for the
> >> GIC in most modern SoCs, but was only introduced later into the kernel.
> >> For historic reasons and to keep compatibility, some SoC DTs were thus
> >> using a combination of this name and one of the older strings, which
> >> currently the binding denies.
> >>
> >> Add a stanza to the DT binding to allow "arm,gic-400", followed by
> >> either "arm,cortex-a15-gic" or "arm,cortex-a7-gic". This fixes binding
> >> compliance for quite some SoC .dtsi files in the kernel tree.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >
> > Thanks for your patch, I was just looking into this issue ;-)
> >
> >> --- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
> >> +++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
> >> @@ -39,6 +39,12 @@ properties:
> >> - qcom,msm-8660-qgic
> >> - qcom,msm-qgic2
> >>
> >> + - items:
> >> + - const: arm,gic-400
> >> + - enum:
> >> + - arm,cortex-a15-gic
> >> + - arm,cortex-a7-gic
> >> +
> >> - items:
> >> - const: arm,arm1176jzf-devchip-gic
> >> - const: arm,arm11mp-gic
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/r9a06g032.dtsi#n177
> > has them in the other order.
> >
> > What do you think is the preferred solution: reverting the order, or dropping
> > one or the other?
>
> Reverting the order would be the right thing. Theoretically this might
> change what the drivers match against, but there should be no difference
> between those strings anyway. And certainly Linux does not care which of
> the many strings it sees.
>
> The proper order is not really obvious here, but the cortex-a{15,7}-gic
> names serve as the missing "arm,gic-v2" generic fallback string here, I
> think just for historical reasons.
Really, they probably should have been mutually exclusive given an
implementation has one or the other. Since we have both, the order in
the schema is correct given gic-400 is a superset (multi-cluster) and
the compatible came later in time.
Rob
^ permalink raw reply
* [PATCH 2/2] mmc: mmci_sdmmc: fix DMA API warning max segment size
From: Ludovic Barre @ 2020-05-26 15:51 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: srinivas.kandagatla, Maxime Coquelin, Alexandre Torgue,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
linux-stm32, Ludovic Barre
In-Reply-To: <20200526155103.12514-1-ludovic.barre@st.com>
Turning on CONFIG_DMA_API_DEBUG_SG results in the following warning:
WARNING: CPU: 1 PID: 85 at kernel/dma/debug.c:1302 debug_dma_map_sg+0x2a0/0x3cc
mmci-pl18x 58005000.sdmmc: DMA-API: mapping sg segment longer than device claims to support [len=126976] [max=65536]
dma api debug checks and compares the segment size to
dma_get_max_seg_size (dev->dma_parms->max_segment_size),
the sdmmc variant has an internal DMA and should define
its max_segment_size constraint to avoid this warning.
This Patch defines the dev->dma_parms->max_segment_size
with the constraint already set for mmc core
(host->mmc->max_seg_size).
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
drivers/mmc/host/mmci_stm32_sdmmc.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index 2965b1c062e1..51db30acf4dc 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -119,20 +119,19 @@ static void sdmmc_idma_unprep_data(struct mmci_host *host,
static int sdmmc_idma_setup(struct mmci_host *host)
{
struct sdmmc_idma *idma;
+ struct device *dev = mmc_dev(host->mmc);
- idma = devm_kzalloc(mmc_dev(host->mmc), sizeof(*idma), GFP_KERNEL);
+ idma = devm_kzalloc(dev, sizeof(*idma), GFP_KERNEL);
if (!idma)
return -ENOMEM;
host->dma_priv = idma;
if (host->variant->dma_lli) {
- idma->sg_cpu = dmam_alloc_coherent(mmc_dev(host->mmc),
- SDMMC_LLI_BUF_LEN,
+ idma->sg_cpu = dmam_alloc_coherent(dev, SDMMC_LLI_BUF_LEN,
&idma->sg_dma, GFP_KERNEL);
if (!idma->sg_cpu) {
- dev_err(mmc_dev(host->mmc),
- "Failed to alloc IDMA descriptor\n");
+ dev_err(dev, "Failed to alloc IDMA descriptor\n");
return -ENOMEM;
}
host->mmc->max_segs = SDMMC_LLI_BUF_LEN /
@@ -143,7 +142,7 @@ static int sdmmc_idma_setup(struct mmci_host *host)
host->mmc->max_seg_size = host->mmc->max_req_size;
}
- return 0;
+ return dma_set_max_seg_size(dev, host->mmc->max_seg_size);
}
static int sdmmc_idma_start(struct mmci_host *host, unsigned int *datactrl)
--
2.17.1
^ permalink raw reply related
* [PATCH 1/2] mmc: mmci_sdmmc: fix DMA API warning overlapping mappings
From: Ludovic Barre @ 2020-05-26 15:51 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: srinivas.kandagatla, Maxime Coquelin, Alexandre Torgue,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
linux-stm32, Ludovic Barre
In-Reply-To: <20200526155103.12514-1-ludovic.barre@st.com>
Turning on CONFIG_DMA_API_DEBUG_SG results in the following warning:
WARNING: CPU: 1 PID: 20 at kernel/dma/debug.c:500 add_dma_entry+0x16c/0x17c
DMA-API: exceeded 7 overlapping mappings of cacheline 0x031d2645
Modules linked in:
CPU: 1 PID: 20 Comm: kworker/1:1 Not tainted 5.5.0-rc2-00021-gdeda30999c2b-dirty #49
Hardware name: STM32 (Device Tree Support)
Workqueue: events_freezable mmc_rescan
[<c03138c0>] (unwind_backtrace) from [<c030d760>] (show_stack+0x10/0x14)
[<c030d760>] (show_stack) from [<c0f2eb28>] (dump_stack+0xc0/0xd4)
[<c0f2eb28>] (dump_stack) from [<c034a14c>] (__warn+0xd0/0xf8)
[<c034a14c>] (__warn) from [<c034a530>] (warn_slowpath_fmt+0x94/0xb8)
[<c034a530>] (warn_slowpath_fmt) from [<c03bca0c>] (add_dma_entry+0x16c/0x17c)
[<c03bca0c>] (add_dma_entry) from [<c03bdf54>] (debug_dma_map_sg+0xe4/0x3d4)
[<c03bdf54>] (debug_dma_map_sg) from [<c0d09244>] (sdmmc_idma_prep_data+0x94/0xf8)
[<c0d09244>] (sdmmc_idma_prep_data) from [<c0d05a2c>] (mmci_prep_data+0x2c/0xb0)
[<c0d05a2c>] (mmci_prep_data) from [<c0d073ec>] (mmci_start_data+0x134/0x2f0)
[<c0d073ec>] (mmci_start_data) from [<c0d078d0>] (mmci_request+0xe8/0x154)
[<c0d078d0>] (mmci_request) from [<c0cecb44>] (mmc_start_request+0x94/0xbc)
DMA api debug brings to light leaking dma-mappings, dma_map_sg and
dma_unmap_sg are not correctly balanced.
If a request is prepared, the dma_map/unmap are done in asynchronous
call pre_req (prep_data) and post_req (unprep_data). In this case
the dma-mapping is right balanced.
But if the request was not prepared, the data->host_cookie is
define to zero and the dma_map/unmap must be done in the request.
The dma_map is called by mmci_dma_start (prep_data), but there is
no dma_unmap in this case.
This patch adds dma_unmap_sg when the dma is finalized and
the data cookie is zero (request not prepared).
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
drivers/mmc/host/mmci_stm32_sdmmc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index 14f99d8aa3f0..2965b1c062e1 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -188,6 +188,9 @@ static int sdmmc_idma_start(struct mmci_host *host, unsigned int *datactrl)
static void sdmmc_idma_finalize(struct mmci_host *host, struct mmc_data *data)
{
writel_relaxed(0, host->base + MMCI_STM32_IDMACTRLR);
+
+ if (!data->host_cookie)
+ sdmmc_idma_unprep_data(host, data, 0);
}
static void mmci_sdmmc_set_clkreg(struct mmci_host *host, unsigned int desired)
--
2.17.1
^ permalink raw reply related
* [PATCH 0/2] mmc: mmci_sdmmc: fix dma api warnings
From: Ludovic Barre @ 2020-05-26 15:51 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: srinivas.kandagatla, Maxime Coquelin, Alexandre Torgue,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
linux-stm32, Ludovic Barre
This patch series fixes warnings see with DMA_API_DEBUG_SG=y
Ludovic Barre (2):
mmc: mmci_sdmmc: fix DMA API warning overlapping mappings
mmc: mmci_sdmmc: fix DMA API warning max segment size
drivers/mmc/host/mmci_stm32_sdmmc.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH v3 03/16] mfd: mfd-core: match device tree node against reg property
From: Michael Walle @ 2020-05-26 15:54 UTC (permalink / raw)
To: Lee Jones
Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski, Rob Herring,
Jean Delvare, Guenter Roeck, Thierry Reding,
Uwe Kleine-König, Wim Van Sebroeck, Shawn Guo, Li Yang,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Mark Brown,
Greg Kroah-Hartman, linux-gpio, devicetree, linux-kernel,
linux-hwmon, linux-pwm, linux-watchdog, linux-arm-kernel
In-Reply-To: <20200526072427.GC3628@dell>
Am 2020-05-26 09:24, schrieb Lee Jones:
> On Mon, 25 May 2020, Michael Walle wrote:
>
>> Am 2020-05-15 12:28, schrieb Lee Jones:
>> > On Thu, 30 Apr 2020, Michael Walle wrote:
>> >
>> > > Hi Lee,
>> > >
>> > > Am 2020-04-23 19:45, schrieb Michael Walle:
>> > > > There might be multiple children with the device tree compatible, for
>> > > > example if a MFD has multiple instances of the same function. In this
>> > > > case only the first is matched and the other children get a wrong
>> > > > of_node reference.
>> > > > Add a new option to match also against the unit address of the child
>> > > > node. Additonally, a new helper OF_MFD_CELL_REG is added.
>> > >
>> > >
>> > > Do you think this is feasible? I guess this is the biggest uncertainty
>> > > for me at the moment in this patch series.
>> >
>> > I think it sounds fine in principle. So long as it doesn't change the
>> > existing behaviour when of_reg isn't set.
>> >
>> > > > Signed-off-by: Michael Walle <michael@walle.cc>
>> > > > ---
>> > > > drivers/mfd/mfd-core.c | 29 ++++++++++++++++++++---------
>> > > > include/linux/mfd/core.h | 26 ++++++++++++++++++++------
>> > > > 2 files changed, 40 insertions(+), 15 deletions(-)
>
> [...]
>
>> > > > diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
>> > > > index d01d1299e49d..c2c0ad6b14f3 100644
>> > > > --- a/include/linux/mfd/core.h
>> > > > +++ b/include/linux/mfd/core.h
>> > > > @@ -13,8 +13,11 @@
>> > > > #include <linux/platform_device.h>
>> > > >
>> > > > #define MFD_RES_SIZE(arr) (sizeof(arr) / sizeof(struct resource))
>> > > > +#define MFD_OF_REG_VALID BIT(31)
>> >
>> > What about 64bit platforms?
>>
>> The idea was to have this as a logical number. I.e. for now you may
>> only
>> have one subdevice per unique compatible string. In fact, if you have
>> a
>> look at the ab8500.c, there are multiple "stericsson,ab8500-pwm"
>> subdevices. But there is only one DT node for all three of it. I guess
>> this works as long as you don't use phandles to reference the pwm node
>> in the device tree. Or you don't want to use device tree properties
>> per subdevice (for example the "timeout-sec" of a watchdog device).
>>
>> So to circumvent this, I thought of having the unit-address (and thus
>> the "reg" property) to differentiate between multiple subdevices. Now
>> there is one special case for me: this board management controller
>> might be upgradable and it might change internally. Thus I came up
>> with that logical numbering of subdevices. Rob doesn't seem to be a
>> fan of that, though. Therefore, having bit 31 as a valid indicator
>> leaves you with 2^31 logical devices, which should be enough ;)
>>
>> Rob proposed to have the internal offset as the unit-address. But
>> in that case I can also use devm_of_platform_populate() and don't
>> need the OF_MFD_CELL_REG; I'd just parse the reg offset in each
>> individual subdevice driver. But like I said, I wanted to keep the
>> internal offsets out of the device tree.
>
> Oh, I see what you're doing.
>
> So you're adding an arbitrary ID to the device's reg property in DT?
Yes.
> How is this not a hack?
Well IMHO this is not more or less a hack as the current of_node
handling of MFD devices, which happens to work only because there
is only one device per compatible string (or it doesn't really work,
like in the stericsson,ab8500-pwm case). The of_node is assigned
according to the compatible string, just like in my case, only that
I have two subdevices with the same compatible string.
> Why don't you use the full address for identification?
Like I said, in the long term I would like to have support for
different versions of the board management controller without having
to change the device tree and have device tree bindings for the
subdevices at the same time. But it seems, that this is not possible
and I guess I have to bite the bullet and may need to provide another
device tree if the controller might be updated.
-michael
^ permalink raw reply
* Re: [PATCH v3 03/16] mfd: mfd-core: match device tree node against reg property
From: Andy Shevchenko @ 2020-05-26 16:03 UTC (permalink / raw)
To: Michael Walle
Cc: Lee Jones, Linus Walleij, Bartosz Golaszewski, Rob Herring,
Jean Delvare, Guenter Roeck, Thierry Reding,
Uwe Kleine-König, Wim Van Sebroeck, Shawn Guo, Li Yang,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Mark Brown,
Greg Kroah-Hartman, linux-gpio, devicetree, linux-kernel,
linux-hwmon, linux-pwm, linux-watchdog, linux-arm-kernel
In-Reply-To: <f5704ce5a3e280f63c81fe35efb08234@walle.cc>
On Tue, May 26, 2020 at 05:54:38PM +0200, Michael Walle wrote:
> Am 2020-05-26 09:24, schrieb Lee Jones:
...
> Like I said, in the long term I would like to have support for
> different versions of the board management controller
> without having to change the device tree and have device tree bindings for the
> subdevices at the same time.
But isn't device tree to describe *very specific platform* rather than *class
of platforms*?
> But it seems, that this is not possible
> and I guess I have to bite the bullet and may need to provide another
> device tree if the controller might be updated.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 07/10] media: i2c: imx290: Add RAW12 mode support
From: Dave Stevenson @ 2020-05-26 16:05 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Mauro Carvalho Chehab, Sakari Ailus, manivannan.sadhasivam,
Linux Media Mailing List, LKML, devicetree, linux-arm-kernel,
c.barrett, a.brela, Peter Griffin
In-Reply-To: <20200524192505.20682-8-andrey.konovalov@linaro.org>
Hi Andrey
Thanks for the patch.
On Sun, 24 May 2020 at 20:26, Andrey Konovalov
<andrey.konovalov@linaro.org> wrote:
>
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> IMX290 is capable of outputting frames in both Raw Bayer (packed) 10 and
> 12 bit formats. Since the driver already supports RAW10 mode, let's add
> the missing RAW12 mode as well.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
> ---
> drivers/media/i2c/imx290.c | 36 +++++++++++++++++++++++++++++++++---
> 1 file changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index 162c345fffac..6e70ff22bc5f 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -71,6 +71,7 @@ struct imx290 {
> struct clk *xclk;
> struct regmap *regmap;
> u8 nlanes;
> + u8 bpp;
>
> struct v4l2_subdev sd;
> struct v4l2_fwnode_endpoint ep;
> @@ -90,10 +91,12 @@ struct imx290 {
>
> struct imx290_pixfmt {
> u32 code;
> + u8 bpp;
> };
>
> static const struct imx290_pixfmt imx290_formats[] = {
> - { MEDIA_BUS_FMT_SRGGB10_1X10 },
> + { MEDIA_BUS_FMT_SRGGB10_1X10, 10 },
> + { MEDIA_BUS_FMT_SRGGB12_1X12, 12 },
> };
>
> static const struct regmap_config imx290_regmap_config = {
> @@ -261,6 +264,18 @@ static const struct imx290_regval imx290_10bit_settings[] = {
> { 0x300b, 0x00},
> };
>
> +static const struct imx290_regval imx290_12bit_settings[] = {
> + { 0x3005, 0x01 },
> + { 0x3046, 0x01 },
> + { 0x3129, 0x00 },
> + { 0x317c, 0x00 },
> + { 0x31ec, 0x0e },
> + { 0x3441, 0x0c },
> + { 0x3442, 0x0c },
> + { 0x300a, 0xf0 },
> + { 0x300b, 0x00 },
> +};
> +
> /* supported link frequencies */
> static const s64 imx290_link_freq_2lanes[] = {
> 891000000, /* 1920x1080 - 2 lane */
> @@ -421,7 +436,12 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
> } else {
> imx290_write_reg(imx290, IMX290_PGCTRL, 0x00);
> msleep(10);
> - imx290_write_reg(imx290, IMX290_BLKLEVEL_LOW, 0x3c);
> + if (imx290->bpp == 10)
> + imx290_write_reg(imx290, IMX290_BLKLEVEL_LOW,
> + 0x3c);
> + else /* 12 bits per pixel */
> + imx290_write_reg(imx290, IMX290_BLKLEVEL_LOW,
> + 0xf0);
> imx290_write_reg(imx290, IMX290_BLKLEVEL_HIGH, 0x00);
> }
> break;
> @@ -496,7 +516,7 @@ static u64 imx290_calc_pixel_rate(struct imx290 *imx290)
> u8 nlanes = imx290->nlanes;
>
> /* pixel rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
> - return (link_freq * 2 * nlanes / 10);
> + return (link_freq * 2 * nlanes / imx290->bpp);
This doesn't link on a 32bit system as it's a 64bit divide:
ERROR: "__aeabi_ldivmod" [drivers/media/i2c/imx290.ko] undefined!
It ought to be using do_div().
Admittedly it didn't compile before as you still had a s64 divide by
10, but I hadn't tried that :-)
Dave
> }
>
> static int imx290_set_fmt(struct v4l2_subdev *sd,
> @@ -533,6 +553,7 @@ static int imx290_set_fmt(struct v4l2_subdev *sd,
> } else {
> format = &imx290->current_format;
> imx290->current_mode = mode;
> + imx290->bpp = imx290_formats[i].bpp;
>
> if (imx290->link_freq)
> __v4l2_ctrl_s_ctrl(imx290->link_freq,
> @@ -577,6 +598,15 @@ static int imx290_write_current_format(struct imx290 *imx290)
> return ret;
> }
> break;
> + case MEDIA_BUS_FMT_SRGGB12_1X12:
> + ret = imx290_set_register_array(imx290, imx290_12bit_settings,
> + ARRAY_SIZE(
> + imx290_12bit_settings));
> + if (ret < 0) {
> + dev_err(imx290->dev, "Could not set format registers\n");
> + return ret;
> + }
> + break;
> default:
> dev_err(imx290->dev, "Unknown pixel format\n");
> return -EINVAL;
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v3 3/6] dt-bindings: memory: Add Baikal-T1 L2-cache Control Block binding
From: Rob Herring @ 2020-05-26 16:09 UTC (permalink / raw)
To: Serge Semin
Cc: Thomas Bogendoerfer, linux-kernel, Rob Herring, Serge Semin,
Alexey Malahov, Greg Kroah-Hartman, linux-mips, Arnd Bergmann,
Olof Johansson, soc, Paul Burton, devicetree
In-Reply-To: <20200526125928.17096-4-Sergey.Semin@baikalelectronics.ru>
On Tue, 26 May 2020 15:59:25 +0300, Serge Semin wrote:
> There is a single register provided by the SoC system controller,
> which can be used to tune the L2-cache RAM up. It only provides a way
> to change the L2-RAM access latencies. So aside from "be,bt1-l2-ctl"
> compatible string the device node can be optionally equipped with the
> properties of Tag/Data/WS latencies.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: linux-mips@vger.kernel.org
> Cc: soc@kernel.org
>
> ---
>
> Changelog v2:
> - Move driver to the memory subsystem.
> - Use dual GPL/BSD license.
> - Use single lined copyright header.
> - Move "allOf" restrictions to the root level of the properties.
> - Discard syscon compatible string and reg property.
> - The DT node is supposed to be a child of the Baikal-T1 system controller
> node.
>
> Changelog v3:
> - Get the reg property back even though the driver is using the parental
> syscon regmap.
> - The DT schema will live separately from the system controller, but the
> corresponding sub-node of the later DT schema will $ref this one.
> - Set non-default latencies in the example.
> ---
> .../memory-controllers/baikal,bt1-l2-ctl.yaml | 63 +++++++++++++++++++
> 1 file changed, 63 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
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