diff mbox series

[V2,4/4] pinctrl: add one more "const" for generic function groups

Message ID 20211216162206.8027-4-zajec5@gmail.com
State Accepted
Headers show
Series [V2,1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const | expand

Commit Message

Rafał Miłecki Dec. 16, 2021, 4:22 p.m. UTC 
From: Rafał Miłecki <rafal@milecki.pl>

Generic code doesn't modify those strings and .get_function_groups
callback has that extra "const" as well. This allows more flexibility in
GENERIC_PINMUX_FUNCTIONS users.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/pinctrl/pinmux.c | 2 +-
 drivers/pinctrl/pinmux.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Nathan Chancellor Jan. 11, 2022, 3:34 p.m. UTC | #1 
Hi Rafał,

On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Generic code doesn't modify those strings and .get_function_groups
> callback has that extra "const" as well. This allows more flexibility in
> GENERIC_PINMUX_FUNCTIONS users.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  drivers/pinctrl/pinmux.c | 2 +-
>  drivers/pinctrl/pinmux.h | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> index 6cdbd9ccf2f0..f94d43b082d9 100644
> --- a/drivers/pinctrl/pinmux.c
> +++ b/drivers/pinctrl/pinmux.c
> @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
>   */
>  int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>  				const char *name,
> -				const char **groups,
> +				const char * const *groups,
>  				const unsigned int num_groups,
>  				void *data)
>  {
> diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
> index 78c3a31be882..72fcf03eaa43 100644
> --- a/drivers/pinctrl/pinmux.h
> +++ b/drivers/pinctrl/pinmux.h
> @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
>   */
>  struct function_desc {
>  	const char *name;
> -	const char **group_names;
> +	const char * const *group_names;
>  	int num_group_names;
>  	void *data;
>  };
> @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
>  
>  int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>  				const char *name,
> -				const char **groups,
> +				const char * const *groups,
>  				unsigned const num_groups,
>  				void *data);
>  
> -- 
> 2.31.1
> 
> 

I have not seen this reported yet, even though it has been broken for a
couple of weeks now. I see the following error in -next:

$ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
  815 |                         grp = func->group_names;
      |                             ^
cc1: all warnings being treated as errors

Looks like something like the third patch of the series is needed for
the Thunderbay driver, which it appears was in development at the same
time as this series.

Cheers,
Nathan
Rafał Miłecki Jan. 11, 2022, 4:51 p.m. UTC | #2 
On 11.01.2022 16:34, Nathan Chancellor wrote:
> On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Generic code doesn't modify those strings and .get_function_groups
>> callback has that extra "const" as well. This allows more flexibility in
>> GENERIC_PINMUX_FUNCTIONS users.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>   drivers/pinctrl/pinmux.c | 2 +-
>>   drivers/pinctrl/pinmux.h | 4 ++--
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
>> index 6cdbd9ccf2f0..f94d43b082d9 100644
>> --- a/drivers/pinctrl/pinmux.c
>> +++ b/drivers/pinctrl/pinmux.c
>> @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
>>    */
>>   int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>>   				const char *name,
>> -				const char **groups,
>> +				const char * const *groups,
>>   				const unsigned int num_groups,
>>   				void *data)
>>   {
>> diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
>> index 78c3a31be882..72fcf03eaa43 100644
>> --- a/drivers/pinctrl/pinmux.h
>> +++ b/drivers/pinctrl/pinmux.h
>> @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
>>    */
>>   struct function_desc {
>>   	const char *name;
>> -	const char **group_names;
>> +	const char * const *group_names;
>>   	int num_group_names;
>>   	void *data;
>>   };
>> @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
>>   
>>   int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>>   				const char *name,
>> -				const char **groups,
>> +				const char * const *groups,
>>   				unsigned const num_groups,
>>   				void *data);
>>   
>> -- 
>> 2.31.1
>>
>>
> 
> I have not seen this reported yet, even though it has been broken for a
> couple of weeks now. I see the following error in -next:
> 
> $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
> drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
> drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
>    815 |                         grp = func->group_names;
>        |                             ^
> cc1: all warnings being treated as errors
> 
> Looks like something like the third patch of the series is needed for
> the Thunderbay driver, which it appears was in development at the same
> time as this series.

