[PATCH v2 0/5] agp: minor fixes

[PATCH v2 0/5] agp: minor fixes

[Corentin Labbe]

Hello

This patch serie fixes some minor problem found in the agp subsystem
There are no change since v1 (posted two years ago)
This is simply a repost for trying to get an answer (gentle ping 6 month
ago got no answer also).

Regards

Corentin Labbe (5):
  agp: remove unused variable size in agp_generic_create_gatt_table
  agp: move AGPGART_MINOR to include/linux/miscdevice.h
  agp: remove unused variable num_segments
  agp: Add bridge parameter documentation
  ia64: agp: Replace empty define with do while

 arch/ia64/include/asm/agp.h |  4 ++--
 drivers/char/agp/frontend.c |  3 +--
 drivers/char/agp/generic.c  | 12 +++++-------
 include/linux/agpgart.h     |  2 --
 include/linux/miscdevice.h  |  1 +
 5 files changed, 9 insertions(+), 13 deletions(-)

-- 
2.23.0

[PATCH v2 1/5] agp: remove unused variable size in agp_generic_create_gatt_table

[Corentin Labbe]

This patch fix the following warning:
drivers/char/agp/generic.c:853:6: attention : variable ‘size’ set but not used [-Wunused-but-set-variable]
by removing the unused variable size in agp_generic_create_gatt_table

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/char/agp/generic.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/char/agp/generic.c b/drivers/char/agp/generic.c
index df1edb5ec0ad..4d2eb0800b2a 100644
--- a/drivers/char/agp/generic.c
+++ b/drivers/char/agp/generic.c
@@ -850,7 +850,6 @@ int agp_generic_create_gatt_table(struct agp_bridge_data *bridge)
 {
 	char *table;
 	char *table_end;
-	int size;
 	int page_order;
 	int num_entries;
 	int i;
@@ -864,25 +863,22 @@ int agp_generic_create_gatt_table(struct agp_bridge_data *bridge)
 	table = NULL;
 	i = bridge->aperture_size_idx;
 	temp = bridge->current_size;
-	size = page_order = num_entries = 0;
+	page_order = num_entries = 0;
 
 	if (bridge->driver->size_type != FIXED_APER_SIZE) {
 		do {
 			switch (bridge->driver->size_type) {
 			case U8_APER_SIZE:
-				size = A_SIZE_8(temp)->size;
 				page_order =
 				    A_SIZE_8(temp)->page_order;
 				num_entries =
 				    A_SIZE_8(temp)->num_entries;
 				break;
 			case U16_APER_SIZE:
-				size = A_SIZE_16(temp)->size;
 				page_order = A_SIZE_16(temp)->page_order;
 				num_entries = A_SIZE_16(temp)->num_entries;
 				break;
 			case U32_APER_SIZE:
-				size = A_SIZE_32(temp)->size;
 				page_order = A_SIZE_32(temp)->page_order;
 				num_entries = A_SIZE_32(temp)->num_entries;
 				break;
@@ -890,7 +886,7 @@ int agp_generic_create_gatt_table(struct agp_bridge_data *bridge)
 			case FIXED_APER_SIZE:
 			case LVL2_APER_SIZE:
 			default:
-				size = page_order = num_entries = 0;
+				page_order = num_entries = 0;
 				break;
 			}
 
@@ -920,7 +916,6 @@ int agp_generic_create_gatt_table(struct agp_bridge_data *bridge)
 			}
 		} while (!table && (i < bridge->driver->num_aperture_sizes));
 	} else {
-		size = ((struct aper_size_info_fixed *) temp)->size;
 		page_order = ((struct aper_size_info_fixed *) temp)->page_order;
 		num_entries = ((struct aper_size_info_fixed *) temp)->num_entries;
 		table = alloc_gatt_pages(page_order);
-- 
2.23.0

[PATCH v2 2/5] agp: move AGPGART_MINOR to include/linux/miscdevice.h

[Corentin Labbe]

This patch move the define for AGPGART_MINOR to include/linux/miscdevice.h.
It is better that all minor number definitions are in the same place.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 include/linux/agpgart.h    | 2 --
 include/linux/miscdevice.h | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/agpgart.h b/include/linux/agpgart.h
index c6b61ca97053..21b34a96cfd8 100644
--- a/include/linux/agpgart.h
+++ b/include/linux/agpgart.h
@@ -30,8 +30,6 @@
 #include <linux/agp_backend.h>
 #include <uapi/linux/agpgart.h>
 
-#define AGPGART_MINOR 175
-
 struct agp_info {
 	struct agp_version version;	/* version of the driver        */
 	u32 bridge_id;		/* bridge vendor/device         */
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index b06b75776a32..becde6981a95 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -33,6 +33,7 @@
 #define SGI_MMTIMER		153
 #define STORE_QUEUE_MINOR	155	/* unused */
 #define I2O_MINOR		166
+#define AGPGART_MINOR		175
 #define HWRNG_MINOR		183
 #define MICROCODE_MINOR		184
 #define IRNET_MINOR		187
-- 
2.23.0

[PATCH v2 3/5] agp: remove unused variable num_segments

[Corentin Labbe]

This patch fix the following build warning:
warning: variable 'num_segments' set but not used [-Wunused-but-set-variable]

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/char/agp/frontend.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/char/agp/frontend.c b/drivers/char/agp/frontend.c
index f6955888e676..47098648502d 100644
--- a/drivers/char/agp/frontend.c
+++ b/drivers/char/agp/frontend.c
@@ -102,14 +102,13 @@ agp_segment_priv *agp_find_seg_in_client(const struct agp_client *client,
 					    int size, pgprot_t page_prot)
 {
 	struct agp_segment_priv *seg;
-	int num_segments, i;
+	int i;
 	off_t pg_start;
 	size_t pg_count;
 
 	pg_start = offset / 4096;
 	pg_count = size / 4096;
 	seg = *(client->segments);
-	num_segments = client->num_segments;
 
 	for (i = 0; i < client->num_segments; i++) {
 		if ((seg[i].pg_start == pg_start) &&
-- 
2.23.0

[PATCH v2 4/5] agp: Add bridge parameter documentation

[Corentin Labbe]

This patch add documentation about the bridge parameter in several
function.

This will fix the following build warning:
drivers/char/agp/generic.c:220: warning: No description found for parameter 'bridge'
drivers/char/agp/generic.c:364: warning: No description found for parameter 'bridge'
drivers/char/agp/generic.c:1283: warning: No description found for parameter 'bridge'

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/char/agp/generic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/char/agp/generic.c b/drivers/char/agp/generic.c
index 4d2eb0800b2a..ab154a75acf0 100644
--- a/drivers/char/agp/generic.c
+++ b/drivers/char/agp/generic.c
@@ -207,6 +207,7 @@ EXPORT_SYMBOL(agp_free_memory);
 /**
  *	agp_allocate_memory  -  allocate a group of pages of a certain type.
  *
+ *	@bridge: an agp_bridge_data struct allocated for the AGP host bridge.
  *	@page_count:	size_t argument of the number of pages
  *	@type:	u32 argument of the type of memory to be allocated.
  *
@@ -355,6 +356,7 @@ EXPORT_SYMBOL_GPL(agp_num_entries);
 /**
  *	agp_copy_info  -  copy bridge state information
  *
+ *	@bridge: an agp_bridge_data struct allocated for the AGP host bridge.
  *	@info:		agp_kern_info pointer.  The caller should insure that this pointer is valid.
  *
  *	This function copies information about the agp bridge device and the state of
@@ -1277,6 +1279,7 @@ EXPORT_SYMBOL(agp_generic_destroy_page);
 /**
  * agp_enable  -  initialise the agp point-to-point connection.
  *
+ * @bridge: an agp_bridge_data struct allocated for the AGP host bridge.
  * @mode:	agp mode register value to configure with.
  */
 void agp_enable(struct agp_bridge_data *bridge, u32 mode)
-- 
2.23.0

[PATCH v2 5/5] ia64: agp: Replace empty define with do while

[Corentin Labbe]

It's dangerous to use empty code define.
Furthermore it lead to the following warning:
drivers/char/agp/generic.c: In function « agp_generic_destroy_page »:
drivers/char/agp/generic.c:1266:28: attention : suggest braces around empty body in an « if » statement [-Wempty-body]

So let's replace emptyness by "do {} while(0)"

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 arch/ia64/include/asm/agp.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/ia64/include/asm/agp.h b/arch/ia64/include/asm/agp.h
index 2b451c4496da..0261507dc264 100644
--- a/arch/ia64/include/asm/agp.h
+++ b/arch/ia64/include/asm/agp.h
@@ -14,8 +14,8 @@
  * in coherent mode, which lets us map the AGP memory as normal (write-back) memory
  * (unlike x86, where it gets mapped "write-coalescing").
  */
-#define map_page_into_agp(page)		/* nothing */
-#define unmap_page_from_agp(page)	/* nothing */
+#define map_page_into_agp(page)		do { } while (0)
+#define unmap_page_from_agp(page)	do { } while (0)
 #define flush_agp_cache()		mb()
 
 /* GATT allocation. Returns/accepts GATT kernel virtual address. */
-- 
2.23.0

Re: [PATCH v2 0/5] agp: minor fixes

[Arnd Bergmann]

On Thu, Nov 21, 2019 at 9:15 AM Corentin Labbe <clabbe@baylibre.com> wrote:
>
> Hello
>
> This patch serie fixes some minor problem found in the agp subsystem
> There are no change since v1 (posted two years ago)
> This is simply a repost for trying to get an answer (gentle ping 6 month
> ago got no answer also).

Looks all good to me,

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH v2 0/5] agp: minor fixes, does the maintainer still there ?

[LABBE Corentin]

On Thu, Nov 21, 2019 at 08:14:40AM +0000, Corentin Labbe wrote:
> Hello
> 
> This patch serie fixes some minor problem found in the agp subsystem
> There are no change since v1 (posted two years ago)
> This is simply a repost for trying to get an answer (gentle ping 6 month
> ago got no answer also).
> 
> Regards
> 

Hello

Does the AGP maintainer still maintain it ?

Regards

Re: [PATCH v2 0/5] agp: minor fixes, does the maintainer still there ?

[David Airlie]

On Mon, Dec 2, 2019 at 11:33 PM LABBE Corentin <clabbe@baylibre.com> wrote:
>
> On Thu, Nov 21, 2019 at 08:14:40AM +0000, Corentin Labbe wrote:
> > Hello
> >
> > This patch serie fixes some minor problem found in the agp subsystem
> > There are no change since v1 (posted two years ago)
> > This is simply a repost for trying to get an answer (gentle ping 6 month
> > ago got no answer also).
> >
> > Regards
> >
>
> Hello
>
> Does the AGP maintainer still maintain it ?

It's maintained but really loathe to touch it, I've no hw to validate
any changes on so making any changes to it really has to get past my
internal, I care enough about this change to risk applying anything to
AGP.

I'll try and look and apply those patches today.

Dave.

Re: [PATCH v2 0/5] agp: minor fixes, does the maintainer still there ?

[LABBE Corentin]

On Tue, Dec 03, 2019 at 06:01:15AM +1000, David Airlie wrote:
> On Mon, Dec 2, 2019 at 11:33 PM LABBE Corentin <clabbe@baylibre.com> wrote:
> >
> > On Thu, Nov 21, 2019 at 08:14:40AM +0000, Corentin Labbe wrote:
> > > Hello
> > >
> > > This patch serie fixes some minor problem found in the agp subsystem
> > > There are no change since v1 (posted two years ago)
> > > This is simply a repost for trying to get an answer (gentle ping 6 month
> > > ago got no answer also).
> > >
> > > Regards
> > >
> >
> > Hello
> >
> > Does the AGP maintainer still maintain it ?
> 
> It's maintained but really loathe to touch it, I've no hw to validate
> any changes on so making any changes to it really has to get past my
> internal, I care enough about this change to risk applying anything to
> AGP.
> 
> I'll try and look and apply those patches today.
> 

Thanks for applying.
Perhaps you need to fix your address in MAINTAINERS.

When you has hardware, What was your tests procedure ?
I can on my freetime add some AGP hw on my kernelCI lab.

Regards

Re: [PATCH v2 0/5] agp: minor fixes, does the maintainer still there ?

[David Airlie]

> Thanks for applying.
> Perhaps you need to fix your address in MAINTAINERS.
>
> When you has hardware, What was your tests procedure ?
Just boot and run gears type thing.

> I can on my freetime add some AGP hw on my kernelCI lab.
>

I don't know where to get AGP hw these days that isn't horrible power
consumption wise, I can't really wish for anyone to ever put it into
CI for an area that we change once a year.

I'd rather we just don't touch AGP too much and let it die and remove
support in 5-10 years.

Dave.