Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] MMC: meson: avoid possible NULL dereference
From: Ulf Hansson @ 2017-01-10 15:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <m237gwcec4.fsf@baylibre.com>

On 6 January 2017 at 18:01, Kevin Hilman <khilman@baylibre.com> wrote:
> Heinrich Schuchardt <xypron.glpk@gmx.de> writes:
>
>> No actual segmentation faults were observed but the coding is
>> at least inconsistent.
>>
>> irqreturn_t meson_mmc_irq():
>>
>> We should not dereference host before checking it.
>>
>> meson_mmc_irq_thread():
>>
>> If cmd or mrq are NULL we should not dereference them after
>> writing a warning.
>>
>> Fixes: 51c5d8447bd7 MMC: meson: initial support for GX platforms
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> Acked-by: Kevin Hilman <khilman@baylibre.com>
>
> Ulf, I assume you can pick this up directly for v4.10-rc?

Thanks, applied for fixes!

Kind regards
Uffe


>
> Thanks,
>
> Kevin
>
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index b352760c041e..09739352834c 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -578,13 +578,15 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
>>  {
>>       struct meson_host *host = dev_id;
>>       struct mmc_request *mrq;
>> -     struct mmc_command *cmd = host->cmd;
>> +     struct mmc_command *cmd;
>>       u32 irq_en, status, raw_status;
>>       irqreturn_t ret = IRQ_HANDLED;
>>
>>       if (WARN_ON(!host))
>>               return IRQ_NONE;
>>
>> +     cmd = host->cmd;
>> +
>>       mrq = host->mrq;
>>
>>       if (WARN_ON(!mrq))
>> @@ -670,10 +672,10 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>       int ret = IRQ_HANDLED;
>>
>>       if (WARN_ON(!mrq))
>> -             ret = IRQ_NONE;
>> +             return IRQ_NONE;
>>
>>       if (WARN_ON(!cmd))
>> -             ret = IRQ_NONE;
>> +             return IRQ_NONE;
>>
>>       data = cmd->data;
>>       if (data) {

^ permalink raw reply

* [PATCH 0/2] mmc: sdhci-iproc: Improve bcm2835 performance
From: Ulf Hansson @ 2017-01-10 15:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483111474-29907-1-git-send-email-stefan.wahren@i2se.com>

On 30 December 2016 at 16:24, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> The sdhci-iproc waste a lot performance potential on bcm2835 because of
> missing capabilities in the platform data. This patch series tries to fix
> this.
>
> Raspberry Pi Zero with a class 10 sdhc card
>
> Before:
> dd if=/dev/zero conv=fdatasync of=test bs=8k count=10000
> 81920000 Bytes (82 MB), 11,0044 s, 7,4 MB/s
>
> sudo dd if=/dev/mmcblk0p1 of=/dev/null
> 62914560 Bytes (63 MB), 5,83784 s, 10,8 MB/s
>
> After:
> dd if=/dev/zero conv=fdatasync of=test bs=8k count=10000
> 81920000 Bytes (82 MB), 9,76938 s, 8,4 MB/s
>
> sudo dd if=/dev/mmcblk0p1 of=/dev/null
> 62914560 Bytes (63 MB), 4,73651 s, 13,3 MB/s
>
> Raspberry Pi Compute Module
>
> Before:
> dd if=/dev/zero conv=fdatasync of=test bs=8k count=10000
> 81920000 Bytes (82 MB), 27,4257 s, 3,0 MB/s
>
> sudo dd if=/dev/mmcblk0p1 of=/dev/null
> 66060288 Bytes (66 MB), 21,4365 s, 3,1 MB/s
>
> After:
> dd if=/dev/zero conv=fdatasync of=test bs=8k count=10000
> 81920000 Bytes (82 MB), 7,19661 s, 11,4 MB/s
>
> sudo dd if=/dev/mmcblk0p1 of=/dev/null
> 66060288 Bytes (66 MB), 4,76453 s, 13,9 MB/s
>
> Any tests with a Raspberry Pi 3 (SD and Wifi over SDIO) are welcome.
>
> Stefan Wahren (2):
>   mmc: sdhci-iproc: Apply caps from bcm2835-mmc driver
>   mmc: sdhci-iproc: Increase max_blk_size for bcm2835
>
>  drivers/mmc/host/sdhci-iproc.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> --
> 1.7.9.5
>

Thanks, applied for next!

Kind regards
Uffe

^ permalink raw reply

* [PATCH v7 01/19] iommu/dma: Implement PCI allocation optimisation
From: Robin Murphy @ 2017-01-10 15:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483969570-3154-2-git-send-email-eric.auger@redhat.com>

On 09/01/17 13:45, Eric Auger wrote:
> From: Robin Murphy <robin.murphy@arm.com>
> 
> Whilst PCI devices may have 64-bit DMA masks, they still benefit from
> using 32-bit addresses wherever possible in order to avoid DAC (PCI) or
> longer address packets (PCIe), which may incur a performance overhead.
> Implement the same optimisation as other allocators by trying to get a
> 32-bit address first, only falling back to the full mask if that fails.

Oops, this was just some development work which got promoted to my misc
branch for safe keeping; it really has nothing to do with this series.

I'd managed to overlook that one of the __alloc_iova() conflicts hit the
MSI cookie patch, sorry. Things are now in a more appropriate order in
my tree.

Robin.

> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  drivers/iommu/dma-iommu.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 2db0d64..a59ca47 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -204,19 +204,28 @@ int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
>  }
>  
>  static struct iova *__alloc_iova(struct iommu_domain *domain, size_t size,
> -		dma_addr_t dma_limit)
> +		dma_addr_t dma_limit, struct device *dev)
>  {
>  	struct iova_domain *iovad = cookie_iovad(domain);
>  	unsigned long shift = iova_shift(iovad);
>  	unsigned long length = iova_align(iovad, size) >> shift;
> +	struct iova *iova = NULL;
>  
>  	if (domain->geometry.force_aperture)
>  		dma_limit = min(dma_limit, domain->geometry.aperture_end);
> +
> +	/* Try to get PCI devices a SAC address */
> +	if (dma_limit > DMA_BIT_MASK(32) && dev_is_pci(dev))
> +		iova = alloc_iova(iovad, length, DMA_BIT_MASK(32) >> shift,
> +				  true);
>  	/*
>  	 * Enforce size-alignment to be safe - there could perhaps be an
>  	 * attribute to control this per-device, or at least per-domain...
>  	 */
> -	return alloc_iova(iovad, length, dma_limit >> shift, true);
> +	if (!iova)
> +		iova = alloc_iova(iovad, length, dma_limit >> shift, true);
> +
> +	return iova;
>  }
>  
>  /* The IOVA allocator knows what we mapped, so just unmap whatever that was */
> @@ -369,7 +378,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
>  	if (!pages)
>  		return NULL;
>  
> -	iova = __alloc_iova(domain, size, dev->coherent_dma_mask);
> +	iova = __alloc_iova(domain, size, dev->coherent_dma_mask, dev);
>  	if (!iova)
>  		goto out_free_pages;
>  
> @@ -440,7 +449,7 @@ static dma_addr_t __iommu_dma_map(struct device *dev, phys_addr_t phys,
>  	struct iova_domain *iovad = cookie_iovad(domain);
>  	size_t iova_off = iova_offset(iovad, phys);
>  	size_t len = iova_align(iovad, size + iova_off);
> -	struct iova *iova = __alloc_iova(domain, len, dma_get_mask(dev));
> +	struct iova *iova = __alloc_iova(domain, len, dma_get_mask(dev), dev);
>  
>  	if (!iova)
>  		return DMA_ERROR_CODE;
> @@ -598,7 +607,7 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
>  		prev = s;
>  	}
>  
> -	iova = __alloc_iova(domain, iova_len, dma_get_mask(dev));
> +	iova = __alloc_iova(domain, iova_len, dma_get_mask(dev), dev);
>  	if (!iova)
>  		goto out_restore_sg;
>  
> @@ -675,7 +684,7 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
>  	if (!msi_page)
>  		return NULL;
>  
> -	iova = __alloc_iova(domain, iovad->granule, dma_get_mask(dev));
> +	iova = __alloc_iova(domain, iovad->granule, dma_get_mask(dev), dev);
>  	if (!iova)
>  		goto out_free_page;
>  
> 

^ permalink raw reply

* [PATCH v2 3/5] ARM: davinci_all_defconfig: enable iio and ADS7950
From: David Lechner @ 2017-01-10 15:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cb9455bc-2658-9826-823b-2aa845237fed@ti.com>

On 01/09/2017 06:29 AM, Sekhar Nori wrote:
> On Friday 06 January 2017 10:03 AM, David Lechner wrote:
>> This enables the iio subsystem and the TI ADS7950 driver. This is used by
>> LEGO MINDSTORMS EV3, which has an ADS7957 chip.
>
> Can you add your sign-off?
>
>> ---
>>
>> The CONFIG_TI_ADS7950 driver is currently in iio/testing, so some coordination
>> may be needed before picking up this patch.
>>
>>  arch/arm/configs/davinci_all_defconfig | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
>> index 2b1967a..a899876 100644
>> --- a/arch/arm/configs/davinci_all_defconfig
>> +++ b/arch/arm/configs/davinci_all_defconfig
>> @@ -200,6 +200,13 @@ CONFIG_TI_EDMA=y
>>  CONFIG_MEMORY=y
>>  CONFIG_TI_AEMIF=m
>>  CONFIG_DA8XX_DDRCTL=y
>> +CONFIG_IIO=m
>> +CONFIG_IIO_BUFFER_CB=m
>> +CONFIG_IIO_SW_DEVICE=m
>> +CONFIG_IIO_SW_TRIGGER=m
>
>> +CONFIG_TI_ADS7950=m
>
> Can you separate this from rest of the patch. I would like to enable
> this option only after I can find the symbol in linux-next.

Will resend without CONFIG_TI_ADS7950

>
>> +CONFIG_IIO_HRTIMER_TRIGGER=m
>> +CONFIG_IIO_SYSFS_TRIGGER=m
>
> Need CONFIG_IIO_TRIGGER=y also for these two options to take effect.

CONFIG_IIO_TRIGGER is selected by IIO_TRIGGERED_BUFFER [=m] && IIO [=m] 
&& IIO_BUFFER [=y], so save_defconfig does not pick it up.


>
> Thanks,
> Sekhar
>

^ permalink raw reply

