* [PATCH AUTOSEL 4.14 1/8] ARM: 8977/1: ptrace: Fix mask for thumb breakpoint hook
From: Sasha Levin @ 2020-06-05 12:26 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Russell King, Fredrik Strupe, Oleg Nesterov,
linux-arm-kernel
From: Fredrik Strupe <fredrik@strupe.net>
[ Upstream commit 3866f217aaa81bf7165c7f27362eee5d7919c496 ]
call_undef_hook() in traps.c applies the same instr_mask for both 16-bit
and 32-bit thumb instructions. If instr_mask then is only 16 bits wide
(0xffff as opposed to 0xffffffff), the first half-word of 32-bit thumb
instructions will be masked out. This makes the function match 32-bit
thumb instructions where the second half-word is equal to instr_val,
regardless of the first half-word.
The result in this case is that all undefined 32-bit thumb instructions
with the second half-word equal to 0xde01 (udf #1) work as breakpoints
and will raise a SIGTRAP instead of a SIGILL, instead of just the one
intended 16-bit instruction. An example of such an instruction is
0xeaa0de01, which is unallocated according to Arm ARM and should raise a
SIGILL, but instead raises a SIGTRAP.
This patch fixes the issue by setting all the bits in instr_mask, which
will still match the intended 16-bit thumb instruction (where the
upper half is always 0), but not any 32-bit thumb instructions.
Cc: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Fredrik Strupe <fredrik@strupe.net>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm/kernel/ptrace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 58e3771e4c5b..368b4b404985 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -228,8 +228,8 @@ static struct undef_hook arm_break_hook = {
};
static struct undef_hook thumb_break_hook = {
- .instr_mask = 0xffff,
- .instr_val = 0xde01,
+ .instr_mask = 0xffffffff,
+ .instr_val = 0x0000de01,
.cpsr_mask = PSR_T_BIT,
.cpsr_val = PSR_T_BIT,
.fn = break_trap,
--
2.25.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 AUTOSEL 4.19 2/9] ARM: 8977/1: ptrace: Fix mask for thumb breakpoint hook
From: Sasha Levin @ 2020-06-05 12:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Russell King, Fredrik Strupe, Oleg Nesterov,
linux-arm-kernel
In-Reply-To: <20200605122558.2882712-1-sashal@kernel.org>
From: Fredrik Strupe <fredrik@strupe.net>
[ Upstream commit 3866f217aaa81bf7165c7f27362eee5d7919c496 ]
call_undef_hook() in traps.c applies the same instr_mask for both 16-bit
and 32-bit thumb instructions. If instr_mask then is only 16 bits wide
(0xffff as opposed to 0xffffffff), the first half-word of 32-bit thumb
instructions will be masked out. This makes the function match 32-bit
thumb instructions where the second half-word is equal to instr_val,
regardless of the first half-word.
The result in this case is that all undefined 32-bit thumb instructions
with the second half-word equal to 0xde01 (udf #1) work as breakpoints
and will raise a SIGTRAP instead of a SIGILL, instead of just the one
intended 16-bit instruction. An example of such an instruction is
0xeaa0de01, which is unallocated according to Arm ARM and should raise a
SIGILL, but instead raises a SIGTRAP.
This patch fixes the issue by setting all the bits in instr_mask, which
will still match the intended 16-bit thumb instruction (where the
upper half is always 0), but not any 32-bit thumb instructions.
Cc: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Fredrik Strupe <fredrik@strupe.net>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm/kernel/ptrace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 36718a424358..492ac74a63f4 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -229,8 +229,8 @@ static struct undef_hook arm_break_hook = {
};
static struct undef_hook thumb_break_hook = {
- .instr_mask = 0xffff,
- .instr_val = 0xde01,
+ .instr_mask = 0xffffffff,
+ .instr_val = 0x0000de01,
.cpsr_mask = PSR_T_BIT,
.cpsr_val = PSR_T_BIT,
.fn = break_trap,
--
2.25.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 AUTOSEL 5.4 04/14] net: stmmac: enable timestamp snapshot for required PTP packets in dwmac v5.10a
From: Sasha Levin @ 2020-06-05 12:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Fugang Duan, netdev, linux-stm32, David S . Miller,
linux-arm-kernel
In-Reply-To: <20200605122540.2882539-1-sashal@kernel.org>
From: Fugang Duan <fugang.duan@nxp.com>
[ Upstream commit f2fb6b6275eba9d312957ca44c487bd780da6169 ]
For rx filter 'HWTSTAMP_FILTER_PTP_V2_EVENT', it should be
PTP v2/802.AS1, any layer, any kind of event packet, but HW only
take timestamp snapshot for below PTP message: sync, Pdelay_req,
Pdelay_resp.
Then it causes below issue when test E2E case:
ptp4l[2479.534]: port 1: received DELAY_REQ without timestamp
ptp4l[2481.423]: port 1: received DELAY_REQ without timestamp
ptp4l[2481.758]: port 1: received DELAY_REQ without timestamp
ptp4l[2483.524]: port 1: received DELAY_REQ without timestamp
ptp4l[2484.233]: port 1: received DELAY_REQ without timestamp
ptp4l[2485.750]: port 1: received DELAY_REQ without timestamp
ptp4l[2486.888]: port 1: received DELAY_REQ without timestamp
ptp4l[2487.265]: port 1: received DELAY_REQ without timestamp
ptp4l[2487.316]: port 1: received DELAY_REQ without timestamp
Timestamp snapshot dependency on register bits in received path:
SNAPTYPSEL TSMSTRENA TSEVNTENA PTP_Messages
01 x 0 SYNC, Follow_Up, Delay_Req,
Delay_Resp, Pdelay_Req, Pdelay_Resp,
Pdelay_Resp_Follow_Up
01 0 1 SYNC, Pdelay_Req, Pdelay_Resp
For dwmac v5.10a, enabling all events by setting register
DWC_EQOS_TIME_STAMPING[SNAPTYPSEL] to 2’b01, clearing bit [TSEVNTENA]
to 0’b0, which can support all required events.
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 1623516efb17..982be75fde83 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -630,7 +630,8 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
ptp_v2 = PTP_TCR_TSVER2ENA;
snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
- ts_event_en = PTP_TCR_TSEVNTENA;
+ if (priv->synopsys_id != DWMAC_CORE_5_10)
+ ts_event_en = PTP_TCR_TSEVNTENA;
ptp_over_ipv4_udp = PTP_TCR_TSIPV4ENA;
ptp_over_ipv6_udp = PTP_TCR_TSIPV6ENA;
ptp_over_ethernet = PTP_TCR_TSIPENA;
--
2.25.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 AUTOSEL 5.4 02/14] ARM: 8977/1: ptrace: Fix mask for thumb breakpoint hook
From: Sasha Levin @ 2020-06-05 12:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Russell King, Fredrik Strupe, Oleg Nesterov,
linux-arm-kernel
In-Reply-To: <20200605122540.2882539-1-sashal@kernel.org>
From: Fredrik Strupe <fredrik@strupe.net>
[ Upstream commit 3866f217aaa81bf7165c7f27362eee5d7919c496 ]
call_undef_hook() in traps.c applies the same instr_mask for both 16-bit
and 32-bit thumb instructions. If instr_mask then is only 16 bits wide
(0xffff as opposed to 0xffffffff), the first half-word of 32-bit thumb
instructions will be masked out. This makes the function match 32-bit
thumb instructions where the second half-word is equal to instr_val,
regardless of the first half-word.
The result in this case is that all undefined 32-bit thumb instructions
with the second half-word equal to 0xde01 (udf #1) work as breakpoints
and will raise a SIGTRAP instead of a SIGILL, instead of just the one
intended 16-bit instruction. An example of such an instruction is
0xeaa0de01, which is unallocated according to Arm ARM and should raise a
SIGILL, but instead raises a SIGTRAP.
This patch fixes the issue by setting all the bits in instr_mask, which
will still match the intended 16-bit thumb instruction (where the
upper half is always 0), but not any 32-bit thumb instructions.
Cc: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Fredrik Strupe <fredrik@strupe.net>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm/kernel/ptrace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 324352787aea..db9401581cd2 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -219,8 +219,8 @@ static struct undef_hook arm_break_hook = {
};
static struct undef_hook thumb_break_hook = {
- .instr_mask = 0xffff,
- .instr_val = 0xde01,
+ .instr_mask = 0xffffffff,
+ .instr_val = 0x0000de01,
.cpsr_mask = PSR_T_BIT,
.cpsr_val = PSR_T_BIT,
.fn = break_trap,
--
2.25.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 AUTOSEL 5.6 05/17] net: stmmac: enable timestamp snapshot for required PTP packets in dwmac v5.10a
From: Sasha Levin @ 2020-06-05 12:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Fugang Duan, netdev, linux-stm32, David S . Miller,
linux-arm-kernel
In-Reply-To: <20200605122517.2882338-1-sashal@kernel.org>
From: Fugang Duan <fugang.duan@nxp.com>
[ Upstream commit f2fb6b6275eba9d312957ca44c487bd780da6169 ]
For rx filter 'HWTSTAMP_FILTER_PTP_V2_EVENT', it should be
PTP v2/802.AS1, any layer, any kind of event packet, but HW only
take timestamp snapshot for below PTP message: sync, Pdelay_req,
Pdelay_resp.
Then it causes below issue when test E2E case:
ptp4l[2479.534]: port 1: received DELAY_REQ without timestamp
ptp4l[2481.423]: port 1: received DELAY_REQ without timestamp
ptp4l[2481.758]: port 1: received DELAY_REQ without timestamp
ptp4l[2483.524]: port 1: received DELAY_REQ without timestamp
ptp4l[2484.233]: port 1: received DELAY_REQ without timestamp
ptp4l[2485.750]: port 1: received DELAY_REQ without timestamp
ptp4l[2486.888]: port 1: received DELAY_REQ without timestamp
ptp4l[2487.265]: port 1: received DELAY_REQ without timestamp
ptp4l[2487.316]: port 1: received DELAY_REQ without timestamp
Timestamp snapshot dependency on register bits in received path:
SNAPTYPSEL TSMSTRENA TSEVNTENA PTP_Messages
01 x 0 SYNC, Follow_Up, Delay_Req,
Delay_Resp, Pdelay_Req, Pdelay_Resp,
Pdelay_Resp_Follow_Up
01 0 1 SYNC, Pdelay_Req, Pdelay_Resp
For dwmac v5.10a, enabling all events by setting register
DWC_EQOS_TIME_STAMPING[SNAPTYPSEL] to 2’b01, clearing bit [TSEVNTENA]
to 0’b0, which can support all required events.
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d564459290ce..bcb39012d34d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -630,7 +630,8 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
ptp_v2 = PTP_TCR_TSVER2ENA;
snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
- ts_event_en = PTP_TCR_TSEVNTENA;
+ if (priv->synopsys_id != DWMAC_CORE_5_10)
+ ts_event_en = PTP_TCR_TSEVNTENA;
ptp_over_ipv4_udp = PTP_TCR_TSIPV4ENA;
ptp_over_ipv6_udp = PTP_TCR_TSIPV6ENA;
ptp_over_ethernet = PTP_TCR_TSIPENA;
--
2.25.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 AUTOSEL 5.6 03/17] ARM: 8977/1: ptrace: Fix mask for thumb breakpoint hook
From: Sasha Levin @ 2020-06-05 12:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Russell King, Fredrik Strupe, Oleg Nesterov,
linux-arm-kernel
In-Reply-To: <20200605122517.2882338-1-sashal@kernel.org>
From: Fredrik Strupe <fredrik@strupe.net>
[ Upstream commit 3866f217aaa81bf7165c7f27362eee5d7919c496 ]
call_undef_hook() in traps.c applies the same instr_mask for both 16-bit
and 32-bit thumb instructions. If instr_mask then is only 16 bits wide
(0xffff as opposed to 0xffffffff), the first half-word of 32-bit thumb
instructions will be masked out. This makes the function match 32-bit
thumb instructions where the second half-word is equal to instr_val,
regardless of the first half-word.
The result in this case is that all undefined 32-bit thumb instructions
with the second half-word equal to 0xde01 (udf #1) work as breakpoints
and will raise a SIGTRAP instead of a SIGILL, instead of just the one
intended 16-bit instruction. An example of such an instruction is
0xeaa0de01, which is unallocated according to Arm ARM and should raise a
SIGILL, but instead raises a SIGTRAP.
This patch fixes the issue by setting all the bits in instr_mask, which
will still match the intended 16-bit thumb instruction (where the
upper half is always 0), but not any 32-bit thumb instructions.
Cc: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Fredrik Strupe <fredrik@strupe.net>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm/kernel/ptrace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index b606cded90cd..4cc6a7eff635 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -219,8 +219,8 @@ static struct undef_hook arm_break_hook = {
};
static struct undef_hook thumb_break_hook = {
- .instr_mask = 0xffff,
- .instr_val = 0xde01,
+ .instr_mask = 0xffffffff,
+ .instr_val = 0x0000de01,
.cpsr_mask = PSR_T_BIT,
.cpsr_val = PSR_T_BIT,
.fn = break_trap,
--
2.25.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 v2 0/2] ASoC: mediatek: mt6358: support DMIC one-wire mode
From: Tzung-Bi Shih @ 2020-06-05 12:24 UTC (permalink / raw)
To: Jiaxin Yu
Cc: Hariprasad Kelam, ALSA development, Linux Kernel Mailing List,
howie.huang, Liam Girdwood, Takashi Iwai, Mark Brown,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <1591353222-18576-1-git-send-email-jiaxin.yu@mediatek.com>
On Fri, Jun 5, 2020 at 6:37 PM Jiaxin Yu <jiaxin.yu@mediatek.com> wrote:
> Jiaxin Yu (2):
> ASoC: mediatek: mt6358: support DMIC one-wire mode
Has done previous round review on https://crrev.com/c/2230089
> ASoC: dt-bindings: mediatek: mt6358: add dmic-mode property
Has done previous round review on https://crrev.com/c/2230086
> Documentation/devicetree/bindings/sound/mt6358.txt | 6 ++++++
> sound/soc/codecs/mt6358.c | 23 +++++++++++++++++++++-
> 2 files changed, 28 insertions(+), 1 deletion(-)
With that:
Reviewed-by: Tzung-Bi Shih <tzungbi@google.com>
_______________________________________________
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] efi/arm: decompressor: deal with HYP mode boot gracefully
From: Heinrich Schuchardt @ 2020-06-05 12:20 UTC (permalink / raw)
To: Ard Biesheuvel, linux-efi; +Cc: maz, linux, linux-arm-kernel
In-Reply-To: <20200605115200.413921-1-ardb@kernel.org>
On 05.06.20 13:52, Ard Biesheuvel wrote:
> EFI on ARM only supports short descriptors, and given that it mandates
> that the MMU and caches are on, it is implied that booting in HYP mode
> is not supported.
>
> However, implementations of EFI exist (i.e., U-Boot) that ignore this
> requirement, which is not entirely unreasonable, given that it does
> not make a lot of sense to begin with.
Hello Ard,
thanks for investigating the differences between EDK2 and U-Boot.
What I still do not understand is if there is a bug in U-Boot where
U-Boot does not conform to the UEFI specification? In this case I would
expect a fix in U-Boot.
Or is it simply a deficiency of Linux that it does not properly support
HYP/EL2 mode on 32-bit ARM?
Up to now I never experience a problem booting a 32bit board via U-Boot
-> GRUB-EFI -> Linux. Where did you have a problem when booting via
U-Boot's UEFI implementation and the current Linux kernel?
Does this patch relate to the retirement of KVM on 32 ARM in Linux 5.7
541ad0150ca4 ("arm: Remove 32bit KVM host support")? Without that patch
I would have expected Linux to work fine in HYP mode.
Best regards
Heinrich
>
> So let's make sure that we can deal with this condition gracefully.
> We already tolerate booting the EFI stub with the caches off (even
> though this violates the EFI spec as well), and so we should deal
> with HYP mode boot with MMU and caches either on or off.
>
> - When the MMU and caches are on, we can ignore the HYP stub altogether,
> since we can just use the existing mappings, and just branch into
> the decompressed kernel as usual after disabling MMU and caches.
>
> - When the MMU and caches are off, we have to drop to SVC mode so that
> we can set up the page tables using short descriptors. In this case,
> we need to install the HYP stub so that we can return to HYP mode
> when handing over to the kernel proper.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/arm/boot/compressed/head.S | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index c79db44ba128..986db86ba334 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -1436,6 +1436,35 @@ ENTRY(efi_enter_kernel)
> mrc p15, 0, r0, c1, c0, 0 @ read SCTLR
> tst r0, #0x1 @ MMU enabled?
> orreq r4, r4, #1 @ set LSB if not
> +#ifdef CONFIG_ARM_VIRT_EXT
> + @
> + @ The EFI spec does not support booting on ARM in HYP mode,
> + @ since it mandates that the MMU and caches are on, with all
> + @ 32-bit addressable DRAM mapped 1:1 using short descriptors.
> + @ While the EDK2 reference implementation adheres to this,
> + @ U-Boot might decide to enter the EFI stub in HYP mode anyway,
> + @ with the MMU and caches either on or off.
> + @ In the former case, we're better off just carrying on using
> + @ the cached 1:1 mapping that the firmware provided, and pretend
> + @ that we are in SVC mode as far as the decompressor is
> + @ concerned. However, if the caches are off, we need to drop
> + @ into SVC mode now, and let the decompressor set up its cached
> + @ 1:1 mapping as usual.
> + @
> + mov r0, #SVC_MODE
> + msr spsr_cxsf, r0 @ record that we are in SVC mode
> + bne 1f @ skip HYP stub install if MMU is on
> +
> + mov r9, r4 @ preserve image base
> + bl __hyp_stub_install @ returns boot mode in r4
> + cmp r4, #HYP_MODE @ did we boot in HYP?
> + bne 1f @ skip drop to SVC if we did not
> +
> + safe_svcmode_maskall r0
> + msr spsr_cxsf, r4 @ record boot mode
> + mov r4, r9 @ restore image base
> +1:
> +#endif
>
> mov r0, r8 @ DT start
> add r1, r8, r2 @ DT end
>
_______________________________________________
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] spi: bcm2835: Enable shared interrupt support
From: Mark Brown @ 2020-06-05 12:20 UTC (permalink / raw)
To: linux-kernel, Florian Fainelli
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Scott Branden, Ray Jui, Rob Herring, open list:SPI SUBSYSTEM,
lukas,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Martin Sperl, Nicolas Saenz Julienne,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
In-Reply-To: <159135564425.14579.13716287498736798458.b4-ty@kernel.org>
[-- Attachment #1.1: Type: text/plain, Size: 295 bytes --]
On Fri, Jun 05, 2020 at 12:14:07PM +0100, Mark Brown wrote:
> [1/1] spi: bcm2835: Enable shared interrupt support
> commit: ecfbd3cf3b8bb73ac6a80ddf430b5912fd4402a6
Eh, sorry - this was me fat fingering another fix. At the very least
this needs to wait for the end of the merge window.
[-- 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: [kvmtool][PATCH] arm64: Obtain text offset from kernel image
From: Alexandru Elisei @ 2020-06-05 12:16 UTC (permalink / raw)
To: Marc Zyngier, kvmarm, linux-arm-kernel
Cc: Will Deacon, Ard Biesheuvel, Julien Thierry
In-Reply-To: <20200605104907.1307967-1-maz@kernel.org>
Hi Marc,
On 6/5/20 11:49 AM, Marc Zyngier wrote:
> Recent changes made to Linux 5.8 have outlined that kvmtool
> hardcodes the text offset instead of reading it from the arm64
> image itself.
>
> To address this, import the image header structure into kvmtool
> and do the right thing. 32bit guests are still loaded to their
> usual locations.
>
> Reported-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> Makefile | 1 +
> arm/aarch32/include/kvm/kvm-arch.h | 2 +-
> arm/aarch64/include/asm/image.h | 59 ++++++++++++++++++++++++++++++
> arm/aarch64/include/kvm/kvm-arch.h | 5 +--
> arm/aarch64/kvm.c | 30 +++++++++++++++
> arm/kvm.c | 2 +-
> 6 files changed, 94 insertions(+), 5 deletions(-)
> create mode 100644 arm/aarch64/include/asm/image.h
> create mode 100644 arm/aarch64/kvm.c
>
> [..]
This is a great addition to kvmtool, thank you! Before I do a more in-depth
review, I have some general questions.
Regarding the actual value of text_offset, the booting.rst document says:
"Prior to v3.17, the endianness of text_offset was not specified. In these cases
image_size is zero and text_offset is 0x80000 in the endianness of the kernel.
Where image_size is non-zero image_size is little-endian and must be respected.
Where image_size is zero, text_offset can be assumed to be 0x80000".
All header fields are declared little-endian, which looks to me like it would
break kernels older than 3.17. If that was intentional, I think it's worth
documenting somewhere, or at least a comment for the kvm__arch_get_kern_offset
function.
Now that we are parsing the kernel header, have you considered checking the magic
number to make sure the user specified a valid kernel image? It might save someone
some time debugging why the kernel isn't booting, if, for example, they are
booting an armv7 kernel, but they forgot to specify --aarch32.
Thanks,
Alex
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [V7, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Sakari Ailus @ 2020-06-05 12:14 UTC (permalink / raw)
To: Dongchun Zhu
Cc: mark.rutland, drinkcat, andriy.shevchenko, srv_heupstream,
devicetree, linus.walleij, shengnan.wang, tfiga, bgolaszewski,
sj.huang, robh+dt, linux-mediatek, louis.kuo, matthias.bgg,
bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <20200605105412.18813-3-dongchun.zhu@mediatek.com>
Hi Dongchun,
Thank you for the update.
On Fri, Jun 05, 2020 at 06:54:12PM +0800, Dongchun Zhu wrote:
> Add a V4L2 sub-device driver for DW9768 voice coil motor, providing
> control to set the desired focus via IIC serial interface.
>
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> ---
> MAINTAINERS | 1 +
> drivers/media/i2c/Kconfig | 13 ++
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/dw9768.c | 566 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 581 insertions(+)
> create mode 100644 drivers/media/i2c/dw9768.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8d72c41..c92dc99 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5157,6 +5157,7 @@ L: linux-media@vger.kernel.org
> S: Maintained
> T: git git://linuxtv.org/media_tree.git
> F: Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
> +F: drivers/media/i2c/dw9768.c
>
> DONGWOON DW9807 LENS VOICE COIL DRIVER
> M: Sakari Ailus <sakari.ailus@linux.intel.com>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 125d596..afdf994 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -1040,6 +1040,19 @@ config VIDEO_DW9714
> capability. This is designed for linear control of
> voice coil motors, controlled via I2C serial interface.
>
> +config VIDEO_DW9768
> + tristate "DW9768 lens voice coil support"
> + depends on I2C && VIDEO_V4L2
> + depends on PM
> + select MEDIA_CONTROLLER
> + select VIDEO_V4L2_SUBDEV_API
> + select V4L2_FWNODE
> + help
> + This is a driver for the DW9768 camera lens voice coil.
> + DW9768 is a 10 bit DAC with 100mA output current sink
> + capability. This is designed for linear control of
> + voice coil motors, controlled via I2C serial interface.
> +
> config VIDEO_DW9807_VCM
> tristate "DW9807 lens voice coil support"
> depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 77bf7d0..4057476 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
> obj-$(CONFIG_VIDEO_AD5820) += ad5820.o
> obj-$(CONFIG_VIDEO_AK7375) += ak7375.o
> obj-$(CONFIG_VIDEO_DW9714) += dw9714.o
> +obj-$(CONFIG_VIDEO_DW9768) += dw9768.o
> obj-$(CONFIG_VIDEO_DW9807_VCM) += dw9807-vcm.o
> obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
> obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
> diff --git a/drivers/media/i2c/dw9768.c b/drivers/media/i2c/dw9768.c
> new file mode 100644
> index 0000000..f34a8ed
> --- /dev/null
> +++ b/drivers/media/i2c/dw9768.c
> @@ -0,0 +1,566 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2020 MediaTek Inc.
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +#include <media/v4l2-async.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define DW9768_NAME "dw9768"
> +#define DW9768_MAX_FOCUS_POS (1024 - 1)
> +/*
> + * This sets the minimum granularity for the focus positions.
> + * A value of 1 gives maximum accuracy for a desired focus position
> + */
> +#define DW9768_FOCUS_STEPS 1
> +
> +/*
> + * Ring control and Power control register
> + * Bit[1] RING_EN
> + * 0: Direct mode
> + * 1: AAC mode (ringing control mode)
> + * Bit[0] PD
> + * 0: Normal operation mode
> + * 1: Power down mode
> + * DW9768 requires waiting time of Topr after PD reset takes place.
> + */
> +#define DW9768_RING_PD_CONTROL_REG 0x02
> +#define DW9768_PD_MODE_OFF 0x00
> +#define DW9768_PD_MODE_EN BIT(0)
> +#define DW9768_AAC_MODE_EN BIT(1)
> +
> +/*
> + * DW9768 separates two registers to control the VCM position.
> + * One for MSB value, another is LSB value.
> + * DAC_MSB: D[9:8] (ADD: 0x03)
> + * DAC_LSB: D[7:0] (ADD: 0x04)
> + * D[9:0] DAC data input: positive output current = D[9:0] / 1023 * 100[mA]
> + */
> +#define DW9768_MSB_ADDR 0x03
> +#define DW9768_LSB_ADDR 0x04
> +#define DW9768_STATUS_ADDR 0x05
> +
> +/*
> + * AAC mode control & prescale register
> + * Bit[7:5] Namely AC[2:0], decide the VCM mode and operation time.
> + * 001 AAC2 0.48 x Tvib
> + * 010 AAC3 0.70 x Tvib
> + * 011 AAC4 0.75 x Tvib
> + * 101 AAC8 1.13 x Tvib
> + * Bit[2:0] Namely PRESC[2:0], set the internal clock dividing rate as follow.
> + * 000 2
> + * 001 1
> + * 010 1/2
> + * 011 1/4
> + * 100 8
> + * 101 4
> + */
> +#define DW9768_AAC_PRESC_REG 0x06
> +#define DW9768_AAC_MODE_SEL_MASK GENMASK(7, 5)
> +#define DW9768_CLOCK_PRE_SCALE_SEL_MASK GENMASK(2, 0)
> +
> +/*
> + * VCM period of vibration register
> + * Bit[5:0] Defined as VCM rising periodic time (Tvib) together with PRESC[2:0]
> + * Tvib = (6.3ms + AACT[5:0] * 0.1ms) * Dividing Rate
> + * Dividing Rate is the internal clock dividing rate that is defined at
> + * PRESCALE register (ADD: 0x06)
> + */
> +#define DW9768_AAC_TIME_REG 0x07
> +
> +/*
> + * DW9768 requires waiting time (delay time) of t_OPR after power-up,
> + * or in the case of PD reset taking place.
> + */
> +#define DW9768_T_OPR_US 1000
> +#define DW9768_Tvib_MS_BASE10 (64 - 1)
> +#define DW9768_AAC_MODE_DEFAULT 2
> +#define DW9768_AAC_TIME_DEFAULT 0x20
> +#define DW9768_CLOCK_PRE_SCALE_DEFAULT 1
> +
> +/*
> + * This acts as the minimum granularity of lens movement.
> + * Keep this value power of 2, so the control steps can be
> + * uniformly adjusted for gradual lens movement, with desired
> + * number of control steps.
> + */
> +#define DW9768_MOVE_STEPS 16
> +
> +static const char * const dw9768_supply_names[] = {
> + "vin", /* Digital I/O power */
> + "vdd", /* Digital core power */
> +};
> +
> +/* dw9768 device structure */
> +struct dw9768 {
> + struct regulator_bulk_data supplies[ARRAY_SIZE(dw9768_supply_names)];
> + struct v4l2_ctrl_handler ctrls;
> + struct v4l2_ctrl *focus;
> + struct v4l2_subdev sd;
> +
> + u32 aac_mode;
> + u32 aac_timing;
> + u32 clock_presc;
> +};
> +
> +static inline struct dw9768 *sd_to_dw9768(struct v4l2_subdev *subdev)
> +{
> + return container_of(subdev, struct dw9768, sd);
> +}
> +
> +struct regval_list {
> + u8 reg_num;
> + u8 value;
> +};
> +
> +struct dw9768_aac_mode_ot_multi {
> + u32 aac_mode_enum;
> + u32 ot_multi_base100;
> +};
> +
> +struct dw9768_clk_presc_dividing_rate {
> + u32 clk_presc_enum;
> + u32 dividing_rate_base100;
> +};
> +
> +static const struct dw9768_aac_mode_ot_multi aac_mode_ot_multi[] = {
> + {1, 48},
> + {2, 70},
> + {3, 75},
> + {5, 113},
> +};
> +
> +static const struct dw9768_clk_presc_dividing_rate presc_dividing_rate[] = {
> + {0, 200},
> + {1, 100},
> + {2, 50},
> + {3, 25},
> + {4, 800},
> + {5, 400},
> +};
> +
> +static u32 dw9768_find_ot_multi(u32 aac_mode_param)
> +{
> + u32 cur_ot_multi_base100 = 70;
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(aac_mode_ot_multi); i++) {
> + if (aac_mode_ot_multi[i].aac_mode_enum == aac_mode_param) {
> + cur_ot_multi_base100 =
> + aac_mode_ot_multi[i].ot_multi_base100;
> + }
> + }
> +
> + return cur_ot_multi_base100;
> +}
> +
> +static u32 dw9768_find_dividing_rate(u32 presc_param)
> +{
> + u32 cur_clk_dividing_rate_base100 = 100;
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(presc_dividing_rate); i++) {
> + if (presc_dividing_rate[i].clk_presc_enum == presc_param) {
> + cur_clk_dividing_rate_base100 =
> + presc_dividing_rate[i].dividing_rate_base100;
> + }
> + }
> +
> + return cur_clk_dividing_rate_base100;
> +}
> +
> +/*
> + * DW9768_AAC_PRESC_REG & DW9768_AAC_TIME_REG determine VCM operation time.
> + * For current VCM mode: AAC3, Operation Time would be 0.70 x Tvib.
> + * Tvib = (6.3ms + AACT[5:0] * 0.1MS) * Dividing Rate.
> + * Below is calculation of the operation delay for each step.
> + */
> +static inline u32 dw9768_cal_move_delay(u32 aac_mode_param, u32 presc_param,
> + u32 aac_timing_param)
> +{
> + u32 Tvib_us;
> + u32 ot_multi_base100;
> + u32 clk_dividing_rate_base100;
> +
> + ot_multi_base100 = dw9768_find_ot_multi(aac_mode_param);
> +
> + clk_dividing_rate_base100 = dw9768_find_dividing_rate(presc_param);
> +
> + Tvib_us = (DW9768_Tvib_MS_BASE10 + aac_timing_param) *
> + clk_dividing_rate_base100;
> +
> + return Tvib_us * ot_multi_base100;
> +}
> +
> +static int dw9768_mod_reg(struct dw9768 *dw9768, u8 reg, u8 mask, u8 val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + val = ((unsigned char)ret & ~mask) | (val & mask);
> +
> + return i2c_smbus_write_byte_data(client, reg, val);
> +}
> +
> +static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> +
> + /* Write VCM position to registers */
> + return i2c_smbus_write_word_swapped(client, DW9768_MSB_ADDR, val);
> +}
> +
> +static int dw9768_init(struct dw9768 *dw9768)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + u32 move_delay_us;
> + int ret, val;
> +
> + /* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
> + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> + DW9768_PD_MODE_OFF);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * DW9769 requires waiting delay time of t_OPR
> + * after PD reset takes place.
> + */
> + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> +
> + /* Set DW9768_RING_PD_CONTROL_REG to DW9768_AAC_MODE_EN(0x01) */
> + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> + DW9768_AAC_MODE_EN);
> + if (ret < 0)
> + return ret;
> +
> + /* Set AAC mode */
> + ret = dw9768_mod_reg(dw9768, DW9768_AAC_PRESC_REG,
> + DW9768_AAC_MODE_SEL_MASK,
> + dw9768->aac_mode << 5);
> + if (ret < 0)
> + return ret;
> +
> + /* Set clock presc */
> + if (dw9768->clock_presc != DW9768_CLOCK_PRE_SCALE_DEFAULT) {
> + ret = dw9768_mod_reg(dw9768, DW9768_AAC_PRESC_REG,
> + DW9768_CLOCK_PRE_SCALE_SEL_MASK,
> + dw9768->clock_presc);
> + if (ret < 0)
> + return ret;
> + }
> +
> + /* Set AAC Timing */
> + if (dw9768->aac_timing != DW9768_AAC_TIME_DEFAULT) {
> + ret = i2c_smbus_write_byte_data(client, DW9768_AAC_TIME_REG,
> + dw9768->aac_timing);
> + if (ret < 0)
> + return ret;
> + }
> +
> + move_delay_us = dw9768_cal_move_delay(dw9768->aac_mode,
> + dw9768->clock_presc,
> + dw9768->aac_timing) / 100;
> +
> + for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
> + val <= dw9768->focus->val;
> + val += DW9768_MOVE_STEPS) {
> + ret = dw9768_set_dac(dw9768, val);
> + if (ret) {
> + dev_err(&client->dev, "%s I2C failure: %d",
> + __func__, ret);
> + return ret;
> + }
> + usleep_range(move_delay_us, move_delay_us + 1000);
> + }
> +
> + return 0;
> +}
> +
> +static int dw9768_release(struct dw9768 *dw9768)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + u32 move_delay_us = dw9768_cal_move_delay(dw9768->aac_mode,
> + dw9768->clock_presc,
> + dw9768->aac_timing) / 100;
> + int ret, val;
> +
> + val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
> + for ( ; val >= 0; val -= DW9768_MOVE_STEPS) {
> + ret = dw9768_set_dac(dw9768, val);
> + if (ret) {
> + dev_err(&client->dev, "I2C write fail: %d", ret);
> + return ret;
> + }
> + usleep_range(move_delay_us, move_delay_us + 1000);
> + }
> +
> + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> + DW9768_PD_MODE_EN);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * DW9769 requires waiting delay time of t_OPR
> + * after PD reset takes place.
> + */
> + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> +
> + return 0;
> +}
> +
> +static int dw9768_runtime_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> +
> + dw9768_release(dw9768);
> + regulator_bulk_disable(ARRAY_SIZE(dw9768_supply_names),
> + dw9768->supplies);
> +
> + return 0;
> +}
> +
> +static int dw9768_runtime_resume(struct device *dev)
__maybe_unused for this and the suspend callback.
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> + int ret;
> +
> + ret = regulator_bulk_enable(ARRAY_SIZE(dw9768_supply_names),
> + dw9768->supplies);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable regulators\n");
> + return ret;
> + }
> +
> + /*
> + * The datasheet refers to t_OPR that needs to be waited before sending
> + * I2C commands after power-up.
> + */
> + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> +
> + ret = dw9768_init(dw9768);
> + if (ret < 0)
> + goto disable_regulator;
> +
> + return 0;
> +
> +disable_regulator:
> + regulator_bulk_disable(ARRAY_SIZE(dw9768_supply_names),
> + dw9768->supplies);
> +
> + return ret;
> +}
> +
> +static int dw9768_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct dw9768 *dw9768 = container_of(ctrl->handler,
> + struct dw9768, ctrls);
> +
> + if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
> + return dw9768_set_dac(dw9768, ctrl->val);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops dw9768_ctrl_ops = {
> + .s_ctrl = dw9768_set_ctrl,
> +};
> +
> +static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + int ret;
> +
> + ret = pm_runtime_get_sync(sd->dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(sd->dev);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + pm_runtime_put(sd->dev);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_subdev_internal_ops dw9768_int_ops = {
> + .open = dw9768_open,
> + .close = dw9768_close,
> +};
> +
> +static const struct v4l2_subdev_ops dw9768_ops = { };
> +
> +static int dw9768_init_controls(struct dw9768 *dw9768)
> +{
> + struct v4l2_ctrl_handler *hdl = &dw9768->ctrls;
> + const struct v4l2_ctrl_ops *ops = &dw9768_ctrl_ops;
> +
> + v4l2_ctrl_handler_init(hdl, 1);
> +
> + dw9768->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE, 0,
> + DW9768_MAX_FOCUS_POS,
> + DW9768_FOCUS_STEPS, 0);
> +
> + if (hdl->error)
> + return hdl->error;
> +
> + dw9768->sd.ctrl_handler = hdl;
> +
> + return 0;
> +}
> +
> +static int dw9768_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct dw9768 *dw9768;
> + u32 aac_mode_select;
> + u32 aac_timing_select;
> + u32 clock_presc_select;
> + unsigned int i;
> + int ret;
> +
> + dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
> + if (!dw9768)
> + return -ENOMEM;
> +
> + /* Initialize subdev */
> + v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
> +
> + dw9768->aac_mode = DW9768_AAC_MODE_DEFAULT;
> + dw9768->aac_timing = DW9768_AAC_TIME_DEFAULT;
> + dw9768->clock_presc = DW9768_CLOCK_PRE_SCALE_DEFAULT;
> +
> + /* Optional indication of AAC mode select */
> + ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,aac-mode",
> + &aac_mode_select);
> +
> + if (!ret)
> + dw9768->aac_mode = aac_mode_select;
> +
> + /* Optional indication of clock pre-scale select */
> + ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,clock-presc",
> + &clock_presc_select);
> +
> + if (!ret)
> + dw9768->clock_presc = clock_presc_select;
> +
> + /* Optional indication of AAC Timing */
> + ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,aac-timing",
> + &aac_timing_select);
> +
> + if (!ret)
> + dw9768->aac_timing = aac_timing_select;
You can assign the defaults to the dw9768 struct and use the fwnode
property API to read the properties into the same fields. No return values
need to be checked.
> +
> + for (i = 0; i < ARRAY_SIZE(dw9768_supply_names); i++)
> + dw9768->supplies[i].supply = dw9768_supply_names[i];
> +
> + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(dw9768_supply_names),
> + dw9768->supplies);
> + if (ret) {
> + dev_err(dev, "failed to get regulators\n");
> + return ret;
> + }
> +
> + /* Initialize controls */
> + ret = dw9768_init_controls(dw9768);
> + if (ret)
> + goto err_free_handler;
> +
> + /* Initialize subdev */
> + dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + dw9768->sd.internal_ops = &dw9768_int_ops;
> +
> + ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
> + if (ret < 0)
> + goto err_free_handler;
> +
> + dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
> +
> + pm_runtime_enable(dev);
> + if (!pm_runtime_enabled(dev)) {
> + ret = dw9768_runtime_resume(dev);
> + if (ret < 0) {
> + dev_err(dev, "failed to power on: %d\n", ret);
> + goto err_clean_entity;
> + }
> + }
> +
> + ret = v4l2_async_register_subdev(&dw9768->sd);
> + if (ret < 0) {
> + dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> + goto error_async_register;
> + }
> +
> + return 0;
> +
> +error_async_register:
> + if (!pm_runtime_enabled(dev))
> + dw9768_runtime_suspend(dev);
> +err_clean_entity:
> + media_entity_cleanup(&dw9768->sd.entity);
> +err_free_handler:
> + v4l2_ctrl_handler_free(&dw9768->ctrls);
> +
> + return ret;
> +}
> +
> +static int dw9768_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> +
> + v4l2_async_unregister_subdev(&dw9768->sd);
> + v4l2_ctrl_handler_free(&dw9768->ctrls);
> + media_entity_cleanup(&dw9768->sd.entity);
> + pm_runtime_disable(&client->dev);
> + if (!pm_runtime_status_suspended(&client->dev))
> + dw9768_runtime_suspend(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id dw9768_of_table[] = {
> + { .compatible = "dongwoon,dw9768" },
> + { .compatible = "giantec,gt9769" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, dw9768_of_table);
> +
> +static const struct dev_pm_ops dw9768_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(dw9768_runtime_suspend, dw9768_runtime_resume, NULL)
> +};
> +
> +static struct i2c_driver dw9768_i2c_driver = {
> + .driver = {
> + .name = DW9768_NAME,
> + .pm = &dw9768_pm_ops,
> + .of_match_table = dw9768_of_table,
> + },
> + .probe_new = dw9768_probe,
> + .remove = dw9768_remove,
> +};
> +module_i2c_driver(dw9768_i2c_driver);
> +
> +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> +MODULE_DESCRIPTION("DW9768 VCM driver");
> +MODULE_LICENSE("GPL v2");
--
Kind regards,
Sakari Ailus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: Please help to confirm the risk if using TPIDRRO_EL0 to save CPU number, thanks.
From: Mark Brown @ 2020-06-05 12:10 UTC (permalink / raw)
To: Will Deacon
Cc: fujun (F), Wuxuecheng, linux-arm-kernel@lists.infradead.org,
Lixin (Victor, Kirin)
In-Reply-To: <20200601070311.GA8601@willie-the-truck>
[-- Attachment #1.1: Type: text/plain, Size: 2294 bytes --]
On Mon, Jun 01, 2020 at 08:03:12AM +0100, Will Deacon wrote:
> On Fri, May 29, 2020 at 09:03:37AM +0000, Lixin (Victor, Kirin) wrote:
> > Intel optimized getcpu syscall on Linux/Android system by using vDSO, but
> > ARM doesn't do any optimizations for getcpu syscall.
> > In Apple open source, TPIDRRO_EL0/TPIDRURO is used to save the CPU number,
> > [1]https://opensource.apple.com/source/xnu/xnu-4570.1.46/osfmk/arm/cswitch.s.auto.html
> > Is there any risk if using TPIDRRO_EL0/TPIDRURO to implement
> > the vDSO for getcpu? Is there any possible to break any ARM ABI? Can you
> > help us to confirm the considerations?
> Do you have a use-case for high-performance getcpu() that isn't better
> suited to rseq()?
I actually have an implementation of this that I'd been waiting for the
end of the merge window to post, largely because I first heard of the
use of restartable sequences for this after I'd already implemented the
vDSO version - this stuff is not as discoverable as one might desire.
It doesn't store the CPU ID directly in TPIDRRO but rather uses TPIDDRRO
to store the offset of a per-CPU struct in the vDSO data in order to
allow for the addition of further data in the future. I'll post it
today for discussion.
The latest version of the Mathieu's glibc integration patches is:
https://lore.kernel.org/lkml/20200527185130.5604-3-mathieu.desnoyers@efficios.com/
The only things I can see where the vDSO does better are support for the
node parameter of getcpu() and the ease of implementation for the users,
the restartable sequences code was merged all the way back in v4.18 and
it's still not used by any of the libcs as far as I can see. The node
to CPU mapping is static so I'm not sure how exciting that is, it could
be looked up separately when processing data if it's important, but the
ease of use feels like something.
One important caveat with using TPIDRRO is that if KPTI is active then
the KPTI trampoline uses TPIDRRO as a scratch register so unless we can
find another register for scratch usage the user would need to give up
the protections offered by KPTI or run on future hardware which can use
E0PD instead. This severely limits the usefulness on current systems.
[-- 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: [PATCH v4 07/11] hwmon: add support for the sl28cpld hardware monitoring controller
From: Andy Shevchenko @ 2020-06-05 12:06 UTC (permalink / raw)
To: Michael Walle
Cc: devicetree, Linus Walleij, Thierry Reding, Lee Jones,
Jason Cooper, Andy Shevchenko, Marc Zyngier, Bartosz Golaszewski,
Uwe Kleine-König, Guenter Roeck, linux-pwm, Jean Delvare,
linux-watchdog, open list:GPIO SUBSYSTEM, Mark Brown,
Thomas Gleixner, Wim Van Sebroeck, linux-arm Mailing List,
linux-hwmon, Greg Kroah-Hartman, Linux Kernel Mailing List,
Li Yang, Rob Herring, Shawn Guo
In-Reply-To: <20200604211039.12689-8-michael@walle.cc>
On Fri, Jun 5, 2020 at 12:14 AM Michael Walle <michael@walle.cc> wrote:
>
> Add support for the hardware monitoring controller of the sl28cpld board
> management controller. This driver is part of a multi-function device.
...
> +#include <linux/of_device.h>
mod_devicetable.h ?
...
> + hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
> + "sl28cpld_hwmon", hwmon,
> + &sl28cpld_hwmon_chip_info, NULL);
> + if (IS_ERR(hwmon_dev)) {
> + dev_err(&pdev->dev, "failed to register as hwmon device");
> + return PTR_ERR(hwmon_dev);
> + }
> +
> + return 0;
PTR_ERR_OR_ZERO() ?
...
> +static const struct of_device_id sl28cpld_hwmon_of_match[] = {
> + { .compatible = "kontron,sl28cpld-fan" },
> + {},
No comma.
> +};
--
With Best Regards,
Andy Shevchenko
_______________________________________________
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 2/3] media: stm32-dcmi: Set minimum cpufreq requirement
From: Valentin Schneider @ 2020-06-05 12:05 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: alexandre.torgue, rjw, linux-kernel, mcoquelin.stm32,
hugues.fruchet, mchehab, linux-stm32, linux-arm-kernel,
linux-media
In-Reply-To: <20200604123932.20512-3-benjamin.gaignard@st.com>
On 04/06/20 13:39, Benjamin Gaignard wrote:
> Before start streaming set cpufreq minimum frequency requirement.
> The cpufreq governor will adapt the frequencies and we will have
> no latency for handling interrupts.
> The frequency requirement is retrieved from the device-tree node.
>
> While streaming be notified if the IRQ affinity change thanks to
> irq_affinity_notify callback.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
> version 3:
> - add a cpumask field to track boosted CPUs
> - add irq_affinity_notify callback
> - protect cpumask field with a mutex
>
> drivers/media/platform/stm32/stm32-dcmi.c | 187 ++++++++++++++++++++++++++++--
> 1 file changed, 179 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
> index b8931490b83b..fb6ab09eaff0 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> +static void dcmi_irq_notifier_notify(struct irq_affinity_notify *notify,
> + const cpumask_t *mask)
> +{
> + struct stm32_dcmi *dcmi = container_of(notify,
> + struct stm32_dcmi,
> + notify);
> + struct cpufreq_policy *p;
> + int cpu;
> +
> + mutex_lock(&dcmi->freq_lock);
> + /*
> + * For all boosted CPUs check if it is still the case
> + * if not remove the request
> + */
> + for_each_cpu(cpu, dcmi->boosted) {
> + if (cpumask_test_cpu(cpu, mask))
> + continue;
> +
> + p = cpufreq_cpu_get(cpu);
> + if (!p)
> + continue;
> +
> + freq_qos_remove_request(&per_cpu(qos_req, cpu));
> + cpumask_andnot(dcmi->boosted, dcmi->boosted, p->cpus);
> +
> + cpufreq_cpu_put(p);
> + }
> +
> + /*
> + * For CPUs in the mask check if they are boosted if not add
> + * a request
> + */
> + for_each_cpu(cpu, mask) {
> + if (cpumask_test_cpu(cpu, dcmi->boosted))
> + continue;
> +
> + p = cpufreq_cpu_get(cpu);
> + if (!p)
> + continue;
> +
> + freq_qos_add_request(&p->constraints, &per_cpu(qos_req, cpu),
> + FREQ_QOS_MIN, dcmi->min_frequency);
> + cpumask_or(dcmi->boosted, dcmi->boosted, p->cpus);
> + cpufreq_cpu_put(p);
> + }
> +
> + mutex_unlock(&dcmi->freq_lock);
That looks about right.
> +}
> +
> +static void dcmi_irq_notifier_release(struct kref *ref)
> +{
> + /*
> + * This is required by affinity notifier. We don't have anything to
> + * free here.
> + */
> +}
> +
> +static void dcmi_get_cpu_policy(struct stm32_dcmi *dcmi)
> +{
> + struct cpufreq_policy *p;
> + int cpu;
> +
> + if (!alloc_cpumask_var(&dcmi->boosted, GFP_KERNEL))
> + return;
I think you want to actually return i.e. -ENOMEM and do cleanups in the
probe; otherwise you'll silently continue.
> +
> + mutex_lock(&dcmi->freq_lock);
> +
> + for_each_cpu(cpu, irq_get_affinity_mask(dcmi->irq)) {
When I suggested serialization, I was thinking we may want to use the irq's
desc lock to prevent the affinity from moving under our feet. Something
like:
CPU A CPU B
for_each_cpu(cpu, mask)
cpu = cpumask_next(cpu, mask)
// ... cpumask_copy(desc->irq_common_data.affinity, mask)
cpu = cpumask_next(cpu, mask)
Now, should that happen, we would still queue the notifier and run it
shortly after - and since you track which CPUs are boosted, I don't think
we have any loss of information here.
We may have yet another affinity change while the notifier is still queued;
but the notifier boilerplate does grab the desc lock, so I think it's all
good - it wasn't all super obvious so I figured I'd still point it out.
> + if (cpumask_test_cpu(cpu, dcmi->boosted))
> + continue;
> +
> + p = cpufreq_cpu_get(cpu);
> + if (!p)
> + continue;
> +
> + freq_qos_add_request(&p->constraints, &per_cpu(qos_req, cpu),
> + FREQ_QOS_MIN, FREQ_QOS_MIN_DEFAULT_VALUE);
> +
> + cpumask_or(dcmi->boosted, dcmi->boosted, p->cpus);
> +
> + cpufreq_cpu_put(p);
> + }
> +
> + mutex_unlock(&dcmi->freq_lock);
> +}
> +
> +static void dcmi_put_cpu_policy(struct stm32_dcmi *dcmi)
> +{
> + struct cpufreq_policy *p;
> + int cpu;
> +
> + mutex_lock(&dcmi->freq_lock);
> +
> + for_each_cpu(cpu, irq_get_affinity_mask(dcmi->irq)) {
> + if (!cpumask_test_cpu(cpu, dcmi->boosted))
> + continue;
> +
> + p = cpufreq_cpu_get(cpu);
> + if (!p)
> + continue;
> +
> + freq_qos_remove_request(&per_cpu(qos_req, cpu));
> + cpumask_andnot(dcmi->boosted, dcmi->boosted, p->cpus);
> +
> + cpufreq_cpu_put(p);
> + }
> +
> + free_cpumask_var(dcmi->boosted);
> +
> + mutex_unlock(&dcmi->freq_lock);
> +}
> +
> +static void dcmi_set_min_frequency(struct stm32_dcmi *dcmi, s32 freq)
> +{
> + struct irq_affinity_notify *notify = &dcmi->notify;
> + int cpu;
> +
> + mutex_lock(&dcmi->freq_lock);
> +
> + for_each_cpu(cpu, irq_get_affinity_mask(dcmi->irq)) {
> + if (!cpumask_test_cpu(cpu, dcmi->boosted))
> + continue;
> +
If the affinity changed between say .probe() and .start_streaming(), IIUC
you may skip CPUs here - and even if you initialize the notifier earlier in
the function (see below), that won't help you.
I think dcmi_irq_notifier_notify() does almost all you want, if it also did
the QoS update for CPUs that weren't affected by the affinity change, you
may be able to just do:
dcmi_irq_notifier_notify(irq_get_affinity_mask(dcmi->irq));
Or something along those lines.
> + if (!freq_qos_request_active(&per_cpu(qos_req, cpu)))
> + continue;
> +
> + freq_qos_update_request(&per_cpu(qos_req, cpu), freq);
> + }
> +
> + mutex_unlock(&dcmi->freq_lock);
> +
> + if (freq != FREQ_QOS_MIN_DEFAULT_VALUE) {
_______________________________________________
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 20/25] clk: bcm: rpi: Add an enum for the firmware clocks
From: Nicolas Saenz Julienne @ 2020-06-05 12:04 UTC (permalink / raw)
To: Maxime Ripard
Cc: Tim Gover, Dave Stevenson, linux-kernel, bcm-kernel-feedback-list,
linux-rpi-kernel, Phil Elwell, linux-arm-kernel
In-Reply-To: <c56fb0a912fe254416ed5a247e6fb6d79fb604bc.1590594293.git-series.maxime@cerno.tech>
[-- Attachment #1.1: Type: text/plain, Size: 2155 bytes --]
On Wed, 2020-05-27 at 17:45 +0200, Maxime Ripard wrote:
> While the firmware allows us to discover the available clocks, we need to
> discriminate those clocks to only register the ones meaningful to Linux.
> The firmware also doesn't provide a clock name, so having a list of the ID
> will help us to give clocks a proper name later on.
>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
> drivers/clk/bcm/clk-raspberrypi.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-
> raspberrypi.c
> index 5f4e2d49432f..eebd16040f8a 100644
> --- a/drivers/clk/bcm/clk-raspberrypi.c
> +++ b/drivers/clk/bcm/clk-raspberrypi.c
> @@ -18,7 +18,23 @@
>
> #include <soc/bcm2835/raspberrypi-firmware.h>
>
> -#define RPI_FIRMWARE_ARM_CLK_ID 0x00000003
> +enum rpi_firmware_clk_id {
> + RPI_FIRMWARE_EMMC_CLK_ID = 1,
> + RPI_FIRMWARE_UART_CLK_ID,
> + RPI_FIRMWARE_ARM_CLK_ID,
> + RPI_FIRMWARE_CORE_CLK_ID,
> + RPI_FIRMWARE_V3D_CLK_ID,
> + RPI_FIRMWARE_H264_CLK_ID,
> + RPI_FIRMWARE_ISP_CLK_ID,
> + RPI_FIRMWARE_SDRAM_CLK_ID,
> + RPI_FIRMWARE_PIXEL_CLK_ID,
> + RPI_FIRMWARE_PWM_CLK_ID,
> + RPI_FIRMWARE_HEVC_CLK_ID,
> + RPI_FIRMWARE_EMMC2_CLK_ID,
> + RPI_FIRMWARE_M2MC_CLK_ID,
> + RPI_FIRMWARE_PIXEL_BVB_CLK_ID,
> + RPI_FIRMWARE_NUM_CLK_ID,
> +};
>
> #define RPI_FIRMWARE_STATE_ENABLE_BIT BIT(0)
> #define RPI_FIRMWARE_STATE_WAIT_BIT BIT(1)
> @@ -31,8 +47,6 @@
>
> #define A2W_PLL_FRAC_BITS 20
>
> -#define NUM_FW_CLKS 16
> -
> struct raspberrypi_clk {
> struct device *dev;
> struct rpi_firmware *firmware;
> @@ -320,7 +334,8 @@ static int raspberrypi_clk_probe(struct platform_device
> *pdev)
> rpi->firmware = firmware;
> platform_set_drvdata(pdev, rpi);
>
> - clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, NUM_FW_CLKS),
> + clk_data = devm_kzalloc(dev, struct_size(clk_data, hws,
> + RPI_FIRMWARE_NUM_CLK_ID),
nit: you're allocating one structure too many right?
Acked-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Regards,
Nicolas
[-- Attachment #1.2: This is a digitally signed message part --]
[-- 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: [PATCH v4 06/11] gpio: add support for the sl28cpld GPIO controller
From: Andy Shevchenko @ 2020-06-05 12:00 UTC (permalink / raw)
To: Michael Walle
Cc: devicetree, Linus Walleij, Thierry Reding, Lee Jones,
Jason Cooper, Andy Shevchenko, Marc Zyngier, Bartosz Golaszewski,
Uwe Kleine-König, Guenter Roeck, linux-pwm, Jean Delvare,
linux-watchdog, open list:GPIO SUBSYSTEM, Mark Brown,
Thomas Gleixner, Wim Van Sebroeck, linux-arm Mailing List,
linux-hwmon, Greg Kroah-Hartman, Linux Kernel Mailing List,
Li Yang, Rob Herring, Shawn Guo
In-Reply-To: <20200604211039.12689-7-michael@walle.cc>
On Fri, Jun 5, 2020 at 12:14 AM Michael Walle <michael@walle.cc> wrote:
> Add support for the GPIO controller of the sl28 board management
> controller. This driver is part of a multi-function device.
>
> A controller has 8 lines. There are three different flavors:
> full-featured GPIO with interrupt support, input-only and output-only.
...
> +#include <linux/of_device.h>
I think also not needed.
But wait...
> + return devm_regmap_add_irq_chip_np(dev, dev_of_node(dev), regmap,
It seems regmap needs to be converted to use fwnode.
> + irq, IRQF_SHARED | IRQF_ONESHOT, 0,
> + irq_chip, &gpio->irq_data);
...
> + if (!pdev->dev.parent)
> + return -ENODEV;
Are we expecting to get shot into foot? I mean why we need this check?
> + dev_id = platform_get_device_id(pdev);
> + if (dev_id)
> + type = dev_id->driver_data;
Oh, no. In new code we don't need this. We have facilities to provide
platform data in a form of fwnode.
> + else
> + type = (uintptr_t)of_device_get_match_data(&pdev->dev);
So does this. device_get_match_data().
> + if (!type)
> + return -ENODEV;
...
> + if (irq_support &&
Why do you need this flag? Can't simple IRQ number be sufficient?
> + device_property_read_bool(&pdev->dev, "interrupt-controller")) {
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> + ret = sl28cpld_gpio_irq_init(&pdev->dev, gpio, regmap,
> + base, irq);
> + if (ret)
> + return ret;
> +
> + config.irq_domain = regmap_irq_get_domain(gpio->irq_data);
> + }
...
> +static const struct of_device_id sl28cpld_gpio_of_match[] = {
> + { .compatible = "kontron,sl28cpld-gpio",
> + .data = (void *)SL28CPLD_GPIO },
> + { .compatible = "kontron,sl28cpld-gpi",
> + .data = (void *)SL28CPLD_GPI },
> + { .compatible = "kontron,sl28cpld-gpo",
> + .data = (void *)SL28CPLD_GPO },
All above can be twice less LOCs.
> + {},
No comma.
> +};
...
> + .name = KBUILD_MODNAME,
This actually not good idea in long term. File name can change and break an ABI.
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] efi/arm: decompressor: deal with HYP mode boot gracefully
From: Ard Biesheuvel @ 2020-06-05 11:52 UTC (permalink / raw)
To: linux-efi; +Cc: Ard Biesheuvel, maz, linux, linux-arm-kernel, xypron.glpk
EFI on ARM only supports short descriptors, and given that it mandates
that the MMU and caches are on, it is implied that booting in HYP mode
is not supported.
However, implementations of EFI exist (i.e., U-Boot) that ignore this
requirement, which is not entirely unreasonable, given that it does
not make a lot of sense to begin with.
So let's make sure that we can deal with this condition gracefully.
We already tolerate booting the EFI stub with the caches off (even
though this violates the EFI spec as well), and so we should deal
with HYP mode boot with MMU and caches either on or off.
- When the MMU and caches are on, we can ignore the HYP stub altogether,
since we can just use the existing mappings, and just branch into
the decompressed kernel as usual after disabling MMU and caches.
- When the MMU and caches are off, we have to drop to SVC mode so that
we can set up the page tables using short descriptors. In this case,
we need to install the HYP stub so that we can return to HYP mode
when handing over to the kernel proper.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/boot/compressed/head.S | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index c79db44ba128..986db86ba334 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -1436,6 +1436,35 @@ ENTRY(efi_enter_kernel)
mrc p15, 0, r0, c1, c0, 0 @ read SCTLR
tst r0, #0x1 @ MMU enabled?
orreq r4, r4, #1 @ set LSB if not
+#ifdef CONFIG_ARM_VIRT_EXT
+ @
+ @ The EFI spec does not support booting on ARM in HYP mode,
+ @ since it mandates that the MMU and caches are on, with all
+ @ 32-bit addressable DRAM mapped 1:1 using short descriptors.
+ @ While the EDK2 reference implementation adheres to this,
+ @ U-Boot might decide to enter the EFI stub in HYP mode anyway,
+ @ with the MMU and caches either on or off.
+ @ In the former case, we're better off just carrying on using
+ @ the cached 1:1 mapping that the firmware provided, and pretend
+ @ that we are in SVC mode as far as the decompressor is
+ @ concerned. However, if the caches are off, we need to drop
+ @ into SVC mode now, and let the decompressor set up its cached
+ @ 1:1 mapping as usual.
+ @
+ mov r0, #SVC_MODE
+ msr spsr_cxsf, r0 @ record that we are in SVC mode
+ bne 1f @ skip HYP stub install if MMU is on
+
+ mov r9, r4 @ preserve image base
+ bl __hyp_stub_install @ returns boot mode in r4
+ cmp r4, #HYP_MODE @ did we boot in HYP?
+ bne 1f @ skip drop to SVC if we did not
+
+ safe_svcmode_maskall r0
+ msr spsr_cxsf, r4 @ record boot mode
+ mov r4, r9 @ restore image base
+1:
+#endif
mov r0, r8 @ DT start
add r1, r8, r2 @ DT end
--
2.26.2
_______________________________________________
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 v4 02/11] mfd: Add support for Kontron sl28cpld management controller
From: Michael Walle @ 2020-06-05 11:51 UTC (permalink / raw)
To: Andy Shevchenko
Cc: devicetree, Linus Walleij, Thierry Reding, Lee Jones,
Jason Cooper, Andy Shevchenko, Marc Zyngier, Bartosz Golaszewski,
Uwe Kleine-König, Guenter Roeck, linux-pwm, Jean Delvare,
linux-watchdog, open list:GPIO SUBSYSTEM, Mark Brown,
Thomas Gleixner, Wim Van Sebroeck, linux-arm Mailing List,
linux-hwmon, Greg Kroah-Hartman, Linux Kernel Mailing List,
Li Yang, Rob Herring, Shawn Guo
In-Reply-To: <CAHp75Vf00w_UUvXULVd=OgSVM+p_pmNMJRPVnf8GNZW10c_j5w@mail.gmail.com>
Am 2020-06-05 12:48, schrieb Andy Shevchenko:
> On Fri, Jun 5, 2020 at 1:09 PM Michael Walle <michael@walle.cc> wrote:
>> Am 2020-06-05 10:01, schrieb Andy Shevchenko:
>> > On Fri, Jun 5, 2020 at 12:16 AM Michael Walle <michael@walle.cc> wrote:
>
> ...
>
>> >> + bool "Kontron sl28 core driver"
>> >> + depends on I2C=y
>> >
>> > Why not module?
>>
>> There are users of the interupt lines provided by the interrupt
>> controller.
>> For example, the gpio-button driver. If this is compiled into the
>> kernel
>> (which it is by default in the arm64 defconfig), probing will fail
>> because
>> the interrupt is not found. Is there a better way for that? I guess
>> the
>> same
>> is true for the GPIO driver.
>
> And GPIO nicely handles this via deferred probe mechanism. Why it
> can't be used here?
> So, we really need to have a strong argument to limit module nowadays
> to be only builtin.
Was that a question for me? TBH thats how other interrupt drivers doing
it for now. And it would be the users who need to be fixed, right? Or
even the platform code? Because it will complain with
[ 2.962990] irq: no irq domain found for interrupt-controller@1c !
[ 2.975762] gpio-keys buttons0: Found button without gpio or irq
[ 2.981872] gpio-keys: probe of buttons0 failed with error -22
>> >> + depends on OF
>> >
>> > I didn't find an evidence this is needed.
>
>> >> +#include <linux/of_platform.h>
>> >
>> > No evidence of user of this.
>> > I think you meant mod_devicetable.h.
>>
>> devm_of_platform_populate(), so I need CONFIG_OF, too right?
>
> Ah, this explains header, thanks!
> But it doesn't explain depends OF.
>
> So, perhaps,
>
> depends OF || COMPILE_TEST will be more informative, i.e.
> tells "okay, this driver can be compiled w/o OF, but won't be
> functional".
ok
--
-michael
_______________________________________________
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 v4 1/6] PCI: dwc: Add msi_host_isr() callback
From: Gustavo Pimentel @ 2020-06-05 11:44 UTC (permalink / raw)
To: Kunihiko Hayashi, Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han,
Rob Herring, Masahiro Yamada, Marc Zyngier
Cc: devicetree@vger.kernel.org, Masami Hiramatsu,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
Jassi Brar, linux-arm-kernel@lists.infradead.org
In-Reply-To: <1591350276-15816-2-git-send-email-hayashi.kunihiko@socionext.com>
On Fri, Jun 5, 2020 at 10:44:31, Kunihiko Hayashi
<hayashi.kunihiko@socionext.com> wrote:
> This adds msi_host_isr() callback function support to describe
> SoC-dependent service triggered by MSI.
>
> For example, when AER interrupt is triggered by MSI, the callback function
> reads SoC-dependent registers and detects that the interrupt is from AER,
> and invoke AER interrupts related to MSI.
>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
> drivers/pci/controller/dwc/pcie-designware-host.c | 3 +++
> drivers/pci/controller/dwc/pcie-designware.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 0a4a5aa..026edb1 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -83,6 +83,9 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
> u32 status, num_ctrls;
> irqreturn_t ret = IRQ_NONE;
>
> + if (pp->ops->msi_host_isr)
> + pp->ops->msi_host_isr(pp);
> +
> num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
>
> for (i = 0; i < num_ctrls; i++) {
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 656e00f..e741967 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -170,6 +170,7 @@ struct dw_pcie_host_ops {
> void (*scan_bus)(struct pcie_port *pp);
> void (*set_num_vectors)(struct pcie_port *pp);
> int (*msi_host_init)(struct pcie_port *pp);
> + void (*msi_host_isr)(struct pcie_port *pp);
> };
>
> struct pcie_port {
> --
> 2.7.4
Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
_______________________________________________
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 v4 05/11] pwm: add support for sl28cpld PWM controller
From: Michael Walle @ 2020-06-05 11:39 UTC (permalink / raw)
To: Lee Jones
Cc: devicetree, Linus Walleij, Thierry Reding, Jason Cooper,
Andy Shevchenko, Marc Zyngier, Bartosz Golaszewski,
Uwe Kleine-König, Guenter Roeck, linux-pwm, Jean Delvare,
linux-watchdog, linux-gpio, Mark Brown, Thomas Gleixner,
Wim Van Sebroeck, linux-arm-kernel, linux-hwmon,
Greg Kroah-Hartman, linux-kernel, Li Yang, Rob Herring, Shawn Guo
In-Reply-To: <20200605084915.GE3714@dell>
Am 2020-06-05 10:49, schrieb Lee Jones:
> On Thu, 04 Jun 2020, Michael Walle wrote:
>
>> Add support for the PWM controller of the sl28cpld board management
>> controller. This is part of a multi-function device driver.
>>
>> The controller has one PWM channel and can just generate four distinct
>> frequencies.
>>
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>> drivers/pwm/Kconfig | 10 ++
>> drivers/pwm/Makefile | 1 +
>> drivers/pwm/pwm-sl28cpld.c | 201
>> +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 212 insertions(+)
>> create mode 100644 drivers/pwm/pwm-sl28cpld.c
>>
>> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
>> index cb8d739067d2..a39371c11ff6 100644
>> --- a/drivers/pwm/Kconfig
>> +++ b/drivers/pwm/Kconfig
>> @@ -437,6 +437,16 @@ config PWM_SIFIVE
>> To compile this driver as a module, choose M here: the module
>> will be called pwm-sifive.
>>
>> +config PWM_SL28CPLD
>> + tristate "Kontron sl28 PWM support"
>> + depends on MFD_SL28CPLD
>> + help
>> + Generic PWM framework driver for board management controller
>> + found on the Kontron sl28 CPLD.
>> +
>> + To compile this driver as a module, choose M here: the module
>> + will be called pwm-sl28cpld.
>> +
>> config PWM_SPEAR
>> tristate "STMicroelectronics SPEAr PWM support"
>> depends on PLAT_SPEAR || COMPILE_TEST
>> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
>> index a59c710e98c7..c479623724e8 100644
>> --- a/drivers/pwm/Makefile
>> +++ b/drivers/pwm/Makefile
>> @@ -41,6 +41,7 @@ obj-$(CONFIG_PWM_RENESAS_TPU) += pwm-renesas-tpu.o
>> obj-$(CONFIG_PWM_ROCKCHIP) += pwm-rockchip.o
>> obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
>> obj-$(CONFIG_PWM_SIFIVE) += pwm-sifive.o
>> +obj-$(CONFIG_PWM_SL28CPLD) += pwm-sl28cpld.o
>> obj-$(CONFIG_PWM_SPEAR) += pwm-spear.o
>> obj-$(CONFIG_PWM_SPRD) += pwm-sprd.o
>> obj-$(CONFIG_PWM_STI) += pwm-sti.o
>> diff --git a/drivers/pwm/pwm-sl28cpld.c b/drivers/pwm/pwm-sl28cpld.c
>> new file mode 100644
>> index 000000000000..d82303f509f5
>> --- /dev/null
>> +++ b/drivers/pwm/pwm-sl28cpld.c
>> @@ -0,0 +1,201 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * sl28cpld PWM driver.
>> + *
>> + * Copyright 2019 Kontron Europe GmbH
>
> This is out of date.
ok
>
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pwm.h>
>> +#include <linux/regmap.h>
>> +
>> +/*
>> + * PWM timer block registers.
>> + */
>> +#define PWM_CTRL 0x00
>> +#define PWM_ENABLE BIT(7)
>> +#define PWM_MODE_250HZ 0
>> +#define PWM_MODE_500HZ 1
>> +#define PWM_MODE_1KHZ 2
>> +#define PWM_MODE_2KHZ 3
>> +#define PWM_MODE_MASK GENMASK(1, 0)
>> +#define PWM_CYCLE 0x01
>> +#define PWM_CYCLE_MAX 0x7f
>> +
>> +struct sl28cpld_pwm {
>> + struct pwm_chip pwm_chip;
>> + struct regmap *regmap;
>> + u32 offset;
>> +};
>> +
>> +struct sl28cpld_pwm_periods {
>> + u8 ctrl;
>> + unsigned long duty_cycle;
>> +};
>> +
>> +struct sl28cpld_pwm_config {
>> + unsigned long period_ns;
>> + u8 max_duty_cycle;
>> +};
>
> Also, instead of hand rolling your own structure here, I think it
> would be prudent to re-use something that already exists. Seeing as
> this will be used to describe possible state, perhaps 'struct
> pwm_state' would be suitable - leaving polarity and enabled
> unpopulated of course.
>
> Ah wait (sorry, thinking allowed and on-the-fly here), what is
> max_duty_cycle here? I assume this does not have the same
> meaning/value-type as the one in 'struct pwm_state'. What does
> max_duty_cycle represent in your use-case?
Its the max value of the PWM_CYCLE register, with one exception
of the 250Hz mode. There it would be 0x7f; but it is used as a scaling
factor too. Thus I added the "fixup" below.
>> +static struct sl28cpld_pwm_config sl28cpld_pwm_config[] = {
>> + [PWM_MODE_250HZ] = { .period_ns = 4000000, .max_duty_cycle = 0x80 },
>> + [PWM_MODE_500HZ] = { .period_ns = 2000000, .max_duty_cycle = 0x40 },
>> + [PWM_MODE_1KHZ] = { .period_ns = 1000000, .max_duty_cycle = 0x20 },
>> + [PWM_MODE_2KHZ] = { .period_ns = 500000, .max_duty_cycle = 0x10 },
>> +};
>
> Tiny nit: If you lined these up from the '{'s it would be easier to
> see/compare the period_ns values at first glance, rather than having
> to count the ' 's and '0's.
yep.
>
>> +static inline struct sl28cpld_pwm *to_sl28cpld_pwm(struct pwm_chip
>> *chip)
>> +{
>> + return container_of(chip, struct sl28cpld_pwm, pwm_chip);
>> +}
>
> Why not save yourself the trouble and just:
>
> struct sl28cpld_pwm *pwm = dev_get_drvdata(chip->dev);
looks better, yes.
>
>> +static void sl28cpld_pwm_get_state(struct pwm_chip *chip,
>> + struct pwm_device *pwm,
>> + struct pwm_state *state)
>> +{
>> + struct sl28cpld_pwm *spc = to_sl28cpld_pwm(chip);
>> + static struct sl28cpld_pwm_config *config;
>> + unsigned int reg;
>> + unsigned long cycle;
>
> Why is this 'long' here and 'long long' in *_apply()?
cycle has a max value of "u8_max * <defined ulong from config above>",
where below it might be "ulong * ulong". But for consinstency, I could
make it unsigned long long here, too.
>> + unsigned int mode;
>> +
>> + regmap_read(spc->regmap, spc->offset + PWM_CTRL, ®);
>> +
>> + state->enabled = reg & PWM_ENABLE;
>> +
>> + mode = FIELD_GET(PWM_MODE_MASK, reg);
>> + config = &sl28cpld_pwm_config[mode];
>> + state->period = config->period_ns;
>> +
>> + regmap_read(spc->regmap, spc->offset + PWM_CYCLE, ®);
>> + cycle = reg * config->period_ns;
>> + state->duty_cycle = DIV_ROUND_CLOSEST_ULL(cycle,
>> + config->max_duty_cycle);
>> +}
>> +
>> +static int sl28cpld_pwm_apply(struct pwm_chip *chip, struct
>> pwm_device *pwm,
>> + const struct pwm_state *state)
>> +{
>> + struct sl28cpld_pwm *spc = to_sl28cpld_pwm(chip);
>> + struct sl28cpld_pwm_config *config;
>> + unsigned long long cycle;
>> + int ret;
>> + int mode;
>> + u8 ctrl;
>> +
>> + /* update config, first search best matching period */
>
> Please use correct grammar (less full stops) in comments.
ok
>
>> + for (mode = 0; mode < ARRAY_SIZE(sl28cpld_pwm_config); mode++) {
>> + config = &sl28cpld_pwm_config[mode];
>> + if (state->period == config->period_ns)
>> + break;
>> + }
>> +
>> + if (mode == ARRAY_SIZE(sl28cpld_pwm_config))
>> + return -EINVAL;
>> +
>> + ctrl = FIELD_PREP(PWM_MODE_MASK, mode);
>> + if (state->enabled)
>> + ctrl |= PWM_ENABLE;
>> +
>> + cycle = state->duty_cycle * config->max_duty_cycle;
>> + do_div(cycle, state->period);
>
> Forgive my ignorance (I'm new here!), but what are these 2 lines
> doing? Here we are multiplying the current duty_cycle with the
> maximum value, then dividing by the period.
>
> So in the case of PWM_MODE_1KHZ with a 50% duty cycle, you'd have:
>
> (500000 * 0x20[16]) / 1000000 = [0x10]16
>
> Thus, the above gives as a proportional representation of the maximum
> valid value for placement into the cycle control register(s), right?
correct.
> Either way (whether I'm correct or not), I think it would be nice to
> mention this in a comment. Maybe even clarify with a simple example.
yes, I'll also look into the helper Andy mentioned. Thus it might be
even self explanatory.
>> + /*
>> + * The hardware doesn't allow to set max_duty_cycle if the
>> + * 250Hz mode is enabled. But since this is "all-high" output
>> + * just use the 500Hz mode with the duty cycle to max value.
>> + */
>> + if (cycle == config->max_duty_cycle) {
>> + ctrl &= ~PWM_MODE_MASK;
>> + ctrl |= FIELD_PREP(PWM_MODE_MASK, PWM_MODE_500HZ);
>> + cycle = PWM_CYCLE_MAX;
>> + }
>
> This is being executed even when 250Hz mode is not enabled.
>
> Is that by design?
Yes because the mode doesn't matter if you have a duty cycle of 100%.
You'd be free to choose any mode except 250Hz.
> If so, it doesn't match the comment.
Mh? Ok its a bit confusing and it might imply that this is only done
for the 250Hz case. But it doensn't mention it is _only_ used for this
mode.
/*
* The hardware doesn't allow to set max_duty_cycle if the
* 250Hz mode is enabled, thus we have to trap that here.
* But because a 100% duty cycle is equal on all modes, i.e.
* it is just a "all-high" output, we trap any case with a
* 100% duty cycle and use the 500Hz mode.
*/
>> + ret = regmap_write(spc->regmap, spc->offset + PWM_CTRL, ctrl);
>> + if (ret)
>> + return ret;
>> +
>> + return regmap_write(spc->regmap, spc->offset + PWM_CYCLE,
>> (u8)cycle);
>> +}
>> +
>> +static const struct pwm_ops sl28cpld_pwm_ops = {
>> + .apply = sl28cpld_pwm_apply,
>> + .get_state = sl28cpld_pwm_get_state,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static int sl28cpld_pwm_probe(struct platform_device *pdev)
>> +{
>> + struct sl28cpld_pwm *pwm;
>
> This is super confusing. Here you call it 'pwm', but when you bring
> the data to the fore for consumption, you call it something different
> ('spc') for some reason.
yeah it is :(
> Is there logic behind this?
And no it is not. sorry for that.
>> + struct pwm_chip *chip;
>> + int ret;
>> +
>> + if (!pdev->dev.parent)
>> + return -ENODEV;
>> +
>> + pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
>> + if (!pwm)
>> + return -ENOMEM;
>> +
>> + pwm->regmap = dev_get_regmap(pdev->dev.parent, NULL);
>> + if (!pwm->regmap)
>> + return -ENODEV;
>> +
>> + ret = device_property_read_u32(&pdev->dev, "reg", &pwm->offset);
>
> Really? Can you use the 'reg' property in this way?
Well formerly it was IORESOURCE_REG, which gives you a register offset,
see commit 72dcb1197228b ("resources: Add register address resource
type").
There is also the of_get_address(), but I doubt that would be correct
here,
because it does bus mapping etc.
So I looked at how other MFD drivers does it, most of the MFD have the
advantage of having fixed register offsets and then just use hardcoded
offsets. But there are some drivers which pull their offset out of the
reg property from the device tree itself.
$ grep -r "read_u32.*\"reg\"" drivers/
$ grep -r "read_u32.*\"reg\".*base" drivers/
Does anyone have a better idea?
> Side question:
> Do any of your child address spaces actually overlap/intersect?
nope. they are distinct.
>
>> + if (ret)
>> + return -EINVAL;
>> +
>> + /* initialize struct pwm_chip */
>
> Proper grammar please.
ok
>
>> + chip = &pwm->pwm_chip;
>> + chip->dev = &pdev->dev;
>> + chip->ops = &sl28cpld_pwm_ops;
>> + chip->base = -1;
>> + chip->npwm = 1;
>> +
>> + ret = pwmchip_add(&pwm->pwm_chip);
>> + if (ret < 0)
>
> Is '> 0' even valid?
>
> Suggest "!ret" here, as you have done above.
Yes, same comment as Andy had on the other patches.
>> + return ret;
>> +
>> + platform_set_drvdata(pdev, pwm);
>> +
>> + return 0;
>> +}
>> +
>> +static int sl28cpld_pwm_remove(struct platform_device *pdev)
>> +{
>> + struct sl28cpld_pwm *pwm = platform_get_drvdata(pdev);
>> +
>> + return pwmchip_remove(&pwm->pwm_chip);
>> +}
>> +
>> +static const struct of_device_id sl28cpld_pwm_of_match[] = {
>> + { .compatible = "kontron,sl28cpld-pwm" },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, sl28cpld_pwm_of_match);
>> +
>> +static const struct platform_device_id sl28cpld_pwm_id_table[] = {
>> + {"sl28cpld-pwm"},
>
> Spaces either side of the "'s please.
ok
>
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(platform, sl28cpld_pwm_id_table);
>
> What are you using this for?
They are from the time when these drivers were mfd_cells. But I wanted
to keep them here, if in the future there is another mfd driver which
uses these drivers.
>> +static struct platform_driver sl28cpld_pwm_driver = {
>> + .probe = sl28cpld_pwm_probe,
>> + .remove = sl28cpld_pwm_remove,
>> + .id_table = sl28cpld_pwm_id_table,
>> + .driver = {
>> + .name = KBUILD_MODNAME,
>
> Please just use the quoted name in full.
Mhh, is there any rule for this? Sometimes KBUILD_MODNAME is used
and sometimes an hardcoded name. I thought KBUILD_MODNAME is nice
because it is filled automatically. And the platform probe use
the .id_table anyway.
>> + .of_match_table = sl28cpld_pwm_of_match,
>> + },
>> +};
>> +module_platform_driver(sl28cpld_pwm_driver);
>> +
>> +MODULE_DESCRIPTION("sl28cpld PWM Driver");
>
> "SL28CPLD" ?
Actually no, I want to call that "sl28cpld". The first part
is "sl28" (not SL28) and sl28CPLD looks pretty weird. I tried
to be consistent on Kconfig/dt-bindings/drivers about this
naming.
>> +MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
>> +MODULE_LICENSE("GPL");
--
-michael
_______________________________________________
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] spi: bcm2835: Enable shared interrupt support
From: Robin Murphy @ 2020-06-05 11:34 UTC (permalink / raw)
To: Florian Fainelli, linux-kernel
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Scott Branden, lukas, Ray Jui, Mark Brown,
open list:SPI SUBSYSTEM, Nicolas Saenz Julienne, Rob Herring,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Martin Sperl,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
In-Reply-To: <20200604212819.715-1-f.fainelli@gmail.com>
On 2020-06-04 22:28, Florian Fainelli wrote:
> The 4 SPI controller instances added in BCM2711 and BCM7211 SoCs (SPI3,
> SPI4, SPI5 and SPI6) share the same interrupt line with SPI0.
>
> For the BCM2835 case which is deemed performance critical, we would like
> to continue using an interrupt handler which does not have the extra
> comparison on BCM2835_SPI_CS_INTR.
FWIW, if I'm reading the patch correctly, then with sensible codegen
that "overhead" should amount to a bit test on a live register plus a
not-taken conditional branch - according to the 1176 TRM that should add
up to a whopping 2 cycles. If that's really significant then I'd have to
wonder whether you want to be at the mercy of the whole generic IRQ
stack at all, and should perhaps consider using FIQ instead.
> To support that requirement the common interrupt handling code between
> the shared and non-shared interrupt paths is split into a
> bcm2835_spi_interrupt_common() and both bcm2835_spi_interrupt() as well
> as bcm2835_spi_shared_interrupt() make use of it.
>
> During probe, we determine if there is at least another instance of this
> SPI controller, and if there is, then we install a shared interrupt
> handler.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Changes in v2:
>
> - identify other available SPI nodes to determine if we need to set-up
> interrupt sharing. This needs to happen for the very first instance
> since we cannot know for the first instance whether interrupt sharing
> is needed or not.
>
> drivers/spi/spi-bcm2835.c | 61 ++++++++++++++++++++++++++++++++-------
> 1 file changed, 50 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
> index 237bd306c268..0288b5b3de1e 100644
> --- a/drivers/spi/spi-bcm2835.c
> +++ b/drivers/spi/spi-bcm2835.c
> @@ -361,11 +361,10 @@ static void bcm2835_spi_reset_hw(struct spi_controller *ctlr)
> bcm2835_wr(bs, BCM2835_SPI_DLEN, 0);
> }
>
> -static irqreturn_t bcm2835_spi_interrupt(int irq, void *dev_id)
> +static inline irqreturn_t bcm2835_spi_interrupt_common(struct spi_controller *ctlr,
> + u32 cs)
> {
> - struct spi_controller *ctlr = dev_id;
> struct bcm2835_spi *bs = spi_controller_get_devdata(ctlr);
> - u32 cs = bcm2835_rd(bs, BCM2835_SPI_CS);
>
> /*
> * An interrupt is signaled either if DONE is set (TX FIFO empty)
> @@ -394,6 +393,27 @@ static irqreturn_t bcm2835_spi_interrupt(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static irqreturn_t bcm2835_spi_interrupt(int irq, void *dev_id)
> +{
> + struct spi_controller *ctlr = dev_id;
> + struct bcm2835_spi *bs = spi_controller_get_devdata(ctlr);
> + u32 cs = bcm2835_rd(bs, BCM2835_SPI_CS);
> +
> + return bcm2835_spi_interrupt_common(ctlr, cs);
> +}
> +
> +static irqreturn_t bcm2835_spi_shared_interrupt(int irq, void *dev_id)
> +{
> + struct spi_controller *ctlr = dev_id;
> + struct bcm2835_spi *bs = spi_controller_get_devdata(ctlr);
> + u32 cs = bcm2835_rd(bs, BCM2835_SPI_CS);
> +
> + if (!(cs & BCM2835_SPI_CS_INTR))
> + return IRQ_NONE;
> +
> + return bcm2835_spi_interrupt_common(ctlr, cs);
> +}
> +
> static int bcm2835_spi_transfer_one_irq(struct spi_controller *ctlr,
> struct spi_device *spi,
> struct spi_transfer *tfr,
> @@ -1287,12 +1307,37 @@ static int bcm2835_spi_setup(struct spi_device *spi)
> return 0;
> }
>
> +static const struct of_device_id bcm2835_spi_match[] = {
> + { .compatible = "brcm,bcm2835-spi", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, bcm2835_spi_match);
> +
> static int bcm2835_spi_probe(struct platform_device *pdev)
> {
> + irq_handler_t bcm2835_spi_isr_func = bcm2835_spi_interrupt;
> struct spi_controller *ctlr;
> + unsigned long flags = 0;
> + struct device_node *dn;
> struct bcm2835_spi *bs;
> int err;
>
> + /* On BCM2711 there can be multiple SPI controllers enabled sharing the
> + * same interrupt line, but we also want to minimize the overhead if
> + * there is no need to support interrupt sharing. If we find at least
> + * another available instane (not counting the one we are probed from),
> + * then we assume that interrupt sharing is necessary.
> + */
> + for_each_compatible_node(dn, NULL, bcm2835_spi_match[0].compatible) {
> + err = of_device_is_available(dn) && dn != pdev->dev.of_node;
> + of_node_put(dn);
This is in the wrong place - it should only be where you terminate the
loop early and thus bypass the "of_node_put(from)" call in the iterator
itself.
> + if (err) {
> + flags = IRQF_SHARED;
Is there really any harm to setting IRQF_SHARED even when the interrupt
isn't shared in hardware? Sure, it means you lose a degree of API-level
validation and some other driver might also be able to simultaneously
claim it on bcm283x, but in that case the DT would be wrong and
*something* isn't going to work correctly anyway, so does this one
driver really need to care about trying to be the DT police?
Robin.
> + bcm2835_spi_isr_func = bcm2835_spi_shared_interrupt;
> + break;
> + }
> + }
> +
> ctlr = spi_alloc_master(&pdev->dev, ALIGN(sizeof(*bs),
> dma_get_cache_alignment()));
> if (!ctlr)
> @@ -1344,8 +1389,8 @@ static int bcm2835_spi_probe(struct platform_device *pdev)
> bcm2835_wr(bs, BCM2835_SPI_CS,
> BCM2835_SPI_CS_CLEAR_RX | BCM2835_SPI_CS_CLEAR_TX);
>
> - err = devm_request_irq(&pdev->dev, bs->irq, bcm2835_spi_interrupt, 0,
> - dev_name(&pdev->dev), ctlr);
> + err = devm_request_irq(&pdev->dev, bs->irq, bcm2835_spi_isr_func,
> + flags, dev_name(&pdev->dev), ctlr);
> if (err) {
> dev_err(&pdev->dev, "could not request IRQ: %d\n", err);
> goto out_dma_release;
> @@ -1400,12 +1445,6 @@ static void bcm2835_spi_shutdown(struct platform_device *pdev)
> dev_err(&pdev->dev, "failed to shutdown\n");
> }
>
> -static const struct of_device_id bcm2835_spi_match[] = {
> - { .compatible = "brcm,bcm2835-spi", },
> - {}
> -};
> -MODULE_DEVICE_TABLE(of, bcm2835_spi_match);
> -
> static struct platform_driver bcm2835_spi_driver = {
> .driver = {
> .name = DRV_NAME,
>
_______________________________________________
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] spi: bcm2835: Enable shared interrupt support
From: Mark Brown @ 2020-06-05 11:14 UTC (permalink / raw)
To: linux-kernel, Florian Fainelli
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Scott Branden, Ray Jui, Rob Herring, open list:SPI SUBSYSTEM,
lukas,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Martin Sperl, Nicolas Saenz Julienne,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
In-Reply-To: <20200604212819.715-1-f.fainelli@gmail.com>
On Thu, 4 Jun 2020 14:28:19 -0700, Florian Fainelli wrote:
> The 4 SPI controller instances added in BCM2711 and BCM7211 SoCs (SPI3,
> SPI4, SPI5 and SPI6) share the same interrupt line with SPI0.
>
> For the BCM2835 case which is deemed performance critical, we would like
> to continue using an interrupt handler which does not have the extra
> comparison on BCM2835_SPI_CS_INTR.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[1/1] spi: bcm2835: Enable shared interrupt support
commit: ecfbd3cf3b8bb73ac6a80ddf430b5912fd4402a6
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
_______________________________________________
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] soc: xilinx: Fix error code in zynqmp_pm_probe()
From: Michal Simek @ 2020-06-05 11:07 UTC (permalink / raw)
To: Dan Carpenter, Michal Simek, Tejas Patel
Cc: Rajan Vaja, Greg Kroah-Hartman, kernel-janitors, linux-kernel,
Jolly Shah, linux-arm-kernel
In-Reply-To: <20200605110020.GA978434@mwanda>
On 05. 06. 20 13:00, Dan Carpenter wrote:
> This should be returning PTR_ERR() but it returns IS_ERR() instead.
>
> Fixes: ffdbae28d9d1 ("drivers: soc: xilinx: Use mailbox IPI callback")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/soc/xilinx/zynqmp_power.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/xilinx/zynqmp_power.c b/drivers/soc/xilinx/zynqmp_power.c
> index 31ff49fcd078b..c556623dae024 100644
> --- a/drivers/soc/xilinx/zynqmp_power.c
> +++ b/drivers/soc/xilinx/zynqmp_power.c
> @@ -205,7 +205,7 @@ static int zynqmp_pm_probe(struct platform_device *pdev)
> rx_chan = mbox_request_channel_byname(client, "rx");
> if (IS_ERR(rx_chan)) {
> dev_err(&pdev->dev, "Failed to request rx channel\n");
> - return IS_ERR(rx_chan);
> + return PTR_ERR(rx_chan);
> }
> } else if (of_find_property(pdev->dev.of_node, "interrupts", NULL)) {
> irq = platform_get_irq(pdev, 0);
>
Reviewed-by: Michal Simek <michal.simek@xilinx.com>
Thanks,
Michal
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [V7, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Dongchun Zhu @ 2020-06-05 10:54 UTC (permalink / raw)
To: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
mark.rutland, sakari.ailus, drinkcat, tfiga, matthias.bgg,
bingbu.cao
Cc: devicetree, srv_heupstream, shengnan.wang, sj.huang,
linux-mediatek, dongchun.zhu, louis.kuo, linux-arm-kernel,
linux-media
In-Reply-To: <20200605105412.18813-1-dongchun.zhu@mediatek.com>
Add a V4L2 sub-device driver for DW9768 voice coil motor, providing
control to set the desired focus via IIC serial interface.
Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
---
MAINTAINERS | 1 +
drivers/media/i2c/Kconfig | 13 ++
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/dw9768.c | 566 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 581 insertions(+)
create mode 100644 drivers/media/i2c/dw9768.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 8d72c41..c92dc99 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5157,6 +5157,7 @@ L: linux-media@vger.kernel.org
S: Maintained
T: git git://linuxtv.org/media_tree.git
F: Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
+F: drivers/media/i2c/dw9768.c
DONGWOON DW9807 LENS VOICE COIL DRIVER
M: Sakari Ailus <sakari.ailus@linux.intel.com>
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 125d596..afdf994 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -1040,6 +1040,19 @@ config VIDEO_DW9714
capability. This is designed for linear control of
voice coil motors, controlled via I2C serial interface.
+config VIDEO_DW9768
+ tristate "DW9768 lens voice coil support"
+ depends on I2C && VIDEO_V4L2
+ depends on PM
+ select MEDIA_CONTROLLER
+ select VIDEO_V4L2_SUBDEV_API
+ select V4L2_FWNODE
+ help
+ This is a driver for the DW9768 camera lens voice coil.
+ DW9768 is a 10 bit DAC with 100mA output current sink
+ capability. This is designed for linear control of
+ voice coil motors, controlled via I2C serial interface.
+
config VIDEO_DW9807_VCM
tristate "DW9807 lens voice coil support"
depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 77bf7d0..4057476 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
obj-$(CONFIG_VIDEO_AD5820) += ad5820.o
obj-$(CONFIG_VIDEO_AK7375) += ak7375.o
obj-$(CONFIG_VIDEO_DW9714) += dw9714.o
+obj-$(CONFIG_VIDEO_DW9768) += dw9768.o
obj-$(CONFIG_VIDEO_DW9807_VCM) += dw9807-vcm.o
obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
diff --git a/drivers/media/i2c/dw9768.c b/drivers/media/i2c/dw9768.c
new file mode 100644
index 0000000..f34a8ed
--- /dev/null
+++ b/drivers/media/i2c/dw9768.c
@@ -0,0 +1,566 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2020 MediaTek Inc.
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+#include <media/v4l2-async.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-subdev.h>
+
+#define DW9768_NAME "dw9768"
+#define DW9768_MAX_FOCUS_POS (1024 - 1)
+/*
+ * This sets the minimum granularity for the focus positions.
+ * A value of 1 gives maximum accuracy for a desired focus position
+ */
+#define DW9768_FOCUS_STEPS 1
+
+/*
+ * Ring control and Power control register
+ * Bit[1] RING_EN
+ * 0: Direct mode
+ * 1: AAC mode (ringing control mode)
+ * Bit[0] PD
+ * 0: Normal operation mode
+ * 1: Power down mode
+ * DW9768 requires waiting time of Topr after PD reset takes place.
+ */
+#define DW9768_RING_PD_CONTROL_REG 0x02
+#define DW9768_PD_MODE_OFF 0x00
+#define DW9768_PD_MODE_EN BIT(0)
+#define DW9768_AAC_MODE_EN BIT(1)
+
+/*
+ * DW9768 separates two registers to control the VCM position.
+ * One for MSB value, another is LSB value.
+ * DAC_MSB: D[9:8] (ADD: 0x03)
+ * DAC_LSB: D[7:0] (ADD: 0x04)
+ * D[9:0] DAC data input: positive output current = D[9:0] / 1023 * 100[mA]
+ */
+#define DW9768_MSB_ADDR 0x03
+#define DW9768_LSB_ADDR 0x04
+#define DW9768_STATUS_ADDR 0x05
+
+/*
+ * AAC mode control & prescale register
+ * Bit[7:5] Namely AC[2:0], decide the VCM mode and operation time.
+ * 001 AAC2 0.48 x Tvib
+ * 010 AAC3 0.70 x Tvib
+ * 011 AAC4 0.75 x Tvib
+ * 101 AAC8 1.13 x Tvib
+ * Bit[2:0] Namely PRESC[2:0], set the internal clock dividing rate as follow.
+ * 000 2
+ * 001 1
+ * 010 1/2
+ * 011 1/4
+ * 100 8
+ * 101 4
+ */
+#define DW9768_AAC_PRESC_REG 0x06
+#define DW9768_AAC_MODE_SEL_MASK GENMASK(7, 5)
+#define DW9768_CLOCK_PRE_SCALE_SEL_MASK GENMASK(2, 0)
+
+/*
+ * VCM period of vibration register
+ * Bit[5:0] Defined as VCM rising periodic time (Tvib) together with PRESC[2:0]
+ * Tvib = (6.3ms + AACT[5:0] * 0.1ms) * Dividing Rate
+ * Dividing Rate is the internal clock dividing rate that is defined at
+ * PRESCALE register (ADD: 0x06)
+ */
+#define DW9768_AAC_TIME_REG 0x07
+
+/*
+ * DW9768 requires waiting time (delay time) of t_OPR after power-up,
+ * or in the case of PD reset taking place.
+ */
+#define DW9768_T_OPR_US 1000
+#define DW9768_Tvib_MS_BASE10 (64 - 1)
+#define DW9768_AAC_MODE_DEFAULT 2
+#define DW9768_AAC_TIME_DEFAULT 0x20
+#define DW9768_CLOCK_PRE_SCALE_DEFAULT 1
+
+/*
+ * This acts as the minimum granularity of lens movement.
+ * Keep this value power of 2, so the control steps can be
+ * uniformly adjusted for gradual lens movement, with desired
+ * number of control steps.
+ */
+#define DW9768_MOVE_STEPS 16
+
+static const char * const dw9768_supply_names[] = {
+ "vin", /* Digital I/O power */
+ "vdd", /* Digital core power */
+};
+
+/* dw9768 device structure */
+struct dw9768 {
+ struct regulator_bulk_data supplies[ARRAY_SIZE(dw9768_supply_names)];
+ struct v4l2_ctrl_handler ctrls;
+ struct v4l2_ctrl *focus;
+ struct v4l2_subdev sd;
+
+ u32 aac_mode;
+ u32 aac_timing;
+ u32 clock_presc;
+};
+
+static inline struct dw9768 *sd_to_dw9768(struct v4l2_subdev *subdev)
+{
+ return container_of(subdev, struct dw9768, sd);
+}
+
+struct regval_list {
+ u8 reg_num;
+ u8 value;
+};
+
+struct dw9768_aac_mode_ot_multi {
+ u32 aac_mode_enum;
+ u32 ot_multi_base100;
+};
+
+struct dw9768_clk_presc_dividing_rate {
+ u32 clk_presc_enum;
+ u32 dividing_rate_base100;
+};
+
+static const struct dw9768_aac_mode_ot_multi aac_mode_ot_multi[] = {
+ {1, 48},
+ {2, 70},
+ {3, 75},
+ {5, 113},
+};
+
+static const struct dw9768_clk_presc_dividing_rate presc_dividing_rate[] = {
+ {0, 200},
+ {1, 100},
+ {2, 50},
+ {3, 25},
+ {4, 800},
+ {5, 400},
+};
+
+static u32 dw9768_find_ot_multi(u32 aac_mode_param)
+{
+ u32 cur_ot_multi_base100 = 70;
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(aac_mode_ot_multi); i++) {
+ if (aac_mode_ot_multi[i].aac_mode_enum == aac_mode_param) {
+ cur_ot_multi_base100 =
+ aac_mode_ot_multi[i].ot_multi_base100;
+ }
+ }
+
+ return cur_ot_multi_base100;
+}
+
+static u32 dw9768_find_dividing_rate(u32 presc_param)
+{
+ u32 cur_clk_dividing_rate_base100 = 100;
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(presc_dividing_rate); i++) {
+ if (presc_dividing_rate[i].clk_presc_enum == presc_param) {
+ cur_clk_dividing_rate_base100 =
+ presc_dividing_rate[i].dividing_rate_base100;
+ }
+ }
+
+ return cur_clk_dividing_rate_base100;
+}
+
+/*
+ * DW9768_AAC_PRESC_REG & DW9768_AAC_TIME_REG determine VCM operation time.
+ * For current VCM mode: AAC3, Operation Time would be 0.70 x Tvib.
+ * Tvib = (6.3ms + AACT[5:0] * 0.1MS) * Dividing Rate.
+ * Below is calculation of the operation delay for each step.
+ */
+static inline u32 dw9768_cal_move_delay(u32 aac_mode_param, u32 presc_param,
+ u32 aac_timing_param)
+{
+ u32 Tvib_us;
+ u32 ot_multi_base100;
+ u32 clk_dividing_rate_base100;
+
+ ot_multi_base100 = dw9768_find_ot_multi(aac_mode_param);
+
+ clk_dividing_rate_base100 = dw9768_find_dividing_rate(presc_param);
+
+ Tvib_us = (DW9768_Tvib_MS_BASE10 + aac_timing_param) *
+ clk_dividing_rate_base100;
+
+ return Tvib_us * ot_multi_base100;
+}
+
+static int dw9768_mod_reg(struct dw9768 *dw9768, u8 reg, u8 mask, u8 val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(client, reg);
+ if (ret < 0)
+ return ret;
+
+ val = ((unsigned char)ret & ~mask) | (val & mask);
+
+ return i2c_smbus_write_byte_data(client, reg, val);
+}
+
+static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+
+ /* Write VCM position to registers */
+ return i2c_smbus_write_word_swapped(client, DW9768_MSB_ADDR, val);
+}
+
+static int dw9768_init(struct dw9768 *dw9768)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+ u32 move_delay_us;
+ int ret, val;
+
+ /* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
+ ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
+ DW9768_PD_MODE_OFF);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * DW9769 requires waiting delay time of t_OPR
+ * after PD reset takes place.
+ */
+ usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
+
+ /* Set DW9768_RING_PD_CONTROL_REG to DW9768_AAC_MODE_EN(0x01) */
+ ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
+ DW9768_AAC_MODE_EN);
+ if (ret < 0)
+ return ret;
+
+ /* Set AAC mode */
+ ret = dw9768_mod_reg(dw9768, DW9768_AAC_PRESC_REG,
+ DW9768_AAC_MODE_SEL_MASK,
+ dw9768->aac_mode << 5);
+ if (ret < 0)
+ return ret;
+
+ /* Set clock presc */
+ if (dw9768->clock_presc != DW9768_CLOCK_PRE_SCALE_DEFAULT) {
+ ret = dw9768_mod_reg(dw9768, DW9768_AAC_PRESC_REG,
+ DW9768_CLOCK_PRE_SCALE_SEL_MASK,
+ dw9768->clock_presc);
+ if (ret < 0)
+ return ret;
+ }
+
+ /* Set AAC Timing */
+ if (dw9768->aac_timing != DW9768_AAC_TIME_DEFAULT) {
+ ret = i2c_smbus_write_byte_data(client, DW9768_AAC_TIME_REG,
+ dw9768->aac_timing);
+ if (ret < 0)
+ return ret;
+ }
+
+ move_delay_us = dw9768_cal_move_delay(dw9768->aac_mode,
+ dw9768->clock_presc,
+ dw9768->aac_timing) / 100;
+
+ for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
+ val <= dw9768->focus->val;
+ val += DW9768_MOVE_STEPS) {
+ ret = dw9768_set_dac(dw9768, val);
+ if (ret) {
+ dev_err(&client->dev, "%s I2C failure: %d",
+ __func__, ret);
+ return ret;
+ }
+ usleep_range(move_delay_us, move_delay_us + 1000);
+ }
+
+ return 0;
+}
+
+static int dw9768_release(struct dw9768 *dw9768)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+ u32 move_delay_us = dw9768_cal_move_delay(dw9768->aac_mode,
+ dw9768->clock_presc,
+ dw9768->aac_timing) / 100;
+ int ret, val;
+
+ val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
+ for ( ; val >= 0; val -= DW9768_MOVE_STEPS) {
+ ret = dw9768_set_dac(dw9768, val);
+ if (ret) {
+ dev_err(&client->dev, "I2C write fail: %d", ret);
+ return ret;
+ }
+ usleep_range(move_delay_us, move_delay_us + 1000);
+ }
+
+ ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
+ DW9768_PD_MODE_EN);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * DW9769 requires waiting delay time of t_OPR
+ * after PD reset takes place.
+ */
+ usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
+
+ return 0;
+}
+
+static int dw9768_runtime_suspend(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct dw9768 *dw9768 = sd_to_dw9768(sd);
+
+ dw9768_release(dw9768);
+ regulator_bulk_disable(ARRAY_SIZE(dw9768_supply_names),
+ dw9768->supplies);
+
+ return 0;
+}
+
+static int dw9768_runtime_resume(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct dw9768 *dw9768 = sd_to_dw9768(sd);
+ int ret;
+
+ ret = regulator_bulk_enable(ARRAY_SIZE(dw9768_supply_names),
+ dw9768->supplies);
+ if (ret < 0) {
+ dev_err(dev, "failed to enable regulators\n");
+ return ret;
+ }
+
+ /*
+ * The datasheet refers to t_OPR that needs to be waited before sending
+ * I2C commands after power-up.
+ */
+ usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
+
+ ret = dw9768_init(dw9768);
+ if (ret < 0)
+ goto disable_regulator;
+
+ return 0;
+
+disable_regulator:
+ regulator_bulk_disable(ARRAY_SIZE(dw9768_supply_names),
+ dw9768->supplies);
+
+ return ret;
+}
+
+static int dw9768_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct dw9768 *dw9768 = container_of(ctrl->handler,
+ struct dw9768, ctrls);
+
+ if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
+ return dw9768_set_dac(dw9768, ctrl->val);
+
+ return 0;
+}
+
+static const struct v4l2_ctrl_ops dw9768_ctrl_ops = {
+ .s_ctrl = dw9768_set_ctrl,
+};
+
+static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+ int ret;
+
+ ret = pm_runtime_get_sync(sd->dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(sd->dev);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+ pm_runtime_put(sd->dev);
+
+ return 0;
+}
+
+static const struct v4l2_subdev_internal_ops dw9768_int_ops = {
+ .open = dw9768_open,
+ .close = dw9768_close,
+};
+
+static const struct v4l2_subdev_ops dw9768_ops = { };
+
+static int dw9768_init_controls(struct dw9768 *dw9768)
+{
+ struct v4l2_ctrl_handler *hdl = &dw9768->ctrls;
+ const struct v4l2_ctrl_ops *ops = &dw9768_ctrl_ops;
+
+ v4l2_ctrl_handler_init(hdl, 1);
+
+ dw9768->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE, 0,
+ DW9768_MAX_FOCUS_POS,
+ DW9768_FOCUS_STEPS, 0);
+
+ if (hdl->error)
+ return hdl->error;
+
+ dw9768->sd.ctrl_handler = hdl;
+
+ return 0;
+}
+
+static int dw9768_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct dw9768 *dw9768;
+ u32 aac_mode_select;
+ u32 aac_timing_select;
+ u32 clock_presc_select;
+ unsigned int i;
+ int ret;
+
+ dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
+ if (!dw9768)
+ return -ENOMEM;
+
+ /* Initialize subdev */
+ v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
+
+ dw9768->aac_mode = DW9768_AAC_MODE_DEFAULT;
+ dw9768->aac_timing = DW9768_AAC_TIME_DEFAULT;
+ dw9768->clock_presc = DW9768_CLOCK_PRE_SCALE_DEFAULT;
+
+ /* Optional indication of AAC mode select */
+ ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,aac-mode",
+ &aac_mode_select);
+
+ if (!ret)
+ dw9768->aac_mode = aac_mode_select;
+
+ /* Optional indication of clock pre-scale select */
+ ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,clock-presc",
+ &clock_presc_select);
+
+ if (!ret)
+ dw9768->clock_presc = clock_presc_select;
+
+ /* Optional indication of AAC Timing */
+ ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,aac-timing",
+ &aac_timing_select);
+
+ if (!ret)
+ dw9768->aac_timing = aac_timing_select;
+
+ for (i = 0; i < ARRAY_SIZE(dw9768_supply_names); i++)
+ dw9768->supplies[i].supply = dw9768_supply_names[i];
+
+ ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(dw9768_supply_names),
+ dw9768->supplies);
+ if (ret) {
+ dev_err(dev, "failed to get regulators\n");
+ return ret;
+ }
+
+ /* Initialize controls */
+ ret = dw9768_init_controls(dw9768);
+ if (ret)
+ goto err_free_handler;
+
+ /* Initialize subdev */
+ dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+ dw9768->sd.internal_ops = &dw9768_int_ops;
+
+ ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
+ if (ret < 0)
+ goto err_free_handler;
+
+ dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
+
+ pm_runtime_enable(dev);
+ if (!pm_runtime_enabled(dev)) {
+ ret = dw9768_runtime_resume(dev);
+ if (ret < 0) {
+ dev_err(dev, "failed to power on: %d\n", ret);
+ goto err_clean_entity;
+ }
+ }
+
+ ret = v4l2_async_register_subdev(&dw9768->sd);
+ if (ret < 0) {
+ dev_err(dev, "failed to register V4L2 subdev: %d", ret);
+ goto error_async_register;
+ }
+
+ return 0;
+
+error_async_register:
+ if (!pm_runtime_enabled(dev))
+ dw9768_runtime_suspend(dev);
+err_clean_entity:
+ media_entity_cleanup(&dw9768->sd.entity);
+err_free_handler:
+ v4l2_ctrl_handler_free(&dw9768->ctrls);
+
+ return ret;
+}
+
+static int dw9768_remove(struct i2c_client *client)
+{
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct dw9768 *dw9768 = sd_to_dw9768(sd);
+
+ v4l2_async_unregister_subdev(&dw9768->sd);
+ v4l2_ctrl_handler_free(&dw9768->ctrls);
+ media_entity_cleanup(&dw9768->sd.entity);
+ pm_runtime_disable(&client->dev);
+ if (!pm_runtime_status_suspended(&client->dev))
+ dw9768_runtime_suspend(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
+
+ return 0;
+}
+
+static const struct of_device_id dw9768_of_table[] = {
+ { .compatible = "dongwoon,dw9768" },
+ { .compatible = "giantec,gt9769" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, dw9768_of_table);
+
+static const struct dev_pm_ops dw9768_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+ SET_RUNTIME_PM_OPS(dw9768_runtime_suspend, dw9768_runtime_resume, NULL)
+};
+
+static struct i2c_driver dw9768_i2c_driver = {
+ .driver = {
+ .name = DW9768_NAME,
+ .pm = &dw9768_pm_ops,
+ .of_match_table = dw9768_of_table,
+ },
+ .probe_new = dw9768_probe,
+ .remove = dw9768_remove,
+};
+module_i2c_driver(dw9768_i2c_driver);
+
+MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
+MODULE_DESCRIPTION("DW9768 VCM driver");
+MODULE_LICENSE("GPL v2");
--
2.9.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [V7, 1/2] media: dt-bindings: media: i2c: Document DW9768 bindings
From: Dongchun Zhu @ 2020-06-05 10:54 UTC (permalink / raw)
To: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
mark.rutland, sakari.ailus, drinkcat, tfiga, matthias.bgg,
bingbu.cao
Cc: devicetree, srv_heupstream, shengnan.wang, sj.huang,
linux-mediatek, dongchun.zhu, louis.kuo, linux-arm-kernel,
linux-media
In-Reply-To: <20200605105412.18813-1-dongchun.zhu@mediatek.com>
Add DeviceTree binding documentation for Dongwoon Anatech DW9768 voice
coil actuator.
Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
---
.../bindings/media/i2c/dongwoon,dw9768.yaml | 100 +++++++++++++++++++++
MAINTAINERS | 7 ++
2 files changed, 107 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
new file mode 100644
index 0000000..cb96e95
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (c) 2020 MediaTek Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9768.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dongwoon Anatech DW9768 Voice Coil Motor (VCM) Lens Device Tree Bindings
+
+maintainers:
+ - Dongchun Zhu <dongchun.zhu@mediatek.com>
+
+description: |-
+ The Dongwoon DW9768 is a single 10-bit digital-to-analog (DAC) converter
+ with 100 mA output current sink capability. VCM current is controlled with
+ a linear mode driver. The DAC is controlled via a 2-wire (I2C-compatible)
+ serial interface that operates at clock rates up to 1MHz. This chip
+ integrates Advanced Actuator Control (AAC) technology and is intended for
+ driving voice coil lenses in camera modules.
+
+properties:
+ compatible:
+ enum:
+ - dongwoon,dw9768 # for DW9768 VCM
+ - giantec,gt9769 # for GT9769 VCM
+
+ reg:
+ maxItems: 1
+
+ vin-supply:
+ description:
+ Definition of the regulator used as Digital I/O voltage supply.
+
+ vdd-supply:
+ description:
+ Definition of the regulator used as Digital core voltage supply.
+
+ dongwoon,aac-mode:
+ description:
+ Indication of AAC mode select.
+ allOf:
+ - $ref: "/schemas/types.yaml#/definitions/uint32"
+ - enum:
+ - 1 # AAC2 mode(operation time# 0.48 x Tvib)
+ - 2 # AAC3 mode(operation time# 0.70 x Tvib)
+ - 3 # AAC4 mode(operation time# 0.75 x Tvib)
+ - 5 # AAC8 mode(operation time# 1.13 x Tvib)
+ default: 2
+
+ dongwoon,aac-timing:
+ description:
+ Number of AAC Timing count that controlled by one 6-bit period of
+ vibration register AACT[5:0], the unit of which is 100 us.
+ allOf:
+ - $ref: "/schemas/types.yaml#/definitions/uint32"
+ - default: 0x20
+ minimum: 0x00
+ maximum: 0x3f
+
+ dongwoon,clock-presc:
+ description:
+ Indication of VCM internal clock dividing rate select, as one multiple
+ factor to calculate VCM ring periodic time Tvib.
+ allOf:
+ - $ref: "/schemas/types.yaml#/definitions/uint32"
+ - enum:
+ - 0 # Dividing Rate - 2
+ - 1 # Dividing Rate - 1
+ - 2 # Dividing Rate - 1/2
+ - 3 # Dividing Rate - 1/4
+ - 4 # Dividing Rate - 8
+ - 5 # Dividing Rate - 4
+ default: 1
+
+required:
+ - compatible
+ - reg
+ - vin-supply
+ - vdd-supply
+
+additionalProperties: false
+
+examples:
+ - |
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dw9768: camera-lens@c {
+ compatible = "dongwoon,dw9768";
+ reg = <0x0c>;
+
+ vin-supply = <&mt6358_vcamio_reg>;
+ vdd-supply = <&mt6358_vcama2_reg>;
+ dongwoon,aac-timing = <0x39>;
+ };
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index e64e5db..8d72c41 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5151,6 +5151,13 @@ T: git git://linuxtv.org/media_tree.git
F: Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt
F: drivers/media/i2c/dw9714.c
+DONGWOON DW9768 LENS VOICE COIL DRIVER
+M: Dongchun Zhu <dongchun.zhu@mediatek.com>
+L: linux-media@vger.kernel.org
+S: Maintained
+T: git git://linuxtv.org/media_tree.git
+F: Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
+
DONGWOON DW9807 LENS VOICE COIL DRIVER
M: Sakari Ailus <sakari.ailus@linux.intel.com>
L: linux-media@vger.kernel.org
--
2.9.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
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