* [PATCH v1 1/1] powerpc/8xx: Drop legacy-of-mm-gpiochip.h header @ 2024-11-15 13:38 Andy Shevchenko 2024-11-16 10:44 ` Christophe Leroy 0 siblings, 1 reply; 5+ messages in thread From: Andy Shevchenko @ 2024-11-15 13:38 UTC (permalink / raw) To: Andy Shevchenko, linuxppc-dev, linux-kernel Cc: Christophe Leroy, Michael Ellerman, Nicholas Piggin, Naveen N Rao, Madhavan Srinivasan Remove legacy-of-mm-gpiochip.h header file, replace of_* functions and structs with appropriate alternatives. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- arch/powerpc/platforms/8xx/cpm1.c | 119 +++++++++++++++--------------- 1 file changed, 60 insertions(+), 59 deletions(-) diff --git a/arch/powerpc/platforms/8xx/cpm1.c b/arch/powerpc/platforms/8xx/cpm1.c index b24d4102fbf6..1262bff5ba2e 100644 --- a/arch/powerpc/platforms/8xx/cpm1.c +++ b/arch/powerpc/platforms/8xx/cpm1.c @@ -45,7 +45,7 @@ #include <sysdev/fsl_soc.h> #ifdef CONFIG_8xx_GPIO -#include <linux/gpio/legacy-of-mm-gpiochip.h> +#include <linux/gpio/driver.h> #endif #define CPM_MAP_SIZE (0x4000) @@ -376,7 +376,8 @@ int __init cpm1_clk_setup(enum cpm_clk_target target, int clock, int mode) #ifdef CONFIG_8xx_GPIO struct cpm1_gpio16_chip { - struct of_mm_gpio_chip mm_gc; + struct gpio_chip gc; + void __iomem *regs; spinlock_t lock; /* shadowed data register to clear/set bits safely */ @@ -386,19 +387,17 @@ struct cpm1_gpio16_chip { int irq[16]; }; -static void cpm1_gpio16_save_regs(struct of_mm_gpio_chip *mm_gc) +static void cpm1_gpio16_save_regs(struct cpm1_gpio16_chip *cpm1_gc) { - struct cpm1_gpio16_chip *cpm1_gc = - container_of(mm_gc, struct cpm1_gpio16_chip, mm_gc); - struct cpm_ioport16 __iomem *iop = mm_gc->regs; + struct cpm_ioport16 __iomem *iop = cpm1_gc->regs; cpm1_gc->cpdata = in_be16(&iop->dat); } static int cpm1_gpio16_get(struct gpio_chip *gc, unsigned int gpio) { - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); - struct cpm_ioport16 __iomem *iop = mm_gc->regs; + struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(gc); + struct cpm_ioport16 __iomem *iop = cpm1_gc->regs; u16 pin_mask; pin_mask = 1 << (15 - gpio); @@ -406,11 +405,9 @@ static int cpm1_gpio16_get(struct gpio_chip *gc, unsigned int gpio) return !!(in_be16(&iop->dat) & pin_mask); } -static void __cpm1_gpio16_set(struct of_mm_gpio_chip *mm_gc, u16 pin_mask, - int value) +static void __cpm1_gpio16_set(struct cpm1_gpio16_chip *cpm1_gc, u16 pin_mask, int value) { - struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); - struct cpm_ioport16 __iomem *iop = mm_gc->regs; + struct cpm_ioport16 __iomem *iop = cpm1_gc->regs; if (value) cpm1_gc->cpdata |= pin_mask; @@ -422,38 +419,35 @@ static void __cpm1_gpio16_set(struct of_mm_gpio_chip *mm_gc, u16 pin_mask, static void cpm1_gpio16_set(struct gpio_chip *gc, unsigned int gpio, int value) { - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); - struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); + struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(gc); unsigned long flags; u16 pin_mask = 1 << (15 - gpio); spin_lock_irqsave(&cpm1_gc->lock, flags); - __cpm1_gpio16_set(mm_gc, pin_mask, value); + __cpm1_gpio16_set(cpm1_gc, pin_mask, value); spin_unlock_irqrestore(&cpm1_gc->lock, flags); } static int cpm1_gpio16_to_irq(struct gpio_chip *gc, unsigned int gpio) { - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); - struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); + struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(gc); return cpm1_gc->irq[gpio] ? : -ENXIO; } static int cpm1_gpio16_dir_out(struct gpio_chip *gc, unsigned int gpio, int val) { - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); - struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); - struct cpm_ioport16 __iomem *iop = mm_gc->regs; + struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(gc); + struct cpm_ioport16 __iomem *iop = cpm1_gc->regs; unsigned long flags; u16 pin_mask = 1 << (15 - gpio); spin_lock_irqsave(&cpm1_gc->lock, flags); setbits16(&iop->dir, pin_mask); - __cpm1_gpio16_set(mm_gc, pin_mask, val); + __cpm1_gpio16_set(cpm1_gc, pin_mask, val); spin_unlock_irqrestore(&cpm1_gc->lock, flags); @@ -462,9 +456,8 @@ static int cpm1_gpio16_dir_out(struct gpio_chip *gc, unsigned int gpio, int val) static int cpm1_gpio16_dir_in(struct gpio_chip *gc, unsigned int gpio) { - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); - struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); - struct cpm_ioport16 __iomem *iop = mm_gc->regs; + struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(gc); + struct cpm_ioport16 __iomem *iop = cpm1_gc->regs; unsigned long flags; u16 pin_mask = 1 << (15 - gpio); @@ -481,11 +474,10 @@ int cpm1_gpiochip_add16(struct device *dev) { struct device_node *np = dev->of_node; struct cpm1_gpio16_chip *cpm1_gc; - struct of_mm_gpio_chip *mm_gc; struct gpio_chip *gc; u16 mask; - cpm1_gc = kzalloc(sizeof(*cpm1_gc), GFP_KERNEL); + cpm1_gc = devm_kzalloc(dev, sizeof(*cpm1_gc), GFP_KERNEL); if (!cpm1_gc) return -ENOMEM; @@ -499,10 +491,8 @@ int cpm1_gpiochip_add16(struct device *dev) cpm1_gc->irq[i] = irq_of_parse_and_map(np, j++); } - mm_gc = &cpm1_gc->mm_gc; - gc = &mm_gc->gc; - - mm_gc->save_regs = cpm1_gpio16_save_regs; + gc = &cpm1_gc->gc; + gc->base = -1; gc->ngpio = 16; gc->direction_input = cpm1_gpio16_dir_in; gc->direction_output = cpm1_gpio16_dir_out; @@ -512,30 +502,39 @@ int cpm1_gpiochip_add16(struct device *dev) gc->parent = dev; gc->owner = THIS_MODULE; - return of_mm_gpiochip_add_data(np, mm_gc, cpm1_gc); + g->label = devm_kasprintf(dev, GFP_KERNEL, "%pOF", np); + if (!gc->label) + return -ENOMEM; + + cpm1_gc->regs = devm_of_iomap(dev, np, 0); + if (IS_ERR(cpm1_gc->regs)) + return PTR_ERR(cpm1_gc->regs); + + cpm1_gpio16_save_regs(cpm1_gc); + + return devm_gpiochip_add_data(dev, gc, cpm1_gc); } struct cpm1_gpio32_chip { - struct of_mm_gpio_chip mm_gc; + struct gpio_chip gc; + void __iomem *regs; spinlock_t lock; /* shadowed data register to clear/set bits safely */ u32 cpdata; }; -static void cpm1_gpio32_save_regs(struct of_mm_gpio_chip *mm_gc) +static void cpm1_gpio32_save_regs(struct cpm1_gpio32_chip *cpm1_gc) { - struct cpm1_gpio32_chip *cpm1_gc = - container_of(mm_gc, struct cpm1_gpio32_chip, mm_gc); - struct cpm_ioport32b __iomem *iop = mm_gc->regs; + struct cpm_ioport32b __iomem *iop = cpm1_gc->regs; cpm1_gc->cpdata = in_be32(&iop->dat); } static int cpm1_gpio32_get(struct gpio_chip *gc, unsigned int gpio) { - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); - struct cpm_ioport32b __iomem *iop = mm_gc->regs; + struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(gc); + struct cpm_ioport32b __iomem *iop = cpm1_gc->regs; u32 pin_mask; pin_mask = 1 << (31 - gpio); @@ -543,11 +542,9 @@ static int cpm1_gpio32_get(struct gpio_chip *gc, unsigned int gpio) return !!(in_be32(&iop->dat) & pin_mask); } -static void __cpm1_gpio32_set(struct of_mm_gpio_chip *mm_gc, u32 pin_mask, - int value) +static void __cpm1_gpio32_set(cpm1_gpio32_chip *cpm1_gc, u32 pin_mask, int value) { - struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); - struct cpm_ioport32b __iomem *iop = mm_gc->regs; + struct cpm_ioport32b __iomem *iop = cpm1_gc->regs; if (value) cpm1_gc->cpdata |= pin_mask; @@ -559,30 +556,28 @@ static void __cpm1_gpio32_set(struct of_mm_gpio_chip *mm_gc, u32 pin_mask, static void cpm1_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value) { - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); - struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); + struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(gc); unsigned long flags; u32 pin_mask = 1 << (31 - gpio); spin_lock_irqsave(&cpm1_gc->lock, flags); - __cpm1_gpio32_set(mm_gc, pin_mask, value); + __cpm1_gpio32_set(cpm1_gc, pin_mask, value); spin_unlock_irqrestore(&cpm1_gc->lock, flags); } static int cpm1_gpio32_dir_out(struct gpio_chip *gc, unsigned int gpio, int val) { - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); - struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); - struct cpm_ioport32b __iomem *iop = mm_gc->regs; + struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(gc); + struct cpm_ioport32b __iomem *iop = cpm1_gc->regs; unsigned long flags; u32 pin_mask = 1 << (31 - gpio); spin_lock_irqsave(&cpm1_gc->lock, flags); setbits32(&iop->dir, pin_mask); - __cpm1_gpio32_set(mm_gc, pin_mask, val); + __cpm1_gpio32_set(cpm1_gc, pin_mask, val); spin_unlock_irqrestore(&cpm1_gc->lock, flags); @@ -591,9 +586,8 @@ static int cpm1_gpio32_dir_out(struct gpio_chip *gc, unsigned int gpio, int val) static int cpm1_gpio32_dir_in(struct gpio_chip *gc, unsigned int gpio) { - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); - struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); - struct cpm_ioport32b __iomem *iop = mm_gc->regs; + struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(gc); + struct cpm_ioport32b __iomem *iop = cpm1_gc->regs; unsigned long flags; u32 pin_mask = 1 << (31 - gpio); @@ -610,19 +604,16 @@ int cpm1_gpiochip_add32(struct device *dev) { struct device_node *np = dev->of_node; struct cpm1_gpio32_chip *cpm1_gc; - struct of_mm_gpio_chip *mm_gc; struct gpio_chip *gc; - cpm1_gc = kzalloc(sizeof(*cpm1_gc), GFP_KERNEL); + cpm1_gc = devm_kzalloc(dev, sizeof(*cpm1_gc), GFP_KERNEL); if (!cpm1_gc) return -ENOMEM; spin_lock_init(&cpm1_gc->lock); - mm_gc = &cpm1_gc->mm_gc; - gc = &mm_gc->gc; - - mm_gc->save_regs = cpm1_gpio32_save_regs; + gc = &cpm1_gc->gc; + gc->base = -1; gc->ngpio = 32; gc->direction_input = cpm1_gpio32_dir_in; gc->direction_output = cpm1_gpio32_dir_out; @@ -631,7 +622,17 @@ int cpm1_gpiochip_add32(struct device *dev) gc->parent = dev; gc->owner = THIS_MODULE; - return of_mm_gpiochip_add_data(np, mm_gc, cpm1_gc); + g->label = devm_kasprintf(dev, GFP_KERNEL, "%pOF", np); + if (!gc->label) + return -ENOMEM; + + cpm1_gc->regs = devm_of_iomap(dev, np, 0); + if (IS_ERR(cpm1_gc->regs)) + return PTR_ERR(cpm1_gc->regs); + + cpm1_gpio32_save_regs(cpm1_gc); + + return devm_gpiochip_add_data(dev, gc, cpm1_gc); } #endif /* CONFIG_8xx_GPIO */ -- 2.43.0.rc1.1336.g36b5255a03ac ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] powerpc/8xx: Drop legacy-of-mm-gpiochip.h header 2024-11-15 13:38 [PATCH v1 1/1] powerpc/8xx: Drop legacy-of-mm-gpiochip.h header Andy Shevchenko @ 2024-11-16 10:44 ` Christophe Leroy 2024-11-18 9:20 ` Andy Shevchenko 0 siblings, 1 reply; 5+ messages in thread From: Christophe Leroy @ 2024-11-16 10:44 UTC (permalink / raw) To: Andy Shevchenko, linuxppc-dev, linux-kernel Cc: Michael Ellerman, Nicholas Piggin, Naveen N Rao, Madhavan Srinivasan Le 15/11/2024 à 14:38, Andy Shevchenko a écrit : > Remove legacy-of-mm-gpiochip.h header file, replace of_* functions > and structs with appropriate alternatives. Looks like you don't really have an alternative to of_mm_gpiochip_add_data(), you are replacing one single line by 11 new ones, and that is done twice (once for cpm1_gpiochip_add16(), once for cpm1_gpiochip_add32()). > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > arch/powerpc/platforms/8xx/cpm1.c | 119 +++++++++++++++--------------- Does not build: CC arch/powerpc/platforms/8xx/cpm1.o arch/powerpc/platforms/8xx/cpm1.c: In function 'cpm1_gpiochip_add16': arch/powerpc/platforms/8xx/cpm1.c:505:2: error: 'g' undeclared (first use in this function); did you mean 'gc'? g->label = devm_kasprintf(dev, GFP_KERNEL, "%pOF", np); ^ gc arch/powerpc/platforms/8xx/cpm1.c:505:2: note: each undeclared identifier is reported only once for each function it appears in arch/powerpc/platforms/8xx/cpm1.c:509:18: error: too few arguments to function 'devm_of_iomap' cpm1_gc->regs = devm_of_iomap(dev, np, 0); ^~~~~~~~~~~~~ In file included from ./include/linux/dma-mapping.h:8:0, from arch/powerpc/platforms/8xx/cpm1.c:27: ./include/linux/device.h:372:15: note: declared here void __iomem *devm_of_iomap(struct device *dev, ^~~~~~~~~~~~~ arch/powerpc/platforms/8xx/cpm1.c: At top level: arch/powerpc/platforms/8xx/cpm1.c:545:31: error: unknown type name 'cpm1_gpio32_chip' static void __cpm1_gpio32_set(cpm1_gpio32_chip *cpm1_gc, u32 pin_mask, int value) ^~~~~~~~~~~~~~~~ arch/powerpc/platforms/8xx/cpm1.c: In function 'cpm1_gpio32_set': arch/powerpc/platforms/8xx/cpm1.c:565:2: error: implicit declaration of function '__cpm1_gpio32_set'; did you mean 'cpm1_gpio32_set'? [-Werror=implicit-function-declaration] __cpm1_gpio32_set(cpm1_gc, pin_mask, value); ^~~~~~~~~~~~~~~~~ cpm1_gpio32_set arch/powerpc/platforms/8xx/cpm1.c: In function 'cpm1_gpiochip_add32': arch/powerpc/platforms/8xx/cpm1.c:625:2: error: 'g' undeclared (first use in this function); did you mean 'gc'? g->label = devm_kasprintf(dev, GFP_KERNEL, "%pOF", np); ^ gc arch/powerpc/platforms/8xx/cpm1.c:629:18: error: too few arguments to function 'devm_of_iomap' cpm1_gc->regs = devm_of_iomap(dev, np, 0); ^~~~~~~~~~~~~ In file included from ./include/linux/dma-mapping.h:8:0, from arch/powerpc/platforms/8xx/cpm1.c:27: ./include/linux/device.h:372:15: note: declared here void __iomem *devm_of_iomap(struct device *dev, ^~~~~~~~~~~~~ > 1 file changed, 60 insertions(+), 59 deletions(-) > > diff --git a/arch/powerpc/platforms/8xx/cpm1.c b/arch/powerpc/platforms/8xx/cpm1.c > index b24d4102fbf6..1262bff5ba2e 100644 > --- a/arch/powerpc/platforms/8xx/cpm1.c > +++ b/arch/powerpc/platforms/8xx/cpm1.c > @@ -45,7 +45,7 @@ > #include <sysdev/fsl_soc.h> > > #ifdef CONFIG_8xx_GPIO > -#include <linux/gpio/legacy-of-mm-gpiochip.h> > +#include <linux/gpio/driver.h> > #endif > > #define CPM_MAP_SIZE (0x4000) > @@ -376,7 +376,8 @@ int __init cpm1_clk_setup(enum cpm_clk_target target, int clock, int mode) > #ifdef CONFIG_8xx_GPIO > > struct cpm1_gpio16_chip { > - struct of_mm_gpio_chip mm_gc; > + struct gpio_chip gc; > + void __iomem *regs; > spinlock_t lock; > > /* shadowed data register to clear/set bits safely */ > @@ -386,19 +387,17 @@ struct cpm1_gpio16_chip { > int irq[16]; > }; > > -static void cpm1_gpio16_save_regs(struct of_mm_gpio_chip *mm_gc) > +static void cpm1_gpio16_save_regs(struct cpm1_gpio16_chip *cpm1_gc) > { > - struct cpm1_gpio16_chip *cpm1_gc = > - container_of(mm_gc, struct cpm1_gpio16_chip, mm_gc); > - struct cpm_ioport16 __iomem *iop = mm_gc->regs; > + struct cpm_ioport16 __iomem *iop = cpm1_gc->regs; > > cpm1_gc->cpdata = in_be16(&iop->dat); > } > > static int cpm1_gpio16_get(struct gpio_chip *gc, unsigned int gpio) > { > - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > - struct cpm_ioport16 __iomem *iop = mm_gc->regs; > + struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(gc); > + struct cpm_ioport16 __iomem *iop = cpm1_gc->regs; > u16 pin_mask; > > pin_mask = 1 << (15 - gpio); > @@ -406,11 +405,9 @@ static int cpm1_gpio16_get(struct gpio_chip *gc, unsigned int gpio) > return !!(in_be16(&iop->dat) & pin_mask); > } > > -static void __cpm1_gpio16_set(struct of_mm_gpio_chip *mm_gc, u16 pin_mask, > - int value) > +static void __cpm1_gpio16_set(struct cpm1_gpio16_chip *cpm1_gc, u16 pin_mask, int value) > { > - struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); > - struct cpm_ioport16 __iomem *iop = mm_gc->regs; > + struct cpm_ioport16 __iomem *iop = cpm1_gc->regs; > > if (value) > cpm1_gc->cpdata |= pin_mask; > @@ -422,38 +419,35 @@ static void __cpm1_gpio16_set(struct of_mm_gpio_chip *mm_gc, u16 pin_mask, > > static void cpm1_gpio16_set(struct gpio_chip *gc, unsigned int gpio, int value) > { > - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > - struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); > + struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(gc); > unsigned long flags; > u16 pin_mask = 1 << (15 - gpio); > > spin_lock_irqsave(&cpm1_gc->lock, flags); > > - __cpm1_gpio16_set(mm_gc, pin_mask, value); > + __cpm1_gpio16_set(cpm1_gc, pin_mask, value); > > spin_unlock_irqrestore(&cpm1_gc->lock, flags); > } > > static int cpm1_gpio16_to_irq(struct gpio_chip *gc, unsigned int gpio) > { > - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > - struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); > + struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(gc); > > return cpm1_gc->irq[gpio] ? : -ENXIO; > } > > static int cpm1_gpio16_dir_out(struct gpio_chip *gc, unsigned int gpio, int val) > { > - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > - struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); > - struct cpm_ioport16 __iomem *iop = mm_gc->regs; > + struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(gc); > + struct cpm_ioport16 __iomem *iop = cpm1_gc->regs; > unsigned long flags; > u16 pin_mask = 1 << (15 - gpio); > > spin_lock_irqsave(&cpm1_gc->lock, flags); > > setbits16(&iop->dir, pin_mask); > - __cpm1_gpio16_set(mm_gc, pin_mask, val); > + __cpm1_gpio16_set(cpm1_gc, pin_mask, val); > > spin_unlock_irqrestore(&cpm1_gc->lock, flags); > > @@ -462,9 +456,8 @@ static int cpm1_gpio16_dir_out(struct gpio_chip *gc, unsigned int gpio, int val) > > static int cpm1_gpio16_dir_in(struct gpio_chip *gc, unsigned int gpio) > { > - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > - struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); > - struct cpm_ioport16 __iomem *iop = mm_gc->regs; > + struct cpm1_gpio16_chip *cpm1_gc = gpiochip_get_data(gc); > + struct cpm_ioport16 __iomem *iop = cpm1_gc->regs; > unsigned long flags; > u16 pin_mask = 1 << (15 - gpio); > > @@ -481,11 +474,10 @@ int cpm1_gpiochip_add16(struct device *dev) > { > struct device_node *np = dev->of_node; > struct cpm1_gpio16_chip *cpm1_gc; > - struct of_mm_gpio_chip *mm_gc; > struct gpio_chip *gc; > u16 mask; > > - cpm1_gc = kzalloc(sizeof(*cpm1_gc), GFP_KERNEL); > + cpm1_gc = devm_kzalloc(dev, sizeof(*cpm1_gc), GFP_KERNEL); > if (!cpm1_gc) > return -ENOMEM; > > @@ -499,10 +491,8 @@ int cpm1_gpiochip_add16(struct device *dev) > cpm1_gc->irq[i] = irq_of_parse_and_map(np, j++); > } > > - mm_gc = &cpm1_gc->mm_gc; > - gc = &mm_gc->gc; > - > - mm_gc->save_regs = cpm1_gpio16_save_regs; > + gc = &cpm1_gc->gc; > + gc->base = -1; > gc->ngpio = 16; > gc->direction_input = cpm1_gpio16_dir_in; > gc->direction_output = cpm1_gpio16_dir_out; > @@ -512,30 +502,39 @@ int cpm1_gpiochip_add16(struct device *dev) > gc->parent = dev; > gc->owner = THIS_MODULE; > > - return of_mm_gpiochip_add_data(np, mm_gc, cpm1_gc); > + g->label = devm_kasprintf(dev, GFP_KERNEL, "%pOF", np); > + if (!gc->label) > + return -ENOMEM; > + > + cpm1_gc->regs = devm_of_iomap(dev, np, 0); > + if (IS_ERR(cpm1_gc->regs)) > + return PTR_ERR(cpm1_gc->regs); > + > + cpm1_gpio16_save_regs(cpm1_gc); > + > + return devm_gpiochip_add_data(dev, gc, cpm1_gc); > } > > struct cpm1_gpio32_chip { > - struct of_mm_gpio_chip mm_gc; > + struct gpio_chip gc; > + void __iomem *regs; > spinlock_t lock; > > /* shadowed data register to clear/set bits safely */ > u32 cpdata; > }; > > -static void cpm1_gpio32_save_regs(struct of_mm_gpio_chip *mm_gc) > +static void cpm1_gpio32_save_regs(struct cpm1_gpio32_chip *cpm1_gc) > { > - struct cpm1_gpio32_chip *cpm1_gc = > - container_of(mm_gc, struct cpm1_gpio32_chip, mm_gc); > - struct cpm_ioport32b __iomem *iop = mm_gc->regs; > + struct cpm_ioport32b __iomem *iop = cpm1_gc->regs; > > cpm1_gc->cpdata = in_be32(&iop->dat); > } > > static int cpm1_gpio32_get(struct gpio_chip *gc, unsigned int gpio) > { > - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > - struct cpm_ioport32b __iomem *iop = mm_gc->regs; > + struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(gc); > + struct cpm_ioport32b __iomem *iop = cpm1_gc->regs; > u32 pin_mask; > > pin_mask = 1 << (31 - gpio); > @@ -543,11 +542,9 @@ static int cpm1_gpio32_get(struct gpio_chip *gc, unsigned int gpio) > return !!(in_be32(&iop->dat) & pin_mask); > } > > -static void __cpm1_gpio32_set(struct of_mm_gpio_chip *mm_gc, u32 pin_mask, > - int value) > +static void __cpm1_gpio32_set(cpm1_gpio32_chip *cpm1_gc, u32 pin_mask, int value) > { > - struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); > - struct cpm_ioport32b __iomem *iop = mm_gc->regs; > + struct cpm_ioport32b __iomem *iop = cpm1_gc->regs; > > if (value) > cpm1_gc->cpdata |= pin_mask; > @@ -559,30 +556,28 @@ static void __cpm1_gpio32_set(struct of_mm_gpio_chip *mm_gc, u32 pin_mask, > > static void cpm1_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value) > { > - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > - struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); > + struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(gc); > unsigned long flags; > u32 pin_mask = 1 << (31 - gpio); > > spin_lock_irqsave(&cpm1_gc->lock, flags); > > - __cpm1_gpio32_set(mm_gc, pin_mask, value); > + __cpm1_gpio32_set(cpm1_gc, pin_mask, value); > > spin_unlock_irqrestore(&cpm1_gc->lock, flags); > } > > static int cpm1_gpio32_dir_out(struct gpio_chip *gc, unsigned int gpio, int val) > { > - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > - struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); > - struct cpm_ioport32b __iomem *iop = mm_gc->regs; > + struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(gc); > + struct cpm_ioport32b __iomem *iop = cpm1_gc->regs; > unsigned long flags; > u32 pin_mask = 1 << (31 - gpio); > > spin_lock_irqsave(&cpm1_gc->lock, flags); > > setbits32(&iop->dir, pin_mask); > - __cpm1_gpio32_set(mm_gc, pin_mask, val); > + __cpm1_gpio32_set(cpm1_gc, pin_mask, val); > > spin_unlock_irqrestore(&cpm1_gc->lock, flags); > > @@ -591,9 +586,8 @@ static int cpm1_gpio32_dir_out(struct gpio_chip *gc, unsigned int gpio, int val) > > static int cpm1_gpio32_dir_in(struct gpio_chip *gc, unsigned int gpio) > { > - struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > - struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(&mm_gc->gc); > - struct cpm_ioport32b __iomem *iop = mm_gc->regs; > + struct cpm1_gpio32_chip *cpm1_gc = gpiochip_get_data(gc); > + struct cpm_ioport32b __iomem *iop = cpm1_gc->regs; > unsigned long flags; > u32 pin_mask = 1 << (31 - gpio); > > @@ -610,19 +604,16 @@ int cpm1_gpiochip_add32(struct device *dev) > { > struct device_node *np = dev->of_node; > struct cpm1_gpio32_chip *cpm1_gc; > - struct of_mm_gpio_chip *mm_gc; > struct gpio_chip *gc; > > - cpm1_gc = kzalloc(sizeof(*cpm1_gc), GFP_KERNEL); > + cpm1_gc = devm_kzalloc(dev, sizeof(*cpm1_gc), GFP_KERNEL); > if (!cpm1_gc) > return -ENOMEM; > > spin_lock_init(&cpm1_gc->lock); > > - mm_gc = &cpm1_gc->mm_gc; > - gc = &mm_gc->gc; > - > - mm_gc->save_regs = cpm1_gpio32_save_regs; > + gc = &cpm1_gc->gc; > + gc->base = -1; > gc->ngpio = 32; > gc->direction_input = cpm1_gpio32_dir_in; > gc->direction_output = cpm1_gpio32_dir_out; > @@ -631,7 +622,17 @@ int cpm1_gpiochip_add32(struct device *dev) > gc->parent = dev; > gc->owner = THIS_MODULE; > > - return of_mm_gpiochip_add_data(np, mm_gc, cpm1_gc); > + g->label = devm_kasprintf(dev, GFP_KERNEL, "%pOF", np); > + if (!gc->label) > + return -ENOMEM; > + > + cpm1_gc->regs = devm_of_iomap(dev, np, 0); > + if (IS_ERR(cpm1_gc->regs)) > + return PTR_ERR(cpm1_gc->regs); > + > + cpm1_gpio32_save_regs(cpm1_gc); > + > + return devm_gpiochip_add_data(dev, gc, cpm1_gc); > } > > #endif /* CONFIG_8xx_GPIO */ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] powerpc/8xx: Drop legacy-of-mm-gpiochip.h header 2024-11-16 10:44 ` Christophe Leroy @ 2024-11-18 9:20 ` Andy Shevchenko 2024-11-18 11:02 ` Christophe Leroy 0 siblings, 1 reply; 5+ messages in thread From: Andy Shevchenko @ 2024-11-18 9:20 UTC (permalink / raw) To: Christophe Leroy Cc: linuxppc-dev, linux-kernel, Michael Ellerman, Nicholas Piggin, Naveen N Rao, Madhavan Srinivasan On Sat, Nov 16, 2024 at 11:44:35AM +0100, Christophe Leroy wrote: > Le 15/11/2024 à 14:38, Andy Shevchenko a écrit : > > Remove legacy-of-mm-gpiochip.h header file, replace of_* functions > > and structs with appropriate alternatives. > > Looks like you don't really have an alternative to > of_mm_gpiochip_add_data(), you are replacing one single line by 11 new ones, > and that is done twice (once for cpm1_gpiochip_add16(), once for > cpm1_gpiochip_add32()). True, but that's the issue that we have of_specific API. If someone propose the common API for the agnostic approach, it would be nice, but I am not the one. And TBH I do not see the advantage of it right now as almost every GPIO driver is using its own labeling schema (*). Note, that this patch also fixes a memory leak as a side effect. *) the legacy API is mostly used by PPC code, do you want that of_mm_* thingy to be moved to PPC specific code instead of killing it? Would be done this way as well. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > --- > > arch/powerpc/platforms/8xx/cpm1.c | 119 +++++++++++++++--------------- > > Does not build: Crap, I most likely built something else and not these files... I have carefully build-test this for v2. Note to myself: Never ever send the patches on Friday evenings! :-) -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] powerpc/8xx: Drop legacy-of-mm-gpiochip.h header 2024-11-18 9:20 ` Andy Shevchenko @ 2024-11-18 11:02 ` Christophe Leroy 2024-11-18 12:32 ` Andy Shevchenko 0 siblings, 1 reply; 5+ messages in thread From: Christophe Leroy @ 2024-11-18 11:02 UTC (permalink / raw) To: Andy Shevchenko Cc: linuxppc-dev, linux-kernel, Michael Ellerman, Nicholas Piggin, Naveen N Rao, Madhavan Srinivasan Le 18/11/2024 à 10:20, Andy Shevchenko a écrit : > On Sat, Nov 16, 2024 at 11:44:35AM +0100, Christophe Leroy wrote: >> Le 15/11/2024 à 14:38, Andy Shevchenko a écrit : >>> Remove legacy-of-mm-gpiochip.h header file, replace of_* functions >>> and structs with appropriate alternatives. >> >> Looks like you don't really have an alternative to >> of_mm_gpiochip_add_data(), you are replacing one single line by 11 new ones, >> and that is done twice (once for cpm1_gpiochip_add16(), once for >> cpm1_gpiochip_add32()). > > True, but that's the issue that we have of_specific API. If someone propose > the common API for the agnostic approach, it would be nice, but I am not > the one. And TBH I do not see the advantage of it right now as almost every > GPIO driver is using its own labeling schema (*). Note, that this patch also > fixes a memory leak as a side effect. Can you explain that in the commit message instead of saying you use appropriate alternatives that do not exist ? Don't hesitate to mention the memory leak it in the commit message as well. > > *) the legacy API is mostly used by PPC code, do you want that of_mm_* thingy > to be moved to PPC specific code instead of killing it? Would be done this > way as well. No no, your change is ok for me, just need an accurate commit message. > >>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>> --- >>> arch/powerpc/platforms/8xx/cpm1.c | 119 +++++++++++++++--------------- >> >> Does not build: > > Crap, I most likely built something else and not these files... > I have carefully build-test this for v2. Just use mpc885_ads_defconfig > > Note to myself: Never ever send the patches on Friday evenings! :-) > Christophe ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] powerpc/8xx: Drop legacy-of-mm-gpiochip.h header 2024-11-18 11:02 ` Christophe Leroy @ 2024-11-18 12:32 ` Andy Shevchenko 0 siblings, 0 replies; 5+ messages in thread From: Andy Shevchenko @ 2024-11-18 12:32 UTC (permalink / raw) To: Christophe Leroy Cc: linuxppc-dev, linux-kernel, Michael Ellerman, Nicholas Piggin, Naveen N Rao, Madhavan Srinivasan On Mon, Nov 18, 2024 at 12:02:44PM +0100, Christophe Leroy wrote: > Le 18/11/2024 à 10:20, Andy Shevchenko a écrit : > > On Sat, Nov 16, 2024 at 11:44:35AM +0100, Christophe Leroy wrote: > > > Le 15/11/2024 à 14:38, Andy Shevchenko a écrit : > > > > Remove legacy-of-mm-gpiochip.h header file, replace of_* functions > > > > and structs with appropriate alternatives. > > > > > > Looks like you don't really have an alternative to > > > of_mm_gpiochip_add_data(), you are replacing one single line by 11 new ones, > > > and that is done twice (once for cpm1_gpiochip_add16(), once for > > > cpm1_gpiochip_add32()). Note, the overall statistics is +1 LoC only! > > True, but that's the issue that we have of_specific API. If someone propose > > the common API for the agnostic approach, it would be nice, but I am not > > the one. And TBH I do not see the advantage of it right now as almost every > > GPIO driver is using its own labeling schema (*). Note, that this patch also > > fixes a memory leak as a side effect. > > Can you explain that in the commit message instead of saying you use > appropriate alternatives that do not exist ? > > Don't hesitate to mention the memory leak it in the commit message as well. > > > *) the legacy API is mostly used by PPC code, do you want that of_mm_* thingy > > to be moved to PPC specific code instead of killing it? Would be done this > > way as well. > > No no, your change is ok for me, just need an accurate commit message. Fine, just sent a v2, which is now compile-tested as suggested below. ... > > > Does not build: > > > > Crap, I most likely built something else and not these files... > > I have carefully build-test this for v2. > > Just use mpc885_ads_defconfig Thanks, that is helpful! -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-11-18 12:33 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-11-15 13:38 [PATCH v1 1/1] powerpc/8xx: Drop legacy-of-mm-gpiochip.h header Andy Shevchenko 2024-11-16 10:44 ` Christophe Leroy 2024-11-18 9:20 ` Andy Shevchenko 2024-11-18 11:02 ` Christophe Leroy 2024-11-18 12:32 ` Andy Shevchenko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.