* [PATCH] bus: ti-sysc: Fix clock handling for no-idle quirks
From: Tony Lindgren @ 2019-09-05 14:03 UTC (permalink / raw)
To: linux-omap
Cc: Nishanth Menon, Tero Kristo, Grygorii Strashko,
Vignesh Raghavendra, Dave Gerlach, Greg Kroah-Hartman,
linux-kernel, Peter Ujfalusi, Faiz Abbas, Keerthy,
linux-arm-kernel, Roger Quadros
NFSroot can fail on dra7 when cpsw is probed using ti-sysc interconnect
target module driver as reported by Keerthy.
Device clocks and the interconnect target module may or may not be
enabled by the bootloader on init, but we currently assume the clocks
and module are on from the bootloader for "ti,no-idle" and
"ti,no-idle-on-init" quirks as reported by Grygorii Strashko.
Let's fix the issue by always enabling clocks init, and
never disable them for "ti,no-idle" quirk. For "ti,no-idle-on-init"
quirk, we must decrement the usage count later on to allow PM
runtime to idle the module if requested.
Fixes: 1a5cd7c23cc5 ("bus: ti-sysc: Enable all clocks directly during init to read revision")
Cc: Keerthy <j-keerthy@ti.com>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Reported-by: Keerthy <j-keerthy@ti.com>
Reported-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/bus/ti-sysc.c | 48 +++++++++++++++++++++++++++++++++----------
1 file changed, 37 insertions(+), 11 deletions(-)
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -1632,17 +1632,19 @@ static int sysc_init_module(struct sysc *ddata)
if (error)
return error;
- if (manage_clocks) {
- sysc_clkdm_deny_idle(ddata);
+ sysc_clkdm_deny_idle(ddata);
- error = sysc_enable_opt_clocks(ddata);
- if (error)
- return error;
+ /*
+ * Always enable clocks. The bootloader may or may not have enabled
+ * the related clocks.
+ */
+ error = sysc_enable_opt_clocks(ddata);
+ if (error)
+ return error;
- error = sysc_enable_main_clocks(ddata);
- if (error)
- goto err_opt_clocks;
- }
+ error = sysc_enable_main_clocks(ddata);
+ if (error)
+ goto err_opt_clocks;
if (!(ddata->cfg.quirks & SYSC_QUIRK_NO_RESET_ON_INIT)) {
error = sysc_rstctrl_reset_deassert(ddata, true);
@@ -1660,7 +1662,7 @@ static int sysc_init_module(struct sysc *ddata)
goto err_main_clocks;
}
- if (!ddata->legacy_mode && manage_clocks) {
+ if (!ddata->legacy_mode) {
error = sysc_enable_module(ddata->dev);
if (error)
goto err_main_clocks;
@@ -1677,6 +1679,7 @@ static int sysc_init_module(struct sysc *ddata)
if (manage_clocks)
sysc_disable_main_clocks(ddata);
err_opt_clocks:
+ /* No re-enable of clockdomain autoidle to prevent module autoidle */
if (manage_clocks) {
sysc_disable_opt_clocks(ddata);
sysc_clkdm_allow_idle(ddata);
@@ -2357,6 +2360,28 @@ static void ti_sysc_idle(struct work_struct *work)
ddata = container_of(work, struct sysc, idle_work.work);
+ /*
+ * One time decrement of clock usage counts if left on from init.
+ * Note that we disable opt clocks unconditionally in this case
+ * as they are enabled unconditionally during init without
+ * considering sysc_opt_clks_needed() at that point.
+ */
+ if (ddata->cfg.quirks & (SYSC_QUIRK_NO_IDLE |
+ SYSC_QUIRK_NO_IDLE_ON_INIT)) {
+ sysc_clkdm_deny_idle(ddata);
+ sysc_disable_main_clocks(ddata);
+ sysc_disable_opt_clocks(ddata);
+ sysc_clkdm_allow_idle(ddata);
+ }
+
+ /* Keep permanent PM runtime usage count for SYSC_QUIRK_NO_IDLE */
+ if (ddata->cfg.quirks & SYSC_QUIRK_NO_IDLE)
+ return;
+
+ /*
+ * Decrement PM runtime usage count for SYSC_QUIRK_NO_IDLE_ON_INIT
+ * and SYSC_QUIRK_NO_RESET_ON_INIT
+ */
if (pm_runtime_active(ddata->dev))
pm_runtime_put_sync(ddata->dev);
}
@@ -2445,7 +2470,8 @@ static int sysc_probe(struct platform_device *pdev)
INIT_DELAYED_WORK(&ddata->idle_work, ti_sysc_idle);
/* At least earlycon won't survive without deferred idle */
- if (ddata->cfg.quirks & (SYSC_QUIRK_NO_IDLE_ON_INIT |
+ if (ddata->cfg.quirks & (SYSC_QUIRK_NO_IDLE |
+ SYSC_QUIRK_NO_IDLE_ON_INIT |
SYSC_QUIRK_NO_RESET_ON_INIT)) {
schedule_delayed_work(&ddata->idle_work, 3000);
} else {
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] pinctrl: bcm: remove redundant assignment to pointer log
From: Colin King @ 2019-09-05 14:09 UTC (permalink / raw)
To: Linus Walleij, Ray Jui, Scott Branden, bcm-kernel-feedback-list,
linux-gpio, linux-arm-kernel
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The pointer log is being initialized with a value that is never read
and is being re-assigned a little later on. The assignment is
redundant and hence can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/pinctrl/bcm/pinctrl-cygnus-mux.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/pinctrl/bcm/pinctrl-cygnus-mux.c b/drivers/pinctrl/bcm/pinctrl-cygnus-mux.c
index 44df35942a43..dcab2204c60c 100644
--- a/drivers/pinctrl/bcm/pinctrl-cygnus-mux.c
+++ b/drivers/pinctrl/bcm/pinctrl-cygnus-mux.c
@@ -923,7 +923,6 @@ static int cygnus_mux_log_init(struct cygnus_pinctrl *pinctrl)
if (!pinctrl->mux_log)
return -ENOMEM;
- log = pinctrl->mux_log;
for (i = 0; i < CYGNUS_NUM_IOMUX_REGS; i++) {
for (j = 0; j < CYGNUS_NUM_MUX_PER_REG; j++) {
log = &pinctrl->mux_log[i * CYGNUS_NUM_MUX_PER_REG
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] bus: ti-sysc: Fix clock handling for no-idle quirks
From: Grygorii Strashko @ 2019-09-05 14:17 UTC (permalink / raw)
To: Tony Lindgren, linux-omap
Cc: Nishanth Menon, Tero Kristo, Vignesh Raghavendra, Dave Gerlach,
Greg Kroah-Hartman, linux-kernel, Peter Ujfalusi, Faiz Abbas,
Keerthy, linux-arm-kernel, Roger Quadros
In-Reply-To: <20190905140337.19373-1-tony@atomide.com>
On 05/09/2019 17:03, Tony Lindgren wrote:
> NFSroot can fail on dra7 when cpsw is probed using ti-sysc interconnect
> target module driver as reported by Keerthy.
>
> Device clocks and the interconnect target module may or may not be
> enabled by the bootloader on init, but we currently assume the clocks
> and module are on from the bootloader for "ti,no-idle" and
> "ti,no-idle-on-init" quirks as reported by Grygorii Strashko.
>
> Let's fix the issue by always enabling clocks init, and
> never disable them for "ti,no-idle" quirk. For "ti,no-idle-on-init"
> quirk, we must decrement the usage count later on to allow PM
> runtime to idle the module if requested.
>
> Fixes: 1a5cd7c23cc5 ("bus: ti-sysc: Enable all clocks directly during init to read revision")
> Cc: Keerthy <j-keerthy@ti.com>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Reported-by: Keerthy <j-keerthy@ti.com>
> Reported-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> drivers/bus/ti-sysc.c | 48 +++++++++++++++++++++++++++++++++----------
> 1 file changed, 37 insertions(+), 11 deletions(-)
>
Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
Thank you, Tony.
--
Best regards,
grygorii
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 01/11] kselftest: arm64: add skeleton Makefile
From: Dave Martin @ 2019-09-05 14:18 UTC (permalink / raw)
To: Cristian Marussi
Cc: amit.kachhap, andreyknvl, shuah, linux-kselftest,
linux-arm-kernel
In-Reply-To: <4e7f583f-df36-1d7b-7a41-160abc60a296@arm.com>
On Thu, Sep 05, 2019 at 02:45:39PM +0100, Cristian Marussi wrote:
> On 04/09/2019 12:47, Dave Martin wrote:
> > On Mon, Sep 02, 2019 at 12:29:22pm +0100, Cristian Marussi wrote:
> >> Add a new arm64-specific empty subsystem amongst TARGETS of KSFT build
> >> framework; keep these new arm64 KSFT testcases separated into distinct
> >
> > Nit: this isn't true any more, since the tags tests already added the
> > arm64 subsystem here.
>
> Ok
> >
> >> subdirs inside tools/testing/selftests/arm64/ depending on the specific
> >> subsystem targeted.
> >>
> >> Add into toplevel arm64 KSFT Makefile a mechanism to guess the effective
> >> location of Kernel headers as installed by KSFT framework.
> >
> > This:
> >
> >> Merge with
> >>
> >> commit 9ce1263033cd ("selftests, arm64: add a selftest for passing
> >> tagged pointers to kernel")
> >>
> >> while moving such KSFT tags tests inside their own subdirectory
> >> (arm64/tags).
> >
> > ...could be put under the tearoff, but it doesn't really belong in the
> > commit message IMHO.
> >
> > I suggest rewriting the commit message to reflect the current
> > situation (but it can be kept brief).
> >
> > Basically, what this patch now seems to do is to prepare for adding
> > more arm64 tests, by moving the tags tests into their own subdirectory
> > and extending the existing skeleton Makefile as appropriate.
> >
>
> Ok
> >> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> >> ---
> >> v4 --> v5
> >> - rebased on arm64/for-next/core
> >> - merged this patch with KSFT arm64 tags patch, while moving the latter
> >> into its own subdir
> >> - moved kernel header includes search mechanism from KSFT arm64
> >> SIGNAL Makefile
> >> - export proper top_srcdir ENV for lib.mk
> >> v3 --> v4
> >> - comment reword
> >> - simplified documentation in README
> >> - dropped README about standalone
> >> ---
> >
> > [...]
> >
> >> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> >> index a61b2e743e99..5dbb0ffdfc9a 100644
> >> --- a/tools/testing/selftests/arm64/Makefile
> >> +++ b/tools/testing/selftests/arm64/Makefile
> >> @@ -1,11 +1,69 @@
> >> # SPDX-License-Identifier: GPL-2.0
> >> +# Copyright (C) 2019 ARM Limited
> >
> > Change of copyright? This isn't pure Arm IP upstream IIUC.
> >
> > Maybe just drop it: Makefiles don't usually contain significant IP, so
> > many have no copyright message anyway.
> >
> Right. I'll drop.
> >> -# ARCH can be overridden by the user for cross compiling
> >> -ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> >> +# When ARCH not overridden for crosscompiling, lookup machine
> >> +ARCH ?= $(shell uname -m)
> >> +ARCH := $(shell echo $(ARCH) | sed -e s/aarch64/arm64/)
> >>
> >> -ifneq (,$(filter $(ARCH),aarch64 arm64))
> >> -TEST_GEN_PROGS := tags_test
> >> -TEST_PROGS := run_tags_test.sh
> >> +ifeq ("x$(ARCH)", "xarm64")
> >> +SUBDIRS := tags
> >> +else
> >> +SUBDIRS :=
> >> endif
> >>
> >> -include ../lib.mk
> >> +CFLAGS := -Wall -O2 -g
> >> +
> >> +# A proper top_srcdir is needed by KSFT(lib.mk)
> >> +top_srcdir = ../../../../..
> >> +
> >> +# Additional include paths needed by kselftest.h and local headers
> >> +CFLAGS += -I$(top_srcdir)/tools/testing/selftests/
> >> +
> >> +# Guessing where the Kernel headers could have been installed
> >> +# depending on ENV config
> >> +ifeq ($(KBUILD_OUTPUT),)
> >> +khdr_dir = $(top_srcdir)/usr/include
> >> +else
> >> +# the KSFT preferred location when KBUILD_OUTPUT is set
> >> +khdr_dir = $(KBUILD_OUTPUT)/kselftest/usr/include
> >> +endif
> >
> > Looking at this, can we just pass the directory in from the toplevel
> > "all" rule instead of guessing?
> >
> Do you mean toplevel in KSFT ?
> I think it's how should be done at the end, but I was trying to keep this series on
> arm64/ lands only. (also maybe I'm missing something obvious in KSFT handling of this
> situation....even though many other KSFT use built CFLAGS like: -I../../../usr/include
> or similar)
>
> > Maybe don't churn this for now though. It's something that could be
> > looked at later.
> >
>
> Ok. I'll leave here and fix it to avoid relative paths...which could be problematic
> when exported to lower level Makefiles.
>
> Cheers
>
> Cristian
Sounds reasonable
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 09/11] kselftest: arm64: fake_sigreturn_duplicated_fpsimd
From: Dave Martin @ 2019-09-05 14:20 UTC (permalink / raw)
To: Cristian Marussi
Cc: amit.kachhap, andreyknvl, shuah, linux-arm-kernel,
linux-kselftest
In-Reply-To: <365b527e-793e-ad83-47a8-5d8692ed50c4@arm.com>
On Thu, Sep 05, 2019 at 02:32:09PM +0100, Cristian Marussi wrote:
> On 05/09/2019 13:39, Dave Martin wrote:
> > On Thu, Sep 05, 2019 at 01:15:58PM +0100, Cristian Marussi wrote:
> >> Hi
> >>
> >> On 04/09/2019 12:49, Dave Martin wrote:
> >>> On Mon, Sep 02, 2019 at 12:29:30pm +0100, Cristian Marussi wrote:
> >>>> Add a simple fake_sigreturn testcase which builds a ucontext_t with
> >>>> an anomalous additional fpsimd_context and place it onto the stack.
> >>>> Expects a SIGSEGV on test PASS.
> >>>>
> >>>> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> >>>> ---
> >>>> v3 --> v4
> >>>> - fix commit
> >>>> - missing include
> >>>> - using new get_starting_head() helper
> >>>> - added test description
> >>>> ---
> >>>> .../fake_sigreturn_duplicated_fpsimd.c | 52 +++++++++++++++++++
> >>>> 1 file changed, 52 insertions(+)
> >>>> create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> >>>>
> >>>> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> >>>> new file mode 100644
> >>>> index 000000000000..c7122c44f53f
> >>>> --- /dev/null
> >>>> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> >>>> @@ -0,0 +1,52 @@
> >>>> +// SPDX-License-Identifier: GPL-2.0
> >>>> +/*
> >>>> + * Copyright (C) 2019 ARM Limited
> >>>> + *
> >>>> + * Place a fake sigframe on the stack including an additional FPSIMD
> >>>> + * record: on sigreturn Kernel must spot this attempt and the test
> >>>> + * case is expected to be terminated via SEGV.
> >>>> + */
> >>>> +
> >>>> +#include <signal.h>
> >>>> +#include <ucontext.h>
> >>>> +
> >>>> +#include "test_signals_utils.h"
> >>>> +#include "testcases.h"
> >>>> +
> >>>> +struct fake_sigframe sf;
> >>>> +
> >>>> +static int fake_sigreturn_duplicated_fpsimd_run(struct tdescr *td,
> >>>> + siginfo_t *si, ucontext_t *uc)
> >>>> +{
> >>>> + size_t resv_sz, need_sz;
> >>>> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> >>>> +
> >>>> + /* just to fill the ucontext_t with something real */
> >>>> + if (!get_current_context(td, &sf.uc))
> >>>> + return 1;
> >>>> +
> >>>> + resv_sz = GET_SF_RESV_SIZE(sf);
> >>>> + need_sz = HDR_SZ + sizeof(struct fpsimd_context);
> >>>
> >>> Nit: Maybe write this sum in the same order as the records we're going
> >>> o append, i.e.:
> >>>
> >>> need_sz = sizeof(struct fpsimd_context) + HDR_SZ; /* for terminator */
> >>
> >> Ok
> >>
> >>>
> >>> Also, maybe fail this test if there is no fpsimd_context in the initial
> >>> frame from get_current_context(): that would be a kernel bug, but we
> >>> wouldn't be giving fake_sigreturn() duplicate fpsimd_contexts in that
> >>> case -- so this test wouldn't test the thing it's supposed to test.
> >>>
> >>
> >> Any context grabbed by get_current_context() is verified at first to be sane
> >> when is copied in the handler by ASSERT_GOOD_CONTEXT()
> >>
> >>> } else if (signum == sig_copyctx && current->live_uc) {
> >>> memcpy(current->live_uc, uc, current->live_sz);
> >>> ASSERT_GOOD_CONTEXT(current->live_uc);
> >>> current->live_uc_valid = 1;
> >>
> >> A missing fpsimd in the original sigframe would lead to an abort()
> >> straight away while preparing the test, and the test will fail.
> >
> > OK, but there is no abort() in ASSERT_GOOD_CONTEXT(), only assert(0).
> > Can you add an abort() after the assert() in there in patch 2?
> >
> Ok yes, I meant the abort coming from assert(0) in fact....I'll review all
> the critical asserts for additional aborts in V6
OK -- I guess this only matters for things that should be reported as
test failures. Things that are purely bugs in the test are less of a
concern.
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] pinctrl: at91-pio4: implement .get_multiple and .set_multiple
From: Alexandre Belloni @ 2019-09-05 14:13 UTC (permalink / raw)
To: Linus Walleij
Cc: Alexandre Belloni, linux-kernel, linux-gpio, Ludovic Desroches,
linux-arm-kernel
Implement .get_multiple and .set_multiple to allow reading or setting
multiple pins simultaneously. Pins in the same bank will all be switched at
the same time, improving synchronization and performances.
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
drivers/pinctrl/pinctrl-at91-pio4.c | 60 +++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c
index d6de4d360cd4..488a302a60d4 100644
--- a/drivers/pinctrl/pinctrl-at91-pio4.c
+++ b/drivers/pinctrl/pinctrl-at91-pio4.c
@@ -328,6 +328,35 @@ static int atmel_gpio_get(struct gpio_chip *chip, unsigned offset)
return !!(reg & BIT(pin->line));
}
+static int atmel_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask,
+ unsigned long *bits)
+{
+ struct atmel_pioctrl *atmel_pioctrl = gpiochip_get_data(chip);
+ unsigned int bank;
+
+ bitmap_zero(bits, atmel_pioctrl->npins);
+
+ for (bank = 0; bank < atmel_pioctrl->nbanks; bank++) {
+ unsigned int word = bank;
+ unsigned int offset = 0;
+ unsigned int reg;
+
+#if ATMEL_PIO_NPINS_PER_BANK != BITS_PER_LONG
+ word = BIT_WORD(bank * ATMEL_PIO_NPINS_PER_BANK);
+ offset = bank * ATMEL_PIO_NPINS_PER_BANK % BITS_PER_LONG;
+#endif
+ if (!mask[word])
+ continue;
+
+ reg = atmel_gpio_read(atmel_pioctrl, bank, ATMEL_PIO_PDSR);
+ bits[word] |= mask[word] & (reg << offset);
+
+ pr_err("ABE: %d %08x\n", bank, bits[word]);
+ }
+
+ return 0;
+}
+
static int atmel_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
int value)
{
@@ -358,11 +387,42 @@ static void atmel_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
BIT(pin->line));
}
+static void atmel_gpio_set_multiple(struct gpio_chip *chip, unsigned long *mask,
+ unsigned long *bits)
+{
+ struct atmel_pioctrl *atmel_pioctrl = gpiochip_get_data(chip);
+ unsigned int bank;
+
+ for (bank = 0; bank < atmel_pioctrl->nbanks; bank++) {
+ unsigned int bitmask;
+ unsigned int word = bank;
+
+#if ATMEL_PIO_NPINS_PER_BANK != BITS_PER_LONG
+ word = BIT_WORD(bank * ATMEL_PIO_NPINS_PER_BANK);
+#endif
+ if (!mask[word])
+ continue;
+
+ bitmask = mask[word] & bits[word];
+ atmel_gpio_write(atmel_pioctrl, bank, ATMEL_PIO_SODR, bitmask);
+
+ bitmask = mask[word] & ~bits[word];
+ atmel_gpio_write(atmel_pioctrl, bank, ATMEL_PIO_CODR, bitmask);
+
+#if ATMEL_PIO_NPINS_PER_BANK != BITS_PER_LONG
+ mask[word] >>= ATMEL_PIO_NPINS_PER_BANK;
+ bits[word] >>= ATMEL_PIO_NPINS_PER_BANK;
+#endif
+ }
+}
+
static struct gpio_chip atmel_gpio_chip = {
.direction_input = atmel_gpio_direction_input,
.get = atmel_gpio_get,
+ .get_multiple = atmel_gpio_get_multiple,
.direction_output = atmel_gpio_direction_output,
.set = atmel_gpio_set,
+ .set_multiple = atmel_gpio_set_multiple,
.to_irq = atmel_gpio_to_irq,
.base = 0,
};
--
2.21.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC PATCH V2 3/4] media: platform: Add Mediatek FD driver KConfig
From: Jerry-ch Chen @ 2019-09-05 14:14 UTC (permalink / raw)
To: Laurent Pinchart
Cc: devicetree@vger.kernel.org, Sean Cheng (鄭昇弘),
laurent.pinchart+renesas@ideasonboard.com,
Rynn Wu (吳育恩),
Christie Yu (游雅惠), srv_heupstream,
Po-Yang Huang (黃柏陽), suleiman@chromium.org,
shik@chromium.org, tfiga@chromium.org,
Jungo Lin (林明俊),
Sj Huang (黃信璋), yuzhao@chromium.org,
hans.verkuil@cisco.com, zwisler@chromium.org,
Frederic Chen (陳俊元), matthias.bgg@gmail.com,
linux-mediatek@lists.infradead.org, mchehab@kernel.org,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <20190905123054.GL5035@pendragon.ideasonboard.com>
Hi Laurent,
On Thu, 2019-09-05 at 20:30 +0800, Laurent Pinchart wrote:
> Hi Jerry,
>
> Thank you for the patch.
>
> On Tue, Jul 09, 2019 at 04:41:11PM +0800, Jerry-ch Chen wrote:
> > From: Jerry-ch Chen <jerry-ch.chen@mediatek.com>
> >
> > This patch adds KConfig for Mediatek Face Detection driver (FD).
> > FD is embedded in Mediatek SoCs. It can provide hardware
> > accelerated face detection function.
> >
> > Signed-off-by: Jerry-ch Chen <jerry-ch.chen@mediatek.com>
>
> You can squash this patch with 4/4, there's no need to keep it separate.
>
I appreciate your comments,
Ok, I will squash it.
> > ---
> > drivers/media/platform/Kconfig | 2 ++
> > drivers/media/platform/mtk-isp/fd/Kconfig | 17 +++++++++++++++++
> > 2 files changed, 19 insertions(+)
> > create mode 100644 drivers/media/platform/mtk-isp/fd/Kconfig
> >
> > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> > index a505e9f..ae99258e 100644
> > --- a/drivers/media/platform/Kconfig
> > +++ b/drivers/media/platform/Kconfig
> > @@ -32,6 +32,8 @@ source "drivers/media/platform/davinci/Kconfig"
> >
> > source "drivers/media/platform/omap/Kconfig"
> >
> > +source "drivers/media/platform/mtk-isp/fd/Kconfig"
> > +
> > config VIDEO_ASPEED
> > tristate "Aspeed AST2400 and AST2500 Video Engine driver"
> > depends on VIDEO_V4L2
> > diff --git a/drivers/media/platform/mtk-isp/fd/Kconfig b/drivers/media/platform/mtk-isp/fd/Kconfig
> > new file mode 100644
> > index 0000000..0c5eaf0
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-isp/fd/Kconfig
> > @@ -0,0 +1,17 @@
> > +config VIDEO_MEDIATEK_FD
> > + bool "Mediatek face detection processing function"
> > + select DMA_SHARED_BUFFER
> > + select VIDEOBUF2_DMA_CONTIG
> > + select VIDEOBUF2_CORE
> > + select VIDEOBUF2_V4L2
> > + select VIDEOBUF2_MEMOPS
> > + select VIDEOBUF2_VMALLOC
>
> Do you need both VIDEOBUF2_DMA_CONTIG and VIDEOBUF2_VMALLOC ? The driver
> doesn't seem to make use of VIDEOBUF2_VMALLOC.
>
No, I should remove it. and also would like to update as following:
depends on VIDEO_V4L2
depends on ARCH_MEDIATEK || COMPILE_TEST
select VIDEOBUF2_DMA_CONTIG
select VIDEOBUF2_CORE
select VIDEOBUF2_V4L2
select VIDEOBUF2_MEMOPS
select MEDIA_CONTROLLER
select MTK_SCP
> > + select MEDIA_CONTROLLER
> > +
> > + default n
> > + help
> > + Support the Face Detectioin (FD) feature.
>
> s/Detectioin/Detection/
>
Typo fixed.
> Maybe "... feature found in the Mediatek <list of SoCs> SoCs." ?
I will refine as:
Support the Face Detection (FD) feature in the Mediatek mt8183 Soc.
Thanks and best regards,
Jerry
>
> > +
> > + FD driver is a V4L2 memory-to-memory device driver which
> > + provides hardware accelerated face detection function,
> > + it can detect different sizes of faces in a raw image.
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] pinctrl: at91-pio4: implement .get_multiple and .set_multiple
From: Claudiu.Beznea @ 2019-09-05 14:33 UTC (permalink / raw)
To: alexandre.belloni, linus.walleij
Cc: linux-gpio, Ludovic.Desroches, linux-kernel, linux-arm-kernel
In-Reply-To: <20190905141304.22005-1-alexandre.belloni@bootlin.com>
On 05.09.2019 17:13, Alexandre Belloni wrote:
> + pr_err("ABE: %d %08x\n", bank, bits[word]);
Is this needed?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 1/1] i2c: qcom-geni: Provide an option to disable DMA processing
From: Lee Jones @ 2019-09-05 14:33 UTC (permalink / raw)
To: Peter Rosin
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, Wolfram Sang,
linux-kernel@vger.kernel.org, agross@kernel.org,
robh+dt@kernel.org, bjorn.andersson@linaro.org, vkoul@kernel.org,
alokc@codeaurora.org, linux-i2c@vger.kernel.org,
linux-arm-msm@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <3458ed2a-ae49-b46b-3e89-ce039a2749b4@axentia.se>
On Thu, 05 Sep 2019, Peter Rosin wrote:
> On 2019-09-05 15:49, Wolfram Sang wrote:
> > Hi Lee,
> >
> > I understand you are in a hurry, but please double check before
> > sending...
>
> Linus indicated that an rc8 is coming up, which should provide an extra week.
> https://lwn.net/Articles/798152/
That is good news.
> > On Thu, Sep 05, 2019 at 11:22:47AM +0100, Lee Jones wrote:
> >> We have a production-level laptop (Lenovo Yoga C630) which is exhibiting
> >> a rather horrific bug. When I2C HID devices are being scanned for at
> >> boot-time the QCom Geni based I2C (Serial Engine) attempts to use DMA.
> >> When it does, the laptop reboots and the user never sees the OS.
> >>
> >> The beautiful thing about this approach is that, *if* the Geni SE DMA
> >> ever starts working, we can remove the C code and any old properties
> >> left in older DTs just become NOOP. Older kernels with newer DTs (less
> >> of a priority) *still* will not work - but they do not work now anyway.
> >
> > ... becasue this paragraph doesn't fit anymore. Needs to be reworded.
Yes, you're right. I noticed almost the moment I pressed send. :(
> >> Fixes: 8bc529b25354 ("soc: qcom: geni: Add support for ACPI")
> >
> > As said in the other thread, I don't get it, but this is not a show
> > stopper for me.
Ah wait. Yes, this is applied against the wrong patch.
Please ignore.
> WAG: because ACPI made some driver load at all, and when it
> did it something started happening which crashed some machines.
I'm not sure I understand this sentence.
... resending now.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/2] i2c: qcom-geni: Provide an option to disable DMA processing
From: Lee Jones @ 2019-09-05 14:34 UTC (permalink / raw)
To: Wolfram Sang
Cc: mark.rutland, devicetree, linux-kernel, linux-arm-msm, agross,
robh+dt, bjorn.andersson, vkoul, alokc, linux-i2c,
linux-arm-kernel
In-Reply-To: <20190905134338.GF1157@kunai>
On Thu, 05 Sep 2019, Wolfram Sang wrote:
> On Thu, Sep 05, 2019 at 10:28:16AM +0100, Lee Jones wrote:
> > On Thu, 05 Sep 2019, Wolfram Sang wrote:
> >
> > >
> > > > Fixes: 8bc529b25354 ("soc: qcom: geni: Add support for ACPI")
> > >
> > > Are you sure? From visual inspection, I don't see a correlation between
> > > this commit and the fix here.
> >
> > This patch should have been part of the commit, or at the very least,
> > part of the set, alluded to above. Unfortunately, I was carrying
> > Bjorn's hack which simply returned early from geni_se_rx_dma_prep()
> > with an error, so it masked the issue.
>
> I still don't see why this basic ACPI enabling code (not touching DMA
> but only clocks and pinctrl) causes and additional handling for DMA. Am
> I overlooking something obvious?
Please ignore, I'm discussing with another patch in mind.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1] watchdog: imx_sc: this patch just fixes whitespaces
From: Oliver Graute @ 2019-09-05 14:35 UTC (permalink / raw)
To: Guenter Roeck
Cc: Mark Rutland, Aisheng Dong, Ulf Hansson,
linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org,
Shawn Guo, Sascha Hauer, linux-kernel@vger.kernel.org,
Daniel Baluta, Rob Herring, NXP Linux Team,
Pengutronix Kernel Team, Wim Van Sebroeck, Fabio Estevam,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <016e268c-f289-4127-a47e-3d8faa790ded@roeck-us.net>
On 05/09/19, Guenter Roeck wrote:
> On 9/5/19 12:44 AM, Oliver Graute wrote:
> > Fix only whitespace errors in imx_sc_wdt_probe()
> >
> > Signed-off-by: Oliver Graute <oliver.graute@kococonnector.com>
>
> This patch no longer applies due to commit "watchdog: imx_sc: Remove
> unnecessary error log".
>
ok I'll rebase patch against linux-staging/watchdog-next
Best regards,
Oliver
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2] watchdog: imx_sc: this patch just fixes whitespaces
From: Oliver Graute @ 2019-09-05 14:36 UTC (permalink / raw)
To: oliver.graute@gmail.com
Cc: Aisheng Dong, Ulf Hansson, linux-watchdog@vger.kernel.org,
Shawn Guo, Oliver Graute, linux-kernel@vger.kernel.org,
NXP Linux Team, Pengutronix Kernel Team, Wim Van Sebroeck,
Fabio Estevam, Sascha Hauer, Guenter Roeck,
linux-arm-kernel@lists.infradead.org
Fix only whitespace errors in imx_sc_wdt_probe()
Signed-off-by: Oliver Graute <oliver.graute@kococonnector.com>
---
drivers/watchdog/imx_sc_wdt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/watchdog/imx_sc_wdt.c b/drivers/watchdog/imx_sc_wdt.c
index 9260475439eb..7ea5cf54e94a 100644
--- a/drivers/watchdog/imx_sc_wdt.c
+++ b/drivers/watchdog/imx_sc_wdt.c
@@ -176,8 +176,8 @@ static int imx_sc_wdt_probe(struct platform_device *pdev)
ret = devm_watchdog_register_device(dev, wdog);
if (ret)
- return ret;
-
+ return ret;
+
ret = imx_scu_irq_group_enable(SC_IRQ_GROUP_WDOG,
SC_IRQ_WDOG,
true);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RESEND v3 1/1] i2c: qcom-geni: Provide an option to disable DMA processing
From: Lee Jones @ 2019-09-05 14:41 UTC (permalink / raw)
To: alokc, agross, robh+dt, mark.rutland, bjorn.andersson, vkoul, wsa
Cc: devicetree, linux-arm-msm, linux-kernel, linux-i2c, Lee Jones,
linux-arm-kernel
We have a production-level laptop (Lenovo Yoga C630) which is exhibiting
a rather horrific bug. When I2C HID devices are being scanned for at
boot-time the QCom Geni based I2C (Serial Engine) attempts to use DMA.
When it does, the laptop reboots and the user never sees the OS.
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/i2c/busses/i2c-qcom-geni.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index a89bfce5388e..17abf60c94ae 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -355,11 +355,13 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
{
dma_addr_t rx_dma;
unsigned long time_left;
- void *dma_buf;
+ void *dma_buf = NULL;
struct geni_se *se = &gi2c->se;
size_t len = msg->len;
- dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
+ if (!of_machine_is_compatible("lenovo,yoga-c630"))
+ dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
+
if (dma_buf)
geni_se_select_mode(se, GENI_SE_DMA);
else
@@ -394,11 +396,13 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
{
dma_addr_t tx_dma;
unsigned long time_left;
- void *dma_buf;
+ void *dma_buf = NULL;
struct geni_se *se = &gi2c->se;
size_t len = msg->len;
- dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
+ if (!of_machine_is_compatible("lenovo,yoga-c630"))
+ dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
+
if (dma_buf)
geni_se_select_mode(se, GENI_SE_DMA);
else
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2] pinctrl: at91-pio4: implement .get_multiple and .set_multiple
From: Alexandre Belloni @ 2019-09-05 14:48 UTC (permalink / raw)
To: Linus Walleij
Cc: Alexandre Belloni, linux-kernel, linux-gpio, Ludovic Desroches,
Claudiu.Beznea, linux-arm-kernel
Implement .get_multiple and .set_multiple to allow reading or setting
multiple pins simultaneously. Pins in the same bank will all be switched at
the same time, improving synchronization and performances.
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
Changes since v1
https://lore.kernel.org/lkml/20190905141304.22005-1-alexandre.belloni@bootlin.com/ :
- Removed debug line
drivers/pinctrl/pinctrl-at91-pio4.c | 58 +++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c
index d6de4d360cd4..d281ec40e098 100644
--- a/drivers/pinctrl/pinctrl-at91-pio4.c
+++ b/drivers/pinctrl/pinctrl-at91-pio4.c
@@ -328,6 +328,33 @@ static int atmel_gpio_get(struct gpio_chip *chip, unsigned offset)
return !!(reg & BIT(pin->line));
}
+static int atmel_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask,
+ unsigned long *bits)
+{
+ struct atmel_pioctrl *atmel_pioctrl = gpiochip_get_data(chip);
+ unsigned int bank;
+
+ bitmap_zero(bits, atmel_pioctrl->npins);
+
+ for (bank = 0; bank < atmel_pioctrl->nbanks; bank++) {
+ unsigned int word = bank;
+ unsigned int offset = 0;
+ unsigned int reg;
+
+#if ATMEL_PIO_NPINS_PER_BANK != BITS_PER_LONG
+ word = BIT_WORD(bank * ATMEL_PIO_NPINS_PER_BANK);
+ offset = bank * ATMEL_PIO_NPINS_PER_BANK % BITS_PER_LONG;
+#endif
+ if (!mask[word])
+ continue;
+
+ reg = atmel_gpio_read(atmel_pioctrl, bank, ATMEL_PIO_PDSR);
+ bits[word] |= mask[word] & (reg << offset);
+ }
+
+ return 0;
+}
+
static int atmel_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
int value)
{
@@ -358,11 +385,42 @@ static void atmel_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
BIT(pin->line));
}
+static void atmel_gpio_set_multiple(struct gpio_chip *chip, unsigned long *mask,
+ unsigned long *bits)
+{
+ struct atmel_pioctrl *atmel_pioctrl = gpiochip_get_data(chip);
+ unsigned int bank;
+
+ for (bank = 0; bank < atmel_pioctrl->nbanks; bank++) {
+ unsigned int bitmask;
+ unsigned int word = bank;
+
+#if ATMEL_PIO_NPINS_PER_BANK != BITS_PER_LONG
+ word = BIT_WORD(bank * ATMEL_PIO_NPINS_PER_BANK);
+#endif
+ if (!mask[word])
+ continue;
+
+ bitmask = mask[word] & bits[word];
+ atmel_gpio_write(atmel_pioctrl, bank, ATMEL_PIO_SODR, bitmask);
+
+ bitmask = mask[word] & ~bits[word];
+ atmel_gpio_write(atmel_pioctrl, bank, ATMEL_PIO_CODR, bitmask);
+
+#if ATMEL_PIO_NPINS_PER_BANK != BITS_PER_LONG
+ mask[word] >>= ATMEL_PIO_NPINS_PER_BANK;
+ bits[word] >>= ATMEL_PIO_NPINS_PER_BANK;
+#endif
+ }
+}
+
static struct gpio_chip atmel_gpio_chip = {
.direction_input = atmel_gpio_direction_input,
.get = atmel_gpio_get,
+ .get_multiple = atmel_gpio_get_multiple,
.direction_output = atmel_gpio_direction_output,
.set = atmel_gpio_set,
+ .set_multiple = atmel_gpio_set_multiple,
.to_irq = atmel_gpio_to_irq,
.base = 0,
};
--
2.21.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v4 1/1] arm64: dts: qcom: Add Lenovo Yoga C630
From: Lee Jones @ 2019-09-05 14:51 UTC (permalink / raw)
To: agross, robh+dt, mark.rutland, bjorn.andersson, arnd
Cc: devicetree, linux-arm-msm, linux-kernel, soc, Lee Jones,
linux-arm-kernel
From: Bjorn Andersson <bjorn.andersson@linaro.org>
The Lenovo Yoga C630 is built on the SDM850 from Qualcomm, but this seem
to be similar enough to the SDM845 that we can reuse the sdm845.dtsi.
Supported by this patch is: keyboard, battery monitoring, UFS storage,
USB host and Bluetooth.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Vinod Koul <vkoul@kernel.org>
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
[Lee] Reorder, change licence, remove non-upstream device node
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
Changelog:
* Reorder nodes alphabetically
* Remove superfluous node for driver not yet upstream
* Add (then remove) 'no-dma' property
* Change licence to BSD
arch/arm64/boot/dts/qcom/Makefile | 1 +
.../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 446 ++++++++++++++++++
2 files changed, 447 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 0a7e5dfce6f7..670c6c65f9e9 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -12,5 +12,6 @@ dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r2.dtb
dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r3.dtb
dtb-$(CONFIG_ARCH_QCOM) += sdm845-db845c.dtb
dtb-$(CONFIG_ARCH_QCOM) += sdm845-mtp.dtb
+dtb-$(CONFIG_ARCH_QCOM) += sdm850-lenovo-yoga-c630.dtb
dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-1000.dtb
dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-4000.dtb
diff --git a/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts b/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
new file mode 100644
index 000000000000..ded120d3aef5
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
@@ -0,0 +1,446 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Lenovo Yoga C630
+ *
+ * Copyright (c) 2019, Linaro Ltd.
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+#include "sdm845.dtsi"
+#include "pm8998.dtsi"
+
+/ {
+ model = "Lenovo Yoga C630";
+ compatible = "lenovo,yoga-c630", "qcom,sdm845";
+
+ aliases {
+ hsuart0 = &uart6;
+ };
+};
+
+&apps_rsc {
+ pm8998-rpmh-regulators {
+ compatible = "qcom,pm8998-rpmh-regulators";
+ qcom,pmic-id = "a";
+
+ vdd-l2-l8-l17-supply = <&vreg_s3a_1p35>;
+ vdd-l7-l12-l14-l15-supply = <&vreg_s5a_2p04>;
+
+ vreg_s2a_1p125: smps2 {
+ };
+
+ vreg_s3a_1p35: smps3 {
+ regulator-min-microvolt = <1352000>;
+ regulator-max-microvolt = <1352000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_s4a_1p8: smps4 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_s5a_2p04: smps5 {
+ regulator-min-microvolt = <2040000>;
+ regulator-max-microvolt = <2040000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_s7a_1p025: smps7 {
+ };
+
+ vdd_qusb_hs0:
+ vdda_hp_pcie_core:
+ vdda_mipi_csi0_0p9:
+ vdda_mipi_csi1_0p9:
+ vdda_mipi_csi2_0p9:
+ vdda_mipi_dsi0_pll:
+ vdda_mipi_dsi1_pll:
+ vdda_qlink_lv:
+ vdda_qlink_lv_ck:
+ vdda_qrefs_0p875:
+ vdda_pcie_core:
+ vdda_pll_cc_ebi01:
+ vdda_pll_cc_ebi23:
+ vdda_sp_sensor:
+ vdda_ufs1_core:
+ vdda_ufs2_core:
+ vdda_usb1_ss_core:
+ vdda_usb2_ss_core:
+ vreg_l1a_0p875: ldo1 {
+ regulator-min-microvolt = <880000>;
+ regulator-max-microvolt = <880000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vddpx_10:
+ vreg_l2a_1p2: ldo2 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ regulator-always-on;
+ };
+
+ vreg_l3a_1p0: ldo3 {
+ };
+
+ vdd_wcss_cx:
+ vdd_wcss_mx:
+ vdda_wcss_pll:
+ vreg_l5a_0p8: ldo5 {
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vddpx_13:
+ vreg_l6a_1p8: ldo6 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l7a_1p8: ldo7 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l8a_1p2: ldo8 {
+ };
+
+ vreg_l9a_1p8: ldo9 {
+ };
+
+ vreg_l10a_1p8: ldo10 {
+ };
+
+ vreg_l11a_1p0: ldo11 {
+ };
+
+ vdd_qfprom:
+ vdd_qfprom_sp:
+ vdda_apc1_cs_1p8:
+ vdda_gfx_cs_1p8:
+ vdda_qrefs_1p8:
+ vdda_qusb_hs0_1p8:
+ vddpx_11:
+ vreg_l12a_1p8: ldo12 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vddpx_2:
+ vreg_l13a_2p95: ldo13 {
+ };
+
+ vreg_l14a_1p88: ldo14 {
+ regulator-min-microvolt = <1880000>;
+ regulator-max-microvolt = <1880000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ regulator-always-on;
+ };
+
+ vreg_l15a_1p8: ldo15 {
+ };
+
+ vreg_l16a_2p7: ldo16 {
+ };
+
+ vreg_l17a_1p3: ldo17 {
+ regulator-min-microvolt = <1304000>;
+ regulator-max-microvolt = <1304000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l18a_2p7: ldo18 {
+ };
+
+ vreg_l19a_3p0: ldo19 {
+ regulator-min-microvolt = <3100000>;
+ regulator-max-microvolt = <3108000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l20a_2p95: ldo20 {
+ regulator-min-microvolt = <2960000>;
+ regulator-max-microvolt = <2960000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l21a_2p95: ldo21 {
+ };
+
+ vreg_l22a_2p85: ldo22 {
+ };
+
+ vreg_l23a_3p3: ldo23 {
+ };
+
+ vdda_qusb_hs0_3p1:
+ vreg_l24a_3p075: ldo24 {
+ regulator-min-microvolt = <3075000>;
+ regulator-max-microvolt = <3083000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l25a_3p3: ldo25 {
+ regulator-min-microvolt = <3104000>;
+ regulator-max-microvolt = <3112000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vdda_hp_pcie_1p2:
+ vdda_hv_ebi0:
+ vdda_hv_ebi1:
+ vdda_hv_ebi2:
+ vdda_hv_ebi3:
+ vdda_mipi_csi_1p25:
+ vdda_mipi_dsi0_1p2:
+ vdda_mipi_dsi1_1p2:
+ vdda_pcie_1p2:
+ vdda_ufs1_1p2:
+ vdda_ufs2_1p2:
+ vdda_usb1_ss_1p2:
+ vdda_usb2_ss_1p2:
+ vreg_l26a_1p2: ldo26 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1208000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l28a_3p0: ldo28 {
+ };
+
+ vreg_lvs1a_1p8: lvs1 {
+ };
+
+ vreg_lvs2a_1p8: lvs2 {
+ };
+ };
+};
+
+&apps_smmu {
+ /* TODO: Figure out how to survive booting with this enabled */
+ status = "disabled";
+};
+
+&gcc {
+ protected-clocks = <GCC_QSPI_CORE_CLK>,
+ <GCC_QSPI_CORE_CLK_SRC>,
+ <GCC_QSPI_CNOC_PERIPH_AHB_CLK>;
+};
+
+&i2c1 {
+ status = "okay";
+ clock-frequency = <400000>;
+};
+
+&i2c3 {
+ status = "okay";
+ clock-frequency = <400000>;
+
+ hid@15 {
+ compatible = "hid-over-i2c";
+ reg = <0x15>;
+ hid-descr-addr = <0x1>;
+
+ interrupts-extended = <&tlmm 37 IRQ_TYPE_EDGE_RISING>;
+ };
+
+ hid@2c {
+ compatible = "hid-over-i2c";
+ reg = <0x2c>;
+ hid-descr-addr = <0x20>;
+
+ interrupts-extended = <&tlmm 37 IRQ_TYPE_EDGE_RISING>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c2_hid_active>;
+ };
+};
+
+&i2c5 {
+ status = "okay";
+ clock-frequency = <400000>;
+
+ hid@10 {
+ compatible = "hid-over-i2c";
+ reg = <0x10>;
+ hid-descr-addr = <0x1>;
+
+ interrupts-extended = <&tlmm 125 IRQ_TYPE_EDGE_FALLING>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c6_hid_active>;
+ };
+};
+
+&i2c11 {
+ status = "okay";
+ clock-frequency = <400000>;
+
+ hid@5c {
+ compatible = "hid-over-i2c";
+ reg = <0x5c>;
+ hid-descr-addr = <0x1>;
+
+ interrupts-extended = <&tlmm 92 IRQ_TYPE_LEVEL_LOW>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c12_hid_active>;
+ };
+};
+
+&qup_i2c12_default {
+ drive-strength = <2>;
+ bias-disable;
+};
+
+&qup_uart6_default {
+ pinmux {
+ pins = "gpio45", "gpio46", "gpio47", "gpio48";
+ function = "qup6";
+ };
+
+ cts {
+ pins = "gpio45";
+ bias-pull-down;
+ };
+
+ rts-tx {
+ pins = "gpio46", "gpio47";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ rx {
+ pins = "gpio48";
+ bias-pull-up;
+ };
+};
+
+&qupv3_id_0 {
+ status = "okay";
+};
+
+&qupv3_id_1 {
+ status = "okay";
+};
+
+&tlmm {
+ gpio-reserved-ranges = <0 4>, <81 4>;
+
+ i2c2_hid_active: i2c2-hid-active {
+ pins = <37>;
+ function = "gpio";
+
+ input-enable;
+ bias-pull-up;
+ drive-strength = <2>;
+ };
+
+ i2c6_hid_active: i2c6-hid-active {
+ pins = <125>;
+ function = "gpio";
+
+ input-enable;
+ bias-pull-up;
+ drive-strength = <2>;
+ };
+
+ i2c12_hid_active: i2c12-hid-active {
+ pins = <92>;
+ function = "gpio";
+
+ input-enable;
+ bias-pull-up;
+ drive-strength = <2>;
+ };
+};
+
+&uart6 {
+ status = "okay";
+
+ bluetooth {
+ compatible = "qcom,wcn3990-bt";
+
+ vddio-supply = <&vreg_s4a_1p8>;
+ vddxo-supply = <&vreg_l7a_1p8>;
+ vddrf-supply = <&vreg_l17a_1p3>;
+ vddch0-supply = <&vreg_l25a_3p3>;
+ max-speed = <3200000>;
+ };
+};
+
+&ufs_mem_hc {
+ status = "okay";
+
+ vcc-supply = <&vreg_l20a_2p95>;
+ vcc-max-microamp = <600000>;
+};
+
+&ufs_mem_phy {
+ status = "okay";
+
+ vdda-phy-supply = <&vdda_ufs1_core>;
+ vdda-pll-supply = <&vdda_ufs1_1p2>;
+};
+
+&usb_1 {
+ status = "okay";
+};
+
+&usb_1_dwc3 {
+ dr_mode = "host";
+};
+
+&usb_1_hsphy {
+ status = "okay";
+
+ vdd-supply = <&vdda_usb1_ss_core>;
+ vdda-pll-supply = <&vdda_qusb_hs0_1p8>;
+ vdda-phy-dpdm-supply = <&vdda_qusb_hs0_3p1>;
+
+ qcom,imp-res-offset-value = <8>;
+ qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_21_6_MA>;
+ qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_5_PERCENT>;
+ qcom,preemphasis-width = <QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT>;
+};
+
+&usb_1_qmpphy {
+ status = "okay";
+
+ vdda-phy-supply = <&vdda_usb1_ss_1p2>;
+ vdda-pll-supply = <&vdda_usb1_ss_core>;
+};
+
+&usb_2 {
+ status = "okay";
+};
+
+&usb_2_dwc3 {
+ dr_mode = "host";
+};
+
+&usb_2_hsphy {
+ status = "okay";
+
+ vdd-supply = <&vdda_usb2_ss_core>;
+ vdda-pll-supply = <&vdda_qusb_hs0_1p8>;
+ vdda-phy-dpdm-supply = <&vdda_qusb_hs0_3p1>;
+
+ qcom,imp-res-offset-value = <8>;
+ qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_22_8_MA>;
+};
+
+&usb_2_qmpphy {
+ status = "okay";
+
+ vdda-phy-supply = <&vdda_usb2_ss_1p2>;
+ vdda-pll-supply = <&vdda_usb2_ss_core>;
+};
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* RE: [PATCH] ACPI: support for NXP i2c controller
From: Biwen Li @ 2019-09-05 15:04 UTC (permalink / raw)
To: 'andy.shevchenko@gmail.com', 'rafael@kernel.org',
Leo Li, Meenakshi Aggarwal, Udit Kumar,
'wsa@the-dreams.de', 'rjw@rjwysocki.net'
Cc: 's.hauer@pengutronix.de',
'linux-kernel@vger.kernel.org',
'linux-acpi@vger.kernel.org',
'linux-i2c@vger.kernel.org', Chuanhua Han,
'shawnguo@kernel.org',
'linux-arm-kernel@lists.infradead.org'
In-Reply-To: <DB7PR04MB4490E737C00CF9CA173AB2898FBB0@DB7PR04MB4490.eurprd04.prod.outlook.com>
>
> > Hi,
> >
> > On 02.09.19 23:16, Andy Shevchenko wrote:
> > > On Mon, Sep 2, 2019 at 11:58 PM Rafael J. Wysocki
> > > <rafael@kernel.org>
> > wrote:
> > >>
> > >> On Thu, Jul 11, 2019 at 12:35 PM Chuanhua Han
> > >> <chuanhua.han@nxp.com>
> > wrote:
> > >>>
> > >>> Enable NXP i2c controller to boot with ACPI
> > >>>
> > >>> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > >>> Signed-off-by: Udit Kumar <udit.kumar@nxp.com>
> > >>> Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
> > >>
> > >> Wolfram, any objections to this from the i2c side?
> > >
> > > May I propose amendment(s)?
> > >
> > >>> @@ -44,6 +44,7 @@
> > >>> #include <linux/pm_runtime.h>
> > >>> #include <linux/sched.h>
> > >>> #include <linux/slab.h>
> > >
> > >>> +#include <linux/acpi.h>
> > >
> > > If it's kept in order, better to go with it. (Yes, it is as I have
> > > checked) However, property.h should be included instead, see below.
Ok, got it. I will fix it in v2.
> > >
> > >>> const struct of_device_id *of_id =
> > of_match_device(i2c_imx_dt_ids,
> > >>>
> > >>> &pdev->dev);
> > >>> + const struct acpi_device_id *acpi_id =
> > >>> + acpi_match_device(i2c_imx_acpi_ids,
> > >>> + &pdev->dev);
> > >
> > >
> > >>> if (of_id)
> > >>> i2c_imx->hwdata = of_id->data;
> > >>> + else if (acpi_id)
> > >>> + i2c_imx->hwdata = (struct imx_i2c_hwdata *)
> > >>> + acpi_id->driver_data;
> > >
> > >
> > > The above altogher may be replaced with
> > >
> > > const struct imx_i2c_hwdata *match;
> > > ...
> > > match = device_get_match_data(&pdev->dev);
> > > if (match)
> > > i2c_imx->hwdata = match;
> > > else
> > > ...
> >
Ok, I will correct it in v2.
> > Instead of "may be replaced", I would say: it should be replaced :)
> >
> > >>> + .acpi_match_table = ACPI_PTR(i2c_imx_acpi_ids),
> > >
> > > Since there is no #ifdef guard no need to use ACPI_PTR().
> > >
> >
> > What iMX/(other NXP?) SoCs are with ACPI support? Where I can get
> > one? I would like to know more about it.
> - Nxp has variety Socs, include i.MX, Layerscape, etc.
> - You can get one from here
> https://www.nxp.com/design/qoriq-developer-resources/qoriq-lx2160a-develo
> pment-board:LX2160A-RDB
>
> >
> > Kind regards,
> > Oleksij Rempel
> >
> > --
> > Pengutronix e.K. |
> > |
> > Industrial Linux Solutions |
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.p
> > e
> ngutronix.de%2F&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%
> >
> 7C640eb015a91f4959d3b508d7303168fb%7C686ea1d3bc2b4c6fa92cd99c5c
> >
> 301635%7C0%7C0%7C637030861076879938&sdata=sPWtkVtHHDvoRR
> > ZmWJuipCO%2BEwG%2BcupgZvcIV1%2BrlEY%3D&reserved=0 |
> Peiner Str.
> > 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> > |
> > Amtsgericht Hildesheim, HRA 2686 | Fax:
> > +49-5121-206917-5555 |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1] watchdog: imx_sc: this patch just fixes whitespaces
From: Guenter Roeck @ 2019-09-05 15:06 UTC (permalink / raw)
To: Oliver Graute
Cc: Mark Rutland, Aisheng Dong, Ulf Hansson,
linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org,
Shawn Guo, Sascha Hauer, linux-kernel@vger.kernel.org,
Daniel Baluta, Rob Herring, NXP Linux Team,
Pengutronix Kernel Team, Wim Van Sebroeck, Fabio Estevam,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190905143525.GE23537@optiplex>
On Thu, Sep 05, 2019 at 02:35:37PM +0000, Oliver Graute wrote:
> On 05/09/19, Guenter Roeck wrote:
> > On 9/5/19 12:44 AM, Oliver Graute wrote:
> > > Fix only whitespace errors in imx_sc_wdt_probe()
> > >
> > > Signed-off-by: Oliver Graute <oliver.graute@kococonnector.com>
> >
> > This patch no longer applies due to commit "watchdog: imx_sc: Remove
> > unnecessary error log".
> >
>
> ok I'll rebase patch against linux-staging/watchdog-next
>
I should have said "no longer necessary". The mentioned patch
makes the same change (and drops the error message).
Guenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] watchdog: imx_sc: this patch just fixes whitespaces
From: Guenter Roeck @ 2019-09-05 15:08 UTC (permalink / raw)
To: Oliver Graute
Cc: Aisheng Dong, Ulf Hansson, linux-watchdog@vger.kernel.org,
Shawn Guo, Sascha Hauer, linux-kernel@vger.kernel.org,
oliver.graute@gmail.com, NXP Linux Team, Pengutronix Kernel Team,
Wim Van Sebroeck, Fabio Estevam,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190905143644.20952-1-oliver.graute@kococonnector.com>
On Thu, Sep 05, 2019 at 02:36:49PM +0000, Oliver Graute wrote:
> Fix only whitespace errors in imx_sc_wdt_probe()
>
> Signed-off-by: Oliver Graute <oliver.graute@kococonnector.com>
Ah, there are indeed extra spaces in these lines.
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/imx_sc_wdt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/imx_sc_wdt.c b/drivers/watchdog/imx_sc_wdt.c
> index 9260475439eb..7ea5cf54e94a 100644
> --- a/drivers/watchdog/imx_sc_wdt.c
> +++ b/drivers/watchdog/imx_sc_wdt.c
> @@ -176,8 +176,8 @@ static int imx_sc_wdt_probe(struct platform_device *pdev)
>
> ret = devm_watchdog_register_device(dev, wdog);
> if (ret)
> - return ret;
> -
> + return ret;
> +
> ret = imx_scu_irq_group_enable(SC_IRQ_GROUP_WDOG,
> SC_IRQ_WDOG,
> true);
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: next/master boot: 310 boots: 11 failed, 292 passed with 6 offline, 1 untried/unknown (next-20190904)
From: Mark Brown @ 2019-09-05 15:39 UTC (permalink / raw)
To: Kevin Hilman, Christoph Hellwig
Cc: Steven Liu, kernel-build-reports, Catalin Marinas, linux-mediatek,
Matthias Brugger, Will Deacon, linux-arm-kernel
In-Reply-To: <7hzhjjsime.fsf@baylibre.com>
[-- Attachment #1.1: Type: text/plain, Size: 5671 bytes --]
On Wed, Sep 04, 2019 at 05:15:53PM -0700, Kevin Hilman wrote:
> [ + Steven Liu who donated this board to my kernelCI lab ]
Also adding Christoph since this was bisected to his commit and Catalin
and Will since this was an architecture change.
> Mark Brown <broonie@kernel.org> writes:
> > On Wed, Sep 04, 2019 at 12:05:57PM -0700, kernelci.org bot wrote:
> > Since 30th August -next fails to boot with no kernel output on
> > mt7622-rfb1:
...
> > There's logging from ATF so it looks like we try to boot the kernel:
> >
> > Starting kernel ...
> >
> > [ATF][ 36.199793]save kernel info
> > [ATF][ 36.202824]Kernel_EL2
> > [ATF][ 36.205580]Kernel is 64Bit
> > [ATF][ 36.208768]pc=0x40080000, r0=0x5cf48000, r1=0x0
> > INFO: BL3-1: Preparing for EL3 exit to normal world, Kernel
> > INFO: BL3-1: Next image address = 0x40080000
> > INFO: BL3-1: Next image spsr = 0x3c9
> > [ATF][ 36.227037]el3_exit
> > but no output. More details including full logs at:
> >
> > https://kernelci.org/boot/id/5d6fe70059b514164ef1224d/
> > https://kernelci.org/boot/id/5d6fe6e259b514164ef12243/
>
> Bisected down to this commit[1], full bisect log here[2]. It didn't
> revert cleanly on top of next-20190904, so I didn't get any further.
> [1]
> 419e2f1838819e954071dfa1d1f820ab3386ada1 is the first bad commit
> commit 419e2f1838819e954071dfa1d1f820ab3386ada1
> Author: Christoph Hellwig <hch@lst.de>
> Date: Mon Aug 26 09:03:44 2019 +0200
>
> dma-mapping: remove arch_dma_mmap_pgprot
>
> arch_dma_mmap_pgprot is used for two things:
>
> 1) to override the "normal" uncached page attributes for mapping
> memory coherent to devices that can't snoop the CPU caches
> 2) to provide the special DMA_ATTR_WRITE_COMBINE semantics on older
> arm systems and some mips platforms
>
> Replace one with the pgprot_dmacoherent macro that is already provided
> by arm and much simpler to use, and lift the DMA_ATTR_WRITE_COMBINE
> handling to common code with an explicit arch opt-in.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> # m68k
> Acked-by: Paul Burton <paul.burton@mips.com> # mips
>
> arch/arm/Kconfig | 2 +-
> arch/arm/mm/dma-mapping.c | 6 ------
> arch/arm64/Kconfig | 1 -
> arch/arm64/include/asm/pgtable.h | 4 ++++
> arch/arm64/mm/dma-mapping.c | 6 ------
> arch/m68k/Kconfig | 1 -
> arch/m68k/include/asm/pgtable_mm.h | 3 +++
> arch/m68k/kernel/dma.c | 3 +--
> arch/mips/Kconfig | 2 +-
> arch/mips/mm/dma-noncoherent.c | 8 --------
> include/linux/dma-noncoherent.h | 13 +++++++++++--
> kernel/dma/Kconfig | 12 +++++++++---
> kernel/dma/mapping.c | 8 +++++---
> 13 files changed, 35 insertions(+), 34 deletions(-)
> bisect run success
>
> [2]
> $ git bisect log
> git bisect start
> # bad: [35394d031b710e832849fca60d0f53b513f0c390] Add linux-next specific files for 20190904
> git bisect bad 35394d031b710e832849fca60d0f53b513f0c390
> # good: [089cf7f6ecb266b6a4164919a2e69bd2f938374a] Linux 5.3-rc7
> git bisect good 089cf7f6ecb266b6a4164919a2e69bd2f938374a
> # bad: [493424779be81b03fb4aca89cc05ba95e9fc0c31] Merge remote-tracking branch 'crypto/master'
> git bisect bad 493424779be81b03fb4aca89cc05ba95e9fc0c31
> # bad: [58a02f90fddfdc9e4dfbd6579ff788ffdc22afe4] Merge remote-tracking branch 'hid/for-next'
> git bisect bad 58a02f90fddfdc9e4dfbd6579ff788ffdc22afe4
> # bad: [27c3f6e1d84b47def9060fd481be92838d819a9b] Merge remote-tracking branch 'csky/linux-next'
> git bisect bad 27c3f6e1d84b47def9060fd481be92838d819a9b
> # good: [f119c164735d85f2a41d14503cb9933d219c539e] arm-soc: document merges
> git bisect good f119c164735d85f2a41d14503cb9933d219c539e
> # bad: [e87b432d6c45697defc03eb69261661060c85245] Merge remote-tracking branch 'actions/for-next'
> git bisect bad e87b432d6c45697defc03eb69261661060c85245
> # good: [1ab97157f64dadb44d029096c6a92305d6631ab2] Merge remote-tracking branch 'kbuild/for-next'
> git bisect good 1ab97157f64dadb44d029096c6a92305d6631ab2
> # good: [ac12cf85d682a2c1948210c65f7fb21ef01dd9f6] Merge branches 'for-next/52-bit-kva', 'for-next/cpu-topology', 'for-next/error-injection', 'for-next/perf', 'for-next/psci-cpuidle', 'for-next/rng', 'for-next/smpboot', 'for-next/tbi' and 'for-next/tlbi' into for-next/core
> git bisect good ac12cf85d682a2c1948210c65f7fb21ef01dd9f6
> # bad: [4934d349f6e5afc9345a44acb0daa3066594088a] Merge remote-tracking branch 'asm-generic/master'
> git bisect bad 4934d349f6e5afc9345a44acb0daa3066594088a
> # good: [5251a1c90f7f4e458dc3154920e09624311f54b6] Merge remote-tracking branch 'compiler-attributes/compiler-attributes'
> git bisect good 5251a1c90f7f4e458dc3154920e09624311f54b6
> # skip: [38c38cb73223218f6eedf485280917af1f8a0af2] mmc: queue: use bigger segments if DMA MAP layer can merge the segments
> git bisect skip 38c38cb73223218f6eedf485280917af1f8a0af2
> # bad: [419e2f1838819e954071dfa1d1f820ab3386ada1] dma-mapping: remove arch_dma_mmap_pgprot
> git bisect bad 419e2f1838819e954071dfa1d1f820ab3386ada1
> # good: [5518ea1ad2c0c7f38d067f621d9349e6a11c8879] unicore32: remove the unused pgprot_dmacoherent define
> git bisect good 5518ea1ad2c0c7f38d067f621d9349e6a11c8879
> # good: [b898e50f9f49f7d90f3bca94ac046145072034a2] arm-nommu: remove the unused pgprot_dmacoherent define
> git bisect good b898e50f9f49f7d90f3bca94ac046145072034a2
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: next/master boot: 310 boots: 11 failed, 292 passed with 6 offline, 1 untried/unknown (next-20190904)
From: Christoph Hellwig @ 2019-09-05 15:40 UTC (permalink / raw)
To: Mark Brown
Cc: Steven Liu, kernel-build-reports, Kevin Hilman, linux-mediatek,
Catalin Marinas, Matthias Brugger, Will Deacon, Christoph Hellwig,
linux-arm-kernel
In-Reply-To: <20190905153938.GB4053@sirena.co.uk>
On Thu, Sep 05, 2019 at 04:39:38PM +0100, Mark Brown wrote:
> On Wed, Sep 04, 2019 at 05:15:53PM -0700, Kevin Hilman wrote:
> > [ + Steven Liu who donated this board to my kernelCI lab ]
>
> Also adding Christoph since this was bisected to his commit and Catalin
> and Will since this was an architecture change.
Given Will in CC the problem is on arm64?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: next/master boot: 310 boots: 11 failed, 292 passed with 6 offline, 1 untried/unknown (next-20190904)
From: Mark Brown @ 2019-09-05 15:43 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Steven Liu, kernel-build-reports, Kevin Hilman, linux-mediatek,
Catalin Marinas, Matthias Brugger, Will Deacon, linux-arm-kernel
In-Reply-To: <20190905154053.GA21701@lst.de>
[-- Attachment #1.1: Type: text/plain, Size: 590 bytes --]
On Thu, Sep 05, 2019 at 05:40:53PM +0200, Christoph Hellwig wrote:
> On Thu, Sep 05, 2019 at 04:39:38PM +0100, Mark Brown wrote:
> > On Wed, Sep 04, 2019 at 05:15:53PM -0700, Kevin Hilman wrote:
> > > [ + Steven Liu who donated this board to my kernelCI lab ]
> > Also adding Christoph since this was bisected to his commit and Catalin
> > and Will since this was an architecture change.
> Given Will in CC the problem is on arm64?
Yes, this is an arm64 system. It seems to be the only one in KernelCI
that's been affected so it's not like the entire architecture exploded
or anything.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL] ARM: aspeed: devicetree changes for 5.4, round two
From: Arnd Bergmann @ 2019-09-05 15:51 UTC (permalink / raw)
To: Joel Stanley; +Cc: Andrew Jeffery, SoC Team, arm, Linux ARM, linux-aspeed
In-Reply-To: <CACPK8Xdp4gVuetmiu2bRTTH6oHhRrC7FELHWKVB2ZGSbPbH7HQ@mail.gmail.com>
On Thu, Sep 5, 2019 at 2:43 AM Joel Stanley <joel@jms.id.au> wrote:
>
> Hello ARM maintainers,
>
> Here are some late fixes I collected for the ASPEED boards.
>
> I've thrown the commits on top of the ones in the first pull request, which
> you've merged. I've not sent a second pull before so if that's not the done
> thing then let me know what you prefer.
This is the best way to do it.
> ASPEED device tree updates for 5.4, second round
>
> - Alternate flash support for Vesnin
> - Minor cleanups and fixes
Pulled into arm/dt, thanks!
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL 1/2] Rockchip DT32 changes for 5.4 - round2
From: Arnd Bergmann @ 2019-09-05 15:53 UTC (permalink / raw)
To: Heiko Stuebner
Cc: open list:ARM/Rockchip SoC support, SoC Team, arm-soc, Linux ARM
In-Reply-To: <1718334.9BoTfW0Ujv@phil>
On Thu, Sep 5, 2019 at 2:18 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> Hi (arm-)soc people,
>
> please find below and in the reply a small second round of Rockchip changes
> targetted at 5.4.
>
> Mainly only small cleanups and have been in linux-next for a while now,
> I just didn't get to pull-requests due to travel and jetlag.
> Linus wrote that he wants to do an -rc8 but if the stuff below is too late
> I can also easily move it to 5.5.
This looks simple enough, so pulled into arm/dt, thanks!
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL 2/2] Rockchip DT64 changes for 5.4 - round2
From: Arnd Bergmann @ 2019-09-05 15:54 UTC (permalink / raw)
To: Heiko Stuebner
Cc: open list:ARM/Rockchip SoC support, SoC Team, arm-soc, Linux ARM
In-Reply-To: <43564855.cWDBQSyQMS@phil>
On Thu, Sep 5, 2019 at 2:18 PM Heiko Stuebner <heiko@sntech.de> wrote:
> RK3328 mmc clockrate limit and addition of vpu node as well
> as a regulator fix for the rk3328-rock64.
Pulled into arm/dt, thanks!
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [V3, 2/2] media: i2c: Add Omnivision OV02A10 camera sensor driver
From: Dongchun Zhu @ 2019-09-05 15:54 UTC (permalink / raw)
To: Tomasz Figa
Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, shengnan.wang,
louis.kuo, sj.huang, robh+dt, linux-mediatek, sakari.ailus,
matthias.bgg, bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <20190821103038.GA148543@chromium.org>
Hi Tomasz,
Thanks very much for your careful review.
Reply to the comments see belows.
Any missing please let me know.
On Wed, 2019-08-21 at 19:30 +0900, Tomasz Figa wrote:
> Hi Dongchun,
>
> On Mon, Aug 19, 2019 at 11:43:31AM +0800, dongchun.zhu@mediatek.com wrote:
> > From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> >
> > This patch adds a V4L2 sub-device driver for OV02A10 image sensor.
> > The OV02A10 is a 1/5" CMOS sensor from Omnivision,
> > which supports output format: 10-bit Raw.
> > The OV02A10 has a single MIPI lane interface and use the I2C bus
> > for control and the CSI-2 bus for data.
> >
> > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > ---
> > MAINTAINERS | 1 +
> > drivers/media/i2c/Kconfig | 11 +
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/ov02a10.c | 1018 +++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 1031 insertions(+)
> > create mode 100644 drivers/media/i2c/ov02a10.c
> >
>
> Thanks for the patch! Please see my comments inline.
>
> [snip]
>
> > +#define CHIP_ID 0x2509
> > +#define OV02A10_REG_CHIP_ID_H 0x02
> > +#define OV02A10_REG_CHIP_ID_L 0x03
> > +#define OV02A10_ID(_msb, _lsb) ((_msb) << 8 | (_lsb))
> > +
> > +/* Bit[1] vertical upside down */
> > +/* Bit[0] horizontal mirror */
> > +#define REG_MIRROR_FLIP_CONTROL 0x3f
> > +
> > +/* Orientation */
> > +#define REG_CONFIG_MIRROR_FLIP 0x03
> > +
> > +#define REG_PAGE_SWITCH 0xfd
> > +#define REG_GLOBAL_EFFECTIVE 0x01
> > +#define REG_ENABLE BIT(0)
> > +
> > +#define REG_SC_CTRL_MODE 0xac
> > +#define SC_CTRL_MODE_STANDBY 0x00
> > +#define SC_CTRL_MODE_STREAMING 0x01
> > +
> > +#define OV02A10_REG_EXPOSURE_H 0x03
> > +#define OV02A10_REG_EXPOSURE_L 0x04
> > +#define OV02A10_EXPOSURE_MIN 4
> > +#define OV02A10_EXPOSURE_STEP 1
> > +
> > +#define OV02A10_REG_VTS_H 0x05
> > +#define OV02A10_REG_VTS_L 0x06
> > +#define OV02A10_VTS_MAX 0x209f
> > +#define OV02A10_VTS_MIN 0x04cf
> > +#define OV02A10_BASIC_LINE 1224
> > +
> > +#define OV02A10_REG_GAIN 0x24
> > +#define OV02A10_GAIN_MIN 0x10
> > +#define OV02A10_GAIN_MAX 0xf8
> > +#define OV02A10_GAIN_STEP 0x01
> > +#define OV02A10_GAIN_DEFAULT 0x40
> > +
> > +#define REG_NULL 0xff
> > +
> > +#define OV02A10_LANES 1
> > +#define OV02A10_BITS_PER_SAMPLE 10
> > +
>
> I think there is something wrong with the indentation in the code above.
> Please use tabs wherever possible.
>
Sorry for the typo.
Fixed in next release.
> [snip]
>
> > +
> > +#define to_ov02a10(sd) container_of(sd, struct ov02a10, subdev)
>
> Please use a static inline function for added compile-time type checks.
>
Fixed in next release.
> > +
> > +static inline void msleep_range(unsigned int delay_base)
> > +{
> > + usleep_range(delay_base * 1000, delay_base * 1000 + 500);
> > +}
>
> Why not just use msleep()?
>
Generally for shorter sleep(e.g.less than 10ms), usleep_range is
preferred. We would use msleep for bigger sleep.
> > +
> > +/* MIPI color bar enable output */
> > +static const struct regval ov02a10_test_pattern_enable_regs[] = {
> > + {0xfd, 0x01},
> > + {0x0d, 0x00},
> > + {0xb6, 0x01},
> > + {0x01, 0x01},
> > + {0xfd, 0x01},
> > + {0xac, 0x01},
> > + {REG_NULL, 0x00}
> > +};
> > +
> > +/* MIPI color bar disable output */
> > +static const struct regval ov02a10_test_pattern_disable_regs[] = {
> > + {0xfd, 0x01},
> > + {0x0d, 0x00},
> > + {0xb6, 0x00},
> > + {0x01, 0x01},
> > + {0xfd, 0x01},
> > + {0xac, 0x01},
> > + {REG_NULL, 0x00}
> > +};
>
> Hmm, only the register 0xb6 seems to here. Could we just set it directly,
> without these arrays?
>
R0x0d is another color bar control register.
But R0x01 and R0xac both are essential, which could make color bar
register writing behavior global effective.
> > +
> > +/*
> > + * xvclk 24Mhz
>
> This seems to assume 24MHz, but the driver allows a range in probe. Is that
> correct?
>
That's OK.
As the input clock for sensor, the frequency of Master Clock(xvclk) is
regularly 24MHz. But actually the sensor xvclk could allow some little
tolerance. For instance, 1%.
> [snip]]
>
> > +/* Write a register */
> > +static int ov02a10_write_reg(struct ov02a10 *ov02a10, u8 addr, u8 val)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + u8 buf[2] = {addr, val};
> > + int ret;
> > +
> > + ret = i2c_master_send(client, buf, 2);
> > +
> > + if (ret != 2) {
> > + dev_err(&client->dev, "%s: error: reg=%x, val=%x\n",
> > + __func__, addr, val);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
>
> Could this be replaced with i2c_smbus_write_byte_data()?
>
We would have a try using this generic API.
> > +
> > +static int ov02a10_write_array(struct ov02a10 *ov02a10,
> > + const struct regval *regs)
> > +{
> > + u32 i;
> > + int ret;
> > +
> > + for (i = 0; regs[i].addr != REG_NULL; i++) {
> > + ret = ov02a10_write_reg(ov02a10, regs[i].addr, regs[i].val);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/* Read a register */
> > +static int ov02a10_read_reg(struct ov02a10 *ov02a10, u8 reg, u8 *val)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + u8 data = reg;
> > + struct i2c_msg msg = {
> > + .addr = client->addr,
> > + .flags = 0,
> > + .len = 1,
> > + .buf = &data,
> > + };
> > + int ret;
> > +
> > + ret = i2c_transfer(client->adapter, &msg, 1);
> > + if (ret < 0)
> > + goto err_wr;
> > +
> > + msg.flags = I2C_M_RD;
> > + ret = i2c_transfer(client->adapter, &msg, 1);
> > + if (ret < 0)
> > + goto err_rd;
>
> Could we just have 2 messages in an array and just call i2c_transfer() once
> for both write and read?
>
> Or actually it sounds like the i2c_smbus_read_byte_data() helper could work
> here.
>
Understood. We would have a try, too.
> > +
> > + *val = data;
> > + return 0;
> > +
> > +err_rd:
> > + dev_err(&client->dev, "i2c_transfer --I2C_M_RD failed\n");
> > +err_wr:
> > + dev_err(&client->dev, "read error: reg=0x%02x: %d\n", reg, ret);
> > + return ret;
> > +}
> > +
> > +static void ov02a10_fill_fmt(const struct ov02a10_mode *mode,
> > + struct v4l2_mbus_framefmt *fmt)
> > +{
> > + fmt->width = mode->width;
> > + fmt->height = mode->height;
> > + fmt->field = V4L2_FIELD_NONE;
> > +}
> > +
> > +static int ov02a10_set_fmt(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *fmt)
> > +{
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > + struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
> > +
> > + mutex_lock(&ov02a10->mutex);
> > +
> > + if (ov02a10->streaming) {
> > + mutex_unlock(&ov02a10->mutex);
> > + return -EBUSY;
> > + }
> > +
> > + /* Only one sensor mode supported */
> > + mbus_fmt->code = ov02a10->fmt.code;
> > + ov02a10_fill_fmt(ov02a10->cur_mode, mbus_fmt);
> > + ov02a10->fmt = fmt->format;
> > +
> > + mutex_unlock(&ov02a10->mutex);
> > +
> > + return 0;
> > +}
> > +
> > +static int ov02a10_get_fmt(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *fmt)
> > +{
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > + struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
> > +
> > + mutex_lock(&ov02a10->mutex);
> > +
> > + fmt->format = ov02a10->fmt;
> > + mbus_fmt->code = ov02a10->fmt.code;
> > + ov02a10_fill_fmt(ov02a10->cur_mode, mbus_fmt);
> > +
> > + mutex_unlock(&ov02a10->mutex);
> > +
> > + return 0;
> > +}
> > +
> > +static int ov02a10_enum_mbus_code(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_mbus_code_enum *code)
> > +{
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +
> > + if (code->index >= ARRAY_SIZE(supported_modes) || !(code->index))
>
> Hmm, ARRAY_SIZE(supported_modes) is 1 and we don't allow code->index to be
> 0 either. Is there a code->index value that wouldn't return an error here?
>
Fixed in next release.
> > + return -EINVAL;
> > +
> > + code->code = ov02a10->fmt.code;
> > +
> > + return 0;
> > +}
> > +
> > +static int ov02a10_enum_frame_sizes(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_frame_size_enum *fse)
> > +{
> > + if (fse->index >= ARRAY_SIZE(supported_modes) || !(fse->index))
>
> Same here.
>
Fixed in next release.
> > + return -EINVAL;
> > +
> > + fse->min_width = supported_modes[fse->index].width;
> > + fse->max_width = supported_modes[fse->index].width;
> > + fse->max_height = supported_modes[fse->index].height;
> > + fse->min_height = supported_modes[fse->index].height;
> > +
> > + return 0;
> > +}
> > +
> > +static int __ov02a10_power_on(struct ov02a10 *ov02a10)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + struct device *dev = &client->dev;
> > + int ret;
> > +
> > + ret = clk_prepare_enable(ov02a10->xvclk);
> > + if (ret < 0) {
> > + dev_err(dev, "Failed to enable xvclk\n");
> > + return ret;
> > + }
>
> Is it really correct to enable the clock before the regulators?
>
> According to the datasheet, it should be:
> - PD pin HIGH,
> - nRST pin LOW,
> - DVDDIO and AVDD28 power up and stabilize,
> - clock enabled,
> - min 5 ms delay,
> - PD pin LOW,
> - min 4 ms delay,
> - nRST pin HIGH,
> - min 5 ms delay,
> - I2C interface ready.
>
xvclk as the clock source of sensor, we could enable it initially to let
PLL ready. In fact, the power up sequence mainly focus on PD, RST, and
IOVDD/AVDD28/DVDD pins.
> > +
> > + /* Note: set 0 is high, set 1 is low */
>
> Why is that? If there is some inverter on the way that should be handled
> outside of this driver. (GPIO DT bindings have flags for this purpose.
>
> If the pins are nRESET and nPOWERDOWN in the hardware datasheet, we should
> call them like this in the driver too (+/- the lowercase and underscore
> convention).
>
> According to the datasheet, the reset pin is called RST and inverted, so we should
> call it n_rst, but the powerdown signal, called PD, is not inverted, so pd
> would be the right name.
>
Sorry. Could make some more explanations about this?
> > + gpiod_set_value_cansleep(ov02a10->reset_gpio, 1);
> > + gpiod_set_value_cansleep(ov02a10->powerdown_gpio, 0);
> > +
> > + ret = regulator_bulk_enable(OV02A10_NUM_SUPPLIES, ov02a10->supplies);
> > + if (ret < 0) {
> > + dev_err(dev, "Failed to enable regulators\n");
> > + goto disable_clk;
> > + }
> > + msleep_range(7);
> > +
> > + gpiod_set_value_cansleep(ov02a10->powerdown_gpio, 1);
> > + msleep_range(10);
> > +
> > + gpiod_set_value_cansleep(ov02a10->reset_gpio, 0);
> > + msleep_range(10);
> > +
> > + return 0;
> > +
> > +disable_clk:
> > + clk_disable_unprepare(ov02a10->xvclk);
> > +
> > + return ret;
> > +}
> > +
> > +static void __ov02a10_power_off(struct ov02a10 *ov02a10)
> > +{
> > + clk_disable_unprepare(ov02a10->xvclk);
> > + gpiod_set_value_cansleep(ov02a10->reset_gpio, 1);
> > + gpiod_set_value_cansleep(ov02a10->powerdown_gpio, 1);
> > + regulator_bulk_disable(OV02A10_NUM_SUPPLIES, ov02a10->supplies);
>
> This also doesn't seem to match my datasheet. The sequence there is:
> - nRST goes LOW,
> - clock stops,
> - PD goes HIGH,
> - regulators are powerd down.
>
Got it.
This would be fixed in next release.
> > +}
> > +
> > +static int __ov02a10_start_stream(struct ov02a10 *ov02a10)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + int ret;
> > +
> > + /* Apply default values of current mode */
> > + ret = ov02a10_write_array(ov02a10, ov02a10->cur_mode->reg_list);
> > + if (ret)
> > + return ret;
> > +
> > + /* Apply customized values from user */
> > + ret = __v4l2_ctrl_handler_setup(ov02a10->subdev.ctrl_handler);
> > + if (ret)
> > + return ret;
> > +
> > + /* Set orientation to 180 degree */
> > + if (ov02a10->upside_down) {
> > + ret = ov02a10_write_reg(ov02a10, REG_MIRROR_FLIP_CONTROL,
> > + REG_CONFIG_MIRROR_FLIP);
> > + if (ret) {
> > + dev_err(&client->dev, "%s failed to set orientation\n",
> > + __func__);
> > + return ret;
> > + }
> > + ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> > + REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + /* Set stream on register */
> > + return ov02a10_write_reg(ov02a10,
> > + REG_SC_CTRL_MODE, SC_CTRL_MODE_STREAMING);
> > +}
> > +
> > +static int __ov02a10_stop_stream(struct ov02a10 *ov02a10)
> > +{
> > + return ov02a10_write_reg(ov02a10,
> > + REG_SC_CTRL_MODE, SC_CTRL_MODE_STANDBY);
> > +}
> > +
> > +static int ov02a10_entity_init_cfg(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_pad_config *cfg)
> > +{
> > + struct v4l2_subdev_format fmt = { 0 };
> > +
> > + fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> > + fmt.format.width = 1600;
> > + fmt.format.height = 1200;
>
> Where do these values come from? Should we have some macros for them?
>
Fixed in next release.
> > +
> > + ov02a10_set_fmt(subdev, cfg, &fmt);
> > +
> > + return 0;
> > +}
> > +
> > +static int ov02a10_s_stream(struct v4l2_subdev *sd, int on)
> > +{
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + int ret = 0;
> > +
> > + mutex_lock(&ov02a10->mutex);
> > +
> > + if (ov02a10->streaming == on)
> > + goto unlock_and_return;
> > +
> > + if (on) {
> > + ret = pm_runtime_get_sync(&client->dev);
> > + if (ret < 0) {
> > + pm_runtime_put_noidle(&client->dev);
> > + goto unlock_and_return;
> > + }
> > +
> > + ret = __ov02a10_start_stream(ov02a10);
> > + if (ret) {
> > + __ov02a10_stop_stream(ov02a10);
> > + ov02a10->streaming = !on;
> > + goto err_rpm_put;
> > + }
> > + } else {
> > + __ov02a10_stop_stream(ov02a10);
> > + pm_runtime_put(&client->dev);
> > + }
> > +
> > + ov02a10->streaming = on;
> > + mutex_unlock(&ov02a10->mutex);
> > +
> > + return ret;
> > +
> > +err_rpm_put:
> > + pm_runtime_put(&client->dev);
> > +unlock_and_return:
> > + mutex_unlock(&ov02a10->mutex);
> > +
> > + return ret;
> > +}
> > +
> > +static int ov02a10_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > +{
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > + struct v4l2_mbus_framefmt *try_fmt = v4l2_subdev_get_try_format(sd,
> > + fh->pad,
> > + 0);
>
> Please separate the initialization from the declaration, because there
> isn't just enough space on this line anymore.
>
As Sakari pointed out, indeed this function is repeated executed since
init_cfg is implemented already. Thus we would omit the open callback in
next release.
> > +
> > + mutex_lock(&ov02a10->mutex);
> > + /* Initialize try_fmt */
> > + try_fmt->code = ov02a10->fmt.code;
> > + ov02a10_fill_fmt(&supported_modes[0], try_fmt);
> > +
> > + mutex_unlock(&ov02a10->mutex);
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused ov02a10_runtime_resume(struct device *dev)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +
> > + return __ov02a10_power_on(ov02a10);
> > +}
> > +
> > +static int __maybe_unused ov02a10_runtime_suspend(struct device *dev)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +
> > + __ov02a10_power_off(ov02a10);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct dev_pm_ops ov02a10_pm_ops = {
> > + SET_RUNTIME_PM_OPS(ov02a10_runtime_suspend,
> > + ov02a10_runtime_resume, NULL)
>
> Don't we need to implement and provide system PM ops too?
>
Fixed in next release.
> > +};
> > +
> > +static int ov02a10_set_test_pattern(struct ov02a10 *ov02a10, s32 value)
> > +{
> > + if (value)
> > + return ov02a10_write_array(ov02a10,
> > + ov02a10_test_pattern_enable_regs);
> > +
> > + return ov02a10_write_array(ov02a10,
> > + ov02a10_test_pattern_disable_regs);
> > +}
> > +
> > +static int ov02a10_set_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct ov02a10 *ov02a10 = container_of(ctrl->handler,
> > + struct ov02a10, ctrl_handler);
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + s64 max_expo;
> > + int ret;
> > +
> > + /* Propagate change of current control to all related controls */
> > + if (ctrl->id == V4L2_CID_VBLANK) {
> > + /* Update max exposure while meeting expected vblanking */
> > + max_expo = ov02a10->cur_mode->height + ctrl->val - 4;
> > + __v4l2_ctrl_modify_range(ov02a10->exposure,
> > + ov02a10->exposure->minimum, max_expo,
> > + ov02a10->exposure->step,
> > + ov02a10->exposure->default_value);
> > + }
> > +
> > + /* V4L2 controls values will be applied only when power is already up */
> > + if (!pm_runtime_get_if_in_use(&client->dev))
> > + return 0;
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_EXPOSURE:
> > + ret = ov02a10_write_reg(ov02a10, REG_PAGE_SWITCH, REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_EXPOSURE_H,
> > + ((ctrl->val >> 8) & 0xFF));
> > + if (!ret) {
> > + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_EXPOSURE_L,
> > + (ctrl->val & 0xFF));
> > + if (ret < 0)
> > + return ret;
> > + }
> > + ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> > + REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + break;
> > + case V4L2_CID_ANALOGUE_GAIN:
> > + ret = ov02a10_write_reg(ov02a10, REG_PAGE_SWITCH, REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_GAIN,
> > + (ctrl->val & 0xFF));
> > + if (ret < 0)
> > + return ret;
> > + ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> > + REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + break;
> > + case V4L2_CID_VBLANK:
> > + ret = ov02a10_write_reg(ov02a10, REG_PAGE_SWITCH, REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_VTS_H,
> > + (((ctrl->val +
> > + ov02a10->cur_mode->height -
> > + OV02A10_BASIC_LINE) >> 8)
> > + & 0xFF));
> > + if (!ret) {
> > + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_VTS_L,
> > + ((ctrl->val +
> > + ov02a10->cur_mode->height -
> > + OV02A10_BASIC_LINE) & 0xFF));
> > + if (ret < 0)
> > + return ret;
> > + }
> > + ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> > + REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + break;
> > + case V4L2_CID_TEST_PATTERN:
> > + ret = ov02a10_set_test_pattern(ov02a10, ctrl->val);
> > + if (ret < 0)
> > + return ret;
> > + break;
> > + default:
> > + dev_warn(&client->dev, "%s Unhandled id:0x%x, val:0x%x\n",
> > + __func__, ctrl->id, ctrl->val);
> > + ret = -EINVAL;
> > + break;
>
> We shouldn't need to handle this, as the control framework wouldn't call us
> with a control that we didn't register explicitly.
>
I see other sensors like ov5645 and ov8856 also have this error ctrl id
handle. Did you mean that "default:..." could be omitted?
> > + };
> > +
> > + pm_runtime_put(&client->dev);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct v4l2_subdev_video_ops ov02a10_video_ops = {
> > + .s_stream = ov02a10_s_stream,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops ov02a10_pad_ops = {
> > + .init_cfg = ov02a10_entity_init_cfg,
> > + .enum_mbus_code = ov02a10_enum_mbus_code,
> > + .enum_frame_size = ov02a10_enum_frame_sizes,
> > + .get_fmt = ov02a10_get_fmt,
> > + .set_fmt = ov02a10_set_fmt,
> > +};
> > +
> > +static const struct v4l2_subdev_ops ov02a10_subdev_ops = {
> > + .video = &ov02a10_video_ops,
> > + .pad = &ov02a10_pad_ops,
> > +};
> > +
> > +static const struct media_entity_operations ov02a10_subdev_entity_ops = {
> > + .link_validate = v4l2_subdev_link_validate,
> > +};
> > +
> > +static const struct v4l2_subdev_internal_ops ov02a10_internal_ops = {
> > + .open = ov02a10_open,
> > +};
> > +
> > +static const struct v4l2_ctrl_ops ov02a10_ctrl_ops = {
> > + .s_ctrl = ov02a10_set_ctrl,
> > +};
> > +
> > +static int ov02a10_initialize_controls(struct ov02a10 *ov02a10)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + const struct ov02a10_mode *mode;
> > + struct v4l2_ctrl_handler *handler;
> > + struct v4l2_ctrl *ctrl;
> > + u64 exposure_max;
> > + u32 pixel_rate, h_blank;
> > + int ret;
> > +
> > + handler = &ov02a10->ctrl_handler;
> > + mode = ov02a10->cur_mode;
> > + ret = v4l2_ctrl_handler_init(handler, 10);
>
> I can see 6 controls registered below.
>
I think handler num could be little more than the implemented num.
But as you suggested, this would be fixed in next release.
> > + if (ret)
> > + return ret;
> > + handler->lock = &ov02a10->mutex;
> > +
> > + ctrl = v4l2_ctrl_new_int_menu(handler, NULL, V4L2_CID_LINK_FREQ,
> > + 0, 0, link_freq_menu_items);
> > + if (ctrl)
> > + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > + pixel_rate = (link_freq_menu_items[0] * 2 * OV02A10_LANES) /
> > + OV02A10_BITS_PER_SAMPLE;
> > + v4l2_ctrl_new_std(handler, NULL, V4L2_CID_PIXEL_RATE,
> > + 0, pixel_rate, 1, pixel_rate);
> > +
> > + h_blank = mode->hts_def - mode->width;
> > + ov02a10->hblank = v4l2_ctrl_new_std(handler, NULL, V4L2_CID_HBLANK,
> > + h_blank, h_blank, 1, h_blank);
> > + if (ov02a10->hblank)
> > + ov02a10->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > + ov02a10->vblank = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> > + V4L2_CID_VBLANK, mode->vts_def -
> > + mode->height,
> > + OV02A10_VTS_MAX - mode->height, 1,
> > + mode->vts_def - mode->height);
> > +
> > + exposure_max = mode->vts_def - 4;
> > + ov02a10->exposure = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> > + V4L2_CID_EXPOSURE,
> > + OV02A10_EXPOSURE_MIN,
> > + exposure_max,
> > + OV02A10_EXPOSURE_STEP,
> > + mode->exp_def);
> > +
> > + ov02a10->anal_gain = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> > + V4L2_CID_ANALOGUE_GAIN,
> > + OV02A10_GAIN_MIN,
> > + OV02A10_GAIN_MAX,
> > + OV02A10_GAIN_STEP,
> > + OV02A10_GAIN_DEFAULT);
> > +
> > + ov02a10->test_pattern =
> > + v4l2_ctrl_new_std_menu_items(handler,
> > + &ov02a10_ctrl_ops,
> > + V4L2_CID_TEST_PATTERN,
> > + ARRAY_SIZE(ov02a10_test_pattern_menu) -
> > + 1, 0, 0, ov02a10_test_pattern_menu);
> > +
> > + if (handler->error) {
> > + ret = handler->error;
> > + dev_err(&client->dev,
> > + "Failed to init controls(%d)\n", ret);
> > + goto err_free_handler;
> > + }
> > +
> > + ov02a10->subdev.ctrl_handler = handler;
> > +
> > + return 0;
> > +
> > +err_free_handler:
> > + v4l2_ctrl_handler_free(handler);
> > +
> > + return ret;
> > +}
> > +
> > +static int ov02a10_check_sensor_id(struct ov02a10 *ov02a10)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + u16 id;
> > + u8 pid = 0;
> > + u8 ver = 0;
> > + int ret;
> > +
> > + /* Check sensor revision */
> > + ret = ov02a10_read_reg(ov02a10, OV02A10_REG_CHIP_ID_H, &pid);
> > + if (ret)
> > + return ret;
> > +
> > + ret = ov02a10_read_reg(ov02a10, OV02A10_REG_CHIP_ID_L, &ver);
> > + if (ret)
> > + return ret;
> > +
> > + id = OV02A10_ID(pid, ver);
> > + if (id != CHIP_ID) {
> > + dev_err(&client->dev, "Unexpected sensor id(%04x)\n", id);
> > + return ret;
> > + }
> > +
> > + dev_info(&client->dev, "Detected OV%04X sensor\n", id);
> > +
> > + return 0;
> > +}
> > +
> > +static int ov02a10_configure_regulators(struct ov02a10 *ov02a10)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + unsigned int i;
> > +
> > + for (i = 0; i < OV02A10_NUM_SUPPLIES; i++)
> > + ov02a10->supplies[i].supply = ov02a10_supply_names[i];
> > +
> > + return devm_regulator_bulk_get(&client->dev,
> > + OV02A10_NUM_SUPPLIES,
> > + ov02a10->supplies);
> > +}
>
> I think we can just have this directly inside probe.
>
Understood.
We would have a try.
> > +
> > +static int ov02a10_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct ov02a10 *ov02a10;
> > + u32 rotation;
> > + u32 xclk_freq;
> > + int ret;
> > +
> > + ov02a10 = devm_kzalloc(dev, sizeof(*ov02a10), GFP_KERNEL);
> > + if (!ov02a10)
> > + return -ENOMEM;
> > +
> > + v4l2_i2c_subdev_init(&ov02a10->subdev, client, &ov02a10_subdev_ops);
> > + ov02a10->fmt.code = MEDIA_BUS_FMT_SBGGR10_1X10;
> > +
> > + /* Optional indication of physical rotation of sensor */
> > + ret = fwnode_property_read_u32(dev_fwnode(dev), "rotation",
> > + &rotation);
> > + if (!ret) {
> > + switch (rotation) {
> > + case 180:
> > + ov02a10->upside_down = true;
> > + ov02a10->fmt.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> > + break;
> > + case 0:
> > + break;
> > + default:
> > + dev_warn(dev, "%u degrees rotation is not supported, ignoring...\n",
> > + rotation);
> > + }
> > + }
> > +
> > + /* Get system clock (xvclk) */
> > + ov02a10->xvclk = devm_clk_get(dev, "xvclk");
> > + if (IS_ERR(ov02a10->xvclk)) {
> > + dev_err(dev, "Failed to get xvclk\n");
> > + return -EINVAL;
> > + }
>
> Hmm, it's called eclk in my datasheet.
>
Does this really matter?
Should we follow the common naming, refer to other sensors?
> > +
> > + ret = of_property_read_u32(dev->of_node, "clock-frequency", &xclk_freq);
> > + if (ret) {
> > + dev_err(dev, "Failed to get xclk frequency\n");
> > + return ret;
> > + }
> > +
> > + /* External clock must be 24MHz, allow 1% tolerance */
> > + if (xclk_freq < 23760000 || xclk_freq > 24240000) {
>
> How do we support a range of frequencies? I don't see the driver calculate
> any register values based on this frequency. Are you sure that the register
> arrays don't assume one specific frequency?
>
In fact, clock-frequency defined in DT is always 24MHz, which is the
clock frequency that sensor requires to keep itself work normally.
> > + dev_err(dev, "external clock frequency %u is not supported\n",
> > + xclk_freq);
> > + return -EINVAL;
> > + }
> > + dev_dbg(dev, "external clock frequency %u\n", xclk_freq);
> > +
> > + ret = clk_set_rate(ov02a10->xvclk, xclk_freq);
> > + if (ret) {
> > + dev_err(dev, "Failed to set xvclk frequency (24MHz)\n");
> > + return ret;
> > + }
> > +
> > + ov02a10->powerdown_gpio = devm_gpiod_get(dev, "powerdown",
> > + GPIOD_OUT_LOW);
>
> Hmm, shouldn't this be HIGH? At least the datasheet has it so for the
> powered down state.
>
The gpio state for this API callback seems to be inverse.
> > + if (IS_ERR(ov02a10->powerdown_gpio)) {
> > + dev_err(dev, "Failed to get powerdown-gpios\n");
> > + return -EINVAL;
> > + }
> > +
> > + ov02a10->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
>
> Also LOW here, similarly to the above.
>
Ditto.
> > + if (IS_ERR(ov02a10->reset_gpio)) {
> > + dev_err(dev, "Failed to get reset-gpios\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = ov02a10_configure_regulators(ov02a10);
> > + if (ret) {
> > + dev_err(dev, "Failed to get power regulators\n");
> > + return ret;
> > + }
> > +
> > + mutex_init(&ov02a10->mutex);
> > + ov02a10->cur_mode = &supported_modes[0];
> > + ret = ov02a10_initialize_controls(ov02a10);
> > + if (ret) {
> > + dev_err(dev, "Failed to initialize controls\n");
> > + goto err_destroy_mutex;
> > + }
> > +
> > + ret = __ov02a10_power_on(ov02a10);
> > + if (ret)
> > + goto err_free_handler;
> > +
> > + ret = ov02a10_check_sensor_id(ov02a10);
> > + if (ret)
> > + goto err_power_off;
> > +
> > + ov02a10->subdev.internal_ops = &ov02a10_internal_ops;
> > + ov02a10->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > + ov02a10->subdev.entity.ops = &ov02a10_subdev_entity_ops;
> > + ov02a10->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > + ov02a10->pad.flags = MEDIA_PAD_FL_SOURCE;
> > + ret = media_entity_pads_init(&ov02a10->subdev.entity, 1, &ov02a10->pad);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to init entity pads: %d", ret);
> > + goto err_power_off;
> > + }
> > +
> > + ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > + if (ret) {
> > + dev_err(dev, "failed to register V4L2 subdev: %d",
> > + ret);
> > + goto err_clean_entity;
> > + }
> > +
> > + pm_runtime_set_active(dev);
> > + pm_runtime_enable(dev);
> > + pm_runtime_idle(dev);
> > +
> > + dev_info(dev, "ov02a10 probe --\n");
>
> Please remove this.
>
Fixed in next release.
> > + return 0;
> > +
> > +err_clean_entity:
> > + media_entity_cleanup(&ov02a10->subdev.entity);
> > +err_power_off:
> > + __ov02a10_power_off(ov02a10);
> > +err_free_handler:
> > + v4l2_ctrl_handler_free(ov02a10->subdev.ctrl_handler);
> > +err_destroy_mutex:
> > + mutex_destroy(&ov02a10->mutex);
> > +
> > + return ret;
> > +}
> > +
> > +static int ov02a10_remove(struct i2c_client *client)
> > +{
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +
> > + v4l2_async_unregister_subdev(sd);
> > + media_entity_cleanup(&sd->entity);
> > + v4l2_ctrl_handler_free(sd->ctrl_handler);
> > + pm_runtime_disable(&client->dev);
> > + if (!pm_runtime_status_suspended(&client->dev))
> > + __ov02a10_power_off(ov02a10);
> > + pm_runtime_set_suspended(&client->dev);
> > + mutex_destroy(&ov02a10->mutex);
> > +
> > + return 0;
> > +}
> > +
> > +#if IS_ENABLED(CONFIG_OF)
> > +static const struct of_device_id ov02a10_of_match[] = {
> > + { .compatible = "ovti,ov02a10" },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, ov02a10_of_match);
> > +#endif
> > +
> > +static struct i2c_driver ov02a10_i2c_driver = {
> > + .driver = {
> > + .name = "ov02a10",
> > + .pm = &ov02a10_pm_ops,
> > + .of_match_table = ov02a10_of_match,
>
> Please use of_match_ptr() wrapper.
>
Sorry. I am a little confused now.
It seems that Sakari had one different ideas about this.
https://patchwork.kernel.org/patch/10957225/
> Best regards,
> Tomasz
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox