[PATCH v3 2/2] Subject: Update PWM driver imported from BBBIO

[Martin Galvan]

Tested and built successfully, so I pushed both patches:

https://git.rtems.org/rtems/commit/?id=6dc5c03fad3ddd51423d21b5d60d24b62bb653e9
https://git.rtems.org/rtems/commit/?id=5e3096db5a1d766ece4068fbfe625c8a3db31b23

Congratulations PV!

On Mon, Jul 4, 2016 at 8:16 AM, punit vara <punitvara at gmail.com> wrote:

>> (Quiz: why did I need to wrap the bitwise & in parens? The answer will surprise you.
>> It certainly surprised me and a coworker back when we were working on the BBB interrupt
>> handler and the damn thing would keep hanging).
>
> I would like to know this solution as everything looks fine to me :)

As I mentioned in our chat, the answer can be found in this article
(look at the "Neonatal C" section):

https://www.bell-labs.com/usr/dmr/www/chist.html

>> +int beagle_pwmss_setting(uint32_t pwm_id, float pwm_freq, float dutyA, float dutyB)
> dutyA represents duty cycle for channel A and dutyB represents duty
> cycle for channel B. Any suggestions here ?

Makes sense. I can live with duty_a and duty_b, if there's a
doxycomment somewhere explaining what they are.

>> +  /*setting clock divider and freeze time base*/
>> +  REG16(baseAddr + AM335X_EPWM_CMPB) = (unsigned short)((float)(NearTBPRD) * dutyB);
>> +  REG16(baseAddr + AM335X_EPWM_CMPA) = (unsigned short)((float)(NearTBPRD) * dutyA);
>> +  REG16(baseAddr + AM335X_EPWM_TBPRD) = (unsigned short)NearTBPRD;
>> +  REG16(baseAddr + AM335X_EPWM_TBCNT) = 0;
>
> TBPRD  is 32 bit value and CMPB is 16 bit so We have to use typecasting here.

I still think it's wrong to cast like this. If I'm not mistaken,
there's no need for NearTBPRD to be an int in the first place; we
could just declare it as float and avoid the inner cast. The outer
cast isn't needed either; REG16 casts to uint16_t so the conversion
will be performed anyways.

Going through it again, I wonder if it's even required that these
variables are floating point instead of just integers. We end up
truncating the decimal part anyway, though maybe we want to operate
with floats to keep precision while doing the calculations above. The
reason why I'm thinking about using integers is that floating point
operations might be more expensive, though I understand the BBB has a
fairly decent CPU with VFP and everything. I'd say remove the casts
but keep the floats for now, unless anyone has a better suggestion.