* [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
From: Petr Mladek @ 2017-01-10 15:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161208224655.GA6319@gce>

On Thu 2016-12-08 22:46:55, Abel Vesa wrote:
> On Thu, Dec 08, 2016 at 09:46:35PM +0000, Abel Vesa wrote:
> > From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
> > 
> > From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
> 
> >From statement twice in the commit message. Will resend.
> > 
> > The DYNAMIC_FTRACE_WITH_REGS configuration makes it possible for a ftrace
> > operation to specify if registers need to saved/restored by the ftrace handler.
> > This is needed by kgraft and possibly other ftrace-based tools, and the ARM
> > architecture is currently lacking this feature. It would also be the first step
> > to support the "Kprobes-on-ftrace" optimization on ARM.
> > 
> > This patch introduces a new ftrace handler that stores the registers on the
> > stack before calling the next stage. The registers are restored from the stack
> > before going back to the instrumented function.
> > 
> > A side-effect of this patch is to activate the support for ftrace_modify_call()
> > as it defines ARCH_SUPPORTS_FTRACE_OPS for the ARM architecture
> > 
> > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
> > Signed-off-by: Abel Vesa <abelvesa@linux.com>
> > ---
> >  arch/arm/Kconfig               |  2 ++
> >  arch/arm/include/asm/ftrace.h  |  4 +++
> >  arch/arm/kernel/entry-ftrace.S | 78 ++++++++++++++++++++++++++++++++++++++++++
> >  arch/arm/kernel/ftrace.c       | 33 ++++++++++++++++++
> >  4 files changed, 117 insertions(+)
> > 
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index b5d529f..87f1a9f 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -50,6 +50,7 @@ config ARM
> >  	select HAVE_DMA_API_DEBUG
> >  	select HAVE_DMA_CONTIGUOUS if MMU
> >  	select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
> > +	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> >  	select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
> >  	select HAVE_EXIT_THREAD
> >  	select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
> > @@ -90,6 +91,7 @@ config ARM
> >  	select PERF_USE_VMALLOC
> >  	select RTC_LIB
> >  	select SYS_SUPPORTS_APM_EMULATION
> > +	select FRAME_POINTER if DYNAMIC_FTRACE_WITH_REGS && FUNCTION_GRAPH_TRACER

FRAME_POINTER is not for free. It takes space on the stack. Also there
is a performance penalty. Do we really need to depend on it? If so,
it might be worth a note in the commit message.

I made only a quick look at the patch. It looks reasonable. But I do
not have enough knowledge about the arm architecture, assembly, and
ftrace-specifics. Also I cannot test it easily. So issues might
be hidden to my eyes.

Best Regards,
Petr

^ permalink raw reply

* [PATCH] virtio_mmio: Set DMA masks appropriately
From: Robin Murphy @ 2017-01-10 15:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170110165221-mutt-send-email-mst@kernel.org>

On 10/01/17 14:54, Michael S. Tsirkin wrote:
> On Tue, Jan 10, 2017 at 12:26:01PM +0000, Robin Murphy wrote:
>> Once DMA API usage is enabled, it becomes apparent that virtio-mmio is
>> inadvertently relying on the default 32-bit DMA mask, which leads to
>> problems like rapidly exhausting SWIOTLB bounce buffers.
>>
>> Ensure that we set the appropriate 64-bit DMA mask whenever possible,
>> with the coherent mask suitably limited for the legacy vring as per
>> a0be1db4304f ("virtio_pci: Limit DMA mask to 44 bits for legacy virtio
>> devices").
>>
>> Reported-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
>> Fixes: b42111382f0e ("virtio_mmio: Use the DMA API if enabled")
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>  drivers/virtio/virtio_mmio.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
>> index 48bfea91dbca..b5c5d49ca598 100644
>> --- a/drivers/virtio/virtio_mmio.c
>> +++ b/drivers/virtio/virtio_mmio.c
>> @@ -59,6 +59,7 @@
>>  #define pr_fmt(fmt) "virtio-mmio: " fmt
>>  
>>  #include <linux/acpi.h>
>> +#include <linux/dma-mapping.h>
>>  #include <linux/highmem.h>
>>  #include <linux/interrupt.h>
>>  #include <linux/io.h>
>> @@ -497,6 +498,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>>  	struct virtio_mmio_device *vm_dev;
>>  	struct resource *mem;
>>  	unsigned long magic;
>> +	int rc;
>>  
>>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>  	if (!mem)
>> @@ -548,6 +550,14 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>>  	if (vm_dev->version == 1)
>>  		writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
>>  
>> +	rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>> +	if (rc)
>> +		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>> +	else if (vm_dev->version == 1)
>> +		dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32 + PAGE_SHIFT));
> 
> That's a very convoluted way to do this, for version 1 you
> set coherent mask to 64 then override it.
> why not
> 
> if (vm_dev->version == 1) {
> 	dma_set_mask
> 	dma_set_coherent_mask
> } else {
> 	dma_set_mask_and_coherent
> }
> 
> if (rc)
> 	dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));

Purely because it's fewer lines of code - if you'd prefer separate
legacy vs. modern flows for clarity that's fine by me.

>> +	if (rc)
>> +		dev_warn(&pdev->dev, "Failed to enable 64-bit or 32-bit DMA.  Trying to continue, but this might not work.\n");
>> +
> 
> is there a chance it actually still might work?

If we're not actually using the DMA API, we may still get away with it,
otherwise it's a fairly sure bet that the subsequent dma_map/dma_alloc
calls will fail and we'll get nowhere. If I change this to be a probe
failure condition (and correspondingly in the PCI drivers too), would
you rather that be predicated on vring_use_dma_api(), or always (given
the TODO in virtio_ring.c)?

Robin.

>>  	platform_set_drvdata(pdev, vm_dev);
>>  
>>  	return register_virtio_device(&vm_dev->vdev);
>> -- 
>> 2.10.2.dirty

^ permalink raw reply

* [PATCH v7 00/19] KVM PCIe/MSI passthrough on ARM/ARM64 and IOVA reserved regions
From: Auger Eric @ 2017-01-10 15:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170110140924.GC1318@8bytes.org>

Hi all,

On 10/01/2017 15:09, Joerg Roedel wrote:
> On Mon, Jan 09, 2017 at 01:45:51PM +0000, Eric Auger wrote:
>> Eric Auger (17):
>>   iommu: Rename iommu_dm_regions into iommu_resv_regions
>>   iommu: Add a new type field in iommu_resv_region
>>   iommu: iommu_alloc_resv_region
>>   iommu: Only map direct mapped regions
>>   iommu: iommu_get_group_resv_regions
>>   iommu: Implement reserved_regions iommu-group sysfs file
>>   iommu/vt-d: Implement reserved region get/put callbacks
>>   iommu/amd: Declare MSI and HT regions as reserved IOVA regions
>>   iommu/arm-smmu: Implement reserved region get/put callbacks
>>   iommu/arm-smmu-v3: Implement reserved region get/put callbacks
> 
> IOMMU patches look good, what is the plan to merge this? I'd like to
> take the IOMMU patches and can provide a branch for someone else to base
> the rest on.

I will respin asap taking into account Marc's nits, Robin's update. Also
I would like to do one change in

[PATCH v7 08/19] iommu: Implement reserved_regions iommu-group sysfs file

I will send a separate email.

Thanks

Eric

> 
> 
> 	Joerg
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* [PATCH] ARM: davinci_all_defconfig: enable iio
From: David Lechner @ 2017-01-10 16:00 UTC (permalink / raw)
  To: linux-arm-kernel

This enables the iio subsystem. This will be used by LEGO MINDSTORMS EV3,
which has a TI ADS7957 A/DC chip.

Signed-off-by: David Lechner <david@lechnology.com>
---
 arch/arm/configs/davinci_all_defconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
index 2b1967a..0faf238 100644
--- a/arch/arm/configs/davinci_all_defconfig
+++ b/arch/arm/configs/davinci_all_defconfig
@@ -200,6 +200,12 @@ CONFIG_TI_EDMA=y
 CONFIG_MEMORY=y
 CONFIG_TI_AEMIF=m
 CONFIG_DA8XX_DDRCTL=y
+CONFIG_IIO=m
+CONFIG_IIO_BUFFER_CB=m
+CONFIG_IIO_SW_DEVICE=m
+CONFIG_IIO_SW_TRIGGER=m
+CONFIG_IIO_HRTIMER_TRIGGER=m
+CONFIG_IIO_SYSFS_TRIGGER=m
 CONFIG_PWM=y
 CONFIG_PWM_TIECAP=m
 CONFIG_PWM_TIEHRPWM=m
-- 
2.7.4

^ permalink raw reply related

* [PATCH] usb: dwc3-exynos fix unspecified suspend clk error handling
From: Bartlomiej Zolnierkiewicz @ 2017-01-10 16:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3a113696-d0cd-8944-4acd-c87646fbee15@osg.samsung.com>


Hi,

On Tuesday, January 10, 2017 07:36:35 AM Shuah Khan wrote:
> On 01/10/2017 07:16 AM, Shuah Khan wrote:
> > On 01/10/2017 05:05 AM, Bartlomiej Zolnierkiewicz wrote:
> >>
> >> Hi,
> >>
> >> On Monday, January 09, 2017 07:21:31 PM Shuah Khan wrote:
> >>> Fix dwc3_exynos_probe() to call clk_prepare_enable() only when suspend
> >>> clock is specified. Call clk_disable_unprepare() from remove and probe
> >>> error path only when susp_clk has been set from remove and probe error
> >>> paths.
> >>
> >> It is legal to call clk_prepare_enable() and clk_disable_unprepare()
> >> for NULL clock.  Also your patch changes susp_clk handling while
> >> leaves axius_clk handling (which also can be NULL) untouched.
> >>
> >> Do you actually see some runtime problem with the current code?
> >>
> >> If not then the patch should probably be dropped.
> >>
> >> Best regards,
> >> --
> >> Bartlomiej Zolnierkiewicz
> >> Samsung R&D Institute Poland
> >> Samsung Electronics
> > 
> > Hi Bartlomiej,
> > 
> > I am seeing the "no suspend clk specified" message in dmesg.
> > After that it sets the exynos->susp_clk = NULL and starts
> > calling clk_prepare_enable(exynos->susp_clk);
> > 
> > That can't be good. If you see the logic right above this
> > one for exynos->clk, it returns error and fails the probe.
> > This this case it doesn't, but tries to use null susp_clk.

exynos->susp_clk is optional, exynos->clk is not.

> > I believe this patch is necessary.
> 
> Let me clarify this a bit further. Since we already know
> susp_clk is null, with this patch we can avoid extra calls
> to clk_prepare_enable() and clk_disable_unprepare().
> 
> One can say, it also adds extra checks, hence I will let you
> decide one way or the other. :)

