lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAHp75VegxMgAamS3ORiJ2=D4MH7asD9PiWrM+3JAm-QOuEgcrg@mail.gmail.com> Date: Wed, 3 May 2023 12:41:56 +0300 From: Andy Shevchenko <andy.shevchenko@...il.com> To: Rohit Agarwal <quic_rohiagar@...cinc.com> Cc: agross@...nel.org, andersson@...nel.org, konrad.dybcio@...aro.org, linus.walleij@...aro.org, robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org, richardcochran@...il.com, manivannan.sadhasivam@...aro.org, linux-arm-msm@...r.kernel.org, linux-gpio@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, netdev@...r.kernel.org Subject: Re: [PATCH v5 2/3] pinctrl: qcom: Refactor target specific pinctrl driver On Wed, May 3, 2023 at 8:39 AM Rohit Agarwal <quic_rohiagar@...cinc.com> 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 #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. ... > +#include <linux/pinctrl/pinctrl.h> Keep it separate, and below the generic ones... > #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. ... > #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)) \ > } -- With Best Regards, Andy Shevchenko
Powered by blists - more mailing lists