* [PATCH v2 11/16] irqchip/irq-mvebu-icu: add support for System Error Interrupts (SEI)
From: Miquel Raynal @ 2018-06-08 13:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cd000f57-e01f-e5b9-77f6-baf4e8f45551@arm.com>
Hi Marc,
Thank you for the review.
On Wed, 23 May 2018 15:23:48 +0100, Marc Zyngier <marc.zyngier@arm.com>
wrote:
> On 22/05/18 10:40, Miquel Raynal wrote:
> > An SEI driver provides an MSI domain through which it is possible to
> > raise SEIs.
> >
> > Handle the NSR probe function in a more generic way to support other
> > type of interrupts (ie. the SEIs).
> >
> > For clarity we do not use tree IRQ domains for now but linear ones
> > instead, allocating the 207 ICU lines for each interrupt group.
>
> What's the rational for not using trees? Because that's effectively a
> 100% overhead...
There is none.
I had a look at how to do it.
In the ICU driver I would like to just drop the nvec parameter (number
of interrupts in the domain) when calling
platform_msi_create_device_domain().
The above function would call irq_domain_create_hierarchy() which would
create a tree domain instead of a linear one because of nvec being 0.
However, there is a check in platform_msi_alloc_priv_data() (also
called by platform_msi_create_device_domain()) that will error out if
nvec is null.
I'm not 100% sure this is safe but I don't see the point of
prohibiting nvec to be null here. So would you accept this
change?
--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -203,7 +203,7 @@ platform_msi_alloc_priv_data(struct device *dev,
unsigned int nvec,
* accordingly (which would impact the max number of MSI
* capable devices).
*/
- if (!dev->msi_domain || !write_msi_msg || !nvec || nvec > MAX_DEV_MSIS)
+ if (!dev->msi_domain || !write_msi_msg || nvec > MAX_DEV_MSIS)
return ERR_PTR(-EINVAL);
if (dev->msi_domain->bus_token != DOMAIN_BUS_PLATFORM_MSI) {
>
> > Reallocating an ICU slot is prevented by the use of an ICU-wide bitmap.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
[...]
> > @@ -131,7 +160,8 @@ static int
> > mvebu_icu_irq_domain_translate(struct irq_domain *d, struct irq_fwspec *fwspec,
> > unsigned long *hwirq, unsigned int *type)
> > {
> > - struct mvebu_icu *icu = platform_msi_get_host_data(d);
> > + struct mvebu_icu_msi_data *msi_data = platform_msi_get_host_data(d);
> > + struct mvebu_icu *icu = msi_data->icu;
> > unsigned int param_count = icu->legacy_bindings ? 3 : 2;
> >
> > /* Check the count of the parameters in dt */
> > @@ -172,7 +202,9 @@ mvebu_icu_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> > int err;
> > unsigned long hwirq;
> > struct irq_fwspec *fwspec = args;
> > - struct mvebu_icu *icu = platform_msi_get_host_data(domain);
> > + struct mvebu_icu_msi_data *msi_data =
> > + platform_msi_get_host_data(domain);
> > + struct mvebu_icu *icu = msi_data->icu;
> > struct mvebu_icu_irq_data *icu_irqd;
> >
> > icu_irqd = kmalloc(sizeof(*icu_irqd), GFP_KERNEL);
> > @@ -186,16 +218,22 @@ mvebu_icu_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> > goto free_irqd;
> > }
> >
> > + spin_lock(&icu->msi_lock);
> > + err = bitmap_allocate_region(icu->msi_bitmap, hwirq, 0);
> > + spin_unlock(&icu->msi_lock);
>
> This (and the freeing counterpart) could deserve a couple of helpers.
Sure.
>
> > + if (err < 0)
> > + goto free_irqd;
> > +
> > if (icu->legacy_bindings)
> > icu_irqd->icu_group = fwspec->param[0];
> > else
> > - icu_irqd->icu_group = ICU_GRP_NSR;
> > + icu_irqd->icu_group = msi_data->subset_data->icu_group;
> > icu_irqd->icu = icu;
> >
> > err = platform_msi_domain_alloc(domain, virq, nr_irqs);
> > if (err) {
> > dev_err(icu->dev, "failed to allocate ICU interrupt in parent domain\n");
> > - goto free_irqd;
> > + goto free_bitmap;
> > }
> >
> > /* Make sure there is no interrupt left pending by the firmware */
[...]
> > @@ -268,9 +332,30 @@ static int mvebu_icu_subset_probe(struct platform_device *pdev)
> > return 0;
> > }
> >
> > +static const struct mvebu_icu_subset_data mvebu_icu_nsr_subset_data = {
> > + .icu_group = ICU_GRP_NSR,
> > + .offset_set_ah = ICU_SETSPI_NSR_AH,
> > + .offset_set_al = ICU_SETSPI_NSR_AL,
> > + .offset_clr_ah = ICU_CLRSPI_NSR_AH,
> > + .offset_clr_al = ICU_CLRSPI_NSR_AL,
> > +};
> > +
> > +static const struct mvebu_icu_subset_data mvebu_icu_sei_subset_data = {
> > + .icu_group = ICU_GRP_SEI,
> > + .offset_set_ah = ICU_SET_SEI_AH,
> > + .offset_set_al = ICU_SET_SEI_AL,
> > + .offset_clr_ah = ICU_CLR_SEI_AH,
> > + .offset_clr_al = ICU_CLR_SEI_AL,
>
> I thought SEI was edge only, given what you do in mvebu_icu_init.
> Confused...
AFAIK, the ICU can produce both level and edge MSI. Currently,
when it comes to SEI, we don't use the .offset_clr_a[hl] entries
because the SEI block expects edge-MSIs, but I thought useful to fill
them anyway. I will remove them both to avoid the confusion.
Thanks,
Miqu?l
^ permalink raw reply
* [PATCHv4 06/10] arm64: add basic pointer authentication support
From: Kristina Martsenko @ 2018-06-08 13:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180503132031.25705-7-mark.rutland@arm.com>
Hi Mark,
On 03/05/18 14:20, Mark Rutland wrote:
> This patch adds basic support for pointer authentication, allowing
> userspace to make use of APIAKey. The kernel maintains an APIAKey value
> for each process (shared by all threads within), which is initialised to
> a random value at exec() time.
>
> To describe that address authentication instructions are available, the
> ID_AA64ISAR0.{APA,API} fields are exposed to userspace. A new hwcap,
> APIA, is added to describe that the kernel manages APIAKey.
>
> Instructions using other keys (APIBKey, APDAKey, APDBKey) are disabled,
> and will behave as NOPs. These may be made use of in future patches.
>
> No support is added for the generic key (APGAKey), though this cannot be
> trapped or made to behave as a NOP. Its presence is not advertised with
> a hwcap.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
> arch/arm64/include/asm/mmu.h | 5 +++
> arch/arm64/include/asm/mmu_context.h | 11 ++++-
> arch/arm64/include/asm/pointer_auth.h | 75 +++++++++++++++++++++++++++++++++++
> arch/arm64/include/uapi/asm/hwcap.h | 1 +
> arch/arm64/kernel/cpufeature.c | 9 +++++
> arch/arm64/kernel/cpuinfo.c | 1 +
> 6 files changed, 101 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/include/asm/pointer_auth.h
>
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index dd320df0d026..f6480ea7b0d5 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -25,10 +25,15 @@
>
> #ifndef __ASSEMBLY__
>
> +#include <asm/pointer_auth.h>
> +
> typedef struct {
> atomic64_t id;
> void *vdso;
> unsigned long flags;
> +#ifdef CONFIG_ARM64_PTR_AUTH
> + struct ptrauth_keys ptrauth_keys;
> +#endif
> } mm_context_t;
>
> /*
> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
> index 39ec0b8a689e..83eadbc6b946 100644
> --- a/arch/arm64/include/asm/mmu_context.h
> +++ b/arch/arm64/include/asm/mmu_context.h
> @@ -168,7 +168,14 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp)
> #define destroy_context(mm) do { } while(0)
> void check_and_switch_context(struct mm_struct *mm, unsigned int cpu);
>
> -#define init_new_context(tsk,mm) ({ atomic64_set(&(mm)->context.id, 0); 0; })
> +static inline int init_new_context(struct task_struct *tsk,
> + struct mm_struct *mm)
> +{
> + atomic64_set(&mm->context.id, 0);
> + mm_ctx_ptrauth_init(&mm->context);
> +
> + return 0;
> +}>
> #ifdef CONFIG_ARM64_SW_TTBR0_PAN
> static inline void update_saved_ttbr0(struct task_struct *tsk,
> @@ -216,6 +223,8 @@ static inline void __switch_mm(struct mm_struct *next)
> return;
> }
>
> + mm_ctx_ptrauth_switch(&next->context);
> +
> check_and_switch_context(next, cpu);
> }
It seems you've removed arch_dup_mmap here (as Catalin suggested [1]),
but forgotten to move the key initialization from init_new_context to
arch_bprm_mm_init. In my tests I'm seeing child processes get different
keys than the parent after a fork().
Kristina
[1] https://lkml.org/lkml/2018/4/25/506
^ permalink raw reply
* [PATCH v2 0/5] add virt-dma support for imx-sdma
From: Robin Gong @ 2018-06-08 13:44 UTC (permalink / raw)
To: linux-arm-kernel
The legacy sdma driver has below limitations or drawbacks:
1. Hardcode the max BDs number as "PAGE_SIZE / sizeof(*)", and alloc
one page size for one channel regardless of only few BDs needed
most time. But in few cases, the max PAGE_SIZE maybe not enough.
2. One SDMA channel can't stop immediatley once channel disabled which
means SDMA interrupt may come in after this channel terminated.There
are some patches for this corner case such as commit "2746e2c389f9",
but not cover non-cyclic.
The common virt-dma overcomes the above limitations. It can alloc bd
dynamically and free bd once this tx transfer done. No memory wasted or
maximum limititation here, only depends on how many memory can be requested
from kernel. For No.2, such issue can be workaround by checking if there
is available descript("sdmac->desc") now once the unwanted interrupt
coming. At last the common virt-dma is easier for sdma driver maintain.
Change from v1:
1. split v1 patch into 5 patches.
2. remove some unnecessary condition check.
3. remove unneccessary 'pending' list.
Robin Gong (5):
dmaengine: imx-sdma: add virt-dma support
Revert "dmaengine: imx-sdma: fix pagefault when channel is disabled
during interrupt"
dmaengine: imx-sdma: remove usless lock
dmaengine: imx-sdma: remove the maximum limation for bd numbers
dmaengine: imx-sdma: add sdma_transfer_init to decrease code overlap
drivers/dma/Kconfig | 1 +
drivers/dma/imx-sdma.c | 392 ++++++++++++++++++++++++++++---------------------
2 files changed, 227 insertions(+), 166 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v2 1/5] dmaengine: imx-sdma: add virt-dma support
From: Robin Gong @ 2018-06-08 13:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528465490-19684-1-git-send-email-yibin.gong@nxp.com>
The legacy sdma driver has below limitations or drawbacks:
1. Hardcode the max BDs number as "PAGE_SIZE / sizeof(*)", and alloc
one page size for one channel regardless of only few BDs needed
most time. But in few cases, the max PAGE_SIZE maybe not enough.
2. One SDMA channel can't stop immediatley once channel disabled which
means SDMA interrupt may come in after this channel terminated.There
are some patches for this corner case such as commit "2746e2c389f9",
but not cover non-cyclic.
The common virt-dma overcomes the above limitations. It can alloc bd
dynamically and free bd once this tx transfer done. No memory wasted or
maximum limititation here, only depends on how many memory can be requested
from kernel. For No.2, such issue can be workaround by checking if there
is available descript("sdmac->desc") now once the unwanted interrupt
coming. At last the common virt-dma is easier for sdma driver maintain.
The main changes as below:
--new "sdma_desc" structure containing virt_dma_desc and some members
which moved from "sdma_channel" such as "num_bd","bd_phys","bd",etc,
since multi descriptors may exist on virtual dma framework
rather than only one as before.
--remove some members of "sdma_channel" structure since it's handled
by virtual dma common framework, such as "tasklet", "dma_chan",etc.
--add specific BD0 for channel0 since such bd descriptor is must and
basic for other dma channel, no need alloc/free as other channel,so
request it during probe.
--remove sdma_request_channel(),sdma_tx_submit(),etc.
--alloc/free bd descriptor added and code changes for virtual dma.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/Kconfig | 1 +
drivers/dma/imx-sdma.c | 332 ++++++++++++++++++++++++++++++++-----------------
2 files changed, 220 insertions(+), 113 deletions(-)
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index ca1680a..d4a4230 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -250,6 +250,7 @@ config IMX_SDMA
tristate "i.MX SDMA support"
depends on ARCH_MXC
select DMA_ENGINE
+ select DMA_VIRTUAL_CHANNELS
help
Support the i.MX SDMA engine. This engine is integrated into
Freescale i.MX25/31/35/51/53/6 chips.
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index ccd03c3..8d0c1fd 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -48,6 +48,7 @@
#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
#include "dmaengine.h"
+#include "virt-dma.h"
/* SDMA registers */
#define SDMA_H_C0PTR 0x000
@@ -296,6 +297,31 @@ struct sdma_context_data {
struct sdma_engine;
/**
+ * struct sdma_desc - descriptor structor for one transfer
+ * @vd descriptor for virt dma
+ * @num_bd max NUM_BD. number of descriptors currently handling
+ * @buf_tail ID of the buffer that was processed
+ * @buf_ptail ID of the previous buffer that was processed
+ * @period_len period length, used in cyclic.
+ * @chn_real_count the real count updated from bd->mode.count
+ * @chn_count the transfer count setuped
+ * @sdmac sdma_channel pointer
+ * @bd pointer of alloced bd
+ */
+struct sdma_desc {
+ struct virt_dma_desc vd;
+ unsigned int num_bd;
+ dma_addr_t bd_phys;
+ unsigned int buf_tail;
+ unsigned int buf_ptail;
+ unsigned int period_len;
+ unsigned int chn_real_count;
+ unsigned int chn_count;
+ struct sdma_channel *sdmac;
+ struct sdma_buffer_descriptor *bd;
+};
+
+/**
* struct sdma_channel - housekeeping for a SDMA channel
*
* @sdma pointer to the SDMA engine for this channel
@@ -305,11 +331,10 @@ struct sdma_engine;
* @event_id0 aka dma request line
* @event_id1 for channels that use 2 events
* @word_size peripheral access size
- * @buf_tail ID of the buffer that was processed
- * @buf_ptail ID of the previous buffer that was processed
- * @num_bd max NUM_BD. number of descriptors currently handling
*/
struct sdma_channel {
+ struct virt_dma_chan vc;
+ struct sdma_desc *desc;
struct sdma_engine *sdma;
unsigned int channel;
enum dma_transfer_direction direction;
@@ -317,12 +342,6 @@ struct sdma_channel {
unsigned int event_id0;
unsigned int event_id1;
enum dma_slave_buswidth word_size;
- unsigned int buf_tail;
- unsigned int buf_ptail;
- unsigned int num_bd;
- unsigned int period_len;
- struct sdma_buffer_descriptor *bd;
- dma_addr_t bd_phys;
unsigned int pc_from_device, pc_to_device;
unsigned int device_to_device;
unsigned long flags;
@@ -330,13 +349,10 @@ struct sdma_channel {
unsigned long event_mask[2];
unsigned long watermark_level;
u32 shp_addr, per_addr;
- struct dma_chan chan;
spinlock_t lock;
- struct dma_async_tx_descriptor desc;
enum dma_status status;
unsigned int chn_count;
unsigned int chn_real_count;
- struct tasklet_struct tasklet;
struct imx_dma_data data;
bool enabled;
};
@@ -398,6 +414,8 @@ struct sdma_engine {
u32 spba_start_addr;
u32 spba_end_addr;
unsigned int irq;
+ dma_addr_t bd0_phys;
+ struct sdma_buffer_descriptor *bd0;
};
static struct sdma_driver_data sdma_imx31 = {
@@ -632,7 +650,7 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
u32 address)
{
- struct sdma_buffer_descriptor *bd0 = sdma->channel[0].bd;
+ struct sdma_buffer_descriptor *bd0 = sdma->bd0;
void *buf_virt;
dma_addr_t buf_phys;
int ret;
@@ -688,6 +706,35 @@ static void sdma_event_disable(struct sdma_channel *sdmac, unsigned int event)
writel_relaxed(val, sdma->regs + chnenbl);
}
+static struct sdma_desc *to_sdma_desc(struct dma_async_tx_descriptor *t)
+{
+ return container_of(t, struct sdma_desc, vd.tx);
+}
+
+static void sdma_start_desc(struct sdma_channel *sdmac)
+{
+ struct virt_dma_desc *vd = vchan_next_desc(&sdmac->vc);
+ struct sdma_desc *desc;
+ struct sdma_engine *sdma = sdmac->sdma;
+ int channel = sdmac->channel;
+
+ if (!vd) {
+ sdmac->desc = NULL;
+ return;
+ }
+ sdmac->desc = desc = to_sdma_desc(&vd->tx);
+ /*
+ * Do not delete the node in desc_issued list in cyclic mode, otherwise
+ * the desc alloced will never be freed in vchan_dma_desc_free_list
+ */
+ if (!(sdmac->flags & IMX_DMA_SG_LOOP))
+ list_del(&vd->node);
+
+ sdma->channel_control[channel].base_bd_ptr = desc->bd_phys;
+ sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
+ sdma_enable_channel(sdma, sdmac->channel);
+}
+
static void sdma_update_channel_loop(struct sdma_channel *sdmac)
{
struct sdma_buffer_descriptor *bd;
@@ -706,8 +753,10 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
* loop mode. Iterate over descriptors, re-setup them and
* call callback function.
*/
- while (1) {
- bd = &sdmac->bd[sdmac->buf_tail];
+ while (sdmac->desc) {
+ struct sdma_desc *desc = sdmac->desc;
+
+ bd = &desc->bd[desc->buf_tail];
if (bd->mode.status & BD_DONE)
break;
@@ -723,11 +772,11 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
* the number of bytes present in the current buffer descriptor.
*/
- sdmac->chn_real_count = bd->mode.count;
+ desc->chn_real_count = bd->mode.count;
bd->mode.status |= BD_DONE;
- bd->mode.count = sdmac->period_len;
- sdmac->buf_ptail = sdmac->buf_tail;
- sdmac->buf_tail = (sdmac->buf_tail + 1) % sdmac->num_bd;
+ bd->mode.count = desc->period_len;
+ desc->buf_ptail = desc->buf_tail;
+ desc->buf_tail = (desc->buf_tail + 1) % desc->num_bd;
/*
* The callback is called from the interrupt context in order
@@ -736,40 +785,36 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
* executed.
*/
- dmaengine_desc_get_callback_invoke(&sdmac->desc, NULL);
+ dmaengine_desc_get_callback_invoke(&desc->vd.tx, NULL);
if (error)
sdmac->status = old_status;
}
}
-static void mxc_sdma_handle_channel_normal(unsigned long data)
+static void mxc_sdma_handle_channel_normal(struct sdma_channel *data)
{
struct sdma_channel *sdmac = (struct sdma_channel *) data;
struct sdma_buffer_descriptor *bd;
int i, error = 0;
- sdmac->chn_real_count = 0;
+ sdmac->desc->chn_real_count = 0;
/*
* non loop mode. Iterate over all descriptors, collect
* errors and call callback function
*/
- for (i = 0; i < sdmac->num_bd; i++) {
- bd = &sdmac->bd[i];
+ for (i = 0; i < sdmac->desc->num_bd; i++) {
+ bd = &sdmac->desc->bd[i];
if (bd->mode.status & (BD_DONE | BD_RROR))
error = -EIO;
- sdmac->chn_real_count += bd->mode.count;
+ sdmac->desc->chn_real_count += bd->mode.count;
}
if (error)
sdmac->status = DMA_ERROR;
else
sdmac->status = DMA_COMPLETE;
-
- dma_cookie_complete(&sdmac->desc);
-
- dmaengine_desc_get_callback_invoke(&sdmac->desc, NULL);
}
static irqreturn_t sdma_int_handler(int irq, void *dev_id)
@@ -785,13 +830,22 @@ static irqreturn_t sdma_int_handler(int irq, void *dev_id)
while (stat) {
int channel = fls(stat) - 1;
struct sdma_channel *sdmac = &sdma->channel[channel];
-
- if (sdmac->flags & IMX_DMA_SG_LOOP)
- sdma_update_channel_loop(sdmac);
- else
- tasklet_schedule(&sdmac->tasklet);
+ struct sdma_desc *desc;
+
+ spin_lock(&sdmac->vc.lock);
+ desc = sdmac->desc;
+ if (desc) {
+ if (sdmac->flags & IMX_DMA_SG_LOOP) {
+ sdma_update_channel_loop(sdmac);
+ } else {
+ mxc_sdma_handle_channel_normal(sdmac);
+ vchan_cookie_complete(&desc->vd);
+ sdma_start_desc(sdmac);
+ }
+ }
__clear_bit(channel, &stat);
+ spin_unlock(&sdmac->vc.lock);
}
return IRQ_HANDLED;
@@ -897,7 +951,7 @@ static int sdma_load_context(struct sdma_channel *sdmac)
int channel = sdmac->channel;
int load_address;
struct sdma_context_data *context = sdma->context;
- struct sdma_buffer_descriptor *bd0 = sdma->channel[0].bd;
+ struct sdma_buffer_descriptor *bd0 = sdma->bd0;
int ret;
unsigned long flags;
@@ -946,7 +1000,7 @@ static int sdma_load_context(struct sdma_channel *sdmac)
static struct sdma_channel *to_sdma_chan(struct dma_chan *chan)
{
- return container_of(chan, struct sdma_channel, chan);
+ return container_of(chan, struct sdma_channel, vc.chan);
}
static int sdma_disable_channel(struct dma_chan *chan)
@@ -968,7 +1022,16 @@ static int sdma_disable_channel(struct dma_chan *chan)
static int sdma_disable_channel_with_delay(struct dma_chan *chan)
{
+ struct sdma_channel *sdmac = to_sdma_chan(chan);
+ unsigned long flags;
+ LIST_HEAD(head);
+
sdma_disable_channel(chan);
+ spin_lock_irqsave(&sdmac->vc.lock, flags);
+ vchan_get_all_descriptors(&sdmac->vc, &head);
+ sdmac->desc = NULL;
+ spin_unlock_irqrestore(&sdmac->vc.lock, flags);
+ vchan_dma_desc_free_list(&sdmac->vc, &head);
/*
* According to NXP R&D team a delay of one BD SDMA cost time
@@ -1097,42 +1160,55 @@ static int sdma_set_channel_priority(struct sdma_channel *sdmac,
return 0;
}
-static int sdma_request_channel(struct sdma_channel *sdmac)
+static int sdma_request_channel0(struct sdma_engine *sdma)
{
- struct sdma_engine *sdma = sdmac->sdma;
- int channel = sdmac->channel;
- int ret = -EBUSY;
+ int ret = 0;
- sdmac->bd = dma_zalloc_coherent(NULL, PAGE_SIZE, &sdmac->bd_phys,
+ sdma->bd0 = dma_zalloc_coherent(NULL, PAGE_SIZE, &sdma->bd0_phys,
GFP_KERNEL);
- if (!sdmac->bd) {
+ if (!sdma->bd0) {
ret = -ENOMEM;
goto out;
}
- sdma->channel_control[channel].base_bd_ptr = sdmac->bd_phys;
- sdma->channel_control[channel].current_bd_ptr = sdmac->bd_phys;
+ sdma->channel_control[0].base_bd_ptr = sdma->bd0_phys;
+ sdma->channel_control[0].current_bd_ptr = sdma->bd0_phys;
- sdma_set_channel_priority(sdmac, MXC_SDMA_DEFAULT_PRIORITY);
+ sdma_set_channel_priority(&sdma->channel[0], MXC_SDMA_DEFAULT_PRIORITY);
return 0;
out:
-
return ret;
}
-static dma_cookie_t sdma_tx_submit(struct dma_async_tx_descriptor *tx)
+static int sdma_alloc_bd(struct sdma_desc *desc)
{
- unsigned long flags;
- struct sdma_channel *sdmac = to_sdma_chan(tx->chan);
- dma_cookie_t cookie;
+ u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
+ int ret = 0;
- spin_lock_irqsave(&sdmac->lock, flags);
+ desc->bd = dma_zalloc_coherent(NULL, bd_size, &desc->bd_phys,
+ GFP_KERNEL);
+ if (!desc->bd) {
+ ret = -ENOMEM;
+ goto out;
+ }
+out:
+
+ return ret;
+}
- cookie = dma_cookie_assign(tx);
+static void sdma_free_bd(struct sdma_desc *desc)
+{
+ u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
- spin_unlock_irqrestore(&sdmac->lock, flags);
+ dma_free_coherent(NULL, bd_size, desc->bd, desc->bd_phys);
+}
+
+static void sdma_desc_free(struct virt_dma_desc *vd)
+{
+ struct sdma_desc *desc = container_of(vd, struct sdma_desc, vd);
- return cookie;
+ sdma_free_bd(desc);
+ kfree(desc);
}
static int sdma_alloc_chan_resources(struct dma_chan *chan)
@@ -1168,19 +1244,10 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan)
if (ret)
goto disable_clk_ipg;
- ret = sdma_request_channel(sdmac);
- if (ret)
- goto disable_clk_ahb;
-
ret = sdma_set_channel_priority(sdmac, prio);
if (ret)
goto disable_clk_ahb;
- dma_async_tx_descriptor_init(&sdmac->desc, chan);
- sdmac->desc.tx_submit = sdma_tx_submit;
- /* txd.flags will be overwritten in prep funcs */
- sdmac->desc.flags = DMA_CTRL_ACK;
-
return 0;
disable_clk_ahb:
@@ -1195,7 +1262,7 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
struct sdma_channel *sdmac = to_sdma_chan(chan);
struct sdma_engine *sdma = sdmac->sdma;
- sdma_disable_channel(chan);
+ sdma_disable_channel_with_delay(chan);
if (sdmac->event_id0)
sdma_event_disable(sdmac, sdmac->event_id0);
@@ -1207,8 +1274,6 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
sdma_set_channel_priority(sdmac, 0);
- dma_free_coherent(NULL, PAGE_SIZE, sdmac->bd, sdmac->bd_phys);
-
clk_disable(sdma->clk_ipg);
clk_disable(sdma->clk_ahb);
}
@@ -1223,6 +1288,7 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
int ret, i, count;
int channel = sdmac->channel;
struct scatterlist *sg;
+ struct sdma_desc *desc;
if (sdmac->status == DMA_IN_PROGRESS)
return NULL;
@@ -1230,9 +1296,20 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
sdmac->flags = 0;
- sdmac->buf_tail = 0;
- sdmac->buf_ptail = 0;
- sdmac->chn_real_count = 0;
+ desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+ if (!desc)
+ goto err_out;
+
+ desc->buf_tail = 0;
+ desc->buf_ptail = 0;
+ desc->sdmac = sdmac;
+ desc->num_bd = sg_len;
+ desc->chn_real_count = 0;
+
+ if (sdma_alloc_bd(desc)) {
+ kfree(desc);
+ goto err_out;
+ }
dev_dbg(sdma->dev, "setting up %d entries for channel %d.\n",
sg_len, channel);
@@ -1240,18 +1317,18 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
sdmac->direction = direction;
ret = sdma_load_context(sdmac);
if (ret)
- goto err_out;
+ goto err_bd_out;
if (sg_len > NUM_BD) {
dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
channel, sg_len, NUM_BD);
ret = -EINVAL;
- goto err_out;
+ goto err_bd_out;
}
- sdmac->chn_count = 0;
+ desc->chn_count = 0;
for_each_sg(sgl, sg, sg_len, i) {
- struct sdma_buffer_descriptor *bd = &sdmac->bd[i];
+ struct sdma_buffer_descriptor *bd = &desc->bd[i];
int param;
bd->buffer_addr = sg->dma_address;
@@ -1262,33 +1339,33 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
dev_err(sdma->dev, "SDMA channel %d: maximum bytes for sg entry exceeded: %d > %d\n",
channel, count, 0xffff);
ret = -EINVAL;
- goto err_out;
+ goto err_bd_out;
}
bd->mode.count = count;
- sdmac->chn_count += count;
+ desc->chn_count += count;
if (sdmac->word_size > DMA_SLAVE_BUSWIDTH_4_BYTES) {
ret = -EINVAL;
- goto err_out;
+ goto err_bd_out;
}
switch (sdmac->word_size) {
case DMA_SLAVE_BUSWIDTH_4_BYTES:
bd->mode.command = 0;
if (count & 3 || sg->dma_address & 3)
- return NULL;
+ goto err_bd_out;
break;
case DMA_SLAVE_BUSWIDTH_2_BYTES:
bd->mode.command = 2;
if (count & 1 || sg->dma_address & 1)
- return NULL;
+ goto err_bd_out;
break;
case DMA_SLAVE_BUSWIDTH_1_BYTE:
bd->mode.command = 1;
break;
default:
- return NULL;
+ goto err_bd_out;
}
param = BD_DONE | BD_EXTD | BD_CONT;
@@ -1307,10 +1384,10 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
bd->mode.status = param;
}
- sdmac->num_bd = sg_len;
- sdma->channel_control[channel].current_bd_ptr = sdmac->bd_phys;
-
- return &sdmac->desc;
+ return vchan_tx_prep(&sdmac->vc, &desc->vd, flags);
+err_bd_out:
+ sdma_free_bd(desc);
+ kfree(desc);
err_out:
sdmac->status = DMA_ERROR;
return NULL;
@@ -1326,6 +1403,7 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
int num_periods = buf_len / period_len;
int channel = sdmac->channel;
int ret, i = 0, buf = 0;
+ struct sdma_desc *desc;
dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
@@ -1334,31 +1412,43 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
sdmac->status = DMA_IN_PROGRESS;
- sdmac->buf_tail = 0;
- sdmac->buf_ptail = 0;
- sdmac->chn_real_count = 0;
- sdmac->period_len = period_len;
+ desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+ if (!desc)
+ goto err_out;
+
+ desc->buf_tail = 0;
+ desc->buf_ptail = 0;
+ desc->sdmac = sdmac;
+ desc->num_bd = num_periods;
+ desc->chn_real_count = 0;
+ desc->period_len = period_len;
sdmac->flags |= IMX_DMA_SG_LOOP;
sdmac->direction = direction;
+
+ if (sdma_alloc_bd(desc)) {
+ kfree(desc);
+ goto err_out;
+ }
+
ret = sdma_load_context(sdmac);
if (ret)
- goto err_out;
+ goto err_bd_out;
if (num_periods > NUM_BD) {
dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
channel, num_periods, NUM_BD);
- goto err_out;
+ goto err_bd_out;
}
if (period_len > 0xffff) {
dev_err(sdma->dev, "SDMA channel %d: maximum period size exceeded: %zu > %d\n",
channel, period_len, 0xffff);
- goto err_out;
+ goto err_bd_out;
}
while (buf < buf_len) {
- struct sdma_buffer_descriptor *bd = &sdmac->bd[i];
+ struct sdma_buffer_descriptor *bd = &desc->bd[i];
int param;
bd->buffer_addr = dma_addr;
@@ -1366,7 +1456,7 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
bd->mode.count = period_len;
if (sdmac->word_size > DMA_SLAVE_BUSWIDTH_4_BYTES)
- goto err_out;
+ goto err_bd_out;
if (sdmac->word_size == DMA_SLAVE_BUSWIDTH_4_BYTES)
bd->mode.command = 0;
else
@@ -1389,10 +1479,10 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
i++;
}
- sdmac->num_bd = num_periods;
- sdma->channel_control[channel].current_bd_ptr = sdmac->bd_phys;
-
- return &sdmac->desc;
+ return vchan_tx_prep(&sdmac->vc, &desc->vd, flags);
+err_bd_out:
+ sdma_free_bd(desc);
+ kfree(desc);
err_out:
sdmac->status = DMA_ERROR;
return NULL;
@@ -1432,12 +1522,30 @@ static enum dma_status sdma_tx_status(struct dma_chan *chan,
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
u32 residue;
+ struct virt_dma_desc *vd;
+ struct sdma_desc *desc;
+ enum dma_status ret;
+ unsigned long flags;
- if (sdmac->flags & IMX_DMA_SG_LOOP)
- residue = (sdmac->num_bd - sdmac->buf_ptail) *
- sdmac->period_len - sdmac->chn_real_count;
- else
- residue = sdmac->chn_count - sdmac->chn_real_count;
+ ret = dma_cookie_status(chan, cookie, txstate);
+ if (ret == DMA_COMPLETE || !txstate)
+ return ret;
+
+ spin_lock_irqsave(&sdmac->vc.lock, flags);
+ vd = vchan_find_desc(&sdmac->vc, cookie);
+ if (vd) {
+ desc = to_sdma_desc(&vd->tx);
+ if (sdmac->flags & IMX_DMA_SG_LOOP)
+ residue = (desc->num_bd - desc->buf_ptail) *
+ desc->period_len - desc->chn_real_count;
+ else
+ residue = desc->chn_count - desc->chn_real_count;
+ } else if (sdmac->desc && sdmac->desc->vd.tx.cookie == cookie) {
+ residue = sdmac->desc->chn_count - sdmac->desc->chn_real_count;
+ } else {
+ residue = 0;
+ }
+ spin_unlock_irqrestore(&sdmac->vc.lock, flags);
dma_set_tx_state(txstate, chan->completed_cookie, chan->cookie,
residue);
@@ -1448,10 +1556,12 @@ static enum dma_status sdma_tx_status(struct dma_chan *chan,
static void sdma_issue_pending(struct dma_chan *chan)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
- struct sdma_engine *sdma = sdmac->sdma;
+ unsigned long flags;
- if (sdmac->status == DMA_IN_PROGRESS)
- sdma_enable_channel(sdma, sdmac->channel);
+ spin_lock_irqsave(&sdmac->vc.lock, flags);
+ if (vchan_issue_pending(&sdmac->vc) && !sdmac->desc)
+ sdma_start_desc(sdmac);
+ spin_unlock_irqrestore(&sdmac->vc.lock, flags);
}
#define SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V1 34
@@ -1657,7 +1767,7 @@ static int sdma_init(struct sdma_engine *sdma)
for (i = 0; i < MAX_DMA_CHANNELS; i++)
writel_relaxed(0, sdma->regs + SDMA_CHNPRI_0 + i * 4);
- ret = sdma_request_channel(&sdma->channel[0]);
+ ret = sdma_request_channel0(sdma);
if (ret)
goto err_dma_alloc;
@@ -1821,20 +1931,15 @@ static int sdma_probe(struct platform_device *pdev)
sdmac->sdma = sdma;
spin_lock_init(&sdmac->lock);
- sdmac->chan.device = &sdma->dma_device;
- dma_cookie_init(&sdmac->chan);
sdmac->channel = i;
-
- tasklet_init(&sdmac->tasklet, mxc_sdma_handle_channel_normal,
- (unsigned long) sdmac);
+ sdmac->vc.desc_free = sdma_desc_free;
/*
* Add the channel to the DMAC list. Do not add channel 0 though
* because we need it internally in the SDMA driver. This also means
* that channel 0 in dmaengine counting matches sdma channel 1.
*/
if (i)
- list_add_tail(&sdmac->chan.device_node,
- &sdma->dma_device.channels);
+ vchan_init(&sdmac->vc, &sdma->dma_device);
}
ret = sdma_init(sdma);
@@ -1939,7 +2044,8 @@ static int sdma_remove(struct platform_device *pdev)
for (i = 0; i < MAX_DMA_CHANNELS; i++) {
struct sdma_channel *sdmac = &sdma->channel[i];
- tasklet_kill(&sdmac->tasklet);
+ tasklet_kill(&sdmac->vc.task);
+ sdma_free_chan_resources(&sdmac->vc.chan);
}
platform_set_drvdata(pdev, NULL);
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/5] Revert "dmaengine: imx-sdma: fix pagefault when channel is disabled during interrupt"
From: Robin Gong @ 2018-06-08 13:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528465490-19684-1-git-send-email-yibin.gong@nxp.com>
This reverts commit 2746e2c389f9d50043d21e2204270403efb9d62f.
Don't need this patch anymore,since we can easily check 'sdmac->desc' to avoid
handling dma interrupt after channel disabled if virt-dma used.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 21 ---------------------
1 file changed, 21 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 8d0c1fd..d93b58f 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -354,7 +354,6 @@ struct sdma_channel {
unsigned int chn_count;
unsigned int chn_real_count;
struct imx_dma_data data;
- bool enabled;
};
#define IMX_DMA_SG_LOOP BIT(0)
@@ -615,14 +614,7 @@ static int sdma_config_ownership(struct sdma_channel *sdmac,
static void sdma_enable_channel(struct sdma_engine *sdma, int channel)
{
- unsigned long flags;
- struct sdma_channel *sdmac = &sdma->channel[channel];
-
writel(BIT(channel), sdma->regs + SDMA_H_START);
-
- spin_lock_irqsave(&sdmac->lock, flags);
- sdmac->enabled = true;
- spin_unlock_irqrestore(&sdmac->lock, flags);
}
/*
@@ -740,14 +732,6 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
struct sdma_buffer_descriptor *bd;
int error = 0;
enum dma_status old_status = sdmac->status;
- unsigned long flags;
-
- spin_lock_irqsave(&sdmac->lock, flags);
- if (!sdmac->enabled) {
- spin_unlock_irqrestore(&sdmac->lock, flags);
- return;
- }
- spin_unlock_irqrestore(&sdmac->lock, flags);
/*
* loop mode. Iterate over descriptors, re-setup them and
@@ -1008,15 +992,10 @@ static int sdma_disable_channel(struct dma_chan *chan)
struct sdma_channel *sdmac = to_sdma_chan(chan);
struct sdma_engine *sdma = sdmac->sdma;
int channel = sdmac->channel;
- unsigned long flags;
writel_relaxed(BIT(channel), sdma->regs + SDMA_H_STATSTOP);
sdmac->status = DMA_ERROR;
- spin_lock_irqsave(&sdmac->lock, flags);
- sdmac->enabled = false;
- spin_unlock_irqrestore(&sdmac->lock, flags);
-
return 0;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v2 3/5] dmaengine: imx-sdma: remove usless lock
From: Robin Gong @ 2018-06-08 13:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528465490-19684-1-git-send-email-yibin.gong@nxp.com>
No need anymore for 'lock' now since virtual dma will provide
the common lock instead.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index d93b58f..6faec89 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -349,7 +349,6 @@ struct sdma_channel {
unsigned long event_mask[2];
unsigned long watermark_level;
u32 shp_addr, per_addr;
- spinlock_t lock;
enum dma_status status;
unsigned int chn_count;
unsigned int chn_real_count;
@@ -1908,7 +1907,6 @@ static int sdma_probe(struct platform_device *pdev)
struct sdma_channel *sdmac = &sdma->channel[i];
sdmac->sdma = sdma;
- spin_lock_init(&sdmac->lock);
sdmac->channel = i;
sdmac->vc.desc_free = sdma_desc_free;
--
2.7.4
^ permalink raw reply related
* [PATCH v2 4/5] dmaengine: imx-sdma: remove the maximum limation for bd numbers
From: Robin Gong @ 2018-06-08 13:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528465490-19684-1-git-send-email-yibin.gong@nxp.com>
No this limitation now after virtual dma used since bd is allocated
dynamically instead of static.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 6faec89..afaee72 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -292,7 +292,6 @@ struct sdma_context_data {
u32 scratch7;
} __attribute__ ((packed));
-#define NUM_BD (int)(PAGE_SIZE / sizeof(struct sdma_buffer_descriptor))
struct sdma_engine;
@@ -1297,13 +1296,6 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
if (ret)
goto err_bd_out;
- if (sg_len > NUM_BD) {
- dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
- channel, sg_len, NUM_BD);
- ret = -EINVAL;
- goto err_bd_out;
- }
-
desc->chn_count = 0;
for_each_sg(sgl, sg, sg_len, i) {
struct sdma_buffer_descriptor *bd = &desc->bd[i];
@@ -1413,12 +1405,6 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
if (ret)
goto err_bd_out;
- if (num_periods > NUM_BD) {
- dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
- channel, num_periods, NUM_BD);
- goto err_bd_out;
- }
-
if (period_len > 0xffff) {
dev_err(sdma->dev, "SDMA channel %d: maximum period size exceeded: %zu > %d\n",
channel, period_len, 0xffff);
--
2.7.4
^ permalink raw reply related
* [PATCH v2 5/5] dmaengine: imx-sdma: add sdma_transfer_init to decrease code overlap
From: Robin Gong @ 2018-06-08 13:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528465490-19684-1-git-send-email-yibin.gong@nxp.com>
There are lot of codes overlap between prep_sg and prep_cyclic function.
Add sdma_transfer_init() function to elimated the code overlap.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 83 ++++++++++++++++++++++----------------------------
1 file changed, 37 insertions(+), 46 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index afaee72..25de89e 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1255,6 +1255,40 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
clk_disable(sdma->clk_ahb);
}
+static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
+ enum dma_transfer_direction direction, u32 bds)
+{
+ struct sdma_desc *desc;
+
+ desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+ if (!desc)
+ goto err_out;
+
+ sdmac->status = DMA_IN_PROGRESS;
+ sdmac->direction = direction;
+ sdmac->flags = 0;
+
+ desc->chn_count = 0;
+ desc->chn_real_count = 0;
+ desc->buf_tail = 0;
+ desc->buf_ptail = 0;
+ desc->sdmac = sdmac;
+ desc->num_bd = bds;
+
+ if (sdma_alloc_bd(desc))
+ goto err_desc_out;
+
+ if (sdma_load_context(sdmac))
+ goto err_desc_out;
+
+ return desc;
+
+err_desc_out:
+ kfree(desc);
+err_out:
+ return NULL;
+}
+
static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
struct dma_chan *chan, struct scatterlist *sgl,
unsigned int sg_len, enum dma_transfer_direction direction,
@@ -1267,36 +1301,13 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
struct scatterlist *sg;
struct sdma_desc *desc;
- if (sdmac->status == DMA_IN_PROGRESS)
- return NULL;
- sdmac->status = DMA_IN_PROGRESS;
-
- sdmac->flags = 0;
-
- desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+ desc = sdma_transfer_init(sdmac, direction, sg_len);
if (!desc)
goto err_out;
- desc->buf_tail = 0;
- desc->buf_ptail = 0;
- desc->sdmac = sdmac;
- desc->num_bd = sg_len;
- desc->chn_real_count = 0;
-
- if (sdma_alloc_bd(desc)) {
- kfree(desc);
- goto err_out;
- }
-
dev_dbg(sdma->dev, "setting up %d entries for channel %d.\n",
sg_len, channel);
- sdmac->direction = direction;
- ret = sdma_load_context(sdmac);
- if (ret)
- goto err_bd_out;
-
- desc->chn_count = 0;
for_each_sg(sgl, sg, sg_len, i) {
struct sdma_buffer_descriptor *bd = &desc->bd[i];
int param;
@@ -1372,38 +1383,18 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
struct sdma_engine *sdma = sdmac->sdma;
int num_periods = buf_len / period_len;
int channel = sdmac->channel;
- int ret, i = 0, buf = 0;
+ int i = 0, buf = 0;
struct sdma_desc *desc;
dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
- if (sdmac->status == DMA_IN_PROGRESS)
- return NULL;
- sdmac->status = DMA_IN_PROGRESS;
-
- desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+ desc = sdma_transfer_init(sdmac, direction, num_periods);
if (!desc)
goto err_out;
-
- desc->buf_tail = 0;
- desc->buf_ptail = 0;
- desc->sdmac = sdmac;
- desc->num_bd = num_periods;
- desc->chn_real_count = 0;
desc->period_len = period_len;
sdmac->flags |= IMX_DMA_SG_LOOP;
- sdmac->direction = direction;
-
- if (sdma_alloc_bd(desc)) {
- kfree(desc);
- goto err_out;
- }
-
- ret = sdma_load_context(sdmac);
- if (ret)
- goto err_bd_out;
if (period_len > 0xffff) {
dev_err(sdma->dev, "SDMA channel %d: maximum period size exceeded: %zu > %d\n",
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/5] arm64: dts: renesas: r8a77980: add VSPD support
From: Simon Horman @ 2018-06-08 13:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5f151ec5-bec7-ed06-cb94-4f0cfe8e43dc@cogentembedded.com>
[CC Laurent, Geert]
On Thu, Jun 07, 2018 at 11:20:47PM +0300, Sergei Shtylyov wrote:
> Describe VSPD0 in the R8A77980 device tree; it will be used by DU in
> the next patch...
>
> Based on the original (and large) patch by Vladimir Barinov.
>
> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> arch/arm64/boot/dts/renesas/r8a77980.dtsi | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> ===================================================================
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -653,6 +653,16 @@
> resets = <&cpg 408>;
> };
>
> + vspd0: vsp at fea20000 {
> + compatible = "renesas,vsp2";
> + reg = <0 0xfea20000 0 0x4000>;
As per "[PATCH] arm64: dts: renesas: Fix VSPD registers range"
I think the width of the range should be 0x5000.
> + interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cpg CPG_MOD 623>;
> + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> + resets = <&cpg 623>;
> + renesas,fcp = <&fcpvd0>;
> + };
> +
> fcpvd0: fcp at fea27000 {
> compatible = "renesas,fcpv";
> reg = <0 0xfea27000 0 0x200>;
>
^ permalink raw reply
* [PATCH v2 12/16] dt-bindings/interrupt-controller: update Marvell ICU bindings
From: Miquel Raynal @ 2018-06-08 14:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180605202902.GA8875@rob-hp-laptop>
Hi Rob,
Thanks for reviewing.
On Tue, 5 Jun 2018 14:29:02 -0600, Rob Herring <robh@kernel.org> wrote:
> On Tue, May 22, 2018 at 11:40:38AM +0200, Miquel Raynal wrote:
> > Change the documentation to reflect the new bindings used for Marvell
> > ICU. This involves describing each interrupt group as a subnode of the
> > ICU node. Each of them having their own compatible.
>
> Need to explain why you need to do this and why breaking backwards
> compatibility is okay.
As explained by Thomas, backward compatibility is not broken as old
bindings are still documented and supported.
I will update the commit message to reflect that point.
>
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > .../bindings/interrupt-controller/marvell,icu.txt | 81 ++++++++++++++++++----
> > 1 file changed, 69 insertions(+), 12 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/marvell,icu.txt b/Documentation/devicetree/bindings/interrupt-controller/marvell,icu.txt
> > index 649b7ec9d9b1..6f7e4355b3d8 100644
> > --- a/Documentation/devicetree/bindings/interrupt-controller/marvell,icu.txt
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/marvell,icu.txt
> > @@ -5,6 +5,8 @@ The Marvell ICU (Interrupt Consolidation Unit) controller is
> > responsible for collecting all wired-interrupt sources in the CP and
> > communicating them to the GIC in the AP, the unit translates interrupt
> > requests on input wires to MSG memory mapped transactions to the GIC.
> > +These messages will access a different GIC memory area depending on
> > +their type (NSR, SR, SEI, REI, etc).
> >
> > Required properties:
> >
> > @@ -12,20 +14,19 @@ Required properties:
> >
> > - reg: Should contain ICU registers location and length.
> >
> > +Subnodes: Each group of interrupt is declared as a subnode of the ICU,
> > +with their own compatible.
> > +
> > +Required properties for the icu_nsr/icu_sei subnodes:
> > +
> > +- compatible: Should be "marvell,cp110-icu-nsr" or "marvell,cp110-icu-sei".
> > +
> > - #interrupt-cells: Specifies the number of cells needed to encode an
> > - interrupt source. The value shall be 3.
> > + interrupt source. The value shall be 2.
> >
> > - The 1st cell is the group type of the ICU interrupt. Possible group
> > - types are:
> > + The 1st cell is the index of the interrupt in the ICU unit.
> >
> > - ICU_GRP_NSR (0x0) : Shared peripheral interrupt, non-secure
> > - ICU_GRP_SR (0x1) : Shared peripheral interrupt, secure
> > - ICU_GRP_SEI (0x4) : System error interrupt
> > - ICU_GRP_REI (0x5) : RAM error interrupt
>
> What happens to SR and REI interrupts?
They were unused since the beginning.
These values are still detailed below (in the legacy section).
[...]
>
> > -
> > - The 2nd cell is the index of the interrupt in the ICU unit.
> > -
> > - The 3rd cell is the type of the interrupt. See arm,gic.txt for
> > + The 2nd cell is the type of the interrupt. See arm,gic.txt for
> > details.
> >
> > - interrupt-controller: Identifies the node as an interrupt
> > @@ -35,17 +36,73 @@ Required properties:
> > that allows to trigger interrupts using MSG memory mapped
> > transactions.
> >
> > +Note: each 'interrupts' property referring to any 'icu_xxx' node shall
> > + have a different number within [0:206].
> > +
> > Example:
> >
> > icu: interrupt-controller at 1e0000 {
> > compatible = "marvell,cp110-icu";
> > reg = <0x1e0000 0x440>;
> > +
> > + CP110_LABEL(icu_nsr): icu-nsr {
>
> 'interrupt-controller' is the proper node name. Is there no register
> range associated sub nodes?
I will update the name.
A few are used only for NSR, a few only for SEI and others are used for
both. But ok, I will add register ranges.
>
> > + compatible = "marvell,cp110-icu-nsr";
> > + #interrupt-cells = <2>;
> > + interrupt-controller;
> > + msi-parent = <&gicp>;
> > + };
> > +
> > + CP110_LABEL(icu_sei): icu-sei {
> > + compatible = "marvell,cp110-icu-sei";
> > + #interrupt-cells = <2>;
> > + interrupt-controller;
> > + msi-parent = <&sei>;
> > + };
>
> Mixture of tabs and spaces.
Oops.
>
> > +};
> > +
Thanks,
Miqu?l
^ permalink raw reply
* [PATCH v2 2/5] arm64: dts: renesas: r8a77980: add VSPD support
From: Laurent Pinchart @ 2018-06-08 14:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180608135456.cathgcuyp24csbwx@verge.net.au>
Hello Simon,
On Friday, 8 June 2018 16:54:56 EEST Simon Horman wrote:
> [CC Laurent, Geert]
>
> On Thu, Jun 07, 2018 at 11:20:47PM +0300, Sergei Shtylyov wrote:
> > Describe VSPD0 in the R8A77980 device tree; it will be used by DU in
> > the next patch...
> >
> > Based on the original (and large) patch by Vladimir Barinov.
> >
> > Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >
> > ---
> >
> > arch/arm64/boot/dts/renesas/r8a77980.dtsi | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> > ===================================================================
> > --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> > +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> > @@ -653,6 +653,16 @@
> > resets = <&cpg 408>;
> > };
> >
> > + vspd0: vsp at fea20000 {
> > + compatible = "renesas,vsp2";
> > + reg = <0 0xfea20000 0 0x4000>;
>
> As per "[PATCH] arm64: dts: renesas: Fix VSPD registers range"
> I think the width of the range should be 0x5000.
I agree with that.
> > + interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&cpg CPG_MOD 623>;
> > + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> > + resets = <&cpg 623>;
> > + renesas,fcp = <&fcpvd0>;
> > + };
> > +
> > fcpvd0: fcp at fea27000 {
> > compatible = "renesas,fcpv";
> > reg = <0 0xfea27000 0 0x200>;
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v2 3/5] arm64: dts: renesas: r8a77980: add DU support
From: Laurent Pinchart @ 2018-06-08 14:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <842200af-095f-7096-a542-1d2865543264@cogentembedded.com>
Hi Sergei,
Thank you for the patch.
On Thursday, 7 June 2018 23:21:38 EEST Sergei Shtylyov wrote:
> Define the generic R8A77980 part of the DU device node.
>
> Based on the original (and large) patch by Vladimir Barinov.
>
> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm64/boot/dts/renesas/r8a77980.dtsi | 30 ++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> ===================================================================
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -671,6 +671,36 @@
> resets = <&cpg 603>;
> };
>
> + du: display at feb00000 {
> + compatible = "renesas,du-r8a77980",
> + "renesas,du-r8a77970";
> + reg = <0 0xfeb00000 0 0x80000>;
> + interrupts = <GIC_SPI 256 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cpg CPG_MOD 724>;
> + clock-names = "du.0";
> + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> + resets = <&cpg 724>;
> + vsps = <&vspd0>;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port at 0 {
> + reg = <0>;
> + du_out_rgb: endpoint {
> + };
> + };
> +
> + port at 1 {
> + reg = <1>;
> + du_out_lvds0: endpoint {
> + };
> + };
> + };
> + };
> +
> prr: chipid at fff00044 {
> compatible = "renesas,prr";
> reg = <0 0xfff00044 0 4>;
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v2 1/5] arm64: dts: renesas: r8a77980: add FCPVD support
From: Laurent Pinchart @ 2018-06-08 14:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <73f61c34-d0ec-75cd-b6fd-a41cfb39078f@cogentembedded.com>
Hi Sergei,
Thank you for the patch.
On Thursday, 7 June 2018 23:19:31 EEST Sergei Shtylyov wrote:
> Describe FCPVD0 in the R8A77980 device tree; it will be used by VSPD0 in
> the next patch...
>
> Based on the original (and large) patch by Vladimir Barinov.
>
> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm64/boot/dts/renesas/r8a77980.dtsi | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> ===================================================================
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -653,6 +653,14 @@
> resets = <&cpg 408>;
> };
>
> + fcpvd0: fcp at fea27000 {
> + compatible = "renesas,fcpv";
> + reg = <0 0xfea27000 0 0x200>;
> + clocks = <&cpg CPG_MOD 603>;
> + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> + resets = <&cpg 603>;
> + };
> +
> prr: chipid at fff00044 {
> compatible = "renesas,prr";
> reg = <0 0xfff00044 0 4>;
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH 1/2] reset: imx7: Fix always writing bits as 0
From: Lucas Stach @ 2018-06-08 14:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c441872d7dd355ae09e945447441389e93e3b888.1527621510.git.leonard.crestez@nxp.com>
Am Dienstag, den 29.05.2018, 22:39 +0300 schrieb Leonard Crestez:
> Right now the only user of reset-imx7 is pci-imx6 and the
> reset_control_assert and deassert calls on pciephy_reset don't toggle
> the PCIEPHY_BTN and PCIEPHY_G_RST bits as expected. Fix this by writing
> 1 or 0 respectively.
>
> The reference manual is not very clear regarding SRC_PCIEPHY_RCR but for
> other registers like MIPIPHY and HSICPHY the bits are explicitly
> documented as "1 means assert, 0 means deassert".
>
> The values are still reversed for IMX7_RESET_PCIE_CTRL_APPS_EN.
>
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> ?drivers/reset/reset-imx7.c | 2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> index 4db177bc89bc..fdeac1946429 100644
> --- a/drivers/reset/reset-imx7.c
> +++ b/drivers/reset/reset-imx7.c
> @@ -78,11 +78,11 @@ static struct imx7_src *to_imx7_src(struct reset_controller_dev *rcdev)
> ?static int imx7_reset_set(struct reset_controller_dev *rcdev,
> > ? ??unsigned long id, bool assert)
> ?{
> > ? struct imx7_src *imx7src = to_imx7_src(rcdev);
> > ? const struct imx7_src_signal *signal = &imx7_src_signals[id];
> > - unsigned int value = 0;
> > + unsigned int value = assert ? signal->bit : 0;
> ?
> > ? switch (id) {
> > ? case IMX7_RESET_PCIEPHY:
> > ? /*
> > ? ?* wait for more than 10us to release phy g_rst and
^ permalink raw reply
* [PATCH v2 4/5] arm64: dts: renesas: r8a77980: add LVDS support
From: Laurent Pinchart @ 2018-06-08 14:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3e46c70c-c6be-d019-95a9-bf09c888c9c0@cogentembedded.com>
Hi Sergei,
Thank you for the patch.
On Thursday, 7 June 2018 23:23:06 EEST Sergei Shtylyov wrote:
> Define the generic R8A77980 part of the LVDS device node.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm64/boot/dts/renesas/r8a77980.dtsi | 29 ++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> ===================================================================
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -696,6 +696,35 @@
> port at 1 {
> reg = <1>;
> du_out_lvds0: endpoint {
> + remote-endpoint = <&lvds0_in>;
> + };
> + };
> + };
> + };
> +
> + lvds0: lvds-encoder at feb90000 {
> + compatible = "renesas,r8a77980-lvds";
> + reg = <0 0xfeb90000 0 0x14>;
> + clocks = <&cpg CPG_MOD 727>;
> + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> + resets = <&cpg 727>;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port at 0 {
> + reg = <0>;
> + lvds0_in: endpoint {
> + remote-endpoint =
> + <&du_out_lvds0>;
> + };
> + };
> +
> + port at 1 {
> + reg = <1>;
> + lvds0_out: endpoint {
> };
> };
> };
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v2 0/5] Add R8A77980/Condor/V3HSK LVDS/HDMI support
From: Laurent Pinchart @ 2018-06-08 14:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <390fabf0-4caf-1600-3780-1893207d94f7@cogentembedded.com>
Hi Sergei,
On Thursday, 7 June 2018 23:17:03 EEST Sergei Shtylyov wrote:
> Hello!
>
> Here's the set of 5 patches against Simon Horman's 'renesas.git' repo's
> 'renesas-devel-20180604-v4.17' tag. We're adding the R8A77980 FCPVD/VSPD/
> DU/LVDS device nodes and then describing the LVDS decoder and HDMI encoder
> connected to the LVDS output. These patches depend on the Thine THC63LVD1024
> driver and the R8A77980 LVDS support patch in order to work, and R8A77980
> GPIO DT patches in order to apply/compile...
>
> [1/5] arm64: dts: renesas: r8a77980: add FCPVD support
> [2/5] arm64: dts: renesas: r8a77980: add VSPD support
> [3/5] arm64: dts: renesas: r8a77980: add DU support
> [4/5] arm64: dts: renesas: r8a77980: add LVDS support
Based on the recent request of the ARM SoC maintainers to avoid a plethora of
small patches, I think you can squash 1/5 to 4/5 all together.
> [5/5] arm64: dts: renesas: condor/v3hsk: add DU/LVDS/HDMI support
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH 2/2] PCI: imx: Initial imx7d pm support
From: Lucas Stach @ 2018-06-08 14:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a6176c8bcfbca08ce4e93e304bdcd48c4600f8e2.1527621510.git.leonard.crestez@nxp.com>
Am Dienstag, den 29.05.2018, 22:39 +0300 schrieb Leonard Crestez:
> On imx7d the phy is turned off in suspend and must be reset on resume.
> Right now lspci -v fails after a suspend/resume cycle, fix this by
> adding minimal suspend/resume code from the nxp vendor tree.
>
> This is currently only enabled for imx7 but the same sequence can be
> applied to other imx pcie variants.
>
> Tested on imx7d-sabresd with an Intel 5622ANHMW wireless pcie adapter.
>
> > The original author is mostly Richard Zhu <hongxing.zhu@nxp.com>, this
> patch adjusts the code to the upstream imx7d implemention using reset
> controls and power domains.
>
> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
> ?drivers/pci/dwc/pci-imx6.c | 94 ++++++++++++++++++++++++++++++++++++--
> ?1 file changed, 89 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
> index 4818ef875f8a..ff6077eeb387 100644
> --- a/drivers/pci/dwc/pci-imx6.c
> +++ b/drivers/pci/dwc/pci-imx6.c
> @@ -540,10 +540,27 @@ static int imx6_pcie_wait_for_speed_change(struct imx6_pcie *imx6_pcie)
> ?
> > ? dev_err(dev, "Speed change timeout\n");
> > ? return -EINVAL;
> ?}
> ?
> +static void imx6_pcie_ltssm_enable(struct device *dev)
> +{
> > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> +
> > + switch (imx6_pcie->variant) {
> > + case IMX6Q:
> > + case IMX6SX:
> > + case IMX6QP:
> > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > + ???IMX6Q_GPR12_PCIE_CTL_2,
> > + ???IMX6Q_GPR12_PCIE_CTL_2);
> > + break;
> > + case IMX7D:
> > + reset_control_deassert(imx6_pcie->apps_reset);
> > + }
> +}
> +
> ?static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
> ?{
> > ? struct dw_pcie *pci = imx6_pcie->pci;
> > ? struct device *dev = pci->dev;
> > ? u32 tmp;
> @@ -558,15 +575,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
> > ? tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK;
> > ? tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1;
> > ? dw_pcie_writel_dbi(pci, PCIE_RC_LCR, tmp);
> ?
> > ? /* Start LTSSM. */
> > - if (imx6_pcie->variant == IMX7D)
> > - reset_control_deassert(imx6_pcie->apps_reset);
> > - else
> > - regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > - ???IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
> > + imx6_pcie_ltssm_enable(dev);
> ?
> > ? ret = imx6_pcie_wait_for_link(imx6_pcie);
> > ? if (ret)
> > ? goto err_reset_phy;
> ?
> @@ -681,10 +694,80 @@ static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie,
> ?
> ?static const struct dw_pcie_ops dw_pcie_ops = {
> > ? .link_up = imx6_pcie_link_up,
> ?};
> ?
> +#ifdef CONFIG_PM_SLEEP
> +static int imx6_pcie_suspend_noirq(struct device *dev)
> +{
> > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> +
> + if (imx6_pcie->variant == IMX7D) {
Instead of this large indented block, please just have an early return
at the start of this function, like:
if (imx6_pcie->variant != IMX7D)
return 0;
Same for the resume function.
> + /* Disable clks */
> > + clk_disable_unprepare(imx6_pcie->pcie);
> > + clk_disable_unprepare(imx6_pcie->pcie_phy);
> > + clk_disable_unprepare(imx6_pcie->pcie_bus);
> > + /* turn off external osc input */
> > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > + ???IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> > + ???IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> > + }
> +
> > + return 0;
> +}
> +
> +static void imx6_pcie_ltssm_disable(struct device *dev)
> +{
> > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> +
> > + switch (imx6_pcie->variant) {
> > + case IMX6Q:
> > + case IMX6SX:
> > + case IMX6QP:
> > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > + ???IMX6Q_GPR12_PCIE_CTL_2, 0);
> > + break;
> > + case IMX7D:
> > + reset_control_assert(imx6_pcie->apps_reset);
> > + break;
> > + }
> +}
> +
> +static int imx6_pcie_resume_noirq(struct device *dev)
> +{
> > + int ret = 0;
> > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> > + struct pcie_port *pp = &imx6_pcie->pci->pp;
> +
> > + if (imx6_pcie->variant == IMX7D) {
> > + imx6_pcie_ltssm_disable(dev);
The LTSSM disable seems misplaced here. I would have expected it to be
in the suspend function.
> + imx6_pcie_assert_core_reset(imx6_pcie);
> > + imx6_pcie_init_phy(imx6_pcie);
> > + imx6_pcie_deassert_core_reset(imx6_pcie);
> +
> > + /*
> > + ?* controller maybe turn off, re-configure again
> > + ?*/
> > + dw_pcie_setup_rc(pp);
> +
> > + imx6_pcie_ltssm_enable(dev);
> +
> > + ret = imx6_pcie_wait_for_link(imx6_pcie);
> > + if (ret < 0)
> + pr_info("pcie link is down after resume.\n");
If the PHY has been powered down and LTSSM stopped I guess we need to
do the full imx6_pcie_establish_link() dance again here to reliably re-
establish the link. The above seems unsafe.
> + }
> +
> > + return 0;
> +}
> +
> +static const struct dev_pm_ops imx6_pcie_pm_ops = {
> > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx6_pcie_suspend_noirq,
> > + ??????imx6_pcie_resume_noirq)
> +};
> +#endif
This structure must be outside of the ifdef, or you'll break the build
on !CONFIG_PM_SLEEP.
> ?static int imx6_pcie_probe(struct platform_device *pdev)
> ?{
> > ? struct device *dev = &pdev->dev;
> > ? struct dw_pcie *pci;
> > ? struct imx6_pcie *imx6_pcie;
> @@ -847,10 +930,11 @@ static const struct of_device_id imx6_pcie_of_match[] = {
> ?static struct platform_driver imx6_pcie_driver = {
> > ? .driver = {
> > > ? .name = "imx6q-pcie",
> > ? .of_match_table = imx6_pcie_of_match,
> > ? .suppress_bind_attrs = true,
> > + .pm = &imx6_pcie_pm_ops,
> > ? },
> > ? .probe????= imx6_pcie_probe,
> > ? .shutdown = imx6_pcie_shutdown,
> ?};
> ?
Regards,
Lucas
^ permalink raw reply
* [PATCH v2 13/16] dt-bindings/interrupt-controller: add documentation for Marvell SEI controller
From: Miquel Raynal @ 2018-06-08 14:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180605205121.GA19249@rob-hp-laptop>
Hi Rob, Marc,
On Tue, 5 Jun 2018 14:51:21 -0600, Rob Herring <robh@kernel.org> wrote:
> On Tue, May 22, 2018 at 11:40:39AM +0200, Miquel Raynal wrote:
> > Describe the System Error Interrupt (SEI) controller. It aggregates two
> > types of interrupts, wired and MSIs from respectively the AP and the
> > CPs, into a single SPI interrupt.
> >
> > Suggested-by: Haim Boot <hayim@marvell.com>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > .../bindings/interrupt-controller/marvell,sei.txt | 50 ++++++++++++++++++++++
> > 1 file changed, 50 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> > new file mode 100644
> > index 000000000000..689981036c30
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> > @@ -0,0 +1,50 @@
> > +Marvell SEI (System Error Interrupt) Controller
> > +-----------------------------------------------
> > +
> > +Marvell SEI (System Error Interrupt) controller is an interrupt
> > +aggregator. It receives interrupts from several sources and aggregates
> > +them to a single interrupt line (an SPI) on the parent interrupt
> > +controller.
> > +
> > +This interrupt controller can handle up to 64 SEIs, a set comes from the
> > +AP and is wired while a second set comes from the CPs by the mean of
> > +MSIs. Each 'domain' is represented as a subnode.
> > +
> > +Required properties:
> > +
> > +- compatible: should be "marvell,armada-8k-sei".
> > +- reg: SEI registers location and length.
> > +- interrupts: identifies the parent IRQ that will be triggered.
> > +
> > +Child node 'sei-wired-controller' required properties:
> > +
> > +- marvell,sei-ranges: ranges of wired interrupts.
> > +- #interrupt-cells: number of cells to define an SEI wired interrupt
> > + coming from the AP, should be 1. The cell is the IRQ
> > + number.
> > +- interrupt-controller: identifies the node as an interrupt controller.
> > +
> > +Child node 'sei-msi-controller' required properties:
> > +
> > +- marvell,sei-ranges: ranges of non-wired interrupts triggered by way of
> > + MSIs.
> > +- msi-controller: identifies the node as an MSI controller.
> > +
> > +Example:
> > +
> > + sei: sei at 3f0200 {
> > + compatible = "marvell,armada-8k-sei";
> > + reg = <0x3f0200 0x40>;
> > + interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > + sei_wired_controller: sei-wired-controller at 0 {
> > + marvell,sei-ranges = <0 21>;
> > + #interrupt-cells = <1>;
> > + interrupt-controller;
> > + };
> > +
> > + sei_msi_controller: sei-msi-controller at 21 {
> > + marvell,sei-ranges = <21 43>;
> > + msi-controller;
> > + };
>
> I still think this should just be all one node. There's several examples
> in the tree of nodes which are both interrupt-controller and
> msi-controller. Marvell MPIC is one example.
I checked Marvell MPIC example (armada 370 XP), it does not use
hierarchy domains, so I totally understand your point but I'm not sure
how I could get inspired by this driver (I'm looking for others).
Here I'm stuck. I know from a pure DT point of view the following is
not a valid argument. But from Linux, there is no easy way to handle
this situation without two different device nodes due to the internals
of the irqchip subsystem. There is simply no easy solution and having
only one node would require consequent changes in the core.
Maybe Marc will have an idea, but I think we already gave up on this
topic :/
Regards,
Miqu?l
^ permalink raw reply
* [PATCH v2 5/5] arm64: perf: Add support for chaining event counters
From: Suzuki K Poulose @ 2018-06-08 14:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180606180119.4ofhges6codarbmk@lakrids.cambridge.arm.com>
Hi Mark,
On 06/06/2018 07:01 PM, Mark Rutland wrote:
> On Tue, May 29, 2018 at 11:55:56AM +0100, Suzuki K Poulose wrote:
>> Add support for 64bit event by using chained event counters
>> and 64bit cycle counters.
>>
>> Arm v8 PMUv3 allows chaining a pair of adjacent PMU counters
>> (with the lower counter number being always "even"). The low
>> counter is programmed to count the event of interest and the
>> high counter(odd numbered) is programmed with a special event
>> code (0x1e - Chain).
>
> I found this a little difficult to read. How about:
>
> PMUv3 allows chaining a pair of adjacent 32-bit counters, effectively
> forming a 64-bit counter. The low/even counter is programmed to count
> the event of interest, and the high/odd counter is programmed to count
> the CHAIN event, taken when the low/even counter overflows.
>
Sure, that looks better.
>> Thus we need special allocation schemes
>> to make the full use of available counters. So, we allocate the
>> counters from either ends. i.e, chained counters are allocated
>> from the lower end in pairs of two and the normal counters are
>> allocated from the higher number. Also makes necessary changes to
>> handle the chained events as a single event with 2 counters.
>
> Why do we need to allocate in this way?
>
> I can see this might make allocating a pair of counters more likely in
> some cases, but there are still others where it wouldn't be possible
> (and we'd rely on the rotation logic to repack the counters for us).
It makes the efficient use of the counters in all cases and allows
counting maximum number of events with any given set, keeping the
precedence on the order of their "inserts".
e.g, if the number of counters happened to be "odd" (not sure if it is
even possible).
>
> [...]
>
>> +static inline u32 armv8pmu_read_evcntr(int idx)
>> +{
>> + return (armv8pmu_select_counter(idx) == idx) ?
>> + read_sysreg(pmxevcntr_el0) : 0;
>> +}
>
> Given armv8pmu_select_counter() always returns the idx passed to it, I
> don't think we need to check anything here. We can clean that up to be
> void, and remove the existing checks.
>
> [...]
OK.
>
>> +static inline u64 armv8pmu_read_chain_counter(int idx)
>> +{
>> + u64 prev_hi, hi, lo;
>> +
>> + hi = armv8pmu_read_evcntr(idx);
>> + do {
>> + prev_hi = hi;
>> + isb();
>> + lo = armv8pmu_read_evcntr(idx - 1);
>> + isb();
>> + hi = armv8pmu_read_evcntr(idx);
>> + } while (prev_hi != hi);
>> +
>> + return (hi << 32) | lo;
>> +}
>
>> +static inline void armv8pmu_write_chain_counter(int idx, u64 value)
>> +{
>> + armv8pmu_write_evcntr(idx, value >> 32);
>> + isb();
>> + armv8pmu_write_evcntr(idx - 1, value);
>> +}
>
> Can we use upper_32_bits() and lower_32_bits() here?
>
> As a more general thing, I think we need to clean up the way we
> read/write counters, because I don't think that it's right that we poke
> them while they're running -- that means you get some arbitrary skew on
> counter groups.
>
> It looks like the only case we do that is the IRQ handler, so we should
> be able to stop/start the PMU there.
Since we don't stop the "counting" of events usually when an IRQ is
triggered, the skew will be finally balanced when the events are stopped
in a the group. So, I think, stopping the PMU may not be really a good
thing after all. Just my thought.
>
> With that, we can get rid of the ISB here, and likewise in
> read_chain_counter, which wouldn't need to be a loop.
>
>> +
>> +static inline void armv8pmu_write_hw_counter(struct perf_event *event,
>> + u64 value)
>> +{
>> + int idx = event->hw.idx;
>> +
>> + if (armv8pmu_event_is_chained(event))
>> + armv8pmu_write_chain_counter(idx, value);
>> + else
>> + armv8pmu_write_evcntr(idx, value);
>> +}
>> +
>> static inline void armv8pmu_write_counter(struct perf_event *event, u64 value)
>> {
>> struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
>> @@ -541,14 +612,14 @@ static inline void armv8pmu_write_counter(struct perf_event *event, u64 value)
>> smp_processor_id(), idx);
>> else if (idx == ARMV8_IDX_CYCLE_COUNTER) {
>> /*
>> - * Set the upper 32bits as this is a 64bit counter but we only
>> - * count using the lower 32bits and we want an interrupt when
>> - * it overflows.
>> + * Set the upper 32bits if we are counting this in
>> + * 32bit mode, as this is a 64bit counter.
>> */
>
> It would be good to keep the explaination as to why.
>
Sure
>> - value |= 0xffffffff00000000ULL;
>> + if (!armv8pmu_event_is_64bit(event))
>> + value |= 0xffffffff00000000ULL;
>> write_sysreg(value, pmccntr_el0);
>> - } else if (armv8pmu_select_counter(idx) == idx)
>> - write_sysreg(value, pmxevcntr_el0);
>> + } else
>> + armv8pmu_write_hw_counter(event, value);
>> }
>
>> +static inline void armv8pmu_write_event_type(struct perf_event *event)
>> +{
>> + struct hw_perf_event *hwc = &event->hw;
>> + int idx = hwc->idx;
>> +
>> + /*
>> + * For chained events, write the the low counter event type
>> + * followed by the high counter. The high counter is programmed
>> + * with CHAIN event code with filters set to count at all ELs.
>> + */
>> + if (armv8pmu_event_is_chained(event)) {
>> + u32 chain_evt = ARMV8_PMUV3_PERFCTR_CHAIN |
>> + ARMV8_PMU_INCLUDE_EL2;
>> +
>> + armv8pmu_write_evtype(idx - 1, hwc->config_base);
>> + isb();
>> + armv8pmu_write_evtype(idx, chain_evt);
>
> The ISB isn't necessary here, AFAICT. We only do this while the PMU is
> disabled; no?
You're right. I was just following the ARM ARM.
>
>> + } else
>> + armv8pmu_write_evtype(idx, hwc->config_base);
>> +}
>
> [...]
>
>> @@ -679,6 +679,12 @@ static void cpu_pm_pmu_setup(struct arm_pmu *armpmu, unsigned long cmd)
>> continue;
>>
>> event = hw_events->events[idx];
>> + /*
>> + * If there is no event at this idx (e.g, an idx used
>> + * by a chained event in Arm v8 PMUv3), skip it.
>> + */
>> + if (!event)
>> + continue;
>
> We may as well lose the used_mask test if we're looking at the event
> regardless.
Ok
Suzuki
^ permalink raw reply
* [PATCH v2 5/5] arm64: perf: Add support for chaining event counters
From: Mark Rutland @ 2018-06-08 15:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4a5b5e7f-fc0b-84e3-fc65-b9f860029207@arm.com>
On Fri, Jun 08, 2018 at 03:46:57PM +0100, Suzuki K Poulose wrote:
> On 06/06/2018 07:01 PM, Mark Rutland wrote:
> > > Thus we need special allocation schemes
> > > to make the full use of available counters. So, we allocate the
> > > counters from either ends. i.e, chained counters are allocated
> > > from the lower end in pairs of two and the normal counters are
> > > allocated from the higher number. Also makes necessary changes to
> > > handle the chained events as a single event with 2 counters.
> >
> > Why do we need to allocate in this way?
> >
> > I can see this might make allocating a pair of counters more likely in
> > some cases, but there are still others where it wouldn't be possible
> > (and we'd rely on the rotation logic to repack the counters for us).
>
> It makes the efficient use of the counters in all cases and allows
> counting maximum number of events with any given set, keeping the precedence
> on the order of their "inserts".
> e.g, if the number of counters happened to be "odd" (not sure if it is even
> possible).
Unfortunately, that doesn't always work.
Say you have a system with 8 counters, and you open 8 (32-bit) events.
Then you close the events in counters 0, 2, 4, and 6. The live events
aren't moved, and stay where they are, in counters 1, 3, 5, and 7.
Now, say you open a 64-bit event. When we try to add it, we try to
allocate an index for two consecutive counters, and find that we can't,
despite 4 counters being free.
We return -EAGAIN to the core perf code, whereupon it removes any other
events in that group (none in this case).
Then we wait for the rotation logic to pull all of the events out and
schedule them back in, re-packing them, which should (eventually) work
regardless of how we allocate counters.
... we might need to re-pack events to solve that. :/
[...]
> > > +static inline void armv8pmu_write_chain_counter(int idx, u64 value)
> > > +{
> > > + armv8pmu_write_evcntr(idx, value >> 32);
> > > + isb();
> > > + armv8pmu_write_evcntr(idx - 1, value);
> > > +}
> >
> > Can we use upper_32_bits() and lower_32_bits() here?
> >
> > As a more general thing, I think we need to clean up the way we
> > read/write counters, because I don't think that it's right that we poke
> > them while they're running -- that means you get some arbitrary skew on
> > counter groups.
> >
> > It looks like the only case we do that is the IRQ handler, so we should
> > be able to stop/start the PMU there.
>
> Since we don't stop the "counting" of events usually when an IRQ is
> triggered, the skew will be finally balanced when the events are stopped
> in a the group. So, I think, stopping the PMU may not be really a good
> thing after all. Just my thought.
That's not quite right -- if one event in a group overflows, we'll
reprogram it *while* other events are counting, losing some events in
the process.
Stopping the PMU for the duration of the IRQ handler ensures that events
in a group are always in-sync with one another.
Thanks,
Mark.
^ permalink raw reply
* arm: mach-mvebu: dts: enable-method is always overwritten
From: Yves Lefloch @ 2018-06-08 15:43 UTC (permalink / raw)
To: linux-arm-kernel
Hello everybody,
I'm facing an issue that I believe to be a conflict between device-tree and
machine_desc.
My platform is arm/mach-mvebu. I have a DT based on "armada-xp-db-dxbc2.dts" (I
just included it and added a few okays), my CPU is a Marvell Bobcat2 switching
chip. My kernel is a vanilla 4.16.
Everything works fine except that my second core won't boot: `CPU1: failed to
come online'.
I tracked down the problem to arch/arm/mach-mvebu/platsmp.c: in this file is
defined a machine_desc that hardcodes the SMP ops to `marvell,armada-xp-smp'
whereas my device tree (by including armada-xp-98dx3236.dtsi) attempts to set
the ops to `marvell,98dx3236-smp' through enable-method. In setup_arch() the
machine_desc's ops overwrites the enable-method's ops, causing the wrong
smp_boot_secondary() call to be issued.
Now there is a note from 2014 saying that this machine_desc's `smp' field is
hardcoded like that because of "old Device Trees that were not specifying the
cpus enable-method property". As far as I can tell, this is still the case, for
instance "armada-370-db.dts" doesn't have any enable-method property.
I have worked around this by commenting out `armada_xp_smp_ops.smp' but
obviously I would prefer to keep a vanilla kernel.
So I propose to:
- Add `enable-method = "marvell,armada-xp-smp"' to armada-370-xp.dtsi, because
it seems that all Armada 370/XP include it;
- Remove the `smp' field of `armada_xp_smp_ops'.
If you agree with the diagnosis and the proposed fix I will write a patch.
Regards,
Yves Lefloch.
^ permalink raw reply
* [PATCH v4 1/7] interconnect: Add generic on-chip interconnect API
From: Alexandre Bailon @ 2018-06-08 15:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180309210958.16672-2-georgi.djakov@linaro.org>
On 03/09/2018 10:09 PM, Georgi Djakov wrote:
> This patch introduce a new API to get requirements and configure the
> interconnect buses across the entire chipset to fit with the current
> demand.
>
> The API is using a consumer/provider-based model, where the providers are
> the interconnect buses and the consumers could be various drivers.
> The consumers request interconnect resources (path) between endpoints and
> set the desired constraints on this data flow path. The providers receive
> requests from consumers and aggregate these requests for all master-slave
> pairs on that path. Then the providers configure each participating in the
> topology node according to the requested data flow path, physical links and
> constraints. The topology could be complicated and multi-tiered and is SoC
> specific.
>
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
> Documentation/interconnect/interconnect.rst | 96 ++++++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/interconnect/Kconfig | 10 +
> drivers/interconnect/Makefile | 1 +
> drivers/interconnect/core.c | 489 ++++++++++++++++++++++++++++
> include/linux/interconnect-provider.h | 109 +++++++
> include/linux/interconnect.h | 40 +++
> 8 files changed, 748 insertions(+)
> create mode 100644 Documentation/interconnect/interconnect.rst
> create mode 100644 drivers/interconnect/Kconfig
> create mode 100644 drivers/interconnect/Makefile
> create mode 100644 drivers/interconnect/core.c
> create mode 100644 include/linux/interconnect-provider.h
> create mode 100644 include/linux/interconnect.h
>
> diff --git a/Documentation/interconnect/interconnect.rst b/Documentation/interconnect/interconnect.rst
> new file mode 100644
> index 000000000000..23eba68e8424
> --- /dev/null
> +++ b/Documentation/interconnect/interconnect.rst
> @@ -0,0 +1,96 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=====================================
> +GENERIC SYSTEM INTERCONNECT SUBSYSTEM
> +=====================================
> +
> +Introduction
> +------------
> +
> +This framework is designed to provide a standard kernel interface to control
> +the settings of the interconnects on a SoC. These settings can be throughput,
> +latency and priority between multiple interconnected devices or functional
> +blocks. This can be controlled dynamically in order to save power or provide
> +maximum performance.
> +
> +The interconnect bus is a hardware with configurable parameters, which can be
> +set on a data path according to the requests received from various drivers.
> +An example of interconnect buses are the interconnects between various
> +components or functional blocks in chipsets. There can be multiple interconnects
> +on a SoC that can be multi-tiered.
> +
> +Below is a simplified diagram of a real-world SoC interconnect bus topology.
> +
> +::
> +
> + +----------------+ +----------------+
> + | HW Accelerator |--->| M NoC |<---------------+
> + +----------------+ +----------------+ |
> + | | +------------+
> + +-----+ +-------------+ V +------+ | |
> + | DDR | | +--------+ | PCIe | | |
> + +-----+ | | Slaves | +------+ | |
> + ^ ^ | +--------+ | | C NoC |
> + | | V V | |
> + +------------------+ +------------------------+ | | +-----+
> + | |-->| |-->| |-->| CPU |
> + | |-->| |<--| | +-----+
> + | Mem NoC | | S NoC | +------------+
> + | |<--| |---------+ |
> + | |<--| |<------+ | | +--------+
> + +------------------+ +------------------------+ | | +-->| Slaves |
> + ^ ^ ^ ^ ^ | | +--------+
> + | | | | | | V
> + +------+ | +-----+ +-----+ +---------+ +----------------+ +--------+
> + | CPUs | | | GPU | | DSP | | Masters |-->| P NoC |-->| Slaves |
> + +------+ | +-----+ +-----+ +---------+ +----------------+ +--------+
> + |
> + +-------+
> + | Modem |
> + +-------+
> +
> +Terminology
> +-----------
> +
> +Interconnect provider is the software definition of the interconnect hardware.
> +The interconnect providers on the above diagram are M NoC, S NoC, C NoC and Mem
> +NoC.
> +
> +Interconnect node is the software definition of the interconnect hardware
> +port. Each interconnect provider consists of multiple interconnect nodes,
> +which are connected to other SoC components including other interconnect
> +providers. The point on the diagram where the CPUs connects to the memory is
> +called an interconnect node, which belongs to the Mem NoC interconnect provider.
> +
> +Interconnect endpoints are the first or the last element of the path. Every
> +endpoint is a node, but not every node is an endpoint.
> +
> +Interconnect path is everything between two endpoints including all the nodes
> +that have to be traversed to reach from a source to destination node. It may
> +include multiple master-slave pairs across several interconnect providers.
> +
> +Interconnect consumers are the entities which make use of the data paths exposed
> +by the providers. The consumers send requests to providers requesting various
> +throughput, latency and priority. Usually the consumers are device drivers, that
> +send request based on their needs. An example for a consumer is a video decoder
> +that supports various formats and image sizes.
> +
> +Interconnect providers
> +----------------------
> +
> +Interconnect provider is an entity that implements methods to initialize and
> +configure a interconnect bus hardware. The interconnect provider drivers should
> +be registered with the interconnect provider core.
> +
> +The interconnect framework provider API functions are documented in
> +.. kernel-doc:: include/linux/interconnect-provider.h
> +
> +Interconnect consumers
> +----------------------
> +
> +Interconnect consumers are the clients which use the interconnect APIs to
> +get paths between endpoints and set their bandwidth/latency/QoS requirements
> +for these interconnect paths.
> +
> +The interconnect framework consumer API functions are documented in
> +.. kernel-doc:: include/linux/interconnect.h
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 879dc0604cba..96a1db022cee 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -219,4 +219,6 @@ source "drivers/siox/Kconfig"
>
> source "drivers/slimbus/Kconfig"
>
> +source "drivers/interconnect/Kconfig"
> +
> endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 24cd47014657..0cca95740d9b 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -185,3 +185,4 @@ obj-$(CONFIG_TEE) += tee/
> obj-$(CONFIG_MULTIPLEXER) += mux/
> obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
> obj-$(CONFIG_SIOX) += siox/
> +obj-$(CONFIG_INTERCONNECT) += interconnect/
> diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
> new file mode 100644
> index 000000000000..a261c7d41deb
> --- /dev/null
> +++ b/drivers/interconnect/Kconfig
> @@ -0,0 +1,10 @@
> +menuconfig INTERCONNECT
> + tristate "On-Chip Interconnect management support"
> + help
> + Support for management of the on-chip interconnects.
> +
> + This framework is designed to provide a generic interface for
> + managing the interconnects in a SoC.
> +
> + If unsure, say no.
> +
> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> new file mode 100644
> index 000000000000..5edf0ae80818
> --- /dev/null
> +++ b/drivers/interconnect/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_INTERCONNECT) += core.o
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> new file mode 100644
> index 000000000000..6306e258b9b9
> --- /dev/null
> +++ b/drivers/interconnect/core.c
> @@ -0,0 +1,489 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Interconnect framework core driver
> + *
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/idr.h>
> +#include <linux/init.h>
> +#include <linux/interconnect.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +
> +static DEFINE_IDR(icc_idr);
> +static LIST_HEAD(icc_provider_list);
> +static DEFINE_MUTEX(icc_provider_list_mutex);
> +static DEFINE_MUTEX(icc_path_mutex);
> +
> +/**
> + * struct icc_req - constraints that are attached to each node
> + *
> + * @req_node: entry in list of requests for the particular @node
> + * @node: the interconnect node to which this constraint applies
> + * @avg_bw: an integer describing the average bandwidth in kbps
> + * @peak_bw: an integer describing the peak bandwidth in kbps
> + */
> +struct icc_req {
> + struct hlist_node req_node;
> + struct icc_node *node;
> + u32 avg_bw;
> + u32 peak_bw;
> +};
> +
> +/**
> + * struct icc_path - interconnect path structure
> + * @num_nodes: number of hops (nodes)
> + * @reqs: array of the requests applicable to this path of nodes
> + */
> +struct icc_path {
> + size_t num_nodes;
> + struct icc_req reqs[0];
> +};
> +
> +static struct icc_node *node_find(const int id)
> +{
> + struct icc_node *node;
> +
> + node = idr_find(&icc_idr, id);
> +
> + return node;
> +}
> +
> +static struct icc_path *path_allocate(struct icc_node *node, ssize_t num_nodes)
> +{
> + struct icc_path *path;
> + size_t i;
> +
> + path = kzalloc(sizeof(*path) + num_nodes * sizeof(*path->reqs),
> + GFP_KERNEL);
> + if (!path)
> + return ERR_PTR(-ENOMEM);
> +
> + path->num_nodes = num_nodes;
> +
> + for (i = 0; i < num_nodes; i++) {
> + hlist_add_head(&path->reqs[i].req_node, &node->req_list);
> +
> + path->reqs[i].node = node;
> + /* reference to previous node was saved during path traversal */
> + node = node->reverse;
> + }
> +
> + return path;
> +}
> +
> +static struct icc_path *path_find(struct icc_node *src, struct icc_node *dst)
> +{
> + struct icc_node *node = NULL;
> + struct list_head traverse_list;
> + struct list_head edge_list;
> + struct list_head tmp_list;
> + size_t i, number = 0;
> + bool found = false;
> +
> + INIT_LIST_HEAD(&traverse_list);
> + INIT_LIST_HEAD(&edge_list);
> + INIT_LIST_HEAD(&tmp_list);
> +
> + list_add_tail(&src->search_list, &traverse_list);
> +
> + do {
> + list_for_each_entry(node, &traverse_list, search_list) {
> + if (node == dst) {
> + found = true;
> + list_add(&node->search_list, &tmp_list);
> + break;
> + }
> + for (i = 0; i < node->num_links; i++) {
> + struct icc_node *tmp = node->links[i];
> +
> + if (!tmp)
> + return ERR_PTR(-ENOENT);
> +
> + if (tmp->is_traversed)
> + continue;
> +
> + tmp->is_traversed = true;
> + tmp->reverse = node;
> + list_add_tail(&tmp->search_list, &edge_list);
> + }
> + }
> + if (found)
> + break;
> +
> + list_splice_init(&traverse_list, &tmp_list);
> + list_splice_init(&edge_list, &traverse_list);
> +
> + /* count the number of nodes */
> + number++;
> +
> + } while (!list_empty(&traverse_list));
> +
> + /* reset the traversed state */
> + list_for_each_entry(node, &tmp_list, search_list)
> + node->is_traversed = false;
> +
> + if (found)
> + return path_allocate(dst, number);
> +
> + return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static int path_init(struct icc_path *path)
> +{
> + struct icc_node *node;
> + size_t i;
> +
> + for (i = 0; i < path->num_nodes; i++) {
> + node = path->reqs[i].node;
> +
> + mutex_lock(&node->provider->lock);
> + node->provider->users++;
> + mutex_unlock(&node->provider->lock);
> + }
> +
> + return 0;
> +}
> +
> +static void node_aggregate(struct icc_node *node)
> +{
> + struct icc_req *r;
> + u32 agg_avg = 0;
> + u32 agg_peak = 0;
> +
> + hlist_for_each_entry(r, &node->req_list, req_node) {
> + /* sum(averages) and max(peaks) */
> + agg_avg += r->avg_bw;
> + agg_peak = max(agg_peak, r->peak_bw);
> + }
> +
> + node->avg_bw = agg_avg;
> + node->peak_bw = agg_peak;
> +}
> +
> +static void provider_aggregate(struct icc_provider *provider, u32 *avg_bw,
> + u32 *peak_bw)
> +{
> + struct icc_node *n;
> + u32 agg_avg = 0;
> + u32 agg_peak = 0;
> +
> + /* aggregate for the interconnect provider */
> + list_for_each_entry(n, &provider->nodes, node_list) {
> + /* sum the average and max the peak */
> + agg_avg += n->avg_bw;
> + agg_peak = max(agg_peak, n->peak_bw);
> + }
> +
> + *avg_bw = agg_avg;
> + *peak_bw = agg_peak;
> +}
> +
> +static int constraints_apply(struct icc_path *path)
> +{
> + struct icc_node *next, *prev = NULL;
> + int i;
> +
> + for (i = 0; i < path->num_nodes; i++, prev = next) {
> + struct icc_provider *provider;
> + u32 avg_bw = 0;
> + u32 peak_bw = 0;
> + int ret;
> +
> + next = path->reqs[i].node;
> + /*
> + * Both endpoints should be valid master-slave pairs of the
> + * same interconnect provider that will be configured.
> + */
> + if (!next || !prev)
> + continue;
> +
> + if (next->provider != prev->provider)
> + continue;
> +
> + provider = next->provider;
> + mutex_lock(&provider->lock);
> +
> + /* aggregate requests for the provider */
> + provider_aggregate(provider, &avg_bw, &peak_bw);
> +
> + if (provider->set) {
> + /* set the constraints */
> + ret = provider->set(prev, next, avg_bw, peak_bw);
> + }
> +
> + mutex_unlock(&provider->lock);
> +
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * icc_set() - set constraints on an interconnect path between two endpoints
> + * @path: reference to the path returned by icc_get()
> + * @avg_bw: average bandwidth in kbps
> + * @peak_bw: peak bandwidth in kbps
> + *
> + * This function is used by an interconnect consumer to express its own needs
> + * in term of bandwidth and QoS for a previously requested path between two
> + * endpoints. The requests are aggregated and each node is updated accordingly.
> + *
> + * Returns 0 on success, or an approproate error code otherwise.
> + */
> +int icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw)
> +{
> + struct icc_node *node;
> + size_t i;
> + int ret;
> +
> + if (!path)
> + return 0;
> +
> + for (i = 0; i < path->num_nodes; i++) {
> + node = path->reqs[i].node;
> +
> + mutex_lock(&icc_path_mutex);
> +
> + /* update the consumer request for this path */
> + path->reqs[i].avg_bw = avg_bw;
> + path->reqs[i].peak_bw = peak_bw;
> +
> + /* aggregate requests for this node */
> + node_aggregate(node);
> +
> + mutex_unlock(&icc_path_mutex);
> + }
> +
> + ret = constraints_apply(path);
> + if (ret)
> + pr_err("interconnect: error applying constraints (%d)", ret);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(icc_set);
> +
> +/**
> + * icc_get() - return a handle for path between two endpoints
> + * @src_id: source device port id
> + * @dst_id: destination device port id
> + *
> + * This function will search for a path between two endpoints and return an
> + * icc_path handle on success. Use icc_put() to release
> + * constraints when the they are not needed anymore.
> + *
> + * Return: icc_path pointer on success, or ERR_PTR() on error
> + */
> +struct icc_path *icc_get(const int src_id, const int dst_id)
> +{
> + struct icc_node *src, *dst;
> + struct icc_path *path = ERR_PTR(-EPROBE_DEFER);
> +
> + src = node_find(src_id);
> + if (!src)
> + goto out;
> +
> + dst = node_find(dst_id);
> + if (!dst)
> + goto out;
> +
> + mutex_lock(&icc_path_mutex);
> + path = path_find(src, dst);
> + mutex_unlock(&icc_path_mutex);
> + if (IS_ERR(path))
> + goto out;
> +
> + path_init(path);
> +
> +out:
> + return path;
> +}
> +EXPORT_SYMBOL_GPL(icc_get);
> +
> +/**
> + * icc_put() - release the reference to the icc_path
> + * @path: interconnect path
> + *
> + * Use this function to release the constraints on a path when the path is
> + * no longer needed. The constraints will be re-aggregated.
> + */
> +void icc_put(struct icc_path *path)
> +{
> + struct icc_node *node;
> + size_t i;
> + int ret;
> +
> + if (!path || WARN_ON_ONCE(IS_ERR(path)))
> + return;
> +
> + ret = icc_set(path, 0, 0);
> + if (ret)
> + pr_err("%s: error (%d)\n", __func__, ret);
> +
> + for (i = 0; i < path->num_nodes; i++) {
> + node = path->reqs[i].node;
> + hlist_del(&path->reqs[i].req_node);
> +
> + mutex_lock(&node->provider->lock);
> + node->provider->users--;
> + mutex_unlock(&node->provider->lock);
> + }
> +
> + kfree(path);
> +}
> +EXPORT_SYMBOL_GPL(icc_put);
> +
> +/**
> + * icc_node_create() - create a node
> + * @id: node id
> + *
> + * Return: icc_node pointer on success, or ERR_PTR() on error
> + */
> +struct icc_node *icc_node_create(int id)
> +{
> + struct icc_node *node;
> +
> + /* check if node already exists */
> + node = node_find(id);
> + if (node)
> + return node;
> +
> + node = kzalloc(sizeof(*node), GFP_KERNEL);
> + if (!node)
> + return ERR_PTR(-ENOMEM);
> +
> + id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
> + if (WARN(id < 0, "couldn't get idr"))
> + return ERR_PTR(id);
> +
> + node->id = id;
> +
> + return node;
> +}
> +EXPORT_SYMBOL_GPL(icc_node_create);
> +
> +/**
> + * icc_link_create() - create a link between two nodes
> + * @src_id: source node id
I guess src_id has become node and is not an id anymore,
so it should be updated.
> + * @dst_id: destination node id
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_link_create(struct icc_node *node, const int dst_id)
> +{
> + struct icc_node *dst;
> + struct icc_node **new;
> + int ret = 0;
> +
> + if (IS_ERR_OR_NULL(node))
> + return PTR_ERR(node);
> +
> + mutex_lock(&node->provider->lock);
> +
> + dst = node_find(dst_id);
> + if (!dst)
> + dst = icc_node_create(dst_id);
> +
> + new = krealloc(node->links,
> + (node->num_links + 1) * sizeof(*node->links),
> + GFP_KERNEL);
> + if (!new) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + node->links = new;
> + node->links[node->num_links++] = dst;
> +
> +out:
> + mutex_unlock(&node->provider->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_link_create);
> +
> +/**
> + * icc_add_node() - add an interconnect node to interconnect provider
> + * @node: pointer to the interconnect node
> + * @provider: pointer to the interconnect provider
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_node_add(struct icc_node *node, struct icc_provider *provider)
> +{
> + if (WARN_ON(!node))
> + return -EINVAL;
> +
> + if (WARN_ON(!provider))
> + return -EINVAL;
> +
> + node->provider = provider;
> +
> + mutex_lock(&provider->lock);
> + list_add_tail(&node->node_list, &provider->nodes);
> + mutex_unlock(&provider->lock);
> +
> + return 0;
> +}
> +
> +/**
> + * icc_add_provider() - add a new interconnect provider
> + * @icc_provider: the interconnect provider that will be added into topology
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_add_provider(struct icc_provider *provider)
> +{
> + if (WARN_ON(!provider))
> + return -EINVAL;
> +
> + if (WARN_ON(!provider->set))
> + return -EINVAL;
> +
> + mutex_init(&provider->lock);
> + INIT_LIST_HEAD(&provider->nodes);
> +
> + mutex_lock(&icc_provider_list_mutex);
> + list_add(&provider->provider_list, &icc_provider_list);
> + mutex_unlock(&icc_provider_list_mutex);
> +
> + dev_dbg(provider->dev, "interconnect provider added to topology\n");
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_add_provider);
> +
> +/**
> + * icc_del_provider() - delete previously added interconnect provider
> + * @icc_provider: the interconnect provider that will be removed from topology
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_del_provider(struct icc_provider *provider)
> +{
> + mutex_lock(&provider->lock);
> + if (provider->users) {
> + pr_warn("interconnect provider still has %d users\n",
> + provider->users);
> + }
> + mutex_unlock(&provider->lock);
> +
> + mutex_lock(&icc_provider_list_mutex);
> + list_del(&provider->provider_list);
> + mutex_unlock(&icc_provider_list_mutex);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_del_provider);
> +
> +MODULE_AUTHOR("Georgi Djakov <georgi.djakov@linaro.org");
> +MODULE_DESCRIPTION("Interconnect Driver Core");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/interconnect-provider.h b/include/linux/interconnect-provider.h
> new file mode 100644
> index 000000000000..779b5b5b1306
> --- /dev/null
> +++ b/include/linux/interconnect-provider.h
> @@ -0,0 +1,109 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#ifndef _LINUX_INTERCONNECT_PROVIDER_H
> +#define _LINUX_INTERCONNECT_PROVIDER_H
> +
> +#include <linux/interconnect.h>
> +
> +struct icc_node;
> +
> +/**
> + * struct icc_provider - interconnect provider (controller) entity that might
> + * provide multiple interconnect controls
> + *
> + * @provider_list: list of the registered interconnect providers
> + * @nodes: internal list of the interconnect provider nodes
> + * @set: pointer to device specific set operation function
> + * @dev: the device this interconnect provider belongs to
> + * @lock: lock to provide consistency during aggregation/update of constraints
> + * @users: count of active users
> + * @data: pointer to private data
> + */
> +struct icc_provider {
> + struct list_head provider_list;
> + struct list_head nodes;
> + int (*set)(struct icc_node *src, struct icc_node *dst,
> + u32 avg_bw, u32 peak_bw);
> + struct device *dev;
> + struct mutex lock;
> + int users;
> + void *data;
> +};
> +
> +/**
> + * struct icc_node - entity that is part of the interconnect topology
> + *
> + * @id: platform specific node id
> + * @name: node name used in debugfs
> + * @links: a list of targets where we can go next when traversing
> + * @num_links: number of links to other interconnect nodes
> + * @provider: points to the interconnect provider of this node
> + * @node_list: list of interconnect nodes associated with @provider
> + * @search_list: list used when walking the nodes graph
> + * @reverse: pointer to previous node when walking the nodes graph
> + * @is_traversed: flag that is used when walking the nodes graph
> + * @req_list: a list of QoS constraint requests associated with this node
> + * @avg_bw: aggregated value of average bandwidth
> + * @peak_bw: aggregated value of peak bandwidth
> + * @data: pointer to private data
> + */
> +struct icc_node {
> + int id;
> + const char *name;
> + struct icc_node **links;
> + size_t num_links;
> +
> + struct icc_provider *provider;
> + struct list_head node_list;
> + struct list_head orphan_list;
> + struct list_head search_list;
> + struct icc_node *reverse;
> + bool is_traversed;
> + struct hlist_head req_list;
> + u32 avg_bw;
> + u32 peak_bw;
> + void *data;
> +};
> +
> +#if IS_ENABLED(CONFIG_INTERCONNECT)
> +
> +struct icc_node *icc_node_create(int id);
> +int icc_node_add(struct icc_node *node, struct icc_provider *provider);
> +int icc_link_create(struct icc_node *node, const int dst_id);
> +int icc_add_provider(struct icc_provider *provider);
> +int icc_del_provider(struct icc_provider *provider);
> +
> +#else
> +
> +static inline struct icc_node *icc_node_create(int id)
> +{
> + return ERR_PTR(-ENOTSUPP);
> +}
> +
> +int icc_node_add(struct icc_node *node, struct icc_provider *provider)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline int icc_link_create(struct icc_node *node, const int dst_id)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline int icc_add_provider(struct icc_provider *provider)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline int icc_del_provider(struct icc_provider *provider)
> +{
> + return -ENOTSUPP;
> +}
> +
> +#endif /* CONFIG_INTERCONNECT */
> +
> +#endif /* _LINUX_INTERCONNECT_PROVIDER_H */
> diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
> new file mode 100644
> index 000000000000..5a7cf72b76a5
> --- /dev/null
> +++ b/include/linux/interconnect.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#ifndef _LINUX_INTERCONNECT_H
> +#define _LINUX_INTERCONNECT_H
> +
> +#include <linux/types.h>
> +#include <linux/mutex.h>
> +
> +struct icc_path;
> +struct device;
> +
> +#if IS_ENABLED(CONFIG_INTERCONNECT)
> +
> +struct icc_path *icc_get(const int src_id, const int dst_id);
> +void icc_put(struct icc_path *path);
> +int icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw);
> +
> +#else
> +
> +static inline struct icc_path *icc_get(const int src_id, const int dst_id)
> +{
> + return NULL;
> +}
> +
> +static inline void icc_put(struct icc_path *path)
> +{
> +}
> +
> +static inline int icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw)
> +{
> + return 0;
> +}
> +
> +#endif /* CONFIG_INTERCONNECT */
> +
> +#endif /* _LINUX_INTERCONNECT_H */
>
^ permalink raw reply
* arm: mach-mvebu: dts: enable-method is always overwritten
From: Andrew Lunn @ 2018-06-08 15:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a2d4701e-28b3-07ee-6b2f-588b2e9a75c4@online.net>
On Fri, Jun 08, 2018 at 05:43:11PM +0200, Yves Lefloch wrote:
> Hello everybody,
Adding Chris Packham. He did some work in this area.
Andrew
>
> I'm facing an issue that I believe to be a conflict between device-tree and
> machine_desc.
>
> My platform is arm/mach-mvebu. I have a DT based on "armada-xp-db-dxbc2.dts"
> (I just included it and added a few okays), my CPU is a Marvell Bobcat2
> switching chip. My kernel is a vanilla 4.16.
>
> Everything works fine except that my second core won't boot: `CPU1: failed
> to come online'.
> I tracked down the problem to arch/arm/mach-mvebu/platsmp.c: in this file is
> defined a machine_desc that hardcodes the SMP ops to `marvell,armada-xp-smp'
> whereas my device tree (by including armada-xp-98dx3236.dtsi) attempts to
> set the ops to `marvell,98dx3236-smp' through enable-method. In setup_arch()
> the machine_desc's ops overwrites the enable-method's ops, causing the wrong
> smp_boot_secondary() call to be issued.
>
> Now there is a note from 2014 saying that this machine_desc's `smp' field is
> hardcoded like that because of "old Device Trees that were not specifying
> the cpus enable-method property". As far as I can tell, this is still the
> case, for instance "armada-370-db.dts" doesn't have any enable-method
> property.
>
> I have worked around this by commenting out `armada_xp_smp_ops.smp' but
> obviously I would prefer to keep a vanilla kernel.
>
> So I propose to:
> - Add `enable-method = "marvell,armada-xp-smp"' to armada-370-xp.dtsi,
> because it seems that all Armada 370/XP include it;
> - Remove the `smp' field of `armada_xp_smp_ops'.
>
> If you agree with the diagnosis and the proposed fix I will write a patch.
>
> Regards,
> Yves Lefloch.
^ permalink raw reply
* [PATCH v2 5/5] arm64: perf: Add support for chaining event counters
From: Suzuki K Poulose @ 2018-06-08 16:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180608152419.kamu7ptmgsoah5m3@lakrids.cambridge.arm.com>
On 08/06/18 16:24, Mark Rutland wrote:
> On Fri, Jun 08, 2018 at 03:46:57PM +0100, Suzuki K Poulose wrote:
>> On 06/06/2018 07:01 PM, Mark Rutland wrote:
>>>> Thus we need special allocation schemes
>>>> to make the full use of available counters. So, we allocate the
>>>> counters from either ends. i.e, chained counters are allocated
>>>> from the lower end in pairs of two and the normal counters are
>>>> allocated from the higher number. Also makes necessary changes to
>>>> handle the chained events as a single event with 2 counters.
>>>
>>> Why do we need to allocate in this way?
>>>
>>> I can see this might make allocating a pair of counters more likely in
>>> some cases, but there are still others where it wouldn't be possible
>>> (and we'd rely on the rotation logic to repack the counters for us).
>>
>> It makes the efficient use of the counters in all cases and allows
>> counting maximum number of events with any given set, keeping the precedence
>> on the order of their "inserts".
>> e.g, if the number of counters happened to be "odd" (not sure if it is even
>> possible).
>
> Unfortunately, that doesn't always work.
>
> Say you have a system with 8 counters, and you open 8 (32-bit) events.
I was talking about the following (imaginary) case :
We have 7 counters, and you have 5 32bit counters and 1 64bit counter.
Without the above scheme, you would place first 5 events on the first
5 counters and then you can't place the 64bit counter, as you are now
left with a low/odd counter and a high/even counter.
>
> Then you close the events in counters 0, 2, 4, and 6. The live events
> aren't moved, and stay where they are, in counters 1, 3, 5, and 7.
>
> Now, say you open a 64-bit event. When we try to add it, we try to
> allocate an index for two consecutive counters, and find that we can't,
> despite 4 counters being free.
>
> We return -EAGAIN to the core perf code, whereupon it removes any other
> events in that group (none in this case).
>
> Then we wait for the rotation logic to pull all of the events out and
> schedule them back in, re-packing them, which should (eventually) work
> regardless of how we allocate counters.
>
> ... we might need to re-pack events to solve that. :/
I agree, removing and putting them back in is not going to work unless
we re-pack or delay allocating the counters until we start the PMU.
>
> [...]
>
>>>> +static inline void armv8pmu_write_chain_counter(int idx, u64 value)
>>>> +{
>>>> + armv8pmu_write_evcntr(idx, value >> 32);
>>>> + isb();
>>>> + armv8pmu_write_evcntr(idx - 1, value);
>>>> +}
>>>
>>> Can we use upper_32_bits() and lower_32_bits() here?
>>>
>>> As a more general thing, I think we need to clean up the way we
>>> read/write counters, because I don't think that it's right that we poke
>>> them while they're running -- that means you get some arbitrary skew on
>>> counter groups.
>>>
>>> It looks like the only case we do that is the IRQ handler, so we should
>>> be able to stop/start the PMU there.
>>
>> Since we don't stop the "counting" of events usually when an IRQ is
>> triggered, the skew will be finally balanced when the events are stopped
>> in a the group. So, I think, stopping the PMU may not be really a good
>> thing after all. Just my thought.
>
> That's not quite right -- if one event in a group overflows, we'll
> reprogram it *while* other events are counting, losing some events in
> the process.
Oh yes, you're right. I can fix it.
>
> Stopping the PMU for the duration of the IRQ handler ensures that events
> in a group are always in-sync with one another.
Suzuki
^ permalink raw reply
* [GIT PULL] arm64 patches for 4.18
From: Catalin Marinas @ 2018-06-08 16:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi Linus,
Please pull the arm64 updates for 4.18 below. Apart from the core arm64
and perf changes, the Spectre v4 mitigation touches the arm KVM code and
the ACPI PPTT support touches drivers/ (acpi and cacheinfo). I should
have the maintainers' acks in place.
Thanks.
The following changes since commit 75bc37fefc4471e718ba8e651aa74673d4e0a9eb:
Linux 4.17-rc4 (2018-05-06 16:57:38 -1000)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux tags/arm64-upstream
for you to fetch changes up to 0fe42512b2f03f9e5a20b9f55ef1013a68b4cd48:
arm64: Fix syscall restarting around signal suppressed by tracer (2018-06-08 13:21:39 +0100)
----------------------------------------------------------------
arm64 updates for 4.18:
- Spectre v4 mitigation (Speculative Store Bypass Disable) support for
arm64 using SMC firmware call to set a hardware chicken bit
- ACPI PPTT (Processor Properties Topology Table) parsing support and
enable the feature for arm64
- Report signal frame size to user via auxv (AT_MINSIGSTKSZ). The
primary motivation is Scalable Vector Extensions which requires more
space on the signal frame than the currently defined MINSIGSTKSZ
- ARM perf patches: allow building arm-cci as module, demote dev_warn()
to dev_dbg() in arm-ccn event_init(), miscellaneous cleanups
- cmpwait() WFE optimisation to avoid some spurious wakeups
- L1_CACHE_BYTES reverted back to 64 (for performance reasons that have
to do with some network allocations) while keeping ARCH_DMA_MINALIGN
to 128. cache_line_size() returns the actual hardware Cache Writeback
Granule
- Turn LSE atomics on by default in Kconfig
- Kernel fault reporting tidying
- Some #include and miscellaneous cleanups
----------------------------------------------------------------
Arnd Bergmann (3):
drivers/bus: arm-cci: fix build warnings
ARM: mcpm, perf/arm-cci: export mcpm_is_available
arm64: cpu_errata: include required headers
Catalin Marinas (4):
Revert "arm64: Increase the max granular size"
arm64: Increase ARCH_DMA_MINALIGN to 128
Merge branch 'for-next/perf' of git://git.kernel.org/.../will/linux
arm64: KVM: Move VCPU_WORKAROUND_2_FLAG macros to the top of the file
Dave Martin (4):
arm64/sve: Write ZCR_EL1 on context switch only if changed
arm64/sve: Thin out initialisation sanity-checks for sve_max_vl
arm64: signal: Report signal frame size to userspace via auxv
arm64: Fix syscall restarting around signal suppressed by tracer
Jeremy Linton (13):
drivers: base: cacheinfo: move cache_setup_of_node()
drivers: base: cacheinfo: setup DT cache properties early
cacheinfo: rename of_node to fw_token
arm64/acpi: Create arch specific cpu to acpi id helper
ACPI/PPTT: Add Processor Properties Topology Table parsing
ACPI: Enable PPTT support on ARM64
drivers: base cacheinfo: Add support for ACPI based firmware tables
arm64: Add support for ACPI based firmware tables
arm64: topology: rename cluster_id
arm64: topology: enable ACPI/PPTT based CPU topology
ACPI: Add PPTT to injectable table list
arm64: topology: divorce MC scheduling domain from core_siblings
arm64: topology: Avoid checking numa mask for scheduler MC selection
John Garry (1):
drivers/perf: Remove ARM_SPE_PMU explicit PERF_EVENTS dependency
Marc Zyngier (14):
arm/arm64: smccc: Add SMCCC-specific return codes
arm64: Call ARCH_WORKAROUND_2 on transitions between EL0 and EL1
arm64: Add per-cpu infrastructure to call ARCH_WORKAROUND_2
arm64: Add ARCH_WORKAROUND_2 probing
arm64: Add 'ssbd' command-line option
arm64: ssbd: Add global mitigation state accessor
arm64: ssbd: Skip apply_ssbd if not using dynamic mitigation
arm64: ssbd: Restore mitigation status on CPU resume
arm64: ssbd: Introduce thread flag to control userspace mitigation
arm64: ssbd: Add prctl interface for per-thread mitigation
arm64: KVM: Add HYP per-cpu accessors
arm64: KVM: Add ARCH_WORKAROUND_2 support for guests
arm64: KVM: Handle guest's ARCH_WORKAROUND_2 requests
arm64: KVM: Add ARCH_WORKAROUND_2 discovery through ARCH_FEATURES_FUNC_ID
Mark Rutland (4):
arm_pmu: simplify arm_pmu::handle_irq
drivers/perf: arm-ccn: don't log to dmesg in event_init
arm64: make is_permission_fault() name clearer
arm64: Unify kernel fault reporting
Masahiro Yamada (1):
arm64: remove no-op macro VMLINUX_SYMBOL()
Robin Murphy (5):
arm64: Select ARCH_HAS_FAST_MULTIPLIER
perf/arm-cci: Remove unnecessary period adjustment
perf/arm-cc*: Fix MODULE_LICENSE() tags
perf/arm-cci: Remove pointless PMU disabling
perf/arm-cci: Allow building as a module
Sudeep Holla (1):
ACPI / PPTT: fix build when CONFIG_ACPI_PPTT is not enabled
Vincenzo Frascino (1):
arm64: Remove duplicate include
Will Deacon (2):
arm64: cmpwait: Clear event register before arming exclusive monitor
arm64: Kconfig: Enable LSE atomics by default
Wolfram Sang (1):
perf: simplify getting .drvdata
Documentation/admin-guide/kernel-parameters.txt | 17 +
arch/arm/common/mcpm_entry.c | 2 +
arch/arm/include/asm/kvm_host.h | 12 +
arch/arm/include/asm/kvm_mmu.h | 5 +
arch/arm/kernel/perf_event_v6.c | 4 +-
arch/arm/kernel/perf_event_v7.c | 3 +-
arch/arm/kernel/perf_event_xscale.c | 6 +-
arch/arm64/Kconfig | 15 +-
arch/arm64/include/asm/acpi.h | 4 +
arch/arm64/include/asm/cache.h | 6 +-
arch/arm64/include/asm/cmpxchg.h | 4 +-
arch/arm64/include/asm/cpucaps.h | 3 +-
arch/arm64/include/asm/cpufeature.h | 22 +
arch/arm64/include/asm/elf.h | 13 +
arch/arm64/include/asm/fpsimdmacros.h | 12 +-
arch/arm64/include/asm/kvm_asm.h | 30 +-
arch/arm64/include/asm/kvm_host.h | 26 +
arch/arm64/include/asm/kvm_mmu.h | 25 +-
arch/arm64/include/asm/processor.h | 5 +
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/include/asm/topology.h | 6 +-
arch/arm64/include/uapi/asm/auxvec.h | 3 +-
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/armv8_deprecated.c | 3 +-
arch/arm64/kernel/asm-offsets.c | 1 +
arch/arm64/kernel/cacheinfo.c | 15 +-
arch/arm64/kernel/cpu_errata.c | 182 +++++++
arch/arm64/kernel/cpufeature.c | 10 +-
arch/arm64/kernel/entry-fpsimd.S | 2 +-
arch/arm64/kernel/entry.S | 30 ++
arch/arm64/kernel/fpsimd.c | 18 +-
arch/arm64/kernel/hibernate.c | 11 +
arch/arm64/kernel/perf_event.c | 3 +-
arch/arm64/kernel/ptrace.c | 5 -
arch/arm64/kernel/signal.c | 57 ++-
arch/arm64/kernel/ssbd.c | 110 ++++
arch/arm64/kernel/suspend.c | 8 +
arch/arm64/kernel/topology.c | 104 +++-
arch/arm64/kernel/vmlinux.lds.S | 20 +-
arch/arm64/kvm/hyp/hyp-entry.S | 38 +-
arch/arm64/kvm/hyp/switch.c | 42 ++
arch/arm64/kvm/reset.c | 4 +
arch/arm64/mm/dma-mapping.c | 5 +
arch/arm64/mm/fault.c | 46 +-
arch/riscv/kernel/cacheinfo.c | 1 -
drivers/acpi/Kconfig | 3 +
drivers/acpi/Makefile | 1 +
drivers/acpi/pptt.c | 655 ++++++++++++++++++++++++
drivers/acpi/tables.c | 2 +-
drivers/base/cacheinfo.c | 157 +++---
drivers/perf/Kconfig | 36 +-
drivers/perf/arm-cci.c | 47 +-
drivers/perf/arm-ccn.c | 22 +-
drivers/perf/arm_pmu.c | 2 +-
drivers/perf/arm_spe_pmu.c | 6 +-
include/linux/acpi.h | 19 +
include/linux/arm-smccc.h | 10 +
include/linux/cacheinfo.h | 25 +-
include/linux/perf/arm_pmu.h | 2 +-
virt/kvm/arm/arm.c | 4 +
virt/kvm/arm/psci.c | 18 +-
61 files changed, 1689 insertions(+), 260 deletions(-)
create mode 100644 arch/arm64/kernel/ssbd.c
create mode 100644 drivers/acpi/pptt.c
--
Catalin
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox