On Tue, 18 Dec 2012 17:52:11 -0800, David Cohen <david.a.cohen@xxxxxxxxx> wrote:This patch cleans up cosmetic issues, remove useless functions and addHi David. Comments below...
to_lnw_priv() macro to replace many usages of container_of().
Change-Id: I70a8fadd20a42493271d91633739bdddff19c8d8
Signed-off-by: David Cohen <david.a.cohen@xxxxxxxxx>
---Nice cleanup above.
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)Umm, this looks entirely wrong. There needs to be either mask/unmask or
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) {}
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 = {Also good.
.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,Renaming the labels just increases the noise in the diff. I prefer to
@@ -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;
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.
.