[PATCH 1/1] mm: cma: check the max limit for cma allocation
CMA allocation request size is represented by size_t that
gets truncated when same is passed as int to
bitmap_find_next_zero_area_off.
We observe that during fuzz testing when cma allocation
request is too high, bitmap_find_next_zero_area_off still
returns success due to the truncation. This leads to
kernel crash, as subsequent code assumes that requested
memory is available.
Fail cma allocation in case the request breaches the
corresponding cma region size.
Signed-off-by: Shiraz Hashim <shas...@codeaurora.org>
---
mm/cma.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/mm/cma.c b/mm/cma.c
index 384c2cb..c960459 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -385,6 +385,9 @@ struct page *cma_alloc(struct cma *cma, size_t count,
unsigned int align)
bitmap_maxno = cma_bitmap_maxno(cma);
bitmap_count = cma_bitmap_pages_to_bits(cma, count);
+ if (bitmap_count > bitmap_maxno)
+ return NULL;
+
for (;;) {
mutex_lock(>lock);
bitmap_no = bitmap_find_next_zero_area_off(cma->bitmap,
--
Shiraz Hashim
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation
[PATCH 1/1] mm: cma: check the max limit for cma allocation
CMA allocation request size is represented by size_t that
gets truncated when same is passed as int to
bitmap_find_next_zero_area_off.
We observe that during fuzz testing when cma allocation
request is too high, bitmap_find_next_zero_area_off still
returns success due to the truncation. This leads to
kernel crash, as subsequent code assumes that requested
memory is available.
Fail cma allocation in case the request breaches the
corresponding cma region size.
Signed-off-by: Shiraz Hashim
---
mm/cma.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/mm/cma.c b/mm/cma.c
index 384c2cb..c960459 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -385,6 +385,9 @@ struct page *cma_alloc(struct cma *cma, size_t count,
unsigned int align)
bitmap_maxno = cma_bitmap_maxno(cma);
bitmap_count = cma_bitmap_pages_to_bits(cma, count);
+ if (bitmap_count > bitmap_maxno)
+ return NULL;
+
for (;;) {
mutex_lock(>lock);
bitmap_no = bitmap_find_next_zero_area_off(cma->bitmap,
--
Shiraz Hashim
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408!
On Thu, Jan 21, 2016 at 11:08 PM, Christoph Lameter wrote:
> Subject: vmstat: Queue work before clearing cpu_stat_off
>
> There is a race between vmstat_shepherd and quiet_vmstat() because
> the responsibility for checking for counter updates changes depending
> on the state of teh bit in cpu_stat_off. So queue the work before
> changing state of the bit in vmstat_shepherd. That way quiet_vmstat
> is guaranteed to remove the work request when clearing the bit and the
> bug in vmstat_update wont trigger anymore.
>
> Signed-off-by: Christoph Lameter
>
> Index: linux/mm/vmstat.c
> ===
> --- linux.orig/mm/vmstat.c
> +++ linux/mm/vmstat.c
> @@ -1480,12 +1480,14 @@ static void vmstat_shepherd(struct work_
> get_online_cpus();
> /* Check processors whose vmstat worker threads have been disabled */
> for_each_cpu(cpu, cpu_stat_off)
> - if (need_update(cpu) &&
> - cpumask_test_and_clear_cpu(cpu, cpu_stat_off))
> + if (need_update(cpu)) {
>
> queue_delayed_work_on(cpu, vmstat_wq,
> _cpu(vmstat_work, cpu), 0);
>
> + cpumask_clear_cpu(smp_processor_id(), cpu_stat_off);
> + }
> +
> put_online_cpus();
>
> schedule_delayed_work(,
This can alternatively lead to following where vmstat may not be
scheduled for cpu when it is back from idle.
CPU0:CPU1:
vmstat_shepherd
queue_delayed_work_on(CPU0)
quiet_vmstat
cancel_delayed_work
cpumask_test_and_set_cpu (0->1)
cpumask_clear_cpu(CPU0) (1->0)
--
regards
Shiraz Hashim
Re: mm, vmstat: kernel BUG at mm/vmstat.c:1408!
On Thu, Jan 21, 2016 at 11:08 PM, Christoph Lameter <c...@linux.com> wrote:
> Subject: vmstat: Queue work before clearing cpu_stat_off
>
> There is a race between vmstat_shepherd and quiet_vmstat() because
> the responsibility for checking for counter updates changes depending
> on the state of teh bit in cpu_stat_off. So queue the work before
> changing state of the bit in vmstat_shepherd. That way quiet_vmstat
> is guaranteed to remove the work request when clearing the bit and the
> bug in vmstat_update wont trigger anymore.
>
> Signed-off-by: Christoph Lameter <c...@linux.com>
>
> Index: linux/mm/vmstat.c
> ===
> --- linux.orig/mm/vmstat.c
> +++ linux/mm/vmstat.c
> @@ -1480,12 +1480,14 @@ static void vmstat_shepherd(struct work_
> get_online_cpus();
> /* Check processors whose vmstat worker threads have been disabled */
> for_each_cpu(cpu, cpu_stat_off)
> - if (need_update(cpu) &&
> - cpumask_test_and_clear_cpu(cpu, cpu_stat_off))
> + if (need_update(cpu)) {
>
> queue_delayed_work_on(cpu, vmstat_wq,
> _cpu(vmstat_work, cpu), 0);
>
> + cpumask_clear_cpu(smp_processor_id(), cpu_stat_off);
> + }
> +
> put_online_cpus();
>
> schedule_delayed_work(,
This can alternatively lead to following where vmstat may not be
scheduled for cpu when it is back from idle.
CPU0:CPU1:
vmstat_shepherd
queue_delayed_work_on(CPU0)
quiet_vmstat
cancel_delayed_work
cpumask_test_and_set_cpu (0->1)
cpumask_clear_cpu(CPU0) (1->0)
--
regards
Shiraz Hashim
Re: vmstat: make vmstat_updater deferrable again and shut down on idle
On Wed, Jan 20, 2016 at 8:42 PM, Christoph Lameter wrote: > On Wed, 20 Jan 2016, Shiraz Hashim wrote: > >> The patch makes vmstat_shepherd deferable which if is quiesed >> would not schedule vmstat update on other cpus. Wouldn't this >> aggravate the problem of vmstat for rest cpus not gettng updated. > > Its only "deferred" in order to make it at the next tick and not cause an > extra event. This means that vmstat will run periodically from tick > processing. It merely causes a synching so that we have one interruption > that does both. > > On idle we fold counters immediately. So there is no loss of accuracy. > vmstat is scheduled by shepherd or by itself (conditionally). In case shepherd is deferred and vmstat doesn't schedule itself, then vmstat needs to wait for shepherd to be up and then schedule it. This may end up in delayed status update for all live cpus. Isn't it ? -- regards Shiraz Hashim
Re: vmstat: make vmstat_updater deferrable again and shut down on idle
On Wed, Jan 20, 2016 at 8:42 PM, Christoph Lameter <c...@linux.com> wrote: > On Wed, 20 Jan 2016, Shiraz Hashim wrote: > >> The patch makes vmstat_shepherd deferable which if is quiesed >> would not schedule vmstat update on other cpus. Wouldn't this >> aggravate the problem of vmstat for rest cpus not gettng updated. > > Its only "deferred" in order to make it at the next tick and not cause an > extra event. This means that vmstat will run periodically from tick > processing. It merely causes a synching so that we have one interruption > that does both. > > On idle we fold counters immediately. So there is no loss of accuracy. > vmstat is scheduled by shepherd or by itself (conditionally). In case shepherd is deferred and vmstat doesn't schedule itself, then vmstat needs to wait for shepherd to be up and then schedule it. This may end up in delayed status update for all live cpus. Isn't it ? -- regards Shiraz Hashim
[PATCH] mm: pagewalk: call pte_hole() for VM_PFNMAP during walk_page_range
walk_page_range silently skips vma having VM_PFNMAP set,
which leads to undesirable behaviour at client end (who
called walk_page_range). For example for pagemap_read,
when no callbacks are called against VM_PFNMAP vma,
pagemap_read may prepare pagemap data for next virtual
address range at wrong index.
Signed-off-by: Shiraz Hashim
---
The fix is revised, based upon the suggestion here at
http://www.spinics.net/lists/linux-mm/msg83058.html
mm/pagewalk.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index ad83195..b264bda 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -199,7 +199,10 @@ int walk_page_range(unsigned long addr, unsigned long end,
*/
if ((vma->vm_start <= addr) &&
(vma->vm_flags & VM_PFNMAP)) {
- next = vma->vm_end;
+ if (walk->pte_hole)
+ err = walk->pte_hole(addr, next, walk);
+ if (err)
+ break;
pgd = pgd_offset(walk->mm, next);
continue;
}
--
Shiraz Hashim
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation
--
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/
[PATCH] mm: pagewalk: call pte_hole() for VM_PFNMAP during walk_page_range
walk_page_range silently skips vma having VM_PFNMAP set,
which leads to undesirable behaviour at client end (who
called walk_page_range). For example for pagemap_read,
when no callbacks are called against VM_PFNMAP vma,
pagemap_read may prepare pagemap data for next virtual
address range at wrong index.
Signed-off-by: Shiraz Hashim shas...@codeaurora.org
---
The fix is revised, based upon the suggestion here at
http://www.spinics.net/lists/linux-mm/msg83058.html
mm/pagewalk.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index ad83195..b264bda 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -199,7 +199,10 @@ int walk_page_range(unsigned long addr, unsigned long end,
*/
if ((vma-vm_start = addr)
(vma-vm_flags VM_PFNMAP)) {
- next = vma-vm_end;
+ if (walk-pte_hole)
+ err = walk-pte_hole(addr, next, walk);
+ if (err)
+ break;
pgd = pgd_offset(walk-mm, next);
continue;
}
--
Shiraz Hashim
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation
--
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 1/1] mm: pagemap: limit scan to virtual region being asked
On Wed, Jan 14, 2015 at 01:08:40AM +, Naoya Horiguchi wrote:
> On Tue, Jan 13, 2015 at 05:57:04PM +0530, Shiraz Hashim wrote:
> > pagemap_read scans through the virtual address space of a
> > task till it prepares 'count' pagemaps or it reaches end
> > of task.
> >
> > This presents a problem when the page walk doesn't happen
> > for vma with VM_PFNMAP set. In which case walk is silently
> > skipped and no pagemap is prepare, in turn making
> > pagemap_read to scan through task end, even crossing beyond
> > 'count', landing into a different vma region. This leads to
> > wrong presentation of mappings for that vma.
> >
> > Fix this by limiting end_vaddr to the end of the virtual
> > address region being scanned.
> >
> > Signed-off-by: Shiraz Hashim
>
> This patch works in some case, but there still seems a problem in
> another case.
>
> Consider that we have two vmas within some narrow
> (PAGEMAP_WALK_SIZE) region. One vma in lower address is VM_PFNMAP,
> and the other vma in higher address is not. Then a single call of
> walk_page_range() skips the first vma and scans the second vma, but
> the pagemap record of the second vma will be stored on the wrong
> offset in the buffer, because we just skip vma(VM_PFNMAP) without
> calling any callbacks (within which add_to_pagemap() increments
> pm.pos).
>
> So calling pte_hole() for vma(VM_PFNMAP) looks a better fix to me.
>
Thanks. That makes sense, If you are okay, I can send following patch formally.
diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index ad83195..b16ea60 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -200,6 +200,11 @@ int walk_page_range(unsigned long addr, unsigned long end,
if ((vma->vm_start <= addr) &&
(vma->vm_flags & VM_PFNMAP)) {
next = vma->vm_end;
+ if (walk->pte_hole)
+ err = walk->pte_hole(addr, next, walk);
+ if (err)
+ break;
+
pgd = pgd_offset(walk->mm, next);
continue;
}
regards
Shiraz
--
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 1/1] mm: pagemap: limit scan to virtual region being asked
On Wed, Jan 14, 2015 at 01:08:40AM +, Naoya Horiguchi wrote:
On Tue, Jan 13, 2015 at 05:57:04PM +0530, Shiraz Hashim wrote:
pagemap_read scans through the virtual address space of a
task till it prepares 'count' pagemaps or it reaches end
of task.
This presents a problem when the page walk doesn't happen
for vma with VM_PFNMAP set. In which case walk is silently
skipped and no pagemap is prepare, in turn making
pagemap_read to scan through task end, even crossing beyond
'count', landing into a different vma region. This leads to
wrong presentation of mappings for that vma.
Fix this by limiting end_vaddr to the end of the virtual
address region being scanned.
Signed-off-by: Shiraz Hashim shas...@codeaurora.org
This patch works in some case, but there still seems a problem in
another case.
Consider that we have two vmas within some narrow
(PAGEMAP_WALK_SIZE) region. One vma in lower address is VM_PFNMAP,
and the other vma in higher address is not. Then a single call of
walk_page_range() skips the first vma and scans the second vma, but
the pagemap record of the second vma will be stored on the wrong
offset in the buffer, because we just skip vma(VM_PFNMAP) without
calling any callbacks (within which add_to_pagemap() increments
pm.pos).
So calling pte_hole() for vma(VM_PFNMAP) looks a better fix to me.
Thanks. That makes sense, If you are okay, I can send following patch formally.
diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index ad83195..b16ea60 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -200,6 +200,11 @@ int walk_page_range(unsigned long addr, unsigned long end,
if ((vma-vm_start = addr)
(vma-vm_flags VM_PFNMAP)) {
next = vma-vm_end;
+ if (walk-pte_hole)
+ err = walk-pte_hole(addr, next, walk);
+ if (err)
+ break;
+
pgd = pgd_offset(walk-mm, next);
continue;
}
regards
Shiraz
--
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/
[PATCH 1/1] mm: pagemap: limit scan to virtual region being asked
pagemap_read scans through the virtual address space of a task till it prepares 'count' pagemaps or it reaches end of task. This presents a problem when the page walk doesn't happen for vma with VM_PFNMAP set. In which case walk is silently skipped and no pagemap is prepare, in turn making pagemap_read to scan through task end, even crossing beyond 'count', landing into a different vma region. This leads to wrong presentation of mappings for that vma. Fix this by limiting end_vaddr to the end of the virtual address region being scanned. Signed-off-by: Shiraz Hashim --- fs/proc/task_mmu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index 246eae8..04362e4 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -1270,7 +1270,9 @@ static ssize_t pagemap_read(struct file *file, char __user *buf, src = *ppos; svpfn = src / PM_ENTRY_BYTES; start_vaddr = svpfn << PAGE_SHIFT; - end_vaddr = TASK_SIZE_OF(task); + end_vaddr = start_vaddr + ((count / PM_ENTRY_BYTES) << PAGE_SHIFT); + if ((end_vaddr > TASK_SIZE_OF(task)) || (end_vaddr < start_vaddr)) + end_vaddr = TASK_SIZE_OF(task); /* watch out for wraparound */ if (svpfn > TASK_SIZE_OF(task) >> PAGE_SHIFT) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- 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/
[PATCH 1/1] mm: pagemap: limit scan to virtual region being asked
pagemap_read scans through the virtual address space of a task till it prepares 'count' pagemaps or it reaches end of task. This presents a problem when the page walk doesn't happen for vma with VM_PFNMAP set. In which case walk is silently skipped and no pagemap is prepare, in turn making pagemap_read to scan through task end, even crossing beyond 'count', landing into a different vma region. This leads to wrong presentation of mappings for that vma. Fix this by limiting end_vaddr to the end of the virtual address region being scanned. Signed-off-by: Shiraz Hashim shas...@codeaurora.org --- fs/proc/task_mmu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index 246eae8..04362e4 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -1270,7 +1270,9 @@ static ssize_t pagemap_read(struct file *file, char __user *buf, src = *ppos; svpfn = src / PM_ENTRY_BYTES; start_vaddr = svpfn PAGE_SHIFT; - end_vaddr = TASK_SIZE_OF(task); + end_vaddr = start_vaddr + ((count / PM_ENTRY_BYTES) PAGE_SHIFT); + if ((end_vaddr TASK_SIZE_OF(task)) || (end_vaddr start_vaddr)) + end_vaddr = TASK_SIZE_OF(task); /* watch out for wraparound */ if (svpfn TASK_SIZE_OF(task) PAGE_SHIFT) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- 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 2/2] pwm: mxs: Remove unused *dev from struct spear_pwm_chip
Hi Axel,
Perhaps you meant pwm: spear: (instead of pwm: mxs) in the subject
line.
On Mon, Apr 01, 2013 at 10:49:25AM +0800, Axel Lin wrote:
> Signed-off-by: Axel Lin
> ---
> drivers/pwm/pwm-spear.c |3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
> index 3223b57..9563599 100644
> --- a/drivers/pwm/pwm-spear.c
> +++ b/drivers/pwm/pwm-spear.c
> @@ -49,13 +49,11 @@
> * @mmio_base: base address of pwm chip
> * @clk: pointer to clk structure of pwm chip
> * @chip: linux pwm chip representation
> - * @dev: pointer to device structure of pwm chip
> */
> struct spear_pwm_chip {
> void __iomem *mmio_base;
> struct clk *clk;
> struct pwm_chip chip;
> - struct device *dev;
> };
>
> static inline struct spear_pwm_chip *to_spear_pwm_chip(struct pwm_chip *chip)
> @@ -200,7 +198,6 @@ static int spear_pwm_probe(struct platform_device *pdev)
> if (IS_ERR(pc->clk))
> return PTR_ERR(pc->clk);
>
> - pc->dev = >dev;
> platform_set_drvdata(pdev, pc);
>
> pc->chip.dev = >dev;
> --
> 1.7.10.4
otherwise,
Acked-by: Shiraz Hashim
--
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 v2] pwm: spear: Fix enable pwm output
On Sat, Mar 30, 2013 at 05:55:41PM +0800, Viresh Kumar wrote: > On 30 March 2013 11:07, Axel Lin wrote: > > The logic to check return value of clk_enable() is reversed, thus > > when clk_enable() passes PWMCR_PWM_ENABLE bit is not set. Fix it. > > > > Signed-off-by: Axel Lin > > --- > > drivers/pwm/pwm-spear.c |4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > I am so sorry. clk_prepare also has the same issue :( > Please fix that too and i will ack it ASAP. > > @Shiraz: Any idea how they went through? I don't remember how :(, surely a mistake which escaped tests also. -- regards Shiraz -- 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 v2] pwm: spear: Fix enable pwm output
On Sat, Mar 30, 2013 at 05:55:41PM +0800, Viresh Kumar wrote: On 30 March 2013 11:07, Axel Lin axel@ingics.com wrote: The logic to check return value of clk_enable() is reversed, thus when clk_enable() passes PWMCR_PWM_ENABLE bit is not set. Fix it. Signed-off-by: Axel Lin axel@ingics.com --- drivers/pwm/pwm-spear.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) I am so sorry. clk_prepare also has the same issue :( Please fix that too and i will ack it ASAP. @Shiraz: Any idea how they went through? I don't remember how :(, surely a mistake which escaped tests also. -- regards Shiraz -- 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 2/2] pwm: mxs: Remove unused *dev from struct spear_pwm_chip
Hi Axel,
Perhaps you meant pwm: spear: (instead of pwm: mxs) in the subject
line.
On Mon, Apr 01, 2013 at 10:49:25AM +0800, Axel Lin wrote:
Signed-off-by: Axel Lin axel@ingics.com
---
drivers/pwm/pwm-spear.c |3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
index 3223b57..9563599 100644
--- a/drivers/pwm/pwm-spear.c
+++ b/drivers/pwm/pwm-spear.c
@@ -49,13 +49,11 @@
* @mmio_base: base address of pwm chip
* @clk: pointer to clk structure of pwm chip
* @chip: linux pwm chip representation
- * @dev: pointer to device structure of pwm chip
*/
struct spear_pwm_chip {
void __iomem *mmio_base;
struct clk *clk;
struct pwm_chip chip;
- struct device *dev;
};
static inline struct spear_pwm_chip *to_spear_pwm_chip(struct pwm_chip *chip)
@@ -200,7 +198,6 @@ static int spear_pwm_probe(struct platform_device *pdev)
if (IS_ERR(pc-clk))
return PTR_ERR(pc-clk);
- pc-dev = pdev-dev;
platform_set_drvdata(pdev, pc);
pc-chip.dev = pdev-dev;
--
1.7.10.4
otherwise,
Acked-by: Shiraz Hashim shiraz.has...@st.com
--
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 1/3] SPEAr13xx_defconfig: Enable Memory split 2G/2G needed
Hi Nicolas, On Fri, Feb 08, 2013 at 12:49:22PM -0500, Nicolas Pitre wrote: > On Fri, 8 Feb 2013, Viresh Kumar wrote: > > > > By mistake you have added an ST internal list in cc, fixed it now. > > > > Subject should be: > > > > ARM: SPEAr13xx: Enable 2G/2G Memory split in defconfig > > > > On 8 February 2013 16:16, Vijay Kumar Mishra wrote: > > > Memory split 2G/2G is enabled as needed for SPEAr1310 RevC board to boot. > > > Before enabling this option the boot was hanging at uncompressing linux. > > I disagree. The subject could have been "put our head in the sand and > paper over bugs". > > Please don't do that. There is no reason for any machine not to boot > way past "Uncompressing Linux" with the default split. The likely > reason it works with the 2G:2G split is because in that case the virtual > and physical RAM addresses in the kernel are the same, and therefore > missing p2v or v2p conversions are invisible. I agree. Actually the details are misleading. SPEAr13xx chose this option just for its own preference over high memory support. @Vijay, I think you should drop this patch and just enable high memory support rather, if SPEAr13xx doesn't have any problem with that. -- regards Shiraz -- 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 1/3] SPEAr13xx_defconfig: Enable Memory split 2G/2G needed
Hi Nicolas, On Fri, Feb 08, 2013 at 12:49:22PM -0500, Nicolas Pitre wrote: On Fri, 8 Feb 2013, Viresh Kumar wrote: By mistake you have added an ST internal list in cc, fixed it now. Subject should be: ARM: SPEAr13xx: Enable 2G/2G Memory split in defconfig On 8 February 2013 16:16, Vijay Kumar Mishra vijay.ku...@st.com wrote: Memory split 2G/2G is enabled as needed for SPEAr1310 RevC board to boot. Before enabling this option the boot was hanging at uncompressing linux. I disagree. The subject could have been put our head in the sand and paper over bugs. Please don't do that. There is no reason for any machine not to boot way past Uncompressing Linux with the default split. The likely reason it works with the 2G:2G split is because in that case the virtual and physical RAM addresses in the kernel are the same, and therefore missing p2v or v2p conversions are invisible. I agree. Actually the details are misleading. SPEAr13xx chose this option just for its own preference over high memory support. @Vijay, I think you should drop this patch and just enable high memory support rather, if SPEAr13xx doesn't have any problem with that. -- regards Shiraz -- 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] gpio: stmpe: Add DT support for stmpe gpio
On Sat, Dec 1, 2012 at 10:05 PM, Linus Walleij wrote: > On Mon, Nov 26, 2012 at 12:32 PM, Shiraz Hashim wrote: >> On Mon, Nov 26, 2012 at 11:28:23AM +, Lee Jones wrote: >>> >>> Doesn't pinctrl normally handle this kind of stuff? >> >> Yes, but I think it is only for managing the SoC and its pads. > > No pinctrl is also for off-SoC pin controllers, you can have > pinctrl0, pinctrl1 ... pinctrlN on a system. The number space is > local per-pincontroller too. OK. Thanks for correcting. -- regards Shiraz Hashim -- 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] gpio: stmpe: Add DT support for stmpe gpio
On Sat, Dec 1, 2012 at 10:05 PM, Linus Walleij linus.wall...@linaro.org wrote: On Mon, Nov 26, 2012 at 12:32 PM, Shiraz Hashim shiraz.has...@st.com wrote: On Mon, Nov 26, 2012 at 11:28:23AM +, Lee Jones wrote: Doesn't pinctrl normally handle this kind of stuff? Yes, but I think it is only for managing the SoC and its pads. No pinctrl is also for off-SoC pin controllers, you can have pinctrl0, pinctrl1 ... pinctrlN on a system. The number space is local per-pincontroller too. OK. Thanks for correcting. -- regards Shiraz Hashim -- 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] gpio: stmpe: Add DT support for stmpe gpio
On Mon, Nov 26, 2012 at 11:28:23AM +, Lee Jones wrote: > On Fri, 23 Nov 2012, Shiraz Hashim wrote: > > > On Fri, Nov 23, 2012 at 12:14:13PM +, Lee Jones wrote: > > > > >> + if (np) > > > > >> + of_property_read_u32(np, "st,norequest-mask", > > > > >> + >norequest_mask); > > > > > > > > > > Can you explain to me what this does? > > > > > > > > You mean pdata->norequest_mask? It marks few gpios as unusable. > > > > Because these pads might be used by other blocks of stmpe. > > > > > > I'm not sure if that should be set with DT or not. > > > > > > Second opinion anyone? > > > > This is a board dependent parameter which just informs gpio driver > > about pins, which must not be requested. It may happen for a stmpe > > variant where such gpio pins are multiplexed with some other > > function. > > > > Hence it must be part of DT itself. > > Doesn't pinctrl normally handle this kind of stuff? Yes, but I think it is only for managing the SoC and its pads. -- regards Shiraz -- 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] gpio: stmpe: Add DT support for stmpe gpio
On Mon, Nov 26, 2012 at 11:28:23AM +, Lee Jones wrote: On Fri, 23 Nov 2012, Shiraz Hashim wrote: On Fri, Nov 23, 2012 at 12:14:13PM +, Lee Jones wrote: + if (np) + of_property_read_u32(np, st,norequest-mask, + pdata-norequest_mask); Can you explain to me what this does? You mean pdata-norequest_mask? It marks few gpios as unusable. Because these pads might be used by other blocks of stmpe. I'm not sure if that should be set with DT or not. Second opinion anyone? This is a board dependent parameter which just informs gpio driver about pins, which must not be requested. It may happen for a stmpe variant where such gpio pins are multiplexed with some other function. Hence it must be part of DT itself. Doesn't pinctrl normally handle this kind of stuff? Yes, but I think it is only for managing the SoC and its pads. -- regards Shiraz -- 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] gpio: stmpe: Add DT support for stmpe gpio
On Fri, Nov 23, 2012 at 12:14:13PM +, Lee Jones wrote: > > >> + if (np) > > >> + of_property_read_u32(np, "st,norequest-mask", > > >> + >norequest_mask); > > > > > > Can you explain to me what this does? > > > > You mean pdata->norequest_mask? It marks few gpios as unusable. > > Because these pads might be used by other blocks of stmpe. > > I'm not sure if that should be set with DT or not. > > Second opinion anyone? This is a board dependent parameter which just informs gpio driver about pins, which must not be requested. It may happen for a stmpe variant where such gpio pins are multiplexed with some other function. Hence it must be part of DT itself. -- regards Shiraz -- 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] gpio: stmpe: Add DT support for stmpe gpio
On Fri, Nov 23, 2012 at 12:14:13PM +, Lee Jones wrote: + if (np) + of_property_read_u32(np, st,norequest-mask, + pdata-norequest_mask); Can you explain to me what this does? You mean pdata-norequest_mask? It marks few gpios as unusable. Because these pads might be used by other blocks of stmpe. I'm not sure if that should be set with DT or not. Second opinion anyone? This is a board dependent parameter which just informs gpio driver about pins, which must not be requested. It may happen for a stmpe variant where such gpio pins are multiplexed with some other function. Hence it must be part of DT itself. -- regards Shiraz -- 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 V2 2/2] mfd: stmpe: Extend DT support in stmpe driver
On Thu, Nov 22, 2012 at 10:31:48PM +0530, Viresh Kumar wrote: > On 22 November 2012 21:16, Lee Jones wrote: > > The STMPE GPIO controller can't be used by Device Tree yet in > > any case, because it doesn't have an IRQ domain. This is > > compulsory, or it won't work. Have you tried to test this > > functionality yet? > > I don't have SPEAr board to test it anymore. I have moved out of > ST now and working in linaro as ARM asignee. Just pushing these > as an part time activity. > > Though ST guys would have tested stmpe, but stmpe-gpio, i am not > sure about. Let me bring some more information here. I totally understand Jones concerns, but the way stmpe (and may be other mfd devices) are handled is this that the parent block (i.e. stmpe) decides on the variants (say by probing device itself) and then prepares associated data for the (probed) variant and creates a platform device for the same. For the interrupts case also, it is stmpe which registers the irq domain. This is because, stmpe driver probes variant and populates its platform data and stmpe-gpio may not be aware of the variant it serves. At the same time, it (stmpe) needs few of the (virtual) interrupts for its internal purpose also. Hence stmpe passes irq_base to the stmpe-gpio driver while allocating and registering irq domain by itself. With this approach we have tested the functionality on SPEAr platform. -- regards Shiraz -- 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 V2 2/2] mfd: stmpe: Extend DT support in stmpe driver
On Thu, Nov 22, 2012 at 10:31:48PM +0530, Viresh Kumar wrote: On 22 November 2012 21:16, Lee Jones lee.jo...@linaro.org wrote: The STMPE GPIO controller can't be used by Device Tree yet in any case, because it doesn't have an IRQ domain. This is compulsory, or it won't work. Have you tried to test this functionality yet? I don't have SPEAr board to test it anymore. I have moved out of ST now and working in linaro as ARM asignee. Just pushing these as an part time activity. Though ST guys would have tested stmpe, but stmpe-gpio, i am not sure about. Let me bring some more information here. I totally understand Jones concerns, but the way stmpe (and may be other mfd devices) are handled is this that the parent block (i.e. stmpe) decides on the variants (say by probing device itself) and then prepares associated data for the (probed) variant and creates a platform device for the same. For the interrupts case also, it is stmpe which registers the irq domain. This is because, stmpe driver probes variant and populates its platform data and stmpe-gpio may not be aware of the variant it serves. At the same time, it (stmpe) needs few of the (virtual) interrupts for its internal purpose also. Hence stmpe passes irq_base to the stmpe-gpio driver while allocating and registering irq domain by itself. With this approach we have tested the functionality on SPEAr platform. -- regards Shiraz -- 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: possibility of parent clock selection through DT
Hi Mike, On Tue, Nov 13, 2012 at 2:52 AM, Mike Turquette wrote: > Quoting Shiraz Hashim (2012-11-06 22:36:10) >> On Wed, Nov 7, 2012 at 11:42 AM, Shiraz Hashim >> wrote: >> > Hi Mike, Rob, >> > >> > Devices in a SoC can have multiple possible clock sources which is >> > perfectly captured through clk framework. >> > >> > But the device itself may not be aware of the complex hierarchy above it. >> > In this case how do you suggest a board (through DT) should select its >> > preference. >> > >> > Is there some work already going on in this direction ? >> >> Just to make it clear, I already have referred the clock DT bindings and >> Shawn Guo patch on removing clk look up registration from kernel code. >> >> Here I am talking about possibility of selecting desired clock hierarchy >> by the boards about which device nodes are not aware. >> > > One way to achieve this is to use clk_set_rate as a way to switch > parents at run-time. The OMAP CCF code currently does this when > relocking PLLs and makes use of __clk_reparent to update the clock > framework's representation of the hierarchy dynamically. > > Maybe something like the following is helpful to you: > http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=arch/arm/mach-omap2/dpll3xxx.c;h=f72dedb4eee892ce4cd5bdf22cc8c22510f3d526;hb=clk-omap-3.8#l542 Thanks for the information. So the generic SoC part can be handled through set_rate but what about boards preferences and restrictions. For e.g., it prefers to route a clock from a particular clk source only and wants its own fixed (as per its design) clk source selections. -- regards Shiraz Hashim -- 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: possibility of parent clock selection through DT
Hi Mike, On Tue, Nov 13, 2012 at 2:52 AM, Mike Turquette mturque...@ti.com wrote: Quoting Shiraz Hashim (2012-11-06 22:36:10) On Wed, Nov 7, 2012 at 11:42 AM, Shiraz Hashim shiraz.linux.ker...@gmail.com wrote: Hi Mike, Rob, Devices in a SoC can have multiple possible clock sources which is perfectly captured through clk framework. But the device itself may not be aware of the complex hierarchy above it. In this case how do you suggest a board (through DT) should select its preference. Is there some work already going on in this direction ? Just to make it clear, I already have referred the clock DT bindings and Shawn Guo patch on removing clk look up registration from kernel code. Here I am talking about possibility of selecting desired clock hierarchy by the boards about which device nodes are not aware. One way to achieve this is to use clk_set_rate as a way to switch parents at run-time. The OMAP CCF code currently does this when relocking PLLs and makes use of __clk_reparent to update the clock framework's representation of the hierarchy dynamically. Maybe something like the following is helpful to you: http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=arch/arm/mach-omap2/dpll3xxx.c;h=f72dedb4eee892ce4cd5bdf22cc8c22510f3d526;hb=clk-omap-3.8#l542 Thanks for the information. So the generic SoC part can be handled through set_rate but what about boards preferences and restrictions. For e.g., it prefers to route a clock from a particular clk source only and wants its own fixed (as per its design) clk source selections. -- regards Shiraz Hashim -- 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] Set bit 22 in the PL310 (cache controller) AuxCtlr register
Hi Catalin, On Mon, Nov 12, 2012 at 10:56:41AM +, Will Deacon wrote: > On Mon, Nov 12, 2012 at 06:45:47AM +0000, Shiraz Hashim wrote: > > On Fri, Nov 09, 2012 at 09:54:01AM +, Will Deacon wrote: > > > On Fri, Nov 09, 2012 at 04:01:52AM +0000, Shiraz Hashim wrote: > > > > From: Catalin Marinas > > > > > > > > Clearing bit 22 in the PL310 Auxiliary Control register (shared > > > > attribute override enable) has the side effect of transforming Normal > > > > Shared Non-cacheable reads into Cacheable no-allocate reads. > > > > > > > > Coherent DMA buffers in Linux always have a Cacheable alias via the > > > > kernel linear mapping and the processor can speculatively load cache > > > > lines into the PL310 controller. With bit 22 cleared, Non-cacheable > > > > reads would unexpectedly hit such cache lines leading to buffer > > > > corruption. > > > > > > Is this still the case with recent kernels? I thought the dma-mapping/cma > > > work avoided the cacheable alias, but perhaps I'm mistaken. > > > > I haven't used CMA but DMA mappings are still normal memory > > non-cacheable. > > Ok, so trawling through the list reveals we only have this issue for normal > DMA mappings and not with CMA: > > > http://lists.infradead.org/pipermail/linux-arm-kernel/2012-October/124276.html > > I wonder whether we shouldn't just fix that, rather than work around it with > a PL310-specific hack? What do you say? -- regards Shiraz -- 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] Set bit 22 in the PL310 (cache controller) AuxCtlr register
Hi Catalin, On Mon, Nov 12, 2012 at 10:56:41AM +, Will Deacon wrote: On Mon, Nov 12, 2012 at 06:45:47AM +, Shiraz Hashim wrote: On Fri, Nov 09, 2012 at 09:54:01AM +, Will Deacon wrote: On Fri, Nov 09, 2012 at 04:01:52AM +, Shiraz Hashim wrote: From: Catalin Marinas catalin.mari...@arm.com Clearing bit 22 in the PL310 Auxiliary Control register (shared attribute override enable) has the side effect of transforming Normal Shared Non-cacheable reads into Cacheable no-allocate reads. Coherent DMA buffers in Linux always have a Cacheable alias via the kernel linear mapping and the processor can speculatively load cache lines into the PL310 controller. With bit 22 cleared, Non-cacheable reads would unexpectedly hit such cache lines leading to buffer corruption. Is this still the case with recent kernels? I thought the dma-mapping/cma work avoided the cacheable alias, but perhaps I'm mistaken. I haven't used CMA but DMA mappings are still normal memory non-cacheable. Ok, so trawling through the list reveals we only have this issue for normal DMA mappings and not with CMA: http://lists.infradead.org/pipermail/linux-arm-kernel/2012-October/124276.html I wonder whether we shouldn't just fix that, rather than work around it with a PL310-specific hack? What do you say? -- regards Shiraz -- 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] Set bit 22 in the PL310 (cache controller) AuxCtlr register
On Fri, Nov 09, 2012 at 09:54:01AM +, Will Deacon wrote: > On Fri, Nov 09, 2012 at 04:01:52AM +0000, Shiraz Hashim wrote: > > From: Catalin Marinas > > > > Clearing bit 22 in the PL310 Auxiliary Control register (shared > > attribute override enable) has the side effect of transforming Normal > > Shared Non-cacheable reads into Cacheable no-allocate reads. > > > > Coherent DMA buffers in Linux always have a Cacheable alias via the > > kernel linear mapping and the processor can speculatively load cache > > lines into the PL310 controller. With bit 22 cleared, Non-cacheable > > reads would unexpectedly hit such cache lines leading to buffer > > corruption. > > Is this still the case with recent kernels? I thought the dma-mapping/cma > work avoided the cacheable alias, but perhaps I'm mistaken. I haven't used CMA but DMA mappings are still normal memory non-cacheable. -- regards Shiraz -- 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] Set bit 22 in the PL310 (cache controller) AuxCtlr register
On Fri, Nov 09, 2012 at 09:54:01AM +, Will Deacon wrote: On Fri, Nov 09, 2012 at 04:01:52AM +, Shiraz Hashim wrote: From: Catalin Marinas catalin.mari...@arm.com Clearing bit 22 in the PL310 Auxiliary Control register (shared attribute override enable) has the side effect of transforming Normal Shared Non-cacheable reads into Cacheable no-allocate reads. Coherent DMA buffers in Linux always have a Cacheable alias via the kernel linear mapping and the processor can speculatively load cache lines into the PL310 controller. With bit 22 cleared, Non-cacheable reads would unexpectedly hit such cache lines leading to buffer corruption. Is this still the case with recent kernels? I thought the dma-mapping/cma work avoided the cacheable alias, but perhaps I'm mistaken. I haven't used CMA but DMA mappings are still normal memory non-cacheable. -- regards Shiraz -- 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/
[PATCH V5] PWM: Add SPEAr PWM chip driver support
Add support for PWM chips present on SPEAr platforms. These PWM
chips support 4 channel output with programmable duty cycle and
frequency.
More details on these PWM chips can be obtained from relevant
chapter of reference manual, present at following[1] location.
1. http://www.st.com/internet/mcu/product/251211.jsp
Cc: Thierry Reding
Signed-off-by: Shiraz Hashim
Signed-off-by: Viresh Kumar
Reviewed-by: Vipin Kumar
Acked-by: Viresh Kumar
---
Changes:-
V4 --> V5:
* replace tab by space in structure element declaration
* restructure probe to register pwm_chip at end when clk is prepared, and
all initializations done.
* move clk_enable/disable in probe under if block which checks "1340"
compatibility
* Replace (ret < 0) if condition by (!ret) at places where ret should be 0
on success
V3 --> V4:
* simplify remove
* maintain alphabetical order in Makefile
* donot check for device node in probe
* move few assignment lines in probe
V2 --> V3:
* remove "disabled" line from pwm dt binding documentation
* remove un-necessary check on pwm chip (for NULL) in remove.
V1 --> V2:
* make proper reference to pwm and pwm chip
* take care to capitalize PWM at appropriate places
* fix compatible string to the SoC where pwm chip was introduced
* Rename the documentation file to the name of driver
* Fix cosmetic changes like names, function name alignment, paragraph
formating, comments placement and formating, etc.
* Group and associate the bit field definitions to their registers
* Fix kerneldoc for structure definition
* Use chip to name pwm device and pwm for the channel instance
* Remove init section qualifiers
* Remove ifdefs around device tree from code and add dependency on CONFIG_OF
* prepare/unprepare clock once in probe/remove and just enable/disable
at rest of the places.
* Use _relaxed for readl/writel.
* Fix pwm disable part in remove
.../devicetree/bindings/pwm/spear-pwm.txt | 18 ++
drivers/pwm/Kconfig| 11 +
drivers/pwm/Makefile |1 +
drivers/pwm/pwm-spear.c| 276
4 files changed, 306 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/spear-pwm.txt
create mode 100644 drivers/pwm/pwm-spear.c
diff --git a/Documentation/devicetree/bindings/pwm/spear-pwm.txt
b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
new file mode 100644
index 000..3ac779d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
@@ -0,0 +1,18 @@
+== ST SPEAr SoC PWM controller ==
+
+Required properties:
+- compatible: should be one of:
+ - "st,spear320-pwm"
+ - "st,spear1340-pwm"
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
+ SPEAr. The first cell specifies the per-chip index of the PWM to use and
+ the second cell is the period in nanoseconds.
+
+Example:
+
+pwm: pwm@a800 {
+compatible ="st,spear320-pwm";
+reg = <0xa800 0x1000>;
+#pwm-cells = <2>;
+};
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index ed81720..6e556c7 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -112,6 +112,17 @@ config PWM_SAMSUNG
To compile this driver as a module, choose M here: the module
will be called pwm-samsung.
+config PWM_SPEAR
+ tristate "STMicroelectronics SPEAr PWM support"
+ depends on PLAT_SPEAR
+ depends on OF
+ help
+ Generic PWM framework driver for the PWM controller on ST
+ SPEAr SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-spear.
+
config PWM_TEGRA
tristate "NVIDIA Tegra PWM support"
depends on ARCH_TEGRA
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..3b3f4c9a 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_PWM_MXS) += pwm-mxs.o
obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
+obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
new file mode 100644
index 000..6a8fd9b
--- /dev/null
+++ b/drivers/pwm/pwm-spear.c
@@ -0,0 +1,276 @@
+/*
+ * ST Microelectronics SPEAr Pulse Width Modulator driver
+ *
+ * Copyright (C) 2012 ST Microelectronics
+ * Shiraz Hashim
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is li
Re: [PATCH V4] PWM: Add SPEAr PWM chip driver support
On Wed, Oct 24, 2012 at 07:51:37AM +0200, Thierry Reding wrote:
> On Mon, Oct 22, 2012 at 04:36:41PM +0530, Shiraz Hashim wrote:
> [...]
> > +struct spear_pwm_chip {
> > + void __iomem *mmio_base;
> > + struct clk *clk;
> > + struct pwm_chip chip;
>
> My editor shows a tab between pwm_chip and chip. This should really be a
> space.
>
> > + ret = pwmchip_add(>chip);
> > + if (ret < 0) {
> > + dev_err(>dev, "pwmchip_add() failed: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = clk_prepare_enable(pc->clk);
> > + if (ret < 0)
> > + return pwmchip_remove(>chip);
>
> I think in order to fix the potential race condition that Viresh
> mentioned we should move the clk_prepare_enable() before the
> pwmchip_add(), but don't forget to disable and unprepare the clock if
> pwmchip_add() fails.
>
> Actually, can't we make it a clk_prepare() only at this point and move
> the clk_enable() and clk_disable() into the if block below? In case the
> compatible value is not "st,spear1340-pwm" we don't need the clock
> enabled.
>
> > +
> > + if (of_device_is_compatible(np, "st,spear1340-pwm")) {
> > + /*
> > +* Following enables PWM chip, channels would still be
> > +* enabled individually through their control register
> > +*/
> > + val = readl_relaxed(pc->mmio_base + PWMMCR);
> > + val |= PWMMCR_PWM_ENABLE;
> > + writel_relaxed(val, pc->mmio_base + PWMMCR);
> > +
>
> Oh, and a spurious newline here... =)
>
> > + }
> > +
> > + /* only disable the clk and leave it prepared */
> > + clk_disable(pc->clk);
>
> This can go into the if block to match the clk_enable().
All suggestions would be included in V5. I hope this would be the
last one :).
--
regards
Shiraz
--
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 V4] PWM: Add SPEAr PWM chip driver support
On Wed, Oct 24, 2012 at 07:51:37AM +0200, Thierry Reding wrote:
On Mon, Oct 22, 2012 at 04:36:41PM +0530, Shiraz Hashim wrote:
[...]
+struct spear_pwm_chip {
+ void __iomem *mmio_base;
+ struct clk *clk;
+ struct pwm_chip chip;
My editor shows a tab between pwm_chip and chip. This should really be a
space.
+ ret = pwmchip_add(pc-chip);
+ if (ret 0) {
+ dev_err(pdev-dev, pwmchip_add() failed: %d\n, ret);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(pc-clk);
+ if (ret 0)
+ return pwmchip_remove(pc-chip);
I think in order to fix the potential race condition that Viresh
mentioned we should move the clk_prepare_enable() before the
pwmchip_add(), but don't forget to disable and unprepare the clock if
pwmchip_add() fails.
Actually, can't we make it a clk_prepare() only at this point and move
the clk_enable() and clk_disable() into the if block below? In case the
compatible value is not st,spear1340-pwm we don't need the clock
enabled.
+
+ if (of_device_is_compatible(np, st,spear1340-pwm)) {
+ /*
+* Following enables PWM chip, channels would still be
+* enabled individually through their control register
+*/
+ val = readl_relaxed(pc-mmio_base + PWMMCR);
+ val |= PWMMCR_PWM_ENABLE;
+ writel_relaxed(val, pc-mmio_base + PWMMCR);
+
Oh, and a spurious newline here... =)
+ }
+
+ /* only disable the clk and leave it prepared */
+ clk_disable(pc-clk);
This can go into the if block to match the clk_enable().
All suggestions would be included in V5. I hope this would be the
last one :).
--
regards
Shiraz
--
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/
[PATCH V5] PWM: Add SPEAr PWM chip driver support
Add support for PWM chips present on SPEAr platforms. These PWM
chips support 4 channel output with programmable duty cycle and
frequency.
More details on these PWM chips can be obtained from relevant
chapter of reference manual, present at following[1] location.
1. http://www.st.com/internet/mcu/product/251211.jsp
Cc: Thierry Reding thierry.red...@avionic-design.de
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
Reviewed-by: Vipin Kumar vipin.ku...@st.com
Acked-by: Viresh Kumar viresh.ku...@linaro.org
---
Changes:-
V4 -- V5:
* replace tab by space in structure element declaration
* restructure probe to register pwm_chip at end when clk is prepared, and
all initializations done.
* move clk_enable/disable in probe under if block which checks 1340
compatibility
* Replace (ret 0) if condition by (!ret) at places where ret should be 0
on success
V3 -- V4:
* simplify remove
* maintain alphabetical order in Makefile
* donot check for device node in probe
* move few assignment lines in probe
V2 -- V3:
* remove disabled line from pwm dt binding documentation
* remove un-necessary check on pwm chip (for NULL) in remove.
V1 -- V2:
* make proper reference to pwm and pwm chip
* take care to capitalize PWM at appropriate places
* fix compatible string to the SoC where pwm chip was introduced
* Rename the documentation file to the name of driver
* Fix cosmetic changes like names, function name alignment, paragraph
formating, comments placement and formating, etc.
* Group and associate the bit field definitions to their registers
* Fix kerneldoc for structure definition
* Use chip to name pwm device and pwm for the channel instance
* Remove init section qualifiers
* Remove ifdefs around device tree from code and add dependency on CONFIG_OF
* prepare/unprepare clock once in probe/remove and just enable/disable
at rest of the places.
* Use _relaxed for readl/writel.
* Fix pwm disable part in remove
.../devicetree/bindings/pwm/spear-pwm.txt | 18 ++
drivers/pwm/Kconfig| 11 +
drivers/pwm/Makefile |1 +
drivers/pwm/pwm-spear.c| 276
4 files changed, 306 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/spear-pwm.txt
create mode 100644 drivers/pwm/pwm-spear.c
diff --git a/Documentation/devicetree/bindings/pwm/spear-pwm.txt
b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
new file mode 100644
index 000..3ac779d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
@@ -0,0 +1,18 @@
+== ST SPEAr SoC PWM controller ==
+
+Required properties:
+- compatible: should be one of:
+ - st,spear320-pwm
+ - st,spear1340-pwm
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
+ SPEAr. The first cell specifies the per-chip index of the PWM to use and
+ the second cell is the period in nanoseconds.
+
+Example:
+
+pwm: pwm@a800 {
+compatible =st,spear320-pwm;
+reg = 0xa800 0x1000;
+#pwm-cells = 2;
+};
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index ed81720..6e556c7 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -112,6 +112,17 @@ config PWM_SAMSUNG
To compile this driver as a module, choose M here: the module
will be called pwm-samsung.
+config PWM_SPEAR
+ tristate STMicroelectronics SPEAr PWM support
+ depends on PLAT_SPEAR
+ depends on OF
+ help
+ Generic PWM framework driver for the PWM controller on ST
+ SPEAr SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-spear.
+
config PWM_TEGRA
tristate NVIDIA Tegra PWM support
depends on ARCH_TEGRA
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..3b3f4c9a 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_PWM_MXS) += pwm-mxs.o
obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
+obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
new file mode 100644
index 000..6a8fd9b
--- /dev/null
+++ b/drivers/pwm/pwm-spear.c
@@ -0,0 +1,276 @@
+/*
+ * ST Microelectronics SPEAr Pulse Width Modulator driver
+ *
+ * Copyright (C) 2012 ST Microelectronics
+ * Shiraz Hashim shiraz.has...@st.com
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2
[PATCH V4] PWM: Add SPEAr PWM chip driver support
Add support for PWM chips present on SPEAr platforms. These PWM
chips support 4 channel output with programmable duty cycle and
frequency.
More details on these PWM chips can be obtained from relevant
chapter of reference manual, present at following[1] location.
1. http://www.st.com/internet/mcu/product/251211.jsp
Cc: Thierry Reding
Signed-off-by: Shiraz Hashim
Signed-off-by: Viresh Kumar
Reviewed-by: Vipin Kumar
---
Changes:-
V3 --> V4:
* simplify remove
* maintain alphabetical order in Makefile
* donot check for device node in probe
* move few assignment lines in probe
V2 --> V3:
* remove "disabled" line from pwm dt binding documentation
* remove un-necessary check on pwm chip (for NULL) in remove.
V1 --> V2:
* make proper reference to pwm and pwm chip
* take care to capitalize PWM at appropriate places
* fix compatible string to the SoC where pwm chip was introduced
* Rename the documentation file to the name of driver
* Fix cosmetic changes like names, function name alignment, paragraph
formating, comments placement and formating, etc.
* Group and associate the bit field definitions to their registers
* Fix kerneldoc for structure definition
* Use chip to name pwm device and pwm for the channel instance
* Remove init section qualifiers
* Remove ifdefs around device tree from code and add dependency on CONFIG_OF
* prepare/unprepare clock once in probe/remove and just enable/disable
at rest of the places.
* Use _relaxed for readl/writel.
* Fix pwm disable part in remove
.../devicetree/bindings/pwm/spear-pwm.txt | 18 ++
drivers/pwm/Kconfig| 11 +
drivers/pwm/Makefile |1 +
drivers/pwm/pwm-spear.c| 273
4 files changed, 303 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/spear-pwm.txt
create mode 100644 drivers/pwm/pwm-spear.c
diff --git a/Documentation/devicetree/bindings/pwm/spear-pwm.txt
b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
new file mode 100644
index 000..3ac779d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
@@ -0,0 +1,18 @@
+== ST SPEAr SoC PWM controller ==
+
+Required properties:
+- compatible: should be one of:
+ - "st,spear320-pwm"
+ - "st,spear1340-pwm"
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
+ SPEAr. The first cell specifies the per-chip index of the PWM to use and
+ the second cell is the period in nanoseconds.
+
+Example:
+
+pwm: pwm@a800 {
+compatible ="st,spear320-pwm";
+reg = <0xa800 0x1000>;
+#pwm-cells = <2>;
+};
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index ed81720..6e556c7 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -112,6 +112,17 @@ config PWM_SAMSUNG
To compile this driver as a module, choose M here: the module
will be called pwm-samsung.
+config PWM_SPEAR
+ tristate "STMicroelectronics SPEAr PWM support"
+ depends on PLAT_SPEAR
+ depends on OF
+ help
+ Generic PWM framework driver for the PWM controller on ST
+ SPEAr SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-spear.
+
config PWM_TEGRA
tristate "NVIDIA Tegra PWM support"
depends on ARCH_TEGRA
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..3b3f4c9a 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_PWM_MXS) += pwm-mxs.o
obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
+obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
new file mode 100644
index 000..35124a01
--- /dev/null
+++ b/drivers/pwm/pwm-spear.c
@@ -0,0 +1,273 @@
+/*
+ * ST Microelectronics SPEAr Pulse Width Modulator driver
+ *
+ * Copyright (C) 2012 ST Microelectronics
+ * Shiraz Hashim
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define NUM_PWM4
+
+/* PWM registers and bits definitions */
+#define PWMCR 0x00/* Control Register */
+#define PWMCR_PWM_ENABL
Re: [PATCH V3] PWM: Add SPEAr PWM chip driver support
Hi Viresh,
On Mon, Oct 22, 2012 at 09:39:21AM +0530, viresh kumar wrote:
> Every time you read a code, you figure out new things about it.
> Sorry for these comments Now :(
No problem, it is important to fix now than catch them later.
> On Mon, Oct 22, 2012 at 9:21 AM, Shiraz Hashim wrote:
> > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> > index acfe482..6512786 100644
> > --- a/drivers/pwm/Makefile
> > +++ b/drivers/pwm/Makefile
> > @@ -9,6 +9,7 @@ obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
> > obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
> > obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
> > obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
> > +obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
>
> I have gone through this on every version of this patch, but couldn't figure
> out that you were trying to add it in alphabetical order, but you couldn't.
>
Would fix this.
> > diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
>
> > +static int spear_pwm_probe(struct platform_device *pdev)
> > +{
> > + pc = devm_kzalloc(>dev, sizeof(*pc), GFP_KERNEL);
> > + if (!pc) {
> > + dev_err(>dev, "failed to allocate memory\n");
> > + return -ENOMEM;
> > + }
> > +
> > + pc->dev = >dev;
>
> If you are going to send another version, then please move this
>
> > + pc->mmio_base = devm_request_and_ioremap(>dev, r);
> > + if (!pc->mmio_base)
> > + return -EADDRNOTAVAIL;
> > +
> > + platform_set_drvdata(pdev, pc);
>
> and this
>
> > + pc->clk = devm_clk_get(>dev, NULL);
> > + if (IS_ERR(pc->clk))
> > + return PTR_ERR(pc->clk);
>
> to this place :)
> So, that we don't do these for failure cases.
>
Okay.
> > + pc->chip.dev = >dev;
> > + pc->chip.ops = _pwm_ops;
> > + pc->chip.base = -1;
> > + pc->chip.npwm = NUM_PWM;
> > +
>
> > + if (np && of_device_is_compatible(np, "st,spear1340-pwm")) {
>
> I have noticed it earlier, but don't know why didn't i gave a comment here?
> you don't need to check for np here. It can't be NULL as you depend on
> CONFIG_OF.
Okay, would remove this.
> > + /*
> > +* Following enables PWM chip, channels would still be
> > +* enabled individually through their control register
> > +*/
> > + val = readl_relaxed(pc->mmio_base + PWMMCR);
> > + val |= PWMMCR_PWM_ENABLE;
> > + writel_relaxed(val, pc->mmio_base + PWMMCR);
> > +
> > + }
> > +
> > + /* only disable the clk and leave it prepared */
> > + clk_disable(pc->clk);
> > +
> > + return 0;
> > +}
> > +
> > +static int spear_pwm_remove(struct platform_device *pdev)
> > +{
> > + struct spear_pwm_chip *pc = platform_get_drvdata(pdev);
> > + int i;
> > +
> > + for (i = 0; i < NUM_PWM; i++) {
> > + struct pwm_device *pwm = >chip.pwms[i];
> > +
> > + if (test_bit(PWMF_ENABLED, >flags)) {
> > + spear_pwm_writel(pc, i, PWMCR, 0);
> > + clk_disable(pc->clk);
> > + }
> > + }
> > +
> > + /* clk was prepared in probe, hence unprepare it here */
> > + clk_unprepare(pc->clk);
>
> I believe you need to remove the chip first and then do above to
> avoid any race conditions, that might occur.
>
I am afraid, I would loose all chips and their related information
(PWMF_ENABLED) then.
> > + return pwmchip_remove(>chip);
> > +}
> > +
>
> After all this please add my:
> Acked-by: Viresh Kumar
I had already added your sob as you were the original author,
should I add a separate acked-by also ?
> Sorry Shiraz for so late comments, i can understand your
> frustration :(
No issues, review is higienic :)
--
regards
Shiraz
--
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 V3] PWM: Add SPEAr PWM chip driver support
Hi Viresh,
On Mon, Oct 22, 2012 at 09:39:21AM +0530, viresh kumar wrote:
Every time you read a code, you figure out new things about it.
Sorry for these comments Now :(
No problem, it is important to fix now than catch them later.
On Mon, Oct 22, 2012 at 9:21 AM, Shiraz Hashim shiraz.has...@st.com wrote:
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..6512786 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
+obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
I have gone through this on every version of this patch, but couldn't figure
out that you were trying to add it in alphabetical order, but you couldn't.
Would fix this.
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
+static int spear_pwm_probe(struct platform_device *pdev)
+{
+ pc = devm_kzalloc(pdev-dev, sizeof(*pc), GFP_KERNEL);
+ if (!pc) {
+ dev_err(pdev-dev, failed to allocate memory\n);
+ return -ENOMEM;
+ }
+
+ pc-dev = pdev-dev;
If you are going to send another version, then please move this
+ pc-mmio_base = devm_request_and_ioremap(pdev-dev, r);
+ if (!pc-mmio_base)
+ return -EADDRNOTAVAIL;
+
+ platform_set_drvdata(pdev, pc);
and this
+ pc-clk = devm_clk_get(pdev-dev, NULL);
+ if (IS_ERR(pc-clk))
+ return PTR_ERR(pc-clk);
to this place :)
So, that we don't do these for failure cases.
Okay.
+ pc-chip.dev = pdev-dev;
+ pc-chip.ops = spear_pwm_ops;
+ pc-chip.base = -1;
+ pc-chip.npwm = NUM_PWM;
+
+ if (np of_device_is_compatible(np, st,spear1340-pwm)) {
I have noticed it earlier, but don't know why didn't i gave a comment here?
you don't need to check for np here. It can't be NULL as you depend on
CONFIG_OF.
Okay, would remove this.
+ /*
+* Following enables PWM chip, channels would still be
+* enabled individually through their control register
+*/
+ val = readl_relaxed(pc-mmio_base + PWMMCR);
+ val |= PWMMCR_PWM_ENABLE;
+ writel_relaxed(val, pc-mmio_base + PWMMCR);
+
+ }
+
+ /* only disable the clk and leave it prepared */
+ clk_disable(pc-clk);
+
+ return 0;
+}
+
+static int spear_pwm_remove(struct platform_device *pdev)
+{
+ struct spear_pwm_chip *pc = platform_get_drvdata(pdev);
+ int i;
+
+ for (i = 0; i NUM_PWM; i++) {
+ struct pwm_device *pwm = pc-chip.pwms[i];
+
+ if (test_bit(PWMF_ENABLED, pwm-flags)) {
+ spear_pwm_writel(pc, i, PWMCR, 0);
+ clk_disable(pc-clk);
+ }
+ }
+
+ /* clk was prepared in probe, hence unprepare it here */
+ clk_unprepare(pc-clk);
I believe you need to remove the chip first and then do above to
avoid any race conditions, that might occur.
I am afraid, I would loose all chips and their related information
(PWMF_ENABLED) then.
+ return pwmchip_remove(pc-chip);
+}
+
After all this please add my:
Acked-by: Viresh Kumar viresh.ku...@linaro.org
I had already added your sob as you were the original author,
should I add a separate acked-by also ?
Sorry Shiraz for so late comments, i can understand your
frustration :(
No issues, review is higienic :)
--
regards
Shiraz
--
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/
[PATCH V4] PWM: Add SPEAr PWM chip driver support
Add support for PWM chips present on SPEAr platforms. These PWM
chips support 4 channel output with programmable duty cycle and
frequency.
More details on these PWM chips can be obtained from relevant
chapter of reference manual, present at following[1] location.
1. http://www.st.com/internet/mcu/product/251211.jsp
Cc: Thierry Reding thierry.red...@avionic-design.de
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
Reviewed-by: Vipin Kumar vipin.ku...@st.com
---
Changes:-
V3 -- V4:
* simplify remove
* maintain alphabetical order in Makefile
* donot check for device node in probe
* move few assignment lines in probe
V2 -- V3:
* remove disabled line from pwm dt binding documentation
* remove un-necessary check on pwm chip (for NULL) in remove.
V1 -- V2:
* make proper reference to pwm and pwm chip
* take care to capitalize PWM at appropriate places
* fix compatible string to the SoC where pwm chip was introduced
* Rename the documentation file to the name of driver
* Fix cosmetic changes like names, function name alignment, paragraph
formating, comments placement and formating, etc.
* Group and associate the bit field definitions to their registers
* Fix kerneldoc for structure definition
* Use chip to name pwm device and pwm for the channel instance
* Remove init section qualifiers
* Remove ifdefs around device tree from code and add dependency on CONFIG_OF
* prepare/unprepare clock once in probe/remove and just enable/disable
at rest of the places.
* Use _relaxed for readl/writel.
* Fix pwm disable part in remove
.../devicetree/bindings/pwm/spear-pwm.txt | 18 ++
drivers/pwm/Kconfig| 11 +
drivers/pwm/Makefile |1 +
drivers/pwm/pwm-spear.c| 273
4 files changed, 303 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/spear-pwm.txt
create mode 100644 drivers/pwm/pwm-spear.c
diff --git a/Documentation/devicetree/bindings/pwm/spear-pwm.txt
b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
new file mode 100644
index 000..3ac779d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
@@ -0,0 +1,18 @@
+== ST SPEAr SoC PWM controller ==
+
+Required properties:
+- compatible: should be one of:
+ - st,spear320-pwm
+ - st,spear1340-pwm
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
+ SPEAr. The first cell specifies the per-chip index of the PWM to use and
+ the second cell is the period in nanoseconds.
+
+Example:
+
+pwm: pwm@a800 {
+compatible =st,spear320-pwm;
+reg = 0xa800 0x1000;
+#pwm-cells = 2;
+};
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index ed81720..6e556c7 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -112,6 +112,17 @@ config PWM_SAMSUNG
To compile this driver as a module, choose M here: the module
will be called pwm-samsung.
+config PWM_SPEAR
+ tristate STMicroelectronics SPEAr PWM support
+ depends on PLAT_SPEAR
+ depends on OF
+ help
+ Generic PWM framework driver for the PWM controller on ST
+ SPEAr SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-spear.
+
config PWM_TEGRA
tristate NVIDIA Tegra PWM support
depends on ARCH_TEGRA
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..3b3f4c9a 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_PWM_MXS) += pwm-mxs.o
obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
+obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
new file mode 100644
index 000..35124a01
--- /dev/null
+++ b/drivers/pwm/pwm-spear.c
@@ -0,0 +1,273 @@
+/*
+ * ST Microelectronics SPEAr Pulse Width Modulator driver
+ *
+ * Copyright (C) 2012 ST Microelectronics
+ * Shiraz Hashim shiraz.has...@st.com
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed as is without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include linux/clk.h
+#include linux/err.h
+#include linux/io.h
+#include linux/ioport.h
+#include linux/kernel.h
+#include linux/math64.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include linux/pwm.h
+#include linux/slab.h
+#include linux/types.h
+
+#define NUM_PWM
[PATCH V3] PWM: Add SPEAr PWM chip driver support
Add support for PWM chips present on SPEAr platforms. These PWM
chips support 4 channel output with programmable duty cycle and
frequency.
More details on these PWM chips can be obtained from relevant
chapter of reference manual, present at following[1] location.
1. http://www.st.com/internet/mcu/product/251211.jsp
Cc: Thierry Reding
Signed-off-by: Shiraz Hashim
Signed-off-by: Viresh Kumar
Reviewed-by: Vipin Kumar
---
Changes:-
V2 --> V3:
* remove "disabled" line from pwm dt binding documentation
* remove un-necessary check on pwm chip (for NULL) in remove.
V1 --> V2:
* make proper reference to pwm and pwm chip
* take care to capitalize PWM at appropriate places
* fix compatible string to the SoC where pwm chip was introduced
* Rename the documentation file to the name of driver
* Fix cosmetic changes like names, function name alignment, paragraph
formating, comments placement and formating, etc.
* Group and associate the bit field definitions to their registers
* Fix kerneldoc for structure definition
* Use chip to name pwm device and pwm for the channel instance
* Remove init section qualifiers
* Remove ifdefs around device tree from code and add dependency on CONFIG_OF
* prepare/unprepare clock once in probe/remove and just enable/disable
at rest of the places.
* Use _relaxed for readl/writel.
* Fix pwm disable part in remove
.../devicetree/bindings/pwm/spear-pwm.txt | 18 ++
drivers/pwm/Kconfig| 11 +
drivers/pwm/Makefile |1 +
drivers/pwm/pwm-spear.c| 280
4 files changed, 310 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/spear-pwm.txt
create mode 100644 drivers/pwm/pwm-spear.c
diff --git a/Documentation/devicetree/bindings/pwm/spear-pwm.txt
b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
new file mode 100644
index 000..3ac779d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
@@ -0,0 +1,18 @@
+== ST SPEAr SoC PWM controller ==
+
+Required properties:
+- compatible: should be one of:
+ - "st,spear320-pwm"
+ - "st,spear1340-pwm"
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
+ SPEAr. The first cell specifies the per-chip index of the PWM to use and
+ the second cell is the period in nanoseconds.
+
+Example:
+
+pwm: pwm@a800 {
+compatible ="st,spear320-pwm";
+reg = <0xa800 0x1000>;
+#pwm-cells = <2>;
+};
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index ed81720..6e556c7 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -112,6 +112,17 @@ config PWM_SAMSUNG
To compile this driver as a module, choose M here: the module
will be called pwm-samsung.
+config PWM_SPEAR
+ tristate "STMicroelectronics SPEAr PWM support"
+ depends on PLAT_SPEAR
+ depends on OF
+ help
+ Generic PWM framework driver for the PWM controller on ST
+ SPEAr SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-spear.
+
config PWM_TEGRA
tristate "NVIDIA Tegra PWM support"
depends on ARCH_TEGRA
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..6512786 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
+obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TWL6030) += pwm-twl6030.o
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
new file mode 100644
index 000..bb63508
--- /dev/null
+++ b/drivers/pwm/pwm-spear.c
@@ -0,0 +1,280 @@
+/*
+ * ST Microelectronics SPEAr Pulse Width Modulator driver
+ *
+ * Copyright (C) 2012 ST Microelectronics
+ * Shiraz Hashim
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define NUM_PWM4
+
+/* PWM registers and bits definitions */
+#define PWMCR 0x00/* Control Register */
+#define PWMCR_PWM_ENABLE 0x1
+#define PWMCR_PRESCALE_SHIFT 2
+#define PWMCR_MIN_PRESCALE 0x00
+#define PWMCR_MAX_PRESCALE 0x3FFF
+
+#define PWMDCR 0
[PATCH V3] PWM: Add SPEAr PWM chip driver support
Add support for PWM chips present on SPEAr platforms. These PWM
chips support 4 channel output with programmable duty cycle and
frequency.
More details on these PWM chips can be obtained from relevant
chapter of reference manual, present at following[1] location.
1. http://www.st.com/internet/mcu/product/251211.jsp
Cc: Thierry Reding thierry.red...@avionic-design.de
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
Reviewed-by: Vipin Kumar vipin.ku...@st.com
---
Changes:-
V2 -- V3:
* remove disabled line from pwm dt binding documentation
* remove un-necessary check on pwm chip (for NULL) in remove.
V1 -- V2:
* make proper reference to pwm and pwm chip
* take care to capitalize PWM at appropriate places
* fix compatible string to the SoC where pwm chip was introduced
* Rename the documentation file to the name of driver
* Fix cosmetic changes like names, function name alignment, paragraph
formating, comments placement and formating, etc.
* Group and associate the bit field definitions to their registers
* Fix kerneldoc for structure definition
* Use chip to name pwm device and pwm for the channel instance
* Remove init section qualifiers
* Remove ifdefs around device tree from code and add dependency on CONFIG_OF
* prepare/unprepare clock once in probe/remove and just enable/disable
at rest of the places.
* Use _relaxed for readl/writel.
* Fix pwm disable part in remove
.../devicetree/bindings/pwm/spear-pwm.txt | 18 ++
drivers/pwm/Kconfig| 11 +
drivers/pwm/Makefile |1 +
drivers/pwm/pwm-spear.c| 280
4 files changed, 310 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/spear-pwm.txt
create mode 100644 drivers/pwm/pwm-spear.c
diff --git a/Documentation/devicetree/bindings/pwm/spear-pwm.txt
b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
new file mode 100644
index 000..3ac779d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
@@ -0,0 +1,18 @@
+== ST SPEAr SoC PWM controller ==
+
+Required properties:
+- compatible: should be one of:
+ - st,spear320-pwm
+ - st,spear1340-pwm
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
+ SPEAr. The first cell specifies the per-chip index of the PWM to use and
+ the second cell is the period in nanoseconds.
+
+Example:
+
+pwm: pwm@a800 {
+compatible =st,spear320-pwm;
+reg = 0xa800 0x1000;
+#pwm-cells = 2;
+};
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index ed81720..6e556c7 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -112,6 +112,17 @@ config PWM_SAMSUNG
To compile this driver as a module, choose M here: the module
will be called pwm-samsung.
+config PWM_SPEAR
+ tristate STMicroelectronics SPEAr PWM support
+ depends on PLAT_SPEAR
+ depends on OF
+ help
+ Generic PWM framework driver for the PWM controller on ST
+ SPEAr SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-spear.
+
config PWM_TEGRA
tristate NVIDIA Tegra PWM support
depends on ARCH_TEGRA
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..6512786 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
+obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TWL6030) += pwm-twl6030.o
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
new file mode 100644
index 000..bb63508
--- /dev/null
+++ b/drivers/pwm/pwm-spear.c
@@ -0,0 +1,280 @@
+/*
+ * ST Microelectronics SPEAr Pulse Width Modulator driver
+ *
+ * Copyright (C) 2012 ST Microelectronics
+ * Shiraz Hashim shiraz.has...@st.com
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed as is without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include linux/clk.h
+#include linux/err.h
+#include linux/io.h
+#include linux/ioport.h
+#include linux/kernel.h
+#include linux/math64.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include linux/pwm.h
+#include linux/slab.h
+#include linux/types.h
+
+#define NUM_PWM4
+
+/* PWM registers and bits definitions */
+#define PWMCR 0x00/* Control Register */
+#define PWMCR_PWM_ENABLE
Re: [PATCH V2] PWM: Add SPEAr PWM chip driver support
Hi Viresh,
On Fri, Oct 19, 2012 at 7:14 PM, Viresh Kumar wrote:
> On 19 October 2012 15:45, Shiraz Hashim wrote:
>> diff --git a/Documentation/devicetree/bindings/pwm/spear-pwm.txt
>> b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
>> +pwm: pwm@a800 {
>> +compatible ="st,spear320-pwm";
>> +reg = <0xa800 0x1000>;
>> +#pwm-cells = <2>;
>> +status = "disabled";
>
> Must remove disabled from here. Isn't it?
yes, would remove it.
>> diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#define NUM_PWM4
>> +
>> +/* PWM registers and bits definitions */
>> +#define PWMCR 0x00/* Control Register */
>> +#define PWMCR_PWM_ENABLE 0x1
>> +#define PWMCR_PRESCALE_SHIFT 2
>> +#define PWMCR_MIN_PRESCALE 0x00
>> +#define PWMCR_MAX_PRESCALE 0x3FFF
>
> I would do it as to make it more readable, your call:
>
> #define PWMCR 0x00/* Control Register */
> #define PWMCR_PWM_ENABLE 0x1
> #define PWMCR_PRESCALE_SHIFT 2
> #define PWMCR_MIN_PRESCALE 0x00
> #define PWMCR_MAX_PRESCALE 0x3FFF
There are some who don't like this (personally I see it quite clear),
so it becomes a matter of choice. I intentionaly prefixed each
bit definition by its register name to make association clear.
>
>> +static int spear_pwm_remove(struct platform_device *pdev)
>> +{
>> + struct spear_pwm_chip *pc = platform_get_drvdata(pdev);
>> + int i;
>> +
>> + if (WARN_ON(!pc))
>> + return -ENODEV;
>
> Sorry for not asking earlier, how can this be true anytime?
Probably never :), just copied from some other implementation.
Would remove this in V3.
--
regards
Shiraz Hashim
--
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/
[PATCH V2] PWM: Add SPEAr PWM chip driver support
Add support for PWM chips present on SPEAr platforms. These PWM
chips support 4 channel output with programmable duty cycle and
frequency.
More details on these PWM chips can be obtained from relevant
chapter of reference manual, present at following[1] location.
1. http://www.st.com/internet/mcu/product/251211.jsp
Cc: Thierry Reding
Signed-off-by: Shiraz Hashim
Signed-off-by: Viresh Kumar
Reviewed-by: Vipin Kumar
---
Changes V1 --> V2:
* make proper reference to pwm and pwm chip
* take care to capitalize PWM at appropriate places
* fix compatible string to the SoC where pwm chip was introduced
* Rename the documentation file to the name of driver
* Fix cosmetic changes like names, function name alignment, paragraph
formating, comments placement and formating, etc.
* Group and associate the bit field definitions to their registers
* Fix kerneldoc for structure definition
* Use chip to name pwm device and pwm for the channel instance
* Remove init section qualifiers
* Remove ifdefs around device tree from code and add dependency on CONFIG_OF
* prepare/unprepare clock once in probe/remove and just enable/disable
at rest of the places.
* Use _relaxed for readl/writel.
* Fix pwm disable part in remove
.../devicetree/bindings/pwm/spear-pwm.txt | 19 ++
drivers/pwm/Kconfig| 11 +
drivers/pwm/Makefile |1 +
drivers/pwm/pwm-spear.c| 283
4 files changed, 314 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/spear-pwm.txt
create mode 100644 drivers/pwm/pwm-spear.c
diff --git a/Documentation/devicetree/bindings/pwm/spear-pwm.txt
b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
new file mode 100644
index 000..768a4dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
@@ -0,0 +1,19 @@
+== ST SPEAr SoC PWM controller ==
+
+Required properties:
+- compatible: should be one of:
+ - "st,spear320-pwm"
+ - "st,spear1340-pwm"
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
+ SPEAr. The first cell specifies the per-chip index of the PWM to use and
+ the second cell is the period in nanoseconds.
+
+Example:
+
+pwm: pwm@a800 {
+compatible ="st,spear320-pwm";
+reg = <0xa800 0x1000>;
+#pwm-cells = <2>;
+status = "disabled";
+};
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index ed81720..6e556c7 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -112,6 +112,17 @@ config PWM_SAMSUNG
To compile this driver as a module, choose M here: the module
will be called pwm-samsung.
+config PWM_SPEAR
+ tristate "STMicroelectronics SPEAr PWM support"
+ depends on PLAT_SPEAR
+ depends on OF
+ help
+ Generic PWM framework driver for the PWM controller on ST
+ SPEAr SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-spear.
+
config PWM_TEGRA
tristate "NVIDIA Tegra PWM support"
depends on ARCH_TEGRA
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..6512786 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
+obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TWL6030) += pwm-twl6030.o
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
new file mode 100644
index 000..5128a62
--- /dev/null
+++ b/drivers/pwm/pwm-spear.c
@@ -0,0 +1,283 @@
+/*
+ * ST Microelectronics SPEAr Pulse Width Modulator driver
+ *
+ * Copyright (C) 2012 ST Microelectronics
+ * Shiraz Hashim
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define NUM_PWM4
+
+/* PWM registers and bits definitions */
+#define PWMCR 0x00/* Control Register */
+#define PWMCR_PWM_ENABLE 0x1
+#define PWMCR_PRESCALE_SHIFT 2
+#define PWMCR_MIN_PRESCALE 0x00
+#define PWMCR_MAX_PRESCALE 0x3FFF
+
+#define PWMDCR 0x04/* Duty Cycle Register */
+#define PWMDCR_MIN_DUTY0x0001
+#define PWMDCR_MAX_DUTY0x
+
+#d
Re: [PATCH] pwm: add spear pwm driver support
On Fri, Oct 19, 2012 at 11:29:43AM +0530, Shiraz HASHIM wrote: > On Thu, Oct 18, 2012 at 11:11:06PM +0530, viresh kumar wrote: > > On Thu, Oct 18, 2012 at 4:58 PM, Shiraz Hashim wrote: > > > + pc->mmio_base = devm_request_and_ioremap(>dev, r); > > > + if (!pc->mmio_base) > > > + return -EADDRNOTAVAIL; > > > > Just check, i believe there is a routine which will do above two in a single > > call.. > > > > I would cross check. > Couldn't find a suitable call. -- regards Shiraz -- 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] pwm: add spear pwm driver support
Hi Viresh,
On Fri, Oct 19, 2012 at 12:23:08PM +0530, viresh kumar wrote:
> On Fri, Oct 19, 2012 at 11:29 AM, Shiraz Hashim wrote:
> > On Thu, Oct 18, 2012 at 11:11:06PM +0530, viresh kumar wrote:
> >> On Thu, Oct 18, 2012 at 4:58 PM, Shiraz Hashim
> >> wrote:
>
> >> > +static int __devexit spear_pwm_remove(struct platform_device *pdev)
> >> > +{
> >> > + struct spear_pwm_chip *pc = platform_get_drvdata(pdev);
> >> > + int i;
> >> > +
> >> > + if (WARN_ON(!pc))
> >> > + return -ENODEV;
> >> > +
> >> > + for (i = 0; i < NUM_PWM; i++) {
> >> > + struct pwm_device *pwmd = >chip.pwms[i];
> >> > +
> >> > + if (!test_bit(PWMF_ENABLED, >flags))
>
> One point here: If i am not wrong you want to disable pwmd if it is enabled.
> Shouldn't you check for if (test_bit(PWMF_ENABLED, >flags)) instead?
>
You are right. The code is un-necessarily disabling all pwms while trying to
enable the clock for those which are not enabled also.
> >> > + if (clk_prepare_enable(pc->clk) < 0)
> >> > + continue;
> >> > +
> >> > + spear_pwm_writel(pc, i, PWMCR, 0);
> >> > + clk_disable_unprepare(pc->clk);
> >> > + }
> >>
> >> You are enabling/disabling clock N times here and each of these will
> >> write to an register. Do something better.
> >>
> >
> > I need to shut down all active pwms, how else would you suggest that ?
>
> Sorry, i misread the code on the second go :(
>
> I am proposing something like:
>
> static int __devexit spear_pwm_remove(struct platform_device *pdev)
> {
>struct spear_pwm_chip *pc = platform_get_drvdata(pdev);
>int i, clk_enabled = 0;
>
>if (WARN_ON(!pc))
>return -ENODEV;
>
>for (i = 0; i < NUM_PWM; i++) {
>struct pwm_device *pwmd = >chip.pwms[i];
>
>if (!test_bit(PWMF_ENABLED, >flags) && !clk_enabled)
>if (clk_prepare_enable(pc->clk) < 0)
>continue;
> else
> clk_enabled++;
>
>spear_pwm_writel(pc, i, PWMCR, 0);
>}
>
>if (clk_enabled)
> clk_disable_unprepare(pc->clk);
>...
> }
It may not be required as pwms which are not enabled do not have
their clocks enabled. Hence, perhaps we can do following,
8<---
static int spear_pwm_remove(struct platform_device *pdev)
{
struct spear_pwm_chip *pc = platform_get_drvdata(pdev);
int i;
if (WARN_ON(!pc))
return -ENODEV;
for (i = 0; i < NUM_PWM; i++) {
struct pwm_device *pwm = >chip.pwms[i];
if (test_bit(PWMF_ENABLED, >flags)) {
spear_pwm_writel(pc, i, PWMCR, 0);
clk_disable(pc->clk);
}
}
/* clk was prepared in probe, hence unprepare it here */
clk_unprepare(pc->clk);
return pwmchip_remove(>chip);
}
>8
--
regards
Shiraz
--
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] pwm: add spear pwm driver support
Hi Viresh,
On Thu, Oct 18, 2012 at 11:11:06PM +0530, viresh kumar wrote:
> On Thu, Oct 18, 2012 at 4:58 PM, Shiraz Hashim wrote:
> > diff --git a/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
> > b/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
> > +== ST SPEAr SoC PWM controller ==
> > +
> > +Required properties:
> > +- compatible: should be one of:
> > + - "st,spear-pwm"
> > + - "st,spear13xx-pwm"
>
> This has be matching with an version of the IP, as discussed earlier for many
> driver.
>
> Because ST hasn't released any specific version numbers, you must mention
> name of the SoC where the IP first appeared. That will mark its version. So,
> in short don't use generic names like spear or spear13xx :)
>
Okay. So I would rename compatible fields and accordingly as suggested
by Thierry, I would choose doc file name as 'spear-pwm.txt'.
> > +- reg: physical base address and length of the controller's registers
> > +- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
> > SPEAr. The
> > + first cell specifies the per-chip index of the PWM to use and the second
> > + cell is the duty cycle in nanoseconds.
> > +
> > +Example:
> > +
> > +pwm: pwm@a800 {
> > +compatible ="st,spear-pwm";
> > +reg = <0xa800 0x1000>;
> > +#pwm-cells = <2>;
> > +status = "disabled";
> > +};
>
> An example on how other nodes will link to it by passing id and duty cycle
> would be better.
>
I don't think it is required here, it is already covered in pwm.txt.
> > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> > index ed81720..3ff3e6f 100644
> > --- a/drivers/pwm/Kconfig
> > +++ b/drivers/pwm/Kconfig
> > @@ -112,6 +112,16 @@ config PWM_SAMSUNG
> > To compile this driver as a module, choose M here: the module
> > will be called pwm-samsung.
> >
> > +config PWM_SPEAR
> > + tristate "STMicroelectronics SPEAR PWM support"
>
> SPEAr
>
Yes, already pointed by Thierry.
> > + depends on PLAT_SPEAR
> > + help
> > + Generic PWM framework driver for the PWM controller on ST
> > + SPEAr SoCs.
> > +
> > + To compile this driver as a module, choose M here: the module
> > + will be called pwm-spear.
> > +
> > config PWM_TEGRA
> > tristate "NVIDIA Tegra PWM support"
> > depends on ARCH_TEGRA
> > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> > index acfe482..6512786 100644
> > --- a/drivers/pwm/Makefile
> > +++ b/drivers/pwm/Makefile
> > @@ -9,6 +9,7 @@ obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
> > obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
> > obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
> > obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
> > +obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
> > obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
> > obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
> > obj-$(CONFIG_PWM_TWL6030) += pwm-twl6030.o
> > diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
> > new file mode 100644
> > index 000..814395b
> > --- /dev/null
> > +++ b/drivers/pwm/pwm-spear.c
> > @@ -0,0 +1,287 @@
> > +/*
> > + * ST Microelectronics SPEAr Pulse Width Modulator driver
> > + *
> > + * Copyright (C) 2012 ST Microelectronics
> > + * Shiraz Hashim
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +/* PWM registers and bits definitions */
> > +#define PWMCR 0x00/* Control Register */
> > +#define PWMDCR 0x04/* Duty Cycle Register */
> > +#define PWMPCR 0x08/* Period Register */
> > +/* Following only available on 13xx SoCs */
> > +#define PWMMCR 0x3C/* Master Control Register */
> > +
> > +#define PWM_ENABLE 0x1
> > +
> > +#define MIN_PRESCALE 0x00
> > +#define MAX_PRESCALE 0x3FFF
> > +#define PRESCALE_SHIFT
Re: [PATCH] pwm: add spear pwm driver support
Hi Viresh,
On Thu, Oct 18, 2012 at 11:11:06PM +0530, viresh kumar wrote:
On Thu, Oct 18, 2012 at 4:58 PM, Shiraz Hashim shiraz.has...@st.com wrote:
diff --git a/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
b/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
+== ST SPEAr SoC PWM controller ==
+
+Required properties:
+- compatible: should be one of:
+ - st,spear-pwm
+ - st,spear13xx-pwm
This has be matching with an version of the IP, as discussed earlier for many
driver.
Because ST hasn't released any specific version numbers, you must mention
name of the SoC where the IP first appeared. That will mark its version. So,
in short don't use generic names like spear or spear13xx :)
Okay. So I would rename compatible fields and accordingly as suggested
by Thierry, I would choose doc file name as 'spear-pwm.txt'.
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
SPEAr. The
+ first cell specifies the per-chip index of the PWM to use and the second
+ cell is the duty cycle in nanoseconds.
+
+Example:
+
+pwm: pwm@a800 {
+compatible =st,spear-pwm;
+reg = 0xa800 0x1000;
+#pwm-cells = 2;
+status = disabled;
+};
An example on how other nodes will link to it by passing id and duty cycle
would be better.
I don't think it is required here, it is already covered in pwm.txt.
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index ed81720..3ff3e6f 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -112,6 +112,16 @@ config PWM_SAMSUNG
To compile this driver as a module, choose M here: the module
will be called pwm-samsung.
+config PWM_SPEAR
+ tristate STMicroelectronics SPEAR PWM support
SPEAr
Yes, already pointed by Thierry.
+ depends on PLAT_SPEAR
+ help
+ Generic PWM framework driver for the PWM controller on ST
+ SPEAr SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-spear.
+
config PWM_TEGRA
tristate NVIDIA Tegra PWM support
depends on ARCH_TEGRA
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..6512786 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
+obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TWL6030) += pwm-twl6030.o
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
new file mode 100644
index 000..814395b
--- /dev/null
+++ b/drivers/pwm/pwm-spear.c
@@ -0,0 +1,287 @@
+/*
+ * ST Microelectronics SPEAr Pulse Width Modulator driver
+ *
+ * Copyright (C) 2012 ST Microelectronics
+ * Shiraz Hashim shiraz.has...@st.com
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed as is without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include linux/clk.h
+#include linux/err.h
+#include linux/io.h
+#include linux/ioport.h
+#include linux/kernel.h
+#include linux/math64.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include linux/pwm.h
+#include linux/slab.h
+
+/* PWM registers and bits definitions */
+#define PWMCR 0x00/* Control Register */
+#define PWMDCR 0x04/* Duty Cycle Register */
+#define PWMPCR 0x08/* Period Register */
+/* Following only available on 13xx SoCs */
+#define PWMMCR 0x3C/* Master Control Register */
+
+#define PWM_ENABLE 0x1
+
+#define MIN_PRESCALE 0x00
+#define MAX_PRESCALE 0x3FFF
+#define PRESCALE_SHIFT 2
+
+#define MIN_DUTY 0x0001
+#define MAX_DUTY 0x
+
+#define MIN_PERIOD 0x0001
+#define MAX_PERIOD 0x
+
+#define NUM_PWM4
+
+/**
+ * struct pwm: struct representing pwm ip
spear_pwm_chip: struct representing pwm chip
The whole kernel doc requires a fix, would do that.
+ * mmio_base: base address of pwm
base would be enough.
mmio_base isn't too bad.
+ * clk: pointer to clk structure of pwm ip
+ * chip: linux pwm chip representation
+ * dev: pointer to device structure of pwm
+ */
+struct spear_pwm_chip {
+ void __iomem *mmio_base;
+ struct clk *clk;
+ struct pwm_chip chip
Re: [PATCH] pwm: add spear pwm driver support
Hi Viresh,
On Fri, Oct 19, 2012 at 12:23:08PM +0530, viresh kumar wrote:
On Fri, Oct 19, 2012 at 11:29 AM, Shiraz Hashim shiraz.has...@st.com wrote:
On Thu, Oct 18, 2012 at 11:11:06PM +0530, viresh kumar wrote:
On Thu, Oct 18, 2012 at 4:58 PM, Shiraz Hashim shiraz.has...@st.com
wrote:
+static int __devexit spear_pwm_remove(struct platform_device *pdev)
+{
+ struct spear_pwm_chip *pc = platform_get_drvdata(pdev);
+ int i;
+
+ if (WARN_ON(!pc))
+ return -ENODEV;
+
+ for (i = 0; i NUM_PWM; i++) {
+ struct pwm_device *pwmd = pc-chip.pwms[i];
+
+ if (!test_bit(PWMF_ENABLED, pwmd-flags))
One point here: If i am not wrong you want to disable pwmd if it is enabled.
Shouldn't you check for if (test_bit(PWMF_ENABLED, pwmd-flags)) instead?
You are right. The code is un-necessarily disabling all pwms while trying to
enable the clock for those which are not enabled also.
+ if (clk_prepare_enable(pc-clk) 0)
+ continue;
+
+ spear_pwm_writel(pc, i, PWMCR, 0);
+ clk_disable_unprepare(pc-clk);
+ }
You are enabling/disabling clock N times here and each of these will
write to an register. Do something better.
I need to shut down all active pwms, how else would you suggest that ?
Sorry, i misread the code on the second go :(
I am proposing something like:
static int __devexit spear_pwm_remove(struct platform_device *pdev)
{
struct spear_pwm_chip *pc = platform_get_drvdata(pdev);
int i, clk_enabled = 0;
if (WARN_ON(!pc))
return -ENODEV;
for (i = 0; i NUM_PWM; i++) {
struct pwm_device *pwmd = pc-chip.pwms[i];
if (!test_bit(PWMF_ENABLED, pwmd-flags) !clk_enabled)
if (clk_prepare_enable(pc-clk) 0)
continue;
else
clk_enabled++;
spear_pwm_writel(pc, i, PWMCR, 0);
}
if (clk_enabled)
clk_disable_unprepare(pc-clk);
...
}
It may not be required as pwms which are not enabled do not have
their clocks enabled. Hence, perhaps we can do following,
8---
static int spear_pwm_remove(struct platform_device *pdev)
{
struct spear_pwm_chip *pc = platform_get_drvdata(pdev);
int i;
if (WARN_ON(!pc))
return -ENODEV;
for (i = 0; i NUM_PWM; i++) {
struct pwm_device *pwm = pc-chip.pwms[i];
if (test_bit(PWMF_ENABLED, pwm-flags)) {
spear_pwm_writel(pc, i, PWMCR, 0);
clk_disable(pc-clk);
}
}
/* clk was prepared in probe, hence unprepare it here */
clk_unprepare(pc-clk);
return pwmchip_remove(pc-chip);
}
8
--
regards
Shiraz
--
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] pwm: add spear pwm driver support
On Fri, Oct 19, 2012 at 11:29:43AM +0530, Shiraz HASHIM wrote: On Thu, Oct 18, 2012 at 11:11:06PM +0530, viresh kumar wrote: On Thu, Oct 18, 2012 at 4:58 PM, Shiraz Hashim shiraz.has...@st.com wrote: + pc-mmio_base = devm_request_and_ioremap(pdev-dev, r); + if (!pc-mmio_base) + return -EADDRNOTAVAIL; Just check, i believe there is a routine which will do above two in a single call.. I would cross check. Couldn't find a suitable call. -- regards Shiraz -- 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/
[PATCH V2] PWM: Add SPEAr PWM chip driver support
Add support for PWM chips present on SPEAr platforms. These PWM
chips support 4 channel output with programmable duty cycle and
frequency.
More details on these PWM chips can be obtained from relevant
chapter of reference manual, present at following[1] location.
1. http://www.st.com/internet/mcu/product/251211.jsp
Cc: Thierry Reding thierry.red...@avionic-design.de
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
Reviewed-by: Vipin Kumar vipin.ku...@st.com
---
Changes V1 -- V2:
* make proper reference to pwm and pwm chip
* take care to capitalize PWM at appropriate places
* fix compatible string to the SoC where pwm chip was introduced
* Rename the documentation file to the name of driver
* Fix cosmetic changes like names, function name alignment, paragraph
formating, comments placement and formating, etc.
* Group and associate the bit field definitions to their registers
* Fix kerneldoc for structure definition
* Use chip to name pwm device and pwm for the channel instance
* Remove init section qualifiers
* Remove ifdefs around device tree from code and add dependency on CONFIG_OF
* prepare/unprepare clock once in probe/remove and just enable/disable
at rest of the places.
* Use _relaxed for readl/writel.
* Fix pwm disable part in remove
.../devicetree/bindings/pwm/spear-pwm.txt | 19 ++
drivers/pwm/Kconfig| 11 +
drivers/pwm/Makefile |1 +
drivers/pwm/pwm-spear.c| 283
4 files changed, 314 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/spear-pwm.txt
create mode 100644 drivers/pwm/pwm-spear.c
diff --git a/Documentation/devicetree/bindings/pwm/spear-pwm.txt
b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
new file mode 100644
index 000..768a4dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
@@ -0,0 +1,19 @@
+== ST SPEAr SoC PWM controller ==
+
+Required properties:
+- compatible: should be one of:
+ - st,spear320-pwm
+ - st,spear1340-pwm
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
+ SPEAr. The first cell specifies the per-chip index of the PWM to use and
+ the second cell is the period in nanoseconds.
+
+Example:
+
+pwm: pwm@a800 {
+compatible =st,spear320-pwm;
+reg = 0xa800 0x1000;
+#pwm-cells = 2;
+status = disabled;
+};
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index ed81720..6e556c7 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -112,6 +112,17 @@ config PWM_SAMSUNG
To compile this driver as a module, choose M here: the module
will be called pwm-samsung.
+config PWM_SPEAR
+ tristate STMicroelectronics SPEAr PWM support
+ depends on PLAT_SPEAR
+ depends on OF
+ help
+ Generic PWM framework driver for the PWM controller on ST
+ SPEAr SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-spear.
+
config PWM_TEGRA
tristate NVIDIA Tegra PWM support
depends on ARCH_TEGRA
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..6512786 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
+obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TWL6030) += pwm-twl6030.o
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
new file mode 100644
index 000..5128a62
--- /dev/null
+++ b/drivers/pwm/pwm-spear.c
@@ -0,0 +1,283 @@
+/*
+ * ST Microelectronics SPEAr Pulse Width Modulator driver
+ *
+ * Copyright (C) 2012 ST Microelectronics
+ * Shiraz Hashim shiraz.has...@st.com
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed as is without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include linux/clk.h
+#include linux/err.h
+#include linux/io.h
+#include linux/ioport.h
+#include linux/kernel.h
+#include linux/math64.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include linux/pwm.h
+#include linux/slab.h
+#include linux/types.h
+
+#define NUM_PWM4
+
+/* PWM registers and bits definitions */
+#define PWMCR 0x00/* Control Register */
+#define PWMCR_PWM_ENABLE 0x1
+#define PWMCR_PRESCALE_SHIFT 2
+#define PWMCR_MIN_PRESCALE 0x00
+#define PWMCR_MAX_PRESCALE 0x3FFF
Re: [PATCH V2] PWM: Add SPEAr PWM chip driver support
Hi Viresh,
On Fri, Oct 19, 2012 at 7:14 PM, Viresh Kumar viresh.ku...@linaro.org wrote:
On 19 October 2012 15:45, Shiraz Hashim shiraz.has...@st.com wrote:
diff --git a/Documentation/devicetree/bindings/pwm/spear-pwm.txt
b/Documentation/devicetree/bindings/pwm/spear-pwm.txt
+pwm: pwm@a800 {
+compatible =st,spear320-pwm;
+reg = 0xa800 0x1000;
+#pwm-cells = 2;
+status = disabled;
Must remove disabled from here. Isn't it?
yes, would remove it.
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
+#include linux/clk.h
+#include linux/err.h
+#include linux/io.h
+#include linux/ioport.h
+#include linux/kernel.h
+#include linux/math64.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include linux/pwm.h
+#include linux/slab.h
+#include linux/types.h
+
+#define NUM_PWM4
+
+/* PWM registers and bits definitions */
+#define PWMCR 0x00/* Control Register */
+#define PWMCR_PWM_ENABLE 0x1
+#define PWMCR_PRESCALE_SHIFT 2
+#define PWMCR_MIN_PRESCALE 0x00
+#define PWMCR_MAX_PRESCALE 0x3FFF
I would do it as to make it more readable, your call:
#define PWMCR 0x00/* Control Register */
#define PWMCR_PWM_ENABLE 0x1
#define PWMCR_PRESCALE_SHIFT 2
#define PWMCR_MIN_PRESCALE 0x00
#define PWMCR_MAX_PRESCALE 0x3FFF
There are some who don't like this (personally I see it quite clear),
so it becomes a matter of choice. I intentionaly prefixed each
bit definition by its register name to make association clear.
+static int spear_pwm_remove(struct platform_device *pdev)
+{
+ struct spear_pwm_chip *pc = platform_get_drvdata(pdev);
+ int i;
+
+ if (WARN_ON(!pc))
+ return -ENODEV;
Sorry for not asking earlier, how can this be true anytime?
Probably never :), just copied from some other implementation.
Would remove this in V3.
--
regards
Shiraz Hashim
--
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] pwm: add spear pwm driver support
Hi Thierry,
Thanks for the quick review.
On Thu, Oct 18, 2012 at 02:08:20PM +0200, Thierry Reding wrote:
> On Thu, Oct 18, 2012 at 04:58:32PM +0530, Shiraz Hashim wrote:
> > Add support for pwm devices present on SPEAr platforms. These pwm
> > devices support 4 channel output with programmable duty cycle and
> > frequency.
> >
> > More details on these pwm devices can be obtained from relevant chapter
> > of reference manual, present at following[1] location.
>
> Please make sure to spell PWM consistently. It should be in all
> uppercase in text. Lowercase should only be used in variable names or
> the patch subject prefix. See other commit messages for examples. The
> same applies to the rest of this patch, which seems to use a random mix
> of both.
>
> And maybe this shouldn't say "Add support for pwm devices" but rather
> "Add support for PWM chips..." and "These PWM chips support..."
I would do that.
> >
> > 1. http://www.st.com/internet/mcu/product/251211.jsp
> >
> > Cc: Thierry Reding
> > Signed-off-by: Shiraz Hashim
> > Signed-off-by: Viresh Kumar
> > Reviewed-by: Vipin Kumar
> > ---
> > .../devicetree/bindings/pwm/st-spear-pwm.txt | 19 ++
> > drivers/pwm/Kconfig| 10 +
> > drivers/pwm/Makefile |1 +
> > drivers/pwm/pwm-spear.c| 287
> >
> > 4 files changed, 317 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
> > create mode 100644 drivers/pwm/pwm-spear.c
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
> > b/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
> > new file mode 100644
> > index 000..b3dd1a0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
>
> I think this should either be "spear-pwm.txt" to mirror the driver name,
> or, to follow the Tegra example, "st,spear-pwm.txt" to mirror the
> compatible property.
Okay. I would rename it to 'st,spear-pwm.txt'.
> > @@ -0,0 +1,19 @@
> > +== ST SPEAr SoC PWM controller ==
> > +
> > +Required properties:
> > +- compatible: should be one of:
> > + - "st,spear-pwm"
> > + - "st,spear13xx-pwm"
> > +- reg: physical base address and length of the controller's registers
> > +- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
> > SPEAr. The
>
> I think you can break the "The" to the next line to make it fit within
> the 80 character limit.
Sure.
> > + first cell specifies the per-chip index of the PWM to use and the second
> > + cell is the duty cycle in nanoseconds.
>
> This should be "period in nanoseconds". I know this is wrong in the
> binding documentation for other drivers but I've recently committed a
> patch that fixes it.
Okay but I couldn't see use of pwm->period set by of_pwm_simple_xlate
anywhere. Am I missing something ?
>
> > +
> > +Example:
> > +
> > +pwm: pwm@a800 {
> > +compatible ="st,spear-pwm";
> > +reg = <0xa800 0x1000>;
> > +#pwm-cells = <2>;
> > +status = "disabled";
> > +};
> > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> > index ed81720..3ff3e6f 100644
> > --- a/drivers/pwm/Kconfig
> > +++ b/drivers/pwm/Kconfig
> > @@ -112,6 +112,16 @@ config PWM_SAMSUNG
> > To compile this driver as a module, choose M here: the module
> > will be called pwm-samsung.
> >
> > +config PWM_SPEAR
> > + tristate "STMicroelectronics SPEAR PWM support"
>
> You've spelled this "SPEAr" above and below, why not keep that spelling
> here as well?
>
Thanks for pointing out, would fix it.
> > + depends on PLAT_SPEAR
> > + help
> > + Generic PWM framework driver for the PWM controller on ST
> > + SPEAr SoCs.
> > +
> > + To compile this driver as a module, choose M here: the module
> > + will be called pwm-spear.
> > +
> > config PWM_TEGRA
> > tristate "NVIDIA Tegra PWM support"
> > depends on ARCH_TEGRA
> > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> > index acfe482..6512786 100644
> > --- a/drivers/pwm/Makefile
> > +++ b/drivers/pwm/Makefile
> > @@ -9,6 +9,7 @@ obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
> > obj-$(CONFIG_PWM_PXA)
[PATCH] pwm: add spear pwm driver support
Add support for pwm devices present on SPEAr platforms. These pwm
devices support 4 channel output with programmable duty cycle and
frequency.
More details on these pwm devices can be obtained from relevant chapter
of reference manual, present at following[1] location.
1. http://www.st.com/internet/mcu/product/251211.jsp
Cc: Thierry Reding
Signed-off-by: Shiraz Hashim
Signed-off-by: Viresh Kumar
Reviewed-by: Vipin Kumar
---
.../devicetree/bindings/pwm/st-spear-pwm.txt | 19 ++
drivers/pwm/Kconfig| 10 +
drivers/pwm/Makefile |1 +
drivers/pwm/pwm-spear.c| 287
4 files changed, 317 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
create mode 100644 drivers/pwm/pwm-spear.c
diff --git a/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
b/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
new file mode 100644
index 000..b3dd1a0
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
@@ -0,0 +1,19 @@
+== ST SPEAr SoC PWM controller ==
+
+Required properties:
+- compatible: should be one of:
+ - "st,spear-pwm"
+ - "st,spear13xx-pwm"
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
SPEAr. The
+ first cell specifies the per-chip index of the PWM to use and the second
+ cell is the duty cycle in nanoseconds.
+
+Example:
+
+pwm: pwm@a800 {
+compatible ="st,spear-pwm";
+reg = <0xa800 0x1000>;
+#pwm-cells = <2>;
+status = "disabled";
+};
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index ed81720..3ff3e6f 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -112,6 +112,16 @@ config PWM_SAMSUNG
To compile this driver as a module, choose M here: the module
will be called pwm-samsung.
+config PWM_SPEAR
+ tristate "STMicroelectronics SPEAR PWM support"
+ depends on PLAT_SPEAR
+ help
+ Generic PWM framework driver for the PWM controller on ST
+ SPEAr SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-spear.
+
config PWM_TEGRA
tristate "NVIDIA Tegra PWM support"
depends on ARCH_TEGRA
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..6512786 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
+obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TWL6030) += pwm-twl6030.o
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
new file mode 100644
index 000..814395b
--- /dev/null
+++ b/drivers/pwm/pwm-spear.c
@@ -0,0 +1,287 @@
+/*
+ * ST Microelectronics SPEAr Pulse Width Modulator driver
+ *
+ * Copyright (C) 2012 ST Microelectronics
+ * Shiraz Hashim
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* PWM registers and bits definitions */
+#define PWMCR 0x00/* Control Register */
+#define PWMDCR 0x04/* Duty Cycle Register */
+#define PWMPCR 0x08/* Period Register */
+/* Following only available on 13xx SoCs */
+#define PWMMCR 0x3C/* Master Control Register */
+
+#define PWM_ENABLE 0x1
+
+#define MIN_PRESCALE 0x00
+#define MAX_PRESCALE 0x3FFF
+#define PRESCALE_SHIFT 2
+
+#define MIN_DUTY 0x0001
+#define MAX_DUTY 0x
+
+#define MIN_PERIOD 0x0001
+#define MAX_PERIOD 0x
+
+#define NUM_PWM4
+
+/**
+ * struct pwm: struct representing pwm ip
+ *
+ * mmio_base: base address of pwm
+ * clk: pointer to clk structure of pwm ip
+ * chip: linux pwm chip representation
+ * dev: pointer to device structure of pwm
+ */
+struct spear_pwm_chip {
+ void __iomem *mmio_base;
+ struct clk *clk;
+ struct pwm_chip chip;
+ struct device *dev;
+};
+
+static inline struct spear_pwm_chip *to_spear_pwm_chip(struct pwm_chip *chip)
+{
+ return container_of(chip, struct spear_pwm_chip, chip);
+}
+
+static inline u32 spear_pwm_readl(struct spear_pwm_chip *chip, unsigned int
num,
+
[PATCH] pwm: add spear pwm driver support
Add support for pwm devices present on SPEAr platforms. These pwm
devices support 4 channel output with programmable duty cycle and
frequency.
More details on these pwm devices can be obtained from relevant chapter
of reference manual, present at following[1] location.
1. http://www.st.com/internet/mcu/product/251211.jsp
Cc: Thierry Reding thierry.red...@avionic-design.de
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
Reviewed-by: Vipin Kumar vipin.ku...@st.com
---
.../devicetree/bindings/pwm/st-spear-pwm.txt | 19 ++
drivers/pwm/Kconfig| 10 +
drivers/pwm/Makefile |1 +
drivers/pwm/pwm-spear.c| 287
4 files changed, 317 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
create mode 100644 drivers/pwm/pwm-spear.c
diff --git a/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
b/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
new file mode 100644
index 000..b3dd1a0
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
@@ -0,0 +1,19 @@
+== ST SPEAr SoC PWM controller ==
+
+Required properties:
+- compatible: should be one of:
+ - st,spear-pwm
+ - st,spear13xx-pwm
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
SPEAr. The
+ first cell specifies the per-chip index of the PWM to use and the second
+ cell is the duty cycle in nanoseconds.
+
+Example:
+
+pwm: pwm@a800 {
+compatible =st,spear-pwm;
+reg = 0xa800 0x1000;
+#pwm-cells = 2;
+status = disabled;
+};
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index ed81720..3ff3e6f 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -112,6 +112,16 @@ config PWM_SAMSUNG
To compile this driver as a module, choose M here: the module
will be called pwm-samsung.
+config PWM_SPEAR
+ tristate STMicroelectronics SPEAR PWM support
+ depends on PLAT_SPEAR
+ help
+ Generic PWM framework driver for the PWM controller on ST
+ SPEAr SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-spear.
+
config PWM_TEGRA
tristate NVIDIA Tegra PWM support
depends on ARCH_TEGRA
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..6512786 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
+obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TWL6030) += pwm-twl6030.o
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
new file mode 100644
index 000..814395b
--- /dev/null
+++ b/drivers/pwm/pwm-spear.c
@@ -0,0 +1,287 @@
+/*
+ * ST Microelectronics SPEAr Pulse Width Modulator driver
+ *
+ * Copyright (C) 2012 ST Microelectronics
+ * Shiraz Hashim shiraz.has...@st.com
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed as is without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include linux/clk.h
+#include linux/err.h
+#include linux/io.h
+#include linux/ioport.h
+#include linux/kernel.h
+#include linux/math64.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include linux/pwm.h
+#include linux/slab.h
+
+/* PWM registers and bits definitions */
+#define PWMCR 0x00/* Control Register */
+#define PWMDCR 0x04/* Duty Cycle Register */
+#define PWMPCR 0x08/* Period Register */
+/* Following only available on 13xx SoCs */
+#define PWMMCR 0x3C/* Master Control Register */
+
+#define PWM_ENABLE 0x1
+
+#define MIN_PRESCALE 0x00
+#define MAX_PRESCALE 0x3FFF
+#define PRESCALE_SHIFT 2
+
+#define MIN_DUTY 0x0001
+#define MAX_DUTY 0x
+
+#define MIN_PERIOD 0x0001
+#define MAX_PERIOD 0x
+
+#define NUM_PWM4
+
+/**
+ * struct pwm: struct representing pwm ip
+ *
+ * mmio_base: base address of pwm
+ * clk: pointer to clk structure of pwm ip
+ * chip: linux pwm chip representation
+ * dev: pointer to device structure of pwm
+ */
+struct spear_pwm_chip {
+ void __iomem *mmio_base;
+ struct clk *clk;
+ struct pwm_chip chip;
+ struct device *dev;
+};
+
+static inline struct spear_pwm_chip *to_spear_pwm_chip(struct pwm_chip *chip
Re: [PATCH] pwm: add spear pwm driver support
Hi Thierry,
Thanks for the quick review.
On Thu, Oct 18, 2012 at 02:08:20PM +0200, Thierry Reding wrote:
On Thu, Oct 18, 2012 at 04:58:32PM +0530, Shiraz Hashim wrote:
Add support for pwm devices present on SPEAr platforms. These pwm
devices support 4 channel output with programmable duty cycle and
frequency.
More details on these pwm devices can be obtained from relevant chapter
of reference manual, present at following[1] location.
Please make sure to spell PWM consistently. It should be in all
uppercase in text. Lowercase should only be used in variable names or
the patch subject prefix. See other commit messages for examples. The
same applies to the rest of this patch, which seems to use a random mix
of both.
And maybe this shouldn't say Add support for pwm devices but rather
Add support for PWM chips... and These PWM chips support...
I would do that.
1. http://www.st.com/internet/mcu/product/251211.jsp
Cc: Thierry Reding thierry.red...@avionic-design.de
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
Reviewed-by: Vipin Kumar vipin.ku...@st.com
---
.../devicetree/bindings/pwm/st-spear-pwm.txt | 19 ++
drivers/pwm/Kconfig| 10 +
drivers/pwm/Makefile |1 +
drivers/pwm/pwm-spear.c| 287
4 files changed, 317 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
create mode 100644 drivers/pwm/pwm-spear.c
diff --git a/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
b/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
new file mode 100644
index 000..b3dd1a0
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/st-spear-pwm.txt
I think this should either be spear-pwm.txt to mirror the driver name,
or, to follow the Tegra example, st,spear-pwm.txt to mirror the
compatible property.
Okay. I would rename it to 'st,spear-pwm.txt'.
@@ -0,0 +1,19 @@
+== ST SPEAr SoC PWM controller ==
+
+Required properties:
+- compatible: should be one of:
+ - st,spear-pwm
+ - st,spear13xx-pwm
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: number of cells used to specify PWM which is fixed to 2 on
SPEAr. The
I think you can break the The to the next line to make it fit within
the 80 character limit.
Sure.
+ first cell specifies the per-chip index of the PWM to use and the second
+ cell is the duty cycle in nanoseconds.
This should be period in nanoseconds. I know this is wrong in the
binding documentation for other drivers but I've recently committed a
patch that fixes it.
Okay but I couldn't see use of pwm-period set by of_pwm_simple_xlate
anywhere. Am I missing something ?
+
+Example:
+
+pwm: pwm@a800 {
+compatible =st,spear-pwm;
+reg = 0xa800 0x1000;
+#pwm-cells = 2;
+status = disabled;
+};
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index ed81720..3ff3e6f 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -112,6 +112,16 @@ config PWM_SAMSUNG
To compile this driver as a module, choose M here: the module
will be called pwm-samsung.
+config PWM_SPEAR
+ tristate STMicroelectronics SPEAR PWM support
You've spelled this SPEAr above and below, why not keep that spelling
here as well?
Thanks for pointing out, would fix it.
+ depends on PLAT_SPEAR
+ help
+ Generic PWM framework driver for the PWM controller on ST
+ SPEAr SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-spear.
+
config PWM_TEGRA
tristate NVIDIA Tegra PWM support
depends on ARCH_TEGRA
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index acfe482..6512786 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
+obj-$(CONFIG_PWM_SPEAR)+= pwm-spear.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TWL6030) += pwm-twl6030.o
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
new file mode 100644
index 000..814395b
--- /dev/null
+++ b/drivers/pwm/pwm-spear.c
@@ -0,0 +1,287 @@
+/*
+ * ST Microelectronics SPEAr Pulse Width Modulator driver
+ *
+ * Copyright (C) 2012 ST Microelectronics
+ * Shiraz Hashim shiraz.has...@st.com
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed as is without any
+ * warranty
Re: [PATCH v6 1/3] spi/pl022: Add chip select handling via GPIO
Hi Linus, On Sun, Sep 2, 2012 at 12:48 PM, Linus Walleij wrote: > On Sat, Sep 1, 2012 at 1:14 PM, shiraz hashim > wrote: >> Hi Roland, >> >> On Wed, Aug 22, 2012 at 7:19 PM, Roland Stigge wrote: >>> @@ -2016,6 +2030,8 @@ pl022_probe(struct amba_device *adev, co >>> pl022->master_info = platform_info; >>> pl022->adev = adev; >>> pl022->vendor = id->data; >>> + /* Point chipselects to allocated memory beyond the main struct */ >>> + pl022->chipselects = (int *) pl022 + sizeof(struct pl022); >> >> This is going beyond memory allocated for chipselects >> as it adds 4 * sizeof(struct pl022) bytes to pl022. > > Yes that is why the allocation looks like this: > > + master = spi_alloc_master(dev, sizeof(struct pl022) + sizeof(int) * > + platform_info->num_chipselect); > The allocation is such because type of chipselects is int. The statement for allocation is correct, but pl022->chipselects = (int *) pl022 + sizeof(struct pl022); is not adding sizeof(struct pl022) bytes to pl022 base (which we want), but infact 4 times the size of pl022 (because type of pl022 is now int *). Do you get my point ? This would go way beyond memory allocated for chipselects. -- regards Shiraz Hashim -- 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 v6 1/3] spi/pl022: Add chip select handling via GPIO
Hi Linus, On Sun, Sep 2, 2012 at 12:48 PM, Linus Walleij linus.wall...@linaro.org wrote: On Sat, Sep 1, 2012 at 1:14 PM, shiraz hashim shiraz.linux.ker...@gmail.com wrote: Hi Roland, On Wed, Aug 22, 2012 at 7:19 PM, Roland Stigge sti...@antcom.de wrote: @@ -2016,6 +2030,8 @@ pl022_probe(struct amba_device *adev, co pl022-master_info = platform_info; pl022-adev = adev; pl022-vendor = id-data; + /* Point chipselects to allocated memory beyond the main struct */ + pl022-chipselects = (int *) pl022 + sizeof(struct pl022); This is going beyond memory allocated for chipselects as it adds 4 * sizeof(struct pl022) bytes to pl022. Yes that is why the allocation looks like this: + master = spi_alloc_master(dev, sizeof(struct pl022) + sizeof(int) * + platform_info-num_chipselect); The allocation is such because type of chipselects is int. The statement for allocation is correct, but pl022-chipselects = (int *) pl022 + sizeof(struct pl022); is not adding sizeof(struct pl022) bytes to pl022 base (which we want), but infact 4 times the size of pl022 (because type of pl022 is now int *). Do you get my point ? This would go way beyond memory allocated for chipselects. -- regards Shiraz Hashim -- 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 v6 1/3] spi/pl022: Add chip select handling via GPIO
Hi Roland, On Wed, Aug 22, 2012 at 7:19 PM, Roland Stigge wrote: > @@ -2016,6 +2030,8 @@ pl022_probe(struct amba_device *adev, co > pl022->master_info = platform_info; > pl022->adev = adev; > pl022->vendor = id->data; > + /* Point chipselects to allocated memory beyond the main struct */ > + pl022->chipselects = (int *) pl022 + sizeof(struct pl022); This is going beyond memory allocated for chipselects as it adds 4 * sizeof(struct pl022) bytes to pl022. pl022->chipselects = (int *) [1]; can be musch safer. -- regards Shiraz Hashim -- 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 v6 1/3] spi/pl022: Add chip select handling via GPIO
Hi Roland, On Wed, Aug 22, 2012 at 7:19 PM, Roland Stigge sti...@antcom.de wrote: @@ -2016,6 +2030,8 @@ pl022_probe(struct amba_device *adev, co pl022-master_info = platform_info; pl022-adev = adev; pl022-vendor = id-data; + /* Point chipselects to allocated memory beyond the main struct */ + pl022-chipselects = (int *) pl022 + sizeof(struct pl022); This is going beyond memory allocated for chipselects as it adds 4 * sizeof(struct pl022) bytes to pl022. pl022-chipselects = (int *) pl022[1]; can be musch safer. -- regards Shiraz Hashim -- 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/
