New build warnings

New build warnings

[Russell King - ARM Linux]

Okay, so last night's build run shows some new warnings...

drivers/regulator/twl-regulator.c: In function 'twlreg_probe':
drivers/regulator/twl-regulator.c:1151: warning: assignment discards qualifiers from pointer target type
drivers/regulator/twl-regulator.c:1160: warning: assignment discards qualifiers from pointer target type

These two look like missing const - looks like they were introduced by:

commit 98d7bbb9929bcc14e11ac8a55245a4f2dc174e17
Author: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date:   Thu Jun 7 12:20:14 2012 +0200

    of: add const to struct *of_device_id.data

    Drivers should never need to modify the data of a device id. So it can
    be const which in turn allows more consts in the driver.

    Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>




drivers/regulator/twl-regulator.c: At top level:
drivers/regulator/twl-regulator.c:1051: warning: 'TWLRES_INFO_CLK32KG' defined but not used

This one looks like it was caused by:

commit 0e8e5c34cf1a8beaaf0a6a05c053592693bf8cb4
Author: Mark Brown <broonie@opensource.wolfsonmicro.com>
Date:   Mon Jun 4 17:23:13 2012 +0100

    regulator: twl: Remove references to 32kHz clock from DT bindings

    Due to the lack of a generic clock API we'd had the 32kHz clock in the
    regulator driver but this is definitely a Linux-specific thing and now
    we have a clock API hopefully the code can be moved elsewhere. Try to
    avoid getting DTs deployed relying on the 32kHz clock by removing it
    from the bindings, grep seems to tell me it's not currently used anyway.

    Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

which removes:

-#define TWLRES_OF_MATCH(comp, label) TWL_OF_MATCH(comp, TWLRES, label)
-       TWLRES_OF_MATCH("ti,twl6030-clk32kg", CLK32KG),

but completely forgets that this:

TWL6030_FIXED_RESOURCE(CLK32KG, 0x8C, 0);

defines a structure that it was using.  (Welcome to why macros are horrid
when they hide the names of stuff they create.)
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: New build warnings

[Mark Brown]

On Mon, Jul 30, 2012 at 05:44:44PM +0100, Russell King - ARM Linux wrote:

> -#define TWLRES_OF_MATCH(comp, label) TWL_OF_MATCH(comp, TWLRES, label)
> -       TWLRES_OF_MATCH("ti,twl6030-clk32kg", CLK32KG),

> but completely forgets that this:

> TWL6030_FIXED_RESOURCE(CLK32KG, 0x8C, 0);

> defines a structure that it was using.  (Welcome to why macros are horrid
> when they hide the names of stuff they create.)

I actually have the fix in a separate patch which completely removes
the "regulator" but wasn't happy about pushing the patch for 3.6 since I
can't test (I don't think anyone's ever actually used those and
obviously they should be using the clk API now).

Re: New build warnings

[Russell King - ARM Linux]

On Mon, Jul 30, 2012 at 06:40:43PM +0100, Mark Brown wrote:
> On Mon, Jul 30, 2012 at 05:44:44PM +0100, Russell King - ARM Linux wrote:
> 
> > -#define TWLRES_OF_MATCH(comp, label) TWL_OF_MATCH(comp, TWLRES, label)
> > -       TWLRES_OF_MATCH("ti,twl6030-clk32kg", CLK32KG),
> 
> > but completely forgets that this:
> 
> > TWL6030_FIXED_RESOURCE(CLK32KG, 0x8C, 0);
> 
> > defines a structure that it was using.  (Welcome to why macros are horrid
> > when they hide the names of stuff they create.)
> 
> I actually have the fix in a separate patch which completely removes
> the "regulator" but wasn't happy about pushing the patch for 3.6 since I
> can't test (I don't think anyone's ever actually used those and
> obviously they should be using the clk API now).