Correct, this driver didn't exist in Linus's tree when I developed my changes.

Too bad thunderbay copies that old & complex logic that I just fixed in the keembay driver. I'll have to redo my changes for the thunderbay now.

I don't agree with the idea of reverting my patchset and working on V3 though. It's a relatively simple thing we need to fix, it just be just a follow-up commit.
Linus Walleij Jan. 16, 2022, 12:51 a.m. UTC | #3 
On Tue, Jan 11, 2022 at 5:51 PM Rafał Miłecki <zajec5@gmail.com> wrote:

> > Looks like something like the third patch of the series is needed for
> > the Thunderbay driver, which it appears was in development at the same
> > time as this series.
>
> Correct, this driver didn't exist in Linus's tree when I developed my changes.
>
> Too bad thunderbay copies that old & complex logic that I just fixed in the keembay driver. I'll have to redo my changes for the thunderbay now.
>
> I don't agree with the idea of reverting my patchset and working on V3 though. It's a relatively simple thing we need to fix, it just be just a follow-up commit.

Yeah this kind of things happens, we just deal with it throughout the -rc:s
no big deal. Release candidates exist for a reason. Let's make a fixup patch.

Yours,
Linus Walleij
Uwe Kleine-König Jan. 18, 2022, 7:02 a.m. UTC | #4 
On Tue, Jan 11, 2022 at 05:51:44PM +0100, Rafał Miłecki wrote:
> On 11.01.2022 16:34, Nathan Chancellor wrote:
> > On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
> > > From: Rafał Miłecki <rafal@milecki.pl>
> > > 
> > > Generic code doesn't modify those strings and .get_function_groups
> > > callback has that extra "const" as well. This allows more flexibility in
> > > GENERIC_PINMUX_FUNCTIONS users.
> > > 
> > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > > ---
> > >   drivers/pinctrl/pinmux.c | 2 +-
> > >   drivers/pinctrl/pinmux.h | 4 ++--
> > >   2 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> > > index 6cdbd9ccf2f0..f94d43b082d9 100644
> > > --- a/drivers/pinctrl/pinmux.c
> > > +++ b/drivers/pinctrl/pinmux.c
> > > @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
> > >    */
> > >   int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
> > >   				const char *name,
> > > -				const char **groups,
> > > +				const char * const *groups,
> > >   				const unsigned int num_groups,
> > >   				void *data)
> > >   {
> > > diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
> > > index 78c3a31be882..72fcf03eaa43 100644
> > > --- a/drivers/pinctrl/pinmux.h
> > > +++ b/drivers/pinctrl/pinmux.h
> > > @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
> > >    */
> > >   struct function_desc {
> > >   	const char *name;
> > > -	const char **group_names;
> > > +	const char * const *group_names;
> > >   	int num_group_names;
> > >   	void *data;
> > >   };
> > > @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
> > >   int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
> > >   				const char *name,
> > > -				const char **groups,
> > > +				const char * const *groups,
> > >   				unsigned const num_groups,
> > >   				void *data);
> > > -- 
> > > 2.31.1
> > > 
> > > 
> > 
> > I have not seen this reported yet, even though it has been broken for a
> > couple of weeks now. I see the following error in -next:
> > 
> > $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
> > drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
> > drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
> >    815 |                         grp = func->group_names;
> >        |                             ^
> > cc1: all warnings being treated as errors
> > 
> > Looks like something like the third patch of the series is needed for
> > the Thunderbay driver, which it appears was in development at the same
> > time as this series.
> 
> Correct, this driver didn't exist in Linus's tree when I developed my changes.

I stumbled above this issue, too. For the record, this patch fixes the
build issue:

