pinctrl: Add new pinctrl/GPIO driver
Subsystems (3)
v2 cover pinctrl: Add new pinctrl/GPIO driver
Hi,
This series is to add pinctrl & GPIO controller driver for a new SoC. Patch 1 adds pinmux & GPIO controller driver.
Patch 2 adds the corresponding dt bindings YAML document.
Patches are against Linux 5.4-rc1 at below Git tree:git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git
v2:
- Enable GENERIC_PINMUX_FUNCTIONS & GENERIC_PINCTRL_GROUPS and use core provided code for pinmux_ops & pinctrl_ops. Remove related code from the driver.
- Enable GENERIC_PINCONF & use core provided pinconf code. Remove related code from the driver.
- Use GPIOLIB_IRQCHIP framework core code instead of implementing separtely in the driver.
- Enable GPIO_GENERIC and switch to core provided memory mapped GPIO banks design.
- Use standard pinctrl DT properties instead of custom made properties.
- Address code quality related review concerns.
v1:
- Initial version.
Rahul Tanwar (2):
pinctrl: Add pinmux & GPIO controller driver for a new SoC
dt-bindings: pinctrl: intel: Add for new SoC
.../bindings/pinctrl/intel,lgm-pinctrl.yaml | 114 +++ drivers/pinctrl/Kconfig | 18 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-equilibrium.c | 957 +++++++++++++++++++++ drivers/pinctrl/pinctrl-equilibrium.h | 150 ++++ 5 files changed, 1240 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/intel,lgm-pinctrl.yaml create mode 100644 drivers/pinctrl/pinctrl-equilibrium.c create mode 100644 drivers/pinctrl/pinctrl-equilibrium.h
Signature · 1 line
2.11.0
PATCH v2 1/2 pinctrl: Add pinmux & GPIO controller driver for a new SoC
Intel Lightning Mountain SoC has a pinmux controller & GPIO controller IP which controls pin multiplexing & configuration including GPIO functions selection & GPIO attributes configuration.
This IP is not based on & does not have anything in common with Chassis specification. The pinctrl drivers under pinctrl/intel/* are all based upon Chassis spec compliant pinctrl IPs. So this driver doesn't fit & can not use pinctrl framework under pinctrl/intel/* and it requires a separate new driver.
Add a new GPIO & pin control framework based driver for this IP.
--- drivers/pinctrl/Kconfig | 18 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-equilibrium.c | 957 ++++++++++++++++++++++++++++++++++ drivers/pinctrl/pinctrl-equilibrium.h | 150 ++++++ 4 files changed, 1126 insertions(+) create mode 100644 drivers/pinctrl/pinctrl-equilibrium.c create mode 100644 drivers/pinctrl/pinctrl-equilibrium.h
drivers/pinctrl/Kconfig +18 -0
@@ -420,4 +420,22 @@ config PINCTRL_TB10X depends on OF && ARC_PLAT_TB10X select GPIOLIB +config PINCTRL_EQUILIBRIUM + tristate "Generic pinctrl and GPIO driver for Intel Lightning Mountain SoC" + select PINMUX + select PINCONF + select GPIOLIB + select GPIO_GENERIC + select GPIOLIB_IRQCHIP + select GENERIC_PINCONF + select GENERIC_PINCTRL_GROUPS + select GENERIC_PINMUX_FUNCTIONS + + help + Equilibrium pinctrl driver is a pinctrl & GPIO driver for Intel Lightning + Mountain network processor SoC that supports both the linux GPIO and pin + control frameworks. It provides interfaces to setup pinmux, assign desired + pin functions, configure GPIO attributes for LGM SoC pins. Pinmux and + pinconf settings are retrieved from device tree. + endif
drivers/pinctrl/Makefile +1 -0
@@ -46,6 +46,7 @@ obj-$(CONFIG_PINCTRL_ZYNQ) += pinctrl-zynq.o obj-$(CONFIG_PINCTRL_INGENIC) += pinctrl-ingenic.o obj-$(CONFIG_PINCTRL_RK805) += pinctrl-rk805.o obj-$(CONFIG_PINCTRL_OCELOT) += pinctrl-ocelot.o +obj-$(CONFIG_PINCTRL_EQUILIBRIUM) += pinctrl-equilibrium.o obj-y += actions/ obj-$(CONFIG_ARCH_ASPEED) += aspeed/
drivers/pinctrl/pinctrl-equilibrium.c new file +957 -0
957 lines not on this page · raw message
drivers/pinctrl/pinctrl-equilibrium.h new file +150 -0
150 lines not on this page · raw message
Signature · 1 line
2.11.0
PATCH v2 2/2 dt-bindings: pinctrl: intel: Add for new SoC
In reply to Rahul TanwarAdd dt bindings document for pinmux & GPIO controller driver of Intel Lightning Mountain SoC.
--- .../bindings/pinctrl/intel,lgm-pinctrl.yaml | 114 +++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/intel,lgm-pinctrl.yaml
Documentation/devicetree/bindings/pinctrl/intel,lgm-pinctrl.yaml new file +114 -0
@@ -0,0 +1,114 @@ +# SPDX-License-Identifier: GPL-2.0-only +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/bindings/pinctrl/intel,lgm-pinctrl.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Intel Lightning Mountain SoC pinmux & GPIO controller binding + +maintainers: + - Rahul Tanwar <[email protected]> + +description: | + Pinmux & GPIO controller controls pin multiplexing & configuration including + GPIO function selection & GPIO attributes configuration. + + Please refer to [1] for details of the common pinctrl bindings used by the + client devices. + + [1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt + +properties: + compatible: + const: intel,lgm-pinctrl + + reg: + maxItems: 1 + +# Client device subnode's properties +patternProperties: + "^.*@[0-9a-fA-F]+$": + type: object + description: + Pinctrl node's client devices use subnodes for desired pin configuration. + Client device subnodes use below standard properties. + + properties: + function: + $ref: /schemas/types.yaml#/definitions/string + description: + A string containing the name of the function to mux to the group. + + groups: + $ref: /schemas/types.yaml#/definitions/string-array + description: + An array of strings identifying the list of groups. + + pins: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: + List of pins to select with this function. + + pinmux: + description: The applicable mux group. + allOf: + - $ref: "/schemas/types.yaml#/definitions/uint32" + - enum: + - 0 #PINMUX_GPIO + - 1 + - 2 + - 3 + - 4 + + bias-pull-up: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Specifies pull-up configuration. + + bias-pull-down: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Specifies pull-down configuration. + + drive-strength: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Enables driver-current. + + slew-rate: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Enables slew-rate. + + drive-open-drain: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Specifies open-drain configuration. + + output-enable: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Specifies if the pin is to be configured as output. + + + required: + - intel,function + - intel,groups + +required: + - compatible + - reg + +examples: + # Pinmux controller node + - | + pinctrl: pinctrl@e2880000 { + compatible = "intel,lgm-pinctrl"; + reg = <0xe2880000 0x100000>; + + # Client device subnode + uart0:uart0 { + pins = <64>, /* UART_RX0 */ + <65>; /* UART_TX0 */ + function = "CONSOLE_UART0"; + pinmux = <1>, + <1>; + groups = "CONSOLE_UART0"; + }; + }; + +...
Signature · 1 line
2.11.0
Re: [PATCH v2 1/2] pinctrl: Add pinmux & GPIO controller driver for a new SoC
In reply to Rahul TanwarQuoted 10 lines · Rahul Tanwar
Intel Lightning Mountain SoC has a pinmux controller & GPIO controller IP which controls pin multiplexing & configuration including GPIO functions selection & GPIO attributes configuration.
This IP is not based on & does not have anything in common with Chassis specification. The pinctrl drivers under pinctrl/intel/* are all based upon Chassis spec compliant pinctrl IPs. So this driver doesn't fit & can not use pinctrl framework under pinctrl/intel/* and it requires a separate new driver.
Add a new GPIO & pin control framework based driver for this IP.
Thanks for an update, my comments below.
drivers/pinctrl/pinctrl-equilibrium.c line 25
Quoted 5 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +4
-0+#define PIN_NAME_FMT "io-%d" +#define PIN_NAME_LEN 10 +static inline void eqbr_set_val (void __iomem *addr,u32 offset, +u32 mask,u32 set,raw_spinlock_t *lock)
Why is it marked with inline?
drivers/pinctrl/pinctrl-equilibrium.c line 32
Quoted 8 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +7
-0+{ +u32 val; +unsigned long flags; + +raw_spin_lock_irqsave (lock, flags); + val =readl (addr); + val = (val & ~(mask << offset)) | ((set & mask) << offset);
This is unusual, why offset can't be applied once to the mask?
drivers/pinctrl/pinctrl-equilibrium.c line 34
Quoted 2 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +2
-0+writel (val, addr); +raw_spin_unlock_irqrestore (lock, flags);
Hmm... Don't you have more complicated workflow that requires few reads/writes/updates to be called atomically?
drivers/pinctrl/pinctrl-equilibrium.c line 43
Quoted 9 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +7
-0+} +static void eqbr_gpio_disable_irq (struct irq_data *d) +{ +unsigned int offset =irqd_to_hwirq (d); +struct gpio_chip *gc =irq_data_get_irq_chip_data (d); +struct eqbr_gpio_desc *desc =gpiochip_get_data (gc); +writel (BIT (offset), desc->membase +GPIO_IRNENCLR );
Is it okay to be without spin lock?
Same Q to the rest similar places.
drivers/pinctrl/pinctrl-equilibrium.c line 74
Quoted 6 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +5
-0+} +static inline void eqbr_cfg_bit (void __iomem *addr, +unsigned int offset,unsigned int set) +{ +if (!set)
Why not to use positive condition?
drivers/pinctrl/pinctrl-equilibrium.c line 100
Quoted 6 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +5
-0+writel (readl (addr) & ~BIT (offset), addr); + else + writel(readl(addr) | BIT(offset), addr); +} +struct gpio_irq_type it;
Not sure if this is used properly. Linus may clarify this.
drivers/pinctrl/pinctrl-equilibrium.c line 161
Quoted 12 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +11
-0+static void eqbr_irq_handler (struct irq_desc *desc) +{ +struct gpio_chip *gc =irq_desc_get_handler_data (desc); +struct eqbr_gpio_desc *gpio_desc =gpiochip_get_data (gc); +struct irq_chip *ic =irq_desc_get_chip (desc); +u32 pins, offset; + +chained_irq_enter (ic, desc); + pins =readl (gpio_desc->membase +GPIO_IRNCR ); + +for_each_set_bit (offset, (unsigned long *)&pins, gc->ngpio)
This casting is no go.
drivers/pinctrl/pinctrl-equilibrium.c line 173
Quoted 13 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +11
-0+generic_handle_irq (irq_find_mapping (gc->irq.domain, offset)); + +chained_irq_exit (ic, desc); +} +static int gpiochip_setup (struct device *dev,struct eqbr_gpio_desc *desc) +{ +struct gpio_irq_chip *girq; +struct gpio_chip *gc; + + gc = &desc->chip; + gc->owner =THIS_MODULE ;
Do we still need this in the drivers?
drivers/pinctrl/pinctrl-equilibrium.c line 177
Quoted 5 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +4
-0+ gc->label = desc->name; + gc->of_node = desc->node; + +if (!of_property_read_bool (desc->node,"interrupt-controller" )) {
Why is it fatal?
drivers/pinctrl/pinctrl-equilibrium.c line 195
Quoted 7 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +6
-0+dev_info (dev,"gc %s: doesn't act as interrupt controller!\n" , + desc->name); +return 0 ; + } + girq->parents =devm_kcalloc (dev,1 ,sizeof (*girq->parents), +GFP_KERNEL );
I believe it's fine to have it on one line.
drivers/pinctrl/pinctrl-equilibrium.c line 200
Quoted 6 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +5
-0+if (!girq->parents) +return -ENOMEM ; + + girq->default_type =IRQ_TYPE_NONE ; + girq->handler = handle_level_irq;
Not bad IRQ handler?
Quoted 22 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +21
-0+ girq->parents[0 ] = desc->virq; + +return 0 ; +} +static int gpiolib_reg (struct eqbr_pinctrl_drv_data *drvdata) +{ +struct device_node *np; +struct eqbr_gpio_desc *desc; +struct device *dev; +int i, ret; +struct resource res; + + dev = drvdata->dev; +for (i =0 ; i < drvdata->nr_gpio_descs; i++) { + desc = drvdata->gpio_desc + i; + np = desc->node; + + desc->name =devm_kasprintf (dev,GFP_KERNEL ,"gpiochip%d" , i); +if (!desc->name) +return -ENOMEM ; +
drivers/pinctrl/pinctrl-equilibrium.c line 230
Quoted 10 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +8
-0+if (of_address_to_resource (np,0 , &res)) { +dev_err (dev,"Failed to get GPIO register address\n" ); +return -ENXIO ; + } + + desc->membase =devm_ioremap_resource (dev, &res); +if (IS_ERR (desc->membase)) { +dev_err (dev,"ioremap fail\n" );
Redundant.
Review
Quoted 2 lines
(quoted diff) +2
-0+ return PTR_ERR(desc->membase); + }
Is it per descriptor?!
Quoted 12 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +11
-0+ + desc->virq =irq_of_parse_and_map (np,0 ); +if (!desc->virq) { +dev_err (dev,"%s: failed to parse and map irq\n" , + desc->name); +return -ENXIO ; + } +raw_spin_lock_init (&desc->lock); + } + +return 0 ;
drivers/pinctrl/pinctrl-equilibrium.c line 288
Quoted 10 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +9
-0+} +static const struct pinmux_ops eqbr_pinmux_ops = { + .get_functions_count = pinmux_generic_get_function_count, + .get_function_name = pinmux_generic_get_function_name, + .get_function_groups = pinmux_generic_get_function_groups, + .set_mux = eqbr_pinmux_set_mux, + .gpio_request_enable = eqbr_pinmux_gpio_request, + .strict = true, +};
TABs/spaces mix.
Quoted 2 lines
(quoted diff) +2
-0+ return 0; +}
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v2 1/2] pinctrl: Add pinmux & GPIO controller driver for a new SoC
Hi Andy,
Quoted 10 lines · Andy Shevchenko
Quoted 9 lines · Rahul Tanwar
Intel Lightning Mountain SoC has a pinmux controller & GPIO controller IP which controls pin multiplexing & configuration including GPIO functions selection & GPIO attributes configuration.
This IP is not based on & does not have anything in common with Chassis specification. The pinctrl drivers under pinctrl/intel/* are all based upon Chassis spec compliant pinctrl IPs. So this driver doesn't fit & can not use pinctrl framework under pinctrl/intel/* and it requires a separate new driver.
Quoted 6 lines
drivers/pinctrl/pinctrl-equilibrium.c line 25
Quoted 4 lines
Add a new GPIO & pin control framework based driver for this IP.
drivers/pinctrl/pinctrl-equilibrium.c new file +2
-0+static inline void eqbr_set_val (void __iomem *addr,u32 offset, +u32 mask,u32 set,raw_spinlock_t *lock)Why is it marked with inline?
Hard to justify. I can remove inline if you recommend so..
Quoted 8 lines
drivers/pinctrl/pinctrl-equilibrium.c line 32
Quoted 7 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +7
-0+{ +u32 val; +unsigned long flags; + +raw_spin_lock_irqsave (lock, flags); + val =readl (addr); + val = (val & ~(mask << offset)) | ((set & mask) << offset);This is unusual, why offset can't be applied once to the mask?
Do you mean like below ? (set still needs to be left shifted by offset)
mask = mask << offset;
val = (val & ~mask) | ((set << offset) & mask);
Quoted 4 lines
drivers/pinctrl/pinctrl-equilibrium.c line 34
Quoted 2 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +2
-0+writel (val, addr); +raw_spin_unlock_irqrestore (lock, flags);Hmm... Don't you have more complicated workflow that requires few reads/writes/updates to be called atomically?
I don't really see any complicated register programming in this driver. Just forpin configuration, mux setting & GPIO IRQ configuration..
Quoted 9 lines
drivers/pinctrl/pinctrl-equilibrium.c line 43
Quoted 7 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +7
-0+} +static void eqbr_gpio_disable_irq (struct irq_data *d) +{ +unsigned int offset =irqd_to_hwirq (d); +struct gpio_chip *gc =irq_data_get_irq_chip_data (d); +struct eqbr_gpio_desc *desc =gpiochip_get_data (gc); +writel (BIT (offset), desc->membase +GPIO_IRNENCLR );Is it okay to be without spin lock?
Same Q to the rest similar places.
Yes, you are right. GPIO IRQ ops also needs locking. I was incorrectly assuming that gpiolib will provide locking when it calls these ops. I will fix it in v3. Thanks.
Quoted 6 lines
drivers/pinctrl/pinctrl-equilibrium.c line 74
Quoted 5 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +5
-0+} +static inline void eqbr_cfg_bit (void __iomem *addr, +unsigned int offset,unsigned int set) +{ +if (!set)Why not to use positive condition?
Well noted.
Quoted 7 lines
drivers/pinctrl/pinctrl-equilibrium.c line 100
Quoted 5 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +5
-0+writel (readl (addr) & ~BIT (offset), addr); + else + writel(readl(addr) | BIT(offset), addr); +} +struct gpio_irq_type it;Not sure if this is used properly. Linus may clarify this.
Quoted 12 lines
drivers/pinctrl/pinctrl-equilibrium.c line 161
Quoted 11 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +11
-0+static void eqbr_irq_handler (struct irq_desc *desc) +{ +struct gpio_chip *gc =irq_desc_get_handler_data (desc); +struct eqbr_gpio_desc *gpio_desc =gpiochip_get_data (gc); +struct irq_chip *ic =irq_desc_get_chip (desc); +u32 pins, offset; + +chained_irq_enter (ic, desc); + pins =readl (gpio_desc->membase +GPIO_IRNCR ); + +for_each_set_bit (offset, (unsigned long *)&pins, gc->ngpio)This casting is no go.
Well noted. Copy paste mistake. Will fix it. Thanks.
Quoted 4 lines
Quoted 4 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +4
-0+generic_handle_irq (irq_find_mapping (gc->irq.domain, offset)); + +chained_irq_exit (ic, desc); +}
Quoted 8 lines
drivers/pinctrl/pinctrl-equilibrium.c line 173
Quoted 7 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +7
-0+static int gpiochip_setup (struct device *dev,struct eqbr_gpio_desc *desc) +{ +struct gpio_irq_chip *girq; +struct gpio_chip *gc; + + gc = &desc->chip; + gc->owner =THIS_MODULE ;Do we still need this in the drivers?
Agree it is redundant. Will remove it. Thanks.
Quoted 5 lines
drivers/pinctrl/pinctrl-equilibrium.c line 177
Quoted 4 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +4
-0+ gc->label = desc->name; + gc->of_node = desc->node; + +if (!of_property_read_bool (desc->node,"interrupt-controller" )) {Why is it fatal?
It is not fatal, that's why it returns 0 instead of error i.e. if this gpio chip is notintended to be used as interrupt controller then do not setup gpio_irq_chipand proceed to register gpio chip without interrupt support.
Quoted 7 lines
drivers/pinctrl/pinctrl-equilibrium.c line 195
Quoted 6 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +6
-0+dev_info (dev,"gc %s: doesn't act as interrupt controller!\n" , + desc->name); +return 0 ; + } + girq->parents =devm_kcalloc (dev,1 ,sizeof (*girq->parents), +GFP_KERNEL );I believe it's fine to have it on one line.
Well noted.
Quoted 6 lines
drivers/pinctrl/pinctrl-equilibrium.c line 200
Quoted 5 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +5
-0+if (!girq->parents) +return -ENOMEM ; + + girq->default_type =IRQ_TYPE_NONE ; + girq->handler = handle_level_irq;Not bad IRQ handler?
Yes, it should be handle_bad_irq(). Will fix, thanks.
Quoted 22 lines
Quoted 22 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +22
-0+ girq->parents[0 ] = desc->virq; + +return 0 ; +} +static int gpiolib_reg (struct eqbr_pinctrl_drv_data *drvdata) +{ +struct device_node *np; +struct eqbr_gpio_desc *desc; +struct device *dev; +int i, ret; +struct resource res; + + dev = drvdata->dev; +for (i =0 ; i < drvdata->nr_gpio_descs; i++) { + desc = drvdata->gpio_desc + i; + np = desc->node; + + desc->name =devm_kasprintf (dev,GFP_KERNEL ,"gpiochip%d" , i); +if (!desc->name) +return -ENOMEM ; + +if (of_address_to_resource (np,0 , &res)) {
Quoted 8 lines
drivers/pinctrl/pinctrl-equilibrium.c line 230
Quoted 7 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +7
-0+dev_err (dev,"Failed to get GPIO register address\n" ); +return -ENXIO ; + } + + desc->membase =devm_ioremap_resource (dev, &res); +if (IS_ERR (desc->membase)) { +dev_err (dev,"ioremap fail\n" );Redundant.
Well noted.
Quoted 3 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ return PTR_ERR(desc->membase); + }Is it per descriptor?!
Yes, it is per descriptor i.e. per gpio_chip. Each gpio_chip has different membase.
Quoted 13 lines
Quoted 13 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +13
-0+ + desc->virq =irq_of_parse_and_map (np,0 ); +if (!desc->virq) { +dev_err (dev,"%s: failed to parse and map irq\n" , + desc->name); +return -ENXIO ; + } +raw_spin_lock_init (&desc->lock); + } + +return 0 ; +} +static const struct pinmux_ops eqbr_pinmux_ops = {
Quoted 8 lines
drivers/pinctrl/pinctrl-equilibrium.c line 357
Quoted 7 lines
drivers/pinctrl/pinctrl-equilibrium.c new file +7
-0+ .get_functions_count = pinmux_generic_get_function_count, + .get_function_name = pinmux_generic_get_function_name, + .get_function_groups = pinmux_generic_get_function_groups, + .set_mux = eqbr_pinmux_set_mux, + .gpio_request_enable = eqbr_pinmux_gpio_request, + .strict = true, +};TABs/spaces mix.
Well noted. Thanks.
Regards,
Rahul
kbuild test robot
build botRe: [PATCH v2 1/2] pinctrl: Add pinmux & GPIO controller driver for a new SoC
In reply to Rahul TanwarThank you for the patch! Yet something to improve:
Hi Rahul,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pinctrl/devel]
[also build test ERROR on v5.4-rc5 next-20191031]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Rahul-Tanwar/pinctrl-Add-new-pinctrl-GPIO-driver/20191101-120332 base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel config: um-allmodconfig (attached as .config) compiler: gcc-7 (Debian 7.4.0-14) 7.4.0 reproduce: # save the attached .config to linux build tree make ARCH=um
If you fix the issue, kindly add following tag
All errors (new ones prefixed by >>):
drivers/pinctrl/pinctrl-equilibrium.c: In function 'gpiochip_setup':
>> drivers/pinctrl/pinctrl-equilibrium.c:175:4: error: 'struct gpio_chip' has no member named 'of_node'
gc->of_node = desc->node;
^~vim +175 drivers/pinctrl/pinctrl-equilibrium.c
166
167 static int gpiochip_setup(struct device *dev, struct eqbr_gpio_desc *desc)
168 {
169 struct gpio_irq_chip *girq;
170 struct gpio_chip *gc;
171
172 gc = &desc->chip;
173 gc->owner = THIS_MODULE;
174 gc->label = desc->name;
> 175 gc->of_node = desc->node;
176
177 if (!of_property_read_bool(desc->node, "interrupt-controller")) {
178 dev_info(dev, "gc %s: doesn't act as interrupt controller!\n",
179 desc->name);
180 return 0;
181 }
182
183 desc->ic.name = "gpio_irq";
184 desc->ic.irq_mask = eqbr_gpio_disable_irq;
185 desc->ic.irq_unmask = eqbr_gpio_enable_irq;
186 desc->ic.irq_ack = eqbr_gpio_ack_irq;
187 desc->ic.irq_mask_ack = eqbr_gpio_mask_ack_irq;
188 desc->ic.irq_set_type = eqbr_gpio_set_irq_type;
189
190 girq = &desc->chip.irq;
191 girq->chip = &desc->ic;
192 girq->parent_handler = eqbr_irq_handler;
193 girq->num_parents = 1;
194 girq->parents = devm_kcalloc(dev, 1, sizeof(*girq->parents),
195 GFP_KERNEL);
196 if (!girq->parents)
197 return -ENOMEM;
198
199 girq->default_type = IRQ_TYPE_NONE;
200 girq->handler = handle_level_irq;
201 girq->parents[0] = desc->virq;
202
203 return 0;
204 }
205
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation