Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [V4, 1/2] media: dt-bindings: media: i2c: Add bindings for OV02A10
From: Andy Shevchenko @ 2019-09-09  9:45 UTC (permalink / raw)
  To: dongchun.zhu
  Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, shengnan.wang,
	tfiga, louis.kuo, sj.huang, robh+dt, linux-mediatek, sakari.ailus,
	matthias.bgg, bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <20190907092728.23897-2-dongchun.zhu@mediatek.com>

On Sat, Sep 07, 2019 at 05:27:27PM +0800, dongchun.zhu@mediatek.com wrote:
> From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> 
> This patch adds device tree bindings documentation for OV02A10 camera sensor.
> 
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> ---
>  .../devicetree/bindings/media/i2c/ov02a10.txt      | 54 ++++++++++++++++++++++

>  MAINTAINERS                                        |  7 +++

This doesn't belong to dt-bindings.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v9 2/8] mm: Adjust shuffle code to allow for future coalescing
From: Kirill A. Shutemov @ 2019-09-09  9:47 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: yang.zhang.wz, pagupta, kvm, david, catalin.marinas, mhocko,
	linux-mm, alexander.h.duyck, will, aarcange, virtio-dev, mst,
	willy, wei.w.wang, ying.huang, riel, dan.j.williams, lcapitulino,
	linux-arm-kernel, osalvador, nitesh, konrad.wilk, dave.hansen,
	linux-kernel, pbonzini, akpm, fengguang.wu, kirill.shutemov
In-Reply-To: <20190907172520.10910.83100.stgit@localhost.localdomain>

On Sat, Sep 07, 2019 at 10:25:20AM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
> Move the head/tail adding logic out of the shuffle code and into the
> __free_one_page function since ultimately that is where it is really
> needed anyway. By doing this we should be able to reduce the overhead
> and can consolidate all of the list addition bits in one spot.
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> ---
>  include/linux/mmzone.h |   12 --------
>  mm/page_alloc.c        |   70 +++++++++++++++++++++++++++---------------------
>  mm/shuffle.c           |    9 +-----
>  mm/shuffle.h           |   12 ++++++++
>  4 files changed, 53 insertions(+), 50 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index bda20282746b..125f300981c6 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -116,18 +116,6 @@ static inline void add_to_free_area_tail(struct page *page, struct free_area *ar
>  	area->nr_free++;
>  }
>  
> -#ifdef CONFIG_SHUFFLE_PAGE_ALLOCATOR
> -/* Used to preserve page allocation order entropy */
> -void add_to_free_area_random(struct page *page, struct free_area *area,
> -		int migratetype);
> -#else
> -static inline void add_to_free_area_random(struct page *page,
> -		struct free_area *area, int migratetype)
> -{
> -	add_to_free_area(page, area, migratetype);
> -}
> -#endif
> -
>  /* Used for pages which are on another list */
>  static inline void move_to_free_area(struct page *page, struct free_area *area,
>  			     int migratetype)

Looks like add_to_free_area() and add_to_free_area_tail() can be moved to
mm/page_alloc.c as all users are there now. And the same for struct
free_area definition (but not declaration).

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index c5d62f1c2851..4e4356ba66c7 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -878,6 +878,36 @@ static inline struct capture_control *task_capc(struct zone *zone)
>  #endif /* CONFIG_COMPACTION */
>  
>  /*
> + * If this is not the largest possible page, check if the buddy
> + * of the next-highest order is free. If it is, it's possible
> + * that pages are being freed that will coalesce soon. In case,
> + * that is happening, add the free page to the tail of the list
> + * so it's less likely to be used soon and more likely to be merged
> + * as a higher order page
> + */
> +static inline bool
> +buddy_merge_likely(unsigned long pfn, unsigned long buddy_pfn,
> +		   struct page *page, unsigned int order)
> +{
> +	struct page *higher_page, *higher_buddy;
> +	unsigned long combined_pfn;
> +
> +	if (order >= MAX_ORDER - 2)
> +		return false;
> +
> +	if (!pfn_valid_within(buddy_pfn))
> +		return false;
> +
> +	combined_pfn = buddy_pfn & pfn;
> +	higher_page = page + (combined_pfn - pfn);
> +	buddy_pfn = __find_buddy_pfn(combined_pfn, order + 1);
> +	higher_buddy = higher_page + (buddy_pfn - combined_pfn);
> +
> +	return pfn_valid_within(buddy_pfn) &&
> +	       page_is_buddy(higher_page, higher_buddy, order + 1);
> +}

Okay, that's much easier to read.

> +
> +/*
>   * Freeing function for a buddy system allocator.
>   *
>   * The concept of a buddy system is to maintain direct-mapped table
> @@ -906,11 +936,12 @@ static inline void __free_one_page(struct page *page,
>  		struct zone *zone, unsigned int order,
>  		int migratetype)
>  {
> -	unsigned long combined_pfn;
> +	struct capture_control *capc = task_capc(zone);
>  	unsigned long uninitialized_var(buddy_pfn);
> -	struct page *buddy;
> +	unsigned long combined_pfn;
> +	struct free_area *area;
>  	unsigned int max_order;
> -	struct capture_control *capc = task_capc(zone);
> +	struct page *buddy;
>  
>  	max_order = min_t(unsigned int, MAX_ORDER, pageblock_order + 1);
>  
> @@ -979,35 +1010,12 @@ static inline void __free_one_page(struct page *page,
>  done_merging:
>  	set_page_order(page, order);
>  
> -	/*
> -	 * If this is not the largest possible page, check if the buddy
> -	 * of the next-highest order is free. If it is, it's possible
> -	 * that pages are being freed that will coalesce soon. In case,
> -	 * that is happening, add the free page to the tail of the list
> -	 * so it's less likely to be used soon and more likely to be merged
> -	 * as a higher order page
> -	 */
> -	if ((order < MAX_ORDER-2) && pfn_valid_within(buddy_pfn)
> -			&& !is_shuffle_order(order)) {
> -		struct page *higher_page, *higher_buddy;
> -		combined_pfn = buddy_pfn & pfn;
> -		higher_page = page + (combined_pfn - pfn);
> -		buddy_pfn = __find_buddy_pfn(combined_pfn, order + 1);
> -		higher_buddy = higher_page + (buddy_pfn - combined_pfn);
> -		if (pfn_valid_within(buddy_pfn) &&
> -		    page_is_buddy(higher_page, higher_buddy, order + 1)) {
> -			add_to_free_area_tail(page, &zone->free_area[order],
> -					      migratetype);
> -			return;
> -		}
> -	}
> -
> -	if (is_shuffle_order(order))
> -		add_to_free_area_random(page, &zone->free_area[order],
> -				migratetype);
> +	area = &zone->free_area[order];
> +	if (is_shuffle_order(order) ? shuffle_pick_tail() :
> +	    buddy_merge_likely(pfn, buddy_pfn, page, order))

Too loaded condition to my taste. Maybe

	bool to_tail;
	...
	if (is_shuffle_order(order))
		to_tail = shuffle_pick_tail();
	else if (buddy_merge_likely(pfn, buddy_pfn, page, order))
		to_tail = true;
	else
		to_tail = false;

	if (to_tail)
		add_to_free_area_tail(page, area, migratetype);
	else
		add_to_free_area(page, area, migratetype);

> +		add_to_free_area_tail(page, area, migratetype);
>  	else
> -		add_to_free_area(page, &zone->free_area[order], migratetype);
> -
> +		add_to_free_area(page, area, migratetype);
>  }
>  
>  /*
> diff --git a/mm/shuffle.c b/mm/shuffle.c
> index 9ba542ecf335..345cb4347455 100644
> --- a/mm/shuffle.c
> +++ b/mm/shuffle.c
> @@ -4,7 +4,6 @@
>  #include <linux/mm.h>
>  #include <linux/init.h>
>  #include <linux/mmzone.h>
> -#include <linux/random.h>
>  #include <linux/moduleparam.h>
>  #include "internal.h"
>  #include "shuffle.h"

Why do you move #include <linux/random.h> from .c to .h?
It's not obvious to me.

> @@ -190,8 +189,7 @@ struct batched_bit_entropy {
>  
>  static DEFINE_PER_CPU(struct batched_bit_entropy, batched_entropy_bool);
>  
> -void add_to_free_area_random(struct page *page, struct free_area *area,
> -		int migratetype)
> +bool __shuffle_pick_tail(void)
>  {
>  	struct batched_bit_entropy *batch;
>  	unsigned long entropy;
> @@ -213,8 +211,5 @@ void add_to_free_area_random(struct page *page, struct free_area *area,
>  	batch->position = position;
>  	entropy = batch->entropy_bool;
>  
> -	if (1ul & (entropy >> position))
> -		add_to_free_area(page, area, migratetype);
> -	else
> -		add_to_free_area_tail(page, area, migratetype);
> +	return 1ul & (entropy >> position);
>  }
> diff --git a/mm/shuffle.h b/mm/shuffle.h
> index 777a257a0d2f..0723eb97f22f 100644
> --- a/mm/shuffle.h
> +++ b/mm/shuffle.h
> @@ -3,6 +3,7 @@
>  #ifndef _MM_SHUFFLE_H
>  #define _MM_SHUFFLE_H
>  #include <linux/jump_label.h>
> +#include <linux/random.h>
>  
>  /*
>   * SHUFFLE_ENABLE is called from the command line enabling path, or by
> @@ -22,6 +23,7 @@ enum mm_shuffle_ctl {
>  DECLARE_STATIC_KEY_FALSE(page_alloc_shuffle_key);
>  extern void page_alloc_shuffle(enum mm_shuffle_ctl ctl);
>  extern void __shuffle_free_memory(pg_data_t *pgdat);
> +extern bool __shuffle_pick_tail(void);
>  static inline void shuffle_free_memory(pg_data_t *pgdat)
>  {
>  	if (!static_branch_unlikely(&page_alloc_shuffle_key))
> @@ -43,6 +45,11 @@ static inline bool is_shuffle_order(int order)
>  		return false;
>  	return order >= SHUFFLE_ORDER;
>  }
> +
> +static inline bool shuffle_pick_tail(void)
> +{
> +	return __shuffle_pick_tail();
> +}

I don't see a reason in __shuffle_pick_tail() existing if you call it
unconditionally.

>  #else
>  static inline void shuffle_free_memory(pg_data_t *pgdat)
>  {
> @@ -60,5 +67,10 @@ static inline bool is_shuffle_order(int order)
>  {
>  	return false;
>  }
> +
> +static inline bool shuffle_pick_tail(void)
> +{
> +	return false;
> +}
>  #endif
>  #endif /* _MM_SHUFFLE_H */
> 
> 