I would prefer to leave the things as they are currently.

The code in question is not performance sensitive so extra
calls are not a problem.  No extra checks means less code.

Also the current code seems to be more in line with the rest
of the kernel.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> thanks,
> -- Shuah
> 
> > 
> > thanks,
> > -- Shuah
> > 
> >>
> >>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> >>> ---
> >>>  drivers/usb/dwc3/dwc3-exynos.c | 10 ++++++----
> >>>  1 file changed, 6 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> >>> index e27899b..f97a3d7 100644
> >>> --- a/drivers/usb/dwc3/dwc3-exynos.c
> >>> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> >>> @@ -131,8 +131,8 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>  	if (IS_ERR(exynos->susp_clk)) {
> >>>  		dev_info(dev, "no suspend clk specified\n");
> >>>  		exynos->susp_clk = NULL;
> >>> -	}
> >>> -	clk_prepare_enable(exynos->susp_clk);
> >>> +	} else
> >>> +		clk_prepare_enable(exynos->susp_clk);
> >>>  
> >>>  	if (of_device_is_compatible(node, "samsung,exynos7-dwusb3")) {
> >>>  		exynos->axius_clk = devm_clk_get(dev, "usbdrd30_axius_clk");
> >>> @@ -196,7 +196,8 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>  	regulator_disable(exynos->vdd33);
> >>>  err2:
> >>>  	clk_disable_unprepare(exynos->axius_clk);
> >>> -	clk_disable_unprepare(exynos->susp_clk);
> >>> +	if (exynos->susp_clk)
> >>> +		clk_disable_unprepare(exynos->susp_clk);
> >>>  	clk_disable_unprepare(exynos->clk);
> >>>  	return ret;
> >>>  }
> >>> @@ -210,7 +211,8 @@ static int dwc3_exynos_remove(struct platform_device *pdev)
> >>>  	platform_device_unregister(exynos->usb3_phy);
> >>>  
> >>>  	clk_disable_unprepare(exynos->axius_clk);
> >>> -	clk_disable_unprepare(exynos->susp_clk);
> >>> +	if (exynos->susp_clk)
> >>> +		clk_disable_unprepare(exynos->susp_clk);
> >>>  	clk_disable_unprepare(exynos->clk);
> >>>  
> >>>  	regulator_disable(exynos->vdd33);

^ permalink raw reply

* [PATCH] virtio_mmio: Set DMA masks appropriately
From: Michael S. Tsirkin @ 2017-01-10 16:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <b60d0de1-a79b-50c0-f773-5a509b0e94d5@arm.com>

On Tue, Jan 10, 2017 at 03:55:31PM +0000, Robin Murphy wrote:
> On 10/01/17 14:54, Michael S. Tsirkin wrote:
> > On Tue, Jan 10, 2017 at 12:26:01PM +0000, Robin Murphy wrote:
> >> Once DMA API usage is enabled, it becomes apparent that virtio-mmio is
> >> inadvertently relying on the default 32-bit DMA mask, which leads to
> >> problems like rapidly exhausting SWIOTLB bounce buffers.
> >>
> >> Ensure that we set the appropriate 64-bit DMA mask whenever possible,
> >> with the coherent mask suitably limited for the legacy vring as per
> >> a0be1db4304f ("virtio_pci: Limit DMA mask to 44 bits for legacy virtio
> >> devices").
> >>
> >> Reported-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> >> Fixes: b42111382f0e ("virtio_mmio: Use the DMA API if enabled")
> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> >> ---
> >>  drivers/virtio/virtio_mmio.c | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> >> index 48bfea91dbca..b5c5d49ca598 100644
> >> --- a/drivers/virtio/virtio_mmio.c
> >> +++ b/drivers/virtio/virtio_mmio.c
> >> @@ -59,6 +59,7 @@
> >>  #define pr_fmt(fmt) "virtio-mmio: " fmt
> >>  
> >>  #include <linux/acpi.h>
> >> +#include <linux/dma-mapping.h>
> >>  #include <linux/highmem.h>
> >>  #include <linux/interrupt.h>
> >>  #include <linux/io.h>
> >> @@ -497,6 +498,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
> >>  	struct virtio_mmio_device *vm_dev;
> >>  	struct resource *mem;
> >>  	unsigned long magic;
> >> +	int rc;
> >>  
> >>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>  	if (!mem)
> >> @@ -548,6 +550,14 @@ static int virtio_mmio_probe(struct platform_device *pdev)
> >>  	if (vm_dev->version == 1)
> >>  		writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
> >>  
> >> +	rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> >> +	if (rc)
> >> +		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> >> +	else if (vm_dev->version == 1)
> >> +		dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32 + PAGE_SHIFT));
> > 
> > That's a very convoluted way to do this, for version 1 you
> > set coherent mask to 64 then override it.
> > why not
> > 
> > if (vm_dev->version == 1) {
> > 	dma_set_mask
> > 	dma_set_coherent_mask
> > } else {
> > 	dma_set_mask_and_coherent
> > }
> > 
> > if (rc)
> > 	dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> 
> Purely because it's fewer lines of code - if you'd prefer separate
> legacy vs. modern flows for clarity that's fine by me.

Yes please. It's less legacy vs modern and more that I don't like
line 2 undoig what line 1 did.

> >> +	if (rc)
> >> +		dev_warn(&pdev->dev, "Failed to enable 64-bit or 32-bit DMA.  Trying to continue, but this might not work.\n");
> >> +
> > 
> > is there a chance it actually still might work?
> 
> If we're not actually using the DMA API, we may still get away with it,
> otherwise it's a fairly sure bet that the subsequent dma_map/dma_alloc
> calls will fail and we'll get nowhere. If I change this to be a probe
> failure condition (and correspondingly in the PCI drivers too), would
> you rather that be predicated on vring_use_dma_api(), or always (given
> the TODO in virtio_ring.c)?
> 
> Robin.

Oh I see you are just mirroring what pci does. I think it's fine.

> >>  	platform_set_drvdata(pdev, vm_dev);
> >>  
> >>  	return register_virtio_device(&vm_dev->vdev);
> >> -- 
> >> 2.10.2.dirty

^ permalink raw reply

* [PATCH] of: alloc anywhere from memblock if range not specified
From: Leonard Crestez @ 2017-01-10 16:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1456148744-7583-1-git-send-email-vinmenon@codeaurora.org>

Hello,

I have some trouble with this patch.

It seems the intention is to allow CMA to be placed in highmem. If the 
CMA area is larger than highmem and no alloc-ranges is specified (just a 
size) it is possible to end up allocating a area that spans from 
multiple zones. This later breaks checks in cma_activate_area and makes 
most dma allocations fail.

Am I missing something or this a bug?

