* Re: Issue with i2c-designware-platdrv's suspend/runtime-suspend handling
From: John Stultz @ 2017-01-30 22:05 UTC (permalink / raw)
To: Jarkko Nikula
Cc: Andy Shevchenko, Mika Westerberg, Wolfram Sang, linux-i2c, lkml,
zhangfei, Guodong Xu, Amit Pundir
In-Reply-To: <CALAqxLU7-77hEwUFNNdsvnRnrPPu+-eZfkDMHxeTT3MLMiKhfA@mail.gmail.com>
On Tue, Jan 24, 2017 at 2:03 PM, John Stultz <john.stultz@linaro.org> wrote:
> I noticed that with my hikey board, on resume from suspend I'm getting
> the following WARNING:
>
> [ 54.334054] ------------[ cut here ]------------
> [ 54.334077] WARNING: CPU: 0 PID: 2217 at drivers/clk/clk.c:594
> clk_core_disable+0x20/0x78
> [ 54.334080]
> [ 54.334090] CPU: 0 PID: 2217 Comm: system_server Not tainted
> 4.9.0-00046-gee9ec2c #2067
> [ 54.334094] Hardware name: HiKey Development Board (DT)
> [ 54.334099] task: ffffffc074863200 task.stack: ffffffc061d1c000
> [ 54.334105] PC is at clk_core_disable+0x20/0x78
> [ 54.334111] LR is at clk_core_disable_lock+0x20/0x38
> [ 54.334116] pc : [<ffffff80084ab728>] lr : [<ffffff80084ab848>]
> pstate: 800001c5
> [ 54.334119] sp : ffffffc061d1faf0
> [ 54.334128] x29: ffffffc061d1faf0 x28: 0000000000000000
> [ 54.334136] x27: 0000000000000002 x26: ffffff8008d47000
> [ 54.334143] x25: ffffff8008596000 x24: ffffff8008d15498
> [ 54.334151] x23: ffffff8008db9000 x22: ffffffc075074870
> [ 54.334158] x21: 0000000000000002 x20: ffffffc005f09500
> [ 54.334165] x19: 0000000000000140 x18: 0000000000000001
> [ 54.334172] x17: 0000000000000000 x16: 0000000000000000
> [ 54.334180] x15: ffffffc005f43dc0 x14: 0000000000010000
> [ 54.334187] x13: ffffff8008d906f8 x12: ffffff8008d15790
> [ 54.334196] x11: ffffff8008d15000 x10: ffffff8008d8d000
> [ 54.334204] x9 : 0000000000000000 x8 : ffffffc077f26218
> [ 54.334212] x7 : 0000000000000000 x6 : 0000000000000000
> [ 54.334220] x5 : ffffffc077f2b090 x4 : ffffffc061d1fa80
> [ 54.334228] x3 : ffffff80084ab62c x2 : ffffff80084ab62c
> [ 54.334236] x1 : 0000000000000000 x0 : ffffffc005f09500
> [ 54.334239]
> [ 54.334243] ---[ end trace 6474a624fb2fd658 ]---
> [ 54.334248] Call trace:
> [ 54.334254] Exception stack(0xffffffc061d1f920 to 0xffffffc061d1fa50)
> [ 54.334262] f920: 0000000000000140 0000008000000000
> ffffffc061d1faf0 ffffff80084ab728
> [ 54.334269] f940: ffffff8008ccc090 ffffffc074863200
> 0000000000000000 6f6674616c703d4d
> [ 54.334276] f960: ffffffc061d1f970 ffffff8008087990
> ffffffc061d1f9b0 ffffff8008450c6c
> [ 54.334283] f980: ffffff8008cc8000 ffffffc061d1fa90
> ffffff8008ccc090 ffffffc074863200
> [ 54.334290] f9a0: ffffff8008db6170 ffffffc061d1fa08
> ffffffc061d1f9c0 ffffff8008087990
> [ 54.334297] f9c0: ffffffc005f09500 0000000000000000
> ffffff80084ab62c ffffff80084ab62c
> [ 54.334303] f9e0: ffffffc061d1fa80 ffffffc077f2b090
> 0000000000000000 0000000000000000
> [ 54.334310] fa00: ffffffc077f26218 0000000000000000
> ffffff8008d8d000 ffffff8008d15000
> [ 54.334317] fa20: ffffff8008d15790 ffffff8008d906f8
> 0000000000010000 ffffffc005f43dc0
> [ 54.334322] fa40: 0000000000000000 0000000000000000
> [ 54.334328] [<ffffff80084ab728>] clk_core_disable+0x20/0x78
> [ 54.334336] [<ffffff80084ad26c>] clk_disable+0x1c/0x30
> [ 54.334349] [<ffffff80086d926c>] i2c_dw_plat_prepare_clk.isra.0+0x44/0x80
> [ 54.334356] [<ffffff80086d930c>] dw_i2c_plat_suspend+0x24/0x38
> [ 54.334367] [<ffffff800858ad54>] platform_pm_suspend+0x24/0x58
> [ 54.334377] [<ffffff8008595558>] dpm_run_callback.isra.7+0x20/0x68
> [ 54.334385] [<ffffff8008595fb4>] __device_suspend+0x10c/0x298
> [ 54.334393] [<ffffff8008597154>] dpm_suspend+0x10c/0x228
> [ 54.334401] [<ffffff8008597540>] dpm_suspend_start+0x68/0x78
> [ 54.334412] [<ffffff80080fa058>] suspend_devices_and_enter+0xb8/0x458
> [ 54.334419] [<ffffff80080fa5e4>] pm_suspend+0x1ec/0x248
> [ 54.334426] [<ffffff80080f924c>] state_store+0x94/0xa8
> [ 54.334434] [<ffffff80084366dc>] kobj_attr_store+0x14/0x28
> [ 54.334444] [<ffffff8008249a28>] sysfs_kf_write+0x48/0x58
> [ 54.334451] [<ffffff8008248d20>] kernfs_fop_write+0xb0/0x1d8
> [ 54.334461] [<ffffff80081cd714>] __vfs_write+0x1c/0x100
> [ 54.334469] [<ffffff80081cd9a0>] vfs_write+0xa0/0x1b8
> [ 54.334477] [<ffffff80081cdb9c>] SyS_write+0x44/0xa0
> [ 54.334485] [<ffffff8008082ef0>] el0_svc_naked+0x24/0x28
>
>
> Doing some further debugging, it seems the problem is that the device
> is being runtime suspended, and then at suspend time, we're calling
> the same logic, calling i2c_dw_plat_prepare_clk, which causes the clk
> count warning.
>
> Removing the runtime pm ops:
> - SET_RUNTIME_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume, NULL)
> +// SET_RUNTIME_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume, NULL)
>
> seems to avoid the warning, but clearly isn't ideal. :)
>
> Should there be some logic keep track of the suspend state for the
> dw_i2c_dev device so we don't try to suspend (or resume) it twice? Or
> is there something else I'm missing to keep this from happening?
Ping? Any thoughts on how best to fix this? I'm leaning towards
adding a suspended state to the struct dw_i2c_dev. Any objections?
thanks
-john
^ permalink raw reply
* [RFC/PATCH 1/4] Start moving overlay handling from parser into dtc core
From: Stephen Boyd @ 2017-01-31 0:02 UTC (permalink / raw)
To: David Gibson
Cc: devicetree-compiler, Frank Rowand, Pantelis Antoniou, Rob Herring,
Mark Brown, Grant Likely, Mark Rutland, mporter, Koen Kooi,
Guenter Roeck, marex, Wolfram Sang, devicetree, linux-kernel,
linux-i2c
In-Reply-To: <20170131000259.28576-1-stephen.boyd@linaro.org>
From: David Gibson <david@gibson.dropbear.id.au>
To make includes more useful, dtc for some time has allowed "overlays" in
dts files, defining the tree in several chunks which are merged together.
Recent Linux kernels make use of dynamic DT overlays which have similar
mechanics, but are implemented at DT load time with specially formatted
dtbs.
In order to provide better support for these dynamic overlays in dtc, we
want to explicitly manipulate overlays, rather than just fold them together
during the parse phase. This patch is a first step towards this.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[stephen.boyd@linaro.org: Have resolve_overlays() loop with a
cursor]
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
dtc-parser.y | 63 +++++++++++++++++++++++++++++-------------------------------
dtc.c | 13 +++++++++++++
dtc.h | 18 +++++++++++++++--
flattree.c | 2 +-
fstree.c | 2 +-
livetree.c | 40 ++++++++++++++++++++++++++++++++++++--
6 files changed, 99 insertions(+), 39 deletions(-)
diff --git a/dtc-parser.y b/dtc-parser.y
index ca3f5003427c..3d2ce372c286 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -51,6 +51,8 @@ extern bool treesource_error;
struct property *proplist;
struct node *node;
struct node *nodelist;
+ struct overlay *overlay;
+ struct overlay *overlaylist;
struct reserve_info *re;
uint64_t integer;
unsigned int flags;
@@ -83,11 +85,14 @@ extern bool treesource_error;
%type <prop> propdef
%type <proplist> proplist
-%type <node> devicetree
%type <node> nodedef
%type <node> subnode
%type <nodelist> subnodes
+%type <node> basetree
+%type <overlay> overlay
+%type <overlaylist> overlays
+
%type <integer> integer_prim
%type <integer> integer_unary
%type <integer> integer_mul
@@ -106,9 +111,9 @@ extern bool treesource_error;
%%
sourcefile:
- headers memreserves devicetree
+ headers memreserves basetree overlays
{
- parser_output = build_dt_info($1, $2, $3,
+ parser_output = build_dt_info($1, $2, $3, $4,
guess_boot_cpuid($3));
}
;
@@ -157,48 +162,40 @@ memreserve:
}
;
-devicetree:
+basetree:
'/' nodedef
{
$$ = name_node($2, "");
}
- | devicetree '/' nodedef
+ ;
+
+overlay: basetree
{
- $$ = merge_nodes($1, $3);
+ $$ = build_overlay("/", $1);
}
-
- | devicetree DT_LABEL DT_REF nodedef
+ | DT_REF nodedef
{
- struct node *target = get_node_by_ref($1, $3);
-
- if (target) {
- add_label(&target->labels, $2);
- merge_nodes(target, $4);
- } else
- ERROR(&@3, "Label or path %s not found", $3);
- $$ = $1;
+ $$ = build_overlay($1, $2);
}
- | devicetree DT_REF nodedef
+ | DT_DEL_NODE DT_REF ';'
{
- struct node *target = get_node_by_ref($1, $2);
-
- if (target)
- merge_nodes(target, $3);
- else
- ERROR(&@2, "Label or path %s not found", $2);
- $$ = $1;
+ $$ = build_overlay($2, NULL);
}
- | devicetree DT_DEL_NODE DT_REF ';'
+ | DT_LABEL overlay
{
- struct node *target = get_node_by_ref($1, $3);
-
- if (target)
- delete_node(target);
- else
- ERROR(&@3, "Label or path %s not found", $3);
-
+ add_label(&$2->dt->labels, $1);
+ $$ = $2;
+ }
+ ;
- $$ = $1;
+overlays:
+ /* empty */
+ {
+ $$ = NULL;
+ }
+ | overlay overlays
+ {
+ $$ = chain_overlay($1, $2);
}
;
diff --git a/dtc.c b/dtc.c
index a4edf4c7aebf..91e4c18b891e 100644
--- a/dtc.c
+++ b/dtc.c
@@ -58,6 +58,16 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
fill_fullpaths(child, tree->fullpath);
}
+static void resolve_overlays(struct dt_info *dti)
+{
+ struct overlay *o = dti->overlays;
+
+ while (o) {
+ apply_overlay(dti->dt, o);
+ o = o->next;
+ }
+}
+
/* Usage related data. */
#define FDT_VERSION(version) _FDT_VERSION(version)
#define _FDT_VERSION(version) #version
@@ -317,7 +327,10 @@ int main(int argc, char *argv[])
if (cmdline_boot_cpuid != -1)
dti->boot_cpuid_phys = cmdline_boot_cpuid;
+ resolve_overlays(dti);
+
fill_fullpaths(dti->dt, "");
+
process_checks(force, dti);
/* on a plugin, generate by default */
diff --git a/dtc.h b/dtc.h
index c6f125c68ba8..c4eb6421eabf 100644
--- a/dtc.h
+++ b/dtc.h
@@ -164,6 +164,12 @@ struct node {
struct label *labels;
};
+struct overlay {
+ char *target;
+ struct node *dt;
+ struct overlay *next;
+};
+
#define for_each_label_withdel(l0, l) \
for ((l) = (l0); (l); (l) = (l)->next)
@@ -208,6 +214,10 @@ void delete_node(struct node *node);
void append_to_property(struct node *node,
char *name, const void *data, int len);
+struct overlay *build_overlay(char *target, struct node *dt);
+struct overlay *chain_overlay(struct overlay *first, struct overlay *list);
+void apply_overlay(struct node *base, struct overlay *overlay);
+
const char *get_unitname(struct node *node);
struct property *get_property(struct node *node, const char *propname);
cell_t propval_cell(struct property *prop);
@@ -245,7 +255,9 @@ struct dt_info {
unsigned int dtsflags;
struct reserve_info *reservelist;
uint32_t boot_cpuid_phys;
- struct node *dt; /* the device tree */
+
+ struct node *dt;
+ struct overlay *overlays;
};
/* DTS version flags definitions */
@@ -254,7 +266,9 @@ struct dt_info {
struct dt_info *build_dt_info(unsigned int dtsflags,
struct reserve_info *reservelist,
- struct node *tree, uint32_t boot_cpuid_phys);
+ struct node *basetree,
+ struct overlay *overlays,
+ uint32_t boot_cpuid_phys);
void sort_tree(struct dt_info *dti);
void generate_label_tree(struct dt_info *dti, char *name, bool allocph);
void generate_fixups_tree(struct dt_info *dti, char *name);
diff --git a/flattree.c b/flattree.c
index ebac548b3fa8..13e4a4a46b6b 100644
--- a/flattree.c
+++ b/flattree.c
@@ -942,5 +942,5 @@ struct dt_info *dt_from_blob(const char *fname)
fclose(f);
- return build_dt_info(DTSF_V1, reservelist, tree, boot_cpuid_phys);
+ return build_dt_info(DTSF_V1, reservelist, tree, NULL, boot_cpuid_phys);
}
diff --git a/fstree.c b/fstree.c
index ae7d06c3c492..08ee93d26157 100644
--- a/fstree.c
+++ b/fstree.c
@@ -86,5 +86,5 @@ struct dt_info *dt_from_fs(const char *dirname)
tree = read_fstree(dirname);
tree = name_node(tree, "");
- return build_dt_info(DTSF_V1, NULL, tree, guess_boot_cpuid(tree));
+ return build_dt_info(DTSF_V1, NULL, tree, NULL, guess_boot_cpuid(tree));
}
diff --git a/livetree.c b/livetree.c
index afa2f67b142a..dd51223d1e61 100644
--- a/livetree.c
+++ b/livetree.c
@@ -313,6 +313,39 @@ void append_to_property(struct node *node,
}
}
+struct overlay *build_overlay(char *target, struct node *dt)
+{
+ struct overlay *new = xmalloc(sizeof(*new));
+
+ new->target = target;
+ new->dt = dt;
+ new->next = NULL;
+ return new;
+}
+
+struct overlay *chain_overlay(struct overlay *first, struct overlay *list)
+{
+ assert(first->next == NULL);
+
+ first->next = list;
+ return first;
+}
+
+void apply_overlay(struct node *base, struct overlay *overlay)
+{
+ struct node *target = get_node_by_ref(base, overlay->target);
+
+ if (!target)
+ die("Couldn't find label or path %s for overlay\n",
+ overlay->target);
+
+ if (!overlay->dt)
+ /* Deletion */
+ delete_node(target);
+ else
+ merge_nodes(target, overlay->dt);
+}
+
struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
{
struct reserve_info *new = xmalloc(sizeof(*new));
@@ -354,15 +387,18 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
struct dt_info *build_dt_info(unsigned int dtsflags,
struct reserve_info *reservelist,
- struct node *tree, uint32_t boot_cpuid_phys)
+ struct node *basetree,
+ struct overlay *overlays,
+ uint32_t boot_cpuid_phys)
{
struct dt_info *dti;
dti = xmalloc(sizeof(*dti));
dti->dtsflags = dtsflags;
dti->reservelist = reservelist;
- dti->dt = tree;
dti->boot_cpuid_phys = boot_cpuid_phys;
+ dti->dt = basetree;
+ dti->overlays = overlays;
return dti;
}
--
2.10.0.297.gf6727b0
^ permalink raw reply related
* [RFC/PATCH 2/4] dtc: Add syntactic sugar version of overlays
From: Stephen Boyd @ 2017-01-31 0:02 UTC (permalink / raw)
To: David Gibson
Cc: devicetree-compiler, Frank Rowand, Pantelis Antoniou, Rob Herring,
Mark Brown, Grant Likely, Mark Rutland, mporter, Koen Kooi,
Guenter Roeck, marex, Wolfram Sang, devicetree, linux-kernel,
linux-i2c
In-Reply-To: <20170131000259.28576-1-stephen.boyd@linaro.org>
Instead of writing overlays with the target property and
__overlay__ node, i.e.
fragment@0 {
target = <&foo>;
__overlay__ {
bar-property = <0x200>;
};
}
support the syntactic sugar version of overlays where this can
be written as
&foo {
bar-property = <0x200>;
};
and have dtc convert that into a fragment node.
Based on a patch by Pantelis Antoniou.
Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
dtc.c | 2 +-
dtc.h | 3 ++-
livetree.c | 40 ++++++++++++++++++++++++++++++++++++----
3 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/dtc.c b/dtc.c
index 91e4c18b891e..d52d6c77a8cd 100644
--- a/dtc.c
+++ b/dtc.c
@@ -63,7 +63,7 @@ static void resolve_overlays(struct dt_info *dti)
struct overlay *o = dti->overlays;
while (o) {
- apply_overlay(dti->dt, o);
+ apply_overlay(dti->dt, o, dti->dtsflags);
o = o->next;
}
}
diff --git a/dtc.h b/dtc.h
index c4eb6421eabf..be75aac1f185 100644
--- a/dtc.h
+++ b/dtc.h
@@ -216,7 +216,8 @@ void append_to_property(struct node *node,
struct overlay *build_overlay(char *target, struct node *dt);
struct overlay *chain_overlay(struct overlay *first, struct overlay *list);
-void apply_overlay(struct node *base, struct overlay *overlay);
+void apply_overlay(struct node *base, struct overlay *overlay,
+ unsigned int dtsflags);
const char *get_unitname(struct node *node);
struct property *get_property(struct node *node, const char *propname);
diff --git a/livetree.c b/livetree.c
index dd51223d1e61..798a87ed587e 100644
--- a/livetree.c
+++ b/livetree.c
@@ -331,14 +331,46 @@ struct overlay *chain_overlay(struct overlay *first, struct overlay *list)
return first;
}
-void apply_overlay(struct node *base, struct overlay *overlay)
+static void add_fragment(struct node *base, struct overlay *overlay)
{
- struct node *target = get_node_by_ref(base, overlay->target);
+ static unsigned int next_fragment = 0;
+ struct node *node;
+ struct property *p;
+ struct data d = empty_data;
+ char *name;
- if (!target)
- die("Couldn't find label or path %s for overlay\n",
+ if (!overlay->dt)
+ die("Deletions not supported at runtime for %s\n",
overlay->target);
+ /* Insert a target = <&phandle> property */
+ d = data_add_marker(d, REF_PHANDLE, overlay->target);
+ d = data_append_integer(d, 0xffffffff, 32);
+
+ p = build_property("target", d);
+
+ xasprintf(&name, "fragment@%u", next_fragment++);
+ name_node(overlay->dt, "__overlay__");
+ node = build_node(p, overlay->dt);
+ name_node(node, name);
+
+ add_child(base, node);
+}
+
+void apply_overlay(struct node *base, struct overlay *overlay,
+ unsigned int dtsflags)
+{
+ struct node *target = get_node_by_ref(base, overlay->target);
+
+ if (!target) {
+ if (dtsflags & DTSF_PLUGIN) {
+ add_fragment(base, overlay);
+ return;
+ } else
+ die("Couldn't find label or path %s for overlay\n",
+ overlay->target);
+ }
+
if (!overlay->dt)
/* Deletion */
delete_node(target);
--
2.10.0.297.gf6727b0
^ permalink raw reply related
* [RFC/PATCH 3/4] tests: Add a test for overlays syntactic sugar
From: Stephen Boyd @ 2017-01-31 0:02 UTC (permalink / raw)
To: David Gibson
Cc: Pantelis Antoniou, devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
Frank Rowand, Rob Herring, Mark Brown, Grant Likely, Mark Rutland,
mporter-OWPKS81ov/FWk0Htik3J/w, Koen Kooi, Guenter Roeck,
marex-ynQEQJNshbs, Wolfram Sang,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170131000259.28576-1-stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
Add a single test making sure the &foo { }; syntax works.
Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
[stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org: Update test to add required root node]
Signed-off-by: Stephen Boyd <stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
tests/overlay_overlay_simple.dts | 3 +++
tests/run_tests.sh | 6 ++++++
2 files changed, 9 insertions(+)
diff --git a/tests/overlay_overlay_simple.dts b/tests/overlay_overlay_simple.dts
index 8657e1e7a15a..ab19cc723210 100644
--- a/tests/overlay_overlay_simple.dts
+++ b/tests/overlay_overlay_simple.dts
@@ -7,6 +7,9 @@
/dts-v1/;
/plugin/;
+/ {
+};
+
&test {
test-int-property = <43>;
};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index ed489dbdd269..e2fb084b4434 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -231,6 +231,12 @@ dtc_overlay_tests () {
run_test check_path overlay_base_with_aliases.dtb not-exists "/__symbols__"
run_test check_path overlay_base_with_aliases.dtb not-exists "/__fixups__"
run_test check_path overlay_base_with_aliases.dtb not-exists "/__local_fixups__"
+
+ # Test simplified plugin syntax
+ run_dtc_test -@ -I dts -O dtb -o overlay_overlay_simple.dtb overlay_overlay_simple.dts
+
+ # Verify non-generation of local fixups
+ run_test check_path overlay_overlay_simple.dtb not-exists "/__local_fixups__"
}
tree1_tests () {
--
2.10.0.297.gf6727b0
--
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
* [RFC/PATCH 0/4] New style overlay and /expansion/ support
From: Stephen Boyd @ 2017-01-31 0:02 UTC (permalink / raw)
To: David Gibson
Cc: devicetree-compiler, Frank Rowand, Pantelis Antoniou, Rob Herring,
Mark Brown, Grant Likely, Mark Rutland, mporter, Koen Kooi,
Guenter Roeck, marex, Wolfram Sang, devicetree, linux-kernel,
linux-i2c
This series revives the new style of overlay syntax that never got
merged. David said he wanted to handle it outside of the parse
phase[1], and he proposed a patch to move overlay application to the
livetree phase. I've applied that patch and reworked Pantelis' last
two patches for the new style "syntactic sugar" on top[2].
In addition, I've included the new /expansion/ keyword support for
overlays that don't reference any labels that are outside of the
input file. The new expansion keyword just transforms the unresolved
references into strings that go into a new 'target-alias' property
instead of the 'target' property. This is in line with what David
has proposed on the mailing list a few months ago[3].
RFC part: The current syntax requires a root node or overlays are rejected.
Making the root node optional would require some non-trivial rework of the
grammar to support it. This means the proposed 'new style' syntax doesn't
work without the root node.
[1] https://lkml.kernel.org/r/20161125041124.GB12287@umbus.fritz.box
[2] https://lkml.kernel.org/r/1481114903-8197-7-git-send-email-pantelis.antoniou@konsulko.com
[3] https://lkml.kernel.org/r/20160718142037.GS16769@voom.fritz.box
David Gibson (1):
Start moving overlay handling from parser into dtc core
Pantelis Antoniou (1):
tests: Add a test for overlays syntactic sugar
Stephen Boyd (2):
dtc: Add syntactic sugar version of overlays
dtc: Add /expansion/ support
checks.c | 2 +-
dtc-lexer.l | 6 +++
dtc-parser.y | 68 +++++++++++++++++-----------------
dtc.c | 16 +++++++-
dtc.h | 20 +++++++++-
flattree.c | 2 +-
fstree.c | 2 +-
livetree.c | 80 +++++++++++++++++++++++++++++++++++++++-
tests/overlay_overlay_simple.dts | 3 ++
tests/run_tests.sh | 6 +++
10 files changed, 163 insertions(+), 42 deletions(-)
--
2.10.0.297.gf6727b0
^ permalink raw reply
* [RFC/PATCH 4/4] dtc: Add /expansion/ support
From: Stephen Boyd @ 2017-01-31 0:02 UTC (permalink / raw)
To: David Gibson
Cc: devicetree-compiler, Frank Rowand, Pantelis Antoniou, Rob Herring,
Mark Brown, Grant Likely, Mark Rutland, mporter, Koen Kooi,
Guenter Roeck, marex, Wolfram Sang, devicetree, linux-kernel,
linux-i2c
In-Reply-To: <20170131000259.28576-1-stephen.boyd@linaro.org>
There's typically a standard pinout for expansion boards on
platforms like raspberry pi, 96boards, C.H.I.P, etc. Introduce a
new syntax to describe these types of expansion boards in
devicetree.
In general, the resulting dtb format is very similar to the
/plugin/ style of overlays, except the 'target' property in the
fragment node is replaced with a 'target-alias' property that
contains a string instead of a phandle.
fragment@0 {
target-alias = "i2c_1";
};
instead of
fragment@0 {
target = <&i2c_1>;
};
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
checks.c | 2 +-
dtc-lexer.l | 6 ++++++
dtc-parser.y | 5 +++++
dtc.c | 3 +--
dtc.h | 1 +
livetree.c | 22 +++++++++++++++-------
6 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/checks.c b/checks.c
index 3d18e45374c8..44101def69d5 100644
--- a/checks.c
+++ b/checks.c
@@ -487,7 +487,7 @@ static void fixup_phandle_references(struct check *c, struct dt_info *dti,
refnode = get_node_by_ref(dt, m->ref);
if (! refnode) {
- if (!(dti->dtsflags & DTSF_PLUGIN))
+ if (!(dti->dtsflags & (DTSF_PLUGIN | DTSF_EXPANSION)))
FAIL(c, "Reference to non-existent node or "
"label \"%s\"\n", m->ref);
else /* mark the entry as unresolved */
diff --git a/dtc-lexer.l b/dtc-lexer.l
index c600603044f3..c5683e0197ec 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -126,6 +126,12 @@ static void lexical_error(const char *fmt, ...);
return DT_PLUGIN;
}
+<*>"/expansion/" {
+ DPRINT("Keyword: /expansion/\n");
+ return DT_EXPANSION;
+ }
+
+
<*>"/memreserve/" {
DPRINT("Keyword: /memreserve/\n");
BEGIN_DEFAULT();
diff --git a/dtc-parser.y b/dtc-parser.y
index 3d2ce372c286..28de8f6c1b61 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -60,6 +60,7 @@ extern bool treesource_error;
%token DT_V1
%token DT_PLUGIN
+%token DT_EXPANSION
%token DT_MEMRESERVE
%token DT_LSHIFT DT_RSHIFT DT_LE DT_GE DT_EQ DT_NE DT_AND DT_OR
%token DT_BITS
@@ -127,6 +128,10 @@ header:
{
$$ = DTSF_V1 | DTSF_PLUGIN;
}
+ | DT_V1 ';' DT_EXPANSION ';'
+ {
+ $$ = DTSF_V1 | DTSF_EXPANSION;
+ }
;
headers:
diff --git a/dtc.c b/dtc.c
index d52d6c77a8cd..8f0b47b53d8c 100644
--- a/dtc.c
+++ b/dtc.c
@@ -334,9 +334,8 @@ int main(int argc, char *argv[])
process_checks(force, dti);
/* on a plugin, generate by default */
- if (dti->dtsflags & DTSF_PLUGIN) {
+ if (dti->dtsflags & (DTSF_PLUGIN | DTSF_EXPANSION))
generate_fixups = 1;
- }
if (auto_label_aliases)
generate_label_tree(dti, "aliases", false);
diff --git a/dtc.h b/dtc.h
index be75aac1f185..f751757d63f6 100644
--- a/dtc.h
+++ b/dtc.h
@@ -264,6 +264,7 @@ struct dt_info {
/* DTS version flags definitions */
#define DTSF_V1 0x0001 /* /dts-v1/ */
#define DTSF_PLUGIN 0x0002 /* /plugin/ */
+#define DTSF_EXPANSION 0x0004 /* /expansion/ */
struct dt_info *build_dt_info(unsigned int dtsflags,
struct reserve_info *reservelist,
diff --git a/livetree.c b/livetree.c
index 798a87ed587e..f8abb875be8e 100644
--- a/livetree.c
+++ b/livetree.c
@@ -331,7 +331,8 @@ struct overlay *chain_overlay(struct overlay *first, struct overlay *list)
return first;
}
-static void add_fragment(struct node *base, struct overlay *overlay)
+static void add_fragment(struct node *base, struct overlay *overlay,
+ bool expansion)
{
static unsigned int next_fragment = 0;
struct node *node;
@@ -343,11 +344,18 @@ static void add_fragment(struct node *base, struct overlay *overlay)
die("Deletions not supported at runtime for %s\n",
overlay->target);
- /* Insert a target = <&phandle> property */
- d = data_add_marker(d, REF_PHANDLE, overlay->target);
- d = data_append_integer(d, 0xffffffff, 32);
+ if (expansion) {
+ /* Insert a target-alias = "<target>" property */
+ p = build_property("target-alias",
+ data_copy_mem(overlay->target,
+ strlen(overlay->target) + 1));
+ } else {
+ /* Insert a target = <&phandle> property */
+ d = data_add_marker(d, REF_PHANDLE, overlay->target);
+ d = data_append_integer(d, 0xffffffff, 32);
- p = build_property("target", d);
+ p = build_property("target", d);
+ }
xasprintf(&name, "fragment@%u", next_fragment++);
name_node(overlay->dt, "__overlay__");
@@ -363,8 +371,8 @@ void apply_overlay(struct node *base, struct overlay *overlay,
struct node *target = get_node_by_ref(base, overlay->target);
if (!target) {
- if (dtsflags & DTSF_PLUGIN) {
- add_fragment(base, overlay);
+ if (dtsflags & (DTSF_PLUGIN | DTSF_EXPANSION)) {
+ add_fragment(base, overlay, dtsflags & DTSF_EXPANSION);
return;
} else
die("Couldn't find label or path %s for overlay\n",
--
2.10.0.297.gf6727b0
^ permalink raw reply related
* Re: [PATCH v8 07/12] dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings
From: Peter Rosin @ 2017-01-31 7:36 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Wolfram Sang, Mark Rutland, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
Andrew Morton, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqLVrzm4BEv3kpOGRoaRq+cqvSm+A6weZfU8H8i9dc=tcA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 2017-01-30 18:20, Rob Herring wrote:
> On Sat, Jan 28, 2017 at 4:42 PM, Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org> wrote:
>> On 2017-01-27 20:39, Rob Herring wrote:
>>> On Wed, Jan 18, 2017 at 04:57:10PM +0100, Peter Rosin wrote:
>>>> Describe how a generic multiplexer controller is used to mux an i2c bus.
>>>>
>>>> Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
>>>> ---
>>>> .../devicetree/bindings/i2c/i2c-mux-simple.txt | 81 ++++++++++++++++++++++
>>>> 1 file changed, 81 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
>>>> new file mode 100644
>>>> index 000000000000..253d5027843b
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
>>>> @@ -0,0 +1,81 @@
>>>> +Simple I2C Bus Mux
>>>> +
>>>> +This binding describes an I2C bus multiplexer that uses a mux controller
>>>> +from the mux subsystem to route the I2C signals.
>>>> +
>>>> + .-----. .-----.
>>>> + | dev | | dev |
>>>> + .------------. '-----' '-----'
>>>> + | SoC | | |
>>>> + | | .--------+--------'
>>>> + | .------. | .------+ child bus A, on MUX value set to 0
>>>> + | | I2C |-|--| Mux |
>>>> + | '------' | '--+---+ child bus B, on MUX value set to 1
>>>> + | .------. | | '----------+--------+--------.
>>>> + | | MUX- | | | | | |
>>>> + | | Ctrl |-|-----+ .-----. .-----. .-----.
>>>> + | '------' | | dev | | dev | | dev |
>>>> + '------------' '-----' '-----' '-----'
>>>> +
>>>> +Required properties:
>>>> +- compatible: i2c-mux-simple,mux-locked or i2c-mux-simple,parent-locked
>>>
>>> Not a fan of using "simple" nor the ','. Perhaps lock type should be
>>> separate property.
>>
>> How about just i2c-mux for the compatible? Because i2c-mux-mux (which
>> follows the naming of previous i2c muxes) looks really stupid. Or
>> perhaps i2c-mux-generic?
>
> I like "generic" only slightly more than "simple". :)
>
> If the mux is gpio controlled, then it should still be called
> i2c-gpio-mux. Let's not invent brand new bindings when current ones
> are easily extended. We already have pretty generic names here, let's
> not make them more generic.
Ahh, but the whole point of this new driver is that it does not know
if the mux is gpio controlled, if it is controlled by an adg792 chip
or whatever might appear down the line. So, I think i2c-mux-gpio (and
i2c-gpio-mux) is actively wrong.
This new driver is in fact an i2c-mux driver that (in this aspect) is more
generic than any of the existing i2c-mux drivers, hence my suggestion.
If you see this new driver as something that is superseding the existing
i2c-mux-gpio driver, I'm sad to inform you that the code is not simply
not there. i2c-mux-gpio has acpi support and users may provide platform
data from code. The existing i2c-mux-gpio driver also has the below
mentioned locking heuristic. Adding all those things to the new driver
would make it big and unwieldy and having even more unwanted tentacles
into other subsystems. And why should it be only i2c-mux-gpio that is
merged into this new i2c-mux driver? Why not implement a mux-pinctrl
driver for the new mux subsustem and then merge i2c-mux-pinctrl as well?
I say no, that way lies madness.
>>
>> I'm also happy to have the lock type as a separate property. One lock
>> type, e.g. parent-locked, could be the default and adding a 'mux-locked'
>> property could change that. Would that be ok?
>
> I prefer this. Then existing bindings can use it.
>
>> Or should it be a requirement that one of 'mux-locked'/'parent-locked'
>> is always present?
>
> I would make it boolean and make not present be the more common case.
> Not present could also mean determined via other means as you have
> today with existing bindings. Maybe then you need both properties.
Ok. Lets define that every type of i2c-mux have a default locking.
If no property is found that overrides this default, a heuristic may
be applied that is only allowed to very defensively use non-default
locking (like the current heuristics in i2c-mux-gpio/i2c-mux-pinctrl
attempt to do, they only adjust the locking for impossible cases that
would invariably lead to a deadlock with the default locking). Then,
there should never exist a need to enforce the default locking with
a property.
And given that no current i2c-mux that is mux-locked by default even
have the ability to be parent-locked, there is (currently) no need
for more than a bool 'mux-locked' property.
>>> I'm not sure I get the mux vs. parent locked fully. How do I determine
>>> what type I have? We already have bindings for several types of i2c
>>> muxes. How does the locking annotation fit into those?
>>
>> We have briefly discussed this before [1] in the context of i2c-mux-gpio
>> and i2c-mux-pinctrl, when I added the mux-locked/parent-locked distinction
>> to the i2c-mux infrastructure (it wasn't named mux-locked/parent-locked
>> way back then though). There is more detail on what the difference is
>> between the two in Documentation/i2c/i2c-topology.
>>
>> Side note regarding your remark "use an I2C controlled mux instead"; it
>> appears that I'm not alone [2] with this kind of requirement...
>>
>> [1] https://lkml.org/lkml/2016/1/6/437
>> [2] http://marc.info/?t=147877959100002&r=1&w=2
>>
>> But, now that I have pondered on this for a year or so, I firmly
>> believe it was a mistake to have the code in i2c-mux-gpio and
>> i2c-mux-pinctrl automatically try to deduce if the mux should be
>> mux-locked or parent-locked. It might be easy to make that call
>> in some trivial cases, but it is not difficult to dream up
>> scenarios where it would be extremely hard for the code to get
>> this decision right. It's just fragile. But now we have code in
>> those two muxes that has unwanted tentacles into the guts of the
>> gpio and pinctrl subsystems. Hopefully those unwanted tentacles
>> can be replaced with something based on device links? However, it
>> is still not hard to come up with scenarios that will require
>> manual intervention in order to select the right kind of i2c mux
>> locking. So, I fear that we have inadequate code trying to make a
>> decision automatically, and that we at some point down the line
>> will have some impossible case needing a binding that trumps the
>> heuristic. Why have a heuristic at all in that case? In short, it
>> should have been a binding from the start, methinks.
>>
>> That was a long rant regarding i2c-mux-gpio and i2c-mux-pinctrl.
>> I obviously think it is bad to have this new i2c mux go in the
>> same direction as those two drivers. I.e. I think a binding that
>> specifies the desired locking is preferable to any attempted
>> heuristic.
>
> Having a separate, common property(ies) for these would solve this then.
Yep, the plan is now to make this new i2c-mux parent-locked by default
and then look for an optional mux-locked property, thus eliminating the
comma and the suffix in the compatible string, leaving only whatever is
eventually picked from 'i2c-mux-simple', 'i2c-mux', 'i2c-mux-generic' and
'i2c-mux-whatever'.
Do I need to re-document the mux-locked/parent-locked difference? I.e.,
is it enough to refer to Documentation/i2c/i2c-topology, or does the
Documentation/devicetree subdir need to self-contained?
Cheers,
peda
--
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: Issue with i2c-designware-platdrv's suspend/runtime-suspend handling
From: Jarkko Nikula @ 2017-01-31 9:32 UTC (permalink / raw)
To: John Stultz
Cc: Andy Shevchenko, Mika Westerberg, Wolfram Sang, linux-i2c, lkml,
zhangfei, Guodong Xu, Amit Pundir
In-Reply-To: <CALAqxLVzW6Di5GM6iXi7PUen2Dq0TZ=awXYXY74BUK02Nfwm0Q@mail.gmail.com>
On 01/31/2017 12:05 AM, John Stultz wrote:
> On Tue, Jan 24, 2017 at 2:03 PM, John Stultz <john.stultz@linaro.org> wrote:
>> Doing some further debugging, it seems the problem is that the device
>> is being runtime suspended, and then at suspend time, we're calling
>> the same logic, calling i2c_dw_plat_prepare_clk, which causes the clk
>> count warning.
>>
>> Removing the runtime pm ops:
>> - SET_RUNTIME_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume, NULL)
>> +// SET_RUNTIME_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume, NULL)
>>
>> seems to avoid the warning, but clearly isn't ideal. :)
>>
>> Should there be some logic keep track of the suspend state for the
>> dw_i2c_dev device so we don't try to suspend (or resume) it twice? Or
>> is there something else I'm missing to keep this from happening?
>
> Ping? Any thoughts on how best to fix this? I'm leaning towards
> adding a suspended state to the struct dw_i2c_dev. Any objections?
>
I wonder why device PM doesn't take care of this?
If i2c-designware is enumerated from ACPI then drivers/acpi/device_pm.c:
acpi_subsys_suspend() resumes devices suspended at run time before
system suspend so dw_i2c_plat_suspend() won't be called twice.
But still I think device PM should not attempt to suspend already
suspended device?
--
Jarkko
^ permalink raw reply
* [PATCH] i2c: i801: Add support for Intel Gemini Lake
From: Mika Westerberg @ 2017-01-31 10:17 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang; +Cc: Jarkko Nikula, Mika Westerberg, linux-i2c
Intel Gemini Lake has the same SMBus host controller than Intel Broxton.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
drivers/i2c/busses/i2c-i801.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index e242db43774b..6484fa6dbb84 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -65,6 +65,7 @@
* Lewisburg (PCH) 0xa1a3 32 hard yes yes yes
* Lewisburg Supersku (PCH) 0xa223 32 hard yes yes yes
* Kaby Lake PCH-H (PCH) 0xa2a3 32 hard yes yes yes
+ * Gemini Lake (SOC) 0x31d4 32 hard yes yes yes
*
* Features supported by this driver:
* Software PEC no
@@ -213,6 +214,7 @@
#define PCI_DEVICE_ID_INTEL_BRASWELL_SMBUS 0x2292
#define PCI_DEVICE_ID_INTEL_DH89XXCC_SMBUS 0x2330
#define PCI_DEVICE_ID_INTEL_COLETOCREEK_SMBUS 0x23b0
+#define PCI_DEVICE_ID_INTEL_GEMINILAKE_SMBUS 0x31d4
#define PCI_DEVICE_ID_INTEL_5_3400_SERIES_SMBUS 0x3b30
#define PCI_DEVICE_ID_INTEL_BROXTON_SMBUS 0x5ad4
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_SMBUS 0x8c22
@@ -1012,6 +1014,7 @@ static const struct pci_device_id i801_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WELLSBURG_SMBUS_MS1) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WELLSBURG_SMBUS_MS2) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_COLETOCREEK_SMBUS) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_GEMINILAKE_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WILDCATPOINT_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BAYTRAIL_SMBUS) },
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] i2c: piix4: Fix request_region size
From: Jean Delvare @ 2017-01-31 13:42 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Andy Shevchenko, Wolfram Sang, linux-i2c, linux-kernel
In-Reply-To: <20170127145930.13055-1-ricardo.ribalda@gmail.com>
Hi Ricardo, Wolfram,
On Fri, 27 Jan 2017 15:59:30 +0100, Ricardo Ribalda Delgado wrote:
> Since '701dc207bf55 ("i2c: piix4: Avoid race conditions with IMC")' we
> are using the SMBSLVCNT register at offset 0x8. We need to request it.
>
> Fixes: 701dc207bf55 ("i2c: piix4: Avoid race conditions with IMC")
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> Please note, not tested on hardware.
> drivers/i2c/busses/i2c-piix4.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> index e34d82e79b98..73cc6799cc59 100644
> --- a/drivers/i2c/busses/i2c-piix4.c
> +++ b/drivers/i2c/busses/i2c-piix4.c
> @@ -58,7 +58,7 @@
> #define SMBSLVDAT (0xC + piix4_smba)
>
> /* count for request_region */
> -#define SMBIOSIZE 8
> +#define SMBIOSIZE 9
>
> /* PCI Address Constants */
> #define SMBBA 0x090
I checked all the datasheets I have access to, and it should be good.
Signed-off-by: Jean Delvare <jdelvare@suse.de>
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
* Re: [PATCH v3] i2c: piix4: Avoid race conditions with IMC
From: Jean Delvare @ 2017-01-31 13:55 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Wolfram Sang, Andy Shevchenko, linux-i2c, linux-kernel
In-Reply-To: <20170112194946.13925-1-ricardo.ribalda@gmail.com>
Hi all,
On Thu, 12 Jan 2017 20:49:46 +0100, Ricardo Ribalda Delgado wrote:
> From: Ricardo Ribalda <ricardo.ribalda@gmail.com>
>
> On AMD's SB800 and upwards, the SMBus is shared with the Integrated
> Micro Controller (IMC).
>
> The platform provides a hardware semaphore to avoid race conditions
> among them. (Check page 288 of the SB800-Series Southbridges Register
> Reference Guide http://support.amd.com/TechDocs/45482.pdf)
>
> Without this patch, many access to the SMBus end with an invalid
> transaction or even with the bus stalled.
>
> Reported-by: Alexandre Desnoyers <alex@qtec.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
> v3: Suggestions by Andy Shevchenko <andy.shevchenko@gmail.com> and
> Wolfram Sang <wsa@the-dreams.de>:
> -Use Reported-by instead of Credit-to
>
> v2: Suggestions by Andy Shevchenko <andy.shevchenko@gmail.com>:
> -Rename timeout to retries
> -Use do {} while(--retries) pattern
> -Group new variables
>
> drivers/i2c/busses/i2c-piix4.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> index c2268cdf38e8..e34d82e79b98 100644
> --- a/drivers/i2c/busses/i2c-piix4.c
> +++ b/drivers/i2c/busses/i2c-piix4.c
> @@ -585,10 +585,29 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
> u8 command, int size, union i2c_smbus_data *data)
> {
> struct i2c_piix4_adapdata *adapdata = i2c_get_adapdata(adap);
> + unsigned short piix4_smba = adapdata->smba;
> + int retries = MAX_TIMEOUT;
> + int smbslvcnt;
> u8 smba_en_lo;
> u8 port;
> int retval;
>
> + /* Request the SMBUS semaphore, avoid conflicts with the IMC */
> + smbslvcnt = inb_p(SMBSLVCNT);
> + do {
> + outb_p(smbslvcnt | 0x10, SMBSLVCNT);
> +
> + /* Check the semaphore status */
> + smbslvcnt = inb_p(SMBSLVCNT);
> + if (smbslvcnt & 0x10)
> + break;
> +
> + usleep_range(1000, 2000);
> + } while (--retries);
> + /* SMBus is still owned by the IMC, we give up */
> + if (!retries)
> + return -EBUSY;
> +
> mutex_lock(&piix4_mutex_sb800);
Sorry for being late to the party but... Shouldn't the IMC semaphore
check be done with piix4_mutex_sb800 held? If we do it before taking
piix4_mutex_sb800, as done above, several SMBus users could try to
write to and read from SMBSLVCNT at the same time. That doesn't look
safe to me. I can imagine that one (starting) SMBus transaction would
try to gain the IMC semaphore above...
>
> outb_p(piix4_port_sel_sb800, SB800_PIIX4_SMB_IDX);
> @@ -606,6 +625,9 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
>
> mutex_unlock(&piix4_mutex_sb800);
>
> + /* Release the semaphore */
> + outb_p(smbslvcnt | 0x20, SMBSLVCNT);
> +
... while another (finishing) SMBus transaction is releasing it. At
which point we may end up using the SMBus while we do not own the IMC
semaphore.
And things could get even worse if SMBSLVCNT is ever used somewhere
else in the driver.
> return retval;
> }
>
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
* Re: [PATCH] i2c: busses: constify i2c_algorithm structures
From: Jean Delvare @ 2017-01-31 14:22 UTC (permalink / raw)
To: Bhumika Goyal
Cc: julia.lawall, ludovic.desroches, wsa, sonic.zhang, jarkko.nikula,
andriy.shevchenko, mika.westerberg, patrice.chotard, linux-i2c,
linux-kernel, adi-buildroot-devel, linux-arm-kernel, kernel
In-Reply-To: <1485540377-13686-1-git-send-email-bhumirks@gmail.com>
On Fri, 27 Jan 2017 23:36:17 +0530, Bhumika Goyal wrote:
> Declare i2c_algorithm structures as const as they are only stored in the
> algo field of an i2c_adapter structure. This field is of type const, so
> i2c_algorithm structures having this property can be made const too.
> (...)
I have vague memories that I tried to do the same long ago and it
didn't work back then. But I can't no longer remember the details, and
today it seems just fine to do it.
Reviewed-by: Jean Delvare <jdelvare@suse.de>
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
* Re: [PATCH v4 00/14] ARM: dts: r8a779x: use demuxer for I2C
From: Niklas Söderlund @ 2017-01-31 15:42 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Simon Horman, Wolfram Sang, linux-renesas-soc, linux-i2c
In-Reply-To: <20170109093420.GG29854@bigcity.dyn.berto.se>
Hi Wolfram,
On 2017-01-09 10:34:20 +0100, Niklas Söderlund wrote:
> On 2017-01-04 11:25:50 +0100, Wolfram Sang wrote:
> > Bump...
> >
> > > Yes, this is a bug in the rcar-vin driver which is addressed in the Gen3
> > > patches. However I'm not sure those patches will make it to v4.10, not
> > > much review from the V4L2 side yet (Geert and Sergei have had a few
> > > comments so there will at lest be one more iteration).
> >
> > Seems your predictions were correct. I still get:
> >
> > * OOPS with renesas/dt-for-v4.11 and these patches on top
> > * no OOPS with renesas-drivers/master and these patches on top
> >
> > So, it seems to me that we still should wait with the HDMI related
> > patches of this series until the rcar-vin-gen3 branch is in -next.
> > D'accord everyone?
>
> I'm fine with waiting. But as I hinted in my previous email the review
> on the rcar-vin-gen3 branch is slow, that is nothing have happened in
> that thread since last we spoke of this...
>
> If this is blocking or other create other issue for you please let me
> know and I will do my best to extract the fix and post it separately
> from Gen3 enablement.
I have now posted a broken out series for rcar-vin which solves this
issue. Let's hope we can resolve this blocker with that series.
--
Regards,
Niklas Söderlund
^ permalink raw reply
* Re: [PATCH v3 3/3] spi: acpi: Initialize modalias from of_compatible
From: Mark Brown @ 2017-01-31 20:17 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Dan O'Donovan, ACPI Devel Maling List, Rafael J . Wysocki,
Jarkko Nikula, Mika Westerberg, Len Brown, linux-i2c,
Wolfram Sang, linux-spi, Linux Kernel Mailing List,
Crestez Dan Leonard
In-Reply-To: <CAJZ5v0hAoOQN+d4iBSgSkkYHBOy-Pt9HLrKG4SjcQUB6Ya1bgA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 426 bytes --]
On Mon, Jan 30, 2017 at 10:41:33AM +0100, Rafael J. Wysocki wrote:
> On Mon, Jan 23, 2017 at 5:08 PM, Dan O'Donovan <dan@emutex.com> wrote:
> Mark, any objections here?
I've not looked yet, please allow a reasonable time for review (there
still seemed to be ongoing discussion at the ACPI level). I continue to
be concerned about the amount of duplication and special casing that the
various firmware paths seem to entail.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/3] spi: acpi: Initialize modalias from of_compatible
From: Rafael J. Wysocki @ 2017-01-31 21:18 UTC (permalink / raw)
To: Rafael J. Wysocki, Dan O'Donovan, ACPI Devel Maling List,
Rafael J . Wysocki, Jarkko Nikula, Mika Westerberg, Len Brown,
linux-i2c, Wolfram Sang, linux-spi, Linux Kernel Mailing List,
Crestez Dan Leonard
In-Reply-To: <20170131201736.cmiyq2obzrhgdsck-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
On Tue, Jan 31, 2017 at 9:17 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Jan 30, 2017 at 10:41:33AM +0100, Rafael J. Wysocki wrote:
>> On Mon, Jan 23, 2017 at 5:08 PM, Dan O'Donovan <dan-M3NBUjLqch7QT0dZR+AlfA@public.gmane.org> wrote:
>
>> Mark, any objections here?
>
> I've not looked yet, please allow a reasonable time for review
Sure, no problem at all.
> (there still seemed to be ongoing discussion at the ACPI level).
Honestly, I'm not sure what you mean.
The first patch in the series (which is the ACPI level here) does not
seem to be controversial at all and the i2c one has been acked
already.
> I continue to be concerned about the amount of duplication and special casing that the
> various firmware paths seem to entail.
Well, this particular change is relatively straightforward and one can
argue that it fixes a bug. It does address a problem for sure.
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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
* Bitte kontaktieren Sie mich für weitere Details!
From: Miss Marbell @ 2017-02-01 8:20 UTC (permalink / raw)
Sehr geehrte Damen und Herren,
Ich brauche Ihre Unterstützung in Ihrem Land zu verlagern und zu investieren.Ich bitte Sie um Hilfe, weil ich nicht das Wissen über
Geschäft und die Regeln, die Ihr Land für eine sichere Investition führen.
Werden Sie versprechen, mit mir aufrichtig zu sein?
Bitte kontaktieren Sie mich für weitere Details!
Mit freundlichen Grüßen,
Fräulein Marbell.
^ permalink raw reply
* Re: [PATCH] i2c: i801: Add support for Intel Gemini Lake
From: Jean Delvare @ 2017-02-01 10:38 UTC (permalink / raw)
To: Mika Westerberg; +Cc: Jean Delvare, Wolfram Sang, Jarkko Nikula, linux-i2c
In-Reply-To: <20170131101733.117705-1-mika.westerberg@linux.intel.com>
Hi Mika,
On Tue, 31 Jan 2017 13:17:33 +0300, Mika Westerberg wrote:
> Intel Gemini Lake has the same SMBus host controller than Intel Broxton.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
> drivers/i2c/busses/i2c-i801.c | 3 +++
> 1 file changed, 3 insertions(+)
Please also update drivers/i2c/busses/Kconfig and
Documentation/i2c/busses/i2c-i801.
Also please submit the new device to http://pci-ids.ucw.cz/read/PC/8086.
>
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index e242db43774b..6484fa6dbb84 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -65,6 +65,7 @@
> * Lewisburg (PCH) 0xa1a3 32 hard yes yes yes
> * Lewisburg Supersku (PCH) 0xa223 32 hard yes yes yes
> * Kaby Lake PCH-H (PCH) 0xa2a3 32 hard yes yes yes
> + * Gemini Lake (SOC) 0x31d4 32 hard yes yes yes
> *
> * Features supported by this driver:
> * Software PEC no
> @@ -213,6 +214,7 @@
> #define PCI_DEVICE_ID_INTEL_BRASWELL_SMBUS 0x2292
> #define PCI_DEVICE_ID_INTEL_DH89XXCC_SMBUS 0x2330
> #define PCI_DEVICE_ID_INTEL_COLETOCREEK_SMBUS 0x23b0
> +#define PCI_DEVICE_ID_INTEL_GEMINILAKE_SMBUS 0x31d4
> #define PCI_DEVICE_ID_INTEL_5_3400_SERIES_SMBUS 0x3b30
> #define PCI_DEVICE_ID_INTEL_BROXTON_SMBUS 0x5ad4
> #define PCI_DEVICE_ID_INTEL_LYNXPOINT_SMBUS 0x8c22
> @@ -1012,6 +1014,7 @@ static const struct pci_device_id i801_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WELLSBURG_SMBUS_MS1) },
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WELLSBURG_SMBUS_MS2) },
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_COLETOCREEK_SMBUS) },
> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_GEMINILAKE_SMBUS) },
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WILDCATPOINT_SMBUS) },
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP_SMBUS) },
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BAYTRAIL_SMBUS) },
Signed-off-by: Jean Delvare <jdelvare@suse.de>
Thanks,
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
* Re: [PATCH] i2c: i801: Add support for Intel Gemini Lake
From: Mika Westerberg @ 2017-02-01 10:50 UTC (permalink / raw)
To: Jean Delvare; +Cc: Jean Delvare, Wolfram Sang, Jarkko Nikula, linux-i2c
In-Reply-To: <20170201113832.647a1f3e@endymion>
On Wed, Feb 01, 2017 at 11:38:32AM +0100, Jean Delvare wrote:
> Hi Mika,
>
> On Tue, 31 Jan 2017 13:17:33 +0300, Mika Westerberg wrote:
> > Intel Gemini Lake has the same SMBus host controller than Intel Broxton.
> >
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> > drivers/i2c/busses/i2c-i801.c | 3 +++
> > 1 file changed, 3 insertions(+)
>
> Please also update drivers/i2c/busses/Kconfig and
> Documentation/i2c/busses/i2c-i801.
OK.
> Also please submit the new device to http://pci-ids.ucw.cz/read/PC/8086.
I will but it need to wait a bit since we don't yet have "marketing"
name for Gemini Lake. It is on my list so I'll do that immediately when
I know the right name.
> >
> > diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> > index e242db43774b..6484fa6dbb84 100644
> > --- a/drivers/i2c/busses/i2c-i801.c
> > +++ b/drivers/i2c/busses/i2c-i801.c
> > @@ -65,6 +65,7 @@
> > * Lewisburg (PCH) 0xa1a3 32 hard yes yes yes
> > * Lewisburg Supersku (PCH) 0xa223 32 hard yes yes yes
> > * Kaby Lake PCH-H (PCH) 0xa2a3 32 hard yes yes yes
> > + * Gemini Lake (SOC) 0x31d4 32 hard yes yes yes
> > *
> > * Features supported by this driver:
> > * Software PEC no
> > @@ -213,6 +214,7 @@
> > #define PCI_DEVICE_ID_INTEL_BRASWELL_SMBUS 0x2292
> > #define PCI_DEVICE_ID_INTEL_DH89XXCC_SMBUS 0x2330
> > #define PCI_DEVICE_ID_INTEL_COLETOCREEK_SMBUS 0x23b0
> > +#define PCI_DEVICE_ID_INTEL_GEMINILAKE_SMBUS 0x31d4
> > #define PCI_DEVICE_ID_INTEL_5_3400_SERIES_SMBUS 0x3b30
> > #define PCI_DEVICE_ID_INTEL_BROXTON_SMBUS 0x5ad4
> > #define PCI_DEVICE_ID_INTEL_LYNXPOINT_SMBUS 0x8c22
> > @@ -1012,6 +1014,7 @@ static const struct pci_device_id i801_ids[] = {
> > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WELLSBURG_SMBUS_MS1) },
> > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WELLSBURG_SMBUS_MS2) },
> > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_COLETOCREEK_SMBUS) },
> > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_GEMINILAKE_SMBUS) },
> > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WILDCATPOINT_SMBUS) },
> > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP_SMBUS) },
> > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BAYTRAIL_SMBUS) },
>
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
>
> Thanks,
> --
> Jean Delvare
> SUSE L3 Support
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: deprecate trivial-devices.txt for i2c
From: Rob Herring @ 2017-02-01 13:48 UTC (permalink / raw)
To: Wolfram Sang; +Cc: devicetree, linux-i2c
In-Reply-To: <20170126225544.5431-2-wsa@the-dreams.de>
On Thu, Jan 26, 2017 at 11:55:43PM +0100, Wolfram Sang wrote:
> The idea to collect trivial devices in a seperate file is coming from
> the early DT days and turned out to be a bad decision. It regularly
> creates maintenance pain and the information is not where users would
> expect it (i2c folder is for i2c bus drivers not clients). Also, we do
> not maintain such a file for other subsystems, so we should put an end
> to this as well. First step is to deprecate it and mention that it
> should be refactored into proper bindings.
>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> ---
> Documentation/devicetree/bindings/i2c/trivial-devices.txt | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
Actually, I prefer to keep it. Move it up a level and I will take
patches for it.
Rob
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: move trivial devices out of i2c subdir
From: Rob Herring @ 2017-02-01 13:50 UTC (permalink / raw)
To: Wolfram Sang; +Cc: devicetree, linux-i2c
In-Reply-To: <20170126225544.5431-3-wsa@the-dreams.de>
On Thu, Jan 26, 2017 at 11:55:44PM +0100, Wolfram Sang wrote:
> The i2c folder is for i2c bus drivers, not clients. Move the now
> deprecated trivial devices file to the bindings base directory, so it
> can be refactored from there to proper bindings in the relevant
> subdirectories. This will ease maintenance for the I2C subsystem (less
> irrelevant patches automatically CCed by get_maintainer.pl) and it will
> make aware that we deal with the file differently now.
>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> ---
> .../bindings/{i2c/trivial-devices.txt => i2c-trivial-devices.txt} | 0
> 1 file changed, 0 insertions(+), 0 deletions(-)
> rename Documentation/devicetree/bindings/{i2c/trivial-devices.txt => i2c-trivial-devices.txt} (100%)
Just keep the same name. We can use it for SPI too at least.
^ permalink raw reply
* [PATCH v2] i2c: i801: Add support for Intel Gemini Lake
From: Mika Westerberg @ 2017-02-01 16:20 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang; +Cc: Jarkko Nikula, Mika Westerberg, linux-i2c
Intel Gemini Lake has the same SMBus host controller than Intel Broxton.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Jean Delvare <jdelvare@suse.de>
---
Changes from the previous version:
* Updated documentation and Kconfig
* Added Jean's SoB
Documentation/i2c/busses/i2c-i801 | 1 +
drivers/i2c/busses/Kconfig | 1 +
drivers/i2c/busses/i2c-i801.c | 3 +++
3 files changed, 5 insertions(+)
diff --git a/Documentation/i2c/busses/i2c-i801 b/Documentation/i2c/busses/i2c-i801
index 1bba38dd2637..820d9040de16 100644
--- a/Documentation/i2c/busses/i2c-i801
+++ b/Documentation/i2c/busses/i2c-i801
@@ -33,6 +33,7 @@ Supported adapters:
* Intel DNV (SOC)
* Intel Broxton (SOC)
* Intel Lewisburg (PCH)
+ * Intel Gemini Lake (SOC)
Datasheets: Publicly available at the Intel website
On Intel Patsburg and later chipsets, both the normal host SMBus controller
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 0cdc8443deab..f2dea4e8edb2 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -128,6 +128,7 @@ config I2C_I801
DNV (SOC)
Broxton (SOC)
Lewisburg (PCH)
+ Gemini Lake (SOC)
This driver can also be built as a module. If so, the module
will be called i2c-i801.
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index e242db43774b..6484fa6dbb84 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -65,6 +65,7 @@
* Lewisburg (PCH) 0xa1a3 32 hard yes yes yes
* Lewisburg Supersku (PCH) 0xa223 32 hard yes yes yes
* Kaby Lake PCH-H (PCH) 0xa2a3 32 hard yes yes yes
+ * Gemini Lake (SOC) 0x31d4 32 hard yes yes yes
*
* Features supported by this driver:
* Software PEC no
@@ -213,6 +214,7 @@
#define PCI_DEVICE_ID_INTEL_BRASWELL_SMBUS 0x2292
#define PCI_DEVICE_ID_INTEL_DH89XXCC_SMBUS 0x2330
#define PCI_DEVICE_ID_INTEL_COLETOCREEK_SMBUS 0x23b0
+#define PCI_DEVICE_ID_INTEL_GEMINILAKE_SMBUS 0x31d4
#define PCI_DEVICE_ID_INTEL_5_3400_SERIES_SMBUS 0x3b30
#define PCI_DEVICE_ID_INTEL_BROXTON_SMBUS 0x5ad4
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_SMBUS 0x8c22
@@ -1012,6 +1014,7 @@ static const struct pci_device_id i801_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WELLSBURG_SMBUS_MS1) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WELLSBURG_SMBUS_MS2) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_COLETOCREEK_SMBUS) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_GEMINILAKE_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WILDCATPOINT_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BAYTRAIL_SMBUS) },
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v4 3/3] spi: acpi: Initialize modalias from of_compatible
From: Mark Brown @ 2017-02-01 18:34 UTC (permalink / raw)
To: Dan O'Donovan
Cc: linux-acpi, Andy Shevchenko, Rafael J . Wysocki, Jarkko Nikula,
Mika Westerberg, Len Brown, linux-i2c, Wolfram Sang, linux-spi,
linux-kernel, Crestez Dan Leonard
In-Reply-To: <1485523815-9629-4-git-send-email-dan@emutex.com>
[-- Attachment #1: Type: text/plain, Size: 684 bytes --]
On Fri, Jan 27, 2017 at 01:30:15PM +0000, Dan O'Donovan wrote:
> + if (acpi_of_modalias(adev, spi->modalias, sizeof(spi->modalias)))
> + strlcpy(spi->modalias, acpi_device_hid(adev),
> + sizeof(spi->modalias));
I can't help but think that this would be simpler to use if the
interface were something like acpi_set_modalias() and it did the
fallback to the ACPI HID internally. It'd both avoid a small amount of
code duplication and make the code read a bit more naturally - I have to
think to realise that this means "if there isn't an OF name use the
HID" since it looks like a boolean function. Then again just putting a
!= 0 in the test would have the same effect probably.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: move trivial devices out of i2c subdir
From: Wolfram Sang @ 2017-02-01 18:35 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170201135045.yass267eakmlpyru@rob-hp-laptop>
[-- Attachment #1: Type: text/plain, Size: 390 bytes --]
> > .../bindings/{i2c/trivial-devices.txt => i2c-trivial-devices.txt} | 0
> > 1 file changed, 0 insertions(+), 0 deletions(-)
> > rename Documentation/devicetree/bindings/{i2c/trivial-devices.txt => i2c-trivial-devices.txt} (100%)
>
> Just keep the same name. We can use it for SPI too at least.
OK, will resend with your requested changes.
Thanks!
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/3] ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node
From: Rafael J. Wysocki @ 2017-02-02 11:41 UTC (permalink / raw)
To: Dan O'Donovan
Cc: linux-acpi, Andy Shevchenko, Jarkko Nikula, Mika Westerberg,
Mark Brown, Len Brown, linux-i2c, Wolfram Sang, linux-spi,
linux-kernel
In-Reply-To: <1485523815-9629-2-git-send-email-dan@emutex.com>
On Friday, January 27, 2017 01:30:13 PM Dan O'Donovan wrote:
> From: Crestez Dan Leonard <leonard.crestez@intel.com>
>
> When using devicetree stuff like i2c_client.name or spi_device.modalias
> is initialized to the first DT compatible id with the vendor prefix
> stripped. Since some drivers rely on this try to replicate it when using
> ACPI with DT ids.
>
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
> Signed-off-by: Dan O'Donovan <dan@emutex.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
First off, the original author of the patch doesn't seem to be reachable under
the Intel address any more, so his S-o-b is meaningless and it shouldn't be
there. In such cases you can send the patch as From: you and give a credit
to the original author in the changelog.
Second, as Mark said ->
> ---
> drivers/acpi/bus.c | 35 +++++++++++++++++++++++++++++++++++
> include/acpi/acpi_bus.h | 1 +
> 2 files changed, 36 insertions(+)
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 95855cb..8b9657f 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -677,6 +677,41 @@ static bool acpi_of_match_device(struct acpi_device *adev,
> return false;
> }
>
> +/**
> + * acpi_of_modalias - Like of_modalias_node for ACPI with DT ids
> + * @adev: ACPI device object to match.
> + * @outstr: Pointer to buffer for result
> + * @outlen: Length of outstr value
> + *
> + * This is a counterpart of of_modalias_node() for struct acpi_device
> + * objects. If there is a compatible string for @adev, copy it to the
> + * @outstr location with the vendor prefix stripped.
> + *
> + * Returns 0 on success or negative errno on failure.
> + */
> +int acpi_of_modalias(struct acpi_device *adev, char *outstr, size_t outlen)
-> this could be a bool function and you could provide something like
acpi_set_modalias() doing the entire
if (!acpi_of_modalias(adev, outstr, outlen))
strlcpy(outstr, dev_name(&adev->dev), outlen);
thing, so that its users don't have to duplicate this conditional.
And then acpi_of_modalias() could be static and the only thing to export
would be acpi_set_modalias().
> +{
> + const union acpi_object *of_compatible;
> + const union acpi_object *obj;
> + const char *str, *chr;
> +
> + of_compatible = adev->data.of_compatible;
> + if (!of_compatible)
> + return -ENODEV;
> +
> + if (of_compatible->type == ACPI_TYPE_PACKAGE)
> + obj = of_compatible->package.elements;
> + else /* Must be ACPI_TYPE_STRING. */
> + obj = of_compatible;
> +
> + str = obj->string.pointer;
> + chr = strchr(str, ',');
> + strlcpy(outstr, chr ? chr + 1 : str, outlen);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(acpi_of_modalias);
> +
> static bool __acpi_match_device_cls(const struct acpi_device_id *id,
> struct acpi_hardware_id *hwid)
> {
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 4242c31..351b4a4 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -522,6 +522,7 @@ void acpi_bus_trim(struct acpi_device *start);
> acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
> int acpi_match_device_ids(struct acpi_device *device,
> const struct acpi_device_id *ids);
> +int acpi_of_modalias(struct acpi_device *adev, char *outstr, size_t outlen);
> int acpi_create_dir(struct acpi_device *);
> void acpi_remove_dir(struct acpi_device *);
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH v4 1/3] ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node
From: Dan O'Donovan @ 2017-02-02 13:30 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-acpi, Andy Shevchenko, Jarkko Nikula, Mika Westerberg,
Mark Brown, Len Brown, linux-i2c, Wolfram Sang, linux-spi,
linux-kernel
In-Reply-To: <2538517.VnXrn8zi1x@aspire.rjw.lan>
On 02/02/2017 11:41 AM, Rafael J. Wysocki wrote:
> On Friday, January 27, 2017 01:30:13 PM Dan O'Donovan wrote:
>> From: Crestez Dan Leonard <leonard.crestez@intel.com>
>>
>> When using devicetree stuff like i2c_client.name or spi_device.modalias
>> is initialized to the first DT compatible id with the vendor prefix
>> stripped. Since some drivers rely on this try to replicate it when using
>> ACPI with DT ids.
>>
>> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
>> Signed-off-by: Dan O'Donovan <dan@emutex.com>
>> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
>> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> First off, the original author of the patch doesn't seem to be reachable under
> the Intel address any more, so his S-o-b is meaningless and it shouldn't be
> there. In such cases you can send the patch as From: you and give a credit
> to the original author in the changelog.
>
> Second, as Mark said ->
>
>> ---
>> drivers/acpi/bus.c | 35 +++++++++++++++++++++++++++++++++++
>> include/acpi/acpi_bus.h | 1 +
>> 2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
>> index 95855cb..8b9657f 100644
>> --- a/drivers/acpi/bus.c
>> +++ b/drivers/acpi/bus.c
>> @@ -677,6 +677,41 @@ static bool acpi_of_match_device(struct acpi_device *adev,
>> return false;
>> }
>>
>> +/**
>> + * acpi_of_modalias - Like of_modalias_node for ACPI with DT ids
>> + * @adev: ACPI device object to match.
>> + * @outstr: Pointer to buffer for result
>> + * @outlen: Length of outstr value
>> + *
>> + * This is a counterpart of of_modalias_node() for struct acpi_device
>> + * objects. If there is a compatible string for @adev, copy it to the
>> + * @outstr location with the vendor prefix stripped.
>> + *
>> + * Returns 0 on success or negative errno on failure.
>> + */
>> +int acpi_of_modalias(struct acpi_device *adev, char *outstr, size_t outlen)
> -> this could be a bool function and you could provide something like
> acpi_set_modalias() doing the entire
>
> if (!acpi_of_modalias(adev, outstr, outlen))
> strlcpy(outstr, dev_name(&adev->dev), outlen);
>
> thing, so that its users don't have to duplicate this conditional.
>
> And then acpi_of_modalias() could be static and the only thing to export
> would be acpi_set_modalias().
Thanks Mark and Rafael for the latest feedback. Those suggestions make
sense. I'll send an updated patch-set soon.
>
>> +{
>> + const union acpi_object *of_compatible;
>> + const union acpi_object *obj;
>> + const char *str, *chr;
>> +
>> + of_compatible = adev->data.of_compatible;
>> + if (!of_compatible)
>> + return -ENODEV;
>> +
>> + if (of_compatible->type == ACPI_TYPE_PACKAGE)
>> + obj = of_compatible->package.elements;
>> + else /* Must be ACPI_TYPE_STRING. */
>> + obj = of_compatible;
>> +
>> + str = obj->string.pointer;
>> + chr = strchr(str, ',');
>> + strlcpy(outstr, chr ? chr + 1 : str, outlen);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(acpi_of_modalias);
>> +
>> static bool __acpi_match_device_cls(const struct acpi_device_id *id,
>> struct acpi_hardware_id *hwid)
>> {
>> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
>> index 4242c31..351b4a4 100644
>> --- a/include/acpi/acpi_bus.h
>> +++ b/include/acpi/acpi_bus.h
>> @@ -522,6 +522,7 @@ void acpi_bus_trim(struct acpi_device *start);
>> acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
>> int acpi_match_device_ids(struct acpi_device *device,
>> const struct acpi_device_id *ids);
>> +int acpi_of_modalias(struct acpi_device *adev, char *outstr, size_t outlen);
>> int acpi_create_dir(struct acpi_device *);
>> void acpi_remove_dir(struct acpi_device *);
> Thanks,
> Rafael
>
>
^ 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