-- 
 Kirill A. Shutemov

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v9 3/8] mm: Move set/get_pcppage_migratetype to mmzone.h
From: Kirill A. Shutemov @ 2019-09-09  9:56 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: yang.zhang.wz, pagupta, kvm, david, catalin.marinas, mhocko,
	linux-mm, alexander.h.duyck, will, aarcange, virtio-dev, mst,
	willy, wei.w.wang, ying.huang, riel, dan.j.williams, lcapitulino,
	linux-arm-kernel, osalvador, nitesh, konrad.wilk, dave.hansen,
	linux-kernel, pbonzini, akpm, fengguang.wu, kirill.shutemov
In-Reply-To: <20190907172528.10910.37051.stgit@localhost.localdomain>

On Sat, Sep 07, 2019 at 10:25:28AM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
> In order to support page reporting it will be necessary to store and
> retrieve the migratetype of a page. To enable that I am moving the set and
> get operations for pcppage_migratetype into the mm/internal.h header so
> that they can be used outside of the page_alloc.c file.
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>

I'm not sure that it's great idea to export this functionality beyond
mm/page_alloc.c without any additional safeguards. How would we avoid to
messing with ->index when the page is not in the right state of its
life-cycle. Can we add some VM_BUG_ON()s here?

-- 
 Kirill A. Shutemov

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [V4, 1/2] media: dt-bindings: media: i2c: Add bindings for OV02A10
From: Sakari Ailus @ 2019-09-09  9:57 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, shengnan.wang,
	tfiga, louis.kuo, sj.huang, robh+dt, linux-mediatek, dongchun.zhu,
	matthias.bgg, bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <20190909094501.GO2680@smile.fi.intel.com>

Hi Andy,

On Mon, Sep 09, 2019 at 12:45:01PM +0300, Andy Shevchenko wrote:
> On Sat, Sep 07, 2019 at 05:27:27PM +0800, dongchun.zhu@mediatek.com wrote:
> > From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > 
> > This patch adds device tree bindings documentation for OV02A10 camera sensor.
> > 
> > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > ---
> >  .../devicetree/bindings/media/i2c/ov02a10.txt      | 54 ++++++++++++++++++++++
> 
> >  MAINTAINERS                                        |  7 +++
> 
> This doesn't belong to dt-bindings.

The practice has been to add the MAINTAINERS entry along with the DT
bindings.

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v5 1/4] arm64: mm: use arm64_dma_phys_limit instead of calling max_zone_dma_phys()
From: Nicolas Saenz Julienne @ 2019-09-09  9:58 UTC (permalink / raw)
  To: catalin.marinas, hch, wahrenst, marc.zyngier, robh+dt,
	linux-arm-kernel, linux-mm, linux-riscv, linux-kernel
  Cc: f.fainelli, will, mbrugger, linux-rpi-kernel, phill, robin.murphy,
	nsaenzjulienne, m.szyprowski
In-Reply-To: <20190909095807.18709-1-nsaenzjulienne@suse.de>

By the time we call zones_sizes_init() arm64_dma_phys_limit already
contains the result of max_zone_dma_phys(). We use the variable instead
of calling the function directly to save some precious cpu time.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/mm/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index f3c795278def..6112d6c90fa8 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -181,7 +181,7 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
 	unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 
 #ifdef CONFIG_ZONE_DMA32
-	max_zone_pfns[ZONE_DMA32] = PFN_DOWN(max_zone_dma_phys());
+	max_zone_pfns[ZONE_DMA32] = PFN_DOWN(arm64_dma_phys_limit);
 #endif
 	max_zone_pfns[ZONE_NORMAL] = max;
 
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v5 0/4] Raspberry Pi 4 DMA addressing support
From: Nicolas Saenz Julienne @ 2019-09-09  9:58 UTC (permalink / raw)
  To: catalin.marinas, hch, wahrenst, marc.zyngier, robh+dt,
	linux-arm-kernel, linux-mm, linux-riscv
  Cc: f.fainelli, will, linux-kernel, mbrugger, linux-rpi-kernel, phill,
	robin.murphy, nsaenzjulienne, m.szyprowski

Hi all,
this series attempts to address some issues we found while bringing up
the new Raspberry Pi 4 in arm64 and it's intended to serve as a follow
up of these discussions:
v4: https://lkml.org/lkml/2019/9/6/352
v3: https://lkml.org/lkml/2019/9/2/589
v2: https://lkml.org/lkml/2019/8/20/767
v1: https://lkml.org/lkml/2019/7/31/922
RFC: https://lkml.org/lkml/2019/7/17/476

The new Raspberry Pi 4 has up to 4GB of memory but most peripherals can
only address the first GB: their DMA address range is
0xc0000000-0xfc000000 which is aliased to the first GB of physical
memory 0x00000000-0x3c000000. Note that only some peripherals have these
limitations: the PCIe, V3D, GENET, and 40-bit DMA channels have a wider
view of the address space by virtue of being hooked up trough a second
interconnect.

Part of this is solved on arm32 by setting up the machine specific
'.dma_zone_size = SZ_1G', which takes care of reserving the coherent
memory area at the right spot. That said no buffer bouncing (needed for
dma streaming) is available at the moment, but that's a story for
another series.

Unfortunately there is no such thing as 'dma_zone_size' in arm64. Only
ZONE_DMA32 is created which is interpreted by dma-direct and the arm64
arch code as if all peripherals where be able to address the first 4GB
of memory.

In the light of this, the series implements the following changes:

- Create both DMA zones in arm64, ZONE_DMA will contain the first 1G
  area and ZONE_DMA32 the rest of the 32 bit addressable memory. So far
  the RPi4 is the only arm64 device with such DMA addressing limitations
  so this hardcoded solution was deemed preferable.

- Properly set ARCH_ZONE_DMA_BITS.

- Reserve the CMA area in a place suitable for all peripherals.

This series has been tested on multiple devices both by checking the
zones setup matches the expectations and by double-checking physical
addresses on pages allocated on the three relevant areas GFP_DMA,
GFP_DMA32, GFP_KERNEL:

- On an RPi4 with variations on the ram memory size. But also forcing
  the situation where all three memory zones are nonempty by setting a 3G
  ZONE_DMA32 ceiling on a 4G setup. Both with and without NUMA support.

- On a Synquacer box[1] with 32G of memory.

- On an ACPI based Huawei TaiShan server[2] with 256G of memory.

- On a QEMU virtual machine running arm64's OpenSUSE Tumbleweed.

That's all.

Regards,
Nicolas

[1] https://www.96boards.org/product/developerbox/
[2] https://e.huawei.com/en/products/cloud-computing-dc/servers/taishan-server/taishan-2280-v2

---

Changes in v5:
- Fix issue with swiotlb initialization

Changes in v4:
- Rebased to linux-next
- Fix issue when NUMA=n and ZONE_DMA=n
- Merge two max_zone_dma*_phys() functions

Changes in v3:
- Fixed ZONE_DMA's size to 1G
- Update mmzone.h's comment to match changes in arm64
- Remove all dma-direct patches

Changes in v2:
- Update comment to reflect new zones split
- ZONE_DMA will never be left empty
- Try another approach merging both ZONE_DMA comments into one
- Address Christoph's comments
- If this approach doesn't get much traction I'll just drop the patch
  from the series as it's not really essential

Nicolas Saenz Julienne (4):
  arm64: mm: use arm64_dma_phys_limit instead of calling
    max_zone_dma_phys()
  arm64: rename variables used to calculate ZONE_DMA32's size
  arm64: use both ZONE_DMA and ZONE_DMA32
  mm: refresh ZONE_DMA and ZONE_DMA32 comments in 'enum zone_type'

 arch/arm64/Kconfig            |  4 ++
 arch/arm64/include/asm/page.h |  2 +
 arch/arm64/mm/init.c          | 71 +++++++++++++++++++++++++----------
 include/linux/mmzone.h        | 45 ++++++++++++----------
 4 files changed, 83 insertions(+), 39 deletions(-)

-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v5 2/4] arm64: rename variables used to calculate ZONE_DMA32's size
From: Nicolas Saenz Julienne @ 2019-09-09  9:58 UTC (permalink / raw)
  To: catalin.marinas, hch, wahrenst, marc.zyngier, robh+dt,
	linux-arm-kernel, linux-mm, linux-riscv, linux-kernel
  Cc: f.fainelli, will, mbrugger, linux-rpi-kernel, phill, robin.murphy,
	nsaenzjulienne, m.szyprowski
In-Reply-To: <20190909095807.18709-1-nsaenzjulienne@suse.de>

Let the name indicate that they are used to calculate ZONE_DMA32's size
as opposed to ZONE_DMA.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/mm/init.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 098c0f5bedf6..8e9bc64c5878 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -56,7 +56,7 @@ EXPORT_SYMBOL(physvirt_offset);
 struct page *vmemmap __ro_after_init;
 EXPORT_SYMBOL(vmemmap);
 
-phys_addr_t arm64_dma_phys_limit __ro_after_init;
+phys_addr_t arm64_dma32_phys_limit __ro_after_init;
 
 #ifdef CONFIG_KEXEC_CORE
 /*
@@ -174,7 +174,7 @@ static void __init reserve_elfcorehdr(void)
  * currently assumes that for memory starting above 4G, 32-bit devices will
  * use a DMA offset.
  */
-static phys_addr_t __init max_zone_dma_phys(void)
+static phys_addr_t __init max_zone_dma32_phys(void)
 {
 	phys_addr_t offset = memblock_start_of_DRAM() & GENMASK_ULL(63, 32);
 	return min(offset + (1ULL << 32), memblock_end_of_DRAM());
@@ -187,7 +187,7 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
 	unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 
 #ifdef CONFIG_ZONE_DMA32
-	max_zone_pfns[ZONE_DMA32] = PFN_DOWN(arm64_dma_phys_limit);
+	max_zone_pfns[ZONE_DMA32] = PFN_DOWN(arm64_dma32_phys_limit);
 #endif
 	max_zone_pfns[ZONE_NORMAL] = max;
 
@@ -200,16 +200,16 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
 {
 	struct memblock_region *reg;
 	unsigned long zone_size[MAX_NR_ZONES], zhole_size[MAX_NR_ZONES];
-	unsigned long max_dma = min;
+	unsigned long max_dma32 = min;
 
 	memset(zone_size, 0, sizeof(zone_size));
 
 	/* 4GB maximum for 32-bit only capable devices */
 #ifdef CONFIG_ZONE_DMA32
-	max_dma = PFN_DOWN(arm64_dma_phys_limit);
-	zone_size[ZONE_DMA32] = max_dma - min;
+	max_dma32 = PFN_DOWN(arm64_dma32_phys_limit);
+	zone_size[ZONE_DMA32] = max_dma32 - min;
 #endif
-	zone_size[ZONE_NORMAL] = max - max_dma;
+	zone_size[ZONE_NORMAL] = max - max_dma32;
 
 	memcpy(zhole_size, zone_size, sizeof(zhole_size));
 
@@ -221,14 +221,14 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
 			continue;
 
 #ifdef CONFIG_ZONE_DMA32
-		if (start < max_dma) {
-			unsigned long dma_end = min(end, max_dma);
+		if (start < max_dma32) {
+			unsigned long dma_end = min(end, max_dma32);
 			zhole_size[ZONE_DMA32] -= dma_end - start;
 		}
 #endif
-		if (end > max_dma) {
+		if (end > max_dma32) {
 			unsigned long normal_end = min(end, max);
-			unsigned long normal_start = max(start, max_dma);
+			unsigned long normal_start = max(start, max_dma32);
 			zhole_size[ZONE_NORMAL] -= normal_end - normal_start;
 		}
 	}
@@ -420,9 +420,9 @@ void __init arm64_memblock_init(void)
 
 	/* 4GB maximum for 32-bit only capable devices */
 	if (IS_ENABLED(CONFIG_ZONE_DMA32))
-		arm64_dma_phys_limit = max_zone_dma_phys();
+		arm64_dma32_phys_limit = max_zone_dma32_phys();
 	else
-		arm64_dma_phys_limit = PHYS_MASK + 1;
+		arm64_dma32_phys_limit = PHYS_MASK + 1;
 
 	reserve_crashkernel();
 
@@ -430,7 +430,7 @@ void __init arm64_memblock_init(void)
 
 	high_memory = __va(memblock_end_of_DRAM() - 1) + 1;
 
-	dma_contiguous_reserve(arm64_dma_phys_limit);
+	dma_contiguous_reserve(arm64_dma32_phys_limit);
 }
 
 void __init bootmem_init(void)
@@ -534,7 +534,7 @@ static void __init free_unused_memmap(void)
 void __init mem_init(void)
 {
 	if (swiotlb_force == SWIOTLB_FORCE ||
-	    max_pfn > (arm64_dma_phys_limit >> PAGE_SHIFT))
+	    max_pfn > (arm64_dma32_phys_limit >> PAGE_SHIFT))
 		swiotlb_init(1);
 	else
 		swiotlb_force = SWIOTLB_NO_FORCE;
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v5 3/4] arm64: use both ZONE_DMA and ZONE_DMA32
From: Nicolas Saenz Julienne @ 2019-09-09  9:58 UTC (permalink / raw)
  To: catalin.marinas, hch, wahrenst, marc.zyngier, robh+dt,
	linux-arm-kernel, linux-mm, linux-riscv, Will Deacon
  Cc: f.fainelli, linux-kernel, mbrugger, linux-rpi-kernel, phill,
	robin.murphy, nsaenzjulienne, m.szyprowski
In-Reply-To: <20190909095807.18709-1-nsaenzjulienne@suse.de>

So far all arm64 devices have supported 32 bit DMA masks for their
peripherals. This is not true anymore for the Raspberry Pi 4 as most of
it's peripherals can only address the first GB of memory on a total of
up to 4 GB.

This goes against ZONE_DMA32's intent, as it's expected for ZONE_DMA32
to be addressable with a 32 bit mask. So it was decided to re-introduce
ZONE_DMA in arm64.

ZONE_DMA will contain the lower 1G of memory, which is currently the
memory area addressable by any peripheral on an arm64 device.
ZONE_DMA32 will contain the rest of the 32 bit addressable memory.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

---

Changes in v5:
- Fixed swiotlb initialization

Changes in v4:
- Fixed issue when NUMA=n and ZONE_DMA=n
- Merged two max_zone_dma*_phys() functions

Changes in v3:
- Used fixed size ZONE_DMA
- Fix check befor swiotlb_init()

Changes in v2:
- Update comment to reflect new zones split
- ZONE_DMA will never be left empty

 arch/arm64/Kconfig            |  4 +++
 arch/arm64/include/asm/page.h |  2 ++
 arch/arm64/mm/init.c          | 52 +++++++++++++++++++++++++----------
 3 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 6b6362b83004..2dbe0165bd15 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -267,6 +267,10 @@ config GENERIC_CSUM
 config GENERIC_CALIBRATE_DELAY
 	def_bool y
 
+config ZONE_DMA
+	bool "Support DMA zone" if EXPERT
+	default y
+
 config ZONE_DMA32
 	bool "Support DMA32 zone" if EXPERT
 	default y
diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
index d39ddb258a04..7b8c98830101 100644
--- a/arch/arm64/include/asm/page.h
+++ b/arch/arm64/include/asm/page.h
@@ -38,4 +38,6 @@ extern int pfn_valid(unsigned long);
 
 #include <asm-generic/getorder.h>
 
+#define ARCH_ZONE_DMA_BITS 30
+
 #endif
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 8e9bc64c5878..92c911fc2ff9 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -56,6 +56,13 @@ EXPORT_SYMBOL(physvirt_offset);
 struct page *vmemmap __ro_after_init;
 EXPORT_SYMBOL(vmemmap);
 
+/*
+ * We create both ZONE_DMA and ZONE_DMA32. ZONE_DMA covers the first 1G of
+ * memory as some devices, namely the Raspberry Pi 4, have peripherals with
+ * this limited view of the memory. ZONE_DMA32 will cover the rest of the 32
+ * bit addressable memory area.
+ */
+phys_addr_t arm64_dma_phys_limit __ro_after_init;
 phys_addr_t arm64_dma32_phys_limit __ro_after_init;
 
 #ifdef CONFIG_KEXEC_CORE
@@ -169,15 +176,16 @@ static void __init reserve_elfcorehdr(void)
 {
 }
 #endif /* CONFIG_CRASH_DUMP */
+
 /*
- * Return the maximum physical address for ZONE_DMA32 (DMA_BIT_MASK(32)). It
- * currently assumes that for memory starting above 4G, 32-bit devices will
- * use a DMA offset.
+ * Return the maximum physical address for a zone with a given address size
+ * limit. It currently assumes that for memory starting above 4G, 32-bit
+ * devices will use a DMA offset.
  */
-static phys_addr_t __init max_zone_dma32_phys(void)
+static phys_addr_t __init max_zone_phys(unsigned int zone_bits)
 {
 	phys_addr_t offset = memblock_start_of_DRAM() & GENMASK_ULL(63, 32);
-	return min(offset + (1ULL << 32), memblock_end_of_DRAM());
+	return min(offset + (1ULL << zone_bits), memblock_end_of_DRAM());
 }
 
 #ifdef CONFIG_NUMA
@@ -186,6 +194,9 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
 {
 	unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 
+#ifdef CONFIG_ZONE_DMA
+	max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
+#endif
 #ifdef CONFIG_ZONE_DMA32
 	max_zone_pfns[ZONE_DMA32] = PFN_DOWN(arm64_dma32_phys_limit);
 #endif
@@ -201,13 +212,18 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
 	struct memblock_region *reg;
 	unsigned long zone_size[MAX_NR_ZONES], zhole_size[MAX_NR_ZONES];
 	unsigned long max_dma32 = min;
+	unsigned long max_dma = min;
 
 	memset(zone_size, 0, sizeof(zone_size));
 
-	/* 4GB maximum for 32-bit only capable devices */
+#ifdef CONFIG_ZONE_DMA
+	max_dma = PFN_DOWN(arm64_dma_phys_limit);
+	zone_size[ZONE_DMA] = max_dma - min;
+	max_dma32 = max_dma;
+#endif
 #ifdef CONFIG_ZONE_DMA32
 	max_dma32 = PFN_DOWN(arm64_dma32_phys_limit);
-	zone_size[ZONE_DMA32] = max_dma32 - min;
+	zone_size[ZONE_DMA32] = max_dma32 - max_dma;
 #endif
 	zone_size[ZONE_NORMAL] = max - max_dma32;
 
@@ -219,11 +235,17 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
 
 		if (start >= max)
 			continue;
-
+#ifdef CONFIG_ZONE_DMA
+		if (start < max_dma) {
+			unsigned long dma_end = min_not_zero(end, max_dma);
+			zhole_size[ZONE_DMA] -= dma_end - start;
+		}
+#endif
 #ifdef CONFIG_ZONE_DMA32
 		if (start < max_dma32) {
-			unsigned long dma_end = min(end, max_dma32);
-			zhole_size[ZONE_DMA32] -= dma_end - start;
+			unsigned long dma32_end = min(end, max_dma32);
+			unsigned long dma32_start = max(start, max_dma);
+			zhole_size[ZONE_DMA32] -= dma32_end - dma32_start;
 		}
 #endif
 		if (end > max_dma32) {
@@ -418,9 +440,11 @@ void __init arm64_memblock_init(void)
 
 	early_init_fdt_scan_reserved_mem();
 
-	/* 4GB maximum for 32-bit only capable devices */
+	if (IS_ENABLED(CONFIG_ZONE_DMA))
+		arm64_dma_phys_limit = max_zone_phys(ARCH_ZONE_DMA_BITS);
+
 	if (IS_ENABLED(CONFIG_ZONE_DMA32))
-		arm64_dma32_phys_limit = max_zone_dma32_phys();
+		arm64_dma32_phys_limit = max_zone_phys(32);
 	else
 		arm64_dma32_phys_limit = PHYS_MASK + 1;
 
@@ -430,7 +454,7 @@ void __init arm64_memblock_init(void)
 
 	high_memory = __va(memblock_end_of_DRAM() - 1) + 1;
 
-	dma_contiguous_reserve(arm64_dma32_phys_limit);
+	dma_contiguous_reserve(arm64_dma_phys_limit ? : arm64_dma32_phys_limit);
 }
 
 void __init bootmem_init(void)
@@ -534,7 +558,7 @@ static void __init free_unused_memmap(void)
 void __init mem_init(void)
 {
 	if (swiotlb_force == SWIOTLB_FORCE ||
-	    max_pfn > (arm64_dma32_phys_limit >> PAGE_SHIFT))
+	    max_pfn > PFN_DOWN(arm64_dma_phys_limit ? : arm64_dma32_phys_limit))
 		swiotlb_init(1);
 	else
 		swiotlb_force = SWIOTLB_NO_FORCE;
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v5 4/4] mm: refresh ZONE_DMA and ZONE_DMA32 comments in 'enum zone_type'
From: Nicolas Saenz Julienne @ 2019-09-09  9:58 UTC (permalink / raw)
  To: catalin.marinas, hch, wahrenst, marc.zyngier, robh+dt,
	linux-arm-kernel, linux-mm, linux-riscv, Paul Walmsley,
	Palmer Dabbelt, Albert Ou
  Cc: f.fainelli, will, linux-kernel, mbrugger, linux-rpi-kernel, phill,
	robin.murphy, nsaenzjulienne, m.szyprowski
In-Reply-To: <20190909095807.18709-1-nsaenzjulienne@suse.de>

These zones usage has evolved with time and the comments were outdated.
This joins both ZONE_DMA and ZONE_DMA32 explanation and gives up to date
examples on how they are used on different architectures.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

---

Changes in v3:
- Update comment to match changes in arm64

Changes in v2:
- Try another approach merging both ZONE_DMA comments into one
- Address Christoph's comments
- If this approach doesn't get much traction I'll just drop the patch
  from the series as it's not really essential

 include/linux/mmzone.h | 45 ++++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 3f38c30d2f13..bf1b916c9ecb 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -357,33 +357,40 @@ struct per_cpu_nodestat {
 #endif /* !__GENERATING_BOUNDS.H */
 
 enum zone_type {
-#ifdef CONFIG_ZONE_DMA
 	/*
-	 * ZONE_DMA is used when there are devices that are not able
-	 * to do DMA to all of addressable memory (ZONE_NORMAL). Then we
-	 * carve out the portion of memory that is needed for these devices.
-	 * The range is arch specific.
+	 * ZONE_DMA and ZONE_DMA32 are used when there are peripherals not able
+	 * to DMA to all of the addressable memory (ZONE_NORMAL).
+	 * On architectures where this area covers the whole 32 bit address
+	 * space ZONE_DMA32 is used. ZONE_DMA is left for the ones with smaller
+	 * DMA addressing constraints. This distinction is important as a 32bit
+	 * DMA mask is assumed when ZONE_DMA32 is defined. Some 64-bit
+	 * platforms may need both zones as they support peripherals with
+	 * different DMA addressing limitations.
+	 *
+	 * Some examples:
+	 *
+	 *  - i386 and x86_64 have a fixed 16M ZONE_DMA and ZONE_DMA32 for the
+	 *    rest of the lower 4G.
+	 *
+	 *  - arm only uses ZONE_DMA, the size, up to 4G, may vary depending on
+	 *    the specific device.
+	 *
+	 *  - arm64 has a fixed 1G ZONE_DMA and ZONE_DMA32 for the rest of the
+	 *    lower 4G.
 	 *
-	 * Some examples
+	 *  - powerpc only uses ZONE_DMA, the size, up to 2G, may vary
+	 *    depending on the specific device.
 	 *
-	 * Architecture		Limit
-	 * ---------------------------
-	 * parisc, ia64, sparc	<4G
-	 * s390, powerpc	<2G
-	 * arm			Various
-	 * alpha		Unlimited or 0-16MB.
+	 *  - s390 uses ZONE_DMA fixed to the lower 2G.
 	 *
-	 * i386, x86_64 and multiple other arches
-	 * 			<16M.
+	 *  - ia64 and riscv only use ZONE_DMA32.
+	 *
+	 *  - parisc uses neither.
 	 */
+#ifdef CONFIG_ZONE_DMA
 	ZONE_DMA,
 #endif
 #ifdef CONFIG_ZONE_DMA32
-	/*
-	 * x86_64 needs two ZONE_DMAs because it supports devices that are
-	 * only able to do DMA to the lower 16M but also 32 bit devices that
-	 * can only do DMA areas below 4G.
-	 */
 	ZONE_DMA32,
 #endif
 	/*
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [V1, 2/2] media: i2c: Add more sensor mode for ov8856 camera sensor
From: Sakari Ailus @ 2019-09-09 10:06 UTC (permalink / raw)
  To: Dongchun Zhu
  Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, shengnan.wang,
	linux-mediatek, ben.kao, tfiga, louis.kuo, sj.huang, robh+dt,
	Sakari Ailus, matthias.bgg, bingbu.cao, mchehab, linux-arm-kernel,
	linux-media
In-Reply-To: <1568018775.21623.221.camel@mhfsdcap03>

Hi Dongchun,

On Mon, Sep 09, 2019 at 04:46:15PM +0800, Dongchun Zhu wrote:
> Hi Sakari,
> 
> On Thu, 2019-08-08 at 16:53 +0300, Sakari Ailus wrote:
> > Hi Dongchun,
> > 
> > Thanks for the patch.
> > 
> > On Thu, Aug 08, 2019 at 05:22:15PM +0800, dongchun.zhu@mediatek.com wrote:
> > > From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > 
> > > This patch mainly adds two more sensor modes for OV8856 image sensor.
> > > The OV8856 driver currently supports output format: 10-bit Raw,
> > > the resolution of 1632*1224 and 3264*2448, and the bayer order of BGGR.
> > > The hardware version also differs in some OTP regiser,
> > > as well as PLL register setting.
> > > 
> > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > ---
> > >  drivers/media/i2c/ov8856.c | 624 ++++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 621 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
> > > index cd347d6..e0610b6 100644
> > > --- a/drivers/media/i2c/ov8856.c
> > > +++ b/drivers/media/i2c/ov8856.c
> > > @@ -1,12 +1,15 @@
> > >  // SPDX-License-Identifier: GPL-2.0
> > >  // Copyright (c) 2019 Intel Corporation.
> > >  
> > > +#include <linux/clk.h>
> > >  #include <asm/unaligned.h>
> > >  #include <linux/acpi.h>
> > >  #include <linux/delay.h>
> > > +#include <linux/gpio/consumer.h>
> > >  #include <linux/i2c.h>
> > >  #include <linux/module.h>
> > >  #include <linux/pm_runtime.h>
> > > +#include <linux/regulator/consumer.h>
> > >  #include <media/v4l2-ctrls.h>
> > >  #include <media/v4l2-device.h>
> > >  #include <media/v4l2-fwnode.h>
> > > @@ -19,6 +22,7 @@
> > >  #define OV8856_LINK_FREQ_180MHZ		180000000ULL
> > >  #define OV8856_SCLK			144000000ULL
> > >  #define OV8856_MCLK			19200000
> > > +#define OV8856_XVCLK_FREQ		24000000
> > 
> > The driver currenctly uses, perhaps misleadingly, OV8856_MCLK for this
> > purpose. You could rename the existing MCLK as XVCLK.
> > 
> 
> OV8856_MCLK would be replaced by OV8856_XVCLK in next release.
> 
> > This also means the driver needs to differentiate configurations for 24 and
> > 19,2 MHz which it currently does not do. I think it may make sense to make
> > this a separate patch from the rest.
> > 
> 
> From datasheet, OV8856 could support input clock XVCLK range 6-27MHz.
> But the typical frequency is 24MHz.
> That's the value we now provide.

You'll still need to check for it in the driver, to avoid breaking existing
systems that use another frequency.

...

> > > @@ -566,6 +1018,10 @@ struct ov8856 {
> > >  	struct media_pad pad;
> > >  	struct v4l2_ctrl_handler ctrl_handler;
> > >  
> > > +	struct clk		*xvclk;
> > > +	struct gpio_desc	*reset_gpio;
> > > +	struct regulator_bulk_data supplies[OV8856_NUM_SUPPLIES];
> > > +
> > >  	/* V4L2 Controls */
> > >  	struct v4l2_ctrl *link_freq;
> > >  	struct v4l2_ctrl *pixel_rate;
> > > @@ -576,6 +1032,9 @@ struct ov8856 {
> > >  	/* Current mode */
> > >  	const struct ov8856_mode *cur_mode;
> > >  
> > > +	/* module hardware version */
> > > +	bool is_1B_module;
> > 
> > What other hardware versions are there, and what are the differences?
> > 
> 
> According to OV, there are two MP hardware versions (i.e., 1B and 2A)
> for OV8856.
> There is one difference between 1B and 2A module - which is 0x3614
> register.

Could you document the difference in a comment, and also how the B1 variant
is told apart from the 2A one?

> 
> > > +
> > >  	/* To serialize asynchronus callbacks */
> > >  	struct mutex mutex;
> > >  
> > > @@ -696,6 +1155,24 @@ static int ov8856_test_pattern(struct ov8856 *ov8856, u32 pattern)
> > >  				OV8856_REG_VALUE_08BIT, pattern);
> > >  }
> > >  
> > > +static int ov8856_update_otp_reg(struct ov8856 *ov8856)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = ov8856_write_reg(ov8856, OV8856_REG_MODE_SELECT,
> > > +			       OV8856_REG_VALUE_08BIT, OV8856_MODE_STREAMING);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = ov8856_write_reg(ov8856, OV8856_OTP_REG_ONE,
> > > +			       OV8856_REG_VALUE_08BIT, OV8856_MODE_STANDBY);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return ov8856_write_reg(ov8856, OV8856_OTP_REG_TWO,
> > > +				OV8856_REG_VALUE_08BIT, OV8856_MODE_STREAMING);
> > > +}
> > 
> > What does this do?
> > 
> 
> R700f is OTP register, the register value corresponds different hardware
> version.
> Mainly including:
> 01: 2A: EMI improvement
> 02: 1B: remove some mask for cost; initial setting cannot share with 1A

If you're accessing EEPROM here, please tell that.

Do you really need to start streaming to do that? Please add comments to
what do these steps actually achieve.

> 
> To identify sensor chip version, the following steps needs to be done
> before reading out R700f.
> R0100 write 0x01.
> R3d84 write 0x00
> R3d81 write 0x01
> 
> To make more clear, ov8856_update_otp_reg() would be renamed to
> ov8856_check_revision() in next release.

Sounds good to me.

> 
> > > +
> > >  static int ov8856_set_ctrl(struct v4l2_ctrl *ctrl)
> > >  {
> > >  	struct ov8856 *ov8856 = container_of(ctrl->handler,
> > > @@ -825,7 +1302,13 @@ static void ov8856_update_pad_format(const struct ov8856_mode *mode,
> > >  {
> > >  	fmt->width = mode->width;
> > >  	fmt->height = mode->height;
> > > -	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
> > > +
> > > +	/* Bayer Order is determined by image resolution */
> > 
> > Ouch. 
> > 
> > This rather looks like a side effect of vertical cropping. How about
> > either cropping one line above or below, to keep the same Bayer order?
> > 
> > The driver is based on register lists that heavily restricts the
> > possibilities of configuring the sensor. The alternative, should more
> > free-form configuration be enabled, would be to expose the cropping
> > capability to the user --- as well as binning.
> > 
> 
> Oh, this is quite a long story.
> From datasheet, the  Bayer Order should be BGGR, and the whole active
> pixel array should be 3264x2448.
> In fact, I am a little confused about why there exits another image size
> (3280x2464).
> Or maybe this is one old sensor version.
> 
> +Cc: Ben.
> Ben, could help introduce some background at that time?
> 
> 
> > > +	if (fmt->width == 3264 || fmt->width == 1632)
> > > +		fmt->code = MEDIA_BUS_FMT_SBGGR10_1X10;
> > > +	else
> > > +		fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
> > > +
> > >  	fmt->field = V4L2_FIELD_NONE;
> > >  }
> > >  
> > > @@ -850,6 +1333,17 @@ static int ov8856_start_streaming(struct ov8856 *ov8856)
> > >  		return ret;
> > >  	}
> > >  
> > > +	/* update R3614 for 1B module */
> > > +	if (ov8856->is_1B_module) {
> > > +		ret = ov8856_write_reg(ov8856, OV8856_CLK_REG,
> > > +				       OV8856_REG_VALUE_08BIT,
> > > +				       OV8856_CLK_REG_1B_VAL);
> > > +		if (ret) {
> > > +			dev_err(&client->dev, "failed to set R3614");
> > > +			return ret;
> > > +		}
> > > +	}
> > > +
> > >  	ret = __v4l2_ctrl_handler_setup(ov8856->sd.ctrl_handler);
> > >  	if (ret)
> > >  		return ret;
> > > @@ -882,6 +1376,8 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
> > >  	if (ov8856->streaming == enable)
> > >  		return 0;
> > >  
> > > +	dev_dbg(&client->dev, "hardware version: (%d)\n", ov8856->is_1B_module);
> > > +
> > >  	mutex_lock(&ov8856->mutex);
> > >  	if (enable) {
> > >  		ret = pm_runtime_get_sync(&client->dev);
> > > @@ -908,6 +1404,54 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
> > >  	return ret;
> > >  }
> > >  
> > > +/* Calculate the delay in us by clock rate and clock cycles */
> > > +static inline u32 ov8856_cal_delay(u32 cycles)
> > > +{
> > > +	return DIV_ROUND_UP(cycles, OV8856_XVCLK_FREQ / 1000 / 1000);
> > 
> > The frequency is rounded down. As it is used to calculate a delay needed,
> > rounding up should be done for the frequency, too.
> > 
> 
> It seems that OV8856_XVCLK_FREQ / 1000 / 1000 is a rounded-down value.
> But Note that here OV8856_XVCLK_FREQ is 24MHz.
> So OV8856_XVCLK_FREQ / 1000 / 1000 is exactly an integer value.

This breaks already on the frequency already supported by the driver (19,2
MHz).

...

> > > +
> > > +	return 0;
> > > +
> > > +disable_clk:
> > > +	clk_disable_unprepare(ov8856->xvclk);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static void __ov8856_power_off(struct ov8856 *ov8856)
> > > +{
> > > +	clk_disable_unprepare(ov8856->xvclk);
> > > +	gpiod_set_value_cansleep(ov8856->reset_gpio, 1);
> > > +
> > > +	regulator_bulk_disable(OV8856_NUM_SUPPLIES, ov8856->supplies);
> > > +}
> > > +
> > >  static int __maybe_unused ov8856_suspend(struct device *dev)
> > >  {
> > >  	struct i2c_client *client = to_i2c_client(dev);
> > > @@ -915,8 +1459,8 @@ static int __maybe_unused ov8856_suspend(struct device *dev)
> > >  	struct ov8856 *ov8856 = to_ov8856(sd);
> > >  
> > >  	mutex_lock(&ov8856->mutex);
> > > -	if (ov8856->streaming)
> > > -		ov8856_stop_streaming(ov8856);
> > 
> > This seems like an unrelated change.
> > 
> 
> Sorry, there is one concerns.
> For set_stream(0), it seems that pm_runtime_put would be called after
> ov8856_stop_streaming.
> I wonder that whether ov8856_stop_streaming is called twice when device
> suspended.

If you think that's the case, please fix it in a separate patch.

...

> > > +
> > > +	for (i = 0; i < OV8856_NUM_SUPPLIES; i++)
> > > +		ov8856->supplies[i].supply = ov8856_supply_names[i];
> > > +
> > > +	return devm_regulator_bulk_get(&client->dev,
> > > +				       OV8856_NUM_SUPPLIES,
> > > +				       ov8856->supplies);
> > 
> > Remember to put the regulators, too.
> > 
> 
> Thanks for reminder.
> ov8856_power_off would call regulator_bulk_disable.

I missed you were using the devm_ variant. No need to put the regulators
(but disable you'll still need).

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Applied "ASoC: rockchip: ignore 0Hz sysclk" to the asoc tree
From: Mark Brown @ 2019-09-09 10:07 UTC (permalink / raw)
  To: Katsuhiro Suzuki
  Cc: alsa-devel, Mark Brown, Heiko Stuebner, linux-arm-kernel,
	linux-kernel
In-Reply-To: <20190907174332.19586-1-katsuhiro@katsuster.net>

The patch

   ASoC: rockchip: ignore 0Hz sysclk

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From f1879d7b98dc9081cffc7718b644c6c41628cd18 Mon Sep 17 00:00:00 2001
From: Katsuhiro Suzuki <katsuhiro@katsuster.net>
Date: Sun, 8 Sep 2019 02:43:32 +0900
Subject: [PATCH] ASoC: rockchip: ignore 0Hz sysclk

This patch ignores sysclk setting if it is 0Hz.

Some codecs treat 0Hz sysclk as signal of applying no constraints.
This driver does not have such feature but current implementation
outputs 'Failed to set mclk' error message if machine driver sets
0Hz sysclk to this driver.

Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
Link: https://lore.kernel.org/r/20190907174332.19586-1-katsuhiro@katsuster.net
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/rockchip/rockchip_i2s.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 88ebaf6e1880..af2d5a6124c8 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -419,6 +419,9 @@ static int rockchip_i2s_set_sysclk(struct snd_soc_dai *cpu_dai, int clk_id,
 	struct rk_i2s_dev *i2s = to_info(cpu_dai);
 	int ret;
 
+	if (freq == 0)
+		return 0;
+
 	ret = clk_set_rate(i2s->mclk, freq);
 	if (ret)
 		dev_err(i2s->dev, "Fail to set mclk %d\n", ret);
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [V4, 1/2] media: dt-bindings: media: i2c: Add bindings for OV02A10
From: Andy Shevchenko @ 2019-09-09 10:11 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, shengnan.wang,
	tfiga, louis.kuo, sj.huang, robh+dt, linux-mediatek, dongchun.zhu,
	matthias.bgg, bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <20190909095715.GA5781@paasikivi.fi.intel.com>

On Mon, Sep 09, 2019 at 12:57:15PM +0300, Sakari Ailus wrote:
> Hi Andy,
> 
> On Mon, Sep 09, 2019 at 12:45:01PM +0300, Andy Shevchenko wrote:
> > On Sat, Sep 07, 2019 at 05:27:27PM +0800, dongchun.zhu@mediatek.com wrote:
> > > From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > 
> > > This patch adds device tree bindings documentation for OV02A10 camera sensor.
> > > 
> > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > ---
> > >  .../devicetree/bindings/media/i2c/ov02a10.txt      | 54 ++++++++++++++++++++++
> > 
> > >  MAINTAINERS                                        |  7 +++
> > 
> > This doesn't belong to dt-bindings.
> 
> The practice has been to add the MAINTAINERS entry along with the DT
> bindings.

Ah, okay, good to know!

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 3/3] arm64: alpine: select AL_POS
From: Shenhar, Talel @ 2019-09-09 10:16 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mark Rutland, mjourdan, Catalin Marinas, Linus Walleij,
	linux-kernel@vger.kernel.org, jonnyc, Mauro Carvalho Chehab,
	ronenk, Will Deacon, Benjamin Herrenschmidt, DTML, Maxime Ripard,
	Rob Herring, Santosh Shilimkar, Thomas Gleixner, hanochu,
	Linux ARM, barakw, hhhawa, gregkh, paul.kocialkowski,
	Patrick Venture, Olof Johansson, David Miller, David Woodhouse
In-Reply-To: <CAK8P3a0DEMeFWK+RuAdSLyDYduWWwj9DxP_Beipays-d_6ixnA@mail.gmail.com>


On 9/9/2019 12:40 PM, Arnd Bergmann wrote:
> On Mon, Sep 9, 2019 at 11:14 AM Talel Shenhar <talel@amazon.com> wrote:
>> Amazon's Annapurna Labs SoCs uses al-pos driver, enable it.
>>
>> Signed-off-by: Talel Shenhar <talel@amazon.com>
>> ---
>>   arch/arm64/Kconfig.platforms | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>> index 4778c77..bd86b15 100644
>> --- a/arch/arm64/Kconfig.platforms
>> +++ b/arch/arm64/Kconfig.platforms
>> @@ -25,6 +25,7 @@ config ARCH_SUNXI
>>   config ARCH_ALPINE
>>          bool "Annapurna Labs Alpine platform"
>>          select ALPINE_MSI if PCI
>> +       select AL_POS
>>          help
>>            This enables support for the Annapurna Labs Alpine
>>            Soc family.
> Generally I think this kind of thing should go into the defconfig
> rather than being hard-selected. There might be users that
> want to not enable the driver.
>
>         Arnd

The reason for selecting it is because this is a driver that we will 
always want for ARCH_ALPINE.



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V4 03/11] clk: imx: scu: add two cells binding support
From: Dong Aisheng @ 2019-09-09 10:23 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Dong Aisheng, Michael Turquette, dl-linux-imx, Sascha Hauer,
	Fabio Estevam, Shawn Guo, linux-clk,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20190906170643.B310F20578@mail.kernel.org>

Hi Stephen,

Thanks for the review.

On Sat, Sep 7, 2019 at 5:29 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Dong Aisheng (2019-08-20 04:13:17)
> > diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
> > index 5e2903e..1ad3f2a 100644
> > --- a/drivers/clk/imx/clk-imx8qxp.c
> > +++ b/drivers/clk/imx/clk-imx8qxp.c
> > @@ -134,7 +134,12 @@ static int imx8qxp_clk_probe(struct platform_device *pdev)
> >                                 i, PTR_ERR(clks[i]));
> >         }
> >
> > -       return of_clk_add_hw_provider(ccm_node, of_clk_hw_onecell_get, clk_data);
> > +       if (clock_cells == 2)
>
> Can you just read this from the DT node again instead of having a global
> variable called "clock_cells" for this?
>

I tried thinking about it.
One problem is that we also need this information in the exist clk
registration API to
keep the backwards compatibility:
e.g.
 static inline struct clk_hw *imx_clk_scu(const char *name, u32 rsrc_id,
                                         u8 clk_type)
 {
-       return __imx_clk_scu(name, NULL, 0, rsrc_id, clk_type);
+       if (clock_cells == 2)
+               return imx_clk_scu_alloc_dev(name, NULL, 0, rsrc_id, clk_type);
+       else
+               return __imx_clk_scu(name, NULL, 0, rsrc_id, clk_type);
 }

Parsing it for all clocks seems not good.

In the future, i planned to totally remove the legacy binding support which
is a premature one and missing continued support.
Then we will also remove this unneeded clock_cells.

> > +               ret = of_clk_add_hw_provider(ccm_node, imx_scu_of_clk_src_get, imx_scu_clks);
> > +       else
> > +               ret = of_clk_add_hw_provider(ccm_node, of_clk_hw_onecell_get, clk_data);
> > +
> > +       return ret;
> >  }
> >
> >  static const struct of_device_id imx8qxp_match[] = {
> > diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
> > index fbef740..48bfb08 100644
> > --- a/drivers/clk/imx/clk-scu.c
> > +++ b/drivers/clk/imx/clk-scu.c
> > @@ -16,6 +19,21 @@
> >  #define IMX_SIP_SET_CPUFREQ            0x00
> >
> >  static struct imx_sc_ipc *ccm_ipc_handle;
> > +struct device_node *pd_np;
> > +u32 clock_cells;
> > +
> > +struct imx_scu_clk_node {
> > +       const char *name;
> > +       u32 rsrc;
> > +       u8 clk_type;
> > +       const char * const *parents;
> > +       int num_parents;
> > +
> > +       struct clk_hw *hw;
> > +       struct list_head node;
> > +};
> > +
> > +struct list_head imx_scu_clks[IMX_SC_R_LAST];
> >
> >  /*
> >   * struct clk_scu - Description of one SCU clock
> > @@ -128,9 +146,29 @@ static inline struct clk_scu *to_clk_scu(struct clk_hw *hw)
> >         return container_of(hw, struct clk_scu, hw);
> >  }
> >
> > -int imx_clk_scu_init(void)
> > +int imx_clk_scu_init(struct device_node *np)
> >  {
> > -       return imx_scu_get_handle(&ccm_ipc_handle);
> > +       struct platform_device *pd_dev;
> > +       int ret, i;
> > +
> > +       ret = imx_scu_get_handle(&ccm_ipc_handle);
> > +       if (ret)
> > +               return ret;
> > +
> > +       if (of_property_read_u32(np, "#clock-cells", &clock_cells))
> > +               return -EINVAL;
> > +
> > +       if (clock_cells == 2) {
> > +               for (i = 0; i < IMX_SC_R_LAST; i++)
> > +                       INIT_LIST_HEAD(&imx_scu_clks[i]);
> > +
> > +               pd_np = of_find_compatible_node(NULL, NULL, "fsl,scu-pd");
> > +               pd_dev = of_find_device_by_node(pd_np);
> > +               if (!pd_dev || !device_is_bound(&pd_dev->dev))
> > +                       return -EPROBE_DEFER;
>
> Do you need to put some nodes here with of_node_put() one failure or
> when they're done being used?
>

Good catch.
We should put the node for of_find_compatible_node().

> > +       }
> > +
> > +       return 0;
> >  }
> >
> >  /*
> > @@ -387,3 +425,99 @@ struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
> [...]
> > +
> > +struct clk_hw *imx_clk_scu_alloc_dev(const char *name,
> > +                                    const char * const *parents,
> > +                                    int num_parents, u32 rsrc_id, u8 clk_type)
> > +{
> > +       struct imx_scu_clk_node clk = {
> > +               .name = name,
> > +               .rsrc = rsrc_id,
> > +               .clk_type = clk_type,
> > +               .parents = parents,
> > +               .num_parents = num_parents,
> > +       };
> > +       struct platform_device *pdev;
> > +       int ret;
> > +
> > +       pdev = platform_device_alloc(name, PLATFORM_DEVID_NONE);
> > +       if (!pdev) {
> > +               pr_err("%s: failed to allocate scu clk dev rsrc %d type %d\n",
> > +                      name, rsrc_id, clk_type);
> > +               return ERR_PTR(-ENOMEM);
> > +       }
> > +
> > +       ret = platform_device_add_data(pdev, &clk, sizeof(clk));
> > +       if (ret) {
> > +               platform_device_put(pdev);
> > +               return ERR_PTR(-ENOMEM);
>
> Why not ERR_PTR(ret)?
>

Good catch. Will fix.

Regards
Aisheng

> > +       }
> > +
> > +       pdev->driver_override = "imx-scu-clk";
> > +
> > +       ret = imx_clk_scu_attach_pd(&pdev->dev, rsrc_id);
> > +       if (ret)
> > +               pr_warn("%s: failed to attached the power domain %d\n",
> > +                       name, ret);
> > +
> > +       platform_device_add(pdev);
> > +
> > +       /* For API backwards compatiblilty, simply return NULL for success */
> > +       return NULL;
> > +}

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V4 04/11] clk: imx: scu: bypass cpu power domains
From: Dong Aisheng @ 2019-09-09 10:24 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Dong Aisheng, Michael Turquette, dl-linux-imx, Sascha Hauer,
	Fabio Estevam, Shawn Guo, linux-clk,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20190906170713.81A1A20578@mail.kernel.org>

On Sat, Sep 7, 2019 at 5:28 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Dong Aisheng (2019-08-20 04:13:18)
> > diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
> > index 48bfb08..5f935b1 100644
> > --- a/drivers/clk/imx/clk-scu.c
> > +++ b/drivers/clk/imx/clk-scu.c
> > @@ -479,6 +479,10 @@ static int imx_clk_scu_attach_pd(struct device *dev, u32 rsrc_id)
> >                 .args[0] = rsrc_id,
> >         };
> >
> > +       if ((rsrc_id == IMX_SC_R_A35) || (rsrc_id == IMX_SC_R_A53) ||
> > +           (rsrc_id == IMX_SC_R_A72))
>
> Please drop the extra parenthesis. It makes it hard to read.
>

Will drop it, thx

Regards
Aisheng

> > +               return 0;
> > +
> >         return of_genpd_add_device(&genpdspec, dev);

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V4 07/11] clk: imx: scu: add suspend/resume support
From: Dong Aisheng @ 2019-09-09 10:35 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Dong Aisheng, Michael Turquette, dl-linux-imx, Sascha Hauer,
	Fabio Estevam, Shawn Guo, linux-clk,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20190906170944.B861620578@mail.kernel.org>

On Sat, Sep 7, 2019 at 5:32 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Dong Aisheng (2019-08-20 04:13:21)
> > Clock state will be lost when its power domain is completely off
> > during system suspend/resume. So we save and restore the state
> > accordingly in suspend/resume callback.
>
> And this doesn't need any coordination with other clks in the clk tree
> right?

AFAIK no as SC firmware may have handled it properly.

>
> > diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
> > index edc39d7..8d9cfa2 100644
> > --- a/drivers/clk/imx/clk-scu.c
> > +++ b/drivers/clk/imx/clk-scu.c
> > @@ -46,6 +46,10 @@ struct clk_scu {
> >         struct clk_hw hw;
> >         u16 rsrc_id;
> >         u8 clk_type;
> > +
> > +       /* for state save&restore */
> > +       bool is_enabled;
> > +       u32 rate;
> >  };
> >
> >  /*
> > @@ -425,6 +429,9 @@ struct clk_hw *__imx_clk_scu(struct device *dev, const char *name,
> >                 hw = ERR_PTR(ret);
> >         }
> >
> > +       if (dev)
> > +               dev_set_drvdata(dev, clk);
> > +
> >         return hw;
> >  }
> >
> > @@ -481,10 +488,52 @@ static int imx_clk_scu_probe(struct platform_device *pdev)
> >         return 0;
> >  }
> >
> > +int __maybe_unused imx_clk_scu_suspend(struct device *dev)
>
> static?
>
> > +{
> > +       struct clk_scu *clk = dev_get_drvdata(dev);
> > +
> > +       clk->rate = clk_hw_get_rate(&clk->hw);
> > +       clk->is_enabled = clk_hw_is_enabled(&clk->hw);
> > +
> > +       if (clk->rate)
> > +               dev_dbg(dev, "save rate %d\n", clk->rate);
> > +
> > +       if (clk->is_enabled)
> > +               dev_dbg(dev, "save enabled state\n");
> > +
> > +       return 0;
> > +}
> > +
> > +int __maybe_unused imx_clk_scu_resume(struct device *dev)
>
> static?
>
> > +{
> > +       struct clk_scu *clk = dev_get_drvdata(dev);
> > +       int ret = 0;
> > +
> > +       if (clk->rate) {
> > +               ret = clk_scu_set_rate(&clk->hw, clk->rate, 0);
> > +               dev_dbg(dev, "restore rate %d %s\n", clk->rate,
> > +                       !ret ? "success" : "failed");
> > +       }
> > +
> > +       if (clk->is_enabled) {
> > +               ret = clk_scu_prepare(&clk->hw);
> > +               dev_dbg(dev, "restore enabled state %s\n",
> > +                       !ret ? "success" : "failed");
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> > +const struct dev_pm_ops imx_clk_scu_pm_ops = {
>
> static?
>

Sorry that i missed to update here as those function are changed to be
used within
this file now.
Will fix.

Regards
Aisheng

> > +       SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_clk_scu_suspend,
> > +                                     imx_clk_scu_resume)
> > +};
> > +

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 1/2] mm/page_ext: support to record the last stack of page
From: David Hildenbrand @ 2019-09-09 10:57 UTC (permalink / raw)
  To: Walter Wu, Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov,
	Matthias Brugger, Andrew Morton, Martin Schwidefsky, Will Deacon,
	Andrey Konovalov, Arnd Bergmann, Thomas Gleixner, Michal Hocko,
	Qian Cai
  Cc: wsd_upstream, linux-kernel, kasan-dev, linux-mm, linux-mediatek,
	linux-arm-kernel
In-Reply-To: <20190909085339.25350-1-walter-zh.wu@mediatek.com>

On 09.09.19 10:53, Walter Wu wrote:
> KASAN will record last stack of page in order to help programmer
> to see memory corruption caused by page.
> 
> What is difference between page_owner and our patch?
> page_owner records alloc stack of page, but our patch is to record
> last stack(it may be alloc or free stack of page).
> 
> Signed-off-by: Walter Wu <walter-zh.wu@mediatek.com>
> ---
>  mm/page_ext.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/mm/page_ext.c b/mm/page_ext.c
> index 5f5769c7db3b..7ca33dcd9ffa 100644
> --- a/mm/page_ext.c
> +++ b/mm/page_ext.c
> @@ -65,6 +65,9 @@ static struct page_ext_operations *page_ext_ops[] = {
>  #if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
>  	&page_idle_ops,
>  #endif
> +#ifdef CONFIG_KASAN
> +	&page_stack_ops,
> +#endif
>  };
>  
>  static unsigned long total_usage;
> 

Are you sure this patch compiles?

t460s: ~/git/linux virtio-mem $ git grep page_stack_ops
t460s: ~/git/linux virtio-mem $

-- 

Thanks,

David / dhildenb

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 06/14] dmaengine: ti: Add cppi5 header for UDMA
From: Peter Ujfalusi @ 2019-09-09 10:59 UTC (permalink / raw)
  To: Vinod Koul
  Cc: nm, devicetree, grygorii.strashko, lokeshvutla, j-keerthy,
	linux-kernel, t-kristo, tony, robh+dt, ssantosh, dmaengine,
	dan.j.williams, linux-arm-kernel
In-Reply-To: <20190908142528.GP2672@vkoul-mobl>



On 08/09/2019 17.25, Vinod Koul wrote:
> On 30-07-19, 12:34, Peter Ujfalusi wrote:
> 
>> +/**
>> + * Descriptor header, present in all types of descriptors
>> + */
>> +struct cppi5_desc_hdr_t {
>> +	u32 pkt_info0;	/* Packet info word 0 (n/a in Buffer desc) */
>> +	u32 pkt_info1;	/* Packet info word 1 (n/a in Buffer desc) */
>> +	u32 pkt_info2;	/* Packet info word 2 Buffer reclamation info */
>> +	u32 src_dst_tag; /* Packet info word 3 (n/a in Buffer desc) */
> 
> Can we move these comments to kernel-doc style please

Sure, I'll move all struct and enums.

>> +/**
>> + * cppi5_desc_get_type - get descriptor type
>> + * @desc_hdr: packet descriptor/TR header
>> + *
>> + * Returns descriptor type:
>> + * CPPI5_INFO0_DESC_TYPE_VAL_HOST
>> + * CPPI5_INFO0_DESC_TYPE_VAL_MONO
>> + * CPPI5_INFO0_DESC_TYPE_VAL_TR
>> + */
>> +static inline u32 cppi5_desc_get_type(struct cppi5_desc_hdr_t *desc_hdr)
>> +{
>> +	WARN_ON(!desc_hdr);
> 
> why WARN_ON and not return error!

these helpers were intended to be as simple as possible.
I can go through with all of the WARN_ONs and replace them with if()
pr_warn() and either just return or return with 0.

Would that be acceptable?

>> +/**
>> + * cppi5_hdesc_calc_size - Calculate Host Packet Descriptor size
>> + * @epib: is EPIB present
>> + * @psdata_size: PSDATA size
>> + * @sw_data_size: SWDATA size
>> + *
>> + * Returns required Host Packet Descriptor size
>> + * 0 - if PSDATA > CPPI5_INFO0_HDESC_PSDATA_MAX_SIZE
>> + */
>> +static inline u32 cppi5_hdesc_calc_size(bool epib, u32 psdata_size,
>> +					u32 sw_data_size)
>> +{
>> +	u32 desc_size;
>> +
>> +	if (psdata_size > CPPI5_INFO0_HDESC_PSDATA_MAX_SIZE)
>> +		return 0;
>> +	//TODO_GS: align
> 
> :)

Leftover TODO from Grygorii, the align is already done.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH RFC] gpio: define gpio-init nodes to initialize pins similar to hogs
From: Uwe Kleine-König @ 2019-09-09 10:59 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski; +Cc: linux-gpio, linux-arm-kernel, kernel

Sometimes it is handy to be able to easily define a "safe" state for a
GPIO. This might for example be used to ensure that an ethernet phy is
properly reset during startup or just that all pins have a defined state
to minimize leakage current. As such a pin must be requestable (and
changable) by a device driver, a gpio-hog cannot be used.

So define a GPIO initializer with a syntax identical to a GPIO hog just
using "gpio-init" as identifier instead of "gpio-hog".

The usage I have in mind (and also implemented in a custom patch stack
on top of barebox already) is targeting the bootloader and not
necessarily Linux as such an boot-up initialisation should be done as
early as possible.

Not-yet-signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

maybe it also makes sense to use "gpio-safe"? Maybe it (then) makes
sense to reset the gpio in the indicated state after it is released?

Also it might be beneficial to make the wording more explicit in the
description and for example tell that only one of gpio-hog and gpio-init
must be provided. 

Best regards
Uwe

 Documentation/devicetree/bindings/gpio/gpio.txt | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
index a8895d339bfe..5b7883f5520f 100644
--- a/Documentation/devicetree/bindings/gpio/gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio.txt
@@ -182,13 +182,16 @@ gpio-controller@00000000 {
 		"poweroff", "reset";
 }
 
-The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism
-providing automatic GPIO request and configuration as part of the
-gpio-controller's driver probe function.
+The GPIO chip may contain GPIO hog and init definitions. GPIO hogging is a
+mechanism providing automatic GPIO request and configuration as part of the
+gpio-controller's driver probe function. An GPIO initializer is similar but
+doesn't prevent later requesting and reconfiguration.
 
 Each GPIO hog definition is represented as a child node of the GPIO controller.
 Required properties:
 - gpio-hog:   A property specifying that this child node represents a GPIO hog.
+- gpio-init:  A property specifying that this child node represents a GPIO
+	      initializer.
 - gpios:      Store the GPIO information (id, flags, ...) for each GPIO to
 	      affect. Shall contain an integer multiple of the number of cells
 	      specified in its parent node (GPIO controller node).
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] irqchip/atmel-aic5: add support for sam9x60 irqchip
From: Claudiu Beznea @ 2019-09-09 11:00 UTC (permalink / raw)
  To: tglx, jason, maz, robh+dt, mark.rutland, nicolas.ferre,
	alexandre.belloni, ludovic.desroches
  Cc: devicetree, Sandeep Sheriker Mallikarjun, linux-kernel,
	linux-arm-kernel, Claudiu Beznea

