Re: [PATCH] arm64: dts: hi3660: improve pmu description

2017-11-09 Thread YiPing Xu 



On 2017/11/8 23:55, Rob Herring wrote:

On Wed, Nov 8, 2017 at 2:59 AM, Xu YiPing  wrote:

cortex a73 pmu is supported, use it instead of armpmu-v3


The subject is misleading and you need a better commit message. Why is
this change needed? You are going from 1 to 2 devices.

Missing your S-o-B, too.


---
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi 
b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index 13ae69f..f638897 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -203,21 +203,25 @@
 IRQ_TYPE_LEVEL_HIGH)>;
};

-   pmu {
-   compatible = "arm,armv8-pmuv3";
+   pmu_a53 {


Don't use '_' in node names. Building with W=2 will tell you this.


OK, i'll modify it



+   compatible = "arm,cortex-a53-pmu";
interrupts = ,
 ,
 ,
-,
-,
-,
-,
-;
+;
interrupt-affinity = <&cpu0>,
 <&cpu1>,
 <&cpu2>,
-<&cpu3>,
-<&cpu4>,
+<&cpu3>;
+   };
+
+   pmu_a73 {
+   compatible = "arm,cortex-a73-pmu";
+   interrupts = ,
+,
+,
+;
+   interrupt-affinity = <&cpu4>,
 <&cpu5>,
 <&cpu6>,
 <&cpu7>;
--
2.7.4


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


.

Re: [PATCH] rtc: interface: set the next alarm event appropriately

2017-09-20 Thread YiPing Xu 



On 2017/9/20 17:16, Alexandre Belloni wrote:

Hi,

On 20/09/2017 at 11:22:31 +0800, Xu Yiping wrote:

From: Xu YiPing  

After commit 2b2f5ff00f63 ("rtc: interface: ignore expired timers when
enqueuing new timers"), the rtc_timer_enqueue will not reprogram the RTC
when there is any non-expired timers in the timerqueue. If we set a
RTC_TIMER between now and the next non-expired timers, it won't go into
effect in time.

So, besides ignoring the expired timers, we should take the next effect
timer into account, and reprogram the RTC timer appropriately.



Can you try this patch instead? I think it solves this issue:
http://patchwork.ozlabs.org/patch/792482/


   We've tested this patch, it works too.

   Will it be merged into the main tree?


Signed-off-by: Xu YiPing 
Signed-off-by: Chen Jun 
---
 drivers/rtc/interface.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index 8cec9a0..e237166 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -766,20 +766,23 @@ static int rtc_timer_enqueue(struct rtc_device *rtc, 
struct rtc_timer *timer)
struct timerqueue_node *next = timerqueue_getnext(&rtc->timerqueue);
struct rtc_time tm;
ktime_t now;
+   ktime_t next_effect = KTIME_MAX;

timer->enabled = 1;
__rtc_read_time(rtc, &tm);
now = rtc_tm_to_ktime(tm);

-   /* Skip over expired timers */
+   /* Skip over expired timers, get next effect timer */
while (next) {
-   if (next->expires >= now)
+   if (next->expires >= now) {
+   next_effect = next->expires;
break;
+   }
next = timerqueue_iterate_next(next);
}

timerqueue_add(&rtc->timerqueue, &timer->node);
-   if (!next) {
+   if (timer->node.expires < next_effect) {
struct rtc_wkalrm alarm;
int err;
alarm.time = rtc_ktime_to_tm(timer->node.expires);
--
2.7.4

Re: [PATCH] rtc: interface: set the next alarm event appropriately

2017-09-20 Thread YiPing Xu 

Hi,

On 2017/9/20 17:16, Alexandre Belloni wrote:

Hi,

On 20/09/2017 at 11:22:31 +0800, Xu Yiping wrote:

From: Xu YiPing  

After commit 2b2f5ff00f63 ("rtc: interface: ignore expired timers when
enqueuing new timers"), the rtc_timer_enqueue will not reprogram the RTC
when there is any non-expired timers in the timerqueue. If we set a
RTC_TIMER between now and the next non-expired timers, it won't go into
effect in time.

So, besides ignoring the expired timers, we should take the next effect
timer into account, and reprogram the RTC timer appropriately.



Can you try this patch instead? I think it solves this issue:
http://patchwork.ozlabs.org/patch/792482/


it looks work.

will it be merged into the main tree ?


Signed-off-by: Xu YiPing 
Signed-off-by: Chen Jun 
---
 drivers/rtc/interface.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index 8cec9a0..e237166 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -766,20 +766,23 @@ static int rtc_timer_enqueue(struct rtc_device *rtc, 
struct rtc_timer *timer)
struct timerqueue_node *next = timerqueue_getnext(&rtc->timerqueue);
struct rtc_time tm;
ktime_t now;
+   ktime_t next_effect = KTIME_MAX;

timer->enabled = 1;
__rtc_read_time(rtc, &tm);
now = rtc_tm_to_ktime(tm);

-   /* Skip over expired timers */
+   /* Skip over expired timers, get next effect timer */
while (next) {
-   if (next->expires >= now)
+   if (next->expires >= now) {
+   next_effect = next->expires;
break;
+   }
next = timerqueue_iterate_next(next);
}

timerqueue_add(&rtc->timerqueue, &timer->node);
-   if (!next) {
+   if (timer->node.expires < next_effect) {
struct rtc_wkalrm alarm;
int err;
alarm.time = rtc_ktime_to_tm(timer->node.expires);
--
2.7.4

[PATCH] of: remove redundant memset in overlay

2017-01-07 Thread YiPing Xu 
From: XuYing 

memset in of_build_overlay_info is redundant, the ovinfo has been
zeroed in of_fill_overlay_info when error.

Signed-off-by: YiPing Xu 
---
 drivers/of/overlay.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 0d4cda7..4b1b6b3 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -314,7 +314,6 @@ static int of_build_overlay_info(struct of_overlay *ov,
 
cnt = 0;
for_each_child_of_node(tree, node) {
-   memset(&ovinfo[cnt], 0, sizeof(*ovinfo));
err = of_fill_overlay_info(ov, node, &ovinfo[cnt]);
if (err == 0)
cnt++;
-- 
1.9.1

Re: [patch] staging: ion: use two separate locks for heaps and clients in ion_device

2016-10-07 Thread YiPing Xu 



On 2016/10/5 2:02, Laura Abbott wrote:

On 09/30/2016 01:18 AM, Xu YiPing wrote:

ion_alloc may get into slow path to get free page,
the call stack:

__alloc_pages_slowpath
ion_page_pool_alloc_pages
alloc_buffer_page
ion_system_heap_allocate
ion_buffer_create  <-- hold ion_device->lock
ion_alloc

after that, kernel invokes low-memory killer to kill some apps in
order to free memory in android system. However, sometimes, the
killing is blocked,
the app's call stack:

rwsem_down_write_failed
down_write
ion_client_destroy
ion_release
fput
do_signal

the killing is blocked because ion_device->lock is held by ion_alloc.

ion_alloc hold the lock for accessing the heaps list,
ion_destroy_client hold the lock for accessing the clients list.

so, we can use two separate locks for heaps and clients, to avoid the
unnecessary race.



I've reviewed this and it looks okay at first pass but I don't want it
applied just yet. Ion locking is a bit of a mess and has been added


yes, and now "debugfs_mutex" and "ion_root_client" is redundant, after 
commit 49d200deaa680501f19a247b1fffb29301e51d2b and 
9fd4dcece43a53e5a9e65a973df5693702ee6401.



once piece at a time. It needs a fundamental review. There will be Ion
discussions at plumbers at the end of October. Let's come back to this
after plumbers.


Signed-off-by: Xu YiPing 
---
 drivers/staging/android/ion/ion.c | 37
-
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index a2cf93b..331ad0f 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -46,7 +46,8 @@
  * @dev:the actual misc device
  * @buffers:an rb tree of all the existing buffers
  * @buffer_lock:lock protecting the tree of buffers
- * @lock:rwsem protecting the tree of heaps and clients
+ * @client_lock:rwsem protecting the tree of clients
+ * @heap_lock:rwsem protecting the tree of heaps
  * @heaps:list of all the heaps in the system
  * @user_clients:list of all the clients created from userspace
  */
@@ -54,7 +55,8 @@ struct ion_device {
 struct miscdevice dev;
 struct rb_root buffers;
 struct mutex buffer_lock;
-struct rw_semaphore lock;
+struct rw_semaphore client_lock;
+struct rw_semaphore heap_lock;
 struct plist_head heaps;
 long (*custom_ioctl)(struct ion_client *client, unsigned int cmd,
  unsigned long arg);
@@ -146,7 +148,7 @@ static inline void ion_buffer_page_clean(struct
page **page)
 *page = (struct page *)((unsigned long)(*page) & ~(1UL));
 }

-/* this function should only be called while dev->lock is held */
+/* this function should only be called while dev->heap_lock is held */
 static void ion_buffer_add(struct ion_device *dev,
struct ion_buffer *buffer)
 {
@@ -172,7 +174,7 @@ static void ion_buffer_add(struct ion_device *dev,
 rb_insert_color(&buffer->node, &dev->buffers);
 }

-/* this function should only be called while dev->lock is held */
+/* this function should only be called while dev->heap_lock is held */
 static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
  struct ion_device *dev,
  unsigned long len,
@@ -511,7 +513,7 @@ struct ion_handle *ion_alloc(struct ion_client
*client, size_t len,
 if (!len)
 return ERR_PTR(-EINVAL);

-down_read(&dev->lock);
+down_read(&dev->heap_lock);
 plist_for_each_entry(heap, &dev->heaps, node) {
 /* if the caller didn't specify this heap id */
 if (!((1 << heap->id) & heap_id_mask))
@@ -520,7 +522,7 @@ struct ion_handle *ion_alloc(struct ion_client
*client, size_t len,
 if (!IS_ERR(buffer))
 break;
 }
-up_read(&dev->lock);
+up_read(&dev->heap_lock);

 if (buffer == NULL)
 return ERR_PTR(-ENODEV);
@@ -713,7 +715,7 @@ static int is_client_alive(struct ion_client *client)
 node = ion_root_client->rb_node;
 dev = container_of(ion_root_client, struct ion_device, clients);

-down_read(&dev->lock);
+down_read(&dev->client_lock);
 while (node) {
 tmp = rb_entry(node, struct ion_client, node);
 if (client < tmp) {
@@ -721,12 +723,12 @@ static int is_client_alive(struct ion_client
*client)
 } else if (client > tmp) {
 node = node->rb_right;
 } else {
-up_read(&dev->lock);
+up_read(&dev->client_lock);
 return 1;
 }
 }

-up_read(&dev->lock);
+up_read(&dev->client_lock);
 return 0;
 }

@@ -841,12 +843,12 @@ struct ion_client *ion_client_create(struct
ion_device *dev,
 if (!client->name)
 goto err_free_client;

-down_write(&dev->lock);
+down_write(&dev->client_lock);
 client->display_serial = ion_get_client_serial(&dev->clients, name);
 client->display_name = kasprintf(
 GFP_KERNEL, "%s-%d"

Re: [PATCH v1 11/19] zsmalloc: squeeze freelist into page->mapping

2016-03-19 Thread YiPing Xu 



On 2016/3/15 14:51, Minchan Kim wrote:

On Tue, Mar 15, 2016 at 03:40:53PM +0900, Sergey Senozhatsky wrote:

On (03/11/16 16:30), Minchan Kim wrote:

-static void *location_to_obj(struct page *page, unsigned long obj_idx)
+static void objidx_to_page_and_ofs(struct size_class *class,
+   struct page *first_page,
+   unsigned long obj_idx,
+   struct page **obj_page,
+   unsigned long *ofs_in_page)


this looks big; 5 params, function "returning" both page and offset...
any chance to split it in two steps, perhaps?


Yes, it's rather ugly but I don't have a good idea.
Feel free to suggest if you have a better idea.

>


besides, it is more intuitive (at least to me) when 'offset'
shortened to 'offt', not 'ofs'.


	the purpose to get 'obj_page' and 'ofs_in_page' is to map the page and 
get the meta-data pointer in the page, so, we can finish this in a 
single function.


just like this, and maybe we could have a better function name

static unsigned long *map_handle(struct size_class *class,
struct page *first_page, unsigned long obj_idx)
{
struct page *cursor = first_page;
unsigned long offset = obj_idx * class->size;
int nr_page = offset >> PAGE_SHIFT;
unsigned long offset_in_page = offset & ~PAGE_MASK;
void *addr;
int i;

if (class->huge) {
VM_BUG_ON_PAGE(!is_first_page(page), page);
return &page_private(page);
}

for (i = 0; i < nr_page; i++)
cursor = get_next_page(cursor);

addr = kmap_atomic(cursor);

return addr + offset_in_page;
}

static void unmap_handle(unsigned long *addr)
{
if (class->huge) {
return;
}

kunmap_atomic(addr & ~PAGE_MASK);
}

	all functions called "objidx_to_page_and_ofs" could use it like this, 
for example:


static unsigned long handle_from_obj(struct size_class *class,
struct page *first_page, int obj_idx)
{
unsigned long *head = map_handle(class, first_page, obj_idx);

if (*head & OBJ_ALLOCATED_TAG)
handle = *head & ~OBJ_ALLOCATED_TAG;

unmap_handle(*head);

return handle;
}

'freeze_zspage', u'nfreeze_zspage' use it in the same way.

but in 'obj_malloc', we still have to get the page to get obj.

obj = location_to_obj(m_page, obj);



Indeed. I will change it to get_page_and_offset instead of
abbreviation if we cannot refactor it more.



-ss


  {
-   unsigned long obj;
+   int i;
+   unsigned long ofs;
+   struct page *cursor;
+   int nr_page;

-   if (!page) {
-   VM_BUG_ON(obj_idx);
-   return NULL;
-   }
+   ofs = obj_idx * class->size;
+   cursor = first_page;
+   nr_page = ofs >> PAGE_SHIFT;

-   obj = page_to_pfn(page) << OBJ_INDEX_BITS;
-   obj |= ((obj_idx) & OBJ_INDEX_MASK);
-   obj <<= OBJ_TAG_BITS;
+   *ofs_in_page = ofs & ~PAGE_MASK;
+
+   for (i = 0; i < nr_page; i++)
+   cursor = get_next_page(cursor);

-   return (void *)obj;
+   *obj_page = cursor;
  }


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: mailto:"d...@kvack.org";> em...@kvack.org 

.

[PATCH] zsmalloc: drop unused member 'mapping_area->huge'

2016-02-16 Thread YiPing Xu 
When unmapping a huge class page in zs_unmap_object, the page will
be unmapped by kmap_atomic. the "!area->huge" branch in
__zs_unmap_object is alway true, and no code set "area->huge" now,
so we can drop it.

Signed-off-by: YiPing Xu 
---
 mm/zsmalloc.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 2d7c4c1..43e4cbc 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -281,7 +281,6 @@ struct mapping_area {
 #endif
char *vm_addr; /* address of kmap_atomic()'ed pages */
enum zs_mapmode vm_mm; /* mapping mode */
-   bool huge;
 };
 
 static int create_handle_cache(struct zs_pool *pool)
@@ -1127,11 +1126,9 @@ static void __zs_unmap_object(struct mapping_area *area,
goto out;
 
buf = area->vm_buf;
-   if (!area->huge) {
-   buf = buf + ZS_HANDLE_SIZE;
-   size -= ZS_HANDLE_SIZE;
-   off += ZS_HANDLE_SIZE;
-   }
+   buf = buf + ZS_HANDLE_SIZE;
+   size -= ZS_HANDLE_SIZE;
+   off += ZS_HANDLE_SIZE;
 
sizes[0] = PAGE_SIZE - off;
sizes[1] = size - sizes[0];
-- 
1.8.3.2

Re: [PATCH v2 2/3] mailbox: Hi6220: add mailbox driver

2015-08-19 Thread YiPing Xu 

On 2015/8/20 10:53, Leo Yan wrote:

Add driver for Hi6220 mailbox, the mailbox communicates with MCU; for
sending data, it can support two methods for low level implementation:
one is to use interrupt as acknowledge, another is automatic mode which
without any acknowledge. These two methods have been supported in the
driver. For receiving data, it will depend on the interrupt to notify
the channel has incoming message; enhance rx channel's message queue,
which is based on the code in drivers/mailbox/omap-mailbox.c.

Now mailbox driver is used to send message to MCU to control dynamic
voltage and frequency scaling for CPU, GPU and DDR.

Signed-off-by: Leo Yan 
---
  drivers/mailbox/Kconfig  |   8 +
  drivers/mailbox/Makefile |   2 +
  drivers/mailbox/hi6220-mailbox.c | 519 +++
  3 files changed, 529 insertions(+)
  create mode 100644 drivers/mailbox/hi6220-mailbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index e269f08..21b71dd 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -70,4 +70,12 @@ config BCM2835_MBOX
  the services of the Videocore. Say Y here if you want to use the
  BCM2835 Mailbox.

+config HI6220_MBOX
+   tristate "Hi6220 Mailbox"
+   depends on ARCH_HISI
+   help
+ An implementation of the hi6220 mailbox. It is used to send message
+ between application processors and MCU. Say Y here if you want to 
build
+ the Hi6220 mailbox controller driver.
+
  endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 8e6d822..4ba9f5f 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -13,3 +13,5 @@ obj-$(CONFIG_PCC) += pcc.o
  obj-$(CONFIG_ALTERA_MBOX) += mailbox-altera.o

  obj-$(CONFIG_BCM2835_MBOX)+= bcm2835-mailbox.o
+
+obj-$(CONFIG_HI6220_MBOX)  += hi6220-mailbox.o
diff --git a/drivers/mailbox/hi6220-mailbox.c b/drivers/mailbox/hi6220-mailbox.c
new file mode 100644
index 000..8f63d0d
--- /dev/null
+++ b/drivers/mailbox/hi6220-mailbox.c
@@ -0,0 +1,519 @@
+/*
+ * Hisilicon's Hi6220 mailbox driver
+ *
+ * RX channel's message queue is based on the code written in
+ * drivers/mailbox/omap-mailbox.c.
+ *
+ * Copyright (c) 2015 Hisilicon Limited.
+ * Copyright (c) 2015 Linaro Limited.
+ *
+ * Author: Leo Yan 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define HI6220_MBOX_CHAN_MAX   32
+#define HI6220_MBOX_CHAN_NUM   2
+#define HI6220_MBOX_CHAN_SLOT_SIZE 64
+
+#define HI6220_MBOX_RX 0x0
+#define HI6220_MBOX_TX 0x1
+
+/* Mailbox message length: 32 bytes */
+#define HI6220_MBOX_MSG_LEN32
+
+/* Mailbox kfifo size */
+#define HI6220_MBOX_MSG_FIFO_SIZE  512
+
+/* Status & Mode Register */
+#define HI6220_MBOX_MODE_REG   0x0
+
+#define HI6220_MBOX_STATUS_MASK(0xF << 4)
+#define HI6220_MBOX_STATUS_IDLE(0x1 << 4)
+#define HI6220_MBOX_STATUS_TX  (0x2 << 4)
+#define HI6220_MBOX_STATUS_RX  (0x4 << 4)
+#define HI6220_MBOX_STATUS_ACK (0x8 << 4)
+#define HI6220_MBOX_ACK_CONFIG_MASK(0x1 << 0)
+#define HI6220_MBOX_ACK_AUTOMATIC  (0x1 << 0)
+#define HI6220_MBOX_ACK_IRQ(0x0 << 0)
+
+/* Data Registers */
+#define HI6220_MBOX_DATA_REG(i)(0x4 + (i << 2))
+
+/* ACPU Interrupt Register */
+#define HI6220_MBOX_ACPU_INT_RAW_REG   0x400
+#define HI6220_MBOX_ACPU_INT_MSK_REG   0x404
+#define HI6220_MBOX_ACPU_INT_STAT_REG  0x408
+#define HI6220_MBOX_ACPU_INT_CLR_REG   0x40c
+#define HI6220_MBOX_ACPU_INT_ENA_REG   0x500
+#define HI6220_MBOX_ACPU_INT_DIS_REG   0x504
+
+/* MCU Interrupt Register */
+#define HI6220_MBOX_MCU_INT_RAW_REG0x420
+
+/* Core Id */
+#define HI6220_CORE_ACPU   0x0
+#define HI6220_CORE_MCU0x2
+
+struct hi6220_mbox_queue {
+   struct kfifo fifo;
+   struct work_struct work;
+   struct mbox_chan *chan;
+   bool full;
+};
+
+struct hi6220_mbox_chan {
+
+   /*
+* Description for channel's hardware info:
+*  - direction;
+*  - peer core id for communication;
+*  - local irq vector or number;
+*  - remoted irq vector or number for peer core;
+*/
+   unsigned int dir;
+   unsigned int peer_core;
+   unsigned int remote_irq;
+   unsigned int local_irq;
+
+   /*
+* Slot addre

Re: [PATCH v2 4/6] clk: hi6220: Clock driver support for Hisilicon hi6220 SoC

2015-04-14 Thread YiPing Xu 

在 2015/4/13 23:34, Arnd Bergmann 写道:

On Monday 13 April 2015 21:57:46 Bintian wrote:

Hello Arnd,

Thanks for your code review.

On 2015/4/13 21:30, Arnd Bergmann wrote:

On Monday 13 April 2015 17:17:38 Bintian Wang wrote:

+#define HI6220_CFG_CSI2PHY 8
+#define HI6220_ISP_SCLK_GATE   9
+#define HI6220_ISP_SCLK_GATE1  10
+#define HI6220_ADE_CORE_GATE   11
+#define HI6220_CODEC_VPU_GATE  12
+#define HI6220_MED_SYSPLL  13
+
+/* mux clocks */
+#define HI6220_1440_1200   20
+#define HI6220_1000_1200   21
+#define HI6220_1000_1440   22
+
+/* divider clocks */
+#define HI6220_CODEC_JPEG  30
+#define HI6220_ISP_SCLK_SRC31
+#define HI6220_ISP_SCLK1   32



The numbers seem rather arbitrary, and you have both holes as well as duplicate
numbers here. I would suggest you do one of two things instead:



I just worry about some special clocks may be added later so keep some
holes for them;

The duplicate numbers means clocks belong to different system control
domains.


I don't understand. How would there be additional clocks added later?
Wouldn't that be a new chip?


  There are some clocks not used in linux system. e.g, some clocks are 
used in base band processor. So, some numbers are not defined here.



If you have separate system control domains, doesn't that mean that you
also have separate DT bindings?


a) have a separate header file per clock driver and make all the
 numbers unique and consecutive within that header

b) use the same numbers as the hardware registers so you can put the
 numbers directly into the dts and don't need a header to create
 an artificial ABI between the clock driver and the boot loader.

This header file will be used by device tree (I like using the clock
name instead "magic number" in dts :) )


That's not how it works though: The dts file is the place to define
the hardware numbers, we do that for all sorts of numbers: interrupts,
gpios, register ranges etc are all defined in dts to avoid putting
magic numbers in external header files.

There are some cases where it gets too ugly for clock controllers
that are highly irregular, but yours doesn't seem to be that kind.

E.g. all the fixed rate clocks should just be separate device nodes,
which lets you remove the binding for that node.


so how about keep them in one header file and let dts just include
one header file (not four files), but remove the holes?


The header files constantly cause problems with merge dependencies,
and we have some other companies that keep releasing new chips
that each time require a new header file. If hisilicon plans to make
more chips like this one, please consider coming up with more
generic bindings to avoid this.

Arnd

.




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 1/2] thermal: hisilicon: add new hisilicon thermal sensor driver

2015-03-27 Thread YiPing Xu 

在 2015/3/27 16:30, Xinwei Kong 写道:



On 2015/3/26 17:14, YiPing Xu wrote:

在 2015/3/25 15:50, Xinwei Kong 写道:

From: kongxinwei 

This patch adds the support for hisilicon thermal sensor, within
hisilicon SoC. there will register sensors for thermal framework
and use device tree to bind cooling device.

Signed-off-by: Leo Yan 
Signed-off-by: kongxinwei 
---
   drivers/thermal/Kconfig|   8 +
   drivers/thermal/Makefile   |   1 +
   drivers/thermal/hisi_thermal.c | 526 
+
   3 files changed, 535 insertions(+)
   create mode 100644 drivers/thermal/hisi_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index af40db0..81aee01 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -136,6 +136,14 @@ config THERMAL_EMULATION
 because userland can easily disable the thermal policy by simply
 flooding this sysfs node with low temperature values.

+config HISI_THERMAL
+tristate "Hisilicon thermal driver"
+depends on ARCH_HISI && CPU_THERMAL && OF
+help
+  Enable this to plug hisilicon's thermal sensor driver into the Linux
+  thermal framework. cpufreq is used as the cooling device to throttle
+  CPUs when the passive trip is crossed.
+
   config IMX_THERMAL
   tristate "Temperature sensor driver for Freescale i.MX SoCs"
   depends on CPU_THERMAL
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index fa0dc48..08ae7ac 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -39,3 +39,4 @@ obj-$(CONFIG_TI_SOC_THERMAL)+= ti-soc-thermal/
   obj-$(CONFIG_INT340X_THERMAL)  += int340x_thermal/
   obj-$(CONFIG_ST_THERMAL)+= st/
   obj-$(CONFIG_TEGRA_SOCTHERM)+= tegra_soctherm.o
+obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o
diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
new file mode 100644
index 000..8edf83a
--- /dev/null
+++ b/drivers/thermal/hisi_thermal.c


are you sure all the SOCs in hisilicon use the same thermal IP ?

if not, this driver should not use the "hisi_" prefix.


We can use this hisi thermal driver framework to satisfy diff thermal
IP, all of SoC will use this drvier by adding a struct about diff IP.
We may talk about hisi thermal diff IP how to satisfy this driver.

Thanks
Xinwei




@@ -0,0 +1,526 @@
+/*
+ * Hisilicon thermal sensor driver
+ *
+ * Copyright (c) 2014-2015 Hisilicon Limited.
+ * Copyright (c) 2014-2015 Linaro Limited.
+ *
+ * Xinwei Kong 
+ * Leo Yan 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "thermal_core.h"
+
+#define TEMP0_LAG(0x0)
+#define TEMP0_TH(0x4)
+#define TEMP0_RST_TH(0x8)
+#define TEMP0_CFG(0xC)
+#define TEMP0_EN(0x10)
+#define TEMP0_INT_EN(0x14)
+#define TEMP0_INT_CLR(0x18)
+#define TEMP0_RST_MSK(0x1C)
+#define TEMP0_RAW_INT(0x20)
+#define TEMP0_MSK_INT(0x24)
+#define TEMP0_VALUE(0x28)
+
+#define HISI_TEMP_BASE(-60)
+#define HISI_TEMP_RESET(10)
+#define HISI_TEMP_PASSIVE(85000)
+
+#define HISI_MAX_SENSORS4
+
+struct hisi_thermal_sensor {
+struct hisi_thermal_data *thermal;
+struct thermal_zone_device *tzd;
+const struct thermal_trip *trip;
+
+uint32_t id;
+uint32_t thres_temp, reset_temp;
+};
+
+struct hisi_thermal_data {
+struct platform_device *pdev;
+struct clk *clk;
+
+int irq, irq_bind_sensor;
+bool irq_enabled;
+
+unsigned int sensors_num;
+long sensor_temp[HISI_MAX_SENSORS];
+struct hisi_thermal_sensor sensors[HISI_MAX_SENSORS];
+
+void __iomem *regs;
+};
+
+static DEFINE_SPINLOCK(thermal_lock);
+


"thermal_lock" should be defined in "hisi_thermal_data" to support multi 
device instance.



ok,good comments


+/* in millicelsius */
+static inline int _step_to_temp(int step)
+{
+/*
+ * Every step equals (1 * 200) / 255 celsius, and finally
+ * need convert to millicelsius.
+ */
+return (HISI_TEMP_BASE + (step * 200 / 255)) * 1000;
+}
+
+static inline int _temp_to_step(int temp)
+{
+return ((temp / 1000 - HISI_TEMP_BASE) * 255 / 200);
+}
+
+static long hisi_thermal_get_sensor_temp(struct hisi_the

Re: [PATCH v1 1/2] thermal: hisilicon: add new hisilicon thermal sensor driver

2015-03-26 Thread YiPing Xu 

在 2015/3/25 15:50, Xinwei Kong 写道:

From: kongxinwei 

This patch adds the support for hisilicon thermal sensor, within
hisilicon SoC. there will register sensors for thermal framework
and use device tree to bind cooling device.

Signed-off-by: Leo Yan 
Signed-off-by: kongxinwei 
---
  drivers/thermal/Kconfig|   8 +
  drivers/thermal/Makefile   |   1 +
  drivers/thermal/hisi_thermal.c | 526 +
  3 files changed, 535 insertions(+)
  create mode 100644 drivers/thermal/hisi_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index af40db0..81aee01 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -136,6 +136,14 @@ config THERMAL_EMULATION
  because userland can easily disable the thermal policy by simply
  flooding this sysfs node with low temperature values.

+config HISI_THERMAL
+   tristate "Hisilicon thermal driver"
+   depends on ARCH_HISI && CPU_THERMAL && OF
+   help
+ Enable this to plug hisilicon's thermal sensor driver into the Linux
+ thermal framework. cpufreq is used as the cooling device to throttle
+ CPUs when the passive trip is crossed.
+
  config IMX_THERMAL
tristate "Temperature sensor driver for Freescale i.MX SoCs"
depends on CPU_THERMAL
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index fa0dc48..08ae7ac 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -39,3 +39,4 @@ obj-$(CONFIG_TI_SOC_THERMAL)  += ti-soc-thermal/
  obj-$(CONFIG_INT340X_THERMAL)  += int340x_thermal/
  obj-$(CONFIG_ST_THERMAL)  += st/
  obj-$(CONFIG_TEGRA_SOCTHERM)  += tegra_soctherm.o
+obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o
diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
new file mode 100644
index 000..8edf83a
--- /dev/null
+++ b/drivers/thermal/hisi_thermal.c


   are you sure all the SOCs in hisilicon use the same thermal IP ?

   if not, this driver should not use the "hisi_" prefix.


@@ -0,0 +1,526 @@
+/*
+ * Hisilicon thermal sensor driver
+ *
+ * Copyright (c) 2014-2015 Hisilicon Limited.
+ * Copyright (c) 2014-2015 Linaro Limited.
+ *
+ * Xinwei Kong 
+ * Leo Yan 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "thermal_core.h"
+
+#define TEMP0_LAG  (0x0)
+#define TEMP0_TH   (0x4)
+#define TEMP0_RST_TH   (0x8)
+#define TEMP0_CFG  (0xC)
+#define TEMP0_EN   (0x10)
+#define TEMP0_INT_EN   (0x14)
+#define TEMP0_INT_CLR  (0x18)
+#define TEMP0_RST_MSK  (0x1C)
+#define TEMP0_RAW_INT  (0x20)
+#define TEMP0_MSK_INT  (0x24)
+#define TEMP0_VALUE(0x28)
+
+#define HISI_TEMP_BASE (-60)
+#define HISI_TEMP_RESET(10)
+#define HISI_TEMP_PASSIVE  (85000)
+
+#define HISI_MAX_SENSORS   4
+
+struct hisi_thermal_sensor {
+   struct hisi_thermal_data *thermal;
+   struct thermal_zone_device *tzd;
+   const struct thermal_trip *trip;
+
+   uint32_t id;
+   uint32_t thres_temp, reset_temp;
+};
+
+struct hisi_thermal_data {
+   struct platform_device *pdev;
+   struct clk *clk;
+
+   int irq, irq_bind_sensor;
+   bool irq_enabled;
+
+   unsigned int sensors_num;
+   long sensor_temp[HISI_MAX_SENSORS];
+   struct hisi_thermal_sensor sensors[HISI_MAX_SENSORS];
+
+   void __iomem *regs;
+};
+
+static DEFINE_SPINLOCK(thermal_lock);
+


   "thermal_lock" should be defined in "hisi_thermal_data" to support 
multi device instance.



+/* in millicelsius */
+static inline int _step_to_temp(int step)
+{
+   /*
+* Every step equals (1 * 200) / 255 celsius, and finally
+* need convert to millicelsius.
+*/
+   return (HISI_TEMP_BASE + (step * 200 / 255)) * 1000;
+}
+
+static inline int _temp_to_step(int temp)
+{
+   return ((temp / 1000 - HISI_TEMP_BASE) * 255 / 200);
+}
+
+static long hisi_thermal_get_sensor_temp(struct hisi_thermal_data *data,
+struct hisi_thermal_sensor *sensor)
+{
+   unsigned long flags;
+   int val;
+
+   spin_lock_irqsave(&thermal_lock, flags);
+
+   /* disable module firstly */