diff --git a/drivers/pinctrl/pinctrl-thunderbay.c b/drivers/pinctrl/pinctrl-thunderbay.c
index b5b47f4dd774..a6a9a0cca6bf 100644
--- a/drivers/pinctrl/pinctrl-thunderbay.c
+++ b/drivers/pinctrl/pinctrl-thunderbay.c
@@ -812,7 +812,7 @@ static int thunderbay_add_functions(struct thunderbay_pinctrl *tpc, struct funct
 				}
 			}
 
-			grp = func->group_names;
+			grp = (void *)func->group_names;
 			while (*grp)
 				grp++;
 
(however it's probably not safe for runtime).

Best regards
Uwe
Rafał Miłecki Jan. 18, 2022, 7:21 a.m. UTC | #5 
On 18.01.2022 08:02, Uwe Kleine-König wrote:
> On Tue, Jan 11, 2022 at 05:51:44PM +0100, Rafał Miłecki wrote:
>> On 11.01.2022 16:34, Nathan Chancellor wrote:
>>> On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> Generic code doesn't modify those strings and .get_function_groups
>>>> callback has that extra "const" as well. This allows more flexibility in
>>>> GENERIC_PINMUX_FUNCTIONS users.
>>>>
>>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>>> ---
>>>>    drivers/pinctrl/pinmux.c | 2 +-
>>>>    drivers/pinctrl/pinmux.h | 4 ++--
>>>>    2 files changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
>>>> index 6cdbd9ccf2f0..f94d43b082d9 100644
>>>> --- a/drivers/pinctrl/pinmux.c
>>>> +++ b/drivers/pinctrl/pinmux.c
>>>> @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
>>>>     */
>>>>    int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>>>>    				const char *name,
>>>> -				const char **groups,
>>>> +				const char * const *groups,
>>>>    				const unsigned int num_groups,
>>>>    				void *data)
>>>>    {
>>>> diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
>>>> index 78c3a31be882..72fcf03eaa43 100644
>>>> --- a/drivers/pinctrl/pinmux.h
>>>> +++ b/drivers/pinctrl/pinmux.h
>>>> @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
>>>>     */
>>>>    struct function_desc {
>>>>    	const char *name;
>>>> -	const char **group_names;
>>>> +	const char * const *group_names;
>>>>    	int num_group_names;
>>>>    	void *data;
>>>>    };
>>>> @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
>>>>    int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>>>>    				const char *name,
>>>> -				const char **groups,
>>>> +				const char * const *groups,
>>>>    				unsigned const num_groups,
>>>>    				void *data);
>>>> -- 
>>>> 2.31.1
>>>>
>>>>
>>>
>>> I have not seen this reported yet, even though it has been broken for a
>>> couple of weeks now. I see the following error in -next:
>>>
>>> $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
>>> drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
>>> drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
>>>     815 |                         grp = func->group_names;
>>>         |                             ^
>>> cc1: all warnings being treated as errors
>>>
>>> Looks like something like the third patch of the series is needed for
>>> the Thunderbay driver, which it appears was in development at the same
>>> time as this series.
>>
>> Correct, this driver didn't exist in Linus's tree when I developed my changes.
> 
> I stumbled above this issue, too. For the record, this patch fixes the
> build issue:

[PATCH 5.17 1/2] pinctrl: thunderbay: comment process of building functions a bit
[PATCH 5.17 2/2] pinctrl: thunderbay: rework loops looking for groups names
https://patchwork.ozlabs.org/project/linux-gpio/list/?series=280568

