Thanks for the patch. FYI we usually tag patches as [PATCH v2], [PATCH
v3] and so on for patch versions greater than 1.
One thing I've noticed (and that I missed for v1) is that in many
places you have things like if((pwmid<3) & (pwmid >=0)), using bitwise
& instead of logical &&. Your tests must've worked because you only
tried valid PWM IDs; do some negative testing as well to cover all the
cases.
A few more comments below.
On Thu, Jun 30, 2016 at 8:58 AM, Punit Vara <punitv...@gmail.com> wrote:
> +
> + default:
> + printf("PWM output is not available on this pin\n");
> + break;
> +}
> +return true;
Looks like you're always returning true, even if an error occurred.
> /* PWMSS setting
> - * set pluse rgument of epwm module
> + * set pulse argument of epwm module
> *
> - * @param PWMID : EPWMSS number , 0~2
> - * @param HZ : pluse HZ
> + * @param pwm_id : EPWMSS number , 0~2
> + * @param pwm_freq : frequency to be generated
> * @param dutyA : Duty Cycle in ePWM A
> * @param dutyB : Duty Cycle in ePWM B
> *
> * @return : 1 for success , 0 for failed
> *
> - * @example : BBBIO_PWMSS_Setting(0 , 50.0f , 50.0f , 25.0f);
> // Generate 50HZ pwm in PWM0 ,
> - *
> // duty cycle is 50% for ePWM0A , 25% for ePWM0B
> + * @example : PWMSS_Setting(0 , 50.0f , 50.0f , 25.0f); //
> Generate 50HZ pwm in PWM0 ,
> + *
> // duty cycle is 50% for ePWM0A , 25% for ePWM0B
> *
> - * @Note :
> - * find an number nearst 65535 for TBPRD , to improve duty
> precision,
> + * @Note :
> + * find an number nearst 65535 for TBPRD , to improve duty
> precision,
> *
> - * Using big TBPRD can increase the range of CMPA and CMPB ,
> - * and it means we can get better precision on duty cycle.
> + * Using big TBPRD can increase the range of CMPA and CMPB ,
> + * and it means we can get better precision on duty cycle.
> *
> - * EX : 20.25% duty cycle
> + * EX : 20.25% duty cycle
> * on TBPRD = 62500 , CMPA = 12656.25 ( .25 rejection) ,
> real duty : 20.2496% (12656 /62500)
> * on TBPRD = 6250 , CMPA = 1265.625 ( .625 rejection),
> real duty : 20.24% (1265 6250)
> * on TBPRD = 500 , CMPA = 101.25 ( .25 rejection) ,
> real duty : 20.2% (101/500)
> *
> - * Divisor = CLKDIV * HSPCLKDIV
> - * 1 TBPRD : 10 ns (default)
> - * 65535 TBPRD : 655350 ns
> - * 65535 TBPRD : 655350 * Divisor ns = X TBPRD : Cyclens
> + * Divisor = CLKDIV * HSPCLKDIV
> + * 1 TBPRD : 10 ns (default)
> + * 65535 TBPRD : 655350 ns
> + * 65535 TBPRD : 655350 * Divisor ns = X TBPRD :
> Cyclens
> *
> - * accrooding to that , we must find a Divisor value , let X
> nearest 65535 .
> - * so , Divisor must Nearest Cyclens/655350
> -*/
No need to repeat this comment here if you already placed it above the
function declaration.
> +int beagle_pwmss_setting(uint32_t pwm_id, float pwm_freq, float dutyA, float
> dutyB)
> +{
> + uint32_t baseAddr;
> + int param_error =1;
> + if(pwm_freq < 0)
> + param_error =0;
> + if(dutyA < 0.0f || dutyA > 100.0f || dutyB < 0.0f || dutyB > 100.0f)
> + param_error = 0;
> + if(param_error == 0) {
> + printf("ERROR in parameter \n");
> + }
You could use this param_error to actually return a status code, which
the function already does at a couple of places.
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