On Mon, Feb 22, 2016 at 3:45 PM, Vinayak Menon <vinmenon@codeaurora.org> 
wrote:
>
> early_init_dt_alloc_reserved_memory_arch passes end as 0 to
> __memblock_alloc_base, when limits are not specified. But
> __memblock_alloc_base takes end value of 0 as MEMBLOCK_ALLOC_ACCESSIBLE
> and limits the end to memblock.current_limit. This results in regions
> never being placed in HIGHMEM area, for e.g. CMA.
> Let __memblock_alloc_base allocate from anywhere in memory if limits are
> not specified.
>
> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
> ---
>  drivers/of/of_reserved_mem.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 1a3556a..ed01c01 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -32,11 +32,13 @@ int __init __weak early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>         phys_addr_t align, phys_addr_t start, phys_addr_t end, bool nomap,
>         phys_addr_t *res_base)
>  {
> +       phys_addr_t base;
>         /*
>          * We use __memblock_alloc_base() because memblock_alloc_base()
>          * panic()s on allocation failure.
>          */
> -       phys_addr_t base = __memblock_alloc_base(size, align, end);
> +       end = !end ? MEMBLOCK_ALLOC_ANYWHERE : end;
> +       base = __memblock_alloc_base(size, align, end);
>         if (!base)
>                 return -ENOMEM;
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> member of the Code Aurora Forum, hosted by The Linux Foundation
>

^ permalink raw reply

* noise issues when recording sound on i.MX28
From: Jörg Krause @ 2017-01-10 16:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160217085502.39e4cb24@ipc1.ka-ro>

Hi all,

On Wed, 2016-02-17 at 08:55 +0100, Lothar Wa?mann wrote:
> Hi,
> 
> On Tue, 16 Feb 2016 13:01:19 -0200 Fabio Estevam wrote:
> > On Thu, Feb 11, 2016 at 6:40 AM, Uwe Kleine-K?nig
> > <u.kleine-koenig@pengutronix.de> wrote:
> > 
> > > root at hostname:/sys/kernel/debug/clk find -name saif?_sel
> > > ./ref_xtal/pll0/saif0_sel
> > > ./ref_xtal/pll0/saif1_sel
> > > 
> > > root at hostname:/sys/kernel/debug/clk cat
> > > ./ref_xtal/pll0/saif?_sel/clk_rate
> > > 480000000
> > > 480000000
> > 
> > Ok, what about the SAIF0 and SAIF1 sampling rates: are they always
> > the same?
> > 
> 
> Yes, of course. The driver wouldn't allow using both interfaces with
> different sample rates any way.

Are there any updates about this issue?

I am facing the same problem when using SAIF1 for capture and SAIF0 for
playback. SAIF1 uses SAIF0 as the master and I do set
MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0 when bringing up the board. However,
whenever I record with

$ arecord -D hw:0,1 -t wav -c 2 -r 48000 -f S16_LE -d 5 /tmp/out.wav

and playback afterwards:

$ aplay -D hw:0,0 /tmp/out.wav

I only get noise.

Note, that I am using the Audio Codec Board from Mikroelectronika, but
replaced the on-board 12.288MHz quarz with the MCLK line from SAIF0.

Best regards,
J?rg Krause

^ permalink raw reply

* [PATCH 1/3] ARM: at91: flush the L2 cache before entering cpu idle
From: Alexandre Belloni @ 2017-01-10 16:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170106065947.30631-2-wenyou.yang@atmel.com>

I though a bit more about it, and I don't really like the new compatible
string. I don't feel this should be necessary.

What about the following:

diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index b4332b727e9c..0333aca63e44 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -53,6 +53,7 @@ extern void at91_pinctrl_gpio_resume(void);
 static struct {
 	unsigned long uhp_udp_mask;
 	int memctrl;
+	bool has_l2_cache;
 } at91_pm_data;
 
 void __iomem *at91_ramc_base[2];
@@ -267,6 +268,11 @@ static void at91_ddr_standby(void)
 	u32 lpr0, lpr1 = 0;
 	u32 saved_lpr0, saved_lpr1 = 0;
 
+	if (at91_pm_data.has_l2_cache) {
+		flush_cache_all();
+		outer_disable();
+	}
+
 	if (at91_ramc_base[1]) {
 		saved_lpr1 = at91_ramc_read(1, AT91_DDRSDRC_LPR);
 		lpr1 = saved_lpr1 & ~AT91_DDRSDRC_LPCB;
@@ -287,6 +293,9 @@ static void at91_ddr_standby(void)
 	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr0);
 	if (at91_ramc_base[1])
 		at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
+
+	if (at91_pm_data.has_l2_cache)
+		outer_resume();
 }
 
 /* We manage both DDRAM/SDRAM controllers, we need more than one value
 * to
@@ -353,6 +362,11 @@ static __init void at91_dt_ramc(void)
 		return;
 	}
 
+	np = of_find_compatible_node(NULL, NULL, "arm,pl310-cache");
+	if (np)
+		at91_pm_data.has_l2_cache = true;
+	of_node_put(np);
+
 	at91_pm_set_standby(standby);
 }


This has the following benefits:
 - everybody will have the fix, regardless of whether the dtb is updated
 - has_l2_cache can be used later in at91_pm_suspend instead of calling
   it unconditionnaly (I'll send a patch)


On 06/01/2017 at 14:59:45 +0800, Wenyou Yang wrote :
> For the SoCs such as SAMA5D2 and SAMA5D4 which have L2 cache,
> flush the L2 cache first before entering the cpu idle.
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> ---
> 
>  arch/arm/mach-at91/pm.c       | 19 +++++++++++++++++++
>  drivers/memory/atmel-sdramc.c |  1 +
>  2 files changed, 20 insertions(+)
> 
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index b4332b727e9c..1a60dede1a01 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -289,6 +289,24 @@ static void at91_ddr_standby(void)
>  		at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
>  }
>  
> +static void at91_ddr_cache_standby(void)
> +{
> +	u32 saved_lpr;
> +
> +	flush_cache_all();
> +	outer_disable();
> +
> +	saved_lpr = at91_ramc_read(0, AT91_DDRSDRC_LPR);
> +	at91_ramc_write(0, AT91_DDRSDRC_LPR, (saved_lpr &
> +			(~AT91_DDRSDRC_LPCB)) | AT91_DDRSDRC_LPCB_SELF_REFRESH);
> +
> +	cpu_do_idle();
> +
> +	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr);
> +
> +	outer_resume();
> +}
> +
>  /* We manage both DDRAM/SDRAM controllers, we need more than one value to
>   * remember.
>   */
> @@ -324,6 +342,7 @@ static const struct of_device_id const ramc_ids[] __initconst = {
>  	{ .compatible = "atmel,at91sam9260-sdramc", .data = at91sam9_sdram_standby },
>  	{ .compatible = "atmel,at91sam9g45-ddramc", .data = at91_ddr_standby },
>  	{ .compatible = "atmel,sama5d3-ddramc", .data = at91_ddr_standby },
> +	{ .compatible = "atmel,sama5d4-ddramc", .data = at91_ddr_cache_standby },
>  	{ /*sentinel*/ }
>  };
>  
> diff --git a/drivers/memory/atmel-sdramc.c b/drivers/memory/atmel-sdramc.c
> index b418b39af180..7e5c5c6c1348 100644
> --- a/drivers/memory/atmel-sdramc.c
> +++ b/drivers/memory/atmel-sdramc.c
> @@ -48,6 +48,7 @@ static const struct of_device_id atmel_ramc_of_match[] = {
>  	{ .compatible = "atmel,at91sam9260-sdramc", .data = &at91rm9200_caps, },
>  	{ .compatible = "atmel,at91sam9g45-ddramc", .data = &at91sam9g45_caps, },
>  	{ .compatible = "atmel,sama5d3-ddramc", .data = &sama5d3_caps, },
> +	{ .compatible = "atmel,sama5d4-ddramc", .data = &sama5d3_caps, },
>  	{},
>  };
>  
> -- 
> 2.11.0
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply related

* [RFC 00/55] Nested Virtualization on KVM/ARM
From: Jintack Lim @ 2017-01-10 16:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <7991b5fa-cbd3-a3b3-3f1c-8b8655ebdcfd@redhat.com>

On Mon, Jan 9, 2017 at 10:05 AM, David Hildenbrand <david@redhat.com> wrote:
>
>> Even though this work is not complete (see limitations below), I'd
>> appreciate
>> early feedback on this RFC. Specifically, I'm interested in:
>> - Is it better to have a kernel config or to make it configurable at
>> runtime?
>
>
> x86 and s390x have a kernel module parameter (nested) that can only be
> changed when loading the module and should default to false. So the
> admin explicitly has to enable it. Maybe going the same path makes
> sense.

I think that makes sense. Thanks!

>
> --
>
> David
>

^ permalink raw reply

* [PATCH v7 08/19] iommu: Implement reserved_regions iommu-group sysfs file
From: Auger Eric @ 2017-01-10 16:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483969570-3154-9-git-send-email-eric.auger@redhat.com>

Hi Joerg,

On 09/01/2017 14:45, Eric Auger wrote:
> A new iommu-group sysfs attribute file is introduced. It contains
> the list of reserved regions for the iommu-group. Each reserved
> region is described on a separate line:
> - first field is the start IOVA address,
> - second is the end IOVA address,
> - third is the type.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v6 -> v7:
> - also report the type of the reserved region as a string
> - updated ABI documentation
> 
> v3 -> v4:
> - add cast to long long int when printing to avoid warning on
>   i386
> - change S_IRUGO into 0444
> - remove sort. The list is natively sorted now.
> 
> The file layout is inspired of /sys/bus/pci/devices/BDF/resource.
> I also read Documentation/filesystems/sysfs.txt so I expect this
> to be frowned upon.
> ---
>  .../ABI/testing/sysfs-kernel-iommu_groups          | 12 +++++++
>  drivers/iommu/iommu.c                              | 38 ++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-kernel-iommu_groups b/Documentation/ABI/testing/sysfs-kernel-iommu_groups
> index 9b31556..35c64e0 100644
> --- a/Documentation/ABI/testing/sysfs-kernel-iommu_groups
> +++ b/Documentation/ABI/testing/sysfs-kernel-iommu_groups
> @@ -12,3 +12,15 @@ Description:	/sys/kernel/iommu_groups/ contains a number of sub-
>  		file if the IOMMU driver has chosen to register a more
>  		common name for the group.
>  Users:
> +
> +What:		/sys/kernel/iommu_groups/reserved_regions
> +Date: 		January 2017
> +KernelVersion:  v4.11
> +Contact: 	Eric Auger <eric.auger@redhat.com>
> +Description:    /sys/kernel/iommu_groups/reserved_regions list IOVA
> +		regions that are reserved. Not necessarily all
> +		reserved regions are listed. This is typically used to
> +		output direct-mapped, MSI, non mappable regions. Each
> +		region is described on a single line: the 1st field is
> +		the base IOVA, the second is the end IOVA and the third
> +		field describes the type of the region.
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 640056b..0123daa 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -68,6 +68,12 @@ struct iommu_group_attribute {
>  			 const char *buf, size_t count);
>  };
>  
> +static const char * const iommu_group_resv_type_string[] = {
> +	[IOMMU_RESV_DIRECT]	= "direct",
> +	[IOMMU_RESV_RESERVED]	= "reserved",
> +	[IOMMU_RESV_MSI]	= "msi",
> +};
> +
>  #define IOMMU_GROUP_ATTR(_name, _mode, _show, _store)		\
>  struct iommu_group_attribute iommu_group_attr_##_name =		\
>  	__ATTR(_name, _mode, _show, _store)
> @@ -231,8 +237,33 @@ int iommu_get_group_resv_regions(struct iommu_group *group,
>  }
>  EXPORT_SYMBOL_GPL(iommu_get_group_resv_regions);
>  
> +static ssize_t iommu_group_show_resv_regions(struct iommu_group *group,
> +					     char *buf)
> +{
> +	struct iommu_resv_region *region, *next;
> +	struct list_head group_resv_regions;
> +	char *str = buf;
> +
> +	INIT_LIST_HEAD(&group_resv_regions);
> +	iommu_get_group_resv_regions(group, &group_resv_regions);
> +
> +	list_for_each_entry_safe(region, next, &group_resv_regions, list) {
> +		str += sprintf(str, "0x%016llx 0x%016llx %s\n",
> +			       (long long int)region->start,
> +			       (long long int)(region->start +
> +						region->length - 1),
> +			       iommu_group_resv_type_string[region->type]);
> +		kfree(region);
> +	}
> +
> +	return (str - buf);
> +}
> +
>  static IOMMU_GROUP_ATTR(name, S_IRUGO, iommu_group_show_name, NULL);
>  
> +static IOMMU_GROUP_ATTR(reserved_regions, 0444,
> +			iommu_group_show_resv_regions, NULL);
> +
>  static void iommu_group_release(struct kobject *kobj)
>  {
>  	struct iommu_group *group = to_iommu_group(kobj);
> @@ -247,6 +278,8 @@ static void iommu_group_release(struct kobject *kobj)
>  	if (group->default_domain)
>  		iommu_domain_free(group->default_domain);
>  
> +	iommu_group_remove_file(group, &iommu_group_attr_reserved_regions);

The /sys/kernel/iommu_groups/n directory seems to be removed before this
gets called and this may produce a WARNING when devices get removed from
the group. I intend to remove the call since I have the feeling
everything gets cleaned up properly.

Do you see any issue?

Thanks

Eric

[  350.753618] iommu: Removing device 0000:01:10.0 from group 7
[  350.759331] kernfs: can not remove 'reserved_regions', no directory
[  350.765603] ------------[ cut here ]------------
[  350.770216] WARNING: CPU: 3 PID: 2617 at fs/kernfs/dir.c:1406
kernfs_remove_by_name_ns+0x8c/0x98
../..
[  351.028154] [<ffff00000828819c>] kernfs_remove_by_name_ns+0x8c/0x98
[  351.034414] [<ffff000008289bf8>] sysfs_remove_file_ns+0x14/0x1c
[  351.040325] [<ffff00000856d918>] iommu_group_release+0x68/0x9c
[  351.046150] [<ffff0000083d152c>] kobject_cleanup+0x8c/0x194
[  351.051715] [<ffff0000083d13c8>] kobject_put+0x44/0x6c
[  351.056844] [<ffff0000083d1490>] kobject_del+0x40/0x50
[  351.061974] [<ffff0000083d1508>] kobject_cleanup+0x68/0x194
[  351.067538] [<ffff0000083d13c8>] kobject_put+0x44/0x6c
[  351.072668] [<ffff00000856df30>] iommu_group_remove_device+0x128/0x1d4
[  351.079187] [<ffff000008575eb0>] arm_smmu_remove_device+0x12c/0x158




> +
>  	kfree(group->name);
>  	kfree(group);
>  }
> @@ -310,6 +343,11 @@ struct iommu_group *iommu_group_alloc(void)
>  	 */
>  	kobject_put(&group->kobj);
>  
> +	ret = iommu_group_create_file(group,
> +				      &iommu_group_attr_reserved_regions);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
>  	pr_debug("Allocated group %d\n", group->id);
>  
>  	return group;
> 

