Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v14 10/16] vfio/type1: vfio_find_dma accepting a type argument
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>

In our RB-tree we get prepared to insert slots of different types
(USER and RESERVED). It becomes useful to be able to search for dma
slots of a specific type or any type.

This patch introduces vfio_find_dma_from_node which starts the
search from a given node and stops on the first node that matches
the @start and @size parameters. If this node also matches the
@type parameter, the node is returned else NULL is returned.

At the moment we only have USER SLOTS so the type will always match.

In a separate patch, this function will be enhanced to pursue the
search recursively in case a node with a different type is
encountered.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v13 -> v14:
- remove top_node variable
---
 drivers/vfio/vfio_iommu_type1.c | 52 +++++++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index a9f8b93..1bd16ff 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -94,25 +94,55 @@ struct vfio_group {
  * into DMA'ble space using the IOMMU
  */
 
-static struct vfio_dma *vfio_find_dma(struct vfio_iommu *iommu,
-				      dma_addr_t start, size_t size)
+/**
+ * vfio_find_dma_from_node: looks for a dma slot intersecting a window
+ * from a given rb tree node
+ * @top: top rb tree node where the search starts (including this node)
+ * @start: window start
+ * @size: window size
+ * @type: window type
+ */
+static struct vfio_dma *vfio_find_dma_from_node(struct rb_node *top,
+						dma_addr_t start, size_t size,
+						enum vfio_iova_type type)
 {
-	struct rb_node *node = iommu->dma_list.rb_node;
+	struct rb_node *node = top;
+	struct vfio_dma *dma;
 
 	while (node) {
-		struct vfio_dma *dma = rb_entry(node, struct vfio_dma, node);
-
+		dma = rb_entry(node, struct vfio_dma, node);
 		if (start + size <= dma->iova)
 			node = node->rb_left;
 		else if (start >= dma->iova + dma->size)
 			node = node->rb_right;
 		else
-			return dma;
+			break;
 	}
+	if (!node)
+		return NULL;
+
+	/* a dma slot intersects our window, check the type also matches */
+	if (type == VFIO_IOVA_ANY || dma->type == type)
+		return dma;
 
 	return NULL;
 }
 
+/**
+ * vfio_find_dma: find a dma slot intersecting a given window
+ * @iommu: vfio iommu handle
+ * @start: window base iova
+ * @size: window size
+ * @type: window type
+ */
+static struct vfio_dma *vfio_find_dma(struct vfio_iommu *iommu,
+				      dma_addr_t start, size_t size,
+				      enum vfio_iova_type type)
+{
+	return vfio_find_dma_from_node(iommu->dma_list.rb_node,
+				       start, size, type);
+}
+
 static void vfio_link_dma(struct vfio_iommu *iommu, struct vfio_dma *new)
 {
 	struct rb_node **link = &iommu->dma_list.rb_node, *parent = NULL;
@@ -484,19 +514,21 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
 	 * mappings within the range.
 	 */
 	if (iommu->v2) {
-		dma = vfio_find_dma(iommu, unmap->iova, 0);
+		dma = vfio_find_dma(iommu, unmap->iova, 0, VFIO_IOVA_USER);
 		if (dma && dma->iova != unmap->iova) {
 			ret = -EINVAL;
 			goto unlock;
 		}
-		dma = vfio_find_dma(iommu, unmap->iova + unmap->size - 1, 0);
+		dma = vfio_find_dma(iommu, unmap->iova + unmap->size - 1, 0,
+				    VFIO_IOVA_USER);
 		if (dma && dma->iova + dma->size != unmap->iova + unmap->size) {
 			ret = -EINVAL;
 			goto unlock;
 		}
 	}
 
-	while ((dma = vfio_find_dma(iommu, unmap->iova, unmap->size))) {
+	while ((dma = vfio_find_dma(iommu, unmap->iova, unmap->size,
+				    VFIO_IOVA_USER))) {
 		if (!iommu->v2 && unmap->iova > dma->iova)
 			break;
 		unmapped += dma->size;
@@ -600,7 +632,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
 
 	mutex_lock(&iommu->lock);
 
-	if (vfio_find_dma(iommu, iova, size)) {
+	if (vfio_find_dma(iommu, iova, size, VFIO_IOVA_ANY)) {
 		mutex_unlock(&iommu->lock);
 		return -EEXIST;
 	}
-- 
1.9.1

^ permalink raw reply related

* [PATCH v14 09/16] vfio: Introduce a vfio_dma type field
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>

We introduce a vfio_dma type since we will need to discriminate
different types of dma slots:
- VFIO_IOVA_USER: IOVA region used to map user vaddr
- VFIO_IOVA_RESERVED_MSI: IOVA region reserved to map MSI doorbells

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Acked-by: Alex Williamson <alex.williamson@redhat.com>

---
v9 -> v10:
- renamed VFIO_IOVA_RESERVED into VFIO_IOVA_RESERVED_MSI
- explicitly set type to VFIO_IOVA_USER on dma_map

v6 -> v7:
- add VFIO_IOVA_ANY
- do not introduce yet any VFIO_IOVA_RESERVED handling
---
 drivers/vfio/vfio_iommu_type1.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 2ba1942..a9f8b93 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -53,6 +53,12 @@ module_param_named(disable_hugepages,
 MODULE_PARM_DESC(disable_hugepages,
 		 "Disable VFIO IOMMU support for IOMMU hugepages.");
 
+enum vfio_iova_type {
+	VFIO_IOVA_USER = 0,	/* standard IOVA used to map user vaddr */
+	VFIO_IOVA_RESERVED_MSI,	/* reserved to map MSI doorbells */
+	VFIO_IOVA_ANY,		/* matches any IOVA type */
+};
+
 struct vfio_iommu {
 	struct list_head	domain_list;
 	struct mutex		lock;
@@ -75,6 +81,7 @@ struct vfio_dma {
 	unsigned long		vaddr;		/* Process virtual addr */
 	size_t			size;		/* Map size (bytes) */
 	int			prot;		/* IOMMU_READ/WRITE */
+	enum vfio_iova_type	type;		/* type of IOVA */
 };
 
 struct vfio_group {
@@ -607,6 +614,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
 	dma->iova = iova;
 	dma->vaddr = vaddr;
 	dma->prot = prot;
+	dma->type = VFIO_IOVA_USER;
 
 	/* Insert zero-sized and grow as we map chunks of it */
 	vfio_link_dma(iommu, dma);
-- 
1.9.1

^ permalink raw reply related

* [PATCH v14 08/16] irqchip/gicv3-its: Register the MSI doorbell
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>

This patch registers the ITS global doorbell. Registered information
are needed to set up the KVM passthrough use case.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---
v13 -> v14:
- use iommu_msi_doorbell_alloc/free

v12 -> v13:
- use new doorbell registration prototype

v11 -> v12:
- use new irq_get_msi_doorbell_info name
- simplify error handling

v10 -> v11:
- adapt to new doorbell registration API and implement msi_doorbell_info
---
 drivers/irqchip/irq-gic-v3-its.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 98ff669..b42e006 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -86,6 +86,7 @@ struct its_node {
 	u32			ite_size;
 	u32			device_ids;
 	int			numa_node;
+	struct iommu_msi_doorbell_info	*doorbell_info;
 };
 
 #define ITS_ITT_ALIGN		SZ_256
@@ -1717,6 +1718,7 @@ static int __init its_probe(struct device_node *node,
 
 	if (of_property_read_bool(node, "msi-controller")) {
 		struct msi_domain_info *info;
+		phys_addr_t translater;
 
 		info = kzalloc(sizeof(*info), GFP_KERNEL);
 		if (!info) {
@@ -1724,10 +1726,21 @@ static int __init its_probe(struct device_node *node,
 			goto out_free_tables;
 		}
 
+		translater = its->phys_base + GITS_TRANSLATER;
+		its->doorbell_info =
+			iommu_msi_doorbell_alloc(translater, sizeof(u32), true);
+
+		if (IS_ERR(its->doorbell_info))  {
+			kfree(info);
+			goto out_free_tables;
+		}
+
+
 		inner_domain = irq_domain_add_tree(node, &its_domain_ops, its);
 		if (!inner_domain) {
 			err = -ENOMEM;
 			kfree(info);
+			iommu_msi_doorbell_free(its->doorbell_info);
 			goto out_free_tables;
 		}
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v14 07/16] irqchip/gic-v2m: Register the MSI doorbell
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>

Register the GIC V2M global doorbell. The registered information
are used to set up the KVM passthrough use case.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---
v13 -> v14:
- use iommu_msi_doorbell_alloc/free

v12 -> v13:
- use new msi doorbell registration prototype
- remove iommu protection attributes
- add unregistration in teardown

v11 -> v12:
- use irq_get_msi_doorbell_info new name
- simplify error handling

v10 -> v11:
- use the new registration API and re-implement the msi_doorbell_info
  ops

v9 -> v10:
- introduce the registration concept in place of msi_doorbell_info
  callback

v8 -> v9:
- use global_doorbell instead of percpu_doorbells

v7 -> v8:
- gicv2m_msi_doorbell_info does not return a pointer to const
- remove spurious !v2m check
- add IOMMU_MMIO flag

v7: creation
---
 drivers/irqchip/irq-gic-v2m.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index 863e073..33acfe0 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -70,6 +70,7 @@ struct v2m_data {
 	u32 spi_offset;		/* offset to be subtracted from SPI number */
 	unsigned long *bm;	/* MSI vector bitmap */
 	u32 flags;		/* v2m flags for specific implementation */
+	struct iommu_msi_doorbell_info *doorbell_info; /* MSI doorbell */
 };
 
 static void gicv2m_mask_msi_irq(struct irq_data *d)
@@ -254,6 +255,7 @@ static void gicv2m_teardown(void)
 	struct v2m_data *v2m, *tmp;
 
 	list_for_each_entry_safe(v2m, tmp, &v2m_nodes, entry) {
+		iommu_msi_doorbell_free(v2m->doorbell_info);
 		list_del(&v2m->entry);
 		kfree(v2m->bm);
 		iounmap(v2m->base);
@@ -370,12 +372,18 @@ static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
 		goto err_iounmap;
 	}
 
+	v2m->doorbell_info = iommu_msi_doorbell_alloc(v2m->res.start,
+						      sizeof(u32), false);
+	if (IS_ERR(v2m->doorbell_info))
+		goto err_free_bm;
+
 	list_add_tail(&v2m->entry, &v2m_nodes);
 
 	pr_info("range%pR, SPI[%d:%d]\n", res,
 		v2m->spi_start, (v2m->spi_start + v2m->nr_spis - 1));
 	return 0;
-
+err_free_bm:
+	kfree(v2m->bm);
 err_iounmap:
 	iounmap(v2m->base);
 err_free_v2m:
-- 
1.9.1

^ permalink raw reply related

* [PATCH v14 06/16] iommu/arm-smmu: Implement domain_get_attr for DOMAIN_ATTR_MSI_RESV
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>

ARM smmu and smmu-v3 translate MSI transactions so their driver
are must implement domain_get_attr for DOMAIN_ATTR_MSI_RESV.

This allows to retrieve the size and alignment requirements of
the MSI reserved IOVA window.

Also IOMMU_DMA gets selected since it exposes the API to map the
MSIs.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 drivers/iommu/Kconfig       | 4 ++--
 drivers/iommu/arm-smmu-v3.c | 7 +++++++
 drivers/iommu/arm-smmu.c    | 7 +++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 8ee54d7..f5e5e4b 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -297,7 +297,7 @@ config SPAPR_TCE_IOMMU
 config ARM_SMMU
 	bool "ARM Ltd. System MMU (SMMU) Support"
 	depends on (ARM64 || ARM) && MMU
-	select IOMMU_API
+	select IOMMU_DMA
 	select IOMMU_IO_PGTABLE_LPAE
 	select ARM_DMA_USE_IOMMU if ARM
 	help
@@ -310,7 +310,7 @@ config ARM_SMMU
 config ARM_SMMU_V3
 	bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
 	depends on ARM64
-	select IOMMU_API
+	select IOMMU_DMA
 	select IOMMU_IO_PGTABLE_LPAE
 	select GENERIC_MSI_IRQ_DOMAIN
 	help
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 15c01c3..572cad8 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1559,6 +1559,9 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain)
 	if (ret < 0)
 		free_io_pgtable_ops(pgtbl_ops);
 
+	if (domain->type == IOMMU_DOMAIN_UNMANAGED)
+		iommu_calc_msi_resv(domain);
+
 	return ret;
 }
 
@@ -1840,6 +1843,10 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
 	case DOMAIN_ATTR_NESTING:
 		*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
 		return 0;
+	case DOMAIN_ATTR_MSI_RESV:
+		*(struct iommu_domain_msi_resv *)data =
+			smmu_domain->domain.msi_resv;
+		return 0;
 	default:
 		return -ENODEV;
 	}
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index ac4aab9..ae20b9c 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -943,6 +943,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	domain->geometry.aperture_end = (1UL << ias) - 1;
 	domain->geometry.force_aperture = true;
 
+	if (domain->type == IOMMU_DOMAIN_UNMANAGED)
+		iommu_calc_msi_resv(domain);
+
 	/* Initialise the context bank with our page table cfg */
 	arm_smmu_init_context_bank(smmu_domain, &pgtbl_cfg);
 
@@ -1486,6 +1489,10 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
 	case DOMAIN_ATTR_NESTING:
 		*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
 		return 0;
+	case DOMAIN_ATTR_MSI_RESV:
+		*(struct iommu_domain_msi_resv *)data =
+			smmu_domain->domain.msi_resv;
+		return 0;
 	default:
 		return -ENODEV;
 	}
-- 
1.9.1

^ permalink raw reply related

* [PATCH v14 05/16] iommu/dma: Introduce iommu_calc_msi_resv
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>

iommu_calc_msi_resv() sum up the number of iommu pages of the lowest
order supported by the iommu domain requested to map all the registered
doorbells. This function will allow to dimension the intermediate
physical address (IPA) aperture requested to map the MSI doorbells.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v13 -> v14
- name and proto changed, moved to dma-iommu
---
 drivers/iommu/dma-iommu.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/dma-iommu.h | 11 +++++++-
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d8a7d86..3a4b73b 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -830,3 +830,73 @@ bool iommu_msi_doorbell_safe(void)
 	return !nb_unsafe_doorbells;
 }
 EXPORT_SYMBOL_GPL(iommu_msi_doorbell_safe);
+
+/**
+ * calc_region_reqs - compute the number of pages requested to map a region
+ *
+ * @addr: physical base address of the region
+ * @size: size of the region
+ * @order: the page order
+ *
+ * Return: the number of requested pages to map this region
+ */
+static int calc_region_reqs(phys_addr_t addr, size_t size, unsigned int order)
+{
+	phys_addr_t offset, granule;
+	unsigned int nb_pages;
+
+	granule = (uint64_t)(1 << order);
+	offset = addr & (granule - 1);
+	size = ALIGN(size + offset, granule);
+	nb_pages = size >> order;
+
+	return nb_pages;
+}
+
+/**
+ * calc_dbinfo_reqs - compute the number of pages requested to map a given
+ * MSI doorbell
+ *
+ * @dbi: doorbell info descriptor
+ * @order: page order
+ *
+ * Return: the number of requested pages to map this doorbell
+ */
+static int calc_dbinfo_reqs(struct iommu_msi_doorbell_info *dbi,
+			    unsigned int order)
+{
+	int ret = 0;
+
+	if (!dbi->doorbell_is_percpu) {
+		ret = calc_region_reqs(dbi->global_doorbell, dbi->size, order);
+	} else {
+		phys_addr_t __percpu *pbase;
+		int cpu;
+
+		for_each_possible_cpu(cpu) {
+			pbase = per_cpu_ptr(dbi->percpu_doorbells, cpu);
+			ret += calc_region_reqs(*pbase, dbi->size, order);
+		}
+	}
+	return ret;
+}
+
+int iommu_calc_msi_resv(struct iommu_domain *domain)
+{
+	unsigned long order = __ffs(domain->pgsize_bitmap);
+	struct iommu_msi_doorbell *db;
+	phys_addr_t granule;
+	int size = 0;
+
+	mutex_lock(&iommu_msi_doorbell_mutex);
+	list_for_each_entry(db, &iommu_msi_doorbell_list, next)
+		size += calc_dbinfo_reqs(&db->info, order);
+
+	mutex_unlock(&iommu_msi_doorbell_mutex);
+
+	granule = (uint64_t)(1 << order);
+	domain->msi_resv.size = size * granule;
+	domain->msi_resv.alignment = granule;
+
+	return 0;
+}
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 9640a27..95875c8 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -97,6 +97,16 @@ void iommu_msi_doorbell_free(struct iommu_msi_doorbell_info *db);
  */
 bool iommu_msi_doorbell_safe(void);
 
+/**
+ * iommu_calc_msi_resv - compute the number of pages of the lowest order
+ * supported by @domain, requested to map all the registered doorbells.
+ *
+ * @domain: iommu_domain
+ * @msi_resv: MSI reserved window requirements
+ *
+ */
+int iommu_calc_msi_resv(struct iommu_domain *domain);
+
 #else
 
 struct iommu_domain;
@@ -139,7 +149,6 @@ static inline bool iommu_msi_doorbell_safe(void)
 {
 	return false;
 }
-
 #endif	/* CONFIG_IOMMU_DMA */
 #endif	/* __KERNEL__ */
 #endif	/* __DMA_IOMMU_H */
-- 
1.9.1

^ permalink raw reply related

* [PATCH v14 04/16] iommu/dma: MSI doorbell alloc/free
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>

We introduce the capability to (un)register MSI doorbells.

A doorbell region is characterized by its physical address base, size,
and whether it its safe (ie. it implements IRQ remapping). A doorbell
can be per-cpu or global. We currently only care about global doorbells.

A function returns whether all registered doorbells are safe.

MSI controllers likely to work along with IOMMU that translate MSI
transaction must register their doorbells to allow device assignment
with MSI support.  Otherwise the MSI transactions will cause IOMMU faults.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v13 -> v14:
- previously in msi-doorbell.h/c
---
 drivers/iommu/dma-iommu.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/dma-iommu.h | 41 ++++++++++++++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d45f9a0..d8a7d86 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -43,6 +43,38 @@ struct iommu_dma_cookie {
 	spinlock_t		msi_lock;
 };
 
+/**
+ * struct iommu_msi_doorbell_info - MSI doorbell region descriptor
+ * @percpu_doorbells: per cpu doorbell base address
+ * @global_doorbell: base address of the doorbell
+ * @doorbell_is_percpu: is the doorbell per cpu or global?
+ * @safe: true if irq remapping is implemented
+ * @size: size of the doorbell
+ */
+struct iommu_msi_doorbell_info {
+	union {
+		phys_addr_t __percpu    *percpu_doorbells;
+		phys_addr_t             global_doorbell;
+	};
+	bool    doorbell_is_percpu;
+	bool    safe;
+	size_t  size;
+};
+
+struct iommu_msi_doorbell {
+	struct iommu_msi_doorbell_info	info;
+	struct list_head		next;
+};
+
+/* list of registered MSI doorbells */
+static LIST_HEAD(iommu_msi_doorbell_list);
+
+/* counts the number of unsafe registered doorbells */
+static uint nb_unsafe_doorbells;
+
+/* protects the list and nb_unsafe_doorbells */
+static DEFINE_MUTEX(iommu_msi_doorbell_mutex);
+
 static inline struct iova_domain *cookie_iovad(struct iommu_domain *domain)
 {
 	return &((struct iommu_dma_cookie *)domain->iova_cookie)->iovad;
@@ -755,3 +787,46 @@ int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
 	return 0;
 }
 EXPORT_SYMBOL(iommu_get_dma_msi_region_cookie);
+
+struct iommu_msi_doorbell_info *
+iommu_msi_doorbell_alloc(phys_addr_t base, size_t size, bool safe)
+{
+	struct iommu_msi_doorbell *db;
+
+	db = kzalloc(sizeof(*db), GFP_KERNEL);
+	if (!db)
+		return ERR_PTR(-ENOMEM);
+
+	db->info.global_doorbell = base;
+	db->info.size = size;
+	db->info.safe = safe;
+
+	mutex_lock(&iommu_msi_doorbell_mutex);
+	list_add(&db->next, &iommu_msi_doorbell_list);
+	if (!db->info.safe)
+		nb_unsafe_doorbells++;
+	mutex_unlock(&iommu_msi_doorbell_mutex);
+	return &db->info;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_doorbell_alloc);
+
+void iommu_msi_doorbell_free(struct iommu_msi_doorbell_info *dbinfo)
+{
+	struct iommu_msi_doorbell *db;
+
+	db = container_of(dbinfo, struct iommu_msi_doorbell, info);
+
+	mutex_lock(&iommu_msi_doorbell_mutex);
+	list_del(&db->next);
+	if (!db->info.safe)
+		nb_unsafe_doorbells--;
+	mutex_unlock(&iommu_msi_doorbell_mutex);
+	kfree(db);
+}
+EXPORT_SYMBOL_GPL(iommu_msi_doorbell_free);
+
+bool iommu_msi_doorbell_safe(void)
+{
+	return !nb_unsafe_doorbells;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_doorbell_safe);
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 05ab5b4..9640a27 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -19,6 +19,8 @@
 #ifdef __KERNEL__
 #include <asm/errno.h>
 
+struct iommu_msi_doorbell_info;
+
 #ifdef CONFIG_IOMMU_DMA
 #include <linux/iommu.h>
 #include <linux/msi.h>
@@ -70,6 +72,31 @@ void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
 int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
 				    dma_addr_t base, u64 size);
 
+/**
+ * iommu_msi_doorbell_alloc - allocate a global doorbell
+ * @base: physical base address of the doorbell
+ * @size: size of the doorbell
+ * @safe: true is irq_remapping implemented for this doorbell
+ *
+ * Return: the newly allocated doorbell info or a pointer converted error
+ */
+struct iommu_msi_doorbell_info *
+iommu_msi_doorbell_alloc(phys_addr_t base, size_t size, bool safe);
+
+/**
+ * iommu_msi_doorbell_free - free a global doorbell
+ * @db: doorbell info to free
+ */
+void iommu_msi_doorbell_free(struct iommu_msi_doorbell_info *db);
+
+/**
+ * iommu_msi_doorbell_safe - return whether all registered doorbells are safe
+ *
+ * Safe doorbells are those which implement irq remapping
+ * Return: true if all doorbells are safe, false otherwise
+ */
+bool iommu_msi_doorbell_safe(void);
+
 #else
 
 struct iommu_domain;
@@ -99,6 +126,20 @@ static inline int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
 	return -ENODEV;
 }
 
+static inline struct iommu_msi_doorbell_info *
+iommu_msi_doorbell_alloc(phys_addr_t base, size_t size, bool safe)
+{
+	return NULL;
+}
+
+static inline void
+iommu_msi_doorbell_free(struct msi_doorbell_info *db) {}
+
+static inline bool iommu_msi_doorbell_safe(void)
+{
+	return false;
+}
+
 #endif	/* CONFIG_IOMMU_DMA */
 #endif	/* __KERNEL__ */
 #endif	/* __DMA_IOMMU_H */
-- 
1.9.1

^ permalink raw reply related

* [PATCH v14 03/16] iommu/dma: Allow MSI-only cookies
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>

From: Robin Murphy <robin.murphy@arm.com>

IOMMU domain users such as VFIO face a similar problem to DMA API ops
with regard to mapping MSI messages in systems where the MSI write is
subject to IOMMU translation. With the relevant infrastructure now in
place for managed DMA domains, it's actually really simple for other
users to piggyback off that and reap the benefits without giving up
their own IOVA management, and without having to reinvent their own
wheel in the MSI layer.

Allow such users to opt into automatic MSI remapping by dedicating a
region of their IOVA space to a managed cookie.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v13 -> v14:
- restore reserve_iova for iova >= base + size

v1 -> v13 incorpration:
- compared to Robin's version
- add NULL last param to iommu_dma_init_domain
- set the msi_geometry aperture
- I removed
  if (base < U64_MAX - size)
     reserve_iova(iovad, iova_pfn(iovad, base + size), ULONG_MAX);
  don't get why we would reserve something out of the scope of the iova domain?
  what do I miss?
---
 drivers/iommu/dma-iommu.c | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/dma-iommu.h |  9 +++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index c5ab866..d45f9a0 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -716,3 +716,42 @@ void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
 		msg->address_lo += lower_32_bits(msi_page->iova);
 	}
 }
+
+/**
+ * iommu_get_dma_msi_region_cookie - Configure a domain for MSI remapping only
+ * @domain: IOMMU domain to prepare
+ * @base: Base address of IOVA region to use as the MSI remapping aperture
+ * @size: Size of the desired MSI aperture
+ *
+ * Users who manage their own IOVA allocation and do not want DMA API support,
+ * but would still like to take advantage of automatic MSI remapping, can use
+ * this to initialise their own domain appropriately.
+ */
+int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
+			       dma_addr_t base, u64 size)
+{
+	struct iommu_dma_cookie *cookie;
+	struct iova_domain *iovad;
+	int ret;
+
+	if (domain->type == IOMMU_DOMAIN_DMA)
+		return -EINVAL;
+
+	ret = iommu_get_dma_cookie(domain);
+	if (ret)
+		return ret;
+
+	ret = iommu_dma_init_domain(domain, base, size, NULL);
+	if (ret) {
+		iommu_put_dma_cookie(domain);
+		return ret;
+	}
+
+	cookie = domain->iova_cookie;
+	iovad = &cookie->iovad;
+	if (base < U64_MAX - size)
+		reserve_iova(iovad, iova_pfn(iovad, base + size), ULONG_MAX);
+
+	return 0;
+}
+EXPORT_SYMBOL(iommu_get_dma_msi_region_cookie);
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 32c5890..05ab5b4 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -67,6 +67,9 @@ int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
 /* The DMA API isn't _quite_ the whole story, though... */
 void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
 
