Hello Joe & All,
On Tue, 2020-10-20 at 11:36 -0700, Joe Perches wrote:
> On Tue, 2020-10-20 at 11:48 +0000, Vaittinen, Matti wrote:
> > On Tue, 2020-10-20 at 13:07 +0300, Matti Vaittinen wrote:
> > > Thanks Tom,
> > >
> > > On Mon, 2020-10-19 at 12:33 -0700, trix@xxxxxxxxxx wrote:
> > > > From: Tom Rix <trix@xxxxxxxxxx>
> > > >
> > > > A break is not needed if it is preceded by a return
> > > >
> > > > Signed-off-by: Tom Rix <trix@xxxxxxxxxx>
> > > > ---
> > > > drivers/gpio/gpio-bd70528.c | 3 ---
> > > > 1 file changed, 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpio/gpio-bd70528.c b/drivers/gpio/gpio-
> > > > bd70528.c
> > > > index 45b3da8da336..931e5765fe92 100644
> > > > --- a/drivers/gpio/gpio-bd70528.c
> > > > +++ b/drivers/gpio/gpio-bd70528.c
> > > > @@ -71,17 +71,14 @@ static int bd70528_gpio_set_config(struct
> > > > gpio_chip *chip, unsigned int offset,
> > > > GPIO_OUT_REG(offset),
> > > > BD70528_GPIO_DRIVE_MA
> > > > SK,
> > > > BD70528_GPIO_OPEN_DRA
> > > > IN);
> > > > - break;
> > > My personal taste is also to omit these breaks but I am pretty
> > > sure I
> > > saw some tooling issuing a warning about falling through the
> > > switch-
> > > case back when I wrote this. Most probably checkpatch didn't like
> > > that
> > > back then.
> >
> > I did a test and removed the breaks. Then I copied the modified
> > file to
> > drivers/gpio/dummy.c
> > Next I committed this dummy.c in git, ran git-format-patch -s and
> > finally ran the checkpatch on this... Following was produced:
> >
> >
> > [mvaittin@localhost linux]$ scripts/checkpatch.pl 0001-gpio-add-
> > dummy.patch
> > Traceback (most recent call last):
> > File "scripts/spdxcheck.py", line 6, in <module>
> > from ply import lex, yacc
> > ImportError: No module named ply
> > WARNING: added, moved or deleted file(s), does MAINTAINERS need
> > updating?
> > #15:
> > new file mode 100644
> >
> > WARNING: Possible switch case/default not preceded by break or
> > fallthrough comment
> > #91: FILE: drivers/gpio/dummy.c:72:
> > + case PIN_CONFIG_DRIVE_PUSH_PULL:
> >
> > WARNING: Possible switch case/default not preceded by break or
> > fallthrough comment
> > #96: FILE: drivers/gpio/dummy.c:77:
> > + case PIN_CONFIG_INPUT_DEBOUNCE:
> >
> > total: 0 errors, 3 warnings, 229 lines checked
> >
> > NOTE: For some of the reported defects, checkpatch may be able to
> > mechanically convert to the typical style using --fix or --
> > fix-
> > inplace.
> >
> > 0001-gpio-add-dummy.patch has style problems, please review.
> >
> > NOTE: If any of the errors are false positives, please report
> > them to the maintainer, see CHECKPATCH in MAINTAINERS.
> >
> > I guess that explains the odd "fallthrough" comments you mentioned
> > in
> > another email. I guess the checkpatch should be fixed before you
> > put
> > too much effort in clean-up...
> >
> >
> > And for peeps who have not been following - following function
> > triggers
> > the checkpatch error above:
>
> Huh? what version of checkpatch are you using?
> Send it to me please.
I was actually accidentally running the checkpatch from stable release
5.4 yesterday. I did today run it from my development repo which
currently was based on regulator-v5.8. I didn't test the latest
versions.
Please find my version of checkpatch and the patch to trigger the
warning attached.
>
> > static int bd70528_gpio_set_config(struct gpio_chip *chip, unsigned
> > int
> > offset,
> > unsigned long config)
> > {
> > struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
> >
> > switch (pinconf_to_config_param(config)) {
> > case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> > return regmap_update_bits(bdgpio->chip.regmap,
> > GPIO_OUT_REG(offset),
> > BD70528_GPIO_DRIVE_MASK,
> > BD70528_GPIO_OPEN_DRAIN);
> > case PIN_CONFIG_DRIVE_PUSH_PULL:
> > return regmap_update_bits(bdgpio->chip.regmap,
> > GPIO_OUT_REG(offset),
> > BD70528_GPIO_DRIVE_MASK,
> > BD70528_GPIO_PUSH_PULL);
> > case PIN_CONFIG_INPUT_DEBOUNCE:
> > return bd70528_set_debounce(bdgpio, offset,
> > pinconf_to_config_argument(
> > config));
> > default:
> > break;
> > }
> > return -ENOTSUPP;
> > }
> >
> >
> > Best Regards
> > Matti Vaittinen
> >
From 5e70eba5d3b0e8e0505378400c417fb4060463c4 Mon Sep 17 00:00:00 2001
From: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx>
Date: Tue, 20 Oct 2020 14:35:16 +0300
Subject: [PATCH] gpio: add dummy
this description is likely to not make any sense.
Signed-off-by: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx>
---
drivers/gpio/dummy.c | 229 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 229 insertions(+)
create mode 100644 drivers/gpio/dummy.c
diff --git a/drivers/gpio/dummy.c b/drivers/gpio/dummy.c
new file mode 100644
index 000000000000..75c1bae94514
--- /dev/null
+++ b/drivers/gpio/dummy.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 ROHM Semiconductors
+// gpio-bd70528.c ROHM BD70528MWV gpio driver
+
+#include <linux/gpio/driver.h>
+#include <linux/mfd/rohm-bd70528.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define GPIO_IN_REG(offset) (BD70528_REG_GPIO1_IN + (offset) * 2)
+#define GPIO_OUT_REG(offset) (BD70528_REG_GPIO1_OUT + (offset) * 2)
+
+struct bd70528_gpio {
+ struct rohm_regmap_dev chip;
+ struct gpio_chip gpio;
+};
+
+static int bd70528_set_debounce(struct bd70528_gpio *bdgpio,
+ unsigned int offset, unsigned int debounce)
+{
+ u8 val;
+
+ switch (debounce) {
+ case 0:
+ val = BD70528_DEBOUNCE_DISABLE;
+ break;
+ case 1 ... 15000:
+ val = BD70528_DEBOUNCE_15MS;
+ break;
+ case 15001 ... 30000:
+ val = BD70528_DEBOUNCE_30MS;
+ break;
+ case 30001 ... 50000:
+ val = BD70528_DEBOUNCE_50MS;
+ break;
+ default:
+ dev_err(bdgpio->chip.dev,
+ "Invalid debounce value %u\n", debounce);
+ return -EINVAL;
+ }
+ return regmap_update_bits(bdgpio->chip.regmap, GPIO_IN_REG(offset),
+ BD70528_DEBOUNCE_MASK, val);
+}
+
+static int bd70528_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+ struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+ int val, ret;
+
+ /* Do we need to do something to IRQs here? */
+ ret = regmap_read(bdgpio->chip.regmap, GPIO_OUT_REG(offset), &val);
+ if (ret) {
+ dev_err(bdgpio->chip.dev, "Could not read gpio direction\n");
+ return ret;
+ }
+
+ return !(val & BD70528_GPIO_OUT_EN_MASK);
+}
+
+static int bd70528_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
+ unsigned long config)
+{
+ struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+
+ switch (pinconf_to_config_param(config)) {
+ case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+ return regmap_update_bits(bdgpio->chip.regmap,
+ GPIO_OUT_REG(offset),
+ BD70528_GPIO_DRIVE_MASK,
+ BD70528_GPIO_OPEN_DRAIN);
+ case PIN_CONFIG_DRIVE_PUSH_PULL:
+ return regmap_update_bits(bdgpio->chip.regmap,
+ GPIO_OUT_REG(offset),
+ BD70528_GPIO_DRIVE_MASK,
+ BD70528_GPIO_PUSH_PULL);
+ case PIN_CONFIG_INPUT_DEBOUNCE:
+ return bd70528_set_debounce(bdgpio, offset,
+ pinconf_to_config_argument(config));
+ default:
+ break;
+ }
+ return -ENOTSUPP;
+}
+
+static int bd70528_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+ struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+
+ /* Do we need to do something to IRQs here? */
+ return regmap_update_bits(bdgpio->chip.regmap, GPIO_OUT_REG(offset),
+ BD70528_GPIO_OUT_EN_MASK,
+ BD70528_GPIO_OUT_DISABLE);
+}
+
+static void bd70528_gpio_set(struct gpio_chip *chip, unsigned int offset,
+ int value)
+{
+ int ret;
+ struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+ u8 val = (value) ? BD70528_GPIO_OUT_HI : BD70528_GPIO_OUT_LO;
+
+ ret = regmap_update_bits(bdgpio->chip.regmap, GPIO_OUT_REG(offset),
+ BD70528_GPIO_OUT_MASK, val);
+ if (ret)
+ dev_err(bdgpio->chip.dev, "Could not set gpio to %d\n", value);
+}
+
+static int bd70528_direction_output(struct gpio_chip *chip, unsigned int offset,
+ int value)
+{
+ struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+
+ bd70528_gpio_set(chip, offset, value);
+ return regmap_update_bits(bdgpio->chip.regmap, GPIO_OUT_REG(offset),
+ BD70528_GPIO_OUT_EN_MASK,
+ BD70528_GPIO_OUT_ENABLE);
+}
+
+#define GPIO_IN_STATE_MASK(offset) (BD70528_GPIO_IN_STATE_BASE << (offset))
+
+static int bd70528_gpio_get_o(struct bd70528_gpio *bdgpio, unsigned int offset)
+{
+ int ret;
+ unsigned int val;
+
+ ret = regmap_read(bdgpio->chip.regmap, GPIO_OUT_REG(offset), &val);
+ if (!ret)
+ ret = !!(val & BD70528_GPIO_OUT_MASK);
+ else
+ dev_err(bdgpio->chip.dev, "GPIO (out) state read failed\n");
+
+ return ret;
+}
+
+static int bd70528_gpio_get_i(struct bd70528_gpio *bdgpio, unsigned int offset)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(bdgpio->chip.regmap, BD70528_REG_GPIO_STATE, &val);
+
+ if (!ret)
+ ret = !(val & GPIO_IN_STATE_MASK(offset));
+ else
+ dev_err(bdgpio->chip.dev, "GPIO (in) state read failed\n");
+
+ return ret;
+}
+
+static int bd70528_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+ int ret;
+ struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+
+ /*
+ * There is a race condition where someone might be changing the
+ * GPIO direction after we get it but before we read the value. But
+ * application design where GPIO direction may be changed just when
+ * we read GPIO value would be pointless as reader could not know
+ * whether the returned high/low state is caused by input or output.
+ * Or then there must be other ways to mitigate the issue. Thus
+ * locking would make no sense.
+ */
+ ret = bd70528_get_direction(chip, offset);
+ if (ret == 0)
+ ret = bd70528_gpio_get_o(bdgpio, offset);
+ else if (ret == 1)
+ ret = bd70528_gpio_get_i(bdgpio, offset);
+ else
+ dev_err(bdgpio->chip.dev, "failed to read GPIO direction\n");
+
+ return ret;
+}
+
+static int bd70528_probe(struct platform_device *pdev)
+{
+ struct bd70528_gpio *bdgpio;
+ struct rohm_regmap_dev *bd70528;
+ int ret;
+
+ bd70528 = dev_get_drvdata(pdev->dev.parent);
+ if (!bd70528) {
+ dev_err(&pdev->dev, "No MFD driver data\n");
+ return -EINVAL;
+ }
+
+ bdgpio = devm_kzalloc(&pdev->dev, sizeof(*bdgpio),
+ GFP_KERNEL);
+ if (!bdgpio)
+ return -ENOMEM;
+ bdgpio->chip.dev = &pdev->dev;
+ bdgpio->gpio.parent = pdev->dev.parent;
+ bdgpio->gpio.label = "bd70528-gpio";
+ bdgpio->gpio.owner = THIS_MODULE;
+ bdgpio->gpio.get_direction = bd70528_get_direction;
+ bdgpio->gpio.direction_input = bd70528_direction_input;
+ bdgpio->gpio.direction_output = bd70528_direction_output;
+ bdgpio->gpio.set_config = bd70528_gpio_set_config;
+ bdgpio->gpio.can_sleep = true;
+ bdgpio->gpio.get = bd70528_gpio_get;
+ bdgpio->gpio.set = bd70528_gpio_set;
+ bdgpio->gpio.ngpio = 4;
+ bdgpio->gpio.base = -1;
+#ifdef CONFIG_OF_GPIO
+ bdgpio->gpio.of_node = pdev->dev.parent->of_node;
+#endif
+ bdgpio->chip.regmap = bd70528->regmap;
+
+ ret = devm_gpiochip_add_data(&pdev->dev, &bdgpio->gpio,
+ bdgpio);
+ if (ret)
+ dev_err(&pdev->dev, "gpio_init: Failed to add bd70528-gpio\n");
+
+ return ret;
+}
+
+static struct platform_driver bd70528_gpio = {
+ .driver = {
+ .name = "bd70528-gpio"
+ },
+ .probe = bd70528_probe,
+};
+
+module_platform_driver(bd70528_gpio);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx>");
+MODULE_DESCRIPTION("BD70528 voltage regulator driver");
+MODULE_LICENSE("GPL");
--
2.21.3
Attachment:
checkpatch.pl
Description: checkpatch.pl
