2009-01-18 15:28:58

by Hannes Eder

[permalink] [raw]
Subject: [PATCH 0/3] drivers/gpu/drm: fix (sparse) warnings

The following series fixes a couple of (sparse) warnings.

---

Hannes Eder (3):
drivers/gpu/drm: comment out dead code
drivers/gpu/drm: fix sparse warnings: unexport symbols
drivers/gpu/drm: fix sparse warnings: make symbols static


drivers/gpu/drm/drm_crtc.c | 17 ++++++++++-------
drivers/gpu/drm/drm_crtc_helper.c | 6 +++---
drivers/gpu/drm/drm_edid.c | 4 ++--
drivers/gpu/drm/drm_modes.c | 13 +++++++------
4 files changed, 22 insertions(+), 18 deletions(-)


2009-01-18 15:29:34

by Hannes Eder

[permalink] [raw]
Subject: [PATCH 1/3] drivers/gpu/drm: fix sparse warnings: make symbols static

Fix this sparse warnings:

drivers/gpu/drm/drm_crtc.c:270:17: warning: symbol 'drm_crtc_from_fb' was not declared. Should it be static?
drivers/gpu/drm/drm_crtc.c:810:5: warning: symbol 'drm_mode_group_init' was not declared. Should it be static?
drivers/gpu/drm/drm_crtc.c:913:6: warning: symbol 'drm_crtc_convert_to_umode' was not declared. Should it be static?
drivers/gpu/drm/drm_crtc.c:945:6: warning: symbol 'drm_crtc_convert_umode' was not declared. Should it be static?
drivers/gpu/drm/drm_edid.c:252:25: warning: symbol 'drm_mode_std' was not declared. Should it be static?
drivers/gpu/drm/drm_modes.c:446:6: warning: symbol 'list_sort' was not declared. Should it be static?