+int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
+				    dma_addr_t base, u64 size);
+
 #else
 
 struct iommu_domain;
@@ -90,6 +93,12 @@ static inline void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
 {
 }
 
+static inline int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
+						  dma_addr_t base, u64 size)
+{
+	return -ENODEV;
+}
+
 #endif	/* CONFIG_IOMMU_DMA */
 #endif	/* __KERNEL__ */
 #endif	/* __DMA_IOMMU_H */
-- 
1.9.1

^ permalink raw reply related

* [PATCH v14 02/16] iommu: Introduce DOMAIN_ATTR_MSI_RESV
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>

Introduce a new DOMAIN_ATTR_MSI_RESV domain attribute and associated
iommu_domain msi_resv field. It comprises the size and alignment
of the IOVA reserved window dedicated to MSI mapping. This attribute
only is supported when MSI must be IOMMU mapped.

This is the case on ARM.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Suggested-by: Alex Williamson <alex.williamson@redhat.com>

---
v13 -> v14:
- new msi_resv type and name

v12 -> v13:
- reword the commit message

v8 -> v9:
- rename programmable into iommu_msi_supported
- add iommu_domain_msi_aperture_valid

v8: creation
- deprecates DOMAIN_ATTR_MSI_MAPPING flag
---
 include/linux/iommu.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 436dc21..aaeb598 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -52,6 +52,12 @@ struct iommu_domain_geometry {
 	bool force_aperture;       /* DMA only allowed in mappable range? */
 };
 
+/* MSI reserved IOVA window requirements */
+struct iommu_domain_msi_resv {
+	size_t size;		/* size in bytes	*/
+	size_t alignment;	/* byte alignment	*/
+};
+
 /* Domain feature flags */
 #define __IOMMU_DOMAIN_PAGING	(1U << 0)  /* Support for iommu_map/unmap */
 #define __IOMMU_DOMAIN_DMA_API	(1U << 1)  /* Domain for use in DMA-API
@@ -83,6 +89,7 @@ struct iommu_domain {
 	iommu_fault_handler_t handler;
 	void *handler_token;
 	struct iommu_domain_geometry geometry;
+	struct iommu_domain_msi_resv msi_resv;
 	void *iova_cookie;
 };
 
@@ -108,6 +115,7 @@ enum iommu_cap {
 
 enum iommu_attr {
 	DOMAIN_ATTR_GEOMETRY,
+	DOMAIN_ATTR_MSI_RESV,
 	DOMAIN_ATTR_PAGING,
 	DOMAIN_ATTR_WINDOWS,
 	DOMAIN_ATTR_FSL_PAMU_STASH,
-- 
1.9.1

^ permalink raw reply related

* [PATCH v14 01/16] iommu/iova: fix __alloc_and_insert_iova_range
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>

Fix the size check within start_pfn and limit_pfn.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

the issue was observed when playing with 1 page iova domain with
higher iova reserved.
---
 drivers/iommu/iova.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index e23001b..ee29dbf 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -147,7 +147,7 @@ move_left:
 	if (!curr) {
 		if (size_aligned)
 			pad_size = iova_get_pad_size(size, limit_pfn);
-		if ((iovad->start_pfn + size + pad_size) > limit_pfn) {
+		if ((iovad->start_pfn + size + pad_size - 1) > limit_pfn) {
 			spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
 			return -ENOMEM;
 		}
-- 
1.9.1

^ permalink raw reply related

* [PATCH v14 00/16] KVM PCIe/MSI passthrough on ARM/ARM64
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
  To: linux-arm-kernel

This is the second respin on top of Robin's series [1], addressing Alex' comments.

Major changes are:
- MSI-doorbell API now is moved to DMA IOMMU API following Alex suggestion
  to put all API pieces at the same place (so eventually in the IOMMU
  subsystem)
- new iommu_domain_msi_resv struct and accessor through DOMAIN_ATTR_MSI_RESV
  domain with mirror VFIO capability
- more robustness I think in the VFIO layer
- added "iommu/iova: fix __alloc_and_insert_iova_range" since with the current
  code I failed allocating an IOVA page in a single page domain with upper part
  reserved

IOVA range exclusion will be handled in a separate series

The priority really is to discuss and freeze the API and especially the MSI
doorbell's handling. Do we agree to put that in DMA IOMMU?

Note: the size computation does not take into account possible page overlaps
between doorbells but it would add quite a lot of complexity i think.

Tested on AMD Overdrive (single GICv2m frame) with I350 VF assignment.

dependency:
the series depends on Robin's generic-v7 branch:
[1] [PATCH v7 00/22] Generic DT bindings for PCI IOMMUs and ARM SMMU
http://www.spinics.net/lists/arm-kernel/msg531110.html

Best Regards

Eric

Git: complete series available at
https://github.com/eauger/linux/tree/generic-v7-pcie-passthru-v14

the above branch includes a temporary patch to work around a ThunderX pci
bus reset crash (which I think unrelated to this series):
"vfio: pci: HACK! workaround thunderx pci_try_reset_bus crash"
Do not take this one for other platforms.


Eric Auger (15):
  iommu/iova: fix __alloc_and_insert_iova_range
  iommu: Introduce DOMAIN_ATTR_MSI_RESV
  iommu/dma: MSI doorbell alloc/free
  iommu/dma: Introduce iommu_calc_msi_resv
  iommu/arm-smmu: Implement domain_get_attr for DOMAIN_ATTR_MSI_RESV
  irqchip/gic-v2m: Register the MSI doorbell
  irqchip/gicv3-its: Register the MSI doorbell
  vfio: Introduce a vfio_dma type field
  vfio/type1: vfio_find_dma accepting a type argument
  vfio/type1: Implement recursive vfio_find_dma_from_node
  vfio/type1: Handle unmap/unpin and replay for VFIO_IOVA_RESERVED slots
  vfio: Allow reserved msi iova registration
  vfio/type1: Check doorbell safety
  iommu/arm-smmu: Do not advertise IOMMU_CAP_INTR_REMAP
  vfio/type1: Introduce MSI_RESV capability

Robin Murphy (1):
  iommu/dma: Allow MSI-only cookies

 drivers/iommu/Kconfig            |   4 +-
 drivers/iommu/arm-smmu-v3.c      |  10 +-
 drivers/iommu/arm-smmu.c         |  10 +-
 drivers/iommu/dma-iommu.c        | 184 ++++++++++++++++++++++++++
 drivers/iommu/iova.c             |   2 +-
 drivers/irqchip/irq-gic-v2m.c    |  10 +-
 drivers/irqchip/irq-gic-v3-its.c |  13 ++
 drivers/vfio/vfio_iommu_type1.c  | 279 +++++++++++++++++++++++++++++++++++++--
 include/linux/dma-iommu.h        |  59 +++++++++
 include/linux/iommu.h            |   8 ++
 include/uapi/linux/vfio.h        |  30 ++++-
 11 files changed, 587 insertions(+), 22 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Adrian Hunter @ 2016-10-12 13:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4009a66b-1e09-0780-b7c1-128ff1401b5f@marvell.com>

On 12/10/16 14:58, Ziji Hu wrote:
> Hi Adrian,
> 
> 	Thank you very much for your review.
> 	I will firstly fix the typo.
> 
> On 2016/10/11 20:37, Adrian Hunter wrote:
>> On 07/10/16 18:22, Gregory CLEMENT wrote:
>>> From: Ziji Hu <huziji@marvell.com>
>>>
>>> Add Xenon eMMC/SD/SDIO host controller core functionality.
>>> Add Xenon specific intialization process.
>>> Add Xenon specific mmc_host_ops APIs.
>>> Add Xenon specific register definitions.
>>>
>>> Add CONFIG_MMC_SDHCI_XENON support in drivers/mmc/host/Kconfig.
>>>
>>> Marvell Xenon SDHC conforms to SD Physical Layer Specification
>>> Version 3.01 and is designed according to the guidelines provided
>>> in the SD Host Controller Standard Specification Version 3.00.
>>>
>>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>>> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>
>> I looked at a couple of things but you need to sort out the issues with
>> card_candidate before going further.
>>
> 	Understood.
> 	I will improve the card_candidate. Please help check the details in below.
> 
>>> ---
> <snip>
>>> +
>>> +static int xenon_emmc_signal_voltage_switch(struct mmc_host *mmc,
>>> +					    struct mmc_ios *ios)
>>> +{
>>> +	unsigned char voltage = ios->signal_voltage;
>>> +
>>> +	if ((voltage == MMC_SIGNAL_VOLTAGE_330) ||
>>> +	    (voltage == MMC_SIGNAL_VOLTAGE_180))
>>> +		return __emmc_signal_voltage_switch(mmc, voltage);
>>> +
>>> +	dev_err(mmc_dev(mmc), "Unsupported signal voltage: %d\n",
>>> +		voltage);
>>> +	return -EINVAL;
>>> +}
>>> +
>>> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
>>> +					     struct mmc_ios *ios)
>>> +{
>>> +	struct sdhci_host *host = mmc_priv(mmc);
>>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>> +
>>> +	/*
>>> +	 * Before SD/SDIO set signal voltage, SD bus clock should be
>>> +	 * disabled. However, sdhci_set_clock will also disable the Internal
>>> +	 * clock in mmc_set_signal_voltage().
>>> +	 * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
>>> +	 * Thus here manually enable internal clock.
>>> +	 *
>>> +	 * After switch completes, it is unnecessary to disable internal clock,
>>> +	 * since keeping internal clock active obeys SD spec.
>>> +	 */
>>> +	enable_xenon_internal_clk(host);
>>> +
>>> +	if (priv->card_candidate) {
>>
>> mmc_power_up() calls __mmc_set_signal_voltage() calls
>> host->ops->start_signal_voltage_switch so priv->card_candidate could be an
>> invalid reference to an old card.
>>
>> So that's not going to work if the card changes - not only for removable
>> cards but even for eMMC if init fails and retries.
>>
> 	As you point out, this piece of code have defects, even though it actually works on Marvell multiple platforms, unless eMMC card is removable.
> 
> 	I can add a property to explicitly indicate eMMC type in DTS.
> 	Then card_candidate access can be removed here.
> 	Does it sounds more reasonable to you?

Sure

> 
>>> +		if (mmc_card_mmc(priv->card_candidate))
>>> +			return xenon_emmc_signal_voltage_switch(mmc, ios);
>>
>> So if all you need to know is whether it is a eMMC, why can't DT tell you?
>>
> 	I can add an eMMC type property in DTS, to remove the card_candidate access here.
> 
>>> +	}
>>> +
>>> +	return sdhci_start_signal_voltage_switch(mmc, ios);
>>> +}
>>> +
>>> +/*
>>> + * After determining which slot is used for SDIO,
>>> + * some additional task is required.
>>> + */
>>> +static void xenon_init_card(struct mmc_host *mmc, struct mmc_card *card)
>>> +{
>>> +	struct sdhci_host *host = mmc_priv(mmc);
>>> +	u32 reg;
>>> +	u8 slot_idx;
>>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>> +
>>> +	/* Link the card for delay adjustment */
>>> +	priv->card_candidate = card;
>>
>> You really need a better way to get the card.  I suggest you take up the
>> issue with Ulf.  One possibility is to have mmc core set host->card = card
>> much earlier.
>>
> 	Could you please tell me if any issue related to card_candidate still exists, after the card_candidate is removed from xenon_start_signal_voltage_switch() in above?
> 	It seems that when init_card is called, the structure card has already been updated and stable in MMC/SD/SDIO initialization sequence.
> 	May I keep it here?

It works by accident rather than by design.  We can do better.

> 
>>> +	/* Set tuning functionality of this slot */
>>> +	xenon_slot_tuning_setup(host);
>>> +
>>> +	slot_idx = priv->slot_idx;
>>> +	if (!mmc_card_sdio(card)) {
>>> +		/* Re-enable the Auto-CMD12 cap flag. */
>>> +		host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
>>> +		host->flags |= SDHCI_AUTO_CMD12;
>>> +
>>> +		/* Clear SDIO Card Inserted indication */
>>> +		reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
>>> +		reg &= ~(1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
>>> +		sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
>>> +
>>> +		if (mmc_card_mmc(card)) {
>>> +			mmc->caps |= MMC_CAP_NONREMOVABLE;
>>> +			if (!(host->quirks2 & SDHCI_QUIRK2_NO_1_8_V))
>>> +				mmc->caps |= MMC_CAP_1_8V_DDR;
>>> +			/*
>>> +			 * Force to clear BUS_TEST to
>>> +			 * skip bus_test_pre and bus_test_post
>>> +			 */
>>> +			mmc->caps &= ~MMC_CAP_BUS_WIDTH_TEST;
>>> +			mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ |
>>> +				      MMC_CAP2_PACKED_CMD;
>>> +			if (mmc->caps & MMC_CAP_8_BIT_DATA)
>>> +				mmc->caps2 |= MMC_CAP2_HS400_1_8V;
>>> +		}
>>> +	} else {
>>> +		/*
>>> +		 * Delete the Auto-CMD12 cap flag.
>>> +		 * Otherwise, when sending multi-block CMD53,
>>> +		 * Driver will set Transfer Mode Register to enable Auto CMD12.
>>> +		 * However, SDIO device cannot recognize CMD12.
>>> +		 * Thus SDHC will time-out for waiting for CMD12 response.
>>> +		 */
>>> +		host->quirks &= ~SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
>>> +		host->flags &= ~SDHCI_AUTO_CMD12;
>>
>> sdhci_set_transfer_mode() won't enable auto-CMD12 for CMD53 anyway, so is
>> this needed?
>>
> 	In Xenon driver, Auto-CMD12 flag is set to enable full support to Auto-CMD feature, both Auto-CMD12 and Auto-CMD23.
> 	As a result, when Xenon SDHC slot can both support SD and SDIO, Auto-CMD12 is disabled when SDIO card is inserted, and renabled when SD is inserted.
> 
> 	I recheck the sdhci code to set Auto-CMD bit in Transfer Mode register, in sdhci_set_transfer_mode():
> 	if (mmc_op_multi(cmd->opcode) || data->blocks > 1)
> 	As you can see, as long as it is CMD18/CMD25 OR there are multiple data blocks, Auto-CMD field will be set.
> 	CMD53 doesn't have CMD23. Thus Auto-CMD12 is selected since Auto-CMD12 flag is set.
> 	Thus I have to clear Auto-CMD12 to avoid issuing Auto-CMD12 in SDIO transfer.


The code is:

	if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
		mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
		/*
		 * If we are sending CMD23, CMD12 never gets sent
		 * on successful completion (so no Auto-CMD12).
		 */
		if (sdhci_auto_cmd12(host, cmd->mrq) &&
		    (cmd->opcode != SD_IO_RW_EXTENDED))
			mode |= SDHCI_TRNS_AUTO_CMD12;
		else if (cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23)) {
			mode |= SDHCI_TRNS_AUTO_CMD23;
			sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
		}
	}

You can see the check for SD_IO_RW_EXTENDED which is CMD53.

> 
> 	I just meet a similar issue in RPMB.
> 	When Auto-CMD12 flag is set, eMMC RPMB access will trigger Auto-CMD12, since CMD25 is in use.
> 	It will cause RPMB access failed.

Can you explain more about the RPMB issue.  Doesn't it use CMD23, so CMD12
wouldn't be used - auto or manually.

> 
> 	One possible solution is to drop Auto-CMD12 support and use Auto-CMD23 only, in Xenon driver.
> 	May I know you opinion, please?

I don't use auto-CMD12 because I don't know if it provides any benefit and
sdhci does not seem to have implemented Auto CMD12 Error Recovery, although
I have never looked at it closely.

^ permalink raw reply

* [PATCH 2/2] power/reset: at91-poweroff: timely shitdown LPDDR memories
From: Jean-Jacques Hiblot @ 2016-10-12 12:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161007163427.11454-3-alexandre.belloni@free-electrons.com>

2016-10-07 18:34 GMT+02:00 Alexandre Belloni
<alexandre.belloni@free-electrons.com>:
> LPDDR memories can only handle up to 400 uncontrolled power off. Ensure the
> proper power off sequence is used before shutting down the platform.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>  drivers/power/reset/at91-poweroff.c      | 52 +++++++++++++++++++++++++++++++-
>  drivers/power/reset/at91-sama5d2_shdwc.c | 48 ++++++++++++++++++++++++++++-
>  2 files changed, 98 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/power/reset/at91-poweroff.c b/drivers/power/reset/at91-poweroff.c
> index e9e24df35f26..bf97390e6cd7 100644
> --- a/drivers/power/reset/at91-poweroff.c
> +++ b/drivers/power/reset/at91-poweroff.c
> @@ -14,9 +14,12 @@
>  #include <linux/io.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_address.h>
>  #include <linux/platform_device.h>
>  #include <linux/printk.h>
>
> +#include <soc/at91/at91sam9_ddrsdr.h>
> +
>  #define AT91_SHDW_CR   0x00            /* Shut Down Control Register */
>  #define AT91_SHDW_SHDW         BIT(0)                  /* Shut Down command */
>  #define AT91_SHDW_KEY          (0xa5 << 24)            /* KEY Password */
> @@ -50,6 +53,7 @@ static const char *shdwc_wakeup_modes[] = {
>
>  static void __iomem *at91_shdwc_base;
>  static struct clk *sclk;
> +static void __iomem *mpddrc_base;
>
>  static void __init at91_wakeup_status(void)
>  {
> @@ -73,6 +77,28 @@ static void at91_poweroff(void)
>         writel(AT91_SHDW_KEY | AT91_SHDW_SHDW, at91_shdwc_base + AT91_SHDW_CR);
>  }
>
> +static void at91_lpddr_poweroff(void)
> +{
> +       asm volatile(
> +               /* Align to cache lines */
> +               ".balign 32\n\t"
> +
> +               "       ldr     r6, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
At first sight, it looks useless. I assume it's used to preload the
TLB before the LPDDR is turned off.
A comment to explain why this line is useful would prevent its removal.
> +
> +               /* Power down SDRAM0 */
> +               "       str     %1, [%0, #" __stringify(AT91_DDRSDRC_LPR) "]\n\t"
> +               /* Shutdown CPU */
> +               "       str     %3, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
> +
> +               "       b       .\n\t"
> +               :
> +               : "r" (mpddrc_base),
> +                 "r" cpu_to_le32(AT91_DDRSDRC_LPDDR2_PWOFF),
> +                 "r" (at91_shdwc_base),
> +                 "r" cpu_to_le32(AT91_SHDW_KEY | AT91_SHDW_SHDW)
> +               : "r0");
> +}
> +
>  static int at91_poweroff_get_wakeup_mode(struct device_node *np)
>  {
>         const char *pm;
> @@ -124,6 +150,8 @@ static void at91_poweroff_dt_set_wakeup_mode(struct platform_device *pdev)
>  static int __init at91_poweroff_probe(struct platform_device *pdev)
>  {
>         struct resource *res;
> +       struct device_node *np;
> +       u32 ddr_type;
>         int ret;
>
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -150,12 +178,29 @@ static int __init at91_poweroff_probe(struct platform_device *pdev)
>
>         pm_power_off = at91_poweroff;
>
> +       np = of_find_compatible_node(NULL, NULL, "atmel,sama5d3-ddramc");
> +       if (!np)
> +               return 0;
> +
> +       mpddrc_base = of_iomap(np, 0);
> +       of_node_put(np);
> +
> +       if (!mpddrc_base)
> +               return 0;
> +
> +       ddr_type = readl(mpddrc_base + AT91_DDRSDRC_MDR) & AT91_DDRSDRC_MD;
> +       if ((ddr_type == AT91_DDRSDRC_MD_LPDDR2) ||
> +           (ddr_type == AT91_DDRSDRC_MD_LPDDR3))
Souldn't there be something like "pm_power_off = at91_lpddr_poweroff;" here ?

Jean-Jacques

> +       else
> +               iounmap(mpddrc_base);
> +
>         return 0;
>  }
>
>  static int __exit at91_poweroff_remove(struct platform_device *pdev)
>  {
> -       if (pm_power_off == at91_poweroff)
> +       if (pm_power_off == at91_poweroff ||
> +           pm_power_off == at91_lpddr_poweroff)
>                 pm_power_off = NULL;
>
>         clk_disable_unprepare(sclk);
> @@ -163,6 +208,11 @@ static int __exit at91_poweroff_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> +static const struct of_device_id at91_ramc_of_match[] = {
> +       { .compatible = "atmel,sama5d3-ddramc", },
> +       { /* sentinel */ }
> +};
> +
>  static const struct of_device_id at91_poweroff_of_match[] = {
>         { .compatible = "atmel,at91sam9260-shdwc", },
>         { .compatible = "atmel,at91sam9rl-shdwc", },
> diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c b/drivers/power/reset/at91-sama5d2_shdwc.c
> index 8a5ac9706c9c..5736f360b374 100644
> --- a/drivers/power/reset/at91-sama5d2_shdwc.c
> +++ b/drivers/power/reset/at91-sama5d2_shdwc.c
> @@ -22,9 +22,12 @@
>  #include <linux/io.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_address.h>
>  #include <linux/platform_device.h>
>  #include <linux/printk.h>
>
> +#include <soc/at91/at91sam9_ddrsdr.h>
> +
>  #define SLOW_CLOCK_FREQ        32768
>
>  #define AT91_SHDW_CR   0x00            /* Shut Down Control Register */
> @@ -75,6 +78,7 @@ struct shdwc {
>   */
>  static struct shdwc *at91_shdwc;
>  static struct clk *sclk;
> +static void __iomem *mpddrc_base;
>
>  static const unsigned long long sdwc_dbc_period[] = {
>         0, 3, 32, 512, 4096, 32768,
> @@ -108,6 +112,28 @@ static void at91_poweroff(void)
>                at91_shdwc->at91_shdwc_base + AT91_SHDW_CR);
>  }
>
> +static void at91_lpddr_poweroff(void)
> +{
> +       asm volatile(
> +               /* Align to cache lines */
> +               ".balign 32\n\t"
> +
> +               "       ldr     r6, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
> +
> +               /* Power down SDRAM0 */
> +               "       str     %1, [%0, #" __stringify(AT91_DDRSDRC_LPR) "]\n\t"
> +               /* Shutdown CPU */
> +               "       str     %3, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
> +
> +               "       b       .\n\t"
> +               :
> +               : "r" (mpddrc_base),
> +                 "r" cpu_to_le32(AT91_DDRSDRC_LPDDR2_PWOFF),
> +                 "r" (at91_shdwc->at91_shdwc_base),
> +                 "r" cpu_to_le32(AT91_SHDW_KEY | AT91_SHDW_SHDW)
> +               : "r0");
> +}
> +
>  static u32 at91_shdwc_debouncer_value(struct platform_device *pdev,
>                                       u32 in_period_us)
>  {
> @@ -212,6 +238,8 @@ static int __init at91_shdwc_probe(struct platform_device *pdev)
>  {
>         struct resource *res;
>         const struct of_device_id *match;
> +       struct device_node *np;
> +       u32 ddr_type;
>         int ret;
>
>         if (!pdev->dev.of_node)
> @@ -249,6 +277,23 @@ static int __init at91_shdwc_probe(struct platform_device *pdev)
>
>         pm_power_off = at91_poweroff;
>
> +       np = of_find_compatible_node(NULL, NULL, "atmel,sama5d3-ddramc");
> +       if (!np)
> +               return 0;
> +
> +       mpddrc_base = of_iomap(np, 0);
> +       of_node_put(np);
> +
> +       if (!mpddrc_base)
> +               return 0;
> +
> +       ddr_type = readl(mpddrc_base + AT91_DDRSDRC_MDR) & AT91_DDRSDRC_MD;
> +       if ((ddr_type == AT91_DDRSDRC_MD_LPDDR2) ||
> +           (ddr_type == AT91_DDRSDRC_MD_LPDDR3))
> +               pm_power_off = at91_lpddr_poweroff;
> +       else
> +               iounmap(mpddrc_base);
> +
>         return 0;
>  }
>
> @@ -256,7 +301,8 @@ static int __exit at91_shdwc_remove(struct platform_device *pdev)
>  {
>         struct shdwc *shdw = platform_get_drvdata(pdev);
>
> -       if (pm_power_off == at91_poweroff)
> +       if (pm_power_off == at91_poweroff ||
> +           pm_power_off == at91_lpddr_poweroff)
>                 pm_power_off = NULL;
>
>         /* Reset values to disable wake-up features  */
> --
> 2.9.3
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [RFC PATCH 05/11] pci: rename *host* directory to *controller*
From: Christoph Hellwig @ 2016-10-12 12:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473829927-20466-6-git-send-email-kishon@ti.com>

This is a big and painful change.  I'd suggest to either drop it for
now or convince Bjorn to take it as a scripted renamed just after -rc1.

^ permalink raw reply

* [RFC PATCH 02/11] pci: endpoint: introduce configfs entry for configuring EP functions
From: Christoph Hellwig @ 2016-10-12 12:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473829927-20466-3-git-send-email-kishon@ti.com>

On Wed, Sep 14, 2016 at 10:41:58AM +0530, Kishon Vijay Abraham I wrote:
> diff --git a/drivers/pci/endpoint/Kconfig b/drivers/pci/endpoint/Kconfig
> index a6d827c..f1dd206 100644
> --- a/drivers/pci/endpoint/Kconfig
> +++ b/drivers/pci/endpoint/Kconfig
> @@ -13,7 +13,9 @@ config PCI_ENDPOINT
>  
>  	   Enabling this option will build the endpoint library, which
>  	   includes endpoint controller library and endpoint function
> -	   library.
> +	   library. This will also enable the configfs entry required to
> +	   configure the endpoint function and used to bind the
> +	   function with a endpoint controller.
>  
>  	   If in doubt, say "N" to disable Endpoint support.

This needs to grow a

	depends on CONFIGFS_FS

> +/**
> + * pci-ep-cfs.c - configfs to configure the PCI endpoint

Please don't use the file name in the top of the file comment, it's
only bound to get out of date..

> +struct pci_epf_info {
> +	struct config_item pci_epf;
> +	struct pci_epf *epf;
> +};

Any reason not to simply embedd the config_item into the pci_epf structure?

^ permalink raw reply

* [RFC PATCH 01/11] pci: endpoint: add EP core layer to enable EP controller and EP functions
From: Christoph Hellwig @ 2016-10-12 12:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473829927-20466-2-git-send-email-kishon@ti.com>

> +/**
> + * pci_epc_stop() - stop the PCI link
> + * @epc: the link of the EPC device that has to be stopped
> + *
> + * Invoke to stop the PCI link
> + */
> +void pci_epc_stop(struct pci_epc *epc)
> +{
> +	if (IS_ERR(epc) || !epc->ops->stop)
> +		return;
> +
> +	spin_lock_irq(&epc->irq_lock);
> +	epc->ops->stop(epc);
> +	spin_unlock_irq(&epc->irq_lock);
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_stop);

Can you elaborate on the synchronization strategy here?  It seems
like irq_lock is generally taken irq save and just around method
calls.  Wou;dn't it be better to leave locking to the methods
themselves?

> +/**
> + * struct pci_epc - represents the PCI EPC device
> + * @dev: PCI EPC device
> + * @ops: function pointers for performing endpoint operations
> + * @mutex: mutex to protect pci_epc ops
> + */
> +struct pci_epc {
> +	struct device			dev;
> +	/* support only single function PCI device for now */
> +	struct pci_epf			*epf;
> +	const struct pci_epc_ops	*ops;
> +	spinlock_t			irq_lock;
> +};

And this still documentes a mutex instead of the irq save spinlock,
while we're at it..

> +/**
> + * struct pci_epf_bar - represents the BAR of EPF device
> + * @phys_addr: physical address that should be mapped to the BAR
> + * @size: the size of the address space present in BAR
> + */
> +struct pci_epf_bar {
> +	dma_addr_t	phys_addr;
> +	size_t		size;
> +};

Just curious: shouldn't this be a phys_addr_t instead of a dma_addr_t?


Otherwise this looks like a nice little framework to get started!

^ permalink raw reply

* [PATCH v2 2/2] PCI: aardvark: Remove unused platform data
From: Bjorn Helgaas @ 2016-10-12 12:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161012123001.22186.4053.stgit@bhelgaas-glaptop2.roam.corp.google.com>

The aardvark driver never uses the platform drvdata pointer, so don't
bother setting it.  No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/pci/host/pci-aardvark.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c
index 16421d1..4fce494 100644
--- a/drivers/pci/host/pci-aardvark.c
+++ b/drivers/pci/host/pci-aardvark.c
@@ -926,7 +926,6 @@ static int advk_pcie_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	pcie->pdev = pdev;
-	platform_set_drvdata(pdev, pcie);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	pcie->base = devm_ioremap_resource(dev, res);

^ permalink raw reply related

* [PATCH v2 1/2] PCI: aardvark: Add local struct device pointers
From: Bjorn Helgaas @ 2016-10-12 12:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161012123001.22186.4053.stgit@bhelgaas-glaptop2.roam.corp.google.com>

Use a local "struct device *dev" for brevity and consistency with other
drivers.  No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/pci/host/pci-aardvark.c |   38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c
index e4a5b7e..16421d1 100644
--- a/drivers/pci/host/pci-aardvark.c
+++ b/drivers/pci/host/pci-aardvark.c
@@ -230,20 +230,20 @@ static int advk_pcie_link_up(struct advk_pcie *pcie)
 
 static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
 {
+	struct device *dev = &pcie->pdev->dev;
 	int retries;
 
 	/* check if the link is up or not */
 	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
 		if (advk_pcie_link_up(pcie)) {
-			dev_info(&pcie->pdev->dev, "link up\n");
+			dev_info(dev, "link up\n");
 			return 0;
 		}
 
 		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
 	}
 
-	dev_err(&pcie->pdev->dev, "link never came up\n");
-
+	dev_err(dev, "link never came up\n");
 	return -ETIMEDOUT;
 }
 
@@ -376,6 +376,7 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
 
 static void advk_pcie_check_pio_status(struct advk_pcie *pcie)
 {
+	struct device *dev = &pcie->pdev->dev;
 	u32 reg;
 	unsigned int status;
 	char *strcomp_status, *str_posted;
@@ -407,12 +408,13 @@ static void advk_pcie_check_pio_status(struct advk_pcie *pcie)
 	else
 		str_posted = "Posted";
 
-	dev_err(&pcie->pdev->dev, "%s PIO Response Status: %s, %#x @ %#x\n",
+	dev_err(dev, "%s PIO Response Status: %s, %#x @ %#x\n",
 		str_posted, strcomp_status, reg, advk_readl(pcie, PIO_ADDR_LS));
 }
 
 static int advk_pcie_wait_pio(struct advk_pcie *pcie)
 {
+	struct device *dev = &pcie->pdev->dev;
 	unsigned long timeout;
 
 	timeout = jiffies + msecs_to_jiffies(PIO_TIMEOUT_MS);
@@ -426,7 +428,7 @@ static int advk_pcie_wait_pio(struct advk_pcie *pcie)
 			return 0;
 	}
 
-	dev_err(&pcie->pdev->dev, "config read/write timed out\n");
+	dev_err(dev, "config read/write timed out\n");
 	return -ETIMEDOUT;
 }
 
@@ -560,10 +562,11 @@ static int advk_pcie_alloc_msi(struct advk_pcie *pcie)
 
 static void advk_pcie_free_msi(struct advk_pcie *pcie, int hwirq)
 {
+	struct device *dev = &pcie->pdev->dev;
+
 	mutex_lock(&pcie->msi_used_lock);
 	if (!test_bit(hwirq, pcie->msi_irq_in_use))
-		dev_err(&pcie->pdev->dev, "trying to free unused MSI#%d\n",
-			hwirq);
+		dev_err(dev, "trying to free unused MSI#%d\n", hwirq);
 	else
 		clear_bit(hwirq, pcie->msi_irq_in_use);
 	mutex_unlock(&pcie->msi_used_lock);
@@ -910,6 +913,7 @@ out_release_res:
 
 static int advk_pcie_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct advk_pcie *pcie;
 	struct resource *res;
 	struct pci_bus *bus, *child;
@@ -917,8 +921,7 @@ static int advk_pcie_probe(struct platform_device *pdev)
 	struct device_node *msi_node;
 	int ret, irq;
 
-	pcie = devm_kzalloc(&pdev->dev, sizeof(struct advk_pcie),
-			    GFP_KERNEL);
+	pcie = devm_kzalloc(dev, sizeof(struct advk_pcie), GFP_KERNEL);
 	if (!pcie)
 		return -ENOMEM;
 
@@ -926,22 +929,22 @@ static int advk_pcie_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, pcie);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	pcie->base = devm_ioremap_resource(&pdev->dev, res);
+	pcie->base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(pcie->base))
 		return PTR_ERR(pcie->base);
 
 	irq = platform_get_irq(pdev, 0);
-	ret = devm_request_irq(&pdev->dev, irq, advk_pcie_irq_handler,
+	ret = devm_request_irq(dev, irq, advk_pcie_irq_handler,
 			       IRQF_SHARED | IRQF_NO_THREAD, "advk-pcie",
 			       pcie);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to register interrupt\n");
+		dev_err(dev, "Failed to register interrupt\n");
 		return ret;
 	}
 
 	ret = advk_pcie_parse_request_of_pci_ranges(pcie);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to parse resources\n");
+		dev_err(dev, "Failed to parse resources\n");
 		return ret;
 	}
 
@@ -949,24 +952,24 @@ static int advk_pcie_probe(struct platform_device *pdev)
 
 	ret = advk_pcie_init_irq_domain(pcie);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to initialize irq\n");
+		dev_err(dev, "Failed to initialize irq\n");
 		return ret;
 	}
 
 	ret = advk_pcie_init_msi_irq_domain(pcie);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to initialize irq\n");
+		dev_err(dev, "Failed to initialize irq\n");
 		advk_pcie_remove_irq_domain(pcie);
 		return ret;
 	}
 
-	msi_node = of_parse_phandle(pdev->dev.of_node, "msi-parent", 0);
+	msi_node = of_parse_phandle(dev->of_node, "msi-parent", 0);
 	if (msi_node)
 		msi = of_pci_find_msi_chip_by_node(msi_node);
 	else
 		msi = NULL;
 
-	bus = pci_scan_root_bus_msi(&pdev->dev, 0, &advk_pcie_ops,
+	bus = pci_scan_root_bus_msi(dev, 0, &advk_pcie_ops,
 				    pcie, &pcie->resources, &pcie->msi);
 	if (!bus) {
 		advk_pcie_remove_msi_irq_domain(pcie);
@@ -980,7 +983,6 @@ static int advk_pcie_probe(struct platform_device *pdev)
 		pcie_bus_configure_settings(child);
 
 	pci_bus_add_devices(bus);
-
 	return 0;
 }
 

^ permalink raw reply related

* [PATCH v2 0/2] PCI: aardvark: Cleanup
From: Bjorn Helgaas @ 2016-10-12 12:36 UTC (permalink / raw)
  To: linux-arm-kernel

Add local "dev" pointers to reduce repetition of things like "&pdev->dev"
and remove unused drvdata.

Changes from v1:
  I dropped the following because they added a lot of churn for
  questionable benefit:
    PCI: aardvark: Name private struct pointer "advk" consistently
    PCI: aardvark: Reorder accessor functions
    PCI: aardvark: Swap order of advk_write() reg/val arguments

---

Bjorn Helgaas (2):
      PCI: aardvark: Add local struct device pointers
      PCI: aardvark: Remove unused platform data


 drivers/pci/host/pci-aardvark.c |   39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

^ permalink raw reply

* [RFC PATCH 00/11] pci: support for configurable PCI endpoint
From: Christoph Hellwig @ 2016-10-12 12:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57E8BB69.4020804@ti.com>

On Mon, Sep 26, 2016 at 11:38:41AM +0530, Kishon Vijay Abraham I wrote:
> > Ok, so in theory there can be other hardware (and quite likely is)
> > that supports multiple functions, and we can extend the framework
> > to support them without major obstacles, but your hardware doesn't,
> > so you kept it simple with one hardcoded function, right?
> 
> right, PCIe can have upto 8 functions. So the issues with the current framework
> has to be fixed. I don't expect major obstacles with this as of now.

I wouldn't be too worried about.  We have two kinds of functions in
PCIe: physical functions, or virtual functions using SR-IOV.

For the first one we pretty much just need the controller driver to
report them separately as there is almost no interaction between
functions.

SR-IOV support will be more interesting as the physical functions
controls creation of the associated virtual functions.  I'd like to
defer that problem until we get hold of a software programmable
controller that supports SR-IOV and has open documentation.  (That
beeing said, if someone has a pointer to such a beast send it my way!)

> > We should still find out whether it's important that you can have
> > a single PCI function with a software multi-function support of some
> > sort. We'd still be limited to six BARs in total, and would also need
> > something to identify those sub-functions, so implementing that might
> > get quite hairy.
> > 
> > Possibly this could be done at a higher level, e.g. by implementing
> > a PCI-virtio multiplexer that can host multiple virtio based devices
> > inside of a single PCI function. If we think that would be a good idea,
> > we should make sure the configfs interface is extensible enough to
> > handle that.
> 
> Okay. So here the main function (actual PCI function) *can* perform the work of
> virtio muliplexer if the platform wants to support sub-functions or it can be a
> normal PCI function. right?

I really don't think we should be worried about this multiplexer.  It's
not something real PCIe devices do (sane ones anyway, the rest is
handled by ad-hoc multiplexers), and we should avoid creating our own
magic periphals for it.

> > One use case I have in mind for this is to have a PCI function that
> > can use virtio to provide rootfs (virtio-blk or 9pfs), network
> > and console to the system that implements the PCI function (note
> > that this is the opposite direction of what almost everyone else
> > uses PCI devices for).
> 
> Do you mean the virtio should actually be in the host side? Even here the
> system that implements PCI function should have multiple functions right? (one
> for network, other for console etc..). So there should be a virtio multiplexer
> both in the host side and in the device side?

We already support virtio over phsysical PCIe buses to support intel MIC
devices.  Take a look at drivers/misc/mic/bus/vop_bus.c and
drivers/misc/mic/vop (yes, what a horrible place for that code, not my
fault)

^ permalink raw reply

* [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-10-12 12:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <bfe30317-bb3b-b3dc-e9df-87b6ba100307@intel.com>

Hi Adrian,

On 2016/10/11 20:39, Adrian Hunter wrote:
> On 07/10/16 18:22, Gregory CLEMENT wrote:
>> From: Ziji Hu <huziji@marvell.com>
>>
>> Marvell Xenon eMMC/SD/SDIO Host Controller contains PHY.
>> Three types of PHYs are supported.
>>
>> Add support to multiple types of PHYs init and configuration.
>> Add register definitions of PHYs.
>>
>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>>  MAINTAINERS                        |    1 +-
>>  drivers/mmc/host/Makefile          |    2 +-
>>  drivers/mmc/host/sdhci-xenon-phy.c | 1141 +++++++++++++++++++++++++++++-
>>  drivers/mmc/host/sdhci-xenon-phy.h |  157 ++++-
>>  drivers/mmc/host/sdhci-xenon.c     |    4 +-
>>  drivers/mmc/host/sdhci-xenon.h     |   17 +-
>>  6 files changed, 1321 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/mmc/host/sdhci-xenon-phy.c
>>  create mode 100644 drivers/mmc/host/sdhci-xenon-phy.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 859420e5dfd3..b5673c2ee5f2 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7583,6 +7583,7 @@ M:	Ziji Hu <huziji@marvell.com>
>>  L:	linux-mmc at vger.kernel.org
>>  S:	Supported
>>  F:	drivers/mmc/host/sdhci-xenon.*
>> +F:	drivers/mmc/host/sdhci-xenon-phy.*
>>  F:	Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
>>  
>>  MATROX FRAMEBUFFER DRIVER
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index 75eaf743486c..4f2854556ff7 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -82,4 +82,4 @@ ifeq ($(CONFIG_CB710_DEBUG),y)
>>  endif
>>  
>>  obj-$(CONFIG_MMC_SDHCI_XENON)	+= sdhci-xenon-driver.o
>> -sdhci-xenon-driver-y		+= sdhci-xenon.o
>> +sdhci-xenon-driver-y		+= sdhci-xenon.o sdhci-xenon-phy.o
>> diff --git a/drivers/mmc/host/sdhci-xenon-phy.c b/drivers/mmc/host/sdhci-xenon-phy.c
>> new file mode 100644
>> index 000000000000..4eb8fea1bec9
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-xenon-phy.c
> 
> <SNIP>
> 
>> +static int __xenon_emmc_delay_adj_test(struct mmc_card *card)
>> +{
>> +	int err;
>> +	u8 *ext_csd = NULL;
>> +
>> +	err = mmc_get_ext_csd(card, &ext_csd);
>> +	kfree(ext_csd);
>> +
>> +	return err;
>> +}
>> +
>> +static int __xenon_sdio_delay_adj_test(struct mmc_card *card)
>> +{
>> +	struct mmc_command cmd = {0};
>> +	int err;
>> +
>> +	cmd.opcode = SD_IO_RW_DIRECT;
>> +	cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
>> +
>> +	err = mmc_wait_for_cmd(card->host, &cmd, 0);
>> +	if (err)
>> +		return err;
>> +
>> +	if (cmd.resp[0] & R5_ERROR)
>> +		return -EIO;
>> +	if (cmd.resp[0] & R5_FUNCTION_NUMBER)
>> +		return -EINVAL;
>> +	if (cmd.resp[0] & R5_OUT_OF_RANGE)
>> +		return -ERANGE;
>> +	return 0;
>> +}
>> +
>> +static int __xenon_sd_delay_adj_test(struct mmc_card *card)
>> +{
>> +	struct mmc_command cmd = {0};
>> +	int err;
>> +
>> +	cmd.opcode = MMC_SEND_STATUS;
>> +	cmd.arg = card->rca << 16;
>> +	cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
>> +
>> +	err = mmc_wait_for_cmd(card->host, &cmd, 0);
>> +	return err;
>> +}
>> +
>> +static int xenon_delay_adj_test(struct mmc_card *card)
>> +{
>> +	WARN_ON(!card);
>> +	WARN_ON(!card->host);
>> +
>> +	if (mmc_card_mmc(card))
>> +		return __xenon_emmc_delay_adj_test(card);
>> +	else if (mmc_card_sd(card))
>> +		return __xenon_sd_delay_adj_test(card);
>> +	else if (mmc_card_sdio(card))
>> +		return __xenon_sdio_delay_adj_test(card);
>> +	else
>> +		return -EINVAL;
>> +}
> 
> So you are issuing commands from the ->set_ios() callback.  I would want to
> get Ulf's OK for that before going further.
> 
	Yes, you are correct.
	In some speed mode, Xenon SDHC has to send a series of transfers to search for a perfect sampling point in PHY delay line.
	It is like tuning process.

> One thing: you will need to ensure you don't trigger get HS400 re-tuning
> because it will call back into ->set_ios().
> 
	Could you please make the term "HS400 re-tuning" more detailed?
	In current MMC driver, "HS400 re-tuning" will go back to HS200, execute HS200 tuning and come back to HS400.
	I'm sure our Xenon SDHC will not execute it.

	However, in coming eMMC 5.2, there is a real HS400 re-tuning, in which tuning can be directly executed in HS400 mode.
	Our Xenon SDHC will neither trigger this HS400 re-tuning.
	But since so far there is no such feature in MMC driver, I cannot give you a 100% guarantee now.

> And you have the problem that you need to get a reference to the card before
> the card device has been added.  As I wrote in response to the previous
> patch, you should get Ulf's help with that too.
> 
	Sure.
	I will get card_candidate solved at first.
	Thank you again for your review and help.

	Thank you.

Best regards,
Hu Ziji
> 

^ permalink raw reply

* [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ziji Hu @ 2016-10-12 11:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1b015f16-aa37-591c-3299-9431f6e82cce@intel.com>

Hi Adrian,

	Thank you very much for your review.
	I will firstly fix the typo.

On 2016/10/11 20:37, Adrian Hunter wrote:
> On 07/10/16 18:22, Gregory CLEMENT wrote:
>> From: Ziji Hu <huziji@marvell.com>
>>
>> Add Xenon eMMC/SD/SDIO host controller core functionality.
>> Add Xenon specific intialization process.
>> Add Xenon specific mmc_host_ops APIs.
>> Add Xenon specific register definitions.
>>
>> Add CONFIG_MMC_SDHCI_XENON support in drivers/mmc/host/Kconfig.
>>
>> Marvell Xenon SDHC conforms to SD Physical Layer Specification
>> Version 3.01 and is designed according to the guidelines provided
>> in the SD Host Controller Standard Specification Version 3.00.
>>
>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> 
> I looked at a couple of things but you need to sort out the issues with
> card_candidate before going further.
> 
	Understood.
	I will improve the card_candidate. Please help check the details in below.

>> ---
<snip>
>> +
>> +static int xenon_emmc_signal_voltage_switch(struct mmc_host *mmc,
>> +					    struct mmc_ios *ios)
>> +{
>> +	unsigned char voltage = ios->signal_voltage;
>> +
>> +	if ((voltage == MMC_SIGNAL_VOLTAGE_330) ||
>> +	    (voltage == MMC_SIGNAL_VOLTAGE_180))
>> +		return __emmc_signal_voltage_switch(mmc, voltage);
>> +
>> +	dev_err(mmc_dev(mmc), "Unsupported signal voltage: %d\n",
>> +		voltage);
>> +	return -EINVAL;
>> +}
>> +
>> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
>> +					     struct mmc_ios *ios)
>> +{
>> +	struct sdhci_host *host = mmc_priv(mmc);
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +
>> +	/*
>> +	 * Before SD/SDIO set signal voltage, SD bus clock should be
>> +	 * disabled. However, sdhci_set_clock will also disable the Internal
>> +	 * clock in mmc_set_signal_voltage().
>> +	 * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
>> +	 * Thus here manually enable internal clock.
>> +	 *
>> +	 * After switch completes, it is unnecessary to disable internal clock,
>> +	 * since keeping internal clock active obeys SD spec.
>> +	 */
>> +	enable_xenon_internal_clk(host);
>> +
>> +	if (priv->card_candidate) {
> 
> mmc_power_up() calls __mmc_set_signal_voltage() calls
> host->ops->start_signal_voltage_switch so priv->card_candidate could be an
> invalid reference to an old card.
> 
> So that's not going to work if the card changes - not only for removable
> cards but even for eMMC if init fails and retries.
> 
	As you point out, this piece of code have defects, even though it actually works on Marvell multiple platforms, unless eMMC card is removable.

	I can add a property to explicitly indicate eMMC type in DTS.
	Then card_candidate access can be removed here.
	Does it sounds more reasonable to you?

>> +		if (mmc_card_mmc(priv->card_candidate))
>> +			return xenon_emmc_signal_voltage_switch(mmc, ios);
> 
> So if all you need to know is whether it is a eMMC, why can't DT tell you?
> 
	I can add an eMMC type property in DTS, to remove the card_candidate access here.

>> +	}
>> +
>> +	return sdhci_start_signal_voltage_switch(mmc, ios);
>> +}
>> +
>> +/*
>> + * After determining which slot is used for SDIO,
>> + * some additional task is required.
>> + */
>> +static void xenon_init_card(struct mmc_host *mmc, struct mmc_card *card)
>> +{
>> +	struct sdhci_host *host = mmc_priv(mmc);
>> +	u32 reg;
>> +	u8 slot_idx;
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +
>> +	/* Link the card for delay adjustment */
>> +	priv->card_candidate = card;
> 
> You really need a better way to get the card.  I suggest you take up the
> issue with Ulf.  One possibility is to have mmc core set host->card = card
> much earlier.
> 
	Could you please tell me if any issue related to card_candidate still exists, after the card_candidate is removed from xenon_start_signal_voltage_switch() in above?
	It seems that when init_card is called, the structure card has already been updated and stable in MMC/SD/SDIO initialization sequence.
	May I keep it here?

>> +	/* Set tuning functionality of this slot */
>> +	xenon_slot_tuning_setup(host);
>> +
>> +	slot_idx = priv->slot_idx;
>> +	if (!mmc_card_sdio(card)) {
>> +		/* Re-enable the Auto-CMD12 cap flag. */
>> +		host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
>> +		host->flags |= SDHCI_AUTO_CMD12;
>> +
>> +		/* Clear SDIO Card Inserted indication */
>> +		reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
>> +		reg &= ~(1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
>> +		sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
>> +
>> +		if (mmc_card_mmc(card)) {
>> +			mmc->caps |= MMC_CAP_NONREMOVABLE;
>> +			if (!(host->quirks2 & SDHCI_QUIRK2_NO_1_8_V))
>> +				mmc->caps |= MMC_CAP_1_8V_DDR;
>> +			/*
>> +			 * Force to clear BUS_TEST to
>> +			 * skip bus_test_pre and bus_test_post
>> +			 */
>> +			mmc->caps &= ~MMC_CAP_BUS_WIDTH_TEST;
>> +			mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ |
>> +				      MMC_CAP2_PACKED_CMD;
>> +			if (mmc->caps & MMC_CAP_8_BIT_DATA)
>> +				mmc->caps2 |= MMC_CAP2_HS400_1_8V;
>> +		}
>> +	} else {
>> +		/*
>> +		 * Delete the Auto-CMD12 cap flag.
>> +		 * Otherwise, when sending multi-block CMD53,
>> +		 * Driver will set Transfer Mode Register to enable Auto CMD12.
>> +		 * However, SDIO device cannot recognize CMD12.
>> +		 * Thus SDHC will time-out for waiting for CMD12 response.
>> +		 */
>> +		host->quirks &= ~SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
>> +		host->flags &= ~SDHCI_AUTO_CMD12;
> 
> sdhci_set_transfer_mode() won't enable auto-CMD12 for CMD53 anyway, so is
> this needed?
> 
	In Xenon driver, Auto-CMD12 flag is set to enable full support to Auto-CMD feature, both Auto-CMD12 and Auto-CMD23.
	As a result, when Xenon SDHC slot can both support SD and SDIO, Auto-CMD12 is disabled when SDIO card is inserted, and renabled when SD is inserted.

	I recheck the sdhci code to set Auto-CMD bit in Transfer Mode register, in sdhci_set_transfer_mode():
	if (mmc_op_multi(cmd->opcode) || data->blocks > 1)
	As you can see, as long as it is CMD18/CMD25 OR there are multiple data blocks, Auto-CMD field will be set.
	CMD53 doesn't have CMD23. Thus Auto-CMD12 is selected since Auto-CMD12 flag is set.
	Thus I have to clear Auto-CMD12 to avoid issuing Auto-CMD12 in SDIO transfer.

	I just meet a similar issue in RPMB.
	When Auto-CMD12 flag is set, eMMC RPMB access will trigger Auto-CMD12, since CMD25 is in use.
	It will cause RPMB access failed.

	One possible solution is to drop Auto-CMD12 support and use Auto-CMD23 only, in Xenon driver.
	May I know you opinion, please?

>> +
>> +		/*
>> +		 * Set SDIO Card Inserted indication
>> +		 * to inform that the current slot is for SDIO
>> +		 */
>> +		reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
>> +		reg |= (1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
>> +		sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
>> +	}
>> +}
>> +
>> +static int xenon_execute_tuning(struct mmc_host *mmc, u32 opcode)
>> +{
>> +	struct sdhci_host *host = mmc_priv(mmc);
>> +
>> +	if (host->timing == MMC_TIMING_UHS_DDR50)
>> +		return 0;
>> +
>> +	return sdhci_execute_tuning(mmc, opcode);
>> +}
>> +
>> +static void xenon_replace_mmc_host_ops(struct sdhci_host *host)
>> +{
>> +	host->mmc_host_ops.set_ios = xenon_set_ios;
>> +	host->mmc_host_ops.start_signal_voltage_switch =
>> +			xenon_start_signal_voltage_switch;
>> +	host->mmc_host_ops.init_card = xenon_init_card;
>> +	host->mmc_host_ops.execute_tuning = xenon_execute_tuning;
>> +}
>> +
>> +static int xenon_probe_dt(struct platform_device *pdev)
>> +{
>> +	struct device_node *np = pdev->dev.of_node;
>> +	struct sdhci_host *host = platform_get_drvdata(pdev);
>> +	struct mmc_host *mmc = host->mmc;
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +	int err;
>> +	u32 slot_idx, nr_slot;
>> +	u32 tuning_count;
>> +	u32 reg;
>> +
>> +	/* Standard MMC property */
>> +	err = mmc_of_parse(mmc);
>> +	if (err)
>> +		return err;
>> +
>> +	/* Standard SDHCI property */
>> +	sdhci_get_of_property(pdev);
>> +
>> +	/*
>> +	 * Xenon Specific property:
>> +	 * slotno: the index of slot. Refer to SDHC_SYS_CFG_INFO register
>> +	 * tuning-count: the interval between re-tuning
>> +	 * PHY type: "sdhc phy", "emmc phy 5.0" or "emmc phy 5.1"
>> +	 */
>> +	if (!of_property_read_u32(np, "xenon,slotno", &slot_idx)) {
>> +		nr_slot = sdhci_readl(host, SDHC_SYS_CFG_INFO);
>> +		nr_slot &= NR_SUPPORTED_SLOT_MASK;
>> +		if (unlikely(slot_idx > nr_slot)) {
>> +			dev_err(mmc_dev(mmc), "Slot Index %d exceeds Number of slots %d\n",
>> +				slot_idx, nr_slot);
>> +			return -EINVAL;
>> +		}
>> +	} else {
>> +		priv->slot_idx = 0x0;
>> +	}
>> +
>> +	if (!of_property_read_u32(np, "xenon,tuning-count", &tuning_count)) {
>> +		if (unlikely(tuning_count >= TMR_RETUN_NO_PRESENT)) {
>> +			dev_err(mmc_dev(mmc), "Wrong Re-tuning Count. Set default value %d\n",
>> +				DEF_TUNING_COUNT);
>> +			tuning_count = DEF_TUNING_COUNT;
>> +		}
>> +	} else {
>> +		priv->tuning_count = DEF_TUNING_COUNT;
>> +	}
>> +
>> +	if (of_property_read_bool(np, "xenon,mask-conflict-err")) {
>> +		reg = sdhci_readl(host, SDHC_SYS_EXT_OP_CTRL);
>> +		reg |= MASK_CMD_CONFLICT_ERROR;
>> +		sdhci_writel(host, reg, SDHC_SYS_EXT_OP_CTRL);
>> +	}
>> +
>> +	return err;
>> +}
>> +
>> +static int xenon_slot_probe(struct sdhci_host *host)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +	u8 slot_idx = priv->slot_idx;
>> +
>> +	/* Enable slot */
>> +	xenon_enable_slot(host, slot_idx);
>> +
>> +	/* Enable ACG */
>> +	xenon_set_acg(host, true);
>> +
>> +	/* Enable Parallel Transfer Mode */
>> +	xenon_enable_slot_parallel_tran(host, slot_idx);
>> +
>> +	priv->timing = MMC_TIMING_FAKE;
>> +	priv->clock = 0;
>> +
>> +	return 0;
>> +}
>> +
>> +static void xenon_slot_remove(struct sdhci_host *host)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +	u8 slot_idx = priv->slot_idx;
>> +
>> +	/* disable slot */
>> +	xenon_disable_slot(host, slot_idx);
>> +}
>> +
>> +static int sdhci_xenon_probe(struct platform_device *pdev)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host;
>> +	struct sdhci_host *host;
>> +	struct clk *clk, *axi_clk;
>> +	struct sdhci_xenon_priv *priv;
>> +	int err;
>> +
>> +	host = sdhci_pltfm_init(pdev, &sdhci_xenon_pdata,
>> +				sizeof(struct sdhci_xenon_priv));
>> +	if (IS_ERR(host))
>> +		return PTR_ERR(host);
>> +
>> +	pltfm_host = sdhci_priv(host);
>> +	priv = sdhci_pltfm_priv(pltfm_host);
>> +
>> +	xenon_set_acg(host, false);
>> +
>> +	/*
>> +	 * Link Xenon specific mmc_host_ops function,
>> +	 * to replace standard ones in sdhci_ops.
>> +	 */
>> +	xenon_replace_mmc_host_ops(host);
>> +
>> +	clk = devm_clk_get(&pdev->dev, "core");
>> +	if (IS_ERR(clk)) {
>> +		dev_err(&pdev->dev, "Failed to setup input clk.\n");
>> +		err = PTR_ERR(clk);
>> +		goto free_pltfm;
>> +	}
>> +	clk_prepare_enable(clk);
>> +	pltfm_host->clk = clk;
>> +
>> +	/*
>> +	 * Some SOCs require additional clock to
>> +	 * manage AXI bus clock.
>> +	 * It is optional.
>> +	 */
>> +	axi_clk = devm_clk_get(&pdev->dev, "axi");
>> +	if (!IS_ERR(axi_clk)) {
>> +		clk_prepare_enable(axi_clk);
>> +		priv->axi_clk = axi_clk;
>> +	}
>> +
>> +	err = xenon_probe_dt(pdev);
>> +	if (err)
>> +		goto err_clk;
>> +
>> +	err = xenon_slot_probe(host);
>> +	if (err)
>> +		goto err_clk;
>> +
>> +	err = sdhci_add_host(host);
>> +	if (err)
>> +		goto remove_slot;
>> +
>> +	return 0;
>> +
>> +remove_slot:
>> +	xenon_slot_remove(host);
>> +err_clk:
>> +	clk_disable_unprepare(pltfm_host->clk);
>> +	if (!IS_ERR(axi_clk))
>> +		clk_disable_unprepare(axi_clk);
>> +free_pltfm:
>> +	sdhci_pltfm_free(pdev);
>> +	return err;
>> +}
>> +
>> +static int sdhci_xenon_remove(struct platform_device *pdev)
>> +{
>> +	struct sdhci_host *host = platform_get_drvdata(pdev);
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +	int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xFFFFFFFF);
>> +
>> +	xenon_slot_remove(host);
>> +
>> +	sdhci_remove_host(host, dead);
>> +
>> +	clk_disable_unprepare(pltfm_host->clk);
>> +	clk_disable_unprepare(priv->axi_clk);
>> +
>> +	sdhci_pltfm_free(pdev);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id sdhci_xenon_dt_ids[] = {
>> +	{ .compatible = "marvell,sdhci-xenon",},
>> +	{ .compatible = "marvell,armada-3700-sdhci",},
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids);
>> +
>> +static struct platform_driver sdhci_xenon_driver = {
>> +	.driver	= {
>> +		.name	= "sdhci-xenon",
>> +		.of_match_table = sdhci_xenon_dt_ids,
>> +		.pm = &sdhci_pltfm_pmops,
>> +	},
>> +	.probe	= sdhci_xenon_probe,
>> +	.remove	= sdhci_xenon_remove,
>> +};
>> +
>> +module_platform_driver(sdhci_xenon_driver);
>> +
>> +MODULE_DESCRIPTION("SDHCI platform driver for Marvell Xenon SDHC");
>> +MODULE_AUTHOR("Hu Ziji <huziji@marvell.com>");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/mmc/host/sdhci-xenon.h b/drivers/mmc/host/sdhci-xenon.h
>> new file mode 100644
>> index 000000000000..c2370493fbe8
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-xenon.h
>> @@ -0,0 +1,134 @@
>> +/*
>> + * Copyright (C) 2016 Marvell, All Rights Reserved.
>> + *
>> + * Author:	Hu Ziji <huziji@marvell.com>
>> + * Date:	2016-8-24
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + */
>> +#ifndef SDHCI_XENON_H_
>> +#define SDHCI_XENON_H_
>> +
>> +#include <linux/clk.h>
>> +#include <linux/mmc/card.h>
>> +#include <linux/of.h>
>> +#include "sdhci.h"
>> +
>> +/* Register Offset of SD Host Controller SOCP self-defined register */
>> +#define SDHC_SYS_CFG_INFO			0x0104
>> +#define SLOT_TYPE_SDIO_SHIFT			24
>> +#define SLOT_TYPE_EMMC_MASK			0xFF
>> +#define SLOT_TYPE_EMMC_SHIFT			16
>> +#define SLOT_TYPE_SD_SDIO_MMC_MASK		0xFF
>> +#define SLOT_TYPE_SD_SDIO_MMC_SHIFT		8
>> +#define NR_SUPPORTED_SLOT_MASK			0x7
>> +
>> +#define SDHC_SYS_OP_CTRL			0x0108
>> +#define AUTO_CLKGATE_DISABLE_MASK		BIT(20)
>> +#define SDCLK_IDLEOFF_ENABLE_SHIFT		8
>> +#define SLOT_ENABLE_SHIFT			0
>> +
>> +#define SDHC_SYS_EXT_OP_CTRL			0x010C
>> +#define MASK_CMD_CONFLICT_ERROR			BIT(8)
>> +
>> +#define SDHC_SLOT_OP_STATUS_CTRL		0x0128
>> +#define DELAY_90_DEGREE_MASK_EMMC5		BIT(7)
>> +#define DELAY_90_DEGREE_SHIFT_EMMC5		7
>> +#define EMMC_5_0_PHY_FIXED_DELAY_MASK		0x7F
>> +#define EMMC_PHY_FIXED_DELAY_MASK		0xFF
>> +#define EMMC_PHY_FIXED_DELAY_WINDOW_MIN		(EMMC_PHY_FIXED_DELAY_MASK >> 3)
>> +#define SDH_PHY_FIXED_DELAY_MASK		0x1FF
>> +#define SDH_PHY_FIXED_DELAY_WINDOW_MIN		(SDH_PHY_FIXED_DELAY_MASK >> 4)
>> +
>> +#define TUN_CONSECUTIVE_TIMES_SHIFT		16
>> +#define TUN_CONSECUTIVE_TIMES_MASK		0x7
>> +#define TUN_CONSECUTIVE_TIMES			0x4
>> +#define TUNING_STEP_SHIFT			12
>> +#define TUNING_STEP_MASK			0xF
>> +#define TUNING_STEP_DIVIDER			BIT(6)
>> +
>> +#define FORCE_SEL_INVERSE_CLK_SHIFT		11
>> +
>> +#define SDHC_SLOT_EMMC_CTRL			0x0130
>> +#define ENABLE_DATA_STROBE			BIT(24)
>> +#define SET_EMMC_RSTN				BIT(16)
>> +#define DISABLE_RD_DATA_CRC			BIT(14)
>> +#define DISABLE_CRC_STAT_TOKEN			BIT(13)
>> +#define EMMC_VCCQ_MASK				0x3
>> +#define EMMC_VCCQ_1_8V				0x1
>> +#define EMMC_VCCQ_3_3V				0x3
>> +
>> +#define SDHC_SLOT_RETUNING_REQ_CTRL		0x0144
>> +/* retuning compatible */
>> +#define RETUNING_COMPATIBLE			0x1
>> +
>> +#define SDHC_SLOT_EXT_PRESENT_STATE		0x014C
>> +#define LOCK_STATE				0x1
>> +
>> +#define SDHC_SLOT_DLL_CUR_DLY_VAL		0x0150
>> +
>> +/* Tuning Parameter */
>> +#define TMR_RETUN_NO_PRESENT			0xF
>> +#define DEF_TUNING_COUNT			0x9
>> +
>> +#define MMC_TIMING_FAKE				0xFF
>> +
>> +#define DEFAULT_SDCLK_FREQ			(400000)
>> +
>> +/* Xenon specific Mode Select value */
>> +#define XENON_SDHCI_CTRL_HS200			0x5
>> +#define XENON_SDHCI_CTRL_HS400			0x6
>> +
>> +struct sdhci_xenon_priv {
>> +	/*
>> +	 * The bus_width, timing, and clock fields in below
>> +	 * record the current setting of Xenon SDHC.
>> +	 * Driver will call a Sampling Fixed Delay Adjustment
>> +	 * if any setting is changed.
>> +	 */
>> +	unsigned char	bus_width;
>> +	unsigned char	timing;
>> +	unsigned char	tuning_count;
>> +	unsigned int	clock;
>> +	struct clk	*axi_clk;
>> +
>> +	/* Slot idx */
>> +	u8		slot_idx;
>> +
>> +	/*
>> +	 * When initializing card, Xenon has to determine card type and
>> +	 * adjust Sampling Fixed delay.
>> +	 * However, at that time, card structure is not linked to mmc_host.
>> +	 * Thus a card pointer is added here to provide
>> +	 * the delay adjustment function with the card structure
>> +	 * of the card during initialization
>> +	 */
>> +	struct mmc_card *card_candidate;
>> +};
>> +
>> +static inline int enable_xenon_internal_clk(struct sdhci_host *host)
>> +{
>> +	u32 reg;
>> +	u8 timeout;
>> +
>> +	reg = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
>> +	reg |= SDHCI_CLOCK_INT_EN;
>> +	sdhci_writel(host, reg, SDHCI_CLOCK_CONTROL);
>> +	/* Wait max 20 ms */
>> +	timeout = 20;
>> +	while (!((reg = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
>> +			& SDHCI_CLOCK_INT_STABLE)) {
>> +		if (timeout == 0) {
>> +			pr_err("%s: Internal clock never stabilised.\n",
>> +			       mmc_hostname(host->mmc));
>> +			return -ETIMEDOUT;
>> +		}
>> +		timeout--;
>> +		mdelay(1);
>> +	}
>> +
>> +	return 0;
>> +}
>> +#endif
>>
> 

^ permalink raw reply

* [PATCH RESEND] ARM: dts: keystone-k2*: Increase SPI Flash partition size for U-Boot
From: Russell King - ARM Linux @ 2016-10-12 11:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <c3be6bfc-9cfa-bcd8-5988-ee8bdd9fa506@ti.com>

On Wed, Oct 12, 2016 at 04:30:28PM +0530, Vignesh R wrote:
> Hi,
> 
> On Monday 10 October 2016 08:01 PM, Russell King - ARM Linux wrote:
> > On Mon, Oct 10, 2016 at 07:41:41PM +0530, Vignesh R wrote:
> >> U-Boot SPI Boot image is now more than 512KB for Keystone2 devices and
> >> cannot fit into existing partition. So, increase the SPI Flash partition
> >> for U-Boot to 1MB for all Keystone2 devices.
> >>
> >> Signed-off-by: Vignesh R <vigneshr@ti.com>
> >> ---
> >>
> >> This was submitted to v4.9 merge window but was never picked up:
> >> https://patchwork.kernel.org/patch/9135023/
> > 
> > I think you need to explain why it's safe to change the layout of the
> > flash partitions like this.
> > 
> > - What is this "misc" partition?
> > 
> 
> This partition seems to exists from the very beginning.  I believe, this
> is just a spare area of flash that can be used as per end-user
> requirement. Either to store a small filesystem or kernel. Copying
> Murali who added above partition if he has any input here.
> 
> > - Why is it safe to move the "misc" partition in this way?
> > 
> > - Do users need to do anything with data stored in the "misc" partition
> >   when changing kernels?
> > 
> 
> MTD layer will take care of most abstractions (like start address etc).
> Will add a note in commit message informing about the reduction in size
> of the partition.
> 
> > If the "misc" partition is simply unused space on the flash device, why
> > list it in DT?
> > 
> 
> If the unused space is not listed in the DT, then there is no /dev/mtdX
> node created for the unused section. User will then have to manually
> edit DT, in order to get the node and mount it. Instead, lets make it
> available by default.

So, taken all together, your argument is:

- We want a user partition
- It's okay to destroy the data in the user's partition by moving it
  around randomly between kernel versions.

The two do not naturally go together at all.  You're messing with user
expectations in ways you should not be.  This really is not an acceptable
approach.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [bug report] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver
From: Dan Carpenter @ 2016-10-12 11:32 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Tai Nguyen,

The patch 832c927d119b: "perf: xgene: Add APM X-Gene SoC Performance
Monitoring Unit driver" from Jul 15, 2016, leads to the following
static checker warning:

	drivers/perf/xgene_pmu.c:1014 acpi_get_pmu_hw_inf()
	warn: '&res' isn't an ERR_PTR

drivers/perf/xgene_pmu.c
   992  static struct
   993  xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
   994                                         struct acpi_device *adev, u32 type)
   995  {
   996          struct device *dev = xgene_pmu->dev;
   997          struct list_head resource_list;
   998          struct xgene_pmu_dev_ctx *ctx;
   999          const union acpi_object *obj;
  1000          struct hw_pmu_info *inf;
  1001          void __iomem *dev_csr;
  1002          struct resource res;
  1003          int enable_bit;
  1004          int rc;
  1005  
  1006          ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
  1007          if (!ctx)
  1008                  return NULL;
  1009  
  1010          INIT_LIST_HEAD(&resource_list);
  1011          rc = acpi_dev_get_resources(adev, &resource_list,
  1012                                      acpi_pmu_dev_add_resource, &res);
  1013          acpi_dev_free_resource_list(&resource_list);
  1014          if (rc < 0 || IS_ERR(&res)) {
                                     ^^^^
Obviously this is a stack address and not an error pointer.  I'm not
sure what was intended here.

  1015                  dev_err(dev, "PMU type %d: No resource address found\n", type);
  1016                  goto err;
  1017          }
  1018  


regards,
dan carpenter

^ permalink raw reply

* [PATCH v3 5/5] arm64: mm: round memstart_addr to contiguous PUD/PMD size
From: Ard Biesheuvel @ 2016-10-12 11:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476271425-19401-1-git-send-email-ard.biesheuvel@linaro.org>

Given that contiguous PUDs/PMDs allow the linear region to be mapped more
efficiently, ensure that the relative alignment between the linear virtual
and physical mappings of system RAM are equal modulo the contiguous PUD
size (or contiguous PMD size, for 16k/64k granule size).

Note that this reduces the KASLR randomization of the linear range by
PUD_SHIFT (or PMD_SHIFT) bits.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/include/asm/kernel-pgtable.h | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/asm/kernel-pgtable.h b/arch/arm64/include/asm/kernel-pgtable.h
index 7e51d1b57c0c..71dbd9e1e809 100644
--- a/arch/arm64/include/asm/kernel-pgtable.h
+++ b/arch/arm64/include/asm/kernel-pgtable.h
@@ -83,16 +83,13 @@
 /*
  * To make optimal use of block mappings when laying out the linear
  * mapping, round down the base of physical memory to a size that can
- * be mapped efficiently, i.e., either PUD_SIZE (4k granule) or PMD_SIZE
- * (64k granule), or a multiple that can be mapped using contiguous bits
- * in the page tables: 32 * PMD_SIZE (16k granule)
+ * be mapped efficiently, i.e., either CONT_PUD_SIZE (4k granule) or
+ * CONT_PMD_SIZE (16k/64k granules)
  */
 #if defined(CONFIG_ARM64_4K_PAGES)
-#define ARM64_MEMSTART_SHIFT		PUD_SHIFT
-#elif defined(CONFIG_ARM64_16K_PAGES)
-#define ARM64_MEMSTART_SHIFT		(PMD_SHIFT + 5)
+#define ARM64_MEMSTART_SHIFT		(PUD_SHIFT + CONT_PUD_SHIFT)
 #else
-#define ARM64_MEMSTART_SHIFT		PMD_SHIFT
+#define ARM64_MEMSTART_SHIFT		(PMD_SHIFT + CONT_PMD_SHIFT)
 #endif
 
 /*
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox