On 5/3/2023 3:11 PM, Andy Shevchenko wrote:
On Wed, May 3, 2023 at 8:39 AM Rohit Agarwal <quic_rohiagar@xxxxxxxxxxx> wrote:
Update the msm_function and msm_pingroup structure to reuse the generic
structures
pinfunction and pingroup structures. Also refactor pinctrl drivers to adjust
the new macro and updated structure defined in pinctrl.h and pinctrl_msm.h
respectively.
Thanks for this, my comments below.
...
#define FUNCTION(fname) \
[APQ_MUX_##fname] = { \
- .name = #fname, \
- .groups = fname##_groups, \
- .ngroups = ARRAY_SIZE(fname##_groups), \
- }
+ .func = PINCTRL_PINFUNCTION(#fname, \
+ fname##_groups, \
+ ARRAY_SIZE(fname##_groups)) \
+ }
Does it really make sense to keep an additional wrapper data type that
does not add any value? Can't we simply have
This was done as part of embeding the pinfunction structure in msm_function.
Will drop this in the next.
#define FUNCTION(fname) [...fname] = PINCTRL_PINFUNCTION(...)
?
...
+ .grp = PINCTRL_PINGROUP("gpio"#id, gpio##id##_pins, \
+ (unsigned int)ARRAY_SIZE(gpio##id##_pins)), \
Why do you need this casting? Same Q to all the rest of the similar cases.
Will drop it.
...
+#include <linux/pinctrl/pinctrl.h>
Keep it separate, and below the generic ones...
Sure
#include <linux/pm.h>
#include <linux/types.h>
...like here (note also a blank line).
...
/**
* struct msm_function - a pinmux function
- * @name: Name of the pinmux function.
- * @groups: List of pingroups for this function.
- * @ngroups: Number of entries in @groups.
+ * @func: Generic data of the pin function (name and groups of pins)
*/
struct msm_function {
- const char *name;
- const char * const *groups;
- unsigned ngroups;
+ struct pinfunction func;
};
But why? Just kill the entire structure.
Got it. Can we have a typedef for pinfunction to msm_function in the msm
header file?
...
#define FUNCTION(fname) \
This definition appears in many files, instead you can make a generic
to this drivers one and use it here
#define QCOM_FUNCTION(_prefix_, _fname_)
[_prefix_##_fname_] = PINCTRL_PINFUNCTION(...)
#define FUNCTION(fname) QCOM_FUNCTION(msm_mux, fname)
(this just a pseudocode, might not even be compilable)
[msm_mux_##fname] = { \
- .name = #fname, \
- .groups = fname##_groups, \
- .ngroups = ARRAY_SIZE(fname##_groups), \
+ .func = PINCTRL_PINFUNCTION(#fname, \
+ fname##_groups, \
+ ARRAY_SIZE(fname##_groups)) \
}
Got your point. Maybe your option 2 of using MSM_PIN_FUNCTION seems more
generic,
as there wont be any redefinition of any macro FUNCTION altogether in
the target specific files.
Thanks,
Rohit.