From: Sandeep Sheriker Mallikarjun <sandeepsheriker.mallikarjun@microchip.com>

Add support for SAM9X60 irqchip.

Signed-off-by: Sandeep Sheriker Mallikarjun <sandeepsheriker.mallikarjun@microchip.com>
[claudiu.beznea@microchip.com: update aic5_irq_fixups[], update
 documentation]
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 .../devicetree/bindings/interrupt-controller/atmel,aic.txt     |  7 +++++--
 drivers/irqchip/irq-atmel-aic5.c                               | 10 ++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt b/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt
index f4c5d34c4111..7079d44bf3ba 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt
@@ -1,8 +1,11 @@
 * Advanced Interrupt Controller (AIC)
 
 Required properties:
-- compatible: Should be "atmel,<chip>-aic"
-  <chip> can be "at91rm9200", "sama5d2", "sama5d3" or "sama5d4"
+- compatible: Should be:
+    - "atmel,<chip>-aic" where  <chip> can be "at91rm9200", "sama5d2",
+      "sama5d3" or "sama5d4"
+    - "microchip,<chip>-aic" where <chip> can be "sam9x60"
+
 - interrupt-controller: Identifies the node as an interrupt controller.
 - #interrupt-cells: The number of cells to define the interrupts. It should be 3.
   The first cell is the IRQ number (aka "Peripheral IDentifier" on datasheet).
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 6acad2ea0fb3..29333497ba10 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -313,6 +313,7 @@ static void __init sama5d3_aic_irq_fixup(void)
 static const struct of_device_id aic5_irq_fixups[] __initconst = {
 	{ .compatible = "atmel,sama5d3", .data = sama5d3_aic_irq_fixup },
 	{ .compatible = "atmel,sama5d4", .data = sama5d3_aic_irq_fixup },
+	{ .compatible = "microchip,sam9x60", .data = sama5d3_aic_irq_fixup },
 	{ /* sentinel */ },
 };
 
