This series adds support for capture to stm32-pwm driver.
Capture is based on DMAs.
- First two patches are precursor patches
- Subsequent two patches add support for requesting DMAs to MFD core
- Next three patches add support for capture to stm32-pwm driver
- This has been tested on stm32429i-eval board.
---
Resend v2:
- Add collected Acks
Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Rework pwm capture routines to adopt this change
- Comment on optional dma support, beautify DMAs probe
Fabrice Gasnier (8):
pwm: stm32: fix, remove unused struct device
pwm: stm32: protect common prescaler for all channels
dt-bindings: mfd: stm32-timers: add support for dmas
mfd: stm32-timers: add support for dmas
pwm: stm32: add capture support
pwm: stm32: improve capture by tuning counter prescaler
pwm: stm32: use input prescaler to improve period capture
ARM: dts: stm32: Enable pwm3 input capture on stm32f429i-eval
.../devicetree/bindings/mfd/stm32-timers.txt | 20 ++
arch/arm/boot/dts/stm32429i-eval.dts | 3 +
drivers/mfd/stm32-timers.c | 215 +++++++++++++++-
drivers/pwm/pwm-stm32.c | 276 ++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 39 +++
5 files changed, 547 insertions(+), 6 deletions(-)
--
1.9.1
Add support for DMAs to STM32 timers. STM32 Timers can support up to 7
dma requests: up to 4 channels, update, compare and trigger.
DMAs may be used to transfer data from pwm capture for instance.
DMA support is made optional, PWM capture support is also an option.
This is much more wise system-wide to avoid shortage on DMA request
lines as there's significant amount of timer instances that can
request up to 7 channels.
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Rob Herring <redacted>
Reviewed-by: Benjamin Gaignard <redacted>
---
.../devicetree/bindings/mfd/stm32-timers.txt | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
@@ -19,6 +19,11 @@ Required parameters: Optional parameters: - resets: Phandle to the parent reset controller. See ../reset/st,stm32-rcc.txt+- dmas: List of phandle to dma channels that can be used for+ this timer instance. There may be up to 7 dma channels.+- dma-names: List of dma names. Must match 'dmas' property. Valid+ names are: "ch1", "ch2", "ch3", "ch4", "up", "trig",+ "com". Optional subnodes: - pwm: See ../pwm/pwm-stm32.txt
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Currently, capture is based on timeout window to configure prescaler.
PWM capture framework provides 1s window at the time of writing.
There's place for improvement, after input signal has been captured once:
- Finer tune counter clock prescaler, by using 1st capture result (with
arbitrary margin).
- Do a 2nd capture, with scaled capture window.
This increases accuracy, especially at high rates.
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Benjamin Gaignard <redacted>
---
Changes in v2:
- Adopt DMA read from MFD core.
---
drivers/pwm/pwm-stm32.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
@@ -220,6 +220,28 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,if(ret)gotostop;+/*+*Gotacapture.Trytoimproveaccuracyathighrates:+*-decreasecounterclockprescaler,scaleuptomaxrate.+*/+if(raw_prd){+u32max_arr=priv->max_arr-0x1000;/* arbitrary margin */++scale=max_arr/min(max_arr,raw_prd);+}else{+scale=priv->max_arr;/* bellow resolution, use max scale */+}++if(psc&&scale>1){+/* 2nd measure with new scale */+psc/=scale;+regmap_write(priv->regmap,TIM_PSC,psc);+ret=stm32_pwm_raw_capture(priv,pwm,tmo_ms,&raw_prd,+&raw_dty);+if(ret)+gotostop;+}+prd=(unsignedlonglong)raw_prd*(psc+1)*NSEC_PER_SEC;result->period=DIV_ROUND_UP_ULL(prd,rate);dty=(unsignedlonglong)raw_dty*(psc+1)*NSEC_PER_SEC;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
STM32 Timers can support up to 7 DMA requests:
- 4 channels, update, compare and trigger.
Optionally request part, or all DMAs from stm32-timers MFD core.
Also add routine to implement burst reads using DMA from timer registers.
This is exported. So, it can be used by child drivers, PWM capture
for instance (but not limited to).
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Benjamin Gaignard <redacted>
---
Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Add comments on optional dma support
---
drivers/mfd/stm32-timers.c | 215 ++++++++++++++++++++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 27 +++++
2 files changed, 238 insertions(+), 4 deletions(-)
dev is never assigned nor used. remove it.
Fixes: 7edf7369205b ("pwm: Add driver for STM32 plaftorm")
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Benjamin Gaignard <redacted>
---
drivers/pwm/pwm-stm32.c | 1 -
1 file changed, 1 deletion(-)
There may be a race, when configuring two pwm channels, with different
prescaler values, when there's no active channel yet.
Add mutex lock to avoid concurrent access on pwm apply state.
This is also precursor patch for pwm capture support.
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Benjamin Gaignard <redacted>
---
drivers/pwm/pwm-stm32.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
@@ -213,9 +214,23 @@ static int stm32_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,returnret;}+staticintstm32_pwm_apply_locked(structpwm_chip*chip,structpwm_device*pwm,+structpwm_state*state)+{+structstm32_pwm*priv=to_stm32_pwm_dev(chip);+intret;++/* protect common prescaler for all active channels */+mutex_lock(&priv->lock);+ret=stm32_pwm_apply(chip,pwm,state);+mutex_unlock(&priv->lock);++returnret;+}+staticconststructpwm_opsstm32pwm_ops={.owner=THIS_MODULE,-.apply=stm32_pwm_apply,+.apply=stm32_pwm_apply_locked,};staticintstm32_pwm_set_breakinput(structstm32_pwm*priv,
@@ -335,6 +350,7 @@ static int stm32_pwm_probe(struct platform_device *pdev)if(!priv)return-ENOMEM;+mutex_init(&priv->lock);priv->regmap=ddata->regmap;priv->clk=ddata->clk;priv->max_arr=ddata->max_arr;
Using input prescaler, capture unit will trigger DMA once every
configurable /2, /4 or /8 events (rising edge). This helps improve
period (only) capture accuracy at high rates.
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Benjamin Gaignard <redacted>
---
Changes in v2:
- Adopt DMA read from MFD core.
---
drivers/pwm/pwm-stm32.c | 63 ++++++++++++++++++++++++++++++++++++++--
include/linux/mfd/stm32-timers.h | 1 +
2 files changed, 62 insertions(+), 2 deletions(-)
@@ -242,8 +244,65 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,gotostop;}+/* Compute intermediate period not to exceed timeout at low rates */prd=(unsignedlonglong)raw_prd*(psc+1)*NSEC_PER_SEC;-result->period=DIV_ROUND_UP_ULL(prd,rate);+do_div(prd,rate);++for(icpsc=0;icpsc<MAX_TIM_ICPSC;icpsc++){+/* input prescaler: also keep arbitrary margin */+if(raw_prd>=(priv->max_arr-0x1000)>>(icpsc+1))+break;+if(prd>=(tmo_ms*NSEC_PER_MSEC)>>(icpsc+2))+break;+}++if(!icpsc)+gotodone;++/* Last chance to improve period accuracy, using input prescaler */+regmap_update_bits(priv->regmap,+pwm->hwpwm<2?TIM_CCMR1:TIM_CCMR2,+TIM_CCMR_IC1PSC|TIM_CCMR_IC2PSC,+FIELD_PREP(TIM_CCMR_IC1PSC,icpsc)|+FIELD_PREP(TIM_CCMR_IC2PSC,icpsc));++ret=stm32_pwm_raw_capture(priv,pwm,tmo_ms,&raw_prd,&raw_dty);+if(ret)+gotostop;++if(raw_dty>=(raw_prd>>icpsc)){+/*+*Wemayfallhereusinginputprescaler,wheninput+*capturestartsonhighside(beforefallingedge).+*Examplewithicpsctocaptureoneach4events:+*+*start1stcapture2ndcapture+*vvv+*___________________________+*TI1..4|__||__||__||__||__|+*vv.....vv+*icpsc1/3:.0.1.2.3.0+*icpsc2/4:01230+*vvvv+*CCR1/3......t0..............................t2+*CCR2/4..t1..............................t1'...+*...+*Capture0:.<----------------------------->.+*Capture1:.<-------------------------->..+*...+*Period:.<------>..+*Lowside:.<>.+*+*Result:+*-Period=Capture0/icpsc+*-Duty=Period-Lowside=Period-(Capture0-Capture1)+*/+raw_dty=(raw_prd>>icpsc)-(raw_prd-raw_dty);+}++done:+prd=(unsignedlonglong)raw_prd*(psc+1)*NSEC_PER_SEC;+result->period=DIV_ROUND_UP_ULL(prd,rate<<icpsc);dty=(unsignedlonglong)raw_dty*(psc+1)*NSEC_PER_SEC;result->duty_cycle=DIV_ROUND_UP_ULL(dty,rate);stop:
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Add support for PMW input mode on pwm-stm32. STM32 timers support
period and duty cycle capture as long as they have at least two PWM
channels. One capture channel is used for period (rising-edge), one
for duty-cycle (falling-edge).
When there's only one channel available, only period can be captured.
Duty-cycle is simply zero'ed in such a case.
Capture requires exclusive access (e.g. no pwm output running at the
same time, to protect common prescaler).
Timer DMA burst mode (from MFD core) is being used, to take two
snapshots of capture registers (upon each period rising edge).
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Benjamin Gaignard <redacted>
---
Changes in v2:
- DMA handling has been moved to MFD core. Rework capture routines to
use it.
---
drivers/pwm/pwm-stm32.c | 176 +++++++++++++++++++++++++++++++++++++++
include/linux/mfd/stm32-timers.h | 11 +++
2 files changed, 187 insertions(+)
@@ -63,6 +64,178 @@ static int write_ccrx(struct stm32_pwm *dev, int ch, u32 value)return-EINVAL;}+#define TIM_CCER_CC12P (TIM_CCER_CC1P | TIM_CCER_CC2P)+#define TIM_CCER_CC12E (TIM_CCER_CC1E | TIM_CCER_CC2E)+#define TIM_CCER_CC34P (TIM_CCER_CC3P | TIM_CCER_CC4P)+#define TIM_CCER_CC34E (TIM_CCER_CC3E | TIM_CCER_CC4E)++/*+*CaptureusingPWMinputmode:+*______+*TI[1,2,3or4]:........._||________|+*^0^1^2+*...+*..XXXXX+*..XXXXX|+*.XXXXX.|+*XXXXX..|+*COUNTER:______XXXXX...|_XXX+*start^...^stop+*....+*vv.v+*v+*CCR1/CCR3:tx..........t0...........t2+*CCR2/CCR4:tx..............t1.........+*+*DMAbursttransfer:||+*vv+*DMAbuffer:{t0,tx}{t2,t1}+*DMAdone:^+*+*0:IC1/3snapchotonrisingedge:countervalue->CCR1/CCR3+*+DMAtransferCCR[1/3]&CCR[2/4]values(t0,tx:doesn'tcare)+*1:IC2/4snapchotonfallingedge:countervalue->CCR2/CCR4+*2:IC1/3snapchotonrisingedge:countervalue->CCR1/CCR3+*+DMAtransferCCR[1/3]&CCR[2/4]values(t2,t1)+*+*DMAdone,compute:+*-Period=t2-t0+*-Dutycycle=t1-t0+*/+staticintstm32_pwm_raw_capture(structstm32_pwm*priv,structpwm_device*pwm,+unsignedlongtmo_ms,u32*raw_prd,+u32*raw_dty)+{+structstm32_timers*ddata=dev_get_drvdata(priv->chip.dev->parent);+enumstm32_timers_dmasdma_id;+u32ccen,ccr;+intret;++/* Ensure registers have been updated, enable counter and capture */+regmap_update_bits(priv->regmap,TIM_EGR,TIM_EGR_UG,TIM_EGR_UG);+regmap_update_bits(priv->regmap,TIM_CR1,TIM_CR1_CEN,TIM_CR1_CEN);++/* Use cc1 or cc3 DMA resp for PWM input channels 1 & 2 or 3 & 4 */+dma_id=pwm->hwpwm<2?STM32_TIMERS_DMA_CH1:STM32_TIMERS_DMA_CH3;+ccen=pwm->hwpwm<2?TIM_CCER_CC12E:TIM_CCER_CC34E;+ccr=pwm->hwpwm<2?TIM_CCR1:TIM_CCR3;+regmap_update_bits(priv->regmap,TIM_CCER,ccen,ccen);++/*+*TimerDMAburstmode.Request2registers,2bursts,togetboth+*CCR1&CCR2(orCCR3&CCR4)oneachcaptureevent.+*We'llgettwocapturesnapchots:{CCR1,CCR2},{CCR1,CCR2}+*or{CCR3,CCR4},{CCR3,CCR4}+*/+ret=stm32_timers_dma_burst_read(ddata,priv->capture,dma_id,ccr,2,+2,tmo_ms);+if(ret)+gotostop;++/* Period: t2 - t0 (take care of counter overflow) */+if(priv->capture[0]<=priv->capture[2])+*raw_prd=priv->capture[2]-priv->capture[0];+else+*raw_prd=priv->max_arr-priv->capture[0]+priv->capture[2];++/* Duty cycle capture requires at least two capture units */+if(pwm->chip->npwm<2)+*raw_dty=0;+elseif(priv->capture[0]<=priv->capture[3])+*raw_dty=priv->capture[3]-priv->capture[0];+else+*raw_dty=priv->max_arr-priv->capture[0]+priv->capture[3];++if(*raw_dty>*raw_prd){+/*+*RacebeetweenPWMinputandDMA:itmayhappen+*fallingedgetriggersnewcaptureonTI2/4beforeDMA+*hadachancetoreadCCR2/4.Itmeanscapture[1]+*containsperiod+duty_cycle.So,subtractperiod.+*/+*raw_dty-=*raw_prd;+}++stop:+regmap_update_bits(priv->regmap,TIM_CCER,ccen,0);+regmap_update_bits(priv->regmap,TIM_CR1,TIM_CR1_CEN,0);++returnret;+}++staticintstm32_pwm_capture(structpwm_chip*chip,structpwm_device*pwm,+structpwm_capture*result,unsignedlongtmo_ms)+{+structstm32_pwm*priv=to_stm32_pwm_dev(chip);+unsignedlonglongprd,div,dty;+unsignedlongrate;+unsignedintpsc=0;+u32raw_prd,raw_dty;+intret=0;++mutex_lock(&priv->lock);++if(active_channels(priv)){+ret=-EBUSY;+gotounlock;+}++ret=clk_enable(priv->clk);+if(ret){+dev_err(priv->chip.dev,"failed to enable counter clock\n");+gotounlock;+}++rate=clk_get_rate(priv->clk);+if(!rate){+ret=-EINVAL;+gotoclk_dis;+}++/* prescaler: fit timeout window provided by upper layer */+div=(unsignedlonglong)rate*(unsignedlonglong)tmo_ms;+do_div(div,MSEC_PER_SEC);+prd=div;+while((div>priv->max_arr)&&(psc<MAX_TIM_PSC)){+psc++;+div=prd;+do_div(div,psc+1);+}+regmap_write(priv->regmap,TIM_ARR,priv->max_arr);+regmap_write(priv->regmap,TIM_PSC,psc);++/* Map TI1 or TI2 PWM input to IC1 & IC2 (or TI3/4 to IC3 & IC4) */+regmap_update_bits(priv->regmap,+pwm->hwpwm<2?TIM_CCMR1:TIM_CCMR2,+TIM_CCMR_CC1S|TIM_CCMR_CC2S,pwm->hwpwm&0x1?+TIM_CCMR_CC1S_TI2|TIM_CCMR_CC2S_TI2:+TIM_CCMR_CC1S_TI1|TIM_CCMR_CC2S_TI1);++/* Capture period on IC1/3 rising edge, duty cycle on IC2/4 falling. */+regmap_update_bits(priv->regmap,TIM_CCER,pwm->hwpwm<2?+TIM_CCER_CC12P:TIM_CCER_CC34P,pwm->hwpwm<2?+TIM_CCER_CC2P:TIM_CCER_CC4P);++ret=stm32_pwm_raw_capture(priv,pwm,tmo_ms,&raw_prd,&raw_dty);+if(ret)+gotostop;++prd=(unsignedlonglong)raw_prd*(psc+1)*NSEC_PER_SEC;+result->period=DIV_ROUND_UP_ULL(prd,rate);+dty=(unsignedlonglong)raw_dty*(psc+1)*NSEC_PER_SEC;+result->duty_cycle=DIV_ROUND_UP_ULL(dty,rate);+stop:+regmap_write(priv->regmap,TIM_CCER,0);+regmap_write(priv->regmap,pwm->hwpwm<2?TIM_CCMR1:TIM_CCMR2,0);+regmap_write(priv->regmap,TIM_PSC,0);+clk_dis:+clk_disable(priv->clk);+unlock:+mutex_unlock(&priv->lock);++returnret;+}+staticintstm32_pwm_config(structstm32_pwm*priv,intch,intduty_ns,intperiod_ns){
This series adds support for capture to stm32-pwm driver.
Capture is based on DMAs.
- First two patches are precursor patches
- Subsequent two patches add support for requesting DMAs to MFD core
- Next three patches add support for capture to stm32-pwm driver
- This has been tested on stm32429i-eval board.
Hi all,
Gentle ping to review this series, since DT Bindings has been reviewed
by Rob and the series by Benjamin.
Many thanks in advance,
Regards,
Fabrice
---
Resend v2:
- Add collected Acks
Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Rework pwm capture routines to adopt this change
- Comment on optional dma support, beautify DMAs probe
Fabrice Gasnier (8):
pwm: stm32: fix, remove unused struct device
pwm: stm32: protect common prescaler for all channels
dt-bindings: mfd: stm32-timers: add support for dmas
mfd: stm32-timers: add support for dmas
pwm: stm32: add capture support
pwm: stm32: improve capture by tuning counter prescaler
pwm: stm32: use input prescaler to improve period capture
ARM: dts: stm32: Enable pwm3 input capture on stm32f429i-eval
.../devicetree/bindings/mfd/stm32-timers.txt | 20 ++
arch/arm/boot/dts/stm32429i-eval.dts | 3 +
drivers/mfd/stm32-timers.c | 215 +++++++++++++++-
drivers/pwm/pwm-stm32.c | 276 ++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 39 +++
5 files changed, 547 insertions(+), 6 deletions(-)
On Wed, Feb 14, 2018 at 11:04:31AM +0100, Fabrice Gasnier wrote:
This series adds support for capture to stm32-pwm driver.
Capture is based on DMAs.
- First two patches are precursor patches
- Subsequent two patches add support for requesting DMAs to MFD core
- Next three patches add support for capture to stm32-pwm driver
- This has been tested on stm32429i-eval board.
---
Resend v2:
- Add collected Acks
Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Rework pwm capture routines to adopt this change
- Comment on optional dma support, beautify DMAs probe
Fabrice Gasnier (8):
pwm: stm32: fix, remove unused struct device
pwm: stm32: protect common prescaler for all channels
dt-bindings: mfd: stm32-timers: add support for dmas
mfd: stm32-timers: add support for dmas
pwm: stm32: add capture support
pwm: stm32: improve capture by tuning counter prescaler
pwm: stm32: use input prescaler to improve period capture
ARM: dts: stm32: Enable pwm3 input capture on stm32f429i-eval
.../devicetree/bindings/mfd/stm32-timers.txt | 20 ++
arch/arm/boot/dts/stm32429i-eval.dts | 3 +
drivers/mfd/stm32-timers.c | 215 +++++++++++++++-
drivers/pwm/pwm-stm32.c | 276 ++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 39 +++
5 files changed, 547 insertions(+), 6 deletions(-)
I can't apply patches 1-2 and 5-6 since they depend on patches 3-4 for
which I'd need an Acked-by: from Lee if I'm to pick them up. Same goes
for patch 7.
By the looks of it there are minor conflicts with the MFD tree, but no
major ones. Perhaps it'd be better for Lee to pick up 3-4 for v4.17-rc1
and ack patch 7, then I can take the rest after v4.17-rc1?
Thierry
On Wed, Mar 28, 2018 at 12:51:02AM +0200, Thierry Reding wrote:
On Wed, Feb 14, 2018 at 11:04:31AM +0100, Fabrice Gasnier wrote:
quoted
This series adds support for capture to stm32-pwm driver.
Capture is based on DMAs.
- First two patches are precursor patches
- Subsequent two patches add support for requesting DMAs to MFD core
- Next three patches add support for capture to stm32-pwm driver
- This has been tested on stm32429i-eval board.
---
Resend v2:
- Add collected Acks
Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Rework pwm capture routines to adopt this change
- Comment on optional dma support, beautify DMAs probe
Fabrice Gasnier (8):
pwm: stm32: fix, remove unused struct device
pwm: stm32: protect common prescaler for all channels
dt-bindings: mfd: stm32-timers: add support for dmas
mfd: stm32-timers: add support for dmas
pwm: stm32: add capture support
pwm: stm32: improve capture by tuning counter prescaler
pwm: stm32: use input prescaler to improve period capture
ARM: dts: stm32: Enable pwm3 input capture on stm32f429i-eval
.../devicetree/bindings/mfd/stm32-timers.txt | 20 ++
arch/arm/boot/dts/stm32429i-eval.dts | 3 +
drivers/mfd/stm32-timers.c | 215 +++++++++++++++-
drivers/pwm/pwm-stm32.c | 276 ++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 39 +++
5 files changed, 547 insertions(+), 6 deletions(-)
I can't apply patches 1-2 and 5-6 since they depend on patches 3-4 for
which I'd need an Acked-by: from Lee if I'm to pick them up. Same goes
for patch 7.
By the looks of it there are minor conflicts with the MFD tree, but no
major ones. Perhaps it'd be better for Lee to pick up 3-4 for v4.17-rc1
and ack patch 7, then I can take the rest after v4.17-rc1?
I can pick up 1-2 which are separate from the capture changes and have
no dependencies.
Thierry
From: Lee Jones <hidden> Date: 2018-03-28 10:03:16
On Wed, 28 Mar 2018, Thierry Reding wrote:
On Wed, Feb 14, 2018 at 11:04:31AM +0100, Fabrice Gasnier wrote:
quoted
This series adds support for capture to stm32-pwm driver.
Capture is based on DMAs.
- First two patches are precursor patches
- Subsequent two patches add support for requesting DMAs to MFD core
- Next three patches add support for capture to stm32-pwm driver
- This has been tested on stm32429i-eval board.
---
Resend v2:
- Add collected Acks
Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Rework pwm capture routines to adopt this change
- Comment on optional dma support, beautify DMAs probe
Fabrice Gasnier (8):
pwm: stm32: fix, remove unused struct device
pwm: stm32: protect common prescaler for all channels
dt-bindings: mfd: stm32-timers: add support for dmas
mfd: stm32-timers: add support for dmas
pwm: stm32: add capture support
pwm: stm32: improve capture by tuning counter prescaler
pwm: stm32: use input prescaler to improve period capture
ARM: dts: stm32: Enable pwm3 input capture on stm32f429i-eval
.../devicetree/bindings/mfd/stm32-timers.txt | 20 ++
arch/arm/boot/dts/stm32429i-eval.dts | 3 +
drivers/mfd/stm32-timers.c | 215 +++++++++++++++-
drivers/pwm/pwm-stm32.c | 276 ++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 39 +++
5 files changed, 547 insertions(+), 6 deletions(-)
I can't apply patches 1-2 and 5-6 since they depend on patches 3-4 for
which I'd need an Acked-by: from Lee if I'm to pick them up. Same goes
for patch 7.
By the looks of it there are minor conflicts with the MFD tree, but no
major ones. Perhaps it'd be better for Lee to pick up 3-4 for v4.17-rc1
and ack patch 7, then I can take the rest after v4.17-rc1?
Sounds like a faff.
Why don't I just pick them all up (except the ARM patch)?
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
From: Lee Jones <hidden> Date: 2018-03-28 11:16:28
On Wed, 14 Feb 2018, Fabrice Gasnier wrote:
Add support for DMAs to STM32 timers. STM32 Timers can support up to 7
dma requests: up to 4 channels, update, compare and trigger.
DMAs may be used to transfer data from pwm capture for instance.
DMA support is made optional, PWM capture support is also an option.
This is much more wise system-wide to avoid shortage on DMA request
lines as there's significant amount of timer instances that can
request up to 7 channels.
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Benjamin Gaignard <redacted>
---
.../devicetree/bindings/mfd/stm32-timers.txt | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
For my own reference:
Acked-for-MFD-by: Lee Jones [off-list ref]
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
From: Lee Jones <hidden> Date: 2018-03-28 15:22:58
On Wed, 14 Feb 2018, Fabrice Gasnier wrote:
quoted hunk
STM32 Timers can support up to 7 DMA requests:
- 4 channels, update, compare and trigger.
Optionally request part, or all DMAs from stm32-timers MFD core.
Also add routine to implement burst reads using DMA from timer registers.
This is exported. So, it can be used by child drivers, PWM capture
for instance (but not limited to).
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Benjamin Gaignard <redacted>
---
Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Add comments on optional dma support
---
drivers/mfd/stm32-timers.c | 215 ++++++++++++++++++++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 27 +++++
2 files changed, 238 insertions(+), 4 deletions(-)
This looks odd to me. Why can't you expand the current ddata
structure? Wouldn't it be better to create a stm32_timers_dma
structure to place all this information in (except *dev, that should
live in the ddata struct), then place a reference in the existing
stm32_timers struct?
STM32 Timers can support up to 7 DMA requests:
- 4 channels, update, compare and trigger.
Optionally request part, or all DMAs from stm32-timers MFD core.
Also add routine to implement burst reads using DMA from timer registers.
This is exported. So, it can be used by child drivers, PWM capture
for instance (but not limited to).
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Benjamin Gaignard <redacted>
---
Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Add comments on optional dma support
---
drivers/mfd/stm32-timers.c | 215 ++++++++++++++++++++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 27 +++++
2 files changed, 238 insertions(+), 4 deletions(-)
This looks odd to me. Why can't you expand the current ddata
structure? Wouldn't it be better to create a stm32_timers_dma
structure to place all this information in (except *dev, that should
live in the ddata struct), then place a reference in the existing
stm32_timers struct?
... "passing in the physical address of the parent MFD into
a child device doesn't quite sit right with me"
I introduced this private struct in MFD parent, and completely hide it
from the child.
So, do you suggest to add struct definition here ? But make it part of
struct stm32_timers *ddata?
And only put declaration in include/linux/mfd/stm32-timers.h:
+ struct stm32_timers_dma;
struct stm32_timers {
struct clk *clk;
struct regmap *regmap;
u32 max_arr;
+ struct stm32_timers_dma;
};
I can probably spare the *dev then... use dev->parent in child driver.
Can you just confirm this please?
I can use devm_of_platform_depopulate() here if you prefer, and keep
devm_of_platform_populate() in probe.
Please let me know
Thanks for reviewing,
Best Regards,
Fabrice
On Wed, Mar 28, 2018 at 11:03:09AM +0100, Lee Jones wrote:
On Wed, 28 Mar 2018, Thierry Reding wrote:
quoted
On Wed, Feb 14, 2018 at 11:04:31AM +0100, Fabrice Gasnier wrote:
quoted
This series adds support for capture to stm32-pwm driver.
Capture is based on DMAs.
- First two patches are precursor patches
- Subsequent two patches add support for requesting DMAs to MFD core
- Next three patches add support for capture to stm32-pwm driver
- This has been tested on stm32429i-eval board.
---
Resend v2:
- Add collected Acks
Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Rework pwm capture routines to adopt this change
- Comment on optional dma support, beautify DMAs probe
Fabrice Gasnier (8):
pwm: stm32: fix, remove unused struct device
pwm: stm32: protect common prescaler for all channels
dt-bindings: mfd: stm32-timers: add support for dmas
mfd: stm32-timers: add support for dmas
pwm: stm32: add capture support
pwm: stm32: improve capture by tuning counter prescaler
pwm: stm32: use input prescaler to improve period capture
ARM: dts: stm32: Enable pwm3 input capture on stm32f429i-eval
.../devicetree/bindings/mfd/stm32-timers.txt | 20 ++
arch/arm/boot/dts/stm32429i-eval.dts | 3 +
drivers/mfd/stm32-timers.c | 215 +++++++++++++++-
drivers/pwm/pwm-stm32.c | 276 ++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 39 +++
5 files changed, 547 insertions(+), 6 deletions(-)
I can't apply patches 1-2 and 5-6 since they depend on patches 3-4 for
which I'd need an Acked-by: from Lee if I'm to pick them up. Same goes
for patch 7.
By the looks of it there are minor conflicts with the MFD tree, but no
major ones. Perhaps it'd be better for Lee to pick up 3-4 for v4.17-rc1
and ack patch 7, then I can take the rest after v4.17-rc1?
Sounds like a faff.
Why don't I just pick them all up (except the ARM patch)?
Fine with me. For all the PWM patches in this series:
Acked-by: Thierry Reding <redacted>
From: Lee Jones <hidden> Date: 2018-03-29 12:59:20
On Wed, 28 Mar 2018, Fabrice Gasnier wrote:
On 03/28/2018 05:22 PM, Lee Jones wrote:
quoted
On Wed, 14 Feb 2018, Fabrice Gasnier wrote:
quoted
STM32 Timers can support up to 7 DMA requests:
- 4 channels, update, compare and trigger.
Optionally request part, or all DMAs from stm32-timers MFD core.
Also add routine to implement burst reads using DMA from timer registers.
This is exported. So, it can be used by child drivers, PWM capture
for instance (but not limited to).
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Benjamin Gaignard <redacted>
---
Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Add comments on optional dma support
---
drivers/mfd/stm32-timers.c | 215 ++++++++++++++++++++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 27 +++++
2 files changed, 238 insertions(+), 4 deletions(-)
This looks odd to me. Why can't you expand the current ddata
structure? Wouldn't it be better to create a stm32_timers_dma
structure to place all this information in (except *dev, that should
live in the ddata struct), then place a reference in the existing
stm32_timers struct?
... "passing in the physical address of the parent MFD into
a child device doesn't quite sit right with me"
I introduced this private struct in MFD parent, and completely hide it
from the child.
So, do you suggest to add struct definition here ? But make it part of
struct stm32_timers *ddata?
And only put declaration in include/linux/mfd/stm32-timers.h:
+ struct stm32_timers_dma;
struct stm32_timers {
struct clk *clk;
struct regmap *regmap;
u32 max_arr;
+ struct stm32_timers_dma;
};
Yes, that's the basic idea.
I can probably spare the *dev then... use dev->parent in child driver.
I can use devm_of_platform_depopulate() here if you prefer, and keep
devm_of_platform_populate() in probe.
The point of devm_* is that you don't have to call depopulate.
It happens automatically once this driver is unbound.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
STM32 Timers can support up to 7 DMA requests:
- 4 channels, update, compare and trigger.
Optionally request part, or all DMAs from stm32-timers MFD core.
Also add routine to implement burst reads using DMA from timer registers.
This is exported. So, it can be used by child drivers, PWM capture
for instance (but not limited to).
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Benjamin Gaignard <redacted>
---
Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Add comments on optional dma support
---
drivers/mfd/stm32-timers.c | 215 ++++++++++++++++++++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 27 +++++
2 files changed, 238 insertions(+), 4 deletions(-)
This looks odd to me. Why can't you expand the current ddata
structure? Wouldn't it be better to create a stm32_timers_dma
structure to place all this information in (except *dev, that should
live in the ddata struct), then place a reference in the existing
stm32_timers struct?
... "passing in the physical address of the parent MFD into
a child device doesn't quite sit right with me"
I introduced this private struct in MFD parent, and completely hide it
from the child.
So, do you suggest to add struct definition here ? But make it part of
struct stm32_timers *ddata?
And only put declaration in include/linux/mfd/stm32-timers.h:
+ struct stm32_timers_dma;
struct stm32_timers {
struct clk *clk;
struct regmap *regmap;
u32 max_arr;
+ struct stm32_timers_dma;
};
Yes, that's the basic idea.
quoted
I can probably spare the *dev then... use dev->parent in child driver.
What would you use dev->parent for?
Hi Lee,
This is to follow your sugestion to use *dev instead of *ddata when
calling stm32_timers_dma_burst_read(), the idea is to use it on child side:
stm32_timers_dma_burst_read(dev->parent,...) from pwm driver.
Then there is no need to keep *dev inside ddata struct.
I can use devm_of_platform_depopulate() here if you prefer, and keep
devm_of_platform_populate() in probe.
The point of devm_* is that you don't have to call depopulate.
It happens automatically once this driver is unbound.
Ok, so to clarify, keeping devm_ here may be a bit racy:
of_platform_depopulate will happen after dma has been released (there is
no devm_ variant to release dma).
Only way to prevent race condition here, is to enforce
of_platform_depopulate() is called before dma release (e.g. in reverse
order compared to probe).
Do you wish I add a comment about it ?
Best Regards,
Fabrice
From: Lee Jones <hidden> Date: 2018-03-29 14:31:35
On Thu, 29 Mar 2018, Fabrice Gasnier wrote:
On 03/29/2018 02:59 PM, Lee Jones wrote:
quoted
On Wed, 28 Mar 2018, Fabrice Gasnier wrote:
quoted
On 03/28/2018 05:22 PM, Lee Jones wrote:
quoted
On Wed, 14 Feb 2018, Fabrice Gasnier wrote:
quoted
STM32 Timers can support up to 7 DMA requests:
- 4 channels, update, compare and trigger.
Optionally request part, or all DMAs from stm32-timers MFD core.
Also add routine to implement burst reads using DMA from timer registers.
This is exported. So, it can be used by child drivers, PWM capture
for instance (but not limited to).
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Benjamin Gaignard <redacted>
---
Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Add comments on optional dma support
---
drivers/mfd/stm32-timers.c | 215 ++++++++++++++++++++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 27 +++++
2 files changed, 238 insertions(+), 4 deletions(-)
This looks odd to me. Why can't you expand the current ddata
structure? Wouldn't it be better to create a stm32_timers_dma
structure to place all this information in (except *dev, that should
live in the ddata struct), then place a reference in the existing
stm32_timers struct?
... "passing in the physical address of the parent MFD into
a child device doesn't quite sit right with me"
I introduced this private struct in MFD parent, and completely hide it
from the child.
So, do you suggest to add struct definition here ? But make it part of
struct stm32_timers *ddata?
And only put declaration in include/linux/mfd/stm32-timers.h:
+ struct stm32_timers_dma;
struct stm32_timers {
struct clk *clk;
struct regmap *regmap;
u32 max_arr;
+ struct stm32_timers_dma;
};
Yes, that's the basic idea.
quoted
I can probably spare the *dev then... use dev->parent in child driver.
What would you use dev->parent for?
Hi Lee,
This is to follow your sugestion to use *dev instead of *ddata when
calling stm32_timers_dma_burst_read(), the idea is to use it on child side:
stm32_timers_dma_burst_read(dev->parent,...) from pwm driver.
Then there is no need to keep *dev inside ddata struct.
I'm wondering if it would be neater to us the child's *dev, then do
the ->parent deference in the parent MFD (with a comment to say what
you're doing of course).
I can use devm_of_platform_depopulate() here if you prefer, and keep
devm_of_platform_populate() in probe.
The point of devm_* is that you don't have to call depopulate.
It happens automatically once this driver is unbound.
Ok, so to clarify, keeping devm_ here may be a bit racy:
of_platform_depopulate will happen after dma has been released (there is
no devm_ variant to release dma).
Only way to prevent race condition here, is to enforce
of_platform_depopulate() is called before dma release (e.g. in reverse
order compared to probe).
Do you wish I add a comment about it ?
Best thing to do then is keep the non-devm variant and provide a
comment as to why is it not possible to use devm_*.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
STM32 Timers can support up to 7 DMA requests:
- 4 channels, update, compare and trigger.
Optionally request part, or all DMAs from stm32-timers MFD core.
Also add routine to implement burst reads using DMA from timer registers.
This is exported. So, it can be used by child drivers, PWM capture
for instance (but not limited to).
Signed-off-by: Fabrice Gasnier <redacted>
Reviewed-by: Benjamin Gaignard <redacted>
---
Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Add comments on optional dma support
---
drivers/mfd/stm32-timers.c | 215 ++++++++++++++++++++++++++++++++++++++-
include/linux/mfd/stm32-timers.h | 27 +++++
2 files changed, 238 insertions(+), 4 deletions(-)
This looks odd to me. Why can't you expand the current ddata
structure? Wouldn't it be better to create a stm32_timers_dma
structure to place all this information in (except *dev, that should
live in the ddata struct), then place a reference in the existing
stm32_timers struct?
... "passing in the physical address of the parent MFD into
a child device doesn't quite sit right with me"
I introduced this private struct in MFD parent, and completely hide it
from the child.
So, do you suggest to add struct definition here ? But make it part of
struct stm32_timers *ddata?
And only put declaration in include/linux/mfd/stm32-timers.h:
+ struct stm32_timers_dma;
struct stm32_timers {
struct clk *clk;
struct regmap *regmap;
u32 max_arr;
+ struct stm32_timers_dma;
};
Yes, that's the basic idea.
quoted
I can probably spare the *dev then... use dev->parent in child driver.
What would you use dev->parent for?
Hi Lee,
This is to follow your sugestion to use *dev instead of *ddata when
calling stm32_timers_dma_burst_read(), the idea is to use it on child side:
stm32_timers_dma_burst_read(dev->parent,...) from pwm driver.
Then there is no need to keep *dev inside ddata struct.
I'm wondering if it would be neater to us the child's *dev, then do
the ->parent deference in the parent MFD (with a comment to say what
you're doing of course).
There's already dev.parent dereference in child drivers, for same
purpose: dev_get_drvdata(pdev->dev.parent). So, I guess same can be done
here ?
Thanks for you review,
I'll update all this and send a v3.
Best regards,
Fabrice
I can use devm_of_platform_depopulate() here if you prefer, and keep
devm_of_platform_populate() in probe.
The point of devm_* is that you don't have to call depopulate.
It happens automatically once this driver is unbound.
Ok, so to clarify, keeping devm_ here may be a bit racy:
of_platform_depopulate will happen after dma has been released (there is
no devm_ variant to release dma).
Only way to prevent race condition here, is to enforce
of_platform_depopulate() is called before dma release (e.g. in reverse
order compared to probe).
Do you wish I add a comment about it ?
Best thing to do then is keep the non-devm variant and provide a
comment as to why is it not possible to use devm_*.