| 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:
<PUZPR06MB5887C7BA551EBAFC05564400EF1FA@PUZPR06MB5887.apcprd06.prod.outlook.com>
Date: Thu, 25 Sep 2025 10:34:44 +0000
From: Gary Yang <Gary.Yang@...tech.com>
To: Linus Walleij <linus.walleij@...aro.org>
CC: "robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, cix-kernel-upstream
<cix-kernel-upstream@...tech.com>
Subject:
回复: [v2 1/3] pinctrl: cix: Add pin-controller support for sky1
Hi Linus,
>
> New scheme with macros has verified ok. I just want to confirm with you
> before submit codes
>
> > EXTERNAL EMAIL
> >
> > On Mon, Sep 15, 2025 at 9:09 AM Gary Yang <gary.yang@...tech.com>
> wrote:
> >
> > > > Using a NULL func->name to terminate the array looks a bit dangerous.
> > >
> > > Checking whether this pointer is a null pointer is generally
> > > acceptable. A
> > name maps to a mux value.
> > > I think that it is safe. Of course, your suggestion is also a good
> > > idea. If you think this is not safe, we will change codes as your suggestions.
> >
> > It's OK just a suggestion. There are many ways to do this, first fix
> > other problems.
> >
> > There are things in the language and the kernel that can help you to
> > check boundaries of arrays such as these functions so you can't write
> > code that index out of range, e.g.
> >
> > +struct sky1_pin_desc {
> > + const struct pinctrl_pin_desc pin;
> > + const struct sky1_function_desc functions[4]; };
> > +
> > +struct sky1_pinctrl_soc_info {
> > + const struct sky1_pin_desc *pins;
> > + unsigned int npins;
> > +};
> >
> > It is possible to use a flexible array with the intrinsic __counted by() here, e.g.
> > instead of:
> >
> > struct sky1_pin_desc {
> > const struct pinctrl_pin_desc pin;
> > const struct sky1_function_desc functions[4];
> >
> > You can use:
> >
> > + size_t nfunctions;
> > + const struct sky1_function_desc functions[]
> > + __counted_by(nfunctions);
> >
> > If you grep counted_by in the kernel you find many other examples of
> > how we use this.
> >
> > But flexible arrays is a bit complicated and dangerous so maybe you
> > want to avoid it altogether. Also I'm not sure it works when you put
> > things containing a flexible array into another array... I never tried it.
> >
> > > > Then you can use nfuncs to iterate over the array of function
> > > > names, and define a macro like this:
> > > >
> > > > #define SKY_PINFUNCTION(_muxval, _functions, _nfunctions) \
> > > > (struct sky1_function_desc) { \
> > > > .muxval = (muxval), \
> > > > .functions = (_functions), \
> > > > .nfuncs = (_nfunctions), \
> > > > }
> > > >
> > > > And then this:
> > > >
> > > > +static const struct sky1_pin_desc sky1_pinctrl_s5_pads[] = {
> > > > > + {
> > > > > + .pin = PINCTRL_PIN(0, "GPIO1"),
> > > > > + .functions = {
> > > > > + [0] = {0, "GPIO1"},
> > > > > + },
> > > > > + },
> > > > > + {
> > > > > + .pin = PINCTRL_PIN(1, "GPIO2"),
> > > > > + .functions = {
> > > > > + [0] = {0, "GPIO2"},
> > > > > + },
> > > >
> > > > > + },
> > > >
> > > > becomes
> > > >
> > > > static const struct sky1_pin_desc sky1_pinctrl_s5_pads[] = {
> > > > SKY_PINFUNCTION(PINCTRL_PIN(0, "GPIO1"), "GPIO1", 1),
> > > > SKY_PINFUNCTION(PINCTRL_PIN(1, "GPIO2"), "GPIO2", 1),
> > > >
> > > > I don't know about using the PINCTRL_PIN() macro here though,
> > > > can't you just put in 0, 1...?
> > > >
> > > > Anyway I think you get the idea.
> > > >
> > >
> > > First, let us review the struct sky1_pin_desc, it contains two
> > > members, one is
> > the struct pinctrl_pin_desc.
> > >
> > > struct pinctrl_pin_desc {
> > > unsigned int number;
> > > const char *name;
> > > void *drv_data;
> > > };
> > >
> > > PINCTRL_PIN is used to initialize this struct in kernel. It locates
> > > in include/linux/pinctrl/pinctrl.h
> > >
> > > #define PINCTRL_PIN(a, b) { .number = a, .name = b }
> > >
> > > PINCTRL_PIN(0, "GPIO1") defines a pin, its number is 0, its name is
> "GPIO1".
> >
> > Ah I saw it wrong, sorry :(
> >
> > You're right about this of course.
> >
> > But I think you can still use a macro to define the long pin tables?
> > Albeit macros with flexible arguments is a bit hard to write.
> > Save it until everything else is working.
> >
> In header file:
>
> struct sky1_pin_desc {
> const struct pinctrl_pin_desc pin;
> const char **func_group;
> unsigned int nfunc;
> };
>
> #define SKY_PINFUNCTION(_pin, _func) \
> (struct sky1_pin_desc) { \
> .pin = _pin, \
> .func_group = _func##_group, \
> .nfunc = ARRAY_SIZE(_func##_group), \
> }
>
> In C file:
>
> static const char *gpio1_group[] = {"GPIO1"}; static const char *gpio2_group[]
> = {"GPIO2"};
>
> static const struct sky1_pin_desc sky1_pinctrl_s5_pads[] = {
> SKY_PINFUNCTION(PINCTRL_PIN(0, "GPIO1"), gpio1),
> SKY_PINFUNCTION(PINCTRL_PIN(1, "GPIO2"), gpio2),
> .......
> };
>
> What's your suggestion? Thanks
>
Ping....
> >
> > Yours,
> > Linus Walleij
>
Best wishes
Gary
Powered by blists - more mailing lists