Signed-off-by: Hannes Eder <[email protected]>
---
drivers/gpu/drm/drm_crtc.c | 15 ++++++++-------
drivers/gpu/drm/drm_edid.c | 4 ++--
drivers/gpu/drm/drm_modes.c | 4 ++--
3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 5b2cbb7..d378306 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -267,8 +267,8 @@ EXPORT_SYMBOL(drm_mode_object_find);
* RETURNS:
* Pointer to the CRTC or NULL if it wasn't found.
*/
-struct drm_crtc *drm_crtc_from_fb(struct drm_device *dev,
- struct drm_framebuffer *fb)
+static struct drm_crtc *drm_crtc_from_fb(struct drm_device *dev,
+ struct drm_framebuffer *fb)
{
struct drm_crtc *crtc;

@@ -807,7 +807,8 @@ void drm_mode_config_init(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_mode_config_init);

-int drm_mode_group_init(struct drm_device *dev, struct drm_mode_group *group)
+static int drm_mode_group_init(struct drm_device *dev,
+ struct drm_mode_group *group)
{
uint32_t total_objects = 0;

@@ -910,8 +911,8 @@ EXPORT_SYMBOL(drm_mode_config_cleanup);
* Convert a drm_display_mode into a drm_mode_modeinfo structure to return to
* the user.
*/
-void drm_crtc_convert_to_umode(struct drm_mode_modeinfo *out,
- struct drm_display_mode *in)
+static void drm_crtc_convert_to_umode(struct drm_mode_modeinfo *out,
+ struct drm_display_mode *in)
{
out->clock = in->clock;
out->hdisplay = in->hdisplay;
@@ -942,8 +943,8 @@ void drm_crtc_convert_to_umode(struct drm_mode_modeinfo *out,
* Convert a drm_mode_modeinfo into a drm_display_mode structure to return to
* the caller.
*/
-void drm_crtc_convert_umode(struct drm_display_mode *out,
- struct drm_mode_modeinfo *in)
+static void drm_crtc_convert_umode(struct drm_display_mode *out,
+ struct drm_mode_modeinfo *in)
{
out->clock = in->clock;
out->hdisplay = in->hdisplay;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0fbb0da..47737b2 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -249,8 +249,8 @@ static void edid_fixup_preferred(struct drm_connector *connector,
* Punts for now, but should eventually use the FB layer's CVT based mode
* generation code.
*/
-struct drm_display_mode *drm_mode_std(struct drm_device *dev,
- struct std_timing *t)
+static struct drm_display_mode *drm_mode_std(struct drm_device *dev,
+ struct std_timing *t)
{
struct drm_display_mode *mode;
int hsize = t->hsize * 8 + 248, vsize;
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index c9b80fd..7c36f16 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -443,8 +443,8 @@ static int drm_mode_compare(struct list_head *lh_a, struct list_head *lh_b)

/* FIXME: what we don't have a list sort function? */
/* list sort from Mark J Roberts ([email protected]) */
-void list_sort(struct list_head *head,
- int (*cmp)(struct list_head *a, struct list_head *b))
+static void list_sort(struct list_head *head,
+ int (*cmp)(struct list_head *a, struct list_head *b))
{
struct list_head *p, *q, *e, *list, *tail, *oldhead;
int insize, nmerges, psize, qsize, i;

2009-01-18 15:30:00

by Hannes Eder

[permalink] [raw]
Subject: [PATCH 2/3] drivers/gpu/drm: fix sparse warnings: unexport symbols

Fix this sparse warnings:

drivers/gpu/drm/drm_crtc_helper.c:137:6: warning: symbol 'drm_helper_probe_connector_modes' was not declared. Should it be static?
drivers/gpu/drm/drm_modes.c:360:6: warning: symbol 'drm_mode_validate_clocks' was not declared. Should it be static?

Signed-off-by: Hannes Eder <[email protected]>
---
drivers/gpu/drm/drm_crtc_helper.c | 6 +++---
drivers/gpu/drm/drm_modes.c | 7 +++----
2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index d8a982b..a0b2601 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -134,8 +134,9 @@ void drm_helper_probe_single_connector_modes(struct drm_connector *connector,
}
EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);

-void drm_helper_probe_connector_modes(struct drm_device *dev, uint32_t maxX,
- uint32_t maxY)
+static void drm_helper_probe_connector_modes(struct drm_device *dev,
+ uint32_t maxX,
+ uint32_t maxY)
{
struct drm_connector *connector;

@@ -143,7 +144,6 @@ void drm_helper_probe_connector_modes(struct drm_device *dev, uint32_t maxX,
drm_helper_probe_single_connector_modes(connector, maxX, maxY);
}
}
-EXPORT_SYMBOL(drm_helper_probe_connector_modes);


/**
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 7c36f16..c0cfe39 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -357,9 +357,9 @@ EXPORT_SYMBOL(drm_mode_validate_size);
* sure each mode falls within a given range (defined by @min and @max
* arrays) and sets @mode->status as needed.
*/
-void drm_mode_validate_clocks(struct drm_device *dev,
- struct list_head *mode_list,
- int *min, int *max, int n_ranges)
+static void drm_mode_validate_clocks(struct drm_device *dev,
+ struct list_head *mode_list,
+ int *min, int *max, int n_ranges)
{
struct drm_display_mode *mode;
int i;
@@ -376,7 +376,6 @@ void drm_mode_validate_clocks(struct drm_device *dev,
mode->status = MODE_CLOCK_RANGE;
}
}
-EXPORT_SYMBOL(drm_mode_validate_clocks);

/**
* drm_mode_prune_invalid - remove invalid modes from mode list

2009-01-18 15:30:58

by Hannes Eder

[permalink] [raw]
Subject: [PATCH 3/3] drivers/gpu/drm: comment out dead code

The functions:

drm_crtc_from_fb
drm_mode_validate_clocks

are not used, so comment them out.

Signed-off-by: Hannes Eder <[email protected]>
---
drivers/gpu/drm/drm_crtc.c | 2 ++
drivers/gpu/drm/drm_modes.c | 2 ++
2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d378306..c44bbde 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -254,6 +254,7 @@ void *drm_mode_object_find(struct drm_device *dev, uint32_t id, uint32_t type)
}
EXPORT_SYMBOL(drm_mode_object_find);

+#ifdef UNUSED
/**
* drm_crtc_from_fb - find the CRTC structure associated with an fb
* @dev: DRM device
@@ -278,6 +279,7 @@ static struct drm_crtc *drm_crtc_from_fb(struct drm_device *dev,
}
return NULL;
}
+#endif /* UNUSED */

/**
* drm_framebuffer_init - initialize a framebuffer
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index c0cfe39..8ebcf51 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -341,6 +341,7 @@ void drm_mode_validate_size(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_mode_validate_size);

+#ifdef UNUSED
/**
* drm_mode_validate_clocks - validate modes against clock limits
* @dev: DRM device
@@ -376,6 +377,7 @@ static void drm_mode_validate_clocks(struct drm_device *dev,
mode->status = MODE_CLOCK_RANGE;
}
}
+#endif /* UNUSED */

/**
* drm_mode_prune_invalid - remove invalid modes from mode list

2009-01-18 15:56:35

by Jaswinder Singh Rajput

[permalink] [raw]
Subject: Re: [PATCH 2/3] drivers/gpu/drm: fix sparse warnings: unexport symbols

On Sun, 2009-01-18 at 16:28 +0100, Hannes Eder wrote:
> Fix this sparse warnings:
>
> drivers/gpu/drm/drm_crtc_helper.c:137:6: warning: symbol 'drm_helper_probe_connector_modes' was not declared. Should it be static?
> drivers/gpu/drm/drm_modes.c:360:6: warning: symbol 'drm_mode_validate_clocks' was not declared. Should it be static?
>
> Signed-off-by: Hannes Eder <[email protected]>
> ---
> drivers/gpu/drm/drm_crtc_helper.c | 6 +++---
> drivers/gpu/drm/drm_modes.c | 7 +++----
> 2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index d8a982b..a0b2601 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -134,8 +134,9 @@ void drm_helper_probe_single_connector_modes(struct drm_connector *connector,
> }
> EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
>
> -void drm_helper_probe_connector_modes(struct drm_device *dev, uint32_t maxX,
> - uint32_t maxY)
> +static void drm_helper_probe_connector_modes(struct drm_device *dev,
> + uint32_t maxX,
> + uint32_t maxY)
> {
> struct drm_connector *connector;
>
> @@ -143,7 +144,6 @@ void drm_helper_probe_connector_modes(struct drm_device *dev, uint32_t maxX,
> drm_helper_probe_single_connector_modes(connector, maxX, maxY);
> }
> }
> -EXPORT_SYMBOL(drm_helper_probe_connector_modes);
>

hmm, what you are trying to do ?

>
> /**
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 7c36f16..c0cfe39 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -357,9 +357,9 @@ EXPORT_SYMBOL(drm_mode_validate_size);
> * sure each mode falls within a given range (defined by @min and @max
> * arrays) and sets @mode->status as needed.
> */
> -void drm_mode_validate_clocks(struct drm_device *dev,
> - struct list_head *mode_list,
> - int *min, int *max, int n_ranges)
> +static void drm_mode_validate_clocks(struct drm_device *dev,
> + struct list_head *mode_list,
> + int *min, int *max, int n_ranges)
> {
> struct drm_display_mode *mode;
> int i;
> @@ -376,7 +376,6 @@ void drm_mode_validate_clocks(struct drm_device *dev,
> mode->status = MODE_CLOCK_RANGE;
> }
> }
> -EXPORT_SYMBOL(drm_mode_validate_clocks);

??

--
JSR

2009-01-18 16:04:54

by Hannes Eder

[permalink] [raw]
Subject: Re: [PATCH 2/3] drivers/gpu/drm: fix sparse warnings: unexport symbols

On Sun, Jan 18, 2009 at 4:56 PM, Jaswinder Singh Rajput
<[email protected]> wrote:
> On Sun, 2009-01-18 at 16:28 +0100, Hannes Eder wrote:
>> Fix this sparse warnings:
>>
>> drivers/gpu/drm/drm_crtc_helper.c:137:6: warning: symbol 'drm_helper_probe_connector_modes' was not declared. Should it be static?
>> drivers/gpu/drm/drm_modes.c:360:6: warning: symbol 'drm_mode_validate_clocks' was not declared. Should it be static?
>>
>> Signed-off-by: Hannes Eder <[email protected]>
>> ---
>> drivers/gpu/drm/drm_crtc_helper.c | 6 +++---
>> drivers/gpu/drm/drm_modes.c | 7 +++----
>> 2 files changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
>> index d8a982b..a0b2601 100644
>> --- a/drivers/gpu/drm/drm_crtc_helper.c
>> +++ b/drivers/gpu/drm/drm_crtc_helper.c
>> @@ -134,8 +134,9 @@ void drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>> }
>> EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
>>
>> -void drm_helper_probe_connector_modes(struct drm_device *dev, uint32_t maxX,
>> - uint32_t maxY)
>> +static void drm_helper_probe_connector_modes(struct drm_device *dev,
>> + uint32_t maxX,
>> + uint32_t maxY)
>> {
>> struct drm_connector *connector;
>>
>> @@ -143,7 +144,6 @@ void drm_helper_probe_connector_modes(struct drm_device *dev, uint32_t maxX,
>> drm_helper_probe_single_connector_modes(connector, maxX, maxY);
>> }
>> }
>> -EXPORT_SYMBOL(drm_helper_probe_connector_modes);
>>
>
> hmm, what you are trying to do ?
>
>>
>> /**
>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
>> index 7c36f16..c0cfe39 100644
>> --- a/drivers/gpu/drm/drm_modes.c
>> +++ b/drivers/gpu/drm/drm_modes.c
>> @@ -357,9 +357,9 @@ EXPORT_SYMBOL(drm_mode_validate_size);
>> * sure each mode falls within a given range (defined by @min and @max
>> * arrays) and sets @mode->status as needed.
>> */
>> -void drm_mode_validate_clocks(struct drm_device *dev,
>> - struct list_head *mode_list,
>> - int *min, int *max, int n_ranges)
>> +static void drm_mode_validate_clocks(struct drm_device *dev,
>> + struct list_head *mode_list,
>> + int *min, int *max, int n_ranges)
>> {
>> struct drm_display_mode *mode;
>> int i;
>> @@ -376,7 +376,6 @@ void drm_mode_validate_clocks(struct drm_device *dev,
>> mode->status = MODE_CLOCK_RANGE;
>> }
>> }
>> -EXPORT_SYMBOL(drm_mode_validate_clocks);
>
> ??

In both cases: a grep over the entire kernel tree shows that the
function is only used within the compilation unit it is defined,
therefore make it static and un-EXPORT it.

Hannes

2009-01-18 16:11:24

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH 2/3] drivers/gpu/drm: fix sparse warnings: unexport symbols

> >> -EXPORT_SYMBOL(drm_mode_validate_clocks);
> >
> > ??
>
> In both cases: a grep over the entire kernel tree shows that the
> function is only used within the compilation unit it is defined,
> therefore make it static and un-EXPORT it.

I don't know anything about this specific context, but aren't things in
general exported so that external modules can use them? If that is the
case, grepping over the kernel sources is not sufficient.

julia

2009-01-18 16:18:53

by Jaswinder Singh Rajput

[permalink] [raw]
Subject: Re: [PATCH 2/3] drivers/gpu/drm: fix sparse warnings: unexport symbols

On Sun, 2009-01-18 at 17:04 +0100, Hannes Eder wrote:
> >> /**
> >> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> >> index 7c36f16..c0cfe39 100644
> >> --- a/drivers/gpu/drm/drm_modes.c
> >> +++ b/drivers/gpu/drm/drm_modes.c
> >> @@ -357,9 +357,9 @@ EXPORT_SYMBOL(drm_mode_validate_size);
> >> * sure each mode falls within a given range (defined by @min and @max
> >> * arrays) and sets @mode->status as needed.
> >> */
> >> -void drm_mode_validate_clocks(struct drm_device *dev,
> >> - struct list_head *mode_list,
> >> - int *min, int *max, int n_ranges)
> >> +static void drm_mode_validate_clocks(struct drm_device *dev,
> >> + struct list_head *mode_list,
> >> + int *min, int *max, int n_ranges)
> >> {
> >> struct drm_display_mode *mode;
> >> int i;
> >> @@ -376,7 +376,6 @@ void drm_mode_validate_clocks(struct drm_device *dev,
> >> mode->status = MODE_CLOCK_RANGE;
> >> }
> >> }
> >> -EXPORT_SYMBOL(drm_mode_validate_clocks);
> >
> > ??
>
> In both cases: a grep over the entire kernel tree shows that the
> function is only used within the compilation unit it is defined,
> therefore make it static and un-EXPORT it.

If it is EXPORTED you cannot make it static. EXPORTED means it can be
used from a kernel module.

you need to define the declaration of these functions on related header
file to fix these sparse warnings.

--
JSR

2009-01-18 17:42:31

by Hannes Eder

[permalink] [raw]
Subject: Re: [PATCH 2/3] drivers/gpu/drm: fix sparse warnings: unexport symbols

On Sun, Jan 18, 2009 at 5:11 PM, Julia Lawall <[email protected]> wrote:
> I don't know anything about this specific context, but aren't things in
> general exported so that external modules can use them? If that is the
> case, grepping over the kernel sources is not sufficient.

Good point, but it indicates that the function does not have a caller
within kernel tree.
Ok, there could be other clients. So this patch might just be a hint
for the original author or maintainer that there is maybe no need to
export this function.

On Sun, Jan 18, 2009 at 5:18 PM, Jaswinder Singh Rajput
<[email protected]> wrote:
> If it is EXPORTED you cannot make it static. EXPORTED means it can be
> used from a kernel module.

Right, that's why I have made it static and removed the EXPORT_SYMBOL,
all under the assumption that there is _no_ client for this function,
but there may be some outside the kernel tree. I can not judge this.

Hannes