If the structure is unused, the fix is to delete the
"TWL6030_FIXED_RESOURCE(CLK32KG, 0x8C, 0);" line because nothing is using
that (if it was, we wouldn't end up with the warning).

Re: New build warnings

[Mark Brown]

On Mon, Jul 30, 2012 at 07:06:23PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 30, 2012 at 06:40:43PM +0100, Mark Brown wrote:

> > I actually have the fix in a separate patch which completely removes
> > the "regulator" but wasn't happy about pushing the patch for 3.6 since I
> > can't test (I don't think anyone's ever actually used those and
> > obviously they should be using the clk API now).

> If the structure is unused, the fix is to delete the
> "TWL6030_FIXED_RESOURCE(CLK32KG, 0x8C, 0);" line because nothing is using
> that (if it was, we wouldn't end up with the warning).

Well, clearly.  I had thought there were implicit references somewhere
but apparently not since it's not in an array or anything.  Oh, well.

Re: New build warnings

[Russell King - ARM Linux]

On Mon, Jul 30, 2012 at 05:44:44PM +0100, Russell King - ARM Linux wrote:
> Okay, so last night's build run shows some new warnings...
> 
> drivers/regulator/twl-regulator.c: In function 'twlreg_probe':
> drivers/regulator/twl-regulator.c:1151: warning: assignment discards qualifiers from pointer target type
> drivers/regulator/twl-regulator.c:1160: warning: assignment discards qualifiers from pointer target type
> 
> These two look like missing const - looks like they were introduced by:
> 
> commit 98d7bbb9929bcc14e11ac8a55245a4f2dc174e17
> Author: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Date:   Thu Jun 7 12:20:14 2012 +0200
> 
>     of: add const to struct *of_device_id.data
> 
>     Drivers should never need to modify the data of a device id. So it can
>     be const which in turn allows more consts in the driver.
> 
>     Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>     Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

I notice this one is still present in the kernel.  Uwe, what's happening
with it?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: New build warnings

[Uwe Kleine-König]

On Sat, Sep 08, 2012 at 09:38:49AM +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 30, 2012 at 05:44:44PM +0100, Russell King - ARM Linux wrote:
> > Okay, so last night's build run shows some new warnings...
> > 
> > drivers/regulator/twl-regulator.c: In function 'twlreg_probe':
> > drivers/regulator/twl-regulator.c:1151: warning: assignment discards qualifiers from pointer target type
> > drivers/regulator/twl-regulator.c:1160: warning: assignment discards qualifiers from pointer target type
> > 
> > These two look like missing const - looks like they were introduced by:
> > 
> > commit 98d7bbb9929bcc14e11ac8a55245a4f2dc174e17
> > Author: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Date:   Thu Jun 7 12:20:14 2012 +0200
> > 
> >     of: add const to struct *of_device_id.data
> > 
> >     Drivers should never need to modify the data of a device id. So it can
> >     be const which in turn allows more consts in the driver.
> > 
> >     Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >     Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> I notice this one is still present in the kernel.  Uwe, what's happening
> with it?
Yeah, the warning was introduced by my patch. It's not trivial to fix
because the twl-regulator driver modifies the data used to probe devices
during probe. That is the result of two probes might depend on their
order. I mentioned that problem in the cover letter of my series but got
no feedback from someone feeling responsible for this driver.
Arnd sent a patch for that problem, don't know its state though.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: New build warnings

[Mark Brown]

On Sun, Sep 09, 2012 at 09:12:40PM +0200, Uwe Kleine-König wrote:
> On Sat, Sep 08, 2012 at 09:38:49AM +0100, Russell King - ARM Linux wrote:

> > >     of: add const to struct *of_device_id.data
> > > 
> > >     Drivers should never need to modify the data of a device id. So it can
> > >     be const which in turn allows more consts in the driver.

> > I notice this one is still present in the kernel.  Uwe, what's happening
> > with it?

> Yeah, the warning was introduced by my patch. It's not trivial to fix
> because the twl-regulator driver modifies the data used to probe devices
> during probe. That is the result of two probes might depend on their
> order. I mentioned that problem in the cover letter of my series but got
> no feedback from someone feeling responsible for this driver.
> Arnd sent a patch for that problem, don't know its state though.

I don't recall such a patch being sent to me.  The twl-regulator driver
is unmaintained so it's unlikely anyone is going to actively work on it.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: New build warnings

[Uwe Kleine-König]

Hi Mark,

On Mon, Sep 10, 2012 at 07:33:34AM +0800, Mark Brown wrote:
> On Sun, Sep 09, 2012 at 09:12:40PM +0200, Uwe Kleine-König wrote:
> > On Sat, Sep 08, 2012 at 09:38:49AM +0100, Russell King - ARM Linux wrote:
> 
> > > >     of: add const to struct *of_device_id.data
> > > > 
> > > >     Drivers should never need to modify the data of a device id. So it can
> > > >     be const which in turn allows more consts in the driver.
> 
> > > I notice this one is still present in the kernel.  Uwe, what's happening
> > > with it?
> 
> > Yeah, the warning was introduced by my patch. It's not trivial to fix
> > because the twl-regulator driver modifies the data used to probe devices
> > during probe. That is the result of two probes might depend on their
> > order. I mentioned that problem in the cover letter of my series but got
> > no feedback from someone feeling responsible for this driver.
> > Arnd sent a patch for that problem, don't know its state though.
> 
> I don't recall such a patch being sent to me.  The twl-regulator driver
> is unmaintained so it's unlikely anyone is going to actively work on it.
Message-id: 201208141241.51379.arnd@arndb.de

Mark, you even replied that you applied it!? It wasn't sent to a mailing
list though.

Uwe
-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: New build warnings

[Mark Brown]

On Mon, Sep 10, 2012 at 08:40:02AM +0200, Uwe Kleine-König wrote:

> Message-id: 201208141241.51379.arnd@arndb.de

This is illegible, did the message have a subject line or other content
that a human might understand?  Just as with commit IDs you should
*always* provide a human readable version, message IDs are even worse
than commit IDs in this respect.

> Mark, you even replied that you applied it!? It wasn't sent to a mailing
> list though.

Well, as I've no idea what you're talking about it's hard for me to
take a look.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: New build warnings

[Uwe Kleine-König]

On Mon, Sep 10, 2012 at 02:42:37PM +0800, Mark Brown wrote:
> On Mon, Sep 10, 2012 at 08:40:02AM +0200, Uwe Kleine-König wrote:
> 
> > Message-id: 201208141241.51379.arnd@arndb.de
> 
> This is illegible, did the message have a subject line or other content
> that a human might understand?  Just as with commit IDs you should
> *always* provide a human readable version, message IDs are even worse
> than commit IDs in this respect.
The full headers are:

Date: Tue, 14 Aug 2012 12:41:50 +0000
From: Arnd Bergmann <arnd@arndb.de>
To: Axel Lin <axel.lin@gmail.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>, Rajendra Nayak <rnayak@ti.com>, Tero Kristo <t-kristo@ti.com>, Uwe  Kleine-König <u.kleine-koenig@pengutronix.de> 
Subject: [PATCH] regulator: twl: make twl_info tables const
Message-Id: <201208141241.51379.arnd@arndb.de>

It used to be possible to get a posting by message id via:

	http://mid.gmane.org/201208141241.51379.arnd@arndb.de

but as this message was not send to a mailing list this doesn't work
here and even for public mails this failed most of the time for me in
the past.

Other than that in Mutt you can search using

	~i 201208141241.51379.arnd@arndb.de

as search string, for notmuch it's

	id:201208141241.51379.arnd@arndb.de

.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: New build warnings

[Mark Brown]

On Mon, Sep 10, 2012 at 09:05:47AM +0200, Uwe Kleine-König wrote:

> Subject: [PATCH] regulator: twl: make twl_info tables const

This is in my drivers branch and in -next, according to git it was
applied on Tuesday, so I'm not sure what you're querying here?

> It used to be possible to get a posting by message id via:
> Other than that in Mutt you can search using
> as search string, for notmuch it's

All of which rely on some combination of live internet access or a copy
of the message and don't work terribly well if those are missing due to
using a mailing list archive or whatever (and even when they do work
they require people to go away and fire up something to do a search).
Making things directly readable by humans really is much better.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html