* Re: [PATCH v2 4/6] pinctrl: aspeed: Read and write bits in LPCHC and GFX controllers
From: Andrew Jeffery @ 2016-11-04 3:59 UTC (permalink / raw)
To: Joel Stanley
Cc: Lee Jones, Linus Walleij, Mark Rutland, Rob Herring, linux-gpio,
linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <CACPK8Xd1aisN4d2C01NXAYTZ1FW=C4b_hjvHce-RY9ReRq2cDw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 27669 bytes --]
On Fri, 2016-11-04 at 09:54 +1030, Joel Stanley wrote:
> On Thu, Nov 3, 2016 at 1:07 AM, Andrew Jeffery <andrew@aj.id.au> wrote:
> >
> > The System Control Unit IP block in the Aspeed SoCs is typically where
> > the pinmux configuration is found, but not always. A number of pins
> > depend on state in one of LPC Host Control (LPCHC) or SoC Display
> > Controller (GFX) IP blocks, so the Aspeed pinmux drivers should have the
> > means to adjust these as necessary.
> >
> > We use syscon to cast a regmap over the GFX and LPCHCR blocks, which is
> > used as an arbitration layer between the relevant driver and the pinctrl
> > subsystem. The regmaps are then exposed to the SoC-specific pinctrl
> > drivers by phandles in the devicetree, and are selected during a mux
> > request by querying a new 'ip' member in struct aspeed_sig_desc.
> >
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> I like this a lot more than the first go. Good work.
>
> Some minor comments below.
>
> >
> > ---
> > Since v1:
> >
> > The change is now proactive: instead of reporting that we need to flip bits in
> > controllers we can't access, the patch provides access via regmaps for the
> > relevant controllers. The implementation also splits out the IP block ID into
> > its own variable rather than packing the value into the upper bits of the reg
> > member of struct aspeed_sig_desc. This drives some churn in the diff, but I've
> > tried to minimise it.
> >
> > .../devicetree/bindings/pinctrl/pinctrl-aspeed.txt | 50 +++++++++++++---
> > drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c | 18 +++---
> > drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 39 ++++++++++---
> > drivers/pinctrl/aspeed/pinctrl-aspeed.c | 66 +++++++++++++---------
> > drivers/pinctrl/aspeed/pinctrl-aspeed.h | 32 ++++++++---
> > 5 files changed, 144 insertions(+), 61 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
> > index 2ad18c4ea55c..115b0cce6c1c 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
> > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
> > @@ -4,12 +4,19 @@ Aspeed Pin Controllers
> > The Aspeed SoCs vary in functionality inside a generation but have a common mux
> > device register layout.
> >
> > -Required properties:
> > -- compatible : Should be any one of the following:
> > - "aspeed,ast2400-pinctrl"
> > - "aspeed,g4-pinctrl"
> > - "aspeed,ast2500-pinctrl"
> > - "aspeed,g5-pinctrl"
> > +Required properties for g4:
> > +- compatible : Should be any one of the following:
> > + "aspeed,ast2400-pinctrl"
> > + "aspeed,g4-pinctrl"
> > +
> > +Required properties for g5:
> > +- compatible : Should be any one of the following:
> > + "aspeed,ast2500-pinctrl"
> > + "aspeed,g5-pinctrl"
> > +
> > +- aspeed,external-nodes: A cell of phandles to external controller nodes:
> > + 0: compatible with "aspeed,ast2500-gfx", "syscon"
> > + 1: compatible with "aspeed,ast2500-lpchc", "syscon"
> >
> > The pin controller node should be a child of a syscon node with the required
> > property:
> > @@ -47,7 +54,7 @@ RGMII1 RGMII2 RMII1 RMII2 SD1 SPI1 SPI1DEBUG SPI1PASSTHRU TIMER4 TIMER5 TIMER6
> > TIMER7 TIMER8 VGABIOSROM
> >
> >
> > -Examples:
> > +g4 Example:
> >
> > syscon: scu@1e6e2000 {
> > compatible = "syscon", "simple-mfd";
> > @@ -63,5 +70,34 @@ syscon: scu@1e6e2000 {
> > };
> > };
> >
> > +g5 Example:
> > +
> > +apb {
> > + gfx: display@1e6e6000 {
> > + compatible = "aspeed,ast2500-gfx", "syscon";
> > + reg = <0x1e6e6000 0x1000>;
> > + };
> > +
> > + lpchc: lpchc@1e7890a0 {
> > + compatible = "aspeed,ast2500-lpchc", "syscon";
> > + reg = <0x1e7890a0 0xc4>;
> > + };
> > +
> > + syscon: scu@1e6e2000 {
> > + compatible = "syscon", "simple-mfd";
> > + reg = <0x1e6e2000 0x1a8>;
> > +
> > + pinctrl: pinctrl {
> > + compatible = "aspeed,g5-pinctrl";
> > + aspeed,external-nodes = <&gfx, &lpchc>;
> > +
> > + pinctrl_i2c3_default: i2c3_default {
> > + function = "I2C3";
> > + groups = "I2C3";
> > + };
> > + };
> > + };
> > +};
> > +
> > Please refer to pinctrl-bindings.txt in this directory for details of the
> > common pinctrl bindings used by client devices.
> > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
> > index a21b071ff290..558bd102416c 100644
> > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
> > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
> > @@ -292,7 +292,7 @@ SSSF_PIN_DECL(U18, GPIOG7, FLWP, SIG_DESC_SET(SCU84, 7));
> > #define UART6_DESC SIG_DESC_SET(SCU90, 7)
> > #define ROM16_DESC SIG_DESC_SET(SCU90, 6)
> > #define FLASH_WIDE SIG_DESC_SET(HW_STRAP1, 4)
> > -#define BOOT_SRC_NOR { HW_STRAP1, GENMASK(1, 0), 0, 0 }
> > +#define BOOT_SRC_NOR { ASPEED_IP_SCU, HW_STRAP1, GENMASK(1, 0), 0, 0 }
> >
> > #define A8 56
> > SIG_EXPR_DECL(ROMD8, ROM16, ROM16_DESC);
> > @@ -418,9 +418,9 @@ FUNC_GROUP_DECL(I2C8, G5, F3);
> > #define U1 88
> > SSSF_PIN_DECL(U1, GPIOL0, NCTS1, SIG_DESC_SET(SCU84, 16));
> >
> > -#define VPI18_DESC { SCU90, GENMASK(5, 4), 1, 0 }
> > -#define VPI24_DESC { SCU90, GENMASK(5, 4), 2, 0 }
> > -#define VPI30_DESC { SCU90, GENMASK(5, 4), 3, 0 }
> > +#define VPI18_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 1, 0 }
> > +#define VPI24_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 2, 0 }
> > +#define VPI30_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 3, 0 }
> >
> > #define T5 89
> > #define T5_DESC SIG_DESC_SET(SCU84, 17)
> > @@ -641,11 +641,11 @@ SSSF_PIN_DECL(Y22, GPIOR2, ROMCS3, SIG_DESC_SET(SCU88, 26));
> > #define U19 139
> > SSSF_PIN_DECL(U19, GPIOR3, ROMCS4, SIG_DESC_SET(SCU88, 27));
> >
> > -#define VPOOFF0_DESC { SCU94, GENMASK(1, 0), 0, 0 }
> > -#define VPO12_DESC { SCU94, GENMASK(1, 0), 1, 0 }
> > -#define VPO24_DESC { SCU94, GENMASK(1, 0), 2, 0 }
> > -#define VPOOFF1_DESC { SCU94, GENMASK(1, 0), 3, 0 }
> > -#define VPO_OFF_12 { SCU94, 0x2, 0, 0 }
> > +#define VPOOFF0_DESC { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 0, 0 }
> > +#define VPO12_DESC { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 1, 0 }
> > +#define VPO24_DESC { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 2, 0 }
> > +#define VPOOFF1_DESC { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 3, 0 }
> > +#define VPO_OFF_12 { ASPEED_IP_SCU, SCU94, 0x2, 0, 0 }
> > #define VPO_24_OFF SIG_DESC_SET(SCU94, 1)
> >
> > #define V21 140
> > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> > index 87b46390b695..99c4fa9bf861 100644
> > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> > @@ -10,6 +10,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -26,8 +27,8 @@
> >
> > #define ASPEED_G5_NR_PINS 228
> >
> > -#define COND1 { SCU90, BIT(6), 0, 0 }
> > -#define COND2 { SCU94, GENMASK(1, 0), 0, 0 }
> > +#define COND1 { ASPEED_IP_SCU, SCU90, BIT(6), 0, 0 }
> > +#define COND2 { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 0, 0 }
> >
> > #define B14 0
> > SSSF_PIN_DECL(B14, GPIOA0, MAC1LINK, SIG_DESC_SET(SCU80, 0));
> > @@ -186,9 +187,12 @@ MS_PIN_DECL(C20, GPIOE1, NDCD3, GPIE0OUT);
> >
> > FUNC_GROUP_DECL(GPIE0, B20, C20);
> >
> > -#define SPI1_DESC { HW_STRAP1, GENMASK(13, 12), 1, 0 }
> > -#define SPI1DEBUG_DESC { HW_STRAP1, GENMASK(13, 12), 2, 0 }
> > -#define SPI1PASSTHRU_DESC { HW_STRAP1, GENMASK(13, 12), 3, 0 }
> > +#define SPI1_DESC \
> > + { ASPEED_IP_SCU, HW_STRAP1, GENMASK(13, 12), 1, 0 }
> > +#define SPI1DEBUG_DESC \
> > + { ASPEED_IP_SCU, HW_STRAP1, GENMASK(13, 12), 2, 0 }
> > +#define SPI1PASSTHRU_DESC \
> > + { ASPEED_IP_SCU, HW_STRAP1, GENMASK(13, 12), 3, 0 }
> >
> > #define C18 64
> > SIG_EXPR_DECL(SYSCS, SPI1DEBUG, COND1, SPI1DEBUG_DESC);
> > @@ -325,10 +329,11 @@ SS_PIN_DECL(R1, GPIOK7, SDA8);
> >
> > FUNC_GROUP_DECL(I2C8, P2, R1);
> >
> > -#define VPIOFF0_DESC { SCU90, GENMASK(5, 4), 0, 0 }
> > -#define VPIOFF1_DESC { SCU90, GENMASK(5, 4), 1, 0 }
> > -#define VPI24_DESC { SCU90, GENMASK(5, 4), 2, 0 }
> > -#define VPIRSVD_DESC { SCU90, GENMASK(5, 4), 3, 0 }
> > +#define VPIOFF0_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 0, 0 }
> > +#define VPIOFF1_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 1, 0 }
> > +#define VPI24_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 2, 0 }
> > +#define VPIRSVD_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 3, 0 }
> > +
> >
> > #define V2 104
> > #define V2_DESC SIG_DESC_SET(SCU88, 0)
> > @@ -848,10 +853,26 @@ static struct pinctrl_desc aspeed_g5_pinctrl_desc = {
> > static int aspeed_g5_pinctrl_probe(struct platform_device *pdev)
> > {
> > int i;
> > + struct regmap **map;
> > + struct device_node *node;
> >
> > for (i = 0; i < ARRAY_SIZE(aspeed_g5_pins); i++)
> > aspeed_g5_pins[i].number = i;
> >
> > + map = &aspeed_g5_pinctrl_data.maps[ASPEED_IP_GFX];
> > + node = of_parse_phandle(pdev->dev.of_node, "aspeed,external-nodes", 0);
> > + *map = syscon_node_to_regmap(node);
> I think you can use syscon_regmap_lookup_by_phandle to replace both of
> these lines.
Good call, will do.
>
> >
> > + of_node_put(node);
> > + if (IS_ERR(*map))
> > + return PTR_ERR(*map);
> Do we want to fail, or warn and continue?
We would need to add further checks to defend against null dereferences
if we were to continue. I think the broken devicetree should be fixed.
>
> The sequence is a bit messy. How about:
>
> struct regmap *map;
>
> map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> "aspeed,external-nodes");
> if (IS_ERR(map))
> return PTR_ERR(map);
>
> aspeed_g5_pinctrl_data.maps[ASPEED_IP_GFX] = map;
That looks neater, I will switch.
>
>
> >
> > +
> > + map = &aspeed_g5_pinctrl_data.maps[ASPEED_IP_LPCHC];
> > + node = of_parse_phandle(pdev->dev.of_node, "aspeed,external-nodes", 1);
> > + *map = syscon_node_to_regmap(node);
> > + of_node_put(node);
> > + if (IS_ERR(*map))
> > + return PTR_ERR(*map);
> > +
> Same comments as above.
Ack.
>
> >
> > return aspeed_pinctrl_probe(pdev, &aspeed_g5_pinctrl_desc,
> > &aspeed_g5_pinctrl_data);
> > }
> > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> > index 49aeba912531..23586aac7a5a 100644
> > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> > @@ -14,6 +14,12 @@
> > #include "../core.h"
> > #include "pinctrl-aspeed.h"
> >
> > +static const char *const aspeed_pinmux_ips[] = {
> > + [ASPEED_IP_SCU] = "SCU",
> > + [ASPEED_IP_GFX] = "GFX",
> > + [ASPEED_IP_LPCHC] = "LHCR",
> We've got both LPCHC and LHCR here. As I said when commenting on the
> regmap bindings, I like LHC(R) better.
>
> >
> > +};
> > +
> > int aspeed_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
> > {
> > struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
> > @@ -78,7 +84,8 @@ int aspeed_pinmux_get_fn_groups(struct pinctrl_dev *pctldev,
> > static inline void aspeed_sig_desc_print_val(
> > const struct aspeed_sig_desc *desc, bool enable, u32 rv)
> What's a rv? Perhaps "reg" or "value"?
>
> >
> > {
> > - pr_debug("SCU%x[0x%08x]=0x%x, got 0x%x from 0x%08x\n", desc->reg,
> > + pr_debug("Want %s%X[0x%08X]=0x%X, got 0x%X from 0x%08X\n",
> > + aspeed_pinmux_ips[desc->ip], desc->reg,
> > desc->mask, enable ? desc->enable : desc->disable,
> > (rv & desc->mask) >> __ffs(desc->mask), rv);
> > }
> > @@ -88,7 +95,7 @@ static inline void aspeed_sig_desc_print_val(
> > *
> > * @desc: The signal descriptor of interest
> > * @enabled: True to query the enabled state, false to query disabled state
> > - * @regmap: The SCU regmap instance
> > + * @regmap: The IP block's regmap instance
> > *
> > * @return True if the descriptor's bitfield is configured to the state
> > * selected by @enabled, false otherwise
> > @@ -119,7 +126,7 @@ static bool aspeed_sig_desc_eval(const struct aspeed_sig_desc *desc,
> > *
> > * @expr: An expression controlling the signal for a mux function on a pin
> > * @enabled: True to query the enabled state, false to query disabled state
> > - * @regmap: The SCU regmap instance
> > + * @maps: The list of regmap instances
> > *
> > * @return True if the expression composed by @enabled evaluates true, false
> > * otherwise
> > @@ -136,15 +143,16 @@ static bool aspeed_sig_desc_eval(const struct aspeed_sig_desc *desc,
> > * either condition as required.
> > */
> > static bool aspeed_sig_expr_eval(const struct aspeed_sig_expr *expr,
> > - bool enabled, struct regmap *map)
> > + bool enabled, struct regmap * const *maps)
> > {
> > int i;
> >
> > for (i = 0; i < expr->ndescs; i++) {
> > const struct aspeed_sig_desc *desc = &expr->descs[i];
> >
> > - if (!aspeed_sig_desc_eval(desc, enabled, map))
> > + if (!aspeed_sig_desc_eval(desc, enabled, maps[desc->ip]))
> > return false;
> > +
> > }
> >
> > return true;
> > @@ -158,12 +166,12 @@ static bool aspeed_sig_expr_eval(const struct aspeed_sig_expr *expr,
> > * configured
> > * @enable: true to enable an function's signal through a pin's signal
> > * expression, false to disable the function's signal
> > - * @map: The SCU's regmap instance for pinmux register access.
> > + * @maps: The list of regmap instances for pinmux register access.
> > *
> > * @return true if the expression is configured as requested, false otherwise
> > */
> > static bool aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
> > - bool enable, struct regmap *map)
> > + bool enable, struct regmap * const *maps)
> > {
> > int i;
> >
> > @@ -171,6 +179,7 @@ static bool aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
> > bool ret;
> > const struct aspeed_sig_desc *desc = &expr->descs[i];
> > u32 pattern = enable ? desc->enable : desc->disable;
> > + u32 val = (pattern << __ffs(desc->mask));
> >
> > /*
> > * Strap registers are configured in hardware or by early-boot
> > @@ -179,48 +188,49 @@ static bool aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
> > * deconfigured and is the reason we re-evaluate after writing
> > * all descriptor bits.
> > */
> > - if (desc->reg == HW_STRAP1 || desc->reg == HW_STRAP2)
> > + if ((desc->reg == HW_STRAP1 || desc->reg == HW_STRAP2) &&
> > + desc->ip == ASPEED_IP_SCU)
> > continue;
> >
> > - ret = regmap_update_bits(map, desc->reg, desc->mask,
> > - pattern << __ffs(desc->mask)) == 0;
> > + ret = regmap_update_bits(maps[desc->ip], desc->reg,
> > + desc->mask, val) == 0;
> >
> > if (!ret)
> > return ret;
> > }
> >
> > - return aspeed_sig_expr_eval(expr, enable, map);
> > + return aspeed_sig_expr_eval(expr, enable, maps);
> > }
> >
> > static bool aspeed_sig_expr_enable(const struct aspeed_sig_expr *expr,
> > - struct regmap *map)
> > + struct regmap * const *maps)
> > {
> > - if (aspeed_sig_expr_eval(expr, true, map))
> > + if (aspeed_sig_expr_eval(expr, true, maps))
> > return true;
> >
> > - return aspeed_sig_expr_set(expr, true, map);
> > + return aspeed_sig_expr_set(expr, true, maps);
> > }
> >
> > static bool aspeed_sig_expr_disable(const struct aspeed_sig_expr *expr,
> > - struct regmap *map)
> > + struct regmap * const *maps)
> > {
> > - if (!aspeed_sig_expr_eval(expr, true, map))
> > + if (!aspeed_sig_expr_eval(expr, true, maps))
> > return true;
> >
> > - return aspeed_sig_expr_set(expr, false, map);
> > + return aspeed_sig_expr_set(expr, false, maps);
> > }
> >
> > /**
> > * Disable a signal on a pin by disabling all provided signal expressions.
> > *
> > * @exprs: The list of signal expressions (from a priority level on a pin)
> > - * @map: The SCU's regmap instance for pinmux register access.
> > + * @maps: The list of regmap instances for pinmux register access.
> > *
> > * @return true if all expressions in the list are successfully disabled, false
> > * otherwise
> > */
> > static bool aspeed_disable_sig(const struct aspeed_sig_expr **exprs,
> > - struct regmap *map)
> > + struct regmap * const *maps)
> > {
> > bool disabled = true;
> >
> > @@ -230,7 +240,7 @@ static bool aspeed_disable_sig(const struct aspeed_sig_expr **exprs,
> > while (*exprs) {
> > bool ret;
> >
> > - ret = aspeed_sig_expr_disable(*exprs, map);
> > + ret = aspeed_sig_expr_disable(*exprs, maps);
> > disabled = disabled && ret;
> >
> > exprs++;
> > @@ -343,6 +353,8 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
> > const struct aspeed_sig_expr **funcs;
> > const struct aspeed_sig_expr ***prios;
> >
> > + pr_debug("Muxing pin %d for %s\n", pin, pfunc->name);
> > +
> > if (!pdesc)
> > return -EINVAL;
> >
> > @@ -358,7 +370,7 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
> > if (expr)
> > break;
> >
> > - if (!aspeed_disable_sig(funcs, pdata->map))
> > + if (!aspeed_disable_sig(funcs, pdata->maps))
> > return -EPERM;
> >
> > prios++;
> > @@ -377,7 +389,7 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
> > return -ENXIO;
> > }
> >
> > - if (!aspeed_sig_expr_enable(expr, pdata->map))
> > + if (!aspeed_sig_expr_enable(expr, pdata->maps))
> > return -EPERM;
> > }
> >
> > @@ -432,7 +444,7 @@ int aspeed_gpio_request_enable(struct pinctrl_dev *pctldev,
> > if (aspeed_gpio_in_exprs(funcs))
> > break;
> >
> > - if (!aspeed_disable_sig(funcs, pdata->map))
> > + if (!aspeed_disable_sig(funcs, pdata->maps))
> > return -EPERM;
> >
> > prios++;
> > @@ -462,7 +474,7 @@ int aspeed_gpio_request_enable(struct pinctrl_dev *pctldev,
> > * If GPIO is not the lowest priority signal type, assume there is only
> > * one expression defined to enable the GPIO function
> > */
> > - if (!aspeed_sig_expr_enable(expr, pdata->map))
> > + if (!aspeed_sig_expr_enable(expr, pdata->maps))
> > return -EPERM;
> >
> > return 0;
> > @@ -481,10 +493,10 @@ int aspeed_pinctrl_probe(struct platform_device *pdev,
> > return -ENODEV;
> > }
> >
> > - pdata->map = syscon_node_to_regmap(parent->of_node);
> > - if (IS_ERR(pdata->map)) {
> > + pdata->maps[ASPEED_IP_SCU] = syscon_node_to_regmap(parent->of_node);
> > + if (IS_ERR(pdata->maps[ASPEED_IP_SCU])) {
> > dev_err(&pdev->dev, "No regmap for syscon pincontroller parent\n");
> > - return PTR_ERR(pdata->map);
> > + return PTR_ERR(pdata->maps[ASPEED_IP_SCU]);
> > }
> >
> > pctl = pinctrl_register(pdesc, &pdev->dev, pdata);
> > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.h b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
> > index 3e72ef8c54bf..727728b86c07 100644
> > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.h
> > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
> > @@ -232,6 +232,11 @@
> > * group.
> > */
> >
> > +#define ASPEED_IP_SCU 0
> > +#define ASPEED_IP_GFX 1
> > +#define ASPEED_IP_LPCHC 2
> > +#define ASPEED_NR_PINMUX_IPS 3
> > +
> > /*
> > * The "Multi-function Pins Mapping and Control" table in the SoC datasheet
> > * references registers by the device/offset mnemonic. The register macros
> > @@ -261,7 +266,9 @@
> > * A signal descriptor, which describes the register, bits and the
> > * enable/disable values that should be compared or written.
> > *
> > - * @reg: The register offset from base in bytes
> > + * @ip: The IP block identifier, used as an index into the regmap array in
> > + * struct aspeed_pinctrl_data
> > + * @reg: The register offset with respect to the base address of the IP block
> > * @mask: The mask to apply to the register. The lowest set bit of the mask is
> > * used to derive the shift value.
> > * @enable: The value that enables the function. Value should be in the LSBs,
> > @@ -270,6 +277,7 @@
> > * LSBs, not at the position of the mask.
> > */
> > struct aspeed_sig_desc {
> > + unsigned int ip;
> > unsigned int reg;
> > u32 mask;
> > u32 enable;
> > @@ -313,24 +321,30 @@ struct aspeed_pin_desc {
> >
> > /* Macro hell */
> >
> > +#define SIG_DESC_IP_BIT(ip, reg, idx, val) \
> > + { ip, reg, BIT_MASK(idx), val, (((val) + 1) & 1) }
> > +
> > /**
> > - * Short-hand macro for describing a configuration enabled by the state of one
> > - * bit. The disable value is derived.
> > + * Short-hand macro for describing an SCU descriptor enabled by the state of
> > + * one bit. The disable value is derived.
> > *
> > * @reg: The signal's associated register, offset from base
> > * @idx: The signal's bit index in the register
> > * @val: The value (0 or 1) that enables the function
> > */
> > #define SIG_DESC_BIT(reg, idx, val) \
> > - { reg, BIT_MASK(idx), val, (((val) + 1) & 1) }
> > + SIG_DESC_IP_BIT(ASPEED_IP_SCU, reg, idx, val)
> > +
> > +#define SIG_DESC_IP_SET(ip, reg, idx) SIG_DESC_IP_BIT(ip, reg, idx, 1)
> >
> > /**
> > - * A further short-hand macro describing a configuration enabled with a set bit.
> > + * A further short-hand macro expanding to an SCU descriptor enabled by a set
> > + * bit.
> > *
> > - * @reg: The configuration's associated register, offset from base
> > - * @idx: The configuration's bit index in the register
> > + * @reg: The register, offset from base
> > + * @idx: The bit index in the register
> > */
> > -#define SIG_DESC_SET(reg, idx) SIG_DESC_BIT(reg, idx, 1)
> > +#define SIG_DESC_SET(reg, idx) SIG_DESC_IP_BIT(ASPEED_IP_SCU, reg, idx, 1)
> >
> > #define SIG_DESC_LIST_SYM(sig, func) sig_descs_ ## sig ## _ ## func
> > #define SIG_DESC_LIST_DECL(sig, func, ...) \
> > @@ -500,7 +514,7 @@ struct aspeed_pin_desc {
> > MS_PIN_DECL_(pin, SIG_EXPR_LIST_PTR(gpio))
> >
> > struct aspeed_pinctrl_data {
> > - struct regmap *map;
> > + struct regmap *maps[ASPEED_NR_PINMUX_IPS];
> >
> > const struct pinctrl_pin_desc *pins;
> > const unsigned int npins;
> > --
> > 2.7.4
> >
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 3/6] mfd: dt: Add bindings for the Aspeed LPC Host Controller (LPCHC)
From: Andrew Jeffery @ 2016-11-04 3:45 UTC (permalink / raw)
To: Joel Stanley
Cc: Lee Jones, Linus Walleij, Mark Rutland, Rob Herring,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CACPK8XezumurK6TStNhmTnHdhoJuZBzkJrS3iOArKz1tf1=nLw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2711 bytes --]
On Fri, 2016-11-04 at 09:36 +1030, Joel Stanley wrote:
> On Thu, Nov 3, 2016 at 1:07 AM, Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org> wrote:
> >
> > The Aspeed LPC Host Controller is presented as a syscon device to
> > arbitrate access by LPC and pinmux drivers. LPC pinmux configuration on
> > fifth generation SoCs depends on bits in both the System Control Unit
> > and the LPC Host Controller.
> >
> > Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
> > ---
> > Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt b/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> > new file mode 100644
> > index 000000000000..792651488c3d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> > @@ -0,0 +1,17 @@
> > +* Device tree bindings for the Aspeed LPC Host Controller (LPCHC)
> I had to check the data sheet for that acronym. They call the
> registers LHC. I somewhat prefer that name, but if you're happy with
> it as-is then that's fine.
I had an internal debate about this. I figured LPCHC might give a bit
more context to the acronym. I'm not unhappy with it but I wouldn't
claim I'm happy either. I will change it to LHC since you somewhat
prefer it, and it better aligns with the datasheet.
>
> I assume this is not an issue on the g4/ast2400?
Correct, we don't have the issue of pinmux needing to reach into the
LPC IO space on the AST2400. I don't think we've had anything else to
drive us to looking at the host controller space there, so I wasn't
going to add it to the bindings yet.
>
> >
> > +
> > +The LPCHC registers configure LPC behaviour between the BMC and the host
> > +system. The LPCHC also participates in pinmux requests on g5 SoCs and is
> > +therefore considered a syscon device.
> > +
> > +Required properties:
> > +- compatible: "aspeed,ast2500-lpchc", "syscon"
> > +- reg: contains offset/length value of the LPCHC memory
> > + region.
> > +
> > +Example:
> > +
> > +lpchc: lpchc@1e7890a0 {
> > + compatible = "aspeed,ast2500-lpchc", "syscon";
> > + reg = <0x1e7890a0 0xc4>;
> Where's the 0xc4 come from? I can see 9 registers, which would mean
> the length should be 0x24?
Yes, it should be 0x24. I can't even claim that 'c' is near '2'. Thanks
for catching that.
Andrew
>
> Cheers,
>
> Joel
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 5/6] arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support
From: Bjorn Andersson @ 2016-11-04 1:57 UTC (permalink / raw)
To: Jeremy McNicoll
Cc: Stephen Boyd, Andy Gross, Jeremy McNicoll,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A,
arnd-r2nGTMty4D4, mark.rutland-5wv7dgnIgG8,
michael.scott-QSEj5FYQhm4dnm+yROfE0A, Bastian K?cher
In-Reply-To: <20161104002005.GA15578-IfqqoHeSVXkD/aak0adQqVaTQe2KTcn/@public.gmane.org>
On Thu 03 Nov 17:20 PDT 2016, Jeremy McNicoll wrote:
> On Thu, Nov 03, 2016 at 04:04:29PM -0700, Bjorn Andersson wrote:
> > On Thu 03 Nov 15:42 PDT 2016, Stephen Boyd wrote:
> >
> > > On 11/03/2016 03:32 PM, Andy Gross wrote:
> > > > On Thu, Oct 27, 2016 at 05:54:01PM -0700, Jeremy McNicoll wrote:
> > > >> On 2016-10-27 5:06 PM, Stephen Boyd wrote:
> > > >>> On 10/25, Jeremy McNicoll wrote:
> > > >>>> diff --git a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> > > >>>> new file mode 100644
> > > >>>> index 0000000..2fc68c4
> > > >>>> --- /dev/null
> > > >>>> +++ b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> > > >>>> @@ -0,0 +1,40 @@
> > > >>>> +/* Copyright (c) 2015, Huawei Inc. All rights reserved.
> > > >>>> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> > > >>>> + *
> > > >>>> + * This program is free software; you can redistribute it and/or modify
> > > >>>> + * it under the terms of the GNU General Public License version 2 and
> > > >>>> + * only version 2 as published by the Free Software Foundation.
> > > >>>> + *
> > > >>>> + * This program is distributed in the hope that it will be useful,
> > > >>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > >>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > >>>> + * GNU General Public License for more details.
> > > >>>> + */
> > > >>>> +
> > > >>>> +/dts-v1/;
> > > >>>> +
> > > >>>> +#include "msm8994-v2.0.dtsi"
> > > >>>> +
> > > >>>> +/ {
> > > >>>> + model = "HUAWEI MSM8994 ANGLER rev-1.01";
> > > >>>> + compatible = "qcom,msm8994";
> > > >>> Please replace this with something more specific for the actual
> > > >>> board. Preferably with the board manufacturer vendor prefix and
> > > >>> some sort of string for the board.
> > > > Something like?
> > > >
> > > > "huawei,nexus-6P", "qcom,msm8994"
> > >
> > > Sure, except completely remove qcom,msm8994 from there.
> >
> > Why? So far we have always used the form <device>, <platform> as
> > compatible. This is even in line with ePAPR, what changed?
> >
> > Also, the compatible should be "huawei,angler" (if I read the internet
> > correctly) and the model should contain the product name
> > (Huawei Nexus 6P).
> >
>
> What about
>
> model = "Huawei Nexus 6P";
> compatible = "huawei, angler", "qcom,msm8994";
^
|
unwanted space
Apart from that, I think it looks good and in line with how we've done
the other boards.
Regards,
Bjorn
--
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 06/14] ASoC: sun4i-codec: Add support for A31 playback through headphone output
From: Chen-Yu Tsai @ 2016-11-04 1:08 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Rob Herring, Mark Rutland, Linux-ALSA,
linux-arm-kernel, linux-kernel, devicetree, linux-sunxi
In-Reply-To: <20161103173602.zchyciwj66zdibc7@lukather>
On Fri, Nov 4, 2016 at 1:36 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Thu, Nov 03, 2016 at 03:55:48PM +0800, Chen-Yu Tsai wrote:
>> +/* headphone controls */
>> +static const char * const sun6i_codec_hp_src_enum_text[] = {
>> + "DAC", "Mixer",
>> +};
>> +
>> +static SOC_ENUM_DOUBLE_DECL(sun6i_codec_hp_src_enum,
>> + SUN6I_CODEC_OM_DACA_CTRL,
>> + SUN6I_CODEC_OM_DACA_CTRL_LHPIS,
>> + SUN6I_CODEC_OM_DACA_CTRL_RHPIS,
>> + sun6i_codec_hp_src_enum_text);
>> +
>> +static const struct snd_kcontrol_new sun6i_codec_hp_src[] = {
>> + SOC_DAPM_ENUM("Headphone Source Playback Route",
>> + sun6i_codec_hp_src_enum),
>> +};
>
> What is that route exactly? A muxer?
Yup. The following is part of the widgets list later in the code:
+ /* Headphone output path */
+ SND_SOC_DAPM_MUX("Headphone Source Playback Route",
+ SND_SOC_NOPM, 0, 0, sun6i_codec_hp_src),
ChenYu
^ permalink raw reply
* [PATCH v2 11/13] Documentation: devicetree: dwc2: Add host DMA binding
From: John Youn @ 2016-11-04 0:56 UTC (permalink / raw)
To: John Youn, Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
In-Reply-To: <cover.1478220875.git.johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Add the snps,host-dma-disable binding. This controls whether to disable
DMA in host mode.
Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
---
Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 2c30a54..9472111 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -25,6 +25,7 @@ Optional properties:
Refer to phy/phy-bindings.txt for generic phy consumer properties
- dr_mode: shall be one of "host", "peripheral" and "otg"
Refer to usb/generic.txt
+- snps,host-dma-disable: disable host DMA mode.
- g-use-dma: enable dma usage in gadget driver.
- g-rx-fifo-size: size of rx fifo size in gadget mode.
- g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
--
2.10.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 v2 11/13] Documentation: devicetree: dwc2: Add host DMA binding
From: John Youn @ 2016-11-04 0:56 UTC (permalink / raw)
To: John Youn, Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
In-Reply-To: <cover.1478220875.git.johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Add the snps,host-dma-disable binding. This controls whether to disable
DMA in host mode.
Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
---
Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 2c30a54..9472111 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -25,6 +25,7 @@ Optional properties:
Refer to phy/phy-bindings.txt for generic phy consumer properties
- dr_mode: shall be one of "host", "peripheral" and "otg"
Refer to usb/generic.txt
+- snps,host-dma-disable: disable host DMA mode.
- g-use-dma: enable dma usage in gadget driver.
- g-rx-fifo-size: size of rx fifo size in gadget mode.
- g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
--
2.10.0
--
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 v2 00/13] usb: dwc2: Fix up, consolidate, and simplify driver parameters
From: John Youn @ 2016-11-04 0:55 UTC (permalink / raw)
To: John Youn, Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
Cc: Vahram Aharonyan
This patch series cleans up and simplifies the parameter handling in
the dwc2 driver so that it is easier to set these parameters and
easier to maintain the driver and support more platforms in the long
run.
The long-term goal is to remove all static and legacy parameters in
favor of autodetection and/or devicetree properties.
However, this patch series is mostly a cleanup and refactoring to
allow for this. Then, it adds the current gadget-specific properties.
And for host-mode, it adds the DMA property. Lastly, it adds the
ability to set properties from the PCI driver so that we can perform
IP validation with the HAPS platform.
Later patch series will address the other properties and also allow
for them to be set by the PCI driver via debugfs.
Tested on DWC_hsotg IP version 3.30a on Synopsys HAPS platform.
v2:
* Renamed host-dma to "snps,host-dma-disable".
* Removed params->host_dma from being set statically.
* Reverted descriptor dma params renaming (to be addressed in the
future).
* Simplified and removed unused code.
Regards,
John
John Youn (12):
usb: dwc2: Remove unnecessary kfree
usb: dwc2: Remove unused hardware parameter
usb: dwc2: Add params.c file
usb: dwc2: Declare the core params struct statically
usb: dwc2: Move parameter initialization into params.c
usb: dwc2: Remove dwc2_set_all_params function
usb: dwc2: Remove unnecessary prototypes
usb: dwc2: Rename host_rx_fifo_size hardware parameter
usb: dwc2: Move gadget settings into core_params
usb: dwc2: Rename the dma_enable parameter to host_dma
Documentation: devicetree: dwc2: Add host DMA binding
usb: dwc2: Get host DMA device properties
Vahram Aharonyan (1):
usb: dwc2: Add PCI properties
Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
drivers/usb/dwc2/Makefile | 1 +
drivers/usb/dwc2/core.c | 930 +---------------
drivers/usb/dwc2/core.h | 280 +----
drivers/usb/dwc2/core_intr.c | 6 +-
drivers/usb/dwc2/gadget.c | 95 +-
drivers/usb/dwc2/hcd.c | 193 ++--
drivers/usb/dwc2/hcd_ddma.c | 4 +-
drivers/usb/dwc2/hcd_intr.c | 48 +-
drivers/usb/dwc2/hcd_queue.c | 18 +-
drivers/usb/dwc2/params.c | 1412 ++++++++++++++++++++++++
drivers/usb/dwc2/pci.c | 19 +
drivers/usb/dwc2/platform.c | 207 +---
13 files changed, 1653 insertions(+), 1561 deletions(-)
create mode 100644 drivers/usb/dwc2/params.c
--
2.10.0
--
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
* [PATCH v2 00/13] usb: dwc2: Fix up, consolidate, and simplify driver parameters
From: John Youn @ 2016-11-04 0:55 UTC (permalink / raw)
To: John Youn, Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
Cc: Vahram Aharonyan
This patch series cleans up and simplifies the parameter handling in
the dwc2 driver so that it is easier to set these parameters and
easier to maintain the driver and support more platforms in the long
run.
The long-term goal is to remove all static and legacy parameters in
favor of autodetection and/or devicetree properties.
However, this patch series is mostly a cleanup and refactoring to
allow for this. Then, it adds the current gadget-specific properties.
And for host-mode, it adds the DMA property. Lastly, it adds the
ability to set properties from the PCI driver so that we can perform
IP validation with the HAPS platform.
Later patch series will address the other properties and also allow
for them to be set by the PCI driver via debugfs.
Tested on DWC_hsotg IP version 3.30a on Synopsys HAPS platform.
v2:
* Renamed host-dma to "snps,host-dma-disable".
* Removed params->host_dma from being set statically.
* Reverted descriptor dma params renaming (to be addressed in the
future).
* Simplified and removed unused code.
Regards,
John
John Youn (12):
usb: dwc2: Remove unnecessary kfree
usb: dwc2: Remove unused hardware parameter
usb: dwc2: Add params.c file
usb: dwc2: Declare the core params struct statically
usb: dwc2: Move parameter initialization into params.c
usb: dwc2: Remove dwc2_set_all_params function
usb: dwc2: Remove unnecessary prototypes
usb: dwc2: Rename host_rx_fifo_size hardware parameter
usb: dwc2: Move gadget settings into core_params
usb: dwc2: Rename the dma_enable parameter to host_dma
Documentation: devicetree: dwc2: Add host DMA binding
usb: dwc2: Get host DMA device properties
Vahram Aharonyan (1):
usb: dwc2: Add PCI properties
Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
drivers/usb/dwc2/Makefile | 1 +
drivers/usb/dwc2/core.c | 930 +---------------
drivers/usb/dwc2/core.h | 280 +----
drivers/usb/dwc2/core_intr.c | 6 +-
drivers/usb/dwc2/gadget.c | 95 +-
drivers/usb/dwc2/hcd.c | 193 ++--
drivers/usb/dwc2/hcd_ddma.c | 4 +-
drivers/usb/dwc2/hcd_intr.c | 48 +-
drivers/usb/dwc2/hcd_queue.c | 18 +-
drivers/usb/dwc2/params.c | 1412 ++++++++++++++++++++++++
drivers/usb/dwc2/pci.c | 19 +
drivers/usb/dwc2/platform.c | 207 +---
13 files changed, 1653 insertions(+), 1561 deletions(-)
create mode 100644 drivers/usb/dwc2/params.c
--
2.10.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 3/6] msm8994 clocks: global clock support for msm8994 SOC.
From: Jeremy McNicoll @ 2016-11-04 0:37 UTC (permalink / raw)
To: Stephen Boyd
Cc: Jeremy McNicoll, linux-arm-msm, linux-soc, devicetree, robh,
andy.gross, arnd, bjorn.andersson, mark.rutland, michael.scott
In-Reply-To: <20161028001800.GK26139@codeaurora.org>
On Thu, Oct 27, 2016 at 05:18:00PM -0700, Stephen Boyd wrote:
> On 10/25, Jeremy McNicoll wrote:
> > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> > index 0146d3c..3b78803 100644
> > --- a/drivers/clk/qcom/Kconfig
> > +++ b/drivers/clk/qcom/Kconfig
> > @@ -132,6 +132,15 @@ config MSM_MMCC_8974
> > Say Y if you want to support multimedia devices such as display,
> > graphics, video encode/decode, camera, etc.
> >
> > +config MSM_GCC_8994
> > + tristate "MSM8994 Global Clock Controller"
> > + select QCOM_GDSC
>
> But we aren't populating GDSCs so this is unnecessary right now.
>
> > + depends on COMMON_CLK_QCOM
> > + help
> > + Support for the global clock controller on msm8994 devices.
> > + Say Y if you want to use peripheral devices such as UART, SPI,
> > + i2c, USB, SD/eMMC, SATA, PCIe, etc.
>
> Is there a sata controller on 8994? Is there an emmc/sd
> controller?
>
According to this marketing slide.
https://www.qualcomm.com/products/snapdragon/processors/810
We will strip down the list to something like:
....Say Y if you want to use peripheral devices such as UART, USB, etc..
BTW - the help message was identical to the msm8996 which is currently
in tree. Hopefully that help message is accurately describes the HW.
-jeremy
> > +
> > config MSM_GCC_8996
> > tristate "MSM8996 Global Clock Controller"
> > select QCOM_GDSC
> > diff --git a/drivers/clk/qcom/gcc-msm8994.c b/drivers/clk/qcom/gcc-msm8994.c
> > new file mode 100644
> > index 0000000..39b40d4
> > --- /dev/null
> > +++ b/drivers/clk/qcom/gcc-msm8994.c
> > @@ -0,0 +1,2501 @@
> > +/* Copyright (c) 2013-2016, The Linux Foundation. All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 and
> > + * only version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/init.h>
> > +#include <linux/err.h>
> > +#include <linux/ctype.h>
> > +#include <linux/io.h>
> > +#include <linux/clk.h>
>
> Is this include used? Should probably be clk-provider instead.
>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +
> > +#include <dt-bindings/clock/qcom,gcc-msm8994.h>
> > +
> > +#include "common.h"
> > +#include "clk-regmap.h"
> > +#include "clk-pll.h"
>
> Is this include used?
>
> > +#include "clk-alpha-pll.h"
> > +#include "clk-rcg.h"
> > +#include "clk-branch.h"
> > +#include "reset.h"
> > +
> > +static struct clk_branch gcc_usb30_mock_utmi_clk = {
> > + .halt_reg = 0x03D0,
>
> Lowercase hex everywhere please.
>
> > +
> > +
> > +static void msm_gcc_8994v2_fixup(void)
> > +{
> > + ufs_axi_clk_src.freq_tbl = ftbl_ufs_axi_clk_src_v2;
> > +
> > + blsp1_qup1_spi_apps_clk_src.freq_tbl = ftbl_blspqup_spi_apps_clk_src_v2;
> > + blsp1_qup2_spi_apps_clk_src.freq_tbl = ftbl_blspqup_spi_apps_clk_src_v2;
> > + blsp1_qup3_spi_apps_clk_src.freq_tbl = ftbl_blspqup_spi_apps_clk_src_v2;
> > + blsp1_qup4_spi_apps_clk_src.freq_tbl = ftbl_blspqup_spi_apps_clk_src_v2;
> > + blsp1_qup5_spi_apps_clk_src.freq_tbl = ftbl_blspqup_spi_apps_clk_src_v2;
> > + blsp1_qup6_spi_apps_clk_src.freq_tbl = ftbl_blspqup_spi_apps_clk_src_v2;
> > + blsp2_qup1_spi_apps_clk_src.freq_tbl = ftbl_blspqup_spi_apps_clk_src_v2;
> > + blsp2_qup2_spi_apps_clk_src.freq_tbl = ftbl_blspqup_spi_apps_clk_src_v2;
> > + blsp2_qup3_spi_apps_clk_src.freq_tbl = ftbl_blspqup_spi_apps_clk_src_v2;
> > + blsp2_qup4_spi_apps_clk_src.freq_tbl = ftbl_blspqup_spi_apps_clk_src_v2;
> > + blsp2_qup5_spi_apps_clk_src.freq_tbl = ftbl_blspqup_spi_apps_clk_src_v2;
> > + blsp2_qup6_spi_apps_clk_src.freq_tbl = ftbl_blspqup_spi_apps_clk_src_v2;
>
> Please no, just assume v2.
>
> > +}
> > +
> > +static const struct regmap_config gcc_msm8994_regmap_config = {
> > + .reg_bits = 32,
> > + .reg_stride = 4,
> > + .val_bits = 32,
> > + .max_register = 0x2000,
> > + .fast_io = true,
> > +};
> > +
> > +static const struct qcom_cc_desc gcc_msm8994_desc = {
> > + .config = &gcc_msm8994_regmap_config,
> > + .clks = gcc_msm8994_clocks,
> > + .num_clks = ARRAY_SIZE(gcc_msm8994_clocks),
> > + .resets = NULL,
> > + .num_resets = 0,
> > + .gdscs = NULL,
> > + .num_gdscs = 0,
>
> We don't have to be explicit. Does this even work? I thought
> common.c expected there to be a reset list? I guess we would have
> a reset controller with no resets. Should be possible to grab
> some of the resets out of the android driver though.
>
> > +};
> > +
> > +static const struct of_device_id gcc_msm8994_match_table[] = {
> > + { .compatible = "qcom,gcc-8994" },
> > + { .compatible = "qcom,gcc-8994v2" },
>
> Please add msm and drop the v2 one.
>
> > + {}
> > +}
> > +
> > +MODULE_DEVICE_TABLE(of, gcc_msm8994_match_table);
> > +
> > +static int gcc_msm8994_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct clk *clk;
> > + const char *compat = NULL;
> > + int compatlen = 0;
> > + bool is_v2 = false;
> > +
> > + clk = devm_clk_register(dev, &xo.hw);
> > + if (IS_ERR(clk))
> > + return PTR_ERR(clk);
> > +
> > + compat = of_get_property(pdev->dev.of_node, "compatible", &compatlen);
> > + if (!compat || (compatlen <= 0))
> > + return -EINVAL;
> > +
> > + is_v2 = !strcmp(compat, "qcom,gcc-8994v2");
> > + if (is_v2)
> > + msm_gcc_8994v2_fixup();
>
> This should simplify greatly.
>
> > +
> > + return qcom_cc_probe(pdev, &gcc_msm8994_desc);
> > +}
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 5/6] arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support
From: Jeremy McNicoll @ 2016-11-04 0:20 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Stephen Boyd, Andy Gross, Jeremy McNicoll,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A,
arnd-r2nGTMty4D4, mark.rutland-5wv7dgnIgG8,
michael.scott-QSEj5FYQhm4dnm+yROfE0A, Bastian K?cher
In-Reply-To: <20161103230429.GW25787@tuxbot>
On Thu, Nov 03, 2016 at 04:04:29PM -0700, Bjorn Andersson wrote:
> On Thu 03 Nov 15:42 PDT 2016, Stephen Boyd wrote:
>
> > On 11/03/2016 03:32 PM, Andy Gross wrote:
> > > On Thu, Oct 27, 2016 at 05:54:01PM -0700, Jeremy McNicoll wrote:
> > >> On 2016-10-27 5:06 PM, Stephen Boyd wrote:
> > >>> On 10/25, Jeremy McNicoll wrote:
> > >>>> diff --git a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> > >>>> new file mode 100644
> > >>>> index 0000000..2fc68c4
> > >>>> --- /dev/null
> > >>>> +++ b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> > >>>> @@ -0,0 +1,40 @@
> > >>>> +/* Copyright (c) 2015, Huawei Inc. All rights reserved.
> > >>>> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> > >>>> + *
> > >>>> + * This program is free software; you can redistribute it and/or modify
> > >>>> + * it under the terms of the GNU General Public License version 2 and
> > >>>> + * only version 2 as published by the Free Software Foundation.
> > >>>> + *
> > >>>> + * This program is distributed in the hope that it will be useful,
> > >>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > >>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > >>>> + * GNU General Public License for more details.
> > >>>> + */
> > >>>> +
> > >>>> +/dts-v1/;
> > >>>> +
> > >>>> +#include "msm8994-v2.0.dtsi"
> > >>>> +
> > >>>> +/ {
> > >>>> + model = "HUAWEI MSM8994 ANGLER rev-1.01";
> > >>>> + compatible = "qcom,msm8994";
> > >>> Please replace this with something more specific for the actual
> > >>> board. Preferably with the board manufacturer vendor prefix and
> > >>> some sort of string for the board.
> > > Something like?
> > >
> > > "huawei,nexus-6P", "qcom,msm8994"
> >
> > Sure, except completely remove qcom,msm8994 from there.
>
> Why? So far we have always used the form <device>, <platform> as
> compatible. This is even in line with ePAPR, what changed?
>
> Also, the compatible should be "huawei,angler" (if I read the internet
> correctly) and the model should contain the product name
> (Huawei Nexus 6P).
>
What about
model = "Huawei Nexus 6P";
compatible = "huawei, angler", "qcom,msm8994";
?
Obviously I will update the Nexus5X to look similar.
-jeremy
> Regards,
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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
* [PATCH v2 1/3] dt-bindings: firmware: scm: Add MSM8996 DT bindings
From: Sarangdhar Joshi @ 2016-11-04 0:10 UTC (permalink / raw)
To: Andy Gross, David Brown, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon
Cc: Sarangdhar Joshi, linux-arm-msm, linux-soc, devicetree,
linux-arm-kernel, linux-kernel, Bjorn Andersson, Jordan Crouse,
Stephen Boyd, Trilok Soni
In-Reply-To: <1478218237-1737-1-git-send-email-spjoshi@codeaurora.org>
Add SCM DT bindings for Qualcomm's MSM8996 platform.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sarangdhar Joshi <spjoshi@codeaurora.org>
---
Documentation/devicetree/bindings/firmware/qcom,scm.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.txt b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
index 3b4436e..20f26fb 100644
--- a/Documentation/devicetree/bindings/firmware/qcom,scm.txt
+++ b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
@@ -10,8 +10,10 @@ Required properties:
* "qcom,scm-apq8064" for APQ8064 platforms
* "qcom,scm-msm8660" for MSM8660 platforms
* "qcom,scm-msm8690" for MSM8690 platforms
+ * "qcom,scm-msm8996" for MSM8996 platforms
* "qcom,scm" for later processors (MSM8916, APQ8084, MSM8974, etc)
- clocks: One to three clocks may be required based on compatible.
+ * No clock required for "qcom,scm-msm8996"
* Only core clock required for "qcom,scm-apq8064", "qcom,scm-msm8660", and "qcom,scm-msm8960"
* Core, iface, and bus clocks required for "qcom,scm"
- clock-names: Must contain "core" for the core clock, "iface" for the interface
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH v2 0/3] Remove clocks dependency from SCM driver
From: Sarangdhar Joshi @ 2016-11-04 0:10 UTC (permalink / raw)
To: Andy Gross, David Brown, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Bjorn Andersson
Cc: Sarangdhar Joshi, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stephen Boyd, Trilok Soni
On earlier chiptsets (APQ8064, MSM8660, MSM8690, MSM8916,
APQ8084, MSM8974) crypto operations of TZ were depends on crypto
clocks controlled by users/clients. However on MSM8996 crypto clocks
control is handled internally in TZ itself. The current series of
patches handle this clock dependency in SCM driver.
Changes since v1:
- Added Rob's Acked-by
- Removed of_device_is_compatible check from probe (Stephen)
- Modified typecast to take care of 32-bit pointer
Sarangdhar Joshi (3):
dt-bindings: firmware: scm: Add MSM8996 DT bindings
firmware: qcom: scm: Remove core, iface and bus clocks dependency
firmware: qcom: scm: Return PTR_ERR when devm_clk_get fails
.../devicetree/bindings/firmware/qcom,scm.txt | 2 +
drivers/firmware/qcom_scm.c | 49 ++++++++++++++++------
2 files changed, 39 insertions(+), 12 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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 3/6] msm8994 clocks: global clock support for msm8994 SOC.
From: Jeremy McNicoll @ 2016-11-03 23:43 UTC (permalink / raw)
To: Mark Rutland, mail-LJ92rlH3Dns
Cc: Jeremy McNicoll, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A,
andy.gross-QSEj5FYQhm4dnm+yROfE0A, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
arnd-r2nGTMty4D4, bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
michael.scott-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <20161103224234.GB30414@remoulade>
On Thu, Nov 03, 2016 at 10:42:34PM +0000, Mark Rutland wrote:
> On Tue, Oct 25, 2016 at 04:16:58AM -0700, Jeremy McNicoll wrote:
> > +static const struct qcom_cc_desc gcc_msm8994_desc = {
> > + .config = &gcc_msm8994_regmap_config,
> > + .clks = gcc_msm8994_clocks,
> > + .num_clks = ARRAY_SIZE(gcc_msm8994_clocks),
> > + .resets = NULL,
> > + .num_resets = 0,
> > + .gdscs = NULL,
> > + .num_gdscs = 0,
> > +};
> > +
> > +static const struct of_device_id gcc_msm8994_match_table[] = {
> > + { .compatible = "qcom,gcc-8994" },
> > + { .compatible = "qcom,gcc-8994v2" },
> > + {}
> > +}
> > +
> > +MODULE_DEVICE_TABLE(of, gcc_msm8994_match_table);
> > +
> > +static int gcc_msm8994_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct clk *clk;
> > + const char *compat = NULL;
> > + int compatlen = 0;
> > + bool is_v2 = false;
> > +
> > + clk = devm_clk_register(dev, &xo.hw);
> > + if (IS_ERR(clk))
> > + return PTR_ERR(clk);
> > +
> > + compat = of_get_property(pdev->dev.of_node, "compatible", &compatlen);
> > + if (!compat || (compatlen <= 0))
> > + return -EINVAL;
>
> It is almost never correct to use of_get_property(). Please avoid it.
>
Hi Bastian,
Mark is basically suggesting that instead of using of_get_property() you
should be using something like of_device_compatible_match, or
of_device_is_compatible. This should simplify the code a little bit
too.
Unless I am misunderstanding Mark's comment.
-jeremy
> > +
> > + is_v2 = !strcmp(compat, "qcom,gcc-8994v2");
> > + if (is_v2)
> > + msm_gcc_8994v2_fixup();
>
> Either put some data in the of_device_id tables, and use that to determine what
> to do here, or use of_device_is_compatible() or of_match_node() to check the
> string.
>
> Thanks,
> Mark.
--
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 4/6] pinctrl: aspeed: Read and write bits in LPCHC and GFX controllers
From: Joel Stanley @ 2016-11-03 23:24 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Lee Jones, Linus Walleij, Mark Rutland, Rob Herring, linux-gpio,
linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <1478097481-14895-5-git-send-email-andrew@aj.id.au>
On Thu, Nov 3, 2016 at 1:07 AM, Andrew Jeffery <andrew@aj.id.au> wrote:
> The System Control Unit IP block in the Aspeed SoCs is typically where
> the pinmux configuration is found, but not always. A number of pins
> depend on state in one of LPC Host Control (LPCHC) or SoC Display
> Controller (GFX) IP blocks, so the Aspeed pinmux drivers should have the
> means to adjust these as necessary.
>
> We use syscon to cast a regmap over the GFX and LPCHCR blocks, which is
> used as an arbitration layer between the relevant driver and the pinctrl
> subsystem. The regmaps are then exposed to the SoC-specific pinctrl
> drivers by phandles in the devicetree, and are selected during a mux
> request by querying a new 'ip' member in struct aspeed_sig_desc.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
I like this a lot more than the first go. Good work.
Some minor comments below.
> ---
> Since v1:
>
> The change is now proactive: instead of reporting that we need to flip bits in
> controllers we can't access, the patch provides access via regmaps for the
> relevant controllers. The implementation also splits out the IP block ID into
> its own variable rather than packing the value into the upper bits of the reg
> member of struct aspeed_sig_desc. This drives some churn in the diff, but I've
> tried to minimise it.
>
> .../devicetree/bindings/pinctrl/pinctrl-aspeed.txt | 50 +++++++++++++---
> drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c | 18 +++---
> drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 39 ++++++++++---
> drivers/pinctrl/aspeed/pinctrl-aspeed.c | 66 +++++++++++++---------
> drivers/pinctrl/aspeed/pinctrl-aspeed.h | 32 ++++++++---
> 5 files changed, 144 insertions(+), 61 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
> index 2ad18c4ea55c..115b0cce6c1c 100644
> --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
> @@ -4,12 +4,19 @@ Aspeed Pin Controllers
> The Aspeed SoCs vary in functionality inside a generation but have a common mux
> device register layout.
>
> -Required properties:
> -- compatible : Should be any one of the following:
> - "aspeed,ast2400-pinctrl"
> - "aspeed,g4-pinctrl"
> - "aspeed,ast2500-pinctrl"
> - "aspeed,g5-pinctrl"
> +Required properties for g4:
> +- compatible : Should be any one of the following:
> + "aspeed,ast2400-pinctrl"
> + "aspeed,g4-pinctrl"
> +
> +Required properties for g5:
> +- compatible : Should be any one of the following:
> + "aspeed,ast2500-pinctrl"
> + "aspeed,g5-pinctrl"
> +
> +- aspeed,external-nodes: A cell of phandles to external controller nodes:
> + 0: compatible with "aspeed,ast2500-gfx", "syscon"
> + 1: compatible with "aspeed,ast2500-lpchc", "syscon"
>
> The pin controller node should be a child of a syscon node with the required
> property:
> @@ -47,7 +54,7 @@ RGMII1 RGMII2 RMII1 RMII2 SD1 SPI1 SPI1DEBUG SPI1PASSTHRU TIMER4 TIMER5 TIMER6
> TIMER7 TIMER8 VGABIOSROM
>
>
> -Examples:
> +g4 Example:
>
> syscon: scu@1e6e2000 {
> compatible = "syscon", "simple-mfd";
> @@ -63,5 +70,34 @@ syscon: scu@1e6e2000 {
> };
> };
>
> +g5 Example:
> +
> +apb {
> + gfx: display@1e6e6000 {
> + compatible = "aspeed,ast2500-gfx", "syscon";
> + reg = <0x1e6e6000 0x1000>;
> + };
> +
> + lpchc: lpchc@1e7890a0 {
> + compatible = "aspeed,ast2500-lpchc", "syscon";
> + reg = <0x1e7890a0 0xc4>;
> + };
> +
> + syscon: scu@1e6e2000 {
> + compatible = "syscon", "simple-mfd";
> + reg = <0x1e6e2000 0x1a8>;
> +
> + pinctrl: pinctrl {
> + compatible = "aspeed,g5-pinctrl";
> + aspeed,external-nodes = <&gfx, &lpchc>;
> +
> + pinctrl_i2c3_default: i2c3_default {
> + function = "I2C3";
> + groups = "I2C3";
> + };
> + };
> + };
> +};
> +
> Please refer to pinctrl-bindings.txt in this directory for details of the
> common pinctrl bindings used by client devices.
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
> index a21b071ff290..558bd102416c 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
> @@ -292,7 +292,7 @@ SSSF_PIN_DECL(U18, GPIOG7, FLWP, SIG_DESC_SET(SCU84, 7));
> #define UART6_DESC SIG_DESC_SET(SCU90, 7)
> #define ROM16_DESC SIG_DESC_SET(SCU90, 6)
> #define FLASH_WIDE SIG_DESC_SET(HW_STRAP1, 4)
> -#define BOOT_SRC_NOR { HW_STRAP1, GENMASK(1, 0), 0, 0 }
> +#define BOOT_SRC_NOR { ASPEED_IP_SCU, HW_STRAP1, GENMASK(1, 0), 0, 0 }
>
> #define A8 56
> SIG_EXPR_DECL(ROMD8, ROM16, ROM16_DESC);
> @@ -418,9 +418,9 @@ FUNC_GROUP_DECL(I2C8, G5, F3);
> #define U1 88
> SSSF_PIN_DECL(U1, GPIOL0, NCTS1, SIG_DESC_SET(SCU84, 16));
>
> -#define VPI18_DESC { SCU90, GENMASK(5, 4), 1, 0 }
> -#define VPI24_DESC { SCU90, GENMASK(5, 4), 2, 0 }
> -#define VPI30_DESC { SCU90, GENMASK(5, 4), 3, 0 }
> +#define VPI18_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 1, 0 }
> +#define VPI24_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 2, 0 }
> +#define VPI30_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 3, 0 }
>
> #define T5 89
> #define T5_DESC SIG_DESC_SET(SCU84, 17)
> @@ -641,11 +641,11 @@ SSSF_PIN_DECL(Y22, GPIOR2, ROMCS3, SIG_DESC_SET(SCU88, 26));
> #define U19 139
> SSSF_PIN_DECL(U19, GPIOR3, ROMCS4, SIG_DESC_SET(SCU88, 27));
>
> -#define VPOOFF0_DESC { SCU94, GENMASK(1, 0), 0, 0 }
> -#define VPO12_DESC { SCU94, GENMASK(1, 0), 1, 0 }
> -#define VPO24_DESC { SCU94, GENMASK(1, 0), 2, 0 }
> -#define VPOOFF1_DESC { SCU94, GENMASK(1, 0), 3, 0 }
> -#define VPO_OFF_12 { SCU94, 0x2, 0, 0 }
> +#define VPOOFF0_DESC { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 0, 0 }
> +#define VPO12_DESC { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 1, 0 }
> +#define VPO24_DESC { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 2, 0 }
> +#define VPOOFF1_DESC { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 3, 0 }
> +#define VPO_OFF_12 { ASPEED_IP_SCU, SCU94, 0x2, 0, 0 }
> #define VPO_24_OFF SIG_DESC_SET(SCU94, 1)
>
> #define V21 140
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> index 87b46390b695..99c4fa9bf861 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> @@ -10,6 +10,7 @@
> #include <linux/init.h>
> #include <linux/io.h>
> #include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> #include <linux/mutex.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> @@ -26,8 +27,8 @@
>
> #define ASPEED_G5_NR_PINS 228
>
> -#define COND1 { SCU90, BIT(6), 0, 0 }
> -#define COND2 { SCU94, GENMASK(1, 0), 0, 0 }
> +#define COND1 { ASPEED_IP_SCU, SCU90, BIT(6), 0, 0 }
> +#define COND2 { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 0, 0 }
>
> #define B14 0
> SSSF_PIN_DECL(B14, GPIOA0, MAC1LINK, SIG_DESC_SET(SCU80, 0));
> @@ -186,9 +187,12 @@ MS_PIN_DECL(C20, GPIOE1, NDCD3, GPIE0OUT);
>
> FUNC_GROUP_DECL(GPIE0, B20, C20);
>
> -#define SPI1_DESC { HW_STRAP1, GENMASK(13, 12), 1, 0 }
> -#define SPI1DEBUG_DESC { HW_STRAP1, GENMASK(13, 12), 2, 0 }
> -#define SPI1PASSTHRU_DESC { HW_STRAP1, GENMASK(13, 12), 3, 0 }
> +#define SPI1_DESC \
> + { ASPEED_IP_SCU, HW_STRAP1, GENMASK(13, 12), 1, 0 }
> +#define SPI1DEBUG_DESC \
> + { ASPEED_IP_SCU, HW_STRAP1, GENMASK(13, 12), 2, 0 }
> +#define SPI1PASSTHRU_DESC \
> + { ASPEED_IP_SCU, HW_STRAP1, GENMASK(13, 12), 3, 0 }
>
> #define C18 64
> SIG_EXPR_DECL(SYSCS, SPI1DEBUG, COND1, SPI1DEBUG_DESC);
> @@ -325,10 +329,11 @@ SS_PIN_DECL(R1, GPIOK7, SDA8);
>
> FUNC_GROUP_DECL(I2C8, P2, R1);
>
> -#define VPIOFF0_DESC { SCU90, GENMASK(5, 4), 0, 0 }
> -#define VPIOFF1_DESC { SCU90, GENMASK(5, 4), 1, 0 }
> -#define VPI24_DESC { SCU90, GENMASK(5, 4), 2, 0 }
> -#define VPIRSVD_DESC { SCU90, GENMASK(5, 4), 3, 0 }
> +#define VPIOFF0_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 0, 0 }
> +#define VPIOFF1_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 1, 0 }
> +#define VPI24_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 2, 0 }
> +#define VPIRSVD_DESC { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 3, 0 }
> +
>
> #define V2 104
> #define V2_DESC SIG_DESC_SET(SCU88, 0)
> @@ -848,10 +853,26 @@ static struct pinctrl_desc aspeed_g5_pinctrl_desc = {
> static int aspeed_g5_pinctrl_probe(struct platform_device *pdev)
> {
> int i;
> + struct regmap **map;
> + struct device_node *node;
>
> for (i = 0; i < ARRAY_SIZE(aspeed_g5_pins); i++)
> aspeed_g5_pins[i].number = i;
>
> + map = &aspeed_g5_pinctrl_data.maps[ASPEED_IP_GFX];
> + node = of_parse_phandle(pdev->dev.of_node, "aspeed,external-nodes", 0);
> + *map = syscon_node_to_regmap(node);
I think you can use syscon_regmap_lookup_by_phandle to replace both of
these lines.
> + of_node_put(node);
> + if (IS_ERR(*map))
> + return PTR_ERR(*map);
Do we want to fail, or warn and continue?
The sequence is a bit messy. How about:
struct regmap *map;
map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
"aspeed,external-nodes");
if (IS_ERR(map))
return PTR_ERR(map);
aspeed_g5_pinctrl_data.maps[ASPEED_IP_GFX] = map;
> +
> + map = &aspeed_g5_pinctrl_data.maps[ASPEED_IP_LPCHC];
> + node = of_parse_phandle(pdev->dev.of_node, "aspeed,external-nodes", 1);
> + *map = syscon_node_to_regmap(node);
> + of_node_put(node);
> + if (IS_ERR(*map))
> + return PTR_ERR(*map);
> +
Same comments as above.
> return aspeed_pinctrl_probe(pdev, &aspeed_g5_pinctrl_desc,
> &aspeed_g5_pinctrl_data);
> }
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> index 49aeba912531..23586aac7a5a 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> @@ -14,6 +14,12 @@
> #include "../core.h"
> #include "pinctrl-aspeed.h"
>
> +static const char *const aspeed_pinmux_ips[] = {
> + [ASPEED_IP_SCU] = "SCU",
> + [ASPEED_IP_GFX] = "GFX",
> + [ASPEED_IP_LPCHC] = "LHCR",
We've got both LPCHC and LHCR here. As I said when commenting on the
regmap bindings, I like LHC(R) better.
> +};
> +
> int aspeed_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
> {
> struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
> @@ -78,7 +84,8 @@ int aspeed_pinmux_get_fn_groups(struct pinctrl_dev *pctldev,
> static inline void aspeed_sig_desc_print_val(
> const struct aspeed_sig_desc *desc, bool enable, u32 rv)
What's a rv? Perhaps "reg" or "value"?
> {
> - pr_debug("SCU%x[0x%08x]=0x%x, got 0x%x from 0x%08x\n", desc->reg,
> + pr_debug("Want %s%X[0x%08X]=0x%X, got 0x%X from 0x%08X\n",
> + aspeed_pinmux_ips[desc->ip], desc->reg,
> desc->mask, enable ? desc->enable : desc->disable,
> (rv & desc->mask) >> __ffs(desc->mask), rv);
> }
> @@ -88,7 +95,7 @@ static inline void aspeed_sig_desc_print_val(
> *
> * @desc: The signal descriptor of interest
> * @enabled: True to query the enabled state, false to query disabled state
> - * @regmap: The SCU regmap instance
> + * @regmap: The IP block's regmap instance
> *
> * @return True if the descriptor's bitfield is configured to the state
> * selected by @enabled, false otherwise
> @@ -119,7 +126,7 @@ static bool aspeed_sig_desc_eval(const struct aspeed_sig_desc *desc,
> *
> * @expr: An expression controlling the signal for a mux function on a pin
> * @enabled: True to query the enabled state, false to query disabled state
> - * @regmap: The SCU regmap instance
> + * @maps: The list of regmap instances
> *
> * @return True if the expression composed by @enabled evaluates true, false
> * otherwise
> @@ -136,15 +143,16 @@ static bool aspeed_sig_desc_eval(const struct aspeed_sig_desc *desc,
> * either condition as required.
> */
> static bool aspeed_sig_expr_eval(const struct aspeed_sig_expr *expr,
> - bool enabled, struct regmap *map)
> + bool enabled, struct regmap * const *maps)
> {
> int i;
>
> for (i = 0; i < expr->ndescs; i++) {
> const struct aspeed_sig_desc *desc = &expr->descs[i];
>
> - if (!aspeed_sig_desc_eval(desc, enabled, map))
> + if (!aspeed_sig_desc_eval(desc, enabled, maps[desc->ip]))
> return false;
> +
> }
>
> return true;
> @@ -158,12 +166,12 @@ static bool aspeed_sig_expr_eval(const struct aspeed_sig_expr *expr,
> * configured
> * @enable: true to enable an function's signal through a pin's signal
> * expression, false to disable the function's signal
> - * @map: The SCU's regmap instance for pinmux register access.
> + * @maps: The list of regmap instances for pinmux register access.
> *
> * @return true if the expression is configured as requested, false otherwise
> */
> static bool aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
> - bool enable, struct regmap *map)
> + bool enable, struct regmap * const *maps)
> {
> int i;
>
> @@ -171,6 +179,7 @@ static bool aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
> bool ret;
> const struct aspeed_sig_desc *desc = &expr->descs[i];
> u32 pattern = enable ? desc->enable : desc->disable;
> + u32 val = (pattern << __ffs(desc->mask));
>
> /*
> * Strap registers are configured in hardware or by early-boot
> @@ -179,48 +188,49 @@ static bool aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
> * deconfigured and is the reason we re-evaluate after writing
> * all descriptor bits.
> */
> - if (desc->reg == HW_STRAP1 || desc->reg == HW_STRAP2)
> + if ((desc->reg == HW_STRAP1 || desc->reg == HW_STRAP2) &&
> + desc->ip == ASPEED_IP_SCU)
> continue;
>
> - ret = regmap_update_bits(map, desc->reg, desc->mask,
> - pattern << __ffs(desc->mask)) == 0;
> + ret = regmap_update_bits(maps[desc->ip], desc->reg,
> + desc->mask, val) == 0;
>
> if (!ret)
> return ret;
> }
>
> - return aspeed_sig_expr_eval(expr, enable, map);
> + return aspeed_sig_expr_eval(expr, enable, maps);
> }
>
> static bool aspeed_sig_expr_enable(const struct aspeed_sig_expr *expr,
> - struct regmap *map)
> + struct regmap * const *maps)
> {
> - if (aspeed_sig_expr_eval(expr, true, map))
> + if (aspeed_sig_expr_eval(expr, true, maps))
> return true;
>
> - return aspeed_sig_expr_set(expr, true, map);
> + return aspeed_sig_expr_set(expr, true, maps);
> }
>
> static bool aspeed_sig_expr_disable(const struct aspeed_sig_expr *expr,
> - struct regmap *map)
> + struct regmap * const *maps)
> {
> - if (!aspeed_sig_expr_eval(expr, true, map))
> + if (!aspeed_sig_expr_eval(expr, true, maps))
> return true;
>
> - return aspeed_sig_expr_set(expr, false, map);
> + return aspeed_sig_expr_set(expr, false, maps);
> }
>
> /**
> * Disable a signal on a pin by disabling all provided signal expressions.
> *
> * @exprs: The list of signal expressions (from a priority level on a pin)
> - * @map: The SCU's regmap instance for pinmux register access.
> + * @maps: The list of regmap instances for pinmux register access.
> *
> * @return true if all expressions in the list are successfully disabled, false
> * otherwise
> */
> static bool aspeed_disable_sig(const struct aspeed_sig_expr **exprs,
> - struct regmap *map)
> + struct regmap * const *maps)
> {
> bool disabled = true;
>
> @@ -230,7 +240,7 @@ static bool aspeed_disable_sig(const struct aspeed_sig_expr **exprs,
> while (*exprs) {
> bool ret;
>
> - ret = aspeed_sig_expr_disable(*exprs, map);
> + ret = aspeed_sig_expr_disable(*exprs, maps);
> disabled = disabled && ret;
>
> exprs++;
> @@ -343,6 +353,8 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
> const struct aspeed_sig_expr **funcs;
> const struct aspeed_sig_expr ***prios;
>
> + pr_debug("Muxing pin %d for %s\n", pin, pfunc->name);
> +
> if (!pdesc)
> return -EINVAL;
>
> @@ -358,7 +370,7 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
> if (expr)
> break;
>
> - if (!aspeed_disable_sig(funcs, pdata->map))
> + if (!aspeed_disable_sig(funcs, pdata->maps))
> return -EPERM;
>
> prios++;
> @@ -377,7 +389,7 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
> return -ENXIO;
> }
>
> - if (!aspeed_sig_expr_enable(expr, pdata->map))
> + if (!aspeed_sig_expr_enable(expr, pdata->maps))
> return -EPERM;
> }
>
> @@ -432,7 +444,7 @@ int aspeed_gpio_request_enable(struct pinctrl_dev *pctldev,
> if (aspeed_gpio_in_exprs(funcs))
> break;
>
> - if (!aspeed_disable_sig(funcs, pdata->map))
> + if (!aspeed_disable_sig(funcs, pdata->maps))
> return -EPERM;
>
> prios++;
> @@ -462,7 +474,7 @@ int aspeed_gpio_request_enable(struct pinctrl_dev *pctldev,
> * If GPIO is not the lowest priority signal type, assume there is only
> * one expression defined to enable the GPIO function
> */
> - if (!aspeed_sig_expr_enable(expr, pdata->map))
> + if (!aspeed_sig_expr_enable(expr, pdata->maps))
> return -EPERM;
>
> return 0;
> @@ -481,10 +493,10 @@ int aspeed_pinctrl_probe(struct platform_device *pdev,
> return -ENODEV;
> }
>
> - pdata->map = syscon_node_to_regmap(parent->of_node);
> - if (IS_ERR(pdata->map)) {
> + pdata->maps[ASPEED_IP_SCU] = syscon_node_to_regmap(parent->of_node);
> + if (IS_ERR(pdata->maps[ASPEED_IP_SCU])) {
> dev_err(&pdev->dev, "No regmap for syscon pincontroller parent\n");
> - return PTR_ERR(pdata->map);
> + return PTR_ERR(pdata->maps[ASPEED_IP_SCU]);
> }
>
> pctl = pinctrl_register(pdesc, &pdev->dev, pdata);
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.h b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
> index 3e72ef8c54bf..727728b86c07 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.h
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
> @@ -232,6 +232,11 @@
> * group.
> */
>
> +#define ASPEED_IP_SCU 0
> +#define ASPEED_IP_GFX 1
> +#define ASPEED_IP_LPCHC 2
> +#define ASPEED_NR_PINMUX_IPS 3
> +
> /*
> * The "Multi-function Pins Mapping and Control" table in the SoC datasheet
> * references registers by the device/offset mnemonic. The register macros
> @@ -261,7 +266,9 @@
> * A signal descriptor, which describes the register, bits and the
> * enable/disable values that should be compared or written.
> *
> - * @reg: The register offset from base in bytes
> + * @ip: The IP block identifier, used as an index into the regmap array in
> + * struct aspeed_pinctrl_data
> + * @reg: The register offset with respect to the base address of the IP block
> * @mask: The mask to apply to the register. The lowest set bit of the mask is
> * used to derive the shift value.
> * @enable: The value that enables the function. Value should be in the LSBs,
> @@ -270,6 +277,7 @@
> * LSBs, not at the position of the mask.
> */
> struct aspeed_sig_desc {
> + unsigned int ip;
> unsigned int reg;
> u32 mask;
> u32 enable;
> @@ -313,24 +321,30 @@ struct aspeed_pin_desc {
>
> /* Macro hell */
>
> +#define SIG_DESC_IP_BIT(ip, reg, idx, val) \
> + { ip, reg, BIT_MASK(idx), val, (((val) + 1) & 1) }
> +
> /**
> - * Short-hand macro for describing a configuration enabled by the state of one
> - * bit. The disable value is derived.
> + * Short-hand macro for describing an SCU descriptor enabled by the state of
> + * one bit. The disable value is derived.
> *
> * @reg: The signal's associated register, offset from base
> * @idx: The signal's bit index in the register
> * @val: The value (0 or 1) that enables the function
> */
> #define SIG_DESC_BIT(reg, idx, val) \
> - { reg, BIT_MASK(idx), val, (((val) + 1) & 1) }
> + SIG_DESC_IP_BIT(ASPEED_IP_SCU, reg, idx, val)
> +
> +#define SIG_DESC_IP_SET(ip, reg, idx) SIG_DESC_IP_BIT(ip, reg, idx, 1)
>
> /**
> - * A further short-hand macro describing a configuration enabled with a set bit.
> + * A further short-hand macro expanding to an SCU descriptor enabled by a set
> + * bit.
> *
> - * @reg: The configuration's associated register, offset from base
> - * @idx: The configuration's bit index in the register
> + * @reg: The register, offset from base
> + * @idx: The bit index in the register
> */
> -#define SIG_DESC_SET(reg, idx) SIG_DESC_BIT(reg, idx, 1)
> +#define SIG_DESC_SET(reg, idx) SIG_DESC_IP_BIT(ASPEED_IP_SCU, reg, idx, 1)
>
> #define SIG_DESC_LIST_SYM(sig, func) sig_descs_ ## sig ## _ ## func
> #define SIG_DESC_LIST_DECL(sig, func, ...) \
> @@ -500,7 +514,7 @@ struct aspeed_pin_desc {
> MS_PIN_DECL_(pin, SIG_EXPR_LIST_PTR(gpio))
>
> struct aspeed_pinctrl_data {
> - struct regmap *map;
> + struct regmap *maps[ASPEED_NR_PINMUX_IPS];
>
> const struct pinctrl_pin_desc *pins;
> const unsigned int npins;
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v2 3/6] mfd: dt: Add bindings for the Aspeed LPC Host Controller (LPCHC)
From: Joel Stanley @ 2016-11-03 23:06 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Lee Jones, Linus Walleij, Mark Rutland, Rob Herring, linux-gpio,
linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <1478097481-14895-4-git-send-email-andrew@aj.id.au>
On Thu, Nov 3, 2016 at 1:07 AM, Andrew Jeffery <andrew@aj.id.au> wrote:
> The Aspeed LPC Host Controller is presented as a syscon device to
> arbitrate access by LPC and pinmux drivers. LPC pinmux configuration on
> fifth generation SoCs depends on bits in both the System Control Unit
> and the LPC Host Controller.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt b/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> new file mode 100644
> index 000000000000..792651488c3d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> @@ -0,0 +1,17 @@
> +* Device tree bindings for the Aspeed LPC Host Controller (LPCHC)
I had to check the data sheet for that acronym. They call the
registers LHC. I somewhat prefer that name, but if you're happy with
it as-is then that's fine.
I assume this is not an issue on the g4/ast2400?
> +
> +The LPCHC registers configure LPC behaviour between the BMC and the host
> +system. The LPCHC also participates in pinmux requests on g5 SoCs and is
> +therefore considered a syscon device.
> +
> +Required properties:
> +- compatible: "aspeed,ast2500-lpchc", "syscon"
> +- reg: contains offset/length value of the LPCHC memory
> + region.
> +
> +Example:
> +
> +lpchc: lpchc@1e7890a0 {
> + compatible = "aspeed,ast2500-lpchc", "syscon";
> + reg = <0x1e7890a0 0xc4>;
Where's the 0xc4 come from? I can see 9 registers, which would mean
the length should be 0x24?
Cheers,
Joel
^ permalink raw reply
* Re: [PATCH 5/6] arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support
From: Bjorn Andersson @ 2016-11-03 23:04 UTC (permalink / raw)
To: Stephen Boyd
Cc: Andy Gross, Jeremy McNicoll, Jeremy McNicoll, linux-arm-msm,
linux-soc, devicetree, robh, arnd, mark.rutland, michael.scott,
Bastian K?cher
In-Reply-To: <9e0555fa-d689-e26e-feba-9daa815eb1c5@codeaurora.org>
On Thu 03 Nov 15:42 PDT 2016, Stephen Boyd wrote:
> On 11/03/2016 03:32 PM, Andy Gross wrote:
> > On Thu, Oct 27, 2016 at 05:54:01PM -0700, Jeremy McNicoll wrote:
> >> On 2016-10-27 5:06 PM, Stephen Boyd wrote:
> >>> On 10/25, Jeremy McNicoll wrote:
> >>>> diff --git a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> >>>> new file mode 100644
> >>>> index 0000000..2fc68c4
> >>>> --- /dev/null
> >>>> +++ b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> >>>> @@ -0,0 +1,40 @@
> >>>> +/* Copyright (c) 2015, Huawei Inc. All rights reserved.
> >>>> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> >>>> + *
> >>>> + * This program is free software; you can redistribute it and/or modify
> >>>> + * it under the terms of the GNU General Public License version 2 and
> >>>> + * only version 2 as published by the Free Software Foundation.
> >>>> + *
> >>>> + * This program is distributed in the hope that it will be useful,
> >>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >>>> + * GNU General Public License for more details.
> >>>> + */
> >>>> +
> >>>> +/dts-v1/;
> >>>> +
> >>>> +#include "msm8994-v2.0.dtsi"
> >>>> +
> >>>> +/ {
> >>>> + model = "HUAWEI MSM8994 ANGLER rev-1.01";
> >>>> + compatible = "qcom,msm8994";
> >>> Please replace this with something more specific for the actual
> >>> board. Preferably with the board manufacturer vendor prefix and
> >>> some sort of string for the board.
> > Something like?
> >
> > "huawei,nexus-6P", "qcom,msm8994"
>
> Sure, except completely remove qcom,msm8994 from there.
Why? So far we have always used the form <device>, <platform> as
compatible. This is even in line with ePAPR, what changed?
Also, the compatible should be "huawei,angler" (if I read the internet
correctly) and the model should contain the product name
(Huawei Nexus 6P).
Regards,
Bjorn
^ permalink raw reply
* Re: [PATCH v2 1/6] pinctrl-aspeed-g5: Never set SCU90[6]
From: Joel Stanley @ 2016-11-03 22:59 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Lee Jones, Linus Walleij, Mark Rutland, Rob Herring,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478097481-14895-2-git-send-email-andrew-zrmu5oMJ5Fs@public.gmane.org>
On Thu, Nov 3, 2016 at 1:07 AM, Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org> wrote:
> If a pin depending on bit 6 in SCU90 is requested for GPIO, the export
> will succeed but changes to the GPIO's value will not be accepted by the
> hardware. This is because the pinmux driver has misconfigured the SCU by
> writing 1 to the reserved bit.
>
> The description of SCU90[6] from the datasheet is 'Reserved, must keep
> at value ”0”'. The fix is to switch pinmux from the bit-flipping macro
> to explicitly configuring the .enable and .disable values to zero.
>
> The patch has been tested on an AST2500 EVB.
>
> Fixes: 56e57cb6c07f (pinctrl: Add pinctrl-aspeed-g5 driver)
> Reported-by: Uma Yadlapati <yadlapat-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
Reviewed-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
And tested-by.
> This patch should be applied for 4.9.
In the future I think we should send fixes separately from the rest of
the series, so it's clear to Linus where we expect patches to end up.
Perhaps Linus can share his preference with us?
Cheers,
Joel
> drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> index c8c72e8259d3..87b46390b695 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> @@ -26,7 +26,7 @@
>
> #define ASPEED_G5_NR_PINS 228
>
> -#define COND1 SIG_DESC_BIT(SCU90, 6, 0)
> +#define COND1 { SCU90, BIT(6), 0, 0 }
> #define COND2 { SCU94, GENMASK(1, 0), 0, 0 }
>
> #define B14 0
> --
> 2.7.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
* Re: [PATCH 5/6] arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support
From: Mark Rutland @ 2016-11-03 22:44 UTC (permalink / raw)
To: Jeremy McNicoll
Cc: linux-arm-msm, linux-soc, devicetree, robh, andy.gross, sboyd,
arnd, bjorn.andersson, michael.scott
In-Reply-To: <1477394221-30963-6-git-send-email-jeremymc@redhat.com>
On Tue, Oct 25, 2016 at 04:17:00AM -0700, Jeremy McNicoll wrote:
> + clocks {
> + xo_board: xo_board {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <19200000>;
> + };
> +
> + sleep_clk: sleep_clk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <32768>;
> + };
> + };
As with the other patch, please get rid of the clocks node and put these
directly under the root.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH 5/6] arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support
From: Andy Gross @ 2016-11-03 22:44 UTC (permalink / raw)
To: Stephen Boyd
Cc: Jeremy McNicoll, Jeremy McNicoll,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A,
arnd-r2nGTMty4D4, bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, michael.scott-QSEj5FYQhm4dnm+yROfE0A,
Bastian Köcher
In-Reply-To: <9e0555fa-d689-e26e-feba-9daa815eb1c5-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On Thu, Nov 03, 2016 at 03:42:36PM -0700, Stephen Boyd wrote:
> On 11/03/2016 03:32 PM, Andy Gross wrote:
> > On Thu, Oct 27, 2016 at 05:54:01PM -0700, Jeremy McNicoll wrote:
> >> On 2016-10-27 5:06 PM, Stephen Boyd wrote:
> >>> On 10/25, Jeremy McNicoll wrote:
> >>>> diff --git a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> >>>> new file mode 100644
> >>>> index 0000000..2fc68c4
> >>>> --- /dev/null
> >>>> +++ b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> >>>> @@ -0,0 +1,40 @@
> >>>> +/* Copyright (c) 2015, Huawei Inc. All rights reserved.
> >>>> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> >>>> + *
> >>>> + * This program is free software; you can redistribute it and/or modify
> >>>> + * it under the terms of the GNU General Public License version 2 and
> >>>> + * only version 2 as published by the Free Software Foundation.
> >>>> + *
> >>>> + * This program is distributed in the hope that it will be useful,
> >>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >>>> + * GNU General Public License for more details.
> >>>> + */
> >>>> +
> >>>> +/dts-v1/;
> >>>> +
> >>>> +#include "msm8994-v2.0.dtsi"
> >>>> +
> >>>> +/ {
> >>>> + model = "HUAWEI MSM8994 ANGLER rev-1.01";
> >>>> + compatible = "qcom,msm8994";
> >>> Please replace this with something more specific for the actual
> >>> board. Preferably with the board manufacturer vendor prefix and
> >>> some sort of string for the board.
> > Something like?
> >
> > "huawei,nexus-6P", "qcom,msm8994"
>
> Sure, except completely remove qcom,msm8994 from there.
Hmmm ok. I just briefly looked at one of the sony boards and used that as an
example. So there shouldn't be any link to the underlying soc in the compat?
--
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 5/6] arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support
From: Stephen Boyd @ 2016-11-03 22:42 UTC (permalink / raw)
To: Andy Gross, Jeremy McNicoll
Cc: Jeremy McNicoll, linux-arm-msm, linux-soc, devicetree, robh, arnd,
bjorn.andersson, mark.rutland, michael.scott, Bastian Köcher
In-Reply-To: <20161103223246.GA5135@hector>
On 11/03/2016 03:32 PM, Andy Gross wrote:
> On Thu, Oct 27, 2016 at 05:54:01PM -0700, Jeremy McNicoll wrote:
>> On 2016-10-27 5:06 PM, Stephen Boyd wrote:
>>> On 10/25, Jeremy McNicoll wrote:
>>>> diff --git a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
>>>> new file mode 100644
>>>> index 0000000..2fc68c4
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
>>>> @@ -0,0 +1,40 @@
>>>> +/* Copyright (c) 2015, Huawei Inc. All rights reserved.
>>>> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License version 2 and
>>>> + * only version 2 as published by the Free Software Foundation.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU General Public License for more details.
>>>> + */
>>>> +
>>>> +/dts-v1/;
>>>> +
>>>> +#include "msm8994-v2.0.dtsi"
>>>> +
>>>> +/ {
>>>> + model = "HUAWEI MSM8994 ANGLER rev-1.01";
>>>> + compatible = "qcom,msm8994";
>>> Please replace this with something more specific for the actual
>>> board. Preferably with the board manufacturer vendor prefix and
>>> some sort of string for the board.
> Something like?
>
> "huawei,nexus-6P", "qcom,msm8994"
Sure, except completely remove qcom,msm8994 from there.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 3/6] msm8994 clocks: global clock support for msm8994 SOC.
From: Mark Rutland @ 2016-11-03 22:42 UTC (permalink / raw)
To: Jeremy McNicoll
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A,
andy.gross-QSEj5FYQhm4dnm+yROfE0A, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
arnd-r2nGTMty4D4, bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
michael.scott-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1477394221-30963-4-git-send-email-jeremymc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, Oct 25, 2016 at 04:16:58AM -0700, Jeremy McNicoll wrote:
> +static const struct qcom_cc_desc gcc_msm8994_desc = {
> + .config = &gcc_msm8994_regmap_config,
> + .clks = gcc_msm8994_clocks,
> + .num_clks = ARRAY_SIZE(gcc_msm8994_clocks),
> + .resets = NULL,
> + .num_resets = 0,
> + .gdscs = NULL,
> + .num_gdscs = 0,
> +};
> +
> +static const struct of_device_id gcc_msm8994_match_table[] = {
> + { .compatible = "qcom,gcc-8994" },
> + { .compatible = "qcom,gcc-8994v2" },
> + {}
> +}
> +
> +MODULE_DEVICE_TABLE(of, gcc_msm8994_match_table);
> +
> +static int gcc_msm8994_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct clk *clk;
> + const char *compat = NULL;
> + int compatlen = 0;
> + bool is_v2 = false;
> +
> + clk = devm_clk_register(dev, &xo.hw);
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + compat = of_get_property(pdev->dev.of_node, "compatible", &compatlen);
> + if (!compat || (compatlen <= 0))
> + return -EINVAL;
It is almost never correct to use of_get_property(). Please avoid it.
> +
> + is_v2 = !strcmp(compat, "qcom,gcc-8994v2");
> + if (is_v2)
> + msm_gcc_8994v2_fixup();
Either put some data in the of_device_id tables, and use that to determine what
to do here, or use of_device_is_compatible() or of_match_node() to check the
string.
Thanks,
Mark.
--
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 1/6] arm64: dts: msm8992 SoC and LG Bullhead (Nexus 5X) support
From: Mark Rutland @ 2016-11-03 22:38 UTC (permalink / raw)
To: Jeremy McNicoll
Cc: linux-arm-msm, linux-soc, devicetree, robh, andy.gross, sboyd,
arnd, bjorn.andersson, michael.scott
In-Reply-To: <1477394221-30963-2-git-send-email-jeremymc@redhat.com>
On Tue, Oct 25, 2016 at 04:16:56AM -0700, Jeremy McNicoll wrote:
> + memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
These shouldn't be here. There are no sub-nodes that this should affect.
> + device_type = "memory";
> + reg = <0 0 0 0>; // bootloader will update
> + };
> +
> + clocks {
> + xo_board: xo_board {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <19200000>;
> + clock-output-names = "xo_board";
> + };
> +
> + sleep_clk: sleep_clk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <32768>;
> + clock-output-names = "sleep_clk";
> + };
> + };
Please get rid of the clocks container node, and put these directly under the
root node.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH 5/6] arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support
From: Andy Gross @ 2016-11-03 22:32 UTC (permalink / raw)
To: Jeremy McNicoll
Cc: Stephen Boyd, Jeremy McNicoll, linux-arm-msm, linux-soc,
devicetree, robh, arnd, bjorn.andersson, mark.rutland,
michael.scott, Bastian Köcher
In-Reply-To: <822ebad4-4fdf-574a-12cb-75b0165f4746@redhat.com>
On Thu, Oct 27, 2016 at 05:54:01PM -0700, Jeremy McNicoll wrote:
> On 2016-10-27 5:06 PM, Stephen Boyd wrote:
> >On 10/25, Jeremy McNicoll wrote:
> >>diff --git a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> >>new file mode 100644
> >>index 0000000..2fc68c4
> >>--- /dev/null
> >>+++ b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> >>@@ -0,0 +1,40 @@
> >>+/* Copyright (c) 2015, Huawei Inc. All rights reserved.
> >>+ * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> >>+ *
> >>+ * This program is free software; you can redistribute it and/or modify
> >>+ * it under the terms of the GNU General Public License version 2 and
> >>+ * only version 2 as published by the Free Software Foundation.
> >>+ *
> >>+ * This program is distributed in the hope that it will be useful,
> >>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >>+ * GNU General Public License for more details.
> >>+ */
> >>+
> >>+/dts-v1/;
> >>+
> >>+#include "msm8994-v2.0.dtsi"
> >>+
> >>+/ {
> >>+ model = "HUAWEI MSM8994 ANGLER rev-1.01";
> >>+ compatible = "qcom,msm8994";
> >
> >Please replace this with something more specific for the actual
> >board. Preferably with the board manufacturer vendor prefix and
> >some sort of string for the board.
Something like?
"huawei,nexus-6P", "qcom,msm8994"
^ permalink raw reply
* Re: [PATCH v4] media: et8ek8: add device tree binding documentation
From: Sakari Ailus @ 2016-11-03 22:20 UTC (permalink / raw)
To: Pavel Machek
Cc: Rob Herring, ivo.g.dimitrov.75, sre, pali.rohar, linux-media,
pawel.moll, mark.rutland, ijc+devicetree, galak, mchehab,
devicetree, linux-kernel
In-Reply-To: <20161103124749.GA22180@amd>
Hi Pavel and Rob,
On Thu, Nov 03, 2016 at 01:47:49PM +0100, Pavel Machek wrote:
> Hi!
>
> > > +Mandatory properties
> > > +--------------------
> > > +
> > > +- compatible: "toshiba,et8ek8"
> > > +- reg: I2C address (0x3e, or an alternative address)
> > > +- vana-supply: Analogue voltage supply (VANA), 2.8 volts
> > > +- clocks: External clock to the sensor
> > > +- clock-frequency: Frequency of the external clock to the sensor. Camera
> > > + driver will set this frequency on the external clock.
> >
> > This is fine if the frequency is fixed (e.g. an oscillator), but you
> > should use the clock binding if clocks are programable.
>
> It is fixed. So I assume this can stay as is? Or do you want me to add
> "The clock frequency is a pre-determined frequency known to be
> suitable to the board." as Sakari suggests?
>
> > > +- reset-gpios: XSHUTDOWN GPIO
> >
> > Please state what the active polarity is.
>
> As in "This gpio will be set to 1 when the chip is powered." ?
How about:
"The XSHUTDOWN signal is active high. The sensor is in hardware standby
mode when the signal is in low state."
These bindings start looking more precise than the smiapp ones. :-)
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply
* [PATCH] PM / Domains: Fix compatible for domain idle state
From: Lina Iyer @ 2016-11-03 21:54 UTC (permalink / raw)
To: ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
Cc: andy.gross, sboyd, linux-arm-msm, brendan.jackman,
lorenzo.pieralisi, sudeep.holla, Juri.Lelli, Lina Iyer,
devicetree, Rob Herring
In-Reply-To: <1478210075-92045-1-git-send-email-lina.iyer@linaro.org>
Re-using idle state definition provided by arm,idle-state for domain
idle states creates a lot of confusion and limits further evolution of
the domain idle definition. To keep things clear and simple, define a
idle states for domain using a new compatible "domain-idle-state".
Fix existing PM domains code to look for the newly defined compatible.
Cc: <devicetree@vger.kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
.../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
.../devicetree/bindings/power/power_domain.txt | 8 +++---
drivers/base/power/domain.c | 2 +-
3 files changed, 38 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
new file mode 100644
index 0000000..eefc7ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
@@ -0,0 +1,33 @@
+PM Domain Idle State Node:
+
+A domain idle state node represents the state parameters that will be used to
+select the state when there are no active components in the domain.
+
+The state node has the following parameters -
+
+- compatible:
+ Usage: Required
+ Value type: <string>
+ Definition: Must be "domain-idle-state".
+
+- entry-latency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing worst case latency in
+ microseconds required to enter the idle state.
+ The exit-latency-us duration may be guaranteed
+ only after entry-latency-us has passed.
+
+- exit-latency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing worst case latency
+ in microseconds required to exit the idle state.
+
+- min-residency-us
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: u32 value representing minimum residency duration
+ in microseconds after which the idle state will yield
+ power benefits after overcoming the overhead in entering
+i the idle state.
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index e165036..723e1ad 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -31,7 +31,7 @@ Optional properties:
- domain-idle-states : A phandle of an idle-state that shall be soaked into a
generic domain power state. The idle state definitions are
- compatible with arm,idle-state specified in [1].
+ compatible with domain-idle-state specified in [1].
The domain-idle-state property reflects the idle state of this PM domain and
not the idle states of the devices or sub-domains in the PM domain. Devices
and sub-domains have their own idle-states independent of the parent
@@ -85,7 +85,7 @@ Example 3:
};
DOMAIN_RET: state@0 {
- compatible = "arm,idle-state";
+ compatible = "domain-idle-state";
reg = <0x0>;
entry-latency-us = <1000>;
exit-latency-us = <2000>;
@@ -93,7 +93,7 @@ Example 3:
};
DOMAIN_PWR_DN: state@1 {
- compatible = "arm,idle-state";
+ compatible = "domain-idle-state";
reg = <0x1>;
entry-latency-us = <5000>;
exit-latency-us = <8000>;
@@ -118,4 +118,4 @@ The node above defines a typical PM domain consumer device, which is located
inside a PM domain with index 0 of a power controller represented by a node
with the label "power".
-[1]. Documentation/devicetree/bindings/arm/idle-states.txt
+[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 661737c..f0bc672 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
static const struct of_device_id idle_state_match[] = {
- { .compatible = "arm,idle-state", },
+ { .compatible = "domain-idle-state", },
{ }
};
--
2.7.4
^ permalink raw reply related
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