^ permalink raw reply

* [PATCH] usb: dwc3-exynos fix unspecified suspend clk error handling
From: Shuah Khan @ 2017-01-10 16:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8784459.rxWqZlDLVg@amdc3058>

On 01/10/2017 09:05 AM, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Tuesday, January 10, 2017 07:36:35 AM Shuah Khan wrote:
>> On 01/10/2017 07:16 AM, Shuah Khan wrote:
>>> On 01/10/2017 05:05 AM, Bartlomiej Zolnierkiewicz wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Monday, January 09, 2017 07:21:31 PM Shuah Khan wrote:
>>>>> Fix dwc3_exynos_probe() to call clk_prepare_enable() only when suspend
>>>>> clock is specified. Call clk_disable_unprepare() from remove and probe
>>>>> error path only when susp_clk has been set from remove and probe error
>>>>> paths.
>>>>
>>>> It is legal to call clk_prepare_enable() and clk_disable_unprepare()
>>>> for NULL clock.  Also your patch changes susp_clk handling while
>>>> leaves axius_clk handling (which also can be NULL) untouched.
>>>>
>>>> Do you actually see some runtime problem with the current code?
>>>>
>>>> If not then the patch should probably be dropped.
>>>>
>>>> Best regards,
>>>> --
>>>> Bartlomiej Zolnierkiewicz
>>>> Samsung R&D Institute Poland
>>>> Samsung Electronics
>>>
>>> Hi Bartlomiej,
>>>
>>> I am seeing the "no suspend clk specified" message in dmesg.
>>> After that it sets the exynos->susp_clk = NULL and starts
>>> calling clk_prepare_enable(exynos->susp_clk);
>>>
>>> That can't be good. If you see the logic right above this
>>> one for exynos->clk, it returns error and fails the probe.
>>> This this case it doesn't, but tries to use null susp_clk.
> 
> exynos->susp_clk is optional, exynos->clk is not.

Right. That is clear since we don't fail the probe.

> 
>>> I believe this patch is necessary.
>>
>> Let me clarify this a bit further. Since we already know
>> susp_clk is null, with this patch we can avoid extra calls
>> to clk_prepare_enable() and clk_disable_unprepare().
>>
>> One can say, it also adds extra checks, hence I will let you
>> decide one way or the other. :)
> 
> I would prefer to leave the things as they are currently.
> 
> The code in question is not performance sensitive so extra
> calls are not a problem.  No extra checks means less code.
> 
> Also the current code seems to be more in line with the rest
> of the kernel.

What functionality is missing without the suspend clock? Would
it make sense to change the info. message to include what it
means. At the moment it doesn't anything more than "no suspend
clock" which is a very cryptic user visible message. It would be
helpful for it to also include what functionality is impacted.

thanks,
-- Shuah

> 
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 
>> thanks,
>> -- Shuah
>>
>>>
>>> thanks,
>>> -- Shuah
>>>
>>>>
>>>>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>>>>> ---
>>>>>  drivers/usb/dwc3/dwc3-exynos.c | 10 ++++++----
>>>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
>>>>> index e27899b..f97a3d7 100644
>>>>> --- a/drivers/usb/dwc3/dwc3-exynos.c
>>>>> +++ b/drivers/usb/dwc3/dwc3-exynos.c
>>>>> @@ -131,8 +131,8 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>>>>>  	if (IS_ERR(exynos->susp_clk)) {
>>>>>  		dev_info(dev, "no suspend clk specified\n");
>>>>>  		exynos->susp_clk = NULL;
>>>>> -	}
>>>>> -	clk_prepare_enable(exynos->susp_clk);
>>>>> +	} else
>>>>> +		clk_prepare_enable(exynos->susp_clk);
>>>>>  
>>>>>  	if (of_device_is_compatible(node, "samsung,exynos7-dwusb3")) {
>>>>>  		exynos->axius_clk = devm_clk_get(dev, "usbdrd30_axius_clk");
>>>>> @@ -196,7 +196,8 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>>>>>  	regulator_disable(exynos->vdd33);
>>>>>  err2:
>>>>>  	clk_disable_unprepare(exynos->axius_clk);
>>>>> -	clk_disable_unprepare(exynos->susp_clk);
>>>>> +	if (exynos->susp_clk)
>>>>> +		clk_disable_unprepare(exynos->susp_clk);
>>>>>  	clk_disable_unprepare(exynos->clk);
>>>>>  	return ret;
>>>>>  }
>>>>> @@ -210,7 +211,8 @@ static int dwc3_exynos_remove(struct platform_device *pdev)
>>>>>  	platform_device_unregister(exynos->usb3_phy);
>>>>>  
>>>>>  	clk_disable_unprepare(exynos->axius_clk);
>>>>> -	clk_disable_unprepare(exynos->susp_clk);
>>>>> +	if (exynos->susp_clk)
>>>>> +		clk_disable_unprepare(exynos->susp_clk);
>>>>>  	clk_disable_unprepare(exynos->clk);
>>>>>  
>>>>>  	regulator_disable(exynos->vdd33);
> 

^ permalink raw reply

* [PATCH 1/2] ARM: dts: imx6: Specify 'anatop-enable-bit' where appropriate
From: Andrey Smirnov @ 2017-01-10 16:30 UTC (permalink / raw)
  To: linux-arm-kernel

ENABLE_LINREG bit is implemented by 3P0, 1P1 and 2P5 regulators on
i.MX6. This property is present in similar code in Fresscale BSP and
made its way upstream in imx6ul.dtsi, so this patch adds this property
to the rest of i.MX6 family for completness.

Cc: yurovsky at gmail.com
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: devicetree at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/boot/dts/imx6qdl.dtsi | 3 +++
 arch/arm/boot/dts/imx6sl.dtsi  | 3 +++
 arch/arm/boot/dts/imx6sx.dtsi  | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 89b834f..9702e18 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -635,6 +635,7 @@
 					anatop-min-bit-val = <4>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1375000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-3p0 {
@@ -649,6 +650,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2625000>;
 					anatop-max-voltage = <3400000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-2p5 {
@@ -663,6 +665,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2000000>;
 					anatop-max-voltage = <2750000>;
+					anatop-enable-bit = <0>;
 				};
 
 				reg_arm: regulator-vddcore {
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 19cbd87..a478a06f 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -522,6 +522,7 @@
 					anatop-min-bit-val = <4>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1375000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-3p0 {
@@ -536,6 +537,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2625000>;
 					anatop-max-voltage = <3400000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-2p5 {
@@ -550,6 +552,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2100000>;
 					anatop-max-voltage = <2850000>;
+					anatop-enable-bit = <0>;
 				};
 
 				reg_arm: regulator-vddcore {
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 10f3330..3de3cca 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -578,6 +578,7 @@
 					anatop-min-bit-val = <4>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1375000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-3p0 {
@@ -592,6 +593,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2625000>;
 					anatop-max-voltage = <3400000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-2p5 {
@@ -606,6 +608,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2100000>;
 					anatop-max-voltage = <2875000>;
+					anatop-enable-bit = <0>;
 				};
 
 				reg_arm: regulator-vddcore {
-- 
2.9.3

^ permalink raw reply related

* [PATCH 2/2] ARM: dts: imx7s: Adjust anatop-enable-bit for 'reg_1p0d'
From: Andrey Smirnov @ 2017-01-10 16:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170110163015.22444-1-andrew.smirnov@gmail.com>

In PMU_REG_1P0Dn ENABLE_LINREG is bit 0. Bit 31 is called OVERRIDE and
it serves the funtion of granting permission to GPC IP block to alter
various other bit-fields of the register. The reason why this property,
that trickeld here from Freescale BSP, is set up like that is because in
the code it came from it is used in conjunction with a notifier handler
for REGULATOR_EVENT_PRE_DO_ENABLE and REGULATOR_EVENT_PRE_DO_DISABLE
events (not found in upstream kernel) that triggers GPC to start
manipulating aforementioned other bitfields.

Since:
	a) none of the aforementioned machinery is implemented by
	   upstream
	b) using 'anatop-enable-bit' in that capacity is a bit of a
	   semantic stretch

simplify the situation by setting the value of 'anatop-enable-bit' to
point to ENABLE_LINREG (same as i.MX6).

Cc: yurovsky at gmail.com
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: devicetree at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/boot/dts/imx7s.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 8ff2cbdd..c80d0db 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -509,7 +509,7 @@
 					anatop-min-bit-val = <8>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1200000>;
-					anatop-enable-bit = <31>;
+					anatop-enable-bit = <0>;
 				};
 			};
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH 1/3] ARM: at91: flush the L2 cache before entering cpu idle
From: Alexandre Belloni @ 2017-01-10 16:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170110161821.vp673jyfqx6s76pg@piout.net>

On 10/01/2017 at 17:18:21 +0100, Alexandre Belloni wrote :
> I though a bit more about it, and I don't really like the new compatible
> string. I don't feel this should be necessary.
> 
> What about the following:
> 
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index b4332b727e9c..0333aca63e44 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -53,6 +53,7 @@ extern void at91_pinctrl_gpio_resume(void);
>  static struct {
>  	unsigned long uhp_udp_mask;
>  	int memctrl;
> +	bool has_l2_cache;
>  } at91_pm_data;
>  
>  void __iomem *at91_ramc_base[2];
> @@ -267,6 +268,11 @@ static void at91_ddr_standby(void)
>  	u32 lpr0, lpr1 = 0;
>  	u32 saved_lpr0, saved_lpr1 = 0;
>  
> +	if (at91_pm_data.has_l2_cache) {
> +		flush_cache_all();
> +		outer_disable();
> +	}
> +
>  	if (at91_ramc_base[1]) {
>  		saved_lpr1 = at91_ramc_read(1, AT91_DDRSDRC_LPR);
>  		lpr1 = saved_lpr1 & ~AT91_DDRSDRC_LPCB;
> @@ -287,6 +293,9 @@ static void at91_ddr_standby(void)
>  	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr0);
>  	if (at91_ramc_base[1])
>  		at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
> +
> +	if (at91_pm_data.has_l2_cache)
> +		outer_resume();
>  }
>  
>  /* We manage both DDRAM/SDRAM controllers, we need more than one value
>  * to
> @@ -353,6 +362,11 @@ static __init void at91_dt_ramc(void)
>  		return;
>  	}
>  
> +	np = of_find_compatible_node(NULL, NULL, "arm,pl310-cache");
> +	if (np)
> +		at91_pm_data.has_l2_cache = true;
> +	of_node_put(np);
> +
>  	at91_pm_set_standby(standby);
>  }
> 
> 
> This has the following benefits:
>  - everybody will have the fix, regardless of whether the dtb is updated
>  - has_l2_cache can be used later in at91_pm_suspend instead of calling
>    it unconditionnaly (I'll send a patch)
> 

I forgot to add that the added latency on at91sam9 and sama5d3 is exactly 5 instructions.

> 
> On 06/01/2017 at 14:59:45 +0800, Wenyou Yang wrote :
> > For the SoCs such as SAMA5D2 and SAMA5D4 which have L2 cache,
> > flush the L2 cache first before entering the cpu idle.
> > 
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> > ---
> > 
> >  arch/arm/mach-at91/pm.c       | 19 +++++++++++++++++++
> >  drivers/memory/atmel-sdramc.c |  1 +
> >  2 files changed, 20 insertions(+)
> > 
> > diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> > index b4332b727e9c..1a60dede1a01 100644
> > --- a/arch/arm/mach-at91/pm.c
> > +++ b/arch/arm/mach-at91/pm.c
> > @@ -289,6 +289,24 @@ static void at91_ddr_standby(void)
> >  		at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
> >  }
> >  
> > +static void at91_ddr_cache_standby(void)
> > +{
> > +	u32 saved_lpr;
> > +
> > +	flush_cache_all();
> > +	outer_disable();
> > +
> > +	saved_lpr = at91_ramc_read(0, AT91_DDRSDRC_LPR);
> > +	at91_ramc_write(0, AT91_DDRSDRC_LPR, (saved_lpr &
> > +			(~AT91_DDRSDRC_LPCB)) | AT91_DDRSDRC_LPCB_SELF_REFRESH);
> > +
> > +	cpu_do_idle();
> > +
> > +	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr);
> > +
> > +	outer_resume();
> > +}
> > +
> >  /* We manage both DDRAM/SDRAM controllers, we need more than one value to
> >   * remember.
> >   */
> > @@ -324,6 +342,7 @@ static const struct of_device_id const ramc_ids[] __initconst = {
> >  	{ .compatible = "atmel,at91sam9260-sdramc", .data = at91sam9_sdram_standby },
> >  	{ .compatible = "atmel,at91sam9g45-ddramc", .data = at91_ddr_standby },
> >  	{ .compatible = "atmel,sama5d3-ddramc", .data = at91_ddr_standby },
> > +	{ .compatible = "atmel,sama5d4-ddramc", .data = at91_ddr_cache_standby },
> >  	{ /*sentinel*/ }
> >  };
> >  
> > diff --git a/drivers/memory/atmel-sdramc.c b/drivers/memory/atmel-sdramc.c
> > index b418b39af180..7e5c5c6c1348 100644
> > --- a/drivers/memory/atmel-sdramc.c
> > +++ b/drivers/memory/atmel-sdramc.c
> > @@ -48,6 +48,7 @@ static const struct of_device_id atmel_ramc_of_match[] = {
> >  	{ .compatible = "atmel,at91sam9260-sdramc", .data = &at91rm9200_caps, },
> >  	{ .compatible = "atmel,at91sam9g45-ddramc", .data = &at91sam9g45_caps, },
> >  	{ .compatible = "atmel,sama5d3-ddramc", .data = &sama5d3_caps, },
> > +	{ .compatible = "atmel,sama5d4-ddramc", .data = &sama5d3_caps, },
> >  	{},
> >  };
> >  
> > -- 
> > 2.11.0
> > 
> 
> -- 
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH 1/3] ARM: at91: flush the L2 cache before entering cpu idle
From: Jean-Jacques Hiblot @ 2017-01-10 16:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170110161821.vp673jyfqx6s76pg@piout.net>

2017-01-10 17:18 GMT+01:00 Alexandre Belloni
<alexandre.belloni@free-electrons.com>:
> I though a bit more about it, and I don't really like the new compatible
> string. I don't feel this should be necessary.
>
> What about the following:
>
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index b4332b727e9c..0333aca63e44 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -53,6 +53,7 @@ extern void at91_pinctrl_gpio_resume(void);
>  static struct {
>         unsigned long uhp_udp_mask;
>         int memctrl;
> +       bool has_l2_cache;
>  } at91_pm_data;
>
>  void __iomem *at91_ramc_base[2];
> @@ -267,6 +268,11 @@ static void at91_ddr_standby(void)
>         u32 lpr0, lpr1 = 0;
>         u32 saved_lpr0, saved_lpr1 = 0;
>

> +       if (at91_pm_data.has_l2_cache) {
> +               flush_cache_all();
what is the point of calling flush_cache_all() here ? Do we really
care that dirty data in L1 is written to DDR ? I may be missing
something but to me it's just extra latency.
> +               outer_disable();
It seems to me that if there's no L2 cache, then outer_disable()  is a
no-op. It could be called unconditionally.
> +       }
> +
>         if (at91_ramc_base[1]) {
>                 saved_lpr1 = at91_ramc_read(1, AT91_DDRSDRC_LPR);
>                 lpr1 = saved_lpr1 & ~AT91_DDRSDRC_LPCB;
> @@ -287,6 +293,9 @@ static void at91_ddr_standby(void)
>         at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr0);
>         if (at91_ramc_base[1])
>                 at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
> +
> +       if (at91_pm_data.has_l2_cache)
> +               outer_resume();

same remark as for outer_disable()

Jean-Jacques

>  }
>
>  /* We manage both DDRAM/SDRAM controllers, we need more than one value
>  * to
> @@ -353,6 +362,11 @@ static __init void at91_dt_ramc(void)
>                 return;
>         }
>
> +       np = of_find_compatible_node(NULL, NULL, "arm,pl310-cache");
> +       if (np)
> +               at91_pm_data.has_l2_cache = true;
> +       of_node_put(np);
> +
>         at91_pm_set_standby(standby);
>  }
>
>
> This has the following benefits:
>  - everybody will have the fix, regardless of whether the dtb is updated
>  - has_l2_cache can be used later in at91_pm_suspend instead of calling
>    it unconditionnaly (I'll send a patch)
>
>
> On 06/01/2017 at 14:59:45 +0800, Wenyou Yang wrote :
>> For the SoCs such as SAMA5D2 and SAMA5D4 which have L2 cache,
>> flush the L2 cache first before entering the cpu idle.
>>
>> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
>> ---
>>
>>  arch/arm/mach-at91/pm.c       | 19 +++++++++++++++++++
>>  drivers/memory/atmel-sdramc.c |  1 +
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
>> index b4332b727e9c..1a60dede1a01 100644
>> --- a/arch/arm/mach-at91/pm.c
>> +++ b/arch/arm/mach-at91/pm.c
>> @@ -289,6 +289,24 @@ static void at91_ddr_standby(void)
>>               at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
>>  }
>>
>> +static void at91_ddr_cache_standby(void)
>> +{
>> +     u32 saved_lpr;
>> +
>> +     flush_cache_all();
>> +     outer_disable();
>> +
>> +     saved_lpr = at91_ramc_read(0, AT91_DDRSDRC_LPR);
>> +     at91_ramc_write(0, AT91_DDRSDRC_LPR, (saved_lpr &
>> +                     (~AT91_DDRSDRC_LPCB)) | AT91_DDRSDRC_LPCB_SELF_REFRESH);
>> +
>> +     cpu_do_idle();
>> +
>> +     at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr);
>> +
>> +     outer_resume();
>> +}
>> +
>>  /* We manage both DDRAM/SDRAM controllers, we need more than one value to
>>   * remember.
>>   */
>> @@ -324,6 +342,7 @@ static const struct of_device_id const ramc_ids[] __initconst = {
>>       { .compatible = "atmel,at91sam9260-sdramc", .data = at91sam9_sdram_standby },
>>       { .compatible = "atmel,at91sam9g45-ddramc", .data = at91_ddr_standby },
>>       { .compatible = "atmel,sama5d3-ddramc", .data = at91_ddr_standby },
>> +     { .compatible = "atmel,sama5d4-ddramc", .data = at91_ddr_cache_standby },
>>       { /*sentinel*/ }
>>  };
>>
>> diff --git a/drivers/memory/atmel-sdramc.c b/drivers/memory/atmel-sdramc.c
>> index b418b39af180..7e5c5c6c1348 100644
>> --- a/drivers/memory/atmel-sdramc.c
>> +++ b/drivers/memory/atmel-sdramc.c
>> @@ -48,6 +48,7 @@ static const struct of_device_id atmel_ramc_of_match[] = {
>>       { .compatible = "atmel,at91sam9260-sdramc", .data = &at91rm9200_caps, },
>>       { .compatible = "atmel,at91sam9g45-ddramc", .data = &at91sam9g45_caps, },
>>       { .compatible = "atmel,sama5d3-ddramc", .data = &sama5d3_caps, },
>> +     { .compatible = "atmel,sama5d4-ddramc", .data = &sama5d3_caps, },
>>       {},
>>  };
>>
>> --
>> 2.11.0
>>
>
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [RESEND] spi: davinci: Allow device tree devices to use DMA
From: Mark Brown @ 2017-01-10 16:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <dc47ecd2-fb94-ef7c-af15-590b2233e79f@lechnology.com>

On Mon, Jan 09, 2017 at 02:40:50PM -0600, David Lechner wrote:
> On 01/09/2017 01:48 PM, Mark Brown wrote:
> > On Thu, Jan 05, 2017 at 09:26:17PM -0600, David Lechner wrote:

> > > Unfortunately, this excludes the possibility of using one SPI device with
> > > DMA and one without on the same master.

> > Why would you ever want to do that?  What would ever make sense about
> > not using DMA if it's available and the transfer is suitably large, or
> > conversely why would one want to force DMA even if PIO would be more
> > performant?

> I don't particularly want to do that, but that is the way the spi-davinci
> driver currently works. The choice between DMA or PIO is specified in the
> platform data on a per-device basis.

> What I get from your remarks is that this is wrong and it needs to be fixed.
> If that is so, could someone please point out a driver that does it the
> right way and I will try to fix it.

Pretty much any driver doing DMA...  off the top of my head the i.MX and
Samsung drivers ought to be reasonable examples.  If you've got time to
look at fixing the driver the other thing that jumps out with it is the
use of a threaded interrupt handler with no actual handling in the
thread, that looks like some magic timing based hack which seems risky
(or completely unneeded which would be better).

> > > When I originally submitted this patch, there was some discussion as to whether
> > > dspi->dma_rx should be changed to return an error rather than being null.
> > 
> > > However, I prefer it the way it is and don't see a compelling reason to change
> > > it.

> > I don't know what the above comment means, sorry (and don't recall
> > having seen any earlier versions of this).

> FWIW, you can find the previous conversation at
> https://patchwork.kernel.org/patch/9437901/

What Sekhar is saying there is right, it is better style to use an error
pointer rather than NULL as someone could potentially come up with a way
to make NULL a valid pointer in the API.  But it doesn't matter greatly.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170110/aac7bf99/attachment.sig>

^ permalink raw reply

* [RFC 4/8] KVM: arm/arm64: Initialize the emulated EL1 physical timer
From: Jintack Lim @ 2017-01-10 17:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170109120235.GD4348@cbox>

On Mon, Jan 9, 2017 at 7:02 AM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Mon, Dec 26, 2016 at 12:12:02PM -0500, Jintack Lim wrote:
>> Initialize the emulated EL1 physical timer with the default irq number.
>>
>> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
>> ---
>>  arch/arm/kvm/reset.c         |  9 ++++++++-
>>  arch/arm64/kvm/reset.c       |  9 ++++++++-
>>  include/kvm/arm_arch_timer.h |  3 ++-
>>  virt/kvm/arm/arch_timer.c    | 12 ++++++++++--
>>  4 files changed, 28 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/kvm/reset.c b/arch/arm/kvm/reset.c
>> index 4b5e802..1da8b2d 100644
>> --- a/arch/arm/kvm/reset.c
>> +++ b/arch/arm/kvm/reset.c
>> @@ -37,6 +37,11 @@
>>       .usr_regs.ARM_cpsr = SVC_MODE | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT,
>>  };
>>
>> +static const struct kvm_irq_level cortexa_ptimer_irq = {
>> +     { .irq = 30 },
>> +     .level = 1,
>> +};
>> +
>>  static const struct kvm_irq_level cortexa_vtimer_irq = {
>>       { .irq = 27 },
>>       .level = 1,
>> @@ -58,6 +63,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>>  {
>>       struct kvm_regs *reset_regs;
>>       const struct kvm_irq_level *cpu_vtimer_irq;
>> +     const struct kvm_irq_level *cpu_ptimer_irq;
>>
>>       switch (vcpu->arch.target) {
>>       case KVM_ARM_TARGET_CORTEX_A7:
>> @@ -65,6 +71,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>>               reset_regs = &cortexa_regs_reset;
>>               vcpu->arch.midr = read_cpuid_id();
>>               cpu_vtimer_irq = &cortexa_vtimer_irq;
>> +             cpu_ptimer_irq = &cortexa_ptimer_irq;
>>               break;
>>       default:
>>               return -ENODEV;
>> @@ -77,5 +84,5 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>>       kvm_reset_coprocs(vcpu);
>>
>>       /* Reset arch_timer context */
>> -     return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
>> +     return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq, cpu_ptimer_irq);
>>  }
>> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
>> index 5bc4608..74322c2 100644
>> --- a/arch/arm64/kvm/reset.c
>> +++ b/arch/arm64/kvm/reset.c
>> @@ -46,6 +46,11 @@
>>                       COMPAT_PSR_I_BIT | COMPAT_PSR_F_BIT),
>>  };
>>
>> +static const struct kvm_irq_level default_ptimer_irq = {
>> +     .irq    = 30,
>> +     .level  = 1,
>> +};
>> +
>>  static const struct kvm_irq_level default_vtimer_irq = {
>>       .irq    = 27,
>>       .level  = 1,
>> @@ -110,6 +115,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
>>  int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>>  {
>>       const struct kvm_irq_level *cpu_vtimer_irq;
>> +     const struct kvm_irq_level *cpu_ptimer_irq;
>>       const struct kvm_regs *cpu_reset;
>>
>>       switch (vcpu->arch.target) {
>> @@ -123,6 +129,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>>               }
>>
>>               cpu_vtimer_irq = &default_vtimer_irq;
>> +             cpu_ptimer_irq = &default_ptimer_irq;
>>               break;
>>       }
>>
>> @@ -136,5 +143,5 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>>       kvm_pmu_vcpu_reset(vcpu);
>>
>>       /* Reset timer */
>> -     return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
>> +     return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq, cpu_ptimer_irq);
>>  }
>> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
>> index d21652a..04ed9c1 100644
>> --- a/include/kvm/arm_arch_timer.h
>> +++ b/include/kvm/arm_arch_timer.h
>> @@ -61,7 +61,8 @@ struct arch_timer_cpu {
>>  int kvm_timer_enable(struct kvm_vcpu *vcpu);
>>  void kvm_timer_init(struct kvm *kvm);
>>  int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
>> -                      const struct kvm_irq_level *irq);
>> +                      const struct kvm_irq_level *virt_irq,
>> +                      const struct kvm_irq_level *phys_irq);
>>  void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu);
>>  void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu);
>>  void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu);
>> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
>> index 3bd6063..ed80864 100644
>> --- a/virt/kvm/arm/arch_timer.c
>> +++ b/virt/kvm/arm/arch_timer.c
>> @@ -339,9 +339,11 @@ void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
>>  }
>>
>>  int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
>> -                      const struct kvm_irq_level *irq)
>> +                      const struct kvm_irq_level *virt_irq,
>> +                      const struct kvm_irq_level *phys_irq)
>>  {
>>       struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
>> +     struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>>
>>       /*
>>        * The vcpu timer irq number cannot be determined in
>> @@ -349,7 +351,8 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
>>        * kvm_vcpu_set_target(). To handle this, we determine
>>        * vcpu timer irq number when the vcpu is reset.
>>        */
>> -     vtimer->irq.irq = irq->irq;
>> +     vtimer->irq.irq = virt_irq->irq;
>> +     ptimer->irq.irq = phys_irq->irq;
>>
>>       /*
>>        * The bits in CNTV_CTL are architecturally reset to UNKNOWN for ARMv8
>> @@ -358,6 +361,7 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
>>        * the ARMv7 architecture.
>>        */
>>       vtimer->cnt_ctl = 0;
>> +     ptimer->cnt_ctl = 0;
>>       kvm_timer_update_state(vcpu);
>>
>>       return 0;
>> @@ -477,11 +481,15 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu)
>>  int kvm_timer_enable(struct kvm_vcpu *vcpu)
>>  {
>>       struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
>> +     struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>>       struct irq_desc *desc;
>>       struct irq_data *data;
>>       int phys_irq;
>>       int ret;
>>
>> +     /* Always enable emulated the EL1 physical timer */
>
> Dubious comment the way it stands.
>
> Does the rest of the code really support one timer being enabled while
> another one not so?

No. The code never check if the physical timer is enabled. I think
it's not necessary to set enable bit for this emulated physical timer.
We, however, may need to set/check this enable bit later if we decide
to give the EL1 physical timer to the guest instead of emulating it.

>
> In any case, the comment should explain the rationale as opposed to
> reiterate what the following code line does, or just not be there.
>
>> +     ptimer->enabled = 1;
>> +
>>       if (vtimer->enabled)
>>               return 0;
>>
>> --
>> 1.9.1
>>
>>
>
> Thanks,
> -Christoffer
>

^ permalink raw reply

* [PATCH] usb: dwc3-exynos fix unspecified suspend clk error handling
From: Bartlomiej Zolnierkiewicz @ 2017-01-10 17:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <28815306-e220-4847-99ce-b058baae0d07@osg.samsung.com>


Hi,

On Tuesday, January 10, 2017 09:28:52 AM Shuah Khan wrote:
> On 01/10/2017 09:05 AM, Bartlomiej Zolnierkiewicz wrote:
> > 
> > Hi,
> > 
> > On Tuesday, January 10, 2017 07:36:35 AM Shuah Khan wrote:
> >> On 01/10/2017 07:16 AM, Shuah Khan wrote:
> >>> On 01/10/2017 05:05 AM, Bartlomiej Zolnierkiewicz wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> On Monday, January 09, 2017 07:21:31 PM Shuah Khan wrote:
> >>>>> Fix dwc3_exynos_probe() to call clk_prepare_enable() only when suspend
> >>>>> clock is specified. Call clk_disable_unprepare() from remove and probe
> >>>>> error path only when susp_clk has been set from remove and probe error
> >>>>> paths.
> >>>>
> >>>> It is legal to call clk_prepare_enable() and clk_disable_unprepare()
> >>>> for NULL clock.  Also your patch changes susp_clk handling while
> >>>> leaves axius_clk handling (which also can be NULL) untouched.
> >>>>
> >>>> Do you actually see some runtime problem with the current code?
> >>>>
> >>>> If not then the patch should probably be dropped.
> >>>>
> >>>> Best regards,
> >>>> --
> >>>> Bartlomiej Zolnierkiewicz
> >>>> Samsung R&D Institute Poland
> >>>> Samsung Electronics
> >>>
> >>> Hi Bartlomiej,
> >>>
> >>> I am seeing the "no suspend clk specified" message in dmesg.
> >>> After that it sets the exynos->susp_clk = NULL and starts
> >>> calling clk_prepare_enable(exynos->susp_clk);
> >>>
> >>> That can't be good. If you see the logic right above this
> >>> one for exynos->clk, it returns error and fails the probe.
> >>> This this case it doesn't, but tries to use null susp_clk.
> > 
> > exynos->susp_clk is optional, exynos->clk is not.
> 
> Right. That is clear since we don't fail the probe.
> 
> > 
> >>> I believe this patch is necessary.
> >>
> >> Let me clarify this a bit further. Since we already know
> >> susp_clk is null, with this patch we can avoid extra calls
> >> to clk_prepare_enable() and clk_disable_unprepare().
> >>
> >> One can say, it also adds extra checks, hence I will let you
> >> decide one way or the other. :)
> > 
> > I would prefer to leave the things as they are currently.
> > 
> > The code in question is not performance sensitive so extra
> > calls are not a problem.  No extra checks means less code.
> > 
> > Also the current code seems to be more in line with the rest
> > of the kernel.
> 
> What functionality is missing without the suspend clock? Would
> it make sense to change the info. message to include what it
> means. At the moment it doesn't anything more than "no suspend
> clock" which is a very cryptic user visible message. It would be
> helpful for it to also include what functionality is impacted.

