Re: how to look for source code in kernel

[Jonathan Neuschäfer]

On Fri, Dec 28, 2012 at 11:49:53AM -0800, Eric W. Biederman wrote:
 Al Viro v...@zeniv.linux.org.uk writes:
 
  On Thu, Dec 27, 2012 at 11:36:13PM -0800, Eric W. Biederman wrote:
  But then I am probably peculiar keeping an index of the source code in
  my head.  When I need to look for something and I don't know where to
  find it I do.
  
  git-ls-files | xargs fgrep 'struct f2fs_inode'
 
  What's wrong with git grep?
 
 I haven't learned it yet.  git-ls-files is a lot better than find
 speed wise so is very much worth doing.I haven't a clue if
 there is an advantage to git-grep, over just knowing find, xargs, and
 grep.

Brevity.

Jonathan
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] soc: qcom: smsm: Avoid the use of an uninitialized value

[Jonathan Neuschäfer]

If qcom_smem_get returns an error besides -EPROBE_DEFER or -ENOENT,
don't assume that size has been set.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 drivers/soc/qcom/smsm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/smsm.c b/drivers/soc/qcom/smsm.c
index 3918645e5708..632c060c9cc0 100644
--- a/drivers/soc/qcom/smsm.c
+++ b/drivers/soc/qcom/smsm.c
@@ -441,7 +441,8 @@ static int smsm_get_size_info(struct qcom_smsm *smsm)
info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_SMSM_SIZE_INFO, );
if (PTR_ERR(info) == -EPROBE_DEFER) {
return PTR_ERR(info);
-   } else if (PTR_ERR(info) == -ENOENT || size != sizeof(*info)) {
+   } else if (PTR_ERR(info) == -ENOENT ||
+   (!IS_ERR(info) && size != sizeof(*info))) {
dev_warn(smsm->dev, "no smsm size info, using defaults\n");
smsm->num_entries = SMSM_DEFAULT_NUM_ENTRIES;
smsm->num_hosts = SMSM_DEFAULT_NUM_HOSTS;
-- 
2.11.0

[PATCH 1/2] soc: qcom: smsm: Handle probe deferral

[Jonathan Neuschäfer]

If qcom_smem_get or qcom_smem_alloc return -EPROBE_DEFER, let the caller
the caller handle it, instead of treating it as an error.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>

---
v1:
- TODO: Reading qcom_smsm_probe, I noticed memory leaks in error paths:
  smsm, smsm->entries, etc. are allocated (with devm_kzalloc), but not
  freed when the function returns early. This should be addressed at
  some point (in a separate patch).
---
 drivers/soc/qcom/smsm.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/smsm.c b/drivers/soc/qcom/smsm.c
index d0337b2a71c8..3918645e5708 100644
--- a/drivers/soc/qcom/smsm.c
+++ b/drivers/soc/qcom/smsm.c
@@ -439,7 +439,9 @@ static int smsm_get_size_info(struct qcom_smsm *smsm)
} *info;
 
info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_SMSM_SIZE_INFO, );
-   if (PTR_ERR(info) == -ENOENT || size != sizeof(*info)) {
+   if (PTR_ERR(info) == -EPROBE_DEFER) {
+   return PTR_ERR(info);
+   } else if (PTR_ERR(info) == -ENOENT || size != sizeof(*info)) {
dev_warn(smsm->dev, "no smsm size info, using defaults\n");
smsm->num_entries = SMSM_DEFAULT_NUM_ENTRIES;
smsm->num_hosts = SMSM_DEFAULT_NUM_HOSTS;
@@ -515,7 +517,9 @@ static int qcom_smsm_probe(struct platform_device *pdev)
/* Acquire the main SMSM state vector */
ret = qcom_smem_alloc(QCOM_SMEM_HOST_ANY, SMEM_SMSM_SHARED_STATE,
  smsm->num_entries * sizeof(u32));
-   if (ret < 0 && ret != -EEXIST) {
+   if (ret == -EPROBE_DEFER) {
+   return ret;
+   } else if (ret < 0 && ret != -EEXIST) {
dev_err(>dev, "unable to allocate shared state entry\n");
return ret;
}
-- 
2.11.0

Re: [PATCH 1/2] soc: qcom: smsm: Handle probe deferral

[Jonathan Neuschäfer]

On Wed, Mar 15, 2017 at 12:43:56PM +0100, Jonathan Neuschäfer wrote:
[...]
> - TODO: Reading qcom_smsm_probe, I noticed memory leaks in error paths:
>   smsm, smsm->entries, etc. are allocated (with devm_kzalloc), but not
>   freed when the function returns early. This should be addressed at
>   some point (in a separate patch).

This is not actually true (see Documentation/driver-model/devres.txt).
Sorry for the noise.


Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [PATCH 1/2] soc: qcom: smsm: Handle probe deferral

[Jonathan Neuschäfer]

On Mon, Mar 27, 2017 at 11:18:29PM -0700, Bjorn Andersson wrote:
> On Wed 15 Mar 04:43 PDT 2017, Jonathan Neusch?fer wrote:
[...]
> > info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_SMSM_SIZE_INFO, );
> > -   if (PTR_ERR(info) == -ENOENT || size != sizeof(*info)) {
> > +   if (PTR_ERR(info) == -EPROBE_DEFER) {
> > +   return PTR_ERR(info);
> > +   } else if (PTR_ERR(info) == -ENOENT || size != sizeof(*info)) {
> 
> The following elseif was supposed to take care of this case, but I
> clearly screwed this up.
> 
> Rather than adding a special case for EPROBE_DEFER before the two checks
> and then fix up the original expression to make errors fall back to the
> original else, I think you should rearrange the conditionals.
> 
> Probably better to write it like this instead:
> 
> if (IS_ERR(info) && PTR_ERR(info) != -ENOENT) {
> if (PTR_ERR(info) != -EPROBE_DEFER)
> dev_err(smsm->dev, "unable to retrieve smsm size 
> info\n");
> return PTR_ERR(info);
> } else if (IS_ERR(info) || size != sizeof(*info)) {
> dev_warn(smsm->dev, "no smsm size info, using defaults\n");
> smsm->num_entries = SMSM_DEFAULT_NUM_ENTRIES;
> smsm->num_hosts = SMSM_DEFAULT_NUM_HOSTS;
> return 0;
> }

Indeed, this looks better.

And it also obsoletes my patch 2/2, which is nice.

> > dev_warn(smsm->dev, "no smsm size info, using defaults\n");
> > smsm->num_entries = SMSM_DEFAULT_NUM_ENTRIES;
> > smsm->num_hosts = SMSM_DEFAULT_NUM_HOSTS;
> > @@ -515,7 +517,9 @@ static int qcom_smsm_probe(struct platform_device *pdev)
> > /* Acquire the main SMSM state vector */
> > ret = qcom_smem_alloc(QCOM_SMEM_HOST_ANY, SMEM_SMSM_SHARED_STATE,
> >   smsm->num_entries * sizeof(u32));
> > -   if (ret < 0 && ret != -EEXIST) {
> > +   if (ret == -EPROBE_DEFER) {
> > +   return ret;
> > +   } else if (ret < 0 && ret != -EEXIST) {
> 
> The idiomatic way to write this is:
> 
> if (ret < 0 && ret != -EEXIST) {
>   if (ret != -EPROBE_DEFER)
>   dev_err();
>   return ret;
> }
> 
> However, for us to reach this point in smsm_probe() the above
> qcom_smem_get() must have returned successfully, i.e. we have SMEM in
> place so there's no need to handle this case specifically.

I came to the same conclusion but wasn't sure. I'll drop this part from
my patch.

I'll send a v2 of this series, although after applying your suggestions,
I can't claim much originality anymore.


Thanks for the review,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [PATCH] drm/udl: Fix unaligned memory access in udl_render_hline

[Jonathan Neuschäfer]

On Tue, Apr 11, 2017 at 09:30:53AM -0400, Sean Paul wrote:
> On Fri, Apr 07, 2017 at 10:02:29PM +0200, Jonathan Neuschäfer wrote:
> > On SPARC, the udl driver filled my kernel log with these messages:
> > 
> > [186668.910612] Kernel unaligned access at TPC[76609c] 
> > udl_render_hline+0x13c/0x3a0
> > 
> > Use put_unaligned_be16 to avoid them. On x86 this results in the same
> > code, but on SPARC the compiler emits two single-byte stores.
> > 
> 
> Pushed to drm-misc-fixes with Dave's IRC Ack.
> 
> Thanks,
> 
> Sean

Thanks as well :-)


Jonathan


signature.asc
Description: PGP signature

[PATCH v2 1/2] ARM: dts: msm8974: Hook up adsp-pil's xo clock

[Jonathan Neuschäfer]

Without this patch (and with CONFIG_QCOM_ADSP_PIL), I get this error:

[0.711529] qcom_adsp_pil adsp-pil: failed to get xo clock
[0.711540] remoteproc remoteproc0: releasing adsp-pil

With this patch, adsp-pil can initialize correctly.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
Reviewed-by: Bjorn Andersson <bjorn.anders...@linaro.org>

---
v2:
* Add Bjorn Andersson's R-b
---
 arch/arm/boot/dts/qcom-msm8974.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi 
b/arch/arm/boot/dts/qcom-msm8974.dtsi
index d3e1a61b8671..cccd8cba8872 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -250,6 +250,9 @@
 
cx-supply = <_s2>;
 
+   clocks = <_board>;
+   clock-names = "xo";
+
memory-region = <_region>;
 
qcom,smem-states = <_smp2p_out 0>;
-- 
2.11.0

[PATCH v2 2/2] ARM: qcom_defconfig: Enable Qualcomm remoteproc and related drivers

[Jonathan Neuschäfer]

An adsp-pil node is present in at least the MSM8974 SoC. Simply enable
all Qualcomm remoteproc drivers to avoid more work in the future.

The SMP2P driver is required for adsp-pil to initialize correctly.

Enable the SMSM driver at Bjorn Andersson's request: "We also need
CONFIG_QCOM_SMSM=y here, its currently used to signal state of the ring
buffers for WiFi."

CONFIG_QCOM_WCNSS_CTRL is required to load firmware/configuration data
into the WCNSS core, which handles WiFi and Bluetooth.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
Acked-by: Bjorn Andersson <bjorn.anders...@linaro.org>

---
v2:
* Extend the commit message
* Enable CONFIG_QCOM_SMSM and add Bjorn Andersson's A-b
* Enable CONFIG_QCOM_WCNSS_CTRL as well
---
 arch/arm/configs/qcom_defconfig | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig
index 4ffdd607205d..07666a7a9de5 100644
--- a/arch/arm/configs/qcom_defconfig
+++ b/arch/arm/configs/qcom_defconfig
@@ -190,11 +190,18 @@ CONFIG_MDM_LCC_9615=y
 CONFIG_MSM_MMCC_8960=y
 CONFIG_MSM_MMCC_8974=y
 CONFIG_HWSPINLOCK_QCOM=y
+CONFIG_REMOTEPROC=y
+CONFIG_QCOM_ADSP_PIL=y
+CONFIG_QCOM_Q6V5_PIL=y
+CONFIG_QCOM_WCNSS_PIL=y
 CONFIG_QCOM_GSBI=y
 CONFIG_QCOM_PM=y
 CONFIG_QCOM_SMEM=y
 CONFIG_QCOM_SMD=y
 CONFIG_QCOM_SMD_RPM=y
+CONFIG_QCOM_SMP2P=y
+CONFIG_QCOM_SMSM=y
+CONFIG_QCOM_WCNSS_CTRL=y
 CONFIG_IIO=y
 CONFIG_IIO_BUFFER_CB=y
 CONFIG_IIO_SW_TRIGGER=y
-- 
2.11.0

[PATCH v2] soc: qcom: smsm: Improve error handling, quiesce probe deferral

[Jonathan Neuschäfer]

Don't use size if info indicates an error condition. Previously a
non-ENOENT error (such as -EPROBE_DEFER) would lead to size being used
even though it hadn't necessarily been initialized in qcom_smem_get.

Don't print an error message in the -EPROBE_DEFER case.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>

---
v2:
- Rewrite based on Bjorn's suggestion.
- Don't check for -EPROBE_DEFER again after qcom_smem_alloc was called
  in qcom_smsm_probe. This point is only reached if smsm_get_size_info
  returned success, which can only happen if probe deferral is over.
v1:
- was "[PATCH 1/2] soc: qcom: smsm: Handle probe deferral"
---
 drivers/soc/qcom/smsm.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/qcom/smsm.c b/drivers/soc/qcom/smsm.c
index d0337b2a71c8..dc540ea92e9d 100644
--- a/drivers/soc/qcom/smsm.c
+++ b/drivers/soc/qcom/smsm.c
@@ -439,14 +439,15 @@ static int smsm_get_size_info(struct qcom_smsm *smsm)
} *info;
 
info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_SMSM_SIZE_INFO, );
-   if (PTR_ERR(info) == -ENOENT || size != sizeof(*info)) {
+   if (IS_ERR(info) && PTR_ERR(info) != -ENOENT) {
+   if (PTR_ERR(info) != -EPROBE_DEFER)
+   dev_err(smsm->dev, "unable to retrieve smsm size 
info\n");
+   return PTR_ERR(info);
+   } else if (IS_ERR(info) || size != sizeof(*info)) {
dev_warn(smsm->dev, "no smsm size info, using defaults\n");
smsm->num_entries = SMSM_DEFAULT_NUM_ENTRIES;
smsm->num_hosts = SMSM_DEFAULT_NUM_HOSTS;
return 0;
-   } else if (IS_ERR(info)) {
-   dev_err(smsm->dev, "unable to retrieve smsm size info\n");
-   return PTR_ERR(info);
}
 
smsm->num_entries = info->num_entries;
-- 
2.11.0

[PATCH] drm/udl: Fix unaligned memory access in udl_render_hline

[Jonathan Neuschäfer]

On SPARC, the udl driver filled my kernel log with these messages:

[186668.910612] Kernel unaligned access at TPC[76609c] 
udl_render_hline+0x13c/0x3a0

Use put_unaligned_be16 to avoid them. On x86 this results in the same
code, but on SPARC the compiler emits two single-byte stores.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 drivers/gpu/drm/udl/udl_transfer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_transfer.c 
b/drivers/gpu/drm/udl/udl_transfer.c
index 917dcb978c2c..0c87b1ac6b68 100644
--- a/drivers/gpu/drm/udl/udl_transfer.c
+++ b/drivers/gpu/drm/udl/udl_transfer.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include "udl_drv.h"
@@ -163,7 +164,7 @@ static void udl_compress_hline16(
const u8 *const start = pixel;
const uint16_t repeating_pixel_val16 = pixel_val16;
 
-   *(uint16_t *)cmd = cpu_to_be16(pixel_val16);
+   put_unaligned_be16(pixel_val16, cmd);
 
cmd += 2;
pixel += bpp;
-- 
2.11.0

Re: [patches] [PATCH v7 05/15] irqchip: New RISC-V PLIC Driver

[Jonathan Neuschäfer]

Hi,

On Mon, Jul 31, 2017 at 05:59:59PM -0700, Palmer Dabbelt wrote:
> This patch adds a driver for the Platform Level Interrupt Controller
> (PLIC) specified as part of the RISC-V supervisor level ISA manual.
> The PLIC connocts global interrupt sources to the local interrupt

s/connocts/connects/

> controller on each hart.  A PLIC is present on all RISC-V systems.
> 
> Signed-off-by: Palmer Dabbelt <pal...@dabbelt.com>
> ---
[...]
> +/*
> + * From the RISC-V Privlidged Spec v1.10:
> + *
> + * Global interrupt sources are assigned small unsigned integer identifiers,
> + * beginning at the value 1.  An interrupt ID of 0 is reserved to mean “no
> + * interrupt”.  Interrupt identifiers are also used to break ties when two or
> + * more interrupt sources have the same assigned priority. Smaller values of
> + * interrupt ID take precedence over larger values of interrupt ID.
> + *
> + * While the RISC-V supervisor spec doesn't define the maximum number of
> + * devices supported by the PLIC, the largest number supported by devices
> + * marked as 'riscv,plic0' (which is the only device type this driver 
> supports,
> + * and is the only extant PLIC as of now) is 1024.  As mentioned above, 
> device
> + * 0 is defined to be non-existant so this device really only supports 1023
> + * devices.
> + */
> +#define MAX_DEVICES  1024
> +#define MAX_CONTEXTS 15872

How do you derive 15872 as the value of MAX_CONTEXTS?

> +
> +/*
> + * The PLIC consists of memory-mapped control registers, with a memory map as
> + * follows:
> + *
> + * base + 0x00: Reserved (interrupt source 0 does not exist)
> + * base + 0x04: Interrupt source 1 priority
> + * base + 0x08: Interrupt source 2 priority
> + * ...
> + * base + 0x000FFC: Interrupt source 1023 priority
> + * base + 0x001000: Pending 0
> + * base + 0x001FFF: Pending

"Pending"?

> + * base + 0x002000: Enable bits for sources 0-31 on context 0
> + * base + 0x002004: Enable bits for sources 32-63 on context 0
> + * ...

> + * base + 0x0020FC: Enable bits for sources 992-1023 on context 0
> + * base + 0x002080: Enable bits for sources 0-31 on context 1

This seems to overlap. Are more than 512 sources per context actually
possible?

> + * ...
> + * base + 0x002100: Enable bits for sources 0-31 on context 2
> + * ...
> + * base + 0x1F1F80: Enable bits for sources 992-1023 on context 15871
> + * base + 0x1F1F84: Reserved
> + * ...  (higher context IDs would fit here, but wouldn't fit
> + *   inside the per-context priority vector)
> + * base + 0x1C: Reserved
> + * base + 0x20: Priority threshold for context 0
> + * base + 0x24: Claim/complete for context 0
> + * base + 0x28: Reserved
> + * ...
> + * base + 0x200FFC: Reserved
> + * base + 0x201000: Priority threshold for context 1
> + * base + 0x201004: Claim/complete for context 1
> + * ...
> + * base + 0xFFE000: Priority threshold for context 15871
> + * base + 0xFFE004: Claim/complete for context 15871
> + * base + 0xFFF008: Reserved

0xFFE004 and 0xFFF008 are 0x1004 bytes apart. Is 0xFFF008 a typo?

> + * ...
> + * base + 0xFC: Reserved

As far as I can see, given that the Priority threshold/Claim/complete
area begins at base+0x20 and ends at base+0x100 (exclusive), and
the space occupied by one context is 0x1000 bytes, there should be space
for (0x100-0x20)/0x1000 = 0xe00 = 3584, not 15872 contexts.
Am I missing something?


Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [patches] Re: [PATCH v7 01/15] MAINTAINERS: Add RISC-V

[Jonathan Neuschäfer]

On Tue, Aug 01, 2017 at 04:03:28PM +0300, Andy Shevchenko wrote:
> On Mon, 2017-07-31 at 17:59 -0700, Palmer Dabbelt wrote:
> > From: Jonathan Neuschäfer <j.neuschae...@gmx.net>
> > 
> > RISC-V needs a MAINTAINERS entry. Let's add one.
> > 
> 
> Have you checked this by a brand new script (*) from Linus? 
> 
> (*) parse-maintainers.pl

I've checked the git version of this patch[0], and parse-maintainers.pl
doesn't move the RISC-V entry. (After fixing an unrelated error: SYNC
FILE FRAMEWORK is listed twice.)

[0]: 
https://github.com/riscv/riscv-linux/commit/281ce9f11e462f6d603516d56a02a9565d81587d


Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [patches] [PATCH 2/9] RISC-V: Atomic and Locking Code

[Jonathan Neuschäfer]

On Tue, Jul 04, 2017 at 12:50:55PM -0700, Palmer Dabbelt wrote:
[...]
> +/* These barries need to enforce ordering on both devices or memory. */

Very minor nit: s/barries/barriers/ (in several places)


Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [patches] [PATCH 1/9] RISC-V: Init and Halt Code

[Jonathan Neuschäfer]

On Thu, Jul 06, 2017 at 03:34:39PM -0700, Palmer Dabbelt wrote:
> On Tue, 04 Jul 2017 14:54:01 PDT (-0700), j.neuschae...@gmx.net wrote:
[...]
> >> +#define DO_ERROR_INFO(name, signo, code, str) 
> >> \
> >> +asmlinkage void name(struct pt_regs *regs)
> >> \
> >> +{ \
> >> +  do_trap_error(regs, signo, code, regs->sepc, "Oops - " str);\
> >> +}
> >> +
> >> +DO_ERROR_INFO(do_trap_unknown,
> >> +  SIGILL, ILL_ILLTRP, "unknown exception");
> >> +DO_ERROR_INFO(do_trap_insn_misaligned,
> >> +  SIGBUS, BUS_ADRALN, "instruction address misaligned");
> >> +DO_ERROR_INFO(do_trap_insn_fault,
> >> +  SIGBUS, BUS_ADRALN, "instruction access fault");
> >
> > For a general instruction access fault, BUS_ADRALN seems wrong. A
> > variant of SIGSEGV seems more appropriate, IMHO.
> 
> How does this look?
> 
>   diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
>   index 4c693b5b9980..3ce9ac6e736e 100644
>   --- a/arch/riscv/kernel/traps.c
>   +++ b/arch/riscv/kernel/traps.c
>   @@ -112,7 +112,7 @@ DO_ERROR_INFO(do_trap_unknown,
>DO_ERROR_INFO(do_trap_insn_misaligned,
>   SIGBUS, BUS_ADRALN, "instruction address misaligned");
>DO_ERROR_INFO(do_trap_insn_fault,
>   -   SIGBUS, BUS_ADRALN, "instruction access fault");
>   +   SIGBUS, SEGV_ACCERR, "instruction access fault");
>DO_ERROR_INFO(do_trap_insn_illegal,
>   SIGILL, ILL_ILLOPC, "illegal instruction");
>DO_ERROR_INFO(do_trap_load_misaligned,

I'm not familiar with the trap handling infrastructure, but looking at
include/uapi/asm-generic/siginfo.h, SEGV_ACCERR would alias to
BUS_ADRERR (both are defined as (__SI_FAULT|2)). So if you use SEGV_*,
you need to use SIGSEGV, too.

With DO_ERROR_INFO(..., SIGSEGV, SEGV_ACCERR, ...); it looks good to me.


Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [patches] Re: [PATCH 04/17] MAINTAINERS: Add RISC-V

[Jonathan Neuschäfer]

On Wed, Jul 12, 2017 at 01:31:17PM +0200, Arnd Bergmann wrote:
> On Wed, Jul 12, 2017 at 1:16 PM, James Hogan  wrote:
> > On Tue, Jul 11, 2017 at 06:31:17PM -0700, Palmer Dabbelt wrote:
[...]
> >> +RISC-V ARCHITECTURE
> >> +M:   Palmer Dabbelt 
> >> +M:   Albert Ou 
> >> +L:   patc...@groups.riscv.org
> >> +T:   git https://github.com/riscv/riscv-linux
> >> +S:   Supported
> >> +F:   arch/riscv/
> >
> > Should this include other tightly riscv specific drivers outside of
> > arch/riscv/ (or maybe they should have their own MAINTAINERS entries if
> > they aren't so tightly bound to the architecture or are maintained
> > separately)?
> >
> > drivers/clocksource/riscv_timer.c
> > drivers/irqchip/irq-riscv-intc.c
> > drivers/irqchip/irq-riscv-plic.c
> > drivers/tty/hvc/hvc_sbi.c
> > include/linux/timer_riscv.h
> >
> 
> I would suggest including a
> 
> K: riscv
> 
> keyword line that should catch the closely related drivers, and have
> separate entries for the rest. Maybe rename the hvc_sbi file
> to fall within that keyword match.
> 
> Arnd

All good suggestions. Palmer, would you mind to take this patch from
here, to reduce the communication overhead? (Besides, you know better
than I what to include, anyway)


Jonathan


signature.asc
Description: PGP signature

Re: [patches] [PATCH 17/17] RISC-V: Build Infastructure

[Jonathan Neuschäfer]

On Tue, Jul 11, 2017 at 06:31:30PM -0700, Palmer Dabbelt wrote:
> This patch contains all the build infastructure that actually enables
> the RISC-V port.  This includes Makefiles, linker scripts, and Kconfig
> files.  It also contains the only top-level change, which adds RISC-V to
> the list of architectures that need a sed run to produce the ARCH
> variable when building locally.
> 
> Signed-off-by: Palmer Dabbelt <pal...@dabbelt.com>
> ---
[...]
> +config ISA_C
> + bool "Emit compressed instructions when building Linux"

As a user, I'd prefer to have slightly more globally-recognisable names
than ISA_ for RISC-V instruction set architecture
extensions. A quick "git grep -A2 'config ISA'" shows the following
Kconfig symbols:

* ISA, ISA_BUS_API, ISA_DMA_API, ISAPNP:
  Settings related to the historic ISA bus.
* ISA_ARCOMPACT/ISA_ARCV2 (arch/arc),
  ISA_M32R/ISA_M32R2/ISA_DSP_LEVEL2/ISA_DUAL_ISSUE (arch/m32r):
  Instruction set options.

Four out of the six instruction set options have ARC/M32R in the name,
and I think that makes things slightly more readable. Therefore I
humbly propose something longer, and with a hint of RISC-V in the name,
such as ISA_RVC.

(Take this with a grain of salt, perhaps.)


Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [patches] [PATCH 17/17] RISC-V: Build Infastructure

[Jonathan Neuschäfer]

On Tue, Jul 25, 2017 at 10:20:50PM -0700, Palmer Dabbelt wrote:
> On Tue, 25 Jul 2017 19:57:17 PDT (-0700), j.neuschae...@gmx.net wrote:
> > On Tue, Jul 11, 2017 at 06:31:30PM -0700, Palmer Dabbelt wrote:
[...]
> >> +config ISA_C
> >> +  bool "Emit compressed instructions when building Linux"
> >
> > As a user, I'd prefer to have slightly more globally-recognisable names
> > than ISA_ for RISC-V instruction set architecture
> > extensions. A quick "git grep -A2 'config ISA'" shows the following
> > Kconfig symbols:
> >
> > * ISA, ISA_BUS_API, ISA_DMA_API, ISAPNP:
> >   Settings related to the historic ISA bus.
> > * ISA_ARCOMPACT/ISA_ARCV2 (arch/arc),
> >   ISA_M32R/ISA_M32R2/ISA_DSP_LEVEL2/ISA_DUAL_ISSUE (arch/m32r):
> >   Instruction set options.
> >
> > Four out of the six instruction set options have ARC/M32R in the name,
> > and I think that makes things slightly more readable. Therefore I
> > humbly propose something longer, and with a hint of RISC-V in the name,
> > such as ISA_RVC.
> >
> > (Take this with a grain of salt, perhaps.)
> 
> Good timing: I was about to submit a v6 patch set.  I'm cool with
> CONFIG_ISA_RVC and friends, do you mind submitting a patch?

I'm not sure about ISA_A, because as I understand the mails in one of
the previous review threads, RVA is now required by Linux, so there
shouldn't be a need for CONFIG_ISA_A (or an equivalent option).

CONFIG_RISCV_ISA_C (which Arnd suggested) makes it even clearer that
these are RISC-V related options.

Here's my patch, for reference (untested, because I currently don't have
a riscv compiler installed):


Subject: [PATCH] RISC-V: Rename CONFIG_ISA_C to CONFIG_ISA_RVC

To make it clearer that ISA_C is a RISC-V related option, rename it to
ISA_RVC.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 arch/riscv/Kconfig  | 2 +-
 arch/riscv/Makefile | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index cc274bbc29a7..8c43e5c73892 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -135,7 +135,7 @@ config TUNE_GENERIC
 
 endchoice
 
-config ISA_C
+config ISA_RVC
bool "Emit compressed instructions when building Linux"
default n
help
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 66c4a5e383f9..7ac91bcf9fe7 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -40,7 +40,7 @@ KBUILD_CFLAGS += -Wall
 ifeq ($(CONFIG_ISA_A),y)
KBUILD_ARCH_A = a
 endif
-ifeq ($(CONFIG_ISA_C),y)
+ifeq ($(CONFIG_ISA_RVC),y)
KBUILD_ARCH_C = c
 endif
 
-- 
2.11.0



signature.asc
Description: PGP signature

Re: [patches] [PATCH 1/9] RISC-V: Init and Halt Code

[Jonathan Neuschäfer]

Hi, below are some small comments.

On Tue, Jul 04, 2017 at 12:50:54PM -0700, Palmer Dabbelt wrote:
> This contains the various __init C functions, the initial assembly
> kernel entry point, and the code to reset the system.  When a file was
> init-related, it contains

It contains what?

> Signed-off-by: Palmer Dabbelt <pal...@dabbelt.com>
[...]

> +#ifdef CONFIG_GENERIC_BUG
> +#define __BUG_INSN   _AC(0x00100073, UL) /* sbreak */

This should be ebreak, not sbreak, in Priv Spec 1.10, AFAICT.
I guess binutils still understands sbreak, but it's nicer to stick to
the spec, IMHO.

> +#define BUG()\
> +do { \
> + __asm__ __volatile__ (  \
> + "1:\n\t"\
> + "sbreak\n"  \

ebreak

> + ".pushsection __bug_table,\"a\"\n\t"\
> + "2:\n\t"\
> + __BUG_ENTRY "\n\t"  \
> + ".org 2b + %2\n\t"  \
> + ".popsection"   \
> + :   \
> + : "i" (__FILE__), "i" (__LINE__),   \
> +   "i" (sizeof(struct bug_entry)));  \
> + unreachable();  \
> +} while (0)
> +#endif /* !__ASSEMBLY__ */
> +#else /* CONFIG_GENERIC_BUG */
> +#ifndef __ASSEMBLY__
> +#define BUG()\
> +do { \
> + __asm__ __volatile__ ("sbreak\n");  \

ebreak

> + unreachable();  \
> +} while (0)
> +#endif /* !__ASSEMBLY__ */
> +#endif /* CONFIG_GENERIC_BUG */
[...]

> +#define DO_ERROR_INFO(name, signo, code, str)
> \
> +asmlinkage void name(struct pt_regs *regs)   \
> +{\
> + do_trap_error(regs, signo, code, regs->sepc, "Oops - " str);\
> +}
> +
> +DO_ERROR_INFO(do_trap_unknown,
> + SIGILL, ILL_ILLTRP, "unknown exception");
> +DO_ERROR_INFO(do_trap_insn_misaligned,
> + SIGBUS, BUS_ADRALN, "instruction address misaligned");
> +DO_ERROR_INFO(do_trap_insn_fault,
> + SIGBUS, BUS_ADRALN, "instruction access fault");

For a general instruction access fault, BUS_ADRALN seems wrong. A
variant of SIGSEGV seems more appropriate, IMHO.


Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [PATCH 4/5] arm64: dts: Add ipq8074 SoC and MTP board support

[Jonathan Neuschäfer]

Hi,

On Fri, Apr 28, 2017 at 03:26:42PM +0530, Varadarajan Narayanan wrote:
> Subject: [PATCH 4/5] arm64: dts: Add ipq8074 SoC and MTP board support

s/MTP/HK01/ ?

> Add initial device tree support for the Qualcomm IPQ8074 SoC and
> HK01 evaluation board.
> 
> Signed-off-by: Manoharan Vijaya Raghavan <mragh...@codeaurora.org>
> Signed-off-by: Abhishek Sahu <abs...@codeaurora.org>
> Signed-off-by: Varadarajan Narayanan <var...@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/Makefile |   1 +
>  arch/arm64/boot/dts/qcom/ipq8074-hk01.dts |  48 ++
>  arch/arm64/boot/dts/qcom/ipq8074.dtsi | 153 
> ++
>  3 files changed, 202 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/ipq8074-hk01.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/ipq8074.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile 
> b/arch/arm64/boot/dts/qcom/Makefile
> index cc0f02d..7c6963e 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -4,6 +4,7 @@ dtb-$(CONFIG_ARCH_QCOM)   += msm8916-mtp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)  += msm8992-bullhead-rev-101.dtb
>  dtb-$(CONFIG_ARCH_QCOM)  += msm8994-angler-rev-101.dtb
>  dtb-$(CONFIG_ARCH_QCOM)  += msm8996-mtp.dtb
> +dtb-$(CONFIG_ARCH_QCOM)  += ipq8074-hk01.dtb

Maybe this list should be alphabetically sorted ('i' before 'm').
(I have no strong preference)


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

[PATCH 3/6] Documentation: kernel-docs: Remove "Here is its" at the end of lines

[Jonathan Neuschäfer]

Before commit 9e03ea7f683e ("Documentation/kernel-docs.txt: convert it to
ReST markup"), it read:

   Description: Linux Journal Kernel Korner article. Here is its
   abstract: "..."

In Sphinx' HTML formatting, however, the "Here is its" doesn't make
sense anymore, because the "Abstract:" is clearly separated.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/process/kernel-docs.rst | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/process/kernel-docs.rst 
b/Documentation/process/kernel-docs.rst
index 6ff8291194ee..a9aad381c3a4 100644
--- a/Documentation/process/kernel-docs.rst
+++ b/Documentation/process/kernel-docs.rst
@@ -356,7 +356,7 @@ On-line docs
   :URL: http://www.linuxjournal.com/article.php?sid=2391
   :Date: 1997
   :Keywords: RAID, MD driver.
-  :Description: Linux Journal Kernel Korner article. Here is its
+  :Description: Linux Journal Kernel Korner article.
   :Abstract: *A description of the implementation of the RAID-1,
 RAID-4 and RAID-5 personalities of the MD device driver in the
 Linux kernel, providing users with high performance and reliable,
@@ -381,7 +381,7 @@ On-line docs
   :Date: 1996
   :Keywords: device driver, module, loading/unloading modules,
 allocating resources.
-  :Description: Linux Journal Kernel Korner article. Here is its
+  :Description: Linux Journal Kernel Korner article.
   :Abstract: *This is the first of a series of four articles
 co-authored by Alessandro Rubini and Georg Zezchwitz which present
 a practical approach to writing Linux device drivers as kernel
@@ -397,7 +397,7 @@ On-line docs
   :Keywords: character driver, init_module, clean_up module,
 autodetection, mayor number, minor number, file operations,
 open(), close().
-  :Description: Linux Journal Kernel Korner article. Here is its
+  :Description: Linux Journal Kernel Korner article.
   :Abstract: *This article, the second of four, introduces part of
 the actual code to create custom module implementing a character
 device driver. It describes the code for module initialization and
@@ -410,7 +410,7 @@ On-line docs
   :Date: 1996
   :Keywords: read(), write(), select(), ioctl(), blocking/non
 blocking mode, interrupt handler.
-  :Description: Linux Journal Kernel Korner article. Here is its
+  :Description: Linux Journal Kernel Korner article.
   :Abstract: *This article, the third of four on writing character
 device drivers, introduces concepts of reading, writing, and using
 ioctl-calls*.
@@ -421,7 +421,7 @@ On-line docs
   :URL: http://www.linuxjournal.com/article.php?sid=1222
   :Date: 1996
   :Keywords: interrupts, irqs, DMA, bottom halves, task queues.
-  :Description: Linux Journal Kernel Korner article. Here is its
+  :Description: Linux Journal Kernel Korner article.
   :Abstract: *This is the fourth in a series of articles about
 writing character device drivers as loadable kernel modules. This
 month, we further investigate the field of interrupt handling.
-- 
2.11.0

[PATCH 5/6] Documentation: howto: Remove outdated info about bugzilla mailing lists

[Jonathan Neuschäfer]

The mailing list archives[1,2] show no activity since September 2011.

[1]: https://lists.linuxfoundation.org/pipermail/bugme-new/
[2]: https://lists.linuxfoundation.org/pipermail/bugme-janitors/

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/process/howto.rst | 8 
 1 file changed, 8 deletions(-)

diff --git a/Documentation/process/howto.rst b/Documentation/process/howto.rst
index 340fa18ff341..b696a51a832c 100644
--- a/Documentation/process/howto.rst
+++ b/Documentation/process/howto.rst
@@ -380,14 +380,6 @@ bugs is one of the best ways to get merits among other 
developers, because
 not many people like wasting time fixing other people's bugs.
 
 To work in the already reported bug reports, go to https://bugzilla.kernel.org.
-If you want to be advised of the future bug reports, you can subscribe to the
-bugme-new mailing list (only new bug reports are mailed here) or to the
-bugme-janitor mailing list (every change in the bugzilla is mailed here)
-
-   https://lists.linux-foundation.org/mailman/listinfo/bugme-new
-
-   https://lists.linux-foundation.org/mailman/listinfo/bugme-janitors
-
 
 
 Mailing lists
-- 
2.11.0

[PATCH 4/6] Documentation: Remove outdated info about -git patches

[Jonathan Neuschäfer]

Since the 3.2 cycle, there were no -git patches/tarballs on kernel.org.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/process/applying-patches.rst | 40 +-
 Documentation/process/howto.rst|  9 ---
 2 files changed, 1 insertion(+), 48 deletions(-)

diff --git a/Documentation/process/applying-patches.rst 
b/Documentation/process/applying-patches.rst
index a0d058cc6d25..eaf3e0296d6d 100644
--- a/Documentation/process/applying-patches.rst
+++ b/Documentation/process/applying-patches.rst
@@ -344,7 +344,7 @@ possible.
 
 This is a good branch to run for people who want to help out testing
 development kernels but do not want to run some of the really experimental
-stuff (such people should see the sections about -git and -mm kernels below).
+stuff (such people should see the sections about -mm kernels below).
 
 The -rc patches are not incremental, they apply to a base 4.x kernel, just
 like the 4.x.y patches described above. The kernel version before the -rcN
@@ -380,44 +380,6 @@ Here are 3 examples of how to apply these patches::
$ mv linux-4.7.3 linux-4.8-rc5  # rename the kernel source dir
 
 
-The -git kernels
-
-
-These are daily snapshots of Linus' kernel tree (managed in a git
-repository, hence the name).
-
-These patches are usually released daily and represent the current state of
-Linus's tree. They are more experimental than -rc kernels since they are
-generated automatically without even a cursory glance to see if they are
-sane.
-
--git patches are not incremental and apply either to a base 4.x kernel or
-a base 4.x-rc kernel -- you can see which from their name.
-A patch named 4.7-git1 applies to the 4.7 kernel source and a patch
-named 4.8-rc3-git2 applies to the source of the 4.8-rc3 kernel.
-
-Here are some examples of how to apply these patches::
-
-   # moving from 4.7 to 4.7-git1
-
-   $ cd ~/linux-4.7# change to the kernel source 
dir
-   $ patch -p1 < ../patch-4.7-git1 # apply the 4.7-git1 patch
-   $ cd ..
-   $ mv linux-4.7 linux-4.7-git1   # rename the kernel source dir
-
-   # moving from 4.7-git1 to 4.8-rc2-git3
-
-   $ cd ~/linux-4.7-git1   # change to the kernel source 
dir
-   $ patch -p1 -R < ../patch-4.7-git1  # revert the 4.7-git1 patch
-   # we now have a 4.7 kernel
-   $ patch -p1 < ../patch-4.8-rc2  # apply the 4.8-rc2 patch
-   # the kernel is now 4.8-rc2
-   $ patch -p1 < ../patch-4.8-rc2-git3 # apply the 4.8-rc2-git3 patch
-   # the kernel is now 4.8-rc2-git3
-   $ cd ..
-   $ mv linux-4.7-git1 linux-4.8-rc2-git3  # rename source dir
-
-
 The -mm patches and the linux-next tree
 ===
 
diff --git a/Documentation/process/howto.rst b/Documentation/process/howto.rst
index 1260f60d4cb9..340fa18ff341 100644
--- a/Documentation/process/howto.rst
+++ b/Documentation/process/howto.rst
@@ -314,15 +314,6 @@ The file Documentation/process/stable-kernel-rules.rst in 
the kernel tree
 documents what kinds of changes are acceptable for the -stable tree, and
 how the release process works.
 
-4.x -git patches
-
-
-These are daily snapshots of Linus' kernel tree which are managed in a
-git repository (hence the name.) These patches are usually released
-daily and represent the current state of Linus' tree.  They are more
-experimental than -rc kernels since they are generated automatically
-without even a cursory glance to see if they are sane.
-
 Subsystem Specific kernel trees and patches
 ~~~
 
-- 
2.11.0

[PATCH 6/6] Documentation: mono: Update links and s/CVS/Git/

[Jonathan Neuschäfer]

The old URLs redirect to the new ones, so just update them in mono.rst.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/admin-guide/mono.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/mono.rst 
b/Documentation/admin-guide/mono.rst
index cdddc099af64..77045ce548a0 100644
--- a/Documentation/admin-guide/mono.rst
+++ b/Documentation/admin-guide/mono.rst
@@ -9,14 +9,14 @@ This will allow you to execute Mono-based .NET binaries just 
like any
 other program after you have done the following:
 
 1) You MUST FIRST install the Mono CLR support, either by downloading
-   a binary package, a source tarball or by installing from CVS. Binary
+   a binary package, a source tarball or by installing from Git. Binary
packages for several distributions can be found at:
 
-   http://go-mono.com/download.html
+   http://www.mono-project.com/download/
 
Instructions for compiling Mono can be found at:
 
-   http://www.go-mono.com/compiling.html
+   http://www.mono-project.com/docs/compiling-mono/
 
Once the Mono CLR support has been installed, just check that
``/usr/bin/mono`` (which could be located elsewhere, for example
-- 
2.11.0

[PATCH 2/6] Documentation: kernel-docs: Move vfs.txt under "Docs at the Linux Kernel tree"

[Jonathan Neuschäfer]

It's unneccessary to point to an external mirror of the Documentation
directory. Also, drop the date field, because in-kernel documentation is
continually updated.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/process/kernel-docs.rst | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/Documentation/process/kernel-docs.rst 
b/Documentation/process/kernel-docs.rst
index 05a7857a4a83..6ff8291194ee 100644
--- a/Documentation/process/kernel-docs.rst
+++ b/Documentation/process/kernel-docs.rst
@@ -84,6 +84,17 @@ The Sphinx books should be built with ``make {htmldocs | 
pdfdocs | epubdocs}``.
 different". Freely redistributable under the conditions of the GNU
 General Public License.
 
+* Title: **Overview of the Virtual File System**
+
+  :Author: Richard Gooch.
+  :Location: Documentation/filesystems/vfs.txt
+  :Keywords: VFS, File System, mounting filesystems, opening files,
+dentries, dcache.
+  :Description: Brief introduction to the Linux Virtual File System.
+What is it, how it works, operations taken when opening a file or
+mounting a file system and description of important data
+structures explaining the purpose of each of their entries.
+
 On-line docs
 
 
@@ -127,18 +138,6 @@ On-line docs
 [...]. This paper examines some common problems for
 submitting larger changes and some strategies to avoid problems.
 
-* Title: **Overview of the Virtual File System**
-
-  :Author: Richard Gooch.
-  :URL: http://www.mjmwired.net/kernel/Documentation/filesystems/vfs.txt
-  :Date: 2007
-  :Keywords: VFS, File System, mounting filesystems, opening files,
-dentries, dcache.
-  :Description: Brief introduction to the Linux Virtual File System.
-What is it, how it works, operations taken when opening a file or
-mounting a file system and description of important data
-structures explaining the purpose of each of their entries.
-
 * Title: **Linux Device Drivers, Third Edition**
 
   :Author: Jonathan Corbet, Alessandro Rubini, Greg Kroah-Hartman
-- 
2.11.0

[PATCH 1/6] Documentation: coding-style: Escape \n\t to fix HTML rendering

[Jonathan Neuschäfer]

Without this patch, Sphinx renders the sentence as follows, thus hiding
the backslashes:

[...] end each string except the last with nt to
properly indent the next instruction [...]

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/process/coding-style.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/process/coding-style.rst 
b/Documentation/process/coding-style.rst
index d20d52a4d812..7710c7e0240c 100644
--- a/Documentation/process/coding-style.rst
+++ b/Documentation/process/coding-style.rst
@@ -980,7 +980,7 @@ do so, though, and doing so unnecessarily can limit 
optimization.
 
 When writing a single inline assembly statement containing multiple
 instructions, put each instruction on a separate line in a separate quoted
-string, and end each string except the last with \n\t to properly indent the
+string, and end each string except the last with ``\n\t`` to properly indent 
the
 next instruction in the assembly output:
 
 .. code-block:: c
-- 
2.11.0

Re: [PATCH] iommu: of: Fix check for returning EPROBE_DEFER

[Jonathan Neuschäfer]

Hi, I have a few suggestions on how to make this commit message more
readable:

On Tue, May 16, 2017 at 10:54:13AM +0530, Sricharan R wrote:
> While returning EPROBE_DEFER for iommu masters

Add a comma at the end of this line

> take in to account of iommu nodes that could be

s/in to/into/
s/of iommu/of_iommu/

> marked in DT as 'status=disabled', in which case
> simply return NULL and let the master's probe
> continue rather than deferring.
> 
> Signed-off-by: Sricharan R <sricha...@codeaurora.org>
> Tested-by: Will Deacon <will.dea...@arm.com>
> Acked-by: Will Deacon <will.dea...@arm.com>


Thanks,
Jonathan Neuschäfer

Re: [PATCH v2] soc: qcom: smsm: Improve error handling, quiesce probe deferral

[Jonathan Neuschäfer]

On Wed, Apr 05, 2017 at 10:01:23AM -0700, Bjorn Andersson wrote:
> On Wed 05 Apr 05:10 PDT 2017, Jonathan Neusch?fer wrote:
> 
> > Don't use size if info indicates an error condition. Previously a
> > non-ENOENT error (such as -EPROBE_DEFER) would lead to size being used
> > even though it hadn't necessarily been initialized in qcom_smem_get.
> > 
> > Don't print an error message in the -EPROBE_DEFER case.
> > 
> > Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
> 
> Reviewed-by: Bjorn Andersson <bjorn.anders...@linaro.org>

Ping. Andy, are you going to merge this patch?


Thanks,
Jonathan


signature.asc
Description: PGP signature

Re: [patches] [PATCH v8 08/18] irqchip: New RISC-V PLIC Driver

[Jonathan Neuschäfer]

Hi,

On Tue, Sep 12, 2017 at 02:57:05PM -0700, Palmer Dabbelt wrote:
> This patch adds a driver for the Platform Level Interrupt Controller
> (PLIC) specified as part of the RISC-V supervisor level ISA manual.
> The PLIC connocts global interrupt sources to the local interrupt

s/connocts/connects/?

> controller on each hart.  A PLIC is present on all RISC-V systems.
> 
> Signed-off-by: Palmer Dabbelt <pal...@dabbelt.com>
> ---
[...]
> +/*
> + * The PLIC consists of memory-mapped control registers, with a memory map as
> + * follows:

This memory map is mostly still as confusing as before (see my email in
the v7 thread).

> + *
> + * base + 0x00: Reserved (interrupt source 0 does not exist)
> + * base + 0x04: Interrupt source 1 priority
> + * base + 0x08: Interrupt source 2 priority
> + * ...
> + * base + 0x000FFC: Interrupt source 1023 priority
> + * base + 0x001000: Pending 0
> + * base + 0x001FFF: Pending
> + * base + 0x002000: Enable bits for sources 0-31 on context 0
> + * base + 0x002004: Enable bits for sources 32-63 on context 0
> + * ...
> + * base + 0x0020FC: Enable bits for sources 992-1023 on context 0
> + * base + 0x002080: Enable bits for sources 0-31 on context 1
> + * ...
> + * base + 0x002100: Enable bits for sources 0-31 on context 2
> + * ...
> + * base + 0x1F1F80: Enable bits for sources 992-1023 on context 15871
> + * base + 0x1F1F84: Reserved
> + * ...  (higher context IDs would fit here, but wouldn't fit
> + *   inside the per-context priority vector)
> + * base + 0x1C: Reserved
> + * base + 0x20: Priority threshold for context 0
> + * base + 0x24: Claim/complete for context 0
> + * base + 0x28: Reserved
> + * ...
> + * base + 0x200FFC: Reserved
> + * base + 0x201000: Priority threshold for context 1
> + * base + 0x201004: Claim/complete for context 1
> + * ...
> + * base + 0xFFE000: Priority threshold for context 15871
> + * base + 0xFFE004: Claim/complete for context 15871
> + * base + 0xFFE008: Reserved
> + * ...
> + * base + 0xFC: Reserved
> + */


Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [Patch v6 1/7] slimbus: Device management on SLIMbus

[Jonathan Neuschäfer]

 *   Bandwidth allocation is done dynamically by the manager based on active
> + *   channels on the bus, message-bandwidth requests made by slimbus devices.
> + *   Based on current bandwidth usage, manager chooses a frequency to run
> + *   the bus at (in steps of 'clock-gear', 1 through 10, each clock gear
> + *   representing twice the frequency than the previous gear).
> + *   Manager is also responsible for entering (and exiting) low-power-mode
> + *   (known as 'clock pause').
> + *   Manager can do handover of framer if there are multiple framers on the
> + *   bus and a certain usecase warrants using certain framer to avoid keeping
> + *   previous framer being powered-on.
> + *
> + *   Controller here performs duties of the manager device, and 'interface
> + *   device'. Interface device is responsible for monitoring the bus and
> + *   reporting information such as loss-of-synchronization, data
> + *   slot-collision.
> + * @dev: Device interface to this driver
> + * @nr: Board-specific number identifier for this controller/bus
> + * @list: Link with other slimbus controllers

I don't see list in the struct.

> + * @name: Name for this controller
> + * @min_cg: Minimum clock gear supported by this controller (default value: 
> 1)
> + * @max_cg: Maximum clock gear supported by this controller (default value: 
> 10)
> + * @clkgear: Current clock gear in which this bus is running
> + * @a_framer: Active framer which is clocking the bus managed by this 
> controller
> + * @m_ctrl: Mutex protecting controller data structures
> + * @addrt: Logical address table
> + * @num_dev: Number of active slimbus slaves on this bus
> + * @wq: Workqueue per controller used to notify devices when they report 
> present
> + * @xfer_msg: Transfer a message on this controller (this can be a broadcast
> + *   control/status message like data channel setup, or a unicast message
> + *   like value element read/write.

I don't see xfer_msg in the struct.

> + * @set_laddr: Setup logical address at laddr for the slave with elemental
> + *   address e_addr. Drivers implementing controller will be expected to
> + *   send unicast message to this device with its logical address.
> + * @get_laddr: It is possible that controller needs to set fixed logical
> + *   address table and get_laddr can be used in that case so that controller
> + *   can do this assignment.
> + */
> +struct slim_controller {
> + struct device   dev;
> + unsigned intnr;
> + charname[SLIMBUS_NAME_SIZE];
> + int min_cg;
> + int max_cg;
> + int clkgear;
> + struct slim_framer  *a_framer;
> + struct mutexm_ctrl;
> + struct slim_addrt   *addrt;
> + u8  num_dev;
> + struct workqueue_struct *wq;
> + int (*set_laddr)(struct slim_controller *ctrl,
> +  struct slim_eaddr *ea, u8 laddr);
> + int (*get_laddr)(struct slim_controller *ctrl,
> +  struct slim_eaddr *ea, u8 *laddr);
> +};


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [Patch v6 2/7] slimbus: Add messaging APIs to slimbus framework

[Jonathan Neuschäfer]

On Sat, Oct 07, 2017 at 11:24:33AM +0100, Srinivas Kandagatla wrote:
> Thanks for the comments.
> 
> On 07/10/17 07:42, Jonathan Neuschäfer wrote:
> > Hi,
> > 
> > On Fri, Oct 06, 2017 at 05:51:31PM +0200, srinivas.kandaga...@linaro.org 
> > wrote:
> > > From: Sagar Dharia <sdha...@codeaurora.org>
[...]
> > > +int slim_xfer_msg(struct slim_controller *ctrl,
> > > + struct slim_device *sbdev, struct slim_val_inf *msg,
> > > + u8 mc)
> > > +{
> > > + DEFINE_SLIM_LDEST_TXN(txn_stack, mc, 6, sbdev->laddr, msg);
> > > + struct slim_msg_txn *txn = _stack;
> > > + int ret;
> > > + u16 sl, cur;
> > > +
> > > + ret = slim_val_inf_sanity(ctrl, msg, mc);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + sl = slim_slicesize(msg->num_bytes);
> > > +
> > > + dev_dbg(>dev, "SB xfer msg:os:%x, len:%d, MC:%x, sl:%x\n",
> > > + msg->start_offset, msg->num_bytes, mc, sl);
> > > +
> > > + cur = slim_slicecodefromsize(sl);
> > > + txn->ec = ((sl | (1 << 3)) | ((msg->start_offset & 0xFFF) << 4));
> > 
> > Shouldn't this be (cur | (1 << 3)?

I misread the code here: I thought that cur was assigned the
"compressed" message length (in the range 0..7) here, but that's not
true, as slim_slicecodefromsize returns an "uncompressed" number. Thus
cur is a "quantized"[1] version of msg->num_bytes.

> cur seems to be redundant TBH, the only difference between cur and sl is
> that the slim_slicesize() can give slice size to program for any lengths
> between 1-16 bytes. However the slim_slicecodefromsize() can only handle
> 1,2,3,4, 6,8,12,16 byte sizes.

In any case, cur is only assigned and not used, as the code currently
is.

> So we can delete slim_slicecodefromsize() call and function together.
> looks like it was a leftover from downstream.

I agree. I don't know how it *might* be used, because I haven't read the
SLIMbus spec, but it is unused here.

> > (Also, what does cur mean? Cursor? Current?)
> No Idea!! :-) it is supposed to return slice size as per number of bytes.

Another problem solved by deleting slim_slicecodefromsize :-)

(As a small side-note, I think slim_slicesize and slim_slicecodefromsize
are named backwards: I would call sl, as used above, a "slice code",
because it encodes the message length)


> > > +/*
> > > + * slim_request_val_element: change and request a given value element
> 
> name should be fixed here..

Good catch.

> > > + * @sb: client handle requesting elemental message reads, writes.
> > > + * @msg: Input structure for start-offset, number of bytes to write.
> > > + * context: can sleep
> > > + * Returns:
> > > + * -EINVAL: Invalid parameters
> > > + * -ETIMEDOUT: If transmission of this message timed out (e.g. due to 
> > > bus lines
> > > + *   not being clocked or driven by controller)
> > > + * -ENOTCONN: If the transmitted message was not ACKed by destination 
> > > device.
> > 
> > Does rbuf contain the old value after this function finishes?
> > 
> Yep, device should send a reply value with the old value with matching tid.

I think you should document this in the comment to help readers.


Thanks,
Jonathan Neuschäfer

[1]: https://en.wikipedia.org/wiki/Quantization


signature.asc
Description: PGP signature

Re: [Patch v6 6/7] regmap: add SLIMBUS support

[Jonathan Neuschäfer]

Hi,

On Fri, Oct 06, 2017 at 05:51:35PM +0200, srinivas.kandaga...@linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>
> 
> This patch adds support to read/write slimbus value elements.
> Currently it only supports byte read/write. Adding this support in
> regmap would give codec drivers more flexibility when there are more
> than 2 control interfaces like slimbus, i2c.
> 
> Without this patch each codec driver has to directly call slimbus value
> element apis, and this could would get messy once we want to add i2c
> interface to it.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>
> ---
[...]
> +static int regmap_slimbus_byte_reg_read(void *context, unsigned int reg,
> + unsigned int *val)
> +{
> + struct slim_device *slim = context;
> + struct slim_val_inf msg = {0,};
> +
> + msg.start_offset = reg;
> + msg.num_bytes = 1;
> + msg.rbuf = (void *)val;
> +
> + return slim_request_val_element(slim, );
> +}

This looks like it won't work on big-endian systems. I know big endian
is pretty uncommon in devices that will likely have SLIMBus, but it's
better to be endian-independent.

> +static int regmap_slimbus_byte_reg_write(void *context, unsigned int reg,
> +  unsigned int val)
> +{
> + struct slim_device *slim = context;
> + struct slim_val_inf msg = {0,};
> +
> + msg.start_offset = reg;
> + msg.num_bytes = 1;
> + msg.wbuf = (void *)
> +
> + return slim_change_val_element(slim, );
> +}

dito

> +static struct regmap_bus regmap_slimbus_bus = {
> + .reg_write = regmap_slimbus_byte_reg_write,
> + .reg_read = regmap_slimbus_byte_reg_read,
> +};


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [Patch v6 3/7] slimbus: qcom: Add Qualcomm Slimbus controller driver

[Jonathan Neuschäfer]

lethread_workqueue("msm_slim_rx");
> + if (!dev->rxwq) {
> + dev_err(dev->dev, "Failed to start Rx WQ\n");
> + return -ENOMEM;
> + }
> +
> + dev->framer.rootfreq = SLIM_ROOT_FREQ >> 3;
> + dev->framer.superfreq =
> + dev->framer.rootfreq / SLIM_CL_PER_SUPERFRAME_DIV8;
> + dev->ctrl.a_framer = >framer;
> + dev->ctrl.clkgear = SLIM_MAX_CLK_GEAR;
> + dev->ctrl.dev.parent = >dev;
> + dev->ctrl.dev.of_node = pdev->dev.of_node;
> +
> + msm_slim_prg_slew(pdev, dev);
> +
> + ret = devm_request_irq(>dev, dev->irq, msm_slim_interrupt,
> + IRQF_TRIGGER_HIGH, "msm_slim_irq", dev);
> + if (ret) {
> + dev_err(>dev, "request IRQ failed\n");
> + goto err_request_irq_failed;
> + }
> +
> + ret = clk_prepare_enable(hclk);
> + if (ret)
> + goto err_hclk_enable_failed;
> +
> + ret = clk_prepare_enable(rclk);
> + if (ret)
> + goto err_rclk_enable_failed;
> +
> +
> + ctrl->tx.base = dma_alloc_coherent(>dev,
> +(ctrl->tx.sl_sz * ctrl->tx.n),
> +>tx.phy, GFP_KERNEL);

Use dmam_alloc_coherent?

> + if (!ctrl->tx.base) {
> + ret = -ENOMEM;
> + goto tx_alloc_failed;
> + }
> +
> + ctrl->rx.base = dma_alloc_coherent(>dev,
> +(ctrl->rx.sl_sz * ctrl->rx.n),
> +>rx.phy, GFP_KERNEL);

ditto

> + if (!ctrl->rx.base) {
> + ret = -ENOMEM;
> + goto rx_alloc_failed;
> + }
> +
> +
> + /* Register with framework before enabling frame, clock */
> + ret = slim_register_controller(>ctrl);
> + if (ret) {
> + dev_err(dev->dev, "error adding controller\n");
> + goto err_ctrl_failed;
> + }
> +
> + dev->ver = readl_relaxed(dev->base);
> + /* Version info in 16 MSbits */
> + dev->ver >>= 16;
> + /* Component register initialization */
> + writel_relaxed(1, dev->base + CFG_PORT(COMP_CFG, dev->ver));
> + writel_relaxed((EE_MGR_RSC_GRP | EE_NGD_2 | EE_NGD_1),
> + dev->base + CFG_PORT(COMP_TRUST_CFG, dev->ver));
> +
> + writel_relaxed((MGR_INT_TX_NACKED_2 |
> + MGR_INT_MSG_BUF_CONTE | MGR_INT_RX_MSG_RCVD |
> + MGR_INT_TX_MSG_SENT), dev->base + MGR_INT_EN);
> + writel_relaxed(1, dev->base + MGR_CFG);
> + /*
> +  * Framer registers are beyond 1K memory region after Manager and/or
> +  * component registers. Make sure those writes are ordered
> +  * before framer register writes
> +  */
> + wmb();
> +
> + /* Framer register initialization */
> + writel_relaxed((1 << INTR_WAKE) | (0xA << REF_CLK_GEAR) |
> + (0xA << CLK_GEAR) | (1 << ROOT_FREQ) | (1 << FRM_ACTIVE) | 1,
> + dev->base + FRM_CFG);
> + /*
> +  * Make sure that framer wake-up and enabling writes go through
> +  * before any other component is enabled. Framer is responsible for
> +  * clocking the bus and enabling framer first will ensure that other
> +  * devices can report presence when they are enabled
> +  */
> + mb();
> +
> + writel_relaxed(MGR_CFG_ENABLE, dev->base + MGR_CFG);
> + /*
> +  * Make sure that manager-enable is written through before interface
> +  * device is enabled
> +  */
> + mb();
> + writel_relaxed(1, dev->base + INTF_CFG);
> + /*
> +  * Make sure that interface-enable is written through before enabling
> +  * ported generic device inside MSM manager
> +  */
> + mb();
> +
> + writel_relaxed(1, dev->base + CFG_PORT(COMP_CFG, dev->ver));
> + /*
> +  * Make sure that all writes have gone through before exiting this
> +  * function
> +  */
> + mb();
> +
> + dev_dbg(dev->dev, "MSM SB controller is up:ver:0x%x!\n", dev->ver);
> + return 0;
> +
> +err_ctrl_failed:
> + dma_free_coherent(>dev, (ctrl->rx.sl_sz * ctrl->rx.n),
> +   ctrl->rx.base, ctrl->rx.phy);
> +rx_alloc_failed:
> + dma_free_coherent(ctrl->dev.parent, (ctrl->tx.sl_sz * ctrl->tx.n),
> +   ctrl->tx.base, ctrl->tx.phy);
> +tx_alloc_failed:
> + clk_disable_unprepare(dev->rclk);
> +err_rclk_enable_failed:
> + clk_disable_unprepare(dev->hclk);
> +
> +err_hclk_enable_failed:
> +err_request_irq_failed:
> + destroy_workqueue(dev->rxwq);
> + return ret;
> +}


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [Patch v6 4/7] slimbus: Add support for 'clock-pause' feature

[Jonathan Neuschäfer]

Hi, some trivial comments below.

On Fri, Oct 06, 2017 at 05:51:33PM +0200, srinivas.kandaga...@linaro.org wrote:
> From: Sagar Dharia <sdha...@codeaurora.org>
> 
> Per slimbus specification, a reconfiguration sequence known as
> 'clock pause' needs to be broadcast over the bus while entering low-
> power mode. Clock-pause is initiated by the controller driver.
> To exit clock-pause, controller typically wakes up the framer device.
> Since wakeup precedure is controller-specific, framework calls it via
> controller's function pointer to invoke it.
> 
> Signed-off-by: Sagar Dharia <sdha...@codeaurora.org>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>
> ---
[...]
> @@ -429,6 +444,14 @@ void slim_return_tx(struct slim_controller *ctrl, int 
> err)
>   cur.cb(cur.ctx, err);
>  
>   up(>tx_sem);
> + if (!cur.clk_pause && (!cur.need_tid || err)) {
> + /**

This isn't really a kerneldoc comment.

> +  * remove runtime-pm vote if this was TX only, or
> +  * if there was error during this transaction
> +  */
> + pm_runtime_mark_last_busy(ctrl->dev.parent);
> + pm_runtime_put_autosuspend(ctrl->dev.parent);
> + }
>  }
>  EXPORT_SYMBOL_GPL(slim_return_tx);
>  
[...]
> +/**
> + * slim_ctrl_clk_pause: Called by slimbus controller to enter/exit 'clock 
> pause'
> + * Slimbus specification needs this sequence to turn-off clocks for the bus.
> + * The sequence involves sending 3 broadcast messages (reconfiguration
> + * sequence) to inform all devices on the bus.
> + * To exit clock-pause, controller typically wakes up active framer device.
> + * @ctrl: controller requesting bus to be paused or woken up
> + * @wakeup: Wakeup this controller from clock pause.
> + * @restart: Restart time value per spec used for clock pause. This value
> + *   isn't used when controller is to be woken up.
> + * This API executes clock pause reconfiguration sequence if wakeup is false.
> + * If wakeup is true, controller's wakeup is called.
> + * For entering clock-pause, -EBUSY is returned if a message txn in pending.
> + */
> +int slim_ctrl_clk_pause(struct slim_controller *ctrl, bool wakeup, u8 
> restart)
> +{
> + int i, ret = 0;
> + unsigned long flags;
> + struct slim_sched *sched = >sched;
> + struct slim_val_inf msg = {0, 0, NULL, NULL, NULL, NULL};
> +
> + DEFINE_SLIM_BCAST_TXN(txn, SLIM_MSG_MC_BEGIN_RECONFIGURATION,
> + 3, SLIM_LA_MANAGER, );
> +
> + if (wakeup == false && restart > SLIM_CLK_UNSPECIFIED)
> + return -EINVAL;
> +
> + mutex_lock(>m_reconf);
> + if (wakeup) {
> + if (sched->clk_state == SLIM_CLK_ACTIVE) {
> + mutex_unlock(>m_reconf);
> + return 0;
> + }
> +
> + /**

ditto

> +  * Fine-tune calculation based on clock gear,
> +  * message-bandwidth after bandwidth management
> +  */
> + ret = wait_for_completion_timeout(>pause_comp,
> + msecs_to_jiffies(100));
> + if (!ret) {
> + mutex_unlock(>m_reconf);
> + pr_err("Previous clock pause did not finish");
> + return -ETIMEDOUT;
> + }
> + ret = 0;
> +
> + /**

ditto

> +  * Slimbus framework will call controller wakeup
> +  * Controller should make sure that it sets active framer
> +  * out of clock pause
> +  */
> + if (sched->clk_state == SLIM_CLK_PAUSED && ctrl->wakeup)
> + ret = ctrl->wakeup(ctrl);
> + if (!ret)
> + sched->clk_state = SLIM_CLK_ACTIVE;
> + mutex_unlock(>m_reconf);
> +
> + return ret;
> + }
[...]


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [Patch v6 5/7] slimbus: qcom: Add runtime-pm support using clock-pause feature

[Jonathan Neuschäfer]

Hi, some more trivial comments below.

On Fri, Oct 06, 2017 at 05:51:34PM +0200, srinivas.kandaga...@linaro.org wrote:
> From: Sagar Dharia <sdha...@codeaurora.org>
> 
> Slimbus HW mandates that clock-pause sequence has to be executed
> before disabling relevant interface and core clocks.
> Runtime-PM's autosuspend feature is used here to enter/exit low
> power mode for Qualcomm's Slimbus controller. Autosuspend feature
> enables driver to avoid changing power-modes too frequently since
> entering clock-pause is an expensive sequence
> 
> Signed-off-by: Sagar Dharia <sdha...@codeaurora.org>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>
> ---
[...]
> +static int msm_clk_pause_wakeup(struct slim_controller *ctrl)
> +{
> + struct msm_slim_ctrl *dev = slim_get_ctrldata(ctrl);
> +
> + clk_prepare_enable(dev->hclk);
> + clk_prepare_enable(dev->rclk);
> + enable_irq(dev->irq);
> +
> + writel_relaxed(1, dev->base + FRM_WAKEUP);
> + /* Make sure framer wakeup write goes through before ISR fires */
> + mb();
> + /**

This isn't really a kerneldoc comment.

> +  * HW Workaround: Currently, slave is reporting lost-sync messages
> +  * after slimbus comes out of clock pause.
> +  * Transaction with slave fail before slave reports that message
> +  * Give some time for that report to come
> +  * Slimbus wakes up in clock gear 10 at 24.576MHz. With each superframe
> +  * being 250 usecs, we wait for 5-10 superframes here to ensure
> +  * we get the message
> +  */
> + usleep_range(1250, 2500);
> + return 0;
> +}
[...]
> +#ifdef CONFIG_PM_SLEEP
> +static int msm_slim_suspend(struct device *dev)
> +{
> + int ret = 0;
> +
> + if (!pm_runtime_enabled(dev) ||
> + (!pm_runtime_suspended(dev))) {
> + dev_dbg(dev, "system suspend");
> + ret = msm_slim_runtime_suspend(dev);
> + }
> + if (ret == -EISCONN) {
> + /**

ditto.
Also, it looks misindented.

> +  * If the clock pause failed due to active channels, there is
> +  * a possibility that some audio stream is active during suspend.
> +  * (e.g. modem usecase during suspend)
> +  * We dont want to return suspend failure in that case so that
> +  * display and relevant components can still go to suspend.
> +  * If there is some other error, then it should prevent
> +  * system level suspend
> +  */
> + ret = 0;
> + }
> + return ret;
> +}


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [Patch v6 2/7] slimbus: Add messaging APIs to slimbus framework

[Jonathan Neuschäfer]

Hi,

On Fri, Oct 06, 2017 at 05:51:31PM +0200, srinivas.kandaga...@linaro.org wrote:
> From: Sagar Dharia <sdha...@codeaurora.org>
> 
> Slimbus devices use value-element, and information elements to
> control device parameters (e.g. value element is used to represent
> gain for codec, information element is used to represent interrupt
> status for codec when codec interrupt fires).
> Messaging APIs are used to set/get these value and information
> elements. Slimbus specification uses 8-bit "transaction IDs" for
> messages where a read-value is anticipated. Framework uses a table
> of pointers to store those TIDs and responds back to the caller in
> O(1).
> Caller can opt to do synchronous, or asynchronous reads/writes. For
> asynchronous operations, the callback will be called from atomic
> context.
> TX and RX circular rings are used to allow queuing of multiple
> transfers per controller. Controller can choose size of these rings
> based of controller HW implementation. The buffers are coerently

s/based of/based on/
s/coerently/coherently/

> mapped so that controller can utilize DMA operations for the
> transactions without remapping every transaction buffer.
> Statically allocated rings help to improve performance by avoiding
> overhead of dynamically allocating transactions on need basis.
> 
> Signed-off-by: Sagar Dharia <sdha...@codeaurora.org>
> Tested-by: Naveen Kaje <nk...@codeaurora.org>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>
> ---
[...]
> +static u16 slim_slicecodefromsize(u16 req)
> +{
> + static const u8 codetosize[8] = {1, 2, 3, 4, 6, 8, 12, 16};
> +
> + if (req >= ARRAY_SIZE(codetosize))
> + return 0;
> + else
> + return codetosize[req];
> +}
> +
> +static u16 slim_slicesize(int code)
> +{
> + static const u8 sizetocode[16] = {
> + 0, 1, 2, 3, 3, 4, 4, 5, 5, 5, 5, 6, 6, 6, 6, 7
> + };
> +
> + clamp(code, 1, (int)ARRAY_SIZE(sizetocode));
> + return sizetocode[code - 1];
> +}
> +
> +int slim_xfer_msg(struct slim_controller *ctrl,
> + struct slim_device *sbdev, struct slim_val_inf *msg,
> + u8 mc)
> +{
> + DEFINE_SLIM_LDEST_TXN(txn_stack, mc, 6, sbdev->laddr, msg);
> + struct slim_msg_txn *txn = _stack;
> + int ret;
> + u16 sl, cur;
> +
> + ret = slim_val_inf_sanity(ctrl, msg, mc);
> + if (ret)
> + return ret;
> +
> + sl = slim_slicesize(msg->num_bytes);
> +
> + dev_dbg(>dev, "SB xfer msg:os:%x, len:%d, MC:%x, sl:%x\n",
> + msg->start_offset, msg->num_bytes, mc, sl);
> +
> + cur = slim_slicecodefromsize(sl);
> + txn->ec = ((sl | (1 << 3)) | ((msg->start_offset & 0xFFF) << 4));

Shouldn't this be (cur | (1 << 3)?
(Also, what does cur mean? Cursor? Current?)

> +
> + switch (mc) {
> + case SLIM_MSG_MC_REQUEST_CHANGE_VALUE:
> + case SLIM_MSG_MC_CHANGE_VALUE:
> + case SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION:
> + case SLIM_MSG_MC_CLEAR_INFORMATION:
> + txn->rl += msg->num_bytes;
> + default:
> + break;
> + }
> +
> + if (slim_tid_txn(txn->mt, txn->mc))
> + txn->rl++;
> +
> + return slim_processtxn(ctrl, txn);
> +}
> +EXPORT_SYMBOL_GPL(slim_xfer_msg);
[...]
> +/*
> + * slim_request_val_element: change and request a given value element
> + * @sb: client handle requesting elemental message reads, writes.
> + * @msg: Input structure for start-offset, number of bytes to write.
> + * context: can sleep
> + * Returns:
> + * -EINVAL: Invalid parameters
> + * -ETIMEDOUT: If transmission of this message timed out (e.g. due to bus 
> lines
> + *   not being clocked or driven by controller)
> + * -ENOTCONN: If the transmitted message was not ACKed by destination device.

Does rbuf contain the old value after this function finishes?

> + */
> +int slim_request_change_val_element(struct slim_device *sb,
> + struct slim_val_inf *msg)
> +{
> + struct slim_controller *ctrl = sb->ctrl;
> +
> + if (!ctrl)
> + return -EINVAL;
> +
> + return slim_xfer_msg(ctrl, sb, msg, SLIM_MSG_MC_REQUEST_CHANGE_VALUE);
> +}
> +EXPORT_SYMBOL_GPL(slim_request_change_val_element);
[...]
> +/**
> + * struct slim_pending: context of pending transfers
> + * @cb: callback for this transfer
> + * @ctx: contex for the callback function

s/contex/context/

> + * @need_tid: True if this transfer need Transaction ID
> + */
> +struct slim_pending {
> + void (*cb)(void *ctx, int err);
> + void *ctx;
> + bool need_tid;
> +};


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [PATCH 2/3] ARM: qcom_defconfig: enable MSM IOMMU for display

[Jonathan Neuschäfer]

Hi,

On Mon, Nov 27, 2017 at 11:44:57AM +, srinivas.kandaga...@linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>
> 
> MSM IOMMU is required for apq8064 display, so enable it by default.

What exactly do you mean by "display"? Which ip block requires IOMMU
support?


Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [PATCH] drm/i915/guc: Fix doc reference to intel_guc_fw.c

[Jonathan Neuschäfer]

On Tue, Nov 28, 2017 at 09:51:13AM +0100, Michal Wajdeczko wrote:
> On Tue, 28 Nov 2017 07:50:52 +0100, Jonathan Neuschäfer
> <j.neuschae...@gmx.net> wrote:
> 
> > Sphinx complains that it can't find intel_guc_loader.c, and rightly so:
> > The file has been renamed.
> > 
> > Fixes: e8668bbcb0f9 ("drm/i915/guc: Rename intel_guc_loader.c to
> > intel_guc_fw.c")
> > Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
> > Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
> > ---
> >  Documentation/gpu/i915.rst | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
> > index 2e7ee0313c1c..e21698e16534 100644
> > --- a/Documentation/gpu/i915.rst
> > +++ b/Documentation/gpu/i915.rst
> > @@ -341,10 +341,10 @@ GuC
> >  GuC-specific firmware loader
> >  
> > -.. kernel-doc:: drivers/gpu/drm/i915/intel_guc_loader.c
> > +.. kernel-doc:: drivers/gpu/drm/i915/intel_guc_fw.c
> > :doc: GuC-specific firmware loader
> > -.. kernel-doc:: drivers/gpu/drm/i915/intel_guc_loader.c
> > +.. kernel-doc:: drivers/gpu/drm/i915/intel_guc_fw.c
> > :internal:
> > GuC-based command submission
> 
> + Ville
> 
> Well, this will fix sphinx error, but in my opinion it will not make
> i915 documentation any better. See my earlier patch/comments in [1].

Thanks for the pointer.

Hmm, right, given that there's no "DOC:" line in intel_guc_fw.c anymore,
the ":doc:" directive above is not useful.

As a tiny step towards more complete documentation (and to keep people
from patching this spot again ;), IMHO it makes sense to do this (it's
of course up to the maintainers whether they agree):

---
diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
index 2e7ee0313c1c..e94d3ac2bdd0 100644
--- a/Documentation/gpu/i915.rst
+++ b/Documentation/gpu/i915.rst
@@ -341,10 +341,7 @@ GuC
 GuC-specific firmware loader
 
 
-.. kernel-doc:: drivers/gpu/drm/i915/intel_guc_loader.c
-   :doc: GuC-specific firmware loader
-
-.. kernel-doc:: drivers/gpu/drm/i915/intel_guc_loader.c
+.. kernel-doc:: drivers/gpu/drm/i915/intel_guc_fw.c
    :internal:
 
 GuC-based command submission
---

> So maybe better to wait for other comments which way to go.

Makes sense.


Thanks,
Jonathan Neuschäfer

> 
> Thanks for the patch,
> Michal
> 
> [1] https://patchwork.freedesktop.org/patch/188424/


signature.asc
Description: PGP signature

Re: [PATCH 1/2] MAINTAINERS: regulator: Add Documentation/power/regulator/

[Jonathan Neuschäfer]

On Tue, Nov 28, 2017 at 03:15:37PM +, Mark Brown wrote:
> On Tue, Nov 28, 2017 at 05:22:02AM +0100, Jonathan Neuschäfer wrote:
> > On Sun, Nov 19, 2017 at 06:09:06AM +0100, Jonathan Neuschäfer wrote:
> > > Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
> > > ---
> > >  MAINTAINERS | 1 +
> > >  1 file changed, 1 insertion(+)
> > 
> > Ping.
> > 
> > Should I resend this series with Cc: linux-...@vger.kernel.org?
> 
> Please don't send content free pings and please allow a reasonable time
> for review.  People get busy, go on holiday, attend conferences and so 
> on so unless there is some reason for urgency (like critical bug fixes)
> please allow at least a couple of weeks for review.  If there have been
> review comments then people may be waiting for those to be addressed.

Okay. Sorry.

> Sending content free pings adds to the mail volume (if they are seen at
> all) which is often the problem and since they can't be reviewed
> directly if something has gone wrong you'll have to resend the patches
> anyway, though there are some other maintainers who like them - if in
> doubt look at how patches for the subsystem are normally handled.

Right, this makes sense.


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [PATCH v7 11/13] slimbus: qcom: Add Qualcomm Slimbus controller driver

[Jonathan Neuschäfer]

Hello Srinivas and Charles,

On Thu, Nov 23, 2017 at 10:07:03AM +, Charles Keepax wrote:
> On Wed, Nov 15, 2017 at 02:10:41PM +, srinivas.kandaga...@linaro.org 
> wrote:
> > From: Sagar Dharia <sdha...@codeaurora.org>
> > 
> > This controller driver programs manager, interface, and framer
> > devices for Qualcomm's slimbus HW block.
> > Manager component currently implements logical address setting,
> > and messaging interface.
> > Interface device reports bus synchronization information, and framer
> > device clocks the bus from the time it's woken up, until clock-pause
> > is executed by the manager device.
> > 
> > Signed-off-by: Sagar Dharia <sdha...@codeaurora.org>
> > Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>
> > ---
[...]
> > +static void qcom_slim_rxwq(struct work_struct *work)
> > +{
> > +   u8 buf[SLIM_MSGQ_BUF_LEN];
> > +   u8 mc, mt, len;
> > +   int i, ret;
> > +   struct qcom_slim_ctrl *ctrl = container_of(work, struct qcom_slim_ctrl,
> > +wd);
> > +
> > +   while ((slim_get_current_rxbuf(ctrl, buf)) != -ENODATA) {
> > +   len = SLIM_HEADER_GET_RL(buf[0]);
> > +   mt = SLIM_HEADER_GET_MT(buf[0]);
> > +   mc = SLIM_HEADER_GET_MC(buf[1]);
> > +   if (mt == SLIM_MSG_MT_CORE &&
> > +   mc == SLIM_MSG_MC_REPORT_PRESENT) {
> > +   u8 laddr;
> > +   struct slim_eaddr ea;
> > +   u8 e_addr[6];
> > +
> > +   for (i = 0; i < 6; i++)
> > +   e_addr[i] = buf[7-i];
> > +
> > +   ea.manf_id = (u16)(e_addr[5] << 8) | e_addr[4];
> > +   ea.prod_code = (u16)(e_addr[3] << 8) | e_addr[2];
> > +   ea.dev_index = e_addr[1];
> > +   ea.instance = e_addr[0];
> 
> If we are just bitshifting this out of the bytes does it really
> make it much more clear to reverse the byte order first? Feels
> like you might as well shift it out of buf directly.

In any case, there is a predefined function to make this code a little
nicer in :

le16_to_cpu(x):  Converts the 16-bit little endian value x to
 CPU-endian
le16_to_cpup(p): Converts the 16-bit little endian value pointed
 to by p to CPU-endian

If you use le16_to_cpup, you need to cast your pointer to __le16 *:

ea.manf_id = le16_to_cpup((__le16 *)_addr[4]);

Like Charles, I don't quite see the point of the for loop that fills
e_addr. I guess it did effectively a byteswap, so the original code,
that assumed little-endian, could simply dereference a u16 *. This does
not make a lot of sense anymore, once you use properly (CPU-)endian-
independent code. (Of course, you'll need to replace le16 with be16 if
you drop that loop.)


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [PATCH 1/2] MAINTAINERS: regulator: Add Documentation/power/regulator/

[Jonathan Neuschäfer]

On Sun, Nov 19, 2017 at 06:09:06AM +0100, Jonathan Neuschäfer wrote:
> Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)

Ping.

Should I resend this series with Cc: linux-...@vger.kernel.org?

> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2811a211632c..a644d41e088c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14447,6 +14447,7 @@ W:http://www.slimlogic.co.uk/?p=48
>  T:   git git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
>  S:   Supported
>  F:   Documentation/devicetree/bindings/regulator/
> +F:   Documentation/power/regulator/
>  F:   drivers/regulator/
>  F:   include/dt-bindings/regulator/
>  F:   include/linux/regulator/
> -- 
> 2.11.0
> 


signature.asc
Description: PGP signature

[PATCH] drm/i915/guc: Fix doc reference to intel_guc_fw.c

[Jonathan Neuschäfer]

Sphinx complains that it can't find intel_guc_loader.c, and rightly so:
The file has been renamed.

Fixes: e8668bbcb0f9 ("drm/i915/guc: Rename intel_guc_loader.c to 
intel_guc_fw.c")
Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/gpu/i915.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
index 2e7ee0313c1c..e21698e16534 100644
--- a/Documentation/gpu/i915.rst
+++ b/Documentation/gpu/i915.rst
@@ -341,10 +341,10 @@ GuC
 GuC-specific firmware loader
 
 
-.. kernel-doc:: drivers/gpu/drm/i915/intel_guc_loader.c
+.. kernel-doc:: drivers/gpu/drm/i915/intel_guc_fw.c
:doc: GuC-specific firmware loader
 
-.. kernel-doc:: drivers/gpu/drm/i915/intel_guc_loader.c
+.. kernel-doc:: drivers/gpu/drm/i915/intel_guc_fw.c
:internal:
 
 GuC-based command submission
-- 
2.15.0

[PATCH] genericirq.rst: Remove :c:func:`...` in code blocks

[Jonathan Neuschäfer]

In code blocks, :c:func:`...` annotations don't result in
cross-references. Instead, they are rendered verbatim.  Remove these
broken annotations, and mark function calls with parentheses() again.

Fixes: 76d40fae1351 ("genericirq.rst: add cross-reference links and use 
monospaced fonts")
Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/core-api/genericirq.rst | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/core-api/genericirq.rst 
b/Documentation/core-api/genericirq.rst
index 0054bd48be84..4da67b65cecf 100644
--- a/Documentation/core-api/genericirq.rst
+++ b/Documentation/core-api/genericirq.rst
@@ -225,9 +225,9 @@ interrupts.
 
 The following control flow is implemented (simplified excerpt)::
 
-:c:func:`desc->irq_data.chip->irq_mask_ack`;
+desc->irq_data.chip->irq_mask_ack();
 handle_irq_event(desc->action);
-:c:func:`desc->irq_data.chip->irq_unmask`;
+desc->irq_data.chip->irq_unmask();
 
 
 Default Fast EOI IRQ flow handler
@@ -239,7 +239,7 @@ which only need an EOI at the end of the handler.
 The following control flow is implemented (simplified excerpt)::
 
 handle_irq_event(desc->action);
-:c:func:`desc->irq_data.chip->irq_eoi`;
+desc->irq_data.chip->irq_eoi();
 
 
 Default Edge IRQ flow handler
@@ -251,15 +251,15 @@ interrupts.
 The following control flow is implemented (simplified excerpt)::
 
 if (desc->status & running) {
-:c:func:`desc->irq_data.chip->irq_mask_ack`;
+desc->irq_data.chip->irq_mask_ack();
 desc->status |= pending | masked;
 return;
 }
-:c:func:`desc->irq_data.chip->irq_ack`;
+desc->irq_data.chip->irq_ack();
 desc->status |= running;
 do {
 if (desc->status & masked)
-:c:func:`desc->irq_data.chip->irq_unmask`;
+desc->irq_data.chip->irq_unmask();
 desc->status &= ~pending;
 handle_irq_event(desc->action);
 } while (status & pending);
@@ -293,10 +293,10 @@ simplified version without locking.
 The following control flow is implemented (simplified excerpt)::
 
 if (desc->irq_data.chip->irq_ack)
-:c:func:`desc->irq_data.chip->irq_ack`;
+desc->irq_data.chip->irq_ack();
 handle_irq_event(desc->action);
 if (desc->irq_data.chip->irq_eoi)
-:c:func:`desc->irq_data.chip->irq_eoi`;
+desc->irq_data.chip->irq_eoi();
 
 
 EOI Edge IRQ flow handler
-- 
2.15.0

Re: [PATCH v8 01/13] Documentation: Add SLIMbus summary

[Jonathan Neuschäfer]

low-power mode 
> so
> +that corresponding clocks and/or power-rails can be turned off to save power.
> +Clock-pause is exited by waking up framer device (if controller driver 
> initiates
> +exiting low power mode), or by toggling the data line (if a slave device 
> wants
> +to initiate it).
> +
> +Messaging APIs:
> +---
> +The framework supports APIs to exchange control-information with a SLIMbus
> +device. APIs can be synchronous or asynchronous.
> +From controller's perspective, multiple buffers can be queued to/from
> +hardware for sending/receiving data using slim_ctrl_buf circular buffer.
> +The header file  has more documentation about messaging 
> APIs.

Once the kerneldoc documentation (i.e. the /** ... */ comments in the
source) is included somewhere, I think it would make sense to make
slim_ctrl_buf a clickable link to the struct's documentation.


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [PATCH] dt-bindings: chosen: Document linux,initrd-{start,end}

[Jonathan Neuschäfer]

On Fri, Dec 15, 2017 at 03:01:47PM -0600, Rob Herring wrote:
> On Sat, Dec 09, 2017 at 04:33:02PM +0100, Jonathan Neuschäfer wrote:
> > These properties have been in use for a very long time (at least since
> > 2005), but were never documented in chosen.txt.
> > 
> > Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
> > ---
> >  Documentation/devicetree/bindings/chosen.txt | 15 +++
> >  1 file changed, 15 insertions(+)
> 
> Applied.

Thanks.

> I'm inclined to say to document these in the DT spec, but I'm assuming 
> there was some reason why they weren't put into the spec (ePAPR at the 
> time) originally.

I don't know about the history of this, but I think if and when these
properties were specified in DTSpec, they should get a non-linux-specific
name, such as initrd-start/initrd-end, and a compatibility fallback to
linux,initrd-* (similar to stdout-path and phandle).


Jonathan Neuschäfer


signature.asc
Description: PGP signature

[PATCH] tools/gpio: Don't use u_int32_t

[Jonathan Neuschäfer]

u_int32_t is a non-standard version of uint32_t, that was apparently
introduced by BSD. Use uint32_t from stdint.h instead.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 tools/gpio/gpio-event-mon.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/gpio/gpio-event-mon.c b/tools/gpio/gpio-event-mon.c
index 1c14c2595158..be6768e21b09 100644
--- a/tools/gpio/gpio-event-mon.c
+++ b/tools/gpio/gpio-event-mon.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -27,8 +28,8 @@
 
 int monitor_device(const char *device_name,
   unsigned int line,
-  u_int32_t handleflags,
-  u_int32_t eventflags,
+  uint32_t handleflags,
+  uint32_t eventflags,
   unsigned int loops)
 {
struct gpioevent_request req;
@@ -145,8 +146,8 @@ int main(int argc, char **argv)
const char *device_name = NULL;
unsigned int line = -1;
unsigned int loops = 0;
-   u_int32_t handleflags = GPIOHANDLE_REQUEST_INPUT;
-   u_int32_t eventflags = 0;
+   uint32_t handleflags = GPIOHANDLE_REQUEST_INPUT;
+   uint32_t eventflags = 0;
int c;
 
while ((c = getopt(argc, argv, "c:n:o:dsrf?")) != -1) {
-- 
2.15.0

[PATCH] dt-bindings: display: panel: Fix compatible string for Toshiba LT089AC29000

[Jonathan Neuschäfer]

The compatible string for this panel was specified as
toshiba,lt089ac29000.txt. I believe this is a mistake.

Fixes: 06e733e41f87 ("drm/panel: simple: add Toshiba LT089AC19000")
Cc: Lucas Stach <l.st...@pengutronix.de>
Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 .../devicetree/bindings/display/panel/toshiba,lt089ac29000.txt  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/Documentation/devicetree/bindings/display/panel/toshiba,lt089ac29000.txt 
b/Documentation/devicetree/bindings/display/panel/toshiba,lt089ac29000.txt
index 4c0caaf246c9..89826116628c 100644
--- a/Documentation/devicetree/bindings/display/panel/toshiba,lt089ac29000.txt
+++ b/Documentation/devicetree/bindings/display/panel/toshiba,lt089ac29000.txt
@@ -1,7 +1,7 @@
 Toshiba 8.9" WXGA (1280x768) TFT LCD panel
 
 Required properties:
-- compatible: should be "toshiba,lt089ac29000.txt"
+- compatible: should be "toshiba,lt089ac29000"
 - power-supply: as specified in the base binding
 
 This binding is compatible with the simple-panel binding, which is specified
-- 
2.15.0

[PATCH] dt-bindings: trivial-devices: Remove fsl,mc13892

[Jonathan Neuschäfer]

This device's bindings are not trivial: Additional properties are
documented in in Documentation/devicetree/bindings/mfd/mc13xxx.txt.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/devicetree/bindings/trivial-devices.txt | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/trivial-devices.txt 
b/Documentation/devicetree/bindings/trivial-devices.txt
index af284fbd4d23..6e8fa1183210 100644
--- a/Documentation/devicetree/bindings/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/trivial-devices.txt
@@ -57,7 +57,6 @@ epson,rx8010  I2C-BUS INTERFACE REAL TIME CLOCK MODULE
 epson,rx8025   High-Stability. I2C-Bus INTERFACE REAL TIME CLOCK MODULE
 epson,rx8581   I2C-BUS INTERFACE REAL TIME CLOCK MODULE
 fsl,mag3110MAG3110: Xtrinsic High Accuracy, 3D Magnetometer
-fsl,mc13892MC13892: Power Management Integrated Circuit (PMIC) for 
i.MX35/51
 fsl,mma7660MMA7660FC: 3-Axis Orientation/Motion Detection Sensor
 fsl,mma8450MMA8450Q: Xtrinsic Low-power, 3-axis Xtrinsic 
Accelerometer
 fsl,mpl3115MPL3115: Absolute Digital Pressure Sensor
-- 
2.11.0

Re: [PATCH] dt-bindings: Add a RISC-V SBI firmware node

[Jonathan Neuschäfer]

On Mon, Nov 20, 2017 at 11:50:00AM -0800, Palmer Dabbelt wrote:
> The RISC-V privileged ISA mandates the presence of an SBI, but there's
> no reason not to put it in the device tree.  This would allow us to
> possibly remove the SBI later.

Thanks!

> 
> CC: Jonathan Neuschäfer <j.neuschae...@gmx.net>
> Signed-off-by: Palmer Dabbelt <pal...@sifive.com>
> ---
>  .../devicetree/bindings/firmware/riscv.sbi.txt   | 20 
> 
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/riscv.sbi.txt
> 
> diff --git a/Documentation/devicetree/bindings/firmware/riscv.sbi.txt 
> b/Documentation/devicetree/bindings/firmware/riscv.sbi.txt
> new file mode 100644
> index ..42384d5d52cf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/riscv.sbi.txt

Nit: Other bindings use either a comma (as in the compatible string,
"riscv,sbi.txt") or a dash (vendor-product.txt, "riscv-sbi.txt") in the
file name.

> @@ -0,0 +1,20 @@
> +RISC-V Supervisor Binary Interface (SBI)
> +
> +The RISC-V privileged ISA specification mandates the presence of a supervisor
> +binary interface that performs some operations which might otherwise require
> +particularly complicated instructions.  This interface includes
> +inter-processor interrupts, TLB flushes, i-cache and TLB shootdowns, a
> +console, and power management.
> +
> +Required properties:
> +- compatible: must contain one of the following
> + * "riscv,sbi" for the SBI defined by the privileged specification of the
> +   system.

"of the system" seems to imply that different RISC-V systems (different
RISC-V implementations) can have different privileged specifications.

I think it's better to refer to concrete documents, that don't depend on
the rest of the system, instead. Either:

 * "riscv,sbi" for the SBI defined by the RISC-V Privileged ISA Specification.

Or something like:

 * "sifive,sbi" for the SBI defined by SiFive document XYZ.


[ I know that there currently is no SBI spec, because the chapter has
  been removed from the Priv Spec, but this can be fixed later, once
  the final name of the document describing the SBI is clear. ]

> +
> +Example:
> +
> +firmware {
> + sbi {
> + compatible = "riscv,sbi";
> + };
> +};
> -- 


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [patches] Re: [PATCH] dt-bindings: Add a RISC-V SBI firmware node

[Jonathan Neuschäfer]

On Mon, Nov 20, 2017 at 01:28:01PM -0800, Palmer Dabbelt wrote:
[...]
> > > +++ b/Documentation/devicetree/bindings/firmware/riscv.sbi.txt
> > 
> > Nit: Other bindings use either a comma (as in the compatible string,
> > "riscv,sbi.txt") or a dash (vendor-product.txt, "riscv-sbi.txt") in the
> > file name.
> 
> That was just a typo, I'll fix it.

Ok

> > > @@ -0,0 +1,20 @@
> > > +RISC-V Supervisor Binary Interface (SBI)
> > > +
> > > +The RISC-V privileged ISA specification mandates the presence of a 
> > > supervisor
> > > +binary interface that performs some operations which might otherwise 
> > > require
> > > +particularly complicated instructions.  This interface includes
> > > +inter-processor interrupts, TLB flushes, i-cache and TLB shootdowns, a
> > > +console, and power management.
> > > +
> > > +Required properties:
> > > +- compatible: must contain one of the following
> > > + * "riscv,sbi" for the SBI defined by the privileged specification of the
> > > +   system.
> > 
> > "of the system" seems to imply that different RISC-V systems (different
> > RISC-V implementations) can have different privileged specifications.
> 
> Actually, that was intentional -- I wrote it this way because different
> RISC-V systems do have different privileged specifications.  The RISC-V
> specifications aren't frozen in time, they're just guaranteed to be
> compatible in the future.  For example, the user ISA document has been
> updated multiple times (the C spec, eliminating some unspecified behavior)
> and will continue to be updated (V and other extensions, the memory model).
> The privileged spec will be updated in a compatible way just like the user
> spec will be -- I know there's at least hypervisor support in the works, and
> I saw some things to remove undefined behavior go past as well.
> 
> In a similar fashion, the ABI and SBI will continue to evolve.  For example,
> we'll probably add new system calls to extend the user ABI and new hyper
> calls to extend the SBI.

My problem with the wording was that the OS somehow has to know which
version and variant of the SBI it is talking to -- either through
in-band communication (an SBI call to request SBI information, etc.), or
through devicetree or similar mechanisms.

> 
> > I think it's better to refer to concrete documents, that don't depend on
> > the rest of the system, instead. Either:


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [PATCH] dt-bindings: trivial-devices: Remove fsl,mc13892

[Jonathan Neuschäfer]

On Mon, Nov 20, 2017 at 03:10:45PM -0600, Rob Herring wrote:
> On Sat, Nov 18, 2017 at 03:22:32AM +0100, Jonathan Neuschäfer wrote:
> > This device's bindings are not trivial: Additional properties are
> > documented in in Documentation/devicetree/bindings/mfd/mc13xxx.txt.
> > 
> > Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
> > ---
> >  Documentation/devicetree/bindings/trivial-devices.txt | 1 -
> >  1 file changed, 1 deletion(-)
> 
> Applied.

Thanks!


signature.asc
Description: PGP signature

[PATCH 1/2] MAINTAINERS: regulator: Add Documentation/power/regulator/

[Jonathan Neuschäfer]

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2811a211632c..a644d41e088c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14447,6 +14447,7 @@ W:  http://www.slimlogic.co.uk/?p=48
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
 S: Supported
 F: Documentation/devicetree/bindings/regulator/
+F: Documentation/power/regulator/
 F: drivers/regulator/
 F: include/dt-bindings/regulator/
 F: include/linux/regulator/
-- 
2.11.0

[PATCH 2/2] regulator: Update code examples in documentation

[Jonathan Neuschäfer]

This involves using the REGULATOR_SUPPLY initializer macro and
reindenting some of the code.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/power/regulator/machine.txt | 36 ++-
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/Documentation/power/regulator/machine.txt 
b/Documentation/power/regulator/machine.txt
index 757e3b53dc11..eff4dcaaa252 100644
--- a/Documentation/power/regulator/machine.txt
+++ b/Documentation/power/regulator/machine.txt
@@ -23,16 +23,12 @@ struct regulator_consumer_supply {
 e.g. for the machine above
 
 static struct regulator_consumer_supply regulator1_consumers[] = {
-{
-   .dev_name   = "dev_name(consumer B)",
-   .supply = "Vcc",
-},};
+   REGULATOR_SUPPLY("Vcc", "consumer B"),
+};
 
 static struct regulator_consumer_supply regulator2_consumers[] = {
-{
-   .dev= "dev_name(consumer A"),
-   .supply = "Vcc",
-},};
+   REGULATOR_SUPPLY("Vcc", "consumer A"),
+};
 
 This maps Regulator-1 to the 'Vcc' supply for Consumer B and maps Regulator-2
 to the 'Vcc' supply for Consumer A.
@@ -78,20 +74,20 @@ static struct regulator_init_data regulator2_data = {
 Finally the regulator devices must be registered in the usual manner.
 
 static struct platform_device regulator_devices[] = {
-{
-   .name = "regulator",
-   .id = DCDC_1,
-   .dev = {
-   .platform_data = _data,
+   {
+   .name = "regulator",
+   .id = DCDC_1,
+   .dev = {
+   .platform_data = _data,
+   },
},
-},
-{
-   .name = "regulator",
-   .id = DCDC_2,
-   .dev = {
-   .platform_data = _data,
+   {
+   .name = "regulator",
+   .id = DCDC_2,
+   .dev = {
+   .platform_data = _data,
+   },
},
-},
 };
 /* register regulator 1 device */
 platform_device_register(_devices[0]);
-- 
2.11.0

Re: [patches] Re: [PATCH v9 03/12] dt-bindings: RISC-V CPU Bindings

[Jonathan Neuschäfer]

Hi Palmer,

On Thu, Oct 05, 2017 at 11:16:33AM +0100, Mark Rutland wrote:
[...]
> I would *strongly* recommend that from day one, you determine the SMP
> bringup mechanism via an enable-method property, and document the
> contract with FW/bootloader somewhere in the kernel tree.

Somewhat, but not quite related: Please consider making the availability
of the Supervisor Binary Interface explicit in the devicetree.
I understand that the general plan is to make the SBI a mandatory
feature of every RISC-V system capable of running Linux, but I do want
to explore the possibility of running without run-time resident firmware
at some point in the future. Thus it would be nice if the devicetree
would indicate the presence of the SBI from the start, to avoid having
to invent a way to express its *absence* later on.

It could look something like this (modelled after qcom,scm):

/ {
firmware {
sbi {
compatible = "riscv,sbi";
};
};
};

This topic may warrant some discussion, because other people may have
different opinions, and there hasn't been a discussion about it, AFAICS.


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [patches] Re: [PATCH] dt-bindings: Add a RISC-V SBI firmware node

[Jonathan Neuschäfer]

On Tue, Nov 21, 2017 at 09:37:02AM -0800, Palmer Dabbelt wrote:
[...]
> This isn't really a big deal to me, as I'm only interested in RISC-V
> systems, but there's been some pushback on the concept of an SBI so it
> seemed like a simple way to allow people to build non-SBI (and there for not
> really RISC-V) systems.

For those reading along: I suggested the /firmware/sbi node to Palmer,
because I'm interested in such "not really RISC-V" systems, (because it
makes the firmware's job easier to not implement the SBI — speaking with
my coreboot hat, here.)

> One option that wouldn't require a device tree node
> would be to have Linux boot in machine mode [...] and then provide its
> own SBI implementation.

I think this can work.


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [PATCH v2 5/5] samples: Introduce Qualcomm QMI sample client

[Jonathan Neuschäfer]

Hi, some small comments below.

On Mon, Nov 06, 2017 at 09:20:42PM -0800, Bjorn Andersson wrote:
> Introduce a sample driver that register for server notifications and
> spawn clients for each available test service (service 15). The spawned
> clients implements the interface for encoding "ping" and "data" requests
> and decode the responses from the remote.
> 
> Signed-off-by: Bjorn Andersson <bjorn.anders...@linaro.org>
> ---
[...]
> +/*
> + * ping_pong_store() - ping_pong debugfs file write handler

The function name is ping_write now.

> + * @file:debugfs file context
> + * @user_buf:reference to the user data
> + * @count:   number of bytes in @user_buf
> + * @ppos:offset in @file to write
> + *
> + * Returns @count, or negative errno on failure.
> + *
> + * This function allows user space to send out a ping_pong QMI encoded 
> message
> + * to the associated remote test service and will return with the result of 
> the
> + * transaction. It serves as an example of how to provide a custom response
> + * handler.
> + */
> +static ssize_t ping_write(struct file *file, const char __user *user_buf,
> +   size_t count, loff_t *ppos)

The data in user_buf is completely ignored, right? Perhaps that's worth
mentioning in the doc comment.

> +{
> + struct qmi_handle *qmi = file->private_data;
> + struct test_ping_req_msg_v01 req = {0};
> + struct qmi_txn txn;
> + int ret;
> +
> + memcpy(req.ping, "ping", sizeof(req.ping));
> +
> + ret = qmi_txn_init(qmi, , NULL, NULL);
> + if (ret < 0)
> + return ret;
> +
> + ret = qmi_send_request(qmi, NULL, ,
> +TEST_PING_REQ_MSG_ID_V01,
> +TEST_PING_REQ_MAX_MSG_LEN_V01,
> +test_ping_req_msg_v01_ei, );
> + if (ret < 0) {
> + qmi_txn_cancel();
> + return ret;
> + }
> +
> + ret = qmi_txn_wait(, 5 * HZ);
> + if (ret < 0)
> + count = ret;
> +
> + return count;
> +}
[...]
> +
> +/*
> + * data_store() - data debugfs file write handler

data_write

> + * @file:debugfs file context
> + * @user_buf:reference to the user data
> + * @count:   number of bytes in @user_buf
> + * @ppos:offset in @file to write
> + *
> + * Returns @count, or negative errno on failure.
> + *
> + * This function allows user space to send out a data QMI encoded message to
> + * the associated remote test service and will return with the result of the
> + * transaction. It serves as an example of how to have the QMI helpers 
> decode a
> + * transaction response into a provided object automatically.
> + */
> +static ssize_t data_write(struct file *file, const char __user *user_buf,
> +   size_t count, loff_t *ppos)



Jonathan Neuschäfer


signature.asc
Description: PGP signature

[PATCH] Documentation: mono: Update links and s/CVS/Git/

[Jonathan Neuschäfer]

The URLs in mono.rst redirect to pages on www.mono-project.com, so let's
update them. I took the liberty to update the compilation instructions
to the Linux-specific version, because readers of the kernel
documentation will most likely use Linux.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/admin-guide/mono.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/mono.rst 
b/Documentation/admin-guide/mono.rst
index cdddc099af64..59e6d59f0ed9 100644
--- a/Documentation/admin-guide/mono.rst
+++ b/Documentation/admin-guide/mono.rst
@@ -9,14 +9,14 @@ This will allow you to execute Mono-based .NET binaries just 
like any
 other program after you have done the following:
 
 1) You MUST FIRST install the Mono CLR support, either by downloading
-   a binary package, a source tarball or by installing from CVS. Binary
+   a binary package, a source tarball or by installing from Git. Binary
packages for several distributions can be found at:
 
-   http://go-mono.com/download.html
+   http://www.mono-project.com/download/
 
Instructions for compiling Mono can be found at:
 
-   http://www.go-mono.com/compiling.html
+   http://www.mono-project.com/docs/compiling-mono/linux/
 
Once the Mono CLR support has been installed, just check that
``/usr/bin/mono`` (which could be located elsewhere, for example
-- 
2.15.0

[PATCH] dt-bindings: chosen: Document linux,initrd-{start,end}

[Jonathan Neuschäfer]

These properties have been in use for a very long time (at least since
2005), but were never documented in chosen.txt.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/devicetree/bindings/chosen.txt | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/chosen.txt 
b/Documentation/devicetree/bindings/chosen.txt
index e3b13ea7d2ae..45e79172a646 100644
--- a/Documentation/devicetree/bindings/chosen.txt
+++ b/Documentation/devicetree/bindings/chosen.txt
@@ -120,3 +120,18 @@ e.g.
 While this property does not represent a real hardware, the address
 and the size are expressed in #address-cells and #size-cells,
 respectively, of the root node.
+
+linux,initrd-start and linux,initrd-end
+---
+
+These properties hold the physical start and end address of an initrd that's
+loaded by the bootloader. Note that linux,initrd-start is inclusive, but
+linux,initrd-end is exclusive.
+e.g.
+
+/ {
+   chosen {
+   linux,initrd-start = <0x8200>;
+   linux,initrd-end = <0x8280>;
+   };
+};
-- 
2.15.0

Re: [PATCH 2/2] powerpc: wii_defconfig: Enable GPIO-related options

[Jonathan Neuschäfer]

On Mon, Apr 30, 2018 at 03:42:47PM +0200, Jonathan Neuschäfer wrote:
> Now that there's a GPIO driver for the Wii, let's enable the following
> drivers:
> 
> - the GPIO driver itself
> - gpio-keys
> - gpio-poweroff
> - gpio-leds and a few LED triggers
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
> ---
[...]
> +CONFIG_LEDS_CLASS=y
> +CONFIG_LEDS_GPIO=y
> +CONFIG_LEDS_TRIGGER_HEARTBEAT=y
> +CONFIG_LEDS_TRIGGER_PANIC=y

Oops, this doesn't work without first enabling CONFIG_NEW_LEDS and
CONFIG_LEDS_TRIGGERS. I'll send a v2.


Jonathan Neuschäfer


signature.asc
Description: PGP signature

[PATCH v2 1/4] powerpc: wii_defconfig: Disable Ethernet driver support code

[Jonathan Neuschäfer]

The Wii doesn't have built-in Ethernet and USB Ethernet adapters are in
a different menu. Disable CONFIG_ETHERNET to save some space in support
code for Ethernet drivers.

Note that this patch doesn't disable any Ethernet drivers, because they
are not enabled by default.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
v2:
- Drop the bloat-o-meter output from the commit message
---
 arch/powerpc/configs/wii_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/configs/wii_defconfig 
b/arch/powerpc/configs/wii_defconfig
index 0b0f78823a1b..3167b9d7f3e5 100644
--- a/arch/powerpc/configs/wii_defconfig
+++ b/arch/powerpc/configs/wii_defconfig
@@ -49,6 +49,7 @@ CONFIG_BLK_DEV_RAM_COUNT=2
 CONFIG_SCSI=y
 CONFIG_BLK_DEV_SD=y
 CONFIG_NETDEVICES=y
+# CONFIG_ETHERNET is not set
 CONFIG_B43=y
 CONFIG_B43_SDIO=y
 # CONFIG_B43_PHY_LP is not set
-- 
2.17.0

[PATCH v2 3/4] powerpc: wii_defconfig: Enable Wii SDHCI driver

[Jonathan Neuschäfer]

This allows access to the SD card and the BCM4318 Wifi module.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---

Note that until some fixes in the interrupt controller drivers used on
the Wii, the SDHCI controllers will not be usable.

v2:
- Patch added to the series
---
 arch/powerpc/configs/wii_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/configs/wii_defconfig 
b/arch/powerpc/configs/wii_defconfig
index c5933a38e5ad..a674dd420f0f 100644
--- a/arch/powerpc/configs/wii_defconfig
+++ b/arch/powerpc/configs/wii_defconfig
@@ -93,6 +93,8 @@ CONFIG_HID_APPLE=m
 CONFIG_HID_WACOM=m
 CONFIG_MMC=y
 CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_PLTFM=y
+CONFIG_MMC_SDHCI_OF_HLWD=y
 CONFIG_NEW_LEDS=y
 CONFIG_LEDS_CLASS=y
 CONFIG_LEDS_GPIO=y
-- 
2.17.0

[PATCH v2 4/4] powerpc: wii_defconfig: Disable BCMA support

[Jonathan Neuschäfer]

The B43 driver only needs CONFIG_SSB to support the WLAN card found in
the Wii. Configure it accordingly, and disable BCMA bus support to save
a bit of space.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---

v2:
- Patch added to the series
---
 arch/powerpc/configs/wii_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/configs/wii_defconfig 
b/arch/powerpc/configs/wii_defconfig
index a674dd420f0f..10940533da71 100644
--- a/arch/powerpc/configs/wii_defconfig
+++ b/arch/powerpc/configs/wii_defconfig
@@ -51,6 +51,7 @@ CONFIG_BLK_DEV_SD=y
 CONFIG_NETDEVICES=y
 # CONFIG_ETHERNET is not set
 CONFIG_B43=y
+CONFIG_B43_BUSES_SSB=y
 CONFIG_B43_SDIO=y
 # CONFIG_B43_PHY_LP is not set
 CONFIG_B43_DEBUG=y
-- 
2.17.0

[PATCH v2 2/4] powerpc: wii_defconfig: Enable GPIO-related options

[Jonathan Neuschäfer]

Now that there's a GPIO driver for the Wii, let's enable the following
drivers:

- the GPIO driver itself
- gpio-keys
- gpio-poweroff
- gpio-leds and a few LED triggers

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
v2:
- Set CONFIG_NEW_LEDS=y and CONFIG_LEDS_TRIGGERS=y, without which some
  of the other options can't be set.
---
 arch/powerpc/configs/wii_defconfig | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/arch/powerpc/configs/wii_defconfig 
b/arch/powerpc/configs/wii_defconfig
index 3167b9d7f3e5..c5933a38e5ad 100644
--- a/arch/powerpc/configs/wii_defconfig
+++ b/arch/powerpc/configs/wii_defconfig
@@ -58,6 +58,7 @@ CONFIG_INPUT_FF_MEMLESS=m
 CONFIG_INPUT_JOYDEV=y
 CONFIG_INPUT_EVDEV=y
 # CONFIG_KEYBOARD_ATKBD is not set
+CONFIG_KEYBOARD_GPIO=y
 # CONFIG_MOUSE_PS2 is not set
 CONFIG_INPUT_JOYSTICK=y
 CONFIG_INPUT_MISC=y
@@ -72,6 +73,9 @@ CONFIG_I2C_CHARDEV=y
 CONFIG_I2C_GPIO=y
 CONFIG_GPIOLIB=y
 CONFIG_GPIO_SYSFS=y
+CONFIG_GPIO_HLWD=y
+CONFIG_POWER_RESET=y
+CONFIG_POWER_RESET_GPIO=y
 # CONFIG_HWMON is not set
 CONFIG_SSB_DEBUG=y
 CONFIG_FB=y
@@ -89,6 +93,12 @@ CONFIG_HID_APPLE=m
 CONFIG_HID_WACOM=m
 CONFIG_MMC=y
 CONFIG_MMC_SDHCI=y
+CONFIG_NEW_LEDS=y
+CONFIG_LEDS_CLASS=y
+CONFIG_LEDS_GPIO=y
+CONFIG_LEDS_TRIGGERS=y
+CONFIG_LEDS_TRIGGER_HEARTBEAT=y
+CONFIG_LEDS_TRIGGER_PANIC=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_GENERIC=y
 CONFIG_EXT2_FS=y
-- 
2.17.0

[PATCH 1/2] powerpc: flipper-pic: Don't match all IRQ domains

[Jonathan Neuschäfer]

On the Wii, there is a secondary IRQ controller (hlwd-pic), so
flipper-pic's match operation should not be hardcoded to return 1.
In fact, the default matching logic is sufficient, and we can completely
omit flipper_pic_match.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---

Note: This shouldn't break Linux on the GameCube, but I've only tested
on the Wii. Some confirmation that I didn't break interrupt handling on
the GC would be nice. (If someone still runs mainline on the GC.)
---
 arch/powerpc/platforms/embedded6xx/flipper-pic.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/arch/powerpc/platforms/embedded6xx/flipper-pic.c 
b/arch/powerpc/platforms/embedded6xx/flipper-pic.c
index 7206f3f573d4..db0be007fd06 100644
--- a/arch/powerpc/platforms/embedded6xx/flipper-pic.c
+++ b/arch/powerpc/platforms/embedded6xx/flipper-pic.c
@@ -108,16 +108,8 @@ static int flipper_pic_map(struct irq_domain *h, unsigned 
int virq,
return 0;
 }
 
-static int flipper_pic_match(struct irq_domain *h, struct device_node *np,
-enum irq_domain_bus_token bus_token)
-{
-   return 1;
-}
-
-
 static const struct irq_domain_ops flipper_irq_domain_ops = {
.map = flipper_pic_map,
-   .match = flipper_pic_match,
 };
 
 /*
-- 
2.17.0

[PATCH 0/2] powerpc: Wii IRQ fixes

[Jonathan Neuschäfer]

This series makes it possible to use the SD card on the Wii. The WLAN
now also works on the SDIO level, but fails to connect to a network for
some reason.

Patch 1 seems quite obvious, although I don't know why the code was
broken in this particular way.

Patch 2 might not be the right solution for the problem at hand, but it
works and should only cause problems when both processors in the system
(PPC and ARM) try to use the same interrupt, if at all.

Jonathan Neuschäfer (2):
  powerpc: flipper-pic: Don't match all IRQ domains
  powerpc: hlwd-pic: Prevent interrupts from being handled by Starlet

 arch/powerpc/platforms/embedded6xx/flipper-pic.c | 8 
 arch/powerpc/platforms/embedded6xx/hlwd-pic.c| 5 +
 2 files changed, 5 insertions(+), 8 deletions(-)

-- 
2.17.0

[PATCH 2/2] powerpc: hlwd-pic: Prevent interrupts from being handled by Starlet

[Jonathan Neuschäfer]

The interrupt controller inside the Wii's Hollywood chip is connected to
two masters, the "Broadway" PowerPC and the "Starlet" ARM926, each with
their own interrupt status and mask registers.

When booting the Wii with mini[1], interrupts from the SD card
controller (IRQ 7) are handled by the ARM, because mini provides SD
access over IPC. Linux however can't currently use or disable this IPC
service, so both sides try to handle IRQ 7 without coordination.

Let's instead make sure that all interrupts that are unmasked on the PPC
side are masked on the ARM side; this will also make sure that Linux can
properly talk to the SD card controller (and potentially other devices).

If access to a device through IPC is desired in the future, interrupts
from that device should not be handled by Linux directly.

[1]: https://github.com/lewurm/mini

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 arch/powerpc/platforms/embedded6xx/hlwd-pic.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/powerpc/platforms/embedded6xx/hlwd-pic.c 
b/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
index e3e3af73e9d8..8112b39879d6 100644
--- a/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
+++ b/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
@@ -35,6 +35,8 @@
  */
 #define HW_BROADWAY_ICR0x00
 #define HW_BROADWAY_IMR0x04
+#define HW_STARLET_ICR 0x08
+#define HW_STARLET_IMR 0x0c
 
 
 /*
@@ -74,6 +76,9 @@ static void hlwd_pic_unmask(struct irq_data *d)
void __iomem *io_base = irq_data_get_irq_chip_data(d);
 
setbits32(io_base + HW_BROADWAY_IMR, 1 << irq);
+
+   /* Make sure the ARM (aka. Starlet) doesn't handle this interrupt. */
+   clrbits32(io_base + HW_STARLET_IMR, 1 << irq);
 }
 
 
-- 
2.17.0

Re: [PATCH v2 0/4] powerpc: wii_defconfig updates

[Jonathan Neuschäfer]

I forgot to CC the right set of people/mailing lists on the cover
letter. Sorry. Here it is:

On Mon, May 07, 2018 at 04:20:15PM +0200, Jonathan Neuschäfer wrote:
> v1: https://www.spinics.net/lists/kernel/msg2790389.html
> https://www.spinics.net/lists/kernel/msg2790385.html
> 
> In the previous version of patch 2, I forgot to set CONFIG_NEW_LEDS and
> CONFIG_LEDS_TRIGGERS, so the more specific LED-related options weren't
> actually enabled, due to Kconfig dependencies. This is now fixed.
> 
> I took the opportunity of a v2 to add two more patches that I wanted to
> send anyway. The SDHCIs in the Wii are currently unusable due to bugs/
> problems in the flipper-pic/hlwd-pic drivers, but I know how to fix
> those, and will send patches.
> 
> Jonathan Neuschäfer (4):
>   powerpc: wii_defconfig: Disable Ethernet driver support code
>   powerpc: wii_defconfig: Enable GPIO-related options
>   powerpc: wii_defconfig: Enable Wii SDHCI driver
>   powerpc: wii_defconfig: Disable BCMA support
> 
>  arch/powerpc/configs/wii_defconfig | 14 ++
>  1 file changed, 14 insertions(+)
> 
> -- 
> 2.17.0
> 


signature.asc
Description: PGP signature

[PATCH] Documentation: gpio: driver: Fix a typo and some odd grammar

[Jonathan Neuschäfer]

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/driver-api/gpio/driver.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/driver-api/gpio/driver.rst 
b/Documentation/driver-api/gpio/driver.rst
index 505ee906d7d9..cbe0242842d1 100644
--- a/Documentation/driver-api/gpio/driver.rst
+++ b/Documentation/driver-api/gpio/driver.rst
@@ -44,7 +44,7 @@ common to each controller of that type:
 
  - methods to establish GPIO line direction
  - methods used to access GPIO line values
- - method to set electrical configuration to a a given GPIO line
+ - method to set electrical configuration for a given GPIO line
  - method to return the IRQ number associated to a given GPIO line
  - flag saying whether calls to its methods may sleep
  - optional line names array to identify lines
@@ -143,7 +143,7 @@ resistor will make the line tend to high level unless one 
of the transistors on
 the rail actively pulls it down.
 
 The level on the line will go as high as the VDD on the pull-up resistor, which
-may be higher than the level supported by the transistor, achieveing a
+may be higher than the level supported by the transistor, achieving a
 level-shift to the higher VDD.
 
 Integrated electronics often have an output driver stage in the form of a CMOS
@@ -382,7 +382,7 @@ Real-Time compliance for GPIO IRQ chips
 
 Any provider of irqchips needs to be carefully tailored to support Real Time
 preemption. It is desirable that all irqchips in the GPIO subsystem keep this
-in mind and does the proper testing to assure they are real time-enabled.
+in mind and do the proper testing to assure they are real time-enabled.
 So, pay attention on above " RT_FULL:" notes, please.
 The following is a checklist to follow when preparing a driver for real
 time-compliance:
-- 
2.17.0

[PATCH] genirq: Fix editing error in a comment

[Jonathan Neuschäfer]

When the comment was reflowed to a wider format, the "*" snuck in.

Fixes: ae88a23b32fa ("irq: refactor and clean up the free_irq() code flow")
Signed-off-by: Jonathan Neuschäfer 
---
 kernel/irq/manage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index daeabd791d58..591cfe901162 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1638,7 +1638,7 @@ static struct irqaction *__free_irq(struct irq_desc 
*desc, void *dev_id)
 * is so by doing an extra call to the handler 
 *
 * ( We do this after actually deregistering it, to make sure that a
-*   'real' IRQ doesn't run in * parallel with our fake. )
+*   'real' IRQ doesn't run in parallel with our fake. )
 */
if (action->flags & IRQF_SHARED) {
local_irq_save(flags);
-- 
2.17.1

[PATCH 2/2] powerpc: wii_defconfig: Enable GPIO-related options

[Jonathan Neuschäfer]

Now that there's a GPIO driver for the Wii, let's enable the following
drivers:

- the GPIO driver itself
- gpio-keys
- gpio-poweroff
- gpio-leds and a few LED triggers

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 arch/powerpc/configs/wii_defconfig | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/configs/wii_defconfig 
b/arch/powerpc/configs/wii_defconfig
index 3167b9d7f3e5..40a3b5c09765 100644
--- a/arch/powerpc/configs/wii_defconfig
+++ b/arch/powerpc/configs/wii_defconfig
@@ -58,6 +58,7 @@ CONFIG_INPUT_FF_MEMLESS=m
 CONFIG_INPUT_JOYDEV=y
 CONFIG_INPUT_EVDEV=y
 # CONFIG_KEYBOARD_ATKBD is not set
+CONFIG_KEYBOARD_GPIO=y
 # CONFIG_MOUSE_PS2 is not set
 CONFIG_INPUT_JOYSTICK=y
 CONFIG_INPUT_MISC=y
@@ -72,6 +73,9 @@ CONFIG_I2C_CHARDEV=y
 CONFIG_I2C_GPIO=y
 CONFIG_GPIOLIB=y
 CONFIG_GPIO_SYSFS=y
+CONFIG_GPIO_HLWD=y
+CONFIG_POWER_RESET=y
+CONFIG_POWER_RESET_GPIO=y
 # CONFIG_HWMON is not set
 CONFIG_SSB_DEBUG=y
 CONFIG_FB=y
@@ -89,6 +93,10 @@ CONFIG_HID_APPLE=m
 CONFIG_HID_WACOM=m
 CONFIG_MMC=y
 CONFIG_MMC_SDHCI=y
+CONFIG_LEDS_CLASS=y
+CONFIG_LEDS_GPIO=y
+CONFIG_LEDS_TRIGGER_HEARTBEAT=y
+CONFIG_LEDS_TRIGGER_PANIC=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_GENERIC=y
 CONFIG_EXT2_FS=y
-- 
2.17.0

[PATCH 1/2] powerpc: wii_defconfig: Disable Ethernet driver support code

[Jonathan Neuschäfer]

The Wii doesn't have built-in Ethernet and USB Ethernet adapters are in
a different menu. Disable CONFIG_ETHERNET to save some space in support
code for Ethernet drivers.

add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-367 (-367)
Function old new   delta
kernel_config_data 13691   13324-367
Total: Before=8341718, After=8341351, chg -0.00%

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 arch/powerpc/configs/wii_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/configs/wii_defconfig 
b/arch/powerpc/configs/wii_defconfig
index 0b0f78823a1b..3167b9d7f3e5 100644
--- a/arch/powerpc/configs/wii_defconfig
+++ b/arch/powerpc/configs/wii_defconfig
@@ -49,6 +49,7 @@ CONFIG_BLK_DEV_RAM_COUNT=2
 CONFIG_SCSI=y
 CONFIG_BLK_DEV_SD=y
 CONFIG_NETDEVICES=y
+# CONFIG_ETHERNET is not set
 CONFIG_B43=y
 CONFIG_B43_SDIO=y
 # CONFIG_B43_PHY_LP is not set
-- 
2.17.0

Re: [PATCH 3/6] gpio: Add GPIO driver for Nintendo Wii

[Jonathan Neuschäfer]

On Tue, Jan 16, 2018 at 10:42:54AM +0100, Linus Walleij wrote:
> On Mon, Jan 15, 2018 at 4:13 AM, Jonathan Neuschäfer
> <j.neuschae...@gmx.net> wrote:
> 
> > This patch is based on code developed by Albert Herranz and the GameCube
> > Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
> > available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
> > has grown quite dissimilar.
> 
> I'm impressed by this effort. As with all reverse engineering.
> 
> > This driver currently uses __raw_readl and __raw_writel to access the
> > GPIO controller's MMIO registers. I wonder if readl/writel plus explicit
> > byte-swapping would be more correct, because it could be independent of
> > the CPU's endianness. That said, this hardware only exists in two
> > big-endian machines (Wii and Wii U).
> 
> I don't know about PPC but I think you're supposed to use
> ioread32be() and iowrite32be() to do explicit BE access.

Ah, that's the name! I didn't find ioread32*/iowrite32* in the
documentation or source code.

> But when I look at it, I think you can just use the gpio-mmio library
> for this driver and cut down code cosiderably.

I'll look into it. So far it looks good (drivers/gpio/gpio-iop.c has
just 60 lines).

> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
> Can't you just save a pointer to struct device *dev in the
> state container and use dev_info(state->dev, ...) etc instead
> of this?

Makes sense. I'll try this out.

> > +#include 
> 
> This include should not be needed.

Okay.

> > +/*
> > + * Update the bit with the given bit offset in the given register to a 
> > given
> > + * value
> > + */
> > +static void hlwd_gpio_update_bit(struct gpio_chip *gc, unsigned int reg,
> > +   int offset, int value)
> > +{
> > +   struct hlwd_gpio *hlwd = gpiochip_get_data(gc);
> > +   unsigned long flags;
> > +   u32 bit = 1UL << offset;
> 
> #include 
> 
> u32 bit = BIT(offset);
> 
> > +   u32 tmp;
> > +
> > +   spin_lock_irqsave(>lock, flags);
> > +   tmp = __raw_readl(hlwd->regs + reg);
> > +   if (value)
> > +   __raw_writel(tmp | bit, hlwd->regs + reg);
> > +   else
> > +   __raw_writel(tmp & ~bit, hlwd->regs + reg);
> > +   spin_unlock_irqrestore(>lock, flags);
> > +}
> 
> This looks very much like it is reimplementing the stuff we already
> have in drivers/gpio/gpio-mmio.h.
> 
> There is even a big endian access flag for the library.
> And you get so much for free with gpio-mmio.
> 
> select GPIO_GENERIC
> in Kconfig
> 
> the helpers come in from 
> 
> Look at other drivers for inspiration:
> git grep bgpio_init
> 
> If you need IRQ support you should probably have your own file
> for this driver, but it will be just a few lines of wrapper using
> bgpio_init() and BGPIOF_BIG_ENDIAN and/or possibly
> BGPIOF_BIG_ENDIAN_BYTE_ORDER.

Yes, I plan to add IRQ support in a later patch.

> 
> See the other drivers.

Yep, gpio-mmio looks like a good option, thanks for the pointer!


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

[PATCH v2 5/6] powerpc: wii.dts: Add ngpios property

[Jonathan Neuschäfer]

The Hollywood GPIO controller supports 32 GPIOs, but on the Wii, only 24
are used.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---

v2:
- no change
---
 arch/powerpc/boot/dts/wii.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/boot/dts/wii.dts b/arch/powerpc/boot/dts/wii.dts
index 40b324b6391e..7235e375919c 100644
--- a/arch/powerpc/boot/dts/wii.dts
+++ b/arch/powerpc/boot/dts/wii.dts
@@ -176,6 +176,7 @@
compatible = "nintendo,hollywood-gpio";
reg = <0x0d8000c0 0x40>;
gpio-controller;
+   ngpios = <24>;
 
/*
 * This is commented out while a standard binding
-- 
2.15.1

[PATCH v2 6/6] powerpc: wii.dts: Add GPIO line names

[Jonathan Neuschäfer]

These are the GPIO line names on a Nintendo Wii, as documented in:
https://wiibrew.org/wiki/Hardware/Hollywood_GPIOs

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---

v2:
- no change
---
 arch/powerpc/boot/dts/wii.dts | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/boot/dts/wii.dts b/arch/powerpc/boot/dts/wii.dts
index 7235e375919c..07d5e84e98b1 100644
--- a/arch/powerpc/boot/dts/wii.dts
+++ b/arch/powerpc/boot/dts/wii.dts
@@ -178,6 +178,14 @@
gpio-controller;
ngpios = <24>;
 
+   gpio-line-names =
+   "POWER", "SHUTDOWN", "FAN", "DC_DC",
+   "DI_SPIN", "SLOT_LED", "EJECT_BTN", "SLOT_IN",
+   "SENSOR_BAR", "DO_EJECT", "EEP_CS", "EEP_CLK",
+   "EEP_MOSI", "EEP_MISO", "AVE_SCL", "AVE_SDA",
+   "DEBUG0", "DEBUG1", "DEBUG2", "DEBUG3",
+   "DEBUG4", "DEBUG5", "DEBUG6", "DEBUG7";
+
/*
 * This is commented out while a standard binding
 * for i2c over gpio is defined.
-- 
2.15.1

[PATCH v2 4/6] dt-bindings: gpio: Add binding for Wii GPIO controller

[Jonathan Neuschäfer]

The Nintendo Wii game console has a GPIO controller, which is used for
the optical disk slot LED, buttons, poweroff, etc. This patch adds a
binding for this GPIO controller.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
Reviewed-by: Rob Herring <r...@kernel.org>
---

v2:
- Drop the leading zero in the example, as suggested by Rob Herring
- Add some text to the commit message, as suggested by Linus Walleij
---
 .../bindings/gpio/nintendo,hollywood-gpio.txt  | 27 ++
 .../devicetree/bindings/powerpc/nintendo/wii.txt   |  9 +---
 2 files changed, 28 insertions(+), 8 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt

diff --git a/Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt 
b/Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt
new file mode 100644
index ..20fc72d9e61e
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt
@@ -0,0 +1,27 @@
+Nintendo Wii (Hollywood) GPIO controller
+
+Required properties:
+- compatible: "nintendo,hollywood-gpio
+- reg: Physical base address and length of the controller's registers.
+- gpio-controller: Marks the device node as a GPIO controller.
+- #gpio-cells: Should be <2>. The first cell is the pin number and the
+  second cell is used to specify optional parameters:
+   - bit 0 specifies polarity (0 for normal, 1 for inverted).
+
+Optional properties:
+- ngpios: see Documentation/devicetree/bindings/gpio/gpio.txt
+- interrupt-controller: Marks the device node as an interrupt controller.
+- #interrupt-cells: Should be two.
+- interrupts: Interrupt specifier for the controller's Broadway (PowerPC)
+  interrupt.
+- interrupt-parent: phandle of the parent interrupt controller.
+
+Example:
+
+   GPIO: gpio@d8000c0 {
+   #gpio-cells = <2>;
+   compatible = "nintendo,hollywood-gpio";
+   reg = <0x0d8000c0 0x40>;
+   gpio-controller;
+   ngpios = <24>;
+   }
diff --git a/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt 
b/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
index 36afa322b04b..a3dc4b9fa11a 100644
--- a/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
+++ b/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
@@ -152,14 +152,7 @@ Nintendo Wii device tree
 
 1.l) The General Purpose I/O (GPIO) controller node
 
-  Represents the dual access 32 GPIO controller interface.
-
-  Required properties:
-
-  - #gpio-cells : <2>
-  - compatible : should be "nintendo,hollywood-gpio"
-  - reg : should contain the IPC registers location and length
-  - gpio-controller
+  see Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt
 
 1.m) The control node
 
-- 
2.15.1

[PATCH v2 3/6] gpio: Add GPIO driver for Nintendo Wii

[Jonathan Neuschäfer]

The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
that supports a configurable number of pins (up to 32), interrupts, and
some special mechanisms to share the controller between the system's
security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
not supported.

This patch adds a basic driver for this GPIO controller. Interrupt
support will come in a later patch.

This patch is based on code developed by Albert Herranz and the GameCube
Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
has grown quite dissimilar.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
Cc: Albert Herranz <albert_herr...@yahoo.es>
Cc: Segher Boessenkool <seg...@kernel.crashing.org>
---

v2:
- Change hlwd_gpio_driver.driver.name to "gpio-hlwd" to match the
  filename (was "hlwd_gpio")
- Remove unnecessary include of linux/of_gpio.h, as suggested by Linus
  Walleij.
- Add struct device pointer to context struct to make it possible to use
  dev_info(hlwd->dev, "..."), as suggested by Linus Walleij
- Use the GPIO_GENERIC library to reduce code size, as suggested by
  Linus Walleij
- Use iowrite32be instead of __raw_writel for big-endian MMIO access, as
  suggested by Linus Walleij
- Remove commit message paragraph suggesting to diff against the
  original driver, because it's so different now
---
 drivers/gpio/Kconfig |   9 
 drivers/gpio/Makefile|   1 +
 drivers/gpio/gpio-hlwd.c | 123 +++
 3 files changed, 133 insertions(+)
 create mode 100644 drivers/gpio/gpio-hlwd.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d6a8e851ad13..47606dfe06cc 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -229,6 +229,15 @@ config GPIO_GRGPIO
  Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
  VHDL IP core library.
 
+config GPIO_HLWD
+   tristate "Nintendo Wii (Hollywood) GPIO"
+   depends on OF_GPIO
+   select GPIO_GENERIC
+   help
+ Select this to support the GPIO controller of the Nintendo Wii.
+
+ If unsure, say N.
+
 config GPIO_ICH
tristate "Intel ICH GPIO"
depends on PCI && X86
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4bc24febb889..492f62d0eb59 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_GPIO_FTGPIO010)  += gpio-ftgpio010.o
 obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
 obj-$(CONFIG_GPIO_GPIO_MM) += gpio-gpio-mm.o
 obj-$(CONFIG_GPIO_GRGPIO)  += gpio-grgpio.o
+obj-$(CONFIG_GPIO_HLWD)+= gpio-hlwd.o
 obj-$(CONFIG_HTC_EGPIO)+= gpio-htc-egpio.o
 obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
 obj-$(CONFIG_GPIO_INGENIC) += gpio-ingenic.o
diff --git a/drivers/gpio/gpio-hlwd.c b/drivers/gpio/gpio-hlwd.c
new file mode 100644
index ..cf3f05a1621c
--- /dev/null
+++ b/drivers/gpio/gpio-hlwd.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (C) 2008-2009 The GameCube Linux Team
+// Copyright (C) 2008,2009 Albert Herranz
+// Copyright (C) 2017-2018 Jonathan Neuschäfer
+//
+// Nintendo Wii (Hollywood) GPIO driver
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * Register names and offsets courtesy of WiiBrew:
+ * https://wiibrew.org/wiki/Hardware/Hollywood_GPIOs
+ *
+ * Note that for most registers, there are two versions:
+ * - HW_GPIOB_* Is always accessible by the Broadway PowerPC core, but does
+ *   always give access to all GPIO lines
+ * - HW_GPIO_* Is only accessible by the Broadway PowerPC code if the memory
+ *   firewall (AHBPROT) in the Hollywood chipset has been configured to allow
+ *   such access.
+ *
+ * The ownership of each GPIO line can be configured in the HW_GPIO_OWNER
+ * register: A one bit configures the line for access via the HW_GPIOB_*
+ * registers, a zero bit indicates access via HW_GPIO_*. This driver uses
+ * HW_GPIOB_*.
+ */
+#define HW_GPIOB_OUT   0x00
+#define HW_GPIOB_DIR   0x04
+#define HW_GPIOB_IN0x08
+#define HW_GPIOB_INTLVL0x0c
+#define HW_GPIOB_INTFLAG   0x10
+#define HW_GPIOB_INTMASK   0x14
+#define HW_GPIOB_INMIR 0x18
+#define HW_GPIO_ENABLE 0x1c
+#define HW_GPIO_OUT0x20
+#define HW_GPIO_DIR0x24
+#define HW_GPIO_IN 0x28
+#define HW_GPIO_INTLVL 0x2c
+#define HW_GPIO_INTFLAG0x30
+#define HW_GPIO_INTMASK0x34
+#define HW_GPIO_INMIR  0x38
+#define HW_GPIO_OWNER  0x3c
+
+
+struct hlwd_gpio {
+   struct gpio_chip gpioc;
+   void __iomem *regs;
+   struct device *dev;
+};
+
+static int hlwd_gpio_probe(struct platform_device *pdev)
+{
+   struct hlwd_gpio *hlwd;
+   struct re

[PATCH v2 2/6] powerpc: wii: Explicitly configure GPIO owner for poweroff pin

[Jonathan Neuschäfer]

The Hollywood chipset's GPIO controller has two sets of registers: One
for access by the PowerPC CPU, and one for access by the ARM coprocessor
(but both are accessible from the PPC because the memory firewall
(AHBPROT) is usually disabled when booting Linux, today).

The wii_power_off function currently assumes that the poweroff GPIO pin
is configured for use via the ARM side, but the upcoming GPIO driver
configures all pins for use via the PPC side, breaking poweroff.

Configure the owner register explicitly in wii_power_off to make
wii_power_off work with and without the new GPIO driver.

I think the Wii can be switched to the generic gpio-poweroff driver,
after the GPIO driver is merged.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---

v2:
- no change
---
 arch/powerpc/platforms/embedded6xx/wii.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/powerpc/platforms/embedded6xx/wii.c 
b/arch/powerpc/platforms/embedded6xx/wii.c
index 79a1fe54ebc9..6e6db1e16d71 100644
--- a/arch/powerpc/platforms/embedded6xx/wii.c
+++ b/arch/powerpc/platforms/embedded6xx/wii.c
@@ -45,6 +45,7 @@
 #define HW_GPIO_BASE(idx)  (idx * 0x20)
 #define HW_GPIO_OUT(idx)   (HW_GPIO_BASE(idx) + 0)
 #define HW_GPIO_DIR(idx)   (HW_GPIO_BASE(idx) + 4)
+#define HW_GPIO_OWNER  (HW_GPIO_BASE(1) + 0x1c)
 
 #define HW_GPIO_SHUTDOWN   (1<<1)
 #define HW_GPIO_SLOT_LED   (1<<5)
@@ -177,6 +178,12 @@ static void wii_power_off(void)
local_irq_disable();
 
if (hw_gpio) {
+   /*
+* set the owner of the shutdown pin to ARM, because it is
+* accessed through the registers for the ARM, below
+*/
+   clrbits32(hw_gpio + HW_GPIO_OWNER, HW_GPIO_SHUTDOWN);
+
/* make sure that the poweroff GPIO is configured as output */
setbits32(hw_gpio + HW_GPIO_DIR(1), HW_GPIO_SHUTDOWN);
 
-- 
2.15.1

[PATCH v2 0/6] Nintendo Wii GPIO driver

[Jonathan Neuschäfer]

This series adds a driver for the GPIO controller used in the Nintendo
Wii game console.

The driver itself, and the related devicetree work should be pretty
uncontroversial, but due to the system architecture of the Wii, I also
had to extend an old resource allocation hack to kernel/resource.c: On
the Wii, there are two separate RAM ranges, with MMIO right in the
middle, but AFAIK, Linux on PPC32 doesn't support discontiguous memory
properly. So the hack is to allocate one big RAM range with a hole
(marked as reserved memory) for MMIO in the middle.

Because this series touches different subsystems (GPIO, DT, core
resource management), I guess it should be picked up patch-by-patch by
the different maintainers.

The main difference between v2 and the previous version is that I
rewrote the driver on top of the GPIO_GENERIC library, saving 60 lines
of code.

Jonathan Neuschäfer (6):
  resource: Extend the PPC32 reserved memory hack
  powerpc: wii: Explicitly configure GPIO owner for poweroff pin
  gpio: Add GPIO driver for Nintendo Wii
  dt-bindings: gpio: Add binding for Wii GPIO controller
  powerpc: wii.dts: Add ngpios property
  powerpc: wii.dts: Add GPIO line names

 .../bindings/gpio/nintendo,hollywood-gpio.txt  |  27 +
 .../devicetree/bindings/powerpc/nintendo/wii.txt   |   9 +-
 arch/powerpc/boot/dts/wii.dts  |   9 ++
 arch/powerpc/platforms/embedded6xx/wii.c   |   7 ++
 drivers/gpio/Kconfig   |   9 ++
 drivers/gpio/Makefile  |   1 +
 drivers/gpio/gpio-hlwd.c   | 123 +
 kernel/resource.c  |  21 +++-
 8 files changed, 197 insertions(+), 9 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt
 create mode 100644 drivers/gpio/gpio-hlwd.c

-- 
2.15.1

[PATCH v2 1/6] resource: Extend the PPC32 reserved memory hack

[Jonathan Neuschäfer]

On the Nintendo Wii, there are two ranges of physical memory, and MMIO
in between, but Linux on ppc32 doesn't support discontiguous memory.
Therefore a hack was introduced in commit c5df7f775148 ("powerpc: allow
ioremap within reserved memory regions") and commit de32400dd26e ("wii:
use both mem1 and mem2 as ram"):

 - Treat the area from the start of the first memory area (MEM1) to the
   end of the second (MEM2) as one big memory area, but mark the part
   that doesn't belong to MEM1 or MEM2 as reserved.
 - Only on the Wii, allow ioremap to be used on reserved memory.

This hack, however, doesn't account for the "resource"-based API in
kernel/resource.c, because __request_region performs its own checks.

Extend the hack to kernel/resource.c, to allow more drivers to allocate
their MMIO regions on the Wii.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
Cc: Albert Herranz <albert_herr...@yahoo.es>
---

v2:
- CC Albert Herranz, who introduced this hack in 2009.
---
 kernel/resource.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 54ba6de3757c..bb3d329329da 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1134,6 +1134,24 @@ resource_size_t resource_alignment(struct resource *res)
 
 static DECLARE_WAIT_QUEUE_HEAD(muxed_resource_wait);
 
+/*
+ * On some ppc32 platforms (Nintendo Wii), reserved memory is used to work
+ * around the fact that Linux doesn't support discontiguous memory (all memory
+ * is treated as one large area with holes punched in it), and reserved memory
+ * is allowed to be allocated.
+ */
+#ifdef CONFIG_PPC32
+static bool conflict_ignored(struct resource *conflict)
+{
+   extern int __allow_ioremap_reserved;
+
+   return __allow_ioremap_reserved &&
+   (conflict->flags & IORESOURCE_SYSRAM);
+}
+#else
+static bool conflict_ignored(struct resource *conflict) { return false; }
+#endif
+
 /**
  * __request_region - create a new busy resource region
  * @parent: parent resource descriptor
@@ -1166,8 +1184,9 @@ struct resource * __request_region(struct resource 
*parent,
res->desc = parent->desc;
 
conflict = __request_resource(parent, res);
-   if (!conflict)
+   if (!conflict || conflict_ignored(conflict))
break;
+
if (conflict != parent) {
if (!(conflict->flags & IORESOURCE_BUSY)) {
parent = conflict;
-- 
2.15.1

[PATCH 0/6] Nintendo Wii GPIO driver

[Jonathan Neuschäfer]

This series adds a driver for the GPIO controller used in the Nintendo
Wii game console.

The driver itself, and the related devicetree work should be pretty
uncontroversial, but due to the system architecture of the Wii, I also
had to extend an old resource allocation hack to kernel/resource.c: On
the Wii, there are two separate RAM ranges, with MMIO right in the
middle, but AFAIK, Linux on PPC32 doesn't support discontiguous memory
properly. So the hack is to allocate one big RAM range with a hole
(marked as reserved memory) for MMIO in the middle.

Because this series touches different subsystems (GPIO, DT, core
resource management), I guess it should be picked up patch-by-patch by
the different maintainers.

Jonathan Neuschäfer (6):
  resource: Extend the PPC32 reserved memory hack
  powerpc: wii: Explicitly configure GPIO owner for poweroff pin
  gpio: Add GPIO driver for Nintendo Wii
  dt-bindings: gpio: Add binding for Wii GPIO controller
  powerpc: wii.dts: Add ngpios property
  powerpc: wii.dts: Add GPIO line names

 .../bindings/gpio/nintendo,hollywood-gpio.txt  |  27 +++
 .../devicetree/bindings/powerpc/nintendo/wii.txt   |   9 +-
 arch/powerpc/boot/dts/wii.dts  |   9 +
 arch/powerpc/platforms/embedded6xx/wii.c   |   7 +
 drivers/gpio/Kconfig   |   8 +
 drivers/gpio/Makefile  |   1 +
 drivers/gpio/gpio-hlwd.c   | 183 +
 kernel/resource.c  |  21 ++-
 8 files changed, 256 insertions(+), 9 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt
 create mode 100644 drivers/gpio/gpio-hlwd.c

-- 
2.15.1

[PATCH 6/6] powerpc: wii.dts: Add GPIO line names

[Jonathan Neuschäfer]

These are the GPIO line names on a Nintendo Wii, as documented in:
https://wiibrew.org/wiki/Hardware/Hollywood_GPIOs

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 arch/powerpc/boot/dts/wii.dts | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/boot/dts/wii.dts b/arch/powerpc/boot/dts/wii.dts
index 7235e375919c..07d5e84e98b1 100644
--- a/arch/powerpc/boot/dts/wii.dts
+++ b/arch/powerpc/boot/dts/wii.dts
@@ -178,6 +178,14 @@
gpio-controller;
ngpios = <24>;
 
+   gpio-line-names =
+   "POWER", "SHUTDOWN", "FAN", "DC_DC",
+   "DI_SPIN", "SLOT_LED", "EJECT_BTN", "SLOT_IN",
+   "SENSOR_BAR", "DO_EJECT", "EEP_CS", "EEP_CLK",
+   "EEP_MOSI", "EEP_MISO", "AVE_SCL", "AVE_SDA",
+   "DEBUG0", "DEBUG1", "DEBUG2", "DEBUG3",
+   "DEBUG4", "DEBUG5", "DEBUG6", "DEBUG7";
+
/*
 * This is commented out while a standard binding
 * for i2c over gpio is defined.
-- 
2.15.1

[PATCH 2/6] powerpc: wii: Explicitly configure GPIO owner for poweroff pin

[Jonathan Neuschäfer]

The Hollywood chipset's GPIO controller has two sets of registers: One
for access by the PowerPC CPU, and one for access by the ARM coprocessor
(but both are accessible from the PPC because the memory firewall
(AHBPROT) is usually disabled when booting Linux, today).

The wii_power_off function currently assumes that the poweroff GPIO pin
is configured for use via the ARM side, but the upcoming GPIO driver
configures all pins for use via the PPC side, breaking poweroff.

Configure the owner register explicitly in wii_power_off to make
wii_power_off work with and without the new GPIO driver.

I think the Wii can be switched to the generic gpio-poweroff driver,
after the GPIO driver is merged.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 arch/powerpc/platforms/embedded6xx/wii.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/powerpc/platforms/embedded6xx/wii.c 
b/arch/powerpc/platforms/embedded6xx/wii.c
index 79a1fe54ebc9..6e6db1e16d71 100644
--- a/arch/powerpc/platforms/embedded6xx/wii.c
+++ b/arch/powerpc/platforms/embedded6xx/wii.c
@@ -45,6 +45,7 @@
 #define HW_GPIO_BASE(idx)  (idx * 0x20)
 #define HW_GPIO_OUT(idx)   (HW_GPIO_BASE(idx) + 0)
 #define HW_GPIO_DIR(idx)   (HW_GPIO_BASE(idx) + 4)
+#define HW_GPIO_OWNER  (HW_GPIO_BASE(1) + 0x1c)
 
 #define HW_GPIO_SHUTDOWN   (1<<1)
 #define HW_GPIO_SLOT_LED   (1<<5)
@@ -177,6 +178,12 @@ static void wii_power_off(void)
local_irq_disable();
 
if (hw_gpio) {
+   /*
+* set the owner of the shutdown pin to ARM, because it is
+* accessed through the registers for the ARM, below
+*/
+   clrbits32(hw_gpio + HW_GPIO_OWNER, HW_GPIO_SHUTDOWN);
+
/* make sure that the poweroff GPIO is configured as output */
setbits32(hw_gpio + HW_GPIO_DIR(1), HW_GPIO_SHUTDOWN);
 
-- 
2.15.1

[PATCH 3/6] gpio: Add GPIO driver for Nintendo Wii

[Jonathan Neuschäfer]

The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
that supports a configurable number of pins (up to 32), interrupts, and
some special mechanisms to share the controller between the system's
security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
not supported.

This patch adds a basic driver for this GPIO controller. Interrupt
support will come in a later patch.

This patch is based on code developed by Albert Herranz and the GameCube
Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
has grown quite dissimilar.

To compare this version of the driver against the original code:
$ git fetch https://github.com/DeltaResero/GC-Wii-Linux-Kernels
$ git co FETCH_HEAD -- arch/powerpc/platforms/embedded6xx/hlwd-gpio.c
$ diff -u arch/powerpc/platforms/embedded6xx/hlwd-gpio.c \
  drivers/gpio/gpio-hlwd.c

Cc: Albert Herranz <albert_herr...@yahoo.es>
Cc: Segher Boessenkool <seg...@kernel.crashing.org>
Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>

---
This driver currently uses __raw_readl and __raw_writel to access the
GPIO controller's MMIO registers. I wonder if readl/writel plus explicit
byte-swapping would be more correct, because it could be independent of
the CPU's endianness. That said, this hardware only exists in two
big-endian machines (Wii and Wii U).
---
 drivers/gpio/Kconfig |   8 +++
 drivers/gpio/Makefile|   1 +
 drivers/gpio/gpio-hlwd.c | 183 +++
 3 files changed, 192 insertions(+)
 create mode 100644 drivers/gpio/gpio-hlwd.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d6a8e851ad13..4f85c2053f7d 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -229,6 +229,14 @@ config GPIO_GRGPIO
  Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
  VHDL IP core library.
 
+config GPIO_HLWD
+   tristate "Nintendo Wii (Hollywood) GPIO"
+   depends on OF_GPIO
+   help
+ Select this to support the GPIO controller of the Nintendo Wii.
+
+ If unsure, say N.
+
 config GPIO_ICH
tristate "Intel ICH GPIO"
depends on PCI && X86
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4bc24febb889..492f62d0eb59 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_GPIO_FTGPIO010)  += gpio-ftgpio010.o
 obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
 obj-$(CONFIG_GPIO_GPIO_MM) += gpio-gpio-mm.o
 obj-$(CONFIG_GPIO_GRGPIO)  += gpio-grgpio.o
+obj-$(CONFIG_GPIO_HLWD)+= gpio-hlwd.o
 obj-$(CONFIG_HTC_EGPIO)+= gpio-htc-egpio.o
 obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
 obj-$(CONFIG_GPIO_INGENIC) += gpio-ingenic.o
diff --git a/drivers/gpio/gpio-hlwd.c b/drivers/gpio/gpio-hlwd.c
new file mode 100644
index ..0f8942ea6ed6
--- /dev/null
+++ b/drivers/gpio/gpio-hlwd.c
@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (C) 2008-2009 The GameCube Linux Team
+// Copyright (C) 2008,2009 Albert Herranz
+// Copyright (C) 2017-2018 Jonathan Neuschäfer
+//
+// Nintendo Wii (Hollywood) GPIO driver
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * Register names and offsets courtesy of WiiBrew:
+ * https://wiibrew.org/wiki/Hardware/Hollywood_GPIOs
+ *
+ * Note that for most registers, there are two versions:
+ * - HW_GPIOB_* Is always accessible by the Broadway PowerPC core, but does
+ *   always give access to all GPIO lines
+ * - HW_GPIO_* Is only accessible by the Broadway PowerPC code if the memory
+ *   firewall (AHBPROT) in the Hollywood chipset has been configured to allow
+ *   such access.
+ *
+ * The ownership of each GPIO line can be configured in the HW_GPIO_OWNER
+ * register: A one bit configures the line for access via the HW_GPIOB_*
+ * registers, a zero bit indicates access via HW_GPIO_*. This driver uses
+ * HW_GPIOB_*.
+ */
+#define HW_GPIOB_OUT   0x00
+#define HW_GPIOB_DIR   0x04
+#define HW_GPIOB_IN0x08
+#define HW_GPIOB_INTLVL0x0c
+#define HW_GPIOB_INTFLAG   0x10
+#define HW_GPIOB_INTMASK   0x14
+#define HW_GPIOB_INMIR 0x18
+#define HW_GPIO_ENABLE 0x1c
+#define HW_GPIO_OUT0x20
+#define HW_GPIO_DIR0x24
+#define HW_GPIO_IN 0x28
+#define HW_GPIO_INTLVL 0x2c
+#define HW_GPIO_INTFLAG0x30
+#define HW_GPIO_INTMASK0x34
+#define HW_GPIO_INMIR  0x38
+#define HW_GPIO_OWNER  0x3c
+
+
+struct hlwd_gpio {
+   struct gpio_chip gpioc;
+   void __iomem *regs;
+   spinlock_t lock;
+};
+
+/*
+ * Update the bit with the given bit offset in the given register to a given
+ * value
+ */
+s

[PATCH 5/6] powerpc: wii.dts: Add ngpios property

[Jonathan Neuschäfer]

The Hollywood GPIO controller supports 32 GPIOs, but on the Wii, only 24
are used.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 arch/powerpc/boot/dts/wii.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/boot/dts/wii.dts b/arch/powerpc/boot/dts/wii.dts
index 40b324b6391e..7235e375919c 100644
--- a/arch/powerpc/boot/dts/wii.dts
+++ b/arch/powerpc/boot/dts/wii.dts
@@ -176,6 +176,7 @@
compatible = "nintendo,hollywood-gpio";
reg = <0x0d8000c0 0x40>;
gpio-controller;
+   ngpios = <24>;
 
/*
 * This is commented out while a standard binding
-- 
2.15.1

[PATCH 1/6] resource: Extend the PPC32 reserved memory hack

[Jonathan Neuschäfer]

On the Nintendo Wii, there are two ranges of physical memory, and MMIO
in between, but Linux on ppc32 doesn't support discontiguous memory.
Therefore a hack was introduced in commit c5df7f775148 ("powerpc: allow
ioremap within reserved memory regions") and commit de32400dd26e ("wii:
use both mem1 and mem2 as ram"):

 - Treat the area from the start of the first memory area (MEM1) to the
   end of the second (MEM2) as one big memory area, but mark the part
   that doesn't belong to MEM1 or MEM2 as reserved.
 - Only on the Wii, allow ioremap to be used on reserved memory.

This hack, however, doesn't account for the "resource"-based API in
kernel/resource.c, because __request_region performs its own checks.

Extend the hack to kernel/resource.c, to allow more drivers to allocate
their MMIO regions on the Wii.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 kernel/resource.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 54ba6de3757c..bb3d329329da 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1134,6 +1134,24 @@ resource_size_t resource_alignment(struct resource *res)
 
 static DECLARE_WAIT_QUEUE_HEAD(muxed_resource_wait);
 
+/*
+ * On some ppc32 platforms (Nintendo Wii), reserved memory is used to work
+ * around the fact that Linux doesn't support discontiguous memory (all memory
+ * is treated as one large area with holes punched in it), and reserved memory
+ * is allowed to be allocated.
+ */
+#ifdef CONFIG_PPC32
+static bool conflict_ignored(struct resource *conflict)
+{
+   extern int __allow_ioremap_reserved;
+
+   return __allow_ioremap_reserved &&
+   (conflict->flags & IORESOURCE_SYSRAM);
+}
+#else
+static bool conflict_ignored(struct resource *conflict) { return false; }
+#endif
+
 /**
  * __request_region - create a new busy resource region
  * @parent: parent resource descriptor
@@ -1166,8 +1184,9 @@ struct resource * __request_region(struct resource 
*parent,
res->desc = parent->desc;
 
conflict = __request_resource(parent, res);
-   if (!conflict)
+   if (!conflict || conflict_ignored(conflict))
break;
+
if (conflict != parent) {
if (!(conflict->flags & IORESOURCE_BUSY)) {
parent = conflict;
-- 
2.15.1

[PATCH 4/6] dt-bindings: gpio: Add binding for Wii GPIO controller

[Jonathan Neuschäfer]

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 .../bindings/gpio/nintendo,hollywood-gpio.txt  | 27 ++
 .../devicetree/bindings/powerpc/nintendo/wii.txt   |  9 +---
 2 files changed, 28 insertions(+), 8 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt

diff --git a/Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt 
b/Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt
new file mode 100644
index ..a97ce6b5b724
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt
@@ -0,0 +1,27 @@
+Nintendo Wii (Hollywood) GPIO controller
+
+Required properties:
+- compatible: "nintendo,hollywood-gpio
+- reg: Physical base address and length of the controller's registers.
+- gpio-controller: Marks the device node as a GPIO controller.
+- #gpio-cells: Should be <2>. The first cell is the pin number and the
+  second cell is used to specify optional parameters:
+   - bit 0 specifies polarity (0 for normal, 1 for inverted).
+
+Optional properties:
+- ngpios: see Documentation/devicetree/bindings/gpio/gpio.txt
+- interrupt-controller: Marks the device node as an interrupt controller.
+- #interrupt-cells: Should be two.
+- interrupts: Interrupt specifier for the controller's Broadway (PowerPC)
+  interrupt.
+- interrupt-parent: phandle of the parent interrupt controller.
+
+Example:
+
+   GPIO: gpio@0d8000c0 {
+   #gpio-cells = <2>;
+   compatible = "nintendo,hollywood-gpio";
+   reg = <0x0d8000c0 0x40>;
+   gpio-controller;
+   ngpios = <24>;
+   }
diff --git a/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt 
b/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
index 36afa322b04b..a3dc4b9fa11a 100644
--- a/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
+++ b/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
@@ -152,14 +152,7 @@ Nintendo Wii device tree
 
 1.l) The General Purpose I/O (GPIO) controller node
 
-  Represents the dual access 32 GPIO controller interface.
-
-  Required properties:
-
-  - #gpio-cells : <2>
-  - compatible : should be "nintendo,hollywood-gpio"
-  - reg : should contain the IPC registers location and length
-  - gpio-controller
+  see Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt
 
 1.m) The control node
 
-- 
2.15.1

Re: [PATCH v2 1/6] resource: Extend the PPC32 reserved memory hack

[Jonathan Neuschäfer]

On Wed, Jan 24, 2018 at 12:23:05PM +1100, Michael Ellerman wrote:
> Jonathan Neuschäfer <j.neuschae...@gmx.net> writes:
[...]
> > Do you have any pointer on how to implement discontiguous memory
> > support? CONFIG_ARCH_SPARSEMEM_ENABLE seems relevant.
> 
> I'm not really sure what the key impediment to it working is.
> 
> You don't need to go all the way to SPARSEMEM, there is DISCONTIGMEM
> which IIUI is quite a bit simpler.
> 
> I'd actually be interested to know what happens (ie. breaks) if you just
> add the two memblocks and leave the hole in between. Is it the generic
> code that breaks or is it something in the powerpc code? If it's the
> later maybe we can do a small fix/hack to work around that.

Ok, I did some experimentation.

First, I made wii_memory_fixups return early, before actually doing
anything[1].

[0.00] __ioremap(): phys addr 0xc003000 is RAM lr flipper_pic_init
[0.00] flipper-pic: controller at 0x0c003000 mapped to 0x  (null)
[0.00] Unable to handle kernel paging request for data at address 
0x0004

* __ioremap_caller detects overlap with RAM like this: p < 
virt_to_phys(high_memory)
* flipper_pic_init gets NULL from ioremap, but doesn't check for NULL


Then I hacked up __ioremap_caller to use memblock_is_map_memory[2],
because it considers memblocks correctly. The result was that the system
boots further, but then enters the sleep mode where the power LED shines
yellow. In this mode the ARM runs but the PPC doesn't. The same thing
would happen if GPIO 3 ("DC_DC"[3]) was pulled low. These are the last few
lines:

[0.770324] io scheduler mq-deadline registered
[0.772472] io scheduler kyber registered

I don't know what exactly is triggering this effect.


Thanks for your help,
Jonathan Neuschäfer


[1]: diff --git a/arch/powerpc/platforms/embedded6xx/wii.c 
b/arch/powerpc/platforms/embedded6xx/wii.c
index 6e6db1e16d71..cddd5606a63d 100644
--- a/arch/powerpc/platforms/embedded6xx/wii.c
+++ b/arch/powerpc/platforms/embedded6xx/wii.c
@@ -81,6 +81,9 @@ void __init wii_memory_fixups(void)
BUG_ON(memblock.memory.cnt != 2);
BUG_ON(!page_aligned(p[0].base) || !page_aligned(p[1].base));
 
+   /* don't fix the memory map */
+   return;
+
/* trim unaligned tail */
memblock_remove(ALIGN(p[1].base + p[1].size, PAGE_SIZE),
(phys_addr_t)ULLONG_MAX);
[2]: diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index f6c7f54c0515..bff581003c50 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -154,8 +154,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, 
unsigned long flags,
 * Don't allow anybody to remap normal RAM that we're using.
 * mem_init() sets high_memory so only do the check after that.
 */
-   if (slab_is_available() && (p < virt_to_phys(high_memory)) &&
-   !(__allow_ioremap_reserved && memblock_is_region_reserved(p, 
size))) {
+   if (slab_is_available() && memblock_is_map_memory(p)) {
printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
   (unsigned long long)p, __builtin_return_address(0));
return NULL;
[3]: http://wiibrew.org/wiki/Hardware/Hollywood_GPIOs


signature.asc
Description: PGP signature

Re: [PATCH 06/20] riscv: Remove ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE select

[Jonathan Neuschäfer]

On Mon, Feb 05, 2018 at 02:21:18AM +0100, Ulf Magnusson wrote:
> The ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE symbol was removed in
> commit 51a021244b9d ("atomic64: no need for
> CONFIG_ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE").
> 
> Remove the ARCH_HAS_ATOMIC64_DEC_IS_POSITIVE select from RISCV.
> 
> Discovered with the
> https://github.com/ulfalizer/Kconfiglib/blob/master/examples/list_undefined.py
> script.
> 
> Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
> ---
>  arch/riscv/Kconfig | 1 -
>  1 file changed, 1 deletion(-)

Looks good.

Reviewed-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>

> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index b6722c246d9c..ff69c77b9e78 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -8,7 +8,6 @@ config RISCV
>   select OF
>   select OF_EARLY_FLATTREE
>   select OF_IRQ
> - select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>   select ARCH_WANT_FRAME_POINTERS
>   select CLONE_BACKWARDS
>   select COMMON_CLK


signature.asc
Description: PGP signature

Re: [PATCH v2 3/6] gpio: Add GPIO driver for Nintendo Wii

[Jonathan Neuschäfer]

Hi,

On Sun, Jan 28, 2018 at 07:31:58PM +0200, Andy Shevchenko wrote:
> On Mon, Jan 22, 2018 at 7:04 AM, Jonathan Neuschäfer
> <j.neuschae...@gmx.net> wrote:
> 
> Style issues below.
> 
> > +#define HW_GPIO_OWNER  0x3c
> > +
> > +
> > +struct hlwd_gpio {
> 
> No need extra empty line in between.

Ok.

> > +   struct gpio_chip gpioc;
> > +   void __iomem *regs;
> > +   struct device *dev;
> > +};
> > +
> > +static int hlwd_gpio_probe(struct platform_device *pdev)
> > +{
> > +   struct hlwd_gpio *hlwd;
> > +   struct resource *regs_resource;
> > +   u32 ngpios;
> > +   int res;
> > +
> > +   hlwd = devm_kzalloc(>dev, sizeof(*hlwd), GFP_KERNEL);
> > +   if (!hlwd)
> > +   return -ENOMEM;
> > +
> 
> > +   /* Save the struct device pointer so dev_info, etc. can be used. */
> 
> Useless.

Ok

> > +   hlwd->dev = >dev;
> > +
> 
> > +   regs_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> 
> > +   if (IS_ERR(regs_resource))
> > +   return PTR_ERR(regs_resource);
> > +
> 
> This is redundant. Below does it for ya.

devm_ioremap_resource does not check if the resource argument is a
negative error (which is what I'm trying to catch in the above code).
But it seems that platform_get_resource can't return a negative error,
so you're right. Thanks.

> 
> > +   hlwd->regs = devm_ioremap_resource(>dev, regs_resource);
> > +   if (IS_ERR(hlwd->regs))
> > +   return PTR_ERR(hlwd->regs);
> 
> 
> > +   res = bgpio_init(>gpioc, >dev, 4,
> > +   hlwd->regs + HW_GPIOB_IN, hlwd->regs + HW_GPIOB_OUT,
> > +   NULL, hlwd->regs + HW_GPIOB_DIR, NULL,
> > +   BGPIOF_BIG_ENDIAN_BYTE_ORDER);
> 
> > +
> 
> Remove this extra line.

Ok.

> 
> > +   if (res < 0) {
> > +   dev_warn(hlwd->dev, "bgpio_init failed: %d\n", res);
> > +   return res;
> > +   }
> 
> > +   if (of_property_read_u32(pdev->dev.of_node, "ngpios", ))
> > +   ngpios = 32;
> 
> A nit: I would rather go with
> res = of_property_read(...);
> if (res)
>   ngpios = 32;

Ok, I have no strong opinion on this, so I'll do what you suggest.


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

[PATCH] powerpc: wii: Probe the whole devicetree

[Jonathan Neuschäfer]

Previously, wii_device_probe would only initialize devices under the
/hollywood node. After this patch, platform devices placed outside of
/hollywood will also be initialized.

The intended usecase for this are devices located outside of the
Hollywood chip, such as GPIO LEDs and GPIO buttons.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 arch/powerpc/platforms/embedded6xx/wii.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/embedded6xx/wii.c 
b/arch/powerpc/platforms/embedded6xx/wii.c
index f598d5fe1d72..4de26c3c4f6d 100644
--- a/arch/powerpc/platforms/embedded6xx/wii.c
+++ b/arch/powerpc/platforms/embedded6xx/wii.c
@@ -248,7 +248,7 @@ static int __init wii_device_probe(void)
if (!machine_is(wii))
return 0;
 
-   of_platform_bus_probe(NULL, wii_of_bus, NULL);
+   of_platform_populate(NULL, wii_of_bus, NULL, NULL);
return 0;
 }
 device_initcall(wii_device_probe);
-- 
2.15.1

Re: [PATCH v3 2/4] gpio: Add GPIO driver for Nintendo Wii

[Jonathan Neuschäfer]

On Fri, Feb 09, 2018 at 05:30:55PM +0200, Andy Shevchenko wrote:
> On Fri, Feb 9, 2018 at 2:07 PM, Jonathan Neuschäfer
> <j.neuschae...@gmx.net> wrote:
> > The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
> > that supports a configurable number of pins (up to 32), interrupts, and
> > some special mechanisms to share the controller between the system's
> > security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
> > not supported.
> >
> > This patch adds a basic driver for this GPIO controller. Interrupt
> > support will come in a later patch.
> >
> > This patch is based on code developed by Albert Herranz and the GameCube
> > Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
> > available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
> > has grown quite dissimilar.
> >
> 
> Fine to me, though one comment below.
> In any case,
> 
> Reviewed-by: Andy Shevchenko <andy.shevche...@gmail.com>

Thank you.


[...]
> > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> > index d6a8e851ad13..47606dfe06cc 100644
> > --- a/drivers/gpio/Kconfig
> > +++ b/drivers/gpio/Kconfig
> > @@ -229,6 +229,15 @@ config GPIO_GRGPIO
> >   Select this to support Aeroflex Gaisler GRGPIO cores from the 
> > GRLIB
> >   VHDL IP core library.
> >
> > +config GPIO_HLWD
> > +   tristate "Nintendo Wii (Hollywood) GPIO"
> 
> > +   depends on OF_GPIO
> 
> You may get rid of it if...

[ Even if this driver isn't switched to the unified device property API,
  I think "depends on OF" would be enough here, because it doesn't use
  the code that's guarded by CONFIG_OF_GPIO (gpiolib-of.c), but this
  applies to other drivers (e.g. gpio-aspeed, gpio-bcm-kona) as well, so
  this would ideally be a bigger cleanup patch. ]


> > +   res = of_property_read_u32(pdev->dev.of_node, "ngpios", );
> 
> ...if you switch to unified device property API.

I don't think this change is worth making, unless/until the of_property
API is deprecated. I'm rather sure this GPIO controller won't appear in
an ACPI-based system.


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [PATCH v3 2/4] gpio: Add GPIO driver for Nintendo Wii

[Jonathan Neuschäfer]

On Fri, Feb 09, 2018 at 01:07:29PM +0100, Jonathan Neuschäfer wrote:
> The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
> that supports a configurable number of pins (up to 32), interrupts, and
> some special mechanisms to share the controller between the system's
> security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
> not supported.
> 
> This patch adds a basic driver for this GPIO controller. Interrupt
> support will come in a later patch.
> 
> This patch is based on code developed by Albert Herranz and the GameCube
> Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
> available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
> has grown quite dissimilar.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
> Cc: Albert Herranz <albert_herr...@yahoo.es>
> Cc: Segher Boessenkool <seg...@kernel.crashing.org>
> <---

Ooops, I just noticed that I broke the separator here. This should be a
normal --- line, obviously.


Jonathan Neuschäfer


signature.asc
Description: PGP signature

[PATCH] Documentation/process/howto: Remove outdated info about bugzilla mailing lists

[Jonathan Neuschäfer]

The mailing list archives[1,2] show no activity since September 2011.

[1]: https://lists.linuxfoundation.org/pipermail/bugme-new/
[2]: https://lists.linuxfoundation.org/pipermail/bugme-janitors/

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/process/howto.rst | 8 
 1 file changed, 8 deletions(-)

diff --git a/Documentation/process/howto.rst b/Documentation/process/howto.rst
index c6875b1db56f..fbd236e037c2 100644
--- a/Documentation/process/howto.rst
+++ b/Documentation/process/howto.rst
@@ -381,14 +381,6 @@ bugs is one of the best ways to get merits among other 
developers, because
 not many people like wasting time fixing other people's bugs.
 
 To work in the already reported bug reports, go to https://bugzilla.kernel.org.
-If you want to be advised of the future bug reports, you can subscribe to the
-bugme-new mailing list (only new bug reports are mailed here) or to the
-bugme-janitor mailing list (every change in the bugzilla is mailed here)
-
-   https://lists.linux-foundation.org/mailman/listinfo/bugme-new
-
-   https://lists.linux-foundation.org/mailman/listinfo/bugme-janitors
-
 
 
 Mailing lists
-- 
2.15.1

[PATCH] admin-guide: Fix list formatting in tained-kernels.html

[Jonathan Neuschäfer]

Without this patch, the points 1-9 in the list are rendered as an HTML
blockquote containing a list, causing them to be indented further than
the rest of the list.

While at it, also fix the quotation marks around G and P.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---
 Documentation/admin-guide/tainted-kernels.rst | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/admin-guide/tainted-kernels.rst 
b/Documentation/admin-guide/tainted-kernels.rst
index 1df03b5cb02f..28a869c509a0 100644
--- a/Documentation/admin-guide/tainted-kernels.rst
+++ b/Documentation/admin-guide/tainted-kernels.rst
@@ -6,34 +6,34 @@ counter. This indicates that the kernel has been tainted by 
some
 mechanism.  The string is followed by a series of position-sensitive
 characters, each representing a particular tainted value.
 
-  1) 'G' if all modules loaded have a GPL or compatible license, 'P' if
+ 1)  ``G`` if all modules loaded have a GPL or compatible license, ``P`` if
  any proprietary module has been loaded.  Modules without a
  MODULE_LICENSE or with a MODULE_LICENSE that is not recognised by
  insmod as GPL compatible are assumed to be proprietary.
 
-  2) ``F`` if any module was force loaded by ``insmod -f``, ``' '`` if all
+ 2)  ``F`` if any module was force loaded by ``insmod -f``, ``' '`` if all
  modules were loaded normally.
 
-  3) ``S`` if the oops occurred on an SMP kernel running on hardware that
+ 3)  ``S`` if the oops occurred on an SMP kernel running on hardware that
  hasn't been certified as safe to run multiprocessor.
  Currently this occurs only on various Athlons that are not
  SMP capable.
 
-  4) ``R`` if a module was force unloaded by ``rmmod -f``, ``' '`` if all
+ 4)  ``R`` if a module was force unloaded by ``rmmod -f``, ``' '`` if all
  modules were unloaded normally.
 
-  5) ``M`` if any processor has reported a Machine Check Exception,
+ 5)  ``M`` if any processor has reported a Machine Check Exception,
  ``' '`` if no Machine Check Exceptions have occurred.
 
-  6) ``B`` if a page-release function has found a bad page reference or
+ 6)  ``B`` if a page-release function has found a bad page reference or
  some unexpected page flags.
 
-  7) ``U`` if a user or user application specifically requested that the
+ 7)  ``U`` if a user or user application specifically requested that the
  Tainted flag be set, ``' '`` otherwise.
 
-  8) ``D`` if the kernel has died recently, i.e. there was an OOPS or BUG.
+ 8)  ``D`` if the kernel has died recently, i.e. there was an OOPS or BUG.
 
-  9) ``A`` if the ACPI table has been overridden.
+ 9)  ``A`` if the ACPI table has been overridden.
 
  10) ``W`` if a warning has previously been issued by the kernel.
  (Though some warnings may set more specific taint flags.)
-- 
2.15.1

[PATCH v3 0/4] Nintendo Wii GPIO driver

[Jonathan Neuschäfer]

v2: https://www.spinics.net/lists/devicetree/msg211283.html

This series adds a driver for the GPIO controller used in the Nintendo
Wii game console.

Previous versions of this series included a patch to kernel/resource.c
("resource: Extend the PPC32 reserved memory hack") to work around a
resource allocation problem on PPC32. In this version, I dropped this
patch, because the problem will be solved differently and in a separate
patchset.

I also dropped the dt-bindings patch, because Linus Walleij has already
applied it.

Jonathan Neuschäfer (4):
  powerpc: wii: Explicitly configure GPIO owner for poweroff pin
  gpio: Add GPIO driver for Nintendo Wii
  powerpc: wii.dts: Add ngpios property
  powerpc: wii.dts: Add GPIO line names

 arch/powerpc/boot/dts/wii.dts|   9 +++
 arch/powerpc/platforms/embedded6xx/wii.c |   7 ++
 drivers/gpio/Kconfig |   9 +++
 drivers/gpio/Makefile|   1 +
 drivers/gpio/gpio-hlwd.c | 115 +++
 5 files changed, 141 insertions(+)
 create mode 100644 drivers/gpio/gpio-hlwd.c

-- 
2.15.1

[PATCH v3 1/4] powerpc: wii: Explicitly configure GPIO owner for poweroff pin

[Jonathan Neuschäfer]

The Hollywood chipset's GPIO controller has two sets of registers: One
for access by the PowerPC CPU, and one for access by the ARM coprocessor
(but both are accessible from the PPC because the memory firewall
(AHBPROT) is usually disabled when booting Linux, today).

The wii_power_off function currently assumes that the poweroff GPIO pin
is configured for use via the ARM side, but the upcoming GPIO driver
configures all pins for use via the PPC side, breaking poweroff.

Configure the owner register explicitly in wii_power_off to make
wii_power_off work with and without the new GPIO driver.

I think the Wii can be switched to the generic gpio-poweroff driver,
after the GPIO driver is merged.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---

v2, v3:
- no change
---
 arch/powerpc/platforms/embedded6xx/wii.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/powerpc/platforms/embedded6xx/wii.c 
b/arch/powerpc/platforms/embedded6xx/wii.c
index 160fa09533ba..4682327f76a9 100644
--- a/arch/powerpc/platforms/embedded6xx/wii.c
+++ b/arch/powerpc/platforms/embedded6xx/wii.c
@@ -45,6 +45,7 @@
 #define HW_GPIO_BASE(idx)  (idx * 0x20)
 #define HW_GPIO_OUT(idx)   (HW_GPIO_BASE(idx) + 0)
 #define HW_GPIO_DIR(idx)   (HW_GPIO_BASE(idx) + 4)
+#define HW_GPIO_OWNER  (HW_GPIO_BASE(1) + 0x1c)
 
 #define HW_GPIO_SHUTDOWN   (1<<1)
 #define HW_GPIO_SLOT_LED   (1<<5)
@@ -177,6 +178,12 @@ static void wii_power_off(void)
local_irq_disable();
 
if (hw_gpio) {
+   /*
+* set the owner of the shutdown pin to ARM, because it is
+* accessed through the registers for the ARM, below
+*/
+   clrbits32(hw_gpio + HW_GPIO_OWNER, HW_GPIO_SHUTDOWN);
+
/* make sure that the poweroff GPIO is configured as output */
setbits32(hw_gpio + HW_GPIO_DIR(1), HW_GPIO_SHUTDOWN);
 
-- 
2.15.1

[PATCH v3 2/4] gpio: Add GPIO driver for Nintendo Wii

[Jonathan Neuschäfer]

The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
that supports a configurable number of pins (up to 32), interrupts, and
some special mechanisms to share the controller between the system's
security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
not supported.

This patch adds a basic driver for this GPIO controller. Interrupt
support will come in a later patch.

This patch is based on code developed by Albert Herranz and the GameCube
Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
has grown quite dissimilar.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
Cc: Albert Herranz <albert_herr...@yahoo.es>
Cc: Segher Boessenkool <seg...@kernel.crashing.org>
<---

v3:
- Do some style cleanups, as suggest by Andy Shevchenko

v2:
- Change hlwd_gpio_driver.driver.name to "gpio-hlwd" to match the
  filename (was "hlwd_gpio")
- Remove unnecessary include of linux/of_gpio.h, as suggested by Linus
  Walleij.
- Add struct device pointer to context struct to make it possible to use
  dev_info(hlwd->dev, "..."), as suggested by Linus Walleij
- Use the GPIO_GENERIC library to reduce code size, as suggested by
  Linus Walleij
- Use iowrite32be instead of __raw_writel for big-endian MMIO access, as
  suggested by Linus Walleij
- Remove commit message paragraph suggesting to diff against the
  original driver, because it's so different now
---
 drivers/gpio/Kconfig |   9 
 drivers/gpio/Makefile|   1 +
 drivers/gpio/gpio-hlwd.c | 115 +++
 3 files changed, 125 insertions(+)
 create mode 100644 drivers/gpio/gpio-hlwd.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d6a8e851ad13..47606dfe06cc 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -229,6 +229,15 @@ config GPIO_GRGPIO
  Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
  VHDL IP core library.
 
+config GPIO_HLWD
+   tristate "Nintendo Wii (Hollywood) GPIO"
+   depends on OF_GPIO
+   select GPIO_GENERIC
+   help
+ Select this to support the GPIO controller of the Nintendo Wii.
+
+ If unsure, say N.
+
 config GPIO_ICH
tristate "Intel ICH GPIO"
depends on PCI && X86
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4bc24febb889..492f62d0eb59 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_GPIO_FTGPIO010)  += gpio-ftgpio010.o
 obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
 obj-$(CONFIG_GPIO_GPIO_MM) += gpio-gpio-mm.o
 obj-$(CONFIG_GPIO_GRGPIO)  += gpio-grgpio.o
+obj-$(CONFIG_GPIO_HLWD)+= gpio-hlwd.o
 obj-$(CONFIG_HTC_EGPIO)+= gpio-htc-egpio.o
 obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
 obj-$(CONFIG_GPIO_INGENIC) += gpio-ingenic.o
diff --git a/drivers/gpio/gpio-hlwd.c b/drivers/gpio/gpio-hlwd.c
new file mode 100644
index ..a63136a68ba3
--- /dev/null
+++ b/drivers/gpio/gpio-hlwd.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (C) 2008-2009 The GameCube Linux Team
+// Copyright (C) 2008,2009 Albert Herranz
+// Copyright (C) 2017-2018 Jonathan Neuschäfer
+//
+// Nintendo Wii (Hollywood) GPIO driver
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * Register names and offsets courtesy of WiiBrew:
+ * https://wiibrew.org/wiki/Hardware/Hollywood_GPIOs
+ *
+ * Note that for most registers, there are two versions:
+ * - HW_GPIOB_* Is always accessible by the Broadway PowerPC core, but does
+ *   always give access to all GPIO lines
+ * - HW_GPIO_* Is only accessible by the Broadway PowerPC code if the memory
+ *   firewall (AHBPROT) in the Hollywood chipset has been configured to allow
+ *   such access.
+ *
+ * The ownership of each GPIO line can be configured in the HW_GPIO_OWNER
+ * register: A one bit configures the line for access via the HW_GPIOB_*
+ * registers, a zero bit indicates access via HW_GPIO_*. This driver uses
+ * HW_GPIOB_*.
+ */
+#define HW_GPIOB_OUT   0x00
+#define HW_GPIOB_DIR   0x04
+#define HW_GPIOB_IN0x08
+#define HW_GPIOB_INTLVL0x0c
+#define HW_GPIOB_INTFLAG   0x10
+#define HW_GPIOB_INTMASK   0x14
+#define HW_GPIOB_INMIR 0x18
+#define HW_GPIO_ENABLE 0x1c
+#define HW_GPIO_OUT0x20
+#define HW_GPIO_DIR0x24
+#define HW_GPIO_IN 0x28
+#define HW_GPIO_INTLVL 0x2c
+#define HW_GPIO_INTFLAG0x30
+#define HW_GPIO_INTMASK0x34
+#define HW_GPIO_INMIR  0x38
+#define HW_GPIO_OWNER  0x3c
+
+struct hlwd_gpio {
+   struct gpio_chip gpioc;
+   void __iomem *regs;
+};
+
+static int hlwd_gpio_probe(struct platform_device *pdev)
+{
+   struct 

[PATCH v3 4/4] powerpc: wii.dts: Add GPIO line names

[Jonathan Neuschäfer]

These are the GPIO line names on a Nintendo Wii, as documented in:
https://wiibrew.org/wiki/Hardware/Hollywood_GPIOs

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---

v2, v3:
- no change
---
 arch/powerpc/boot/dts/wii.dts | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/boot/dts/wii.dts b/arch/powerpc/boot/dts/wii.dts
index 7235e375919c..07d5e84e98b1 100644
--- a/arch/powerpc/boot/dts/wii.dts
+++ b/arch/powerpc/boot/dts/wii.dts
@@ -178,6 +178,14 @@
gpio-controller;
ngpios = <24>;
 
+   gpio-line-names =
+   "POWER", "SHUTDOWN", "FAN", "DC_DC",
+   "DI_SPIN", "SLOT_LED", "EJECT_BTN", "SLOT_IN",
+   "SENSOR_BAR", "DO_EJECT", "EEP_CS", "EEP_CLK",
+   "EEP_MOSI", "EEP_MISO", "AVE_SCL", "AVE_SDA",
+   "DEBUG0", "DEBUG1", "DEBUG2", "DEBUG3",
+   "DEBUG4", "DEBUG5", "DEBUG6", "DEBUG7";
+
/*
 * This is commented out while a standard binding
 * for i2c over gpio is defined.
-- 
2.15.1

[PATCH v3 3/4] powerpc: wii.dts: Add ngpios property

[Jonathan Neuschäfer]

The Hollywood GPIO controller supports 32 GPIOs, but on the Wii, only 24
are used.

Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
---

v2, v3:
- no change
---
 arch/powerpc/boot/dts/wii.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/boot/dts/wii.dts b/arch/powerpc/boot/dts/wii.dts
index 40b324b6391e..7235e375919c 100644
--- a/arch/powerpc/boot/dts/wii.dts
+++ b/arch/powerpc/boot/dts/wii.dts
@@ -176,6 +176,7 @@
compatible = "nintendo,hollywood-gpio";
reg = <0x0d8000c0 0x40>;
gpio-controller;
+   ngpios = <24>;
 
/*
 * This is commented out while a standard binding
-- 
2.15.1

Re: [PATCH v2 3/6] gpio: Add GPIO driver for Nintendo Wii

[Jonathan Neuschäfer]

On Wed, Feb 07, 2018 at 01:29:45PM +0100, Linus Walleij wrote:
> On Mon, Jan 22, 2018 at 6:04 AM, Jonathan Neuschäfer
> <j.neuschae...@gmx.net> wrote:
> 
> > The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
> > that supports a configurable number of pins (up to 32), interrupts, and
> > some special mechanisms to share the controller between the system's
> > security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
> > not supported.
> >
> > This patch adds a basic driver for this GPIO controller. Interrupt
> > support will come in a later patch.
> >
> > This patch is based on code developed by Albert Herranz and the GameCube
> > Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
> > available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
> > has grown quite dissimilar.
> >
> > Signed-off-by: Jonathan Neuschäfer <j.neuschae...@gmx.net>
> > Cc: Albert Herranz <albert_herr...@yahoo.es>
> > Cc: Segher Boessenkool <seg...@kernel.crashing.org>
> > ---
> >
> > v2:
> 
> All looks very good to me, Andy had some additional comments,
> once addressed in v3 I will apply this.
> 
> You only need to resend this patch as I already applied the DT
> bindings.

This driver can't be used until the resource mapping problem on PPC32
(see patch 1/6 and the related discussion) is solved or worked around
with out-of-tree patches (such as patch 1/6). Should I send v3 anyway?


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature

Re: [PATCH 1/6] powerpc/mm/32: Use pfn_valid to check if pointer is in RAM

[Jonathan Neuschäfer]

Hello Christophe,

On Tue, Feb 20, 2018 at 06:45:09PM +0100, christophe leroy wrote:
[...]
> > -   if (slab_is_available() && (p < virt_to_phys(high_memory)) &&
> > +   if (slab_is_available() && pfn_valid(__phys_to_pfn(p)) &&
> 
> I'm not sure this is equivalent:
> 
> high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
> #define ARCH_PFN_OFFSET   ((unsigned long)(MEMORY_START >> 
> PAGE_SHIFT))
> #define pfn_valid(pfn)((pfn) >= ARCH_PFN_OFFSET && (pfn) < 
> max_mapnr)
> set_max_mapnr(max_pfn);
> 
> So in the current implementation it checks against max_low_pfn while your
> patch checks against max_pfn
> 
>   max_low_pfn = max_pfn = memblock_end_of_DRAM() >> PAGE_SHIFT;
> #ifdef CONFIG_HIGHMEM
>   max_low_pfn = lowmem_end_addr >> PAGE_SHIFT;
> #endif

Good point, I haven't considered CONFIG_HIGHMEM before.

As far as I understand it, in the non-CONFIG_HIGHMEM case:

  - max_low_pfn is set to the same value as max_pfn, so the ioremap
check should detect the same PFNs as RAM.

and with CONFIG_HIGHMEM:

  - max_low_pfn is set to lowmem_end_addr >> PAGE_SHIFT
  - but max_pfn isn't

So, I think you're right.


While looking through arch/powerpc/mm, I noticed that there's a
page_is_ram function, which simply uses the memblocks directly, on
PPC32. It seems like a good candidate for the RAM check in
__ioremap_caller, except that there's this code, which apparently
trashes memblock 0 completely on non-CONFIG_NEED_MULTIPLE_NODES:

  https://elixir.bootlin.com/linux/v4.16-rc2/source/arch/powerpc/mm/mem.c#L223


Thanks,
Jonathan Neuschäfer


signature.asc
Description: PGP signature