mbox series

[v2,0/4] pinctrl: s32: driver improvements and generic struct use

Message ID 20230320163823.886-1-clin@suse.com
Headers
Series pinctrl: s32: driver improvements and generic struct use |

Message

Chester Lin March 20, 2023, 4:38 p.m. UTC 
  Hello,

This patch series contains some improvements for s32 pinctrl drivers suggested
by upstream[1], such as
  - Fix error shadowings and improve return value handlings.
  - Fix print format.
  - Remove unnecessary blanks.
  - Use proper macros and helpers to simplify codes.
  - Refactor config param parsing and remove config arguments that are never used.
  - Use generic struct pingroup and struct pinfunction to describe pin data.

Regards,
Chester

[1] https://lore.kernel.org/all/20230220023320.3499-1-clin@suse.com/

Changes in v2:
- Use of_device_get_match_data() to get matched of_device_id data.
- Enhance sizeof() arguments.
- Fix blanks and remove unnecessary parentheses.
- Drop unnecessary marcos and s32_pin_config() implemented in v1 and set/clear
  mask/config values transparently.
- Put pull-function related cases together in s32_pin_set_pull().
- Simply use generic 'struct pinfunction' rather than having extra 'struct
  s32_pmx_func'.

Chester Lin (4):
  pinctrl: s32: use of_device_get_match_data() to get device data
  pinctrl: s32: refine error/return/config checks and simplify driver
    codes
  pinctrl: s32cc: refactor pin config parsing
  pinctrl: s32cc: embed generic struct pingroup and pinfunction

 drivers/pinctrl/nxp/pinctrl-s32.h   |  26 +--
 drivers/pinctrl/nxp/pinctrl-s32cc.c | 261 +++++++++++++++-------------
 drivers/pinctrl/nxp/pinctrl-s32g2.c |  14 +-
 3 files changed, 152 insertions(+), 149 deletions(-)

Comments

Andy Shevchenko March 21, 2023, 7:39 a.m. UTC | #1 
On Tue, Mar 21, 2023 at 7:09 AM Chester Lin <clin@suse.com> wrote:
> On Mon, Mar 20, 2023 at 07:10:40PM +0200, Andy Shevchenko wrote:
> > On Mon, Mar 20, 2023 at 6:39 PM Chester Lin <clin@suse.com> wrote:

...

> > >         for_each_child_of_node(np, child) {
> > > -               func->groups[i] = child->name;
> > > +               groups[i] = (char *)child->name;

Here is also questionable casting.

...

> > > +       func->groups = (const char **)groups;
> >
> > Hmm... Why is casting needed?
>
> It's used for fulfilling the type checking done by kbuild otherwise an error will occur:
>
> drivers/pinctrl/nxp/pinctrl-s32cc.c:815:22: error: assignment to 'const char * const*' from incompatible pointer type 'char **' [-Werror=incompatible-pointer-types]
>
> In 'struct pinfunction', the member 'groups' is declared as (const char * const *).

So, please decouple `struct pingroup` change to a separate patch and
hence `struct pinfunction` on its own.

After, consider changing types elsewhere that are following the types
in that data structures.