[PATCH] remove klist iterators

[PATCH] remove klist iterators

[James Bottomley]

commit 11c3b5c3e08f4d855cbef52883c266b9ab9df879
Author: Greg Kroah-Hartman <gregkh@suse.de>
Date:   Tue Dec 16 12:24:56 2008 -0800

    driver core: move klist_children into private structure

Broke our parisc build pretty badly because we touch the klists directly
in three cases (AGP, SBA and GSC).  Although GregKH will revert this
patch, there's no reason we should be using the iterators directly, we
can just move to the standard device_for_each_child() API.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

---

diff --git a/drivers/char/agp/parisc-agp.c b/drivers/char/agp/parisc-agp.c
index db60539..699e342 100644
--- a/drivers/char/agp/parisc-agp.c
+++ b/drivers/char/agp/parisc-agp.c
@@ -359,9 +359,16 @@ fail:
 	return error;
 }
 
-static struct device *next_device(struct klist_iter *i) {
-	struct klist_node * n = klist_next(i);
-	return n ? container_of(n, struct device, knode_parent) : NULL;
+static int
+find_quicksilver(struct device *dev, void *data)
+{
+	struct parisc_device **lba = data;
+	struct parisc_device *padev = to_parisc_device(dev);
+
+	if (IS_QUICKSILVER(padev))
+		*lba = padev;
+
+	return 0;
 }
 
 static int
@@ -372,8 +379,6 @@ parisc_agp_init(void)
 	int err = -1;
 	struct parisc_device *sba = NULL, *lba = NULL;
 	struct lba_device *lbadev = NULL;
-	struct device *dev = NULL;
-	struct klist_iter i;
 
 	if (!sba_list)
 		goto out;
@@ -386,13 +391,7 @@ parisc_agp_init(void)
 	}
 
 	/* Now search our Pluto for our precious AGP device... */
-	klist_iter_init(&sba->dev.klist_children, &i);
-	while ((dev = next_device(&i))) {
-		struct parisc_device *padev = to_parisc_device(dev);
-		if (IS_QUICKSILVER(padev))
-			lba = padev;
-	}
-	klist_iter_exit(&i);
+	device_for_each_child(&sba->dev, &lba, find_quicksilver);
 
 	if (!lba) {
 		printk(KERN_INFO DRVPFX "No AGP devices found.\n");
diff --git a/drivers/parisc/gsc.c b/drivers/parisc/gsc.c
index e76db9e..ea8f600 100644
--- a/drivers/parisc/gsc.c
+++ b/drivers/parisc/gsc.c
@@ -186,29 +186,34 @@ void gsc_asic_assign_irq(struct gsc_asic *asic, int local_irq, int *irqp)
 	*irqp = irq;
 }
 
-static struct device *next_device(struct klist_iter *i)
+struct gsc_fixup_struct {
+	void (*choose_irq)(struct parisc_device *, void *);
+	void *ctrl;
+};
+
+static int gsc_fixup_irqs_callback(struct device *dev, void *data)
 {
-	struct klist_node * n = klist_next(i);
-	return n ? container_of(n, struct device, knode_parent) : NULL;
+	struct parisc_device *padev = to_parisc_device(dev);
+	struct gsc_fixup_struct *gf = data;
+
+	/* work-around for 715/64 and others which have parent 
+	   at path [5] and children at path [5/0/x] */
+	if (padev->id.hw_type == HPHW_FAULTY)
+		gsc_fixup_irqs(padev, gf->ctrl, gf->choose_irq);
+	gf->choose_irq(padev, gf->ctrl);
+
+	return 0;
 }
 
 void gsc_fixup_irqs(struct parisc_device *parent, void *ctrl,
 			void (*choose_irq)(struct parisc_device *, void *))
 {
-	struct device *dev;
-	struct klist_iter i;
-
-	klist_iter_init(&parent->dev.klist_children, &i);
-	while ((dev = next_device(&i))) {
-		struct parisc_device *padev = to_parisc_device(dev);
-
-		/* work-around for 715/64 and others which have parent 
-		   at path [5] and children at path [5/0/x] */
-		if (padev->id.hw_type == HPHW_FAULTY)
-			return gsc_fixup_irqs(padev, ctrl, choose_irq);
-		choose_irq(padev, ctrl);
-	}
-	klist_iter_exit(&i);
+	struct gsc_fixup_struct data = {
+		.choose_irq	= choose_irq,
+		.ctrl		= ctrl,
+	};
+
+	device_for_each_child(&parent->dev, &data, gsc_fixup_irqs_callback);
 }
 
 int gsc_common_setup(struct parisc_device *parent, struct gsc_asic *gsc_asic)
diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
index 3fac8f8..1f5ebc2 100644
--- a/drivers/parisc/sba_iommu.c
+++ b/drivers/parisc/sba_iommu.c
@@ -1206,31 +1206,49 @@ sba_alloc_pdir(unsigned int pdir_size)
 	return (void *) pdir_base;
 }
 
-static struct device *next_device(struct klist_iter *i)
+struct ibase_data_struct {
+	struct ioc *ioc;
+	int ioc_num;
+};
+
+static int setup_ibase_imask_callback(struct device *dev, void *data)
 {
-        struct klist_node * n = klist_next(i);
-        return n ? container_of(n, struct device, knode_parent) : NULL;
+	/* lba_set_iregs() is in drivers/parisc/lba_pci.c */
+        extern void lba_set_iregs(struct parisc_device *, u32, u32);
+	struct parisc_device *lba = to_parisc_device(dev);
+	struct ibase_data_struct *ibd = data;
+	int rope_num = (lba->hpa.start >> 13) & 0xf;
+	if (rope_num >> 3 == ibd->ioc_num)
+		lba_set_iregs(lba, ibd->ioc->ibase, ibd->ioc->imask);
+	return 0;
 }
 
 /* setup Mercury or Elroy IBASE/IMASK registers. */
 static void 
 setup_ibase_imask(struct parisc_device *sba, struct ioc *ioc, int ioc_num)
 {
-	/* lba_set_iregs() is in drivers/parisc/lba_pci.c */
-        extern void lba_set_iregs(struct parisc_device *, u32, u32);
-	struct device *dev;
-	struct klist_iter i;
-
-	klist_iter_init(&sba->dev.klist_children, &i);
-	while ((dev = next_device(&i))) {
-		struct parisc_device *lba = to_parisc_device(dev);
-		int rope_num = (lba->hpa.start >> 13) & 0xf;
-		if (rope_num >> 3 == ioc_num)
-			lba_set_iregs(lba, ioc->ibase, ioc->imask);
-	}
-	klist_iter_exit(&i);
+	struct ibase_data_struct ibase_data = {
+		.ioc		= ioc,
+		.ioc_num	= ioc_num,
+	};
+
+	device_for_each_child(&sba->dev, &ibase_data,
+			      setup_ibase_imask_callback);
 }
 
+#ifdef SBA_AGP_SUPPORT
+static int
+sba_ioc_find_quicksilver(struct device *dev, void *data)
+{
+	int *agp_found = data;
+	struct parisc_device *lba = to_parisc_device(dev);
+
+	if (IS_QUICKSILVER(lba))
+		*agp_found = 1;
+	return 0;
+}
+#endif
+
 static void
 sba_ioc_init_pluto(struct parisc_device *sba, struct ioc *ioc, int ioc_num)
 {
@@ -1332,9 +1350,6 @@ sba_ioc_init_pluto(struct parisc_device *sba, struct ioc *ioc, int ioc_num)
 	WRITE_REG(ioc->ibase | 31, ioc->ioc_hpa + IOC_PCOM);
 
 #ifdef SBA_AGP_SUPPORT
-{
-	struct klist_iter i;
-	struct device *dev = NULL;
 
 	/*
 	** If an AGP device is present, only use half of the IOV space
@@ -1344,13 +1359,7 @@ sba_ioc_init_pluto(struct parisc_device *sba, struct ioc *ioc, int ioc_num)
 	** We program the next pdir index after we stop w/ a key for
 	** the GART code to handshake on.
 	*/
-	klist_iter_init(&sba->dev.klist_children, &i);
-	while ((dev = next_device(&i))) {
-		struct parisc_device *lba = to_parisc_device(dev);
-		if (IS_QUICKSILVER(lba))
-			agp_found = 1;
-	}
-	klist_iter_exit(&i);
+	device_for_each_child(&sba->dev, &agp_found, sba_ioc_find_quicksilver);
 
 	if (agp_found && sba_reserve_agpgart) {
 		printk(KERN_INFO "%s: reserving %dMb of IOVA space for agpgart\n",

Re: [PATCH] remove klist iterators

[Helge Deller]

James Bottomley wrote:
> commit 11c3b5c3e08f4d855cbef52883c266b9ab9df879
> Author: Greg Kroah-Hartman <gregkh@suse.de>
> Date:   Tue Dec 16 12:24:56 2008 -0800
> 
>     driver core: move klist_children into private structure
> 
> Broke our parisc build pretty badly because we touch the klists directly
> in three cases (AGP, SBA and GSC).  Although GregKH will revert this
> patch, there's no reason we should be using the iterators directly, we
> can just move to the standard device_for_each_child() API.
> 
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

I just tested this patch on my 715/64 and c3000 with 2.6.29-rc1. Everything OK.

Tested-by: Helge Deller <deller@gmx.de>



> ---
> 
> diff --git a/drivers/char/agp/parisc-agp.c b/drivers/char/agp/parisc-agp.c
> index db60539..699e342 100644
> --- a/drivers/char/agp/parisc-agp.c
> +++ b/drivers/char/agp/parisc-agp.c
> @@ -359,9 +359,16 @@ fail:
>  	return error;
>  }
>  
> -static struct device *next_device(struct klist_iter *i) {
> -	struct klist_node * n = klist_next(i);
> -	return n ? container_of(n, struct device, knode_parent) : NULL;
> +static int
> +find_quicksilver(struct device *dev, void *data)
> +{
> +	struct parisc_device **lba = data;
> +	struct parisc_device *padev = to_parisc_device(dev);
> +
> +	if (IS_QUICKSILVER(padev))
> +		*lba = padev;
> +
> +	return 0;
>  }
>  
>  static int
> @@ -372,8 +379,6 @@ parisc_agp_init(void)
>  	int err = -1;
>  	struct parisc_device *sba = NULL, *lba = NULL;
>  	struct lba_device *lbadev = NULL;
> -	struct device *dev = NULL;
> -	struct klist_iter i;
>  
>  	if (!sba_list)
>  		goto out;
> @@ -386,13 +391,7 @@ parisc_agp_init(void)
>  	}
>  
>  	/* Now search our Pluto for our precious AGP device... */
> -	klist_iter_init(&sba->dev.klist_children, &i);
> -	while ((dev = next_device(&i))) {
> -		struct parisc_device *padev = to_parisc_device(dev);
> -		if (IS_QUICKSILVER(padev))
> -			lba = padev;
> -	}
> -	klist_iter_exit(&i);
> +	device_for_each_child(&sba->dev, &lba, find_quicksilver);
>  
>  	if (!lba) {
>  		printk(KERN_INFO DRVPFX "No AGP devices found.\n");
> diff --git a/drivers/parisc/gsc.c b/drivers/parisc/gsc.c
> index e76db9e..ea8f600 100644
> --- a/drivers/parisc/gsc.c
> +++ b/drivers/parisc/gsc.c
> @@ -186,29 +186,34 @@ void gsc_asic_assign_irq(struct gsc_asic *asic, int local_irq, int *irqp)
>  	*irqp = irq;
>  }
>  
> -static struct device *next_device(struct klist_iter *i)
> +struct gsc_fixup_struct {
> +	void (*choose_irq)(struct parisc_device *, void *);
> +	void *ctrl;
> +};
> +
> +static int gsc_fixup_irqs_callback(struct device *dev, void *data)
>  {
> -	struct klist_node * n = klist_next(i);
> -	return n ? container_of(n, struct device, knode_parent) : NULL;
> +	struct parisc_device *padev = to_parisc_device(dev);
> +	struct gsc_fixup_struct *gf = data;
> +
> +	/* work-around for 715/64 and others which have parent 
> +	   at path [5] and children at path [5/0/x] */
> +	if (padev->id.hw_type == HPHW_FAULTY)
> +		gsc_fixup_irqs(padev, gf->ctrl, gf->choose_irq);
> +	gf->choose_irq(padev, gf->ctrl);
> +
> +	return 0;
>  }
>  
>  void gsc_fixup_irqs(struct parisc_device *parent, void *ctrl,
>  			void (*choose_irq)(struct parisc_device *, void *))
>  {
> -	struct device *dev;
> -	struct klist_iter i;
> -
> -	klist_iter_init(&parent->dev.klist_children, &i);
> -	while ((dev = next_device(&i))) {
> -		struct parisc_device *padev = to_parisc_device(dev);
> -
> -		/* work-around for 715/64 and others which have parent 
> -		   at path [5] and children at path [5/0/x] */
> -		if (padev->id.hw_type == HPHW_FAULTY)
> -			return gsc_fixup_irqs(padev, ctrl, choose_irq);
> -		choose_irq(padev, ctrl);
> -	}
> -	klist_iter_exit(&i);
> +	struct gsc_fixup_struct data = {
> +		.choose_irq	= choose_irq,
> +		.ctrl		= ctrl,
> +	};
> +
> +	device_for_each_child(&parent->dev, &data, gsc_fixup_irqs_callback);
>  }
>  
>  int gsc_common_setup(struct parisc_device *parent, struct gsc_asic *gsc_asic)
> diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
> index 3fac8f8..1f5ebc2 100644
> --- a/drivers/parisc/sba_iommu.c
> +++ b/drivers/parisc/sba_iommu.c
> @@ -1206,31 +1206,49 @@ sba_alloc_pdir(unsigned int pdir_size)
>  	return (void *) pdir_base;
>  }
>  
> -static struct device *next_device(struct klist_iter *i)
> +struct ibase_data_struct {
> +	struct ioc *ioc;
> +	int ioc_num;
> +};
> +
> +static int setup_ibase_imask_callback(struct device *dev, void *data)
>  {
> -        struct klist_node * n = klist_next(i);
> -        return n ? container_of(n, struct device, knode_parent) : NULL;
> +	/* lba_set_iregs() is in drivers/parisc/lba_pci.c */
> +        extern void lba_set_iregs(struct parisc_device *, u32, u32);
> +	struct parisc_device *lba = to_parisc_device(dev);
> +	struct ibase_data_struct *ibd = data;
> +	int rope_num = (lba->hpa.start >> 13) & 0xf;
> +	if (rope_num >> 3 == ibd->ioc_num)
> +		lba_set_iregs(lba, ibd->ioc->ibase, ibd->ioc->imask);
> +	return 0;
>  }
>  
>  /* setup Mercury or Elroy IBASE/IMASK registers. */
>  static void 
>  setup_ibase_imask(struct parisc_device *sba, struct ioc *ioc, int ioc_num)
>  {
> -	/* lba_set_iregs() is in drivers/parisc/lba_pci.c */
> -        extern void lba_set_iregs(struct parisc_device *, u32, u32);
> -	struct device *dev;
> -	struct klist_iter i;
> -
> -	klist_iter_init(&sba->dev.klist_children, &i);
> -	while ((dev = next_device(&i))) {
> -		struct parisc_device *lba = to_parisc_device(dev);
> -		int rope_num = (lba->hpa.start >> 13) & 0xf;
> -		if (rope_num >> 3 == ioc_num)
> -			lba_set_iregs(lba, ioc->ibase, ioc->imask);
> -	}
> -	klist_iter_exit(&i);
> +	struct ibase_data_struct ibase_data = {
> +		.ioc		= ioc,
> +		.ioc_num	= ioc_num,
> +	};
> +
> +	device_for_each_child(&sba->dev, &ibase_data,
> +			      setup_ibase_imask_callback);
>  }
>  
> +#ifdef SBA_AGP_SUPPORT
> +static int
> +sba_ioc_find_quicksilver(struct device *dev, void *data)
> +{
> +	int *agp_found = data;
> +	struct parisc_device *lba = to_parisc_device(dev);
> +
> +	if (IS_QUICKSILVER(lba))
> +		*agp_found = 1;
> +	return 0;
> +}
> +#endif
> +
>  static void
>  sba_ioc_init_pluto(struct parisc_device *sba, struct ioc *ioc, int ioc_num)
>  {
> @@ -1332,9 +1350,6 @@ sba_ioc_init_pluto(struct parisc_device *sba, struct ioc *ioc, int ioc_num)
>  	WRITE_REG(ioc->ibase | 31, ioc->ioc_hpa + IOC_PCOM);
>  
>  #ifdef SBA_AGP_SUPPORT
> -{
> -	struct klist_iter i;
> -	struct device *dev = NULL;
>  
>  	/*
>  	** If an AGP device is present, only use half of the IOV space
> @@ -1344,13 +1359,7 @@ sba_ioc_init_pluto(struct parisc_device *sba, struct ioc *ioc, int ioc_num)
>  	** We program the next pdir index after we stop w/ a key for
>  	** the GART code to handshake on.
>  	*/
> -	klist_iter_init(&sba->dev.klist_children, &i);
> -	while ((dev = next_device(&i))) {
> -		struct parisc_device *lba = to_parisc_device(dev);
> -		if (IS_QUICKSILVER(lba))
> -			agp_found = 1;
> -	}
> -	klist_iter_exit(&i);
> +	device_for_each_child(&sba->dev, &agp_found, sba_ioc_find_quicksilver);
>  
>  	if (agp_found && sba_reserve_agpgart) {
>  		printk(KERN_INFO "%s: reserving %dMb of IOVA space for agpgart\n",
> 
> 

Re: [PATCH] remove klist iterators

[Kyle McMartin]

On Tue, Jan 13, 2009 at 09:54:15PM +0100, Helge Deller wrote:
> James Bottomley wrote:
> > commit 11c3b5c3e08f4d855cbef52883c266b9ab9df879
> > Author: Greg Kroah-Hartman <gregkh@suse.de>
> > Date:   Tue Dec 16 12:24:56 2008 -0800
> > 
> >     driver core: move klist_children into private structure
> > 
> > Broke our parisc build pretty badly because we touch the klists directly
> > in three cases (AGP, SBA and GSC).  Although GregKH will revert this
> > patch, there's no reason we should be using the iterators directly, we
> > can just move to the standard device_for_each_child() API.
> > 
> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> I just tested this patch on my 715/64 and c3000 with 2.6.29-rc1. Everything OK.
> 
> Tested-by: Helge Deller <deller@gmx.de>
> 

Thanks. Applied.

Re: [PATCH] remove klist iterators

[James Bottomley]

On Tue, 2009-01-13 at 15:58 -0500, Kyle McMartin wrote:
> On Tue, Jan 13, 2009 at 09:54:15PM +0100, Helge Deller wrote:
> > James Bottomley wrote:
> > > commit 11c3b5c3e08f4d855cbef52883c266b9ab9df879
> > > Author: Greg Kroah-Hartman <gregkh@suse.de>
> > > Date:   Tue Dec 16 12:24:56 2008 -0800
> > > 
> > >     driver core: move klist_children into private structure
> > > 
> > > Broke our parisc build pretty badly because we touch the klists directly
> > > in three cases (AGP, SBA and GSC).  Although GregKH will revert this
> > > patch, there's no reason we should be using the iterators directly, we
> > > can just move to the standard device_for_each_child() API.
> > > 
> > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> > 
> > I just tested this patch on my 715/64 and c3000 with 2.6.29-rc1. Everything OK.
> > 
> > Tested-by: Helge Deller <deller@gmx.de>
> > 
> 
> Thanks. Applied.

So you think I'd send an untested and uncompiled patch to the list?  ...
Hey, I strongly resemble that allegation!

The quicksilver AGP will need testing on a c8000 system, I think.

James

Re: [PATCH] remove klist iterators

[Kyle McMartin]

On Tue, Jan 13, 2009 at 03:21:11PM -0600, James Bottomley wrote:
> So you think I'd send an untested and uncompiled patch to the list?  ...
> Hey, I strongly resemble that allegation!
> 
> The quicksilver AGP will need testing on a c8000 system, I think.
> 

I'm sure every user will be horribly upset if it breaks. Oh wait. It's
just me. ;-)