@@ -390,3 +391,12 @@ static int __init sama5d4_aic5_of_init(struct device_node *node,
 	return aic5_of_init(node, parent, NR_SAMA5D4_IRQS);
 }
 IRQCHIP_DECLARE(sama5d4_aic5, "atmel,sama5d4-aic", sama5d4_aic5_of_init);
+
+#define NR_SAM9X60_IRQS		50
+
+static int __init sam9x60_aic5_of_init(struct device_node *node,
+				       struct device_node *parent)
+{
+	return aic5_of_init(node, parent, NR_SAM9X60_IRQS);
+}
+IRQCHIP_DECLARE(sam9x60_aic5, "microchip,sam9x60-aic", sam9x60_aic5_of_init);
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH 2/3] soc: amazon: al-pos: Introduce Amazon's Annapurna Labs POS driver
From: Shenhar, Talel @ 2019-09-09 11:12 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mark Rutland, mjourdan, Catalin Marinas, Linus Walleij,
	linux-kernel@vger.kernel.org, jonnyc, Mauro Carvalho Chehab,
	ronenk, Will Deacon, Benjamin Herrenschmidt, DTML, Maxime Ripard,
	Rob Herring, Santosh Shilimkar, Thomas Gleixner, hanochu,
	Linux ARM, barakw, hhhawa, gregkh, paul.kocialkowski,
	Patrick Venture, Olof Johansson, David Miller, David Woodhouse
In-Reply-To: <CAK8P3a3UF7xPV1U3eW6Jdu754P1bzG208UxD9KUxEm1JjZudww@mail.gmail.com>


On 9/9/2019 12:44 PM, Arnd Bergmann wrote:
> On Mon, Sep 9, 2019 at 11:14 AM Talel Shenhar <talel@amazon.com> wrote:
>> The Amazon's Annapurna Labs SoCs includes Point Of Serialization error
>> logging unit that reports an error in case write error (e.g. attempt to
>> write to a read only register).
>> This patch introduces the support for this unit.
>>
>> Signed-off-by: Talel Shenhar <talel@amazon.com>
> Looks ok overall, juts a few minor comments:
Thanks.
>
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_AUTHOR("Talel Shenhar");
>> +MODULE_DESCRIPTION("Amazon's Annapurna Labs POS driver");
> These usually go to the end of the file.
Ack, Will move them as part of v2.
>
>> +       log1 = readl_relaxed(pos->mmio_base + AL_POS_ERROR_LOG_1);
>> +       if (!FIELD_GET(AL_POS_ERROR_LOG_1_VALID, log1))
>> +               return IRQ_NONE;
>> +
>> +       log0 = readl_relaxed(pos->mmio_base + AL_POS_ERROR_LOG_0);
>> +       writel_relaxed(0, pos->mmio_base + AL_POS_ERROR_LOG_1);
> Why do you require _relaxed() accessors here? Please add a comment
> explaining that, or use the regular readl()/writel().

I don't think commenting is needed here as there is nothing special in 
this type of access.

I don't see this is common to comment the use of the _relaxed accessors.

This driver is for SoC using arm64 cpu.

If one uses the non-relaxed version of readl while running on arm64, he 
shall cause read barrier, which is then doing dsm(ld).. This barrier is 
not needed here, so we spare the use of the more heavy readl in favor of 
the less "harmful" one.

Let me know what you think.

>
>> +       resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       pos->mmio_base = devm_ioremap_resource(&pdev->dev, resource);
> This can be simplified to devm_platform_ioremap_resource().
Ack, Will simplify them in v2.
>
>> +       pos->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> And this is usually written as platform_get_irq()
Ack, Will replace them with platform_get_irq() in v2.
>
>         Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 7/8] thermal: mediatek: add another get_temp ops for thermal sensors
From: Hsin-Yi Wang @ 2019-09-09 11:13 UTC (permalink / raw)
  To: michael.kao
  Cc: Mark Rutland, Devicetree List, linux-pm, Daniel Lezcano, lkml,
	Eduardo Valentin, Rob Herring, linux-mediatek, Matthias Brugger,
	Zhang Rui,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <1557494826-6044-8-git-send-email-michael.kao@mediatek.com>

On Fri, May 10, 2019 at 9:27 PM michael.kao <michael.kao@mediatek.com> wrote:
>
> From: Michael Kao <michael.kao@mediatek.com>
>
> Provide thermal zone to read thermal sensor
> in the SoC. We can read all the thermal sensors
> value in the SoC by the node /sys/class/thermal/
>
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  drivers/thermal/mtk_thermal.c | 68 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 60 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index cb41e46..d5c78b0 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -230,6 +230,11 @@ enum {
>
>  struct mtk_thermal;
>
> +struct mtk_thermal_zone {
> +       struct mtk_thermal *mt;
> +       int id;
> +};
> +
>  struct thermal_bank_cfg {
>         unsigned int num_sensors;
>         const int *sensors;
> @@ -612,7 +617,7 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
>                  * not immediately shut down.
>                  */
>                 if (temp > 200000)
> -                       temp = 0;
> +                       temp = -EACCES;
Should we return -EAGAIN here? Otherwise there would be warning:
[    0.344167] thermal thermal_zone5: failed to read out thermal zone (-13)

Since first read is bogus value, I think EAGAIN also makes sense here.
>
>                 if (temp > max)
>                         max = temp;
> @@ -623,7 +628,8 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
>
>  static int mtk_read_temp(void *data, int *temperature)
>  {
> -       struct mtk_thermal *mt = data;
> +       struct mtk_thermal_zone *tz = data;
> +       struct mtk_thermal *mt = tz->mt;
>         int i;
>         int tempmax = INT_MIN;
>
> @@ -636,16 +642,48 @@ static int mtk_read_temp(void *data, int *temperature)
>
>                 mtk_thermal_put_bank(bank);
>         }
> -
>         *temperature = tempmax;
>
>         return 0;
>  }
>
> +static int mtk_read_sensor_temp(void *data, int *temperature)
> +{
> +       struct mtk_thermal_zone *tz = data;
> +       struct mtk_thermal *mt = tz->mt;
> +       const struct mtk_thermal_data *conf = mt->conf;
> +       int id = tz->id - 1;
> +       int temp = INT_MIN;
> +       u32 raw;
> +
> +       if (id < 0)
> +               return  -EACCES;
> +
> +       raw = readl(mt->thermal_base + conf->msr[id]);
> +
> +       temp = raw_to_mcelsius(mt, id, raw);
> +
> +       /*
> +        * The first read of a sensor often contains very high bogus
> +        * temperature value. Filter these out so that the system does
> +        * not immediately shut down.
> +        */
> +
> +       if (temp > 200000)
> +               return  -EACCES;
Same here.
> +
> +       *temperature = temp;
> +       return 0;
> +}
> +
>  static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
>         .get_temp = mtk_read_temp,
>  };
>
> +static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
> +       .get_temp = mtk_read_sensor_temp,
> +};
> +
>  static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
>                                   u32 apmixed_phys_base, u32 auxadc_phys_base,
>                                   int ctrl_id)
> @@ -878,6 +916,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>         struct resource *res;
>         u64 auxadc_phys_base, apmixed_phys_base;
>         struct thermal_zone_device *tzdev;
> +       struct mtk_thermal_zone *tz;
>
>         mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
>         if (!mt)
> @@ -959,11 +998,24 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>
>         platform_set_drvdata(pdev, mt);
>
> -       tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
> -                                                    &mtk_thermal_ops);
> -       if (IS_ERR(tzdev)) {
> -               ret = PTR_ERR(tzdev);
> -               goto err_disable_clk_peri_therm;
> +       for (i = 0; i < mt->conf->num_sensors + 1; i++) {
> +               tz = kmalloc(sizeof(*tz), GFP_KERNEL);
> +               if (!tz)
> +                       return -ENOMEM;
> +
> +               tz->mt = mt;
> +               tz->id = i;
> +
> +               tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i,
> +                               tz, (i == 0) ?
> +                               &mtk_thermal_ops : &mtk_thermal_sensor_ops);
> +
> +               if (IS_ERR(tzdev)) {
> +                       if (IS_ERR(tzdev) != -EACCES) {
> +                               ret = PTR_ERR(tzdev);
> +                               goto err_disable_clk_peri_therm;
> +                       }
> +               }
>         }
>
>         return 0;
> --
> 2.6.4
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: Status of led-backlight driver
From: Daniel Thompson @ 2019-09-09 11:14 UTC (permalink / raw)
  To: Pavel Machek
  Cc: mpartap, tony, merlijn, kernel list, sre, nekit1000,
	tomi.valkeinen, jjhiblot, linux-omap, linux-arm-kernel
In-Reply-To: <20190907100726.GA12763@amd>

On Sat, Sep 07, 2019 at 12:07:27PM +0200, Pavel Machek wrote:
> Hi!
> 
> I don't see the LED-backlight driver in -next. Could it be pushed? It
> is one of last pieces to get working backlight on Motorola Droid 4...

Sorry, I dropped the ball on this and was therefore rather late
reviewing this patchset.

Assuming I have read the code correctly I think there are some
problems with the max_brightness handling in the backlight code.


Daniel.



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: dts: renesas: Add /soc dma-ranges
From: Geert Uytterhoeven @ 2019-09-09 11:18 UTC (permalink / raw)
  To: Marek Vasut
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Geert Uytterhoeven, Wolfram Sang, Linux-Renesas, Linux ARM,
	Marek Vasut
In-Reply-To: <20190907161634.27378-1-marek.vasut@gmail.com>

Hi Marek,

On Sat, Sep 7, 2019 at 6:16 PM <marek.vasut@gmail.com> wrote:
> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>
> Add dma-ranges property into /soc node to describe the DMA capabilities
> of the bus. This is currently needed to translate PCI DMA ranges, which
> are limited to 32bit addresses.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

> --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> @@ -330,6 +330,7 @@
>                 #address-cells = <2>;
>                 #size-cells = <2>;
>                 ranges;
> +               dma-ranges = <0 0x40000000 0 0x40000000 0 0xc0000000>;

Shouldn't the length be 0x80000000 (for all SoCs)?
Or should we allow DMA to internal System RAM, too?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V4 08/11] clk: imx: imx8qxp-lpcg: add parsing clocks from device tree
From: Dong Aisheng @ 2019-09-09 11:23 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Dong Aisheng, Michael Turquette, dl-linux-imx, Sascha Hauer,
	Fabio Estevam, Shawn Guo, linux-clk,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20190906171323.8847820640@mail.kernel.org>

]On Sat, Sep 7, 2019 at 5:35 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Dong Aisheng (2019-08-20 04:13:22)
> > Add parsing clocks from device tree.
>
> Please describe some more here.

Will improve.

> > +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +       base = devm_ioremap_resource(&pdev->dev, res);
> > +       if (IS_ERR(base))
> > +               return PTR_ERR(base);
> > +
> > +       count = of_property_count_u32_elems(np, "clock-indices");
> > +       if (count < 0) {
> > +               dev_err(&pdev->dev, "failed to count clocks\n");
> > +               return -EINVAL;
> > +       }
>
> Is 'count' expected to be equal to IMX_LPCG_MAX_CLKS? Because later on
> in this function we set the num of clks to the MAX instead of the count
> from clock-indices.
>

No. Here is a tricky to ease the clk getting.
For example, one LPCG supports up to 8 clock outputs which each of them
is fixed to 4 bits. Then we can easily use the bit-offset/clk-indices
parsed from DT
to fetch the corresponding clock by hws[clkspec->args[0] / 4].
And the cost is very limited with only a few pointers.

> > +
> > +       clk_data = devm_kzalloc(&pdev->dev, struct_size(clk_data, hws, IMX_LPCG_MAX_CLKS),
>
> This line is too long.
>

Will improve.

> > +                               GFP_KERNEL);
> > +       if (!clk_data)
> > +               return -ENOMEM;
> > +
> > +       clk_data->num = IMX_LPCG_MAX_CLKS;
> > +       clk_hws = clk_data->hws;
> > +
> > +       ret = of_property_read_u32_array(np, "clock-indices", bit_offset,
> > +                                        count);
> > +       if (ret < 0) {
> > +               dev_err(&pdev->dev, "failed to read clocks bit-offset\n");
>
> This isn't called bit-offset anymore.
>

Will improve.

> > +               return -EINVAL;
> > +       }
> > +
> > +       ret = of_clk_parent_fill(np, parent_names, count);
> > +       if (ret != count) {
> > +               dev_err(&pdev->dev, "failed to get clock parent names\n");
> > +               return -EINVAL;
>
> return count?
>

Okay

Regards
Aisheng

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ 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