[PATCH] Fix for Beaglebone BSP PWM bug

[James Fitzsimons]

Hi Christian and Chris,

On Sun, 12 Jul 2020 at 06:27, Christian Mauderer <oss at c-mauderer.de> wrote:

> Hello Fitzsimons,
>
> sorry for the late review and thanks Chris for poking me.
>
> I have some troubles understanding the patch and the current code:
>
> == Regarding your patch:
>
> There is no pin GPMC_AD(18) or GPMC_AD(19). That offset will lead you to
> the pins GPMC_A8 and GPMC_A9. The correct macros to use would be
> AM335X_CONF_GPMC_A8 and AM335X_CONF_GPMC_A9. But that's only a style topic.
>
> What's a bit odd is that GPMC_A8 seems to be connected to the LED USR3
> on the BBB. Reasonable output for a PWM. But GPMC_A9 is a HDMI_INT,
> which seems a bit odd


This is my fault for not really understanding the RTEMS header structure
for the beaglebone bsp properly prior to now. I've dug into it a lot more
over the last week or so while I've been working on implementing the eQEP
driver and now have a much better understanding of how all the register
definitions work.

To be honest I found the following two macro definitions a bit unnecessary
but thought I was doing the right thing by following the established
pattern,
/**
 * @brief  BeagleBone Black PWM Macros.
 */
#define BBB_CONTROL_CONF_GPMC_AD(n)   (0x800 + (n * 4))
#define BBB_CONTROL_CONF_LCD_DATA(n)   (0x8a0 + (n * 4))


>
== Regarding the old pins
>
> The old pins on the other hand (GPMC_AD2 and AD3) are connected to
> MMC1_DAT2 and 3 which is on P8 pin 5 and 6. That sounds a lot more
> reasonable. But the pins are definitively not what the pin_no suggests.
>
> == What (maybe) should be the correct one
>
> P9_14 and P9_16 are EHRPWM1A and EHRPWM1B on the beagle schematic. These
> are GPMC_AD8 and GPMC_AD9. These pins have a PWM function as an
> alternative function. So I would expect that you have to use these.
>
> It's really hard to find a documentation from TI for the pinmux but I
> found a list:
>
>
> https://git.ti.com/cgit/sitara-dss-files/am335x-dss-files/tree/padconf/am335x-pinmux.data


A much better and easier to understand resource are the following two
documents from Derek Molloy's Exploring Beaglebone website:

http://exploringbeaglebone.com/wp-content/uploads/2019/01/533160-c06f008.png
http://exploringbeaglebone.com/wp-content/uploads/2019/01/533160-c06f009.png


As you can see from the P9 header sheet, the Address we want for EHRPWM1A
on pin P9_14 is 0x848, and EHRPWM1B on pin P9_16 is 0x84c.

It seems like all the register definitions in the am335x.h header in RTEMS
are using the pin mode 1 name as the #define. So, for  EHRPWM1A  on pin
P9_14 I should have used AM335X_CONF_GPMC_A2, and for  EHRPWM1B on pin
P9_16 I should have used AM335X_CONF_GPMC_A3.
I have just made this change and tested it and it works as expected. I'll
submit an updated patch for your review.

Can you please double check the registers used in your patch
>

BTW - the previous patch did actually calculate the correct register offset
- I confirmed this both by printing the register value in a debug statement
and by physically measuring a PWM output signal on the pins.

Cheers,
James


> Best regards
>
> Christian
>
>
> On 05/07/2020 12:39, James Fitzsimons wrote:
> > ---
> > Fixed incorrect register offset values for EHRPWM1A on P9_14
> >  and EHRPWM1B on P9_16
> >
> >  bsps/arm/beagle/pwm/pwm.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/bsps/arm/beagle/pwm/pwm.c b/bsps/arm/beagle/pwm/pwm.c
> > index 0bc5d125bf..9a346995aa 100644
> > --- a/bsps/arm/beagle/pwm/pwm.c
> > +++ b/bsps/arm/beagle/pwm/pwm.c
> > @@ -102,9 +102,9 @@ bool beagle_pwm_pinmux_setup(bbb_pwm_pin_t pin_no,
> BBB_PWMSS pwm_id)
> >       } else if (pin_no == BBB_P8_36_1A) {
> >         REG(AM335X_PADCONF_BASE + BBB_CONTROL_CONF_LCD_DATA(10)) =
> BBB_MUXMODE(BBB_P8_36_MUX_PWM);
> >       } else if (pin_no == BBB_P9_14_1A) {
> > -       REG(AM335X_PADCONF_BASE + BBB_CONTROL_CONF_GPMC_AD(2)) =
> BBB_MUXMODE(BBB_P9_14_MUX_PWM);
> > +       REG(AM335X_PADCONF_BASE + BBB_CONTROL_CONF_GPMC_AD(18)) =
> BBB_MUXMODE(BBB_P9_14_MUX_PWM);
> >       } else if (pin_no == BBB_P9_16_1B) {
> > -       REG(AM335X_PADCONF_BASE + BBB_CONTROL_CONF_GPMC_AD(3)) =
> BBB_MUXMODE(BBB_P9_16_MUX_PWM);
> > +       REG(AM335X_PADCONF_BASE + BBB_CONTROL_CONF_GPMC_AD(19)) =
> BBB_MUXMODE(BBB_P9_16_MUX_PWM);
> >       } else {
> >         is_valid = false;
> >          }
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20200713/e2d9c38a/attachment-0001.html>