Hi Christian, Thanks very much for that, much appreciated.
Cheers, James On Tue, 14 Jul 2020, 03:52 Christian Mauderer, <o...@c-mauderer.de> wrote: > > > On 13/07/2020 11:55, James Fitzsimons wrote: > > Hi Christian and Chris, > > > > On Sun, 12 Jul 2020 at 06:27, Christian Mauderer <o...@c-mauderer.de > > <mailto:o...@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)) > > > > The BBB headers have grown a bit. It's a cheap platform and therefore it > has been used during quite some GSoC and entry level projects. With that > the files have been written by much more authors then most other BSPs. > The headers most likely would need some clean up to get a consistent > structure. > > > > > > > == 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. > > You are right. I'm not sure where I got my offset wrong. Of course it > should be GPMC_A2 and GPMC_A3. > > Your new patch is on top of the first one (replaces ...AD(18) with > ...A2). I'll squash both patches into one and push that one. > > Best regards > > Christian > > > > > 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; > > > } > > > > > >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
