On 23/8/20 7:51 pm, James Fitzsimons wrote:
> On Sat, 22 Aug 2020 at 20:41, Chris Johns <chr...@rtems.org
> <mailto:chr...@rtems.org>> wrote:
>
> On 21/8/20 8:01 pm, James Fitzsimons wrote:
> ...
> > +
> > +
> > +typedef void (*bbb_eqep_timer_callback)(BBB_PWMSS);
>
> What is BBB_PWMSS used for as an argument?
>
> The BBB_PWMSS argument is provided to the user call back function so that the
> user knows which QEP module raised the interrupt.
Great, please add a comment to say this.
> It is quite likely that a user
> would have multiple QEP modules configured. Thinking about this now, the call
> back could also have the count provided as an argument so the user doesn't
> need
> to call the "int32_t beagle_qep_get_position(BBB_PWMSS pwmss_id);" function
> from
> their call back handler.
Sounds good. Adding something like this as a comment helps new users to the
code.
> Could a `void* user` be passed back? It helps if the interrupt provides a
> user
> specific data pointer.
>
> I'm sorry I don't understand what you mean here. Do you mean the argument to
> the
> call back should be "void* user" which presumably would be a pointer to an
> arbitrary data structure?
Add another argument that is a 'void*' that is a transparent user set value. A
user provides a pointer or NULL when configuring the QEP module and it is
returned here as an argument. The driver does not touch the value.
> > +/**
> > + * @brief This structure represents an eQEP module instance.
> > + *
> > + * The members are closely modelled on the FDT structure. *
> > + */
> > +typedef struct {
> > + const BBB_PWMSS pwmss_id;
> > + const uint32_t mmio_base;
> > + const rtems_vector_number irq;
> > + bbb_eqep_timer_callback timer_callback;
>
> Is this part of the FDT? I think some extra documentation here would be
> good to
> have.
>
> Good catch - that comment needs updating. When I first added the bbb_eqep
> structure it did closely resemble the structure used in the FDT configuration
> (at least for Linux). However, I then added additional members such as the
> rtems_vector_number and the bbb_eqep_timer_callback which have no relationship
> to the FDT. I can update the comment to more accurately reflect the current
> state.
Thank you.
> Can we have a `void* user;`?
>
> Are you suggesting replacing "bbb_eqep_timer_callback timer_callback;" with
> "void* user;"? If so yes I can do that. Is there a convention for passing
> arguments to that function from the interrupt handler?
No it is additional. I can then provide a private pointer to some private data I
have that is transparently passed back as an argument to the callback handler.
For example I may 2 or more separate pieces of code controlling separate QEP
modules each with a different set of data held in different type structures.
Configuing a pointer per QEP modules means I do not need to collect and hold
these pointers in global data structures I need to lock somehow to access from
the callback handler. I simply cast the `void*` to my data and then use it.
>
> Many thanks for your review and comments!
>
No problem and thank you. It is close.
Chris
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