Patches already queued in the:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=fixes
Uwe Kleine-König Jan. 18, 2022, 7:31 a.m. UTC | #6 
On Tue, Jan 18, 2022 at 08:21:55AM +0100, Rafał Miłecki wrote:
> On 18.01.2022 08:02, Uwe Kleine-König wrote:
> > On Tue, Jan 11, 2022 at 05:51:44PM +0100, Rafał Miłecki wrote:
> > > On 11.01.2022 16:34, Nathan Chancellor wrote:
> > > > On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
> > > > > From: Rafał Miłecki <rafal@milecki.pl>
> > > > > 
> > > > > Generic code doesn't modify those strings and .get_function_groups
> > > > > callback has that extra "const" as well. This allows more flexibility in
> > > > > GENERIC_PINMUX_FUNCTIONS users.
> > > > > 
> > > > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > > > > ---
> > > > >    drivers/pinctrl/pinmux.c | 2 +-
> > > > >    drivers/pinctrl/pinmux.h | 4 ++--
> > > > >    2 files changed, 3 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> > > > > index 6cdbd9ccf2f0..f94d43b082d9 100644
> > > > > --- a/drivers/pinctrl/pinmux.c
> > > > > +++ b/drivers/pinctrl/pinmux.c
> > > > > @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
> > > > >     */
> > > > >    int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
> > > > >    				const char *name,
> > > > > -				const char **groups,
> > > > > +				const char * const *groups,
> > > > >    				const unsigned int num_groups,
> > > > >    				void *data)
> > > > >    {
> > > > > diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
> > > > > index 78c3a31be882..72fcf03eaa43 100644
> > > > > --- a/drivers/pinctrl/pinmux.h
> > > > > +++ b/drivers/pinctrl/pinmux.h
> > > > > @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
> > > > >     */
> > > > >    struct function_desc {
> > > > >    	const char *name;
> > > > > -	const char **group_names;
> > > > > +	const char * const *group_names;
> > > > >    	int num_group_names;
> > > > >    	void *data;
> > > > >    };
> > > > > @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
> > > > >    int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
> > > > >    				const char *name,
> > > > > -				const char **groups,
> > > > > +				const char * const *groups,
> > > > >    				unsigned const num_groups,
> > > > >    				void *data);
> > > > > -- 
> > > > > 2.31.1
> > > > > 
> > > > > 
> > > > 
> > > > I have not seen this reported yet, even though it has been broken for a
> > > > couple of weeks now. I see the following error in -next:
> > > > 
> > > > $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
> > > > drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
> > > > drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
> > > >     815 |                         grp = func->group_names;
> > > >         |                             ^
> > > > cc1: all warnings being treated as errors
> > > > 
> > > > Looks like something like the third patch of the series is needed for
> > > > the Thunderbay driver, which it appears was in development at the same
> > > > time as this series.
> > > 
> > > Correct, this driver didn't exist in Linus's tree when I developed my changes.
> > 
> > I stumbled above this issue, too. For the record, this patch fixes the
> > build issue:
> 
> [PATCH 5.17 1/2] pinctrl: thunderbay: comment process of building functions a bit
> [PATCH 5.17 2/2] pinctrl: thunderbay: rework loops looking for groups names
> https://patchwork.ozlabs.org/project/linux-gpio/list/?series=280568
> 
> Patches already queued in the:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=fixes

Ah great. I hope this makes it into Linus Torvald's tree before -rc1 is
tagged. Having allmodconfig build broken in release candidates is really
annoying.

Best regards
Uwe
diff mbox series

Patch

diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 6cdbd9ccf2f0..f94d43b082d9 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -875,7 +875,7 @@  EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
  */
 int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
 				const char *name,
-				const char **groups,
+				const char * const *groups,
 				const unsigned int num_groups,
 				void *data)
 {
diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
index 78c3a31be882..72fcf03eaa43 100644
--- a/drivers/pinctrl/pinmux.h
+++ b/drivers/pinctrl/pinmux.h
@@ -129,7 +129,7 @@  static inline void pinmux_init_device_debugfs(struct dentry *devroot,
  */
 struct function_desc {
 	const char *name;
-	const char **group_names;
+	const char * const *group_names;
 	int num_group_names;
 	void *data;
 };
@@ -150,7 +150,7 @@  struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
 
 int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
 				const char *name,
-				const char **groups,
+				const char * const *groups,
 				unsigned const num_groups,
 				void *data);