Well, all I know is what the original commit descriptions says and
that the commit itself comes from patchset adding Exynos7 USB 3.0
support [1]:

commit 72d996fc7a01c2e4d581a15db7d001e2799ffb29
Author: Vivek Gautam <gautam.vivek@samsung.com>
Date:   Fri Nov 21 19:05:46 2014 +0530

    usb: dwc3: exynos: Add provision for suspend clock
    
    DWC3 controller on Exynos SoC series have separate control for
    suspend clock which replaces pipe3_rx_pclk as clock source to
    a small part of DWC3 core that operates when SS PHY is in its
    lowest power state (P3) in states SS.disabled and U3.
    
    Suggested-by: Anton Tikhomirov <av.tikhomirov@samsung.com>
    Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
    Signed-off-by: Felipe Balbi <balbi@ti.com>

Anton's & Vivek's Samsung email addresses are no longer valid but
I added current Vivek's email address to Cc:.

BTW What is interesting is that the Exynos7 dts patch [2] has never
made it into upstream for some reason.  In the meantime however
Exynos5433 (similar to Exynos7 to some degree) became the user of
susp_clk.

[1] https://lkml.org/lkml/2014/11/21/247
[2] https://patchwork.kernel.org/patch/5355161/

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> thanks,
> -- Shuah
> 
> > 
> > Best regards,
> > --
> > Bartlomiej Zolnierkiewicz
> > Samsung R&D Institute Poland
> > Samsung Electronics
> > 
> >> thanks,
> >> -- Shuah
> >>
> >>>
> >>> thanks,
> >>> -- Shuah
> >>>
> >>>>
> >>>>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> >>>>> ---
> >>>>>  drivers/usb/dwc3/dwc3-exynos.c | 10 ++++++----
> >>>>>  1 file changed, 6 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> >>>>> index e27899b..f97a3d7 100644
> >>>>> --- a/drivers/usb/dwc3/dwc3-exynos.c
> >>>>> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> >>>>> @@ -131,8 +131,8 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>>>  	if (IS_ERR(exynos->susp_clk)) {
> >>>>>  		dev_info(dev, "no suspend clk specified\n");
> >>>>>  		exynos->susp_clk = NULL;
> >>>>> -	}
> >>>>> -	clk_prepare_enable(exynos->susp_clk);
> >>>>> +	} else
> >>>>> +		clk_prepare_enable(exynos->susp_clk);
> >>>>>  
> >>>>>  	if (of_device_is_compatible(node, "samsung,exynos7-dwusb3")) {
> >>>>>  		exynos->axius_clk = devm_clk_get(dev, "usbdrd30_axius_clk");
> >>>>> @@ -196,7 +196,8 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>>>  	regulator_disable(exynos->vdd33);
> >>>>>  err2:
> >>>>>  	clk_disable_unprepare(exynos->axius_clk);
> >>>>> -	clk_disable_unprepare(exynos->susp_clk);
> >>>>> +	if (exynos->susp_clk)
> >>>>> +		clk_disable_unprepare(exynos->susp_clk);
> >>>>>  	clk_disable_unprepare(exynos->clk);
> >>>>>  	return ret;
> >>>>>  }
> >>>>> @@ -210,7 +211,8 @@ static int dwc3_exynos_remove(struct platform_device *pdev)
> >>>>>  	platform_device_unregister(exynos->usb3_phy);
> >>>>>  
> >>>>>  	clk_disable_unprepare(exynos->axius_clk);
> >>>>> -	clk_disable_unprepare(exynos->susp_clk);
> >>>>> +	if (exynos->susp_clk)
> >>>>> +		clk_disable_unprepare(exynos->susp_clk);
> >>>>>  	clk_disable_unprepare(exynos->clk);
> >>>>>  
> >>>>>  	regulator_disable(exynos->vdd33);

^ permalink raw reply

* [PATCH v7 08/19] iommu: Implement reserved_regions iommu-group sysfs file
From: Joerg Roedel @ 2017-01-10 17:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <9ee5aa6b-6e63-c153-0727-729e0e9592c6@redhat.com>

On Tue, Jan 10, 2017 at 05:20:34PM +0100, Auger Eric wrote:
> The /sys/kernel/iommu_groups/n directory seems to be removed before this
> gets called and this may produce a WARNING when devices get removed from
> the group. I intend to remove the call since I have the feeling
> everything gets cleaned up properly.

A feeling is not enough, please check that in the code.


	Joerg

^ permalink raw reply

* [PATCH] arm64: avoid increasing DMA masks above what hardware supports
From: Robin Murphy @ 2017-01-10 17:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1484056844-9567-1-git-send-email-nikita.yoush@cogentembedded.com>

On 10/01/17 14:00, Nikita Yushchenko wrote:
> There are cases when device supports wide DMA addresses wider than
> device's connection supports.
> 
> In this case driver sets DMA mask based on knowledge of device
> capabilities. That must succeed to allow drivers to initialize.
> 
> However, swiotlb or iommu still need knowledge about actual device
> capabilities. To avoid breakage, actual mask must not be set wider than
> device connection allows.
> 
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> CC: Arnd Bergmann <arnd@arndb.de>
> CC: Robin Murphy <robin.murphy@arm.com>
> CC: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/Kconfig                   |  3 +++
>  arch/arm64/include/asm/device.h      |  1 +
>  arch/arm64/include/asm/dma-mapping.h |  3 +++
>  arch/arm64/mm/dma-mapping.c          | 43 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 50 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1117421..afb2c08 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -216,6 +216,9 @@ config NEED_DMA_MAP_STATE
>  config NEED_SG_DMA_LENGTH
>  	def_bool y
>  
> +config ARCH_HAS_DMA_SET_COHERENT_MASK
> +	def_bool y
> +
>  config SMP
>  	def_bool y
>  
> diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
> index 243ef25..a57e7bb 100644
> --- a/arch/arm64/include/asm/device.h
> +++ b/arch/arm64/include/asm/device.h
> @@ -22,6 +22,7 @@ struct dev_archdata {
>  	void *iommu;			/* private IOMMU data */
>  #endif
>  	bool dma_coherent;
> +	u64 parent_dma_mask;
>  };
>  
>  struct pdev_archdata {
> diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
> index ccea82c..eab36d2 100644
> --- a/arch/arm64/include/asm/dma-mapping.h
> +++ b/arch/arm64/include/asm/dma-mapping.h
> @@ -51,6 +51,9 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
>  			const struct iommu_ops *iommu, bool coherent);
>  #define arch_setup_dma_ops	arch_setup_dma_ops
>  
> +#define HAVE_ARCH_DMA_SET_MASK 1
> +extern int dma_set_mask(struct device *dev, u64 dma_mask);
> +
>  #ifdef CONFIG_IOMMU_DMA
>  void arch_teardown_dma_ops(struct device *dev);
>  #define arch_teardown_dma_ops	arch_teardown_dma_ops
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index e040827..7b1bb87 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -203,6 +203,37 @@ static void __dma_free(struct device *dev, size_t size,
>  	__dma_free_coherent(dev, size, swiotlb_addr, dma_handle, attrs);
>  }
>  
> +int dma_set_mask(struct device *dev, u64 dma_mask)
> +{
> +	const struct dma_map_ops *ops = get_dma_ops(dev);
> +
> +	if (mask > dev->archdata.parent_dma_mask)
> +		mask = dev->archdata.parent_dma_mask;
> +
> +	if (ops->set_dma_mask)
> +		return ops->set_dma_mask(dev, mask);
> +
> +	if (!dev->dma_mask || !dma_supported(dev, mask))
> +		return -EIO;
> +
> +	*dev->dma_mask = mask;
> +	return 0;
> +}
> +EXPORT_SYMBOL(dma_set_mask);
> +
> +int dma_set_coherent_mask(struct device *dev, u64 mask)
> +{
> +	if (mask > dev->archdata.parent_dma_mask)
> +		mask = dev->archdata.parent_dma_mask;
> +
> +	if (!dma_supported(dev, mask))
> +		return -EIO;
> +
> +	dev->coherent_dma_mask = mask;
> +	return 0;
> +}
> +EXPORT_SYMBOL(dma_set_coherent_mask);
> +
>  static dma_addr_t __swiotlb_map_page(struct device *dev, struct page *page,
>  				     unsigned long offset, size_t size,
>  				     enum dma_data_direction dir,
> @@ -958,6 +989,18 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
>  	if (!dev->archdata.dma_ops)
>  		dev->archdata.dma_ops = &swiotlb_dma_ops;
>  
> +	/*
> +	 * we don't yet support buses that have a non-zero mapping.
> +	 *  Let's hope we won't need it
> +	 */
> +	WARN_ON(dma_base != 0);

I believe we now accomodate the bus remap bits on BCM2837 as a DMA
offset, so unfortunately I think this is no longer true.

> +	/*
> +	 * Whatever the parent bus can set. A device must not set
> +	 * a DMA mask larger than this.
> +	 */
> +	dev->archdata.parent_dma_mask = size - 1;

This will effectively constrain *all* DMA masks to be 32-bit, since for
99% of devices we're going to see a size derived from the default mask
passed in here. I worry that that's liable to lead to performance and
stability regressions (now that the block layer can apparently generate
sufficient readahead to ovflow a typical SWIOTLB bounce buffer[1]).
Whilst DT users would be able to mitigate that by putting explicit
"dma-ranges" properties on every device node, it's less clear what we'd
do for ACPI.

I reckon the easiest way forward would be to pass in some flag to
arch_setup_dma_ops to indicate whether it's an explicitly-configured
range or not - then simply initialising parent_dma_mask to ~0 for the
default case *should* keep things working as before.

Robin.

[1]:https://www.mail-archive.com/virtualization at lists.linux-foundation.org/msg26532.html

> +
>  	dev->archdata.dma_coherent = coherent;
>  	__iommu_setup_dma_ops(dev, dma_base, size, iommu);
>  }
> 

^ permalink raw reply


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