* RE: [PATCH] can: rcar_canfd: Correct order of interrupt specifiers
From: Ramesh Shanmugasundaram @ 2016-11-23 14:04 UTC (permalink / raw)
To: Geert Uytterhoeven, Wolfgang Grandegger, Marc Kleine-Budde,
Chris Paterson
Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <1479908686-14028-1-git-send-email-geert+renesas@glider.be>
Hi Geert,
> Subject: [PATCH] can: rcar_canfd: Correct order of interrupt specifiers
>
> According to both DTS (example and actual files), and Linux driver code,
> the first interrupt specifier should be the Channel interrupt, while the
> second interrupt specifier should be the Global interrupt.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Documentation/devicetree/bindings/net/can/rcar_canfd.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> index 22a6f10bab057f46..58c27cd839e3a2ac 100644
> --- a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> +++ b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> @@ -11,7 +11,7 @@ Required properties:
> family-specific and/or generic versions.
>
> - reg: physical base address and size of the R-Car CAN FD register map.
> -- interrupts: interrupt specifier for the Global & Channel interrupts
> +- interrupts: interrupt specifiers for the Channel & Global interrupts
Thanks for correcting it.
Acked-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
Thanks,
Ramesh
^ permalink raw reply
* [PATCH RESEND 0/2] add support for AXP209 GPIOs functions
From: Quentin Schulz @ 2016-11-23 14:11 UTC (permalink / raw)
To: linus.walleij, gnurou, robh+dt, mark.rutland, wens, maxime.ripard
Cc: Quentin Schulz, linux-gpio, devicetree, linux-kernel,
linux-arm-kernel
The AXP209 PMIC has three GPIOs. Two of them can be muxed in other modes
(namely adc or regulator)[1] which cannot be used while the pin is in one
of GPIO modes.
This adds the possibility to use all functions of the GPIOs present in
the AXP209 PMIC thanks to the pinctrl subsystem.
An upcoming ADC driver for the AXP209 PMIC will make use of this pinctrl to
read ADC values of GPIO0 and GPIO1. At the moment, no driver is pinctrling
these GPIOs.
This patch also corrects the register used to read GPIO input status.
[1] see registers 90H, 92H and 93H at
http://dl.linux-sunxi.org/AXP/AXP209_Datasheet_v1.0en.pdf
Quentin Schulz (2):
gpio: axp209: use correct register for GPIO input status
gpio: axp209: add pinctrl support
.../devicetree/bindings/gpio/gpio-axp209.txt | 28 +-
drivers/gpio/gpio-axp209.c | 557 ++++++++++++++++++---
2 files changed, 504 insertions(+), 81 deletions(-)
--
Adding Maxime Ripard (original driver author) and LKML to mail recipients.
2.9.3
^ permalink raw reply
* [PATCH RESEND 1/2] gpio: axp209: use correct register for GPIO input status
From: Quentin Schulz @ 2016-11-23 14:11 UTC (permalink / raw)
To: linus.walleij, gnurou, robh+dt, mark.rutland, wens, maxime.ripard
Cc: Quentin Schulz, linux-gpio, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <20161123141151.25315-1-quentin.schulz@free-electrons.com>
The GPIO input status was read from control register
(AXP20X_GPIO[210]_CTRL) instead of status register (AXP20X_GPIO20_SS).
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
drivers/gpio/gpio-axp209.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpio-axp209.c b/drivers/gpio/gpio-axp209.c
index d9c2a51..4a346b7 100644
--- a/drivers/gpio/gpio-axp209.c
+++ b/drivers/gpio/gpio-axp209.c
@@ -64,13 +64,9 @@ static int axp20x_gpio_get(struct gpio_chip *chip, unsigned offset)
{
struct axp20x_gpio *gpio = gpiochip_get_data(chip);
unsigned int val;
- int reg, ret;
-
- reg = axp20x_gpio_get_reg(offset);
- if (reg < 0)
- return reg;
+ int ret;
- ret = regmap_read(gpio->regmap, reg, &val);
+ ret = regmap_read(gpio->regmap, AXP20X_GPIO20_SS, &val);
if (ret)
return ret;
--
2.9.3
^ permalink raw reply related
* [PATCH RESEND 2/2] gpio: axp209: add pinctrl support
From: Quentin Schulz @ 2016-11-23 14:11 UTC (permalink / raw)
To: linus.walleij, gnurou, robh+dt, mark.rutland, wens, maxime.ripard
Cc: Quentin Schulz, linux-gpio, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <20161123141151.25315-1-quentin.schulz@free-electrons.com>
The GPIOs present in the AXP209 PMIC have multiple functions. They
typically allow a pin to be used as GPIO input or output and can also be
used as ADC or regulator for example.[1]
This adds the possibility to use all functions of the GPIOs present in
the AXP209 PMIC thanks to pinctrl subsystem.
[1] see registers 90H, 92H and 93H at
http://dl.linux-sunxi.org/AXP/AXP209_Datasheet_v1.0en.pdf
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
.../devicetree/bindings/gpio/gpio-axp209.txt | 28 +-
drivers/gpio/gpio-axp209.c | 551 ++++++++++++++++++---
2 files changed, 503 insertions(+), 76 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
index a661130..a5bfe87 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
@@ -1,4 +1,4 @@
-AXP209 GPIO controller
+AXP209 GPIO & pinctrl controller
This driver follows the usual GPIO bindings found in
Documentation/devicetree/bindings/gpio/gpio.txt
@@ -28,3 +28,29 @@ axp209: pmic@34 {
#gpio-cells = <2>;
};
};
+
+The GPIOs can be muxed to other functions and therefore, must be a subnode of
+axp_gpio.
+
+Example:
+
+&axp_gpio {
+ gpio0_adc: gpio0_adc {
+ pin = "GPIO0";
+ function = "adc";
+ };
+};
+
+&example_node {
+ pinctrl-names = "default";
+ pinctrl-0 = <&gpio0_adc>;
+};
+
+GPIOs and their functions
+-------------------------
+
+GPIO | Functions
+------------------------
+GPIO0 | gpio_in, gpio_out, ldo, adc
+GPIO1 | gpio_in, gpio_out, ldo, adc
+GPIO2 | gpio_in, gpio_out
diff --git a/drivers/gpio/gpio-axp209.c b/drivers/gpio/gpio-axp209.c
index 4a346b7..0a64cfc 100644
--- a/drivers/gpio/gpio-axp209.c
+++ b/drivers/gpio/gpio-axp209.c
@@ -1,7 +1,8 @@
/*
- * AXP20x GPIO driver
+ * AXP20x Pin control driver
*
* Copyright (C) 2016 Maxime Ripard <maxime.ripard@free-electrons.com>
+ * Copyright (C) 2016 Quentin Schulz <quentin.schulz@free-electrons.com>
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
@@ -21,52 +22,103 @@
#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/slab.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/pinconf-generic.h>
#define AXP20X_GPIO_FUNCTIONS 0x7
#define AXP20X_GPIO_FUNCTION_OUT_LOW 0
#define AXP20X_GPIO_FUNCTION_OUT_HIGH 1
#define AXP20X_GPIO_FUNCTION_INPUT 2
-struct axp20x_gpio {
- struct gpio_chip chip;
- struct regmap *regmap;
-};
+#define AXP20X_PINCTRL_PIN(_pin_num, _pin, _regs) \
+ { \
+ .number = _pin_num, \
+ .name = _pin, \
+ .drv_data = _regs, \
+ }
-static int axp20x_gpio_get_reg(unsigned offset)
-{
- switch (offset) {
- case 0:
- return AXP20X_GPIO0_CTRL;
- case 1:
- return AXP20X_GPIO1_CTRL;
- case 2:
- return AXP20X_GPIO2_CTRL;
+#define AXP20X_PIN(_pin, ...) \
+ { \
+ .pin = _pin, \
+ .functions = (struct axp20x_desc_function[]) { \
+ __VA_ARGS__, { } }, \
}
- return -EINVAL;
-}
+#define AXP20X_FUNCTION(_val, _name) \
+ { \
+ .name = _name, \
+ .muxval = _val, \
+ }
-static int axp20x_gpio_input(struct gpio_chip *chip, unsigned offset)
-{
- struct axp20x_gpio *gpio = gpiochip_get_data(chip);
- int reg;
+struct axp20x_desc_function {
+ const char *name;
+ u8 muxval;
+};
- reg = axp20x_gpio_get_reg(offset);
- if (reg < 0)
- return reg;
+struct axp20x_desc_pin {
+ struct pinctrl_pin_desc pin;
+ struct axp20x_desc_function *functions;
+};
- return regmap_update_bits(gpio->regmap, reg,
- AXP20X_GPIO_FUNCTIONS,
- AXP20X_GPIO_FUNCTION_INPUT);
-}
+struct axp20x_pinctrl_desc {
+ const struct axp20x_desc_pin *pins;
+ int npins;
+ unsigned int pin_base;
+};
+
+struct axp20x_pinctrl_function {
+ const char *name;
+ const char **groups;
+ unsigned int ngroups;
+};
+
+struct axp20x_pinctrl_group {
+ const char *name;
+ unsigned long config;
+ unsigned int pin;
+};
+
+struct axp20x_pctl {
+ struct pinctrl_dev *pctl_dev;
+ struct device *dev;
+ struct gpio_chip chip;
+ struct regmap *regmap;
+ const struct axp20x_pinctrl_desc *desc;
+ struct axp20x_pinctrl_group *groups;
+ unsigned int ngroups;
+ struct axp20x_pinctrl_function *functions;
+ unsigned int nfunctions;
+};
+
+static const struct axp20x_desc_pin axp209_pins[] = {
+ AXP20X_PIN(AXP20X_PINCTRL_PIN(0, "GPIO0", (void *)AXP20X_GPIO0_CTRL),
+ AXP20X_FUNCTION(0x0, "gpio_out"),
+ AXP20X_FUNCTION(0x2, "gpio_in"),
+ AXP20X_FUNCTION(0x3, "ldo"),
+ AXP20X_FUNCTION(0x4, "adc")),
+ AXP20X_PIN(AXP20X_PINCTRL_PIN(1, "GPIO1", (void *)AXP20X_GPIO1_CTRL),
+ AXP20X_FUNCTION(0x0, "gpio_out"),
+ AXP20X_FUNCTION(0x2, "gpio_in"),
+ AXP20X_FUNCTION(0x3, "ldo"),
+ AXP20X_FUNCTION(0x4, "adc")),
+ AXP20X_PIN(AXP20X_PINCTRL_PIN(2, "GPIO2", (void *)AXP20X_GPIO2_CTRL),
+ AXP20X_FUNCTION(0x0, "gpio_out"),
+ AXP20X_FUNCTION(0x2, "gpio_in")),
+};
+
+static const struct axp20x_pinctrl_desc axp20x_pinctrl_data = {
+ .pins = axp209_pins,
+ .npins = ARRAY_SIZE(axp209_pins),
+};
static int axp20x_gpio_get(struct gpio_chip *chip, unsigned offset)
{
- struct axp20x_gpio *gpio = gpiochip_get_data(chip);
+ struct axp20x_pctl *pctl = gpiochip_get_data(chip);
unsigned int val;
int ret;
- ret = regmap_read(gpio->regmap, AXP20X_GPIO20_SS, &val);
+ ret = regmap_read(pctl->regmap, AXP20X_GPIO20_SS, &val);
if (ret)
return ret;
@@ -75,15 +127,12 @@ static int axp20x_gpio_get(struct gpio_chip *chip, unsigned offset)
static int axp20x_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
{
- struct axp20x_gpio *gpio = gpiochip_get_data(chip);
+ struct axp20x_pctl *pctl = gpiochip_get_data(chip);
+ int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
unsigned int val;
- int reg, ret;
-
- reg = axp20x_gpio_get_reg(offset);
- if (reg < 0)
- return reg;
+ int ret;
- ret = regmap_read(gpio->regmap, reg, &val);
+ ret = regmap_read(pctl->regmap, pin_reg, &val);
if (ret)
return ret;
@@ -102,33 +151,335 @@ static int axp20x_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
return val & 2;
}
-static int axp20x_gpio_output(struct gpio_chip *chip, unsigned offset,
+static void axp20x_gpio_set(struct gpio_chip *chip, unsigned int offset,
+ int value)
+{
+ struct axp20x_pctl *pctl = gpiochip_get_data(chip);
+ int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
+
+ regmap_update_bits(pctl->regmap, pin_reg,
+ AXP20X_GPIO_FUNCTIONS,
+ value ? AXP20X_GPIO_FUNCTION_OUT_HIGH
+ : AXP20X_GPIO_FUNCTION_OUT_LOW);
+}
+
+static int axp20x_gpio_input(struct gpio_chip *chip, unsigned int offset)
+{
+ return pinctrl_gpio_direction_input(chip->base + offset);
+}
+
+static int axp20x_gpio_output(struct gpio_chip *chip, unsigned int offset,
int value)
{
- struct axp20x_gpio *gpio = gpiochip_get_data(chip);
- int reg;
+ chip->set(chip, offset, value);
- reg = axp20x_gpio_get_reg(offset);
- if (reg < 0)
- return reg;
+ return 0;
+}
- return regmap_update_bits(gpio->regmap, reg,
- AXP20X_GPIO_FUNCTIONS,
- value ? AXP20X_GPIO_FUNCTION_OUT_HIGH
- : AXP20X_GPIO_FUNCTION_OUT_LOW);
+static int axp20x_pmx_set(struct pinctrl_dev *pctldev, unsigned int offset,
+ u8 config)
+{
+ struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
+
+ return regmap_update_bits(pctl->regmap, pin_reg, AXP20X_GPIO_FUNCTIONS,
+ config);
}
-static void axp20x_gpio_set(struct gpio_chip *chip, unsigned offset,
- int value)
+static int axp20x_pmx_func_cnt(struct pinctrl_dev *pctldev)
+{
+ struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->nfunctions;
+}
+
+static const char *axp20x_pmx_func_name(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->functions[selector].name;
+}
+
+static int axp20x_pmx_func_groups(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ const char * const **groups,
+ unsigned int *num_groups)
+{
+ struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ *groups = pctl->functions[selector].groups;
+ *num_groups = pctl->functions[selector].ngroups;
+
+ return 0;
+}
+
+static struct axp20x_desc_function *
+axp20x_pinctrl_desc_find_func_by_name(struct axp20x_pctl *pctl,
+ const char *group, const char *func)
+{
+ const struct axp20x_desc_pin *pin;
+ struct axp20x_desc_function *desc_func;
+ int i;
+
+ for (i = 0; i < pctl->desc->npins; i++) {
+ pin = &pctl->desc->pins[i];
+
+ if (!strcmp(pin->pin.name, group)) {
+ desc_func = pin->functions;
+
+ while (desc_func->name) {
+ if (!strcmp(desc_func->name, func))
+ return desc_func;
+ desc_func++;
+ }
+
+ /*
+ * Pins are uniquely named. Groups are named after one
+ * pin name. If one pin matches group name but its
+ * function cannot be found, no other pin will match
+ * group name.
+ */
+ return NULL;
+ }
+ }
+
+ return NULL;
+}
+
+static int axp20x_pmx_set_mux(struct pinctrl_dev *pctldev,
+ unsigned int function, unsigned int group)
+{
+ struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ struct axp20x_pinctrl_group *g = pctl->groups + group;
+ struct axp20x_pinctrl_function *func = pctl->functions + function;
+ struct axp20x_desc_function *desc_func =
+ axp20x_pinctrl_desc_find_func_by_name(pctl, g->name,
+ func->name);
+ if (!desc_func)
+ return -EINVAL;
+
+ return axp20x_pmx_set(pctldev, g->pin, desc_func->muxval);
+}
+
+static struct axp20x_desc_function *
+axp20x_pctl_desc_find_func_by_pin(struct axp20x_pctl *pctl, unsigned int offset,
+ const char *func)
+{
+ const struct axp20x_desc_pin *pin;
+ struct axp20x_desc_function *desc_func;
+ int i;
+
+ for (i = 0; i < pctl->desc->npins; i++) {
+ pin = &pctl->desc->pins[i];
+
+ if (pin->pin.number == offset) {
+ desc_func = pin->functions;
+
+ while (desc_func->name) {
+ if (!strcmp(desc_func->name, func))
+ return desc_func;
+
+ desc_func++;
+ }
+ }
+ }
+
+ return NULL;
+}
+
+static int axp20x_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range *range,
+ unsigned int offset, bool input)
+{
+ struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ struct axp20x_desc_function *desc_func;
+ const char *func;
+
+ if (input)
+ func = "gpio_in";
+ else
+ func = "gpio_out";
+
+ desc_func = axp20x_pctl_desc_find_func_by_pin(pctl, offset, func);
+ if (!desc_func)
+ return -EINVAL;
+
+ return axp20x_pmx_set(pctldev, offset, desc_func->muxval);
+}
+
+static const struct pinmux_ops axp20x_pmx_ops = {
+ .get_functions_count = axp20x_pmx_func_cnt,
+ .get_function_name = axp20x_pmx_func_name,
+ .get_function_groups = axp20x_pmx_func_groups,
+ .set_mux = axp20x_pmx_set_mux,
+ .gpio_set_direction = axp20x_pmx_gpio_set_direction,
+ .strict = true,
+};
+
+static int axp20x_groups_cnt(struct pinctrl_dev *pctldev)
+{
+ struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->ngroups;
+}
+
+static int axp20x_group_pins(struct pinctrl_dev *pctldev, unsigned int selector,
+ const unsigned int **pins, unsigned int *num_pins)
+{
+ struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ struct axp20x_pinctrl_group *g = pctl->groups + selector;
+
+ *pins = (unsigned int *)&g->pin;
+ *num_pins = 1;
+
+ return 0;
+}
+
+static const char *axp20x_group_name(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->groups[selector].name;
+}
+
+static const struct pinctrl_ops axp20x_pctrl_ops = {
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_group,
+ .dt_free_map = pinconf_generic_dt_free_map,
+ .get_groups_count = axp20x_groups_cnt,
+ .get_group_name = axp20x_group_name,
+ .get_group_pins = axp20x_group_pins,
+};
+
+static struct axp20x_pinctrl_function *
+axp20x_pinctrl_function_by_name(struct axp20x_pctl *pctl, const char *name)
+{
+ struct axp20x_pinctrl_function *func = pctl->functions;
+
+ while (func->name) {
+ if (!strcmp(func->name, name))
+ return func;
+ func++;
+ }
+
+ return NULL;
+}
+
+static int axp20x_pinctrl_add_function(struct axp20x_pctl *pctl,
+ const char *name)
{
- axp20x_gpio_output(chip, offset, value);
+ struct axp20x_pinctrl_function *func = pctl->functions;
+
+ while (func->name) {
+ if (!strcmp(func->name, name)) {
+ func->ngroups++;
+ return -EEXIST;
+ }
+
+ func++;
+ }
+
+ func->name = name;
+ func->ngroups = 1;
+
+ pctl->nfunctions++;
+
+ return 0;
}
-static int axp20x_gpio_probe(struct platform_device *pdev)
+static int axp20x_attach_group_function(struct platform_device *pdev,
+ const struct axp20x_desc_pin *pin)
+{
+ struct axp20x_pctl *pctl = platform_get_drvdata(pdev);
+ struct axp20x_desc_function *desc_func = pin->functions;
+ struct axp20x_pinctrl_function *func;
+ const char **func_grp;
+
+ while (desc_func->name) {
+ func = axp20x_pinctrl_function_by_name(pctl, desc_func->name);
+ if (!func)
+ return -EINVAL;
+
+ if (!func->groups) {
+ func->groups = devm_kzalloc(&pdev->dev,
+ func->ngroups * sizeof(const char *),
+ GFP_KERNEL);
+ if (!func->groups)
+ return -ENOMEM;
+ }
+
+ func_grp = func->groups;
+ while (*func_grp)
+ func_grp++;
+
+ *func_grp = pin->pin.name;
+ desc_func++;
+ }
+
+ return 0;
+}
+
+static int axp20x_build_state(struct platform_device *pdev)
+{
+ struct axp20x_pctl *pctl = platform_get_drvdata(pdev);
+ unsigned int npins = pctl->desc->npins;
+ const struct axp20x_desc_pin *pin;
+ struct axp20x_desc_function *func;
+ int i, ret;
+
+ pctl->ngroups = npins;
+ pctl->groups = devm_kzalloc(&pdev->dev,
+ pctl->ngroups * sizeof(*pctl->groups),
+ GFP_KERNEL);
+ if (!pctl->groups)
+ return -ENOMEM;
+
+ for (i = 0; i < npins; i++) {
+ pctl->groups[i].name = pctl->desc->pins[i].pin.name;
+ pctl->groups[i].pin = pctl->desc->pins[i].pin.number;
+ }
+
+ /* We assume 4 functions per pin should be enough as a default max */
+ pctl->functions = devm_kzalloc(&pdev->dev,
+ npins * 4 * sizeof(*pctl->functions),
+ GFP_KERNEL);
+ if (!pctl->functions)
+ return -ENOMEM;
+
+ /* Create a list of uniquely named functions */
+ for (i = 0; i < npins; i++) {
+ pin = &pctl->desc->pins[i];
+ func = pin->functions;
+
+ while (func->name) {
+ axp20x_pinctrl_add_function(pctl, func->name);
+ func++;
+ }
+ }
+
+ pctl->functions = krealloc(pctl->functions,
+ pctl->nfunctions * sizeof(*pctl->functions),
+ GFP_KERNEL);
+
+ for (i = 0; i < npins; i++) {
+ pin = &pctl->desc->pins[i];
+ ret = axp20x_attach_group_function(pdev, pin);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int axp20x_pctl_probe(struct platform_device *pdev)
{
struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
- struct axp20x_gpio *gpio;
- int ret;
+ const struct axp20x_desc_pin *pin;
+ struct axp20x_pctl *pctl;
+ struct pinctrl_desc *pctrl_desc;
+ struct pinctrl_pin_desc *pins;
+ int ret, i;
if (!of_device_is_available(pdev->dev.of_node))
return -ENODEV;
@@ -138,51 +489,101 @@ static int axp20x_gpio_probe(struct platform_device *pdev)
return -EINVAL;
}
- gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
- if (!gpio)
+ pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL);
+ if (!pctl)
+ return -ENOMEM;
+
+ pctl->chip.base = -1;
+ pctl->chip.can_sleep = true;
+ pctl->chip.request = gpiochip_generic_request;
+ pctl->chip.free = gpiochip_generic_free;
+ pctl->chip.parent = &pdev->dev;
+ pctl->chip.label = dev_name(&pdev->dev);
+ pctl->chip.owner = THIS_MODULE;
+ pctl->chip.get = axp20x_gpio_get;
+ pctl->chip.get_direction = axp20x_gpio_get_direction;
+ pctl->chip.set = axp20x_gpio_set;
+ pctl->chip.direction_input = axp20x_gpio_input;
+ pctl->chip.direction_output = axp20x_gpio_output;
+ pctl->chip.ngpio = 3;
+ pctl->chip.can_sleep = true;
+
+ pctl->regmap = axp20x->regmap;
+
+ pctl->desc = &axp20x_pinctrl_data;
+ pctl->dev = &pdev->dev;
+
+ platform_set_drvdata(pdev, pctl);
+
+ ret = axp20x_build_state(pdev);
+ if (ret)
+ return ret;
+
+ pins = devm_kzalloc(&pdev->dev, pctl->desc->npins * sizeof(*pins),
+ GFP_KERNEL);
+ if (!pins)
return -ENOMEM;
- gpio->chip.base = -1;
- gpio->chip.can_sleep = true;
- gpio->chip.parent = &pdev->dev;
- gpio->chip.label = dev_name(&pdev->dev);
- gpio->chip.owner = THIS_MODULE;
- gpio->chip.get = axp20x_gpio_get;
- gpio->chip.get_direction = axp20x_gpio_get_direction;
- gpio->chip.set = axp20x_gpio_set;
- gpio->chip.direction_input = axp20x_gpio_input;
- gpio->chip.direction_output = axp20x_gpio_output;
- gpio->chip.ngpio = 3;
-
- gpio->regmap = axp20x->regmap;
-
- ret = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
+ for (i = 0; i < pctl->desc->npins; i++)
+ pins[i] = pctl->desc->pins[i].pin;
+
+ pctrl_desc = devm_kzalloc(&pdev->dev, sizeof(*pctrl_desc), GFP_KERNEL);
+ if (!pctrl_desc)
+ return -ENOMEM;
+
+ pctrl_desc->name = dev_name(&pdev->dev);
+ pctrl_desc->owner = THIS_MODULE;
+ pctrl_desc->pins = pins;
+ pctrl_desc->npins = pctl->desc->npins;
+ pctrl_desc->pctlops = &axp20x_pctrl_ops;
+ pctrl_desc->pmxops = &axp20x_pmx_ops;
+
+ pctl->pctl_dev = devm_pinctrl_register(&pdev->dev, pctrl_desc, pctl);
+ if (IS_ERR(pctl->pctl_dev)) {
+ dev_err(&pdev->dev, "couldn't register pinctrl driver\n");
+ return PTR_ERR(pctl->pctl_dev);
+ }
+
+ ret = devm_gpiochip_add_data(&pdev->dev, &pctl->chip, pctl);
if (ret) {
dev_err(&pdev->dev, "Failed to register GPIO chip\n");
return ret;
}
+ for (i = 0; i < pctl->desc->npins; i++) {
+ pin = pctl->desc->pins + i;
+
+ ret = gpiochip_add_pin_range(&pctl->chip, dev_name(&pdev->dev),
+ pin->pin.number, pin->pin.number,
+ 1);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to add pin range\n");
+ return ret;
+ }
+ }
+
dev_info(&pdev->dev, "AXP209 GPIO driver loaded\n");
return 0;
}
-static const struct of_device_id axp20x_gpio_match[] = {
+static const struct of_device_id axp20x_pctl_match[] = {
{ .compatible = "x-powers,axp209-gpio" },
{ }
};
-MODULE_DEVICE_TABLE(of, axp20x_gpio_match);
+MODULE_DEVICE_TABLE(of, axp20x_pctl_match);
-static struct platform_driver axp20x_gpio_driver = {
- .probe = axp20x_gpio_probe,
+static struct platform_driver axp20x_pctl_driver = {
+ .probe = axp20x_pctl_probe,
.driver = {
.name = "axp20x-gpio",
- .of_match_table = axp20x_gpio_match,
+ .of_match_table = axp20x_pctl_match,
},
};
-module_platform_driver(axp20x_gpio_driver);
+module_platform_driver(axp20x_pctl_driver);
MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
+MODULE_AUTHOR("Quentin Schulz <quentin.schulz@free-electrons.com>");
MODULE_DESCRIPTION("AXP20x PMIC GPIO driver");
MODULE_LICENSE("GPL");
--
2.9.3
^ permalink raw reply related
* Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-11-23 14:16 UTC (permalink / raw)
To: Gabriele Paoloni
Cc: linux-arm-kernel@lists.infradead.org, mark.rutland@arm.com,
benh@kernel.crashing.org, catalin.marinas@arm.com,
liviu.dudau@arm.com, Linuxarm, lorenzo.pieralisi@arm.com,
xuwei (O), Jason Gunthorpe, linux-serial@vger.kernel.org,
linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
minyard@acm.org, will.deacon@arm.com, John Garry, zourongrong
In-Reply-To: <EE11001F9E5DDD47B7634E2F8A612F2E1F921A0F@lhreml507-mbx>
On Friday, November 18, 2016 5:03:11 PM CET Gabriele Paoloni wrote:
> > On Friday, November 18, 2016 4:18:07 PM CET Gabriele Paoloni wrote:
> > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > > On Friday, November 18, 2016 12:53:08 PM CET Gabriele Paoloni
> > wrote:
> > > > For the ISA/LPC spaces there are only 4k of addresses, they
> > > > the bus addresses always overlap, but we can trivially
> > > > figure out the bus address from Linux I/O port number
> > > > by subtracting the start of the range.
> > >
> > > Are you saying that our LPC controller should specify a
> > > range property to map bus addresses into a cpu address range?
> >
> > No. There is not CPU address associated with it, because it's
> > not memory mapped.
> >
> > Instead, we need to associate a bus address with a logical
> > Linux port number, both in of_address_to_resource and
> > in inb()/outb().
>
> I think this is effectively what we are doing so far with patch 2/3.
> The problem with this patch is that we are carving out a "forbidden"
> IO tokens range that goes from 0 to PCIBIOS_MIN_IO.
>
> I think that the proper solution would be to have the LPC driver to
> set the carveout threshold used in pci_register_io_range(),
> pci_pio_to_address(), pci_address_to_pio(), but this would impose
> a probe dependency on the LPC itself that should be probed before
> the PCI controller (or before any other devices calling these
> functions...)
Why do you think the order matters? My point was that we should
be able to register any region of logical port numbers for any
bus here.
> > > > > To be honest with you I would keep things simple for this
> > > > > LPC and introduce more complex reworks later if more devices
> > > > > need to be introduced.
> > > > >
> > > > > What if we stick on a single domain now where we introduce a
> > > > > reserved threshold for the IO space (say INDIRECT_MAX_IO).
> > > >
> > > > I said having a single domain is fine, but I still don't
> > > > like the idea of reserving low port numbers for this hack,
> > > > it would mean that the numbers change for everyone else.
> > >
> > > I don't get this much...I/O tokens that are passed to the I/O
> > > accessors are not fixed anyway and they vary depending on the order
> > > of adding ranges to io_range_list...so I don't see a big issue
> > > with this...
> >
> > On machines with a legacy devices behind the PCI bridge,
> > there may still be a reason to have the low I/O port range
> > reserved for the primary bus, e.g. to get a VGA text console
> > to work.
> >
> > On powerpc, this is called the "primary" PCI host, i.e. the
> > only one that is allowed to have an ISA bridge.
>
> Yes but
> 1) isn't the PCI controller range property that defines how IO bus address
> map into physical CPU addresses?
Correct, but the DT knows nothing about logical port numbers in Linux.
> 2) How can you guarantee that the cpu range associated with this
> IO bus range is the first to be registered in pci_register_io_range()?
> ( i.e. are you saying that they are just relying on the fact that it is the
> only IO range in the system and by chance the IO tokens and corresponding
> bus addresses are the same? )
To clarify: the special properties of having the first 0x1000 logical
port numbers go to a particular physical bus are very obscure. I think
it's more important to not change the behavior for existing systems
that might rely on it than for new systems that have no such legacy.
The ipmi and uart drivers in particular will get the port numbers filled
in their platform device from the DT bus scanning, so they don't care
at all about having the same numeric value for port numbers on the bus
and logical numbers, but other drivers might rely on particular ports
to be mapped on a specific PCI host, especially when those drivers
are used only on systems that don't have more than one PCI domain.
Arnd
^ permalink raw reply
* [PATCH 1/4] bindings: net: stmmac: correct note about TSO
From: Niklas Cassel @ 2016-11-23 14:24 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, David S. Miller, Giuseppe CAVALLARO,
Alexandre TORGUE, Phil Reid, Eric Engestrom
Cc: Niklas Cassel, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
From: Niklas Cassel <niklas.cassel-VrBV9hrLPhE@public.gmane.org>
snps,tso was previously placed under AXI BUS Mode parameters,
suggesting that the property should be in the stmmac-axi-config node.
TSO (TCP Segmentation Offloading) has nothing to do with AXI BUS Mode
parameters, and the parser actually expects it to be in the root node,
not in the stmmac-axi-config.
Also added a note about snps,tso only being available on GMAC4 and newer.
Signed-off-by: Niklas Cassel <niklas.cassel-VrBV9hrLPhE@public.gmane.org>
---
Documentation/devicetree/bindings/net/stmmac.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
index 41b49e6075f5..b95ff998ba73 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -1,7 +1,7 @@
* STMicroelectronics 10/100/1000 Ethernet driver (GMAC)
Required properties:
-- compatible: Should be "snps,dwmac-<ip_version>" "snps,dwmac"
+- compatible: Should be "snps,dwmac-<ip_version>", "snps,dwmac"
For backwards compatibility: "st,spear600-gmac" is also supported.
- reg: Address and length of the register set for the device
- interrupt-parent: Should be the phandle for the interrupt controller
@@ -50,6 +50,8 @@ Optional properties:
- snps,ps-speed: port selection speed that can be passed to the core when
PCS is supported. For example, this is used in case of SGMII
and MAC2MAC connection.
+- snps,tso: this enables the TSO feature otherwise it will be managed by
+ MAC HW capability register. Only for GMAC4 and newer.
- AXI BUS Mode parameters: below the list of all the parameters to program the
AXI register inside the DMA module:
- snps,lpi_en: enable Low Power Interface
@@ -62,8 +64,6 @@ Optional properties:
- snps,fb: fixed-burst
- snps,mb: mixed-burst
- snps,rb: rebuild INCRx Burst
- - snps,tso: this enables the TSO feature otherwise it will be managed by
- MAC HW capability register.
- mdio: with compatible = "snps,dwmac-mdio", create and register mdio bus.
Examples:
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 2/4] ARM: dts: exynos: specify snps,dwmac in compatible string for gmac
From: Niklas Cassel @ 2016-11-23 14:24 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Russell King, Kukjin Kim,
Krzysztof Kozlowski, Javier Martinez Canillas
Cc: Niklas Cassel, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel
From: Niklas Cassel <niklas.cassel@axis.com>
devicetree binding for stmmac states:
- compatible: Should be "snps,dwmac-<ip_version>", "snps,dwmac"
For backwards compatibility: "st,spear600-gmac" is also supported.
No functional change intended.
Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
arch/arm/boot/dts/exynos5440.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi
index 97c9f0e38526..2a2e570bbee6 100644
--- a/arch/arm/boot/dts/exynos5440.dtsi
+++ b/arch/arm/boot/dts/exynos5440.dtsi
@@ -197,7 +197,7 @@
};
gmac: ethernet@00230000 {
- compatible = "snps,dwmac-3.70a";
+ compatible = "snps,dwmac-3.70a", "snps,dwmac";
reg = <0x00230000 0x8000>;
interrupt-parent = <&gic>;
interrupts = <GIC_SPI 31 4>;
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] v4l: async: make v4l2 coexists with devicetree nodes in a dt overlay
From: Javier Martinez Canillas @ 2016-11-23 14:25 UTC (permalink / raw)
To: Javi Merino, linux-media
Cc: linux-kernel, devicetree, Pantelis Antoniou,
Mauro Carvalho Chehab, Sakari Ailus
In-Reply-To: <1479895797-7946-1-git-send-email-javi.merino@kernel.org>
Hello Javi,
On 11/23/2016 07:09 AM, Javi Merino wrote:
> In asd's configured with V4L2_ASYNC_MATCH_OF, if the v4l2 subdev is in
> a devicetree overlay, its of_node pointer will be different each time
> the overlay is applied. We are not interested in matching the
> pointer, what we want to match is that the path is the one we are
> expecting. Change to use of_node_cmp() so that we continue matching
> after the overlay has been removed and reapplied.
>
I'm still not that familiar with DT overlays (and I guess others aren't
either) so I think that including an example of a base tree and overlay
DTS where this is an issue, could make things more clear in the commit.
IIUC, it should be something like this?
-- base tree --
&i2c1 {
camera: camera@10 {
reg = <0x10>;
port {
cam_ep: endpoint {
...
};
};
};
};
&media_bridge {
...
ports {
port@0 {
reg = <0>;
ep: endpoint {
remote-endpoint = <&cam_ep>;
};
};
};
};
-- overlay --
/plugin/;
/ {
...
fragment@0 {
target = <&camera>;
__overlay__ {
compatible = "foo,bar";
...
port {
cam_ep: endpoint {
...
};
};
};
}
}
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Javi Merino <javi.merino@kernel.org>
> ---
> Hi,
>
> I feel it is a bit of a hack, but I couldn't think of anything better.
> I'm ccing devicetree@ and Pantelis because there may be a simpler
> solution.
>
I also couldn't think a better way to do this, since IIUC the node's name is
the only thing that doesn't change, and is available at the time the bridge
driver calls v4l2_async_notifier_register() when parsing the base tree.
> drivers/media/v4l2-core/v4l2-async.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 5bada20..d33a17c 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -42,7 +42,8 @@ static bool match_devname(struct v4l2_subdev *sd,
>
> static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> {
> - return sd->of_node == asd->match.of.node;
> + return !of_node_cmp(of_node_full_name(sd->of_node),
> + of_node_full_name(asd->match.of.node));
> }
>
> static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* Re: [PATCH 3/3] ARM: dts: sunxi: enable SDIO Wi-Fi on Orange Pi Zero
From: Chen-Yu Tsai @ 2016-11-23 14:25 UTC (permalink / raw)
To: Maxime Ripard
Cc: Mark Rutland, devicetree, Vishnu Patekar, Arnd Bergmann,
Jonathan Corbet, Andre Przywara, linux-doc@vger.kernel.org,
Russell King, linux-kernel, Hans de Goede, Chen-Yu Tsai,
Icenowy Zheng, linux-arm-kernel
In-Reply-To: <20161123075950.fjtplylunwale6j4@lukather>
On Wed, Nov 23, 2016 at 3:59 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Tue, Nov 22, 2016 at 12:24:21AM +0800, Icenowy Zheng wrote:
>> There's a Allwinner's XR819 SDIO Wi-Fi module soldered on the board of
>> Orange Pi Zero, which used a dedicated regulator to power.
>>
>> Add the device tree node of the regulator, the enable gpio (with
>> mmc-pwrseq) and the sdio controller.
>>
>> There's a out-of-tree driver tested to work with this device tree.
>>
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>> ---
>> New patch in the patchset, since a out-of-tree working xradio driver is done.
>>
>> If there is any problem in this patch, it can be omitted.
>
> No particular problem with this one, however it can and should be
> merged with the previous one.
>
> Minor comments below though.
>
>>
>> arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts | 42 ++++++++++++++++++++++++
>> 1 file changed, 42 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>> index b428e47..39cac26 100644
>> --- a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>> +++ b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>> @@ -79,6 +79,24 @@
>> gpios = <&pio 0 17 GPIO_ACTIVE_HIGH>;
>> };
>> };
>> +
>> + reg_vcc_wifi: reg_vcc_wifi {
>> + compatible = "regulator-fixed";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&vcc_wifi_pin_opi0>;
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-name = "vcc-wifi";
>> + enable-active-high;
>> + gpio = <&pio 0 20 GPIO_ACTIVE_HIGH>;
>> + };
>> +
>> + wifi_pwrseq: wifi_pwrseq {
>> + compatible = "mmc-pwrseq-simple";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&wifi_pwrseq_pin_opi0>;
>> + reset-gpios = <&r_pio 0 7 GPIO_ACTIVE_LOW>;
>> + };
>> };
>>
>> &ehci1 {
>> @@ -95,6 +113,20 @@
>> status = "okay";
>> };
>>
>> +&mmc1 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&mmc1_pins_a>;
>> + vmmc-supply = <®_vcc_wifi>;
>> + mmc-pwrseq = <&wifi_pwrseq>;
>> + bus-width = <4>;
>> + non-removable;
>> + status = "okay";
>> +};
>> +
>> +&mmc1_pins_a {
>> + allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>
> This should be bias-pull-up.
IIRC I already added this for _all_ existing mmc pinmux settings?
>
>> +};
>> +
>> &ohci1 {
>> status = "okay";
>> };
>> @@ -104,6 +136,11 @@
>> pins = "PA17";
>> function = "gpio_out";
>> };
>> +
>> + vcc_wifi_pin_opi0: vcc_wifi_pin@0 {
>> + allwinner,pins = "PA20";
>
> This should be pins
>
>> + allwinner,function = "gpio_out";
>
> This should be function
>
>> + };
>> };
>>
>> &r_pio {
>> @@ -111,6 +148,11 @@
>> pins = "PL10";
>> function = "gpio_out";
>> };
>> +
>> + wifi_pwrseq_pin_opi0: wifi_pwrseq_pin@0 {
>> + allwinner,pins = "PL7";
>> + allwinner,function = "gpio_out";
>
> And same thing here.
Might we do away with the pinmux for gpio pins tradition?
Recent patches I've sent all omit them.
ChenYu
^ permalink raw reply
* Re: [PATCH 3/3] ARM: dts: sunxi: enable SDIO Wi-Fi on Orange Pi Zero
From: Hans de Goede @ 2016-11-23 14:29 UTC (permalink / raw)
To: Chen-Yu Tsai, Maxime Ripard
Cc: Mark Rutland, devicetree, Vishnu Patekar, Arnd Bergmann,
Jonathan Corbet, Andre Przywara, linux-doc@vger.kernel.org,
Russell King, linux-kernel, Icenowy Zheng, linux-arm-kernel
In-Reply-To: <CAGb2v64taF9x9MDYW+KUEEUUoSx0bF68QNc7uZXQoNrsozMGtg@mail.gmail.com>
Hi,
On 23-11-16 15:25, Chen-Yu Tsai wrote:
> On Wed, Nov 23, 2016 at 3:59 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> Hi,
>>
>> On Tue, Nov 22, 2016 at 12:24:21AM +0800, Icenowy Zheng wrote:
>>> There's a Allwinner's XR819 SDIO Wi-Fi module soldered on the board of
>>> Orange Pi Zero, which used a dedicated regulator to power.
>>>
>>> Add the device tree node of the regulator, the enable gpio (with
>>> mmc-pwrseq) and the sdio controller.
>>>
>>> There's a out-of-tree driver tested to work with this device tree.
>>>
>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>>> ---
>>> New patch in the patchset, since a out-of-tree working xradio driver is done.
>>>
>>> If there is any problem in this patch, it can be omitted.
>>
>> No particular problem with this one, however it can and should be
>> merged with the previous one.
>>
>> Minor comments below though.
>>
>>>
>>> arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts | 42 ++++++++++++++++++++++++
>>> 1 file changed, 42 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>>> index b428e47..39cac26 100644
>>> --- a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>>> +++ b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>>> @@ -79,6 +79,24 @@
>>> gpios = <&pio 0 17 GPIO_ACTIVE_HIGH>;
>>> };
>>> };
>>> +
>>> + reg_vcc_wifi: reg_vcc_wifi {
>>> + compatible = "regulator-fixed";
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&vcc_wifi_pin_opi0>;
>>> + regulator-min-microvolt = <3300000>;
>>> + regulator-max-microvolt = <3300000>;
>>> + regulator-name = "vcc-wifi";
>>> + enable-active-high;
>>> + gpio = <&pio 0 20 GPIO_ACTIVE_HIGH>;
>>> + };
>>> +
>>> + wifi_pwrseq: wifi_pwrseq {
>>> + compatible = "mmc-pwrseq-simple";
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&wifi_pwrseq_pin_opi0>;
>>> + reset-gpios = <&r_pio 0 7 GPIO_ACTIVE_LOW>;
>>> + };
>>> };
>>>
>>> &ehci1 {
>>> @@ -95,6 +113,20 @@
>>> status = "okay";
>>> };
>>>
>>> +&mmc1 {
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&mmc1_pins_a>;
>>> + vmmc-supply = <®_vcc_wifi>;
>>> + mmc-pwrseq = <&wifi_pwrseq>;
>>> + bus-width = <4>;
>>> + non-removable;
>>> + status = "okay";
>>> +};
>>> +
>>> +&mmc1_pins_a {
>>> + allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>>
>> This should be bias-pull-up.
>
> IIRC I already added this for _all_ existing mmc pinmux settings?
>
>>
>>> +};
>>> +
>>> &ohci1 {
>>> status = "okay";
>>> };
>>> @@ -104,6 +136,11 @@
>>> pins = "PA17";
>>> function = "gpio_out";
>>> };
>>> +
>>> + vcc_wifi_pin_opi0: vcc_wifi_pin@0 {
>>> + allwinner,pins = "PA20";
>>
>> This should be pins
>>
>>> + allwinner,function = "gpio_out";
>>
>> This should be function
>>
>>> + };
>>> };
>>>
>>> &r_pio {
>>> @@ -111,6 +148,11 @@
>>> pins = "PL10";
>>> function = "gpio_out";
>>> };
>>> +
>>> + wifi_pwrseq_pin_opi0: wifi_pwrseq_pin@0 {
>>> + allwinner,pins = "PL7";
>>> + allwinner,function = "gpio_out";
>>
>> And same thing here.
>
> Might we do away with the pinmux for gpio pins tradition?
> Recent patches I've sent all omit them.
I'm in favor of doing away with them, except there were
we need to configure bias / strength.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Simon Horman @ 2016-11-23 14:29 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Mark Rutland, devicetree, Chris Paterson, Magnus Damm, linux-can,
linux-renesas-soc, Rob Herring, linux-arm-kernel,
Ramesh Shanmugasundaram, Wolfgang Grandegger
In-Reply-To: <f358442c-8373-328c-1270-709498c133f5@pengutronix.de>
On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
> On 11/23/2016 01:14 PM, Chris Paterson wrote:
> > This patch series adds CAN and CAN FD support to the r8a7796.
> >
> > Based on renesas-devel-20161122-v4.9-rc6.
> >
> > Chris Paterson (3):
> > arm64: dts: r8a7796: Add CAN external clock support
> > arm64: dts: r8a7796: Add CAN support
> > arm64: dts: r8a7796: Add CAN FD support
> >
> > .../devicetree/bindings/net/can/rcar_can.txt | 12 +++--
> > .../devicetree/bindings/net/can/rcar_canfd.txt | 12 +++--
> > arch/arm64/boot/dts/renesas/r8a7796.dtsi | 61 ++++++++++++++++++++++
> > 3 files changed, 75 insertions(+), 10 deletions(-)
>
> For all three:
>
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
>
> Who takes this series?
I would like to see these patches split up so that the
.../devicetree/bindings/ portions can go through you whole
the arch/arm64/boot/dts/renesas/ portions go thorugh my renesas tree.
Regarding the arch/arm64/boot/dts/renesas/ portion, I would like
some consideration given to what effect enabling memory above 4Gb
(64bit addressing) would have.
^ permalink raw reply
* Re: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Marc Kleine-Budde @ 2016-11-23 14:31 UTC (permalink / raw)
To: Simon Horman
Cc: Mark Rutland, devicetree, Chris Paterson, Magnus Damm, linux-can,
linux-renesas-soc, Rob Herring, linux-arm-kernel,
Ramesh Shanmugasundaram, Wolfgang Grandegger
In-Reply-To: <20161123142938.GF9057@verge.net.au>
[-- Attachment #1.1.1: Type: text/plain, Size: 1519 bytes --]
On 11/23/2016 03:29 PM, Simon Horman wrote:
> On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
>> On 11/23/2016 01:14 PM, Chris Paterson wrote:
>>> This patch series adds CAN and CAN FD support to the r8a7796.
>>>
>>> Based on renesas-devel-20161122-v4.9-rc6.
>>>
>>> Chris Paterson (3):
>>> arm64: dts: r8a7796: Add CAN external clock support
>>> arm64: dts: r8a7796: Add CAN support
>>> arm64: dts: r8a7796: Add CAN FD support
>>>
>>> .../devicetree/bindings/net/can/rcar_can.txt | 12 +++--
>>> .../devicetree/bindings/net/can/rcar_canfd.txt | 12 +++--
>>> arch/arm64/boot/dts/renesas/r8a7796.dtsi | 61 ++++++++++++++++++++++
>>> 3 files changed, 75 insertions(+), 10 deletions(-)
>>
>> For all three:
>>
>> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
>>
>> Who takes this series?
>
> I would like to see these patches split up so that the
> .../devicetree/bindings/ portions can go through you whole
> the arch/arm64/boot/dts/renesas/ portions go thorugh my renesas tree.
Ok
> Regarding the arch/arm64/boot/dts/renesas/ portion, I would like
> some consideration given to what effect enabling memory above 4Gb
> (64bit addressing) would have.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 3/5] i2c: designware: Add slave definitions
From: Luis Oliveira @ 2016-11-23 14:36 UTC (permalink / raw)
To: Rob Herring, Andy Shevchenko
Cc: Luis Oliveira, wsa, mark.rutland, jarkko.nikula, mika.westerberg,
linux-i2c, devicetree, linux-kernel, Ramiro.Oliveira, Joao.Pinto,
CARLOS.PALMINHA
In-Reply-To: <20161118170155.chrdpguohgl6vo5f@rob-hp-laptop>
OK, I will create a "mode" string property in the devicetree that can be
"master" or "slave".
Thank you all,
Luis
On 18-Nov-16 17:01, Rob Herring wrote:
> On Fri, Nov 18, 2016 at 02:35:52PM +0200, Andy Shevchenko wrote:
>> On Fri, 2016-11-18 at 11:19 +0000, Luis Oliveira wrote:
>>> - Add slave defintitions to i2c-designware-core
>>> - Changes in Kconfig to auto-enable I2C_SLAVE when compiling the
>>> modules
>>> - Add compatible string to designware-core.txt explaining the
>>> devicetree bindings
>>>
>>
>>> --- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
>>> @@ -2,7 +2,9 @@
>>>
>>> Required properties :
>>>
>>> - - compatible : should be "snps,designware-i2c"
>>> + - compatible : should be:
>>> + - "snps,designware-i2c" to setup the hardware block as I2C master.
>>> + - "snps,designware-i2c-slave" to setup the hardware block as I2C
>>> slave.
>> Not sure about this one.
>>
>> Compatible string is more generic than list of modes. Basically you have
>> to add a property which selects mode.
> Yes, agreed. And come up with a common property.
>
>> DT people's ACK is a must for this change.
>>
>>
>> --- a/drivers/i2c/busses/Kconfig
>>> +++ b/drivers/i2c/busses/Kconfig
>>> @@ -470,6 +470,7 @@ config I2C_DESIGNWARE_CORE
>>> config I2C_DESIGNWARE_PLATFORM
>>> tristate "Synopsys DesignWare Platform"
>>> select I2C_DESIGNWARE_CORE
>>> + select I2C_SLAVE
>>>
>> Common rule, generic dependencies usually go first
>>
>> select I2C_SLAVE
>> select I2C_DESIGNWARE_CORE
>>
>> --
>> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Intel Finland Oy
--
Best regards,
Luis
^ permalink raw reply
* [PATCH v3] iommu/ipmmu-vmsa: Add r8a7796 DT binding
From: Magnus Damm @ 2016-11-23 14:40 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: mark.rutland-5wv7dgnIgG8,
laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ, Magnus Damm,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ
From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
Update the IPMMU DT binding documentation to include the r8a7796 compat
string for R-Car M3-W.
Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Simon Horman <horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
---
This particular patch seems ready to merge IMO. How to proceed?
Changes since V2:
- Added Acked-by from Rob Herring and Simon Horman - thanks!
Changes since V1:
- Added Acked-by from Laurent - thanks!
Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt | 1 +
1 file changed, 1 insertion(+)
--- 0001/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
+++ work/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt 2016-06-06 11:27:37.560607110 +0900
@@ -16,6 +16,7 @@ Required Properties:
- "renesas,ipmmu-r8a7793" for the R8A7793 (R-Car M2-N) IPMMU.
- "renesas,ipmmu-r8a7794" for the R8A7794 (R-Car E2) IPMMU.
- "renesas,ipmmu-r8a7795" for the R8A7795 (R-Car H3) IPMMU.
+ - "renesas,ipmmu-r8a7796" for the R8A7796 (R-Car M3-W) IPMMU.
- "renesas,ipmmu-vmsa" for generic R-Car Gen2 VMSA-compatible IPMMU.
- reg: Base address and size of the IPMMU registers.
^ permalink raw reply
* Re: [PATCH RFC] ARM: dts: add support for Turris Omnia
From: Andrew Lunn @ 2016-11-23 14:59 UTC (permalink / raw)
To: tomas.hlavacek
Cc: Mark Rutland, marex, Jason Cooper, Uwe Kleine-König,
devicetree, Rob Herring, Gregory Clement, linux-arm-kernel,
Sebastian Hesselbarth
In-Reply-To: <1479851991.26813.2@smtp.gmail.com>
> >CZ11NIC12 is indicated on my board.
>
> :-( Well, this board version has wrongly matched length of some
> differential pairs, IRQ from 88E1514 is connected differently, there
> are slight differences in power supplies and (if I am not mistaken)
> something changed in RTC support circuitry. It looks like a huge
> mistake on our side.
Hi Tomas
Would these problems also explain why the Ethernet links to the switch
don't work? Maybe the differential pairs?
> It seems that libphy is probed before pca9538 and we end up with:
> [ 4.217550] libphy: orion_mdio_bus: probed
> [ 4.221777] irq: no irq domain found for
> /soc/internal-regs/i2c@11000/i2cmux@70/i2c@7/gpio@71 !
>
> Any clue where to look in order to defer probing libphy or at least
> orion_mdio_bus?
I think there is a known phylib problem here. Somewhere in the call
chain there is a void function, so the EPROBE_DEFFER gets
discarded. But i could be remembering this wrongly.
Andrew
^ permalink raw reply
* Re: [PATCH] mfd: cpcap: Add minimal support
From: Tony Lindgren @ 2016-11-23 15:03 UTC (permalink / raw)
To: Rob Herring
Cc: Lee Jones, Samuel Ortiz, linux-kernel, linux-omap, devicetree,
Marcel Partap, Mark Rutland, Michael Scott
In-Reply-To: <20161121163439.22ztjxnq535lo6dd@rob-hp-laptop>
* Rob Herring <robh@kernel.org> [161121 08:34]:
> On Mon, Nov 21, 2016 at 11:45:58AM +0000, Lee Jones wrote:
> > On Fri, 18 Nov 2016, Tony Lindgren wrote:
> > > +Example:
> > > +
> > > +&mcspi1 {
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> > > + ranges;
> > > + cpcap: pmic@0 {
> > > + compatible = "motorola,cpcap", "st,6556002";
> > > + reg = <0 0>; /* cs0, size 0 */
> >
> > Is this really correct?
>
> No, SPI devices are 1 cell and there shouldn't be a ranges prop.
>
> >
> > How can ranges have a size of 0x8000 and this 0?
>
> reg here doesn't affect ranges and address translation.
>
> Perhaps this is trying to make address translation work, but if that
> does, it is by chance. Children of pmic addresses in the range of
> 0-0x8000 would get translated to "cpu address" 0-0x8000 as long as the
> DT has empty ranges up to the root. If the parent bus (i.e. SoC bus) has
> any base address, then that is going to get added which would not be
> good.
Yes I was thinking we can just get the register offset from the
cpcap base from dts for children. So a range from 0-0x8000.
Rob, do you have any better suggestions for doing that?
Anyways, the children can just rely on regmap too, so I'll just
drop the ranges here. If we ever come up with suitable ranges for
cases like this we can change it later.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH] v4l: async: make v4l2 coexists with devicetree nodes in a dt overlay
From: Sakari Ailus @ 2016-11-23 15:10 UTC (permalink / raw)
To: Javi Merino
Cc: linux-media, linux-kernel, devicetree, Pantelis Antoniou,
Mauro Carvalho Chehab, Javier Martinez Canillas, Sakari Ailus
In-Reply-To: <1479895797-7946-1-git-send-email-javi.merino@kernel.org>
Hi Javi,
On Wed, Nov 23, 2016 at 10:09:57AM +0000, Javi Merino wrote:
> In asd's configured with V4L2_ASYNC_MATCH_OF, if the v4l2 subdev is in
> a devicetree overlay, its of_node pointer will be different each time
> the overlay is applied. We are not interested in matching the
> pointer, what we want to match is that the path is the one we are
> expecting. Change to use of_node_cmp() so that we continue matching
> after the overlay has been removed and reapplied.
>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Javi Merino <javi.merino@kernel.org>
> ---
> Hi,
>
> I feel it is a bit of a hack, but I couldn't think of anything better.
> I'm ccing devicetree@ and Pantelis because there may be a simpler
> solution.
First I have to admit that I'm not an expert when it comes to DT overlays.
That said, my understanding is that the sub-device and the async sub-device
are supposed to point to the exactly same DT node. I wonder if there's
actually anything wrong in the current code.
If the overlay has changed between probing the driver for the async notifier
and the async sub-device, there should be no match here, should there? The
two nodes actually point to a node in a different overlay in that case.
>
> drivers/media/v4l2-core/v4l2-async.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 5bada20..d33a17c 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -42,7 +42,8 @@ static bool match_devname(struct v4l2_subdev *sd,
>
> static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> {
> - return sd->of_node == asd->match.of.node;
> + return !of_node_cmp(of_node_full_name(sd->of_node),
> + of_node_full_name(asd->match.of.node));
> }
>
> static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply
* RE: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Gabriele Paoloni @ 2016-11-23 15:22 UTC (permalink / raw)
To: Arnd Bergmann
Cc: mark.rutland@arm.com, catalin.marinas@arm.com,
linux-pci@vger.kernel.org, liviu.dudau@arm.com, Linuxarm,
lorenzo.pieralisi@arm.com, xuwei (O), Jason Gunthorpe,
T homas Petazzoni, linux-serial@vger.kernel.org,
benh@kernel.crashing.org, devicetree@vger.kernel.org,
minyard@acm.org, will.deacon@arm.com, John Garry, olof@lixom.net,
robh+dt@kernel.org, bhelgaas@go og le.com,
"kantyzc@163.com" <kan>
In-Reply-To: <2359248.XjnRfPbj1B@wuerfel>
Hi Arnd
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 23 November 2016 14:16
> To: Gabriele Paoloni
> Cc: linux-arm-kernel@lists.infradead.org; mark.rutland@arm.com;
> benh@kernel.crashing.org; catalin.marinas@arm.com; liviu.dudau@arm.com;
> Linuxarm; lorenzo.pieralisi@arm.com; xuwei (O); Jason Gunthorpe; linux-
> serial@vger.kernel.org; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; minyard@acm.org; will.deacon@arm.com; John
> Garry; zourongrong@gmail.com; robh+dt@kernel.org; bhelgaas@go og
> le.com; kantyzc@163.com; zhichang.yuan02@gmail.com; T homas Petazzoni;
> linux-kernel@vger.kernel.org; Yuanzhichang; olof@lixom.net
> Subject: Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on
> Hip06
>
> On Friday, November 18, 2016 5:03:11 PM CET Gabriele Paoloni wrote:
> > > On Friday, November 18, 2016 4:18:07 PM CET Gabriele Paoloni wrote:
> > > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > > > On Friday, November 18, 2016 12:53:08 PM CET Gabriele Paoloni
> > > wrote:
> > > > > For the ISA/LPC spaces there are only 4k of addresses, they
> > > > > the bus addresses always overlap, but we can trivially
> > > > > figure out the bus address from Linux I/O port number
> > > > > by subtracting the start of the range.
> > > >
> > > > Are you saying that our LPC controller should specify a
> > > > range property to map bus addresses into a cpu address range?
> > >
> > > No. There is not CPU address associated with it, because it's
> > > not memory mapped.
> > >
> > > Instead, we need to associate a bus address with a logical
> > > Linux port number, both in of_address_to_resource and
> > > in inb()/outb().
> >
> > I think this is effectively what we are doing so far with patch 2/3.
> > The problem with this patch is that we are carving out a "forbidden"
> > IO tokens range that goes from 0 to PCIBIOS_MIN_IO.
> >
> > I think that the proper solution would be to have the LPC driver to
> > set the carveout threshold used in pci_register_io_range(),
> > pci_pio_to_address(), pci_address_to_pio(), but this would impose
> > a probe dependency on the LPC itself that should be probed before
> > the PCI controller (or before any other devices calling these
> > functions...)
>
> Why do you think the order matters? My point was that we should
> be able to register any region of logical port numbers for any
> bus here.
Maybe I have not followed well so let's roll back to your previous
comment...
"we need to associate a bus address with a logical Linux port number,
both in of_address_to_resource and in inb()/outb()"
Actually of_address_to_resource() returns the port number to used
in inb/outb(); inb() and outb() add the port number to PCI_IOBASE
to rd/wr to the right virtual address.
Our LPC cannot operate on the virtual address and it operates on
a bus address range that for LPC is also equal to the cpu address
range and goes from 0 to 0x1000.
Now as I understand it is risky and not appropriate to reserve
the logical port numbers from 0 to 0x1000 or to whatever other
upper bound because existing systems may rely on these port numbers
retrieved by __of_address_to_resource().
In this scenario I think the best thing to do would be
in the probe function of the LPC driver:
1) call pci_register_io_range() passing [0, 0x1000] (that is the
range for LPC)
2) retrieve the logical port numbers associated to the LPC range
by calling pci_address_to_pio() for 0 and 0x1000 and assign
them to extio_ops_node->start and extio_ops_node->end
3) implement the LPC accessors to operate on the logical ports
associated to the LPC range (in practice in the accessors
implementation we will call pci_pio_to_address to retrieve
the cpu address to operate on)
What do you think?
Thanks
Gab
>
>
> > > > > > To be honest with you I would keep things simple for this
> > > > > > LPC and introduce more complex reworks later if more devices
> > > > > > need to be introduced.
> > > > > >
> > > > > > What if we stick on a single domain now where we introduce a
> > > > > > reserved threshold for the IO space (say INDIRECT_MAX_IO).
> > > > >
> > > > > I said having a single domain is fine, but I still don't
> > > > > like the idea of reserving low port numbers for this hack,
> > > > > it would mean that the numbers change for everyone else.
> > > >
> > > > I don't get this much...I/O tokens that are passed to the I/O
> > > > accessors are not fixed anyway and they vary depending on the
> order
> > > > of adding ranges to io_range_list...so I don't see a big issue
> > > > with this...
> > >
> > > On machines with a legacy devices behind the PCI bridge,
> > > there may still be a reason to have the low I/O port range
> > > reserved for the primary bus, e.g. to get a VGA text console
> > > to work.
> > >
> > > On powerpc, this is called the "primary" PCI host, i.e. the
> > > only one that is allowed to have an ISA bridge.
> >
> > Yes but
> > 1) isn't the PCI controller range property that defines how IO bus
> address
> > map into physical CPU addresses?
>
> Correct, but the DT knows nothing about logical port numbers in Linux.
>
> > 2) How can you guarantee that the cpu range associated with this
> > IO bus range is the first to be registered in
> pci_register_io_range()?
> > ( i.e. are you saying that they are just relying on the fact that
> it is the
> > only IO range in the system and by chance the IO tokens and
> corresponding
> > bus addresses are the same? )
>
> To clarify: the special properties of having the first 0x1000 logical
> port numbers go to a particular physical bus are very obscure. I think
> it's more important to not change the behavior for existing systems
> that might rely on it than for new systems that have no such legacy.
>
> The ipmi and uart drivers in particular will get the port numbers
> filled
> in their platform device from the DT bus scanning, so they don't care
> at all about having the same numeric value for port numbers on the bus
> and logical numbers, but other drivers might rely on particular ports
> to be mapped on a specific PCI host, especially when those drivers
> are used only on systems that don't have more than one PCI domain.
>
> Arnd
^ permalink raw reply
* Re: [PATCH] mfd: cpcap: Add minimal support
From: Tony Lindgren @ 2016-11-23 15:26 UTC (permalink / raw)
To: Lee Jones
Cc: Samuel Ortiz, linux-kernel, linux-omap, devicetree, Marcel Partap,
Mark Rutland, Michael Scott, Rob Herring
In-Reply-To: <20161121114558.GJ32509@dell>
* Lee Jones <lee.jones@linaro.org> [161121 03:43]:
> On Fri, 18 Nov 2016, Tony Lindgren wrote:
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -97,6 +97,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
> > obj-$(CONFIG_MFD_CORE) += mfd-core.o
> >
> > obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
> > +obj-$(CONFIG_MFD_CPCAP) += cpcap.o
>
> Who is the manufacturer?
Hmm that I don't know. There seems to be both ST and TI versions
of this chip manufactured for Motorola. So my guess is that it
should be Motorola unless there's some similar catalog part
available from ST used by others. If anybody has more info
on this please let me know :)
> > + cpcap->vendor = (val >> 6) & 0x0007;
> > + cpcap->revision = ((val >> 3) & 0x0007) | ((val << 3) & 0x0038);
>
> Lots of magic numbers here. I suggest you define them.
I'll check if some earlier code has these defined. Otherwise I'll
just add a comment on the lack of available documentation.
> > + error = cpcap_init_irq_bank(cpcap, 0, 0, 16);
>
> 'ret' is more traditional.
FYI error seems to be preferred over ret as it's meaning is
clear, git grep "error =" drivers/input for example.
I can of course change it if you prefer ret over error.
> > + error = cpcap_init_irq_bank(cpcap, 2, 32, 64);
> > + if (error)
> > + return error;
>
> I don't think I've seen this method of adding bulk IRQ chips before.
> Isn't there a cleaner or generic way to do this?
I'll check.
...
> > +#define CPCAP_REG_LDEB 0x1270 /* LMR Debounce Settings */
> > +#define CPCAP_REG_LGDET 0x1274 /* LMR GCAI Detach Detect */
> > +#define CPCAP_REG_LMISC 0x1278 /* LMR Misc Bits */
> > +#define CPCAP_REG_LMACE 0x127c /* LMR Mace IC Support */
> > +
> > +#define CPCAP_REG_TEST 0x7c00 /* Test */
> > +
> > +#define CPCAP_REG_ST_TEST1 0x7d08 /* ST Test1 */
> > +
> > +#define CPCAP_REG_ST_TEST2 0x7d18 /* ST Test2 */
>
> It would be nice to line up the entire file. #OCD
Hmm care to clarify what you mean here? I think it's lined up with
tabs to line up. I left empty lines where the registers are not
contiguous. What does #OCD mean, Obsessive Compulsive Disorder over
header files maybe? :)
Anywys thanks for the review, the rest of the comments I will just
fix and repost.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT
From: Sakari Ailus @ 2016-11-23 15:37 UTC (permalink / raw)
To: Kevin Hilman
Cc: linux-media-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil,
devicetree-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori, Axel Haslam,
Bartosz Gołaszewski, Alexandre Bailon, David Lechner
In-Reply-To: <20161122155244.802-4-khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Hi Kevin,
On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
> Allow getting of subdevs from DT ports and endpoints.
>
> The _get_pdata() function was larely inspired by (i.e. stolen from)
vpif_capture_get_pdata and "largely"?
> am437x-vpfe.c
>
> Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
> drivers/media/platform/davinci/vpif_capture.c | 130 +++++++++++++++++++++++++-
> include/media/davinci/vpif_types.h | 9 +-
> 2 files changed, 133 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> index 94ee6cf03f02..47a4699157e7 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -26,6 +26,8 @@
> #include <linux/slab.h>
>
> #include <media/v4l2-ioctl.h>
> +#include <media/v4l2-of.h>
> +#include <media/i2c/tvp514x.h>
Do you need this header?
>
> #include "vpif.h"
> #include "vpif_capture.h"
> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
>
> vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>
> + if (!chan_cfg)
> + return -1;
> + if (input_index >= chan_cfg->input_count)
> + return -1;
> subdev_name = chan_cfg->inputs[input_index].subdev_name;
> if (subdev_name == NULL)
> return -1;
> @@ -657,7 +663,7 @@ static int vpif_input_to_subdev(
> /* loop through the sub device list to get the sub device info */
> for (i = 0; i < vpif_cfg->subdev_count; i++) {
> subdev_info = &vpif_cfg->subdev_info[i];
> - if (!strcmp(subdev_info->name, subdev_name))
> + if (subdev_info && !strcmp(subdev_info->name, subdev_name))
> return i;
> }
> return -1;
> @@ -1327,6 +1333,21 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
> {
> int i;
>
> + for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
> + struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
> + const struct device_node *node = _asd->match.of.node;
> +
> + if (node == subdev->of_node) {
> + vpif_obj.sd[i] = subdev;
> + vpif_obj.config->chan_config->inputs[i].subdev_name =
> + (char *)subdev->of_node->full_name;
> + vpif_dbg(2, debug,
> + "%s: setting input %d subdev_name = %s\n",
> + __func__, i, subdev->of_node->full_name);
> + return 0;
> + }
> + }
> +
> for (i = 0; i < vpif_obj.config->subdev_count; i++)
> if (!strcmp(vpif_obj.config->subdev_info[i].name,
> subdev->name)) {
> @@ -1422,6 +1443,110 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
> return vpif_probe_complete();
> }
>
> +static struct vpif_capture_config *
> +vpif_capture_get_pdata(struct platform_device *pdev)
> +{
> + struct device_node *endpoint = NULL;
> + struct v4l2_of_endpoint bus_cfg;
> + struct vpif_capture_config *pdata;
> + struct vpif_subdev_info *sdinfo;
> + struct vpif_capture_chan_config *chan;
> + unsigned int i;
> +
> + dev_dbg(&pdev->dev, "vpif_get_pdata\n");
> +
> + if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
> + return pdev->dev.platform_data;
> +
> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return NULL;
> + pdata->subdev_info =
> + devm_kzalloc(&pdev->dev, sizeof(*pdata->subdev_info) *
> + VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
> +
> + if (!pdata->subdev_info)
> + return NULL;
> + dev_dbg(&pdev->dev, "%s\n", __func__);
> +
> + for (i = 0; ; i++) {
> + struct device_node *rem;
> + unsigned int flags;
> + int err;
> +
> + endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
> + endpoint);
> + if (!endpoint)
> + break;
> +
> + sdinfo = &pdata->subdev_info[i];
subdev_info[] has got VPIF_CAPTURE_MAX_CHANNELS entries only.
> + chan = &pdata->chan_config[i];
> + chan->inputs = devm_kzalloc(&pdev->dev,
> + sizeof(*chan->inputs) *
> + VPIF_DISPLAY_MAX_CHANNELS,
> + GFP_KERNEL);
> +
> + chan->input_count++;
> + chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
I wonder what's the purpose of using index i on this array as well.
If you use that to access a corresponding entry in a different array, I'd
just create a struct that contains the port configuration and the async
sub-device. The omap3isp driver does that, for instance; see
isp_of_parse_nodes() in drivers/media/platform/omap3isp/isp.c if you're
interested. Up to you.
> + chan->inputs[i].input.std = V4L2_STD_ALL;
> + chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
> +
> + /* FIXME: need a new property? ch0:composite ch1: s-video */
> + if (i == 0)
Can you assume that the first endopoint has got a particular kind of input?
What if it's not connected?
If this is a different physical port (not in the meaning another) in the
device, I'd use the reg property for this. Please see
Documentation/devicetree/bindings/media/video-interfaces.txt .
> + chan->inputs[i].input_route = INPUT_CVBS_VI2B;
> + else
> + chan->inputs[i].input_route = INPUT_SVIDEO_VI2C_VI1C;
> + chan->inputs[i].output_route = OUTPUT_10BIT_422_EMBEDDED_SYNC;
> +
> + err = v4l2_of_parse_endpoint(endpoint, &bus_cfg);
> + if (err) {
> + dev_err(&pdev->dev, "Could not parse the endpoint\n");
> + goto done;
> + }
> + dev_dbg(&pdev->dev, "Endpoint %s, bus_width = %d\n",
> + endpoint->full_name, bus_cfg.bus.parallel.bus_width);
> + flags = bus_cfg.bus.parallel.flags;
> +
> + if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> + chan->vpif_if.hd_pol = 1;
> +
> + if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
> + chan->vpif_if.vd_pol = 1;
> +
> + chan->vpif_if.if_type = VPIF_IF_BT656;
> + rem = of_graph_get_remote_port_parent(endpoint);
> + if (!rem) {
> + dev_dbg(&pdev->dev, "Remote device at %s not found\n",
> + endpoint->full_name);
> + goto done;
> + }
> +
> + dev_dbg(&pdev->dev, "Remote device %s, %s found\n",
> + rem->name, rem->full_name);
> + sdinfo->name = rem->full_name;
> +
> + pdata->asd[i] = devm_kzalloc(&pdev->dev,
> + sizeof(struct v4l2_async_subdev),
> + GFP_KERNEL);
Do you ensure somewhere that i isn't overrunning the pdata->asd[] array?
It's got VPIF_CAPTURE_MAX_CHANNELS entries.
> + if (!pdata->asd[i]) {
> + of_node_put(rem);
> + pdata = NULL;
> + goto done;
> + }
> +
> + pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_OF;
> + pdata->asd[i]->match.of.node = rem;
> + of_node_put(rem);
> + }
> +
> +done:
> + pdata->asd_sizes[0] = i;
> + pdata->subdev_count = i;
> + pdata->card_name = "DA850/OMAP-L138 Video Capture";
> +
> + return pdata;
> +}
> +
> /**
> * vpif_probe : This function probes the vpif capture driver
> * @pdev: platform device pointer
> @@ -1438,6 +1563,7 @@ static __init int vpif_probe(struct platform_device *pdev)
> int res_idx = 0;
> int i, err;
>
> + pdev->dev.platform_data = vpif_capture_get_pdata(pdev);
> if (!pdev->dev.platform_data) {
> dev_warn(&pdev->dev, "Missing platform data. Giving up.\n");
> return -EINVAL;
> @@ -1480,7 +1606,7 @@ static __init int vpif_probe(struct platform_device *pdev)
> goto vpif_unregister;
> }
>
> - if (!vpif_obj.config->asd_sizes) {
> + if (!vpif_obj.config->asd_sizes[0]) {
> i2c_adap = i2c_get_adapter(1);
> for (i = 0; i < subdev_count; i++) {
> subdevdata = &vpif_obj.config->subdev_info[i];
> diff --git a/include/media/davinci/vpif_types.h b/include/media/davinci/vpif_types.h
> index 3cb1704a0650..4ee3b41975db 100644
> --- a/include/media/davinci/vpif_types.h
> +++ b/include/media/davinci/vpif_types.h
> @@ -65,14 +65,14 @@ struct vpif_display_config {
>
> struct vpif_input {
> struct v4l2_input input;
> - const char *subdev_name;
> + char *subdev_name;
> u32 input_route;
> u32 output_route;
> };
>
> struct vpif_capture_chan_config {
> struct vpif_interface vpif_if;
> - const struct vpif_input *inputs;
> + struct vpif_input *inputs;
> int input_count;
> };
>
> @@ -83,7 +83,8 @@ struct vpif_capture_config {
> struct vpif_subdev_info *subdev_info;
> int subdev_count;
> const char *card_name;
> - struct v4l2_async_subdev **asd; /* Flat array, arranged in groups */
> - int *asd_sizes; /* 0-terminated array of asd group sizes */
> +
> + struct v4l2_async_subdev *asd[VPIF_CAPTURE_MAX_CHANNELS];
> + int asd_sizes[VPIF_CAPTURE_MAX_CHANNELS];
> };
> #endif /* _VPIF_TYPES_H */
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] ALSA SoC MAX98927 driver - Initial release
From: Mark Brown @ 2016-11-23 15:46 UTC (permalink / raw)
To: Ryan Lee
Cc: lgirdwood, robh+dt, mark.rutland, perex, tiwai, arnd, michael,
oder_chiou, yesanishhere, jacob, Damien.Horsley, bardliao,
kuninori.morimoto.gx, petr, lars, nh6z, alsa-devel, devicetree,
linux-kernel
In-Reply-To: <1479877026-5172-1-git-send-email-RyanS.Lee@maximintegrated.com>
[-- Attachment #1: Type: text/plain, Size: 4634 bytes --]
On Wed, Nov 23, 2016 at 01:57:06PM +0900, Ryan Lee wrote:
> +static struct reg_default max98927_reg_map[] = {
> + {0x0014, 0x78},
> + {0x0015, 0xFF},
> + {0x0043, 0x04},
> + {0x0017, 0x55},
> + /* For mono driver we are just enabling one channel*/
If this table contains anything other than the physical defaults the
device has it is broken.
> + {MAX98927_PCM_Rx_Enables_A, 0x03},
> + {MAX98927_PCM_Tx_HiZ_Control_A, 0xfc},
> + {MAX98927_PCM_Tx_HiZ_Control_B, 0xff},
> + {MAX98927_PCM_Tx_Channel_Sources_A, 0x01},
> + {MAX98927_PCM_Tx_Channel_Sources_B, 0x01},
> + {MAX98927_Measurement_DSP_Config, 0xf7},
> + {0x0025, 0x80},
This random mix of strangely formatted #defines and numbers isn't great
- can we please be consistent and ideally use normal style defines?
> +void max98927_wrapper_write(struct max98927_priv *max98927,
> + unsigned int reg, unsigned int val)
> +{
> + if (max98927->regmap)
> + regmap_write(max98927->regmap, reg, val);
> + if (max98927->sub_regmap)
> + regmap_write(max98927->sub_regmap, reg, val);
> +}
I don't really know what this is doing but it looks very confused.
Having multiple regmaps is a bit worrying but even more so is having
some of those regmaps be optional. If the device does sensibly have
multiple register maps I'd really not expect to see them appearing and
disappearing at runtime. Whatever this is doing it at least needs to be
documented.
> + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> + case SND_SOC_DAIFMT_CBS_CFS:
> + max98927_wrap_update_bits(max98927, MAX98927_PCM_Master_Mode,
> + MAX98927_PCM_Master_Mode_PCM_MSTR_MODE_Mask,
> + MAX98927_PCM_Master_Mode_PCM_MSTR_MODE_SLAVE);
> + break;
Please use a normal kernel coding style, I can't think of any coding
style where it's normal to indent continuation lines in a multi line
statement like this. There are severe coding style problems throughout
the driver which make it hard to read, it doesn't visually resemble
normal Linux kernel code.
> + case SND_SOC_DAIFMT_IB_NF:
> + invert = MAX98927_PCM_Mode_Config_PCM_BCLKEDGE;
> + break;
> + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> + case SND_SOC_DAIFMT_I2S:
> + max98927->iface |= SND_SOC_DAIFMT_I2S;
> + max98927_wrap_update_bits(max98927,
> + MAX98927_PCM_Mode_Config,
> + max98927->iface, max98927->iface);
> + break;
The indentation of the break statements isn't consistent within this
function :( The former is the normal kernel coding style.
> + if (i == ARRAY_SIZE(rate_table)) {
> + pr_err("%s couldn't get the MCLK to match codec\n",
> + __func__);
dev_err() and I'm not sure anyone's going to be able to understand that
error message...
> +static void max98927_handle_pdata(struct snd_soc_codec *codec)
> +{
> + struct max98927_priv *max98927 = snd_soc_codec_get_drvdata(codec);
> + struct max98927_reg_default *regInfo;
> + int cfg_size = 0;
> + int x;
> +
> + if (max98927->regcfg != NULL)
> + cfg_size = max98927->regcfg_sz / sizeof(uint32_t);
> +
> + if (cfg_size <= 0) {
> + dev_dbg(codec->dev,
> + "Register configuration is not required.\n");
> + return;
> + }
> +
> + /* direct configuration from device tree */
> + for (x = 0; x < cfg_size; x += 3) {
> + regInfo = (struct max98927_reg_default *)&max98927->regcfg[x];
> + dev_info(codec->dev, "CH:%d, reg:0x%02x, value:0x%02x\n",
> + be32_to_cpu(regInfo->ch),
> + be32_to_cpu(regInfo->reg),
> + be32_to_cpu(regInfo->def));
> + if (be32_to_cpu(regInfo->ch) == PRI_MAX98927
> + && max98927->regmap)
> + regmap_write(max98927->regmap,
> + be32_to_cpu(regInfo->reg),
> + be32_to_cpu(regInfo->def));
> + else if (be32_to_cpu(regInfo->ch) == SEC_MAX98927
> + && max98927->sub_regmap)
> + regmap_write(max98927->sub_regmap,
> + be32_to_cpu(regInfo->reg),
> + be32_to_cpu(regInfo->def));
> + }
> +}
This also looks like it probably shouldn't be doing whatever it is doing
but needs some documentation.
I've stopped here. In general it seems like this driver needs a *lot*
of work to work with the kernel interfaces in a normal style. Aside
from the coding style issues (which really get in the way) the bulk of
the code appears to be coming from unusual and undocumented ways of
working with kernel APIs. I'd strongly recommend taking a look at other
drivers for similar hardware and making sure that your driver looks like
them textually and structurally. If there are things about your
hardware that mean it needs something unusual then it should be clear to
someone reading the code what's going on.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] PM / Domains: Introduce domain-performance-state binding
From: Kevin Hilman @ 2016-11-23 15:51 UTC (permalink / raw)
To: Vincent Guittot
Cc: Viresh Kumar, Rob Herring, Rafael Wysocki,
linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org,
linux-kernel, Mark Rutland, Ulf Hansson, Lina Iyer,
devicetree@vger.kernel.org, Stephen Boyd, Nayak Rajendra
In-Reply-To: <CAKfTPtDR6Y5UgaqJ+D5T0yBeFRSYWm6OT1+r4ZABrtqtvF2D0w@mail.gmail.com>
Vincent Guittot <vincent.guittot@linaro.org> writes:
> On 22 November 2016 at 19:12, Kevin Hilman <khilman@baylibre.com> wrote:
>> Viresh Kumar <viresh.kumar@linaro.org> writes:
>>
>>> On 21-11-16, 09:07, Rob Herring wrote:
>>>> On Fri, Nov 18, 2016 at 02:53:12PM +0530, Viresh Kumar wrote:
>>>> > Some platforms have the capability to configure the performance state of
>>>> > their Power Domains. The performance levels are represented by positive
>>>> > integer values, a lower value represents lower performance state.
>>>> >
>>>> > The power-domains until now were only concentrating on the idle state
>>>> > management of the device and this needs to change in order to reuse the
>>>> > infrastructure of power domains for active state management.
>>>> >
>>>> > This patch introduces a new optional property for the consumers of the
>>>> > power-domains: domain-performance-state.
>>>> >
>>>> > If the consumers don't need the capability of switching to different
>>>> > domain performance states at runtime, then they can simply define their
>>>> > required domain performance state in their node directly. Otherwise the
>>>> > consumers can define their requirements with help of other
>>>> > infrastructure, for example the OPP table.
>>>> >
>>>> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>>>> > ---
>>>> > Documentation/devicetree/bindings/power/power_domain.txt | 6 ++++++
>>>> > 1 file changed, 6 insertions(+)
>>>> >
>>>> > diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>>>> > index e1650364b296..db42eacf8b5c 100644
>>>> > --- a/Documentation/devicetree/bindings/power/power_domain.txt
>>>> > +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>>>> > @@ -106,6 +106,12 @@ domain provided by the 'parent' power controller.
>>>> > - power-domains : A phandle and PM domain specifier as defined by bindings of
>>>> > the power controller specified by phandle.
>>>> >
>>>> > +Optional properties:
>>>> > +- domain-performance-state: A positive integer value representing the minimum
>>>> > + performance level (of the parent domain) required by the consumer for its
>>>> > + working. The integer value '1' represents the lowest performance level and the
>>>> > + highest value represents the highest performance level.
>>>>
>>>> How does one come up with the range of values?
>>>
>>> Why would we need a range here? The value here represents the minimum 'state'
>>> and the assumption is that everything above that level would be fine. So the
>>> range is automatically: domain-performance-state -> MAX.
>>>
>>>> It seems like you are
>>>> just making up numbers. Couldn't the domain performance level be an OPP
>>>> in the sense that it is a collection of clock frequencies and voltage
>>>> settings?
>>>
>>> The clock is going to be handled by the device itself (at least for the case we
>>> have today) and the performance-state lies with the power-domain which is
>>> configured separately. If the performance level includes both clk and voltage,
>>> then why would we need to show the clock rates in the DT ? Wouldn't a
>>> performance level be enough in such cases?
>>
>> I think the question is: what does the performance-level of a domain
>> actually mean? Or, what are the units?
>>
>> Depending on the SoC, there's probably a few things this could mean. It
>> might mean is that an underlying bus/interconnect can be configured to
>> guarantee a specific bandwidth or throughput. That in turn might mean
>> that that bus/interconnect might have to be set at a specific
>> frequency/voltage.
>>
>> In your case, IIUC, you're just passing some magic value to some
>> firmware running on a micro-controller, but under the hood that uC is
>> probably configuring a frequency/voltage someplace.
>
> In the case described by Viresh, it's only about setting the voltage
> of a power domain that is shared between different devices. these
> devices wants to run at different frequency (set by the devices) but
> we have to select a Volateg value that will match with the constraint
> of all devices (in this case the highest voltage)
Then, at least for this use case, we're talking about voltage, not some
unspecified units.
But that makes me wonder, this performance state sounds like something
that is changing dynamically at runtime, so why do you want to describe
this statically in DT?
This sounds to me like the job of the genpd. When any device in the
domain does its pm_runtime_get(), the domain could check the device
frequency and see if it needs to change the domain voltage in order for
that device to operate at that frequency. When the device goes away
(using pm_runtime_put()) the domain can check again if it could lower
the voltage and still meet the requirements of the remaining devices.
Kevin
^ permalink raw reply
* Re: [PATCH 1/2] PM / Domains: Introduce domain-performance-state binding
From: Vincent Guittot @ 2016-11-23 15:55 UTC (permalink / raw)
To: Kevin Hilman
Cc: Viresh Kumar, Rob Herring, Rafael Wysocki,
linaro-kernel-cunTk1MwBs8s++Sfvej+rw@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel,
Mark Rutland, Ulf Hansson, Lina Iyer,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Stephen Boyd,
Nayak Rajendra
In-Reply-To: <m2wpfuw5wq.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
On 23 November 2016 at 16:51, Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
> Vincent Guittot <vincent.guittot-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:
>
>> On 22 November 2016 at 19:12, Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
>>> Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:
>>>
>>>> On 21-11-16, 09:07, Rob Herring wrote:
>>>>> On Fri, Nov 18, 2016 at 02:53:12PM +0530, Viresh Kumar wrote:
>>>>> > Some platforms have the capability to configure the performance state of
>>>>> > their Power Domains. The performance levels are represented by positive
>>>>> > integer values, a lower value represents lower performance state.
>>>>> >
>>>>> > The power-domains until now were only concentrating on the idle state
>>>>> > management of the device and this needs to change in order to reuse the
>>>>> > infrastructure of power domains for active state management.
>>>>> >
>>>>> > This patch introduces a new optional property for the consumers of the
>>>>> > power-domains: domain-performance-state.
>>>>> >
>>>>> > If the consumers don't need the capability of switching to different
>>>>> > domain performance states at runtime, then they can simply define their
>>>>> > required domain performance state in their node directly. Otherwise the
>>>>> > consumers can define their requirements with help of other
>>>>> > infrastructure, for example the OPP table.
>>>>> >
>>>>> > Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>> > ---
>>>>> > Documentation/devicetree/bindings/power/power_domain.txt | 6 ++++++
>>>>> > 1 file changed, 6 insertions(+)
>>>>> >
>>>>> > diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>>>>> > index e1650364b296..db42eacf8b5c 100644
>>>>> > --- a/Documentation/devicetree/bindings/power/power_domain.txt
>>>>> > +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>>>>> > @@ -106,6 +106,12 @@ domain provided by the 'parent' power controller.
>>>>> > - power-domains : A phandle and PM domain specifier as defined by bindings of
>>>>> > the power controller specified by phandle.
>>>>> >
>>>>> > +Optional properties:
>>>>> > +- domain-performance-state: A positive integer value representing the minimum
>>>>> > + performance level (of the parent domain) required by the consumer for its
>>>>> > + working. The integer value '1' represents the lowest performance level and the
>>>>> > + highest value represents the highest performance level.
>>>>>
>>>>> How does one come up with the range of values?
>>>>
>>>> Why would we need a range here? The value here represents the minimum 'state'
>>>> and the assumption is that everything above that level would be fine. So the
>>>> range is automatically: domain-performance-state -> MAX.
>>>>
>>>>> It seems like you are
>>>>> just making up numbers. Couldn't the domain performance level be an OPP
>>>>> in the sense that it is a collection of clock frequencies and voltage
>>>>> settings?
>>>>
>>>> The clock is going to be handled by the device itself (at least for the case we
>>>> have today) and the performance-state lies with the power-domain which is
>>>> configured separately. If the performance level includes both clk and voltage,
>>>> then why would we need to show the clock rates in the DT ? Wouldn't a
>>>> performance level be enough in such cases?
>>>
>>> I think the question is: what does the performance-level of a domain
>>> actually mean? Or, what are the units?
>>>
>>> Depending on the SoC, there's probably a few things this could mean. It
>>> might mean is that an underlying bus/interconnect can be configured to
>>> guarantee a specific bandwidth or throughput. That in turn might mean
>>> that that bus/interconnect might have to be set at a specific
>>> frequency/voltage.
>>>
>>> In your case, IIUC, you're just passing some magic value to some
>>> firmware running on a micro-controller, but under the hood that uC is
>>> probably configuring a frequency/voltage someplace.
>>
>> In the case described by Viresh, it's only about setting the voltage
>> of a power domain that is shared between different devices. these
>> devices wants to run at different frequency (set by the devices) but
>> we have to select a Volateg value that will match with the constraint
>> of all devices (in this case the highest voltage)
>
> Then, at least for this use case, we're talking about voltage, not some
> unspecified units.
>
> But that makes me wonder, this performance state sounds like something
> that is changing dynamically at runtime, so why do you want to describe
> this statically in DT?
>
> This sounds to me like the job of the genpd. When any device in the
> domain does its pm_runtime_get(), the domain could check the device
> frequency and see if it needs to change the domain voltage in order for
> that device to operate at that frequency. When the device goes away
> (using pm_runtime_put()) the domain can check again if it could lower
> the voltage and still meet the requirements of the remaining devices.
That's only part of the job. The device can change its frequency and
as a result ask for a new voltage index while it is already running
Vincent
>
> Kevin
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/2] ARM64: dts: Add support for Meson GXM
From: Kevin Hilman @ 2016-11-23 15:58 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: Neil Armstrong, carlo, linux-amlogic, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <CAFBinCAbUK-6uLc_ceJ5d-oD4N_HGz9rbtCvivTp1N8OOVBNBg@mail.gmail.com>
Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
> On Tue, Nov 22, 2016 at 11:00 AM, Neil Armstrong
> <narmstrong@baylibre.com> wrote:
>> Following the Amlogic Linux kernel, it seem the only differences
>> between the GXL and GXM SoCs are the CPU Clusters.
>>
>> This commit renames the gxl-s905d-p23x DTSI in a common file for
>> S905D p23x and S912 q20x boards.
>>
>> Then adds a meson-gxm dtsi and reproduce the P23x to Q20x boards
>> dts files since the S905D and S912 SoCs shares the same pinout
>> and the P23x and Q20x boards are identical.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
> I have tested this successfully on a Mecool BB2 (similar to the Q200
> reference board).
Thanks for testing (and reporting),
Kevin
^ permalink raw reply
* Re: [PATCH] v4l: async: make v4l2 coexists with devicetree nodes in a dt overlay
From: Javi Merino @ 2016-11-23 16:03 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: linux-media, linux-kernel, devicetree, Pantelis Antoniou,
Mauro Carvalho Chehab, Sakari Ailus
In-Reply-To: <cf31105b-e8c1-4379-cd03-0bdcbdea64d6@osg.samsung.com>
On Wed, Nov 23, 2016 at 11:25:39AM -0300, Javier Martinez Canillas wrote:
> Hello Javi,
>
> On 11/23/2016 07:09 AM, Javi Merino wrote:
> > In asd's configured with V4L2_ASYNC_MATCH_OF, if the v4l2 subdev is in
> > a devicetree overlay, its of_node pointer will be different each time
> > the overlay is applied. We are not interested in matching the
> > pointer, what we want to match is that the path is the one we are
> > expecting. Change to use of_node_cmp() so that we continue matching
> > after the overlay has been removed and reapplied.
> >
>
> I'm still not that familiar with DT overlays (and I guess others aren't
> either) so I think that including an example of a base tree and overlay
> DTS where this is an issue, could make things more clear in the commit.
>
> IIUC, it should be something like this?
>
> -- base tree --
>
> &i2c1 {
> camera: camera@10 {
> reg = <0x10>;
> port {
> cam_ep: endpoint {
> ...
> };
> };
> };
> };
>
> &media_bridge {
> ...
> ports {
> port@0 {
> reg = <0>;
> ep: endpoint {
> remote-endpoint = <&cam_ep>;
> };
> };
> };
> };
>
> -- overlay --
>
> /plugin/;
> / {
> ...
> fragment@0 {
> target = <&camera>;
> __overlay__ {
> compatible = "foo,bar";
> ...
> port {
> cam_ep: endpoint {
> ...
> };
> };
> };
> }
> }
Yes, that's right. What I have is that the whole camera can be
plugged or unplugged, so the overlay adds/removes the camera node:
/ {
fragment@0 {
target-path = "/i2c0";
__overlay__ {
my_cam {
compatible = "foo,bar";
port {
camera0: endpoint {
remote-endpoint = <&vin2a>;
...
};
};
};
};
};
I will add it to the commit message.
> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> > Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Signed-off-by: Javi Merino <javi.merino@kernel.org>
> > ---
> > Hi,
> >
> > I feel it is a bit of a hack, but I couldn't think of anything better.
> > I'm ccing devicetree@ and Pantelis because there may be a simpler
> > solution.
> >
>
> I also couldn't think a better way to do this, since IIUC the node's name is
> the only thing that doesn't change, and is available at the time the bridge
> driver calls v4l2_async_notifier_register() when parsing the base tree.
>
> > drivers/media/v4l2-core/v4l2-async.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> > index 5bada20..d33a17c 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -42,7 +42,8 @@ static bool match_devname(struct v4l2_subdev *sd,
> >
> > static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> > {
> > - return sd->of_node == asd->match.of.node;
> > + return !of_node_cmp(of_node_full_name(sd->of_node),
> > + of_node_full_name(asd->match.of.node));
> > }
> >
> > static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> >
>
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
>
> Best regards,
> --
> Javier Martinez Canillas
> Open Source Group
> Samsung Research America
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox