[tip: ras/core] x86/mce: Rename kill_it to kill_current_task
The following commit has been merged into the ras/core branch of tip:
Commit-ID: e1c06d2366e743475b91045ef0c2ce1bbd028cb6
Gitweb:
https://git.kernel.org/tip/e1c06d2366e743475b91045ef0c2ce1bbd028cb6
Author:Gabriele Paoloni
AuthorDate:Fri, 27 Nov 2020 16:18:19
Committer: Borislav Petkov
CommitterDate: Tue, 01 Dec 2020 18:58:50 +01:00
x86/mce: Rename kill_it to kill_current_task
Currently, if an MCE happens in user-mode or while the kernel is copying
data from user space, 'kill_it' is used to check if execution of the
interrupted task can be recovered or not; the flag name however is not
very meaningful, hence rename it to match its goal.
[ bp: Massage commit message, rename the queue_task_work() arg too. ]
Signed-off-by: Gabriele Paoloni
Signed-off-by: Borislav Petkov
Link:
https://lkml.kernel.org/r/20201127161819.3106432-6-gabriele.paol...@intel.com
---
arch/x86/kernel/cpu/mce/core.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index a9991a9..6af6a3c 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1266,14 +1266,14 @@ static void kill_me_maybe(struct callback_head *cb)
}
}
-static void queue_task_work(struct mce *m, int kill_it)
+static void queue_task_work(struct mce *m, int kill_current_task)
{
current->mce_addr = m->addr;
current->mce_kflags = m->kflags;
current->mce_ripv = !!(m->mcgstatus & MCG_STATUS_RIPV);
current->mce_whole_page = whole_page(m);
- if (kill_it)
+ if (kill_current_task)
current->mce_kill_me.func = kill_me_now;
else
current->mce_kill_me.func = kill_me_maybe;
@@ -1321,10 +1321,10 @@ noinstr void do_machine_check(struct pt_regs *regs)
int no_way_out = 0;
/*
-* If kill_it gets set, there might be a way to recover from this
+* If kill_current_task is not set, there might be a way to recover
from this
* error.
*/
- int kill_it = 0;
+ int kill_current_task = 0;
/*
* MCEs are always local on AMD. Same is determined by MCG_STATUS_LMCES
@@ -1351,7 +1351,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
* severity is MCE_AR_SEVERITY we have other options.
*/
if (!(m.mcgstatus & MCG_STATUS_RIPV))
- kill_it = (cfg->tolerant == 3) ? 0 : 1;
+ kill_current_task = (cfg->tolerant == 3) ? 0 : 1;
/*
* Check if this MCE is signaled to only this logical processor,
* on Intel, Zhaoxin only.
@@ -1406,7 +1406,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
}
}
- if (worst != MCE_AR_SEVERITY && !kill_it)
+ if (worst != MCE_AR_SEVERITY && !kill_current_task)
goto out;
/* Fault was in user mode and we need to take some action */
@@ -1414,7 +1414,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
/* If this triggers there is no way to recover. Die hard. */
BUG_ON(!on_thread_stack() || !user_mode(regs));
- queue_task_work(, kill_it);
+ queue_task_work(, kill_current_task);
} else {
/*
@@ -1432,7 +1432,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
}
if (m.kflags & MCE_IN_KERNEL_COPYIN)
- queue_task_work(, kill_it);
+ queue_task_work(, kill_current_task);
}
out:
mce_wrmsrl(MSR_IA32_MCG_STATUS, 0);
[tip: ras/core] x86/mce: Panic for LMCE only if mca_cfg.tolerant < 3
The following commit has been merged into the ras/core branch of tip:
Commit-ID: 3a866b16fd2360a9c4ebf71cfbf7ebfe968c1409
Gitweb:
https://git.kernel.org/tip/3a866b16fd2360a9c4ebf71cfbf7ebfe968c1409
Author:Gabriele Paoloni
AuthorDate:Fri, 27 Nov 2020 16:18:17
Committer: Borislav Petkov
CommitterDate: Tue, 01 Dec 2020 18:49:29 +01:00
x86/mce: Panic for LMCE only if mca_cfg.tolerant < 3
Right now for LMCE, if no_way_out is set, mce_panic() is called
regardless of mca_cfg.tolerant. This is not correct as, if
mca_cfg.tolerant = 3, the code should never panic.
Add that check.
[ bp: use local ptr 'cfg'. ]
Signed-off-by: Gabriele Paoloni
Signed-off-by: Borislav Petkov
Reviewed-by: Tony Luck
Link:
https://lkml.kernel.org/r/20201127161819.3106432-4-gabriele.paol...@intel.com
---
arch/x86/kernel/cpu/mce/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index ebaa52a..99da2e0 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1368,7 +1368,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
* to see it will clear it.
*/
if (lmce) {
- if (no_way_out)
+ if (no_way_out && cfg->tolerant < 3)
mce_panic("Fatal local machine check", , msg);
} else {
order = mce_start(_way_out);
[tip: ras/core] x86/mce: Move the mce_panic() call and 'kill_it' assignments to the right places
The following commit has been merged into the ras/core branch of tip:
Commit-ID: e273e6e12ab1db3eb57712bd60655744d0091fa3
Gitweb:
https://git.kernel.org/tip/e273e6e12ab1db3eb57712bd60655744d0091fa3
Author:Gabriele Paoloni
AuthorDate:Fri, 27 Nov 2020 16:18:16
Committer: Borislav Petkov
CommitterDate: Tue, 01 Dec 2020 18:45:56 +01:00
x86/mce: Move the mce_panic() call and 'kill_it' assignments to the right places
Right now, for local MCEs the machine calls panic(), if needed, right
after lmce is set. For MCE broadcasting, mce_reign() takes care of
calling mce_panic().
Hence:
- improve readability by moving the conditional evaluation of
tolerant up to when kill_it is set first;
- move the mce_panic() call up into the statement where mce_end()
fails.
[ bp: Massage, remove comment in the mce_end() failure case because it
is superfluous; use local ptr 'cfg' in both tests. ]
Signed-off-by: Gabriele Paoloni
Signed-off-by: Borislav Petkov
Reviewed-by: Tony Luck
Link:
https://lkml.kernel.org/r/20201127161819.3106432-3-gabriele.paol...@intel.com
---
arch/x86/kernel/cpu/mce/core.c | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index f319bed..ebaa52a 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1351,8 +1351,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
* severity is MCE_AR_SEVERITY we have other options.
*/
if (!(m.mcgstatus & MCG_STATUS_RIPV))
- kill_it = 1;
-
+ kill_it = (cfg->tolerant == 3) ? 0 : 1;
/*
* Check if this MCE is signaled to only this logical processor,
* on Intel, Zhaoxin only.
@@ -1388,6 +1387,9 @@ noinstr void do_machine_check(struct pt_regs *regs)
if (mce_end(order) < 0) {
if (!no_way_out)
no_way_out = worst >= MCE_PANIC_SEVERITY;
+
+ if (no_way_out && cfg->tolerant < 3)
+ mce_panic("Fatal machine check on current CPU",
, msg);
}
} else {
/*
@@ -1404,15 +1406,6 @@ noinstr void do_machine_check(struct pt_regs *regs)
}
}
- /*
-* If tolerant is at an insane level we drop requests to kill
-* processes and continue even when there is no way out.
-*/
- if (cfg->tolerant == 3)
- kill_it = 0;
- else if (no_way_out)
- mce_panic("Fatal machine check on current CPU", , msg);
-
if (worst > 0)
irq_work_queue(_irq_work);
[tip: ras/core] x86/mce: Remove redundant call to irq_work_queue()
The following commit has been merged into the ras/core branch of tip: Commit-ID: d5b38e3d0fdb1a16994b449bc338fb8b26816b07 Gitweb: https://git.kernel.org/tip/d5b38e3d0fdb1a16994b449bc338fb8b26816b07 Author:Gabriele Paoloni AuthorDate:Fri, 27 Nov 2020 16:18:18 Committer: Borislav Petkov CommitterDate: Tue, 01 Dec 2020 18:54:32 +01:00 x86/mce: Remove redundant call to irq_work_queue() Currently, __mc_scan_banks() in do_machine_check() does the following callchain: __mc_scan_banks()->mce_log()->irq_work_queue(_irq_work). Hence, the call to irq_work_queue() below after __mc_scan_banks() seems redundant. Just remove it. Signed-off-by: Gabriele Paoloni Signed-off-by: Borislav Petkov Reviewed-by: Tony Luck Link: https://lkml.kernel.org/r/20201127161819.3106432-5-gabriele.paol...@intel.com --- arch/x86/kernel/cpu/mce/core.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index 99da2e0..a9991a9 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -1406,9 +1406,6 @@ noinstr void do_machine_check(struct pt_regs *regs) } } - if (worst > 0) - irq_work_queue(_irq_work); - if (worst != MCE_AR_SEVERITY && !kill_it) goto out;
[tip: x86/urgent] x86/mce: Do not overwrite no_way_out if mce_end() fails
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: 25bc65d8ddfc17cc1d7a45bd48e9bdc0e729ced3
Gitweb:
https://git.kernel.org/tip/25bc65d8ddfc17cc1d7a45bd48e9bdc0e729ced3
Author:Gabriele Paoloni
AuthorDate:Fri, 27 Nov 2020 16:18:15
Committer: Borislav Petkov
CommitterDate: Fri, 27 Nov 2020 17:38:36 +01:00
x86/mce: Do not overwrite no_way_out if mce_end() fails
Currently, if mce_end() fails, no_way_out - the variable denoting
whether the machine can recover from this MCE - is determined by whether
the worst severity that was found across the MCA banks associated with
the current CPU, is of panic severity.
However, at this point no_way_out could have been already set by
mca_start() after looking at all severities of all CPUs that entered the
MCE handler. If mce_end() fails, check first if no_way_out is already
set and, if so, stick to it, otherwise use the local worst value.
[ bp: Massage. ]
Signed-off-by: Gabriele Paoloni
Signed-off-by: Borislav Petkov
Reviewed-by: Tony Luck
Cc:
Link:
https://lkml.kernel.org/r/20201127161819.3106432-2-gabriele.paol...@intel.com
---
arch/x86/kernel/cpu/mce/core.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 4102b86..32b7099 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1384,8 +1384,10 @@ noinstr void do_machine_check(struct pt_regs *regs)
* When there's any problem use only local no_way_out state.
*/
if (!lmce) {
- if (mce_end(order) < 0)
- no_way_out = worst >= MCE_PANIC_SEVERITY;
+ if (mce_end(order) < 0) {
+ if (!no_way_out)
+ no_way_out = worst >= MCE_PANIC_SEVERITY;
+ }
} else {
/*
* If there was a fatal machine check we should have
[PATCH v2 5/5] x86/mce: rename kill_it as kill_current_task
Currently if an MCE happens in user-mode or while the kernel
is copying data from user space, 'kill_it' is used to check
if we can recover the execution of the interrupted task or
not; the flag name however is not much meaningful, hence
rename it to match its goal.
Signed-off-by: Gabriele Paoloni
---
arch/x86/kernel/cpu/mce/core.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 802302c54762..740a4fcc1e90 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1320,10 +1320,10 @@ noinstr void do_machine_check(struct pt_regs *regs)
int no_way_out = 0;
/*
-* If kill_it gets set, there might be a way to recover from this
+* If kill_current_task is not set, there might be a way to recover
from this
* error.
*/
- int kill_it = 0;
+ int kill_current_task = 0;
/*
* MCEs are always local on AMD. Same is determined by MCG_STATUS_LMCES
@@ -1350,7 +1350,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
* severity is MCE_AR_SEVERITY we have other options.
*/
if (!(m.mcgstatus & MCG_STATUS_RIPV))
- kill_it = (cfg->tolerant == 3) ? 0 : 1;
+ kill_current_task = (cfg->tolerant == 3) ? 0 : 1;
/*
* Check if this MCE is signaled to only this logical processor,
* on Intel, Zhaoxin only.
@@ -1408,7 +1408,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
}
}
- if (worst != MCE_AR_SEVERITY && !kill_it)
+ if (worst != MCE_AR_SEVERITY && !kill_current_task)
goto out;
/* Fault was in user mode and we need to take some action */
@@ -1416,7 +1416,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
/* If this triggers there is no way to recover. Die hard. */
BUG_ON(!on_thread_stack() || !user_mode(regs));
- queue_task_work(, kill_it);
+ queue_task_work(, kill_current_task);
} else {
/*
@@ -1434,7 +1434,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
}
if (m.kflags & MCE_IN_KERNEL_COPYIN)
- queue_task_work(, kill_it);
+ queue_task_work(, kill_current_task);
}
out:
mce_wrmsrl(MSR_IA32_MCG_STATUS, 0);
--
2.20.1
-
INTEL CORPORATION ITALIA S.p.A. con unico socio
Sede: Milanofiori Palazzo E 4
CAP 20094 Assago (MI)
Capitale Sociale Euro 104.000,00 interamente versato
Partita I.V.A. e Codice Fiscale 04236760155
Repertorio Economico Amministrativo n. 997124
Registro delle Imprese di Milano nr. 183983/5281/33
Soggetta ad attivita' di direzione e coordinamento di
INTEL CORPORATION, USA
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
[PATCH v2 4/5] x86/mce: remove redundant call to irq_work_queue()
Right now in do_machine_check() __mc_scan_banks() triggers the following call tree: __mc_scan_banks()->mce_log()->irq_work_queue(_irq_work). Hence the call of irq_work_queue() below after __mc_scan_banks() seems redundant. Just remove it. Signed-off-by: Gabriele Paoloni Reviewed-by: Tony Luck --- arch/x86/kernel/cpu/mce/core.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index d766a3f6a343..802302c54762 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -1408,9 +1408,6 @@ noinstr void do_machine_check(struct pt_regs *regs) } } - if (worst > 0) - irq_work_queue(_irq_work); - if (worst != MCE_AR_SEVERITY && !kill_it) goto out; -- 2.20.1 - INTEL CORPORATION ITALIA S.p.A. con unico socio Sede: Milanofiori Palazzo E 4 CAP 20094 Assago (MI) Capitale Sociale Euro 104.000,00 interamente versato Partita I.V.A. e Codice Fiscale 04236760155 Repertorio Economico Amministrativo n. 997124 Registro delle Imprese di Milano nr. 183983/5281/33 Soggetta ad attivita' di direzione e coordinamento di INTEL CORPORATION, USA This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
[PATCH v2 2/5] x86/mce: move the mce_panic() call and 'kill_it' assignments to the right places
Right now for local MCEs we panic(),if needed, right after lmce is
set. For global MCEs mce_reign() takes care of calling mce_panic().
Hence:
- improve readibility by moving the conditional evaluation of
tolerant up to when kill_it is set first;
- move the mce_panic() call up into the statement where mce_end()
fails.
Signed-off-by: Gabriele Paoloni
Reviewed-by: Tony Luck
---
arch/x86/kernel/cpu/mce/core.c | 18 +++---
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 32b7099e3511..50e9b0893a92 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1350,8 +1350,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
* severity is MCE_AR_SEVERITY we have other options.
*/
if (!(m.mcgstatus & MCG_STATUS_RIPV))
- kill_it = 1;
-
+ kill_it = (cfg->tolerant == 3) ? 0 : 1;
/*
* Check if this MCE is signaled to only this logical processor,
* on Intel, Zhaoxin only.
@@ -1387,6 +1386,12 @@ noinstr void do_machine_check(struct pt_regs *regs)
if (mce_end(order) < 0) {
if (!no_way_out)
no_way_out = worst >= MCE_PANIC_SEVERITY;
+ /*
+* mce_reign() has probably failed hence evaluate if we
need
+* to panic
+*/
+ if (no_way_out && mca_cfg.tolerant < 3)
+ mce_panic("Fatal machine check on current CPU",
, msg);
}
} else {
/*
@@ -1403,15 +1408,6 @@ noinstr void do_machine_check(struct pt_regs *regs)
}
}
- /*
-* If tolerant is at an insane level we drop requests to kill
-* processes and continue even when there is no way out.
-*/
- if (cfg->tolerant == 3)
- kill_it = 0;
- else if (no_way_out)
- mce_panic("Fatal machine check on current CPU", , msg);
-
if (worst > 0)
irq_work_queue(_irq_work);
--
2.20.1
-
INTEL CORPORATION ITALIA S.p.A. con unico socio
Sede: Milanofiori Palazzo E 4
CAP 20094 Assago (MI)
Capitale Sociale Euro 104.000,00 interamente versato
Partita I.V.A. e Codice Fiscale 04236760155
Repertorio Economico Amministrativo n. 997124
Registro delle Imprese di Milano nr. 183983/5281/33
Soggetta ad attivita' di direzione e coordinamento di
INTEL CORPORATION, USA
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
[PATCH v2 3/5] x86/mce: for LMCE panic only if mca_cfg.tolerant < 3
Right now for LMCE if no_way_out is set mce_panic() is called
regardless of mca_cfg.tolerant. This is not correct as, if
mca_cfg.tolerant = 3, the code should never panic.
Signed-off-by: Gabriele Paoloni
Reviewed-by: Tony Luck
---
arch/x86/kernel/cpu/mce/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 50e9b0893a92..d766a3f6a343 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1367,7 +1367,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
* to see it will clear it.
*/
if (lmce) {
- if (no_way_out)
+ if (no_way_out && mca_cfg.tolerant < 3)
mce_panic("Fatal local machine check", , msg);
} else {
order = mce_start(_way_out);
--
2.20.1
-
INTEL CORPORATION ITALIA S.p.A. con unico socio
Sede: Milanofiori Palazzo E 4
CAP 20094 Assago (MI)
Capitale Sociale Euro 104.000,00 interamente versato
Partita I.V.A. e Codice Fiscale 04236760155
Repertorio Economico Amministrativo n. 997124
Registro delle Imprese di Milano nr. 183983/5281/33
Soggetta ad attivita' di direzione e coordinamento di
INTEL CORPORATION, USA
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
[PATCH v2 1/5] x86/mce: do not overwrite no_way_out if mce_end() fails
Currently if mce_end() fails 'no_way_out' is set equal to 'worst'.
'worst' is the worst severity that was found across the MCA banks
associated with the current CPU; however at this point 'no_way_out'
could have been already set by mca_start() by looking at all
severities of all CPUs that entered the MCE handler.
If mce_end() fails, check first if no_way_out is already set and,
if so, stick to it, otherwise use the local worst value.
Cc:
Signed-off-by: Gabriele Paoloni
Reviewed-by: Tony Luck
---
arch/x86/kernel/cpu/mce/core.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 4102b866e7c0..32b7099e3511 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1384,8 +1384,10 @@ noinstr void do_machine_check(struct pt_regs *regs)
* When there's any problem use only local no_way_out state.
*/
if (!lmce) {
- if (mce_end(order) < 0)
- no_way_out = worst >= MCE_PANIC_SEVERITY;
+ if (mce_end(order) < 0) {
+ if (!no_way_out)
+ no_way_out = worst >= MCE_PANIC_SEVERITY;
+ }
} else {
/*
* If there was a fatal machine check we should have
--
2.20.1
-
INTEL CORPORATION ITALIA S.p.A. con unico socio
Sede: Milanofiori Palazzo E 4
CAP 20094 Assago (MI)
Capitale Sociale Euro 104.000,00 interamente versato
Partita I.V.A. e Codice Fiscale 04236760155
Repertorio Economico Amministrativo n. 997124
Registro delle Imprese di Milano nr. 183983/5281/33
Soggetta ad attivita' di direzione e coordinamento di
INTEL CORPORATION, USA
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
[PATCH v2 0/5] x86/MCE: some minor fixes
During the safety analysis that was done in the context of the ELISA project by the safety architecture working group some incorrectnesses were spotted. This patchset proposes some fixes. Changes since v1: - fixed grammar - improved readibility of patch1 and Cc'd for stable - kill_it flag renamed to kill_current_task Signed-off-by: Gabriele Paoloni Reviewed-by: Tony Luck Gabriele Paoloni (5): x86/mce: do not overwrite no_way_out if mce_end() fails x86/mce: move the mce_panic() call and 'kill_it' assignments to the right places x86/mce: for LMCE panic only if mca_cfg.tolerant < 3 x86/mce: remove redundant call to irq_work_queue() x86/mce: rename kill_it as kill_current_task arch/x86/kernel/cpu/mce/core.c | 39 +++--- 1 file changed, 17 insertions(+), 22 deletions(-) -- 2.20.1 - INTEL CORPORATION ITALIA S.p.A. con unico socio Sede: Milanofiori Palazzo E 4 CAP 20094 Assago (MI) Capitale Sociale Euro 104.000,00 interamente versato Partita I.V.A. e Codice Fiscale 04236760155 Repertorio Economico Amministrativo n. 997124 Registro delle Imprese di Milano nr. 183983/5281/33 Soggetta ad attivita' di direzione e coordinamento di INTEL CORPORATION, USA This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
[PATCH 0/4] x86/MCE: some minor fixes
During the safety analysis that was done in the context of the ELISA project by the safety architecture working group some incorrectnesses were spotted. This patchset proposes some fixes. Signed-off-by: Gabriele Paoloni Reviewed-by: Tony Luck Gabriele Paoloni (4): x86/mce: do not overwrite no_way_out if mce_end() fails x86/mce: move the mce_panic() call and kill_it assignments at the right places x86/mce: for LMCE panic only if mca_cfg.tolerant < 3 x86/mce: remove redundant call to irq_work_queue() arch/x86/kernel/cpu/mce/core.c | 28 +++- 1 file changed, 11 insertions(+), 17 deletions(-) -- 2.20.1 - INTEL CORPORATION ITALIA S.p.A. con unico socio Sede: Milanofiori Palazzo E 4 CAP 20094 Assago (MI) Capitale Sociale Euro 104.000,00 interamente versato Partita I.V.A. e Codice Fiscale 04236760155 Repertorio Economico Amministrativo n. 997124 Registro delle Imprese di Milano nr. 183983/5281/33 Soggetta ad attivita' di direzione e coordinamento di INTEL CORPORATION, USA This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
[PATCH 1/4] x86/mce: do not overwrite no_way_out if mce_end() fails
Currently if mce_end() fails no_way_out is set equal to worst.
worst is the worst severirty that was found in the MCA banks
associated to the current CPU; however at this point no_way_out
could be already set by mca_start() by looking at all severities
of all CPUs that entered the MCE handler.
if mce_end() fails we first check if no_way_out is already set and
if so we stick to it, otherwise we use the local worst value
Signed-off-by: Gabriele Paoloni
Reviewed-by: Tony Luck
---
arch/x86/kernel/cpu/mce/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 4102b866e7c0..b990892c6766 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1385,7 +1385,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
*/
if (!lmce) {
if (mce_end(order) < 0)
- no_way_out = worst >= MCE_PANIC_SEVERITY;
+ no_way_out = no_way_out ? no_way_out : worst >=
MCE_PANIC_SEVERITY;
} else {
/*
* If there was a fatal machine check we should have
--
2.20.1
-
INTEL CORPORATION ITALIA S.p.A. con unico socio
Sede: Milanofiori Palazzo E 4
CAP 20094 Assago (MI)
Capitale Sociale Euro 104.000,00 interamente versato
Partita I.V.A. e Codice Fiscale 04236760155
Repertorio Economico Amministrativo n. 997124
Registro delle Imprese di Milano nr. 183983/5281/33
Soggetta ad attivita' di direzione e coordinamento di
INTEL CORPORATION, USA
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
[PATCH 4/4] x86/mce: remove redundant call to irq_work_queue()
Right now in do_machine_check() we have: __mc_scan_banks()->mce_log()->irq_work_queue(_irq_work) hence the call of irq_work_queue() below after __mc_scan_banks() seems redundant. Just remove it. Signed-off-by: Gabriele Paoloni Reviewed-by: Tony Luck --- arch/x86/kernel/cpu/mce/core.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index d16cbb05b09c..f2f7bfc60c67 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -1407,9 +1407,6 @@ noinstr void do_machine_check(struct pt_regs *regs) } } - if (worst > 0) - irq_work_queue(_irq_work); - if (worst != MCE_AR_SEVERITY && !kill_it) goto out; -- 2.20.1 - INTEL CORPORATION ITALIA S.p.A. con unico socio Sede: Milanofiori Palazzo E 4 CAP 20094 Assago (MI) Capitale Sociale Euro 104.000,00 interamente versato Partita I.V.A. e Codice Fiscale 04236760155 Repertorio Economico Amministrativo n. 997124 Registro delle Imprese di Milano nr. 183983/5281/33 Soggetta ad attivita' di direzione e coordinamento di INTEL CORPORATION, USA This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
[PATCH 2/4] x86/mce: move the mce_panic() call and kill_it assignments at the right places
Right now for local MCEs we panic(),if needed, right after lmce is
set. For global MCEs mce_reign() takes care of calling mce_panic().
Hence this patch:
- improves readibility by moving the conditional evaluation of
tolerant up to when kill_it is set first
- moves the mce_panic() call up into the statement where mce_end()
fails
Signed-off-by: Gabriele Paoloni
Reviewed-by: Tony Luck
---
arch/x86/kernel/cpu/mce/core.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index b990892c6766..e025ff04438f 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1350,8 +1350,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
* severity is MCE_AR_SEVERITY we have other options.
*/
if (!(m.mcgstatus & MCG_STATUS_RIPV))
- kill_it = 1;
-
+ kill_it = (cfg->tolerant == 3) ? 0 : 1;
/*
* Check if this MCE is signaled to only this logical processor,
* on Intel, Zhaoxin only.
@@ -1384,8 +1383,15 @@ noinstr void do_machine_check(struct pt_regs *regs)
* When there's any problem use only local no_way_out state.
*/
if (!lmce) {
- if (mce_end(order) < 0)
+ if (mce_end(order) < 0) {
no_way_out = no_way_out ? no_way_out : worst >=
MCE_PANIC_SEVERITY;
+ /*
+* mce_reign() has probably failed hence evaluate if we
need
+* to panic
+*/
+ if (no_way_out && mca_cfg.tolerant < 3)
+ mce_panic("Fatal machine check on current CPU",
, msg);
+ }
} else {
/*
* If there was a fatal machine check we should have
@@ -1401,15 +1407,6 @@ noinstr void do_machine_check(struct pt_regs *regs)
}
}
- /*
-* If tolerant is at an insane level we drop requests to kill
-* processes and continue even when there is no way out.
-*/
- if (cfg->tolerant == 3)
- kill_it = 0;
- else if (no_way_out)
- mce_panic("Fatal machine check on current CPU", , msg);
-
if (worst > 0)
irq_work_queue(_irq_work);
--
2.20.1
-
INTEL CORPORATION ITALIA S.p.A. con unico socio
Sede: Milanofiori Palazzo E 4
CAP 20094 Assago (MI)
Capitale Sociale Euro 104.000,00 interamente versato
Partita I.V.A. e Codice Fiscale 04236760155
Repertorio Economico Amministrativo n. 997124
Registro delle Imprese di Milano nr. 183983/5281/33
Soggetta ad attivita' di direzione e coordinamento di
INTEL CORPORATION, USA
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
[PATCH 3/4] x86/mce: for LMCE panic only if mca_cfg.tolerant < 3
Right now for LMCE if no_way_out is set mce_panic() is called
regardless of mca_cfg.tolerant. This is not correct as if
mca_cfg.tolerant = 3 we should never panic.
Signed-off-by: Gabriele Paoloni
Reviewed-by: Tony Luck
---
arch/x86/kernel/cpu/mce/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index e025ff04438f..d16cbb05b09c 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1367,7 +1367,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
* to see it will clear it.
*/
if (lmce) {
- if (no_way_out)
+ if (no_way_out && mca_cfg.tolerant < 3)
mce_panic("Fatal local machine check", , msg);
} else {
order = mce_start(_way_out);
--
2.20.1
-
INTEL CORPORATION ITALIA S.p.A. con unico socio
Sede: Milanofiori Palazzo E 4
CAP 20094 Assago (MI)
Capitale Sociale Euro 104.000,00 interamente versato
Partita I.V.A. e Codice Fiscale 04236760155
Repertorio Economico Amministrativo n. 997124
Registro delle Imprese di Milano nr. 183983/5281/33
Soggetta ad attivita' di direzione e coordinamento di
INTEL CORPORATION, USA
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
RE: [PATCH v10 0/9] LPC: legacy ISA I/O support
Hi Dann > -Original Message- > From: dann frazier [mailto:dann.fraz...@canonical.com] > Sent: 09 November 2017 16:16 > To: Gabriele Paoloni > Cc: Catalin Marinas; Will Deacon; Rob Herring; Frank Rowand; Bjorn > Helgaas; raf...@kernel.org; Arnd Bergmann; linux-arm-kernel; > lorenzo.pieral...@arm.com; Mark Rutland; Brian Starkey; o...@lixom.net; > b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux- > a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org; Corey > Minyard; John Garry; xuwei (O) > Subject: Re: [PATCH v10 0/9] LPC: legacy ISA I/O support > > On Fri, Oct 27, 2017 at 10:11 AM, Gabriele Paoloni > <gabriele.paol...@huawei.com> wrote: > > From: gabriele paoloni <gabriele.paol...@huawei.com> > > > > This patchset supports the IPMI-bt device attached to the Low-Pin- > Count > > interface implemented on Hisilicon Hip06/Hip07 SoC. > > fwiw, I tested this on one of our D05 boards and verified that the > IPMI SI worked fine. > > Tested-by: dann frazier <dann.fraz...@canonical.com> Many thanks for this Gab
RE: [PATCH v10 0/9] LPC: legacy ISA I/O support
Hi Dann > -Original Message- > From: dann frazier [mailto:dann.fraz...@canonical.com] > Sent: 09 November 2017 16:16 > To: Gabriele Paoloni > Cc: Catalin Marinas; Will Deacon; Rob Herring; Frank Rowand; Bjorn > Helgaas; raf...@kernel.org; Arnd Bergmann; linux-arm-kernel; > lorenzo.pieral...@arm.com; Mark Rutland; Brian Starkey; o...@lixom.net; > b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux- > a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org; Corey > Minyard; John Garry; xuwei (O) > Subject: Re: [PATCH v10 0/9] LPC: legacy ISA I/O support > > On Fri, Oct 27, 2017 at 10:11 AM, Gabriele Paoloni > wrote: > > From: gabriele paoloni > > > > This patchset supports the IPMI-bt device attached to the Low-Pin- > Count > > interface implemented on Hisilicon Hip06/Hip07 SoC. > > fwiw, I tested this on one of our D05 boards and verified that the > IPMI SI worked fine. > > Tested-by: dann frazier Many thanks for this Gab
RE: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings
Hi Dann [...] > > + > > +#define START_WORK 0x01 > > Any reason not to put this in the LPC_ namespace as well? No, not really. We'll make it consistent in the next patchset > > > +/* The minimal nanosecond interval for each query on LPC cycle > status. */ [...] > > + * hisilpc_target_in - trigger a series of lpc cycles to read > required data > > + *from target peripheral. > > + * @pdev: pointer to hisi lpc device > > It's now lpcdev > Thanks, we'll change it. > > + * @para: some parameters used to control the lpc I/O operations [...] > > + * @pio: the target I/O port address. > > @outval & @pio are in the opposite order of the actual function Thanks, we'll change it. > [...] > > + lpcdev->io_host->devpara = NULL; > > + dev_err(dev, "OF: scan hisilpc children got failed(%d)\n", > > + ret); > > nit: Maybe "OF: scanning hisilpc children failed(%d)" ? > Yes, thanks we'll change it. > > + return ret; > > + } > > + > > + dev_info(dev, "hslpc end probing. range[%pa - sz:%pa]\n", > > + >io_host->io_start, > > + >io_host->size); > > + > > + return ret; > > +} > > > -dann
RE: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings
Hi Dann [...] > > + > > +#define START_WORK 0x01 > > Any reason not to put this in the LPC_ namespace as well? No, not really. We'll make it consistent in the next patchset > > > +/* The minimal nanosecond interval for each query on LPC cycle > status. */ [...] > > + * hisilpc_target_in - trigger a series of lpc cycles to read > required data > > + *from target peripheral. > > + * @pdev: pointer to hisi lpc device > > It's now lpcdev > Thanks, we'll change it. > > + * @para: some parameters used to control the lpc I/O operations [...] > > + * @pio: the target I/O port address. > > @outval & @pio are in the opposite order of the actual function Thanks, we'll change it. > [...] > > + lpcdev->io_host->devpara = NULL; > > + dev_err(dev, "OF: scan hisilpc children got failed(%d)\n", > > + ret); > > nit: Maybe "OF: scanning hisilpc children failed(%d)" ? > Yes, thanks we'll change it. > > + return ret; > > + } > > + > > + dev_info(dev, "hslpc end probing. range[%pa - sz:%pa]\n", > > + >io_host->io_start, > > + >io_host->size); > > + > > + return ret; > > +} > > > -dann
RE: [PATCH v10 2/9] PCI: remove unused __weak attribute in pci_register_io_range()
> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: 07 November 2017 00:23
> To: Gabriele Paoloni
> Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org;
> a...@arndb.de; linux-arm-ker...@lists.infradead.org;
> lorenzo.pieral...@arm.com; mark.rutl...@arm.com; brian.star...@arm.com;
> o...@lixom.net; b...@kernel.crashing.org; linux-kernel@vger.kernel.org;
> linux-a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org;
> miny...@acm.org; John Garry; xuwei (O)
> Subject: Re: [PATCH v10 2/9] PCI: remove unused __weak attribute in
> pci_register_io_range()
>
> > PCI: remove unused __weak attribute in pci_register_io_range()
>
> Please capitalize to follow drivers/pci convention, e.g.,
>
> PCI: Remove __weak attribute from pci_register_io_range()
>
> (Also do the same for the other PCI patches.)
Agreed, many thanks
Gab
>
> On Fri, Oct 27, 2017 at 05:11:20PM +0100, Gabriele Paoloni wrote:
> > From: gabriele paoloni <gabriele.paol...@huawei.com>
> >
> > Currently pci_register_io_range() has only one definition;
> > therefore there is no use of the __weak attribute.
> >
> > Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
> > Acked-by: Bjorn Helgaas <helg...@kernel.org>
> > ---
> > drivers/pci/pci.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index af0cc34..eee967c 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3270,7 +3270,7 @@ static DEFINE_SPINLOCK(io_range_lock);
> > * Record the PCI IO range (expressed as CPU physical address +
> size).
> > * Return a negative value if an error has occured, zero otherwise
> > */
> > -int __weak pci_register_io_range(phys_addr_t addr, resource_size_t
> size)
> > +int pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > {
> > int err = 0;
> >
> > --
> > 2.7.4
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi"
> in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v10 2/9] PCI: remove unused __weak attribute in pci_register_io_range()
> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: 07 November 2017 00:23
> To: Gabriele Paoloni
> Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org;
> a...@arndb.de; linux-arm-ker...@lists.infradead.org;
> lorenzo.pieral...@arm.com; mark.rutl...@arm.com; brian.star...@arm.com;
> o...@lixom.net; b...@kernel.crashing.org; linux-kernel@vger.kernel.org;
> linux-a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org;
> miny...@acm.org; John Garry; xuwei (O)
> Subject: Re: [PATCH v10 2/9] PCI: remove unused __weak attribute in
> pci_register_io_range()
>
> > PCI: remove unused __weak attribute in pci_register_io_range()
>
> Please capitalize to follow drivers/pci convention, e.g.,
>
> PCI: Remove __weak attribute from pci_register_io_range()
>
> (Also do the same for the other PCI patches.)
Agreed, many thanks
Gab
>
> On Fri, Oct 27, 2017 at 05:11:20PM +0100, Gabriele Paoloni wrote:
> > From: gabriele paoloni
> >
> > Currently pci_register_io_range() has only one definition;
> > therefore there is no use of the __weak attribute.
> >
> > Signed-off-by: Gabriele Paoloni
> > Acked-by: Bjorn Helgaas
> > ---
> > drivers/pci/pci.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index af0cc34..eee967c 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3270,7 +3270,7 @@ static DEFINE_SPINLOCK(io_range_lock);
> > * Record the PCI IO range (expressed as CPU physical address +
> size).
> > * Return a negative value if an error has occured, zero otherwise
> > */
> > -int __weak pci_register_io_range(phys_addr_t addr, resource_size_t
> size)
> > +int pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > {
> > int err = 0;
> >
> > --
> > 2.7.4
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi"
> in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v10 2/9] PCI: remove unused __weak attribute in pci_register_io_range()
> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: 07 November 2017 00:25
> To: Gabriele Paoloni
> Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org;
> a...@arndb.de; linux-arm-ker...@lists.infradead.org;
> lorenzo.pieral...@arm.com; mark.rutl...@arm.com; brian.star...@arm.com;
> o...@lixom.net; b...@kernel.crashing.org; linux-kernel@vger.kernel.org;
> linux-a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org;
> miny...@acm.org; John Garry; xuwei (O)
> Subject: Re: [PATCH v10 2/9] PCI: remove unused __weak attribute in
> pci_register_io_range()
>
> On Fri, Oct 27, 2017 at 05:11:20PM +0100, Gabriele Paoloni wrote:
> > From: gabriele paoloni <gabriele.paol...@huawei.com>
> >
> > Currently pci_register_io_range() has only one definition;
> > therefore there is no use of the __weak attribute.
> >
> > Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
> > Acked-by: Bjorn Helgaas <helg...@kernel.org>
>
> Also, I normally use this (Google address instead of kernel.org):
>
> Acked-by: Bjorn Helgaas <bhelg...@google.com>
>
> (Also affects your other PCI patches.)
Ok no probs, we'll change all the Acked-by in the next patchset
Many Thanks
Gab
>
> > ---
> > drivers/pci/pci.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index af0cc34..eee967c 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3270,7 +3270,7 @@ static DEFINE_SPINLOCK(io_range_lock);
> > * Record the PCI IO range (expressed as CPU physical address +
> size).
> > * Return a negative value if an error has occured, zero otherwise
> > */
> > -int __weak pci_register_io_range(phys_addr_t addr, resource_size_t
> size)
> > +int pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > {
> > int err = 0;
> >
> > --
> > 2.7.4
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi"
> in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v10 2/9] PCI: remove unused __weak attribute in pci_register_io_range()
> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: 07 November 2017 00:25
> To: Gabriele Paoloni
> Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org;
> a...@arndb.de; linux-arm-ker...@lists.infradead.org;
> lorenzo.pieral...@arm.com; mark.rutl...@arm.com; brian.star...@arm.com;
> o...@lixom.net; b...@kernel.crashing.org; linux-kernel@vger.kernel.org;
> linux-a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org;
> miny...@acm.org; John Garry; xuwei (O)
> Subject: Re: [PATCH v10 2/9] PCI: remove unused __weak attribute in
> pci_register_io_range()
>
> On Fri, Oct 27, 2017 at 05:11:20PM +0100, Gabriele Paoloni wrote:
> > From: gabriele paoloni
> >
> > Currently pci_register_io_range() has only one definition;
> > therefore there is no use of the __weak attribute.
> >
> > Signed-off-by: Gabriele Paoloni
> > Acked-by: Bjorn Helgaas
>
> Also, I normally use this (Google address instead of kernel.org):
>
> Acked-by: Bjorn Helgaas
>
> (Also affects your other PCI patches.)
Ok no probs, we'll change all the Acked-by in the next patchset
Many Thanks
Gab
>
> > ---
> > drivers/pci/pci.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index af0cc34..eee967c 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3270,7 +3270,7 @@ static DEFINE_SPINLOCK(io_range_lock);
> > * Record the PCI IO range (expressed as CPU physical address +
> size).
> > * Return a negative value if an error has occured, zero otherwise
> > */
> > -int __weak pci_register_io_range(phys_addr_t addr, resource_size_t
> size)
> > +int pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > {
> > int err = 0;
> >
> > --
> > 2.7.4
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi"
> in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v10 9/9] MANTAINERS: Add maintainer for HiSilicon LPC driver
Hi Bjorn Many Thanks for reviewing > -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: 07 November 2017 00:21 > To: Gabriele Paoloni > Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org; > a...@arndb.de; linux-arm-ker...@lists.infradead.org; > lorenzo.pieral...@arm.com; mark.rutl...@arm.com; brian.star...@arm.com; > o...@lixom.net; b...@kernel.crashing.org; linux-kernel@vger.kernel.org; > linux-a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org; > miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v10 9/9] MANTAINERS: Add maintainer for HiSilicon > LPC driver > > s/MANTAINERS/MAINTAINERS/ Oops thanks > > On Fri, Oct 27, 2017 at 05:11:27PM +0100, Gabriele Paoloni wrote: > > Added maintainer for drivers/bus/hisi_lpc.c > > > > Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com> > > --- > > MAINTAINERS | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index f66488d..b49d4c0 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -6130,6 +6130,13 @@ F: include/uapi/linux/if_hippi.h > > F: net/802/hippi.c > > F: drivers/net/hippi/ > > > > +HISILICON LPC BUS DRIVER > > +L: linux...@huawei.com > > +W: http://www.hisilicon.com > > +S: Maintained > > +F: drivers/bus/hisi_lpc.c > > +F: Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low- > pin-count.txt > > Does this really count as a "maintainer" since it's only a mailing > list, not a real person? I personally prefer a specific person > because there's no accountability with a mailing list. Right; before the maintainer was meant to be me but as I am moving to a different role we decided to put down a mailing list. However after discussing internally we agreed to have a person as maintainer and we'll specify him in the next patchset. Cheers Gab > > > + > > HISILICON NETWORK SUBSYSTEM DRIVER > > M: Yisen Zhuang <yisen.zhu...@huawei.com> > > M: Salil Mehta <salil.me...@huawei.com> > > -- > > 2.7.4 > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-acpi" > in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v10 9/9] MANTAINERS: Add maintainer for HiSilicon LPC driver
Hi Bjorn Many Thanks for reviewing > -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: 07 November 2017 00:21 > To: Gabriele Paoloni > Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org; > a...@arndb.de; linux-arm-ker...@lists.infradead.org; > lorenzo.pieral...@arm.com; mark.rutl...@arm.com; brian.star...@arm.com; > o...@lixom.net; b...@kernel.crashing.org; linux-kernel@vger.kernel.org; > linux-a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org; > miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v10 9/9] MANTAINERS: Add maintainer for HiSilicon > LPC driver > > s/MANTAINERS/MAINTAINERS/ Oops thanks > > On Fri, Oct 27, 2017 at 05:11:27PM +0100, Gabriele Paoloni wrote: > > Added maintainer for drivers/bus/hisi_lpc.c > > > > Signed-off-by: Gabriele Paoloni > > --- > > MAINTAINERS | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index f66488d..b49d4c0 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -6130,6 +6130,13 @@ F: include/uapi/linux/if_hippi.h > > F: net/802/hippi.c > > F: drivers/net/hippi/ > > > > +HISILICON LPC BUS DRIVER > > +L: linux...@huawei.com > > +W: http://www.hisilicon.com > > +S: Maintained > > +F: drivers/bus/hisi_lpc.c > > +F: Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low- > pin-count.txt > > Does this really count as a "maintainer" since it's only a mailing > list, not a real person? I personally prefer a specific person > because there's no accountability with a mailing list. Right; before the maintainer was meant to be me but as I am moving to a different role we decided to put down a mailing list. However after discussing internally we agreed to have a person as maintainer and we'll specify him in the next patchset. Cheers Gab > > > + > > HISILICON NETWORK SUBSYSTEM DRIVER > > M: Yisen Zhuang > > M: Salil Mehta > > -- > > 2.7.4 > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-acpi" > in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings
Hi Randy > -Original Message- > From: Randy Dunlap [mailto:rdun...@infradead.org] > Sent: 27 October 2017 17:44 > To: Gabriele Paoloni; catalin.mari...@arm.com; will.dea...@arm.com; > robh...@kernel.org; frowand.l...@gmail.com; bhelg...@google.com; > raf...@kernel.org; a...@arndb.de; linux-arm-ker...@lists.infradead.org; > lorenzo.pieral...@arm.com > Cc: mark.rutl...@arm.com; brian.star...@arm.com; o...@lixom.net; > b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux- > a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org; > miny...@acm.org; John Garry; xuwei (O); zhichang.yuan > Subject: Re: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 > with DT bindings > > On 10/27/17 09:11, Gabriele Paoloni wrote: > > diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig > > index 2408ea3..358eed3 100644 > > --- a/drivers/bus/Kconfig > > +++ b/drivers/bus/Kconfig > > @@ -64,6 +64,15 @@ config BRCMSTB_GISB_ARB > > arbiter. This driver provides timeout and target abort error > handling > > and internal bus master decoding. > > > > +config HISILICON_LPC > > + bool "Support for ISA I/O space on Hisilicon Hip0X" > > + depends on (ARM64 && (ARCH_HISI || COMPILE_TEST)) > > + select LOGIC_PIO > > + select INDIRECT_PIO > > + help > > + Driver needed for some legacy ISA devices attached to Low-Pin- > Count > > + on Hisilicon Hip0X SoC. > > + > > config IMX_WEIM > > bool "Freescale EIM DRIVER" > > depends on ARCH_MXC > > Hi, > > Why bool? why not tristate? Well for the nature of our HW it would not make much sense to have the LPC modular. Also you can see in patch 8 the LPC host is "translating" the resources of its children before these are actually probed and this is done by acpi_indirectio_scan_init() as part of the ACPI init process. Thanks Gab > > -- > ~Randy
RE: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings
Hi Randy > -Original Message- > From: Randy Dunlap [mailto:rdun...@infradead.org] > Sent: 27 October 2017 17:44 > To: Gabriele Paoloni; catalin.mari...@arm.com; will.dea...@arm.com; > robh...@kernel.org; frowand.l...@gmail.com; bhelg...@google.com; > raf...@kernel.org; a...@arndb.de; linux-arm-ker...@lists.infradead.org; > lorenzo.pieral...@arm.com > Cc: mark.rutl...@arm.com; brian.star...@arm.com; o...@lixom.net; > b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux- > a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org; > miny...@acm.org; John Garry; xuwei (O); zhichang.yuan > Subject: Re: [PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 > with DT bindings > > On 10/27/17 09:11, Gabriele Paoloni wrote: > > diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig > > index 2408ea3..358eed3 100644 > > --- a/drivers/bus/Kconfig > > +++ b/drivers/bus/Kconfig > > @@ -64,6 +64,15 @@ config BRCMSTB_GISB_ARB > > arbiter. This driver provides timeout and target abort error > handling > > and internal bus master decoding. > > > > +config HISILICON_LPC > > + bool "Support for ISA I/O space on Hisilicon Hip0X" > > + depends on (ARM64 && (ARCH_HISI || COMPILE_TEST)) > > + select LOGIC_PIO > > + select INDIRECT_PIO > > + help > > + Driver needed for some legacy ISA devices attached to Low-Pin- > Count > > + on Hisilicon Hip0X SoC. > > + > > config IMX_WEIM > > bool "Freescale EIM DRIVER" > > depends on ARCH_MXC > > Hi, > > Why bool? why not tristate? Well for the nature of our HW it would not make much sense to have the LPC modular. Also you can see in patch 8 the LPC host is "translating" the resources of its children before these are actually probed and this is done by acpi_indirectio_scan_init() as part of the ACPI init process. Thanks Gab > > -- > ~Randy
RE: [PATCH v10 1/9] LIB: Introduce a generic PIO mapping method
Hi Corey Many Thanks for your comments [...] > > #define IO_SPACE_LIMIT 0x > > #endif > > > > +#include > > This whole thing would be a lot simpler if you had: > > #ifdef CONFIG_INDIRECT_PIO > #define inb logic_inb > #define outb logic outb > . > . > #endif /* CONFIG_INDIRECT_PIO */ > > and let the "ifndef XXX" below handle not enabling the standard code. > You could even put that in logic_pio.h to avoid polluting io.h. Agreed. I'll move it into "logic_pio.h" > > You might have to add "#ifndef inb", etc. above, but I still think it > would > be better. Yes agreed also on adding the "#ifndef ***" around each function re-definition in logic_pio.h This will be fixed in v11 > > I'm not sure if this wouldn't be better done in arm64/include/asm/io.h, > though. > A specific machine may want to only do this in ranges, for instance. No I don’t think so...this io functions re-0definition has nothing to do with ARM architecture and I think it is better to keep everything in logic_pio.h including the file from "include/asm-generic/io.h" Cheers Gab > > -corey [...] > > --- a/lib/Kconfig > > +++ b/lib/Kconfig > > @@ -59,6 +59,32 @@ config ARCH_USE_CMPXCHG_LOCKREF > > config ARCH_HAS_FAST_MULTIPLIER > > bool > > > > +config LOGIC_PIO > > + bool "Generic logical I/O management" > > + def_bool y if PCI && !X86 && !IA64 && !POWERPC > > + help > > + For some architectures, there are no IO space. To support the > > + accesses to legacy I/O devices on those architectures, kernel > > + implemented the memory mapped I/O mechanism based on bridge bus > > + supports. But for some buses which do not support MMIO, the > > + peripherals there should be accessed with device-specific way. > > + To abstract those different I/O accesses into unified I/O > accessors, > > + this option provide a generic I/O space management way after > mapping > > + the device I/O to system logical/fake I/O and help to hide all > the > > + hardware detail. > > + > > +config INDIRECT_PIO > > + bool "Access I/O in non-MMIO mode" if LOGIC_PIO > > + help > > + On some platforms where no separate I/O space exist, there are > I/O > > + hosts which can not be accessed in MMIO mode. Based on > LOGIC_PIO > > + mechanism, the host-local I/O resource can be mapped into > system > > + logic PIO space shared with MMIO hosts, such as PCI/PCIE, then > system > > + can access the I/O devices with the mapped logic PIO through > I/O > > + accessors. > > + This way has a little I/O performance cost. Please make sure > your > > + devices really need this configure item enabled. > > + > > If this is always available on the hisilicon chips, I think you would > want to just always > enable this on those chips. In patch 6/9 we have +config HISILICON_LPC + bool "Support for ISA I/O space on Hisilicon Hip0X" + depends on (ARM64 && (ARCH_HISI || COMPILE_TEST)) + select LOGIC_PIO + select INDIRECT_PIO So the LPC host controller driver is selecting INDIRECT_PIO... > > -corey > Cheers Gab
RE: [PATCH v10 1/9] LIB: Introduce a generic PIO mapping method
Hi Corey Many Thanks for your comments [...] > > #define IO_SPACE_LIMIT 0x > > #endif > > > > +#include > > This whole thing would be a lot simpler if you had: > > #ifdef CONFIG_INDIRECT_PIO > #define inb logic_inb > #define outb logic outb > . > . > #endif /* CONFIG_INDIRECT_PIO */ > > and let the "ifndef XXX" below handle not enabling the standard code. > You could even put that in logic_pio.h to avoid polluting io.h. Agreed. I'll move it into "logic_pio.h" > > You might have to add "#ifndef inb", etc. above, but I still think it > would > be better. Yes agreed also on adding the "#ifndef ***" around each function re-definition in logic_pio.h This will be fixed in v11 > > I'm not sure if this wouldn't be better done in arm64/include/asm/io.h, > though. > A specific machine may want to only do this in ranges, for instance. No I don’t think so...this io functions re-0definition has nothing to do with ARM architecture and I think it is better to keep everything in logic_pio.h including the file from "include/asm-generic/io.h" Cheers Gab > > -corey [...] > > --- a/lib/Kconfig > > +++ b/lib/Kconfig > > @@ -59,6 +59,32 @@ config ARCH_USE_CMPXCHG_LOCKREF > > config ARCH_HAS_FAST_MULTIPLIER > > bool > > > > +config LOGIC_PIO > > + bool "Generic logical I/O management" > > + def_bool y if PCI && !X86 && !IA64 && !POWERPC > > + help > > + For some architectures, there are no IO space. To support the > > + accesses to legacy I/O devices on those architectures, kernel > > + implemented the memory mapped I/O mechanism based on bridge bus > > + supports. But for some buses which do not support MMIO, the > > + peripherals there should be accessed with device-specific way. > > + To abstract those different I/O accesses into unified I/O > accessors, > > + this option provide a generic I/O space management way after > mapping > > + the device I/O to system logical/fake I/O and help to hide all > the > > + hardware detail. > > + > > +config INDIRECT_PIO > > + bool "Access I/O in non-MMIO mode" if LOGIC_PIO > > + help > > + On some platforms where no separate I/O space exist, there are > I/O > > + hosts which can not be accessed in MMIO mode. Based on > LOGIC_PIO > > + mechanism, the host-local I/O resource can be mapped into > system > > + logic PIO space shared with MMIO hosts, such as PCI/PCIE, then > system > > + can access the I/O devices with the mapped logic PIO through > I/O > > + accessors. > > + This way has a little I/O performance cost. Please make sure > your > > + devices really need this configure item enabled. > > + > > If this is always available on the hisilicon chips, I think you would > want to just always > enable this on those chips. In patch 6/9 we have +config HISILICON_LPC + bool "Support for ISA I/O space on Hisilicon Hip0X" + depends on (ARM64 && (ARCH_HISI || COMPILE_TEST)) + select LOGIC_PIO + select INDIRECT_PIO So the LPC host controller driver is selecting INDIRECT_PIO... > > -corey > Cheers Gab
RE: [PATCH v10 0/9] LPC: legacy ISA I/O support
Hi David [...] > FWIW my thoughts on this are WTF! > > Looks to me horribly over complicated and over generalised. > > Surely is it could be done the same way that x86 does IO cycles? No > So you encode the information into the 'address' the driver passes > to ioread16() (etc) to allow it to do either a normal bus cycle or > the indirect cycle onto the external bus. In order to do that you need to have a special PCI bridge that is able to detect the special IO addresses and initiate such special IO cycles on the external bus. This is not supported by our HW (and this why we need the LPC accessors) Gab > > So you have one kernel option that makes these real functions. > > David
RE: [PATCH v10 0/9] LPC: legacy ISA I/O support
Hi David [...] > FWIW my thoughts on this are WTF! > > Looks to me horribly over complicated and over generalised. > > Surely is it could be done the same way that x86 does IO cycles? No > So you encode the information into the 'address' the driver passes > to ioread16() (etc) to allow it to do either a normal bus cycle or > the indirect cycle onto the external bus. In order to do that you need to have a special PCI bridge that is able to detect the special IO addresses and initiate such special IO cycles on the external bus. This is not supported by our HW (and this why we need the LPC accessors) Gab > > So you have one kernel option that makes these real functions. > > David
[PATCH v10 9/9] MANTAINERS: Add maintainer for HiSilicon LPC driver
Added maintainer for drivers/bus/hisi_lpc.c Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com> --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index f66488d..b49d4c0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6130,6 +6130,13 @@ F: include/uapi/linux/if_hippi.h F: net/802/hippi.c F: drivers/net/hippi/ +HISILICON LPC BUS DRIVER +L: linux...@huawei.com +W: http://www.hisilicon.com +S: Maintained +F: drivers/bus/hisi_lpc.c +F: Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt + HISILICON NETWORK SUBSYSTEM DRIVER M: Yisen Zhuang <yisen.zhu...@huawei.com> M: Salil Mehta <salil.me...@huawei.com> -- 2.7.4
[PATCH v10 9/9] MANTAINERS: Add maintainer for HiSilicon LPC driver
Added maintainer for drivers/bus/hisi_lpc.c Signed-off-by: Gabriele Paoloni --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index f66488d..b49d4c0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6130,6 +6130,13 @@ F: include/uapi/linux/if_hippi.h F: net/802/hippi.c F: drivers/net/hippi/ +HISILICON LPC BUS DRIVER +L: linux...@huawei.com +W: http://www.hisilicon.com +S: Maintained +F: drivers/bus/hisi_lpc.c +F: Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt + HISILICON NETWORK SUBSYSTEM DRIVER M: Yisen Zhuang M: Salil Mehta -- 2.7.4
[PATCH v10 3/9] PCI: add fwnode handler as input param of pci_register_io_range()
From: gabriele paoloni <gabriele.paol...@huawei.com>
In preparation for having the PCI MMIO helpers to use the new generic
I/O space management(LOGIC_PIO) we need to add the fwnode handler as
extra input parameter.
This patch changes the signature of pci_register_io_range() and of
its callers as needed.
Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
Acked-by: Bjorn Helgaas <helg...@kernel.org>
---
drivers/acpi/pci_root.c | 8 +---
drivers/of/address.c| 4 +++-
drivers/pci/pci.c | 3 ++-
include/linux/pci.h | 3 ++-
4 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 9eec309..40164e3 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -729,7 +729,8 @@ static void acpi_pci_root_validate_resources(struct device
*dev,
}
}
-static void acpi_pci_root_remap_iospace(struct resource_entry *entry)
+static void acpi_pci_root_remap_iospace(struct fwnode_handle *fwnode,
+ struct resource_entry *entry)
{
#ifdef PCI_IOBASE
struct resource *res = entry->res;
@@ -738,7 +739,7 @@ static void acpi_pci_root_remap_iospace(struct
resource_entry *entry)
resource_size_t length = resource_size(res);
unsigned long port;
- if (pci_register_io_range(cpu_addr, length))
+ if (pci_register_io_range(fwnode, cpu_addr, length))
goto err;
port = pci_address_to_pio(cpu_addr);
@@ -780,7 +781,8 @@ int acpi_pci_probe_root_resources(struct acpi_pci_root_info
*info)
else {
resource_list_for_each_entry_safe(entry, tmp, list) {
if (entry->res->flags & IORESOURCE_IO)
- acpi_pci_root_remap_iospace(entry);
+ acpi_pci_root_remap_iospace(>fwnode,
+ entry);
if (entry->res->flags & IORESOURCE_DISABLED)
resource_list_destroy_entry(entry);
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 580bbf6..9d3ba07 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -2,6 +2,7 @@
#define pr_fmt(fmt)"OF: " fmt
#include
+#include
#include
#include
#include
@@ -323,7 +324,8 @@ int of_pci_range_to_resource(struct of_pci_range *range,
if (res->flags & IORESOURCE_IO) {
unsigned long port;
- err = pci_register_io_range(range->cpu_addr, range->size);
+ err = pci_register_io_range(>fwnode, range->cpu_addr,
+ range->size);
if (err)
goto invalid_range;
port = pci_address_to_pio(range->cpu_addr);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index eee967c..bcecb47 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3270,7 +3270,8 @@ static DEFINE_SPINLOCK(io_range_lock);
* Record the PCI IO range (expressed as CPU physical address + size).
* Return a negative value if an error has occured, zero otherwise
*/
-int pci_register_io_range(phys_addr_t addr, resource_size_t size)
+int pci_register_io_range(struct fwnode_handle *fwnode, phys_addr_t addr,
+ resource_size_t size)
{
int err = 0;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4869e66..1d958d7 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1205,7 +1205,8 @@ int __must_check pci_bus_alloc_resource(struct pci_bus
*bus,
void *alignf_data);
-int pci_register_io_range(phys_addr_t addr, resource_size_t size);
+int pci_register_io_range(struct fwnode_handle *fwnode, phys_addr_t addr,
+ resource_size_t size);
unsigned long pci_address_to_pio(phys_addr_t addr);
phys_addr_t pci_pio_to_address(unsigned long pio);
int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr);
--
2.7.4
[PATCH v10 3/9] PCI: add fwnode handler as input param of pci_register_io_range()
From: gabriele paoloni
In preparation for having the PCI MMIO helpers to use the new generic
I/O space management(LOGIC_PIO) we need to add the fwnode handler as
extra input parameter.
This patch changes the signature of pci_register_io_range() and of
its callers as needed.
Signed-off-by: Gabriele Paoloni
Acked-by: Bjorn Helgaas
---
drivers/acpi/pci_root.c | 8 +---
drivers/of/address.c| 4 +++-
drivers/pci/pci.c | 3 ++-
include/linux/pci.h | 3 ++-
4 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 9eec309..40164e3 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -729,7 +729,8 @@ static void acpi_pci_root_validate_resources(struct device
*dev,
}
}
-static void acpi_pci_root_remap_iospace(struct resource_entry *entry)
+static void acpi_pci_root_remap_iospace(struct fwnode_handle *fwnode,
+ struct resource_entry *entry)
{
#ifdef PCI_IOBASE
struct resource *res = entry->res;
@@ -738,7 +739,7 @@ static void acpi_pci_root_remap_iospace(struct
resource_entry *entry)
resource_size_t length = resource_size(res);
unsigned long port;
- if (pci_register_io_range(cpu_addr, length))
+ if (pci_register_io_range(fwnode, cpu_addr, length))
goto err;
port = pci_address_to_pio(cpu_addr);
@@ -780,7 +781,8 @@ int acpi_pci_probe_root_resources(struct acpi_pci_root_info
*info)
else {
resource_list_for_each_entry_safe(entry, tmp, list) {
if (entry->res->flags & IORESOURCE_IO)
- acpi_pci_root_remap_iospace(entry);
+ acpi_pci_root_remap_iospace(>fwnode,
+ entry);
if (entry->res->flags & IORESOURCE_DISABLED)
resource_list_destroy_entry(entry);
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 580bbf6..9d3ba07 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -2,6 +2,7 @@
#define pr_fmt(fmt)"OF: " fmt
#include
+#include
#include
#include
#include
@@ -323,7 +324,8 @@ int of_pci_range_to_resource(struct of_pci_range *range,
if (res->flags & IORESOURCE_IO) {
unsigned long port;
- err = pci_register_io_range(range->cpu_addr, range->size);
+ err = pci_register_io_range(>fwnode, range->cpu_addr,
+ range->size);
if (err)
goto invalid_range;
port = pci_address_to_pio(range->cpu_addr);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index eee967c..bcecb47 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3270,7 +3270,8 @@ static DEFINE_SPINLOCK(io_range_lock);
* Record the PCI IO range (expressed as CPU physical address + size).
* Return a negative value if an error has occured, zero otherwise
*/
-int pci_register_io_range(phys_addr_t addr, resource_size_t size)
+int pci_register_io_range(struct fwnode_handle *fwnode, phys_addr_t addr,
+ resource_size_t size)
{
int err = 0;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4869e66..1d958d7 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1205,7 +1205,8 @@ int __must_check pci_bus_alloc_resource(struct pci_bus
*bus,
void *alignf_data);
-int pci_register_io_range(phys_addr_t addr, resource_size_t size);
+int pci_register_io_range(struct fwnode_handle *fwnode, phys_addr_t addr,
+ resource_size_t size);
unsigned long pci_address_to_pio(phys_addr_t addr);
phys_addr_t pci_pio_to_address(unsigned long pio);
int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr);
--
2.7.4
[PATCH v10 2/9] PCI: remove unused __weak attribute in pci_register_io_range()
From: gabriele paoloni <gabriele.paol...@huawei.com>
Currently pci_register_io_range() has only one definition;
therefore there is no use of the __weak attribute.
Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
Acked-by: Bjorn Helgaas <helg...@kernel.org>
---
drivers/pci/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index af0cc34..eee967c 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3270,7 +3270,7 @@ static DEFINE_SPINLOCK(io_range_lock);
* Record the PCI IO range (expressed as CPU physical address + size).
* Return a negative value if an error has occured, zero otherwise
*/
-int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
+int pci_register_io_range(phys_addr_t addr, resource_size_t size)
{
int err = 0;
--
2.7.4
[PATCH v10 2/9] PCI: remove unused __weak attribute in pci_register_io_range()
From: gabriele paoloni
Currently pci_register_io_range() has only one definition;
therefore there is no use of the __weak attribute.
Signed-off-by: Gabriele Paoloni
Acked-by: Bjorn Helgaas
---
drivers/pci/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index af0cc34..eee967c 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3270,7 +3270,7 @@ static DEFINE_SPINLOCK(io_range_lock);
* Record the PCI IO range (expressed as CPU physical address + size).
* Return a negative value if an error has occured, zero otherwise
*/
-int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
+int pci_register_io_range(phys_addr_t addr, resource_size_t size)
{
int err = 0;
--
2.7.4
[PATCH v10 8/9] LPC: Add the ACPI LPC support
From: "zhichang.yuan" <yuanzhich...@hisilicon.com>
Based on the provious patches, this patch supports the ACPI LPC host on
Hip06/Hip07.
Signed-off-by: zhichang.yuan <yuanzhich...@hisilicon.com>
Signed-off-by: John Garry <john.ga...@huawei.com>
Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
Tested-by: dann frazier <dann.fraz...@canonical.com>
---
drivers/acpi/arm64/acpi_indirectio.c | 3 +
drivers/acpi/arm64/acpi_indirectio.h | 4 +
drivers/bus/hisi_lpc.c | 164 +++
3 files changed, 154 insertions(+), 17 deletions(-)
diff --git a/drivers/acpi/arm64/acpi_indirectio.c
b/drivers/acpi/arm64/acpi_indirectio.c
index 013db6f..424f3bd 100644
--- a/drivers/acpi/arm64/acpi_indirectio.c
+++ b/drivers/acpi/arm64/acpi_indirectio.c
@@ -123,6 +123,9 @@ int acpi_set_logicio_resource(struct device *child,
/* All the host devices which apply indirect-IO can be listed here. */
static const struct acpi_device_id acpi_indirect_host_id[] = {
+#ifdef CONFIG_HISILICON_LPC
+ {"HISI0191", INDIRECT_IO_INFO(lpc_host_desc)},
+#endif
{""},
};
diff --git a/drivers/acpi/arm64/acpi_indirectio.h
b/drivers/acpi/arm64/acpi_indirectio.h
index 8102ea0..1b18590 100644
--- a/drivers/acpi/arm64/acpi_indirectio.h
+++ b/drivers/acpi/arm64/acpi_indirectio.h
@@ -18,6 +18,10 @@ struct indirectio_device_desc {
int (*pre_setup)(struct acpi_device *adev, void *pdata);
};
+#ifdef CONFIG_HISILICON_LPC
+extern const struct indirectio_device_desc lpc_host_desc;
+#endif
+
int acpi_set_logicio_resource(struct device *child,
struct device *hostdev, const struct resource **res,
int *num_res);
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index c885483..26043f7 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -438,7 +438,6 @@ static int hisilpc_probe(struct platform_device *pdev)
struct device *dev = >dev;
struct resource *res;
struct hisilpc_dev *lpcdev;
- struct logic_pio_hwaddr *range;
int ret = 0;
lpcdev = devm_kzalloc(dev, sizeof(struct hisilpc_dev), GFP_KERNEL);
@@ -460,21 +459,32 @@ static int hisilpc_probe(struct platform_device *pdev)
}
/* register the LPC host PIO resources */
- range = devm_kzalloc(dev, sizeof(*range), GFP_KERNEL);
- if (!range)
- return -ENOMEM;
- range->fwnode = dev->fwnode;
- range->flags = PIO_INDIRECT;
- range->size = LPC_BUS_IO_SIZE;
- range->hw_start = LPC_MIN_BUS_RANGE;
-
- ret = logic_pio_register_range(range);
- if (ret) {
- kfree(range);
- dev_err(dev, "OF: register IO range FAIL!\n");
- return -ret;
+ if (has_acpi_companion(dev)) {
+ lpcdev->io_host = find_io_range_by_fwnode(dev->fwnode);
+ if (!lpcdev->io_host) {
+ dev_err(dev, "HiSilicon LPC range not registered!\n");
+ return -EFAULT;
+ }
+ } else {
+ struct logic_pio_hwaddr *range;
+
+ range = devm_kzalloc(dev, sizeof(*range), GFP_KERNEL);
+ if (!range)
+ return -ENOMEM;
+ range->fwnode = dev->fwnode;
+ range->flags = PIO_INDIRECT;
+ range->size = LPC_BUS_IO_SIZE;
+ range->hw_start = LPC_MIN_BUS_RANGE;
+
+ ret = logic_pio_register_range(range);
+ if (ret) {
+ kfree(range);
+ dev_err(dev, "OF: register IO range FAIL!\n");
+ return -ret;
+ }
+ lpcdev->io_host = range;
}
- lpcdev->io_host = range;
+
lpcdev->io_host->devpara = lpcdev;
lpcdev->io_host->ops = _lpc_ops;
@@ -486,8 +496,11 @@ static int hisilpc_probe(struct platform_device *pdev)
* to avoid some children which complete the scanning trigger the
* MMIO accesses which will probably cause panic.
*/
- dev_info(dev, " calling of_platform_populate");
- ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
+ if (!has_acpi_companion(dev)) {
+ dev_info(dev, " calling of_platform_populate");
+ ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
+ }
+
if (ret) {
/*
* When LPC probing is not completely successful, set 'devpara'
@@ -515,10 +528,127 @@ static const struct of_device_id hisilpc_of_match[] = {
{},
};
+#ifdef CONFIG_ACPI
+#include "../acpi/arm64/acpi_indirectio.h"
+#include
+
+struct lpc_private_data {
+ resource_size_t io_size;
+ resource_size_t io_start;
+};
+
+static struct lpc
[PATCH v10 8/9] LPC: Add the ACPI LPC support
From: "zhichang.yuan"
Based on the provious patches, this patch supports the ACPI LPC host on
Hip06/Hip07.
Signed-off-by: zhichang.yuan
Signed-off-by: John Garry
Signed-off-by: Gabriele Paoloni
Tested-by: dann frazier
---
drivers/acpi/arm64/acpi_indirectio.c | 3 +
drivers/acpi/arm64/acpi_indirectio.h | 4 +
drivers/bus/hisi_lpc.c | 164 +++
3 files changed, 154 insertions(+), 17 deletions(-)
diff --git a/drivers/acpi/arm64/acpi_indirectio.c
b/drivers/acpi/arm64/acpi_indirectio.c
index 013db6f..424f3bd 100644
--- a/drivers/acpi/arm64/acpi_indirectio.c
+++ b/drivers/acpi/arm64/acpi_indirectio.c
@@ -123,6 +123,9 @@ int acpi_set_logicio_resource(struct device *child,
/* All the host devices which apply indirect-IO can be listed here. */
static const struct acpi_device_id acpi_indirect_host_id[] = {
+#ifdef CONFIG_HISILICON_LPC
+ {"HISI0191", INDIRECT_IO_INFO(lpc_host_desc)},
+#endif
{""},
};
diff --git a/drivers/acpi/arm64/acpi_indirectio.h
b/drivers/acpi/arm64/acpi_indirectio.h
index 8102ea0..1b18590 100644
--- a/drivers/acpi/arm64/acpi_indirectio.h
+++ b/drivers/acpi/arm64/acpi_indirectio.h
@@ -18,6 +18,10 @@ struct indirectio_device_desc {
int (*pre_setup)(struct acpi_device *adev, void *pdata);
};
+#ifdef CONFIG_HISILICON_LPC
+extern const struct indirectio_device_desc lpc_host_desc;
+#endif
+
int acpi_set_logicio_resource(struct device *child,
struct device *hostdev, const struct resource **res,
int *num_res);
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index c885483..26043f7 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -438,7 +438,6 @@ static int hisilpc_probe(struct platform_device *pdev)
struct device *dev = >dev;
struct resource *res;
struct hisilpc_dev *lpcdev;
- struct logic_pio_hwaddr *range;
int ret = 0;
lpcdev = devm_kzalloc(dev, sizeof(struct hisilpc_dev), GFP_KERNEL);
@@ -460,21 +459,32 @@ static int hisilpc_probe(struct platform_device *pdev)
}
/* register the LPC host PIO resources */
- range = devm_kzalloc(dev, sizeof(*range), GFP_KERNEL);
- if (!range)
- return -ENOMEM;
- range->fwnode = dev->fwnode;
- range->flags = PIO_INDIRECT;
- range->size = LPC_BUS_IO_SIZE;
- range->hw_start = LPC_MIN_BUS_RANGE;
-
- ret = logic_pio_register_range(range);
- if (ret) {
- kfree(range);
- dev_err(dev, "OF: register IO range FAIL!\n");
- return -ret;
+ if (has_acpi_companion(dev)) {
+ lpcdev->io_host = find_io_range_by_fwnode(dev->fwnode);
+ if (!lpcdev->io_host) {
+ dev_err(dev, "HiSilicon LPC range not registered!\n");
+ return -EFAULT;
+ }
+ } else {
+ struct logic_pio_hwaddr *range;
+
+ range = devm_kzalloc(dev, sizeof(*range), GFP_KERNEL);
+ if (!range)
+ return -ENOMEM;
+ range->fwnode = dev->fwnode;
+ range->flags = PIO_INDIRECT;
+ range->size = LPC_BUS_IO_SIZE;
+ range->hw_start = LPC_MIN_BUS_RANGE;
+
+ ret = logic_pio_register_range(range);
+ if (ret) {
+ kfree(range);
+ dev_err(dev, "OF: register IO range FAIL!\n");
+ return -ret;
+ }
+ lpcdev->io_host = range;
}
- lpcdev->io_host = range;
+
lpcdev->io_host->devpara = lpcdev;
lpcdev->io_host->ops = _lpc_ops;
@@ -486,8 +496,11 @@ static int hisilpc_probe(struct platform_device *pdev)
* to avoid some children which complete the scanning trigger the
* MMIO accesses which will probably cause panic.
*/
- dev_info(dev, " calling of_platform_populate");
- ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
+ if (!has_acpi_companion(dev)) {
+ dev_info(dev, " calling of_platform_populate");
+ ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
+ }
+
if (ret) {
/*
* When LPC probing is not completely successful, set 'devpara'
@@ -515,10 +528,127 @@ static const struct of_device_id hisilpc_of_match[] = {
{},
};
+#ifdef CONFIG_ACPI
+#include "../acpi/arm64/acpi_indirectio.h"
+#include
+
+struct lpc_private_data {
+ resource_size_t io_size;
+ resource_size_t io_start;
+};
+
+static struct lpc_private_data lpc_data = {
+ .io_size = LPC_BUS_IO_SIZE,
+ .io_start = LPC_MIN_BUS_RANGE,
+};
+
+static struct mfd_cell_acpi_match hisi_lpc_ipmi_acpi_ma
[PATCH v10 4/9] PCI: Apply the new generic I/O management on PCI IO hosts
From: gabriele paoloni <gabriele.paol...@huawei.com>
After introducing the new generic I/O space management(LOGIC_PIO), the
original PCI MMIO relevant helpers need to be updated based on the new
interfaces defined in LOGIC_PIO.
This patch adapts the corresponding code to match the changes introduced
by LOGIC_PIO.
Signed-off-by: zhichang.yuan <yuanzhich...@hisilicon.com>
Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
Signed-off-by: Arnd Bergmann <a...@arndb.de>#earlier draft
Acked-by: Bjorn Helgaas <helg...@kernel.org>
---
drivers/pci/pci.c| 95 +---
include/asm-generic/io.h | 2 +-
2 files changed, 18 insertions(+), 79 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index bcecb47..a1ae4d3 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -3255,17 +3256,6 @@ int pci_request_regions_exclusive(struct pci_dev *pdev,
const char *res_name)
}
EXPORT_SYMBOL(pci_request_regions_exclusive);
-#ifdef PCI_IOBASE
-struct io_range {
- struct list_head list;
- phys_addr_t start;
- resource_size_t size;
-};
-
-static LIST_HEAD(io_range_list);
-static DEFINE_SPINLOCK(io_range_lock);
-#endif
-
/*
* Record the PCI IO range (expressed as CPU physical address + size).
* Return a negative value if an error has occured, zero otherwise
@@ -3273,51 +3263,28 @@ static DEFINE_SPINLOCK(io_range_lock);
int pci_register_io_range(struct fwnode_handle *fwnode, phys_addr_t addr,
resource_size_t size)
{
- int err = 0;
-
+ int ret = 0;
#ifdef PCI_IOBASE
- struct io_range *range;
- resource_size_t allocated_size = 0;
-
- /* check if the range hasn't been previously recorded */
- spin_lock(_range_lock);
- list_for_each_entry(range, _range_list, list) {
- if (addr >= range->start && addr + size <= range->start + size)
{
- /* range already registered, bail out */
- goto end_register;
- }
- allocated_size += range->size;
- }
+ struct logic_pio_hwaddr *range;
- /* range not registed yet, check for available space */
- if (allocated_size + size - 1 > IO_SPACE_LIMIT) {
- /* if it's too big check if 64K space can be reserved */
- if (allocated_size + SZ_64K - 1 > IO_SPACE_LIMIT) {
- err = -E2BIG;
- goto end_register;
- }
-
- size = SZ_64K;
- pr_warn("Requested IO range too big, new size set to 64K\n");
- }
+ if (!size || addr + size < addr)
+ return -EINVAL;
- /* add the range to the list */
range = kzalloc(sizeof(*range), GFP_ATOMIC);
- if (!range) {
- err = -ENOMEM;
- goto end_register;
- }
+ if (!range)
+ return -ENOMEM;
- range->start = addr;
+ range->fwnode = fwnode;
range->size = size;
+ range->hw_start = addr;
+ range->flags = PIO_CPU_MMIO;
- list_add_tail(>list, _range_list);
-
-end_register:
- spin_unlock(_range_lock);
+ ret = logic_pio_register_range(range);
+ if (ret)
+ kfree(range);
#endif
- return err;
+ return ret;
}
phys_addr_t pci_pio_to_address(unsigned long pio)
@@ -3325,21 +3292,10 @@ phys_addr_t pci_pio_to_address(unsigned long pio)
phys_addr_t address = (phys_addr_t)OF_BAD_ADDR;
#ifdef PCI_IOBASE
- struct io_range *range;
- resource_size_t allocated_size = 0;
-
- if (pio > IO_SPACE_LIMIT)
+ if (pio >= MMIO_UPPER_LIMIT)
return address;
- spin_lock(_range_lock);
- list_for_each_entry(range, _range_list, list) {
- if (pio >= allocated_size && pio < allocated_size +
range->size) {
- address = range->start + pio - allocated_size;
- break;
- }
- allocated_size += range->size;
- }
- spin_unlock(_range_lock);
+ address = logic_pio_to_hwaddr(pio);
#endif
return address;
@@ -3348,25 +3304,8 @@ phys_addr_t pci_pio_to_address(unsigned long pio)
unsigned long __weak pci_address_to_pio(phys_addr_t address)
{
#ifdef PCI_IOBASE
- struct io_range *res;
- resource_size_t offset = 0;
- unsigned long addr = -1;
-
- spin_lock(_range_lock);
- list_for_each_entry(res, _range_list, list) {
- if (address >= res->start && address < res->start + res->size) {
- addr = address - res->start + offset;
- break;
- }
-
[PATCH v10 4/9] PCI: Apply the new generic I/O management on PCI IO hosts
From: gabriele paoloni
After introducing the new generic I/O space management(LOGIC_PIO), the
original PCI MMIO relevant helpers need to be updated based on the new
interfaces defined in LOGIC_PIO.
This patch adapts the corresponding code to match the changes introduced
by LOGIC_PIO.
Signed-off-by: zhichang.yuan
Signed-off-by: Gabriele Paoloni
Signed-off-by: Arnd Bergmann #earlier draft
Acked-by: Bjorn Helgaas
---
drivers/pci/pci.c| 95 +---
include/asm-generic/io.h | 2 +-
2 files changed, 18 insertions(+), 79 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index bcecb47..a1ae4d3 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -3255,17 +3256,6 @@ int pci_request_regions_exclusive(struct pci_dev *pdev,
const char *res_name)
}
EXPORT_SYMBOL(pci_request_regions_exclusive);
-#ifdef PCI_IOBASE
-struct io_range {
- struct list_head list;
- phys_addr_t start;
- resource_size_t size;
-};
-
-static LIST_HEAD(io_range_list);
-static DEFINE_SPINLOCK(io_range_lock);
-#endif
-
/*
* Record the PCI IO range (expressed as CPU physical address + size).
* Return a negative value if an error has occured, zero otherwise
@@ -3273,51 +3263,28 @@ static DEFINE_SPINLOCK(io_range_lock);
int pci_register_io_range(struct fwnode_handle *fwnode, phys_addr_t addr,
resource_size_t size)
{
- int err = 0;
-
+ int ret = 0;
#ifdef PCI_IOBASE
- struct io_range *range;
- resource_size_t allocated_size = 0;
-
- /* check if the range hasn't been previously recorded */
- spin_lock(_range_lock);
- list_for_each_entry(range, _range_list, list) {
- if (addr >= range->start && addr + size <= range->start + size)
{
- /* range already registered, bail out */
- goto end_register;
- }
- allocated_size += range->size;
- }
+ struct logic_pio_hwaddr *range;
- /* range not registed yet, check for available space */
- if (allocated_size + size - 1 > IO_SPACE_LIMIT) {
- /* if it's too big check if 64K space can be reserved */
- if (allocated_size + SZ_64K - 1 > IO_SPACE_LIMIT) {
- err = -E2BIG;
- goto end_register;
- }
-
- size = SZ_64K;
- pr_warn("Requested IO range too big, new size set to 64K\n");
- }
+ if (!size || addr + size < addr)
+ return -EINVAL;
- /* add the range to the list */
range = kzalloc(sizeof(*range), GFP_ATOMIC);
- if (!range) {
- err = -ENOMEM;
- goto end_register;
- }
+ if (!range)
+ return -ENOMEM;
- range->start = addr;
+ range->fwnode = fwnode;
range->size = size;
+ range->hw_start = addr;
+ range->flags = PIO_CPU_MMIO;
- list_add_tail(>list, _range_list);
-
-end_register:
- spin_unlock(_range_lock);
+ ret = logic_pio_register_range(range);
+ if (ret)
+ kfree(range);
#endif
- return err;
+ return ret;
}
phys_addr_t pci_pio_to_address(unsigned long pio)
@@ -3325,21 +3292,10 @@ phys_addr_t pci_pio_to_address(unsigned long pio)
phys_addr_t address = (phys_addr_t)OF_BAD_ADDR;
#ifdef PCI_IOBASE
- struct io_range *range;
- resource_size_t allocated_size = 0;
-
- if (pio > IO_SPACE_LIMIT)
+ if (pio >= MMIO_UPPER_LIMIT)
return address;
- spin_lock(_range_lock);
- list_for_each_entry(range, _range_list, list) {
- if (pio >= allocated_size && pio < allocated_size +
range->size) {
- address = range->start + pio - allocated_size;
- break;
- }
- allocated_size += range->size;
- }
- spin_unlock(_range_lock);
+ address = logic_pio_to_hwaddr(pio);
#endif
return address;
@@ -3348,25 +3304,8 @@ phys_addr_t pci_pio_to_address(unsigned long pio)
unsigned long __weak pci_address_to_pio(phys_addr_t address)
{
#ifdef PCI_IOBASE
- struct io_range *res;
- resource_size_t offset = 0;
- unsigned long addr = -1;
-
- spin_lock(_range_lock);
- list_for_each_entry(res, _range_list, list) {
- if (address >= res->start && address < res->start + res->size) {
- addr = address - res->start + offset;
- break;
- }
- offset += res->size;
- }
- spin_unlock(_range_lock);
-
- return addr;
+ return logic_pio_trans_cpuaddr(address);
#else
-
[PATCH v10 5/9] OF: Add missing I/O range exception for indirect-IO devices
From: "zhichang.yuan" <yuanzhich...@hisilicon.com>
There are some special ISA/LPC devices that work on a specific I/O range
where it is not correct to specify a 'ranges' property in DTS parent node
as cpu addresses translated from DTS node are only for memory space on
some architectures, such as Arm64. Without the parent 'ranges' property,
current of_translate_address() return an error.
Here we add special handlings for this case.
During the OF address translation, some checkings will be perfromed to
identify whether the device node is registered as indirect-IO. If yes,
the I/O translation will be done in a different way from that one of PCI
MMIO. In this way, the I/O 'reg' property of the special ISA/LPC devices
will be parsed correctly.
Signed-off-by: zhichang.yuan <yuanzhich...@hisilicon.com>
Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
Signed-off-by: Arnd Bergmann <a...@arndb.de>#earlier draft
Acked-by: Rob Herring <r...@kernel.org>
---
drivers/of/address.c | 91 +++-
1 file changed, 75 insertions(+), 16 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 9d3ba07..323ee56 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -551,9 +551,14 @@ static int of_translate_one(struct device_node *parent,
struct of_bus *bus,
* that translation is impossible (that is we are not dealing with a value
* that can be mapped to a cpu physical address). This is not really specified
* that way, but this is traditionally the way IBM at least do things
+ *
+ * Whenever the translation fails, the *host pointer will be set to the
+ * device that had registered logical PIO mapping, and the return code is
+ * relative to that node.
*/
static u64 __of_translate_address(struct device_node *dev,
- const __be32 *in_addr, const char *rprop)
+ const __be32 *in_addr, const char *rprop,
+ struct device_node **host)
{
struct device_node *parent = NULL;
struct of_bus *bus, *pbus;
@@ -566,6 +571,7 @@ static u64 __of_translate_address(struct device_node *dev,
/* Increase refcount at current level */
of_node_get(dev);
+ *host = NULL;
/* Get parent & match bus type */
parent = of_get_parent(dev);
if (parent == NULL)
@@ -586,6 +592,8 @@ static u64 __of_translate_address(struct device_node *dev,
/* Translate */
for (;;) {
+ struct logic_pio_hwaddr *iorange;
+
/* Switch to parent bus */
of_node_put(dev);
dev = parent;
@@ -598,6 +606,19 @@ static u64 __of_translate_address(struct device_node *dev,
break;
}
+ /*
+* For indirectIO device which has no ranges property, get
+* the address from reg directly.
+*/
+ iorange = find_io_range_by_fwnode(>fwnode);
+ if (iorange && (iorange->flags != PIO_CPU_MMIO)) {
+ result = of_read_number(addr + 1, na - 1);
+ pr_debug("indirectIO matched(%s) 0x%llx\n",
+ of_node_full_name(dev), result);
+ *host = of_node_get(dev);
+ break;
+ }
+
/* Get new parent bus and counts */
pbus = of_match_bus(parent);
pbus->count_cells(dev, , );
@@ -630,13 +651,32 @@ static u64 __of_translate_address(struct device_node *dev,
u64 of_translate_address(struct device_node *dev, const __be32 *in_addr)
{
- return __of_translate_address(dev, in_addr, "ranges");
+ struct device_node *host;
+ u64 ret;
+
+ ret = __of_translate_address(dev, in_addr, "ranges", );
+ if (host) {
+ of_node_put(host);
+ return OF_BAD_ADDR;
+ }
+
+ return ret;
}
EXPORT_SYMBOL(of_translate_address);
u64 of_translate_dma_address(struct device_node *dev, const __be32 *in_addr)
{
- return __of_translate_address(dev, in_addr, "dma-ranges");
+ struct device_node *host;
+ u64 ret;
+
+ ret = __of_translate_address(dev, in_addr, "dma-ranges", );
+
+ if (host) {
+ of_node_put(host);
+ return OF_BAD_ADDR;
+ }
+
+ return ret;
}
EXPORT_SYMBOL(of_translate_dma_address);
@@ -678,29 +718,48 @@ const __be32 *of_get_address(struct device_node *dev, int
index, u64 *size,
}
EXPORT_SYMBOL(of_get_address);
+static u64 of_translate_ioport(struct device_node *dev, const __be32 *in_addr,
+ u64 size)
+{
+ u64 taddr;
+ unsigned long port;
+ struct device_node *host;
+
+ taddr = __of_translate_address(dev,
[PATCH v10 5/9] OF: Add missing I/O range exception for indirect-IO devices
From: "zhichang.yuan"
There are some special ISA/LPC devices that work on a specific I/O range
where it is not correct to specify a 'ranges' property in DTS parent node
as cpu addresses translated from DTS node are only for memory space on
some architectures, such as Arm64. Without the parent 'ranges' property,
current of_translate_address() return an error.
Here we add special handlings for this case.
During the OF address translation, some checkings will be perfromed to
identify whether the device node is registered as indirect-IO. If yes,
the I/O translation will be done in a different way from that one of PCI
MMIO. In this way, the I/O 'reg' property of the special ISA/LPC devices
will be parsed correctly.
Signed-off-by: zhichang.yuan
Signed-off-by: Gabriele Paoloni
Signed-off-by: Arnd Bergmann #earlier draft
Acked-by: Rob Herring
---
drivers/of/address.c | 91 +++-
1 file changed, 75 insertions(+), 16 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 9d3ba07..323ee56 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -551,9 +551,14 @@ static int of_translate_one(struct device_node *parent,
struct of_bus *bus,
* that translation is impossible (that is we are not dealing with a value
* that can be mapped to a cpu physical address). This is not really specified
* that way, but this is traditionally the way IBM at least do things
+ *
+ * Whenever the translation fails, the *host pointer will be set to the
+ * device that had registered logical PIO mapping, and the return code is
+ * relative to that node.
*/
static u64 __of_translate_address(struct device_node *dev,
- const __be32 *in_addr, const char *rprop)
+ const __be32 *in_addr, const char *rprop,
+ struct device_node **host)
{
struct device_node *parent = NULL;
struct of_bus *bus, *pbus;
@@ -566,6 +571,7 @@ static u64 __of_translate_address(struct device_node *dev,
/* Increase refcount at current level */
of_node_get(dev);
+ *host = NULL;
/* Get parent & match bus type */
parent = of_get_parent(dev);
if (parent == NULL)
@@ -586,6 +592,8 @@ static u64 __of_translate_address(struct device_node *dev,
/* Translate */
for (;;) {
+ struct logic_pio_hwaddr *iorange;
+
/* Switch to parent bus */
of_node_put(dev);
dev = parent;
@@ -598,6 +606,19 @@ static u64 __of_translate_address(struct device_node *dev,
break;
}
+ /*
+* For indirectIO device which has no ranges property, get
+* the address from reg directly.
+*/
+ iorange = find_io_range_by_fwnode(>fwnode);
+ if (iorange && (iorange->flags != PIO_CPU_MMIO)) {
+ result = of_read_number(addr + 1, na - 1);
+ pr_debug("indirectIO matched(%s) 0x%llx\n",
+ of_node_full_name(dev), result);
+ *host = of_node_get(dev);
+ break;
+ }
+
/* Get new parent bus and counts */
pbus = of_match_bus(parent);
pbus->count_cells(dev, , );
@@ -630,13 +651,32 @@ static u64 __of_translate_address(struct device_node *dev,
u64 of_translate_address(struct device_node *dev, const __be32 *in_addr)
{
- return __of_translate_address(dev, in_addr, "ranges");
+ struct device_node *host;
+ u64 ret;
+
+ ret = __of_translate_address(dev, in_addr, "ranges", );
+ if (host) {
+ of_node_put(host);
+ return OF_BAD_ADDR;
+ }
+
+ return ret;
}
EXPORT_SYMBOL(of_translate_address);
u64 of_translate_dma_address(struct device_node *dev, const __be32 *in_addr)
{
- return __of_translate_address(dev, in_addr, "dma-ranges");
+ struct device_node *host;
+ u64 ret;
+
+ ret = __of_translate_address(dev, in_addr, "dma-ranges", );
+
+ if (host) {
+ of_node_put(host);
+ return OF_BAD_ADDR;
+ }
+
+ return ret;
}
EXPORT_SYMBOL(of_translate_dma_address);
@@ -678,29 +718,48 @@ const __be32 *of_get_address(struct device_node *dev, int
index, u64 *size,
}
EXPORT_SYMBOL(of_get_address);
+static u64 of_translate_ioport(struct device_node *dev, const __be32 *in_addr,
+ u64 size)
+{
+ u64 taddr;
+ unsigned long port;
+ struct device_node *host;
+
+ taddr = __of_translate_address(dev, in_addr, "ranges", );
+ if (host) {
+ /* host specific port access */
+ port = logi
[PATCH v10 7/9] ACPI: Translate the I/O range of non-MMIO devices before scanning
From: gabriele paoloni <gabriele.paol...@huawei.com>
On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices access
I/O with some special host-local I/O ports known on x86. As their I/O space
are not memory mapped like PCI/PCIE MMIO host bridges, this patch is meant
to support a new class of I/O host controllers where the local IO ports of
the children devices are translated into the Indirect I/O address space.
Through the handler attach callback, all the I/O translations are done
before starting the enumeration on children devices and the translated
addresses are replaced in the children resources.
Signed-off-by: zhichang.yuan <yuanzhich...@hisilicon.com>
Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
---
drivers/acpi/arm64/Makefile | 1 +
drivers/acpi/arm64/acpi_indirectio.c | 156 +++
drivers/acpi/arm64/acpi_indirectio.h | 24 ++
drivers/acpi/internal.h | 5 ++
drivers/acpi/scan.c | 1 +
5 files changed, 187 insertions(+)
create mode 100644 drivers/acpi/arm64/acpi_indirectio.c
create mode 100644 drivers/acpi/arm64/acpi_indirectio.h
diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile
index 1017def..6a33ac6 100644
--- a/drivers/acpi/arm64/Makefile
+++ b/drivers/acpi/arm64/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_ACPI_IORT)+= iort.o
obj-$(CONFIG_ACPI_GTDT)+= gtdt.o
+obj-$(CONFIG_INDIRECT_PIO) += acpi_indirectio.o
diff --git a/drivers/acpi/arm64/acpi_indirectio.c
b/drivers/acpi/arm64/acpi_indirectio.c
new file mode 100644
index 000..013db6f
--- /dev/null
+++ b/drivers/acpi/arm64/acpi_indirectio.c
@@ -0,0 +1,156 @@
+/*
+ * ACPI support for indirect-IO bus.
+ *
+ * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved.
+ * Author: Gabriele Paoloni <gabriele.paol...@huawei.com>
+ * Author: Zhichang Yuan <yuanzhich...@hisilicon.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+
+#include "acpi_indirectio.h"
+
+ACPI_MODULE_NAME("indirect IO");
+
+#define INDIRECT_IO_INFO(desc) ((unsigned long))
+
+static int acpi_translate_logiciores(struct acpi_device *adev,
+ struct acpi_device *host, struct resource *resource)
+{
+ unsigned long sys_port;
+ struct device *dev = >dev;
+ resource_size_t length = resource->end - resource->start;
+
+ sys_port = logic_pio_trans_hwaddr(>fwnode, resource->start,
+ length);
+
+ if (sys_port == -1) {
+ dev_err(dev, "translate bus-addr(0x%llx) fail!\n",
+ resource->start);
+ return -EFAULT;
+ }
+
+ resource->start = sys_port;
+ resource->end = sys_port + length;
+
+ return 0;
+}
+
+/*
+ * update/set the current I/O resource of the designated device node.
+ * after this calling, the enumeration can be started as the I/O resource
+ * had been translated to logicial I/O from bus-local I/O.
+ *
+ * @child: the device node to be updated the I/O resource;
+ * @hostdev: the device node where 'adev' is attached, which can be not
+ * the parent of 'adev';
+ * @res: double pointer to be set to the address of the updated resources
+ * @num_res: address of the variable to contain the number of updated resources
+ *
+ * return 0 when successful, negative is for failure.
+ */
+int acpi_set_logicio_resource(struct device *child,
+ struct device *hostdev, const struct resource **res,
+ int *num_res)
+{
+ struct acpi_device *adev;
+ struct acpi_device *host;
+ struct resource_entry *rentry;
+ LIST_HEAD(resource_list);
+ struct resource *resources = NULL;
+ int count;
+ int i;
+ int ret = -EIO;
+
+ if (!child || !hostdev)
+ return -EINVAL;
+
+ host = to_acpi_device(hostdev);
+ adev = to_acpi_device(child);
+
+ /* check the device state */
+ if (!adev->status.present) {
+ dev_info(child, "ACPI: device is not present!\n");
+ return 0;
+ }
+ /* whether the child had been enumerated? */
+ if (acpi_device_enumerated(adev)) {
+ dev_info(child, "ACPI: had been enumerated!\n");
+ return 0;
+ }
+
+ count = acpi_dev_get_resources(adev, _list, NULL, NULL);
+ if (count <= 0) {
+ dev_err(>dev, "failed to get ACPI resources\n");
+ return count ? count : -EIO;
+ }
+
+ resources = kcalloc(count, sizeof(struct resource), GFP_KERNEL);
+ if (!resources) {
+ dev_err(>dev, "No memory for resources\n");
+ ac
[PATCH v10 7/9] ACPI: Translate the I/O range of non-MMIO devices before scanning
From: gabriele paoloni
On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices access
I/O with some special host-local I/O ports known on x86. As their I/O space
are not memory mapped like PCI/PCIE MMIO host bridges, this patch is meant
to support a new class of I/O host controllers where the local IO ports of
the children devices are translated into the Indirect I/O address space.
Through the handler attach callback, all the I/O translations are done
before starting the enumeration on children devices and the translated
addresses are replaced in the children resources.
Signed-off-by: zhichang.yuan
Signed-off-by: Gabriele Paoloni
---
drivers/acpi/arm64/Makefile | 1 +
drivers/acpi/arm64/acpi_indirectio.c | 156 +++
drivers/acpi/arm64/acpi_indirectio.h | 24 ++
drivers/acpi/internal.h | 5 ++
drivers/acpi/scan.c | 1 +
5 files changed, 187 insertions(+)
create mode 100644 drivers/acpi/arm64/acpi_indirectio.c
create mode 100644 drivers/acpi/arm64/acpi_indirectio.h
diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile
index 1017def..6a33ac6 100644
--- a/drivers/acpi/arm64/Makefile
+++ b/drivers/acpi/arm64/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_ACPI_IORT)+= iort.o
obj-$(CONFIG_ACPI_GTDT)+= gtdt.o
+obj-$(CONFIG_INDIRECT_PIO) += acpi_indirectio.o
diff --git a/drivers/acpi/arm64/acpi_indirectio.c
b/drivers/acpi/arm64/acpi_indirectio.c
new file mode 100644
index 000..013db6f
--- /dev/null
+++ b/drivers/acpi/arm64/acpi_indirectio.c
@@ -0,0 +1,156 @@
+/*
+ * ACPI support for indirect-IO bus.
+ *
+ * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved.
+ * Author: Gabriele Paoloni
+ * Author: Zhichang Yuan
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+
+#include "acpi_indirectio.h"
+
+ACPI_MODULE_NAME("indirect IO");
+
+#define INDIRECT_IO_INFO(desc) ((unsigned long))
+
+static int acpi_translate_logiciores(struct acpi_device *adev,
+ struct acpi_device *host, struct resource *resource)
+{
+ unsigned long sys_port;
+ struct device *dev = >dev;
+ resource_size_t length = resource->end - resource->start;
+
+ sys_port = logic_pio_trans_hwaddr(>fwnode, resource->start,
+ length);
+
+ if (sys_port == -1) {
+ dev_err(dev, "translate bus-addr(0x%llx) fail!\n",
+ resource->start);
+ return -EFAULT;
+ }
+
+ resource->start = sys_port;
+ resource->end = sys_port + length;
+
+ return 0;
+}
+
+/*
+ * update/set the current I/O resource of the designated device node.
+ * after this calling, the enumeration can be started as the I/O resource
+ * had been translated to logicial I/O from bus-local I/O.
+ *
+ * @child: the device node to be updated the I/O resource;
+ * @hostdev: the device node where 'adev' is attached, which can be not
+ * the parent of 'adev';
+ * @res: double pointer to be set to the address of the updated resources
+ * @num_res: address of the variable to contain the number of updated resources
+ *
+ * return 0 when successful, negative is for failure.
+ */
+int acpi_set_logicio_resource(struct device *child,
+ struct device *hostdev, const struct resource **res,
+ int *num_res)
+{
+ struct acpi_device *adev;
+ struct acpi_device *host;
+ struct resource_entry *rentry;
+ LIST_HEAD(resource_list);
+ struct resource *resources = NULL;
+ int count;
+ int i;
+ int ret = -EIO;
+
+ if (!child || !hostdev)
+ return -EINVAL;
+
+ host = to_acpi_device(hostdev);
+ adev = to_acpi_device(child);
+
+ /* check the device state */
+ if (!adev->status.present) {
+ dev_info(child, "ACPI: device is not present!\n");
+ return 0;
+ }
+ /* whether the child had been enumerated? */
+ if (acpi_device_enumerated(adev)) {
+ dev_info(child, "ACPI: had been enumerated!\n");
+ return 0;
+ }
+
+ count = acpi_dev_get_resources(adev, _list, NULL, NULL);
+ if (count <= 0) {
+ dev_err(>dev, "failed to get ACPI resources\n");
+ return count ? count : -EIO;
+ }
+
+ resources = kcalloc(count, sizeof(struct resource), GFP_KERNEL);
+ if (!resources) {
+ dev_err(>dev, "No memory for resources\n");
+ acpi_dev_free_resource_list(_list);
+ return -ENOMEM;
+ }
+ count = 0;
+ list_for_each_entry(rentry, _list, node)
+ resources[count
[PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings
From: "zhichang.yuan" <yuanzhich...@hisilicon.com>
The low-pin-count(LPC) interface of Hip06/Hip07 accesses the peripherals in
I/O port addresses. This patch implements the LPC host controller driver
which perform the I/O operations on the underlying hardware.
We don't want to touch those existing peripherals' driver, such as ipmi-bt.
So this driver applies the indirect-IO introduced in the previous patch
after registering an indirect-IO node to the indirect-IO devices list which
will be searched in the I/O accessors to retrieve the host-local I/O port.
Signed-off-by: Zou Rongrong <zourongr...@huawei.com>
Signed-off-by: zhichang.yuan <yuanzhich...@hisilicon.com>
Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
Acked-by: Rob Herring <r...@kernel.org> #dts part
---
.../arm/hisilicon/hisilicon-low-pin-count.txt | 33 ++
drivers/bus/Kconfig| 9 +
drivers/bus/Makefile | 1 +
drivers/bus/hisi_lpc.c | 526 +
4 files changed, 569 insertions(+)
create mode 100644
Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
create mode 100644 drivers/bus/hisi_lpc.c
diff --git
a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
new file mode 100644
index 000..213181f
--- /dev/null
+++
b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
@@ -0,0 +1,33 @@
+Hisilicon Hip06 low-pin-count device
+ Hisilicon Hip06 SoCs implement a Low Pin Count (LPC) controller, which
+ provides I/O access to some legacy ISA devices.
+ Hip06 is based on arm64 architecture where there is no I/O space. So, the
+ I/O ports here are not cpu addresses, and there is no 'ranges' property in
+ LPC device node.
+
+Required properties:
+- compatible: value should be as follows:
+ (a) "hisilicon,hip06-lpc"
+ (b) "hisilicon,hip07-lpc"
+- #address-cells: must be 2 which stick to the ISA/EISA binding doc.
+- #size-cells: must be 1 which stick to the ISA/EISA binding doc.
+- reg: base memory range where the LPC register set is mapped.
+
+Note:
+ The node name before '@' must be "isa" to represent the binding stick to the
+ ISA/EISA binding specification.
+
+Example:
+
+isa@a01b {
+ compatible = "hisilicon,hip06-lpc";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ reg = <0x0 0xa01b 0x0 0x1000>;
+
+ ipmi0: bt@e4 {
+ compatible = "ipmi-bt";
+ device_type = "ipmi";
+ reg = <0x01 0xe4 0x04>;
+ };
+};
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 2408ea3..358eed3 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -64,6 +64,15 @@ config BRCMSTB_GISB_ARB
arbiter. This driver provides timeout and target abort error handling
and internal bus master decoding.
+config HISILICON_LPC
+ bool "Support for ISA I/O space on Hisilicon Hip0X"
+ depends on (ARM64 && (ARCH_HISI || COMPILE_TEST))
+ select LOGIC_PIO
+ select INDIRECT_PIO
+ help
+ Driver needed for some legacy ISA devices attached to Low-Pin-Count
+ on Hisilicon Hip0X SoC.
+
config IMX_WEIM
bool "Freescale EIM DRIVER"
depends on ARCH_MXC
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index cc6364b..28e3862 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_ARM_CCI) += arm-cci.o
obj-$(CONFIG_ARM_CCN) += arm-ccn.o
obj-$(CONFIG_BRCMSTB_GISB_ARB) += brcmstb_gisb.o
+obj-$(CONFIG_HISILICON_LPC)+= hisi_lpc.o
obj-$(CONFIG_IMX_WEIM) += imx-weim.o
obj-$(CONFIG_MIPS_CDMM)+= mips_cdmm.o
obj-$(CONFIG_MVEBU_MBUS) += mvebu-mbus.o
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
new file mode 100644
index 000..c885483
--- /dev/null
+++ b/drivers/bus/hisi_lpc.c
@@ -0,0 +1,526 @@
+/*
+ * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved.
+ * Author: Zhichang Yuan <yuanzhich...@hisilicon.com>
+ * Author: Zou Rongrong <zourongr...@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/&
[PATCH v10 6/9] LPC: Support the LPC host on Hip06/Hip07 with DT bindings
From: "zhichang.yuan"
The low-pin-count(LPC) interface of Hip06/Hip07 accesses the peripherals in
I/O port addresses. This patch implements the LPC host controller driver
which perform the I/O operations on the underlying hardware.
We don't want to touch those existing peripherals' driver, such as ipmi-bt.
So this driver applies the indirect-IO introduced in the previous patch
after registering an indirect-IO node to the indirect-IO devices list which
will be searched in the I/O accessors to retrieve the host-local I/O port.
Signed-off-by: Zou Rongrong
Signed-off-by: zhichang.yuan
Signed-off-by: Gabriele Paoloni
Acked-by: Rob Herring #dts part
---
.../arm/hisilicon/hisilicon-low-pin-count.txt | 33 ++
drivers/bus/Kconfig| 9 +
drivers/bus/Makefile | 1 +
drivers/bus/hisi_lpc.c | 526 +
4 files changed, 569 insertions(+)
create mode 100644
Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
create mode 100644 drivers/bus/hisi_lpc.c
diff --git
a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
new file mode 100644
index 000..213181f
--- /dev/null
+++
b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
@@ -0,0 +1,33 @@
+Hisilicon Hip06 low-pin-count device
+ Hisilicon Hip06 SoCs implement a Low Pin Count (LPC) controller, which
+ provides I/O access to some legacy ISA devices.
+ Hip06 is based on arm64 architecture where there is no I/O space. So, the
+ I/O ports here are not cpu addresses, and there is no 'ranges' property in
+ LPC device node.
+
+Required properties:
+- compatible: value should be as follows:
+ (a) "hisilicon,hip06-lpc"
+ (b) "hisilicon,hip07-lpc"
+- #address-cells: must be 2 which stick to the ISA/EISA binding doc.
+- #size-cells: must be 1 which stick to the ISA/EISA binding doc.
+- reg: base memory range where the LPC register set is mapped.
+
+Note:
+ The node name before '@' must be "isa" to represent the binding stick to the
+ ISA/EISA binding specification.
+
+Example:
+
+isa@a01b {
+ compatible = "hisilicon,hip06-lpc";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ reg = <0x0 0xa01b 0x0 0x1000>;
+
+ ipmi0: bt@e4 {
+ compatible = "ipmi-bt";
+ device_type = "ipmi";
+ reg = <0x01 0xe4 0x04>;
+ };
+};
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 2408ea3..358eed3 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -64,6 +64,15 @@ config BRCMSTB_GISB_ARB
arbiter. This driver provides timeout and target abort error handling
and internal bus master decoding.
+config HISILICON_LPC
+ bool "Support for ISA I/O space on Hisilicon Hip0X"
+ depends on (ARM64 && (ARCH_HISI || COMPILE_TEST))
+ select LOGIC_PIO
+ select INDIRECT_PIO
+ help
+ Driver needed for some legacy ISA devices attached to Low-Pin-Count
+ on Hisilicon Hip0X SoC.
+
config IMX_WEIM
bool "Freescale EIM DRIVER"
depends on ARCH_MXC
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index cc6364b..28e3862 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_ARM_CCI) += arm-cci.o
obj-$(CONFIG_ARM_CCN) += arm-ccn.o
obj-$(CONFIG_BRCMSTB_GISB_ARB) += brcmstb_gisb.o
+obj-$(CONFIG_HISILICON_LPC)+= hisi_lpc.o
obj-$(CONFIG_IMX_WEIM) += imx-weim.o
obj-$(CONFIG_MIPS_CDMM)+= mips_cdmm.o
obj-$(CONFIG_MVEBU_MBUS) += mvebu-mbus.o
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
new file mode 100644
index 000..c885483
--- /dev/null
+++ b/drivers/bus/hisi_lpc.c
@@ -0,0 +1,526 @@
+/*
+ * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved.
+ * Author: Zhichang Yuan
+ * Author: Zou Rongrong
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define LPC_MIN_BUS_RANGE 0x0
+
+/*
+ * The default maximal IO size for Hip06/Hip07 L
[PATCH v10 0/9] LPC: legacy ISA I/O support
From: gabriele paoloni <gabriele.paol...@huawei.com> This patchset supports the IPMI-bt device attached to the Low-Pin-Count interface implemented on Hisilicon Hip06/Hip07 SoC. --- | LPC host| | | --- | _V___LPC | | V V | BT(ipmi)| When master accesses those peripherals beneath the Hip06/Hip07 LPC, a specific LPC driver is needed to make LPC host generate the standard LPC I/O cycles with the target peripherals'I/O port addresses. But on curent arm64 world, there is no real I/O accesses. All the I/O operations through in/out accessors are based on MMIO ranges; on Hip06/Hip07 LPC the I/O accesses are performed through driver specific accessors rather than MMIO. To solve this issue and keep the relevant existing peripherals' drivers untouched, this patchset: - introduces a generic I/O space management framework, LIBIO, to support I/O operations on host controllers operating either on MMIO buses or on buses requiring specific driver I/O accessors; - redefines the in/out accessors to provide a unified interface for both MMIO and driver specific I/O operations. Using LIBIO, th call of in/out() from the host children drivers, such as ipmi-si, will be redirected to the corresponding device-specific I/O hooks to perform the I/O accesses. Based on this patch-set, all the I/O accesses to Hip06/Hip07 LPC peripherals can be supported without any changes on the existing ipmi-si driver. The whole patchset has been tested on Hip07 D05 board both using DTB and ACPI. Changes from v9: - patch 2 has been split into 3 patches according to Bjorn comments on v9 thread - patch 1 has been reworked accordign to Bjorn comments on v9 - now logic_pio_trans_hwaddr() has a sanity check to make sure the resource size fits into the assigned range - in patch 5 the MFD framework has been used to probe the LPC children according to the suggestion from Mika Westerberg - Maintaner has changed to Huawei Linuxarm mailing list Changes from v8: - Simplified LIB IO framewrok - Moved INDIRECT PIO ACPI framework under acpi/arm64 - Renamed occurrences of "lib io" and "indirect io" to "lib pio" and "indirect pio" to keep the patchset nomenclature consistent - Removed Alignment reuqirements - Moved LPC specific code out of ACPI common framework - Now PIO indirect HW ranges can overlap - Changed HiSilicon LPC driver maintainer (Gabriele Paoloni now) and split maintaner file modifications in a separate commit - Removed the commit with the DT nodes support for hip06 and hip07 (to be pushed separately) - Added a checking on ioport_map() not to break that function as Arnd points out in V7 review thread; - fixed the compile issues on alpha, m68k; Changes from V7: - Based on Arnd's comment, rename the LIBIO as LOGIC_PIO; - Improved the mapping process in LOGIC_PIO to gain better efficiency when redirecting the I/O accesses to right device driver; - To reduce the impact on PCI MMIO to a minimum, add a new CONFIG_INDIRECT_PIO for indirect-IO hosts/devices; - Added a new ACPI handler for indirect-IO hosts/devices; - Fixed the compile issues on V6; Changes from V6: - According to the comments from Bjorn and Alex, merge PCI IO and indirect-IO into a generic I/O space management, LIBIO; - Adopted the '_DEP' to replace the platform bus notifier. In this way, we can ensure the LPC peripherals' I/O resources had been translated to logical IO before the LPC peripheral enumeration; - Replaced the rwlock with rcu list based on Alex's suggestion; - Applied relaxed write/read to LPC driver; - Some bugs fixing and some optimazations based on the comments of V6; Changes from V5: - Made the extio driver more generic and locate in lib/; - Supported multiple indirect-IO bus instances; - Extended the pci_register_io_range() to support indirect-IO, then dropped the I/O reservation used in previous patchset; - Reimplemented the ACPI LPC support; - Fixed some bugs, including the compile error on other archs, the module building failure found by Ming Lei, etc; Changes from V4: - Some revises based on the comments from Bjorn, Rob on V4; - Fixed the compile error on some platforms, such as openrisc; Changes from V3: - UART support deferred to a separate patchset; This patchset only support ipmi device under LPC; - LPC bus I/O range is fixed to 0 ~ (PCIBIOS_MIN_IO - 1), which is separeted from PCI/PCIE PIO space; - Based on Arnd's remarks, removed the ranges prope
[PATCH v10 0/9] LPC: legacy ISA I/O support
From: gabriele paoloni This patchset supports the IPMI-bt device attached to the Low-Pin-Count interface implemented on Hisilicon Hip06/Hip07 SoC. --- | LPC host| | | --- | _V___LPC | | V V | BT(ipmi)| When master accesses those peripherals beneath the Hip06/Hip07 LPC, a specific LPC driver is needed to make LPC host generate the standard LPC I/O cycles with the target peripherals'I/O port addresses. But on curent arm64 world, there is no real I/O accesses. All the I/O operations through in/out accessors are based on MMIO ranges; on Hip06/Hip07 LPC the I/O accesses are performed through driver specific accessors rather than MMIO. To solve this issue and keep the relevant existing peripherals' drivers untouched, this patchset: - introduces a generic I/O space management framework, LIBIO, to support I/O operations on host controllers operating either on MMIO buses or on buses requiring specific driver I/O accessors; - redefines the in/out accessors to provide a unified interface for both MMIO and driver specific I/O operations. Using LIBIO, th call of in/out() from the host children drivers, such as ipmi-si, will be redirected to the corresponding device-specific I/O hooks to perform the I/O accesses. Based on this patch-set, all the I/O accesses to Hip06/Hip07 LPC peripherals can be supported without any changes on the existing ipmi-si driver. The whole patchset has been tested on Hip07 D05 board both using DTB and ACPI. Changes from v9: - patch 2 has been split into 3 patches according to Bjorn comments on v9 thread - patch 1 has been reworked accordign to Bjorn comments on v9 - now logic_pio_trans_hwaddr() has a sanity check to make sure the resource size fits into the assigned range - in patch 5 the MFD framework has been used to probe the LPC children according to the suggestion from Mika Westerberg - Maintaner has changed to Huawei Linuxarm mailing list Changes from v8: - Simplified LIB IO framewrok - Moved INDIRECT PIO ACPI framework under acpi/arm64 - Renamed occurrences of "lib io" and "indirect io" to "lib pio" and "indirect pio" to keep the patchset nomenclature consistent - Removed Alignment reuqirements - Moved LPC specific code out of ACPI common framework - Now PIO indirect HW ranges can overlap - Changed HiSilicon LPC driver maintainer (Gabriele Paoloni now) and split maintaner file modifications in a separate commit - Removed the commit with the DT nodes support for hip06 and hip07 (to be pushed separately) - Added a checking on ioport_map() not to break that function as Arnd points out in V7 review thread; - fixed the compile issues on alpha, m68k; Changes from V7: - Based on Arnd's comment, rename the LIBIO as LOGIC_PIO; - Improved the mapping process in LOGIC_PIO to gain better efficiency when redirecting the I/O accesses to right device driver; - To reduce the impact on PCI MMIO to a minimum, add a new CONFIG_INDIRECT_PIO for indirect-IO hosts/devices; - Added a new ACPI handler for indirect-IO hosts/devices; - Fixed the compile issues on V6; Changes from V6: - According to the comments from Bjorn and Alex, merge PCI IO and indirect-IO into a generic I/O space management, LIBIO; - Adopted the '_DEP' to replace the platform bus notifier. In this way, we can ensure the LPC peripherals' I/O resources had been translated to logical IO before the LPC peripheral enumeration; - Replaced the rwlock with rcu list based on Alex's suggestion; - Applied relaxed write/read to LPC driver; - Some bugs fixing and some optimazations based on the comments of V6; Changes from V5: - Made the extio driver more generic and locate in lib/; - Supported multiple indirect-IO bus instances; - Extended the pci_register_io_range() to support indirect-IO, then dropped the I/O reservation used in previous patchset; - Reimplemented the ACPI LPC support; - Fixed some bugs, including the compile error on other archs, the module building failure found by Ming Lei, etc; Changes from V4: - Some revises based on the comments from Bjorn, Rob on V4; - Fixed the compile error on some platforms, such as openrisc; Changes from V3: - UART support deferred to a separate patchset; This patchset only support ipmi device under LPC; - LPC bus I/O range is fixed to 0 ~ (PCIBIOS_MIN_IO - 1), which is separeted from PCI/PCIE PIO space; - Based on Arnd's remarks, removed the ranges property from Hip06 lpc dts and added
[PATCH v10 1/9] LIB: Introduce a generic PIO mapping method
From: "zhichang.yuan" <yuanzhich...@hisilicon.com>
In commit 41f8bba7f555 ("of/pci: Add pci_register_io_range() and
pci_pio_to_address()"), a new I/O space management was supported. With
that driver, the I/O ranges configured for PCI/PCIE hosts on some
architectures can be mapped to logical PIO, converted easily between
CPU address and the corresponding logicial PIO. Based on this, PCI
I/O devices can be accessed in a memory read/write way through the
unified in/out accessors.
But on some archs/platforms, there are bus hosts which access I/O
peripherals with host-local I/O port addresses rather than memory
addresses after memory-mapped.
To support those devices, a more generic I/O mapping method is introduced
here. Through this patch, both the CPU addresses and the host-local port
can be mapped into the logical PIO space with different logical/fake PIOs.
After this, all the I/O accesses to either PCI MMIO devices or host-local
I/O peripherals can be unified into the existing I/O accessors defined in
asm-generic/io.h and be redirected to the right device-specific hooks
based on the input logical PIO.
Signed-off-by: zhichang.yuan <yuanzhich...@hisilicon.com>
Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
---
include/asm-generic/io.h | 26 +
include/linux/logic_pio.h | 118 +++
lib/Kconfig | 26 +
lib/Makefile | 2 +
lib/logic_pio.c | 286 ++
5 files changed, 458 insertions(+)
create mode 100644 include/linux/logic_pio.h
create mode 100644 lib/logic_pio.c
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 7ef015e..334e5db 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -351,6 +351,8 @@ static inline void writesq(volatile void __iomem *addr,
const void *buffer,
#define IO_SPACE_LIMIT 0x
#endif
+#include
+
/*
* {in,out}{b,w,l}() access little endian I/O. {in,out}{b,w,l}_p() can be
* implemented on hardware that needs an additional delay for I/O accesses to
@@ -358,51 +360,75 @@ static inline void writesq(volatile void __iomem *addr,
const void *buffer,
*/
#ifndef inb
+#ifdef CONFIG_INDIRECT_PIO
+#define inb logic_inb
+#else
#define inb inb
static inline u8 inb(unsigned long addr)
{
return readb(PCI_IOBASE + addr);
}
+#endif /* CONFIG_INDIRECT_PIO */
#endif
#ifndef inw
+#ifdef CONFIG_INDIRECT_PIO
+#define inw logic_inw
+#else
#define inw inw
static inline u16 inw(unsigned long addr)
{
return readw(PCI_IOBASE + addr);
}
+#endif /* CONFIG_INDIRECT_PIO */
#endif
#ifndef inl
+#ifdef CONFIG_INDIRECT_PIO
+#define inl logic_inl
+#else
#define inl inl
static inline u32 inl(unsigned long addr)
{
return readl(PCI_IOBASE + addr);
}
+#endif /* CONFIG_INDIRECT_PIO */
#endif
#ifndef outb
+#ifdef CONFIG_INDIRECT_PIO
+#define outb logic_outb
+#else
#define outb outb
static inline void outb(u8 value, unsigned long addr)
{
writeb(value, PCI_IOBASE + addr);
}
+#endif /* CONFIG_INDIRECT_PIO */
#endif
#ifndef outw
+#ifdef CONFIG_INDIRECT_PIO
+#define outw logic_outw
+#else
#define outw outw
static inline void outw(u16 value, unsigned long addr)
{
writew(value, PCI_IOBASE + addr);
}
+#endif /* CONFIG_INDIRECT_PIO */
#endif
#ifndef outl
+#ifdef CONFIG_INDIRECT_PIO
+#define outl logic_outl
+#else
#define outl outl
static inline void outl(u32 value, unsigned long addr)
{
writel(value, PCI_IOBASE + addr);
}
+#endif /* CONFIG_INDIRECT_PIO */
#endif
#ifndef inb_p
diff --git a/include/linux/logic_pio.h b/include/linux/logic_pio.h
new file mode 100644
index 000..f0a6f15
--- /dev/null
+++ b/include/linux/logic_pio.h
@@ -0,0 +1,118 @@
+/*
+ * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved.
+ * Author: Gabriele Paoloni <gabriele.paol...@huawei.com>
+ * Author: Zhichang Yuan <yuanzhich...@hisilicon.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __LINUX_LOGIC_PIO_H
+#define __LINUX_LOGIC_PIO_H
+
+#ifdef __KERNEL__
+
+#include
+
+#define PIO_INDIRECT 0x01UL /* indirect IO flag */
+#define PIO_CPU_MMIO 0x00UL /* memory mapped io flag */
+
+struct logic_pio_hwaddr {
+ struct list_head list;
+ struct fwnode_handle *fwnode;
+ resource_size_t hw_start;
+ resource_size_t io_star
[PATCH v10 1/9] LIB: Introduce a generic PIO mapping method
From: "zhichang.yuan"
In commit 41f8bba7f555 ("of/pci: Add pci_register_io_range() and
pci_pio_to_address()"), a new I/O space management was supported. With
that driver, the I/O ranges configured for PCI/PCIE hosts on some
architectures can be mapped to logical PIO, converted easily between
CPU address and the corresponding logicial PIO. Based on this, PCI
I/O devices can be accessed in a memory read/write way through the
unified in/out accessors.
But on some archs/platforms, there are bus hosts which access I/O
peripherals with host-local I/O port addresses rather than memory
addresses after memory-mapped.
To support those devices, a more generic I/O mapping method is introduced
here. Through this patch, both the CPU addresses and the host-local port
can be mapped into the logical PIO space with different logical/fake PIOs.
After this, all the I/O accesses to either PCI MMIO devices or host-local
I/O peripherals can be unified into the existing I/O accessors defined in
asm-generic/io.h and be redirected to the right device-specific hooks
based on the input logical PIO.
Signed-off-by: zhichang.yuan
Signed-off-by: Gabriele Paoloni
---
include/asm-generic/io.h | 26 +
include/linux/logic_pio.h | 118 +++
lib/Kconfig | 26 +
lib/Makefile | 2 +
lib/logic_pio.c | 286 ++
5 files changed, 458 insertions(+)
create mode 100644 include/linux/logic_pio.h
create mode 100644 lib/logic_pio.c
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 7ef015e..334e5db 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -351,6 +351,8 @@ static inline void writesq(volatile void __iomem *addr,
const void *buffer,
#define IO_SPACE_LIMIT 0x
#endif
+#include
+
/*
* {in,out}{b,w,l}() access little endian I/O. {in,out}{b,w,l}_p() can be
* implemented on hardware that needs an additional delay for I/O accesses to
@@ -358,51 +360,75 @@ static inline void writesq(volatile void __iomem *addr,
const void *buffer,
*/
#ifndef inb
+#ifdef CONFIG_INDIRECT_PIO
+#define inb logic_inb
+#else
#define inb inb
static inline u8 inb(unsigned long addr)
{
return readb(PCI_IOBASE + addr);
}
+#endif /* CONFIG_INDIRECT_PIO */
#endif
#ifndef inw
+#ifdef CONFIG_INDIRECT_PIO
+#define inw logic_inw
+#else
#define inw inw
static inline u16 inw(unsigned long addr)
{
return readw(PCI_IOBASE + addr);
}
+#endif /* CONFIG_INDIRECT_PIO */
#endif
#ifndef inl
+#ifdef CONFIG_INDIRECT_PIO
+#define inl logic_inl
+#else
#define inl inl
static inline u32 inl(unsigned long addr)
{
return readl(PCI_IOBASE + addr);
}
+#endif /* CONFIG_INDIRECT_PIO */
#endif
#ifndef outb
+#ifdef CONFIG_INDIRECT_PIO
+#define outb logic_outb
+#else
#define outb outb
static inline void outb(u8 value, unsigned long addr)
{
writeb(value, PCI_IOBASE + addr);
}
+#endif /* CONFIG_INDIRECT_PIO */
#endif
#ifndef outw
+#ifdef CONFIG_INDIRECT_PIO
+#define outw logic_outw
+#else
#define outw outw
static inline void outw(u16 value, unsigned long addr)
{
writew(value, PCI_IOBASE + addr);
}
+#endif /* CONFIG_INDIRECT_PIO */
#endif
#ifndef outl
+#ifdef CONFIG_INDIRECT_PIO
+#define outl logic_outl
+#else
#define outl outl
static inline void outl(u32 value, unsigned long addr)
{
writel(value, PCI_IOBASE + addr);
}
+#endif /* CONFIG_INDIRECT_PIO */
#endif
#ifndef inb_p
diff --git a/include/linux/logic_pio.h b/include/linux/logic_pio.h
new file mode 100644
index 000..f0a6f15
--- /dev/null
+++ b/include/linux/logic_pio.h
@@ -0,0 +1,118 @@
+/*
+ * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved.
+ * Author: Gabriele Paoloni
+ * Author: Zhichang Yuan
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __LINUX_LOGIC_PIO_H
+#define __LINUX_LOGIC_PIO_H
+
+#ifdef __KERNEL__
+
+#include
+
+#define PIO_INDIRECT 0x01UL /* indirect IO flag */
+#define PIO_CPU_MMIO 0x00UL /* memory mapped io flag */
+
+struct logic_pio_hwaddr {
+ struct list_head list;
+ struct fwnode_handle *fwnode;
+ resource_size_t hw_start;
+ resource_size_t io_start;
+ resource_size_t size; /* range size populated */
+ unsigned long flags;
+
+ void *devpara; /* private parameter of the host device */
+ struct hostio_
RE: [PATCH v3] PCIe AER: report uncorrectable errors only to the functions that logged the errors
[...] > > > > Applied to pci/aer for v4.15, thanks! > > > > I rewrote some of the changelog to say "non-fatal" instead of > > "uncorrectable", since "uncorrectable" also includes fatal errors, > > and you're not changing those. Take a look and let me know if > > I broke anything. > > Here it is so you don't have to look it up :) > Many thanks Bjorn! Gab [...]
RE: [PATCH v3] PCIe AER: report uncorrectable errors only to the functions that logged the errors
[...] > > > > Applied to pci/aer for v4.15, thanks! > > > > I rewrote some of the changelog to say "non-fatal" instead of > > "uncorrectable", since "uncorrectable" also includes fatal errors, > > and you're not changing those. Take a look and let me know if > > I broke anything. > > Here it is so you don't have to look it up :) > Many thanks Bjorn! Gab [...]
[PATCH v3] PCIe AER: report uncorrectable errors only to the functions that logged the errors
Currently if an uncorrectable error is reported by an EP the AER
driver walks over all the devices connected to the upstream port
bus and in turns call the report_error_detected() callback.
If any of the devices connected to the bus does not implement
dev->driver->err_handler->error_detected() do_recovery() will fail
leaving all the bus hierarchy devices unrecovered.
According to section "6.2.2.2.2. Non-Fatal Errors" of the PCIe specs
<< Non-fatal errors are uncorrectable errors which cause a particular
transaction to be unreliable but the Link is otherwise fully functional.
Isolating Non-fatal from Fatal errors provides Requester/Receiver logic
in a device or system management software the opportunity to recover
from the error without resetting the components on the Link and
disturbing other transactions in progress. Devices not associated with
the transaction in error are not impacted by the error.>>
therefore for non fatal errors the PCIe link should not be considered
compromised and it makes sense to report the error only to all the
functions that logged an error.
This patch implements this new behaviour for non fatal errors.
Also this patch fixes a bug (filed as in the link below)
Link: https://bugzilla.kernel.org/show_bug.cgi?id=197055
Fixes: 6c2b374d7485 ("PCI-Express AER implemetation: AER core and aerdriver")
Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
Signed-off-by: Dongdong Liu <liudongdo...@huawei.com>
---
Changes from v2:
- no functional changes
- Added reference in the commit log to the bugzilla ticket
- Added reference in the commit log the commit that this patch fixes
- Added reference in the commit log to the PCIe specs for Non-fatal
error handling rules
Changes from v1:
- now errors are reported only to the fucntions that logged the error
instead of all the functions in the same device.
- the patch subject has changed to match the new implementation
---
drivers/pci/pcie/aer/aerdrv_core.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pcie/aer/aerdrv_core.c
b/drivers/pci/pcie/aer/aerdrv_core.c
index 890efcc..7448052 100644
--- a/drivers/pci/pcie/aer/aerdrv_core.c
+++ b/drivers/pci/pcie/aer/aerdrv_core.c
@@ -390,7 +390,14 @@ static pci_ers_result_t broadcast_error_message(struct
pci_dev *dev,
* If the error is reported by an end point, we think this
* error is related to the upstream link of the end point.
*/
- pci_walk_bus(dev->bus, cb, _data);
+ if (state == pci_channel_io_normal)
+ /*
+* the error is non fatal so the bus is ok, just invoke
+* the callback for the function that logged the error.
+*/
+ cb(dev, _data);
+ else
+ pci_walk_bus(dev->bus, cb, _data);
}
return result_data.result;
--
2.7.4
[PATCH v3] PCIe AER: report uncorrectable errors only to the functions that logged the errors
Currently if an uncorrectable error is reported by an EP the AER
driver walks over all the devices connected to the upstream port
bus and in turns call the report_error_detected() callback.
If any of the devices connected to the bus does not implement
dev->driver->err_handler->error_detected() do_recovery() will fail
leaving all the bus hierarchy devices unrecovered.
According to section "6.2.2.2.2. Non-Fatal Errors" of the PCIe specs
<< Non-fatal errors are uncorrectable errors which cause a particular
transaction to be unreliable but the Link is otherwise fully functional.
Isolating Non-fatal from Fatal errors provides Requester/Receiver logic
in a device or system management software the opportunity to recover
from the error without resetting the components on the Link and
disturbing other transactions in progress. Devices not associated with
the transaction in error are not impacted by the error.>>
therefore for non fatal errors the PCIe link should not be considered
compromised and it makes sense to report the error only to all the
functions that logged an error.
This patch implements this new behaviour for non fatal errors.
Also this patch fixes a bug (filed as in the link below)
Link: https://bugzilla.kernel.org/show_bug.cgi?id=197055
Fixes: 6c2b374d7485 ("PCI-Express AER implemetation: AER core and aerdriver")
Signed-off-by: Gabriele Paoloni
Signed-off-by: Dongdong Liu
---
Changes from v2:
- no functional changes
- Added reference in the commit log to the bugzilla ticket
- Added reference in the commit log the commit that this patch fixes
- Added reference in the commit log to the PCIe specs for Non-fatal
error handling rules
Changes from v1:
- now errors are reported only to the fucntions that logged the error
instead of all the functions in the same device.
- the patch subject has changed to match the new implementation
---
drivers/pci/pcie/aer/aerdrv_core.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pcie/aer/aerdrv_core.c
b/drivers/pci/pcie/aer/aerdrv_core.c
index 890efcc..7448052 100644
--- a/drivers/pci/pcie/aer/aerdrv_core.c
+++ b/drivers/pci/pcie/aer/aerdrv_core.c
@@ -390,7 +390,14 @@ static pci_ers_result_t broadcast_error_message(struct
pci_dev *dev,
* If the error is reported by an end point, we think this
* error is related to the upstream link of the end point.
*/
- pci_walk_bus(dev->bus, cb, _data);
+ if (state == pci_channel_io_normal)
+ /*
+* the error is non fatal so the bus is ok, just invoke
+* the callback for the function that logged the error.
+*/
+ cb(dev, _data);
+ else
+ pci_walk_bus(dev->bus, cb, _data);
}
return result_data.result;
--
2.7.4
RE: [PATCH v2] PCIe AER: report uncorrectable errors only to the functions that logged the errors
Hi Bjorn > -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: 25 September 2017 19:34 > To: Gabriele Paoloni > Cc: Linuxarm; liudongdong (C); linux-...@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH v2] PCIe AER: report uncorrectable errors only to > the functions that logged the errors > > On Wed, Sep 06, 2017 at 10:56:42AM +, Gabriele Paoloni wrote: > > Hi Bjorn > > > > > -Original Message- > > > From: Bjorn Helgaas [mailto:helg...@kernel.org] > > > Sent: 02 September 2017 18:33 > > > To: Gabriele Paoloni > > > Cc: Linuxarm; liudongdong (C); linux-...@vger.kernel.org; linux- > > > ker...@vger.kernel.org > > > Subject: Re: [PATCH v2] PCIe AER: report uncorrectable errors only > to > > > the functions that logged the errors > > > > > > On Fri, Sep 01, 2017 at 11:39:35AM +, Gabriele Paoloni wrote: > > > > Hi Bjorn > > > > > > > > Many thanks for looking at this > > > > > > > > > -Original Message- > > > > > From: Bjorn Helgaas [mailto:helg...@kernel.org] > > > > > Sent: 01 September 2017 05:43 > > > > > To: Gabriele Paoloni > > > > > Cc: Linuxarm; liudongdong (C); linux-...@vger.kernel.org; > linux- > > > > > ker...@vger.kernel.org > > > > > Subject: Re: [PATCH v2] PCIe AER: report uncorrectable errors > only > > > to > > > > > the functions that logged the errors > > > > > > > > > > On Thu, Aug 31, 2017 at 03:03:44PM -0500, Bjorn Helgaas wrote: > > > > > > On Fri, Aug 18, 2017 at 12:02:21PM +0100, Gabriele Paoloni > wrote: > > > > > > > Currently if an uncorrectable error is reported by an EP > the > > > AER > > > > > > > driver walks over all the devices connected to the upstream > > > port > > > > > > > bus and in turns call the report_error_detected() callback. > > > > > > > If any of the devices connected to the bus does not > implement > > > > > > > dev->driver->err_handler->error_detected() do_recovery() > will > > > fail > > > > > > > leaving all the bus hierarchy devices unrecovered. > > > > > > > > > > > > > > However for non fatal errors the PCIe link should not be > > > considered > > > > > > > compromised, therefore it makes sense to report the error > only > > > to > > > > > > > all the functions that logged an error. > > > > > > > > > > > > Can you include a pointer to the relevant part of the spec > here? > > > > > > > > Sure > > > > According to section "6.2.2.2.2. Non-Fatal Errors" > > > > << Non-fatal errors are uncorrectable errors which cause a > particular > > > > transaction to be unreliable but the Link is otherwise fully > > > functional. > > > > Isolating Non-fatal from Fatal errors provides Requester/Receiver > > > logic > > > > in a device or system management software the opportunity to > recover > > > > from the error without resetting the components on the Link and > > > > disturbing other transactions in progress. Devices not associated > > > with > > > > the transaction in error are not impacted by the error.>> > > > > > > > > I will add it to the commit msg: > > > > > > > > > > > > > > Also, I forgot to ask: can you outline the problem this fixes? > I'm > > > > > curious about why this hasn't been an issue in the past. My > guess > > > is > > > > > there's something new about your configuration, and the config > and > > > the > > > > > symptoms might help connect this fix to similar problems. > > > > > > > > I already replied about this in the previous patch... > > > > << In Hi1620 we have some integrated controllers that appear as > PCIe > > > EPs > > > > under the same bus. Some of these controllers (e.g. the SATA > > > > controller) are missing the err_handler callbacks. > > > > > > > > If one device reports a non-fatal uncorrectable error with the > > > current > > > > AER core code the callbacks for all the devices under the same > bus > > > will > > > >
RE: [PATCH v2] PCIe AER: report uncorrectable errors only to the functions that logged the errors
Hi Bjorn > -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: 25 September 2017 19:34 > To: Gabriele Paoloni > Cc: Linuxarm; liudongdong (C); linux-...@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH v2] PCIe AER: report uncorrectable errors only to > the functions that logged the errors > > On Wed, Sep 06, 2017 at 10:56:42AM +, Gabriele Paoloni wrote: > > Hi Bjorn > > > > > -Original Message- > > > From: Bjorn Helgaas [mailto:helg...@kernel.org] > > > Sent: 02 September 2017 18:33 > > > To: Gabriele Paoloni > > > Cc: Linuxarm; liudongdong (C); linux-...@vger.kernel.org; linux- > > > ker...@vger.kernel.org > > > Subject: Re: [PATCH v2] PCIe AER: report uncorrectable errors only > to > > > the functions that logged the errors > > > > > > On Fri, Sep 01, 2017 at 11:39:35AM +, Gabriele Paoloni wrote: > > > > Hi Bjorn > > > > > > > > Many thanks for looking at this > > > > > > > > > -Original Message- > > > > > From: Bjorn Helgaas [mailto:helg...@kernel.org] > > > > > Sent: 01 September 2017 05:43 > > > > > To: Gabriele Paoloni > > > > > Cc: Linuxarm; liudongdong (C); linux-...@vger.kernel.org; > linux- > > > > > ker...@vger.kernel.org > > > > > Subject: Re: [PATCH v2] PCIe AER: report uncorrectable errors > only > > > to > > > > > the functions that logged the errors > > > > > > > > > > On Thu, Aug 31, 2017 at 03:03:44PM -0500, Bjorn Helgaas wrote: > > > > > > On Fri, Aug 18, 2017 at 12:02:21PM +0100, Gabriele Paoloni > wrote: > > > > > > > Currently if an uncorrectable error is reported by an EP > the > > > AER > > > > > > > driver walks over all the devices connected to the upstream > > > port > > > > > > > bus and in turns call the report_error_detected() callback. > > > > > > > If any of the devices connected to the bus does not > implement > > > > > > > dev->driver->err_handler->error_detected() do_recovery() > will > > > fail > > > > > > > leaving all the bus hierarchy devices unrecovered. > > > > > > > > > > > > > > However for non fatal errors the PCIe link should not be > > > considered > > > > > > > compromised, therefore it makes sense to report the error > only > > > to > > > > > > > all the functions that logged an error. > > > > > > > > > > > > Can you include a pointer to the relevant part of the spec > here? > > > > > > > > Sure > > > > According to section "6.2.2.2.2. Non-Fatal Errors" > > > > << Non-fatal errors are uncorrectable errors which cause a > particular > > > > transaction to be unreliable but the Link is otherwise fully > > > functional. > > > > Isolating Non-fatal from Fatal errors provides Requester/Receiver > > > logic > > > > in a device or system management software the opportunity to > recover > > > > from the error without resetting the components on the Link and > > > > disturbing other transactions in progress. Devices not associated > > > with > > > > the transaction in error are not impacted by the error.>> > > > > > > > > I will add it to the commit msg: > > > > > > > > > > > > > > Also, I forgot to ask: can you outline the problem this fixes? > I'm > > > > > curious about why this hasn't been an issue in the past. My > guess > > > is > > > > > there's something new about your configuration, and the config > and > > > the > > > > > symptoms might help connect this fix to similar problems. > > > > > > > > I already replied about this in the previous patch... > > > > << In Hi1620 we have some integrated controllers that appear as > PCIe > > > EPs > > > > under the same bus. Some of these controllers (e.g. the SATA > > > > controller) are missing the err_handler callbacks. > > > > > > > > If one device reports a non-fatal uncorrectable error with the > > > current > > > > AER core code the callbacks for all the devices under the same > bus > > > will > > > >
RE: [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM-specific register range for ACPI case
Hi Ard Sorry to reply late on this > -Original Message- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: 14 September 2017 15:07 > To: Gabriele Paoloni > Cc: Lorenzo Pieralisi; Hanjun Guo; Marcin Wojtas; Wangyijing; linux- > p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; liudongdong > (C); linux-a...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM- > specific register range for ACPI case > > (remove most cc's) > > Hi all, > > Apologies for digging up this old thread. > > I am currently looking into whether it is feasible to refactor some of > the ACPI PCI quirk code we have so it can apply to more instances of > the Synopsys Designware PCIe (i.e., for Marvell and Socionext SOCs), > which all suffer from the same issue that type 0 config TLPs are not > filtered before being sent out on the link, resulting in devices > appearing multiple times. What do you mean by "filtered"? Looking at https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f00f1a0178cf52928366a5e1f376a65f1f3f389 you can see that the issue is that accesses to the root buses config space are done using a specific HW location rather than the ECAM space area (used for the rest of the hierarchy). Also only 32b accessors can operate on the root bus config space. > > The pci-hisi.c quirk already appears to do exactly what would solve > the problem on other SoCs as well. So I will try to refactor this code > into something that I could propose for upstream, while probably > getting the ARM SBSA authors to offer some guidance that this IP must > not be used for new designs. Mmm I am not sure how Kernel maintainers would open to accept more quirks... as I remember we discussed to accept quirks only for existing platform but not for any future one (i.e. future HW should not use the non-ECAM DW IP). However this is my understanding, I may be wrong here... > > In any case, the chances of any such changes being acceptable upstream > are probably higher if we can reuse the exact same quirk as we have > already implemented for HiSilicon, so I wonder if anyone from Huawei > could comment on some questions I have: > - Why does the config space accessor override use 32-bit accesses only > for bus #0? Is that really necessary? I have talked to our SoC HW designers and they said that 32b accesses is a constraint due to Synopsys DW IP limitation and yes it is necessary > - Why does the Hisi quirk use different config base addresses for bus > #0 and the other buses? Is this simply because the firmware does not > use contiguous iATU windows for bus #0 and buses #1 and up? So is As I remember there is a dedicated continuous device memory slice that is dedicated for all the root buses (therefore you cannot spilt such area to have root bus slices contiguous with the different ECAM areas of each RP hierarchy) > there anything mapped in the 1 MB slice at the base of the respective > 256 MB mappings that generate type1 config cycles? If I remember correctly we map device memory to the 1MB slice but we never access it (instead we in the accessors we use cfg->busr.start to understand is the current cfg rd/wr if being done on a root bus and eventually rout the access to the dedicate root bus cfg space) > > Having just a shared set of config space accessor overrides would > already be an improvement, given that they can be shared with a DT > based driver for this IP in ECAM mode as well. Probably we could have shared accessors, but again we need to see if the community is open to accept more quirks Thanks Gab > > Thanks, > Ard.
RE: [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM-specific register range for ACPI case
Hi Ard Sorry to reply late on this > -Original Message- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: 14 September 2017 15:07 > To: Gabriele Paoloni > Cc: Lorenzo Pieralisi; Hanjun Guo; Marcin Wojtas; Wangyijing; linux- > p...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; liudongdong > (C); linux-a...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM- > specific register range for ACPI case > > (remove most cc's) > > Hi all, > > Apologies for digging up this old thread. > > I am currently looking into whether it is feasible to refactor some of > the ACPI PCI quirk code we have so it can apply to more instances of > the Synopsys Designware PCIe (i.e., for Marvell and Socionext SOCs), > which all suffer from the same issue that type 0 config TLPs are not > filtered before being sent out on the link, resulting in devices > appearing multiple times. What do you mean by "filtered"? Looking at https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f00f1a0178cf52928366a5e1f376a65f1f3f389 you can see that the issue is that accesses to the root buses config space are done using a specific HW location rather than the ECAM space area (used for the rest of the hierarchy). Also only 32b accessors can operate on the root bus config space. > > The pci-hisi.c quirk already appears to do exactly what would solve > the problem on other SoCs as well. So I will try to refactor this code > into something that I could propose for upstream, while probably > getting the ARM SBSA authors to offer some guidance that this IP must > not be used for new designs. Mmm I am not sure how Kernel maintainers would open to accept more quirks... as I remember we discussed to accept quirks only for existing platform but not for any future one (i.e. future HW should not use the non-ECAM DW IP). However this is my understanding, I may be wrong here... > > In any case, the chances of any such changes being acceptable upstream > are probably higher if we can reuse the exact same quirk as we have > already implemented for HiSilicon, so I wonder if anyone from Huawei > could comment on some questions I have: > - Why does the config space accessor override use 32-bit accesses only > for bus #0? Is that really necessary? I have talked to our SoC HW designers and they said that 32b accesses is a constraint due to Synopsys DW IP limitation and yes it is necessary > - Why does the Hisi quirk use different config base addresses for bus > #0 and the other buses? Is this simply because the firmware does not > use contiguous iATU windows for bus #0 and buses #1 and up? So is As I remember there is a dedicated continuous device memory slice that is dedicated for all the root buses (therefore you cannot spilt such area to have root bus slices contiguous with the different ECAM areas of each RP hierarchy) > there anything mapped in the 1 MB slice at the base of the respective > 256 MB mappings that generate type1 config cycles? If I remember correctly we map device memory to the 1MB slice but we never access it (instead we in the accessors we use cfg->busr.start to understand is the current cfg rd/wr if being done on a root bus and eventually rout the access to the dedicate root bus cfg space) > > Having just a shared set of config space accessor overrides would > already be an improvement, given that they can be shared with a DT > based driver for this IP in ECAM mode as well. Probably we could have shared accessors, but again we need to see if the community is open to accept more quirks Thanks Gab > > Thanks, > Ard.
RE: [PATCH v2] PCIe AER: report uncorrectable errors only to the functions that logged the errors
Hi Bjorn > -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: 02 September 2017 18:33 > To: Gabriele Paoloni > Cc: Linuxarm; liudongdong (C); linux-...@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH v2] PCIe AER: report uncorrectable errors only to > the functions that logged the errors > > On Fri, Sep 01, 2017 at 11:39:35AM +, Gabriele Paoloni wrote: > > Hi Bjorn > > > > Many thanks for looking at this > > > > > -Original Message- > > > From: Bjorn Helgaas [mailto:helg...@kernel.org] > > > Sent: 01 September 2017 05:43 > > > To: Gabriele Paoloni > > > Cc: Linuxarm; liudongdong (C); linux-...@vger.kernel.org; linux- > > > ker...@vger.kernel.org > > > Subject: Re: [PATCH v2] PCIe AER: report uncorrectable errors only > to > > > the functions that logged the errors > > > > > > On Thu, Aug 31, 2017 at 03:03:44PM -0500, Bjorn Helgaas wrote: > > > > On Fri, Aug 18, 2017 at 12:02:21PM +0100, Gabriele Paoloni wrote: > > > > > Currently if an uncorrectable error is reported by an EP the > AER > > > > > driver walks over all the devices connected to the upstream > port > > > > > bus and in turns call the report_error_detected() callback. > > > > > If any of the devices connected to the bus does not implement > > > > > dev->driver->err_handler->error_detected() do_recovery() will > fail > > > > > leaving all the bus hierarchy devices unrecovered. > > > > > > > > > > However for non fatal errors the PCIe link should not be > considered > > > > > compromised, therefore it makes sense to report the error only > to > > > > > all the functions that logged an error. > > > > > > > > Can you include a pointer to the relevant part of the spec here? > > > > Sure > > According to section "6.2.2.2.2. Non-Fatal Errors" > > << Non-fatal errors are uncorrectable errors which cause a particular > > transaction to be unreliable but the Link is otherwise fully > functional. > > Isolating Non-fatal from Fatal errors provides Requester/Receiver > logic > > in a device or system management software the opportunity to recover > > from the error without resetting the components on the Link and > > disturbing other transactions in progress. Devices not associated > with > > the transaction in error are not impacted by the error.>> > > > > I will add it to the commit msg: > > > > > > > > Also, I forgot to ask: can you outline the problem this fixes? I'm > > > curious about why this hasn't been an issue in the past. My guess > is > > > there's something new about your configuration, and the config and > the > > > symptoms might help connect this fix to similar problems. > > > > I already replied about this in the previous patch... > > << In Hi1620 we have some integrated controllers that appear as PCIe > EPs > > under the same bus. Some of these controllers (e.g. the SATA > > controller) are missing the err_handler callbacks. > > > > If one device reports a non-fatal uncorrectable error with the > current > > AER core code the callbacks for all the devices under the same bus > will > > be called and, if any of the devices is missing the callback all the > > devices in the subtree are left in error state without recovery... > > This patch is needed to sort out a situation like this one.>> > > > > Should I add this to the commit msg? > > Thanks for the reminder. I thought I remembered some details but > hadn't dug them out again. Yes, I was hoping for something we could > include in the commit message. I'm still not sure what specifically > is *new* about this situation. Maybe the particular mix of functions > in a multi-function device? Maybe the fact that you're seeing more > AER errors than before (or maybe just doing more testing?) Hi Bjorn as I said here we have some integrated controllers that appear as PCIe EPs under the same bus. Usually PCIe is p2p (no more that 1 device per bus), but in our SoC we have different devices under the same bus (not MF devices): RC---bus0---|-SAS ctrl | |-SATA ctrl | |-XGE ctrl Since this is unusual this is maybe the reason why this has not been seen yet elsewhere... > > Since this is actually a bug fix, this might be a good opportunity to > open a bugzilla for it. Then we have a place to attach the complete > "l
RE: [PATCH v2] PCIe AER: report uncorrectable errors only to the functions that logged the errors
Hi Bjorn > -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: 02 September 2017 18:33 > To: Gabriele Paoloni > Cc: Linuxarm; liudongdong (C); linux-...@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH v2] PCIe AER: report uncorrectable errors only to > the functions that logged the errors > > On Fri, Sep 01, 2017 at 11:39:35AM +, Gabriele Paoloni wrote: > > Hi Bjorn > > > > Many thanks for looking at this > > > > > -Original Message- > > > From: Bjorn Helgaas [mailto:helg...@kernel.org] > > > Sent: 01 September 2017 05:43 > > > To: Gabriele Paoloni > > > Cc: Linuxarm; liudongdong (C); linux-...@vger.kernel.org; linux- > > > ker...@vger.kernel.org > > > Subject: Re: [PATCH v2] PCIe AER: report uncorrectable errors only > to > > > the functions that logged the errors > > > > > > On Thu, Aug 31, 2017 at 03:03:44PM -0500, Bjorn Helgaas wrote: > > > > On Fri, Aug 18, 2017 at 12:02:21PM +0100, Gabriele Paoloni wrote: > > > > > Currently if an uncorrectable error is reported by an EP the > AER > > > > > driver walks over all the devices connected to the upstream > port > > > > > bus and in turns call the report_error_detected() callback. > > > > > If any of the devices connected to the bus does not implement > > > > > dev->driver->err_handler->error_detected() do_recovery() will > fail > > > > > leaving all the bus hierarchy devices unrecovered. > > > > > > > > > > However for non fatal errors the PCIe link should not be > considered > > > > > compromised, therefore it makes sense to report the error only > to > > > > > all the functions that logged an error. > > > > > > > > Can you include a pointer to the relevant part of the spec here? > > > > Sure > > According to section "6.2.2.2.2. Non-Fatal Errors" > > << Non-fatal errors are uncorrectable errors which cause a particular > > transaction to be unreliable but the Link is otherwise fully > functional. > > Isolating Non-fatal from Fatal errors provides Requester/Receiver > logic > > in a device or system management software the opportunity to recover > > from the error without resetting the components on the Link and > > disturbing other transactions in progress. Devices not associated > with > > the transaction in error are not impacted by the error.>> > > > > I will add it to the commit msg: > > > > > > > > Also, I forgot to ask: can you outline the problem this fixes? I'm > > > curious about why this hasn't been an issue in the past. My guess > is > > > there's something new about your configuration, and the config and > the > > > symptoms might help connect this fix to similar problems. > > > > I already replied about this in the previous patch... > > << In Hi1620 we have some integrated controllers that appear as PCIe > EPs > > under the same bus. Some of these controllers (e.g. the SATA > > controller) are missing the err_handler callbacks. > > > > If one device reports a non-fatal uncorrectable error with the > current > > AER core code the callbacks for all the devices under the same bus > will > > be called and, if any of the devices is missing the callback all the > > devices in the subtree are left in error state without recovery... > > This patch is needed to sort out a situation like this one.>> > > > > Should I add this to the commit msg? > > Thanks for the reminder. I thought I remembered some details but > hadn't dug them out again. Yes, I was hoping for something we could > include in the commit message. I'm still not sure what specifically > is *new* about this situation. Maybe the particular mix of functions > in a multi-function device? Maybe the fact that you're seeing more > AER errors than before (or maybe just doing more testing?) Hi Bjorn as I said here we have some integrated controllers that appear as PCIe EPs under the same bus. Usually PCIe is p2p (no more that 1 device per bus), but in our SoC we have different devices under the same bus (not MF devices): RC---bus0---|-SAS ctrl | |-SATA ctrl | |-XGE ctrl Since this is unusual this is maybe the reason why this has not been seen yet elsewhere... > > Since this is actually a bug fix, this might be a good opportunity to > open a bugzilla for it. Then we have a place to attach the complete > "lspci -vv&q
RE: [PATCH v2] PCIe AER: report uncorrectable errors only to the functions that logged the errors
Hi Bjorn Many thanks for looking at this > -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: 01 September 2017 05:43 > To: Gabriele Paoloni > Cc: Linuxarm; liudongdong (C); linux-...@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH v2] PCIe AER: report uncorrectable errors only to > the functions that logged the errors > > On Thu, Aug 31, 2017 at 03:03:44PM -0500, Bjorn Helgaas wrote: > > On Fri, Aug 18, 2017 at 12:02:21PM +0100, Gabriele Paoloni wrote: > > > Currently if an uncorrectable error is reported by an EP the AER > > > driver walks over all the devices connected to the upstream port > > > bus and in turns call the report_error_detected() callback. > > > If any of the devices connected to the bus does not implement > > > dev->driver->err_handler->error_detected() do_recovery() will fail > > > leaving all the bus hierarchy devices unrecovered. > > > > > > However for non fatal errors the PCIe link should not be considered > > > compromised, therefore it makes sense to report the error only to > > > all the functions that logged an error. > > > > Can you include a pointer to the relevant part of the spec here? Sure According to section "6.2.2.2.2. Non-Fatal Errors" << Non-fatal errors are uncorrectable errors which cause a particular transaction to be unreliable but the Link is otherwise fully functional. Isolating Non-fatal from Fatal errors provides Requester/Receiver logic in a device or system management software the opportunity to recover from the error without resetting the components on the Link and disturbing other transactions in progress. Devices not associated with the transaction in error are not impacted by the error.>> I will add it to the commit msg: > > Also, I forgot to ask: can you outline the problem this fixes? I'm > curious about why this hasn't been an issue in the past. My guess is > there's something new about your configuration, and the config and the > symptoms might help connect this fix to similar problems. I already replied about this in the previous patch... << In Hi1620 we have some integrated controllers that appear as PCIe EPs under the same bus. Some of these controllers (e.g. the SATA controller) are missing the err_handler callbacks. If one device reports a non-fatal uncorrectable error with the current AER core code the callbacks for all the devices under the same bus will be called and, if any of the devices is missing the callback all the devices in the subtree are left in error state without recovery... This patch is needed to sort out a situation like this one.>> Should I add this to the commit msg? > > > > This patch implements this new behaviour for non fatal errors. > > > > > > Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com> > > > Signed-off-by: Dongdong Liu <liudongdo...@huawei.com> > > > --- > > > Changes from v1: > > >- now errors are reported only to the fucntions that logged the > error > > > instead of all the functions in the same device. > > >- the patch subject has changed to match the new implementation > > > --- > > > drivers/pci/pcie/aer/aerdrv_core.c | 9 - > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/pci/pcie/aer/aerdrv_core.c > b/drivers/pci/pcie/aer/aerdrv_core.c > > > index b1303b3..057465ad 100644 > > > --- a/drivers/pci/pcie/aer/aerdrv_core.c > > > +++ b/drivers/pci/pcie/aer/aerdrv_core.c > > > @@ -390,7 +390,14 @@ static pci_ers_result_t > broadcast_error_message(struct pci_dev *dev, > > >* If the error is reported by an end point, we think this > > >* error is related to the upstream link of the end point. > > >*/ > > > - pci_walk_bus(dev->bus, cb, _data); > > > + if (state == pci_channel_io_normal) > > > + /* > > > + * the error is non fatal so the bus is ok, just > invoke > > > + * the callback for the function that logged the > error. > > > + */ > > > + cb(dev, _data); > > > + else > > > + pci_walk_bus(dev->bus, cb, _data); > > > > I think the concept of this change makes sense, but I don't like the > > implicit connection of PCI_ERR_ROOT_UNCOR_RCV -> AER_NONFATAL -> > > pci_channel_io_normal. That makes it harder than it should be to > read > > the code. > > > > What would you thi
RE: [PATCH v2] PCIe AER: report uncorrectable errors only to the functions that logged the errors
Hi Bjorn Many thanks for looking at this > -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: 01 September 2017 05:43 > To: Gabriele Paoloni > Cc: Linuxarm; liudongdong (C); linux-...@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH v2] PCIe AER: report uncorrectable errors only to > the functions that logged the errors > > On Thu, Aug 31, 2017 at 03:03:44PM -0500, Bjorn Helgaas wrote: > > On Fri, Aug 18, 2017 at 12:02:21PM +0100, Gabriele Paoloni wrote: > > > Currently if an uncorrectable error is reported by an EP the AER > > > driver walks over all the devices connected to the upstream port > > > bus and in turns call the report_error_detected() callback. > > > If any of the devices connected to the bus does not implement > > > dev->driver->err_handler->error_detected() do_recovery() will fail > > > leaving all the bus hierarchy devices unrecovered. > > > > > > However for non fatal errors the PCIe link should not be considered > > > compromised, therefore it makes sense to report the error only to > > > all the functions that logged an error. > > > > Can you include a pointer to the relevant part of the spec here? Sure According to section "6.2.2.2.2. Non-Fatal Errors" << Non-fatal errors are uncorrectable errors which cause a particular transaction to be unreliable but the Link is otherwise fully functional. Isolating Non-fatal from Fatal errors provides Requester/Receiver logic in a device or system management software the opportunity to recover from the error without resetting the components on the Link and disturbing other transactions in progress. Devices not associated with the transaction in error are not impacted by the error.>> I will add it to the commit msg: > > Also, I forgot to ask: can you outline the problem this fixes? I'm > curious about why this hasn't been an issue in the past. My guess is > there's something new about your configuration, and the config and the > symptoms might help connect this fix to similar problems. I already replied about this in the previous patch... << In Hi1620 we have some integrated controllers that appear as PCIe EPs under the same bus. Some of these controllers (e.g. the SATA controller) are missing the err_handler callbacks. If one device reports a non-fatal uncorrectable error with the current AER core code the callbacks for all the devices under the same bus will be called and, if any of the devices is missing the callback all the devices in the subtree are left in error state without recovery... This patch is needed to sort out a situation like this one.>> Should I add this to the commit msg? > > > > This patch implements this new behaviour for non fatal errors. > > > > > > Signed-off-by: Gabriele Paoloni > > > Signed-off-by: Dongdong Liu > > > --- > > > Changes from v1: > > >- now errors are reported only to the fucntions that logged the > error > > > instead of all the functions in the same device. > > >- the patch subject has changed to match the new implementation > > > --- > > > drivers/pci/pcie/aer/aerdrv_core.c | 9 - > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/pci/pcie/aer/aerdrv_core.c > b/drivers/pci/pcie/aer/aerdrv_core.c > > > index b1303b3..057465ad 100644 > > > --- a/drivers/pci/pcie/aer/aerdrv_core.c > > > +++ b/drivers/pci/pcie/aer/aerdrv_core.c > > > @@ -390,7 +390,14 @@ static pci_ers_result_t > broadcast_error_message(struct pci_dev *dev, > > >* If the error is reported by an end point, we think this > > >* error is related to the upstream link of the end point. > > >*/ > > > - pci_walk_bus(dev->bus, cb, _data); > > > + if (state == pci_channel_io_normal) > > > + /* > > > + * the error is non fatal so the bus is ok, just > invoke > > > + * the callback for the function that logged the > error. > > > + */ > > > + cb(dev, _data); > > > + else > > > + pci_walk_bus(dev->bus, cb, _data); > > > > I think the concept of this change makes sense, but I don't like the > > implicit connection of PCI_ERR_ROOT_UNCOR_RCV -> AER_NONFATAL -> > > pci_channel_io_normal. That makes it harder than it should be to > read > > the code. > > > > What would you think of changing the signature of do_recovery() and >
RE: [PATCH v2] PCIe AER: report uncorrectable errors only to the functions that logged the errors
ping... > -Original Message- > From: Gabriele Paoloni > Sent: 18 August 2017 12:02 > To: helg...@kernel.org > Cc: Gabriele Paoloni; Linuxarm; liudongdong (C); linux- > p...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: [PATCH v2] PCIe AER: report uncorrectable errors only to the > functions that logged the errors > > Currently if an uncorrectable error is reported by an EP the AER > driver walks over all the devices connected to the upstream port > bus and in turns call the report_error_detected() callback. > If any of the devices connected to the bus does not implement > dev->driver->err_handler->error_detected() do_recovery() will fail > leaving all the bus hierarchy devices unrecovered. > > However for non fatal errors the PCIe link should not be considered > compromised, therefore it makes sense to report the error only to > all the functions that logged an error. > This patch implements this new behaviour for non fatal errors. > > Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com> > Signed-off-by: Dongdong Liu <liudongdo...@huawei.com> > --- > Changes from v1: >- now errors are reported only to the fucntions that logged the > error > instead of all the functions in the same device. >- the patch subject has changed to match the new implementation > --- > drivers/pci/pcie/aer/aerdrv_core.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pcie/aer/aerdrv_core.c > b/drivers/pci/pcie/aer/aerdrv_core.c > index b1303b3..057465ad 100644 > --- a/drivers/pci/pcie/aer/aerdrv_core.c > +++ b/drivers/pci/pcie/aer/aerdrv_core.c > @@ -390,7 +390,14 @@ static pci_ers_result_t > broadcast_error_message(struct pci_dev *dev, >* If the error is reported by an end point, we think this >* error is related to the upstream link of the end point. >*/ > - pci_walk_bus(dev->bus, cb, _data); > + if (state == pci_channel_io_normal) > + /* > + * the error is non fatal so the bus is ok, just > invoke > + * the callback for the function that logged the > error. > + */ > + cb(dev, _data); > + else > + pci_walk_bus(dev->bus, cb, _data); > } > > return result_data.result; > -- > 2.7.4 >
RE: [PATCH v2] PCIe AER: report uncorrectable errors only to the functions that logged the errors
ping... > -Original Message- > From: Gabriele Paoloni > Sent: 18 August 2017 12:02 > To: helg...@kernel.org > Cc: Gabriele Paoloni; Linuxarm; liudongdong (C); linux- > p...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: [PATCH v2] PCIe AER: report uncorrectable errors only to the > functions that logged the errors > > Currently if an uncorrectable error is reported by an EP the AER > driver walks over all the devices connected to the upstream port > bus and in turns call the report_error_detected() callback. > If any of the devices connected to the bus does not implement > dev->driver->err_handler->error_detected() do_recovery() will fail > leaving all the bus hierarchy devices unrecovered. > > However for non fatal errors the PCIe link should not be considered > compromised, therefore it makes sense to report the error only to > all the functions that logged an error. > This patch implements this new behaviour for non fatal errors. > > Signed-off-by: Gabriele Paoloni > Signed-off-by: Dongdong Liu > --- > Changes from v1: >- now errors are reported only to the fucntions that logged the > error > instead of all the functions in the same device. >- the patch subject has changed to match the new implementation > --- > drivers/pci/pcie/aer/aerdrv_core.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pcie/aer/aerdrv_core.c > b/drivers/pci/pcie/aer/aerdrv_core.c > index b1303b3..057465ad 100644 > --- a/drivers/pci/pcie/aer/aerdrv_core.c > +++ b/drivers/pci/pcie/aer/aerdrv_core.c > @@ -390,7 +390,14 @@ static pci_ers_result_t > broadcast_error_message(struct pci_dev *dev, >* If the error is reported by an end point, we think this >* error is related to the upstream link of the end point. >*/ > - pci_walk_bus(dev->bus, cb, _data); > + if (state == pci_channel_io_normal) > + /* > + * the error is non fatal so the bus is ok, just > invoke > + * the callback for the function that logged the > error. > + */ > + cb(dev, _data); > + else > + pci_walk_bus(dev->bus, cb, _data); > } > > return result_data.result; > -- > 2.7.4 >
[PATCH v2] PCIe AER: report uncorrectable errors only to the functions that logged the errors
Currently if an uncorrectable error is reported by an EP the AER driver walks over all the devices connected to the upstream port bus and in turns call the report_error_detected() callback. If any of the devices connected to the bus does not implement dev->driver->err_handler->error_detected() do_recovery() will fail leaving all the bus hierarchy devices unrecovered. However for non fatal errors the PCIe link should not be considered compromised, therefore it makes sense to report the error only to all the functions that logged an error. This patch implements this new behaviour for non fatal errors. Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com> Signed-off-by: Dongdong Liu <liudongdo...@huawei.com> --- Changes from v1: - now errors are reported only to the fucntions that logged the error instead of all the functions in the same device. - the patch subject has changed to match the new implementation --- drivers/pci/pcie/aer/aerdrv_core.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c index b1303b3..057465ad 100644 --- a/drivers/pci/pcie/aer/aerdrv_core.c +++ b/drivers/pci/pcie/aer/aerdrv_core.c @@ -390,7 +390,14 @@ static pci_ers_result_t broadcast_error_message(struct pci_dev *dev, * If the error is reported by an end point, we think this * error is related to the upstream link of the end point. */ - pci_walk_bus(dev->bus, cb, _data); + if (state == pci_channel_io_normal) + /* +* the error is non fatal so the bus is ok, just invoke +* the callback for the function that logged the error. +*/ + cb(dev, _data); + else + pci_walk_bus(dev->bus, cb, _data); } return result_data.result; -- 2.7.4
[PATCH v2] PCIe AER: report uncorrectable errors only to the functions that logged the errors
Currently if an uncorrectable error is reported by an EP the AER driver walks over all the devices connected to the upstream port bus and in turns call the report_error_detected() callback. If any of the devices connected to the bus does not implement dev->driver->err_handler->error_detected() do_recovery() will fail leaving all the bus hierarchy devices unrecovered. However for non fatal errors the PCIe link should not be considered compromised, therefore it makes sense to report the error only to all the functions that logged an error. This patch implements this new behaviour for non fatal errors. Signed-off-by: Gabriele Paoloni Signed-off-by: Dongdong Liu --- Changes from v1: - now errors are reported only to the fucntions that logged the error instead of all the functions in the same device. - the patch subject has changed to match the new implementation --- drivers/pci/pcie/aer/aerdrv_core.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c index b1303b3..057465ad 100644 --- a/drivers/pci/pcie/aer/aerdrv_core.c +++ b/drivers/pci/pcie/aer/aerdrv_core.c @@ -390,7 +390,14 @@ static pci_ers_result_t broadcast_error_message(struct pci_dev *dev, * If the error is reported by an end point, we think this * error is related to the upstream link of the end point. */ - pci_walk_bus(dev->bus, cb, _data); + if (state == pci_channel_io_normal) + /* +* the error is non fatal so the bus is ok, just invoke +* the callback for the function that logged the error. +*/ + cb(dev, _data); + else + pci_walk_bus(dev->bus, cb, _data); } return result_data.result; -- 2.7.4
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Andy [...] > > You may do it on stack. Define your cell statically (but not const) > and apply resources just before mfd_add_devices() call. Ok thanks got it > There are examples in the existing drivers. Intel LPC comes to my mind > and perhaps PMC (Broxton), though latter has too much other stuff > around. Uh yes I see now in lpc_ich.c (base address is read from PCI config space and resources are set accordingly). Cheers Gab > > -- > With Best Regards, > Andy Shevchenko
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Andy [...] > > You may do it on stack. Define your cell statically (but not const) > and apply resources just before mfd_add_devices() call. Ok thanks got it > There are examples in the existing drivers. Intel LPC comes to my mind > and perhaps PMC (Broxton), though latter has too much other stuff > around. Uh yes I see now in lpc_ich.c (base address is read from PCI config space and resources are set accordingly). Cheers Gab > > -- > With Best Regards, > Andy Shevchenko
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Andy [...] > > JFYI: Mika on vacation. Thanks for letting me know > > > I had a look into the MFD framework. If my understanding is correct > the mfd > > framework create a platform device for each declared mfd_cell that is > passed > > to mfd_add_devices(). > > Right. > > > However there is something that I do not quite understand: > > from > > http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd- > core.c#L207 > > it seems that mfd_add_device() will create the platform device using > the > > resources that are statically declared in the respective mfd_cell. > > It's one possibility. > > > In my case I'd like to have a platform device using the resources > that are > > parsed from the ACPI table (i.e. as it is done now by > > acpi_create_platform_device()). > > So far so good. Nothing prevents you to do that. > > > If my understanding is correct, if I declared an mfd_cell for my IPMI > child > > the mfd subsystem would create a platform device for such child and > > therefore acpi_create_platform_device() would fail to create a new > platform > > device as adev->physical_node_count will be non zero. > > However as things stand now mfd_cell devices can only use the > resources > > that are statically defined in the code (and therefore not the ones > in the > > ACPI nodes)...am I right? > > You may file resources first and then register MFD cells. See many > existing examples in the kernel. Well I had a look around the Kernel I have seen no mfd cells using Resources that are not statically defined: i.e. cell->resources in mfd_add_device() always points to statically defined resource structures. Usually for ACPI devices first you need to parse the ACPI resources from the table calling acpi_dev_get_resources(), then you iterate over the resource list and fill the resource array by calling acpi_platform_fill_resurces() (as in acpi_create_platform_device()) With respect to my case are you suggesting dynamically allocate a resource array and fill it using the same fashion as acpi_create_platform_device(), then point cell->resources to such array before calling mfd_add_device() ? Thanks Gab > > -- > With Best Regards, > Andy Shevchenko
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Andy [...] > > JFYI: Mika on vacation. Thanks for letting me know > > > I had a look into the MFD framework. If my understanding is correct > the mfd > > framework create a platform device for each declared mfd_cell that is > passed > > to mfd_add_devices(). > > Right. > > > However there is something that I do not quite understand: > > from > > http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd- > core.c#L207 > > it seems that mfd_add_device() will create the platform device using > the > > resources that are statically declared in the respective mfd_cell. > > It's one possibility. > > > In my case I'd like to have a platform device using the resources > that are > > parsed from the ACPI table (i.e. as it is done now by > > acpi_create_platform_device()). > > So far so good. Nothing prevents you to do that. > > > If my understanding is correct, if I declared an mfd_cell for my IPMI > child > > the mfd subsystem would create a platform device for such child and > > therefore acpi_create_platform_device() would fail to create a new > platform > > device as adev->physical_node_count will be non zero. > > However as things stand now mfd_cell devices can only use the > resources > > that are statically defined in the code (and therefore not the ones > in the > > ACPI nodes)...am I right? > > You may file resources first and then register MFD cells. See many > existing examples in the kernel. Well I had a look around the Kernel I have seen no mfd cells using Resources that are not statically defined: i.e. cell->resources in mfd_add_device() always points to statically defined resource structures. Usually for ACPI devices first you need to parse the ACPI resources from the table calling acpi_dev_get_resources(), then you iterate over the resource list and fill the resource array by calling acpi_platform_fill_resurces() (as in acpi_create_platform_device()) With respect to my case are you suggesting dynamically allocate a resource array and fill it using the same fashion as acpi_create_platform_device(), then point cell->resources to such array before calling mfd_add_device() ? Thanks Gab > > -- > With Best Regards, > Andy Shevchenko
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
+CC Lee Jones > -Original Message- > From: Gabriele Paoloni > Sent: 03 July 2017 17:08 > To: Gabriele Paoloni; Mika Westerberg > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > Hi Mika > > > -Original Message- > > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- > > ow...@vger.kernel.org] On Behalf Of Gabriele Paoloni > > Sent: 19 June 2017 11:05 > > To: Mika Westerberg > > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux- > arm- > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; > linux- > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > Subject: RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > > devices before scanning > > > > Hi Mika > > > > > -Original Message- > > > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > > > Sent: 19 June 2017 11:02 > > > To: Gabriele Paoloni > > > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux- > > arm- > > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; > > linux- > > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; > linux- > > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non- > MMIO > > > devices before scanning > > > > > > On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > > > > Many thanks for your response and your help here. > > > > > > > > I guess that as conclusion with respect to the current v9 > patchset > > we > > > can > > > > disregard the idea of MFD and modify the current v9 so that it > > > doesn't > > > > touch directly ACPI resources. > > > > Instead as I proposed before we can have the scan handler to > > > enumerate > > > > the children devices and translate its addresses filling dev- > > > >resources[] and > > > > at the same time we can modify acpi_default_enumeration to check > > > > acpi_device_enumerated() before continuing with device > > > enumeration...? > > > > > > > > Do you think it as a viable solution? > > > > > > No, I think MFD + scan handler inside the MFD driver is the way to > > go. > > > We don't want to trash ACPI core with stuff that does not belong > > there > > > IMHO. > > > > Ok Many thanks I will investigate this direction > > I had a look into the MFD framework. If my understanding is correct the > mfd > framework create a platform device for each declared mfd_cell that is > passed > to mfd_add_devices(). > However there is something that I do not quite understand: > from > http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd- > core.c#L207 > it seems that mfd_add_device() will create the platform device using > the > resources that are statically declared in the respective mfd_cell. > > In my case I'd like to have a platform device using the resources that > are > parsed from the ACPI table (i.e. as it is done now by > acpi_create_platform_device()). > > If my understanding is correct, if I declared an mfd_cell for my IPMI > child > the mfd subsystem would create a platform device for such child and > therefore acpi_create_platform_device() would fail to create a new > platform > device as adev->physical_node_count will be non zero. > However as things stand now mfd_cell devices can only use the resources > that are statically defined in the code (and therefore not the ones in > the > ACPI nodes)...am I right? > > Thanks > Gab > > > > > > > > > Also you don't need to modify acpi_default_enumeration() because > you > > > can > > > mark your device enumerated in the MFD driver. So all the dirty > > details > > > will be in the MFD driver and not in ACPI core. > > > > Ok got it :) > > > > Cheers > > Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
+CC Lee Jones > -Original Message- > From: Gabriele Paoloni > Sent: 03 July 2017 17:08 > To: Gabriele Paoloni; Mika Westerberg > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > Hi Mika > > > -Original Message- > > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- > > ow...@vger.kernel.org] On Behalf Of Gabriele Paoloni > > Sent: 19 June 2017 11:05 > > To: Mika Westerberg > > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux- > arm- > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; > linux- > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > Subject: RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > > devices before scanning > > > > Hi Mika > > > > > -Original Message- > > > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > > > Sent: 19 June 2017 11:02 > > > To: Gabriele Paoloni > > > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux- > > arm- > > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; > > linux- > > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; > linux- > > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non- > MMIO > > > devices before scanning > > > > > > On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > > > > Many thanks for your response and your help here. > > > > > > > > I guess that as conclusion with respect to the current v9 > patchset > > we > > > can > > > > disregard the idea of MFD and modify the current v9 so that it > > > doesn't > > > > touch directly ACPI resources. > > > > Instead as I proposed before we can have the scan handler to > > > enumerate > > > > the children devices and translate its addresses filling dev- > > > >resources[] and > > > > at the same time we can modify acpi_default_enumeration to check > > > > acpi_device_enumerated() before continuing with device > > > enumeration...? > > > > > > > > Do you think it as a viable solution? > > > > > > No, I think MFD + scan handler inside the MFD driver is the way to > > go. > > > We don't want to trash ACPI core with stuff that does not belong > > there > > > IMHO. > > > > Ok Many thanks I will investigate this direction > > I had a look into the MFD framework. If my understanding is correct the > mfd > framework create a platform device for each declared mfd_cell that is > passed > to mfd_add_devices(). > However there is something that I do not quite understand: > from > http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd- > core.c#L207 > it seems that mfd_add_device() will create the platform device using > the > resources that are statically declared in the respective mfd_cell. > > In my case I'd like to have a platform device using the resources that > are > parsed from the ACPI table (i.e. as it is done now by > acpi_create_platform_device()). > > If my understanding is correct, if I declared an mfd_cell for my IPMI > child > the mfd subsystem would create a platform device for such child and > therefore acpi_create_platform_device() would fail to create a new > platform > device as adev->physical_node_count will be non zero. > However as things stand now mfd_cell devices can only use the resources > that are statically defined in the code (and therefore not the ones in > the > ACPI nodes)...am I right? > > Thanks > Gab > > > > > > > > > Also you don't need to modify acpi_default_enumeration() because > you > > > can > > > mark your device enumerated in the MFD driver. So all the dirty > > details > > > will be in the MFD driver and not in ACPI core. > > > > Ok got it :) > > > > Cheers > > Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- > ow...@vger.kernel.org] On Behalf Of Gabriele Paoloni > Sent: 19 June 2017 11:05 > To: Mika Westerberg > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > Hi Mika > > > -Original Message- > > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > > Sent: 19 June 2017 11:02 > > To: Gabriele Paoloni > > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux- > arm- > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; > linux- > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > > devices before scanning > > > > On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > > > Many thanks for your response and your help here. > > > > > > I guess that as conclusion with respect to the current v9 patchset > we > > can > > > disregard the idea of MFD and modify the current v9 so that it > > doesn't > > > touch directly ACPI resources. > > > Instead as I proposed before we can have the scan handler to > > enumerate > > > the children devices and translate its addresses filling dev- > > >resources[] and > > > at the same time we can modify acpi_default_enumeration to check > > > acpi_device_enumerated() before continuing with device > > enumeration...? > > > > > > Do you think it as a viable solution? > > > > No, I think MFD + scan handler inside the MFD driver is the way to > go. > > We don't want to trash ACPI core with stuff that does not belong > there > > IMHO. > > Ok Many thanks I will investigate this direction I had a look into the MFD framework. If my understanding is correct the mfd framework create a platform device for each declared mfd_cell that is passed to mfd_add_devices(). However there is something that I do not quite understand: from http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd-core.c#L207 it seems that mfd_add_device() will create the platform device using the resources that are statically declared in the respective mfd_cell. In my case I'd like to have a platform device using the resources that are parsed from the ACPI table (i.e. as it is done now by acpi_create_platform_device()). If my understanding is correct, if I declared an mfd_cell for my IPMI child the mfd subsystem would create a platform device for such child and therefore acpi_create_platform_device() would fail to create a new platform device as adev->physical_node_count will be non zero. However as things stand now mfd_cell devices can only use the resources that are statically defined in the code (and therefore not the ones in the ACPI nodes)...am I right? Thanks Gab > > > > > Also you don't need to modify acpi_default_enumeration() because you > > can > > mark your device enumerated in the MFD driver. So all the dirty > details > > will be in the MFD driver and not in ACPI core. > > Ok got it :) > > Cheers > Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci- > ow...@vger.kernel.org] On Behalf Of Gabriele Paoloni > Sent: 19 June 2017 11:05 > To: Mika Westerberg > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > Hi Mika > > > -Original Message- > > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > > Sent: 19 June 2017 11:02 > > To: Gabriele Paoloni > > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux- > arm- > > ker...@lists.infradead.org; mark.rutl...@arm.com; > > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; > linux- > > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > > devices before scanning > > > > On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > > > Many thanks for your response and your help here. > > > > > > I guess that as conclusion with respect to the current v9 patchset > we > > can > > > disregard the idea of MFD and modify the current v9 so that it > > doesn't > > > touch directly ACPI resources. > > > Instead as I proposed before we can have the scan handler to > > enumerate > > > the children devices and translate its addresses filling dev- > > >resources[] and > > > at the same time we can modify acpi_default_enumeration to check > > > acpi_device_enumerated() before continuing with device > > enumeration...? > > > > > > Do you think it as a viable solution? > > > > No, I think MFD + scan handler inside the MFD driver is the way to > go. > > We don't want to trash ACPI core with stuff that does not belong > there > > IMHO. > > Ok Many thanks I will investigate this direction I had a look into the MFD framework. If my understanding is correct the mfd framework create a platform device for each declared mfd_cell that is passed to mfd_add_devices(). However there is something that I do not quite understand: from http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/mfd-core.c#L207 it seems that mfd_add_device() will create the platform device using the resources that are statically declared in the respective mfd_cell. In my case I'd like to have a platform device using the resources that are parsed from the ACPI table (i.e. as it is done now by acpi_create_platform_device()). If my understanding is correct, if I declared an mfd_cell for my IPMI child the mfd subsystem would create a platform device for such child and therefore acpi_create_platform_device() would fail to create a new platform device as adev->physical_node_count will be non zero. However as things stand now mfd_cell devices can only use the resources that are statically defined in the code (and therefore not the ones in the ACPI nodes)...am I right? Thanks Gab > > > > > Also you don't need to modify acpi_default_enumeration() because you > > can > > mark your device enumerated in the MFD driver. So all the dirty > details > > will be in the MFD driver and not in ACPI core. > > Ok got it :) > > Cheers > Gab
RE: [PATCH v5 0/2] add MSI support for PCIe port services and DPC IRQ support
Hi Bjorn [...] ++-- > -- > > 2 files changed, 50 insertions(+), 21 deletions(-) > > Applied to pci/portdrv for v4.13, thanks! > > I tweaked the PME/hotplug no-MSI logic in pcie_init_service_irqs() > because I thought it was getting hard to read with the negations and > conjunctions. Please double-check to make sure I didn't break it. liudongdong tested the patchset as reworked in your branch and it works ok > > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/log/?h= > pci/portdrv > > I also capitalized your name consistently, Gabriele :) Many thanks, next time I should be consistent with myself :) Cheers Gab
RE: [PATCH v5 0/2] add MSI support for PCIe port services and DPC IRQ support
Hi Bjorn [...] ++-- > -- > > 2 files changed, 50 insertions(+), 21 deletions(-) > > Applied to pci/portdrv for v4.13, thanks! > > I tweaked the PME/hotplug no-MSI logic in pcie_init_service_irqs() > because I thought it was getting hard to read with the negations and > conjunctions. Please double-check to make sure I didn't break it. liudongdong tested the patchset as reworked in your branch and it works ok > > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/log/?h= > pci/portdrv > > I also capitalized your name consistently, Gabriele :) Many thanks, next time I should be consistent with myself :) Cheers Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > Sent: 19 June 2017 11:02 > To: Gabriele Paoloni > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > > Many thanks for your response and your help here. > > > > I guess that as conclusion with respect to the current v9 patchset we > can > > disregard the idea of MFD and modify the current v9 so that it > doesn't > > touch directly ACPI resources. > > Instead as I proposed before we can have the scan handler to > enumerate > > the children devices and translate its addresses filling dev- > >resources[] and > > at the same time we can modify acpi_default_enumeration to check > > acpi_device_enumerated() before continuing with device > enumeration...? > > > > Do you think it as a viable solution? > > No, I think MFD + scan handler inside the MFD driver is the way to go. > We don't want to trash ACPI core with stuff that does not belong there > IMHO. Ok Many thanks I will investigate this direction > > Also you don't need to modify acpi_default_enumeration() because you > can > mark your device enumerated in the MFD driver. So all the dirty details > will be in the MFD driver and not in ACPI core. Ok got it :) Cheers Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika > -Original Message- > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > Sent: 19 June 2017 11:02 > To: Gabriele Paoloni > Cc: Rafael J. Wysocki; Lorenzo Pieralisi; Rafael J. Wysocki; > catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Mon, Jun 19, 2017 at 09:50:49AM +, Gabriele Paoloni wrote: > > Many thanks for your response and your help here. > > > > I guess that as conclusion with respect to the current v9 patchset we > can > > disregard the idea of MFD and modify the current v9 so that it > doesn't > > touch directly ACPI resources. > > Instead as I proposed before we can have the scan handler to > enumerate > > the children devices and translate its addresses filling dev- > >resources[] and > > at the same time we can modify acpi_default_enumeration to check > > acpi_device_enumerated() before continuing with device > enumeration...? > > > > Do you think it as a viable solution? > > No, I think MFD + scan handler inside the MFD driver is the way to go. > We don't want to trash ACPI core with stuff that does not belong there > IMHO. Ok Many thanks I will investigate this direction > > Also you don't need to modify acpi_default_enumeration() because you > can > mark your device enumerated in the MFD driver. So all the dirty details > will be in the MFD driver and not in ACPI core. Ok got it :) Cheers Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Rafael, Mika, Lorenzo > -Original Message- > From: rjwyso...@gmail.com [mailto:rjwyso...@gmail.com] On Behalf Of > Rafael J. Wysocki > Sent: 16 June 2017 13:23 > To: Mika Westerberg > Cc: Rafael J. Wysocki; Gabriele Paoloni; Lorenzo Pieralisi; Rafael J. > Wysocki; catalin.mari...@arm.com; will.dea...@arm.com; > robh...@kernel.org; frowand.l...@gmail.com; bhelg...@google.com; > a...@arndb.de; linux-arm-ker...@lists.infradead.org; > mark.rutl...@arm.com; brian.star...@arm.com; o...@lixom.net; > b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux- > a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org; > miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Fri, Jun 16, 2017 at 2:00 PM, Mika Westerberg > <mika.westerb...@linux.intel.com> wrote: > > On Fri, Jun 16, 2017 at 01:24:32PM +0200, Rafael J. Wysocki wrote: > >> > In fact it may be that it is not sufficient in this case because > the > >> > ACPI core might enumerate child devices before the LPC driver even > gets > >> > a chance to probe so you would need to add also scan handler to > the > >> > child devices and mark them already enumerated or something like > that. > >> > >> Or extend the special I2C/SPI handling to them. > > > > Sure but those have I2c/SpiSerialBus() resources which we can use to > > identify them but for the ipmi thing there is nothing else than _HID > so > > we would need to keep a list of such devices in ACPI core. > > OK, so adding a scan handler for that would be the way to go IMO. Many thanks for your response and your help here. I guess that as conclusion with respect to the current v9 patchset we can disregard the idea of MFD and modify the current v9 so that it doesn't touch directly ACPI resources. Instead as I proposed before we can have the scan handler to enumerate the children devices and translate its addresses filling dev->resources[] and at the same time we can modify acpi_default_enumeration to check acpi_device_enumerated() before continuing with device enumeration...? Do you think it as a viable solution? Thanks Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Rafael, Mika, Lorenzo > -Original Message- > From: rjwyso...@gmail.com [mailto:rjwyso...@gmail.com] On Behalf Of > Rafael J. Wysocki > Sent: 16 June 2017 13:23 > To: Mika Westerberg > Cc: Rafael J. Wysocki; Gabriele Paoloni; Lorenzo Pieralisi; Rafael J. > Wysocki; catalin.mari...@arm.com; will.dea...@arm.com; > robh...@kernel.org; frowand.l...@gmail.com; bhelg...@google.com; > a...@arndb.de; linux-arm-ker...@lists.infradead.org; > mark.rutl...@arm.com; brian.star...@arm.com; o...@lixom.net; > b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux- > a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org; > miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > On Fri, Jun 16, 2017 at 2:00 PM, Mika Westerberg > wrote: > > On Fri, Jun 16, 2017 at 01:24:32PM +0200, Rafael J. Wysocki wrote: > >> > In fact it may be that it is not sufficient in this case because > the > >> > ACPI core might enumerate child devices before the LPC driver even > gets > >> > a chance to probe so you would need to add also scan handler to > the > >> > child devices and mark them already enumerated or something like > that. > >> > >> Or extend the special I2C/SPI handling to them. > > > > Sure but those have I2c/SpiSerialBus() resources which we can use to > > identify them but for the ipmi thing there is nothing else than _HID > so > > we would need to keep a list of such devices in ACPI core. > > OK, so adding a scan handler for that would be the way to go IMO. Many thanks for your response and your help here. I guess that as conclusion with respect to the current v9 patchset we can disregard the idea of MFD and modify the current v9 so that it doesn't touch directly ACPI resources. Instead as I proposed before we can have the scan handler to enumerate the children devices and translate its addresses filling dev->resources[] and at the same time we can modify acpi_default_enumeration to check acpi_device_enumerated() before continuing with device enumeration...? Do you think it as a viable solution? Thanks Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika
> -Original Message-
> From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci-
> ow...@vger.kernel.org] On Behalf Of Mika Westerberg
> Sent: 13 June 2017 21:04
> To: Gabriele Paoloni
> Cc: Lorenzo Pieralisi; raf...@kernel.org; Rafael J. Wysocki;
> catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm-
> ker...@lists.infradead.org; mark.rutl...@arm.com;
> brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux-
> ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux-
> p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O)
> Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO
> devices before scanning
>
> On Tue, Jun 13, 2017 at 07:01:38PM +, Gabriele Paoloni wrote:
> > I am not very familiar with Linux MFD however the main issue here is
> that
> > 1) for IPMI we want to re-use the standard IPMI driver without
> touching it:
> >see
> >
> >static const struct acpi_device_id acpi_ipmi_match[] = {
> > { "IPI0001", 0 },
> > { },
> >};
> >
> >in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard
> driver
> >of an LPC child)
> >
> > 2) We need a way to guarantee that all LPC children are not
> enumerated
> >by acpi_default_enumeration() (so for example if an ipmi node is
> an LPC#
> >child it should not be enumerated, otherwise it should be)
> >Currently acpi_default_enumeration() skips spi/i2c slaves by
> checking:
> >1) if the acpi resource type is a serial bus
> >2) if the type of serial bus descriptor is I2C or SPI
> >
> >For LPC we cannot leverage on any ACPI property to "recognize"
> that our
> >devices are LPC children; hence before I proposed for
> acpi_default_enumeration()
> >to skip devices that have already been enumerated (by calling
> >acpi_device_enumerated() ).
> >
> > So in the current scenario, how do you think that MFD can help?
>
> If you look at Documentation/acpi/enumeration.txt there is a chapter
> "MFD devices". I think it pretty much maches what you have here. An LPC
> device (MFD device) and bunch of child devices. The driver for your LPC
> device can specify _HID for each child device. Those are then mached by
> the MFD ACPI code to the corresponding ACPI nodes from which platform
> devices are created including "IPI0001".
So I guess here in the LPC driver I would have an MFD cell for IPMI. I.e.:
static struct mfd_cell_acpi_match hisi_lpc_ipmi_acpi_match = {
.pnpid = "IPI0001",
};
correct?
>
> It causes acpi_default_enumeration() to be called but it should be fine
> as we are dealing with platform device anyway.
I do not quite understand how declaring such MFD cell above would make sure
that the LPC probe is called before the IPMI device is enumerated...
Could you point me to the code that does this?
Many Thanks
Gab
>
> Once the platform device is created your ipmi driver will be probed by
> the driver core.
>
> Does that make sense?
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika
> -Original Message-
> From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci-
> ow...@vger.kernel.org] On Behalf Of Mika Westerberg
> Sent: 13 June 2017 21:04
> To: Gabriele Paoloni
> Cc: Lorenzo Pieralisi; raf...@kernel.org; Rafael J. Wysocki;
> catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm-
> ker...@lists.infradead.org; mark.rutl...@arm.com;
> brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux-
> ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux-
> p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O)
> Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO
> devices before scanning
>
> On Tue, Jun 13, 2017 at 07:01:38PM +, Gabriele Paoloni wrote:
> > I am not very familiar with Linux MFD however the main issue here is
> that
> > 1) for IPMI we want to re-use the standard IPMI driver without
> touching it:
> >see
> >
> >static const struct acpi_device_id acpi_ipmi_match[] = {
> > { "IPI0001", 0 },
> > { },
> >};
> >
> >in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard
> driver
> >of an LPC child)
> >
> > 2) We need a way to guarantee that all LPC children are not
> enumerated
> >by acpi_default_enumeration() (so for example if an ipmi node is
> an LPC#
> >child it should not be enumerated, otherwise it should be)
> >Currently acpi_default_enumeration() skips spi/i2c slaves by
> checking:
> >1) if the acpi resource type is a serial bus
> >2) if the type of serial bus descriptor is I2C or SPI
> >
> >For LPC we cannot leverage on any ACPI property to "recognize"
> that our
> >devices are LPC children; hence before I proposed for
> acpi_default_enumeration()
> >to skip devices that have already been enumerated (by calling
> >acpi_device_enumerated() ).
> >
> > So in the current scenario, how do you think that MFD can help?
>
> If you look at Documentation/acpi/enumeration.txt there is a chapter
> "MFD devices". I think it pretty much maches what you have here. An LPC
> device (MFD device) and bunch of child devices. The driver for your LPC
> device can specify _HID for each child device. Those are then mached by
> the MFD ACPI code to the corresponding ACPI nodes from which platform
> devices are created including "IPI0001".
So I guess here in the LPC driver I would have an MFD cell for IPMI. I.e.:
static struct mfd_cell_acpi_match hisi_lpc_ipmi_acpi_match = {
.pnpid = "IPI0001",
};
correct?
>
> It causes acpi_default_enumeration() to be called but it should be fine
> as we are dealing with platform device anyway.
I do not quite understand how declaring such MFD cell above would make sure
that the LPC probe is called before the IPMI device is enumerated...
Could you point me to the code that does this?
Many Thanks
Gab
>
> Once the platform device is created your ipmi driver will be probed by
> the driver core.
>
> Does that make sense?
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika
> -Original Message-
> From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com]
> Sent: 13 June 2017 16:10
> To: Gabriele Paoloni
> Cc: Lorenzo Pieralisi; raf...@kernel.org; Rafael J. Wysocki;
> catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm-
> ker...@lists.infradead.org; mark.rutl...@arm.com;
> brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux-
> ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux-
> p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O)
> Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO
> devices before scanning
>
> On Tue, Jun 13, 2017 at 02:38:26PM +, Gabriele Paoloni wrote:
> > > Is there an example ASL showing how these LPC devices are
> > > currently presented in ACPI?
> >
> > Please find below the asl sketch for our LPC and IPMI
> >
> > //
> > // LPC
> > //
> >
> > Scope(_SB) {
> > Device (LPC0) {
> > Name (_HID, "HISI0191") // HiSi LPC
> > Name (_CRS, ResourceTemplate () {
> > Memory32Fixed (ReadWrite, 0xa01b, 0x1000)
> > })
> > }
> >
> > Device (LPC0.IPMI) {
> > Name (_HID, "IPI0001")
> > Method (_IFT) {
> > Return (0x03)
> > }
> > Name (LORS, ResourceTemplate() {
> > QWordIO (
> > ResourceConsumer,
> > MinNotFixed, // _MIF
> > MaxNotFixed, // _MAF
> > PosDecode,
> > EntireRange,
> > 0x0, // _GRA
> > 0xe4,// _MIN
> > 0x3fff, // _MAX
> > 0x0, // _TRA
> > 0x04,// _LEN
> > , ,
> > BTIO
> > )
> > })
> > CreateQWordField (LORS, BTIO._MIN, CMIN)
> > CreateQWordField (LORS, BTIO._MAX, CMAX)
> > CreateQWordField (LORS, BTIO._LEN, CLEN)
> >
> > Method (_PRS, 0) {
> > Return (LORS)
> > }
> >
> > Method (_CRS, 0) {
> > Return (LORS)
> > }
> > Method (_SRS, 1) {
> > CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MIN, IMIN)
> > Store (IMIN, CMIN)
> > CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MAX, IMAX)
> > Store (IMAX, CMAX)
> > }
> > }
> > [...]
> > }
>
> Thanks. So this looks pretty much like normal Linux MFD device which we
> already have support for adding ACPI bindings to child devices. It
> should also support splitting resources to child devices IIRC.
I am not very familiar with Linux MFD however the main issue here is that
1) for IPMI we want to re-use the standard IPMI driver without touching it:
see
static const struct acpi_device_id acpi_ipmi_match[] = {
{ "IPI0001", 0 },
{ },
};
in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard driver
of an LPC child)
2) We need a way to guarantee that all LPC children are not enumerated
by acpi_default_enumeration() (so for example if an ipmi node is an LPC#
child it should not be enumerated, otherwise it should be)
Currently acpi_default_enumeration() skips spi/i2c slaves by checking:
1) if the acpi resource type is a serial bus
2) if the type of serial bus descriptor is I2C or SPI
For LPC we cannot leverage on any ACPI property to "recognize" that our
devices are LPC children; hence before I proposed for
acpi_default_enumeration()
to skip devices that have already been enumerated (by calling
acpi_device_enumerated() ).
So in the current scenario, how do you think that MFD can help?
Do you see any possible solution?
Many thanks
Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika
> -Original Message-
> From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com]
> Sent: 13 June 2017 16:10
> To: Gabriele Paoloni
> Cc: Lorenzo Pieralisi; raf...@kernel.org; Rafael J. Wysocki;
> catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm-
> ker...@lists.infradead.org; mark.rutl...@arm.com;
> brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux-
> ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux-
> p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O)
> Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO
> devices before scanning
>
> On Tue, Jun 13, 2017 at 02:38:26PM +, Gabriele Paoloni wrote:
> > > Is there an example ASL showing how these LPC devices are
> > > currently presented in ACPI?
> >
> > Please find below the asl sketch for our LPC and IPMI
> >
> > //
> > // LPC
> > //
> >
> > Scope(_SB) {
> > Device (LPC0) {
> > Name (_HID, "HISI0191") // HiSi LPC
> > Name (_CRS, ResourceTemplate () {
> > Memory32Fixed (ReadWrite, 0xa01b, 0x1000)
> > })
> > }
> >
> > Device (LPC0.IPMI) {
> > Name (_HID, "IPI0001")
> > Method (_IFT) {
> > Return (0x03)
> > }
> > Name (LORS, ResourceTemplate() {
> > QWordIO (
> > ResourceConsumer,
> > MinNotFixed, // _MIF
> > MaxNotFixed, // _MAF
> > PosDecode,
> > EntireRange,
> > 0x0, // _GRA
> > 0xe4,// _MIN
> > 0x3fff, // _MAX
> > 0x0, // _TRA
> > 0x04,// _LEN
> > , ,
> > BTIO
> > )
> > })
> > CreateQWordField (LORS, BTIO._MIN, CMIN)
> > CreateQWordField (LORS, BTIO._MAX, CMAX)
> > CreateQWordField (LORS, BTIO._LEN, CLEN)
> >
> > Method (_PRS, 0) {
> > Return (LORS)
> > }
> >
> > Method (_CRS, 0) {
> > Return (LORS)
> > }
> > Method (_SRS, 1) {
> > CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MIN, IMIN)
> > Store (IMIN, CMIN)
> > CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MAX, IMAX)
> > Store (IMAX, CMAX)
> > }
> > }
> > [...]
> > }
>
> Thanks. So this looks pretty much like normal Linux MFD device which we
> already have support for adding ACPI bindings to child devices. It
> should also support splitting resources to child devices IIRC.
I am not very familiar with Linux MFD however the main issue here is that
1) for IPMI we want to re-use the standard IPMI driver without touching it:
see
static const struct acpi_device_id acpi_ipmi_match[] = {
{ "IPI0001", 0 },
{ },
};
in "drivers/char/ipmi/ipmi_si_intf.c" (and in general any standard driver
of an LPC child)
2) We need a way to guarantee that all LPC children are not enumerated
by acpi_default_enumeration() (so for example if an ipmi node is an LPC#
child it should not be enumerated, otherwise it should be)
Currently acpi_default_enumeration() skips spi/i2c slaves by checking:
1) if the acpi resource type is a serial bus
2) if the type of serial bus descriptor is I2C or SPI
For LPC we cannot leverage on any ACPI property to "recognize" that our
devices are LPC children; hence before I proposed for
acpi_default_enumeration()
to skip devices that have already been enumerated (by calling
acpi_device_enumerated() ).
So in the current scenario, how do you think that MFD can help?
Do you see any possible solution?
Many thanks
Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika
> -Original Message-
> From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com]
> Sent: 13 June 2017 09:49
> To: Lorenzo Pieralisi
> Cc: Gabriele Paoloni; raf...@kernel.org; Rafael J. Wysocki;
> catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm-
> ker...@lists.infradead.org; mark.rutl...@arm.com;
> brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux-
> ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux-
> p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O)
> Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO
> devices before scanning
>
> On Mon, Jun 12, 2017 at 04:57:00PM +0100, Lorenzo Pieralisi wrote:
> > I had a more in-depth look at this series and from my understanding
> > the problem are the following to manage the LPC bindings in ACPI.
> >
> > (1) Child devices of an LPC controller require special handling when
> > filling their resources (ie they need to be translated - in DT
> > that's guaranteed by the "isa" binding, in ACPI it has to be
> > done by new code)
> > (2) In DT systems, LPC child devices are created by the LPC bus
> > controller driver through an of_platform_populate() call with
> > the LPC controller node as the fwnode root. For ACPI to work
> > the same way there must be a way to prevent LPC children to
> > be enumerated in acpi_default_enumeration() something like
> > I2C does (and then the LPC driver would enumerate its children as
> > DT does)
> >
> > I am not sure how (1) and (2) can be managed with current ACPI
> bindings
> > and kernel code - I suspect it may be done by mirroring what's done
> > for I2C but I am not sure, that's why I CC'ed Mika (ie the LPC
> adapter
> > is matched as a platform device and it takes care of enumerating its
> > children - problem though is preventing enumeration from core ACPI
> code).
>
> Is there an example ASL showing how these LPC devices are
> currently presented in ACPI?
Please find below the asl sketch for our LPC and IPMI
//
// LPC
//
Scope(_SB) {
Device (LPC0) {
Name (_HID, "HISI0191") // HiSi LPC
Name (_CRS, ResourceTemplate () {
Memory32Fixed (ReadWrite, 0xa01b, 0x1000)
})
}
Device (LPC0.IPMI) {
Name (_HID, "IPI0001")
Method (_IFT) {
Return (0x03)
}
Name (LORS, ResourceTemplate() {
QWordIO (
ResourceConsumer,
MinNotFixed, // _MIF
MaxNotFixed, // _MAF
PosDecode,
EntireRange,
0x0, // _GRA
0xe4,// _MIN
0x3fff, // _MAX
0x0, // _TRA
0x04,// _LEN
, ,
BTIO
)
})
CreateQWordField (LORS, BTIO._MIN, CMIN)
CreateQWordField (LORS, BTIO._MAX, CMAX)
CreateQWordField (LORS, BTIO._LEN, CLEN)
Method (_PRS, 0) {
Return (LORS)
}
Method (_CRS, 0) {
Return (LORS)
}
Method (_SRS, 1) {
CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MIN, IMIN)
Store (IMIN, CMIN)
CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MAX, IMAX)
Store (IMAX, CMAX)
}
}
[...]
}
Many thanks
Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Mika
> -Original Message-
> From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com]
> Sent: 13 June 2017 09:49
> To: Lorenzo Pieralisi
> Cc: Gabriele Paoloni; raf...@kernel.org; Rafael J. Wysocki;
> catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm-
> ker...@lists.infradead.org; mark.rutl...@arm.com;
> brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux-
> ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux-
> p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O)
> Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO
> devices before scanning
>
> On Mon, Jun 12, 2017 at 04:57:00PM +0100, Lorenzo Pieralisi wrote:
> > I had a more in-depth look at this series and from my understanding
> > the problem are the following to manage the LPC bindings in ACPI.
> >
> > (1) Child devices of an LPC controller require special handling when
> > filling their resources (ie they need to be translated - in DT
> > that's guaranteed by the "isa" binding, in ACPI it has to be
> > done by new code)
> > (2) In DT systems, LPC child devices are created by the LPC bus
> > controller driver through an of_platform_populate() call with
> > the LPC controller node as the fwnode root. For ACPI to work
> > the same way there must be a way to prevent LPC children to
> > be enumerated in acpi_default_enumeration() something like
> > I2C does (and then the LPC driver would enumerate its children as
> > DT does)
> >
> > I am not sure how (1) and (2) can be managed with current ACPI
> bindings
> > and kernel code - I suspect it may be done by mirroring what's done
> > for I2C but I am not sure, that's why I CC'ed Mika (ie the LPC
> adapter
> > is matched as a platform device and it takes care of enumerating its
> > children - problem though is preventing enumeration from core ACPI
> code).
>
> Is there an example ASL showing how these LPC devices are
> currently presented in ACPI?
Please find below the asl sketch for our LPC and IPMI
//
// LPC
//
Scope(_SB) {
Device (LPC0) {
Name (_HID, "HISI0191") // HiSi LPC
Name (_CRS, ResourceTemplate () {
Memory32Fixed (ReadWrite, 0xa01b, 0x1000)
})
}
Device (LPC0.IPMI) {
Name (_HID, "IPI0001")
Method (_IFT) {
Return (0x03)
}
Name (LORS, ResourceTemplate() {
QWordIO (
ResourceConsumer,
MinNotFixed, // _MIF
MaxNotFixed, // _MAF
PosDecode,
EntireRange,
0x0, // _GRA
0xe4,// _MIN
0x3fff, // _MAX
0x0, // _TRA
0x04,// _LEN
, ,
BTIO
)
})
CreateQWordField (LORS, BTIO._MIN, CMIN)
CreateQWordField (LORS, BTIO._MAX, CMAX)
CreateQWordField (LORS, BTIO._LEN, CLEN)
Method (_PRS, 0) {
Return (LORS)
}
Method (_CRS, 0) {
Return (LORS)
}
Method (_SRS, 1) {
CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MIN, IMIN)
Store (IMIN, CMIN)
CreateQWordField (Arg0, \_SB.LPC0.IPMI.BTIO._MAX, IMAX)
Store (IMAX, CMAX)
}
}
[...]
}
Many thanks
Gab
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Lorenzo, Rafael > -Original Message- > From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com] > Sent: 12 June 2017 16:57 > To: Gabriele Paoloni; raf...@kernel.org; Rafael J. Wysocki; Mika > Westerberg > Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > [+Mika] > > Gab, Rafael, > > On Tue, Jun 06, 2017 at 09:55:53AM +0100, Lorenzo Pieralisi wrote: > > Hi Gab, Rafael, > > > > On Wed, May 31, 2017 at 10:24:47AM +, Gabriele Paoloni wrote: > > > > [...] > > > > > > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > > > > > index e39ec7b..37dd23c 100644 > > > > > --- a/drivers/acpi/scan.c > > > > > +++ b/drivers/acpi/scan.c > > > > > @@ -2035,6 +2035,7 @@ int __init acpi_scan_init(void) > > > > > acpi_int340x_thermal_init(); > > > > > acpi_amba_init(); > > > > > acpi_watchdog_init(); > > > > > + acpi_indirectio_scan_init(); > > > > Unfortunately this is becoming a pattern and we are ending up > > with a static ordering of "subsystems" init (even though for this > > LPC series it is just the Hisilicon driver that requires this call) > > and I am not sure I see any way of avoiding it. I think that's always > > been the case in x86, with fewer subsystems/kernel paths to care > > about, I wanted to flag this up though to check your opinion since > > I am not sure this is the right direction we are taking. > > > > I also think that relying on _DEP to build any dependency is not > > entirely a) usable (owing to legacy bindings and previous _DEP > misuse) > > and b) compliant with ACPI bindings given that _DEP has to be used > > for operation regions only. > > I had a more in-depth look at this series and from my understanding > the problem are the following to manage the LPC bindings in ACPI. > > (1) Child devices of an LPC controller require special handling when > filling their resources (ie they need to be translated - in DT > that's guaranteed by the "isa" binding, in ACPI it has to be > done by new code) Correct, LPC resources need to be translated in a virtual IO port address space. We cannot strictly follow the ISA bindings as the LPC host does not define a mapping (through the "range" property) between a CPU address range and an LPC address range. Instead LPC has got his own bus accessors; therefore the bus address range that LPC operates on is directly mapped into the IO port address range and the IO in/out standard accessors (include/asm-generic/io.h) are redefined to use the LPC accessors for the virtual IO port address range that corresponds to LPC. > (2) In DT systems, LPC child devices are created by the LPC bus > controller driver through an of_platform_populate() call with > the LPC controller node as the fwnode root. For ACPI to work > the same way there must be a way to prevent LPC children to > be enumerated in acpi_default_enumeration() something like > I2C does (and then the LPC driver would enumerate its children as > DT does) Correct. > > I am not sure how (1) and (2) can be managed with current ACPI bindings > and kernel code - I suspect it may be done by mirroring what's done > for I2C but I am not sure, that's why I CC'ed Mika (ie the LPC adapter > is matched as a platform device and it takes care of enumerating its > children - problem though is preventing enumeration from core ACPI > code). Yes my idea was to have a scan handler that enumerate the children devices and translate its addresses filling dev->resources[] and at the same time we can modify acpi_default_enumeration() to check acpi_device_enumerated() before continuing with device enumeration...? Many thanks Gab > > I will let Gabriele and Hisilicon guys chime in if I missed something, > which is likely, please let me know your opinion on how this code > can be made functional on ACPI systems - it is uncharted territory. > > Thank you ! > Lorenzo > > > > > Thoughts ? > > > > Thanks, > > Lorenzo > > > > > > > acpi_scan_add_handler(_device_handler); > > > > > > > > > > diff
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Lorenzo, Rafael > -Original Message- > From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com] > Sent: 12 June 2017 16:57 > To: Gabriele Paoloni; raf...@kernel.org; Rafael J. Wysocki; Mika > Westerberg > Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org; > frowand.l...@gmail.com; bhelg...@google.com; a...@arndb.de; linux-arm- > ker...@lists.infradead.org; mark.rutl...@arm.com; > brian.star...@arm.com; o...@lixom.net; b...@kernel.crashing.org; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; Linuxarm; linux- > p...@vger.kernel.org; miny...@acm.org; John Garry; xuwei (O) > Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO > devices before scanning > > [+Mika] > > Gab, Rafael, > > On Tue, Jun 06, 2017 at 09:55:53AM +0100, Lorenzo Pieralisi wrote: > > Hi Gab, Rafael, > > > > On Wed, May 31, 2017 at 10:24:47AM +, Gabriele Paoloni wrote: > > > > [...] > > > > > > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > > > > > index e39ec7b..37dd23c 100644 > > > > > --- a/drivers/acpi/scan.c > > > > > +++ b/drivers/acpi/scan.c > > > > > @@ -2035,6 +2035,7 @@ int __init acpi_scan_init(void) > > > > > acpi_int340x_thermal_init(); > > > > > acpi_amba_init(); > > > > > acpi_watchdog_init(); > > > > > + acpi_indirectio_scan_init(); > > > > Unfortunately this is becoming a pattern and we are ending up > > with a static ordering of "subsystems" init (even though for this > > LPC series it is just the Hisilicon driver that requires this call) > > and I am not sure I see any way of avoiding it. I think that's always > > been the case in x86, with fewer subsystems/kernel paths to care > > about, I wanted to flag this up though to check your opinion since > > I am not sure this is the right direction we are taking. > > > > I also think that relying on _DEP to build any dependency is not > > entirely a) usable (owing to legacy bindings and previous _DEP > misuse) > > and b) compliant with ACPI bindings given that _DEP has to be used > > for operation regions only. > > I had a more in-depth look at this series and from my understanding > the problem are the following to manage the LPC bindings in ACPI. > > (1) Child devices of an LPC controller require special handling when > filling their resources (ie they need to be translated - in DT > that's guaranteed by the "isa" binding, in ACPI it has to be > done by new code) Correct, LPC resources need to be translated in a virtual IO port address space. We cannot strictly follow the ISA bindings as the LPC host does not define a mapping (through the "range" property) between a CPU address range and an LPC address range. Instead LPC has got his own bus accessors; therefore the bus address range that LPC operates on is directly mapped into the IO port address range and the IO in/out standard accessors (include/asm-generic/io.h) are redefined to use the LPC accessors for the virtual IO port address range that corresponds to LPC. > (2) In DT systems, LPC child devices are created by the LPC bus > controller driver through an of_platform_populate() call with > the LPC controller node as the fwnode root. For ACPI to work > the same way there must be a way to prevent LPC children to > be enumerated in acpi_default_enumeration() something like > I2C does (and then the LPC driver would enumerate its children as > DT does) Correct. > > I am not sure how (1) and (2) can be managed with current ACPI bindings > and kernel code - I suspect it may be done by mirroring what's done > for I2C but I am not sure, that's why I CC'ed Mika (ie the LPC adapter > is matched as a platform device and it takes care of enumerating its > children - problem though is preventing enumeration from core ACPI > code). Yes my idea was to have a scan handler that enumerate the children devices and translate its addresses filling dev->resources[] and at the same time we can modify acpi_default_enumeration() to check acpi_device_enumerated() before continuing with device enumeration...? Many thanks Gab > > I will let Gabriele and Hisilicon guys chime in if I missed something, > which is likely, please let me know your opinion on how this code > can be made functional on ACPI systems - it is uncharted territory. > > Thank you ! > Lorenzo > > > > > Thoughts ? > > > > Thanks, > > Lorenzo > > > > > > > acpi_scan_add_handler(_device_handler); > > > > > > > > > > diff --
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Lorenzo
Many thanks for reviewing
> -Original Message-
> From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com]
> Sent: 30 May 2017 14:24
> To: Gabriele Paoloni
> Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org;
> a...@arndb.de; linux-arm-ker...@lists.infradead.org;
> mark.rutl...@arm.com; brian.star...@arm.com; o...@lixom.net;
> b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux-
> a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org;
> miny...@acm.org; John Garry; xuwei (O)
> Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO
> devices before scanning
>
> Hi Gab,
>
> On Thu, May 25, 2017 at 12:37:26PM +0100, Gabriele Paoloni wrote:
> > From: Gabriele <gabriele.paol...@huawei.com>
> >
> > On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices
> access
> > I/O with some special host-local I/O ports known on x86. As their I/O
> > space are not memory mapped like PCI/PCIE MMIO host bridges, this
> patch is
> > meant to support a new class of I/O host controllers where the local
> IO
> > ports of the children devices are translated into the Indirect I/O
> address
> > space.
> > Through the handler attach callback, all the I/O translations are
> done
> > before starting the enumeration on children devices and the
> translated
> > addresses are replaced in the children resources.
>
> I do not understand why this is done through a scan handler and to
> be frank I do not see how this mechanism is supposed to be a generic
> kernel layer, possibly used by other platforms, when there is no notion
> in ACPI to cater for that.
Well, the main reason is that we need to translate the bus addresses of
the LPC children before the respective children's drivers probe.
>
> As far as I am concerned this patch code should live in the Hisilicon
> LPC driver - as things stand it is neither ACPI generic code nor ARM64
> specific, it is code to make ACPI work like DT bindings without any
> ACPI
> binding at all; I still have some concerns from an ACPI standpoint
> below.
Well the reason why this patch cannot live in the LPC driver probe is
that AFAIK currently there is no way to guarantee the LPC probe to be
called before the children probe. With respect to the ACPI concerns
please see below.
>
> > Signed-off-by: zhichang.yuan <yuanzhich...@hisilicon.com>
> > Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
> > ---
> > drivers/acpi/arm64/Makefile| 1 +
> > drivers/acpi/arm64/acpi_indirect_pio.c | 301
> +
> > drivers/acpi/internal.h| 5 +
> > drivers/acpi/scan.c| 1 +
> > include/acpi/acpi_indirect_pio.h | 24 +++
> > 5 files changed, 332 insertions(+)
> > create mode 100644 drivers/acpi/arm64/acpi_indirect_pio.c
> > create mode 100644 include/acpi/acpi_indirect_pio.h
> >
> > diff --git a/drivers/acpi/arm64/Makefile
> b/drivers/acpi/arm64/Makefile
> > index 1017def..3944775 100644
> > --- a/drivers/acpi/arm64/Makefile
> > +++ b/drivers/acpi/arm64/Makefile
> > @@ -1,2 +1,3 @@
> > obj-$(CONFIG_ACPI_IORT)+= iort.o
> > obj-$(CONFIG_ACPI_GTDT)+= gtdt.o
> > +obj-$(CONFIG_INDIRECT_PIO) += acpi_indirect_pio.o
> > diff --git a/drivers/acpi/arm64/acpi_indirect_pio.c
> b/drivers/acpi/arm64/acpi_indirect_pio.c
> > new file mode 100644
> > index 000..7813f73
> > --- /dev/null
> > +++ b/drivers/acpi/arm64/acpi_indirect_pio.c
> > @@ -0,0 +1,301 @@
> > +/*
> > + * ACPI support for indirect-PIO bus.
> > + *
> > + * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved.
> > + * Author: Gabriele Paoloni <gabriele.paol...@huawei.com>
> > + * Author: Zhichang Yuan <yuanzhich...@hisilicon.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +
> > +ACPI_MODULE_NAME("indirect PIO");
> > +
> > +#define INDIRECT_PIO_INFO(desc) ((unsigned long))
> > +
> > +static acpi_status acpi_count_logic_iores(struct acpi_resource *res,
> > + void *data)
> > +{
> > + int *res_cnt = data;
> > +
> > + if (!acpi_dev_filter_resource_type(res, IORESOURCE_IO))
> >
RE: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO devices before scanning
Hi Lorenzo
Many thanks for reviewing
> -Original Message-
> From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com]
> Sent: 30 May 2017 14:24
> To: Gabriele Paoloni
> Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org;
> a...@arndb.de; linux-arm-ker...@lists.infradead.org;
> mark.rutl...@arm.com; brian.star...@arm.com; o...@lixom.net;
> b...@kernel.crashing.org; linux-kernel@vger.kernel.org; linux-
> a...@vger.kernel.org; Linuxarm; linux-...@vger.kernel.org;
> miny...@acm.org; John Garry; xuwei (O)
> Subject: Re: [PATCH v9 5/7] ACPI: Translate the I/O range of non-MMIO
> devices before scanning
>
> Hi Gab,
>
> On Thu, May 25, 2017 at 12:37:26PM +0100, Gabriele Paoloni wrote:
> > From: Gabriele
> >
> > On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices
> access
> > I/O with some special host-local I/O ports known on x86. As their I/O
> > space are not memory mapped like PCI/PCIE MMIO host bridges, this
> patch is
> > meant to support a new class of I/O host controllers where the local
> IO
> > ports of the children devices are translated into the Indirect I/O
> address
> > space.
> > Through the handler attach callback, all the I/O translations are
> done
> > before starting the enumeration on children devices and the
> translated
> > addresses are replaced in the children resources.
>
> I do not understand why this is done through a scan handler and to
> be frank I do not see how this mechanism is supposed to be a generic
> kernel layer, possibly used by other platforms, when there is no notion
> in ACPI to cater for that.
Well, the main reason is that we need to translate the bus addresses of
the LPC children before the respective children's drivers probe.
>
> As far as I am concerned this patch code should live in the Hisilicon
> LPC driver - as things stand it is neither ACPI generic code nor ARM64
> specific, it is code to make ACPI work like DT bindings without any
> ACPI
> binding at all; I still have some concerns from an ACPI standpoint
> below.
Well the reason why this patch cannot live in the LPC driver probe is
that AFAIK currently there is no way to guarantee the LPC probe to be
called before the children probe. With respect to the ACPI concerns
please see below.
>
> > Signed-off-by: zhichang.yuan
> > Signed-off-by: Gabriele Paoloni
> > ---
> > drivers/acpi/arm64/Makefile| 1 +
> > drivers/acpi/arm64/acpi_indirect_pio.c | 301
> +
> > drivers/acpi/internal.h| 5 +
> > drivers/acpi/scan.c| 1 +
> > include/acpi/acpi_indirect_pio.h | 24 +++
> > 5 files changed, 332 insertions(+)
> > create mode 100644 drivers/acpi/arm64/acpi_indirect_pio.c
> > create mode 100644 include/acpi/acpi_indirect_pio.h
> >
> > diff --git a/drivers/acpi/arm64/Makefile
> b/drivers/acpi/arm64/Makefile
> > index 1017def..3944775 100644
> > --- a/drivers/acpi/arm64/Makefile
> > +++ b/drivers/acpi/arm64/Makefile
> > @@ -1,2 +1,3 @@
> > obj-$(CONFIG_ACPI_IORT)+= iort.o
> > obj-$(CONFIG_ACPI_GTDT)+= gtdt.o
> > +obj-$(CONFIG_INDIRECT_PIO) += acpi_indirect_pio.o
> > diff --git a/drivers/acpi/arm64/acpi_indirect_pio.c
> b/drivers/acpi/arm64/acpi_indirect_pio.c
> > new file mode 100644
> > index 000..7813f73
> > --- /dev/null
> > +++ b/drivers/acpi/arm64/acpi_indirect_pio.c
> > @@ -0,0 +1,301 @@
> > +/*
> > + * ACPI support for indirect-PIO bus.
> > + *
> > + * Copyright (C) 2017 Hisilicon Limited, All Rights Reserved.
> > + * Author: Gabriele Paoloni
> > + * Author: Zhichang Yuan
> > + *
> > + * This program is free software; you can redistribute it and/or
> modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +
> > +ACPI_MODULE_NAME("indirect PIO");
> > +
> > +#define INDIRECT_PIO_INFO(desc) ((unsigned long))
> > +
> > +static acpi_status acpi_count_logic_iores(struct acpi_resource *res,
> > + void *data)
> > +{
> > + int *res_cnt = data;
> > +
> > + if (!acpi_dev_filter_resource_type(res, IORESOURCE_IO))
> > + (*res_cnt)++;
> > +
> > + return AE_OK;
> > +}
> > +
> > +static acpi_status acpi_read_one_logicpiores(struct acpi_resource
>
RE: [PATCH v9 1/7] LIB: Introduce a generic PIO mapping method
Hi Bjorn
> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: 26 May 2017 21:58
> To: Gabriele Paoloni
> Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org;
> a...@arndb.de; linux-arm-ker...@lists.infradead.org;
> lorenzo.pieral...@arm.com; mark.rutl...@arm.com; miny...@acm.org;
> b...@kernel.crashing.org; John Garry; linux-kernel@vger.kernel.org;
> xuwei (O); Linuxarm; linux-a...@vger.kernel.org; zhichang.yuan; linux-
> p...@vger.kernel.org; o...@lixom.net; brian.star...@arm.com
> Subject: Re: [PATCH v9 1/7] LIB: Introduce a generic PIO mapping method
>
> On Thu, May 25, 2017 at 12:37:22PM +0100, Gabriele Paoloni wrote:
> > From: "zhichang.yuan" <yuanzhich...@hisilicon.com>
> >
> > In 'commit 41f8bba7f555 ("of/pci: Add pci_register_io_range() and
> > pci_pio_to_address()")' a new I/O space management was supported.
> With that
> > driver, the I/O ranges configured for PCI/PCIE hosts on some
> architectures
> > can be mapped to logical PIO, converted easily between CPU address
> and the
> > corresponding logicial PIO. Based on this, PCI I/O devices can be
> accessed
> > in a memory read/write way through the unified in/out accessors.
> >
> > But on some archs/platforms, there are bus hosts which access I/O
> > peripherals with host-local I/O port addresses rather than memory
> > addresses after memory-mapped.
> > To support those devices, a more generic I/O mapping method is
> introduced
> > here. Through this patch, both the CPU addresses and the host-local
> port
> > can be mapped into the logical PIO space with different logical/fake
> PIOs.
> > After this, all the I/O accesses to either PCI MMIO devices or host-
> local
> > I/O peripherals can be unified into the existing I/O accessors
> defined in
> > asm-generic/io.h and be redirected to the right device-specific hooks
> > based on the input logical PIO.
> >
> > Signed-off-by: zhichang.yuan <yuanzhich...@hisilicon.com>
> > Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
> > ---
> > include/asm-generic/io.h | 50 +
> > include/linux/logic_pio.h | 110 ++
> > lib/Kconfig | 26 +
> > lib/Makefile | 2 +
> > lib/logic_pio.c | 280
> ++
> > 5 files changed, 468 insertions(+)
> > create mode 100644 include/linux/logic_pio.h
> > create mode 100644 lib/logic_pio.c
> >
> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > index 7ef015e..f7fbec3 100644
> > --- a/include/asm-generic/io.h
> > +++ b/include/asm-generic/io.h
> > ...
>
> > #ifndef inb
> > +#ifdef CONFIG_INDIRECT_PIO
> > +#define inb logic_inb
> > +#else
> > #define inb inb
> > static inline u8 inb(unsigned long addr)
> > {
> > return readb(PCI_IOBASE + addr);
> > }
> > +#endif /* CONFIG_INDIRECT_PIO */
> > #endif
> >
> > #ifndef inw
> > +#ifdef CONFIG_INDIRECT_PIO
> > +#define inw logic_inw
>
> Cosmetic: could these ifdefs all be collected in one place, e.g.,
>
> #ifdef CONFIG_INDIRECT_PIO
> #define inb logic_inb
> #define inw logic_inw
> #define inl logic_inl
> ...
> #endif
>
> to avoid cluttering every one of the default definitions? Could the
> collection be in logic_pio.h itself, next to the extern declarations?
Yes I think it should be doable. I will rework this in the next patchset
>
> > +#else
> > #define inw inw
> > static inline u16 inw(unsigned long addr)
> > {
> > return readw(PCI_IOBASE + addr);
> > }
> > +#endif /* CONFIG_INDIRECT_PIO */
> > #endif
>
> > #ifndef insb_p
> > diff --git a/include/linux/logic_pio.h b/include/linux/logic_pio.h
> > new file mode 100644
> > index 000..8e4dc65
> > --- /dev/null
> > +++ b/include/linux/logic_pio.h
> > ...
>
> > +extern u8 logic_inb(unsigned long addr);
>
> I think you only build the definitions for these if
> CONFIG_INDIRECT_PIO, so the declarations could be under that #idef,
> too.
Yes agreed
>
> In PCI code, I omit the "extern" from function declarations. This
> isn't PCI code, and I don't know if there's a real consensus on this,
> but there is some precedent: 5bd085b5fbd8 ("x86: remove "extern" from
> function prototypes in ")
>
To be honest I have no clue...
If you look at include/asm-generic/io.h we
RE: [PATCH v9 1/7] LIB: Introduce a generic PIO mapping method
Hi Bjorn
> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: 26 May 2017 21:58
> To: Gabriele Paoloni
> Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org;
> a...@arndb.de; linux-arm-ker...@lists.infradead.org;
> lorenzo.pieral...@arm.com; mark.rutl...@arm.com; miny...@acm.org;
> b...@kernel.crashing.org; John Garry; linux-kernel@vger.kernel.org;
> xuwei (O); Linuxarm; linux-a...@vger.kernel.org; zhichang.yuan; linux-
> p...@vger.kernel.org; o...@lixom.net; brian.star...@arm.com
> Subject: Re: [PATCH v9 1/7] LIB: Introduce a generic PIO mapping method
>
> On Thu, May 25, 2017 at 12:37:22PM +0100, Gabriele Paoloni wrote:
> > From: "zhichang.yuan"
> >
> > In 'commit 41f8bba7f555 ("of/pci: Add pci_register_io_range() and
> > pci_pio_to_address()")' a new I/O space management was supported.
> With that
> > driver, the I/O ranges configured for PCI/PCIE hosts on some
> architectures
> > can be mapped to logical PIO, converted easily between CPU address
> and the
> > corresponding logicial PIO. Based on this, PCI I/O devices can be
> accessed
> > in a memory read/write way through the unified in/out accessors.
> >
> > But on some archs/platforms, there are bus hosts which access I/O
> > peripherals with host-local I/O port addresses rather than memory
> > addresses after memory-mapped.
> > To support those devices, a more generic I/O mapping method is
> introduced
> > here. Through this patch, both the CPU addresses and the host-local
> port
> > can be mapped into the logical PIO space with different logical/fake
> PIOs.
> > After this, all the I/O accesses to either PCI MMIO devices or host-
> local
> > I/O peripherals can be unified into the existing I/O accessors
> defined in
> > asm-generic/io.h and be redirected to the right device-specific hooks
> > based on the input logical PIO.
> >
> > Signed-off-by: zhichang.yuan
> > Signed-off-by: Gabriele Paoloni
> > ---
> > include/asm-generic/io.h | 50 +
> > include/linux/logic_pio.h | 110 ++
> > lib/Kconfig | 26 +
> > lib/Makefile | 2 +
> > lib/logic_pio.c | 280
> ++
> > 5 files changed, 468 insertions(+)
> > create mode 100644 include/linux/logic_pio.h
> > create mode 100644 lib/logic_pio.c
> >
> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > index 7ef015e..f7fbec3 100644
> > --- a/include/asm-generic/io.h
> > +++ b/include/asm-generic/io.h
> > ...
>
> > #ifndef inb
> > +#ifdef CONFIG_INDIRECT_PIO
> > +#define inb logic_inb
> > +#else
> > #define inb inb
> > static inline u8 inb(unsigned long addr)
> > {
> > return readb(PCI_IOBASE + addr);
> > }
> > +#endif /* CONFIG_INDIRECT_PIO */
> > #endif
> >
> > #ifndef inw
> > +#ifdef CONFIG_INDIRECT_PIO
> > +#define inw logic_inw
>
> Cosmetic: could these ifdefs all be collected in one place, e.g.,
>
> #ifdef CONFIG_INDIRECT_PIO
> #define inb logic_inb
> #define inw logic_inw
> #define inl logic_inl
> ...
> #endif
>
> to avoid cluttering every one of the default definitions? Could the
> collection be in logic_pio.h itself, next to the extern declarations?
Yes I think it should be doable. I will rework this in the next patchset
>
> > +#else
> > #define inw inw
> > static inline u16 inw(unsigned long addr)
> > {
> > return readw(PCI_IOBASE + addr);
> > }
> > +#endif /* CONFIG_INDIRECT_PIO */
> > #endif
>
> > #ifndef insb_p
> > diff --git a/include/linux/logic_pio.h b/include/linux/logic_pio.h
> > new file mode 100644
> > index 000..8e4dc65
> > --- /dev/null
> > +++ b/include/linux/logic_pio.h
> > ...
>
> > +extern u8 logic_inb(unsigned long addr);
>
> I think you only build the definitions for these if
> CONFIG_INDIRECT_PIO, so the declarations could be under that #idef,
> too.
Yes agreed
>
> In PCI code, I omit the "extern" from function declarations. This
> isn't PCI code, and I don't know if there's a real consensus on this,
> but there is some precedent: 5bd085b5fbd8 ("x86: remove "extern" from
> function prototypes in ")
>
To be honest I have no clue...
If you look at include/asm-generic/io.h we have extern declarations...
BTW I can remove the extern and then let's see if somebody complains...
RE: [PATCH v9 2/7] PCI: Apply the new generic I/O management on PCI IO hosts
Hi Bjorn
> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: 26 May 2017 22:20
> To: Gabriele Paoloni
> Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org;
> a...@arndb.de; linux-arm-ker...@lists.infradead.org;
> lorenzo.pieral...@arm.com; mark.rutl...@arm.com; miny...@acm.org;
> b...@kernel.crashing.org; John Garry; linux-kernel@vger.kernel.org;
> xuwei (O); Linuxarm; linux-a...@vger.kernel.org; zhichang.yuan; linux-
> p...@vger.kernel.org; o...@lixom.net; brian.star...@arm.com
> Subject: Re: [PATCH v9 2/7] PCI: Apply the new generic I/O management
> on PCI IO hosts
>
> On Thu, May 25, 2017 at 12:37:23PM +0100, Gabriele Paoloni wrote:
> > From: "zhichang.yuan" <yuanzhich...@hisilicon.com>
> >
> > After introducing the new generic I/O space management(LOGIC_IO), the
> > original PCI MMIO relevant helpers need to be updated based on the
> new
> > interfaces defined in LOGIC_IO.
> > This patch adapts the corresponding code to match the changes
> introduced
> > by LOGIC_IO.
> >
> > Signed-off-by: zhichang.yuan <yuanzhich...@hisilicon.com>
> > Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
> > Signed-off-by: Arnd Bergmann <a...@arndb.de>#earlier draft
>
> Not sure how you plan to merge this, but here's my ack:
>
> Acked-by: Bjorn Helgaas <bhelg...@google.com>
Many thanks for your Ack :)
Maybe Arnd can take the whole patchset (or just the lib framework + DT part
leaving the ACPI part to Lorenzo)?
>
> If you split this as suggested below, add my ack to all three patches.
Sure I will
>
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index b01bd5b..c9fe12b 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> >...
>
> > -int __weak pci_register_io_range(phys_addr_t addr, resource_size_t
> size)
> > +int pci_register_io_range(struct fwnode_handle *fwnode, phys_addr_t
> addr,
> > + resource_size_t size)
>
> It's trivial and nit-picky, but I would do the __weak removal in its
> own patch. It's obviously fine because there's only one
> implementation, but it's unrelated to the main point of this patch.
Agreed
>
> I would split the signature change (fwnode addition) to a separate
> patch, too, just to make the actual change more obvious, especially
> since that's the only part that crosses subsystems (ACPI, PCI, OF).
Agreed too
>
> > {
> > - int err = 0;
> > -
> > + int ret = 0;
> > #ifdef PCI_IOBASE
> > - struct io_range *range;
> > - resource_size_t allocated_size = 0;
> > -
> > - /* check if the range hasn't been previously recorded */
> > - spin_lock(_range_lock);
> > - list_for_each_entry(range, _range_list, list) {
> > - if (addr >= range->start && addr + size <= range->start +
> size) {
> > - /* range already registered, bail out */
> > - goto end_register;
> > - }
> > - allocated_size += range->size;
> > - }
> > -
> > - /* range not registed yet, check for available space */
> > - if (allocated_size + size - 1 > IO_SPACE_LIMIT) {
> > - /* if it's too big check if 64K space can be reserved */
> > - if (allocated_size + SZ_64K - 1 > IO_SPACE_LIMIT) {
> > - err = -E2BIG;
> > - goto end_register;
> > - }
> > -
> > - size = SZ_64K;
> > - pr_warn("Requested IO range too big, new size set to
> 64K\n");
> > - }
> > + struct logic_pio_hwaddr *range;
> >
> > - /* add the range to the list */
> > - range = kzalloc(sizeof(*range), GFP_ATOMIC);
> > - if (!range) {
> > - err = -ENOMEM;
> > - goto end_register;
> > - }
> > + if (!size || addr + size < addr)
> > + return -EINVAL;
> >
> > - range->start = addr;
> > + range = kzalloc(sizeof(*range), GFP_KERNEL);
> > + if (!range)
> > + return -ENOMEM;
>
> Add a blank line here.
Yep I will
Cheers
Gab
>
> > + range->fwnode = fwnode;
> > range->size = size;
> > + range->hw_start = addr;
> > + range->flags = PIO_CPU_MMIO;
> >
> > - list_add_tail(>list, _range_list);
> > -
> > -end_register:
> > - spin_unlock(_range_lock);
> > + ret = logic_pio_register_range(range);
> > + if (ret)
> > + kfree(range);
> > #endif
> >
> > - return err;
> > + return ret;
> > }
RE: [PATCH v9 2/7] PCI: Apply the new generic I/O management on PCI IO hosts
Hi Bjorn
> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: 26 May 2017 22:20
> To: Gabriele Paoloni
> Cc: catalin.mari...@arm.com; will.dea...@arm.com; robh...@kernel.org;
> frowand.l...@gmail.com; bhelg...@google.com; raf...@kernel.org;
> a...@arndb.de; linux-arm-ker...@lists.infradead.org;
> lorenzo.pieral...@arm.com; mark.rutl...@arm.com; miny...@acm.org;
> b...@kernel.crashing.org; John Garry; linux-kernel@vger.kernel.org;
> xuwei (O); Linuxarm; linux-a...@vger.kernel.org; zhichang.yuan; linux-
> p...@vger.kernel.org; o...@lixom.net; brian.star...@arm.com
> Subject: Re: [PATCH v9 2/7] PCI: Apply the new generic I/O management
> on PCI IO hosts
>
> On Thu, May 25, 2017 at 12:37:23PM +0100, Gabriele Paoloni wrote:
> > From: "zhichang.yuan"
> >
> > After introducing the new generic I/O space management(LOGIC_IO), the
> > original PCI MMIO relevant helpers need to be updated based on the
> new
> > interfaces defined in LOGIC_IO.
> > This patch adapts the corresponding code to match the changes
> introduced
> > by LOGIC_IO.
> >
> > Signed-off-by: zhichang.yuan
> > Signed-off-by: Gabriele Paoloni
> > Signed-off-by: Arnd Bergmann #earlier draft
>
> Not sure how you plan to merge this, but here's my ack:
>
> Acked-by: Bjorn Helgaas
Many thanks for your Ack :)
Maybe Arnd can take the whole patchset (or just the lib framework + DT part
leaving the ACPI part to Lorenzo)?
>
> If you split this as suggested below, add my ack to all three patches.
Sure I will
>
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index b01bd5b..c9fe12b 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> >...
>
> > -int __weak pci_register_io_range(phys_addr_t addr, resource_size_t
> size)
> > +int pci_register_io_range(struct fwnode_handle *fwnode, phys_addr_t
> addr,
> > + resource_size_t size)
>
> It's trivial and nit-picky, but I would do the __weak removal in its
> own patch. It's obviously fine because there's only one
> implementation, but it's unrelated to the main point of this patch.
Agreed
>
> I would split the signature change (fwnode addition) to a separate
> patch, too, just to make the actual change more obvious, especially
> since that's the only part that crosses subsystems (ACPI, PCI, OF).
Agreed too
>
> > {
> > - int err = 0;
> > -
> > + int ret = 0;
> > #ifdef PCI_IOBASE
> > - struct io_range *range;
> > - resource_size_t allocated_size = 0;
> > -
> > - /* check if the range hasn't been previously recorded */
> > - spin_lock(_range_lock);
> > - list_for_each_entry(range, _range_list, list) {
> > - if (addr >= range->start && addr + size <= range->start +
> size) {
> > - /* range already registered, bail out */
> > - goto end_register;
> > - }
> > - allocated_size += range->size;
> > - }
> > -
> > - /* range not registed yet, check for available space */
> > - if (allocated_size + size - 1 > IO_SPACE_LIMIT) {
> > - /* if it's too big check if 64K space can be reserved */
> > - if (allocated_size + SZ_64K - 1 > IO_SPACE_LIMIT) {
> > - err = -E2BIG;
> > - goto end_register;
> > - }
> > -
> > - size = SZ_64K;
> > - pr_warn("Requested IO range too big, new size set to
> 64K\n");
> > - }
> > + struct logic_pio_hwaddr *range;
> >
> > - /* add the range to the list */
> > - range = kzalloc(sizeof(*range), GFP_ATOMIC);
> > - if (!range) {
> > - err = -ENOMEM;
> > - goto end_register;
> > - }
> > + if (!size || addr + size < addr)
> > + return -EINVAL;
> >
> > - range->start = addr;
> > + range = kzalloc(sizeof(*range), GFP_KERNEL);
> > + if (!range)
> > + return -ENOMEM;
>
> Add a blank line here.
Yep I will
Cheers
Gab
>
> > + range->fwnode = fwnode;
> > range->size = size;
> > + range->hw_start = addr;
> > + range->flags = PIO_CPU_MMIO;
> >
> > - list_add_tail(>list, _range_list);
> > -
> > -end_register:
> > - spin_unlock(_range_lock);
> > + ret = logic_pio_register_range(range);
> > + if (ret)
> > + kfree(range);
> > #endif
> >
> > - return err;
> > + return ret;
> > }
RE: [PATCH v9 6/7] LPC: Add the ACPI LPC support
[...] > Hi zhichang.yuan, > > [auto build test ERROR on linus/master] > [also build test ERROR on v4.12-rc2 next-20170525] > [if your patch is applied to the wrong git tree, please drop us a note > to help improve the system] > > url:https://github.com/0day-ci/linux/commits/Gabriele-Paoloni/LPC- > legacy-ISA-I-O-support/20170526-033719 > config: ia64-allmodconfig (attached as .config) > compiler: ia64-linux-gcc (GCC) 6.2.0 > reproduce: > wget https://raw.githubusercontent.com/01org/lkp- > tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=ia64 > > All errors (new ones prefixed by >>): > >drivers/built-in.o: In function `lpc_host_io_setup': > >> hisi_lpc.c:(.text+0x252): undefined reference to > `acpi_set_logic_pio_resource' >drivers/built-in.o: In function `acpi_scan_init': >(.init.text+0x6742): undefined reference to > `acpi_indirectio_scan_init' I believe the problem is in patch 4/7 config HISILICON_LPC bool "Support for ISA I/O space on Hisilicon Hip0X" depends on (ARM64 && ARCH_HISI) || COMPILE_TEST the COMPILE_TEST above allowing HISILICON_LPC also for other architectures. I think we can remove "|| COMPILE_TEST"...I will fix it in v10 Gab + select LOGIC_PIO + select INDIRECT_PIO + help + Driver needed for some legacy ISA devices attached to Low-Pin-Count + on Hisilicon Hip0X SoC. > > --- > 0-DAY kernel test infrastructureOpen Source Technology > Center > https://lists.01.org/pipermail/kbuild-all Intel > Corporation
RE: [PATCH v9 6/7] LPC: Add the ACPI LPC support
[...] > Hi zhichang.yuan, > > [auto build test ERROR on linus/master] > [also build test ERROR on v4.12-rc2 next-20170525] > [if your patch is applied to the wrong git tree, please drop us a note > to help improve the system] > > url:https://github.com/0day-ci/linux/commits/Gabriele-Paoloni/LPC- > legacy-ISA-I-O-support/20170526-033719 > config: ia64-allmodconfig (attached as .config) > compiler: ia64-linux-gcc (GCC) 6.2.0 > reproduce: > wget https://raw.githubusercontent.com/01org/lkp- > tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=ia64 > > All errors (new ones prefixed by >>): > >drivers/built-in.o: In function `lpc_host_io_setup': > >> hisi_lpc.c:(.text+0x252): undefined reference to > `acpi_set_logic_pio_resource' >drivers/built-in.o: In function `acpi_scan_init': >(.init.text+0x6742): undefined reference to > `acpi_indirectio_scan_init' I believe the problem is in patch 4/7 config HISILICON_LPC bool "Support for ISA I/O space on Hisilicon Hip0X" depends on (ARM64 && ARCH_HISI) || COMPILE_TEST the COMPILE_TEST above allowing HISILICON_LPC also for other architectures. I think we can remove "|| COMPILE_TEST"...I will fix it in v10 Gab + select LOGIC_PIO + select INDIRECT_PIO + help + Driver needed for some legacy ISA devices attached to Low-Pin-Count + on Hisilicon Hip0X SoC. > > --- > 0-DAY kernel test infrastructureOpen Source Technology > Center > https://lists.01.org/pipermail/kbuild-all Intel > Corporation
[PATCH v9 7/7] MANTAINERS: Add maintainer for HiSilicon LPC driver
Added maintainer for drivers/bus/hisi_lpc.c Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com> --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9e98464..8fb9006 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6061,6 +6061,14 @@ F: include/uapi/linux/if_hippi.h F: net/802/hippi.c F: drivers/net/hippi/ +HISILICON LPC BUS DRIVER +M: Gabriele Paoloni <gabriele.paol...@huawei.com> +L: linux-arm-ker...@lists.infradead.org +W: http://www.hisilicon.com +S: Maintained +F: drivers/bus/hisi_lpc.c +F: Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt + HISILICON NETWORK SUBSYSTEM DRIVER M: Yisen Zhuang <yisen.zhu...@huawei.com> M: Salil Mehta <salil.me...@huawei.com> -- 2.7.4
[PATCH v9 7/7] MANTAINERS: Add maintainer for HiSilicon LPC driver
Added maintainer for drivers/bus/hisi_lpc.c Signed-off-by: Gabriele Paoloni --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9e98464..8fb9006 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6061,6 +6061,14 @@ F: include/uapi/linux/if_hippi.h F: net/802/hippi.c F: drivers/net/hippi/ +HISILICON LPC BUS DRIVER +M: Gabriele Paoloni +L: linux-arm-ker...@lists.infradead.org +W: http://www.hisilicon.com +S: Maintained +F: drivers/bus/hisi_lpc.c +F: Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt + HISILICON NETWORK SUBSYSTEM DRIVER M: Yisen Zhuang M: Salil Mehta -- 2.7.4
