* Re: Question regarding clocks in the DW-HDMI DT bindings
From: Fabio Estevam @ 2016-11-25 12:43 UTC (permalink / raw)
To: Laurent Pinchart
Cc: devicetree@vger.kernel.org, Mike Turquette, Stephen Boyd,
DRI mailing list, Andy Yan, Vladimir Zapolskiy
In-Reply-To: <4461318.dYHS3nW9CP@avalon>
Hi Laurent,
On Thu, Nov 24, 2016 at 9:26 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> 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.
What if you register the clock as a "dummy" clock instead?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v9 3/4] dtc: Plugin and fixup support
From: Pantelis Antoniou @ 2016-11-25 12:44 UTC (permalink / raw)
To: David Gibson
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: <20161125112613.GK12287-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
Hi David,
> On Nov 25, 2016, at 13:26 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>
> On Fri, Nov 25, 2016 at 12:55:25PM +0200, Pantelis Antoniou wrote:
>> Hi David,
>>
>>> On Nov 25, 2016, at 06:11 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>>>
>>> On Thu, Nov 24, 2016 at 02:31:32PM +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 disabled. This is in order to give time for DT blob
>>>> methods to be updated.
>>>>
>>>> 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 | 25 +++++-
>>>> checks.c | 8 +-
>>>> dtc-lexer.l | 5 ++
>>>> dtc-parser.y | 49 +++++++++--
>>>> dtc.c | 39 +++++++-
>>>> dtc.h | 20 ++++-
>>>> fdtdump.c | 2 +-
>>>> flattree.c | 17 ++--
>>>> fstree.c | 2 +-
>>>> libfdt/fdt.c | 2 +-
>>>> libfdt/fdt.h | 3 +-
>>>> livetree.c | 225 ++++++++++++++++++++++++++++++++++++++++++++++-
>>>> tests/mangle-layout.c | 7 +-
>>>> treesource.c | 7 +-
>>>> 14 files changed, 380 insertions(+), 31 deletions(-)
>>>>
>>>> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
>>>> index 398de32..65fbf09 100644
>>>> --- a/Documentation/manual.txt
>>>> +++ b/Documentation/manual.txt
>>>> @@ -119,6 +119,24 @@ 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 new FDT magic number. By default blobs with the
>>>> + standard FDT magic number are generated.
>>>
>>> First, this description is incomplete since -M *only* affects the
>>> magic number for /plugin/ input, not in other cases. Second, the
>>> default is the wrong way around. If we make old-style the default,
>>> then new-style will never be used, which defeats the purpose.
>>
>> Then we’ll break user-space that has this assumption (i.e. that the magic is the same).
>> I can certainly do it the other way around.
>
> Which userspace in particular?
>
Kernel unflatteners in various OSes/bootloaders? All those would have to be updated since
they don’t grok the new magic number.
>>>> +
>>>> -S <bytes>
>>>> Ensure the blob at least <bytes> long, adding additional
>>>> space if needed.
>>>> @@ -146,13 +164,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 609975a..bc03d42 100644
>>>> --- a/checks.c
>>>> +++ b/checks.c
>>>> @@ -486,8 +486,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..4afc592 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"
>>>> @@ -33,6 +34,9 @@ extern void yyerror(char const *s);
>>>>
>>>> extern struct boot_info *the_boot_info;
>>>> extern bool treesource_error;
>>>> +
>>>> +/* temporary while the tree is not built */
>>>> +static unsigned int the_versionflags;
>>>
>>> Hrm. Using a global during parsing is pretty dangerous - it makes
>>> assumptions about the order in which bison will execute semantic
>>> actions that may not always be correct.
>>>
>>> It'll probably work in practice, so I *might* be convinced it's
>>> adequate for a first cut. I'm a bit reluctant though, since I suspect
>>> once merged it will become entrenched.
>>>
>>
>> We use bison, globals are the way of life. It’s not going to be used
>> anywhere else, it’s static in the parser file.
>
> Using globals to communicate the final result is inevitable (well, not
> without using the whole different re-entrant interface stuff). That
> just assumes that the start symbol's semantic action is executed
> before the parser competes, which is guaranteed.
>
> This is a different matter - using a global to communicate between
> different parts of the parser. More specifically different parts of
> the parser that are in different arms of the parse tree. That makes
> assumptions about the relative order of semantic actions which are
> *not* guaranteed. In theory the parser could generate the entire
> parse tree and semantic action dependency graph, then execute them in
> an arbitrary order (well, it would have to be a topologically sorted
> order, but that won't help).
>
I’ve given it a little bit more thought and it’s easily fixed by using
inherited attributes which is safe to do and avoid the global all together.
>> We could allocate the boot_info earlier (when the v1tag is detected) but
>> that would be quite a big change for something as trivial.
>
> That wouldn't help. You still wouldn't have a guarantee on the order
> between setting the version flags and using the version flags.
>
>>> The correct way to handle this, IMO, is not to ever attempt to apply
>>> overlays during the parse. Basically, we'd change the overall
>>> structure so that the output from the parser is not a single tree, but
>>> a list of overlays / fragments. Then, once the parse is complete, so
>>> versioninfo (which could now become a member of bootinfo) is well
>>> defined, we either apply the fragments in place (base tree) or encode
>>> them into the overlay structure (plugin mode).
>>>
>>> See https://github.com/dgibson/dtc/tree/overlay for some incomplete
>>> work I did in this direction.
>>>
>>
>> This tree is pretty stale; last commit was back in march.
>
> Yes, it was a while since I worked on it. It rebased clean, though.
>
>> I thing there’s a wrong assumption here. The purpose of this patch is not
>> to apply overlays during compile time, is to generate a blob that can be
>> applied at runtime by another piece of software.
>
> No, I realise what you're doing. But the input is in the form of a
> batch of overlays, regardless. You then have two modes of operation:
> for base trees you resolve those overlays during the compile, for
> plugins you assemble those overlays into a blob which can be applied
> later.
>
> Because it wasn't designed with runtime overlays in mind, the current
> code handles the resolution of those overlays during the parse. Your
> patch extends that by rather than resolving them, just putting them
> together with metadata into the dtbo.
>
> I'm saying that resolution or assembly should be moved out of the
> parser. Instead, the parser output would be an (ordered) list of
> overlay fragments. In the main program the two modes of operation
> become explicit: for base trees we resolve the overlays into a single
> tree, for plugins we collate the pieces into the dtbo format.
>
The case for building an overlay is only for the syntactic sugar version.
This is not the common use case at all. I could easily remove it and then
there would be no overlays built at all in the parser.
In fact the extra fixup nodes are generated now after the parse stage,
after the check stage and before the sort stage.
Perhaps it should be separated out so that we don’t get sidetracked?
>>> In addition to not making unsafe assumptions about parser operation, I
>>> think this will also allow for better error reporting. It also moves
>>> more code into plain-old-C instead of potentially hard to follow bison
>>> code fragments.
>>>
>>
>> We don’t try to apply overlays during parse, and especially in the parse code.
>
> The existing code does this, and you don't change it. Furthermore you
> absolutely do do the assembly of the fragments within the parser -
> that's exactly what add_orphan_node() called directly from semantic
> actions does. To verify this, all you need to do is look at the
> parser output: the boot_info structure has just a single tree, not a
> list of overlays.
>
Only for the un-common case.
>> The global is used only for the syntactic sugar method of turning a &ref { };
>> node into an overlay.
>
> I'm saying that treating that as mere syntactic sugar is a fundamental
> error in design. We could get away with it when the &ref {} stuff was
> always resolved immediately. Now that that resolution can be delayed
> until later, it gets worse. We should move that resolution outside of
> the parser.
>
The &ref { } stuff is sidetracking us. This is not the core issue.
In fact out in the community there are not a lot of cases where it is used.
>>
>>>> %}
>>>>
>>>> %union {
>>>> @@ -52,9 +56,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 +77,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 +109,36 @@ extern bool treesource_error;
>>>> %%
>>>>
>>>> sourcefile:
>>>> - v1tag memreserves devicetree
>>>> + versioninfo plugindecl memreserves devicetree
>>>> + {
>>>> + the_boot_info = build_boot_info($1 | $2, $3, $4,
>>>> + guess_boot_cpuid($4));
>>>> + }
>>>> + ;
>>>> +
>>>> +versioninfo:
>>>> + v1tag
>>>> {
>>>> - the_boot_info = build_boot_info($2, $3,
>>>> - guess_boot_cpuid($3));
>>>> + the_versionflags |= VF_DT_V1;
>>>> + $$ = the_versionflags;
>>>> }
>>>> ;
>>>>
>>>> v1tag:
>>>> DT_V1 ';'
>>>> + | DT_V1
>>>> | DT_V1 ';' v1tag
>>>> +
>>>> +plugindecl:
>>>> + DT_PLUGIN ';'
>>>> + {
>>>> + the_versionflags |= VF_PLUGIN;
>>>> + $$ = VF_PLUGIN;
>>>> + }
>>>> + | /* empty */
>>>> + {
>>>> + $$ = 0;
>>>> + }
>>>> ;
>>>>
>>>> memreserves:
>>>> @@ -161,10 +189,14 @@ devicetree:
>>>> {
>>>> struct node *target = get_node_by_ref($1, $2);
>>>>
>>>> - if (target)
>>>> + if (target) {
>>>> merge_nodes(target, $3);
>>>> - else
>>>> - ERROR(&@2, "Label or path %s not found", $2);
>>>> + } else {
>>>> + if (the_versionflags & VF_PLUGIN)
>>>> + add_orphan_node($1, $3, $2);
>>>> + else
>>>> + ERROR(&@2, "Label or path %s not found", $2);
>>>> + }
>>>> $$ = $1;
>>>> }
>>>> | devicetree DT_DEL_NODE DT_REF ';'
>>>> @@ -179,6 +211,11 @@ devicetree:
>>>>
>>>> $$ = $1;
>>>> }
>>>> + | /* empty */
>>>> + {
>>>> + /* build empty node */
>>>> + $$ = name_node(build_node(NULL, NULL), "");
>>>> + }
>>>> ;
>>>>
>>>> nodedef:
>>>> diff --git a/dtc.c b/dtc.c
>>>> index 9dcf640..a654267 100644
>>>> --- a/dtc.c
>>>> +++ b/dtc.c
>>>> @@ -32,6 +32,9 @@ 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 new_magic; /* use new FDT magic values for objects */
>>>>
>>>> static int is_power_of_2(int x)
>>>> {
>>>> @@ -59,7 +62,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:@AMhv";
>>>> static struct option const usage_long_opts[] = {
>>>> {"quiet", no_argument, NULL, 'q'},
>>>> {"in-format", a_argument, NULL, 'I'},
>>>> @@ -78,6 +81,9 @@ 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'},
>>>> + {"new-magic", no_argument, NULL, 'M'},
>>>> {"help", no_argument, NULL, 'h'},
>>>> {"version", no_argument, NULL, 'v'},
>>>> {NULL, no_argument, NULL, 0x0},
>>>> @@ -109,6 +115,9 @@ 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\tUse new blog magic value",
>>>> "\n\tPrint this help and exit",
>>>> "\n\tPrint version and exit",
>>>> NULL,
>>>> @@ -153,7 +162,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 +181,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 +259,16 @@ 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':
>>>> + new_magic = 1;
>>>> + break;
>>>> +
>>>> case 'h':
>>>> usage(NULL);
>>>> default:
>>>> @@ -306,6 +326,14 @@ int main(int argc, char *argv[])
>>>> fill_fullpaths(bi->dt, "");
>>>> process_checks(force, bi);
>>>>
>>>> + if (auto_label_aliases)
>>>> + generate_label_tree(bi->dt, "aliases", false);
>>>> +
>>>> + if (symbol_fixup_support) {
>>>> + generate_label_tree(bi->dt, "__symbols__", true);
>>>> + generate_fixups_tree(bi->dt);
>>>> + }
>>>> +
>>>> if (sort)
>>>> sort_tree(bi);
>>>>
>>>> @@ -318,12 +346,15 @@ int main(int argc, char *argv[])
>>>> outname, strerror(errno));
>>>> }
>>>>
>>>> + if (new_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..889b8f8 100644
>>>> --- a/dtc.h
>>>> +++ b/dtc.h
>>>> @@ -55,6 +55,9 @@ 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 new_magic; /* use new FDT magic values for objects */
>>>>
>>>> #define PHANDLE_LEGACY 0x1
>>>> #define PHANDLE_EPAPR 0x2
>>>> @@ -195,6 +198,7 @@ struct node *build_node_delete(void);
>>>> struct node *name_node(struct node *node, char *name);
>>>> struct node *chain_node(struct node *first, struct node *list);
>>>> struct node *merge_nodes(struct node *old_node, struct node *new_node);
>>>> +void add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
>>>>
>>>> void add_property(struct node *node, struct property *prop);
>>>> void delete_property_by_name(struct node *node, char *name);
>>>> @@ -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,22 @@ 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/ */
>>>> +
>>>> +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 node *dt, char *gen_node_name, bool allocph);
>>>> +void generate_fixups_tree(struct node *dt);
>>>>
>>>> /* Checks */
>>>>
>>>> @@ -253,8 +267,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;
>>>>
>>>> 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..1a2f4b1 100644
>>>> --- a/livetree.c
>>>> +++ b/livetree.c
>>>> @@ -216,6 +216,31 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>>>> return old_node;
>>>> }
>>>>
>>>> +void add_orphan_node(struct node *dt, struct node *new_node, char *ref)
>>>> +{
>>>> + static unsigned int next_orphan_fragment = 0;
>>>> + struct node *node = xmalloc(sizeof(*node));
>>>
>>> You shouldn't use a bare malloc() for a node. Use build_node() instead.
>>>
>>
>> OK.
>>
>>>> + struct property *p;
>>>> + struct data d = empty_data;
>>>> + char *name;
>>>> +
>>>> + memset(node, 0, sizeof(*node));
>>>> +
>>>> + d = data_add_marker(d, REF_PHANDLE, ref);
>>>> + d = data_append_integer(d, 0xffffffff, 32);
>>>> +
>>>> + p = build_property("target", d);
>>>> + add_property(node, p);
>>>> +
>>>> + xasprintf(&name, "fragment@%u",
>>>> + next_orphan_fragment++);
>>>> + name_node(node, name);
>>>> + name_node(new_node, "__overlay__");
>>>> +
>>>> + add_child(dt, node);
>>>> + add_child(node, new_node);
>>>> +}
>>>> +
>>>> struct node *chain_node(struct node *first, struct node *list)
>>>> {
>>>> assert(first->next_sibling == NULL);
>>>> @@ -296,6 +321,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 +377,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 +753,182 @@ 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 node *dt, struct node *node,
>>>> + struct node *an, bool allocph)
>>>> +{
>>>> + 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_escape_string(node->fullpath,
>>>> + strlen(node->fullpath)));
>>>
>>> Um.. what? The node path should absolutely not be an escape string.
>>> It shouldn't contain escapes to begin with, and if it does, we
>>> absolutely shouldn't be turning them into special characters here.
>>>
>>
>> OK, trivial enough to do. I tried to play it safe here but this is an overkill.
>
> It's not overkill, it's Just Plain Wrong. Resolving escapes is not
> idempotent. If you somehow had a node path with a backslash in it -
> well, that would be bad to begin with - but trying to resolve that
> backslash as an escape would be unambiguously worse.
>
>>>> + 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(dt, c, an, allocph);
>>>> +}
>>>> +
>>>> +void generate_label_tree(struct node *dt, char *gen_node_name, bool allocph)
>>>> +{
>>>> + struct node *an;
>>>> +
>>>> + for_each_child(dt, an)
>>>> + if (streq(gen_node_name, an->name))
>>>> + break;
>>>> +
>>>> + if (!an)
>>>> + an = build_and_name_child_node(dt, gen_node_name);
>>>> + if (!an)
>>>> + die("Could not build label node /%s\n", gen_node_name);
>>>> +
>>>> + generate_label_tree_internal(dt, dt, an, allocph);
>>>> +}
>>>> +
>>>> +static char *fixups_name = "__fixups__";
>>>> +static char *local_fixups_name = "__local_fixups__";
>>>
>>> I'd actually prefer #defines for these, and all-caps names, so it's
>>> more obvious when they're used that these are global constants.
>>>
>>
>> OK.
>>
>>>> +
>>>> +static void add_fixup_entry(struct node *dt, struct node *node,
>>>> + struct property *prop, struct marker *m)
>>>> +{
>>>> + struct node *fn; /* fixup node */
>>>> + char *entry;
>>>> +
>>>> + /* m->ref can only be a REF_PHANDLE, but check anyway */
>>>> + assert(m->type == REF_PHANDLE);
>>>> +
>>>> + /* fn is the node we're putting entries in */
>>>> + fn = get_subnode(dt, fixups_name);
>>>> + assert(fn != NULL);
>>>> +
>>>> + /* 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 add_local_fixup_entry(struct node *dt, struct node *node,
>>>> + struct property *prop, struct marker *m,
>>>> + struct node *refnode)
>>>> +{
>>>> + struct node *lfn, *wn, *nwn; /* local fixup node, walk node, new */
>>>> + uint32_t value_32;
>>>> + char *s, *e, *comp;
>>>> + int len;
>>>> +
>>>> + /* fn is the node we're putting entries in */
>>>> + lfn = get_subnode(dt, local_fixups_name);
>>>> + assert(lfn != NULL);
>>>> +
>>>> + /* walk the path components creating nodes if they don't exist */
>>>> + comp = xmalloc(strlen(node->fullpath) + 1);
>>>> + /* start skipping the first / */
>>>> + s = node->fullpath + 1;
>>>> + wn = lfn;
>>>> + while (*s) {
>>>> + /* retrieve path component */
>>>> + e = strchr(s, '/');
>>>> + if (e == NULL)
>>>> + e = s + strlen(s);
>>>> + len = e - s;
>>>> + memcpy(comp, s, len);
>>>> + comp[len] = '\0';
>>>> +
>>>> + /* if no node exists, create it */
>>>> + nwn = get_subnode(wn, comp);
>>>> + if (!nwn)
>>>> + nwn = build_and_name_child_node(wn, comp);
>>>> + wn = nwn;
>>>> +
>>>> + /* last path component */
>>>> + if (!*e)
>>>> + break;
>>>> +
>>>> + /* next path component */
>>>> + s = e + 1;
>>>> + }
>>>> + free(comp);
>>>> +
>>>> + value_32 = cpu_to_fdt32(m->offset);
>>>> + append_to_property(wn, prop->name, &value_32, sizeof(value_32));
>>>> +}
>>>> +
>>>> +static void generate_fixups_tree_internal(struct node *dt, struct node *node)
>>>> +{
>>>> + 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(dt, node, prop, m);
>>>> + else
>>>> + add_local_fixup_entry(dt, node, prop, m,
>>>> + refnode);
>>>> + }
>>>> + }
>>>> +
>>>> + for_each_child(node, c)
>>>> + generate_fixups_tree_internal(dt, c);
>>>> +}
>>>> +
>>>> +void generate_fixups_tree(struct node *dt)
>>>> +{
>>>> + struct node *an;
>>>> +
>>>> + for_each_child(dt, an)
>>>> + if (streq(fixups_name, an->name))
>>>> + break;
>>>> +
>>>> + if (!an)
>>>> + build_and_name_child_node(dt, fixups_name);
>>>> +
>>>> + for_each_child(dt, an)
>>>> + if (streq(local_fixups_name, an->name))
>>>> + break;
>>>> +
>>>> + if (!an)
>>>> + build_and_name_child_node(dt, local_fixups_name);
>>>> +
>>>> + generate_fixups_tree_internal(dt, 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);
>>>> diff --git a/treesource.c b/treesource.c
>>>> index a55d1d1..75e920d 100644
>>>> --- a/treesource.c
>>>> +++ b/treesource.c
>>>> @@ -267,7 +267,12 @@ void dt_to_source(FILE *f, struct boot_info *bi)
>>>> {
>>>> struct reserve_info *re;
>>>>
>>>> - fprintf(f, "/dts-v1/;\n\n");
>>>> + fprintf(f, "/dts-v1/");
>>>> +
>>>> + if (bi->versionflags & VF_PLUGIN)
>>>> + fprintf(f, " /plugin/");
>>>> +
>>>> + fprintf(f, ";\n\n");
>>>
>>> I'm not sure this really makes sense. The /plugin/ tag triggers the
>>> fixup construction and encoding of overlay fragments. But in an
>>> incoming dtb, that processing has already been done once, doing it
>>> again would not make sense. So I think the output should not have the
>>> /plugin/ tag, even if the input did.
>>>
>>> Unless, of course, you parsed the input into an explicit list of
>>> overlays, then output it here again as a list of overlays, not the
>>> encoded fragments. i.e. if this was the original tree:
>>>
>>> /dts-v1/ /plugin/;
>>>
>>> &somwhere {
>>> name = "value";
>>> };
>>>
>>> then legitimately the output of -I dts -O dts could be either:
>>>
>>> /dts-v1/ /plugin/;
>>>
>>> &somwhere {
>>> name = "value";
>>> };
>>>
>>> OR
>>>
>>> /dts-v1/;
>>>
>>> / {
>>> fragment@0 {
>>> target = <0xffffffff>;
>>> __overlay__{
>>> name = "value";
>>> };
>>> };
>>> __fixups__ {
>>> somewhere = "/fragment@0:target:0";
>>> };
>>> };
>>>
>>> But it doesn't make sense to combine the two: the second structure
>>> with the "/plugin/" tag.
>>>
>>
>>> Another advantage of moving the overlay application out of the parser
>>> is you can sanely produce either output, both of which could be useful
>>> in the right circumstances. You can potentially even produce the
>>> first output (or something close) from dtb input, with correct parsing
>>> of the overlay structure on dtb input.
>>>
>>
>> Yes, this makes sense. For now I’ll remove the plugin tag, but if the blob
>> was compiled with -@ you could conceivably reverse back to a form that contains
>> labels and phandle references correctly.
>>
>> But this is quite complicated to undertake in this patch series.
>>
>> To re-iterate though there is no overlay application here :)
>
> Well, I think we've both been a bit sloppy with terminology - does
> "overlay" refer to a dtbo file, or to one of the &ref { ... }
> fragments in the source, or to both.
>
> But whatever the right term is for the &ref { .. } fragments in the
> source they absolutely *are* applied during compile for the base tree
> case. And while they're not resolved fully, they are encoded into
> part of a larger tree structure for the plugin case.
>
> The confusion will be reduced if we make these
> overlays/fragments/whatever first class objects in the "live tree"
> phase of the compile, and move the resolution and/or assembly of them
> a stage that's separate from the parse. In addition, it opens the way
> for decompiling a dtbo more naturally, though as you say that's a
> moderate amount of extra work.
>
How about we get the non &ref { } case sorted out and we can talk about
the syntactic sugar version done?
v10 has been sent out.
> --
> 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
Regards
— Pantelis
^ permalink raw reply
* Re: [RFC PATCH] ARM: dts: Add support for Turris Omnia
From: Tomas Hlavacek @ 2016-11-25 12:49 UTC (permalink / raw)
To: Andrew Lunn
Cc: Mark Rutland, devicetree, Jason Cooper, Uwe Kleine-König,
Russell King, linux-kernel, Rob Herring, Gregory Clement,
linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20161124150700.GD7155@lunn.ch>
Hi!
On Thu, Nov 24, 2016 at 4:07 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> @Tomas: I think it doesn't make sense when we alternate sending
>> patches
>> without prior arrangement. Do you already work on a v5? If not I
>> can do
>> that to fix the last few comments. Not sure when a submission is too
>> late to enter v4.10, but I think the window isn't that big any more.
>
> It is getting a bit late. But maybe Linus will add in another -rc
> week.
>
>>
>> > No leds? No buttons via gpio-keys?
>>
>> The leds are controlled by a Cortex-M0 and without intervention
>> blink
>> according to a hardware function (network, power, pci). IMHO that's
>> ok
>> for an initial setup.
>
> Yes. That is fine. It is just unusual. Most boards have gpio-led and
> gpio-keys, which are easy to add. That is why i asked. Adding an LED
> driver which talks to this M0 can be added later.
Actually the WiP driver for MCU LED interface, that we use in our
kernel is here:
https://github.com/tmshlvck/omnia-linux/commit/2121afd8fbd2e4c720edcdd472b11b5303bc0dfb
It definitely needs some cleanup and it adds non-standard features
(main PWM for all LEDs, autonomous blink mode, colors) via custom /sys
files, which I suspect that is not going to be acceptable for upstream.
Let's keep it for the next iteration.
Regarding the button, we actually have one, that is connected to the
MCU and by default it sets LED brigtness autonomously, but it should be
able to generate IRQs via the MCU if we change the mode in the MCU.
I'll look into that.
Tomas
^ permalink raw reply
* Re: [PATCH] devicetree: bindings: Add vendor prefix for Oki
From: Simon Horman @ 2016-11-25 12:56 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Rob Herring, Mark Rutland, devicetree, linux-renesas-soc
In-Reply-To: <1480068945-29822-1-git-send-email-geert+renesas@glider.be>
On Fri, Nov 25, 2016 at 11:15:45AM +0100, Geert Uytterhoeven wrote:
> Already in use for "oki,ml86v7667".
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply
* Re: [PATCH v2 1/8] mfd: sun6i-prcm: Add codec analog controls sub-device for Allwinner A23
From: Maxime Ripard @ 2016-11-25 12:57 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Liam Girdwood, Mark Brown, Lee Jones,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mylene Josserand
In-Reply-To: <20161125123442.28410-2-wens-jdAy2FN1RRM@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 512 bytes --]
On Fri, Nov 25, 2016 at 08:34:35PM +0800, Chen-Yu Tsai wrote:
> The PRCM block on the A23 contains a message box like interface to
> the registers for the analog path controls of the internal codec.
>
> Add a sub-device for it.
>
> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/8] ASoC: sun4i-codec: Add support for A23 codec
From: Maxime Ripard @ 2016-11-25 12:57 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Liam Girdwood, Mark Brown, Lee Jones,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mylene Josserand
In-Reply-To: <20161125123442.28410-3-wens-jdAy2FN1RRM@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 805 bytes --]
On Fri, Nov 25, 2016 at 08:34:36PM +0800, Chen-Yu Tsai wrote:
> The codec in the A23 is similar to the one found on the A31. One key
> difference is the analog path controls are routed through the PRCM
> block. This is supported by the sun8i-codec-analog driver, and tied
> into this codec driver with the audio card's aux_dev.
>
> In addition, the A23 does not have LINEOUT, and it does not support
> headset jack detection or buttons.
>
> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v2 6/8] ASoC: sun4i-codec: Add support for H3 codec
From: Maxime Ripard @ 2016-11-25 12:58 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: devicetree, alsa-devel, Liam Girdwood, linux-kernel, Mark Brown,
Mylene Josserand, Lee Jones, linux-arm-kernel
In-Reply-To: <20161125123442.28410-7-wens@csie.org>
[-- Attachment #1.1: Type: text/plain, Size: 753 bytes --]
On Fri, Nov 25, 2016 at 08:34:40PM +0800, Chen-Yu Tsai wrote:
> The codec on the H3 is similar to the one found on the A31. One key
> difference is the analog path controls are routed through the PRCM
> block. This is supported by the sun8i-codec-analog driver, and tied
> into this codec driver with the audio card's aux_dev.
>
> In addition, the H3 has no HP (headphone) and HBIAS support, and no
> MIC3 input. The FIFO related registers are slightly rearranged.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ulf Hansson @ 2016-11-25 13:01 UTC (permalink / raw)
To: Ziji Hu
Cc: Adrian Hunter, Gregory CLEMENT,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao, Doug Jones,
Victor Gu, Wei(SOCP) Liu, Wilson Ding
In-Reply-To: <76ce72f8-4b86-3b83-544f-b9a7ef871393-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
[...]
>>
>> Moreover, we have ->enable_sdio_irq() ops that deals with this.
>>
> Yes. I mean the SDIO irqs on DAT1 line in async mode.
> This field enables our host to recognize the async SDIO irq from SDIO device.
> It controls our host side behavior, other than the SDIO device.
>
> I think ->enable_sdio_irq() is a more reasonable place to put this workraound.
> I will export sdhci_enable_sdio_irq() and implement out host own
> enable_sdio_irq() calling sdhci_enable_sdio)irq() plus this workaround.
> Does it sound reasonable to you?
Yes.
[...]
>>
>> Got it. Although I think we can cope with that fine just by using the
>> different SD/eMMC speed modes settings defined in DT (or from the
>> SDHCI caps register)
>>
> In my very opinion, I'm not sure if there is any corner case that driver cannot
> determine the eMMC card type from DT and SDHC caps.
>
>>> Unfortunately, MMC driver cannot determine the card type yet when eMMC voltage
>>> setting should be executed.
>>> Thus an flag is required here to tell driver to execute eMMC voltage setting.
>>>
>>> Besides, additional eMMC specific settings might be implemented in future, besides
>>> voltage setting. Most of them should be completed before MMC driver recognizes the
>>> card type. Thus I have to keep this flag to indicate current SDHC is for eMMC.
>>
>> I doubt you will need a generic "eMMC" flag, but let's see when we go forward.
>>
>> Currently it's clear you don't need such a flag, so I will submit a
>> change adding a DT binding for "mmc-ddr-3_3v" then we can take it from
>> there, to see if it suits your needs.
>>
>
> Actually, our eMMC is usually fixed as 1.8V.
>
> The pair "no-sd" + "no-sdio" can provide the similar information.
> But I'm not sure if it is proper to use those two property in such a way.
Well, potentially those could be used like that.
The point of why we added them was to allow hosts that don't support
the different protocols (which is somewhat true for your case, because
of signal voltage issues) to tell the mmc core about it. That instead
of having to all kind of crazy hacks in the drives, that in then
didn't work out so well.
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2 0/7] stmmac: dwmac-meson8b: configurable RGMII TX delay
From: Martin Blumenstingl @ 2016-11-25 13:01 UTC (permalink / raw)
To: linux-amlogic, devicetree, netdev, davem, khilman, mark.rutland,
robh+dt
Cc: linux-arm-kernel, alexandre.torgue, peppe.cavallaro, will.deacon,
catalin.marinas, carlo, f.fainelli, Martin Blumenstingl
In-Reply-To: <20161124143417.10178-1-martin.blumenstingl@googlemail.com>
Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
cycle TX clock delay. This seems to work fine for many boards (for
example Odroid-C2 or Amlogic's reference boards) but there are some
others where TX traffic is simply broken.
There are probably multiple reasons why it's working on some boards
while it's broken on others:
- some of Amlogic's reference boards are using a Micrel PHY
- hardware circuit design
- maybe more...
iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
TX clock delay disabled on the MAC (as it's enabled in the PHY driver).
TX throughput was virtually zero before:
$ iperf3 -c 192.168.1.100 -R
Connecting to host 192.168.1.100, port 5201
Reverse mode, remote host 192.168.1.100 is sending
[ 4] local 192.168.1.206 port 52828 connected to 192.168.1.100 port 5201
[ ID] Interval Transfer Bandwidth
[ 4] 0.00-1.00 sec 108 MBytes 901 Mbits/sec
[ 4] 1.00-2.00 sec 94.2 MBytes 791 Mbits/sec
[ 4] 2.00-3.00 sec 96.5 MBytes 810 Mbits/sec
[ 4] 3.00-4.00 sec 96.2 MBytes 808 Mbits/sec
[ 4] 4.00-5.00 sec 96.6 MBytes 810 Mbits/sec
[ 4] 5.00-6.00 sec 96.5 MBytes 810 Mbits/sec
[ 4] 6.00-7.00 sec 96.6 MBytes 810 Mbits/sec
[ 4] 7.00-8.00 sec 96.5 MBytes 809 Mbits/sec
[ 4] 8.00-9.00 sec 105 MBytes 884 Mbits/sec
[ 4] 9.00-10.00 sec 111 MBytes 934 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bandwidth Retr
[ 4] 0.00-10.00 sec 1000 MBytes 839 Mbits/sec 0 sender
[ 4] 0.00-10.00 sec 998 MBytes 837 Mbits/sec receiver
iperf Done.
$ iperf3 -c 192.168.1.100
Connecting to host 192.168.1.100, port 5201
[ 4] local 192.168.1.206 port 52832 connected to 192.168.1.100 port 5201
[ ID] Interval Transfer Bandwidth Retr Cwnd
[ 4] 0.00-1.01 sec 99.5 MBytes 829 Mbits/sec 117 139 KBytes
[ 4] 1.01-2.00 sec 105 MBytes 884 Mbits/sec 129 70.7 KBytes
[ 4] 2.00-3.01 sec 107 MBytes 889 Mbits/sec 106 187 KBytes
[ 4] 3.01-4.01 sec 105 MBytes 878 Mbits/sec 92 143 KBytes
[ 4] 4.01-5.00 sec 105 MBytes 882 Mbits/sec 140 129 KBytes
[ 4] 5.00-6.01 sec 106 MBytes 883 Mbits/sec 115 195 KBytes
[ 4] 6.01-7.00 sec 102 MBytes 863 Mbits/sec 133 70.7 KBytes
[ 4] 7.00-8.01 sec 106 MBytes 884 Mbits/sec 143 97.6 KBytes
[ 4] 8.01-9.01 sec 104 MBytes 875 Mbits/sec 124 107 KBytes
[ 4] 9.01-10.01 sec 105 MBytes 876 Mbits/sec 90 139 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bandwidth Retr
[ 4] 0.00-10.01 sec 1.02 GBytes 874 Mbits/sec 1189 sender
[ 4] 0.00-10.01 sec 1.02 GBytes 873 Mbits/sec receiver
iperf Done.
I get similar TX throughput on my Meson GXBB "MXQ Pro+" board when I
disable the PHY's TX-delay and configure a 4ms TX-delay on the MAC.
So changes to at least the RTL8211F PHY driver are needed to get it
working properly in all situations.
NOTE: patches 3-7 should be taken though the Amlogic tree. patches 1
and 2 can be taken through the net-tree or through the Amlogic tree.
There shouldn't be a runtime dependency as long as phy-mode "rgmii"
(or the not-relevant-for-this-case "rmii") is used (which is currently
the case for all ARM64 meson-gx boards) due to the dwmac-meson8b's
default 2ns TX-delay.
Changes since v1:
- renamed the devicetree property "amlogic,tx-delay" to
"amlogic,tx-delay-ns", which makes the .dts easier to read as we can
simply specify human-readable values instead of having "preprocessor
defines and calculation in human brain". Thanks to Andrew Lunn for
the suggestion!
- improved documentation to indicate when the MAC TX-delay should be
configured and how to use the PHY's TX-delay
- changed the default TX-delay in the dwmac-meson8b driver from 2ns
to 0ms when any of the rgmii-*id modes are used (the 2ns default
value still applies for phy-mode "rgmii")
- added patches to properly reset the PHY on Meson GXBB devices and to
use a similar configuration than the one we use on Meson GXL devices
(by passing a phy-handle to stmmac and defining the PHY in the mdio0
bus - patch 3-6)
- add the "amlogic,tx-delay-ns" property to all boards which are using
the RGMII PHY (patch 7)
Martin Blumenstingl (7):
net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable
ARM64: dts: meson-gx: move the MDIO node to meson-gx
ARM64: dts: meson-gxbb-odroidc2: add reset for the ethernet PHY
ARM64: dts: meson-gxbb-p20x: add reset for the ethernet PHY
ARM64: dts: meson-gxbb-vega-s95: add reset for the ethernet PHY
ARM64: dts: amlogic: add the ethernet TX delay configuration
.../devicetree/bindings/net/meson-dwmac.txt | 14 ++++++++++++
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 6 +++++
.../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 17 ++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 17 ++++++++++++++
.../boot/dts/amlogic/meson-gxbb-vega-s95.dtsi | 17 ++++++++++++++
.../boot/dts/amlogic/meson-gxl-s905d-p230.dts | 2 ++
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 6 -----
.../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 2 ++
.../arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts | 2 ++
.../net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 26 +++++++++++++++++-----
10 files changed, 97 insertions(+), 12 deletions(-)
--
2.10.2
^ permalink raw reply
* [PATCH v2 1/7] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
From: Martin Blumenstingl @ 2016-11-25 13:01 UTC (permalink / raw)
To: linux-amlogic, devicetree, netdev, davem, khilman, mark.rutland,
robh+dt
Cc: linux-arm-kernel, alexandre.torgue, peppe.cavallaro, will.deacon,
catalin.marinas, carlo, f.fainelli, Martin Blumenstingl
In-Reply-To: <20161125130156.17879-1-martin.blumenstingl@googlemail.com>
This allows configuring the RGMII TX clock delay. The RGMII clock is
generated by underlying hardware of the the Meson 8b / GXBB DWMAC glue.
The configuration depends on the actual hardware (no delay may be
needed due to the design of the actual circuit, the PHY might add this
delay, etc.).
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
Documentation/devicetree/bindings/net/meson-dwmac.txt | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/meson-dwmac.txt b/Documentation/devicetree/bindings/net/meson-dwmac.txt
index 89e62dd..f8bc540 100644
--- a/Documentation/devicetree/bindings/net/meson-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/meson-dwmac.txt
@@ -25,6 +25,20 @@ Required properties on Meson8b and newer:
- "clkin0" - first parent clock of the internal mux
- "clkin1" - second parent clock of the internal mux
+Optional properties on Meson8b and newer:
+- amlogic,tx-delay-ns: The internal RGMII TX clock delay (provided
+ by this driver) in nanoseconds. Allowed values
+ are: 0ns, 2ns, 4ns, 6ns.
+ This must be configured when the phy-mode is
+ "rgmii" (typically a value of 2ns is used in
+ this case).
+ When phy-mode is set to "rgmii-id" or
+ "rgmii-txid" the TX clock delay is already
+ provided by the PHY. In that case this
+ property should be set to 0ns (which disables
+ the TX clock delay in the MAC to prevent the
+ clock from going off because both PHY and MAC
+ are adding a delay).
Example for Meson6:
--
2.10.2
^ permalink raw reply related
* [PATCH v2 2/7] net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable
From: Martin Blumenstingl @ 2016-11-25 13:01 UTC (permalink / raw)
To: linux-amlogic, devicetree, netdev, davem, khilman, mark.rutland,
robh+dt
Cc: linux-arm-kernel, alexandre.torgue, peppe.cavallaro, will.deacon,
catalin.marinas, carlo, f.fainelli, Martin Blumenstingl
In-Reply-To: <20161125130156.17879-1-martin.blumenstingl@googlemail.com>
Prior to this patch we were using a hardcoded RGMII TX clock delay of
2ns (= 1/4 cycle of the 125MHz RGMII TX clock). This value works for
many boards, but unfortunately not for all (due to the way the actual
circuit is designed, sometimes because the TX delay is enabled in the
PHY, etc.). Making the TX delay on the MAC side configurable allows us
to support all possible hardware combinations.
This allows fixing a compatibility issue on some boards, where the
RTL8211F PHY is configured to generate the TX delay. We can now turn
off the TX delay in the MAC, because otherwise we would be applying the
delay twice (which results in non-working TX traffic).
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
.../net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 26 +++++++++++++++++-----
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 250e4ce..8ba33be 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -35,10 +35,6 @@
#define PRG_ETH0_TXDLY_SHIFT 5
#define PRG_ETH0_TXDLY_MASK GENMASK(6, 5)
-#define PRG_ETH0_TXDLY_OFF (0x0 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_QUARTER (0x1 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_HALF (0x2 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_THREE_QUARTERS (0x3 << PRG_ETH0_TXDLY_SHIFT)
/* divider for the result of m250_sel */
#define PRG_ETH0_CLK_M250_DIV_SHIFT 7
@@ -69,6 +65,8 @@ struct meson8b_dwmac {
struct clk_divider m25_div;
struct clk *m25_div_clk;
+
+ u32 tx_delay_ns;
};
static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
@@ -179,6 +177,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
{
int ret;
unsigned long clk_rate;
+ u8 tx_dly_val;
switch (dwmac->phy_mode) {
case PHY_INTERFACE_MODE_RGMII:
@@ -196,9 +195,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
PRG_ETH0_INVERTED_RMII_CLK, 0);
- /* TX clock delay - all known boards use a 1/4 cycle delay */
+ /* TX clock delay in ns = "8ns / 4 * tx_dly_val" (where
+ * 8ns are exactly one cycle of the 125MHz RGMII TX clock):
+ * 0ns = 0x0, 2ns = 0x1, 4ns = 0x2, 6ns = 0x3
+ */
+ tx_dly_val = dwmac->tx_delay_ns >> 1;
meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
- PRG_ETH0_TXDLY_QUARTER);
+ tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
break;
case PHY_INTERFACE_MODE_RMII:
@@ -277,6 +280,17 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
if (dwmac->phy_mode < 0) {
dev_err(&pdev->dev, "missing phy-mode property\n");
return -EINVAL;
+ } else if (dwmac->phy_mode != PHY_INTERFACE_MODE_RMII) {
+ ret = of_property_read_u32(pdev->dev.of_node,
+ "amlogic,tx-delay-ns",
+ &dwmac->tx_delay_ns);
+ if (ret && dwmac->phy_mode == PHY_INTERFACE_MODE_RGMII)
+ /* default to a TX clock delay of 2ns when the PHY is
+ * connected via RGMII (with RGMII_ID and RGMII_TXID
+ * the TX clock delay is generated by the PHY and thus
+ * we use the default 0ns delay in these case).
+ */
+ dwmac->tx_delay_ns = 2;
}
ret = meson8b_init_clk(dwmac);
--
2.10.2
^ permalink raw reply related
* [PATCH v2 3/7] ARM64: dts: meson-gx: move the MDIO node to meson-gx
From: Martin Blumenstingl @ 2016-11-25 13:01 UTC (permalink / raw)
To: linux-amlogic, devicetree, netdev, davem, khilman, mark.rutland,
robh+dt
Cc: linux-arm-kernel, alexandre.torgue, peppe.cavallaro, will.deacon,
catalin.marinas, carlo, f.fainelli, Martin Blumenstingl
In-Reply-To: <20161125130156.17879-1-martin.blumenstingl@googlemail.com>
stmmac's MDIO bus is currently only defined in meson-gxl.dtsi. Move it
up to meson-gx to allow us to keep the stmmac configuration for
meson-gxbb similar to the configuration on meson-gxl.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 6 ++++++
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 6 ------
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 47ab306..a2c3ca6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -371,6 +371,12 @@
interrupt-names = "macirq";
phy-mode = "rgmii";
status = "disabled";
+
+ mdio0: mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "snps,dwmac-mdio";
+ };
};
apb: apb@d0000000 {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index 3af54dc..aa3cd80 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -57,12 +57,6 @@
<&clkc CLKID_FCLK_DIV2>,
<&clkc CLKID_MPLL2>;
clock-names = "stmmaceth", "clkin0", "clkin1";
-
- mdio0: mdio {
- #address-cells = <1>;
- #size-cells = <0>;
- compatible = "snps,dwmac-mdio";
- };
};
&aobus {
--
2.10.2
^ permalink raw reply related
* [PATCH v2 4/7] ARM64: dts: meson-gxbb-odroidc2: add reset for the ethernet PHY
From: Martin Blumenstingl @ 2016-11-25 13:01 UTC (permalink / raw)
To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, khilman-rdvid1DuHRBWk0Htik3J/w,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
carlo-KA+7E9HrN00dnm+yROfE0A, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
Martin Blumenstingl
In-Reply-To: <20161125130156.17879-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This resets the ethernet PHY during boot to get the PHY into a "clean"
state. While here also specify the phy-handle of the ethmac node to
make the PHY configuration similar to the one we have on GXL devices.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index 238fbea..cbaf024 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -143,10 +143,25 @@
pinctrl-names = "default";
};
+&mdio0 {
+ ethernet_phy0: ethernet-phy@0 {
+ compatible = "ethernet-phy-id001c.c916", "ethernet-phy-ieee802.3-c22";
+ reg = <0>;
+ };
+};
+
ðmac {
status = "okay";
pinctrl-0 = <ð_rgmii_pins>;
pinctrl-names = "default";
+
+ phy-handle = <ðernet_phy0>;
+
+ snps,reset-gpio = <&gpio GPIOZ_14 0>;
+ snps,reset-delays-us = <0 10000 1000000>;
+ snps,reset-active-low;
+
+ phy-mode = "rgmii";
};
&ir {
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2 5/7] ARM64: dts: meson-gxbb-p20x: add reset for the ethernet PHY
From: Martin Blumenstingl @ 2016-11-25 13:01 UTC (permalink / raw)
To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, khilman-rdvid1DuHRBWk0Htik3J/w,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
carlo-KA+7E9HrN00dnm+yROfE0A, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
Martin Blumenstingl
In-Reply-To: <20161125130156.17879-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This resets the ethernet PHY during boot to get the PHY into a "clean"
state. While here also specify the phy-handle of the ethmac node to
make the PHY configuration similar to the one we have on GXL devices.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index 203be28..2abc553 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -134,10 +134,25 @@
pinctrl-names = "default";
};
+&mdio0 {
+ ethernet_phy0: ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <0>;
+ };
+};
+
ðmac {
status = "okay";
pinctrl-0 = <ð_rgmii_pins>;
pinctrl-names = "default";
+
+ phy-handle = <ðernet_phy0>;
+
+ snps,reset-gpio = <&gpio GPIOZ_14 0>;
+ snps,reset-delays-us = <0 10000 1000000>;
+ snps,reset-active-low;
+
+ phy-mode = "rgmii";
};
&ir {
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2 6/7] ARM64: dts: meson-gxbb-vega-s95: add reset for the ethernet PHY
From: Martin Blumenstingl @ 2016-11-25 13:01 UTC (permalink / raw)
To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, khilman-rdvid1DuHRBWk0Htik3J/w,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
carlo-KA+7E9HrN00dnm+yROfE0A, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
Martin Blumenstingl
In-Reply-To: <20161125130156.17879-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This resets the ethernet PHY during boot to get the PHY into a "clean"
state. While here also specify the phy-handle of the ethmac node to
make the PHY configuration similar to the one we have on GXL devices.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
index e59ad30..a0e92e3 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
@@ -113,10 +113,25 @@
pinctrl-names = "default";
};
+&mdio0 {
+ ethernet_phy0: ethernet-phy@0 {
+ compatible = "ethernet-phy-id001c.c916", "ethernet-phy-ieee802.3-c22";
+ reg = <0>;
+ };
+};
+
ðmac {
status = "okay";
pinctrl-0 = <ð_rgmii_pins>;
pinctrl-names = "default";
+
+ phy-handle = <ðernet_phy0>;
+
+ snps,reset-gpio = <&gpio GPIOZ_14 0>;
+ snps,reset-delays-us = <0 10000 1000000>;
+ snps,reset-active-low;
+
+ phy-mode = "rgmii";
};
&usb0_phy {
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2 7/7] ARM64: dts: amlogic: add the ethernet TX delay configuration
From: Martin Blumenstingl @ 2016-11-25 13:01 UTC (permalink / raw)
To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, khilman-rdvid1DuHRBWk0Htik3J/w,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
carlo-KA+7E9HrN00dnm+yROfE0A, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
Martin Blumenstingl
In-Reply-To: <20161125130156.17879-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This adds the amlogic,tx-delay-ns with the old (hardcoded) default value
of 2ns to all boards which are using an RGMII ethernet PHY.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 2 ++
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 2 ++
arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi | 2 ++
arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts | 2 ++
arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 2 ++
arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts | 2 ++
6 files changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index cbaf024..fdade07 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -161,6 +161,8 @@
snps,reset-delays-us = <0 10000 1000000>;
snps,reset-active-low;
+ amlogic,tx-delay-ns = <2>;
+
phy-mode = "rgmii";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index 2abc553..8172e12 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -152,6 +152,8 @@
snps,reset-delays-us = <0 10000 1000000>;
snps,reset-active-low;
+ amlogic,tx-delay-ns = <2>;
+
phy-mode = "rgmii";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
index a0e92e3..ab49712 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
@@ -131,6 +131,8 @@
snps,reset-delays-us = <0 10000 1000000>;
snps,reset-active-low;
+ amlogic,tx-delay-ns = <2>;
+
phy-mode = "rgmii";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
index f66939c..7fd11c6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
@@ -64,6 +64,8 @@
snps,reset-delays-us = <0 10000 1000000>;
snps,reset-active-low;
+ amlogic,tx-delay-ns = <2>;
+
/* External PHY is in RGMII */
phy-mode = "rgmii";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
index d320727..f83d6dc 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
@@ -156,6 +156,8 @@
snps,reset-delays-us = <0 10000 1000000>;
snps,reset-active-low;
+ amlogic,tx-delay-ns = <2>;
+
/* External PHY is in RGMII */
phy-mode = "rgmii";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
index 5dbc660..e428e29 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
@@ -64,6 +64,8 @@
snps,reset-delays-us = <0 10000 1000000>;
snps,reset-active-low;
+ amlogic,tx-delay-ns = <2>;
+
/* External PHY is in RGMII */
phy-mode = "rgmii";
};
--
2.10.2
--
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
* Re: [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ulf Hansson @ 2016-11-25 13:06 UTC (permalink / raw)
To: Ziji Hu
Cc: Adrian Hunter, Gregory CLEMENT, 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: <07e71ce4-a78e-750c-6325-0a88891519d5@marvell.com>
[...]
>>>>>> +
>>>>>> + /*
>>>>>> + * Xenon Specific property:
>>>>>> + * emmc: explicitly indicate whether this slot is for eMMC
>>>>>> + * slotno: the index of slot. Refer to SDHC_SYS_CFG_INFO register
>>>>>> + * tun-count: the interval between re-tuning
>>>>>> + * PHY type: "sdhc phy", "emmc phy 5.0" or "emmc phy 5.1"
>>>>>> + */
>>>>>> + if (of_property_read_bool(np, "marvell,xenon-emmc"))
>>>>>> + priv->emmc_slot = true;
>>>>>
>>>>> So, you need this because of the eMMC voltage switch behaviour, right?
>>>>>
>>>>> Then I would rather like to describe this a generic DT bindings for
>>>>> the eMMC voltage level support. There have acutally been some earlier
>>>>> discussions for this, but we haven't yet made some changes.
>>>>>
>>>>> I think what is missing is a mmc-ddr-3_3v DT binding, which when set,
>>>>> allows the host driver to accept I/O voltage switches to 3.3V. If not
>>>>> supported the ->start_signal_voltage_switch() ops may return -EINVAL.
>>>>> This would inform the mmc core to move on to the next supported
>>>>> voltage level. There might be some minor additional changes to the mmc
>>>>> card initialization sequence, but those should be simple.
>>>>>
>>>>> I can help out to look into this, unless you want to do it yourself of course!?
>>>>>
>>>> Yes. One of the reasons is to provide eMMC specific voltage setting.
>>>> But in my very own opinion, it should be irrelevant to voltage level.
>>>> The eMMC voltage setting on our SDHC is different from SD/SDIO voltage switch.
>>>> It will become more complex with different SOC implementation details.
>>>
>>> Got it. Although I think we can cope with that fine just by using the
>>> different SD/eMMC speed modes settings defined in DT (or from the
>>> SDHCI caps register)
>>>
>> In my very opinion, I'm not sure if there is any corner case that driver cannot
>> determine the eMMC card type from DT and SDHC caps.
>>
>>>> Unfortunately, MMC driver cannot determine the card type yet when eMMC voltage
>>>> setting should be executed.
>>>> Thus an flag is required here to tell driver to execute eMMC voltage setting.
>>>>
>>>> Besides, additional eMMC specific settings might be implemented in future, besides
>>>> voltage setting. Most of them should be completed before MMC driver recognizes the
>>>> card type. Thus I have to keep this flag to indicate current SDHC is for eMMC.
>>>
>>> I doubt you will need a generic "eMMC" flag, but let's see when we go forward.
>>>
>>> Currently it's clear you don't need such a flag, so I will submit a
>>> change adding a DT binding for "mmc-ddr-3_3v" then we can take it from
>>> there, to see if it suits your needs.
>>>
>
> Another reason for a special "xenon-emmc" property is that our host IP usually can
> support both eMMC and SD. Whether a host is used as eMMC or SD depends on the
> final implementation of the actual product.
> Thus our host driver needs to know whether current SDHC is fixed as eMMC or SD.
> So far, It can only get the information from DT.
As a matter of fact for mounted non-removable cards, such as eMMC, we
already have the option to describe some of their characteristics in
DT. Perhaps that's what you need?
Please have a look at:
Documentation/devicetree/bindings/mmc/mmc-card.txt
>
> After out host driver get the card type information from DT, it can prepare eMMC
> specific voltage, set eMMC specific mmc->caps/caps2 flags and do other
> vendor specific init, before card init procedure.
> Otherwise, our host driver has to wait until card type is determined in mmc_rescan().
>
> A generic "eMMC" flag is unnecessary. I just require a private property,
> which is only used in our host driver and DT.
>
> Thank you.
>
> Best regards,
> Hu Ziji
>
>>
>> Actually, our eMMC is usually fixed as 1.8V.
>>
>> The pair "no-sd" + "no-sdio" can provide the similar information.
>> But I'm not sure if it is proper to use those two property in such a way.
>>
>> Thank you.
>>
>> Best regards
>> Hu Ziji
>>
>>> [...]
>>>
>>> Kind regards
>>> Uffe
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
Kind regards
Uffe
^ permalink raw reply
* [PATCH 0/2] net: phy: realtek: fix RTL8211F TX-delay handling
From: Martin Blumenstingl @ 2016-11-25 13:11 UTC (permalink / raw)
To: f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
sean.wang-NuS5LvNUpcJWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
jbrunet-rdvid1DuHRBWk0Htik3J/w, Martin Blumenstingl
The RTL8211F PHY driver currently enables the TX-delay only when the
phy-mode is PHY_INTERFACE_MODE_RGMII. This is incorrect, because there
are three RGMII variations of the phy-mode which explicitly request the
PHY to enable the RX and/or TX delay, while PHY_INTERFACE_MODE_RGMII
specifies that the PHY should disable the RX and/or TX delays.
Additionally to the RTL8211F PHY driver change this contains a small
update to the phy-mode documentation to clarify the purpose of the
RGMII phy-modes.
While this may not be perfect yet it's at least a start. Please feel
free to drop this patch from this series and send an improved version
yourself.
These patches are the results of recent discussions, see [0]
[0] http://lists.infradead.org/pipermail/linux-amlogic/2016-November/001688.html
Martin Blumenstingl (2):
Documentation: devicetree: clarify usage of the RGMII phy-modes
net: phy: realtek: fix enabling of the TX-delay for RTL8211F
Documentation/devicetree/bindings/net/ethernet.txt | 24 ++++++++++++++++++----
drivers/net/phy/realtek.c | 20 ++++++++++--------
2 files changed, 32 insertions(+), 12 deletions(-)
--
2.10.2
--
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 1/2] Documentation: devicetree: clarify usage of the RGMII phy-modes
From: Martin Blumenstingl @ 2016-11-25 13:12 UTC (permalink / raw)
To: f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
sean.wang-NuS5LvNUpcJWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
jbrunet-rdvid1DuHRBWk0Htik3J/w, Martin Blumenstingl
In-Reply-To: <20161125131201.19994-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
RGMII requires special RX and/or TX delays depending on the actual
hardware circuit/wiring. These delays can be added by the MAC, the PHY
or the designer of the circuit (the latter means that no delay has to
be added by PHY or MAC).
There are 4 RGMII phy-modes used describe where a delay should be
applied:
- rgmii: the RX and TX delays are either added by the MAC (where the
exact delay is typically configurable, and can be turned off when no
extra delay is needed) or not needed at all (because the hardware
wiring adds the delay already). The PHY should neither add the RX nor
TX delay in this case.
- rgmii-rxid: configures the PHY to enable the RX delay. The MAC should
not add the RX delay in this case.
- rgmii-txid: configures the PHY to enable the TX delay. The MAC should
not add the TX delay in this case.
- rgmii-id: combines rgmii-rxid and rgmii-txid and thus configures the
PHY to enable the RX and TX delays. The MAC should neither add the RX
nor TX delay in this case.
Document these cases in the ethernet.txt documentation to make it clear
when to use each mode.
If applied incorrectly one might end up with MAC and PHY both enabling
for example the TX delay, which breaks ethernet TX traffic on 1000Mbit/s
links.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
Documentation/devicetree/bindings/net/ethernet.txt | 24 ++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt
index e1d7681..0515095 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt
@@ -9,10 +9,26 @@ The following properties are common to the Ethernet controllers:
- max-speed: number, specifies maximum speed in Mbit/s supported by the device;
- max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather than
the maximum frame size (there's contradiction in ePAPR).
-- phy-mode: string, operation mode of the PHY interface; supported values are
- "mii", "gmii", "sgmii", "qsgmii", "tbi", "rev-mii", "rmii", "rgmii", "rgmii-id",
- "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii", "trgmii"; this is now a
- de-facto standard property;
+- phy-mode: string, operation mode of the PHY interface. This is now a de-facto
+ standard property; supported values are:
+ * "mii"
+ * "gmii"
+ * "sgmii"
+ * "qsgmii"
+ * "tbi"
+ * "rev-mii"
+ * "rmii"
+ * "rgmii" (RX and TX delays are added by the MAC when required)
+ * "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, the
+ MAC should not add the RX or TX delays in this case)
+ * "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, the MAC
+ should not add an RX delay in this case)
+ * "rgmii-txid" (RGMII with internal TX delay provided by the PHY, the MAC
+ should not add an TX delay in this case)
+ * "rtbi"
+ * "smii"
+ * "xgmii"
+ * "trgmii"
- phy-connection-type: the same as "phy-mode" property but described in ePAPR;
- phy-handle: phandle, specifies a reference to a node representing a PHY
device; this property is described in ePAPR and so preferred;
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 2/2] net: phy: realtek: fix enabling of the TX-delay for RTL8211F
From: Martin Blumenstingl @ 2016-11-25 13:12 UTC (permalink / raw)
To: f.fainelli, robh+dt, mark.rutland, davem, sean.wang, netdev,
devicetree
Cc: linux-amlogic, jbrunet, Martin Blumenstingl
In-Reply-To: <20161125131201.19994-1-martin.blumenstingl@googlemail.com>
The old logic always enabled the TX-delay when the phy-mode was set to
PHY_INTERFACE_MODE_RGMII. There are dedicated phy-modes which tell the
PHY driver to enable the RX and/or TX delays:
- PHY_INTERFACE_MODE_RGMII should disable the RX and TX delay in the
PHY (if required, the MAC should add the delays in this case)
- PHY_INTERFACE_MODE_RGMII_ID should enable RX and TX delay in the PHY
- PHY_INTERFACE_MODE_RGMII_TXID should enable the TX delay in the PHY
- PHY_INTERFACE_MODE_RGMII_RXID should enable the RX delay in the PHY
(currently not supported by RTL8211F)
With this patch we enable the TX delay for PHY_INTERFACE_MODE_RGMII_ID
and PHY_INTERFACE_MODE_RGMII_TXID.
Additionally we now explicity disable the TX-delay, which seems to be
enabled automatically after a hard-reset of the PHY (by triggering it's
reset pin) to get a consistent state (as defined by the phy-mode).
This fixes a compatibility problem with some SoCs where the TX-delay was
also added by the MAC. With the TX-delay being applied twice the TX
clock was off and TX traffic was broken or very slow (<10Mbit/s) on
1000Mbit/s links.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
drivers/net/phy/realtek.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index aadd6e9..9cbe645 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -102,15 +102,19 @@ static int rtl8211f_config_init(struct phy_device *phydev)
if (ret < 0)
return ret;
- if (phydev->interface == PHY_INTERFACE_MODE_RGMII) {
- /* enable TXDLY */
- phy_write(phydev, RTL8211F_PAGE_SELECT, 0xd08);
- reg = phy_read(phydev, 0x11);
+ phy_write(phydev, RTL8211F_PAGE_SELECT, 0xd08);
+ reg = phy_read(phydev, 0x11);
+
+ /* enable TX-delay for rgmii-id and rgmii-txid, otherwise disable it */
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+ phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
reg |= RTL8211F_TX_DELAY;
- phy_write(phydev, 0x11, reg);
- /* restore to default page 0 */
- phy_write(phydev, RTL8211F_PAGE_SELECT, 0x0);
- }
+ else
+ reg &= ~RTL8211F_TX_DELAY;
+
+ phy_write(phydev, 0x11, reg);
+ /* restore to default page 0 */
+ phy_write(phydev, RTL8211F_PAGE_SELECT, 0x0);
return 0;
}
--
2.10.2
^ permalink raw reply related
* Re: Question regarding clocks in the DW-HDMI DT bindings
From: Laurent Pinchart @ 2016-11-25 13:25 UTC (permalink / raw)
To: Fabio Estevam
Cc: devicetree@vger.kernel.org, Mike Turquette, Stephen Boyd,
DRI mailing list, Andy Yan, Vladimir Zapolskiy
In-Reply-To: <CAOMZO5CgUm39E1hhLrs5SX9ALMj9BGiRKaY874exNWoQhH2AWQ@mail.gmail.com>
Hi Fabio,
On Friday 25 Nov 2016 10:43:04 Fabio Estevam wrote:
> On Thu, Nov 24, 2016 at 9:26 PM, Laurent Pinchart wrote:
> > 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.
>
> What if you register the clock as a "dummy" clock instead?
In that case I can as well specify the correct clock. My point was that, for
clocks that we know are always on, specifying them in DT will lead to runtime
overhead (registration of the clock, lookup, runtime handling, ...) that we
could as well avoid. I think the question has a broader scope than just the
dw-hdmi driver, hence the widened audience in CC.
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 1/2] mtd: spi-nor: Bindings for Rockchip serial flash controller
From: Marek Vasut @ 2016-11-25 13:30 UTC (permalink / raw)
To: Shawn Lin, David Woodhouse, Brian Norris
Cc: Cyrille Pitchen, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Heiko Stuebner
In-Reply-To: <1479437945-27918-2-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On 11/18/2016 03:59 AM, Shawn Lin wrote:
> Add binding document for the Rockchip serial flash controller.
>
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>
> Changes in v2: None
>
> .../devicetree/bindings/mtd/rockchip-sfc.txt | 31 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mtd/rockchip-sfc.txt
>
> diff --git a/Documentation/devicetree/bindings/mtd/rockchip-sfc.txt b/Documentation/devicetree/bindings/mtd/rockchip-sfc.txt
> new file mode 100644
> index 0000000..28430ce
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/rockchip-sfc.txt
> @@ -0,0 +1,31 @@
> +Rockchip Serial Flash Controller
> +
> +Required properties:
> +- compatible : Should be
> + "rockchip,rk1108-sfc", "rockchip,sfc" for ROCKCHIP RK1108.
> +- address-cells : Should be 1.
> +- size-cells : Should be 0.
Shouldn't these two props have a # prefix ? I'm not sure they should
even be part of this binding document at all.
> +- clocks: Must contain two entries for each entry in clock-names.
> +- clock-names: Shall be "sfc" for the transfer-clock, and "hsfc" for
> + the peripheral clock.
> +- interrupts : Should contain the interrupt for the device.
> +- reg: Physical base address of the controller and length of memory mapped.
> +
> +Optional properties:
> +- rockchip,sfc-no-dma: Indicate the controller doesn't support dma transfer.
DMA should be in capital letters.
> +Example:
> +nor_flash: sfc@301c0000 {
> + compatible = "rockchip,rk1108-sfc", "rockchip,sfc";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + clocks = <&cru SCLK_SFC>, <&cru HCLK_SFC>;
> + clock-names = "sfc", "hsfc";
> + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
> + reg = <0x301c0000 0x1000>;
> + spi-nor@0 {
> + compatible = "jedec,spi-nor";
> + spi-max-frequency = <12000000>;
> + reg = <0>;
> + };
> +};
>
--
Best regards,
Marek Vasut
--
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 mmc/next] mmc: sh_mmcif: Document r8a73a4, r8a7778 and sh73a0 DT bindings
From: Ulf Hansson @ 2016-11-25 13:31 UTC (permalink / raw)
To: Simon Horman
Cc: linux-mmc@vger.kernel.org, devicetree@vger.kernel.org,
Magnus Damm, Linux-Renesas
In-Reply-To: <20161125075614.GB14431@verge.net.au>
On 25 November 2016 at 08:56, Simon Horman <horms+renesas@verge.net.au> wrote:
> Simply document new compatibility strings as the driver is already
> activated using a fallback compatibility string.
>
> These compat strings are in keeping with those for all other
> Renesas ARM based SoCs with sh_mmcif enabled in mainline.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Thanks, applied for next!
Kind regards
Uffe
> ---
> Reposted with r8a7778 instead of r8a7779 in subject
>
> I have also posted patches to use these new compat strings
> to bring the DT files of the SoCs in question in-line with those
> for other Renesas ARM based SoCs with sh_mmcif enabled in mainline.
> ---
> Documentation/devicetree/bindings/mmc/renesas,mmcif.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> index ff611fa66871..e4ba92aa035e 100644
> --- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> +++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> @@ -8,11 +8,14 @@ Required properties:
>
> - compatible: should be "renesas,mmcif-<soctype>", "renesas,sh-mmcif" as a
> fallback. Examples with <soctype> are:
> + - "renesas,mmcif-r8a73a4" for the MMCIF found in r8a73a4 SoCs
> - "renesas,mmcif-r8a7740" for the MMCIF found in r8a7740 SoCs
> + - "renesas,mmcif-r8a7778" for the MMCIF found in r8a7778 SoCs
> - "renesas,mmcif-r8a7790" for the MMCIF found in r8a7790 SoCs
> - "renesas,mmcif-r8a7791" for the MMCIF found in r8a7791 SoCs
> - "renesas,mmcif-r8a7793" for the MMCIF found in r8a7793 SoCs
> - "renesas,mmcif-r8a7794" for the MMCIF found in r8a7794 SoCs
> + - "renesas,mmcif-sh73a0" for the MMCIF found in sh73a0 SoCs
>
> - clocks: reference to the functional clock
>
> --
> 2.7.0.rc3.207.g0ac5344
>
^ permalink raw reply
* Re: [PATCH 1/3] pinctrl: sx150x: use correct registers for reg_sense (sx1502 and sx1508)
From: Linus Walleij @ 2016-11-25 13:40 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel@vger.kernel.org, Rob Herring, Mark Rutland,
Andrey Smirnov, Neil Armstrong, linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <1480020320-28354-2-git-send-email-peda@axentia.se>
On Thu, Nov 24, 2016 at 9:45 PM, Peter Rosin <peda@axentia.se> wrote:
> All other registers on these chips are 8-bit, but reg_sense is 16-bits
> and therefore needs to be moved down one notch.
> This was apparently overlooked in the conversion to regmap, which only
> updated the register locations for the 16-bit chips.
>
> Fixes: 6489677f86c3 ("pinctrl-sx150x: Replace sx150x_*_cfg by means of regmap API")
> Signed-off-by: Peter Rosin <peda@axentia.se>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply
* RE: [PATCH v2 0/7] stmmac: dwmac-meson8b: configurable RGMII TX delay
From: David Laight @ 2016-11-25 13:41 UTC (permalink / raw)
To: 'Martin Blumenstingl', linux-amlogic@lists.infradead.org,
devicetree@vger.kernel.org, netdev@vger.kernel.org,
davem@davemloft.net, khilman@baylibre.com, mark.rutland@arm.com,
robh+dt@kernel.org
Cc: f.fainelli@gmail.com, alexandre.torgue@st.com,
catalin.marinas@arm.com, will.deacon@arm.com, carlo@caione.org,
peppe.cavallaro@st.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20161125130156.17879-1-martin.blumenstingl@googlemail.com>
From: Martin Blumenstingl
> Sent: 25 November 2016 13:02
> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
> cycle TX clock delay. This seems to work fine for many boards (for
> example Odroid-C2 or Amlogic's reference boards) but there are some
> others where TX traffic is simply broken.
> There are probably multiple reasons why it's working on some boards
> while it's broken on others:
> - some of Amlogic's reference boards are using a Micrel PHY
> - hardware circuit design
> - maybe more...
...
Interesting thought.
Can you test phy loopback with different delays?
It might be then possible to default to 'auto'.
David
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox