* [PATCH] EDAC: mpc85xx: Add T2080 l2-cache support
From: Chris Packham @ 2016-11-29 2:20 UTC (permalink / raw)
To: linux-edac
Cc: Chris Packham, Rob Herring, Mark Rutland, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Johannes Thumshirn,
Borislav Petkov, Mauro Carvalho Chehab, devicetree, linuxppc-dev,
linux-kernel
The l2-cache controller on the T2080 SoC has similar capabilities to the
others already supported by the mpc85xx_edac driver. Add it to the list
of compatible devices.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
arch/powerpc/boot/dts/fsl/t2081si-post.dtsi | 1 +
drivers/edac/mpc85xx_edac.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
index c744569a20e1..a97296c64eb2 100644
--- a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
@@ -678,5 +678,6 @@
compatible = "fsl,t2080-l2-cache-controller";
reg = <0xc20000 0x40000>;
next-level-cache = <&cpc>;
+ interrupts = <16 2 1 9>;
};
};
diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
index ff0567526ee3..aee6dcdae02a 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/mpc85xx_edac.c
@@ -613,6 +613,7 @@ static const struct of_device_id mpc85xx_l2_err_of_match[] = {
{ .compatible = "fsl,p1020-l2-cache-controller", },
{ .compatible = "fsl,p1021-l2-cache-controller", },
{ .compatible = "fsl,p2020-l2-cache-controller", },
+ { .compatible = "fsl,t2080-l2-cache-controller", },
{},
};
MODULE_DEVICE_TABLE(of, mpc85xx_l2_err_of_match);
--
2.10.2
^ permalink raw reply related
* Re: [PATCH v11 2/7] dtc: Plugin and fixup support
From: David Gibson @ 2016-11-29 2:38 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480349141-14145-3-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 27551 bytes --]
On Mon, Nov 28, 2016 at 06:05:36PM +0200, Pantelis Antoniou wrote:
> This patch enable the generation of symbols & local fixup information
> for trees compiled with the -@ (--symbols) option.
>
> Using this patch labels in the tree and their users emit information
> in __symbols__ and __local_fixups__ nodes.
>
> The __fixups__ node make possible the dynamic resolution of phandle
> references which are present in the plugin tree but lie in the
> tree that are applying the overlay against.
>
> While there is a new magic number for dynamic device tree/overlays blobs
> it is by default enabled. Remember to use -M to generate compatible
> blobs.
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Signed-off-by: Jan Luebbe <jlu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> Documentation/manual.txt | 29 ++++++-
> checks.c | 8 +-
> dtc-lexer.l | 5 ++
> dtc-parser.y | 29 ++++++-
> dtc.c | 51 +++++++++++-
> dtc.h | 21 ++++-
> fdtdump.c | 2 +-
> flattree.c | 17 ++--
> fstree.c | 2 +-
> libfdt/fdt.c | 2 +-
> libfdt/fdt.h | 3 +-
> livetree.c | 208 ++++++++++++++++++++++++++++++++++++++++++++++-
> tests/mangle-layout.c | 7 +-
> 13 files changed, 357 insertions(+), 27 deletions(-)
>
> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
> index 398de32..92a4966 100644
> --- a/Documentation/manual.txt
> +++ b/Documentation/manual.txt
> @@ -119,6 +119,28 @@ Options:
> Make space for <number> reserve map entries
> Relevant for dtb and asm output only.
>
> + -@
> + Generates a __symbols__ node at the root node of the resulting blob
> + for any node labels used, and for any local references using phandles
> + it also generates a __local_fixups__ node that tracks them.
> +
> + When using the /plugin/ tag all unresolved label references to
> + be tracked in the __fixups__ node, making dynamic resolution possible.
> +
> + -A
> + Generate automatically aliases for all node labels. This is similar to
> + the -@ option (the __symbols__ node contain identical information) but
> + the semantics are slightly different since no phandles are automatically
> + generated for labeled nodes.
> +
> + -M
> + Generate blobs with the old FDT magic number for device tree objects.
> + By default blobs use the DTBO FDT magic number instead.
> +
> + -F
> + Suppress generation of fixups when -@ is used. This is useful for generating
> + a base tree with symbols but without fixups that take some amount of space.
Urgh, yet another option?
Can you give me more details on how __fixups__ is useful in a base tree?
> -S <bytes>
> Ensure the blob at least <bytes> long, adding additional
> space if needed.
> @@ -146,13 +168,18 @@ Additionally, dtc performs various sanity checks on the tree.
> Here is a very rough overview of the layout of a DTS source file:
>
>
> - sourcefile: list_of_memreserve devicetree
> + sourcefile: versioninfo plugindecl list_of_memreserve devicetree
>
> memreserve: label 'memreserve' ADDR ADDR ';'
> | label 'memreserve' ADDR '-' ADDR ';'
>
> devicetree: '/' nodedef
>
> + versioninfo: '/' 'dts-v1' '/' ';'
> +
> + plugindecl: '/' 'plugin' '/' ';'
> + | /* empty */
> +
> nodedef: '{' list_of_property list_of_subnode '}' ';'
>
> property: label PROPNAME '=' propdata ';'
> diff --git a/checks.c b/checks.c
> index 2bd27a4..4292f4b 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -487,8 +487,12 @@ static void fixup_phandle_references(struct check *c, struct boot_info *bi,
>
> refnode = get_node_by_ref(dt, m->ref);
> if (! refnode) {
> - FAIL(c, "Reference to non-existent node or label \"%s\"\n",
> - m->ref);
> + if (!(bi->versionflags & VF_PLUGIN))
> + FAIL(c, "Reference to non-existent node or "
> + "label \"%s\"\n", m->ref);
> + else /* mark the entry as unresolved */
> + *((cell_t *)(prop->val.val + m->offset)) =
> + cpu_to_fdt32(0xffffffff);
> continue;
> }
>
> diff --git a/dtc-lexer.l b/dtc-lexer.l
> index 790fbf6..40bbc87 100644
> --- a/dtc-lexer.l
> +++ b/dtc-lexer.l
> @@ -121,6 +121,11 @@ static void lexical_error(const char *fmt, ...);
> return DT_V1;
> }
>
> +<*>"/plugin/" {
> + DPRINT("Keyword: /plugin/\n");
> + return DT_PLUGIN;
> + }
> +
> <*>"/memreserve/" {
> DPRINT("Keyword: /memreserve/\n");
> BEGIN_DEFAULT();
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 14aaf2e..ad3dbe2 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -19,6 +19,7 @@
> */
> %{
> #include <stdio.h>
> +#include <inttypes.h>
>
> #include "dtc.h"
> #include "srcpos.h"
> @@ -52,9 +53,11 @@ extern bool treesource_error;
> struct node *nodelist;
> struct reserve_info *re;
> uint64_t integer;
> + unsigned int flags;
> }
>
> %token DT_V1
> +%token DT_PLUGIN
> %token DT_MEMRESERVE
> %token DT_LSHIFT DT_RSHIFT DT_LE DT_GE DT_EQ DT_NE DT_AND DT_OR
> %token DT_BITS
> @@ -71,6 +74,8 @@ extern bool treesource_error;
>
> %type <data> propdata
> %type <data> propdataprefix
> +%type <flags> versioninfo
> +%type <flags> plugindecl
> %type <re> memreserve
> %type <re> memreserves
> %type <array> arrayprefix
> @@ -101,16 +106,34 @@ extern bool treesource_error;
> %%
>
> sourcefile:
> - v1tag memreserves devicetree
> + versioninfo plugindecl memreserves devicetree
> {
> - the_boot_info = build_boot_info($2, $3,
> - guess_boot_cpuid($3));
> + the_boot_info = build_boot_info($1 | $2, $3, $4,
> + guess_boot_cpuid($4));
> + }
> + ;
> +
> +versioninfo:
> + v1tag
> + {
> + $$ = VF_DT_V1;
> }
> ;
>
> v1tag:
> DT_V1 ';'
> + | DT_V1
> | DT_V1 ';' v1tag
> +
> +plugindecl:
> + DT_PLUGIN ';'
> + {
> + $$ = VF_PLUGIN;
> + }
> + | /* empty */
> + {
> + $$ = 0;
> + }
> ;
>
> memreserves:
> diff --git a/dtc.c b/dtc.c
> index 9dcf640..947d082 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -32,6 +32,10 @@ int minsize; /* Minimum blob size */
> int padsize; /* Additional padding to blob */
> int alignsize; /* Additional padding to blob accroding to the alignsize */
> int phandle_format = PHANDLE_BOTH; /* Use linux,phandle or phandle properties */
> +int symbol_fixup_support; /* enable symbols & fixup support */
> +int auto_label_aliases; /* auto generate labels -> aliases */
> +int no_dtbo_magic; /* use old FDT magic values for objects */
> +int suppress_fixups; /* suppress generation of fixups on symbol support */
The symbol_fixup_support and suppress_fixups flags semantics are
starting to confuse me. I think rework these to 'generate_symbols'
and 'generate_fixups' which should have clearer logic.
Note that - for now - I'm just suggesting a change in the internal
variables, they can still be set appropriately based on the existing
semantics of the command line options.
> static int is_power_of_2(int x)
> {
> @@ -59,7 +63,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
> #define FDT_VERSION(version) _FDT_VERSION(version)
> #define _FDT_VERSION(version) #version
> static const char usage_synopsis[] = "dtc [options] <input file>";
> -static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:hv";
> +static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AMFhv";
> static struct option const usage_long_opts[] = {
> {"quiet", no_argument, NULL, 'q'},
> {"in-format", a_argument, NULL, 'I'},
> @@ -78,6 +82,10 @@ static struct option const usage_long_opts[] = {
> {"phandle", a_argument, NULL, 'H'},
> {"warning", a_argument, NULL, 'W'},
> {"error", a_argument, NULL, 'E'},
> + {"symbols", no_argument, NULL, '@'},
> + {"auto-alias", no_argument, NULL, 'A'},
> + {"no-dtbo-magic", no_argument, NULL, 'M'},
> + {"suppress-fixups", no_argument, NULL, 'F'},
> {"help", no_argument, NULL, 'h'},
> {"version", no_argument, NULL, 'v'},
> {NULL, no_argument, NULL, 0x0},
> @@ -109,6 +117,10 @@ static const char * const usage_opts_help[] = {
> "\t\tboth - Both \"linux,phandle\" and \"phandle\" properties",
> "\n\tEnable/disable warnings (prefix with \"no-\")",
> "\n\tEnable/disable errors (prefix with \"no-\")",
> + "\n\tEnable symbols/fixup support",
> + "\n\tEnable auto-alias of labels",
> + "\n\tDo not use DTBO magic value for plugin objects",
> + "\n\tSuppress generation of fixups when symbol support enabled",
> "\n\tPrint this help and exit",
> "\n\tPrint version and exit",
> NULL,
> @@ -153,7 +165,7 @@ static const char *guess_input_format(const char *fname, const char *fallback)
> fclose(f);
>
> magic = fdt32_to_cpu(magic);
> - if (magic == FDT_MAGIC)
> + if (magic == FDT_MAGIC || magic == FDT_MAGIC_DTBO)
> return "dtb";
>
> return guess_type_by_name(fname, fallback);
> @@ -172,6 +184,7 @@ int main(int argc, char *argv[])
> FILE *outf = NULL;
> int outversion = DEFAULT_FDT_VERSION;
> long long cmdline_boot_cpuid = -1;
> + fdt32_t out_magic = FDT_MAGIC;
>
> quiet = 0;
> reservenum = 0;
> @@ -249,6 +262,19 @@ int main(int argc, char *argv[])
> parse_checks_option(false, true, optarg);
> break;
>
> + case '@':
> + symbol_fixup_support = 1;
> + break;
> + case 'A':
> + auto_label_aliases = 1;
> + break;
> + case 'M':
> + no_dtbo_magic = 1;
> + break;
> + case 'F':
> + suppress_fixups = 1;
> + break;
> +
> case 'h':
> usage(NULL);
> default:
> @@ -306,6 +332,20 @@ int main(int argc, char *argv[])
> fill_fullpaths(bi->dt, "");
> process_checks(force, bi);
>
> + if (auto_label_aliases)
> + generate_label_tree(bi, "aliases", false);
> +
> + /* symbol support or plugin is detected */
> + if (symbol_fixup_support || (bi->versionflags & VF_PLUGIN))
> + generate_label_tree(bi, "__symbols__", true);
> +
> + /* plugin or symbol support and fixups are not suppressed */
> + if ((bi->versionflags & VF_PLUGIN) ||
> + (symbol_fixup_support && !suppress_fixups)) {
> + generate_fixups_tree(bi, "__fixups__");
> + generate_local_fixups_tree(bi, "__local_fixups__");
> + }
Elaborating on the suggestion above, I'd like to see this just be:
if (generate_symbols)
generate_label_tree(...);
if (generate_fixups) {
generate_fixups_tree(..);
generate_local_fixups_tree(...);
}
With the two option flags set earlier according to the command line
options + /plugin/ tag.
> if (sort)
> sort_tree(bi);
>
> @@ -318,12 +358,15 @@ int main(int argc, char *argv[])
> outname, strerror(errno));
> }
>
> + if (!no_dtbo_magic && (bi->versionflags & VF_PLUGIN))
> + out_magic = FDT_MAGIC_DTBO;
> +
> if (streq(outform, "dts")) {
> dt_to_source(outf, bi);
> } else if (streq(outform, "dtb")) {
> - dt_to_blob(outf, bi, outversion);
> + dt_to_blob(outf, bi, out_magic, outversion);
> } else if (streq(outform, "asm")) {
> - dt_to_asm(outf, bi, outversion);
> + dt_to_asm(outf, bi, out_magic, outversion);
> } else if (streq(outform, "null")) {
> /* do nothing */
> } else {
> diff --git a/dtc.h b/dtc.h
> index 32009bc..4da5a37 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -55,6 +55,10 @@ extern int minsize; /* Minimum blob size */
> extern int padsize; /* Additional padding to blob */
> extern int alignsize; /* Additional padding to blob accroding to the alignsize */
> extern int phandle_format; /* Use linux,phandle or phandle properties */
> +extern int symbol_fixup_support;/* enable symbols & fixup support */
> +extern int auto_label_aliases; /* auto generate labels -> aliases */
> +extern int no_dtbo_magic; /* use old FDT magic values for objects */
> +extern int suppress_fixups; /* suppress generation of fixups on symbol support */
>
> #define PHANDLE_LEGACY 0x1
> #define PHANDLE_EPAPR 0x2
> @@ -202,6 +206,8 @@ void delete_property(struct property *prop);
> void add_child(struct node *parent, struct node *child);
> void delete_node_by_name(struct node *parent, char *name);
> void delete_node(struct node *node);
> +void append_to_property(struct node *node,
> + char *name, const void *data, int len);
>
> const char *get_unitname(struct node *node);
> struct property *get_property(struct node *node, const char *propname);
> @@ -237,14 +243,23 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
>
>
> struct boot_info {
> + unsigned int versionflags;
> struct reserve_info *reservelist;
> struct node *dt; /* the device tree */
> uint32_t boot_cpuid_phys;
> };
>
> -struct boot_info *build_boot_info(struct reserve_info *reservelist,
> +/* version flags definitions */
> +#define VF_DT_V1 0x0001 /* /dts-v1/ */
> +#define VF_PLUGIN 0x0002 /* /plugin/ */
Hm, sorry, didn't mention this minor nit before. Can you rename
versionflags and the associated constants to make it clear these are
about the *dts* version, since we also have entirely different
versioning info for the *dtb* version.
> +struct boot_info *build_boot_info(unsigned int versionflags,
> + struct reserve_info *reservelist,
> struct node *tree, uint32_t boot_cpuid_phys);
> void sort_tree(struct boot_info *bi);
> +void generate_label_tree(struct boot_info *bi, char *name, bool allocph);
> +void generate_fixups_tree(struct boot_info *bi, char *name);
> +void generate_local_fixups_tree(struct boot_info *bi, char *name);
>
> /* Checks */
>
> @@ -253,8 +268,8 @@ void process_checks(bool force, struct boot_info *bi);
>
> /* Flattened trees */
>
> -void dt_to_blob(FILE *f, struct boot_info *bi, int version);
> -void dt_to_asm(FILE *f, struct boot_info *bi, int version);
> +void dt_to_blob(FILE *f, struct boot_info *bi, fdt32_t magic, int version);
> +void dt_to_asm(FILE *f, struct boot_info *bi, fdt32_t magic, int version);
>
> struct boot_info *dt_from_blob(const char *fname);
>
> diff --git a/fdtdump.c b/fdtdump.c
> index a9a2484..dd63ac2 100644
> --- a/fdtdump.c
> +++ b/fdtdump.c
> @@ -201,7 +201,7 @@ int main(int argc, char *argv[])
> p = memchr(p, smagic[0], endp - p - FDT_MAGIC_SIZE);
> if (!p)
> break;
> - if (fdt_magic(p) == FDT_MAGIC) {
> + if (fdt_magic(p) == FDT_MAGIC || fdt_magic(p) == FDT_MAGIC_DTBO) {
> /* try and validate the main struct */
> off_t this_len = endp - p;
> fdt32_t max_version = 17;
> diff --git a/flattree.c b/flattree.c
> index a9d9520..57d76cf 100644
> --- a/flattree.c
> +++ b/flattree.c
> @@ -335,6 +335,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
> }
>
> static void make_fdt_header(struct fdt_header *fdt,
> + fdt32_t magic,
> struct version_info *vi,
> int reservesize, int dtsize, int strsize,
> int boot_cpuid_phys)
> @@ -345,7 +346,7 @@ static void make_fdt_header(struct fdt_header *fdt,
>
> memset(fdt, 0xff, sizeof(*fdt));
>
> - fdt->magic = cpu_to_fdt32(FDT_MAGIC);
> + fdt->magic = cpu_to_fdt32(magic);
> fdt->version = cpu_to_fdt32(vi->version);
> fdt->last_comp_version = cpu_to_fdt32(vi->last_comp_version);
>
> @@ -366,7 +367,7 @@ static void make_fdt_header(struct fdt_header *fdt,
> fdt->size_dt_struct = cpu_to_fdt32(dtsize);
> }
>
> -void dt_to_blob(FILE *f, struct boot_info *bi, int version)
> +void dt_to_blob(FILE *f, struct boot_info *bi, fdt32_t magic, int version)
> {
> struct version_info *vi = NULL;
> int i;
> @@ -390,7 +391,7 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version)
> reservebuf = flatten_reserve_list(bi->reservelist, vi);
>
> /* Make header */
> - make_fdt_header(&fdt, vi, reservebuf.len, dtbuf.len, strbuf.len,
> + make_fdt_header(&fdt, magic, vi, reservebuf.len, dtbuf.len, strbuf.len,
> bi->boot_cpuid_phys);
>
> /*
> @@ -467,7 +468,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf)
> }
> }
>
> -void dt_to_asm(FILE *f, struct boot_info *bi, int version)
> +void dt_to_asm(FILE *f, struct boot_info *bi, fdt32_t magic, int version)
> {
> struct version_info *vi = NULL;
> int i;
> @@ -830,6 +831,7 @@ struct boot_info *dt_from_blob(const char *fname)
> struct node *tree;
> uint32_t val;
> int flags = 0;
> + unsigned int versionflags = VF_DT_V1;
Especially here calling this just 'versionflags' is potentially
confusing, since it's not related to the dtb version that we're also
dealing with in this vicinity.
> f = srcfile_relative_open(fname, NULL);
>
> @@ -845,9 +847,12 @@ struct boot_info *dt_from_blob(const char *fname)
> }
>
> magic = fdt32_to_cpu(magic);
> - if (magic != FDT_MAGIC)
> + if (magic != FDT_MAGIC && magic != FDT_MAGIC_DTBO)
> die("Blob has incorrect magic number\n");
>
> + if (magic == FDT_MAGIC_DTBO)
> + versionflags |= VF_PLUGIN;
> +
> rc = fread(&totalsize, sizeof(totalsize), 1, f);
> if (ferror(f))
> die("Error reading DT blob size: %s\n", strerror(errno));
> @@ -942,5 +947,5 @@ struct boot_info *dt_from_blob(const char *fname)
>
> fclose(f);
>
> - return build_boot_info(reservelist, tree, boot_cpuid_phys);
> + return build_boot_info(versionflags, reservelist, tree, boot_cpuid_phys);
> }
> diff --git a/fstree.c b/fstree.c
> index 6d1beec..54f520b 100644
> --- a/fstree.c
> +++ b/fstree.c
> @@ -86,6 +86,6 @@ struct boot_info *dt_from_fs(const char *dirname)
> tree = read_fstree(dirname);
> tree = name_node(tree, "");
>
> - return build_boot_info(NULL, tree, guess_boot_cpuid(tree));
> + return build_boot_info(VF_DT_V1, NULL, tree, guess_boot_cpuid(tree));
> }
>
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index 22286a1..28d422c 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -57,7 +57,7 @@
>
> int fdt_check_header(const void *fdt)
> {
> - if (fdt_magic(fdt) == FDT_MAGIC) {
> + if (fdt_magic(fdt) == FDT_MAGIC || fdt_magic(fdt) == FDT_MAGIC_DTBO) {
> /* Complete tree */
> if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
> return -FDT_ERR_BADVERSION;
> diff --git a/libfdt/fdt.h b/libfdt/fdt.h
> index 526aedb..493cd55 100644
> --- a/libfdt/fdt.h
> +++ b/libfdt/fdt.h
> @@ -55,7 +55,7 @@
> #ifndef __ASSEMBLY__
>
> struct fdt_header {
> - fdt32_t magic; /* magic word FDT_MAGIC */
> + fdt32_t magic; /* magic word FDT_MAGIC[|_DTBO] */
> fdt32_t totalsize; /* total size of DT block */
> fdt32_t off_dt_struct; /* offset to structure */
> fdt32_t off_dt_strings; /* offset to strings */
> @@ -93,6 +93,7 @@ struct fdt_property {
> #endif /* !__ASSEMBLY */
>
> #define FDT_MAGIC 0xd00dfeed /* 4: version, 4: total size */
> +#define FDT_MAGIC_DTBO 0xd00dfdb0 /* DTBO magic */
> #define FDT_TAGSIZE sizeof(fdt32_t)
>
> #define FDT_BEGIN_NODE 0x1 /* Start node: full name */
> diff --git a/livetree.c b/livetree.c
> index 3dc7559..17f8310 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -296,6 +296,23 @@ void delete_node(struct node *node)
> delete_labels(&node->labels);
> }
>
> +void append_to_property(struct node *node,
> + char *name, const void *data, int len)
> +{
> + struct data d;
> + struct property *p;
> +
> + p = get_property(node, name);
> + if (p) {
> + d = data_append_data(p->val, data, len);
> + p->val = d;
> + } else {
> + d = data_append_data(empty_data, data, len);
> + p = build_property(name, d);
> + add_property(node, p);
> + }
> +}
> +
> struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
> {
> struct reserve_info *new = xmalloc(sizeof(*new));
> @@ -335,12 +352,14 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
> return list;
> }
>
> -struct boot_info *build_boot_info(struct reserve_info *reservelist,
> +struct boot_info *build_boot_info(unsigned int versionflags,
> + struct reserve_info *reservelist,
> struct node *tree, uint32_t boot_cpuid_phys)
> {
> struct boot_info *bi;
>
> bi = xmalloc(sizeof(*bi));
> + bi->versionflags = versionflags;
> bi->reservelist = reservelist;
> bi->dt = tree;
> bi->boot_cpuid_phys = boot_cpuid_phys;
> @@ -709,3 +728,190 @@ void sort_tree(struct boot_info *bi)
> sort_reserve_entries(bi);
> sort_node(bi->dt);
> }
> +
> +/* utility helper to avoid code duplication */
> +static struct node *build_and_name_child_node(struct node *parent, char *name)
> +{
> + struct node *node;
> +
> + node = build_node(NULL, NULL);
> + name_node(node, xstrdup(name));
> + add_child(parent, node);
> +
> + return node;
> +}
> +
> +static void generate_label_tree_internal(struct boot_info *bi,
> + struct node *an,
> + struct node *node,
> + bool allocph)
> +{
> + struct node *dt = bi->dt;
> + struct node *c;
> + struct property *p;
> + struct label *l;
> +
> + /* if if there are labels */
> + if (node->labels) {
> + /* now add the label in the node */
> + for_each_label(node->labels, l) {
> + /* check whether the label already exists */
> + p = get_property(an, l->label);
> + if (p) {
> + fprintf(stderr, "WARNING: label %s already"
> + " exists in /%s", l->label,
> + an->name);
> + continue;
> + }
> +
> + /* insert it */
> + p = build_property(l->label,
> + data_copy_mem(node->fullpath,
> + strlen(node->fullpath) + 1));
> + add_property(an, p);
> + }
> +
> + /* force allocation of a phandle for this node */
> + if (allocph)
> + (void)get_node_phandle(dt, node);
> + }
> +
> + for_each_child(node, c)
> + generate_label_tree_internal(bi, an, c, allocph);
> +}
> +
> +static void add_fixup_entry(struct boot_info *bi, struct node *fn,
> + struct node *node, struct property *prop,
> + struct marker *m)
> +{
> + char *entry;
> +
> + /* m->ref can only be a REF_PHANDLE, but check anyway */
> + assert(m->type == REF_PHANDLE);
> +
> + /* there shouldn't be any ':' in the arguments */
> + if (strchr(node->fullpath, ':') || strchr(prop->name, ':'))
> + die("arguments should not contain ':'\n");
> +
> + xasprintf(&entry, "%s:%s:%u",
> + node->fullpath, prop->name, m->offset);
> + append_to_property(fn, m->ref, entry, strlen(entry) + 1);
> +}
> +
> +static void generate_fixups_tree_internal(struct boot_info *bi,
> + struct node *fn,
> + struct node *node)
> +{
> + struct node *dt = bi->dt;
> + struct node *c;
> + struct property *prop;
> + struct marker *m;
> + struct node *refnode;
> +
> + for_each_property(node, prop) {
> + m = prop->val.markers;
> + for_each_marker_of_type(m, REF_PHANDLE) {
> + refnode = get_node_by_ref(dt, m->ref);
> + if (!refnode)
> + add_fixup_entry(bi, fn, node, prop, m);
> + }
> + }
> +
> + for_each_child(node, c)
> + generate_fixups_tree_internal(bi, fn, c);
> +}
> +
> +static void add_local_fixup_entry(struct boot_info *bi,
> + struct node *lfn, struct node *node,
> + struct property *prop, struct marker *m,
> + struct node *refnode)
> +{
> + struct node *wn, *nwn; /* local fixup node, walk node, new */
> + uint32_t value_32;
> + char **compp;
> + int i, depth;
> +
> + /* walk back retreiving depth */
> + depth = 0;
> + for (wn = node; wn; wn = wn->parent)
> + depth++;
> +
> + /* allocate name array */
> + compp = xmalloc(sizeof(*compp) * depth);
> +
> + /* store names in the array */
> + for (wn = node, i = depth - 1; wn; wn = wn->parent, i--)
> + compp[i] = wn->name;
> +
> + /* walk the path components creating nodes if they don't exist */
> + for (wn = lfn, i = 1; i < depth; i++, wn = nwn) {
> + /* if no node exists, create it */
> + nwn = get_subnode(wn, compp[i]);
> + if (!nwn)
> + nwn = build_and_name_child_node(wn, compp[i]);
> + }
> +
> + free(compp);
> +
> + value_32 = cpu_to_fdt32(m->offset);
> + append_to_property(wn, prop->name, &value_32, sizeof(value_32));
> +}
> +
> +static void generate_local_fixups_tree_internal(struct boot_info *bi,
> + struct node *lfn,
> + struct node *node)
> +{
> + struct node *dt = bi->dt;
> + struct node *c;
> + struct property *prop;
> + struct marker *m;
> + struct node *refnode;
> +
> + for_each_property(node, prop) {
> + m = prop->val.markers;
> + for_each_marker_of_type(m, REF_PHANDLE) {
> + refnode = get_node_by_ref(dt, m->ref);
> + if (!refnode)
> + continue;
> + add_local_fixup_entry(bi, lfn, node, prop, m, refnode);
> + }
> + }
> +
> + for_each_child(node, c)
> + generate_local_fixups_tree_internal(bi, lfn, c);
> +}
> +
> +static struct node *build_root_node(struct node *dt, char *name)
> +{
> + struct node *an;
> +
> + for_each_child(dt, an)
> + if (streq(name, an->name))
> + break;
> +
> + if (!an)
> + an = build_and_name_child_node(dt, name);
> +
> + if (!an)
> + die("Could not build root node /%s\n", name);
> +
> + return an;
> +}
> +
> +void generate_label_tree(struct boot_info *bi, char *name, bool allocph)
> +{
> + generate_label_tree_internal(bi, build_root_node(bi->dt, name),
> + bi->dt, allocph);
> +}
> +
> +void generate_fixups_tree(struct boot_info *bi, char *name)
> +{
> + generate_fixups_tree_internal(bi, build_root_node(bi->dt, name),
> + bi->dt);
> +}
> +
> +void generate_local_fixups_tree(struct boot_info *bi, char *name)
> +{
> + generate_local_fixups_tree_internal(bi, build_root_node(bi->dt, name),
> + bi->dt);
> +}
> diff --git a/tests/mangle-layout.c b/tests/mangle-layout.c
> index a76e51e..d29ebc6 100644
> --- a/tests/mangle-layout.c
> +++ b/tests/mangle-layout.c
> @@ -42,7 +42,8 @@ static void expand_buf(struct bufstate *buf, int newsize)
> buf->size = newsize;
> }
>
> -static void new_header(struct bufstate *buf, int version, const void *fdt)
> +static void new_header(struct bufstate *buf, fdt32_t magic, int version,
> + const void *fdt)
> {
> int hdrsize;
>
> @@ -56,7 +57,7 @@ static void new_header(struct bufstate *buf, int version, const void *fdt)
> expand_buf(buf, hdrsize);
> memset(buf->buf, 0, hdrsize);
>
> - fdt_set_magic(buf->buf, FDT_MAGIC);
> + fdt_set_magic(buf->buf, magic);
> fdt_set_version(buf->buf, version);
> fdt_set_last_comp_version(buf->buf, 16);
> fdt_set_boot_cpuid_phys(buf->buf, fdt_boot_cpuid_phys(fdt));
> @@ -145,7 +146,7 @@ int main(int argc, char *argv[])
> if (fdt_version(fdt) < 17)
> CONFIG("Input tree must be v17");
>
> - new_header(&buf, version, fdt);
> + new_header(&buf, FDT_MAGIC, version, fdt);
>
> while (*blockorder) {
> add_block(&buf, version, *blockorder, fdt);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v11 3/7] tests: Add check_path test
From: David Gibson @ 2016-11-29 2:40 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480349141-14145-4-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4113 bytes --]
On Mon, Nov 28, 2016 at 06:05:37PM +0200, Pantelis Antoniou wrote:
> Add a test that checks for existence or not of a node.
> It is useful for testing the various cases when generating
> symbols and fixups for dynamic device tree objects.
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
Can you please add a test-for-the-test by putting a couple of
invocations of this on test_tree1 into the runner script (one 'exists'
and one 'not-exists' should suffice).
> ---
> tests/.gitignore | 1 +
> tests/Makefile.tests | 3 +-
> tests/check_path.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 85 insertions(+), 1 deletion(-)
> create mode 100644 tests/check_path.c
>
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 354b565..9e209d5 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -8,6 +8,7 @@ tmp.*
> /asm_tree_dump
> /boot-cpuid
> /char_literal
> +/check_path
> /del_node
> /del_property
> /dtbs_equal_ordered
> diff --git a/tests/Makefile.tests b/tests/Makefile.tests
> index eb039c5..3d7a4f8 100644
> --- a/tests/Makefile.tests
> +++ b/tests/Makefile.tests
> @@ -25,7 +25,8 @@ LIB_TESTS_L = get_mem_rsv \
> integer-expressions \
> property_iterate \
> subnode_iterate \
> - overlay overlay_bad_fixup
> + overlay overlay_bad_fixup \
> + check_path
> LIB_TESTS = $(LIB_TESTS_L:%=$(TESTS_PREFIX)%)
>
> LIBTREE_TESTS_L = truncated_property
> diff --git a/tests/check_path.c b/tests/check_path.c
> new file mode 100644
> index 0000000..0d6a73b
> --- /dev/null
> +++ b/tests/check_path.c
> @@ -0,0 +1,82 @@
> +/*
> + * libfdt - Flat Device Tree manipulation
> + * Testcase for node existence
> + * Copyright (C) 2016 Konsulko Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either version 2.1 of
> + * the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <stdio.h>
> +
> +#include <libfdt.h>
> +
> +#include "tests.h"
> +
> +#define CHECK(code) \
> + { \
> + if (code) \
> + FAIL(#code ": %s", fdt_strerror(code)); \
> + }
> +
> +/* 4k ought to be enough for anybody */
> +#define FDT_COPY_SIZE (4 * 1024)
> +
> +static void *open_dt(char *path)
> +{
> + void *dt, *copy;
> +
> + dt = load_blob(path);
> + copy = xmalloc(FDT_COPY_SIZE);
> +
> + /*
> + * Resize our DTs to 4k so that we have room to operate on
> + */
> + CHECK(fdt_open_into(dt, copy, FDT_COPY_SIZE));
> +
> + return copy;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + void *fdt_base;
> + int fail_config, exists, check_exists;
> +
> + test_init(argc, argv);
> + fail_config = 0;
> +
> + if (argc != 4)
> + fail_config = 1;
> +
> + if (!fail_config) {
> + if (!strcmp(argv[2], "exists"))
> + check_exists = 1;
> + else if (!strcmp(argv[2], "not-exists"))
> + check_exists = 0;
> + else
> + fail_config = 1;
> + }
> +
> + if (fail_config)
> + CONFIG("Usage: %s <base dtb> <[exists|not-exists]> <node-path>", argv[0]);
> +
> + fdt_base = open_dt(argv[1]);
> +
> + exists = fdt_path_offset(fdt_base, argv[3]) >= 0;
> +
> + if (exists == check_exists)
> + PASS();
> + else
> + FAIL();
> +}
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-11-29 2:53 UTC (permalink / raw)
To: Ulf Hansson
Cc: Gregory CLEMENT, Adrian Hunter, linux-mmc@vger.kernel.org,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree@vger.kernel.org, Thomas Petazzoni,
linux-arm-kernel@lists.infradead.org, Jimmy Xu, Jisheng Zhang,
Nadav Haklai, Ryan Gao, Doug Jones, Victor Gu, Wei(SOCP) Liu,
Wilson Ding
In-Reply-To: <CAPDyKFr8rX04iY92OeQpSkS+3HN2-FxijCCiDb2sSKvP+TZPog@mail.gmail.com>
Hi Ulf,
On 2016/11/28 23:16, Ulf Hansson wrote:
> On 28 November 2016 at 12:38, Ziji Hu <huziji@marvell.com> wrote:
>> Hi Ulf,
>>
>> On 2016/11/28 19:13, Ulf Hansson wrote:
>>>>
>>>> As you suggest, I replace mmc_wait_for_cmd() with mmc_send_tuning(), to
>>>> send commands for testing current sampling point set in our host PHY.
>>>>
>>>> According to my test result, it shows that mmc_send_tuning() can only support
>>>> tuning command (CMD21/CMD19).
>>>> As a result, we cannot use mmc_send_tuning() when card is in the speed modes
>>>> which doesn't support tuning, such as eMMC HS SDR, eMMC HS DRR and
>>>> SD SDR 12/SDR25/DDR50. Card will not response to tuning commands in those
>>>> speed modes.
>>>>
>>>> Could you please provide suggestions for the speed mode in which tuning is
>>>> not available?
>>>>
>>>
>>> Normally the mmc host driver shouldn't have to care about what the
>>> card supports, as that is the responsibility of the mmc core to
>>> manage.
>>>
>>> The host should only need to implement the ->execute_tuning() ops,
>>> which gets called when the card supports tuning (CMD19/21). Does it
>>> make sense?
>>>
>> I think it is irrelevant to tuning procedure.
>>
>> Our host requires to adjust PHY setting after each time ios setting
>> (SDCLK/bus width/speed mode) is changed.
>> The simplified sequence is:
>> mmc change ios --> mmc_set_ios() --> ->set_ios() --> after sdhci_set_ios(),
>> adjust PHY setting.
>> During PHY setting adjustment, out host driver has to send commands to
>> test current sampling point. Tuning is another independent step.
>
> For those speed modes (or other ios changes) that *don't* requires
> tuning, then what will you do when you send the command to confirm the
> change of PHY setting and it fails?
>
> My assumption is that you will fail anyway, by propagating the error
> to the mmc core. At least that what was my understanding from your
> earlier replies, right!?
>
> Then, I think there are no point having the host driver sending a
> command to confirm the PHY settings, as the mmc core will anyway
> discover if something goes wrong when the next command is sent.
>
> Please correct me if I am wrong!
>
Sorry that I didn't make myself clear.
Our host PHY delay line consists of hundreds of sampling points.
Each sampling point represents a different phase shift.
In lower speed mode, our host driver will scan the delay line.
It will select and test multiple sampling points, other than testing
only single sampling point.
If a sampling point fails to transfer cmd/data, our host driver will
move to test next sampling point, until we find out a group of successful
sampling points which can transfer cmd/data. At last we will select
a perfect one from them.
Thank you.
Best regards,
Hu Ziji
>>
>> Thus our host needs a valid command in PHY setting adjustment. Tuning command
>> can be borrowed to complete this task in SD SDR50. But for other speed mode,
>> we have to find out a valid command.
>
> I thought we agreed on this wasn't necessary? Please see my upper response.
>
> Kind regards
> Uffe
>
^ permalink raw reply
* Re: [PATCH v11 4/7] tests: Add overlay tests
From: David Gibson @ 2016-11-29 3:08 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480349141-14145-5-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 9292 bytes --]
On Mon, Nov 28, 2016 at 06:05:38PM +0200, Pantelis Antoniou wrote:
> Add a number of tests for dynamic objects/overlays.
>
> Re-use the original test by moving the contents to a .dtsi include
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> ---
> tests/overlay_overlay_dtc.dts | 76 +----------------------------------
> tests/overlay_overlay_dtc.dtsi | 83 +++++++++++++++++++++++++++++++++++++++
> tests/overlay_overlay_new_dtc.dts | 11 ++++++
> tests/overlay_overlay_simple.dts | 12 ++++++
> tests/run_tests.sh | 41 +++++++++++++++++++
> 5 files changed, 148 insertions(+), 75 deletions(-)
> create mode 100644 tests/overlay_overlay_dtc.dtsi
> create mode 100644 tests/overlay_overlay_new_dtc.dts
> create mode 100644 tests/overlay_overlay_simple.dts
>
> diff --git a/tests/overlay_overlay_dtc.dts b/tests/overlay_overlay_dtc.dts
> index 30d2362..ca943ea 100644
> --- a/tests/overlay_overlay_dtc.dts
> +++ b/tests/overlay_overlay_dtc.dts
> @@ -8,78 +8,4 @@
> /dts-v1/;
> /plugin/;
>
> -/ {
> - /* Test that we can change an int by another */
> - fragment@0 {
> - target = <&test>;
> -
> - __overlay__ {
> - test-int-property = <43>;
> - };
> - };
> -
> - /* Test that we can replace a string by a longer one */
> - fragment@1 {
> - target = <&test>;
> -
> - __overlay__ {
> - test-str-property = "foobar";
> - };
> - };
> -
> - /* Test that we add a new property */
> - fragment@2 {
> - target = <&test>;
> -
> - __overlay__ {
> - test-str-property-2 = "foobar2";
> - };
> - };
> -
> - /* Test that we add a new node (by phandle) */
> - fragment@3 {
> - target = <&test>;
> -
> - __overlay__ {
> - new-node {
> - new-property;
> - };
> - };
> - };
> -
> - fragment@5 {
> - target = <&test>;
> -
> - __overlay__ {
> - local: new-local-node {
> - new-property;
> - };
> - };
> - };
> -
> - fragment@6 {
> - target = <&test>;
> -
> - __overlay__ {
> - test-phandle = <&test>, <&local>;
> - };
> - };
> -
> - fragment@7 {
> - target = <&test>;
> -
> - __overlay__ {
> - test-several-phandle = <&local>, <&local>;
> - };
> - };
> -
> - fragment@8 {
> - target = <&test>;
> -
> - __overlay__ {
> - sub-test-node {
> - new-sub-test-property;
> - };
> - };
> - };
> -};
> +/include/ "overlay_overlay_dtc.dtsi"
Don't duplicate this, just replace it with the new style. This only
existed as essentially documentation for the libfdt overlay
application stuff. Since the new dtc won't support the old tag
format, there's no point having a test for it.
> diff --git a/tests/overlay_overlay_dtc.dtsi b/tests/overlay_overlay_dtc.dtsi
> new file mode 100644
> index 0000000..8ea8d5d
> --- /dev/null
> +++ b/tests/overlay_overlay_dtc.dtsi
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright (c) 2016 NextThing Co
> + * Copyright (c) 2016 Free Electrons
> + * Copyright (c) 2016 Konsulko Inc.
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +/ {
> + /* Test that we can change an int by another */
> + fragment@0 {
> + target = <&test>;
> +
> + __overlay__ {
> + test-int-property = <43>;
> + };
> + };
> +
> + /* Test that we can replace a string by a longer one */
> + fragment@1 {
> + target = <&test>;
> +
> + __overlay__ {
> + test-str-property = "foobar";
> + };
> + };
> +
> + /* Test that we add a new property */
> + fragment@2 {
> + target = <&test>;
> +
> + __overlay__ {
> + test-str-property-2 = "foobar2";
> + };
> + };
> +
> + /* Test that we add a new node (by phandle) */
> + fragment@3 {
> + target = <&test>;
> +
> + __overlay__ {
> + new-node {
> + new-property;
> + };
> + };
> + };
> +
> + fragment@5 {
> + target = <&test>;
> +
> + __overlay__ {
> + local: new-local-node {
> + new-property;
> + };
> + };
> + };
> +
> + fragment@6 {
> + target = <&test>;
> +
> + __overlay__ {
> + test-phandle = <&test>, <&local>;
> + };
> + };
> +
> + fragment@7 {
> + target = <&test>;
> +
> + __overlay__ {
> + test-several-phandle = <&local>, <&local>;
> + };
> + };
> +
> + fragment@8 {
> + target = <&test>;
> +
> + __overlay__ {
> + sub-test-node {
> + new-sub-test-property;
> + };
> + };
> + };
> +};
> diff --git a/tests/overlay_overlay_new_dtc.dts b/tests/overlay_overlay_new_dtc.dts
> new file mode 100644
> index 0000000..14d3f54
> --- /dev/null
> +++ b/tests/overlay_overlay_new_dtc.dts
> @@ -0,0 +1,11 @@
> +/*
> + * Copyright (c) 2016 NextThing Co
> + * Copyright (c) 2016 Free Electrons
> + * Copyright (c) 2016 Konsulko Inc.
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +/dts-v1/ /plugin/;
> +
> +/include/ "overlay_overlay_dtc.dtsi"
> diff --git a/tests/overlay_overlay_simple.dts b/tests/overlay_overlay_simple.dts
> new file mode 100644
> index 0000000..8657e1e
> --- /dev/null
> +++ b/tests/overlay_overlay_simple.dts
> @@ -0,0 +1,12 @@
> +/*
> + * Copyright (c) 2016 Konsulko Inc.
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +&test {
> + test-int-property = <43>;
> +};
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index e4139dd..74af0ff 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -181,6 +181,47 @@ overlay_tests () {
> run_dtc_test -@ -I dts -O dtb -o overlay_base_with_symbols.test.dtb overlay_base.dts
> run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.dtb overlay_overlay_dtc.dts
> run_test overlay overlay_base_with_symbols.test.dtb overlay_overlay_with_symbols.test.dtb
> +
> + # new /plugin/ format
> + run_dtc_test -@ -I dts -O dtb -o overlay_overlay_new_with_symbols.test.dtb overlay_overlay_new_dtc.dts
> + run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__symbols__"
> + run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__fixups__"
> + run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__local_fixups__"
Looks like you're mixing tabs and spaces here. I don't really mind
which, but keep it consistent at least at the same indentation level.
> + # test new magic option
> + run_dtc_test -M@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_dtc.dts
> + run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__symbols__"
> + run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__fixups__"
> + run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__local_fixups__"
> +
> + # test plugin source to dtb and back
> + run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc.test.dts overlay_overlay_with_symbols.test.dtb
> + run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.test.dtb overlay_overlay_dtc.test.dts
> + run_test dtbs_equal_ordered overlay_overlay_with_symbols.test.dtb overlay_overlay_with_symbols.test.test.dtb
> +
> + # test plugin source to dtb and back (with new magic)
> + run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc_new_magic.test.dts overlay_overlay_with_symbols_new_magic.test.dtb
> + run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.test.dtb overlay_overlay_dtc_new_magic.test.dts
> + run_test dtbs_equal_ordered overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_with_symbols_new_magic.test.test.dtb
> +
> + # test plugin auto-generation without using -@
> + run_dtc_test -I dts -O dtb -o overlay_overlay_new_with_symbols_auto.test.dtb overlay_overlay_dtc.dts
> + run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__symbols__"
> + run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__fixups__"
> + run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__local_fixups__"
> +
> + # Test suppression of fixups
> + run_dtc_test -F -@ -I dts -O dtb -o overlay_base_with_symbols_no_fixups.test.dtb overlay_base.dts
> + run_test check_path overlay_base_with_symbols_no_fixups.test.dtb exists "/__symbols__"
> + run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__fixups__"
> + run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__local_fixups__"
> +
> + # Test generation of aliases insted of symbols
> + run_dtc_test -A -I dts -O dtb -o overlay_overlay_with_aliases.dtb overlay_overlay_dtc.dts
> + run_test check_path overlay_overlay_with_aliases.dtb exists "/aliases"
> + run_test check_path overlay_overlay_with_aliases.dtb exists "/__symbols__"
> + run_test check_path overlay_overlay_with_aliases.dtb exists "/__fixups__"
> + run_test check_path overlay_overlay_with_aliases.dtb exists "/__local_fixups__"
> fi
>
> # Bad fixup tests
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax
From: David Gibson @ 2016-11-29 3:10 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480349141-14145-6-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1875 bytes --]
On Mon, Nov 28, 2016 at 06:05:39PM +0200, Pantelis Antoniou wrote:
> There exists a syntactic sugar version of overlays which
> make them simpler to write for the trivial case of a single target.
>
> Document it in the device tree object internals.
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
I'm with Frank that I think this, rather than being regarded mere
syntactic sugar, should be considered the primary way of describing
overlays.
Obviously we need to support the fully written out version as well.
> ---
> Documentation/dt-object-internal.txt | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
> index 026d4ee..d5b841e 100644
> --- a/Documentation/dt-object-internal.txt
> +++ b/Documentation/dt-object-internal.txt
> @@ -300,3 +300,19 @@ local reference is being made. No matter how phandles are allocated from dtc
> the run time loader must apply an offset to each phandle in every dynamic
> DT object loaded. The __local_fixups__ node records the place of every
> local reference so that the loader can apply the offset.
> +
> +There is an alternative syntax to the expanded form for overlays with phandle
> +targets which makes the format similar to the one using in .dtsi include files.
> +
> +So for the &ocp target example above one can simply write:
> +
> +/dts-v1/ /plugin/;
> +&ocp {
> + /* bar peripheral */
> + bar {
> + compatible = "corp,bar";
> + ... /* various properties and child nodes */
> + }
> +};
> +
> +The resulting dtb object is identical.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst
From: John Youn @ 2016-11-29 3:32 UTC (permalink / raw)
To: Christian Lamparter, John Youn
Cc: Rob Herring, Stefan Wahren, Felipe Balbi,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Rutland,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
Michael Ellerman, Paul Mackerras, Benjamin Herrenschmidt
In-Reply-To: <1495076.fZ1uLW9fli@debian64>
On 11/22/2016 12:51 PM, Christian Lamparter wrote:
> On Monday, November 21, 2016 7:32:30 PM CET John Youn wrote:
>> On 11/21/2016 1:10 PM, Christian Lamparter wrote:
>>> On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
>>>> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
>>>>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
>>>>>> Also, perhaps you should allow that the compatible string can define the
>>>>>> default.
>>>>>>
>>>>> I hoped you would say that :).
>>>>>
>>>>> I've attached a patch (on top of John Youn changes) [...]
>>>>> ---
>>>>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
>>>>> [...]
>>>>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
>>>>> +/* [...] */
>>>>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
>>>>> + {
>>>>> + .compatible = "amcc,dwc-otg",
>>>>> + .data = (void *) GAHBCFG_HBSTLEN_INCR16,
>>>>> + },
>>>>> +};
>> [...]
>>>>>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>>>>> ret = device_property_read_string(hsotg->dev, "snps,ahb-burst", &str);
>>>>> if (ret < 0) {
>>>>> + const struct of_device_id *match;
>>>>> +
>>>>> + match = of_match_node(dwc2_compat_ahb_bursts, node);
>>>>> + if (match)
>>>>> + ret = (int)match->data;
>>>>> +
>> [...]
>>>> I'd prefer if you use the binding which requires no extra code in
>>>> dwc2.
>>> I'm fine with either option. However it think that this would require
>>> that either Mark or Rob would allow an exception to the "keep existing
>>> dts the way they are) and ack the following change to the canyonlands.dts.
>>
>> I don't know about that. Under what circumstance can the dts change?
> As far as I know, the justification for not changing the DTS is that a
> compiled DTB might be stored in an read-only ROM on a board. So it would
> be impossible to update it. Hence, the driver have work with the existing
> (and sometimes buggy or incomplete) information to stay compatible.
>
> (Note: Thankfully, the canyonlands dtb is stored in flash, it's possible
> to update it. But it is an extra step that's not done automatically
> with make install).
>
>> The canyonlands dts was binding to an external vendor driver. So it
>> wasn't documented nor expected to work with dwc2 until your recent
>> patch adding the compatible string.
>
> Oh, no that's not what happend. Let me explain why there was no "external
> vendor driver": AMCC/APM were planing to upstream their hole platform. And
> in fact, the devs tried very hard to include their driver back in 2011 [0].
> But this driver was denied inclusion back then due to:
>
> "[...]
> I would also like to point out that the same Synopsys USB controller
> is used in a number of other SoCs (especially ARM chips), and
> supported by other drivers, some of these even in mainline.
>
> See http://thread.gmane.org/gmane.linux.usb.general/61714/focus=62139
> for a related thread.
>
> Instead of trying to add a completely new driver to mainline (and one
> which has been repeatedly been rejected), I vote for focusing on the
> existing driver code that is already in mainline, and testing and
> improving this so we can use a single implementation of this driver
> code for all SoCs that use the same IP block." [1]
>
> Of course: The listed link goes the "USB Host driver for i.MX28" driver.
> And this is an ehci-hcd like driver... Which is as you are well aware not
> that similar to the dwc2 OTG. And as far as I can tell: AMCC abandoned
> the patch series right there.
>
> Note: AMCC did however succeed in pushing your employer's Synopsys
> DesignWare SATA and DMA drivers to the kernel back then. And I'm happy
> to report that both drivers are still around and working fine for the 460EX
> (sata_dwc_460ex.c[2] and the DW AHB DMA [3]). (The drivers also work for
> different platforms than the original PPC. I know that because I helped
> Andy Shevchenko with testing and pushing some fixes to it when he was
> adding support for the Intel Quark SoC, which uses the DWC SATA and DMA).
Ok thanks for clearing that up. I understand.
For now we can just set the property to "INCR16" based on the
compatible string. Perhaps in the future do this from a glue-layer
driver which binds to all compatible strings other than "snps,dwc2".
I won't be able to do anything with this until next week though.
Regards,
John
--
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: [RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
From: Harini Katakam @ 2016-11-29 4:11 UTC (permalink / raw)
To: Andrew Lunn
Cc: Harini Katakam, Nicolas Ferre, davem, Rob Herring, Pawel Moll,
Mark Rutland, ijc+devicetree@hellion.org.uk, Kumar Gala,
Boris Brezillon, alexandre.belloni, netdev,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
michals@xilinx.com, Punnaiah Choudary Kalluri
In-Reply-To: <20161128163356.GJ17704@lunn.ch>
Hi Andrew,
On Mon, Nov 28, 2016 at 10:03 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Mon, Nov 28, 2016 at 03:19:14PM +0530, Harini Katakam wrote:
>> This patch is to add support for the hardware with multiple ethernet
>> MAC controllers and a single MDIO bus connected to multiple PHY devices.
>> MDIO lines are connected to any one of the ethernet MAC controllers and
>> all the PHY devices will be accessed using the PHY maintenance interface
>> in that MAC controller. This handling along with PHY functionality is
>> moved to macb_mdio.c
>>
>> Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
>> Signed-off-by: Harini Katakam <harinik@xilinx.com>
>> ---
>> drivers/net/ethernet/cadence/Makefile | 2 +-
>> drivers/net/ethernet/cadence/macb.c | 169 +++-----------------
>> drivers/net/ethernet/cadence/macb.h | 2 +
>> drivers/net/ethernet/cadence/macb_mdio.c | 266 +++++++++++++++++++++++++++++++
>> 4 files changed, 294 insertions(+), 145 deletions(-)
>> create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
>>
<snip>
>> + bus->irq = devm_kzalloc(&pdev->dev, sizeof(int) * PHY_MAX_ADDR,
>> + GFP_KERNEL);
>
> This looks wrong, or at least old. It used to be a pointer to an array,
> but it is now an actual array.
Sorry, this was a mistake.
I changed this after rebase, will update in next version.
>
>> +static const struct of_device_id macb_mdio_dt_ids[] = {
>> + { .compatible = "cdns,macb-mdio" },
>> +
>> +};
>
>
> I've not looked hard enough to know, but can you keep backwards
> compatibility? Won't old device tree's assume the mdio bus is always
> present? Now you need an explicit node otherwise there will not be an
> mdio bus?
Yes, an explicit MDIO bus is required. But I'm not sure
how to maintain backward compatibility (without using this separate
macb_mdio) and have different MACs use the same MDIO bus
with separate PHYs.
Regards,
Harini
^ permalink raw reply
* Re: [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax
From: Frank Rowand @ 2016-11-29 4:36 UTC (permalink / raw)
To: David Gibson, Pantelis Antoniou
Cc: Jon Loeliger, Grant Likely, Rob Herring, Jan Luebbe, Sascha Hauer,
Phil Elwell, Simon Glass, Maxime Ripard, Thomas Petazzoni,
Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161129031054.GI13307-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
On 11/28/16 19:10, David Gibson wrote:
> On Mon, Nov 28, 2016 at 06:05:39PM +0200, Pantelis Antoniou wrote:
>> There exists a syntactic sugar version of overlays which
>> make them simpler to write for the trivial case of a single target.
It also works for multiple targets. (See the example I provided in
my comment to v10.)
>>
>> Document it in the device tree object internals.
>>
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>
> I'm with Frank that I think this, rather than being regarded mere
> syntactic sugar, should be considered the primary way of describing
> overlays.
>
> Obviously we need to support the fully written out version as well.
If we need to support the fully written out version, can we make that
a discouraged, non-preferred method? Maybe require an option to
enable compiling this style of dts?
I can imagine some reasons to support the fully written out version,
but can we document what those reasons are?
-Frank
>
>> ---
>> Documentation/dt-object-internal.txt | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
>> index 026d4ee..d5b841e 100644
>> --- a/Documentation/dt-object-internal.txt
>> +++ b/Documentation/dt-object-internal.txt
>> @@ -300,3 +300,19 @@ local reference is being made. No matter how phandles are allocated from dtc
>> the run time loader must apply an offset to each phandle in every dynamic
>> DT object loaded. The __local_fixups__ node records the place of every
>> local reference so that the loader can apply the offset.
>> +
>> +There is an alternative syntax to the expanded form for overlays with phandle
>> +targets which makes the format similar to the one using in .dtsi include files.
>> +
>> +So for the &ocp target example above one can simply write:
>> +
>> +/dts-v1/ /plugin/;
>> +&ocp {
>> + /* bar peripheral */
>> + bar {
>> + compatible = "corp,bar";
>> + ... /* various properties and child nodes */
>> + }
>> +};
>> +
>> +The resulting dtb object is identical.
>
^ permalink raw reply
* Re: [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax
From: David Gibson @ 2016-11-29 5:10 UTC (permalink / raw)
To: Frank Rowand
Cc: Pantelis Antoniou, Jon Loeliger, Grant Likely, Rob Herring,
Jan Luebbe, Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <583D05B7.4040109-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]
On Mon, Nov 28, 2016 at 08:36:07PM -0800, Frank Rowand wrote:
> On 11/28/16 19:10, David Gibson wrote:
> > On Mon, Nov 28, 2016 at 06:05:39PM +0200, Pantelis Antoniou wrote:
> >> There exists a syntactic sugar version of overlays which
> >> make them simpler to write for the trivial case of a single target.
>
> It also works for multiple targets. (See the example I provided in
> my comment to v10.)
>
>
> >>
> >> Document it in the device tree object internals.
> >>
> >> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> >
> > I'm with Frank that I think this, rather than being regarded mere
> > syntactic sugar, should be considered the primary way of describing
> > overlays.
> >
> > Obviously we need to support the fully written out version as well.
>
> If we need to support the fully written out version, can we make that
> a discouraged, non-preferred method? Maybe require an option to
> enable compiling this style of dts?
Yeah. To avoid further proliferation of options, I'm thinking a
single "backwards compat" option which would:
- Use the dtb magic instead of dtb magic
- Disable checks which would reject explicit creation of
__overlay__ / __symbols__ / __fixups__ nodes
- Anything other special behaviour we need
> I can imagine some reasons to support the fully written out version,
> but can we document what those reasons are?
I believe the main one is the dts files in this format out in the
field. Mind you, I guess we're already requiring them to tweak how
they declare the /plugin/ option.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: phy: Add support for QUSB2 phy
From: Vivek Gautam @ 2016-11-29 5:20 UTC (permalink / raw)
To: Rob Herring
Cc: kishon, Mark Rutland, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Srinivas Kandagatla, Stephen Boyd,
linux-arm-msm
In-Reply-To: <20161128141944.4sfptmymh5kcpcwm@rob-hp-laptop>
Hi Rob,
On Mon, Nov 28, 2016 at 7:49 PM, Rob Herring <robh@kernel.org> wrote:
Thanks for reviewing the patch.
> On Tue, Nov 22, 2016 at 05:32:40PM +0530, Vivek Gautam wrote:
>> Qualcomm chipsets have QUSB2 phy controller that provides
>> HighSpeed functionality for DWC3 controller.
>> Adding dt binding information for the same.
>>
>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>> ---
>>
>> Changes since v1:
>> - New patch, forked out of the original driver patch:
>> "phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips"
>> - Updated dt bindings to remove 'hstx-trim-bit-offset' and
>> 'hstx-trim-bit-len' bindings.
>>
>> .../devicetree/bindings/phy/qcom-qusb2-phy.txt | 55 ++++++++++++++++++++++
>> 1 file changed, 55 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
>>
>> diff --git a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
>> new file mode 100644
>> index 0000000..38c8b30
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
>> @@ -0,0 +1,55 @@
>> +Qualcomm QUSB2 phy controller
>> +=============================
>> +
>> +QUSB2 controller supports LS/FS/HS usb connectivity on Qualcomm chipsets.
>> +
>> +Required properties:
>> + - compatible: compatible list, contains "qcom,msm8996-qusb2-phy".
>> + - reg: offset and length of the PHY register set.
>> + - #phy-cells: must be 0.
>> +
>> + - clocks: a list of phandles and clock-specifier pairs,
>> + one for each entry in clock-names.
>> + - clock-names: must be "cfg_ahb" for phy config clock,
>> + "ref_clk" for 19.2 MHz ref clk,
>> + "ref_clk_src" reference clock source.
>> + "iface" for phy interface clock (Optional).
>> +
>> + - vdd-phy-supply: Phandle to a regulator supply to PHY core block.
>> + - vdda-pll-supply: Phandle to 1.8V regulator supply to PHY refclk pll block.
>> + - vdda-phy-dpdm: Phandle to 3.1V regulator supply to Dp/Dm port signals.
>
> Needs '-supply'
sure, will add.
>
>> +
>> + - resets: a list of phandles and reset controller specifier pairs,
>> + one for each entry in reset-names.
>> + - reset-names: must be "phy" for reset of phy block.
>> +
>> +Optional properties:
>> + - nvmem-cells: a list of phandles to nvmem cells that contain fused
>> + tuning parameters for qusb2 phy, one for each entry
>> + in nvmem-cell-names.
>> + - nvmem-cell-names: must be "tune2_hstx_trim_efuse" for cell containing
>> + HS Tx trim value.
>> +
>> + - qcom,tcsr-syscon: Phandle to TCSR syscon register region.
>> +
>> +Example:
>> + hsphy: qusb2phy@7411000 {
>
> usb-phy@...
Or may be just 'phy' for the node name, and then label could be 'usb_hs_phy'?
[...]
Thanks
Vivek
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: phy: Add support for QUSB2 phy
From: Vivek Gautam @ 2016-11-29 5:22 UTC (permalink / raw)
To: Stephen Boyd
Cc: kishon, robh+dt, Mark Rutland, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Srinivas Kandagatla, linux-arm-msm
In-Reply-To: <20161128224953.GL6095@codeaurora.org>
Hi Stephen,
On Tue, Nov 29, 2016 at 4:19 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
Thanks for reviewing the patch-series.
> On 11/22, Vivek Gautam wrote:
>> diff --git a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
>> new file mode 100644
>> index 0000000..38c8b30
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
>> @@ -0,0 +1,55 @@
>> +Optional properties:
>> + - nvmem-cells: a list of phandles to nvmem cells that contain fused
>> + tuning parameters for qusb2 phy, one for each entry
>> + in nvmem-cell-names.
>> + - nvmem-cell-names: must be "tune2_hstx_trim_efuse" for cell containing
>
> Do we really need efuse in the name? Seems redundant given this
> is already an nvmem.
Correct, we don't need 'efuse' in the name. Thanks for pointing out.
Best Regards
Vivek
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v2 3/4] dt-bindings: phy: Add support for QMP phy
From: Vivek Gautam @ 2016-11-29 5:25 UTC (permalink / raw)
To: Stephen Boyd
Cc: kishon, robh+dt, Mark Rutland, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Srinivas Kandagatla, linux-arm-msm
In-Reply-To: <20161128225543.GM6095@codeaurora.org>
Hi,
On Tue, Nov 29, 2016 at 4:25 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 11/22, Vivek Gautam wrote:
>> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>> new file mode 100644
>> index 0000000..ffb173b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>> @@ -0,0 +1,74 @@
>> +Qualcomm QMP PHY
>> +----------------
>> +
>> +QMP phy controller supports physical layer functionality for a number of
>> +controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
>> +
>> +Required properties:
>> + - compatible: compatible list, contains:
>> + "qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
>> + "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996.
>> + - reg: list of offset and length pair of the PHY register sets.
>> + at index 0: offset and length of register set for PHY common
>> + serdes block.
>> + from index 1 - N: offset and length of register set for each lane,
>> + for N number of phy lanes (ports).
>> + - lane-offsets: array of offsets to tx, rx and pcs blocks for phy lanes.
>> + - #phy-cells: must be 1
>> + - Cell after phy phandle should be the port (lane) number.
>> + - clocks: a list of phandles and clock-specifier pairs,
>> + one for each entry in clock-names.
>> + - clock-names: must be "cfg_ahb" for phy config clock,
>> + "aux" for phy aux clock,
>> + "ref_clk" for 19.2 MHz ref clk,
>> + "ref_clk_src" for reference clock source,
>
> We typically leave "clk" out of clk names because it's redundant.
Right, will drop 'clk' from these names.
>
>> + "pipe<port-number>" for pipe clock specific to
>> + each port/lane (Optional).
>
> The pipe clocks are orphaned right now. We should add an output
> clock from the phy to go into the controller and back into the
> phy if I recall correctly. The phy should be a clock provider
> itself so it can output the pipe clock source into GCC and back
> into the phy and controller.
>
>> + - resets: a list of phandles and reset controller specifier pairs,
>> + one for each entry in reset-names.
>> + - reset-names: must be "phy" for reset of phy block,
>> + "common" for phy common block reset,
>> + "cfg" for phy's ahb cfg block reset (Optional).
>> + "port<port-number>" for reset specific to
>> + each port/lane (Optional).
>> + - vdda-phy-supply: Phandle to a regulator supply to PHY core block.
>> + - vdda-pll-supply: Phandle to 1.8V regulator supply to PHY refclk pll block.
>> +
>> +Optional properties:
>> + - vddp-ref-clk-supply: Phandle to a regulator supply to any specific refclk
>> + pll block.
>> +
>> +Example:
>> + pcie_phy: pciephy@34000 {
>
> pcie-phy or just phy as the node name?
How about just 'phy'? The label pcie_phy anyways explains the use.
Thanks
Vivek
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v2 0/3] increase TSCADC clock to 24MHz and fix ti,charge-delay to represent in nS
From: Mugunthan V N @ 2016-11-29 5:41 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Lee Jones, linux-input-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
Rob Herring, Mark Rutland, Sekhar Nori, Vignesh R,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161125095918.GZ10134-Re9dqnLqz4GzQB+pC5nmwQ@public.gmane.org>
On Friday 25 November 2016 03:29 PM, Lee Jones wrote:
> On Fri, 25 Nov 2016, Mugunthan V N wrote:
>
>> Hi Dmitry Torokhov,
>>
>> On Thursday 10 November 2016 10:05 PM, Mugunthan V N wrote:
>>> This patch series enables ADC to be clocked at 24MHz as the
>>> TI AM335x ADC driver has already adopted to use DMA to transfer
>>> ADC samples. Now ADC can generated upto 800K Samples per second
>>> with the patch [1] on AM335x BBB and AM437x GP EVM.
>>>
>>> when ADC ref clock is set at 24MHz, I am seeing some issue with
>>> touch screen pointer as the pointer jumps to random locations
>>> with free draw application. The issue is due to increase in ADC
>>> clock and charge delay for the touchscreen ADC line duration
>>> reduced.
>>>
>>> So the notation of ti,charge-delay in terms of ADC clock is
>>> wrong, it has to be represented in time and driver has to convert
>>> the charge delay time to ADC clocks based on what ADC clock
>>> frequency is set.
>>>
>>> Measured the performance with the iio_generic_buffer with the
>>> patch [2] applied
>>>
>>> Verified the touch screen on AM335x GP EVM and AM335x BBB LCD7
>>> cape with [3] dts for display and touch screen to work.
>>>
>>
>> Since there are acks from DT and MFD maintainers, can you pull the patch
>> series if you do not have any more comments.
>
> Cant do anything without *all* Acks.
>
Hi Dmitry Torokhov,
Can you provide your inputs on the patch series.
Regards
Mugunthan V N
--
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: Question regarding clocks in the DW-HDMI DT bindings
From: Laurent Pinchart @ 2016-11-29 6:04 UTC (permalink / raw)
To: Michael Turquette
Cc: Andy Yan, Vladimir Zapolskiy, Fabio Estevam, DRI mailing list,
Linux-DT, Stephen Boyd, nickey.yang-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <CAEG3pNDtMY+Pf1_w6vQEswaMVZ=jOC69R749jSFwB2NiU8r58Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Mike,
On Monday 28 Nov 2016 13:56:11 Michael Turquette wrote:
> On Fri, Nov 25, 2016 at 7:22 AM, Laurent Pinchart wrote:
> > On Friday 25 Nov 2016 10:56:53 Andy Yan wrote:
> >> On 2016年11月25日 07:26, Laurent Pinchart wrote:
> >>> On Friday 25 Nov 2016 00:16:00 Vladimir Zapolskiy wrote:
> >>>> On 11/25/2016 12:07 AM, Fabio Estevam wrote:
> >>>>> On Thu, Nov 24, 2016 at 7:16 PM, Laurent Pinchart wrote:
> >>>>>> Hi Andy,
> >>>>>>
> >>>>>> As the author of the DW-HDMI DT bindings this question is addressed
> >>>>>> to you, but information from anyone is more than welcome.
> >>>>>>
> >>>>>> The DT bindings specify two clocks named "iahb" and "isfr" but don't
> >>>>>> describe them. While I assume that the "isfr" clock corresponds to
> >>>>>> the "isfrclk" input signal of the DW HDMI, there is no "iahb" clock
> >>>>>> described in the IP core datasheet.
> >>>>>
> >>>>> i.MX6Q has a DW-HDMI IP block.
> >>>>>
> >>>>> The names in the devicetree binding matches the ones listed at the
> >>>>> i.MX6Q Reference Manual - Table 33-1. HDMI Clocks
> >>>>
> >>>> correct, for your convenience the table is copied below:
> >>>>
> >>>> Clock name | Clock Root | Description
> >>>> -----------+--------------------+-------------------------------------
> >>>> iahbclk | ahb_clk_root | Bus clock
> >>>> icecclk | ckil_sync_clk_root | CEC low-frequency clock (32kHZ)
> >>>> ihclk | ahb_clk_root | Module clock
> >>>> isfrclk | video_27m_clk_root | Internal SFR clock (video clock
> >>>> 27MHz)
> >>>>
> >>>> Here AHB stands for ARM Advanced High-performance Bus.
> >>>
> >>> That's what I suspected. I believe the "iahb" name is wrong, as the DW
> >>> HDMI TX IP core clearly documents the bus clock as being called
> >>> "iapbclk". We could rename that in the DT bindings (with compatibility
> >>> code in the driver to keep supporting the old name) but it might not be
> >>> worth it. The bindings should however document that the "iahb" clock is
> >>> the IP core's "iapbclk" bus clock.
> >>
> >> I got the clock name from I.MX6Q TRM, I also checked the name again
> >> with Rockchip IC design team now, hope to get some new information soon.
> >
> > Thank you. While at it, could you ask them which version of the DW HDMI IP
> > used in the SoC ?
> >
> >>> Another question I have about the bus clock (CC'ing the devicetree
> >>> mailing list as well as the clock maintainers) is whether it should be
> >>> made optional. The clock is obviously mandatory from a hardware point
> >>> of view (given that APB is a synchronous bus and thus requires a
> >>> clock), but in some SoCs (specifically for the Renesas SoCs) that clock
> >>> is always on and can't be controlled. We already omit bus clocks in DT
> >>> for most IP cores when the clock can never be controlled (and we also
> >>> omit a bunch of other clocks that we don't even know exist), so it
> >>> could make sense to make the clock optional. Otherwise there would be
> >>> runtime overhead trying to handle a clock that can't be controlled.
> >>
> >> If this is the case on Renesas SOCs, we can consider make the clock as
> >> optional. Or move all the clock operations to platform specific
> >> code(dw_hdmi-rockchip.c/dw_hdmi-imx.c)?
> >
> > I'd prefer keeping the code generic, otherwise we'd end up with platform-
> > specific code that would perform the same operations on most platforms.
> > I'll submit a patch soon to make the clock optional, we can discuss it
> > then.
>
> Yes, let's keep the code generic. Absence of a "standard' clock is OK
> and we should accept the small overhead incurred in providing a
> solution that works for everyone. This prevents hardware-specific
> hacks in the driver.
>
> Related: we really should model bus clocks whenever possible. I've
> seen other attempts to merge functional/logic and bus clocks into a
> single entity (e.g. a single struct clk_hw/clk_core that turns both
> clocks on and off) and this defeats some fine-grained power management
> scenarios that the hardware designers had in mind when creating
> separate controls for the clocks.
Sure, but that wasn't really the question :-) When the bus clock is separately
controllable then I agree it should be modelled separately in DT. In my case
the bus clock is always on, and I'm thus wondering whether it would be better
to make it optional in DT to reduce the runtime overhead incurred by trying to
control something that can't be controlled.
> >>>> By the way while we're discussing DW HDMI bindings specific to iMX,
> >>>> I would recommend to remove utterly hackish and iMX only "gpr"
> >>>> property from the example in bindings/display/bridge/dw_hdmi.txt
--
Regards,
Laurent Pinchart
--
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 v3 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings
From: Archit Taneja @ 2016-11-29 6:07 UTC (permalink / raw)
To: laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Archit Taneja,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480399662-8858-1-git-send-email-architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Add the regulator supply properties needed by ADV7511 and ADV7533.
The regulators are specified as optional properties since there can
be boards which have a fixed supply directly routed to the pins, and
these may not be modelled as regulator supplies.
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
v3:
- Revert back to having a common avdd-supply property for the 1.8V
supplies
Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 6532a59..13d53bc 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -56,6 +56,15 @@ Optional properties:
- adi,disable-timing-generator: Only for ADV7533. Disables the internal timing
generator. The chip will rely on the sync signals in the DSI data lanes,
rather than generate its own timings for HDMI output.
+- avdd-supply: A common 1.8V supply that powers up the AVDD, DVDD and PVDD
+ pins. On ADV7511, it also feeds to the BGVDD pin. On ADV7533, it also powers
+ up the A2VDD pin.
+- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
+ ADV7511 and V3P3 on ADV7533.
+
+ADV7533 specific supplies:
+- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be
+ either 1.2V or 1.8V.
Required nodes:
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
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] arm64: dts: exynos: Add support for s6e3ha2 panel device for TM2
From: Hoegeun Kwon @ 2016-11-29 6:16 UTC (permalink / raw)
To: kgene, krzk, devicetree
Cc: linux-samsung-soc, hoegeun.kwon, Hyungwon Hwang, Andrzej Hajda,
Chanwoo Choi
From: Hyungwon Hwang <human.hwang@samsung.com>
This patch adds the Panel Device Tree node for s6e3ha2 display
controller to Exynos5433 SoC dts.
Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 35 +++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index db879f4..d27f27d 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -252,11 +252,46 @@
reg = <1>;
dsi_out: endpoint {
+ remote-endpoint = <&dsi_in>;
samsung,burst-clock-frequency = <512000000>;
samsung,esc-clock-frequency = <16000000>;
};
};
};
+
+ panel@0 {
+ compatible = "samsung,s6e3ha2";
+ reg = <0>;
+ vdd3-supply = <&ldo27_reg>;
+ vci-supply = <&ldo28_reg>;
+ reset-gpios = <&gpg0 0 0>;
+ panel-en-gpios = <&gpf1 5 0>;
+ te-gpios = <&gpf1 3 1>;
+ power-on-delay= <5>;
+ init-delay = <120>;
+ panel-width-mm = <71>;
+ panel-height-mm = <125>;
+
+ display-timings {
+ timing-0 {
+ clock-frequency = <14874444>;
+ hactive = <1440>;
+ vactive = <2560>;
+ hfront-porch = <1>;
+ hback-porch = <1>;
+ hsync-len = <1>;
+ vfront-porch = <1>;
+ vback-porch = <15>;
+ vsync-len = <1>;
+ };
+ };
+
+ port {
+ dsi_in: endpoint {
+ remote-endpoint = <&dsi_out>;
+ };
+ };
+ };
};
&hsi2c_0 {
--
1.9.1
^ permalink raw reply related
* Re: Question regarding clocks in the DW-HDMI DT bindings
From: Michael Turquette @ 2016-11-29 6:27 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Andy Yan, Vladimir Zapolskiy, Fabio Estevam, DRI mailing list,
Linux-DT, nickey.yang-TNX95d0MmH7DzftRWevZcw, Stephen Boyd
In-Reply-To: <1938338.cMuGjElf2l@avalon>
Hi Laurent,
[fixing Stephen's email address]
On Mon, Nov 28, 2016 at 10:04 PM, Laurent Pinchart
<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> Hi Mike,
>
> On Monday 28 Nov 2016 13:56:11 Michael Turquette wrote:
>> On Fri, Nov 25, 2016 at 7:22 AM, Laurent Pinchart wrote:
>> > On Friday 25 Nov 2016 10:56:53 Andy Yan wrote:
>> >> On 2016年11月25日 07:26, Laurent Pinchart wrote:
>> >>> On Friday 25 Nov 2016 00:16:00 Vladimir Zapolskiy wrote:
>> >>>> On 11/25/2016 12:07 AM, Fabio Estevam wrote:
>> >>>>> On Thu, Nov 24, 2016 at 7:16 PM, Laurent Pinchart wrote:
>> >>>>>> Hi Andy,
>> >>>>>>
>> >>>>>> As the author of the DW-HDMI DT bindings this question is addressed
>> >>>>>> to you, but information from anyone is more than welcome.
>> >>>>>>
>> >>>>>> The DT bindings specify two clocks named "iahb" and "isfr" but don't
>> >>>>>> describe them. While I assume that the "isfr" clock corresponds to
>> >>>>>> the "isfrclk" input signal of the DW HDMI, there is no "iahb" clock
>> >>>>>> described in the IP core datasheet.
>> >>>>>
>> >>>>> i.MX6Q has a DW-HDMI IP block.
>> >>>>>
>> >>>>> The names in the devicetree binding matches the ones listed at the
>> >>>>> i.MX6Q Reference Manual - Table 33-1. HDMI Clocks
>> >>>>
>> >>>> correct, for your convenience the table is copied below:
>> >>>>
>> >>>> Clock name | Clock Root | Description
>> >>>> -----------+--------------------+-------------------------------------
>> >>>> iahbclk | ahb_clk_root | Bus clock
>> >>>> icecclk | ckil_sync_clk_root | CEC low-frequency clock (32kHZ)
>> >>>> ihclk | ahb_clk_root | Module clock
>> >>>> isfrclk | video_27m_clk_root | Internal SFR clock (video clock
>> >>>> 27MHz)
>> >>>>
>> >>>> Here AHB stands for ARM Advanced High-performance Bus.
>> >>>
>> >>> That's what I suspected. I believe the "iahb" name is wrong, as the DW
>> >>> HDMI TX IP core clearly documents the bus clock as being called
>> >>> "iapbclk". We could rename that in the DT bindings (with compatibility
>> >>> code in the driver to keep supporting the old name) but it might not be
>> >>> worth it. The bindings should however document that the "iahb" clock is
>> >>> the IP core's "iapbclk" bus clock.
>> >>
>> >> I got the clock name from I.MX6Q TRM, I also checked the name again
>> >> with Rockchip IC design team now, hope to get some new information soon.
>> >
>> > Thank you. While at it, could you ask them which version of the DW HDMI IP
>> > used in the SoC ?
>> >
>> >>> Another question I have about the bus clock (CC'ing the devicetree
>> >>> mailing list as well as the clock maintainers) is whether it should be
>> >>> made optional. The clock is obviously mandatory from a hardware point
>> >>> of view (given that APB is a synchronous bus and thus requires a
>> >>> clock), but in some SoCs (specifically for the Renesas SoCs) that clock
>> >>> is always on and can't be controlled. We already omit bus clocks in DT
>> >>> for most IP cores when the clock can never be controlled (and we also
>> >>> omit a bunch of other clocks that we don't even know exist), so it
>> >>> could make sense to make the clock optional. Otherwise there would be
>> >>> runtime overhead trying to handle a clock that can't be controlled.
>> >>
>> >> If this is the case on Renesas SOCs, we can consider make the clock as
>> >> optional. Or move all the clock operations to platform specific
>> >> code(dw_hdmi-rockchip.c/dw_hdmi-imx.c)?
>> >
>> > I'd prefer keeping the code generic, otherwise we'd end up with platform-
>> > specific code that would perform the same operations on most platforms.
>> > I'll submit a patch soon to make the clock optional, we can discuss it
>> > then.
>>
>> Yes, let's keep the code generic. Absence of a "standard' clock is OK
>> and we should accept the small overhead incurred in providing a
>> solution that works for everyone. This prevents hardware-specific
>> hacks in the driver.
>>
>> Related: we really should model bus clocks whenever possible. I've
>> seen other attempts to merge functional/logic and bus clocks into a
>> single entity (e.g. a single struct clk_hw/clk_core that turns both
>> clocks on and off) and this defeats some fine-grained power management
>> scenarios that the hardware designers had in mind when creating
>> separate controls for the clocks.
>
> Sure, but that wasn't really the question :-) When the bus clock is separately
> controllable then I agree it should be modelled separately in DT. In my case
> the bus clock is always on, and I'm thus wondering whether it would be better
> to make it optional in DT to reduce the runtime overhead incurred by trying to
> control something that can't be controlled.
I thought I answered this, but maybe not directly enough :-)
We should make the clock mandatory in DT if the physical line must be
there. This is regardless of whether a given chip/IP variant has
control over that clock; so long as the physical clock line always
exists then it is not really "optional".
In the case where there is an absence of the physical clock line, then
making it optional in DT makes sense.
As an aside, we did discuss the fact that the vast majority of clocks
are not modeled in DT, and I'm not saying that we transcribe the RTL
into DT. I'm just saying that if there is a debate over whether or not
to make a clock optional in DT, when it is always physically there,
then don't make it optional. Whether or not the control is exposed on
a particular chip is less important.
Anyways, this is more DT ridiculousness and I won't block either
method getting merged. I'm just picking my favorite color to paint the
bikeshed.
Regards,
Mike
>
>> >>>> By the way while we're discussing DW HDMI bindings specific to iMX,
>> >>>> I would recommend to remove utterly hackish and iMX only "gpr"
>> >>>> property from the example in bindings/display/bridge/dw_hdmi.txt
>
> --
> Regards,
>
> Laurent Pinchart
>
--
Michael Turquette
CEO
BayLibre - At the Heart of Embedded Linux
http://baylibre.com/
--
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 v3 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings
From: Laurent Pinchart @ 2016-11-29 6:33 UTC (permalink / raw)
To: Archit Taneja
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480399662-8858-2-git-send-email-architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Hi Archit,
Thank you for the patch.
On Tuesday 29 Nov 2016 11:37:41 Archit Taneja wrote:
> Add the regulator supply properties needed by ADV7511 and ADV7533.
>
> The regulators are specified as optional properties since there can
> be boards which have a fixed supply directly routed to the pins, and
> these may not be modelled as regulator supplies.
That's why we have support for dummy supplies in the kernel, isn't it ? Isn't
it better to make the supplies mandatory in the bindings (and obviously
handling them as optional in the driver for backward-compatibility) ?
Apart from that,
Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> v3:
> - Revert back to having a common avdd-supply property for the 1.8V
> supplies
>
> Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt | 9 ++++++
> 1 file changed, 9 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index
> 6532a59..13d53bc 100644
> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> @@ -56,6 +56,15 @@ Optional properties:
> - adi,disable-timing-generator: Only for ADV7533. Disables the internal
> timing generator. The chip will rely on the sync signals in the DSI data
> lanes, rather than generate its own timings for HDMI output.
> +- avdd-supply: A common 1.8V supply that powers up the AVDD, DVDD and PVDD
> + pins. On ADV7511, it also feeds to the BGVDD pin. On ADV7533, it also
> powers
> + up the A2VDD pin.
> +- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
> + ADV7511 and V3P3 on ADV7533.
> +
> +ADV7533 specific supplies:
> +- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be
> + either 1.2V or 1.8V.
>
> Required nodes:
--
Regards,
Laurent Pinchart
--
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 V5 00/10] PM / OPP: Multiple regulator support
From: Viresh Kumar @ 2016-11-29 6:36 UTC (permalink / raw)
To: Rafael Wysocki, nm-l0cyMroinI0, sboyd-sgV2jX0FEOL9JmXXK+q4OQ
Cc: linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
Viresh Kumar
Hi,
Some platforms (like TI) have complex DVFS configuration for CPU
devices, where multiple regulators are required to be configured to
change DVFS state of the device. This was explained well by Nishanth
earlier [1].
One of the major complaints around multiple regulators case was that the
DT isn't responsible in any way to represent the order in which multiple
supplies need to be programmed, before or after a frequency change. It
was considered in this patch and such information is left for the
platform specific OPP driver now, which can register its own
opp_set_rate() callback with the OPP core and the OPP core will then
call it during DVFS.
The patches are tested on Exynos5250 (Dual A15). I have hacked around DT
and code to pass values for multiple regulators and verified that they
are all properly read by the kernel (using debugfs interface).
Dave Gerlach has already tested [2] it on the real TI platforms and it
works well for him.
This is rebased over: linux-next branch in the PM tree.
V4->V5:
- Stephen boyd had some minor review comments and gave his Reviewed-by
tag for the rest. Only 2 patches don't have his RBY tag.
- Individual patches contain the version history from V4 to V5.
V3->V4:
- Separate out cpu-supply fix in the binding in a separate patch (Mark).
- Add more documentation to the binding to explain that the relation to
the supplies and the order of programming them is left for the
platform specific bindings and that every platform using multiple
regulators for their devices needs to provide a separate binding
document explaining their implementation (Mark).
- @Rob and Stephen: I have kept your Acks for the bindings as the
bindings only got a bit reworded (improved) since the time you guys
Acked them. Please let me know if you want more improvement in the
bindings now.
- V4 for 10/10 was sent earlier, which added a missing
rcu_read_unlock(). Nothing else changed in it.
- Added some missing Kernel documentation comments
V2->V3:
- The last patch is new
- Removed a debug leftover pr_info() message
- Renamed few names as s/set_rate/set_opp
- Removed a TODO comment (as it is done now with this series)
- created struct for min_uV and max_uV
- kerneldoc comments for structures in pm_opp.h
- s/const char */const char * const
- use kasprintf()
- Some more minor reformatting
- More Ack/RBY tags added
V1->V2:
- Ack from Rob for 1st patch
- Moved the supplies structure to pm_opp.h (Dave)
- Fixed an compilation warning.
--
viresh
[1] https://marc.info/?l=linux-pm&m=145684495832764&w=2
[2] https://marc.info/?l=linux-kernel&m=147924789305276&w=2
Viresh Kumar (10):
PM / OPP: Fix incorrect cpu-supply property in binding
PM / OPP: Reword binding supporting multiple regulators per device
PM / OPP: Don't use OPP structure outside of rcu protected section
PM / OPP: Manage supply's voltage/current in a separate structure
PM / OPP: Pass struct dev_pm_opp_supply to _set_opp_voltage()
PM / OPP: Add infrastructure to manage multiple regulators
PM / OPP: Separate out _generic_set_opp()
PM / OPP: Allow platform specific custom set_opp() callbacks
PM / OPP: Don't WARN on multiple calls to dev_pm_opp_set_regulators()
PM / OPP: Don't assume platform doesn't have regulators
Documentation/devicetree/bindings/opp/opp.txt | 27 +-
drivers/base/power/opp/core.c | 536 ++++++++++++++++++++------
drivers/base/power/opp/debugfs.c | 52 ++-
drivers/base/power/opp/of.c | 105 +++--
drivers/base/power/opp/opp.h | 22 +-
drivers/cpufreq/cpufreq-dt.c | 9 +-
include/linux/pm_opp.h | 69 +++-
7 files changed, 634 insertions(+), 186 deletions(-)
--
2.7.1.410.g6faf27b
--
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 V5 01/10] PM / OPP: Fix incorrect cpu-supply property in binding
From: Viresh Kumar @ 2016-11-29 6:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1480401041.git.viresh.kumar@linaro.org>
The regulator bindings allow the "<name>-supply" property to define a
single parent supply and not a list of parents.
Fix the wrong example code present in OPP bindings.
While at it also change the compatible string as Rob pointed out earlier
that none of A7 implementation have multiple supplies for the CPU core.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
Documentation/devicetree/bindings/opp/opp.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index ee91cbdd95ee..f0239f68d186 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -386,10 +386,12 @@ Example 4: Handling multiple regulators
/ {
cpus {
cpu@0 {
- compatible = "arm,cortex-a7";
+ compatible = "vendor,cpu-type";
...
- cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
+ vcc0-supply = <&cpu_supply0>;
+ vcc1-supply = <&cpu_supply1>;
+ vcc2-supply = <&cpu_supply2>;
operating-points-v2 = <&cpu0_opp_table>;
};
};
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V5 02/10] PM / OPP: Reword binding supporting multiple regulators per device
From: Viresh Kumar @ 2016-11-29 6:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1480401041.git.viresh.kumar@linaro.org>
On certain platforms (like TI), DVFS for a single device (CPU) requires
configuring multiple power supplies.
The OPP bindings already contains binding and example to explain this
case, but it isn't sufficient.
- There is no way for the code parsing these bindings to know which
voltage values belong to which power supply.
- It is not possible to know the order in which the supplies need to be
configured while switching OPPs.
This patch clarifies on those details by mentioning that such
information is left for the implementation specific bindings to explain.
They may want to hardcode such details or implement their own properties
to get such information. All implementations using multiple regulators
for their devices must provide a binding document explaining their
implementation.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
Documentation/devicetree/bindings/opp/opp.txt | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index f0239f68d186..9f5ca4457b5f 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -86,8 +86,14 @@ properties.
Single entry is for target voltage and three entries are for <target min max>
voltages.
- Entries for multiple regulators must be present in the same order as
- regulators are specified in device's DT node.
+ Entries for multiple regulators shall be provided in the same field separated
+ by angular brackets <>. The OPP binding doesn't provide any provisions to
+ relate the values to their power supplies or the order in which the supplies
+ need to be configured and that is left for the implementation specific
+ binding.
+
+ Entries for all regulators shall be of the same size, i.e. either all use a
+ single value or triplets.
- opp-microvolt-<name>: Named opp-microvolt property. This is exactly similar to
the above opp-microvolt property, but allows multiple voltage ranges to be
@@ -104,10 +110,13 @@ properties.
Should only be set if opp-microvolt is set for the OPP.
- Entries for multiple regulators must be present in the same order as
- regulators are specified in device's DT node. If this property isn't required
- for few regulators, then this should be marked as zero for them. If it isn't
- required for any regulator, then this property need not be present.
+ Entries for multiple regulators shall be provided in the same field separated
+ by angular brackets <>. If current values aren't required for a regulator,
+ then it shall be filled with 0. If current values aren't required for any of
+ the regulators, then this field is not required. The OPP binding doesn't
+ provide any provisions to relate the values to their power supplies or the
+ order in which the supplies need to be configured and that is left for the
+ implementation specific binding.
- opp-microamp-<name>: Named opp-microamp property. Similar to
opp-microvolt-<name> property, but for microamp instead.
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V5 03/10] PM / OPP: Don't use OPP structure outside of rcu protected section
From: Viresh Kumar @ 2016-11-29 6:36 UTC (permalink / raw)
To: Rafael Wysocki, nm-l0cyMroinI0, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
Viresh Kumar
Cc: linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
Viresh Kumar, # v4 . 6+
In-Reply-To: <cover.1480401041.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
The OPP structure must not be used out of the rcu protected section.
Cache the values to be used in separate variables instead.
Cc: # v4.6+ <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Tested-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
---
drivers/base/power/opp/core.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 4c7c6da7a989..056527a3fb4e 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -584,6 +584,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
struct clk *clk;
unsigned long freq, old_freq;
unsigned long u_volt, u_volt_min, u_volt_max;
+ unsigned long old_u_volt, old_u_volt_min, old_u_volt_max;
int ret;
if (unlikely(!target_freq)) {
@@ -633,6 +634,14 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
return ret;
}
+ if (IS_ERR(old_opp)) {
+ old_u_volt = 0;
+ } else {
+ old_u_volt = old_opp->u_volt;
+ old_u_volt_min = old_opp->u_volt_min;
+ old_u_volt_max = old_opp->u_volt_max;
+ }
+
u_volt = opp->u_volt;
u_volt_min = opp->u_volt_min;
u_volt_max = opp->u_volt_max;
@@ -677,9 +686,10 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
__func__, old_freq);
restore_voltage:
/* This shouldn't harm even if the voltages weren't updated earlier */
- if (!IS_ERR(old_opp))
- _set_opp_voltage(dev, reg, old_opp->u_volt,
- old_opp->u_volt_min, old_opp->u_volt_max);
+ if (old_u_volt) {
+ _set_opp_voltage(dev, reg, old_u_volt, old_u_volt_min,
+ old_u_volt_max);
+ }
return ret;
}
--
2.7.1.410.g6faf27b
--
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 V5 04/10] PM / OPP: Manage supply's voltage/current in a separate structure
From: Viresh Kumar @ 2016-11-29 6:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1480401041.git.viresh.kumar@linaro.org>
This is a preparatory step for multiple regulator per device support.
Move the voltage/current variables to a new structure.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/base/power/opp/core.c | 44 +++++++++++++++++++++-------------------
drivers/base/power/opp/debugfs.c | 8 ++++----
drivers/base/power/opp/of.c | 18 ++++++++--------
drivers/base/power/opp/opp.h | 11 +++-------
include/linux/pm_opp.h | 16 +++++++++++++++
5 files changed, 55 insertions(+), 42 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 056527a3fb4e..8d6006151c9a 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -112,7 +112,7 @@ unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
if (IS_ERR_OR_NULL(tmp_opp))
pr_err("%s: Invalid parameters\n", __func__);
else
- v = tmp_opp->u_volt;
+ v = tmp_opp->supply.u_volt;
return v;
}
@@ -246,10 +246,10 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
if (!opp->available)
continue;
- if (opp->u_volt_min < min_uV)
- min_uV = opp->u_volt_min;
- if (opp->u_volt_max > max_uV)
- max_uV = opp->u_volt_max;
+ if (opp->supply.u_volt_min < min_uV)
+ min_uV = opp->supply.u_volt_min;
+ if (opp->supply.u_volt_max > max_uV)
+ max_uV = opp->supply.u_volt_max;
}
rcu_read_unlock();
@@ -637,14 +637,14 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
if (IS_ERR(old_opp)) {
old_u_volt = 0;
} else {
- old_u_volt = old_opp->u_volt;
- old_u_volt_min = old_opp->u_volt_min;
- old_u_volt_max = old_opp->u_volt_max;
+ old_u_volt = old_opp->supply.u_volt;
+ old_u_volt_min = old_opp->supply.u_volt_min;
+ old_u_volt_max = old_opp->supply.u_volt_max;
}
- u_volt = opp->u_volt;
- u_volt_min = opp->u_volt_min;
- u_volt_max = opp->u_volt_max;
+ u_volt = opp->supply.u_volt;
+ u_volt_min = opp->supply.u_volt_min;
+ u_volt_max = opp->supply.u_volt_max;
reg = opp_table->regulator;
@@ -957,10 +957,11 @@ static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,
struct regulator *reg = opp_table->regulator;
if (!IS_ERR(reg) &&
- !regulator_is_supported_voltage(reg, opp->u_volt_min,
- opp->u_volt_max)) {
+ !regulator_is_supported_voltage(reg, opp->supply.u_volt_min,
+ opp->supply.u_volt_max)) {
pr_warn("%s: OPP minuV: %lu maxuV: %lu, not supported by regulator\n",
- __func__, opp->u_volt_min, opp->u_volt_max);
+ __func__, opp->supply.u_volt_min,
+ opp->supply.u_volt_max);
return false;
}
@@ -993,11 +994,12 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
/* Duplicate OPPs */
dev_warn(dev, "%s: duplicate OPPs detected. Existing: freq: %lu, volt: %lu, enabled: %d. New: freq: %lu, volt: %lu, enabled: %d\n",
- __func__, opp->rate, opp->u_volt, opp->available,
- new_opp->rate, new_opp->u_volt, new_opp->available);
+ __func__, opp->rate, opp->supply.u_volt,
+ opp->available, new_opp->rate, new_opp->supply.u_volt,
+ new_opp->available);
- return opp->available && new_opp->u_volt == opp->u_volt ?
- 0 : -EEXIST;
+ return opp->available &&
+ new_opp->supply.u_volt == opp->supply.u_volt ? 0 : -EEXIST;
}
new_opp->opp_table = opp_table;
@@ -1064,9 +1066,9 @@ int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
/* populate the opp table */
new_opp->rate = freq;
tol = u_volt * opp_table->voltage_tolerance_v1 / 100;
- new_opp->u_volt = u_volt;
- new_opp->u_volt_min = u_volt - tol;
- new_opp->u_volt_max = u_volt + tol;
+ new_opp->supply.u_volt = u_volt;
+ new_opp->supply.u_volt_min = u_volt - tol;
+ new_opp->supply.u_volt_max = u_volt + tol;
new_opp->available = true;
new_opp->dynamic = dynamic;
diff --git a/drivers/base/power/opp/debugfs.c b/drivers/base/power/opp/debugfs.c
index ef1ae6b52042..c897676ca35f 100644
--- a/drivers/base/power/opp/debugfs.c
+++ b/drivers/base/power/opp/debugfs.c
@@ -63,16 +63,16 @@ int opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
if (!debugfs_create_ulong("rate_hz", S_IRUGO, d, &opp->rate))
return -ENOMEM;
- if (!debugfs_create_ulong("u_volt_target", S_IRUGO, d, &opp->u_volt))
+ if (!debugfs_create_ulong("u_volt_target", S_IRUGO, d, &opp->supply.u_volt))
return -ENOMEM;
- if (!debugfs_create_ulong("u_volt_min", S_IRUGO, d, &opp->u_volt_min))
+ if (!debugfs_create_ulong("u_volt_min", S_IRUGO, d, &opp->supply.u_volt_min))
return -ENOMEM;
- if (!debugfs_create_ulong("u_volt_max", S_IRUGO, d, &opp->u_volt_max))
+ if (!debugfs_create_ulong("u_volt_max", S_IRUGO, d, &opp->supply.u_volt_max))
return -ENOMEM;
- if (!debugfs_create_ulong("u_amp", S_IRUGO, d, &opp->u_amp))
+ if (!debugfs_create_ulong("u_amp", S_IRUGO, d, &opp->supply.u_amp))
return -ENOMEM;
if (!debugfs_create_ulong("clock_latency_ns", S_IRUGO, d,
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index 5b3755e49731..bdf409d42126 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -148,14 +148,14 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
return -EINVAL;
}
- opp->u_volt = microvolt[0];
+ opp->supply.u_volt = microvolt[0];
if (count == 1) {
- opp->u_volt_min = opp->u_volt;
- opp->u_volt_max = opp->u_volt;
+ opp->supply.u_volt_min = opp->supply.u_volt;
+ opp->supply.u_volt_max = opp->supply.u_volt;
} else {
- opp->u_volt_min = microvolt[1];
- opp->u_volt_max = microvolt[2];
+ opp->supply.u_volt_min = microvolt[1];
+ opp->supply.u_volt_max = microvolt[2];
}
/* Search for "opp-microamp-<name>" */
@@ -173,7 +173,7 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
}
if (prop && !of_property_read_u32(opp->np, name, &val))
- opp->u_amp = val;
+ opp->supply.u_amp = val;
return 0;
}
@@ -303,9 +303,9 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
mutex_unlock(&opp_table_lock);
pr_debug("%s: turbo:%d rate:%lu uv:%lu uvmin:%lu uvmax:%lu latency:%lu\n",
- __func__, new_opp->turbo, new_opp->rate, new_opp->u_volt,
- new_opp->u_volt_min, new_opp->u_volt_max,
- new_opp->clock_latency_ns);
+ __func__, new_opp->turbo, new_opp->rate,
+ new_opp->supply.u_volt, new_opp->supply.u_volt_min,
+ new_opp->supply.u_volt_max, new_opp->clock_latency_ns);
/*
* Notify the changes in the availability of the operable
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 96cd30ac6c1d..8a02516542c2 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -61,10 +61,7 @@ extern struct list_head opp_tables;
* @turbo: true if turbo (boost) OPP
* @suspend: true if suspend OPP
* @rate: Frequency in hertz
- * @u_volt: Target voltage in microvolts corresponding to this OPP
- * @u_volt_min: Minimum voltage in microvolts corresponding to this OPP
- * @u_volt_max: Maximum voltage in microvolts corresponding to this OPP
- * @u_amp: Maximum current drawn by the device in microamperes
+ * @supply: Power supply voltage/current values
* @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
* frequency from any other OPP's frequency.
* @opp_table: points back to the opp_table struct this opp belongs to
@@ -83,10 +80,8 @@ struct dev_pm_opp {
bool suspend;
unsigned long rate;
- unsigned long u_volt;
- unsigned long u_volt_min;
- unsigned long u_volt_max;
- unsigned long u_amp;
+ struct dev_pm_opp_supply supply;
+
unsigned long clock_latency_ns;
struct opp_table *opp_table;
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index bca26157f5b6..f69126e2bb59 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -24,6 +24,22 @@ enum dev_pm_opp_event {
OPP_EVENT_ADD, OPP_EVENT_REMOVE, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE,
};
+/**
+ * struct dev_pm_opp_supply - Power supply voltage/current values
+ * @u_volt: Target voltage in microvolts corresponding to this OPP
+ * @u_volt_min: Minimum voltage in microvolts corresponding to this OPP
+ * @u_volt_max: Maximum voltage in microvolts corresponding to this OPP
+ * @u_amp: Maximum current drawn by the device in microamperes
+ *
+ * This structure stores the voltage/current values for a single power supply.
+ */
+struct dev_pm_opp_supply {
+ unsigned long u_volt;
+ unsigned long u_volt_min;
+ unsigned long u_volt_max;
+ unsigned long u_amp;
+};
+
#if defined(CONFIG_PM_OPP)
unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp);
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V5 05/10] PM / OPP: Pass struct dev_pm_opp_supply to _set_opp_voltage()
From: Viresh Kumar @ 2016-11-29 6:36 UTC (permalink / raw)
To: Rafael Wysocki, nm, sboyd, Viresh Kumar
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1480401041.git.viresh.kumar@linaro.org>
Pass the entire supply structure instead of all of its fields.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/base/power/opp/core.c | 44 +++++++++++++++++--------------------------
1 file changed, 17 insertions(+), 27 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 8d6006151c9a..37fad2eb0f47 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -542,8 +542,7 @@ static struct clk *_get_opp_clk(struct device *dev)
}
static int _set_opp_voltage(struct device *dev, struct regulator *reg,
- unsigned long u_volt, unsigned long u_volt_min,
- unsigned long u_volt_max)
+ struct dev_pm_opp_supply *supply)
{
int ret;
@@ -554,14 +553,15 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg,
return 0;
}
- dev_dbg(dev, "%s: voltages (mV): %lu %lu %lu\n", __func__, u_volt_min,
- u_volt, u_volt_max);
+ dev_dbg(dev, "%s: voltages (mV): %lu %lu %lu\n", __func__,
+ supply->u_volt_min, supply->u_volt, supply->u_volt_max);
- ret = regulator_set_voltage_triplet(reg, u_volt_min, u_volt,
- u_volt_max);
+ ret = regulator_set_voltage_triplet(reg, supply->u_volt_min,
+ supply->u_volt, supply->u_volt_max);
if (ret)
dev_err(dev, "%s: failed to set voltage (%lu %lu %lu mV): %d\n",
- __func__, u_volt_min, u_volt, u_volt_max, ret);
+ __func__, supply->u_volt_min, supply->u_volt,
+ supply->u_volt_max, ret);
return ret;
}
@@ -583,8 +583,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
struct regulator *reg;
struct clk *clk;
unsigned long freq, old_freq;
- unsigned long u_volt, u_volt_min, u_volt_max;
- unsigned long old_u_volt, old_u_volt_min, old_u_volt_max;
+ struct dev_pm_opp_supply old_supply, new_supply;
int ret;
if (unlikely(!target_freq)) {
@@ -634,17 +633,12 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
return ret;
}
- if (IS_ERR(old_opp)) {
- old_u_volt = 0;
- } else {
- old_u_volt = old_opp->supply.u_volt;
- old_u_volt_min = old_opp->supply.u_volt_min;
- old_u_volt_max = old_opp->supply.u_volt_max;
- }
+ if (IS_ERR(old_opp))
+ old_supply.u_volt = 0;
+ else
+ memcpy(&old_supply, &old_opp->supply, sizeof(old_supply));
- u_volt = opp->supply.u_volt;
- u_volt_min = opp->supply.u_volt_min;
- u_volt_max = opp->supply.u_volt_max;
+ memcpy(&new_supply, &opp->supply, sizeof(new_supply));
reg = opp_table->regulator;
@@ -652,8 +646,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
/* Scaling up? Scale voltage before frequency */
if (freq > old_freq) {
- ret = _set_opp_voltage(dev, reg, u_volt, u_volt_min,
- u_volt_max);
+ ret = _set_opp_voltage(dev, reg, &new_supply);
if (ret)
goto restore_voltage;
}
@@ -672,8 +665,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
/* Scaling down? Scale voltage after frequency */
if (freq < old_freq) {
- ret = _set_opp_voltage(dev, reg, u_volt, u_volt_min,
- u_volt_max);
+ ret = _set_opp_voltage(dev, reg, &new_supply);
if (ret)
goto restore_freq;
}
@@ -686,10 +678,8 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
__func__, old_freq);
restore_voltage:
/* This shouldn't harm even if the voltages weren't updated earlier */
- if (old_u_volt) {
- _set_opp_voltage(dev, reg, old_u_volt, old_u_volt_min,
- old_u_volt_max);
- }
+ if (old_supply.u_volt)
+ _set_opp_voltage(dev, reg, &old_supply);
return ret;
}
--
2.7.1.410.g6faf27b
^ 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