Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 04/11] dt-bindings: phy-mtk-tphy: add a new reference clock
From: Rob Herring @ 2019-08-29 20:05 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Mark Rutland, devicetree, linux-kernel, Kishon Vijay Abraham I,
	linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <f6ee7d33103b43b2f1e1331c23c36057ef20b20d.1566542697.git.chunfeng.yun@mediatek.com>

On Fri, Aug 23, 2019 at 03:00:11PM +0800, Chunfeng Yun wrote:
> Usually the digital and anolog phys use the same reference clock,
> but on some platforms, they are separated, so add another optional
> clock to support it.
> In order to keep the clock names consistent with PHY IP's, use
> the da_ref for anolog phy and ref clock for digital phy.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
> index dbc143ed5999..ed9a2641f204 100644
> --- a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
> @@ -41,9 +41,12 @@ Optional properties (PHY_TYPE_USB2 port (child) node):
>  - clocks	: a list of phandle + clock-specifier pairs, one for each
>  		  entry in clock-names
>  - clock-names	: may contain
> -		  "ref": 48M reference clock for HighSpeed anolog phy; and 26M
> -			reference clock for SuperSpeed anolog phy, sometimes is
> +		  "ref": 48M reference clock for HighSpeed (digital) phy; and 26M
> +			reference clock for SuperSpeed (digital) phy, sometimes is
>  			24M, 25M or 27M, depended on platform.
> +		  "da_ref": the reference clock of anolog phy, used if the clocks
> +			of anolog and digital phys are separated, otherwise uses

s/amolog/analog/

> +			"ref" clock only if need.

needed.

>  
>  - mediatek,eye-src	: u32, the value of slew rate calibrate
>  - mediatek,eye-vrt	: u32, the selection of VRT reference voltage
> -- 
> 2.23.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 03/11] dt-bindings: phy-mtk-tphy: remove unused u3phya_ref clock
From: Rob Herring @ 2019-08-29 20:03 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Mark Rutland, devicetree, linux-kernel, Kishon Vijay Abraham I,
	Chunfeng Yun, linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <35d020857cd0e2fdc77023dad36221288d7a5fe1.1566542697.git.chunfeng.yun@mediatek.com>

On Fri, 23 Aug 2019 15:00:10 +0800, Chunfeng Yun wrote:
> The u3phya_ref clock is already moved into sub-node, and
> renamed as ref clock, no used anymore now, so remove it
> to avoid confusion
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt | 4 ----
>  1 file changed, 4 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

_______________________________________________
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] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer
From: Nathan Chancellor @ 2019-08-29 19:34 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Arnd Bergmann, Russell King, Stefan Agner, LKML,
	clang-built-linux, Matthias Kaehlcke, Linux ARM
In-Reply-To: <CAKwvOdkXSWE+_JCZsuQdkCSrK5pJSp9n_Cd27asFP0mHBfHg6w@mail.gmail.com>

On Thu, Aug 29, 2019 at 10:55:28AM -0700, Nick Desaulniers wrote:
> On Wed, Aug 28, 2019 at 11:27 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > Currently, multi_v7_defconfig + CONFIG_FUNCTION_TRACER fails to build
> > with clang:
> >
> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `_local_bh_enable':
> > softirq.c:(.text+0x504): undefined reference to `mcount'
> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `__local_bh_enable_ip':
> > softirq.c:(.text+0x58c): undefined reference to `mcount'
> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `do_softirq':
> > softirq.c:(.text+0x6c8): undefined reference to `mcount'
> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_enter':
> > softirq.c:(.text+0x75c): undefined reference to `mcount'
> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_exit':
> > softirq.c:(.text+0x840): undefined reference to `mcount'
> > arm-linux-gnueabi-ld: kernel/softirq.o:softirq.c:(.text+0xa50): more undefined references to `mcount' follow
> >
> > clang can emit a working mcount symbol, __gnu_mcount_nc, when
> > '-meabi gnu' is passed to it. Until r369147 in LLVM, this was
> > broken and caused the kernel not to boot because the calling
> > convention was not correct. Now that it is fixed, add this to
> > the command line when clang is 10.0.0 or newer so everything
> > works properly.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/35
> > Link: https://bugs.llvm.org/show_bug.cgi?id=33845
> > Link: https://github.com/llvm/llvm-project/commit/16fa8b09702378bacfa3d07081afe6b353b99e60
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >  arch/arm/Makefile | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> > index c3624ca6c0bc..7b5a26a866fc 100644
> > --- a/arch/arm/Makefile
> > +++ b/arch/arm/Makefile
> > @@ -112,6 +112,12 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
> >  CFLAGS_ABI     +=-funwind-tables
> >  endif
> >
> > +ifeq ($(CONFIG_CC_IS_CLANG),y)
> > +ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 100000; echo $$?),0)
> > +CFLAGS_ABI     +=-meabi gnu
> > +endif
> > +endif
> > +
> 
> Thanks for the patch!  I think this is one of the final issues w/ 32b
> ARM configs when building w/ Clang.
> 
> I'm not super enthused about the version check.  The flag is indeed
> not recognized by GCC, but I think it would actually be more concise
> with $(cc-option) and no compiler or version check.
> 
> Further, I think that the working __gnu_mcount_nc in Clang would
> better be represented as marking the arch/arm/KConfig option for
> CONFIG_FUNCTION_TRACER for dependent on a version of Clang greater
> than or equal to Clang 10, not conditionally adding this flag. (We
> should always add the flag when supported, IMO.  __gnu_mcount_nc's
> calling convention being broken is orthogonal to the choice of
> __gnu_mcount_nc vs mcount, and it's the former's that should be
> checked, not the latter as in this patch.

I will test with or without CONFIG_AEABI like Matthias asked and I will
implement your Kconfig suggestion if it passes all of my tests. The
reason that I did it this way is because I didn't want a user to end up
with a non-booting kernel since -meabi gnu works with older versions of
clang at build time, the issue happens at boot time but the Kconfig
suggestion + cc-option should fix that.

I should have a v2 out this evening.

Cheers,
Nathan

_______________________________________________
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 02/11] dt-bindings: phy-mtk-tphy: make the ref clock optional
From: Rob Herring @ 2019-08-29 19:25 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Mark Rutland, devicetree, linux-kernel, Kishon Vijay Abraham I,
	linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <a31d78484b64f853a16e7dcb16fae9fc0de45ebb.1566542696.git.chunfeng.yun@mediatek.com>

On Fri, Aug 23, 2019 at 03:00:09PM +0800, Chunfeng Yun wrote:
> Make the ref clock optional, then we no need refer to a fixed-clock
> in DTS anymore when the clock of USB3 PHY comes from oscillator
> directly
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  .../devicetree/bindings/phy/phy-mtk-tphy.txt        | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
> index d5b327f85fa2..1c18bf10b2fe 100644
> --- a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
> @@ -34,12 +34,6 @@ Optional properties (controller (parent) node):
>  
>  Required properties (port (child) node):
>  - reg		: address and length of the register set for the port.
> -- clocks	: a list of phandle + clock-specifier pairs, one for each
> -		  entry in clock-names
> -- clock-names	: must contain
> -		  "ref": 48M reference clock for HighSpeed analog phy; and 26M
> -			reference clock for SuperSpeed analog phy, sometimes is
> -			24M, 25M or 27M, depended on platform.
>  - #phy-cells	: should be 1 (See second example)
>  		  cell after port phandle is phy type from:
>  			- PHY_TYPE_USB2
> @@ -48,6 +42,13 @@ Required properties (port (child) node):
>  			- PHY_TYPE_SATA
>  
>  Optional properties (PHY_TYPE_USB2 port (child) node):
> +- clocks	: a list of phandle + clock-specifier pairs, one for each
> +		  entry in clock-names
> +- clock-names	: may contain
> +		  "ref": 48M reference clock for HighSpeed anolog phy; and 26M
> +			reference clock for SuperSpeed anolog phy, sometimes is
> +			24M, 25M or 27M, depended on platform.

How do you know the frequency when it is not present?

> +
>  - mediatek,eye-src	: u32, the value of slew rate calibrate
>  - mediatek,eye-vrt	: u32, the selection of VRT reference voltage
>  - mediatek,eye-term	: u32, the selection of HS_TX TERM reference voltage
> -- 
> 2.23.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 01/11] dt-bindings: phy-mtk-tphy: add two optional properties for u2phy
From: Rob Herring @ 2019-08-29 19:23 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Mark Rutland, devicetree, linux-kernel, Kishon Vijay Abraham I,
	linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <e99c0d7a55869a4425250c601b80a3331c9d0976.1566542696.git.chunfeng.yun@mediatek.com>

On Fri, Aug 23, 2019 at 03:00:08PM +0800, Chunfeng Yun wrote:
> Add two optional properties, one for J-K test, another for disconnect
> threshold, both of them can be used to debug disconnection issues.

Testing and debug properties aren't really things that belong in DT.

> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
> index a5f7a4f0dbc1..d5b327f85fa2 100644
> --- a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
> @@ -52,6 +52,8 @@ Optional properties (PHY_TYPE_USB2 port (child) node):
>  - mediatek,eye-vrt	: u32, the selection of VRT reference voltage
>  - mediatek,eye-term	: u32, the selection of HS_TX TERM reference voltage
>  - mediatek,bc12	: bool, enable BC12 of u2phy if support it
> +- mediatek,discth	: u32, the voltage of disconnect threshold
> +- mediatek,intr	: u32, the value of internal R (resistance)

These need units as defined in property-units.txt.

>  
>  Example:
>  
> -- 
> 2.23.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V3 02/10] dt-bindings: soc: Add opp table on scpsys bindings
From: Rob Herring @ 2019-08-29 19:16 UTC (permalink / raw)
  To: Henry Chen
  Cc: Nicolas Boichat, Weiyi Lu, James Liao, Viresh Kumar, linux-kernel,
	Henry Chen, Stephen Boyd, Fan Chen, devicetree, Rob Herring,
	Ryan Case, Matthias Brugger, linux-mediatek, Georgi Djakov,
	linux-arm-kernel
In-Reply-To: <1566995328-15158-3-git-send-email-henryc.chen@mediatek.com>

On Wed, 28 Aug 2019 20:28:40 +0800, Henry Chen wrote:
> Add opp table on scpsys dt-bindings for Mediatek SoC.
> 
> Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/scpsys.txt    | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 

Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.

_______________________________________________
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 01/10] dt-bindings: soc: Add dvfsrc driver bindings
From: Rob Herring @ 2019-08-29 19:16 UTC (permalink / raw)
  To: Henry Chen
  Cc: Nicolas Boichat, Weiyi Lu, James Liao, Viresh Kumar, linux-kernel,
	Henry Chen, Stephen Boyd, Fan Chen, devicetree, Rob Herring,
	Ryan Case, Matthias Brugger, linux-mediatek, Georgi Djakov,
	linux-arm-kernel
In-Reply-To: <1566995328-15158-2-git-send-email-henryc.chen@mediatek.com>

On Wed, 28 Aug 2019 20:28:39 +0800, Henry Chen wrote:
> Document the binding for enabling dvfsrc on MediaTek SoC.
> 
> Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/dvfsrc.txt    | 23 ++++++++++++++++++++++
>  include/dt-bindings/soc/mtk,dvfsrc.h               | 14 +++++++++++++
>  2 files changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/mediatek/dvfsrc.txt
>  create mode 100644 include/dt-bindings/soc/mtk,dvfsrc.h
> 

Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.

_______________________________________________
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 5/8] media: cedrus: Detect first slice of a frame
From: Jernej Škrabec @ 2019-08-29 19:04 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: devel, acourbot, pawel, jonas, gregkh, wens, mripard, tfiga,
	paul.kocialkowski, kyungmin.park, linux-media, linux-arm-kernel,
	hverkuil-cisco, mchehab, ezequiel, linux-kernel, m.szyprowski
In-Reply-To: <20190826202831.311c7c20@collabora.com>

Dne ponedeljek, 26. avgust 2019 ob 20:28:31 CEST je Boris Brezillon 
napisal(a):
> Hi Jernej,
> 
> On Thu, 22 Aug 2019 21:44:57 +0200
> 
> Jernej Skrabec <jernej.skrabec@siol.net> wrote:
> > When codec supports multiple slices in one frame, VPU has to know when
> > first slice of each frame is being processed, presumably to correctly
> > clear/set data in auxiliary buffers.
> > 
> > Add first_slice field to cedrus_run structure and set it according to
> > timestamps of capture and output buffers. If timestamps are different,
> > it's first slice and viceversa.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> > 
> >  drivers/staging/media/sunxi/cedrus/cedrus.h     | 1 +
> >  drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
> >  2 files changed, 3 insertions(+)
> > 
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > b/drivers/staging/media/sunxi/cedrus/cedrus.h index
> > 2f017a651848..32cb38e541c6 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> > @@ -70,6 +70,7 @@ struct cedrus_mpeg2_run {
> > 
> >  struct cedrus_run {
> >  
> >  	struct vb2_v4l2_buffer	*src;
> >  	struct vb2_v4l2_buffer	*dst;
> > 
> > +	bool first_slice;
> > 
> >  	union {
> >  	
> >  		struct cedrus_h264_run	h264;
> > 
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c index
> > 56ca4c9ad01c..d7b54accfe83 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > @@ -31,6 +31,8 @@ void cedrus_device_run(void *priv)
> > 
> >  	run.src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> >  	run.dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > 
> > +	run.first_slice =
> > +		run.src->vb2_buf.timestamp != run.dst-
>vb2_buf.timestamp;
> 
> Can't we use slice->first_mb_in_slice to determine if a slice is the
> first? I'd expect ->first_mb_in_slice to be 0 (unless we decide to
> support ASO).

I looked in all VPU documentation available to me (which isn't much) and there 
is no indication if ASO is supported or not. Do you have any sample video with 
out-of-order slices? It's my understanding that this is uncommon. If it's 
supported, I would leave code as-is.

Best regards,
Jernej

> 
> >  	/* Apply request(s) controls if needed. */
> >  	src_req = run.src->vb2_buf.req_obj.req;





_______________________________________________
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 3/3] i2c: bcm2835: Add full name of devicetree node to adapter name
From: Wolfram Sang @ 2019-08-29 18:52 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Florian Fainelli, Scott Branden, Ray Jui, Eric Anholt, linux-i2c,
	linux-arm-kernel
In-Reply-To: <1566925456-5928-4-git-send-email-wahrenst@gmx.net>


[-- Attachment #1.1: Type: text/plain, Size: 374 bytes --]

On Tue, Aug 27, 2019 at 07:04:16PM +0200, Stefan Wahren wrote:
> Inspired by Lori Hikichi's patch for iproc, this adds the full name of
> the devicetree node to the adapter name. With the introduction of
> BCM2711 it's very difficult to distinguish between the multiple instances.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Applied to for-next, thanks!


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 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 2/3] i2c: bcm2835: Avoid clk stretch quirk for BCM2711
From: Wolfram Sang @ 2019-08-29 18:52 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Florian Fainelli, Scott Branden, Ray Jui, Eric Anholt, linux-i2c,
	linux-arm-kernel
In-Reply-To: <1566925456-5928-3-git-send-email-wahrenst@gmx.net>


[-- Attachment #1.1: Type: text/plain, Size: 330 bytes --]

On Tue, Aug 27, 2019 at 07:04:15PM +0200, Stefan Wahren wrote:
> The I2C block on the BCM2711 isn't affected by the clk stretching bug.
> So there is no need to apply the corresponding quirk.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Reviewed-by: Eric Anholt <eric@anholt.net>

Applied to for-next, thanks!


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 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 1/3] dt-bindings: i2c: bcm2835: Add brcm,bcm2711 compatible
From: Wolfram Sang @ 2019-08-29 18:52 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Florian Fainelli, Scott Branden, Ray Jui, Eric Anholt, linux-i2c,
	linux-arm-kernel
In-Reply-To: <1566925456-5928-2-git-send-email-wahrenst@gmx.net>


[-- Attachment #1.1: Type: text/plain, Size: 322 bytes --]

On Tue, Aug 27, 2019 at 07:04:14PM +0200, Stefan Wahren wrote:
> Add a new compatible for the BCM2711, which hasn't the clock stretch bug.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Reviewed-by: Eric Anholt <eric@anholt.net>
> Reviewed-by: Rob Herring <robh@kernel.org>

Applied to for-next, thanks!


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 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

* [GIT PULL] arm64: dts: Amlogic updates for v5.4 (round 2)
From: Kevin Hilman @ 2019-08-29 18:36 UTC (permalink / raw)
  To: arm, soc; +Cc: linux-amlogic, linux-arm-kernel

Hello Arnd, Olof,

Another (final) round of 64-bit DT updates for Amlogic SoCs for v5.4.
Highlights are in the tag description, but of note is a tag pulled in
from the clock tree due to a handful of new clocks used for DVFS and
power domains.

Please pull.

Thanks,

Kevin


The following changes since commit e9a12e14322d7ddafeed6aec0d3fb02c0b5dc03c:

  arm64: dts: add support for SM1 based SEI Robotics SEI610 (2019-08-20 13:31:11 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-amlogic.git tags/amlogic-dt64-2

for you to fetch changes up to 703d5ec6f8aa59964d78d28d07ca3251345d483f:

  arm64: dts: khadas-vim3: add support for the SM1 based VIM3L (2019-08-29 11:13:39 -0700)

----------------------------------------------------------------
arm64: dts: Amlogic updates for v5.4 (round 2)
- new board: Khadas VIM3L (SM1/S905D3 SoC)
- support power domains on G12[AB] and SM1 SoCs
- DT binding fixups based on YAML schema
- add a bunch of remote control keymap
- enable DVFS on SM1/SEI610 board

----------------------------------------------------------------
Christian Hewitt (7):
      arm64: dts: meson-g12b-odroid-n2: add rc-odroid keymap
      arm64: dts: meson-g12a-x96-max: add rc-x96max keymap
      arm64: dts: meson-gxbb-wetek-hub: add rc-wetek-hub keymap
      arm64: dts: meson-gxbb-wetek-play2: add rc-wetek-play2 keymap
      arm64: dts: meson-gxl-s905x-khadas-vim: use rc-khadas keymap
      arm64: dts: meson-gxl-s905w-tx3-mini: add rc-tx3mini keymap
      arm64: dts: meson-gxm-khadas-vim2: use rc-khadas keymap

Jerome Brunet (3):
      dt-bindings: clock: meson: add resets to the audio clock controller
      arm64: dts: meson: g12a: audio clock controller provides resets
      arm64: dts: meson: g12a: add reset to tdm formatters

Kevin Hilman (2):
      arm64: dts: meson: g12a-common: add VRTC
      Merge tag 'clk-meson-dt-v5.4-3' of git://github.com/BayLibre/clk-meson into v5.4/dt64-rebase

Neil Armstrong (23):
      dt-bindings: clk: meson: add sm1 periph clock controller bindings
      arm64: dts: meson: fix ethernet mac reg format
      arm64: dts: meson-gx: drop the vpu dmc memory cell
      arm64: dts: meson-gx: fix reset controller compatible
      arm64: dts: meson-gx: fix spifc compatible
      arm64: dts: meson-gx: fix watchdog compatible
      arm64: dts: meson-gx: fix mhu compatible
      arm64: dts: meson-gx: fix periphs bus node name
      arm64: dts: meson-gxl: fix internal phy compatible
      arm64: dts: meson-axg: fix MHU compatible
      arm64: dts: meson-g12a: fix reset controller compatible
      arm64: dts: meson-g12a-x96-max: fix compatible
      arm64: dts: meson-gxbb-nanopi-k2: add missing model
      arm64: dts: meson-gxbb-p201: fix snps, reset-delays-us format
      arm64: dts: meson: fix boards regulators states format
      arm64: meson-g12: add Everything-Else power domain controller
      arm64: dts: meson-sm1-sei610: add HDMI display support
      arm64: dts: meson-sm1-sei610: add USB support
      arm64: dts: meson-sm1-sei610: enable DVFS
      dt-bindings: power: add Amlogic Everything-Else power domains bindings
      arm64: dts: khadas-vim3: move common nodes into meson-khadas-vim3.dtsi
      dt-bindings: arm: amlogic: add Amlogic SM1 based Khadas VIM3L bindings
      arm64: dts: khadas-vim3: add support for the SM1 based VIM3L

 Documentation/devicetree/bindings/arm/amlogic.yaml                 |   3 +-
 Documentation/devicetree/bindings/clock/amlogic,axg-audio-clkc.txt |   1 +
 Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt      |   1 +
 Documentation/devicetree/bindings/power/amlogic,meson-ee-pwrc.yaml |  93 +++++++++++++++++++++++++
 arch/arm64/boot/dts/amlogic/Makefile                               |   1 +
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi                         |   6 +-
 arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi                  | 113 +++++++++++++++++--------------
 arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts                 |   3 +-
 arch/arm64/boot/dts/amlogic/meson-g12a.dtsi                        |   9 +++
 arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts       |   1 +
 arch/arm64/boot/dts/amlogic/meson-g12b-khadas-vim3.dtsi            | 355 -----------------------------------------------------------------------------------------------
 arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts               |   5 +-
 arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts       |   1 +
 arch/arm64/boot/dts/amlogic/meson-g12b.dtsi                        |   9 +++
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi                          |  19 +++---
 arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts               |   1 +
 arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts             |   4 +-
 arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts                |   4 +-
 arch/arm64/boot/dts/amlogic/meson-gxbb-p201.dts                    |   2 +-
 arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi                   |   4 +-
 arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-hub.dts               |   4 ++
 arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts             |   4 ++
 arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts           |   4 ++
 arch/arm64/boot/dts/amlogic/meson-gxl-s905x-hwacom-amazetv.dts     |   4 +-
 arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts         |   2 +-
 arch/arm64/boot/dts/amlogic/meson-gxl-s905x-nexbox-a95x.dts        |   4 +-
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi                         |   5 +-
 arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts              |   2 +-
 arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi                 | 360 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts             |  70 +++++++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts                   |  83 +++++++++++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-sm1.dtsi                         |  85 ++++++++++++++++++++++-
 include/dt-bindings/clock/g12a-clkc.h                              |   5 ++
 include/dt-bindings/power/meson-g12a-power.h                       |  13 ++++
 include/dt-bindings/power/meson-sm1-power.h                        |  18 +++++
 include/dt-bindings/reset/amlogic,meson-g12a-audio-reset.h         |  38 +++++++++++
 36 files changed, 895 insertions(+), 441 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/amlogic,meson-ee-pwrc.yaml
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
 create mode 100644 include/dt-bindings/power/meson-g12a-power.h
 create mode 100644 include/dt-bindings/power/meson-sm1-power.h
 create mode 100644 include/dt-bindings/reset/amlogic,meson-g12a-audio-reset.h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCHv1 0/3] Odroid c2 missing regulator linking
From: Anand Moon @ 2019-08-29 18:35 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: devicetree, Martin Blumenstingl, Kevin Hilman, Linux Kernel,
	Rob Herring, linux-amlogic, linux-arm-kernel, Jerome Brunet
In-Reply-To: <8c40f334-c723-b524-857c-73734b7d0827@baylibre.com>

Hi Neil,

On Thu, 29 Aug 2019 at 13:58, Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 28/08/2019 22:27, Anand Moon wrote:
> > Below small changes help re-configure or fix missing inter linking
> > of regulator node.
> >
> > Changes based top on my prevoius series.
>
> For the serie:
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
>

Thanks for your review.

> >
> > [0] https://patchwork.kernel.org/cover/11113091/
> >
> > TOOD: Add support for DVFS GXBB odroid board in next series.
>
> I'm curious how you will do this !

I was just studying you previous series on how you have implemented
this feature for C1, N2 and VIM3 boards.

[0] https://patchwork.kernel.org/cover/11114125/

I started gathering key inputs needed for this ie *clk / pwm*
like VDDCPU and VDDE clk changes.

But it looks like of the complex clk framework needed, so I leave this to the
expert like your team of developers to do this much quick and efficiently.

Best Regards,
-Anand

_______________________________________________
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 0/3] arm64: dts: meson-g12: specify suspend OPP
From: Kevin Hilman @ 2019-08-29 18:32 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: linux-amlogic, linux-kernel, linux-arm-kernel, Neil Armstrong
In-Reply-To: <7hy2zeuv9z.fsf@baylibre.com>

Kevin Hilman <khilman@baylibre.com> writes:

> Neil Armstrong <narmstrong@baylibre.com> writes:
>
>> Tag the 1,2GHz OPP as suspend OPP to be set before going in suspend mode,
>> for the G12A, G12B and SM1 SoCs.
>>
>> It has been reported that using various OPPs can lead to error or
>> resume with a different OPP from the ROM, thus use this safe OPP as
>> it is the default OPP used by the BL2 boot firmware.
>>
>> Neil Armstrong (3):
>>   arm64: dts: meson-g12a: specify suspend OPP
>>   arm64: dts: meson-sm1: specify suspend OPP
>>   arm64: dts: meson-g12b: specify suspend OPP
>
> Queued patches 1, 3 for v5.4.
>
> The SM1 patch has a dependency on the SM1 DVFS series, which in turn has
> a dependency on clock changes.  Once I get a stable tag for the SM1
> clock changes, I'll queue up the rest.

FYI... I decided not to queue these for v5.4.

I'm pretty sure we'll need these, but I I think we need to do a bit more
suspend/resume testing to be sure we have the right OPPs. here.

For now, this series is in my `v5.4/testing` branch, which is included
in `integ` so it can get a bit broader testing.

Kevin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [GIT PULL] soc: amlogic: updates for v5.4 (round 2)
From: Kevin Hilman @ 2019-08-29 18:24 UTC (permalink / raw)
  To: arm, soc; +Cc: linux-amlogic, linux-arm-kernel

The following changes since commit 49ed86f503be80aac158a567c4cfd31cf1cd181e:

  soc: amlogic: meson-gx-socinfo: Add of_node_put() before return (2019-08-20 14:53:33 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-amlogic.git tags/amlogic-drivers-2

for you to fetch changes up to eef3c2ba0a42a6aa709828e968b64bd11f4aeb19:

  soc: amlogic: Add support for Everything-Else power domains controller (2019-08-28 14:29:37 -0700)

----------------------------------------------------------------
soc: amlogic: updates for v5.4 (round 2)
- add power domain controller

----------------------------------------------------------------
Neil Armstrong (1):
      soc: amlogic: Add support for Everything-Else power domains controller

 drivers/soc/amlogic/Kconfig         |  11 +++
 drivers/soc/amlogic/Makefile        |   1 +
 drivers/soc/amlogic/meson-ee-pwrc.c | 492 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 504 insertions(+)
 create mode 100644 drivers/soc/amlogic/meson-ee-pwrc.c

_______________________________________________
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] arm64: dts: meson-sm1-sei610: add stdout-path property back
From: Kevin Hilman @ 2019-08-29 18:12 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: linux-amlogic, linux-kernel, linux-arm-kernel, Neil Armstrong
In-Reply-To: <20190829132728.20042-1-narmstrong@baylibre.com>

Neil Armstrong <narmstrong@baylibre.com> writes:

> The commit d4609acce187 ("arm64: dts: meson-sm1-sei610: enable DVFS")
> incorrectly removed the chosen node and the stdout-path property.
>
> Add these back.
>
> Fixes: d4609acce187 ("arm64: dts: meson-sm1-sei610: enable DVFS")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

Queued for v5.4,

I'll probably squash with the original.

Thanks,

Kevin

_______________________________________________
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 2/6] thermal: amlogic: Add thermal driver to support G12 SoCs
From: Kevin Hilman @ 2019-08-29 18:11 UTC (permalink / raw)
  To: rui.zhang, edubezval, daniel.lezcano
  Cc: devicetree, linux-pm, linux-kernel, Guillaume La Roque,
	linux-amlogic, linux-arm-kernel
In-Reply-To: <20190821222421.30242-3-glaroque@baylibre.com>

Hello thermal maintainers,

Guillaume La Roque <glaroque@baylibre.com> writes:

> Amlogic G12A and G12B SoCs integrate two thermal sensors
> with the same design.
> One is located close to the DDR controller and the other one is
> located close to the PLLs (between the CPU and GPU).
>
> The calibration data for each of the thermal sensors instance is
> stored in a different location within the AO region.
>
> Implement reading the temperature from each thermal sensor.
>
> The IP block has more functionality, which may be added to this driver
> in the future:
> - chip reset when the temperature exceeds a configurable threshold
> - up to four interrupts when the temperature has risen above a
> configurable threshold
> - up to four interrupts when the temperature has fallen below a
> configurable threshold
>
> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>

Could we get a review/merge of this driver ( and hopefully queued up for
v5.4 ?)

This has been reviewed and tested by users on this platform and it's
working well.

Reviewed-by: Kevin Hilman <khilman@baylibre.com>
Tested-by: Kevin Hilman <khilman@baylibre.com>

Thanks,

Kevin

_______________________________________________
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] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer
From: Nick Desaulniers @ 2019-08-29 17:55 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Arnd Bergmann, Russell King, Stefan Agner, LKML,
	clang-built-linux, Matthias Kaehlcke, Linux ARM
In-Reply-To: <20190829062635.45609-1-natechancellor@gmail.com>

On Wed, Aug 28, 2019 at 11:27 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> Currently, multi_v7_defconfig + CONFIG_FUNCTION_TRACER fails to build
> with clang:
>
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `_local_bh_enable':
> softirq.c:(.text+0x504): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `__local_bh_enable_ip':
> softirq.c:(.text+0x58c): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `do_softirq':
> softirq.c:(.text+0x6c8): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_enter':
> softirq.c:(.text+0x75c): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_exit':
> softirq.c:(.text+0x840): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o:softirq.c:(.text+0xa50): more undefined references to `mcount' follow
>
> clang can emit a working mcount symbol, __gnu_mcount_nc, when
> '-meabi gnu' is passed to it. Until r369147 in LLVM, this was
> broken and caused the kernel not to boot because the calling
> convention was not correct. Now that it is fixed, add this to
> the command line when clang is 10.0.0 or newer so everything
> works properly.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/35
> Link: https://bugs.llvm.org/show_bug.cgi?id=33845
> Link: https://github.com/llvm/llvm-project/commit/16fa8b09702378bacfa3d07081afe6b353b99e60
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>  arch/arm/Makefile | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index c3624ca6c0bc..7b5a26a866fc 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -112,6 +112,12 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
>  CFLAGS_ABI     +=-funwind-tables
>  endif
>
> +ifeq ($(CONFIG_CC_IS_CLANG),y)
> +ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 100000; echo $$?),0)
> +CFLAGS_ABI     +=-meabi gnu
> +endif
> +endif
> +

Thanks for the patch!  I think this is one of the final issues w/ 32b
ARM configs when building w/ Clang.

I'm not super enthused about the version check.  The flag is indeed
not recognized by GCC, but I think it would actually be more concise
with $(cc-option) and no compiler or version check.

Further, I think that the working __gnu_mcount_nc in Clang would
better be represented as marking the arch/arm/KConfig option for
CONFIG_FUNCTION_TRACER for dependent on a version of Clang greater
than or equal to Clang 10, not conditionally adding this flag. (We
should always add the flag when supported, IMO.  __gnu_mcount_nc's
calling convention being broken is orthogonal to the choice of
__gnu_mcount_nc vs mcount, and it's the former's that should be
checked, not the latter as in this patch.

>  # Accept old syntax despite ".syntax unified"
>  AFLAGS_NOWARN  :=$(call as-option,-Wa$(comma)-mno-warn-deprecated,-Wa$(comma)-W)
>
> --
> 2.23.0
>


-- 
Thanks,
~Nick Desaulniers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v5 05/10] arm64: atomics: Remove atomic_ll_sc compilation unit
From: Nick Desaulniers @ 2019-08-29 17:47 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, Tri Vo, Peter Zijlstra, Catalin Marinas,
	Ard.Biesheuvel, andrew.murray, Nathan Chancellor, Robin Murphy,
	Linux ARM
In-Reply-To: <20190829154834.26547-6-will@kernel.org>

On Thu, Aug 29, 2019 at 8:48 AM Will Deacon <will@kernel.org> wrote:
>
> From: Andrew Murray <andrew.murray@arm.com>
>
> We no longer fall back to out-of-line atomics on systems with
> CONFIG_ARM64_LSE_ATOMICS where ARM64_HAS_LSE_ATOMICS is not set.
>
> Remove the unused compilation unit which provided these symbols.
>
> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/lib/Makefile       | 19 -------------------
>  arch/arm64/lib/atomic_ll_sc.c |  3 ---
>  2 files changed, 22 deletions(-)
>  delete mode 100644 arch/arm64/lib/atomic_ll_sc.c
>
> diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> index 33c2a4abda04..f10809ef1690 100644
> --- a/arch/arm64/lib/Makefile
> +++ b/arch/arm64/lib/Makefile
> @@ -11,25 +11,6 @@ CFLAGS_REMOVE_xor-neon.o     += -mgeneral-regs-only
>  CFLAGS_xor-neon.o              += -ffreestanding
>  endif
>
> -# Tell the compiler to treat all general purpose registers (with the
> -# exception of the IP registers, which are already handled by the caller
> -# in case of a PLT) as callee-saved, which allows for efficient runtime
> -# patching of the bl instruction in the caller with an atomic instruction
> -# when supported by the CPU. Result and argument registers are handled
> -# correctly, based on the function prototype.
> -lib-$(CONFIG_ARM64_LSE_ATOMICS) += atomic_ll_sc.o
> -CFLAGS_atomic_ll_sc.o  := -ffixed-x1 -ffixed-x2                        \
> -                  -ffixed-x3 -ffixed-x4 -ffixed-x5 -ffixed-x6          \
> -                  -ffixed-x7 -fcall-saved-x8 -fcall-saved-x9           \
> -                  -fcall-saved-x10 -fcall-saved-x11 -fcall-saved-x12   \
> -                  -fcall-saved-x13 -fcall-saved-x14 -fcall-saved-x15   \
> -                  -fcall-saved-x18 -fomit-frame-pointer

+ Tri (who implemented support for -fcall-saved-x*, -ffixed-x* in
Clang).  I won't be sad to see the use of these flags go.

> -CFLAGS_REMOVE_atomic_ll_sc.o := $(CC_FLAGS_FTRACE)
> -GCOV_PROFILE_atomic_ll_sc.o    := n
> -KASAN_SANITIZE_atomic_ll_sc.o  := n
> -KCOV_INSTRUMENT_atomic_ll_sc.o := n
> -UBSAN_SANITIZE_atomic_ll_sc.o  := n
> -
>  lib-$(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) += uaccess_flushcache.o
>
>  obj-$(CONFIG_CRC32) += crc32.o
> diff --git a/arch/arm64/lib/atomic_ll_sc.c b/arch/arm64/lib/atomic_ll_sc.c
> deleted file mode 100644
> index b0c538b0da28..000000000000
> --- a/arch/arm64/lib/atomic_ll_sc.c
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#include <asm/atomic.h>
> -#define __ARM64_IN_ATOMIC_IMPL
> -#include <asm/atomic_ll_sc.h>
> --
> 2.11.0
>


-- 
Thanks,
~Nick Desaulniers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v5 10/10] arm64: atomics: Use K constraint when toolchain appears to support it
From: Nick Desaulniers @ 2019-08-29 17:45 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, Peter Zijlstra, Catalin Marinas, Ard.Biesheuvel,
	andrew.murray, Nathan Chancellor, Robin Murphy, Linux ARM
In-Reply-To: <20190829165457.grindfmgpdpsbt4i@willie-the-truck>

On Thu, Aug 29, 2019 at 9:55 AM Will Deacon <will@kernel.org> wrote:
>
> On Thu, Aug 29, 2019 at 04:48:34PM +0100, Will Deacon wrote:
> > diff --git a/arch/arm64/include/asm/atomic_ll_sc.h b/arch/arm64/include/asm/atomic_ll_sc.h
> > index 95091f72228b..7fa042f5444e 100644
> > --- a/arch/arm64/include/asm/atomic_ll_sc.h
> > +++ b/arch/arm64/include/asm/atomic_ll_sc.h
> > @@ -23,6 +23,10 @@ asm_ops "\n"                                                               \
> >  #define __LL_SC_FALLBACK(asm_ops) asm_ops
> >  #endif
> >
> > +#ifndef CONFIG_CC_HAS_K_CONSTRAINT
> > +#define K
> > +#endif
>
> Bah, I need to use something like __stringify when the constraint is used
> in order for this to get expanded properly. Updated diff below.
>
> Will

Hi Will, thanks for cc'ing me on the patch set.  I'd be happy to help
test w/ Clang.  Would you mind pushing this set with the below diff to
a publicly available tree+branch I can pull from?  (I haven't yet
figured out how to download multiple diff's from gmail rather than 1
by 1, and TBH I'd rather just use git).

>
> --->8
>
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 61de992bbea3..0cef056b5fb1 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -39,6 +39,12 @@ $(warning LSE atomics not supported by binutils)
>    endif
>  endif
>
> +cc_has_k_constraint := $(call try-run,echo                             \
> +       'int main(void) {                                               \
> +               asm volatile("and w0, w0, %w0" :: "K" (4294967295));    \
> +               return 0;                                               \
> +       }' | $(CC) -S -x c -o "$$TMP" -,,-DCONFIG_CC_HAS_K_CONSTRAINT=1)
> +
>  ifeq ($(CONFIG_ARM64), y)
>  brokengasinst := $(call as-instr,1:\n.inst 0\n.rept . - 1b\n\nnop\n.endr\n,,-DCONFIG_BROKEN_GAS_INST=1)
>
> @@ -63,7 +69,8 @@ ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y)
>    endif
>  endif
>
> -KBUILD_CFLAGS  += -mgeneral-regs-only $(lseinstr) $(brokengasinst) $(compat_vdso)
> +KBUILD_CFLAGS  += -mgeneral-regs-only $(lseinstr) $(brokengasinst)     \
> +                  $(compat_vdso) $(cc_has_k_constraint)
>  KBUILD_CFLAGS  += -fno-asynchronous-unwind-tables
>  KBUILD_CFLAGS  += $(call cc-disable-warning, psabi)
>  KBUILD_AFLAGS  += $(lseinstr) $(brokengasinst) $(compat_vdso)
> diff --git a/arch/arm64/include/asm/atomic_ll_sc.h b/arch/arm64/include/asm/atomic_ll_sc.h
> index 95091f72228b..7b012148bfd6 100644
> --- a/arch/arm64/include/asm/atomic_ll_sc.h
> +++ b/arch/arm64/include/asm/atomic_ll_sc.h
> @@ -10,6 +10,8 @@
>  #ifndef __ASM_ATOMIC_LL_SC_H
>  #define __ASM_ATOMIC_LL_SC_H
>
> +#include <linux/stringify.h>
> +
>  #if IS_ENABLED(CONFIG_ARM64_LSE_ATOMICS) && IS_ENABLED(CONFIG_AS_LSE)
>  #define __LL_SC_FALLBACK(asm_ops)                                      \
>  "      b       3f\n"                                                   \
> @@ -23,6 +25,10 @@ asm_ops "\n"                                                         \
>  #define __LL_SC_FALLBACK(asm_ops) asm_ops
>  #endif
>
> +#ifndef CONFIG_CC_HAS_K_CONSTRAINT
> +#define K
> +#endif
> +
>  /*
>   * AArch64 UP and SMP safe atomic ops.  We use load exclusive and
>   * store exclusive to ensure that these are atomic.  We may loop
> @@ -44,7 +50,7 @@ __ll_sc_atomic_##op(int i, atomic_t *v)                                       \
>  "      stxr    %w1, %w0, %2\n"                                         \
>  "      cbnz    %w1, 1b\n")                                             \
>         : "=&r" (result), "=&r" (tmp), "+Q" (v->counter)                \
> -       : #constraint "r" (i));                                         \
> +       : __stringify(constraint) "r" (i));                             \
>  }
>
>  #define ATOMIC_OP_RETURN(name, mb, acq, rel, cl, op, asm_op, constraint)\
> @@ -63,7 +69,7 @@ __ll_sc_atomic_##op##_return##name(int i, atomic_t *v)                        \
>  "      cbnz    %w1, 1b\n"                                              \
>  "      " #mb )                                                         \
>         : "=&r" (result), "=&r" (tmp), "+Q" (v->counter)                \
> -       : #constraint "r" (i)                                           \
> +       : __stringify(constraint) "r" (i)                               \
>         : cl);                                                          \
>                                                                         \
>         return result;                                                  \
> @@ -85,7 +91,7 @@ __ll_sc_atomic_fetch_##op##name(int i, atomic_t *v)                   \
>  "      cbnz    %w2, 1b\n"                                              \
>  "      " #mb )                                                         \
>         : "=&r" (result), "=&r" (val), "=&r" (tmp), "+Q" (v->counter)   \
> -       : #constraint "r" (i)                                           \
> +       : __stringify(constraint) "r" (i)                               \
>         : cl);                                                          \
>                                                                         \
>         return result;                                                  \
> @@ -113,10 +119,15 @@ ATOMIC_OPS(sub, sub, J)
>         ATOMIC_FETCH_OP (_acquire,        , a,  , "memory", __VA_ARGS__)\
>         ATOMIC_FETCH_OP (_release,        ,  , l, "memory", __VA_ARGS__)
>
> -ATOMIC_OPS(and, and, )
> +ATOMIC_OPS(and, and, K)
> +ATOMIC_OPS(or, orr, K)
> +ATOMIC_OPS(xor, eor, K)
> +/*
> + * GAS converts the mysterious and undocumented BIC (immediate) alias to
> + * an AND (immediate) instruction with the immediate inverted. We don't
> + * have a constraint for this, so fall back to register.
> + */
>  ATOMIC_OPS(andnot, bic, )
> -ATOMIC_OPS(or, orr, )
> -ATOMIC_OPS(xor, eor, )
>
>  #undef ATOMIC_OPS
>  #undef ATOMIC_FETCH_OP
> @@ -138,7 +149,7 @@ __ll_sc_atomic64_##op(s64 i, atomic64_t *v)                         \
>  "      stxr    %w1, %0, %2\n"                                          \
>  "      cbnz    %w1, 1b")                                               \
>         : "=&r" (result), "=&r" (tmp), "+Q" (v->counter)                \
> -       : #constraint "r" (i));                                         \
> +       : __stringify(constraint) "r" (i));                             \
>  }
>
>  #define ATOMIC64_OP_RETURN(name, mb, acq, rel, cl, op, asm_op, constraint)\
> @@ -157,7 +168,7 @@ __ll_sc_atomic64_##op##_return##name(s64 i, atomic64_t *v)          \
>  "      cbnz    %w1, 1b\n"                                              \
>  "      " #mb )                                                         \
>         : "=&r" (result), "=&r" (tmp), "+Q" (v->counter)                \
> -       : #constraint "r" (i)                                           \
> +       : __stringify(constraint) "r" (i)                               \
>         : cl);                                                          \
>                                                                         \
>         return result;                                                  \
> @@ -179,7 +190,7 @@ __ll_sc_atomic64_fetch_##op##name(s64 i, atomic64_t *v)             \
>  "      cbnz    %w2, 1b\n"                                              \
>  "      " #mb )                                                         \
>         : "=&r" (result), "=&r" (val), "=&r" (tmp), "+Q" (v->counter)   \
> -       : #constraint "r" (i)                                           \
> +       : __stringify(constraint) "r" (i)                               \
>         : cl);                                                          \
>                                                                         \
>         return result;                                                  \
> @@ -208,9 +219,14 @@ ATOMIC64_OPS(sub, sub, J)
>         ATOMIC64_FETCH_OP (_release,,  , l, "memory", __VA_ARGS__)
>
>  ATOMIC64_OPS(and, and, L)
> -ATOMIC64_OPS(andnot, bic, )
>  ATOMIC64_OPS(or, orr, L)
>  ATOMIC64_OPS(xor, eor, L)
> +/*
> + * GAS converts the mysterious and undocumented BIC (immediate) alias to
> + * an AND (immediate) instruction with the immediate inverted. We don't
> + * have a constraint for this, so fall back to register.
> + */
> +ATOMIC64_OPS(andnot, bic, )
>
>  #undef ATOMIC64_OPS
>  #undef ATOMIC64_FETCH_OP
> @@ -269,7 +285,7 @@ __ll_sc__cmpxchg_case_##name##sz(volatile void *ptr,                        \
>         "2:")                                                           \
>         : [tmp] "=&r" (tmp), [oldval] "=&r" (oldval),                   \
>           [v] "+Q" (*(u##sz *)ptr)                                      \
> -       : [old] #constraint "r" (old), [new] "r" (new)                  \
> +       : [old] __stringify(constraint) "r" (old), [new] "r" (new)      \
>         : cl);                                                          \
>                                                                         \
>         return oldval;                                                  \
> @@ -280,21 +296,21 @@ __ll_sc__cmpxchg_case_##name##sz(volatile void *ptr,                      \
>   * handle the 'K' constraint for the value 4294967295 - thus we use no
>   * constraint for 32 bit operations.
>   */
> -__CMPXCHG_CASE(w, b,     ,  8,        ,  ,  ,         , )
> -__CMPXCHG_CASE(w, h,     , 16,        ,  ,  ,         , )
> -__CMPXCHG_CASE(w,  ,     , 32,        ,  ,  ,         , )
> +__CMPXCHG_CASE(w, b,     ,  8,        ,  ,  ,         , K)
> +__CMPXCHG_CASE(w, h,     , 16,        ,  ,  ,         , K)
> +__CMPXCHG_CASE(w,  ,     , 32,        ,  ,  ,         , K)
>  __CMPXCHG_CASE( ,  ,     , 64,        ,  ,  ,         , L)
> -__CMPXCHG_CASE(w, b, acq_,  8,        , a,  , "memory", )
> -__CMPXCHG_CASE(w, h, acq_, 16,        , a,  , "memory", )
> -__CMPXCHG_CASE(w,  , acq_, 32,        , a,  , "memory", )
> +__CMPXCHG_CASE(w, b, acq_,  8,        , a,  , "memory", K)
> +__CMPXCHG_CASE(w, h, acq_, 16,        , a,  , "memory", K)
> +__CMPXCHG_CASE(w,  , acq_, 32,        , a,  , "memory", K)
>  __CMPXCHG_CASE( ,  , acq_, 64,        , a,  , "memory", L)
> -__CMPXCHG_CASE(w, b, rel_,  8,        ,  , l, "memory", )
> -__CMPXCHG_CASE(w, h, rel_, 16,        ,  , l, "memory", )
> -__CMPXCHG_CASE(w,  , rel_, 32,        ,  , l, "memory", )
> +__CMPXCHG_CASE(w, b, rel_,  8,        ,  , l, "memory", K)
> +__CMPXCHG_CASE(w, h, rel_, 16,        ,  , l, "memory", K)
> +__CMPXCHG_CASE(w,  , rel_, 32,        ,  , l, "memory", K)
>  __CMPXCHG_CASE( ,  , rel_, 64,        ,  , l, "memory", L)
> -__CMPXCHG_CASE(w, b,  mb_,  8, dmb ish,  , l, "memory", )
> -__CMPXCHG_CASE(w, h,  mb_, 16, dmb ish,  , l, "memory", )
> -__CMPXCHG_CASE(w,  ,  mb_, 32, dmb ish,  , l, "memory", )
> +__CMPXCHG_CASE(w, b,  mb_,  8, dmb ish,  , l, "memory", K)
> +__CMPXCHG_CASE(w, h,  mb_, 16, dmb ish,  , l, "memory", K)
> +__CMPXCHG_CASE(w,  ,  mb_, 32, dmb ish,  , l, "memory", K)
>  __CMPXCHG_CASE( ,  ,  mb_, 64, dmb ish,  , l, "memory", L)
>
>  #undef __CMPXCHG_CASE
> @@ -332,5 +348,6 @@ __CMPXCHG_DBL(   ,        ,  ,         )
>  __CMPXCHG_DBL(_mb, dmb ish, l, "memory")
>
>  #undef __CMPXCHG_DBL
> +#undef K
>
>  #endif /* __ASM_ATOMIC_LL_SC_H */



-- 
Thanks,
~Nick Desaulniers

_______________________________________________
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] [RFC] tty/serial: imx: make use of format specifier %dE
From: Uwe Kleine-König @ 2019-08-29 17:19 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jani Nikula, Petr Mladek, open list:SERIAL DRIVERS,
	Jonathan Corbet, Greg Kroah-Hartman, Linux Documentation List,
	Linux Kernel Mailing List, Steven Rostedt, Enrico Weigelt,
	NXP Linux Team, Sascha Hauer, Jiri Slaby, Shawn Guo,
	Andrew Morton, Fabio Estevam, linux-arm Mailing List,
	Sergey Senozhatsky
In-Reply-To: <CAHp75VeV8jDP1uP3HtkJ+j7+SbkB50cs4V9tJ+j9tS6icO95FQ@mail.gmail.com>


[-- Attachment #1.1.1: Type: text/plain, Size: 1386 bytes --]

On 8/29/19 3:43 PM, Andy Shevchenko wrote:
> On Thu, Aug 29, 2019 at 7:40 AM Uwe Kleine-König <uwe@kleine-koenig.org> wrote:
>>
>> I created a patch that teaches printk et al to emit a symbolic error
>> name for an error valued integer[1]. With that applied
>>
>>         dev_err(&pdev->dev, "failed to get ipg clk: %dE\n", ret);
>>
>> emits
>>
>>         ... failed to get ipg clk: EPROBE_DEFER
>>
>> if ret is -EPROBE_DEFER. Petr Mladek (i.e. one of the printk
>> maintainers) had concerns if this would be well received and worth the
>> effort. He asked to present it to a few subsystems. So for now, this
>> patch converting the imx UART driver shouldn't be applied yet but it
>> would be great to get some feedback about if you think that being able
>> to easily printk (for example) "EIO" instead of "-5" is a good idea.
> 
>> Would it help you? Do you think it helps your users?
> 
> No, it makes sense only for debug where the user is supposed to be
> developer and thus needs anyway to know code base better than average.

Would you go so far as to claim that

	... failed to get ipg clk: -517

is better sometimes than the same message with a named error? I'd say it
is never better and in some cases worse because readers who don't
understand what -EPROBE_DEFER means won't understand -517 either. So
there is net win.

Best regards
Uwe


[-- Attachment #1.2: OpenPGP digital signature --]
[-- 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 v3 01/10] KVM: arm64: Document PV-time interface
From: Andrew Jones @ 2019-08-29 17:15 UTC (permalink / raw)
  To: Steven Price
  Cc: kvm, linux-doc, Marc Zyngier, linux-kernel, Russell King,
	Catalin Marinas, Paolo Bonzini, Will Deacon, kvmarm,
	linux-arm-kernel
In-Reply-To: <20190821153656.33429-2-steven.price@arm.com>

On Wed, Aug 21, 2019 at 04:36:47PM +0100, Steven Price wrote:
> Introduce a paravirtualization interface for KVM/arm64 based on the
> "Arm Paravirtualized Time for Arm-Base Systems" specification DEN 0057A.
> 
> This only adds the details about "Stolen Time" as the details of "Live
> Physical Time" have not been fully agreed.
> 
> User space can specify a reserved area of memory for the guest and
> inform KVM to populate the memory with information on time that the host
> kernel has stolen from the guest.
> 
> A hypercall interface is provided for the guest to interrogate the
> hypervisor's support for this interface and the location of the shared
> memory structures.
> 
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
>  Documentation/virt/kvm/arm/pvtime.txt | 100 ++++++++++++++++++++++++++
>  1 file changed, 100 insertions(+)
>  create mode 100644 Documentation/virt/kvm/arm/pvtime.txt
> 
> diff --git a/Documentation/virt/kvm/arm/pvtime.txt b/Documentation/virt/kvm/arm/pvtime.txt
> new file mode 100644
> index 000000000000..1ceb118694e7
> --- /dev/null
> +++ b/Documentation/virt/kvm/arm/pvtime.txt
> @@ -0,0 +1,100 @@
> +Paravirtualized time support for arm64
> +======================================
> +
> +Arm specification DEN0057/A defined a standard for paravirtualised time
> +support for AArch64 guests:
> +
> +https://developer.arm.com/docs/den0057/a
> +
> +KVM/arm64 implements the stolen time part of this specification by providing
> +some hypervisor service calls to support a paravirtualized guest obtaining a
> +view of the amount of time stolen from its execution.
> +
> +Two new SMCCC compatible hypercalls are defined:
> +
> +PV_FEATURES 0xC5000020
> +PV_TIME_ST  0xC5000022
> +
> +These are only available in the SMC64/HVC64 calling convention as
> +paravirtualized time is not available to 32 bit Arm guests. The existence of
> +the PV_FEATURES hypercall should be probed using the SMCCC 1.1 ARCH_FEATURES
> +mechanism before calling it.
> +
> +PV_FEATURES
> +    Function ID:  (uint32)  : 0xC5000020
> +    PV_func_id:   (uint32)  : Either PV_TIME_LPT or PV_TIME_ST
> +    Return value: (int32)   : NOT_SUPPORTED (-1) or SUCCESS (0) if the relevant
> +                              PV-time feature is supported by the hypervisor.
> +
> +PV_TIME_ST
> +    Function ID:  (uint32)  : 0xC5000022
> +    Return value: (int64)   : IPA of the stolen time data structure for this
> +                              (V)CPU. On failure:

Why the () around the V in VCPU?

> +                              NOT_SUPPORTED (-1)
> +
> +The IPA returned by PV_TIME_ST should be mapped by the guest as normal memory
> +with inner and outer write back caching attributes, in the inner shareable
> +domain. A total of 16 bytes from the IPA returned are guaranteed to be
> +meaningfully filled by the hypervisor (see structure below).
> +
> +PV_TIME_ST returns the structure for the calling VCPU.

The above sentence seems redundant here.

> +
> +Stolen Time
> +-----------
> +
> +The structure pointed to by the PV_TIME_ST hypercall is as follows:
> +
> +  Field       | Byte Length | Byte Offset | Description
> +  ----------- | ----------- | ----------- | --------------------------
> +  Revision    |      4      |      0      | Must be 0 for version 0.1
> +  Attributes  |      4      |      4      | Must be 0
> +  Stolen time |      8      |      8      | Stolen time in unsigned
> +              |             |             | nanoseconds indicating how
> +              |             |             | much time this VCPU thread
> +              |             |             | was involuntarily not
> +              |             |             | running on a physical CPU.
> +
> +The structure will be updated by the hypervisor prior to scheduling a VCPU. It
> +will be present within a reserved region of the normal memory given to the
> +guest. The guest should not attempt to write into this memory. There is a
> +structure per VCPU of the guest.
> +
> +User space interface
> +====================
> +
> +User space can request that KVM provide the paravirtualized time interface to
> +a guest by creating a KVM_DEV_TYPE_ARM_PV_TIME device, for example:
> +
> +    struct kvm_create_device pvtime_device = {
> +            .type = KVM_DEV_TYPE_ARM_PV_TIME,
> +            .attr = 0,
> +            .flags = 0,
> +    };
> +
> +    pvtime_fd = ioctl(vm_fd, KVM_CREATE_DEVICE, &pvtime_device);

The ioctl doesn't return the fd. If the ioctl returns zero the fd will be
in pvtime_device.fd.

> +
> +Creation of the device should be done after creating the vCPUs of the virtual
> +machine.

Or else what? Will an error be reported in that case?

> +
> +The IPA of the structures must be given to KVM. This is the base address
> +of an array of stolen time structures (one for each VCPU). The base address
> +must be page aligned. The size must be at least 64 * number of VCPUs and be a
> +multiple of PAGE_SIZE.
> +
> +The memory for these structures should be added to the guest in the usual
> +manner (e.g. using KVM_SET_USER_MEMORY_REGION).

Above it says the guest shouldn't attempt to write the memory. Should
KVM_MEM_READONLY be used with KVM_SET_USER_MEMORY_REGION for it?

> +
> +For example:
> +
> +    struct kvm_dev_arm_st_region region = {
> +            .gpa = <IPA of guest base address>,
> +            .size = <size in bytes>
> +    };
> +
> +    struct kvm_device_attr st_base = {
> +            .group = KVM_DEV_ARM_PV_TIME_PADDR,

This is KVM_DEV_ARM_PV_TIME_REGION in the code.

> +            .attr = KVM_DEV_ARM_PV_TIME_ST,
> +            .addr = (u64)&region
> +    };
> +
> +    ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &st_base);
> -- 
> 2.20.1
>

Thanks,
drew 

_______________________________________________
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 0/3] arm64: meson-sm1: add support for the SM1 based VIM3L
From: Kevin Hilman @ 2019-08-29 17:09 UTC (permalink / raw)
  To: Neil Armstrong; +Cc: linux-amlogic, linux-kernel, linux-arm-kernel
In-Reply-To: <70d75312-68f0-351c-26b8-0f357721dd9e@baylibre.com>

Neil Armstrong <narmstrong@baylibre.com> writes:

> On 28/08/2019 19:55, Kevin Hilman wrote:
>> Neil Armstrong <narmstrong@baylibre.com> writes:
>> 
>>> This patchset adds support for the Amlogic SM1 based Khadas VIM3L variant.
>>>
>>> The S903D3 package variant of SM1 is pin-to-pin compatible with the
>>> S922X and A311d, so only internal DT changes are needed :
>>> - DVFS support is different
>>> - Audio support not yet available for SM1
>>>
>>> This patchset moved all the non-g12b nodes to meson-khadas-vim3.dtsi
>>> and add the sm1 specific nodes into meson-sm1-khadas-vim3l.dts.
>> 
>> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
>> Tested-by: Kevin Hilman <khilman@baylibre.com>
>> 
>> Basic boot test + suspend/resume test OK on my vim3L (thanks to Khadas
>> for the board!)
>> 
>>> Display has a color conversion bug on SM1 by using a more recent vendor
>>> bootloader on the SM1 based VIM3, this is out of scope of this patchset
>>> and will be fixed in the drm/meson driver.
>>>
>>> Dependencies:
>>> - patch 1,2: None
>>> - patch 3: Depends on the "arm64: meson-sm1: add support for DVFS" patchset at [1]
>> 
>> I tested in my integ branch where this series is applied, but I'm not
>> seeing any OPPs created (/sys/devices/system/cpu/cpufreq/)
>
> These patches were sent from your integ branch, on top of :
> commit 395df5af4c782ad19fb34b9a2009ca240eeb9749 (khilman-amlogic/v5.4/integ)
> Merge: 2fcc5666dd45 9557737987bb
> Author: Kevin Hilman <khilman@baylibre.com>
> Date:   Tue Aug 27 15:39:46 2019 -0700
>
>     Merge branch 'v5.4/testing' into tmp/aml-rebuild
>
> Rebuilt and retested, and I get the OPPs just fine :
> # cat /sys/bus/cpu/devices/cpu0/cpufreq/scaling_available_frequencies
> 100000 250000 500000 666666 1000000 1200000 1404000 1500000 1608000 1704000 1800000 1908000

Thanks for confirming.

Indeed, there was an issue with my most recent `integ` branch (it was
missing the driver side of some SM1 clocks.)  Fixing that issue, and
retesting this series it all works well.

Queuing for v5.4,

Thanks,

Kevin

_______________________________________________
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 2/3] media: i2c: Add IMX290 CMOS image sensor driver
From: Manivannan Sadhasivam @ 2019-08-29 17:04 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: devicetree, c.barrett, linux-kernel, a.brela, robh+dt, mchehab,
	linux-arm-kernel, linux-media
In-Reply-To: <20190813105920.GH835@valkosipuli.retiisi.org.uk>

Hi Sakari,

Thanks for the review!

On Tue, Aug 13, 2019 at 01:59:20PM +0300, Sakari Ailus wrote:
> Hi Manivannan,
> 
> On Tue, Aug 06, 2019 at 06:39:37PM +0530, Manivannan Sadhasivam wrote:
> > Add driver for Sony IMX290 CMOS image sensor driver. The driver only
> > supports I2C interface for programming and MIPI CSI-2 for sensor output.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  drivers/media/i2c/Kconfig  |  11 +
> >  drivers/media/i2c/Makefile |   1 +
> >  drivers/media/i2c/imx290.c | 845 +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 857 insertions(+)
> >  create mode 100644 drivers/media/i2c/imx290.c
> > 
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index cb8db944aa41..256edd289abe 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -594,6 +594,17 @@ config VIDEO_IMX274
> >  	  This is a V4L2 sensor driver for the Sony IMX274
> >  	  CMOS image sensor.
> >  
> > +config VIDEO_IMX290
> > +	tristate "Sony IMX290 sensor support"
> > +	depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> > +	depends on MEDIA_CAMERA_SUPPORT
> > +	help
> > +	  This is a Video4Linux2 sensor driver for the Sony
> > +	  IMX290 camera sensor.
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called imx290.
> > +
> >  config VIDEO_IMX319
> >  	tristate "Sony IMX319 sensor support"
> >  	depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> > index d8ad9dad495d..490e59070407 100644
> > --- a/drivers/media/i2c/Makefile
> > +++ b/drivers/media/i2c/Makefile
> > @@ -111,6 +111,7 @@ obj-$(CONFIG_VIDEO_TC358743)	+= tc358743.o
> >  obj-$(CONFIG_VIDEO_IMX214)	+= imx214.o
> >  obj-$(CONFIG_VIDEO_IMX258)	+= imx258.o
> >  obj-$(CONFIG_VIDEO_IMX274)	+= imx274.o
> > +obj-$(CONFIG_VIDEO_IMX290)	+= imx290.o
> >  obj-$(CONFIG_VIDEO_IMX319)	+= imx319.o
> >  obj-$(CONFIG_VIDEO_IMX355)	+= imx355.o
> >  obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o
> > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> > new file mode 100644
> > index 000000000000..8c50fbd51ca5
> > --- /dev/null
> > +++ b/drivers/media/i2c/imx290.c
> > @@ -0,0 +1,845 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Sony IMX290 CMOS Image Sensor Driver
> > + *
> > + * Copyright (C) 2019 FRAMOS GmbH.
> > + *
> > + * Copyright (C) 2019 Linaro Ltd.
> > + * Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <media/media-entity.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +#define IMX290_STANDBY 0x3000
> > +#define IMX290_REGHOLD 0x3001
> > +#define IMX290_XMSTA 0x3002
> > +#define IMX290_GAIN 0x3014
> > +
> > +static const char * const imx290_supply_name[] = {
> > +	"vdda",
> > +	"vddd",
> > +	"vdddo",
> > +};
> > +
> > +#define IMX290_NUM_SUPPLIES ARRAY_SIZE(imx290_supply_name)
> > +
> > +struct imx290_regval {
> > +	u16 reg;
> > +	u8 val;
> > +};
> > +
> > +struct imx290_mode {
> > +	u32 width;
> > +	u32 height;
> > +	u32 pixel_rate;
> > +	u32 link_freq_index;
> > +
> > +	const struct imx290_regval *data;
> > +	u32 data_size;
> > +};
> > +
> > +struct imx290 {
> > +	struct device *dev;
> > +	struct clk *xclk;
> > +	struct regmap *regmap;
> > +
> > +	struct v4l2_subdev sd;
> > +	struct v4l2_fwnode_endpoint ep;
> > +	struct media_pad pad;
> > +	struct v4l2_mbus_framefmt current_format;
> > +	const struct imx290_mode *current_mode;
> > +
> > +	struct regulator_bulk_data supplies[IMX290_NUM_SUPPLIES];
> > +	struct gpio_desc *rst_gpio;
> > +
> > +	struct v4l2_ctrl_handler ctrls;
> > +	struct v4l2_ctrl *link_freq;
> > +	struct v4l2_ctrl *pixel_rate;
> > +
> > +	struct mutex lock;
> > +};
> > +
> > +struct imx290_pixfmt {
> > +	u32 code;
> > +	u32 colorspace;
> > +};
> > +
> > +static const struct imx290_pixfmt imx290_formats[] = {
> > +	{ MEDIA_BUS_FMT_SRGGB10_1X10, V4L2_COLORSPACE_SRGB, },
> 
> I bet you won't use other colorspaces in this driver. Therefore there's
> there's no need to put it in an array.
> 

Ack.

> > +};
> > +
> > +static struct regmap_config imx290_regmap_config = {
> > +	.reg_bits = 16,
> > +	.val_bits = 8,
> > +	.cache_type = REGCACHE_RBTREE,
> > +};
> > +
> > +static const struct imx290_regval imx290_global_init_settings[] = {
> > +	{ 0x3007, 0x00 },
> > +	{ 0x3009, 0x00 },
> > +	{ 0x3018, 0x65 },
> > +	{ 0x3019, 0x04 },
> > +	{ 0x301a, 0x00 },
> > +	{ 0x3443, 0x03 },
> > +	{ 0x3444, 0x20 },
> > +	{ 0x3445, 0x25 },
> > +	{ 0x3407, 0x03 },
> > +	{ 0x303a, 0x0c },
> > +	{ 0x3040, 0x00 },
> > +	{ 0x3041, 0x00 },
> > +	{ 0x303c, 0x00 },
> > +	{ 0x303d, 0x00 },
> > +	{ 0x3042, 0x9c },
> > +	{ 0x3043, 0x07 },
> > +	{ 0x303e, 0x49 },
> > +	{ 0x303f, 0x04 },
> > +	{ 0x304b, 0x0a },
> > +	{ 0x300f, 0x00 },
> > +	{ 0x3010, 0x21 },
> > +	{ 0x3012, 0x64 },
> > +	{ 0x3016, 0x09 },
> > +	{ 0x3070, 0x02 },
> > +	{ 0x3071, 0x11 },
> > +	{ 0x309b, 0x10 },
> > +	{ 0x309c, 0x22 },
> > +	{ 0x30a2, 0x02 },
> > +	{ 0x30a6, 0x20 },
> > +	{ 0x30a8, 0x20 },
> > +	{ 0x30aa, 0x20 },
> > +	{ 0x30ac, 0x20 },
> > +	{ 0x30b0, 0x43 },
> > +	{ 0x3119, 0x9e },
> > +	{ 0x311c, 0x1e },
> > +	{ 0x311e, 0x08 },
> > +	{ 0x3128, 0x05 },
> > +	{ 0x313d, 0x83 },
> > +	{ 0x3150, 0x03 },
> > +	{ 0x317e, 0x00 },
> > +	{ 0x32b8, 0x50 },
> > +	{ 0x32b9, 0x10 },
> > +	{ 0x32ba, 0x00 },
> > +	{ 0x32bb, 0x04 },
> > +	{ 0x32c8, 0x50 },
> > +	{ 0x32c9, 0x10 },
> > +	{ 0x32ca, 0x00 },
> > +	{ 0x32cb, 0x04 },
> > +	{ 0x332c, 0xd3 },
> > +	{ 0x332d, 0x10 },
> > +	{ 0x332e, 0x0d },
> > +	{ 0x3358, 0x06 },
> > +	{ 0x3359, 0xe1 },
> > +	{ 0x335a, 0x11 },
> > +	{ 0x3360, 0x1e },
> > +	{ 0x3361, 0x61 },
> > +	{ 0x3362, 0x10 },
> > +	{ 0x33b0, 0x50 },
> > +	{ 0x33b2, 0x1a },
> > +	{ 0x33b3, 0x04 },
> > +};
> > +
> > +static const struct imx290_regval imx290_1080p_settings[] = {
> > +	/* mode settings */
> > +	{ 0x3007, 0x00 },
> > +	{ 0x303a, 0x0c },
> > +	{ 0x3414, 0x0a },
> > +	{ 0x3472, 0x80 },
> > +	{ 0x3473, 0x07 },
> > +	{ 0x3418, 0x38 },
> > +	{ 0x3419, 0x04 },
> > +	{ 0x3012, 0x64 },
> > +	{ 0x3013, 0x00 },
> > +	{ 0x305c, 0x18 },
> > +	{ 0x305d, 0x03 },
> > +	{ 0x305e, 0x20 },
> > +	{ 0x305f, 0x01 },
> > +	{ 0x315e, 0x1a },
> > +	{ 0x3164, 0x1a },
> > +	{ 0x3480, 0x49 },
> > +	/* data rate settings */
> > +	{ 0x3009, 0x01 },
> > +	{ 0x3405, 0x10 },
> > +	{ 0x3446, 0x57 },
> > +	{ 0x3447, 0x00 },
> > +	{ 0x3448, 0x37 },
> > +	{ 0x3449, 0x00 },
> > +	{ 0x344a, 0x1f },
> > +	{ 0x344b, 0x00 },
> > +	{ 0x344c, 0x1f },
> > +	{ 0x344d, 0x00 },
> > +	{ 0x344e, 0x1f },
> > +	{ 0x344f, 0x00 },
> > +	{ 0x3450, 0x77 },
> > +	{ 0x3451, 0x00 },
> > +	{ 0x3452, 0x1f },
> > +	{ 0x3453, 0x00 },
> > +	{ 0x3454, 0x17 },
> > +	{ 0x3455, 0x00 },
> > +	{ 0x301c, 0x98 },
> > +	{ 0x301d, 0x08 },
> > +};
> > +
> > +static const struct imx290_regval imx290_720p_settings[] = {
> > +	/* mode settings */
> > +	{ 0x3007, 0x10 },
> > +	{ 0x303a, 0x06 },
> > +	{ 0x3414, 0x04 },
> > +	{ 0x3472, 0x00 },
> > +	{ 0x3473, 0x05 },
> > +	{ 0x3418, 0xd0 },
> > +	{ 0x3419, 0x02 },
> > +	{ 0x3012, 0x64 },
> > +	{ 0x3013, 0x00 },
> > +	{ 0x305c, 0x20 },
> > +	{ 0x305d, 0x00 },
> > +	{ 0x305e, 0x20 },
> > +	{ 0x305f, 0x01 },
> > +	{ 0x315e, 0x1a },
> > +	{ 0x3164, 0x1a },
> > +	{ 0x3480, 0x49 },
> > +	/* data rate settings */
> > +	{ 0x3009, 0x01 },
> > +	{ 0x3405, 0x10 },
> > +	{ 0x3446, 0x4f },
> > +	{ 0x3447, 0x00 },
> > +	{ 0x3448, 0x2f },
> > +	{ 0x3449, 0x00 },
> > +	{ 0x344a, 0x17 },
> > +	{ 0x344b, 0x00 },
> > +	{ 0x344c, 0x17 },
> > +	{ 0x344d, 0x00 },
> > +	{ 0x344e, 0x17 },
> > +	{ 0x344f, 0x00 },
> > +	{ 0x3450, 0x57 },
> > +	{ 0x3451, 0x00 },
> > +	{ 0x3452, 0x17 },
> > +	{ 0x3453, 0x00 },
> > +	{ 0x3454, 0x17 },
> > +	{ 0x3455, 0x00 },
> > +	{ 0x301c, 0xe4 },
> > +	{ 0x301d, 0x0c },
> > +};
> > +
> > +static const struct imx290_regval imx290_10bit_settings[] = {
> > +	{ 0x3005, 0x00},
> > +	{ 0x3046, 0x00},
> > +	{ 0x3129, 0x1d},
> > +	{ 0x317c, 0x12},
> > +	{ 0x31ec, 0x37},
> > +	{ 0x3441, 0x0a},
> > +	{ 0x3442, 0x0a},
> > +	{ 0x300a, 0x3c},
> > +	{ 0x300b, 0x00},
> > +};
> > +
> > +/* supported link frequencies */
> > +static const s64 imx290_link_freq[] = {
> > +	445500000,
> > +};
> > +
> > +/* Mode configs */
> > +static const struct imx290_mode imx290_modes[] = {
> > +	{
> > +		.width = 1920,
> > +		.height = 1080,
> > +		.data = imx290_1080p_settings,
> > +		.data_size = ARRAY_SIZE(imx290_1080p_settings),
> > +		.pixel_rate = 178200000,
> > +		.link_freq_index = 0,
> > +	},
> > +	{
> > +		.width = 1280,
> > +		.height = 720,
> > +		.data = imx290_720p_settings,
> > +		.data_size = ARRAY_SIZE(imx290_720p_settings),
> > +		.pixel_rate = 178200000,
> > +		.link_freq_index = 0,
> > +	},
> > +};
> > +
> > +static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd)
> > +{
> > +	return container_of(_sd, struct imx290, sd);
> > +}
> > +
> > +static inline int imx290_read_reg(struct imx290 *imx290, u16 addr, u8 *value)
> > +{
> > +	u32 regval;
> > +	int ret;
> > +
> > +	ret = regmap_read(imx290->regmap, addr, &regval);
> > +	if (ret) {
> > +		dev_err(imx290->dev, "I2C read failed for addr: %x\n", addr);
> > +		return ret;
> > +	}
> > +
> > +	*value = regval & 0xFF;
> 
> Lower case hexadecimals are preferred.
> 

Ack.

> > +
> > +	return 0;
> > +}
> > +
> > +static int imx290_write_reg(struct imx290 *imx290, u16 addr, u8 value)
> > +{
> > +	int ret;
> > +
> > +	ret = regmap_write(imx290->regmap, addr, value);
> > +	if (ret) {
> > +		dev_err(imx290->dev, "I2C write failed for addr: %x\n", addr);
> > +		return ret;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int imx290_set_register_array(struct imx290 *imx290,
> > +				     const struct imx290_regval *settings,
> > +				     unsigned int num_settings)
> > +{
> > +	unsigned int i;
> > +	int ret;
> > +
> > +	for (i = 0; i < num_settings; ++i, ++settings) {
> > +		ret = imx290_write_reg(imx290, settings->reg, settings->val);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		/* Settle time is 10ms for all registers */
> > +		msleep(10);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int imx290_write_buffered_reg(struct imx290 *imx290, u16 address_low,
> > +				     u8 nr_regs, u32 value)
> > +{
> > +	int ret, i;
> 
> unsigned int i
> 

okay.

> > +	u8 val = 0;
> > +
> > +	ret = imx290_write_reg(imx290, IMX290_REGHOLD, 0x01);
> > +	if (ret) {
> > +		dev_err(imx290->dev, "Error setting hold register\n");
> > +		return ret;
> > +	}
> > +
> > +	for (i = 0; i < nr_regs; i++) {
> > +		imx290_read_reg(imx290, address_low + i, &val);
> 
> What's the purpose of reading back val, and doing the same again below?
> 
> You could do this in a much more simple way in a single write, see e.g.
> ov5670_write_reg().
> 

Right, this came from the vendor driver but looks like we don't need to read
before and after. Will remove the read calls.

> > +		ret = imx290_write_reg(imx290, address_low + i,
> > +				       (u8)(value >> (i * 8)));
> > +		if (ret) {
> > +			dev_err(imx290->dev, "Error writing buffered registers\n");
> > +			return ret;
> > +		}
> > +		imx290_read_reg(imx290, address_low + i, &val);
> > +	}
> > +
> > +	ret = imx290_write_reg(imx290, IMX290_REGHOLD, 0x00);
> > +	if (ret) {
> > +		dev_err(imx290->dev, "Error setting hold register\n");
> > +		return ret;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int imx290_set_gain(struct imx290 *imx290, u32 value)
> > +{
> > +	int ret;
> > +
> > +	u32 adjusted_value = (value * 10) / 3;
> > +
> > +	ret = imx290_write_buffered_reg(imx290, IMX290_GAIN, 1, adjusted_value);
> > +	if (ret)
> > +		dev_err(imx290->dev, "Unable to write gain\n");
> > +
> > +	return ret;
> > +}
> > +
> > +static int imx290_set_power_on(struct imx290 *imx290)
> > +{
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(imx290->xclk);
> > +	if (ret) {
> > +		dev_err(imx290->dev, "Failed to enable clock\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = regulator_bulk_enable(IMX290_NUM_SUPPLIES,
> > +				    imx290->supplies);
> > +	if (ret) {
> > +		dev_err(imx290->dev, "Failed to enable regulators\n");
> > +		goto xclk_off;
> > +	}
> > +
> > +	usleep_range(1, 2);
> > +	gpiod_set_value_cansleep(imx290->rst_gpio, 1);
> > +	usleep_range(30000, 31000);
> > +
> > +	return 0;
> > +
> > +xclk_off:
> > +	clk_disable_unprepare(imx290->xclk);
> > +	return ret;
> > +}
> > +
> > +/* Stop streaming */
> > +static int imx290_stop_streaming(struct imx290 *imx290)
> > +{
> > +	int ret;
> > +
> > +	ret = imx290_write_reg(imx290, IMX290_STANDBY, 0x01);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	msleep(30);
> > +
> > +	return imx290_write_reg(imx290, IMX290_XMSTA, 0x01);
> > +}
> > +
> > +static void imx290_set_power_off(struct imx290 *imx290)
> > +{
> > +	clk_disable_unprepare(imx290->xclk);
> > +	gpiod_set_value_cansleep(imx290->rst_gpio, 0);
> > +	regulator_bulk_disable(IMX290_NUM_SUPPLIES, imx290->supplies);
> > +}
> > +
> > +static int imx290_s_power(struct v4l2_subdev *sd, int on)
> > +{
> > +	struct imx290 *imx290 = to_imx290(sd);
> > +	int ret = 0;
> > +
> > +	mutex_lock(&imx290->lock);
> > +
> > +	if (on) {
> > +		ret = imx290_set_power_on(imx290);
> > +		if (ret < 0)
> > +			goto exit;
> > +
> > +		ret = imx290_set_register_array(imx290,
> > +				imx290_global_init_settings,
> > +				ARRAY_SIZE(imx290_global_init_settings));
> > +		if (ret < 0) {
> > +			dev_err(imx290->dev,
> > +				"Could not set init registers\n");
> > +			imx290_set_power_off(imx290);
> > +			goto exit;
> > +		}
> > +
> > +		imx290_stop_streaming(imx290);
> > +	} else {
> > +		imx290_set_power_off(imx290);
> > +	}
> > +
> > +exit:
> > +	mutex_unlock(&imx290->lock);
> > +
> > +	return ret;
> 
> Please use runtime PM instead. It simplifies the driver. See e.g.
> drivers/media/i2c/ov5670.c or drivers/media/i2c/ov5695.c .
> 

Ack.

> > +}
> > +
> > +static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +	struct imx290 *imx290 = container_of(ctrl->handler,
> > +					     struct imx290, ctrls);
> > +	int ret = 0;
> > +
> > +	mutex_lock(&imx290->lock);
> > +
> > +	switch (ctrl->id) {
> > +	case V4L2_CID_GAIN:
> > +		ret = imx290_set_gain(imx290, ctrl->val);
> > +		break;
> > +	default:
> > +		dev_info(imx290->dev, "ctrl(id:0x%x,val:0x%x) is not handled",
> > +			 ctrl->id, ctrl->val);
> 
> Please return an error; I'd do it instead of printing that.
> 

Okay.

> > +		break;
> > +	}
> > +
> > +	mutex_unlock(&imx290->lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops imx290_ctrl_ops = {
> > +	.s_ctrl = imx290_set_ctrl,
> > +};
> > +
> > +static int imx290_enum_mbus_code(struct v4l2_subdev *sd,
> > +				 struct v4l2_subdev_pad_config *cfg,
> > +				 struct v4l2_subdev_mbus_code_enum *code)
> > +{
> > +	if (code->index >= ARRAY_SIZE(imx290_formats))
> > +		return -EINVAL;
> > +
> > +	code->code = imx290_formats[code->index].code;
> > +
> > +	return 0;
> > +}
> > +
> > +static int imx290_get_fmt(struct v4l2_subdev *sd,
> > +			  struct v4l2_subdev_pad_config *cfg,
> > +			  struct v4l2_subdev_format *fmt)
> > +{
> > +	struct imx290 *imx290 = to_imx290(sd);
> > +	struct v4l2_mbus_framefmt *framefmt;
> > +
> > +	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> > +		framefmt = v4l2_subdev_get_try_format(&imx290->sd, cfg,
> > +						      fmt->pad);
> > +	else
> > +		framefmt = &imx290->current_format;
> > +
> > +	fmt->format = *framefmt;
> > +
> > +	return 0;
> > +}
> > +
> > +static int imx290_set_fmt(struct v4l2_subdev *sd,
> > +			  struct v4l2_subdev_pad_config *cfg,
> > +		      struct v4l2_subdev_format *fmt)
> > +{
> > +	struct imx290 *imx290 = to_imx290(sd);
> > +	const struct imx290_mode *mode;
> > +	struct v4l2_mbus_framefmt *format;
> > +	int i, ret = 0;
> 
> Note that sub-device drivers need to serialise access through the uAPI to
> their own data.
> 

You mean guarding with mutex?

> > +
> > +	mode = v4l2_find_nearest_size(imx290_modes,
> > +				      ARRAY_SIZE(imx290_modes),
> > +				      width, height,
> > +				      fmt->format.width, fmt->format.height);
> > +
> > +	fmt->format.width = mode->width;
> > +	fmt->format.height = mode->height;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(imx290_formats); i++)
> > +		if (imx290_formats[i].code == fmt->format.code)
> > +			break;
> > +
> > +	if (i >= ARRAY_SIZE(imx290_formats))
> > +		i = 0;
> > +
> > +	fmt->format.code = imx290_formats[i].code;
> > +	fmt->format.colorspace = imx290_formats[i].colorspace;
> > +	fmt->format.field = V4L2_FIELD_NONE;
> > +
> > +	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> > +		format = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
> > +	} else {
> > +		format = &imx290->current_format;
> > +		__v4l2_ctrl_s_ctrl(imx290->link_freq, mode->link_freq_index);
> > +		__v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate, mode->pixel_rate);
> > +
> > +		imx290->current_mode = mode;
> > +	}
> > +
> > +	*format = fmt->format;
> > +
> > +	return ret;
> > +}
> > +
> > +static int imx290_entity_init_cfg(struct v4l2_subdev *subdev,
> > +				  struct v4l2_subdev_pad_config *cfg)
> > +{
> > +	struct v4l2_subdev_format fmt = { 0 };
> > +
> > +	fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> > +	fmt.format.width = 1920;
> > +	fmt.format.height = 1080;
> > +
> > +	imx290_set_fmt(subdev, cfg, &fmt);
> > +
> > +	return 0;
> > +}
> > +
> > +static int imx290_write_current_format(struct imx290 *imx290,
> > +				       struct v4l2_mbus_framefmt *format)
> > +{
> > +	int ret;
> > +
> > +	switch (format->code) {
> > +	case MEDIA_BUS_FMT_SRGGB10_1X10:
> > +		ret = imx290_set_register_array(imx290, imx290_10bit_settings,
> > +					ARRAY_SIZE(imx290_10bit_settings));
> > +		if (ret < 0) {
> > +			dev_err(imx290->dev, "Could not set format registers\n");
> > +			return ret;
> > +		}
> > +		break;
> > +	default:
> > +		dev_err(imx290->dev, "Unknown pixel format\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/* Start streaming */
> > +static int imx290_start_streaming(struct imx290 *imx290)
> > +{
> > +	int ret;
> > +
> > +	/* Set current frame format */
> > +	ret = imx290_write_current_format(imx290, &imx290->current_format);
> > +	if (ret < 0) {
> > +		dev_err(imx290->dev, "Could not set frame format\n");
> > +		return ret;
> > +	}
> > +
> > +	/* Apply default values of current mode */
> > +	ret = imx290_set_register_array(imx290, imx290->current_mode->data,
> > +					imx290->current_mode->data_size);
> > +	if (ret < 0) {
> > +		dev_err(imx290->dev, "Could not set current mode\n");
> > +		return ret;
> > +	}
> > +
> > +	/* Apply customized values from user */
> > +	ret = v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler);
> > +	if (ret) {
> > +		dev_err(imx290->dev, "Could not sync v4l2 controls\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = imx290_write_reg(imx290, IMX290_STANDBY, 0x00);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	msleep(30);
> > +
> > +	/* Start streaming */
> > +	return imx290_write_reg(imx290, IMX290_XMSTA, 0x00);
> > +}
> > +
> > +static int imx290_set_stream(struct v4l2_subdev *sd, int enable)
> > +{
> > +	struct imx290 *imx290 = to_imx290(sd);
> > +	int ret = 0;
> > +
> > +	if (enable)
> > +		ret = imx290_start_streaming(imx290);
> > +	else
> > +		imx290_stop_streaming(imx290);
> > +
> > +	return ret;
> > +}
> > +
> > +static int imx290_get_regulators(struct device *dev, struct imx290 *imx290)
> > +{
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < IMX290_NUM_SUPPLIES; i++)
> > +		imx290->supplies[i].supply = imx290_supply_name[i];
> > +
> > +	return devm_regulator_bulk_get(dev, IMX290_NUM_SUPPLIES,
> > +				       imx290->supplies);
> > +}
> > +
> > +static const struct v4l2_subdev_core_ops imx290_subdev_core_ops = {
> > +	.s_power = imx290_s_power,
> > +};
> > +
> > +static const struct v4l2_subdev_video_ops imx290_video_ops = {
> > +	.s_stream = imx290_set_stream,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops imx290_pad_ops = {
> > +	.init_cfg = imx290_entity_init_cfg,
> > +	.enum_mbus_code = imx290_enum_mbus_code,
> > +	.get_fmt = imx290_get_fmt,
> > +	.set_fmt = imx290_set_fmt,
> > +};
> > +
> > +static const struct v4l2_subdev_ops imx290_subdev_ops = {
> > +	.core = &imx290_subdev_core_ops,
> > +	.video = &imx290_video_ops,
> > +	.pad = &imx290_pad_ops,
> > +};
> > +
> > +static const struct media_entity_operations imx290_subdev_entity_ops = {
> > +	.link_validate = v4l2_subdev_link_validate,
> > +};
> > +
> > +static int imx290_probe(struct i2c_client *client,
> > +			const struct i2c_device_id *id)
> > +{
> > +	struct device *dev = &client->dev;
> > +	struct fwnode_handle *endpoint;
> > +	struct imx290 *imx290;
> > +	u32 xclk_freq;
> > +	int ret;
> > +
> > +	imx290 = devm_kzalloc(dev, sizeof(*imx290), GFP_KERNEL);
> > +	if (!imx290)
> > +		return -ENOMEM;
> > +
> > +	imx290->dev = dev;
> > +	imx290->regmap = devm_regmap_init_i2c(client, &imx290_regmap_config);
> > +	if (IS_ERR(imx290->regmap)) {
> > +		dev_err(dev, "Unable to initialize I2C\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
> > +	if (!endpoint) {
> > +		dev_err(dev, "Endpoint node not found\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = v4l2_fwnode_endpoint_parse(endpoint, &imx290->ep);
> > +	fwnode_handle_put(endpoint);
> > +	if (ret < 0) {
> > +		dev_err(dev, "Parsing endpoint node failed\n");
> > +		return ret;
> > +	}
> > +
> > +	/* Only CSI2 is supported for now */
> > +	if (imx290->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {
> > +		dev_err(dev, "Unsupported bus type, should be CSI2\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Set default mode to max resolution */
> > +	imx290->current_mode = &imx290_modes[0];
> > +
> > +	/* get system clock (xclk) */
> > +	imx290->xclk = devm_clk_get(dev, "xclk");
> > +	if (IS_ERR(imx290->xclk)) {
> > +		dev_err(dev, "Could not get xclk");
> > +		return PTR_ERR(imx290->xclk);
> > +	}
> > +
> > +	ret = of_property_read_u32(dev->of_node, "clock-frequency", &xclk_freq);
> > +	if (ret) {
> > +		dev_err(dev, "Could not get xclk frequency\n");
> > +		return ret;
> > +	}
> > +
> > +	/* external clock must be 37.125 MHz */
> > +	if (xclk_freq != 37125000) {
> > +		dev_err(dev, "External clock frequency %u is not supported\n",
> > +			xclk_freq);
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = clk_set_rate(imx290->xclk, xclk_freq);
> > +	if (ret) {
> > +		dev_err(dev, "Could not set xclk frequency\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = imx290_get_regulators(dev, imx290);
> > +	if (ret < 0) {
> > +		dev_err(dev, "Cannot get regulators\n");
> > +		return ret;
> > +	}
> > +
> > +	imx290->rst_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
> > +	if (IS_ERR(imx290->rst_gpio)) {
> > +		dev_err(dev, "Cannot get reset gpio\n");
> 
> Remember to put the regulators from now on. Or grab them later.
> 

Shouldn't that happen by default with devm_regulator* APIs?

> > +		return PTR_ERR(imx290->rst_gpio);
> > +	}
> > +
> > +	mutex_init(&imx290->lock);
> > +
> > +	v4l2_ctrl_handler_init(&imx290->ctrls, 3);
> > +
> > +	v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> > +			  V4L2_CID_GAIN, 0, 72, 1, 0);
> > +	imx290->link_freq = v4l2_ctrl_new_int_menu(&imx290->ctrls,
> > +					&imx290_ctrl_ops,
> > +					V4L2_CID_LINK_FREQ,
> > +					ARRAY_SIZE(imx290_link_freq) - 1,
> > +					0, imx290_link_freq);
> > +	if (imx290->link_freq)
> > +		imx290->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > +	imx290->pixel_rate = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> > +					       V4L2_CID_PIXEL_RATE, 1,
> > +					       INT_MAX, 1,
> > +					       imx290_modes[0].pixel_rate);
> > +
> > +	imx290->sd.ctrl_handler = &imx290->ctrls;
> > +
> > +	if (imx290->ctrls.error) {
> > +		dev_err(dev, "Control initialization error %d\n",
> > +			imx290->ctrls.error);
> > +		ret = imx290->ctrls.error;
> > +		goto free_ctrl;
> > +	}
> > +
> > +	v4l2_i2c_subdev_init(&imx290->sd, client, &imx290_subdev_ops);
> > +	imx290->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > +	imx290->sd.dev = &client->dev;
> > +	imx290->sd.entity.ops = &imx290_subdev_entity_ops;
> > +	imx290->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > +
> > +	imx290->pad.flags = MEDIA_PAD_FL_SOURCE;
> > +	ret = media_entity_pads_init(&imx290->sd.entity, 1, &imx290->pad);
> > +	if (ret < 0) {
> > +		dev_err(dev, "Could not register media entity\n");
> > +		goto free_ctrl;
> > +	}
> > +
> > +	ret = v4l2_async_register_subdev(&imx290->sd);
> > +	if (ret < 0) {
> > +		dev_err(dev, "Could not register v4l2 device\n");
> > +		goto free_entity;
> > +	}
> > +
> > +	return 0;
> > +
> > +free_entity:
> > +	media_entity_cleanup(&imx290->sd.entity);
> > +free_ctrl:
> > +	v4l2_ctrl_handler_free(&imx290->ctrls);
> > +	mutex_destroy(&imx290->lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static int imx290_remove(struct i2c_client *client)
> > +{
> > +	struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > +	struct imx290 *imx290 = to_imx290(sd);
> > +
> > +	v4l2_async_unregister_subdev(sd);
> > +	media_entity_cleanup(&sd->entity);
> > +	v4l2_ctrl_handler_free(sd->ctrl_handler);
> > +
> > +	imx290_set_power_off(imx290);
> > +	mutex_destroy(&imx290->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id imx290_of_match[] = {
> > +	{ .compatible = "sony,imx290" },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, imx290_of_match);
> > +
> > +static struct i2c_driver imx290_i2c_driver = {
> > +	.probe  = imx290_probe,
> 
> Please use probe_new instead.
> 

Ack.

Thanks,
Mani

> > +	.remove = imx290_remove,
> > +	.driver = {
> > +		.name  = "imx290",
> > +		.of_match_table = of_match_ptr(imx290_of_match),
> > +	},
> > +};
> > +
> > +module_i2c_driver(imx290_i2c_driver);
> > +
> > +MODULE_DESCRIPTION("Sony IMX290 CMOS Image Sensor Driver");
> > +MODULE_AUTHOR("FRAMOS GmbH");
> > +MODULE_AUTHOR("Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>");
> > +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: [PATCH 6/7] media: don't do an unsigned int with a 31 bit shift
From: Kees Cook @ 2019-08-29 17:03 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Kate Stewart, Kamil Debski, Hans Verkuil, Andrzej Hajda,
	Lad, Prabhakar, Bluecherry Maintainers, Krzysztof Kozlowski,
	Sylwester Nawrocki, Nathan Chancellor, Andrzej Pietrasiewicz,
	Anton Sviridenko, Ezequiel Garcia, Andrey Utkin, Antti Palosaari,
	Steve Longerbeam, Ismael Luceno, Linux Media Mailing List,
	linux-arm-msm, Stanimir Varbanov, Jeongtae Park,
	linux-samsung-soc, Jacek Anaszewski, Thomas Gleixner,
	Mauro Carvalho Chehab, Mike Isely, linux-arm-kernel, Andy Walls,
	Richard Fontana, Greg Kroah-Hartman, Randy Dunlap, Andy Gross,
	Tomasz Figa, Paul Kocialkowski, Kyungmin Park, Boris Brezillon,
	Kukjin Kim, Sakari Ailus, Colin Ian King
In-Reply-To: <1a78a757b37d2628312e1d56d7a741ba89d42a91.1566502743.git.mchehab+samsung@kernel.org>

On Thu, Aug 22, 2019 at 04:39:33PM -0300, Mauro Carvalho Chehab wrote:
> Doing something like:
> 
> 	i32 foo = 1, bar;
> 
> 	bar = foo << 31;
> 
> has an undefined behavior in C, as warned by cppcheck, as we're
> shifting a signed integer.
> 
> Instead, force the numbers to be unsigned, in order to solve this
> issue.

I also recommend using the BIT() macro, which does the ULing correctly,
etc.

i.e. instead of:

-	keyup = (gpio & ir->mask_keyup) ? 1 << 31 : 0;
+	keyup = (gpio & ir->mask_keyup) ? 1UL << 31 : 0;

use:

-	keyup = (gpio & ir->mask_keyup) ? 1 << 31 : 0;
+	keyup = (gpio & ir->mask_keyup) ? BIT(31) : 0;

-- 
Kees Cook

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox