* [PATCH 1/3] dt-bindings: pinctrl: rt2880: add binding document
2020-12-07 19:21 [PATCH 0/3] pinctrl: ralink: pinctrl driver for the rt2880 family Sergio Paracuellos
@ 2020-12-07 19:21 ` Sergio Paracuellos
2020-12-07 22:42 ` Linus Walleij
2020-12-07 19:21 ` [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family Sergio Paracuellos
2020-12-07 19:21 ` [PATCH 3/3] staging: mt7621-pinctrl: remove driver from staging Sergio Paracuellos
2 siblings, 1 reply; 10+ messages in thread
From: Sergio Paracuellos @ 2020-12-07 19:21 UTC (permalink / raw)
To: linus.walleij; +Cc: devel, devicetree, yanaijie, gregkh, linux-gpio, robh+dt
The commit adds rt2880 compatible node in binding document.
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
.../pinctrl/ralink,rt2880-pinmux.yaml | 82 +++++++++++++++++++
1 file changed, 82 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
diff --git a/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
new file mode 100644
index 000000000000..d946219a115c
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/ralink,rt2880-pinmux.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ralink rt2880 pinmux controller
+
+maintainers:
+ - Sergio Paracuellos <sergio.paracuellos@gmail.com>
+
+description:
+ The rt2880 pinmux can only set the muxing of pin groups. muxing indiviual pins
+ is not supported. There is no pinconf support.
+
+properties:
+ compatible:
+ enum:
+ - ralink,rt2880-pinmux
+
+ pinctrl-0:
+ description:
+ A phandle to the node containing the subnodes containing default
+ configurations.
+
+ pinctrl-names:
+ description:
+ A pinctrl state named "default" must be defined.
+ const: default
+
+required:
+ - compatible
+ - pinctrl-names
+ - pinctrl-0
+
+patternProperties:
+ '^.*$':
+ if:
+ type: object
+ description: |
+ A pinctrl node should contain at least one subnodes representing the
+ pinctrl groups available on the machine.
+ $ref: "pinmux-node.yaml"
+ required:
+ - groups
+ - function
+ additionalProperties: false
+ then:
+ properties:
+ groups:
+ description:
+ Name of the pin group to use for the functions.
+ $ref: "/schemas/types.yaml#/definitions/string"
+ enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
+ pcie, sdhci]
+ function:
+ description:
+ The mux function to select
+ $ref: "/schemas/types.yaml#/definitions/string"
+ enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
+ mdio, nand1, nand2, sdhci]
+
+additionalProperties: false
+
+examples:
+ # Pinmux controller node
+ - |
+ pinctrl {
+ compatible = "ralink,rt2880-pinmux";
+ pinctrl-names = "default";
+ pinctrl-0 = <&state_default>;
+
+ state_default: pinctrl0 {
+ };
+
+ i2c_pins: i2c0 {
+ i2c0 {
+ groups = "i2c";
+ function = "i2c";
+ };
+ };
+ };
--
2.25.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] dt-bindings: pinctrl: rt2880: add binding document
2020-12-07 19:21 ` [PATCH 1/3] dt-bindings: pinctrl: rt2880: add binding document Sergio Paracuellos
@ 2020-12-07 22:42 ` Linus Walleij
2020-12-08 6:46 ` Sergio Paracuellos
0 siblings, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2020-12-07 22:42 UTC (permalink / raw)
To: Sergio Paracuellos
Cc: open list:STAGING SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jason Yan, Greg KH, open list:GPIO SUBSYSTEM, Rob Herring
Hi Sergio,
thanks for driving this!
On Mon, Dec 7, 2020 at 8:21 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
> The commit adds rt2880 compatible node in binding document.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
(...)
> +description:
> + The rt2880 pinmux can only set the muxing of pin groups. muxing indiviual pins
> + is not supported. There is no pinconf support.
OK!
> +properties:
> + compatible:
> + enum:
> + - ralink,rt2880-pinmux
> +
> + pinctrl-0:
> + description:
> + A phandle to the node containing the subnodes containing default
> + configurations.
As it is a node on the pin controller itself, this is a hog so write something
about that this is for pinctrl hogs.
> + pinctrl-names:
> + description:
> + A pinctrl state named "default" must be defined.
> + const: default
Is it really compulsory?
> +required:
> + - compatible
> + - pinctrl-names
> + - pinctrl-0
I wonder if the hogs are really compulsory.
> +patternProperties:
> + '^.*$':
That's liberal node naming!
What about [a-z0-9_-]+ or something?
> + if:
> + type: object
> + description: |
> + A pinctrl node should contain at least one subnodes representing the
> + pinctrl groups available on the machine.
> + $ref: "pinmux-node.yaml"
> + required:
> + - groups
> + - function
> + additionalProperties: false
> + then:
> + properties:
> + groups:
> + description:
> + Name of the pin group to use for the functions.
> + $ref: "/schemas/types.yaml#/definitions/string"
> + enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
> + pcie, sdhci]
> + function:
> + description:
> + The mux function to select
> + $ref: "/schemas/types.yaml#/definitions/string"
> + enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
> + mdio, nand1, nand2, sdhci]
Why do we have this complex if: clause?
$ref: "pinmux-node.yaml" should bring in the groups and
function properties. Then you can add some further restrictions
on top of that, right?
I would just do:
patternProperties:
'^[a-z0-9_]+$':
type: object
description: node for pinctrl
$ref "pinmux-node.yaml"
properties:
groups:
description: groups...
enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
pcie, sdhci]
function:
description: function...
enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
mdio, nand1, nand2, sdhci]
Note: the function names are fine but the group names are a bit
confusion since often a group can be used for more than one
function, and e.g.
function = "i2c";
group = "uart1";
to use the uart1 pins for an i2c is gonna look mildly confusing.
But if this is what the hardware calls it I suppose it is
fine.
Yours,
Linus Walleij
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] dt-bindings: pinctrl: rt2880: add binding document
2020-12-07 22:42 ` Linus Walleij
@ 2020-12-08 6:46 ` Sergio Paracuellos
0 siblings, 0 replies; 10+ messages in thread
From: Sergio Paracuellos @ 2020-12-08 6:46 UTC (permalink / raw)
To: Linus Walleij
Cc: open list:STAGING SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jason Yan, Greg KH, open list:GPIO SUBSYSTEM, Rob Herring
Hi Linus,
Thanks for the review. There weren't too many yaml samples for this so
as you had seen this was a bit messy and I really needed this review,
especially in the 'if' clause part :).
On Mon, Dec 7, 2020 at 11:42 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Hi Sergio,
>
> thanks for driving this!
>
> On Mon, Dec 7, 2020 at 8:21 PM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
>
> > The commit adds rt2880 compatible node in binding document.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> (...)
> > +description:
> > + The rt2880 pinmux can only set the muxing of pin groups. muxing indiviual pins
> > + is not supported. There is no pinconf support.
>
> OK!
>
> > +properties:
> > + compatible:
> > + enum:
> > + - ralink,rt2880-pinmux
> > +
> > + pinctrl-0:
> > + description:
> > + A phandle to the node containing the subnodes containing default
> > + configurations.
>
> As it is a node on the pin controller itself, this is a hog so write something
> about that this is for pinctrl hogs.
Ok, will do.
>
> > + pinctrl-names:
> > + description:
> > + A pinctrl state named "default" must be defined.
> > + const: default
>
> Is it really compulsory?
Not really, I guess. The current device tree contains one so I added
here because of this.
>
> > +required:
> > + - compatible
> > + - pinctrl-names
> > + - pinctrl-0
>
> I wonder if the hogs are really compulsory.
Ok, so I guess I should remove both 'pinctrl-names' and ' pinctrl-0'
from the required but maintain its desciption.
>
> > +patternProperties:
> > + '^.*$':
>
> That's liberal node naming!
> What about [a-z0-9_-]+ or something?
hahaha. Yeah, I like freedom :), but yes, you are right, I will change
the pattern using the one proposed here.
>
> > + if:
> > + type: object
> > + description: |
> > + A pinctrl node should contain at least one subnodes representing the
> > + pinctrl groups available on the machine.
> > + $ref: "pinmux-node.yaml"
> > + required:
> > + - groups
> > + - function
> > + additionalProperties: false
> > + then:
> > + properties:
> > + groups:
> > + description:
> > + Name of the pin group to use for the functions.
> > + $ref: "/schemas/types.yaml#/definitions/string"
> > + enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
> > + pcie, sdhci]
> > + function:
> > + description:
> > + The mux function to select
> > + $ref: "/schemas/types.yaml#/definitions/string"
> > + enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
> > + mdio, nand1, nand2, sdhci]
>
> Why do we have this complex if: clause?
To be honest to avoid problems with other pinctrl root nodes because
they are not type 'object' and not having real idea in what way this
should be achieved :).
> $ref: "pinmux-node.yaml" should bring in the groups and
> function properties. Then you can add some further restrictions
> on top of that, right?
>
> I would just do:
>
> patternProperties:
> '^[a-z0-9_]+$':
> type: object
> description: node for pinctrl
> $ref "pinmux-node.yaml"
>
> properties:
> groups:
> description: groups...
> enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
> pcie, sdhci]
> function:
> description: function...
> enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
> mdio, nand1, nand2, sdhci]
>
> Note: the function names are fine but the group names are a bit
> confusion since often a group can be used for more than one
> function, and e.g.
>
> function = "i2c";
> group = "uart1";
>
> to use the uart1 pins for an i2c is gonna look mildly confusing.
>
> But if this is what the hardware calls it I suppose it is
> fine.
This is the way is currently being used in the device tree.
>
> Yours,
> Linus Walleij
Thanks again. I will change this and send v2.
Best regards,
Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family
2020-12-07 19:21 [PATCH 0/3] pinctrl: ralink: pinctrl driver for the rt2880 family Sergio Paracuellos
2020-12-07 19:21 ` [PATCH 1/3] dt-bindings: pinctrl: rt2880: add binding document Sergio Paracuellos
@ 2020-12-07 19:21 ` Sergio Paracuellos
2020-12-07 23:00 ` Linus Walleij
2020-12-08 10:17 ` Dan Carpenter
2020-12-07 19:21 ` [PATCH 3/3] staging: mt7621-pinctrl: remove driver from staging Sergio Paracuellos
2 siblings, 2 replies; 10+ messages in thread
From: Sergio Paracuellos @ 2020-12-07 19:21 UTC (permalink / raw)
To: linus.walleij; +Cc: devel, devicetree, yanaijie, gregkh, linux-gpio, robh+dt
These Socs have 1-3 banks of 8-32 gpios. Rather then setting the muxing of each
pin individually, these socs have mux groups that when set will effect 1-N pins.
Pin groups have a 2, 4 or 8 different muxes.
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
drivers/pinctrl/Kconfig | 6 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-rt2880.c | 370 +++++++++++++++++++++++++++++++
3 files changed, 377 insertions(+)
create mode 100644 drivers/pinctrl/pinctrl-rt2880.c
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 815095326e2d..e97970ec1210 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -215,6 +215,12 @@ config PINCTRL_ROCKCHIP
select MFD_SYSCON
select OF_GPIO
+config PINCTRL_RT2880
+ bool "RT2880 pinctrl driver for RALINK/Mediatek SOCs"
+ depends on RALINK
+ select PINMUX
+ select GENERIC_PINCONF
+
config PINCTRL_SINGLE
tristate "One-register-per-pin type device tree based pinctrl driver"
depends on OF
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index f53933b2ff02..0ebc24dad259 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_PINCTRL_STMFX) += pinctrl-stmfx.o
obj-$(CONFIG_PINCTRL_ZYNQ) += pinctrl-zynq.o
obj-$(CONFIG_PINCTRL_INGENIC) += pinctrl-ingenic.o
obj-$(CONFIG_PINCTRL_RK805) += pinctrl-rk805.o
+obj-$(CONFIG_PINCTRL_RT2880) += pinctrl-rt2880.o
obj-$(CONFIG_PINCTRL_OCELOT) += pinctrl-ocelot.o
obj-$(CONFIG_PINCTRL_EQUILIBRIUM) += pinctrl-equilibrium.o
diff --git a/drivers/pinctrl/pinctrl-rt2880.c b/drivers/pinctrl/pinctrl-rt2880.c
new file mode 100644
index 000000000000..e61dbe186bc9
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-rt2880.c
@@ -0,0 +1,370 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2013 John Crispin <blogic@openwrt.org>
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/machine.h>
+
+#include <asm/mach-ralink/ralink_regs.h>
+#include <asm/mach-ralink/pinmux.h>
+#include <asm/mach-ralink/mt7620.h>
+
+#include "core.h"
+#include "pinctrl-utils.h"
+
+#define SYSC_REG_GPIO_MODE 0x60
+#define SYSC_REG_GPIO_MODE2 0x64
+
+struct rt2880_priv {
+ struct device *dev;
+
+ struct pinctrl_pin_desc *pads;
+ struct pinctrl_desc *desc;
+
+ struct rt2880_pmx_func **func;
+ int func_count;
+
+ struct rt2880_pmx_group *groups;
+ const char **group_names;
+ int group_count;
+
+ u8 *gpio;
+ int max_pins;
+};
+
+static int rt2880_get_group_count(struct pinctrl_dev *pctrldev)
+{
+ struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+ return p->group_count;
+}
+
+static const char *rt2880_get_group_name(struct pinctrl_dev *pctrldev,
+ unsigned int group)
+{
+ struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+ return (group >= p->group_count) ? NULL : p->group_names[group];
+}
+
+static int rt2880_get_group_pins(struct pinctrl_dev *pctrldev,
+ unsigned int group,
+ const unsigned int **pins,
+ unsigned int *num_pins)
+{
+ struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+ if (group >= p->group_count)
+ return -EINVAL;
+
+ *pins = p->groups[group].func[0].pins;
+ *num_pins = p->groups[group].func[0].pin_count;
+
+ return 0;
+}
+
+static const struct pinctrl_ops rt2880_pctrl_ops = {
+ .get_groups_count = rt2880_get_group_count,
+ .get_group_name = rt2880_get_group_name,
+ .get_group_pins = rt2880_get_group_pins,
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_all,
+ .dt_free_map = pinconf_generic_dt_free_map,
+};
+
+static int rt2880_pmx_func_count(struct pinctrl_dev *pctrldev)
+{
+ struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+ return p->func_count;
+}
+
+static const char *rt2880_pmx_func_name(struct pinctrl_dev *pctrldev,
+ unsigned int func)
+{
+ struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+ return p->func[func]->name;
+}
+
+static int rt2880_pmx_group_get_groups(struct pinctrl_dev *pctrldev,
+ unsigned int func,
+ const char * const **groups,
+ unsigned int * const num_groups)
+{
+ struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+ if (p->func[func]->group_count == 1)
+ *groups = &p->group_names[p->func[func]->groups[0]];
+ else
+ *groups = p->group_names;
+
+ *num_groups = p->func[func]->group_count;
+
+ return 0;
+}
+
+static int rt2880_pmx_group_enable(struct pinctrl_dev *pctrldev,
+ unsigned int func, unsigned int group)
+{
+ struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+ u32 mode = 0;
+ u32 reg = SYSC_REG_GPIO_MODE;
+ int i;
+ int shift;
+
+ /* dont allow double use */
+ if (p->groups[group].enabled) {
+ dev_err(p->dev, "%s is already enabled\n",
+ p->groups[group].name);
+ return -EBUSY;
+ }
+
+ p->groups[group].enabled = 1;
+ p->func[func]->enabled = 1;
+
+ shift = p->groups[group].shift;
+ if (shift >= 32) {
+ shift -= 32;
+ reg = SYSC_REG_GPIO_MODE2;
+ }
+ mode = rt_sysc_r32(reg);
+ mode &= ~(p->groups[group].mask << shift);
+
+ /* mark the pins as gpio */
+ for (i = 0; i < p->groups[group].func[0].pin_count; i++)
+ p->gpio[p->groups[group].func[0].pins[i]] = 1;
+
+ /* function 0 is gpio and needs special handling */
+ if (func == 0) {
+ mode |= p->groups[group].gpio << shift;
+ } else {
+ for (i = 0; i < p->func[func]->pin_count; i++)
+ p->gpio[p->func[func]->pins[i]] = 0;
+ mode |= p->func[func]->value << shift;
+ }
+ rt_sysc_w32(mode, reg);
+
+ return 0;
+}
+
+static int rt2880_pmx_group_gpio_request_enable(struct pinctrl_dev *pctrldev,
+ struct pinctrl_gpio_range *range,
+ unsigned int pin)
+{
+ struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+ if (!p->gpio[pin]) {
+ dev_err(p->dev, "pin %d is not set to gpio mux\n", pin);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static const struct pinmux_ops rt2880_pmx_group_ops = {
+ .get_functions_count = rt2880_pmx_func_count,
+ .get_function_name = rt2880_pmx_func_name,
+ .get_function_groups = rt2880_pmx_group_get_groups,
+ .set_mux = rt2880_pmx_group_enable,
+ .gpio_request_enable = rt2880_pmx_group_gpio_request_enable,
+};
+
+static struct pinctrl_desc rt2880_pctrl_desc = {
+ .owner = THIS_MODULE,
+ .name = "rt2880-pinmux",
+ .pctlops = &rt2880_pctrl_ops,
+ .pmxops = &rt2880_pmx_group_ops,
+};
+
+static struct rt2880_pmx_func gpio_func = {
+ .name = "gpio",
+};
+
+static int rt2880_pinmux_index(struct rt2880_priv *p)
+{
+ struct rt2880_pmx_func **f;
+ struct rt2880_pmx_group *mux = p->groups;
+ int i, j, c = 0;
+
+ /* count the mux functions */
+ while (mux->name) {
+ p->group_count++;
+ mux++;
+ }
+
+ /* allocate the group names array needed by the gpio function */
+ p->group_names = devm_kcalloc(p->dev, p->group_count,
+ sizeof(char *), GFP_KERNEL);
+ if (!p->group_names)
+ return -1;
+
+ for (i = 0; i < p->group_count; i++) {
+ p->group_names[i] = p->groups[i].name;
+ p->func_count += p->groups[i].func_count;
+ }
+
+ /* we have a dummy function[0] for gpio */
+ p->func_count++;
+
+ /* allocate our function and group mapping index buffers */
+ f = p->func = devm_kcalloc(p->dev,
+ p->func_count,
+ sizeof(*p->func),
+ GFP_KERNEL);
+ gpio_func.groups = devm_kcalloc(p->dev, p->group_count, sizeof(int),
+ GFP_KERNEL);
+ if (!f || !gpio_func.groups)
+ return -1;
+
+ /* add a backpointer to the function so it knows its group */
+ gpio_func.group_count = p->group_count;
+ for (i = 0; i < gpio_func.group_count; i++)
+ gpio_func.groups[i] = i;
+
+ f[c] = &gpio_func;
+ c++;
+
+ /* add remaining functions */
+ for (i = 0; i < p->group_count; i++) {
+ for (j = 0; j < p->groups[i].func_count; j++) {
+ f[c] = &p->groups[i].func[j];
+ f[c]->groups = devm_kzalloc(p->dev, sizeof(int),
+ GFP_KERNEL);
+ f[c]->groups[0] = i;
+ f[c]->group_count = 1;
+ c++;
+ }
+ }
+ return 0;
+}
+
+static int rt2880_pinmux_pins(struct rt2880_priv *p)
+{
+ int i, j;
+
+ /*
+ * loop over the functions and initialize the pins array.
+ * also work out the highest pin used.
+ */
+ for (i = 0; i < p->func_count; i++) {
+ int pin;
+
+ if (!p->func[i]->pin_count)
+ continue;
+
+ p->func[i]->pins = devm_kcalloc(p->dev,
+ p->func[i]->pin_count,
+ sizeof(int),
+ GFP_KERNEL);
+ for (j = 0; j < p->func[i]->pin_count; j++)
+ p->func[i]->pins[j] = p->func[i]->pin_first + j;
+
+ pin = p->func[i]->pin_first + p->func[i]->pin_count;
+ if (pin > p->max_pins)
+ p->max_pins = pin;
+ }
+
+ /* the buffer that tells us which pins are gpio */
+ p->gpio = devm_kcalloc(p->dev, p->max_pins, sizeof(u8), GFP_KERNEL);
+ /* the pads needed to tell pinctrl about our pins */
+ p->pads = devm_kcalloc(p->dev, p->max_pins,
+ sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
+ if (!p->pads || !p->gpio) {
+ dev_err(p->dev, "Failed to allocate gpio data\n");
+ return -ENOMEM;
+ }
+
+ memset(p->gpio, 1, sizeof(u8) * p->max_pins);
+ for (i = 0; i < p->func_count; i++) {
+ if (!p->func[i]->pin_count)
+ continue;
+
+ for (j = 0; j < p->func[i]->pin_count; j++)
+ p->gpio[p->func[i]->pins[j]] = 0;
+ }
+
+ /* pin 0 is always a gpio */
+ p->gpio[0] = 1;
+
+ /* set the pads */
+ for (i = 0; i < p->max_pins; i++) {
+ /* strlen("ioXY") + 1 = 5 */
+ char *name = devm_kzalloc(p->dev, 5, GFP_KERNEL);
+
+ if (!name)
+ return -ENOMEM;
+ snprintf(name, 5, "io%d", i);
+ p->pads[i].number = i;
+ p->pads[i].name = name;
+ }
+ p->desc->pins = p->pads;
+ p->desc->npins = p->max_pins;
+
+ return 0;
+}
+
+static int rt2880_pinmux_probe(struct platform_device *pdev)
+{
+ struct rt2880_priv *p;
+ struct pinctrl_dev *dev;
+
+ if (!rt2880_pinmux_data)
+ return -ENOTSUPP;
+
+ /* setup the private data */
+ p = devm_kzalloc(&pdev->dev, sizeof(struct rt2880_priv), GFP_KERNEL);
+ if (!p)
+ return -ENOMEM;
+
+ p->dev = &pdev->dev;
+ p->desc = &rt2880_pctrl_desc;
+ p->groups = rt2880_pinmux_data;
+ platform_set_drvdata(pdev, p);
+
+ /* init the device */
+ if (rt2880_pinmux_index(p)) {
+ dev_err(&pdev->dev, "failed to load index\n");
+ return -EINVAL;
+ }
+ if (rt2880_pinmux_pins(p)) {
+ dev_err(&pdev->dev, "failed to load pins\n");
+ return -EINVAL;
+ }
+ dev = pinctrl_register(p->desc, &pdev->dev, p);
+ if (IS_ERR(dev))
+ return PTR_ERR(dev);
+
+ return 0;
+}
+
+static const struct of_device_id rt2880_pinmux_match[] = {
+ { .compatible = "ralink,rt2880-pinmux" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, rt2880_pinmux_match);
+
+static struct platform_driver rt2880_pinmux_driver = {
+ .probe = rt2880_pinmux_probe,
+ .driver = {
+ .name = "rt2880-pinmux",
+ .of_match_table = rt2880_pinmux_match,
+ },
+};
+
+int __init rt2880_pinmux_init(void)
+{
+ return platform_driver_register(&rt2880_pinmux_driver);
+}
+
+core_initcall_sync(rt2880_pinmux_init);
--
2.25.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family
2020-12-07 19:21 ` [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family Sergio Paracuellos
@ 2020-12-07 23:00 ` Linus Walleij
2020-12-08 6:53 ` Sergio Paracuellos
2020-12-08 10:17 ` Dan Carpenter
1 sibling, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2020-12-07 23:00 UTC (permalink / raw)
To: Sergio Paracuellos
Cc: open list:STAGING SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jason Yan, Greg KH, open list:GPIO SUBSYSTEM, Rob Herring
Hi Serigio,
I dug around some to try to understand the patch I think I get
it now :)
Squash this with the third patch so it becomes a
"move" of this file, preserving history. With that:
Acked-by: Linus Walleij <linus.walleij@linaro.org>
I have ideas, but it is better to move the driver out
of staging and improve it in pinctrl.
Since there might be many sub-SoCs for this pin
controller, what about creating
drivers/pinctrl/ralink/* for this at the same time?
On Mon, Dec 7, 2020 at 8:21 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> These Socs have 1-3 banks of 8-32 gpios. Rather then setting the muxing of each
> pin individually, these socs have mux groups that when set will effect 1-N pins.
> Pin groups have a 2, 4 or 8 different muxes.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
(...)
> +#include <asm/mach-ralink/ralink_regs.h>
> +#include <asm/mach-ralink/pinmux.h>
> +#include <asm/mach-ralink/mt7620.h>
I think in the next step we should move the contents of
rt2880_pinmux_data into this driver, then we can drop these
mach-headers and show the way for the rest of the ralink
chips to push their data down into this driver (or subdrivers)
and depopulate mach-ralink a bit.
> + p->groups = rt2880_pinmux_data;
So this is where the driver actually gets a pointer to all
groups and functions, and these groups and functions all
come from arch/mips/ralink/mt7621.c right?
I think after this first step we should move mt7621.c
to pinctrl and become a subdriver for this pin controller
and then we can hopefully move the rest as well once
you set the pattern for how we do this.
Yours,
Linus Walleij
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family
2020-12-07 23:00 ` Linus Walleij
@ 2020-12-08 6:53 ` Sergio Paracuellos
0 siblings, 0 replies; 10+ messages in thread
From: Sergio Paracuellos @ 2020-12-08 6:53 UTC (permalink / raw)
To: Linus Walleij
Cc: open list:STAGING SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jason Yan, Greg KH, open list:GPIO SUBSYSTEM, Rob Herring
Hi Linus,
On Tue, Dec 8, 2020 at 12:00 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Hi Serigio,
>
> I dug around some to try to understand the patch I think I get
> it now :)
>
> Squash this with the third patch so it becomes a
> "move" of this file, preserving history. With that:
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
Ok, will squash those two if you prefer that way with your 'Acked-by'.
>
> I have ideas, but it is better to move the driver out
> of staging and improve it in pinctrl.
>
> Since there might be many sub-SoCs for this pin
> controller, what about creating
> drivers/pinctrl/ralink/* for this at the same time?
Ok, I will put this inside a ralink subdirectory in pinctrl.
>
> On Mon, Dec 7, 2020 at 8:21 PM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
> >
> > These Socs have 1-3 banks of 8-32 gpios. Rather then setting the muxing of each
> > pin individually, these socs have mux groups that when set will effect 1-N pins.
> > Pin groups have a 2, 4 or 8 different muxes.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> (...)
> > +#include <asm/mach-ralink/ralink_regs.h>
> > +#include <asm/mach-ralink/pinmux.h>
> > +#include <asm/mach-ralink/mt7620.h>
>
> I think in the next step we should move the contents of
> rt2880_pinmux_data into this driver, then we can drop these
> mach-headers and show the way for the rest of the ralink
> chips to push their data down into this driver (or subdrivers)
> and depopulate mach-ralink a bit.
Agree. Doing that no arch dependencies are included and we can cleanly
enable the driver also for COMPILE_TEST without adding special flags
in pinctrl Makefile.
>
> > + p->groups = rt2880_pinmux_data;
>
> So this is where the driver actually gets a pointer to all
> groups and functions, and these groups and functions all
> come from arch/mips/ralink/mt7621.c right?
Yes, all of that is defined there.
>
> I think after this first step we should move mt7621.c
> to pinctrl and become a subdriver for this pin controller
> and then we can hopefully move the rest as well once
> you set the pattern for how we do this.
I see. Thanks for advices.
>
> Yours,
> Linus Walleij
Best regards,
Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family
2020-12-07 19:21 ` [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family Sergio Paracuellos
2020-12-07 23:00 ` Linus Walleij
@ 2020-12-08 10:17 ` Dan Carpenter
2020-12-08 11:48 ` Sergio Paracuellos
1 sibling, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2020-12-08 10:17 UTC (permalink / raw)
To: Sergio Paracuellos
Cc: devel, devicetree, yanaijie, gregkh, linus.walleij, linux-gpio, robh+dt
On Mon, Dec 07, 2020 at 08:21:03PM +0100, Sergio Paracuellos wrote:
> +static struct pinctrl_desc rt2880_pctrl_desc = {
> + .owner = THIS_MODULE,
> + .name = "rt2880-pinmux",
> + .pctlops = &rt2880_pctrl_ops,
> + .pmxops = &rt2880_pmx_group_ops,
> +};
> +
> +static struct rt2880_pmx_func gpio_func = {
> + .name = "gpio",
> +};
> +
> +static int rt2880_pinmux_index(struct rt2880_priv *p)
This function name is not great. I assumed that it would return the
index.
> +{
> + struct rt2880_pmx_func **f;
Get rid of this "f" variable and use "p->func" instead.
> + struct rt2880_pmx_group *mux = p->groups;
> + int i, j, c = 0;
> +
> + /* count the mux functions */
> + while (mux->name) {
> + p->group_count++;
> + mux++;
> + }
> +
> + /* allocate the group names array needed by the gpio function */
> + p->group_names = devm_kcalloc(p->dev, p->group_count,
> + sizeof(char *), GFP_KERNEL);
> + if (!p->group_names)
> + return -1;
Return proper error codes in this function. s/-1/-ENOMEM/
> +
> + for (i = 0; i < p->group_count; i++) {
> + p->group_names[i] = p->groups[i].name;
> + p->func_count += p->groups[i].func_count;
> + }
> +
> + /* we have a dummy function[0] for gpio */
> + p->func_count++;
> +
> + /* allocate our function and group mapping index buffers */
> + f = p->func = devm_kcalloc(p->dev,
> + p->func_count,
> + sizeof(*p->func),
> + GFP_KERNEL);
> + gpio_func.groups = devm_kcalloc(p->dev, p->group_count, sizeof(int),
> + GFP_KERNEL);
> + if (!f || !gpio_func.groups)
> + return -1;
> +
> + /* add a backpointer to the function so it knows its group */
> + gpio_func.group_count = p->group_count;
> + for (i = 0; i < gpio_func.group_count; i++)
> + gpio_func.groups[i] = i;
> +
> + f[c] = &gpio_func;
> + c++;
> +
> + /* add remaining functions */
> + for (i = 0; i < p->group_count; i++) {
> + for (j = 0; j < p->groups[i].func_count; j++) {
> + f[c] = &p->groups[i].func[j];
> + f[c]->groups = devm_kzalloc(p->dev, sizeof(int),
> + GFP_KERNEL);
Add a NULL check.
> + f[c]->groups[0] = i;
> + f[c]->group_count = 1;
> + c++;
> + }
> + }
> + return 0;
> +}
> +
> +static int rt2880_pinmux_pins(struct rt2880_priv *p)
> +{
> + int i, j;
> +
> + /*
> + * loop over the functions and initialize the pins array.
> + * also work out the highest pin used.
> + */
> + for (i = 0; i < p->func_count; i++) {
> + int pin;
> +
> + if (!p->func[i]->pin_count)
> + continue;
> +
> + p->func[i]->pins = devm_kcalloc(p->dev,
> + p->func[i]->pin_count,
> + sizeof(int),
> + GFP_KERNEL);
This can fit on two lines.
p->func[i]->pins = devm_kcalloc(p->dev, p->func[i]->pin_count,
sizeof(int), GFP_KERNEL);
> + for (j = 0; j < p->func[i]->pin_count; j++)
> + p->func[i]->pins[j] = p->func[i]->pin_first + j;
> +
> + pin = p->func[i]->pin_first + p->func[i]->pin_count;
> + if (pin > p->max_pins)
> + p->max_pins = pin;
> + }
> +
> + /* the buffer that tells us which pins are gpio */
> + p->gpio = devm_kcalloc(p->dev, p->max_pins, sizeof(u8), GFP_KERNEL);
> + /* the pads needed to tell pinctrl about our pins */
> + p->pads = devm_kcalloc(p->dev, p->max_pins,
> + sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
> + if (!p->pads || !p->gpio) {
> + dev_err(p->dev, "Failed to allocate gpio data\n");
Delete this error message. #checkpatch.pl
> + return -ENOMEM;
> + }
> +
> + memset(p->gpio, 1, sizeof(u8) * p->max_pins);
> + for (i = 0; i < p->func_count; i++) {
> + if (!p->func[i]->pin_count)
> + continue;
> +
> + for (j = 0; j < p->func[i]->pin_count; j++)
> + p->gpio[p->func[i]->pins[j]] = 0;
> + }
> +
> + /* pin 0 is always a gpio */
> + p->gpio[0] = 1;
> +
> + /* set the pads */
> + for (i = 0; i < p->max_pins; i++) {
> + /* strlen("ioXY") + 1 = 5 */
> + char *name = devm_kzalloc(p->dev, 5, GFP_KERNEL);
> +
char *name;
name = kasprintf(GFP_KERNEL, "io%d", i);
> + if (!name)
> + return -ENOMEM;
> + snprintf(name, 5, "io%d", i);
> + p->pads[i].number = i;
> + p->pads[i].name = name;
> + }
> + p->desc->pins = p->pads;
> + p->desc->npins = p->max_pins;
> +
> + return 0;
> +}
> +
> +static int rt2880_pinmux_probe(struct platform_device *pdev)
> +{
> + struct rt2880_priv *p;
> + struct pinctrl_dev *dev;
> +
> + if (!rt2880_pinmux_data)
> + return -ENOTSUPP;
> +
> + /* setup the private data */
> + p = devm_kzalloc(&pdev->dev, sizeof(struct rt2880_priv), GFP_KERNEL);
> + if (!p)
> + return -ENOMEM;
> +
> + p->dev = &pdev->dev;
> + p->desc = &rt2880_pctrl_desc;
> + p->groups = rt2880_pinmux_data;
> + platform_set_drvdata(pdev, p);
> +
> + /* init the device */
> + if (rt2880_pinmux_index(p)) {
> + dev_err(&pdev->dev, "failed to load index\n");
> + return -EINVAL;
Preserve the error code:
err = rt2880_pinmux_index(p);
if (err) {
dev_err(&pdev->dev, "failed to load index\n");
return err;
}
> + }
> + if (rt2880_pinmux_pins(p)) {
> + dev_err(&pdev->dev, "failed to load pins\n");
> + return -EINVAL;
Same.
> + }
> + dev = pinctrl_register(p->desc, &pdev->dev, p);
> + if (IS_ERR(dev))
> + return PTR_ERR(dev);
> +
> + return 0;
> +}
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family
2020-12-08 10:17 ` Dan Carpenter
@ 2020-12-08 11:48 ` Sergio Paracuellos
0 siblings, 0 replies; 10+ messages in thread
From: Sergio Paracuellos @ 2020-12-08 11:48 UTC (permalink / raw)
To: Dan Carpenter
Cc: open list:STAGING SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jason Yan, Greg KH, Linus Walleij, open list:GPIO SUBSYSTEM,
Rob Herring
Hi Dan,
Thanks for the review.
On Tue, Dec 8, 2020 at 11:17 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Mon, Dec 07, 2020 at 08:21:03PM +0100, Sergio Paracuellos wrote:
> > +static struct pinctrl_desc rt2880_pctrl_desc = {
> > + .owner = THIS_MODULE,
> > + .name = "rt2880-pinmux",
> > + .pctlops = &rt2880_pctrl_ops,
> > + .pmxops = &rt2880_pmx_group_ops,
> > +};
> > +
> > +static struct rt2880_pmx_func gpio_func = {
> > + .name = "gpio",
> > +};
> > +
> > +static int rt2880_pinmux_index(struct rt2880_priv *p)
>
>
> This function name is not great. I assumed that it would return the
> index.
>
> > +{
> > + struct rt2880_pmx_func **f;
>
> Get rid of this "f" variable and use "p->func" instead.
>
> > + struct rt2880_pmx_group *mux = p->groups;
> > + int i, j, c = 0;
> > +
> > + /* count the mux functions */
> > + while (mux->name) {
> > + p->group_count++;
> > + mux++;
> > + }
> > +
> > + /* allocate the group names array needed by the gpio function */
> > + p->group_names = devm_kcalloc(p->dev, p->group_count,
> > + sizeof(char *), GFP_KERNEL);
> > + if (!p->group_names)
> > + return -1;
>
> Return proper error codes in this function. s/-1/-ENOMEM/
>
> > +
> > + for (i = 0; i < p->group_count; i++) {
> > + p->group_names[i] = p->groups[i].name;
> > + p->func_count += p->groups[i].func_count;
> > + }
> > +
> > + /* we have a dummy function[0] for gpio */
> > + p->func_count++;
> > +
> > + /* allocate our function and group mapping index buffers */
> > + f = p->func = devm_kcalloc(p->dev,
> > + p->func_count,
> > + sizeof(*p->func),
> > + GFP_KERNEL);
> > + gpio_func.groups = devm_kcalloc(p->dev, p->group_count, sizeof(int),
> > + GFP_KERNEL);
> > + if (!f || !gpio_func.groups)
> > + return -1;
> > +
> > + /* add a backpointer to the function so it knows its group */
> > + gpio_func.group_count = p->group_count;
> > + for (i = 0; i < gpio_func.group_count; i++)
> > + gpio_func.groups[i] = i;
> > +
> > + f[c] = &gpio_func;
> > + c++;
> > +
> > + /* add remaining functions */
> > + for (i = 0; i < p->group_count; i++) {
> > + for (j = 0; j < p->groups[i].func_count; j++) {
> > + f[c] = &p->groups[i].func[j];
> > + f[c]->groups = devm_kzalloc(p->dev, sizeof(int),
> > + GFP_KERNEL);
>
> Add a NULL check.
>
> > + f[c]->groups[0] = i;
> > + f[c]->group_count = 1;
> > + c++;
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > +static int rt2880_pinmux_pins(struct rt2880_priv *p)
> > +{
> > + int i, j;
> > +
> > + /*
> > + * loop over the functions and initialize the pins array.
> > + * also work out the highest pin used.
> > + */
> > + for (i = 0; i < p->func_count; i++) {
> > + int pin;
> > +
> > + if (!p->func[i]->pin_count)
> > + continue;
> > +
> > + p->func[i]->pins = devm_kcalloc(p->dev,
> > + p->func[i]->pin_count,
> > + sizeof(int),
> > + GFP_KERNEL);
>
> This can fit on two lines.
>
> p->func[i]->pins = devm_kcalloc(p->dev, p->func[i]->pin_count,
> sizeof(int), GFP_KERNEL);
>
> > + for (j = 0; j < p->func[i]->pin_count; j++)
> > + p->func[i]->pins[j] = p->func[i]->pin_first + j;
> > +
> > + pin = p->func[i]->pin_first + p->func[i]->pin_count;
> > + if (pin > p->max_pins)
> > + p->max_pins = pin;
> > + }
> > +
> > + /* the buffer that tells us which pins are gpio */
> > + p->gpio = devm_kcalloc(p->dev, p->max_pins, sizeof(u8), GFP_KERNEL);
> > + /* the pads needed to tell pinctrl about our pins */
> > + p->pads = devm_kcalloc(p->dev, p->max_pins,
> > + sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
> > + if (!p->pads || !p->gpio) {
> > + dev_err(p->dev, "Failed to allocate gpio data\n");
>
> Delete this error message. #checkpatch.pl
>
> > + return -ENOMEM;
> > + }
> > +
> > + memset(p->gpio, 1, sizeof(u8) * p->max_pins);
> > + for (i = 0; i < p->func_count; i++) {
> > + if (!p->func[i]->pin_count)
> > + continue;
> > +
> > + for (j = 0; j < p->func[i]->pin_count; j++)
> > + p->gpio[p->func[i]->pins[j]] = 0;
> > + }
> > +
> > + /* pin 0 is always a gpio */
> > + p->gpio[0] = 1;
> > +
> > + /* set the pads */
> > + for (i = 0; i < p->max_pins; i++) {
> > + /* strlen("ioXY") + 1 = 5 */
> > + char *name = devm_kzalloc(p->dev, 5, GFP_KERNEL);
> > +
>
> char *name;
>
> name = kasprintf(GFP_KERNEL, "io%d", i);
>
>
> > + if (!name)
> > + return -ENOMEM;
> > + snprintf(name, 5, "io%d", i);
> > + p->pads[i].number = i;
> > + p->pads[i].name = name;
> > + }
> > + p->desc->pins = p->pads;
> > + p->desc->npins = p->max_pins;
> > +
> > + return 0;
> > +}
> > +
> > +static int rt2880_pinmux_probe(struct platform_device *pdev)
> > +{
> > + struct rt2880_priv *p;
> > + struct pinctrl_dev *dev;
> > +
> > + if (!rt2880_pinmux_data)
> > + return -ENOTSUPP;
> > +
> > + /* setup the private data */
> > + p = devm_kzalloc(&pdev->dev, sizeof(struct rt2880_priv), GFP_KERNEL);
> > + if (!p)
> > + return -ENOMEM;
> > +
> > + p->dev = &pdev->dev;
> > + p->desc = &rt2880_pctrl_desc;
> > + p->groups = rt2880_pinmux_data;
> > + platform_set_drvdata(pdev, p);
> > +
> > + /* init the device */
> > + if (rt2880_pinmux_index(p)) {
> > + dev_err(&pdev->dev, "failed to load index\n");
> > + return -EINVAL;
>
> Preserve the error code:
>
> err = rt2880_pinmux_index(p);
> if (err) {
> dev_err(&pdev->dev, "failed to load index\n");
> return err;
> }
>
>
> > + }
> > + if (rt2880_pinmux_pins(p)) {
> > + dev_err(&pdev->dev, "failed to load pins\n");
> > + return -EINVAL;
>
> Same.
>
> > + }
> > + dev = pinctrl_register(p->desc, &pdev->dev, p);
> > + if (IS_ERR(dev))
> > + return PTR_ERR(dev);
> > +
> > + return 0;
> > +}
This is already applied but needs more work in its correct place
afterwards. So I will take into account all of these comments and will
send patches to properly address all of them.
>
> regards,
> dan carpenter
Thanks again.
Best regards,
Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] staging: mt7621-pinctrl: remove driver from staging
2020-12-07 19:21 [PATCH 0/3] pinctrl: ralink: pinctrl driver for the rt2880 family Sergio Paracuellos
2020-12-07 19:21 ` [PATCH 1/3] dt-bindings: pinctrl: rt2880: add binding document Sergio Paracuellos
2020-12-07 19:21 ` [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family Sergio Paracuellos
@ 2020-12-07 19:21 ` Sergio Paracuellos
2 siblings, 0 replies; 10+ messages in thread
From: Sergio Paracuellos @ 2020-12-07 19:21 UTC (permalink / raw)
To: linus.walleij; +Cc: devel, devicetree, yanaijie, gregkh, linux-gpio, robh+dt
Remove this driver from staging because it has been moved
into its properly place in the kernel.
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
drivers/staging/Kconfig | 2 -
drivers/staging/Makefile | 1 -
drivers/staging/mt7621-pinctrl/Kconfig | 6 -
drivers/staging/mt7621-pinctrl/Makefile | 4 -
drivers/staging/mt7621-pinctrl/TODO | 6 -
.../staging/mt7621-pinctrl/pinctrl-rt2880.c | 370 ------------------
6 files changed, 389 deletions(-)
delete mode 100644 drivers/staging/mt7621-pinctrl/Kconfig
delete mode 100644 drivers/staging/mt7621-pinctrl/Makefile
delete mode 100644 drivers/staging/mt7621-pinctrl/TODO
delete mode 100644 drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 9b7cb7c5766a..c42708e60afc 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -94,8 +94,6 @@ source "drivers/staging/mt7621-pci/Kconfig"
source "drivers/staging/mt7621-pci-phy/Kconfig"
-source "drivers/staging/mt7621-pinctrl/Kconfig"
-
source "drivers/staging/mt7621-dma/Kconfig"
source "drivers/staging/ralink-gdma/Kconfig"
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 38226737c9f3..ebcc646d7b51 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -37,7 +37,6 @@ obj-$(CONFIG_BCM2835_VCHIQ) += vc04_services/
obj-$(CONFIG_PI433) += pi433/
obj-$(CONFIG_PCI_MT7621) += mt7621-pci/
obj-$(CONFIG_PCI_MT7621_PHY) += mt7621-pci-phy/
-obj-$(CONFIG_PINCTRL_RT2880) += mt7621-pinctrl/
obj-$(CONFIG_SOC_MT7621) += mt7621-dma/
obj-$(CONFIG_DMA_RALINK) += ralink-gdma/
obj-$(CONFIG_SOC_MT7621) += mt7621-dts/
diff --git a/drivers/staging/mt7621-pinctrl/Kconfig b/drivers/staging/mt7621-pinctrl/Kconfig
deleted file mode 100644
index f42974026480..000000000000
--- a/drivers/staging/mt7621-pinctrl/Kconfig
+++ /dev/null
@@ -1,6 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-config PINCTRL_RT2880
- bool "RT2800 pinctrl driver for RALINK/Mediatek SOCs"
- depends on RALINK
- select PINMUX
- select GENERIC_PINCONF
diff --git a/drivers/staging/mt7621-pinctrl/Makefile b/drivers/staging/mt7621-pinctrl/Makefile
deleted file mode 100644
index 49445f40c3cd..000000000000
--- a/drivers/staging/mt7621-pinctrl/Makefile
+++ /dev/null
@@ -1,4 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_PINCTRL_RT2880) += pinctrl-rt2880.o
-
-ccflags-y += -I$(srctree)/drivers/pinctrl
diff --git a/drivers/staging/mt7621-pinctrl/TODO b/drivers/staging/mt7621-pinctrl/TODO
deleted file mode 100644
index b2c235a16d5c..000000000000
--- a/drivers/staging/mt7621-pinctrl/TODO
+++ /dev/null
@@ -1,6 +0,0 @@
-
-- general code review and cleanup
-- should probably be always selected by 'config RALINK'
-- ensure device-tree requirements are documented
-
-Cc: NeilBrown <neil@brown.name>
diff --git a/drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c b/drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c
deleted file mode 100644
index e61dbe186bc9..000000000000
--- a/drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c
+++ /dev/null
@@ -1,370 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (C) 2013 John Crispin <blogic@openwrt.org>
- */
-
-#include <linux/module.h>
-#include <linux/device.h>
-#include <linux/io.h>
-#include <linux/platform_device.h>
-#include <linux/slab.h>
-#include <linux/of.h>
-#include <linux/pinctrl/pinctrl.h>
-#include <linux/pinctrl/pinconf.h>
-#include <linux/pinctrl/pinconf-generic.h>
-#include <linux/pinctrl/pinmux.h>
-#include <linux/pinctrl/consumer.h>
-#include <linux/pinctrl/machine.h>
-
-#include <asm/mach-ralink/ralink_regs.h>
-#include <asm/mach-ralink/pinmux.h>
-#include <asm/mach-ralink/mt7620.h>
-
-#include "core.h"
-#include "pinctrl-utils.h"
-
-#define SYSC_REG_GPIO_MODE 0x60
-#define SYSC_REG_GPIO_MODE2 0x64
-
-struct rt2880_priv {
- struct device *dev;
-
- struct pinctrl_pin_desc *pads;
- struct pinctrl_desc *desc;
-
- struct rt2880_pmx_func **func;
- int func_count;
-
- struct rt2880_pmx_group *groups;
- const char **group_names;
- int group_count;
-
- u8 *gpio;
- int max_pins;
-};
-
-static int rt2880_get_group_count(struct pinctrl_dev *pctrldev)
-{
- struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
- return p->group_count;
-}
-
-static const char *rt2880_get_group_name(struct pinctrl_dev *pctrldev,
- unsigned int group)
-{
- struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
- return (group >= p->group_count) ? NULL : p->group_names[group];
-}
-
-static int rt2880_get_group_pins(struct pinctrl_dev *pctrldev,
- unsigned int group,
- const unsigned int **pins,
- unsigned int *num_pins)
-{
- struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
- if (group >= p->group_count)
- return -EINVAL;
-
- *pins = p->groups[group].func[0].pins;
- *num_pins = p->groups[group].func[0].pin_count;
-
- return 0;
-}
-
-static const struct pinctrl_ops rt2880_pctrl_ops = {
- .get_groups_count = rt2880_get_group_count,
- .get_group_name = rt2880_get_group_name,
- .get_group_pins = rt2880_get_group_pins,
- .dt_node_to_map = pinconf_generic_dt_node_to_map_all,
- .dt_free_map = pinconf_generic_dt_free_map,
-};
-
-static int rt2880_pmx_func_count(struct pinctrl_dev *pctrldev)
-{
- struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
- return p->func_count;
-}
-
-static const char *rt2880_pmx_func_name(struct pinctrl_dev *pctrldev,
- unsigned int func)
-{
- struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
- return p->func[func]->name;
-}
-
-static int rt2880_pmx_group_get_groups(struct pinctrl_dev *pctrldev,
- unsigned int func,
- const char * const **groups,
- unsigned int * const num_groups)
-{
- struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
- if (p->func[func]->group_count == 1)
- *groups = &p->group_names[p->func[func]->groups[0]];
- else
- *groups = p->group_names;
-
- *num_groups = p->func[func]->group_count;
-
- return 0;
-}
-
-static int rt2880_pmx_group_enable(struct pinctrl_dev *pctrldev,
- unsigned int func, unsigned int group)
-{
- struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
- u32 mode = 0;
- u32 reg = SYSC_REG_GPIO_MODE;
- int i;
- int shift;
-
- /* dont allow double use */
- if (p->groups[group].enabled) {
- dev_err(p->dev, "%s is already enabled\n",
- p->groups[group].name);
- return -EBUSY;
- }
-
- p->groups[group].enabled = 1;
- p->func[func]->enabled = 1;
-
- shift = p->groups[group].shift;
- if (shift >= 32) {
- shift -= 32;
- reg = SYSC_REG_GPIO_MODE2;
- }
- mode = rt_sysc_r32(reg);
- mode &= ~(p->groups[group].mask << shift);
-
- /* mark the pins as gpio */
- for (i = 0; i < p->groups[group].func[0].pin_count; i++)
- p->gpio[p->groups[group].func[0].pins[i]] = 1;
-
- /* function 0 is gpio and needs special handling */
- if (func == 0) {
- mode |= p->groups[group].gpio << shift;
- } else {
- for (i = 0; i < p->func[func]->pin_count; i++)
- p->gpio[p->func[func]->pins[i]] = 0;
- mode |= p->func[func]->value << shift;
- }
- rt_sysc_w32(mode, reg);
-
- return 0;
-}
-
-static int rt2880_pmx_group_gpio_request_enable(struct pinctrl_dev *pctrldev,
- struct pinctrl_gpio_range *range,
- unsigned int pin)
-{
- struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
- if (!p->gpio[pin]) {
- dev_err(p->dev, "pin %d is not set to gpio mux\n", pin);
- return -EINVAL;
- }
-
- return 0;
-}
-
-static const struct pinmux_ops rt2880_pmx_group_ops = {
- .get_functions_count = rt2880_pmx_func_count,
- .get_function_name = rt2880_pmx_func_name,
- .get_function_groups = rt2880_pmx_group_get_groups,
- .set_mux = rt2880_pmx_group_enable,
- .gpio_request_enable = rt2880_pmx_group_gpio_request_enable,
-};
-
-static struct pinctrl_desc rt2880_pctrl_desc = {
- .owner = THIS_MODULE,
- .name = "rt2880-pinmux",
- .pctlops = &rt2880_pctrl_ops,
- .pmxops = &rt2880_pmx_group_ops,
-};
-
-static struct rt2880_pmx_func gpio_func = {
- .name = "gpio",
-};
-
-static int rt2880_pinmux_index(struct rt2880_priv *p)
-{
- struct rt2880_pmx_func **f;
- struct rt2880_pmx_group *mux = p->groups;
- int i, j, c = 0;
-
- /* count the mux functions */
- while (mux->name) {
- p->group_count++;
- mux++;
- }
-
- /* allocate the group names array needed by the gpio function */
- p->group_names = devm_kcalloc(p->dev, p->group_count,
- sizeof(char *), GFP_KERNEL);
- if (!p->group_names)
- return -1;
-
- for (i = 0; i < p->group_count; i++) {
- p->group_names[i] = p->groups[i].name;
- p->func_count += p->groups[i].func_count;
- }
-
- /* we have a dummy function[0] for gpio */
- p->func_count++;
-
- /* allocate our function and group mapping index buffers */
- f = p->func = devm_kcalloc(p->dev,
- p->func_count,
- sizeof(*p->func),
- GFP_KERNEL);
- gpio_func.groups = devm_kcalloc(p->dev, p->group_count, sizeof(int),
- GFP_KERNEL);
- if (!f || !gpio_func.groups)
- return -1;
-
- /* add a backpointer to the function so it knows its group */
- gpio_func.group_count = p->group_count;
- for (i = 0; i < gpio_func.group_count; i++)
- gpio_func.groups[i] = i;
-
- f[c] = &gpio_func;
- c++;
-
- /* add remaining functions */
- for (i = 0; i < p->group_count; i++) {
- for (j = 0; j < p->groups[i].func_count; j++) {
- f[c] = &p->groups[i].func[j];
- f[c]->groups = devm_kzalloc(p->dev, sizeof(int),
- GFP_KERNEL);
- f[c]->groups[0] = i;
- f[c]->group_count = 1;
- c++;
- }
- }
- return 0;
-}
-
-static int rt2880_pinmux_pins(struct rt2880_priv *p)
-{
- int i, j;
-
- /*
- * loop over the functions and initialize the pins array.
- * also work out the highest pin used.
- */
- for (i = 0; i < p->func_count; i++) {
- int pin;
-
- if (!p->func[i]->pin_count)
- continue;
-
- p->func[i]->pins = devm_kcalloc(p->dev,
- p->func[i]->pin_count,
- sizeof(int),
- GFP_KERNEL);
- for (j = 0; j < p->func[i]->pin_count; j++)
- p->func[i]->pins[j] = p->func[i]->pin_first + j;
-
- pin = p->func[i]->pin_first + p->func[i]->pin_count;
- if (pin > p->max_pins)
- p->max_pins = pin;
- }
-
- /* the buffer that tells us which pins are gpio */
- p->gpio = devm_kcalloc(p->dev, p->max_pins, sizeof(u8), GFP_KERNEL);
- /* the pads needed to tell pinctrl about our pins */
- p->pads = devm_kcalloc(p->dev, p->max_pins,
- sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
- if (!p->pads || !p->gpio) {
- dev_err(p->dev, "Failed to allocate gpio data\n");
- return -ENOMEM;
- }
-
- memset(p->gpio, 1, sizeof(u8) * p->max_pins);
- for (i = 0; i < p->func_count; i++) {
- if (!p->func[i]->pin_count)
- continue;
-
- for (j = 0; j < p->func[i]->pin_count; j++)
- p->gpio[p->func[i]->pins[j]] = 0;
- }
-
- /* pin 0 is always a gpio */
- p->gpio[0] = 1;
-
- /* set the pads */
- for (i = 0; i < p->max_pins; i++) {
- /* strlen("ioXY") + 1 = 5 */
- char *name = devm_kzalloc(p->dev, 5, GFP_KERNEL);
-
- if (!name)
- return -ENOMEM;
- snprintf(name, 5, "io%d", i);
- p->pads[i].number = i;
- p->pads[i].name = name;
- }
- p->desc->pins = p->pads;
- p->desc->npins = p->max_pins;
-
- return 0;
-}
-
-static int rt2880_pinmux_probe(struct platform_device *pdev)
-{
- struct rt2880_priv *p;
- struct pinctrl_dev *dev;
-
- if (!rt2880_pinmux_data)
- return -ENOTSUPP;
-
- /* setup the private data */
- p = devm_kzalloc(&pdev->dev, sizeof(struct rt2880_priv), GFP_KERNEL);
- if (!p)
- return -ENOMEM;
-
- p->dev = &pdev->dev;
- p->desc = &rt2880_pctrl_desc;
- p->groups = rt2880_pinmux_data;
- platform_set_drvdata(pdev, p);
-
- /* init the device */
- if (rt2880_pinmux_index(p)) {
- dev_err(&pdev->dev, "failed to load index\n");
- return -EINVAL;
- }
- if (rt2880_pinmux_pins(p)) {
- dev_err(&pdev->dev, "failed to load pins\n");
- return -EINVAL;
- }
- dev = pinctrl_register(p->desc, &pdev->dev, p);
- if (IS_ERR(dev))
- return PTR_ERR(dev);
-
- return 0;
-}
-
-static const struct of_device_id rt2880_pinmux_match[] = {
- { .compatible = "ralink,rt2880-pinmux" },
- {},
-};
-MODULE_DEVICE_TABLE(of, rt2880_pinmux_match);
-
-static struct platform_driver rt2880_pinmux_driver = {
- .probe = rt2880_pinmux_probe,
- .driver = {
- .name = "rt2880-pinmux",
- .of_match_table = rt2880_pinmux_match,
- },
-};
-
-int __init rt2880_pinmux_init(void)
-{
- return platform_driver_register(&rt2880_pinmux_driver);
-}
-
-core_initcall_sync(rt2880_pinmux_init);
--
2.25.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread