* [PATCH 0/3] gpio-langwell updates
@ 2012-12-19 1:52 David Cohen
2012-12-19 1:52 ` [PATCH 1/3] gpio-langwell: cleanup driver David Cohen
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: David Cohen @ 2012-12-19 1:52 UTC (permalink / raw)
To: grant.likely; +Cc: linux-kernel, alan, David Cohen
Hi,
Here goes some Intel updates done on gpio-langwell driver.
Kind regards,
David
---
David Cohen (2):
gpio-langwell: cleanup driver
gpio-langwell: update pci device table
Li, Ning (1):
gpio-langwell: implement irq shutdown interface
drivers/gpio/gpio-langwell.c | 80 ++++++++++++++++++++----------------------
1 file changed, 39 insertions(+), 41 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] gpio-langwell: cleanup driver 2012-12-19 1:52 [PATCH 0/3] gpio-langwell updates David Cohen @ 2012-12-19 1:52 ` David Cohen 2012-12-20 1:18 ` Grant Likely 2012-12-19 1:52 ` [PATCH 2/3] gpio-langwell: update pci device table David Cohen 2012-12-19 1:52 ` [PATCH 3/3] gpio-langwell: implement irq shutdown interface David Cohen 2 siblings, 1 reply; 10+ messages in thread From: David Cohen @ 2012-12-19 1:52 UTC (permalink / raw) To: grant.likely; +Cc: linux-kernel, alan, David Cohen This patch cleans up cosmetic issues, remove useless functions and add to_lnw_priv() macro to replace many usages of container_of(). Change-Id: I70a8fadd20a42493271d91633739bdddff19c8d8 Signed-off-by: David Cohen <david.a.cohen@intel.com> --- drivers/gpio/gpio-langwell.c | 64 ++++++++++++++---------------------------- 1 file changed, 21 insertions(+), 43 deletions(-) diff --git a/drivers/gpio/gpio-langwell.c b/drivers/gpio/gpio-langwell.c index 202a992..8220c04 100644 --- a/drivers/gpio/gpio-langwell.c +++ b/drivers/gpio/gpio-langwell.c @@ -71,10 +71,12 @@ struct lnw_gpio { struct irq_domain *domain; }; +#define to_lnw_priv(chip) container_of(chip, struct lnw_gpio, chip) + static void __iomem *gpio_reg(struct gpio_chip *chip, unsigned offset, enum GPIO_REG reg_type) { - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); + struct lnw_gpio *lnw = to_lnw_priv(chip); unsigned nreg = chip->ngpio / 32; u8 reg = offset / 32; void __iomem *ptr; @@ -86,7 +88,7 @@ static void __iomem *gpio_reg(struct gpio_chip *chip, unsigned offset, static void __iomem *gpio_reg_2bit(struct gpio_chip *chip, unsigned offset, enum GPIO_REG reg_type) { - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); + struct lnw_gpio *lnw = to_lnw_priv(chip); unsigned nreg = chip->ngpio / 32; u8 reg = offset / 16; void __iomem *ptr; @@ -130,7 +132,7 @@ static void lnw_gpio_set(struct gpio_chip *chip, unsigned offset, int value) static int lnw_gpio_direction_input(struct gpio_chip *chip, unsigned offset) { - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); + struct lnw_gpio *lnw = to_lnw_priv(chip); void __iomem *gpdr = gpio_reg(chip, offset, GPDR); u32 value; unsigned long flags; @@ -153,7 +155,7 @@ static int lnw_gpio_direction_input(struct gpio_chip *chip, unsigned offset) static int lnw_gpio_direction_output(struct gpio_chip *chip, unsigned offset, int value) { - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); + struct lnw_gpio *lnw = to_lnw_priv(chip); void __iomem *gpdr = gpio_reg(chip, offset, GPDR); unsigned long flags; @@ -176,7 +178,7 @@ static int lnw_gpio_direction_output(struct gpio_chip *chip, static int lnw_gpio_to_irq(struct gpio_chip *chip, unsigned offset) { - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); + struct lnw_gpio *lnw = to_lnw_priv(chip); return irq_create_mapping(lnw->domain, offset); } @@ -215,19 +217,14 @@ static int lnw_irq_type(struct irq_data *d, unsigned type) return 0; } -static void lnw_irq_unmask(struct irq_data *d) -{ -} - -static void lnw_irq_mask(struct irq_data *d) -{ -} +static void lnw_irq_noop(struct irq_data *d) {} static struct irq_chip lnw_irqchip = { .name = "LNW-GPIO", - .irq_mask = lnw_irq_mask, - .irq_unmask = lnw_irq_unmask, + .irq_mask = lnw_irq_noop, + .irq_unmask = lnw_irq_noop, .irq_set_type = lnw_irq_type, + .irq_ack = lnw_irq_noop, }; static DEFINE_PCI_DEVICE_TABLE(lnw_gpio_ids) = { /* pin number */ @@ -299,17 +296,6 @@ static const struct irq_domain_ops lnw_gpio_irq_ops = { .xlate = irq_domain_xlate_twocell, }; -#ifdef CONFIG_PM -static int lnw_gpio_runtime_resume(struct device *dev) -{ - return 0; -} - -static int lnw_gpio_runtime_suspend(struct device *dev) -{ - return 0; -} - static int lnw_gpio_runtime_idle(struct device *dev) { int err = pm_schedule_suspend(dev, 500); @@ -320,16 +306,8 @@ static int lnw_gpio_runtime_idle(struct device *dev) return -EBUSY; } -#else -#define lnw_gpio_runtime_suspend NULL -#define lnw_gpio_runtime_resume NULL -#define lnw_gpio_runtime_idle NULL -#endif - static const struct dev_pm_ops lnw_gpio_pm_ops = { - .runtime_suspend = lnw_gpio_runtime_suspend, - .runtime_resume = lnw_gpio_runtime_resume, - .runtime_idle = lnw_gpio_runtime_idle, + SET_RUNTIME_PM_OPS(NULL, NULL, lnw_gpio_runtime_idle) }; static int __devinit lnw_gpio_probe(struct pci_dev *pdev, @@ -349,7 +327,7 @@ static int __devinit lnw_gpio_probe(struct pci_dev *pdev, retval = pci_request_regions(pdev, "langwell_gpio"); if (retval) { dev_err(&pdev->dev, "error requesting resources\n"); - goto err2; + goto err; } /* get the gpio_base from bar1 */ start = pci_resource_start(pdev, 1); @@ -358,7 +336,7 @@ static int __devinit lnw_gpio_probe(struct pci_dev *pdev, if (!base) { dev_err(&pdev->dev, "error mapping bar1\n"); retval = -EFAULT; - goto err3; + goto err2; } gpio_base = *((u32 *)base + 1); /* release the IO mapping, since we already get the info from bar1 */ @@ -370,21 +348,21 @@ static int __devinit lnw_gpio_probe(struct pci_dev *pdev, if (!base) { dev_err(&pdev->dev, "error mapping bar0\n"); retval = -EFAULT; - goto err3; + goto err2; } - lnw = devm_kzalloc(&pdev->dev, sizeof(struct lnw_gpio), GFP_KERNEL); + lnw = devm_kzalloc(&pdev->dev, sizeof(*lnw), GFP_KERNEL); if (!lnw) { dev_err(&pdev->dev, "can't allocate langwell_gpio chip data\n"); retval = -ENOMEM; - goto err3; + goto err2; } lnw->domain = irq_domain_add_linear(pdev->dev.of_node, ngpio, &lnw_gpio_irq_ops, lnw); if (!lnw->domain) { retval = -ENOMEM; - goto err3; + goto err2; } lnw->reg_base = base; @@ -403,7 +381,7 @@ static int __devinit lnw_gpio_probe(struct pci_dev *pdev, retval = gpiochip_add(&lnw->chip); if (retval) { dev_err(&pdev->dev, "langwell gpiochip_add error %d\n", retval); - goto err3; + goto err2; } lnw_irq_init_hw(lnw); @@ -418,9 +396,9 @@ static int __devinit lnw_gpio_probe(struct pci_dev *pdev, return 0; -err3: - pci_release_regions(pdev); err2: + pci_release_regions(pdev); +err: pci_disable_device(pdev); return retval; } -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] gpio-langwell: cleanup driver 2012-12-19 1:52 ` [PATCH 1/3] gpio-langwell: cleanup driver David Cohen @ 2012-12-20 1:18 ` Grant Likely 2012-12-20 21:35 ` David Cohen 0 siblings, 1 reply; 10+ messages in thread From: Grant Likely @ 2012-12-20 1:18 UTC (permalink / raw) To: David Cohen; +Cc: linux-kernel, alan, David Cohen On Tue, 18 Dec 2012 17:52:11 -0800, David Cohen <david.a.cohen@intel.com> wrote: > This patch cleans up cosmetic issues, remove useless functions and add > to_lnw_priv() macro to replace many usages of container_of(). > > Change-Id: I70a8fadd20a42493271d91633739bdddff19c8d8 > Signed-off-by: David Cohen <david.a.cohen@intel.com> Hi David. Comments below... > --- > drivers/gpio/gpio-langwell.c | 64 ++++++++++++++---------------------------- > 1 file changed, 21 insertions(+), 43 deletions(-) > > diff --git a/drivers/gpio/gpio-langwell.c b/drivers/gpio/gpio-langwell.c > index 202a992..8220c04 100644 > --- a/drivers/gpio/gpio-langwell.c > +++ b/drivers/gpio/gpio-langwell.c > @@ -71,10 +71,12 @@ struct lnw_gpio { > struct irq_domain *domain; > }; > > +#define to_lnw_priv(chip) container_of(chip, struct lnw_gpio, chip) > + > static void __iomem *gpio_reg(struct gpio_chip *chip, unsigned offset, > enum GPIO_REG reg_type) > { > - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); > + struct lnw_gpio *lnw = to_lnw_priv(chip); > unsigned nreg = chip->ngpio / 32; > u8 reg = offset / 32; > void __iomem *ptr; > @@ -86,7 +88,7 @@ static void __iomem *gpio_reg(struct gpio_chip *chip, unsigned offset, > static void __iomem *gpio_reg_2bit(struct gpio_chip *chip, unsigned offset, > enum GPIO_REG reg_type) > { > - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); > + struct lnw_gpio *lnw = to_lnw_priv(chip); > unsigned nreg = chip->ngpio / 32; > u8 reg = offset / 16; > void __iomem *ptr; > @@ -130,7 +132,7 @@ static void lnw_gpio_set(struct gpio_chip *chip, unsigned offset, int value) > > static int lnw_gpio_direction_input(struct gpio_chip *chip, unsigned offset) > { > - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); > + struct lnw_gpio *lnw = to_lnw_priv(chip); > void __iomem *gpdr = gpio_reg(chip, offset, GPDR); > u32 value; > unsigned long flags; > @@ -153,7 +155,7 @@ static int lnw_gpio_direction_input(struct gpio_chip *chip, unsigned offset) > static int lnw_gpio_direction_output(struct gpio_chip *chip, > unsigned offset, int value) > { > - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); > + struct lnw_gpio *lnw = to_lnw_priv(chip); > void __iomem *gpdr = gpio_reg(chip, offset, GPDR); > unsigned long flags; > > @@ -176,7 +178,7 @@ static int lnw_gpio_direction_output(struct gpio_chip *chip, > > static int lnw_gpio_to_irq(struct gpio_chip *chip, unsigned offset) > { > - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); > + struct lnw_gpio *lnw = to_lnw_priv(chip); > return irq_create_mapping(lnw->domain, offset); > } Nice cleanup above. > > @@ -215,19 +217,14 @@ static int lnw_irq_type(struct irq_data *d, unsigned type) > return 0; > } > > -static void lnw_irq_unmask(struct irq_data *d) > -{ > -} > - > -static void lnw_irq_mask(struct irq_data *d) > -{ > -} > +static void lnw_irq_noop(struct irq_data *d) {} Umm, this looks entirely wrong. There needs to be either mask/unmask or enable/disable ops for irq_chips. Yes I see that this patch is just consolidating two empty functions, but they are two empty functions that appear to be completely wrong. > > static struct irq_chip lnw_irqchip = { > .name = "LNW-GPIO", > - .irq_mask = lnw_irq_mask, > - .irq_unmask = lnw_irq_unmask, > + .irq_mask = lnw_irq_noop, > + .irq_unmask = lnw_irq_noop, > .irq_set_type = lnw_irq_type, > + .irq_ack = lnw_irq_noop, > }; > > static DEFINE_PCI_DEVICE_TABLE(lnw_gpio_ids) = { /* pin number */ > @@ -299,17 +296,6 @@ static const struct irq_domain_ops lnw_gpio_irq_ops = { > .xlate = irq_domain_xlate_twocell, > }; > > -#ifdef CONFIG_PM > -static int lnw_gpio_runtime_resume(struct device *dev) > -{ > - return 0; > -} > - > -static int lnw_gpio_runtime_suspend(struct device *dev) > -{ > - return 0; > -} > - > static int lnw_gpio_runtime_idle(struct device *dev) > { > int err = pm_schedule_suspend(dev, 500); > @@ -320,16 +306,8 @@ static int lnw_gpio_runtime_idle(struct device *dev) > return -EBUSY; > } > > -#else > -#define lnw_gpio_runtime_suspend NULL > -#define lnw_gpio_runtime_resume NULL > -#define lnw_gpio_runtime_idle NULL > -#endif > - > static const struct dev_pm_ops lnw_gpio_pm_ops = { > - .runtime_suspend = lnw_gpio_runtime_suspend, > - .runtime_resume = lnw_gpio_runtime_resume, > - .runtime_idle = lnw_gpio_runtime_idle, > + SET_RUNTIME_PM_OPS(NULL, NULL, lnw_gpio_runtime_idle) > }; Also good. > > static int __devinit lnw_gpio_probe(struct pci_dev *pdev, > @@ -349,7 +327,7 @@ static int __devinit lnw_gpio_probe(struct pci_dev *pdev, > retval = pci_request_regions(pdev, "langwell_gpio"); > if (retval) { > dev_err(&pdev->dev, "error requesting resources\n"); > - goto err2; > + goto err; Renaming the labels just increases the noise in the diff. I prefer to see labels in the form "err-what-i-just-tried-to-do:" so they don't need to get reshuffled every time the code logic changes. There is no good reason for this change. Please drop it. g. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] gpio-langwell: cleanup driver 2012-12-20 1:18 ` Grant Likely @ 2012-12-20 21:35 ` David Cohen 2012-12-20 21:56 ` Grant Likely 0 siblings, 1 reply; 10+ messages in thread From: David Cohen @ 2012-12-20 21:35 UTC (permalink / raw) To: Grant Likely; +Cc: linux-kernel, alan On 12/19/2012 05:18 PM, Grant Likely wrote: > On Tue, 18 Dec 2012 17:52:11 -0800, David Cohen <david.a.cohen@intel.com> wrote: >> This patch cleans up cosmetic issues, remove useless functions and add >> to_lnw_priv() macro to replace many usages of container_of(). >> >> Change-Id: I70a8fadd20a42493271d91633739bdddff19c8d8 >> Signed-off-by: David Cohen <david.a.cohen@intel.com> > Hi David. Comments below... Hi Grant, Thanks for comments. Let me go through them. > >> --- >> drivers/gpio/gpio-langwell.c | 64 ++++++++++++++---------------------------- >> 1 file changed, 21 insertions(+), 43 deletions(-) >> >> diff --git a/drivers/gpio/gpio-langwell.c b/drivers/gpio/gpio-langwell.c >> index 202a992..8220c04 100644 >> --- a/drivers/gpio/gpio-langwell.c >> +++ b/drivers/gpio/gpio-langwell.c >> @@ -71,10 +71,12 @@ struct lnw_gpio { >> struct irq_domain *domain; >> }; >> >> +#define to_lnw_priv(chip) container_of(chip, struct lnw_gpio, chip) >> + >> static void __iomem *gpio_reg(struct gpio_chip *chip, unsigned offset, >> enum GPIO_REG reg_type) >> { >> - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); >> + struct lnw_gpio *lnw = to_lnw_priv(chip); >> unsigned nreg = chip->ngpio / 32; >> u8 reg = offset / 32; >> void __iomem *ptr; >> @@ -86,7 +88,7 @@ static void __iomem *gpio_reg(struct gpio_chip *chip, unsigned offset, >> static void __iomem *gpio_reg_2bit(struct gpio_chip *chip, unsigned offset, >> enum GPIO_REG reg_type) >> { >> - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); >> + struct lnw_gpio *lnw = to_lnw_priv(chip); >> unsigned nreg = chip->ngpio / 32; >> u8 reg = offset / 16; >> void __iomem *ptr; >> @@ -130,7 +132,7 @@ static void lnw_gpio_set(struct gpio_chip *chip, unsigned offset, int value) >> >> static int lnw_gpio_direction_input(struct gpio_chip *chip, unsigned offset) >> { >> - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); >> + struct lnw_gpio *lnw = to_lnw_priv(chip); >> void __iomem *gpdr = gpio_reg(chip, offset, GPDR); >> u32 value; >> unsigned long flags; >> @@ -153,7 +155,7 @@ static int lnw_gpio_direction_input(struct gpio_chip *chip, unsigned offset) >> static int lnw_gpio_direction_output(struct gpio_chip *chip, >> unsigned offset, int value) >> { >> - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); >> + struct lnw_gpio *lnw = to_lnw_priv(chip); >> void __iomem *gpdr = gpio_reg(chip, offset, GPDR); >> unsigned long flags; >> >> @@ -176,7 +178,7 @@ static int lnw_gpio_direction_output(struct gpio_chip *chip, >> >> static int lnw_gpio_to_irq(struct gpio_chip *chip, unsigned offset) >> { >> - struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip); >> + struct lnw_gpio *lnw = to_lnw_priv(chip); >> return irq_create_mapping(lnw->domain, offset); >> } > Nice cleanup above. > >> >> @@ -215,19 +217,14 @@ static int lnw_irq_type(struct irq_data *d, unsigned type) >> return 0; >> } >> >> -static void lnw_irq_unmask(struct irq_data *d) >> -{ >> -} >> - >> -static void lnw_irq_mask(struct irq_data *d) >> -{ >> -} >> +static void lnw_irq_noop(struct irq_data *d) {} > Umm, this looks entirely wrong. There needs to be either mask/unmask or > enable/disable ops for irq_chips. Yes I see that this patch is just > consolidating two empty functions, but they are two empty functions that > appear to be completely wrong. I see your point. The solution does not belong to a clean up patch, so I'll just remove it from here. > >> >> static struct irq_chip lnw_irqchip = { >> .name = "LNW-GPIO", >> - .irq_mask = lnw_irq_mask, >> - .irq_unmask = lnw_irq_unmask, >> + .irq_mask = lnw_irq_noop, >> + .irq_unmask = lnw_irq_noop, >> .irq_set_type = lnw_irq_type, >> + .irq_ack = lnw_irq_noop, >> }; >> >> static DEFINE_PCI_DEVICE_TABLE(lnw_gpio_ids) = { /* pin number */ >> @@ -299,17 +296,6 @@ static const struct irq_domain_ops lnw_gpio_irq_ops = { >> .xlate = irq_domain_xlate_twocell, >> }; >> >> -#ifdef CONFIG_PM >> -static int lnw_gpio_runtime_resume(struct device *dev) >> -{ >> - return 0; >> -} >> - >> -static int lnw_gpio_runtime_suspend(struct device *dev) >> -{ >> - return 0; >> -} >> - >> static int lnw_gpio_runtime_idle(struct device *dev) >> { >> int err = pm_schedule_suspend(dev, 500); >> @@ -320,16 +306,8 @@ static int lnw_gpio_runtime_idle(struct device *dev) >> return -EBUSY; >> } >> >> -#else >> -#define lnw_gpio_runtime_suspend NULL >> -#define lnw_gpio_runtime_resume NULL >> -#define lnw_gpio_runtime_idle NULL >> -#endif >> - >> static const struct dev_pm_ops lnw_gpio_pm_ops = { >> - .runtime_suspend = lnw_gpio_runtime_suspend, >> - .runtime_resume = lnw_gpio_runtime_resume, >> - .runtime_idle = lnw_gpio_runtime_idle, >> + SET_RUNTIME_PM_OPS(NULL, NULL, lnw_gpio_runtime_idle) >> }; > Also good. > >> >> static int __devinit lnw_gpio_probe(struct pci_dev *pdev, >> @@ -349,7 +327,7 @@ static int __devinit lnw_gpio_probe(struct pci_dev *pdev, >> retval = pci_request_regions(pdev, "langwell_gpio"); >> if (retval) { >> dev_err(&pdev->dev, "error requesting resources\n"); >> - goto err2; >> + goto err; > Renaming the labels just increases the noise in the diff. I prefer to > see labels in the form "err-what-i-just-tried-to-do:" so they don't need > to get reshuffled every time the code logic changes. > > There is no good reason for this change. Please drop it. Maybe instead of drop it I'd prefer to fix the labels. They are wrong currently. Br, David > > g. > . > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] gpio-langwell: cleanup driver 2012-12-20 21:35 ` David Cohen @ 2012-12-20 21:56 ` Grant Likely 0 siblings, 0 replies; 10+ messages in thread From: Grant Likely @ 2012-12-20 21:56 UTC (permalink / raw) To: David Cohen; +Cc: Linux Kernel Mailing List, Alan Cox On Thu, Dec 20, 2012 at 9:35 PM, David Cohen <david.a.cohen@intel.com> wrote: > On 12/19/2012 05:18 PM, Grant Likely wrote: >> On Tue, 18 Dec 2012 17:52:11 -0800, David Cohen <david.a.cohen@intel.com> >>> @@ -215,19 +217,14 @@ static int lnw_irq_type(struct irq_data *d, >>> unsigned type) >>> return 0; >>> } >>> -static void lnw_irq_unmask(struct irq_data *d) >>> -{ >>> -} >>> - >>> -static void lnw_irq_mask(struct irq_data *d) >>> -{ >>> -} >>> +static void lnw_irq_noop(struct irq_data *d) {} >> >> Umm, this looks entirely wrong. There needs to be either mask/unmask or >> enable/disable ops for irq_chips. Yes I see that this patch is just >> consolidating two empty functions, but they are two empty functions that >> appear to be completely wrong. > > > I see your point. The solution does not belong to a clean up patch, > so I'll just remove it from here. Right, that is appropriate. >>> static int __devinit lnw_gpio_probe(struct pci_dev *pdev, >>> @@ -349,7 +327,7 @@ static int __devinit lnw_gpio_probe(struct pci_dev >>> *pdev, >>> retval = pci_request_regions(pdev, "langwell_gpio"); >>> if (retval) { >>> dev_err(&pdev->dev, "error requesting resources\n"); >>> - goto err2; >>> + goto err; >> >> Renaming the labels just increases the noise in the diff. I prefer to >> see labels in the form "err-what-i-just-tried-to-do:" so they don't need >> to get reshuffled every time the code logic changes. >> >> There is no good reason for this change. Please drop it. > > > Maybe instead of drop it I'd prefer to fix the labels. They > are wrong currently. If there is real breakage here, then split it off into a separate bug fix patch. Otherwise I really wouldn't bother. g. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] gpio-langwell: update pci device table 2012-12-19 1:52 [PATCH 0/3] gpio-langwell updates David Cohen 2012-12-19 1:52 ` [PATCH 1/3] gpio-langwell: cleanup driver David Cohen @ 2012-12-19 1:52 ` David Cohen 2012-12-20 1:20 ` Grant Likely 2012-12-19 1:52 ` [PATCH 3/3] gpio-langwell: implement irq shutdown interface David Cohen 2 siblings, 1 reply; 10+ messages in thread From: David Cohen @ 2012-12-19 1:52 UTC (permalink / raw) To: grant.likely; +Cc: linux-kernel, alan, David Cohen This patch adds Cloverview ids to pci device table. Signed-off-by: David Cohen <david.a.cohen@intel.com> --- drivers/gpio/gpio-langwell.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpio/gpio-langwell.c b/drivers/gpio/gpio-langwell.c index 8220c04..dc534a9 100644 --- a/drivers/gpio/gpio-langwell.c +++ b/drivers/gpio/gpio-langwell.c @@ -231,6 +231,8 @@ static DEFINE_PCI_DEVICE_TABLE(lnw_gpio_ids) = { /* pin number */ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x080f), .driver_data = 64 }, { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x081f), .driver_data = 96 }, { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x081a), .driver_data = 96 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x08eb), .driver_data = 96 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x08f7), .driver_data = 96 }, { 0, } }; MODULE_DEVICE_TABLE(pci, lnw_gpio_ids); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] gpio-langwell: update pci device table 2012-12-19 1:52 ` [PATCH 2/3] gpio-langwell: update pci device table David Cohen @ 2012-12-20 1:20 ` Grant Likely 0 siblings, 0 replies; 10+ messages in thread From: Grant Likely @ 2012-12-20 1:20 UTC (permalink / raw) To: David Cohen; +Cc: linux-kernel, alan, David Cohen On Tue, 18 Dec 2012 17:52:12 -0800, David Cohen <david.a.cohen@intel.com> wrote: > This patch adds Cloverview ids to pci device table. > > Signed-off-by: David Cohen <david.a.cohen@intel.com> Applied, thanks. g. > --- > drivers/gpio/gpio-langwell.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpio/gpio-langwell.c b/drivers/gpio/gpio-langwell.c > index 8220c04..dc534a9 100644 > --- a/drivers/gpio/gpio-langwell.c > +++ b/drivers/gpio/gpio-langwell.c > @@ -231,6 +231,8 @@ static DEFINE_PCI_DEVICE_TABLE(lnw_gpio_ids) = { /* pin number */ > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x080f), .driver_data = 64 }, > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x081f), .driver_data = 96 }, > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x081a), .driver_data = 96 }, > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x08eb), .driver_data = 96 }, > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x08f7), .driver_data = 96 }, > { 0, } > }; > MODULE_DEVICE_TABLE(pci, lnw_gpio_ids); > -- > 1.7.10.4 > -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] gpio-langwell: implement irq shutdown interface 2012-12-19 1:52 [PATCH 0/3] gpio-langwell updates David Cohen 2012-12-19 1:52 ` [PATCH 1/3] gpio-langwell: cleanup driver David Cohen 2012-12-19 1:52 ` [PATCH 2/3] gpio-langwell: update pci device table David Cohen @ 2012-12-19 1:52 ` David Cohen 2012-12-20 1:26 ` Grant Likely 2 siblings, 1 reply; 10+ messages in thread From: David Cohen @ 2012-12-19 1:52 UTC (permalink / raw) To: grant.likely; +Cc: linux-kernel, alan, Li, Ning, David Cohen From: "Li, Ning" <ning.li@intel.com> Signed-off-by: David Cohen <david.a.cohen@intel.com> Signed-off-by: Li, Ning <ning.li@intel.com> --- drivers/gpio/gpio-langwell.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/gpio/gpio-langwell.c b/drivers/gpio/gpio-langwell.c index dc534a9..c702d3d 100644 --- a/drivers/gpio/gpio-langwell.c +++ b/drivers/gpio/gpio-langwell.c @@ -219,12 +219,30 @@ static int lnw_irq_type(struct irq_data *d, unsigned type) static void lnw_irq_noop(struct irq_data *d) {} +static void lnw_irq_shutdown(struct irq_data *d) +{ + struct lnw_gpio *lnw = irq_data_get_irq_chip_data(d); + u32 gpio = irqd_to_hwirq(d); + unsigned long flags; + u32 value; + void __iomem *grer = gpio_reg(&lnw->chip, gpio, GRER); + void __iomem *gfer = gpio_reg(&lnw->chip, gpio, GFER); + + spin_lock_irqsave(&lnw->lock, flags); + value = readl(grer) & ~BIT(gpio % 32); + writel(value, grer); + value = readl(gfer) & ~BIT(gpio % 32); + writel(value, gfer); + spin_unlock_irqrestore(&lnw->lock, flags); +}; + static struct irq_chip lnw_irqchip = { .name = "LNW-GPIO", .irq_mask = lnw_irq_noop, .irq_unmask = lnw_irq_noop, .irq_set_type = lnw_irq_type, .irq_ack = lnw_irq_noop, + .irq_shutdown = lnw_irq_shutdown, }; static DEFINE_PCI_DEVICE_TABLE(lnw_gpio_ids) = { /* pin number */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] gpio-langwell: implement irq shutdown interface 2012-12-19 1:52 ` [PATCH 3/3] gpio-langwell: implement irq shutdown interface David Cohen @ 2012-12-20 1:26 ` Grant Likely 2012-12-20 21:37 ` David Cohen 0 siblings, 1 reply; 10+ messages in thread From: Grant Likely @ 2012-12-20 1:26 UTC (permalink / raw) To: David Cohen; +Cc: linux-kernel, alan, Li, Ning, David Cohen On Tue, 18 Dec 2012 17:52:13 -0800, David Cohen <david.a.cohen@intel.com> wrote: > From: "Li, Ning" <ning.li@intel.com> > > Signed-off-by: David Cohen <david.a.cohen@intel.com> > Signed-off-by: Li, Ning <ning.li@intel.com> I could use some help interpreting this patch since it doesn't have any commit text describing why this patch is needed, nor does it have any comments in the code describing what it is doing. Having good commit text is important. Use it to tell me and future readers why this patch is important. What it does and how it was tested. Otherwise I end up trying to read your mind. (The patch does actually look fine to me, but it isn't okay to neglect a patch descirption on anything other than the most trivial of patches... and even then it triggers severe eyebrow raising) g. > --- > drivers/gpio/gpio-langwell.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/gpio/gpio-langwell.c b/drivers/gpio/gpio-langwell.c > index dc534a9..c702d3d 100644 > --- a/drivers/gpio/gpio-langwell.c > +++ b/drivers/gpio/gpio-langwell.c > @@ -219,12 +219,30 @@ static int lnw_irq_type(struct irq_data *d, unsigned type) > > static void lnw_irq_noop(struct irq_data *d) {} > > +static void lnw_irq_shutdown(struct irq_data *d) > +{ > + struct lnw_gpio *lnw = irq_data_get_irq_chip_data(d); > + u32 gpio = irqd_to_hwirq(d); > + unsigned long flags; > + u32 value; > + void __iomem *grer = gpio_reg(&lnw->chip, gpio, GRER); > + void __iomem *gfer = gpio_reg(&lnw->chip, gpio, GFER); > + > + spin_lock_irqsave(&lnw->lock, flags); > + value = readl(grer) & ~BIT(gpio % 32); > + writel(value, grer); > + value = readl(gfer) & ~BIT(gpio % 32); > + writel(value, gfer); > + spin_unlock_irqrestore(&lnw->lock, flags); > +}; > + > static struct irq_chip lnw_irqchip = { > .name = "LNW-GPIO", > .irq_mask = lnw_irq_noop, > .irq_unmask = lnw_irq_noop, > .irq_set_type = lnw_irq_type, > .irq_ack = lnw_irq_noop, > + .irq_shutdown = lnw_irq_shutdown, > }; > > static DEFINE_PCI_DEVICE_TABLE(lnw_gpio_ids) = { /* pin number */ > -- > 1.7.10.4 > -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] gpio-langwell: implement irq shutdown interface 2012-12-20 1:26 ` Grant Likely @ 2012-12-20 21:37 ` David Cohen 0 siblings, 0 replies; 10+ messages in thread From: David Cohen @ 2012-12-20 21:37 UTC (permalink / raw) To: Grant Likely; +Cc: linux-kernel, alan, Li, Ning On 12/19/2012 05:26 PM, Grant Likely wrote: > On Tue, 18 Dec 2012 17:52:13 -0800, David Cohen <david.a.cohen@intel.com> wrote: >> From: "Li, Ning" <ning.li@intel.com> >> >> Signed-off-by: David Cohen <david.a.cohen@intel.com> >> Signed-off-by: Li, Ning <ning.li@intel.com> > I could use some help interpreting this patch since it doesn't have any > commit text describing why this patch is needed, nor does it have any > comments in the code describing what it is doing. > > Having good commit text is important. Use it to tell me and future > readers why this patch is important. What it does and how it was tested. > Otherwise I end up trying to read your mind. > > (The patch does actually look fine to me, but it isn't okay to neglect > a patch descirption on anything other than the most trivial of > patches... and even then it triggers severe eyebrow raising) Thanks for your reply. I'll add the description and resend it. Br, David > > g. > >> --- >> drivers/gpio/gpio-langwell.c | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/drivers/gpio/gpio-langwell.c b/drivers/gpio/gpio-langwell.c >> index dc534a9..c702d3d 100644 >> --- a/drivers/gpio/gpio-langwell.c >> +++ b/drivers/gpio/gpio-langwell.c >> @@ -219,12 +219,30 @@ static int lnw_irq_type(struct irq_data *d, unsigned type) >> >> static void lnw_irq_noop(struct irq_data *d) {} >> >> +static void lnw_irq_shutdown(struct irq_data *d) >> +{ >> + struct lnw_gpio *lnw = irq_data_get_irq_chip_data(d); >> + u32 gpio = irqd_to_hwirq(d); >> + unsigned long flags; >> + u32 value; >> + void __iomem *grer = gpio_reg(&lnw->chip, gpio, GRER); >> + void __iomem *gfer = gpio_reg(&lnw->chip, gpio, GFER); >> + >> + spin_lock_irqsave(&lnw->lock, flags); >> + value = readl(grer) & ~BIT(gpio % 32); >> + writel(value, grer); >> + value = readl(gfer) & ~BIT(gpio % 32); >> + writel(value, gfer); >> + spin_unlock_irqrestore(&lnw->lock, flags); >> +}; >> + >> static struct irq_chip lnw_irqchip = { >> .name = "LNW-GPIO", >> .irq_mask = lnw_irq_noop, >> .irq_unmask = lnw_irq_noop, >> .irq_set_type = lnw_irq_type, >> .irq_ack = lnw_irq_noop, >> + .irq_shutdown = lnw_irq_shutdown, >> }; >> >> static DEFINE_PCI_DEVICE_TABLE(lnw_gpio_ids) = { /* pin number */ >> -- >> 1.7.10.4 >> ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-12-20 21:56 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-12-19 1:52 [PATCH 0/3] gpio-langwell updates David Cohen 2012-12-19 1:52 ` [PATCH 1/3] gpio-langwell: cleanup driver David Cohen 2012-12-20 1:18 ` Grant Likely 2012-12-20 21:35 ` David Cohen 2012-12-20 21:56 ` Grant Likely 2012-12-19 1:52 ` [PATCH 2/3] gpio-langwell: update pci device table David Cohen 2012-12-20 1:20 ` Grant Likely 2012-12-19 1:52 ` [PATCH 3/3] gpio-langwell: implement irq shutdown interface David Cohen 2012-12-20 1:26 ` Grant Likely 2012-12-20 21:37 ` David Cohen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).